Re: [PATCH] media: i2c: imx355: Add support for 24 MHz external clock

[David Heidelberg]

On 17/06/2026 19:40, Dave Stevenson wrote:

Hi David

On Thu, 16 Apr 2026 at 12:26, Dave Stevenson
<dave.steven...@raspberrypi.com> wrote:

Hi David

On Tue, 14 Apr 2026 at 11:17, David Heidelberg via B4 Relay
<devnull+david.ixit...@kernel.org> wrote:

From: David Heidelberg <da...@ixit.cz>

The IMX355 sensor supports multiple external clock frequencies,
including 19.2 MHz and 24 MHz. The driver currently supports only
fixed 19.2 MHz input clock.

Refactor the clock handling to make the PLL configuration dependent
on the external clock frequency and add support for 24 MHz. Introduce
a table of clock parameter sets and program the corresponding EXTCLK
frequency and PLL multipliers to maintain consistent internal VCO
frequencies across supported inputs.

The PLL settings are adjusted so that:
   - VT VCO remains at 1152 MHz
   - OP VCO remains at 720 MHz

This preserves existing timing characteristics while allowing systems
using a 24 MHz clock to operate correctly.

I happened to have someone asking for this same requirement, so tried
out your patch.

I don't have a datasheet for IMX355, but the patterns very closely
follow IMX477 and IMX708 for which I do.
Those both have a single PLL and a dual PLL mode selected via register
PLL_MULT_DRIV (0x0310). The imx355 driver is setting that to 0 for
single PLL mode, which means that the PREDIV_IVT (0x0305) and MPY_IVT
PLL (0x0306/7) settings do nothing as the IVT block is driven from
IOPCK.

I now have the datasheet and software reference manual for IMX355.

It says that dual PLL mode is not available.
"In PLL single mode, IOP_PREPLLCK_DIV and IOP_PLL_MPY are applied to
IOPCK PLL, and IVT_PREPLLCK_DIV and IVT_PLL_MPY are ignored".
So there is no need to have alternate pll_vt_mpy values for 24MHz.

I have no idea why Sony say that dual PLL mode isn't available. When I
was adding the 2 lane support I used it without any issues. I'm now
reworking those patches to avoid dual PLL mode.

Maybe there is some ERRATA which makes it unstable or problematic?

Then again the datasheet also says that LINE_LENGTH_PCK is constrained
to 3672 in full res and 4x4 binning mode, and 1836 in 2x2 binned modes
"to avoid sensor internal interference (FPN)", so they obviously hit
some odd behaviours and just added constraints.

Do you wish to send a V2 dropping pll_vt_mpy, or shall I pull it into my series?

What works for you better (since u sending bigger changes than I do).

David

Thanks
   Dave

Your patch therefore works, but does more than is necessary - you
really can set pll_vt_mpy to any value and it works exactly the same.
I guess changing the unconnected IVTCK clock within the sensor could
feasibly change EMC emissions, but that feels pretty unlikely.
Possibly add a comment to your existing comment of "VT VCO = 1152 MHz"
to say that it's unused to avoid others going down the rabbit hole I
encountered.

(For those referencing the other datasheets, the description for
single PLL mode lists configuring IVT_PREPLLCK_DIV and IVT_PLL_MPY,
but the diagram shows that IOP is always driven from IOPCK, and IVT is
muxed between IOPCK and IVTCK)

No functional change for existing 19.2 MHz users.

Assisted-by: Claude:claude-opus-4-6
Signed-off-by: David Heidelberg <da...@ixit.cz>

Whilst useful comments could be added, it does what it says and works:

Reviewed-by: Dave Stevenson <dave.steven...@raspberrypi.com>
Tested-by: Dave Stevenson <dave.steven...@raspberrypi.com>

---
Known users: Pixel 3 and 3a.
---
  drivers/media/i2c/imx355.c | 114 +++++++++++++++++++++------------------------
  1 file changed, 54 insertions(+), 60 deletions(-)

diff --git a/drivers/media/i2c/imx355.c b/drivers/media/i2c/imx355.c
index 27a5c212a527f..f9ec13bb27d10 100644
--- a/drivers/media/i2c/imx355.c
+++ b/drivers/media/i2c/imx355.c
@@ -25,6 +25,11 @@
  #define IMX355_REG_CHIP_ID             0x0016
  #define IMX355_CHIP_ID                 0x0355

+/* PLL registers that depend on the external clock frequency */
+#define IMX355_REG_EXTCLK_FREQ         0x0136
+#define IMX355_REG_PLL_VT_MUL          0x0306
+#define IMX355_REG_PLL_OP_MUL          0x030e
+
  /* V_TIMING internal */
  #define IMX355_REG_FLL                 0x0340
  #define IMX355_FLL_MAX                 0xffff
@@ -63,7 +68,6 @@

  /* default link frequency and external clock */
  #define IMX355_LINK_FREQ_DEFAULT       360000000LL
-#define IMX355_EXT_CLK                 19200000
  #define IMX355_LINK_FREQ_INDEX         0

  /* number of data lanes */
@@ -100,6 +104,33 @@ struct imx355_mode {
         struct imx355_reg_list reg_list;
  };

+struct imx355_clk_params {
+       u32 ext_clk;
+       u16 extclk_freq; /* External clock (MHz) in 8.8 fixed point) */
+       u16 pll_vt_mpy; /* VT system PLL multiplier */
+       u16 pll_op_mpy; /* OP system PLL multiplier */
+};
+
+/*
+ * All modes use the same PLL dividers (PREPLLCK_VT_DIV=2, PREPLLCK_OP_DIV=2),
+ * so the multipliers are adjusted to produce the same VCO frequencies:
+ *   VT VCO = 1152 MHz, OP VCO = 720 MHz
+ */
+static const struct imx355_clk_params imx355_clk_params[] = {
+       {
+               .ext_clk = 19200000,
+               .extclk_freq = 0x1333,  /* 19.2 MHz */
+               .pll_vt_mpy = 120,      /* 19.2 / 2 * 120 = 1152 MHz */
+               .pll_op_mpy = 75,       /* 19.2 / 2 * 75  = 720 MHz */
+       },
+       {
+               .ext_clk = 24000000,
+               .extclk_freq = 0x1800,  /* 24.0 MHz */
+               .pll_vt_mpy = 96,       /* 24.0 / 2 * 96  = 1152 MHz */
+               .pll_op_mpy = 60,       /* 24.0 / 2 * 60  = 720 MHz */
+       },
+};
+
  struct imx355_hwcfg {
         unsigned long link_freq_bitmap;
  };
@@ -125,6 +156,7 @@ struct imx355 {
         const struct imx355_mode *cur_mode;

         struct imx355_hwcfg *hwcfg;
+       const struct imx355_clk_params *clk_params;

         /*
          * Mutex for serialized access:
@@ -144,8 +176,6 @@ static const struct regulator_bulk_data imx355_supplies[] = 
{
  };

  static const struct imx355_reg imx355_global_regs[] = {
-       { 0x0136, 0x13 },
-       { 0x0137, 0x33 },
         { 0x304e, 0x03 },
         { 0x4348, 0x16 },
         { 0x4350, 0x19 },
@@ -231,12 +261,8 @@ static const struct imx355_reg mode_3268x2448_regs[] = {
         { 0x0301, 0x05 },
         { 0x0303, 0x01 },
         { 0x0305, 0x02 },
-       { 0x0306, 0x00 },
-       { 0x0307, 0x78 },
         { 0x030b, 0x01 },
         { 0x030d, 0x02 },
-       { 0x030e, 0x00 },
-       { 0x030f, 0x4b },
         { 0x0310, 0x00 },
         { 0x0700, 0x00 },
         { 0x0701, 0x10 },
@@ -280,12 +306,8 @@ static const struct imx355_reg mode_3264x2448_regs[] = {
         { 0x0301, 0x05 },
         { 0x0303, 0x01 },
         { 0x0305, 0x02 },
-       { 0x0306, 0x00 },
-       { 0x0307, 0x78 },
         { 0x030b, 0x01 },
         { 0x030d, 0x02 },
-       { 0x030e, 0x00 },
-       { 0x030f, 0x4b },
         { 0x0310, 0x00 },
         { 0x0700, 0x00 },
         { 0x0701, 0x10 },
@@ -329,12 +351,8 @@ static const struct imx355_reg mode_3280x2464_regs[] = {
         { 0x0301, 0x05 },
         { 0x0303, 0x01 },
         { 0x0305, 0x02 },
-       { 0x0306, 0x00 },
-       { 0x0307, 0x78 },
         { 0x030b, 0x01 },
         { 0x030d, 0x02 },
-       { 0x030e, 0x00 },
-       { 0x030f, 0x4b },
         { 0x0310, 0x00 },
         { 0x0700, 0x00 },
         { 0x0701, 0x10 },
@@ -378,12 +396,8 @@ static const struct imx355_reg mode_1940x1096_regs[] = {
         { 0x0301, 0x05 },
         { 0x0303, 0x01 },
         { 0x0305, 0x02 },
-       { 0x0306, 0x00 },
-       { 0x0307, 0x78 },
         { 0x030b, 0x01 },
         { 0x030d, 0x02 },
-       { 0x030e, 0x00 },
-       { 0x030f, 0x4b },
         { 0x0310, 0x00 },
         { 0x0700, 0x00 },
         { 0x0701, 0x10 },
@@ -427,12 +441,8 @@ static const struct imx355_reg mode_1936x1096_regs[] = {
         { 0x0301, 0x05 },
         { 0x0303, 0x01 },
         { 0x0305, 0x02 },
-       { 0x0306, 0x00 },
-       { 0x0307, 0x78 },
         { 0x030b, 0x01 },
         { 0x030d, 0x02 },
-       { 0x030e, 0x00 },
-       { 0x030f, 0x4b },
         { 0x0310, 0x00 },
         { 0x0700, 0x00 },
         { 0x0701, 0x10 },
@@ -476,12 +486,8 @@ static const struct imx355_reg mode_1924x1080_regs[] = {
         { 0x0301, 0x05 },
         { 0x0303, 0x01 },
         { 0x0305, 0x02 },
-       { 0x0306, 0x00 },
-       { 0x0307, 0x78 },
         { 0x030b, 0x01 },
         { 0x030d, 0x02 },
-       { 0x030e, 0x00 },
-       { 0x030f, 0x4b },
         { 0x0310, 0x00 },
         { 0x0700, 0x00 },
         { 0x0701, 0x10 },
@@ -525,12 +531,8 @@ static const struct imx355_reg mode_1920x1080_regs[] = {
         { 0x0301, 0x05 },
         { 0x0303, 0x01 },
         { 0x0305, 0x02 },
-       { 0x0306, 0x00 },
-       { 0x0307, 0x78 },
         { 0x030b, 0x01 },
         { 0x030d, 0x02 },
-       { 0x030e, 0x00 },
-       { 0x030f, 0x4b },
         { 0x0310, 0x00 },
         { 0x0700, 0x00 },
         { 0x0701, 0x10 },
@@ -574,12 +576,8 @@ static const struct imx355_reg mode_1640x1232_regs[] = {
         { 0x0301, 0x05 },
         { 0x0303, 0x01 },
         { 0x0305, 0x02 },
-       { 0x0306, 0x00 },
-       { 0x0307, 0x78 },
         { 0x030b, 0x01 },
         { 0x030d, 0x02 },
-       { 0x030e, 0x00 },
-       { 0x030f, 0x4b },
         { 0x0310, 0x00 },
         { 0x0700, 0x00 },
         { 0x0701, 0x10 },
@@ -623,12 +621,8 @@ static const struct imx355_reg mode_1640x922_regs[] = {
         { 0x0301, 0x05 },
         { 0x0303, 0x01 },
         { 0x0305, 0x02 },
-       { 0x0306, 0x00 },
-       { 0x0307, 0x78 },
         { 0x030b, 0x01 },
         { 0x030d, 0x02 },
-       { 0x030e, 0x00 },
-       { 0x030f, 0x4b },
         { 0x0310, 0x00 },
         { 0x0700, 0x00 },
         { 0x0701, 0x10 },
@@ -672,12 +666,8 @@ static const struct imx355_reg mode_1300x736_regs[] = {
         { 0x0301, 0x05 },
         { 0x0303, 0x01 },
         { 0x0305, 0x02 },
-       { 0x0306, 0x00 },
-       { 0x0307, 0x78 },
         { 0x030b, 0x01 },
         { 0x030d, 0x02 },
-       { 0x030e, 0x00 },
-       { 0x030f, 0x4b },
         { 0x0310, 0x00 },
         { 0x0700, 0x00 },
         { 0x0701, 0x10 },
@@ -721,12 +711,8 @@ static const struct imx355_reg mode_1296x736_regs[] = {
         { 0x0301, 0x05 },
         { 0x0303, 0x01 },
         { 0x0305, 0x02 },
-       { 0x0306, 0x00 },
-       { 0x0307, 0x78 },
         { 0x030b, 0x01 },
         { 0x030d, 0x02 },
-       { 0x030e, 0x00 },
-       { 0x030f, 0x4b },
         { 0x0310, 0x00 },
         { 0x0700, 0x00 },
         { 0x0701, 0x10 },
@@ -770,12 +756,8 @@ static const struct imx355_reg mode_1284x720_regs[] = {
         { 0x0301, 0x05 },
         { 0x0303, 0x01 },
         { 0x0305, 0x02 },
-       { 0x0306, 0x00 },
-       { 0x0307, 0x78 },
         { 0x030b, 0x01 },
         { 0x030d, 0x02 },
-       { 0x030e, 0x00 },
-       { 0x030f, 0x4b },
         { 0x0310, 0x00 },
         { 0x0700, 0x00 },
         { 0x0701, 0x10 },
@@ -819,12 +801,8 @@ static const struct imx355_reg mode_1280x720_regs[] = {
         { 0x0301, 0x05 },
         { 0x0303, 0x01 },
         { 0x0305, 0x02 },
-       { 0x0306, 0x00 },
-       { 0x0307, 0x78 },
         { 0x030b, 0x01 },
         { 0x030d, 0x02 },
-       { 0x030e, 0x00 },
-       { 0x030f, 0x4b },
         { 0x0310, 0x00 },
         { 0x0700, 0x00 },
         { 0x0701, 0x10 },
@@ -868,12 +846,8 @@ static const struct imx355_reg mode_820x616_regs[] = {
         { 0x0301, 0x05 },
         { 0x0303, 0x01 },
         { 0x0305, 0x02 },
-       { 0x0306, 0x00 },
-       { 0x0307, 0x78 },
         { 0x030b, 0x01 },
         { 0x030d, 0x02 },
-       { 0x030e, 0x00 },
-       { 0x030f, 0x4b },
         { 0x0310, 0x00 },
         { 0x0700, 0x02 },
         { 0x0701, 0x78 },
@@ -1422,6 +1396,20 @@ static int imx355_start_streaming(struct imx355 *imx355)
                 return ret;
         }

+       /* Set PLL registers for the external clock frequency */
+       ret = imx355_write_reg(imx355, IMX355_REG_EXTCLK_FREQ, 2,
+                              imx355->clk_params->extclk_freq);
+       if (ret)
+               return ret;
+       ret = imx355_write_reg(imx355, IMX355_REG_PLL_VT_MUL, 2,
+                              imx355->clk_params->pll_vt_mpy);
+       if (ret)
+               return ret;
+       ret = imx355_write_reg(imx355, IMX355_REG_PLL_OP_MUL, 2,
+                              imx355->clk_params->pll_op_mpy);
+       if (ret)
+               return ret;
+
         /* set digital gain control to all color mode */
         ret = imx355_write_reg(imx355, IMX355_REG_DPGA_USE_GLOBAL_GAIN, 1, 1);
         if (ret)
@@ -1749,7 +1737,13 @@ static int imx355_probe(struct i2c_client *client)
                                      "failed to get clock\n");

         freq = clk_get_rate(imx355->clk);
-       if (freq != IMX355_EXT_CLK)
+       for (unsigned int i = 0; i < ARRAY_SIZE(imx355_clk_params); i++) {
+               if (freq == imx355_clk_params[i].ext_clk) {
+                       imx355->clk_params = &imx355_clk_params[i];
+                       break;
+               }
+       }
+       if (!imx355->clk_params)
                 return dev_err_probe(imx355->dev, -EINVAL,
                                      "external clock %lu is not supported\n",
                                      freq);

---
base-commit: 66672af7a095d89f082c5327f3b15bc2f93d558e
change-id: 20260414-imx355-24mhz-b8ccfab3adfb

Best regards,
--
David Heidelberg <da...@ixit.cz>

--
David Heidelberg