Linux-Media Archive on lore.kernel.org
 help / color / Atom feed
* [V1, 0/2] media: ov8856: DT bindings and sensor mode improvements
@ 2019-08-08  9:22 dongchun.zhu
  2019-08-08  9:22 ` [V1, 1/2] media: dt-bindings: media: i2c: Add bindings for ov8856 dongchun.zhu
  2019-08-08  9:22 ` [V1, 2/2] media: i2c: Add more sensor mode for ov8856 camera sensor dongchun.zhu
  0 siblings, 2 replies; 6+ messages in thread
From: dongchun.zhu @ 2019-08-08  9:22 UTC (permalink / raw)
  To: mchehab, robh+dt, mark.rutland, sakari.ailus, drinkcat, tfiga,
	matthias.bgg, bingbu.cao
  Cc: srv_heupstream, linux-mediatek, linux-arm-kernel, sj.huang,
	linux-media, devicetree, louis.kuo, shengnan.wang, dongchun.zhu

From: Dongchun Zhu <dongchun.zhu@mediatek.com>

Hello,

This series adds DT bindings and some more sensor modes for users to use.

From the latest ov8856 datasheet, it is proposed to adopt the resolution of 1632*1224 and 3264*2448,
together with Bayer Order of BGGR. Thus here we provide two more scenarios.

In addition, the hardware version of ov8856 is also distinguished from one OTP SRAM register R700F.
PLL register R3614 requires to be correspondingly updated.
For instance, 0x20 is suggested for 1B camera module.

It is emphasized that I2C still actives while device is to runtime suspended.
This makes ov8856_suspend function need to remove the operation of stoping streaming.

Finally, ARM and device tree matching were added as an alternative.

Dongchun Zhu (2):
  media: dt-bindings: media: i2c: Add bindings for OV8856
  media: i2c: Add more sensor mode for OV8856 camera sensor

 .../devicetree/bindings/media/i2c/ov8856.txt       |  41 ++
 MAINTAINERS                                        |   1 +
 drivers/media/i2c/ov8856.c                         | 624 ++++++++++++++++++++-
 3 files changed, 663 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ov8856.txt

-- 
2.9.2


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

* [V1, 1/2] media: dt-bindings: media: i2c: Add bindings for ov8856
  2019-08-08  9:22 [V1, 0/2] media: ov8856: DT bindings and sensor mode improvements dongchun.zhu
@ 2019-08-08  9:22 ` dongchun.zhu
  2019-08-08 16:54   ` Sakari Ailus
  2019-08-08  9:22 ` [V1, 2/2] media: i2c: Add more sensor mode for ov8856 camera sensor dongchun.zhu
  1 sibling, 1 reply; 6+ messages in thread
From: dongchun.zhu @ 2019-08-08  9:22 UTC (permalink / raw)
  To: mchehab, robh+dt, mark.rutland, sakari.ailus, drinkcat, tfiga,
	matthias.bgg, bingbu.cao
  Cc: srv_heupstream, linux-mediatek, linux-arm-kernel, sj.huang,
	linux-media, devicetree, louis.kuo, shengnan.wang, dongchun.zhu

From: Dongchun Zhu <dongchun.zhu@mediatek.com>

Add device tree binding documentation for the ov8856 camera sensor.

Signed-off-by: Dongchun Zhu <dongchun.zhu@mediatek.com>
---
 .../devicetree/bindings/media/i2c/ov8856.txt       | 41 ++++++++++++++++++++++
 MAINTAINERS                                        |  1 +
 2 files changed, 42 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ov8856.txt

diff --git a/Documentation/devicetree/bindings/media/i2c/ov8856.txt b/Documentation/devicetree/bindings/media/i2c/ov8856.txt
new file mode 100644
index 0000000..96b10e0
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ov8856.txt
@@ -0,0 +1,41 @@
+* Omnivision OV8856 MIPI CSI-2 sensor
+
+Required Properties:
+- compatible: shall be "ovti,ov8856"
+- clocks: reference to the xvclk input clock
+- clock-names: shall be "xvclk"
+- avdd-supply: Analog voltage supply, 2.8 volts
+- dovdd-supply: Digital I/O voltage supply, 1.8 volts
+- dvdd-supply: Digital core voltage supply, 1.2 volts
+- reset-gpios: Low active reset gpio
+
+The device node shall contain one 'port' child node with an
+'endpoint' subnode for its digital output video port,
+in accordance with the video interface bindings defined in
+Documentation/devicetree/bindings/media/video-interfaces.txt.
+The endpoint optional property 'data-lanes' shall be "<0 1 3 4>".
+
+Example:
+&i2c7 {
+	ov8856: camera-sensor@10 {
+		compatible = "ovti,ov8856";
+		reg = <0x10>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&clk_24m_cam>;
+
+		clocks = <&cru SCLK_TESTCLKOUT1>;
+		clock-names = "xvclk";
+
+		avdd-supply = <&mt6358_vcama2_reg>;
+		dvdd-supply = <&mt6358_vcamd_reg>;
+		dovdd-supply = <&mt6358_vcamio_reg>;
+		reset-gpios = <&gpio2 3 GPIO_ACTIVE_LOW>;
+
+		port {
+			wcam_out: endpoint {
+				remote-endpoint = <&mipi_in_wcam>;
+				data-lanes = <0 1 3 4>;
+			};
+		};
+	};
+};
diff --git a/MAINTAINERS b/MAINTAINERS
index 783569e..7746c6b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11889,6 +11889,7 @@ L:	linux-media@vger.kernel.org
 T:	git git://linuxtv.org/media_tree.git
 S:	Maintained
 F:	drivers/media/i2c/ov8856.c
+F:	Documentation/devicetree/bindings/media/i2c/ov8856.txt
 
 OMNIVISION OV9650 SENSOR DRIVER
 M:	Sakari Ailus <sakari.ailus@linux.intel.com>
-- 
2.9.2


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

* [V1, 2/2] media: i2c: Add more sensor mode for ov8856 camera sensor
  2019-08-08  9:22 [V1, 0/2] media: ov8856: DT bindings and sensor mode improvements dongchun.zhu
  2019-08-08  9:22 ` [V1, 1/2] media: dt-bindings: media: i2c: Add bindings for ov8856 dongchun.zhu
@ 2019-08-08  9:22 ` dongchun.zhu
  2019-08-08 13:53   ` Sakari Ailus
  1 sibling, 1 reply; 6+ messages in thread
From: dongchun.zhu @ 2019-08-08  9:22 UTC (permalink / raw)
  To: mchehab, robh+dt, mark.rutland, sakari.ailus, drinkcat, tfiga,
	matthias.bgg, bingbu.cao
  Cc: srv_heupstream, linux-mediatek, linux-arm-kernel, sj.huang,
	linux-media, devicetree, louis.kuo, shengnan.wang, dongchun.zhu

From: Dongchun Zhu <dongchun.zhu@mediatek.com>

This patch mainly adds two more sensor modes for OV8856 image sensor.
The OV8856 driver currently supports output format: 10-bit Raw,
the resolution of 1632*1224 and 3264*2448, and the bayer order of BGGR.
The hardware version also differs in some OTP regiser,
as well as PLL register setting.

Signed-off-by: Dongchun Zhu <dongchun.zhu@mediatek.com>
---
 drivers/media/i2c/ov8856.c | 624 ++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 621 insertions(+), 3 deletions(-)

diff --git a/drivers/media/i2c/ov8856.c b/drivers/media/i2c/ov8856.c
index cd347d6..e0610b6 100644
--- a/drivers/media/i2c/ov8856.c
+++ b/drivers/media/i2c/ov8856.c
@@ -1,12 +1,15 @@
 // SPDX-License-Identifier: GPL-2.0
 // Copyright (c) 2019 Intel Corporation.
 
+#include <linux/clk.h>
 #include <asm/unaligned.h>
 #include <linux/acpi.h>
 #include <linux/delay.h>
+#include <linux/gpio/consumer.h>
 #include <linux/i2c.h>
 #include <linux/module.h>
 #include <linux/pm_runtime.h>
+#include <linux/regulator/consumer.h>
 #include <media/v4l2-ctrls.h>
 #include <media/v4l2-device.h>
 #include <media/v4l2-fwnode.h>
@@ -19,6 +22,7 @@
 #define OV8856_LINK_FREQ_180MHZ		180000000ULL
 #define OV8856_SCLK			144000000ULL
 #define OV8856_MCLK			19200000
+#define OV8856_XVCLK_FREQ		24000000
 #define OV8856_DATA_LANES		4
 #define OV8856_RGB_DEPTH		10
 
@@ -29,6 +33,18 @@
 #define OV8856_MODE_STANDBY		0x00
 #define OV8856_MODE_STREAMING		0x01
 
+/* define 1B module */
+#define OV8856_1B_MODULE		0x02
+
+/* otp sram register */
+#define OV8856_OTP_REG			0x700f
+#define OV8856_OTP_REG_ONE		0x3d84
+#define OV8856_OTP_REG_TWO		0x3d81
+
+/* clock register */
+#define OV8856_CLK_REG			0x3614
+#define OV8856_CLK_REG_1B_VAL		0x20
+
 /* vertical-timings from sensor */
 #define OV8856_REG_VTS			0x380e
 #define OV8856_VTS_MAX			0x7fff
@@ -64,6 +80,14 @@
 
 #define to_ov8856(_sd)			container_of(_sd, struct ov8856, sd)
 
+static const char * const ov8856_supply_names[] = {
+	"dovdd",	/* Digital I/O power */
+	"avdd",		/* Analog power */
+	"dvdd",		/* Digital core power */
+};
+
+#define OV8856_NUM_SUPPLIES ARRAY_SIZE(ov8856_supply_names)
+
 enum {
 	OV8856_LINK_FREQ_720MBPS,
 	OV8856_LINK_FREQ_360MBPS,
@@ -316,6 +340,208 @@ static const struct ov8856_reg mode_3280x2464_regs[] = {
 	{0x5e00, 0x00}
 };
 
+static const struct ov8856_reg mode_3264x2448_regs[] = {
+	{0x0103, 0x01},
+	{0x0302, 0x3c},
+	{0x0303, 0x01},
+	{0x031e, 0x0c},
+	{0x3000, 0x00},
+	{0x300e, 0x00},
+	{0x3010, 0x00},
+	{0x3015, 0x84},
+	{0x3018, 0x72},
+	{0x3021, 0x23},
+	{0x3033, 0x24},
+	{0x3500, 0x00},
+	{0x3501, 0x9a},
+	{0x3502, 0x20},
+	{0x3503, 0x08},
+	{0x3505, 0x83},
+	{0x3508, 0x01},
+	{0x3509, 0x80},
+	{0x350c, 0x00},
+	{0x350d, 0x80},
+	{0x350e, 0x04},
+	{0x350f, 0x00},
+	{0x3510, 0x00},
+	{0x3511, 0x02},
+	{0x3512, 0x00},
+	{0x3600, 0x72},
+	{0x3601, 0x40},
+	{0x3602, 0x30},
+	{0x3610, 0xc5},
+	{0x3611, 0x58},
+	{0x3612, 0x5c},
+	{0x3613, 0xca},
+	{0x3614, 0x60},
+	{0x3628, 0xff},
+	{0x3629, 0xff},
+	{0x362a, 0xff},
+	{0x3633, 0x10},
+	{0x3634, 0x10},
+	{0x3635, 0x10},
+	{0x3636, 0x10},
+	{0x3663, 0x08},
+	{0x3669, 0x34},
+	{0x366d, 0x00},
+	{0x366e, 0x10},
+	{0x3706, 0x86},
+	{0x370b, 0x7e},
+	{0x3714, 0x23},
+	{0x3730, 0x12},
+	{0x3733, 0x10},
+	{0x3764, 0x00},
+	{0x3765, 0x00},
+	{0x3769, 0x62},
+	{0x376a, 0x2a},
+	{0x376b, 0x30},
+	{0x3780, 0x00},
+	{0x3781, 0x24},
+	{0x3782, 0x00},
+	{0x3783, 0x23},
+	{0x3798, 0x2f},
+	{0x37a1, 0x60},
+	{0x37a8, 0x6a},
+	{0x37ab, 0x3f},
+	{0x37c2, 0x04},
+	{0x37c3, 0xf1},
+	{0x37c9, 0x80},
+	{0x37cb, 0x16},
+	{0x37cc, 0x16},
+	{0x37cd, 0x16},
+	{0x37ce, 0x16},
+	{0x3800, 0x00},
+	{0x3801, 0x00},
+	{0x3802, 0x00},
+	{0x3803, 0x0c},
+	{0x3804, 0x0c},
+	{0x3805, 0xdf},
+	{0x3806, 0x09},
+	{0x3807, 0xa3},
+	{0x3808, 0x0c},
+	{0x3809, 0xc0},
+	{0x380a, 0x09},
+	{0x380b, 0x90},
+	{0x380c, 0x07},
+	{0x380d, 0x8c},
+	{0x380e, 0x09},
+	{0x380f, 0xb2},
+	{0x3810, 0x00},
+	{0x3811, 0x04},
+	{0x3812, 0x00},
+	{0x3813, 0x02},
+	{0x3814, 0x01},
+	{0x3815, 0x01},
+	{0x3816, 0x00},
+	{0x3817, 0x00},
+	{0x3818, 0x00},
+	{0x3819, 0x00},
+	{0x3820, 0x80},
+	{0x3821, 0x46},
+	{0x382a, 0x01},
+	{0x382b, 0x01},
+	{0x3830, 0x06},
+	{0x3836, 0x02},
+	{0x3862, 0x04},
+	{0x3863, 0x08},
+	{0x3cc0, 0x33},
+	{0x3d85, 0x17},
+	{0x3d8c, 0x73},
+	{0x3d8d, 0xde},
+	{0x4001, 0xe0},
+	{0x4003, 0x40},
+	{0x4008, 0x00},
+	{0x4009, 0x0b},
+	{0x400a, 0x00},
+	{0x400b, 0x84},
+	{0x400f, 0x80},
+	{0x4010, 0xf0},
+	{0x4011, 0xff},
+	{0x4012, 0x02},
+	{0x4013, 0x01},
+	{0x4014, 0x01},
+	{0x4015, 0x01},
+	{0x4042, 0x00},
+	{0x4043, 0x80},
+	{0x4044, 0x00},
+	{0x4045, 0x80},
+	{0x4046, 0x00},
+	{0x4047, 0x80},
+	{0x4048, 0x00},
+	{0x4049, 0x80},
+	{0x4041, 0x03},
+	{0x404c, 0x20},
+	{0x404d, 0x00},
+	{0x404e, 0x20},
+	{0x4203, 0x80},
+	{0x4307, 0x30},
+	{0x4317, 0x00},
+	{0x4502, 0x50},
+	{0x4503, 0x08},
+	{0x4601, 0x80},
+	{0x4800, 0x44},
+	{0x4816, 0x53},
+	{0x481b, 0x50},
+	{0x481f, 0x27},
+	{0x4823, 0x3c},
+	{0x482b, 0x00},
+	{0x4831, 0x66},
+	{0x4837, 0x16},
+	{0x483c, 0x0f},
+	{0x484b, 0x05},
+	{0x5000, 0x77},
+	{0x5001, 0x0a},
+	{0x5003, 0xc8},
+	{0x5004, 0x04},
+	{0x5006, 0x00},
+	{0x5007, 0x00},
+	{0x502e, 0x03},
+	{0x5030, 0x41},
+	{0x5780, 0x14},
+	{0x5781, 0x0f},
+	{0x5782, 0x44},
+	{0x5783, 0x02},
+	{0x5784, 0x01},
+	{0x5785, 0x01},
+	{0x5786, 0x00},
+	{0x5787, 0x04},
+	{0x5788, 0x02},
+	{0x5789, 0x0f},
+	{0x578a, 0xfd},
+	{0x578b, 0xf5},
+	{0x578c, 0xf5},
+	{0x578d, 0x03},
+	{0x578e, 0x08},
+	{0x578f, 0x0c},
+	{0x5790, 0x08},
+	{0x5791, 0x04},
+	{0x5792, 0x00},
+	{0x5793, 0x52},
+	{0x5794, 0xa3},
+	{0x5795, 0x02},
+	{0x5796, 0x20},
+	{0x5797, 0x20},
+	{0x5798, 0xd5},
+	{0x5799, 0xd5},
+	{0x579a, 0x00},
+	{0x579b, 0x50},
+	{0x579c, 0x00},
+	{0x579d, 0x2c},
+	{0x579e, 0x0c},
+	{0x579f, 0x40},
+	{0x57a0, 0x09},
+	{0x57a1, 0x40},
+	{0x59f8, 0x3d},
+	{0x5a08, 0x02},
+	{0x5b00, 0x02},
+	{0x5b01, 0x10},
+	{0x5b02, 0x03},
+	{0x5b03, 0xcf},
+	{0x5b05, 0x6c},
+	{0x5e00, 0x00},
+	{0x5e10, 0xfc}
+};
+
 static const struct ov8856_reg mode_1640x1232_regs[] = {
 	{0x3000, 0x20},
 	{0x3003, 0x08},
@@ -506,6 +732,208 @@ static const struct ov8856_reg mode_1640x1232_regs[] = {
 	{0x5e00, 0x00}
 };
 
+static const struct ov8856_reg mode_1632x1224_regs[] = {
+	{0x0103, 0x01},
+	{0x0302, 0x3c},
+	{0x0303, 0x01},
+	{0x031e, 0x0c},
+	{0x3000, 0x00},
+	{0x300e, 0x00},
+	{0x3010, 0x00},
+	{0x3015, 0x84},
+	{0x3018, 0x72},
+	{0x3021, 0x23},
+	{0x3033, 0x24},
+	{0x3500, 0x00},
+	{0x3501, 0x4c},
+	{0x3502, 0xe0},
+	{0x3503, 0x08},
+	{0x3505, 0x83},
+	{0x3508, 0x01},
+	{0x3509, 0x80},
+	{0x350c, 0x00},
+	{0x350d, 0x80},
+	{0x350e, 0x04},
+	{0x350f, 0x00},
+	{0x3510, 0x00},
+	{0x3511, 0x02},
+	{0x3512, 0x00},
+	{0x3600, 0x72},
+	{0x3601, 0x40},
+	{0x3602, 0x30},
+	{0x3610, 0xc5},
+	{0x3611, 0x58},
+	{0x3612, 0x5c},
+	{0x3613, 0xca},
+	{0x3614, 0x60},
+	{0x3628, 0xff},
+	{0x3629, 0xff},
+	{0x362a, 0xff},
+	{0x3633, 0x10},
+	{0x3634, 0x10},
+	{0x3635, 0x10},
+	{0x3636, 0x10},
+	{0x3663, 0x08},
+	{0x3669, 0x34},
+	{0x366d, 0x00},
+	{0x366e, 0x08},
+	{0x3706, 0x86},
+	{0x370b, 0x7e},
+	{0x3714, 0x27},
+	{0x3730, 0x12},
+	{0x3733, 0x10},
+	{0x3764, 0x00},
+	{0x3765, 0x00},
+	{0x3769, 0x62},
+	{0x376a, 0x2a},
+	{0x376b, 0x30},
+	{0x3780, 0x00},
+	{0x3781, 0x24},
+	{0x3782, 0x00},
+	{0x3783, 0x23},
+	{0x3798, 0x2f},
+	{0x37a1, 0x60},
+	{0x37a8, 0x6a},
+	{0x37ab, 0x3f},
+	{0x37c2, 0x14},
+	{0x37c3, 0xf1},
+	{0x37c9, 0x80},
+	{0x37cb, 0x16},
+	{0x37cc, 0x16},
+	{0x37cd, 0x16},
+	{0x37ce, 0x16},
+	{0x3800, 0x00},
+	{0x3801, 0x00},
+	{0x3802, 0x00},
+	{0x3803, 0x0c},
+	{0x3804, 0x0c},
+	{0x3805, 0xdf},
+	{0x3806, 0x09},
+	{0x3807, 0xa3},
+	{0x3808, 0x06},
+	{0x3809, 0x60},
+	{0x380a, 0x04},
+	{0x380b, 0xc8},
+	{0x380c, 0x07},
+	{0x380d, 0x8c},
+	{0x380e, 0x09},
+	{0x380f, 0xb2},
+	{0x3810, 0x00},
+	{0x3811, 0x02},
+	{0x3812, 0x00},
+	{0x3813, 0x02},
+	{0x3814, 0x03},
+	{0x3815, 0x01},
+	{0x3816, 0x00},
+	{0x3817, 0x00},
+	{0x3818, 0x00},
+	{0x3819, 0x00},
+	{0x3820, 0x80},
+	{0x3821, 0x47},
+	{0x382a, 0x03},
+	{0x382b, 0x01},
+	{0x3830, 0x06},
+	{0x3836, 0x02},
+	{0x3862, 0x04},
+	{0x3863, 0x08},
+	{0x3cc0, 0x33},
+	{0x3d85, 0x17},
+	{0x3d8c, 0x73},
+	{0x3d8d, 0xde},
+	{0x4001, 0xe0},
+	{0x4003, 0x40},
+	{0x4008, 0x00},
+	{0x4009, 0x05},
+	{0x400a, 0x00},
+	{0x400b, 0x84},
+	{0x400f, 0x80},
+	{0x4010, 0xf0},
+	{0x4011, 0xff},
+	{0x4012, 0x02},
+	{0x4013, 0x01},
+	{0x4014, 0x01},
+	{0x4015, 0x01},
+	{0x4042, 0x00},
+	{0x4043, 0x80},
+	{0x4044, 0x00},
+	{0x4045, 0x80},
+	{0x4046, 0x00},
+	{0x4047, 0x80},
+	{0x4048, 0x00},
+	{0x4049, 0x80},
+	{0x4041, 0x03},
+	{0x404c, 0x20},
+	{0x404d, 0x00},
+	{0x404e, 0x20},
+	{0x4203, 0x80},
+	{0x4307, 0x30},
+	{0x4317, 0x00},
+	{0x4502, 0x50},
+	{0x4503, 0x08},
+	{0x4601, 0x80},
+	{0x4800, 0x44},
+	{0x4816, 0x53},
+	{0x481b, 0x50},
+	{0x481f, 0x27},
+	{0x4823, 0x3c},
+	{0x482b, 0x00},
+	{0x4831, 0x66},
+	{0x4837, 0x16},
+	{0x483c, 0x0f},
+	{0x484b, 0x05},
+	{0x5000, 0x77},
+	{0x5001, 0x0a},
+	{0x5003, 0xc8},
+	{0x5004, 0x04},
+	{0x5006, 0x00},
+	{0x5007, 0x00},
+	{0x502e, 0x03},
+	{0x5030, 0x41},
+	{0x5795, 0x00},
+	{0x5796, 0x10},
+	{0x5797, 0x10},
+	{0x5798, 0x73},
+	{0x5799, 0x73},
+	{0x579a, 0x00},
+	{0x579b, 0x28},
+	{0x579c, 0x00},
+	{0x579d, 0x16},
+	{0x579e, 0x06},
+	{0x579f, 0x20},
+	{0x57a0, 0x04},
+	{0x57a1, 0xa0},
+	{0x5780, 0x14},
+	{0x5781, 0x0f},
+	{0x5782, 0x44},
+	{0x5783, 0x02},
+	{0x5784, 0x01},
+	{0x5785, 0x01},
+	{0x5786, 0x00},
+	{0x5787, 0x04},
+	{0x5788, 0x02},
+	{0x5789, 0x0f},
+	{0x578a, 0xfd},
+	{0x578b, 0xf5},
+	{0x578c, 0xf5},
+	{0x578d, 0x03},
+	{0x578e, 0x08},
+	{0x578f, 0x0c},
+	{0x5790, 0x08},
+	{0x5791, 0x04},
+	{0x5792, 0x00},
+	{0x5793, 0x52},
+	{0x5794, 0xa3},
+	{0x59f8, 0x3d},
+	{0x5a08, 0x02},
+	{0x5b00, 0x02},
+	{0x5b01, 0x10},
+	{0x5b02, 0x03},
+	{0x5b03, 0xcf},
+	{0x5b05, 0x6c},
+	{0x5e00, 0x00},
+	{0x5e10, 0xfc}
+};
+
 static const char * const ov8856_test_pattern_menu[] = {
 	"Disabled",
 	"Standard Color Bar",
@@ -548,6 +976,18 @@ static const struct ov8856_mode supported_modes[] = {
 		.link_freq_index = OV8856_LINK_FREQ_720MBPS,
 	},
 	{
+		.width    = 3264,
+		.height   = 2448,
+		.hts      = 1932,
+		.vts_def  = 2482,
+		.vts_min  = 2482,
+		.reg_list = {
+			.num_of_regs = ARRAY_SIZE(mode_3264x2448_regs),
+			.regs = mode_3264x2448_regs,
+		},
+		.link_freq_index = OV8856_LINK_FREQ_720MBPS,
+	},
+	{
 		.width = 1640,
 		.height = 1232,
 		.hts = 3820,
@@ -558,6 +998,18 @@ static const struct ov8856_mode supported_modes[] = {
 			.regs = mode_1640x1232_regs,
 		},
 		.link_freq_index = OV8856_LINK_FREQ_360MBPS,
+	},
+	{
+		.width    = 1632,
+		.height   = 1224,
+		.hts      = 1932,
+		.vts_def  = 2482,
+		.vts_min  = 2482,
+		.reg_list = {
+			.num_of_regs = ARRAY_SIZE(mode_1632x1224_regs),
+			.regs = mode_1632x1224_regs,
+		},
+		.link_freq_index = OV8856_LINK_FREQ_360MBPS,
 	}
 };
 
@@ -566,6 +1018,10 @@ struct ov8856 {
 	struct media_pad pad;
 	struct v4l2_ctrl_handler ctrl_handler;
 
+	struct clk		*xvclk;
+	struct gpio_desc	*reset_gpio;
+	struct regulator_bulk_data supplies[OV8856_NUM_SUPPLIES];
+
 	/* V4L2 Controls */
 	struct v4l2_ctrl *link_freq;
 	struct v4l2_ctrl *pixel_rate;
@@ -576,6 +1032,9 @@ struct ov8856 {
 	/* Current mode */
 	const struct ov8856_mode *cur_mode;
 
+	/* module hardware version */
+	bool is_1B_module;
+
 	/* To serialize asynchronus callbacks */
 	struct mutex mutex;
 
@@ -696,6 +1155,24 @@ static int ov8856_test_pattern(struct ov8856 *ov8856, u32 pattern)
 				OV8856_REG_VALUE_08BIT, pattern);
 }
 
+static int ov8856_update_otp_reg(struct ov8856 *ov8856)
+{
+	int ret;
+
+	ret = ov8856_write_reg(ov8856, OV8856_REG_MODE_SELECT,
+			       OV8856_REG_VALUE_08BIT, OV8856_MODE_STREAMING);
+	if (ret)
+		return ret;
+
+	ret = ov8856_write_reg(ov8856, OV8856_OTP_REG_ONE,
+			       OV8856_REG_VALUE_08BIT, OV8856_MODE_STANDBY);
+	if (ret)
+		return ret;
+
+	return ov8856_write_reg(ov8856, OV8856_OTP_REG_TWO,
+				OV8856_REG_VALUE_08BIT, OV8856_MODE_STREAMING);
+}
+
 static int ov8856_set_ctrl(struct v4l2_ctrl *ctrl)
 {
 	struct ov8856 *ov8856 = container_of(ctrl->handler,
@@ -825,7 +1302,13 @@ static void ov8856_update_pad_format(const struct ov8856_mode *mode,
 {
 	fmt->width = mode->width;
 	fmt->height = mode->height;
-	fmt->code = MEDIA_BUS_FMT_SGRBG10_1X10;
+
+	/* Bayer Order is determined by image resolution */
+	if (fmt->width == 3264 || fmt->width == 1632)
+		fmt->code = MEDIA_BUS_FMT_SBGGR10_1X10;
+	else
+		fmt->code = MEDIA_BUS_FMT_SGRBG10_1X10;
+
 	fmt->field = V4L2_FIELD_NONE;
 }
 
@@ -850,6 +1333,17 @@ static int ov8856_start_streaming(struct ov8856 *ov8856)
 		return ret;
 	}
 
+	/* update R3614 for 1B module */
+	if (ov8856->is_1B_module) {
+		ret = ov8856_write_reg(ov8856, OV8856_CLK_REG,
+				       OV8856_REG_VALUE_08BIT,
+				       OV8856_CLK_REG_1B_VAL);
+		if (ret) {
+			dev_err(&client->dev, "failed to set R3614");
+			return ret;
+		}
+	}
+
 	ret = __v4l2_ctrl_handler_setup(ov8856->sd.ctrl_handler);
 	if (ret)
 		return ret;
@@ -882,6 +1376,8 @@ static int ov8856_set_stream(struct v4l2_subdev *sd, int enable)
 	if (ov8856->streaming == enable)
 		return 0;
 
+	dev_dbg(&client->dev, "hardware version: (%d)\n", ov8856->is_1B_module);
+
 	mutex_lock(&ov8856->mutex);
 	if (enable) {
 		ret = pm_runtime_get_sync(&client->dev);
@@ -908,6 +1404,54 @@ static int ov8856_set_stream(struct v4l2_subdev *sd, int enable)
 	return ret;
 }
 
+/* Calculate the delay in us by clock rate and clock cycles */
+static inline u32 ov8856_cal_delay(u32 cycles)
+{
+	return DIV_ROUND_UP(cycles, OV8856_XVCLK_FREQ / 1000 / 1000);
+}
+
+static int __ov8856_power_on(struct ov8856 *ov8856)
+{
+	int ret;
+	u32 delay_us;
+	struct i2c_client *client = v4l2_get_subdevdata(&ov8856->sd);
+
+	ret = clk_prepare_enable(ov8856->xvclk);
+	if (ret < 0) {
+		dev_err(&client->dev, "Failed to enable xvclk\n");
+		return ret;
+	}
+
+	gpiod_set_value_cansleep(ov8856->reset_gpio, 1);
+
+	ret = regulator_bulk_enable(OV8856_NUM_SUPPLIES, ov8856->supplies);
+	if (ret < 0) {
+		dev_err(&client->dev, "Failed to enable regulators\n");
+		goto disable_clk;
+	}
+
+	gpiod_set_value_cansleep(ov8856->reset_gpio, 0);
+
+	/* 8192 cycles prior to first SCCB transaction */
+	delay_us = ov8856_cal_delay(8192);
+	usleep_range(delay_us  * 2, delay_us * 4);
+
+	return 0;
+
+disable_clk:
+	clk_disable_unprepare(ov8856->xvclk);
+
+	return ret;
+}
+
+static void __ov8856_power_off(struct ov8856 *ov8856)
+{
+	clk_disable_unprepare(ov8856->xvclk);
+	gpiod_set_value_cansleep(ov8856->reset_gpio, 1);
+
+	regulator_bulk_disable(OV8856_NUM_SUPPLIES, ov8856->supplies);
+}
+
 static int __maybe_unused ov8856_suspend(struct device *dev)
 {
 	struct i2c_client *client = to_i2c_client(dev);
@@ -915,8 +1459,8 @@ static int __maybe_unused ov8856_suspend(struct device *dev)
 	struct ov8856 *ov8856 = to_ov8856(sd);
 
 	mutex_lock(&ov8856->mutex);
-	if (ov8856->streaming)
-		ov8856_stop_streaming(ov8856);
+
+	__ov8856_power_off(ov8856);
 
 	mutex_unlock(&ov8856->mutex);
 
@@ -1089,6 +1633,20 @@ static int ov8856_identify_module(struct ov8856 *ov8856)
 		return -ENXIO;
 	}
 
+	/* set R3614 to distinguish harward versions */
+	ret = ov8856_update_otp_reg(ov8856);
+	if (ret) {
+		dev_err(&client->dev, "failed to set otp register");
+		return ret;
+	}
+
+	ret = ov8856_read_reg(ov8856, OV8856_OTP_REG,
+			      OV8856_REG_VALUE_08BIT, &val);
+	if (ret)
+		return ret;
+
+	ov8856->is_1B_module = (val == OV8856_1B_MODULE) ? 1 : 0;
+
 	return 0;
 }
 
@@ -1164,11 +1722,27 @@ static int ov8856_remove(struct i2c_client *client)
 	media_entity_cleanup(&sd->entity);
 	v4l2_ctrl_handler_free(sd->ctrl_handler);
 	pm_runtime_disable(&client->dev);
+	if (!pm_runtime_status_suspended(&client->dev))
+		__ov8856_power_off(ov8856);
+	pm_runtime_set_suspended(&client->dev);
 	mutex_destroy(&ov8856->mutex);
 
 	return 0;
 }
 
+static int ov8856_configure_regulators(struct ov8856 *ov8856)
+{
+	struct i2c_client *client = v4l2_get_subdevdata(&ov8856->sd);
+	int i;
+
+	for (i = 0; i < OV8856_NUM_SUPPLIES; i++)
+		ov8856->supplies[i].supply = ov8856_supply_names[i];
+
+	return devm_regulator_bulk_get(&client->dev,
+				       OV8856_NUM_SUPPLIES,
+				       ov8856->supplies);
+}
+
 static int ov8856_probe(struct i2c_client *client)
 {
 	struct ov8856 *ov8856;
@@ -1186,6 +1760,40 @@ static int ov8856_probe(struct i2c_client *client)
 		return -ENOMEM;
 
 	v4l2_i2c_subdev_init(&ov8856->sd, client, &ov8856_subdev_ops);
+
+	ov8856->xvclk = devm_clk_get(&client->dev, "xvclk");
+	if (IS_ERR(ov8856->xvclk)) {
+		dev_err(&client->dev, "Failed to get xvclk\n");
+		return -EINVAL;
+	}
+
+	ret = clk_set_rate(ov8856->xvclk, OV8856_XVCLK_FREQ);
+	if (ret < 0) {
+		dev_err(&client->dev, "Failed to set xvclk rate (24MHz)\n");
+		return ret;
+	}
+	if (clk_get_rate(ov8856->xvclk) != OV8856_XVCLK_FREQ)
+		dev_warn(&client->dev,
+			 "xvclk mismatched, modes are based on 24MHz\n");
+
+	ov8856->reset_gpio = devm_gpiod_get(&client->dev,
+					    "reset",
+					    GPIOD_OUT_LOW);
+	if (IS_ERR(ov8856->reset_gpio)) {
+		dev_err(&client->dev, "Failed to get reset-gpios\n");
+		return -EINVAL;
+	}
+
+	ret = ov8856_configure_regulators(ov8856);
+	if (ret) {
+		dev_err(&client->dev, "Failed to get power regulators\n");
+		return ret;
+	}
+
+	ret = __ov8856_power_on(ov8856);
+	if (ret)
+		goto probe_error_v4l2_ctrl_handler_free;
+
 	ret = ov8856_identify_module(ov8856);
 	if (ret) {
 		dev_err(&client->dev, "failed to find sensor: %d", ret);
@@ -1251,11 +1859,21 @@ static const struct acpi_device_id ov8856_acpi_ids[] = {
 MODULE_DEVICE_TABLE(acpi, ov8856_acpi_ids);
 #endif
 
+#if IS_ENABLED(CONFIG_OF)
+static const struct of_device_id ov8856_of_match[] = {
+	{ .compatible = "ovti,ov8856" },
+	{},
+};
+
+MODULE_DEVICE_TABLE(of, ov8856_of_match);
+#endif
+
 static struct i2c_driver ov8856_i2c_driver = {
 	.driver = {
 		.name = "ov8856",
 		.pm = &ov8856_pm_ops,
 		.acpi_match_table = ACPI_PTR(ov8856_acpi_ids),
+		.of_match_table = of_match_ptr(ov8856_of_match),
 	},
 	.probe_new = ov8856_probe,
 	.remove = ov8856_remove,
-- 
2.9.2


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

* Re: [V1, 2/2] media: i2c: Add more sensor mode for ov8856 camera sensor
  2019-08-08  9:22 ` [V1, 2/2] media: i2c: Add more sensor mode for ov8856 camera sensor dongchun.zhu
@ 2019-08-08 13:53   ` Sakari Ailus
  0 siblings, 0 replies; 6+ messages in thread
From: Sakari Ailus @ 2019-08-08 13:53 UTC (permalink / raw)
  To: dongchun.zhu
  Cc: mchehab, robh+dt, mark.rutland, sakari.ailus, drinkcat, tfiga,
	matthias.bgg, bingbu.cao, srv_heupstream, linux-mediatek,
	linux-arm-kernel, sj.huang, linux-media, devicetree, louis.kuo,
	shengnan.wang

Hi Dongchun,

Thanks for the patch.

On Thu, Aug 08, 2019 at 05:22:15PM +0800, dongchun.zhu@mediatek.com wrote:
> From: Dongchun Zhu <dongchun.zhu@mediatek.com>
> 
> This patch mainly adds two more sensor modes for OV8856 image sensor.
> The OV8856 driver currently supports output format: 10-bit Raw,
> the resolution of 1632*1224 and 3264*2448, and the bayer order of BGGR.
> The hardware version also differs in some OTP regiser,
> as well as PLL register setting.
> 
> Signed-off-by: Dongchun Zhu <dongchun.zhu@mediatek.com>
> ---
>  drivers/media/i2c/ov8856.c | 624 ++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 621 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/i2c/ov8856.c b/drivers/media/i2c/ov8856.c
> index cd347d6..e0610b6 100644
> --- a/drivers/media/i2c/ov8856.c
> +++ b/drivers/media/i2c/ov8856.c
> @@ -1,12 +1,15 @@
>  // SPDX-License-Identifier: GPL-2.0
>  // Copyright (c) 2019 Intel Corporation.
>  
> +#include <linux/clk.h>
>  #include <asm/unaligned.h>
>  #include <linux/acpi.h>
>  #include <linux/delay.h>
> +#include <linux/gpio/consumer.h>
>  #include <linux/i2c.h>
>  #include <linux/module.h>
>  #include <linux/pm_runtime.h>
> +#include <linux/regulator/consumer.h>
>  #include <media/v4l2-ctrls.h>
>  #include <media/v4l2-device.h>
>  #include <media/v4l2-fwnode.h>
> @@ -19,6 +22,7 @@
>  #define OV8856_LINK_FREQ_180MHZ		180000000ULL
>  #define OV8856_SCLK			144000000ULL
>  #define OV8856_MCLK			19200000
> +#define OV8856_XVCLK_FREQ		24000000

The driver currenctly uses, perhaps misleadingly, OV8856_MCLK for this
purpose. You could rename the existing MCLK as XVCLK.

This also means the driver needs to differentiate configurations for 24 and
19,2 MHz which it currently does not do. I think it may make sense to make
this a separate patch from the rest.

>  #define OV8856_DATA_LANES		4
>  #define OV8856_RGB_DEPTH		10
>  
> @@ -29,6 +33,18 @@
>  #define OV8856_MODE_STANDBY		0x00
>  #define OV8856_MODE_STREAMING		0x01
>  
> +/* define 1B module */
> +#define OV8856_1B_MODULE		0x02
> +
> +/* otp sram register */
> +#define OV8856_OTP_REG			0x700f
> +#define OV8856_OTP_REG_ONE		0x3d84
> +#define OV8856_OTP_REG_TWO		0x3d81
> +
> +/* clock register */
> +#define OV8856_CLK_REG			0x3614
> +#define OV8856_CLK_REG_1B_VAL		0x20
> +
>  /* vertical-timings from sensor */
>  #define OV8856_REG_VTS			0x380e
>  #define OV8856_VTS_MAX			0x7fff
> @@ -64,6 +80,14 @@
>  
>  #define to_ov8856(_sd)			container_of(_sd, struct ov8856, sd)
>  
> +static const char * const ov8856_supply_names[] = {
> +	"dovdd",	/* Digital I/O power */
> +	"avdd",		/* Analog power */
> +	"dvdd",		/* Digital core power */
> +};
> +
> +#define OV8856_NUM_SUPPLIES ARRAY_SIZE(ov8856_supply_names)
> +
>  enum {
>  	OV8856_LINK_FREQ_720MBPS,
>  	OV8856_LINK_FREQ_360MBPS,
> @@ -316,6 +340,208 @@ static const struct ov8856_reg mode_3280x2464_regs[] = {
>  	{0x5e00, 0x00}
>  };
>  
> +static const struct ov8856_reg mode_3264x2448_regs[] = {
> +	{0x0103, 0x01},
> +	{0x0302, 0x3c},
> +	{0x0303, 0x01},
> +	{0x031e, 0x0c},
> +	{0x3000, 0x00},
> +	{0x300e, 0x00},
> +	{0x3010, 0x00},
> +	{0x3015, 0x84},
> +	{0x3018, 0x72},
> +	{0x3021, 0x23},
> +	{0x3033, 0x24},
> +	{0x3500, 0x00},
> +	{0x3501, 0x9a},
> +	{0x3502, 0x20},
> +	{0x3503, 0x08},
> +	{0x3505, 0x83},
> +	{0x3508, 0x01},
> +	{0x3509, 0x80},
> +	{0x350c, 0x00},
> +	{0x350d, 0x80},
> +	{0x350e, 0x04},
> +	{0x350f, 0x00},
> +	{0x3510, 0x00},
> +	{0x3511, 0x02},
> +	{0x3512, 0x00},
> +	{0x3600, 0x72},
> +	{0x3601, 0x40},
> +	{0x3602, 0x30},
> +	{0x3610, 0xc5},
> +	{0x3611, 0x58},
> +	{0x3612, 0x5c},
> +	{0x3613, 0xca},
> +	{0x3614, 0x60},
> +	{0x3628, 0xff},
> +	{0x3629, 0xff},
> +	{0x362a, 0xff},
> +	{0x3633, 0x10},
> +	{0x3634, 0x10},
> +	{0x3635, 0x10},
> +	{0x3636, 0x10},
> +	{0x3663, 0x08},
> +	{0x3669, 0x34},
> +	{0x366d, 0x00},
> +	{0x366e, 0x10},
> +	{0x3706, 0x86},
> +	{0x370b, 0x7e},
> +	{0x3714, 0x23},
> +	{0x3730, 0x12},
> +	{0x3733, 0x10},
> +	{0x3764, 0x00},
> +	{0x3765, 0x00},
> +	{0x3769, 0x62},
> +	{0x376a, 0x2a},
> +	{0x376b, 0x30},
> +	{0x3780, 0x00},
> +	{0x3781, 0x24},
> +	{0x3782, 0x00},
> +	{0x3783, 0x23},
> +	{0x3798, 0x2f},
> +	{0x37a1, 0x60},
> +	{0x37a8, 0x6a},
> +	{0x37ab, 0x3f},
> +	{0x37c2, 0x04},
> +	{0x37c3, 0xf1},
> +	{0x37c9, 0x80},
> +	{0x37cb, 0x16},
> +	{0x37cc, 0x16},
> +	{0x37cd, 0x16},
> +	{0x37ce, 0x16},
> +	{0x3800, 0x00},
> +	{0x3801, 0x00},
> +	{0x3802, 0x00},
> +	{0x3803, 0x0c},
> +	{0x3804, 0x0c},
> +	{0x3805, 0xdf},
> +	{0x3806, 0x09},
> +	{0x3807, 0xa3},
> +	{0x3808, 0x0c},
> +	{0x3809, 0xc0},
> +	{0x380a, 0x09},
> +	{0x380b, 0x90},
> +	{0x380c, 0x07},
> +	{0x380d, 0x8c},
> +	{0x380e, 0x09},
> +	{0x380f, 0xb2},
> +	{0x3810, 0x00},
> +	{0x3811, 0x04},
> +	{0x3812, 0x00},
> +	{0x3813, 0x02},
> +	{0x3814, 0x01},
> +	{0x3815, 0x01},
> +	{0x3816, 0x00},
> +	{0x3817, 0x00},
> +	{0x3818, 0x00},
> +	{0x3819, 0x00},
> +	{0x3820, 0x80},
> +	{0x3821, 0x46},
> +	{0x382a, 0x01},
> +	{0x382b, 0x01},
> +	{0x3830, 0x06},
> +	{0x3836, 0x02},
> +	{0x3862, 0x04},
> +	{0x3863, 0x08},
> +	{0x3cc0, 0x33},
> +	{0x3d85, 0x17},
> +	{0x3d8c, 0x73},
> +	{0x3d8d, 0xde},
> +	{0x4001, 0xe0},
> +	{0x4003, 0x40},
> +	{0x4008, 0x00},
> +	{0x4009, 0x0b},
> +	{0x400a, 0x00},
> +	{0x400b, 0x84},
> +	{0x400f, 0x80},
> +	{0x4010, 0xf0},
> +	{0x4011, 0xff},
> +	{0x4012, 0x02},
> +	{0x4013, 0x01},
> +	{0x4014, 0x01},
> +	{0x4015, 0x01},
> +	{0x4042, 0x00},
> +	{0x4043, 0x80},
> +	{0x4044, 0x00},
> +	{0x4045, 0x80},
> +	{0x4046, 0x00},
> +	{0x4047, 0x80},
> +	{0x4048, 0x00},
> +	{0x4049, 0x80},
> +	{0x4041, 0x03},
> +	{0x404c, 0x20},
> +	{0x404d, 0x00},
> +	{0x404e, 0x20},
> +	{0x4203, 0x80},
> +	{0x4307, 0x30},
> +	{0x4317, 0x00},
> +	{0x4502, 0x50},
> +	{0x4503, 0x08},
> +	{0x4601, 0x80},
> +	{0x4800, 0x44},
> +	{0x4816, 0x53},
> +	{0x481b, 0x50},
> +	{0x481f, 0x27},
> +	{0x4823, 0x3c},
> +	{0x482b, 0x00},
> +	{0x4831, 0x66},
> +	{0x4837, 0x16},
> +	{0x483c, 0x0f},
> +	{0x484b, 0x05},
> +	{0x5000, 0x77},
> +	{0x5001, 0x0a},
> +	{0x5003, 0xc8},
> +	{0x5004, 0x04},
> +	{0x5006, 0x00},
> +	{0x5007, 0x00},
> +	{0x502e, 0x03},
> +	{0x5030, 0x41},
> +	{0x5780, 0x14},
> +	{0x5781, 0x0f},
> +	{0x5782, 0x44},
> +	{0x5783, 0x02},
> +	{0x5784, 0x01},
> +	{0x5785, 0x01},
> +	{0x5786, 0x00},
> +	{0x5787, 0x04},
> +	{0x5788, 0x02},
> +	{0x5789, 0x0f},
> +	{0x578a, 0xfd},
> +	{0x578b, 0xf5},
> +	{0x578c, 0xf5},
> +	{0x578d, 0x03},
> +	{0x578e, 0x08},
> +	{0x578f, 0x0c},
> +	{0x5790, 0x08},
> +	{0x5791, 0x04},
> +	{0x5792, 0x00},
> +	{0x5793, 0x52},
> +	{0x5794, 0xa3},
> +	{0x5795, 0x02},
> +	{0x5796, 0x20},
> +	{0x5797, 0x20},
> +	{0x5798, 0xd5},
> +	{0x5799, 0xd5},
> +	{0x579a, 0x00},
> +	{0x579b, 0x50},
> +	{0x579c, 0x00},
> +	{0x579d, 0x2c},
> +	{0x579e, 0x0c},
> +	{0x579f, 0x40},
> +	{0x57a0, 0x09},
> +	{0x57a1, 0x40},
> +	{0x59f8, 0x3d},
> +	{0x5a08, 0x02},
> +	{0x5b00, 0x02},
> +	{0x5b01, 0x10},
> +	{0x5b02, 0x03},
> +	{0x5b03, 0xcf},
> +	{0x5b05, 0x6c},
> +	{0x5e00, 0x00},
> +	{0x5e10, 0xfc}
> +};
> +
>  static const struct ov8856_reg mode_1640x1232_regs[] = {
>  	{0x3000, 0x20},
>  	{0x3003, 0x08},
> @@ -506,6 +732,208 @@ static const struct ov8856_reg mode_1640x1232_regs[] = {
>  	{0x5e00, 0x00}
>  };
>  
> +static const struct ov8856_reg mode_1632x1224_regs[] = {
> +	{0x0103, 0x01},
> +	{0x0302, 0x3c},
> +	{0x0303, 0x01},
> +	{0x031e, 0x0c},
> +	{0x3000, 0x00},
> +	{0x300e, 0x00},
> +	{0x3010, 0x00},
> +	{0x3015, 0x84},
> +	{0x3018, 0x72},
> +	{0x3021, 0x23},
> +	{0x3033, 0x24},
> +	{0x3500, 0x00},
> +	{0x3501, 0x4c},
> +	{0x3502, 0xe0},
> +	{0x3503, 0x08},
> +	{0x3505, 0x83},
> +	{0x3508, 0x01},
> +	{0x3509, 0x80},
> +	{0x350c, 0x00},
> +	{0x350d, 0x80},
> +	{0x350e, 0x04},
> +	{0x350f, 0x00},
> +	{0x3510, 0x00},
> +	{0x3511, 0x02},
> +	{0x3512, 0x00},
> +	{0x3600, 0x72},
> +	{0x3601, 0x40},
> +	{0x3602, 0x30},
> +	{0x3610, 0xc5},
> +	{0x3611, 0x58},
> +	{0x3612, 0x5c},
> +	{0x3613, 0xca},
> +	{0x3614, 0x60},
> +	{0x3628, 0xff},
> +	{0x3629, 0xff},
> +	{0x362a, 0xff},
> +	{0x3633, 0x10},
> +	{0x3634, 0x10},
> +	{0x3635, 0x10},
> +	{0x3636, 0x10},
> +	{0x3663, 0x08},
> +	{0x3669, 0x34},
> +	{0x366d, 0x00},
> +	{0x366e, 0x08},
> +	{0x3706, 0x86},
> +	{0x370b, 0x7e},
> +	{0x3714, 0x27},
> +	{0x3730, 0x12},
> +	{0x3733, 0x10},
> +	{0x3764, 0x00},
> +	{0x3765, 0x00},
> +	{0x3769, 0x62},
> +	{0x376a, 0x2a},
> +	{0x376b, 0x30},
> +	{0x3780, 0x00},
> +	{0x3781, 0x24},
> +	{0x3782, 0x00},
> +	{0x3783, 0x23},
> +	{0x3798, 0x2f},
> +	{0x37a1, 0x60},
> +	{0x37a8, 0x6a},
> +	{0x37ab, 0x3f},
> +	{0x37c2, 0x14},
> +	{0x37c3, 0xf1},
> +	{0x37c9, 0x80},
> +	{0x37cb, 0x16},
> +	{0x37cc, 0x16},
> +	{0x37cd, 0x16},
> +	{0x37ce, 0x16},
> +	{0x3800, 0x00},
> +	{0x3801, 0x00},
> +	{0x3802, 0x00},
> +	{0x3803, 0x0c},
> +	{0x3804, 0x0c},
> +	{0x3805, 0xdf},
> +	{0x3806, 0x09},
> +	{0x3807, 0xa3},
> +	{0x3808, 0x06},
> +	{0x3809, 0x60},
> +	{0x380a, 0x04},
> +	{0x380b, 0xc8},
> +	{0x380c, 0x07},
> +	{0x380d, 0x8c},
> +	{0x380e, 0x09},
> +	{0x380f, 0xb2},
> +	{0x3810, 0x00},
> +	{0x3811, 0x02},
> +	{0x3812, 0x00},
> +	{0x3813, 0x02},
> +	{0x3814, 0x03},
> +	{0x3815, 0x01},
> +	{0x3816, 0x00},
> +	{0x3817, 0x00},
> +	{0x3818, 0x00},
> +	{0x3819, 0x00},
> +	{0x3820, 0x80},
> +	{0x3821, 0x47},
> +	{0x382a, 0x03},
> +	{0x382b, 0x01},
> +	{0x3830, 0x06},
> +	{0x3836, 0x02},
> +	{0x3862, 0x04},
> +	{0x3863, 0x08},
> +	{0x3cc0, 0x33},
> +	{0x3d85, 0x17},
> +	{0x3d8c, 0x73},
> +	{0x3d8d, 0xde},
> +	{0x4001, 0xe0},
> +	{0x4003, 0x40},
> +	{0x4008, 0x00},
> +	{0x4009, 0x05},
> +	{0x400a, 0x00},
> +	{0x400b, 0x84},
> +	{0x400f, 0x80},
> +	{0x4010, 0xf0},
> +	{0x4011, 0xff},
> +	{0x4012, 0x02},
> +	{0x4013, 0x01},
> +	{0x4014, 0x01},
> +	{0x4015, 0x01},
> +	{0x4042, 0x00},
> +	{0x4043, 0x80},
> +	{0x4044, 0x00},
> +	{0x4045, 0x80},
> +	{0x4046, 0x00},
> +	{0x4047, 0x80},
> +	{0x4048, 0x00},
> +	{0x4049, 0x80},
> +	{0x4041, 0x03},
> +	{0x404c, 0x20},
> +	{0x404d, 0x00},
> +	{0x404e, 0x20},
> +	{0x4203, 0x80},
> +	{0x4307, 0x30},
> +	{0x4317, 0x00},
> +	{0x4502, 0x50},
> +	{0x4503, 0x08},
> +	{0x4601, 0x80},
> +	{0x4800, 0x44},
> +	{0x4816, 0x53},
> +	{0x481b, 0x50},
> +	{0x481f, 0x27},
> +	{0x4823, 0x3c},
> +	{0x482b, 0x00},
> +	{0x4831, 0x66},
> +	{0x4837, 0x16},
> +	{0x483c, 0x0f},
> +	{0x484b, 0x05},
> +	{0x5000, 0x77},
> +	{0x5001, 0x0a},
> +	{0x5003, 0xc8},
> +	{0x5004, 0x04},
> +	{0x5006, 0x00},
> +	{0x5007, 0x00},
> +	{0x502e, 0x03},
> +	{0x5030, 0x41},
> +	{0x5795, 0x00},
> +	{0x5796, 0x10},
> +	{0x5797, 0x10},
> +	{0x5798, 0x73},
> +	{0x5799, 0x73},
> +	{0x579a, 0x00},
> +	{0x579b, 0x28},
> +	{0x579c, 0x00},
> +	{0x579d, 0x16},
> +	{0x579e, 0x06},
> +	{0x579f, 0x20},
> +	{0x57a0, 0x04},
> +	{0x57a1, 0xa0},
> +	{0x5780, 0x14},
> +	{0x5781, 0x0f},
> +	{0x5782, 0x44},
> +	{0x5783, 0x02},
> +	{0x5784, 0x01},
> +	{0x5785, 0x01},
> +	{0x5786, 0x00},
> +	{0x5787, 0x04},
> +	{0x5788, 0x02},
> +	{0x5789, 0x0f},
> +	{0x578a, 0xfd},
> +	{0x578b, 0xf5},
> +	{0x578c, 0xf5},
> +	{0x578d, 0x03},
> +	{0x578e, 0x08},
> +	{0x578f, 0x0c},
> +	{0x5790, 0x08},
> +	{0x5791, 0x04},
> +	{0x5792, 0x00},
> +	{0x5793, 0x52},
> +	{0x5794, 0xa3},
> +	{0x59f8, 0x3d},
> +	{0x5a08, 0x02},
> +	{0x5b00, 0x02},
> +	{0x5b01, 0x10},
> +	{0x5b02, 0x03},
> +	{0x5b03, 0xcf},
> +	{0x5b05, 0x6c},
> +	{0x5e00, 0x00},
> +	{0x5e10, 0xfc}
> +};
> +
>  static const char * const ov8856_test_pattern_menu[] = {
>  	"Disabled",
>  	"Standard Color Bar",
> @@ -548,6 +976,18 @@ static const struct ov8856_mode supported_modes[] = {
>  		.link_freq_index = OV8856_LINK_FREQ_720MBPS,
>  	},
>  	{
> +		.width    = 3264,
> +		.height   = 2448,
> +		.hts      = 1932,
> +		.vts_def  = 2482,
> +		.vts_min  = 2482,
> +		.reg_list = {
> +			.num_of_regs = ARRAY_SIZE(mode_3264x2448_regs),
> +			.regs = mode_3264x2448_regs,
> +		},
> +		.link_freq_index = OV8856_LINK_FREQ_720MBPS,
> +	},
> +	{
>  		.width = 1640,
>  		.height = 1232,
>  		.hts = 3820,
> @@ -558,6 +998,18 @@ static const struct ov8856_mode supported_modes[] = {
>  			.regs = mode_1640x1232_regs,
>  		},
>  		.link_freq_index = OV8856_LINK_FREQ_360MBPS,
> +	},
> +	{
> +		.width    = 1632,
> +		.height   = 1224,
> +		.hts      = 1932,
> +		.vts_def  = 2482,
> +		.vts_min  = 2482,
> +		.reg_list = {
> +			.num_of_regs = ARRAY_SIZE(mode_1632x1224_regs),
> +			.regs = mode_1632x1224_regs,
> +		},
> +		.link_freq_index = OV8856_LINK_FREQ_360MBPS,
>  	}
>  };
>  
> @@ -566,6 +1018,10 @@ struct ov8856 {
>  	struct media_pad pad;
>  	struct v4l2_ctrl_handler ctrl_handler;
>  
> +	struct clk		*xvclk;
> +	struct gpio_desc	*reset_gpio;
> +	struct regulator_bulk_data supplies[OV8856_NUM_SUPPLIES];
> +
>  	/* V4L2 Controls */
>  	struct v4l2_ctrl *link_freq;
>  	struct v4l2_ctrl *pixel_rate;
> @@ -576,6 +1032,9 @@ struct ov8856 {
>  	/* Current mode */
>  	const struct ov8856_mode *cur_mode;
>  
> +	/* module hardware version */
> +	bool is_1B_module;

What other hardware versions are there, and what are the differences?

> +
>  	/* To serialize asynchronus callbacks */
>  	struct mutex mutex;
>  
> @@ -696,6 +1155,24 @@ static int ov8856_test_pattern(struct ov8856 *ov8856, u32 pattern)
>  				OV8856_REG_VALUE_08BIT, pattern);
>  }
>  
> +static int ov8856_update_otp_reg(struct ov8856 *ov8856)
> +{
> +	int ret;
> +
> +	ret = ov8856_write_reg(ov8856, OV8856_REG_MODE_SELECT,
> +			       OV8856_REG_VALUE_08BIT, OV8856_MODE_STREAMING);
> +	if (ret)
> +		return ret;
> +
> +	ret = ov8856_write_reg(ov8856, OV8856_OTP_REG_ONE,
> +			       OV8856_REG_VALUE_08BIT, OV8856_MODE_STANDBY);
> +	if (ret)
> +		return ret;
> +
> +	return ov8856_write_reg(ov8856, OV8856_OTP_REG_TWO,
> +				OV8856_REG_VALUE_08BIT, OV8856_MODE_STREAMING);
> +}

What does this do?

> +
>  static int ov8856_set_ctrl(struct v4l2_ctrl *ctrl)
>  {
>  	struct ov8856 *ov8856 = container_of(ctrl->handler,
> @@ -825,7 +1302,13 @@ static void ov8856_update_pad_format(const struct ov8856_mode *mode,
>  {
>  	fmt->width = mode->width;
>  	fmt->height = mode->height;
> -	fmt->code = MEDIA_BUS_FMT_SGRBG10_1X10;
> +
> +	/* Bayer Order is determined by image resolution */

Ouch. 

This rather looks like a side effect of vertical cropping. How about
either cropping one line above or below, to keep the same Bayer order?

The driver is based on register lists that heavily restricts the
possibilities of configuring the sensor. The alternative, should more
free-form configuration be enabled, would be to expose the cropping
capability to the user --- as well as binning.

> +	if (fmt->width == 3264 || fmt->width == 1632)
> +		fmt->code = MEDIA_BUS_FMT_SBGGR10_1X10;
> +	else
> +		fmt->code = MEDIA_BUS_FMT_SGRBG10_1X10;
> +
>  	fmt->field = V4L2_FIELD_NONE;
>  }
>  
> @@ -850,6 +1333,17 @@ static int ov8856_start_streaming(struct ov8856 *ov8856)
>  		return ret;
>  	}
>  
> +	/* update R3614 for 1B module */
> +	if (ov8856->is_1B_module) {
> +		ret = ov8856_write_reg(ov8856, OV8856_CLK_REG,
> +				       OV8856_REG_VALUE_08BIT,
> +				       OV8856_CLK_REG_1B_VAL);
> +		if (ret) {
> +			dev_err(&client->dev, "failed to set R3614");
> +			return ret;
> +		}
> +	}
> +
>  	ret = __v4l2_ctrl_handler_setup(ov8856->sd.ctrl_handler);
>  	if (ret)
>  		return ret;
> @@ -882,6 +1376,8 @@ static int ov8856_set_stream(struct v4l2_subdev *sd, int enable)
>  	if (ov8856->streaming == enable)
>  		return 0;
>  
> +	dev_dbg(&client->dev, "hardware version: (%d)\n", ov8856->is_1B_module);
> +
>  	mutex_lock(&ov8856->mutex);
>  	if (enable) {
>  		ret = pm_runtime_get_sync(&client->dev);
> @@ -908,6 +1404,54 @@ static int ov8856_set_stream(struct v4l2_subdev *sd, int enable)
>  	return ret;
>  }
>  
> +/* Calculate the delay in us by clock rate and clock cycles */
> +static inline u32 ov8856_cal_delay(u32 cycles)
> +{
> +	return DIV_ROUND_UP(cycles, OV8856_XVCLK_FREQ / 1000 / 1000);

The frequency is rounded down. As it is used to calculate a delay needed,
rounding up should be done for the frequency, too.

> +}
> +
> +static int __ov8856_power_on(struct ov8856 *ov8856)
> +{
> +	int ret;
> +	u32 delay_us;
> +	struct i2c_client *client = v4l2_get_subdevdata(&ov8856->sd);
> +
> +	ret = clk_prepare_enable(ov8856->xvclk);
> +	if (ret < 0) {
> +		dev_err(&client->dev, "Failed to enable xvclk\n");
> +		return ret;
> +	}
> +
> +	gpiod_set_value_cansleep(ov8856->reset_gpio, 1);
> +
> +	ret = regulator_bulk_enable(OV8856_NUM_SUPPLIES, ov8856->supplies);
> +	if (ret < 0) {
> +		dev_err(&client->dev, "Failed to enable regulators\n");
> +		goto disable_clk;
> +	}
> +
> +	gpiod_set_value_cansleep(ov8856->reset_gpio, 0);
> +
> +	/* 8192 cycles prior to first SCCB transaction */
> +	delay_us = ov8856_cal_delay(8192);
> +	usleep_range(delay_us  * 2, delay_us * 4);

Why multiply by 2?

Note that the driver still needs to work even if the resources aren't
visible to the software. That's the case e.g. on ACPI based systems.

> +
> +	return 0;
> +
> +disable_clk:
> +	clk_disable_unprepare(ov8856->xvclk);
> +
> +	return ret;
> +}
> +
> +static void __ov8856_power_off(struct ov8856 *ov8856)
> +{
> +	clk_disable_unprepare(ov8856->xvclk);
> +	gpiod_set_value_cansleep(ov8856->reset_gpio, 1);
> +
> +	regulator_bulk_disable(OV8856_NUM_SUPPLIES, ov8856->supplies);
> +}
> +
>  static int __maybe_unused ov8856_suspend(struct device *dev)
>  {
>  	struct i2c_client *client = to_i2c_client(dev);
> @@ -915,8 +1459,8 @@ static int __maybe_unused ov8856_suspend(struct device *dev)
>  	struct ov8856 *ov8856 = to_ov8856(sd);
>  
>  	mutex_lock(&ov8856->mutex);
> -	if (ov8856->streaming)
> -		ov8856_stop_streaming(ov8856);

This seems like an unrelated change.

> +
> +	__ov8856_power_off(ov8856);
>  
>  	mutex_unlock(&ov8856->mutex);
>  
> @@ -1089,6 +1633,20 @@ static int ov8856_identify_module(struct ov8856 *ov8856)
>  		return -ENXIO;
>  	}
>  
> +	/* set R3614 to distinguish harward versions */
> +	ret = ov8856_update_otp_reg(ov8856);
> +	if (ret) {
> +		dev_err(&client->dev, "failed to set otp register");
> +		return ret;
> +	}
> +
> +	ret = ov8856_read_reg(ov8856, OV8856_OTP_REG,
> +			      OV8856_REG_VALUE_08BIT, &val);
> +	if (ret)
> +		return ret;
> +
> +	ov8856->is_1B_module = (val == OV8856_1B_MODULE) ? 1 : 0;
> +
>  	return 0;
>  }
>  
> @@ -1164,11 +1722,27 @@ static int ov8856_remove(struct i2c_client *client)
>  	media_entity_cleanup(&sd->entity);
>  	v4l2_ctrl_handler_free(sd->ctrl_handler);
>  	pm_runtime_disable(&client->dev);
> +	if (!pm_runtime_status_suspended(&client->dev))
> +		__ov8856_power_off(ov8856);
> +	pm_runtime_set_suspended(&client->dev);
>  	mutex_destroy(&ov8856->mutex);
>  
>  	return 0;
>  }
>  
> +static int ov8856_configure_regulators(struct ov8856 *ov8856)
> +{
> +	struct i2c_client *client = v4l2_get_subdevdata(&ov8856->sd);
> +	int i;

unsigned int

> +
> +	for (i = 0; i < OV8856_NUM_SUPPLIES; i++)
> +		ov8856->supplies[i].supply = ov8856_supply_names[i];
> +
> +	return devm_regulator_bulk_get(&client->dev,
> +				       OV8856_NUM_SUPPLIES,
> +				       ov8856->supplies);

Remember to put the regulators, too.

> +}
> +
>  static int ov8856_probe(struct i2c_client *client)
>  {
>  	struct ov8856 *ov8856;
> @@ -1186,6 +1760,40 @@ static int ov8856_probe(struct i2c_client *client)
>  		return -ENOMEM;
>  
>  	v4l2_i2c_subdev_init(&ov8856->sd, client, &ov8856_subdev_ops);
> +
> +	ov8856->xvclk = devm_clk_get(&client->dev, "xvclk");
> +	if (IS_ERR(ov8856->xvclk)) {
> +		dev_err(&client->dev, "Failed to get xvclk\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = clk_set_rate(ov8856->xvclk, OV8856_XVCLK_FREQ);
> +	if (ret < 0) {
> +		dev_err(&client->dev, "Failed to set xvclk rate (24MHz)\n");
> +		return ret;
> +	}
> +	if (clk_get_rate(ov8856->xvclk) != OV8856_XVCLK_FREQ)
> +		dev_warn(&client->dev,
> +			 "xvclk mismatched, modes are based on 24MHz\n");
> +
> +	ov8856->reset_gpio = devm_gpiod_get(&client->dev,
> +					    "reset",

Fits on the previous line.

> +					    GPIOD_OUT_LOW);
> +	if (IS_ERR(ov8856->reset_gpio)) {
> +		dev_err(&client->dev, "Failed to get reset-gpios\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = ov8856_configure_regulators(ov8856);
> +	if (ret) {
> +		dev_err(&client->dev, "Failed to get power regulators\n");
> +		return ret;
> +	}
> +
> +	ret = __ov8856_power_on(ov8856);
> +	if (ret)
> +		goto probe_error_v4l2_ctrl_handler_free;
> +
>  	ret = ov8856_identify_module(ov8856);
>  	if (ret) {
>  		dev_err(&client->dev, "failed to find sensor: %d", ret);
> @@ -1251,11 +1859,21 @@ static const struct acpi_device_id ov8856_acpi_ids[] = {
>  MODULE_DEVICE_TABLE(acpi, ov8856_acpi_ids);
>  #endif
>  
> +#if IS_ENABLED(CONFIG_OF)
> +static const struct of_device_id ov8856_of_match[] = {
> +	{ .compatible = "ovti,ov8856" },
> +	{},
> +};
> +
> +MODULE_DEVICE_TABLE(of, ov8856_of_match);
> +#endif
> +
>  static struct i2c_driver ov8856_i2c_driver = {
>  	.driver = {
>  		.name = "ov8856",
>  		.pm = &ov8856_pm_ops,
>  		.acpi_match_table = ACPI_PTR(ov8856_acpi_ids),
> +		.of_match_table = of_match_ptr(ov8856_of_match),
>  	},
>  	.probe_new = ov8856_probe,
>  	.remove = ov8856_remove,

-- 
Regards,

Sakari Ailus

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

* Re: [V1, 1/2] media: dt-bindings: media: i2c: Add bindings for ov8856
  2019-08-08  9:22 ` [V1, 1/2] media: dt-bindings: media: i2c: Add bindings for ov8856 dongchun.zhu
@ 2019-08-08 16:54   ` Sakari Ailus
  2019-08-13 11:55     ` Sakari Ailus
  0 siblings, 1 reply; 6+ messages in thread
From: Sakari Ailus @ 2019-08-08 16:54 UTC (permalink / raw)
  To: dongchun.zhu
  Cc: mchehab, robh+dt, mark.rutland, drinkcat, tfiga, matthias.bgg,
	bingbu.cao, srv_heupstream, linux-mediatek, linux-arm-kernel,
	sj.huang, linux-media, devicetree, louis.kuo, shengnan.wang,
	Ben Kao

Hi Dongchun,

(Cc'ing Ben, too.)

On Thu, Aug 08, 2019 at 05:22:14PM +0800, dongchun.zhu@mediatek.com wrote:
> From: Dongchun Zhu <dongchun.zhu@mediatek.com>
> 
> Add device tree binding documentation for the ov8856 camera sensor.
> 
> Signed-off-by: Dongchun Zhu <dongchun.zhu@mediatek.com>
> ---
>  .../devicetree/bindings/media/i2c/ov8856.txt       | 41 ++++++++++++++++++++++
>  MAINTAINERS                                        |  1 +
>  2 files changed, 42 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov8856.txt
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/ov8856.txt b/Documentation/devicetree/bindings/media/i2c/ov8856.txt
> new file mode 100644
> index 0000000..96b10e0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/ov8856.txt
> @@ -0,0 +1,41 @@
> +* Omnivision OV8856 MIPI CSI-2 sensor
> +
> +Required Properties:
> +- compatible: shall be "ovti,ov8856"
> +- clocks: reference to the xvclk input clock
> +- clock-names: shall be "xvclk"

Could you add "clock-frequency" property here, please, and specify the
upper and lower limits?

> +- avdd-supply: Analog voltage supply, 2.8 volts
> +- dovdd-supply: Digital I/O voltage supply, 1.8 volts
> +- dvdd-supply: Digital core voltage supply, 1.2 volts
> +- reset-gpios: Low active reset gpio
> +
> +The device node shall contain one 'port' child node with an
> +'endpoint' subnode for its digital output video port,
> +in accordance with the video interface bindings defined in
> +Documentation/devicetree/bindings/media/video-interfaces.txt.
> +The endpoint optional property 'data-lanes' shall be "<0 1 3 4>".

If you don't support lane reordering, then monotonically incrementing lane
numbers from 1 onwards are recommended.

Please also make the property mandatory.

> +
> +Example:
> +&i2c7 {
> +	ov8856: camera-sensor@10 {
> +		compatible = "ovti,ov8856";
> +		reg = <0x10>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&clk_24m_cam>;
> +
> +		clocks = <&cru SCLK_TESTCLKOUT1>;
> +		clock-names = "xvclk";
> +
> +		avdd-supply = <&mt6358_vcama2_reg>;
> +		dvdd-supply = <&mt6358_vcamd_reg>;
> +		dovdd-supply = <&mt6358_vcamio_reg>;
> +		reset-gpios = <&gpio2 3 GPIO_ACTIVE_LOW>;
> +
> +		port {
> +			wcam_out: endpoint {
> +				remote-endpoint = <&mipi_in_wcam>;
> +				data-lanes = <0 1 3 4>;
> +			};
> +		};
> +	};
> +};
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 783569e..7746c6b 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11889,6 +11889,7 @@ L:	linux-media@vger.kernel.org
>  T:	git git://linuxtv.org/media_tree.git
>  S:	Maintained
>  F:	drivers/media/i2c/ov8856.c
> +F:	Documentation/devicetree/bindings/media/i2c/ov8856.txt
>  
>  OMNIVISION OV9650 SENSOR DRIVER
>  M:	Sakari Ailus <sakari.ailus@linux.intel.com>

-- 
Regards,

Sakari Ailus
sakari.ailus@linux.intel.com

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

* Re: [V1, 1/2] media: dt-bindings: media: i2c: Add bindings for ov8856
  2019-08-08 16:54   ` Sakari Ailus
@ 2019-08-13 11:55     ` Sakari Ailus
  0 siblings, 0 replies; 6+ messages in thread
From: Sakari Ailus @ 2019-08-13 11:55 UTC (permalink / raw)
  To: dongchun.zhu
  Cc: mchehab, robh+dt, mark.rutland, drinkcat, tfiga, matthias.bgg,
	bingbu.cao, srv_heupstream, linux-mediatek, linux-arm-kernel,
	sj.huang, linux-media, devicetree, louis.kuo, shengnan.wang,
	Ben Kao

On Thu, Aug 08, 2019 at 07:54:16PM +0300, Sakari Ailus wrote:
> Hi Dongchun,
> 
> (Cc'ing Ben, too.)
> 
> On Thu, Aug 08, 2019 at 05:22:14PM +0800, dongchun.zhu@mediatek.com wrote:
> > From: Dongchun Zhu <dongchun.zhu@mediatek.com>
> > 
> > Add device tree binding documentation for the ov8856 camera sensor.
> > 
> > Signed-off-by: Dongchun Zhu <dongchun.zhu@mediatek.com>
> > ---
> >  .../devicetree/bindings/media/i2c/ov8856.txt       | 41 ++++++++++++++++++++++
> >  MAINTAINERS                                        |  1 +
> >  2 files changed, 42 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov8856.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/media/i2c/ov8856.txt b/Documentation/devicetree/bindings/media/i2c/ov8856.txt
> > new file mode 100644
> > index 0000000..96b10e0
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/i2c/ov8856.txt
> > @@ -0,0 +1,41 @@
> > +* Omnivision OV8856 MIPI CSI-2 sensor
> > +
> > +Required Properties:
> > +- compatible: shall be "ovti,ov8856"
> > +- clocks: reference to the xvclk input clock
> > +- clock-names: shall be "xvclk"
> 
> Could you add "clock-frequency" property here, please, and specify the
> upper and lower limits?

Can you also add the link-frequencies property, please?

-- 
Sakari Ailus
sakari.ailus@linux.intel.com

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

end of thread, back to index

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-08  9:22 [V1, 0/2] media: ov8856: DT bindings and sensor mode improvements dongchun.zhu
2019-08-08  9:22 ` [V1, 1/2] media: dt-bindings: media: i2c: Add bindings for ov8856 dongchun.zhu
2019-08-08 16:54   ` Sakari Ailus
2019-08-13 11:55     ` Sakari Ailus
2019-08-08  9:22 ` [V1, 2/2] media: i2c: Add more sensor mode for ov8856 camera sensor dongchun.zhu
2019-08-08 13:53   ` Sakari Ailus

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 linux-media@archiver.kernel.org
	public-inbox-index linux-media


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