Re: [PATCH v2 4/7] leds: aw2013: use dev_err_probe

[Andreas Kemnade <andreas@kemnade.info>]

On Mon, 05 Oct 2026 20:46:05 -0700
Paul Sajna <sajattack@postmarketos.org> wrote:

> Failure to use dev_err_probe() for regulator requests violates
> LED subsystem guidelines.
> 
This cleanup is good.

But your comment sounds like there are documented guidelines and not just common
practice, so please add a link if it is not in an usual place which it is not:

~/linux/Documentation/leds$ grep -R probe *
leds-blinkm.rst:  $ modprobe ledtrig-heartbeat
well-known-leds.txt:wants to use particular feature, you should probe for good name, first,

~/linux/Documentation/leds$ grep -R http *
leds-lm3556.rst:* Datasheet: http://www.national.com/ds/LM/LM3556.pdf
leds-lp3944.rst:	http://www.national.com/pf/LP/LP3944.html
leds-lp5521.rst:* Datasheet: http://www.national.com/pf/LP/LP5521.html
leds-lp5523.rst:* Datasheet: http://www.national.com/pf/LP/LP5523.html
leds-lp5812.rst:* Datasheet: https://www.ti.com/product/LP5812#tech-docs

No links to additional documentation either.

If no such documentation exist, then do not talk about it. Just
say something like "to simplify und unify error reporting" as reason.

Regards,
Andreas

> The original driver didn't use it, but it's worth cleaning up
> while I'm here.
> 
> Signed-off-by: Paul Sajna <sajattack@postmarketos.org>
> ---
>  drivers/leds/leds-aw2013.c | 28 +++++++++++++---------------
>  1 file changed, 13 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/leds/leds-aw2013.c b/drivers/leds/leds-aw2013.c
> index 340eefdda04f..705de95c6eac 100644
> --- a/drivers/leds/leds-aw2013.c
> +++ b/drivers/leds/leds-aw2013.c
> @@ -448,9 +448,8 @@ static int aw20xx_probe(struct i2c_client *client)
>  
>  	chip->regmap = devm_regmap_init_i2c(client, chip->cdef->regmap_cfg);
>  	if (IS_ERR(chip->regmap)) {
> -		ret = PTR_ERR(chip->regmap);
> -		dev_err(&client->dev, "Failed to allocate register map: %d\n",
> -			ret);
> +		ret = dev_err_probe(&client->dev, PTR_ERR(chip->regmap),
> +				    "Failed to allocate register map\n");
>  		goto error;
>  	}
>  
> @@ -460,30 +459,30 @@ static int aw20xx_probe(struct i2c_client *client)
>  				      ARRAY_SIZE(chip->regulators),
>  				      chip->regulators);
>  	if (ret < 0) {
> -		if (ret != -EPROBE_DEFER)
> -			dev_err(&client->dev,
> -				"Failed to request regulators: %d\n", ret);
> +		ret = dev_err_probe(&client->dev, ret,
> +				     "Failed to request regulators\n");
>  		goto error;
>  	}
>  
>  	ret = regulator_bulk_enable(ARRAY_SIZE(chip->regulators),
>  				    chip->regulators);
>  	if (ret) {
> -		dev_err(&client->dev,
> -			"Failed to enable regulators: %d\n", ret);
> +		ret = dev_err_probe(&client->dev, ret,
> +				     "Failed to enable regulators\n");
>  		goto error;
>  	}
>  
>  	ret = regmap_read(chip->regmap, AW20XX_RSTR, &chipid);
>  	if (ret) {
> -		dev_err(&client->dev, "Failed to read chip ID: %d\n",
> -			ret);
> +		ret = dev_err_probe(&client->dev, ret,
> +				     "Failed to read chip ID\n");
>  		goto error_reg;
>  	}
>  	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, ret,
> +				     "Chip reported wrong ID: %x\n",
> +				     chipid);
>  		goto error_reg;
>  	}
>  
> @@ -498,13 +497,12 @@ static int aw20xx_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;
>  	}
>  
>  	mutex_unlock(&chip->mutex);
> -
>  	return 0;
>  
>  error_reg:
>