[PATCH v2] media: cedrus: Fix failure to clean up hardware on probe failure

[PATCH v2] media: cedrus: Fix failure to clean up hardware on probe failure

[Andrey Skvortsov]

From: Samuel Holland <samuel@sholland.org>

If V4L2 device fails to register, then SRAM still be claimed and as a
result driver will not be able to probe again.

 cedrus 1c0e000.video-codec: Failed to claim SRAM
 cedrus 1c0e000.video-codec: Failed to probe hardware
 cedrus 1c0e000.video-codec: probe with driver cedrus failed with error -16

cedrus_hw_remove undoes everything that was previously done by
cedrus_hw_probe, such as disabling runtime power management and
releasing the claimed SRAM and reserved memory region.

Signed-off-by: Samuel Holland <samuel@sholland.org>
Signed-off-by: Andrey Skvortsov <andrej.skvortzov@gmail.com>
Fixes: 50e761516f2b ("media: platform: Add Cedrus VPU decoder driver")
---

Changes in v2:
 - remove duplicate 'in-body' From: record
 - add more technical details to commit message

 drivers/staging/media/sunxi/cedrus/cedrus.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.c b/drivers/staging/media/sunxi/cedrus/cedrus.c
index 1d2130f35fffc..ee0e286add67d 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus.c
@@ -477,7 +477,7 @@ static int cedrus_probe(struct platform_device *pdev)
 	ret = v4l2_device_register(&pdev->dev, &dev->v4l2_dev);
 	if (ret) {
 		dev_err(&pdev->dev, "Failed to register V4L2 device\n");
-		return ret;
+		goto err_hw;
 	}
 
 	vfd = &dev->vfd;
@@ -538,6 +538,8 @@ static int cedrus_probe(struct platform_device *pdev)
 	v4l2_m2m_release(dev->m2m_dev);
 err_v4l2:
 	v4l2_device_unregister(&dev->v4l2_dev);
+err_hw:
+	cedrus_hw_remove(dev);
 
 	return ret;
 }
-- 
2.51.0

Re: [PATCH v2] media: cedrus: Fix failure to clean up hardware on probe failure

[Paul Kocialkowski]

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

Hi,

On Tue 07 Apr 26, 01:14, Andrey Skvortsov wrote:
> From: Samuel Holland <samuel@sholland.org>
> 
> If V4L2 device fails to register, then SRAM still be claimed and as a
> result driver will not be able to probe again.
> 
>  cedrus 1c0e000.video-codec: Failed to claim SRAM
>  cedrus 1c0e000.video-codec: Failed to probe hardware
>  cedrus 1c0e000.video-codec: probe with driver cedrus failed with error -16
> 
> cedrus_hw_remove undoes everything that was previously done by
> cedrus_hw_probe, such as disabling runtime power management and
> releasing the claimed SRAM and reserved memory region.

Good catch, thanks for the patch!

Acked-by: Paul Kocialkowski <paulk@sys-base.io>

Note that I (still) plan to rework the architecture of this driver in the
future but such fixes are definitely welcome in the meantime.

All the best,

Paul

> Signed-off-by: Samuel Holland <samuel@sholland.org>
> Signed-off-by: Andrey Skvortsov <andrej.skvortzov@gmail.com>
> Fixes: 50e761516f2b ("media: platform: Add Cedrus VPU decoder driver")
> ---
> 
> Changes in v2:
>  - remove duplicate 'in-body' From: record
>  - add more technical details to commit message
> 
>  drivers/staging/media/sunxi/cedrus/cedrus.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.c b/drivers/staging/media/sunxi/cedrus/cedrus.c
> index 1d2130f35fffc..ee0e286add67d 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus.c
> @@ -477,7 +477,7 @@ static int cedrus_probe(struct platform_device *pdev)
>  	ret = v4l2_device_register(&pdev->dev, &dev->v4l2_dev);
>  	if (ret) {
>  		dev_err(&pdev->dev, "Failed to register V4L2 device\n");
> -		return ret;
> +		goto err_hw;
>  	}
>  
>  	vfd = &dev->vfd;
> @@ -538,6 +538,8 @@ static int cedrus_probe(struct platform_device *pdev)
>  	v4l2_m2m_release(dev->m2m_dev);
>  err_v4l2:
>  	v4l2_device_unregister(&dev->v4l2_dev);
> +err_hw:
> +	cedrus_hw_remove(dev);
>  
>  	return ret;
>  }
> -- 
> 2.51.0
> 

-- 
Paul Kocialkowski,

Independent contractor - sys-base - https://www.sys-base.io/
Free software developer - https://www.paulk.fr/

Expert in multimedia, graphics and embedded hardware support with Linux.

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