On Tue, Dec 17, 2024 at 06:01:16PM -0800, Brad Griffis wrote: > > From: Thierry Reding <treding@xxxxxxxxxx> > > > > The device tree specification (v0.4) suggests that #address-cells is > > mandatory for interrupt parent nodes. If this property is missing, Linux > > will default to the value of 0. > > Just to clarify, this relates to interrupt-map specifically. In that > scenario the device tree spec requires that both the child node and parent > node specify #address-cells and #interrupt-cells. It further specifies if a > unit address component is not needed then it must be explicitly defined as > zero. In other words, this does not seem to be just a suggestion, but more > of a firm requirement. > > > A number of device tree files rely on Linux' fallback and don't specify > > an explicit #address-cells as suggested by the specification. This can > > cause issues when these device trees are passed to software with a more > > pedantic interpretation of the DT spec. > > The device tree spec also says that in the case where #address-cells is not > specified that a value of 2 should be assumed. So in this context, I find > the kernel's current practice of assuming #address-cells = <0> also violates > the spec. Possibly. However the assumed value of 2 is generally in the context of 'ranges' and 'reg' properties, so there's at least some argument that the kernel doing something different in the context of interrupt maps is acceptable. > > Add a warning when this case is detected so that device tree files can > > be fixed. > > I think a warning is reasonable, but perhaps we should consider making it an > outright error. Though given the number of impacted device trees, perhaps > that needs to be done in a couple of steps. So, the suggested implementation adds it as another part of the check_interrupt_map() logic, which is a warning (by default). There's no way of different sub-tests within a single named "check" have different priority levels. To do that you'd need to implement this specific test as a separate check with its own entry in the table. That wouldn't be an unreasonable thing to do, but it would be trickier and probably require duplicating some other logic, so I can't really blame Thierry for doing it this way. Note that you can switch the interrupt map check as a whole to be a full error with the command line option -Einterrupt_map -- David Gibson (he or they) | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you, not the other way | around. http://www.ozlabs.org/~dgibson
Attachment:
signature.asc
Description: PGP signature
