Re: [PATCH v3 6/6] leds: aw2013: Move reset register write

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] leds: aw2013: Double mutex unlock on error paths in aw2013_probe()
- [Low] leds: aw2013: Use of dev_err() instead of dev_err_probe() for probe failures
--

commit 96e8f9d5ce7deeb0752d6125191c77378696f3e4
Author: Paul Sajna <sajattack@postmarketos.org>

leds: aw2013: Move reset register write

This patch moves the chip reset register write from aw2013_probe_dt() to
aw2013_probe(), and unlocks the mutex before calling aw2013_probe_dt() to
avoid a deadlock. The deadlock would otherwise occur when LED classdev
registration triggers a brightness set callback.

> diff --git a/drivers/leds/leds-aw2013.c b/drivers/leds/leds-aw2013.c
> index 6e661afd77755..8d1d649064c71 100644
> --- a/drivers/leds/leds-aw2013.c
> +++ b/drivers/leds/leds-aw2013.c
[ ... ]
> @@ -504,6 +502,14 @@ static int aw2013_probe(struct i2c_client *client)
>  	if (ret)
>  		goto error_reg;
>  
> +	ret = regmap_write(chip->regmap, AW2013_RSTR, AW2013_RSTR_RESET);
> +	if (ret) {
> +		dev_err(&chip->client->dev, "Failed to reset the chip: %d\n", ret);

[Severity: Low]
Is dev_err() the right function to use here for reporting a probe failure?
The subsystem guidelines recommend using dev_err_probe() in probe paths
to ensure proper handling of -EPROBE_DEFER and correct kernel log formatting.

> +		goto error_reg;
> +	}
> +
> +	mutex_unlock(&chip->mutex);
> +
>  	ret = aw2013_probe_dt(chip);
>  	if (ret < 0)
>  		goto error_reg;

[Severity: High]
Can jumping to error_reg here cause a double unlock of chip->mutex?

By moving mutex_unlock(&chip->mutex) to execute before aw2013_probe_dt(),
the mutex is now unlocked on this error path. However, jumping to error_reg
causes it to fall through to the error label, which unconditionally unlocks
the mutex again:

error_reg:
	regulator_bulk_disable(ARRAY_SIZE(chip->regulators),
			       chip->regulators);

error:
	mutex_unlock(&chip->mutex);
	return ret;

Additionally, if aw2013_probe_dt() succeeds but the subsequent
regulator_bulk_disable() fails, it also jumps to the error label and unlocks
the already-unlocked mutex.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261012-aw2013-aw20xx-rename-v3-0-cab1fdd050b6@postmarketos.org?part=6