* [PATCH 13/13] tpm: st33zp24: Make st33zp24_remove() return void [not found] <20211011132754.2479853-1-u.kleine-koenig@pengutronix.de> @ 2021-10-11 13:27 ` Uwe Kleine-König 2021-10-12 16:47 ` Jarkko Sakkinen 0 siblings, 1 reply; 4+ messages in thread From: Uwe Kleine-König @ 2021-10-11 13:27 UTC (permalink / raw) To: Peter Huewe, Jarkko Sakkinen Cc: Mark Brown, linux-spi, Jason Gunthorpe, kernel, linux-integrity, Wolfram Sang, linux-i2c Up to now st33zp24_remove() returns zero unconditionally. Make it return void instead which makes it easier to see in the callers that there is no error to handle. Also the return value of i2c and spi remove callbacks is ignored anyway. Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> --- drivers/char/tpm/st33zp24/i2c.c | 5 +---- drivers/char/tpm/st33zp24/spi.c | 5 +---- drivers/char/tpm/st33zp24/st33zp24.c | 3 +-- drivers/char/tpm/st33zp24/st33zp24.h | 2 +- 4 files changed, 4 insertions(+), 11 deletions(-) diff --git a/drivers/char/tpm/st33zp24/i2c.c b/drivers/char/tpm/st33zp24/i2c.c index 7c617edff4ca..3170d59d660c 100644 --- a/drivers/char/tpm/st33zp24/i2c.c +++ b/drivers/char/tpm/st33zp24/i2c.c @@ -267,11 +267,8 @@ static int st33zp24_i2c_probe(struct i2c_client *client, static int st33zp24_i2c_remove(struct i2c_client *client) { struct tpm_chip *chip = i2c_get_clientdata(client); - int ret; - ret = st33zp24_remove(chip); - if (ret) - return ret; + st33zp24_remove(chip); return 0; } diff --git a/drivers/char/tpm/st33zp24/spi.c b/drivers/char/tpm/st33zp24/spi.c index a75dafd39445..ccd9e42b8eab 100644 --- a/drivers/char/tpm/st33zp24/spi.c +++ b/drivers/char/tpm/st33zp24/spi.c @@ -384,11 +384,8 @@ static int st33zp24_spi_probe(struct spi_device *dev) static int st33zp24_spi_remove(struct spi_device *dev) { struct tpm_chip *chip = spi_get_drvdata(dev); - int ret; - ret = st33zp24_remove(chip); - if (ret) - return ret; + st33zp24_remove(chip); return 0; } diff --git a/drivers/char/tpm/st33zp24/st33zp24.c b/drivers/char/tpm/st33zp24/st33zp24.c index 4ec10ab5e576..2b63654c38d6 100644 --- a/drivers/char/tpm/st33zp24/st33zp24.c +++ b/drivers/char/tpm/st33zp24/st33zp24.c @@ -588,10 +588,9 @@ EXPORT_SYMBOL(st33zp24_probe); * @param: tpm_data, the tpm phy. * @return: 0 in case of success. */ -int st33zp24_remove(struct tpm_chip *chip) +void st33zp24_remove(struct tpm_chip *chip) { tpm_chip_unregister(chip); - return 0; } EXPORT_SYMBOL(st33zp24_remove); diff --git a/drivers/char/tpm/st33zp24/st33zp24.h b/drivers/char/tpm/st33zp24/st33zp24.h index 6747be1e2502..b387a476c555 100644 --- a/drivers/char/tpm/st33zp24/st33zp24.h +++ b/drivers/char/tpm/st33zp24/st33zp24.h @@ -34,5 +34,5 @@ int st33zp24_pm_resume(struct device *dev); int st33zp24_probe(void *phy_id, const struct st33zp24_phy_ops *ops, struct device *dev, int irq, int io_lpcpd); -int st33zp24_remove(struct tpm_chip *chip); +void st33zp24_remove(struct tpm_chip *chip); #endif /* __LOCAL_ST33ZP24_H__ */ -- 2.30.2 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 13/13] tpm: st33zp24: Make st33zp24_remove() return void 2021-10-11 13:27 ` [PATCH 13/13] tpm: st33zp24: Make st33zp24_remove() return void Uwe Kleine-König @ 2021-10-12 16:47 ` Jarkko Sakkinen 2021-10-12 17:40 ` Uwe Kleine-König 0 siblings, 1 reply; 4+ messages in thread From: Jarkko Sakkinen @ 2021-10-12 16:47 UTC (permalink / raw) To: Uwe Kleine-König, Peter Huewe Cc: Mark Brown, linux-spi, Jason Gunthorpe, kernel, linux-integrity, Wolfram Sang, linux-i2c On Mon, 2021-10-11 at 15:27 +0200, Uwe Kleine-König wrote: > Up to now st33zp24_remove() returns zero unconditionally. Make it return > void instead which makes it easier to see in the callers that there is > no error to handle. So, void is not a return value. > > Also the return value of i2c and spi remove callbacks is ignored anyway. ~~~ ~~~ I2C SPI > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > --- > drivers/char/tpm/st33zp24/i2c.c | 5 +---- > drivers/char/tpm/st33zp24/spi.c | 5 +---- > drivers/char/tpm/st33zp24/st33zp24.c | 3 +-- > drivers/char/tpm/st33zp24/st33zp24.h | 2 +- > 4 files changed, 4 insertions(+), 11 deletions(-) > > diff --git a/drivers/char/tpm/st33zp24/i2c.c b/drivers/char/tpm/st33zp24/i2c.c > index 7c617edff4ca..3170d59d660c 100644 > --- a/drivers/char/tpm/st33zp24/i2c.c > +++ b/drivers/char/tpm/st33zp24/i2c.c > @@ -267,11 +267,8 @@ static int st33zp24_i2c_probe(struct i2c_client *client, > static int st33zp24_i2c_remove(struct i2c_client *client) > { > struct tpm_chip *chip = i2c_get_clientdata(client); > - int ret; > > - ret = st33zp24_remove(chip); > - if (ret) > - return ret; > + st33zp24_remove(chip); > > return 0; > } > diff --git a/drivers/char/tpm/st33zp24/spi.c b/drivers/char/tpm/st33zp24/spi.c > index a75dafd39445..ccd9e42b8eab 100644 > --- a/drivers/char/tpm/st33zp24/spi.c > +++ b/drivers/char/tpm/st33zp24/spi.c > @@ -384,11 +384,8 @@ static int st33zp24_spi_probe(struct spi_device *dev) > static int st33zp24_spi_remove(struct spi_device *dev) > { > struct tpm_chip *chip = spi_get_drvdata(dev); > - int ret; > > - ret = st33zp24_remove(chip); > - if (ret) > - return ret; > + st33zp24_remove(chip); > > return 0; > } > diff --git a/drivers/char/tpm/st33zp24/st33zp24.c b/drivers/char/tpm/st33zp24/st33zp24.c > index 4ec10ab5e576..2b63654c38d6 100644 > --- a/drivers/char/tpm/st33zp24/st33zp24.c > +++ b/drivers/char/tpm/st33zp24/st33zp24.c > @@ -588,10 +588,9 @@ EXPORT_SYMBOL(st33zp24_probe); > * @param: tpm_data, the tpm phy. > * @return: 0 in case of success. > */ > -int st33zp24_remove(struct tpm_chip *chip) > +void st33zp24_remove(struct tpm_chip *chip) > { > tpm_chip_unregister(chip); > - return 0; > } > EXPORT_SYMBOL(st33zp24_remove); > > diff --git a/drivers/char/tpm/st33zp24/st33zp24.h b/drivers/char/tpm/st33zp24/st33zp24.h > index 6747be1e2502..b387a476c555 100644 > --- a/drivers/char/tpm/st33zp24/st33zp24.h > +++ b/drivers/char/tpm/st33zp24/st33zp24.h > @@ -34,5 +34,5 @@ int st33zp24_pm_resume(struct device *dev); > > int st33zp24_probe(void *phy_id, const struct st33zp24_phy_ops *ops, > struct device *dev, int irq, int io_lpcpd); > -int st33zp24_remove(struct tpm_chip *chip); > +void st33zp24_remove(struct tpm_chip *chip); > #endif /* __LOCAL_ST33ZP24_H__ */ Even though this does not improve things at run-time, this does help to understand what the code does, so in that sense I do think that this a sane change to make. With an appropriate commit message, this can be applied. /Jarkko ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 13/13] tpm: st33zp24: Make st33zp24_remove() return void 2021-10-12 16:47 ` Jarkko Sakkinen @ 2021-10-12 17:40 ` Uwe Kleine-König 2021-10-12 17:46 ` Jarkko Sakkinen 0 siblings, 1 reply; 4+ messages in thread From: Uwe Kleine-König @ 2021-10-12 17:40 UTC (permalink / raw) To: Jarkko Sakkinen Cc: Peter Huewe, linux-spi, Wolfram Sang, Jason Gunthorpe, Mark Brown, linux-i2c, kernel, linux-integrity [-- Attachment #1: Type: text/plain, Size: 4249 bytes --] On Tue, Oct 12, 2021 at 07:47:22PM +0300, Jarkko Sakkinen wrote: > On Mon, 2021-10-11 at 15:27 +0200, Uwe Kleine-König wrote: > > Up to now st33zp24_remove() returns zero unconditionally. Make it return > > void instead which makes it easier to see in the callers that there is > > no error to handle. > > So, void is not a return value. Hmm, would you be more happy with "Make it return no value"? I think this is less understandable than "Make it return void". Do you have a constructive suggestion how to formulate. > > Also the return value of i2c and spi remove callbacks is ignored anyway. > ~~~ ~~~ > I2C SPI I don't agree. "I2C" is fine if you mean the protocol. For the framework names I consider the small letters better, as it matches the directory name below drivers/ and also matches the function name prefix and what is usually used for short log prefixes. Ditto for spi. > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > > --- > > drivers/char/tpm/st33zp24/i2c.c | 5 +---- > > drivers/char/tpm/st33zp24/spi.c | 5 +---- > > drivers/char/tpm/st33zp24/st33zp24.c | 3 +-- > > drivers/char/tpm/st33zp24/st33zp24.h | 2 +- > > 4 files changed, 4 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/char/tpm/st33zp24/i2c.c b/drivers/char/tpm/st33zp24/i2c.c > > index 7c617edff4ca..3170d59d660c 100644 > > --- a/drivers/char/tpm/st33zp24/i2c.c > > +++ b/drivers/char/tpm/st33zp24/i2c.c > > @@ -267,11 +267,8 @@ static int st33zp24_i2c_probe(struct i2c_client *client, > > static int st33zp24_i2c_remove(struct i2c_client *client) > > { > > struct tpm_chip *chip = i2c_get_clientdata(client); > > - int ret; > > > > - ret = st33zp24_remove(chip); > > - if (ret) > > - return ret; > > + st33zp24_remove(chip); > > > > return 0; > > } > > diff --git a/drivers/char/tpm/st33zp24/spi.c b/drivers/char/tpm/st33zp24/spi.c > > index a75dafd39445..ccd9e42b8eab 100644 > > --- a/drivers/char/tpm/st33zp24/spi.c > > +++ b/drivers/char/tpm/st33zp24/spi.c > > @@ -384,11 +384,8 @@ static int st33zp24_spi_probe(struct spi_device *dev) > > static int st33zp24_spi_remove(struct spi_device *dev) > > { > > struct tpm_chip *chip = spi_get_drvdata(dev); > > - int ret; > > > > - ret = st33zp24_remove(chip); > > - if (ret) > > - return ret; > > + st33zp24_remove(chip); > > > > return 0; > > } > > diff --git a/drivers/char/tpm/st33zp24/st33zp24.c b/drivers/char/tpm/st33zp24/st33zp24.c > > index 4ec10ab5e576..2b63654c38d6 100644 > > --- a/drivers/char/tpm/st33zp24/st33zp24.c > > +++ b/drivers/char/tpm/st33zp24/st33zp24.c > > @@ -588,10 +588,9 @@ EXPORT_SYMBOL(st33zp24_probe); > > * @param: tpm_data, the tpm phy. > > * @return: 0 in case of success. > > */ > > -int st33zp24_remove(struct tpm_chip *chip) > > +void st33zp24_remove(struct tpm_chip *chip) > > { > > tpm_chip_unregister(chip); > > - return 0; > > } > > EXPORT_SYMBOL(st33zp24_remove); > > > > diff --git a/drivers/char/tpm/st33zp24/st33zp24.h b/drivers/char/tpm/st33zp24/st33zp24.h > > index 6747be1e2502..b387a476c555 100644 > > --- a/drivers/char/tpm/st33zp24/st33zp24.h > > +++ b/drivers/char/tpm/st33zp24/st33zp24.h > > @@ -34,5 +34,5 @@ int st33zp24_pm_resume(struct device *dev); > > > > int st33zp24_probe(void *phy_id, const struct st33zp24_phy_ops *ops, > > struct device *dev, int irq, int io_lpcpd); > > -int st33zp24_remove(struct tpm_chip *chip); > > +void st33zp24_remove(struct tpm_chip *chip); > > #endif /* __LOCAL_ST33ZP24_H__ */ > > Even though this does not improve things at run-time, this does > help to understand what the code does, That is for focus for now. As written in the cover letter the long term goal is to make struct spi_driver::remove return void, too. > so in that sense I do > think that this a sane change to make. Thanks Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ | [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 13/13] tpm: st33zp24: Make st33zp24_remove() return void 2021-10-12 17:40 ` Uwe Kleine-König @ 2021-10-12 17:46 ` Jarkko Sakkinen 0 siblings, 0 replies; 4+ messages in thread From: Jarkko Sakkinen @ 2021-10-12 17:46 UTC (permalink / raw) To: Uwe Kleine-König Cc: Peter Huewe, linux-spi, Wolfram Sang, Jason Gunthorpe, Mark Brown, linux-i2c, kernel, linux-integrity On Tue, 2021-10-12 at 19:40 +0200, Uwe Kleine-König wrote: > On Tue, Oct 12, 2021 at 07:47:22PM +0300, Jarkko Sakkinen wrote: > > On Mon, 2021-10-11 at 15:27 +0200, Uwe Kleine-König wrote: > > > Up to now st33zp24_remove() returns zero unconditionally. Make it return > > > void instead which makes it easier to see in the callers that there is > > > no error to handle. > > > > So, void is not a return value. > > Hmm, would you be more happy with "Make it return no value"? I think > this is less understandable than "Make it return void". Do you have a > constructive suggestion how to formulate. I think it is semantically more correct to say that it does not return any value, given that it does not do that :-) You can just state e.g. "Make st33zp24_remove() a void function.", it is semantically sound and not really that confusing. /Jarkko ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-10-12 17:46 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20211011132754.2479853-1-u.kleine-koenig@pengutronix.de>
2021-10-11 13:27 ` [PATCH 13/13] tpm: st33zp24: Make st33zp24_remove() return void Uwe Kleine-König
2021-10-12 16:47 ` Jarkko Sakkinen
2021-10-12 17:40 ` Uwe Kleine-König
2021-10-12 17:46 ` Jarkko Sakkinen
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox