[PATCH 1/3] media: ov02c10: Drop duplicate register write

[PATCH 1/3] media: ov02c10: Drop duplicate register write

[Felipe Calliari]

sensor_1928x1092_30fps_setting[] writes {0x395f, 0x00} twice in a row.
The second write is redundant; drop it. No functional change.

Signed-off-by: Felipe Calliari <calliarifelipe@gmail.com>
---
 drivers/media/i2c/ov02c10.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/media/i2c/ov02c10.c b/drivers/media/i2c/ov02c10.c
index cf93d3603..6220461fd 100644
--- a/drivers/media/i2c/ov02c10.c
+++ b/drivers/media/i2c/ov02c10.c
@@ -209,7 +209,6 @@ static const struct reg_sequence sensor_1928x1092_30fps_setting[] = {
 	{0x395d, 0x05},
 	{0x395e, 0x02},
 	{0x395f, 0x00},
-	{0x395f, 0x00},
 	{0x3960, 0x00},
 	{0x3961, 0x00},
 	{0x3962, 0x00},
-- 
2.55.0

[PATCH 2/3] media: ov02c10: Implement get_selection

[Felipe Calliari]

The driver does not implement .get_selection, so userspace cannot query
the sensor's native size or active crop rectangle. libcamera reports
"Unable to get rectangle N on pad 0/0: Inappropriate ioctl for device"
and "The sensor kernel driver needs to be fixed" (see
Documentation/sensor_driver_requirements.rst).

Implement .get_selection returning the fixed geometry of the sensor:

 - V4L2_SEL_TGT_NATIVE_SIZE / V4L2_SEL_TGT_CROP_BOUNDS: the full
   1928x1092 pixel array.
 - V4L2_SEL_TGT_CROP / V4L2_SEL_TGT_CROP_DEFAULT: the 1920x1080 active
   area, offset by (4, 6), matching the readout window programmed in
   sensor_1928x1092_30fps_setting[] (registers 0x3800-0x3807).

The crop is fixed, so sd_state is not consulted.

Signed-off-by: Felipe Calliari <calliarifelipe@gmail.com>
---
 drivers/media/i2c/ov02c10.c | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/drivers/media/i2c/ov02c10.c b/drivers/media/i2c/ov02c10.c
index 6220461fd..114db38c0 100644
--- a/drivers/media/i2c/ov02c10.c
+++ b/drivers/media/i2c/ov02c10.c
@@ -10,6 +10,7 @@
 #include <linux/pm_runtime.h>
 #include <linux/regmap.h>
 #include <media/v4l2-cci.h>
+#include <media/v4l2-common.h>
 #include <media/v4l2-ctrls.h>
 #include <media/v4l2-device.h>
 #include <media/v4l2-fwnode.h>
@@ -18,6 +19,13 @@
 #define OV02C10_MCLK			19200000
 #define OV02C10_RGB_DEPTH		10
 
+#define OV02C10_NATIVE_WIDTH		1928
+#define OV02C10_NATIVE_HEIGHT		1092
+#define OV02C10_ACTIVE_WIDTH		1920
+#define OV02C10_ACTIVE_HEIGHT		1080
+#define OV02C10_ACTIVE_LEFT		4
+#define OV02C10_ACTIVE_TOP		6
+
 #define OV02C10_REG_CHIP_ID		CCI_REG16(0x300a)
 #define OV02C10_CHIP_ID			0x5602
 
@@ -767,11 +775,36 @@ static const struct v4l2_subdev_video_ops ov02c10_video_ops = {
 	.s_stream = v4l2_subdev_s_stream_helper,
 };
 
+static int ov02c10_get_selection(struct v4l2_subdev *sd,
+				 struct v4l2_subdev_state *sd_state,
+				 struct v4l2_subdev_selection *sel)
+{
+	switch (sel->target) {
+	case V4L2_SEL_TGT_NATIVE_SIZE:
+	case V4L2_SEL_TGT_CROP_BOUNDS:
+		sel->r.top = 0;
+		sel->r.left = 0;
+		sel->r.width = OV02C10_NATIVE_WIDTH;
+		sel->r.height = OV02C10_NATIVE_HEIGHT;
+		return 0;
+	case V4L2_SEL_TGT_CROP:
+	case V4L2_SEL_TGT_CROP_DEFAULT:
+		sel->r.top = OV02C10_ACTIVE_TOP;
+		sel->r.left = OV02C10_ACTIVE_LEFT;
+		sel->r.width = OV02C10_ACTIVE_WIDTH;
+		sel->r.height = OV02C10_ACTIVE_HEIGHT;
+		return 0;
+	}
+
+	return -EINVAL;
+}
+
 static const struct v4l2_subdev_pad_ops ov02c10_pad_ops = {
 	.set_fmt = ov02c10_set_format,
 	.get_fmt = v4l2_subdev_get_fmt,
 	.enum_mbus_code = ov02c10_enum_mbus_code,
 	.enum_frame_size = ov02c10_enum_frame_size,
+	.get_selection = ov02c10_get_selection,
 	.enable_streams = ov02c10_enable_streams,
 	.disable_streams = ov02c10_disable_streams,
 };
-- 
2.55.0

Re: [PATCH 2/3] media: ov02c10: Implement get_selection

[Bryan O'Donoghue]

On 05/09/2026 04:07, Felipe Calliari wrote:
> The driver does not implement .get_selection, so userspace cannot query
> the sensor's native size or active crop rectangle. libcamera reports
> "Unable to get rectangle N on pad 0/0: Inappropriate ioctl for device"
> and "The sensor kernel driver needs to be fixed" (see
> Documentation/sensor_driver_requirements.rst).
> 
> Implement .get_selection returning the fixed geometry of the sensor:
> 
>   - V4L2_SEL_TGT_NATIVE_SIZE / V4L2_SEL_TGT_CROP_BOUNDS: the full
>     1928x1092 pixel array.
>   - V4L2_SEL_TGT_CROP / V4L2_SEL_TGT_CROP_DEFAULT: the 1920x1080 active
>     area, offset by (4, 6), matching the readout window programmed in
>     sensor_1928x1092_30fps_setting[] (registers 0x3800-0x3807).
> 
> The crop is fixed, so sd_state is not consulted.
> 
> Signed-off-by: Felipe Calliari<calliarifelipe@gmail.com>

Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

[PATCH 3/3] media: ov02c10: Accept a 26 MHz external clock

[Felipe Calliari]

Several Meteor Lake / Lunar Lake designs (e.g. the Samsung Galaxy Book3/4
series) wire the OV02C10 to a 26 MHz external clock instead of the
19.2 MHz assumed so far. The IPU6 ipu-bridge forwards the rate from the
ACPI SSDB verbatim as the "clock-frequency" property, so probe() just
rejects it today:

  ov02c10 i2c-OVTI02C1:00: external clock 26000000 is not supported

Rename OV02C10_MCLK to OV02C10_MCLK_19_2MHZ, add OV02C10_MCLK_26MHZ and
accept both.

The PLL register tables are the 19.2 MHz ones; OmniVision's 26 MHz PLL
programming is not publicly available. With a 26 MHz input the same
dividers make every internal clock, and therefore the MIPI link, run
26/19.2 = 1.3542x faster: a ~541.7 MHz link and ~40 fps instead of the
nominal 400 MHz / 30 fps. Rather than leave link-frequency and
pixel-rate describing the 19.2 MHz case, add a second
V4L2_CID_LINK_FREQ menu entry (400 MHz * 26 / 19.2) and select it when
the external clock is 26 MHz. pixel-rate is derived from the link
frequency and scales with it, so the frame rate and exposure times
reported to userspace match the hardware, and the IPU6 CSI-2 receiver
programs its D-PHY high-speed frequency range and bandwidth budget for
the rate the sensor actually transmits.

The ipu-bridge fwnode only lists the nominal 400 MHz link frequency
(keyed by ACPI HID, not by clock rate), so v4l2_link_freq_to_bitmap()
still matches on the 400 MHz entry and the 541.7 MHz index is selected
explicitly for the 26 MHz case.

On a Meteor Lake test machine a single CSI-2 "frame sync error" may
still be logged by the IPU6 receiver at stream start, after which
capture runs cleanly; this looks like a sensor PLL settling transient
and is not addressed here.

While touching the clock check, terminate its error string with a
newline.

Signed-off-by: Felipe Calliari <calliarifelipe@gmail.com>
---
 drivers/media/i2c/ov02c10.c | 45 ++++++++++++++++++++++++++++++++-----
 1 file changed, 40 insertions(+), 5 deletions(-)

diff --git a/drivers/media/i2c/ov02c10.c b/drivers/media/i2c/ov02c10.c
index 114db38c0..5184eb1cc 100644
--- a/drivers/media/i2c/ov02c10.c
+++ b/drivers/media/i2c/ov02c10.c
@@ -16,7 +16,16 @@
 #include <media/v4l2-fwnode.h>
 
 #define OV02C10_LINK_FREQ_400MHZ	400000000ULL
-#define OV02C10_MCLK			19200000
+/*
+ * The PLL register tables target a 19.2 MHz input clock.  On boards that
+ * clock the sensor at 26 MHz the same dividers yield a 26/19.2 = 1.3542x
+ * faster MIPI link (and frame rate).  OmniVision's 26 MHz PLL values are
+ * not public, so rather than re-normalise the link the driver advertises
+ * the real, scaled link frequency: 400 MHz * 26 / 19.2 = 541.667 MHz.
+ */
+#define OV02C10_LINK_FREQ_541MHZ	541666667ULL
+#define OV02C10_MCLK_19_2MHZ		19200000
+#define OV02C10_MCLK_26MHZ		26000000
 #define OV02C10_RGB_DEPTH		10
 
 #define OV02C10_NATIVE_WIDTH		1928
@@ -345,8 +354,14 @@ static const char * const ov02c10_test_pattern_menu[] = {
 	"Color Bar type 4",
 };
 
+enum {
+	OV02C10_LINK_FREQ_400MHZ_IDX,	/* 19.2 MHz external clock */
+	OV02C10_LINK_FREQ_541MHZ_IDX,	/* 26 MHz external clock */
+};
+
 static const s64 link_freq_menu_items[] = {
-	OV02C10_LINK_FREQ_400MHZ,
+	[OV02C10_LINK_FREQ_400MHZ_IDX] = OV02C10_LINK_FREQ_400MHZ,
+	[OV02C10_LINK_FREQ_541MHZ_IDX] = OV02C10_LINK_FREQ_541MHZ,
 };
 
 static const struct ov02c10_mode supported_modes[] = {
@@ -396,6 +411,9 @@ struct ov02c10 {
 	/* MIPI lane info */
 	u32 link_freq_index;
 	u8 mipi_lanes;
+
+	/* External (sensor) clock rate, Hz */
+	u32 xvclk_freq;
 };
 
 static inline struct ov02c10 *to_ov02c10(struct v4l2_subdev *subdev)
@@ -507,7 +525,8 @@ static int ov02c10_init_controls(struct ov02c10 *ov02c10)
 	ov02c10->link_freq = v4l2_ctrl_new_int_menu(ctrl_hdlr,
 						    &ov02c10_ctrl_ops,
 						    V4L2_CID_LINK_FREQ,
-						    ov02c10->link_freq_index, 0,
+						    ov02c10->link_freq_index,
+						    ov02c10->link_freq_index,
 						    link_freq_menu_items);
 	if (ov02c10->link_freq)
 		ov02c10->link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY;
@@ -875,6 +894,21 @@ static int ov02c10_check_hwcfg(struct ov02c10 *ov02c10)
 	/* v4l2_link_freq_to_bitmap() guarantees at least 1 bit is set */
 	ov02c10->link_freq_index = ffs(link_freq_bitmap) - 1;
 
+	/*
+	 * The IPU6 ipu-bridge always describes the nominal 19.2 MHz link
+	 * (400 MHz) in the fwnode, keyed by ACPI HID, even on boards that
+	 * clock the sensor at 26 MHz.  There the real link frequency is
+	 * 26/19.2 higher; advertise it so the CSI-2 receiver programs its
+	 * D-PHY frequency band and bandwidth budget for the rate the sensor
+	 * actually transmits.
+	 */
+	if (ov02c10->xvclk_freq == OV02C10_MCLK_26MHZ)
+		ov02c10->link_freq_index = OV02C10_LINK_FREQ_541MHZ_IDX;
+
+	dev_dbg(dev, "%u Hz external clock, link freq %lld Hz\n",
+		ov02c10->xvclk_freq,
+		link_freq_menu_items[ov02c10->link_freq_index]);
+
 	if (bus_cfg.bus.mipi_csi2.num_data_lanes != 1 &&
 	    bus_cfg.bus.mipi_csi2.num_data_lanes != 2) {
 		ret = dev_err_probe(dev, -EINVAL,
@@ -924,10 +958,11 @@ static int ov02c10_probe(struct i2c_client *client)
 				     "failed to get imaging clock\n");
 
 	freq = clk_get_rate(ov02c10->img_clk);
-	if (freq != OV02C10_MCLK)
+	if (freq != OV02C10_MCLK_19_2MHZ && freq != OV02C10_MCLK_26MHZ)
 		return dev_err_probe(ov02c10->dev, -EINVAL,
-				     "external clock %lu is not supported",
+				     "external clock %lu is not supported\n",
 				     freq);
+	ov02c10->xvclk_freq = freq;
 
 	v4l2_i2c_subdev_init(&ov02c10->sd, client, &ov02c10_subdev_ops);
 
-- 
2.55.0

Re: [PATCH 3/3] media: ov02c10: Accept a 26 MHz external clock

[Sakari Ailus]

Hi Felipe,

On Sat, Sep 05, 2026 at 12:07:32AM -0300, Felipe Calliari wrote:
> Several Meteor Lake / Lunar Lake designs (e.g. the Samsung Galaxy Book3/4
> series) wire the OV02C10 to a 26 MHz external clock instead of the
> 19.2 MHz assumed so far. The IPU6 ipu-bridge forwards the rate from the
> ACPI SSDB verbatim as the "clock-frequency" property, so probe() just
> rejects it today:
> 
>   ov02c10 i2c-OVTI02C1:00: external clock 26000000 is not supported
> 
> Rename OV02C10_MCLK to OV02C10_MCLK_19_2MHZ, add OV02C10_MCLK_26MHZ and
> accept both.
> 
> The PLL register tables are the 19.2 MHz ones; OmniVision's 26 MHz PLL
> programming is not publicly available. With a 26 MHz input the same
> dividers make every internal clock, and therefore the MIPI link, run
> 26/19.2 = 1.3542x faster: a ~541.7 MHz link and ~40 fps instead of the
> nominal 400 MHz / 30 fps. Rather than leave link-frequency and
> pixel-rate describing the 19.2 MHz case, add a second
> V4L2_CID_LINK_FREQ menu entry (400 MHz * 26 / 19.2) and select it when
> the external clock is 26 MHz. pixel-rate is derived from the link
> frequency and scales with it, so the frame rate and exposure times
> reported to userspace match the hardware, and the IPU6 CSI-2 receiver
> programs its D-PHY high-speed frequency range and bandwidth budget for
> the rate the sensor actually transmits.
> 
> The ipu-bridge fwnode only lists the nominal 400 MHz link frequency
> (keyed by ACPI HID, not by clock rate), so v4l2_link_freq_to_bitmap()
> still matches on the 400 MHz entry and the 541.7 MHz index is selected
> explicitly for the 26 MHz case.

Please don't use a hard-coded value here. Instead, calculate the pixel
rate.

Registers 0x0304 and 0x0315 (both 16-bit) control the PLL multipliers for
OP and VT PLLs, respectively. You could also change the multipliers to
arrive in a frequency close to the previous configuration. The values would
be 0x28a and 0x1b1, respectively. I don't have the sensor so I can't test
this. The pixel rate would be a bit off, 400,307929 MHz, assuming the
previous value was exactly 400 MHz. This would also require adding the
frequency to the IPU bridge.

Either the pixel rate or the link frequency exported by the driver is
probably wrong.

-- 
Regards,

Sakari Ailus

Re: [PATCH 3/3] media: ov02c10: Accept a 26 MHz external clock

[Felipe Calliari]

Hi Sakari,

On Tue, Sep 08, 2026 at 11:06:49AM +0300, Sakari Ailus wrote:
> Please don't use a hard-coded value here. Instead, calculate the pixel
> rate.
>
> Registers 0x0304 and 0x0315 (both 16-bit) control the PLL multipliers for
> OP and VT PLLs, respectively. You could also change the multipliers to
> arrive in a frequency close to the previous configuration. The values would
> be 0x28a and 0x1b1, respectively. I don't have the sensor so I can't test
> this. The pixel rate would be a bit off, 400,307929 MHz, assuming the
> previous value was exactly 400 MHz. This would also require adding the
> frequency to the IPU bridge.

Thanks -- re-programming the multipliers is the right approach and it
does work, just not with those particular values. I tested both on the
affected hardware (Samsung Galaxy Book3, OV02C10 on a confirmed 26 MHz
external clock, verified via /sys/kernel/debug/clk/clk_summary:
INT3472:01-clk = 26000000).

With 0x0304/0x0305 = 0x028a and 0x0315/0x0316 = 0x01b1 (each pair
big-endian, low address = high byte, as elsewhere in this driver) the
sensor produces no output at all: a 3-frame v4l2-ctl --stream-mmap
capture hangs with 0 bytes written, no I2C errors are logged, and
intel_ipu6_isys logs "stream stop time out" / "stream close time out"
on teardown. Recovering the sensor needed an i2c unbind/rebind.

What does work is scaling the multipliers the driver already programs.
The mode tables set the OP multiplier to 0x0190 = 400 for a 19.2 MHz
clock: the common table writes 0x0304 = 0x01, and the per-lane tables
then override 0x0305 = 0x90. Scaling that by 19.2/26 gives
400 * 19.2 / 26 = 295.4, i.e. 0x0127, applied to both multipliers when
the external clock is 26 MHz -- written after the per-lane table in
enable_streams():

    {0x0304, 0x01},
    {0x0305, 0x27},
    {0x0315, 0x01},
    {0x0316, 0x27},

Measured on the Galaxy Book3, which runs the sensor on two CSI-2 data
lanes (0x3016 reads back 0x32):

 - Reading the registers back over i2c while streaming gives 0x0127 for
   both the OP and the VT multiplier.
 - The frame rate is 29.94 fps, timing a 400-frame capture against a
   100-frame one so that pipeline startup cancels out (13.71 s vs
   3.69 s). The same hardware ran at ~40 fps with the unmodified tables.
 - The exported controls agree with that: hblank 352 and vblank 1236
   make a 2280 x 2328 frame, which at pixel_rate 160000000 works out to
   30.14 fps against the 29.94 measured. link_frequency reads 400000000.
 - No stream stop/close timeouts.

295 * 26 / 19.2 = 399.5 MHz, i.e. within 0.13% of the nominal 400 MHz,
so the 400 MHz entry the IPU bridge already advertises stays accurate.
That means v2 can drop the second, hard-coded link-frequency entry
altogether: there is a single 400 MHz entry again,
v4l2_link_freq_to_bitmap() matches it directly with no index fixup, and
no IPU bridge change is needed. pixel_rate stays derived from the link
frequency as before.

I can't account for 0x28a from here: 650 * 26 / 19.2 = 880 MHz, which is
far outside the D-PHY range the receiver is configured for and would
explain the missing signal. 650 would only line up if the starting
multiplier were ~880, whereas the tables program 400 once the per-lane
0x0305 = 0x90 override is applied. If you meant a different baseline, or
a different register pairing, I'm happy to test that too.

Two things worth flagging. The mode tables never write 0x0315 (only
0x0316 = 0x90), so I can't say what the VT multiplier's high byte was
beforehand -- the driver now writes both bytes explicitly, and the
readback above is after that write. And the single CSI-2 "frame sync
error" logged at stream start is still there with the link back at
~400 MHz, so it isn't caused by the faster clock, as the v1 commit
message speculated.

Unless you'd rather have it done differently, I'll send a v2 of this
patch with the PLL re-programming above and the 541.667 MHz entry
dropped.

Thanks,
Felipe

Re: [PATCH 3/3] media: ov02c10: Accept a 26 MHz external clock

[Bryan O'Donoghue]

On 05/09/2026 04:07, Felipe Calliari wrote:
> Several Meteor Lake / Lunar Lake designs (e.g. the Samsung Galaxy Book3/4
> series) wire the OV02C10 to a 26 MHz external clock instead of the
> 19.2 MHz assumed so far. The IPU6 ipu-bridge forwards the rate from the
> ACPI SSDB verbatim as the "clock-frequency" property, so probe() just
> rejects it today:
> 
>    ov02c10 i2c-OVTI02C1:00: external clock 26000000 is not supported
> 
> Rename OV02C10_MCLK to OV02C10_MCLK_19_2MHZ, add OV02C10_MCLK_26MHZ and
> accept both.
> 
> The PLL register tables are the 19.2 MHz ones; OmniVision's 26 MHz PLL
> programming is not publicly available. With a 26 MHz input the same
> dividers make every internal clock, and therefore the MIPI link, run
> 26/19.2 = 1.3542x faster: a ~541.7 MHz link and ~40 fps instead of the
> nominal 400 MHz / 30 fps. Rather than leave link-frequency and
> pixel-rate describing the 19.2 MHz case, add a second
> V4L2_CID_LINK_FREQ menu entry (400 MHz * 26 / 19.2) and select it when
> the external clock is 26 MHz. pixel-rate is derived from the link
> frequency and scales with it, so the frame rate and exposure times
> reported to userspace match the hardware, and the IPU6 CSI-2 receiver
> programs its D-PHY high-speed frequency range and bandwidth budget for
> the rate the sensor actually transmits.
> 
> The ipu-bridge fwnode only lists the nominal 400 MHz link frequency
> (keyed by ACPI HID, not by clock rate), so v4l2_link_freq_to_bitmap()
> still matches on the 400 MHz entry and the 541.7 MHz index is selected
> explicitly for the 26 MHz case.
> 
> On a Meteor Lake test machine a single CSI-2 "frame sync error" may
> still be logged by the IPU6 receiver at stream start, after which
> capture runs cleanly; this looks like a sensor PLL settling transient
> and is not addressed here.
> 
> While touching the clock check, terminate its error string with a
> newline.
> 
> Signed-off-by: Felipe Calliari<calliarifelipe@gmail.com>
I don't see how this gets handled with Intel's downstream work

https://github.com/intel/ipu6-drivers/blob/master/drivers/media/i2c/ov02c10.c

So perhaps it just isn't handled. Seems a bit odd that several ODMs 
would use the 26MHz clock but there is no update to the ipu sensors 
drivers to match.

Absent the real init sequences for a 26 MHz xvclk, this solution is 
better than nothing.

Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

Re: [PATCH 1/3] media: ov02c10: Drop duplicate register write

[Bryan O'Donoghue]

On 05/09/2026 04:07, Felipe Calliari wrote:
> sensor_1928x1092_30fps_setting[] writes {0x395f, 0x00} twice in a row.
> The second write is redundant; drop it. No functional change.
> 
> Signed-off-by: Felipe Calliari<calliarifelipe@gmail.com>
> ---
>   drivers/media/i2c/ov02c10.c | 1 -
>   1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/media/i2c/ov02c10.c b/drivers/media/i2c/ov02c10.c
> index cf93d3603..6220461fd 100644
> --- a/drivers/media/i2c/ov02c10.c
> +++ b/drivers/media/i2c/ov02c10.c
> @@ -209,7 +209,6 @@ static const struct reg_sequence sensor_1928x1092_30fps_setting[] = {
>   	{0x395d, 0x05},
>   	{0x395e, 0x02},
>   	{0x395f, 0x00},
> -	{0x395f, 0x00},

Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>