Re: [PATCH v2 1/4] media: rcar-csi2: Clarify usage of mbps and msps

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

Hi Niklas,

Thank you for the patch.

On Sun, May 11, 2025 at 07:47:27PM +0200, Niklas Söderlund wrote:
> The helper function to deal with calculating the link speed is designed
> in such a way that it returns the correct type bps (bits per second) for
> D-PHY and sps (symbols per second) for C-PHY. And for historical reasons
> the function kept the name mbps.
> 
> This is confusing, fix it by having the function only deal with bps
> values as this is the most common use-case and convert bps to sps in the
> only function where it is needed to configure the C-PHY.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

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

> ---
>  drivers/media/platform/renesas/rcar-csi2.c | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/media/platform/renesas/rcar-csi2.c b/drivers/media/platform/renesas/rcar-csi2.c
> index 9979de4f6ef1..358e7470befc 100644
> --- a/drivers/media/platform/renesas/rcar-csi2.c
> +++ b/drivers/media/platform/renesas/rcar-csi2.c
> @@ -975,10 +975,6 @@ static int rcsi2_calc_mbps(struct rcar_csi2 *priv, unsigned int bpp,
>  
>  	mbps = div_u64(freq * 2, MEGA);
>  
> -	/* Adjust for C-PHY, divide by 2.8. */
> -	if (priv->cphy)
> -		mbps = div_u64(mbps * 5, 14);
> -
>  	return mbps;
>  }
>  
> @@ -1203,9 +1199,13 @@ static int rcsi2_wait_phy_start_v4h(struct rcar_csi2 *priv, u32 match)
>  	return -ETIMEDOUT;
>  }
>  
> -static int rcsi2_c_phy_setting_v4h(struct rcar_csi2 *priv, int msps)
> +static int rcsi2_c_phy_setting_v4h(struct rcar_csi2 *priv, int mbps)
>  {
>  	const struct rcsi2_cphy_setting *conf;
> +	int msps;
> +
> +	/* Adjust for C-PHY symbols, divide by 2.8. */
> +	msps = div_u64(mbps * 5, 14);
>  
>  	for (conf = cphy_setting_table_r8a779g0; conf->msps != 0; conf++) {
>  		if (conf->msps > msps)
> @@ -1301,7 +1301,7 @@ static int rcsi2_start_receiver_v4h(struct rcar_csi2 *priv,
>  	const struct rcar_csi2_format *format;
>  	const struct v4l2_mbus_framefmt *fmt;
>  	unsigned int lanes;
> -	int msps;
> +	int mbps;
>  	int ret;
>  
>  	/* Use the format on the sink pad to compute the receiver config. */
> @@ -1314,9 +1314,9 @@ static int rcsi2_start_receiver_v4h(struct rcar_csi2 *priv,
>  	if (ret)
>  		return ret;
>  
> -	msps = rcsi2_calc_mbps(priv, format->bpp, lanes);
> -	if (msps < 0)
> -		return msps;
> +	mbps = rcsi2_calc_mbps(priv, format->bpp, lanes);
> +	if (mbps < 0)
> +		return mbps;
>  
>  	/* Reset LINK and PHY*/
>  	rcsi2_write(priv, V4H_CSI2_RESETN_REG, 0);
> @@ -1352,7 +1352,7 @@ static int rcsi2_start_receiver_v4h(struct rcar_csi2 *priv,
>  	rcsi2_write16(priv, V4H_PPI_RW_COMMON_CFG_REG, 0x0003);
>  
>  	/* C-PHY settings */
> -	ret = rcsi2_c_phy_setting_v4h(priv, msps);
> +	ret = rcsi2_c_phy_setting_v4h(priv, mbps);
>  	if (ret)
>  		return ret;
>  
> @@ -1363,7 +1363,7 @@ static int rcsi2_start_receiver_v4h(struct rcar_csi2 *priv,
>  	return 0;
>  }
>  
> -static int rcsi2_d_phy_setting_v4m(struct rcar_csi2 *priv, int data_rate)
> +static int rcsi2_d_phy_setting_v4m(struct rcar_csi2 *priv, int mbps)
>  {
>  	unsigned int timeout;
>  	int ret;

-- 
Regards,

Laurent Pinchart