Re: [PATCH 2/6] media: i2c: imx334: Convert to CCI register access helpers

[Sakari Ailus <sakari.ailus@linux.intel.com>]

Hi Tarang,

On Tue, Mar 11, 2025 at 10:05:12AM +0000, Tarang Raval wrote:
> Hi Sakari,
> 
> > On Tue, Mar 11, 2025 at 06:51:48AM +0000, Shravan.Chippa@microchip.com wrote:
> > > Hi Laurent,
> > >
> > > > -----Original Message-----
> > > > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > > Sent: Tuesday, March 11, 2025 12:09 PM
> > > > To: shravan Chippa - I35088 <Shravan.Chippa@microchip.com>
> > > > Cc: tarang.raval@siliconsignals.io; sakari.ailus@linux.intel.com;
> > > > kieran.bingham@ideasonboard.com; mchehab@kernel.org;
> > > > hverkuil@xs4all.nl; umang.jain@ideasonboard.com; zhi.mao@mediatek.com;
> > > > julien.massot@collabora.com; mike.rudenko@gmail.com;
> > > > benjamin.mugnier@foss.st.com; linux-media@vger.kernel.org; linux-
> > > > kernel@vger.kernel.org
> > > > Subject: Re: [PATCH 2/6] media: i2c: imx334: Convert to CCI register access
> > > > helpers
> > > >
> > > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> > > > content is safe
> > > >
> > > > Hi Shravan,
> > > >
> > > > On Tue, Mar 11, 2025 at 06:14:28AM +0000, Shravan.Chippa@microchip.com
> > > > wrote:
> > > > > Hi Tarang,
> > > > >
> > > > > Thanks for the patch series with CCI register access helpers on top of
> > > > > my patches I have tested (1080p,720p, 480p resolution only) and
> > > > > working on my board with small PLL changes to make it compatible with
> > > > > pfsoc board (mpfs-video-kit).
> > > >
> > > > Could you please provide more information about what those PLL changes are
> > > > ?
> > >
> > > Here is the change for mpfs-video-kit board.
> > >
> > > diff --git a/drivers/media/i2c/imx334.c b/drivers/media/i2c/imx334.c
> > > index 375367314416..30470dbd1f3c 100644
> > > --- a/drivers/media/i2c/imx334.c
> > > +++ b/drivers/media/i2c/imx334.c
> > > @@ -236,9 +236,9 @@ static const struct cci_reg_sequence common_mode_regs[] = {
> > >         { IMX334_REG_XVS_XHS_OUTSEL, 0x20},
> > >         { IMX334_REG_XVS_XHS_DRV, 0x0f},
> > >         { IMX334_REG_BCWAIT_TIME, 0x3b},
> > > -       { IMX334_REG_CPWAIT_TIME, 0x2a},
> > > +       { IMX334_REG_CPWAIT_TIME, 0x29},
> > 
> > A patch converting the driver to use V4L2 CCI / human-readable register
> > names should not change the values written.
> 
> This change is not from my patch.
> 
> You can recheck Shravan’s second patch, there is no change in the 
> CPWAIT_TIME value from my patch or his.
> 
> The change Shravan mentioned was made locally to ensure compatibility
> with the mpfs-video-kit board, as Laurent requested. 
> That’s why he provided those details.

Right. I don't know what this or the other change do exactly, but
presumably the other one is a clock tree divider that reduces the link
frequency (or pixel rate in pixel array). That should be handled
separately.

-- 
Sakari Ailus