On 03/28/2018 09:48 PM, Oleh Kravchenko wrote:
>
>
> On 28.03.18 22:21, Jacek Anaszewski wrote:
>> On 03/28/2018 08:36 AM, Oleh Kravchenko wrote:
>>> Hello Jacek,
>>>
>>> On 27.03.18 23:58, Jacek Anaszewski wrote:
>>>>> +Example
>>>>> +-------
>>>>> +
>>>>> +led-controller@0 {
>>>>> + compatible = "crane,cr0014114";
>>>>> + reg = <0>;
>>>>> + spi-max-frequency = <50000>;
>>>>> + #address-cells = <1>;
>>>>> + #size-cells = <0>;
>>>>> +
>>>>> + led@0 {
>>>>> + reg = <0>;
>>>>> + label = "cr0:red:coin";
>>>>> + };
>>>>> + led@1 {
>>>>> + reg = <1>;
>>>>> + label = "cr0:green:coin";
>>>>> + };
>>>>> + led@2 {
>>>>> + reg = <2>;
>>>>> + label = "cr0:blue:coin";
>>>>> + };
>>>>> + led@3 {
>>>>> + reg = <3>;
>>>>> + label = "cr1:red:bill";
>>>>> + };
>>>>> + led@4 {
>>>>> + reg = <4>;
>>>>> + label = "cr1:green:bill";
>>>>> + };
>>>>> + led@5 {
>>>>> + reg = <5>;
>>>>> + label = "cr1:blue:bill";
>>>>> + };
>>>>
>>>> Why cr0 and cr1? It should be cr0014114 to stick to the
>>>> current LED naming pattern <devicename:colour:function>.
>>>>
>>>> Nonetheless, we lately came to the conclusion that devicename
>>>> segment is redundant in LED class device name, so until we change
>>>> LED naming convention officially, let's remove devicename segment
>>>> at least from DT and prepend the label with "cr0014114" in the driver.
>>>>
>>>> Please compare how it is approached in [0] (not merged yet).
>>>>
>>>
>>> It's just example.
>>> But anyway our applications works with LEDs by numbers.
>>>
>>> Using function names instead numbers will increase code complexity,
>>> so we use numbers :)
>>
>> This is LED class device naming convention in mainline and examples
>> also must stick to it to keep the things consistent.
>>
>> Please switch labels so that they matched the pattern
>>
>> label = "color:function";
>
>> Why cr0 and cr1? It should be cr0014114 to stick to the
>> current LED naming pattern <devicename:colour:function>.
>
> So how it should be? I'm confused :(
You seem to have overlooked second part of my previous message:
"
Nonetheless, we lately came to the conclusion that devicename
segment is redundant in LED class device name, so until we change
LED naming convention officially, let's remove devicename segment
at least from DT and prepend the label with "cr0014114" in the driver.
Please compare how it is approached in [0] (not merged yet).
"
Please note that DT label is not intended to be used as-is
for LED class device name, but it's been frequently abused so.
Here is the example of how you could modify your code:
DT:
led@0 {
reg = <0>;
label = "red:coin";
};
LED class driver:
#include <uapi/linux/uleds.h>
struct cr0014114_led {
char name[LED_MAX_NAME_SIZE];
struct cr0014114 *priv;
struct led_classdev ldev;
u8 brightness;
};
static int cr0014114_probe_dt(struct cr0014114 *priv)
{
...
const char *str;
...
device_for_each_child_node(priv->dev, child)
{
...
if (fwnode_property_read_string(child, "label", &str))
snprintf(led->name, sizeof(led->name), "cr0014114::"),
else
snprintf(led->name, sizeof(led->name), "cr0014114:%s", str),
I hope it makes all clear now.
>
>>
>> Regardless of how LEDs are named in your port of kernel,
>> they for sure have some functions for the vending machine
>> user. Just use those names in DT :-)
>>
>> Best regards,
>> Jacek Anaszewski
>>
>>>>> +};
>>>>> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
>>>>> index ae850d6c0ad3..f17949c365f5 100644
>>>>> --- a/Documentation/devicetree/bindings/vendor-prefixes.txt
>>>>> +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
>>>>> @@ -75,6 +75,7 @@ cnxt Conexant Systems, Inc.
>>>>> compulab CompuLab Ltd.
>>>>> cortina Cortina Systems, Inc.
>>>>> cosmic Cosmic Circuits
>>>>> +crane Crane Connectivity Solutions
>>>>> creative Creative Technology Ltd
>>>>> crystalfontz Crystalfontz America, Inc.
>>>>> cubietech Cubietech, Ltd.
>>>>>
>>>>
>>>> [0] https://www.spinics.net/lists/devicetree/msg220561.html
>>>>
>>>
>>
>
--
Best regards,
Jacek Anaszewski
