Dan,

One more remark below.

On 9/20/19 7:41 PM, Dan Murphy wrote:
> Introduce a multicolor class that groups colored LEDs
> within a LED node.
> 
> The framework allows for dynamically setting individual LEDs
> or setting brightness levels of LEDs and updating them virtually
> simultaneously.
> 
> Signed-off-by: Dan Murphy <dmur...@ti.com>
> ---
>  drivers/leds/Kconfig                 |  10 +
>  drivers/leds/Makefile                |   1 +
>  drivers/leds/led-class-multicolor.c  | 316 +++++++++++++++++++++++++++
>  include/linux/led-class-multicolor.h |  76 +++++++
>  4 files changed, 403 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
[...]
> +static int led_multicolor_init_color_dir(struct led_classdev_mc_data *data,
> +                                      struct led_classdev_mc *mcled_cdev)
> +{
> +     struct led_classdev *led_cdev = mcled_cdev->led_cdev;
> +     u32 color_id;
> +     int ret;
> +     int i, j = 0;
> +
> +     data->mcled_cdev = mcled_cdev;
> +
> +     ret = sysfs_create_group(&led_cdev->dev->kobj, &led_color_group);
> +     if (ret)
> +             return ret;
> +
> +     for (i = 0; i < LED_COLOR_ID_MAX; i++) {
> +             color_id = (mcled_cdev->available_colors & (1 << i));

Please use bitops for accessing available_colors.

> +             if (color_id) {

Here you need: if (test_bit(i, &mcled_cdev->available_colors))

> +                     ret = led_multicolor_init_color(data, mcled_cdev, i, j);
> +                     if (ret)
> +                             break;
> +
> +                     j++;
> +             }
> +     }
> +
> +     return ret;
> +}

-- 
Best regards,
Jacek Anaszewski

Reply via email to