[PATCH 0/5] i3c: Convert to platform remove callback returning void

[PATCH 0/5] i3c: Convert to platform remove callback returning void

[Uwe Kleine-König]

Hello,

this series adapts the platform drivers below drivers/i3c to use the
.remove_new() callback. Compared to the traditional .remove() callback
.remove_new() returns no value. This is a good thing because the driver core
doesn't (and cannot) cope for errors during remove. The only effect of a
non-zero return value in .remove() is that the driver core emits a warning. The
device is removed anyhow and an early return from .remove() usually yields a
resource leak.

By changing the remove callback to return void driver authors cannot
reasonably assume any more that there is some kind of cleanup later.

The first patch simplifies the remove callbacks by making
i3c_master_unregister() return void. After that all drivers are converted
trivially to .remove_new().

Best regards
Uwe

Uwe Kleine-König (5):
  i3c: Make i3c_master_unregister() return void
  i3c: dw: Convert to platform remove callback returning void
  i3c: cdns: Convert to platform remove callback returning void
  i3c: mipi-i3c-hci: Convert to platform remove callback returning void
  i3c: svc: Convert to platform remove callback returning void

 drivers/i3c/master.c                   |  6 +-----
 drivers/i3c/master/dw-i3c-master.c     | 11 +++--------
 drivers/i3c/master/i3c-master-cdns.c   | 11 +++--------
 drivers/i3c/master/mipi-i3c-hci/core.c |  6 +++---
 drivers/i3c/master/svc-i3c-master.c    | 11 +++--------
 include/linux/i3c/master.h             |  2 +-
 6 files changed, 14 insertions(+), 33 deletions(-)


base-commit: fe15c26ee26efa11741a7b632e9f23b01aca4cc6
-- 
2.39.2


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH 1/5] i3c: Make i3c_master_unregister() return void

[Uwe Kleine-König]

The function returned zero unconditionally. Switch the return type to void
and simplify the callers accordingly.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/i3c/master.c                   | 6 +-----
 drivers/i3c/master/dw-i3c-master.c     | 5 +----
 drivers/i3c/master/i3c-master-cdns.c   | 5 +----
 drivers/i3c/master/mipi-i3c-hci/core.c | 4 +++-
 drivers/i3c/master/svc-i3c-master.c    | 5 +----
 include/linux/i3c/master.h             | 2 +-
 6 files changed, 8 insertions(+), 19 deletions(-)

diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index 54e4c34b4a22..04d6d54d2ab8 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -2695,17 +2695,13 @@ EXPORT_SYMBOL_GPL(i3c_master_register);
  * @master: master used to send frames on the bus
  *
  * Basically undo everything done in i3c_master_register().
- *
- * Return: 0 in case of success, a negative error code otherwise.
  */
-int i3c_master_unregister(struct i3c_master_controller *master)
+void i3c_master_unregister(struct i3c_master_controller *master)
 {
 	i3c_master_i2c_adapter_cleanup(master);
 	i3c_master_unregister_i3c_devs(master);
 	i3c_master_bus_cleanup(master);
 	device_unregister(&master->dev);
-
-	return 0;
 }
 EXPORT_SYMBOL_GPL(i3c_master_unregister);
 
diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
index 48954d3e6571..7135daada3e3 100644
--- a/drivers/i3c/master/dw-i3c-master.c
+++ b/drivers/i3c/master/dw-i3c-master.c
@@ -1185,11 +1185,8 @@ static int dw_i3c_probe(struct platform_device *pdev)
 static int dw_i3c_remove(struct platform_device *pdev)
 {
 	struct dw_i3c_master *master = platform_get_drvdata(pdev);
-	int ret;
 
-	ret = i3c_master_unregister(&master->base);
-	if (ret)
-		return ret;
+	i3c_master_unregister(&master->base);
 
 	reset_control_assert(master->core_rst);
 
diff --git a/drivers/i3c/master/i3c-master-cdns.c b/drivers/i3c/master/i3c-master-cdns.c
index 5b37ffe5ad5b..454925c5e097 100644
--- a/drivers/i3c/master/i3c-master-cdns.c
+++ b/drivers/i3c/master/i3c-master-cdns.c
@@ -1665,11 +1665,8 @@ static int cdns_i3c_master_probe(struct platform_device *pdev)
 static int cdns_i3c_master_remove(struct platform_device *pdev)
 {
 	struct cdns_i3c_master *master = platform_get_drvdata(pdev);
-	int ret;
 
-	ret = i3c_master_unregister(&master->base);
-	if (ret)
-		return ret;
+	i3c_master_unregister(&master->base);
 
 	clk_disable_unprepare(master->sysclk);
 	clk_disable_unprepare(master->pclk);
diff --git a/drivers/i3c/master/mipi-i3c-hci/core.c b/drivers/i3c/master/mipi-i3c-hci/core.c
index 6aef5ce43cc1..f9bc58366a72 100644
--- a/drivers/i3c/master/mipi-i3c-hci/core.c
+++ b/drivers/i3c/master/mipi-i3c-hci/core.c
@@ -769,7 +769,9 @@ static int i3c_hci_remove(struct platform_device *pdev)
 {
 	struct i3c_hci *hci = platform_get_drvdata(pdev);
 
-	return i3c_master_unregister(&hci->master);
+	i3c_master_unregister(&hci->master);
+
+	return 0;
 }
 
 static const __maybe_unused struct of_device_id i3c_hci_of_match[] = {
diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c
index d6e9ed74cdcf..e5476d04b403 100644
--- a/drivers/i3c/master/svc-i3c-master.c
+++ b/drivers/i3c/master/svc-i3c-master.c
@@ -1572,11 +1572,8 @@ static int svc_i3c_master_probe(struct platform_device *pdev)
 static int svc_i3c_master_remove(struct platform_device *pdev)
 {
 	struct svc_i3c_master *master = platform_get_drvdata(pdev);
-	int ret;
 
-	ret = i3c_master_unregister(&master->base);
-	if (ret)
-		return ret;
+	i3c_master_unregister(&master->base);
 
 	pm_runtime_dont_use_autosuspend(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h
index 604a126b78c8..4b7bb43bf430 100644
--- a/include/linux/i3c/master.h
+++ b/include/linux/i3c/master.h
@@ -541,7 +541,7 @@ int i3c_master_register(struct i3c_master_controller *master,
 			struct device *parent,
 			const struct i3c_master_controller_ops *ops,
 			bool secondary);
-int i3c_master_unregister(struct i3c_master_controller *master);
+void i3c_master_unregister(struct i3c_master_controller *master);
 
 /**
  * i3c_dev_get_master_data() - get master private data attached to an I3C
-- 
2.39.2


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH 1/5] i3c: Make i3c_master_unregister() return void

[Miquel Raynal]

Hi Uwe,

u.kleine-koenig@pengutronix.de wrote on Sun, 19 Mar 2023 00:33:07 +0100:

> The function returned zero unconditionally. Switch the return type to void
> and simplify the callers accordingly.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>

Thanks,
Miquèl

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH 2/5] i3c: dw: Convert to platform remove callback returning void

[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/i3c/master/dw-i3c-master.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
index 7135daada3e3..f5d3b7c04c88 100644
--- a/drivers/i3c/master/dw-i3c-master.c
+++ b/drivers/i3c/master/dw-i3c-master.c
@@ -1182,7 +1182,7 @@ static int dw_i3c_probe(struct platform_device *pdev)
 	return ret;
 }
 
-static int dw_i3c_remove(struct platform_device *pdev)
+static void dw_i3c_remove(struct platform_device *pdev)
 {
 	struct dw_i3c_master *master = platform_get_drvdata(pdev);
 
@@ -1191,8 +1191,6 @@ static int dw_i3c_remove(struct platform_device *pdev)
 	reset_control_assert(master->core_rst);
 
 	clk_disable_unprepare(master->core_clk);
-
-	return 0;
 }
 
 static const struct of_device_id dw_i3c_master_of_match[] = {
@@ -1203,7 +1201,7 @@ MODULE_DEVICE_TABLE(of, dw_i3c_master_of_match);
 
 static struct platform_driver dw_i3c_driver = {
 	.probe = dw_i3c_probe,
-	.remove = dw_i3c_remove,
+	.remove_new = dw_i3c_remove,
 	.driver = {
 		.name = "dw-i3c-master",
 		.of_match_table = of_match_ptr(dw_i3c_master_of_match),
-- 
2.39.2


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH 3/5] i3c: cdns: Convert to platform remove callback returning void

[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/i3c/master/i3c-master-cdns.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/i3c/master/i3c-master-cdns.c b/drivers/i3c/master/i3c-master-cdns.c
index 454925c5e097..01610fa5b0cc 100644
--- a/drivers/i3c/master/i3c-master-cdns.c
+++ b/drivers/i3c/master/i3c-master-cdns.c
@@ -1662,7 +1662,7 @@ static int cdns_i3c_master_probe(struct platform_device *pdev)
 	return ret;
 }
 
-static int cdns_i3c_master_remove(struct platform_device *pdev)
+static void cdns_i3c_master_remove(struct platform_device *pdev)
 {
 	struct cdns_i3c_master *master = platform_get_drvdata(pdev);
 
@@ -1670,13 +1670,11 @@ static int cdns_i3c_master_remove(struct platform_device *pdev)
 
 	clk_disable_unprepare(master->sysclk);
 	clk_disable_unprepare(master->pclk);
-
-	return 0;
 }
 
 static struct platform_driver cdns_i3c_master = {
 	.probe = cdns_i3c_master_probe,
-	.remove = cdns_i3c_master_remove,
+	.remove_new = cdns_i3c_master_remove,
 	.driver = {
 		.name = "cdns-i3c-master",
 		.of_match_table = cdns_i3c_master_of_ids,
-- 
2.39.2


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH 4/5] i3c: mipi-i3c-hci: Convert to platform remove callback returning void

[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/i3c/master/mipi-i3c-hci/core.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/i3c/master/mipi-i3c-hci/core.c b/drivers/i3c/master/mipi-i3c-hci/core.c
index f9bc58366a72..837af83c85f4 100644
--- a/drivers/i3c/master/mipi-i3c-hci/core.c
+++ b/drivers/i3c/master/mipi-i3c-hci/core.c
@@ -765,13 +765,11 @@ static int i3c_hci_probe(struct platform_device *pdev)
 	return 0;
 }
 
-static int i3c_hci_remove(struct platform_device *pdev)
+static void i3c_hci_remove(struct platform_device *pdev)
 {
 	struct i3c_hci *hci = platform_get_drvdata(pdev);
 
 	i3c_master_unregister(&hci->master);
-
-	return 0;
 }
 
 static const __maybe_unused struct of_device_id i3c_hci_of_match[] = {
@@ -782,7 +780,7 @@ MODULE_DEVICE_TABLE(of, i3c_hci_of_match);
 
 static struct platform_driver i3c_hci_driver = {
 	.probe = i3c_hci_probe,
-	.remove = i3c_hci_remove,
+	.remove_new = i3c_hci_remove,
 	.driver = {
 		.name = "mipi-i3c-hci",
 		.of_match_table = of_match_ptr(i3c_hci_of_match),
-- 
2.39.2


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH 5/5] i3c: svc: Convert to platform remove callback returning void

[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/i3c/master/svc-i3c-master.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c
index e5476d04b403..e3f454123805 100644
--- a/drivers/i3c/master/svc-i3c-master.c
+++ b/drivers/i3c/master/svc-i3c-master.c
@@ -1569,7 +1569,7 @@ static int svc_i3c_master_probe(struct platform_device *pdev)
 	return ret;
 }
 
-static int svc_i3c_master_remove(struct platform_device *pdev)
+static void svc_i3c_master_remove(struct platform_device *pdev)
 {
 	struct svc_i3c_master *master = platform_get_drvdata(pdev);
 
@@ -1577,8 +1577,6 @@ static int svc_i3c_master_remove(struct platform_device *pdev)
 
 	pm_runtime_dont_use_autosuspend(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
-
-	return 0;
 }
 
 static int __maybe_unused svc_i3c_runtime_suspend(struct device *dev)
@@ -1616,7 +1614,7 @@ MODULE_DEVICE_TABLE(of, svc_i3c_master_of_match_tbl);
 
 static struct platform_driver svc_i3c_master = {
 	.probe = svc_i3c_master_probe,
-	.remove = svc_i3c_master_remove,
+	.remove_new = svc_i3c_master_remove,
 	.driver = {
 		.name = "silvaco-i3c-master",
 		.of_match_table = svc_i3c_master_of_match_tbl,
-- 
2.39.2


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH 5/5] i3c: svc: Convert to platform remove callback returning void

[Miquel Raynal]

Hi Uwe,

u.kleine-koenig@pengutronix.de wrote on Sun, 19 Mar 2023 00:33:11 +0100:

> 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: Miquel Raynal <miquel.raynal@bootlin.com>

> ---
>  drivers/i3c/master/svc-i3c-master.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/i3c/master/svc-i3c-master.c b/drivers/i3c/master/svc-i3c-master.c
> index e5476d04b403..e3f454123805 100644
> --- a/drivers/i3c/master/svc-i3c-master.c
> +++ b/drivers/i3c/master/svc-i3c-master.c
> @@ -1569,7 +1569,7 @@ static int svc_i3c_master_probe(struct platform_device *pdev)
>  	return ret;
>  }
>  
> -static int svc_i3c_master_remove(struct platform_device *pdev)
> +static void svc_i3c_master_remove(struct platform_device *pdev)
>  {
>  	struct svc_i3c_master *master = platform_get_drvdata(pdev);
>  
> @@ -1577,8 +1577,6 @@ static int svc_i3c_master_remove(struct platform_device *pdev)
>  
>  	pm_runtime_dont_use_autosuspend(&pdev->dev);
>  	pm_runtime_disable(&pdev->dev);
> -
> -	return 0;
>  }
>  
>  static int __maybe_unused svc_i3c_runtime_suspend(struct device *dev)
> @@ -1616,7 +1614,7 @@ MODULE_DEVICE_TABLE(of, svc_i3c_master_of_match_tbl);
>  
>  static struct platform_driver svc_i3c_master = {
>  	.probe = svc_i3c_master_probe,
> -	.remove = svc_i3c_master_remove,
> +	.remove_new = svc_i3c_master_remove,
>  	.driver = {
>  		.name = "silvaco-i3c-master",
>  		.of_match_table = svc_i3c_master_of_match_tbl,


Thanks,
Miquèl

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH 0/5] i3c: Convert to platform remove callback returning void

[Alexandre Belloni]

On Sun, 19 Mar 2023 00:33:06 +0100, Uwe Kleine-König wrote:
> this series adapts the platform drivers below drivers/i3c to use the
> .remove_new() callback. Compared to the traditional .remove() callback
> .remove_new() returns no value. This is a good thing because the driver core
> doesn't (and cannot) cope for errors during remove. The only effect of a
> non-zero return value in .remove() is that the driver core emits a warning. The
> device is removed anyhow and an early return from .remove() usually yields a
> resource leak.
> 
> [...]

Applied, thanks!

[1/5] i3c: Make i3c_master_unregister() return void
      commit: 0f74f8b6675cc36d689abb4d9b3d75ab4049b7d7
[2/5] i3c: dw: Convert to platform remove callback returning void
      commit: 04b5f1be2673a035b0218c6e0369cfd1460d63a1
[3/5] i3c: cdns: Convert to platform remove callback returning void
      commit: 3f8ad583590f7bfeb3b8ee1ddd1cca5f7f9a5a75
[4/5] i3c: mipi-i3c-hci: Convert to platform remove callback returning void
      commit: f959ec617521bab177758b7c71b0d56a65615079
[5/5] i3c: svc: Convert to platform remove callback returning void
      commit: 2810f1de814adb7728b1068586bebcf733cf1992

Best regards,

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c