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

[Dan Murphy]

Jacek

On 1/14/19 2:28 PM, Jacek Anaszewski wrote:
> 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.
> 

Done

Dan
-- 
------------------
Dan Murphy