Re: [PATCH 1/2] dt: bindings: lp5024: Introduce the lp5024 and lp5018 RGB driver

From: Jacek Anaszewski
Date: Mon Jan 14 2019 - 15:28:10 EST


Dan,

On 1/14/19 9:14 PM, Dan Murphy wrote:
[...]
One last question I am going to add the LP5036 and 30 which have the same technology but slightly different register maps.
Should I rename the driver to LP5036.c as the 30, 24 and 18 would technically be subsets?

How about leds-lp50xx.c ? You can also create a library like
drivers/leds/leds-lp55xx-common.c if that would simplify the code.


A library would be overkill.
Is it just the DT that we don't want to use wild cards in naming?

DT is for concrete board and cpu, so it doesn't make sense to
use wildcards in *.dts file names.

leds-lp50xx.c is a fine name to me.

Apart of that, I've been also mulling over if we shouldn't go for single
"color" sysfs file for setting r,g,b components at one go.
I don't see any downsides. There is no risk that number of elements will
grow, and the benefit will be an atomic way of setting color - the
feature people are looking for. Vesa was mentioning the case where lack
of it had been a real problem [0].


Well thats what I did and have it working. I was going to submit v2 today after I write the documentation.

I basically exposed a "hue" file that takes in a 24 bit R,G,B value and sets the registers accordingly.

I figured hue would be good as that may be the same ABI we have when the RGB framework comes in.

The change over would be transparent to the past users.

I'd prefer "color" over "hue". The latter implies HSV color space, which
was the first thing that came to my mind when reading your message.

--
Best regards,
Jacek Anaszewski