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 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html