On Tue, Jul 08, 2025 at 09:51:55AM +0200, Niklas Söderlund wrote:
> Hi David,
>
> Thanks for your comments.
>
> On 2025-07-08 13:07:12 +1000, David Gibson wrote:
> > On Sun, Jul 06, 2025 at 02:26:38PM +0200, Niklas Söderlund wrote:
> > > The dtc graph_child_address check can't distinguish between bindings
> > > where there can only be a single endpoint, and cases where there can be
> > > multiple endpoints.
> > >
> > > In cases where the bindings allow for multiple endpoints but only one is
> > > described false warnings about unnecessary #address-cells/#size-cells
> > > can be generated, but only if the endpoint described have an address of
> > > 0 (A), for single endpoints with a non-zero address (B) no warnings are
> > > generated.
> > >
> > > A)
> > > ports {
> > > #address-cells = <1>;
> > > #size-cells = <0>;
> > >
> > > port@0 {
> > > #address-cells = <1>;
> > > #size-cells = <0>;
> > >
> > > sourceA: endpoint@0 {
> > > reg = <0>
> > > };
> > > };
> > > };
> > >
> > > B)
> > > ports {
> > > #address-cells = <1>;
> > > #size-cells = <0>;
> > >
> > > port@0 {
> > > #address-cells = <1>;
> > > #size-cells = <0>;
> > >
> > > sourceB: endpoint@1 {
> > > reg = <1>
> > > };
> > > };
> > > };
> > >
> > > Add a comment in the check to document this.
> >
> > Hm. I don't know the graph bindings at all well, so I'll take your
> > word for it on what's happening here. But simply documenting this
> > within the code doesn't seem particularly useful. Someone running dtc
> > will still see the bogus error, and they'd have a pretty long way to
> > go to find this explanation.
>
> It would have been useful for me, I spent a lot of time questioning
> myself on why my dts files produced warnings and where incorrect. I even
> submitted patches to try and work around this issue before learning
> these where false positives. A comment here would have saved me that
> work :-)
Well, true, but you obviously had the wherewithal to track this to the
source in the first place, putting you well ahead of most people, I
think
> I think if the check stays the comment bring some value.
True, but I think we can do better.
> > Probably better to simply remove the check (and maybe comment that it
> > would be nice to check further, but we can't adequately it from a
> > valid case).
>
> I'm OK with removing the check too. This comment was first posted
> together with a change to demote this check to W=2 (instead of W=1) that
> have now been posted separately [1]. I will wait for feedback on that
> and let smarter people then me pick the best way forward.
>
> 1.
> https://lore.kernel.org/all/20250706123243.1050718-1-niklas.soderlund%2Brenesas@xxxxxxxxxxxx/
Right, that's more useful from the point of view of someone building
the kernel. But the underlying fact here is that the check is Just
Plain Wrong - it's giving a warning on a perfectly valid situation.
It should go.
--
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
