On Thu, 18 Jun 2026 at 13:11, David Heidelberg <[email protected]> wrote: > > On 17/06/2026 19:40, Dave Stevenson wrote: > > Hi David > > > > On Thu, 16 Apr 2026 at 12:26, Dave Stevenson > > <[email protected]> wrote: > >> > >> Hi David > >> > >> On Tue, 14 Apr 2026 at 11:17, David Heidelberg via B4 Relay > >> <[email protected]> wrote: > >>> > >>> From: David Heidelberg <[email protected]> > >>> > >>> 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?
Not mentioned in the information I've got, so it must have been deleted as a feature very early in development. C'est la vie. > > 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). I'm happy to merge it into my series as that avoids having to declare dependencies. I had hoped to get my v2 out this week, but it just hasn't happened. It won't be next week, but hopefully within the fortnight. Dave > 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 <[email protected]> > >> > >> Whilst useful comments could be added, it does what it says and works: > >> > >> Reviewed-by: Dave Stevenson <[email protected]> > >> Tested-by: Dave Stevenson <[email protected]> > >> > >>> --- > >>> 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 <[email protected]> > >>> > >>> > >>> > > -- > David Heidelberg >
