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
> > diff --git a/drivers/char/tpm/tpm_tis_i2c_cr50.c b/drivers/char/tpm/tpm_tis_i2c_cr50.c
> > index c89278103703..622cdf622ddc 100644
> > --- a/drivers/char/tpm/tpm_tis_i2c_cr50.c
> > +++ b/drivers/char/tpm/tpm_tis_i2c_cr50.c
> > @@ -751,12 +751,6 @@ static int tpm_cr50_i2c_probe(struct i2c_client *client)
> > static int tpm_cr50_i2c_remove(struct i2c_client *client)
> > {
> > struct tpm_chip *chip = i2c_get_clientdata(client);
> > - struct device *dev = &client->dev;
> > -
> > - if (!chip) {
> > - dev_err(dev, "Could not get client data at remove\n");
> > - return -ENODEV;
> > - }
> >
> > tpm_chip_unregister(chip);
> > tpm_cr50_release_locality(chip, true);
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
Attachment:
signature.asc
Description: PGP signature
