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
