| Message ID | 20190725182818.29556-1-dmurphy@ti.com |
|---|---|
| Headers | show |
| Series | Multicolor Framwork | expand |
Dan, On 7/25/19 8:28 PM, Dan Murphy wrote: > Add DT bindings for the LEDs multicolor class framework. > > Signed-off-by: Dan Murphy <dmurphy@ti.com> > --- > .../bindings/leds/leds-class-multicolor.txt | 96 +++++++++++++++++++ > 1 file changed, 96 insertions(+) > create mode 100644 Documentation/devicetree/bindings/leds/leds-class-multicolor.txt > > diff --git a/Documentation/devicetree/bindings/leds/leds-class-multicolor.txt b/Documentation/devicetree/bindings/leds/leds-class-multicolor.txt > new file mode 100644 > index 000000000000..5d36327b18fc > --- /dev/null > +++ b/Documentation/devicetree/bindings/leds/leds-class-multicolor.txt > @@ -0,0 +1,96 @@ > +* Multicolor LED properties > + > +Multicolor LEDs can consist of a RGB, RGBW or a RGBA LED clusters. These devices There are also RGBAUV LEDs. And maybe some other, I'd not strive to mention all multicolor LED combinations available on the market. > +can be grouped together and also provide a modeling mechanism so that the > +cluster LEDs can vary in hue and intensity to produce a wide range of colors. It looks as if RGBW, RGB and RGBA LEDs could be grouped together with this LED multicolor class, which is not what it does. We need to rephrase that rather to underline the possibility of grouping all the LED colors of e.g. RGBA under single multi color LED child node. > + > +The nodes and properties defined in this document are unique to the multicolor > +LED class. Common LED nodes and properties are inherited from the common.txt > +within this documentation directory. > + > +Required LED Child properties: > + - color : For multicolor LED support this property should be defined as > + LED_COLOR_ID_MULTI and further definition can be found in > + include/linux/leds/common.h. > + > +led-controller@30 { > + #address-cells = <1>; > + #size-cells = <0>; > + compatible = "ti,lp5024"; > + reg = <0x29>; > + > + multi-led@1 { > + #address-cells = <1>; > + #size-cells = <0>; > + reg = <1>; > + color = <LED_COLOR_ID_MULTI>; > + function = LED_FUNCTION_; suffix is missing here > + > + > + led@3 { > + reg = <3>; > + color = <LED_COLOR_ID_RED>; > + }; > + > + led@4 { > + reg = <4>; > + color = <LED_COLOR_ID_GREEN>; > + }; > + > + led@5 { > + reg = <5>; > + color = <LED_COLOR_ID_BLUE>; > + }; > + }; > + > + multi-led@2 { > + #address-cells = <1>; > + #size-cells = <0>; > + color = <LED_COLOR_ID_MULTI>; > + function = LED_FUNCTION_ACTIVITY; > + reg = <2>; > + ti,led-bank = <2 3 5>; > + > + led@6 { > + reg = <0x6>; > + color = <LED_COLOR_ID_RED>; > + led-sources = <6 9 15>; > + }; > + > + led@7 { > + reg = <0x7>; > + color = <LED_COLOR_ID_GREEN>; > + led-sources = <7 10 16>; > + }; > + > + led@8 { > + reg = <0x8>; > + color = <LED_COLOR_ID_BLUE>; > + led-sources = <8 11 17>; > + }; > + }; > + > + multi-led@4 { > + #address-cells = <1>; > + #size-cells = <0>; > + reg = <4>; > + color = <LED_COLOR_ID_MULTI>; > + function = LED_FUNCTION_ACTIVITY; > + > + led@12 { > + reg = <12>; > + color = <LED_COLOR_ID_RED>; > + }; > + > + led@13 { > + reg = <13>; > + color = <LED_COLOR_ID_GREEN>; > + }; > + > + led@14 { > + reg = <14>; > + color = <LED_COLOR_ID_BLUE>; > + }; > + }; > + > +}; > -- Best regards, Jacek Anaszewski
Dan, On 7/25/19 8:28 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 <dmurphy@ti.com> > --- > drivers/leds/Kconfig | 10 + > drivers/leds/Makefile | 1 + > drivers/leds/led-class-multicolor.c | 402 +++++++++++++++++++++++++++ > include/linux/led-class-multicolor.h | 96 +++++++ > 4 files changed, 509 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 f7a3dd7ecf3d..7f780d5b8490 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 s/Class/class/ I'll need to fix it in LED flash class entry too. > + 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 41fb073a39c1..897b810257dd 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..123443e6d3eb > --- /dev/null > +++ b/drivers/leds/led-class-multicolor.c > @@ -0,0 +1,402 @@ > +// 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" > + > +struct led_classdev_mc_priv { > + struct led_classdev_mc *mcled_cdev; > + > + struct device_attribute max_intensity_attr; > + struct device_attribute intensity_attr; > + struct device_attribute color_index_attr; > + > + enum led_brightness max_intensity; > + enum led_brightness intensity; > + enum led_brightness new_intensity; > + > + struct list_head list; > + > + int color_id; > + int color_index; We need to differentiate both more meaningfully. Maybe led_color_id and cluster_color_id? > +}; > +static int calculate_brightness(int brightness, int intensity, int max_intensity) How about led_mc_calculate_brightness ? > +{ > + if (brightness && intensity && max_intensity) > + return brightness * intensity / max_intensity; > + > + return LED_OFF; > +} > + > +void set_cluster_brightness(struct led_classdev_mc *mcled_cdev, > + enum led_brightness brightness, int adj_value[]) s/set_cluster/led_mc_set_cluster/ And I think adj_value is not telling too much. How about: s/adj_value/color_intensity/ > +{ > + struct led_classdev_mc_data *data = mcled_cdev->data; > + struct led_classdev_mc_priv *priv; > + int i = 0; > + > + list_for_each_entry(priv, &data->color_list, list) { > + adj_value[i] = calculate_brightness(brightness, priv->intensity, > + priv->max_intensity); > + i++; > + } > + > + data->cluster_brightness = brightness; This property duplicates brightness stored in struct led_classdev and thus it is redundant. > +} > + > +void get_cluster_brightness(struct led_classdev_mc *mcled_cdev, > + int brightness_val[]) s/get_cluster/led_mc_get_cluster/ s/brightness_val/color_intensity/ > +{ > + struct led_classdev_mc_data *data = mcled_cdev->data; > + struct led_classdev_mc_priv *priv; > + int i = 0; > + > + list_for_each_entry(priv, &data->color_list, list) { > + brightness_val[i] = priv->intensity; > + i++; > + } > +} > + > +static ssize_t color_index_show(struct device *dev, > + struct device_attribute *color_index_attr, > + char *buf) > +{ > + struct led_classdev_mc_priv *priv = container_of(color_index_attr, > + struct led_classdev_mc_priv, > + color_index_attr); > + > + return sprintf(buf, "%d\n", priv->color_id); > +} > + > +static ssize_t color_mix_store(struct device *dev, > + struct device_attribute *color_mix_attr, > + const char *buf, size_t size) > +{ > + struct led_classdev_mc_data *data = container_of(color_mix_attr, > + struct led_classdev_mc_data, > + color_mix_attr); > + struct led_classdev_mc *mcled_cdev = data->mcled_cdev; > + const struct led_multicolor_ops *ops = mcled_cdev->ops; > + struct led_classdev_mc_priv *priv; > + unsigned int value[LED_COLOR_ID_MAX]; > + int adj_brightness; > + int nrchars, offset = 0; > + unsigned int i; > + int ret; > + > + for (i = 0; i < mcled_cdev->num_leds; i++) { > + ret = sscanf(buf + offset, "%i%n", &value[i], &nrchars); > + if (ret != 1) > + break; > + > + offset += nrchars; > + } > + > + if (i != mcled_cdev->num_leds) { Shouldn't we return error if i != mcled_cdev->num_leds - 1 ? How can we know which color the value will be for if there is less values passed than the total number of colors in the cluster? > + for (; i < LED_COLOR_ID_MAX; i++) > + value[i] = 0; What use case is it for? > + } > + > + list_for_each_entry(priv, &data->color_list, list) { > + if (data->cluster_brightness) { > + adj_brightness = calculate_brightness(data->cluster_brightness, > + value[priv->color_index], > + priv->max_intensity); > + ret = ops->set_color_brightness(priv->mcled_cdev, > + priv->color_id, > + adj_brightness); > + if (ret < 0) > + goto done; > + } > + > + priv->intensity = value[priv->color_index]; > + } Here we could use just brightness_set op as a single call. We should always write all colors as a result of write to color_mix anyway. > + > +done: > + return size; > +} > + > +static ssize_t intensity_store(struct device *dev, > + struct device_attribute *intensity_attr, > + const char *buf, size_t size) > +{ > + struct led_classdev_mc_priv *priv = container_of(intensity_attr, > + struct led_classdev_mc_priv, > + intensity_attr); > + struct led_classdev_mc_data *data = priv->mcled_cdev->data; > + struct led_classdev *led_cdev = priv->mcled_cdev->led_cdev; > + struct led_multicolor_ops *ops = priv->mcled_cdev->ops; > + unsigned int adj_value; > + 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; > + } We don't fail in this case in LED class, so let's proceed in the same way here: priv->intensity = min(value, priv->max_brightness); > + > + priv->new_intensity = value; > + > + if (data->cluster_brightness) { > + adj_value = calculate_brightness(data->cluster_brightness, > + priv->new_intensity, > + priv->max_intensity); > + ret = ops->set_color_brightness(priv->mcled_cdev, > + priv->color_id, adj_value); > + if (ret < 0) { > + priv->new_intensity = priv->intensity; This is unnecessary complication. Just write the calculated iout intensity. We need to highlight it in the documentation that exact requested color intensity values are written to the hardware only when brightness == max_brightness. > + goto unlock; > + } > + } > + > + priv->intensity = priv->new_intensity; > + 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_classdev_mc_priv *priv = container_of(intensity_attr, > + struct led_classdev_mc_priv, > + intensity_attr); > + const struct led_multicolor_ops *ops = priv->mcled_cdev->ops; > + int value = 0; > + > + if (ops->get_color_brightness) { > + value = ops->get_color_brightness(priv->mcled_cdev, > + priv->color_id); > + priv->intensity = value; > + } else { > + value = priv->intensity; > + } > + > + return sprintf(buf, "%d\n", value); > +} > + > +static ssize_t max_intensity_show(struct device *dev, > + struct device_attribute *max_intensity_attr, > + char *buf) > +{ > + struct led_classdev_mc_priv *priv = container_of(max_intensity_attr, > + struct led_classdev_mc_priv, > + max_intensity_attr); > + > + return sprintf(buf, "%d\n", priv->max_intensity); > +} > + > +static int led_multicolor_init_color(struct led_classdev_mc_data *data, > + struct led_classdev_mc *mcled_cdev, > + int color_id, int color_index) > +{ > + struct led_classdev *led_cdev = mcled_cdev->led_cdev; > + struct led_classdev_mc_priv *mc_priv; > + int ret; > + > + mc_priv = devm_kzalloc(led_cdev->dev, sizeof(*mc_priv), GFP_KERNEL); > + if (!mc_priv) > + return -ENOMEM; > + > + mc_priv->color_id = color_id; > + mc_priv->mcled_cdev = mcled_cdev; > + > + data->led_kobj = kobject_create_and_add(led_colors[color_id], > + data->color_kobj); You should not use kobject API in the drivers. See [0]. > + if (!data->led_kobj) > + return -EINVAL; > + > + sysfs_attr_init(&mc_priv->intensity_attr.attr); > + mc_priv->intensity_attr.attr.name = "intensity"; > + mc_priv->intensity_attr.attr.mode = S_IRUGO | S_IWUGO; > + mc_priv->intensity_attr.store = intensity_store; > + mc_priv->intensity_attr.show = intensity_show; > + ret = sysfs_create_file(data->led_kobj, > + &mc_priv->intensity_attr.attr); > + if (ret) > + goto err_out; > + > + sysfs_attr_init(&mc_priv->max_intensity_attr.attr); > + mc_priv->max_intensity_attr.attr.name = "max_intensity"; > + mc_priv->max_intensity_attr.attr.mode = S_IRUGO; > + mc_priv->max_intensity_attr.show = max_intensity_show; > + ret = sysfs_create_file(data->led_kobj, > + &mc_priv->max_intensity_attr.attr); > + if (ret) > + goto err_out; > + > + sysfs_attr_init(&mc_priv->color_index_attr.attr); > + mc_priv->color_index_attr.attr.name = "color_id"; > + mc_priv->color_index_attr.attr.mode = S_IRUGO; > + mc_priv->color_index_attr.show = color_index_show; > + ret = sysfs_create_file(data->led_kobj, > + &mc_priv->color_index_attr.attr); > + if (ret) > + goto err_out; > + > + mc_priv->max_intensity = LED_FULL; > + mc_priv->color_index = color_index; > + list_add_tail(&mc_priv->list, &data->color_list); > + > +err_out: > + return ret; > +} > + > +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->color_kobj = kobject_create_and_add("colors", > + &led_cdev->dev->kobj); > + if (!data->color_kobj) > + return -EINVAL; > + > + data->mcled_cdev = mcled_cdev; > + > + for (i = 0; i < LED_COLOR_ID_MAX; i++) { > + color_id = (mcled_cdev->available_colors & (1 << i)); > + if (color_id) { > + ret = led_multicolor_init_color(data, mcled_cdev, i, j); > + if (ret) > + break; > + > + j++; > + } > + } > + > + sysfs_attr_init(&data->color_mix_attr.attr); > + data->color_mix_attr.attr.name = "color_mix"; > + data->color_mix_attr.attr.mode = S_IRUGO | S_IWUGO; > + data->color_mix_attr.store = color_mix_store; > + ret = sysfs_create_file(data->color_kobj, > + &data->color_mix_attr.attr); > + > + 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; > + struct led_multicolor_ops *ops; > + struct led_classdev_mc_data *data; > + int ret; > + > + if (!mcled_cdev) > + return -EINVAL; > + > + ops = mcled_cdev->ops; > + if (!ops || !ops->set_color_brightness) > + return -EINVAL; > + > + data = kzalloc(sizeof(*data), GFP_KERNEL); > + if (!data) > + return -ENOMEM; > + > + mcled_cdev->data = data; > + led_cdev = mcled_cdev->led_cdev; > + INIT_LIST_HEAD(&data->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(data, mcled_cdev); > +} > +EXPORT_SYMBOL_GPL(led_classdev_multicolor_register_ext); > + > +void led_classdev_multicolor_unregister(struct led_classdev_mc *mcled_cdev) > +{ > + if (!mcled_cdev) > + return; > + > + led_classdev_unregister(mcled_cdev->led_cdev); > +} > +EXPORT_SYMBOL_GPL(led_classdev_multicolor_unregister); > + > +static void devm_led_classdev_multicolor_release(struct device *dev, void *res) > +{ > + led_classdev_multicolor_unregister(*(struct led_classdev_mc **)res); > +} > + > +/** > + * devm_of_led_classdev_register - resource managed led_classdev_register() > + * > + * @parent: parent of LED device > + * @led_cdev: the led_classdev structure for this device. > + */ > +int devm_led_classdev_multicolor_register(struct device *parent, > + struct led_classdev_mc *mcled_cdev) > +{ > + struct led_classdev_mc **dr; > + int ret; > + > + dr = devres_alloc(devm_led_classdev_multicolor_release, > + sizeof(*dr), GFP_KERNEL); > + if (!dr) > + return -ENOMEM; > + > + ret = led_classdev_multicolor_register(parent, mcled_cdev); > + if (ret) { > + devres_free(dr); > + return ret; > + } > + > + *dr = mcled_cdev; > + devres_add(parent, dr); > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(devm_led_classdev_multicolor_register); > + > +static int devm_led_classdev_multicolor_match(struct device *dev, > + void *res, void *data) > +{ > + struct mcled_cdev **p = res; > + > + if (WARN_ON(!p || !*p)) > + return 0; > + > + return *p == data; > +} > + > +/** > + * devm_led_classdev_multicolor_unregister() - resource managed > + * led_classdev_multicolor_unregister() > + * @parent: The device to unregister. > + * @mcled_cdev: the led_classdev_mc structure for this device. > + */ > +void devm_led_classdev_multicolor_unregister(struct device *dev, > + struct led_classdev_mc *mcled_cdev) > +{ > + WARN_ON(devres_release(dev, > + devm_led_classdev_multicolor_release, > + devm_led_classdev_multicolor_match, mcled_cdev)); > +} > +EXPORT_SYMBOL_GPL(devm_led_classdev_multicolor_unregister); > + > +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..082883606246 > --- /dev/null > +++ b/include/linux/led-class-multicolor.h > @@ -0,0 +1,96 @@ > +/* 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_classdev_mc_data { > + struct led_classdev_mc *mcled_cdev; > + > + struct kobject *color_kobj; > + struct kobject *led_kobj; > + > + struct device_attribute color_mix_attr; > + > + struct list_head color_list; > + > + enum led_brightness cluster_brightness; > +}; > + > +struct led_multicolor_ops { > + /* Set brightness for a specific color id */ > + int (*set_color_brightness)(struct led_classdev_mc *mcled_cdev, > + int color_id, int value); > + /* Read current color setting */ > + int (*get_color_brightness)(struct led_classdev_mc *mcled_cdev, > + int color_id); > +}; > + > +struct led_classdev_mc { > + /* led class device */ > + struct led_classdev *led_cdev; > + > + /* multicolor led specific ops */ > + struct led_multicolor_ops *ops; > + > + struct led_classdev_mc_data *data; > + > + u32 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 > + */ > +extern 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) > + > +/** > + * 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 > + */ > +extern void led_classdev_multicolor_unregister(struct led_classdev_mc *mcled_cdev); > + > +extern int devm_led_classdev_multicolor_register(struct device *parent, > + struct led_classdev_mc *mcled_cdev); > + > +extern void devm_led_classdev_multicolor_unregister(struct device *parent, > + struct led_classdev_mc *mcled_cdev); > + > +/* Set brightness for the monochrome LED cluster */ > +extern void set_cluster_brightness(struct led_classdev_mc *mcled_cdev, > + enum led_brightness brightness, > + int adj_value[]); > + > +/* Read current cluster brightness */ > +extern void get_cluster_brightness(struct led_classdev_mc *mcled_cdev, > + int brightness_val[]); > + > +#endif /* __LINUX_MULTICOLOR_LEDS_H_INCLUDED */ > I'll review remaining patches (drivers) next time. [0] https://lore.kernel.org/linux-leds/20160825124948.GA15567@kroah.com/ -- Best regards, Jacek Anaszewski
Hi Dan, Thank you for the patch I see few things to improve. On 7/25/19 8:28 PM, Dan Murphy wrote: > Introduce the LP5036/30/24/18 RGB LED driver. > The difference in these parts are the number of > LED outputs where the: > > LP5036 can control 36 LEDs > LP5030 can control 30 LEDs > LP5024 can control 24 LEDs > LP5018 can control 18 LEDs > > The device has the ability to group LED output into control banks > so that multiple LED banks can be controlled with the same mixing and > brightness. Inversely the LEDs can also be controlled independently. > > Signed-off-by: Dan Murphy <dmurphy@ti.com> > --- > drivers/leds/Kconfig | 7 + > drivers/leds/Makefile | 1 + > drivers/leds/leds-lp50xx.c | 789 +++++++++++++++++++++++++++++++++++++ > 3 files changed, 797 insertions(+) > create mode 100644 drivers/leds/leds-lp50xx.c > > diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig > index 7f780d5b8490..69c037020f6b 100644 > --- a/drivers/leds/Kconfig > +++ b/drivers/leds/Kconfig > @@ -350,6 +350,13 @@ config LEDS_LP3952 > To compile this driver as a module, choose M here: the > module will be called leds-lp3952. > > +config LEDS_LP50XX > + tristate "LED Support for TI LP5036/30/24/18 LED driver chip" > + depends on LEDS_CLASS && REGMAP_I2C > + help > + If you say yes here you get support for the Texas Instruments > + LP5036, LP5030, LP5024 and LP5018 LED driver. > + > config LEDS_LP55XX_COMMON > tristate "Common Driver for TI/National LP5521/5523/55231/5562/8501" > depends on LEDS_LP5521 || LEDS_LP5523 || LEDS_LP5562 || LEDS_LP8501 > diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile > index 897b810257dd..438a5499f3ee 100644 > --- a/drivers/leds/Makefile > +++ b/drivers/leds/Makefile > @@ -34,6 +34,7 @@ obj-$(CONFIG_LEDS_GPIO_REGISTER) += leds-gpio-register.o > obj-$(CONFIG_LEDS_GPIO) += leds-gpio.o > obj-$(CONFIG_LEDS_LP3944) += leds-lp3944.o > obj-$(CONFIG_LEDS_LP3952) += leds-lp3952.o > +obj-$(CONFIG_LEDS_LP50XX) += leds-lp50xx.o > obj-$(CONFIG_LEDS_LP55XX_COMMON) += leds-lp55xx-common.o > obj-$(CONFIG_LEDS_LP5521) += leds-lp5521.o > obj-$(CONFIG_LEDS_LP5523) += leds-lp5523.o > diff --git a/drivers/leds/leds-lp50xx.c b/drivers/leds/leds-lp50xx.c > new file mode 100644 > index 000000000000..5a32410e4f34 [...] > +static int lp50xx_probe_dt(struct lp50xx *priv) > +{ > + u32 led_strings[LP5036_MAX_LED_MODULES]; > + struct fwnode_handle *child = NULL; > + struct fwnode_handle *led_node = NULL; > + struct led_init_data init_data; > + struct lp50xx_led *led; > + int num_colors; > + u32 color_id; > + int led_number; > + size_t i = 0; > + int ret; > + > + priv->enable_gpio = devm_gpiod_get_optional(&priv->client->dev, > + "enable", GPIOD_OUT_LOW); > + if (IS_ERR(priv->enable_gpio)) { > + ret = PTR_ERR(priv->enable_gpio); > + dev_err(&priv->client->dev, "Failed to get enable gpio: %d\n", > + ret); > + return ret; > + } > + > + priv->regulator = devm_regulator_get(&priv->client->dev, "vled"); > + if (IS_ERR(priv->regulator)) > + priv->regulator = NULL; > + > + if (priv->model_id == LP5009) > + priv->max_leds = LP5009_MAX_LED_MODULES; > + else if (priv->model_id == LP5012) > + priv->max_leds = LP5012_MAX_LED_MODULES; > + else if (priv->model_id == LP5018) > + priv->max_leds = LP5018_MAX_LED_MODULES; > + else if (priv->model_id == LP5024) > + priv->max_leds = LP5024_MAX_LED_MODULES; > + else if (priv->model_id == LP5030) > + priv->max_leds = LP5030_MAX_LED_MODULES; > + else > + priv->max_leds = LP5036_MAX_LED_MODULES; You could simplify that by initializing data property of lp50xx_leds_match array elements and then call of_match_device. You can compare e.g. drivers/leds/leds-pca9532.c > + device_for_each_child_node(&priv->client->dev, child) { > + led = &priv->leds[i]; > + if (fwnode_property_present(child, "ti,led-bank")) { > + ret = fwnode_property_read_u32_array(child, > + "ti,led-bank", > + NULL, 0); > + ret = fwnode_property_read_u32_array(child, > + "ti,led-bank", > + led_strings, > + ret); > + > + priv->num_of_banked_leds = ARRAY_SIZE(led_strings); > + > + ret = lp50xx_set_banks(priv, led_strings); > + if (ret) { > + dev_err(&priv->client->dev, > + "Cannot setup banked LEDs\n"); > + fwnode_handle_put(child); > + goto child_out; > + } > + led->ctrl_bank_enabled = 1; > + > + } else { > + ret = fwnode_property_read_u32(child, "reg", > + &led_number); > + > + led->led_number = led_number; > + } > + if (ret) { > + dev_err(&priv->client->dev, > + "led sourcing property missing\n"); > + fwnode_handle_put(child); > + goto child_out; > + } > + > + if (led_number > priv->max_leds) { > + dev_err(&priv->client->dev, > + "led-sources property is invalid\n"); > + ret = -EINVAL; > + fwnode_handle_put(child); > + goto child_out; > + } > + > + init_data.fwnode = child; > + init_data.devicename = priv->client->name; We'd rather not have devicename in the name of this LED class device according to the new LED naming convention. It's not a hot-pluggable device, so for properly designed DT we should not encounter name clash. > + init_data.default_label = ":"; default_label should not be set for new drivers. See the comment in the include/linux/leds.h: /* * string to be used for devicename section of LED class device * either for label based LED name composition path or for fwnode * based when devname_mandatory is true */ This is new driver and we should not add any convenience fallbacks for label based DT bindings. Label DT property has been marked deprecated, so for new drivers it should be deemed non-existent. > + > + fwnode_property_read_string(child, "linux,default-trigger", > + &led->led_dev.default_trigger); > + num_colors = 0; > + > + fwnode_for_each_child_node(child, led_node) { > + ret = fwnode_property_read_u32(led_node, "color", > + &color_id); > + if (ret) > + dev_err(&priv->client->dev, > + "Cannot read color\n"); > + > + led->mc_cdev.available_colors |= (1 << color_id); > + num_colors++; > + > + } > + > + led->priv = priv; > + led->mc_cdev.ops = &lp50xx_mc_ops; > + led->mc_cdev.num_leds = num_colors; > + led->mc_cdev.led_cdev = &led->led_dev; > + led->led_dev.brightness_set_blocking = lp50xx_brightness_set; > + led->led_dev.brightness_get = lp50xx_brightness_get; > + ret = led_classdev_multicolor_register_ext(&priv->client->dev, > + &led->mc_cdev, > + &init_data); Why not devm_* prefixed version? [...] -- Best regards, Jacek Anaszewski
Dan, On 7/31/19 9:06 PM, Dan Murphy wrote: > Jacek > > On 7/29/19 3:50 PM, Jacek Anaszewski wrote: >> Dan, >> >> On 7/25/19 8:28 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 <dmurphy@ti.com> >>> --- >>> drivers/leds/Kconfig | 10 + >>> drivers/leds/Makefile | 1 + >>> drivers/leds/led-class-multicolor.c | 402 +++++++++++++++++++++++++++ >>> include/linux/led-class-multicolor.h | 96 +++++++ >>> 4 files changed, 509 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 f7a3dd7ecf3d..7f780d5b8490 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 >> s/Class/class/ >> >> I'll need to fix it in LED flash class entry too. >> >>> + 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 41fb073a39c1..897b810257dd 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..123443e6d3eb >>> --- /dev/null >>> +++ b/drivers/leds/led-class-multicolor.c >>> @@ -0,0 +1,402 @@ >>> +// 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" >>> + >>> +struct led_classdev_mc_priv { >>> + struct led_classdev_mc *mcled_cdev; >>> + >>> + struct device_attribute max_intensity_attr; >>> + struct device_attribute intensity_attr; >>> + struct device_attribute color_index_attr; >>> + >>> + enum led_brightness max_intensity; >>> + enum led_brightness intensity; >>> + enum led_brightness new_intensity; >>> + >>> + struct list_head list; >>> + >>> + int color_id; >>> + int color_index; >> We need to differentiate both more meaningfully. >> Maybe led_color_id and cluster_color_id? > > Thats looks better. > Looking at it now I had to wonder what I had on mind. So maybe even better shot: int color_id // so as original int color_seq_pos // i.e. position in the color_mix sequence [...] >>> +static ssize_t color_mix_store(struct device *dev, >>> + struct device_attribute *color_mix_attr, >>> + const char *buf, size_t size) >>> +{ >>> + struct led_classdev_mc_data *data = container_of(color_mix_attr, >>> + struct led_classdev_mc_data, >>> + color_mix_attr); >>> + struct led_classdev_mc *mcled_cdev = data->mcled_cdev; >>> + const struct led_multicolor_ops *ops = mcled_cdev->ops; >>> + struct led_classdev_mc_priv *priv; >>> + unsigned int value[LED_COLOR_ID_MAX]; >>> + int adj_brightness; >>> + int nrchars, offset = 0; >>> + unsigned int i; >>> + int ret; >>> + >>> + for (i = 0; i < mcled_cdev->num_leds; i++) { >>> + ret = sscanf(buf + offset, "%i%n", &value[i], &nrchars); >>> + if (ret != 1) >>> + break; >>> + >>> + offset += nrchars; >>> + } >>> + >>> + if (i != mcled_cdev->num_leds) { >> Shouldn't we return error if i != mcled_cdev->num_leds - 1 ? >> How can we know which color the value will be for if there is less >> values passed than the total number of colors in the cluster? > > Ok so during my testing if I had the monochrome array as <R G B> > > When I wrote only <R G> and no blue I was getting random values in the > array for the > > remaining indexes and the blue LED would randomly turn on/off at > different levels. > > So if the user passes in less then expected only ids with data will be > written and the other colors will be turned off by the for loop below. From what I see it will lead to wrong mapping of given color to the value array element in the following case: echo "<green> <blue>" > color_mix Then green intensity will be assigned to value[0] (expects red) and blue to value[1] (expects green). Unless I don't get something. Your ABI documentation doesn't mention any way to redefine the color_id returned by <color>/color_id file. And that is good. > >> >>> + for (; i < LED_COLOR_ID_MAX; i++) >>> + value[i] = 0; >> What use case is it for? > > See above but this should be > > for (; i < mcled_cdev->num_leds; i++) > > >> >>> + } >>> + >>> + list_for_each_entry(priv, &data->color_list, list) { >>> + if (data->cluster_brightness) { >>> + adj_brightness = >>> calculate_brightness(data->cluster_brightness, >>> + value[priv->color_index], >>> + priv->max_intensity); >>> + ret = ops->set_color_brightness(priv->mcled_cdev, >>> + priv->color_id, >>> + adj_brightness); >>> + if (ret < 0) >>> + goto done; >>> + } >>> + >>> + priv->intensity = value[priv->color_index]; >>> + } >> Here we could use just brightness_set op as a single call. We should >> always write all colors as a result of write to color_mix anyway. > > I guess what is gained by just passing the array down to the device > driver and having it > > parse the array and do the peripheral call? Those array values would not be directly written to the device, but used for calculating the actual iout intensities. Driver will just have to call calculate_brightness() (sticking to the naming from this patch) and write the results calculated basing on brightness and max_brightness. > [...] > >>> + >>> + priv->new_intensity = value; >>> + >>> + if (data->cluster_brightness) { >>> + adj_value = calculate_brightness(data->cluster_brightness, >>> + priv->new_intensity, >>> + priv->max_intensity); >>> + ret = ops->set_color_brightness(priv->mcled_cdev, >>> + priv->color_id, adj_value); >>> + if (ret < 0) { >>> + priv->new_intensity = priv->intensity; >> This is unnecessary complication. Just write the calculated iout >> intensity. > > Not sure what complication you are referring to. The whole need for new_intensity and cluster_brightness, and then bringing back old intensity value on set_color_brightness() failure. > >> We need to highlight it in the documentation that exact requested color >> intensity values are written to the hardware only when >> brightness == max_brightness. > > But that is not a true statement. Thats not really how it was designed. But it probably should be. It would simplify the design. So my idea is like I previously described the way I had first understood this design: The colors set under colors directory don't reflect the iout intensities, but are only used for calculating them, basing on the brightness and max_intensity values. Effectively, after changing the colors/<color>/intensity the global (legacy monochrome) brightness value will be still valid, since iout color will be recalculated basing on it and the new color intensity. -- Best regards, Jacek Anaszewski
Dan, On 8/2/19 4:14 PM, Dan Murphy wrote: > Jacek > > On 7/31/19 3:44 PM, Jacek Anaszewski wrote: >> Dan, >> > [...] >> + for (i = 0; i < mcled_cdev->num_leds; i++) { >> + ret = sscanf(buf + offset, "%i%n", &value[i], &nrchars); >> + if (ret != 1) >> + break; >> + >> + offset += nrchars; >> + } >> + >> + if (i != mcled_cdev->num_leds) { >>>> Shouldn't we return error if i != mcled_cdev->num_leds - 1 ? >>>> How can we know which color the value will be for if there is less >>>> values passed than the total number of colors in the cluster? >>> Ok so during my testing if I had the monochrome array as <R G B> >>> >>> When I wrote only <R G> and no blue I was getting random values in the >>> array for the >>> >>> remaining indexes and the blue LED would randomly turn on/off at >>> different levels. >>> >>> So if the user passes in less then expected only ids with data will be >>> written and the other colors will be turned off by the for loop below. >> From what I see it will lead to wrong mapping of given color to the >> value array element in the following case: >> >> echo "<green> <blue>" > color_mix >> >> Then green intensity will be assigned to value[0] (expects red) and blue >> to value[1] (expects green). Unless I don't get something. >> Your ABI documentation doesn't mention any way to redefine the color_id >> returned by <color>/color_id file. And that is good. >> > This is exactly the issue I had previously brought up. The user would > need to > > write all leading colors with a value, even if 0, to correctly set the > target LEDs. > > We can protect against the trailing colors but not leading colors. > > The expectation is that the user space would read the color_id from the > file and align > > the array accordingly. This is also why I exposed the intensity under > the color so if the > > user wanted to not use color_mix file they can update the intensity per > LED color. > I can't see the problem. Just require user to _always_ pass the number of values equal to the number of colors under LED multicolor class device. No more, no less. And fail it this condition is not met. >>>>> + for (; i < LED_COLOR_ID_MAX; i++) >>>>> + value[i] = 0; >>>> What use case is it for? >>> See above but this should be >>> >>> for (; i < mcled_cdev->num_leds; i++) >>> > I might be able to eliminate this loop by initializing the array to 0. > > >>>>> + } >>>>> + >>>>> + list_for_each_entry(priv, &data->color_list, list) { >>>>> + if (data->cluster_brightness) { >>>>> + adj_brightness = >>>>> calculate_brightness(data->cluster_brightness, >>>>> + value[priv->color_index], >>>>> + priv->max_intensity); >>>>> + ret = ops->set_color_brightness(priv->mcled_cdev, >>>>> + priv->color_id, >>>>> + adj_brightness); >>>>> + if (ret < 0) >>>>> + goto done; >>>>> + } >>>>> + >>>>> + priv->intensity = value[priv->color_index]; >>>>> + } >>>> Here we could use just brightness_set op as a single call. We should >>>> always write all colors as a result of write to color_mix anyway. >>> I guess what is gained by just passing the array down to the device >>> driver and having it >>> >>> parse the array and do the peripheral call? >> Those array values would not be directly written to the device, >> but used for calculating the actual iout intensities. Driver >> will just have to call calculate_brightness() (sticking to the naming >> from this patch) and write the results calculated basing on brightness >> and max_brightness. > > I would expect that we would do the same behavior for the color_mix file > then. Yes, this is what I meant while mentioning use of brightness_set op for that. The final effect will be always the same - write of the altered iout intensity registers. What will differ will be factors of the equation that calculates iout intensity: for the call originating from color_mix file the changing factor will be color intensity and for the call originating from brightness file it will be brightness. In both cases on the driver side we need to call the same function to calculate iout values using the same equation. > >> [...] >>>>> + >>>>> + priv->new_intensity = value; >>>>> + >>>>> + if (data->cluster_brightness) { >>>>> + adj_value = calculate_brightness(data->cluster_brightness, >>>>> + priv->new_intensity, >>>>> + priv->max_intensity); >>>>> + ret = ops->set_color_brightness(priv->mcled_cdev, >>>>> + priv->color_id, adj_value); >>>>> + if (ret < 0) { >>>>> + priv->new_intensity = priv->intensity; >>>> This is unnecessary complication. Just write the calculated iout >>>> intensity. >>> Not sure what complication you are referring to. >> The whole need for new_intensity and cluster_brightness, and then >> bringing back old intensity value on set_color_brightness() failure. > > OK > >> >>>> We need to highlight it in the documentation that exact requested color >>>> intensity values are written to the hardware only when >>>> brightness == max_brightness. >>> But that is not a true statement. Thats not really how it was designed. >> But it probably should be. It would simplify the design. >> >> So my idea is like I previously described the way I had first understood >> this design: >> >> The colors set under colors directory don't reflect the iout >> intensities, but are only used for calculating them, basing on the >> brightness and max_intensity values. >> >> Effectively, after changing the colors/<color>/intensity the global >> (legacy monochrome) brightness value will be still valid, since iout >> color will be recalculated basing on it and the new color intensity. >> > OK. This this would remove the ops from the driver as it is no longer > needed. > > The color_mix file will work the same way. I've been pondering over the need for color_set, but it should be indeed not needed too. Write to <color>/intensity file will also call brightness_set - it will be comparing anyway old intensities and new ones and will write only the affected register. > What is the trigger then to update the LEDs? > > We cannot write the same brightness value to trigger as the class blocks > calling down > > to the driver if brightness_in == brightness_current. All files (intensity, color_mix and brightness) will use the same brightness_set op for changing the iout values. -- Best regards, Jacek Anaszewski
Jacek OK finally getting back to this. On 7/29/19 3:45 PM, Jacek Anaszewski wrote: > Hi Dan, > > Thank you for the v4. > > I have a bunch of comments below. Please take a look. > > On 7/25/19 8:28 PM, Dan Murphy wrote: >> Add a documentation of LED Multicolor LED class specific >> sysfs attributes. >> >> Signed-off-by: Dan Murphy<dmurphy@ti.com> >> --- >> .../ABI/testing/sysfs-class-led-multicolor | 67 +++++++++++++++++++ >> 1 file changed, 67 insertions(+) >> create mode 100644 Documentation/ABI/testing/sysfs-class-led-multicolor >> >> diff --git a/Documentation/ABI/testing/sysfs-class-led-multicolor b/Documentation/ABI/testing/sysfs-class-led-multicolor >> new file mode 100644 >> index 000000000000..59839f0eae76 >> --- /dev/null >> +++ b/Documentation/ABI/testing/sysfs-class-led-multicolor >> @@ -0,0 +1,67 @@ >> +What: /sys/class/leds/<led>/brightness >> +Date: Sept 2019 >> +KernelVersion: TBD >> +Contact: Dan Murphy<dmurphy@ti.com> >> +Description: read/write >> + The multicolor class will redirect the device drivers call back >> + function for brightness control to the multicolor class >> + brightness control function. >> + >> + Writing to this file will update all LEDs within the group to a >> + calculated percentage of what each color LED in the group is set >> + to. Please refer to the leds-class-multicolor.txt in the >> + Documentation directory for a complete description. > Instead of redirecting the reader to led-class-multicolor.txt I'd prefer > to have at least the formula to calculate the colors laid out here. > Aside of that - it is more helpful to have a full path to the referenced > file. Ack >> + >> + The value of the color is from 0 to >> + /sys/class/leds/<led>/max_brightness. >> + >> +What: /sys/class/leds/<led>/colors/color_mix >> +Date: Sept 2019 >> +KernelVersion: TBD >> +Contact: Dan Murphy<dmurphy@ti.com> >> +Description: read/write >> + The color_mix file allows writing all registered multicolor LEDs >> + virtually at the same time. The value(s) written to this file > I'd drop parentheses form "value(s)". Multi color LED class device is > supposed to always have more then one LED. And if I understand it > correctly we have to pass intensities of all colors supported by LED > multicolor class device here, even we're changing single one. Yes that is true. >> + contain the intensity values for each multicolor LED within >> + the colors directory. The color indexes are reported in the >> + color_id file as defined in this document. > This is a bit misleading. It sounds as if single color_id file would be > reporting more than one index. > >> + Please refer to the leds-class-multicolor.txt in the >> + Documentation directory for a complete description. > Here, similarly as for brightness, I would prefer to have complete > documentation of this file. > > How about: > > The values written to this file should contain the intensity values of > each multicolor LED within the colors directory. The index of given > color is reported by the color_id file present in colors/<color> > directory. The index determines the position in the sequence of > intensities on which the related intensity should be passed to this > file. > > And here we could have the examples from leds-class-multicolor.txt. I prefer to keep the examples in the leds-class-multicolor.rst. This is an ABI document not a document describing the code. I updated the doc to what you have above. >> + >> +What: /sys/class/leds/<led>/colors/<led_color>/color_id >> +Date: Sept 2019 >> +KernelVersion: TBD >> +Contact: Dan Murphy<dmurphy@ti.com> >> +Description: read only >> + This file when read will return the index of the color in the >> + color_mix. >> + Please refer to the leds-class-multicolor.txt in the >> + Documentation directory for a complete description. >> + >> +What: /sys/class/leds/<led>/colors/<led_color>/intensity >> +Date: Sept 2019 >> +KernelVersion: TBD >> +Contact: Dan Murphy<dmurphy@ti.com> >> +Description: read/write >> + The led_color directory is dynamically created based on the >> + colors defined by the registrar of the class. >> + The led_color can be but not limited to red, green, blue, >> + white, amber, yellow and violet. Drivers can also declare a > Instead of this vague sentence about the available colors I propose to > maintain the list of supported colors in leds-class.rst or in a separate > file and keep it in sync with the led_colors array. Then we could refer > to that file here. I would rather point to the file that contains the colors. This way we don't have added documentation maintenance to add a new color. > >> + LED color for presentation. There is one directory per color > I'd not let drivers define their custom colors. It would entail issues > related to lack of generic LED_COLOR_ID and DT parsing failure. Ack Dan > >> + presented. The brightness file is created under each >> + led_color directory and controls the individual LED color >> + setting. >> + >> + The value of the color is from 0 to >> + /sys/class/leds/<led>/colors/<led_color>/max_intensity. >> + >> +What: /sys/class/leds/<led>/colors/<led_color>/max_intensity >> +Date: Sept 2019 >> +KernelVersion: TBD >> +Contact: Dan Murphy<dmurphy@ti.com> >> +Description: read only >> + Maximum intensity level for the LED color, default is >> + 255 (LED_FULL). >> + >> + If the LED does not support different intensity levels, this >> + should be 1. >>