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

[Hans Verkuil]

On 23/05/2023 17:18, Fei Shao wrote:
> On Tue, May 23, 2023 at 8:28 PM Hans Verkuil <hverkuil-cisco@xxxxxxxxx> 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@xxxxxxxxxxxx>

Done!

Regards,

	Hans

> 
> Regards,
> Fei
> 
> 
> 
> 
> 
>>
>> Regards,
>>
>>         Hans
>>
>>>
>>> Regards,
>>>
>>>       Hans
>>>
>>>>
>>>> Signed-off-by: Fei Shao <fshao@xxxxxxxxxxxx>
>>>>
>>>> ---
>>>>
>>>> Changes in v2:
>>>> Use devm_pm_runtime_enable() per suggestion from the previous thread:
>>>> https://lore.kernel.org/lkml/20230510164330.z2ygkl7vws6fci75@xxxxxxxxxxxxxx/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,
>>>
>>