Re: [PATCH 2/2] dt-bindings: leds: Add aw21024 binding

From: Kyle Swenson <kyle.swenson@est.tech>
To: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Cc: pavel@ucw.cz, robh+dt@kernel.org, krzk+dt@kernel.org,
	linux-leds@vger.kernel.org, devicetree@vger.kernel.org
Subject: Re: [PATCH 2/2] dt-bindings: leds: Add aw21024 binding
Date: Wed, 18 May 2022 15:18:16 -0600	[thread overview]
Message-ID: <YoVimFUOy581rJ9W@p620> (raw)
In-Reply-To: <2c9b80ef-74c6-bd60-cfc9-d69349cdf6b1@linaro.org>

On Wed, May 18, 2022 at 10:17:21AM +0200, Krzysztof Kozlowski wrote:
> On 17/05/2022 20:31, Kyle Swenson wrote:
> >>> +
> >>> +            multi-led@1 {
> >>> +                #address-cells = <1>;
> >>> +                #size-cells = <2>;
> >>> +                reg = <0x0 0x1 0x2>;
> >>
> >> This is confusing. Does not match unit address and address/size cells.
> >> Perhaps you wanted three separate regs?
> > The wrong address and size cells and not matching the unit address is a
> > mistake on my part, and the next version will actually pass make
> > dt_binding_check.
> > 
> > That said, it's not clear to me how best to handle a combination of
> > multi-leds and individual LEDs on a particular board. For example, a
> > particular board with this driver might have the first six outputs
> > connected to two RGB LEDs, and then the remainder of the outputs
> > connected to individual LEDs.
> > 
> > My (poor) attempt at handling this resulted in this approach where I
> > (ab)used the 'reg' property to be able to address each individual LED of
> > a multi-led.  I'm sure this problem has been solved before, but I'm
> > struggling finding a driver in the tree that has solved it.
> > 
> > Any advice or pointers will be welcome, and in the mean time I'll plan
> > on fixing the (now obvious) issues with the binding.  At the very least,
> > cleaning up the binding will make the problem I'm trying to solve more
> > clear.
> 
> The immediate solution to the DTS reg issue is to use the same unit
> address, so:
> 
> multi-led@0 {
> 	reg = <0x0>, <0x1>, <0x2>;
> }
> 
> However your case is partially (or entirely) covered by multicolor LEDs.
> You should add allOf:$ref with reference to leds-class-multicolor.yaml.
> I see exactly your pattern being used there - just the fixed one, I
> think. I'll send a patch for it and put you on Cc.

I suspect you're right: mutlicolor LEDs will do exactly what I need and
the patch you cc'd me on teaches me how to specify it in the DTS.  I'll
make the changes and send up a v2 in a few days.

> 
> Best regards,
> Krzysztof

Thanks again for your time and guidance.  I happen to have a board with that
lp50xx LED controller and I'll be happy to test out the example DTS from the
binding if you'd like.

Thanks,
Kyle