Re: [PATCH v9 05/15] leds: multicolor: Introduce a multicolor class definition

From: Jacek Anaszewski
Date: Sat Sep 28 2019 - 06:10:58 EST

Next message: Dominik Brodowski: "[RFC] random: UEFI RNG input is bootloader randomness"
Previous message: Masami Hiramatsu: "[PATCH] tracing/probe: Fix to check the difference of nr_args before adding probe"
In reply to: Dan Murphy: "Re: [PATCH v9 05/15] leds: multicolor: Introduce a multicolor class definition"
Next in thread: Jacek Anaszewski: "Re: [PATCH v9 05/15] leds: multicolor: Introduce a multicolor class definition"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Dan,

On 9/26/19 1:52 PM, Dan Murphy wrote:
> Jacek
>
> On 9/25/19 4:12 PM, Jacek Anaszewski wrote:
>> Dan,
>>
>> On 9/25/19 7:46 PM, Dan Murphy wrote:
>>> Introduce a multicolor class that groups colored LEDs
>>> within a LED node.
>>>
>>> The multi color class groups monochrome LEDs and allows controlling two
>>> aspects of the final combined color: hue and lightness. The former is
>>> controlled via <color>_intensity files and the latter is controlled
>>> via brightness file.
>>>
>>> Signed-off-by: Dan Murphy <dmurphy@xxxxxx>
>>> ---
>>> Â drivers/leds/KconfigÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ |Â 10 ++
>>> Â drivers/leds/MakefileÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ |ÂÂ 1 +
>>> Â drivers/leds/led-class-multicolor.cÂ | 220 +++++++++++++++++++++++++++
>>> Â include/linux/led-class-multicolor.h |Â 74 +++++++++
>>> Â 4 files changed, 305 insertions(+)
>>> Â create mode 100644 drivers/leds/led-class-multicolor.c
>>> Â create mode 100644 include/linux/led-class-multicolor.h
>>>
>>> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
>>> index 6e7703fd03d0..cfb1ebb6517f 100644
>>> --- a/drivers/leds/Kconfig
>>> +++ b/drivers/leds/Kconfig
>>> @@ -30,6 +30,16 @@ config LEDS_CLASS_FLASH
>>> ÂÂÂÂÂÂÂ for the flash related features of a LED device. It can be built
>>> ÂÂÂÂÂÂÂ as a module.
>>> Â +config LEDS_CLASS_MULTI_COLOR
>>> +ÂÂÂ tristate "LED Mulit Color LED Class Support"
>>> +ÂÂÂ depends on LEDS_CLASS
>>> +ÂÂÂ help
>>> +ÂÂÂÂÂ This option enables the multicolor LED sysfs class in
>>> /sys/class/leds.
>>> +ÂÂÂÂÂ It wraps LED class and adds multicolor LED specific sysfs
>>> attributes
>>> +ÂÂÂÂÂ and kernel internal API to it. You'll need this to provide
>>> support
>>> +ÂÂÂÂÂ for multicolor LEDs that are grouped together. This class is not
>>> +ÂÂÂÂÂ intended for single color LEDs. It can be built as a module.
>>> +
>>> Â config LEDS_BRIGHTNESS_HW_CHANGED
>>> ÂÂÂÂÂ bool "LED Class brightness_hw_changed attribute support"
>>> ÂÂÂÂÂ depends on LEDS_CLASS
>>> diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
>>> index 2da39e896ce8..841038cfe35b 100644
>>> --- a/drivers/leds/Makefile
>>> +++ b/drivers/leds/Makefile
>>> @@ -4,6 +4,7 @@
>>> Â obj-$(CONFIG_NEW_LEDS)ÂÂÂÂÂÂÂÂÂÂÂ += led-core.o
>>> Â obj-$(CONFIG_LEDS_CLASS)ÂÂÂÂÂÂÂ += led-class.o
>>> Â obj-$(CONFIG_LEDS_CLASS_FLASH)ÂÂÂÂÂÂÂ += led-class-flash.o
>>> +obj-$(CONFIG_LEDS_CLASS_MULTI_COLOR)ÂÂÂ += led-class-multicolor.o
>>> Â obj-$(CONFIG_LEDS_TRIGGERS)ÂÂÂÂÂÂÂ += led-triggers.o
>>> Â Â # LED Platform Drivers
>>> diff --git a/drivers/leds/led-class-multicolor.c
>>> b/drivers/leds/led-class-multicolor.c
>>> new file mode 100644
>>> index 000000000000..25371bd9a860
>>> --- /dev/null
>>> +++ b/drivers/leds/led-class-multicolor.c
>>> @@ -0,0 +1,220 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +// LED Multi Color class interface
>>> +// Copyright (C) 2019 Texas Instruments Incorporated -
>>> http://www.ti.com/
>>> +
>>> +#include <linux/device.h>
>>> +#include <linux/init.h>
>>> +#include <linux/led-class-multicolor.h>
>>> +#include <linux/module.h>
>>> +#include <linux/slab.h>
>>> +#include <linux/uaccess.h>
>>> +
>>> +#include "leds.h"
>>> +
>>> +#define INTENSITY_NAMEÂÂÂÂÂÂÂ "_intensity"
>>> +#define MAX_INTENSITY_NAMEÂÂÂ "_max_intensity"
>>> +
>>> +void led_mc_calc_brightness(struct led_classdev_mc *mcled_cdev,
>>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ enum led_brightness brightness,
>>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ int brightness_val[])
>>> +{
>>> +ÂÂÂ struct led_mc_color_entry *priv;
>>> +ÂÂÂ int i = 0;
>>> +
>>> +ÂÂÂ list_for_each_entry(priv, &mcled_cdev->color_list, list) {
>> I think we should have some way to let the caller know exact mapping
>> of brightness_val to color_id. Possibly LED mc core should provide
>> a helper to get color_id by color entry index.
>
> Why would we need that?

To make LED mc API more straightforward and avoid making
clients wondering in what order colors will be assigned to array
elements. And finally forcing them to go and analyze LED mc core.

>> But this remark is actually more relevant to the place of calling.
>> I'll try to propose something there.
>>
>> And regarding brightness_val name - how about:
>>
>> s/brightness_val/brightness_component/ ?
>
> component does not make sense to me in this context.Â Actually
> brightness_val
>
> does not make sense either since it is an adjusted intensity and
> brightness is passed in in the second arg
>
> I think intensity_values make more sense.

Every variable carries some value. so this is meaningless.
Intensity OTOH can be confused with <color>_intensity, which
is not going to be written to the hardware, but used
as a factor for calculating final LED iout brightness.

The aim of the equation is to multiple the ratio of
color intensity to its max intensity by global brightness.
The resulting value is written to the hardware to set
brightness of a LED being one of the components producing
the final final combined color.

I had to come up with literal description of the equation
to facilitate devising better variable name :-)

Having the above, how about "color_component" ?

>>
>>
>>> +ÂÂÂÂÂÂÂ brightness_val[i] = brightness *
>>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ priv->intensity / priv->max_intensity;

Now, looking closer at this equation it becomes obvious to me
why Pavel opted for common max_brightness: in case of
<color>_intensity == <color>_max_intensity we will get the result
equal to max_brightness. And this is problematic: think of a case when
calculated value will be greater than given <color>_max_intensity.
It will have to be limited the that constraint and will fail to
produce proper color combination.

As a solution I see limiting max_brightness to the lowest supported
brightness from the LEDs in the cluster.

>>> +ÂÂÂÂÂÂÂ i++;
>>> +ÂÂÂ }
>>> +}
>>> +EXPORT_SYMBOL_GPL(led_mc_calc_brightness);
>>> +
>>> +static ssize_t intensity_store(struct device *dev,
>>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ struct device_attribute *intensity_attr,
>>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ const char *buf, size_t size)
>>> +{
>>> +ÂÂÂ struct led_mc_color_entry *priv = container_of(intensity_attr,
>>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ struct led_mc_color_entry,
>>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ intensity_attr);
>>> +ÂÂÂ struct led_classdev *led_cdev = priv->mcled_cdev->led_cdev;
>>> +ÂÂÂ unsigned long value;
>>> +ÂÂÂ ssize_t ret;
>>> +
>>> +ÂÂÂ mutex_lock(&led_cdev->led_access);
>>> +
>>> +ÂÂÂ ret = kstrtoul(buf, 10, &value);
>>> +ÂÂÂ if (ret)
>>> +ÂÂÂÂÂÂÂ goto unlock;
>>> +
>>> +ÂÂÂ if (value > priv->max_intensity) {
>>> +ÂÂÂÂÂÂÂ ret = -EINVAL;
>>> +ÂÂÂÂÂÂÂ goto unlock;
>>> +ÂÂÂ }
>>> +
>>> +ÂÂÂ priv->intensity = value;
>>> +ÂÂÂ ret = size;
>>> +
>>> +unlock:
>>> +ÂÂÂ mutex_unlock(&led_cdev->led_access);
>>> +ÂÂÂ return ret;
>>> +}
>>> +
>>> +static ssize_t intensity_show(struct device *dev,
>>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ struct device_attribute *intensity_attr,
>>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ char *buf)
>>> +{
>>> +ÂÂÂ struct led_mc_color_entry *priv = container_of(intensity_attr,
>>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ struct led_mc_color_entry,
>>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ intensity_attr);
>>> +
>>> +ÂÂÂ return sprintf(buf, "%d\n", priv->intensity);
>>> +}
>>> +
>>> +static ssize_t max_intensity_show(struct device *dev,
>>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ struct device_attribute *max_intensity_attr,
>>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ char *buf)
>>> +{
>>> +ÂÂÂ struct led_mc_color_entry *priv = container_of(max_intensity_attr,
>>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ struct led_mc_color_entry,
>>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ max_intensity_attr);
>>> +
>>> +ÂÂÂ return sprintf(buf, "%d\n", priv->max_intensity);
>>> +}
>>> +
>>> +static struct attribute *led_color_attrs[] = {
>>> +ÂÂÂ NULL,
>>> +};
>>> +
>>> +static struct attribute_group led_color_group = {
>>> +ÂÂÂ .name = "colors",
>>> +ÂÂÂ .attrs = led_color_attrs,
>>> +};
>>> +
>>> +static int led_multicolor_init_color(struct led_classdev_mc
>>> *mcled_cdev,
>>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ int color_id, int color_index)
>>> +{
>>> +ÂÂÂ struct led_classdev *led_cdev = mcled_cdev->led_cdev;
>>> +ÂÂÂ struct led_mc_color_entry *mc_priv;
>>> +ÂÂÂ char *intensity_file_name;
>>> +ÂÂÂ char *max_intensity_file_name;
>>> +ÂÂÂ size_t len;
>>> +ÂÂÂ int ret;
>>> +
>>> +ÂÂÂ mc_priv = devm_kzalloc(led_cdev->dev, sizeof(*mc_priv),
>>> GFP_KERNEL);
>>> +ÂÂÂ if (!mc_priv)
>>> +ÂÂÂÂÂÂÂ return -ENOMEM;
>>> +
>>> +ÂÂÂ mc_priv->led_color_id = color_id;
>>> +ÂÂÂ mc_priv->mcled_cdev = mcled_cdev;
>>> +
>>> +ÂÂÂ sysfs_attr_init(&mc_priv->intensity_attr.attr);
>>> +ÂÂÂ len = strlen(led_colors[color_id]) + strlen(INTENSITY_NAME) + 1;
>>> +ÂÂÂ intensity_file_name = kzalloc(len, GFP_KERNEL);
>>> +ÂÂÂ if (!intensity_file_name)
>>> +ÂÂÂÂÂÂÂ return -ENOMEM;
>>> +
>>> +ÂÂÂ snprintf(intensity_file_name, len, "%s%s",
>>> +ÂÂÂÂÂÂÂÂ led_colors[color_id], INTENSITY_NAME);
>>> +ÂÂÂ mc_priv->intensity_attr.attr.name = intensity_file_name;
>>> +ÂÂÂ mc_priv->intensity_attr.attr.mode = 644;
>> Proper octal value should begin with 0.
>> But please use combinations of dedicated S_I* definitions
>> from include/uapi/linux/stat.h.
>
> Using the S_I* causes checkpatch warnings
>
> WARNING: Symbolic permissions 'S_IRUGO' are not preferred. Consider
> using octal permissions '0444'.
> #139: FILE: drivers/leds/led-class-multicolor.c:139:
> +ÂÂÂ mc_priv->max_intensity_attr.attr.mode = S_IRUGO;

Ah, right. Anyway, you need to add leading 0.

>>
>>> +ÂÂÂ mc_priv->intensity_attr.store = intensity_store;
>>> +ÂÂÂ mc_priv->intensity_attr.show = intensity_show;
>>> +ÂÂÂ ret = sysfs_add_file_to_group(&led_cdev->dev->kobj,
>>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ &mc_priv->intensity_attr.attr,
>>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ led_color_group.name);
>>> +ÂÂÂ if (ret)
>>> +ÂÂÂÂÂÂÂ goto intensity_err_out;
>>> +
>>> +ÂÂÂ sysfs_attr_init(&mc_priv->max_intensity_attr.attr);
>>> +ÂÂÂ len = strlen(led_colors[color_id]) + strlen(MAX_INTENSITY_NAME)
>>> + 1;
>>> +ÂÂÂ max_intensity_file_name = kzalloc(len, GFP_KERNEL);
>>> +ÂÂÂ if (!max_intensity_file_name) {
>>> +ÂÂÂÂÂÂÂ ret = -ENOMEM;
>>> +ÂÂÂÂÂÂÂ goto intensity_err_out;
>>> +ÂÂÂ }
>>> +
>>> +ÂÂÂ snprintf(max_intensity_file_name, len, "%s%s",
>>> +ÂÂÂÂÂÂÂÂ led_colors[color_id], MAX_INTENSITY_NAME);
>>> +ÂÂÂ mc_priv->max_intensity_attr.attr.name = max_intensity_file_name;
>>> +ÂÂÂ mc_priv->max_intensity_attr.attr.mode = 444;
>>> +ÂÂÂ mc_priv->max_intensity_attr.show = max_intensity_show;
>>> +ÂÂÂ ret = sysfs_add_file_to_group(&led_cdev->dev->kobj,
>>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ &mc_priv->max_intensity_attr.attr,
>>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ led_color_group.name);
>>> +ÂÂÂ if (ret)
>>> +ÂÂÂÂÂÂÂ goto max_intensity_err_out;
>>> +
>>> +ÂÂÂ mc_priv->max_intensity = LED_FULL;
>>> +ÂÂÂ list_add_tail(&mc_priv->list, &mcled_cdev->color_list);
>>> +
>>> +max_intensity_err_out:
>>> +ÂÂÂ kfree(max_intensity_file_name);
>>> +intensity_err_out:
>>> +ÂÂÂ kfree(intensity_file_name);
>>> +ÂÂÂ return ret;
>>> +}
>>> +
>>> +static int led_multicolor_init_color_dir(struct led_classdev_mc
>>> *mcled_cdev)
>>> +{
>>> +ÂÂÂ struct led_classdev *led_cdev = mcled_cdev->led_cdev;
>>> +ÂÂÂ int ret;
>>> +ÂÂÂ int i, color_index = 0;
>>> +
>>> +ÂÂÂ ret = sysfs_create_group(&led_cdev->dev->kobj, &led_color_group);
>>> +ÂÂÂ if (ret)
>>> +ÂÂÂÂÂÂÂ return ret;
>>> +
>>> +ÂÂÂ for (i = 0; i < LED_COLOR_ID_MAX; i++) {
>>> +ÂÂÂÂÂÂÂ if (test_bit(i, &mcled_cdev->available_colors)) {
>>> +ÂÂÂÂÂÂÂÂÂÂÂ ret = led_multicolor_init_color(mcled_cdev, i,
>>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ color_index);
>> color_index is now unused AFAICS.
>
> Ack
>
>>
>>> +ÂÂÂÂÂÂÂÂÂÂÂ if (ret)
>>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ break;
>>> +
>>> +ÂÂÂÂÂÂÂÂÂÂÂ color_index++;
>>> +ÂÂÂÂÂÂÂ }
>>> +ÂÂÂ }
>>> +
>>> +ÂÂÂ return ret;
>>> +}
>>> +
>>> +int led_classdev_multicolor_register_ext(struct device *parent,
>>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ struct led_classdev_mc *mcled_cdev,
>>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ struct led_init_data *init_data)
>>> +{
>>> +ÂÂÂ struct led_classdev *led_cdev;
>>> +ÂÂÂ int ret;
>>> +
>>> +ÂÂÂ if (!mcled_cdev)
>>> +ÂÂÂÂÂÂÂ return -EINVAL;
>>> +
>>> +ÂÂÂ led_cdev = mcled_cdev->led_cdev;
>>> +ÂÂÂ INIT_LIST_HEAD(&mcled_cdev->color_list);
>>> +
>>> +ÂÂÂ /* Register led class device */
>>> +ÂÂÂ ret = led_classdev_register_ext(parent, led_cdev, init_data);
>>> +ÂÂÂ if (ret)
>>> +ÂÂÂÂÂÂÂ return ret;
>>> +
>>> +ÂÂÂ return led_multicolor_init_color_dir(mcled_cdev);
>>> +}
>>> +EXPORT_SYMBOL_GPL(led_classdev_multicolor_register_ext);
>> Why devm_* versions are missing now?
>
> I was using the led-class-flash.c as an example and that class does not
> have the devm_* versions either.

Why did you have them in v8 then?

> Tried to make the 2 child classes look the same.

We can fix the shortcomings while adding new code.

> If they are missing from the led-class-flash code then that needs to be
> fixed as well.

Well, yes.

>>
>>> +
>>> +void led_classdev_multicolor_unregister(struct led_classdev_mc
>>> *mcled_cdev)
>>> +{
>>> +ÂÂÂ struct led_mc_color_entry *priv, *next;
>>> +
>>> +ÂÂÂ if (!mcled_cdev)
>>> +ÂÂÂÂÂÂÂ return;
>>> +
>>> +ÂÂÂ list_for_each_entry_safe(priv, next, &mcled_cdev->color_list, list)
>>> +ÂÂÂÂÂÂÂ list_del(&priv->list);
>>> +
>>> +ÂÂÂ sysfs_remove_group(&mcled_cdev->led_cdev->dev->kobj,
>>> &led_color_group);
>>> +ÂÂÂ led_classdev_unregister(mcled_cdev->led_cdev);
>>> +}
>>> +EXPORT_SYMBOL_GPL(led_classdev_multicolor_unregister);
>>> +
>>> +MODULE_AUTHOR("Dan Murphy <dmurphy@xxxxxx>");
>>> +MODULE_DESCRIPTION("Multi Color LED class interface");
>>> +MODULE_LICENSE("GPL v2");
>>> diff --git a/include/linux/led-class-multicolor.h
>>> b/include/linux/led-class-multicolor.h
>>> new file mode 100644
>>> index 000000000000..280ba5a614b4
>>> --- /dev/null
>>> +++ b/include/linux/led-class-multicolor.h
>>> @@ -0,0 +1,74 @@
>>> +/* SPDX-License-Identifier: GPL-2.0 */
>>> +/* LED Multicolor class interface
>>> + * Copyright (C) 2019 Texas Instruments Incorporated -
>>> http://www.ti.com/
>>> + */
>>> +
>>> +#ifndef __LINUX_MULTICOLOR_LEDS_H_INCLUDED
>>> +#define __LINUX_MULTICOLOR_LEDS_H_INCLUDED
>>> +
>>> +#include <linux/leds.h>
>>> +#include <dt-bindings/leds/common.h>
>>> +
>>> +struct led_classdev_mc;
>>> +
>>> +struct led_mc_color_entry {
>>> +ÂÂÂ struct led_classdev_mc *mcled_cdev;
>>> +
>>> +ÂÂÂ struct device_attribute max_intensity_attr;
>>> +ÂÂÂ struct device_attribute intensity_attr;
>>> +
>>> +ÂÂÂ enum led_brightness max_intensity;
>>> +ÂÂÂ enum led_brightness intensity;
>>> +
>>> +ÂÂÂ struct list_head list;
>>> +
>>> +ÂÂÂ int led_color_id;
>>> +};
>>> +
>>> +struct led_classdev_mc {
>>> +ÂÂÂ /* led class device */
>>> +ÂÂÂ struct led_classdev *led_cdev;
>>> +ÂÂÂ struct list_head color_list;
>>> +
>>> +ÂÂÂ unsigned long available_colors;
>>> +ÂÂÂ int num_leds;
>>> +};
>>> +
>>> +static inline struct led_classdev_mc *lcdev_to_mccdev(
>>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ struct led_classdev *lcdev)
>>> +{
>>> +ÂÂÂ return container_of(lcdev, struct led_classdev_mc, led_cdev);
>>> +}
>>> +
>>> +/**
>>> + * led_classdev_multicolor_register_ext - register a new object of
>>> led_classdev
>>> + *ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ class with support for multicolor LEDs
>>> + * @parent: the multicolor LED to register
>>> + * @mcled_cdev: the led_classdev_mc structure for this device
>>> + * @init_data: the LED class Multi color device initialization data
>>> + *
>>> + * Returns: 0 on success or negative error value on failure
>>> + */
>>> +int led_classdev_multicolor_register_ext(struct device *parent,
>>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ struct led_classdev_mc *mcled_cdev,
>>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ struct led_init_data *init_data);
>>> +
>>> +#define led_classdev_multicolor_register(parent, mcled_cdev)ÂÂÂÂÂÂÂ \
>>> +ÂÂÂ led_classdev_multicolor_register_ext(parent, mcled_cdev, NULL)
>> Please turn it into inline.
>
> Again same statement as above on the led-class-flash.Â This is how this
> is defined there.
>
> If that is not correct in the flash class then that needs to be fixed as
> well.

Did you see my patch fixing these things in leds.h [0]?
I just missed that problem in led-class-flash.h.

>>> +
>>> +/**
>>> + * led_classdev_multicolor_unregister - unregisters an object of
>>> led_classdev
>>> + *ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ class with support for multicolor LEDs
>>> + * @mcled_cdev: the multicolor LED to unregister
>>> + *
>>> + * Unregister a previously registered via
>>> led_classdev_multicolor_register
>>> + * object
>>> + */
>>> +void led_classdev_multicolor_unregister(struct led_classdev_mc
>>> *mcled_cdev);
>>> +
>>> +/* Calculate brightness for the monochrome LED cluster */
>>> +void led_mc_calc_brightness(struct led_classdev_mc *mcled_cdev,
>>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ enum led_brightness brightness,
>>> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ int brightness_val[]);
>>> +
>>> +#endifÂÂÂ /* __LINUX_MULTICOLOR_LEDS_H_INCLUDED */
>>>
>

[0]
https://git.kernel.org/pub/scm/linux/kernel/git/j.anaszewski/linux-leds.git/commit/?h=for-next&id=7c322056e3564da1b5bdc3f3cb79229582955eb2

--
Best regards,
Jacek Anaszewski

Next message: Dominik Brodowski: "[RFC] random: UEFI RNG input is bootloader randomness"
Previous message: Masami Hiramatsu: "[PATCH] tracing/probe: Fix to check the difference of nr_args before adding probe"
In reply to: Dan Murphy: "Re: [PATCH v9 05/15] leds: multicolor: Introduce a multicolor class definition"
Next in thread: Jacek Anaszewski: "Re: [PATCH v9 05/15] leds: multicolor: Introduce a multicolor class definition"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]