Re: [PATCH 13/13] tpm: st33zp24: Make st33zp24_remove() return void

[Jarkko Sakkinen <jarkko@xxxxxxxxxx>]

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@xxxxxxxxxxxxxx>
> ---
>  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