On Mon, 28 Jun 2021 11:21:04 +0200
Geert Uytterhoeven [off-list ref] wrote:

Hi Marek,

On Fri, Jun 25, 2021 at 10:39 PM Marek Behun [off-list ref] wrote:

quoted

On Fri, 25 Jun 2021 14:59:02 +0200
Geert Uytterhoeven [off-list ref] wrote:
 

quoted

Instantiate a single LED for a segment display.  This allows the user to
control display brightness and blinking through the LED class API and
triggers, and exposes the display color.
The LED will be named "auxdisplay:<color>:backlight".  

What if there are multiple "auxdisplay"s ?  

I understand the LED core will just add a suffix on a name collision.

quoted

Doesn't this subsystem have IDs? So that you can use auxdisplayN for
device name, for example?  

Auxdisplay does not have IDs, as there is no subsystem to register
with.  It's just a collection of drivers for auxiliary displays with
no common API.  Some drivers use fbdev, others use a chardev, or an
attribute file in sysfs.

BTW, I just followed Pavel's advice in naming.

Very well.

quoted

quoted

+     of_property_read_u32(node, "color", &color);
+     seg->led.name = devm_kasprintf(dev, GFP_KERNEL,
+                     "auxdisplay:%s:" LED_FUNCTION_BACKLIGHT,
+                     color < LED_COLOR_ID_MAX ? led_colors[color] : "");  

If you use devm_led_classdev_register_ext and pass struct
led_init_data, LED core will generate name of the LED itself.  

Will that make any difference, except for adding more code?

You are hardcoding the backlight function. Using the _ext() registering
function will make it so that the function and color are parsed from
fwnode by LED core. I understand that the function will always be
"backlight" in this case, but this should be specified in the
device-tree anyway, so why not use it?

Looking at the implementation, I still have to use devm_kasprintf()
to combine color and function for led_init_data.default_label?

AFAIK you don't have to fill in default_label. (If the needed OF
properties are not present so that default_label is tried, it means the
device-tree does not correctly specify the device. In that case I don't
think it is a problem if the default_label is not present and LED
core will use the OF node name as the LED name.)

The code could look like this

  struct led_init_data init_data = {};

  init_data.fwnode = of_fwnode_handle(node);
  init_data.devicename = "auxdisplay";
  init_data.devname_mandatory = true;

  ...register_ext();

But if you still don't want to do this then ignore me :)

Marek