From e35ca991a777ef513040cbb36bc8245a031a2633 Mon Sep 17 00:00:00 2001 From: Sven Schwermer Date: Fri, 4 Apr 2025 20:40:36 +0200 Subject: [PATCH 01/16] leds: multicolor: Fix intensity setting while SW blinking When writing to the multi_intensity file, don't unconditionally call led_set_brightness. By only doing this if blinking is inactive we prevent blinking from stopping if the blinking is in its off phase while the file is written. Instead, if blinking is active, the changed intensity values are applied upon the next blink. This is consistent with changing the brightness on monochrome LEDs with active blinking. Suggested-by: Jacek Anaszewski Acked-by: Jacek Anaszewski Acked-by: Pavel Machek Reviewed-by: Tobias Deiminger Tested-by: Sven Schuchmann Signed-off-by: Sven Schwermer Link: https://lore.kernel.org/r/20250404184043.227116-1-sven@svenschwermer.de Signed-off-by: Lee Jones --- drivers/leds/led-class-multicolor.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/leds/led-class-multicolor.c b/drivers/leds/led-class-multicolor.c index b2a87c994816..fd66d2bdeace 100644 --- a/drivers/leds/led-class-multicolor.c +++ b/drivers/leds/led-class-multicolor.c @@ -59,7 +59,8 @@ static ssize_t multi_intensity_store(struct device *dev, for (i = 0; i < mcled_cdev->num_colors; i++) mcled_cdev->subled_info[i].intensity = intensity_value[i]; - led_set_brightness(led_cdev, led_cdev->brightness); + if (!test_bit(LED_BLINK_SW, &led_cdev->work_flags)) + led_set_brightness(led_cdev, led_cdev->brightness); ret = size; err_out: mutex_unlock(&led_cdev->led_access); -- 2.51.0 From bd3d14932923a717ebe475122ca7e17200f87a0c Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Mon, 7 Apr 2025 18:13:26 +0300 Subject: [PATCH 02/16] leds: pca955x: Avoid potential overflow when filling default_label MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit GCC compiler (Debian 14.2.0-17) is not happy about printing into a too short buffer (when build with `make W=1`): drivers/leds/leds-pca955x.c:554:33: note: ‘snprintf’ output between 2 and 12 bytes into a destination of size 8 Indeed, the buffer size is chosen based on some assumptions, while in general the assigned value might not fit (GCC can't prove it does). Fix this by changing the bits field in the struct pca955x_chipdef to u8, with a positive side effect of the better memory footprint, and convert loop iterator to be unsigned. With that done, update format specifiers accordingly. In one case join back string literal as it improves the grepping over the code based on the message and remove duplicating information (the driver name is printed as pert of the dev_*() output [1]) as we touch the same line anyway. Link: https://lore.kernel.org/r/4ac527f2-c59e-70a2-efd4-da52370ea557@dave.eu/ [1] Signed-off-by: Andy Shevchenko Link: https://lore.kernel.org/r/20250407151441.706378-1-andriy.shevchenko@linux.intel.com Signed-off-by: Lee Jones --- drivers/leds/leds-pca955x.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/drivers/leds/leds-pca955x.c b/drivers/leds/leds-pca955x.c index e9cfde9fe4b1..24a40a1cdb15 100644 --- a/drivers/leds/leds-pca955x.c +++ b/drivers/leds/leds-pca955x.c @@ -73,7 +73,7 @@ enum pca955x_type { }; struct pca955x_chipdef { - int bits; + u8 bits; u8 slv_addr; /* 7-bit slave address mask */ int slv_addr_shift; /* Number of bits to ignore */ int blink_div; /* PSC divider */ @@ -142,13 +142,13 @@ struct pca955x_platform_data { }; /* 8 bits per input register */ -static inline int pca955x_num_input_regs(int bits) +static inline u8 pca955x_num_input_regs(u8 bits) { return (bits + 7) / 8; } /* 4 bits per LED selector register */ -static inline int pca955x_num_led_regs(int bits) +static inline u8 pca955x_num_led_regs(u8 bits) { return (bits + 3) / 4; } @@ -581,14 +581,14 @@ static int pca955x_probe(struct i2c_client *client) struct led_classdev *led; struct led_init_data init_data; struct i2c_adapter *adapter; - int i, bit, err, nls, reg; + u8 i, nls, psc0; u8 ls1[4]; u8 ls2[4]; struct pca955x_platform_data *pdata; - u8 psc0; bool keep_psc0 = false; bool set_default_label = false; char default_label[8]; + int bit, err, reg; chip = i2c_get_match_data(client); if (!chip) @@ -610,16 +610,15 @@ static int pca955x_probe(struct i2c_client *client) return -ENODEV; } - dev_info(&client->dev, "leds-pca955x: Using %s %d-bit LED driver at " - "slave address 0x%02x\n", client->name, chip->bits, - client->addr); + dev_info(&client->dev, "Using %s %u-bit LED driver at slave address 0x%02x\n", + client->name, chip->bits, client->addr); if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA)) return -EIO; if (pdata->num_leds != chip->bits) { dev_err(&client->dev, - "board info claims %d LEDs on a %d-bit chip\n", + "board info claims %d LEDs on a %u-bit chip\n", pdata->num_leds, chip->bits); return -ENODEV; } @@ -694,8 +693,7 @@ static int pca955x_probe(struct i2c_client *client) } if (set_default_label) { - snprintf(default_label, sizeof(default_label), - "%d", i); + snprintf(default_label, sizeof(default_label), "%u", i); init_data.default_label = default_label; } else { init_data.default_label = NULL; -- 2.51.0 From 4bab18dcb452e00e28e181eb2d7a3a3a6a5d1413 Mon Sep 17 00:00:00 2001 From: Andrew Davis Date: Mon, 7 Apr 2025 13:35:50 -0500 Subject: [PATCH 03/16] leds: lp8860: Use regmap_multi_reg_write for EEPROM writes This helper does the same thing as manual looping, use it instead. Signed-off-by: Andrew Davis Link: https://lore.kernel.org/r/20250407183555.409687-1-afd@ti.com Signed-off-by: Lee Jones --- drivers/leds/leds-lp8860.c | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/drivers/leds/leds-lp8860.c b/drivers/leds/leds-lp8860.c index 995f2adf8569..2b1c68e60949 100644 --- a/drivers/leds/leds-lp8860.c +++ b/drivers/leds/leds-lp8860.c @@ -103,12 +103,7 @@ struct lp8860_led { struct regulator *regulator; }; -struct lp8860_eeprom_reg { - uint8_t reg; - uint8_t value; -}; - -static struct lp8860_eeprom_reg lp8860_eeprom_disp_regs[] = { +static const struct reg_sequence lp8860_eeprom_disp_regs[] = { { LP8860_EEPROM_REG_0, 0xed }, { LP8860_EEPROM_REG_1, 0xdf }, { LP8860_EEPROM_REG_2, 0xdc }, @@ -238,7 +233,7 @@ out: static int lp8860_init(struct lp8860_led *led) { unsigned int read_buf; - int ret, i, reg_count; + int ret, reg_count; if (led->regulator) { ret = regulator_enable(led->regulator); @@ -266,14 +261,10 @@ static int lp8860_init(struct lp8860_led *led) } reg_count = ARRAY_SIZE(lp8860_eeprom_disp_regs); - for (i = 0; i < reg_count; i++) { - ret = regmap_write(led->eeprom_regmap, - lp8860_eeprom_disp_regs[i].reg, - lp8860_eeprom_disp_regs[i].value); - if (ret) { - dev_err(&led->client->dev, "Failed writing EEPROM\n"); - goto out; - } + ret = regmap_multi_reg_write(led->eeprom_regmap, lp8860_eeprom_disp_regs, reg_count); + if (ret) { + dev_err(&led->client->dev, "Failed writing EEPROM\n"); + goto out; } ret = lp8860_unlock_eeprom(led, LP8860_LOCK_EEPROM); -- 2.51.0 From 87a59548af95db9b6b3a19fdd5b4e6c49bdbc285 Mon Sep 17 00:00:00 2001 From: Andrew Davis Date: Mon, 7 Apr 2025 13:35:51 -0500 Subject: [PATCH 04/16] leds: lp8860: Use new mutex guards to cleanup function exits Use scoped mutex guards to simplify return paths. While here use devm_mutex_init() to register the muxex so it also is cleaned up automatically. Signed-off-by: Andrew Davis Link: https://lore.kernel.org/r/20250407183555.409687-2-afd@ti.com Signed-off-by: Lee Jones --- drivers/leds/leds-lp8860.c | 27 +++++++++++---------------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/drivers/leds/leds-lp8860.c b/drivers/leds/leds-lp8860.c index 2b1c68e60949..2d91f476f0b7 100644 --- a/drivers/leds/leds-lp8860.c +++ b/drivers/leds/leds-lp8860.c @@ -135,7 +135,7 @@ static int lp8860_unlock_eeprom(struct lp8860_led *led, int lock) { int ret; - mutex_lock(&led->lock); + guard(mutex)(&led->lock); if (lock == LP8860_UNLOCK_EEPROM) { ret = regmap_write(led->regmap, @@ -143,7 +143,7 @@ static int lp8860_unlock_eeprom(struct lp8860_led *led, int lock) LP8860_EEPROM_CODE_1); if (ret) { dev_err(&led->client->dev, "EEPROM Unlock failed\n"); - goto out; + return ret; } ret = regmap_write(led->regmap, @@ -151,14 +151,14 @@ static int lp8860_unlock_eeprom(struct lp8860_led *led, int lock) LP8860_EEPROM_CODE_2); if (ret) { dev_err(&led->client->dev, "EEPROM Unlock failed\n"); - goto out; + return ret; } ret = regmap_write(led->regmap, LP8860_EEPROM_UNLOCK, LP8860_EEPROM_CODE_3); if (ret) { dev_err(&led->client->dev, "EEPROM Unlock failed\n"); - goto out; + return ret; } } else { ret = regmap_write(led->regmap, @@ -166,8 +166,6 @@ static int lp8860_unlock_eeprom(struct lp8860_led *led, int lock) LP8860_LOCK_EEPROM); } -out: - mutex_unlock(&led->lock); return ret; } @@ -204,30 +202,29 @@ static int lp8860_brightness_set(struct led_classdev *led_cdev, int disp_brightness = brt_val * 255; int ret; - mutex_lock(&led->lock); + guard(mutex)(&led->lock); ret = lp8860_fault_check(led); if (ret) { dev_err(&led->client->dev, "Cannot read/clear faults\n"); - goto out; + return ret; } ret = regmap_write(led->regmap, LP8860_DISP_CL1_BRT_MSB, (disp_brightness & 0xff00) >> 8); if (ret) { dev_err(&led->client->dev, "Cannot write CL1 MSB\n"); - goto out; + return ret; } ret = regmap_write(led->regmap, LP8860_DISP_CL1_BRT_LSB, disp_brightness & 0xff); if (ret) { dev_err(&led->client->dev, "Cannot write CL1 LSB\n"); - goto out; + return ret; } -out: - mutex_unlock(&led->lock); - return ret; + + return 0; } static int lp8860_init(struct lp8860_led *led) @@ -392,7 +389,7 @@ static int lp8860_probe(struct i2c_client *client) led->client = client; led->led_dev.brightness_set_blocking = lp8860_brightness_set; - mutex_init(&led->lock); + devm_mutex_init(&client->dev, &led->lock); i2c_set_clientdata(client, led); @@ -443,8 +440,6 @@ static void lp8860_remove(struct i2c_client *client) dev_err(&led->client->dev, "Failed to disable regulator\n"); } - - mutex_destroy(&led->lock); } static const struct i2c_device_id lp8860_id[] = { -- 2.51.0 From 0cb55e16bd842364340d24e2e368b9d6c5800423 Mon Sep 17 00:00:00 2001 From: Andrew Davis Date: Mon, 7 Apr 2025 13:35:52 -0500 Subject: [PATCH 05/16] leds: lp8860: Remove default regs when not caching If we are not using regmap caches, then the value will be read in every time, having a default value does not change anything in that case. Remove the unused defaults. Signed-off-by: Andrew Davis Link: https://lore.kernel.org/r/20250407183555.409687-3-afd@ti.com Signed-off-by: Lee Jones --- drivers/leds/leds-lp8860.c | 52 -------------------------------------- 1 file changed, 52 deletions(-) diff --git a/drivers/leds/leds-lp8860.c b/drivers/leds/leds-lp8860.c index 2d91f476f0b7..4cd1b960d504 100644 --- a/drivers/leds/leds-lp8860.c +++ b/drivers/leds/leds-lp8860.c @@ -292,61 +292,11 @@ out: return ret; } -static const struct reg_default lp8860_reg_defs[] = { - { LP8860_DISP_CL1_BRT_MSB, 0x00}, - { LP8860_DISP_CL1_BRT_LSB, 0x00}, - { LP8860_DISP_CL1_CURR_MSB, 0x00}, - { LP8860_DISP_CL1_CURR_LSB, 0x00}, - { LP8860_CL2_BRT_MSB, 0x00}, - { LP8860_CL2_BRT_LSB, 0x00}, - { LP8860_CL2_CURRENT, 0x00}, - { LP8860_CL3_BRT_MSB, 0x00}, - { LP8860_CL3_BRT_LSB, 0x00}, - { LP8860_CL3_CURRENT, 0x00}, - { LP8860_CL4_BRT_MSB, 0x00}, - { LP8860_CL4_BRT_LSB, 0x00}, - { LP8860_CL4_CURRENT, 0x00}, - { LP8860_CONFIG, 0x00}, - { LP8860_FAULT_CLEAR, 0x00}, - { LP8860_EEPROM_CNTRL, 0x80}, - { LP8860_EEPROM_UNLOCK, 0x00}, -}; - static const struct regmap_config lp8860_regmap_config = { .reg_bits = 8, .val_bits = 8, .max_register = LP8860_EEPROM_UNLOCK, - .reg_defaults = lp8860_reg_defs, - .num_reg_defaults = ARRAY_SIZE(lp8860_reg_defs), -}; - -static const struct reg_default lp8860_eeprom_defs[] = { - { LP8860_EEPROM_REG_0, 0x00 }, - { LP8860_EEPROM_REG_1, 0x00 }, - { LP8860_EEPROM_REG_2, 0x00 }, - { LP8860_EEPROM_REG_3, 0x00 }, - { LP8860_EEPROM_REG_4, 0x00 }, - { LP8860_EEPROM_REG_5, 0x00 }, - { LP8860_EEPROM_REG_6, 0x00 }, - { LP8860_EEPROM_REG_7, 0x00 }, - { LP8860_EEPROM_REG_8, 0x00 }, - { LP8860_EEPROM_REG_9, 0x00 }, - { LP8860_EEPROM_REG_10, 0x00 }, - { LP8860_EEPROM_REG_11, 0x00 }, - { LP8860_EEPROM_REG_12, 0x00 }, - { LP8860_EEPROM_REG_13, 0x00 }, - { LP8860_EEPROM_REG_14, 0x00 }, - { LP8860_EEPROM_REG_15, 0x00 }, - { LP8860_EEPROM_REG_16, 0x00 }, - { LP8860_EEPROM_REG_17, 0x00 }, - { LP8860_EEPROM_REG_18, 0x00 }, - { LP8860_EEPROM_REG_19, 0x00 }, - { LP8860_EEPROM_REG_20, 0x00 }, - { LP8860_EEPROM_REG_21, 0x00 }, - { LP8860_EEPROM_REG_22, 0x00 }, - { LP8860_EEPROM_REG_23, 0x00 }, - { LP8860_EEPROM_REG_24, 0x00 }, }; static const struct regmap_config lp8860_eeprom_regmap_config = { @@ -354,8 +304,6 @@ static const struct regmap_config lp8860_eeprom_regmap_config = { .val_bits = 8, .max_register = LP8860_EEPROM_REG_24, - .reg_defaults = lp8860_eeprom_defs, - .num_reg_defaults = ARRAY_SIZE(lp8860_eeprom_defs), }; static int lp8860_probe(struct i2c_client *client) -- 2.51.0 From b0d6394094ee657f50b42b44f4903014d90e912a Mon Sep 17 00:00:00 2001 From: Andrew Davis Date: Mon, 7 Apr 2025 13:35:53 -0500 Subject: [PATCH 06/16] leds: lp8860: Enable regulator using enable_optional helper This allows the regulator to be optional which is the same as done here with all the checks for NULL. This also disables on remove for us, so remove the manual disabling. Signed-off-by: Andrew Davis Link: https://lore.kernel.org/r/20250407183555.409687-4-afd@ti.com Signed-off-by: Lee Jones --- drivers/leds/leds-lp8860.c | 32 ++++---------------------------- 1 file changed, 4 insertions(+), 28 deletions(-) diff --git a/drivers/leds/leds-lp8860.c b/drivers/leds/leds-lp8860.c index 4cd1b960d504..2ceebdb5820e 100644 --- a/drivers/leds/leds-lp8860.c +++ b/drivers/leds/leds-lp8860.c @@ -91,7 +91,6 @@ * @regmap: Devices register map * @eeprom_regmap: EEPROM register map * @enable_gpio: VDDIO/EN gpio to enable communication interface - * @regulator: LED supply regulator pointer */ struct lp8860_led { struct mutex lock; @@ -100,7 +99,6 @@ struct lp8860_led { struct regmap *regmap; struct regmap *eeprom_regmap; struct gpio_desc *enable_gpio; - struct regulator *regulator; }; static const struct reg_sequence lp8860_eeprom_disp_regs[] = { @@ -232,15 +230,6 @@ static int lp8860_init(struct lp8860_led *led) unsigned int read_buf; int ret, reg_count; - if (led->regulator) { - ret = regulator_enable(led->regulator); - if (ret) { - dev_err(&led->client->dev, - "Failed to enable regulator\n"); - return ret; - } - } - gpiod_direction_output(led->enable_gpio, 1); ret = lp8860_fault_check(led); @@ -282,13 +271,6 @@ out: if (ret) gpiod_direction_output(led->enable_gpio, 0); - if (led->regulator) { - ret = regulator_disable(led->regulator); - if (ret) - dev_err(&led->client->dev, - "Failed to disable regulator\n"); - } - return ret; } @@ -330,9 +312,10 @@ static int lp8860_probe(struct i2c_client *client) return ret; } - led->regulator = devm_regulator_get(&client->dev, "vled"); - if (IS_ERR(led->regulator)) - led->regulator = NULL; + ret = devm_regulator_get_enable_optional(&client->dev, "vled"); + if (ret && ret != -ENODEV) + return dev_err_probe(&client->dev, ret, + "Failed to enable vled regulator\n"); led->client = client; led->led_dev.brightness_set_blocking = lp8860_brightness_set; @@ -381,13 +364,6 @@ static void lp8860_remove(struct i2c_client *client) int ret; gpiod_direction_output(led->enable_gpio, 0); - - if (led->regulator) { - ret = regulator_disable(led->regulator); - if (ret) - dev_err(&led->client->dev, - "Failed to disable regulator\n"); - } } static const struct i2c_device_id lp8860_id[] = { -- 2.51.0 From e0b95ba33c0f31ac91741921cc89986536595862 Mon Sep 17 00:00:00 2001 From: Andrew Davis Date: Mon, 7 Apr 2025 13:35:54 -0500 Subject: [PATCH 07/16] leds: lp8860: Only unlock in lp8860_unlock_eeprom() Locking is a single register write, so no need to have the unlock function also lock. This removes the need to pass in the option and reduces the nesting level in the function. Signed-off-by: Andrew Davis Link: https://lore.kernel.org/r/20250407183555.409687-5-afd@ti.com Signed-off-by: Lee Jones --- drivers/leds/leds-lp8860.c | 48 ++++++++++++++------------------------ 1 file changed, 18 insertions(+), 30 deletions(-) diff --git a/drivers/leds/leds-lp8860.c b/drivers/leds/leds-lp8860.c index 2ceebdb5820e..8b3660f80f4c 100644 --- a/drivers/leds/leds-lp8860.c +++ b/drivers/leds/leds-lp8860.c @@ -129,39 +129,27 @@ static const struct reg_sequence lp8860_eeprom_disp_regs[] = { { LP8860_EEPROM_REG_24, 0x3E }, }; -static int lp8860_unlock_eeprom(struct lp8860_led *led, int lock) +static int lp8860_unlock_eeprom(struct lp8860_led *led) { int ret; guard(mutex)(&led->lock); - if (lock == LP8860_UNLOCK_EEPROM) { - ret = regmap_write(led->regmap, - LP8860_EEPROM_UNLOCK, - LP8860_EEPROM_CODE_1); - if (ret) { - dev_err(&led->client->dev, "EEPROM Unlock failed\n"); - return ret; - } - - ret = regmap_write(led->regmap, - LP8860_EEPROM_UNLOCK, - LP8860_EEPROM_CODE_2); - if (ret) { - dev_err(&led->client->dev, "EEPROM Unlock failed\n"); - return ret; - } - ret = regmap_write(led->regmap, - LP8860_EEPROM_UNLOCK, - LP8860_EEPROM_CODE_3); - if (ret) { - dev_err(&led->client->dev, "EEPROM Unlock failed\n"); - return ret; - } - } else { - ret = regmap_write(led->regmap, - LP8860_EEPROM_UNLOCK, - LP8860_LOCK_EEPROM); + ret = regmap_write(led->regmap, LP8860_EEPROM_UNLOCK, LP8860_EEPROM_CODE_1); + if (ret) { + dev_err(&led->client->dev, "EEPROM Unlock failed\n"); + return ret; + } + + ret = regmap_write(led->regmap, LP8860_EEPROM_UNLOCK, LP8860_EEPROM_CODE_2); + if (ret) { + dev_err(&led->client->dev, "EEPROM Unlock failed\n"); + return ret; + } + ret = regmap_write(led->regmap, LP8860_EEPROM_UNLOCK, LP8860_EEPROM_CODE_3); + if (ret) { + dev_err(&led->client->dev, "EEPROM Unlock failed\n"); + return ret; } return ret; @@ -240,7 +228,7 @@ static int lp8860_init(struct lp8860_led *led) if (ret) goto out; - ret = lp8860_unlock_eeprom(led, LP8860_UNLOCK_EEPROM); + ret = lp8860_unlock_eeprom(led); if (ret) { dev_err(&led->client->dev, "Failed unlocking EEPROM\n"); goto out; @@ -253,7 +241,7 @@ static int lp8860_init(struct lp8860_led *led) goto out; } - ret = lp8860_unlock_eeprom(led, LP8860_LOCK_EEPROM); + ret = regmap_write(led->regmap, LP8860_EEPROM_UNLOCK, LP8860_LOCK_EEPROM); if (ret) goto out; -- 2.51.0 From 982e0f042542fc0c2fbcb2b67fbedc431624ae59 Mon Sep 17 00:00:00 2001 From: Andrew Davis Date: Mon, 7 Apr 2025 13:35:55 -0500 Subject: [PATCH 08/16] leds: lp8860: Disable GPIO with devm action This helps prevent mistakes like disable out of order in cleanup functions and forgetting to free on error paths (as was done here). This was the last thing the .remove() function did, so remove that too. Signed-off-by: Andrew Davis Link: https://lore.kernel.org/r/20250407183555.409687-6-afd@ti.com Signed-off-by: Lee Jones --- drivers/leds/leds-lp8860.c | 38 +++++++++++++------------------------- 1 file changed, 13 insertions(+), 25 deletions(-) diff --git a/drivers/leds/leds-lp8860.c b/drivers/leds/leds-lp8860.c index 8b3660f80f4c..52b97c9f2a03 100644 --- a/drivers/leds/leds-lp8860.c +++ b/drivers/leds/leds-lp8860.c @@ -90,7 +90,6 @@ * @led_dev: led class device pointer * @regmap: Devices register map * @eeprom_regmap: EEPROM register map - * @enable_gpio: VDDIO/EN gpio to enable communication interface */ struct lp8860_led { struct mutex lock; @@ -98,7 +97,6 @@ struct lp8860_led { struct led_classdev led_dev; struct regmap *regmap; struct regmap *eeprom_regmap; - struct gpio_desc *enable_gpio; }; static const struct reg_sequence lp8860_eeprom_disp_regs[] = { @@ -218,8 +216,6 @@ static int lp8860_init(struct lp8860_led *led) unsigned int read_buf; int ret, reg_count; - gpiod_direction_output(led->enable_gpio, 1); - ret = lp8860_fault_check(led); if (ret) goto out; @@ -256,9 +252,6 @@ static int lp8860_init(struct lp8860_led *led) return ret; out: - if (ret) - gpiod_direction_output(led->enable_gpio, 0); - return ret; } @@ -276,6 +269,13 @@ static const struct regmap_config lp8860_eeprom_regmap_config = { .max_register = LP8860_EEPROM_REG_24, }; +static void lp8860_disable_gpio(void *data) +{ + struct gpio_desc *gpio = data; + + gpiod_set_value(gpio, 0); +} + static int lp8860_probe(struct i2c_client *client) { int ret; @@ -283,6 +283,7 @@ static int lp8860_probe(struct i2c_client *client) struct device_node *np = dev_of_node(&client->dev); struct device_node *child_node; struct led_init_data init_data = {}; + struct gpio_desc *enable_gpio; led = devm_kzalloc(&client->dev, sizeof(*led), GFP_KERNEL); if (!led) @@ -292,13 +293,11 @@ static int lp8860_probe(struct i2c_client *client) if (!child_node) return -EINVAL; - led->enable_gpio = devm_gpiod_get_optional(&client->dev, - "enable", GPIOD_OUT_LOW); - if (IS_ERR(led->enable_gpio)) { - ret = PTR_ERR(led->enable_gpio); - dev_err(&client->dev, "Failed to get enable gpio: %d\n", ret); - return ret; - } + enable_gpio = devm_gpiod_get_optional(&client->dev, "enable", GPIOD_OUT_LOW); + if (IS_ERR(enable_gpio)) + return dev_err_probe(&client->dev, PTR_ERR(enable_gpio), + "Failed to get enable GPIO\n"); + devm_add_action_or_reset(&client->dev, lp8860_disable_gpio, enable_gpio); ret = devm_regulator_get_enable_optional(&client->dev, "vled"); if (ret && ret != -ENODEV) @@ -310,8 +309,6 @@ static int lp8860_probe(struct i2c_client *client) devm_mutex_init(&client->dev, &led->lock); - i2c_set_clientdata(client, led); - led->regmap = devm_regmap_init_i2c(client, &lp8860_regmap_config); if (IS_ERR(led->regmap)) { ret = PTR_ERR(led->regmap); @@ -346,14 +343,6 @@ static int lp8860_probe(struct i2c_client *client) return 0; } -static void lp8860_remove(struct i2c_client *client) -{ - struct lp8860_led *led = i2c_get_clientdata(client); - int ret; - - gpiod_direction_output(led->enable_gpio, 0); -} - static const struct i2c_device_id lp8860_id[] = { { "lp8860" }, { } @@ -372,7 +361,6 @@ static struct i2c_driver lp8860_driver = { .of_match_table = of_lp8860_leds_match, }, .probe = lp8860_probe, - .remove = lp8860_remove, .id_table = lp8860_id, }; module_i2c_driver(lp8860_driver); -- 2.51.0 From f9a2eacb9107365c3f1e3061e2c435cafd554fc8 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Marek=20Beh=C3=BAn?= Date: Thu, 17 Apr 2025 09:05:07 +0200 Subject: [PATCH 09/16] leds: turris-omnia: Drop commas in the terminator entries MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Drop commas in terminator entries of `struct attribute` array and `struct of_device_id` array. Signed-off-by: Marek Behún Link: https://lore.kernel.org/r/20250417070507.24929-1-kabel@kernel.org Signed-off-by: Lee Jones --- drivers/leds/leds-turris-omnia.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/leds/leds-turris-omnia.c b/drivers/leds/leds-turris-omnia.c index 4fe1a9c0bc1b..25ee5c1eb820 100644 --- a/drivers/leds/leds-turris-omnia.c +++ b/drivers/leds/leds-turris-omnia.c @@ -361,7 +361,7 @@ static DEVICE_ATTR_RW(gamma_correction); static struct attribute *omnia_led_controller_attrs[] = { &dev_attr_brightness.attr, &dev_attr_gamma_correction.attr, - NULL, + NULL }; ATTRIBUTE_GROUPS(omnia_led_controller); @@ -527,7 +527,7 @@ static void omnia_leds_remove(struct i2c_client *client) static const struct of_device_id of_omnia_leds_match[] = { { .compatible = "cznic,turris-omnia-leds", }, - {}, + { } }; MODULE_DEVICE_TABLE(of, of_omnia_leds_match); -- 2.51.0 From 4c6c3ca07b7a70e7b8fe5c25cbf0650c422a3a28 Mon Sep 17 00:00:00 2001 From: Krzysztof Kozlowski Date: Thu, 17 Apr 2025 09:46:56 +0200 Subject: [PATCH 10/16] leds: Do not enable by default during compile testing Enabling the compile test should not cause automatic enabling of all drivers, but only allow to choose to compile them. Signed-off-by: Krzysztof Kozlowski Link: https://lore.kernel.org/r/20250417074656.81626-1-krzysztof.kozlowski@linaro.org Signed-off-by: Lee Jones --- drivers/leds/Kconfig | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig index a104cbb0a001..b107dbe1fa90 100644 --- a/drivers/leds/Kconfig +++ b/drivers/leds/Kconfig @@ -735,7 +735,7 @@ config LEDS_NS2 tristate "LED support for Network Space v2 GPIO LEDs" depends on LEDS_CLASS depends on MACH_KIRKWOOD || MACH_ARMADA_370 || COMPILE_TEST - default y + default y if MACH_KIRKWOOD || MACH_ARMADA_370 help This option enables support for the dual-GPIO LEDs found on the following LaCie/Seagate boards: @@ -750,7 +750,7 @@ config LEDS_NETXBIG depends on LEDS_CLASS depends on MACH_KIRKWOOD || COMPILE_TEST depends on OF_GPIO - default y + default MACH_KIRKWOOD help This option enables support for LEDs found on the LaCie 2Big and 5Big Network v2 boards. The LEDs are wired to a CPLD and are -- 2.51.0 From ee08ec51a0a01ed8702a7f91e5102dad92319172 Mon Sep 17 00:00:00 2001 From: Bartosz Golaszewski Date: Wed, 23 Apr 2025 09:53:50 +0200 Subject: [PATCH 11/16] leds: lgm-sso: Use new GPIO line value setter callbacks struct gpio_chip now has callbacks for setting line values that return an integer, allowing to indicate failures. Convert the driver to using them. Signed-off-by: Bartosz Golaszewski Reviewed-by: Linus Walleij Link: https://lore.kernel.org/r/20250423-gpiochip-set-rv-leds-v1-1-2f42d8fbb525@linaro.org Signed-off-by: Lee Jones --- drivers/leds/blink/leds-lgm-sso.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/leds/blink/leds-lgm-sso.c b/drivers/leds/blink/leds-lgm-sso.c index effaaaf302b5..c9027f9c4bb7 100644 --- a/drivers/leds/blink/leds-lgm-sso.c +++ b/drivers/leds/blink/leds-lgm-sso.c @@ -450,7 +450,7 @@ static int sso_gpio_get(struct gpio_chip *chip, unsigned int offset) return !!(reg_val & BIT(offset)); } -static void sso_gpio_set(struct gpio_chip *chip, unsigned int offset, int value) +static int sso_gpio_set(struct gpio_chip *chip, unsigned int offset, int value) { struct sso_led_priv *priv = gpiochip_get_data(chip); @@ -458,6 +458,8 @@ static void sso_gpio_set(struct gpio_chip *chip, unsigned int offset, int value) if (!priv->gpio.freq) regmap_update_bits(priv->mmap, SSO_CON0, SSO_CON0_SWU, SSO_CON0_SWU); + + return 0; } static int sso_gpio_gc_init(struct device *dev, struct sso_led_priv *priv) @@ -469,7 +471,7 @@ static int sso_gpio_gc_init(struct device *dev, struct sso_led_priv *priv) gc->get_direction = sso_gpio_get_dir; gc->direction_output = sso_gpio_dir_out; gc->get = sso_gpio_get; - gc->set = sso_gpio_set; + gc->set_rv = sso_gpio_set; gc->label = "lgm-sso"; gc->base = -1; -- 2.51.0 From 2aafd2e41cf1434c680ea7410333a079a15747c3 Mon Sep 17 00:00:00 2001 From: Bartosz Golaszewski Date: Wed, 23 Apr 2025 09:53:51 +0200 Subject: [PATCH 12/16] leds: pca955x: Use new GPIO line value setter callbacks struct gpio_chip now has callbacks for setting line values that return an integer, allowing to indicate failures. Convert the driver to using them. Signed-off-by: Bartosz Golaszewski Reviewed-by: Linus Walleij Link: https://lore.kernel.org/r/20250423-gpiochip-set-rv-leds-v1-2-2f42d8fbb525@linaro.org Signed-off-by: Lee Jones --- drivers/leds/leds-pca955x.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/leds/leds-pca955x.c b/drivers/leds/leds-pca955x.c index 24a40a1cdb15..42fe056b1c74 100644 --- a/drivers/leds/leds-pca955x.c +++ b/drivers/leds/leds-pca955x.c @@ -495,10 +495,10 @@ static int pca955x_set_value(struct gpio_chip *gc, unsigned int offset, return pca955x_led_set(&led->led_cdev, PCA955X_GPIO_LOW); } -static void pca955x_gpio_set_value(struct gpio_chip *gc, unsigned int offset, - int val) +static int pca955x_gpio_set_value(struct gpio_chip *gc, unsigned int offset, + int val) { - pca955x_set_value(gc, offset, val); + return pca955x_set_value(gc, offset, val); } static int pca955x_gpio_get_value(struct gpio_chip *gc, unsigned int offset) @@ -737,7 +737,7 @@ static int pca955x_probe(struct i2c_client *client) pca955x->gpio.label = "gpio-pca955x"; pca955x->gpio.direction_input = pca955x_gpio_direction_input; pca955x->gpio.direction_output = pca955x_gpio_direction_output; - pca955x->gpio.set = pca955x_gpio_set_value; + pca955x->gpio.set_rv = pca955x_gpio_set_value; pca955x->gpio.get = pca955x_gpio_get_value; pca955x->gpio.request = pca955x_gpio_request_pin; pca955x->gpio.free = pca955x_gpio_free_pin; -- 2.51.0 From e1cc2c8cc7ccccb52814d10c98de1252f8a7ae61 Mon Sep 17 00:00:00 2001 From: Bartosz Golaszewski Date: Wed, 23 Apr 2025 09:53:52 +0200 Subject: [PATCH 13/16] leds: pca9532: Use new GPIO line value setter callbacks struct gpio_chip now has callbacks for setting line values that return an integer, allowing to indicate failures. Convert the driver to using them. Signed-off-by: Bartosz Golaszewski Reviewed-by: Linus Walleij Link: https://lore.kernel.org/r/20250423-gpiochip-set-rv-leds-v1-3-2f42d8fbb525@linaro.org Signed-off-by: Lee Jones --- drivers/leds/leds-pca9532.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/leds/leds-pca9532.c b/drivers/leds/leds-pca9532.c index 1b47acf54720..7d4c071a6cd0 100644 --- a/drivers/leds/leds-pca9532.c +++ b/drivers/leds/leds-pca9532.c @@ -318,7 +318,8 @@ static int pca9532_gpio_request_pin(struct gpio_chip *gc, unsigned offset) return -EBUSY; } -static void pca9532_gpio_set_value(struct gpio_chip *gc, unsigned offset, int val) +static int pca9532_gpio_set_value(struct gpio_chip *gc, unsigned int offset, + int val) { struct pca9532_data *data = gpiochip_get_data(gc); struct pca9532_led *led = &data->leds[offset]; @@ -329,6 +330,8 @@ static void pca9532_gpio_set_value(struct gpio_chip *gc, unsigned offset, int va led->state = PCA9532_OFF; pca9532_setled(led); + + return 0; } static int pca9532_gpio_get_value(struct gpio_chip *gc, unsigned offset) @@ -351,9 +354,7 @@ static int pca9532_gpio_direction_input(struct gpio_chip *gc, unsigned offset) static int pca9532_gpio_direction_output(struct gpio_chip *gc, unsigned offset, int val) { - pca9532_gpio_set_value(gc, offset, val); - - return 0; + return pca9532_gpio_set_value(gc, offset, val); } #endif /* CONFIG_LEDS_PCA9532_GPIO */ @@ -472,7 +473,7 @@ static int pca9532_configure(struct i2c_client *client, data->gpio.label = "gpio-pca9532"; data->gpio.direction_input = pca9532_gpio_direction_input; data->gpio.direction_output = pca9532_gpio_direction_output; - data->gpio.set = pca9532_gpio_set_value; + data->gpio.set_rv = pca9532_gpio_set_value; data->gpio.get = pca9532_gpio_get_value; data->gpio.request = pca9532_gpio_request_pin; data->gpio.can_sleep = 1; -- 2.51.0 From d1d3205730735b652303a82ba49fcfef7c20510d Mon Sep 17 00:00:00 2001 From: Bartosz Golaszewski Date: Wed, 23 Apr 2025 09:53:53 +0200 Subject: [PATCH 14/16] leds: tca6507: Use new GPIO line value setter callbacks struct gpio_chip now has callbacks for setting line values that return an integer, allowing to indicate failures. Convert the driver to using them. Signed-off-by: Bartosz Golaszewski Reviewed-by: Linus Walleij Link: https://lore.kernel.org/r/20250423-gpiochip-set-rv-leds-v1-4-2f42d8fbb525@linaro.org Signed-off-by: Lee Jones --- drivers/leds/leds-tca6507.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/leds/leds-tca6507.c b/drivers/leds/leds-tca6507.c index acbd8169723c..89c165c8ee9c 100644 --- a/drivers/leds/leds-tca6507.c +++ b/drivers/leds/leds-tca6507.c @@ -588,8 +588,8 @@ static int tca6507_blink_set(struct led_classdev *led_cdev, } #ifdef CONFIG_GPIOLIB -static void tca6507_gpio_set_value(struct gpio_chip *gc, - unsigned offset, int val) +static int tca6507_gpio_set_value(struct gpio_chip *gc, unsigned int offset, + int val) { struct tca6507_chip *tca = gpiochip_get_data(gc); unsigned long flags; @@ -604,13 +604,14 @@ static void tca6507_gpio_set_value(struct gpio_chip *gc, spin_unlock_irqrestore(&tca->lock, flags); if (tca->reg_set) schedule_work(&tca->work); + + return 0; } static int tca6507_gpio_direction_output(struct gpio_chip *gc, unsigned offset, int val) { - tca6507_gpio_set_value(gc, offset, val); - return 0; + return tca6507_gpio_set_value(gc, offset, val); } static int tca6507_probe_gpios(struct device *dev, @@ -636,7 +637,7 @@ static int tca6507_probe_gpios(struct device *dev, tca->gpio.base = -1; tca->gpio.owner = THIS_MODULE; tca->gpio.direction_output = tca6507_gpio_direction_output; - tca->gpio.set = tca6507_gpio_set_value; + tca->gpio.set_rv = tca6507_gpio_set_value; tca->gpio.parent = dev; err = devm_gpiochip_add_data(dev, &tca->gpio, tca); if (err) { -- 2.51.0 From 5039a33fed8851fcf384fae2bcb8fd4858edd597 Mon Sep 17 00:00:00 2001 From: Lee Jones Date: Thu, 24 Apr 2025 15:45:38 +0100 Subject: [PATCH 15/16] leds: Provide skeleton KUnit testing for the LEDs framework Apply a very basic implementation of KUnit LED testing. More tests / use-cases will be added steadily over time. CMD: tools/testing/kunit/kunit.py run --kunitconfig drivers/leds OUTPUT: [15:34:19] Configuring KUnit Kernel ... [15:34:19] Building KUnit Kernel ... Populating config with: $ make ARCH=um O=.kunit olddefconfig Building with: $ make all compile_commands.json scripts_gdb ARCH=um O=.kunit --jobs=20 [15:34:22] Starting KUnit Kernel (1/1)... [15:34:22] ============================================================ Running tests with: $ .kunit/linux kunit.enable=1 mem=1G console=tty kunit_shutdown=halt [15:34:23] ===================== led (1 subtest) ====================== [15:34:23] [PASSED] led_test_class_register [15:34:23] ======================= [PASSED] led ======================= [15:34:23] ============================================================ [15:34:23] Testing complete. Ran 1 tests: passed: 1 [15:34:23] Elapsed time: 4.268s total, 0.001s configuring, 3.048s building, 1.214s running Link: https://lore.kernel.org/r/20250424144544.1438584-1-lee@kernel.org Signed-off-by: Lee Jones --- drivers/leds/.kunitconfig | 4 +++ drivers/leds/Kconfig | 7 ++++ drivers/leds/Makefile | 1 + drivers/leds/led-test.c | 76 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 88 insertions(+) create mode 100644 drivers/leds/.kunitconfig create mode 100644 drivers/leds/led-test.c diff --git a/drivers/leds/.kunitconfig b/drivers/leds/.kunitconfig new file mode 100644 index 000000000000..5180f77910a1 --- /dev/null +++ b/drivers/leds/.kunitconfig @@ -0,0 +1,4 @@ +CONFIG_KUNIT=y +CONFIG_NEW_LEDS=y +CONFIG_LEDS_CLASS=y +CONFIG_LEDS_KUNIT_TEST=y diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig index b107dbe1fa90..6e3dce7e35a4 100644 --- a/drivers/leds/Kconfig +++ b/drivers/leds/Kconfig @@ -55,6 +55,13 @@ config LEDS_BRIGHTNESS_HW_CHANGED See Documentation/ABI/testing/sysfs-class-led for details. +config LEDS_KUNIT_TEST + tristate "KUnit tests for LEDs" + depends on KUNIT && LEDS_CLASS + default KUNIT_ALL_TESTS + help + Say Y here to enable KUnit testing for the LEDs framework. + comment "LED drivers" config LEDS_88PM860X diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile index 2f170d69dcbf..9a0333ec1a86 100644 --- a/drivers/leds/Makefile +++ b/drivers/leds/Makefile @@ -6,6 +6,7 @@ obj-$(CONFIG_LEDS_CLASS) += led-class.o obj-$(CONFIG_LEDS_CLASS_FLASH) += led-class-flash.o obj-$(CONFIG_LEDS_CLASS_MULTICOLOR) += led-class-multicolor.o obj-$(CONFIG_LEDS_TRIGGERS) += led-triggers.o +obj-$(CONFIG_LEDS_KUNIT_TEST) += led-test.o # LED Platform Drivers (keep this sorted, M-| sort) obj-$(CONFIG_LEDS_88PM860X) += leds-88pm860x.o diff --git a/drivers/leds/led-test.c b/drivers/leds/led-test.c new file mode 100644 index 000000000000..068c9d0eb683 --- /dev/null +++ b/drivers/leds/led-test.c @@ -0,0 +1,76 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (C) 2025 Google LLC + * + * Author: Lee Jones + */ + +#include +#include +#include +#include + +struct led_test_ddata { + struct led_classdev cdev; + struct device *dev; +}; + +static void led_test_class_register(struct kunit *test) +{ + struct led_test_ddata *ddata = test->priv; + struct led_classdev *cdev = &ddata->cdev; + struct device *dev = ddata->dev; + int ret; + + cdev->name = "led-test"; + + ret = devm_led_classdev_register(dev, cdev); + KUNIT_ASSERT_EQ(test, ret, 0); + if (ret) + return; +} + +static struct kunit_case led_test_cases[] = { + KUNIT_CASE(led_test_class_register), + { } +}; + +static int led_test_init(struct kunit *test) +{ + struct led_test_ddata *ddata; + struct device *dev; + + ddata = kunit_kzalloc(test, sizeof(*ddata), GFP_KERNEL); + if (!ddata) + return -ENOMEM; + + test->priv = ddata; + + dev = kunit_device_register(test, "led_test"); + if (IS_ERR(dev)) + return PTR_ERR(dev); + + ddata->dev = get_device(dev); + + return 0; +} + +static void led_test_exit(struct kunit *test) +{ + struct led_test_ddata *ddata = test->priv; + + if (ddata && ddata->dev) + put_device(ddata->dev); +} + +static struct kunit_suite led_test_suite = { + .name = "led", + .init = led_test_init, + .exit = led_test_exit, + .test_cases = led_test_cases, +}; +kunit_test_suite(led_test_suite); + +MODULE_AUTHOR("Lee Jones "); +MODULE_DESCRIPTION("KUnit tests for the LED framework"); +MODULE_LICENSE("GPL"); -- 2.51.0 From b441b95a592c42f8448f9bd912b91e1e9b4262ff Mon Sep 17 00:00:00 2001 From: Jesse Karjalainen Date: Sat, 26 Apr 2025 05:04:54 +0300 Subject: [PATCH 16/16] leds: pca995x: Fix typo in pca995x_of_match's of_device_id entry Remove the stray space between the '.' and the 'data' field name in the PCA995x device-tree match table. Signed-off-by: Jesse Karjalainen Link: https://lore.kernel.org/r/20250426020454.47059-1-jesse@ponkila.com Signed-off-by: Lee Jones --- drivers/leds/leds-pca995x.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/leds/leds-pca995x.c b/drivers/leds/leds-pca995x.c index 11c7bb69573e..6ad06ce2bf64 100644 --- a/drivers/leds/leds-pca995x.c +++ b/drivers/leds/leds-pca995x.c @@ -197,7 +197,7 @@ MODULE_DEVICE_TABLE(i2c, pca995x_id); static const struct of_device_id pca995x_of_match[] = { { .compatible = "nxp,pca9952", .data = &pca9952_chipdef }, - { .compatible = "nxp,pca9955b", . data = &pca9955b_chipdef }, + { .compatible = "nxp,pca9955b", .data = &pca9955b_chipdef }, { .compatible = "nxp,pca9956b", .data = &pca9956b_chipdef }, {}, }; -- 2.51.0