[PATCH v4 2/4] media: imx-jpeg: Use devm_pm_runtime_enable() helper

ming.qian@oss.nxp.com posted 4 patches 5 days, 21 hours ago
[PATCH v4 2/4] media: imx-jpeg: Use devm_pm_runtime_enable() helper
Posted by ming.qian@oss.nxp.com 5 days, 21 hours ago
From: Ming Qian <ming.qian@oss.nxp.com>

Use devm_pm_runtime_enable() to simplify probe and exit paths.

No functional change.

Signed-off-by: Ming Qian <ming.qian@oss.nxp.com>
---
 drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
index 39022c1bf36d..877e6c4f7406 100644
--- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
+++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
@@ -2971,12 +2971,22 @@ static int mxc_jpeg_probe(struct platform_device *pdev)
 			  jpeg->dec_vdev->minor);
 
 	platform_set_drvdata(pdev, jpeg);
-	pm_runtime_enable(dev);
+	ret = devm_pm_runtime_enable(dev);
+	if (ret) {
+		dev_err(dev, "Failed to enable runtime PM: %d\n", ret);
+		goto err_pm;
+	}
 
 	return 0;
 
+err_pm:
+	video_unregister_device(jpeg->dec_vdev);
+	/* set NULL to prevent double-free */
+	jpeg->dec_vdev = NULL;
 err_vdev_register:
-	video_device_release(jpeg->dec_vdev);
+	/* Only release if allocation succeeded but registration failed */
+	if (jpeg->dec_vdev)
+		video_device_release(jpeg->dec_vdev);
 
 err_vdev_alloc:
 	v4l2_m2m_release(jpeg->m2m_dev);
@@ -3047,7 +3057,6 @@ static void mxc_jpeg_remove(struct platform_device *pdev)
 
 	mxc_jpeg_free_slot_data(jpeg);
 
-	pm_runtime_disable(&pdev->dev);
 	video_unregister_device(jpeg->dec_vdev);
 	v4l2_m2m_release(jpeg->m2m_dev);
 	v4l2_device_unregister(&jpeg->v4l2_dev);
-- 
2.52.0
Re: [PATCH v4 2/4] media: imx-jpeg: Use devm_pm_runtime_enable() helper
Posted by Frank Li 5 days, 12 hours ago
On Tue, Feb 03, 2026 at 04:23:39PM +0800, ming.qian@oss.nxp.com wrote:
> From: Ming Qian <ming.qian@oss.nxp.com>
>
> Use devm_pm_runtime_enable() to simplify probe and exit paths.
>
> No functional change.
>
> Signed-off-by: Ming Qian <ming.qian@oss.nxp.com>
> ---
>  drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
> index 39022c1bf36d..877e6c4f7406 100644
> --- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
> +++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
> @@ -2971,12 +2971,22 @@ static int mxc_jpeg_probe(struct platform_device *pdev)
>  			  jpeg->dec_vdev->minor);
>
>  	platform_set_drvdata(pdev, jpeg);
> -	pm_runtime_enable(dev);
> +	ret = devm_pm_runtime_enable(dev);
> +	if (ret) {
> +		dev_err(dev, "Failed to enable runtime PM: %d\n", ret);
> +		goto err_pm;
> +	}
>
>  	return 0;
>
> +err_pm:
> +	video_unregister_device(jpeg->dec_vdev);
> +	/* set NULL to prevent double-free */
> +	jpeg->dec_vdev = NULL;

code have not simplied, you add addtional goto label. You can use
devm_add_action_or_reset().

Frank

>  err_vdev_register:
> -	video_device_release(jpeg->dec_vdev);
> +	/* Only release if allocation succeeded but registration failed */
> +	if (jpeg->dec_vdev)
> +		video_device_release(jpeg->dec_vdev);
>
>  err_vdev_alloc:
>  	v4l2_m2m_release(jpeg->m2m_dev);
> @@ -3047,7 +3057,6 @@ static void mxc_jpeg_remove(struct platform_device *pdev)
>
>  	mxc_jpeg_free_slot_data(jpeg);
>
> -	pm_runtime_disable(&pdev->dev);
>  	video_unregister_device(jpeg->dec_vdev);
>  	v4l2_m2m_release(jpeg->m2m_dev);
>  	v4l2_device_unregister(&jpeg->v4l2_dev);
> --
> 2.52.0
>
Re: [PATCH v4 2/4] media: imx-jpeg: Use devm_pm_runtime_enable() helper
Posted by Ming Qian(OSS) 5 days, 4 hours ago
Hi Frank,

On 2/4/2026 12:56 AM, Frank Li wrote:
> On Tue, Feb 03, 2026 at 04:23:39PM +0800, ming.qian@oss.nxp.com wrote:
>> From: Ming Qian <ming.qian@oss.nxp.com>
>>
>> Use devm_pm_runtime_enable() to simplify probe and exit paths.
>>
>> No functional change.
>>
>> Signed-off-by: Ming Qian <ming.qian@oss.nxp.com>
>> ---
>>   drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c | 15 ++++++++++++---
>>   1 file changed, 12 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
>> index 39022c1bf36d..877e6c4f7406 100644
>> --- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
>> +++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
>> @@ -2971,12 +2971,22 @@ static int mxc_jpeg_probe(struct platform_device *pdev)
>>   			  jpeg->dec_vdev->minor);
>>
>>   	platform_set_drvdata(pdev, jpeg);
>> -	pm_runtime_enable(dev);
>> +	ret = devm_pm_runtime_enable(dev);
>> +	if (ret) {
>> +		dev_err(dev, "Failed to enable runtime PM: %d\n", ret);
>> +		goto err_pm;
>> +	}
>>
>>   	return 0;
>>
>> +err_pm:
>> +	video_unregister_device(jpeg->dec_vdev);
>> +	/* set NULL to prevent double-free */
>> +	jpeg->dec_vdev = NULL;
> 
> code have not simplied, you add addtional goto label. You can use
> devm_add_action_or_reset().
> 
> Frank
> 

I don't quite understand the difference compared to directly using
devm_pm_runtime_enable.

int devm_pm_runtime_enable(struct device *dev)
{
	pm_runtime_enable(dev);

	return devm_add_action_or_reset(dev, pm_runtime_disable_action, dev);
}

We still need to check the return value of devm_add_action_or_reset and
perform the corresponding error handling.
We may still add the goto label.

Regards,
Ming

>>   err_vdev_register:
>> -	video_device_release(jpeg->dec_vdev);
>> +	/* Only release if allocation succeeded but registration failed */
>> +	if (jpeg->dec_vdev)
>> +		video_device_release(jpeg->dec_vdev);
>>
>>   err_vdev_alloc:
>>   	v4l2_m2m_release(jpeg->m2m_dev);
>> @@ -3047,7 +3057,6 @@ static void mxc_jpeg_remove(struct platform_device *pdev)
>>
>>   	mxc_jpeg_free_slot_data(jpeg);
>>
>> -	pm_runtime_disable(&pdev->dev);
>>   	video_unregister_device(jpeg->dec_vdev);
>>   	v4l2_m2m_release(jpeg->m2m_dev);
>>   	v4l2_device_unregister(&jpeg->v4l2_dev);
>> --
>> 2.52.0
>>
Re: [PATCH v4 2/4] media: imx-jpeg: Use devm_pm_runtime_enable() helper
Posted by Frank Li 4 days, 14 hours ago
On Wed, Feb 04, 2026 at 09:36:06AM +0800, Ming Qian(OSS) wrote:
> Hi Frank,
>
> On 2/4/2026 12:56 AM, Frank Li wrote:
> > On Tue, Feb 03, 2026 at 04:23:39PM +0800, ming.qian@oss.nxp.com wrote:
> > > From: Ming Qian <ming.qian@oss.nxp.com>
> > >
> > > Use devm_pm_runtime_enable() to simplify probe and exit paths.
> > >
> > > No functional change.
> > >
> > > Signed-off-by: Ming Qian <ming.qian@oss.nxp.com>
> > > ---
> > >   drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c | 15 ++++++++++++---
> > >   1 file changed, 12 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
> > > index 39022c1bf36d..877e6c4f7406 100644
> > > --- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
> > > +++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
> > > @@ -2971,12 +2971,22 @@ static int mxc_jpeg_probe(struct platform_device *pdev)
> > >   			  jpeg->dec_vdev->minor);
> > >
> > >   	platform_set_drvdata(pdev, jpeg);
> > > -	pm_runtime_enable(dev);
> > > +	ret = devm_pm_runtime_enable(dev);
> > > +	if (ret) {
> > > +		dev_err(dev, "Failed to enable runtime PM: %d\n", ret);
> > > +		goto err_pm;
> > > +	}
> > >
> > >   	return 0;
> > >
> > > +err_pm:
> > > +	video_unregister_device(jpeg->dec_vdev);
> > > +	/* set NULL to prevent double-free */
> > > +	jpeg->dec_vdev = NULL;
> >
> > code have not simplied, you add addtional goto label. You can use
> > devm_add_action_or_reset().
> >
> > Frank
> >
>
> I don't quite understand the difference compared to directly using
> devm_pm_runtime_enable.
>
> int devm_pm_runtime_enable(struct device *dev)
> {
> 	pm_runtime_enable(dev);
>
> 	return devm_add_action_or_reset(dev, pm_runtime_disable_action, dev);
> }
>
> We still need to check the return value of devm_add_action_or_reset and
> perform the corresponding error handling.
> We may still add the goto label.

Sorry, my means put video_device_release() to devm_add_action_or_reset() to
avoid goto.

If it is complex, you can keep original one.

Frank
>
> Regards,
> Ming
>
> > >   err_vdev_register:
> > > -	video_device_release(jpeg->dec_vdev);
> > > +	/* Only release if allocation succeeded but registration failed */
> > > +	if (jpeg->dec_vdev)
> > > +		video_device_release(jpeg->dec_vdev);
> > >
> > >   err_vdev_alloc:
> > >   	v4l2_m2m_release(jpeg->m2m_dev);
> > > @@ -3047,7 +3057,6 @@ static void mxc_jpeg_remove(struct platform_device *pdev)
> > >
> > >   	mxc_jpeg_free_slot_data(jpeg);
> > >
> > > -	pm_runtime_disable(&pdev->dev);
> > >   	video_unregister_device(jpeg->dec_vdev);
> > >   	v4l2_m2m_release(jpeg->m2m_dev);
> > >   	v4l2_device_unregister(&jpeg->v4l2_dev);
> > > --
> > > 2.52.0
> > >
Re: [PATCH v4 2/4] media: imx-jpeg: Use devm_pm_runtime_enable() helper
Posted by Ming Qian(OSS) 4 days, 3 hours ago
Hi Frank,

On 2/4/2026 11:42 PM, Frank Li wrote:
> On Wed, Feb 04, 2026 at 09:36:06AM +0800, Ming Qian(OSS) wrote:
>> Hi Frank,
>>
>> On 2/4/2026 12:56 AM, Frank Li wrote:
>>> On Tue, Feb 03, 2026 at 04:23:39PM +0800, ming.qian@oss.nxp.com wrote:
>>>> From: Ming Qian <ming.qian@oss.nxp.com>
>>>>
>>>> Use devm_pm_runtime_enable() to simplify probe and exit paths.
>>>>
>>>> No functional change.
>>>>
>>>> Signed-off-by: Ming Qian <ming.qian@oss.nxp.com>
>>>> ---
>>>>    drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c | 15 ++++++++++++---
>>>>    1 file changed, 12 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
>>>> index 39022c1bf36d..877e6c4f7406 100644
>>>> --- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
>>>> +++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
>>>> @@ -2971,12 +2971,22 @@ static int mxc_jpeg_probe(struct platform_device *pdev)
>>>>    			  jpeg->dec_vdev->minor);
>>>>
>>>>    	platform_set_drvdata(pdev, jpeg);
>>>> -	pm_runtime_enable(dev);
>>>> +	ret = devm_pm_runtime_enable(dev);
>>>> +	if (ret) {
>>>> +		dev_err(dev, "Failed to enable runtime PM: %d\n", ret);
>>>> +		goto err_pm;
>>>> +	}
>>>>
>>>>    	return 0;
>>>>
>>>> +err_pm:
>>>> +	video_unregister_device(jpeg->dec_vdev);
>>>> +	/* set NULL to prevent double-free */
>>>> +	jpeg->dec_vdev = NULL;
>>>
>>> code have not simplied, you add addtional goto label. You can use
>>> devm_add_action_or_reset().
>>>
>>> Frank
>>>
>>
>> I don't quite understand the difference compared to directly using
>> devm_pm_runtime_enable.
>>
>> int devm_pm_runtime_enable(struct device *dev)
>> {
>> 	pm_runtime_enable(dev);
>>
>> 	return devm_add_action_or_reset(dev, pm_runtime_disable_action, dev);
>> }
>>
>> We still need to check the return value of devm_add_action_or_reset and
>> perform the corresponding error handling.
>> We may still add the goto label.
> 
> Sorry, my means put video_device_release() to devm_add_action_or_reset() to
> avoid goto.
> 
> If it is complex, you can keep original one.
> 
> Frank

OK, I get your point, and I prefer to keep it.

Regards,
Ming

>>
>> Regards,
>> Ming
>>
>>>>    err_vdev_register:
>>>> -	video_device_release(jpeg->dec_vdev);
>>>> +	/* Only release if allocation succeeded but registration failed */
>>>> +	if (jpeg->dec_vdev)
>>>> +		video_device_release(jpeg->dec_vdev);
>>>>
>>>>    err_vdev_alloc:
>>>>    	v4l2_m2m_release(jpeg->m2m_dev);
>>>> @@ -3047,7 +3057,6 @@ static void mxc_jpeg_remove(struct platform_device *pdev)
>>>>
>>>>    	mxc_jpeg_free_slot_data(jpeg);
>>>>
>>>> -	pm_runtime_disable(&pdev->dev);
>>>>    	video_unregister_device(jpeg->dec_vdev);
>>>>    	v4l2_m2m_release(jpeg->m2m_dev);
>>>>    	v4l2_device_unregister(&jpeg->v4l2_dev);
>>>> --
>>>> 2.52.0
>>>>