Re: [PATCH v4 02/21] media: i2c: imx355: Add support for 24 MHz external clock

[Sakari Ailus]

Hi Jacopo,

On Thu, Jul 16, 2026 at 03:25:25PM +0200, Jacopo Mondi wrote:
> Hi Sakari
> 
> On Wed, Jul 15, 2026 at 11:19:10PM +0300, Sakari Ailus wrote:
> > Hi Richard,
> >
> > On Wed, Jul 15, 2026 at 03:47:41PM -0400, Richard Acayan wrote:
> > > On Wed, Jul 15, 2026 at 12:43:15PM +0100, Dave Stevenson wrote:
> > > > From: David Heidelberg <david@xxxxxxx>
> > > >
> > > > 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 OP VCO remains at 720 MHz
> > > >
> > > > This preserves existing timing characteristics while allowing systems
> > > > using a 24 MHz clock to operate correctly.
> > > >
> > > > No functional change for existing 19.2 MHz users.
> > > >
> > > > Assisted-by: Claude:claude-opus-4-6
> > > > Signed-off-by: David Heidelberg <david@xxxxxxx>
> > > > Signed-off-by: Dave Stevenson <dave.stevenson@xxxxxxxxxxxxxxx>
> > > > ---
> > > >  drivers/media/i2c/imx355.c | 77 +++++++++++++++++++++++++++-------------------
> > > >  1 file changed, 45 insertions(+), 32 deletions(-)
> > > >
> > > > diff --git a/drivers/media/i2c/imx355.c b/drivers/media/i2c/imx355.c
> > > > index ac59908f57aa..a6ee7532018f 100644
> > > > --- a/drivers/media/i2c/imx355.c
> > > > +++ b/drivers/media/i2c/imx355.c
> > > (snip)
> > > > @@ -1749,7 +1756,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++) {
> > >
> > > Did we start allowing variable declarations mixed with code?
> >
> > C didn't allow that until somewhat recently, hence it hasn't been used in
> > the kernel either.
> 
> Recently as in C99 ? :)
> https://en.cppreference.com/c/language/scope "Block scope"

Well, it's still "recent" for C, isn't it? ;-)

> 
> See: e8c07082a810 ("Kbuild: move to -std=gnu11")
> 
> >
> > In cases like the above it improves readability. E.g. with __free() that is
> > also the recommended pattern.
> >
> > --
> > Regards,
> >
> > Sakari Ailus

-- 
Regards,

Sakari Ailus