[PATCH v8 00/14] media: i2c: Add Omnivision OV02C10 sensor driver

[PATCH v8 00/14] media: i2c: Add Omnivision OV02C10 sensor driver

[Hans de Goede]

Hi All,

Here is v8 of the patch to upstream the OV02C10 sensor driver originally
writen by Intel which Heimir has been working on upstreaming.

At Heimir's request I've taken over the upstreaming process. This new
version addresses all the review remarks from Sakari, Bryan and Stanislaw,
thank you all for the reviews.

While working on fixing the review remarks I've also found and fixed /
improved a bunch of other things myself.

All in all there are quite a few changes, therefor I've chosen to send this
as a patch series. I understand this cannot be merged in this form, I'll
squash everything back together for v9. There are 2 reasons for sending
this v8 as a series:

1. I don't have hardware to test. I hope that others can test this soon,
   if things don't work the idea is that people can apply my cleanups
   1 by 1 and then we will know which change has broken things.

2. There are other sensor drivers from Intel at:
   https://github.com/intel/ipu6-drivers/tree/master/drivers/media/i2c/
   which likely need similar changes. Doing this as an incremental series
   is also intended to document all the cleanups which likely need to be
   applied to other Intel drivers too.

Note to reviewers there are some suboptimal things in this series wrt
adding things and then later removing them again, like e.g. the use of
guard(mutex)(&ov02c10->mutex). I did not bother to fix this since this
will all get squashed together in v9 anyways.

If it is easier for reviewing I can also (at request) post a v9 immediately
with everything squashed together. Even then I still believe this
admittedly weird v8 is useful for the reasons given above.

Regards,

Hans


Hans de Goede (13):
  media: ov02c10: merge shared register settings into a shared
    reg_sequence array
  media: ov02c10: Fix hts for 2 lane mode
  media: ov02c10: Fix vts_min for 2 lane mode
  media: ov02c10: link-freq-index and pixel-rate fixes
  media: ov02c10: ov02c10_check_hwcfg() improvements
  media: ov02c10: CCI usage fixes
  media: ov02c10: Make modes lane-count independent
  media: ov02c10: Drop handshake pin support
  media: ov02c10: ov02c10_get_pm_resources() fixes
  media: ov02c10: Switch to {enable,disable}_streams
  media: ov02c10: Drop system suspend and resume handlers
  media: ov02c10: Switch to using the sub-device state lock
  media: ov02c10: Use v4l2_subdev_get_fmt() as
    v4l2_subdev_pad_ops.get_fmt()

Heimir Thor Sverrisson (1):
  media: i2c: Add Omnivision OV02C10 sensor driver

 drivers/media/i2c/Kconfig   |   10 +
 drivers/media/i2c/Makefile  |    1 +
 drivers/media/i2c/ov02c10.c | 1012 +++++++++++++++++++++++++++++++++++
 3 files changed, 1023 insertions(+)
 create mode 100644 drivers/media/i2c/ov02c10.c

-- 
2.48.1

[PATCH v8 01/14] media: i2c: Add Omnivision OV02C10 sensor driver

[Hans de Goede]

From: Heimir Thor Sverrisson <heimir.sverrisson@gmail.com>

Add a new driver for the Omnivision OV02C10 camera sensor. This is based
on the out of tree driver by Hao Yao <hao.yao@intel.com> from:
https://github.com/intel/ipu6-drivers/blob/master/drivers/media/i2c/ov02c10.c

This has been tested on a Dell XPS 9440 together with the IPU6 isys CSI
driver and the libcamera software ISP code.

Tested-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>
Tested-by: Ingvar Hagelund <ingvar@redpill-linpro.com>
Tested-by: Heimir Thor Sverrisson <heimir.sverrisson@gmail.com>
Signed-off-by: Heimir Thor Sverrisson <heimir.sverrisson@gmail.com>
Link: https://lore.kernel.org/r/20250116232207.217402-1-heimir.sverrisson@gmail.com
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/media/i2c/Kconfig   |   10 +
 drivers/media/i2c/Makefile  |    1 +
 drivers/media/i2c/ov02c10.c | 1296 +++++++++++++++++++++++++++++++++++
 3 files changed, 1307 insertions(+)
 create mode 100644 drivers/media/i2c/ov02c10.c

diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index bb9ab2330d24..99a72b8ee45c 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -365,6 +365,16 @@ config VIDEO_OV02A10
 	  To compile this driver as a module, choose M here: the
 	  module will be called ov02a10.
 
+config VIDEO_OV02C10
+	tristate "OmniVision OV02C10 sensor support"
+	select V4L2_CCI_I2C
+	help
+	  This is a Video4Linux2 sensor driver for the OmniVision
+	  OV02C10 camera.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called ov02c10.
+
 config VIDEO_OV08D10
         tristate "OmniVision OV08D10 sensor support"
         help
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index a17151bb3d49..191c1f7c3f50 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -85,6 +85,7 @@ obj-$(CONFIG_VIDEO_OG01A1B) += og01a1b.o
 obj-$(CONFIG_VIDEO_OV01A10) += ov01a10.o
 obj-$(CONFIG_VIDEO_OV01A1S) += ov01a1s.o
 obj-$(CONFIG_VIDEO_OV02A10) += ov02a10.o
+obj-$(CONFIG_VIDEO_OV02C10) += ov02c10.o
 obj-$(CONFIG_VIDEO_OV08D10) += ov08d10.o
 obj-$(CONFIG_VIDEO_OV08X40) += ov08x40.o
 obj-$(CONFIG_VIDEO_OV13858) += ov13858.o
diff --git a/drivers/media/i2c/ov02c10.c b/drivers/media/i2c/ov02c10.c
new file mode 100644
index 000000000000..291da9ee1788
--- /dev/null
+++ b/drivers/media/i2c/ov02c10.c
@@ -0,0 +1,1296 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2022 Intel Corporation.
+
+#include <linux/acpi.h>
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/pm_runtime.h>
+#include <linux/regmap.h>
+#include <linux/version.h>
+#include <media/v4l2-cci.h>
+#include <media/v4l2-ctrls.h>
+#include <media/v4l2-device.h>
+#include <media/v4l2-fwnode.h>
+
+#define OV02C10_LINK_FREQ_400MHZ	400000000ULL
+#define OV02C10_SCLK			80000000LL
+#define OV02C10_MCLK			19200000
+#define OV02C10_DATA_LANES		1
+#define OV02C10_RGB_DEPTH		10
+
+#define OV02C10_REG_CHIP_ID		CCI_REG16(0x300a)
+#define OV02C10_CHIP_ID			0x5602
+
+#define OV02C10_REG_STREAM_CONTROL	CCI_REG8(0x0100)
+
+/* vertical-timings from sensor */
+#define OV02C10_REG_VTS			CCI_REG16(0x380e)
+#define OV02C10_VTS_MAX			0xffff
+
+/* Exposure controls from sensor */
+#define OV02C10_REG_EXPOSURE		CCI_REG16(0x3501)
+#define OV02C10_EXPOSURE_MIN		4
+#define OV02C10_EXPOSURE_MAX_MARGIN	8
+#define OV02C10_EXPOSURE_STEP		1
+
+/* Analog gain controls from sensor */
+#define OV02C10_REG_ANALOG_GAIN		CCI_REG16(0x3508)
+#define OV02C10_ANAL_GAIN_MIN		0x10
+#define OV02C10_ANAL_GAIN_MAX		0xf8
+#define OV02C10_ANAL_GAIN_STEP		1
+#define OV02C10_ANAL_GAIN_DEFAULT	0x10
+
+/* Digital gain controls from sensor */
+#define OV02C10_REG_DIGITAL_GAIN	CCI_REG24(0x350a)
+#define OV02C10_DGTL_GAIN_MIN		0x0400
+#define OV02C10_DGTL_GAIN_MAX		0x3fff
+#define OV02C10_DGTL_GAIN_STEP		1
+#define OV02C10_DGTL_GAIN_DEFAULT	0x0400
+
+/* Rotate */
+#define OV02C10_ROTATE_CONTROL		CCI_REG8(0x3820)
+#define OV02C10_ISP_X_WIN_CONTROL	CCI_REG16(0x3810)
+#define OV02C10_ISP_Y_WIN_CONTROL	CCI_REG16(0x3812)
+#define OV02C10_CONFIG_ROTATE		0x18
+
+/* Test Pattern Control */
+#define OV02C10_REG_TEST_PATTERN		CCI_REG8(0x4503)
+#define OV02C10_TEST_PATTERN_ENABLE		BIT(7)
+
+struct ov02c10_mode {
+	/* Frame width in pixels */
+	u32 width;
+
+	/* Frame height in pixels */
+	u32 height;
+
+	/* Horizontal timining size */
+	u32 hts;
+
+	/* Default vertical timining size */
+	u32 vts_def;
+
+	/* Min vertical timining size */
+	u32 vts_min;
+
+	/* Link frequency needed for this resolution */
+	u32 link_freq_index;
+
+	/* MIPI lanes used */
+	u8 mipi_lanes;
+
+	/* Sensor register settings for this resolution */
+	const struct reg_sequence *reg_sequence;
+	const int sequence_length;
+};
+
+static const struct reg_sequence sensor_1928x1092_1lane_30fps_setting[] = {
+	{0x0301, 0x08},
+	{0x0303, 0x06},
+	{0x0304, 0x01},
+	{0x0305, 0xe0},
+	{0x0313, 0x40},
+	{0x031c, 0x4f},
+	{0x301b, 0xd2},
+	{0x3020, 0x97},
+	{0x3022, 0x01},
+	{0x3026, 0xb4},
+	{0x3027, 0xe1},
+	{0x303b, 0x00},
+	{0x303c, 0x4f},
+	{0x303d, 0xe6},
+	{0x303e, 0x00},
+	{0x303f, 0x03},
+	{0x3021, 0x23},
+	{0x3501, 0x04},
+	{0x3502, 0x6c},
+	{0x3504, 0x0c},
+	{0x3507, 0x00},
+	{0x3508, 0x08},
+	{0x3509, 0x00},
+	{0x350a, 0x01},
+	{0x350b, 0x00},
+	{0x350c, 0x41},
+	{0x3600, 0x84},
+	{0x3603, 0x08},
+	{0x3610, 0x57},
+	{0x3611, 0x1b},
+	{0x3613, 0x78},
+	{0x3623, 0x00},
+	{0x3632, 0xa0},
+	{0x3642, 0xe8},
+	{0x364c, 0x70},
+	{0x365f, 0x0f},
+	{0x3708, 0x30},
+	{0x3714, 0x24},
+	{0x3725, 0x02},
+	{0x3737, 0x08},
+	{0x3739, 0x28},
+	{0x3749, 0x32},
+	{0x374a, 0x32},
+	{0x374b, 0x32},
+	{0x374c, 0x32},
+	{0x374d, 0x81},
+	{0x374e, 0x81},
+	{0x374f, 0x81},
+	{0x3752, 0x36},
+	{0x3753, 0x36},
+	{0x3754, 0x36},
+	{0x3761, 0x00},
+	{0x376c, 0x81},
+	{0x3774, 0x18},
+	{0x3776, 0x08},
+	{0x377c, 0x81},
+	{0x377d, 0x81},
+	{0x377e, 0x81},
+	{0x37a0, 0x44},
+	{0x37a6, 0x44},
+	{0x37aa, 0x0d},
+	{0x37ae, 0x00},
+	{0x37cb, 0x03},
+	{0x37cc, 0x01},
+	{0x37d8, 0x02},
+	{0x37d9, 0x10},
+	{0x37e1, 0x10},
+	{0x37e2, 0x18},
+	{0x37e3, 0x08},
+	{0x37e4, 0x08},
+	{0x37e5, 0x02},
+	{0x37e6, 0x08},
+
+	/* 1928x1092 */
+	{0x3800, 0x00},
+	{0x3801, 0x00},
+	{0x3802, 0x00},
+	{0x3803, 0x00},
+	{0x3804, 0x07},
+	{0x3805, 0x8f},
+	{0x3806, 0x04},
+	{0x3807, 0x47},
+	{0x3808, 0x07},
+	{0x3809, 0x88},
+	{0x380a, 0x04},
+	{0x380b, 0x44},
+	{0x380c, 0x08},
+	{0x380d, 0xe8},
+	{0x380e, 0x04},
+	{0x380f, 0x8c},
+	{0x3810, 0x00},
+	{0x3811, 0x02},
+	{0x3812, 0x00},
+	{0x3813, 0x02},
+	{0x3814, 0x01},
+	{0x3815, 0x01},
+	{0x3816, 0x01},
+	{0x3817, 0x01},
+
+	{0x3820, 0xb0},
+	{0x3821, 0x00},
+	{0x3822, 0x80},
+	{0x3823, 0x08},
+	{0x3824, 0x00},
+	{0x3825, 0x20},
+	{0x3826, 0x00},
+	{0x3827, 0x08},
+	{0x382a, 0x00},
+	{0x382b, 0x08},
+	{0x382d, 0x00},
+	{0x382e, 0x00},
+	{0x382f, 0x23},
+	{0x3834, 0x00},
+	{0x3839, 0x00},
+	{0x383a, 0xd1},
+	{0x383e, 0x03},
+	{0x393d, 0x29},
+	{0x393f, 0x6e},
+	{0x394b, 0x06},
+	{0x394c, 0x06},
+	{0x394d, 0x08},
+	{0x394e, 0x0b},
+	{0x394f, 0x01},
+	{0x3950, 0x01},
+	{0x3951, 0x01},
+	{0x3952, 0x01},
+	{0x3953, 0x01},
+	{0x3954, 0x01},
+	{0x3955, 0x01},
+	{0x3956, 0x01},
+	{0x3957, 0x0e},
+	{0x3958, 0x08},
+	{0x3959, 0x08},
+	{0x395a, 0x08},
+	{0x395b, 0x13},
+	{0x395c, 0x09},
+	{0x395d, 0x05},
+	{0x395e, 0x02},
+	{0x395f, 0x00},
+	{0x395f, 0x00},
+	{0x3960, 0x00},
+	{0x3961, 0x00},
+	{0x3962, 0x00},
+	{0x3963, 0x00},
+	{0x3964, 0x00},
+	{0x3965, 0x00},
+	{0x3966, 0x00},
+	{0x3967, 0x00},
+	{0x3968, 0x01},
+	{0x3969, 0x01},
+	{0x396a, 0x01},
+	{0x396b, 0x01},
+	{0x396c, 0x10},
+	{0x396d, 0xf0},
+	{0x396e, 0x11},
+	{0x396f, 0x00},
+	{0x3970, 0x37},
+	{0x3971, 0x37},
+	{0x3972, 0x37},
+	{0x3973, 0x37},
+	{0x3974, 0x00},
+	{0x3975, 0x3c},
+	{0x3976, 0x3c},
+	{0x3977, 0x3c},
+	{0x3978, 0x3c},
+	{0x3c00, 0x0f},
+	{0x3c20, 0x01},
+	{0x3c21, 0x08},
+	{0x3f00, 0x8b},
+	{0x3f02, 0x0f},
+	{0x4000, 0xc3},
+	{0x4001, 0xe0},
+	{0x4002, 0x00},
+	{0x4003, 0x40},
+	{0x4008, 0x04},
+	{0x4009, 0x23},
+	{0x400a, 0x04},
+	{0x400b, 0x01},
+	{0x4077, 0x06},
+	{0x4078, 0x00},
+	{0x4079, 0x1a},
+	{0x407a, 0x7f},
+	{0x407b, 0x01},
+	{0x4080, 0x03},
+	{0x4081, 0x84},
+	{0x4308, 0x03},
+	{0x4309, 0xff},
+	{0x430d, 0x00},
+	{0x4806, 0x00},
+	{0x4813, 0x00},
+	{0x4837, 0x10},
+	{0x4857, 0x05},
+	{0x4500, 0x07},
+	{0x4501, 0x00},
+	{0x4503, 0x00},
+	{0x450a, 0x04},
+	{0x450e, 0x00},
+	{0x450f, 0x00},
+	{0x4800, 0x24},
+	{0x4900, 0x00},
+	{0x4901, 0x00},
+	{0x4902, 0x01},
+	{0x5000, 0xf5},
+	{0x5001, 0x50},
+	{0x5006, 0x00},
+	{0x5080, 0x40},
+	{0x5181, 0x2b},
+	{0x5202, 0xa3},
+	{0x5206, 0x01},
+	{0x5207, 0x00},
+	{0x520a, 0x01},
+	{0x520b, 0x00},
+	{0x365d, 0x00},
+	{0x4815, 0x40},
+	{0x4816, 0x12},
+	{0x4f00, 0x01},
+	/* plls */
+	{0x0303, 0x05},
+	{0x0305, 0x90},
+	{0x0316, 0x90},
+	{0x3016, 0x12},
+};
+
+static const struct reg_sequence sensor_1928x1092_2lane_30fps_setting[] = {
+	{0x0301, 0x08},
+	{0x0303, 0x06},
+	{0x0304, 0x01},
+	{0x0305, 0xe0},
+	{0x0313, 0x40},
+	{0x031c, 0x4f},
+	{0x301b, 0xf0},
+	{0x3020, 0x97},
+	{0x3022, 0x01},
+	{0x3026, 0xb4},
+	{0x3027, 0xf1},
+	{0x303b, 0x00},
+	{0x303c, 0x4f},
+	{0x303d, 0xe6},
+	{0x303e, 0x00},
+	{0x303f, 0x03},
+	{0x3021, 0x23},
+	{0x3501, 0x04},
+	{0x3502, 0x6c},
+	{0x3504, 0x0c},
+	{0x3507, 0x00},
+	{0x3508, 0x08},
+	{0x3509, 0x00},
+	{0x350a, 0x01},
+	{0x350b, 0x00},
+	{0x350c, 0x41},
+	{0x3600, 0x84},
+	{0x3603, 0x08},
+	{0x3610, 0x57},
+	{0x3611, 0x1b},
+	{0x3613, 0x78},
+	{0x3623, 0x00},
+	{0x3632, 0xa0},
+	{0x3642, 0xe8},
+	{0x364c, 0x70},
+	{0x365f, 0x0f},
+	{0x3708, 0x30},
+	{0x3714, 0x24},
+	{0x3725, 0x02},
+	{0x3737, 0x08},
+	{0x3739, 0x28},
+	{0x3749, 0x32},
+	{0x374a, 0x32},
+	{0x374b, 0x32},
+	{0x374c, 0x32},
+	{0x374d, 0x81},
+	{0x374e, 0x81},
+	{0x374f, 0x81},
+	{0x3752, 0x36},
+	{0x3753, 0x36},
+	{0x3754, 0x36},
+	{0x3761, 0x00},
+	{0x376c, 0x81},
+	{0x3774, 0x18},
+	{0x3776, 0x08},
+	{0x377c, 0x81},
+	{0x377d, 0x81},
+	{0x377e, 0x81},
+	{0x37a0, 0x44},
+	{0x37a6, 0x44},
+	{0x37aa, 0x0d},
+	{0x37ae, 0x00},
+	{0x37cb, 0x03},
+	{0x37cc, 0x01},
+	{0x37d8, 0x02},
+	{0x37d9, 0x10},
+	{0x37e1, 0x10},
+	{0x37e2, 0x18},
+	{0x37e3, 0x08},
+	{0x37e4, 0x08},
+	{0x37e5, 0x02},
+	{0x37e6, 0x08},
+
+	/* 1928x1092 */
+	{0x3800, 0x00},
+	{0x3801, 0x00},
+	{0x3802, 0x00},
+	{0x3803, 0x00},
+	{0x3804, 0x07},
+	{0x3805, 0x8f},
+	{0x3806, 0x04},
+	{0x3807, 0x47},
+	{0x3808, 0x07},
+	{0x3809, 0x88},
+	{0x380a, 0x04},
+	{0x380b, 0x44},
+	{0x380c, 0x04},
+	{0x380d, 0x74},
+	{0x380e, 0x09},
+	{0x380f, 0x18},
+	{0x3810, 0x00},
+	{0x3811, 0x02},
+	{0x3812, 0x00},
+	{0x3813, 0x02},
+	{0x3814, 0x01},
+	{0x3815, 0x01},
+	{0x3816, 0x01},
+	{0x3817, 0x01},
+
+	{0x3820, 0xb0},
+	{0x3821, 0x00},
+	{0x3822, 0x80},
+	{0x3823, 0x08},
+	{0x3824, 0x00},
+	{0x3825, 0x20},
+	{0x3826, 0x00},
+	{0x3827, 0x08},
+	{0x382a, 0x00},
+	{0x382b, 0x08},
+	{0x382d, 0x00},
+	{0x382e, 0x00},
+	{0x382f, 0x23},
+	{0x3834, 0x00},
+	{0x3839, 0x00},
+	{0x383a, 0xd1},
+	{0x383e, 0x03},
+	{0x393d, 0x29},
+	{0x393f, 0x6e},
+	{0x394b, 0x06},
+	{0x394c, 0x06},
+	{0x394d, 0x08},
+	{0x394e, 0x0a},
+	{0x394f, 0x01},
+	{0x3950, 0x01},
+	{0x3951, 0x01},
+	{0x3952, 0x01},
+	{0x3953, 0x01},
+	{0x3954, 0x01},
+	{0x3955, 0x01},
+	{0x3956, 0x01},
+	{0x3957, 0x0e},
+	{0x3958, 0x08},
+	{0x3959, 0x08},
+	{0x395a, 0x08},
+	{0x395b, 0x13},
+	{0x395c, 0x09},
+	{0x395d, 0x05},
+	{0x395e, 0x02},
+	{0x395f, 0x00},
+	{0x395f, 0x00},
+	{0x3960, 0x00},
+	{0x3961, 0x00},
+	{0x3962, 0x00},
+	{0x3963, 0x00},
+	{0x3964, 0x00},
+	{0x3965, 0x00},
+	{0x3966, 0x00},
+	{0x3967, 0x00},
+	{0x3968, 0x01},
+	{0x3969, 0x01},
+	{0x396a, 0x01},
+	{0x396b, 0x01},
+	{0x396c, 0x10},
+	{0x396d, 0xf0},
+	{0x396e, 0x11},
+	{0x396f, 0x00},
+	{0x3970, 0x37},
+	{0x3971, 0x37},
+	{0x3972, 0x37},
+	{0x3973, 0x37},
+	{0x3974, 0x00},
+	{0x3975, 0x3c},
+	{0x3976, 0x3c},
+	{0x3977, 0x3c},
+	{0x3978, 0x3c},
+	{0x3c00, 0x0f},
+	{0x3c20, 0x01},
+	{0x3c21, 0x08},
+	{0x3f00, 0x8b},
+	{0x3f02, 0x0f},
+	{0x4000, 0xc3},
+	{0x4001, 0xe0},
+	{0x4002, 0x00},
+	{0x4003, 0x40},
+	{0x4008, 0x04},
+	{0x4009, 0x23},
+	{0x400a, 0x04},
+	{0x400b, 0x01},
+	{0x4041, 0x20},
+	{0x4077, 0x06},
+	{0x4078, 0x00},
+	{0x4079, 0x1a},
+	{0x407a, 0x7f},
+	{0x407b, 0x01},
+	{0x4080, 0x03},
+	{0x4081, 0x84},
+	{0x4308, 0x03},
+	{0x4309, 0xff},
+	{0x430d, 0x00},
+	{0x4806, 0x00},
+	{0x4813, 0x00},
+	{0x4837, 0x10},
+	{0x4857, 0x05},
+	{0x4884, 0x04},
+	{0x4500, 0x07},
+	{0x4501, 0x00},
+	{0x4503, 0x00},
+	{0x450a, 0x04},
+	{0x450e, 0x00},
+	{0x450f, 0x00},
+	{0x4800, 0x64},
+	{0x4900, 0x00},
+	{0x4901, 0x00},
+	{0x4902, 0x01},
+	{0x4d00, 0x03},
+	{0x4d01, 0xd8},
+	{0x4d02, 0xba},
+	{0x4d03, 0xa0},
+	{0x4d04, 0xb7},
+	{0x4d05, 0x34},
+	{0x4d0d, 0x00},
+	{0x5000, 0xfd},
+	{0x5001, 0x50},
+	{0x5006, 0x00},
+	{0x5080, 0x40},
+	{0x5181, 0x2b},
+	{0x5202, 0xa3},
+	{0x5206, 0x01},
+	{0x5207, 0x00},
+	{0x520a, 0x01},
+	{0x520b, 0x00},
+	{0x365d, 0x00},
+	{0x4815, 0x40},
+	{0x4816, 0x12},
+	{0x481f, 0x30},
+	{0x4f00, 0x01},
+	/* plls */
+	{0x0303, 0x05},
+	{0x0305, 0x90},
+	{0x0316, 0x90},
+	{0x3016, 0x32},
+};
+
+static const char * const ov02c10_test_pattern_menu[] = {
+	"Disabled",
+	"Color Bar",
+	"Top-Bottom Darker Color Bar",
+	"Right-Left Darker Color Bar",
+	"Color Bar type 4",
+};
+
+static const s64 link_freq_menu_items[] = {
+	OV02C10_LINK_FREQ_400MHZ,
+};
+
+static const struct ov02c10_mode supported_modes[] = {
+	{
+		.width = 1928,
+		.height = 1092,
+		.hts = 2280,
+		.vts_def = 1164,
+		.vts_min = 1164,
+		.mipi_lanes = 1,
+		.reg_sequence = sensor_1928x1092_1lane_30fps_setting,
+		.sequence_length = ARRAY_SIZE(sensor_1928x1092_1lane_30fps_setting),
+	},
+	{
+		.width = 1928,
+		.height = 1092,
+		.hts = 1140,
+		.vts_def = 2328,
+		.vts_min = 2328,
+		.mipi_lanes = 2,
+		.reg_sequence = sensor_1928x1092_2lane_30fps_setting,
+		.sequence_length = ARRAY_SIZE(sensor_1928x1092_2lane_30fps_setting),
+	},
+};
+
+struct ov02c10 {
+	struct v4l2_subdev sd;
+	struct media_pad pad;
+	struct v4l2_ctrl_handler ctrl_handler;
+	struct regmap *regmap;
+
+	/* V4L2 Controls */
+	struct v4l2_ctrl *link_freq;
+	struct v4l2_ctrl *pixel_rate;
+	struct v4l2_ctrl *vblank;
+	struct v4l2_ctrl *hblank;
+	struct v4l2_ctrl *exposure;
+
+	struct clk *img_clk;
+	struct regulator *avdd;
+	struct gpio_desc *reset;
+	struct gpio_desc *handshake;
+
+	/* Current mode */
+	const struct ov02c10_mode *cur_mode;
+
+	/* To serialize asynchronous callbacks */
+	struct mutex mutex;
+
+	/* MIPI lanes used */
+	u8 mipi_lanes;
+
+	/* Streaming on/off */
+	bool streaming;
+};
+
+static inline struct ov02c10 *to_ov02c10(struct v4l2_subdev *subdev)
+{
+	return container_of(subdev, struct ov02c10, sd);
+}
+
+static int ov02c10_test_pattern(struct ov02c10 *ov02c10, int pattern)
+{
+	int ret = 0;
+
+	if (!pattern)
+		return cci_update_bits(ov02c10->regmap, OV02C10_REG_TEST_PATTERN,
+			BIT(7), 0, NULL);
+
+	cci_update_bits(ov02c10->regmap, OV02C10_REG_TEST_PATTERN,
+			0x03, pattern - 1, &ret);
+	if (ret)
+		return ret;
+
+	cci_update_bits(ov02c10->regmap, OV02C10_REG_TEST_PATTERN,
+			BIT(7), OV02C10_TEST_PATTERN_ENABLE, &ret);
+
+	return ret;
+}
+
+static int ov02c10_set_ctrl(struct v4l2_ctrl *ctrl)
+{
+	struct ov02c10 *ov02c10 = container_of(ctrl->handler,
+					     struct ov02c10, ctrl_handler);
+	struct i2c_client *client = v4l2_get_subdevdata(&ov02c10->sd);
+	s64 exposure_max;
+	int ret = 0;
+
+	/* Propagate change of current control to all related controls */
+	if (ctrl->id == V4L2_CID_VBLANK) {
+		/* Update max exposure while meeting expected vblanking */
+		exposure_max = ov02c10->cur_mode->height + ctrl->val -
+			       OV02C10_EXPOSURE_MAX_MARGIN;
+		__v4l2_ctrl_modify_range(ov02c10->exposure,
+					 ov02c10->exposure->minimum,
+					 exposure_max, ov02c10->exposure->step,
+					 exposure_max);
+	}
+
+	/* V4L2 controls values will be applied only when power is already up */
+	if (!pm_runtime_get_if_in_use(&client->dev))
+		return 0;
+
+	switch (ctrl->id) {
+	case V4L2_CID_ANALOGUE_GAIN:
+		cci_write(ov02c10->regmap, OV02C10_REG_ANALOG_GAIN,
+			  ctrl->val << 4, &ret);
+		break;
+
+	case V4L2_CID_DIGITAL_GAIN:
+		cci_write(ov02c10->regmap, OV02C10_REG_DIGITAL_GAIN,
+			  ctrl->val << 6, &ret);
+		break;
+
+	case V4L2_CID_EXPOSURE:
+		cci_write(ov02c10->regmap, OV02C10_REG_EXPOSURE,
+			  ctrl->val, &ret);
+		break;
+
+	case V4L2_CID_VBLANK:
+		cci_write(ov02c10->regmap, OV02C10_REG_VTS,
+			  ov02c10->cur_mode->height + ctrl->val, &ret);
+		break;
+
+	case V4L2_CID_TEST_PATTERN:
+		ret = ov02c10_test_pattern(ov02c10, ctrl->val);
+		break;
+
+	default:
+		ret = -EINVAL;
+		break;
+	}
+
+	pm_runtime_put(&client->dev);
+
+	return ret;
+}
+
+static const struct v4l2_ctrl_ops ov02c10_ctrl_ops = {
+	.s_ctrl = ov02c10_set_ctrl,
+};
+
+static int ov02c10_init_controls(struct ov02c10 *ov02c10)
+{
+	struct v4l2_ctrl_handler *ctrl_hdlr;
+	const struct ov02c10_mode *cur_mode;
+	s64 exposure_max, h_blank;
+	u32 vblank_min, vblank_max, vblank_default;
+	int size;
+	int ret = 0;
+
+	ctrl_hdlr = &ov02c10->ctrl_handler;
+	ret = v4l2_ctrl_handler_init(ctrl_hdlr, 8);
+	if (ret)
+		return ret;
+
+	ctrl_hdlr->lock = &ov02c10->mutex;
+	cur_mode = ov02c10->cur_mode;
+	size = ARRAY_SIZE(link_freq_menu_items);
+
+	ov02c10->link_freq = v4l2_ctrl_new_int_menu(ctrl_hdlr,
+						    &ov02c10_ctrl_ops,
+						    V4L2_CID_LINK_FREQ,
+						    size - 1, 0,
+						    link_freq_menu_items);
+	if (ov02c10->link_freq)
+		ov02c10->link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY;
+
+	ov02c10->pixel_rate = v4l2_ctrl_new_std(ctrl_hdlr, &ov02c10_ctrl_ops,
+						V4L2_CID_PIXEL_RATE, 0,
+						OV02C10_SCLK, 1, OV02C10_SCLK);
+
+	vblank_min = cur_mode->vts_min - cur_mode->height;
+	vblank_max = OV02C10_VTS_MAX - cur_mode->height;
+	vblank_default = cur_mode->vts_def - cur_mode->height;
+	ov02c10->vblank = v4l2_ctrl_new_std(ctrl_hdlr, &ov02c10_ctrl_ops,
+					    V4L2_CID_VBLANK, vblank_min,
+					    vblank_max, 1, vblank_default);
+
+	h_blank = cur_mode->hts - cur_mode->width;
+	ov02c10->hblank = v4l2_ctrl_new_std(ctrl_hdlr, &ov02c10_ctrl_ops,
+					    V4L2_CID_HBLANK, h_blank, h_blank,
+					    1, h_blank);
+	if (ov02c10->hblank)
+		ov02c10->hblank->flags |= V4L2_CTRL_FLAG_READ_ONLY;
+
+	v4l2_ctrl_new_std(ctrl_hdlr, &ov02c10_ctrl_ops, V4L2_CID_ANALOGUE_GAIN,
+			  OV02C10_ANAL_GAIN_MIN, OV02C10_ANAL_GAIN_MAX,
+			  OV02C10_ANAL_GAIN_STEP, OV02C10_ANAL_GAIN_DEFAULT);
+	v4l2_ctrl_new_std(ctrl_hdlr, &ov02c10_ctrl_ops, V4L2_CID_DIGITAL_GAIN,
+			  OV02C10_DGTL_GAIN_MIN, OV02C10_DGTL_GAIN_MAX,
+			  OV02C10_DGTL_GAIN_STEP, OV02C10_DGTL_GAIN_DEFAULT);
+	exposure_max = cur_mode->vts_def - OV02C10_EXPOSURE_MAX_MARGIN;
+	ov02c10->exposure = v4l2_ctrl_new_std(ctrl_hdlr, &ov02c10_ctrl_ops,
+					      V4L2_CID_EXPOSURE,
+					      OV02C10_EXPOSURE_MIN,
+					      exposure_max,
+					      OV02C10_EXPOSURE_STEP,
+					      exposure_max);
+	v4l2_ctrl_new_std_menu_items(ctrl_hdlr, &ov02c10_ctrl_ops,
+				     V4L2_CID_TEST_PATTERN,
+				     ARRAY_SIZE(ov02c10_test_pattern_menu) - 1,
+				     0, 0, ov02c10_test_pattern_menu);
+	if (ctrl_hdlr->error)
+		return ctrl_hdlr->error;
+
+	ov02c10->sd.ctrl_handler = ctrl_hdlr;
+
+	return 0;
+}
+
+static void ov02c10_update_pad_format(const struct ov02c10_mode *mode,
+				      struct v4l2_mbus_framefmt *fmt)
+{
+	fmt->width = mode->width;
+	fmt->height = mode->height;
+	fmt->code = MEDIA_BUS_FMT_SGRBG10_1X10;
+	fmt->field = V4L2_FIELD_NONE;
+}
+
+static int ov02c10_start_streaming(struct ov02c10 *ov02c10)
+{
+	struct i2c_client *client = v4l2_get_subdevdata(&ov02c10->sd);
+	const struct reg_sequence *reg_sequence;
+	int sequence_length;
+	int ret = 0;
+
+	reg_sequence = ov02c10->cur_mode->reg_sequence;
+	sequence_length = ov02c10->cur_mode->sequence_length;
+	ret = regmap_multi_reg_write(ov02c10->regmap,
+				     reg_sequence, sequence_length);
+	if (ret) {
+		dev_err(&client->dev, "failed to set mode");
+		return ret;
+	}
+
+	ret = __v4l2_ctrl_handler_setup(ov02c10->sd.ctrl_handler);
+	if (ret)
+		return ret;
+
+	ret = cci_write(ov02c10->regmap, OV02C10_REG_STREAM_CONTROL, 1, NULL);
+	if (ret)
+		dev_err(&client->dev, "failed to start streaming");
+
+	return ret;
+}
+
+static void ov02c10_stop_streaming(struct ov02c10 *ov02c10)
+{
+	struct i2c_client *client = v4l2_get_subdevdata(&ov02c10->sd);
+	int ret = 0;
+
+	ret = cci_write(ov02c10->regmap, OV02C10_REG_STREAM_CONTROL, 0, NULL);
+	if (ret)
+		dev_err(&client->dev, "failed to stop streaming");
+}
+
+static int ov02c10_set_stream(struct v4l2_subdev *sd, int enable)
+{
+	struct ov02c10 *ov02c10 = to_ov02c10(sd);
+	struct i2c_client *client = v4l2_get_subdevdata(sd);
+	int ret = 0;
+
+	if (ov02c10->streaming == enable)
+		return 0;
+
+	mutex_lock(&ov02c10->mutex);
+	if (enable) {
+		ret = pm_runtime_get_sync(&client->dev);
+		if (ret < 0) {
+			pm_runtime_put_noidle(&client->dev);
+			mutex_unlock(&ov02c10->mutex);
+			return ret;
+		}
+
+		ret = ov02c10_start_streaming(ov02c10);
+		if (ret) {
+			enable = 0;
+			ov02c10_stop_streaming(ov02c10);
+			pm_runtime_put(&client->dev);
+		}
+	} else {
+		ov02c10_stop_streaming(ov02c10);
+		pm_runtime_put(&client->dev);
+	}
+
+	ov02c10->streaming = enable;
+	mutex_unlock(&ov02c10->mutex);
+
+	return ret;
+}
+
+/* This function tries to get power control resources */
+static int ov02c10_get_pm_resources(struct device *dev)
+{
+	struct v4l2_subdev *sd = dev_get_drvdata(dev);
+	struct ov02c10 *ov02c10 = to_ov02c10(sd);
+	int ret;
+
+	ov02c10->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
+	if (IS_ERR(ov02c10->reset))
+		return dev_err_probe(dev, PTR_ERR(ov02c10->reset),
+				     "failed to get reset gpio\n");
+
+	ov02c10->handshake = devm_gpiod_get_optional(dev, "handshake",
+						     GPIOD_OUT_LOW);
+	if (IS_ERR(ov02c10->handshake))
+		return dev_err_probe(dev, PTR_ERR(ov02c10->handshake),
+				     "failed to get handshake gpio\n");
+
+	ov02c10->img_clk = devm_clk_get_optional(dev, NULL);
+	if (IS_ERR(ov02c10->img_clk))
+		return dev_err_probe(dev, PTR_ERR(ov02c10->img_clk),
+				     "failed to get imaging clock\n");
+
+	ov02c10->avdd = devm_regulator_get_optional(dev, "avdd");
+	if (IS_ERR(ov02c10->avdd)) {
+		ret = PTR_ERR(ov02c10->avdd);
+		ov02c10->avdd = NULL;
+		if (ret != -ENODEV)
+			return dev_err_probe(dev, ret,
+					     "failed to get avdd regulator\n");
+	}
+
+	return 0;
+}
+
+static int ov02c10_power_off(struct device *dev)
+{
+	struct v4l2_subdev *sd = dev_get_drvdata(dev);
+	struct ov02c10 *ov02c10 = to_ov02c10(sd);
+	int ret = 0;
+
+	gpiod_set_value_cansleep(ov02c10->reset, 1);
+	gpiod_set_value_cansleep(ov02c10->handshake, 0);
+
+	if (ov02c10->avdd)
+		ret = regulator_disable(ov02c10->avdd);
+
+	clk_disable_unprepare(ov02c10->img_clk);
+
+	return ret;
+}
+
+static int ov02c10_power_on(struct device *dev)
+{
+	struct v4l2_subdev *sd = dev_get_drvdata(dev);
+	struct ov02c10 *ov02c10 = to_ov02c10(sd);
+	int ret;
+
+	ret = clk_prepare_enable(ov02c10->img_clk);
+	if (ret < 0) {
+		dev_err(dev, "failed to enable imaging clock: %d", ret);
+		return ret;
+	}
+
+	if (ov02c10->avdd) {
+		ret = regulator_enable(ov02c10->avdd);
+		if (ret < 0) {
+			dev_err(dev, "failed to enable avdd: %d", ret);
+			clk_disable_unprepare(ov02c10->img_clk);
+			return ret;
+		}
+	}
+	gpiod_set_value_cansleep(ov02c10->handshake, 1);
+	gpiod_set_value_cansleep(ov02c10->reset, 0);
+
+	/* Lattice MIPI aggregator with some version FW needs longer delay
+	 * after handshake triggered. We set 25ms as a safe value and wait
+	 * for a stable version FW.
+	 */
+	msleep_interruptible(25);
+
+	return ret;
+}
+
+static int __maybe_unused ov02c10_suspend(struct device *dev)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct v4l2_subdev *sd = i2c_get_clientdata(client);
+	struct ov02c10 *ov02c10 = to_ov02c10(sd);
+
+	mutex_lock(&ov02c10->mutex);
+	if (ov02c10->streaming)
+		ov02c10_stop_streaming(ov02c10);
+
+	mutex_unlock(&ov02c10->mutex);
+
+	return 0;
+}
+
+static int __maybe_unused ov02c10_resume(struct device *dev)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct v4l2_subdev *sd = i2c_get_clientdata(client);
+	struct ov02c10 *ov02c10 = to_ov02c10(sd);
+	int ret = 0;
+
+	mutex_lock(&ov02c10->mutex);
+	if (!ov02c10->streaming)
+		goto exit;
+
+	ret = ov02c10_start_streaming(ov02c10);
+	if (ret) {
+		ov02c10->streaming = false;
+		ov02c10_stop_streaming(ov02c10);
+	}
+
+exit:
+	mutex_unlock(&ov02c10->mutex);
+	return ret;
+}
+
+static int ov02c10_set_format(struct v4l2_subdev *sd,
+			      struct v4l2_subdev_state *sd_state,
+			      struct v4l2_subdev_format *fmt)
+{
+	struct ov02c10 *ov02c10 = to_ov02c10(sd);
+	const struct ov02c10_mode *mode;
+	s32 vblank_def, h_blank;
+
+	if (ov02c10->mipi_lanes == 1)
+		mode = &supported_modes[0];
+	else
+		mode = &supported_modes[1];
+
+	mutex_lock(&ov02c10->mutex);
+	ov02c10_update_pad_format(mode, &fmt->format);
+	if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) {
+		*v4l2_subdev_state_get_format(sd_state, fmt->pad) = fmt->format;
+	} else {
+		ov02c10->cur_mode = mode;
+		__v4l2_ctrl_s_ctrl(ov02c10->link_freq, mode->link_freq_index);
+		__v4l2_ctrl_s_ctrl_int64(ov02c10->pixel_rate, OV02C10_SCLK);
+
+		/* Update limits and set FPS to default */
+		vblank_def = mode->vts_def - mode->height;
+		__v4l2_ctrl_modify_range(ov02c10->vblank,
+					 mode->vts_min - mode->height,
+					 OV02C10_VTS_MAX - mode->height, 1,
+					 vblank_def);
+		__v4l2_ctrl_s_ctrl(ov02c10->vblank, vblank_def);
+		h_blank = mode->hts - mode->width;
+		__v4l2_ctrl_modify_range(ov02c10->hblank, h_blank, h_blank, 1,
+					 h_blank);
+	}
+	mutex_unlock(&ov02c10->mutex);
+
+	return 0;
+}
+
+static int ov02c10_get_format(struct v4l2_subdev *sd,
+			      struct v4l2_subdev_state *sd_state,
+			      struct v4l2_subdev_format *fmt)
+{
+	struct ov02c10 *ov02c10 = to_ov02c10(sd);
+
+	mutex_lock(&ov02c10->mutex);
+	if (fmt->which == V4L2_SUBDEV_FORMAT_TRY)
+		fmt->format = *v4l2_subdev_state_get_format(sd_state, fmt->pad);
+	else
+		ov02c10_update_pad_format(ov02c10->cur_mode, &fmt->format);
+
+	mutex_unlock(&ov02c10->mutex);
+
+	return 0;
+}
+
+static int ov02c10_enum_mbus_code(struct v4l2_subdev *sd,
+				  struct v4l2_subdev_state *sd_state,
+				  struct v4l2_subdev_mbus_code_enum *code)
+{
+	if (code->index > 0)
+		return -EINVAL;
+
+	code->code = MEDIA_BUS_FMT_SGRBG10_1X10;
+
+	return 0;
+}
+
+static int ov02c10_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))
+		return -EINVAL;
+
+	if (fse->code != MEDIA_BUS_FMT_SGRBG10_1X10)
+		return -EINVAL;
+
+	fse->min_width = supported_modes[fse->index].width;
+	fse->max_width = fse->min_width;
+	fse->min_height = supported_modes[fse->index].height;
+	fse->max_height = fse->min_height;
+
+	return 0;
+}
+
+static int ov02c10_init_state(struct v4l2_subdev *sd,
+			      struct v4l2_subdev_state *sd_state)
+{
+	ov02c10_update_pad_format(&supported_modes[0],
+				  v4l2_subdev_state_get_format(sd_state, 0));
+
+	return 0;
+}
+
+static const struct v4l2_subdev_video_ops ov02c10_video_ops = {
+	.s_stream = ov02c10_set_stream,
+};
+
+static const struct v4l2_subdev_pad_ops ov02c10_pad_ops = {
+	.set_fmt = ov02c10_set_format,
+	.get_fmt = ov02c10_get_format,
+	.enum_mbus_code = ov02c10_enum_mbus_code,
+	.enum_frame_size = ov02c10_enum_frame_size,
+};
+
+static const struct v4l2_subdev_ops ov02c10_subdev_ops = {
+	.video = &ov02c10_video_ops,
+	.pad = &ov02c10_pad_ops,
+};
+
+static const struct media_entity_operations ov02c10_subdev_entity_ops = {
+	.link_validate = v4l2_subdev_link_validate,
+};
+
+static const struct v4l2_subdev_internal_ops ov02c10_internal_ops = {
+	.init_state = ov02c10_init_state,
+};
+
+static int ov02c10_identify_module(struct ov02c10 *ov02c10)
+{
+	struct i2c_client *client = v4l2_get_subdevdata(&ov02c10->sd);
+	u64 chip_id;
+	u32 ret = 0;
+
+	ov02c10->regmap = devm_cci_regmap_init_i2c(client, 16);
+	cci_read(ov02c10->regmap, OV02C10_REG_CHIP_ID, &chip_id, &ret);
+	if (ret)
+		return ret;
+
+	if (chip_id != OV02C10_CHIP_ID) {
+		dev_err(&client->dev, "chip id mismatch: %x!=%llx",
+			OV02C10_CHIP_ID, chip_id);
+		return -ENXIO;
+	}
+
+	return 0;
+}
+
+static int ov02c10_check_hwcfg(struct device *dev, struct ov02c10 *ov02c10)
+{
+	struct v4l2_fwnode_endpoint bus_cfg = {
+		.bus_type = V4L2_MBUS_CSI2_DPHY
+	};
+	struct fwnode_handle *ep;
+	struct fwnode_handle *fwnode = dev_fwnode(dev);
+	unsigned int i, j;
+	int ret;
+	u32 ext_clk;
+
+	if (!fwnode)
+		return -ENXIO;
+
+	ep = fwnode_graph_get_next_endpoint(fwnode, NULL);
+	if (!ep)
+		return -EPROBE_DEFER;
+
+	ret = fwnode_property_read_u32(dev_fwnode(dev), "clock-frequency",
+				       &ext_clk);
+	if (ret) {
+		dev_err(dev, "can't get clock frequency");
+		return ret;
+	}
+
+	ret = v4l2_fwnode_endpoint_alloc_parse(ep, &bus_cfg);
+	fwnode_handle_put(ep);
+	if (ret)
+		return ret;
+
+	if (!bus_cfg.nr_of_link_frequencies) {
+		dev_err(dev, "no link frequencies defined");
+		ret = -EINVAL;
+		goto out_err;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(link_freq_menu_items); i++) {
+		for (j = 0; j < bus_cfg.nr_of_link_frequencies; j++) {
+			if (link_freq_menu_items[i] ==
+				bus_cfg.link_frequencies[j])
+				break;
+		}
+
+		if (j == bus_cfg.nr_of_link_frequencies) {
+			dev_err(dev, "no link frequency %lld supported",
+				link_freq_menu_items[i]);
+			ret = -EINVAL;
+			goto out_err;
+		}
+	}
+
+	if (bus_cfg.bus.mipi_csi2.num_data_lanes != 2 &&
+	    bus_cfg.bus.mipi_csi2.num_data_lanes != 4) {
+		dev_err(dev, "number of CSI2 data lanes %d is not supported",
+			bus_cfg.bus.mipi_csi2.num_data_lanes);
+		return(-EINVAL);
+	}
+	ov02c10->mipi_lanes = bus_cfg.bus.mipi_csi2.num_data_lanes;
+
+out_err:
+	v4l2_fwnode_endpoint_free(&bus_cfg);
+
+	return ret;
+}
+
+static void ov02c10_remove(struct i2c_client *client)
+{
+	struct v4l2_subdev *sd = i2c_get_clientdata(client);
+	struct ov02c10 *ov02c10 = to_ov02c10(sd);
+
+	v4l2_async_unregister_subdev(sd);
+	media_entity_cleanup(&sd->entity);
+	v4l2_ctrl_handler_free(sd->ctrl_handler);
+	pm_runtime_disable(&client->dev);
+	mutex_destroy(&ov02c10->mutex);
+}
+
+static int ov02c10_probe(struct i2c_client *client)
+{
+	struct ov02c10 *ov02c10;
+	int ret = 0;
+
+	ov02c10 = devm_kzalloc(&client->dev, sizeof(*ov02c10), GFP_KERNEL);
+	if (!ov02c10)
+		return -ENOMEM;
+
+	/* Check HW config */
+	ret = ov02c10_check_hwcfg(&client->dev, ov02c10);
+	if (ret) {
+		dev_err(&client->dev, "failed to check hwcfg: %d", ret);
+		return ret;
+	}
+
+	v4l2_i2c_subdev_init(&ov02c10->sd, client, &ov02c10_subdev_ops);
+	ov02c10_get_pm_resources(&client->dev);
+
+	ret = ov02c10_power_on(&client->dev);
+	if (ret) {
+		dev_err_probe(&client->dev, ret, "failed to power on\n");
+		return ret;
+	}
+
+	ret = ov02c10_identify_module(ov02c10);
+	if (ret) {
+		dev_err(&client->dev, "failed to find sensor: %d", ret);
+		goto probe_error_ret;
+	}
+
+	mutex_init(&ov02c10->mutex);
+	ov02c10->cur_mode = &supported_modes[0];
+	if (ov02c10->mipi_lanes == 2)
+		ov02c10->cur_mode = &supported_modes[1];
+	ret = ov02c10_init_controls(ov02c10);
+	if (ret) {
+		dev_err(&client->dev, "failed to init controls: %d", ret);
+		goto probe_error_v4l2_ctrl_handler_free;
+	}
+
+	ov02c10->sd.internal_ops = &ov02c10_internal_ops;
+	ov02c10->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
+	ov02c10->sd.entity.ops = &ov02c10_subdev_entity_ops;
+	ov02c10->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
+	ov02c10->pad.flags = MEDIA_PAD_FL_SOURCE;
+	ret = media_entity_pads_init(&ov02c10->sd.entity, 1, &ov02c10->pad);
+	if (ret) {
+		dev_err(&client->dev, "failed to init entity pads: %d", ret);
+		goto probe_error_v4l2_ctrl_handler_free;
+	}
+
+	ret = v4l2_async_register_subdev_sensor(&ov02c10->sd);
+	if (ret < 0) {
+		dev_err(&client->dev, "failed to register V4L2 subdev: %d",
+			ret);
+		goto probe_error_media_entity_cleanup;
+	}
+
+	/*
+	 * Device is already turned on by i2c-core with ACPI domain PM.
+	 * Enable runtime PM and turn off the device.
+	 */
+	pm_runtime_set_active(&client->dev);
+	pm_runtime_enable(&client->dev);
+	pm_runtime_idle(&client->dev);
+
+	return 0;
+
+probe_error_media_entity_cleanup:
+	media_entity_cleanup(&ov02c10->sd.entity);
+
+probe_error_v4l2_ctrl_handler_free:
+	v4l2_ctrl_handler_free(ov02c10->sd.ctrl_handler);
+	mutex_destroy(&ov02c10->mutex);
+
+probe_error_ret:
+	ov02c10_power_off(&client->dev);
+
+	return ret;
+}
+
+static const struct dev_pm_ops ov02c10_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(ov02c10_suspend, ov02c10_resume)
+	SET_RUNTIME_PM_OPS(ov02c10_power_off, ov02c10_power_on, NULL)
+};
+
+#ifdef CONFIG_ACPI
+static const struct acpi_device_id ov02c10_acpi_ids[] = {
+	{"OVTI02C1"},
+	{}
+};
+
+MODULE_DEVICE_TABLE(acpi, ov02c10_acpi_ids);
+#endif
+
+static struct i2c_driver ov02c10_i2c_driver = {
+	.driver = {
+		.name = "ov02c10",
+		.pm = &ov02c10_pm_ops,
+		.acpi_match_table = ACPI_PTR(ov02c10_acpi_ids),
+	},
+	.probe = ov02c10_probe,
+	.remove = ov02c10_remove,
+};
+
+module_i2c_driver(ov02c10_i2c_driver);
+
+MODULE_AUTHOR("Hao Yao <hao.yao@intel.com>");
+MODULE_DESCRIPTION("OmniVision OV02C10 sensor driver");
+MODULE_LICENSE("GPL");
-- 
2.48.1

[PATCH v8 02/14] media: ov02c10: merge shared register settings into a shared reg_sequence array

[Hans de Goede]

Merge shared register settings into a shared reg_sequence array.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/media/i2c/ov02c10.c | 256 +++++-------------------------------
 1 file changed, 34 insertions(+), 222 deletions(-)

diff --git a/drivers/media/i2c/ov02c10.c b/drivers/media/i2c/ov02c10.c
index 291da9ee1788..f18b48fe8c0d 100644
--- a/drivers/media/i2c/ov02c10.c
+++ b/drivers/media/i2c/ov02c10.c
@@ -85,20 +85,21 @@ struct ov02c10_mode {
 	/* Sensor register settings for this resolution */
 	const struct reg_sequence *reg_sequence;
 	const int sequence_length;
+	/* Sensor register settings for 1 or 2 lane config */
+	const struct reg_sequence *lane_settings;
+	const int lane_settings_length;
 };
 
-static const struct reg_sequence sensor_1928x1092_1lane_30fps_setting[] = {
+static const struct reg_sequence sensor_1928x1092_30fps_setting[] = {
 	{0x0301, 0x08},
 	{0x0303, 0x06},
 	{0x0304, 0x01},
 	{0x0305, 0xe0},
 	{0x0313, 0x40},
 	{0x031c, 0x4f},
-	{0x301b, 0xd2},
 	{0x3020, 0x97},
 	{0x3022, 0x01},
 	{0x3026, 0xb4},
-	{0x3027, 0xe1},
 	{0x303b, 0x00},
 	{0x303c, 0x4f},
 	{0x303d, 0xe6},
@@ -174,10 +175,6 @@ static const struct reg_sequence sensor_1928x1092_1lane_30fps_setting[] = {
 	{0x3809, 0x88},
 	{0x380a, 0x04},
 	{0x380b, 0x44},
-	{0x380c, 0x08},
-	{0x380d, 0xe8},
-	{0x380e, 0x04},
-	{0x380f, 0x8c},
 	{0x3810, 0x00},
 	{0x3811, 0x02},
 	{0x3812, 0x00},
@@ -209,7 +206,6 @@ static const struct reg_sequence sensor_1928x1092_1lane_30fps_setting[] = {
 	{0x394b, 0x06},
 	{0x394c, 0x06},
 	{0x394d, 0x08},
-	{0x394e, 0x0b},
 	{0x394f, 0x01},
 	{0x3950, 0x01},
 	{0x3951, 0x01},
@@ -286,11 +282,9 @@ static const struct reg_sequence sensor_1928x1092_1lane_30fps_setting[] = {
 	{0x450a, 0x04},
 	{0x450e, 0x00},
 	{0x450f, 0x00},
-	{0x4800, 0x24},
 	{0x4900, 0x00},
 	{0x4901, 0x00},
 	{0x4902, 0x01},
-	{0x5000, 0xf5},
 	{0x5001, 0x50},
 	{0x5006, 0x00},
 	{0x5080, 0x40},
@@ -304,6 +298,18 @@ static const struct reg_sequence sensor_1928x1092_1lane_30fps_setting[] = {
 	{0x4815, 0x40},
 	{0x4816, 0x12},
 	{0x4f00, 0x01},
+};
+
+static const struct reg_sequence sensor_1928x1092_30fps_1lane_setting[] = {
+	{0x301b, 0xd2},
+	{0x3027, 0xe1},
+	{0x380c, 0x08},
+	{0x380d, 0xe8},
+	{0x380e, 0x04},
+	{0x380f, 0x8c},
+	{0x394e, 0x0b},
+	{0x4800, 0x24},
+	{0x5000, 0xf5},
 	/* plls */
 	{0x0303, 0x05},
 	{0x0305, 0x90},
@@ -311,211 +317,17 @@ static const struct reg_sequence sensor_1928x1092_1lane_30fps_setting[] = {
 	{0x3016, 0x12},
 };
 
-static const struct reg_sequence sensor_1928x1092_2lane_30fps_setting[] = {
-	{0x0301, 0x08},
-	{0x0303, 0x06},
-	{0x0304, 0x01},
-	{0x0305, 0xe0},
-	{0x0313, 0x40},
-	{0x031c, 0x4f},
+static const struct reg_sequence sensor_1928x1092_30fps_2lane_setting[] = {
 	{0x301b, 0xf0},
-	{0x3020, 0x97},
-	{0x3022, 0x01},
-	{0x3026, 0xb4},
 	{0x3027, 0xf1},
-	{0x303b, 0x00},
-	{0x303c, 0x4f},
-	{0x303d, 0xe6},
-	{0x303e, 0x00},
-	{0x303f, 0x03},
-	{0x3021, 0x23},
-	{0x3501, 0x04},
-	{0x3502, 0x6c},
-	{0x3504, 0x0c},
-	{0x3507, 0x00},
-	{0x3508, 0x08},
-	{0x3509, 0x00},
-	{0x350a, 0x01},
-	{0x350b, 0x00},
-	{0x350c, 0x41},
-	{0x3600, 0x84},
-	{0x3603, 0x08},
-	{0x3610, 0x57},
-	{0x3611, 0x1b},
-	{0x3613, 0x78},
-	{0x3623, 0x00},
-	{0x3632, 0xa0},
-	{0x3642, 0xe8},
-	{0x364c, 0x70},
-	{0x365f, 0x0f},
-	{0x3708, 0x30},
-	{0x3714, 0x24},
-	{0x3725, 0x02},
-	{0x3737, 0x08},
-	{0x3739, 0x28},
-	{0x3749, 0x32},
-	{0x374a, 0x32},
-	{0x374b, 0x32},
-	{0x374c, 0x32},
-	{0x374d, 0x81},
-	{0x374e, 0x81},
-	{0x374f, 0x81},
-	{0x3752, 0x36},
-	{0x3753, 0x36},
-	{0x3754, 0x36},
-	{0x3761, 0x00},
-	{0x376c, 0x81},
-	{0x3774, 0x18},
-	{0x3776, 0x08},
-	{0x377c, 0x81},
-	{0x377d, 0x81},
-	{0x377e, 0x81},
-	{0x37a0, 0x44},
-	{0x37a6, 0x44},
-	{0x37aa, 0x0d},
-	{0x37ae, 0x00},
-	{0x37cb, 0x03},
-	{0x37cc, 0x01},
-	{0x37d8, 0x02},
-	{0x37d9, 0x10},
-	{0x37e1, 0x10},
-	{0x37e2, 0x18},
-	{0x37e3, 0x08},
-	{0x37e4, 0x08},
-	{0x37e5, 0x02},
-	{0x37e6, 0x08},
-
-	/* 1928x1092 */
-	{0x3800, 0x00},
-	{0x3801, 0x00},
-	{0x3802, 0x00},
-	{0x3803, 0x00},
-	{0x3804, 0x07},
-	{0x3805, 0x8f},
-	{0x3806, 0x04},
-	{0x3807, 0x47},
-	{0x3808, 0x07},
-	{0x3809, 0x88},
-	{0x380a, 0x04},
-	{0x380b, 0x44},
 	{0x380c, 0x04},
 	{0x380d, 0x74},
 	{0x380e, 0x09},
 	{0x380f, 0x18},
-	{0x3810, 0x00},
-	{0x3811, 0x02},
-	{0x3812, 0x00},
-	{0x3813, 0x02},
-	{0x3814, 0x01},
-	{0x3815, 0x01},
-	{0x3816, 0x01},
-	{0x3817, 0x01},
-
-	{0x3820, 0xb0},
-	{0x3821, 0x00},
-	{0x3822, 0x80},
-	{0x3823, 0x08},
-	{0x3824, 0x00},
-	{0x3825, 0x20},
-	{0x3826, 0x00},
-	{0x3827, 0x08},
-	{0x382a, 0x00},
-	{0x382b, 0x08},
-	{0x382d, 0x00},
-	{0x382e, 0x00},
-	{0x382f, 0x23},
-	{0x3834, 0x00},
-	{0x3839, 0x00},
-	{0x383a, 0xd1},
-	{0x383e, 0x03},
-	{0x393d, 0x29},
-	{0x393f, 0x6e},
-	{0x394b, 0x06},
-	{0x394c, 0x06},
-	{0x394d, 0x08},
 	{0x394e, 0x0a},
-	{0x394f, 0x01},
-	{0x3950, 0x01},
-	{0x3951, 0x01},
-	{0x3952, 0x01},
-	{0x3953, 0x01},
-	{0x3954, 0x01},
-	{0x3955, 0x01},
-	{0x3956, 0x01},
-	{0x3957, 0x0e},
-	{0x3958, 0x08},
-	{0x3959, 0x08},
-	{0x395a, 0x08},
-	{0x395b, 0x13},
-	{0x395c, 0x09},
-	{0x395d, 0x05},
-	{0x395e, 0x02},
-	{0x395f, 0x00},
-	{0x395f, 0x00},
-	{0x3960, 0x00},
-	{0x3961, 0x00},
-	{0x3962, 0x00},
-	{0x3963, 0x00},
-	{0x3964, 0x00},
-	{0x3965, 0x00},
-	{0x3966, 0x00},
-	{0x3967, 0x00},
-	{0x3968, 0x01},
-	{0x3969, 0x01},
-	{0x396a, 0x01},
-	{0x396b, 0x01},
-	{0x396c, 0x10},
-	{0x396d, 0xf0},
-	{0x396e, 0x11},
-	{0x396f, 0x00},
-	{0x3970, 0x37},
-	{0x3971, 0x37},
-	{0x3972, 0x37},
-	{0x3973, 0x37},
-	{0x3974, 0x00},
-	{0x3975, 0x3c},
-	{0x3976, 0x3c},
-	{0x3977, 0x3c},
-	{0x3978, 0x3c},
-	{0x3c00, 0x0f},
-	{0x3c20, 0x01},
-	{0x3c21, 0x08},
-	{0x3f00, 0x8b},
-	{0x3f02, 0x0f},
-	{0x4000, 0xc3},
-	{0x4001, 0xe0},
-	{0x4002, 0x00},
-	{0x4003, 0x40},
-	{0x4008, 0x04},
-	{0x4009, 0x23},
-	{0x400a, 0x04},
-	{0x400b, 0x01},
 	{0x4041, 0x20},
-	{0x4077, 0x06},
-	{0x4078, 0x00},
-	{0x4079, 0x1a},
-	{0x407a, 0x7f},
-	{0x407b, 0x01},
-	{0x4080, 0x03},
-	{0x4081, 0x84},
-	{0x4308, 0x03},
-	{0x4309, 0xff},
-	{0x430d, 0x00},
-	{0x4806, 0x00},
-	{0x4813, 0x00},
-	{0x4837, 0x10},
-	{0x4857, 0x05},
 	{0x4884, 0x04},
-	{0x4500, 0x07},
-	{0x4501, 0x00},
-	{0x4503, 0x00},
-	{0x450a, 0x04},
-	{0x450e, 0x00},
-	{0x450f, 0x00},
 	{0x4800, 0x64},
-	{0x4900, 0x00},
-	{0x4901, 0x00},
-	{0x4902, 0x01},
 	{0x4d00, 0x03},
 	{0x4d01, 0xd8},
 	{0x4d02, 0xba},
@@ -524,20 +336,7 @@ static const struct reg_sequence sensor_1928x1092_2lane_30fps_setting[] = {
 	{0x4d05, 0x34},
 	{0x4d0d, 0x00},
 	{0x5000, 0xfd},
-	{0x5001, 0x50},
-	{0x5006, 0x00},
-	{0x5080, 0x40},
-	{0x5181, 0x2b},
-	{0x5202, 0xa3},
-	{0x5206, 0x01},
-	{0x5207, 0x00},
-	{0x520a, 0x01},
-	{0x520b, 0x00},
-	{0x365d, 0x00},
-	{0x4815, 0x40},
-	{0x4816, 0x12},
 	{0x481f, 0x30},
-	{0x4f00, 0x01},
 	/* plls */
 	{0x0303, 0x05},
 	{0x0305, 0x90},
@@ -565,8 +364,10 @@ static const struct ov02c10_mode supported_modes[] = {
 		.vts_def = 1164,
 		.vts_min = 1164,
 		.mipi_lanes = 1,
-		.reg_sequence = sensor_1928x1092_1lane_30fps_setting,
-		.sequence_length = ARRAY_SIZE(sensor_1928x1092_1lane_30fps_setting),
+		.reg_sequence = sensor_1928x1092_30fps_setting,
+		.sequence_length = ARRAY_SIZE(sensor_1928x1092_30fps_setting),
+		.lane_settings = sensor_1928x1092_30fps_1lane_setting,
+		.lane_settings_length = ARRAY_SIZE(sensor_1928x1092_30fps_1lane_setting),
 	},
 	{
 		.width = 1928,
@@ -575,8 +376,10 @@ static const struct ov02c10_mode supported_modes[] = {
 		.vts_def = 2328,
 		.vts_min = 2328,
 		.mipi_lanes = 2,
-		.reg_sequence = sensor_1928x1092_2lane_30fps_setting,
-		.sequence_length = ARRAY_SIZE(sensor_1928x1092_2lane_30fps_setting),
+		.reg_sequence = sensor_1928x1092_30fps_setting,
+		.sequence_length = ARRAY_SIZE(sensor_1928x1092_30fps_setting),
+		.lane_settings = sensor_1928x1092_30fps_2lane_setting,
+		.lane_settings_length = ARRAY_SIZE(sensor_1928x1092_30fps_2lane_setting),
 	},
 };
 
@@ -791,6 +594,15 @@ static int ov02c10_start_streaming(struct ov02c10 *ov02c10)
 		return ret;
 	}
 
+	reg_sequence = ov02c10->cur_mode->lane_settings;
+	sequence_length = ov02c10->cur_mode->lane_settings_length;
+	ret = regmap_multi_reg_write(ov02c10->regmap,
+				     reg_sequence, sequence_length);
+	if (ret) {
+		dev_err(&client->dev, "failed to write lane settings\n");
+		return ret;
+	}
+
 	ret = __v4l2_ctrl_handler_setup(ov02c10->sd.ctrl_handler);
 	if (ret)
 		return ret;
-- 
2.48.1

[PATCH v8 03/14] media: ov02c10: Fix hts for 2 lane mode

[Hans de Goede]

Using half the hts value when using 2 lanes results in hts < width,
which results in reporting a negative hblank value to userspace.

The hts value in the mode struct is only used for reporting the hblank
control to userspace so this change does not result in any different
register settings being written to the sensor.

After this change the register-lists are still writing 1140 to the HTS
register of the sensor in 2 lane mode, which seems to be a too low value
for HTS. But maybe the value of this register is multiplied by 2 internally
by the sensor when 2 lanes are used ?

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/media/i2c/ov02c10.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/media/i2c/ov02c10.c b/drivers/media/i2c/ov02c10.c
index f18b48fe8c0d..a33f9d4033e6 100644
--- a/drivers/media/i2c/ov02c10.c
+++ b/drivers/media/i2c/ov02c10.c
@@ -26,6 +26,8 @@
 
 #define OV02C10_REG_STREAM_CONTROL	CCI_REG8(0x0100)
 
+#define OV02C10_REG_HTS			CCI_REG16(0x380c)
+
 /* vertical-timings from sensor */
 #define OV02C10_REG_VTS			CCI_REG16(0x380e)
 #define OV02C10_VTS_MAX			0xffff
@@ -372,7 +374,7 @@ static const struct ov02c10_mode supported_modes[] = {
 	{
 		.width = 1928,
 		.height = 1092,
-		.hts = 1140,
+		.hts = 2280,
 		.vts_def = 2328,
 		.vts_min = 2328,
 		.mipi_lanes = 2,
-- 
2.48.1

[PATCH v8 04/14] media: ov02c10: Fix vts_min for 2 lane mode

[Hans de Goede]

Doubling the default VTS value in 2 lane mode to keep the default fps at 30
fps while doubling the pixelrate make sense. But there is no reason to also
double the minimum VTS value. If userspace wants to use the extra bandwidth
to get more fps (at the expense of the max exposure time) then this should
be allowed.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/media/i2c/ov02c10.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/i2c/ov02c10.c b/drivers/media/i2c/ov02c10.c
index a33f9d4033e6..ad5ab6c8a803 100644
--- a/drivers/media/i2c/ov02c10.c
+++ b/drivers/media/i2c/ov02c10.c
@@ -376,7 +376,7 @@ static const struct ov02c10_mode supported_modes[] = {
 		.height = 1092,
 		.hts = 2280,
 		.vts_def = 2328,
-		.vts_min = 2328,
+		.vts_min = 1164,
 		.mipi_lanes = 2,
 		.reg_sequence = sensor_1928x1092_30fps_setting,
 		.sequence_length = ARRAY_SIZE(sensor_1928x1092_30fps_setting),
-- 
2.48.1

[PATCH v8 05/14] media: ov02c10: link-freq-index and pixel-rate fixes

[Hans de Goede]

link-freq-index and pixel-rate fixes:
- The link_freq_index is (typically) not mode specific move it from
  the mode struct to the ov02c10 struct
- Having one supported link-freq in bus_cfg.link_frequencies[] is enough
  switch to v4l2_link_freq_to_bitmap() to only require one match and store
  the first match in ov02c10->link_freq_index for use when setting up
  the controls
- Use ov02c10->link_freq_index to set the value of the link-freq control
- Note the above are no-ops because currently only 1 link-freq is supported
- Use link-freq + lane-count to calculate the pixelrate instead of hard-
  coding it to 80MHz, this corrects the pixel-rate to 160MHz for 2 lanes
- The link-freq and pixel-rate are set to a fixed value at probe() time and
  never change, drop the unnecessary setting of these controls from
  ov02c10_set_format()

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/media/i2c/ov02c10.c | 52 ++++++++++++++-----------------------
 1 file changed, 19 insertions(+), 33 deletions(-)

diff --git a/drivers/media/i2c/ov02c10.c b/drivers/media/i2c/ov02c10.c
index ad5ab6c8a803..4b1b41f74ca2 100644
--- a/drivers/media/i2c/ov02c10.c
+++ b/drivers/media/i2c/ov02c10.c
@@ -16,9 +16,7 @@
 #include <media/v4l2-fwnode.h>
 
 #define OV02C10_LINK_FREQ_400MHZ	400000000ULL
-#define OV02C10_SCLK			80000000LL
 #define OV02C10_MCLK			19200000
-#define OV02C10_DATA_LANES		1
 #define OV02C10_RGB_DEPTH		10
 
 #define OV02C10_REG_CHIP_ID		CCI_REG16(0x300a)
@@ -78,9 +76,6 @@ struct ov02c10_mode {
 	/* Min vertical timining size */
 	u32 vts_min;
 
-	/* Link frequency needed for this resolution */
-	u32 link_freq_index;
-
 	/* MIPI lanes used */
 	u8 mipi_lanes;
 
@@ -409,7 +404,8 @@ struct ov02c10 {
 	/* To serialize asynchronous callbacks */
 	struct mutex mutex;
 
-	/* MIPI lanes used */
+	/* MIPI lane info */
+	u32 link_freq_index;
 	u8 mipi_lanes;
 
 	/* Streaming on/off */
@@ -506,9 +502,8 @@ static int ov02c10_init_controls(struct ov02c10 *ov02c10)
 {
 	struct v4l2_ctrl_handler *ctrl_hdlr;
 	const struct ov02c10_mode *cur_mode;
-	s64 exposure_max, h_blank;
+	s64 exposure_max, h_blank, pixel_rate;
 	u32 vblank_min, vblank_max, vblank_default;
-	int size;
 	int ret = 0;
 
 	ctrl_hdlr = &ov02c10->ctrl_handler;
@@ -518,19 +513,22 @@ static int ov02c10_init_controls(struct ov02c10 *ov02c10)
 
 	ctrl_hdlr->lock = &ov02c10->mutex;
 	cur_mode = ov02c10->cur_mode;
-	size = ARRAY_SIZE(link_freq_menu_items);
 
 	ov02c10->link_freq = v4l2_ctrl_new_int_menu(ctrl_hdlr,
 						    &ov02c10_ctrl_ops,
 						    V4L2_CID_LINK_FREQ,
-						    size - 1, 0,
+						    ov02c10->link_freq_index, 0,
 						    link_freq_menu_items);
 	if (ov02c10->link_freq)
 		ov02c10->link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY;
 
+	/* MIPI lanes are DDR -> use link-freq * 2 */
+	pixel_rate = link_freq_menu_items[ov02c10->link_freq_index] * 2 *
+		     ov02c10->mipi_lanes / OV02C10_RGB_DEPTH;
+
 	ov02c10->pixel_rate = v4l2_ctrl_new_std(ctrl_hdlr, &ov02c10_ctrl_ops,
 						V4L2_CID_PIXEL_RATE, 0,
-						OV02C10_SCLK, 1, OV02C10_SCLK);
+						pixel_rate, 1, pixel_rate);
 
 	vblank_min = cur_mode->vts_min - cur_mode->height;
 	vblank_max = OV02C10_VTS_MAX - cur_mode->height;
@@ -801,8 +799,6 @@ static int ov02c10_set_format(struct v4l2_subdev *sd,
 		*v4l2_subdev_state_get_format(sd_state, fmt->pad) = fmt->format;
 	} else {
 		ov02c10->cur_mode = mode;
-		__v4l2_ctrl_s_ctrl(ov02c10->link_freq, mode->link_freq_index);
-		__v4l2_ctrl_s_ctrl_int64(ov02c10->pixel_rate, OV02C10_SCLK);
 
 		/* Update limits and set FPS to default */
 		vblank_def = mode->vts_def - mode->height;
@@ -927,9 +923,9 @@ static int ov02c10_check_hwcfg(struct device *dev, struct ov02c10 *ov02c10)
 	};
 	struct fwnode_handle *ep;
 	struct fwnode_handle *fwnode = dev_fwnode(dev);
-	unsigned int i, j;
-	int ret;
+	unsigned long link_freq_bitmap;
 	u32 ext_clk;
+	int ret;
 
 	if (!fwnode)
 		return -ENXIO;
@@ -950,26 +946,16 @@ static int ov02c10_check_hwcfg(struct device *dev, struct ov02c10 *ov02c10)
 	if (ret)
 		return ret;
 
-	if (!bus_cfg.nr_of_link_frequencies) {
-		dev_err(dev, "no link frequencies defined");
-		ret = -EINVAL;
+	ret = v4l2_link_freq_to_bitmap(dev, bus_cfg.link_frequencies,
+				       bus_cfg.nr_of_link_frequencies,
+				       link_freq_menu_items,
+				       ARRAY_SIZE(link_freq_menu_items),
+				       &link_freq_bitmap);
+	if (ret)
 		goto out_err;
-	}
 
-	for (i = 0; i < ARRAY_SIZE(link_freq_menu_items); i++) {
-		for (j = 0; j < bus_cfg.nr_of_link_frequencies; j++) {
-			if (link_freq_menu_items[i] ==
-				bus_cfg.link_frequencies[j])
-				break;
-		}
-
-		if (j == bus_cfg.nr_of_link_frequencies) {
-			dev_err(dev, "no link frequency %lld supported",
-				link_freq_menu_items[i]);
-			ret = -EINVAL;
-			goto out_err;
-		}
-	}
+	/* v4l2_link_freq_to_bitmap() guarantees at least 1 bit is set */
+	ov02c10->link_freq_index = ffs(link_freq_bitmap) - 1;
 
 	if (bus_cfg.bus.mipi_csi2.num_data_lanes != 2 &&
 	    bus_cfg.bus.mipi_csi2.num_data_lanes != 4) {
-- 
2.48.1

[PATCH v8 06/14] media: ov02c10: ov02c10_check_hwcfg() improvements

[Hans de Goede]

ov02c10_check_hwcfg() improvements:
- Drop unnecessary return -ENXIO when there is no fwnode, this is already
  caught by the fwnode_graph_get_next_endpoint() call
- Use dev_err_probe() in ov02c10_check_hwcfg()
- Make sure all error messages have '\n' at the end
- Add missing fwnode_handle_put() on clock-frequency read errors
- Check clock-frequency matches OV02C10_MCLK
- Log an error on v4l2_fwnode_endpoint_alloc_parse() failure
- ov02c10 code supports 1 or 2 lane setups not 2 or 4 lane setups
- replace return -EINVAL no mipi-lanes mismatch with
  goto check_hwcfg_error to properly free the bus_cfg
- Don't log an error in probe() when ov02c10_check_hwcfg() fails, in all
  cases ov02c10_check_hwcfg() already logs an error using dev_probe_err()
  and if the error is -EPROBE_DEFER then we don't want to keep logging that
  multiple times until the dependency is resolved (dev_probe_err()
  suppresses logging for -EPROBE_DEFER errors)

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/media/i2c/ov02c10.c | 55 +++++++++++++++++++++----------------
 1 file changed, 31 insertions(+), 24 deletions(-)

diff --git a/drivers/media/i2c/ov02c10.c b/drivers/media/i2c/ov02c10.c
index 4b1b41f74ca2..a6ea747243e6 100644
--- a/drivers/media/i2c/ov02c10.c
+++ b/drivers/media/i2c/ov02c10.c
@@ -921,30 +921,38 @@ static int ov02c10_check_hwcfg(struct device *dev, struct ov02c10 *ov02c10)
 	struct v4l2_fwnode_endpoint bus_cfg = {
 		.bus_type = V4L2_MBUS_CSI2_DPHY
 	};
-	struct fwnode_handle *ep;
-	struct fwnode_handle *fwnode = dev_fwnode(dev);
+	struct fwnode_handle *ep, *fwnode = dev_fwnode(dev);
 	unsigned long link_freq_bitmap;
-	u32 ext_clk;
+	u32 mclk;
 	int ret;
 
-	if (!fwnode)
-		return -ENXIO;
-
-	ep = fwnode_graph_get_next_endpoint(fwnode, NULL);
+	/*
+	 * Sometimes the fwnode graph is initialized by the bridge driver,
+	 * wait for this.
+	 */
+	ep = fwnode_graph_get_endpoint_by_id(fwnode, 0, 0, 0);
 	if (!ep)
-		return -EPROBE_DEFER;
+		return dev_err_probe(dev, -EPROBE_DEFER,
+				     "waiting for fwnode graph endpoint\n");
 
-	ret = fwnode_property_read_u32(dev_fwnode(dev), "clock-frequency",
-				       &ext_clk);
+	ret = fwnode_property_read_u32(fwnode, "clock-frequency", &mclk);
 	if (ret) {
-		dev_err(dev, "can't get clock frequency");
-		return ret;
+		fwnode_handle_put(ep);
+		return dev_err_probe(dev, ret,
+				     "reading clock-frequency property\n");
+	}
+
+	if (mclk != OV02C10_MCLK) {
+		fwnode_handle_put(ep);
+		return dev_err_probe(dev, -EINVAL,
+				     "external clock %u is not supported\n",
+				     mclk);
 	}
 
 	ret = v4l2_fwnode_endpoint_alloc_parse(ep, &bus_cfg);
 	fwnode_handle_put(ep);
 	if (ret)
-		return ret;
+		return dev_err_probe(dev, ret, "parsing endpoint failed\n");
 
 	ret = v4l2_link_freq_to_bitmap(dev, bus_cfg.link_frequencies,
 				       bus_cfg.nr_of_link_frequencies,
@@ -952,22 +960,23 @@ static int ov02c10_check_hwcfg(struct device *dev, struct ov02c10 *ov02c10)
 				       ARRAY_SIZE(link_freq_menu_items),
 				       &link_freq_bitmap);
 	if (ret)
-		goto out_err;
+		goto check_hwcfg_error;
 
 	/* v4l2_link_freq_to_bitmap() guarantees at least 1 bit is set */
 	ov02c10->link_freq_index = ffs(link_freq_bitmap) - 1;
 
-	if (bus_cfg.bus.mipi_csi2.num_data_lanes != 2 &&
-	    bus_cfg.bus.mipi_csi2.num_data_lanes != 4) {
-		dev_err(dev, "number of CSI2 data lanes %d is not supported",
-			bus_cfg.bus.mipi_csi2.num_data_lanes);
-		return(-EINVAL);
+	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,
+				    "number of CSI2 data lanes %u is not supported\n",
+				    bus_cfg.bus.mipi_csi2.num_data_lanes);
+		goto check_hwcfg_error;
 	}
+
 	ov02c10->mipi_lanes = bus_cfg.bus.mipi_csi2.num_data_lanes;
 
-out_err:
+check_hwcfg_error:
 	v4l2_fwnode_endpoint_free(&bus_cfg);
-
 	return ret;
 }
 
@@ -994,10 +1003,8 @@ static int ov02c10_probe(struct i2c_client *client)
 
 	/* Check HW config */
 	ret = ov02c10_check_hwcfg(&client->dev, ov02c10);
-	if (ret) {
-		dev_err(&client->dev, "failed to check hwcfg: %d", ret);
+	if (ret)
 		return ret;
-	}
 
 	v4l2_i2c_subdev_init(&ov02c10->sd, client, &ov02c10_subdev_ops);
 	ov02c10_get_pm_resources(&client->dev);
-- 
2.48.1

[PATCH v8 07/14] media: ov02c10: CCI usage fixes

[Hans de Goede]

Several fixes to ov02c10's usage of the CCI helpers:
- Fix indentation of some cci_*() calls
- Make sure logged register writing errors end with '\n'
- CCI functions already log errors themselves, drop error
  logging for them
- CCI functions being passed &ret as last argument can be chained
  without need to check ret in between, if ret != 0 then the next
  CCI call(s) will be a no-op
- err/&ret argument passed to cci_*() functions should be signed
- Move devm_cci_regmap_init_i2c() to ov02c10_probe() and add error check

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/media/i2c/ov02c10.c | 30 ++++++++++--------------------
 1 file changed, 10 insertions(+), 20 deletions(-)

diff --git a/drivers/media/i2c/ov02c10.c b/drivers/media/i2c/ov02c10.c
index a6ea747243e6..b9f28368e29f 100644
--- a/drivers/media/i2c/ov02c10.c
+++ b/drivers/media/i2c/ov02c10.c
@@ -423,16 +423,12 @@ static int ov02c10_test_pattern(struct ov02c10 *ov02c10, int pattern)
 
 	if (!pattern)
 		return cci_update_bits(ov02c10->regmap, OV02C10_REG_TEST_PATTERN,
-			BIT(7), 0, NULL);
+				       BIT(7), 0, NULL);
 
 	cci_update_bits(ov02c10->regmap, OV02C10_REG_TEST_PATTERN,
 			0x03, pattern - 1, &ret);
-	if (ret)
-		return ret;
-
 	cci_update_bits(ov02c10->regmap, OV02C10_REG_TEST_PATTERN,
 			BIT(7), OV02C10_TEST_PATTERN_ENABLE, &ret);
-
 	return ret;
 }
 
@@ -590,7 +586,7 @@ static int ov02c10_start_streaming(struct ov02c10 *ov02c10)
 	ret = regmap_multi_reg_write(ov02c10->regmap,
 				     reg_sequence, sequence_length);
 	if (ret) {
-		dev_err(&client->dev, "failed to set mode");
+		dev_err(&client->dev, "failed to set mode\n");
 		return ret;
 	}
 
@@ -607,21 +603,12 @@ static int ov02c10_start_streaming(struct ov02c10 *ov02c10)
 	if (ret)
 		return ret;
 
-	ret = cci_write(ov02c10->regmap, OV02C10_REG_STREAM_CONTROL, 1, NULL);
-	if (ret)
-		dev_err(&client->dev, "failed to start streaming");
-
-	return ret;
+	return cci_write(ov02c10->regmap, OV02C10_REG_STREAM_CONTROL, 1, NULL);
 }
 
 static void ov02c10_stop_streaming(struct ov02c10 *ov02c10)
 {
-	struct i2c_client *client = v4l2_get_subdevdata(&ov02c10->sd);
-	int ret = 0;
-
-	ret = cci_write(ov02c10->regmap, OV02C10_REG_STREAM_CONTROL, 0, NULL);
-	if (ret)
-		dev_err(&client->dev, "failed to stop streaming");
+	cci_write(ov02c10->regmap, OV02C10_REG_STREAM_CONTROL, 0, NULL);
 }
 
 static int ov02c10_set_stream(struct v4l2_subdev *sd, int enable)
@@ -900,10 +887,9 @@ static int ov02c10_identify_module(struct ov02c10 *ov02c10)
 {
 	struct i2c_client *client = v4l2_get_subdevdata(&ov02c10->sd);
 	u64 chip_id;
-	u32 ret = 0;
+	int ret;
 
-	ov02c10->regmap = devm_cci_regmap_init_i2c(client, 16);
-	cci_read(ov02c10->regmap, OV02C10_REG_CHIP_ID, &chip_id, &ret);
+	ret = cci_read(ov02c10->regmap, OV02C10_REG_CHIP_ID, &chip_id, NULL);
 	if (ret)
 		return ret;
 
@@ -1009,6 +995,10 @@ static int ov02c10_probe(struct i2c_client *client)
 	v4l2_i2c_subdev_init(&ov02c10->sd, client, &ov02c10_subdev_ops);
 	ov02c10_get_pm_resources(&client->dev);
 
+	ov02c10->regmap = devm_cci_regmap_init_i2c(client, 16);
+	if (IS_ERR(ov02c10->regmap))
+		return PTR_ERR(ov02c10->regmap);
+
 	ret = ov02c10_power_on(&client->dev);
 	if (ret) {
 		dev_err_probe(&client->dev, ret, "failed to power on\n");
-- 
2.48.1

[PATCH v8 08/14] media: ov02c10: Make modes lane-count independent

[Hans de Goede]

ATM the driver only supports one mode (1928x1092) but before this change
the supported_modes[] had 2 entries, 1 for 1928x1092 when using 1 mipi lane
and 1 for 1928x1092 when using 2 mipi lanes.

This causes enum_framesizes returning 2 1928x1092 modes, instead make it
one mode and dynamically adapt for the number of lanes in
ov02c10_start_streaming() and ov02c10_set_format().

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/media/i2c/ov02c10.c | 63 ++++++++++++++++---------------------
 1 file changed, 27 insertions(+), 36 deletions(-)

diff --git a/drivers/media/i2c/ov02c10.c b/drivers/media/i2c/ov02c10.c
index b9f28368e29f..e1013d1da459 100644
--- a/drivers/media/i2c/ov02c10.c
+++ b/drivers/media/i2c/ov02c10.c
@@ -70,21 +70,15 @@ struct ov02c10_mode {
 	/* Horizontal timining size */
 	u32 hts;
 
-	/* Default vertical timining size */
-	u32 vts_def;
-
 	/* Min vertical timining size */
 	u32 vts_min;
 
-	/* MIPI lanes used */
-	u8 mipi_lanes;
-
 	/* Sensor register settings for this resolution */
 	const struct reg_sequence *reg_sequence;
 	const int sequence_length;
 	/* Sensor register settings for 1 or 2 lane config */
-	const struct reg_sequence *lane_settings;
-	const int lane_settings_length;
+	const struct reg_sequence *lane_settings[2];
+	const int lane_settings_length[2];
 };
 
 static const struct reg_sequence sensor_1928x1092_30fps_setting[] = {
@@ -358,25 +352,17 @@ static const struct ov02c10_mode supported_modes[] = {
 		.width = 1928,
 		.height = 1092,
 		.hts = 2280,
-		.vts_def = 1164,
 		.vts_min = 1164,
-		.mipi_lanes = 1,
 		.reg_sequence = sensor_1928x1092_30fps_setting,
 		.sequence_length = ARRAY_SIZE(sensor_1928x1092_30fps_setting),
-		.lane_settings = sensor_1928x1092_30fps_1lane_setting,
-		.lane_settings_length = ARRAY_SIZE(sensor_1928x1092_30fps_1lane_setting),
-	},
-	{
-		.width = 1928,
-		.height = 1092,
-		.hts = 2280,
-		.vts_def = 2328,
-		.vts_min = 1164,
-		.mipi_lanes = 2,
-		.reg_sequence = sensor_1928x1092_30fps_setting,
-		.sequence_length = ARRAY_SIZE(sensor_1928x1092_30fps_setting),
-		.lane_settings = sensor_1928x1092_30fps_2lane_setting,
-		.lane_settings_length = ARRAY_SIZE(sensor_1928x1092_30fps_2lane_setting),
+		.lane_settings = {
+			sensor_1928x1092_30fps_1lane_setting,
+			sensor_1928x1092_30fps_2lane_setting
+		},
+		.lane_settings_length = {
+			ARRAY_SIZE(sensor_1928x1092_30fps_1lane_setting),
+			ARRAY_SIZE(sensor_1928x1092_30fps_2lane_setting),
+		},
 	},
 };
 
@@ -499,7 +485,7 @@ static int ov02c10_init_controls(struct ov02c10 *ov02c10)
 	struct v4l2_ctrl_handler *ctrl_hdlr;
 	const struct ov02c10_mode *cur_mode;
 	s64 exposure_max, h_blank, pixel_rate;
-	u32 vblank_min, vblank_max, vblank_default;
+	u32 vblank_min, vblank_max, vblank_default, vts_def;
 	int ret = 0;
 
 	ctrl_hdlr = &ov02c10->ctrl_handler;
@@ -526,9 +512,15 @@ static int ov02c10_init_controls(struct ov02c10 *ov02c10)
 						V4L2_CID_PIXEL_RATE, 0,
 						pixel_rate, 1, pixel_rate);
 
+	/*
+	 * For default multiple min by number of lanes to keep the default
+	 * FPS the same indepenedent of the lane count.
+	 */
+	vts_def = cur_mode->vts_min * ov02c10->mipi_lanes;
+
 	vblank_min = cur_mode->vts_min - cur_mode->height;
 	vblank_max = OV02C10_VTS_MAX - cur_mode->height;
-	vblank_default = cur_mode->vts_def - cur_mode->height;
+	vblank_default = vts_def - cur_mode->height;
 	ov02c10->vblank = v4l2_ctrl_new_std(ctrl_hdlr, &ov02c10_ctrl_ops,
 					    V4L2_CID_VBLANK, vblank_min,
 					    vblank_max, 1, vblank_default);
@@ -546,7 +538,7 @@ static int ov02c10_init_controls(struct ov02c10 *ov02c10)
 	v4l2_ctrl_new_std(ctrl_hdlr, &ov02c10_ctrl_ops, V4L2_CID_DIGITAL_GAIN,
 			  OV02C10_DGTL_GAIN_MIN, OV02C10_DGTL_GAIN_MAX,
 			  OV02C10_DGTL_GAIN_STEP, OV02C10_DGTL_GAIN_DEFAULT);
-	exposure_max = cur_mode->vts_def - OV02C10_EXPOSURE_MAX_MARGIN;
+	exposure_max = vts_def - OV02C10_EXPOSURE_MAX_MARGIN;
 	ov02c10->exposure = v4l2_ctrl_new_std(ctrl_hdlr, &ov02c10_ctrl_ops,
 					      V4L2_CID_EXPOSURE,
 					      OV02C10_EXPOSURE_MIN,
@@ -590,8 +582,8 @@ static int ov02c10_start_streaming(struct ov02c10 *ov02c10)
 		return ret;
 	}
 
-	reg_sequence = ov02c10->cur_mode->lane_settings;
-	sequence_length = ov02c10->cur_mode->lane_settings_length;
+	reg_sequence = ov02c10->cur_mode->lane_settings[ov02c10->mipi_lanes - 1];
+	sequence_length = ov02c10->cur_mode->lane_settings_length[ov02c10->mipi_lanes - 1];
 	ret = regmap_multi_reg_write(ov02c10->regmap,
 				     reg_sequence, sequence_length);
 	if (ret) {
@@ -775,10 +767,10 @@ static int ov02c10_set_format(struct v4l2_subdev *sd,
 	const struct ov02c10_mode *mode;
 	s32 vblank_def, h_blank;
 
-	if (ov02c10->mipi_lanes == 1)
-		mode = &supported_modes[0];
-	else
-		mode = &supported_modes[1];
+	mode = v4l2_find_nearest_size(supported_modes,
+				      ARRAY_SIZE(supported_modes), width,
+				      height, fmt->format.width,
+				      fmt->format.height);
 
 	mutex_lock(&ov02c10->mutex);
 	ov02c10_update_pad_format(mode, &fmt->format);
@@ -788,7 +780,7 @@ static int ov02c10_set_format(struct v4l2_subdev *sd,
 		ov02c10->cur_mode = mode;
 
 		/* Update limits and set FPS to default */
-		vblank_def = mode->vts_def - mode->height;
+		vblank_def = mode->vts_min * ov02c10->mipi_lanes - mode->height;
 		__v4l2_ctrl_modify_range(ov02c10->vblank,
 					 mode->vts_min - mode->height,
 					 OV02C10_VTS_MAX - mode->height, 1,
@@ -1013,8 +1005,7 @@ static int ov02c10_probe(struct i2c_client *client)
 
 	mutex_init(&ov02c10->mutex);
 	ov02c10->cur_mode = &supported_modes[0];
-	if (ov02c10->mipi_lanes == 2)
-		ov02c10->cur_mode = &supported_modes[1];
+
 	ret = ov02c10_init_controls(ov02c10);
 	if (ret) {
 		dev_err(&client->dev, "failed to init controls: %d", ret);
-- 
2.48.1

[PATCH v8 09/14] media: ov02c10: Drop handshake pin support

[Hans de Goede]

The handshake GPIO is not a sensor GPIO but is related to the special
Lattice MIPI aggregator chip found on some Intel IPU6/IPU7 laptops.

Since this is not a sensor GPIO it should not be handled by the sensor
driver. See here for the alternative plan to handle this:

https://lore.kernel.org/linux-media/4b87a956-a767-48dc-b98b-f80d9a44adc8@redhat.com/

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/media/i2c/ov02c10.c | 19 +++----------------
 1 file changed, 3 insertions(+), 16 deletions(-)

diff --git a/drivers/media/i2c/ov02c10.c b/drivers/media/i2c/ov02c10.c
index e1013d1da459..38918b1b6a95 100644
--- a/drivers/media/i2c/ov02c10.c
+++ b/drivers/media/i2c/ov02c10.c
@@ -382,7 +382,6 @@ struct ov02c10 {
 	struct clk *img_clk;
 	struct regulator *avdd;
 	struct gpio_desc *reset;
-	struct gpio_desc *handshake;
 
 	/* Current mode */
 	const struct ov02c10_mode *cur_mode;
@@ -650,12 +649,6 @@ static int ov02c10_get_pm_resources(struct device *dev)
 		return dev_err_probe(dev, PTR_ERR(ov02c10->reset),
 				     "failed to get reset gpio\n");
 
-	ov02c10->handshake = devm_gpiod_get_optional(dev, "handshake",
-						     GPIOD_OUT_LOW);
-	if (IS_ERR(ov02c10->handshake))
-		return dev_err_probe(dev, PTR_ERR(ov02c10->handshake),
-				     "failed to get handshake gpio\n");
-
 	ov02c10->img_clk = devm_clk_get_optional(dev, NULL);
 	if (IS_ERR(ov02c10->img_clk))
 		return dev_err_probe(dev, PTR_ERR(ov02c10->img_clk),
@@ -680,7 +673,6 @@ static int ov02c10_power_off(struct device *dev)
 	int ret = 0;
 
 	gpiod_set_value_cansleep(ov02c10->reset, 1);
-	gpiod_set_value_cansleep(ov02c10->handshake, 0);
 
 	if (ov02c10->avdd)
 		ret = regulator_disable(ov02c10->avdd);
@@ -710,16 +702,11 @@ static int ov02c10_power_on(struct device *dev)
 			return ret;
 		}
 	}
-	gpiod_set_value_cansleep(ov02c10->handshake, 1);
+
 	gpiod_set_value_cansleep(ov02c10->reset, 0);
+	usleep_range(1500, 1800);
 
-	/* Lattice MIPI aggregator with some version FW needs longer delay
-	 * after handshake triggered. We set 25ms as a safe value and wait
-	 * for a stable version FW.
-	 */
-	msleep_interruptible(25);
-
-	return ret;
+	return 0;
 }
 
 static int __maybe_unused ov02c10_suspend(struct device *dev)
-- 
2.48.1

[PATCH v8 10/14] media: ov02c10: ov02c10_get_pm_resources() fixes

[Hans de Goede]

A set of ov02c10_get_pm_resources() fixes:

1. Reset should only be de-asserted after enabling the regulators and
   clocks. Request the reset GPIO with GPIOD_OUT_HIGH and on success
   sleep for 1 ms to ensure that it is asserted for at least 1 ms
   before ov02c10_power_on() de-asserts it.

2. Use plain devm_regulator_get() for avdd.

3. Add error checking to the ov02c10_get_pm_resources() call in probe(),
   it may fail with -EPROBE_DEFER.

4. While at it move the v4l2_i2c_subdev_init() call to directly after
   allocating the ov02c10 struct, it has nothing to do with
   the ov02c10_get_pm_resources() call.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/media/i2c/ov02c10.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/media/i2c/ov02c10.c b/drivers/media/i2c/ov02c10.c
index 38918b1b6a95..a46cacf301a2 100644
--- a/drivers/media/i2c/ov02c10.c
+++ b/drivers/media/i2c/ov02c10.c
@@ -642,26 +642,23 @@ static int ov02c10_get_pm_resources(struct device *dev)
 {
 	struct v4l2_subdev *sd = dev_get_drvdata(dev);
 	struct ov02c10 *ov02c10 = to_ov02c10(sd);
-	int ret;
 
-	ov02c10->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
+	ov02c10->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
 	if (IS_ERR(ov02c10->reset))
 		return dev_err_probe(dev, PTR_ERR(ov02c10->reset),
 				     "failed to get reset gpio\n");
+	if (ov02c10->reset)
+		fsleep(1000);
 
 	ov02c10->img_clk = devm_clk_get_optional(dev, NULL);
 	if (IS_ERR(ov02c10->img_clk))
 		return dev_err_probe(dev, PTR_ERR(ov02c10->img_clk),
 				     "failed to get imaging clock\n");
 
-	ov02c10->avdd = devm_regulator_get_optional(dev, "avdd");
-	if (IS_ERR(ov02c10->avdd)) {
-		ret = PTR_ERR(ov02c10->avdd);
-		ov02c10->avdd = NULL;
-		if (ret != -ENODEV)
-			return dev_err_probe(dev, ret,
-					     "failed to get avdd regulator\n");
-	}
+	ov02c10->avdd = devm_regulator_get(dev, "avdd");
+	if (IS_ERR(ov02c10->avdd))
+		return dev_err_probe(dev, PTR_ERR(ov02c10->avdd),
+				     "failed to get avdd regulator\n");
 
 	return 0;
 }
@@ -966,13 +963,16 @@ static int ov02c10_probe(struct i2c_client *client)
 	if (!ov02c10)
 		return -ENOMEM;
 
+	v4l2_i2c_subdev_init(&ov02c10->sd, client, &ov02c10_subdev_ops);
+
 	/* Check HW config */
 	ret = ov02c10_check_hwcfg(&client->dev, ov02c10);
 	if (ret)
 		return ret;
 
-	v4l2_i2c_subdev_init(&ov02c10->sd, client, &ov02c10_subdev_ops);
-	ov02c10_get_pm_resources(&client->dev);
+	ret = ov02c10_get_pm_resources(&client->dev);
+	if (ret)
+		return ret;
 
 	ov02c10->regmap = devm_cci_regmap_init_i2c(client, 16);
 	if (IS_ERR(ov02c10->regmap))
-- 
2.48.1

[PATCH v8 11/14] media: ov02c10: Switch to {enable,disable}_streams

[Hans de Goede]

Switch from s_stream() to enable_streams() and disable_streams() pad
operations. They are preferred and required for streams support.

Note this also stops calling ov02c10_stop_streaming() on enable_streams()
errors. If ov02c10_start_streaming() fails OV02C10_REG_STREAM_CONTROL bit 0
will have never been set so there is no need to clear it on errors.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/media/i2c/ov02c10.c | 59 +++++++++++++++++++++----------------
 1 file changed, 33 insertions(+), 26 deletions(-)

diff --git a/drivers/media/i2c/ov02c10.c b/drivers/media/i2c/ov02c10.c
index a46cacf301a2..da727e18a282 100644
--- a/drivers/media/i2c/ov02c10.c
+++ b/drivers/media/i2c/ov02c10.c
@@ -2,6 +2,7 @@
 // Copyright (c) 2022 Intel Corporation.
 
 #include <linux/acpi.h>
+#include <linux/cleanup.h>
 #include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/gpio/consumer.h>
@@ -602,41 +603,45 @@ static void ov02c10_stop_streaming(struct ov02c10 *ov02c10)
 	cci_write(ov02c10->regmap, OV02C10_REG_STREAM_CONTROL, 0, NULL);
 }
 
-static int ov02c10_set_stream(struct v4l2_subdev *sd, int enable)
+static int ov02c10_enable_streams(struct v4l2_subdev *sd,
+				  struct v4l2_subdev_state *state,
+				  u32 pad, u64 streams_mask)
 {
-	struct ov02c10 *ov02c10 = to_ov02c10(sd);
 	struct i2c_client *client = v4l2_get_subdevdata(sd);
-	int ret = 0;
+	struct ov02c10 *ov02c10 = to_ov02c10(sd);
+	int ret;
 
-	if (ov02c10->streaming == enable)
-		return 0;
+	guard(mutex)(&ov02c10->mutex);
 
-	mutex_lock(&ov02c10->mutex);
-	if (enable) {
-		ret = pm_runtime_get_sync(&client->dev);
-		if (ret < 0) {
-			pm_runtime_put_noidle(&client->dev);
-			mutex_unlock(&ov02c10->mutex);
-			return ret;
-		}
+	ret = pm_runtime_resume_and_get(&client->dev);
+	if (ret)
+		return ret;
 
-		ret = ov02c10_start_streaming(ov02c10);
-		if (ret) {
-			enable = 0;
-			ov02c10_stop_streaming(ov02c10);
-			pm_runtime_put(&client->dev);
-		}
-	} else {
-		ov02c10_stop_streaming(ov02c10);
+	ret = ov02c10_start_streaming(ov02c10);
+	if (ret == 0)
+		ov02c10->streaming = true;
+	else
 		pm_runtime_put(&client->dev);
-	}
-
-	ov02c10->streaming = enable;
-	mutex_unlock(&ov02c10->mutex);
 
 	return ret;
 }
 
+static int ov02c10_disable_streams(struct v4l2_subdev *sd,
+				   struct v4l2_subdev_state *state,
+				   u32 pad, u64 streams_mask)
+{
+	struct i2c_client *client = v4l2_get_subdevdata(sd);
+	struct ov02c10 *ov02c10 = to_ov02c10(sd);
+
+	guard(mutex)(&ov02c10->mutex);
+
+	ov02c10_stop_streaming(ov02c10);
+	ov02c10->streaming = false;
+	pm_runtime_put(&client->dev);
+
+	return 0;
+}
+
 /* This function tries to get power control resources */
 static int ov02c10_get_pm_resources(struct device *dev)
 {
@@ -836,7 +841,7 @@ static int ov02c10_init_state(struct v4l2_subdev *sd,
 }
 
 static const struct v4l2_subdev_video_ops ov02c10_video_ops = {
-	.s_stream = ov02c10_set_stream,
+	.s_stream = v4l2_subdev_s_stream_helper,
 };
 
 static const struct v4l2_subdev_pad_ops ov02c10_pad_ops = {
@@ -844,6 +849,8 @@ static const struct v4l2_subdev_pad_ops ov02c10_pad_ops = {
 	.get_fmt = ov02c10_get_format,
 	.enum_mbus_code = ov02c10_enum_mbus_code,
 	.enum_frame_size = ov02c10_enum_frame_size,
+	.enable_streams = ov02c10_enable_streams,
+	.disable_streams = ov02c10_disable_streams,
 };
 
 static const struct v4l2_subdev_ops ov02c10_subdev_ops = {
-- 
2.48.1

[PATCH v8 12/14] media: ov02c10: Drop system suspend and resume handlers

[Hans de Goede]

Stopping streaming on a camera pipeline at system suspend time, and
restarting it at system resume time, requires coordinated action between
the bridge driver and the camera sensor driver. This is handled by the
bridge driver calling the sensor's .s_stream() handler at system suspend
and resume time. There is thus no need for the sensor to independently
implement system sleep PM operations. Drop them.

The streaming field of the driver's private structure is now unused,
drop it as well.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/media/i2c/ov02c10.c | 53 +++----------------------------------
 1 file changed, 4 insertions(+), 49 deletions(-)

diff --git a/drivers/media/i2c/ov02c10.c b/drivers/media/i2c/ov02c10.c
index da727e18a282..09e70ffcf07a 100644
--- a/drivers/media/i2c/ov02c10.c
+++ b/drivers/media/i2c/ov02c10.c
@@ -393,9 +393,6 @@ struct ov02c10 {
 	/* MIPI lane info */
 	u32 link_freq_index;
 	u8 mipi_lanes;
-
-	/* Streaming on/off */
-	bool streaming;
 };
 
 static inline struct ov02c10 *to_ov02c10(struct v4l2_subdev *subdev)
@@ -618,9 +615,7 @@ static int ov02c10_enable_streams(struct v4l2_subdev *sd,
 		return ret;
 
 	ret = ov02c10_start_streaming(ov02c10);
-	if (ret == 0)
-		ov02c10->streaming = true;
-	else
+	if (ret)
 		pm_runtime_put(&client->dev);
 
 	return ret;
@@ -636,7 +631,6 @@ static int ov02c10_disable_streams(struct v4l2_subdev *sd,
 	guard(mutex)(&ov02c10->mutex);
 
 	ov02c10_stop_streaming(ov02c10);
-	ov02c10->streaming = false;
 	pm_runtime_put(&client->dev);
 
 	return 0;
@@ -711,43 +705,6 @@ static int ov02c10_power_on(struct device *dev)
 	return 0;
 }
 
-static int __maybe_unused ov02c10_suspend(struct device *dev)
-{
-	struct i2c_client *client = to_i2c_client(dev);
-	struct v4l2_subdev *sd = i2c_get_clientdata(client);
-	struct ov02c10 *ov02c10 = to_ov02c10(sd);
-
-	mutex_lock(&ov02c10->mutex);
-	if (ov02c10->streaming)
-		ov02c10_stop_streaming(ov02c10);
-
-	mutex_unlock(&ov02c10->mutex);
-
-	return 0;
-}
-
-static int __maybe_unused ov02c10_resume(struct device *dev)
-{
-	struct i2c_client *client = to_i2c_client(dev);
-	struct v4l2_subdev *sd = i2c_get_clientdata(client);
-	struct ov02c10 *ov02c10 = to_ov02c10(sd);
-	int ret = 0;
-
-	mutex_lock(&ov02c10->mutex);
-	if (!ov02c10->streaming)
-		goto exit;
-
-	ret = ov02c10_start_streaming(ov02c10);
-	if (ret) {
-		ov02c10->streaming = false;
-		ov02c10_stop_streaming(ov02c10);
-	}
-
-exit:
-	mutex_unlock(&ov02c10->mutex);
-	return ret;
-}
-
 static int ov02c10_set_format(struct v4l2_subdev *sd,
 			      struct v4l2_subdev_state *sd_state,
 			      struct v4l2_subdev_format *fmt)
@@ -1047,10 +1004,8 @@ static int ov02c10_probe(struct i2c_client *client)
 	return ret;
 }
 
-static const struct dev_pm_ops ov02c10_pm_ops = {
-	SET_SYSTEM_SLEEP_PM_OPS(ov02c10_suspend, ov02c10_resume)
-	SET_RUNTIME_PM_OPS(ov02c10_power_off, ov02c10_power_on, NULL)
-};
+static DEFINE_RUNTIME_DEV_PM_OPS(ov02c10_pm_ops, ov02c10_power_off,
+				 ov02c10_power_on, NULL);
 
 #ifdef CONFIG_ACPI
 static const struct acpi_device_id ov02c10_acpi_ids[] = {
@@ -1064,7 +1019,7 @@ MODULE_DEVICE_TABLE(acpi, ov02c10_acpi_ids);
 static struct i2c_driver ov02c10_i2c_driver = {
 	.driver = {
 		.name = "ov02c10",
-		.pm = &ov02c10_pm_ops,
+		.pm = pm_sleep_ptr(&ov02c10_pm_ops),
 		.acpi_match_table = ACPI_PTR(ov02c10_acpi_ids),
 	},
 	.probe = ov02c10_probe,
-- 
2.48.1

[PATCH v8 13/14] media: ov02c10: Switch to using the sub-device state lock

[Hans de Goede]

Switch to using the sub-device state lock and properly call
v4l2_subdev_init_finalize() / v4l2_subdev_cleanup() on probe() /
remove().

While at it also properly setup runtime-pm before registering
the subdev.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/media/i2c/ov02c10.c | 51 +++++++++++++++++--------------------
 1 file changed, 23 insertions(+), 28 deletions(-)

diff --git a/drivers/media/i2c/ov02c10.c b/drivers/media/i2c/ov02c10.c
index 09e70ffcf07a..c559a69140ec 100644
--- a/drivers/media/i2c/ov02c10.c
+++ b/drivers/media/i2c/ov02c10.c
@@ -2,7 +2,6 @@
 // Copyright (c) 2022 Intel Corporation.
 
 #include <linux/acpi.h>
-#include <linux/cleanup.h>
 #include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/gpio/consumer.h>
@@ -387,9 +386,6 @@ struct ov02c10 {
 	/* Current mode */
 	const struct ov02c10_mode *cur_mode;
 
-	/* To serialize asynchronous callbacks */
-	struct mutex mutex;
-
 	/* MIPI lane info */
 	u32 link_freq_index;
 	u8 mipi_lanes;
@@ -490,7 +486,6 @@ static int ov02c10_init_controls(struct ov02c10 *ov02c10)
 	if (ret)
 		return ret;
 
-	ctrl_hdlr->lock = &ov02c10->mutex;
 	cur_mode = ov02c10->cur_mode;
 
 	ov02c10->link_freq = v4l2_ctrl_new_int_menu(ctrl_hdlr,
@@ -608,8 +603,6 @@ static int ov02c10_enable_streams(struct v4l2_subdev *sd,
 	struct ov02c10 *ov02c10 = to_ov02c10(sd);
 	int ret;
 
-	guard(mutex)(&ov02c10->mutex);
-
 	ret = pm_runtime_resume_and_get(&client->dev);
 	if (ret)
 		return ret;
@@ -628,8 +621,6 @@ static int ov02c10_disable_streams(struct v4l2_subdev *sd,
 	struct i2c_client *client = v4l2_get_subdevdata(sd);
 	struct ov02c10 *ov02c10 = to_ov02c10(sd);
 
-	guard(mutex)(&ov02c10->mutex);
-
 	ov02c10_stop_streaming(ov02c10);
 	pm_runtime_put(&client->dev);
 
@@ -718,7 +709,6 @@ static int ov02c10_set_format(struct v4l2_subdev *sd,
 				      height, fmt->format.width,
 				      fmt->format.height);
 
-	mutex_lock(&ov02c10->mutex);
 	ov02c10_update_pad_format(mode, &fmt->format);
 	if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) {
 		*v4l2_subdev_state_get_format(sd_state, fmt->pad) = fmt->format;
@@ -736,7 +726,6 @@ static int ov02c10_set_format(struct v4l2_subdev *sd,
 		__v4l2_ctrl_modify_range(ov02c10->hblank, h_blank, h_blank, 1,
 					 h_blank);
 	}
-	mutex_unlock(&ov02c10->mutex);
 
 	return 0;
 }
@@ -747,14 +736,11 @@ static int ov02c10_get_format(struct v4l2_subdev *sd,
 {
 	struct ov02c10 *ov02c10 = to_ov02c10(sd);
 
-	mutex_lock(&ov02c10->mutex);
 	if (fmt->which == V4L2_SUBDEV_FORMAT_TRY)
 		fmt->format = *v4l2_subdev_state_get_format(sd_state, fmt->pad);
 	else
 		ov02c10_update_pad_format(ov02c10->cur_mode, &fmt->format);
 
-	mutex_unlock(&ov02c10->mutex);
-
 	return 0;
 }
 
@@ -909,13 +895,16 @@ static int ov02c10_check_hwcfg(struct device *dev, struct ov02c10 *ov02c10)
 static void ov02c10_remove(struct i2c_client *client)
 {
 	struct v4l2_subdev *sd = i2c_get_clientdata(client);
-	struct ov02c10 *ov02c10 = to_ov02c10(sd);
 
 	v4l2_async_unregister_subdev(sd);
+	v4l2_subdev_cleanup(sd);
 	media_entity_cleanup(&sd->entity);
 	v4l2_ctrl_handler_free(sd->ctrl_handler);
 	pm_runtime_disable(&client->dev);
-	mutex_destroy(&ov02c10->mutex);
+	if (!pm_runtime_status_suspended(&client->dev)) {
+		ov02c10_power_off(&client->dev);
+		pm_runtime_set_suspended(&client->dev);
+	}
 }
 
 static int ov02c10_probe(struct i2c_client *client)
@@ -951,10 +940,9 @@ static int ov02c10_probe(struct i2c_client *client)
 	ret = ov02c10_identify_module(ov02c10);
 	if (ret) {
 		dev_err(&client->dev, "failed to find sensor: %d", ret);
-		goto probe_error_ret;
+		goto probe_error_power_off;
 	}
 
-	mutex_init(&ov02c10->mutex);
 	ov02c10->cur_mode = &supported_modes[0];
 
 	ret = ov02c10_init_controls(ov02c10);
@@ -974,31 +962,38 @@ static int ov02c10_probe(struct i2c_client *client)
 		goto probe_error_v4l2_ctrl_handler_free;
 	}
 
+	ov02c10->sd.state_lock = ov02c10->ctrl_handler.lock;
+	ret = v4l2_subdev_init_finalize(&ov02c10->sd);
+	if (ret < 0) {
+		dev_err(&client->dev, "failed to init subdev: %d", ret);
+		goto probe_error_media_entity_cleanup;
+	}
+
+	pm_runtime_set_active(&client->dev);
+	pm_runtime_enable(&client->dev);
+
 	ret = v4l2_async_register_subdev_sensor(&ov02c10->sd);
 	if (ret < 0) {
 		dev_err(&client->dev, "failed to register V4L2 subdev: %d",
 			ret);
-		goto probe_error_media_entity_cleanup;
+		goto probe_error_v4l2_subdev_cleanup;
 	}
 
-	/*
-	 * Device is already turned on by i2c-core with ACPI domain PM.
-	 * Enable runtime PM and turn off the device.
-	 */
-	pm_runtime_set_active(&client->dev);
-	pm_runtime_enable(&client->dev);
 	pm_runtime_idle(&client->dev);
-
 	return 0;
 
+probe_error_v4l2_subdev_cleanup:
+	pm_runtime_disable(&client->dev);
+	pm_runtime_set_suspended(&client->dev);
+	v4l2_subdev_cleanup(&ov02c10->sd);
+
 probe_error_media_entity_cleanup:
 	media_entity_cleanup(&ov02c10->sd.entity);
 
 probe_error_v4l2_ctrl_handler_free:
 	v4l2_ctrl_handler_free(ov02c10->sd.ctrl_handler);
-	mutex_destroy(&ov02c10->mutex);
 
-probe_error_ret:
+probe_error_power_off:
 	ov02c10_power_off(&client->dev);
 
 	return ret;
-- 
2.48.1

[PATCH v8 14/14] media: ov02c10: Use v4l2_subdev_get_fmt() as v4l2_subdev_pad_ops.get_fmt()

[Hans de Goede]

Store current-mode format in active-state format on non try set_format()
calls and use v4l2_subdev_get_fmt() as v4l2_subdev_pad_ops.get_fmt().

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/media/i2c/ov02c10.c | 42 ++++++++++++-------------------------
 1 file changed, 13 insertions(+), 29 deletions(-)

diff --git a/drivers/media/i2c/ov02c10.c b/drivers/media/i2c/ov02c10.c
index c559a69140ec..5626aa2fe62c 100644
--- a/drivers/media/i2c/ov02c10.c
+++ b/drivers/media/i2c/ov02c10.c
@@ -710,36 +710,20 @@ static int ov02c10_set_format(struct v4l2_subdev *sd,
 				      fmt->format.height);
 
 	ov02c10_update_pad_format(mode, &fmt->format);
-	if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) {
-		*v4l2_subdev_state_get_format(sd_state, fmt->pad) = fmt->format;
-	} else {
-		ov02c10->cur_mode = mode;
-
-		/* Update limits and set FPS to default */
-		vblank_def = mode->vts_min * ov02c10->mipi_lanes - mode->height;
-		__v4l2_ctrl_modify_range(ov02c10->vblank,
-					 mode->vts_min - mode->height,
-					 OV02C10_VTS_MAX - mode->height, 1,
-					 vblank_def);
-		__v4l2_ctrl_s_ctrl(ov02c10->vblank, vblank_def);
-		h_blank = mode->hts - mode->width;
-		__v4l2_ctrl_modify_range(ov02c10->hblank, h_blank, h_blank, 1,
-					 h_blank);
-	}
-
-	return 0;
-}
-
-static int ov02c10_get_format(struct v4l2_subdev *sd,
-			      struct v4l2_subdev_state *sd_state,
-			      struct v4l2_subdev_format *fmt)
-{
-	struct ov02c10 *ov02c10 = to_ov02c10(sd);
+	*v4l2_subdev_state_get_format(sd_state, fmt->pad) = fmt->format;
 
 	if (fmt->which == V4L2_SUBDEV_FORMAT_TRY)
-		fmt->format = *v4l2_subdev_state_get_format(sd_state, fmt->pad);
-	else
-		ov02c10_update_pad_format(ov02c10->cur_mode, &fmt->format);
+		return 0;
+
+	ov02c10->cur_mode = mode;
+
+	/* Update limits and set FPS to default */
+	vblank_def = mode->vts_min * ov02c10->mipi_lanes - mode->height;
+	__v4l2_ctrl_modify_range(ov02c10->vblank, mode->vts_min - mode->height,
+				 OV02C10_VTS_MAX - mode->height, 1, vblank_def);
+	__v4l2_ctrl_s_ctrl(ov02c10->vblank, vblank_def);
+	h_blank = mode->hts - mode->width;
+	__v4l2_ctrl_modify_range(ov02c10->hblank, h_blank, h_blank, 1, h_blank);
 
 	return 0;
 }
@@ -789,7 +773,7 @@ static const struct v4l2_subdev_video_ops ov02c10_video_ops = {
 
 static const struct v4l2_subdev_pad_ops ov02c10_pad_ops = {
 	.set_fmt = ov02c10_set_format,
-	.get_fmt = ov02c10_get_format,
+	.get_fmt = v4l2_subdev_get_fmt,
 	.enum_mbus_code = ov02c10_enum_mbus_code,
 	.enum_frame_size = ov02c10_enum_frame_size,
 	.enable_streams = ov02c10_enable_streams,
-- 
2.48.1

Re: [PATCH v8 00/14] media: i2c: Add Omnivision OV02C10 sensor driver

[Ingvar Hagelund]

to., 13.03.2025 kl. 19.43 +0100, skrev Hans de Goede:
> Here is v8 of the patch to upstream the OV02C10 sensor driver
> originally writen by Intel which Heimir has been working on
> upstreaming.
> 

Many thanks to Heimir and Hans for this excellent work. This makes my
workday easier. 

> (...)
> 
> 1. I don't have hardware to test. I hope that others can test this
> soon,
>    if things don't work the idea is that people can apply my cleanups
>    1 by 1 and then we will know which change has broken things.

Seems to work fine on my Dell XPS 13 9340. I have not found any
glitches so far. Tested with on fedora 41 with qcam, cheese, obs, and
firefox - tested with websites gum, jitsi, and webcamtests.com. All
these work fine. webcamtests.com was even able to select highest
resolution/zoom. Note that chromium does *not* work yet, at least not
in Fedora.

I now use this as my daily camera, without any problems.

I only miss more controls available, for example adjusting colors and
white balance. Some basic automatic adjustment seem to work, like when
changing rooms or lightning, but when for example you sit by a window
with sun on half your face, the image distorts, like getting over- or
underexposed. Also, the colors seems a bit washed out compared to my
usb cam (and to real life colors)


Switching to and fro between v7 and v8 without rebooting gave unstable
and strange results, but I presume that is less important, or even
expected.

Ingvar

Re: [PATCH v8 00/14] media: i2c: Add Omnivision OV02C10 sensor driver

[Hans de Goede]

Hi Ingvar,

On 14-Mar-25 9:52 AM, Ingvar Hagelund wrote:
> to., 13.03.2025 kl. 19.43 +0100, skrev Hans de Goede:
>> Here is v8 of the patch to upstream the OV02C10 sensor driver
>> originally writen by Intel which Heimir has been working on
>> upstreaming.
>>
> 
> Many thanks to Heimir and Hans for this excellent work. This makes my
> workday easier. 
> 
>> (...)
>>
>> 1. I don't have hardware to test. I hope that others can test this
>> soon,
>>    if things don't work the idea is that people can apply my cleanups
>>    1 by 1 and then we will know which change has broken things.
> 
> Seems to work fine on my Dell XPS 13 9340. I have not found any
> glitches so far. Tested with on fedora 41 with qcam, cheese, obs, and
> firefox - tested with websites gum, jitsi, and webcamtests.com. All
> these work fine. webcamtests.com was even able to select highest
> resolution/zoom. Note that chromium does *not* work yet, at least not
> in Fedora.

Great thank you for testing. I was afraid that I would have broken
something it is good to hear that I did not break anything.

Sakari, do you want to take a look at the incremental patches in this
v8 to get an idea of what I changed after your last review and maybe
give feedback on specific changes, or shall I post a squashed v9
which might be easier for you to review ?

> I now use this as my daily camera, without any problems.
> 
> I only miss more controls available, for example adjusting colors and
> white balance. Some basic automatic adjustment seem to work, like when
> changing rooms or lightning, but when for example you sit by a window
> with sun on half your face, the image distorts, like getting over- or
> underexposed. Also, the colors seems a bit washed out compared to my
> usb cam (and to real life colors)

Yes we need to work on improving the image quality and things like
the autoexposure algorithm, note this is all work which needs to be
done on the libcamera / softisp side not on the kernel side.

> Switching to and fro between v7 and v8 without rebooting gave unstable
> and strange results, but I presume that is less important, or even
> expected.

This is expected the sensor and CSI receiver get linked together
once both have loaded, rmmod-ing the sensor driver after this linking
is done is not really supported.

Regards,

Hans

Re: [PATCH v8 00/14] media: i2c: Add Omnivision OV02C10 sensor driver

[Hans de Goede]

Hi Ingvar,

On 14-Mar-25 9:52 AM, Ingvar Hagelund wrote:
> to., 13.03.2025 kl. 19.43 +0100, skrev Hans de Goede:
>> Here is v8 of the patch to upstream the OV02C10 sensor driver
>> originally writen by Intel which Heimir has been working on
>> upstreaming.
>>
> 
> Many thanks to Heimir and Hans for this excellent work. This makes my
> workday easier. 

You're welcome. One more testing request, can you run qcam and
then see what it reports for FPS after letting it run for
a couple of seconds ?

And then report the FPS back here ?

Regards,

Hans

Re: [PATCH v8 00/14] media: i2c: Add Omnivision OV02C10 sensor driver

[Ingvar Hagelund]

fr., 14.03.2025 kl. 10.43 +0100, skrev Hans de Goede:
> 
> One more testing request, can you run qcam and
> then see what it reports for FPS after letting it run for
> a couple of seconds ?
> 
> And then report the FPS back here ?

Sure

It stabilizes at 30 fps (wandering a bit between 30.00 and 30.02 fps).

Also, here is the output of cam -l and modinfo.

I observe that cam says 

Configuration file 'ov02c10.yaml' not found for IPA module 'simple',
falling back to 'uncalibrated.yaml'

Should this file exist, or will the driver downloaded from the settings
from the device, making the configuration file superflous?

Best regards,
Ingvar



$ cam -l 
[2:16:02.661071222] [25536]  INFO Camera camera_manager.cpp:325
libcamera v0.3.2
[2:16:02.677709274] [25539]  WARN CameraSensor camera_sensor.cpp:257
'ov02c10 17-0036': Recommended V4L2 control 0x009a0922 not supported
[2:16:02.677733285] [25539] ERROR V4L2 v4l2_subdevice.cpp:1085 'ov02c10
17-0036': Unable to get rectangle 2 on pad 0/0: Inappropriate ioctl for
device
[2:16:02.677742291] [25539]  WARN CameraSensor camera_sensor.cpp:304
'ov02c10 17-0036': The PixelArraySize property has been defaulted to
1928x1092
[2:16:02.677747409] [25539] ERROR V4L2 v4l2_subdevice.cpp:1085 'ov02c10
17-0036': Unable to get rectangle 1 on pad 0/0: Inappropriate ioctl for
device
[2:16:02.677751299] [25539]  WARN CameraSensor camera_sensor.cpp:315
'ov02c10 17-0036': The PixelArrayActiveAreas property has been
defaulted to (0, 0)/1928x1092
[2:16:02.677757335] [25539] ERROR V4L2 v4l2_subdevice.cpp:1085 'ov02c10
17-0036': Unable to get rectangle 0 on pad 0/0: Inappropriate ioctl for
device
[2:16:02.677760909] [25539]  WARN CameraSensor camera_sensor.cpp:323
'ov02c10 17-0036': Failed to retrieve the sensor crop rectangle
[2:16:02.677764402] [25539]  WARN CameraSensor camera_sensor.cpp:329
'ov02c10 17-0036': The sensor kernel driver needs to be fixed
[2:16:02.677767468] [25539]  WARN CameraSensor camera_sensor.cpp:331
'ov02c10 17-0036': See Documentation/sensor_driver_requirements.rst in
the libcamera sources for more information
[2:16:02.678099594] [25539]  WARN CameraSensorProperties
camera_sensor_properties.cpp:293 No static properties available for
'ov02c10'
[2:16:02.678112388] [25539]  WARN CameraSensorProperties
camera_sensor_properties.cpp:295 Please consider updating the camera
sensor properties database
[2:16:02.678116314] [25539]  WARN CameraSensor camera_sensor.cpp:477
'ov02c10 17-0036': Failed to retrieve the camera location
[2:16:02.678120108] [25539]  WARN CameraSensor camera_sensor.cpp:499
'ov02c10 17-0036': Rotation control not available, default to 0 degrees
[2:16:02.679507146] [25539]  WARN IPAProxy ipa_proxy.cpp:160
Configuration file 'ov02c10.yaml' not found for IPA module 'simple',
falling back to 'uncalibrated.yaml'
[2:16:02.679530599] [25539]  WARN IPASoft soft_simple.cpp:114 Failed to
create camera sensor helper for ov02c10

$ modinfo ov02c10
filename:       /lib/modules/6.13.6-
200.fc41.x86_64/kernel/drivers/media/i2c/ov02c10.ko.xz
license:        GPL
description:    OmniVision OV02C10 sensor driver
author:         Hao Yao <hao.yao@intel.com>
alias:          acpi*:OVTI02C1:*
depends:        videodev,v4l2-cci,v4l2-fwnode,mc,v4l2-async
name:           ov02c10
retpoline:      Y
vermagic:       6.13.6-200.fc41.x86_64 SMP preempt mod_unload 

Re: [PATCH v8 00/14] media: i2c: Add Omnivision OV02C10 sensor driver

[Hans de Goede]

Hi,

On 14-Mar-25 11:01 AM, Ingvar Hagelund wrote:
> fr., 14.03.2025 kl. 10.43 +0100, skrev Hans de Goede:
>>
>> One more testing request, can you run qcam and
>> then see what it reports for FPS after letting it run for
>> a couple of seconds ?
>>
>> And then report the FPS back here ?
> 
> Sure
> 
> It stabilizes at 30 fps (wandering a bit between 30.00 and 30.02 fps).

Great, thank you. That means everything is working as
it should (I had some doubts about 1 of the timing related
register values used by the driver).

> Also, here is the output of cam -l and modinfo.
> 
> I observe that cam says 
> 
> Configuration file 'ov02c10.yaml' not found for IPA module 'simple',
> falling back to 'uncalibrated.yaml'
>
> Should this file exist, or will the driver downloaded from the settings
> from the device, making the configuration file superflous?

There is a bunch of low hanging fruit wrt image quality which we
need to address in libcamera which should help improve things
without needing per sensor calibration. But for the best quality
ideally we would have per sensor / camera module calibration
profiles for certain things. There still is a long road to go
before we will even start looking into that though.

Regards,

Hans