* [PATCH v2 0/3] media: i2c: Add OmniVision OG0VA1B camera sensor driver
@ 2026-07-02 10:52 Wenmeng Liu
2026-07-02 10:52 ` [PATCH v2 1/3] dt-bindings: media: i2c: og0ve1b: Add OmniVision OG0VA1B camera sensor Wenmeng Liu
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Wenmeng Liu @ 2026-07-02 10:52 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Sakari Ailus, Vladimir Zapolskiy
Cc: linux-media, devicetree, linux-kernel, Wenmeng Liu
Add OmniVision OG0VA1B driver support. The OmniVision OG0VA1B is a
1/10-inch monochrome CMOS VGA image sensor. It outputs 10-bit raw (Y10)
frames at up to 640x480 resolution over a single-lane MIPI CSI-2
interface and is controlled via an I2C-compatible SCCB bus.
This driver has been verified on the Purwa EVK.
Signed-off-by: Wenmeng Liu <wenmeng.liu@oss.qualcomm.com>
---
Changes in v2:
- Integrate OG0VA1B into the existing og0ve1b driver and binding
instead. -- Vladimir
- Link to v1: https://lore.kernel.org/r/20260618-og0va1b-v1-0-dda71bb83009@oss.qualcomm.com
---
Wenmeng Liu (3):
dt-bindings: media: i2c: og0ve1b: Add OmniVision OG0VA1B camera sensor
media: i2c: og0ve1b: Introduce per-sensor data structure
media: i2c: og0ve1b: Add support for OmniVision OG0VA1B
.../bindings/media/i2c/ovti,og0ve1b.yaml | 4 +-
drivers/media/i2c/og0ve1b.c | 333 ++++++++++++++++++---
2 files changed, 299 insertions(+), 38 deletions(-)
---
base-commit: 4f441960e691d37c880d2cc004de06bb5b6bd5e4
change-id: 20260618-og0va1b-55bbf3cabb0e
Best regards,
--
Wenmeng Liu <wenmeng.liu@oss.qualcomm.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 1/3] dt-bindings: media: i2c: og0ve1b: Add OmniVision OG0VA1B camera sensor
2026-07-02 10:52 [PATCH v2 0/3] media: i2c: Add OmniVision OG0VA1B camera sensor driver Wenmeng Liu
@ 2026-07-02 10:52 ` Wenmeng Liu
2026-07-02 12:24 ` Vladimir Zapolskiy
2026-07-02 19:21 ` Conor Dooley
2026-07-02 10:52 ` [PATCH v2 2/3] media: i2c: og0ve1b: Introduce per-sensor data structure Wenmeng Liu
2026-07-02 10:52 ` [PATCH v2 3/3] media: i2c: og0ve1b: Add support for OmniVision OG0VA1B Wenmeng Liu
2 siblings, 2 replies; 10+ messages in thread
From: Wenmeng Liu @ 2026-07-02 10:52 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Sakari Ailus, Vladimir Zapolskiy
Cc: linux-media, devicetree, linux-kernel, Wenmeng Liu
The OmniVision OG0VA1B is a monochrome image sensor closely related to
the already supported OG0VE1B. It shares the same SCCB control interface,
power supplies, clock and MIPI D-PHY description, but differs in its
chip ID, register programming and output format.
Signed-off-by: Wenmeng Liu <wenmeng.liu@oss.qualcomm.com>
---
Documentation/devicetree/bindings/media/i2c/ovti,og0ve1b.yaml | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,og0ve1b.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,og0ve1b.yaml
index bd2f1ae23e6547032361924a6953000bab1129df..ae44ec2fa9035b4e19834888a41987ae9e25118f 100644
--- a/Documentation/devicetree/bindings/media/i2c/ovti,og0ve1b.yaml
+++ b/Documentation/devicetree/bindings/media/i2c/ovti,og0ve1b.yaml
@@ -21,7 +21,9 @@ allOf:
properties:
compatible:
- const: ovti,og0ve1b
+ enum:
+ - ovti,og0va1b
+ - ovti,og0ve1b
reg:
maxItems: 1
--
2.34.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 2/3] media: i2c: og0ve1b: Introduce per-sensor data structure
2026-07-02 10:52 [PATCH v2 0/3] media: i2c: Add OmniVision OG0VA1B camera sensor driver Wenmeng Liu
2026-07-02 10:52 ` [PATCH v2 1/3] dt-bindings: media: i2c: og0ve1b: Add OmniVision OG0VA1B camera sensor Wenmeng Liu
@ 2026-07-02 10:52 ` Wenmeng Liu
2026-07-02 11:04 ` sashiko-bot
` (2 more replies)
2026-07-02 10:52 ` [PATCH v2 3/3] media: i2c: og0ve1b: Add support for OmniVision OG0VA1B Wenmeng Liu
2 siblings, 3 replies; 10+ messages in thread
From: Wenmeng Liu @ 2026-07-02 10:52 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Sakari Ailus, Vladimir Zapolskiy
Cc: linux-media, devicetree, linux-kernel, Wenmeng Liu
In preparation for supporting further OmniVision sensors that share most
of this driver, move the sensor-specific parameters (chip id, MCLK
frequency, test pattern register, link frequency menu and the list of
supported modes) into a new struct og0ve1b_sensor_data, selected through
i2c_get_match_data() at probe time.
Signed-off-by: Wenmeng Liu <wenmeng.liu@oss.qualcomm.com>
---
drivers/media/i2c/og0ve1b.c | 101 +++++++++++++++++++++++++++++---------------
1 file changed, 67 insertions(+), 34 deletions(-)
diff --git a/drivers/media/i2c/og0ve1b.c b/drivers/media/i2c/og0ve1b.c
index 84a28cdcade10f8fbcf945999e88f84641b9bc0d..acc06b10bf896f734926289099a70fbc2bb628d5 100644
--- a/drivers/media/i2c/og0ve1b.c
+++ b/drivers/media/i2c/og0ve1b.c
@@ -66,10 +66,21 @@ struct og0ve1b_mode {
u32 hts; /* Horizontal timing size */
u32 vts; /* Default vertical timing size */
u32 bpp; /* Bits per pixel */
+ u32 code; /* MEDIA_BUS_FMT code */
const struct og0ve1b_reg_list reg_list; /* Sensor register setting */
};
+struct og0ve1b_sensor_data {
+ u64 chip_id;
+ unsigned long mclk_freq;
+ u32 test_pattern_reg;
+ const s64 *link_freq_menu;
+ int num_link_freqs;
+ const struct og0ve1b_mode *modes;
+ int num_modes;
+};
+
static const char * const og0ve1b_test_pattern_menu[] = {
"Disabled",
"Vertical Colour Bars",
@@ -97,8 +108,7 @@ struct og0ve1b {
struct v4l2_ctrl *exposure;
struct v4l2_ctrl_handler ctrl_handler;
- /* Saved register value */
- u64 pre_isp;
+ const struct og0ve1b_sensor_data *sensor;
};
static const struct cci_reg_sequence og0ve1b_640x480_120fps_mode[] = {
@@ -254,6 +264,7 @@ static const struct og0ve1b_mode supported_modes[] = {
.hts = 792,
.vts = 568,
.bpp = 8,
+ .code = MEDIA_BUS_FMT_Y8_1X8,
.reg_list = {
.regs = og0ve1b_640x480_120fps_mode,
.num_regs = ARRAY_SIZE(og0ve1b_640x480_120fps_mode),
@@ -261,23 +272,39 @@ static const struct og0ve1b_mode supported_modes[] = {
},
};
+static const struct og0ve1b_sensor_data og0ve1b_data = {
+ .chip_id = OG0VE1B_CHIP_ID,
+ .mclk_freq = OG0VE1B_MCLK_FREQ_24MHZ,
+ .test_pattern_reg = OG0VE1B_REG_PRE_ISP,
+ .link_freq_menu = og0ve1b_link_freq_menu,
+ .num_link_freqs = ARRAY_SIZE(og0ve1b_link_freq_menu),
+ .modes = supported_modes,
+ .num_modes = ARRAY_SIZE(supported_modes),
+};
+
static int og0ve1b_enable_test_pattern(struct og0ve1b *og0ve1b, u32 pattern)
{
- u64 val = og0ve1b->pre_isp;
+ u32 reg = og0ve1b->sensor->test_pattern_reg;
+ u64 val;
+ int ret;
+
+ ret = cci_read(og0ve1b->regmap, reg, &val, NULL);
+ if (ret)
+ return ret;
if (pattern)
val |= OG0VE1B_TEST_PATTERN_ENABLE;
else
val &= ~OG0VE1B_TEST_PATTERN_ENABLE;
- return cci_write(og0ve1b->regmap, OG0VE1B_REG_PRE_ISP, val, NULL);
+ return cci_write(og0ve1b->regmap, reg, val, NULL);
}
static int og0ve1b_set_ctrl(struct v4l2_ctrl *ctrl)
{
struct og0ve1b *og0ve1b = container_of(ctrl->handler, struct og0ve1b,
ctrl_handler);
- const struct og0ve1b_mode *mode = &supported_modes[0];
+ const struct og0ve1b_mode *mode = &og0ve1b->sensor->modes[0];
s64 exposure_max;
int ret;
@@ -333,7 +360,8 @@ static const struct v4l2_ctrl_ops og0ve1b_ctrl_ops = {
static int og0ve1b_init_controls(struct og0ve1b *og0ve1b)
{
struct v4l2_ctrl_handler *ctrl_hdlr = &og0ve1b->ctrl_handler;
- const struct og0ve1b_mode *mode = &supported_modes[0];
+ const struct og0ve1b_mode *mode = &og0ve1b->sensor->modes[0];
+ const struct og0ve1b_sensor_data *sensor = og0ve1b->sensor;
s64 exposure_max, pixel_rate, h_blank, v_blank;
struct v4l2_fwnode_device_properties props;
struct v4l2_ctrl *ctrl;
@@ -343,12 +371,12 @@ static int og0ve1b_init_controls(struct og0ve1b *og0ve1b)
ctrl = v4l2_ctrl_new_int_menu(ctrl_hdlr, &og0ve1b_ctrl_ops,
V4L2_CID_LINK_FREQ,
- ARRAY_SIZE(og0ve1b_link_freq_menu) - 1,
- 0, og0ve1b_link_freq_menu);
+ sensor->num_link_freqs - 1,
+ 0, sensor->link_freq_menu);
if (ctrl)
ctrl->flags |= V4L2_CTRL_FLAG_READ_ONLY;
- pixel_rate = og0ve1b_link_freq_menu[0] / mode->bpp;
+ pixel_rate = sensor->link_freq_menu[0] / mode->bpp;
v4l2_ctrl_new_std(ctrl_hdlr, &og0ve1b_ctrl_ops, V4L2_CID_PIXEL_RATE,
0, pixel_rate, 1, pixel_rate);
@@ -407,7 +435,7 @@ static int og0ve1b_init_controls(struct og0ve1b *og0ve1b)
static void og0ve1b_update_pad_format(const struct og0ve1b_mode *mode,
struct v4l2_mbus_framefmt *fmt)
{
- fmt->code = MEDIA_BUS_FMT_Y8_1X8;
+ fmt->code = mode->code;
fmt->width = mode->width;
fmt->height = mode->height;
fmt->field = V4L2_FIELD_NONE;
@@ -421,8 +449,8 @@ static int og0ve1b_enable_streams(struct v4l2_subdev *sd,
struct v4l2_subdev_state *state, u32 pad,
u64 streams_mask)
{
- const struct og0ve1b_reg_list *reg_list = &supported_modes[0].reg_list;
struct og0ve1b *og0ve1b = to_og0ve1b(sd);
+ const struct og0ve1b_reg_list *reg_list = &og0ve1b->sensor->modes[0].reg_list;
int ret;
ret = pm_runtime_resume_and_get(og0ve1b->dev);
@@ -484,13 +512,14 @@ static int og0ve1b_set_pad_format(struct v4l2_subdev *sd,
struct v4l2_subdev_state *state,
struct v4l2_subdev_format *fmt)
{
+ struct og0ve1b *og0ve1b = to_og0ve1b(sd);
struct v4l2_mbus_framefmt *format;
const struct og0ve1b_mode *mode;
format = v4l2_subdev_state_get_format(state, 0);
- mode = v4l2_find_nearest_size(supported_modes,
- ARRAY_SIZE(supported_modes),
+ mode = v4l2_find_nearest_size(og0ve1b->sensor->modes,
+ og0ve1b->sensor->num_modes,
width, height,
fmt->format.width,
fmt->format.height);
@@ -505,10 +534,12 @@ static int og0ve1b_enum_mbus_code(struct v4l2_subdev *sd,
struct v4l2_subdev_state *sd_state,
struct v4l2_subdev_mbus_code_enum *code)
{
+ struct og0ve1b *og0ve1b = to_og0ve1b(sd);
+
if (code->index > 0)
return -EINVAL;
- code->code = MEDIA_BUS_FMT_Y8_1X8;
+ code->code = og0ve1b->sensor->modes[0].code;
return 0;
}
@@ -517,15 +548,18 @@ static int og0ve1b_enum_frame_size(struct v4l2_subdev *sd,
struct v4l2_subdev_state *sd_state,
struct v4l2_subdev_frame_size_enum *fse)
{
- if (fse->index >= ARRAY_SIZE(supported_modes))
+ struct og0ve1b *og0ve1b = to_og0ve1b(sd);
+ const struct og0ve1b_sensor_data *sensor = og0ve1b->sensor;
+
+ if (fse->index >= sensor->num_modes)
return -EINVAL;
- if (fse->code != MEDIA_BUS_FMT_Y8_1X8)
+ if (fse->code != sensor->modes[fse->index].code)
return -EINVAL;
- fse->min_width = supported_modes[fse->index].width;
+ fse->min_width = sensor->modes[fse->index].width;
fse->max_width = fse->min_width;
- fse->min_height = supported_modes[fse->index].height;
+ fse->min_height = sensor->modes[fse->index].height;
fse->max_height = fse->min_height;
return 0;
@@ -534,13 +568,14 @@ static int og0ve1b_enum_frame_size(struct v4l2_subdev *sd,
static int og0ve1b_init_state(struct v4l2_subdev *sd,
struct v4l2_subdev_state *state)
{
+ struct og0ve1b *og0ve1b = to_og0ve1b(sd);
struct v4l2_subdev_format fmt = {
.which = V4L2_SUBDEV_FORMAT_TRY,
.pad = 0,
.format = {
- .code = MEDIA_BUS_FMT_Y8_1X8,
- .width = supported_modes[0].width,
- .height = supported_modes[0].height,
+ .code = og0ve1b->sensor->modes[0].code,
+ .width = og0ve1b->sensor->modes[0].width,
+ .height = og0ve1b->sensor->modes[0].height,
},
};
@@ -586,18 +621,13 @@ static int og0ve1b_identify_sensor(struct og0ve1b *og0ve1b)
return ret;
}
- if (val != OG0VE1B_CHIP_ID) {
- dev_err(og0ve1b->dev, "chip id mismatch: %x!=%llx\n",
- OG0VE1B_CHIP_ID, val);
+ if (val != og0ve1b->sensor->chip_id) {
+ dev_err(og0ve1b->dev, "chip id mismatch: %llx!=%llx\n",
+ og0ve1b->sensor->chip_id, val);
return -ENODEV;
}
- ret = cci_read(og0ve1b->regmap, OG0VE1B_REG_PRE_ISP,
- &og0ve1b->pre_isp, NULL);
- if (ret)
- dev_err(og0ve1b->dev, "failed to read pre_isp: %d\n", ret);
-
- return ret;
+ return 0;
}
static int og0ve1b_check_hwcfg(struct og0ve1b *og0ve1b)
@@ -624,8 +654,8 @@ static int og0ve1b_check_hwcfg(struct og0ve1b *og0ve1b)
ret = v4l2_link_freq_to_bitmap(og0ve1b->dev,
bus_cfg.link_frequencies,
bus_cfg.nr_of_link_frequencies,
- og0ve1b_link_freq_menu,
- ARRAY_SIZE(og0ve1b_link_freq_menu),
+ og0ve1b->sensor->link_freq_menu,
+ og0ve1b->sensor->num_link_freqs,
&freq_bitmap);
v4l2_fwnode_endpoint_free(&bus_cfg);
@@ -686,6 +716,9 @@ static int og0ve1b_probe(struct i2c_client *client)
return -ENOMEM;
og0ve1b->dev = &client->dev;
+ og0ve1b->sensor = i2c_get_match_data(client);
+ if (!og0ve1b->sensor)
+ return -ENODEV;
v4l2_i2c_subdev_init(&og0ve1b->sd, client, &og0ve1b_subdev_ops);
@@ -700,7 +733,7 @@ static int og0ve1b_probe(struct i2c_client *client)
"failed to get XVCLK clock\n");
freq = clk_get_rate(og0ve1b->xvclk);
- if (freq && freq != OG0VE1B_MCLK_FREQ_24MHZ)
+ if (freq && freq != og0ve1b->sensor->mclk_freq)
return dev_err_probe(og0ve1b->dev, -EINVAL,
"XVCLK clock frequency %lu is not supported\n",
freq);
@@ -819,7 +852,7 @@ static const struct dev_pm_ops og0ve1b_pm_ops = {
};
static const struct of_device_id og0ve1b_of_match[] = {
- { .compatible = "ovti,og0ve1b" },
+ { .compatible = "ovti,og0ve1b", .data = &og0ve1b_data },
{ /* sentinel */ }
};
MODULE_DEVICE_TABLE(of, og0ve1b_of_match);
--
2.34.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 3/3] media: i2c: og0ve1b: Add support for OmniVision OG0VA1B
2026-07-02 10:52 [PATCH v2 0/3] media: i2c: Add OmniVision OG0VA1B camera sensor driver Wenmeng Liu
2026-07-02 10:52 ` [PATCH v2 1/3] dt-bindings: media: i2c: og0ve1b: Add OmniVision OG0VA1B camera sensor Wenmeng Liu
2026-07-02 10:52 ` [PATCH v2 2/3] media: i2c: og0ve1b: Introduce per-sensor data structure Wenmeng Liu
@ 2026-07-02 10:52 ` Wenmeng Liu
2026-07-02 13:16 ` Vladimir Zapolskiy
2 siblings, 1 reply; 10+ messages in thread
From: Wenmeng Liu @ 2026-07-02 10:52 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Sakari Ailus, Vladimir Zapolskiy
Cc: linux-media, devicetree, linux-kernel, Wenmeng Liu
The OmniVision OG0VA1B is a monochrome image sensor closely related to
the OG0VE1B. It shares the SCCB control interface, power supplies and
the single-lane MIPI D-PHY description, and differs in its chip id, the
test pattern register, the register programming and the output format
(10-bit RAW instead of 8-bit).
Add an og0ve1b_sensor_data entry describing the OG0VA1B together with
its 640x480 60fps register sequence.
Signed-off-by: Wenmeng Liu <wenmeng.liu@oss.qualcomm.com>
---
drivers/media/i2c/og0ve1b.c | 234 +++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 230 insertions(+), 4 deletions(-)
diff --git a/drivers/media/i2c/og0ve1b.c b/drivers/media/i2c/og0ve1b.c
index acc06b10bf896f734926289099a70fbc2bb628d5..f1f4fcd195c4e2dd69f78ba3f01f768a052e6c5d 100644
--- a/drivers/media/i2c/og0ve1b.c
+++ b/drivers/media/i2c/og0ve1b.c
@@ -17,8 +17,12 @@
#define OG0VE1B_LINK_FREQ_500MHZ (500 * HZ_PER_MHZ)
#define OG0VE1B_MCLK_FREQ_24MHZ (24 * HZ_PER_MHZ)
+#define OG0VA1B_LINK_FREQ_480MHZ (480 * HZ_PER_MHZ)
+#define OG0VA1B_MCLK_FREQ_19_2MHZ (19200 * HZ_PER_KHZ)
+
#define OG0VE1B_REG_CHIP_ID CCI_REG24(0x300a)
#define OG0VE1B_CHIP_ID 0xc75645
+#define OG0VA1B_CHIP_ID 0xc75641
#define OG0VE1B_REG_MODE_SELECT CCI_REG8(0x0100)
#define OG0VE1B_MODE_STANDBY 0x00
@@ -45,8 +49,9 @@
#define OG0VE1B_REG_VTS CCI_REG16(0x380e)
#define OG0VE1B_VTS_MAX 0xffff
-/* Test pattern */
+/* Test pattern - OG0VE1B uses 0x5e00, OG0VA1B uses 0x5100 */
#define OG0VE1B_REG_PRE_ISP CCI_REG8(0x5e00)
+#define OG0VA1B_REG_TEST_PATTERN CCI_REG8(0x5100)
#define OG0VE1B_TEST_PATTERN_ENABLE BIT(7)
#define to_og0ve1b(_sd) container_of(_sd, struct og0ve1b, sd)
@@ -55,6 +60,10 @@ static const s64 og0ve1b_link_freq_menu[] = {
OG0VE1B_LINK_FREQ_500MHZ,
};
+static const s64 og0va1b_link_freq_menu[] = {
+ OG0VA1B_LINK_FREQ_480MHZ,
+};
+
struct og0ve1b_reg_list {
const struct cci_reg_sequence *regs;
unsigned int num_regs;
@@ -72,9 +81,14 @@ struct og0ve1b_mode {
};
struct og0ve1b_sensor_data {
+ const char *name;
u64 chip_id;
unsigned long mclk_freq;
u32 test_pattern_reg;
+ /* Exposure register unit: OG0VE1B 1/16 line (4), OG0VA1B whole lines (0). */
+ unsigned int exposure_shift;
+ /* Pixel rate multiplier: OG0VA1B uses CSI-2 DDR (2), OG0VE1B keeps 1. */
+ unsigned int pixel_rate_mul;
const s64 *link_freq_menu;
int num_link_freqs;
const struct og0ve1b_mode *modes;
@@ -272,16 +286,223 @@ static const struct og0ve1b_mode supported_modes[] = {
},
};
+static const struct cci_reg_sequence og0va1b_640x480_60fps_mode[] = {
+ { CCI_REG8(0x0302), 0x31 },
+ { CCI_REG8(0x0303), 0x02 },
+ { CCI_REG8(0x0304), 0x01 },
+ { CCI_REG8(0x0305), 0x90 },
+ { CCI_REG8(0x0306), 0x00 },
+ { CCI_REG8(0x0323), 0x02 },
+ { CCI_REG8(0x0325), 0x68 },
+ { CCI_REG8(0x0326), 0xd8 },
+ { CCI_REG8(0x3006), 0x0e },
+ { CCI_REG8(0x300d), 0x08 },
+ { CCI_REG8(0x3018), 0xf0 },
+ { CCI_REG8(0x301c), 0xf0 },
+ { CCI_REG8(0x3020), 0x20 },
+ { CCI_REG8(0x3040), 0x0f },
+ { CCI_REG8(0x3022), 0x01 },
+ { CCI_REG8(0x3107), 0x40 },
+ { CCI_REG8(0x3216), 0x01 },
+ { CCI_REG8(0x3217), 0x00 },
+ { CCI_REG8(0x3218), 0xc0 },
+ { CCI_REG8(0x3219), 0x55 },
+ { CCI_REG8(0x3506), 0x01 },
+ { CCI_REG8(0x3507), 0x50 },
+ { CCI_REG8(0x3508), 0x01 },
+ { CCI_REG8(0x3509), 0x00 },
+ { CCI_REG8(0x350a), 0x01 },
+ { CCI_REG8(0x350b), 0x00 },
+ { CCI_REG8(0x350c), 0x00 },
+ { CCI_REG8(0x3541), 0x00 },
+ { CCI_REG8(0x3542), 0x40 },
+ { CCI_REG8(0x3605), 0x90 },
+ { CCI_REG8(0x3606), 0x41 },
+ { CCI_REG8(0x3612), 0x00 },
+ { CCI_REG8(0x3620), 0x08 },
+ { CCI_REG8(0x3630), 0x17 },
+ { CCI_REG8(0x3631), 0x99 },
+ { CCI_REG8(0x3639), 0x88 },
+ { CCI_REG8(0x3668), 0x00 },
+ { CCI_REG8(0x3674), 0x00 },
+ { CCI_REG8(0x3677), 0x3f },
+ { CCI_REG8(0x368f), 0x06 },
+ { CCI_REG8(0x36a2), 0x19 },
+ { CCI_REG8(0x36a4), 0xf1 },
+ { CCI_REG8(0x36a5), 0x2d },
+ { CCI_REG8(0x3706), 0x30 },
+ { CCI_REG8(0x370d), 0x72 },
+ { CCI_REG8(0x3713), 0x86 },
+ { CCI_REG8(0x3715), 0x03 },
+ { CCI_REG8(0x3716), 0x00 },
+ { CCI_REG8(0x376d), 0x24 },
+ { CCI_REG8(0x3770), 0x3a },
+ { CCI_REG8(0x3778), 0x00 },
+ { CCI_REG8(0x37a8), 0x03 },
+ { CCI_REG8(0x37a9), 0x00 },
+ { CCI_REG8(0x37df), 0x7d },
+ { CCI_REG8(0x3800), 0x00 },
+ { CCI_REG8(0x3801), 0x00 },
+ { CCI_REG8(0x3802), 0x00 },
+ { CCI_REG8(0x3803), 0x00 },
+ { CCI_REG8(0x3804), 0x02 },
+ { CCI_REG8(0x3805), 0x8f },
+ { CCI_REG8(0x3806), 0x01 },
+ { CCI_REG8(0x3807), 0xef },
+ { CCI_REG8(0x3808), 0x02 },
+ { CCI_REG8(0x3809), 0x80 },
+ { CCI_REG8(0x380a), 0x01 },
+ { CCI_REG8(0x380b), 0xe0 },
+ { CCI_REG8(0x380c), 0x01 },
+ { CCI_REG8(0x380d), 0x78 },
+ { CCI_REG8(0x380e), 0x08 },
+ { CCI_REG8(0x380f), 0x30 },
+ { CCI_REG8(0x3810), 0x00 },
+ { CCI_REG8(0x3811), 0x08 },
+ { CCI_REG8(0x3812), 0x00 },
+ { CCI_REG8(0x3813), 0x08 },
+ { CCI_REG8(0x3814), 0x11 },
+ { CCI_REG8(0x3815), 0x11 },
+ { CCI_REG8(0x3816), 0x00 },
+ { CCI_REG8(0x3817), 0x01 },
+ { CCI_REG8(0x3818), 0x00 },
+ { CCI_REG8(0x3819), 0x05 },
+ { CCI_REG8(0x3820), 0x40 },
+ { CCI_REG8(0x3821), 0x04 },
+ { CCI_REG8(0x3823), 0x00 },
+ { CCI_REG8(0x3826), 0x00 },
+ { CCI_REG8(0x3827), 0x00 },
+ { CCI_REG8(0x382b), 0x52 },
+ { CCI_REG8(0x384a), 0xa2 },
+ { CCI_REG8(0x3858), 0x00 },
+ { CCI_REG8(0x3859), 0x00 },
+ { CCI_REG8(0x3860), 0x00 },
+ { CCI_REG8(0x3861), 0x00 },
+ { CCI_REG8(0x3866), 0x0c },
+ { CCI_REG8(0x3867), 0x07 },
+ { CCI_REG8(0x3884), 0x00 },
+ { CCI_REG8(0x3885), 0x08 },
+ { CCI_REG8(0x3888), 0x50 },
+ { CCI_REG8(0x3893), 0x6c },
+ { CCI_REG8(0x3898), 0x00 },
+ { CCI_REG8(0x389a), 0x04 },
+ { CCI_REG8(0x389b), 0x01 },
+ { CCI_REG8(0x389c), 0x0b },
+ { CCI_REG8(0x389d), 0xdc },
+ { CCI_REG8(0x38b1), 0x04 },
+ { CCI_REG8(0x38b2), 0x00 },
+ { CCI_REG8(0x38b3), 0x08 },
+ { CCI_REG8(0x38c1), 0x46 },
+ { CCI_REG8(0x38c9), 0x02 },
+ { CCI_REG8(0x38d4), 0x06 },
+ { CCI_REG8(0x38d5), 0x5a },
+ { CCI_REG8(0x38d6), 0x08 },
+ { CCI_REG8(0x38d7), 0x3a },
+ { CCI_REG8(0x391f), 0x00 },
+ { CCI_REG8(0x3920), 0xaa },
+ { CCI_REG8(0x3921), 0x00 },
+ { CCI_REG8(0x3922), 0x00 },
+ { CCI_REG8(0x3923), 0x00 },
+ { CCI_REG8(0x3924), 0x00 },
+ { CCI_REG8(0x3925), 0x00 },
+ { CCI_REG8(0x3926), 0x00 },
+ { CCI_REG8(0x3927), 0x00 },
+ { CCI_REG8(0x3928), 0x10 },
+ { CCI_REG8(0x3929), 0x01 },
+ { CCI_REG8(0x392a), 0xb4 },
+ { CCI_REG8(0x392b), 0x00 },
+ { CCI_REG8(0x392c), 0x10 },
+ { CCI_REG8(0x392d), 0x01 },
+ { CCI_REG8(0x392e), 0x78 },
+ { CCI_REG8(0x392f), 0x4a },
+ { CCI_REG8(0x391e), 0x01 },
+ { CCI_REG8(0x389f), 0x08 },
+ { CCI_REG8(0x38a0), 0x00 },
+ { CCI_REG8(0x38a1), 0x00 },
+ { CCI_REG8(0x3a06), 0x06 },
+ { CCI_REG8(0x3a07), 0x78 },
+ { CCI_REG8(0x3a08), 0x08 },
+ { CCI_REG8(0x3a09), 0x80 },
+ { CCI_REG8(0x3a52), 0x00 },
+ { CCI_REG8(0x3a53), 0x01 },
+ { CCI_REG8(0x3a54), 0x0c },
+ { CCI_REG8(0x3a55), 0x04 },
+ { CCI_REG8(0x3a58), 0x0c },
+ { CCI_REG8(0x3a59), 0x04 },
+ { CCI_REG8(0x4000), 0xcf },
+ { CCI_REG8(0x4003), 0x40 },
+ { CCI_REG8(0x4008), 0x04 },
+ { CCI_REG8(0x4009), 0x13 },
+ { CCI_REG8(0x400a), 0x02 },
+ { CCI_REG8(0x400b), 0x34 },
+ { CCI_REG8(0x4010), 0x71 },
+ { CCI_REG8(0x4042), 0xc3 },
+ { CCI_REG8(0x4306), 0x04 },
+ { CCI_REG8(0x4307), 0x12 },
+ { CCI_REG8(0x4500), 0x70 },
+ { CCI_REG8(0x4509), 0x00 },
+ { CCI_REG8(0x450b), 0x83 },
+ { CCI_REG8(0x4604), 0x68 },
+ { CCI_REG8(0x481b), 0x44 },
+ { CCI_REG8(0x481f), 0x30 },
+ { CCI_REG8(0x4823), 0x44 },
+ { CCI_REG8(0x4825), 0x35 },
+ { CCI_REG8(0x4837), 0x11 },
+ { CCI_REG8(0x4f00), 0x04 },
+ { CCI_REG8(0x4f10), 0x04 },
+ { CCI_REG8(0x4f21), 0x01 },
+ { CCI_REG8(0x4f22), 0x00 },
+ { CCI_REG8(0x4f23), 0x54 },
+ { CCI_REG8(0x4f24), 0x51 },
+ { CCI_REG8(0x4f25), 0x41 },
+ { CCI_REG8(0x5000), 0x3f },
+ { CCI_REG8(0x5001), 0x80 },
+ { CCI_REG8(0x500a), 0x00 },
+ { CCI_REG8(0x5100), 0x00 },
+ { CCI_REG8(0x5111), 0x20 },
+};
+
+static const struct og0ve1b_mode og0va1b_supported_modes[] = {
+ {
+ .width = 640,
+ .height = 480,
+ .hts = 752,
+ .vts = 2096,
+ .bpp = 10,
+ .code = MEDIA_BUS_FMT_Y10_1X10,
+ .reg_list = {
+ .regs = og0va1b_640x480_60fps_mode,
+ .num_regs = ARRAY_SIZE(og0va1b_640x480_60fps_mode),
+ },
+ },
+};
+
static const struct og0ve1b_sensor_data og0ve1b_data = {
+ .name = "og0ve1b",
.chip_id = OG0VE1B_CHIP_ID,
.mclk_freq = OG0VE1B_MCLK_FREQ_24MHZ,
.test_pattern_reg = OG0VE1B_REG_PRE_ISP,
+ .exposure_shift = 4,
+ .pixel_rate_mul = 1,
.link_freq_menu = og0ve1b_link_freq_menu,
.num_link_freqs = ARRAY_SIZE(og0ve1b_link_freq_menu),
.modes = supported_modes,
.num_modes = ARRAY_SIZE(supported_modes),
};
+static const struct og0ve1b_sensor_data og0va1b_data = {
+ .name = "og0va1b",
+ .chip_id = OG0VA1B_CHIP_ID,
+ .mclk_freq = OG0VA1B_MCLK_FREQ_19_2MHZ,
+ .test_pattern_reg = OG0VA1B_REG_TEST_PATTERN,
+ .exposure_shift = 0,
+ .pixel_rate_mul = 2,
+ .link_freq_menu = og0va1b_link_freq_menu,
+ .num_link_freqs = ARRAY_SIZE(og0va1b_link_freq_menu),
+ .modes = og0va1b_supported_modes,
+ .num_modes = ARRAY_SIZE(og0va1b_supported_modes),
+};
+
static int og0ve1b_enable_test_pattern(struct og0ve1b *og0ve1b, u32 pattern)
{
u32 reg = og0ve1b->sensor->test_pattern_reg;
@@ -334,7 +555,8 @@ static int og0ve1b_set_ctrl(struct v4l2_ctrl *ctrl)
break;
case V4L2_CID_EXPOSURE:
ret = cci_write(og0ve1b->regmap, OG0VE1B_REG_EXPOSURE,
- ctrl->val << 4, NULL);
+ ctrl->val << og0ve1b->sensor->exposure_shift,
+ NULL);
break;
case V4L2_CID_VBLANK:
ret = cci_write(og0ve1b->regmap, OG0VE1B_REG_VTS,
@@ -376,7 +598,8 @@ static int og0ve1b_init_controls(struct og0ve1b *og0ve1b)
if (ctrl)
ctrl->flags |= V4L2_CTRL_FLAG_READ_ONLY;
- pixel_rate = sensor->link_freq_menu[0] / mode->bpp;
+ pixel_rate = sensor->link_freq_menu[0] * sensor->pixel_rate_mul /
+ mode->bpp;
v4l2_ctrl_new_std(ctrl_hdlr, &og0ve1b_ctrl_ops, V4L2_CID_PIXEL_RATE,
0, pixel_rate, 1, pixel_rate);
@@ -721,6 +944,8 @@ static int og0ve1b_probe(struct i2c_client *client)
return -ENODEV;
v4l2_i2c_subdev_init(&og0ve1b->sd, client, &og0ve1b_subdev_ops);
+ v4l2_i2c_subdev_set_name(&og0ve1b->sd, client,
+ og0ve1b->sensor->name, NULL);
og0ve1b->regmap = devm_cci_regmap_init_i2c(client, 16);
if (IS_ERR(og0ve1b->regmap))
@@ -853,6 +1078,7 @@ static const struct dev_pm_ops og0ve1b_pm_ops = {
static const struct of_device_id og0ve1b_of_match[] = {
{ .compatible = "ovti,og0ve1b", .data = &og0ve1b_data },
+ { .compatible = "ovti,og0va1b", .data = &og0va1b_data },
{ /* sentinel */ }
};
MODULE_DEVICE_TABLE(of, og0ve1b_of_match);
@@ -870,5 +1096,5 @@ static struct i2c_driver og0ve1b_i2c_driver = {
module_i2c_driver(og0ve1b_i2c_driver);
MODULE_AUTHOR("Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>");
-MODULE_DESCRIPTION("OmniVision OG0VE1B sensor driver");
+MODULE_DESCRIPTION("OmniVision OG0VE1B/OG0VA1B sensor driver");
MODULE_LICENSE("GPL");
--
2.34.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/3] media: i2c: og0ve1b: Introduce per-sensor data structure
2026-07-02 10:52 ` [PATCH v2 2/3] media: i2c: og0ve1b: Introduce per-sensor data structure Wenmeng Liu
@ 2026-07-02 11:04 ` sashiko-bot
2026-07-02 13:10 ` Vladimir Zapolskiy
2026-07-02 16:31 ` Bryan O'Donoghue
2 siblings, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-07-02 11:04 UTC (permalink / raw)
To: Wenmeng Liu; +Cc: robh, devicetree, conor+dt
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Incorrect return value check of pm_runtime_get_if_active() leads to unbalanced PM usage count and potential log-spam.
--
commit 77fd9b09419f467242cfd9a487b0ac45ff5344ef
Author: Wenmeng Liu <wenmeng.liu@oss.qualcomm.com>
media: i2c: og0ve1b: Introduce per-sensor data structure
This patch moves sensor-specific parameters into a new
struct og0ve1b_sensor_data to prepare for supporting additional
OmniVision sensors. The sensor data is selected at probe time
using i2c_get_match_data().
> diff --git a/drivers/media/i2c/og0ve1b.c b/drivers/media/i2c/og0ve1b.c
> index 84a28cdcade10..acc06b10bf896 100644
> --- a/drivers/media/i2c/og0ve1b.c
> +++ b/drivers/media/i2c/og0ve1b.c
[ ... ]
> @@ -261,23 +272,39 @@ static const struct og0ve1b_mode supported_modes[] = {
[ ... ]
> static int og0ve1b_set_ctrl(struct v4l2_ctrl *ctrl)
> {
> struct og0ve1b *og0ve1b = container_of(ctrl->handler, struct og0ve1b,
> ctrl_handler);
> - const struct og0ve1b_mode *mode = &supported_modes[0];
> + const struct og0ve1b_mode *mode = &og0ve1b->sensor->modes[0];
> s64 exposure_max;
> int ret;
[Severity: High]
This isn't a bug introduced by this patch, but I noticed a pre-existing
issue in this function regarding the return value check of
pm_runtime_get_if_active().
Later in og0ve1b_set_ctrl(), the code does:
if (!pm_runtime_get_if_active(og0ve1b->dev))
return 0;
Because pm_runtime_get_if_active() can return a negative error code
(like -EINVAL when PM is disabled), the logical NOT operator will evaluate
the negative error as false. This bypasses the error condition entirely,
allowing execution to proceed to I2C hardware register writes and
unconditionally calling pm_runtime_put().
Since the initial 'get' call did not increment the usage counter on error,
the subsequent 'put' call decrements it improperly. Could this cause an
unbalanced PM usage count underflow and trigger kernel warnings?
Should this condition be updated to explicitly check if the return value
is less than or equal to zero?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260702-og0va1b-v2-0-0071442caa2a@oss.qualcomm.com?part=2
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/3] dt-bindings: media: i2c: og0ve1b: Add OmniVision OG0VA1B camera sensor
2026-07-02 10:52 ` [PATCH v2 1/3] dt-bindings: media: i2c: og0ve1b: Add OmniVision OG0VA1B camera sensor Wenmeng Liu
@ 2026-07-02 12:24 ` Vladimir Zapolskiy
2026-07-02 19:21 ` Conor Dooley
1 sibling, 0 replies; 10+ messages in thread
From: Vladimir Zapolskiy @ 2026-07-02 12:24 UTC (permalink / raw)
To: Wenmeng Liu, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Sakari Ailus
Cc: linux-media, devicetree, linux-kernel
Hi Wenmeng,
thank you for the updates!
On 7/2/26 13:52, Wenmeng Liu wrote:
> The OmniVision OG0VA1B is a monochrome image sensor closely related to
> the already supported OG0VE1B. It shares the same SCCB control interface,
> power supplies, clock and MIPI D-PHY description, but differs in its
> chip ID, register programming and output format.
>
> Signed-off-by: Wenmeng Liu <wenmeng.liu@oss.qualcomm.com>
> ---
> Documentation/devicetree/bindings/media/i2c/ovti,og0ve1b.yaml | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,og0ve1b.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,og0ve1b.yaml
> index bd2f1ae23e6547032361924a6953000bab1129df..ae44ec2fa9035b4e19834888a41987ae9e25118f 100644
> --- a/Documentation/devicetree/bindings/media/i2c/ovti,og0ve1b.yaml
> +++ b/Documentation/devicetree/bindings/media/i2c/ovti,og0ve1b.yaml
> @@ -21,7 +21,9 @@ allOf:
>
> properties:
> compatible:
> - const: ovti,og0ve1b
> + enum:
> + - ovti,og0va1b
> + - ovti,og0ve1b
>
> reg:
> maxItems: 1
>
Since it becomes a shared dt documentation, it will be accepted to get
a generalised description section also, in any case:
Reviewed-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
--
Best wishes,
Vladimir
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/3] media: i2c: og0ve1b: Introduce per-sensor data structure
2026-07-02 10:52 ` [PATCH v2 2/3] media: i2c: og0ve1b: Introduce per-sensor data structure Wenmeng Liu
2026-07-02 11:04 ` sashiko-bot
@ 2026-07-02 13:10 ` Vladimir Zapolskiy
2026-07-02 16:31 ` Bryan O'Donoghue
2 siblings, 0 replies; 10+ messages in thread
From: Vladimir Zapolskiy @ 2026-07-02 13:10 UTC (permalink / raw)
To: Wenmeng Liu, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Sakari Ailus
Cc: linux-media, devicetree, linux-kernel
Hi Wenmeng.
On 7/2/26 13:52, Wenmeng Liu wrote:
> In preparation for supporting further OmniVision sensors that share most
> of this driver, move the sensor-specific parameters (chip id, MCLK
> frequency, test pattern register, link frequency menu and the list of
> supported modes) into a new struct og0ve1b_sensor_data, selected through
> i2c_get_match_data() at probe time.
>
> Signed-off-by: Wenmeng Liu <wenmeng.liu@oss.qualcomm.com>
> ---
> drivers/media/i2c/og0ve1b.c | 101 +++++++++++++++++++++++++++++---------------
> 1 file changed, 67 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/media/i2c/og0ve1b.c b/drivers/media/i2c/og0ve1b.c
> index 84a28cdcade10f8fbcf945999e88f84641b9bc0d..acc06b10bf896f734926289099a70fbc2bb628d5 100644
> --- a/drivers/media/i2c/og0ve1b.c
> +++ b/drivers/media/i2c/og0ve1b.c
> @@ -66,10 +66,21 @@ struct og0ve1b_mode {
> u32 hts; /* Horizontal timing size */
> u32 vts; /* Default vertical timing size */
> u32 bpp; /* Bits per pixel */
> + u32 code; /* MEDIA_BUS_FMT code */
In this particular case of grayscale only sensors there is no need for both
bpp and media bus format code, since there is a natural one-to-one mapping.
I'm fine with "code" just replacing "bpp", and in og0ve1b_init_controls()
simply replace "bpp" with "(code == MEDIA_BUS_FMT_Y8_1X8 ? 8 : 10)".
>
> const struct og0ve1b_reg_list reg_list; /* Sensor register setting */
> };
>
> +struct og0ve1b_sensor_data {
> + u64 chip_id;
> + unsigned long mclk_freq;
> + u32 test_pattern_reg;
> + const s64 *link_freq_menu;
> + int num_link_freqs;
> + const struct og0ve1b_mode *modes;
> + int num_modes;
> +};
> +
> static const char * const og0ve1b_test_pattern_menu[] = {
> "Disabled",
> "Vertical Colour Bars",
> @@ -97,8 +108,7 @@ struct og0ve1b {
> struct v4l2_ctrl *exposure;
> struct v4l2_ctrl_handler ctrl_handler;
>
> - /* Saved register value */
> - u64 pre_isp;
I'm afraid this will break test pattern on og0ve1b, unfortunately it should
be kept as is for now.
The register is used for something else, and IIRC even cci_update_bits() API
does not work expectedly, it certainly should be read once and written later.
Hence it leaves an option to keep og0ve1b_enable_test_pattern() and then
introduce in 3/3 a new og0va1b_enable_test_pattern() function, the selection
will be done in runtime similarly to other og0ve/og0va branches.
> + const struct og0ve1b_sensor_data *sensor;
Well, the identifier name is confusing to me, it is not "sensor", it's
"sensor data" or just "data", no?
> };
>
> static const struct cci_reg_sequence og0ve1b_640x480_120fps_mode[] = {
> @@ -254,6 +264,7 @@ static const struct og0ve1b_mode supported_modes[] = {
> .hts = 792,
> .vts = 568,
> .bpp = 8,
> + .code = MEDIA_BUS_FMT_Y8_1X8,
> .reg_list = {
> .regs = og0ve1b_640x480_120fps_mode,
> .num_regs = ARRAY_SIZE(og0ve1b_640x480_120fps_mode),
> @@ -261,23 +272,39 @@ static const struct og0ve1b_mode supported_modes[] = {
> },
> };
>
> +static const struct og0ve1b_sensor_data og0ve1b_data = {
> + .chip_id = OG0VE1B_CHIP_ID,
> + .mclk_freq = OG0VE1B_MCLK_FREQ_24MHZ,
> + .test_pattern_reg = OG0VE1B_REG_PRE_ISP,
> + .link_freq_menu = og0ve1b_link_freq_menu,
> + .num_link_freqs = ARRAY_SIZE(og0ve1b_link_freq_menu),
> + .modes = supported_modes,
> + .num_modes = ARRAY_SIZE(supported_modes),
Please use tab symbols or spaces before '=' sign consistently on all lines.
> +};
> +
> static int og0ve1b_enable_test_pattern(struct og0ve1b *og0ve1b, u32 pattern)
> {
> - u64 val = og0ve1b->pre_isp;
> + u32 reg = og0ve1b->sensor->test_pattern_reg;
> + u64 val;
> + int ret;
> +
> + ret = cci_read(og0ve1b->regmap, reg, &val, NULL);
> + if (ret)
> + return ret;
>
> if (pattern)
> val |= OG0VE1B_TEST_PATTERN_ENABLE;
> else
> val &= ~OG0VE1B_TEST_PATTERN_ENABLE;
>
> - return cci_write(og0ve1b->regmap, OG0VE1B_REG_PRE_ISP, val, NULL);
> + return cci_write(og0ve1b->regmap, reg, val, NULL);
> }
So far let's keep the function above unmodified, but call it by pointer stored
in the new struct.
Does OG0VA have also just one "Vertical Colour Bars" test pattern mode?
Let me test this v2 for test pattern regression to formally confirm it shortly.
>
> static int og0ve1b_set_ctrl(struct v4l2_ctrl *ctrl)
> {
> struct og0ve1b *og0ve1b = container_of(ctrl->handler, struct og0ve1b,
> ctrl_handler);
> - const struct og0ve1b_mode *mode = &supported_modes[0];
> + const struct og0ve1b_mode *mode = &og0ve1b->sensor->modes[0];
> s64 exposure_max;
> int ret;
>
> @@ -333,7 +360,8 @@ static const struct v4l2_ctrl_ops og0ve1b_ctrl_ops = {
> static int og0ve1b_init_controls(struct og0ve1b *og0ve1b)
> {
> struct v4l2_ctrl_handler *ctrl_hdlr = &og0ve1b->ctrl_handler;
> - const struct og0ve1b_mode *mode = &supported_modes[0];
> + const struct og0ve1b_mode *mode = &og0ve1b->sensor->modes[0];
> + const struct og0ve1b_sensor_data *sensor = og0ve1b->sensor;
> s64 exposure_max, pixel_rate, h_blank, v_blank;
> struct v4l2_fwnode_device_properties props;
> struct v4l2_ctrl *ctrl;
> @@ -343,12 +371,12 @@ static int og0ve1b_init_controls(struct og0ve1b *og0ve1b)
>
> ctrl = v4l2_ctrl_new_int_menu(ctrl_hdlr, &og0ve1b_ctrl_ops,
> V4L2_CID_LINK_FREQ,
> - ARRAY_SIZE(og0ve1b_link_freq_menu) - 1,
> - 0, og0ve1b_link_freq_menu);
> + sensor->num_link_freqs - 1,
> + 0, sensor->link_freq_menu);
> if (ctrl)
> ctrl->flags |= V4L2_CTRL_FLAG_READ_ONLY;
>
> - pixel_rate = og0ve1b_link_freq_menu[0] / mode->bpp;
> + pixel_rate = sensor->link_freq_menu[0] / mode->bpp;
Since it becomes more complex, can you please move the calculation to
a new inline function, like os05b10_pixel_rate()?
> v4l2_ctrl_new_std(ctrl_hdlr, &og0ve1b_ctrl_ops, V4L2_CID_PIXEL_RATE,
> 0, pixel_rate, 1, pixel_rate);
>
> @@ -407,7 +435,7 @@ static int og0ve1b_init_controls(struct og0ve1b *og0ve1b)
> static void og0ve1b_update_pad_format(const struct og0ve1b_mode *mode,
> struct v4l2_mbus_framefmt *fmt)
> {
> - fmt->code = MEDIA_BUS_FMT_Y8_1X8;
> + fmt->code = mode->code;
> fmt->width = mode->width;
> fmt->height = mode->height;
> fmt->field = V4L2_FIELD_NONE;
> @@ -421,8 +449,8 @@ static int og0ve1b_enable_streams(struct v4l2_subdev *sd,
> struct v4l2_subdev_state *state, u32 pad,
> u64 streams_mask)
> {
> - const struct og0ve1b_reg_list *reg_list = &supported_modes[0].reg_list;
> struct og0ve1b *og0ve1b = to_og0ve1b(sd);
> + const struct og0ve1b_reg_list *reg_list = &og0ve1b->sensor->modes[0].reg_list;
> int ret;
>
> ret = pm_runtime_resume_and_get(og0ve1b->dev);
> @@ -484,13 +512,14 @@ static int og0ve1b_set_pad_format(struct v4l2_subdev *sd,
> struct v4l2_subdev_state *state,
> struct v4l2_subdev_format *fmt)
> {
> + struct og0ve1b *og0ve1b = to_og0ve1b(sd);
> struct v4l2_mbus_framefmt *format;
> const struct og0ve1b_mode *mode;
>
> format = v4l2_subdev_state_get_format(state, 0);
>
> - mode = v4l2_find_nearest_size(supported_modes,
> - ARRAY_SIZE(supported_modes),
> + mode = v4l2_find_nearest_size(og0ve1b->sensor->modes,
> + og0ve1b->sensor->num_modes,
> width, height,
> fmt->format.width,
> fmt->format.height);
> @@ -505,10 +534,12 @@ static int og0ve1b_enum_mbus_code(struct v4l2_subdev *sd,
> struct v4l2_subdev_state *sd_state,
> struct v4l2_subdev_mbus_code_enum *code)
> {
> + struct og0ve1b *og0ve1b = to_og0ve1b(sd);
> +
> if (code->index > 0)
> return -EINVAL;
>
> - code->code = MEDIA_BUS_FMT_Y8_1X8;
> + code->code = og0ve1b->sensor->modes[0].code;
>
> return 0;
> }
> @@ -517,15 +548,18 @@ static int og0ve1b_enum_frame_size(struct v4l2_subdev *sd,
> struct v4l2_subdev_state *sd_state,
> struct v4l2_subdev_frame_size_enum *fse)
> {
> - if (fse->index >= ARRAY_SIZE(supported_modes))
> + struct og0ve1b *og0ve1b = to_og0ve1b(sd);
> + const struct og0ve1b_sensor_data *sensor = og0ve1b->sensor;
> +
> + if (fse->index >= sensor->num_modes)
> return -EINVAL;
>
> - if (fse->code != MEDIA_BUS_FMT_Y8_1X8)
> + if (fse->code != sensor->modes[fse->index].code)
> return -EINVAL;
>
> - fse->min_width = supported_modes[fse->index].width;
> + fse->min_width = sensor->modes[fse->index].width;
> fse->max_width = fse->min_width;
> - fse->min_height = supported_modes[fse->index].height;
> + fse->min_height = sensor->modes[fse->index].height;
> fse->max_height = fse->min_height;
>
> return 0;
> @@ -534,13 +568,14 @@ static int og0ve1b_enum_frame_size(struct v4l2_subdev *sd,
> static int og0ve1b_init_state(struct v4l2_subdev *sd,
> struct v4l2_subdev_state *state)
> {
> + struct og0ve1b *og0ve1b = to_og0ve1b(sd);
struct og0ve1b_sensor_data *data = to_og0ve1b(sd)->sensor; // ->data anticipated
> struct v4l2_subdev_format fmt = {
> .which = V4L2_SUBDEV_FORMAT_TRY,
> .pad = 0,
> .format = {
> - .code = MEDIA_BUS_FMT_Y8_1X8,
> - .width = supported_modes[0].width,
> - .height = supported_modes[0].height,
> + .code = og0ve1b->sensor->modes[0].code,
> + .width = og0ve1b->sensor->modes[0].width,
> + .height = og0ve1b->sensor->modes[0].height,
> },
> };
>
> @@ -586,18 +621,13 @@ static int og0ve1b_identify_sensor(struct og0ve1b *og0ve1b)
> return ret;
> }
>
> - if (val != OG0VE1B_CHIP_ID) {
> - dev_err(og0ve1b->dev, "chip id mismatch: %x!=%llx\n",
> - OG0VE1B_CHIP_ID, val);
> + if (val != og0ve1b->sensor->chip_id) {
> + dev_err(og0ve1b->dev, "chip id mismatch: %llx!=%llx\n",
> + og0ve1b->sensor->chip_id, val);
> return -ENODEV;
> }
>
> - ret = cci_read(og0ve1b->regmap, OG0VE1B_REG_PRE_ISP,
> - &og0ve1b->pre_isp, NULL);
> - if (ret)
> - dev_err(og0ve1b->dev, "failed to read pre_isp: %d\n", ret);
> -
> - return ret;
> + return 0;
> }
>
> static int og0ve1b_check_hwcfg(struct og0ve1b *og0ve1b)
> @@ -624,8 +654,8 @@ static int og0ve1b_check_hwcfg(struct og0ve1b *og0ve1b)
> ret = v4l2_link_freq_to_bitmap(og0ve1b->dev,
> bus_cfg.link_frequencies,
> bus_cfg.nr_of_link_frequencies,
> - og0ve1b_link_freq_menu,
> - ARRAY_SIZE(og0ve1b_link_freq_menu),
> + og0ve1b->sensor->link_freq_menu,
> + og0ve1b->sensor->num_link_freqs,
> &freq_bitmap);
>
> v4l2_fwnode_endpoint_free(&bus_cfg);
> @@ -686,6 +716,9 @@ static int og0ve1b_probe(struct i2c_client *client)
> return -ENOMEM;
>
> og0ve1b->dev = &client->dev;
> + og0ve1b->sensor = i2c_get_match_data(client);
> + if (!og0ve1b->sensor)
> + return -ENODEV;
>
> v4l2_i2c_subdev_init(&og0ve1b->sd, client, &og0ve1b_subdev_ops);
>
> @@ -700,7 +733,7 @@ static int og0ve1b_probe(struct i2c_client *client)
> "failed to get XVCLK clock\n");
>
> freq = clk_get_rate(og0ve1b->xvclk);
> - if (freq && freq != OG0VE1B_MCLK_FREQ_24MHZ)
> + if (freq && freq != og0ve1b->sensor->mclk_freq)
> return dev_err_probe(og0ve1b->dev, -EINVAL,
> "XVCLK clock frequency %lu is not supported\n",
> freq);
> @@ -819,7 +852,7 @@ static const struct dev_pm_ops og0ve1b_pm_ops = {
> };
>
> static const struct of_device_id og0ve1b_of_match[] = {
> - { .compatible = "ovti,og0ve1b" },
> + { .compatible = "ovti,og0ve1b", .data = &og0ve1b_data },
> { /* sentinel */ }
> };
> MODULE_DEVICE_TABLE(of, og0ve1b_of_match);
>
Looks good overall, thank you.
--
Best wishes,
Vladimir
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 3/3] media: i2c: og0ve1b: Add support for OmniVision OG0VA1B
2026-07-02 10:52 ` [PATCH v2 3/3] media: i2c: og0ve1b: Add support for OmniVision OG0VA1B Wenmeng Liu
@ 2026-07-02 13:16 ` Vladimir Zapolskiy
0 siblings, 0 replies; 10+ messages in thread
From: Vladimir Zapolskiy @ 2026-07-02 13:16 UTC (permalink / raw)
To: Wenmeng Liu, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Sakari Ailus
Cc: linux-media, devicetree, linux-kernel
On 7/2/26 13:52, Wenmeng Liu wrote:
> The OmniVision OG0VA1B is a monochrome image sensor closely related to
> the OG0VE1B. It shares the SCCB control interface, power supplies and
> the single-lane MIPI D-PHY description, and differs in its chip id, the
> test pattern register, the register programming and the output format
> (10-bit RAW instead of 8-bit).
>
> Add an og0ve1b_sensor_data entry describing the OG0VA1B together with
> its 640x480 60fps register sequence.
>
> Signed-off-by: Wenmeng Liu <wenmeng.liu@oss.qualcomm.com>
> ---
> drivers/media/i2c/og0ve1b.c | 234 +++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 230 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/i2c/og0ve1b.c b/drivers/media/i2c/og0ve1b.c
> index acc06b10bf896f734926289099a70fbc2bb628d5..f1f4fcd195c4e2dd69f78ba3f01f768a052e6c5d 100644
> --- a/drivers/media/i2c/og0ve1b.c
> +++ b/drivers/media/i2c/og0ve1b.c
> @@ -17,8 +17,12 @@
> #define OG0VE1B_LINK_FREQ_500MHZ (500 * HZ_PER_MHZ)
> #define OG0VE1B_MCLK_FREQ_24MHZ (24 * HZ_PER_MHZ)
>
> +#define OG0VA1B_LINK_FREQ_480MHZ (480 * HZ_PER_MHZ)
> +#define OG0VA1B_MCLK_FREQ_19_2MHZ (19200 * HZ_PER_KHZ)
> +
> #define OG0VE1B_REG_CHIP_ID CCI_REG24(0x300a)
> #define OG0VE1B_CHIP_ID 0xc75645
> +#define OG0VA1B_CHIP_ID 0xc75641
Please keep the list alphabetically sorted, also it's fine if
"OG0VE1B_REG_CHIP_ID" is reduced to "OG0V_REG_CHIP_ID".
>
> #define OG0VE1B_REG_MODE_SELECT CCI_REG8(0x0100)
> #define OG0VE1B_MODE_STANDBY 0x00
> @@ -45,8 +49,9 @@
> #define OG0VE1B_REG_VTS CCI_REG16(0x380e)
> #define OG0VE1B_VTS_MAX 0xffff
>
> -/* Test pattern */
> +/* Test pattern - OG0VE1B uses 0x5e00, OG0VA1B uses 0x5100 */
> #define OG0VE1B_REG_PRE_ISP CCI_REG8(0x5e00)
> +#define OG0VA1B_REG_TEST_PATTERN CCI_REG8(0x5100)
> #define OG0VE1B_TEST_PATTERN_ENABLE BIT(7)
>
> #define to_og0ve1b(_sd) container_of(_sd, struct og0ve1b, sd)
> @@ -55,6 +60,10 @@ static const s64 og0ve1b_link_freq_menu[] = {
> OG0VE1B_LINK_FREQ_500MHZ,
> };
>
> +static const s64 og0va1b_link_freq_menu[] = {
> + OG0VA1B_LINK_FREQ_480MHZ,
> +};
> +
> struct og0ve1b_reg_list {
> const struct cci_reg_sequence *regs;
> unsigned int num_regs;
> @@ -72,9 +81,14 @@ struct og0ve1b_mode {
> };
>
> struct og0ve1b_sensor_data {
> + const char *name;
> u64 chip_id;
> unsigned long mclk_freq;
> u32 test_pattern_reg;
> + /* Exposure register unit: OG0VE1B 1/16 line (4), OG0VA1B whole lines (0). */
> + unsigned int exposure_shift;
> + /* Pixel rate multiplier: OG0VA1B uses CSI-2 DDR (2), OG0VE1B keeps 1. */
> + unsigned int pixel_rate_mul;
> const s64 *link_freq_menu;
> int num_link_freqs;
> const struct og0ve1b_mode *modes;
> @@ -272,16 +286,223 @@ static const struct og0ve1b_mode supported_modes[] = {
> },
> };
>
> +static const struct cci_reg_sequence og0va1b_640x480_60fps_mode[] = {
> + { CCI_REG8(0x0302), 0x31 },
> + { CCI_REG8(0x0303), 0x02 },
> + { CCI_REG8(0x0304), 0x01 },
> + { CCI_REG8(0x0305), 0x90 },
> + { CCI_REG8(0x0306), 0x00 },
> + { CCI_REG8(0x0323), 0x02 },
> + { CCI_REG8(0x0325), 0x68 },
> + { CCI_REG8(0x0326), 0xd8 },
> + { CCI_REG8(0x3006), 0x0e },
> + { CCI_REG8(0x300d), 0x08 },
> + { CCI_REG8(0x3018), 0xf0 },
> + { CCI_REG8(0x301c), 0xf0 },
> + { CCI_REG8(0x3020), 0x20 },
> + { CCI_REG8(0x3040), 0x0f },
> + { CCI_REG8(0x3022), 0x01 },
> + { CCI_REG8(0x3107), 0x40 },
> + { CCI_REG8(0x3216), 0x01 },
> + { CCI_REG8(0x3217), 0x00 },
> + { CCI_REG8(0x3218), 0xc0 },
> + { CCI_REG8(0x3219), 0x55 },
> + { CCI_REG8(0x3506), 0x01 },
> + { CCI_REG8(0x3507), 0x50 },
> + { CCI_REG8(0x3508), 0x01 },
> + { CCI_REG8(0x3509), 0x00 },
> + { CCI_REG8(0x350a), 0x01 },
> + { CCI_REG8(0x350b), 0x00 },
> + { CCI_REG8(0x350c), 0x00 },
> + { CCI_REG8(0x3541), 0x00 },
> + { CCI_REG8(0x3542), 0x40 },
> + { CCI_REG8(0x3605), 0x90 },
> + { CCI_REG8(0x3606), 0x41 },
> + { CCI_REG8(0x3612), 0x00 },
> + { CCI_REG8(0x3620), 0x08 },
> + { CCI_REG8(0x3630), 0x17 },
> + { CCI_REG8(0x3631), 0x99 },
> + { CCI_REG8(0x3639), 0x88 },
> + { CCI_REG8(0x3668), 0x00 },
> + { CCI_REG8(0x3674), 0x00 },
> + { CCI_REG8(0x3677), 0x3f },
> + { CCI_REG8(0x368f), 0x06 },
> + { CCI_REG8(0x36a2), 0x19 },
> + { CCI_REG8(0x36a4), 0xf1 },
> + { CCI_REG8(0x36a5), 0x2d },
> + { CCI_REG8(0x3706), 0x30 },
> + { CCI_REG8(0x370d), 0x72 },
> + { CCI_REG8(0x3713), 0x86 },
> + { CCI_REG8(0x3715), 0x03 },
> + { CCI_REG8(0x3716), 0x00 },
> + { CCI_REG8(0x376d), 0x24 },
> + { CCI_REG8(0x3770), 0x3a },
> + { CCI_REG8(0x3778), 0x00 },
> + { CCI_REG8(0x37a8), 0x03 },
> + { CCI_REG8(0x37a9), 0x00 },
> + { CCI_REG8(0x37df), 0x7d },
> + { CCI_REG8(0x3800), 0x00 },
> + { CCI_REG8(0x3801), 0x00 },
> + { CCI_REG8(0x3802), 0x00 },
> + { CCI_REG8(0x3803), 0x00 },
> + { CCI_REG8(0x3804), 0x02 },
> + { CCI_REG8(0x3805), 0x8f },
> + { CCI_REG8(0x3806), 0x01 },
> + { CCI_REG8(0x3807), 0xef },
> + { CCI_REG8(0x3808), 0x02 },
> + { CCI_REG8(0x3809), 0x80 },
> + { CCI_REG8(0x380a), 0x01 },
> + { CCI_REG8(0x380b), 0xe0 },
> + { CCI_REG8(0x380c), 0x01 },
> + { CCI_REG8(0x380d), 0x78 },
> + { CCI_REG8(0x380e), 0x08 },
> + { CCI_REG8(0x380f), 0x30 },
> + { CCI_REG8(0x3810), 0x00 },
> + { CCI_REG8(0x3811), 0x08 },
> + { CCI_REG8(0x3812), 0x00 },
> + { CCI_REG8(0x3813), 0x08 },
> + { CCI_REG8(0x3814), 0x11 },
> + { CCI_REG8(0x3815), 0x11 },
> + { CCI_REG8(0x3816), 0x00 },
> + { CCI_REG8(0x3817), 0x01 },
> + { CCI_REG8(0x3818), 0x00 },
> + { CCI_REG8(0x3819), 0x05 },
> + { CCI_REG8(0x3820), 0x40 },
> + { CCI_REG8(0x3821), 0x04 },
> + { CCI_REG8(0x3823), 0x00 },
> + { CCI_REG8(0x3826), 0x00 },
> + { CCI_REG8(0x3827), 0x00 },
> + { CCI_REG8(0x382b), 0x52 },
> + { CCI_REG8(0x384a), 0xa2 },
> + { CCI_REG8(0x3858), 0x00 },
> + { CCI_REG8(0x3859), 0x00 },
> + { CCI_REG8(0x3860), 0x00 },
> + { CCI_REG8(0x3861), 0x00 },
> + { CCI_REG8(0x3866), 0x0c },
> + { CCI_REG8(0x3867), 0x07 },
> + { CCI_REG8(0x3884), 0x00 },
> + { CCI_REG8(0x3885), 0x08 },
> + { CCI_REG8(0x3888), 0x50 },
> + { CCI_REG8(0x3893), 0x6c },
> + { CCI_REG8(0x3898), 0x00 },
> + { CCI_REG8(0x389a), 0x04 },
> + { CCI_REG8(0x389b), 0x01 },
> + { CCI_REG8(0x389c), 0x0b },
> + { CCI_REG8(0x389d), 0xdc },
> + { CCI_REG8(0x38b1), 0x04 },
> + { CCI_REG8(0x38b2), 0x00 },
> + { CCI_REG8(0x38b3), 0x08 },
> + { CCI_REG8(0x38c1), 0x46 },
> + { CCI_REG8(0x38c9), 0x02 },
> + { CCI_REG8(0x38d4), 0x06 },
> + { CCI_REG8(0x38d5), 0x5a },
> + { CCI_REG8(0x38d6), 0x08 },
> + { CCI_REG8(0x38d7), 0x3a },
> + { CCI_REG8(0x391f), 0x00 },
> + { CCI_REG8(0x3920), 0xaa },
> + { CCI_REG8(0x3921), 0x00 },
> + { CCI_REG8(0x3922), 0x00 },
> + { CCI_REG8(0x3923), 0x00 },
> + { CCI_REG8(0x3924), 0x00 },
> + { CCI_REG8(0x3925), 0x00 },
> + { CCI_REG8(0x3926), 0x00 },
> + { CCI_REG8(0x3927), 0x00 },
> + { CCI_REG8(0x3928), 0x10 },
> + { CCI_REG8(0x3929), 0x01 },
> + { CCI_REG8(0x392a), 0xb4 },
> + { CCI_REG8(0x392b), 0x00 },
> + { CCI_REG8(0x392c), 0x10 },
> + { CCI_REG8(0x392d), 0x01 },
> + { CCI_REG8(0x392e), 0x78 },
> + { CCI_REG8(0x392f), 0x4a },
> + { CCI_REG8(0x391e), 0x01 },
> + { CCI_REG8(0x389f), 0x08 },
> + { CCI_REG8(0x38a0), 0x00 },
> + { CCI_REG8(0x38a1), 0x00 },
> + { CCI_REG8(0x3a06), 0x06 },
> + { CCI_REG8(0x3a07), 0x78 },
> + { CCI_REG8(0x3a08), 0x08 },
> + { CCI_REG8(0x3a09), 0x80 },
> + { CCI_REG8(0x3a52), 0x00 },
> + { CCI_REG8(0x3a53), 0x01 },
> + { CCI_REG8(0x3a54), 0x0c },
> + { CCI_REG8(0x3a55), 0x04 },
> + { CCI_REG8(0x3a58), 0x0c },
> + { CCI_REG8(0x3a59), 0x04 },
> + { CCI_REG8(0x4000), 0xcf },
> + { CCI_REG8(0x4003), 0x40 },
> + { CCI_REG8(0x4008), 0x04 },
> + { CCI_REG8(0x4009), 0x13 },
> + { CCI_REG8(0x400a), 0x02 },
> + { CCI_REG8(0x400b), 0x34 },
> + { CCI_REG8(0x4010), 0x71 },
> + { CCI_REG8(0x4042), 0xc3 },
> + { CCI_REG8(0x4306), 0x04 },
> + { CCI_REG8(0x4307), 0x12 },
> + { CCI_REG8(0x4500), 0x70 },
> + { CCI_REG8(0x4509), 0x00 },
> + { CCI_REG8(0x450b), 0x83 },
> + { CCI_REG8(0x4604), 0x68 },
> + { CCI_REG8(0x481b), 0x44 },
> + { CCI_REG8(0x481f), 0x30 },
> + { CCI_REG8(0x4823), 0x44 },
> + { CCI_REG8(0x4825), 0x35 },
> + { CCI_REG8(0x4837), 0x11 },
> + { CCI_REG8(0x4f00), 0x04 },
> + { CCI_REG8(0x4f10), 0x04 },
> + { CCI_REG8(0x4f21), 0x01 },
> + { CCI_REG8(0x4f22), 0x00 },
> + { CCI_REG8(0x4f23), 0x54 },
> + { CCI_REG8(0x4f24), 0x51 },
> + { CCI_REG8(0x4f25), 0x41 },
> + { CCI_REG8(0x5000), 0x3f },
> + { CCI_REG8(0x5001), 0x80 },
> + { CCI_REG8(0x500a), 0x00 },
> + { CCI_REG8(0x5100), 0x00 },
> + { CCI_REG8(0x5111), 0x20 },
> +};
> +
> +static const struct og0ve1b_mode og0va1b_supported_modes[] = {
> + {
> + .width = 640,
> + .height = 480,
> + .hts = 752,
> + .vts = 2096,
> + .bpp = 10,
> + .code = MEDIA_BUS_FMT_Y10_1X10,
> + .reg_list = {
> + .regs = og0va1b_640x480_60fps_mode,
> + .num_regs = ARRAY_SIZE(og0va1b_640x480_60fps_mode),
> + },
> + },
> +};
> +
> static const struct og0ve1b_sensor_data og0ve1b_data = {
> + .name = "og0ve1b",
> .chip_id = OG0VE1B_CHIP_ID,
> .mclk_freq = OG0VE1B_MCLK_FREQ_24MHZ,
> .test_pattern_reg = OG0VE1B_REG_PRE_ISP,
> + .exposure_shift = 4,
> + .pixel_rate_mul = 1,
> .link_freq_menu = og0ve1b_link_freq_menu,
> .num_link_freqs = ARRAY_SIZE(og0ve1b_link_freq_menu),
> .modes = supported_modes,
> .num_modes = ARRAY_SIZE(supported_modes),
Can you please rename "supported_modes" to "og0ve1b_supported_modes"?
Likely it should be done in 2/3 change, but it's up to you.
> };
>
> +static const struct og0ve1b_sensor_data og0va1b_data = {
> + .name = "og0va1b",
> + .chip_id = OG0VA1B_CHIP_ID,
> + .mclk_freq = OG0VA1B_MCLK_FREQ_19_2MHZ,
> + .test_pattern_reg = OG0VA1B_REG_TEST_PATTERN,
> + .exposure_shift = 0,
> + .pixel_rate_mul = 2,
> + .link_freq_menu = og0va1b_link_freq_menu,
> + .num_link_freqs = ARRAY_SIZE(og0va1b_link_freq_menu),
> + .modes = og0va1b_supported_modes,
> + .num_modes = ARRAY_SIZE(og0va1b_supported_modes),
> +};
First og0va1b_data, then og0ve1b_data declaration to keep the natural order.
> +
> static int og0ve1b_enable_test_pattern(struct og0ve1b *og0ve1b, u32 pattern)
> {
> u32 reg = og0ve1b->sensor->test_pattern_reg;
> @@ -334,7 +555,8 @@ static int og0ve1b_set_ctrl(struct v4l2_ctrl *ctrl)
> break;
> case V4L2_CID_EXPOSURE:
> ret = cci_write(og0ve1b->regmap, OG0VE1B_REG_EXPOSURE,
> - ctrl->val << 4, NULL);
> + ctrl->val << og0ve1b->sensor->exposure_shift,
> + NULL);
> break;
> case V4L2_CID_VBLANK:
> ret = cci_write(og0ve1b->regmap, OG0VE1B_REG_VTS,
> @@ -376,7 +598,8 @@ static int og0ve1b_init_controls(struct og0ve1b *og0ve1b)
> if (ctrl)
> ctrl->flags |= V4L2_CTRL_FLAG_READ_ONLY;
>
> - pixel_rate = sensor->link_freq_menu[0] / mode->bpp;
> + pixel_rate = sensor->link_freq_menu[0] * sensor->pixel_rate_mul /
> + mode->bpp;
> v4l2_ctrl_new_std(ctrl_hdlr, &og0ve1b_ctrl_ops, V4L2_CID_PIXEL_RATE,
> 0, pixel_rate, 1, pixel_rate);
>
> @@ -721,6 +944,8 @@ static int og0ve1b_probe(struct i2c_client *client)
> return -ENODEV;
>
> v4l2_i2c_subdev_init(&og0ve1b->sd, client, &og0ve1b_subdev_ops);
> + v4l2_i2c_subdev_set_name(&og0ve1b->sd, client,
> + og0ve1b->sensor->name, NULL);
>
> og0ve1b->regmap = devm_cci_regmap_init_i2c(client, 16);
> if (IS_ERR(og0ve1b->regmap))
> @@ -853,6 +1078,7 @@ static const struct dev_pm_ops og0ve1b_pm_ops = {
>
> static const struct of_device_id og0ve1b_of_match[] = {
> { .compatible = "ovti,og0ve1b", .data = &og0ve1b_data },
> + { .compatible = "ovti,og0va1b", .data = &og0va1b_data },
> { /* sentinel */ }
> };
> MODULE_DEVICE_TABLE(of, og0ve1b_of_match);
> @@ -870,5 +1096,5 @@ static struct i2c_driver og0ve1b_i2c_driver = {
> module_i2c_driver(og0ve1b_i2c_driver);
>
> MODULE_AUTHOR("Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>");
> -MODULE_DESCRIPTION("OmniVision OG0VE1B sensor driver");
> +MODULE_DESCRIPTION("OmniVision OG0VE1B/OG0VA1B sensor driver");
> MODULE_LICENSE("GPL");
>
Looks good, thank you!
--
Best wishes,
Vladimir
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/3] media: i2c: og0ve1b: Introduce per-sensor data structure
2026-07-02 10:52 ` [PATCH v2 2/3] media: i2c: og0ve1b: Introduce per-sensor data structure Wenmeng Liu
2026-07-02 11:04 ` sashiko-bot
2026-07-02 13:10 ` Vladimir Zapolskiy
@ 2026-07-02 16:31 ` Bryan O'Donoghue
2 siblings, 0 replies; 10+ messages in thread
From: Bryan O'Donoghue @ 2026-07-02 16:31 UTC (permalink / raw)
To: Wenmeng Liu, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Sakari Ailus,
Vladimir Zapolskiy
Cc: linux-media, devicetree, linux-kernel
On 02/07/2026 11:52, Wenmeng Liu wrote:
> In preparation for supporting further OmniVision sensors that share most
> of this driver, move the sensor-specific parameters (chip id, MCLK
> frequency, test pattern register, link frequency menu and the list of
> supported modes) into a new struct og0ve1b_sensor_data, selected through
> i2c_get_match_data() at probe time.
>
> Signed-off-by: Wenmeng Liu <wenmeng.liu@oss.qualcomm.com>
> ---
> drivers/media/i2c/og0ve1b.c | 101 +++++++++++++++++++++++++++++---------------
> 1 file changed, 67 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/media/i2c/og0ve1b.c b/drivers/media/i2c/og0ve1b.c
> index 84a28cdcade10f8fbcf945999e88f84641b9bc0d..acc06b10bf896f734926289099a70fbc2bb628d5 100644
> --- a/drivers/media/i2c/og0ve1b.c
> +++ b/drivers/media/i2c/og0ve1b.c
> @@ -66,10 +66,21 @@ struct og0ve1b_mode {
> u32 hts; /* Horizontal timing size */
> u32 vts; /* Default vertical timing size */
> u32 bpp; /* Bits per pixel */
> + u32 code; /* MEDIA_BUS_FMT code */
>
> const struct og0ve1b_reg_list reg_list; /* Sensor register setting */
> };
>
> +struct og0ve1b_sensor_data {
> + u64 chip_id;
> + unsigned long mclk_freq;
> + u32 test_pattern_reg;
> + const s64 *link_freq_menu;
> + int num_link_freqs;
> + const struct og0ve1b_mode *modes;
> + int num_modes;
> +};
> +
> static const char * const og0ve1b_test_pattern_menu[] = {
> "Disabled",
> "Vertical Colour Bars",
> @@ -97,8 +108,7 @@ struct og0ve1b {
> struct v4l2_ctrl *exposure;
> struct v4l2_ctrl_handler ctrl_handler;
>
> - /* Saved register value */
> - u64 pre_isp;
> + const struct og0ve1b_sensor_data *sensor;
> };
>
> static const struct cci_reg_sequence og0ve1b_640x480_120fps_mode[] = {
> @@ -254,6 +264,7 @@ static const struct og0ve1b_mode supported_modes[] = {
> .hts = 792,
> .vts = 568,
> .bpp = 8,
> + .code = MEDIA_BUS_FMT_Y8_1X8,
> .reg_list = {
> .regs = og0ve1b_640x480_120fps_mode,
> .num_regs = ARRAY_SIZE(og0ve1b_640x480_120fps_mode),
> @@ -261,23 +272,39 @@ static const struct og0ve1b_mode supported_modes[] = {
> },
> };
>
> +static const struct og0ve1b_sensor_data og0ve1b_data = {
> + .chip_id = OG0VE1B_CHIP_ID,
> + .mclk_freq = OG0VE1B_MCLK_FREQ_24MHZ,
> + .test_pattern_reg = OG0VE1B_REG_PRE_ISP,
> + .link_freq_menu = og0ve1b_link_freq_menu,
> + .num_link_freqs = ARRAY_SIZE(og0ve1b_link_freq_menu),
> + .modes = supported_modes,
> + .num_modes = ARRAY_SIZE(supported_modes),
> +};
> +
> static int og0ve1b_enable_test_pattern(struct og0ve1b *og0ve1b, u32 pattern)
> {
> - u64 val = og0ve1b->pre_isp;
> + u32 reg = og0ve1b->sensor->test_pattern_reg;
> + u64 val;
> + int ret;
> +
> + ret = cci_read(og0ve1b->regmap, reg, &val, NULL);
> + if (ret)
> + return ret;
>
> if (pattern)
> val |= OG0VE1B_TEST_PATTERN_ENABLE;
> else
> val &= ~OG0VE1B_TEST_PATTERN_ENABLE;
>
> - return cci_write(og0ve1b->regmap, OG0VE1B_REG_PRE_ISP, val, NULL);
> + return cci_write(og0ve1b->regmap, reg, val, NULL);
> }
>
> static int og0ve1b_set_ctrl(struct v4l2_ctrl *ctrl)
> {
> struct og0ve1b *og0ve1b = container_of(ctrl->handler, struct og0ve1b,
> ctrl_handler);
> - const struct og0ve1b_mode *mode = &supported_modes[0];
> + const struct og0ve1b_mode *mode = &og0ve1b->sensor->modes[0];
> s64 exposure_max;
> int ret;
>
> @@ -333,7 +360,8 @@ static const struct v4l2_ctrl_ops og0ve1b_ctrl_ops = {
> static int og0ve1b_init_controls(struct og0ve1b *og0ve1b)
> {
> struct v4l2_ctrl_handler *ctrl_hdlr = &og0ve1b->ctrl_handler;
> - const struct og0ve1b_mode *mode = &supported_modes[0];
> + const struct og0ve1b_mode *mode = &og0ve1b->sensor->modes[0];
> + const struct og0ve1b_sensor_data *sensor = og0ve1b->sensor;
> s64 exposure_max, pixel_rate, h_blank, v_blank;
> struct v4l2_fwnode_device_properties props;
> struct v4l2_ctrl *ctrl;
> @@ -343,12 +371,12 @@ static int og0ve1b_init_controls(struct og0ve1b *og0ve1b)
>
> ctrl = v4l2_ctrl_new_int_menu(ctrl_hdlr, &og0ve1b_ctrl_ops,
> V4L2_CID_LINK_FREQ,
> - ARRAY_SIZE(og0ve1b_link_freq_menu) - 1,
> - 0, og0ve1b_link_freq_menu);
> + sensor->num_link_freqs - 1,
> + 0, sensor->link_freq_menu);
> if (ctrl)
> ctrl->flags |= V4L2_CTRL_FLAG_READ_ONLY;
>
> - pixel_rate = og0ve1b_link_freq_menu[0] / mode->bpp;
> + pixel_rate = sensor->link_freq_menu[0] / mode->bpp;
> v4l2_ctrl_new_std(ctrl_hdlr, &og0ve1b_ctrl_ops, V4L2_CID_PIXEL_RATE,
> 0, pixel_rate, 1, pixel_rate);
>
> @@ -407,7 +435,7 @@ static int og0ve1b_init_controls(struct og0ve1b *og0ve1b)
> static void og0ve1b_update_pad_format(const struct og0ve1b_mode *mode,
> struct v4l2_mbus_framefmt *fmt)
> {
> - fmt->code = MEDIA_BUS_FMT_Y8_1X8;
> + fmt->code = mode->code;
> fmt->width = mode->width;
> fmt->height = mode->height;
> fmt->field = V4L2_FIELD_NONE;
> @@ -421,8 +449,8 @@ static int og0ve1b_enable_streams(struct v4l2_subdev *sd,
> struct v4l2_subdev_state *state, u32 pad,
> u64 streams_mask)
> {
> - const struct og0ve1b_reg_list *reg_list = &supported_modes[0].reg_list;
> struct og0ve1b *og0ve1b = to_og0ve1b(sd);
> + const struct og0ve1b_reg_list *reg_list = &og0ve1b->sensor->modes[0].reg_list;
> int ret;
>
> ret = pm_runtime_resume_and_get(og0ve1b->dev);
> @@ -484,13 +512,14 @@ static int og0ve1b_set_pad_format(struct v4l2_subdev *sd,
> struct v4l2_subdev_state *state,
> struct v4l2_subdev_format *fmt)
> {
> + struct og0ve1b *og0ve1b = to_og0ve1b(sd);
> struct v4l2_mbus_framefmt *format;
> const struct og0ve1b_mode *mode;
>
> format = v4l2_subdev_state_get_format(state, 0);
>
> - mode = v4l2_find_nearest_size(supported_modes,
> - ARRAY_SIZE(supported_modes),
> + mode = v4l2_find_nearest_size(og0ve1b->sensor->modes,
> + og0ve1b->sensor->num_modes,
> width, height,
> fmt->format.width,
> fmt->format.height);
> @@ -505,10 +534,12 @@ static int og0ve1b_enum_mbus_code(struct v4l2_subdev *sd,
> struct v4l2_subdev_state *sd_state,
> struct v4l2_subdev_mbus_code_enum *code)
> {
> + struct og0ve1b *og0ve1b = to_og0ve1b(sd);
> +
> if (code->index > 0)
> return -EINVAL;
>
> - code->code = MEDIA_BUS_FMT_Y8_1X8;
> + code->code = og0ve1b->sensor->modes[0].code;
>
> return 0;
> }
> @@ -517,15 +548,18 @@ static int og0ve1b_enum_frame_size(struct v4l2_subdev *sd,
> struct v4l2_subdev_state *sd_state,
> struct v4l2_subdev_frame_size_enum *fse)
> {
> - if (fse->index >= ARRAY_SIZE(supported_modes))
> + struct og0ve1b *og0ve1b = to_og0ve1b(sd);
> + const struct og0ve1b_sensor_data *sensor = og0ve1b->sensor;
> +
> + if (fse->index >= sensor->num_modes)
> return -EINVAL;
>
> - if (fse->code != MEDIA_BUS_FMT_Y8_1X8)
> + if (fse->code != sensor->modes[fse->index].code)
> return -EINVAL;
>
> - fse->min_width = supported_modes[fse->index].width;
> + fse->min_width = sensor->modes[fse->index].width;
> fse->max_width = fse->min_width;
> - fse->min_height = supported_modes[fse->index].height;
> + fse->min_height = sensor->modes[fse->index].height;
> fse->max_height = fse->min_height;
>
> return 0;
> @@ -534,13 +568,14 @@ static int og0ve1b_enum_frame_size(struct v4l2_subdev *sd,
> static int og0ve1b_init_state(struct v4l2_subdev *sd,
> struct v4l2_subdev_state *state)
> {
> + struct og0ve1b *og0ve1b = to_og0ve1b(sd);
> struct v4l2_subdev_format fmt = {
> .which = V4L2_SUBDEV_FORMAT_TRY,
> .pad = 0,
> .format = {
> - .code = MEDIA_BUS_FMT_Y8_1X8,
> - .width = supported_modes[0].width,
> - .height = supported_modes[0].height,
> + .code = og0ve1b->sensor->modes[0].code,
> + .width = og0ve1b->sensor->modes[0].width,
> + .height = og0ve1b->sensor->modes[0].height,
> },
> };
>
> @@ -586,18 +621,13 @@ static int og0ve1b_identify_sensor(struct og0ve1b *og0ve1b)
> return ret;
> }
>
> - if (val != OG0VE1B_CHIP_ID) {
> - dev_err(og0ve1b->dev, "chip id mismatch: %x!=%llx\n",
> - OG0VE1B_CHIP_ID, val);
> + if (val != og0ve1b->sensor->chip_id) {
> + dev_err(og0ve1b->dev, "chip id mismatch: %llx!=%llx\n",
> + og0ve1b->sensor->chip_id, val);
> return -ENODEV;
> }
>
> - ret = cci_read(og0ve1b->regmap, OG0VE1B_REG_PRE_ISP,
> - &og0ve1b->pre_isp, NULL);
> - if (ret)
> - dev_err(og0ve1b->dev, "failed to read pre_isp: %d\n", ret);
> -
> - return ret;
> + return 0;
The one thing that is not immediately clear to me is if we get
equivalent logic WRT OG0VE1B_REG_PRE_ISP after this change.
> }
>
> static int og0ve1b_check_hwcfg(struct og0ve1b *og0ve1b)
> @@ -624,8 +654,8 @@ static int og0ve1b_check_hwcfg(struct og0ve1b *og0ve1b)
> ret = v4l2_link_freq_to_bitmap(og0ve1b->dev,
> bus_cfg.link_frequencies,
> bus_cfg.nr_of_link_frequencies,
> - og0ve1b_link_freq_menu,
> - ARRAY_SIZE(og0ve1b_link_freq_menu),
> + og0ve1b->sensor->link_freq_menu,
> + og0ve1b->sensor->num_link_freqs,
> &freq_bitmap);
>
> v4l2_fwnode_endpoint_free(&bus_cfg);
> @@ -686,6 +716,9 @@ static int og0ve1b_probe(struct i2c_client *client)
> return -ENOMEM;
>
> og0ve1b->dev = &client->dev;
> + og0ve1b->sensor = i2c_get_match_data(client);
> + if (!og0ve1b->sensor)
> + return -ENODEV;
>
> v4l2_i2c_subdev_init(&og0ve1b->sd, client, &og0ve1b_subdev_ops);
>
> @@ -700,7 +733,7 @@ static int og0ve1b_probe(struct i2c_client *client)
> "failed to get XVCLK clock\n");
>
> freq = clk_get_rate(og0ve1b->xvclk);
> - if (freq && freq != OG0VE1B_MCLK_FREQ_24MHZ)
> + if (freq && freq != og0ve1b->sensor->mclk_freq)
> return dev_err_probe(og0ve1b->dev, -EINVAL,
> "XVCLK clock frequency %lu is not supported\n",
> freq);
> @@ -819,7 +852,7 @@ static const struct dev_pm_ops og0ve1b_pm_ops = {
> };
>
> static const struct of_device_id og0ve1b_of_match[] = {
> - { .compatible = "ovti,og0ve1b" },
> + { .compatible = "ovti,og0ve1b", .data = &og0ve1b_data },
> { /* sentinel */ }
> };
> MODULE_DEVICE_TABLE(of, og0ve1b_of_match);
>
> --
> 2.34.1
>
>
Other than that LTGM.
---
bod
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/3] dt-bindings: media: i2c: og0ve1b: Add OmniVision OG0VA1B camera sensor
2026-07-02 10:52 ` [PATCH v2 1/3] dt-bindings: media: i2c: og0ve1b: Add OmniVision OG0VA1B camera sensor Wenmeng Liu
2026-07-02 12:24 ` Vladimir Zapolskiy
@ 2026-07-02 19:21 ` Conor Dooley
1 sibling, 0 replies; 10+ messages in thread
From: Conor Dooley @ 2026-07-02 19:21 UTC (permalink / raw)
To: Wenmeng Liu
Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Sakari Ailus, Vladimir Zapolskiy, linux-media,
devicetree, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 75 bytes --]
Acked-by: Conor Dooley <conor.dooley@microchip.com>
pw-bot: not-applicable
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2026-07-02 19:21 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-02 10:52 [PATCH v2 0/3] media: i2c: Add OmniVision OG0VA1B camera sensor driver Wenmeng Liu
2026-07-02 10:52 ` [PATCH v2 1/3] dt-bindings: media: i2c: og0ve1b: Add OmniVision OG0VA1B camera sensor Wenmeng Liu
2026-07-02 12:24 ` Vladimir Zapolskiy
2026-07-02 19:21 ` Conor Dooley
2026-07-02 10:52 ` [PATCH v2 2/3] media: i2c: og0ve1b: Introduce per-sensor data structure Wenmeng Liu
2026-07-02 11:04 ` sashiko-bot
2026-07-02 13:10 ` Vladimir Zapolskiy
2026-07-02 16:31 ` Bryan O'Donoghue
2026-07-02 10:52 ` [PATCH v2 3/3] media: i2c: og0ve1b: Add support for OmniVision OG0VA1B Wenmeng Liu
2026-07-02 13:16 ` Vladimir Zapolskiy
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox