Re: [PATCH v3] leds: pca9532: Propagate I2C errors

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [High] leds: pca9532: sleeping mutex acquired in atomic
  pca9532_event() callback
--

commit c7d1a2a939904c415bec23e00e60cb3b774b087c
Author: Daniil Demakov <t3127430@gmail.com>

leds: pca9532: Propagate I2C errors

This commit checks and propagates SMBus read and write errors across LED,
PWM, blink, and GPIO operations while serializing shared state under
update_lock.

> diff --git a/drivers/leds/leds-pca9532.c b/drivers/leds/leds-pca9532.c
> index c8085427696ff..eb9020e77b46d 100644
> --- a/drivers/leds/leds-pca9532.c
> +++ b/drivers/leds/leds-pca9532.c

[ ... ]

> @@ -255,15 +369,32 @@ static int pca9532_set_blink(struct led_classdev *led_cdev,

[ ... ]

>  static int pca9532_event(struct input_dev *dev, unsigned int type,
> -	unsigned int code, int value)
> +			 unsigned int code, int value)
>  {
>  	struct pca9532_data *data = input_get_drvdata(dev);
>  
> @@ -271,11 +402,15 @@ static int pca9532_event(struct input_dev *dev, unsigned int type,
>  		return -1;
>  
>  	/* XXX: allow different kind of beeps with psc/pwm modifications */
> +	mutex_lock(&data->update_lock);
> +
>  	if (value > 1 && value < 32767)
>  		data->pwm[PCA9532_PWM_ID_1] = 127;
>  	else
>  		data->pwm[PCA9532_PWM_ID_1] = 0;
>  
> +	mutex_unlock(&data->update_lock);
> +
>  	schedule_work(&data->work);
>  
>  	return 0;

[Severity: High]
Does acquiring data->update_lock here sleep in atomic context?

pca9532_event() is the struct input_dev::event callback, which input core
invokes while holding spin_lock_irqsave(&dev->event_lock) (for example via
input_inject_event() -> input_handle_event() -> input_event_dispose()).
Delivering an EV_SND event when a pin is configured as
PCA9532_TYPE_N2100_BEEP will sleep while atomic and can hang or panic the
kernel if data->update_lock is contended.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261114181438.22204-1-t3127430@gmail.com?part=1