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

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

Hi Shravan, Tarang,

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 exactly the same than your 2nd patch does. It'd be good to
understand why it is different and what is effect of that difference.

>         { IMX334_REG_INCKSEL1, 0x0129},
> -       { IMX334_REG_INCKSEL2, 0x06},
> +       { IMX334_REG_INCKSEL2, 0x0a},

This is a bigger change indeed.

>         { IMX334_REG_INCKSEL3, 0xa0},
>         { IMX334_REG_INCKSEL4, 0x7e},
>         { IMX334_REG_SYS_MODE, 0x02},

-- 
Regards,

Sakari Ailus