Re: [RESEND PATCH v6 2/3] leds: Add driver for the TLC5925 LED controller - Jean-Jacques Hiblot

From: Jean-Jacques Hiblot <jjhiblot@traphandler.com>
To: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Pavel Machek <pavel@ucw.cz>, Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Linux LED Subsystem <linux-leds@vger.kernel.org>,
	devicetree <devicetree@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [RESEND PATCH v6 2/3] leds: Add driver for the TLC5925 LED controller
Date: Wed, 24 Aug 2022 11:58:13 +0200	[thread overview]
Message-ID: <5bb9955e-4c2f-ca55-0e77-c082a868371a@traphandler.com> (raw)
In-Reply-To: <CAHp75Vf1cT81cx38VQ80PbyG9i9xbiegMnQoMWwZEZf+7fWJuQ@mail.gmail.com>


On 24/08/2022 10:55, Andy Shevchenko wrote:
> On Wed, Aug 24, 2022 at 11:39 AM Jean-Jacques Hiblot
> <jjhiblot@traphandler.com> wrote:
>> On 04/08/2022 23:04, Pavel Machek wrote:
>>> On Thu 2022-08-04 22:23:00, Jean-Jacques Hiblot wrote:
>>>> On 31/07/2022 21:28, Andy Shevchenko wrote:
>>>>> On Fri, Jul 22, 2022 at 10:14 AM Jean-Jacques Hiblot
>>>>> <jjhiblot@traphandler.com> wrote:
> ...
>
>>>>> Sorry for my slowpokeness, but I just realized that this driver may
>>>>> not be needed. What is the difference to existing gpio-74x164?
>>>> It might work. However it might not be as practical and efficient as the
>>>> dedicated LED driver.
>>>>
>>>> I'll give a try.
>>> It is certainly preffered solution. If you decide to re-submit the
>>> driver anyway, please mention that we already have GPIO driver for
>>> compatible chip, and explain why this is superior.
>> sorry for the delay. I tried with the  74x164 gpio driver and it works
>> as expected.
>>
>> The only drawbacks are:
>>
>> - as-is the 74x164 gpio driver supports only one output-enable gpio.
>> However in practice I don't think multiple OE GPIOs will ever be used.
> Let's leave it to the case when it will be needed. So, we can skip this point.
>
>> - with this approach, every time a LED status is changed the whole
>> register has to be sent on the SPI bus. In other words, changes cannot
>> be coalesced.
> But isn't it the same as what you do in your driver? To me it looks
> like you send the entire range of the values each time you change one
> LED's brightness. I don't see any differences with the GPIO driver.
No. The TLC5925 driver updates the register asynchronously: the cached 
value of the register is updated synchronously and then it is 
transferred over SPI using a workqueue. This way if multiple LED are set 
in a short time, the changes are coalesced into a single SPI transfer. 
This is however probably not a must-have feature.
>
>> I don't know if this is enough to make a dedicated TLC5925 driver
>> desirable in the kernel.
> I don't think you have enough justification for a new driver.
>