Re: [PATCH 16/19] media: i2c: imx290: Move registers with fixed value to init array

Laurent Pinchart
2022-07-21T10:40:44Z

Hi Alexander,

On Thu, Jul 21, 2022 at 12:08:50PM +0200, Alexander Stein wrote:
> Am Donnerstag, 21. Juli 2022, 10:35:37 CEST schrieb Laurent Pinchart:
> > Registers 0x3012, 0x3013 and 0x3480 are not documented and are set in
> > the per-mode register arrays with values indentical for all modes. Move
> > them to the common array.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> > drivers/media/i2c/imx290.c | 8 ++------
> > 1 file changed, 2 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
> > index 78772c6327a2..fc6e87fada1c 100644
> > --- a/drivers/media/i2c/imx290.c
> > +++ b/drivers/media/i2c/imx290.c
> > @@ -192,6 +192,7 @@ static const struct imx290_regval
> > imx290_global_init_settings[] = { { IMX290_REG_8BIT(0x300f), 0x00 },
> > { IMX290_REG_8BIT(0x3010), 0x21 },
> > { IMX290_REG_8BIT(0x3012), 0x64 },
> > + { IMX290_REG_8BIT(0x3013), 0x00 },
> > { IMX290_REG_8BIT(0x3016), 0x09 },
> > { IMX290_REG_8BIT(0x3070), 0x02 },
> > { IMX290_REG_8BIT(0x3071), 0x11 },
> > @@ -230,6 +231,7 @@ static const struct imx290_regval
> > imx290_global_init_settings[] = { { IMX290_REG_8BIT(0x33b0), 0x50 },
> > { IMX290_REG_8BIT(0x33b2), 0x1a },
> > { IMX290_REG_8BIT(0x33b3), 0x04 },
> > + { IMX290_REG_8BIT(0x3480), 0x49 },
> > };
> >
> > static const struct imx290_regval imx290_1080p_settings[] = {
> > @@ -239,15 +241,12 @@ static const struct imx290_regval
> > imx290_1080p_settings[] = { { IMX290_OPB_SIZE_V, 10 },
> > { IMX290_X_OUT_SIZE, 1920 },
> > { IMX290_Y_OUT_SIZE, 1080 },
> > - { IMX290_REG_8BIT(0x3012), 0x64 },
> > - { IMX290_REG_8BIT(0x3013), 0x00 },
> > { IMX290_INCKSEL1, 0x18 },
> > { IMX290_INCKSEL2, 0x03 },
> > { IMX290_INCKSEL3, 0x20 },
> > { IMX290_INCKSEL4, 0x01 },
> > { IMX290_INCKSEL5, 0x1a },
> > { IMX290_INCKSEL6, 0x1a },
> > - { IMX290_REG_8BIT(0x3480), 0x49 },
> > /* data rate settings */
> > { IMX290_REPETITION, 0x10 },
> > { IMX290_TCLKPOST, 87 },
> > @@ -267,15 +266,12 @@ static const struct imx290_regval
> > imx290_720p_settings[] = { { IMX290_OPB_SIZE_V, 4 },
> > { IMX290_X_OUT_SIZE, 1280 },
> > { IMX290_Y_OUT_SIZE, 720 },
> > - { IMX290_REG_8BIT(0x3012), 0x64 },
> > - { IMX290_REG_8BIT(0x3013), 0x00 },
> > { IMX290_INCKSEL1, 0x20 },
> > { IMX290_INCKSEL2, 0x00 },
> > { IMX290_INCKSEL3, 0x20 },
> > { IMX290_INCKSEL4, 0x01 },
> > { IMX290_INCKSEL5, 0x1a },
> > { IMX290_INCKSEL6, 0x1a },
> > - { IMX290_REG_8BIT(0x3480), 0x49 },
> > /* data rate settings */
> > { IMX290_REPETITION, 0x10 },
> > { IMX290_TCLKPOST, 79 },
>
> 0x3480 is INCKSEL7 for imx327, not sure if that should be set yet for imx290
> (only) driver, without proper imx327 support.

Do you mean the register doesn't exist on the IMX290 ? We could make it
conditional on the sensor model, but it's not added by this patch, it
has been there since the first version of the driver, so I'd rather do
that on top.

--
Regards,

Laurent Pinchart