[PATCH 00/19] drm/bridge: Convert to platform remove callback returning void

[PATCH 00/19] drm/bridge: Convert to platform remove callback returning void

[Uwe Kleine-König]

Hello,

this series adapts the platform drivers below drivers/gpu/drm/bridge 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 cdns-mhdp8546 driver to return zero in its
remove callback unconditionally and thenn all drivers are trivially converted
using coccinelle.

Only patch #3 depends on patch #1, the other are pairwise independant and can
be applied individually.

Best regards
Uwe

Uwe Kleine-König (19):
  drm/bridge: cdns-mhdp8546: Improve error reporting in remove callback
  drm/bridge: cdns-dsi: Convert to platform remove callback returning
    void
  drm/bridge: cdns-mhdp8546: Convert to platform remove callback
    returning void
  drm/bridge: display-connector: Convert to platform remove callback
    returning void
  drm/bridge: fsl-ldb: Convert to platform remove callback returning
    void
  drm/bridge: imx8qm-ldb: Convert to platform remove callback returning
    void
  drm/bridge: imx8qxp-ldb: Convert to platform remove callback returning
    void
  drm/bridge: imx8qxp-pixel-combiner: Convert to platform remove
    callback returning void
  drm/bridge: imx8qxp-pixel-link: Convert to platform remove callback
    returning void
  drm/bridge: imx8qxp-pxl2dpi: Convert to platform remove callback
    returning void
  drm/bridge: lvds-codec: Convert to platform remove callback returning
    void
  drm/bridge: nwl-dsi: Convert to platform remove callback returning
    void
  drm/bridge: simple-bridg: Convert to platform remove callback
    returning void
  drm/bridge: dw-hdmi-ahb-audio: Convert to platform remove callback
    returning void
  drm/bridge: dw-hdmi-cec: Convert to platform remove callback returning
    void
  drm/bridge: dw-hdmi-gp-audio: Convert to platform remove callback
    returning void
  drm/bridge: dw-hdmi-i2s-audio: Convert to platform remove callback
    returning void
  drm/bridge: thc63lvd1024: Convert to platform remove callback
    returning void
  drm/bridge: ti-tfp410: Convert to platform remove callback returning
    void

 .../gpu/drm/bridge/cadence/cdns-dsi-core.c    |  6 ++---
 .../drm/bridge/cadence/cdns-mhdp8546-core.c   | 22 +++++++++----------
 drivers/gpu/drm/bridge/display-connector.c    |  6 ++---
 drivers/gpu/drm/bridge/fsl-ldb.c              |  6 ++---
 drivers/gpu/drm/bridge/imx/imx8qm-ldb-drv.c   |  6 ++---
 drivers/gpu/drm/bridge/imx/imx8qxp-ldb-drv.c  |  6 ++---
 .../drm/bridge/imx/imx8qxp-pixel-combiner.c   |  6 ++---
 .../gpu/drm/bridge/imx/imx8qxp-pixel-link.c   |  6 ++---
 drivers/gpu/drm/bridge/imx/imx8qxp-pxl2dpi.c  |  6 ++---
 drivers/gpu/drm/bridge/lvds-codec.c           |  6 ++---
 drivers/gpu/drm/bridge/nwl-dsi.c              |  5 ++---
 drivers/gpu/drm/bridge/simple-bridge.c        |  6 ++---
 .../drm/bridge/synopsys/dw-hdmi-ahb-audio.c   |  6 ++---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c |  6 ++---
 .../drm/bridge/synopsys/dw-hdmi-gp-audio.c    |  6 ++---
 .../drm/bridge/synopsys/dw-hdmi-i2s-audio.c   |  6 ++---
 drivers/gpu/drm/bridge/thc63lvd1024.c         |  6 ++---
 drivers/gpu/drm/bridge/ti-tfp410.c            |  6 ++---
 18 files changed, 44 insertions(+), 79 deletions(-)


base-commit: fe15c26ee26efa11741a7b632e9f23b01aca4cc6
-- 
2.39.2

[PATCH 01/19] drm/bridge: cdns-mhdp8546: Improve error reporting in remove callback

[Uwe Kleine-König]

Replace the generic error message issued by the driver core when the remove
callback returns non-zero ("remove callback returned a non-zero value. This
will be ignored.") by a message that tells the actual problem.

Also simplify a bit by checking the return value of wait_event_timeout a
bit later.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 .../drm/bridge/cadence/cdns-mhdp8546-core.c    | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
index f6822dfa3805..d74c6fa30ccc 100644
--- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
+++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
@@ -2574,7 +2574,6 @@ static int cdns_mhdp_remove(struct platform_device *pdev)
 {
 	struct cdns_mhdp_device *mhdp = platform_get_drvdata(pdev);
 	unsigned long timeout = msecs_to_jiffies(100);
-	bool stop_fw = false;
 	int ret;
 
 	drm_bridge_remove(&mhdp->bridge);
@@ -2582,18 +2581,19 @@ static int cdns_mhdp_remove(struct platform_device *pdev)
 	ret = wait_event_timeout(mhdp->fw_load_wq,
 				 mhdp->hw_state == MHDP_HW_READY,
 				 timeout);
-	if (ret == 0)
-		dev_err(mhdp->dev, "%s: Timeout waiting for fw loading\n",
-			__func__);
-	else
-		stop_fw = true;
-
 	spin_lock(&mhdp->start_lock);
 	mhdp->hw_state = MHDP_HW_STOPPED;
 	spin_unlock(&mhdp->start_lock);
 
-	if (stop_fw)
+	if (ret == 0) {
+		dev_err(mhdp->dev, "%s: Timeout waiting for fw loading\n",
+			__func__);
+	} else {
 		ret = cdns_mhdp_set_firmware_active(mhdp, false);
+		if (ret)
+			dev_err(mhdp->dev, "Failed to stop firmware (%pe)\n",
+				ERR_PTR(ret));
+	}
 
 	phy_exit(mhdp->phy);
 
@@ -2609,7 +2609,7 @@ static int cdns_mhdp_remove(struct platform_device *pdev)
 
 	clk_disable_unprepare(mhdp->clk);
 
-	return ret;
+	return 0;
 }
 
 static const struct of_device_id mhdp_ids[] = {
-- 
2.39.2

[PATCH 02/19] drm/bridge: cdns-dsi: 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/gpu/drm/bridge/cadence/cdns-dsi-core.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
index 5dbfc7226b31..f50d65f54314 100644
--- a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
+++ b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
@@ -1278,7 +1278,7 @@ static int cdns_dsi_drm_probe(struct platform_device *pdev)
 	return ret;
 }
 
-static int cdns_dsi_drm_remove(struct platform_device *pdev)
+static void cdns_dsi_drm_remove(struct platform_device *pdev)
 {
 	struct cdns_dsi *dsi = platform_get_drvdata(pdev);
 
@@ -1288,8 +1288,6 @@ static int cdns_dsi_drm_remove(struct platform_device *pdev)
 		dsi->platform_ops->deinit(dsi);
 
 	pm_runtime_disable(&pdev->dev);
-
-	return 0;
 }
 
 static const struct of_device_id cdns_dsi_of_match[] = {
@@ -1303,7 +1301,7 @@ MODULE_DEVICE_TABLE(of, cdns_dsi_of_match);
 
 static struct platform_driver cdns_dsi_platform_driver = {
 	.probe  = cdns_dsi_drm_probe,
-	.remove = cdns_dsi_drm_remove,
+	.remove_new = cdns_dsi_drm_remove,
 	.driver = {
 		.name   = "cdns-dsi",
 		.of_match_table = cdns_dsi_of_match,
-- 
2.39.2

[PATCH 03/19] drm/bridge: cdns-mhdp8546: 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/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
index d74c6fa30ccc..45d52baa1adc 100644
--- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
+++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
@@ -2570,7 +2570,7 @@ static int cdns_mhdp_probe(struct platform_device *pdev)
 	return ret;
 }
 
-static int cdns_mhdp_remove(struct platform_device *pdev)
+static void cdns_mhdp_remove(struct platform_device *pdev)
 {
 	struct cdns_mhdp_device *mhdp = platform_get_drvdata(pdev);
 	unsigned long timeout = msecs_to_jiffies(100);
@@ -2608,8 +2608,6 @@ static int cdns_mhdp_remove(struct platform_device *pdev)
 	/* Ignoring mhdp->hdcp.check_work and mhdp->hdcp.prop_work here. */
 
 	clk_disable_unprepare(mhdp->clk);
-
-	return 0;
 }
 
 static const struct of_device_id mhdp_ids[] = {
@@ -2632,7 +2630,7 @@ static struct platform_driver mhdp_driver = {
 		.of_match_table	= of_match_ptr(mhdp_ids),
 	},
 	.probe	= cdns_mhdp_probe,
-	.remove	= cdns_mhdp_remove,
+	.remove_new = cdns_mhdp_remove,
 };
 module_platform_driver(mhdp_driver);
 
-- 
2.39.2

[PATCH 04/19] drm/bridge: display-connector: 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/gpu/drm/bridge/display-connector.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/bridge/display-connector.c b/drivers/gpu/drm/bridge/display-connector.c
index 9a12449ad7b8..fbb3e102c02f 100644
--- a/drivers/gpu/drm/bridge/display-connector.c
+++ b/drivers/gpu/drm/bridge/display-connector.c
@@ -382,7 +382,7 @@ static int display_connector_probe(struct platform_device *pdev)
 	return 0;
 }
 
-static int display_connector_remove(struct platform_device *pdev)
+static void display_connector_remove(struct platform_device *pdev)
 {
 	struct display_connector *conn = platform_get_drvdata(pdev);
 
@@ -396,8 +396,6 @@ static int display_connector_remove(struct platform_device *pdev)
 
 	if (!IS_ERR(conn->bridge.ddc))
 		i2c_put_adapter(conn->bridge.ddc);
-
-	return 0;
 }
 
 static const struct of_device_id display_connector_match[] = {
@@ -426,7 +424,7 @@ MODULE_DEVICE_TABLE(of, display_connector_match);
 
 static struct platform_driver display_connector_driver = {
 	.probe	= display_connector_probe,
-	.remove	= display_connector_remove,
+	.remove_new = display_connector_remove,
 	.driver		= {
 		.name		= "display-connector",
 		.of_match_table	= display_connector_match,
-- 
2.39.2

[PATCH 05/19] drm/bridge: fsl-ldb: 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/gpu/drm/bridge/fsl-ldb.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/bridge/fsl-ldb.c b/drivers/gpu/drm/bridge/fsl-ldb.c
index 6bac160b395b..450b352914f4 100644
--- a/drivers/gpu/drm/bridge/fsl-ldb.c
+++ b/drivers/gpu/drm/bridge/fsl-ldb.c
@@ -347,13 +347,11 @@ static int fsl_ldb_probe(struct platform_device *pdev)
 	return 0;
 }
 
-static int fsl_ldb_remove(struct platform_device *pdev)
+static void fsl_ldb_remove(struct platform_device *pdev)
 {
 	struct fsl_ldb *fsl_ldb = platform_get_drvdata(pdev);
 
 	drm_bridge_remove(&fsl_ldb->bridge);
-
-	return 0;
 }
 
 static const struct of_device_id fsl_ldb_match[] = {
@@ -367,7 +365,7 @@ MODULE_DEVICE_TABLE(of, fsl_ldb_match);
 
 static struct platform_driver fsl_ldb_driver = {
 	.probe	= fsl_ldb_probe,
-	.remove	= fsl_ldb_remove,
+	.remove_new = fsl_ldb_remove,
 	.driver		= {
 		.name		= "fsl-ldb",
 		.of_match_table	= fsl_ldb_match,
-- 
2.39.2

[PATCH 06/19] drm/bridge: imx8qm-ldb: 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/gpu/drm/bridge/imx/imx8qm-ldb-drv.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/bridge/imx/imx8qm-ldb-drv.c b/drivers/gpu/drm/bridge/imx/imx8qm-ldb-drv.c
index 178af8d2d80b..386032a02599 100644
--- a/drivers/gpu/drm/bridge/imx/imx8qm-ldb-drv.c
+++ b/drivers/gpu/drm/bridge/imx/imx8qm-ldb-drv.c
@@ -532,7 +532,7 @@ static int imx8qm_ldb_probe(struct platform_device *pdev)
 	return ret;
 }
 
-static int imx8qm_ldb_remove(struct platform_device *pdev)
+static void imx8qm_ldb_remove(struct platform_device *pdev)
 {
 	struct imx8qm_ldb *imx8qm_ldb = platform_get_drvdata(pdev);
 	struct ldb *ldb = &imx8qm_ldb->base;
@@ -540,8 +540,6 @@ static int imx8qm_ldb_remove(struct platform_device *pdev)
 	ldb_remove_bridge_helper(ldb);
 
 	pm_runtime_disable(&pdev->dev);
-
-	return 0;
 }
 
 static int __maybe_unused imx8qm_ldb_runtime_suspend(struct device *dev)
@@ -573,7 +571,7 @@ MODULE_DEVICE_TABLE(of, imx8qm_ldb_dt_ids);
 
 static struct platform_driver imx8qm_ldb_driver = {
 	.probe	= imx8qm_ldb_probe,
-	.remove = imx8qm_ldb_remove,
+	.remove_new = imx8qm_ldb_remove,
 	.driver	= {
 		.pm = &imx8qm_ldb_pm_ops,
 		.name = DRIVER_NAME,
-- 
2.39.2

[PATCH 07/19] drm/bridge: imx8qxp-ldb: 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/gpu/drm/bridge/imx/imx8qxp-ldb-drv.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/bridge/imx/imx8qxp-ldb-drv.c b/drivers/gpu/drm/bridge/imx/imx8qxp-ldb-drv.c
index 63948d5d20fd..c806576b1e22 100644
--- a/drivers/gpu/drm/bridge/imx/imx8qxp-ldb-drv.c
+++ b/drivers/gpu/drm/bridge/imx/imx8qxp-ldb-drv.c
@@ -667,7 +667,7 @@ static int imx8qxp_ldb_probe(struct platform_device *pdev)
 	return ret;
 }
 
-static int imx8qxp_ldb_remove(struct platform_device *pdev)
+static void imx8qxp_ldb_remove(struct platform_device *pdev)
 {
 	struct imx8qxp_ldb *imx8qxp_ldb = platform_get_drvdata(pdev);
 	struct ldb *ldb = &imx8qxp_ldb->base;
@@ -675,8 +675,6 @@ static int imx8qxp_ldb_remove(struct platform_device *pdev)
 	ldb_remove_bridge_helper(ldb);
 
 	pm_runtime_disable(&pdev->dev);
-
-	return 0;
 }
 
 static int __maybe_unused imx8qxp_ldb_runtime_suspend(struct device *dev)
@@ -708,7 +706,7 @@ MODULE_DEVICE_TABLE(of, imx8qxp_ldb_dt_ids);
 
 static struct platform_driver imx8qxp_ldb_driver = {
 	.probe	= imx8qxp_ldb_probe,
-	.remove = imx8qxp_ldb_remove,
+	.remove_new = imx8qxp_ldb_remove,
 	.driver	= {
 		.pm = &imx8qxp_ldb_pm_ops,
 		.name = DRIVER_NAME,
-- 
2.39.2

[PATCH 08/19] drm/bridge: imx8qxp-pixel-combiner: 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/gpu/drm/bridge/imx/imx8qxp-pixel-combiner.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/bridge/imx/imx8qxp-pixel-combiner.c b/drivers/gpu/drm/bridge/imx/imx8qxp-pixel-combiner.c
index 503bd8db8afe..d0868a6ac6c9 100644
--- a/drivers/gpu/drm/bridge/imx/imx8qxp-pixel-combiner.c
+++ b/drivers/gpu/drm/bridge/imx/imx8qxp-pixel-combiner.c
@@ -357,7 +357,7 @@ static int imx8qxp_pc_bridge_probe(struct platform_device *pdev)
 	return ret;
 }
 
-static int imx8qxp_pc_bridge_remove(struct platform_device *pdev)
+static void imx8qxp_pc_bridge_remove(struct platform_device *pdev)
 {
 	struct imx8qxp_pc *pc = platform_get_drvdata(pdev);
 	struct imx8qxp_pc_channel *ch;
@@ -374,8 +374,6 @@ static int imx8qxp_pc_bridge_remove(struct platform_device *pdev)
 	}
 
 	pm_runtime_disable(&pdev->dev);
-
-	return 0;
 }
 
 static int __maybe_unused imx8qxp_pc_runtime_suspend(struct device *dev)
@@ -435,7 +433,7 @@ MODULE_DEVICE_TABLE(of, imx8qxp_pc_dt_ids);
 
 static struct platform_driver imx8qxp_pc_bridge_driver = {
 	.probe	= imx8qxp_pc_bridge_probe,
-	.remove = imx8qxp_pc_bridge_remove,
+	.remove_new = imx8qxp_pc_bridge_remove,
 	.driver	= {
 		.pm = &imx8qxp_pc_pm_ops,
 		.name = DRIVER_NAME,
-- 
2.39.2

[PATCH 09/19] drm/bridge: imx8qxp-pixel-link: 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/gpu/drm/bridge/imx/imx8qxp-pixel-link.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/bridge/imx/imx8qxp-pixel-link.c b/drivers/gpu/drm/bridge/imx/imx8qxp-pixel-link.c
index 9e5f2b4dc2e5..25dc82a44ef4 100644
--- a/drivers/gpu/drm/bridge/imx/imx8qxp-pixel-link.c
+++ b/drivers/gpu/drm/bridge/imx/imx8qxp-pixel-link.c
@@ -398,13 +398,11 @@ static int imx8qxp_pixel_link_bridge_probe(struct platform_device *pdev)
 	return ret;
 }
 
-static int imx8qxp_pixel_link_bridge_remove(struct platform_device *pdev)
+static void imx8qxp_pixel_link_bridge_remove(struct platform_device *pdev)
 {
 	struct imx8qxp_pixel_link *pl = platform_get_drvdata(pdev);
 
 	drm_bridge_remove(&pl->bridge);
-
-	return 0;
 }
 
 static const struct of_device_id imx8qxp_pixel_link_dt_ids[] = {
@@ -416,7 +414,7 @@ MODULE_DEVICE_TABLE(of, imx8qxp_pixel_link_dt_ids);
 
 static struct platform_driver imx8qxp_pixel_link_bridge_driver = {
 	.probe	= imx8qxp_pixel_link_bridge_probe,
-	.remove = imx8qxp_pixel_link_bridge_remove,
+	.remove_new = imx8qxp_pixel_link_bridge_remove,
 	.driver	= {
 		.of_match_table = imx8qxp_pixel_link_dt_ids,
 		.name = DRIVER_NAME,
-- 
2.39.2

[PATCH 10/19] drm/bridge: imx8qxp-pxl2dpi: 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/gpu/drm/bridge/imx/imx8qxp-pxl2dpi.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/bridge/imx/imx8qxp-pxl2dpi.c b/drivers/gpu/drm/bridge/imx/imx8qxp-pxl2dpi.c
index d0fec82f0cf8..4a886cb808ca 100644
--- a/drivers/gpu/drm/bridge/imx/imx8qxp-pxl2dpi.c
+++ b/drivers/gpu/drm/bridge/imx/imx8qxp-pxl2dpi.c
@@ -455,15 +455,13 @@ static int imx8qxp_pxl2dpi_bridge_probe(struct platform_device *pdev)
 	return ret;
 }
 
-static int imx8qxp_pxl2dpi_bridge_remove(struct platform_device *pdev)
+static void imx8qxp_pxl2dpi_bridge_remove(struct platform_device *pdev)
 {
 	struct imx8qxp_pxl2dpi *p2d = platform_get_drvdata(pdev);
 
 	drm_bridge_remove(&p2d->bridge);
 
 	pm_runtime_disable(&pdev->dev);
-
-	return 0;
 }
 
 static const struct of_device_id imx8qxp_pxl2dpi_dt_ids[] = {
@@ -474,7 +472,7 @@ MODULE_DEVICE_TABLE(of, imx8qxp_pxl2dpi_dt_ids);
 
 static struct platform_driver imx8qxp_pxl2dpi_bridge_driver = {
 	.probe	= imx8qxp_pxl2dpi_bridge_probe,
-	.remove = imx8qxp_pxl2dpi_bridge_remove,
+	.remove_new = imx8qxp_pxl2dpi_bridge_remove,
 	.driver	= {
 		.of_match_table = imx8qxp_pxl2dpi_dt_ids,
 		.name = DRIVER_NAME,
-- 
2.39.2

[PATCH 11/19] drm/bridge: lvds-codec: 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/gpu/drm/bridge/lvds-codec.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/bridge/lvds-codec.c b/drivers/gpu/drm/bridge/lvds-codec.c
index 39e7004de720..67368f23d4aa 100644
--- a/drivers/gpu/drm/bridge/lvds-codec.c
+++ b/drivers/gpu/drm/bridge/lvds-codec.c
@@ -215,13 +215,11 @@ static int lvds_codec_probe(struct platform_device *pdev)
 	return 0;
 }
 
-static int lvds_codec_remove(struct platform_device *pdev)
+static void lvds_codec_remove(struct platform_device *pdev)
 {
 	struct lvds_codec *lvds_codec = platform_get_drvdata(pdev);
 
 	drm_bridge_remove(&lvds_codec->bridge);
-
-	return 0;
 }
 
 static const struct of_device_id lvds_codec_match[] = {
@@ -243,7 +241,7 @@ MODULE_DEVICE_TABLE(of, lvds_codec_match);
 
 static struct platform_driver lvds_codec_driver = {
 	.probe	= lvds_codec_probe,
-	.remove	= lvds_codec_remove,
+	.remove_new = lvds_codec_remove,
 	.driver		= {
 		.name		= "lvds-codec",
 		.of_match_table	= lvds_codec_match,
-- 
2.39.2

[PATCH 12/19] drm/bridge: nwl-dsi: 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/gpu/drm/bridge/nwl-dsi.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/bridge/nwl-dsi.c b/drivers/gpu/drm/bridge/nwl-dsi.c
index 6dc2a4e191d7..4a5f5c4f5dcc 100644
--- a/drivers/gpu/drm/bridge/nwl-dsi.c
+++ b/drivers/gpu/drm/bridge/nwl-dsi.c
@@ -1199,7 +1199,7 @@ static int nwl_dsi_probe(struct platform_device *pdev)
 	return 0;
 }
 
-static int nwl_dsi_remove(struct platform_device *pdev)
+static void nwl_dsi_remove(struct platform_device *pdev)
 {
 	struct nwl_dsi *dsi = platform_get_drvdata(pdev);
 
@@ -1207,12 +1207,11 @@ static int nwl_dsi_remove(struct platform_device *pdev)
 	mipi_dsi_host_unregister(&dsi->dsi_host);
 	drm_bridge_remove(&dsi->bridge);
 	pm_runtime_disable(&pdev->dev);
-	return 0;
 }
 
 static struct platform_driver nwl_dsi_driver = {
 	.probe		= nwl_dsi_probe,
-	.remove		= nwl_dsi_remove,
+	.remove_new	= nwl_dsi_remove,
 	.driver		= {
 		.of_match_table = nwl_dsi_dt_ids,
 		.name	= DRV_NAME,
-- 
2.39.2

[PATCH 13/19] drm/bridge: simple-bridg: 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/gpu/drm/bridge/simple-bridge.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/bridge/simple-bridge.c b/drivers/gpu/drm/bridge/simple-bridge.c
index 2c5c5211bdab..5ede3e111096 100644
--- a/drivers/gpu/drm/bridge/simple-bridge.c
+++ b/drivers/gpu/drm/bridge/simple-bridge.c
@@ -218,13 +218,11 @@ static int simple_bridge_probe(struct platform_device *pdev)
 	return 0;
 }
 
-static int simple_bridge_remove(struct platform_device *pdev)
+static void simple_bridge_remove(struct platform_device *pdev)
 {
 	struct simple_bridge *sbridge = platform_get_drvdata(pdev);
 
 	drm_bridge_remove(&sbridge->bridge);
-
-	return 0;
 }
 
 /*
@@ -301,7 +299,7 @@ MODULE_DEVICE_TABLE(of, simple_bridge_match);
 
 static struct platform_driver simple_bridge_driver = {
 	.probe	= simple_bridge_probe,
-	.remove	= simple_bridge_remove,
+	.remove_new = simple_bridge_remove,
 	.driver		= {
 		.name		= "simple-bridge",
 		.of_match_table	= simple_bridge_match,
-- 
2.39.2

[PATCH 14/19] drm/bridge: dw-hdmi-ahb-audio: 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/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c
index 4efb62bcdb63..67b8d17a722a 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c
@@ -584,13 +584,11 @@ static int snd_dw_hdmi_probe(struct platform_device *pdev)
 	return ret;
 }
 
-static int snd_dw_hdmi_remove(struct platform_device *pdev)
+static void snd_dw_hdmi_remove(struct platform_device *pdev)
 {
 	struct snd_dw_hdmi *dw = platform_get_drvdata(pdev);
 
 	snd_card_free(dw->card);
-
-	return 0;
 }
 
 #if defined(CONFIG_PM_SLEEP) && defined(IS_NOT_BROKEN)
@@ -625,7 +623,7 @@ static SIMPLE_DEV_PM_OPS(snd_dw_hdmi_pm, snd_dw_hdmi_suspend,
 
 static struct platform_driver snd_dw_hdmi_driver = {
 	.probe	= snd_dw_hdmi_probe,
-	.remove	= snd_dw_hdmi_remove,
+	.remove_new = snd_dw_hdmi_remove,
 	.driver	= {
 		.name = DRIVER_NAME,
 		.pm = PM_OPS,
-- 
2.39.2

[PATCH 15/19] drm/bridge: dw-hdmi-cec: 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/gpu/drm/bridge/synopsys/dw-hdmi-cec.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c
index c8f44bcb298a..9389ce526eb1 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c
@@ -296,19 +296,17 @@ static int dw_hdmi_cec_probe(struct platform_device *pdev)
 	return 0;
 }
 
-static int dw_hdmi_cec_remove(struct platform_device *pdev)
+static void dw_hdmi_cec_remove(struct platform_device *pdev)
 {
 	struct dw_hdmi_cec *cec = platform_get_drvdata(pdev);
 
 	cec_notifier_cec_adap_unregister(cec->notify, cec->adap);
 	cec_unregister_adapter(cec->adap);
-
-	return 0;
 }
 
 static struct platform_driver dw_hdmi_cec_driver = {
 	.probe	= dw_hdmi_cec_probe,
-	.remove	= dw_hdmi_cec_remove,
+	.remove_new = dw_hdmi_cec_remove,
 	.driver = {
 		.name = "dw-hdmi-cec",
 	},
-- 
2.39.2

[PATCH 16/19] drm/bridge: dw-hdmi-gp-audio: 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/gpu/drm/bridge/synopsys/dw-hdmi-gp-audio.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-gp-audio.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-gp-audio.c
index 557966239677..423762da2ab4 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-gp-audio.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-gp-audio.c
@@ -172,18 +172,16 @@ static int snd_dw_hdmi_probe(struct platform_device *pdev)
 	return PTR_ERR_OR_ZERO(dw->audio_pdev);
 }
 
-static int snd_dw_hdmi_remove(struct platform_device *pdev)
+static void snd_dw_hdmi_remove(struct platform_device *pdev)
 {
 	struct snd_dw_hdmi *dw = platform_get_drvdata(pdev);
 
 	platform_device_unregister(dw->audio_pdev);
-
-	return 0;
 }
 
 static struct platform_driver snd_dw_hdmi_driver = {
 	.probe	= snd_dw_hdmi_probe,
-	.remove	= snd_dw_hdmi_remove,
+	.remove_new = snd_dw_hdmi_remove,
 	.driver	= {
 		.name = DRIVER_NAME,
 	},
-- 
2.39.2

[PATCH 17/19] drm/bridge: dw-hdmi-i2s-audio: 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/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c
index d751820c6da6..26c187d20d97 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-i2s-audio.c
@@ -216,18 +216,16 @@ static int snd_dw_hdmi_probe(struct platform_device *pdev)
 	return 0;
 }
 
-static int snd_dw_hdmi_remove(struct platform_device *pdev)
+static void snd_dw_hdmi_remove(struct platform_device *pdev)
 {
 	struct platform_device *platform = dev_get_drvdata(&pdev->dev);
 
 	platform_device_unregister(platform);
-
-	return 0;
 }
 
 static struct platform_driver snd_dw_hdmi_driver = {
 	.probe	= snd_dw_hdmi_probe,
-	.remove	= snd_dw_hdmi_remove,
+	.remove_new = snd_dw_hdmi_remove,
 	.driver	= {
 		.name = DRIVER_NAME,
 	},
-- 
2.39.2

[PATCH 18/19] drm/bridge: thc63lvd1024: 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/gpu/drm/bridge/thc63lvd1024.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/bridge/thc63lvd1024.c b/drivers/gpu/drm/bridge/thc63lvd1024.c
index e21078b2f8b5..d4c1a601bbb5 100644
--- a/drivers/gpu/drm/bridge/thc63lvd1024.c
+++ b/drivers/gpu/drm/bridge/thc63lvd1024.c
@@ -230,13 +230,11 @@ static int thc63_probe(struct platform_device *pdev)
 	return 0;
 }
 
-static int thc63_remove(struct platform_device *pdev)
+static void thc63_remove(struct platform_device *pdev)
 {
 	struct thc63_dev *thc63 = platform_get_drvdata(pdev);
 
 	drm_bridge_remove(&thc63->bridge);
-
-	return 0;
 }
 
 static const struct of_device_id thc63_match[] = {
@@ -247,7 +245,7 @@ MODULE_DEVICE_TABLE(of, thc63_match);
 
 static struct platform_driver thc63_driver = {
 	.probe	= thc63_probe,
-	.remove	= thc63_remove,
+	.remove_new = thc63_remove,
 	.driver	= {
 		.name		= "thc63lvd1024",
 		.of_match_table	= thc63_match,
-- 
2.39.2

[PATCH 19/19] drm/bridge: ti-tfp410: 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/gpu/drm/bridge/ti-tfp410.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ti-tfp410.c b/drivers/gpu/drm/bridge/ti-tfp410.c
index 6db69df0e18b..ab63225cd635 100644
--- a/drivers/gpu/drm/bridge/ti-tfp410.c
+++ b/drivers/gpu/drm/bridge/ti-tfp410.c
@@ -355,11 +355,9 @@ static int tfp410_probe(struct platform_device *pdev)
 	return tfp410_init(&pdev->dev, false);
 }
 
-static int tfp410_remove(struct platform_device *pdev)
+static void tfp410_remove(struct platform_device *pdev)
 {
 	tfp410_fini(&pdev->dev);
-
-	return 0;
 }
 
 static const struct of_device_id tfp410_match[] = {
@@ -370,7 +368,7 @@ MODULE_DEVICE_TABLE(of, tfp410_match);
 
 static struct platform_driver tfp410_platform_driver = {
 	.probe	= tfp410_probe,
-	.remove	= tfp410_remove,
+	.remove_new = tfp410_remove,
 	.driver	= {
 		.name		= "tfp410-bridge",
 		.of_match_table	= tfp410_match,
-- 
2.39.2

Re: [PATCH 01/19] drm/bridge: cdns-mhdp8546: Improve error reporting in remove callback

[Laurent Pinchart]

Hi Uwe,

Thank you for the patch.

On Sat, Mar 18, 2023 at 08:07:46PM +0100, Uwe Kleine-König wrote:
> Replace the generic error message issued by the driver core when the remove
> callback returns non-zero ("remove callback returned a non-zero value. This
> will be ignored.") by a message that tells the actual problem.
> 
> Also simplify a bit by checking the return value of wait_event_timeout a
> bit later.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
>  .../drm/bridge/cadence/cdns-mhdp8546-core.c    | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> index f6822dfa3805..d74c6fa30ccc 100644
> --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> @@ -2574,7 +2574,6 @@ static int cdns_mhdp_remove(struct platform_device *pdev)
>  {
>  	struct cdns_mhdp_device *mhdp = platform_get_drvdata(pdev);
>  	unsigned long timeout = msecs_to_jiffies(100);
> -	bool stop_fw = false;
>  	int ret;
>  
>  	drm_bridge_remove(&mhdp->bridge);
> @@ -2582,18 +2581,19 @@ static int cdns_mhdp_remove(struct platform_device *pdev)
>  	ret = wait_event_timeout(mhdp->fw_load_wq,
>  				 mhdp->hw_state == MHDP_HW_READY,
>  				 timeout);
> -	if (ret == 0)
> -		dev_err(mhdp->dev, "%s: Timeout waiting for fw loading\n",
> -			__func__);
> -	else
> -		stop_fw = true;
> -
>  	spin_lock(&mhdp->start_lock);
>  	mhdp->hw_state = MHDP_HW_STOPPED;
>  	spin_unlock(&mhdp->start_lock);
>  
> -	if (stop_fw)
> +	if (ret == 0) {
> +		dev_err(mhdp->dev, "%s: Timeout waiting for fw loading\n",
> +			__func__);
> +	} else {
>  		ret = cdns_mhdp_set_firmware_active(mhdp, false);
> +		if (ret)
> +			dev_err(mhdp->dev, "Failed to stop firmware (%pe)\n",
> +				ERR_PTR(ret));

Why not simply
			dev_err(mhdp->dev, "Failed to stop firmware (%d)\n",
				ret);

? Apart from that,

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

> +	}
>  
>  	phy_exit(mhdp->phy);
>  
> @@ -2609,7 +2609,7 @@ static int cdns_mhdp_remove(struct platform_device *pdev)
>  
>  	clk_disable_unprepare(mhdp->clk);
>  
> -	return ret;
> +	return 0;
>  }
>  
>  static const struct of_device_id mhdp_ids[] = {

-- 
Regards,

Laurent Pinchart

Re: [PATCH 00/19] drm/bridge: Convert to platform remove callback returning void

[Laurent Pinchart]

Hi Uwe,

Thank you for the patches.

On Sat, Mar 18, 2023 at 08:07:45PM +0100, Uwe Kleine-König wrote:
> Hello,
> 
> this series adapts the platform drivers below drivers/gpu/drm/bridge 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 cdns-mhdp8546 driver to return zero in its
> remove callback unconditionally and thenn all drivers are trivially converted
> using coccinelle.
> 
> Only patch #3 depends on patch #1, the other are pairwise independant and can
> be applied individually.

For patches 02/19 to 19/19,

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

> Uwe Kleine-König (19):
>   drm/bridge: cdns-mhdp8546: Improve error reporting in remove callback
>   drm/bridge: cdns-dsi: Convert to platform remove callback returning
>     void
>   drm/bridge: cdns-mhdp8546: Convert to platform remove callback
>     returning void
>   drm/bridge: display-connector: Convert to platform remove callback
>     returning void
>   drm/bridge: fsl-ldb: Convert to platform remove callback returning
>     void
>   drm/bridge: imx8qm-ldb: Convert to platform remove callback returning
>     void
>   drm/bridge: imx8qxp-ldb: Convert to platform remove callback returning
>     void
>   drm/bridge: imx8qxp-pixel-combiner: Convert to platform remove
>     callback returning void
>   drm/bridge: imx8qxp-pixel-link: Convert to platform remove callback
>     returning void
>   drm/bridge: imx8qxp-pxl2dpi: Convert to platform remove callback
>     returning void
>   drm/bridge: lvds-codec: Convert to platform remove callback returning
>     void
>   drm/bridge: nwl-dsi: Convert to platform remove callback returning
>     void
>   drm/bridge: simple-bridg: Convert to platform remove callback
>     returning void
>   drm/bridge: dw-hdmi-ahb-audio: Convert to platform remove callback
>     returning void
>   drm/bridge: dw-hdmi-cec: Convert to platform remove callback returning
>     void
>   drm/bridge: dw-hdmi-gp-audio: Convert to platform remove callback
>     returning void
>   drm/bridge: dw-hdmi-i2s-audio: Convert to platform remove callback
>     returning void
>   drm/bridge: thc63lvd1024: Convert to platform remove callback
>     returning void
>   drm/bridge: ti-tfp410: Convert to platform remove callback returning
>     void
> 
>  .../gpu/drm/bridge/cadence/cdns-dsi-core.c    |  6 ++---
>  .../drm/bridge/cadence/cdns-mhdp8546-core.c   | 22 +++++++++----------
>  drivers/gpu/drm/bridge/display-connector.c    |  6 ++---
>  drivers/gpu/drm/bridge/fsl-ldb.c              |  6 ++---
>  drivers/gpu/drm/bridge/imx/imx8qm-ldb-drv.c   |  6 ++---
>  drivers/gpu/drm/bridge/imx/imx8qxp-ldb-drv.c  |  6 ++---
>  .../drm/bridge/imx/imx8qxp-pixel-combiner.c   |  6 ++---
>  .../gpu/drm/bridge/imx/imx8qxp-pixel-link.c   |  6 ++---
>  drivers/gpu/drm/bridge/imx/imx8qxp-pxl2dpi.c  |  6 ++---
>  drivers/gpu/drm/bridge/lvds-codec.c           |  6 ++---
>  drivers/gpu/drm/bridge/nwl-dsi.c              |  5 ++---
>  drivers/gpu/drm/bridge/simple-bridge.c        |  6 ++---
>  .../drm/bridge/synopsys/dw-hdmi-ahb-audio.c   |  6 ++---
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c |  6 ++---
>  .../drm/bridge/synopsys/dw-hdmi-gp-audio.c    |  6 ++---
>  .../drm/bridge/synopsys/dw-hdmi-i2s-audio.c   |  6 ++---
>  drivers/gpu/drm/bridge/thc63lvd1024.c         |  6 ++---
>  drivers/gpu/drm/bridge/ti-tfp410.c            |  6 ++---
>  18 files changed, 44 insertions(+), 79 deletions(-)
> 
> 
> base-commit: fe15c26ee26efa11741a7b632e9f23b01aca4cc6

-- 
Regards,

Laurent Pinchart

Re: [PATCH 01/19] drm/bridge: cdns-mhdp8546: Improve error reporting in remove callback

[Uwe Kleine-König]

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

On Sun, Mar 19, 2023 at 03:13:01PM +0200, Laurent Pinchart wrote:
> Hi Uwe,
> 
> Thank you for the patch.
> 
> On Sat, Mar 18, 2023 at 08:07:46PM +0100, Uwe Kleine-König wrote:
> > Replace the generic error message issued by the driver core when the remove
> > callback returns non-zero ("remove callback returned a non-zero value. This
> > will be ignored.") by a message that tells the actual problem.
> > 
> > Also simplify a bit by checking the return value of wait_event_timeout a
> > bit later.
> > 
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > ---
> >  .../drm/bridge/cadence/cdns-mhdp8546-core.c    | 18 +++++++++---------
> >  1 file changed, 9 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> > index f6822dfa3805..d74c6fa30ccc 100644
> > --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> > +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> > @@ -2574,7 +2574,6 @@ static int cdns_mhdp_remove(struct platform_device *pdev)
> >  {
> >  	struct cdns_mhdp_device *mhdp = platform_get_drvdata(pdev);
> >  	unsigned long timeout = msecs_to_jiffies(100);
> > -	bool stop_fw = false;
> >  	int ret;
> >  
> >  	drm_bridge_remove(&mhdp->bridge);
> > @@ -2582,18 +2581,19 @@ static int cdns_mhdp_remove(struct platform_device *pdev)
> >  	ret = wait_event_timeout(mhdp->fw_load_wq,
> >  				 mhdp->hw_state == MHDP_HW_READY,
> >  				 timeout);
> > -	if (ret == 0)
> > -		dev_err(mhdp->dev, "%s: Timeout waiting for fw loading\n",
> > -			__func__);
> > -	else
> > -		stop_fw = true;
> > -
> >  	spin_lock(&mhdp->start_lock);
> >  	mhdp->hw_state = MHDP_HW_STOPPED;
> >  	spin_unlock(&mhdp->start_lock);
> >  
> > -	if (stop_fw)
> > +	if (ret == 0) {
> > +		dev_err(mhdp->dev, "%s: Timeout waiting for fw loading\n",
> > +			__func__);
> > +	} else {
> >  		ret = cdns_mhdp_set_firmware_active(mhdp, false);
> > +		if (ret)
> > +			dev_err(mhdp->dev, "Failed to stop firmware (%pe)\n",
> > +				ERR_PTR(ret));
> 
> Why not simply
> 			dev_err(mhdp->dev, "Failed to stop firmware (%d)\n",
> 				ret);
> 
> ? Apart from that,

%pe is superior to %d because

	Failed to stop firmware (EIO)

is easier to understand for humans than

	Failed to stop firmware (-5)

. Don't you agree?

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

Re: [PATCH 01/19] drm/bridge: cdns-mhdp8546: Improve error reporting in remove callback

[Laurent Pinchart]

Hi Uwe,

On Sun, Mar 19, 2023 at 02:59:21PM +0100, Uwe Kleine-König wrote:
> On Sun, Mar 19, 2023 at 03:13:01PM +0200, Laurent Pinchart wrote:
> > On Sat, Mar 18, 2023 at 08:07:46PM +0100, Uwe Kleine-König wrote:
> > > Replace the generic error message issued by the driver core when the remove
> > > callback returns non-zero ("remove callback returned a non-zero value. This
> > > will be ignored.") by a message that tells the actual problem.
> > > 
> > > Also simplify a bit by checking the return value of wait_event_timeout a
> > > bit later.
> > > 
> > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > > ---
> > >  .../drm/bridge/cadence/cdns-mhdp8546-core.c    | 18 +++++++++---------
> > >  1 file changed, 9 insertions(+), 9 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> > > index f6822dfa3805..d74c6fa30ccc 100644
> > > --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> > > +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
> > > @@ -2574,7 +2574,6 @@ static int cdns_mhdp_remove(struct platform_device *pdev)
> > >  {
> > >  	struct cdns_mhdp_device *mhdp = platform_get_drvdata(pdev);
> > >  	unsigned long timeout = msecs_to_jiffies(100);
> > > -	bool stop_fw = false;
> > >  	int ret;
> > >  
> > >  	drm_bridge_remove(&mhdp->bridge);
> > > @@ -2582,18 +2581,19 @@ static int cdns_mhdp_remove(struct platform_device *pdev)
> > >  	ret = wait_event_timeout(mhdp->fw_load_wq,
> > >  				 mhdp->hw_state == MHDP_HW_READY,
> > >  				 timeout);
> > > -	if (ret == 0)
> > > -		dev_err(mhdp->dev, "%s: Timeout waiting for fw loading\n",
> > > -			__func__);
> > > -	else
> > > -		stop_fw = true;
> > > -
> > >  	spin_lock(&mhdp->start_lock);
> > >  	mhdp->hw_state = MHDP_HW_STOPPED;
> > >  	spin_unlock(&mhdp->start_lock);
> > >  
> > > -	if (stop_fw)
> > > +	if (ret == 0) {
> > > +		dev_err(mhdp->dev, "%s: Timeout waiting for fw loading\n",
> > > +			__func__);
> > > +	} else {
> > >  		ret = cdns_mhdp_set_firmware_active(mhdp, false);
> > > +		if (ret)
> > > +			dev_err(mhdp->dev, "Failed to stop firmware (%pe)\n",
> > > +				ERR_PTR(ret));
> > 
> > Why not simply
> > 			dev_err(mhdp->dev, "Failed to stop firmware (%d)\n",
> > 				ret);
> > 
> > ? Apart from that,
> 
> %pe is superior to %d because
> 
> 	Failed to stop firmware (EIO)
> 
> is easier to understand for humans than
> 
> 	Failed to stop firmware (-5)
> 
> . Don't you agree?

Partly :) When I try to match error codes received in userspace with
kernel log messages, or debug core dumps, numerical errors are easier.
At other times, the error name is likely better. I don't have a string
preference here.

-- 
Regards,

Laurent Pinchart

Re: (subset) [PATCH 00/19] drm/bridge: Convert to platform remove callback returning void

[Neil Armstrong]

Hi,

On Sat, 18 Mar 2023 20:07:45 +0100, Uwe Kleine-König wrote:
> this series adapts the platform drivers below drivers/gpu/drm/bridge 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.
> 
> [...]

Thanks, Applied to https://anongit.freedesktop.org/git/drm/drm-misc.git (drm-misc-next)

[02/19] drm/bridge: cdns-dsi: Convert to platform remove callback returning void
        https://cgit.freedesktop.org/drm/drm-misc/commit/?id=d5701d8f5210f8c70ffd1b51283b2373573e7b5a
[04/19] drm/bridge: display-connector: Convert to platform remove callback returning void
        https://cgit.freedesktop.org/drm/drm-misc/commit/?id=d1837136d184dda0585a687c614a9117cc87ad27
[05/19] drm/bridge: fsl-ldb: Convert to platform remove callback returning void
        https://cgit.freedesktop.org/drm/drm-misc/commit/?id=be5c6b177229c5cb40f3fef785250afbfd669175
[06/19] drm/bridge: imx8qm-ldb: Convert to platform remove callback returning void
        https://cgit.freedesktop.org/drm/drm-misc/commit/?id=04751a061ea055c4fa264f20f4558073c3d8fb2e
[07/19] drm/bridge: imx8qxp-ldb: Convert to platform remove callback returning void
        https://cgit.freedesktop.org/drm/drm-misc/commit/?id=3a39841322be5502f5355a558617f0a56b2b3ad2
[08/19] drm/bridge: imx8qxp-pixel-combiner: Convert to platform remove callback returning void
        https://cgit.freedesktop.org/drm/drm-misc/commit/?id=b931c166c47207c8c7bc7eda6c8881dfe0241389
[09/19] drm/bridge: imx8qxp-pixel-link: Convert to platform remove callback returning void
        https://cgit.freedesktop.org/drm/drm-misc/commit/?id=2b438065c3e538844a3b2b56f567303020bf97de
[10/19] drm/bridge: imx8qxp-pxl2dpi: Convert to platform remove callback returning void
        https://cgit.freedesktop.org/drm/drm-misc/commit/?id=23a92abbf3ad44058b5d3cf57937cfb9de972c05
[11/19] drm/bridge: lvds-codec: Convert to platform remove callback returning void
        https://cgit.freedesktop.org/drm/drm-misc/commit/?id=f328e96b764a603b3b70f0e9ef6ee052a78ed2ad
[12/19] drm/bridge: nwl-dsi: Convert to platform remove callback returning void
        https://cgit.freedesktop.org/drm/drm-misc/commit/?id=6780b94d8ed37a0f453a5bc90821ea669ac3c0d2
[13/19] drm/bridge: simple-bridg: Convert to platform remove callback returning void
        https://cgit.freedesktop.org/drm/drm-misc/commit/?id=3fce4c948e2b73ba6f6f2aa7e1c2f101b997e192
[14/19] drm/bridge: dw-hdmi-ahb-audio: Convert to platform remove callback returning void
        https://cgit.freedesktop.org/drm/drm-misc/commit/?id=e85f436824f2e2e067c0a151e954cb403b483234
[15/19] drm/bridge: dw-hdmi-cec: Convert to platform remove callback returning void
        https://cgit.freedesktop.org/drm/drm-misc/commit/?id=8891698273b125e57d0d2b6a91451bac06e968be
[16/19] drm/bridge: dw-hdmi-gp-audio: Convert to platform remove callback returning void
        https://cgit.freedesktop.org/drm/drm-misc/commit/?id=2b9efaed4e4045a40944265f99f52b232df3347f
[17/19] drm/bridge: dw-hdmi-i2s-audio: Convert to platform remove callback returning void
        https://cgit.freedesktop.org/drm/drm-misc/commit/?id=ed58ee126c4da81af79ab40b7c6508a7100c7eac
[18/19] drm/bridge: thc63lvd1024: Convert to platform remove callback returning void
        https://cgit.freedesktop.org/drm/drm-misc/commit/?id=5e3ea76492265fdbadd7a58963ff2a3a2e2e8be4
[19/19] drm/bridge: ti-tfp410: Convert to platform remove callback returning void
        https://cgit.freedesktop.org/drm/drm-misc/commit/?id=67fcd5d2f3ec364cb7af2c307ed1999bedebb606

-- 
Neil

Re: (subset) [PATCH 00/19] drm/bridge: Convert to platform remove callback returning void

[Uwe Kleine-König]

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

Hello Neil,

On Mon, Mar 20, 2023 at 11:54:12AM +0100, Neil Armstrong wrote:
> On Sat, 18 Mar 2023 20:07:45 +0100, Uwe Kleine-König wrote:
> > this series adapts the platform drivers below drivers/gpu/drm/bridge 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.
> > 
> > [...]
> 
> Thanks, Applied to https://anongit.freedesktop.org/git/drm/drm-misc.git (drm-misc-next)
> 
> [02/19] drm/bridge: cdns-dsi: Convert to platform remove callback returning void
>         https://cgit.freedesktop.org/drm/drm-misc/commit/?id=d5701d8f5210f8c70ffd1b51283b2373573e7b5a
> [04/19] drm/bridge: display-connector: Convert to platform remove callback returning void
>         https://cgit.freedesktop.org/drm/drm-misc/commit/?id=d1837136d184dda0585a687c614a9117cc87ad27
> [05/19] drm/bridge: fsl-ldb: Convert to platform remove callback returning void
>         https://cgit.freedesktop.org/drm/drm-misc/commit/?id=be5c6b177229c5cb40f3fef785250afbfd669175
> [...]

I wonder what made you skip patches #1 and #3. I got some feedback by
Laurent for patch #1. In the end it was Laurent's "I don't have a string
preference here." vs me thinking that the patch is good as is. Was that
the reason to skip it? Patch #3 depends on patch #1.

Would it help to resent?

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