Re: [PATCH v3 1/2] dt-bindings: leds: Add binding for spi-byte LED.

[Christian Mauderer <oss@c-mauderer.de>]

Hello Jacek,

On 10/05/2019 22:42, Jacek Anaszewski wrote:
> Hi Christian,
> 
> On 5/10/19 9:50 PM, Christian Mauderer wrote:
>> On 07/05/2019 11:52, Christian Mauderer wrote:
>>> On 06/05/2019 22:25, Pavel Machek wrote:
>>>> Hi!
>>>>
>>>>>>> Ok, I'm afraid I caused this. What should the compatible be, then?
>>>>>>
>>>>>> Knowing nothing about the h/w other than the above description:
>>>>>> ubiquiti,aircube-leds
>>>>>>
>>>>>> Not sure if that's a registered or correct vendor prefix though.
>>>>>>
>>>>>> Rob
>>>>>>
>>>>>
>>>>> Where would such a vendor prefix be registered? Does that mean that
>>>>> only
>>>>> the vendor is allowed to use it? In that case: How would a reverse
>>>>> engineered prefix look like?
>>>>
>>>> You can use it, too. It is in
>>>> Documentation/devicetree/bindings/vendor-prefixes.txt :
>>>>
>>>> ubnt��� Ubiquiti Networks
>>>>
>>>> So you can probably use ubnt, prefix.
>>>>
>>>>> (still with some missing parts like U-Boot) about two weeks later.
>>>>> I had
>>>>> a look at it and they are not using a device tree. So there is no
>>>>> "official" string that I could deduce from that archive.
>>>>
>>>> Mainline is the master. You are more "official" than them ;-).
>>>> ����������������������������������� Pavel
>>>>
>>>
>>> Hello
>>>
>>> let me summarize the direction before I create a v4:
>>>
>>> Rob Herring suggested "ubnt,acb-spi-led" for the binding name in his
>>> Mail from 06.05.2019 17:59 UTC. If no one objects, I'll use that.
>>>
>>> With the more specific name I'll remove the off-value and max-value from
>>> the device tree. Instead I'll create some look up table in the driver.
>>> based on the name or go back to the defines like in the v1 patch. What
>>> kind of solution would be preferable depends on the next question:
>>>
>>> How should I name the driver? Should I use a device specific name like
>>> in v1 again (most likely now acb-spi-led)? That would allow to
>>> potentially add a hardware supported blinking in that driver. The
>>> alternative would be the more generic name that it has now
>>> (leds-spi-byte) without any plans to add the blinking but it could be
>>> potentially used for example for a digital potentiometer based
>>> brightness setting.
>>>
>>> Note that I didn't really had planned to implement the blinking support
>>> because I don't have a use case for it. So it would be either a feature
>>> that I would add because someone insists. Or it could be added in the
>>> future by a user who wants that feature (maybe Ubiquiti when they
>>> upgrade their kernel?).
>>>
>>> If it is a required feature for that driver: Please note that although
>>> of course I would do some basic tests during development it would be a
>>> mostly unused and therefore untested feature.
>>>
>>> Best regards
>>>
>>> Christian
>>>
>>
>> Hello,
>>
>> sorry for repeating my question. I assume I wrote to much text hiding
>> it: How should I name the driver?
>>
>> The name for the binding is clear (ubnt,acb-spi-led). Only the driver is
>> left (keep leds-spi-byte or rename to leds-ubnt-acb-spi or something
>> else).
> 
> Why leds-spi-byte name would prevent addition of blink support? It can
> be always added basing on OF compatible. If it is to be generic SPI
> byte driver, then I'd use leds-spi-byte. Actually also the things
> like allowed brightness levels could be determined basing on that,
> and not in device tree, but in the driver.
> 
> Please compare how e.g. drivers/leds/leds-is31fl32xx.c accomplishes
> that.
> 

I would have expected that adding a lot of device specific code (in that
case blinking) to a multi-purpose driver would be bad style. But I'll go
for the generic name if that is the accepted way. I already mentioned
multiple times that my target is currently only the brightness. So the
device specific code maybe is added quite a bit in the future anyway in
which case it would still be possible to rename a part (if it isn't used
otherwise) or at least split it into it's own c-file.

I'll prepare a v4 in the near future and send it to the list. I only
learned that it would be a good idea to wait for at least a day for some
other opinions before doing that ;-)

Best regards

Christian