Re: New 'make dtbs_check W=1' warnings

Re: New 'make dtbs_check W=1' warnings

[Alexandre Belloni]

Hi,

On 08/04/2021 17:08:26+0200, Arnd Bergmann wrote:
> arch/arm/boot/dts/at91-sama5d2_ptc_ek.dt.yaml: /: 'etm@73C000' does
> not match any of the regexes: '@(0|[1-9a-f][0-9a-f]*)$', '^[^@]+$',
> 'pinctrl-[0-9]+'
> arch/arm/boot/dts/at91-kizbox3-hs.dt.yaml: /: 'etm@73C000' does not
> match any of the regexes: '@(0|[1-9a-f][0-9a-f]*)$', '^[^@]+$',
> 'pinctrl-[0-9]+'
> 

This was introduced by 4d930c421e3b ("ARM: dts: at91: sama5d2: add ETB
and ETM unit name"), trying to fix another warning.

I guess this is because
Documentation/devicetree/bindings/arm/coresight.txt is not yaml yet.

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: New 'make dtbs_check W=1' warnings

[Krzysztof Kozlowski]

On 08/04/2021 17:08, Arnd Bergmann wrote:
> Greetings to all Arm platform maintainers,
> 
> I've just gone through the DT merges I've received so far and, with a
> little help from Rob,
> managed to run 'make dtbs_check W=1' before and after, to see what
> warnings we get.
> The good news is that the number of warnings is going down, but
> unfortunately there
> is still an unmanageable amount of remaining warnings, and some new
> ones crept in.
> 
> I'm still working on my tooling for this, to catch these better, but
> ideally I think we should
> try to not introduce new warnings. I think some platforms are already
> clean, and I did
> not see any new warnings for mvebu, samsung and broadcom. There were a lot of
> warnings from .dtsi files, and I probably did an incomplete job at
> deduplicating those.
> 
> See below for the other platforms, and the new warnings that I found.
> If these are
> valid, please send a fixup before the merge window, and let me know if
> you have ideas
> for how we should handle these in the future.
> 
> For this merge window, I don't think any of them are show-stoppers (Rob, let me
> know if you disagree), but in the long run we may want to gradually enforce
> a rule about not merging changes that introduce any new warnings, in order to
> have a chance of cleaning up the existing ones.

+1 for such rule, although the best would be to get a report about new
warnings on posted patches or shortly after applying, e.g. via 0-day
kbuild robot.

Best regards,
Krzysztof

Re: New 'make dtbs_check W=1' warnings

[Linus Walleij]

On Thu, Apr 8, 2021 at 5:08 PM Arnd Bergmann <arnd@kernel.org> wrote:

> arch/arm/boot/dts/ste-href520-tvk.dt.yaml: accelerometer@19:
> interrupts: [[18, 1], [19, 1]] is too long
> arch/arm/boot/dts/ste-hrefprev60-tvk.dt.yaml: gyroscope@68:
> interrupts-extended: [[22, 0, 1], [21, 31, 1]] is too long
> arch/arm/boot/dts/ste-hrefv60plus-tvk.dt.yaml: gyroscope@68:
> interrupts-extended: [[25, 0, 1], [24, 31, 1]] is too long
> arch/arm/boot/dts/ste-hrefv60plus-tvk.dt.yaml: accelerometer@1c:
> interrupts: [[18, 1], [19, 1]] is too long

These warnings are all because the bindings in
Documentation/devicetree/bindings/iio/st,st-sensors.yaml
are slightly incorrect. Several sensors have more than 1 IRQ.

I was working on a refined version of the bindings but got
sidetracked.
https://lore.kernel.org/linux-iio/20210104093343.2134410-1-linus.walleij@linaro.org/

I'll try to get to it.

Yours,
Linus Walleij

Re: New 'make dtbs_check W=1' warnings

[Florian Fainelli]

On 4/8/2021 8:08 AM, Arnd Bergmann wrote:
> Greetings to all Arm platform maintainers,
> 
> I've just gone through the DT merges I've received so far and, with a
> little help from Rob,
> managed to run 'make dtbs_check W=1' before and after, to see what
> warnings we get.
> The good news is that the number of warnings is going down, but
> unfortunately there
> is still an unmanageable amount of remaining warnings, and some new
> ones crept in.
> 
> I'm still working on my tooling for this, to catch these better, but
> ideally I think we should
> try to not introduce new warnings. I think some platforms are already
> clean, and I did
> not see any new warnings for mvebu, samsung and broadcom. There were a lot of
> warnings from .dtsi files, and I probably did an incomplete job at
> deduplicating those.

There are definitively a ton of warnings for Broacom DTS files, a number
of those warnings exist because the bindings were not converted to YAML.
Rafal, do you think you could help me with taking care of the
BCM5301X/4908 warnings?
-- 
Florian

Re: New 'make dtbs_check W=1' warnings

[Rafał Miłecki]

On 2021-04-09 05:37, Florian Fainelli wrote:
> On 4/8/2021 8:08 AM, Arnd Bergmann wrote:
>> Greetings to all Arm platform maintainers,
>> 
>> I've just gone through the DT merges I've received so far and, with a
>> little help from Rob,
>> managed to run 'make dtbs_check W=1' before and after, to see what
>> warnings we get.
>> The good news is that the number of warnings is going down, but
>> unfortunately there
>> is still an unmanageable amount of remaining warnings, and some new
>> ones crept in.
>> 
>> I'm still working on my tooling for this, to catch these better, but
>> ideally I think we should
>> try to not introduce new warnings. I think some platforms are already
>> clean, and I did
>> not see any new warnings for mvebu, samsung and broadcom. There were a 
>> lot of
>> warnings from .dtsi files, and I probably did an incomplete job at
>> deduplicating those.
> 
> There are definitively a ton of warnings for Broacom DTS files, a 
> number
> of those warnings exist because the bindings were not converted to 
> YAML.
> Rafal, do you think you could help me with taking care of the
> BCM5301X/4908 warnings?

Sure, I got rid of one or two warnings already, I'll keep working on 
that.

Re: New 'make dtbs_check W=1' warnings

[Geert Uytterhoeven]

Hi Arnd,

On Thu, Apr 8, 2021 at 5:08 PM Arnd Bergmann <arnd@kernel.org> wrote:
> I've just gone through the DT merges I've received so far and, with a
> little help from Rob,
> managed to run 'make dtbs_check W=1' before and after, to see what
> warnings we get.
> The good news is that the number of warnings is going down, but
> unfortunately there
> is still an unmanageable amount of remaining warnings, and some new
> ones crept in.
>
> I'm still working on my tooling for this, to catch these better, but
> ideally I think we should
> try to not introduce new warnings. I think some platforms are already
> clean, and I did
> not see any new warnings for mvebu, samsung and broadcom. There were a lot of
> warnings from .dtsi files, and I probably did an incomplete job at
> deduplicating those.

Thanks for running these checks!

> See below for the other platforms, and the new warnings that I found.
> If these are
> valid, please send a fixup before the merge window, and let me know if
> you have ideas
> for how we should handle these in the future.
>
> For this merge window, I don't think any of them are show-stoppers (Rob, let me
> know if you disagree), but in the long run we may want to gradually enforce
> a rule about not merging changes that introduce any new warnings, in order to
> have a chance of cleaning up the existing ones.

This may not be as simple as it sounds, as DT binding updates typically
follow a different path than DTS(i) updates.  DT bindings updates may be
picked up by a subsystem maintainer, by Rob, or by the platform
maintainer.
For trivial updates (e.g. adding a compatible value, and sometimes
extending a limit), a DTS(i) update may be accepted by the platform
maintainer before the corresponding DT binding update.  The latter may
even be merged one or more kernel revisions later, especially when
involving subsystems that are not traditionally rooted into platforms
using DT.

Of course we could mention any expected warning regressions in our pull
requests for soc.

> renesas/r8a774a1-beacon-rzg2m-kit.dt.yaml: csi2@feaa0000: ports:
> 'port@0' is a required property

[...]

I've replied to these as a response to your PR reply, see
https://lore.kernel.org/linux-renesas-soc/CAMuHMdWHLnXgBSjP7VKUdx-YNr9rSKFkE5Ge5q_tarU6HP9Lhw@mail.gmail.com/

Gr{oetje,eeting}s,

                        Geert


--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: New 'make dtbs_check W=1' warnings

[Bjorn Andersson]

On Mon 12 Apr 08:14 CDT 2021, Arnd Bergmann wrote:

> On Mon, Apr 12, 2021 at 1:32 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Thu, Apr 8, 2021 at 5:08 PM Arnd Bergmann <arnd@kernel.org> wrote:
> 
> > >
> > > For this merge window, I don't think any of them are show-stoppers (Rob, let me
> > > know if you disagree), but in the long run we may want to gradually enforce
> > > a rule about not merging changes that introduce any new warnings, in order to
> > > have a chance of cleaning up the existing ones.
> >
> > This may not be as simple as it sounds, as DT binding updates typically
> > follow a different path than DTS(i) updates.  DT bindings updates may be
> > picked up by a subsystem maintainer, by Rob, or by the platform
> > maintainer.
> 
> I checked out the bindings from linux-next, which seems to have covered
> most of these. Sometimes it pays off to be lazy and merge them after
> everyone else.
> 
> > For trivial updates (e.g. adding a compatible value, and sometimes
> > extending a limit), a DTS(i) update may be accepted by the platform
> > maintainer before the corresponding DT binding update.  The latter may
> > even be merged one or more kernel revisions later, especially when
> > involving subsystems that are not traditionally rooted into platforms
> > using DT.
> >
> > Of course we could mention any expected warning regressions in our pull
> > requests for soc.
> 
> Right, that would certainly help. Some maintainers send all binding
> updates both to the driver maintainers and to the soc tree, along
> with the other changes that only go into one tree. That is of course
> also more work on your side, but it solves the problem entirely.
> 

So the same binding patch is picked up both in the driver and soc tree?
I was expecting that to cause (harmless) conflicts when things arrive in
Linus' merge queue?

Or are you saying people go the length to create immutable branches for
each binding?


I'm curious because it's fairly often that we introduce clocks,
interconnects etc where the macros from the dt bindings includes aren't
available for another release (so we use numerical constants and then go
back and fix them up later).

Regards,
Bjorn

> > > renesas/r8a774a1-beacon-rzg2m-kit.dt.yaml: csi2@feaa0000: ports:
> > > 'port@0' is a required property
> >
> > [...]
> >
> > I've replied to these as a response to your PR reply, see
> > https://lore.kernel.org/linux-renesas-soc/CAMuHMdWHLnXgBSjP7VKUdx-YNr9rSKFkE5Ge5q_tarU6HP9Lhw@mail.gmail.com/
> 
> Ok, thanks.
> 
>       Arnd

Re: New 'make dtbs_check W=1' warnings

[Bjorn Andersson]

On Mon 12 Apr 13:52 CDT 2021, Arnd Bergmann wrote:

> On Mon, Apr 12, 2021 at 6:01 PM Bjorn Andersson
> <bjorn.andersson@linaro.org> wrote:
> > On Mon 12 Apr 08:14 CDT 2021, Arnd Bergmann wrote:
> > > On Mon, Apr 12, 2021 at 1:32 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > > On Thu, Apr 8, 2021 at 5:08 PM Arnd Bergmann <arnd@kernel.org> wrote:
> >
> > So the same binding patch is picked up both in the driver and soc tree?
> > I was expecting that to cause (harmless) conflicts when things arrive in
> > Linus' merge queue?
> >
> > Or are you saying people go the length to create immutable branches for
> > each binding?
> 
> I think it's usually one immutable branch for all the bindings of a given
> merge window. This avoids the merge conflicts, and you can add further
> bindings on the same branch before sending it off to the soc tree.
> 

That would be convenient to have, but the binding changes we depend on
in a given window (in particular if dtbs_check is expected to pass) is
scattered over a wide range of maintainer trees.

> > I'm curious because it's fairly often that we introduce clocks,
> > interconnects etc where the macros from the dt bindings includes aren't
> > available for another release (so we use numerical constants and then go
> > back and fix them up later).
> 
> Ah right, it is particularly bad for platforms that don't have a regular
> layout in these blocks and need to define a new constant every time
> another clock/reset/pin/... is discovered in the downstream sources.
> 

Even blocks following some standardized layout has this problem, because
each platform will have it's own (often similar) set of
clocks/resets/pins.

And going back to dtbs_check, you will continue to see the warnings
about missing compatibles, because most of the case they won't end up in
the soc tree.

> I was mainly referring to the simpler problem of defining a binding
> document for a device once, and then merging the nodes.
> 

I'm sure we all love the hardware that's simple to translate to a DT
binding, unfortunately though we're dealing with complex SoCs.

Regards,
Bjorn