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: 259230378c ("leds: Add LED1202 I2C driver")
Signed-off-by: Manuel Fombuena <fombuena@outlook.com>
Assisted-by: LLM
Link: https://patch.msgid.link/GV1PR08MB8497CF8FB57A272EB671F0C6C5BA2@GV1PR08MB8497.eurprd08.prod.outlook.com
Signed-off-by: Lee Jones <lee@kernel.org>
This commit is contained in:
Manuel Fombuena 2026-09-15 13:46:31 +01:00 committed by Lee Jones
parent 5882de94ed
commit 1ec35fa181

View File

@ -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;
}