Re: [PATCH v3 2/6] leds: aw2013: Add AW2027 support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] locking: double mutex unlock on probe error path
- [Medium] leds: aw2013: 4-bit AW2027 local current limit truncated by 2-bit mask
--

commit 117fb3f76f08e72c0e73f7c0543a74753854f334
Author: Paul Sajna <sajattack@postmarketos.org>
leds: aw2013: Add AW2027 support

This commit adds support for the AW2027 LED driver. It introduces a chip
definition structure to handle differences such as maximum current, current
levels for color-mixing, and global versus local current configuration.

> diff --git a/drivers/leds/leds-aw2013.c b/drivers/leds/leds-aw2013.c
> index 216755d6010fe..4da33a86af16a 100644
> --- a/drivers/leds/leds-aw2013.c
> +++ b/drivers/leds/leds-aw2013.c
> @@ -81,17 +144,50 @@ static int aw2013_chip_init(struct aw2013 *chip)

[ ... ]

> +	if (chip->cdef->gcr2_imax) {

[ ... ]

> +		for (i = 0; i < chip->num_leds; i++) {
> +			ret = regmap_update_bits(chip->regmap,
> +						 AW2013_LCFG(chip->leds[i].num),
> +						 AW2013_IMAX_MASK,
> +						 chip->leds[i].imax);

[Severity: Medium]
Does this code inadvertently truncate the AW2027 local current limit?

For the AW2027, aw2013_microamp_to_imax() calculates a 4-bit limit (0-15) based
on the led-max-microamp property. However, AW2013_IMAX_MASK is defined as
BIT(0) | BIT(1) (0x03).

By using a 2-bit mask here, the upper 2 bits of the 4-bit imax value are
silently discarded. Does this effectively limit the maximum LED brightness to a
fraction of its intended capability and break the 16-level color-mixing control?

[ ... ]

> @@ -420,7 +516,8 @@ static int aw2013_probe(struct i2c_client *client)
>  }

[Severity: High]
This problem wasn't introduced by this patch, but patch 6 in this series moved
the mutex_unlock(&chip->mutex) before aw2013_probe_dt() without updating the
error paths.

If aw2013_probe_dt() fails, or if the final regulator_bulk_disable() fails,
the probe function jumps to the error_reg or error cleanup labels:

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

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

Since the mutex is already unlocked prior to calling aw2013_probe_dt(), does
this path cause a double-unlock of chip->mutex?

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