[PATCH 0/4] gpu: Convert to platform remove callback returning void

[PATCH 0/4] gpu: Convert to platform remove callback returning void

[Uwe Kleine-König]

Hello,

with some patches sent earlier[1], this series converts all platform
drivers below drivers/gpu to not use struct platform_device::remove()
any more.

See commit 5c5a7680e67b ("platform: Provide a remove callback that
returns no value") for an extended explanation and the eventual goal.

All conversations are trivial, because the driver's .remove() callbacks
returned zero unconditionally.

There are no interdependencies between these patches. This is merge
window material.

Best regards
Uwe

Uwe Kleine-König (4):
  drm/imagination: Convert to platform remove callback returning void
  drm/mediatek: Convert to platform remove callback returning void
  gpu: host1x: Convert to platform remove callback returning void
  gpu: ipu-v3: Convert to platform remove callback returning void

 drivers/gpu/drm/imagination/pvr_drv.c  | 7 ++-----
 drivers/gpu/drm/mediatek/mtk_padding.c | 5 ++---
 drivers/gpu/host1x/dev.c               | 6 ++----
 drivers/gpu/ipu-v3/ipu-common.c        | 6 ++----
 drivers/gpu/ipu-v3/ipu-pre.c           | 5 ++---
 drivers/gpu/ipu-v3/ipu-prg.c           | 6 ++----
 6 files changed, 12 insertions(+), 23 deletions(-)

base-commit: a053fd3ca5d1b927a8655f239c84b0d790218fda
-- 
2.43.0

[PATCH 1/4] drm/imagination: 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 ignored (apart
from emitting a warning) 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. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().

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/imagination/pvr_drv.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/imagination/pvr_drv.c b/drivers/gpu/drm/imagination/pvr_drv.c
index 5c3b2d58d766..1a0cb7aa9cea 100644
--- a/drivers/gpu/drm/imagination/pvr_drv.c
+++ b/drivers/gpu/drm/imagination/pvr_drv.c
@@ -1451,8 +1451,7 @@ pvr_probe(struct platform_device *plat_dev)
 	return err;
 }
 
-static int
-pvr_remove(struct platform_device *plat_dev)
+static void pvr_remove(struct platform_device *plat_dev)
 {
 	struct drm_device *drm_dev = platform_get_drvdata(plat_dev);
 	struct pvr_device *pvr_dev = to_pvr_device(drm_dev);
@@ -1469,8 +1468,6 @@ pvr_remove(struct platform_device *plat_dev)
 	pvr_watchdog_fini(pvr_dev);
 	pvr_queue_device_fini(pvr_dev);
 	pvr_context_device_fini(pvr_dev);
-
-	return 0;
 }
 
 static const struct of_device_id dt_match[] = {
@@ -1485,7 +1482,7 @@ static const struct dev_pm_ops pvr_pm_ops = {
 
 static struct platform_driver pvr_driver = {
 	.probe = pvr_probe,
-	.remove = pvr_remove,
+	.remove_new = pvr_remove,
 	.driver = {
 		.name = PVR_DRIVER_NAME,
 		.pm = &pvr_pm_ops,
-- 
2.43.0

[PATCH 2/4] drm/mediatek: 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 ignored (apart
from emitting a warning) 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. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().

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/mediatek/mtk_padding.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_padding.c b/drivers/gpu/drm/mediatek/mtk_padding.c
index 0d6451c149b6..9f92b720aaae 100644
--- a/drivers/gpu/drm/mediatek/mtk_padding.c
+++ b/drivers/gpu/drm/mediatek/mtk_padding.c
@@ -137,10 +137,9 @@ static int mtk_padding_probe(struct platform_device *pdev)
 	return 0;
 }
 
-static int mtk_padding_remove(struct platform_device *pdev)
+static void mtk_padding_remove(struct platform_device *pdev)
 {
 	component_del(&pdev->dev, &mtk_padding_component_ops);
-	return 0;
 }
 
 static const struct of_device_id mtk_padding_driver_dt_match[] = {
@@ -151,7 +150,7 @@ MODULE_DEVICE_TABLE(of, mtk_padding_driver_dt_match);
 
 struct platform_driver mtk_padding_driver = {
 	.probe		= mtk_padding_probe,
-	.remove		= mtk_padding_remove,
+	.remove_new	= mtk_padding_remove,
 	.driver		= {
 		.name	= "mediatek-disp-padding",
 		.owner	= THIS_MODULE,
-- 
2.43.0

[PATCH 3/4] gpu: host1x: 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 ignored (apart
from emitting a warning) 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. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().

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/host1x/dev.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/host1x/dev.c b/drivers/gpu/host1x/dev.c
index 89983d7d73ca..bc02aa28458b 100644
--- a/drivers/gpu/host1x/dev.c
+++ b/drivers/gpu/host1x/dev.c
@@ -653,7 +653,7 @@ static int host1x_probe(struct platform_device *pdev)
 	return err;
 }
 
-static int host1x_remove(struct platform_device *pdev)
+static void host1x_remove(struct platform_device *pdev)
 {
 	struct host1x *host = platform_get_drvdata(pdev);
 
@@ -668,8 +668,6 @@ static int host1x_remove(struct platform_device *pdev)
 	host1x_channel_list_free(&host->channel_list);
 	host1x_iommu_exit(host);
 	host1x_bo_cache_destroy(&host->cache);
-
-	return 0;
 }
 
 static int __maybe_unused host1x_runtime_suspend(struct device *dev)
@@ -754,7 +752,7 @@ static struct platform_driver tegra_host1x_driver = {
 		.pm = &host1x_pm_ops,
 	},
 	.probe = host1x_probe,
-	.remove = host1x_remove,
+	.remove_new = host1x_remove,
 };
 
 static struct platform_driver * const drivers[] = {
-- 
2.43.0

[PATCH 4/4] gpu: ipu-v3: 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 ignored (apart
from emitting a warning) 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. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().

Trivially convert the ipu-v3 platform drivers 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/ipu-v3/ipu-common.c | 6 ++----
 drivers/gpu/ipu-v3/ipu-pre.c    | 5 ++---
 drivers/gpu/ipu-v3/ipu-prg.c    | 6 ++----
 3 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/ipu-v3/ipu-common.c b/drivers/gpu/ipu-v3/ipu-common.c
index 71ec1e7f657a..3535be9daa1f 100644
--- a/drivers/gpu/ipu-v3/ipu-common.c
+++ b/drivers/gpu/ipu-v3/ipu-common.c
@@ -1450,7 +1450,7 @@ static int ipu_probe(struct platform_device *pdev)
 	return ret;
 }
 
-static int ipu_remove(struct platform_device *pdev)
+static void ipu_remove(struct platform_device *pdev)
 {
 	struct ipu_soc *ipu = platform_get_drvdata(pdev);
 
@@ -1459,8 +1459,6 @@ static int ipu_remove(struct platform_device *pdev)
 	ipu_irq_exit(ipu);
 
 	clk_disable_unprepare(ipu->clk);
-
-	return 0;
 }
 
 static struct platform_driver imx_ipu_driver = {
@@ -1469,7 +1467,7 @@ static struct platform_driver imx_ipu_driver = {
 		.of_match_table = imx_ipu_dt_ids,
 	},
 	.probe = ipu_probe,
-	.remove = ipu_remove,
+	.remove_new = ipu_remove,
 };
 
 static struct platform_driver * const drivers[] = {
diff --git a/drivers/gpu/ipu-v3/ipu-pre.c b/drivers/gpu/ipu-v3/ipu-pre.c
index aef984a43190..e469272d4f25 100644
--- a/drivers/gpu/ipu-v3/ipu-pre.c
+++ b/drivers/gpu/ipu-v3/ipu-pre.c
@@ -312,7 +312,7 @@ static int ipu_pre_probe(struct platform_device *pdev)
 	return 0;
 }
 
-static int ipu_pre_remove(struct platform_device *pdev)
+static void ipu_pre_remove(struct platform_device *pdev)
 {
 	struct ipu_pre *pre = platform_get_drvdata(pdev);
 
@@ -326,7 +326,6 @@ static int ipu_pre_remove(struct platform_device *pdev)
 	if (pre->buffer_virt)
 		gen_pool_free(pre->iram, (unsigned long)pre->buffer_virt,
 			      IPU_PRE_MAX_WIDTH * IPU_PRE_NUM_SCANLINES * 4);
-	return 0;
 }
 
 static const struct of_device_id ipu_pre_dt_ids[] = {
@@ -336,7 +335,7 @@ static const struct of_device_id ipu_pre_dt_ids[] = {
 
 struct platform_driver ipu_pre_drv = {
 	.probe		= ipu_pre_probe,
-	.remove		= ipu_pre_remove,
+	.remove_new	= ipu_pre_remove,
 	.driver		= {
 		.name	= "imx-ipu-pre",
 		.of_match_table = ipu_pre_dt_ids,
diff --git a/drivers/gpu/ipu-v3/ipu-prg.c b/drivers/gpu/ipu-v3/ipu-prg.c
index 729605709955..4976ac0bb876 100644
--- a/drivers/gpu/ipu-v3/ipu-prg.c
+++ b/drivers/gpu/ipu-v3/ipu-prg.c
@@ -419,15 +419,13 @@ static int ipu_prg_probe(struct platform_device *pdev)
 	return 0;
 }
 
-static int ipu_prg_remove(struct platform_device *pdev)
+static void ipu_prg_remove(struct platform_device *pdev)
 {
 	struct ipu_prg *prg = platform_get_drvdata(pdev);
 
 	mutex_lock(&ipu_prg_list_mutex);
 	list_del(&prg->list);
 	mutex_unlock(&ipu_prg_list_mutex);
-
-	return 0;
 }
 
 #ifdef CONFIG_PM
@@ -471,7 +469,7 @@ static const struct of_device_id ipu_prg_dt_ids[] = {
 
 struct platform_driver ipu_prg_drv = {
 	.probe		= ipu_prg_probe,
-	.remove		= ipu_prg_remove,
+	.remove_new	= ipu_prg_remove,
 	.driver		= {
 		.name	= "imx-ipu-prg",
 		.pm	= &prg_pm_ops,
-- 
2.43.0

Re: [PATCH 0/4] gpu: Convert to platform remove callback returning void

[Uwe Kleine-König]

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

On Tue, Apr 09, 2024 at 07:02:47PM +0200, Uwe Kleine-König wrote:
> Hello,
> 
> with some patches sent earlier[1], this series converts all platform
> drivers below drivers/gpu to not use struct platform_device::remove()
> any more.

I forgot to include footnote with the list of earlier patches. For
completeness:

[1]:
	https://lore.kernel.org/dri-devel/20240409165043.105137-2-u.kleine-koenig@pengutronix.de
	https://lore.kernel.org/dri-devel/20240304091005.717012-2-u.kleine-koenig@pengutronix.de
	https://lore.kernel.org/dri-devel/20240304090555.716327-2-u.kleine-koenig@pengutronix.de

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 0/4] gpu: Convert to platform remove callback returning void

[Thomas Zimmermann]

Hi

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>

for the series.

Best regards
Thomas

Am 09.04.24 um 19:02 schrieb Uwe Kleine-König:
> Hello,
>
> with some patches sent earlier[1], this series converts all platform
> drivers below drivers/gpu to not use struct platform_device::remove()
> any more.
>
> See commit 5c5a7680e67b ("platform: Provide a remove callback that
> returns no value") for an extended explanation and the eventual goal.
>
> All conversations are trivial, because the driver's .remove() callbacks
> returned zero unconditionally.
>
> There are no interdependencies between these patches. This is merge
> window material.
>
> Best regards
> Uwe
>
> Uwe Kleine-König (4):
>    drm/imagination: Convert to platform remove callback returning void
>    drm/mediatek: Convert to platform remove callback returning void
>    gpu: host1x: Convert to platform remove callback returning void
>    gpu: ipu-v3: Convert to platform remove callback returning void
>
>   drivers/gpu/drm/imagination/pvr_drv.c  | 7 ++-----
>   drivers/gpu/drm/mediatek/mtk_padding.c | 5 ++---
>   drivers/gpu/host1x/dev.c               | 6 ++----
>   drivers/gpu/ipu-v3/ipu-common.c        | 6 ++----
>   drivers/gpu/ipu-v3/ipu-pre.c           | 5 ++---
>   drivers/gpu/ipu-v3/ipu-prg.c           | 6 ++----
>   6 files changed, 12 insertions(+), 23 deletions(-)
>
> base-commit: a053fd3ca5d1b927a8655f239c84b0d790218fda

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

Re: [PATCH 4/4] gpu: ipu-v3: Convert to platform remove callback returning void

[Philipp Zabel]

On Di, 2024-04-09 at 19:02 +0200, 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 ignored (apart
> from emitting a warning) 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. Eventually after all drivers
> are converted, .remove_new() will be renamed to .remove().
> 
> Trivially convert the ipu-v3 platform drivers 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: Philipp Zabel <p.zabel@pengutronix.de>

regards
Philipp

Re: [PATCH 1/4] drm/imagination: Convert to platform remove callback returning void

[Matt Coster]

[-- Attachment #1.1: Type: text/plain, Size: 2119 bytes --]

On 09/04/2024 10:02, 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 ignored (apart
> from emitting a warning) 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. Eventually after all drivers
> are converted, .remove_new() will be renamed to .remove().
> 
> 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: Matt Coster <matt.coster@imgtec.com>

> ---
>  drivers/gpu/drm/imagination/pvr_drv.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/imagination/pvr_drv.c b/drivers/gpu/drm/imagination/pvr_drv.c
> index 5c3b2d58d766..1a0cb7aa9cea 100644
> --- a/drivers/gpu/drm/imagination/pvr_drv.c
> +++ b/drivers/gpu/drm/imagination/pvr_drv.c
> @@ -1451,8 +1451,7 @@ pvr_probe(struct platform_device *plat_dev)
>  	return err;
>  }
>  
> -static int
> -pvr_remove(struct platform_device *plat_dev)
> +static void pvr_remove(struct platform_device *plat_dev)
>  {
>  	struct drm_device *drm_dev = platform_get_drvdata(plat_dev);
>  	struct pvr_device *pvr_dev = to_pvr_device(drm_dev);
> @@ -1469,8 +1468,6 @@ pvr_remove(struct platform_device *plat_dev)
>  	pvr_watchdog_fini(pvr_dev);
>  	pvr_queue_device_fini(pvr_dev);
>  	pvr_context_device_fini(pvr_dev);
> -
> -	return 0;
>  }
>  
>  static const struct of_device_id dt_match[] = {
> @@ -1485,7 +1482,7 @@ static const struct dev_pm_ops pvr_pm_ops = {
>  
>  static struct platform_driver pvr_driver = {
>  	.probe = pvr_probe,
> -	.remove = pvr_remove,
> +	.remove_new = pvr_remove,
>  	.driver = {
>  		.name = PVR_DRIVER_NAME,
>  		.pm = &pvr_pm_ops,

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

Re: [PATCH 0/4] gpu: Convert to platform remove callback returning void

[Uwe Kleine-König]

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

Hello,

On Tue, Apr 09, 2024 at 07:02:47PM +0200, Uwe Kleine-König wrote:
> with some patches sent earlier[1], this series converts all platform
> drivers below drivers/gpu to not use struct platform_device::remove()
> any more.
> 
> See commit 5c5a7680e67b ("platform: Provide a remove callback that
> returns no value") for an extended explanation and the eventual goal.
> 
> All conversations are trivial, because the driver's .remove() callbacks
> returned zero unconditionally.
> 
> There are no interdependencies between these patches. This is merge
> window material.

I wonder how this series will make it in. While I would prefer these
patches to go in together (that I can consider this thread completed in
one go), I think with how drm maintenace works, it's best if the patches
are picked up by their individual maintainers. I guess that's:

 - Frank Binns + Matt Coster for imagination

 - Chun-Kuang Hu + Philipp Zabel for mediatek

 - Thierry Reding + Mikko Perttunen for the host1x driver
   (Note there is another patch for this driver set at
    20240409165043.105137-2-u.kleine-koenig@pengutronix.de that is
    relevant for the same quest.)

 - Philipp Zabel for ipu-v3

I plan to send a patch changing struct platform_driver::remove after the
end of the merge window leading to 6.10-rc1 for inclusion in next via
Greg's driver core. So please either care the patches land in 6.10-rc1
or ack that I include them in the submission to Greg.

Thanks for your cooperation,
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 0/4] gpu: Convert to platform remove callback returning void

[Thierry Reding]

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

On Fri Apr 19, 2024 at 9:20 AM CEST, Uwe Kleine-König wrote:
> Hello,
>
> On Tue, Apr 09, 2024 at 07:02:47PM +0200, Uwe Kleine-König wrote:
> > with some patches sent earlier[1], this series converts all platform
> > drivers below drivers/gpu to not use struct platform_device::remove()
> > any more.
> > 
> > See commit 5c5a7680e67b ("platform: Provide a remove callback that
> > returns no value") for an extended explanation and the eventual goal.
> > 
> > All conversations are trivial, because the driver's .remove() callbacks
> > returned zero unconditionally.
> > 
> > There are no interdependencies between these patches. This is merge
> > window material.
>
> I wonder how this series will make it in. While I would prefer these
> patches to go in together (that I can consider this thread completed in
> one go), I think with how drm maintenace works, it's best if the patches
> are picked up by their individual maintainers. I guess that's:
>
>  - Frank Binns + Matt Coster for imagination
>
>  - Chun-Kuang Hu + Philipp Zabel for mediatek
>
>  - Thierry Reding + Mikko Perttunen for the host1x driver
>    (Note there is another patch for this driver set at
>     20240409165043.105137-2-u.kleine-koenig@pengutronix.de that is
>     relevant for the same quest.)
>
>  - Philipp Zabel for ipu-v3
>
> I plan to send a patch changing struct platform_driver::remove after the
> end of the merge window leading to 6.10-rc1 for inclusion in next via
> Greg's driver core. So please either care the patches land in 6.10-rc1
> or ack that I include them in the submission to Greg.

I think the latter would make more sense. I'll go ack those patches.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 3/4] gpu: host1x: Convert to platform remove callback returning void

[Thierry Reding]

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

On Tue Apr 9, 2024 at 7:02 PM CEST, 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 ignored (apart
> from emitting a warning) 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. Eventually after all drivers
> are converted, .remove_new() will be renamed to .remove().
>
> 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/host1x/dev.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)

Acked-by: Thierry Reding <treding@nvidia.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 1/4] drm/imagination: Convert to platform remove callback returning void

[Matt Coster]

[-- Attachment #1.1.1: Type: text/plain, Size: 2114 bytes --]

On 09/04/2024 18:02, 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 ignored (apart
> from emitting a warning) 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. Eventually after all drivers
> are converted, .remove_new() will be renamed to .remove().
> 
> 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: Matt Coster <matt.coster@imgtec.com>

> ---
>  drivers/gpu/drm/imagination/pvr_drv.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/imagination/pvr_drv.c b/drivers/gpu/drm/imagination/pvr_drv.c
> index 5c3b2d58d766..1a0cb7aa9cea 100644
> --- a/drivers/gpu/drm/imagination/pvr_drv.c
> +++ b/drivers/gpu/drm/imagination/pvr_drv.c
> @@ -1451,8 +1451,7 @@ pvr_probe(struct platform_device *plat_dev)
>  	return err;
>  }
>  
> -static int
> -pvr_remove(struct platform_device *plat_dev)
> +static void pvr_remove(struct platform_device *plat_dev)
>  {
>  	struct drm_device *drm_dev = platform_get_drvdata(plat_dev);
>  	struct pvr_device *pvr_dev = to_pvr_device(drm_dev);
> @@ -1469,8 +1468,6 @@ pvr_remove(struct platform_device *plat_dev)
>  	pvr_watchdog_fini(pvr_dev);
>  	pvr_queue_device_fini(pvr_dev);
>  	pvr_context_device_fini(pvr_dev);
> -
> -	return 0;
>  }
>  
>  static const struct of_device_id dt_match[] = {
> @@ -1485,7 +1482,7 @@ static const struct dev_pm_ops pvr_pm_ops = {
>  
>  static struct platform_driver pvr_driver = {
>  	.probe = pvr_probe,
> -	.remove = pvr_remove,
> +	.remove_new = pvr_remove,
>  	.driver = {
>  		.name = PVR_DRIVER_NAME,
>  		.pm = &pvr_pm_ops,

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 1817 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

Re: [PATCH 0/4] gpu: Convert to platform remove callback returning void

[Matt Coster]

[-- Attachment #1.1: Type: text/plain, Size: 1810 bytes --]

On 19/04/2024 08:20, Uwe Kleine-König wrote:
> Hello,
> 
> On Tue, Apr 09, 2024 at 07:02:47PM +0200, Uwe Kleine-König wrote:
>> with some patches sent earlier[1], this series converts all platform
>> drivers below drivers/gpu to not use struct platform_device::remove()
>> any more.
>>
>> See commit 5c5a7680e67b ("platform: Provide a remove callback that
>> returns no value") for an extended explanation and the eventual goal.
>>
>> All conversations are trivial, because the driver's .remove() callbacks
>> returned zero unconditionally.
>>
>> There are no interdependencies between these patches. This is merge
>> window material.
> 
> I wonder how this series will make it in. While I would prefer these
> patches to go in together (that I can consider this thread completed in
> one go), I think with how drm maintenace works, it's best if the patches
> are picked up by their individual maintainers. I guess that's:
> 
>  - Frank Binns + Matt Coster for imagination

I've acked the imagination patch - feel free to land it however you
like. We don't have a separate tree so we'd just land it in
drm-misc-next.

Cheers,
Matt

>  - Chun-Kuang Hu + Philipp Zabel for mediatek
> 
>  - Thierry Reding + Mikko Perttunen for the host1x driver
>    (Note there is another patch for this driver set at
>     20240409165043.105137-2-u.kleine-koenig@pengutronix.de that is
>     relevant for the same quest.)
> 
>  - Philipp Zabel for ipu-v3
> 
> I plan to send a patch changing struct platform_driver::remove after the
> end of the merge window leading to 6.10-rc1 for inclusion in next via
> Greg's driver core. So please either care the patches land in 6.10-rc1
> or ack that I include them in the submission to Greg.
> 
> Thanks for your cooperation,
> Uwe
> 

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

Re: [PATCH 2/4] drm/mediatek: Convert to platform remove callback returning void

[CK Hu (胡俊光)]

Hi, Uwe:

On Tue, 2024-04-09 at 19:02 +0200, 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 ignored (apart
> from emitting a warning) 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. Eventually after all drivers
> are converted, .remove_new() will be renamed to .remove().
> 
> Trivially convert this driver from always returning zero in the remove
> callback to the void returning variant.

Reviewed-by: CK Hu <ck.hu@mediatek.com>

> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
>  drivers/gpu/drm/mediatek/mtk_padding.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_padding.c b/drivers/gpu/drm/mediatek/mtk_padding.c
> index 0d6451c149b6..9f92b720aaae 100644
> --- a/drivers/gpu/drm/mediatek/mtk_padding.c
> +++ b/drivers/gpu/drm/mediatek/mtk_padding.c
> @@ -137,10 +137,9 @@ static int mtk_padding_probe(struct platform_device *pdev)
>  return 0;
>  }
>  
> -static int mtk_padding_remove(struct platform_device *pdev)
> +static void mtk_padding_remove(struct platform_device *pdev)
>  {
>  component_del(&pdev->dev, &mtk_padding_component_ops);
> -return 0;
>  }
>  
>  static const struct of_device_id mtk_padding_driver_dt_match[] = {
> @@ -151,7 +150,7 @@ MODULE_DEVICE_TABLE(of, mtk_padding_driver_dt_match);
>  
>  struct platform_driver mtk_padding_driver = {
>  .probe= mtk_padding_probe,
> -.remove= mtk_padding_remove,
> +.remove_new= mtk_padding_remove,
>  .driver= {
>  .name= "mediatek-disp-padding",
>  .owner= THIS_MODULE,
> -- 
> 2.43.0
> 

Re: [PATCH 2/4] drm/mediatek: Convert to platform remove callback returning void

[AngeloGioacchino Del Regno]

Il 09/04/24 19:02, Uwe Kleine-König ha scritto:
> 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 ignored (apart
> from emitting a warning) 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. Eventually after all drivers
> are converted, .remove_new() will be renamed to .remove().
> 
> 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: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

> ---
>   drivers/gpu/drm/mediatek/mtk_padding.c | 5 ++---
>   1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_padding.c b/drivers/gpu/drm/mediatek/mtk_padding.c
> index 0d6451c149b6..9f92b720aaae 100644
> --- a/drivers/gpu/drm/mediatek/mtk_padding.c
> +++ b/drivers/gpu/drm/mediatek/mtk_padding.c
> @@ -137,10 +137,9 @@ static int mtk_padding_probe(struct platform_device *pdev)
>   	return 0;
>   }
>   
> -static int mtk_padding_remove(struct platform_device *pdev)
> +static void mtk_padding_remove(struct platform_device *pdev)
>   {
>   	component_del(&pdev->dev, &mtk_padding_component_ops);
> -	return 0;
>   }
>   
>   static const struct of_device_id mtk_padding_driver_dt_match[] = {
> @@ -151,7 +150,7 @@ MODULE_DEVICE_TABLE(of, mtk_padding_driver_dt_match);
>   
>   struct platform_driver mtk_padding_driver = {
>   	.probe		= mtk_padding_probe,
> -	.remove		= mtk_padding_remove,
> +	.remove_new	= mtk_padding_remove,
>   	.driver		= {
>   		.name	= "mediatek-disp-padding",
>   		.owner	= THIS_MODULE,