[PATCH v4 0/6] media: mt9p031: Read back the real clock rate

[PATCH v4 0/6] media: mt9p031: Read back the real clock rate

[Stefan Riedmueller]

Hi,

Changes in v4:
 - Add two missing BIT macro conversions
 - Switch to dt-bindings yaml schema before applying changes
 - Drop explicit pclk-sample property documentation patch since it is
   documented in the referenced video-interface schema now. (I hope that
   is correct)

Changes in v3:
 - Dropped 1/5 media: mt9p031: Add support for 8 bit and 10 bit formats
 - Dropped 3/5 media: mt9p031: Implement [gs]_register debug calls
 - Added reviewed-by from Laurent Pinchart to
   media: mt9p031: Read back the real clock rate
 - Dropped unnecessary register reads in
   media: mt9p031: Fix corrupted frame after restarting
 - Changed sorting of register bits from MSB to LSB
 - Added patch to switch to BIT macro
 - Added two additional dt-bindings patches to add missing properties
   documentation

Christian Hemp (1):
  media: mt9p031: Make pixel clock polarity configurable by DT

Dirk Bender (1):
  media: mt9p031: Fix corrupted frame after restarting stream

Enrico Scholz (1):
  media: mt9p031: Read back the real clock rate

Stefan Riedmueller (3):
  media: mt9p031: Use BIT macro
  media: dt-bindings: mt9p031: Convert bindings to yaml
  media: dt-bindings: mt9p031: Add missing required properties

 .../bindings/media/i2c/aptina,mt9p031.yaml    | 97 +++++++++++++++++++
 .../devicetree/bindings/media/i2c/mt9p031.txt | 40 --------
 MAINTAINERS                                   |  1 +
 drivers/media/i2c/Kconfig                     |  1 +
 drivers/media/i2c/mt9p031.c                   | 80 +++++++++++----
 include/media/i2c/mt9p031.h                   |  1 +
 6 files changed, 162 insertions(+), 58 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/aptina,mt9p031.yaml
 delete mode 100644 Documentation/devicetree/bindings/media/i2c/mt9p031.txt

-- 
2.25.1

[PATCH v4 1/6] media: mt9p031: Read back the real clock rate

[Stefan Riedmueller]

From: Enrico Scholz <enrico.scholz@sigma-chemnitz.de>

The real and requested clock can differ and because it is used to
calculate PLL values, the real clock rate should be read.

Signed-off-by: Enrico Scholz <enrico.scholz@sigma-chemnitz.de>
Signed-off-by: Stefan Riedmueller <s.riedmueller@phytec.de>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/mt9p031.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c
index 6eb88ef99783..9dea7c813852 100644
--- a/drivers/media/i2c/mt9p031.c
+++ b/drivers/media/i2c/mt9p031.c
@@ -229,6 +229,7 @@ static int mt9p031_clk_setup(struct mt9p031 *mt9p031)
 
 	struct i2c_client *client = v4l2_get_subdevdata(&mt9p031->subdev);
 	struct mt9p031_platform_data *pdata = mt9p031->pdata;
+	unsigned long ext_freq;
 	int ret;
 
 	mt9p031->clk = devm_clk_get(&client->dev, NULL);
@@ -239,13 +240,15 @@ static int mt9p031_clk_setup(struct mt9p031 *mt9p031)
 	if (ret < 0)
 		return ret;
 
+	ext_freq = clk_get_rate(mt9p031->clk);
+
 	/* If the external clock frequency is out of bounds for the PLL use the
 	 * pixel clock divider only and disable the PLL.
 	 */
-	if (pdata->ext_freq > limits.ext_clock_max) {
+	if (ext_freq > limits.ext_clock_max) {
 		unsigned int div;
 
-		div = DIV_ROUND_UP(pdata->ext_freq, pdata->target_freq);
+		div = DIV_ROUND_UP(ext_freq, pdata->target_freq);
 		div = roundup_pow_of_two(div) / 2;
 
 		mt9p031->clk_div = min_t(unsigned int, div, 64);
@@ -254,7 +257,7 @@ static int mt9p031_clk_setup(struct mt9p031 *mt9p031)
 		return 0;
 	}
 
-	mt9p031->pll.ext_clock = pdata->ext_freq;
+	mt9p031->pll.ext_clock = ext_freq;
 	mt9p031->pll.pix_clock = pdata->target_freq;
 	mt9p031->use_pll = true;
 
-- 
2.25.1

[PATCH v4 2/6] media: mt9p031: Make pixel clock polarity configurable by DT

[Stefan Riedmueller]

From: Christian Hemp <c.hemp@phytec.de>

Evaluate the desired pixel clock polarity from the device tree.

Signed-off-by: Christian Hemp <c.hemp@phytec.de>
Signed-off-by: Stefan Riedmueller <s.riedmueller@phytec.de>
---
 drivers/media/i2c/Kconfig   |  1 +
 drivers/media/i2c/mt9p031.c | 20 +++++++++++++++++++-
 include/media/i2c/mt9p031.h |  1 +
 3 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 588f8eb95984..1f9e98be8066 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -1187,6 +1187,7 @@ config VIDEO_MT9P031
 	select MEDIA_CONTROLLER
 	select VIDEO_V4L2_SUBDEV_API
 	select VIDEO_APTINA_PLL
+	select V4L2_FWNODE
 	help
 	  This is a Video4Linux2 sensor driver for the Aptina
 	  (Micron) mt9p031 5 Mpixel camera.
diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c
index 9dea7c813852..ea90aff576ba 100644
--- a/drivers/media/i2c/mt9p031.c
+++ b/drivers/media/i2c/mt9p031.c
@@ -27,6 +27,7 @@
 #include <media/v4l2-async.h>
 #include <media/v4l2-ctrls.h>
 #include <media/v4l2-device.h>
+#include <media/v4l2-fwnode.h>
 #include <media/v4l2-subdev.h>
 
 #include "aptina-pll.h"
@@ -372,6 +373,14 @@ static int __mt9p031_set_power(struct mt9p031 *mt9p031, bool on)
 		return ret;
 	}
 
+	/* Configure the pixel clock polarity */
+	if (mt9p031->pdata && mt9p031->pdata->pixclk_pol) {
+		ret = mt9p031_write(client, MT9P031_PIXEL_CLOCK_CONTROL,
+				MT9P031_PIXEL_CLOCK_INVERT);
+		if (ret < 0)
+			return ret;
+	}
+
 	return v4l2_ctrl_handler_setup(&mt9p031->ctrls);
 }
 
@@ -1014,8 +1023,11 @@ static const struct v4l2_subdev_internal_ops mt9p031_subdev_internal_ops = {
 static struct mt9p031_platform_data *
 mt9p031_get_pdata(struct i2c_client *client)
 {
-	struct mt9p031_platform_data *pdata;
+	struct mt9p031_platform_data *pdata = NULL;
 	struct device_node *np;
+	struct v4l2_fwnode_endpoint endpoint = {
+		.bus_type = V4L2_MBUS_PARALLEL
+	};
 
 	if (!IS_ENABLED(CONFIG_OF) || !client->dev.of_node)
 		return client->dev.platform_data;
@@ -1024,6 +1036,9 @@ mt9p031_get_pdata(struct i2c_client *client)
 	if (!np)
 		return NULL;
 
+	if (v4l2_fwnode_endpoint_parse(of_fwnode_handle(np), &endpoint) < 0)
+		goto done;
+
 	pdata = devm_kzalloc(&client->dev, sizeof(*pdata), GFP_KERNEL);
 	if (!pdata)
 		goto done;
@@ -1031,6 +1046,9 @@ mt9p031_get_pdata(struct i2c_client *client)
 	of_property_read_u32(np, "input-clock-frequency", &pdata->ext_freq);
 	of_property_read_u32(np, "pixel-clock-frequency", &pdata->target_freq);
 
+	pdata->pixclk_pol = !!(endpoint.bus.parallel.flags &
+			       V4L2_MBUS_PCLK_SAMPLE_RISING);
+
 done:
 	of_node_put(np);
 	return pdata;
diff --git a/include/media/i2c/mt9p031.h b/include/media/i2c/mt9p031.h
index 7c29c53aa988..f933cd0be8e5 100644
--- a/include/media/i2c/mt9p031.h
+++ b/include/media/i2c/mt9p031.h
@@ -10,6 +10,7 @@ struct v4l2_subdev;
  * @target_freq: Pixel clock frequency
  */
 struct mt9p031_platform_data {
+	unsigned int pixclk_pol:1;
 	int ext_freq;
 	int target_freq;
 };
-- 
2.25.1

[PATCH v4 3/6] media: mt9p031: Fix corrupted frame after restarting stream

[Stefan Riedmueller]

From: Dirk Bender <d.bender@phytec.de>

To prevent corrupted frames after starting and stopping the sensor its
datasheet specifies a specific pause sequence to follow:

Stopping:
	Set Pause_Restart Bit -> Set Restart Bit -> Set Chip_Enable Off

Restarting:
	Set Chip_Enable On -> Clear Pause_Restart Bit

The Restart Bit is cleared automatically and must not be cleared
manually as this would cause undefined behavior.

Signed-off-by: Dirk Bender <d.bender@phytec.de>
Signed-off-by: Stefan Riedmueller <s.riedmueller@phytec.de>
---
 drivers/media/i2c/mt9p031.c | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c
index ea90aff576ba..ee2777059682 100644
--- a/drivers/media/i2c/mt9p031.c
+++ b/drivers/media/i2c/mt9p031.c
@@ -79,7 +79,9 @@
 #define		MT9P031_PIXEL_CLOCK_INVERT		(1 << 15)
 #define		MT9P031_PIXEL_CLOCK_SHIFT(n)		((n) << 8)
 #define		MT9P031_PIXEL_CLOCK_DIVIDE(n)		((n) << 0)
-#define MT9P031_FRAME_RESTART				0x0b
+#define MT9P031_RESTART					0x0b
+#define		MT9P031_FRAME_PAUSE_RESTART		(1 << 1)
+#define		MT9P031_FRAME_RESTART			(1 << 0)
 #define MT9P031_SHUTTER_DELAY				0x0c
 #define MT9P031_RST					0x0d
 #define		MT9P031_RST_ENABLE			1
@@ -456,9 +458,23 @@ static int mt9p031_set_params(struct mt9p031 *mt9p031)
 static int mt9p031_s_stream(struct v4l2_subdev *subdev, int enable)
 {
 	struct mt9p031 *mt9p031 = to_mt9p031(subdev);
+	struct i2c_client *client = v4l2_get_subdevdata(subdev);
+	int val;
 	int ret;
 
 	if (!enable) {
+		/* enable pause restart */
+		val = MT9P031_FRAME_PAUSE_RESTART;
+		ret = mt9p031_write(client, MT9P031_RESTART, val);
+		if (ret < 0)
+			return ret;
+
+		/* enable restart + keep pause restart set */
+		val |= MT9P031_FRAME_RESTART;
+		ret = mt9p031_write(client, MT9P031_RESTART, val);
+		if (ret < 0)
+			return ret;
+
 		/* Stop sensor readout */
 		ret = mt9p031_set_output_control(mt9p031,
 						 MT9P031_OUTPUT_CONTROL_CEN, 0);
@@ -478,6 +494,16 @@ static int mt9p031_s_stream(struct v4l2_subdev *subdev, int enable)
 	if (ret < 0)
 		return ret;
 
+	/*
+	 * - clear pause restart
+	 * - don't clear restart as clearing restart manually can cause
+	 *   undefined behavior
+	 */
+	val = MT9P031_FRAME_RESTART;
+	ret = mt9p031_write(client, MT9P031_RESTART, val);
+	if (ret < 0)
+		return ret;
+
 	return mt9p031_pll_enable(mt9p031);
 }
 
-- 
2.25.1

[PATCH v4 4/6] media: mt9p031: Use BIT macro

[Stefan Riedmueller]

Make use of the BIT macro for setting individual bits. This improves
readability and safety with respect to shifts.

When on it also remove two zero value disable defines.

Signed-off-by: Stefan Riedmueller <s.riedmueller@phytec.de>
---
 drivers/media/i2c/mt9p031.c | 27 ++++++++++++---------------
 1 file changed, 12 insertions(+), 15 deletions(-)

diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c
index ee2777059682..cbce8b88dbcf 100644
--- a/drivers/media/i2c/mt9p031.c
+++ b/drivers/media/i2c/mt9p031.c
@@ -76,40 +76,38 @@
 #define	MT9P031_PLL_CONFIG_1				0x11
 #define	MT9P031_PLL_CONFIG_2				0x12
 #define MT9P031_PIXEL_CLOCK_CONTROL			0x0a
-#define		MT9P031_PIXEL_CLOCK_INVERT		(1 << 15)
+#define		MT9P031_PIXEL_CLOCK_INVERT		BIT(15)
 #define		MT9P031_PIXEL_CLOCK_SHIFT(n)		((n) << 8)
 #define		MT9P031_PIXEL_CLOCK_DIVIDE(n)		((n) << 0)
 #define MT9P031_RESTART					0x0b
-#define		MT9P031_FRAME_PAUSE_RESTART		(1 << 1)
-#define		MT9P031_FRAME_RESTART			(1 << 0)
+#define		MT9P031_FRAME_PAUSE_RESTART		BIT(1)
+#define		MT9P031_FRAME_RESTART			BIT(0)
 #define MT9P031_SHUTTER_DELAY				0x0c
 #define MT9P031_RST					0x0d
-#define		MT9P031_RST_ENABLE			1
-#define		MT9P031_RST_DISABLE			0
+#define		MT9P031_RST_ENABLE			BIT(0)
 #define MT9P031_READ_MODE_1				0x1e
 #define MT9P031_READ_MODE_2				0x20
-#define		MT9P031_READ_MODE_2_ROW_MIR		(1 << 15)
-#define		MT9P031_READ_MODE_2_COL_MIR		(1 << 14)
-#define		MT9P031_READ_MODE_2_ROW_BLC		(1 << 6)
+#define		MT9P031_READ_MODE_2_ROW_MIR		BIT(15)
+#define		MT9P031_READ_MODE_2_COL_MIR		BIT(14)
+#define		MT9P031_READ_MODE_2_ROW_BLC		BIT(6)
 #define MT9P031_ROW_ADDRESS_MODE			0x22
 #define MT9P031_COLUMN_ADDRESS_MODE			0x23
 #define MT9P031_GLOBAL_GAIN				0x35
 #define		MT9P031_GLOBAL_GAIN_MIN			8
 #define		MT9P031_GLOBAL_GAIN_MAX			1024
 #define		MT9P031_GLOBAL_GAIN_DEF			8
-#define		MT9P031_GLOBAL_GAIN_MULT		(1 << 6)
+#define		MT9P031_GLOBAL_GAIN_MULT		BIT(6)
 #define MT9P031_ROW_BLACK_TARGET			0x49
 #define MT9P031_ROW_BLACK_DEF_OFFSET			0x4b
 #define MT9P031_GREEN1_OFFSET				0x60
 #define MT9P031_GREEN2_OFFSET				0x61
 #define MT9P031_BLACK_LEVEL_CALIBRATION			0x62
-#define		MT9P031_BLC_MANUAL_BLC			(1 << 0)
+#define		MT9P031_BLC_MANUAL_BLC			BIT(0)
 #define MT9P031_RED_OFFSET				0x63
 #define MT9P031_BLUE_OFFSET				0x64
 #define MT9P031_TEST_PATTERN				0xa0
 #define		MT9P031_TEST_PATTERN_SHIFT		3
-#define		MT9P031_TEST_PATTERN_ENABLE		(1 << 0)
-#define		MT9P031_TEST_PATTERN_DISABLE		(0 << 0)
+#define		MT9P031_TEST_PATTERN_ENABLE		BIT(0)
 #define MT9P031_TEST_PATTERN_GREEN			0xa1
 #define MT9P031_TEST_PATTERN_RED			0xa2
 #define MT9P031_TEST_PATTERN_BLUE			0xa3
@@ -199,7 +197,7 @@ static int mt9p031_reset(struct mt9p031 *mt9p031)
 	ret = mt9p031_write(client, MT9P031_RST, MT9P031_RST_ENABLE);
 	if (ret < 0)
 		return ret;
-	ret = mt9p031_write(client, MT9P031_RST, MT9P031_RST_DISABLE);
+	ret = mt9p031_write(client, MT9P031_RST, 0);
 	if (ret < 0)
 		return ret;
 
@@ -794,8 +792,7 @@ static int mt9p031_s_ctrl(struct v4l2_ctrl *ctrl)
 			if (ret < 0)
 				return ret;
 
-			return mt9p031_write(client, MT9P031_TEST_PATTERN,
-					     MT9P031_TEST_PATTERN_DISABLE);
+			return mt9p031_write(client, MT9P031_TEST_PATTERN, 0);
 		}
 
 		ret = mt9p031_write(client, MT9P031_TEST_PATTERN_GREEN, 0x05a0);
-- 
2.25.1

[PATCH v4 5/6] media: dt-bindings: mt9p031: Convert bindings to yaml

[Stefan Riedmueller]

Convert mt9p031 sensor bindings to yaml schema. Also update the
MAINTAINERS entry.

Signed-off-by: Stefan Riedmueller <s.riedmueller@phytec.de>
---
 .../bindings/media/i2c/aptina,mt9p031.yaml    | 75 +++++++++++++++++++
 .../devicetree/bindings/media/i2c/mt9p031.txt | 40 ----------
 MAINTAINERS                                   |  1 +
 3 files changed, 76 insertions(+), 40 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/aptina,mt9p031.yaml
 delete mode 100644 Documentation/devicetree/bindings/media/i2c/mt9p031.txt

diff --git a/Documentation/devicetree/bindings/media/i2c/aptina,mt9p031.yaml b/Documentation/devicetree/bindings/media/i2c/aptina,mt9p031.yaml
new file mode 100644
index 000000000000..7de62e339895
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/aptina,mt9p031.yaml
@@ -0,0 +1,75 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/i2c/mt9p031.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Aptina 1/2.5-Inch 5Mp CMOS Digital Image Sensor
+
+maintainers:
+  - Laurent Pinchart <laurent.pinchart@ideasonboard.com>
+
+description: |
+  The Aptina MT9P031 is a 1/2.5-inch CMOS active pixel digital image sensor
+  with an active array size of 2592H x 1944V. It is programmable through a
+  simple two-wire serial interface.
+
+properties:
+  compatible:
+    enum:
+      - aptina,mt9p031
+      - aptina,mt9p031m
+
+  reg:
+    description: I2C device address
+    maxItems: 1
+
+  reset-gpios:
+    maxItems: 1
+    description: Chip reset GPIO
+
+  port:
+    $ref: /schemas/graph.yaml#/properties/port
+    addittionalProeprties: false
+
+    properties:
+      endpoint:
+        $ref: /schemas/media/video-interfaces.yaml#
+        unevaluatedProperties: false
+
+        properties:
+          input-clock-frequency: true
+          pixel-clock-frequency: true
+
+        required:
+          - input-clock-frequency
+          - pixel-clock-frequency
+
+required:
+  - compatible
+  - reg
+  - port
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c0 {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        mt9p031@5d {
+            compatible = "aptina,mt9p031";
+            reg = <0x5d>;
+            reset-gpios = <&gpio_sensor 0 0>;
+
+            port {
+                mt9p031_1: endpoint {
+                    input-clock-frequency = <6000000>;
+                    pixel-clock-frequency = <96000000>;
+                };
+            };
+        }:
+    };
+
+...
diff --git a/Documentation/devicetree/bindings/media/i2c/mt9p031.txt b/Documentation/devicetree/bindings/media/i2c/mt9p031.txt
deleted file mode 100644
index cb60443ff78f..000000000000
--- a/Documentation/devicetree/bindings/media/i2c/mt9p031.txt
+++ /dev/null
@@ -1,40 +0,0 @@
-* Aptina 1/2.5-Inch 5Mp CMOS Digital Image Sensor
-
-The Aptina MT9P031 is a 1/2.5-inch CMOS active pixel digital image sensor with
-an active array size of 2592H x 1944V. It is programmable through a simple
-two-wire serial interface.
-
-Required Properties:
-- compatible: value should be either one among the following
-	(a) "aptina,mt9p031" for mt9p031 sensor
-	(b) "aptina,mt9p031m" for mt9p031m sensor
-
-- input-clock-frequency: Input clock frequency.
-
-- pixel-clock-frequency: Pixel clock frequency.
-
-Optional Properties:
-- reset-gpios: Chip reset GPIO
-
-For further reading on port node refer to
-Documentation/devicetree/bindings/media/video-interfaces.txt.
-
-Example:
-
-	i2c0@1c22000 {
-		...
-		...
-		mt9p031@5d {
-			compatible = "aptina,mt9p031";
-			reg = <0x5d>;
-			reset-gpios = <&gpio3 30 0>;
-
-			port {
-				mt9p031_1: endpoint {
-					input-clock-frequency = <6000000>;
-					pixel-clock-frequency = <96000000>;
-				};
-			};
-		};
-		...
-	};
diff --git a/MAINTAINERS b/MAINTAINERS
index c7456776ace5..f2123226baae 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12610,6 +12610,7 @@ M:	Laurent Pinchart <laurent.pinchart@ideasonboard.com>
 L:	linux-media@vger.kernel.org
 S:	Maintained
 T:	git git://linuxtv.org/media_tree.git
+F:	Documentation/devicetree/bindings/media/i2c/aptina,mt9p031.yaml
 F:	drivers/media/i2c/mt9p031.c
 F:	include/media/i2c/mt9p031.h
 
-- 
2.25.1

[PATCH v4 6/6] media: dt-bindings: mt9p031: Add missing required properties

[Stefan Riedmueller]

Add missing required clocks and supply regulator properties for the
sensor input clock and vdd, vdd_io and vaa supply regulators.

Signed-off-by: Stefan Riedmueller <s.riedmueller@phytec.de>
---
 .../bindings/media/i2c/aptina,mt9p031.yaml    | 22 +++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/i2c/aptina,mt9p031.yaml b/Documentation/devicetree/bindings/media/i2c/aptina,mt9p031.yaml
index 7de62e339895..09560d97a59d 100644
--- a/Documentation/devicetree/bindings/media/i2c/aptina,mt9p031.yaml
+++ b/Documentation/devicetree/bindings/media/i2c/aptina,mt9p031.yaml
@@ -24,6 +24,18 @@ properties:
     description: I2C device address
     maxItems: 1
 
+  clocks:
+    maxItems: 1
+
+  vdd-supply:
+    description: Digital supply voltage, 1.8 V
+
+  vdd_io-supply:
+    description: I/O supply voltage, 1.8 or 2.8 V
+
+  vaa-supply:
+    description: Analog supply voltage, 2.8 V
+
   reset-gpios:
     maxItems: 1
     description: Chip reset GPIO
@@ -48,6 +60,10 @@ properties:
 required:
   - compatible
   - reg
+  - clocks
+  - vdd-supply
+  - vdd_io-supply
+  - vaa-supply
   - port
 
 additionalProperties: false
@@ -63,6 +79,12 @@ examples:
             reg = <0x5d>;
             reset-gpios = <&gpio_sensor 0 0>;
 
+            clocks = <&sensor_clk>;
+
+            vdd-supply = <&reg_vdd>;
+            vdd_io-supply = <&reg_vdd_io>;
+            vaa-supply = <&reg_vaa>;
+
             port {
                 mt9p031_1: endpoint {
                     input-clock-frequency = <6000000>;
-- 
2.25.1

Re: [PATCH v4 5/6] media: dt-bindings: mt9p031: Convert bindings to yaml

[Sakari Ailus]

Hi Stefan,

Thanks for the patch.

On Thu, Jul 08, 2021 at 11:19:21AM +0200, Stefan Riedmueller wrote:
> Convert mt9p031 sensor bindings to yaml schema. Also update the
> MAINTAINERS entry.
> 
> Signed-off-by: Stefan Riedmueller <s.riedmueller@phytec.de>
> ---
>  .../bindings/media/i2c/aptina,mt9p031.yaml    | 75 +++++++++++++++++++
>  .../devicetree/bindings/media/i2c/mt9p031.txt | 40 ----------
>  MAINTAINERS                                   |  1 +
>  3 files changed, 76 insertions(+), 40 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/aptina,mt9p031.yaml
>  delete mode 100644 Documentation/devicetree/bindings/media/i2c/mt9p031.txt
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/aptina,mt9p031.yaml b/Documentation/devicetree/bindings/media/i2c/aptina,mt9p031.yaml
> new file mode 100644
> index 000000000000..7de62e339895
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/aptina,mt9p031.yaml
> @@ -0,0 +1,75 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/i2c/mt9p031.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Aptina 1/2.5-Inch 5Mp CMOS Digital Image Sensor
> +
> +maintainers:
> +  - Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> +
> +description: |
> +  The Aptina MT9P031 is a 1/2.5-inch CMOS active pixel digital image sensor
> +  with an active array size of 2592H x 1944V. It is programmable through a
> +  simple two-wire serial interface.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - aptina,mt9p031
> +      - aptina,mt9p031m
> +
> +  reg:
> +    description: I2C device address
> +    maxItems: 1
> +
> +  reset-gpios:
> +    maxItems: 1
> +    description: Chip reset GPIO
> +
> +  port:
> +    $ref: /schemas/graph.yaml#/properties/port

This should probably be:

	$ref: /schemas/graph.yaml#/$defs/port-base

> +    addittionalProeprties: false


Could you run

	make dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/media/i2c/aptina,mt9p031.yaml

please?

> +
> +    properties:
> +      endpoint:
> +        $ref: /schemas/media/video-interfaces.yaml#
> +        unevaluatedProperties: false
> +
> +        properties:
> +          input-clock-frequency: true
> +          pixel-clock-frequency: true

These two legacy properties were not in the endpoint previously.

> +
> +        required:
> +          - input-clock-frequency
> +          - pixel-clock-frequency
> +
> +required:
> +  - compatible
> +  - reg
> +  - port
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    i2c0 {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        mt9p031@5d {
> +            compatible = "aptina,mt9p031";
> +            reg = <0x5d>;
> +            reset-gpios = <&gpio_sensor 0 0>;
> +
> +            port {
> +                mt9p031_1: endpoint {
> +                    input-clock-frequency = <6000000>;
> +                    pixel-clock-frequency = <96000000>;
> +                };
> +            };
> +        }:
> +    };
> +
> +...
> diff --git a/Documentation/devicetree/bindings/media/i2c/mt9p031.txt b/Documentation/devicetree/bindings/media/i2c/mt9p031.txt
> deleted file mode 100644
> index cb60443ff78f..000000000000
> --- a/Documentation/devicetree/bindings/media/i2c/mt9p031.txt
> +++ /dev/null
> @@ -1,40 +0,0 @@
> -* Aptina 1/2.5-Inch 5Mp CMOS Digital Image Sensor
> -
> -The Aptina MT9P031 is a 1/2.5-inch CMOS active pixel digital image sensor with
> -an active array size of 2592H x 1944V. It is programmable through a simple
> -two-wire serial interface.
> -
> -Required Properties:
> -- compatible: value should be either one among the following
> -	(a) "aptina,mt9p031" for mt9p031 sensor
> -	(b) "aptina,mt9p031m" for mt9p031m sensor
> -
> -- input-clock-frequency: Input clock frequency.
> -
> -- pixel-clock-frequency: Pixel clock frequency.
> -
> -Optional Properties:
> -- reset-gpios: Chip reset GPIO
> -
> -For further reading on port node refer to
> -Documentation/devicetree/bindings/media/video-interfaces.txt.
> -
> -Example:
> -
> -	i2c0@1c22000 {
> -		...
> -		...
> -		mt9p031@5d {
> -			compatible = "aptina,mt9p031";
> -			reg = <0x5d>;
> -			reset-gpios = <&gpio3 30 0>;
> -
> -			port {
> -				mt9p031_1: endpoint {
> -					input-clock-frequency = <6000000>;
> -					pixel-clock-frequency = <96000000>;
> -				};
> -			};
> -		};
> -		...
> -	};
> diff --git a/MAINTAINERS b/MAINTAINERS
> index c7456776ace5..f2123226baae 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -12610,6 +12610,7 @@ M:	Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>  L:	linux-media@vger.kernel.org
>  S:	Maintained
>  T:	git git://linuxtv.org/media_tree.git
> +F:	Documentation/devicetree/bindings/media/i2c/aptina,mt9p031.yaml
>  F:	drivers/media/i2c/mt9p031.c
>  F:	include/media/i2c/mt9p031.h
>  

-- 
Kind regards,

Sakari Ailus

Re: [PATCH v4 5/6] media: dt-bindings: mt9p031: Convert bindings to yaml

[Stefan Riedmüller]

Hi Sakari,

On Thu, 2021-07-08 at 14:23 +0300, Sakari Ailus wrote:
> Hi Stefan,
> 
> Thanks for the patch.

Thanks for your feedback!

> 
> On Thu, Jul 08, 2021 at 11:19:21AM +0200, Stefan Riedmueller wrote:
> > Convert mt9p031 sensor bindings to yaml schema. Also update the
> > MAINTAINERS entry.
> > 
> > Signed-off-by: Stefan Riedmueller <s.riedmueller@phytec.de>
> > ---
> >  .../bindings/media/i2c/aptina,mt9p031.yaml    | 75 +++++++++++++++++++
> >  .../devicetree/bindings/media/i2c/mt9p031.txt | 40 ----------
> >  MAINTAINERS                                   |  1 +
> >  3 files changed, 76 insertions(+), 40 deletions(-)
> >  create mode 100644
> > Documentation/devicetree/bindings/media/i2c/aptina,mt9p031.yaml
> >  delete mode 100644
> > Documentation/devicetree/bindings/media/i2c/mt9p031.txt
> > 
> > diff --git
> > a/Documentation/devicetree/bindings/media/i2c/aptina,mt9p031.yaml
> > b/Documentation/devicetree/bindings/media/i2c/aptina,mt9p031.yaml
> > new file mode 100644
> > index 000000000000..7de62e339895
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/i2c/aptina,mt9p031.yaml
> > @@ -0,0 +1,75 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/media/i2c/mt9p031.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Aptina 1/2.5-Inch 5Mp CMOS Digital Image Sensor
> > +
> > +maintainers:
> > +  - Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > +
> > +description: |
> > +  The Aptina MT9P031 is a 1/2.5-inch CMOS active pixel digital image
> > sensor
> > +  with an active array size of 2592H x 1944V. It is programmable through
> > a
> > +  simple two-wire serial interface.
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - aptina,mt9p031
> > +      - aptina,mt9p031m
> > +
> > +  reg:
> > +    description: I2C device address
> > +    maxItems: 1
> > +
> > +  reset-gpios:
> > +    maxItems: 1
> > +    description: Chip reset GPIO
> > +
> > +  port:
> > +    $ref: /schemas/graph.yaml#/properties/port
> 
> This should probably be:
> 
> 	$ref: /schemas/graph.yaml#/$defs/port-base

I actually was not sure which one of these to use because I found both in
existing bindings. Also /schemas/graph.yaml#/properties/port has a reference
to $defs/port-base. It would be nice if you or someone else could give me a
hint why /schemas/graph.yaml#/$defs/port-base would be the correct one here.

> 
> > +    addittionalProeprties: false
> 
> Could you run
> 
> 	make dt_binding_check
> DT_SCHEMA_FILES=Documentation/devicetree/bindings/media/i2c/aptina,mt9p031.y
> aml
> 
> please?

Sorry, I totally forgot about this. I'll do so before sending the next
version.

> 
> > +
> > +    properties:
> > +      endpoint:
> > +        $ref: /schemas/media/video-interfaces.yaml#
> > +        unevaluatedProperties: false
> > +
> > +        properties:
> > +          input-clock-frequency: true
> > +          pixel-clock-frequency: true
> 
> These two legacy properties were not in the endpoint previously.

That's right, but they are being parsed from the endpoint so I figured they
belong here. Should I mention this in the commit message?

Thanks,
Stefan

> 
> > +
> > +        required:
> > +          - input-clock-frequency
> > +          - pixel-clock-frequency
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - port
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    i2c0 {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        mt9p031@5d {
> > +            compatible = "aptina,mt9p031";
> > +            reg = <0x5d>;
> > +            reset-gpios = <&gpio_sensor 0 0>;
> > +
> > +            port {
> > +                mt9p031_1: endpoint {
> > +                    input-clock-frequency = <6000000>;
> > +                    pixel-clock-frequency = <96000000>;
> > +                };
> > +            };
> > +        }:
> > +    };
> > +
> > +...
> > diff --git a/Documentation/devicetree/bindings/media/i2c/mt9p031.txt
> > b/Documentation/devicetree/bindings/media/i2c/mt9p031.txt
> > deleted file mode 100644
> > index cb60443ff78f..000000000000
> > --- a/Documentation/devicetree/bindings/media/i2c/mt9p031.txt
> > +++ /dev/null
> > @@ -1,40 +0,0 @@
> > -* Aptina 1/2.5-Inch 5Mp CMOS Digital Image Sensor
> > -
> > -The Aptina MT9P031 is a 1/2.5-inch CMOS active pixel digital image sensor
> > with
> > -an active array size of 2592H x 1944V. It is programmable through a
> > simple
> > -two-wire serial interface.
> > -
> > -Required Properties:
> > -- compatible: value should be either one among the following
> > -	(a) "aptina,mt9p031" for mt9p031 sensor
> > -	(b) "aptina,mt9p031m" for mt9p031m sensor
> > -
> > -- input-clock-frequency: Input clock frequency.
> > -
> > -- pixel-clock-frequency: Pixel clock frequency.
> > -
> > -Optional Properties:
> > -- reset-gpios: Chip reset GPIO
> > -
> > -For further reading on port node refer to
> > -Documentation/devicetree/bindings/media/video-interfaces.txt.
> > -
> > -Example:
> > -
> > -	i2c0@1c22000 {
> > -		...
> > -		...
> > -		mt9p031@5d {
> > -			compatible = "aptina,mt9p031";
> > -			reg = <0x5d>;
> > -			reset-gpios = <&gpio3 30 0>;
> > -
> > -			port {
> > -				mt9p031_1: endpoint {
> > -					input-clock-frequency = <6000000>;
> > -					pixel-clock-frequency = <96000000>;
> > -				};
> > -			};
> > -		};
> > -		...
> > -	};
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index c7456776ace5..f2123226baae 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -12610,6 +12610,7 @@ M:	Laurent Pinchart <
> > laurent.pinchart@ideasonboard.com>
> >  L:	linux-media@vger.kernel.org
> >  S:	Maintained
> >  T:	git git://linuxtv.org/media_tree.git
> > +F:	Documentation/devicetree/bindings/media/i2c/aptina,mt9p031.yaml
> >  F:	drivers/media/i2c/mt9p031.c
> >  F:	include/media/i2c/mt9p031.h
> >  

Re: [PATCH v4 5/6] media: dt-bindings: mt9p031: Convert bindings to yaml

[sakari.ailus]

Hi Stefan,

On Thu, Jul 08, 2021 at 01:34:17PM +0000, Stefan Riedmüller wrote:
> Hi Sakari,
> 
> On Thu, 2021-07-08 at 14:23 +0300, Sakari Ailus wrote:
> > Hi Stefan,
> > 
> > Thanks for the patch.
> 
> Thanks for your feedback!

You're welcome!

> 
> > 
> > On Thu, Jul 08, 2021 at 11:19:21AM +0200, Stefan Riedmueller wrote:
> > > Convert mt9p031 sensor bindings to yaml schema. Also update the
> > > MAINTAINERS entry.
> > > 
> > > Signed-off-by: Stefan Riedmueller <s.riedmueller@phytec.de>
> > > ---
> > >  .../bindings/media/i2c/aptina,mt9p031.yaml    | 75 +++++++++++++++++++
> > >  .../devicetree/bindings/media/i2c/mt9p031.txt | 40 ----------
> > >  MAINTAINERS                                   |  1 +
> > >  3 files changed, 76 insertions(+), 40 deletions(-)
> > >  create mode 100644
> > > Documentation/devicetree/bindings/media/i2c/aptina,mt9p031.yaml
> > >  delete mode 100644
> > > Documentation/devicetree/bindings/media/i2c/mt9p031.txt
> > > 
> > > diff --git
> > > a/Documentation/devicetree/bindings/media/i2c/aptina,mt9p031.yaml
> > > b/Documentation/devicetree/bindings/media/i2c/aptina,mt9p031.yaml
> > > new file mode 100644
> > > index 000000000000..7de62e339895
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/media/i2c/aptina,mt9p031.yaml
> > > @@ -0,0 +1,75 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/media/i2c/mt9p031.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Aptina 1/2.5-Inch 5Mp CMOS Digital Image Sensor
> > > +
> > > +maintainers:
> > > +  - Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > +
> > > +description: |
> > > +  The Aptina MT9P031 is a 1/2.5-inch CMOS active pixel digital image
> > > sensor
> > > +  with an active array size of 2592H x 1944V. It is programmable through
> > > a
> > > +  simple two-wire serial interface.
> > > +
> > > +properties:
> > > +  compatible:
> > > +    enum:
> > > +      - aptina,mt9p031
> > > +      - aptina,mt9p031m
> > > +
> > > +  reg:
> > > +    description: I2C device address
> > > +    maxItems: 1
> > > +
> > > +  reset-gpios:
> > > +    maxItems: 1
> > > +    description: Chip reset GPIO
> > > +
> > > +  port:
> > > +    $ref: /schemas/graph.yaml#/properties/port
> > 
> > This should probably be:
> > 
> > 	$ref: /schemas/graph.yaml#/$defs/port-base
> 
> I actually was not sure which one of these to use because I found both in
> existing bindings. Also /schemas/graph.yaml#/properties/port has a reference
> to $defs/port-base. It would be nice if you or someone else could give me a
> hint why /schemas/graph.yaml#/$defs/port-base would be the correct one here.

I haven't dug into the details but:

<URL:https://lore.kernel.org/linux-media/20210623164344.2571043-1-robh@kernel.org/T/>

> 
> > 
> > > +    addittionalProeprties: false
> > 
> > Could you run
> > 
> > 	make dt_binding_check
> > DT_SCHEMA_FILES=Documentation/devicetree/bindings/media/i2c/aptina,mt9p031.y
> > aml
> > 
> > please?
> 
> Sorry, I totally forgot about this. I'll do so before sending the next
> version.

No worries.

> 
> > 
> > > +
> > > +    properties:
> > > +      endpoint:
> > > +        $ref: /schemas/media/video-interfaces.yaml#
> > > +        unevaluatedProperties: false
> > > +
> > > +        properties:
> > > +          input-clock-frequency: true
> > > +          pixel-clock-frequency: true
> > 
> > These two legacy properties were not in the endpoint previously.
> 
> That's right, but they are being parsed from the endpoint so I figured they
> belong here. Should I mention this in the commit message?

Please do. I missed the driver did this.

Ideally the driver would use the common properties but it's old and
unlikely to be used in new boards anyway I guess.

> 
> Thanks,
> Stefan
> 
> > 
> > > +
> > > +        required:
> > > +          - input-clock-frequency
> > > +          - pixel-clock-frequency
> > > +
> > > +required:
> > > +  - compatible
> > > +  - reg
> > > +  - port
> > > +
> > > +additionalProperties: false
> > > +
> > > +examples:
> > > +  - |
> > > +    i2c0 {
> > > +        #address-cells = <1>;
> > > +        #size-cells = <0>;
> > > +
> > > +        mt9p031@5d {
> > > +            compatible = "aptina,mt9p031";
> > > +            reg = <0x5d>;
> > > +            reset-gpios = <&gpio_sensor 0 0>;
> > > +
> > > +            port {
> > > +                mt9p031_1: endpoint {
> > > +                    input-clock-frequency = <6000000>;
> > > +                    pixel-clock-frequency = <96000000>;
> > > +                };
> > > +            };
> > > +        }:
> > > +    };
> > > +
> > > +...
> > > diff --git a/Documentation/devicetree/bindings/media/i2c/mt9p031.txt
> > > b/Documentation/devicetree/bindings/media/i2c/mt9p031.txt
> > > deleted file mode 100644
> > > index cb60443ff78f..000000000000
> > > --- a/Documentation/devicetree/bindings/media/i2c/mt9p031.txt
> > > +++ /dev/null
> > > @@ -1,40 +0,0 @@
> > > -* Aptina 1/2.5-Inch 5Mp CMOS Digital Image Sensor
> > > -
> > > -The Aptina MT9P031 is a 1/2.5-inch CMOS active pixel digital image sensor
> > > with
> > > -an active array size of 2592H x 1944V. It is programmable through a
> > > simple
> > > -two-wire serial interface.
> > > -
> > > -Required Properties:
> > > -- compatible: value should be either one among the following
> > > -	(a) "aptina,mt9p031" for mt9p031 sensor
> > > -	(b) "aptina,mt9p031m" for mt9p031m sensor
> > > -
> > > -- input-clock-frequency: Input clock frequency.
> > > -
> > > -- pixel-clock-frequency: Pixel clock frequency.
> > > -
> > > -Optional Properties:
> > > -- reset-gpios: Chip reset GPIO
> > > -
> > > -For further reading on port node refer to
> > > -Documentation/devicetree/bindings/media/video-interfaces.txt.
> > > -
> > > -Example:
> > > -
> > > -	i2c0@1c22000 {
> > > -		...
> > > -		...
> > > -		mt9p031@5d {
> > > -			compatible = "aptina,mt9p031";
> > > -			reg = <0x5d>;
> > > -			reset-gpios = <&gpio3 30 0>;
> > > -
> > > -			port {
> > > -				mt9p031_1: endpoint {
> > > -					input-clock-frequency = <6000000>;
> > > -					pixel-clock-frequency = <96000000>;
> > > -				};
> > > -			};
> > > -		};
> > > -		...
> > > -	};
> > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > index c7456776ace5..f2123226baae 100644
> > > --- a/MAINTAINERS
> > > +++ b/MAINTAINERS
> > > @@ -12610,6 +12610,7 @@ M:	Laurent Pinchart <
> > > laurent.pinchart@ideasonboard.com>
> > >  L:	linux-media@vger.kernel.org
> > >  S:	Maintained
> > >  T:	git git://linuxtv.org/media_tree.git
> > > +F:	Documentation/devicetree/bindings/media/i2c/aptina,mt9p031.yaml
> > >  F:	drivers/media/i2c/mt9p031.c
> > >  F:	include/media/i2c/mt9p031.h
> > >  

-- 
Regards,

Sakari Ailus

Re: [PATCH v4 5/6] media: dt-bindings: mt9p031: Convert bindings to yaml

[Stefan Riedmüller]

Hi Sakari,

On Thu, 2021-07-08 at 17:02 +0300, sakari.ailus@linux.intel.com wrote:
> Hi Stefan,
> 
> On Thu, Jul 08, 2021 at 01:34:17PM +0000, Stefan Riedmüller wrote:
> > Hi Sakari,
> > 
> > On Thu, 2021-07-08 at 14:23 +0300, Sakari Ailus wrote:
> > > Hi Stefan,
> > > 
> > > Thanks for the patch.
> > 
> > Thanks for your feedback!
> 
> You're welcome!
> 
> > > On Thu, Jul 08, 2021 at 11:19:21AM +0200, Stefan Riedmueller wrote:
> > > > Convert mt9p031 sensor bindings to yaml schema. Also update the
> > > > MAINTAINERS entry.
> > > > 
> > > > Signed-off-by: Stefan Riedmueller <s.riedmueller@phytec.de>
> > > > ---
> > > >  .../bindings/media/i2c/aptina,mt9p031.yaml    | 75
> > > > +++++++++++++++++++
> > > >  .../devicetree/bindings/media/i2c/mt9p031.txt | 40 ----------
> > > >  MAINTAINERS                                   |  1 +
> > > >  3 files changed, 76 insertions(+), 40 deletions(-)
> > > >  create mode 100644
> > > > Documentation/devicetree/bindings/media/i2c/aptina,mt9p031.yaml
> > > >  delete mode 100644
> > > > Documentation/devicetree/bindings/media/i2c/mt9p031.txt
> > > > 
> > > > diff --git
> > > > a/Documentation/devicetree/bindings/media/i2c/aptina,mt9p031.yaml
> > > > b/Documentation/devicetree/bindings/media/i2c/aptina,mt9p031.yaml
> > > > new file mode 100644
> > > > index 000000000000..7de62e339895
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/media/i2c/aptina,mt9p031.yaml
> > > > @@ -0,0 +1,75 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/media/i2c/mt9p031.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: Aptina 1/2.5-Inch 5Mp CMOS Digital Image Sensor
> > > > +
> > > > +maintainers:
> > > > +  - Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > > +
> > > > +description: |
> > > > +  The Aptina MT9P031 is a 1/2.5-inch CMOS active pixel digital image
> > > > sensor
> > > > +  with an active array size of 2592H x 1944V. It is programmable
> > > > through
> > > > a
> > > > +  simple two-wire serial interface.
> > > > +
> > > > +properties:
> > > > +  compatible:
> > > > +    enum:
> > > > +      - aptina,mt9p031
> > > > +      - aptina,mt9p031m
> > > > +
> > > > +  reg:
> > > > +    description: I2C device address
> > > > +    maxItems: 1
> > > > +
> > > > +  reset-gpios:
> > > > +    maxItems: 1
> > > > +    description: Chip reset GPIO
> > > > +
> > > > +  port:
> > > > +    $ref: /schemas/graph.yaml#/properties/port
> > > 
> > > This should probably be:
> > > 
> > > 	$ref: /schemas/graph.yaml#/$defs/port-base
> > 
> > I actually was not sure which one of these to use because I found both in
> > existing bindings. Also /schemas/graph.yaml#/properties/port has a
> > reference
> > to $defs/port-base. It would be nice if you or someone else could give me
> > a
> > hint why /schemas/graph.yaml#/$defs/port-base would be the correct one
> > here.
> 
> I haven't dug into the details but:
> 
> <URL:
> https://lore.kernel.org/linux-media/20210623164344.2571043-1-robh@kernel.org/T/
> >

That's what I was looking for, thanks!

I'll send a new version.

Regards,
Stefan

> 
> > > > +    addittionalProeprties: false
> > > 
> > > Could you run
> > > 
> > > 	make dt_binding_check
> > > DT_SCHEMA_FILES=Documentation/devicetree/bindings/media/i2c/aptina,mt9p0
> > > 31.y
> > > aml
> > > 
> > > please?
> > 
> > Sorry, I totally forgot about this. I'll do so before sending the next
> > version.
> 
> No worries.
> 
> > > > +
> > > > +    properties:
> > > > +      endpoint:
> > > > +        $ref: /schemas/media/video-interfaces.yaml#
> > > > +        unevaluatedProperties: false
> > > > +
> > > > +        properties:
> > > > +          input-clock-frequency: true
> > > > +          pixel-clock-frequency: true
> > > 
> > > These two legacy properties were not in the endpoint previously.
> > 
> > That's right, but they are being parsed from the endpoint so I figured
> > they
> > belong here. Should I mention this in the commit message?
> 
> Please do. I missed the driver did this.
> 
> Ideally the driver would use the common properties but it's old and
> unlikely to be used in new boards anyway I guess.
> 
> > Thanks,
> > Stefan
> > 
> > > > +
> > > > +        required:
> > > > +          - input-clock-frequency
> > > > +          - pixel-clock-frequency
> > > > +
> > > > +required:
> > > > +  - compatible
> > > > +  - reg
> > > > +  - port
> > > > +
> > > > +additionalProperties: false
> > > > +
> > > > +examples:
> > > > +  - |
> > > > +    i2c0 {
> > > > +        #address-cells = <1>;
> > > > +        #size-cells = <0>;
> > > > +
> > > > +        mt9p031@5d {
> > > > +            compatible = "aptina,mt9p031";
> > > > +            reg = <0x5d>;
> > > > +            reset-gpios = <&gpio_sensor 0 0>;
> > > > +
> > > > +            port {
> > > > +                mt9p031_1: endpoint {
> > > > +                    input-clock-frequency = <6000000>;
> > > > +                    pixel-clock-frequency = <96000000>;
> > > > +                };
> > > > +            };
> > > > +        }:
> > > > +    };
> > > > +
> > > > +...
> > > > diff --git a/Documentation/devicetree/bindings/media/i2c/mt9p031.txt
> > > > b/Documentation/devicetree/bindings/media/i2c/mt9p031.txt
> > > > deleted file mode 100644
> > > > index cb60443ff78f..000000000000
> > > > --- a/Documentation/devicetree/bindings/media/i2c/mt9p031.txt
> > > > +++ /dev/null
> > > > @@ -1,40 +0,0 @@
> > > > -* Aptina 1/2.5-Inch 5Mp CMOS Digital Image Sensor
> > > > -
> > > > -The Aptina MT9P031 is a 1/2.5-inch CMOS active pixel digital image
> > > > sensor
> > > > with
> > > > -an active array size of 2592H x 1944V. It is programmable through a
> > > > simple
> > > > -two-wire serial interface.
> > > > -
> > > > -Required Properties:
> > > > -- compatible: value should be either one among the following
> > > > -	(a) "aptina,mt9p031" for mt9p031 sensor
> > > > -	(b) "aptina,mt9p031m" for mt9p031m sensor
> > > > -
> > > > -- input-clock-frequency: Input clock frequency.
> > > > -
> > > > -- pixel-clock-frequency: Pixel clock frequency.
> > > > -
> > > > -Optional Properties:
> > > > -- reset-gpios: Chip reset GPIO
> > > > -
> > > > -For further reading on port node refer to
> > > > -Documentation/devicetree/bindings/media/video-interfaces.txt.
> > > > -
> > > > -Example:
> > > > -
> > > > -	i2c0@1c22000 {
> > > > -		...
> > > > -		...
> > > > -		mt9p031@5d {
> > > > -			compatible = "aptina,mt9p031";
> > > > -			reg = <0x5d>;
> > > > -			reset-gpios = <&gpio3 30 0>;
> > > > -
> > > > -			port {
> > > > -				mt9p031_1: endpoint {
> > > > -					input-clock-frequency =
> > > > <6000000>;
> > > > -					pixel-clock-frequency =
> > > > <96000000>;
> > > > -				};
> > > > -			};
> > > > -		};
> > > > -		...
> > > > -	};
> > > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > > index c7456776ace5..f2123226baae 100644
> > > > --- a/MAINTAINERS
> > > > +++ b/MAINTAINERS
> > > > @@ -12610,6 +12610,7 @@ M:	Laurent Pinchart <
> > > > laurent.pinchart@ideasonboard.com>
> > > >  L:	linux-media@vger.kernel.org
> > > >  S:	Maintained
> > > >  T:	git git://linuxtv.org/media_tree.git
> > > > +F:	Documentation/devicetree/bindings/media/i2c/aptina,mt9p031.yam
> > > > l
> > > >  F:	drivers/media/i2c/mt9p031.c
> > > >  F:	include/media/i2c/mt9p031.h
> > > >