[PATCH v2 00/23] media: i2c: ov01a10: Add crop, ov01a1b support

[PATCH v2 00/23] media: i2c: ov01a10: Add crop, ov01a1b support

[Sakari Ailus]

Hi folks,

I've squashed fixups from Hans on v1 plus addressed some trivial issues
brought up by Mehdi. Also I've kept ov01a1s support out for now as it only
supports an IR pattern.

v1 is <20251014174033.20534-1-hansg@kernel.org> on LMML.

Hans de Goede (23):
  media: i2c: ov01a10: Fix the horizontal flip control
  media: i2c: ov01a10: Fix reported pixel-rate value
  media: i2c: ov01a10: Fix analogue gain range
  media: i2c: ov01a10: Add missing v4l2_subdev_cleanup() calls
  media: i2c: ov01a10: Fix passing stream instead of pad to
    v4l2_subdev_state_get_format()
  media: i2c: ov01a10: Fix test-pattern disabling
  media: i2c: ov01a10: Change default vblank value to a vblank resulting
    in 30 fps
  media: i2c: ov01a10: Convert to new CCI register access helpers
  media: i2c: ov01a10: Remove overly verbose probe() error reporting
  media: i2c: ov01a10: Store dev pointer in struct ov01a10
  media: i2c: ov01a10: Add ov01a10_check_hwcfg() function
  media: i2c: ov01a10: Add power on/off sequencing support
  media: i2c: ov01a10: Don't update pixel_rate and link_freq from
    set_fmt
  media: i2c: ov01a10: Move setting of ctrl->flags to after checking
    ctrl_hdlr->error
  media: i2c: ov01a10: Use native and default for pixel-array size names
  media: i2c: ov01a10: Add cropping support / allow arbitrary sizes
  media: i2c: ov01a10: Remove struct ov01a10_reg_list
  media: i2c: ov01a10: Replace exposure->min/step with direct define use
  media: i2c: ov01a10: Only set register 0x0305 once
  media: i2c: ov01a10: Remove values set by controls from
    global_setting[]
  media: i2c: ov01a10: Add ov01a10_sensor_cfg struct
  media: i2c: ov01a10: Optimize setting h/vflip values
  media: i2c: ov01a10: Add ov01a1b support

 drivers/media/i2c/Kconfig   |   1 +
 drivers/media/i2c/ov01a10.c | 918 +++++++++++++++++++++---------------
 2 files changed, 548 insertions(+), 371 deletions(-)

-- 
2.47.3

[PATCH v2 01/23] media: i2c: ov01a10: Fix the horizontal flip control

[Sakari Ailus]

From: Hans de Goede <hansg@kernel.org>

During sensor calibration I noticed that with the hflip control set
to false/disabled the image was mirrored.

So it seems that the horizontal flip control is inverted and needs to
be set to 1 to not flip (just like the similar problem recently fixed
on the ov08x40 sensor).

Invert the hflip control to fix the sensor mirroring by default.

As the comment above the newly added OV01A10_MEDIA_BUS_FMT define explains
the control being inverted also means that the native Bayer-order of
the sensor actually is GBRG not BGGR, but so as to not break userspace
the Bayer-order is kept at BGGR.

Fixes: 0827b58dabff ("media: i2c: add ov01a10 image sensor driver")
Cc: stable@vger.kernel.org
Signed-off-by: Hans de Goede <hansg@kernel.org>
Tested-by: Mehdi Djait <mehdi.djait@linux.intel.com> # Dell XPS 9315
Reviewed-by: Mehdi Djait <mehdi.djait@linux.intel.com>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/ov01a10.c | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/drivers/media/i2c/ov01a10.c b/drivers/media/i2c/ov01a10.c
index 141cb6f75b55..e5df01f97978 100644
--- a/drivers/media/i2c/ov01a10.c
+++ b/drivers/media/i2c/ov01a10.c
@@ -75,6 +75,15 @@
 #define OV01A10_REG_X_WIN		0x3811
 #define OV01A10_REG_Y_WIN		0x3813
 
+/*
+ * The native ov01a10 bayer-pattern is GBRG, but there was a driver bug enabling
+ * hflip/mirroring by default resulting in BGGR. Because of this bug Intel's
+ * proprietary IPU6 userspace stack expects BGGR. So we report BGGR to not break
+ * userspace and fix things up by shifting the crop window-x coordinate by 1
+ * when hflip is *disabled*.
+ */
+#define OV01A10_MEDIA_BUS_FMT		MEDIA_BUS_FMT_SBGGR10_1X10
+
 struct ov01a10_reg {
 	u16 address;
 	u8 val;
@@ -185,14 +194,14 @@ static const struct ov01a10_reg sensor_1280x800_setting[] = {
 	{0x380e, 0x03},
 	{0x380f, 0x80},
 	{0x3810, 0x00},
-	{0x3811, 0x08},
+	{0x3811, 0x09},
 	{0x3812, 0x00},
 	{0x3813, 0x08},
 	{0x3814, 0x01},
 	{0x3815, 0x01},
 	{0x3816, 0x01},
 	{0x3817, 0x01},
-	{0x3820, 0xa0},
+	{0x3820, 0xa8},
 	{0x3822, 0x13},
 	{0x3832, 0x28},
 	{0x3833, 0x10},
@@ -411,7 +420,7 @@ static int ov01a10_set_hflip(struct ov01a10 *ov01a10, u32 hflip)
 	int ret;
 	u32 val, offset;
 
-	offset = hflip ? 0x9 : 0x8;
+	offset = hflip ? 0x8 : 0x9;
 	ret = ov01a10_write_reg(ov01a10, OV01A10_REG_X_WIN, 1, offset);
 	if (ret)
 		return ret;
@@ -420,8 +429,8 @@ static int ov01a10_set_hflip(struct ov01a10 *ov01a10, u32 hflip)
 	if (ret)
 		return ret;
 
-	val = hflip ? val | FIELD_PREP(OV01A10_HFLIP_MASK, 0x1) :
-		val & ~OV01A10_HFLIP_MASK;
+	val = hflip ? val & ~OV01A10_HFLIP_MASK :
+		      val | FIELD_PREP(OV01A10_HFLIP_MASK, 0x1);
 
 	return ov01a10_write_reg(ov01a10, OV01A10_REG_FORMAT1, 1, val);
 }
@@ -610,7 +619,7 @@ static void ov01a10_update_pad_format(const struct ov01a10_mode *mode,
 {
 	fmt->width = mode->width;
 	fmt->height = mode->height;
-	fmt->code = MEDIA_BUS_FMT_SBGGR10_1X10;
+	fmt->code = OV01A10_MEDIA_BUS_FMT;
 	fmt->field = V4L2_FIELD_NONE;
 	fmt->colorspace = V4L2_COLORSPACE_RAW;
 }
@@ -751,7 +760,7 @@ static int ov01a10_enum_mbus_code(struct v4l2_subdev *sd,
 	if (code->index > 0)
 		return -EINVAL;
 
-	code->code = MEDIA_BUS_FMT_SBGGR10_1X10;
+	code->code = OV01A10_MEDIA_BUS_FMT;
 
 	return 0;
 }
@@ -761,7 +770,7 @@ static int ov01a10_enum_frame_size(struct v4l2_subdev *sd,
 				   struct v4l2_subdev_frame_size_enum *fse)
 {
 	if (fse->index >= ARRAY_SIZE(supported_modes) ||
-	    fse->code != MEDIA_BUS_FMT_SBGGR10_1X10)
+	    fse->code != OV01A10_MEDIA_BUS_FMT)
 		return -EINVAL;
 
 	fse->min_width = supported_modes[fse->index].width;
-- 
2.47.3

[PATCH v2 02/23] media: i2c: ov01a10: Fix reported pixel-rate value

[Sakari Ailus]

From: Hans de Goede <hansg@kernel.org>

CSI lanes are double-clocked so with a single lane at 400MHZ the resulting
pixel-rate for 10-bits pixels is 400 MHz * 2 / 10 = 80 MHz, not 40 MHz.

This also matches with the observed frame-rate of 60 fps with the default
vblank setting: 80000000 / (1488 * 896) = 60.

Fixes: 0827b58dabff ("media: i2c: add ov01a10 image sensor driver")
Cc: stable@vger.kernel.org
Signed-off-by: Hans de Goede <hansg@kernel.org>
Tested-by: Mehdi Djait <mehdi.djait@linux.intel.com> # Dell XPS 9315
Reviewed-by: Mehdi Djait <mehdi.djait@linux.intel.com>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/ov01a10.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/i2c/ov01a10.c b/drivers/media/i2c/ov01a10.c
index e5df01f97978..0b1a1ecfffd0 100644
--- a/drivers/media/i2c/ov01a10.c
+++ b/drivers/media/i2c/ov01a10.c
@@ -16,7 +16,7 @@
 #include <media/v4l2-fwnode.h>
 
 #define OV01A10_LINK_FREQ_400MHZ	400000000ULL
-#define OV01A10_SCLK			40000000LL
+#define OV01A10_SCLK			80000000LL
 #define OV01A10_DATA_LANES		1
 
 #define OV01A10_REG_CHIP_ID		0x300a
-- 
2.47.3

[PATCH v2 03/23] media: i2c: ov01a10: Fix analogue gain range

[Sakari Ailus]

From: Hans de Goede <hansg@kernel.org>

A analogue maximum gain of 0xffff / 65525 seems unlikely and testing
indeed shows that the gain control wraps-around at 16383, so set the
maximum gain to 0x3fff / 16383.

The minimum gain of 0x100 is correct. Setting bits 8-11 to 0x0 results
in the same gain values as setting these bits to 0x1, with bits 0-7
still increasing the gain when going from 0x000 - 0x0ff in the exact
same range as when going from 0x100 - 0x1ff.

Fixes: 0827b58dabff ("media: i2c: add ov01a10 image sensor driver")
Cc: stable@vger.kernel.org
Signed-off-by: Hans de Goede <hansg@kernel.org>
Tested-by: Mehdi Djait <mehdi.djait@linux.intel.com> # Dell XPS 9315
Reviewed-by: Mehdi Djait <mehdi.djait@linux.intel.com>
[Sakari Ailus: mention analogue gain and update the limit from 4096.]
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/ov01a10.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/i2c/ov01a10.c b/drivers/media/i2c/ov01a10.c
index 0b1a1ecfffd0..834ca46acb75 100644
--- a/drivers/media/i2c/ov01a10.c
+++ b/drivers/media/i2c/ov01a10.c
@@ -48,7 +48,7 @@
 /* analog gain controls */
 #define OV01A10_REG_ANALOG_GAIN		0x3508
 #define OV01A10_ANAL_GAIN_MIN		0x100
-#define OV01A10_ANAL_GAIN_MAX		0xffff
+#define OV01A10_ANAL_GAIN_MAX		0x3fff
 #define OV01A10_ANAL_GAIN_STEP		1
 
 /* digital gain controls */
-- 
2.47.3

[PATCH v2 04/23] media: i2c: ov01a10: Add missing v4l2_subdev_cleanup() calls

[Sakari Ailus]

From: Hans de Goede <hansg@kernel.org>

Add missing v4l2_subdev_cleanup() calls to cleanup after
v4l2_subdev_init_finalize().

Fixes: 0827b58dabff ("media: i2c: add ov01a10 image sensor driver")
Cc: stable@vger.kernel.org
Signed-off-by: Hans de Goede <hansg@kernel.org>
Tested-by: Mehdi Djait <mehdi.djait@linux.intel.com> # Dell XPS 9315
Reviewed-by: Mehdi Djait <mehdi.djait@linux.intel.com>
Reviewed-by: Bingbu Cao <bingbu.cao@intel.com>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/ov01a10.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/media/i2c/ov01a10.c b/drivers/media/i2c/ov01a10.c
index 834ca46acb75..1e22df12989a 100644
--- a/drivers/media/i2c/ov01a10.c
+++ b/drivers/media/i2c/ov01a10.c
@@ -864,6 +864,7 @@ static void ov01a10_remove(struct i2c_client *client)
 	struct v4l2_subdev *sd = i2c_get_clientdata(client);
 
 	v4l2_async_unregister_subdev(sd);
+	v4l2_subdev_cleanup(sd);
 	media_entity_cleanup(&sd->entity);
 	v4l2_ctrl_handler_free(sd->ctrl_handler);
 
@@ -934,6 +935,7 @@ static int ov01a10_probe(struct i2c_client *client)
 err_pm_disable:
 	pm_runtime_disable(dev);
 	pm_runtime_set_suspended(&client->dev);
+	v4l2_subdev_cleanup(&ov01a10->sd);
 
 err_media_entity_cleanup:
 	media_entity_cleanup(&ov01a10->sd.entity);
-- 
2.47.3

[PATCH v2 05/23] media: i2c: ov01a10: Fix passing stream instead of pad to v4l2_subdev_state_get_format()

[Sakari Ailus]

From: Hans de Goede <hansg@kernel.org>

The 2 argument version of v4l2_subdev_state_get_format() takes the pad
as second argument, not the stream.

Fixes: bc0e8d91feec ("media: v4l: subdev: Switch to stream-aware state functions")
Cc: stable@vger.kernel.org
Signed-off-by: Hans de Goede <hansg@kernel.org>
Tested-by: Mehdi Djait <mehdi.djait@linux.intel.com> # Dell XPS 9315
Reviewed-by: Mehdi Djait <mehdi.djait@linux.intel.com>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/ov01a10.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/i2c/ov01a10.c b/drivers/media/i2c/ov01a10.c
index 1e22df12989a..dd2b6d381175 100644
--- a/drivers/media/i2c/ov01a10.c
+++ b/drivers/media/i2c/ov01a10.c
@@ -731,7 +731,7 @@ static int ov01a10_set_format(struct v4l2_subdev *sd,
 					 h_blank);
 	}
 
-	format = v4l2_subdev_state_get_format(sd_state, fmt->stream);
+	format = v4l2_subdev_state_get_format(sd_state, fmt->pad);
 	*format = fmt->format;
 
 	return 0;
-- 
2.47.3

[PATCH v2 06/23] media: i2c: ov01a10: Fix test-pattern disabling

[Sakari Ailus]

From: Hans de Goede <hansg@kernel.org>

When the test-pattern control gets set to 0 (Disabled) 0 should be written
to the test-pattern register, rather then doing nothing.

Fixes: 0827b58dabff ("media: i2c: add ov01a10 image sensor driver")
Cc: stable@vger.kernel.org
Signed-off-by: Hans de Goede <hansg@kernel.org>
Tested-by: Mehdi Djait <mehdi.djait@linux.intel.com> # Dell XPS 9315
Reviewed-by: Mehdi Djait <mehdi.djait@linux.intel.com>
Reviewed-by: Bingbu Cao <bingbu.cao@intel.com>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/ov01a10.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/media/i2c/ov01a10.c b/drivers/media/i2c/ov01a10.c
index dd2b6d381175..3ad516e4d369 100644
--- a/drivers/media/i2c/ov01a10.c
+++ b/drivers/media/i2c/ov01a10.c
@@ -249,9 +249,8 @@ static const struct ov01a10_reg sensor_1280x800_setting[] = {
 static const char * const ov01a10_test_pattern_menu[] = {
 	"Disabled",
 	"Color Bar",
-	"Top-Bottom Darker Color Bar",
-	"Right-Left Darker Color Bar",
-	"Color Bar type 4",
+	"Left-Right Darker Color Bar",
+	"Bottom-Top Darker Color Bar",
 };
 
 static const s64 link_freq_menu_items[] = {
@@ -406,10 +405,8 @@ static int ov01a10_update_digital_gain(struct ov01a10 *ov01a10, u32 d_gain)
 
 static int ov01a10_test_pattern(struct ov01a10 *ov01a10, u32 pattern)
 {
-	if (!pattern)
-		return 0;
-
-	pattern = (pattern - 1) | OV01A10_TEST_PATTERN_ENABLE;
+	if (pattern)
+		pattern |= OV01A10_TEST_PATTERN_ENABLE;
 
 	return ov01a10_write_reg(ov01a10, OV01A10_REG_TEST_PATTERN, 1, pattern);
 }
-- 
2.47.3

[PATCH v2 07/23] media: i2c: ov01a10: Change default vblank value to a vblank resulting in 30 fps

[Sakari Ailus]

From: Hans de Goede <hansg@kernel.org>

The ov01a10 is quite a small sensor, which does not capture a lot of
light, increase the default vblank so that the sensor runs at 30 fps
by default, doubling the default exposure.

Signed-off-by: Hans de Goede <hansg@kernel.org>
Tested-by: Mehdi Djait <mehdi.djait@linux.intel.com> # Dell XPS 9315
Reviewed-by: Mehdi Djait <mehdi.djait@linux.intel.com>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/ov01a10.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/i2c/ov01a10.c b/drivers/media/i2c/ov01a10.c
index 3ad516e4d369..4f04bb8688b8 100644
--- a/drivers/media/i2c/ov01a10.c
+++ b/drivers/media/i2c/ov01a10.c
@@ -34,7 +34,7 @@
 
 /* vertical and horizontal timings */
 #define OV01A10_REG_VTS			0x380e
-#define OV01A10_VTS_DEF			0x0380
+#define OV01A10_VTS_DEF			0x0700
 #define OV01A10_VTS_MIN			0x0380
 #define OV01A10_VTS_MAX			0xffff
 #define OV01A10_HTS_DEF			1488
@@ -191,8 +191,8 @@ static const struct ov01a10_reg sensor_1280x800_setting[] = {
 	{0x380b, 0x20},
 	{0x380c, 0x02},
 	{0x380d, 0xe8},
-	{0x380e, 0x03},
-	{0x380f, 0x80},
+	{0x380e, 0x07},
+	{0x380f, 0x00},
 	{0x3810, 0x00},
 	{0x3811, 0x09},
 	{0x3812, 0x00},
-- 
2.47.3

[PATCH v2 08/23] media: i2c: ov01a10: Convert to new CCI register access helpers

[Sakari Ailus]

From: Hans de Goede <hansg@kernel.org>

Use the new comon CCI register access helpers to replace the private
register access helpers in the ov01a10 driver.

Signed-off-by: Hans de Goede <hansg@kernel.org>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/Kconfig   |   1 +
 drivers/media/i2c/ov01a10.c | 219 ++++++++++--------------------------
 2 files changed, 59 insertions(+), 161 deletions(-)

diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 4b4db8c4f496..42be3a5f36f6 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -373,6 +373,7 @@ config VIDEO_OG0VE1B
 
 config VIDEO_OV01A10
 	tristate "OmniVision OV01A10 sensor support"
+	select V4L2_CCI_I2C
 	help
 	  This is a Video4Linux2 sensor driver for the OmniVision
 	  OV01A10 camera.
diff --git a/drivers/media/i2c/ov01a10.c b/drivers/media/i2c/ov01a10.c
index 4f04bb8688b8..34b7ab6eb286 100644
--- a/drivers/media/i2c/ov01a10.c
+++ b/drivers/media/i2c/ov01a10.c
@@ -10,7 +10,9 @@
 #include <linux/i2c.h>
 #include <linux/module.h>
 #include <linux/pm_runtime.h>
+#include <linux/regmap.h>
 
+#include <media/v4l2-cci.h>
 #include <media/v4l2-ctrls.h>
 #include <media/v4l2-device.h>
 #include <media/v4l2-fwnode.h>
@@ -19,10 +21,10 @@
 #define OV01A10_SCLK			80000000LL
 #define OV01A10_DATA_LANES		1
 
-#define OV01A10_REG_CHIP_ID		0x300a
+#define OV01A10_REG_CHIP_ID		CCI_REG24(0x300a)
 #define OV01A10_CHIP_ID			0x560141
 
-#define OV01A10_REG_MODE_SELECT		0x0100
+#define OV01A10_REG_MODE_SELECT		CCI_REG8(0x0100)
 #define OV01A10_MODE_STANDBY		0x00
 #define OV01A10_MODE_STREAMING		0x01
 
@@ -33,47 +35,47 @@
 #define OV01A10_ACITVE_HEIGHT		800
 
 /* vertical and horizontal timings */
-#define OV01A10_REG_VTS			0x380e
+#define OV01A10_REG_VTS			CCI_REG16(0x380e)
 #define OV01A10_VTS_DEF			0x0700
 #define OV01A10_VTS_MIN			0x0380
 #define OV01A10_VTS_MAX			0xffff
 #define OV01A10_HTS_DEF			1488
 
 /* exposure controls */
-#define OV01A10_REG_EXPOSURE		0x3501
+#define OV01A10_REG_EXPOSURE		CCI_REG16(0x3501)
 #define OV01A10_EXPOSURE_MIN		4
 #define OV01A10_EXPOSURE_MAX_MARGIN	8
 #define OV01A10_EXPOSURE_STEP		1
 
 /* analog gain controls */
-#define OV01A10_REG_ANALOG_GAIN		0x3508
+#define OV01A10_REG_ANALOG_GAIN		CCI_REG16(0x3508)
 #define OV01A10_ANAL_GAIN_MIN		0x100
 #define OV01A10_ANAL_GAIN_MAX		0x3fff
 #define OV01A10_ANAL_GAIN_STEP		1
 
 /* digital gain controls */
-#define OV01A10_REG_DIGITAL_GAIN_B	0x350a
-#define OV01A10_REG_DIGITAL_GAIN_GB	0x3510
-#define OV01A10_REG_DIGITAL_GAIN_GR	0x3513
-#define OV01A10_REG_DIGITAL_GAIN_R	0x3516
+#define OV01A10_REG_DIGITAL_GAIN_B	CCI_REG24(0x350a)
+#define OV01A10_REG_DIGITAL_GAIN_GB	CCI_REG24(0x3510)
+#define OV01A10_REG_DIGITAL_GAIN_GR	CCI_REG24(0x3513)
+#define OV01A10_REG_DIGITAL_GAIN_R	CCI_REG24(0x3516)
 #define OV01A10_DGTL_GAIN_MIN		0
 #define OV01A10_DGTL_GAIN_MAX		0x3ffff
 #define OV01A10_DGTL_GAIN_STEP		1
 #define OV01A10_DGTL_GAIN_DEFAULT	1024
 
 /* test pattern control */
-#define OV01A10_REG_TEST_PATTERN	0x4503
+#define OV01A10_REG_TEST_PATTERN	CCI_REG8(0x4503)
 #define OV01A10_TEST_PATTERN_ENABLE	BIT(7)
 #define OV01A10_LINK_FREQ_400MHZ_INDEX	0
 
 /* flip and mirror control */
-#define OV01A10_REG_FORMAT1		0x3820
+#define OV01A10_REG_FORMAT1		CCI_REG8(0x3820)
 #define OV01A10_VFLIP_MASK		BIT(4)
 #define OV01A10_HFLIP_MASK		BIT(3)
 
 /* window offset */
-#define OV01A10_REG_X_WIN		0x3811
-#define OV01A10_REG_Y_WIN		0x3813
+#define OV01A10_REG_X_WIN		CCI_REG16(0x3810)
+#define OV01A10_REG_Y_WIN		CCI_REG16(0x3812)
 
 /*
  * The native ov01a10 bayer-pattern is GBRG, but there was a driver bug enabling
@@ -84,14 +86,9 @@
  */
 #define OV01A10_MEDIA_BUS_FMT		MEDIA_BUS_FMT_SBGGR10_1X10
 
-struct ov01a10_reg {
-	u16 address;
-	u8 val;
-};
-
 struct ov01a10_reg_list {
 	u32 num_of_regs;
-	const struct ov01a10_reg *regs;
+	const struct reg_sequence *regs;
 };
 
 struct ov01a10_link_freq_config {
@@ -109,7 +106,7 @@ struct ov01a10_mode {
 	const struct ov01a10_reg_list reg_list;
 };
 
-static const struct ov01a10_reg mipi_data_rate_720mbps[] = {
+static const struct reg_sequence mipi_data_rate_720mbps[] = {
 	{0x0103, 0x01},
 	{0x0302, 0x00},
 	{0x0303, 0x06},
@@ -125,7 +122,7 @@ static const struct ov01a10_reg mipi_data_rate_720mbps[] = {
 	{0x0325, 0x68},
 };
 
-static const struct ov01a10_reg sensor_1280x800_setting[] = {
+static const struct reg_sequence sensor_1280x800_setting[] = {
 	{0x3002, 0xa1},
 	{0x301e, 0xf0},
 	{0x3022, 0x01},
@@ -282,6 +279,7 @@ static const struct ov01a10_mode supported_modes[] = {
 };
 
 struct ov01a10 {
+	struct regmap *regmap;
 	struct v4l2_subdev sd;
 	struct media_pad pad;
 	struct v4l2_ctrl_handler ctrl_handler;
@@ -301,104 +299,15 @@ static inline struct ov01a10 *to_ov01a10(struct v4l2_subdev *subdev)
 	return container_of(subdev, struct ov01a10, sd);
 }
 
-static int ov01a10_read_reg(struct ov01a10 *ov01a10, u16 reg, u16 len, u32 *val)
-{
-	struct i2c_client *client = v4l2_get_subdevdata(&ov01a10->sd);
-	struct i2c_msg msgs[2];
-	u8 addr_buf[2];
-	u8 data_buf[4] = {0};
-	int ret = 0;
-
-	if (len > sizeof(data_buf))
-		return -EINVAL;
-
-	put_unaligned_be16(reg, addr_buf);
-	msgs[0].addr = client->addr;
-	msgs[0].flags = 0;
-	msgs[0].len = sizeof(addr_buf);
-	msgs[0].buf = addr_buf;
-	msgs[1].addr = client->addr;
-	msgs[1].flags = I2C_M_RD;
-	msgs[1].len = len;
-	msgs[1].buf = &data_buf[sizeof(data_buf) - len];
-
-	ret = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
-
-	if (ret != ARRAY_SIZE(msgs))
-		return ret < 0 ? ret : -EIO;
-
-	*val = get_unaligned_be32(data_buf);
-
-	return 0;
-}
-
-static int ov01a10_write_reg(struct ov01a10 *ov01a10, u16 reg, u16 len, u32 val)
-{
-	struct i2c_client *client = v4l2_get_subdevdata(&ov01a10->sd);
-	u8 buf[6];
-	int ret = 0;
-
-	if (len > 4)
-		return -EINVAL;
-
-	put_unaligned_be16(reg, buf);
-	put_unaligned_be32(val << 8 * (4 - len), buf + 2);
-
-	ret = i2c_master_send(client, buf, len + 2);
-	if (ret != len + 2)
-		return ret < 0 ? ret : -EIO;
-
-	return 0;
-}
-
-static int ov01a10_write_reg_list(struct ov01a10 *ov01a10,
-				  const struct ov01a10_reg_list *r_list)
-{
-	struct i2c_client *client = v4l2_get_subdevdata(&ov01a10->sd);
-	unsigned int i;
-	int ret = 0;
-
-	for (i = 0; i < r_list->num_of_regs; i++) {
-		ret = ov01a10_write_reg(ov01a10, r_list->regs[i].address, 1,
-					r_list->regs[i].val);
-		if (ret) {
-			dev_err_ratelimited(&client->dev,
-					    "write reg 0x%4.4x err = %d\n",
-					    r_list->regs[i].address, ret);
-			return ret;
-		}
-	}
-
-	return 0;
-}
-
 static int ov01a10_update_digital_gain(struct ov01a10 *ov01a10, u32 d_gain)
 {
-	struct i2c_client *client = v4l2_get_subdevdata(&ov01a10->sd);
 	u32 real = d_gain << 6;
 	int ret = 0;
 
-	ret = ov01a10_write_reg(ov01a10, OV01A10_REG_DIGITAL_GAIN_B, 3, real);
-	if (ret) {
-		dev_err(&client->dev, "failed to set DIGITAL_GAIN_B\n");
-		return ret;
-	}
-
-	ret = ov01a10_write_reg(ov01a10, OV01A10_REG_DIGITAL_GAIN_GB, 3, real);
-	if (ret) {
-		dev_err(&client->dev, "failed to set DIGITAL_GAIN_GB\n");
-		return ret;
-	}
-
-	ret = ov01a10_write_reg(ov01a10, OV01A10_REG_DIGITAL_GAIN_GR, 3, real);
-	if (ret) {
-		dev_err(&client->dev, "failed to set DIGITAL_GAIN_GR\n");
-		return ret;
-	}
-
-	ret = ov01a10_write_reg(ov01a10, OV01A10_REG_DIGITAL_GAIN_R, 3, real);
-	if (ret)
-		dev_err(&client->dev, "failed to set DIGITAL_GAIN_R\n");
+	cci_write(ov01a10->regmap, OV01A10_REG_DIGITAL_GAIN_B, real, &ret);
+	cci_write(ov01a10->regmap, OV01A10_REG_DIGITAL_GAIN_GB, real, &ret);
+	cci_write(ov01a10->regmap, OV01A10_REG_DIGITAL_GAIN_GR, real, &ret);
+	cci_write(ov01a10->regmap, OV01A10_REG_DIGITAL_GAIN_R, real, &ret);
 
 	return ret;
 }
@@ -408,48 +317,39 @@ static int ov01a10_test_pattern(struct ov01a10 *ov01a10, u32 pattern)
 	if (pattern)
 		pattern |= OV01A10_TEST_PATTERN_ENABLE;
 
-	return ov01a10_write_reg(ov01a10, OV01A10_REG_TEST_PATTERN, 1, pattern);
+	return cci_write(ov01a10->regmap, OV01A10_REG_TEST_PATTERN, pattern,
+			 NULL);
 }
 
 /* for vflip and hflip, use 0x9 as window offset to keep the bayer */
 static int ov01a10_set_hflip(struct ov01a10 *ov01a10, u32 hflip)
 {
-	int ret;
 	u32 val, offset;
+	int ret = 0;
 
 	offset = hflip ? 0x8 : 0x9;
-	ret = ov01a10_write_reg(ov01a10, OV01A10_REG_X_WIN, 1, offset);
-	if (ret)
-		return ret;
+	val = hflip ? 0 : FIELD_PREP(OV01A10_HFLIP_MASK, 0x1);
 
-	ret = ov01a10_read_reg(ov01a10, OV01A10_REG_FORMAT1, 1, &val);
-	if (ret)
-		return ret;
+	cci_write(ov01a10->regmap, OV01A10_REG_X_WIN, offset, &ret);
+	cci_update_bits(ov01a10->regmap, OV01A10_REG_FORMAT1,
+			OV01A10_HFLIP_MASK, val, &ret);
 
-	val = hflip ? val & ~OV01A10_HFLIP_MASK :
-		      val | FIELD_PREP(OV01A10_HFLIP_MASK, 0x1);
-
-	return ov01a10_write_reg(ov01a10, OV01A10_REG_FORMAT1, 1, val);
+	return ret;
 }
 
 static int ov01a10_set_vflip(struct ov01a10 *ov01a10, u32 vflip)
 {
-	int ret;
 	u32 val, offset;
+	int ret = 0;
 
 	offset = vflip ? 0x9 : 0x8;
-	ret = ov01a10_write_reg(ov01a10, OV01A10_REG_Y_WIN, 1, offset);
-	if (ret)
-		return ret;
-
-	ret = ov01a10_read_reg(ov01a10, OV01A10_REG_FORMAT1, 1, &val);
-	if (ret)
-		return ret;
+	val = vflip ? FIELD_PREP(OV01A10_VFLIP_MASK, 0x1) : 0;
 
-	val = vflip ? val | FIELD_PREP(OV01A10_VFLIP_MASK, 0x1) :
-		val & ~OV01A10_VFLIP_MASK;
+	cci_write(ov01a10->regmap, OV01A10_REG_Y_WIN, offset, &ret);
+	cci_update_bits(ov01a10->regmap, OV01A10_REG_FORMAT1,
+			OV01A10_VFLIP_MASK, val, &ret);
 
-	return ov01a10_write_reg(ov01a10, OV01A10_REG_FORMAT1, 1, val);
+	return ret;
 }
 
 static int ov01a10_set_ctrl(struct v4l2_ctrl *ctrl)
@@ -474,8 +374,8 @@ static int ov01a10_set_ctrl(struct v4l2_ctrl *ctrl)
 
 	switch (ctrl->id) {
 	case V4L2_CID_ANALOGUE_GAIN:
-		ret = ov01a10_write_reg(ov01a10, OV01A10_REG_ANALOG_GAIN, 2,
-					ctrl->val);
+		ret = cci_write(ov01a10->regmap, OV01A10_REG_ANALOG_GAIN,
+				ctrl->val, NULL);
 		break;
 
 	case V4L2_CID_DIGITAL_GAIN:
@@ -483,13 +383,13 @@ static int ov01a10_set_ctrl(struct v4l2_ctrl *ctrl)
 		break;
 
 	case V4L2_CID_EXPOSURE:
-		ret = ov01a10_write_reg(ov01a10, OV01A10_REG_EXPOSURE, 2,
-					ctrl->val);
+		ret = cci_write(ov01a10->regmap, OV01A10_REG_EXPOSURE,
+				ctrl->val, NULL);
 		break;
 
 	case V4L2_CID_VBLANK:
-		ret = ov01a10_write_reg(ov01a10, OV01A10_REG_VTS, 2,
-					ov01a10->cur_mode->height + ctrl->val);
+		ret = cci_write(ov01a10->regmap, OV01A10_REG_VTS,
+				ov01a10->cur_mode->height + ctrl->val, NULL);
 		break;
 
 	case V4L2_CID_TEST_PATTERN:
@@ -630,14 +530,16 @@ static int ov01a10_start_streaming(struct ov01a10 *ov01a10)
 
 	link_freq_index = ov01a10->cur_mode->link_freq_index;
 	reg_list = &link_freq_configs[link_freq_index].reg_list;
-	ret = ov01a10_write_reg_list(ov01a10, reg_list);
+	ret = regmap_multi_reg_write(ov01a10->regmap, reg_list->regs,
+				     reg_list->num_of_regs);
 	if (ret) {
 		dev_err(&client->dev, "failed to set plls\n");
 		return ret;
 	}
 
 	reg_list = &ov01a10->cur_mode->reg_list;
-	ret = ov01a10_write_reg_list(ov01a10, reg_list);
+	ret = regmap_multi_reg_write(ov01a10->regmap, reg_list->regs,
+				     reg_list->num_of_regs);
 	if (ret) {
 		dev_err(&client->dev, "failed to set mode\n");
 		return ret;
@@ -647,23 +549,14 @@ static int ov01a10_start_streaming(struct ov01a10 *ov01a10)
 	if (ret)
 		return ret;
 
-	ret = ov01a10_write_reg(ov01a10, OV01A10_REG_MODE_SELECT, 1,
-				OV01A10_MODE_STREAMING);
-	if (ret)
-		dev_err(&client->dev, "failed to start streaming\n");
-
-	return ret;
+	return cci_write(ov01a10->regmap, OV01A10_REG_MODE_SELECT,
+			 OV01A10_MODE_STREAMING, NULL);
 }
 
 static void ov01a10_stop_streaming(struct ov01a10 *ov01a10)
 {
-	struct i2c_client *client = v4l2_get_subdevdata(&ov01a10->sd);
-	int ret = 0;
-
-	ret = ov01a10_write_reg(ov01a10, OV01A10_REG_MODE_SELECT, 1,
-				OV01A10_MODE_STANDBY);
-	if (ret)
-		dev_err(&client->dev, "failed to stop streaming\n");
+	cci_write(ov01a10->regmap, OV01A10_REG_MODE_SELECT,
+		  OV01A10_MODE_STANDBY, NULL);
 }
 
 static int ov01a10_set_stream(struct v4l2_subdev *sd, int enable)
@@ -841,14 +734,14 @@ static int ov01a10_identify_module(struct ov01a10 *ov01a10)
 {
 	struct i2c_client *client = v4l2_get_subdevdata(&ov01a10->sd);
 	int ret;
-	u32 val;
+	u64 val;
 
-	ret = ov01a10_read_reg(ov01a10, OV01A10_REG_CHIP_ID, 3, &val);
+	ret = cci_read(ov01a10->regmap, OV01A10_REG_CHIP_ID, &val, NULL);
 	if (ret)
 		return ret;
 
 	if (val != OV01A10_CHIP_ID) {
-		dev_err(&client->dev, "chip id mismatch: %x!=%x\n",
+		dev_err(&client->dev, "chip id mismatch: %x!=%llx\n",
 			OV01A10_CHIP_ID, val);
 		return -EIO;
 	}
@@ -879,6 +772,10 @@ static int ov01a10_probe(struct i2c_client *client)
 	if (!ov01a10)
 		return -ENOMEM;
 
+	ov01a10->regmap = devm_cci_regmap_init_i2c(client, 16);
+	if (IS_ERR(ov01a10->regmap))
+		return PTR_ERR(ov01a10->regmap);
+
 	v4l2_i2c_subdev_init(&ov01a10->sd, client, &ov01a10_subdev_ops);
 	ov01a10->sd.internal_ops = &ov01a10_internal_ops;
 
-- 
2.47.3

[PATCH v2 09/23] media: i2c: ov01a10: Remove overly verbose probe() error reporting

[Sakari Ailus]

From: Hans de Goede <hansg@kernel.org>

Many of the functions called from ov01a10_probe() are expected to never
fail and they should all already log some message if they fail. Remove
the unnecessarily verbose dev_err[_probe]() calls from the error-exit
paths in probe().

Signed-off-by: Hans de Goede <hansg@kernel.org>
Tested-by: Mehdi Djait <mehdi.djait@linux.intel.com> # Dell XPS 9315
Reviewed-by: Mehdi Djait <mehdi.djait@linux.intel.com>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/ov01a10.c | 19 +++++--------------
 1 file changed, 5 insertions(+), 14 deletions(-)

diff --git a/drivers/media/i2c/ov01a10.c b/drivers/media/i2c/ov01a10.c
index 34b7ab6eb286..f7aea9740a53 100644
--- a/drivers/media/i2c/ov01a10.c
+++ b/drivers/media/i2c/ov01a10.c
@@ -781,16 +781,13 @@ static int ov01a10_probe(struct i2c_client *client)
 
 	ret = ov01a10_identify_module(ov01a10);
 	if (ret)
-		return dev_err_probe(dev, ret,
-				     "failed to find sensor\n");
+		return ret;
 
 	ov01a10->cur_mode = &supported_modes[0];
 
 	ret = ov01a10_init_controls(ov01a10);
-	if (ret) {
-		dev_err(dev, "failed to init controls: %d\n", ret);
+	if (ret)
 		return ret;
-	}
 
 	ov01a10->sd.state_lock = ov01a10->ctrl_handler.lock;
 	ov01a10->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
@@ -799,16 +796,12 @@ static int ov01a10_probe(struct i2c_client *client)
 	ov01a10->pad.flags = MEDIA_PAD_FL_SOURCE;
 
 	ret = media_entity_pads_init(&ov01a10->sd.entity, 1, &ov01a10->pad);
-	if (ret) {
-		dev_err(dev, "Failed to init entity pads: %d\n", ret);
+	if (ret)
 		goto err_handler_free;
-	}
 
 	ret = v4l2_subdev_init_finalize(&ov01a10->sd);
-	if (ret) {
-		dev_err(dev, "Failed to allocate subdev state: %d\n", ret);
+	if (ret)
 		goto err_media_entity_cleanup;
-	}
 
 	/*
 	 * Device is already turned on by i2c-core with ACPI domain PM.
@@ -819,10 +812,8 @@ static int ov01a10_probe(struct i2c_client *client)
 	pm_runtime_idle(dev);
 
 	ret = v4l2_async_register_subdev_sensor(&ov01a10->sd);
-	if (ret < 0) {
-		dev_err(dev, "Failed to register subdev: %d\n", ret);
+	if (ret)
 		goto err_pm_disable;
-	}
 
 	return 0;
 
-- 
2.47.3

[PATCH v2 10/23] media: i2c: ov01a10: Store dev pointer in struct ov01a10

[Sakari Ailus]

From: Hans de Goede <hansg@kernel.org>

Now that the cci_* register access helpers are used we no longer need
the i2c_client in various functions.

Some code is still getting the client just to be able to get to the device
pointer. Directly store a struct device *dev pointing to &client->dev
inside struct ov01a10 to make the code simpler.

This also fixes a mismatch of using dev vs &client->dev in the
runtime_pm_*() calls in probe().

Signed-off-by: Hans de Goede <hansg@kernel.org>
Tested-by: Mehdi Djait <mehdi.djait@linux.intel.com> # Dell XPS 9315
Reviewed-by: Mehdi Djait <mehdi.djait@linux.intel.com>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/ov01a10.c | 35 ++++++++++++++++-------------------
 1 file changed, 16 insertions(+), 19 deletions(-)

diff --git a/drivers/media/i2c/ov01a10.c b/drivers/media/i2c/ov01a10.c
index f7aea9740a53..7677860c28ef 100644
--- a/drivers/media/i2c/ov01a10.c
+++ b/drivers/media/i2c/ov01a10.c
@@ -279,6 +279,7 @@ static const struct ov01a10_mode supported_modes[] = {
 };
 
 struct ov01a10 {
+	struct device *dev;
 	struct regmap *regmap;
 	struct v4l2_subdev sd;
 	struct media_pad pad;
@@ -356,7 +357,6 @@ static int ov01a10_set_ctrl(struct v4l2_ctrl *ctrl)
 {
 	struct ov01a10 *ov01a10 = container_of(ctrl->handler,
 					       struct ov01a10, ctrl_handler);
-	struct i2c_client *client = v4l2_get_subdevdata(&ov01a10->sd);
 	s64 exposure_max;
 	int ret = 0;
 
@@ -369,7 +369,7 @@ static int ov01a10_set_ctrl(struct v4l2_ctrl *ctrl)
 					 exposure_max);
 	}
 
-	if (!pm_runtime_get_if_in_use(&client->dev))
+	if (!pm_runtime_get_if_in_use(ov01a10->dev))
 		return 0;
 
 	switch (ctrl->id) {
@@ -409,7 +409,7 @@ static int ov01a10_set_ctrl(struct v4l2_ctrl *ctrl)
 		break;
 	}
 
-	pm_runtime_put(&client->dev);
+	pm_runtime_put(ov01a10->dev);
 
 	return ret;
 }
@@ -420,7 +420,6 @@ static const struct v4l2_ctrl_ops ov01a10_ctrl_ops = {
 
 static int ov01a10_init_controls(struct ov01a10 *ov01a10)
 {
-	struct i2c_client *client = v4l2_get_subdevdata(&ov01a10->sd);
 	struct v4l2_fwnode_device_properties props;
 	u32 vblank_min, vblank_max, vblank_default;
 	struct v4l2_ctrl_handler *ctrl_hdlr;
@@ -429,7 +428,7 @@ static int ov01a10_init_controls(struct ov01a10 *ov01a10)
 	int ret = 0;
 	int size;
 
-	ret = v4l2_fwnode_device_parse(&client->dev, &props);
+	ret = v4l2_fwnode_device_parse(ov01a10->dev, &props);
 	if (ret)
 		return ret;
 
@@ -523,7 +522,6 @@ static void ov01a10_update_pad_format(const struct ov01a10_mode *mode,
 
 static int ov01a10_start_streaming(struct ov01a10 *ov01a10)
 {
-	struct i2c_client *client = v4l2_get_subdevdata(&ov01a10->sd);
 	const struct ov01a10_reg_list *reg_list;
 	int link_freq_index;
 	int ret = 0;
@@ -533,7 +531,7 @@ static int ov01a10_start_streaming(struct ov01a10 *ov01a10)
 	ret = regmap_multi_reg_write(ov01a10->regmap, reg_list->regs,
 				     reg_list->num_of_regs);
 	if (ret) {
-		dev_err(&client->dev, "failed to set plls\n");
+		dev_err(ov01a10->dev, "failed to set plls\n");
 		return ret;
 	}
 
@@ -541,7 +539,7 @@ static int ov01a10_start_streaming(struct ov01a10 *ov01a10)
 	ret = regmap_multi_reg_write(ov01a10->regmap, reg_list->regs,
 				     reg_list->num_of_regs);
 	if (ret) {
-		dev_err(&client->dev, "failed to set mode\n");
+		dev_err(ov01a10->dev, "failed to set mode\n");
 		return ret;
 	}
 
@@ -562,25 +560,24 @@ static void ov01a10_stop_streaming(struct ov01a10 *ov01a10)
 static int ov01a10_set_stream(struct v4l2_subdev *sd, int enable)
 {
 	struct ov01a10 *ov01a10 = to_ov01a10(sd);
-	struct i2c_client *client = v4l2_get_subdevdata(sd);
 	struct v4l2_subdev_state *state;
 	int ret = 0;
 
 	state = v4l2_subdev_lock_and_get_active_state(sd);
 
 	if (enable) {
-		ret = pm_runtime_resume_and_get(&client->dev);
+		ret = pm_runtime_resume_and_get(ov01a10->dev);
 		if (ret < 0)
 			goto unlock;
 
 		ret = ov01a10_start_streaming(ov01a10);
 		if (ret) {
-			pm_runtime_put(&client->dev);
+			pm_runtime_put(ov01a10->dev);
 			goto unlock;
 		}
 	} else {
 		ov01a10_stop_streaming(ov01a10);
-		pm_runtime_put(&client->dev);
+		pm_runtime_put(ov01a10->dev);
 	}
 
 unlock:
@@ -732,7 +729,6 @@ static const struct media_entity_operations ov01a10_subdev_entity_ops = {
 
 static int ov01a10_identify_module(struct ov01a10 *ov01a10)
 {
-	struct i2c_client *client = v4l2_get_subdevdata(&ov01a10->sd);
 	int ret;
 	u64 val;
 
@@ -741,7 +737,7 @@ static int ov01a10_identify_module(struct ov01a10 *ov01a10)
 		return ret;
 
 	if (val != OV01A10_CHIP_ID) {
-		dev_err(&client->dev, "chip id mismatch: %x!=%llx\n",
+		dev_err(ov01a10->dev, "chip id mismatch: %x!=%llx\n",
 			OV01A10_CHIP_ID, val);
 		return -EIO;
 	}
@@ -764,14 +760,15 @@ static void ov01a10_remove(struct i2c_client *client)
 
 static int ov01a10_probe(struct i2c_client *client)
 {
-	struct device *dev = &client->dev;
 	struct ov01a10 *ov01a10;
 	int ret = 0;
 
-	ov01a10 = devm_kzalloc(dev, sizeof(*ov01a10), GFP_KERNEL);
+	ov01a10 = devm_kzalloc(&client->dev, sizeof(*ov01a10), GFP_KERNEL);
 	if (!ov01a10)
 		return -ENOMEM;
 
+	ov01a10->dev = &client->dev;
+
 	ov01a10->regmap = devm_cci_regmap_init_i2c(client, 16);
 	if (IS_ERR(ov01a10->regmap))
 		return PTR_ERR(ov01a10->regmap);
@@ -808,8 +805,8 @@ static int ov01a10_probe(struct i2c_client *client)
 	 * Enable runtime PM and turn off the device.
 	 */
 	pm_runtime_set_active(&client->dev);
-	pm_runtime_enable(dev);
-	pm_runtime_idle(dev);
+	pm_runtime_enable(&client->dev);
+	pm_runtime_idle(&client->dev);
 
 	ret = v4l2_async_register_subdev_sensor(&ov01a10->sd);
 	if (ret)
@@ -818,7 +815,7 @@ static int ov01a10_probe(struct i2c_client *client)
 	return 0;
 
 err_pm_disable:
-	pm_runtime_disable(dev);
+	pm_runtime_disable(&client->dev);
 	pm_runtime_set_suspended(&client->dev);
 	v4l2_subdev_cleanup(&ov01a10->sd);
 
-- 
2.47.3

[PATCH v2 11/23] media: i2c: ov01a10: Add ov01a10_check_hwcfg() function

[Sakari Ailus]

From: Hans de Goede <hansg@kernel.org>

Add a function to check that the number of mipi-lanes and there frequency
are what the driver expects.

Signed-off-by: Hans de Goede <hansg@kernel.org>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/ov01a10.c | 58 ++++++++++++++++++++++++++++++++++---
 1 file changed, 54 insertions(+), 4 deletions(-)

diff --git a/drivers/media/i2c/ov01a10.c b/drivers/media/i2c/ov01a10.c
index 7677860c28ef..0ef4bbc93d66 100644
--- a/drivers/media/i2c/ov01a10.c
+++ b/drivers/media/i2c/ov01a10.c
@@ -293,6 +293,7 @@ struct ov01a10 {
 	struct v4l2_ctrl *exposure;
 
 	const struct ov01a10_mode *cur_mode;
+	u32 link_freq_index;
 };
 
 static inline struct ov01a10 *to_ov01a10(struct v4l2_subdev *subdev)
@@ -426,7 +427,6 @@ static int ov01a10_init_controls(struct ov01a10 *ov01a10)
 	const struct ov01a10_mode *cur_mode;
 	s64 exposure_max, h_blank;
 	int ret = 0;
-	int size;
 
 	ret = v4l2_fwnode_device_parse(ov01a10->dev, &props);
 	if (ret)
@@ -438,12 +438,11 @@ static int ov01a10_init_controls(struct ov01a10 *ov01a10)
 		return ret;
 
 	cur_mode = ov01a10->cur_mode;
-	size = ARRAY_SIZE(link_freq_menu_items);
 
 	ov01a10->link_freq = v4l2_ctrl_new_int_menu(ctrl_hdlr,
 						    &ov01a10_ctrl_ops,
 						    V4L2_CID_LINK_FREQ,
-						    size - 1, 0,
+						    ov01a10->link_freq_index, 0,
 						    link_freq_menu_items);
 	if (ov01a10->link_freq)
 		ov01a10->link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY;
@@ -745,6 +744,53 @@ static int ov01a10_identify_module(struct ov01a10 *ov01a10)
 	return 0;
 }
 
+static int ov01a10_check_hwcfg(struct ov01a10 *ov01a10)
+{
+	struct v4l2_fwnode_endpoint bus_cfg = {
+		.bus_type = V4L2_MBUS_CSI2_DPHY
+	};
+	struct fwnode_handle *ep, *fwnode = dev_fwnode(ov01a10->dev);
+	unsigned long link_freq_bitmap;
+	int ret;
+
+	/*
+	 * 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 dev_err_probe(ov01a10->dev, -EPROBE_DEFER,
+				     "waiting for fwnode graph endpoint\n");
+
+	ret = v4l2_fwnode_endpoint_alloc_parse(ep, &bus_cfg);
+	fwnode_handle_put(ep);
+	if (ret)
+		return dev_err_probe(ov01a10->dev, ret, "parsing endpoint\n");
+
+	ret = v4l2_link_freq_to_bitmap(ov01a10->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 check_hwcfg_error;
+
+	/* v4l2_link_freq_to_bitmap() guarantees at least 1 bit is set */
+	ov01a10->link_freq_index = ffs(link_freq_bitmap) - 1;
+
+	if (bus_cfg.bus.mipi_csi2.num_data_lanes != OV01A10_DATA_LANES) {
+		ret = dev_err_probe(ov01a10->dev, -EINVAL,
+				    "number of CSI2 data lanes %u is not supported\n",
+				    bus_cfg.bus.mipi_csi2.num_data_lanes);
+		goto check_hwcfg_error;
+	}
+
+check_hwcfg_error:
+	v4l2_fwnode_endpoint_free(&bus_cfg);
+	return ret;
+}
+
 static void ov01a10_remove(struct i2c_client *client)
 {
 	struct v4l2_subdev *sd = i2c_get_clientdata(client);
@@ -761,7 +807,7 @@ static void ov01a10_remove(struct i2c_client *client)
 static int ov01a10_probe(struct i2c_client *client)
 {
 	struct ov01a10 *ov01a10;
-	int ret = 0;
+	int ret;
 
 	ov01a10 = devm_kzalloc(&client->dev, sizeof(*ov01a10), GFP_KERNEL);
 	if (!ov01a10)
@@ -776,6 +822,10 @@ static int ov01a10_probe(struct i2c_client *client)
 	v4l2_i2c_subdev_init(&ov01a10->sd, client, &ov01a10_subdev_ops);
 	ov01a10->sd.internal_ops = &ov01a10_internal_ops;
 
+	ret = ov01a10_check_hwcfg(ov01a10);
+	if (ret)
+		return ret;
+
 	ret = ov01a10_identify_module(ov01a10);
 	if (ret)
 		return ret;
-- 
2.47.3

[PATCH v2 12/23] media: i2c: ov01a10: Add power on/off sequencing support

[Sakari Ailus]

From: Hans de Goede <hansg@kernel.org>

So far the ov01a10 driver has only been used on laptops with an IVSC chip
where the IVSC chip controls the power on/off sequencing of the sensor.

But there are also designs with an ov01a10 sensor where the kernel needs
to directly take care of the power-sequencing, controlling clks, regulators
and GPIOs. Add support for these designs.

The 2 ms minimum reset assertion time is taken from other Omnivision sensor
drivers like the ov5675. The 20 ms delay after reset de-assert comes from
the out of tree ov01a1s driver.

Signed-off-by: Hans de Goede <hansg@kernel.org>
Reviewed-by: Mehdi Djait <mehdi.djait@linux.intel.com>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/ov01a10.c | 126 +++++++++++++++++++++++++++++++++++-
 1 file changed, 123 insertions(+), 3 deletions(-)

diff --git a/drivers/media/i2c/ov01a10.c b/drivers/media/i2c/ov01a10.c
index 0ef4bbc93d66..fee24acef4b2 100644
--- a/drivers/media/i2c/ov01a10.c
+++ b/drivers/media/i2c/ov01a10.c
@@ -7,10 +7,14 @@
 
 #include <linux/acpi.h>
 #include <linux/bitfield.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/regulator/consumer.h>
 
 #include <media/v4l2-cci.h>
 #include <media/v4l2-ctrls.h>
@@ -20,6 +24,7 @@
 #define OV01A10_LINK_FREQ_400MHZ	400000000ULL
 #define OV01A10_SCLK			80000000LL
 #define OV01A10_DATA_LANES		1
+#define OV01A10_MCLK			19200000
 
 #define OV01A10_REG_CHIP_ID		CCI_REG24(0x300a)
 #define OV01A10_CHIP_ID			0x560141
@@ -278,6 +283,12 @@ static const struct ov01a10_mode supported_modes[] = {
 	},
 };
 
+static const char * const ov01a10_supply_names[] = {
+	"dovdd",	/* Digital I/O power */
+	"avdd",		/* Analog power */
+	"dvdd",		/* Digital core power */
+};
+
 struct ov01a10 {
 	struct device *dev;
 	struct regmap *regmap;
@@ -294,6 +305,11 @@ struct ov01a10 {
 
 	const struct ov01a10_mode *cur_mode;
 	u32 link_freq_index;
+
+	struct clk *clk;
+	struct gpio_desc *reset;
+	struct gpio_desc *powerdown;
+	struct regulator_bulk_data supplies[ARRAY_SIZE(ov01a10_supply_names)];
 };
 
 static inline struct ov01a10 *to_ov01a10(struct v4l2_subdev *subdev)
@@ -726,6 +742,92 @@ static const struct media_entity_operations ov01a10_subdev_entity_ops = {
 	.link_validate = v4l2_subdev_link_validate,
 };
 
+static int ov01a10_get_pm_resources(struct ov01a10 *ov01a10)
+{
+	unsigned long freq;
+	int i, ret;
+
+	ov01a10->clk = devm_v4l2_sensor_clk_get(ov01a10->dev, NULL);
+	if (IS_ERR(ov01a10->clk))
+		return dev_err_probe(ov01a10->dev, PTR_ERR(ov01a10->clk),
+				     "getting clock\n");
+
+	freq = clk_get_rate(ov01a10->clk);
+	if (freq != OV01A10_MCLK)
+		return dev_err_probe(ov01a10->dev, -EINVAL,
+				     "external clock %lu is not supported",
+				     freq);
+
+	ov01a10->reset = devm_gpiod_get_optional(ov01a10->dev, "reset",
+						 GPIOD_OUT_HIGH);
+	if (IS_ERR(ov01a10->reset))
+		return dev_err_probe(ov01a10->dev, PTR_ERR(ov01a10->reset),
+				     "getting reset gpio\n");
+
+	ov01a10->powerdown = devm_gpiod_get_optional(ov01a10->dev, "powerdown",
+						 GPIOD_OUT_HIGH);
+	if (IS_ERR(ov01a10->powerdown))
+		return dev_err_probe(ov01a10->dev, PTR_ERR(ov01a10->powerdown),
+				     "getting powerdown gpio\n");
+
+	for (i = 0; i < ARRAY_SIZE(ov01a10_supply_names); i++)
+		ov01a10->supplies[i].supply = ov01a10_supply_names[i];
+
+	ret = devm_regulator_bulk_get(ov01a10->dev,
+				      ARRAY_SIZE(ov01a10_supply_names),
+				      ov01a10->supplies);
+	if (ret)
+		return dev_err_probe(ov01a10->dev, ret, "getting regulators\n");
+
+	return 0;
+}
+
+static int ov01a10_power_on(struct device *dev)
+{
+	struct v4l2_subdev *sd = dev_get_drvdata(dev);
+	struct ov01a10 *ov01a10 = to_ov01a10(sd);
+	int ret;
+
+	ret = clk_prepare_enable(ov01a10->clk);
+	if (ret) {
+		dev_err(dev, "Error enabling clk: %d\n", ret);
+		return ret;
+	}
+
+	ret = regulator_bulk_enable(ARRAY_SIZE(ov01a10_supply_names),
+				    ov01a10->supplies);
+	if (ret) {
+		dev_err(dev, "Error enabling regulators: %d\n", ret);
+		clk_disable_unprepare(ov01a10->clk);
+		return ret;
+	}
+
+	if (ov01a10->reset || ov01a10->powerdown) {
+		/* Assert reset/powerdown for at least 2ms on back to back off-on */
+		fsleep(2000);
+		gpiod_set_value_cansleep(ov01a10->powerdown, 0);
+		gpiod_set_value_cansleep(ov01a10->reset, 0);
+		fsleep(20000);
+	}
+
+	return 0;
+}
+
+static int ov01a10_power_off(struct device *dev)
+{
+	struct v4l2_subdev *sd = dev_get_drvdata(dev);
+	struct ov01a10 *ov01a10 = to_ov01a10(sd);
+
+	gpiod_set_value_cansleep(ov01a10->reset, 1);
+	gpiod_set_value_cansleep(ov01a10->powerdown, 1);
+
+	regulator_bulk_disable(ARRAY_SIZE(ov01a10_supply_names),
+			       ov01a10->supplies);
+
+	clk_disable_unprepare(ov01a10->clk);
+	return 0;
+}
+
 static int ov01a10_identify_module(struct ov01a10 *ov01a10)
 {
 	int ret;
@@ -801,7 +903,10 @@ static void ov01a10_remove(struct i2c_client *client)
 	v4l2_ctrl_handler_free(sd->ctrl_handler);
 
 	pm_runtime_disable(&client->dev);
-	pm_runtime_set_suspended(&client->dev);
+	if (!pm_runtime_status_suspended(&client->dev)) {
+		ov01a10_power_off(&client->dev);
+		pm_runtime_set_suspended(&client->dev);
+	}
 }
 
 static int ov01a10_probe(struct i2c_client *client)
@@ -826,15 +931,23 @@ static int ov01a10_probe(struct i2c_client *client)
 	if (ret)
 		return ret;
 
-	ret = ov01a10_identify_module(ov01a10);
+	ret = ov01a10_get_pm_resources(ov01a10);
+	if (ret)
+		return ret;
+
+	ret = ov01a10_power_on(&client->dev);
 	if (ret)
 		return ret;
 
+	ret = ov01a10_identify_module(ov01a10);
+	if (ret)
+		goto err_power_off;
+
 	ov01a10->cur_mode = &supported_modes[0];
 
 	ret = ov01a10_init_controls(ov01a10);
 	if (ret)
-		return ret;
+		goto err_power_off;
 
 	ov01a10->sd.state_lock = ov01a10->ctrl_handler.lock;
 	ov01a10->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
@@ -875,9 +988,15 @@ static int ov01a10_probe(struct i2c_client *client)
 err_handler_free:
 	v4l2_ctrl_handler_free(ov01a10->sd.ctrl_handler);
 
+err_power_off:
+	ov01a10_power_off(&client->dev);
+
 	return ret;
 }
 
+static DEFINE_RUNTIME_DEV_PM_OPS(ov01a10_pm_ops, ov01a10_power_off,
+				 ov01a10_power_on, NULL);
+
 #ifdef CONFIG_ACPI
 static const struct acpi_device_id ov01a10_acpi_ids[] = {
 	{ "OVTI01A0" },
@@ -890,6 +1009,7 @@ MODULE_DEVICE_TABLE(acpi, ov01a10_acpi_ids);
 static struct i2c_driver ov01a10_i2c_driver = {
 	.driver = {
 		.name = "ov01a10",
+		.pm = pm_sleep_ptr(&ov01a10_pm_ops),
 		.acpi_match_table = ACPI_PTR(ov01a10_acpi_ids),
 	},
 	.probe = ov01a10_probe,
-- 
2.47.3

[PATCH v2 13/23] media: i2c: ov01a10: Don't update pixel_rate and link_freq from set_fmt

[Sakari Ailus]

From: Hans de Goede <hansg@kernel.org>

The pixel_rate and link_freq never change, stop updating them on every
set_fmt.

Signed-off-by: Hans de Goede <hansg@kernel.org>
Tested-by: Mehdi Djait <mehdi.djait@linux.intel.com> # Dell XPS 9315
Reviewed-by: Mehdi Djait <mehdi.djait@linux.intel.com>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/ov01a10.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/media/i2c/ov01a10.c b/drivers/media/i2c/ov01a10.c
index fee24acef4b2..747e91cd8828 100644
--- a/drivers/media/i2c/ov01a10.c
+++ b/drivers/media/i2c/ov01a10.c
@@ -619,8 +619,6 @@ static int ov01a10_set_format(struct v4l2_subdev *sd,
 
 	if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
 		ov01a10->cur_mode = mode;
-		__v4l2_ctrl_s_ctrl(ov01a10->link_freq, mode->link_freq_index);
-		__v4l2_ctrl_s_ctrl_int64(ov01a10->pixel_rate, OV01A10_SCLK);
 
 		vblank_def = mode->vts_def - mode->height;
 		__v4l2_ctrl_modify_range(ov01a10->vblank,
-- 
2.47.3

[PATCH v2 14/23] media: i2c: ov01a10: Move setting of ctrl->flags to after checking ctrl_hdlr->error

[Sakari Ailus]

From: Hans de Goede <hansg@kernel.org>

Instead of checking successful creation of the link_freq and vblank
controls, set their flags after checking ctrl_hdlr->error where it
is guaranteed that the controls will exist.

Signed-off-by: Hans de Goede <hansg@kernel.org>
Tested-by: Mehdi Djait <mehdi.djait@linux.intel.com> # Dell XPS 9315
Reviewed-by: Mehdi Djait <mehdi.djait@linux.intel.com>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/ov01a10.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/media/i2c/ov01a10.c b/drivers/media/i2c/ov01a10.c
index 747e91cd8828..6dcd982cf8eb 100644
--- a/drivers/media/i2c/ov01a10.c
+++ b/drivers/media/i2c/ov01a10.c
@@ -460,8 +460,6 @@ static int ov01a10_init_controls(struct ov01a10 *ov01a10)
 						    V4L2_CID_LINK_FREQ,
 						    ov01a10->link_freq_index, 0,
 						    link_freq_menu_items);
-	if (ov01a10->link_freq)
-		ov01a10->link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY;
 
 	ov01a10->pixel_rate = v4l2_ctrl_new_std(ctrl_hdlr, &ov01a10_ctrl_ops,
 						V4L2_CID_PIXEL_RATE, 0,
@@ -478,8 +476,6 @@ static int ov01a10_init_controls(struct ov01a10 *ov01a10)
 	ov01a10->hblank = v4l2_ctrl_new_std(ctrl_hdlr, &ov01a10_ctrl_ops,
 					    V4L2_CID_HBLANK, h_blank, h_blank,
 					    1, h_blank);
-	if (ov01a10->hblank)
-		ov01a10->hblank->flags |= V4L2_CTRL_FLAG_READ_ONLY;
 
 	v4l2_ctrl_new_std(ctrl_hdlr, &ov01a10_ctrl_ops, V4L2_CID_ANALOGUE_GAIN,
 			  OV01A10_ANAL_GAIN_MIN, OV01A10_ANAL_GAIN_MAX,
@@ -516,6 +512,9 @@ static int ov01a10_init_controls(struct ov01a10 *ov01a10)
 		goto fail;
 	}
 
+	ov01a10->link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY;
+	ov01a10->hblank->flags |= V4L2_CTRL_FLAG_READ_ONLY;
+
 	ov01a10->sd.ctrl_handler = ctrl_hdlr;
 
 	return 0;
-- 
2.47.3

[PATCH v2 15/23] media: i2c: ov01a10: Use native and default for pixel-array size names

[Sakari Ailus]

From: Hans de Goede <hansg@kernel.org>

According to the OV01A10 product-brief PDF the OV01A10 has an active pixel
array size of 1296x816. In otherwords the native and active sizes are
the same.

Replace the (misspelled) ACTIVE defines for the default resolution of
1280x800 with DEFAULT to avoid giving the impression that the active pixel
array size is only 1280x800.

And replace PIXEL_ARRAY with NATIVE to make clear this is the native pixel
array size / to match the V4L2_SEL_TGT_NATIVE_SIZE naming.

Signed-off-by: Hans de Goede <hansg@kernel.org>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/ov01a10.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/media/i2c/ov01a10.c b/drivers/media/i2c/ov01a10.c
index 6dcd982cf8eb..3eb6445b8f00 100644
--- a/drivers/media/i2c/ov01a10.c
+++ b/drivers/media/i2c/ov01a10.c
@@ -34,10 +34,10 @@
 #define OV01A10_MODE_STREAMING		0x01
 
 /* pixel array */
-#define OV01A10_PIXEL_ARRAY_WIDTH	1296
-#define OV01A10_PIXEL_ARRAY_HEIGHT	816
-#define OV01A10_ACITVE_WIDTH		1280
-#define OV01A10_ACITVE_HEIGHT		800
+#define OV01A10_NATIVE_WIDTH		1296
+#define OV01A10_NATIVE_HEIGHT		816
+#define OV01A10_DEFAULT_WIDTH		1280
+#define OV01A10_DEFAULT_HEIGHT		800
 
 /* vertical and horizontal timings */
 #define OV01A10_REG_VTS			CCI_REG16(0x380e)
@@ -270,8 +270,8 @@ static const struct ov01a10_link_freq_config link_freq_configs[] = {
 
 static const struct ov01a10_mode supported_modes[] = {
 	{
-		.width = OV01A10_ACITVE_WIDTH,
-		.height = OV01A10_ACITVE_HEIGHT,
+		.width = OV01A10_DEFAULT_WIDTH,
+		.height = OV01A10_DEFAULT_HEIGHT,
 		.hts = OV01A10_HTS_DEF,
 		.vts_def = OV01A10_VTS_DEF,
 		.vts_min = OV01A10_VTS_MIN,
@@ -642,8 +642,8 @@ static int ov01a10_init_state(struct v4l2_subdev *sd,
 	struct v4l2_subdev_format fmt = {
 		.which = V4L2_SUBDEV_FORMAT_TRY,
 		.format = {
-			.width = OV01A10_ACITVE_WIDTH,
-			.height = OV01A10_ACITVE_HEIGHT,
+			.width = OV01A10_DEFAULT_WIDTH,
+			.height = OV01A10_DEFAULT_HEIGHT,
 		},
 	};
 
@@ -692,17 +692,17 @@ static int ov01a10_get_selection(struct v4l2_subdev *sd,
 	case V4L2_SEL_TGT_CROP_BOUNDS:
 		sel->r.top = 0;
 		sel->r.left = 0;
-		sel->r.width = OV01A10_PIXEL_ARRAY_WIDTH;
-		sel->r.height = OV01A10_PIXEL_ARRAY_HEIGHT;
+		sel->r.width = OV01A10_NATIVE_WIDTH;
+		sel->r.height = OV01A10_NATIVE_HEIGHT;
 		return 0;
 	case V4L2_SEL_TGT_CROP:
 	case V4L2_SEL_TGT_CROP_DEFAULT:
-		sel->r.top = (OV01A10_PIXEL_ARRAY_HEIGHT -
-			      OV01A10_ACITVE_HEIGHT) / 2;
-		sel->r.left = (OV01A10_PIXEL_ARRAY_WIDTH -
-			       OV01A10_ACITVE_WIDTH) / 2;
-		sel->r.width = OV01A10_ACITVE_WIDTH;
-		sel->r.height = OV01A10_ACITVE_HEIGHT;
+		sel->r.top = (OV01A10_NATIVE_HEIGHT -
+			      OV01A10_DEFAULT_HEIGHT) / 2;
+		sel->r.left = (OV01A10_NATIVE_WIDTH -
+			       OV01A10_DEFAULT_WIDTH) / 2;
+		sel->r.width = OV01A10_DEFAULT_WIDTH;
+		sel->r.height = OV01A10_DEFAULT_HEIGHT;
 		return 0;
 	}
 
-- 
2.47.3

[PATCH v2 16/23] media: i2c: ov01a10: Add cropping support / allow arbitrary sizes

[Sakari Ailus]

From: Hans de Goede <hansg@kernel.org>

Remove the fixed mode list and add cropping support. The main reason for
doing this is to allow libcamera to select 1292x812 instead of 1280x800
so that after the extra border which the CPU debayer code needs libcamera
can output 1280x720 instead of 1276x720.

This in turn allows google-meet to use 720p instead of it falling back
to a pretty bad 360p.

This has been tested on a Dell XPS 9320, with both libcamera as well as
with Intel's out-of-tree psys driver + proprietary userspace stack.

Libcamera asks for 1292x812 where as the Intel stack asks for 1280x800
and neither stack explicitly sets the crop-window. Hence the need for
ov01a10_set_format() to adjust the crop-window if necessary.

Note the differentiating between pattern_size and border_size is done in
preparation for adding support for the monochrome OV01A1B model where
coordinates still need to be aligned to a multiple of 2, but there will
be no need for a border (border_size=0).

Link: https://bugzilla.redhat.com/show_bug.cgi?id=2337593
Signed-off-by: Hans de Goede <hansg@kernel.org>
Tested-by: Mehdi Djait <mehdi.djait@linux.intel.com> # Dell XPS 9315
Reviewed-by: Mehdi Djait <mehdi.djait@linux.intel.com>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/ov01a10.c | 348 ++++++++++++++++++++++--------------
 1 file changed, 212 insertions(+), 136 deletions(-)

diff --git a/drivers/media/i2c/ov01a10.c b/drivers/media/i2c/ov01a10.c
index 3eb6445b8f00..349fd3d06df5 100644
--- a/drivers/media/i2c/ov01a10.c
+++ b/drivers/media/i2c/ov01a10.c
@@ -40,7 +40,6 @@
 #define OV01A10_DEFAULT_HEIGHT		800
 
 /* vertical and horizontal timings */
-#define OV01A10_REG_VTS			CCI_REG16(0x380e)
 #define OV01A10_VTS_DEF			0x0700
 #define OV01A10_VTS_MIN			0x0380
 #define OV01A10_VTS_MAX			0xffff
@@ -68,19 +67,26 @@
 #define OV01A10_DGTL_GAIN_STEP		1
 #define OV01A10_DGTL_GAIN_DEFAULT	1024
 
-/* test pattern control */
-#define OV01A10_REG_TEST_PATTERN	CCI_REG8(0x4503)
-#define OV01A10_TEST_PATTERN_ENABLE	BIT(7)
-#define OV01A10_LINK_FREQ_400MHZ_INDEX	0
+/* timing control */
+#define OV01A10_REG_X_ADDR_START	CCI_REG16(0x3800)
+#define OV01A10_REG_Y_ADDR_START	CCI_REG16(0x3802)
+#define OV01A10_REG_X_ADDR_END		CCI_REG16(0x3804)
+#define OV01A10_REG_Y_ADDR_END		CCI_REG16(0x3806)
+#define OV01A10_REG_X_OUTPUT_SIZE	CCI_REG16(0x3808)
+#define OV01A10_REG_Y_OUTPUT_SIZE	CCI_REG16(0x380a)
+#define OV01A10_REG_HTS			CCI_REG16(0x380c) /* in units of 2 pixels */
+#define OV01A10_REG_VTS			CCI_REG16(0x380e)
+#define OV01A10_REG_X_WIN		CCI_REG16(0x3810)
+#define OV01A10_REG_Y_WIN		CCI_REG16(0x3812)
 
 /* flip and mirror control */
 #define OV01A10_REG_FORMAT1		CCI_REG8(0x3820)
 #define OV01A10_VFLIP_MASK		BIT(4)
 #define OV01A10_HFLIP_MASK		BIT(3)
 
-/* window offset */
-#define OV01A10_REG_X_WIN		CCI_REG16(0x3810)
-#define OV01A10_REG_Y_WIN		CCI_REG16(0x3812)
+/* test pattern control */
+#define OV01A10_REG_TEST_PATTERN	CCI_REG8(0x4503)
+#define OV01A10_TEST_PATTERN_ENABLE	BIT(7)
 
 /*
  * The native ov01a10 bayer-pattern is GBRG, but there was a driver bug enabling
@@ -90,6 +96,7 @@
  * when hflip is *disabled*.
  */
 #define OV01A10_MEDIA_BUS_FMT		MEDIA_BUS_FMT_SBGGR10_1X10
+#define OV01A10_BAYER_PATTERN_SIZE	2 /* 2x2 */
 
 struct ov01a10_reg_list {
 	u32 num_of_regs;
@@ -100,17 +107,6 @@ struct ov01a10_link_freq_config {
 	const struct ov01a10_reg_list reg_list;
 };
 
-struct ov01a10_mode {
-	u32 width;
-	u32 height;
-	u32 hts;
-	u32 vts_def;
-	u32 vts_min;
-	u32 link_freq_index;
-
-	const struct ov01a10_reg_list reg_list;
-};
-
 static const struct reg_sequence mipi_data_rate_720mbps[] = {
 	{0x0103, 0x01},
 	{0x0302, 0x00},
@@ -127,7 +123,7 @@ static const struct reg_sequence mipi_data_rate_720mbps[] = {
 	{0x0325, 0x68},
 };
 
-static const struct reg_sequence sensor_1280x800_setting[] = {
+static const struct reg_sequence ov01a10_global_setting[] = {
 	{0x3002, 0xa1},
 	{0x301e, 0xf0},
 	{0x3022, 0x01},
@@ -179,26 +175,6 @@ static const struct reg_sequence sensor_1280x800_setting[] = {
 	{0x37e4, 0x04},
 	{0x37e5, 0x03},
 	{0x37e6, 0x04},
-	{0x3800, 0x00},
-	{0x3801, 0x00},
-	{0x3802, 0x00},
-	{0x3803, 0x00},
-	{0x3804, 0x05},
-	{0x3805, 0x0f},
-	{0x3806, 0x03},
-	{0x3807, 0x2f},
-	{0x3808, 0x05},
-	{0x3809, 0x00},
-	{0x380a, 0x03},
-	{0x380b, 0x20},
-	{0x380c, 0x02},
-	{0x380d, 0xe8},
-	{0x380e, 0x07},
-	{0x380f, 0x00},
-	{0x3810, 0x00},
-	{0x3811, 0x09},
-	{0x3812, 0x00},
-	{0x3813, 0x08},
 	{0x3814, 0x01},
 	{0x3815, 0x01},
 	{0x3816, 0x01},
@@ -260,7 +236,7 @@ static const s64 link_freq_menu_items[] = {
 };
 
 static const struct ov01a10_link_freq_config link_freq_configs[] = {
-	[OV01A10_LINK_FREQ_400MHZ_INDEX] = {
+	{
 		.reg_list = {
 			.num_of_regs = ARRAY_SIZE(mipi_data_rate_720mbps),
 			.regs = mipi_data_rate_720mbps,
@@ -268,19 +244,11 @@ static const struct ov01a10_link_freq_config link_freq_configs[] = {
 	},
 };
 
-static const struct ov01a10_mode supported_modes[] = {
-	{
-		.width = OV01A10_DEFAULT_WIDTH,
-		.height = OV01A10_DEFAULT_HEIGHT,
-		.hts = OV01A10_HTS_DEF,
-		.vts_def = OV01A10_VTS_DEF,
-		.vts_min = OV01A10_VTS_MIN,
-		.reg_list = {
-			.num_of_regs = ARRAY_SIZE(sensor_1280x800_setting),
-			.regs = sensor_1280x800_setting,
-		},
-		.link_freq_index = OV01A10_LINK_FREQ_400MHZ_INDEX,
-	},
+static const struct v4l2_rect ov01a10_default_crop = {
+	.left = (OV01A10_NATIVE_WIDTH - OV01A10_DEFAULT_WIDTH) / 2,
+	.top = (OV01A10_NATIVE_HEIGHT - OV01A10_DEFAULT_HEIGHT) / 2,
+	.width = OV01A10_DEFAULT_WIDTH,
+	.height = OV01A10_DEFAULT_HEIGHT,
 };
 
 static const char * const ov01a10_supply_names[] = {
@@ -303,7 +271,6 @@ struct ov01a10 {
 	struct v4l2_ctrl *hblank;
 	struct v4l2_ctrl *exposure;
 
-	const struct ov01a10_mode *cur_mode;
 	u32 link_freq_index;
 
 	struct clk *clk;
@@ -317,6 +284,22 @@ static inline struct ov01a10 *to_ov01a10(struct v4l2_subdev *subdev)
 	return container_of(subdev, struct ov01a10, sd);
 }
 
+static struct v4l2_mbus_framefmt *ov01a10_get_active_format(struct ov01a10 *ov01a10)
+{
+	struct v4l2_subdev_state *active_state =
+		v4l2_subdev_get_locked_active_state(&ov01a10->sd);
+
+	return v4l2_subdev_state_get_format(active_state, 0);
+}
+
+static struct v4l2_rect *ov01a10_get_active_crop(struct ov01a10 *ov01a10)
+{
+	struct v4l2_subdev_state *active_state =
+		v4l2_subdev_get_locked_active_state(&ov01a10->sd);
+
+	return v4l2_subdev_state_get_crop(active_state, 0);
+}
+
 static int ov01a10_update_digital_gain(struct ov01a10 *ov01a10, u32 d_gain)
 {
 	u32 real = d_gain << 6;
@@ -339,13 +322,16 @@ static int ov01a10_test_pattern(struct ov01a10 *ov01a10, u32 pattern)
 			 NULL);
 }
 
-/* for vflip and hflip, use 0x9 as window offset to keep the bayer */
 static int ov01a10_set_hflip(struct ov01a10 *ov01a10, u32 hflip)
 {
+	struct v4l2_rect *crop = ov01a10_get_active_crop(ov01a10);
 	u32 val, offset;
 	int ret = 0;
 
-	offset = hflip ? 0x8 : 0x9;
+	offset = crop->left;
+	if (!hflip)
+		offset++;
+
 	val = hflip ? 0 : FIELD_PREP(OV01A10_HFLIP_MASK, 0x1);
 
 	cci_write(ov01a10->regmap, OV01A10_REG_X_WIN, offset, &ret);
@@ -357,10 +343,14 @@ static int ov01a10_set_hflip(struct ov01a10 *ov01a10, u32 hflip)
 
 static int ov01a10_set_vflip(struct ov01a10 *ov01a10, u32 vflip)
 {
+	struct v4l2_rect *crop = ov01a10_get_active_crop(ov01a10);
 	u32 val, offset;
 	int ret = 0;
 
-	offset = vflip ? 0x9 : 0x8;
+	offset = crop->top;
+	if (vflip)
+		offset++;
+
 	val = vflip ? FIELD_PREP(OV01A10_VFLIP_MASK, 0x1) : 0;
 
 	cci_write(ov01a10->regmap, OV01A10_REG_Y_WIN, offset, &ret);
@@ -374,12 +364,13 @@ static int ov01a10_set_ctrl(struct v4l2_ctrl *ctrl)
 {
 	struct ov01a10 *ov01a10 = container_of(ctrl->handler,
 					       struct ov01a10, ctrl_handler);
+	struct v4l2_mbus_framefmt *fmt = ov01a10_get_active_format(ov01a10);
 	s64 exposure_max;
 	int ret = 0;
 
 	if (ctrl->id == V4L2_CID_VBLANK) {
-		exposure_max = ov01a10->cur_mode->height + ctrl->val -
-			OV01A10_EXPOSURE_MAX_MARGIN;
+		exposure_max = fmt->height + ctrl->val -
+			       OV01A10_EXPOSURE_MAX_MARGIN;
 		__v4l2_ctrl_modify_range(ov01a10->exposure,
 					 ov01a10->exposure->minimum,
 					 exposure_max, ov01a10->exposure->step,
@@ -406,7 +397,7 @@ static int ov01a10_set_ctrl(struct v4l2_ctrl *ctrl)
 
 	case V4L2_CID_VBLANK:
 		ret = cci_write(ov01a10->regmap, OV01A10_REG_VTS,
-				ov01a10->cur_mode->height + ctrl->val, NULL);
+				fmt->height + ctrl->val, NULL);
 		break;
 
 	case V4L2_CID_TEST_PATTERN:
@@ -440,7 +431,6 @@ static int ov01a10_init_controls(struct ov01a10 *ov01a10)
 	struct v4l2_fwnode_device_properties props;
 	u32 vblank_min, vblank_max, vblank_default;
 	struct v4l2_ctrl_handler *ctrl_hdlr;
-	const struct ov01a10_mode *cur_mode;
 	s64 exposure_max, h_blank;
 	int ret = 0;
 
@@ -453,8 +443,6 @@ static int ov01a10_init_controls(struct ov01a10 *ov01a10)
 	if (ret)
 		return ret;
 
-	cur_mode = ov01a10->cur_mode;
-
 	ov01a10->link_freq = v4l2_ctrl_new_int_menu(ctrl_hdlr,
 						    &ov01a10_ctrl_ops,
 						    V4L2_CID_LINK_FREQ,
@@ -465,14 +453,14 @@ static int ov01a10_init_controls(struct ov01a10 *ov01a10)
 						V4L2_CID_PIXEL_RATE, 0,
 						OV01A10_SCLK, 1, OV01A10_SCLK);
 
-	vblank_min = cur_mode->vts_min - cur_mode->height;
-	vblank_max = OV01A10_VTS_MAX - cur_mode->height;
-	vblank_default = cur_mode->vts_def - cur_mode->height;
+	vblank_min = OV01A10_VTS_MIN - OV01A10_DEFAULT_HEIGHT;
+	vblank_max = OV01A10_VTS_MAX - OV01A10_DEFAULT_HEIGHT;
+	vblank_default = OV01A10_VTS_DEF - OV01A10_DEFAULT_HEIGHT;
 	ov01a10->vblank = v4l2_ctrl_new_std(ctrl_hdlr, &ov01a10_ctrl_ops,
 					    V4L2_CID_VBLANK, vblank_min,
 					    vblank_max, 1, vblank_default);
 
-	h_blank = cur_mode->hts - cur_mode->width;
+	h_blank = OV01A10_HTS_DEF - OV01A10_DEFAULT_WIDTH;
 	ov01a10->hblank = v4l2_ctrl_new_std(ctrl_hdlr, &ov01a10_ctrl_ops,
 					    V4L2_CID_HBLANK, h_blank, h_blank,
 					    1, h_blank);
@@ -484,7 +472,7 @@ static int ov01a10_init_controls(struct ov01a10 *ov01a10)
 			  OV01A10_DGTL_GAIN_MIN, OV01A10_DGTL_GAIN_MAX,
 			  OV01A10_DGTL_GAIN_STEP, OV01A10_DGTL_GAIN_DEFAULT);
 
-	exposure_max = cur_mode->vts_def - OV01A10_EXPOSURE_MAX_MARGIN;
+	exposure_max = OV01A10_VTS_DEF - OV01A10_EXPOSURE_MAX_MARGIN;
 	ov01a10->exposure = v4l2_ctrl_new_std(ctrl_hdlr, &ov01a10_ctrl_ops,
 					      V4L2_CID_EXPOSURE,
 					      OV01A10_EXPOSURE_MIN,
@@ -524,24 +512,48 @@ static int ov01a10_init_controls(struct ov01a10 *ov01a10)
 	return ret;
 }
 
-static void ov01a10_update_pad_format(const struct ov01a10_mode *mode,
-				      struct v4l2_mbus_framefmt *fmt)
+static void ov01a10_fill_format(struct v4l2_mbus_framefmt *fmt,
+				unsigned int width, unsigned int height)
 {
-	fmt->width = mode->width;
-	fmt->height = mode->height;
+	memset(fmt, 0, sizeof(*fmt));
+	fmt->width = width;
+	fmt->height = height;
 	fmt->code = OV01A10_MEDIA_BUS_FMT;
 	fmt->field = V4L2_FIELD_NONE;
 	fmt->colorspace = V4L2_COLORSPACE_RAW;
 }
 
+static int ov01a10_set_mode(struct ov01a10 *ov01a10)
+{
+	struct v4l2_mbus_framefmt *fmt = ov01a10_get_active_format(ov01a10);
+	int ret = 0;
+
+	cci_write(ov01a10->regmap, OV01A10_REG_X_ADDR_START, 0, &ret);
+	cci_write(ov01a10->regmap, OV01A10_REG_Y_ADDR_START, 0, &ret);
+	cci_write(ov01a10->regmap, OV01A10_REG_X_ADDR_END,
+		  OV01A10_NATIVE_WIDTH - 1, &ret);
+	cci_write(ov01a10->regmap, OV01A10_REG_Y_ADDR_END,
+		  OV01A10_NATIVE_HEIGHT - 1, &ret);
+	cci_write(ov01a10->regmap, OV01A10_REG_X_OUTPUT_SIZE,
+		  fmt->width, &ret);
+	cci_write(ov01a10->regmap, OV01A10_REG_Y_OUTPUT_SIZE,
+		  fmt->height, &ret);
+	/* HTS register is in units of 2 pixels */
+	cci_write(ov01a10->regmap, OV01A10_REG_HTS,
+		  OV01A10_HTS_DEF / 2, &ret);
+	/* OV01A10_REG_VTS is set by vblank control */
+	/* OV01A10_REG_X_WIN is set by hlip control */
+	/* OV01A10_REG_Y_WIN is set by vflip control */
+
+	return ret;
+}
+
 static int ov01a10_start_streaming(struct ov01a10 *ov01a10)
 {
 	const struct ov01a10_reg_list *reg_list;
-	int link_freq_index;
-	int ret = 0;
+	int ret;
 
-	link_freq_index = ov01a10->cur_mode->link_freq_index;
-	reg_list = &link_freq_configs[link_freq_index].reg_list;
+	reg_list = &link_freq_configs[ov01a10->link_freq_index].reg_list;
 	ret = regmap_multi_reg_write(ov01a10->regmap, reg_list->regs,
 				     reg_list->num_of_regs);
 	if (ret) {
@@ -549,9 +561,14 @@ static int ov01a10_start_streaming(struct ov01a10 *ov01a10)
 		return ret;
 	}
 
-	reg_list = &ov01a10->cur_mode->reg_list;
-	ret = regmap_multi_reg_write(ov01a10->regmap, reg_list->regs,
-				     reg_list->num_of_regs);
+	ret = regmap_multi_reg_write(ov01a10->regmap, ov01a10_global_setting,
+				     ARRAY_SIZE(ov01a10_global_setting));
+	if (ret) {
+		dev_err(ov01a10->dev, "failed to initialize sensor\n");
+		return ret;
+	}
+
+	ret = ov01a10_set_mode(ov01a10);
 	if (ret) {
 		dev_err(ov01a10->dev, "failed to set mode\n");
 		return ret;
@@ -600,54 +617,64 @@ static int ov01a10_set_stream(struct v4l2_subdev *sd, int enable)
 	return ret;
 }
 
+static void ov01a10_update_blank_ctrls(struct ov01a10 *ov01a10,
+				       unsigned int width, unsigned int height)
+{
+	s32 hblank, vblank_def;
+
+	vblank_def = OV01A10_VTS_DEF - height;
+	__v4l2_ctrl_modify_range(ov01a10->vblank,
+				 OV01A10_VTS_MIN - height,
+				 OV01A10_VTS_MAX - height, 1,
+				 vblank_def);
+	__v4l2_ctrl_s_ctrl(ov01a10->vblank, vblank_def);
+
+	hblank = OV01A10_HTS_DEF - width;
+	__v4l2_ctrl_modify_range(ov01a10->hblank, hblank, hblank, 1, hblank);
+}
+
 static int ov01a10_set_format(struct v4l2_subdev *sd,
 			      struct v4l2_subdev_state *sd_state,
 			      struct v4l2_subdev_format *fmt)
 {
+	struct v4l2_rect *crop = v4l2_subdev_state_get_crop(sd_state, fmt->pad);
+	const int pattern_size = OV01A10_BAYER_PATTERN_SIZE;
+	const int border_size = OV01A10_BAYER_PATTERN_SIZE;
 	struct ov01a10 *ov01a10 = to_ov01a10(sd);
-	const struct ov01a10_mode *mode;
-	struct v4l2_mbus_framefmt *format;
-	s32 vblank_def, h_blank;
-
-	mode = v4l2_find_nearest_size(supported_modes,
-				      ARRAY_SIZE(supported_modes), width,
-				      height, fmt->format.width,
-				      fmt->format.height);
-
-	ov01a10_update_pad_format(mode, &fmt->format);
-
-	if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
-		ov01a10->cur_mode = mode;
-
-		vblank_def = mode->vts_def - mode->height;
-		__v4l2_ctrl_modify_range(ov01a10->vblank,
-					 mode->vts_min - mode->height,
-					 OV01A10_VTS_MAX - mode->height, 1,
-					 vblank_def);
-		__v4l2_ctrl_s_ctrl(ov01a10->vblank, vblank_def);
-		h_blank = mode->hts - mode->width;
-		__v4l2_ctrl_modify_range(ov01a10->hblank, h_blank, h_blank, 1,
-					 h_blank);
+	unsigned int width, height;
+
+	width = clamp_val(ALIGN(fmt->format.width, pattern_size),
+			  pattern_size,
+			  OV01A10_NATIVE_WIDTH - 2 * border_size);
+	height = clamp_val(ALIGN(fmt->format.height, pattern_size),
+			   pattern_size,
+			   OV01A10_NATIVE_HEIGHT - 2 * border_size);
+
+	/* Center image for userspace which does not set the crop first */
+	if (width != crop->width || height != crop->height) {
+		crop->left = ALIGN((OV01A10_NATIVE_WIDTH - width) / 2,
+				   pattern_size);
+		crop->top = ALIGN((OV01A10_NATIVE_HEIGHT - height) / 2,
+				  pattern_size);
+		crop->width = width;
+		crop->height = height;
 	}
 
-	format = v4l2_subdev_state_get_format(sd_state, fmt->pad);
-	*format = fmt->format;
+	ov01a10_fill_format(&fmt->format, width, height);
+	*v4l2_subdev_state_get_format(sd_state, fmt->pad) = fmt->format;
+
+	if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE)
+		ov01a10_update_blank_ctrls(ov01a10, width, height);
 
 	return 0;
 }
 
 static int ov01a10_init_state(struct v4l2_subdev *sd,
-			      struct v4l2_subdev_state *state)
+			      struct v4l2_subdev_state *sd_state)
 {
-	struct v4l2_subdev_format fmt = {
-		.which = V4L2_SUBDEV_FORMAT_TRY,
-		.format = {
-			.width = OV01A10_DEFAULT_WIDTH,
-			.height = OV01A10_DEFAULT_HEIGHT,
-		},
-	};
-
-	ov01a10_set_format(sd, state, &fmt);
+	*v4l2_subdev_state_get_crop(sd_state, 0) = ov01a10_default_crop;
+	ov01a10_fill_format(v4l2_subdev_state_get_format(sd_state, 0),
+			    OV01A10_DEFAULT_WIDTH, OV01A10_DEFAULT_HEIGHT);
 
 	return 0;
 }
@@ -668,14 +695,16 @@ static int ov01a10_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) ||
-	    fse->code != OV01A10_MEDIA_BUS_FMT)
+	const int pattern_size = OV01A10_BAYER_PATTERN_SIZE;
+	const int border_size = OV01A10_BAYER_PATTERN_SIZE;
+
+	if (fse->index)
 		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;
+	fse->min_width = pattern_size;
+	fse->max_width = OV01A10_NATIVE_WIDTH - 2 * border_size;
+	fse->min_height = pattern_size;
+	fse->max_height = OV01A10_NATIVE_HEIGHT - 2 * border_size;
 
 	return 0;
 }
@@ -684,31 +713,79 @@ static int ov01a10_get_selection(struct v4l2_subdev *sd,
 				 struct v4l2_subdev_state *state,
 				 struct v4l2_subdev_selection *sel)
 {
-	if (sel->which != V4L2_SUBDEV_FORMAT_ACTIVE)
-		return -EINVAL;
+	const int border_size = OV01A10_BAYER_PATTERN_SIZE;
 
 	switch (sel->target) {
-	case V4L2_SEL_TGT_NATIVE_SIZE:
+	case V4L2_SEL_TGT_CROP:
+		sel->r = *v4l2_subdev_state_get_crop(state, sel->pad);
+		return 0;
+	case V4L2_SEL_TGT_CROP_DEFAULT:
+		sel->r = ov01a10_default_crop;
+		return 0;
 	case V4L2_SEL_TGT_CROP_BOUNDS:
-		sel->r.top = 0;
+		/* Keep a border for hvflip shift to preserve bayer-pattern */
+		sel->r.left = border_size;
+		sel->r.top = border_size;
+		sel->r.width = OV01A10_NATIVE_WIDTH - 2 * border_size;
+		sel->r.height = OV01A10_NATIVE_HEIGHT - 2 * border_size;
+		return 0;
+	case V4L2_SEL_TGT_NATIVE_SIZE:
 		sel->r.left = 0;
+		sel->r.top = 0;
 		sel->r.width = OV01A10_NATIVE_WIDTH;
 		sel->r.height = OV01A10_NATIVE_HEIGHT;
 		return 0;
-	case V4L2_SEL_TGT_CROP:
-	case V4L2_SEL_TGT_CROP_DEFAULT:
-		sel->r.top = (OV01A10_NATIVE_HEIGHT -
-			      OV01A10_DEFAULT_HEIGHT) / 2;
-		sel->r.left = (OV01A10_NATIVE_WIDTH -
-			       OV01A10_DEFAULT_WIDTH) / 2;
-		sel->r.width = OV01A10_DEFAULT_WIDTH;
-		sel->r.height = OV01A10_DEFAULT_HEIGHT;
-		return 0;
 	}
 
 	return -EINVAL;
 }
 
+static int ov01a10_set_selection(struct v4l2_subdev *sd,
+				 struct v4l2_subdev_state *sd_state,
+				 struct v4l2_subdev_selection *sel)
+{
+	const int pattern_size = OV01A10_BAYER_PATTERN_SIZE;
+	const int border_size = OV01A10_BAYER_PATTERN_SIZE;
+	struct ov01a10 *ov01a10 = to_ov01a10(sd);
+	struct v4l2_mbus_framefmt *format;
+	struct v4l2_rect *crop;
+	struct v4l2_rect rect;
+
+	if (sel->target != V4L2_SEL_TGT_CROP)
+		return -EINVAL;
+
+	/*
+	 * Clamp the boundaries of the crop rectangle to the size of the sensor
+	 * pixel array. Align to pattern-size to ensure pattern isn't disrupted.
+	 */
+	rect.left = clamp_val(ALIGN(sel->r.left, pattern_size), border_size,
+			      OV01A10_NATIVE_WIDTH - 2 * border_size);
+	rect.top = clamp_val(ALIGN(sel->r.top, pattern_size), border_size,
+			     OV01A10_NATIVE_HEIGHT - 2 * border_size);
+	rect.width = clamp_val(ALIGN(sel->r.width, pattern_size), pattern_size,
+			       OV01A10_NATIVE_WIDTH - rect.left - border_size);
+	rect.height = clamp_val(ALIGN(sel->r.height, pattern_size), pattern_size,
+				OV01A10_NATIVE_HEIGHT - rect.top - border_size);
+
+	crop = v4l2_subdev_state_get_crop(sd_state, sel->pad);
+
+	/* Reset the output size if the crop rectangle size has changed */
+	if (rect.width != crop->width || rect.height != crop->height) {
+		format = v4l2_subdev_state_get_format(sd_state, sel->pad);
+		format->width = rect.width;
+		format->height = rect.height;
+
+		if (sel->which == V4L2_SUBDEV_FORMAT_ACTIVE)
+			ov01a10_update_blank_ctrls(ov01a10, rect.width,
+						   rect.height);
+	}
+
+	*crop = rect;
+	sel->r = rect;
+
+	return 0;
+}
+
 static const struct v4l2_subdev_core_ops ov01a10_core_ops = {
 	.log_status = v4l2_ctrl_subdev_log_status,
 };
@@ -721,6 +798,7 @@ static const struct v4l2_subdev_pad_ops ov01a10_pad_ops = {
 	.set_fmt = ov01a10_set_format,
 	.get_fmt = v4l2_subdev_get_fmt,
 	.get_selection = ov01a10_get_selection,
+	.set_selection = ov01a10_set_selection,
 	.enum_mbus_code = ov01a10_enum_mbus_code,
 	.enum_frame_size = ov01a10_enum_frame_size,
 };
@@ -940,8 +1018,6 @@ static int ov01a10_probe(struct i2c_client *client)
 	if (ret)
 		goto err_power_off;
 
-	ov01a10->cur_mode = &supported_modes[0];
-
 	ret = ov01a10_init_controls(ov01a10);
 	if (ret)
 		goto err_power_off;
-- 
2.47.3

[PATCH v2 17/23] media: i2c: ov01a10: Remove struct ov01a10_reg_list

[Sakari Ailus]

From: Hans de Goede <hansg@kernel.org>

After the conversion to the CCI register access helpers, struct
ov01a10_reg_list is only used inside struct ov01a10_link_freq_config.

Simplify things by embedding the ov01a10_reg_list members directly into
struct ov01a10_link_freq_config.

Signed-off-by: Hans de Goede <hansg@kernel.org>
Tested-by: Mehdi Djait <mehdi.djait@linux.intel.com> # Dell XPS 9315
Reviewed-by: Mehdi Djait <mehdi.djait@linux.intel.com>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/ov01a10.c | 22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/drivers/media/i2c/ov01a10.c b/drivers/media/i2c/ov01a10.c
index 349fd3d06df5..1af0af00df24 100644
--- a/drivers/media/i2c/ov01a10.c
+++ b/drivers/media/i2c/ov01a10.c
@@ -98,13 +98,9 @@
 #define OV01A10_MEDIA_BUS_FMT		MEDIA_BUS_FMT_SBGGR10_1X10
 #define OV01A10_BAYER_PATTERN_SIZE	2 /* 2x2 */
 
-struct ov01a10_reg_list {
-	u32 num_of_regs;
-	const struct reg_sequence *regs;
-};
-
 struct ov01a10_link_freq_config {
-	const struct ov01a10_reg_list reg_list;
+	const struct reg_sequence *regs;
+	int regs_len;
 };
 
 static const struct reg_sequence mipi_data_rate_720mbps[] = {
@@ -237,10 +233,8 @@ static const s64 link_freq_menu_items[] = {
 
 static const struct ov01a10_link_freq_config link_freq_configs[] = {
 	{
-		.reg_list = {
-			.num_of_regs = ARRAY_SIZE(mipi_data_rate_720mbps),
-			.regs = mipi_data_rate_720mbps,
-		}
+		.regs = mipi_data_rate_720mbps,
+		.regs_len = ARRAY_SIZE(mipi_data_rate_720mbps),
 	},
 };
 
@@ -550,12 +544,12 @@ static int ov01a10_set_mode(struct ov01a10 *ov01a10)
 
 static int ov01a10_start_streaming(struct ov01a10 *ov01a10)
 {
-	const struct ov01a10_reg_list *reg_list;
+	const struct ov01a10_link_freq_config *freq_cfg;
 	int ret;
 
-	reg_list = &link_freq_configs[ov01a10->link_freq_index].reg_list;
-	ret = regmap_multi_reg_write(ov01a10->regmap, reg_list->regs,
-				     reg_list->num_of_regs);
+	freq_cfg = &link_freq_configs[ov01a10->link_freq_index];
+	ret = regmap_multi_reg_write(ov01a10->regmap, freq_cfg->regs,
+				     freq_cfg->regs_len);
 	if (ret) {
 		dev_err(ov01a10->dev, "failed to set plls\n");
 		return ret;
-- 
2.47.3

[PATCH v2 18/23] media: i2c: ov01a10: Replace exposure->min/step with direct define use

[Sakari Ailus]

From: Hans de Goede <hansg@kernel.org>

The exposure minimum and step are constant use the defines for this
instead of retrieving these from the exposure-control.

Signed-off-by: Hans de Goede <hansg@kernel.org>
Tested-by: Mehdi Djait <mehdi.djait@linux.intel.com> # Dell XPS 9315
Reviewed-by: Mehdi Djait <mehdi.djait@linux.intel.com>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/ov01a10.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/media/i2c/ov01a10.c b/drivers/media/i2c/ov01a10.c
index 1af0af00df24..76c75da4d7c8 100644
--- a/drivers/media/i2c/ov01a10.c
+++ b/drivers/media/i2c/ov01a10.c
@@ -366,9 +366,8 @@ static int ov01a10_set_ctrl(struct v4l2_ctrl *ctrl)
 		exposure_max = fmt->height + ctrl->val -
 			       OV01A10_EXPOSURE_MAX_MARGIN;
 		__v4l2_ctrl_modify_range(ov01a10->exposure,
-					 ov01a10->exposure->minimum,
-					 exposure_max, ov01a10->exposure->step,
-					 exposure_max);
+					 OV01A10_EXPOSURE_MIN, exposure_max,
+					 OV01A10_EXPOSURE_STEP, exposure_max);
 	}
 
 	if (!pm_runtime_get_if_in_use(ov01a10->dev))
-- 
2.47.3

[PATCH v2 19/23] media: i2c: ov01a10: Only set register 0x0305 once

[Sakari Ailus]

From: Hans de Goede <hansg@kernel.org>

Stop setting register 0x0305 to one value from mipi_data_rate_720mbps
only to override it with a different value from sensor_1280x800_setting.

Instead directly set it to 0xf4.

Signed-off-by: Hans de Goede <hansg@kernel.org>
Tested-by: Mehdi Djait <mehdi.djait@linux.intel.com> # Dell XPS 9315
Reviewed-by: Mehdi Djait <mehdi.djait@linux.intel.com>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/ov01a10.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/media/i2c/ov01a10.c b/drivers/media/i2c/ov01a10.c
index 76c75da4d7c8..2480925f9957 100644
--- a/drivers/media/i2c/ov01a10.c
+++ b/drivers/media/i2c/ov01a10.c
@@ -108,7 +108,7 @@ static const struct reg_sequence mipi_data_rate_720mbps[] = {
 	{0x0302, 0x00},
 	{0x0303, 0x06},
 	{0x0304, 0x01},
-	{0x0305, 0xe0},
+	{0x0305, 0xf4},
 	{0x0306, 0x00},
 	{0x0308, 0x01},
 	{0x0309, 0x00},
@@ -216,7 +216,6 @@ static const struct reg_sequence ov01a10_global_setting[] = {
 	{0x5200, 0x18},
 	{0x5004, 0x00},
 	{0x5080, 0x40},
-	{0x0305, 0xf4},
 	{0x0325, 0xc2},
 };
 
-- 
2.47.3

[PATCH v2 20/23] media: i2c: ov01a10: Remove values set by controls from global_setting[]

[Sakari Ailus]

From: Hans de Goede <hansg@kernel.org>

Registers 0x3501 (exposure), 0x3508 (analogue-gain) and 0x4503 (test-
pattern) are already set through __v4l2_ctrl_handler_setup() drop them
from ov01a10_global_setting[].

Signed-off-by: Hans de Goede <hansg@kernel.org>
Tested-by: Mehdi Djait <mehdi.djait@linux.intel.com> # Dell XPS 9315
Reviewed-by: Mehdi Djait <mehdi.djait@linux.intel.com>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/ov01a10.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/media/i2c/ov01a10.c b/drivers/media/i2c/ov01a10.c
index 2480925f9957..1a02772bdf1c 100644
--- a/drivers/media/i2c/ov01a10.c
+++ b/drivers/media/i2c/ov01a10.c
@@ -123,11 +123,7 @@ static const struct reg_sequence ov01a10_global_setting[] = {
 	{0x3002, 0xa1},
 	{0x301e, 0xf0},
 	{0x3022, 0x01},
-	{0x3501, 0x03},
-	{0x3502, 0x78},
 	{0x3504, 0x0c},
-	{0x3508, 0x01},
-	{0x3509, 0x00},
 	{0x3601, 0xc0},
 	{0x3603, 0x71},
 	{0x3610, 0x68},
@@ -197,7 +193,6 @@ static const struct reg_sequence ov01a10_global_setting[] = {
 	{0x4300, 0xff},
 	{0x4301, 0x00},
 	{0x4302, 0x0f},
-	{0x4503, 0x00},
 	{0x4601, 0x50},
 	{0x4800, 0x64},
 	{0x481f, 0x34},
-- 
2.47.3

[PATCH v2 21/23] media: i2c: ov01a10: Add ov01a10_sensor_cfg struct

[Sakari Ailus]

From: Hans de Goede <hansg@kernel.org>

Add a struct with some sensor variant (ov01a10 / ov01a1b / ov01a1s)
specific settings.

This is a preparation patch for adding support for the ov01a1s sensor
which uses the same sensor with a different (RGBI) color-filter.

Signed-off-by: Hans de Goede <hansg@kernel.org>
Tested-by: Mehdi Djait <mehdi.djait@linux.intel.com> # Dell XPS 9315
Reviewed-by: Mehdi Djait <mehdi.djait@linux.intel.com>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/ov01a10.c | 87 +++++++++++++++++++++++++------------
 1 file changed, 60 insertions(+), 27 deletions(-)

diff --git a/drivers/media/i2c/ov01a10.c b/drivers/media/i2c/ov01a10.c
index 1a02772bdf1c..a909f1ff8a76 100644
--- a/drivers/media/i2c/ov01a10.c
+++ b/drivers/media/i2c/ov01a10.c
@@ -88,16 +88,6 @@
 #define OV01A10_REG_TEST_PATTERN	CCI_REG8(0x4503)
 #define OV01A10_TEST_PATTERN_ENABLE	BIT(7)
 
-/*
- * The native ov01a10 bayer-pattern is GBRG, but there was a driver bug enabling
- * hflip/mirroring by default resulting in BGGR. Because of this bug Intel's
- * proprietary IPU6 userspace stack expects BGGR. So we report BGGR to not break
- * userspace and fix things up by shifting the crop window-x coordinate by 1
- * when hflip is *disabled*.
- */
-#define OV01A10_MEDIA_BUS_FMT		MEDIA_BUS_FMT_SBGGR10_1X10
-#define OV01A10_BAYER_PATTERN_SIZE	2 /* 2x2 */
-
 struct ov01a10_link_freq_config {
 	const struct reg_sequence *regs;
 	int regs_len;
@@ -245,9 +235,19 @@ static const char * const ov01a10_supply_names[] = {
 	"dvdd",		/* Digital core power */
 };
 
+struct ov01a10_sensor_cfg {
+	const char *model;
+	u32 bus_fmt;
+	int pattern_size;
+	int border_size;
+	bool invert_hflip_shift;
+	bool invert_vflip_shift;
+};
+
 struct ov01a10 {
 	struct device *dev;
 	struct regmap *regmap;
+	const struct ov01a10_sensor_cfg *cfg;
 	struct v4l2_subdev sd;
 	struct media_pad pad;
 	struct v4l2_ctrl_handler ctrl_handler;
@@ -310,14 +310,15 @@ static int ov01a10_test_pattern(struct ov01a10 *ov01a10, u32 pattern)
 			 NULL);
 }
 
-static int ov01a10_set_hflip(struct ov01a10 *ov01a10, u32 hflip)
+static int ov01a10_set_hflip(struct ov01a10 *ov01a10, bool hflip)
 {
 	struct v4l2_rect *crop = ov01a10_get_active_crop(ov01a10);
+	const struct ov01a10_sensor_cfg *cfg = ov01a10->cfg;
 	u32 val, offset;
 	int ret = 0;
 
 	offset = crop->left;
-	if (!hflip)
+	if ((hflip ^ cfg->invert_hflip_shift) && cfg->border_size)
 		offset++;
 
 	val = hflip ? 0 : FIELD_PREP(OV01A10_HFLIP_MASK, 0x1);
@@ -329,14 +330,15 @@ static int ov01a10_set_hflip(struct ov01a10 *ov01a10, u32 hflip)
 	return ret;
 }
 
-static int ov01a10_set_vflip(struct ov01a10 *ov01a10, u32 vflip)
+static int ov01a10_set_vflip(struct ov01a10 *ov01a10, bool vflip)
 {
 	struct v4l2_rect *crop = ov01a10_get_active_crop(ov01a10);
+	const struct ov01a10_sensor_cfg *cfg = ov01a10->cfg;
 	u32 val, offset;
 	int ret = 0;
 
 	offset = crop->top;
-	if (vflip)
+	if ((vflip ^ cfg->invert_vflip_shift) && cfg->border_size)
 		offset++;
 
 	val = vflip ? FIELD_PREP(OV01A10_VFLIP_MASK, 0x1) : 0;
@@ -499,13 +501,14 @@ static int ov01a10_init_controls(struct ov01a10 *ov01a10)
 	return ret;
 }
 
-static void ov01a10_fill_format(struct v4l2_mbus_framefmt *fmt,
+static void ov01a10_fill_format(struct ov01a10 *ov01a10,
+				struct v4l2_mbus_framefmt *fmt,
 				unsigned int width, unsigned int height)
 {
 	memset(fmt, 0, sizeof(*fmt));
 	fmt->width = width;
 	fmt->height = height;
-	fmt->code = OV01A10_MEDIA_BUS_FMT;
+	fmt->code = ov01a10->cfg->bus_fmt;
 	fmt->field = V4L2_FIELD_NONE;
 	fmt->colorspace = V4L2_COLORSPACE_RAW;
 }
@@ -625,9 +628,9 @@ static int ov01a10_set_format(struct v4l2_subdev *sd,
 			      struct v4l2_subdev_format *fmt)
 {
 	struct v4l2_rect *crop = v4l2_subdev_state_get_crop(sd_state, fmt->pad);
-	const int pattern_size = OV01A10_BAYER_PATTERN_SIZE;
-	const int border_size = OV01A10_BAYER_PATTERN_SIZE;
 	struct ov01a10 *ov01a10 = to_ov01a10(sd);
+	const int pattern_size = ov01a10->cfg->pattern_size;
+	const int border_size = ov01a10->cfg->border_size;
 	unsigned int width, height;
 
 	width = clamp_val(ALIGN(fmt->format.width, pattern_size),
@@ -647,7 +650,7 @@ static int ov01a10_set_format(struct v4l2_subdev *sd,
 		crop->height = height;
 	}
 
-	ov01a10_fill_format(&fmt->format, width, height);
+	ov01a10_fill_format(ov01a10, &fmt->format, width, height);
 	*v4l2_subdev_state_get_format(sd_state, fmt->pad) = fmt->format;
 
 	if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE)
@@ -659,8 +662,10 @@ static int ov01a10_set_format(struct v4l2_subdev *sd,
 static int ov01a10_init_state(struct v4l2_subdev *sd,
 			      struct v4l2_subdev_state *sd_state)
 {
+	struct ov01a10 *ov01a10 = to_ov01a10(sd);
+
 	*v4l2_subdev_state_get_crop(sd_state, 0) = ov01a10_default_crop;
-	ov01a10_fill_format(v4l2_subdev_state_get_format(sd_state, 0),
+	ov01a10_fill_format(ov01a10, v4l2_subdev_state_get_format(sd_state, 0),
 			    OV01A10_DEFAULT_WIDTH, OV01A10_DEFAULT_HEIGHT);
 
 	return 0;
@@ -670,10 +675,12 @@ static int ov01a10_enum_mbus_code(struct v4l2_subdev *sd,
 				  struct v4l2_subdev_state *sd_state,
 				  struct v4l2_subdev_mbus_code_enum *code)
 {
+	struct ov01a10 *ov01a10 = to_ov01a10(sd);
+
 	if (code->index > 0)
 		return -EINVAL;
 
-	code->code = OV01A10_MEDIA_BUS_FMT;
+	code->code = ov01a10->cfg->bus_fmt;
 
 	return 0;
 }
@@ -682,8 +689,9 @@ static int ov01a10_enum_frame_size(struct v4l2_subdev *sd,
 				   struct v4l2_subdev_state *sd_state,
 				   struct v4l2_subdev_frame_size_enum *fse)
 {
-	const int pattern_size = OV01A10_BAYER_PATTERN_SIZE;
-	const int border_size = OV01A10_BAYER_PATTERN_SIZE;
+	struct ov01a10 *ov01a10 = to_ov01a10(sd);
+	const int pattern_size = ov01a10->cfg->pattern_size;
+	const int border_size = ov01a10->cfg->border_size;
 
 	if (fse->index)
 		return -EINVAL;
@@ -700,7 +708,8 @@ static int ov01a10_get_selection(struct v4l2_subdev *sd,
 				 struct v4l2_subdev_state *state,
 				 struct v4l2_subdev_selection *sel)
 {
-	const int border_size = OV01A10_BAYER_PATTERN_SIZE;
+	struct ov01a10 *ov01a10 = to_ov01a10(sd);
+	const int border_size = ov01a10->cfg->border_size;
 
 	switch (sel->target) {
 	case V4L2_SEL_TGT_CROP:
@@ -731,9 +740,9 @@ static int ov01a10_set_selection(struct v4l2_subdev *sd,
 				 struct v4l2_subdev_state *sd_state,
 				 struct v4l2_subdev_selection *sel)
 {
-	const int pattern_size = OV01A10_BAYER_PATTERN_SIZE;
-	const int border_size = OV01A10_BAYER_PATTERN_SIZE;
 	struct ov01a10 *ov01a10 = to_ov01a10(sd);
+	const int pattern_size = ov01a10->cfg->pattern_size;
+	const int border_size = ov01a10->cfg->border_size;
 	struct v4l2_mbus_framefmt *format;
 	struct v4l2_rect *crop;
 	struct v4l2_rect rect;
@@ -973,20 +982,28 @@ static void ov01a10_remove(struct i2c_client *client)
 
 static int ov01a10_probe(struct i2c_client *client)
 {
+	const struct ov01a10_sensor_cfg *cfg;
 	struct ov01a10 *ov01a10;
 	int ret;
 
+	cfg = device_get_match_data(&client->dev);
+	if (!cfg)
+		return -EINVAL;
+
 	ov01a10 = devm_kzalloc(&client->dev, sizeof(*ov01a10), GFP_KERNEL);
 	if (!ov01a10)
 		return -ENOMEM;
 
 	ov01a10->dev = &client->dev;
+	ov01a10->cfg = cfg;
 
 	ov01a10->regmap = devm_cci_regmap_init_i2c(client, 16);
 	if (IS_ERR(ov01a10->regmap))
 		return PTR_ERR(ov01a10->regmap);
 
 	v4l2_i2c_subdev_init(&ov01a10->sd, client, &ov01a10_subdev_ops);
+	/* Override driver->name with actual sensor model */
+	v4l2_i2c_subdev_set_name(&ov01a10->sd, client, cfg->model, NULL);
 	ov01a10->sd.internal_ops = &ov01a10_internal_ops;
 
 	ret = ov01a10_check_hwcfg(ov01a10);
@@ -1058,8 +1075,24 @@ static DEFINE_RUNTIME_DEV_PM_OPS(ov01a10_pm_ops, ov01a10_power_off,
 				 ov01a10_power_on, NULL);
 
 #ifdef CONFIG_ACPI
+/*
+ * The native ov01a10 bayer-pattern is GBRG, but there was a driver bug enabling
+ * hflip/mirroring by default resulting in BGGR. Because of this bug Intel's
+ * proprietary IPU6 userspace stack expects BGGR. So we report BGGR to not break
+ * userspace and fix things up by shifting the crop window-x coordinate by 1
+ * when hflip is *disabled*.
+ */
+static const struct ov01a10_sensor_cfg ov01a10_cfg = {
+	.model = "ov01a10",
+	.bus_fmt = MEDIA_BUS_FMT_SBGGR10_1X10,
+	.pattern_size = 2, /* 2x2 */
+	.border_size = 2,
+	.invert_hflip_shift = true,
+	.invert_vflip_shift = false,
+};
+
 static const struct acpi_device_id ov01a10_acpi_ids[] = {
-	{ "OVTI01A0" },
+	{ "OVTI01A0", (uintptr_t)&ov01a10_cfg },
 	{ }
 };
 
-- 
2.47.3

[PATCH v2 22/23] media: i2c: ov01a10: Optimize setting h/vflip values

[Sakari Ailus]

From: Hans de Goede <hansg@kernel.org>

Since ov01a10_global_setting[] sets the initial format1 register value,
there is no need to do a read-write-modify when setting the flip controls.

Only write format1 when setting the flip-controls and remove the now
unnecessary format1 register init from ov01a10_global_setting[].

Signed-off-by: Hans de Goede <hansg@kernel.org>
Tested-by: Mehdi Djait <mehdi.djait@linux.intel.com> # Dell XPS 9315
Reviewed-by: Mehdi Djait <mehdi.djait@linux.intel.com>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/ov01a10.c | 41 +++++++++++++++++++++++--------------
 1 file changed, 26 insertions(+), 15 deletions(-)

diff --git a/drivers/media/i2c/ov01a10.c b/drivers/media/i2c/ov01a10.c
index a909f1ff8a76..6d245b638b2d 100644
--- a/drivers/media/i2c/ov01a10.c
+++ b/drivers/media/i2c/ov01a10.c
@@ -161,7 +161,6 @@ static const struct reg_sequence ov01a10_global_setting[] = {
 	{0x3815, 0x01},
 	{0x3816, 0x01},
 	{0x3817, 0x01},
-	{0x3820, 0xa8},
 	{0x3822, 0x13},
 	{0x3832, 0x28},
 	{0x3833, 0x10},
@@ -240,6 +239,7 @@ struct ov01a10_sensor_cfg {
 	u32 bus_fmt;
 	int pattern_size;
 	int border_size;
+	u8 format1_base_val;
 	bool invert_hflip_shift;
 	bool invert_vflip_shift;
 };
@@ -258,6 +258,8 @@ struct ov01a10 {
 	struct v4l2_ctrl *vblank;
 	struct v4l2_ctrl *hblank;
 	struct v4l2_ctrl *exposure;
+	struct v4l2_ctrl *hflip;
+	struct v4l2_ctrl *vflip;
 
 	u32 link_freq_index;
 
@@ -310,22 +312,33 @@ static int ov01a10_test_pattern(struct ov01a10 *ov01a10, u32 pattern)
 			 NULL);
 }
 
+static void ov01a10_set_format1(struct ov01a10 *ov01a10, int *ret)
+{
+	u8 val = ov01a10->cfg->format1_base_val;
+
+	/* hflip register bit is inverted */
+	if (!ov01a10->hflip->val)
+		val |= FIELD_PREP(OV01A10_HFLIP_MASK, 0x1);
+
+	if (ov01a10->vflip->val)
+		val |= FIELD_PREP(OV01A10_VFLIP_MASK, 0x1);
+
+	cci_write(ov01a10->regmap, OV01A10_REG_FORMAT1, val, ret);
+}
+
 static int ov01a10_set_hflip(struct ov01a10 *ov01a10, bool hflip)
 {
 	struct v4l2_rect *crop = ov01a10_get_active_crop(ov01a10);
 	const struct ov01a10_sensor_cfg *cfg = ov01a10->cfg;
-	u32 val, offset;
+	u32 offset;
 	int ret = 0;
 
 	offset = crop->left;
 	if ((hflip ^ cfg->invert_hflip_shift) && cfg->border_size)
 		offset++;
 
-	val = hflip ? 0 : FIELD_PREP(OV01A10_HFLIP_MASK, 0x1);
-
 	cci_write(ov01a10->regmap, OV01A10_REG_X_WIN, offset, &ret);
-	cci_update_bits(ov01a10->regmap, OV01A10_REG_FORMAT1,
-			OV01A10_HFLIP_MASK, val, &ret);
+	ov01a10_set_format1(ov01a10, &ret);
 
 	return ret;
 }
@@ -334,18 +347,15 @@ static int ov01a10_set_vflip(struct ov01a10 *ov01a10, bool vflip)
 {
 	struct v4l2_rect *crop = ov01a10_get_active_crop(ov01a10);
 	const struct ov01a10_sensor_cfg *cfg = ov01a10->cfg;
-	u32 val, offset;
+	u32 offset;
 	int ret = 0;
 
 	offset = crop->top;
 	if ((vflip ^ cfg->invert_vflip_shift) && cfg->border_size)
 		offset++;
 
-	val = vflip ? FIELD_PREP(OV01A10_VFLIP_MASK, 0x1) : 0;
-
 	cci_write(ov01a10->regmap, OV01A10_REG_Y_WIN, offset, &ret);
-	cci_update_bits(ov01a10->regmap, OV01A10_REG_FORMAT1,
-			OV01A10_VFLIP_MASK, val, &ret);
+	ov01a10_set_format1(ov01a10, &ret);
 
 	return ret;
 }
@@ -474,10 +484,10 @@ static int ov01a10_init_controls(struct ov01a10 *ov01a10)
 				     ARRAY_SIZE(ov01a10_test_pattern_menu) - 1,
 				     0, 0, ov01a10_test_pattern_menu);
 
-	v4l2_ctrl_new_std(ctrl_hdlr, &ov01a10_ctrl_ops, V4L2_CID_HFLIP,
-			  0, 1, 1, 0);
-	v4l2_ctrl_new_std(ctrl_hdlr, &ov01a10_ctrl_ops, V4L2_CID_VFLIP,
-			  0, 1, 1, 0);
+	ov01a10->hflip = v4l2_ctrl_new_std(ctrl_hdlr, &ov01a10_ctrl_ops,
+					   V4L2_CID_HFLIP, 0, 1, 1, 0);
+	ov01a10->vflip = v4l2_ctrl_new_std(ctrl_hdlr, &ov01a10_ctrl_ops,
+					   V4L2_CID_VFLIP, 0, 1, 1, 0);
 
 	ret = v4l2_ctrl_new_fwnode_properties(ctrl_hdlr, &ov01a10_ctrl_ops,
 					      &props);
@@ -1087,6 +1097,7 @@ static const struct ov01a10_sensor_cfg ov01a10_cfg = {
 	.bus_fmt = MEDIA_BUS_FMT_SBGGR10_1X10,
 	.pattern_size = 2, /* 2x2 */
 	.border_size = 2,
+	.format1_base_val = 0xa0,
 	.invert_hflip_shift = true,
 	.invert_vflip_shift = false,
 };
-- 
2.47.3

[PATCH v2 23/23] media: i2c: ov01a10: Add ov01a1b support

[Sakari Ailus]

From: Hans de Goede <hansg@kernel.org>

Add support for the ov01a1b model which is the exact same sensor as
the ov01a10 without a color-filter.

Note since there is no color-filter there is also no need to shift
the crop-window when flipping, so the crop window set by userspace may
cover the full sensor (border_size=0).

Signed-off-by: Hans de Goede <hansg@kernel.org>
Reviewed-by: Mehdi Djait <mehdi.djait@linux.intel.com>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/ov01a10.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/media/i2c/ov01a10.c b/drivers/media/i2c/ov01a10.c
index 6d245b638b2d..10227ae6fe42 100644
--- a/drivers/media/i2c/ov01a10.c
+++ b/drivers/media/i2c/ov01a10.c
@@ -1102,8 +1102,17 @@ static const struct ov01a10_sensor_cfg ov01a10_cfg = {
 	.invert_vflip_shift = false,
 };
 
+static const struct ov01a10_sensor_cfg ov01a1b_cfg = {
+	.model = "ov01a1b",
+	.bus_fmt = MEDIA_BUS_FMT_Y10_1X10,
+	.pattern_size = 2, /* Keep coordinates aligned to a multiple of 2 */
+	.border_size = 0,
+	.format1_base_val = 0xa0,
+};
+
 static const struct acpi_device_id ov01a10_acpi_ids[] = {
 	{ "OVTI01A0", (uintptr_t)&ov01a10_cfg },
+	{ "OVTI01AB", (uintptr_t)&ov01a1b_cfg },
 	{ }
 };
 
-- 
2.47.3

Re: [PATCH v2 03/23] media: i2c: ov01a10: Fix analogue gain range

[Bingbu Cao]

Sakari and Hans,

Thanks for the fix.
Reviewed-by: Bingbu Cao <bingbu.cao@intel.com>


On 1/12/26 5:59 PM, Sakari Ailus wrote:
> From: Hans de Goede <hansg@kernel.org>
> 
> A analogue maximum gain of 0xffff / 65525 seems unlikely and testing
> indeed shows that the gain control wraps-around at 16383, so set the
> maximum gain to 0x3fff / 16383.
> 
> The minimum gain of 0x100 is correct. Setting bits 8-11 to 0x0 results
> in the same gain values as setting these bits to 0x1, with bits 0-7
> still increasing the gain when going from 0x000 - 0x0ff in the exact
> same range as when going from 0x100 - 0x1ff.
> 
> Fixes: 0827b58dabff ("media: i2c: add ov01a10 image sensor driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Hans de Goede <hansg@kernel.org>
> Tested-by: Mehdi Djait <mehdi.djait@linux.intel.com> # Dell XPS 9315
> Reviewed-by: Mehdi Djait <mehdi.djait@linux.intel.com>
> [Sakari Ailus: mention analogue gain and update the limit from 4096.]
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  drivers/media/i2c/ov01a10.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/i2c/ov01a10.c b/drivers/media/i2c/ov01a10.c
> index 0b1a1ecfffd0..834ca46acb75 100644
> --- a/drivers/media/i2c/ov01a10.c
> +++ b/drivers/media/i2c/ov01a10.c
> @@ -48,7 +48,7 @@
>  /* analog gain controls */
>  #define OV01A10_REG_ANALOG_GAIN		0x3508
>  #define OV01A10_ANAL_GAIN_MIN		0x100
> -#define OV01A10_ANAL_GAIN_MAX		0xffff
> +#define OV01A10_ANAL_GAIN_MAX		0x3fff
>  #define OV01A10_ANAL_GAIN_STEP		1
>  
>  /* digital gain controls */
> 

-- 
Best regards,
Bingbu Cao

Re: [PATCH v2 02/23] media: i2c: ov01a10: Fix reported pixel-rate value

[Bingbu Cao]

Reviewed-by: Bingbu Cao <bingbu.cao@intel.com>

On 1/12/26 5:59 PM, Sakari Ailus wrote:
> From: Hans de Goede <hansg@kernel.org>
> 
> CSI lanes are double-clocked so with a single lane at 400MHZ the resulting
> pixel-rate for 10-bits pixels is 400 MHz * 2 / 10 = 80 MHz, not 40 MHz.
> 
> This also matches with the observed frame-rate of 60 fps with the default
> vblank setting: 80000000 / (1488 * 896) = 60.
> 
> Fixes: 0827b58dabff ("media: i2c: add ov01a10 image sensor driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Hans de Goede <hansg@kernel.org>
> Tested-by: Mehdi Djait <mehdi.djait@linux.intel.com> # Dell XPS 9315
> Reviewed-by: Mehdi Djait <mehdi.djait@linux.intel.com>
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  drivers/media/i2c/ov01a10.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/i2c/ov01a10.c b/drivers/media/i2c/ov01a10.c
> index e5df01f97978..0b1a1ecfffd0 100644
> --- a/drivers/media/i2c/ov01a10.c
> +++ b/drivers/media/i2c/ov01a10.c
> @@ -16,7 +16,7 @@
>  #include <media/v4l2-fwnode.h>
>  
>  #define OV01A10_LINK_FREQ_400MHZ	400000000ULL
> -#define OV01A10_SCLK			40000000LL
> +#define OV01A10_SCLK			80000000LL
>  #define OV01A10_DATA_LANES		1
>  
>  #define OV01A10_REG_CHIP_ID		0x300a
> 

-- 
Best regards,
Bingbu Cao

Re: [PATCH v2 06/23] media: i2c: ov01a10: Fix test-pattern disabling

[Bingbu Cao]

Sakari and Hans,

On 1/12/26 5:59 PM, Sakari Ailus wrote:
> From: Hans de Goede <hansg@kernel.org>
> 
> When the test-pattern control gets set to 0 (Disabled) 0 should be written
> to the test-pattern register, rather then doing nothing.
> 
> Fixes: 0827b58dabff ("media: i2c: add ov01a10 image sensor driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Hans de Goede <hansg@kernel.org>
> Tested-by: Mehdi Djait <mehdi.djait@linux.intel.com> # Dell XPS 9315
> Reviewed-by: Mehdi Djait <mehdi.djait@linux.intel.com>
> Reviewed-by: Bingbu Cao <bingbu.cao@intel.com>
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  drivers/media/i2c/ov01a10.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/media/i2c/ov01a10.c b/drivers/media/i2c/ov01a10.c
> index dd2b6d381175..3ad516e4d369 100644
> --- a/drivers/media/i2c/ov01a10.c
> +++ b/drivers/media/i2c/ov01a10.c
> @@ -249,9 +249,8 @@ static const struct ov01a10_reg sensor_1280x800_setting[] = {
>  static const char * const ov01a10_test_pattern_menu[] = {
>  	"Disabled",
>  	"Color Bar",
> -	"Top-Bottom Darker Color Bar",
> -	"Right-Left Darker Color Bar",
> -	"Color Bar type 4",
> +	"Left-Right Darker Color Bar",
> +	"Bottom-Top Darker Color Bar",
>  };
>  
>  static const s64 link_freq_menu_items[] = {
> @@ -406,10 +405,8 @@ static int ov01a10_update_digital_gain(struct ov01a10 *ov01a10, u32 d_gain)
>  
>  static int ov01a10_test_pattern(struct ov01a10 *ov01a10, u32 pattern)
>  {
> -	if (!pattern)
> -		return 0;
> -
> -	pattern = (pattern - 1) | OV01A10_TEST_PATTERN_ENABLE;
> +	if (pattern)
> +		pattern |= OV01A10_TEST_PATTERN_ENABLE;
>

It should be 'pattern - 1', the pattern value for register start as 0.

>  	return ov01a10_write_reg(ov01a10, OV01A10_REG_TEST_PATTERN, 1, pattern);
>  }
> 

-- 
Best regards,
Bingbu Cao

Re: [PATCH v2 06/23] media: i2c: ov01a10: Fix test-pattern disabling

[Sakari Ailus]

Hi Bingbu,

Thank you for the review!

On Tue, Jan 13, 2026 at 10:59:51AM +0800, Bingbu Cao wrote:
> Sakari and Hans,
> 
> On 1/12/26 5:59 PM, Sakari Ailus wrote:
> > From: Hans de Goede <hansg@kernel.org>
> > 
> > When the test-pattern control gets set to 0 (Disabled) 0 should be written
> > to the test-pattern register, rather then doing nothing.
> > 
> > Fixes: 0827b58dabff ("media: i2c: add ov01a10 image sensor driver")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Hans de Goede <hansg@kernel.org>
> > Tested-by: Mehdi Djait <mehdi.djait@linux.intel.com> # Dell XPS 9315
> > Reviewed-by: Mehdi Djait <mehdi.djait@linux.intel.com>
> > Reviewed-by: Bingbu Cao <bingbu.cao@intel.com>
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > ---
> >  drivers/media/i2c/ov01a10.c | 11 ++++-------
> >  1 file changed, 4 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/media/i2c/ov01a10.c b/drivers/media/i2c/ov01a10.c
> > index dd2b6d381175..3ad516e4d369 100644
> > --- a/drivers/media/i2c/ov01a10.c
> > +++ b/drivers/media/i2c/ov01a10.c
> > @@ -249,9 +249,8 @@ static const struct ov01a10_reg sensor_1280x800_setting[] = {
> >  static const char * const ov01a10_test_pattern_menu[] = {
> >  	"Disabled",
> >  	"Color Bar",
> > -	"Top-Bottom Darker Color Bar",
> > -	"Right-Left Darker Color Bar",
> > -	"Color Bar type 4",
> > +	"Left-Right Darker Color Bar",
> > +	"Bottom-Top Darker Color Bar",
> >  };
> >  
> >  static const s64 link_freq_menu_items[] = {
> > @@ -406,10 +405,8 @@ static int ov01a10_update_digital_gain(struct ov01a10 *ov01a10, u32 d_gain)
> >  
> >  static int ov01a10_test_pattern(struct ov01a10 *ov01a10, u32 pattern)
> >  {
> > -	if (!pattern)
> > -		return 0;
> > -
> > -	pattern = (pattern - 1) | OV01A10_TEST_PATTERN_ENABLE;
> > +	if (pattern)
> > +		pattern |= OV01A10_TEST_PATTERN_ENABLE;
> >
> 
> It should be 'pattern - 1', the pattern value for register start as 0.

I'll use:

	if (pattern)
		pattern = (pattern - 1) | OV01A10_TEST_PATTERN_ENABLE;


I already sent a PR so I'll add a new patch for this.

> 
> >  	return ov01a10_write_reg(ov01a10, OV01A10_REG_TEST_PATTERN, 1, pattern);
> >  }
> > 
> 

-- 
Kind regards,

Sakari Ailus

Re: [PATCH v2 06/23] media: i2c: ov01a10: Fix test-pattern disabling

[Hans de Goede]

Hi Bingbu,

On 13-Jan-26 03:59, Bingbu Cao wrote:
> Sakari and Hans,
> 
> On 1/12/26 5:59 PM, Sakari Ailus wrote:
>> From: Hans de Goede <hansg@kernel.org>
>>
>> When the test-pattern control gets set to 0 (Disabled) 0 should be written
>> to the test-pattern register, rather then doing nothing.
>>
>> Fixes: 0827b58dabff ("media: i2c: add ov01a10 image sensor driver")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Hans de Goede <hansg@kernel.org>
>> Tested-by: Mehdi Djait <mehdi.djait@linux.intel.com> # Dell XPS 9315
>> Reviewed-by: Mehdi Djait <mehdi.djait@linux.intel.com>
>> Reviewed-by: Bingbu Cao <bingbu.cao@intel.com>
>> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
>> ---
>>  drivers/media/i2c/ov01a10.c | 11 ++++-------
>>  1 file changed, 4 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/media/i2c/ov01a10.c b/drivers/media/i2c/ov01a10.c
>> index dd2b6d381175..3ad516e4d369 100644
>> --- a/drivers/media/i2c/ov01a10.c
>> +++ b/drivers/media/i2c/ov01a10.c
>> @@ -249,9 +249,8 @@ static const struct ov01a10_reg sensor_1280x800_setting[] = {
>>  static const char * const ov01a10_test_pattern_menu[] = {
>>  	"Disabled",
>>  	"Color Bar",
>> -	"Top-Bottom Darker Color Bar",
>> -	"Right-Left Darker Color Bar",
>> -	"Color Bar type 4",
>> +	"Left-Right Darker Color Bar",
>> +	"Bottom-Top Darker Color Bar",
>>  };
>>  
>>  static const s64 link_freq_menu_items[] = {
>> @@ -406,10 +405,8 @@ static int ov01a10_update_digital_gain(struct ov01a10 *ov01a10, u32 d_gain)
>>  
>>  static int ov01a10_test_pattern(struct ov01a10 *ov01a10, u32 pattern)
>>  {
>> -	if (!pattern)
>> -		return 0;
>> -
>> -	pattern = (pattern - 1) | OV01A10_TEST_PATTERN_ENABLE;
>> +	if (pattern)
>> +		pattern |= OV01A10_TEST_PATTERN_ENABLE;
>>
> 
> It should be 'pattern - 1', the pattern value for register start as 0.

This was "pattern - 1" in the original v1 patch-set, the - 1 is dropped
deliberately c2 because testing has shown that there is no difference
between writing "0 | OV01A10_TEST_PATTERN_ENABLE" vs
"1 | OV01A10_TEST_PATTERN_ENABLE" to the register.

See the discussion here:

https://lore.kernel.org/linux-media/jgzovuqvd5csxwzmzf5asri7xvftoyb4lqyywtfdsrsgdvwz7i@neqszepmzw3m/

Regards,

Hans

Re: [PATCH v2 06/23] media: i2c: ov01a10: Fix test-pattern disabling

[Hans de Goede]

Hi,

On 13-Jan-26 09:14, Sakari Ailus wrote:
> Hi Bingbu,
> 
> Thank you for the review!
> 
> On Tue, Jan 13, 2026 at 10:59:51AM +0800, Bingbu Cao wrote:
>> Sakari and Hans,
>>
>> On 1/12/26 5:59 PM, Sakari Ailus wrote:
>>> From: Hans de Goede <hansg@kernel.org>
>>>
>>> When the test-pattern control gets set to 0 (Disabled) 0 should be written
>>> to the test-pattern register, rather then doing nothing.
>>>
>>> Fixes: 0827b58dabff ("media: i2c: add ov01a10 image sensor driver")
>>> Cc: stable@vger.kernel.org
>>> Signed-off-by: Hans de Goede <hansg@kernel.org>
>>> Tested-by: Mehdi Djait <mehdi.djait@linux.intel.com> # Dell XPS 9315
>>> Reviewed-by: Mehdi Djait <mehdi.djait@linux.intel.com>
>>> Reviewed-by: Bingbu Cao <bingbu.cao@intel.com>
>>> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
>>> ---
>>>  drivers/media/i2c/ov01a10.c | 11 ++++-------
>>>  1 file changed, 4 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/media/i2c/ov01a10.c b/drivers/media/i2c/ov01a10.c
>>> index dd2b6d381175..3ad516e4d369 100644
>>> --- a/drivers/media/i2c/ov01a10.c
>>> +++ b/drivers/media/i2c/ov01a10.c
>>> @@ -249,9 +249,8 @@ static const struct ov01a10_reg sensor_1280x800_setting[] = {
>>>  static const char * const ov01a10_test_pattern_menu[] = {
>>>  	"Disabled",
>>>  	"Color Bar",
>>> -	"Top-Bottom Darker Color Bar",
>>> -	"Right-Left Darker Color Bar",
>>> -	"Color Bar type 4",
>>> +	"Left-Right Darker Color Bar",
>>> +	"Bottom-Top Darker Color Bar",
>>>  };
>>>  
>>>  static const s64 link_freq_menu_items[] = {
>>> @@ -406,10 +405,8 @@ static int ov01a10_update_digital_gain(struct ov01a10 *ov01a10, u32 d_gain)
>>>  
>>>  static int ov01a10_test_pattern(struct ov01a10 *ov01a10, u32 pattern)
>>>  {
>>> -	if (!pattern)
>>> -		return 0;
>>> -
>>> -	pattern = (pattern - 1) | OV01A10_TEST_PATTERN_ENABLE;
>>> +	if (pattern)
>>> +		pattern |= OV01A10_TEST_PATTERN_ENABLE;
>>>
>>
>> It should be 'pattern - 1', the pattern value for register start as 0.
> 
> I'll use:
> 
> 	if (pattern)
> 		pattern = (pattern - 1) | OV01A10_TEST_PATTERN_ENABLE;
> 
> 
> I already sent a PR so I'll add a new patch for this.

There is no need for this the v2 patch is correct,
copy paste of me reply to Bingbu's original review explaining things:

This was "pattern - 1" in the original v1 patch-set, the - 1 is dropped
deliberately in v2 because testing has shown that there is no difference
between writing "0 | OV01A10_TEST_PATTERN_ENABLE" vs
"1 | OV01A10_TEST_PATTERN_ENABLE" to the register.

See the discussion here:

https://lore.kernel.org/linux-media/jgzovuqvd5csxwzmzf5asri7xvftoyb4lqyywtfdsrsgdvwz7i@neqszepmzw3m/

Regards,

Hans