| Message ID | 20220917081339.3354075-5-jjhiblot@traphandler.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add a multicolor LED driver for groups of monochromatic LEDs | expand |
On Sat, Sep 17, 2022 at 11:14 AM Jean-Jacques Hiblot <jjhiblot@traphandler.com> wrote: > > By allowing to group multiple monochrome LED into multicolor LEDs, > all involved LEDs can be controlled in-sync. This enables using effects > using triggers, etc. ... > +config LEDS_GRP_MULTICOLOR > + tristate "Multi-color LED grouping support" > + help > + This option enables support for monochrome LEDs that are > + grouped into multicolor LEDs. What will be the module name in case of "m" choice? ... > + struct led_mcg_priv *priv = > + container_of(mc_cdev, struct led_mcg_priv, mc_cdev); One line? ... > + /* > + * Scale the intensity according the max brightness of the > + * monochromatic LED Usually we put a grammar period at the end of sentences in multi-line comments. > + */ ... > + actual_led_brightness = DIV_ROUND_CLOSEST( > + mono->max_brightness * mc_cdev->subled_info[i].brightness, > + mc_cdev->led_cdev.max_brightness); Can you fix an indentation, so it won't leave the line ending by open parenthesis? I believe with the help of a temporary variable it can be easily achieved. ... > + for (;;) { > + struct led_classdev *led_cdev; > + led_cdev = devm_of_led_get(dev, count); Why _of_ variant? Please, make this OF independent since it's pretending to cover not only OF-based systems. > + if (IS_ERR(led_cdev)) { > + /* Reached the end of the list ? */ > + if (PTR_ERR(led_cdev) == -ENOENT) > + break; Looks like the above needs an _optional() variant > + return dev_err_probe(dev, PTR_ERR(led_cdev), > + "Unable to get led #%d", count); > + } ... > + priv->monochromatics = devm_krealloc(dev, priv->monochromatics, > + count * sizeof(*priv->monochromatics), > + GFP_KERNEL); This needs at minimum to use one of the helpers from overflow.h, ideally you may implement devm_krealloc_array() as a suitable wrapper for that. > + if (!priv->monochromatics) > + return -ENOMEM; ... > + subled = devm_kzalloc(dev, count * sizeof(*subled), GFP_KERNEL); NIH devm_kcalloc() > + if (!subled) > + return -ENOMEM; -- With Best Regards, Andy Shevchenko
Hi Jean, General question to this driver: since it seems that it is allowed to have duplicate LEDs of the same color, then it will be impossible to distinguish which of the two same colors in multi_index file will refer to which physical LED. Are you aware of this shortcoming? Besides that I have two remarks below. On 9/17/22 10:13, Jean-Jacques Hiblot wrote: > By allowing to group multiple monochrome LED into multicolor LEDs, > all involved LEDs can be controlled in-sync. This enables using effects > using triggers, etc. > > Signed-off-by: Jean-Jacques Hiblot <jjhiblot@traphandler.com> > --- > drivers/leds/rgb/Kconfig | 6 + > drivers/leds/rgb/Makefile | 1 + > drivers/leds/rgb/leds-group-multicolor.c | 153 +++++++++++++++++++++++ > 3 files changed, 160 insertions(+) > create mode 100644 drivers/leds/rgb/leds-group-multicolor.c > > diff --git a/drivers/leds/rgb/Kconfig b/drivers/leds/rgb/Kconfig > index 204cf470beae..c2610c47a82d 100644 > --- a/drivers/leds/rgb/Kconfig > +++ b/drivers/leds/rgb/Kconfig > @@ -2,6 +2,12 @@ > > if LEDS_CLASS_MULTICOLOR > > +config LEDS_GRP_MULTICOLOR > + tristate "Multi-color LED grouping support" > + help > + This option enables support for monochrome LEDs that are > + grouped into multicolor LEDs. > + > config LEDS_PWM_MULTICOLOR > tristate "PWM driven multi-color LED Support" > depends on PWM > diff --git a/drivers/leds/rgb/Makefile b/drivers/leds/rgb/Makefile > index 0675bc0f6e18..4de087ad79bc 100644 > --- a/drivers/leds/rgb/Makefile > +++ b/drivers/leds/rgb/Makefile > @@ -1,4 +1,5 @@ > # SPDX-License-Identifier: GPL-2.0 > > +obj-$(CONFIG_LEDS_GRP_MULTICOLOR) += leds-group-multicolor.o > obj-$(CONFIG_LEDS_PWM_MULTICOLOR) += leds-pwm-multicolor.o > obj-$(CONFIG_LEDS_QCOM_LPG) += leds-qcom-lpg.o > diff --git a/drivers/leds/rgb/leds-group-multicolor.c b/drivers/leds/rgb/leds-group-multicolor.c > new file mode 100644 > index 000000000000..61f9e1954fc4 > --- /dev/null > +++ b/drivers/leds/rgb/leds-group-multicolor.c > @@ -0,0 +1,153 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * multi-color LED built with monochromatic LED devices > + * > + * Copyright 2022 Jean-Jacques Hiblot <jjhiblot@traphandler.com> > + */ > + > +#include <linux/err.h> > +#include <linux/leds.h> > +#include <linux/led-class-multicolor.h> > +#include <linux/math.h> > +#include <linux/module.h> > +#include <linux/mod_devicetable.h> > +#include <linux/platform_device.h> > +#include <linux/property.h> > + > +struct led_mcg_priv { > + struct led_classdev_mc mc_cdev; > + struct led_classdev **monochromatics; > +}; > + > +static int led_mcg_set(struct led_classdev *cdev, > + enum led_brightness brightness) > +{ > + struct led_classdev_mc *mc_cdev = lcdev_to_mccdev(cdev); > + struct led_mcg_priv *priv = > + container_of(mc_cdev, struct led_mcg_priv, mc_cdev); > + int i; > + > + led_mc_calc_color_components(mc_cdev, brightness); > + > + for (i = 0; i < mc_cdev->num_colors; i++) { > + struct led_classdev *mono = priv->monochromatics[i]; > + int actual_led_brightness; > + > + /* > + * Scale the intensity according the max brightness of the > + * monochromatic LED s/according the/according to the/ > + */ > + actual_led_brightness = DIV_ROUND_CLOSEST( > + mono->max_brightness * mc_cdev->subled_info[i].brightness, > + mc_cdev->led_cdev.max_brightness); How about dropping above usage of led_mc_calc_color_components() helper in favour of doing all required calculations here? It would be easier for the reader to grasp the idea then. > + > + led_set_brightness(mono, actual_led_brightness); > + } > + > + return 0; > +} > + > +static int led_mcg_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct led_init_data init_data = {}; > + struct led_classdev *cdev; > + struct mc_subled *subled; > + struct led_mcg_priv *priv; > + int i, count, ret; > + unsigned int max_brightness; > + > + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); > + if (!priv) > + return -ENOMEM; > + > + count = 0; > + max_brightness = 0; > + for (;;) { > + struct led_classdev *led_cdev; > + > + led_cdev = devm_of_led_get(dev, count); > + if (IS_ERR(led_cdev)) { > + /* Reached the end of the list ? */ > + if (PTR_ERR(led_cdev) == -ENOENT) > + break; > + return dev_err_probe(dev, PTR_ERR(led_cdev), > + "Unable to get led #%d", count); > + } > + count++; > + > + priv->monochromatics = devm_krealloc(dev, priv->monochromatics, > + count * sizeof(*priv->monochromatics), > + GFP_KERNEL); > + if (!priv->monochromatics) > + return -ENOMEM; > + > + priv->monochromatics[count - 1] = led_cdev; > + > + max_brightness = max(max_brightness, led_cdev->max_brightness); > + } > + > + subled = devm_kzalloc(dev, count * sizeof(*subled), GFP_KERNEL); > + if (!subled) > + return -ENOMEM; > + priv->mc_cdev.subled_info = subled; > + > + for (i = 0; i < count; i++) { > + struct led_classdev *led_cdev = priv->monochromatics[i]; > + > + subled[i].color_index = led_cdev->color; > + /* configure the LED intensity to its maximum */ > + subled[i].intensity = max_brightness; > + } > + > + /* init the multicolor's LED class device */ > + cdev = &priv->mc_cdev.led_cdev; > + cdev->flags = LED_CORE_SUSPENDRESUME; > + cdev->brightness_set_blocking = led_mcg_set; > + cdev->max_brightness = max_brightness; > + cdev->color = LED_COLOR_ID_MULTI; > + priv->mc_cdev.num_colors = count; > + > + init_data.fwnode = dev_fwnode(dev); > + ret = devm_led_classdev_multicolor_register_ext(dev, &priv->mc_cdev, > + &init_data); > + if (ret) > + return dev_err_probe(dev, ret, > + "failed to register multicolor led for %s.\n", > + cdev->name); > + > + ret = led_mcg_set(cdev, cdev->brightness); > + if (ret) > + return dev_err_probe(dev, ret, > + "failed to set led value for %s.", > + cdev->name); > + > + for (i = 0; i < count; i++) { > + struct led_classdev *led_cdev = priv->monochromatics[i]; > + > + /* Make the sysfs of the monochromatic LED read-only */ > + led_cdev->flags |= LED_SYSFS_DISABLE; > + } > + > + return 0; > +} > + > +static const struct of_device_id of_led_mcg_match[] = { > + { .compatible = "leds-group-multicolor" }, > + {} > +}; > +MODULE_DEVICE_TABLE(of, of_led_mcg_match); > + > +static struct platform_driver led_mcg_driver = { > + .probe = led_mcg_probe, > + .driver = { > + .name = "leds_group_multicolor", > + .of_match_table = of_led_mcg_match, > + } > +}; > +module_platform_driver(led_mcg_driver); > + > +MODULE_AUTHOR("Jean-Jacques Hiblot <jjhiblot@traphandler.com>"); > +MODULE_DESCRIPTION("multi-color LED group driver"); > +MODULE_LICENSE("GPL"); > +MODULE_ALIAS("platform:leds-group-multicolor");
On 17/09/2022 10:37, Andy Shevchenko wrote: > On Sat, Sep 17, 2022 at 11:14 AM Jean-Jacques Hiblot > <jjhiblot@traphandler.com> wrote: >> By allowing to group multiple monochrome LED into multicolor LEDs, >> all involved LEDs can be controlled in-sync. This enables using effects >> using triggers, etc. > ... > >> +config LEDS_GRP_MULTICOLOR >> + tristate "Multi-color LED grouping support" >> + help >> + This option enables support for monochrome LEDs that are >> + grouped into multicolor LEDs. > What will be the module name in case of "m" choice? > > ... > >> + struct led_mcg_priv *priv = >> + container_of(mc_cdev, struct led_mcg_priv, mc_cdev); > One line? > > ... > >> + /* >> + * Scale the intensity according the max brightness of the >> + * monochromatic LED > Usually we put a grammar period at the end of sentences in multi-line comments. > >> + */ > ... > >> + actual_led_brightness = DIV_ROUND_CLOSEST( >> + mono->max_brightness * mc_cdev->subled_info[i].brightness, >> + mc_cdev->led_cdev.max_brightness); > Can you fix an indentation, so it won't leave the line ending by open > parenthesis? I believe with the help of a temporary variable it can be > easily achieved. Could be done but I have no good name for the variable and I think it would just make things harder to understand. In that form I think it is clear that this is a cross-multiplication. > > ... > >> + for (;;) { >> + struct led_classdev *led_cdev; >> + led_cdev = devm_of_led_get(dev, count); > Why _of_ variant? Please, make this OF independent since it's > pretending to cover not only OF-based systems. This is not OF independent. It could be, but that will wait until someone needs it. I don't know much about ACPI and have no hardware to test it on. I'll add the missing dependency on OF in the Kconfig. > > >> + if (IS_ERR(led_cdev)) { >> + /* Reached the end of the list ? */ >> + if (PTR_ERR(led_cdev) == -ENOENT) >> + break; > Looks like the above needs an _optional() variant > >> + return dev_err_probe(dev, PTR_ERR(led_cdev), >> + "Unable to get led #%d", count); >> + } > ... > >> + priv->monochromatics = devm_krealloc(dev, priv->monochromatics, >> + count * sizeof(*priv->monochromatics), >> + GFP_KERNEL); > This needs at minimum to use one of the helpers from overflow.h, > ideally you may implement devm_krealloc_array() as a suitable wrapper > for that. > >> + if (!priv->monochromatics) >> + return -ENOMEM; > ... > >> + subled = devm_kzalloc(dev, count * sizeof(*subled), GFP_KERNEL); > NIH devm_kcalloc() > >> + if (!subled) >> + return -ENOMEM; > -- > With Best Regards, > Andy Shevchenko
On 18/09/2022 16:54, Jacek Anaszewski wrote: > Hi Jean, > > General question to this driver: since it seems that it is allowed to > have duplicate LEDs of the same color, then it will be impossible to > distinguish which of the two same colors in multi_index file will refer > to which physical LED. Are you aware of this shortcoming? Hi Jacek, yes I'm aware of this, but I don't think it can be a problem in a real environment. There will probably few cases were multiple leds of the same color are used and even fewer were those leds need to be controlled differently. > > Besides that I have two remarks below. > > On 9/17/22 10:13, Jean-Jacques Hiblot wrote: >> By allowing to group multiple monochrome LED into multicolor LEDs, >> all involved LEDs can be controlled in-sync. This enables using effects >> using triggers, etc. >> >> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@traphandler.com> >> --- >> drivers/leds/rgb/Kconfig | 6 + >> drivers/leds/rgb/Makefile | 1 + >> drivers/leds/rgb/leds-group-multicolor.c | 153 +++++++++++++++++++++++ >> 3 files changed, 160 insertions(+) >> create mode 100644 drivers/leds/rgb/leds-group-multicolor.c >> >> diff --git a/drivers/leds/rgb/Kconfig b/drivers/leds/rgb/Kconfig >> index 204cf470beae..c2610c47a82d 100644 >> --- a/drivers/leds/rgb/Kconfig >> +++ b/drivers/leds/rgb/Kconfig >> @@ -2,6 +2,12 @@ >> if LEDS_CLASS_MULTICOLOR >> +config LEDS_GRP_MULTICOLOR >> + tristate "Multi-color LED grouping support" >> + help >> + This option enables support for monochrome LEDs that are >> + grouped into multicolor LEDs. >> + >> config LEDS_PWM_MULTICOLOR >> tristate "PWM driven multi-color LED Support" >> depends on PWM >> diff --git a/drivers/leds/rgb/Makefile b/drivers/leds/rgb/Makefile >> index 0675bc0f6e18..4de087ad79bc 100644 >> --- a/drivers/leds/rgb/Makefile >> +++ b/drivers/leds/rgb/Makefile >> @@ -1,4 +1,5 @@ >> # SPDX-License-Identifier: GPL-2.0 >> +obj-$(CONFIG_LEDS_GRP_MULTICOLOR) += leds-group-multicolor.o >> obj-$(CONFIG_LEDS_PWM_MULTICOLOR) += leds-pwm-multicolor.o >> obj-$(CONFIG_LEDS_QCOM_LPG) += leds-qcom-lpg.o >> diff --git a/drivers/leds/rgb/leds-group-multicolor.c >> b/drivers/leds/rgb/leds-group-multicolor.c >> new file mode 100644 >> index 000000000000..61f9e1954fc4 >> --- /dev/null >> +++ b/drivers/leds/rgb/leds-group-multicolor.c >> @@ -0,0 +1,153 @@ >> +// SPDX-License-Identifier: GPL-2.0-only >> +/* >> + * multi-color LED built with monochromatic LED devices >> + * >> + * Copyright 2022 Jean-Jacques Hiblot <jjhiblot@traphandler.com> >> + */ >> + >> +#include <linux/err.h> >> +#include <linux/leds.h> >> +#include <linux/led-class-multicolor.h> >> +#include <linux/math.h> >> +#include <linux/module.h> >> +#include <linux/mod_devicetable.h> >> +#include <linux/platform_device.h> >> +#include <linux/property.h> >> + >> +struct led_mcg_priv { >> + struct led_classdev_mc mc_cdev; >> + struct led_classdev **monochromatics; >> +}; >> + >> +static int led_mcg_set(struct led_classdev *cdev, >> + enum led_brightness brightness) >> +{ >> + struct led_classdev_mc *mc_cdev = lcdev_to_mccdev(cdev); >> + struct led_mcg_priv *priv = >> + container_of(mc_cdev, struct led_mcg_priv, mc_cdev); >> + int i; >> + >> + led_mc_calc_color_components(mc_cdev, brightness); >> + >> + for (i = 0; i < mc_cdev->num_colors; i++) { >> + struct led_classdev *mono = priv->monochromatics[i]; >> + int actual_led_brightness; >> + >> + /* >> + * Scale the intensity according the max brightness of the >> + * monochromatic LED > s/according the/according to the/ > >> + */ >> + actual_led_brightness = DIV_ROUND_CLOSEST( >> + mono->max_brightness * mc_cdev->subled_info[i].brightness, >> + mc_cdev->led_cdev.max_brightness); > > How about dropping above usage of led_mc_calc_color_components() > helper in favour of doing all required calculations here? > It would be easier for the reader to grasp the idea then. > >> + >> + led_set_brightness(mono, actual_led_brightness); >> + } >> + >> + return 0; >> +} >> + >> +static int led_mcg_probe(struct platform_device *pdev) >> +{ >> + struct device *dev = &pdev->dev; >> + struct led_init_data init_data = {}; >> + struct led_classdev *cdev; >> + struct mc_subled *subled; >> + struct led_mcg_priv *priv; >> + int i, count, ret; >> + unsigned int max_brightness; >> + >> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); >> + if (!priv) >> + return -ENOMEM; >> + >> + count = 0; >> + max_brightness = 0; >> + for (;;) { >> + struct led_classdev *led_cdev; >> + >> + led_cdev = devm_of_led_get(dev, count); >> + if (IS_ERR(led_cdev)) { >> + /* Reached the end of the list ? */ >> + if (PTR_ERR(led_cdev) == -ENOENT) >> + break; >> + return dev_err_probe(dev, PTR_ERR(led_cdev), >> + "Unable to get led #%d", count); >> + } >> + count++; >> + >> + priv->monochromatics = devm_krealloc(dev, priv->monochromatics, >> + count * sizeof(*priv->monochromatics), >> + GFP_KERNEL); >> + if (!priv->monochromatics) >> + return -ENOMEM; >> + >> + priv->monochromatics[count - 1] = led_cdev; >> + >> + max_brightness = max(max_brightness, led_cdev->max_brightness); >> + } >> + >> + subled = devm_kzalloc(dev, count * sizeof(*subled), GFP_KERNEL); >> + if (!subled) >> + return -ENOMEM; >> + priv->mc_cdev.subled_info = subled; >> + >> + for (i = 0; i < count; i++) { >> + struct led_classdev *led_cdev = priv->monochromatics[i]; >> + >> + subled[i].color_index = led_cdev->color; >> + /* configure the LED intensity to its maximum */ >> + subled[i].intensity = max_brightness; >> + } >> + >> + /* init the multicolor's LED class device */ >> + cdev = &priv->mc_cdev.led_cdev; >> + cdev->flags = LED_CORE_SUSPENDRESUME; >> + cdev->brightness_set_blocking = led_mcg_set; >> + cdev->max_brightness = max_brightness; >> + cdev->color = LED_COLOR_ID_MULTI; >> + priv->mc_cdev.num_colors = count; >> + >> + init_data.fwnode = dev_fwnode(dev); >> + ret = devm_led_classdev_multicolor_register_ext(dev, >> &priv->mc_cdev, >> + &init_data); >> + if (ret) >> + return dev_err_probe(dev, ret, >> + "failed to register multicolor led for %s.\n", >> + cdev->name); >> + >> + ret = led_mcg_set(cdev, cdev->brightness); >> + if (ret) >> + return dev_err_probe(dev, ret, >> + "failed to set led value for %s.", >> + cdev->name); >> + >> + for (i = 0; i < count; i++) { >> + struct led_classdev *led_cdev = priv->monochromatics[i]; >> + >> + /* Make the sysfs of the monochromatic LED read-only */ >> + led_cdev->flags |= LED_SYSFS_DISABLE; >> + } >> + >> + return 0; >> +} >> + >> +static const struct of_device_id of_led_mcg_match[] = { >> + { .compatible = "leds-group-multicolor" }, >> + {} >> +}; >> +MODULE_DEVICE_TABLE(of, of_led_mcg_match); >> + >> +static struct platform_driver led_mcg_driver = { >> + .probe = led_mcg_probe, >> + .driver = { >> + .name = "leds_group_multicolor", >> + .of_match_table = of_led_mcg_match, >> + } >> +}; >> +module_platform_driver(led_mcg_driver); >> + >> +MODULE_AUTHOR("Jean-Jacques Hiblot <jjhiblot@traphandler.com>"); >> +MODULE_DESCRIPTION("multi-color LED group driver"); >> +MODULE_LICENSE("GPL"); >> +MODULE_ALIAS("platform:leds-group-multicolor"); >
On Fri, Oct 7, 2022 at 9:34 AM Jean-Jacques Hiblot <jjhiblot@traphandler.com> wrote: > On 17/09/2022 10:37, Andy Shevchenko wrote: > > On Sat, Sep 17, 2022 at 11:14 AM Jean-Jacques Hiblot > > <jjhiblot@traphandler.com> wrote: ... > >> + led_cdev = devm_of_led_get(dev, count); > > Why _of_ variant? Please, make this OF independent since it's > > pretending to cover not only OF-based systems. > > This is not OF independent. It could be, but that will wait until > someone needs it. I don't know much about ACPI and have no hardware to > test it on. > > I'll add the missing dependency on OF in the Kconfig. No, please consider getting rid of OF-centric API usage. ... I'm not sure why you left a lot of context in the reply without commenting or replying to it.
On 07/10/2022 10:53, Andy Shevchenko wrote: > On Fri, Oct 7, 2022 at 9:34 AM Jean-Jacques Hiblot > <jjhiblot@traphandler.com> wrote: >> On 17/09/2022 10:37, Andy Shevchenko wrote: >>> On Sat, Sep 17, 2022 at 11:14 AM Jean-Jacques Hiblot >>> <jjhiblot@traphandler.com> wrote: > ... > >>>> + led_cdev = devm_of_led_get(dev, count); >>> Why _of_ variant? Please, make this OF independent since it's >>> pretending to cover not only OF-based systems. >> This is not OF independent. It could be, but that will wait until >> someone needs it. I don't know much about ACPI and have no hardware to >> test it on. >> >> I'll add the missing dependency on OF in the Kconfig. > No, please consider getting rid of OF-centric API usage. The trouble is that the OF-agnostic API for leds doesn't exist yet and I don't really want to add it without any way to test it. > > ... > > I'm not sure why you left a lot of context in the reply without > commenting or replying to it. >
On Fri, Oct 7, 2022 at 3:03 PM Jean-Jacques Hiblot <jjhiblot@traphandler.com> wrote: > On 07/10/2022 10:53, Andy Shevchenko wrote: > > On Fri, Oct 7, 2022 at 9:34 AM Jean-Jacques Hiblot > > <jjhiblot@traphandler.com> wrote: > >> On 17/09/2022 10:37, Andy Shevchenko wrote: > >>> On Sat, Sep 17, 2022 at 11:14 AM Jean-Jacques Hiblot > >>> <jjhiblot@traphandler.com> wrote: ... > >>>> + led_cdev = devm_of_led_get(dev, count); > >>> Why _of_ variant? Please, make this OF independent since it's > >>> pretending to cover not only OF-based systems. > >> This is not OF independent. It could be, but that will wait until > >> someone needs it. I don't know much about ACPI and have no hardware to > >> test it on. > >> > >> I'll add the missing dependency on OF in the Kconfig. > > No, please consider getting rid of OF-centric API usage. > > The trouble is that the OF-agnostic API for leds doesn't exist yet and I > don't really want to add it without any way to test it. Yeah, that might be a problem due to unestablished descriptions outside DT. Anyway, it seems harmless to call that function when there is no OF dependency. In such cases it will fail with a deferred probe.
diff --git a/drivers/leds/rgb/Kconfig b/drivers/leds/rgb/Kconfig index 204cf470beae..c2610c47a82d 100644 --- a/drivers/leds/rgb/Kconfig +++ b/drivers/leds/rgb/Kconfig @@ -2,6 +2,12 @@ if LEDS_CLASS_MULTICOLOR +config LEDS_GRP_MULTICOLOR + tristate "Multi-color LED grouping support" + help + This option enables support for monochrome LEDs that are + grouped into multicolor LEDs. + config LEDS_PWM_MULTICOLOR tristate "PWM driven multi-color LED Support" depends on PWM diff --git a/drivers/leds/rgb/Makefile b/drivers/leds/rgb/Makefile index 0675bc0f6e18..4de087ad79bc 100644 --- a/drivers/leds/rgb/Makefile +++ b/drivers/leds/rgb/Makefile @@ -1,4 +1,5 @@ # SPDX-License-Identifier: GPL-2.0 +obj-$(CONFIG_LEDS_GRP_MULTICOLOR) += leds-group-multicolor.o obj-$(CONFIG_LEDS_PWM_MULTICOLOR) += leds-pwm-multicolor.o obj-$(CONFIG_LEDS_QCOM_LPG) += leds-qcom-lpg.o diff --git a/drivers/leds/rgb/leds-group-multicolor.c b/drivers/leds/rgb/leds-group-multicolor.c new file mode 100644 index 000000000000..61f9e1954fc4 --- /dev/null +++ b/drivers/leds/rgb/leds-group-multicolor.c @@ -0,0 +1,153 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * multi-color LED built with monochromatic LED devices + * + * Copyright 2022 Jean-Jacques Hiblot <jjhiblot@traphandler.com> + */ + +#include <linux/err.h> +#include <linux/leds.h> +#include <linux/led-class-multicolor.h> +#include <linux/math.h> +#include <linux/module.h> +#include <linux/mod_devicetable.h> +#include <linux/platform_device.h> +#include <linux/property.h> + +struct led_mcg_priv { + struct led_classdev_mc mc_cdev; + struct led_classdev **monochromatics; +}; + +static int led_mcg_set(struct led_classdev *cdev, + enum led_brightness brightness) +{ + struct led_classdev_mc *mc_cdev = lcdev_to_mccdev(cdev); + struct led_mcg_priv *priv = + container_of(mc_cdev, struct led_mcg_priv, mc_cdev); + int i; + + led_mc_calc_color_components(mc_cdev, brightness); + + for (i = 0; i < mc_cdev->num_colors; i++) { + struct led_classdev *mono = priv->monochromatics[i]; + int actual_led_brightness; + + /* + * Scale the intensity according the max brightness of the + * monochromatic LED + */ + actual_led_brightness = DIV_ROUND_CLOSEST( + mono->max_brightness * mc_cdev->subled_info[i].brightness, + mc_cdev->led_cdev.max_brightness); + + led_set_brightness(mono, actual_led_brightness); + } + + return 0; +} + +static int led_mcg_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct led_init_data init_data = {}; + struct led_classdev *cdev; + struct mc_subled *subled; + struct led_mcg_priv *priv; + int i, count, ret; + unsigned int max_brightness; + + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); + if (!priv) + return -ENOMEM; + + count = 0; + max_brightness = 0; + for (;;) { + struct led_classdev *led_cdev; + + led_cdev = devm_of_led_get(dev, count); + if (IS_ERR(led_cdev)) { + /* Reached the end of the list ? */ + if (PTR_ERR(led_cdev) == -ENOENT) + break; + return dev_err_probe(dev, PTR_ERR(led_cdev), + "Unable to get led #%d", count); + } + count++; + + priv->monochromatics = devm_krealloc(dev, priv->monochromatics, + count * sizeof(*priv->monochromatics), + GFP_KERNEL); + if (!priv->monochromatics) + return -ENOMEM; + + priv->monochromatics[count - 1] = led_cdev; + + max_brightness = max(max_brightness, led_cdev->max_brightness); + } + + subled = devm_kzalloc(dev, count * sizeof(*subled), GFP_KERNEL); + if (!subled) + return -ENOMEM; + priv->mc_cdev.subled_info = subled; + + for (i = 0; i < count; i++) { + struct led_classdev *led_cdev = priv->monochromatics[i]; + + subled[i].color_index = led_cdev->color; + /* configure the LED intensity to its maximum */ + subled[i].intensity = max_brightness; + } + + /* init the multicolor's LED class device */ + cdev = &priv->mc_cdev.led_cdev; + cdev->flags = LED_CORE_SUSPENDRESUME; + cdev->brightness_set_blocking = led_mcg_set; + cdev->max_brightness = max_brightness; + cdev->color = LED_COLOR_ID_MULTI; + priv->mc_cdev.num_colors = count; + + init_data.fwnode = dev_fwnode(dev); + ret = devm_led_classdev_multicolor_register_ext(dev, &priv->mc_cdev, + &init_data); + if (ret) + return dev_err_probe(dev, ret, + "failed to register multicolor led for %s.\n", + cdev->name); + + ret = led_mcg_set(cdev, cdev->brightness); + if (ret) + return dev_err_probe(dev, ret, + "failed to set led value for %s.", + cdev->name); + + for (i = 0; i < count; i++) { + struct led_classdev *led_cdev = priv->monochromatics[i]; + + /* Make the sysfs of the monochromatic LED read-only */ + led_cdev->flags |= LED_SYSFS_DISABLE; + } + + return 0; +} + +static const struct of_device_id of_led_mcg_match[] = { + { .compatible = "leds-group-multicolor" }, + {} +}; +MODULE_DEVICE_TABLE(of, of_led_mcg_match); + +static struct platform_driver led_mcg_driver = { + .probe = led_mcg_probe, + .driver = { + .name = "leds_group_multicolor", + .of_match_table = of_led_mcg_match, + } +}; +module_platform_driver(led_mcg_driver); + +MODULE_AUTHOR("Jean-Jacques Hiblot <jjhiblot@traphandler.com>"); +MODULE_DESCRIPTION("multi-color LED group driver"); +MODULE_LICENSE("GPL"); +MODULE_ALIAS("platform:leds-group-multicolor");
By allowing to group multiple monochrome LED into multicolor LEDs, all involved LEDs can be controlled in-sync. This enables using effects using triggers, etc. Signed-off-by: Jean-Jacques Hiblot <jjhiblot@traphandler.com> --- drivers/leds/rgb/Kconfig | 6 + drivers/leds/rgb/Makefile | 1 + drivers/leds/rgb/leds-group-multicolor.c | 153 +++++++++++++++++++++++ 3 files changed, 160 insertions(+) create mode 100644 drivers/leds/rgb/leds-group-multicolor.c