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

From: Rob Herring <robh@kernel.org>
To: Alexander Dahl <ada@thorsis.com>
Cc: Jeff LaBundy <jeff@labundy.com>,
	Alexander Dahl <post@lespocky.de>, Pavel Machek <pavel@ucw.cz>,
	Dan Murphy <dmurphy@ti.com>,
	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, 6 Oct 2020 17:27:53 -0500	[thread overview]
Message-ID: <20201006222753.GA2965477@bogus> (raw)
In-Reply-To: <3367098.sbkyfNuaKI@ada>

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