Hi Jacek,
Jacek Anaszewski wrote:
> 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@xxxxxxxxx>
Thanks!
I sent a new version of the DT source patch for N9 / N950; I'll proceed
to send a pull request tomorrow / the day after unless there are further
comments.
--
Regards,
Sakari Ailus
sakari.ailus@xxxxxx
