Re: [PATCH v7 03/12] dt-bindings: mfd: Fix schema warnings for pwm-leds

Rob Herring
2020-10-06T22:27:59Z

On Tue, Oct 06, 2020 at 08:34:23AM +0200, Alexander Dahl wrote:
> Hello Jeff,
>
> Am Dienstag, 6. Oktober 2020, 04:17:29 CEST schrieb Jeff LaBundy:
> > Hi Alexander,
> >
> > On Mon, Oct 05, 2020 at 10:34:42PM +0200, Alexander Dahl wrote:
> > > The node names for devices using the pwm-leds driver follow a certain
> > > naming scheme (now). Parent node name is not enforced, but recommended
> > > by DT project.
> > >
> > > DTC Documentation/devicetree/bindings/mfd/iqs62x.example.dt.yaml
> > > CHECK Documentation/devicetree/bindings/mfd/iqs62x.example.dt.yaml
> > >
> > > /home/alex/build/linux/Documentation/devicetree/bindings/mfd/iqs62x.exampl
> > > e.dt.yaml: pwmleds: 'panel' does not match any of the regexes:
> > > '^led(-[0-9a-f]+)?$', 'pinctrl-[0-9]+'>
> > > From schema:
> > > /home/alex/src/linux/leds/Documentation/devicetree/bindings/leds/
> > > leds-pwm.yaml>
> > > Signed-off-by: Alexander Dahl <post@lespocky.de>
> > > ---
> > >
> > > Notes:
> > > v6 -> v7:
> > > * added warning message to commit message (Krzysztof Kozlowski)
> > >
> > > v6:
> > > * added this patch to series
> > >
> > > Documentation/devicetree/bindings/mfd/iqs62x.yaml | 5 +++--
> > > 1 file changed, 3 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/mfd/iqs62x.yaml
> > > b/Documentation/devicetree/bindings/mfd/iqs62x.yaml index
> > > 541b06d80e73..92dc48a8dfa7 100644
> > > --- a/Documentation/devicetree/bindings/mfd/iqs62x.yaml
> > > +++ b/Documentation/devicetree/bindings/mfd/iqs62x.yaml
> > >
> > > @@ -90,10 +90,11 @@ examples:
> > > };
> > >
> > > };
> > >
> > > - pwmleds {
> > > + led-controller {
> > >
> > > compatible = "pwm-leds";
> > >
> > > - panel {
> > > + led-1 {
> > > + label = "panel";
> > >
> > > pwms = <&iqs620a_pwm 0 1000000>;
> > > max-brightness = <255>;
> > >
> > > };
> >
> > I like the consistency this brings. My only feedback is that in the other
> > examples I found (common.yaml and leds-gpio.yaml), the children count off
> > from 0 (e.g. led-0) instead of 1 as your series appears to.
>
> You're right. And that's also the same in leds-lp50xx.yaml and … well I did
> not look close enough, maybe the numbering scheme on the PCB on my desk
> confused me.

If you can tie the numbering to the PCB, then do that.

> Okay, we are already talking about starting index. What about the parent
> node's "led-controller" then in case there are more than one? IIRC Rob
> acknowledged starting from 1 like "led-controller-1", "led-controller-2" and
> so on.

No, I'd assume we start at 0.

> > That's not a huge deal; it simply seems more consistent to count from the
> > first index allowed by the regex (0). The patch is still fine, and so:
> >
> > Acked-by: Jeff LaBundy <jeff@labundy.com>
>
> Thanks.
>
> I'm not sure how many more iterations of this series we will need, at least
> one for the binding license acks I guess, so I could also adapt the child node
> indexes in schema and actual dts files in v8 or so.
>
> Greets
> Alex
>
>
>