Re: [PATCH 3/3] media: vsp1: Add support for RZ/G2L VSPD

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Biju,

On Wed, Mar 9, 2022 at 8:45 PM Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote:
> The RZ/G2L VSPD provides a single VSPD instance. It has the following
> sub modules MAU, CTU, RPF, DPR, LUT, BRS, WPF and LIF.
>
> The VSPD block on RZ/G2L does not have a version register, so added a
> new compatible string "renesas,vsp2-rzg2l" with a data pointer containing
> the info structure. Also the reset line is shared with the DU module
> so devm_reset_control_get_shared() call is used in case of RZ/G2L.
>
> Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
> ---
> RFC->v1:
>  * Used data pointer containing info structure to retrieve version information
> RFC:
>  * https://patchwork.kernel.org/project/linux-renesas-soc/patch/20220112174612.10773-21-biju.das.jz@xxxxxxxxxxxxxx/

Thanks for the update!

> --- a/drivers/media/platform/vsp1/vsp1_drv.c
> +++ b/drivers/media/platform/vsp1/vsp1_drv.c

> @@ -841,7 +849,14 @@ static int vsp1_probe(struct platform_device *pdev)
>         if (irq < 0)
>                 return irq;
>
> -       vsp1->rstc = devm_reset_control_get_exclusive(&pdev->dev, NULL);
> +       vsp1->info = of_device_get_match_data(&pdev->dev);
> +       if (vsp1->info) {
> +               vsp1->version = vsp1->info->version;
> +               vsp1->rstc = devm_reset_control_get_shared(&pdev->dev, NULL);
> +       } else {
> +               vsp1->rstc = devm_reset_control_get_exclusive(&pdev->dev, NULL);

Making the reset control shared or exclusive dependent on the presence
of match data looks fragile to me.  I think you want to check the IP
version instead (ideally, the SoC, as this is an integration feature).
Or just make it shared unconditionally (in the previous patch)?

> +       }
> +
>         if (IS_ERR(vsp1->rstc))
>                 return dev_err_probe(&pdev->dev, PTR_ERR(vsp1->rstc),
>                                      "failed to get reset ctrl\n");
> @@ -874,13 +889,15 @@ static int vsp1_probe(struct platform_device *pdev)
>         if (ret < 0)
>                 goto done;
>
> -       vsp1->version = vsp1_read(vsp1, VI6_IP_VERSION);
> +       if (!vsp1->info) {
> +               vsp1->version = vsp1_read(vsp1, VI6_IP_VERSION);
>
> -       for (i = 0; i < ARRAY_SIZE(vsp1_device_infos); ++i) {
> -               if ((vsp1->version & VI6_IP_VERSION_MODEL_MASK) ==
> -                   vsp1_device_infos[i].version) {
> -                       vsp1->info = &vsp1_device_infos[i];
> -                       break;
> +               for (i = 0; i < ARRAY_SIZE(vsp1_device_infos); ++i) {
> +                       if ((vsp1->version & VI6_IP_VERSION_MODEL_MASK) ==
> +                           vsp1_device_infos[i].version) {
> +                               vsp1->info = &vsp1_device_infos[i];
> +                               break;
> +                       }
>                 }
>         }
>
> @@ -943,6 +960,7 @@ static int vsp1_remove(struct platform_device *pdev)
>  static const struct of_device_id vsp1_of_match[] = {
>         { .compatible = "renesas,vsp1" },
>         { .compatible = "renesas,vsp2" },
> +       { .compatible = "renesas,vsp2-rzg2l", .data = &vsp1_device_infos[14] },
>         { },

Is VI6_IP_VERSION_MODEL_VSPD_RZG2L = 0x1b an official number?
If yes, it might make sense to change the compatible value to
"renesas,vsp2-0x1b".

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux