On Sat, 2021-11-13 at 22:53 +0100, Uwe Kleine-König wrote:
> Hello,
>
> On Sat, Nov 13, 2021 at 12:53:32PM +0200, Jarkko Sakkinen wrote:
> > On Fri, 2021-11-12 at 23:53 +0100, Uwe Kleine-König wrote:
> > > tpm_cr50_i2c_remove() is only called after tpm_cr50_i2c_probe() returned
> > > successfully. As i2c_get_clientdata() returns driver data for the
> > > client's device and this was set in tpmm_chip_alloc() it won't return
> > > NULL.
> >
> > This does not make the check obsolete, e.g. it would catch a programming
> > error elsewhere.
> >
> > > Simplify accordingly to prepare changing the prototype of the i2c remove
> > > callback to return void. Notice that already today returning an error
> > > code from the remove callback doesn't prevent removal.
> >
> > I don't understand what you are trying to say.
>
> The eventual goal is the following change:
>
> diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> index 16119ac1aa97..c7069ebf5a66 100644
> --- a/include/linux/i2c.h
> +++ b/include/linux/i2c.h
> @@ -273,7 +273,7 @@ struct i2c_driver {
>
> /* Standard driver model interfaces */
> int (*probe)(struct i2c_client *client, const struct i2c_device_id *id);
> - int (*remove)(struct i2c_client *client);
> + void (*remove)(struct i2c_client *client);
>
> /* New driver model interface to aid the seamless removal of the
> * current probe()'s, more commonly unused than used second parameter.
>
> To prepare that I want to change all remove callbacks to unconditionally
> return 0.
>
> The motivation for the above change is that returning an error from an
> i2c (or spi or platform) remove callback doesn't prevent the device from
> being removed. So the ability to return an int leads to wrong
> expectations by driver authors.
>
> The only effect a non-zero return code has, is an error message from the
> i2c core. So if you object to my suggested change, the minimal change I
> want to convince you of is to replace
>
> return -ENODEV;
>
> by
>
> return 0;
>
> .
>
> Best regards
> Uwe
Please then include it to a patch set, where this happens.
/Jarkko
