Re: [PATCH v19 04/18] leds: multicolor: Introduce a multicolor class definition

[Jacek Anaszewski <jacek.anaszewski@gmail.com>]

Dan,

One more nit below.

On 4/2/20 10:42 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.
[...]
>  # 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..838b1d679647
> --- /dev/null
> +++ b/drivers/leds/led-class-multicolor.c
> @@ -0,0 +1,206 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// LED Multi Color class interface
> +// Copyright (C) 2019-20 Texas Instruments Incorporated - http://www.ti.com/
> +// Author: Dan Murphy <dmurphy@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"
> +
> +int led_mc_calc_color_components(struct led_classdev_mc *mcled_cdev,
> +				 enum led_brightness brightness)
> +{
> +	struct led_classdev *led_cdev = &mcled_cdev->led_cdev;
> +	int i;
> +
> +	for (i = 0; i < mcled_cdev->num_colors; i++)
> +		mcled_cdev->multicolor_info[i].color_brightness = (brightness *

If you passed temporary array for collecting calculated brightness
items to this function then you could remove this color_brightness
field from struct multicolor_info.

> +					  mcled_cdev->multicolor_info[i].color_led_intensity /
> +					  led_cdev->max_brightness);
> +
> +	return 0;
[...]
> +MODULE_AUTHOR("Dan Murphy <dmurphy@ti.com>");
> +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..a6a3715c0d48
> --- /dev/null
> +++ b/include/linux/led-class-multicolor.h
> @@ -0,0 +1,121 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* LED Multicolor class interface
> + * Copyright (C) 2019-20 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_multicolor_info {

s/led_multicolor_info/led_mc_subled/ ?

> +	int color_index;
> +	int color_brightness;

This should not be needed.

> +	int color_led_intensity;
> +	int color_channel;
> +};
> +
> +struct led_classdev_mc {
> +	/* led class device */
> +	struct led_classdev led_cdev;
> +	int num_colors;
> +
> +	struct led_multicolor_info multicolor_info[LED_COLOR_ID_MAX];



> +};

-- 
Best regards,
Jacek Anaszewski