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

[Fei Shao]

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>

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,
> >
>