[PATCH 1/5] mmc: dw_mmc: exynos: Obviously always return success in remove callback

[PATCH 1/5] mmc: dw_mmc: exynos: Obviously always return success in remove callback

[Uwe Kleine-König]

dw_mci_pltfm_remove() returns 0 unconditionally and returning an error
in a platform remove callback isn't very sensible. (The only effect of
the latter is that the device core emits a generic warning and then
removes the device anyhow.)

So return 0 unconditionally to make it obvious there is no error
forwarded to the upper layers.

This is a preparation for making platform remove callbacks return void.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/mmc/host/dw_mmc-exynos.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/dw_mmc-exynos.c b/drivers/mmc/host/dw_mmc-exynos.c
index ca5be4445ae0..9f20ac524c8b 100644
--- a/drivers/mmc/host/dw_mmc-exynos.c
+++ b/drivers/mmc/host/dw_mmc-exynos.c
@@ -670,7 +670,9 @@ static int dw_mci_exynos_remove(struct platform_device *pdev)
 	pm_runtime_set_suspended(&pdev->dev);
 	pm_runtime_put_noidle(&pdev->dev);
 
-	return dw_mci_pltfm_remove(pdev);
+	dw_mci_pltfm_remove(pdev);
+
+	return 0;
 }
 
 static const struct dev_pm_ops dw_mci_exynos_pmops = {

base-commit: f2906aa863381afb0015a9eb7fefad885d4e5a56
-- 
2.36.1

[PATCH 2/5] mmc: dw_mmc: hi3789cv200: Obviously always return success in remove callback

[Uwe Kleine-König]

dw_mci_pltfm_remove() returns 0 unconditionally and returning an error
in a platform remove callback isn't very sensible. (The only effect of
the latter is that the device core emits a generic warning and then
removes the device anyhow.)

So return 0 unconditionally to make it obvious there is no error
forwarded to the upper layers.

This is a preparation for making platform remove callbacks return void.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/mmc/host/dw_mmc-hi3798cv200.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/dw_mmc-hi3798cv200.c b/drivers/mmc/host/dw_mmc-hi3798cv200.c
index e9437ef8ef19..6f22fe054087 100644
--- a/drivers/mmc/host/dw_mmc-hi3798cv200.c
+++ b/drivers/mmc/host/dw_mmc-hi3798cv200.c
@@ -179,7 +179,9 @@ static int dw_mci_hi3798cv200_remove(struct platform_device *pdev)
 	clk_disable_unprepare(priv->drive_clk);
 	clk_disable_unprepare(priv->sample_clk);
 
-	return dw_mci_pltfm_remove(pdev);
+	dw_mci_pltfm_remove(pdev);
+
+	return 0;
 }
 
 static const struct of_device_id dw_mci_hi3798cv200_match[] = {
-- 
2.36.1

[PATCH 3/5] mmc: dw_mmc: rockchip: Obviously always return success in remove callback

[Uwe Kleine-König]

dw_mci_pltfm_remove() returns 0 unconditionally and returning an error
in a platform remove callback isn't very sensible. (The only effect of
the latter is that the device core emits a generic warning and then
removes the device anyhow.)

So return 0 unconditionally to make it obvious there is no error
forwarded to the upper layers.

This is a preparation for making platform remove callbacks return void.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/mmc/host/dw_mmc-rockchip.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/dw_mmc-rockchip.c b/drivers/mmc/host/dw_mmc-rockchip.c
index f825487aa739..2a99f15f527f 100644
--- a/drivers/mmc/host/dw_mmc-rockchip.c
+++ b/drivers/mmc/host/dw_mmc-rockchip.c
@@ -377,7 +377,9 @@ static int dw_mci_rockchip_remove(struct platform_device *pdev)
 	pm_runtime_disable(&pdev->dev);
 	pm_runtime_put_noidle(&pdev->dev);
 
-	return dw_mci_pltfm_remove(pdev);
+	dw_mci_pltfm_remove(pdev);
+
+	return 0;
 }
 
 static const struct dev_pm_ops dw_mci_rockchip_dev_pm_ops = {
-- 
2.36.1

[PATCH 4/5] mmc: sdhci-of-arasan: Obviously always return success in remove callback

[Uwe Kleine-König]

sdhci_pltfm_unregister() returns 0 unconditionally and returning an
error in a platform remove callback isn't very sensible. (The only
effect of the latter is that the device core emits a generic warning and
then removes the device anyhow.)

So return 0 unconditionally to make it obvious there is no error
forwarded to the upper layers.

This is a preparation for making platform remove callbacks return void.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/mmc/host/sdhci-of-arasan.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c
index 757801dfc308..3997cad1f793 100644
--- a/drivers/mmc/host/sdhci-of-arasan.c
+++ b/drivers/mmc/host/sdhci-of-arasan.c
@@ -1733,7 +1733,6 @@ static int sdhci_arasan_probe(struct platform_device *pdev)
 
 static int sdhci_arasan_remove(struct platform_device *pdev)
 {
-	int ret;
 	struct sdhci_host *host = platform_get_drvdata(pdev);
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
 	struct sdhci_arasan_data *sdhci_arasan = sdhci_pltfm_priv(pltfm_host);
@@ -1747,11 +1746,11 @@ static int sdhci_arasan_remove(struct platform_device *pdev)
 
 	sdhci_arasan_unregister_sdclk(&pdev->dev);
 
-	ret = sdhci_pltfm_unregister(pdev);
+	sdhci_pltfm_unregister(pdev);
 
 	clk_disable_unprepare(clk_ahb);
 
-	return ret;
+	return 0;
 }
 
 static struct platform_driver sdhci_arasan_driver = {
-- 
2.36.1

Re: [PATCH 4/5] mmc: sdhci-of-arasan: Obviously always return success in remove callback

[Adrian Hunter]

On 11/06/22 00:12, Uwe Kleine-König wrote:
> sdhci_pltfm_unregister() returns 0 unconditionally and returning an
> error in a platform remove callback isn't very sensible. (The only
> effect of the latter is that the device core emits a generic warning and
> then removes the device anyhow.)
> 
> So return 0 unconditionally to make it obvious there is no error
> forwarded to the upper layers.
> 
> This is a preparation for making platform remove callbacks return void.

This preparation seems a bit unnatural.

> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
>  drivers/mmc/host/sdhci-of-arasan.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c
> index 757801dfc308..3997cad1f793 100644
> --- a/drivers/mmc/host/sdhci-of-arasan.c
> +++ b/drivers/mmc/host/sdhci-of-arasan.c
> @@ -1733,7 +1733,6 @@ static int sdhci_arasan_probe(struct platform_device *pdev)
>  
>  static int sdhci_arasan_remove(struct platform_device *pdev)
>  {
> -	int ret;
>  	struct sdhci_host *host = platform_get_drvdata(pdev);
>  	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>  	struct sdhci_arasan_data *sdhci_arasan = sdhci_pltfm_priv(pltfm_host);
> @@ -1747,11 +1746,11 @@ static int sdhci_arasan_remove(struct platform_device *pdev)
>  
>  	sdhci_arasan_unregister_sdclk(&pdev->dev);
>  
> -	ret = sdhci_pltfm_unregister(pdev);
> +	sdhci_pltfm_unregister(pdev);
>  
>  	clk_disable_unprepare(clk_ahb);
>  
> -	return ret;
> +	return 0;
>  }
>  
>  static struct platform_driver sdhci_arasan_driver = {

Re: [PATCH 4/5] mmc: sdhci-of-arasan: Obviously always return success in remove callback

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 2768 bytes --]

On Mon, Jun 13, 2022 at 10:13:16AM +0300, Adrian Hunter wrote:
> On 11/06/22 00:12, Uwe Kleine-König wrote:
> > sdhci_pltfm_unregister() returns 0 unconditionally and returning an
> > error in a platform remove callback isn't very sensible. (The only
> > effect of the latter is that the device core emits a generic warning and
> > then removes the device anyhow.)
> > 
> > So return 0 unconditionally to make it obvious there is no error
> > forwarded to the upper layers.
> > 
> > This is a preparation for making platform remove callbacks return void.
> 
> This preparation seems a bit unnatural.

IMHO it's not. I have to adapt all ~4800 platform drivers together in a
single commit to change the prototype of the return callback.

Now assume you want to review that commit and make sure there are no
relevant changes in behaviour. Without the preparation in the commit
under discussion, the change to sdhci-of-arasan.c would look as follows:

diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c
--- a/drivers/mmc/host/sdhci-of-arasan.c
+++ b/drivers/mmc/host/sdhci-of-arasan.c
@@ -1733,7 +1733,4 @@ static int sdhci_arasan_probe(struct platform_device *pdev)
 
 static int sdhci_arasan_remove(struct platform_device *pdev)
 {
-	int ret;
 	struct sdhci_host *host = platform_get_drvdata(pdev);
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
 	struct sdhci_arasan_data *sdhci_arasan = sdhci_pltfm_priv(pltfm_host);
@@ -1747,11 +1746,11 @@ static int sdhci_arasan_remove(struct platform_device *pdev)
 
 	sdhci_arasan_unregister_sdclk(&pdev->dev);
 
-	ret = sdhci_pltfm_unregister(pdev);
+	sdhci_pltfm_unregister(pdev);
 
 	clk_disable_unprepare(clk_ahb);
- 
-	return ret;
 }
 
 static struct platform_driver sdhci_arasan_driver = {

So you have to look up then what sdhci_pltfm_unregister() does and if
it's ok to ignore the return value. Should I mention that in the commit
log? What about the other drivers?

If however the change to the sdhci-arasan driver is only

diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c
index 3997cad1f793..84c949bd99c8 100644
--- a/drivers/mmc/host/sdhci-of-arasan.c
+++ b/drivers/mmc/host/sdhci-of-arasan.c
@@ -1749,8 +1749,6 @@ static int sdhci_arasan_remove(struct platform_device *pdev)
 	sdhci_pltfm_unregister(pdev);
 
 	clk_disable_unprepare(clk_ahb);
-
-	return 0;
 }
 
 static struct platform_driver sdhci_arasan_driver = {

, it's trivial to see that there is no relevant driver specific change.

Best regards
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 --]

[PATCH 5/5] mmc: sdhci-st: Obviously always return success in remove callback

[Uwe Kleine-König]

sdhci_pltfm_unregister() returns 0 unconditionally and returning an
error in a platform remove callback isn't very sensible. (The only
effect of the latter is that the device core emits a generic warning and
then removes the device anyhow.)

So return 0 unconditionally to make it obvious there is no error
forwarded to the upper layers.

This is a preparation for making platform remove callbacks return void.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/mmc/host/sdhci-st.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/sdhci-st.c b/drivers/mmc/host/sdhci-st.c
index d41582c21aa3..6415916fbd91 100644
--- a/drivers/mmc/host/sdhci-st.c
+++ b/drivers/mmc/host/sdhci-st.c
@@ -440,15 +440,14 @@ static int sdhci_st_remove(struct platform_device *pdev)
 	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
 	struct st_mmc_platform_data *pdata = sdhci_pltfm_priv(pltfm_host);
 	struct reset_control *rstc = pdata->rstc;
-	int ret;
 
-	ret = sdhci_pltfm_unregister(pdev);
+	sdhci_pltfm_unregister(pdev);
 
 	clk_disable_unprepare(pdata->icnclk);
 
 	reset_control_assert(rstc);
 
-	return ret;
+	return 0;
 }
 
 #ifdef CONFIG_PM_SLEEP
-- 
2.36.1

Re: [PATCH 5/5] mmc: sdhci-st: Obviously always return success in remove callback

[Adrian Hunter]

On 11/06/22 00:12, Uwe Kleine-König wrote:
> sdhci_pltfm_unregister() returns 0 unconditionally and returning an
> error in a platform remove callback isn't very sensible. (The only
> effect of the latter is that the device core emits a generic warning and
> then removes the device anyhow.)
> 
> So return 0 unconditionally to make it obvious there is no error
> forwarded to the upper layers.
> 
> This is a preparation for making platform remove callbacks return void.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
>  drivers/mmc/host/sdhci-st.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-st.c b/drivers/mmc/host/sdhci-st.c
> index d41582c21aa3..6415916fbd91 100644
> --- a/drivers/mmc/host/sdhci-st.c
> +++ b/drivers/mmc/host/sdhci-st.c
> @@ -440,15 +440,14 @@ static int sdhci_st_remove(struct platform_device *pdev)
>  	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>  	struct st_mmc_platform_data *pdata = sdhci_pltfm_priv(pltfm_host);
>  	struct reset_control *rstc = pdata->rstc;
> -	int ret;
>  
> -	ret = sdhci_pltfm_unregister(pdev);
> +	sdhci_pltfm_unregister(pdev);
>  
>  	clk_disable_unprepare(pdata->icnclk);
>  
>  	reset_control_assert(rstc);
>  
> -	return ret;
> +	return 0;
>  }
>  
>  #ifdef CONFIG_PM_SLEEP

Re: [PATCH 1/5] mmc: dw_mmc: exynos: Obviously always return success in remove callback

[Ulf Hansson]

On Fri, 10 Jun 2022 at 14:13, Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
>
> dw_mci_pltfm_remove() returns 0 unconditionally and returning an error
> in a platform remove callback isn't very sensible. (The only effect of
> the latter is that the device core emits a generic warning and then
> removes the device anyhow.)
>
> So return 0 unconditionally to make it obvious there is no error
> forwarded to the upper layers.
>
> This is a preparation for making platform remove callbacks return void.
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

This one and the other 4 patches in the series, applied for next. Thanks!

Kind regards
Uffe


> ---
>  drivers/mmc/host/dw_mmc-exynos.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/dw_mmc-exynos.c b/drivers/mmc/host/dw_mmc-exynos.c
> index ca5be4445ae0..9f20ac524c8b 100644
> --- a/drivers/mmc/host/dw_mmc-exynos.c
> +++ b/drivers/mmc/host/dw_mmc-exynos.c
> @@ -670,7 +670,9 @@ static int dw_mci_exynos_remove(struct platform_device *pdev)
>         pm_runtime_set_suspended(&pdev->dev);
>         pm_runtime_put_noidle(&pdev->dev);
>
> -       return dw_mci_pltfm_remove(pdev);
> +       dw_mci_pltfm_remove(pdev);
> +
> +       return 0;
>  }
>
>  static const struct dev_pm_ops dw_mci_exynos_pmops = {
>
> base-commit: f2906aa863381afb0015a9eb7fefad885d4e5a56
> --
> 2.36.1
>