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

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.

-- 
Kind regards,

Sakari Ailus
sakari.ailus-X3B1VOXEql0@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html