Re: [PATCH 2/3] media: i2c: imx283: add support for non-continuous MIPI clock mode

[Kieran Bingham <kieran.bingham@ideasonboard.com>]

Quoting Matthias Fend (2025-12-17 07:06:01)
> Add support for selecting between continuous and non-continuous MIPI clock
> mode.
> 
> Previously, the CSI-2 non-continuous clock endpoint flag was ignored and
> the sensor was always configured for non-continuous clock mode. For
> existing device tree nodes that do not have this property enabled, this
> update will therefore change the actual clock mode.

So this means that not specifying non-continous will now enforce
continuous mode on existing devices ?

Are there any implications to that? I know there are quite a few users
of the sensor on Raspberry Pi for instance.

I think it should be fine though.

> Signed-off-by: Matthias Fend <matthias.fend@emfend.at>
> ---
>  drivers/media/i2c/imx283.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/media/i2c/imx283.c b/drivers/media/i2c/imx283.c
> index 8ab63ad8f385f6e2a2d7432feff0af09a5356dc4..7a6ab2941ea985401b21d60163b58e980cf31ddc 100644
> --- a/drivers/media/i2c/imx283.c
> +++ b/drivers/media/i2c/imx283.c
> @@ -149,6 +149,9 @@
>  #define IMX283_REG_PLSTMG02            CCI_REG8(0x36aa)
>  #define   IMX283_PLSTMG02_VAL          0x00
>  
> +#define IMX283_REG_MIPI_CLK            CCI_REG8(0x3a43)
> +#define   IMX283_MIPI_CLK_NONCONTINUOUS        BIT(0)

I can't find this in my datasheet, so no specific comment on the
register I'm afraid. Did you get this from the vendor or is it an
assumption from other sony drivers?

I assume you can measurably detect that this register impacts the clock
on a CSI scope or such perhaps from the receiver?

So even with all that I think this appears to be correct. I'll test it
more when I can but otherwise:


Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> +
>  #define IMX283_REG_EBD_X_OUT_SIZE      CCI_REG16_LE(0x3a54)
>  
>  /* Test pattern generator */
> @@ -565,6 +568,7 @@ struct imx283 {
>         struct v4l2_ctrl *hblank;
>         struct v4l2_ctrl *vflip;
>  
> +       bool mipi_clk_noncontinuous;
>         unsigned long link_freq_bitmap;
>  
>         u16 hmax;
> @@ -988,6 +992,7 @@ static int imx283_set_pad_format(struct v4l2_subdev *sd,
>  static int imx283_standby_cancel(struct imx283 *imx283)
>  {
>         unsigned int link_freq_idx;
> +       u8 mipi_clk;
>         int ret = 0;
>  
>         cci_write(imx283->cci, IMX283_REG_STANDBY,
> @@ -1007,6 +1012,10 @@ static int imx283_standby_cancel(struct imx283 *imx283)
>         /* Enable PLL */
>         cci_write(imx283->cci, IMX283_REG_STBPL, IMX283_STBPL_NORMAL, &ret);
>  
> +       /* Configure MIPI clock mode */
> +       mipi_clk = imx283->mipi_clk_noncontinuous ? IMX283_MIPI_CLK_NONCONTINUOUS : 0;
> +       cci_write(imx283->cci, IMX283_REG_MIPI_CLK, mipi_clk, &ret);
> +
>         /* Configure the MIPI link speed */
>         link_freq_idx = __ffs(imx283->link_freq_bitmap);
>         cci_multi_reg_write(imx283->cci, link_freq_reglist[link_freq_idx].regs,
> @@ -1426,6 +1435,9 @@ static int imx283_parse_endpoint(struct imx283 *imx283)
>                 goto done_endpoint_free;
>         }
>  
> +       imx283->mipi_clk_noncontinuous =
> +               bus_cfg.bus.mipi_csi2.flags & V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK;
> +
>         ret = v4l2_link_freq_to_bitmap(imx283->dev, bus_cfg.link_frequencies,
>                                        bus_cfg.nr_of_link_frequencies,
>                                        link_frequencies, ARRAY_SIZE(link_frequencies),
> 
> -- 
> 2.34.1
>