Re: [PATCH v4 2/5] iio: light: al3000a: Fix an error handling path in al3000a_probe()

[Jonathan Cameron <jic23@kernel.org>]

On Wed, 02 Apr 2025 21:33:25 +0200
David Heidelberg via B4 Relay <devnull+david.ixit.cz@kernel.org> wrote:

> From: David Heidelberg <david@ixit.cz>
> 
> If regmap_write() fails in al3000a_init(), al3000a_set_pwr_off is
> not called.
> 
> In order to avoid such a situation, move the devm_add_action_or_reset()
> which calls al3000a_set_pwr_off right after a successful
> al3000a_set_pwr_on.
> 
> Signed-off-by: David Heidelberg <david@ixit.cz>
> ---
>  drivers/iio/light/al3000a.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iio/light/al3000a.c b/drivers/iio/light/al3000a.c
> index e2fbb1270040f43d9f0a97838861818a8eaef813..6d5115b2a06c5acce18d831b9c41d3d5121fba12 100644
> --- a/drivers/iio/light/al3000a.c
> +++ b/drivers/iio/light/al3000a.c
> @@ -85,12 +85,17 @@ static void al3000a_set_pwr_off(void *_data)
>  
>  static int al3000a_init(struct al3000a_data *data)
>  {
> +	struct device *dev = regmap_get_device(data->regmap);
>  	int ret;
>  
>  	ret = al3000a_set_pwr_on(data);
>  	if (ret)
>  		return ret;
>  
> +	ret = devm_add_action_or_reset(dev, al3000a_set_pwr_off, data);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to add action\n");
Whilst this is the same thing Christophe pointed out in patch 1, as it is simple code
movement I think it is fine to leave it here 'for now'. 

It probably makes sense to scrub IIO in general for cases of this if we decide
the general rule is no error messages for devm_add_action* failures (given they
are only possible if memory allocation fails).

Jonathan

> +
>  	ret = regmap_write(data->regmap, AL3000A_REG_SYSTEM, AL3000A_CONFIG_RESET);
>  	if (ret)
>  		return ret;
> @@ -157,10 +162,6 @@ static int al3000a_probe(struct i2c_client *client)
>  	if (ret)
>  		return dev_err_probe(dev, ret, "failed to init ALS\n");
>  
> -	ret = devm_add_action_or_reset(dev, al3000a_set_pwr_off, data);
> -	if (ret)
> -		return dev_err_probe(dev, ret, "failed to add action\n");
> -
>  	return devm_iio_device_register(dev, indio_dev);
>  }
>  
>