Linux ARM-MSM sub-architecture
 help / color / mirror / Atom feed
* [PATCH] media: venus: Fix probe error handling
@ 2025-03-27 12:53 ` Loic Poulain
  2025-03-28 16:04   ` Bryan O'Donoghue
                     ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Loic Poulain @ 2025-03-27 12:53 UTC (permalink / raw)
  To: quic_vgarodia, stanimir.k.varbanov
  Cc: linux-media, linux-arm-msm, bryan.odonoghue, Loic Poulain

Video device registering has been moved earlier in the probe function,
but the new order has not been propagated to error handling. This means
we can end with unreleased resources on error (e.g dangling video device
on missing firmware probe aborting).

Fixes: 08b1cf474b7f7 ("media: venus: core, venc, vdec: Fix probe dependency error")
Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
---
 drivers/media/platform/qcom/venus/core.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c
index c4438e4b2d9b..103afda799ed 100644
--- a/drivers/media/platform/qcom/venus/core.c
+++ b/drivers/media/platform/qcom/venus/core.c
@@ -438,7 +438,7 @@ static int venus_probe(struct platform_device *pdev)
 
 	ret = v4l2_device_register(dev, &core->v4l2_dev);
 	if (ret)
-		goto err_core_deinit;
+		goto err_hfi_destroy;
 
 	platform_set_drvdata(pdev, core);
 
@@ -476,24 +476,24 @@ static int venus_probe(struct platform_device *pdev)
 
 	ret = venus_enumerate_codecs(core, VIDC_SESSION_TYPE_DEC);
 	if (ret)
-		goto err_venus_shutdown;
+		goto err_core_deinit;
 
 	ret = venus_enumerate_codecs(core, VIDC_SESSION_TYPE_ENC);
 	if (ret)
-		goto err_venus_shutdown;
+		goto err_core_deinit;
 
 	ret = pm_runtime_put_sync(dev);
 	if (ret) {
 		pm_runtime_get_noresume(dev);
-		goto err_dev_unregister;
+		goto err_core_deinit;
 	}
 
 	venus_dbgfs_init(core);
 
 	return 0;
 
-err_dev_unregister:
-	v4l2_device_unregister(&core->v4l2_dev);
+err_core_deinit:
+	hfi_core_deinit(core, false);
 err_venus_shutdown:
 	venus_shutdown(core);
 err_firmware_deinit:
@@ -506,9 +506,9 @@ static int venus_probe(struct platform_device *pdev)
 	pm_runtime_put_noidle(dev);
 	pm_runtime_disable(dev);
 	pm_runtime_set_suspended(dev);
+	v4l2_device_unregister(&core->v4l2_dev);
+err_hfi_destroy:
 	hfi_destroy(core);
-err_core_deinit:
-	hfi_core_deinit(core, false);
 err_core_put:
 	if (core->pm_ops->core_put)
 		core->pm_ops->core_put(core);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] media: venus: Fix probe error handling
  2025-03-27 12:53 ` [PATCH] media: venus: Fix probe error handling Loic Poulain
@ 2025-03-28 16:04   ` Bryan O'Donoghue
  2025-03-28 16:05   ` Bryan O'Donoghue
  2025-04-02  8:57   ` Dikshita Agarwal
  2 siblings, 0 replies; 4+ messages in thread
From: Bryan O'Donoghue @ 2025-03-28 16:04 UTC (permalink / raw)
  To: Loic Poulain, quic_vgarodia, stanimir.k.varbanov
  Cc: linux-media, linux-arm-msm, bryan.odonoghue

On 27/03/2025 12:53, Loic Poulain wrote:
> Video device registering has been moved earlier in the probe function,
> but the new order has not been propagated to error handling. This means
> we can end with unreleased resources on error (e.g dangling video device
> on missing firmware probe aborting).
> 
> Fixes: 08b1cf474b7f7 ("media: venus: core, venc, vdec: Fix probe dependency error")
> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> ---
>   drivers/media/platform/qcom/venus/core.c | 16 ++++++++--------
>   1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c
> index c4438e4b2d9b..103afda799ed 100644
> --- a/drivers/media/platform/qcom/venus/core.c
> +++ b/drivers/media/platform/qcom/venus/core.c
> @@ -438,7 +438,7 @@ static int venus_probe(struct platform_device *pdev)
> 
>   	ret = v4l2_device_register(dev, &core->v4l2_dev);
>   	if (ret)
> -		goto err_core_deinit;
> +		goto err_hfi_destroy;
> 
>   	platform_set_drvdata(pdev, core);
> 
> @@ -476,24 +476,24 @@ static int venus_probe(struct platform_device *pdev)
> 
>   	ret = venus_enumerate_codecs(core, VIDC_SESSION_TYPE_DEC);
>   	if (ret)
> -		goto err_venus_shutdown;
> +		goto err_core_deinit;
> 
>   	ret = venus_enumerate_codecs(core, VIDC_SESSION_TYPE_ENC);
>   	if (ret)
> -		goto err_venus_shutdown;
> +		goto err_core_deinit;
> 
>   	ret = pm_runtime_put_sync(dev);
>   	if (ret) {
>   		pm_runtime_get_noresume(dev);
> -		goto err_dev_unregister;
> +		goto err_core_deinit;
>   	}
> 
>   	venus_dbgfs_init(core);
> 
>   	return 0;
> 
> -err_dev_unregister:
> -	v4l2_device_unregister(&core->v4l2_dev);
> +err_core_deinit:
> +	hfi_core_deinit(core, false);
>   err_venus_shutdown:
>   	venus_shutdown(core);
>   err_firmware_deinit:
> @@ -506,9 +506,9 @@ static int venus_probe(struct platform_device *pdev)
>   	pm_runtime_put_noidle(dev);
>   	pm_runtime_disable(dev);
>   	pm_runtime_set_suspended(dev);
> +	v4l2_device_unregister(&core->v4l2_dev);
> +err_hfi_destroy:
>   	hfi_destroy(core);
> -err_core_deinit:
> -	hfi_core_deinit(core, false);
>   err_core_put:
>   	if (core->pm_ops->core_put)
>   		core->pm_ops->core_put(core);
> --
> 2.34.1
> 
> 
Reviewed-by: Bryan O'Donoghue <bod@kernel.org>


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] media: venus: Fix probe error handling
  2025-03-27 12:53 ` [PATCH] media: venus: Fix probe error handling Loic Poulain
  2025-03-28 16:04   ` Bryan O'Donoghue
@ 2025-03-28 16:05   ` Bryan O'Donoghue
  2025-04-02  8:57   ` Dikshita Agarwal
  2 siblings, 0 replies; 4+ messages in thread
From: Bryan O'Donoghue @ 2025-03-28 16:05 UTC (permalink / raw)
  To: Loic Poulain, quic_vgarodia, stanimir.k.varbanov
  Cc: linux-media, linux-arm-msm, bryan.odonoghue

On 27/03/2025 12:53, Loic Poulain wrote:
> Video device registering has been moved earlier in the probe function,
> but the new order has not been propagated to error handling. This means
> we can end with unreleased resources on error (e.g dangling video device
> on missing firmware probe aborting).
> 
> Fixes: 08b1cf474b7f7 ("media: venus: core, venc, vdec: Fix probe dependency error")
> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> ---
>   drivers/media/platform/qcom/venus/core.c | 16 ++++++++--------
>   1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c
> index c4438e4b2d9b..103afda799ed 100644
> --- a/drivers/media/platform/qcom/venus/core.c
> +++ b/drivers/media/platform/qcom/venus/core.c
> @@ -438,7 +438,7 @@ static int venus_probe(struct platform_device *pdev)
> 
>   	ret = v4l2_device_register(dev, &core->v4l2_dev);
>   	if (ret)
> -		goto err_core_deinit;
> +		goto err_hfi_destroy;
> 
>   	platform_set_drvdata(pdev, core);
> 
> @@ -476,24 +476,24 @@ static int venus_probe(struct platform_device *pdev)
> 
>   	ret = venus_enumerate_codecs(core, VIDC_SESSION_TYPE_DEC);
>   	if (ret)
> -		goto err_venus_shutdown;
> +		goto err_core_deinit;
> 
>   	ret = venus_enumerate_codecs(core, VIDC_SESSION_TYPE_ENC);
>   	if (ret)
> -		goto err_venus_shutdown;
> +		goto err_core_deinit;
> 
>   	ret = pm_runtime_put_sync(dev);
>   	if (ret) {
>   		pm_runtime_get_noresume(dev);
> -		goto err_dev_unregister;
> +		goto err_core_deinit;
>   	}
> 
>   	venus_dbgfs_init(core);
> 
>   	return 0;
> 
> -err_dev_unregister:
> -	v4l2_device_unregister(&core->v4l2_dev);
> +err_core_deinit:
> +	hfi_core_deinit(core, false);
>   err_venus_shutdown:
>   	venus_shutdown(core);
>   err_firmware_deinit:
> @@ -506,9 +506,9 @@ static int venus_probe(struct platform_device *pdev)
>   	pm_runtime_put_noidle(dev);
>   	pm_runtime_disable(dev);
>   	pm_runtime_set_suspended(dev);
> +	v4l2_device_unregister(&core->v4l2_dev);
> +err_hfi_destroy:
>   	hfi_destroy(core);
> -err_core_deinit:
> -	hfi_core_deinit(core, false);
>   err_core_put:
>   	if (core->pm_ops->core_put)
>   		core->pm_ops->core_put(core);
> --
> 2.34.1
> 
> 
Reviewed-by: Bryan O'Donoghue <bod@kernel.org>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] media: venus: Fix probe error handling
  2025-03-27 12:53 ` [PATCH] media: venus: Fix probe error handling Loic Poulain
  2025-03-28 16:04   ` Bryan O'Donoghue
  2025-03-28 16:05   ` Bryan O'Donoghue
@ 2025-04-02  8:57   ` Dikshita Agarwal
  2 siblings, 0 replies; 4+ messages in thread
From: Dikshita Agarwal @ 2025-04-02  8:57 UTC (permalink / raw)
  To: Loic Poulain, quic_vgarodia, stanimir.k.varbanov
  Cc: linux-media, linux-arm-msm, bryan.odonoghue



On 3/27/2025 6:23 PM, Loic Poulain wrote:
> Video device registering has been moved earlier in the probe function,
> but the new order has not been propagated to error handling. This means
> we can end with unreleased resources on error (e.g dangling video device
> on missing firmware probe aborting).
> 
> Fixes: 08b1cf474b7f7 ("media: venus: core, venc, vdec: Fix probe dependency error")
> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> ---
>  drivers/media/platform/qcom/venus/core.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c
> index c4438e4b2d9b..103afda799ed 100644
> --- a/drivers/media/platform/qcom/venus/core.c
> +++ b/drivers/media/platform/qcom/venus/core.c
> @@ -438,7 +438,7 @@ static int venus_probe(struct platform_device *pdev)
>  
>  	ret = v4l2_device_register(dev, &core->v4l2_dev);
>  	if (ret)
> -		goto err_core_deinit;
> +		goto err_hfi_destroy;
>  
>  	platform_set_drvdata(pdev, core);
>  
> @@ -476,24 +476,24 @@ static int venus_probe(struct platform_device *pdev)
>  
>  	ret = venus_enumerate_codecs(core, VIDC_SESSION_TYPE_DEC);
>  	if (ret)
> -		goto err_venus_shutdown;
> +		goto err_core_deinit;
>  
>  	ret = venus_enumerate_codecs(core, VIDC_SESSION_TYPE_ENC);
>  	if (ret)
> -		goto err_venus_shutdown;
> +		goto err_core_deinit;
>  
>  	ret = pm_runtime_put_sync(dev);
>  	if (ret) {
>  		pm_runtime_get_noresume(dev);
> -		goto err_dev_unregister;
> +		goto err_core_deinit;
>  	}
>  
>  	venus_dbgfs_init(core);
>  
>  	return 0;
>  
> -err_dev_unregister:
> -	v4l2_device_unregister(&core->v4l2_dev);
> +err_core_deinit:
> +	hfi_core_deinit(core, false);
>  err_venus_shutdown:
>  	venus_shutdown(core);
>  err_firmware_deinit:
> @@ -506,9 +506,9 @@ static int venus_probe(struct platform_device *pdev)
>  	pm_runtime_put_noidle(dev);
>  	pm_runtime_disable(dev);
>  	pm_runtime_set_suspended(dev);
> +	v4l2_device_unregister(&core->v4l2_dev);
> +err_hfi_destroy:
>  	hfi_destroy(core);
> -err_core_deinit:
> -	hfi_core_deinit(core, false);
>  err_core_put:
>  	if (core->pm_ops->core_put)
>  		core->pm_ops->core_put(core);

Reviewed-by: Dikshita Agarwal <quic_dikshita@quicinc.com>

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-04-02  8:57 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <riuofNmnHEZYFpNAkKYNZAEin3vihrkFlhukRCsZ_HfO7ZpmXiKVG9N-ICxJKdufFrlmf-DhDoUdjI2EBANAKA==@protonmail.internalid>
2025-03-27 12:53 ` [PATCH] media: venus: Fix probe error handling Loic Poulain
2025-03-28 16:04   ` Bryan O'Donoghue
2025-03-28 16:05   ` Bryan O'Donoghue
2025-04-02  8:57   ` Dikshita Agarwal

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox