diff mbox series

[v2] media: mediatek: vcodec: mtk_vcodec_dec_hw: Use devm_pm_runtime_enable()

Message ID 20230515141610.v2.1.I0d1657be3fea5870f797e975a7aa490291e17993@changeid (mailing list archive)
State Accepted
Delegated to: Hans Verkuil
Headers
Series [v2] media: mediatek: vcodec: mtk_vcodec_dec_hw: Use devm_pm_runtime_enable() |

Commit Message

Fei Shao May 15, 2023, 6:16 a.m. UTC 
  Convert pm_runtime_enable() to the managed version, and clean up error
handling and unnecessary .remove() callback accordingly.

Signed-off-by: Fei Shao <fshao@chromium.org>

---

Changes in v2:
Use devm_pm_runtime_enable() per suggestion from the previous thread:
https://lore.kernel.org/lkml/20230510164330.z2ygkl7vws6fci75@pengutronix.de/T/#m25be91afe3e9554600e859a8a59128ca234fc63d

 .../mediatek/vcodec/mtk_vcodec_dec_hw.c       | 26 ++++++-------------
 1 file changed, 8 insertions(+), 18 deletions(-)

Comments

Hans Verkuil May 23, 2023, 11:42 a.m. UTC | #1 
On 15/05/2023 08:16, Fei Shao wrote:
> Convert pm_runtime_enable() to the managed version, and clean up error
> handling and unnecessary .remove() callback accordingly.

This patch no longer applies. Can you make a v3?

Regards,

	Hans

> 
> Signed-off-by: Fei Shao <fshao@chromium.org>
> 
> ---
> 
> Changes in v2:
> Use devm_pm_runtime_enable() per suggestion from the previous thread:
> https://lore.kernel.org/lkml/20230510164330.z2ygkl7vws6fci75@pengutronix.de/T/#m25be91afe3e9554600e859a8a59128ca234fc63d
> 
>  .../mediatek/vcodec/mtk_vcodec_dec_hw.c       | 26 ++++++-------------
>  1 file changed, 8 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_hw.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_hw.c
> index b753bf54ebd9..e1cb2f8dca33 100644
> --- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_hw.c
> +++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_hw.c
> @@ -148,20 +148,21 @@ static int mtk_vdec_hw_probe(struct platform_device *pdev)
>  	ret = mtk_vcodec_init_dec_clk(pdev, &subdev_dev->pm);
>  	if (ret)
>  		return ret;
> -	pm_runtime_enable(&pdev->dev);
> +
> +	ret = devm_pm_runtime_enable(&pdev->dev);
> +	if (ret)
> +		return ret;
>  
>  	of_id = of_match_device(mtk_vdec_hw_match, dev);
>  	if (!of_id) {
>  		dev_err(dev, "Can't get vdec subdev id.\n");
> -		ret = -EINVAL;
> -		goto err;
> +		return -EINVAL;
>  	}
>  
>  	hw_idx = (enum mtk_vdec_hw_id)(uintptr_t)of_id->data;
>  	if (hw_idx >= MTK_VDEC_HW_MAX) {
>  		dev_err(dev, "Hardware index %d not correct.\n", hw_idx);
> -		ret = -EINVAL;
> -		goto err;
> +		return -EINVAL;
>  	}
>  
>  	main_dev->subdev_dev[hw_idx] = subdev_dev;
> @@ -173,36 +174,25 @@ static int mtk_vdec_hw_probe(struct platform_device *pdev)
>  	if (IS_SUPPORT_VDEC_HW_IRQ(hw_idx)) {
>  		ret = mtk_vdec_hw_init_irq(subdev_dev);
>  		if (ret)
> -			goto err;
> +			return ret;
>  	}
>  
>  	subdev_dev->reg_base[VDEC_HW_MISC] =
>  		devm_platform_ioremap_resource(pdev, 0);
>  	if (IS_ERR((__force void *)subdev_dev->reg_base[VDEC_HW_MISC])) {
>  		ret = PTR_ERR((__force void *)subdev_dev->reg_base[VDEC_HW_MISC]);
> -		goto err;
> +		return ret;
>  	}
>  
>  	if (!main_dev->subdev_prob_done)
>  		main_dev->subdev_prob_done = mtk_vdec_hw_prob_done;
>  
>  	platform_set_drvdata(pdev, subdev_dev);
> -	return 0;
> -err:
> -	pm_runtime_disable(subdev_dev->pm.dev);
> -	return ret;
> -}
> -
> -static int mtk_vdec_hw_remove(struct platform_device *pdev)
> -{
> -	pm_runtime_disable(&pdev->dev);
> -
>  	return 0;
>  }
>  
>  static struct platform_driver mtk_vdec_driver = {
>  	.probe	= mtk_vdec_hw_probe,
> -	.remove = mtk_vdec_hw_remove,
>  	.driver	= {
>  		.name	= "mtk-vdec-comp",
>  		.of_match_table = mtk_vdec_hw_match,
Alexandre Mergnat May 23, 2023, 12:10 p.m. UTC | #2 
On 15/05/2023 08:16, Fei Shao wrote:
> Convert pm_runtime_enable() to the managed version, and clean up error
> handling and unnecessary .remove() callback accordingly.
> 
> Signed-off-by: Fei Shao<fshao@chromium.org>

Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>
Hans Verkuil May 23, 2023, 12:28 p.m. UTC | #3 
On 23/05/2023 13:42, Hans Verkuil wrote:
> On 15/05/2023 08:16, Fei Shao wrote:
>> Convert pm_runtime_enable() to the managed version, and clean up error
>> handling and unnecessary .remove() callback accordingly.
> 
> This patch no longer applies. Can you make a v3?

Sorry, you can ignore this. I now realize that this was a v2 of

https://patchwork.linuxtv.org/project/linux-media/patch/20230510233117.1.I7047714f92ef7569bd21f118ae6aee20b3175a92@changeid/

I had that v1 applied, so obviously this v2 would fail to apply. After dropping
that v1 patch it now applies cleanly.

Regards,

	Hans

> 
> Regards,
> 
> 	Hans
> 
>>
>> Signed-off-by: Fei Shao <fshao@chromium.org>
>>
>> ---
>>
>> Changes in v2:
>> Use devm_pm_runtime_enable() per suggestion from the previous thread:
>> https://lore.kernel.org/lkml/20230510164330.z2ygkl7vws6fci75@pengutronix.de/T/#m25be91afe3e9554600e859a8a59128ca234fc63d
>>
>>  .../mediatek/vcodec/mtk_vcodec_dec_hw.c       | 26 ++++++-------------
>>  1 file changed, 8 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_hw.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_hw.c
>> index b753bf54ebd9..e1cb2f8dca33 100644
>> --- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_hw.c
>> +++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_hw.c
>> @@ -148,20 +148,21 @@ static int mtk_vdec_hw_probe(struct platform_device *pdev)
>>  	ret = mtk_vcodec_init_dec_clk(pdev, &subdev_dev->pm);
>>  	if (ret)
>>  		return ret;
>> -	pm_runtime_enable(&pdev->dev);
>> +
>> +	ret = devm_pm_runtime_enable(&pdev->dev);
>> +	if (ret)
>> +		return ret;
>>  
>>  	of_id = of_match_device(mtk_vdec_hw_match, dev);
>>  	if (!of_id) {
>>  		dev_err(dev, "Can't get vdec subdev id.\n");
>> -		ret = -EINVAL;
>> -		goto err;
>> +		return -EINVAL;
>>  	}
>>  
>>  	hw_idx = (enum mtk_vdec_hw_id)(uintptr_t)of_id->data;
>>  	if (hw_idx >= MTK_VDEC_HW_MAX) {
>>  		dev_err(dev, "Hardware index %d not correct.\n", hw_idx);
>> -		ret = -EINVAL;
>> -		goto err;
>> +		return -EINVAL;
>>  	}
>>  
>>  	main_dev->subdev_dev[hw_idx] = subdev_dev;
>> @@ -173,36 +174,25 @@ static int mtk_vdec_hw_probe(struct platform_device *pdev)
>>  	if (IS_SUPPORT_VDEC_HW_IRQ(hw_idx)) {
>>  		ret = mtk_vdec_hw_init_irq(subdev_dev);
>>  		if (ret)
>> -			goto err;
>> +			return ret;
>>  	}
>>  
>>  	subdev_dev->reg_base[VDEC_HW_MISC] =
>>  		devm_platform_ioremap_resource(pdev, 0);
>>  	if (IS_ERR((__force void *)subdev_dev->reg_base[VDEC_HW_MISC])) {
>>  		ret = PTR_ERR((__force void *)subdev_dev->reg_base[VDEC_HW_MISC]);
>> -		goto err;
>> +		return ret;
>>  	}
>>  
>>  	if (!main_dev->subdev_prob_done)
>>  		main_dev->subdev_prob_done = mtk_vdec_hw_prob_done;
>>  
>>  	platform_set_drvdata(pdev, subdev_dev);
>> -	return 0;
>> -err:
>> -	pm_runtime_disable(subdev_dev->pm.dev);
>> -	return ret;
>> -}
>> -
>> -static int mtk_vdec_hw_remove(struct platform_device *pdev)
>> -{
>> -	pm_runtime_disable(&pdev->dev);
>> -
>>  	return 0;
>>  }
>>  
>>  static struct platform_driver mtk_vdec_driver = {
>>  	.probe	= mtk_vdec_hw_probe,
>> -	.remove = mtk_vdec_hw_remove,
>>  	.driver	= {
>>  		.name	= "mtk-vdec-comp",
>>  		.of_match_table = mtk_vdec_hw_match,
>
AngeloGioacchino Del Regno May 23, 2023, 3:14 p.m. UTC | #4 
Il 15/05/23 08:16, Fei Shao ha scritto:
> Convert pm_runtime_enable() to the managed version, and clean up error
> handling and unnecessary .remove() callback accordingly.
> 
> Signed-off-by: Fei Shao <fshao@chromium.org>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

...but I was probably too late :-)
Fei Shao May 23, 2023, 3:18 p.m. UTC | #5 
On Tue, May 23, 2023 at 8:28 PM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
>
> On 23/05/2023 13:42, Hans Verkuil wrote:
> > On 15/05/2023 08:16, Fei Shao wrote:
> >> Convert pm_runtime_enable() to the managed version, and clean up error
> >> handling and unnecessary .remove() callback accordingly.
> >
> > This patch no longer applies. Can you make a v3?
>
> Sorry, you can ignore this. I now realize that this was a v2 of
>
> https://patchwork.linuxtv.org/project/linux-media/patch/20230510233117.1.I7047714f92ef7569bd21f118ae6aee20b3175a92@changeid/
>
> I had that v1 applied, so obviously this v2 would fail to apply. After dropping
> that v1 patch it now applies cleanly.

Ack, many thanks.
BTW, besides the review tags (appreciation to all), would you mind
adding this tag also as I missed it in the beginning?

Suggested-by: Chen-Yu Tsai <wenst@chromium.org>

Regards,
Fei





>
> Regards,
>
>         Hans
>
> >
> > Regards,
> >
> >       Hans
> >
> >>
> >> Signed-off-by: Fei Shao <fshao@chromium.org>
> >>
> >> ---
> >>
> >> Changes in v2:
> >> Use devm_pm_runtime_enable() per suggestion from the previous thread:
> >> https://lore.kernel.org/lkml/20230510164330.z2ygkl7vws6fci75@pengutronix.de/T/#m25be91afe3e9554600e859a8a59128ca234fc63d
> >>
> >>  .../mediatek/vcodec/mtk_vcodec_dec_hw.c       | 26 ++++++-------------
> >>  1 file changed, 8 insertions(+), 18 deletions(-)
> >>
> >> diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_hw.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_hw.c
> >> index b753bf54ebd9..e1cb2f8dca33 100644
> >> --- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_hw.c
> >> +++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_hw.c
> >> @@ -148,20 +148,21 @@ static int mtk_vdec_hw_probe(struct platform_device *pdev)
> >>      ret = mtk_vcodec_init_dec_clk(pdev, &subdev_dev->pm);
> >>      if (ret)
> >>              return ret;
> >> -    pm_runtime_enable(&pdev->dev);
> >> +
> >> +    ret = devm_pm_runtime_enable(&pdev->dev);
> >> +    if (ret)
> >> +            return ret;
> >>
> >>      of_id = of_match_device(mtk_vdec_hw_match, dev);
> >>      if (!of_id) {
> >>              dev_err(dev, "Can't get vdec subdev id.\n");
> >> -            ret = -EINVAL;
> >> -            goto err;
> >> +            return -EINVAL;
> >>      }
> >>
> >>      hw_idx = (enum mtk_vdec_hw_id)(uintptr_t)of_id->data;
> >>      if (hw_idx >= MTK_VDEC_HW_MAX) {
> >>              dev_err(dev, "Hardware index %d not correct.\n", hw_idx);
> >> -            ret = -EINVAL;
> >> -            goto err;
> >> +            return -EINVAL;
> >>      }
> >>
> >>      main_dev->subdev_dev[hw_idx] = subdev_dev;
> >> @@ -173,36 +174,25 @@ static int mtk_vdec_hw_probe(struct platform_device *pdev)
> >>      if (IS_SUPPORT_VDEC_HW_IRQ(hw_idx)) {
> >>              ret = mtk_vdec_hw_init_irq(subdev_dev);
> >>              if (ret)
> >> -                    goto err;
> >> +                    return ret;
> >>      }
> >>
> >>      subdev_dev->reg_base[VDEC_HW_MISC] =
> >>              devm_platform_ioremap_resource(pdev, 0);
> >>      if (IS_ERR((__force void *)subdev_dev->reg_base[VDEC_HW_MISC])) {
> >>              ret = PTR_ERR((__force void *)subdev_dev->reg_base[VDEC_HW_MISC]);
> >> -            goto err;
> >> +            return ret;
> >>      }
> >>
> >>      if (!main_dev->subdev_prob_done)
> >>              main_dev->subdev_prob_done = mtk_vdec_hw_prob_done;
> >>
> >>      platform_set_drvdata(pdev, subdev_dev);
> >> -    return 0;
> >> -err:
> >> -    pm_runtime_disable(subdev_dev->pm.dev);
> >> -    return ret;
> >> -}
> >> -
> >> -static int mtk_vdec_hw_remove(struct platform_device *pdev)
> >> -{
> >> -    pm_runtime_disable(&pdev->dev);
> >> -
> >>      return 0;
> >>  }
> >>
> >>  static struct platform_driver mtk_vdec_driver = {
> >>      .probe  = mtk_vdec_hw_probe,
> >> -    .remove = mtk_vdec_hw_remove,
> >>      .driver = {
> >>              .name   = "mtk-vdec-comp",
> >>              .of_match_table = mtk_vdec_hw_match,
> >
>
Hans Verkuil May 23, 2023, 3:22 p.m. UTC | #6 
On 23/05/2023 17:18, Fei Shao wrote:
> On Tue, May 23, 2023 at 8:28 PM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
>>
>> On 23/05/2023 13:42, Hans Verkuil wrote:
>>> On 15/05/2023 08:16, Fei Shao wrote:
>>>> Convert pm_runtime_enable() to the managed version, and clean up error
>>>> handling and unnecessary .remove() callback accordingly.
>>>
>>> This patch no longer applies. Can you make a v3?
>>
>> Sorry, you can ignore this. I now realize that this was a v2 of
>>
>> https://patchwork.linuxtv.org/project/linux-media/patch/20230510233117.1.I7047714f92ef7569bd21f118ae6aee20b3175a92@changeid/
>>
>> I had that v1 applied, so obviously this v2 would fail to apply. After dropping
>> that v1 patch it now applies cleanly.
> 
> Ack, many thanks.
> BTW, besides the review tags (appreciation to all), would you mind
> adding this tag also as I missed it in the beginning?
> 
> Suggested-by: Chen-Yu Tsai <wenst@chromium.org>

Done!

Regards,

	Hans

> 
> Regards,
> Fei
> 
> 
> 
> 
> 
>>
>> Regards,
>>
>>         Hans
>>
>>>
>>> Regards,
>>>
>>>       Hans
>>>
>>>>
>>>> Signed-off-by: Fei Shao <fshao@chromium.org>
>>>>
>>>> ---
>>>>
>>>> Changes in v2:
>>>> Use devm_pm_runtime_enable() per suggestion from the previous thread:
>>>> https://lore.kernel.org/lkml/20230510164330.z2ygkl7vws6fci75@pengutronix.de/T/#m25be91afe3e9554600e859a8a59128ca234fc63d
>>>>
>>>>  .../mediatek/vcodec/mtk_vcodec_dec_hw.c       | 26 ++++++-------------
>>>>  1 file changed, 8 insertions(+), 18 deletions(-)
>>>>
>>>> diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_hw.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_hw.c
>>>> index b753bf54ebd9..e1cb2f8dca33 100644
>>>> --- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_hw.c
>>>> +++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_hw.c
>>>> @@ -148,20 +148,21 @@ static int mtk_vdec_hw_probe(struct platform_device *pdev)
>>>>      ret = mtk_vcodec_init_dec_clk(pdev, &subdev_dev->pm);
>>>>      if (ret)
>>>>              return ret;
>>>> -    pm_runtime_enable(&pdev->dev);
>>>> +
>>>> +    ret = devm_pm_runtime_enable(&pdev->dev);
>>>> +    if (ret)
>>>> +            return ret;
>>>>
>>>>      of_id = of_match_device(mtk_vdec_hw_match, dev);
>>>>      if (!of_id) {
>>>>              dev_err(dev, "Can't get vdec subdev id.\n");
>>>> -            ret = -EINVAL;
>>>> -            goto err;
>>>> +            return -EINVAL;
>>>>      }
>>>>
>>>>      hw_idx = (enum mtk_vdec_hw_id)(uintptr_t)of_id->data;
>>>>      if (hw_idx >= MTK_VDEC_HW_MAX) {
>>>>              dev_err(dev, "Hardware index %d not correct.\n", hw_idx);
>>>> -            ret = -EINVAL;
>>>> -            goto err;
>>>> +            return -EINVAL;
>>>>      }
>>>>
>>>>      main_dev->subdev_dev[hw_idx] = subdev_dev;
>>>> @@ -173,36 +174,25 @@ static int mtk_vdec_hw_probe(struct platform_device *pdev)
>>>>      if (IS_SUPPORT_VDEC_HW_IRQ(hw_idx)) {
>>>>              ret = mtk_vdec_hw_init_irq(subdev_dev);
>>>>              if (ret)
>>>> -                    goto err;
>>>> +                    return ret;
>>>>      }
>>>>
>>>>      subdev_dev->reg_base[VDEC_HW_MISC] =
>>>>              devm_platform_ioremap_resource(pdev, 0);
>>>>      if (IS_ERR((__force void *)subdev_dev->reg_base[VDEC_HW_MISC])) {
>>>>              ret = PTR_ERR((__force void *)subdev_dev->reg_base[VDEC_HW_MISC]);
>>>> -            goto err;
>>>> +            return ret;
>>>>      }
>>>>
>>>>      if (!main_dev->subdev_prob_done)
>>>>              main_dev->subdev_prob_done = mtk_vdec_hw_prob_done;
>>>>
>>>>      platform_set_drvdata(pdev, subdev_dev);
>>>> -    return 0;
>>>> -err:
>>>> -    pm_runtime_disable(subdev_dev->pm.dev);
>>>> -    return ret;
>>>> -}
>>>> -
>>>> -static int mtk_vdec_hw_remove(struct platform_device *pdev)
>>>> -{
>>>> -    pm_runtime_disable(&pdev->dev);
>>>> -
>>>>      return 0;
>>>>  }
>>>>
>>>>  static struct platform_driver mtk_vdec_driver = {
>>>>      .probe  = mtk_vdec_hw_probe,
>>>> -    .remove = mtk_vdec_hw_remove,
>>>>      .driver = {
>>>>              .name   = "mtk-vdec-comp",
>>>>              .of_match_table = mtk_vdec_hw_match,
>>>
>>
diff mbox series

Patch

diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_hw.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_hw.c
index b753bf54ebd9..e1cb2f8dca33 100644
--- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_hw.c
+++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_hw.c
@@ -148,20 +148,21 @@  static int mtk_vdec_hw_probe(struct platform_device *pdev)
 	ret = mtk_vcodec_init_dec_clk(pdev, &subdev_dev->pm);
 	if (ret)
 		return ret;
-	pm_runtime_enable(&pdev->dev);
+
+	ret = devm_pm_runtime_enable(&pdev->dev);
+	if (ret)
+		return ret;
 
 	of_id = of_match_device(mtk_vdec_hw_match, dev);
 	if (!of_id) {
 		dev_err(dev, "Can't get vdec subdev id.\n");
-		ret = -EINVAL;
-		goto err;
+		return -EINVAL;
 	}
 
 	hw_idx = (enum mtk_vdec_hw_id)(uintptr_t)of_id->data;
 	if (hw_idx >= MTK_VDEC_HW_MAX) {
 		dev_err(dev, "Hardware index %d not correct.\n", hw_idx);
-		ret = -EINVAL;
-		goto err;
+		return -EINVAL;
 	}
 
 	main_dev->subdev_dev[hw_idx] = subdev_dev;
@@ -173,36 +174,25 @@  static int mtk_vdec_hw_probe(struct platform_device *pdev)
 	if (IS_SUPPORT_VDEC_HW_IRQ(hw_idx)) {
 		ret = mtk_vdec_hw_init_irq(subdev_dev);
 		if (ret)
-			goto err;
+			return ret;
 	}
 
 	subdev_dev->reg_base[VDEC_HW_MISC] =
 		devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR((__force void *)subdev_dev->reg_base[VDEC_HW_MISC])) {
 		ret = PTR_ERR((__force void *)subdev_dev->reg_base[VDEC_HW_MISC]);
-		goto err;
+		return ret;
 	}
 
 	if (!main_dev->subdev_prob_done)
 		main_dev->subdev_prob_done = mtk_vdec_hw_prob_done;
 
 	platform_set_drvdata(pdev, subdev_dev);
-	return 0;
-err:
-	pm_runtime_disable(subdev_dev->pm.dev);
-	return ret;
-}
-
-static int mtk_vdec_hw_remove(struct platform_device *pdev)
-{
-	pm_runtime_disable(&pdev->dev);
-
 	return 0;
 }
 
 static struct platform_driver mtk_vdec_driver = {
 	.probe	= mtk_vdec_hw_probe,
-	.remove = mtk_vdec_hw_remove,
 	.driver	= {
 		.name	= "mtk-vdec-comp",
 		.of_match_table = mtk_vdec_hw_match,