Re: [PATCH v2.1 2/3] leds: as3645a: Add LED flash class driver

Hi Sakari,

On 08/21/2017 03:53 PM, Sakari Ailus wrote:

Hi Jacek,

Jacek Anaszewski wrote:

quoted

Hi Sakari,

Thanks for the update.
I've noticed that you added node labels to the child device nodes
in [0]:

"as3645a_flash : flash" and "as3645a_indicator : indicator"

The phandle references (as3645a_flash and as3645a_indicator) should
actually be moved to the patch adding the flash property to the sensor
device node. It doesn't do anything here, yet.

quoted

I am still seeing problems with this approach:

1) AFAIK these labels are only used for referencing nodes inside dts
   files and they don't affect the name property of struct device_node

That's right.

quoted

2) Even if you changed the node name from flash to as3645a_flash, you
   would get weird LED class device name "as3645a_flash:flash" in case
   label property is absent. Do you have any objections against the
   approach I proposed in the previous review?:


    snprintf(names->flash, sizeof(names->flash),
	     AS_NAME":%s", node->name);

In the current patch, the device node of the flash controller is used,
postfixed with colon and the name of the LED ("flash" or "indicator") if
no label is defined. In other words, with that DT source you'll have
"as3645a:flash" and "as3645a:indicator". So if you change the name of
the device node of the I²C device, that will be reflected in the label.

If a label exists, then the label is used as such.

I don't really have objections to what you're proposing as such but my
question is: is it useful? With that, the flash and indicator labels
will not come from DT if label properties are undefined. They'll always
be "as3645a:flash" and "as3645a:indicator", independently of the names
of the device nodes.

Ah, indeed, the node->name is put in place of devicename segment and
the node points to the LED controller node. Neat approach, likely to
be adopted as a pattern from now on for all new LED class drivers.


For the patch going through media tree:

Acked-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>

-- 
Best regards,
Jacek Anaszewski