Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use DDI interfaces for interrupt stuff, not PROM ones #93

Open
wants to merge 2 commits into
base: arm64-gate
Choose a base branch
from

Conversation

richlowe
Copy link
Owner

This continues trying to tidy up and integrate DDI/nexus stuff.

I think the big thing is the actual use of ...intx_nintrs and folding the NAVAIL/NINTRS cases together. Which seems to be what other platforms and busses do? I think?

@citrus-it @hadfl @rmustacc

- Fix the implementation of `i_ddi_get_intx_nintrs` which
  had a rough time in recent merging because it's unused.
- Use `i_ddi_get_intx_nintrs` now it works
- Add `i_ddi_interrupt_parent` to get access to a dips interrupt parent
- Use `i_ddi_interrupt_parent` etc, in place of `get_interrupt_cells`
  everywhere
- devices at / shouldn't talk about SPARC
- missing newlines
@hadfl
Copy link

hadfl commented Sep 1, 2024

This works well on the RPI 4.

@richlowe
Copy link
Owner Author

i_ddi_get_intx_nintrs is still wrong in this, let me once again come back around on this

@richlowe
Copy link
Owner Author

@r1mikey This is one of the DDI v. PROM things. Though I left the PR because it was hard to get it all done without having enough of the rest of the system to know it was right.

@@ -1157,21 +1157,49 @@ i_ddi_free_intr_phdl(ddi_intr_handle_impl_t *hdlp)
hdlp->ih_private = NULL;
}


dev_info_t *
i_ddi_interrupt_parent(dev_info_t *pdip)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic here seems at odds with devicetree-specification-v0.4.pdf.

The spec states: Because the hierarchy of the nodes in the interrupt tree might not match the devicetree, the interrupt-parent property is available to make the definition of an interrupt parent explicit. The value is the phandle to the interrupt parent. If this property is missing from a device, its interrupt parent is assumed to be its devicetree
parent
.

So perhaps we should just look for interrupt-parent and, if missing, return ddi_get_parent (or search upwards via that chain for a node with the interrupt-controller boolean property. We should probably also be searching with DDI_PROP_DONTPASS.

Also of interest is this snippet: The root of the interrupt tree is determined when traversal of the interrupt tree reaches an interrupt controller node without an interrupts property and thus no explicit interrupt parent.

"interrupts", &ip, &intrlen) == DDI_SUCCESS) {
intr_sz = ddi_prop_get_int(DDI_DEV_T_ANY, dip,
intr_sz = ddi_prop_get_int(DDI_DEV_T_ANY, ipar,
Copy link

@r1mikey r1mikey Nov 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure this is quite right. The spec states that #interrupt-cells is used at the root of an interrupt domain, which in turn is described as:
An interrupt domain is the context in which an interrupt specifier is interpreted. The root of the domain is either (1) an interrupt controller or(2) an interrupt nexus.

  1. An interrupt controller is a physical device and will need a driver to handle interrupts routed through it. It may also cascade into another interrupt domain. An interrupt controller is specified by the presence of an interrupt-controller property on that node in the devicetree.
  2. An interrupt nexus defines a translation between one interrupt domain and another. The translation is based on both domain-specific and bus-specific information. This translation between domains is performed with the interrupt-map property. For example, a PCI controller device node could be an interrup tnexus that defines a translation from the PCI interrupt namespace (INTA,INTB, etc.) to an interrupt controller with Interrupt Request (IRQ) numbers.

So it all gets very complicated, since we'd really need to traverse up the interrupt hierarchy until we hit the first controller or nexus (i.e. a node that defines #interrupt-cells).

@r1mikey
Copy link

r1mikey commented Nov 18, 2024

This cleans up the current logic nicely, but the current logic doesn't really implement the devicetree spec.

Do we want to land this as a cleanup and improve the implementation later?

@richlowe
Copy link
Owner Author

This cleans up the current logic nicely, but the current logic doesn't really implement the devicetree spec.

Do we want to land this as a cleanup and improve the implementation later?

What I realized when I put this out is what you said above, that basically this was incomplete and needs all the rest of the work -- which is currently underpinning the PCI prototype.

My current goal is to get the interrupt stuff of that much more tasteful, and then get the non-PCI parts up for review, but we'll see how that goes.

It's all tied together with "interrupt-map" support, and all of that stuff

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants