Re: [PATCH] checks: Warn about missing #address-cells for interrupt parents

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 3153 bytes --]

On Wed, Dec 18, 2024 at 09:50:27AM -0600, Rob Herring wrote:
> On Tue, Dec 17, 2024 at 8:01 PM Brad Griffis <bgriffis@nvidia.com> wrote:
> >
> > > From: Thierry Reding <treding@nvidia.com>
> > >
> > > 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.
> 
> Yes. That is the only case that really cares and needs it. We used to
> warn on all interrupt-controller nodes if #address-cells was missing.
> But that turns out to be way too many cases to fix, so we dropped that
> part.
> 
> > 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.
> 
> unit-address being 0 and #address-cells being 0 are 2 different
> things.

It's poorly phrased, but I think that was meaning to say that if the
unit address is not needed then #address-cells must be explicitlyzero,
which makes sense.

> > > 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.
> 
> Relying on defaults is not good practice and deprecated for longer
> than we've had DT support on Arm. The default for the kernel on PPC is
> 1 which doesn't agree with dtc nor OpenFirmware. The kernel walking up
> all parents to find #address-cells/#size-cells is not behavior defined
> in any spec either.

Yes, although I believe that behaviour is *very* old, and copied from
/ made to work with old Apple Open Firmwares.

> 6.13 now warns if parent nodes or defaults are used, but not in the
> case of interrupt parsing.
> 
> > > 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.
> 
> I'm more inclined to change the spec if needed. We only need
> #address-cells if we have interrupt-map, so that's all we should
> check.
> 
> If a given binding wants to be stricter, we can do that in its schema.
> 
> Rob
> 

-- 
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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]