| Message ID | 20230203102439.237527-3-alexander.stein@ew.tq-group.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: i2c: imx290: imx327 support | expand |
Hi Alexander On Fri, 3 Feb 2023 at 10:24, Alexander Stein <alexander.stein@ew.tq-group.com> wrote: > > Both sensors are quite similar. Their specs only differ regarding LVDS > and parallel output but are identical regarding MIPI-CSI-2 interface. > But they use a different init setting of hard-coded values, taken from > the datasheet. > > Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com> > --- > drivers/media/i2c/imx290.c | 88 +++++++++++++++++++++++++++++++++----- > 1 file changed, 77 insertions(+), 11 deletions(-) > > diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c > index e642e1df520d..337252b2ec15 100644 > --- a/drivers/media/i2c/imx290.c > +++ b/drivers/media/i2c/imx290.c > @@ -164,6 +164,36 @@ > #define CLK_74_25 1 > #define NUM_CLK 2 > > +enum imx290_model { > + IMX290, > + IMX290_MONO, > + IMX327, > +}; > + > +struct imx290_device_data { > + enum imx290_model model; > + const char *name; > + u8 mono; > +}; > + > +static const struct imx290_device_data imx290_models[] = { > + [IMX290] = { > + .model = IMX290, > + .name = "imx290", > + .mono = 0, > + }, > + [IMX290_MONO] = { > + .model = IMX290_MONO, > + .name = "imx290-mono", > + .mono = 1, > + }, > + [IMX327] = { > + .model = IMX327, > + .name = "imx327", > + .mono = 0, > + }, > +}; > + > struct imx290_regval { > u32 reg; > u32 val; > @@ -210,9 +240,9 @@ struct imx290 { > struct device *dev; > struct clk *xclk; > struct regmap *regmap; > + const struct imx290_device_data *devdata; > u32 xclk_freq; > u8 nlanes; > - u8 mono; > > struct v4l2_subdev sd; > struct media_pad pad; > @@ -240,7 +270,7 @@ static inline struct imx290 *to_imx290(struct v4l2_subdev *_sd) > * Modes and formats > */ > > -static const struct imx290_regval imx290_global_init_settings[] = { > +static const struct imx290_regval imx290_global_init_settings_290[] = { > { IMX290_WINWV_OB, 12 }, > { IMX290_WINPH, 0 }, > { IMX290_WINPV, 0 }, > @@ -292,6 +322,23 @@ static const struct imx290_regval imx290_global_init_settings[] = { > { IMX290_REG_8BIT(0x33b3), 0x04 }, > }; > > +static const struct imx290_regval imx290_global_init_settings_327[] = { > + { IMX290_WINWV_OB, 12 }, > + { IMX290_WINPH, 0 }, > + { IMX290_WINPV, 0 }, > + { IMX290_WINWH, 1948 }, > + { IMX290_WINWV, 1097 }, > + { IMX290_XSOUTSEL, IMX290_XSOUTSEL_XVSOUTSEL_VSYNC | > + IMX290_XSOUTSEL_XHSOUTSEL_HSYNC }, > + { IMX290_REG_8BIT(0x3011), 0x0A }, What datasheet are you working from? Mine (2019/03/25) has a correction listed at v0.3 of: Register 3011h setting 0Ah -> 02h > + { IMX290_REG_8BIT(0x3012), 0x64 }, > + { IMX290_REG_8BIT(0x3013), 0x00 }, > + { IMX290_REG_8BIT(0x309e), 0x4A }, > + { IMX290_REG_8BIT(0x309f), 0x4A }, 309e/f undocumented in my datasheet beyond "default value 5Ah, set to "4Ah"". Not documented in imx290 or imx462 datasheets either. I'll read it back from IMX290 and IMX462 when I get to the office and see if 0x4a is the default anyway, in which case it can be generic. > + { IMX290_REG_8BIT(0x3128), 0x04 }, Correction v0.3 - register address 3128h deleted. > + { IMX290_REG_8BIT(0x313b), 0x41 }, Correction v0.3 - Register address 313Bh setting 41h -> 61h. I'll check the defaults on imx290 and imx462, because there is no harm in adding those register writes if they happen to be the defaults. There is also a fair amount of duplication between imx290_global_init_settings_290 and imx290_global_init_settings_327 - it'd be nice to reduce it down to the minimum set of diffs. > +}; > + > static const struct imx290_regval imx290_37_125mhz_clock[] = { > { IMX290_EXTCK_FREQ, 0x2520 }, > { IMX290_INCKSEL7, 0x49 }, > @@ -558,7 +605,7 @@ imx290_format_info(const struct imx290 *imx290, u32 code) > for (i = 0; i < ARRAY_SIZE(imx290_formats); ++i) { > const struct imx290_format_info *info = &imx290_formats[i]; > > - if (info->code[imx290->mono] == code) > + if (info->code[imx290->devdata->mono] == code) > return info; > } > > @@ -957,11 +1004,27 @@ static int imx290_start_streaming(struct imx290 *imx290, > struct v4l2_subdev_state *state) > { > const struct v4l2_mbus_framefmt *format; > + const struct imx290_regval *regs; > + unsigned int reg_num; > int ret; > > + switch (imx290->devdata->model) { > + case IMX290: > + case IMX290_MONO: > + regs = imx290_global_init_settings_290; > + reg_num = ARRAY_SIZE(imx290_global_init_settings_290); > + break; > + case IMX327: > + regs = imx290_global_init_settings_327; > + reg_num = ARRAY_SIZE(imx290_global_init_settings_327); > + break; > + default: > + dev_err(imx290->dev, "Invalid model: %u\n", imx290->devdata->model); > + return -EINVAL; > + } > + > /* Set init register settings */ > - ret = imx290_set_register_array(imx290, imx290_global_init_settings, > - ARRAY_SIZE(imx290_global_init_settings)); > + ret = imx290_set_register_array(imx290, regs, reg_num); > if (ret < 0) { > dev_err(imx290->dev, "Could not set init registers\n"); > return ret; > @@ -1072,7 +1135,7 @@ static int imx290_enum_mbus_code(struct v4l2_subdev *sd, > if (code->index >= ARRAY_SIZE(imx290_formats)) > return -EINVAL; > > - code->code = imx290_formats[code->index].code[imx290->mono]; > + code->code = imx290_formats[code->index].code[imx290->devdata->mono]; > > return 0; > } > @@ -1114,7 +1177,7 @@ static int imx290_set_fmt(struct v4l2_subdev *sd, > fmt->format.height = mode->height; > > if (!imx290_format_info(imx290, fmt->format.code)) > - fmt->format.code = imx290_formats[0].code[imx290->mono]; > + fmt->format.code = imx290_formats[0].code[imx290->devdata->mono]; > > fmt->format.field = V4L2_FIELD_NONE; > fmt->format.colorspace = V4L2_COLORSPACE_RAW; > @@ -1422,8 +1485,9 @@ static s64 imx290_check_link_freqs(const struct imx290 *imx290, > } > > static const struct of_device_id imx290_of_match[] = { > - { .compatible = "sony,imx290" }, > - { .compatible = "sony,imx290-mono", .data = (void *)1 }, > + { .compatible = "sony,imx290", .data = &imx290_models[IMX290] }, > + { .compatible = "sony,imx290-mono", .data = &imx290_models[IMX290_MONO] }, > + { .compatible = "sony,imx327", .data = &imx290_models[IMX327] }, Based on Laurent's requests my parent to this set will be switching to imx290 (as legacy), imx290lqr and imx290llr as the compatible strings. imx327 ought to follow the same pattern. Dave > { /* sentinel */ } > }; > MODULE_DEVICE_TABLE(of, imx290_of_match); > @@ -1441,8 +1505,7 @@ static int imx290_parse_dt(struct imx290 *imx290) > s64 fq; > > match = i2c_of_match_device(imx290_of_match, client); > - if (match) > - imx290->mono = match->data ? 1 : 0; > + imx290->devdata = match->data; > > endpoint = fwnode_graph_get_next_endpoint(dev_fwnode(imx290->dev), NULL); > if (!endpoint) { > @@ -1561,6 +1624,9 @@ static int imx290_probe(struct i2c_client *client) > if (ret) > goto err_pm; > > + v4l2_i2c_subdev_set_name(&imx290->sd, client, > + imx290->devdata->name, NULL); > + > /* > * Finally, register the V4L2 subdev. This must be done after > * initializing everything as the subdev can be used immediately after > -- > 2.34.1 >
Hi Dave, Am Freitag, 3. Februar 2023, 11:55:55 CET schrieb Dave Stevenson: > Hi Alexander > > On Fri, 3 Feb 2023 at 10:24, Alexander Stein > > <alexander.stein@ew.tq-group.com> wrote: > > Both sensors are quite similar. Their specs only differ regarding LVDS > > and parallel output but are identical regarding MIPI-CSI-2 interface. > > But they use a different init setting of hard-coded values, taken from > > the datasheet. > > > > Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com> > > --- > > > > drivers/media/i2c/imx290.c | 88 +++++++++++++++++++++++++++++++++----- > > 1 file changed, 77 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c > > index e642e1df520d..337252b2ec15 100644 > > --- a/drivers/media/i2c/imx290.c > > +++ b/drivers/media/i2c/imx290.c > > @@ -164,6 +164,36 @@ > > > > #define CLK_74_25 1 > > #define NUM_CLK 2 > > > > +enum imx290_model { > > + IMX290, > > + IMX290_MONO, > > + IMX327, > > +}; > > + > > +struct imx290_device_data { > > + enum imx290_model model; > > + const char *name; > > + u8 mono; > > +}; > > + > > +static const struct imx290_device_data imx290_models[] = { > > + [IMX290] = { > > + .model = IMX290, > > + .name = "imx290", > > + .mono = 0, > > + }, > > + [IMX290_MONO] = { > > + .model = IMX290_MONO, > > + .name = "imx290-mono", > > + .mono = 1, > > + }, > > + [IMX327] = { > > + .model = IMX327, > > + .name = "imx327", > > + .mono = 0, > > + }, > > +}; > > + > > > > struct imx290_regval { > > > > u32 reg; > > u32 val; > > > > @@ -210,9 +240,9 @@ struct imx290 { > > > > struct device *dev; > > struct clk *xclk; > > struct regmap *regmap; > > > > + const struct imx290_device_data *devdata; > > > > u32 xclk_freq; > > u8 nlanes; > > > > - u8 mono; > > > > struct v4l2_subdev sd; > > struct media_pad pad; > > > > @@ -240,7 +270,7 @@ static inline struct imx290 *to_imx290(struct > > v4l2_subdev *_sd)> > > * Modes and formats > > */ > > > > -static const struct imx290_regval imx290_global_init_settings[] = { > > +static const struct imx290_regval imx290_global_init_settings_290[] = { > > > > { IMX290_WINWV_OB, 12 }, > > { IMX290_WINPH, 0 }, > > { IMX290_WINPV, 0 }, > > > > @@ -292,6 +322,23 @@ static const struct imx290_regval > > imx290_global_init_settings[] = {> > > { IMX290_REG_8BIT(0x33b3), 0x04 }, > > > > }; > > > > +static const struct imx290_regval imx290_global_init_settings_327[] = { > > + { IMX290_WINWV_OB, 12 }, > > + { IMX290_WINPH, 0 }, > > + { IMX290_WINPV, 0 }, > > + { IMX290_WINWH, 1948 }, > > + { IMX290_WINWV, 1097 }, > > + { IMX290_XSOUTSEL, IMX290_XSOUTSEL_XVSOUTSEL_VSYNC | > > + IMX290_XSOUTSEL_XHSOUTSEL_HSYNC }, > > + { IMX290_REG_8BIT(0x3011), 0x0A }, > > What datasheet are you working from? Mine (2019/03/25) has a > correction listed at v0.3 of: > Register 3011h setting 0Ah -> 02h I've got a v0.2 (2017/05/25). As you have v0.3 I am not really surprised you have different/additional values. > > + { IMX290_REG_8BIT(0x3012), 0x64 }, > > + { IMX290_REG_8BIT(0x3013), 0x00 }, > > + { IMX290_REG_8BIT(0x309e), 0x4A }, > > + { IMX290_REG_8BIT(0x309f), 0x4A }, > > 309e/f undocumented in my datasheet beyond "default value 5Ah, set to > "4Ah"". Not documented in imx290 or imx462 datasheets either. I'll read it > back from IMX290 and IMX462 when I get to the office and see if 0x4a is the > default anyway, in which case it can be generic. Exactly, they are not documented at all, just marked red indicating it's different from reset default. > > + { IMX290_REG_8BIT(0x3128), 0x04 }, > > Correction v0.3 - register address 3128h deleted. > > > + { IMX290_REG_8BIT(0x313b), 0x41 }, > > Correction v0.3 - Register address 313Bh setting 41h -> 61h. > > > I'll check the defaults on imx290 and imx462, because there is no harm > in adding those register writes if they happen to be the defaults. > There is also a fair amount of duplication between > imx290_global_init_settings_290 and imx290_global_init_settings_327 - > it'd be nice to reduce it down to the minimum set of diffs. Thanks, I'll update to the values you provided and split the common settings. > > +}; > > + > > > > static const struct imx290_regval imx290_37_125mhz_clock[] = { > > > > { IMX290_EXTCK_FREQ, 0x2520 }, > > { IMX290_INCKSEL7, 0x49 }, > > > > @@ -558,7 +605,7 @@ imx290_format_info(const struct imx290 *imx290, u32 > > code)> > > for (i = 0; i < ARRAY_SIZE(imx290_formats); ++i) { > > > > const struct imx290_format_info *info = > > &imx290_formats[i]; > > > > - if (info->code[imx290->mono] == code) > > + if (info->code[imx290->devdata->mono] == code) > > > > return info; > > > > } > > > > @@ -957,11 +1004,27 @@ static int imx290_start_streaming(struct imx290 > > *imx290,> > > struct v4l2_subdev_state *state) > > > > { > > > > const struct v4l2_mbus_framefmt *format; > > > > + const struct imx290_regval *regs; > > + unsigned int reg_num; > > > > int ret; > > > > + switch (imx290->devdata->model) { > > + case IMX290: > > + case IMX290_MONO: > > + regs = imx290_global_init_settings_290; > > + reg_num = ARRAY_SIZE(imx290_global_init_settings_290); > > + break; > > + case IMX327: > > + regs = imx290_global_init_settings_327; > > + reg_num = ARRAY_SIZE(imx290_global_init_settings_327); > > + break; > > + default: > > + dev_err(imx290->dev, "Invalid model: %u\n", > > imx290->devdata->model); + return -EINVAL; > > + } > > + > > > > /* Set init register settings */ > > > > - ret = imx290_set_register_array(imx290, > > imx290_global_init_settings, - > > ARRAY_SIZE(imx290_global_init_settings)); + ret = > > imx290_set_register_array(imx290, regs, reg_num); > > > > if (ret < 0) { > > > > dev_err(imx290->dev, "Could not set init registers\n"); > > return ret; > > > > @@ -1072,7 +1135,7 @@ static int imx290_enum_mbus_code(struct v4l2_subdev > > *sd,> > > if (code->index >= ARRAY_SIZE(imx290_formats)) > > > > return -EINVAL; > > > > - code->code = imx290_formats[code->index].code[imx290->mono]; > > + code->code = > > imx290_formats[code->index].code[imx290->devdata->mono];> > > return 0; > > > > } > > > > @@ -1114,7 +1177,7 @@ static int imx290_set_fmt(struct v4l2_subdev *sd, > > > > fmt->format.height = mode->height; > > > > if (!imx290_format_info(imx290, fmt->format.code)) > > > > - fmt->format.code = imx290_formats[0].code[imx290->mono]; > > + fmt->format.code = > > imx290_formats[0].code[imx290->devdata->mono];> > > fmt->format.field = V4L2_FIELD_NONE; > > fmt->format.colorspace = V4L2_COLORSPACE_RAW; > > > > @@ -1422,8 +1485,9 @@ static s64 imx290_check_link_freqs(const struct > > imx290 *imx290,> > > } > > > > static const struct of_device_id imx290_of_match[] = { > > > > - { .compatible = "sony,imx290" }, > > - { .compatible = "sony,imx290-mono", .data = (void *)1 }, > > + { .compatible = "sony,imx290", .data = &imx290_models[IMX290] }, > > + { .compatible = "sony,imx290-mono", .data = > > &imx290_models[IMX290_MONO] }, + { .compatible = "sony,imx327", > > .data = &imx290_models[IMX327] }, > Based on Laurent's requests my parent to this set will be switching to > imx290 (as legacy), imx290lqr and imx290llr as the compatible strings. > imx327 ought to follow the same pattern. Okay thanks. I'll update to imx327lqr as well. Put me on CC so I'll notice the update & conflict. Best regards, Alexander > Dave > > > { /* sentinel */ } > > > > }; > > MODULE_DEVICE_TABLE(of, imx290_of_match); > > > > @@ -1441,8 +1505,7 @@ static int imx290_parse_dt(struct imx290 *imx290) > > > > s64 fq; > > > > match = i2c_of_match_device(imx290_of_match, client); > > > > - if (match) > > - imx290->mono = match->data ? 1 : 0; > > + imx290->devdata = match->data; > > > > endpoint = fwnode_graph_get_next_endpoint(dev_fwnode(imx290->dev), > > NULL); > > if (!endpoint) { > > > > @@ -1561,6 +1624,9 @@ static int imx290_probe(struct i2c_client *client) > > > > if (ret) > > > > goto err_pm; > > > > + v4l2_i2c_subdev_set_name(&imx290->sd, client, > > + imx290->devdata->name, NULL); > > + > > > > /* > > > > * Finally, register the V4L2 subdev. This must be done after > > * initializing everything as the subdev can be used immediately > > after > > > > -- > > 2.34.1
On Fri, 3 Feb 2023 at 11:10, Alexander Stein <alexander.stein@ew.tq-group.com> wrote: > > Hi Dave, > > Am Freitag, 3. Februar 2023, 11:55:55 CET schrieb Dave Stevenson: > > Hi Alexander > > > > On Fri, 3 Feb 2023 at 10:24, Alexander Stein > > > > <alexander.stein@ew.tq-group.com> wrote: > > > Both sensors are quite similar. Their specs only differ regarding LVDS > > > and parallel output but are identical regarding MIPI-CSI-2 interface. > > > But they use a different init setting of hard-coded values, taken from > > > the datasheet. > > > > > > Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com> > > > --- > > > > > > drivers/media/i2c/imx290.c | 88 +++++++++++++++++++++++++++++++++----- > > > 1 file changed, 77 insertions(+), 11 deletions(-) > > > > > > diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c > > > index e642e1df520d..337252b2ec15 100644 > > > --- a/drivers/media/i2c/imx290.c > > > +++ b/drivers/media/i2c/imx290.c > > > @@ -164,6 +164,36 @@ > > > > > > #define CLK_74_25 1 > > > #define NUM_CLK 2 > > > > > > +enum imx290_model { > > > + IMX290, > > > + IMX290_MONO, > > > + IMX327, > > > +}; > > > + > > > +struct imx290_device_data { > > > + enum imx290_model model; > > > + const char *name; > > > + u8 mono; > > > +}; > > > + > > > +static const struct imx290_device_data imx290_models[] = { > > > + [IMX290] = { > > > + .model = IMX290, > > > + .name = "imx290", > > > + .mono = 0, > > > + }, > > > + [IMX290_MONO] = { > > > + .model = IMX290_MONO, > > > + .name = "imx290-mono", > > > + .mono = 1, > > > + }, > > > + [IMX327] = { > > > + .model = IMX327, > > > + .name = "imx327", > > > + .mono = 0, > > > + }, > > > +}; > > > + > > > > > > struct imx290_regval { > > > > > > u32 reg; > > > u32 val; > > > > > > @@ -210,9 +240,9 @@ struct imx290 { > > > > > > struct device *dev; > > > struct clk *xclk; > > > struct regmap *regmap; > > > > > > + const struct imx290_device_data *devdata; > > > > > > u32 xclk_freq; > > > u8 nlanes; > > > > > > - u8 mono; > > > > > > struct v4l2_subdev sd; > > > struct media_pad pad; > > > > > > @@ -240,7 +270,7 @@ static inline struct imx290 *to_imx290(struct > > > v4l2_subdev *_sd)> > > > * Modes and formats > > > */ > > > > > > -static const struct imx290_regval imx290_global_init_settings[] = { > > > +static const struct imx290_regval imx290_global_init_settings_290[] = { > > > > > > { IMX290_WINWV_OB, 12 }, > > > { IMX290_WINPH, 0 }, > > > { IMX290_WINPV, 0 }, > > > > > > @@ -292,6 +322,23 @@ static const struct imx290_regval > > > imx290_global_init_settings[] = {> > > > { IMX290_REG_8BIT(0x33b3), 0x04 }, > > > > > > }; > > > > > > +static const struct imx290_regval imx290_global_init_settings_327[] = { > > > + { IMX290_WINWV_OB, 12 }, > > > + { IMX290_WINPH, 0 }, > > > + { IMX290_WINPV, 0 }, > > > + { IMX290_WINWH, 1948 }, > > > + { IMX290_WINWV, 1097 }, > > > + { IMX290_XSOUTSEL, IMX290_XSOUTSEL_XVSOUTSEL_VSYNC | > > > + IMX290_XSOUTSEL_XHSOUTSEL_HSYNC }, > > > + { IMX290_REG_8BIT(0x3011), 0x0A }, > > > > What datasheet are you working from? Mine (2019/03/25) has a > > correction listed at v0.3 of: > > Register 3011h setting 0Ah -> 02h > > I've got a v0.2 (2017/05/25). As you have v0.3 I am not really surprised you > have different/additional values. Revision history appears to be 0.1 2017/02/23 0.2 2017/05/25 0.3 2017/09/04 E17Z06 2017/12/18 E17Z06A81 2018/02/01 E1706B93 2019/03/25 so you are a way behind. > > > > + { IMX290_REG_8BIT(0x3012), 0x64 }, > > > + { IMX290_REG_8BIT(0x3013), 0x00 }, > > > + { IMX290_REG_8BIT(0x309e), 0x4A }, > > > + { IMX290_REG_8BIT(0x309f), 0x4A }, > > > > 309e/f undocumented in my datasheet beyond "default value 5Ah, set to > > "4Ah"". Not documented in imx290 or imx462 datasheets either. I'll read it > > back from IMX290 and IMX462 when I get to the office and see if 0x4a is the > > default anyway, in which case it can be generic. I can't read back from my IMX290LLR as Vision Components use a secondary MCU to stop you reading registers. IMX462LQR default 0x5a 0x5a :-( > Exactly, they are not documented at all, just marked red indicating it's > different from reset default. > > > > + { IMX290_REG_8BIT(0x3128), 0x04 }, > > > > Correction v0.3 - register address 3128h deleted. > > > > > + { IMX290_REG_8BIT(0x313b), 0x41 }, > > > > Correction v0.3 - Register address 313Bh setting 41h -> 61h. IMX462LQR default 0x51 > > > > > > I'll check the defaults on imx290 and imx462, because there is no harm > > in adding those register writes if they happen to be the defaults. > > There is also a fair amount of duplication between > > imx290_global_init_settings_290 and imx290_global_init_settings_327 - > > it'd be nice to reduce it down to the minimum set of diffs. > > Thanks, I'll update to the values you provided and split the common settings. So annoyingly that does mean we need a sensor specific table, but I think it is only 3 registers (0x309e, 0x309ef, and 0x313b). None of those need to be set for IMX290 or 462. Dave > > > +}; > > > + > > > > > > static const struct imx290_regval imx290_37_125mhz_clock[] = { > > > > > > { IMX290_EXTCK_FREQ, 0x2520 }, > > > { IMX290_INCKSEL7, 0x49 }, > > > > > > @@ -558,7 +605,7 @@ imx290_format_info(const struct imx290 *imx290, u32 > > > code)> > > > for (i = 0; i < ARRAY_SIZE(imx290_formats); ++i) { > > > > > > const struct imx290_format_info *info = > > > &imx290_formats[i]; > > > > > > - if (info->code[imx290->mono] == code) > > > + if (info->code[imx290->devdata->mono] == code) > > > > > > return info; > > > > > > } > > > > > > @@ -957,11 +1004,27 @@ static int imx290_start_streaming(struct imx290 > > > *imx290,> > > > struct v4l2_subdev_state *state) > > > > > > { > > > > > > const struct v4l2_mbus_framefmt *format; > > > > > > + const struct imx290_regval *regs; > > > + unsigned int reg_num; > > > > > > int ret; > > > > > > + switch (imx290->devdata->model) { > > > + case IMX290: > > > + case IMX290_MONO: > > > + regs = imx290_global_init_settings_290; > > > + reg_num = ARRAY_SIZE(imx290_global_init_settings_290); > > > + break; > > > + case IMX327: > > > + regs = imx290_global_init_settings_327; > > > + reg_num = ARRAY_SIZE(imx290_global_init_settings_327); > > > + break; > > > + default: > > > + dev_err(imx290->dev, "Invalid model: %u\n", > > > imx290->devdata->model); + return -EINVAL; > > > + } > > > + > > > > > > /* Set init register settings */ > > > > > > - ret = imx290_set_register_array(imx290, > > > imx290_global_init_settings, - > > > ARRAY_SIZE(imx290_global_init_settings)); + ret = > > > imx290_set_register_array(imx290, regs, reg_num); > > > > > > if (ret < 0) { > > > > > > dev_err(imx290->dev, "Could not set init registers\n"); > > > return ret; > > > > > > @@ -1072,7 +1135,7 @@ static int imx290_enum_mbus_code(struct v4l2_subdev > > > *sd,> > > > if (code->index >= ARRAY_SIZE(imx290_formats)) > > > > > > return -EINVAL; > > > > > > - code->code = imx290_formats[code->index].code[imx290->mono]; > > > + code->code = > > > imx290_formats[code->index].code[imx290->devdata->mono];> > > > return 0; > > > > > > } > > > > > > @@ -1114,7 +1177,7 @@ static int imx290_set_fmt(struct v4l2_subdev *sd, > > > > > > fmt->format.height = mode->height; > > > > > > if (!imx290_format_info(imx290, fmt->format.code)) > > > > > > - fmt->format.code = imx290_formats[0].code[imx290->mono]; > > > + fmt->format.code = > > > imx290_formats[0].code[imx290->devdata->mono];> > > > fmt->format.field = V4L2_FIELD_NONE; > > > fmt->format.colorspace = V4L2_COLORSPACE_RAW; > > > > > > @@ -1422,8 +1485,9 @@ static s64 imx290_check_link_freqs(const struct > > > imx290 *imx290,> > > > } > > > > > > static const struct of_device_id imx290_of_match[] = { > > > > > > - { .compatible = "sony,imx290" }, > > > - { .compatible = "sony,imx290-mono", .data = (void *)1 }, > > > + { .compatible = "sony,imx290", .data = &imx290_models[IMX290] }, > > > + { .compatible = "sony,imx290-mono", .data = > > > &imx290_models[IMX290_MONO] }, + { .compatible = "sony,imx327", > > > .data = &imx290_models[IMX327] }, > > Based on Laurent's requests my parent to this set will be switching to > > imx290 (as legacy), imx290lqr and imx290llr as the compatible strings. > > imx327 ought to follow the same pattern. > > Okay thanks. I'll update to imx327lqr as well. Put me on CC so I'll notice the > update & conflict. > > Best regards, > Alexander > > > Dave > > > > > { /* sentinel */ } > > > > > > }; > > > MODULE_DEVICE_TABLE(of, imx290_of_match); > > > > > > @@ -1441,8 +1505,7 @@ static int imx290_parse_dt(struct imx290 *imx290) > > > > > > s64 fq; > > > > > > match = i2c_of_match_device(imx290_of_match, client); > > > > > > - if (match) > > > - imx290->mono = match->data ? 1 : 0; > > > + imx290->devdata = match->data; > > > > > > endpoint = fwnode_graph_get_next_endpoint(dev_fwnode(imx290->dev), > > > NULL); > > > if (!endpoint) { > > > > > > @@ -1561,6 +1624,9 @@ static int imx290_probe(struct i2c_client *client) > > > > > > if (ret) > > > > > > goto err_pm; > > > > > > + v4l2_i2c_subdev_set_name(&imx290->sd, client, > > > + imx290->devdata->name, NULL); > > > + > > > > > > /* > > > > > > * Finally, register the V4L2 subdev. This must be done after > > > * initializing everything as the subdev can be used immediately > > > after > > > > > > -- > > > 2.34.1 > > > >
diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c index e642e1df520d..337252b2ec15 100644 --- a/drivers/media/i2c/imx290.c +++ b/drivers/media/i2c/imx290.c @@ -164,6 +164,36 @@ #define CLK_74_25 1 #define NUM_CLK 2 +enum imx290_model { + IMX290, + IMX290_MONO, + IMX327, +}; + +struct imx290_device_data { + enum imx290_model model; + const char *name; + u8 mono; +}; + +static const struct imx290_device_data imx290_models[] = { + [IMX290] = { + .model = IMX290, + .name = "imx290", + .mono = 0, + }, + [IMX290_MONO] = { + .model = IMX290_MONO, + .name = "imx290-mono", + .mono = 1, + }, + [IMX327] = { + .model = IMX327, + .name = "imx327", + .mono = 0, + }, +}; + struct imx290_regval { u32 reg; u32 val; @@ -210,9 +240,9 @@ struct imx290 { struct device *dev; struct clk *xclk; struct regmap *regmap; + const struct imx290_device_data *devdata; u32 xclk_freq; u8 nlanes; - u8 mono; struct v4l2_subdev sd; struct media_pad pad; @@ -240,7 +270,7 @@ static inline struct imx290 *to_imx290(struct v4l2_subdev *_sd) * Modes and formats */ -static const struct imx290_regval imx290_global_init_settings[] = { +static const struct imx290_regval imx290_global_init_settings_290[] = { { IMX290_WINWV_OB, 12 }, { IMX290_WINPH, 0 }, { IMX290_WINPV, 0 }, @@ -292,6 +322,23 @@ static const struct imx290_regval imx290_global_init_settings[] = { { IMX290_REG_8BIT(0x33b3), 0x04 }, }; +static const struct imx290_regval imx290_global_init_settings_327[] = { + { IMX290_WINWV_OB, 12 }, + { IMX290_WINPH, 0 }, + { IMX290_WINPV, 0 }, + { IMX290_WINWH, 1948 }, + { IMX290_WINWV, 1097 }, + { IMX290_XSOUTSEL, IMX290_XSOUTSEL_XVSOUTSEL_VSYNC | + IMX290_XSOUTSEL_XHSOUTSEL_HSYNC }, + { IMX290_REG_8BIT(0x3011), 0x0A }, + { IMX290_REG_8BIT(0x3012), 0x64 }, + { IMX290_REG_8BIT(0x3013), 0x00 }, + { IMX290_REG_8BIT(0x309e), 0x4A }, + { IMX290_REG_8BIT(0x309f), 0x4A }, + { IMX290_REG_8BIT(0x3128), 0x04 }, + { IMX290_REG_8BIT(0x313b), 0x41 }, +}; + static const struct imx290_regval imx290_37_125mhz_clock[] = { { IMX290_EXTCK_FREQ, 0x2520 }, { IMX290_INCKSEL7, 0x49 }, @@ -558,7 +605,7 @@ imx290_format_info(const struct imx290 *imx290, u32 code) for (i = 0; i < ARRAY_SIZE(imx290_formats); ++i) { const struct imx290_format_info *info = &imx290_formats[i]; - if (info->code[imx290->mono] == code) + if (info->code[imx290->devdata->mono] == code) return info; } @@ -957,11 +1004,27 @@ static int imx290_start_streaming(struct imx290 *imx290, struct v4l2_subdev_state *state) { const struct v4l2_mbus_framefmt *format; + const struct imx290_regval *regs; + unsigned int reg_num; int ret; + switch (imx290->devdata->model) { + case IMX290: + case IMX290_MONO: + regs = imx290_global_init_settings_290; + reg_num = ARRAY_SIZE(imx290_global_init_settings_290); + break; + case IMX327: + regs = imx290_global_init_settings_327; + reg_num = ARRAY_SIZE(imx290_global_init_settings_327); + break; + default: + dev_err(imx290->dev, "Invalid model: %u\n", imx290->devdata->model); + return -EINVAL; + } + /* Set init register settings */ - ret = imx290_set_register_array(imx290, imx290_global_init_settings, - ARRAY_SIZE(imx290_global_init_settings)); + ret = imx290_set_register_array(imx290, regs, reg_num); if (ret < 0) { dev_err(imx290->dev, "Could not set init registers\n"); return ret; @@ -1072,7 +1135,7 @@ static int imx290_enum_mbus_code(struct v4l2_subdev *sd, if (code->index >= ARRAY_SIZE(imx290_formats)) return -EINVAL; - code->code = imx290_formats[code->index].code[imx290->mono]; + code->code = imx290_formats[code->index].code[imx290->devdata->mono]; return 0; } @@ -1114,7 +1177,7 @@ static int imx290_set_fmt(struct v4l2_subdev *sd, fmt->format.height = mode->height; if (!imx290_format_info(imx290, fmt->format.code)) - fmt->format.code = imx290_formats[0].code[imx290->mono]; + fmt->format.code = imx290_formats[0].code[imx290->devdata->mono]; fmt->format.field = V4L2_FIELD_NONE; fmt->format.colorspace = V4L2_COLORSPACE_RAW; @@ -1422,8 +1485,9 @@ static s64 imx290_check_link_freqs(const struct imx290 *imx290, } static const struct of_device_id imx290_of_match[] = { - { .compatible = "sony,imx290" }, - { .compatible = "sony,imx290-mono", .data = (void *)1 }, + { .compatible = "sony,imx290", .data = &imx290_models[IMX290] }, + { .compatible = "sony,imx290-mono", .data = &imx290_models[IMX290_MONO] }, + { .compatible = "sony,imx327", .data = &imx290_models[IMX327] }, { /* sentinel */ } }; MODULE_DEVICE_TABLE(of, imx290_of_match); @@ -1441,8 +1505,7 @@ static int imx290_parse_dt(struct imx290 *imx290) s64 fq; match = i2c_of_match_device(imx290_of_match, client); - if (match) - imx290->mono = match->data ? 1 : 0; + imx290->devdata = match->data; endpoint = fwnode_graph_get_next_endpoint(dev_fwnode(imx290->dev), NULL); if (!endpoint) { @@ -1561,6 +1624,9 @@ static int imx290_probe(struct i2c_client *client) if (ret) goto err_pm; + v4l2_i2c_subdev_set_name(&imx290->sd, client, + imx290->devdata->name, NULL); + /* * Finally, register the V4L2 subdev. This must be done after * initializing everything as the subdev can be used immediately after
Both sensors are quite similar. Their specs only differ regarding LVDS and parallel output but are identical regarding MIPI-CSI-2 interface. But they use a different init setting of hard-coded values, taken from the datasheet. Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com> --- drivers/media/i2c/imx290.c | 88 +++++++++++++++++++++++++++++++++----- 1 file changed, 77 insertions(+), 11 deletions(-)