Hi Dave,
On Fri, Nov 15, 2024 at 08:51:55AM +0000, Dave Stevenson wrote:
> On Fri, 15 Nov 2024 at 00:06, Laurent Pinchart wrote:
> > On Thu, Nov 14, 2024 at 04:01:15PM +0000, Dave Stevenson wrote:
> > > IMX462 is the successor to IMX290, and wants very minor
> > > changes to the register setup.
> > >
> > > Add the relevant configuration to support it.
> > >
> > > Signed-off-by: Dave Stevenson <dave.stevenson@xxxxxxxxxxxxxxx>
> > > ---
> > > drivers/media/i2c/imx290.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++
> > > 1 file changed, 66 insertions(+)
> > >
> > > diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
> > > index da654deb444a..f1780cc5d7cc 100644
> > > --- a/drivers/media/i2c/imx290.c
> > > +++ b/drivers/media/i2c/imx290.c
> > > @@ -170,6 +170,8 @@ enum imx290_model {
> > > IMX290_MODEL_IMX290LQR,
> > > IMX290_MODEL_IMX290LLR,
> > > IMX290_MODEL_IMX327LQR,
> > > + IMX290_MODEL_IMX462LQR,
> > > + IMX290_MODEL_IMX462LLR,
> > > };
> > >
> > > struct imx290_model_info {
> > > @@ -316,6 +318,50 @@ static const struct cci_reg_sequence imx290_global_init_settings_290[] = {
> > > { CCI_REG8(0x33b3), 0x04 },
> > > };
> > >
> > > +static const struct cci_reg_sequence imx290_global_init_settings_462[] = {
> > > + { CCI_REG8(0x300f), 0x00 },
> > > + { CCI_REG8(0x3010), 0x21 },
> > > + { CCI_REG8(0x3011), 0x02 },
> >
> > As far as I can tell, the only difference in the init sequence between
> > imx290_global_init_settings_290 and imx290_global_init_settings_462 is
> > 0x3011 register which is not present in imx290_global_init_settings_290.
> > It is however included in imx290_global_init_settings, and set to 0x02.
> > Could we therefore use imx290_global_init_settings_290 for the imx462 ?
>
> I'd done a comparison of the datasheets, and register 0x3011 was the
> only one that changed. I'd missed that it was in
> imx290_global_init_settings.
>
> My datasheets:
> IMX327LQR-C rev E17Z06B93 2019/03/25. 3011h "Set to 02h" (value
> changed in doc rev 0.3 from 0Ah)
> IMX290LQR-C rev E15510G82 2018/02/09. 3011h "Fixed to 00h" (always
> been that value).
> IMX462LQR-C rev E19Y13C13 2021/03/19. 3011h "Set to 02h" (value
> changed in doc rev 0.2 from 00h)
> The default value stated in all of them is 00h. In true Sony fashion,
> there's no description for that register functionality.
>
> So actually it looks like it was the addition of IMX327 in [1] should
> have changed that setting, unless someone else has a more recent
> datasheet for IMX290 that updates that.
I agree with this analysis. It may be that setting the register to 0x02
would be fine, but it's hard to tell. Maybe it could be worth asking
Sony ?
> cc Alexander as the author of that patch. I'll find any discussion on it later.
>
> Dave
>
> [1] https://github.com/torvalds/linux/commit/2d41947ec2c0140c65783982692c2e3d89853c47
>
> > > + { CCI_REG8(0x3016), 0x09 },
> > > + { CCI_REG8(0x3070), 0x02 },
> > > + { CCI_REG8(0x3071), 0x11 },
> > > + { CCI_REG8(0x309b), 0x10 },
> > > + { CCI_REG8(0x309c), 0x22 },
> > > + { CCI_REG8(0x30a2), 0x02 },
> > > + { CCI_REG8(0x30a6), 0x20 },
> > > + { CCI_REG8(0x30a8), 0x20 },
> > > + { CCI_REG8(0x30aa), 0x20 },
> > > + { CCI_REG8(0x30ac), 0x20 },
> > > + { CCI_REG8(0x30b0), 0x43 },
> > > + { CCI_REG8(0x3119), 0x9e },
> > > + { CCI_REG8(0x311c), 0x1e },
> > > + { CCI_REG8(0x311e), 0x08 },
> > > + { CCI_REG8(0x3128), 0x05 },
> > > + { CCI_REG8(0x313d), 0x83 },
> > > + { CCI_REG8(0x3150), 0x03 },
> > > + { CCI_REG8(0x317e), 0x00 },
> > > + { CCI_REG8(0x32b8), 0x50 },
> > > + { CCI_REG8(0x32b9), 0x10 },
> > > + { CCI_REG8(0x32ba), 0x00 },
> > > + { CCI_REG8(0x32bb), 0x04 },
> > > + { CCI_REG8(0x32c8), 0x50 },
> > > + { CCI_REG8(0x32c9), 0x10 },
> > > + { CCI_REG8(0x32ca), 0x00 },
> > > + { CCI_REG8(0x32cb), 0x04 },
> > > + { CCI_REG8(0x332c), 0xd3 },
> > > + { CCI_REG8(0x332d), 0x10 },
> > > + { CCI_REG8(0x332e), 0x0d },
> > > + { CCI_REG8(0x3358), 0x06 },
> > > + { CCI_REG8(0x3359), 0xe1 },
> > > + { CCI_REG8(0x335a), 0x11 },
> > > + { CCI_REG8(0x3360), 0x1e },
> > > + { CCI_REG8(0x3361), 0x61 },
> > > + { CCI_REG8(0x3362), 0x10 },
> > > + { CCI_REG8(0x33b0), 0x50 },
> > > + { CCI_REG8(0x33b2), 0x1a },
> > > + { CCI_REG8(0x33b3), 0x04 },
> > > +};
> > > +
> > > #define IMX290_NUM_CLK_REGS 2
> > > static const struct cci_reg_sequence xclk_regs[][IMX290_NUM_CLK_REGS] = {
> > > [IMX290_CLK_37_125] = {
> > > @@ -1455,6 +1501,20 @@ static const struct imx290_model_info imx290_models[] = {
> > > .max_analog_gain = 98,
> > > .name = "imx327",
> > > },
> > > + [IMX290_MODEL_IMX462LQR] = {
> > > + .colour_variant = IMX290_VARIANT_COLOUR,
> > > + .init_regs = imx290_global_init_settings_462,
> > > + .init_regs_num = ARRAY_SIZE(imx290_global_init_settings_462),
> > > + .max_analog_gain = 98,
> > > + .name = "imx462",
> > > + },
> > > + [IMX290_MODEL_IMX462LLR] = {
> > > + .colour_variant = IMX290_VARIANT_MONO,
> > > + .init_regs = imx290_global_init_settings_462,
> > > + .init_regs_num = ARRAY_SIZE(imx290_global_init_settings_462),
> > > + .max_analog_gain = 98,
> > > + .name = "imx462",
> > > + },
> > > };
> > >
> > > static int imx290_parse_dt(struct imx290 *imx290)
> > > @@ -1653,6 +1713,12 @@ static const struct of_device_id imx290_of_match[] = {
> > > }, {
> > > .compatible = "sony,imx327lqr",
> > > .data = &imx290_models[IMX290_MODEL_IMX327LQR],
> > > + }, {
> > > + .compatible = "sony,imx462lqr",
> > > + .data = &imx290_models[IMX290_MODEL_IMX462LQR],
> > > + }, {
> > > + .compatible = "sony,imx462llr",
> > > + .data = &imx290_models[IMX290_MODEL_IMX462LLR],
> > > },
> > > { /* sentinel */ },
> > > };
--
Regards,
Laurent Pinchart
