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

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

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