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

From: Alexander Dahl <ada@thorsis.com>
To: Jeff LaBundy <jeff@labundy.com>
Cc: Alexander Dahl <post@lespocky.de>, Pavel Machek <pavel@ucw.cz>,
	Dan Murphy <dmurphy@ti.com>, Rob Herring <robh+dt@kernel.org>,
	Jacek Anaszewski <jacek.anaszewski@gmail.com>,
	linux-leds@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-samsung-soc@vger.kernel.org, linux-omap@vger.kernel.org,
	linux-stm32@st-md-mailman.stormreply.com,
	linux-amlogic@lists.infradead.org, linux-mips@vger.kernel.org,
	Lee Jones <lee.jones@linaro.org>
Subject: Re: [PATCH v7 03/12] dt-bindings: mfd: Fix schema warnings for pwm-leds
Date: Tue, 06 Oct 2020 08:34:23 +0200	[thread overview]
Message-ID: <3367098.sbkyfNuaKI@ada> (raw)
In-Reply-To: <20201006021729.GA4822@labundy.com>

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