mbox series

[00/89] i2c: Convert to platform remove callback returning void

Message ID 20230508205306.1474415-1-u.kleine-koenig@pengutronix.de
Headers show
Series i2c: Convert to platform remove callback returning void | expand

Message

Uwe Kleine-König May 8, 2023, 8:51 p.m. UTC
Hello,

this series convers the drivers below drivers/i2c to the .remove_new()
callback of struct platform_driver(). The motivation is to make the
remove callback less prone for errors and wrong assumptions. See commit
5c5a7680e67b ("platform: Provide a remove callback that returns no
value") for a more detailed rationale.

All but one driver already returned zero unconditionally in their
.remove() callback, so converting them to .remove_new() is trivial.
i2c-davinci has two patches in this series, first the error path is
improved to not return an error code, then it's converted as the others
drivers are.

The two davinci patches are also the only interdependency in this
series. I was unsure if I should split the series in two, the busses and
the mux changes; if convenient these can be applied independent of each
other.

Best regards
Uwe

Uwe Kleine-König (89):
  i2c: altera: Convert to platform remove callback returning void
  i2c: amd-mp2-plat: Convert to platform remove callback returning void
  i2c: aspeed: Convert to platform remove callback returning void
  i2c: at91-core: Convert to platform remove callback returning void
  i2c: au1550: Convert to platform remove callback returning void
  i2c: axxia: Convert to platform remove callback returning void
  i2c: bcm-iproc: Convert to platform remove callback returning void
  i2c: bcm-kona: Convert to platform remove callback returning void
  i2c: bcm2835: Convert to platform remove callback returning void
  i2c: brcmstb: Convert to platform remove callback returning void
  i2c: cadence: Convert to platform remove callback returning void
  i2c: cbus-gpio: Convert to platform remove callback returning void
  i2c: cht-wc: Convert to platform remove callback returning void
  i2c: cpm: Convert to platform remove callback returning void
  i2c: cros-ec-tunnel: Convert to platform remove callback returning
    void
  i2c: davinci: Improve error reporting for problems during .remove()
  i2c: davinci: Convert to platform remove callback returning void
  i2c: designware-platdrv: Convert to platform remove callback returning
    void
  i2c: digicolor: Convert to platform remove callback returning void
  i2c: dln2: Convert to platform remove callback returning void
  i2c: emev2: Convert to platform remove callback returning void
  i2c: exynos5: Convert to platform remove callback returning void
  i2c: gpio: Convert to platform remove callback returning void
  i2c: gxp: Convert to platform remove callback returning void
  i2c: highlander: Convert to platform remove callback returning void
  i2c: hix5hd2: Convert to platform remove callback returning void
  i2c: ibm_iic: Convert to platform remove callback returning void
  i2c: img-scb: Convert to platform remove callback returning void
  i2c: imx-lpi2c: Convert to platform remove callback returning void
  i2c: imx: Convert to platform remove callback returning void
  i2c: iop3xx: Convert to platform remove callback returning void
  i2c: isch: Convert to platform remove callback returning void
  i2c: jz4780: Convert to platform remove callback returning void
  i2c: kempld: Convert to platform remove callback returning void
  i2c: lpc2k: Convert to platform remove callback returning void
  i2c: meson: Convert to platform remove callback returning void
  i2c: microchip-corei2c: Convert to platform remove callback returning
    void
  i2c: mlxbf: Convert to platform remove callback returning void
  i2c: mlxcpld: Convert to platform remove callback returning void
  i2c: mpc: Convert to platform remove callback returning void
  i2c: mt65xx: Convert to platform remove callback returning void
  i2c: mt7621: Convert to platform remove callback returning void
  i2c: mv64xxx: Convert to platform remove callback returning void
  i2c: mxs: Convert to platform remove callback returning void
  i2c: npcm7xx: Convert to platform remove callback returning void
  i2c: ocores: Convert to platform remove callback returning void
  i2c: octeon-platdrv: Convert to platform remove callback returning
    void
  i2c: omap: Convert to platform remove callback returning void
  i2c: opal: Convert to platform remove callback returning void
  i2c: pasemi-platform: Convert to platform remove callback returning
    void
  i2c: pca-platform: Convert to platform remove callback returning void
  i2c: pnx: Convert to platform remove callback returning void
  i2c: powermac: Convert to platform remove callback returning void
  i2c: pxa: Convert to platform remove callback returning void
  i2c: qcom-cci: Convert to platform remove callback returning void
  i2c: qcom-geni: Convert to platform remove callback returning void
  i2c: qup: Convert to platform remove callback returning void
  i2c: rcar: Convert to platform remove callback returning void
  i2c: riic: Convert to platform remove callback returning void
  i2c: rk3x: Convert to platform remove callback returning void
  i2c: rzv2m: Convert to platform remove callback returning void
  i2c: s3c2410: Convert to platform remove callback returning void
  i2c: scmi: Convert to platform remove callback returning void
  i2c: scx200_acb: Convert to platform remove callback returning void
  i2c: sh7760: Convert to platform remove callback returning void
  i2c: sh_mobile: Convert to platform remove callback returning void
  i2c: simtec: Convert to platform remove callback returning void
  i2c: st: Convert to platform remove callback returning void
  i2c: stm32f4: Convert to platform remove callback returning void
  i2c: stm32f7: Convert to platform remove callback returning void
  i2c: sun6i-p2wi: Convert to platform remove callback returning void
  i2c: synquacer: Convert to platform remove callback returning void
  i2c: tegra-bpmp: Convert to platform remove callback returning void
  i2c: tegra: Convert to platform remove callback returning void
  i2c: uniphier-f: Convert to platform remove callback returning void
  i2c: uniphier: Convert to platform remove callback returning void
  i2c: versatile: Convert to platform remove callback returning void
  i2c: viperboard: Convert to platform remove callback returning void
  i2c: wmt: Convert to platform remove callback returning void
  i2c: xgene-slimpro: Convert to platform remove callback returning void
  i2c: xiic: Convert to platform remove callback returning void
  i2c: xlp9xx: Convert to platform remove callback returning void
  i2c: mux: arb-gpio-challenge: Convert to platform remove callback
    returning void
  i2c: mux: demux-pinctrl: Convert to platform remove callback returning
    void
  i2c: mux: gpio: Convert to platform remove callback returning void
  i2c: mux: gpmux: Convert to platform remove callback returning void
  i2c: mux: mlxcpld: Convert to platform remove callback returning void
  i2c: mux: pinctrl: Convert to platform remove callback returning void
  i2c: mux: reg: Convert to platform remove callback returning void

 drivers/i2c/busses/i2c-altera.c             |  6 ++----
 drivers/i2c/busses/i2c-amd-mp2-plat.c       |  5 ++---
 drivers/i2c/busses/i2c-aspeed.c             |  6 ++----
 drivers/i2c/busses/i2c-at91-core.c          |  6 ++----
 drivers/i2c/busses/i2c-au1550.c             |  5 ++---
 drivers/i2c/busses/i2c-axxia.c              |  6 ++----
 drivers/i2c/busses/i2c-bcm-iproc.c          |  6 ++----
 drivers/i2c/busses/i2c-bcm-kona.c           |  6 ++----
 drivers/i2c/busses/i2c-bcm2835.c            |  6 ++----
 drivers/i2c/busses/i2c-brcmstb.c            |  5 ++---
 drivers/i2c/busses/i2c-cadence.c            |  6 ++----
 drivers/i2c/busses/i2c-cbus-gpio.c          |  6 ++----
 drivers/i2c/busses/i2c-cht-wc.c             |  6 ++----
 drivers/i2c/busses/i2c-cpm.c                |  6 ++----
 drivers/i2c/busses/i2c-cros-ec-tunnel.c     |  6 ++----
 drivers/i2c/busses/i2c-davinci.c            | 14 ++++++--------
 drivers/i2c/busses/i2c-designware-platdrv.c |  6 ++----
 drivers/i2c/busses/i2c-digicolor.c          |  6 ++----
 drivers/i2c/busses/i2c-dln2.c               |  6 ++----
 drivers/i2c/busses/i2c-emev2.c              |  6 ++----
 drivers/i2c/busses/i2c-exynos5.c            |  6 ++----
 drivers/i2c/busses/i2c-gpio.c               |  6 ++----
 drivers/i2c/busses/i2c-gxp.c                |  6 ++----
 drivers/i2c/busses/i2c-highlander.c         |  6 ++----
 drivers/i2c/busses/i2c-hix5hd2.c            |  6 ++----
 drivers/i2c/busses/i2c-ibm_iic.c            |  6 ++----
 drivers/i2c/busses/i2c-img-scb.c            |  6 ++----
 drivers/i2c/busses/i2c-imx-lpi2c.c          |  6 ++----
 drivers/i2c/busses/i2c-imx.c                |  6 ++----
 drivers/i2c/busses/i2c-iop3xx.c             |  6 ++----
 drivers/i2c/busses/i2c-isch.c               |  6 ++----
 drivers/i2c/busses/i2c-jz4780.c             |  5 ++---
 drivers/i2c/busses/i2c-kempld.c             |  6 ++----
 drivers/i2c/busses/i2c-lpc2k.c              |  6 ++----
 drivers/i2c/busses/i2c-meson.c              |  6 ++----
 drivers/i2c/busses/i2c-microchip-corei2c.c  |  6 ++----
 drivers/i2c/busses/i2c-mlxbf.c              |  6 ++----
 drivers/i2c/busses/i2c-mlxcpld.c            |  6 ++----
 drivers/i2c/busses/i2c-mpc.c                |  6 ++----
 drivers/i2c/busses/i2c-mt65xx.c             |  6 ++----
 drivers/i2c/busses/i2c-mt7621.c             |  6 ++----
 drivers/i2c/busses/i2c-mv64xxx.c            |  6 ++----
 drivers/i2c/busses/i2c-mxs.c                |  6 ++----
 drivers/i2c/busses/i2c-npcm7xx.c            |  5 ++---
 drivers/i2c/busses/i2c-ocores.c             |  6 ++----
 drivers/i2c/busses/i2c-octeon-platdrv.c     |  5 ++---
 drivers/i2c/busses/i2c-omap.c               |  6 ++----
 drivers/i2c/busses/i2c-opal.c               |  6 ++----
 drivers/i2c/busses/i2c-pasemi-platform.c    |  5 ++---
 drivers/i2c/busses/i2c-pca-platform.c       |  6 ++----
 drivers/i2c/busses/i2c-pnx.c                |  6 ++----
 drivers/i2c/busses/i2c-powermac.c           |  6 ++----
 drivers/i2c/busses/i2c-pxa.c                |  6 ++----
 drivers/i2c/busses/i2c-qcom-cci.c           |  6 ++----
 drivers/i2c/busses/i2c-qcom-geni.c          |  5 ++---
 drivers/i2c/busses/i2c-qup.c                |  5 ++---
 drivers/i2c/busses/i2c-rcar.c               |  6 ++----
 drivers/i2c/busses/i2c-riic.c               |  6 ++----
 drivers/i2c/busses/i2c-rk3x.c               |  6 ++----
 drivers/i2c/busses/i2c-rzv2m.c              |  6 ++----
 drivers/i2c/busses/i2c-s3c2410.c            |  6 ++----
 drivers/i2c/busses/i2c-scmi.c               |  6 ++----
 drivers/i2c/busses/i2c-sh7760.c             |  6 ++----
 drivers/i2c/busses/i2c-sh_mobile.c          |  5 ++---
 drivers/i2c/busses/i2c-simtec.c             |  6 ++----
 drivers/i2c/busses/i2c-st.c                 |  6 ++----
 drivers/i2c/busses/i2c-stm32f4.c            |  6 ++----
 drivers/i2c/busses/i2c-stm32f7.c            |  6 ++----
 drivers/i2c/busses/i2c-sun6i-p2wi.c         |  6 ++----
 drivers/i2c/busses/i2c-synquacer.c          |  6 ++----
 drivers/i2c/busses/i2c-tegra-bpmp.c         |  6 ++----
 drivers/i2c/busses/i2c-tegra.c              |  6 ++----
 drivers/i2c/busses/i2c-uniphier-f.c         |  6 ++----
 drivers/i2c/busses/i2c-uniphier.c           |  6 ++----
 drivers/i2c/busses/i2c-versatile.c          |  5 ++---
 drivers/i2c/busses/i2c-viperboard.c         |  6 ++----
 drivers/i2c/busses/i2c-wmt.c                |  6 ++----
 drivers/i2c/busses/i2c-xgene-slimpro.c      |  6 ++----
 drivers/i2c/busses/i2c-xiic.c               |  6 ++----
 drivers/i2c/busses/i2c-xlp9xx.c             |  6 ++----
 drivers/i2c/busses/scx200_acb.c             |  6 ++----
 drivers/i2c/muxes/i2c-arb-gpio-challenge.c  |  5 ++---
 drivers/i2c/muxes/i2c-demux-pinctrl.c       |  6 ++----
 drivers/i2c/muxes/i2c-mux-gpio.c            |  6 ++----
 drivers/i2c/muxes/i2c-mux-gpmux.c           |  6 ++----
 drivers/i2c/muxes/i2c-mux-mlxcpld.c         |  5 ++---
 drivers/i2c/muxes/i2c-mux-pinctrl.c         |  6 ++----
 drivers/i2c/muxes/i2c-mux-reg.c             |  6 ++----
 88 files changed, 180 insertions(+), 343 deletions(-)

base-commit: ac9a78681b921877518763ba0e89202254349d1b

Comments

Chris Pringle May 9, 2023, 7:52 a.m. UTC | #1
On Monday, May 8, 2023 9:52 PM Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote: 
> The .remove() callback for a platform driver returns an int which makes many
> driver authors wrongly assume it's possible to do error handling by returning
> an error code. However the value returned is (mostly) ignored and this
> typically results in resource leaks. To improve here there is a quest to make the
> remove callback return void. In the first step of this quest all drivers are
> converted to .remove_new() which already returns void.
> 
> Trivially convert this driver from always returning zero in the remove callback
> to the void returning variant.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Reviewed-by: Chris Pringle <chris.pringle@phabrix.com>

Thanks,
Chris
Konrad Dybcio May 9, 2023, 8:47 a.m. UTC | #2
On 8.05.2023 22:52, Uwe Kleine-König wrote:
> The .remove() callback for a platform driver returns an int which makes
> many driver authors wrongly assume it's possible to do error handling by
> returning an error code. However the value returned is (mostly) ignored
> and this typically results in resource leaks. To improve here there is a
> quest to make the remove callback return void. In the first step of this
> quest all drivers are converted to .remove_new() which already returns
> void.
> 
> Trivially convert this driver from always returning zero in the remove
> callback to the void returning variant.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>

Konrad
>  drivers/i2c/busses/i2c-qup.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c
> index 2e153f2f71b6..6eef1dbd00de 100644
> --- a/drivers/i2c/busses/i2c-qup.c
> +++ b/drivers/i2c/busses/i2c-qup.c
> @@ -1904,7 +1904,7 @@ static int qup_i2c_probe(struct platform_device *pdev)
>  	return ret;
>  }
>  
> -static int qup_i2c_remove(struct platform_device *pdev)
> +static void qup_i2c_remove(struct platform_device *pdev)
>  {
>  	struct qup_i2c_dev *qup = platform_get_drvdata(pdev);
>  
> @@ -1918,7 +1918,6 @@ static int qup_i2c_remove(struct platform_device *pdev)
>  	i2c_del_adapter(&qup->adap);
>  	pm_runtime_disable(qup->dev);
>  	pm_runtime_set_suspended(qup->dev);
> -	return 0;
>  }
>  
>  #ifdef CONFIG_PM
> @@ -1978,7 +1977,7 @@ MODULE_DEVICE_TABLE(of, qup_i2c_dt_match);
>  
>  static struct platform_driver qup_i2c_driver = {
>  	.probe  = qup_i2c_probe,
> -	.remove = qup_i2c_remove,
> +	.remove_new = qup_i2c_remove,
>  	.driver = {
>  		.name = "i2c_qup",
>  		.pm = &qup_i2c_qup_pm_ops,
Krzysztof Kozlowski May 9, 2023, 3:09 p.m. UTC | #3
On 08/05/2023 22:51, Uwe Kleine-König wrote:
> The .remove() callback for a platform driver returns an int which makes
> many driver authors wrongly assume it's possible to do error handling by
> returning an error code. However the value returned is (mostly) ignored
> and this typically results in resource leaks. To improve here there is a
> quest to make the remove callback return void. In the first step of this
> quest all drivers are converted to .remove_new() which already returns
> void.
> 
> Trivially convert this driver from always returning zero in the remove
> callback to the void returning variant.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof
Matthias Brugger May 10, 2023, 4:55 p.m. UTC | #4
On 08/05/2023 22:52, Uwe Kleine-König wrote:
> The .remove() callback for a platform driver returns an int which makes
> many driver authors wrongly assume it's possible to do error handling by
> returning an error code. However the value returned is (mostly) ignored
> and this typically results in resource leaks. To improve here there is a
> quest to make the remove callback return void. In the first step of this
> quest all drivers are converted to .remove_new() which already returns
> void.
> 
> Trivially convert this driver from always returning zero in the remove
> callback to the void returning variant.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Reviewed-by: Matthias Brugger <matthias.bgg@gmail.com>

> ---
>   drivers/i2c/busses/i2c-mt65xx.c | 6 ++----
>   1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-mt65xx.c b/drivers/i2c/busses/i2c-mt65xx.c
> index a43c4d77739a..7ca3f2221ba6 100644
> --- a/drivers/i2c/busses/i2c-mt65xx.c
> +++ b/drivers/i2c/busses/i2c-mt65xx.c
> @@ -1505,15 +1505,13 @@ static int mtk_i2c_probe(struct platform_device *pdev)
>   	return ret;
>   }
>   
> -static int mtk_i2c_remove(struct platform_device *pdev)
> +static void mtk_i2c_remove(struct platform_device *pdev)
>   {
>   	struct mtk_i2c *i2c = platform_get_drvdata(pdev);
>   
>   	i2c_del_adapter(&i2c->adap);
>   
>   	clk_bulk_unprepare(I2C_MT65XX_CLK_MAX, i2c->clocks);
> -
> -	return 0;
>   }
>   
>   #ifdef CONFIG_PM_SLEEP
> @@ -1555,7 +1553,7 @@ static const struct dev_pm_ops mtk_i2c_pm = {
>   
>   static struct platform_driver mtk_i2c_driver = {
>   	.probe = mtk_i2c_probe,
> -	.remove = mtk_i2c_remove,
> +	.remove_new = mtk_i2c_remove,
>   	.driver = {
>   		.name = I2C_DRV_NAME,
>   		.pm = &mtk_i2c_pm,
Matthias Brugger May 10, 2023, 4:56 p.m. UTC | #5
On 08/05/2023 22:52, Uwe Kleine-König wrote:
> The .remove() callback for a platform driver returns an int which makes
> many driver authors wrongly assume it's possible to do error handling by
> returning an error code. However the value returned is (mostly) ignored
> and this typically results in resource leaks. To improve here there is a
> quest to make the remove callback return void. In the first step of this
> quest all drivers are converted to .remove_new() which already returns
> void.
> 
> Trivially convert this driver from always returning zero in the remove
> callback to the void returning variant.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Reviewed-by: Matthias Brugger <matthias.bgg@gmail.com>

> ---
>   drivers/i2c/busses/i2c-mt7621.c | 6 ++----
>   1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-mt7621.c b/drivers/i2c/busses/i2c-mt7621.c
> index 20eda5738ac4..f9c294e2bd3c 100644
> --- a/drivers/i2c/busses/i2c-mt7621.c
> +++ b/drivers/i2c/busses/i2c-mt7621.c
> @@ -332,19 +332,17 @@ static int mtk_i2c_probe(struct platform_device *pdev)
>   	return ret;
>   }
>   
> -static int mtk_i2c_remove(struct platform_device *pdev)
> +static void mtk_i2c_remove(struct platform_device *pdev)
>   {
>   	struct mtk_i2c *i2c = platform_get_drvdata(pdev);
>   
>   	clk_disable_unprepare(i2c->clk);
>   	i2c_del_adapter(&i2c->adap);
> -
> -	return 0;
>   }
>   
>   static struct platform_driver mtk_i2c_driver = {
>   	.probe		= mtk_i2c_probe,
> -	.remove		= mtk_i2c_remove,
> +	.remove_new	= mtk_i2c_remove,
>   	.driver		= {
>   		.name	= "i2c-mt7621",
>   		.of_match_table = i2c_mtk_dt_ids,
Jernej Škrabec May 10, 2023, 7:03 p.m. UTC | #6
Dne ponedeljek, 08. maj 2023 ob 22:52:48 CEST je Uwe Kleine-König napisal(a):
> The .remove() callback for a platform driver returns an int which makes
> many driver authors wrongly assume it's possible to do error handling by
> returning an error code. However the value returned is (mostly) ignored
> and this typically results in resource leaks. To improve here there is a
> quest to make the remove callback return void. In the first step of this
> quest all drivers are converted to .remove_new() which already returns
> void.
> 
> Trivially convert this driver from always returning zero in the remove
> callback to the void returning variant.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Acked-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Best regards,
Jernej
Bartosz Golaszewski May 11, 2023, 9:02 a.m. UTC | #7
On Mon, May 8, 2023 at 10:53 PM Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
>
> If pm_runtime_get() fails in .remove() the driver used to return the
> error to the driver core. The only effect of this (compared to returning
> zero) is a generic warning that the error value is ignored. (The device
> is unbound then anyhow.)
>
> Emit a better warning and return zero to suppress the generic (and
> little helpful) message. Also disable runtime PM in the error case.
>
> This prepares changing platform device remove callbacks to return void.
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
>  drivers/i2c/busses/i2c-davinci.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.c
> index 9750310f2c96..7ba7e6cbcc97 100644
> --- a/drivers/i2c/busses/i2c-davinci.c
> +++ b/drivers/i2c/busses/i2c-davinci.c
> @@ -894,11 +894,11 @@ static int davinci_i2c_remove(struct platform_device *pdev)
>
>         i2c_del_adapter(&dev->adapter);
>
> -       ret = pm_runtime_resume_and_get(&pdev->dev);
> +       ret = pm_runtime_get_sync(&pdev->dev);
>         if (ret < 0)
> -               return ret;
> -
> -       davinci_i2c_write_reg(dev, DAVINCI_I2C_MDR_REG, 0);
> +               dev_err(&pdev->dev, "Failed to resume device\n");
> +       else
> +               davinci_i2c_write_reg(dev, DAVINCI_I2C_MDR_REG, 0);
>
>         pm_runtime_dont_use_autosuspend(dev->dev);
>         pm_runtime_put_sync(dev->dev);
> --
> 2.39.2
>

Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Bartosz Golaszewski May 11, 2023, 9:03 a.m. UTC | #8
On Mon, May 8, 2023 at 10:53 PM Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
>
> The .remove() callback for a platform driver returns an int which makes
> many driver authors wrongly assume it's possible to do error handling by
> returning an error code. However the value returned is (mostly) ignored
> and this typically results in resource leaks. To improve here there is a
> quest to make the remove callback return void. In the first step of this
> quest all drivers are converted to .remove_new() which already returns
> void.
>
> Trivially convert this driver from always returning zero in the remove
> callback to the void returning variant.
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
>  drivers/i2c/busses/i2c-davinci.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.c
> index 7ba7e6cbcc97..b77f9288c0de 100644
> --- a/drivers/i2c/busses/i2c-davinci.c
> +++ b/drivers/i2c/busses/i2c-davinci.c
> @@ -885,7 +885,7 @@ static int davinci_i2c_probe(struct platform_device *pdev)
>         return r;
>  }
>
> -static int davinci_i2c_remove(struct platform_device *pdev)
> +static void davinci_i2c_remove(struct platform_device *pdev)
>  {
>         struct davinci_i2c_dev *dev = platform_get_drvdata(pdev);
>         int ret;
> @@ -903,8 +903,6 @@ static int davinci_i2c_remove(struct platform_device *pdev)
>         pm_runtime_dont_use_autosuspend(dev->dev);
>         pm_runtime_put_sync(dev->dev);
>         pm_runtime_disable(dev->dev);
> -
> -       return 0;
>  }
>
>  #ifdef CONFIG_PM
> @@ -945,7 +943,7 @@ MODULE_ALIAS("platform:i2c_davinci");
>
>  static struct platform_driver davinci_i2c_driver = {
>         .probe          = davinci_i2c_probe,
> -       .remove         = davinci_i2c_remove,
> +       .remove_new     = davinci_i2c_remove,
>         .driver         = {
>                 .name   = "i2c_davinci",
>                 .pm     = davinci_i2c_pm_ops,
> --
> 2.39.2
>

Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Jochen Friedrich May 16, 2023, 10:59 a.m. UTC | #9
Acked-by: Jochen Friedrich <jochen@scram.de>

Am 08.05.2023 um 22:51 schrieb Uwe Kleine-König:
> The .remove() callback for a platform driver returns an int which makes
> many driver authors wrongly assume it's possible to do error handling by
> returning an error code. However the value returned is (mostly) ignored
> and this typically results in resource leaks. To improve here there is a
> quest to make the remove callback return void. In the first step of this
> quest all drivers are converted to .remove_new() which already returns
> void.
>
> Trivially convert this driver from always returning zero in the remove
> callback to the void returning variant.
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
>   drivers/i2c/busses/i2c-cpm.c | 6 ++----
>   1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-cpm.c b/drivers/i2c/busses/i2c-cpm.c
> index 24d584a1c9a7..732daf6a932b 100644
> --- a/drivers/i2c/busses/i2c-cpm.c
> +++ b/drivers/i2c/busses/i2c-cpm.c
> @@ -676,7 +676,7 @@ static int cpm_i2c_probe(struct platform_device *ofdev)
>   	return result;
>   }
>   
> -static int cpm_i2c_remove(struct platform_device *ofdev)
> +static void cpm_i2c_remove(struct platform_device *ofdev)
>   {
>   	struct cpm_i2c *cpm = platform_get_drvdata(ofdev);
>   
> @@ -685,8 +685,6 @@ static int cpm_i2c_remove(struct platform_device *ofdev)
>   	cpm_i2c_shutdown(cpm);
>   
>   	kfree(cpm);
> -
> -	return 0;
>   }
>   
>   static const struct of_device_id cpm_i2c_match[] = {
> @@ -703,7 +701,7 @@ MODULE_DEVICE_TABLE(of, cpm_i2c_match);
>   
>   static struct platform_driver cpm_i2c_driver = {
>   	.probe		= cpm_i2c_probe,
> -	.remove		= cpm_i2c_remove,
> +	.remove_new	= cpm_i2c_remove,
>   	.driver = {
>   		.name = "fsl-i2c-cpm",
>   		.of_match_table = cpm_i2c_match,
Biju Das June 1, 2023, 7:43 a.m. UTC | #10
> Subject: Re: [PATCH 00/89] i2c: Convert to platform remove callback
> returning void
> 
> [Dropped Phil Edworthy from recipents as his email address has problems]

Phil no longer works with Renesas. Adding Fabrizio who is taking care of
RZ/V2M I2C driver.

Cheers,
Biju
Wolfram Sang June 1, 2023, 1:54 p.m. UTC | #11
> I wonder how this series will go in. My expectation was that Wolfram
> picks up the whole series via his tree?!

Will do. I am currently super-busy, though.
Wolfram Sang June 5, 2023, 7:54 a.m. UTC | #12
On Thu, Jun 01, 2023 at 03:54:50PM +0200, Wolfram Sang wrote:
> 
> > I wonder how this series will go in. My expectation was that Wolfram
> > picks up the whole series via his tree?!
> 
> Will do. I am currently super-busy, though.

Whole series applied to for-next. I squashed all the commits into one.
These are mostly simple changes which we won't revert anyhow, but fix
incrementally if we ever find an issue.