From 1ec35fa181981b4b9581b3dae967bac7ed82c89c Mon Sep 17 00:00:00 2001 From: Manuel Fombuena Date: Tue, 15 Sep 2026 13:46:31 +0100 Subject: [PATCH] leds: st1202: Do not set brightness from atomic context st1202_brightness_set() takes the chip mutex and performs I2C transfers, both of which can sleep, but it is installed as the brightness_set callback. led_set_brightness_nopm() calls that op directly on the assumption that it never sleeps, including from softirq context, so the driver can sleep in atomic context. The driver also installs brightness_set_blocking, but the core only falls back to it when brightness_set is absent, so st1202_led_set() was never reached. It merely turned the channel on or off and ignored the requested brightness, so dropping brightness_set on its own would have lost brightness control entirely. Move the register programming into st1202_led_set() so the blocking callback honours the brightness value, and drop brightness_set. The core now defers the update to a work queue, where sleeping is allowed. Errors from the register writes are propagated rather than discarded. st1202_led_set() was the last user of the locking st1202_channel_set() wrapper, which is removed. Programming every PWM slot to full scale is kept: it is what makes the brightness visible while the global sequencer is running, without having to stop the sequencer and disturb the other channels. Fixes: 259230378c65 ("leds: Add LED1202 I2C driver") Signed-off-by: Manuel Fombuena Assisted-by: LLM Link: https://patch.msgid.link/GV1PR08MB8497CF8FB57A272EB671F0C6C5BA2@GV1PR08MB8497.eurprd08.prod.outlook.com Signed-off-by: Lee Jones --- drivers/leds/leds-st1202.c | 45 +++++++++++++++++++------------------- 1 file changed, 22 insertions(+), 23 deletions(-) diff --git a/drivers/leds/leds-st1202.c b/drivers/leds/leds-st1202.c index ead7a3669312..101468557f6c 100644 --- a/drivers/leds/leds-st1202.c +++ b/drivers/leds/leds-st1202.c @@ -173,27 +173,6 @@ static int __st1202_channel_set(struct st1202_chip *chip, int led_num, bool acti return 0; } -static int st1202_channel_set(struct st1202_chip *chip, int led_num, bool active) -{ - guard(mutex)(&chip->lock); - - return __st1202_channel_set(chip, led_num, active); -} - -static void st1202_brightness_set(struct led_classdev *led_cdev, - enum led_brightness value) -{ - struct st1202_led *led = cdev_to_st1202_led(led_cdev); - struct st1202_chip *chip = led->chip; - - guard(mutex)(&chip->lock); - - for (int pattern = 0; pattern < ST1202_MAX_PATTERNS; pattern++) - st1202_pwm_pattern_write(chip, led->led_num, pattern, ST1202_PATTERN_PWM_FULL); - st1202_write_reg(chip, ST1202_ILED_REG0 + led->led_num, value); - __st1202_channel_set(chip, led->led_num, !!value); -} - static enum led_brightness st1202_brightness_get(struct led_classdev *led_cdev) { struct st1202_led *led = cdev_to_st1202_led(led_cdev); @@ -210,8 +189,29 @@ static enum led_brightness st1202_brightness_get(struct led_classdev *led_cdev) static int st1202_led_set(struct led_classdev *ldev, enum led_brightness value) { struct st1202_led *led = cdev_to_st1202_led(ldev); + struct st1202_chip *chip = led->chip; + int ret; - return st1202_channel_set(led->chip, led->led_num, !!value); + guard(mutex)(&chip->lock); + + /* + * The output of a channel is ILED x Pattern_PWM / 4095. Setting every + * PWM slot to full scale makes it equal ILED whatever the state of the + * sequencer, so the brightness takes effect without stopping the + * sequencer, which is global and would disturb the other channels. + */ + for (int pattern = 0; pattern < ST1202_MAX_PATTERNS; pattern++) { + ret = st1202_pwm_pattern_write(chip, led->led_num, pattern, + ST1202_PATTERN_PWM_FULL); + if (ret) + return ret; + } + + ret = st1202_write_reg(chip, ST1202_ILED_REG0 + led->led_num, value); + if (ret) + return ret; + + return __st1202_channel_set(chip, led->led_num, !!value); } static int st1202_led_pattern_clear(struct led_classdev *ldev) @@ -436,7 +436,6 @@ static int st1202_dt_init(struct st1202_chip *chip) led->led_cdev.pattern_clear = st1202_led_pattern_clear; led->led_cdev.default_trigger = "pattern"; led->led_cdev.blink_set = st1202_blink_set; - led->led_cdev.brightness_set = st1202_brightness_set; led->led_cdev.brightness_get = st1202_brightness_get; }