+Cc: GPIO maintainers On Wed, Aug 24, 2022 at 12:58 PM Jean-Jacques Hiblot <jjhiblot@xxxxxxxxxxxxxxx> wrote: > On 24/08/2022 10:55, Andy Shevchenko wrote: > > On Wed, Aug 24, 2022 at 11:39 AM Jean-Jacques Hiblot > > <jjhiblot@xxxxxxxxxxxxxxx> wrote: ... > >> - 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. Ah, thanks for elaboration. But GPIO supports this type of ops. And the more I think about this feature I find it more harmful than useful. The problem is that delayed operation may take an unpredictable amount of time and on the heavily loaded machine the event might be lost (imagine the blinking LED informing about some critical issue and it blinks only once and then, for example, machine reboots or so). I believe we both understand that for the GPIO is a no-go feature for sure, because sequence of the GPIO signals is highly important (imagine bit-banging of any of the protocols). > >> 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. After this message I first must withdraw my Rb tag, and turn my voice against this driver because of the above. On the contrary we might ask the GPIO library for a specific API to have what you do with the user's consent of side effects. Linus, Bart, I'm talking of the delayed (async) version of gpio_set_multiple(). But personally I think it's not so easy to implement in a bugless manner (because we need to synchronize it forcibly at any time we call another GPIO API against the same chip). Summarize this: - you may add a compatible string to the GPIO driver and DT schema, and we are done. -- With Best Regards, Andy Shevchenko
