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@xxxxxxxxxxxx>
Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx>
> ---
> 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
