Re: [PATCH v6 3/3] leds: aw2013: Use dev_err_probe

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [High] leds: aw2013: I2C write in aw2013_probe_dt() while regulators
  are disabled
- [Low] leds: aw2013: missed dev_err_probe() conversion in
  aw2013_probe()
--

commit 82b1ad2c2b840e1ab229066538d745664fd9ce9b
Author: Paul Sajna <sajattack@postmarketos.org>

leds: aw2013: Use dev_err_probe

This commit converts error logging in aw2013_probe() from dev_err() to
dev_err_probe().

> diff --git a/drivers/leds/leds-aw2013.c b/drivers/leds/leds-aw2013.c
> index 5ff9887086a61..f1106456d4edc 100644
> --- a/drivers/leds/leds-aw2013.c
> +++ b/drivers/leds/leds-aw2013.c

[ ... ]

> @@ -509,30 +508,29 @@ static int aw2013_probe(struct i2c_client *client)

[ ... ]

>  	if (chipid != chip->cdef->chip_id) {
> -		dev_err(&client->dev, "Chip reported wrong ID: %x\n",
> -			chipid);
> -		ret = -ENODEV;
> +		ret = dev_err_probe(&client->dev, -ENODEV,
> +				    "Chip reported wrong ID: %x\n",
> +				     chipid);
>  		goto error_reg;
>  	}

[Severity: Low]
Should the regmap_write(chip->regmap, AW2013_RSTR, AW2013_RSTR_RESET) error
path immediately following this block also use dev_err_probe() for
consistency with the rest of aw2013_probe()?

> @@ -545,8 +543,8 @@ static int aw2013_probe(struct i2c_client *client)
>  	ret = regulator_bulk_disable(ARRAY_SIZE(chip->regulators),
>  				     chip->regulators);
>  	if (ret) {
> -		dev_err(&client->dev,
> -			"Failed to disable regulators: %d\n", ret);
> +		ret = dev_err_probe(&client->dev, ret,
> +				    "Failed to disable regulators\n");
>  		goto error;
>  	}

[Severity: High]
Does aw2013_probe_dt() write to AW20XX_GCR2 over I2C after
regulator_bulk_disable() has powered down the vcc and vio supplies?

drivers/leds/leds-aw2013.c:aw2013_probe_dt() {
	...
	if (chip->cdef->gcr2_imax) {
		mutex_lock(&chip->mutex);
		ret = aw2027_set_global_imax(chip);
		if (ret) {
			mutex_unlock(&chip->mutex);
			return ret;
		}
		mutex_unlock(&chip->mutex);
	}
	...
}

On AW2027 boards with switchable vcc or vio regulators, this I2C access
While the supplies are disabled will fail and abort probe.  Since
aw2013_chip_init() already calls aw2027_set_global_imax() after enabling
the regulators and the chip, the call in aw2013_probe_dt() can be dropped.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112-aw2013-aw20xx-rename-v6-0-251eca340abc@postmarketos.org?part=3