Linux-Media Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 0/3] media: i2c: OV8865 image sensor support
@ 2020-10-23 17:54 Paul Kocialkowski
  2020-10-23 17:54 ` [PATCH 1/3] dt-bindings: media: i2c: Add OV8865 bindings documentation Paul Kocialkowski
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Paul Kocialkowski @ 2020-10-23 17:54 UTC (permalink / raw)
  To: linux-media, devicetree, linux-kernel
  Cc: Mauro Carvalho Chehab, Rob Herring, Liam Girdwood, Mark Brown,
	Paul Kocialkowski, Thomas Petazzoni, Hans Verkuil, Sakari Ailus,
	Maxime Ripard, kevin.lhopital

This series adds support for the OV8865 image sensor, as a V4L2 subdev
driver. Although an initial series was submitted by Kévin L'hôpital some
weeks ago, this version is significantly new and should be considered a
new series.

The final patch (not for merge) shows how to enable the OV8865 on the
Banana Pi Camera Board v2 with the Banana Pi M3.

Cheers,

Paul

Kévin L'hôpital (1):
  ARM: dts: sun8i: a83t: bananapi-m3: Enable MIPI CSI-2 with OV8865

Paul Kocialkowski (2):
  dt-bindings: media: i2c: Add OV8865 bindings documentation
  media: i2c: Add support for the OV8865 image sensor

 .../bindings/media/i2c/ovti,ov8865.yaml       |  124 +
 arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts  |   98 +
 drivers/media/i2c/Kconfig                     |   13 +
 drivers/media/i2c/Makefile                    |    1 +
 drivers/media/i2c/ov8865.c                    | 3031 +++++++++++++++++
 5 files changed, 3267 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml
 create mode 100644 drivers/media/i2c/ov8865.c

-- 
2.28.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/3] dt-bindings: media: i2c: Add OV8865 bindings documentation
  2020-10-23 17:54 [PATCH 0/3] media: i2c: OV8865 image sensor support Paul Kocialkowski
@ 2020-10-23 17:54 ` Paul Kocialkowski
  2020-10-30 16:39   ` Rob Herring
  2020-11-02 23:24   ` Sakari Ailus
  2020-10-23 17:54 ` [PATCH 2/3] media: i2c: Add support for the OV8865 image sensor Paul Kocialkowski
  2020-10-23 17:54 ` [PATCH NOT FOR MERGE 3/3] ARM: dts: sun8i: a83t: bananapi-m3: Enable MIPI CSI-2 with OV8865 Paul Kocialkowski
  2 siblings, 2 replies; 12+ messages in thread
From: Paul Kocialkowski @ 2020-10-23 17:54 UTC (permalink / raw)
  To: linux-media, devicetree, linux-kernel
  Cc: Mauro Carvalho Chehab, Rob Herring, Liam Girdwood, Mark Brown,
	Paul Kocialkowski, Thomas Petazzoni, Hans Verkuil, Sakari Ailus,
	Maxime Ripard, kevin.lhopital, Kévin L'hôpital

This introduces YAML bindings documentation for the OV8865
image sensor.

Co-developed-by: Kévin L'hôpital <kevin.lhopital@bootlin.com>
Signed-off-by: Kévin L'hôpital <kevin.lhopital@bootlin.com>
Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 .../bindings/media/i2c/ovti,ov8865.yaml       | 124 ++++++++++++++++++
 1 file changed, 124 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml

diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml
new file mode 100644
index 000000000000..807f1a94afae
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml
@@ -0,0 +1,124 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/i2c/ovti,ov8865.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: OmniVision OV8865 Image Sensor Device Tree Bindings
+
+maintainers:
+  - Paul Kocialkowski <paul.kocialkowski@bootlin.com>
+
+properties:
+  compatible:
+    const: ovti,ov8865
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: EXTCLK Clock
+
+  clock-names:
+    items:
+      - const: extclk
+
+  dvdd-supply:
+    description: Digital Domain Power Supply
+
+  avdd-supply:
+    description: Analog Domain Power Supply (internal AVDD is used if missing)
+
+  dovdd-supply:
+    description: I/O Domain Power Supply
+
+  powerdown-gpios:
+    maxItems: 1
+    description: Power Down Pin GPIO Control (active low)
+
+  reset-gpios:
+    maxItems: 1
+    description: Reset Pin GPIO Control (active low)
+
+  port:
+    type: object
+    description: Input port, connect to a MIPI CSI-2 receiver
+
+    properties:
+      endpoint:
+        type: object
+
+        properties:
+          remote-endpoint: true
+
+          bus-type:
+            const: 4
+
+          clock-lanes:
+            maxItems: 1
+
+          data-lanes:
+            minItems: 1
+            maxItems: 4
+
+        required:
+          - bus-type
+          - data-lanes
+          - remote-endpoint
+
+        additionalProperties: false
+
+    required:
+      - endpoint
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - dvdd-supply
+  - dovdd-supply
+  - port
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/sun8i-a83t-ccu.h>
+    #include <dt-bindings/gpio/gpio.h>
+
+    i2c2 {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        ov8865: camera@36 {
+            compatible = "ovti,ov8865";
+            reg = <0x36>;
+
+            pinctrl-names = "default";
+            pinctrl-0 = <&csi_mclk_pin>;
+
+            clocks = <&ccu CLK_CSI_MCLK>;
+            clock-names = "extclk";
+
+            avdd-supply = <&reg_ov8865_avdd>;
+            dovdd-supply = <&reg_ov8865_dovdd>;
+            dvdd-supply = <&reg_ov8865_dvdd>;
+
+            powerdown-gpios = <&pio 4 17 GPIO_ACTIVE_LOW>; /* PE17 */
+            reset-gpios = <&pio 4 16 GPIO_ACTIVE_LOW>; /* PE16 */
+
+            port {
+                ov8865_out_mipi_csi2: endpoint {
+                    bus-type = <4>; /* MIPI CSI-2 D-PHY */
+                    clock-lanes = <0>;
+                    data-lanes = <1 2 3 4>;
+
+                    remote-endpoint = <&mipi_csi2_in_ov8865>;
+                };
+            };
+        };
+    };
+
+...
-- 
2.28.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 2/3] media: i2c: Add support for the OV8865 image sensor
  2020-10-23 17:54 [PATCH 0/3] media: i2c: OV8865 image sensor support Paul Kocialkowski
  2020-10-23 17:54 ` [PATCH 1/3] dt-bindings: media: i2c: Add OV8865 bindings documentation Paul Kocialkowski
@ 2020-10-23 17:54 ` Paul Kocialkowski
  2020-10-23 17:54 ` [PATCH NOT FOR MERGE 3/3] ARM: dts: sun8i: a83t: bananapi-m3: Enable MIPI CSI-2 with OV8865 Paul Kocialkowski
  2 siblings, 0 replies; 12+ messages in thread
From: Paul Kocialkowski @ 2020-10-23 17:54 UTC (permalink / raw)
  To: linux-media, devicetree, linux-kernel
  Cc: Mauro Carvalho Chehab, Rob Herring, Liam Girdwood, Mark Brown,
	Paul Kocialkowski, Thomas Petazzoni, Hans Verkuil, Sakari Ailus,
	Maxime Ripard, kevin.lhopital, Kévin L'hôpital

The OV8865 is a 8 Mpx CMOS image sensor producing 3264x2448 at 30 fps.
Other modes (including some with sub-sampling) are available too.
It outputs 10-bit bayer CFA data through a MIPI CSI-2 interface with
up to 4 lanes supported.

Some register initialisation sequences are still needed for this driver,
as they cover registers for which no documentation is available.

This work is based on the first version of the driver submitted by
Kévin L'hôpital, which was adapted to mainline from the Allwinner BSP.
This version is a rewrite of the first version that matches the structure
of the OV5648 driver, with explicit PLL configuration, all the necessary
mode-specific fields, associatied registers and reduced static sequences.

It was tested with the Banana Pi Camera Board v3 and the Banana Pi M3.

Co-developed-by: Kévin L'hôpital <kevin.lhopital@bootlin.com>
Signed-off-by: Kévin L'hôpital <kevin.lhopital@bootlin.com>
Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 drivers/media/i2c/Kconfig  |   13 +
 drivers/media/i2c/Makefile |    1 +
 drivers/media/i2c/ov8865.c | 3031 ++++++++++++++++++++++++++++++++++++
 3 files changed, 3045 insertions(+)
 create mode 100644 drivers/media/i2c/ov8865.c

diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 49f4e261c48e..e53d54c143a6 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -1047,6 +1047,19 @@ config VIDEO_OV8856
 	  To compile this driver as a module, choose M here: the
 	  module will be called ov8856.
 
+config VIDEO_OV8865
+	tristate "OmniVision OV8865 sensor support"
+	depends on I2C && VIDEO_V4L2
+	select MEDIA_CONTROLLER
+	select VIDEO_V4L2_SUBDEV_API
+	select V4L2_FWNODE
+	help
+	  This is a Video4Linux2 sensor driver for OmniVision
+	  OV8865 camera sensor.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called ov8865.
+
 config VIDEO_OV9640
 	tristate "OmniVision OV9640 sensor support"
 	depends on I2C && VIDEO_V4L2
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index 15d4d6382582..505c2067e780 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -82,6 +82,7 @@ obj-$(CONFIG_VIDEO_OV7670) += ov7670.o
 obj-$(CONFIG_VIDEO_OV772X) += ov772x.o
 obj-$(CONFIG_VIDEO_OV7740) += ov7740.o
 obj-$(CONFIG_VIDEO_OV8856) += ov8856.o
+obj-$(CONFIG_VIDEO_OV8865) += ov8865.o
 obj-$(CONFIG_VIDEO_OV9640) += ov9640.o
 obj-$(CONFIG_VIDEO_OV9650) += ov9650.o
 obj-$(CONFIG_VIDEO_OV13858) += ov13858.o
diff --git a/drivers/media/i2c/ov8865.c b/drivers/media/i2c/ov8865.c
new file mode 100644
index 000000000000..fa557d157b4e
--- /dev/null
+++ b/drivers/media/i2c/ov8865.c
@@ -0,0 +1,3031 @@
+/*
+ * OmniVision OV8865 V4L2 driver
+ *
+ * Copyright 2020 Kévin L'hôpital <kevin.lhopital@bootlin.com>
+ * Copyright 2020 Bootlin
+ * Author: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ * This program is distributed .as is. WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/of_graph.h>
+#include <linux/regulator/consumer.h>
+#include <linux/videodev2.h>
+#include <media/v4l2-ctrls.h>
+#include <media/v4l2-device.h>
+#include <media/v4l2-fwnode.h>
+#include <media/v4l2-image-sizes.h>
+#include <media/v4l2-mediabus.h>
+
+/* Clock rate */
+
+#define OV8865_EXTCLK_RATE			24000000
+
+/* Register definitions */
+
+/* System */
+
+#define OV8865_SW_STANDBY_REG			0x100
+#define OV8865_SW_STANDBY_STREAM_ON		BIT(0)
+
+#define OV8865_SW_RESET_REG			0x103
+#define OV8865_SW_RESET_RESET			BIT(0)
+
+#define OV8865_PLL_CTRL0_REG			0x300
+#define OV8865_PLL_CTRL0_PRE_DIV(v)		((v) & GENMASK(2, 0))
+#define OV8865_PLL_CTRL1_REG			0x301
+#define OV8865_PLL_CTRL1_MUL_H(v)		(((v) & GENMASK(9, 8)) >> 8)
+#define OV8865_PLL_CTRL2_REG			0x302
+#define OV8865_PLL_CTRL2_MUL_L(v)		((v) & GENMASK(7, 0))
+#define OV8865_PLL_CTRL3_REG			0x303
+#define OV8865_PLL_CTRL3_M_DIV(v)		(((v) - 1) & GENMASK(3, 0))
+#define OV8865_PLL_CTRL4_REG			0x304
+#define OV8865_PLL_CTRL4_MIPI_DIV(v)		((v) & GENMASK(1, 0))
+#define OV8865_PLL_CTRL5_REG			0x305
+#define OV8865_PLL_CTRL5_SYS_PRE_DIV(v)		((v) & GENMASK(1, 0))
+#define OV8865_PLL_CTRL6_REG			0x306
+#define OV8865_PLL_CTRL6_SYS_DIV(v)		(((v) - 1) & BIT(0))
+
+#define OV8865_PLL_CTRL8_REG			0x308
+#define OV8865_PLL_CTRL9_REG			0x309
+#define OV8865_PLL_CTRLA_REG			0x30a
+#define OV8865_PLL_CTRLA_PRE_DIV_HALF(v)	(((v) - 1) & BIT(0))
+#define OV8865_PLL_CTRLB_REG			0x30b
+#define OV8865_PLL_CTRLB_PRE_DIV(v)		((v) & GENMASK(2, 0))
+#define OV8865_PLL_CTRLC_REG			0x30c
+#define OV8865_PLL_CTRLC_MUL_H(v)		(((v) & GENMASK(9, 8)) >> 8)
+#define OV8865_PLL_CTRLD_REG			0x30d
+#define OV8865_PLL_CTRLD_MUL_L(v)		((v) & GENMASK(7, 0))
+#define OV8865_PLL_CTRLE_REG			0x30e
+#define OV8865_PLL_CTRLE_SYS_DIV(v)		((v) & GENMASK(2, 0))
+#define OV8865_PLL_CTRLF_REG			0x30f
+#define OV8865_PLL_CTRLF_SYS_PRE_DIV(v)		(((v) - 1) & GENMASK(3, 0))
+#define OV8865_PLL_CTRL10_REG			0x310
+#define OV8865_PLL_CTRL11_REG			0x311
+#define OV8865_PLL_CTRL12_REG			0x312
+#define OV8865_PLL_CTRL12_PRE_DIV_HALF(v)	((((v) - 1) << 4) & BIT(4))
+#define OV8865_PLL_CTRL12_DAC_DIV(v)		(((v) - 1) & GENMASK(3, 0))
+
+#define OV8865_PLL_CTRL1B_REG			0x31b
+#define OV8865_PLL_CTRL1C_REG			0x31c
+
+#define OV8865_PLL_CTRL1E_REG			0x31e
+#define OV8865_PLL_CTRL1E_PLL1_NO_LAT		BIT(3)
+
+#define OV8865_PAD_OEN0_REG			0x3000
+
+#define OV8865_PAD_OEN2_REG			0x3002
+
+#define OV8865_CLK_RST5_REG			0x3005
+
+#define OV8865_CHIP_ID_HH_REG			0x300a
+#define OV8865_CHIP_ID_HH_VALUE			0x00
+#define OV8865_CHIP_ID_H_REG			0x300b
+#define OV8865_CHIP_ID_H_VALUE			0x88
+#define OV8865_CHIP_ID_L_REG			0x300c
+#define OV8865_CHIP_ID_L_VALUE			0x65
+#define OV8865_PAD_OUT2_REG			0x300d
+
+#define OV8865_PAD_SEL2_REG			0x3010
+#define OV8865_PAD_PK_REG			0x3011
+#define OV8865_PAD_PK_DRIVE_STRENGTH_1X		(0 << 5)
+#define OV8865_PAD_PK_DRIVE_STRENGTH_2X		(1 << 5)
+#define OV8865_PAD_PK_DRIVE_STRENGTH_3X		(2 << 5)
+#define OV8865_PAD_PK_DRIVE_STRENGTH_4X		(3 << 5)
+
+#define OV8865_PUMP_CLK_DIV_REG			0x3015
+#define OV8865_PUMP_CLK_DIV_PUMP_N(v)		(((v) << 4) & GENMASK(6, 4))
+#define OV8865_PUMP_CLK_DIV_PUMP_P(v)		((v) & GENMASK(2, 0))
+
+#define OV8865_MIPI_SC_CTRL0_REG		0x3018
+#define OV8865_MIPI_SC_CTRL0_LANES(v)		((((v) - 1) << 5) & \
+						 GENMASK(7, 5))
+#define OV8865_MIPI_SC_CTRL0_MIPI_EN		BIT(4)
+#define OV8865_MIPI_SC_CTRL0_UNKNOWN		BIT(1)
+#define OV8865_MIPI_SC_CTRL0_LANES_PD_MIPI	BIT(0)
+#define OV8865_MIPI_SC_CTRL1_REG		0x3019
+#define OV8865_CLK_RST0_REG			0x301a
+#define OV8865_CLK_RST1_REG			0x301b
+#define OV8865_CLK_RST2_REG			0x301c
+#define OV8865_CLK_RST3_REG			0x301d
+#define OV8865_CLK_RST4_REG			0x301e
+
+#define OV8865_PCLK_SEL_REG			0x3020
+#define OV8865_PCLK_SEL_PCLK_DIV_MASK		BIT(3)
+#define OV8865_PCLK_SEL_PCLK_DIV(v)		((((v) - 1) << 3) & BIT(3))
+
+#define OV8865_MISC_CTRL_REG			0x3021
+#define OV8865_MIPI_SC_CTRL2_REG		0x3022
+#define OV8865_MIPI_SC_CTRL2_CLK_LANES_PD_MIPI	BIT(1)
+#define OV8865_MIPI_SC_CTRL2_PD_MIPI_RST_SYNC	BIT(0)
+
+#define OV8865_MIPI_BIT_SEL_REG			0x3031
+#define OV8865_MIPI_BIT_SEL(v)			(((v) << 0) & GENMASK(4, 0))
+#define OV8865_CLK_SEL0_REG			0x3032
+#define OV8865_CLK_SEL0_PLL1_SYS_SEL(v)		(((v) << 7) & BIT(7))
+#define OV8865_CLK_SEL1_REG			0x3033
+#define OV8865_CLK_SEL1_MIPI_EOF		BIT(5)
+#define OV8865_CLK_SEL1_UNKNOWN			BIT(2)
+#define OV8865_CLK_SEL1_PLL_SCLK_SEL_MASK	BIT(1)
+#define OV8865_CLK_SEL1_PLL_SCLK_SEL(v)		(((v) << 1) & BIT(1))
+
+#define OV8865_SCLK_CTRL_REG			0x3106
+#define OV8865_SCLK_CTRL_SCLK_DIV(v)		(((v) << 4) & GENMASK(7, 4))
+#define OV8865_SCLK_CTRL_SCLK_PRE_DIV(v)	(((v) << 2) & GENMASK(3, 2))
+#define OV8865_SCLK_CTRL_UNKNOWN		BIT(0)
+
+/* Exposure/gain */
+
+#define OV8865_EXPOSURE_CTRL_HH_REG		0x3500
+#define OV8865_EXPOSURE_CTRL_HH(v)		(((v) & GENMASK(19, 16)) >> 16)
+#define OV8865_EXPOSURE_CTRL_H_REG		0x3501
+#define OV8865_EXPOSURE_CTRL_H(v)		(((v) & GENMASK(15, 8)) >> 8)
+#define OV8865_EXPOSURE_CTRL_L_REG		0x3502
+#define OV8865_EXPOSURE_CTRL_L(v)		((v) & GENMASK(7, 0))
+#define OV8865_EXPOSURE_GAIN_MANUAL_REG		0x3503
+
+#define OV8865_GAIN_CTRL_H_REG			0x3508
+#define OV8865_GAIN_CTRL_H(v)			(((v) & GENMASK(12, 8)) >> 8)
+#define OV8865_GAIN_CTRL_L_REG			0x3509
+#define OV8865_GAIN_CTRL_L(v)			((v) & GENMASK(7, 0))
+
+/* Timing */
+
+#define OV8865_CROP_START_X_H_REG		0x3800
+#define OV8865_CROP_START_X_H(v)		(((v) & GENMASK(11, 8)) >> 8)
+#define OV8865_CROP_START_X_L_REG		0x3801
+#define OV8865_CROP_START_X_L(v)		((v) & GENMASK(7, 0))
+#define OV8865_CROP_START_Y_H_REG		0x3802
+#define OV8865_CROP_START_Y_H(v)		(((v) & GENMASK(11, 8)) >> 8)
+#define OV8865_CROP_START_Y_L_REG		0x3803
+#define OV8865_CROP_START_Y_L(v)		((v) & GENMASK(7, 0))
+#define OV8865_CROP_END_X_H_REG			0x3804
+#define OV8865_CROP_END_X_H(v)			(((v) & GENMASK(11, 8)) >> 8)
+#define OV8865_CROP_END_X_L_REG			0x3805
+#define OV8865_CROP_END_X_L(v)			((v) & GENMASK(7, 0))
+#define OV8865_CROP_END_Y_H_REG			0x3806
+#define OV8865_CROP_END_Y_H(v)			(((v) & GENMASK(11, 8)) >> 8)
+#define OV8865_CROP_END_Y_L_REG			0x3807
+#define OV8865_CROP_END_Y_L(v)			((v) & GENMASK(7, 0))
+#define OV8865_OUTPUT_SIZE_X_H_REG		0x3808
+#define OV8865_OUTPUT_SIZE_X_H(v)		(((v) & GENMASK(11, 8)) >> 8)
+#define OV8865_OUTPUT_SIZE_X_L_REG		0x3809
+#define OV8865_OUTPUT_SIZE_X_L(v)		((v) & GENMASK(7, 0))
+#define OV8865_OUTPUT_SIZE_Y_H_REG		0x380a
+#define OV8865_OUTPUT_SIZE_Y_H(v)		(((v) & GENMASK(11, 8)) >> 8)
+#define OV8865_OUTPUT_SIZE_Y_L_REG		0x380b
+#define OV8865_OUTPUT_SIZE_Y_L(v)		((v) & GENMASK(7, 0))
+#define OV8865_HTS_H_REG			0x380c
+#define OV8865_HTS_H(v)				(((v) & GENMASK(11, 8)) >> 8)
+#define OV8865_HTS_L_REG			0x380d
+#define OV8865_HTS_L(v)				((v) & GENMASK(7, 0))
+#define OV8865_VTS_H_REG			0x380e
+#define OV8865_VTS_H(v)				(((v) & GENMASK(11, 8)) >> 8)
+#define OV8865_VTS_L_REG			0x380f
+#define OV8865_VTS_L(v)				((v) & GENMASK(7, 0))
+#define OV8865_OFFSET_X_H_REG			0x3810
+#define OV8865_OFFSET_X_H(v)			(((v) & GENMASK(15, 8)) >> 8)
+#define OV8865_OFFSET_X_L_REG			0x3811
+#define OV8865_OFFSET_X_L(v)			((v) & GENMASK(7, 0))
+#define OV8865_OFFSET_Y_H_REG			0x3812
+#define OV8865_OFFSET_Y_H(v)			(((v) & GENMASK(14, 8)) >> 8)
+#define OV8865_OFFSET_Y_L_REG			0x3813
+#define OV8865_OFFSET_Y_L(v)			((v) & GENMASK(7, 0))
+#define OV8865_INC_X_ODD_REG			0x3814
+#define OV8865_INC_X_ODD(v)			((v) & GENMASK(4, 0))
+#define OV8865_INC_X_EVEN_REG			0x3815
+#define OV8865_INC_X_EVEN(v)			((v) & GENMASK(4, 0))
+#define OV8865_VSYNC_START_H_REG		0x3816
+#define OV8865_VSYNC_START_H(v)			(((v) & GENMASK(15, 8)) >> 8)
+#define OV8865_VSYNC_START_L_REG		0x3817
+#define OV8865_VSYNC_START_L(v)			((v) & GENMASK(7, 0))
+#define OV8865_VSYNC_END_H_REG			0x3818
+#define OV8865_VSYNC_END_H(v)			(((v) & GENMASK(15, 8)) >> 8)
+#define OV8865_VSYNC_END_L_REG			0x3819
+#define OV8865_VSYNC_END_L(v)			((v) & GENMASK(7, 0))
+#define OV8865_HSYNC_FIRST_H_REG		0x381a
+#define OV8865_HSYNC_FIRST_H(v)			(((v) & GENMASK(15, 8)) >> 8)
+#define OV8865_HSYNC_FIRST_L_REG		0x381b
+#define OV8865_HSYNC_FIRST_L(v)			((v) & GENMASK(7, 0))
+
+#define OV8865_FORMAT1_REG			0x3820
+#define OV8865_FORMAT1_FLIP_VERT_ISP_EN		BIT(2)
+#define OV8865_FORMAT1_FLIP_VERT_SENSOR_EN	BIT(1)
+#define OV8865_FORMAT2_REG			0x3821
+#define OV8865_FORMAT2_HSYNC_EN			BIT(6)
+#define OV8865_FORMAT2_FST_VBIN_EN		BIT(5)
+#define OV8865_FORMAT2_FST_HBIN_EN		BIT(4)
+#define OV8865_FORMAT2_ISP_HORZ_VAR2_EN		BIT(3)
+#define OV8865_FORMAT2_FLIP_HORZ_ISP_EN		BIT(2)
+#define OV8865_FORMAT2_FLIP_HORZ_SENSOR_EN	BIT(1)
+#define OV8865_FORMAT2_SYNC_HBIN_EN		BIT(0)
+
+
+#define OV8865_INC_Y_ODD_REG			0x382a
+#define OV8865_INC_Y_ODD(v)			((v) & GENMASK(4, 0))
+#define OV8865_INC_Y_EVEN_REG			0x382b
+#define OV8865_INC_Y_EVEN(v)			((v) & GENMASK(4, 0))
+
+#define OV8865_ABLC_NUM_REG			0x3830
+#define OV8865_ABLC_NUM(v)			((v) & GENMASK(4, 0))
+
+#define OV8865_ZLINE_NUM_REG			0x3836
+#define OV8865_ZLINE_NUM(v)			((v) & GENMASK(4, 0))
+
+#define OV8865_AUTO_SIZE_CTRL_REG		0x3841
+#define OV8865_AUTO_SIZE_CTRL_OFFSET_Y_REG	BIT(5)
+#define OV8865_AUTO_SIZE_CTRL_OFFSET_X_REG	BIT(4)
+#define OV8865_AUTO_SIZE_CTRL_CROP_END_Y_REG	BIT(3)
+#define OV8865_AUTO_SIZE_CTRL_CROP_END_X_REG	BIT(2)
+#define OV8865_AUTO_SIZE_CTRL_CROP_START_Y_REG	BIT(1)
+#define OV8865_AUTO_SIZE_CTRL_CROP_START_X_REG	BIT(0)
+#define OV8865_AUTO_SIZE_X_OFFSET_H_REG		0x3842
+#define OV8865_AUTO_SIZE_X_OFFSET_L_REG		0x3843
+#define OV8865_AUTO_SIZE_Y_OFFSET_H_REG		0x3844
+#define OV8865_AUTO_SIZE_Y_OFFSET_L_REG		0x3845
+#define OV8865_AUTO_SIZE_BOUNDARIES_REG		0x3846
+#define OV8865_AUTO_SIZE_BOUNDARIES_Y(v)	(((v) << 4) & GENMASK(7, 4))
+#define OV8865_AUTO_SIZE_BOUNDARIES_X(v)	((v) & GENMASK(3, 0))
+
+/* PSRAM */
+
+#define OV8865_PSRAM_CTRL8_REG			0x3f08
+
+/* Black Level */
+
+#define OV8865_BLC_CTRL0_REG			0x4000
+#define OV8865_BLC_CTRL0_TRIG_RANGE_EN		BIT(7)
+#define OV8865_BLC_CTRL0_TRIG_FORMAT_EN		BIT(6)
+#define OV8865_BLC_CTRL0_TRIG_GAIN_EN		BIT(5)
+#define OV8865_BLC_CTRL0_TRIG_EXPOSURE_EN	BIT(4)
+#define OV8865_BLC_CTRL0_TRIG_MANUAL_EN		BIT(3)
+#define OV8865_BLC_CTRL0_FREEZE_EN		BIT(2)
+#define OV8865_BLC_CTRL0_ALWAYS_EN		BIT(1)
+#define OV8865_BLC_CTRL0_FILTER_EN		BIT(0)
+#define OV8865_BLC_CTRL1_REG			0x4001
+#define OV8865_BLC_CTRL1_DITHER_EN		BIT(7)
+#define OV8865_BLC_CTRL1_ZERO_LINE_DIFF_EN	BIT(6)
+#define OV8865_BLC_CTRL1_COL_SHIFT_256		(0 << 4)
+#define OV8865_BLC_CTRL1_COL_SHIFT_128		(1 << 4)
+#define OV8865_BLC_CTRL1_COL_SHIFT_64		(2 << 4)
+#define OV8865_BLC_CTRL1_COL_SHIFT_32		(3 << 4)
+#define OV8865_BLC_CTRL1_OFFSET_LIMIT_EN	BIT(2)
+#define OV8865_BLC_CTRL1_COLUMN_CANCEL_EN	BIT(1)
+#define OV8865_BLC_CTRL2_REG			0x4002
+#define OV8865_BLC_CTRL3_REG			0x4003
+#define OV8865_BLC_CTRL4_REG			0x4004
+#define OV8865_BLC_CTRL5_REG			0x4005
+#define OV8865_BLC_CTRL6_REG			0x4006
+#define OV8865_BLC_CTRL7_REG			0x4007
+#define OV8865_BLC_CTRL8_REG			0x4008
+#define OV8865_BLC_CTRL9_REG			0x4009
+#define OV8865_BLC_CTRLA_REG			0x400a
+#define OV8865_BLC_CTRLB_REG			0x400b
+#define OV8865_BLC_CTRLC_REG			0x400c
+#define OV8865_BLC_CTRLD_REG			0x400d
+#define OV8865_BLC_CTRLD_OFFSET_TRIGGER(v)	((v) & GENMASK(7, 0))
+
+#define OV8865_BLC_CTRL1F_REG			0x401f
+#define OV8865_BLC_CTRL1F_RB_REVERSE		BIT(3)
+#define OV8865_BLC_CTRL1F_INTERPOL_X_EN		BIT(2)
+#define OV8865_BLC_CTRL1F_INTERPOL_Y_EN		BIT(1)
+
+#define OV8865_BLC_ANCHOR_LEFT_START_H_REG	0x4020
+#define OV8865_BLC_ANCHOR_LEFT_START_H(v)	(((v) & GENMASK(11, 8)) >> 8)
+#define OV8865_BLC_ANCHOR_LEFT_START_L_REG	0x4021
+#define OV8865_BLC_ANCHOR_LEFT_START_L(v)	((v) & GENMASK(7, 0))
+#define OV8865_BLC_ANCHOR_LEFT_END_H_REG	0x4022
+#define OV8865_BLC_ANCHOR_LEFT_END_H(v)		(((v) & GENMASK(11, 8)) >> 8)
+#define OV8865_BLC_ANCHOR_LEFT_END_L_REG	0x4023
+#define OV8865_BLC_ANCHOR_LEFT_END_L(v)		((v) & GENMASK(7, 0))
+#define OV8865_BLC_ANCHOR_RIGHT_START_H_REG	0x4024
+#define OV8865_BLC_ANCHOR_RIGHT_START_H(v)	(((v) & GENMASK(11, 8)) >> 8)
+#define OV8865_BLC_ANCHOR_RIGHT_START_L_REG	0x4025
+#define OV8865_BLC_ANCHOR_RIGHT_START_L(v)	((v) & GENMASK(7, 0))
+#define OV8865_BLC_ANCHOR_RIGHT_END_H_REG	0x4026
+#define OV8865_BLC_ANCHOR_RIGHT_END_H(v)	(((v) & GENMASK(11, 8)) >> 8)
+#define OV8865_BLC_ANCHOR_RIGHT_END_L_REG	0x4027
+#define OV8865_BLC_ANCHOR_RIGHT_END_L(v)	((v) & GENMASK(7, 0))
+
+#define OV8865_BLC_TOP_ZLINE_START_REG		0x4028
+#define OV8865_BLC_TOP_ZLINE_START(v)		((v) & GENMASK(5, 0))
+#define OV8865_BLC_TOP_ZLINE_NUM_REG		0x4029
+#define OV8865_BLC_TOP_ZLINE_NUM(v)		((v) & GENMASK(4, 0))
+#define OV8865_BLC_TOP_BLKLINE_START_REG	0x402a
+#define OV8865_BLC_TOP_BLKLINE_START(v)		((v) & GENMASK(5, 0))
+#define OV8865_BLC_TOP_BLKLINE_NUM_REG		0x402b
+#define OV8865_BLC_TOP_BLKLINE_NUM(v)		((v) & GENMASK(4, 0))
+#define OV8865_BLC_BOT_ZLINE_START_REG		0x402c
+#define OV8865_BLC_BOT_ZLINE_START(v)		((v) & GENMASK(5, 0))
+#define OV8865_BLC_BOT_ZLINE_NUM_REG		0x402d
+#define OV8865_BLC_BOT_ZLINE_NUM(v)		((v) & GENMASK(4, 0))
+#define OV8865_BLC_BOT_BLKLINE_START_REG	0x402e
+#define OV8865_BLC_BOT_BLKLINE_START(v)		((v) & GENMASK(5, 0))
+#define OV8865_BLC_BOT_BLKLINE_NUM_REG		0x402f
+#define OV8865_BLC_BOT_BLKLINE_NUM(v)		((v) & GENMASK(4, 0))
+
+#define OV8865_BLC_OFFSET_LIMIT_REG		0x4034
+#define OV8865_BLC_OFFSET_LIMIT(v)		((v) & GENMASK(7, 0))
+
+/* VFIFO */
+
+#define OV8865_VFIFO_READ_START_H_REG		0x4600
+#define OV8865_VFIFO_READ_START_H(v)		(((v) & GENMASK(15, 8)) >> 8)
+#define OV8865_VFIFO_READ_START_L_REG		0x4601
+#define OV8865_VFIFO_READ_START_L(v)		((v) & GENMASK(7, 0))
+
+/* MIPI */
+
+#define OV8865_MIPI_CTRL0_REG			0x4800
+#define OV8865_MIPI_CTRL1_REG			0x4801
+#define OV8865_MIPI_CTRL2_REG			0x4802
+#define OV8865_MIPI_CTRL3_REG			0x4803
+#define OV8865_MIPI_CTRL4_REG			0x4804
+#define OV8865_MIPI_CTRL5_REG			0x4805
+#define OV8865_MIPI_CTRL6_REG			0x4806
+#define OV8865_MIPI_CTRL7_REG			0x4807
+#define OV8865_MIPI_CTRL8_REG			0x4808
+
+#define OV8865_MIPI_FCNT_MAX_H_REG		0x4810
+#define OV8865_MIPI_FCNT_MAX_L_REG		0x4811
+
+#define OV8865_MIPI_CTRL13_REG			0x4813
+#define OV8865_MIPI_CTRL14_REG			0x4814
+#define OV8865_MIPI_CTRL15_REG			0x4815
+#define OV8865_MIPI_EMBEDDED_DT_REG		0x4816
+
+#define OV8865_MIPI_HS_ZERO_MIN_H_REG		0x4818
+#define OV8865_MIPI_HS_ZERO_MIN_L_REG		0x4819
+#define OV8865_MIPI_HS_TRAIL_MIN_H_REG		0x481a
+#define OV8865_MIPI_HS_TRAIL_MIN_L_REG		0x481b
+#define OV8865_MIPI_CLK_ZERO_MIN_H_REG		0x481c
+#define OV8865_MIPI_CLK_ZERO_MIN_L_REG		0x481d
+#define OV8865_MIPI_CLK_PREPARE_MAX_REG		0x481e
+#define OV8865_MIPI_CLK_PREPARE_MIN_REG		0x481f
+#define OV8865_MIPI_CLK_POST_MIN_H_REG		0x4820
+#define OV8865_MIPI_CLK_POST_MIN_L_REG		0x4821
+#define OV8865_MIPI_CLK_TRAIL_MIN_H_REG		0x4822
+#define OV8865_MIPI_CLK_TRAIL_MIN_L_REG		0x4823
+#define OV8865_MIPI_LPX_P_MIN_H_REG		0x4824
+#define OV8865_MIPI_LPX_P_MIN_L_REG		0x4825
+#define OV8865_MIPI_HS_PREPARE_MIN_REG		0x4826
+#define OV8865_MIPI_HS_PREPARE_MAX_REG		0x4827
+#define OV8865_MIPI_HS_EXIT_MIN_H_REG		0x4828
+#define OV8865_MIPI_HS_EXIT_MIN_L_REG		0x4829
+#define OV8865_MIPI_UI_HS_ZERO_MIN_REG		0x482a
+#define OV8865_MIPI_UI_HS_TRAIL_MIN_REG		0x482b
+#define OV8865_MIPI_UI_CLK_ZERO_MIN_REG		0x482c
+#define OV8865_MIPI_UI_CLK_PREPARE_REG		0x482d
+#define OV8865_MIPI_UI_CLK_POST_MIN_REG		0x482e
+#define OV8865_MIPI_UI_CLK_TRAIL_MIN_REG	0x482f
+#define OV8865_MIPI_UI_LPX_P_MIN_REG		0x4830
+#define OV8865_MIPI_UI_HS_PREPARE_REG		0x4831
+#define OV8865_MIPI_UI_HS_EXIT_MIN_REG		0x4832
+#define OV8865_MIPI_PKT_START_SIZE_REG		0x4833
+
+#define OV8865_MIPI_PCLK_PERIOD_REG		0x4837
+#define OV8865_MIPI_LP_GPIO0_REG		0x4838
+#define OV8865_MIPI_LP_GPIO1_REG		0x4839
+
+#define OV8865_MIPI_CTRL3C_REG			0x483c
+#define OV8865_MIPI_LP_GPIO4_REG		0x483d
+
+#define OV8865_MIPI_CTRL4A_REG			0x484a
+#define OV8865_MIPI_CTRL4B_REG			0x484b
+#define OV8865_MIPI_CTRL4C_REG			0x484c
+#define OV8865_MIPI_LANE_TEST_PATTERN_REG	0x484d
+#define OV8865_MIPI_FRAME_END_DELAY_REG		0x484e
+#define OV8865_MIPI_CLOCK_TEST_PATTERN_REG	0x484f
+#define OV8865_MIPI_LANE_SEL01_REG		0x4850
+#define OV8865_MIPI_LANE_SEL01_LANE0(v)		(((v) << 0) & GENMASK(2, 0))
+#define OV8865_MIPI_LANE_SEL01_LANE1(v)		(((v) << 4) & GENMASK(6, 4))
+#define OV8865_MIPI_LANE_SEL23_REG		0x4851
+#define OV8865_MIPI_LANE_SEL23_LANE2(v)		(((v) << 0) & GENMASK(2, 0))
+#define OV8865_MIPI_LANE_SEL23_LANE3(v)		(((v) << 4) & GENMASK(6, 4))
+
+/* ISP */
+
+#define OV8865_ISP_CTRL0_REG			0x5000
+#define OV8865_ISP_CTRL0_LENC_EN		BIT(7)
+#define OV8865_ISP_CTRL0_WHITE_BALANCE_EN	BIT(4)
+#define OV8865_ISP_CTRL0_DPC_BLACK_EN		BIT(2)
+#define OV8865_ISP_CTRL0_DPC_WHITE_EN		BIT(1)
+#define OV8865_ISP_CTRL1_REG			0x5001
+#define OV8865_ISP_CTRL1_BLC_EN			BIT(0)
+#define OV8865_ISP_CTRL2_REG			0x5002
+#define OV8865_ISP_CTRL2_DEBUG			BIT(3)
+#define OV8865_ISP_CTRL2_VARIOPIXEL_EN		BIT(2)
+#define OV8865_ISP_CTRL2_VSYNC_LATCH_EN		BIT(0)
+#define OV8865_ISP_CTRL3_REG			0x5003
+
+#define OV8865_ISP_GAIN_RED_H_REG		0x5018
+#define OV8865_ISP_GAIN_RED_H(v)		(((v) & GENMASK(13, 6)) >> 6)
+#define OV8865_ISP_GAIN_RED_L_REG		0x5019
+#define OV8865_ISP_GAIN_RED_L(v)		((v) & GENMASK(5, 0))
+#define OV8865_ISP_GAIN_GREEN_H_REG		0x501a
+#define OV8865_ISP_GAIN_GREEN_H(v)		(((v) & GENMASK(13, 6)) >> 6)
+#define OV8865_ISP_GAIN_GREEN_L_REG		0x501b
+#define OV8865_ISP_GAIN_GREEN_L(v)		((v) & GENMASK(5, 0))
+#define OV8865_ISP_GAIN_BLUE_H_REG		0x501c
+#define OV8865_ISP_GAIN_BLUE_H(v)		(((v) & GENMASK(13, 6)) >> 6)
+#define OV8865_ISP_GAIN_BLUE_L_REG		0x501d
+#define OV8865_ISP_GAIN_BLUE_L(v)		((v) & GENMASK(5, 0))
+
+/* VarioPixel */
+
+#define OV8865_VAP_CTRL0_REG			0x5900
+#define OV8865_VAP_CTRL1_REG			0x5901
+#define OV8865_VAP_CTRL1_HSUB_COEF(v)		((((v) - 1) << 2) & \
+						 GENMASK(3, 2))
+#define OV8865_VAP_CTRL1_VSUB_COEF(v)		(((v) - 1) & GENMASK(1, 0))
+
+/* Pre-DSP */
+
+#define OV8865_PRE_CTRL0_REG			0x5e00
+#define OV8865_PRE_CTRL0_PATTERN_EN		BIT(7)
+#define OV8865_PRE_CTRL0_ROLLING_BAR_EN		BIT(6)
+#define OV8865_PRE_CTRL0_TRANSPARENT_MODE	BIT(5)
+#define OV8865_PRE_CTRL0_SQUARES_BW_MODE	BIT(4)
+#define OV8865_PRE_CTRL0_PATTERN_COLOR_BARS	0
+#define OV8865_PRE_CTRL0_PATTERN_RANDOM_DATA	1
+#define OV8865_PRE_CTRL0_PATTERN_COLOR_SQUARES	2
+#define OV8865_PRE_CTRL0_PATTERN_BLACK		3
+
+/* Macros */
+
+#define ov8865_subdev_sensor(subdev) \
+	container_of(subdev, struct ov8865_sensor, subdev)
+
+#define ov8865_ctrl_subdev(ctrl) \
+	(&container_of(ctrl->handler, struct ov8865_sensor, ctrls.handler)->subdev)
+
+/* Data structures */
+
+struct ov8865_register_value {
+	u16 address;
+	u8 value;
+	unsigned int delay_ms;
+};
+
+/*
+ * PLL1 Clock Tree:
+ *
+ * +-< EXTCLK
+ * |
+ * +-+ pll_pre_div_half (0x30a [0])
+ *   |
+ *   +-+ pll_pre_div (0x300 [2:0], special values:
+ *     |              0: 1, 1: 1.5, 3: 2.5, 4: 3, 5: 4, 7: 8)
+ *     +-+ pll_mul (0x301 [1:0], 0x302 [7:0])
+ *       |
+ *       +-+ m_div (0x303 [3:0])
+ *       | |
+ *       | +-> PHY_SCLK
+ *       | |
+ *       | +-+ mipi_div (0x304 [1:0], special values: 0: 4, 1: 5, 2: 6, 3: 8)
+ *       |   |
+ *       |   +-+ pclk_div (0x3020 [3])
+ *       |     |
+ *       |     +-> PCLK
+ *       |
+ *       +-+ sys_pre_div (0x305 [1:0], special values: 0: 3, 1: 4, 2: 5, 3: 6)
+ *         |
+ *         +-+ sys_div (0x306 [0])
+ *           |
+ *           +-+ sys_sel (0x3032 [7], 0: PLL1, 1: PLL2)
+ *             |
+ *             +-+ sclk_sel (0x3033 [1], 0: sys_sel, 1: PLL2 DAC_CLK)
+ *               |
+ *               +-+ sclk_pre_div (0x3106 [3:2], special values:
+ *                 |               0: 1, 1: 2, 2: 4, 3: 1)
+ *                 |
+ *                 +-+ sclk_div (0x3106 [7:4], special values: 0: 1)
+ *                   |
+ *                   +-> SCLK
+ */
+
+struct ov8865_pll1_config {
+	unsigned int pll_pre_div_half;
+	unsigned int pll_pre_div;
+	unsigned int pll_mul;
+	unsigned int m_div;
+	unsigned int mipi_div;
+	unsigned int pclk_div;
+	unsigned int sys_pre_div;
+	unsigned int sys_div;
+};
+
+/*
+ * PLL2 Clock Tree:
+ *
+ * +-< EXTCLK
+ * |
+ * +-+ pll_pre_div_half (0x312 [4])
+ *   |
+ *   +-+ pll_pre_div (0x30b [2:0], special values:
+ *     |              0: 1, 1: 1.5, 3: 2.5, 4: 3, 5: 4, 7: 8)
+ *     +-+ pll_mul (0x30c [1:0], 0x30d [7:0])
+ *       |
+ *       +-+ dac_div (0x312 [3:0])
+ *       | |
+ *       | +-> DAC_CLK
+ *       |
+ *       +-+ sys_pre_div (0x30f [3:0])
+ *         |
+ *         +-+ sys_div (0x30e [2:0], special values:
+ *           |          0: 1, 1: 1.5, 3: 2.5, 4: 3, 5: 3.5, 6: 4, 7:5)
+ *           |
+ *           +-+ sys_sel (0x3032 [7], 0: PLL1, 1: PLL2)
+ *             |
+ *             +-+ sclk_sel (0x3033 [1], 0: sys_sel, 1: PLL2 DAC_CLK)
+ *               |
+ *               +-+ sclk_pre_div (0x3106 [3:2], special values:
+ *                 |               0: 1, 1: 2, 2: 4, 3: 1)
+ *                 |
+ *                 +-+ sclk_div (0x3106 [7:4], special values: 0: 1)
+ *                   |
+ *                   +-> SCLK
+ */
+
+struct ov8865_pll2_config {
+	unsigned int pll_pre_div_half;
+	unsigned int pll_pre_div;
+	unsigned int pll_mul;
+	unsigned int dac_div;
+	unsigned int sys_pre_div;
+	unsigned int sys_div;
+};
+
+struct ov8865_sclk_config {
+	unsigned int sys_sel;
+	unsigned int sclk_sel;
+	unsigned int sclk_pre_div;
+	unsigned int sclk_div;
+};
+
+/*
+ * General formulas for (array-centered) mode calculation:
+ * - photo_array_width = 3296
+ * - crop_start_x = (photo_array_width - output_size_x) / 2
+ * - crop_end_x = crop_start_x + offset_x + output_size_x - 1
+ *
+ * - photo_array_height = 2480
+ * - crop_start_y = (photo_array_height - output_size_y) / 2
+ * - crop_end_y = crop_start_y + offset_y + output_size_y - 1
+ */
+
+struct ov8865_mode {
+	unsigned int crop_start_x;
+	unsigned int offset_x;
+	unsigned int output_size_x;
+	unsigned int crop_end_x;
+	unsigned int hts;
+
+	unsigned int crop_start_y;
+	unsigned int offset_y;
+	unsigned int output_size_y;
+	unsigned int crop_end_y;
+	unsigned int vts;
+
+	/* With auto size, only output and total sizes need to be set. */
+	bool size_auto;
+	unsigned int size_auto_boundary_x;
+	unsigned int size_auto_boundary_y;
+
+	bool binning_x;
+	bool binning_y;
+	bool variopixel;
+	unsigned int variopixel_hsub_coef;
+	unsigned int variopixel_vsub_coef;
+
+	/* Bits for the format register, used for binning. */
+	bool sync_hbin;
+	bool horz_var2;
+
+	unsigned int inc_x_odd;
+	unsigned int inc_x_even;
+	unsigned int inc_y_odd;
+	unsigned int inc_y_even;
+
+	unsigned int vfifo_read_start;
+
+	unsigned int ablc_num;
+	unsigned int zline_num;
+
+	unsigned int blc_top_zero_line_start;
+	unsigned int blc_top_zero_line_num;
+	unsigned int blc_top_black_line_start;
+	unsigned int blc_top_black_line_num;
+
+	unsigned int blc_bottom_zero_line_start;
+	unsigned int blc_bottom_zero_line_num;
+	unsigned int blc_bottom_black_line_start;
+	unsigned int blc_bottom_black_line_num;
+
+	u8 blc_col_shift_mask;
+
+	unsigned int blc_anchor_left_start;
+	unsigned int blc_anchor_left_end;
+	unsigned int blc_anchor_right_start;
+	unsigned int blc_anchor_right_end;
+
+	struct v4l2_fract frame_interval;
+
+	const struct ov8865_pll1_config *pll1_config;
+	const struct ov8865_pll2_config *pll2_config;
+	const struct ov8865_sclk_config *sclk_config;
+
+	const struct ov8865_register_value *register_values;
+	unsigned int register_values_count;
+};
+
+struct ov8865_state {
+	const struct ov8865_mode *mode;
+	u32 mbus_code;
+
+	int power_count;
+	bool streaming;
+};
+
+struct ov8865_ctrls {
+	struct v4l2_ctrl *exposure;
+	struct v4l2_ctrl *gain;
+
+	struct v4l2_ctrl *red_balance;
+	struct v4l2_ctrl *blue_balance;
+
+	struct v4l2_ctrl *flip_horz;
+	struct v4l2_ctrl *flip_vert;
+
+	struct v4l2_ctrl *test_pattern;
+
+	struct v4l2_ctrl *link_freq;
+	struct v4l2_ctrl *pixel_rate;
+
+	struct v4l2_ctrl_handler handler;
+} __packed;
+
+struct ov8865_sensor {
+	struct device *dev;
+	struct i2c_client *i2c_client;
+	struct gpio_desc *reset;
+	struct gpio_desc *powerdown;
+	struct regulator *avdd;
+	struct regulator *dvdd;
+	struct regulator *dovdd;
+	struct clk *extclk;
+
+	struct v4l2_fwnode_endpoint endpoint;
+	struct v4l2_subdev subdev;
+	struct media_pad pad;
+
+	struct mutex mutex;
+
+	struct ov8865_state state;
+	struct ov8865_ctrls ctrls;
+};
+
+/* Static definitions */
+
+/*
+ * EXTCLK = 24 MHz
+ * PHY_SCLK = 720 MHz
+ * MIPI_PCLK = 90 MHz
+ */
+static const struct ov8865_pll1_config ov8865_pll1_config_native = {
+	.pll_pre_div_half	= 1,
+	.pll_pre_div		= 0,
+	.pll_mul		= 30,
+	.m_div			= 1,
+	.mipi_div		= 3,
+	.pclk_div		= 1,
+	.sys_pre_div		= 1,
+	.sys_div		= 2,
+};
+
+/*
+ * EXTCLK = 24 MHz
+ * DAC_CLK = 360 MHz
+ * SCLK = 144 MHz
+ */
+
+static const struct ov8865_pll2_config ov8865_pll2_config_native = {
+	.pll_pre_div_half	= 1,
+	.pll_pre_div		= 0,
+	.pll_mul		= 30,
+	.dac_div		= 2,
+	.sys_pre_div		= 5,
+	.sys_div		= 0,
+};
+
+static const struct ov8865_sclk_config ov8865_sclk_config_native = {
+	.sys_sel		= 1,
+	.sclk_sel		= 0,
+	.sclk_pre_div		= 0,
+	.sclk_div		= 0,
+};
+
+static const struct ov8865_register_value ov8865_register_values_native[] = {
+	/* Sensor */
+
+	{ 0x3700, 0x48 },
+	{ 0x3701, 0x18 },
+	{ 0x3702, 0x50 },
+	{ 0x3703, 0x32 },
+	{ 0x3704, 0x28 },
+	{ 0x3706, 0x70 },
+	{ 0x3707, 0x08 },
+	{ 0x3708, 0x48 },
+	{ 0x3709, 0x80 },
+	{ 0x370a, 0x01 },
+	{ 0x370b, 0x70 },
+	{ 0x370c, 0x07 },
+	{ 0x3718, 0x14 },
+	{ 0x3712, 0x44 },
+	{ 0x371e, 0x31 },
+	{ 0x371f, 0x7f },
+	{ 0x3720, 0x0a },
+	{ 0x3721, 0x0a },
+	{ 0x3724, 0x04 },
+	{ 0x3725, 0x04 },
+	{ 0x3726, 0x0c },
+	{ 0x3728, 0x0a },
+	{ 0x3729, 0x03 },
+	{ 0x372a, 0x06 },
+	{ 0x372b, 0xa6 },
+	{ 0x372c, 0xa6 },
+	{ 0x372d, 0xa6 },
+	{ 0x372e, 0x0c },
+	{ 0x372f, 0x20 },
+	{ 0x3730, 0x02 },
+	{ 0x3731, 0x0c },
+	{ 0x3732, 0x28 },
+	{ 0x3736, 0x30 },
+	{ 0x373a, 0x04 },
+	{ 0x373b, 0x18 },
+	{ 0x373c, 0x14 },
+	{ 0x373e, 0x06 },
+	{ 0x375a, 0x0c },
+	{ 0x375b, 0x26 },
+	{ 0x375d, 0x04 },
+	{ 0x375f, 0x28 },
+	{ 0x3767, 0x1e },
+	{ 0x3772, 0x46 },
+	{ 0x3773, 0x04 },
+	{ 0x3774, 0x2c },
+	{ 0x3775, 0x13 },
+	{ 0x3776, 0x10 },
+	{ 0x37a0, 0x88 },
+	{ 0x37a1, 0x7a },
+	{ 0x37a2, 0x7a },
+	{ 0x37a3, 0x02 },
+	{ 0x37a5, 0x09 },
+	{ 0x37a7, 0x88 },
+	{ 0x37a8, 0xb0 },
+	{ 0x37a9, 0xb0 },
+	{ 0x37aa, 0x88 },
+	{ 0x37ab, 0x5c },
+	{ 0x37ac, 0x5c },
+	{ 0x37ad, 0x55 },
+	{ 0x37ae, 0x19 },
+	{ 0x37af, 0x19 },
+	{ 0x37b3, 0x84 },
+	{ 0x37b4, 0x84 },
+	{ 0x37b5, 0x66 },
+
+	/* PSRAM */
+
+	{ OV8865_PSRAM_CTRL8_REG, 0x16 },
+
+	/* ADC Sync */
+
+	{ 0x4500, 0x68 },
+};
+
+static const struct ov8865_mode ov8865_mode_3264x2448 = {
+	/* Horizontal */
+	.output_size_x			= 3264,
+	.hts				= 1944,
+
+	/* Vertical */
+	.output_size_y			= 2448,
+	.vts				= 2470,
+
+	.size_auto			= true,
+	.size_auto_boundary_x		= 8,
+	.size_auto_boundary_y		= 4,
+
+	/* Subsample increase */
+	.inc_x_odd			= 1,
+	.inc_x_even			= 1,
+	.inc_y_odd			= 1,
+	.inc_y_even			= 1,
+
+	/* VFIFO */
+	.vfifo_read_start		= 16,
+
+	.ablc_num			= 4,
+	.zline_num			= 1,
+
+	/* Black Level */
+
+	.blc_top_zero_line_start	= 0,
+	.blc_top_zero_line_num		= 2,
+	.blc_top_black_line_start	= 4,
+	.blc_top_black_line_num		= 4,
+
+	.blc_bottom_zero_line_start	= 2,
+	.blc_bottom_zero_line_num	= 2,
+	.blc_bottom_black_line_start	= 8,
+	.blc_bottom_black_line_num	= 2,
+
+	.blc_anchor_left_start		= 576,
+	.blc_anchor_left_end		= 831,
+	.blc_anchor_right_start		= 1984,
+	.blc_anchor_right_end		= 2239,
+
+	/* Frame Interval */
+	.frame_interval			= { 1, 30 },
+
+	/* PLL */
+	.pll1_config		= &ov8865_pll1_config_native,
+	.pll2_config		= &ov8865_pll2_config_native,
+	.sclk_config		= &ov8865_sclk_config_native,
+
+	/* Registers */
+	.register_values	= ov8865_register_values_native,
+	.register_values_count	= ARRAY_SIZE(ov8865_register_values_native),
+};
+
+static const struct ov8865_mode ov8865_mode_3264x1836 = {
+	/* Horizontal */
+	.output_size_x			= 3264,
+	.hts				= 2582,
+
+	/* Vertical */
+	.output_size_y			= 1836,
+	.vts				= 2002,
+
+	.size_auto			= true,
+	.size_auto_boundary_x		= 8,
+	.size_auto_boundary_y		= 4,
+
+	/* Subsample increase */
+	.inc_x_odd			= 1,
+	.inc_x_even			= 1,
+	.inc_y_odd			= 1,
+	.inc_y_even			= 1,
+
+	/* VFIFO */
+	.vfifo_read_start		= 16,
+
+	.ablc_num			= 4,
+	.zline_num			= 1,
+
+	/* Black Level */
+
+	.blc_top_zero_line_start	= 0,
+	.blc_top_zero_line_num		= 2,
+	.blc_top_black_line_start	= 4,
+	.blc_top_black_line_num		= 4,
+
+	.blc_bottom_zero_line_start	= 2,
+	.blc_bottom_zero_line_num	= 2,
+	.blc_bottom_black_line_start	= 8,
+	.blc_bottom_black_line_num	= 2,
+
+	.blc_anchor_left_start		= 576,
+	.blc_anchor_left_end		= 831,
+	.blc_anchor_right_start		= 1984,
+	.blc_anchor_right_end		= 2239,
+
+	/* Frame Interval */
+	.frame_interval			= { 1, 30 },
+
+	/* PLL */
+	.pll1_config		= &ov8865_pll1_config_native,
+	.pll2_config		= &ov8865_pll2_config_native,
+	.sclk_config		= &ov8865_sclk_config_native,
+
+	/* Registers */
+	.register_values	= ov8865_register_values_native,
+	.register_values_count	= ARRAY_SIZE(ov8865_register_values_native),
+};
+
+/*
+ * EXTCLK = 24 MHz
+ * DAC_CLK = 360 MHz
+ * SCLK = 80 MHz
+ */
+
+static const struct ov8865_pll2_config ov8865_pll2_config_binning = {
+	.pll_pre_div_half	= 1,
+	.pll_pre_div		= 0,
+	.pll_mul		= 30,
+	.dac_div		= 2,
+	.sys_pre_div		= 10,
+	.sys_div		= 0,
+};
+
+static const struct ov8865_register_value ov8865_register_values_binning[] = {
+	/* Sensor */
+
+	{ 0x3700, 0x24 },
+	{ 0x3701, 0x0c },
+	{ 0x3702, 0x28 },
+	{ 0x3703, 0x19 },
+	{ 0x3704, 0x14 },
+	{ 0x3706, 0x38 },
+	{ 0x3707, 0x04 },
+	{ 0x3708, 0x24 },
+	{ 0x3709, 0x40 },
+	{ 0x370a, 0x00 },
+	{ 0x370b, 0xb8 },
+	{ 0x370c, 0x04 },
+	{ 0x3718, 0x12 },
+	{ 0x3712, 0x42 },
+	{ 0x371e, 0x19 },
+	{ 0x371f, 0x40 },
+	{ 0x3720, 0x05 },
+	{ 0x3721, 0x05 },
+	{ 0x3724, 0x02 },
+	{ 0x3725, 0x02 },
+	{ 0x3726, 0x06 },
+	{ 0x3728, 0x05 },
+	{ 0x3729, 0x02 },
+	{ 0x372a, 0x03 },
+	{ 0x372b, 0x53 },
+	{ 0x372c, 0xa3 },
+	{ 0x372d, 0x53 },
+	{ 0x372e, 0x06 },
+	{ 0x372f, 0x10 },
+	{ 0x3730, 0x01 },
+	{ 0x3731, 0x06 },
+	{ 0x3732, 0x14 },
+	{ 0x3736, 0x20 },
+	{ 0x373a, 0x02 },
+	{ 0x373b, 0x0c },
+	{ 0x373c, 0x0a },
+	{ 0x373e, 0x03 },
+	{ 0x375a, 0x06 },
+	{ 0x375b, 0x13 },
+	{ 0x375d, 0x02 },
+	{ 0x375f, 0x14 },
+	{ 0x3767, 0x1c },
+	{ 0x3772, 0x23 },
+	{ 0x3773, 0x02 },
+	{ 0x3774, 0x16 },
+	{ 0x3775, 0x12 },
+	{ 0x3776, 0x08 },
+	{ 0x37a0, 0x44 },
+	{ 0x37a1, 0x3d },
+	{ 0x37a2, 0x3d },
+	{ 0x37a3, 0x01 },
+	{ 0x37a5, 0x08 },
+	{ 0x37a7, 0x44 },
+	{ 0x37a8, 0x58 },
+	{ 0x37a9, 0x58 },
+	{ 0x37aa, 0x44 },
+	{ 0x37ab, 0x2e },
+	{ 0x37ac, 0x2e },
+	{ 0x37ad, 0x33 },
+	{ 0x37ae, 0x0d },
+	{ 0x37af, 0x0d },
+	{ 0x37b3, 0x42 },
+	{ 0x37b4, 0x42 },
+	{ 0x37b5, 0x33 },
+
+	/* PSRAM */
+
+	{ OV8865_PSRAM_CTRL8_REG, 0x0b },
+
+	/* ADC Sync */
+
+	{ 0x4500, 0x40 },
+};
+
+static const struct ov8865_mode ov8865_mode_1632x1224 = {
+	/* Horizontal */
+	.output_size_x			= 1632,
+	.hts				= 1923,
+
+	/* Vertical */
+	.output_size_y			= 1224,
+	.vts				= 1248,
+
+	.size_auto			= true,
+	.size_auto_boundary_x		= 8,
+	.size_auto_boundary_y		= 8,
+
+	/* Subsample increase */
+	.inc_x_odd			= 3,
+	.inc_x_even			= 1,
+	.inc_y_odd			= 3,
+	.inc_y_even			= 1,
+
+	/* Binning */
+	.binning_y			= true,
+	.sync_hbin			= true,
+
+	/* VFIFO */
+	.vfifo_read_start		= 116,
+
+	.ablc_num			= 8,
+	.zline_num			= 2,
+
+	/* Black Level */
+
+	.blc_top_zero_line_start	= 0,
+	.blc_top_zero_line_num		= 2,
+	.blc_top_black_line_start	= 4,
+	.blc_top_black_line_num		= 4,
+
+	.blc_bottom_zero_line_start	= 2,
+	.blc_bottom_zero_line_num	= 2,
+	.blc_bottom_black_line_start	= 8,
+	.blc_bottom_black_line_num	= 2,
+
+	.blc_anchor_left_start		= 288,
+	.blc_anchor_left_end		= 415,
+	.blc_anchor_right_start		= 992,
+	.blc_anchor_right_end		= 1119,
+
+	/* Frame Interval */
+	.frame_interval			= { 1, 30 },
+
+	/* PLL */
+	.pll1_config		= &ov8865_pll1_config_native,
+	.pll2_config		= &ov8865_pll2_config_binning,
+	.sclk_config		= &ov8865_sclk_config_native,
+
+	/* Registers */
+	.register_values	= ov8865_register_values_binning,
+	.register_values_count	= ARRAY_SIZE(ov8865_register_values_binning),
+};
+
+static const struct ov8865_mode ov8865_mode_svga = {
+	/* Horizontal */
+	.output_size_x			= 800,
+	.hts				= 1250,
+
+	/* Vertical */
+	.output_size_y			= 600,
+	.vts				= 640,
+
+	.size_auto			= true,
+	.size_auto_boundary_x		= 8,
+	.size_auto_boundary_y		= 8,
+
+	/* Subsample increase */
+	.inc_x_odd			= 3,
+	.inc_x_even			= 1,
+	.inc_y_odd			= 5,
+	.inc_y_even			= 3,
+
+	/* Binning */
+	.binning_y			= true,
+	.variopixel			= true,
+	.variopixel_hsub_coef		= 2,
+	.variopixel_vsub_coef		= 1,
+	.sync_hbin			= true,
+	.horz_var2			= true,
+
+	/* VFIFO */
+	.vfifo_read_start		= 80,
+
+	.ablc_num			= 8,
+	.zline_num			= 2,
+
+	/* Black Level */
+
+	.blc_top_zero_line_start	= 0,
+	.blc_top_zero_line_num		= 2,
+	.blc_top_black_line_start	= 2,
+	.blc_top_black_line_num		= 2,
+
+	.blc_bottom_zero_line_start	= 0,
+	.blc_bottom_zero_line_num	= 0,
+	.blc_bottom_black_line_start	= 4,
+	.blc_bottom_black_line_num	= 2,
+
+	.blc_col_shift_mask		= OV8865_BLC_CTRL1_COL_SHIFT_128,
+
+	.blc_anchor_left_start		= 288,
+	.blc_anchor_left_end		= 415,
+	.blc_anchor_right_start		= 992,
+	.blc_anchor_right_end		= 1119,
+
+	/* Frame Interval */
+	.frame_interval			= { 1, 90 },
+
+	/* PLL */
+	.pll1_config		= &ov8865_pll1_config_native,
+	.pll2_config		= &ov8865_pll2_config_binning,
+	.sclk_config		= &ov8865_sclk_config_native,
+
+	/* Registers */
+	.register_values	= ov8865_register_values_binning,
+	.register_values_count	= ARRAY_SIZE(ov8865_register_values_binning),
+};
+
+static const struct ov8865_mode *ov8865_modes[] = {
+	&ov8865_mode_3264x2448,
+	&ov8865_mode_3264x1836,
+	&ov8865_mode_1632x1224,
+	&ov8865_mode_svga,
+};
+
+static const u32 ov8865_mbus_codes[] = {
+	MEDIA_BUS_FMT_SBGGR10_1X10,
+};
+
+static const struct ov8865_register_value ov8865_init_sequence[] = {
+	/* Analog */
+
+	{ 0x3604, 0x04 },
+	{ 0x3602, 0x30 },
+	{ 0x3605, 0x00 },
+	{ 0x3607, 0x20 },
+	{ 0x3608, 0x11 },
+	{ 0x3609, 0x68 },
+	{ 0x360a, 0x40 },
+	{ 0x360c, 0xdd },
+	{ 0x360e, 0x0c },
+	{ 0x3610, 0x07 },
+	{ 0x3612, 0x86 },
+	{ 0x3613, 0x58 },
+	{ 0x3614, 0x28 },
+	{ 0x3617, 0x40 },
+	{ 0x3618, 0x5a },
+	{ 0x3619, 0x9b },
+	{ 0x361c, 0x00 },
+	{ 0x361d, 0x60 },
+	{ 0x3631, 0x60 },
+	{ 0x3633, 0x10 },
+	{ 0x3634, 0x10 },
+	{ 0x3635, 0x10 },
+	{ 0x3636, 0x10 },
+	{ 0x3638, 0xff },
+	{ 0x3641, 0x55 },
+	{ 0x3646, 0x86 },
+	{ 0x3647, 0x27 },
+	{ 0x364a, 0x1b },
+
+	/* Sensor */
+
+	{ 0x3700, 0x24 },
+	{ 0x3701, 0x0c },
+	{ 0x3702, 0x28 },
+	{ 0x3703, 0x19 },
+	{ 0x3704, 0x14 },
+	{ 0x3705, 0x00 },
+	{ 0x3706, 0x38 },
+	{ 0x3707, 0x04 },
+	{ 0x3708, 0x24 },
+	{ 0x3709, 0x40 },
+	{ 0x370a, 0x00 },
+	{ 0x370b, 0xb8 },
+	{ 0x370c, 0x04 },
+	{ 0x3718, 0x12 },
+	{ 0x3719, 0x31 },
+	{ 0x3712, 0x42 },
+	{ 0x3714, 0x12 },
+	{ 0x371e, 0x19 },
+	{ 0x371f, 0x40 },
+	{ 0x3720, 0x05 },
+	{ 0x3721, 0x05 },
+	{ 0x3724, 0x02 },
+	{ 0x3725, 0x02 },
+	{ 0x3726, 0x06 },
+	{ 0x3728, 0x05 },
+	{ 0x3729, 0x02 },
+	{ 0x372a, 0x03 },
+	{ 0x372b, 0x53 },
+	{ 0x372c, 0xa3 },
+	{ 0x372d, 0x53 },
+	{ 0x372e, 0x06 },
+	{ 0x372f, 0x10 },
+	{ 0x3730, 0x01 },
+	{ 0x3731, 0x06 },
+	{ 0x3732, 0x14 },
+	{ 0x3733, 0x10 },
+	{ 0x3734, 0x40 },
+	{ 0x3736, 0x20 },
+	{ 0x373a, 0x02 },
+	{ 0x373b, 0x0c },
+	{ 0x373c, 0x0a },
+	{ 0x373e, 0x03 },
+	{ 0x3755, 0x40 },
+	{ 0x3758, 0x00 },
+	{ 0x3759, 0x4c },
+	{ 0x375a, 0x06 },
+	{ 0x375b, 0x13 },
+	{ 0x375c, 0x40 },
+	{ 0x375d, 0x02 },
+	{ 0x375e, 0x00 },
+	{ 0x375f, 0x14 },
+	{ 0x3767, 0x1c },
+	{ 0x3768, 0x04 },
+	{ 0x3769, 0x20 },
+	{ 0x376c, 0xc0 },
+	{ 0x376d, 0xc0 },
+	{ 0x376a, 0x08 },
+	{ 0x3761, 0x00 },
+	{ 0x3762, 0x00 },
+	{ 0x3763, 0x00 },
+	{ 0x3766, 0xff },
+	{ 0x376b, 0x42 },
+	{ 0x3772, 0x23 },
+	{ 0x3773, 0x02 },
+	{ 0x3774, 0x16 },
+	{ 0x3775, 0x12 },
+	{ 0x3776, 0x08 },
+	{ 0x37a0, 0x44 },
+	{ 0x37a1, 0x3d },
+	{ 0x37a2, 0x3d },
+	{ 0x37a3, 0x01 },
+	{ 0x37a4, 0x00 },
+	{ 0x37a5, 0x08 },
+	{ 0x37a6, 0x00 },
+	{ 0x37a7, 0x44 },
+	{ 0x37a8, 0x58 },
+	{ 0x37a9, 0x58 },
+	{ 0x3760, 0x00 },
+	{ 0x376f, 0x01 },
+	{ 0x37aa, 0x44 },
+	{ 0x37ab, 0x2e },
+	{ 0x37ac, 0x2e },
+	{ 0x37ad, 0x33 },
+	{ 0x37ae, 0x0d },
+	{ 0x37af, 0x0d },
+	{ 0x37b0, 0x00 },
+	{ 0x37b1, 0x00 },
+	{ 0x37b2, 0x00 },
+	{ 0x37b3, 0x42 },
+	{ 0x37b4, 0x42 },
+	{ 0x37b5, 0x33 },
+	{ 0x37b6, 0x00 },
+	{ 0x37b7, 0x00 },
+	{ 0x37b8, 0x00 },
+	{ 0x37b9, 0xff },
+
+	/* ADC Sync */
+
+	{ 0x4503, 0x10 },
+};
+
+static const s64 ov8865_link_freq_menu[] = {
+	360000000,
+};
+
+static const char *const ov8865_test_pattern_menu[] = {
+	"Disabled",
+	"Random data",
+	"Color bars",
+	"Color bars with rolling bar",
+	"Color squares",
+	"Color squares with rolling bar"
+};
+
+static const u8 ov8865_test_pattern_bits[] = {
+	0,
+	OV8865_PRE_CTRL0_PATTERN_EN | OV8865_PRE_CTRL0_PATTERN_RANDOM_DATA,
+	OV8865_PRE_CTRL0_PATTERN_EN | OV8865_PRE_CTRL0_PATTERN_COLOR_BARS,
+	OV8865_PRE_CTRL0_PATTERN_EN | OV8865_PRE_CTRL0_ROLLING_BAR_EN |
+	OV8865_PRE_CTRL0_PATTERN_COLOR_BARS,
+	OV8865_PRE_CTRL0_PATTERN_EN | OV8865_PRE_CTRL0_PATTERN_COLOR_SQUARES,
+	OV8865_PRE_CTRL0_PATTERN_EN | OV8865_PRE_CTRL0_ROLLING_BAR_EN |
+	OV8865_PRE_CTRL0_PATTERN_COLOR_SQUARES,
+};
+
+/* Input/Output */
+
+static int ov8865_read(struct ov8865_sensor *sensor, u16 address, u8 *value)
+{
+	unsigned char data[2] = { address >> 8, address & 0xff };
+	struct i2c_client *client = sensor->i2c_client;
+	int ret;
+
+	ret = i2c_master_send(client, data, sizeof(data));
+	if (ret < 0) {
+		dev_dbg(&client->dev, "i2c send error at address %#04x\n",
+			address);
+		return ret;
+	}
+
+	ret = i2c_master_recv(client, value, 1);
+	if (ret < 0) {
+		dev_dbg(&client->dev, "i2c recv error at address %#04x\n",
+			address);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int ov8865_write(struct ov8865_sensor *sensor, u16 address, u8 value)
+{
+	unsigned char data[3] = { address >> 8, address & 0xff, value };
+	struct i2c_client *client = sensor->i2c_client;
+	int ret;
+	u8 value_prev;
+
+	ov8865_read(sensor, address, &value_prev);
+
+	printk(KERN_ERR "ov8865: %#04x <= %#02x from %#02x\n", address, value, value_prev);
+
+	ret = i2c_master_send(client, data, sizeof(data));
+	if (ret < 0) {
+		dev_dbg(&client->dev, "i2c send error at address %#04x\n",
+			address);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int ov8865_write_sequence(struct ov8865_sensor *sensor,
+				 const struct ov8865_register_value *sequence,
+				 unsigned int sequence_count)
+{
+	unsigned int i;
+	int ret = 0;
+
+	for (i = 0; i < sequence_count; i++) {
+		ret = ov8865_write(sensor, sequence[i].address,
+				   sequence[i].value);
+		if (ret)
+			break;
+
+		if (sequence[i].delay_ms)
+			msleep(sequence[i].delay_ms);
+	}
+
+	return ret;
+}
+
+static int ov8865_update_bits(struct ov8865_sensor *sensor, u16 address,
+			      u8 mask, u8 bits)
+{
+	u8 value = 0;
+	int ret;
+
+	ret = ov8865_read(sensor, address, &value);
+	if (ret)
+		return ret;
+
+	value &= ~mask;
+	value |= bits;
+
+	ret = ov8865_write(sensor, address, value);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+/* Sensor */
+
+static int ov8865_sw_reset(struct ov8865_sensor *sensor)
+{
+	int ret;
+
+	ret = ov8865_write(sensor, OV8865_SW_RESET_REG, OV8865_SW_RESET_RESET);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+static int ov8865_sw_standby(struct ov8865_sensor *sensor, int standby)
+{
+	u8 value = 0;
+	int ret;
+
+	if (!standby)
+		value = OV8865_SW_STANDBY_STREAM_ON;
+
+	ret = ov8865_write(sensor, OV8865_SW_STANDBY_REG, value);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+static int ov8865_chip_id_check(struct ov8865_sensor *sensor)
+{
+	u16 regs[] = { OV8865_CHIP_ID_HH_REG, OV8865_CHIP_ID_H_REG,
+		       OV8865_CHIP_ID_L_REG };
+	u8 values[] = { OV8865_CHIP_ID_HH_VALUE, OV8865_CHIP_ID_H_VALUE,
+			OV8865_CHIP_ID_L_VALUE };
+	unsigned int i;
+	u8 value;
+	int ret;
+
+	for (i = 0; i < ARRAY_SIZE(regs); i++) {
+		ret = ov8865_read(sensor, regs[i], &value);
+		if (ret < 0)
+			return ret;
+
+		if (value != values[i]) {
+			dev_err(sensor->dev,
+				"chip id value mismatch: %#x instead of %#x\n",
+				value, values[i]);
+			return -EINVAL;
+		}
+	}
+
+	return 0;
+}
+
+static int ov8865_charge_pump_configure(struct ov8865_sensor *sensor)
+{
+	int ret;
+
+	ret = ov8865_write(sensor, OV8865_PUMP_CLK_DIV_REG,
+			   OV8865_PUMP_CLK_DIV_PUMP_P(1));
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int ov8865_mipi_configure(struct ov8865_sensor *sensor)
+{
+	struct v4l2_fwnode_bus_mipi_csi2 *bus_mipi_csi2 =
+		&sensor->endpoint.bus.mipi_csi2;
+	unsigned int lanes_count = bus_mipi_csi2->num_data_lanes;
+	int ret;
+
+	ret = ov8865_write(sensor, OV8865_MIPI_SC_CTRL0_REG,
+			   OV8865_MIPI_SC_CTRL0_LANES(lanes_count) |
+			   OV8865_MIPI_SC_CTRL0_MIPI_EN |
+			   OV8865_MIPI_SC_CTRL0_UNKNOWN);
+	if (ret)
+		return ret;
+
+	ret = ov8865_write(sensor, OV8865_MIPI_SC_CTRL2_REG,
+			   OV8865_MIPI_SC_CTRL2_PD_MIPI_RST_SYNC);
+	if (ret)
+		return ret;
+
+	if (lanes_count >= 2) {
+		ret = ov8865_write(sensor, OV8865_MIPI_LANE_SEL01_REG,
+				   OV8865_MIPI_LANE_SEL01_LANE0(0) |
+				   OV8865_MIPI_LANE_SEL01_LANE1(1));
+		if (ret)
+			return ret;
+	}
+
+	if (lanes_count >= 4) {
+		ret = ov8865_write(sensor, OV8865_MIPI_LANE_SEL23_REG,
+				   OV8865_MIPI_LANE_SEL23_LANE2(2) |
+				   OV8865_MIPI_LANE_SEL23_LANE3(3));
+		if (ret)
+			return ret;
+	}
+
+	ret = ov8865_update_bits(sensor, OV8865_CLK_SEL1_REG,
+				 OV8865_CLK_SEL1_MIPI_EOF,
+				 OV8865_CLK_SEL1_MIPI_EOF);
+	if (ret)
+		return ret;
+
+	/*
+	 * This value might need to change depending on PCLK rate,
+	 * but it's unclear how. This value seems to generally work
+	 * while the default value was found to cause transmission errors.
+	 */
+	ret = ov8865_write(sensor, OV8865_MIPI_PCLK_PERIOD_REG, 0x16);
+	if (ret)
+		return ret;
+
+
+	return 0;
+}
+
+static int ov8865_black_level_configure(struct ov8865_sensor *sensor)
+{
+	int ret;
+
+	/* Trigger BLC on relevant events and enable filter. */
+	ret = ov8865_write(sensor, OV8865_BLC_CTRL0_REG,
+			   OV8865_BLC_CTRL0_TRIG_RANGE_EN |
+			   OV8865_BLC_CTRL0_TRIG_FORMAT_EN |
+			   OV8865_BLC_CTRL0_TRIG_GAIN_EN |
+			   OV8865_BLC_CTRL0_TRIG_EXPOSURE_EN |
+			   OV8865_BLC_CTRL0_FILTER_EN);
+	if (ret)
+		return ret;
+
+	/* Lower BLC offset trigger threshold. */
+	ret = ov8865_write(sensor, OV8865_BLC_CTRLD_REG,
+			   OV8865_BLC_CTRLD_OFFSET_TRIGGER(16));
+	if (ret)
+		return ret;
+
+	ret = ov8865_write(sensor, OV8865_BLC_CTRL1F_REG, 0);
+	if (ret)
+		return ret;
+
+	/* Increase BLC offset maximum limit. */
+	ret = ov8865_write(sensor, OV8865_BLC_OFFSET_LIMIT_REG,
+			   OV8865_BLC_OFFSET_LIMIT(63));
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int ov8865_isp_configure(struct ov8865_sensor *sensor)
+{
+	int ret;
+
+	/* Disable lens correction. */
+	ret = ov8865_write(sensor, OV8865_ISP_CTRL0_REG,
+			   OV8865_ISP_CTRL0_WHITE_BALANCE_EN |
+			   OV8865_ISP_CTRL0_DPC_BLACK_EN |
+			   OV8865_ISP_CTRL0_DPC_WHITE_EN);
+	if (ret)
+		return ret;
+
+	ret = ov8865_write(sensor, OV8865_ISP_CTRL1_REG,
+			   OV8865_ISP_CTRL1_BLC_EN);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static unsigned long ov8865_mode_pll1_rate(struct ov8865_sensor *sensor,
+					   const struct ov8865_mode *mode)
+{
+	const struct ov8865_pll1_config *config = mode->pll1_config;
+	unsigned long extclk_rate;
+	unsigned long pll1_rate;
+
+	extclk_rate = clk_get_rate(sensor->extclk);
+	pll1_rate = extclk_rate * config->pll_mul / config->pll_pre_div_half;
+
+	switch (config->pll_pre_div) {
+	case 0:
+		break;
+	case 1:
+		pll1_rate *= 3;
+		pll1_rate /= 2;
+		break;
+	case 3:
+		pll1_rate *= 5;
+		pll1_rate /= 2;
+		break;
+	case 4:
+		pll1_rate /= 3;
+		break;
+	case 5:
+		pll1_rate /= 4;
+		break;
+	case 7:
+		pll1_rate /= 8;
+		break;
+	default:
+		pll1_rate /= config->pll_pre_div;
+		break;
+	}
+
+	return pll1_rate;
+}
+
+static int ov8865_mode_pll1_configure(struct ov8865_sensor *sensor,
+				      const struct ov8865_mode *mode,
+				      u32 mbus_code)
+{
+	const struct ov8865_pll1_config *config = mode->pll1_config;
+	u8 value;
+	int ret;
+
+	switch (mbus_code) {
+	case MEDIA_BUS_FMT_SBGGR10_1X10:
+		value = OV8865_MIPI_BIT_SEL(10);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	ret = ov8865_write(sensor, OV8865_MIPI_BIT_SEL_REG, value);
+	if (ret)
+		return ret;
+
+	ret = ov8865_write(sensor, OV8865_PLL_CTRLA_REG,
+			   OV8865_PLL_CTRLA_PRE_DIV_HALF(config->pll_pre_div_half));
+	if (ret)
+		return ret;
+
+	ret = ov8865_write(sensor, OV8865_PLL_CTRL0_REG,
+			   OV8865_PLL_CTRL0_PRE_DIV(config->pll_pre_div));
+	if (ret)
+		return ret;
+
+	ret = ov8865_write(sensor, OV8865_PLL_CTRL1_REG,
+			   OV8865_PLL_CTRL1_MUL_H(config->pll_mul));
+	if (ret)
+		return ret;
+
+	ret = ov8865_write(sensor, OV8865_PLL_CTRL2_REG,
+			   OV8865_PLL_CTRL2_MUL_L(config->pll_mul));
+	if (ret)
+		return ret;
+
+	ret = ov8865_write(sensor, OV8865_PLL_CTRL3_REG,
+			   OV8865_PLL_CTRL3_M_DIV(config->m_div));
+	if (ret)
+		return ret;
+
+	ret = ov8865_write(sensor, OV8865_PLL_CTRL4_REG,
+			   OV8865_PLL_CTRL4_MIPI_DIV(config->mipi_div));
+	if (ret)
+		return ret;
+
+	ret = ov8865_update_bits(sensor, OV8865_PCLK_SEL_REG,
+			   OV8865_PCLK_SEL_PCLK_DIV_MASK,
+			   OV8865_PCLK_SEL_PCLK_DIV(config->pclk_div));
+	if (ret)
+		return ret;
+
+	ret = ov8865_write(sensor, OV8865_PLL_CTRL5_REG,
+			   OV8865_PLL_CTRL5_SYS_PRE_DIV(config->sys_pre_div));
+	if (ret)
+		return ret;
+
+	ret = ov8865_write(sensor, OV8865_PLL_CTRL6_REG,
+			   OV8865_PLL_CTRL6_SYS_DIV(config->sys_div));
+	if (ret)
+		return ret;
+
+	ret = ov8865_update_bits(sensor, OV8865_PLL_CTRL1E_REG,
+				 OV8865_PLL_CTRL1E_PLL1_NO_LAT,
+				 OV8865_PLL_CTRL1E_PLL1_NO_LAT);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int ov8865_mode_pll2_configure(struct ov8865_sensor *sensor,
+				      const struct ov8865_mode *mode)
+{
+	const struct ov8865_pll2_config *config = mode->pll2_config;
+	int ret;
+
+	ret = ov8865_write(sensor, OV8865_PLL_CTRL12_REG,
+			   OV8865_PLL_CTRL12_PRE_DIV_HALF(config->pll_pre_div_half) |
+			   OV8865_PLL_CTRL12_DAC_DIV(config->dac_div));
+	if (ret)
+		return ret;
+
+	ret = ov8865_write(sensor, OV8865_PLL_CTRLB_REG,
+			   OV8865_PLL_CTRLB_PRE_DIV(config->pll_pre_div));
+	if (ret)
+		return ret;
+
+	ret = ov8865_write(sensor, OV8865_PLL_CTRLC_REG,
+			   OV8865_PLL_CTRLC_MUL_H(config->pll_mul));
+	if (ret)
+		return ret;
+
+	ret = ov8865_write(sensor, OV8865_PLL_CTRLD_REG,
+			   OV8865_PLL_CTRLD_MUL_L(config->pll_mul));
+	if (ret)
+		return ret;
+
+	ret = ov8865_write(sensor, OV8865_PLL_CTRLF_REG,
+			   OV8865_PLL_CTRLF_SYS_PRE_DIV(config->sys_pre_div));
+	if (ret)
+		return ret;
+
+	ret = ov8865_write(sensor, OV8865_PLL_CTRLE_REG,
+			   OV8865_PLL_CTRLE_SYS_DIV(config->sys_div));
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int ov8865_mode_sclk_configure(struct ov8865_sensor *sensor,
+				      const struct ov8865_mode *mode)
+{
+	const struct ov8865_sclk_config *config = mode->sclk_config;
+	int ret;
+
+	ret = ov8865_write(sensor, OV8865_CLK_SEL0_REG,
+			   OV8865_CLK_SEL0_PLL1_SYS_SEL(config->sys_sel));
+	if (ret)
+		return ret;
+
+	ret = ov8865_update_bits(sensor, OV8865_CLK_SEL1_REG,
+				 OV8865_CLK_SEL1_PLL_SCLK_SEL_MASK,
+				 OV8865_CLK_SEL1_PLL_SCLK_SEL(config->sclk_sel));
+	if (ret)
+		return ret;
+
+	ret = ov8865_write(sensor, OV8865_SCLK_CTRL_REG,
+			   OV8865_SCLK_CTRL_UNKNOWN |
+			   OV8865_SCLK_CTRL_SCLK_DIV(config->sclk_div) |
+			   OV8865_SCLK_CTRL_SCLK_PRE_DIV(config->sclk_pre_div));
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int ov8865_mode_binning_configure(struct ov8865_sensor *sensor,
+					 const struct ov8865_mode *mode)
+{
+	unsigned int variopixel_hsub_coef, variopixel_vsub_coef;
+	u8 value;
+	int ret;
+
+	ret = ov8865_write(sensor, OV8865_FORMAT1_REG, 0);
+	if (ret)
+		return ret;
+
+	value = OV8865_FORMAT2_HSYNC_EN;
+
+	if (mode->binning_x)
+		value |= OV8865_FORMAT2_FST_HBIN_EN;
+
+	if (mode->binning_y)
+		value |= OV8865_FORMAT2_FST_VBIN_EN;
+
+	if (mode->sync_hbin)
+		value |= OV8865_FORMAT2_SYNC_HBIN_EN;
+
+	if (mode->horz_var2)
+		value |= OV8865_FORMAT2_ISP_HORZ_VAR2_EN;
+
+	ret = ov8865_write(sensor, OV8865_FORMAT2_REG, value);
+	if (ret)
+		return ret;
+
+	ret = ov8865_update_bits(sensor, OV8865_ISP_CTRL2_REG,
+				 OV8865_ISP_CTRL2_VARIOPIXEL_EN,
+				 mode->variopixel ?
+				 OV8865_ISP_CTRL2_VARIOPIXEL_EN : 0);
+	if (ret)
+		return ret;
+
+	if (mode->variopixel) {
+		/* VarioPixel coefs needs to be > 1. */
+		variopixel_hsub_coef = mode->variopixel_hsub_coef;
+		variopixel_vsub_coef = mode->variopixel_vsub_coef;
+	} else {
+		variopixel_hsub_coef = 1;
+		variopixel_vsub_coef = 1;
+	}
+
+	ret = ov8865_write(sensor, OV8865_VAP_CTRL1_REG,
+			   OV8865_VAP_CTRL1_HSUB_COEF(variopixel_hsub_coef) |
+			   OV8865_VAP_CTRL1_VSUB_COEF(variopixel_vsub_coef));
+	if (ret)
+		return ret;
+
+	ret = ov8865_write(sensor, OV8865_INC_X_ODD_REG,
+			   OV8865_INC_X_ODD(mode->inc_x_odd));
+	if (ret)
+		return ret;
+
+	ret = ov8865_write(sensor, OV8865_INC_X_EVEN_REG,
+			   OV8865_INC_X_EVEN(mode->inc_x_even));
+	if (ret)
+		return ret;
+
+	ret = ov8865_write(sensor, OV8865_INC_Y_ODD_REG,
+			   OV8865_INC_Y_ODD(mode->inc_y_odd));
+	if (ret)
+		return ret;
+
+	ret = ov8865_write(sensor, OV8865_INC_Y_EVEN_REG,
+			   OV8865_INC_Y_EVEN(mode->inc_y_even));
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int ov8865_mode_black_level_configure(struct ov8865_sensor *sensor,
+					     const struct ov8865_mode *mode)
+{
+	int ret;
+
+	/* Note that a zero value for blc_col_shift_mask is the default 256. */
+	ret = ov8865_write(sensor, OV8865_BLC_CTRL1_REG,
+			   mode->blc_col_shift_mask |
+			   OV8865_BLC_CTRL1_OFFSET_LIMIT_EN);
+	if (ret)
+		return ret;
+
+	/* BLC top zero line */
+
+	ret = ov8865_write(sensor, OV8865_BLC_TOP_ZLINE_START_REG,
+			   OV8865_BLC_TOP_ZLINE_START(mode->blc_top_zero_line_start));
+	if (ret)
+		return ret;
+
+	ret = ov8865_write(sensor, OV8865_BLC_TOP_ZLINE_NUM_REG,
+			   OV8865_BLC_TOP_ZLINE_NUM(mode->blc_top_zero_line_num));
+	if (ret)
+		return ret;
+
+	/* BLC top black line */
+
+	ret = ov8865_write(sensor, OV8865_BLC_TOP_BLKLINE_START_REG,
+			   OV8865_BLC_TOP_BLKLINE_START(mode->blc_top_black_line_start));
+	if (ret)
+		return ret;
+
+	ret = ov8865_write(sensor, OV8865_BLC_TOP_BLKLINE_NUM_REG,
+			   OV8865_BLC_TOP_BLKLINE_NUM(mode->blc_top_black_line_num));
+	if (ret)
+		return ret;
+
+	/* BLC bottom zero line */
+
+	ret = ov8865_write(sensor, OV8865_BLC_BOT_ZLINE_START_REG,
+			   OV8865_BLC_BOT_ZLINE_START(mode->blc_bottom_zero_line_start));
+	if (ret)
+		return ret;
+
+	ret = ov8865_write(sensor, OV8865_BLC_BOT_ZLINE_NUM_REG,
+			   OV8865_BLC_BOT_ZLINE_NUM(mode->blc_bottom_zero_line_num));
+	if (ret)
+		return ret;
+
+	/* BLC bottom black line */
+
+	ret = ov8865_write(sensor, OV8865_BLC_BOT_BLKLINE_START_REG,
+			   OV8865_BLC_BOT_BLKLINE_START(mode->blc_bottom_black_line_start));
+	if (ret)
+		return ret;
+
+	ret = ov8865_write(sensor, OV8865_BLC_BOT_BLKLINE_NUM_REG,
+			   OV8865_BLC_BOT_BLKLINE_NUM(mode->blc_bottom_black_line_num));
+	if (ret)
+		return ret;
+
+	/* BLC anchor */
+
+	ret = ov8865_write(sensor, OV8865_BLC_ANCHOR_LEFT_START_H_REG,
+			   OV8865_BLC_ANCHOR_LEFT_START_H(mode->blc_anchor_left_start));
+	if (ret)
+		return ret;
+
+	ret = ov8865_write(sensor, OV8865_BLC_ANCHOR_LEFT_START_L_REG,
+			   OV8865_BLC_ANCHOR_LEFT_START_L(mode->blc_anchor_left_start));
+	if (ret)
+		return ret;
+
+	ret = ov8865_write(sensor, OV8865_BLC_ANCHOR_LEFT_END_H_REG,
+			   OV8865_BLC_ANCHOR_LEFT_END_H(mode->blc_anchor_left_end));
+	if (ret)
+		return ret;
+
+	ret = ov8865_write(sensor, OV8865_BLC_ANCHOR_LEFT_END_L_REG,
+			   OV8865_BLC_ANCHOR_LEFT_END_L(mode->blc_anchor_left_end));
+	if (ret)
+		return ret;
+
+	ret = ov8865_write(sensor, OV8865_BLC_ANCHOR_RIGHT_START_H_REG,
+			   OV8865_BLC_ANCHOR_RIGHT_START_H(mode->blc_anchor_right_start));
+	if (ret)
+		return ret;
+
+	ret = ov8865_write(sensor, OV8865_BLC_ANCHOR_RIGHT_START_L_REG,
+			   OV8865_BLC_ANCHOR_RIGHT_START_L(mode->blc_anchor_right_start));
+	if (ret)
+		return ret;
+
+	ret = ov8865_write(sensor, OV8865_BLC_ANCHOR_RIGHT_END_H_REG,
+			   OV8865_BLC_ANCHOR_RIGHT_END_H(mode->blc_anchor_right_end));
+	if (ret)
+		return ret;
+
+	ret = ov8865_write(sensor, OV8865_BLC_ANCHOR_RIGHT_END_L_REG,
+			   OV8865_BLC_ANCHOR_RIGHT_END_L(mode->blc_anchor_right_end));
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int ov8865_mode_configure(struct ov8865_sensor *sensor,
+				 const struct ov8865_mode *mode, u32 mbus_code)
+{
+	int ret;
+
+	/* Output Size X */
+
+	ret = ov8865_write(sensor, OV8865_OUTPUT_SIZE_X_H_REG,
+			   OV8865_OUTPUT_SIZE_X_H(mode->output_size_x));
+	if (ret)
+		return ret;
+
+	ret = ov8865_write(sensor, OV8865_OUTPUT_SIZE_X_L_REG,
+			   OV8865_OUTPUT_SIZE_X_L(mode->output_size_x));
+	if (ret)
+		return ret;
+
+	/* Horizontal Total Size */
+
+	ret = ov8865_write(sensor, OV8865_HTS_H_REG, OV8865_HTS_H(mode->hts));
+	if (ret)
+		return ret;
+
+	ret = ov8865_write(sensor, OV8865_HTS_L_REG, OV8865_HTS_L(mode->hts));
+	if (ret)
+		return ret;
+
+	/* Output Size Y */
+
+	ret = ov8865_write(sensor, OV8865_OUTPUT_SIZE_Y_H_REG,
+			   OV8865_OUTPUT_SIZE_Y_H(mode->output_size_y));
+	if (ret)
+		return ret;
+
+	ret = ov8865_write(sensor, OV8865_OUTPUT_SIZE_Y_L_REG,
+			   OV8865_OUTPUT_SIZE_Y_L(mode->output_size_y));
+	if (ret)
+		return ret;
+
+	/* Vertical Total Size */
+
+	ret = ov8865_write(sensor, OV8865_VTS_H_REG, OV8865_VTS_H(mode->vts));
+	if (ret)
+		return ret;
+
+	ret = ov8865_write(sensor, OV8865_VTS_L_REG, OV8865_VTS_L(mode->vts));
+	if (ret)
+		return ret;
+
+	if (mode->size_auto) {
+		/* Auto Size */
+
+		ret = ov8865_write(sensor, OV8865_AUTO_SIZE_CTRL_REG,
+				   OV8865_AUTO_SIZE_CTRL_OFFSET_Y_REG |
+				   OV8865_AUTO_SIZE_CTRL_OFFSET_X_REG |
+				   OV8865_AUTO_SIZE_CTRL_CROP_END_Y_REG |
+				   OV8865_AUTO_SIZE_CTRL_CROP_END_X_REG |
+				   OV8865_AUTO_SIZE_CTRL_CROP_START_Y_REG |
+				   OV8865_AUTO_SIZE_CTRL_CROP_START_X_REG);
+		if (ret)
+			return ret;
+
+		ret = ov8865_write(sensor, OV8865_AUTO_SIZE_BOUNDARIES_REG,
+				   OV8865_AUTO_SIZE_BOUNDARIES_Y(mode->size_auto_boundary_y) |
+				   OV8865_AUTO_SIZE_BOUNDARIES_X(mode->size_auto_boundary_x));
+		if (ret)
+			return ret;
+	} else {
+		/* Crop Start X */
+
+		ret = ov8865_write(sensor, OV8865_CROP_START_X_H_REG,
+				   OV8865_CROP_START_X_H(mode->crop_start_x));
+		if (ret)
+			return ret;
+
+		ret = ov8865_write(sensor, OV8865_CROP_START_X_L_REG,
+				   OV8865_CROP_START_X_L(mode->crop_start_x));
+		if (ret)
+			return ret;
+
+		/* Offset X */
+
+		ret = ov8865_write(sensor, OV8865_OFFSET_X_H_REG,
+				   OV8865_OFFSET_X_H(mode->offset_x));
+		if (ret)
+			return ret;
+
+		ret = ov8865_write(sensor, OV8865_OFFSET_X_L_REG,
+				   OV8865_OFFSET_X_L(mode->offset_x));
+		if (ret)
+			return ret;
+
+		/* Crop End X */
+
+		ret = ov8865_write(sensor, OV8865_CROP_END_X_H_REG,
+				   OV8865_CROP_END_X_H(mode->crop_end_x));
+		if (ret)
+			return ret;
+
+		ret = ov8865_write(sensor, OV8865_CROP_END_X_L_REG,
+				   OV8865_CROP_END_X_L(mode->crop_end_x));
+		if (ret)
+			return ret;
+
+		/* Crop Start Y */
+
+		ret = ov8865_write(sensor, OV8865_CROP_START_Y_H_REG,
+				   OV8865_CROP_START_Y_H(mode->crop_start_y));
+		if (ret)
+			return ret;
+
+		ret = ov8865_write(sensor, OV8865_CROP_START_Y_L_REG,
+				   OV8865_CROP_START_Y_L(mode->crop_start_y));
+		if (ret)
+			return ret;
+
+		/* Offset Y */
+
+		ret = ov8865_write(sensor, OV8865_OFFSET_Y_H_REG,
+				   OV8865_OFFSET_Y_H(mode->offset_y));
+		if (ret)
+			return ret;
+
+		ret = ov8865_write(sensor, OV8865_OFFSET_Y_L_REG,
+				   OV8865_OFFSET_Y_L(mode->offset_y));
+		if (ret)
+			return ret;
+
+		/* Crop End Y */
+
+		ret = ov8865_write(sensor, OV8865_CROP_END_Y_H_REG,
+				   OV8865_CROP_END_Y_H(mode->crop_end_y));
+		if (ret)
+			return ret;
+
+		ret = ov8865_write(sensor, OV8865_CROP_END_Y_L_REG,
+				   OV8865_CROP_END_Y_L(mode->crop_end_y));
+		if (ret)
+			return ret;
+	}
+
+	/* VFIFO */
+
+	ret = ov8865_write(sensor, OV8865_VFIFO_READ_START_H_REG,
+			   OV8865_VFIFO_READ_START_H(mode->vfifo_read_start));
+	if (ret)
+		return ret;
+
+	ret = ov8865_write(sensor, OV8865_VFIFO_READ_START_L_REG,
+			   OV8865_VFIFO_READ_START_L(mode->vfifo_read_start));
+	if (ret)
+		return ret;
+
+	ret = ov8865_write(sensor, OV8865_ABLC_NUM_REG,
+			   OV8865_ABLC_NUM(mode->ablc_num));
+	if (ret)
+		return ret;
+
+	ret = ov8865_write(sensor, OV8865_ZLINE_NUM_REG,
+			   OV8865_ZLINE_NUM(mode->zline_num));
+	if (ret)
+		return ret;
+
+	/* Binning */
+
+	ret = ov8865_mode_binning_configure(sensor, mode);
+	if (ret)
+		return ret;
+
+	/* Black Level */
+
+	ret = ov8865_mode_black_level_configure(sensor, mode);
+	if (ret)
+		return ret;
+
+	/* PLLs */
+
+	ret = ov8865_mode_pll1_configure(sensor, mode, mbus_code);
+	if (ret)
+		return ret;
+
+	ret = ov8865_mode_pll2_configure(sensor, mode);
+	if (ret)
+		return ret;
+
+	ret = ov8865_mode_sclk_configure(sensor, mode);
+	if (ret)
+		return ret;
+
+	/* Extra registers */
+
+	if (mode->register_values) {
+		ret = ov8865_write_sequence(sensor, mode->register_values,
+					    mode->register_values_count);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static unsigned long ov8865_mode_mipi_clk_rate(struct ov8865_sensor *sensor,
+					       const struct ov8865_mode *mode)
+{
+	const struct ov8865_pll1_config *config = mode->pll1_config;
+	unsigned long pll1_rate;
+	unsigned long mipi_clk_rate;
+
+	pll1_rate = ov8865_mode_pll1_rate(sensor, mode);
+	mipi_clk_rate = pll1_rate / config->m_div / 2;
+
+	return mipi_clk_rate;
+}
+
+/* Exposure */
+
+static int ov8865_exposure_configure(struct ov8865_sensor *sensor, u32 exposure)
+{
+	int ret;
+
+	ret = ov8865_write(sensor, OV8865_EXPOSURE_CTRL_HH_REG,
+			   OV8865_EXPOSURE_CTRL_HH(exposure));
+	if (ret)
+		return ret;
+
+	ret = ov8865_write(sensor, OV8865_EXPOSURE_CTRL_H_REG,
+			   OV8865_EXPOSURE_CTRL_H(exposure));
+	if (ret)
+		return ret;
+
+	ret = ov8865_write(sensor, OV8865_EXPOSURE_CTRL_L_REG,
+			   OV8865_EXPOSURE_CTRL_L(exposure));
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+/* Gain */
+
+static int ov8865_gain_configure(struct ov8865_sensor *sensor, u32 gain)
+{
+	int ret;
+
+	ret = ov8865_write(sensor, OV8865_GAIN_CTRL_H_REG,
+			   OV8865_GAIN_CTRL_H(gain));
+	if (ret)
+		return ret;
+
+	ret = ov8865_write(sensor, OV8865_GAIN_CTRL_L_REG,
+			   OV8865_GAIN_CTRL_L(gain));
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+/* White Balance */
+
+static int ov8865_red_balance_configure(struct ov8865_sensor *sensor,
+					u32 red_balance)
+{
+	int ret;
+
+	ret = ov8865_write(sensor, OV8865_ISP_GAIN_RED_H_REG,
+			   OV8865_ISP_GAIN_RED_H(red_balance));
+	if (ret)
+		return ret;
+
+	ret = ov8865_write(sensor, OV8865_ISP_GAIN_RED_L_REG,
+			   OV8865_ISP_GAIN_RED_L(red_balance));
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int ov8865_blue_balance_configure(struct ov8865_sensor *sensor,
+					 u32 blue_balance)
+{
+	int ret;
+
+	ret = ov8865_write(sensor, OV8865_ISP_GAIN_BLUE_H_REG,
+			   OV8865_ISP_GAIN_BLUE_H(blue_balance));
+	if (ret)
+		return ret;
+
+	ret = ov8865_write(sensor, OV8865_ISP_GAIN_BLUE_L_REG,
+			   OV8865_ISP_GAIN_BLUE_L(blue_balance));
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+/* Flip */
+
+static int ov8865_flip_vert_configure(struct ov8865_sensor *sensor, bool enable)
+{
+	u8 bits = OV8865_FORMAT1_FLIP_VERT_ISP_EN |
+		  OV8865_FORMAT1_FLIP_VERT_SENSOR_EN;
+	int ret;
+
+	ret = ov8865_update_bits(sensor, OV8865_FORMAT1_REG, bits,
+				 enable ? bits : 0);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int ov8865_flip_horz_configure(struct ov8865_sensor *sensor, bool enable)
+{
+	u8 bits = OV8865_FORMAT2_FLIP_HORZ_ISP_EN |
+		  OV8865_FORMAT2_FLIP_HORZ_SENSOR_EN;
+	int ret;
+
+	ret = ov8865_update_bits(sensor, OV8865_FORMAT2_REG, bits,
+				 enable ? bits : 0);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+/* Test Pattern */
+
+static int ov8865_test_pattern_configure(struct ov8865_sensor *sensor, u8 bits)
+{
+	int ret;
+
+	ret = ov8865_write(sensor, OV8865_PRE_CTRL0_REG, bits);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+/* State */
+
+static int ov8865_state_mipi_configure(struct ov8865_sensor *sensor,
+				       const struct ov8865_mode *mode,
+				       u32 mbus_code)
+{
+	struct ov8865_ctrls *ctrls = &sensor->ctrls;
+	struct v4l2_fwnode_bus_mipi_csi2 *bus_mipi_csi2 =
+		&sensor->endpoint.bus.mipi_csi2;
+	unsigned long mipi_clk_rate;
+	unsigned int bits_per_sample;
+	unsigned int lanes_count;
+	unsigned int i;
+	s64 mipi_pixel_rate;
+
+	mipi_clk_rate = ov8865_mode_mipi_clk_rate(sensor, mode);
+	if (!mipi_clk_rate)
+		return -EINVAL;
+
+	for (i = 0; i < ARRAY_SIZE(ov8865_link_freq_menu); i++) {
+		s64 freq = ov8865_link_freq_menu[i];
+
+		if (freq == mipi_clk_rate)
+			break;
+	}
+
+	if (i == ARRAY_SIZE(ov8865_link_freq_menu)) {
+		dev_err(sensor->dev,
+			"failed to find %lu clk rate in link freq\n",
+			mipi_clk_rate);
+	} else {
+		__v4l2_ctrl_s_ctrl(ctrls->link_freq, i);
+	}
+
+	switch (mbus_code) {
+	case MEDIA_BUS_FMT_SBGGR10_1X10:
+		bits_per_sample = 10;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	lanes_count = bus_mipi_csi2->num_data_lanes;
+	mipi_pixel_rate = mipi_clk_rate * 2 * lanes_count / bits_per_sample;
+
+	__v4l2_ctrl_s_ctrl_int64(ctrls->pixel_rate, mipi_pixel_rate);
+
+	return 0;
+}
+
+static int ov8865_state_configure(struct ov8865_sensor *sensor,
+				  const struct ov8865_mode *mode,
+				  u32 mbus_code)
+{
+	int ret;
+
+	if (sensor->state.streaming)
+		return -EBUSY;
+
+	/* State will be configured at first power on otherwise. */
+	if (sensor->state.power_count > 0) {
+		ret = ov8865_mode_configure(sensor, mode, mbus_code);
+		if (ret)
+			return ret;
+	}
+
+	ret = ov8865_state_mipi_configure(sensor, mode, mbus_code);
+	if (ret)
+		return ret;
+
+	sensor->state.mode = mode;
+	sensor->state.mbus_code = mbus_code;
+
+	return 0;
+}
+
+static int ov8865_state_init(struct ov8865_sensor *sensor)
+{
+	return ov8865_state_configure(sensor, ov8865_modes[0],
+				      ov8865_mbus_codes[0]);
+}
+
+/* Sensor Base */
+
+static int ov8865_sensor_init(struct ov8865_sensor *sensor)
+{
+	int ret;
+
+	ret = ov8865_sw_reset(sensor);
+	if (ret) {
+		dev_err(sensor->dev, "failed to perform sw reset\n");
+		return ret;
+	}
+
+	ret = ov8865_sw_standby(sensor, 1);
+	if (ret) {
+		dev_err(sensor->dev, "failed to set sensor standby\n");
+		return ret;
+	}
+
+	ret = ov8865_chip_id_check(sensor);
+	if (ret) {
+		dev_err(sensor->dev, "failed to check sensor chip id\n");
+		return ret;
+	}
+
+	ret = ov8865_write_sequence(sensor, ov8865_init_sequence,
+				    ARRAY_SIZE(ov8865_init_sequence));
+	if (ret) {
+		dev_err(sensor->dev, "failed to write init sequence\n");
+		return ret;
+	}
+
+	ret = ov8865_charge_pump_configure(sensor);
+	if (ret) {
+		dev_err(sensor->dev, "failed to configure pad\n");
+		return ret;
+	}
+
+	ret = ov8865_mipi_configure(sensor);
+	if (ret) {
+		dev_err(sensor->dev, "failed to configure MIPI\n");
+		return ret;
+	}
+
+	ret = ov8865_isp_configure(sensor);
+	if (ret) {
+		dev_err(sensor->dev, "failed to configure ISP\n");
+		return ret;
+	}
+
+	ret = ov8865_black_level_configure(sensor);
+	if (ret) {
+		dev_err(sensor->dev, "failed to configure black level\n");
+		return ret;
+	}
+
+	/* Configure current mode. */
+	ret = ov8865_state_configure(sensor, sensor->state.mode,
+				     sensor->state.mbus_code);
+	if (ret) {
+		dev_err(sensor->dev, "failed to configure state\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static int ov8865_sensor_power(struct ov8865_sensor *sensor, bool on)
+{
+	/* Keep initialized to zero for disable label. */
+	int ret = 0;
+
+	if (on) {
+		gpiod_set_value_cansleep(sensor->reset, 1);
+		gpiod_set_value_cansleep(sensor->powerdown, 1);
+
+		ret = regulator_enable(sensor->dovdd);
+		if (ret) {
+			dev_err(sensor->dev,
+				"failed to enable DOVDD regulator\n");
+			goto disable;
+		}
+
+		ret = regulator_enable(sensor->avdd);
+		if (ret) {
+			dev_err(sensor->dev,
+				"failed to enable AVDD regulator\n");
+			goto disable;
+		}
+
+		ret = regulator_enable(sensor->dvdd);
+		if (ret) {
+			dev_err(sensor->dev,
+				"failed to enable DVDD regulator\n");
+			goto disable;
+		}
+
+		ret = clk_prepare_enable(sensor->extclk);
+		if (ret) {
+			dev_err(sensor->dev, "failed to enable EXTCLK clock\n");
+			goto disable;
+		}
+
+		gpiod_set_value_cansleep(sensor->reset, 0);
+		gpiod_set_value_cansleep(sensor->powerdown, 0);
+
+		/* Time to enter streaming mode according to power timings. */
+		usleep_range(10000, 12000);
+	} else {
+disable:
+		gpiod_set_value_cansleep(sensor->powerdown, 1);
+		gpiod_set_value_cansleep(sensor->reset, 1);
+
+		clk_disable_unprepare(sensor->extclk);
+
+		regulator_disable(sensor->dvdd);
+		regulator_disable(sensor->avdd);
+		regulator_disable(sensor->dovdd);
+	}
+
+	return ret;
+}
+
+/* Controls */
+
+static int ov8865_s_ctrl(struct v4l2_ctrl *ctrl)
+{
+	struct v4l2_subdev *subdev = ov8865_ctrl_subdev(ctrl);
+	struct ov8865_sensor *sensor = ov8865_subdev_sensor(subdev);
+	struct ov8865_state *state = &sensor->state;
+	u8 bits;
+	int ret;
+
+	/* Wait for the sensor to be on before setting controls. */
+	if (state->power_count == 0)
+		return 0;
+
+	switch (ctrl->id) {
+	case V4L2_CID_EXPOSURE:
+		ret = ov8865_exposure_configure(sensor, ctrl->val);
+		if (ret)
+			return ret;
+		break;
+	case V4L2_CID_GAIN:
+		ret = ov8865_gain_configure(sensor, ctrl->val);
+		if (ret)
+			return ret;
+		break;
+	case V4L2_CID_RED_BALANCE:
+		return ov8865_red_balance_configure(sensor, ctrl->val);
+	case V4L2_CID_BLUE_BALANCE:
+		return ov8865_blue_balance_configure(sensor, ctrl->val);
+	case V4L2_CID_HFLIP:
+		return ov8865_flip_horz_configure(sensor, !!ctrl->val);
+	case V4L2_CID_VFLIP:
+		return ov8865_flip_vert_configure(sensor, !!ctrl->val);
+	case V4L2_CID_TEST_PATTERN:
+		bits = ov8865_test_pattern_bits[ctrl->val];
+		return ov8865_test_pattern_configure(sensor, bits);
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static const struct v4l2_ctrl_ops ov8865_ctrl_ops = {
+	.s_ctrl			= ov8865_s_ctrl,
+};
+
+static int ov8865_ctrls_init(struct ov8865_sensor *sensor)
+{
+	struct ov8865_ctrls *ctrls = &sensor->ctrls;
+	struct v4l2_ctrl_handler *handler = &ctrls->handler;
+	const struct v4l2_ctrl_ops *ops = &ov8865_ctrl_ops;
+	int ret;
+
+	v4l2_ctrl_handler_init(handler, 32);
+
+	/* Use our mutex for ctrl locking. */
+	handler->lock = &sensor->mutex;
+
+	/* Exposure */
+
+	ctrls->exposure = v4l2_ctrl_new_std(handler, ops, V4L2_CID_EXPOSURE,
+					    16, 1048575, 16, 512);
+
+	/* Gain */
+
+	ctrls->gain = v4l2_ctrl_new_std(handler, ops, V4L2_CID_GAIN, 128, 8191,
+					128, 128);
+
+	/* White Balance */
+
+	ctrls->red_balance = v4l2_ctrl_new_std(handler, ops,
+					       V4L2_CID_RED_BALANCE, 1, 32767,
+					       1, 1024);
+
+	ctrls->blue_balance = v4l2_ctrl_new_std(handler, ops,
+						V4L2_CID_BLUE_BALANCE, 1, 32767,
+						1, 1024);
+
+	/* Flip */
+
+	ctrls->flip_horz = v4l2_ctrl_new_std(handler, ops, V4L2_CID_HFLIP, 0, 1,
+					     1, 0);
+	ctrls->flip_vert = v4l2_ctrl_new_std(handler, ops, V4L2_CID_VFLIP, 0, 1,
+					     1, 0);
+
+	/* Test Pattern */
+
+	ctrls->test_pattern =
+		v4l2_ctrl_new_std_menu_items(handler, ops,
+					     V4L2_CID_TEST_PATTERN,
+					     ARRAY_SIZE(ov8865_test_pattern_menu) - 1,
+					     0, 0, ov8865_test_pattern_menu);
+
+	/* MIPI CSI-2 */
+
+	ctrls->link_freq =
+		v4l2_ctrl_new_int_menu(handler, ops, V4L2_CID_LINK_FREQ,
+				       ARRAY_SIZE(ov8865_link_freq_menu) - 1,
+				       0, ov8865_link_freq_menu);
+
+	ctrls->link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY;
+
+	ctrls->pixel_rate = v4l2_ctrl_new_std(handler, ops, V4L2_CID_PIXEL_RATE,
+					      1, INT_MAX, 1, 1);
+
+	ctrls->pixel_rate->flags |= V4L2_CTRL_FLAG_READ_ONLY;
+
+	if (handler->error) {
+		ret = handler->error;
+		goto error_ctrls;
+	}
+
+	sensor->subdev.ctrl_handler = handler;
+
+	return 0;
+
+error_ctrls:
+	v4l2_ctrl_handler_free(handler);
+
+	return ret;
+}
+
+/* Subdev Core Operations */
+
+static int ov8865_s_power(struct v4l2_subdev *subdev, int on)
+{
+	struct ov8865_sensor *sensor = ov8865_subdev_sensor(subdev);
+	struct ov8865_state *state = &sensor->state;
+	bool ctrl_handler_setup = false;
+	int ret = 0;
+
+	mutex_lock(&sensor->mutex);
+
+	state->power_count += on ? 1 : -1;
+
+	/* Neither first power on nor last power off. */
+	if (state->power_count != !!on)
+		goto complete;
+
+	ret = ov8865_sensor_power(sensor, on);
+	if (ret)
+		goto complete;
+
+	/* First power on: init sensor and set controls. */
+	if (on) {
+		ret = ov8865_sensor_init(sensor);
+		if (ret)
+			goto error_power;
+
+		ctrl_handler_setup = true;
+	}
+
+	goto complete;
+
+error_power:
+	ov8865_sensor_power(sensor, !on);
+
+complete:
+	mutex_unlock(&sensor->mutex);
+
+	if (ctrl_handler_setup)
+		ret = v4l2_ctrl_handler_setup(&sensor->ctrls.handler);
+
+	return ret;
+}
+
+static const struct v4l2_subdev_core_ops ov8865_subdev_core_ops = {
+	.s_power		= ov8865_s_power,
+};
+
+/* Subdev Video Operations */
+
+static int ov8865_s_stream(struct v4l2_subdev *subdev, int enable)
+{
+	struct ov8865_sensor *sensor = ov8865_subdev_sensor(subdev);
+	int ret = 0;
+
+	mutex_lock(&sensor->mutex);
+
+	ret = ov8865_sw_standby(sensor, !enable);
+	if (ret)
+		goto complete;
+
+	sensor->state.streaming = !!enable;
+
+complete:
+	mutex_unlock(&sensor->mutex);
+
+	return ret;
+}
+
+static int ov8865_g_frame_interval(struct v4l2_subdev *subdev,
+				   struct v4l2_subdev_frame_interval *interval)
+{
+	struct ov8865_sensor *sensor = ov8865_subdev_sensor(subdev);
+	const struct ov8865_mode *mode;
+	int ret = 0;
+
+	mutex_lock(&sensor->mutex);
+
+	mode = sensor->state.mode;
+	interval->interval = mode->frame_interval;
+
+	mutex_unlock(&sensor->mutex);
+
+	return ret;
+}
+
+static const struct v4l2_subdev_video_ops ov8865_subdev_video_ops = {
+	.s_stream		= ov8865_s_stream,
+	.g_frame_interval	= ov8865_g_frame_interval,
+	.s_frame_interval	= ov8865_g_frame_interval,
+};
+
+/* Subdev Pad Operations */
+
+static int ov8865_enum_mbus_code(struct v4l2_subdev *subdev,
+				 struct v4l2_subdev_pad_config *config,
+				 struct v4l2_subdev_mbus_code_enum *code_enum)
+{
+	if (code_enum->index >= ARRAY_SIZE(ov8865_mbus_codes))
+		return -EINVAL;
+
+	code_enum->code = ov8865_mbus_codes[code_enum->index];
+
+	return 0;
+}
+
+static void ov8865_mbus_format_fill(struct v4l2_mbus_framefmt *mbus_format,
+				    u32 mbus_code,
+				    const struct ov8865_mode *mode)
+{
+	mbus_format->width = mode->output_size_x;
+	mbus_format->height = mode->output_size_y;
+	mbus_format->code = mbus_code;
+
+	mbus_format->field = V4L2_FIELD_NONE;
+	mbus_format->colorspace = V4L2_COLORSPACE_RAW;
+	mbus_format->ycbcr_enc =
+		V4L2_MAP_YCBCR_ENC_DEFAULT(mbus_format->colorspace);
+	mbus_format->quantization = V4L2_QUANTIZATION_FULL_RANGE;
+	mbus_format->xfer_func =
+		V4L2_MAP_XFER_FUNC_DEFAULT(mbus_format->colorspace);
+}
+
+static int ov8865_get_fmt(struct v4l2_subdev *subdev,
+			  struct v4l2_subdev_pad_config *cfg,
+			  struct v4l2_subdev_format *format)
+{
+	struct ov8865_sensor *sensor = ov8865_subdev_sensor(subdev);
+	struct v4l2_mbus_framefmt *mbus_format = &format->format;
+
+	mutex_lock(&sensor->mutex);
+
+	if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
+		*mbus_format =
+			*v4l2_subdev_get_try_format(subdev, cfg, format->pad);
+	} else {
+		ov8865_mbus_format_fill(mbus_format, sensor->state.mbus_code,
+					sensor->state.mode);
+	}
+
+	mutex_unlock(&sensor->mutex);
+
+	return 0;
+}
+
+static int ov8865_set_fmt(struct v4l2_subdev *subdev,
+			  struct v4l2_subdev_pad_config *cfg,
+			  struct v4l2_subdev_format *format)
+{
+	struct ov8865_sensor *sensor = ov8865_subdev_sensor(subdev);
+	struct v4l2_mbus_framefmt *mbus_format = &format->format;
+	const struct ov8865_mode *mode = ov8865_modes[0];
+	u32 mbus_code = ov8865_mbus_codes[0];
+	unsigned int index;
+	int ret = 0;
+
+	mutex_lock(&sensor->mutex);
+
+	if (sensor->state.streaming) {
+		ret = -EBUSY;
+		goto complete;
+	}
+
+	/* Try to find requested mbus code. */
+	for (index = 0; index < ARRAY_SIZE(ov8865_mbus_codes); index++) {
+		if (ov8865_mbus_codes[index] == mbus_format->code) {
+			mbus_code = mbus_format->code;
+			break;
+		}
+	}
+
+	/* Try to find requested mode code. */
+	for (index = 0; index < ARRAY_SIZE(ov8865_modes); index++) {
+		if (ov8865_modes[index]->output_size_x == mbus_format->width &&
+		    ov8865_modes[index]->output_size_y == mbus_format->height) {
+			mode = ov8865_modes[index];
+			break;
+		}
+	}
+
+	ov8865_mbus_format_fill(mbus_format, mbus_code, mode);
+
+	if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
+		*v4l2_subdev_get_try_format(subdev, cfg, format->pad) =
+			*mbus_format;
+	} else if (sensor->state.mode != mode ||
+		   sensor->state.mbus_code != mbus_code) {
+		ret = ov8865_state_configure(sensor, mode, mbus_code);
+		if (ret)
+			goto complete;
+	}
+
+complete:
+	mutex_unlock(&sensor->mutex);
+
+	return ret;
+}
+
+static int ov8865_enum_frame_size(struct v4l2_subdev *subdev,
+				  struct v4l2_subdev_pad_config *config,
+				  struct v4l2_subdev_frame_size_enum *size_enum)
+{
+	const struct ov8865_mode *mode;
+
+	if (size_enum->index >= ARRAY_SIZE(ov8865_modes))
+		return -EINVAL;
+
+	mode = ov8865_modes[size_enum->index];
+
+	size_enum->min_width = size_enum->max_width = mode->output_size_x;
+	size_enum->min_height = size_enum->max_height = mode->output_size_y;
+
+	return 0;
+}
+
+static int ov8865_enum_frame_interval(struct v4l2_subdev *subdev,
+				      struct v4l2_subdev_pad_config *config,
+				      struct v4l2_subdev_frame_interval_enum *interval_enum)
+{
+	const struct ov8865_mode *mode = NULL;
+	unsigned int mode_index;
+	unsigned int interval_index;
+
+	if (interval_enum->index > 0)
+		return -EINVAL;
+	/*
+	 * Multiple modes with the same dimensions may have different frame
+	 * intervals, so look up each relevant mode.
+	 */
+	for (mode_index = 0, interval_index = 0;
+	     mode_index < ARRAY_SIZE(ov8865_modes); mode_index++) {
+		mode = ov8865_modes[mode_index];
+
+		if (mode->output_size_x == interval_enum->width &&
+		    mode->output_size_y == interval_enum->height) {
+			if (interval_index == interval_enum->index)
+				break;
+
+			interval_index++;
+		}
+	}
+
+	if (mode_index == ARRAY_SIZE(ov8865_modes) || !mode)
+		return -EINVAL;
+
+	interval_enum->interval = mode->frame_interval;
+
+	return 0;
+}
+
+static const struct v4l2_subdev_pad_ops ov8865_subdev_pad_ops = {
+	.enum_mbus_code		= ov8865_enum_mbus_code,
+	.get_fmt		= ov8865_get_fmt,
+	.set_fmt		= ov8865_set_fmt,
+	.enum_frame_size	= ov8865_enum_frame_size,
+	.enum_frame_interval	= ov8865_enum_frame_interval,
+};
+
+static const struct v4l2_subdev_ops ov8865_subdev_ops = {
+	.core		= &ov8865_subdev_core_ops,
+	.video		= &ov8865_subdev_video_ops,
+	.pad		= &ov8865_subdev_pad_ops,
+};
+
+static int ov8865_probe(struct i2c_client *client)
+{
+	struct device *dev = &client->dev;
+	struct fwnode_handle *handle;
+	struct ov8865_sensor *sensor;
+	struct v4l2_subdev *subdev;
+	struct media_pad *pad;
+	int ret;
+
+	sensor = devm_kzalloc(dev, sizeof(*sensor), GFP_KERNEL);
+	if (!sensor)
+		return -ENOMEM;
+
+	sensor->dev = dev;
+	sensor->i2c_client = client;
+
+	/* Graph Endpoint */
+
+	handle = fwnode_graph_get_next_endpoint(dev_fwnode(dev), NULL);
+	if (!handle) {
+		dev_err(dev, "unable to find enpoint node\n");
+		return -EINVAL;
+	}
+
+	sensor->endpoint.bus_type = V4L2_MBUS_CSI2_DPHY;
+
+	ret = v4l2_fwnode_endpoint_parse(handle, &sensor->endpoint);
+	fwnode_handle_put(handle);
+	if (ret) {
+		dev_err(dev, "failed to parse endpoint node\n");
+		return ret;
+	}
+
+	/* GPIOs */
+
+	sensor->powerdown = devm_gpiod_get_optional(dev, "powerdown",
+						    GPIOD_OUT_HIGH);
+	if (IS_ERR(sensor->powerdown))
+		return PTR_ERR(sensor->powerdown);
+
+	sensor->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(sensor->reset))
+		return PTR_ERR(sensor->reset);
+
+	/* Regulators */
+
+	/* DVDD: digital core */
+	sensor->dvdd = devm_regulator_get(dev, "dvdd");
+	if (IS_ERR(sensor->dvdd)) {
+		dev_err(dev, "cannot get DVDD (digital core) regulator\n");
+		return PTR_ERR(sensor->dvdd);
+	}
+
+	/* DOVDD: digital I/O */
+	sensor->dovdd = devm_regulator_get(dev, "dovdd");
+	if (IS_ERR(sensor->dvdd)) {
+		dev_err(dev, "cannot get DOVDD (digital I/O) regulator\n");
+		return PTR_ERR(sensor->dvdd);
+	}
+
+	/* AVDD: analog */
+	sensor->avdd = devm_regulator_get(dev, "avdd");
+	if (IS_ERR(sensor->avdd)) {
+		dev_err(dev, "cannot get AVDD (analog) regulator\n");
+		return PTR_ERR(sensor->dvdd);
+	}
+
+	/* External Clock */
+
+	sensor->extclk = devm_clk_get(dev, "extclk");
+	if (IS_ERR(sensor->extclk)) {
+		dev_err(dev, "failed to get extclk external clock\n");
+		return PTR_ERR(sensor->extclk);
+	}
+
+	ret = clk_set_rate_exclusive(sensor->extclk, OV8865_EXTCLK_RATE);
+	if (ret) {
+		dev_err(dev, "failed to set extclk external clock rate\n");
+		return ret;
+	}
+
+	/* Subdev, entity and pad */
+
+	subdev = &sensor->subdev;
+	v4l2_i2c_subdev_init(subdev, client, &ov8865_subdev_ops);
+
+	subdev->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
+	subdev->entity.function = MEDIA_ENT_F_CAM_SENSOR;
+
+	pad = &sensor->pad;
+	pad->flags = MEDIA_PAD_FL_SOURCE;
+
+	ret = media_entity_pads_init(&subdev->entity, 1, pad);
+	if (ret)
+		goto error_entity;
+
+	/* Mutex */
+
+	mutex_init(&sensor->mutex);
+
+	/* Sensor */
+
+	ret = ov8865_ctrls_init(sensor);
+	if (ret)
+		goto error_mutex;
+
+	ret = ov8865_state_init(sensor);
+	if (ret)
+		goto error_ctrls;
+
+	/* V4L2 subdev register */
+
+	ret = v4l2_async_register_subdev_sensor_common(subdev);
+	if (ret)
+		goto error_ctrls;
+
+	return 0;
+
+error_ctrls:
+	v4l2_ctrl_handler_free(&sensor->ctrls.handler);
+
+error_mutex:
+	mutex_destroy(&sensor->mutex);
+
+error_entity:
+	media_entity_cleanup(&sensor->subdev.entity);
+
+	return ret;
+}
+
+static int ov8865_remove(struct i2c_client *client)
+{
+	struct v4l2_subdev *subdev = i2c_get_clientdata(client);
+	struct ov8865_sensor *sensor = ov8865_subdev_sensor(subdev);
+
+	clk_rate_exclusive_put(sensor->extclk);
+
+	v4l2_async_unregister_subdev(subdev);
+	mutex_destroy(&sensor->mutex);
+	media_entity_cleanup(&subdev->entity);
+	v4l2_device_unregister_subdev(subdev);
+
+	return 0;
+}
+
+static const struct i2c_device_id ov8865_id[] = {
+	{ "ov8865", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, ov8865_id);
+
+static const struct of_device_id ov8865_of_match[] = {
+	{ .compatible = "ovti,ov8865" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ov8865_of_match);
+
+static struct i2c_driver ov8865_driver = {
+	.driver = {
+		.of_match_table = ov8865_of_match,
+		.name	= "ov8865",
+	},
+	.probe_new = ov8865_probe,
+	.remove	 = ov8865_remove,
+	.id_table = ov8865_id,
+};
+
+module_i2c_driver(ov8865_driver);
+
+MODULE_AUTHOR("Paul Kocialkowski <paul.kocialkowski@bootlin.com>");
+MODULE_DESCRIPTION("V4L2 driver for the OmniVision OV8865 image sensor");
+MODULE_LICENSE("GPL v2");
-- 
2.28.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH NOT FOR MERGE 3/3] ARM: dts: sun8i: a83t: bananapi-m3: Enable MIPI CSI-2 with OV8865
  2020-10-23 17:54 [PATCH 0/3] media: i2c: OV8865 image sensor support Paul Kocialkowski
  2020-10-23 17:54 ` [PATCH 1/3] dt-bindings: media: i2c: Add OV8865 bindings documentation Paul Kocialkowski
  2020-10-23 17:54 ` [PATCH 2/3] media: i2c: Add support for the OV8865 image sensor Paul Kocialkowski
@ 2020-10-23 17:54 ` Paul Kocialkowski
  2 siblings, 0 replies; 12+ messages in thread
From: Paul Kocialkowski @ 2020-10-23 17:54 UTC (permalink / raw)
  To: linux-media, devicetree, linux-kernel
  Cc: Mauro Carvalho Chehab, Rob Herring, Liam Girdwood, Mark Brown,
	Paul Kocialkowski, Thomas Petazzoni, Hans Verkuil, Sakari Ailus,
	Maxime Ripard, kevin.lhopital, Kévin L'hôpital

From: Kévin L'hôpital <kevin.lhopital@bootlin.com>

The Bananapi M3 supports a camera module which includes an OV8865 sensor
connected via the parallel CSI interface and an OV8865 sensor connected
via MIPI CSI-2.

The I2C2 bus is shared by the two sensors as well as the (active-low)
reset signal, but each sensor has it own shutdown line.

Signed-off-by: Kévin L'hôpital <kevin.lhopital@bootlin.com>
Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts | 98 ++++++++++++++++++++
 1 file changed, 98 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts b/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts
index 9d34eabba121..70d305d47d6d 100644
--- a/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts
+++ b/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts
@@ -85,6 +85,30 @@ green {
 		};
 	};
 
+	reg_ov8865_avdd: ov8865-avdd {
+		compatible = "regulator-fixed";
+		regulator-name = "ov8865-avdd";
+		regulator-min-microvolt = <2800000>;
+		regulator-max-microvolt = <2800000>;
+		vin-supply = <&reg_dldo4>;
+	};
+
+	reg_ov8865_dovdd: ov8865-dovdd {
+		compatible = "regulator-fixed";
+		regulator-name = "ov8865-dovdd";
+		regulator-min-microvolt = <2800000>;
+		regulator-max-microvolt = <2800000>;
+		vin-supply = <&reg_dldo4>;
+	};
+
+	reg_ov8865_dvdd: ov8865-dvdd {
+		compatible = "regulator-fixed";
+		regulator-name = "ov8865-dvdd";
+		regulator-min-microvolt = <1200000>;
+		regulator-max-microvolt = <1200000>;
+		vin-supply = <&reg_eldo1>;
+	};
+
 	reg_usb1_vbus: reg-usb1-vbus {
 		compatible = "regulator-fixed";
 		regulator-name = "usb1-vbus";
@@ -115,6 +139,17 @@ &cpu100 {
 	cpu-supply = <&reg_dcdc3>;
 };
 
+&csi {
+	status = "okay";
+};
+
+&csi_in {
+	csi_in_mipi_csi2: endpoint {
+		bus-type = <4>; /* CSI2_DPHY */
+		remote-endpoint = <&mipi_csi2_out_csi>;
+	};
+};
+
 &de {
 	status = "okay";
 };
@@ -147,6 +182,36 @@ hdmi_out_con: endpoint {
 	};
 };
 
+&i2c2 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&i2c2_pe_pins>;
+	status = "okay";
+
+	ov8865: camera@36 {
+		compatible = "ovti,ov8865";
+		reg = <0x36>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&csi_mclk_pin>;
+		clocks = <&ccu CLK_CSI_MCLK>;
+		clock-names = "extclk";
+		avdd-supply = <&reg_ov8865_avdd>;
+		dovdd-supply = <&reg_ov8865_dovdd>;
+		dvdd-supply = <&reg_ov8865_dvdd>;
+		powerdown-gpios = <&pio 4 17 GPIO_ACTIVE_LOW>; /* PE17 */
+		reset-gpios = <&pio 4 16 GPIO_ACTIVE_LOW>; /* PE16 */
+
+		port {
+			ov8865_out_mipi_csi2: endpoint {
+				bus-type = <4>; /* MIPI CSI-2 D-PHY */
+				clock-lanes = <0>;
+				data-lanes = <1 2 3 4>;
+
+				remote-endpoint = <&mipi_csi2_in_ov8865>;
+			};
+		};
+	};
+};
+
 &mdio {
 	rgmii_phy: ethernet-phy@1 {
 		compatible = "ethernet-phy-ieee802.3-c22";
@@ -154,6 +219,26 @@ rgmii_phy: ethernet-phy@1 {
 	};
 };
 
+&mipi_csi2 {
+	status = "okay";
+};
+
+&mipi_csi2_in {
+	mipi_csi2_in_ov8865: endpoint {
+		bus-type = <4>; /* MIPI CSI-2 D-PHY */
+		clock-lanes = <0>;
+		data-lanes = <1 2 3 4>;
+
+		remote-endpoint = <&ov8865_out_mipi_csi2>;
+	};
+};
+
+&mipi_csi2_out {
+	mipi_csi2_out_csi: endpoint {
+		remote-endpoint = <&csi_in_mipi_csi2>;
+	};
+};
+
 &mmc0 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&mmc0_pins>;
@@ -327,11 +412,24 @@ &reg_dldo3 {
 	regulator-name = "vcc-pd";
 };
 
+&reg_dldo4 {
+	regulator-always-on;
+	regulator-min-microvolt = <2800000>;
+	regulator-max-microvolt = <2800000>;
+	regulator-name = "avdd-csi";
+};
+
 &reg_drivevbus {
 	regulator-name = "usb0-vbus";
 	status = "okay";
 };
 
+&reg_eldo1 {
+	regulator-min-microvolt = <1200000>;
+	regulator-max-microvolt = <1200000>;
+	regulator-name = "dvdd-csi-r";
+};
+
 &reg_fldo1 {
 	regulator-min-microvolt = <1080000>;
 	regulator-max-microvolt = <1320000>;
-- 
2.28.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] dt-bindings: media: i2c: Add OV8865 bindings documentation
  2020-10-23 17:54 ` [PATCH 1/3] dt-bindings: media: i2c: Add OV8865 bindings documentation Paul Kocialkowski
@ 2020-10-30 16:39   ` Rob Herring
  2020-11-02 23:24   ` Sakari Ailus
  1 sibling, 0 replies; 12+ messages in thread
From: Rob Herring @ 2020-10-30 16:39 UTC (permalink / raw)
  To: Paul Kocialkowski
  Cc: linux-media, devicetree, linux-kernel, Mauro Carvalho Chehab,
	Liam Girdwood, Mark Brown, Thomas Petazzoni, Hans Verkuil,
	Sakari Ailus, Maxime Ripard, kevin.lhopital,
	Kévin L'hôpital

On Fri, Oct 23, 2020 at 07:54:04PM +0200, Paul Kocialkowski wrote:
> This introduces YAML bindings documentation for the OV8865
> image sensor.
> 
> Co-developed-by: Kévin L'hôpital <kevin.lhopital@bootlin.com>
> Signed-off-by: Kévin L'hôpital <kevin.lhopital@bootlin.com>
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> ---
>  .../bindings/media/i2c/ovti,ov8865.yaml       | 124 ++++++++++++++++++
>  1 file changed, 124 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml
> new file mode 100644
> index 000000000000..807f1a94afae
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml
> @@ -0,0 +1,124 @@
> +# SPDX-License-Identifier: GPL-2.0

Dual license please. With that,

Reviewed-by: Rob Herring <robh@kernel.org>

> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/i2c/ovti,ov8865.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: OmniVision OV8865 Image Sensor Device Tree Bindings
> +
> +maintainers:
> +  - Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> +
> +properties:
> +  compatible:
> +    const: ovti,ov8865
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    items:
> +      - description: EXTCLK Clock
> +
> +  clock-names:
> +    items:
> +      - const: extclk
> +
> +  dvdd-supply:
> +    description: Digital Domain Power Supply
> +
> +  avdd-supply:
> +    description: Analog Domain Power Supply (internal AVDD is used if missing)
> +
> +  dovdd-supply:
> +    description: I/O Domain Power Supply
> +
> +  powerdown-gpios:
> +    maxItems: 1
> +    description: Power Down Pin GPIO Control (active low)
> +
> +  reset-gpios:
> +    maxItems: 1
> +    description: Reset Pin GPIO Control (active low)
> +
> +  port:
> +    type: object
> +    description: Input port, connect to a MIPI CSI-2 receiver
> +
> +    properties:
> +      endpoint:
> +        type: object
> +
> +        properties:
> +          remote-endpoint: true
> +
> +          bus-type:
> +            const: 4
> +
> +          clock-lanes:
> +            maxItems: 1
> +
> +          data-lanes:
> +            minItems: 1
> +            maxItems: 4
> +
> +        required:
> +          - bus-type
> +          - data-lanes
> +          - remote-endpoint
> +
> +        additionalProperties: false
> +
> +    required:
> +      - endpoint
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - dvdd-supply
> +  - dovdd-supply
> +  - port
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/sun8i-a83t-ccu.h>
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +    i2c2 {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        ov8865: camera@36 {
> +            compatible = "ovti,ov8865";
> +            reg = <0x36>;
> +
> +            pinctrl-names = "default";
> +            pinctrl-0 = <&csi_mclk_pin>;
> +
> +            clocks = <&ccu CLK_CSI_MCLK>;
> +            clock-names = "extclk";
> +
> +            avdd-supply = <&reg_ov8865_avdd>;
> +            dovdd-supply = <&reg_ov8865_dovdd>;
> +            dvdd-supply = <&reg_ov8865_dvdd>;
> +
> +            powerdown-gpios = <&pio 4 17 GPIO_ACTIVE_LOW>; /* PE17 */
> +            reset-gpios = <&pio 4 16 GPIO_ACTIVE_LOW>; /* PE16 */
> +
> +            port {
> +                ov8865_out_mipi_csi2: endpoint {
> +                    bus-type = <4>; /* MIPI CSI-2 D-PHY */
> +                    clock-lanes = <0>;
> +                    data-lanes = <1 2 3 4>;
> +
> +                    remote-endpoint = <&mipi_csi2_in_ov8865>;
> +                };
> +            };
> +        };
> +    };
> +
> +...
> -- 
> 2.28.0
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] dt-bindings: media: i2c: Add OV8865 bindings documentation
  2020-10-23 17:54 ` [PATCH 1/3] dt-bindings: media: i2c: Add OV8865 bindings documentation Paul Kocialkowski
  2020-10-30 16:39   ` Rob Herring
@ 2020-11-02 23:24   ` Sakari Ailus
  2020-11-04 10:26     ` Paul Kocialkowski
  1 sibling, 1 reply; 12+ messages in thread
From: Sakari Ailus @ 2020-11-02 23:24 UTC (permalink / raw)
  To: Paul Kocialkowski
  Cc: linux-media, devicetree, linux-kernel, Mauro Carvalho Chehab,
	Rob Herring, Liam Girdwood, Mark Brown, Thomas Petazzoni,
	Hans Verkuil, Maxime Ripard, kevin.lhopital,
	Kévin L'hôpital

Hi Paul,

On Fri, Oct 23, 2020 at 07:54:04PM +0200, Paul Kocialkowski wrote:
> This introduces YAML bindings documentation for the OV8865
> image sensor.
> 
> Co-developed-by: Kévin L'hôpital <kevin.lhopital@bootlin.com>
> Signed-off-by: Kévin L'hôpital <kevin.lhopital@bootlin.com>
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> ---
>  .../bindings/media/i2c/ovti,ov8865.yaml       | 124 ++++++++++++++++++
>  1 file changed, 124 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml
> new file mode 100644
> index 000000000000..807f1a94afae
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml
> @@ -0,0 +1,124 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/i2c/ovti,ov8865.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: OmniVision OV8865 Image Sensor Device Tree Bindings
> +
> +maintainers:
> +  - Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> +
> +properties:
> +  compatible:
> +    const: ovti,ov8865
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    items:
> +      - description: EXTCLK Clock
> +
> +  clock-names:
> +    items:
> +      - const: extclk

Is this needed with a single clock?

And... shouldn't this also come with assigned-clock-rates etc., to set the
clock frequency?

> +
> +  dvdd-supply:
> +    description: Digital Domain Power Supply
> +
> +  avdd-supply:
> +    description: Analog Domain Power Supply (internal AVDD is used if missing)
> +
> +  dovdd-supply:
> +    description: I/O Domain Power Supply
> +
> +  powerdown-gpios:
> +    maxItems: 1
> +    description: Power Down Pin GPIO Control (active low)
> +
> +  reset-gpios:
> +    maxItems: 1
> +    description: Reset Pin GPIO Control (active low)
> +
> +  port:
> +    type: object
> +    description: Input port, connect to a MIPI CSI-2 receiver
> +
> +    properties:
> +      endpoint:
> +        type: object
> +
> +        properties:
> +          remote-endpoint: true
> +
> +          bus-type:
> +            const: 4
> +
> +          clock-lanes:
> +            maxItems: 1

I believe you can drop clock-lanes and bus-type; these are both constants.

I presume the device does not support lane remapping?

Could you also add link-frequencies, to list which frequencies are known to
be good?

Same comments on the other OV sensor bindings.

> +
> +          data-lanes:
> +            minItems: 1
> +            maxItems: 4
> +
> +        required:
> +          - bus-type
> +          - data-lanes
> +          - remote-endpoint
> +
> +        additionalProperties: false
> +
> +    required:
> +      - endpoint
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - dvdd-supply
> +  - dovdd-supply
> +  - port
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/sun8i-a83t-ccu.h>
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +    i2c2 {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        ov8865: camera@36 {
> +            compatible = "ovti,ov8865";
> +            reg = <0x36>;
> +
> +            pinctrl-names = "default";
> +            pinctrl-0 = <&csi_mclk_pin>;
> +
> +            clocks = <&ccu CLK_CSI_MCLK>;
> +            clock-names = "extclk";
> +
> +            avdd-supply = <&reg_ov8865_avdd>;
> +            dovdd-supply = <&reg_ov8865_dovdd>;
> +            dvdd-supply = <&reg_ov8865_dvdd>;
> +
> +            powerdown-gpios = <&pio 4 17 GPIO_ACTIVE_LOW>; /* PE17 */
> +            reset-gpios = <&pio 4 16 GPIO_ACTIVE_LOW>; /* PE16 */
> +
> +            port {
> +                ov8865_out_mipi_csi2: endpoint {
> +                    bus-type = <4>; /* MIPI CSI-2 D-PHY */
> +                    clock-lanes = <0>;
> +                    data-lanes = <1 2 3 4>;
> +
> +                    remote-endpoint = <&mipi_csi2_in_ov8865>;
> +                };
> +            };
> +        };
> +    };
> +
> +...

-- 
Regards,

Sakari Ailus

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] dt-bindings: media: i2c: Add OV8865 bindings documentation
  2020-11-02 23:24   ` Sakari Ailus
@ 2020-11-04 10:26     ` Paul Kocialkowski
  2020-11-05  8:19       ` Sakari Ailus
  0 siblings, 1 reply; 12+ messages in thread
From: Paul Kocialkowski @ 2020-11-04 10:26 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: linux-media, devicetree, linux-kernel, Mauro Carvalho Chehab,
	Rob Herring, Liam Girdwood, Mark Brown, Thomas Petazzoni,
	Hans Verkuil, Maxime Ripard, kevin.lhopital,
	Kévin L'hôpital


[-- Attachment #1: Type: text/plain, Size: 6023 bytes --]

Hi Sakari and thanks for the review!

On Tue 03 Nov 20, 01:24, Sakari Ailus wrote:
> On Fri, Oct 23, 2020 at 07:54:04PM +0200, Paul Kocialkowski wrote:
> > This introduces YAML bindings documentation for the OV8865
> > image sensor.
> > 
> > Co-developed-by: Kévin L'hôpital <kevin.lhopital@bootlin.com>
> > Signed-off-by: Kévin L'hôpital <kevin.lhopital@bootlin.com>
> > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > ---
> >  .../bindings/media/i2c/ovti,ov8865.yaml       | 124 ++++++++++++++++++
> >  1 file changed, 124 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml
> > new file mode 100644
> > index 000000000000..807f1a94afae
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml
> > @@ -0,0 +1,124 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/media/i2c/ovti,ov8865.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: OmniVision OV8865 Image Sensor Device Tree Bindings
> > +
> > +maintainers:
> > +  - Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > +
> > +properties:
> > +  compatible:
> > +    const: ovti,ov8865
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    items:
> > +      - description: EXTCLK Clock
> > +
> > +  clock-names:
> > +    items:
> > +      - const: extclk
> 
> Is this needed with a single clock?

Yes I think so: we grab the clock with devm_clk_get which takes a name string
that matches the clock-names property.

> And... shouldn't this also come with assigned-clock-rates etc., to set the
> clock frequency?

I'm a bit confused why we would need to do that in the device-tree rather than
setting the clock rate with clk_set_rate in the driver, like any other driver
does. I think this was discussed before (on the initial ov8865 series) and the
conclusion was that there is no particular reason for media i2c drivers to
behave differently. So I believe this is the correct approach.

> > +
> > +  dvdd-supply:
> > +    description: Digital Domain Power Supply
> > +
> > +  avdd-supply:
> > +    description: Analog Domain Power Supply (internal AVDD is used if missing)
> > +
> > +  dovdd-supply:
> > +    description: I/O Domain Power Supply
> > +
> > +  powerdown-gpios:
> > +    maxItems: 1
> > +    description: Power Down Pin GPIO Control (active low)
> > +
> > +  reset-gpios:
> > +    maxItems: 1
> > +    description: Reset Pin GPIO Control (active low)
> > +
> > +  port:
> > +    type: object
> > +    description: Input port, connect to a MIPI CSI-2 receiver
> > +
> > +    properties:
> > +      endpoint:
> > +        type: object
> > +
> > +        properties:
> > +          remote-endpoint: true
> > +
> > +          bus-type:
> > +            const: 4
> > +
> > +          clock-lanes:
> > +            maxItems: 1
> 
> I believe you can drop clock-lanes and bus-type; these are both constants.

I don't understand why bus-type should be dropped because it is constant:
if bus-type is set to something else, the driver will definitely not probe
since we're requesting V4L2_MBUS_CSI2_DPHY for v4l2_fwnode_endpoint_parse.
So I think it's quite important for the bindings to reflect this.

> I presume the device does not support lane remapping?

That's correct so this is indeed not something we can configure.
But shouldn't we instead specift clock-lanes = <0> as a const rather than
getting rid of it?

> Could you also add link-frequencies, to list which frequencies are known to
> be good?

Ah right, I had missed it. I'm a bit unsure about what I should do with the
information from the driver though: should I refuse to use link frequencies that
are not in the list?

Cheers,

Paul

> Same comments on the other OV sensor bindings.
> 
> > +
> > +          data-lanes:
> > +            minItems: 1
> > +            maxItems: 4
> > +
> > +        required:
> > +          - bus-type
> > +          - data-lanes
> > +          - remote-endpoint
> > +
> > +        additionalProperties: false
> > +
> > +    required:
> > +      - endpoint
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - clocks
> > +  - clock-names
> > +  - dvdd-supply
> > +  - dovdd-supply
> > +  - port
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/clock/sun8i-a83t-ccu.h>
> > +    #include <dt-bindings/gpio/gpio.h>
> > +
> > +    i2c2 {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        ov8865: camera@36 {
> > +            compatible = "ovti,ov8865";
> > +            reg = <0x36>;
> > +
> > +            pinctrl-names = "default";
> > +            pinctrl-0 = <&csi_mclk_pin>;
> > +
> > +            clocks = <&ccu CLK_CSI_MCLK>;
> > +            clock-names = "extclk";
> > +
> > +            avdd-supply = <&reg_ov8865_avdd>;
> > +            dovdd-supply = <&reg_ov8865_dovdd>;
> > +            dvdd-supply = <&reg_ov8865_dvdd>;
> > +
> > +            powerdown-gpios = <&pio 4 17 GPIO_ACTIVE_LOW>; /* PE17 */
> > +            reset-gpios = <&pio 4 16 GPIO_ACTIVE_LOW>; /* PE16 */
> > +
> > +            port {
> > +                ov8865_out_mipi_csi2: endpoint {
> > +                    bus-type = <4>; /* MIPI CSI-2 D-PHY */
> > +                    clock-lanes = <0>;
> > +                    data-lanes = <1 2 3 4>;
> > +
> > +                    remote-endpoint = <&mipi_csi2_in_ov8865>;
> > +                };
> > +            };
> > +        };
> > +    };
> > +
> > +...
> 
> -- 
> Regards,
> 
> Sakari Ailus

-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] dt-bindings: media: i2c: Add OV8865 bindings documentation
  2020-11-04 10:26     ` Paul Kocialkowski
@ 2020-11-05  8:19       ` Sakari Ailus
  2020-11-05 15:35         ` Paul Kocialkowski
  0 siblings, 1 reply; 12+ messages in thread
From: Sakari Ailus @ 2020-11-05  8:19 UTC (permalink / raw)
  To: Paul Kocialkowski
  Cc: linux-media, devicetree, linux-kernel, Mauro Carvalho Chehab,
	Rob Herring, Liam Girdwood, Mark Brown, Thomas Petazzoni,
	Hans Verkuil, Maxime Ripard, kevin.lhopital,
	Kévin L'hôpital

Hi Paul,

On Wed, Nov 04, 2020 at 11:26:43AM +0100, Paul Kocialkowski wrote:
> Hi Sakari and thanks for the review!
> 
> On Tue 03 Nov 20, 01:24, Sakari Ailus wrote:
> > On Fri, Oct 23, 2020 at 07:54:04PM +0200, Paul Kocialkowski wrote:
> > > This introduces YAML bindings documentation for the OV8865
> > > image sensor.
> > > 
> > > Co-developed-by: Kévin L'hôpital <kevin.lhopital@bootlin.com>
> > > Signed-off-by: Kévin L'hôpital <kevin.lhopital@bootlin.com>
> > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > > ---
> > >  .../bindings/media/i2c/ovti,ov8865.yaml       | 124 ++++++++++++++++++
> > >  1 file changed, 124 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml
> > > 
> > > diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml
> > > new file mode 100644
> > > index 000000000000..807f1a94afae
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml
> > > @@ -0,0 +1,124 @@
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/media/i2c/ovti,ov8865.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: OmniVision OV8865 Image Sensor Device Tree Bindings
> > > +
> > > +maintainers:
> > > +  - Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > > +
> > > +properties:
> > > +  compatible:
> > > +    const: ovti,ov8865
> > > +
> > > +  reg:
> > > +    maxItems: 1
> > > +
> > > +  clocks:
> > > +    items:
> > > +      - description: EXTCLK Clock
> > > +
> > > +  clock-names:
> > > +    items:
> > > +      - const: extclk
> > 
> > Is this needed with a single clock?
> 
> Yes I think so: we grab the clock with devm_clk_get which takes a name string
> that matches the clock-names property.

That argument may be NULL.

> 
> > And... shouldn't this also come with assigned-clock-rates etc., to set the
> > clock frequency?
> 
> I'm a bit confused why we would need to do that in the device-tree rather than
> setting the clock rate with clk_set_rate in the driver, like any other driver
> does. I think this was discussed before (on the initial ov8865 series) and the
> conclusion was that there is no particular reason for media i2c drivers to
> behave differently. So I believe this is the correct approach.

I'm not exactly sure about that conclusion.

You can use clk_set_rate() if you get the frequency from DT, but we
recently did conclude that camera sensor drivers can expect to get the
frequency indicated by assigned-clock-rate property.

In other words, the driver may not be specific to a particular board and
SoC you have.

Please also read Documentation/driver-api/media/camera-sensor.rst .

> 
> > > +
> > > +  dvdd-supply:
> > > +    description: Digital Domain Power Supply
> > > +
> > > +  avdd-supply:
> > > +    description: Analog Domain Power Supply (internal AVDD is used if missing)
> > > +
> > > +  dovdd-supply:
> > > +    description: I/O Domain Power Supply
> > > +
> > > +  powerdown-gpios:
> > > +    maxItems: 1
> > > +    description: Power Down Pin GPIO Control (active low)
> > > +
> > > +  reset-gpios:
> > > +    maxItems: 1
> > > +    description: Reset Pin GPIO Control (active low)
> > > +
> > > +  port:
> > > +    type: object
> > > +    description: Input port, connect to a MIPI CSI-2 receiver
> > > +
> > > +    properties:
> > > +      endpoint:
> > > +        type: object
> > > +
> > > +        properties:
> > > +          remote-endpoint: true
> > > +
> > > +          bus-type:
> > > +            const: 4
> > > +
> > > +          clock-lanes:
> > > +            maxItems: 1
> > 
> > I believe you can drop clock-lanes and bus-type; these are both constants.
> 
> I don't understand why bus-type should be dropped because it is constant:
> if bus-type is set to something else, the driver will definitely not probe
> since we're requesting V4L2_MBUS_CSI2_DPHY for v4l2_fwnode_endpoint_parse.
> So I think it's quite important for the bindings to reflect this.

This driver is for a particular device that has MIPI CSI-2 on D-PHY as the
data bus. You can assume that in the driver.

> 
> > I presume the device does not support lane remapping?
> 
> That's correct so this is indeed not something we can configure.
> But shouldn't we instead specift clock-lanes = <0> as a const rather than
> getting rid of it?

Why would you put redundant information to DT?

> 
> > Could you also add link-frequencies, to list which frequencies are known to
> > be good?
> 
> Ah right, I had missed it. I'm a bit unsure about what I should do with the
> information from the driver though: should I refuse to use link frequencies that
> are not in the list?

Yes, please.

-- 
Regards,

Sakari Ailus

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] dt-bindings: media: i2c: Add OV8865 bindings documentation
  2020-11-05  8:19       ` Sakari Ailus
@ 2020-11-05 15:35         ` Paul Kocialkowski
  2020-11-11 13:18           ` Sakari Ailus
  0 siblings, 1 reply; 12+ messages in thread
From: Paul Kocialkowski @ 2020-11-05 15:35 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: linux-media, devicetree, linux-kernel, Mauro Carvalho Chehab,
	Rob Herring, Liam Girdwood, Mark Brown, Thomas Petazzoni,
	Hans Verkuil, Maxime Ripard, kevin.lhopital,
	Kévin L'hôpital


[-- Attachment #1: Type: text/plain, Size: 6576 bytes --]

Hi Sakari,

On Thu 05 Nov 20, 10:19, Sakari Ailus wrote:
> Hi Paul,
> 
> On Wed, Nov 04, 2020 at 11:26:43AM +0100, Paul Kocialkowski wrote:
> > Hi Sakari and thanks for the review!
> > 
> > On Tue 03 Nov 20, 01:24, Sakari Ailus wrote:
> > > On Fri, Oct 23, 2020 at 07:54:04PM +0200, Paul Kocialkowski wrote:
> > > > This introduces YAML bindings documentation for the OV8865
> > > > image sensor.
> > > > 
> > > > Co-developed-by: Kévin L'hôpital <kevin.lhopital@bootlin.com>
> > > > Signed-off-by: Kévin L'hôpital <kevin.lhopital@bootlin.com>
> > > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > > > ---
> > > >  .../bindings/media/i2c/ovti,ov8865.yaml       | 124 ++++++++++++++++++
> > > >  1 file changed, 124 insertions(+)
> > > >  create mode 100644 Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml
> > > > new file mode 100644
> > > > index 000000000000..807f1a94afae
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml
> > > > @@ -0,0 +1,124 @@
> > > > +# SPDX-License-Identifier: GPL-2.0
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/media/i2c/ovti,ov8865.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: OmniVision OV8865 Image Sensor Device Tree Bindings
> > > > +
> > > > +maintainers:
> > > > +  - Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > > > +
> > > > +properties:
> > > > +  compatible:
> > > > +    const: ovti,ov8865
> > > > +
> > > > +  reg:
> > > > +    maxItems: 1
> > > > +
> > > > +  clocks:
> > > > +    items:
> > > > +      - description: EXTCLK Clock
> > > > +
> > > > +  clock-names:
> > > > +    items:
> > > > +      - const: extclk
> > > 
> > > Is this needed with a single clock?
> > 
> > Yes I think so: we grab the clock with devm_clk_get which takes a name string
> > that matches the clock-names property.
> 
> That argument may be NULL.

Understood, let's get rid of clock-names then. I see this is done in a few
drivers already, but many also give it a name with a single clock.

It would be nice if that was consistent across all drivers just so that the
expectation is clear (that the best way for that to happen is probably to
fix up a patch myself though).

> > > And... shouldn't this also come with assigned-clock-rates etc., to set the
> > > clock frequency?
> > 
> > I'm a bit confused why we would need to do that in the device-tree rather than
> > setting the clock rate with clk_set_rate in the driver, like any other driver
> > does. I think this was discussed before (on the initial ov8865 series) and the
> > conclusion was that there is no particular reason for media i2c drivers to
> > behave differently. So I believe this is the correct approach.
> 
> I'm not exactly sure about that conclusion.

I may have jumped too far here. It's not exactly clear to me what was the
conclusion from...
https://lore.kernel.org/linux-arm-kernel/20200401080705.j4goeqcqhoswhx4u@gilmour.lan/

> You can use clk_set_rate() if you get the frequency from DT, but we
> recently did conclude that camera sensor drivers can expect to get the
> frequency indicated by assigned-clock-rate property.

...but it looks like clock-frequency was preferred over assigned-clock-rates
and this is what the binding that was merged suggests. Is that correct?

I now understand that the clock frequency may depend on the system integration
for this special case so we have to specify it via dt.

> In other words, the driver may not be specific to a particular board and
> SoC you have.

Although this is sadly more than often the case, because handling a variable
clock rate in the driver is quite complex (and even more with static init tables
that include PLL configuration). And sadly my driver is no exception and
only supports 24 MHz input.

> Please also read Documentation/driver-api/media/camera-sensor.rst .

Thanks, I hadn't seen that document before. It's great that it exists!

Paul

> > > > +
> > > > +  dvdd-supply:
> > > > +    description: Digital Domain Power Supply
> > > > +
> > > > +  avdd-supply:
> > > > +    description: Analog Domain Power Supply (internal AVDD is used if missing)
> > > > +
> > > > +  dovdd-supply:
> > > > +    description: I/O Domain Power Supply
> > > > +
> > > > +  powerdown-gpios:
> > > > +    maxItems: 1
> > > > +    description: Power Down Pin GPIO Control (active low)
> > > > +
> > > > +  reset-gpios:
> > > > +    maxItems: 1
> > > > +    description: Reset Pin GPIO Control (active low)
> > > > +
> > > > +  port:
> > > > +    type: object
> > > > +    description: Input port, connect to a MIPI CSI-2 receiver
> > > > +
> > > > +    properties:
> > > > +      endpoint:
> > > > +        type: object
> > > > +
> > > > +        properties:
> > > > +          remote-endpoint: true
> > > > +
> > > > +          bus-type:
> > > > +            const: 4
> > > > +
> > > > +          clock-lanes:
> > > > +            maxItems: 1
> > > 
> > > I believe you can drop clock-lanes and bus-type; these are both constants.
> > 
> > I don't understand why bus-type should be dropped because it is constant:
> > if bus-type is set to something else, the driver will definitely not probe
> > since we're requesting V4L2_MBUS_CSI2_DPHY for v4l2_fwnode_endpoint_parse.
> > So I think it's quite important for the bindings to reflect this.
> 
> This driver is for a particular device that has MIPI CSI-2 on D-PHY as the
> data bus. You can assume that in the driver.
> 
> > 
> > > I presume the device does not support lane remapping?
> > 
> > That's correct so this is indeed not something we can configure.
> > But shouldn't we instead specift clock-lanes = <0> as a const rather than
> > getting rid of it?
> 
> Why would you put redundant information to DT?
> 
> > 
> > > Could you also add link-frequencies, to list which frequencies are known to
> > > be good?
> > 
> > Ah right, I had missed it. I'm a bit unsure about what I should do with the
> > information from the driver though: should I refuse to use link frequencies that
> > are not in the list?
> 
> Yes, please.
> 
> -- 
> Regards,
> 
> Sakari Ailus

-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] dt-bindings: media: i2c: Add OV8865 bindings documentation
  2020-11-05 15:35         ` Paul Kocialkowski
@ 2020-11-11 13:18           ` Sakari Ailus
  2020-11-13 17:27             ` Maxime Ripard
  0 siblings, 1 reply; 12+ messages in thread
From: Sakari Ailus @ 2020-11-11 13:18 UTC (permalink / raw)
  To: Paul Kocialkowski
  Cc: linux-media, devicetree, linux-kernel, Mauro Carvalho Chehab,
	Rob Herring, Liam Girdwood, Mark Brown, Thomas Petazzoni,
	Hans Verkuil, Maxime Ripard, kevin.lhopital,
	Kévin L'hôpital

Hi Paul,

On Thu, Nov 05, 2020 at 04:35:34PM +0100, Paul Kocialkowski wrote:
> Hi Sakari,
> 
> On Thu 05 Nov 20, 10:19, Sakari Ailus wrote:
> > Hi Paul,
> > 
> > On Wed, Nov 04, 2020 at 11:26:43AM +0100, Paul Kocialkowski wrote:
> > > Hi Sakari and thanks for the review!
> > > 
> > > On Tue 03 Nov 20, 01:24, Sakari Ailus wrote:
> > > > On Fri, Oct 23, 2020 at 07:54:04PM +0200, Paul Kocialkowski wrote:
> > > > > This introduces YAML bindings documentation for the OV8865
> > > > > image sensor.
> > > > > 
> > > > > Co-developed-by: Kévin L'hôpital <kevin.lhopital@bootlin.com>
> > > > > Signed-off-by: Kévin L'hôpital <kevin.lhopital@bootlin.com>
> > > > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > > > > ---
> > > > >  .../bindings/media/i2c/ovti,ov8865.yaml       | 124 ++++++++++++++++++
> > > > >  1 file changed, 124 insertions(+)
> > > > >  create mode 100644 Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml
> > > > > 
> > > > > diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml
> > > > > new file mode 100644
> > > > > index 000000000000..807f1a94afae
> > > > > --- /dev/null
> > > > > +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml
> > > > > @@ -0,0 +1,124 @@
> > > > > +# SPDX-License-Identifier: GPL-2.0
> > > > > +%YAML 1.2
> > > > > +---
> > > > > +$id: http://devicetree.org/schemas/media/i2c/ovti,ov8865.yaml#
> > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > > +
> > > > > +title: OmniVision OV8865 Image Sensor Device Tree Bindings
> > > > > +
> > > > > +maintainers:
> > > > > +  - Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > > > > +
> > > > > +properties:
> > > > > +  compatible:
> > > > > +    const: ovti,ov8865
> > > > > +
> > > > > +  reg:
> > > > > +    maxItems: 1
> > > > > +
> > > > > +  clocks:
> > > > > +    items:
> > > > > +      - description: EXTCLK Clock
> > > > > +
> > > > > +  clock-names:
> > > > > +    items:
> > > > > +      - const: extclk
> > > > 
> > > > Is this needed with a single clock?
> > > 
> > > Yes I think so: we grab the clock with devm_clk_get which takes a name string
> > > that matches the clock-names property.
> > 
> > That argument may be NULL.
> 
> Understood, let's get rid of clock-names then. I see this is done in a few
> drivers already, but many also give it a name with a single clock.
> 
> It would be nice if that was consistent across all drivers just so that the
> expectation is clear (that the best way for that to happen is probably to
> fix up a patch myself though).

I guess somewhat different practices exist depending on the tree albeit
it's all DT bindings. It's also not wrong to have the name of the clock
there, no, but virtually all camera sensors consume a single clock, so you
may as well omit the information.

> 
> > > > And... shouldn't this also come with assigned-clock-rates etc., to set the
> > > > clock frequency?
> > > 
> > > I'm a bit confused why we would need to do that in the device-tree rather than
> > > setting the clock rate with clk_set_rate in the driver, like any other driver
> > > does. I think this was discussed before (on the initial ov8865 series) and the
> > > conclusion was that there is no particular reason for media i2c drivers to
> > > behave differently. So I believe this is the correct approach.
> > 
> > I'm not exactly sure about that conclusion.
> 
> I may have jumped too far here. It's not exactly clear to me what was the
> conclusion from...
> https://lore.kernel.org/linux-arm-kernel/20200401080705.j4goeqcqhoswhx4u@gilmour.lan/

Yes, there has been more discussion on the topic, most recently in this
thread:

<URL:https://lore.kernel.org/linux-arm-kernel/20201102150547.GY26150@paasikivi.fi.intel.com/>

I think this deserves to be added to camera-sensor.rst .

> 
> > You can use clk_set_rate() if you get the frequency from DT, but we
> > recently did conclude that camera sensor drivers can expect to get the
> > frequency indicated by assigned-clock-rate property.
> 
> ...but it looks like clock-frequency was preferred over assigned-clock-rates
> and this is what the binding that was merged suggests. Is that correct?

assigned-clock-rates is fine. The assumption is that the clock frequency
does not change from the value set through DT, and the driver gets that
exact frequency.

> 
> I now understand that the clock frequency may depend on the system integration
> for this special case so we have to specify it via dt.

Correct.

> 
> > In other words, the driver may not be specific to a particular board and
> > SoC you have.
> 
> Although this is sadly more than often the case, because handling a variable
> clock rate in the driver is quite complex (and even more with static init tables
> that include PLL configuration). And sadly my driver is no exception and
> only supports 24 MHz input.

That's fine. If someone needs other frequencies, they can always add
support for those in the driver.

> 
> > Please also read Documentation/driver-api/media/camera-sensor.rst .
> 
> Thanks, I hadn't seen that document before. It's great that it exists!

You're welcome!

This was indeed written to reduce the number of patch revisions needed ot
get a driver to upstream. :-)

-- 
Kind regards,

Sakari Ailus

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] dt-bindings: media: i2c: Add OV8865 bindings documentation
  2020-11-11 13:18           ` Sakari Ailus
@ 2020-11-13 17:27             ` Maxime Ripard
  2020-11-18 22:38               ` Sakari Ailus
  0 siblings, 1 reply; 12+ messages in thread
From: Maxime Ripard @ 2020-11-13 17:27 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: Paul Kocialkowski, linux-media, devicetree, linux-kernel,
	Mauro Carvalho Chehab, Rob Herring, Liam Girdwood, Mark Brown,
	Thomas Petazzoni, Hans Verkuil, kevin.lhopital,
	Kévin L'hôpital


[-- Attachment #1: Type: text/plain, Size: 6070 bytes --]

Hi Sakari,

On Wed, Nov 11, 2020 at 03:18:57PM +0200, Sakari Ailus wrote:
> Hi Paul,
> 
> On Thu, Nov 05, 2020 at 04:35:34PM +0100, Paul Kocialkowski wrote:
> > Hi Sakari,
> > 
> > On Thu 05 Nov 20, 10:19, Sakari Ailus wrote:
> > > Hi Paul,
> > > 
> > > On Wed, Nov 04, 2020 at 11:26:43AM +0100, Paul Kocialkowski wrote:
> > > > Hi Sakari and thanks for the review!
> > > > 
> > > > On Tue 03 Nov 20, 01:24, Sakari Ailus wrote:
> > > > > On Fri, Oct 23, 2020 at 07:54:04PM +0200, Paul Kocialkowski wrote:
> > > > > > This introduces YAML bindings documentation for the OV8865
> > > > > > image sensor.
> > > > > > 
> > > > > > Co-developed-by: Kévin L'hôpital <kevin.lhopital@bootlin.com>
> > > > > > Signed-off-by: Kévin L'hôpital <kevin.lhopital@bootlin.com>
> > > > > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > > > > > ---
> > > > > >  .../bindings/media/i2c/ovti,ov8865.yaml       | 124 ++++++++++++++++++
> > > > > >  1 file changed, 124 insertions(+)
> > > > > >  create mode 100644 Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml
> > > > > > 
> > > > > > diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml
> > > > > > new file mode 100644
> > > > > > index 000000000000..807f1a94afae
> > > > > > --- /dev/null
> > > > > > +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml
> > > > > > @@ -0,0 +1,124 @@
> > > > > > +# SPDX-License-Identifier: GPL-2.0
> > > > > > +%YAML 1.2
> > > > > > +---
> > > > > > +$id: http://devicetree.org/schemas/media/i2c/ovti,ov8865.yaml#
> > > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > > > +
> > > > > > +title: OmniVision OV8865 Image Sensor Device Tree Bindings
> > > > > > +
> > > > > > +maintainers:
> > > > > > +  - Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > > > > > +
> > > > > > +properties:
> > > > > > +  compatible:
> > > > > > +    const: ovti,ov8865
> > > > > > +
> > > > > > +  reg:
> > > > > > +    maxItems: 1
> > > > > > +
> > > > > > +  clocks:
> > > > > > +    items:
> > > > > > +      - description: EXTCLK Clock
> > > > > > +
> > > > > > +  clock-names:
> > > > > > +    items:
> > > > > > +      - const: extclk
> > > > > 
> > > > > Is this needed with a single clock?
> > > > 
> > > > Yes I think so: we grab the clock with devm_clk_get which takes a name string
> > > > that matches the clock-names property.
> > > 
> > > That argument may be NULL.
> > 
> > Understood, let's get rid of clock-names then. I see this is done in a few
> > drivers already, but many also give it a name with a single clock.
> > 
> > It would be nice if that was consistent across all drivers just so that the
> > expectation is clear (that the best way for that to happen is probably to
> > fix up a patch myself though).
> 
> I guess somewhat different practices exist depending on the tree albeit
> it's all DT bindings. It's also not wrong to have the name of the clock
> there, no, but virtually all camera sensors consume a single clock, so you
> may as well omit the information.
> 
> > 
> > > > > And... shouldn't this also come with assigned-clock-rates etc., to set the
> > > > > clock frequency?
> > > > 
> > > > I'm a bit confused why we would need to do that in the device-tree rather than
> > > > setting the clock rate with clk_set_rate in the driver, like any other driver
> > > > does. I think this was discussed before (on the initial ov8865 series) and the
> > > > conclusion was that there is no particular reason for media i2c drivers to
> > > > behave differently. So I believe this is the correct approach.
> > > 
> > > I'm not exactly sure about that conclusion.
> > 
> > I may have jumped too far here. It's not exactly clear to me what was the
> > conclusion from...
> > https://lore.kernel.org/linux-arm-kernel/20200401080705.j4goeqcqhoswhx4u@gilmour.lan/
> 
> Yes, there has been more discussion on the topic, most recently in this
> thread:
> 
> <URL:https://lore.kernel.org/linux-arm-kernel/20201102150547.GY26150@paasikivi.fi.intel.com/>
> 
> I think this deserves to be added to camera-sensor.rst .

It's not really a discussion though :)

We had back in that thread some issues with assigned-clock-rates that
don't get addressed at all.

> > > You can use clk_set_rate() if you get the frequency from DT, but we
> > > recently did conclude that camera sensor drivers can expect to get the
> > > frequency indicated by assigned-clock-rate property.
> > 
> > ...but it looks like clock-frequency was preferred over assigned-clock-rates
> > and this is what the binding that was merged suggests. Is that correct?
> 
> assigned-clock-rates is fine. The assumption is that the clock frequency
> does not change from the value set through DT, and the driver gets that
> exact frequency.

That's two fairly big assumptions though. A clock driver is free to
round the clock to whatever rate it wants, and assigned-clock-rates
doesn't provide any guarantee on this, nor does it let the potential
user know about it.

It might work for one-off cases, but there's no guarantee that it will
in the future since this is very much dependent on the clock setup,
driver and the other devices in the system (and to some extent the
configuration as well).

And since we only rely on it, we can't fix it properly either if it ever
occurs.

And then, semantically, this assigned-clock-rates isn't about what the
devices support but what the driver supports. The clock tree of
omnivision sensors (at least, I can't comment on the imx218 linked
above) allows for a very flexible input clock, it's only the driver that
requires it because most of its configuration relies on it.

It's definitely fine for the driver to do so, but it really doesn't
belong in the DT.

I thought we had an agreement on this last time we discussed it, but I'm
not really sure what made you change your mind.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] dt-bindings: media: i2c: Add OV8865 bindings documentation
  2020-11-13 17:27             ` Maxime Ripard
@ 2020-11-18 22:38               ` Sakari Ailus
  0 siblings, 0 replies; 12+ messages in thread
From: Sakari Ailus @ 2020-11-18 22:38 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Paul Kocialkowski, linux-media, devicetree, linux-kernel,
	Mauro Carvalho Chehab, Rob Herring, Liam Girdwood, Mark Brown,
	Thomas Petazzoni, Hans Verkuil, kevin.lhopital,
	Kévin L'hôpital

Hi Maxime,

Sorry for the late reply.

On Fri, Nov 13, 2020 at 06:27:35PM +0100, Maxime Ripard wrote:
> Hi Sakari,
> 
> On Wed, Nov 11, 2020 at 03:18:57PM +0200, Sakari Ailus wrote:
> > Hi Paul,
> > 
> > On Thu, Nov 05, 2020 at 04:35:34PM +0100, Paul Kocialkowski wrote:
> > > Hi Sakari,
> > > 
> > > On Thu 05 Nov 20, 10:19, Sakari Ailus wrote:
> > > > Hi Paul,
> > > > 
> > > > On Wed, Nov 04, 2020 at 11:26:43AM +0100, Paul Kocialkowski wrote:
> > > > > Hi Sakari and thanks for the review!
> > > > > 
> > > > > On Tue 03 Nov 20, 01:24, Sakari Ailus wrote:
> > > > > > On Fri, Oct 23, 2020 at 07:54:04PM +0200, Paul Kocialkowski wrote:
> > > > > > > This introduces YAML bindings documentation for the OV8865
> > > > > > > image sensor.
> > > > > > > 
> > > > > > > Co-developed-by: Kévin L'hôpital <kevin.lhopital@bootlin.com>
> > > > > > > Signed-off-by: Kévin L'hôpital <kevin.lhopital@bootlin.com>
> > > > > > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > > > > > > ---
> > > > > > >  .../bindings/media/i2c/ovti,ov8865.yaml       | 124 ++++++++++++++++++
> > > > > > >  1 file changed, 124 insertions(+)
> > > > > > >  create mode 100644 Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml
> > > > > > > 
> > > > > > > diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml
> > > > > > > new file mode 100644
> > > > > > > index 000000000000..807f1a94afae
> > > > > > > --- /dev/null
> > > > > > > +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml
> > > > > > > @@ -0,0 +1,124 @@
> > > > > > > +# SPDX-License-Identifier: GPL-2.0
> > > > > > > +%YAML 1.2
> > > > > > > +---
> > > > > > > +$id: http://devicetree.org/schemas/media/i2c/ovti,ov8865.yaml#
> > > > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > > > > +
> > > > > > > +title: OmniVision OV8865 Image Sensor Device Tree Bindings
> > > > > > > +
> > > > > > > +maintainers:
> > > > > > > +  - Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > > > > > > +
> > > > > > > +properties:
> > > > > > > +  compatible:
> > > > > > > +    const: ovti,ov8865
> > > > > > > +
> > > > > > > +  reg:
> > > > > > > +    maxItems: 1
> > > > > > > +
> > > > > > > +  clocks:
> > > > > > > +    items:
> > > > > > > +      - description: EXTCLK Clock
> > > > > > > +
> > > > > > > +  clock-names:
> > > > > > > +    items:
> > > > > > > +      - const: extclk
> > > > > > 
> > > > > > Is this needed with a single clock?
> > > > > 
> > > > > Yes I think so: we grab the clock with devm_clk_get which takes a name string
> > > > > that matches the clock-names property.
> > > > 
> > > > That argument may be NULL.
> > > 
> > > Understood, let's get rid of clock-names then. I see this is done in a few
> > > drivers already, but many also give it a name with a single clock.
> > > 
> > > It would be nice if that was consistent across all drivers just so that the
> > > expectation is clear (that the best way for that to happen is probably to
> > > fix up a patch myself though).
> > 
> > I guess somewhat different practices exist depending on the tree albeit
> > it's all DT bindings. It's also not wrong to have the name of the clock
> > there, no, but virtually all camera sensors consume a single clock, so you
> > may as well omit the information.
> > 
> > > 
> > > > > > And... shouldn't this also come with assigned-clock-rates etc., to set the
> > > > > > clock frequency?
> > > > > 
> > > > > I'm a bit confused why we would need to do that in the device-tree rather than
> > > > > setting the clock rate with clk_set_rate in the driver, like any other driver
> > > > > does. I think this was discussed before (on the initial ov8865 series) and the
> > > > > conclusion was that there is no particular reason for media i2c drivers to
> > > > > behave differently. So I believe this is the correct approach.
> > > > 
> > > > I'm not exactly sure about that conclusion.
> > > 
> > > I may have jumped too far here. It's not exactly clear to me what was the
> > > conclusion from...
> > > https://lore.kernel.org/linux-arm-kernel/20200401080705.j4goeqcqhoswhx4u@gilmour.lan/
> > 
> > Yes, there has been more discussion on the topic, most recently in this
> > thread:
> > 
> > <URL:https://lore.kernel.org/linux-arm-kernel/20201102150547.GY26150@paasikivi.fi.intel.com/>
> > 
> > I think this deserves to be added to camera-sensor.rst .
> 
> It's not really a discussion though :)
> 
> We had back in that thread some issues with assigned-clock-rates that
> don't get addressed at all.
> 
> > > > You can use clk_set_rate() if you get the frequency from DT, but we
> > > > recently did conclude that camera sensor drivers can expect to get the
> > > > frequency indicated by assigned-clock-rate property.
> > > 
> > > ...but it looks like clock-frequency was preferred over assigned-clock-rates
> > > and this is what the binding that was merged suggests. Is that correct?
> > 
> > assigned-clock-rates is fine. The assumption is that the clock frequency
> > does not change from the value set through DT, and the driver gets that
> > exact frequency.
> 
> That's two fairly big assumptions though. A clock driver is free to
> round the clock to whatever rate it wants, and assigned-clock-rates
> doesn't provide any guarantee on this, nor does it let the potential
> user know about it.
> 
> It might work for one-off cases, but there's no guarantee that it will
> in the future since this is very much dependent on the clock setup,
> driver and the other devices in the system (and to some extent the
> configuration as well).
> 
> And since we only rely on it, we can't fix it properly either if it ever
> occurs.
> 
> And then, semantically, this assigned-clock-rates isn't about what the
> devices support but what the driver supports. The clock tree of
> omnivision sensors (at least, I can't comment on the imx218 linked
> above) allows for a very flexible input clock, it's only the driver that
> requires it because most of its configuration relies on it.

There is a semantic difference, yes. The driver in this case assumes the
frequency is the one it is expected to use. In case the frequency is
different, the most likely outcome is that the image sensor is inoperable.
But again that is fairly easy to notice.

If someone prefers to fix this and make it reliable also in principle, that
would require changing the semantics a little as well as adding a function
to the clock framework to set the assigned frequency.

> 
> It's definitely fine for the driver to do so, but it really doesn't
> belong in the DT.
> 
> I thought we had an agreement on this last time we discussed it, but I'm
> not really sure what made you change your mind.

Most new sensor drivers come with assigned-clock-rates etc. properties and
I'll need to explain the same over and over again in review. Then using
"clock-frequency" property requires quite a few more lines of code in the
drivers.

I also recently discussed the matter with Rob H. and his opinion was
basically that we should be using assigned-clock-rates here instead.

I wonder how this works on other places where the frequency can be one of
several options, but the correct frequency is board specific.

-- 
Kind regards,

Sakari Ailus

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, back to index

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-23 17:54 [PATCH 0/3] media: i2c: OV8865 image sensor support Paul Kocialkowski
2020-10-23 17:54 ` [PATCH 1/3] dt-bindings: media: i2c: Add OV8865 bindings documentation Paul Kocialkowski
2020-10-30 16:39   ` Rob Herring
2020-11-02 23:24   ` Sakari Ailus
2020-11-04 10:26     ` Paul Kocialkowski
2020-11-05  8:19       ` Sakari Ailus
2020-11-05 15:35         ` Paul Kocialkowski
2020-11-11 13:18           ` Sakari Ailus
2020-11-13 17:27             ` Maxime Ripard
2020-11-18 22:38               ` Sakari Ailus
2020-10-23 17:54 ` [PATCH 2/3] media: i2c: Add support for the OV8865 image sensor Paul Kocialkowski
2020-10-23 17:54 ` [PATCH NOT FOR MERGE 3/3] ARM: dts: sun8i: a83t: bananapi-m3: Enable MIPI CSI-2 with OV8865 Paul Kocialkowski

Linux-Media Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-media/0 linux-media/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-media linux-media/ https://lore.kernel.org/linux-media \
		linux-media@vger.kernel.org
	public-inbox-index linux-media

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-media


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git