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

[Jacek Anaszewski <jacek.anaszewski@gmail.com>]

Hi Sakari,

On 08/21/2017 03:53 PM, Sakari Ailus wrote:
> Hi Jacek,
> 
> Jacek Anaszewski wrote:
>> 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.
> 
>>
>> 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.
> 
>> 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