| Message ID | 20240114190652.7109-2-jstrauss@mailbox.org |
|---|---|
| State | New |
| Headers | show |
| Series | [v5] Add multicolor support to BlinkM LED driver | expand |
On Sun, 14 Jan 2024, Joseph Strauss wrote: > Add multicolor support to the BlinkM driver, making it easier to control > from userspace. The BlinkM LED is a programmable RGB LED. The driver > currently supports only the regular LED sysfs class, resulting in the > creation of three distinct classes, one for red, green, and blue. The > user then has to input three values into the three seperate brightness > files within those classes. The multicolor LED framework makes the > device easier to control with the multi_intensity file: the user can > input three values at once to form a color, while still controlling the > lightness with the brightness file. > > The main struct blinkm_led has changed slightly. The struct led_classdev > for the regular sysfs classes remain. The blinkm_probe function checks > CONFIG_LEDS_BLINKM_MULTICOLOR to decide whether to load the seperate > sysfs classes or the single multicolor one, but never both. The > blinkm_set_mc_brightness() function had to be added to calculate the > three color components and then set the fields of the blinkm_data > structure accordingly. > > All of the feedback has been much appreciated. Thanks! This needs to go below the '---'. So you've had feedback but no positive reviews resulting in a *-by? Have you Cc:ed all of your reviewers in this version? > Signed-off-by: Joseph Strauss <jstrauss@mailbox.org> > --- > Changes in v2: > - Replaced instances of the constant 3 with NUM_LEDS, where applicable > - Fixed formatting errors > - Replaced loop inside of blinkm_set_mc_brightness() with equivalent > statements > - Changed id of multicolor class from 4 to 3 > - Replaced call to devm_kmalloc_array() with devm_kcalloc() > Changes in v3: > - Add CONFIG_LEDS_BLINKM_MULTICOLOR to check whether to use multicolor > funcitonality > - Extend well-known-leds.txt to include standard names for RGB and indicator > LEDS > - Change name of Blinkm sysfs class according to well-known-leds.txt > - Simplify struct blinkm_led and struct blinkm_data > - Remove magic numbers > - Fix formatting errors > - Remove unrelated changes > Changes in v4: > - Fix indentation > - Add default case to switch statement > Changes in v5: > - Fix warnings related to snprintf on s390 architecture, reported by > 0-DAY CI Kernel Test Service > > Documentation/leds/leds-blinkm.rst | 27 ++- > Documentation/leds/well-known-leds.txt | 8 + > drivers/leds/Kconfig | 8 + > drivers/leds/leds-blinkm.c | 217 +++++++++++++++++-------- > 4 files changed, 185 insertions(+), 75 deletions(-) Looking pretty good. Just a couple of minor nits. > diff --git a/Documentation/leds/leds-blinkm.rst b/Documentation/leds/leds-blinkm.rst > index c74b5bc877b1..16883c2a9a99 100644 > --- a/Documentation/leds/leds-blinkm.rst > +++ b/Documentation/leds/leds-blinkm.rst > @@ -13,9 +13,27 @@ The device accepts RGB and HSB color values through separate commands. > Also you can store blinking sequences as "scripts" in > the controller and run them. Also fading is an option. > > -The interface this driver provides is 2-fold: > +The interface this driver provides is 3-fold: > > -a) LED class interface for use with triggers > +a) LED multicolor class interface for use with triggers > +####################################################### > + > +The registration follows the scheme:: > + > + blinkm-<i2c-bus-nr>-<i2c-device-nr>:rgb:indicator > + > + $ ls -h /sys/class/leds/blinkm-1-9:rgb:indicator The `-h` is superfluous, no? > + brightness device max_brightness multi_index multi_intensity power subsystem trigger uevent > + > +The order in which to write the intensity values can be found in multi_index. > +Exactly three values between 0 and 255 must be written to multi_intensity to change the color:: :: ? > + > + $ echo 255 100 50 > multi_intensity > + > +The overall brightness of the color that you choose can also be changed by > +writing a value between 0 and 255 to the brightness file. So what's the difference between brightness and intensity? > +b) LED class interface for use with triggers > ############################################ > > The registration follows the scheme:: > @@ -50,7 +68,7 @@ E.g.:: > $ > > > -b) Sysfs group to control rgb, fade, hsb, scripts ... > +c) Sysfs group to control rgb, fade, hsb, scripts ... > ##################################################### > > This extended interface is available as folder blinkm > @@ -79,6 +97,7 @@ E.g.:: > > > > -as of 6/2012 > +as of 01/2024 > > dl9pf <at> gmx <dot> de > +jstrauss <at> mailbox <dot> org > diff --git a/Documentation/leds/well-known-leds.txt b/Documentation/leds/well-known-leds.txt > index 2160382c86be..2ac4eaed1454 100644 > --- a/Documentation/leds/well-known-leds.txt > +++ b/Documentation/leds/well-known-leds.txt > @@ -70,3 +70,11 @@ Good: "platform:*:charging" (allwinner sun50i) > * Screen > > Good: ":backlight" (Motorola Droid 4) > + > +* Indicators > + > +Good: ":indicator" (Blinkm) > + > +* RGB > + > +Good: ":rgb" (Blinkm) > diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig > index 499d0f215a8b..f38d786f9a89 100644 > --- a/drivers/leds/Kconfig > +++ b/drivers/leds/Kconfig > @@ -736,6 +736,14 @@ config LEDS_BLINKM > This option enables support for the BlinkM RGB LED connected > through I2C. Say Y to enable support for the BlinkM LED. > > +config LEDS_BLINKM_MULTICOLOR > + bool "Enable multicolor support for BlinkM I2C RGB LED" > + depends on LEDS_BLINKM > + depends on LEDS_CLASS_MULTICOLOR > + help > + This option enables multicolor sysfs class support for BlinkM LED and > + disables the older, separated sysfs interface > + > config LEDS_POWERNV > tristate "LED support for PowerNV Platform" > depends on LEDS_CLASS > diff --git a/drivers/leds/leds-blinkm.c b/drivers/leds/leds-blinkm.c > index e19cc8a7b7ca..d6ce2cccb816 100644 > --- a/drivers/leds/leds-blinkm.c > +++ b/drivers/leds/leds-blinkm.c > @@ -2,6 +2,7 @@ > /* > * leds-blinkm.c > * (c) Jan-Simon Möller (dl9pf@gmx.de) > + * (c) Joseph Strauss (jstrauss@mailbox.org) > */ > > #include <linux/module.h> > @@ -15,6 +16,10 @@ > #include <linux/pm_runtime.h> > #include <linux/leds.h> > #include <linux/delay.h> > +#include <linux/led-class-multicolor.h> > +#include <linux/kconfig.h> > + > +#define NUM_LEDS 3 > > /* Addresses to scan - BlinkM is on 0x09 by default*/ > static const unsigned short normal_i2c[] = { 0x09, I2C_CLIENT_END }; > @@ -22,19 +27,24 @@ static const unsigned short normal_i2c[] = { 0x09, I2C_CLIENT_END }; > static int blinkm_transfer_hw(struct i2c_client *client, int cmd); > static int blinkm_test_run(struct i2c_client *client); > > +/* Contains data structures for both the color-seperated sysfs classes, and the new multicolor class */ > struct blinkm_led { > struct i2c_client *i2c_client; > + /* used when multicolor support is disabled */ There's a syntax for either/or in structs. Isn't it 'union'? Can/does that apply here? > struct led_classdev led_cdev; > + struct led_classdev_mc mcled_cdev; > int id; > }; > > -#define cdev_to_blmled(c) container_of(c, struct blinkm_led, led_cdev) > +#define led_cdev_to_blmled(c) container_of(c, struct blinkm_led, led_cdev) > +#define mcled_cdev_to_led(c) container_of(c, struct blinkm_led, mcled_cdev) > > struct blinkm_data { > struct i2c_client *i2c_client; > struct mutex update_lock; > /* used for led class interface */ > - struct blinkm_led blinkm_leds[3]; > + struct blinkm_led blinkm_leds[NUM_LEDS]; > + > /* used for "blinkm" sysfs interface */ > u8 red; /* color red */ > u8 green; /* color green */ > @@ -419,11 +429,29 @@ static int blinkm_transfer_hw(struct i2c_client *client, int cmd) > return 0; > } > > +static int blinkm_set_mc_brightness(struct led_classdev *led_cdev, > + enum led_brightness value) > +{ > + struct led_classdev_mc *mcled_cdev = lcdev_to_mccdev(led_cdev); > + struct blinkm_led *led = mcled_cdev_to_led(mcled_cdev); > + struct blinkm_data *data = i2c_get_clientdata(led->i2c_client); > + > + led_mc_calc_color_components(mcled_cdev, value); > + > + data->next_red = (u8) mcled_cdev->subled_info[RED].brightness; > + data->next_green = (u8) mcled_cdev->subled_info[GREEN].brightness; > + data->next_blue = (u8) mcled_cdev->subled_info[BLUE].brightness; > + > + blinkm_transfer_hw(led->i2c_client, BLM_GO_RGB); > + > + return 0; > +} > + > static int blinkm_led_common_set(struct led_classdev *led_cdev, > enum led_brightness value, int color) > { > /* led_brightness is 0, 127 or 255 - we just use it here as-is */ > - struct blinkm_led *led = cdev_to_blmled(led_cdev); > + struct blinkm_led *led = led_cdev_to_blmled(led_cdev); > struct blinkm_data *data = i2c_get_clientdata(led->i2c_client); > > switch (color) { > @@ -569,7 +597,11 @@ static int blinkm_probe(struct i2c_client *client, > const struct i2c_device_id *id) > { > struct blinkm_data *data; > - struct blinkm_led *led[3]; > + /* For multicolor support */ > + struct blinkm_led *mc_led; > + struct mc_subled *mc_led_info; > + /* 3 seperate classes for red, green, and blue respectively */ > + struct blinkm_led *leds[NUM_LEDS]; > int err, i; > char blinkm_led_name[28]; > > @@ -580,6 +612,12 @@ static int blinkm_probe(struct i2c_client *client, > goto exit; > } > > + mc_led_info = devm_kcalloc(&client->dev, NUM_LEDS, sizeof(*mc_led_info), > + GFP_KERNEL); > + if (!mc_led_info) { > + err = -ENOMEM; > + goto exit; > + } > data->i2c_addr = 0x08; > /* i2c addr - use fake addr of 0x08 initially (real is 0x09) */ > data->fw_ver = 0xfe; > @@ -598,81 +636,118 @@ static int blinkm_probe(struct i2c_client *client, > goto exit; > } > > - for (i = 0; i < 3; i++) { > - /* RED = 0, GREEN = 1, BLUE = 2 */ > - led[i] = &data->blinkm_leds[i]; > - led[i]->i2c_client = client; > - led[i]->id = i; > - led[i]->led_cdev.max_brightness = 255; > - led[i]->led_cdev.flags = LED_CORE_SUSPENDRESUME; > - switch (i) { > - case RED: > - snprintf(blinkm_led_name, sizeof(blinkm_led_name), > - "blinkm-%d-%d-red", > - client->adapter->nr, > - client->addr); > - led[i]->led_cdev.name = blinkm_led_name; > - led[i]->led_cdev.brightness_set_blocking = > - blinkm_led_red_set; > - err = led_classdev_register(&client->dev, > - &led[i]->led_cdev); > - if (err < 0) { > - dev_err(&client->dev, > - "couldn't register LED %s\n", > - led[i]->led_cdev.name); > - goto failred; > - } > - break; > - case GREEN: > - snprintf(blinkm_led_name, sizeof(blinkm_led_name), > - "blinkm-%d-%d-green", > - client->adapter->nr, > - client->addr); > - led[i]->led_cdev.name = blinkm_led_name; > - led[i]->led_cdev.brightness_set_blocking = > - blinkm_led_green_set; > - err = led_classdev_register(&client->dev, > - &led[i]->led_cdev); > - if (err < 0) { > - dev_err(&client->dev, > - "couldn't register LED %s\n", > - led[i]->led_cdev.name); > - goto failgreen; > - } > - break; > - case BLUE: > - snprintf(blinkm_led_name, sizeof(blinkm_led_name), > - "blinkm-%d-%d-blue", > - client->adapter->nr, > - client->addr); > - led[i]->led_cdev.name = blinkm_led_name; > - led[i]->led_cdev.brightness_set_blocking = > - blinkm_led_blue_set; > - err = led_classdev_register(&client->dev, > - &led[i]->led_cdev); > - if (err < 0) { > - dev_err(&client->dev, > - "couldn't register LED %s\n", > - led[i]->led_cdev.name); > - goto failblue; > - } > - break; > - } /* end switch */ > - } /* end for */ > - > - /* Initialize the blinkm */ > + if (!IS_ENABLED(CONFIG_LEDS_BLINKM_MULTICOLOR)) { > + /* Register red, green, and blue sysfs classes */ > + for (i = 0; i < NUM_LEDS; i++) { *Even* more indentation! Probably worth moving the whole section into a function at this point. > + /* RED = 0, GREEN = 1, BLUE = 2 */ > + leds[i] = &data->blinkm_leds[i]; > + leds[i]->i2c_client = client; > + leds[i]->id = i; > + leds[i]->led_cdev.max_brightness = 255; > + leds[i]->led_cdev.flags = LED_CORE_SUSPENDRESUME; > + switch (i) { > + case RED: > + scnprintf(blinkm_led_name, sizeof(blinkm_led_name), > + "blinkm-%d-%d-red", > + client->adapter->nr, > + client->addr); > + leds[i]->led_cdev.name = blinkm_led_name; > + leds[i]->led_cdev.brightness_set_blocking = > + blinkm_led_red_set; > + err = led_classdev_register(&client->dev, > + &leds[i]->led_cdev); > + if (err < 0) { > + dev_err(&client->dev, > + "couldn't register LED %s\n", > + leds[i]->led_cdev.name); > + goto failred; > + } > + break; > + case GREEN: > + scnprintf(blinkm_led_name, sizeof(blinkm_led_name), > + "blinkm-%d-%d-green", > + client->adapter->nr, > + client->addr); > + leds[i]->led_cdev.name = blinkm_led_name; > + leds[i]->led_cdev.brightness_set_blocking = > + blinkm_led_green_set; > + err = led_classdev_register(&client->dev, > + &leds[i]->led_cdev); > + if (err < 0) { > + dev_err(&client->dev, > + "couldn't register LED %s\n", > + leds[i]->led_cdev.name); > + goto failgreen; > + } > + break; > + case BLUE: > + scnprintf(blinkm_led_name, sizeof(blinkm_led_name), > + "blinkm-%d-%d-blue", > + client->adapter->nr, > + client->addr); > + leds[i]->led_cdev.name = blinkm_led_name; > + leds[i]->led_cdev.brightness_set_blocking = > + blinkm_led_blue_set; > + err = led_classdev_register(&client->dev, > + &leds[i]->led_cdev); > + if (err < 0) { > + dev_err(&client->dev, > + "couldn't register LED %s\n", > + leds[i]->led_cdev.name); > + goto failblue; > + } > + break; > + default: > + break; > + } /* end switch */ > + } /* end for */ > + } else { > + /* Register multicolor sysfs class */ > + /* The first element of leds is used for multicolor facilities */ > + mc_led = &data->blinkm_leds[RED]; > + mc_led->i2c_client = client; > + > + mc_led_info[RED].color_index = LED_COLOR_ID_RED; > + mc_led_info[GREEN].color_index = LED_COLOR_ID_GREEN; > + mc_led_info[BLUE].color_index = LED_COLOR_ID_BLUE; > + > + mc_led->mcled_cdev.subled_info = mc_led_info; > + mc_led->mcled_cdev.num_colors = NUM_LEDS; > + mc_led->mcled_cdev.led_cdev.brightness = 255; > + mc_led->mcled_cdev.led_cdev.max_brightness = 255; > + mc_led->mcled_cdev.led_cdev.flags = LED_CORE_SUSPENDRESUME; > + > + scnprintf(blinkm_led_name, sizeof(blinkm_led_name), > + "blinkm-%d-%d:rgb:indicator", > + client->adapter->nr, > + client->addr); > + mc_led->mcled_cdev.led_cdev.name = blinkm_led_name; > + mc_led->mcled_cdev.led_cdev.brightness_set_blocking = blinkm_set_mc_brightness; > + > + err = led_classdev_multicolor_register(&client->dev, &mc_led->mcled_cdev); > + if (err < 0) { > + dev_err(&client->dev, "couldn't register LED %s\n", > + mc_led->led_cdev.name); > + goto failmulti; > + } > + } > + > blinkm_init_hw(client); > > return 0; > > +failmulti: > + led_classdev_unregister(&leds[BLUE]->led_cdev); > + > failblue: > - led_classdev_unregister(&led[GREEN]->led_cdev); > + led_classdev_unregister(&leds[GREEN]->led_cdev); > > failgreen: > - led_classdev_unregister(&led[RED]->led_cdev); > + led_classdev_unregister(&leds[RED]->led_cdev); > > failred: > sysfs_remove_group(&client->dev.kobj, &blinkm_group); > + > exit: > return err; > } > @@ -684,7 +759,7 @@ static void blinkm_remove(struct i2c_client *client) > int i; > > /* make sure no workqueue entries are pending */ > - for (i = 0; i < 3; i++) > + for (i = 0; i < NUM_LEDS; i++) > led_classdev_unregister(&data->blinkm_leds[i].led_cdev); > > /* reset rgb */ > @@ -741,6 +816,6 @@ static struct i2c_driver blinkm_driver = { > module_i2c_driver(blinkm_driver); > > MODULE_AUTHOR("Jan-Simon Moeller <dl9pf@gmx.de>"); > +MODULE_AUTHOR("Joseph Strauss <jstrauss@mailbox.org>"); > MODULE_DESCRIPTION("BlinkM RGB LED driver"); > MODULE_LICENSE("GPL"); > - > -- > 2.42.0 >
On 24/01/25 01:22PM, Lee Jones wrote: > On Sun, 14 Jan 2024, Joseph Strauss wrote: > > > Add multicolor support to the BlinkM driver, making it easier to control > > from userspace. The BlinkM LED is a programmable RGB LED. The driver > > currently supports only the regular LED sysfs class, resulting in the > > creation of three distinct classes, one for red, green, and blue. The > > user then has to input three values into the three seperate brightness > > files within those classes. The multicolor LED framework makes the > > device easier to control with the multi_intensity file: the user can > > input three values at once to form a color, while still controlling the > > lightness with the brightness file. > > > > The main struct blinkm_led has changed slightly. The struct led_classdev > > for the regular sysfs classes remain. The blinkm_probe function checks > > CONFIG_LEDS_BLINKM_MULTICOLOR to decide whether to load the seperate > > sysfs classes or the single multicolor one, but never both. The > > blinkm_set_mc_brightness() function had to be added to calculate the > > three color components and then set the fields of the blinkm_data > > structure accordingly. > > > > All of the feedback has been much appreciated. Thanks! > > This needs to go below the '---'. > > So you've had feedback but no positive reviews resulting in a *-by? Correct, I've received feedback, but no positive reviews. > > Have you Cc:ed all of your reviewers in this version? > > > Signed-off-by: Joseph Strauss <jstrauss@mailbox.org> > > --- > > Changes in v2: > > - Replaced instances of the constant 3 with NUM_LEDS, where applicable > > - Fixed formatting errors > > - Replaced loop inside of blinkm_set_mc_brightness() with equivalent > > statements > > - Changed id of multicolor class from 4 to 3 > > - Replaced call to devm_kmalloc_array() with devm_kcalloc() > > Changes in v3: > > - Add CONFIG_LEDS_BLINKM_MULTICOLOR to check whether to use multicolor > > funcitonality > > - Extend well-known-leds.txt to include standard names for RGB and indicator > > LEDS > > - Change name of Blinkm sysfs class according to well-known-leds.txt > > - Simplify struct blinkm_led and struct blinkm_data > > - Remove magic numbers > > - Fix formatting errors > > - Remove unrelated changes > > Changes in v4: > > - Fix indentation > > - Add default case to switch statement > > Changes in v5: > > - Fix warnings related to snprintf on s390 architecture, reported by > > 0-DAY CI Kernel Test Service > > > > Documentation/leds/leds-blinkm.rst | 27 ++- > > Documentation/leds/well-known-leds.txt | 8 + > > drivers/leds/Kconfig | 8 + > > drivers/leds/leds-blinkm.c | 217 +++++++++++++++++-------- > > 4 files changed, 185 insertions(+), 75 deletions(-) > > Looking pretty good. Just a couple of minor nits. > > > diff --git a/Documentation/leds/leds-blinkm.rst b/Documentation/leds/leds-blinkm.rst > > index c74b5bc877b1..16883c2a9a99 100644 > > --- a/Documentation/leds/leds-blinkm.rst > > +++ b/Documentation/leds/leds-blinkm.rst > > @@ -13,9 +13,27 @@ The device accepts RGB and HSB color values through separate commands. > > Also you can store blinking sequences as "scripts" in > > the controller and run them. Also fading is an option. > > > > -The interface this driver provides is 2-fold: > > +The interface this driver provides is 3-fold: > > > > -a) LED class interface for use with triggers > > +a) LED multicolor class interface for use with triggers > > +####################################################### > > + > > +The registration follows the scheme:: > > + > > + blinkm-<i2c-bus-nr>-<i2c-device-nr>:rgb:indicator > > + > > + $ ls -h /sys/class/leds/blinkm-1-9:rgb:indicator > > The `-h` is superfluous, no? The current leds-blinkm.rst documentation file uses `ls -h` in several other places, so I continued the pattern. But it does seem superfluous. Would it be better to remove the `-h` only from the line I added, or the other instances as well? > > > + brightness device max_brightness multi_index multi_intensity power subsystem trigger uevent > > + > > +The order in which to write the intensity values can be found in multi_index. > > +Exactly three values between 0 and 255 must be written to multi_intensity to change the color:: > > :: ? I used :: here because I thought a shell command should be shown as code in the documentation. > > > + > > + $ echo 255 100 50 > multi_intensity > > + > > +The overall brightness of the color that you choose can also be changed by > > +writing a value between 0 and 255 to the brightness file. > > So what's the difference between brightness and intensity? Documentation/leds/leds-class-multicolor.rst describes the multi_intensity file as controlling the hue of the LED, whereas the brightness file controls the lightness. I could change my documentation to say "The overall lightness of the color that you choose..." to clarify that. > > > +b) LED class interface for use with triggers > > ############################################ > > > > The registration follows the scheme:: > > @@ -50,7 +68,7 @@ E.g.:: > > $ > > > > > > -b) Sysfs group to control rgb, fade, hsb, scripts ... > > +c) Sysfs group to control rgb, fade, hsb, scripts ... > > ##################################################### > > > > This extended interface is available as folder blinkm > > @@ -79,6 +97,7 @@ E.g.:: > > > > > > > > -as of 6/2012 > > +as of 01/2024 > > > > dl9pf <at> gmx <dot> de > > +jstrauss <at> mailbox <dot> org > > diff --git a/Documentation/leds/well-known-leds.txt b/Documentation/leds/well-known-leds.txt > > index 2160382c86be..2ac4eaed1454 100644 > > --- a/Documentation/leds/well-known-leds.txt > > +++ b/Documentation/leds/well-known-leds.txt > > @@ -70,3 +70,11 @@ Good: "platform:*:charging" (allwinner sun50i) > > * Screen > > > > Good: ":backlight" (Motorola Droid 4) > > + > > +* Indicators > > + > > +Good: ":indicator" (Blinkm) > > + > > +* RGB > > + > > +Good: ":rgb" (Blinkm) > > diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig > > index 499d0f215a8b..f38d786f9a89 100644 > > --- a/drivers/leds/Kconfig > > +++ b/drivers/leds/Kconfig > > @@ -736,6 +736,14 @@ config LEDS_BLINKM > > This option enables support for the BlinkM RGB LED connected > > through I2C. Say Y to enable support for the BlinkM LED. > > > > +config LEDS_BLINKM_MULTICOLOR > > + bool "Enable multicolor support for BlinkM I2C RGB LED" > > + depends on LEDS_BLINKM > > + depends on LEDS_CLASS_MULTICOLOR > > + help > > + This option enables multicolor sysfs class support for BlinkM LED and > > + disables the older, separated sysfs interface > > + > > config LEDS_POWERNV > > tristate "LED support for PowerNV Platform" > > depends on LEDS_CLASS > > diff --git a/drivers/leds/leds-blinkm.c b/drivers/leds/leds-blinkm.c > > index e19cc8a7b7ca..d6ce2cccb816 100644 > > --- a/drivers/leds/leds-blinkm.c > > +++ b/drivers/leds/leds-blinkm.c > > @@ -2,6 +2,7 @@ > > /* > > * leds-blinkm.c > > * (c) Jan-Simon Möller (dl9pf@gmx.de) > > + * (c) Joseph Strauss (jstrauss@mailbox.org) > > */ > > > > #include <linux/module.h> > > @@ -15,6 +16,10 @@ > > #include <linux/pm_runtime.h> > > #include <linux/leds.h> > > #include <linux/delay.h> > > +#include <linux/led-class-multicolor.h> > > +#include <linux/kconfig.h> > > + > > +#define NUM_LEDS 3 > > > > /* Addresses to scan - BlinkM is on 0x09 by default*/ > > static const unsigned short normal_i2c[] = { 0x09, I2C_CLIENT_END }; > > @@ -22,19 +27,24 @@ static const unsigned short normal_i2c[] = { 0x09, I2C_CLIENT_END }; > > static int blinkm_transfer_hw(struct i2c_client *client, int cmd); > > static int blinkm_test_run(struct i2c_client *client); > > > > +/* Contains data structures for both the color-seperated sysfs classes, and the new multicolor class */ > > struct blinkm_led { > > struct i2c_client *i2c_client; > > + /* used when multicolor support is disabled */ > > There's a syntax for either/or in structs. Isn't it 'union'? > > Can/does that apply here? Yes, I think that would be applicable. led_cdev and mcled_cdev are used in two seperate cases. > > > struct led_classdev led_cdev; > > + struct led_classdev_mc mcled_cdev; > > int id; > > }; > > > > -#define cdev_to_blmled(c) container_of(c, struct blinkm_led, led_cdev) > > +#define led_cdev_to_blmled(c) container_of(c, struct blinkm_led, led_cdev) > > +#define mcled_cdev_to_led(c) container_of(c, struct blinkm_led, mcled_cdev) > > > > struct blinkm_data { > > struct i2c_client *i2c_client; > > struct mutex update_lock; > > /* used for led class interface */ > > - struct blinkm_led blinkm_leds[3]; > > + struct blinkm_led blinkm_leds[NUM_LEDS]; > > + > > /* used for "blinkm" sysfs interface */ > > u8 red; /* color red */ > > u8 green; /* color green */ > > @@ -419,11 +429,29 @@ static int blinkm_transfer_hw(struct i2c_client *client, int cmd) > > return 0; > > } > > > > +static int blinkm_set_mc_brightness(struct led_classdev *led_cdev, > > + enum led_brightness value) > > +{ > > + struct led_classdev_mc *mcled_cdev = lcdev_to_mccdev(led_cdev); > > + struct blinkm_led *led = mcled_cdev_to_led(mcled_cdev); > > + struct blinkm_data *data = i2c_get_clientdata(led->i2c_client); > > + > > + led_mc_calc_color_components(mcled_cdev, value); > > + > > + data->next_red = (u8) mcled_cdev->subled_info[RED].brightness; > > + data->next_green = (u8) mcled_cdev->subled_info[GREEN].brightness; > > + data->next_blue = (u8) mcled_cdev->subled_info[BLUE].brightness; > > + > > + blinkm_transfer_hw(led->i2c_client, BLM_GO_RGB); > > + > > + return 0; > > +} > > + > > static int blinkm_led_common_set(struct led_classdev *led_cdev, > > enum led_brightness value, int color) > > { > > /* led_brightness is 0, 127 or 255 - we just use it here as-is */ > > - struct blinkm_led *led = cdev_to_blmled(led_cdev); > > + struct blinkm_led *led = led_cdev_to_blmled(led_cdev); > > struct blinkm_data *data = i2c_get_clientdata(led->i2c_client); > > > > switch (color) { > > @@ -569,7 +597,11 @@ static int blinkm_probe(struct i2c_client *client, > > const struct i2c_device_id *id) > > { > > struct blinkm_data *data; > > - struct blinkm_led *led[3]; > > + /* For multicolor support */ > > + struct blinkm_led *mc_led; > > + struct mc_subled *mc_led_info; > > + /* 3 seperate classes for red, green, and blue respectively */ > > + struct blinkm_led *leds[NUM_LEDS]; > > int err, i; > > char blinkm_led_name[28]; > > > > @@ -580,6 +612,12 @@ static int blinkm_probe(struct i2c_client *client, > > goto exit; > > } > > > > + mc_led_info = devm_kcalloc(&client->dev, NUM_LEDS, sizeof(*mc_led_info), > > + GFP_KERNEL); > > + if (!mc_led_info) { > > + err = -ENOMEM; > > + goto exit; > > + } > > data->i2c_addr = 0x08; > > /* i2c addr - use fake addr of 0x08 initially (real is 0x09) */ > > data->fw_ver = 0xfe; > > @@ -598,81 +636,118 @@ static int blinkm_probe(struct i2c_client *client, > > goto exit; > > } > > > > - for (i = 0; i < 3; i++) { > > - /* RED = 0, GREEN = 1, BLUE = 2 */ > > - led[i] = &data->blinkm_leds[i]; > > - led[i]->i2c_client = client; > > - led[i]->id = i; > > - led[i]->led_cdev.max_brightness = 255; > > - led[i]->led_cdev.flags = LED_CORE_SUSPENDRESUME; > > - switch (i) { > > - case RED: > > - snprintf(blinkm_led_name, sizeof(blinkm_led_name), > > - "blinkm-%d-%d-red", > > - client->adapter->nr, > > - client->addr); > > - led[i]->led_cdev.name = blinkm_led_name; > > - led[i]->led_cdev.brightness_set_blocking = > > - blinkm_led_red_set; > > - err = led_classdev_register(&client->dev, > > - &led[i]->led_cdev); > > - if (err < 0) { > > - dev_err(&client->dev, > > - "couldn't register LED %s\n", > > - led[i]->led_cdev.name); > > - goto failred; > > - } > > - break; > > - case GREEN: > > - snprintf(blinkm_led_name, sizeof(blinkm_led_name), > > - "blinkm-%d-%d-green", > > - client->adapter->nr, > > - client->addr); > > - led[i]->led_cdev.name = blinkm_led_name; > > - led[i]->led_cdev.brightness_set_blocking = > > - blinkm_led_green_set; > > - err = led_classdev_register(&client->dev, > > - &led[i]->led_cdev); > > - if (err < 0) { > > - dev_err(&client->dev, > > - "couldn't register LED %s\n", > > - led[i]->led_cdev.name); > > - goto failgreen; > > - } > > - break; > > - case BLUE: > > - snprintf(blinkm_led_name, sizeof(blinkm_led_name), > > - "blinkm-%d-%d-blue", > > - client->adapter->nr, > > - client->addr); > > - led[i]->led_cdev.name = blinkm_led_name; > > - led[i]->led_cdev.brightness_set_blocking = > > - blinkm_led_blue_set; > > - err = led_classdev_register(&client->dev, > > - &led[i]->led_cdev); > > - if (err < 0) { > > - dev_err(&client->dev, > > - "couldn't register LED %s\n", > > - led[i]->led_cdev.name); > > - goto failblue; > > - } > > - break; > > - } /* end switch */ > > - } /* end for */ > > - > > - /* Initialize the blinkm */ > > + if (!IS_ENABLED(CONFIG_LEDS_BLINKM_MULTICOLOR)) { > > + /* Register red, green, and blue sysfs classes */ > > + for (i = 0; i < NUM_LEDS; i++) { > > *Even* more indentation! > > Probably worth moving the whole section into a function at this point. Yes, I think you are right. Thank you for the review. > > > + /* RED = 0, GREEN = 1, BLUE = 2 */ > > + leds[i] = &data->blinkm_leds[i]; > > + leds[i]->i2c_client = client; > > + leds[i]->id = i; > > + leds[i]->led_cdev.max_brightness = 255; > > + leds[i]->led_cdev.flags = LED_CORE_SUSPENDRESUME; > > + switch (i) { > > + case RED: > > + scnprintf(blinkm_led_name, sizeof(blinkm_led_name), > > + "blinkm-%d-%d-red", > > + client->adapter->nr, > > + client->addr); > > + leds[i]->led_cdev.name = blinkm_led_name; > > + leds[i]->led_cdev.brightness_set_blocking = > > + blinkm_led_red_set; > > + err = led_classdev_register(&client->dev, > > + &leds[i]->led_cdev); > > + if (err < 0) { > > + dev_err(&client->dev, > > + "couldn't register LED %s\n", > > + leds[i]->led_cdev.name); > > + goto failred; > > + } > > + break; > > + case GREEN: > > + scnprintf(blinkm_led_name, sizeof(blinkm_led_name), > > + "blinkm-%d-%d-green", > > + client->adapter->nr, > > + client->addr); > > + leds[i]->led_cdev.name = blinkm_led_name; > > + leds[i]->led_cdev.brightness_set_blocking = > > + blinkm_led_green_set; > > + err = led_classdev_register(&client->dev, > > + &leds[i]->led_cdev); > > + if (err < 0) { > > + dev_err(&client->dev, > > + "couldn't register LED %s\n", > > + leds[i]->led_cdev.name); > > + goto failgreen; > > + } > > + break; > > + case BLUE: > > + scnprintf(blinkm_led_name, sizeof(blinkm_led_name), > > + "blinkm-%d-%d-blue", > > + client->adapter->nr, > > + client->addr); > > + leds[i]->led_cdev.name = blinkm_led_name; > > + leds[i]->led_cdev.brightness_set_blocking = > > + blinkm_led_blue_set; > > + err = led_classdev_register(&client->dev, > > + &leds[i]->led_cdev); > > + if (err < 0) { > > + dev_err(&client->dev, > > + "couldn't register LED %s\n", > > + leds[i]->led_cdev.name); > > + goto failblue; > > + } > > + break; > > + default: > > + break; > > + } /* end switch */ > > + } /* end for */ > > + } else { > > + /* Register multicolor sysfs class */ > > + /* The first element of leds is used for multicolor facilities */ > > + mc_led = &data->blinkm_leds[RED]; > > + mc_led->i2c_client = client; > > + > > + mc_led_info[RED].color_index = LED_COLOR_ID_RED; > > + mc_led_info[GREEN].color_index = LED_COLOR_ID_GREEN; > > + mc_led_info[BLUE].color_index = LED_COLOR_ID_BLUE; > > + > > + mc_led->mcled_cdev.subled_info = mc_led_info; > > + mc_led->mcled_cdev.num_colors = NUM_LEDS; > > + mc_led->mcled_cdev.led_cdev.brightness = 255; > > + mc_led->mcled_cdev.led_cdev.max_brightness = 255; > > + mc_led->mcled_cdev.led_cdev.flags = LED_CORE_SUSPENDRESUME; > > + > > + scnprintf(blinkm_led_name, sizeof(blinkm_led_name), > > + "blinkm-%d-%d:rgb:indicator", > > + client->adapter->nr, > > + client->addr); > > + mc_led->mcled_cdev.led_cdev.name = blinkm_led_name; > > + mc_led->mcled_cdev.led_cdev.brightness_set_blocking = blinkm_set_mc_brightness; > > + > > + err = led_classdev_multicolor_register(&client->dev, &mc_led->mcled_cdev); > > + if (err < 0) { > > + dev_err(&client->dev, "couldn't register LED %s\n", > > + mc_led->led_cdev.name); > > + goto failmulti; > > + } > > + } > > + > > blinkm_init_hw(client); > > > > return 0; > > > > +failmulti: > > + led_classdev_unregister(&leds[BLUE]->led_cdev); > > + > > failblue: > > - led_classdev_unregister(&led[GREEN]->led_cdev); > > + led_classdev_unregister(&leds[GREEN]->led_cdev); > > > > failgreen: > > - led_classdev_unregister(&led[RED]->led_cdev); > > + led_classdev_unregister(&leds[RED]->led_cdev); > > > > failred: > > sysfs_remove_group(&client->dev.kobj, &blinkm_group); > > + > > exit: > > return err; > > } > > @@ -684,7 +759,7 @@ static void blinkm_remove(struct i2c_client *client) > > int i; > > > > /* make sure no workqueue entries are pending */ > > - for (i = 0; i < 3; i++) > > + for (i = 0; i < NUM_LEDS; i++) > > led_classdev_unregister(&data->blinkm_leds[i].led_cdev); > > > > /* reset rgb */ > > @@ -741,6 +816,6 @@ static struct i2c_driver blinkm_driver = { > > module_i2c_driver(blinkm_driver); > > > > MODULE_AUTHOR("Jan-Simon Moeller <dl9pf@gmx.de>"); > > +MODULE_AUTHOR("Joseph Strauss <jstrauss@mailbox.org>"); > > MODULE_DESCRIPTION("BlinkM RGB LED driver"); > > MODULE_LICENSE("GPL"); > > - > > -- > > 2.42.0 > > > > -- > 1) > Lee Jones [李琼斯] Joseph Strauss
diff --git a/Documentation/leds/leds-blinkm.rst b/Documentation/leds/leds-blinkm.rst index c74b5bc877b1..16883c2a9a99 100644 --- a/Documentation/leds/leds-blinkm.rst +++ b/Documentation/leds/leds-blinkm.rst @@ -13,9 +13,27 @@ The device accepts RGB and HSB color values through separate commands. Also you can store blinking sequences as "scripts" in the controller and run them. Also fading is an option. -The interface this driver provides is 2-fold: +The interface this driver provides is 3-fold: -a) LED class interface for use with triggers +a) LED multicolor class interface for use with triggers +####################################################### + +The registration follows the scheme:: + + blinkm-<i2c-bus-nr>-<i2c-device-nr>:rgb:indicator + + $ ls -h /sys/class/leds/blinkm-1-9:rgb:indicator + brightness device max_brightness multi_index multi_intensity power subsystem trigger uevent + +The order in which to write the intensity values can be found in multi_index. +Exactly three values between 0 and 255 must be written to multi_intensity to change the color:: + + $ echo 255 100 50 > multi_intensity + +The overall brightness of the color that you choose can also be changed by +writing a value between 0 and 255 to the brightness file. + +b) LED class interface for use with triggers ############################################ The registration follows the scheme:: @@ -50,7 +68,7 @@ E.g.:: $ -b) Sysfs group to control rgb, fade, hsb, scripts ... +c) Sysfs group to control rgb, fade, hsb, scripts ... ##################################################### This extended interface is available as folder blinkm @@ -79,6 +97,7 @@ E.g.:: -as of 6/2012 +as of 01/2024 dl9pf <at> gmx <dot> de +jstrauss <at> mailbox <dot> org diff --git a/Documentation/leds/well-known-leds.txt b/Documentation/leds/well-known-leds.txt index 2160382c86be..2ac4eaed1454 100644 --- a/Documentation/leds/well-known-leds.txt +++ b/Documentation/leds/well-known-leds.txt @@ -70,3 +70,11 @@ Good: "platform:*:charging" (allwinner sun50i) * Screen Good: ":backlight" (Motorola Droid 4) + +* Indicators + +Good: ":indicator" (Blinkm) + +* RGB + +Good: ":rgb" (Blinkm) diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig index 499d0f215a8b..f38d786f9a89 100644 --- a/drivers/leds/Kconfig +++ b/drivers/leds/Kconfig @@ -736,6 +736,14 @@ config LEDS_BLINKM This option enables support for the BlinkM RGB LED connected through I2C. Say Y to enable support for the BlinkM LED. +config LEDS_BLINKM_MULTICOLOR + bool "Enable multicolor support for BlinkM I2C RGB LED" + depends on LEDS_BLINKM + depends on LEDS_CLASS_MULTICOLOR + help + This option enables multicolor sysfs class support for BlinkM LED and + disables the older, separated sysfs interface + config LEDS_POWERNV tristate "LED support for PowerNV Platform" depends on LEDS_CLASS diff --git a/drivers/leds/leds-blinkm.c b/drivers/leds/leds-blinkm.c index e19cc8a7b7ca..d6ce2cccb816 100644 --- a/drivers/leds/leds-blinkm.c +++ b/drivers/leds/leds-blinkm.c @@ -2,6 +2,7 @@ /* * leds-blinkm.c * (c) Jan-Simon Möller (dl9pf@gmx.de) + * (c) Joseph Strauss (jstrauss@mailbox.org) */ #include <linux/module.h> @@ -15,6 +16,10 @@ #include <linux/pm_runtime.h> #include <linux/leds.h> #include <linux/delay.h> +#include <linux/led-class-multicolor.h> +#include <linux/kconfig.h> + +#define NUM_LEDS 3 /* Addresses to scan - BlinkM is on 0x09 by default*/ static const unsigned short normal_i2c[] = { 0x09, I2C_CLIENT_END }; @@ -22,19 +27,24 @@ static const unsigned short normal_i2c[] = { 0x09, I2C_CLIENT_END }; static int blinkm_transfer_hw(struct i2c_client *client, int cmd); static int blinkm_test_run(struct i2c_client *client); +/* Contains data structures for both the color-seperated sysfs classes, and the new multicolor class */ struct blinkm_led { struct i2c_client *i2c_client; + /* used when multicolor support is disabled */ struct led_classdev led_cdev; + struct led_classdev_mc mcled_cdev; int id; }; -#define cdev_to_blmled(c) container_of(c, struct blinkm_led, led_cdev) +#define led_cdev_to_blmled(c) container_of(c, struct blinkm_led, led_cdev) +#define mcled_cdev_to_led(c) container_of(c, struct blinkm_led, mcled_cdev) struct blinkm_data { struct i2c_client *i2c_client; struct mutex update_lock; /* used for led class interface */ - struct blinkm_led blinkm_leds[3]; + struct blinkm_led blinkm_leds[NUM_LEDS]; + /* used for "blinkm" sysfs interface */ u8 red; /* color red */ u8 green; /* color green */ @@ -419,11 +429,29 @@ static int blinkm_transfer_hw(struct i2c_client *client, int cmd) return 0; } +static int blinkm_set_mc_brightness(struct led_classdev *led_cdev, + enum led_brightness value) +{ + struct led_classdev_mc *mcled_cdev = lcdev_to_mccdev(led_cdev); + struct blinkm_led *led = mcled_cdev_to_led(mcled_cdev); + struct blinkm_data *data = i2c_get_clientdata(led->i2c_client); + + led_mc_calc_color_components(mcled_cdev, value); + + data->next_red = (u8) mcled_cdev->subled_info[RED].brightness; + data->next_green = (u8) mcled_cdev->subled_info[GREEN].brightness; + data->next_blue = (u8) mcled_cdev->subled_info[BLUE].brightness; + + blinkm_transfer_hw(led->i2c_client, BLM_GO_RGB); + + return 0; +} + static int blinkm_led_common_set(struct led_classdev *led_cdev, enum led_brightness value, int color) { /* led_brightness is 0, 127 or 255 - we just use it here as-is */ - struct blinkm_led *led = cdev_to_blmled(led_cdev); + struct blinkm_led *led = led_cdev_to_blmled(led_cdev); struct blinkm_data *data = i2c_get_clientdata(led->i2c_client); switch (color) { @@ -569,7 +597,11 @@ static int blinkm_probe(struct i2c_client *client, const struct i2c_device_id *id) { struct blinkm_data *data; - struct blinkm_led *led[3]; + /* For multicolor support */ + struct blinkm_led *mc_led; + struct mc_subled *mc_led_info; + /* 3 seperate classes for red, green, and blue respectively */ + struct blinkm_led *leds[NUM_LEDS]; int err, i; char blinkm_led_name[28]; @@ -580,6 +612,12 @@ static int blinkm_probe(struct i2c_client *client, goto exit; } + mc_led_info = devm_kcalloc(&client->dev, NUM_LEDS, sizeof(*mc_led_info), + GFP_KERNEL); + if (!mc_led_info) { + err = -ENOMEM; + goto exit; + } data->i2c_addr = 0x08; /* i2c addr - use fake addr of 0x08 initially (real is 0x09) */ data->fw_ver = 0xfe; @@ -598,81 +636,118 @@ static int blinkm_probe(struct i2c_client *client, goto exit; } - for (i = 0; i < 3; i++) { - /* RED = 0, GREEN = 1, BLUE = 2 */ - led[i] = &data->blinkm_leds[i]; - led[i]->i2c_client = client; - led[i]->id = i; - led[i]->led_cdev.max_brightness = 255; - led[i]->led_cdev.flags = LED_CORE_SUSPENDRESUME; - switch (i) { - case RED: - snprintf(blinkm_led_name, sizeof(blinkm_led_name), - "blinkm-%d-%d-red", - client->adapter->nr, - client->addr); - led[i]->led_cdev.name = blinkm_led_name; - led[i]->led_cdev.brightness_set_blocking = - blinkm_led_red_set; - err = led_classdev_register(&client->dev, - &led[i]->led_cdev); - if (err < 0) { - dev_err(&client->dev, - "couldn't register LED %s\n", - led[i]->led_cdev.name); - goto failred; - } - break; - case GREEN: - snprintf(blinkm_led_name, sizeof(blinkm_led_name), - "blinkm-%d-%d-green", - client->adapter->nr, - client->addr); - led[i]->led_cdev.name = blinkm_led_name; - led[i]->led_cdev.brightness_set_blocking = - blinkm_led_green_set; - err = led_classdev_register(&client->dev, - &led[i]->led_cdev); - if (err < 0) { - dev_err(&client->dev, - "couldn't register LED %s\n", - led[i]->led_cdev.name); - goto failgreen; - } - break; - case BLUE: - snprintf(blinkm_led_name, sizeof(blinkm_led_name), - "blinkm-%d-%d-blue", - client->adapter->nr, - client->addr); - led[i]->led_cdev.name = blinkm_led_name; - led[i]->led_cdev.brightness_set_blocking = - blinkm_led_blue_set; - err = led_classdev_register(&client->dev, - &led[i]->led_cdev); - if (err < 0) { - dev_err(&client->dev, - "couldn't register LED %s\n", - led[i]->led_cdev.name); - goto failblue; - } - break; - } /* end switch */ - } /* end for */ - - /* Initialize the blinkm */ + if (!IS_ENABLED(CONFIG_LEDS_BLINKM_MULTICOLOR)) { + /* Register red, green, and blue sysfs classes */ + for (i = 0; i < NUM_LEDS; i++) { + /* RED = 0, GREEN = 1, BLUE = 2 */ + leds[i] = &data->blinkm_leds[i]; + leds[i]->i2c_client = client; + leds[i]->id = i; + leds[i]->led_cdev.max_brightness = 255; + leds[i]->led_cdev.flags = LED_CORE_SUSPENDRESUME; + switch (i) { + case RED: + scnprintf(blinkm_led_name, sizeof(blinkm_led_name), + "blinkm-%d-%d-red", + client->adapter->nr, + client->addr); + leds[i]->led_cdev.name = blinkm_led_name; + leds[i]->led_cdev.brightness_set_blocking = + blinkm_led_red_set; + err = led_classdev_register(&client->dev, + &leds[i]->led_cdev); + if (err < 0) { + dev_err(&client->dev, + "couldn't register LED %s\n", + leds[i]->led_cdev.name); + goto failred; + } + break; + case GREEN: + scnprintf(blinkm_led_name, sizeof(blinkm_led_name), + "blinkm-%d-%d-green", + client->adapter->nr, + client->addr); + leds[i]->led_cdev.name = blinkm_led_name; + leds[i]->led_cdev.brightness_set_blocking = + blinkm_led_green_set; + err = led_classdev_register(&client->dev, + &leds[i]->led_cdev); + if (err < 0) { + dev_err(&client->dev, + "couldn't register LED %s\n", + leds[i]->led_cdev.name); + goto failgreen; + } + break; + case BLUE: + scnprintf(blinkm_led_name, sizeof(blinkm_led_name), + "blinkm-%d-%d-blue", + client->adapter->nr, + client->addr); + leds[i]->led_cdev.name = blinkm_led_name; + leds[i]->led_cdev.brightness_set_blocking = + blinkm_led_blue_set; + err = led_classdev_register(&client->dev, + &leds[i]->led_cdev); + if (err < 0) { + dev_err(&client->dev, + "couldn't register LED %s\n", + leds[i]->led_cdev.name); + goto failblue; + } + break; + default: + break; + } /* end switch */ + } /* end for */ + } else { + /* Register multicolor sysfs class */ + /* The first element of leds is used for multicolor facilities */ + mc_led = &data->blinkm_leds[RED]; + mc_led->i2c_client = client; + + mc_led_info[RED].color_index = LED_COLOR_ID_RED; + mc_led_info[GREEN].color_index = LED_COLOR_ID_GREEN; + mc_led_info[BLUE].color_index = LED_COLOR_ID_BLUE; + + mc_led->mcled_cdev.subled_info = mc_led_info; + mc_led->mcled_cdev.num_colors = NUM_LEDS; + mc_led->mcled_cdev.led_cdev.brightness = 255; + mc_led->mcled_cdev.led_cdev.max_brightness = 255; + mc_led->mcled_cdev.led_cdev.flags = LED_CORE_SUSPENDRESUME; + + scnprintf(blinkm_led_name, sizeof(blinkm_led_name), + "blinkm-%d-%d:rgb:indicator", + client->adapter->nr, + client->addr); + mc_led->mcled_cdev.led_cdev.name = blinkm_led_name; + mc_led->mcled_cdev.led_cdev.brightness_set_blocking = blinkm_set_mc_brightness; + + err = led_classdev_multicolor_register(&client->dev, &mc_led->mcled_cdev); + if (err < 0) { + dev_err(&client->dev, "couldn't register LED %s\n", + mc_led->led_cdev.name); + goto failmulti; + } + } + blinkm_init_hw(client); return 0; +failmulti: + led_classdev_unregister(&leds[BLUE]->led_cdev); + failblue: - led_classdev_unregister(&led[GREEN]->led_cdev); + led_classdev_unregister(&leds[GREEN]->led_cdev); failgreen: - led_classdev_unregister(&led[RED]->led_cdev); + led_classdev_unregister(&leds[RED]->led_cdev); failred: sysfs_remove_group(&client->dev.kobj, &blinkm_group); + exit: return err; } @@ -684,7 +759,7 @@ static void blinkm_remove(struct i2c_client *client) int i; /* make sure no workqueue entries are pending */ - for (i = 0; i < 3; i++) + for (i = 0; i < NUM_LEDS; i++) led_classdev_unregister(&data->blinkm_leds[i].led_cdev); /* reset rgb */ @@ -741,6 +816,6 @@ static struct i2c_driver blinkm_driver = { module_i2c_driver(blinkm_driver); MODULE_AUTHOR("Jan-Simon Moeller <dl9pf@gmx.de>"); +MODULE_AUTHOR("Joseph Strauss <jstrauss@mailbox.org>"); MODULE_DESCRIPTION("BlinkM RGB LED driver"); MODULE_LICENSE("GPL"); -
Add multicolor support to the BlinkM driver, making it easier to control from userspace. The BlinkM LED is a programmable RGB LED. The driver currently supports only the regular LED sysfs class, resulting in the creation of three distinct classes, one for red, green, and blue. The user then has to input three values into the three seperate brightness files within those classes. The multicolor LED framework makes the device easier to control with the multi_intensity file: the user can input three values at once to form a color, while still controlling the lightness with the brightness file. The main struct blinkm_led has changed slightly. The struct led_classdev for the regular sysfs classes remain. The blinkm_probe function checks CONFIG_LEDS_BLINKM_MULTICOLOR to decide whether to load the seperate sysfs classes or the single multicolor one, but never both. The blinkm_set_mc_brightness() function had to be added to calculate the three color components and then set the fields of the blinkm_data structure accordingly. All of the feedback has been much appreciated. Thanks! Signed-off-by: Joseph Strauss <jstrauss@mailbox.org> --- Changes in v2: - Replaced instances of the constant 3 with NUM_LEDS, where applicable - Fixed formatting errors - Replaced loop inside of blinkm_set_mc_brightness() with equivalent statements - Changed id of multicolor class from 4 to 3 - Replaced call to devm_kmalloc_array() with devm_kcalloc() Changes in v3: - Add CONFIG_LEDS_BLINKM_MULTICOLOR to check whether to use multicolor funcitonality - Extend well-known-leds.txt to include standard names for RGB and indicator LEDS - Change name of Blinkm sysfs class according to well-known-leds.txt - Simplify struct blinkm_led and struct blinkm_data - Remove magic numbers - Fix formatting errors - Remove unrelated changes Changes in v4: - Fix indentation - Add default case to switch statement Changes in v5: - Fix warnings related to snprintf on s390 architecture, reported by 0-DAY CI Kernel Test Service Documentation/leds/leds-blinkm.rst | 27 ++- Documentation/leds/well-known-leds.txt | 8 + drivers/leds/Kconfig | 8 + drivers/leds/leds-blinkm.c | 217 +++++++++++++++++-------- 4 files changed, 185 insertions(+), 75 deletions(-)