Re: [PATCH] [media] tua9001: Improve messages in .remove's error path

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Quoting Uwe Kleine-König (2021-10-26 20:40:10)
> If disabling the hardware fails the driver propagates the error code to
> the i2c core. However this only results in a generic error message; the
> device still disappears.
> 
> So instead emit a message that better describes the actual problem than
> the i2c core's "remove failed (ESOMETHING), will be ignored" and return
> 0 to suppress the generic message.

You almost caught me out. I was going to say, this changes the
behaviour of the return code. But It's intentional ;-)

It's still a bit concerning, /not/ returning an error on an error - but
as it's not going to prevent removal, and it won't add further (helpful)
diagnosticts), maybe it makes sense.

My only concern would be if it might be better to keep the return code,
so that the core frameworks know about the issue at least.

The error message is better IMO though so :

Reviewed-by: Kieran Bingham <kieran.bingham+renesas@xxxxxxxxxxxxxxxx>

> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx>
> ---
>  drivers/media/tuners/tua9001.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/drivers/media/tuners/tua9001.c b/drivers/media/tuners/tua9001.c
> index 5e3625e75620..af7d5ea1f77e 100644
> --- a/drivers/media/tuners/tua9001.c
> +++ b/drivers/media/tuners/tua9001.c
> @@ -240,14 +240,10 @@ static int tua9001_remove(struct i2c_client *client)
>                                    DVB_FRONTEND_COMPONENT_TUNER,
>                                    TUA9001_CMD_CEN, 0);
>                 if (ret)
> -                       goto err_kfree;
> +                       dev_err(&client->dev, "Tuner disable failed (%pe)\n", ERR_PTR(ret));
>         }
>         kfree(dev);
>         return 0;
> -err_kfree:
> -       kfree(dev);
> -       dev_dbg(&client->dev, "failed=%d\n", ret);
> -       return ret;
>  }
>  
>  static const struct i2c_device_id tua9001_id_table[] = {
> -- 
> 2.30.2
>




[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux