Dan,

On 05/17/2018 04:34 PM, Dan Murphy wrote:
Jacek

On 05/16/2018 04:17 PM, Dan Murphy wrote:
<snip>



+               if (!ret)

if (ret) sounds more natural. And better just to split

+                       snprintf(led->led_name, sizeof(led->led_name),
+                               "%s:%s", led->led_node->name, name);
+               else
+                       snprintf(led->led_name, sizeof(led->led_name),
+                               "%s:torch", led->led_node->name);

const char *tmp;

ret = device_property_read_...(&tmp);
if (ret)
   tmp = ...
sprintf(...);

We're no longer taking devicename section of a LED class device name
from DT, so it will look differently anyway.


So in adding the device_property code I think we again are reaching the LED 
label issue.
In ARM with DT we would take the parent device node name and append it to the 
label

AFAIR this approach (parent DT node name used for devicename ) was incidentally applied in leds-as3645a.c. Soon after that we started
to use led-controller for parent DT name, according to Rob's request.

In the most of LED class drivers this is a child DT node which is used
for devicename, in case label is absent.

if the optional label property was not available.  In migrating to the 
device_property
APIs we don't or can't depend on that parent node anymore.

So for the case where the label property does not exist should we use a hard 
coded name
or should we try to use the name from a device_id table.

This is how we did this for the leds-lp8860 driver.  If the label did not exist 
we used the
i2c_device_id table and pulled the string from there.

i2c_device_id can't be applied as a generic pattern, but only for I2C
hooked devices. Nonetheless since it allows to save few lines of code
in case of drivers supporting a family of chips we can use it.

We are going to get rid of a devicename section from LED class device
name soon anyway, since it is redundant.

--
Best regards,
Jacek Anaszewski

Reply via email to