Re: [PATCH v2 10/18] media: rzg2l-cru: csi2: Add support for RZ/V2H(P) SoC

Laurent Pinchart
2025-02-23T18:24:27Z

Hi Tommaso,

Thank you for the patch.

On Fri, Feb 21, 2025 at 04:55:24PM +0100, Tommaso Merciai wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> The D-PHY on the RZ/V2H(P) SoC is different from the D-PHY on the RZ/G2L
> SoC. To handle this difference, function pointers for D-PHY enable/disable
> have been added, and the `struct rzg2l_csi2_info` pointer is passed as OF
> data.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> ---
> .../platform/renesas/rzg2l-cru/rzg2l-csi2.c | 94 +++++++++++++++++++
> 1 file changed, 94 insertions(+)
>
> diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
> index 771fa35558be..cd452c84f101 100644
> --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
> +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c
> @@ -85,6 +85,15 @@
> CSIDPHYSKW0_UTIL_DL2_SKW_ADJ(1) | \
> CSIDPHYSKW0_UTIL_DL3_SKW_ADJ(1))
>
> +/* DPHY registers on RZ/V2H(P) SoC */
> +#define CRUm_S_TIMCTL 0x41c
> +#define CRUm_S_TIMCTL_S_HSSETTLECTL(x) ((x) << 8)
> +
> +#define CRUm_S_DPHYCTL_MSB 0x434
> +#define CRUm_S_DPHYCTL_MSB_DESKEW BIT(1)
> +
> +#define CRUm_SWAPCTL 0x438
> +
> #define VSRSTS_RETRIES 20
>
> #define RZG2L_CSI2_MIN_WIDTH 320
> @@ -139,6 +148,30 @@ struct rzg2l_csi2_timings {
> u32 max_hsfreq;
> };
>
> +struct rzv2h_csi2_s_hssettlectl {
> + unsigned int hsfreq;
> + u16 s_hssettlectl;
> +};
> +
> +static const struct rzv2h_csi2_s_hssettlectl rzv2h_s_hssettlectl[] = {
> + { 90, 1 }, { 130, 2 }, { 180, 3 },
> + { 220, 4 }, { 270, 5 }, { 310, 6 },
> + { 360, 7 }, { 400, 8 }, { 450, 9 },
> + { 490, 10 }, { 540, 11 }, { 580, 12 },
> + { 630, 13 }, { 670, 14 }, { 720, 15 },
> + { 760, 16 }, { 810, 17 }, { 850, 18 },
> + { 900, 19 }, { 940, 20 }, { 990, 21 },
> + { 1030, 22 }, { 1080, 23 }, { 1120, 24 },
> + { 1170, 25 }, { 1220, 26 }, { 1260, 27 },
> + { 1310, 28 }, { 1350, 29 }, { 1400, 30 },
> + { 1440, 31 }, { 1490, 32 }, { 1530, 33 },
> + { 1580, 34 }, { 1620, 35 }, { 1670, 36 },
> + { 1710, 37 }, { 1760, 38 }, { 1800, 39 },
> + { 1850, 40 }, { 1890, 41 }, { 1940, 42 },
> + { 1980, 43 }, { 2030, 44 }, { 2070, 45 },
> + { 2100, 46 },
> +};
> +
> static const struct rzg2l_csi2_timings rzg2l_csi2_global_timings[] = {
> {
> .max_hsfreq = 80,
> @@ -427,6 +460,58 @@ static int rzg2l_csi2_mipi_link_disable(struct rzg2l_csi2 *csi2)
> return 0;
> }
>
> +static int rzv2h_csi2_dphy_disable(struct rzg2l_csi2 *csi2)
> +{
> + int ret;
> +
> + /* Reset the CRU (D-PHY) */
> + ret = reset_control_assert(csi2->cmn_rstb);
> + if (ret)
> + return ret;
> +
> + csi2->dphy_enabled = false;
> +
> + return 0;
> +}
> +
> +static int rzv2h_csi2_dphy_enable(struct rzg2l_csi2 *csi2)
> +{
> + unsigned int i;
> + u16 hssettle;
> + int mbps;
> +
> + mbps = rzg2l_csi2_calc_mbps(csi2);
> + if (mbps < 0)
> + return mbps;
> +
> + csi2->hsfreq = mbps;
> +
> + rzg2l_csi2_write(csi2, CRUm_SWAPCTL, 0);
> +

Maybe move this after the error check below to avoid writing any
register if an error occurs.

> + for (i = 0; i < ARRAY_SIZE(rzv2h_s_hssettlectl); i++) {
> + if (csi2->hsfreq <= rzv2h_s_hssettlectl[i].hsfreq)
> + break;
> + }
> +
> + if (i == ARRAY_SIZE(rzv2h_s_hssettlectl))
> + return -EINVAL;
> +
> + hssettle = rzv2h_s_hssettlectl[i].s_hssettlectl;
> + rzg2l_csi2_write(csi2, CRUm_S_TIMCTL,
> + CRUm_S_TIMCTL_S_HSSETTLECTL(hssettle));
> +
> + if (csi2->hsfreq > 1500)
> + rzg2l_csi2_set(csi2, CRUm_S_DPHYCTL_MSB,
> + CRUm_S_DPHYCTL_MSB_DESKEW);
> + else
> + rzg2l_csi2_clr(csi2, CRUm_S_DPHYCTL_MSB,
> + CRUm_S_DPHYCTL_MSB_DESKEW);
> +
> + csi2->dphy_enabled = true;
> +
> + return 0;
> +}
> +
> static int rzg2l_csi2_mipi_link_setting(struct v4l2_subdev *sd, bool on)
> {
> struct rzg2l_csi2 *csi2 = sd_to_csi2(sd);
> @@ -900,12 +985,21 @@ static const struct dev_pm_ops rzg2l_csi2_pm_ops = {
> rzg2l_csi2_pm_runtime_resume, NULL)
> };
>
> +static const struct rzg2l_csi2_info rzv2h_csi2_info = {
> + .dphy_enable = rzv2h_csi2_dphy_enable,
> + .dphy_disable = rzv2h_csi2_dphy_disable,
> +};

Move this just after rzv2h_csi2_dphy_enable().

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

> +
> static const struct rzg2l_csi2_info rzg2l_csi2_info = {
> .dphy_enable = rzg2l_csi2_dphy_enable,
> .dphy_disable = rzg2l_csi2_dphy_disable,
> };
>
> static const struct of_device_id rzg2l_csi2_of_table[] = {
> + {
> + .compatible = "renesas,r9a09g057-csi2",
> + .data = &rzv2h_csi2_info,
> + },
> {
> .compatible = "renesas,rzg2l-csi2",
> .data = &rzg2l_csi2_info,

--
Regards,

Laurent Pinchart