diff mbox series

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

Message ID 20201005203451.9985-4-post@lespocky.de
State Superseded
Headers show
Series leds: pwm: Make automatic labels work | expand

Commit Message

Alexander Dahl Oct. 5, 2020, 8:34 p.m. UTC
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.example.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(-)

Comments

Jeff LaBundy Oct. 6, 2020, 2:17 a.m. UTC | #1
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.example.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>;
>              };
> -- 
> 2.20.1
> 

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.

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>

Kind regards,
Jeff LaBundy
Alexander Dahl Oct. 6, 2020, 6:34 a.m. UTC | #2
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.

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.

> 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
Rob Herring (Arm) Oct. 6, 2020, 10:28 p.m. UTC | #3
On Mon, 05 Oct 2020 22:34:42 +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.example.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(-)
> 

Acked-by: Rob Herring <robh@kernel.org>
Alexander Dahl Oct. 7, 2020, 10:08 a.m. UTC | #4
Hei Pavel,

Am Mittwoch, 7. Oktober 2020, 12:03:59 CEST schrieb Pavel Machek:
> > 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>
> 
> Okay, so I need Rob's ack here, I guess... and rest of the series is
> not really for my tree.

I think so, too.

I saw you applied patch number 1, all the other patches are DT and from my 
side those can wait for the Acks required and maybe I change the indexes again 
and I'm fine resending all that after the 5.10 merge window.

Thanks for applying the non-DT patches for leds subsystem, the rest will be 
sorted out without hurry.

Greets
Alex
Pavel Machek Oct. 8, 2020, 7:50 a.m. UTC | #5
Hi!

> > > 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.example.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>
> > 
> > Okay, so I need Rob's ack here, I guess... and rest of the series is
> > not really for my tree.
> 
> This patch is not for your tree either. ;)

Ah, right. Usually we have one patch series for one maintainer...

Best regards,
									Pavel
diff mbox series

Patch

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>;
             };