Re: [PATCH v1] drivers: adt7470: check the return value of adt7470_read_temperatures

[Guenter Roeck <linux@xxxxxxxxxxxx>]

On 9/21/22 15:27, Li Zhong wrote:
> adt7470_read_temperatures() fails and returns error when operations on
> regmap fail. adt7470_update_thread() currently does not check for it and
> propagate the error.
>
> Signed-off-by: Li Zhong <floridsleeves@xxxxxxxxx>
> ---
>   drivers/hwmon/adt7470.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hwmon/adt7470.c b/drivers/hwmon/adt7470.c
> index c67cd037a93f..0aadb2dc067f 100644
> --- a/drivers/hwmon/adt7470.c
> +++ b/drivers/hwmon/adt7470.c
> @@ -289,12 +289,16 @@ static int adt7470_update_thread(void *p)
>   {
>   	struct i2c_client *client = p;
>   	struct adt7470_data *data = i2c_get_clientdata(client);
> +	int err;
>   
>   	while (!kthread_should_stop()) {
>   		mutex_lock(&data->lock);
> -		adt7470_read_temperatures(data);
> +		err = adt7470_read_temperatures(data);
>   		mutex_unlock(&data->lock);
>   
> +		if (err)
> +			return err;
> +
>   		if (kthread_should_stop())
>   			break;
>   
NACK.

That is a kernel thread which should keep going even after an error.
Aborting the thread after an error would be the absolute wrong thing
to do.

Guenter