[PATCH 0/6] media: rcar-vin: Brush endpoint properties

[PATCH 0/6] media: rcar-vin: Brush endpoint properties

[Jacopo Mondi]

Hello,
   this series touches the bindings and the driver handling endpoint
properties for digital subdevices of the R-Car VIN driver.

The first patch simply documents what are the endpoint properties supported
at the moment, then the second one extends them with 'data-active'.

As the VIN hardware allows to use HSYNC as data enable signal when the CLCKENB
pin is left unconnected, the 'data-active' property presence determinates
if HSYNC has to be used or not as data enable signal. As a consequence, when
running with embedded synchronism, and there is not HSYNC signal, it becomes
mandatory to specify 'data-active' polarity in DTS.

To address this, all Gen-2 boards featuring a composite video input and
running with embedded synchronization, now need that property to be specified
in DTS. Before adding it, remove un-used properties as 'pclk-sample' and
'bus-width' from the Gen-2 bindings, as they are not parsed by the VIN driver
and only confuse users.

Niklas, as you already know I don't have any Gen2 board. Could you give this
a spin on Koelsch if you like the series?

Thanks
   j

Jacopo Mondi (6):
  dt-bindings: media: rcar-vin: Describe optional ep properties
  dt-bindings: media: rcar-vin: Document data-active
  media: rcar-vin: Handle data-active property
  media: rcar-vin: Handle CLOCKENB pin polarity
  ARM: dts: rcar-gen2: Remove unused VIN properties
  ARM: dts: rcar-gen2: Add 'data-active' property

 Documentation/devicetree/bindings/media/rcar_vin.txt | 18 +++++++++++++++++-
 arch/arm/boot/dts/r8a7790-lager.dts                  |  4 +---
 arch/arm/boot/dts/r8a7791-koelsch.dts                |  4 +---
 arch/arm/boot/dts/r8a7791-porter.dts                 |  2 +-
 arch/arm/boot/dts/r8a7793-gose.dts                   |  4 +---
 arch/arm/boot/dts/r8a7794-alt.dts                    |  2 +-
 arch/arm/boot/dts/r8a7794-silk.dts                   |  2 +-
 drivers/media/platform/rcar-vin/rcar-core.c          | 10 ++++++++--
 drivers/media/platform/rcar-vin/rcar-dma.c           | 11 +++++++++++
 9 files changed, 42 insertions(+), 15 deletions(-)

--
2.7.4

[PATCH 1/6] dt-bindings: media: rcar-vin: Describe optional ep properties

[Jacopo Mondi]

Describe the optional endpoint properties for endpoint nodes of port@0
and port@1 of the R-Car VIN driver device tree bindings documentation.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 Documentation/devicetree/bindings/media/rcar_vin.txt | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt
index a19517e1..c53ce4e 100644
--- a/Documentation/devicetree/bindings/media/rcar_vin.txt
+++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
@@ -53,6 +53,16 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
       from external SoC pins described in video-interfaces.txt[1].
       Describing more then one endpoint in port 0 is invalid. Only VIN
       instances that are connected to external pins should have port 0.
+
+      - Optional properties for endpoint nodes of port@0:
+        - hsync-active: active state of the HSYNC signal, 0/1 for LOW/HIGH
+	  respectively. Default is active high.
+        - vsync-active: active state of the VSYNC signal, 0/1 for LOW/HIGH
+	  respectively. Default is active high.
+
+	If both HSYNC and VSYNC polarities are not specified, embedded
+	synchronization is selected.
+
     - port 1 - sub-nodes describing one or more endpoints connected to
       the VIN from local SoC CSI-2 receivers. The endpoint numbers must
       use the following schema.
@@ -62,6 +72,8 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
         - Endpoint 2 - sub-node describing the endpoint connected to CSI40
         - Endpoint 3 - sub-node describing the endpoint connected to CSI41

+      Endpoint nodes of port@1 do not support any optional endpoint property.
+
 Device node example for Gen2 platforms
 --------------------------------------

@@ -112,7 +124,6 @@ Board setup example for Gen2 platforms (vin1 composite video input)

                 vin1ep0: endpoint {
                         remote-endpoint = <&adv7180>;
-                        bus-width = <8>;
                 };
         };
 };
--
2.7.4

[PATCH 1/6] dt-bindings: media: rcar-vin: Describe optional ep properties

[Jacopo Mondi]

Describe the optional endpoint properties for endpoint nodes of port@0
and port@1 of the R-Car VIN driver device tree bindings documentation.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 Documentation/devicetree/bindings/media/rcar_vin.txt | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt
index a19517e1..c53ce4e 100644
--- a/Documentation/devicetree/bindings/media/rcar_vin.txt
+++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
@@ -53,6 +53,16 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
       from external SoC pins described in video-interfaces.txt[1].
       Describing more then one endpoint in port 0 is invalid. Only VIN
       instances that are connected to external pins should have port 0.
+
+      - Optional properties for endpoint nodes of port@0:
+        - hsync-active: active state of the HSYNC signal, 0/1 for LOW/HIGH
+	  respectively. Default is active high.
+        - vsync-active: active state of the VSYNC signal, 0/1 for LOW/HIGH
+	  respectively. Default is active high.
+
+	If both HSYNC and VSYNC polarities are not specified, embedded
+	synchronization is selected.
+
     - port 1 - sub-nodes describing one or more endpoints connected to
       the VIN from local SoC CSI-2 receivers. The endpoint numbers must
       use the following schema.
@@ -62,6 +72,8 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
         - Endpoint 2 - sub-node describing the endpoint connected to CSI40
         - Endpoint 3 - sub-node describing the endpoint connected to CSI41

+      Endpoint nodes of port@1 do not support any optional endpoint property.
+
 Device node example for Gen2 platforms
 --------------------------------------

@@ -112,7 +124,6 @@ Board setup example for Gen2 platforms (vin1 composite video input)

                 vin1ep0: endpoint {
                         remote-endpoint = <&adv7180>;
-                        bus-width = <8>;
                 };
         };
 };
--
2.7.4

[PATCH 1/6] dt-bindings: media: rcar-vin: Describe optional ep properties

[Jacopo Mondi]

Describe the optional endpoint properties for endpoint nodes of port at 0
and port at 1 of the R-Car VIN driver device tree bindings documentation.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 Documentation/devicetree/bindings/media/rcar_vin.txt | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt
index a19517e1..c53ce4e 100644
--- a/Documentation/devicetree/bindings/media/rcar_vin.txt
+++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
@@ -53,6 +53,16 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
       from external SoC pins described in video-interfaces.txt[1].
       Describing more then one endpoint in port 0 is invalid. Only VIN
       instances that are connected to external pins should have port 0.
+
+      - Optional properties for endpoint nodes of port at 0:
+        - hsync-active: active state of the HSYNC signal, 0/1 for LOW/HIGH
+	  respectively. Default is active high.
+        - vsync-active: active state of the VSYNC signal, 0/1 for LOW/HIGH
+	  respectively. Default is active high.
+
+	If both HSYNC and VSYNC polarities are not specified, embedded
+	synchronization is selected.
+
     - port 1 - sub-nodes describing one or more endpoints connected to
       the VIN from local SoC CSI-2 receivers. The endpoint numbers must
       use the following schema.
@@ -62,6 +72,8 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
         - Endpoint 2 - sub-node describing the endpoint connected to CSI40
         - Endpoint 3 - sub-node describing the endpoint connected to CSI41

+      Endpoint nodes of port at 1 do not support any optional endpoint property.
+
 Device node example for Gen2 platforms
 --------------------------------------

@@ -112,7 +124,6 @@ Board setup example for Gen2 platforms (vin1 composite video input)

                 vin1ep0: endpoint {
                         remote-endpoint = <&adv7180>;
-                        bus-width = <8>;
                 };
         };
 };
--
2.7.4

[PATCH 2/6] dt-bindings: media: rcar-vin: Document data-active

[Jacopo Mondi]

Document 'data-active' property in R-Car VIN device tree bindings.
The property is optional when running with explicit synchronization
(eg. BT.601) but mandatory when embedded synchronization is in use (eg.
BT.656) as specified by the hardware manual.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 Documentation/devicetree/bindings/media/rcar_vin.txt | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt
index c53ce4e..17eac8a 100644
--- a/Documentation/devicetree/bindings/media/rcar_vin.txt
+++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
@@ -63,6 +63,11 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
 	If both HSYNC and VSYNC polarities are not specified, embedded
 	synchronization is selected.

+        - data-active: active state of data enable signal (CLOCKENB pin).
+          0/1 for LOW/HIGH respectively. If not specified, use HSYNC as
+          data enable signal. When using embedded synchronization this
+          property is mandatory.
+
     - port 1 - sub-nodes describing one or more endpoints connected to
       the VIN from local SoC CSI-2 receivers. The endpoint numbers must
       use the following schema.
--
2.7.4

[PATCH 2/6] dt-bindings: media: rcar-vin: Document data-active

[Jacopo Mondi]

Document 'data-active' property in R-Car VIN device tree bindings.
The property is optional when running with explicit synchronization
(eg. BT.601) but mandatory when embedded synchronization is in use (eg.
BT.656) as specified by the hardware manual.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 Documentation/devicetree/bindings/media/rcar_vin.txt | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt
index c53ce4e..17eac8a 100644
--- a/Documentation/devicetree/bindings/media/rcar_vin.txt
+++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
@@ -63,6 +63,11 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
 	If both HSYNC and VSYNC polarities are not specified, embedded
 	synchronization is selected.

+        - data-active: active state of data enable signal (CLOCKENB pin).
+          0/1 for LOW/HIGH respectively. If not specified, use HSYNC as
+          data enable signal. When using embedded synchronization this
+          property is mandatory.
+
     - port 1 - sub-nodes describing one or more endpoints connected to
       the VIN from local SoC CSI-2 receivers. The endpoint numbers must
       use the following schema.
--
2.7.4

[PATCH 2/6] dt-bindings: media: rcar-vin: Document data-active

[Jacopo Mondi]

Document 'data-active' property in R-Car VIN device tree bindings.
The property is optional when running with explicit synchronization
(eg. BT.601) but mandatory when embedded synchronization is in use (eg.
BT.656) as specified by the hardware manual.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 Documentation/devicetree/bindings/media/rcar_vin.txt | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt
index c53ce4e..17eac8a 100644
--- a/Documentation/devicetree/bindings/media/rcar_vin.txt
+++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
@@ -63,6 +63,11 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
 	If both HSYNC and VSYNC polarities are not specified, embedded
 	synchronization is selected.

+        - data-active: active state of data enable signal (CLOCKENB pin).
+          0/1 for LOW/HIGH respectively. If not specified, use HSYNC as
+          data enable signal. When using embedded synchronization this
+          property is mandatory.
+
     - port 1 - sub-nodes describing one or more endpoints connected to
       the VIN from local SoC CSI-2 receivers. The endpoint numbers must
       use the following schema.
--
2.7.4

[PATCH 3/6] media: rcar-vin: Handle data-active property

[Jacopo Mondi]

The data-active property has to be specified when running with embedded
synchronization. The VIN peripheral can use HSYNC in place of CLOCKENB
when the CLOCKENB pin is not connected, this requires explicit
synchronization to be in use.

Now that the driver supports 'data-active' property, it makes not sense
to zero the mbus configuration flags when running with implicit synch
(V4L2_MBUS_BT656).

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 drivers/media/platform/rcar-vin/rcar-core.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
index d3072e1..075d08f 100644
--- a/drivers/media/platform/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/rcar-vin/rcar-core.c
@@ -531,15 +531,21 @@ static int rvin_digital_parse_v4l2(struct device *dev,
 		return -ENOTCONN;
 
 	vin->mbus_cfg.type = vep->bus_type;
+	vin->mbus_cfg.flags = vep->bus.parallel.flags;
 
 	switch (vin->mbus_cfg.type) {
 	case V4L2_MBUS_PARALLEL:
 		vin_dbg(vin, "Found PARALLEL media bus\n");
-		vin->mbus_cfg.flags = vep->bus.parallel.flags;
 		break;
 	case V4L2_MBUS_BT656:
 		vin_dbg(vin, "Found BT656 media bus\n");
-		vin->mbus_cfg.flags = 0;
+
+		if (!(vin->mbus_cfg.flags & V4L2_MBUS_DATA_ACTIVE_HIGH) &&
+		    !(vin->mbus_cfg.flags & V4L2_MBUS_DATA_ACTIVE_LOW)) {
+			vin_err(vin,
+				"Missing data enable signal polarity property\n");
+			return -EINVAL;
+		}
 		break;
 	default:
 		vin_err(vin, "Unknown media bus type\n");
-- 
2.7.4

[PATCH 4/6] media: rcar-vin: Handle CLOCKENB pin polarity

[Jacopo Mondi]

Handle CLOCKENB pin polarity, or use HSYNC in its place if polarity is
not specified.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 drivers/media/platform/rcar-vin/rcar-dma.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c
index ac07f99..7a84eae 100644
--- a/drivers/media/platform/rcar-vin/rcar-dma.c
+++ b/drivers/media/platform/rcar-vin/rcar-dma.c
@@ -123,6 +123,8 @@
 /* Video n Data Mode Register 2 bits */
 #define VNDMR2_VPS		(1 << 30)
 #define VNDMR2_HPS		(1 << 29)
+#define VNDMR2_CES		(1 << 28)
+#define VNDMR2_CHS		(1 << 23)
 #define VNDMR2_FTEV		(1 << 17)
 #define VNDMR2_VLV(n)		((n & 0xf) << 12)
 
@@ -691,6 +693,15 @@ static int rvin_setup(struct rvin_dev *vin)
 		dmr2 |= VNDMR2_VPS;
 
 	/*
+	 * Clock-enable active level select.
+	 * Use HSYNC as enable if not specified
+	 */
+	if (vin->mbus_cfg.flags & V4L2_MBUS_DATA_ACTIVE_LOW)
+		dmr2 |= VNDMR2_CES;
+	else if (!(vin->mbus_cfg.flags & V4L2_MBUS_DATA_ACTIVE_HIGH))
+		dmr2 |= VNDMR2_CHS;
+
+	/*
 	 * Output format
 	 */
 	switch (vin->format.pixelformat) {
-- 
2.7.4

[PATCH 5/6] ARM: dts: rcar-gen2: Remove unused VIN properties

[Jacopo Mondi]

The 'bus-width' and 'pclk-sample' properties are not parsed by the VIN
driver and only confuse users. Remove them in all Gen2 SoC that used
them.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 arch/arm/boot/dts/r8a7790-lager.dts   | 3 ---
 arch/arm/boot/dts/r8a7791-koelsch.dts | 3 ---
 arch/arm/boot/dts/r8a7791-porter.dts  | 1 -
 arch/arm/boot/dts/r8a7793-gose.dts    | 3 ---
 arch/arm/boot/dts/r8a7794-alt.dts     | 1 -
 arch/arm/boot/dts/r8a7794-silk.dts    | 1 -
 6 files changed, 12 deletions(-)

diff --git a/arch/arm/boot/dts/r8a7790-lager.dts b/arch/arm/boot/dts/r8a7790-lager.dts
index 063fdb6..b56b309 100644
--- a/arch/arm/boot/dts/r8a7790-lager.dts
+++ b/arch/arm/boot/dts/r8a7790-lager.dts
@@ -873,10 +873,8 @@
 	port {
 		vin0ep2: endpoint {
 			remote-endpoint = <&adv7612_out>;
-			bus-width = <24>;
 			hsync-active = <0>;
 			vsync-active = <0>;
-			pclk-sample = <1>;
 			data-active = <1>;
 		};
 	};
@@ -895,7 +893,6 @@

 		vin1ep0: endpoint {
 			remote-endpoint = <&adv7180>;
-			bus-width = <8>;
 		};
 	};
 };
diff --git a/arch/arm/boot/dts/r8a7791-koelsch.dts b/arch/arm/boot/dts/r8a7791-koelsch.dts
index f40321a..9967666 100644
--- a/arch/arm/boot/dts/r8a7791-koelsch.dts
+++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
@@ -849,10 +849,8 @@

 		vin0ep2: endpoint {
 			remote-endpoint = <&adv7612_out>;
-			bus-width = <24>;
 			hsync-active = <0>;
 			vsync-active = <0>;
-			pclk-sample = <1>;
 			data-active = <1>;
 		};
 	};
@@ -870,7 +868,6 @@

 		vin1ep: endpoint {
 			remote-endpoint = <&adv7180>;
-			bus-width = <8>;
 		};
 	};
 };
diff --git a/arch/arm/boot/dts/r8a7791-porter.dts b/arch/arm/boot/dts/r8a7791-porter.dts
index c14e6fe..055a7f1 100644
--- a/arch/arm/boot/dts/r8a7791-porter.dts
+++ b/arch/arm/boot/dts/r8a7791-porter.dts
@@ -391,7 +391,6 @@

 		vin0ep: endpoint {
 			remote-endpoint = <&adv7180>;
-			bus-width = <8>;
 		};
 	};
 };
diff --git a/arch/arm/boot/dts/r8a7793-gose.dts b/arch/arm/boot/dts/r8a7793-gose.dts
index 9ed6961..9d3fba2 100644
--- a/arch/arm/boot/dts/r8a7793-gose.dts
+++ b/arch/arm/boot/dts/r8a7793-gose.dts
@@ -759,10 +759,8 @@

 		vin0ep2: endpoint {
 			remote-endpoint = <&adv7612_out>;
-			bus-width = <24>;
 			hsync-active = <0>;
 			vsync-active = <0>;
-			pclk-sample = <1>;
 			data-active = <1>;
 		};
 	};
@@ -781,7 +779,6 @@

 		vin1ep: endpoint {
 			remote-endpoint = <&adv7180_out>;
-			bus-width = <8>;
 		};
 	};
 };
diff --git a/arch/arm/boot/dts/r8a7794-alt.dts b/arch/arm/boot/dts/r8a7794-alt.dts
index 26a8834..4bbb9cc 100644
--- a/arch/arm/boot/dts/r8a7794-alt.dts
+++ b/arch/arm/boot/dts/r8a7794-alt.dts
@@ -380,7 +380,6 @@

 		vin0ep: endpoint {
 			remote-endpoint = <&adv7180>;
-			bus-width = <8>;
 		};
 	};
 };
diff --git a/arch/arm/boot/dts/r8a7794-silk.dts b/arch/arm/boot/dts/r8a7794-silk.dts
index 351cb3b..c0c5d31 100644
--- a/arch/arm/boot/dts/r8a7794-silk.dts
+++ b/arch/arm/boot/dts/r8a7794-silk.dts
@@ -480,7 +480,6 @@

 		vin0ep: endpoint {
 			remote-endpoint = <&adv7180>;
-			bus-width = <8>;
 		};
 	};
 };
--
2.7.4

[PATCH 5/6] ARM: dts: rcar-gen2: Remove unused VIN properties

[Jacopo Mondi]

The 'bus-width' and 'pclk-sample' properties are not parsed by the VIN
driver and only confuse users. Remove them in all Gen2 SoC that used
them.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 arch/arm/boot/dts/r8a7790-lager.dts   | 3 ---
 arch/arm/boot/dts/r8a7791-koelsch.dts | 3 ---
 arch/arm/boot/dts/r8a7791-porter.dts  | 1 -
 arch/arm/boot/dts/r8a7793-gose.dts    | 3 ---
 arch/arm/boot/dts/r8a7794-alt.dts     | 1 -
 arch/arm/boot/dts/r8a7794-silk.dts    | 1 -
 6 files changed, 12 deletions(-)

diff --git a/arch/arm/boot/dts/r8a7790-lager.dts b/arch/arm/boot/dts/r8a7790-lager.dts
index 063fdb6..b56b309 100644
--- a/arch/arm/boot/dts/r8a7790-lager.dts
+++ b/arch/arm/boot/dts/r8a7790-lager.dts
@@ -873,10 +873,8 @@
 	port {
 		vin0ep2: endpoint {
 			remote-endpoint = <&adv7612_out>;
-			bus-width = <24>;
 			hsync-active = <0>;
 			vsync-active = <0>;
-			pclk-sample = <1>;
 			data-active = <1>;
 		};
 	};
@@ -895,7 +893,6 @@

 		vin1ep0: endpoint {
 			remote-endpoint = <&adv7180>;
-			bus-width = <8>;
 		};
 	};
 };
diff --git a/arch/arm/boot/dts/r8a7791-koelsch.dts b/arch/arm/boot/dts/r8a7791-koelsch.dts
index f40321a..9967666 100644
--- a/arch/arm/boot/dts/r8a7791-koelsch.dts
+++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
@@ -849,10 +849,8 @@

 		vin0ep2: endpoint {
 			remote-endpoint = <&adv7612_out>;
-			bus-width = <24>;
 			hsync-active = <0>;
 			vsync-active = <0>;
-			pclk-sample = <1>;
 			data-active = <1>;
 		};
 	};
@@ -870,7 +868,6 @@

 		vin1ep: endpoint {
 			remote-endpoint = <&adv7180>;
-			bus-width = <8>;
 		};
 	};
 };
diff --git a/arch/arm/boot/dts/r8a7791-porter.dts b/arch/arm/boot/dts/r8a7791-porter.dts
index c14e6fe..055a7f1 100644
--- a/arch/arm/boot/dts/r8a7791-porter.dts
+++ b/arch/arm/boot/dts/r8a7791-porter.dts
@@ -391,7 +391,6 @@

 		vin0ep: endpoint {
 			remote-endpoint = <&adv7180>;
-			bus-width = <8>;
 		};
 	};
 };
diff --git a/arch/arm/boot/dts/r8a7793-gose.dts b/arch/arm/boot/dts/r8a7793-gose.dts
index 9ed6961..9d3fba2 100644
--- a/arch/arm/boot/dts/r8a7793-gose.dts
+++ b/arch/arm/boot/dts/r8a7793-gose.dts
@@ -759,10 +759,8 @@

 		vin0ep2: endpoint {
 			remote-endpoint = <&adv7612_out>;
-			bus-width = <24>;
 			hsync-active = <0>;
 			vsync-active = <0>;
-			pclk-sample = <1>;
 			data-active = <1>;
 		};
 	};
@@ -781,7 +779,6 @@

 		vin1ep: endpoint {
 			remote-endpoint = <&adv7180_out>;
-			bus-width = <8>;
 		};
 	};
 };
diff --git a/arch/arm/boot/dts/r8a7794-alt.dts b/arch/arm/boot/dts/r8a7794-alt.dts
index 26a8834..4bbb9cc 100644
--- a/arch/arm/boot/dts/r8a7794-alt.dts
+++ b/arch/arm/boot/dts/r8a7794-alt.dts
@@ -380,7 +380,6 @@

 		vin0ep: endpoint {
 			remote-endpoint = <&adv7180>;
-			bus-width = <8>;
 		};
 	};
 };
diff --git a/arch/arm/boot/dts/r8a7794-silk.dts b/arch/arm/boot/dts/r8a7794-silk.dts
index 351cb3b..c0c5d31 100644
--- a/arch/arm/boot/dts/r8a7794-silk.dts
+++ b/arch/arm/boot/dts/r8a7794-silk.dts
@@ -480,7 +480,6 @@

 		vin0ep: endpoint {
 			remote-endpoint = <&adv7180>;
-			bus-width = <8>;
 		};
 	};
 };
--
2.7.4

[PATCH 5/6] ARM: dts: rcar-gen2: Remove unused VIN properties

[Jacopo Mondi]

The 'bus-width' and 'pclk-sample' properties are not parsed by the VIN
driver and only confuse users. Remove them in all Gen2 SoC that used
them.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 arch/arm/boot/dts/r8a7790-lager.dts   | 3 ---
 arch/arm/boot/dts/r8a7791-koelsch.dts | 3 ---
 arch/arm/boot/dts/r8a7791-porter.dts  | 1 -
 arch/arm/boot/dts/r8a7793-gose.dts    | 3 ---
 arch/arm/boot/dts/r8a7794-alt.dts     | 1 -
 arch/arm/boot/dts/r8a7794-silk.dts    | 1 -
 6 files changed, 12 deletions(-)

diff --git a/arch/arm/boot/dts/r8a7790-lager.dts b/arch/arm/boot/dts/r8a7790-lager.dts
index 063fdb6..b56b309 100644
--- a/arch/arm/boot/dts/r8a7790-lager.dts
+++ b/arch/arm/boot/dts/r8a7790-lager.dts
@@ -873,10 +873,8 @@
 	port {
 		vin0ep2: endpoint {
 			remote-endpoint = <&adv7612_out>;
-			bus-width = <24>;
 			hsync-active = <0>;
 			vsync-active = <0>;
-			pclk-sample = <1>;
 			data-active = <1>;
 		};
 	};
@@ -895,7 +893,6 @@

 		vin1ep0: endpoint {
 			remote-endpoint = <&adv7180>;
-			bus-width = <8>;
 		};
 	};
 };
diff --git a/arch/arm/boot/dts/r8a7791-koelsch.dts b/arch/arm/boot/dts/r8a7791-koelsch.dts
index f40321a..9967666 100644
--- a/arch/arm/boot/dts/r8a7791-koelsch.dts
+++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
@@ -849,10 +849,8 @@

 		vin0ep2: endpoint {
 			remote-endpoint = <&adv7612_out>;
-			bus-width = <24>;
 			hsync-active = <0>;
 			vsync-active = <0>;
-			pclk-sample = <1>;
 			data-active = <1>;
 		};
 	};
@@ -870,7 +868,6 @@

 		vin1ep: endpoint {
 			remote-endpoint = <&adv7180>;
-			bus-width = <8>;
 		};
 	};
 };
diff --git a/arch/arm/boot/dts/r8a7791-porter.dts b/arch/arm/boot/dts/r8a7791-porter.dts
index c14e6fe..055a7f1 100644
--- a/arch/arm/boot/dts/r8a7791-porter.dts
+++ b/arch/arm/boot/dts/r8a7791-porter.dts
@@ -391,7 +391,6 @@

 		vin0ep: endpoint {
 			remote-endpoint = <&adv7180>;
-			bus-width = <8>;
 		};
 	};
 };
diff --git a/arch/arm/boot/dts/r8a7793-gose.dts b/arch/arm/boot/dts/r8a7793-gose.dts
index 9ed6961..9d3fba2 100644
--- a/arch/arm/boot/dts/r8a7793-gose.dts
+++ b/arch/arm/boot/dts/r8a7793-gose.dts
@@ -759,10 +759,8 @@

 		vin0ep2: endpoint {
 			remote-endpoint = <&adv7612_out>;
-			bus-width = <24>;
 			hsync-active = <0>;
 			vsync-active = <0>;
-			pclk-sample = <1>;
 			data-active = <1>;
 		};
 	};
@@ -781,7 +779,6 @@

 		vin1ep: endpoint {
 			remote-endpoint = <&adv7180_out>;
-			bus-width = <8>;
 		};
 	};
 };
diff --git a/arch/arm/boot/dts/r8a7794-alt.dts b/arch/arm/boot/dts/r8a7794-alt.dts
index 26a8834..4bbb9cc 100644
--- a/arch/arm/boot/dts/r8a7794-alt.dts
+++ b/arch/arm/boot/dts/r8a7794-alt.dts
@@ -380,7 +380,6 @@

 		vin0ep: endpoint {
 			remote-endpoint = <&adv7180>;
-			bus-width = <8>;
 		};
 	};
 };
diff --git a/arch/arm/boot/dts/r8a7794-silk.dts b/arch/arm/boot/dts/r8a7794-silk.dts
index 351cb3b..c0c5d31 100644
--- a/arch/arm/boot/dts/r8a7794-silk.dts
+++ b/arch/arm/boot/dts/r8a7794-silk.dts
@@ -480,7 +480,6 @@

 		vin0ep: endpoint {
 			remote-endpoint = <&adv7180>;
-			bus-width = <8>;
 		};
 	};
 };
--
2.7.4

[PATCH 6/6] ARM: dts: rcar-gen2: Add 'data-active' property

[Jacopo Mondi]

The 'data-active' property needs to be specified when using embedded
synchronization. Add it to the Gen-2 boards using composite video input.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 arch/arm/boot/dts/r8a7790-lager.dts   | 1 +
 arch/arm/boot/dts/r8a7791-koelsch.dts | 1 +
 arch/arm/boot/dts/r8a7791-porter.dts  | 1 +
 arch/arm/boot/dts/r8a7793-gose.dts    | 1 +
 arch/arm/boot/dts/r8a7794-alt.dts     | 1 +
 arch/arm/boot/dts/r8a7794-silk.dts    | 1 +
 6 files changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/r8a7790-lager.dts b/arch/arm/boot/dts/r8a7790-lager.dts
index b56b309..48fcb44 100644
--- a/arch/arm/boot/dts/r8a7790-lager.dts
+++ b/arch/arm/boot/dts/r8a7790-lager.dts
@@ -893,6 +893,7 @@

 		vin1ep0: endpoint {
 			remote-endpoint = <&adv7180>;
+			data-active = <1>;
 		};
 	};
 };
diff --git a/arch/arm/boot/dts/r8a7791-koelsch.dts b/arch/arm/boot/dts/r8a7791-koelsch.dts
index 9967666..fa0b25f 100644
--- a/arch/arm/boot/dts/r8a7791-koelsch.dts
+++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
@@ -868,6 +868,7 @@

 		vin1ep: endpoint {
 			remote-endpoint = <&adv7180>;
+			data-active = <1>;
 		};
 	};
 };
diff --git a/arch/arm/boot/dts/r8a7791-porter.dts b/arch/arm/boot/dts/r8a7791-porter.dts
index 055a7f1..96b9605 100644
--- a/arch/arm/boot/dts/r8a7791-porter.dts
+++ b/arch/arm/boot/dts/r8a7791-porter.dts
@@ -391,6 +391,7 @@

 		vin0ep: endpoint {
 			remote-endpoint = <&adv7180>;
+			data-active = <1>;
 		};
 	};
 };
diff --git a/arch/arm/boot/dts/r8a7793-gose.dts b/arch/arm/boot/dts/r8a7793-gose.dts
index 9d3fba2..80b4fa8 100644
--- a/arch/arm/boot/dts/r8a7793-gose.dts
+++ b/arch/arm/boot/dts/r8a7793-gose.dts
@@ -779,6 +779,7 @@

 		vin1ep: endpoint {
 			remote-endpoint = <&adv7180_out>;
+			data-active = <1>;
 		};
 	};
 };
diff --git a/arch/arm/boot/dts/r8a7794-alt.dts b/arch/arm/boot/dts/r8a7794-alt.dts
index 4bbb9cc..00df605d 100644
--- a/arch/arm/boot/dts/r8a7794-alt.dts
+++ b/arch/arm/boot/dts/r8a7794-alt.dts
@@ -380,6 +380,7 @@

 		vin0ep: endpoint {
 			remote-endpoint = <&adv7180>;
+			data-active = <1>;
 		};
 	};
 };
diff --git a/arch/arm/boot/dts/r8a7794-silk.dts b/arch/arm/boot/dts/r8a7794-silk.dts
index c0c5d31..ed17376 100644
--- a/arch/arm/boot/dts/r8a7794-silk.dts
+++ b/arch/arm/boot/dts/r8a7794-silk.dts
@@ -480,6 +480,7 @@

 		vin0ep: endpoint {
 			remote-endpoint = <&adv7180>;
+			data-active = <1>;
 		};
 	};
 };
--
2.7.4

[PATCH 6/6] ARM: dts: rcar-gen2: Add 'data-active' property

[Jacopo Mondi]

The 'data-active' property needs to be specified when using embedded
synchronization. Add it to the Gen-2 boards using composite video input.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 arch/arm/boot/dts/r8a7790-lager.dts   | 1 +
 arch/arm/boot/dts/r8a7791-koelsch.dts | 1 +
 arch/arm/boot/dts/r8a7791-porter.dts  | 1 +
 arch/arm/boot/dts/r8a7793-gose.dts    | 1 +
 arch/arm/boot/dts/r8a7794-alt.dts     | 1 +
 arch/arm/boot/dts/r8a7794-silk.dts    | 1 +
 6 files changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/r8a7790-lager.dts b/arch/arm/boot/dts/r8a7790-lager.dts
index b56b309..48fcb44 100644
--- a/arch/arm/boot/dts/r8a7790-lager.dts
+++ b/arch/arm/boot/dts/r8a7790-lager.dts
@@ -893,6 +893,7 @@

 		vin1ep0: endpoint {
 			remote-endpoint = <&adv7180>;
+			data-active = <1>;
 		};
 	};
 };
diff --git a/arch/arm/boot/dts/r8a7791-koelsch.dts b/arch/arm/boot/dts/r8a7791-koelsch.dts
index 9967666..fa0b25f 100644
--- a/arch/arm/boot/dts/r8a7791-koelsch.dts
+++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
@@ -868,6 +868,7 @@

 		vin1ep: endpoint {
 			remote-endpoint = <&adv7180>;
+			data-active = <1>;
 		};
 	};
 };
diff --git a/arch/arm/boot/dts/r8a7791-porter.dts b/arch/arm/boot/dts/r8a7791-porter.dts
index 055a7f1..96b9605 100644
--- a/arch/arm/boot/dts/r8a7791-porter.dts
+++ b/arch/arm/boot/dts/r8a7791-porter.dts
@@ -391,6 +391,7 @@

 		vin0ep: endpoint {
 			remote-endpoint = <&adv7180>;
+			data-active = <1>;
 		};
 	};
 };
diff --git a/arch/arm/boot/dts/r8a7793-gose.dts b/arch/arm/boot/dts/r8a7793-gose.dts
index 9d3fba2..80b4fa8 100644
--- a/arch/arm/boot/dts/r8a7793-gose.dts
+++ b/arch/arm/boot/dts/r8a7793-gose.dts
@@ -779,6 +779,7 @@

 		vin1ep: endpoint {
 			remote-endpoint = <&adv7180_out>;
+			data-active = <1>;
 		};
 	};
 };
diff --git a/arch/arm/boot/dts/r8a7794-alt.dts b/arch/arm/boot/dts/r8a7794-alt.dts
index 4bbb9cc..00df605d 100644
--- a/arch/arm/boot/dts/r8a7794-alt.dts
+++ b/arch/arm/boot/dts/r8a7794-alt.dts
@@ -380,6 +380,7 @@

 		vin0ep: endpoint {
 			remote-endpoint = <&adv7180>;
+			data-active = <1>;
 		};
 	};
 };
diff --git a/arch/arm/boot/dts/r8a7794-silk.dts b/arch/arm/boot/dts/r8a7794-silk.dts
index c0c5d31..ed17376 100644
--- a/arch/arm/boot/dts/r8a7794-silk.dts
+++ b/arch/arm/boot/dts/r8a7794-silk.dts
@@ -480,6 +480,7 @@

 		vin0ep: endpoint {
 			remote-endpoint = <&adv7180>;
+			data-active = <1>;
 		};
 	};
 };
--
2.7.4

[PATCH 6/6] ARM: dts: rcar-gen2: Add 'data-active' property

[Jacopo Mondi]

The 'data-active' property needs to be specified when using embedded
synchronization. Add it to the Gen-2 boards using composite video input.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 arch/arm/boot/dts/r8a7790-lager.dts   | 1 +
 arch/arm/boot/dts/r8a7791-koelsch.dts | 1 +
 arch/arm/boot/dts/r8a7791-porter.dts  | 1 +
 arch/arm/boot/dts/r8a7793-gose.dts    | 1 +
 arch/arm/boot/dts/r8a7794-alt.dts     | 1 +
 arch/arm/boot/dts/r8a7794-silk.dts    | 1 +
 6 files changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/r8a7790-lager.dts b/arch/arm/boot/dts/r8a7790-lager.dts
index b56b309..48fcb44 100644
--- a/arch/arm/boot/dts/r8a7790-lager.dts
+++ b/arch/arm/boot/dts/r8a7790-lager.dts
@@ -893,6 +893,7 @@

 		vin1ep0: endpoint {
 			remote-endpoint = <&adv7180>;
+			data-active = <1>;
 		};
 	};
 };
diff --git a/arch/arm/boot/dts/r8a7791-koelsch.dts b/arch/arm/boot/dts/r8a7791-koelsch.dts
index 9967666..fa0b25f 100644
--- a/arch/arm/boot/dts/r8a7791-koelsch.dts
+++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
@@ -868,6 +868,7 @@

 		vin1ep: endpoint {
 			remote-endpoint = <&adv7180>;
+			data-active = <1>;
 		};
 	};
 };
diff --git a/arch/arm/boot/dts/r8a7791-porter.dts b/arch/arm/boot/dts/r8a7791-porter.dts
index 055a7f1..96b9605 100644
--- a/arch/arm/boot/dts/r8a7791-porter.dts
+++ b/arch/arm/boot/dts/r8a7791-porter.dts
@@ -391,6 +391,7 @@

 		vin0ep: endpoint {
 			remote-endpoint = <&adv7180>;
+			data-active = <1>;
 		};
 	};
 };
diff --git a/arch/arm/boot/dts/r8a7793-gose.dts b/arch/arm/boot/dts/r8a7793-gose.dts
index 9d3fba2..80b4fa8 100644
--- a/arch/arm/boot/dts/r8a7793-gose.dts
+++ b/arch/arm/boot/dts/r8a7793-gose.dts
@@ -779,6 +779,7 @@

 		vin1ep: endpoint {
 			remote-endpoint = <&adv7180_out>;
+			data-active = <1>;
 		};
 	};
 };
diff --git a/arch/arm/boot/dts/r8a7794-alt.dts b/arch/arm/boot/dts/r8a7794-alt.dts
index 4bbb9cc..00df605d 100644
--- a/arch/arm/boot/dts/r8a7794-alt.dts
+++ b/arch/arm/boot/dts/r8a7794-alt.dts
@@ -380,6 +380,7 @@

 		vin0ep: endpoint {
 			remote-endpoint = <&adv7180>;
+			data-active = <1>;
 		};
 	};
 };
diff --git a/arch/arm/boot/dts/r8a7794-silk.dts b/arch/arm/boot/dts/r8a7794-silk.dts
index c0c5d31..ed17376 100644
--- a/arch/arm/boot/dts/r8a7794-silk.dts
+++ b/arch/arm/boot/dts/r8a7794-silk.dts
@@ -480,6 +480,7 @@

 		vin0ep: endpoint {
 			remote-endpoint = <&adv7180>;
+			data-active = <1>;
 		};
 	};
 };
--
2.7.4

Re: [PATCH 0/6] media: rcar-vin: Brush endpoint properties

[Niklas Söderlund]

Hi Jacopo,

On 2018-05-16 18:32:26 +0200, Jacopo Mondi wrote:
> Hello,
>    this series touches the bindings and the driver handling endpoint
> properties for digital subdevices of the R-Car VIN driver.
> 
> The first patch simply documents what are the endpoint properties supported
> at the moment, then the second one extends them with 'data-active'.
> 
> As the VIN hardware allows to use HSYNC as data enable signal when the CLCKENB
> pin is left unconnected, the 'data-active' property presence determinates
> if HSYNC has to be used or not as data enable signal. As a consequence, when
> running with embedded synchronism, and there is not HSYNC signal, it becomes
> mandatory to specify 'data-active' polarity in DTS.
> 
> To address this, all Gen-2 boards featuring a composite video input and
> running with embedded synchronization, now need that property to be specified
> in DTS. Before adding it, remove un-used properties as 'pclk-sample' and
> 'bus-width' from the Gen-2 bindings, as they are not parsed by the VIN driver
> and only confuse users.
> 
> Niklas, as you already know I don't have any Gen2 board. Could you give this
> a spin on Koelsch if you like the series?

I tested this on my Koelsch and capture is still working :-)

> 
> Thanks
>    j
> 
> Jacopo Mondi (6):
>   dt-bindings: media: rcar-vin: Describe optional ep properties
>   dt-bindings: media: rcar-vin: Document data-active
>   media: rcar-vin: Handle data-active property
>   media: rcar-vin: Handle CLOCKENB pin polarity
>   ARM: dts: rcar-gen2: Remove unused VIN properties
>   ARM: dts: rcar-gen2: Add 'data-active' property
> 
>  Documentation/devicetree/bindings/media/rcar_vin.txt | 18 +++++++++++++++++-
>  arch/arm/boot/dts/r8a7790-lager.dts                  |  4 +---
>  arch/arm/boot/dts/r8a7791-koelsch.dts                |  4 +---
>  arch/arm/boot/dts/r8a7791-porter.dts                 |  2 +-
>  arch/arm/boot/dts/r8a7793-gose.dts                   |  4 +---
>  arch/arm/boot/dts/r8a7794-alt.dts                    |  2 +-
>  arch/arm/boot/dts/r8a7794-silk.dts                   |  2 +-
>  drivers/media/platform/rcar-vin/rcar-core.c          | 10 ++++++++--
>  drivers/media/platform/rcar-vin/rcar-dma.c           | 11 +++++++++++
>  9 files changed, 42 insertions(+), 15 deletions(-)
> 
> --
> 2.7.4
> 

-- 
Regards,
Niklas Söderlund

Re: [PATCH 0/6] media: rcar-vin: Brush endpoint properties

[Niklas Söderlund]

Hi Jacopo,

On 2018-05-16 18:32:26 +0200, Jacopo Mondi wrote:
> Hello,
>    this series touches the bindings and the driver handling endpoint
> properties for digital subdevices of the R-Car VIN driver.
> 
> The first patch simply documents what are the endpoint properties supported
> at the moment, then the second one extends them with 'data-active'.
> 
> As the VIN hardware allows to use HSYNC as data enable signal when the CLCKENB
> pin is left unconnected, the 'data-active' property presence determinates
> if HSYNC has to be used or not as data enable signal. As a consequence, when
> running with embedded synchronism, and there is not HSYNC signal, it becomes
> mandatory to specify 'data-active' polarity in DTS.
> 
> To address this, all Gen-2 boards featuring a composite video input and
> running with embedded synchronization, now need that property to be specified
> in DTS. Before adding it, remove un-used properties as 'pclk-sample' and
> 'bus-width' from the Gen-2 bindings, as they are not parsed by the VIN driver
> and only confuse users.
> 
> Niklas, as you already know I don't have any Gen2 board. Could you give this
> a spin on Koelsch if you like the series?

I tested this on my Koelsch and capture is still working :-)

> 
> Thanks
>    j
> 
> Jacopo Mondi (6):
>   dt-bindings: media: rcar-vin: Describe optional ep properties
>   dt-bindings: media: rcar-vin: Document data-active
>   media: rcar-vin: Handle data-active property
>   media: rcar-vin: Handle CLOCKENB pin polarity
>   ARM: dts: rcar-gen2: Remove unused VIN properties
>   ARM: dts: rcar-gen2: Add 'data-active' property
> 
>  Documentation/devicetree/bindings/media/rcar_vin.txt | 18 +++++++++++++++++-
>  arch/arm/boot/dts/r8a7790-lager.dts                  |  4 +---
>  arch/arm/boot/dts/r8a7791-koelsch.dts                |  4 +---
>  arch/arm/boot/dts/r8a7791-porter.dts                 |  2 +-
>  arch/arm/boot/dts/r8a7793-gose.dts                   |  4 +---
>  arch/arm/boot/dts/r8a7794-alt.dts                    |  2 +-
>  arch/arm/boot/dts/r8a7794-silk.dts                   |  2 +-
>  drivers/media/platform/rcar-vin/rcar-core.c          | 10 ++++++++--
>  drivers/media/platform/rcar-vin/rcar-dma.c           | 11 +++++++++++
>  9 files changed, 42 insertions(+), 15 deletions(-)
> 
> --
> 2.7.4
> 

-- 
Regards,
Niklas S�derlund

Re: [PATCH 1/6] dt-bindings: media: rcar-vin: Describe optional ep properties

[Niklas Söderlund]

Hi Jacopo,

Thanks for your patch.

On 2018-05-16 18:32:27 +0200, Jacopo Mondi wrote:
> Describe the optional endpoint properties for endpoint nodes of port@0
> and port@1 of the R-Car VIN driver device tree bindings documentation.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

Acked-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

> ---
>  Documentation/devicetree/bindings/media/rcar_vin.txt | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt
> index a19517e1..c53ce4e 100644
> --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
> +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
> @@ -53,6 +53,16 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
>        from external SoC pins described in video-interfaces.txt[1].
>        Describing more then one endpoint in port 0 is invalid. Only VIN
>        instances that are connected to external pins should have port 0.
> +
> +      - Optional properties for endpoint nodes of port@0:
> +        - hsync-active: active state of the HSYNC signal, 0/1 for LOW/HIGH
> +	  respectively. Default is active high.
> +        - vsync-active: active state of the VSYNC signal, 0/1 for LOW/HIGH
> +	  respectively. Default is active high.
> +
> +	If both HSYNC and VSYNC polarities are not specified, embedded
> +	synchronization is selected.
> +
>      - port 1 - sub-nodes describing one or more endpoints connected to
>        the VIN from local SoC CSI-2 receivers. The endpoint numbers must
>        use the following schema.
> @@ -62,6 +72,8 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
>          - Endpoint 2 - sub-node describing the endpoint connected to CSI40
>          - Endpoint 3 - sub-node describing the endpoint connected to CSI41
> 
> +      Endpoint nodes of port@1 do not support any optional endpoint property.
> +
>  Device node example for Gen2 platforms
>  --------------------------------------
> 
> @@ -112,7 +124,6 @@ Board setup example for Gen2 platforms (vin1 composite video input)
> 
>                  vin1ep0: endpoint {
>                          remote-endpoint = <&adv7180>;
> -                        bus-width = <8>;
>                  };
>          };
>  };
> --
> 2.7.4
> 

-- 
Regards,
Niklas Söderlund

Re: [PATCH 1/6] dt-bindings: media: rcar-vin: Describe optional ep properties

[Niklas Söderlund]

Hi Jacopo,

Thanks for your patch.

On 2018-05-16 18:32:27 +0200, Jacopo Mondi wrote:
> Describe the optional endpoint properties for endpoint nodes of port@0
> and port@1 of the R-Car VIN driver device tree bindings documentation.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

Acked-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

> ---
>  Documentation/devicetree/bindings/media/rcar_vin.txt | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt
> index a19517e1..c53ce4e 100644
> --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
> +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
> @@ -53,6 +53,16 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
>        from external SoC pins described in video-interfaces.txt[1].
>        Describing more then one endpoint in port 0 is invalid. Only VIN
>        instances that are connected to external pins should have port 0.
> +
> +      - Optional properties for endpoint nodes of port@0:
> +        - hsync-active: active state of the HSYNC signal, 0/1 for LOW/HIGH
> +	  respectively. Default is active high.
> +        - vsync-active: active state of the VSYNC signal, 0/1 for LOW/HIGH
> +	  respectively. Default is active high.
> +
> +	If both HSYNC and VSYNC polarities are not specified, embedded
> +	synchronization is selected.
> +
>      - port 1 - sub-nodes describing one or more endpoints connected to
>        the VIN from local SoC CSI-2 receivers. The endpoint numbers must
>        use the following schema.
> @@ -62,6 +72,8 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
>          - Endpoint 2 - sub-node describing the endpoint connected to CSI40
>          - Endpoint 3 - sub-node describing the endpoint connected to CSI41
> 
> +      Endpoint nodes of port@1 do not support any optional endpoint property.
> +
>  Device node example for Gen2 platforms
>  --------------------------------------
> 
> @@ -112,7 +124,6 @@ Board setup example for Gen2 platforms (vin1 composite video input)
> 
>                  vin1ep0: endpoint {
>                          remote-endpoint = <&adv7180>;
> -                        bus-width = <8>;
>                  };
>          };
>  };
> --
> 2.7.4
> 

-- 
Regards,
Niklas Söderlund

Re: [PATCH 1/6] dt-bindings: media: rcar-vin: Describe optional ep properties

[Niklas Söderlund]

Hi Jacopo,

Thanks for your patch.

On 2018-05-16 18:32:27 +0200, Jacopo Mondi wrote:
> Describe the optional endpoint properties for endpoint nodes of port@0
> and port@1 of the R-Car VIN driver device tree bindings documentation.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

Acked-by: Niklas S�derlund <niklas.soderlund+renesas@ragnatech.se>

> ---
>  Documentation/devicetree/bindings/media/rcar_vin.txt | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt
> index a19517e1..c53ce4e 100644
> --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
> +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
> @@ -53,6 +53,16 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
>        from external SoC pins described in video-interfaces.txt[1].
>        Describing more then one endpoint in port 0 is invalid. Only VIN
>        instances that are connected to external pins should have port 0.
> +
> +      - Optional properties for endpoint nodes of port@0:
> +        - hsync-active: active state of the HSYNC signal, 0/1 for LOW/HIGH
> +	  respectively. Default is active high.
> +        - vsync-active: active state of the VSYNC signal, 0/1 for LOW/HIGH
> +	  respectively. Default is active high.
> +
> +	If both HSYNC and VSYNC polarities are not specified, embedded
> +	synchronization is selected.
> +
>      - port 1 - sub-nodes describing one or more endpoints connected to
>        the VIN from local SoC CSI-2 receivers. The endpoint numbers must
>        use the following schema.
> @@ -62,6 +72,8 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
>          - Endpoint 2 - sub-node describing the endpoint connected to CSI40
>          - Endpoint 3 - sub-node describing the endpoint connected to CSI41
> 
> +      Endpoint nodes of port@1 do not support any optional endpoint property.
> +
>  Device node example for Gen2 platforms
>  --------------------------------------
> 
> @@ -112,7 +124,6 @@ Board setup example for Gen2 platforms (vin1 composite video input)
> 
>                  vin1ep0: endpoint {
>                          remote-endpoint = <&adv7180>;
> -                        bus-width = <8>;
>                  };
>          };
>  };
> --
> 2.7.4
> 

-- 
Regards,
Niklas S�derlund

[PATCH 1/6] dt-bindings: media: rcar-vin: Describe optional ep properties

[Niklas Söderlund]

Hi Jacopo,

Thanks for your patch.

On 2018-05-16 18:32:27 +0200, Jacopo Mondi wrote:
> Describe the optional endpoint properties for endpoint nodes of port at 0
> and port at 1 of the R-Car VIN driver device tree bindings documentation.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

Acked-by: Niklas S?derlund <niklas.soderlund+renesas@ragnatech.se>

> ---
>  Documentation/devicetree/bindings/media/rcar_vin.txt | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt
> index a19517e1..c53ce4e 100644
> --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
> +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
> @@ -53,6 +53,16 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
>        from external SoC pins described in video-interfaces.txt[1].
>        Describing more then one endpoint in port 0 is invalid. Only VIN
>        instances that are connected to external pins should have port 0.
> +
> +      - Optional properties for endpoint nodes of port at 0:
> +        - hsync-active: active state of the HSYNC signal, 0/1 for LOW/HIGH
> +	  respectively. Default is active high.
> +        - vsync-active: active state of the VSYNC signal, 0/1 for LOW/HIGH
> +	  respectively. Default is active high.
> +
> +	If both HSYNC and VSYNC polarities are not specified, embedded
> +	synchronization is selected.
> +
>      - port 1 - sub-nodes describing one or more endpoints connected to
>        the VIN from local SoC CSI-2 receivers. The endpoint numbers must
>        use the following schema.
> @@ -62,6 +72,8 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
>          - Endpoint 2 - sub-node describing the endpoint connected to CSI40
>          - Endpoint 3 - sub-node describing the endpoint connected to CSI41
> 
> +      Endpoint nodes of port at 1 do not support any optional endpoint property.
> +
>  Device node example for Gen2 platforms
>  --------------------------------------
> 
> @@ -112,7 +124,6 @@ Board setup example for Gen2 platforms (vin1 composite video input)
> 
>                  vin1ep0: endpoint {
>                          remote-endpoint = <&adv7180>;
> -                        bus-width = <8>;
>                  };
>          };
>  };
> --
> 2.7.4
> 

-- 
Regards,
Niklas S?derlund

Re: [PATCH 2/6] dt-bindings: media: rcar-vin: Document data-active

[Niklas Söderlund]

Hi Jacopo,

Thanks for your work.

On 2018-05-16 18:32:28 +0200, Jacopo Mondi wrote:
> Document 'data-active' property in R-Car VIN device tree bindings.
> The property is optional when running with explicit synchronization
> (eg. BT.601) but mandatory when embedded synchronization is in use (eg.
> BT.656) as specified by the hardware manual.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
>  Documentation/devicetree/bindings/media/rcar_vin.txt | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt
> index c53ce4e..17eac8a 100644
> --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
> +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
> @@ -63,6 +63,11 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
>  	If both HSYNC and VSYNC polarities are not specified, embedded
>  	synchronization is selected.
> 
> +        - data-active: active state of data enable signal (CLOCKENB pin).

I'm not sure what you mean by active state here. video-interfaces.txt 
defines data-active as 'similar to HSYNC and VSYNC, specifies data line 
polarity' so I assume this is the polarity of the CLOCKENB pin?

> +          0/1 for LOW/HIGH respectively. If not specified, use HSYNC as
> +          data enable signal. When using embedded synchronization this
> +          property is mandatory.

I'm confused, why is this mandatory if we have no embedded sync (that is 
hsync-active and vsync-active not defined)? I can't find any reference 
to this in the Gen2 datasheet but I'm sure I'm just missing it :-)

> +
>      - port 1 - sub-nodes describing one or more endpoints connected to
>        the VIN from local SoC CSI-2 receivers. The endpoint numbers must
>        use the following schema.
> --
> 2.7.4
> 

-- 
Regards,
Niklas Söderlund

Re: [PATCH 2/6] dt-bindings: media: rcar-vin: Document data-active

[Niklas Söderlund]

Hi Jacopo,

Thanks for your work.

On 2018-05-16 18:32:28 +0200, Jacopo Mondi wrote:
> Document 'data-active' property in R-Car VIN device tree bindings.
> The property is optional when running with explicit synchronization
> (eg. BT.601) but mandatory when embedded synchronization is in use (eg.
> BT.656) as specified by the hardware manual.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
>  Documentation/devicetree/bindings/media/rcar_vin.txt | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt
> index c53ce4e..17eac8a 100644
> --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
> +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
> @@ -63,6 +63,11 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
>  	If both HSYNC and VSYNC polarities are not specified, embedded
>  	synchronization is selected.
> 
> +        - data-active: active state of data enable signal (CLOCKENB pin).

I'm not sure what you mean by active state here. video-interfaces.txt 
defines data-active as 'similar to HSYNC and VSYNC, specifies data line 
polarity' so I assume this is the polarity of the CLOCKENB pin?

> +          0/1 for LOW/HIGH respectively. If not specified, use HSYNC as
> +          data enable signal. When using embedded synchronization this
> +          property is mandatory.

I'm confused, why is this mandatory if we have no embedded sync (that is 
hsync-active and vsync-active not defined)? I can't find any reference 
to this in the Gen2 datasheet but I'm sure I'm just missing it :-)

> +
>      - port 1 - sub-nodes describing one or more endpoints connected to
>        the VIN from local SoC CSI-2 receivers. The endpoint numbers must
>        use the following schema.
> --
> 2.7.4
> 

-- 
Regards,
Niklas Söderlund

Re: [PATCH 2/6] dt-bindings: media: rcar-vin: Document data-active

[Niklas Söderlund]

Hi Jacopo,

Thanks for your work.

On 2018-05-16 18:32:28 +0200, Jacopo Mondi wrote:
> Document 'data-active' property in R-Car VIN device tree bindings.
> The property is optional when running with explicit synchronization
> (eg. BT.601) but mandatory when embedded synchronization is in use (eg.
> BT.656) as specified by the hardware manual.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
>  Documentation/devicetree/bindings/media/rcar_vin.txt | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt
> index c53ce4e..17eac8a 100644
> --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
> +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
> @@ -63,6 +63,11 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
>  	If both HSYNC and VSYNC polarities are not specified, embedded
>  	synchronization is selected.
> 
> +        - data-active: active state of data enable signal (CLOCKENB pin).

I'm not sure what you mean by active state here. video-interfaces.txt 
defines data-active as 'similar to HSYNC and VSYNC, specifies data line 
polarity' so I assume this is the polarity of the CLOCKENB pin?

> +          0/1 for LOW/HIGH respectively. If not specified, use HSYNC as
> +          data enable signal. When using embedded synchronization this
> +          property is mandatory.

I'm confused, why is this mandatory if we have no embedded sync (that is 
hsync-active and vsync-active not defined)? I can't find any reference 
to this in the Gen2 datasheet but I'm sure I'm just missing it :-)

> +
>      - port 1 - sub-nodes describing one or more endpoints connected to
>        the VIN from local SoC CSI-2 receivers. The endpoint numbers must
>        use the following schema.
> --
> 2.7.4
> 

-- 
Regards,
Niklas S�derlund

[PATCH 2/6] dt-bindings: media: rcar-vin: Document data-active

[Niklas Söderlund]

Hi Jacopo,

Thanks for your work.

On 2018-05-16 18:32:28 +0200, Jacopo Mondi wrote:
> Document 'data-active' property in R-Car VIN device tree bindings.
> The property is optional when running with explicit synchronization
> (eg. BT.601) but mandatory when embedded synchronization is in use (eg.
> BT.656) as specified by the hardware manual.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
>  Documentation/devicetree/bindings/media/rcar_vin.txt | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt
> index c53ce4e..17eac8a 100644
> --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
> +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
> @@ -63,6 +63,11 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
>  	If both HSYNC and VSYNC polarities are not specified, embedded
>  	synchronization is selected.
> 
> +        - data-active: active state of data enable signal (CLOCKENB pin).

I'm not sure what you mean by active state here. video-interfaces.txt 
defines data-active as 'similar to HSYNC and VSYNC, specifies data line 
polarity' so I assume this is the polarity of the CLOCKENB pin?

> +          0/1 for LOW/HIGH respectively. If not specified, use HSYNC as
> +          data enable signal. When using embedded synchronization this
> +          property is mandatory.

I'm confused, why is this mandatory if we have no embedded sync (that is 
hsync-active and vsync-active not defined)? I can't find any reference 
to this in the Gen2 datasheet but I'm sure I'm just missing it :-)

> +
>      - port 1 - sub-nodes describing one or more endpoints connected to
>        the VIN from local SoC CSI-2 receivers. The endpoint numbers must
>        use the following schema.
> --
> 2.7.4
> 

-- 
Regards,
Niklas S?derlund

Re: [PATCH 3/6] media: rcar-vin: Handle data-active property

[Niklas Söderlund]

Hi Jacopo,

Thanks for your work.

On 2018-05-16 18:32:29 +0200, Jacopo Mondi wrote:
> The data-active property has to be specified when running with embedded
> synchronization. The VIN peripheral can use HSYNC in place of CLOCKENB
> when the CLOCKENB pin is not connected, this requires explicit
> synchronization to be in use.

Is this really the intent of the data-active property? I read the 
video-interfaces.txt document as such as if no hsync-active, 
vsync-active and data-active we should use the embedded synchronization 
else set the polarity for the requested pins. What am I not 
understanding here?

> 
> Now that the driver supports 'data-active' property, it makes not sense
> to zero the mbus configuration flags when running with implicit synch
> (V4L2_MBUS_BT656).
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
>  drivers/media/platform/rcar-vin/rcar-core.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
> index d3072e1..075d08f 100644
> --- a/drivers/media/platform/rcar-vin/rcar-core.c
> +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> @@ -531,15 +531,21 @@ static int rvin_digital_parse_v4l2(struct device *dev,
>  		return -ENOTCONN;
>  
>  	vin->mbus_cfg.type = vep->bus_type;
> +	vin->mbus_cfg.flags = vep->bus.parallel.flags;
>  
>  	switch (vin->mbus_cfg.type) {
>  	case V4L2_MBUS_PARALLEL:
>  		vin_dbg(vin, "Found PARALLEL media bus\n");
> -		vin->mbus_cfg.flags = vep->bus.parallel.flags;
>  		break;
>  	case V4L2_MBUS_BT656:
>  		vin_dbg(vin, "Found BT656 media bus\n");
> -		vin->mbus_cfg.flags = 0;
> +
> +		if (!(vin->mbus_cfg.flags & V4L2_MBUS_DATA_ACTIVE_HIGH) &&
> +		    !(vin->mbus_cfg.flags & V4L2_MBUS_DATA_ACTIVE_LOW)) {
> +			vin_err(vin,
> +				"Missing data enable signal polarity property\n");

I fear this can't be an error as that would break backward comp ability 
with existing dtb's.

> +			return -EINVAL;
> +		}
>  		break;
>  	default:
>  		vin_err(vin, "Unknown media bus type\n");
> -- 
> 2.7.4
> 

-- 
Regards,
Niklas Söderlund

Re: [PATCH 3/6] media: rcar-vin: Handle data-active property

[Niklas Söderlund]

Hi Jacopo,

Thanks for your work.

On 2018-05-16 18:32:29 +0200, Jacopo Mondi wrote:
> The data-active property has to be specified when running with embedded
> synchronization. The VIN peripheral can use HSYNC in place of CLOCKENB
> when the CLOCKENB pin is not connected, this requires explicit
> synchronization to be in use.

Is this really the intent of the data-active property? I read the 
video-interfaces.txt document as such as if no hsync-active, 
vsync-active and data-active we should use the embedded synchronization 
else set the polarity for the requested pins. What am I not 
understanding here?

> 
> Now that the driver supports 'data-active' property, it makes not sense
> to zero the mbus configuration flags when running with implicit synch
> (V4L2_MBUS_BT656).
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
>  drivers/media/platform/rcar-vin/rcar-core.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
> index d3072e1..075d08f 100644
> --- a/drivers/media/platform/rcar-vin/rcar-core.c
> +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> @@ -531,15 +531,21 @@ static int rvin_digital_parse_v4l2(struct device *dev,
>  		return -ENOTCONN;
>  
>  	vin->mbus_cfg.type = vep->bus_type;
> +	vin->mbus_cfg.flags = vep->bus.parallel.flags;
>  
>  	switch (vin->mbus_cfg.type) {
>  	case V4L2_MBUS_PARALLEL:
>  		vin_dbg(vin, "Found PARALLEL media bus\n");
> -		vin->mbus_cfg.flags = vep->bus.parallel.flags;
>  		break;
>  	case V4L2_MBUS_BT656:
>  		vin_dbg(vin, "Found BT656 media bus\n");
> -		vin->mbus_cfg.flags = 0;
> +
> +		if (!(vin->mbus_cfg.flags & V4L2_MBUS_DATA_ACTIVE_HIGH) &&
> +		    !(vin->mbus_cfg.flags & V4L2_MBUS_DATA_ACTIVE_LOW)) {
> +			vin_err(vin,
> +				"Missing data enable signal polarity property\n");

I fear this can't be an error as that would break backward comp ability 
with existing dtb's.

> +			return -EINVAL;
> +		}
>  		break;
>  	default:
>  		vin_err(vin, "Unknown media bus type\n");
> -- 
> 2.7.4
> 

-- 
Regards,
Niklas S�derlund

Re: [PATCH 4/6] media: rcar-vin: Handle CLOCKENB pin polarity

[Niklas Söderlund]

Hi Jacopo,

Thanks for your patch.

I'm happy that you dig into this as it clearly needs doing!

On 2018-05-16 18:32:30 +0200, Jacopo Mondi wrote:
> Handle CLOCKENB pin polarity, or use HSYNC in its place if polarity is
> not specified.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
>  drivers/media/platform/rcar-vin/rcar-dma.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c
> index ac07f99..7a84eae 100644
> --- a/drivers/media/platform/rcar-vin/rcar-dma.c
> +++ b/drivers/media/platform/rcar-vin/rcar-dma.c
> @@ -123,6 +123,8 @@
>  /* Video n Data Mode Register 2 bits */
>  #define VNDMR2_VPS		(1 << 30)
>  #define VNDMR2_HPS		(1 << 29)
> +#define VNDMR2_CES		(1 << 28)
> +#define VNDMR2_CHS		(1 << 23)
>  #define VNDMR2_FTEV		(1 << 17)
>  #define VNDMR2_VLV(n)		((n & 0xf) << 12)
>  
> @@ -691,6 +693,15 @@ static int rvin_setup(struct rvin_dev *vin)
>  		dmr2 |= VNDMR2_VPS;
>  
>  	/*
> +	 * Clock-enable active level select.
> +	 * Use HSYNC as enable if not specified
> +	 */
> +	if (vin->mbus_cfg.flags & V4L2_MBUS_DATA_ACTIVE_LOW)
> +		dmr2 |= VNDMR2_CES;
> +	else if (!(vin->mbus_cfg.flags & V4L2_MBUS_DATA_ACTIVE_HIGH))
> +		dmr2 |= VNDMR2_CHS;

After studying the datasheet for a while I'm getting more and more 
convinced this should be (with context leftout in this patch context) 
something like this:

	/* Hsync Signal Polarity Select */
	if (!(vin->mbus_cfg.flags & V4L2_MBUS_HSYNC_ACTIVE_LOW))
	        dmr2 |= VNDMR2_HPS;

	/* Vsync Signal Polarity Select */
	if (!(vin->mbus_cfg.flags & V4L2_MBUS_VSYNC_ACTIVE_LOW))
	        dmr2 |= VNDMR2_VPS;

	/* Clock Enable Signal Polarity Select */
	if (!(vin->mbus_cfg.flags & V4L2_MBUS_DATA_ACTIVE_LOW))
	        dmr2 |= VNDMR2_CES;

	/* Use HSYNC as clock enable if VIn_CLKENB is not available. */
	if (!(vin->mbus_cfg.flags & (V4L2_MBUS_DATA_ACTIVE_LOW | V4L2_MBUS_DATA_ACTIVE_HIGH)))
		dmr2 |= VNDMR2_CHS;

Or am I misunderstanding something?

> +
> +	/*
>  	 * Output format
>  	 */
>  	switch (vin->format.pixelformat) {
> -- 
> 2.7.4
> 

-- 
Regards,
Niklas Söderlund

Re: [PATCH 4/6] media: rcar-vin: Handle CLOCKENB pin polarity

[Niklas Söderlund]

Hi Jacopo,

Thanks for your patch.

I'm happy that you dig into this as it clearly needs doing!

On 2018-05-16 18:32:30 +0200, Jacopo Mondi wrote:
> Handle CLOCKENB pin polarity, or use HSYNC in its place if polarity is
> not specified.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
>  drivers/media/platform/rcar-vin/rcar-dma.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c
> index ac07f99..7a84eae 100644
> --- a/drivers/media/platform/rcar-vin/rcar-dma.c
> +++ b/drivers/media/platform/rcar-vin/rcar-dma.c
> @@ -123,6 +123,8 @@
>  /* Video n Data Mode Register 2 bits */
>  #define VNDMR2_VPS		(1 << 30)
>  #define VNDMR2_HPS		(1 << 29)
> +#define VNDMR2_CES		(1 << 28)
> +#define VNDMR2_CHS		(1 << 23)
>  #define VNDMR2_FTEV		(1 << 17)
>  #define VNDMR2_VLV(n)		((n & 0xf) << 12)
>  
> @@ -691,6 +693,15 @@ static int rvin_setup(struct rvin_dev *vin)
>  		dmr2 |= VNDMR2_VPS;
>  
>  	/*
> +	 * Clock-enable active level select.
> +	 * Use HSYNC as enable if not specified
> +	 */
> +	if (vin->mbus_cfg.flags & V4L2_MBUS_DATA_ACTIVE_LOW)
> +		dmr2 |= VNDMR2_CES;
> +	else if (!(vin->mbus_cfg.flags & V4L2_MBUS_DATA_ACTIVE_HIGH))
> +		dmr2 |= VNDMR2_CHS;

After studying the datasheet for a while I'm getting more and more 
convinced this should be (with context leftout in this patch context) 
something like this:

	/* Hsync Signal Polarity Select */
	if (!(vin->mbus_cfg.flags & V4L2_MBUS_HSYNC_ACTIVE_LOW))
	        dmr2 |= VNDMR2_HPS;

	/* Vsync Signal Polarity Select */
	if (!(vin->mbus_cfg.flags & V4L2_MBUS_VSYNC_ACTIVE_LOW))
	        dmr2 |= VNDMR2_VPS;

	/* Clock Enable Signal Polarity Select */
	if (!(vin->mbus_cfg.flags & V4L2_MBUS_DATA_ACTIVE_LOW))
	        dmr2 |= VNDMR2_CES;

	/* Use HSYNC as clock enable if VIn_CLKENB is not available. */
	if (!(vin->mbus_cfg.flags & (V4L2_MBUS_DATA_ACTIVE_LOW | V4L2_MBUS_DATA_ACTIVE_HIGH)))
		dmr2 |= VNDMR2_CHS;

Or am I misunderstanding something?

> +
> +	/*
>  	 * Output format
>  	 */
>  	switch (vin->format.pixelformat) {
> -- 
> 2.7.4
> 

-- 
Regards,
Niklas S�derlund

Re: [PATCH 5/6] ARM: dts: rcar-gen2: Remove unused VIN properties

[Niklas Söderlund]

Hi Jacopo,

Thanks for your work.

On 2018-05-16 18:32:31 +0200, Jacopo Mondi wrote:
> The 'bus-width' and 'pclk-sample' properties are not parsed by the VIN
> driver and only confuse users. Remove them in all Gen2 SoC that used
> them.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
>  arch/arm/boot/dts/r8a7790-lager.dts   | 3 ---
>  arch/arm/boot/dts/r8a7791-koelsch.dts | 3 ---
>  arch/arm/boot/dts/r8a7791-porter.dts  | 1 -
>  arch/arm/boot/dts/r8a7793-gose.dts    | 3 ---
>  arch/arm/boot/dts/r8a7794-alt.dts     | 1 -
>  arch/arm/boot/dts/r8a7794-silk.dts    | 1 -
>  6 files changed, 12 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/r8a7790-lager.dts b/arch/arm/boot/dts/r8a7790-lager.dts
> index 063fdb6..b56b309 100644
> --- a/arch/arm/boot/dts/r8a7790-lager.dts
> +++ b/arch/arm/boot/dts/r8a7790-lager.dts
> @@ -873,10 +873,8 @@
>  	port {
>  		vin0ep2: endpoint {
>  			remote-endpoint = <&adv7612_out>;
> -			bus-width = <24>;

I can't really make up my mind if this is a good thing or not. Device 
tree describes the hardware and not what the drivers make use of. And 
the fact is that this bus is 24 bits wide. So I'm not sure we should 
remove these properties. But I would love to hear what others think 
about this.

>  			hsync-active = <0>;
>  			vsync-active = <0>;
> -			pclk-sample = <1>;
>  			data-active = <1>;
>  		};
>  	};
> @@ -895,7 +893,6 @@
> 
>  		vin1ep0: endpoint {
>  			remote-endpoint = <&adv7180>;
> -			bus-width = <8>;
>  		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/r8a7791-koelsch.dts b/arch/arm/boot/dts/r8a7791-koelsch.dts
> index f40321a..9967666 100644
> --- a/arch/arm/boot/dts/r8a7791-koelsch.dts
> +++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
> @@ -849,10 +849,8 @@
> 
>  		vin0ep2: endpoint {
>  			remote-endpoint = <&adv7612_out>;
> -			bus-width = <24>;
>  			hsync-active = <0>;
>  			vsync-active = <0>;
> -			pclk-sample = <1>;
>  			data-active = <1>;
>  		};
>  	};
> @@ -870,7 +868,6 @@
> 
>  		vin1ep: endpoint {
>  			remote-endpoint = <&adv7180>;
> -			bus-width = <8>;
>  		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/r8a7791-porter.dts b/arch/arm/boot/dts/r8a7791-porter.dts
> index c14e6fe..055a7f1 100644
> --- a/arch/arm/boot/dts/r8a7791-porter.dts
> +++ b/arch/arm/boot/dts/r8a7791-porter.dts
> @@ -391,7 +391,6 @@
> 
>  		vin0ep: endpoint {
>  			remote-endpoint = <&adv7180>;
> -			bus-width = <8>;
>  		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/r8a7793-gose.dts b/arch/arm/boot/dts/r8a7793-gose.dts
> index 9ed6961..9d3fba2 100644
> --- a/arch/arm/boot/dts/r8a7793-gose.dts
> +++ b/arch/arm/boot/dts/r8a7793-gose.dts
> @@ -759,10 +759,8 @@
> 
>  		vin0ep2: endpoint {
>  			remote-endpoint = <&adv7612_out>;
> -			bus-width = <24>;
>  			hsync-active = <0>;
>  			vsync-active = <0>;
> -			pclk-sample = <1>;
>  			data-active = <1>;
>  		};
>  	};
> @@ -781,7 +779,6 @@
> 
>  		vin1ep: endpoint {
>  			remote-endpoint = <&adv7180_out>;
> -			bus-width = <8>;
>  		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/r8a7794-alt.dts b/arch/arm/boot/dts/r8a7794-alt.dts
> index 26a8834..4bbb9cc 100644
> --- a/arch/arm/boot/dts/r8a7794-alt.dts
> +++ b/arch/arm/boot/dts/r8a7794-alt.dts
> @@ -380,7 +380,6 @@
> 
>  		vin0ep: endpoint {
>  			remote-endpoint = <&adv7180>;
> -			bus-width = <8>;
>  		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/r8a7794-silk.dts b/arch/arm/boot/dts/r8a7794-silk.dts
> index 351cb3b..c0c5d31 100644
> --- a/arch/arm/boot/dts/r8a7794-silk.dts
> +++ b/arch/arm/boot/dts/r8a7794-silk.dts
> @@ -480,7 +480,6 @@
> 
>  		vin0ep: endpoint {
>  			remote-endpoint = <&adv7180>;
> -			bus-width = <8>;
>  		};
>  	};
>  };
> --
> 2.7.4
> 

-- 
Regards,
Niklas Söderlund

Re: [PATCH 5/6] ARM: dts: rcar-gen2: Remove unused VIN properties

[Niklas Söderlund]

Hi Jacopo,

Thanks for your work.

On 2018-05-16 18:32:31 +0200, Jacopo Mondi wrote:
> The 'bus-width' and 'pclk-sample' properties are not parsed by the VIN
> driver and only confuse users. Remove them in all Gen2 SoC that used
> them.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
>  arch/arm/boot/dts/r8a7790-lager.dts   | 3 ---
>  arch/arm/boot/dts/r8a7791-koelsch.dts | 3 ---
>  arch/arm/boot/dts/r8a7791-porter.dts  | 1 -
>  arch/arm/boot/dts/r8a7793-gose.dts    | 3 ---
>  arch/arm/boot/dts/r8a7794-alt.dts     | 1 -
>  arch/arm/boot/dts/r8a7794-silk.dts    | 1 -
>  6 files changed, 12 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/r8a7790-lager.dts b/arch/arm/boot/dts/r8a7790-lager.dts
> index 063fdb6..b56b309 100644
> --- a/arch/arm/boot/dts/r8a7790-lager.dts
> +++ b/arch/arm/boot/dts/r8a7790-lager.dts
> @@ -873,10 +873,8 @@
>  	port {
>  		vin0ep2: endpoint {
>  			remote-endpoint = <&adv7612_out>;
> -			bus-width = <24>;

I can't really make up my mind if this is a good thing or not. Device 
tree describes the hardware and not what the drivers make use of. And 
the fact is that this bus is 24 bits wide. So I'm not sure we should 
remove these properties. But I would love to hear what others think 
about this.

>  			hsync-active = <0>;
>  			vsync-active = <0>;
> -			pclk-sample = <1>;
>  			data-active = <1>;
>  		};
>  	};
> @@ -895,7 +893,6 @@
> 
>  		vin1ep0: endpoint {
>  			remote-endpoint = <&adv7180>;
> -			bus-width = <8>;
>  		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/r8a7791-koelsch.dts b/arch/arm/boot/dts/r8a7791-koelsch.dts
> index f40321a..9967666 100644
> --- a/arch/arm/boot/dts/r8a7791-koelsch.dts
> +++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
> @@ -849,10 +849,8 @@
> 
>  		vin0ep2: endpoint {
>  			remote-endpoint = <&adv7612_out>;
> -			bus-width = <24>;
>  			hsync-active = <0>;
>  			vsync-active = <0>;
> -			pclk-sample = <1>;
>  			data-active = <1>;
>  		};
>  	};
> @@ -870,7 +868,6 @@
> 
>  		vin1ep: endpoint {
>  			remote-endpoint = <&adv7180>;
> -			bus-width = <8>;
>  		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/r8a7791-porter.dts b/arch/arm/boot/dts/r8a7791-porter.dts
> index c14e6fe..055a7f1 100644
> --- a/arch/arm/boot/dts/r8a7791-porter.dts
> +++ b/arch/arm/boot/dts/r8a7791-porter.dts
> @@ -391,7 +391,6 @@
> 
>  		vin0ep: endpoint {
>  			remote-endpoint = <&adv7180>;
> -			bus-width = <8>;
>  		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/r8a7793-gose.dts b/arch/arm/boot/dts/r8a7793-gose.dts
> index 9ed6961..9d3fba2 100644
> --- a/arch/arm/boot/dts/r8a7793-gose.dts
> +++ b/arch/arm/boot/dts/r8a7793-gose.dts
> @@ -759,10 +759,8 @@
> 
>  		vin0ep2: endpoint {
>  			remote-endpoint = <&adv7612_out>;
> -			bus-width = <24>;
>  			hsync-active = <0>;
>  			vsync-active = <0>;
> -			pclk-sample = <1>;
>  			data-active = <1>;
>  		};
>  	};
> @@ -781,7 +779,6 @@
> 
>  		vin1ep: endpoint {
>  			remote-endpoint = <&adv7180_out>;
> -			bus-width = <8>;
>  		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/r8a7794-alt.dts b/arch/arm/boot/dts/r8a7794-alt.dts
> index 26a8834..4bbb9cc 100644
> --- a/arch/arm/boot/dts/r8a7794-alt.dts
> +++ b/arch/arm/boot/dts/r8a7794-alt.dts
> @@ -380,7 +380,6 @@
> 
>  		vin0ep: endpoint {
>  			remote-endpoint = <&adv7180>;
> -			bus-width = <8>;
>  		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/r8a7794-silk.dts b/arch/arm/boot/dts/r8a7794-silk.dts
> index 351cb3b..c0c5d31 100644
> --- a/arch/arm/boot/dts/r8a7794-silk.dts
> +++ b/arch/arm/boot/dts/r8a7794-silk.dts
> @@ -480,7 +480,6 @@
> 
>  		vin0ep: endpoint {
>  			remote-endpoint = <&adv7180>;
> -			bus-width = <8>;
>  		};
>  	};
>  };
> --
> 2.7.4
> 

-- 
Regards,
Niklas Söderlund

Re: [PATCH 5/6] ARM: dts: rcar-gen2: Remove unused VIN properties

[Niklas Söderlund]

Hi Jacopo,

Thanks for your work.

On 2018-05-16 18:32:31 +0200, Jacopo Mondi wrote:
> The 'bus-width' and 'pclk-sample' properties are not parsed by the VIN
> driver and only confuse users. Remove them in all Gen2 SoC that used
> them.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
>  arch/arm/boot/dts/r8a7790-lager.dts   | 3 ---
>  arch/arm/boot/dts/r8a7791-koelsch.dts | 3 ---
>  arch/arm/boot/dts/r8a7791-porter.dts  | 1 -
>  arch/arm/boot/dts/r8a7793-gose.dts    | 3 ---
>  arch/arm/boot/dts/r8a7794-alt.dts     | 1 -
>  arch/arm/boot/dts/r8a7794-silk.dts    | 1 -
>  6 files changed, 12 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/r8a7790-lager.dts b/arch/arm/boot/dts/r8a7790-lager.dts
> index 063fdb6..b56b309 100644
> --- a/arch/arm/boot/dts/r8a7790-lager.dts
> +++ b/arch/arm/boot/dts/r8a7790-lager.dts
> @@ -873,10 +873,8 @@
>  	port {
>  		vin0ep2: endpoint {
>  			remote-endpoint = <&adv7612_out>;
> -			bus-width = <24>;

I can't really make up my mind if this is a good thing or not. Device 
tree describes the hardware and not what the drivers make use of. And 
the fact is that this bus is 24 bits wide. So I'm not sure we should 
remove these properties. But I would love to hear what others think 
about this.

>  			hsync-active = <0>;
>  			vsync-active = <0>;
> -			pclk-sample = <1>;
>  			data-active = <1>;
>  		};
>  	};
> @@ -895,7 +893,6 @@
> 
>  		vin1ep0: endpoint {
>  			remote-endpoint = <&adv7180>;
> -			bus-width = <8>;
>  		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/r8a7791-koelsch.dts b/arch/arm/boot/dts/r8a7791-koelsch.dts
> index f40321a..9967666 100644
> --- a/arch/arm/boot/dts/r8a7791-koelsch.dts
> +++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
> @@ -849,10 +849,8 @@
> 
>  		vin0ep2: endpoint {
>  			remote-endpoint = <&adv7612_out>;
> -			bus-width = <24>;
>  			hsync-active = <0>;
>  			vsync-active = <0>;
> -			pclk-sample = <1>;
>  			data-active = <1>;
>  		};
>  	};
> @@ -870,7 +868,6 @@
> 
>  		vin1ep: endpoint {
>  			remote-endpoint = <&adv7180>;
> -			bus-width = <8>;
>  		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/r8a7791-porter.dts b/arch/arm/boot/dts/r8a7791-porter.dts
> index c14e6fe..055a7f1 100644
> --- a/arch/arm/boot/dts/r8a7791-porter.dts
> +++ b/arch/arm/boot/dts/r8a7791-porter.dts
> @@ -391,7 +391,6 @@
> 
>  		vin0ep: endpoint {
>  			remote-endpoint = <&adv7180>;
> -			bus-width = <8>;
>  		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/r8a7793-gose.dts b/arch/arm/boot/dts/r8a7793-gose.dts
> index 9ed6961..9d3fba2 100644
> --- a/arch/arm/boot/dts/r8a7793-gose.dts
> +++ b/arch/arm/boot/dts/r8a7793-gose.dts
> @@ -759,10 +759,8 @@
> 
>  		vin0ep2: endpoint {
>  			remote-endpoint = <&adv7612_out>;
> -			bus-width = <24>;
>  			hsync-active = <0>;
>  			vsync-active = <0>;
> -			pclk-sample = <1>;
>  			data-active = <1>;
>  		};
>  	};
> @@ -781,7 +779,6 @@
> 
>  		vin1ep: endpoint {
>  			remote-endpoint = <&adv7180_out>;
> -			bus-width = <8>;
>  		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/r8a7794-alt.dts b/arch/arm/boot/dts/r8a7794-alt.dts
> index 26a8834..4bbb9cc 100644
> --- a/arch/arm/boot/dts/r8a7794-alt.dts
> +++ b/arch/arm/boot/dts/r8a7794-alt.dts
> @@ -380,7 +380,6 @@
> 
>  		vin0ep: endpoint {
>  			remote-endpoint = <&adv7180>;
> -			bus-width = <8>;
>  		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/r8a7794-silk.dts b/arch/arm/boot/dts/r8a7794-silk.dts
> index 351cb3b..c0c5d31 100644
> --- a/arch/arm/boot/dts/r8a7794-silk.dts
> +++ b/arch/arm/boot/dts/r8a7794-silk.dts
> @@ -480,7 +480,6 @@
> 
>  		vin0ep: endpoint {
>  			remote-endpoint = <&adv7180>;
> -			bus-width = <8>;
>  		};
>  	};
>  };
> --
> 2.7.4
> 

-- 
Regards,
Niklas S�derlund

[PATCH 5/6] ARM: dts: rcar-gen2: Remove unused VIN properties

[Niklas Söderlund]

Hi Jacopo,

Thanks for your work.

On 2018-05-16 18:32:31 +0200, Jacopo Mondi wrote:
> The 'bus-width' and 'pclk-sample' properties are not parsed by the VIN
> driver and only confuse users. Remove them in all Gen2 SoC that used
> them.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
>  arch/arm/boot/dts/r8a7790-lager.dts   | 3 ---
>  arch/arm/boot/dts/r8a7791-koelsch.dts | 3 ---
>  arch/arm/boot/dts/r8a7791-porter.dts  | 1 -
>  arch/arm/boot/dts/r8a7793-gose.dts    | 3 ---
>  arch/arm/boot/dts/r8a7794-alt.dts     | 1 -
>  arch/arm/boot/dts/r8a7794-silk.dts    | 1 -
>  6 files changed, 12 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/r8a7790-lager.dts b/arch/arm/boot/dts/r8a7790-lager.dts
> index 063fdb6..b56b309 100644
> --- a/arch/arm/boot/dts/r8a7790-lager.dts
> +++ b/arch/arm/boot/dts/r8a7790-lager.dts
> @@ -873,10 +873,8 @@
>  	port {
>  		vin0ep2: endpoint {
>  			remote-endpoint = <&adv7612_out>;
> -			bus-width = <24>;

I can't really make up my mind if this is a good thing or not. Device 
tree describes the hardware and not what the drivers make use of. And 
the fact is that this bus is 24 bits wide. So I'm not sure we should 
remove these properties. But I would love to hear what others think 
about this.

>  			hsync-active = <0>;
>  			vsync-active = <0>;
> -			pclk-sample = <1>;
>  			data-active = <1>;
>  		};
>  	};
> @@ -895,7 +893,6 @@
> 
>  		vin1ep0: endpoint {
>  			remote-endpoint = <&adv7180>;
> -			bus-width = <8>;
>  		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/r8a7791-koelsch.dts b/arch/arm/boot/dts/r8a7791-koelsch.dts
> index f40321a..9967666 100644
> --- a/arch/arm/boot/dts/r8a7791-koelsch.dts
> +++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
> @@ -849,10 +849,8 @@
> 
>  		vin0ep2: endpoint {
>  			remote-endpoint = <&adv7612_out>;
> -			bus-width = <24>;
>  			hsync-active = <0>;
>  			vsync-active = <0>;
> -			pclk-sample = <1>;
>  			data-active = <1>;
>  		};
>  	};
> @@ -870,7 +868,6 @@
> 
>  		vin1ep: endpoint {
>  			remote-endpoint = <&adv7180>;
> -			bus-width = <8>;
>  		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/r8a7791-porter.dts b/arch/arm/boot/dts/r8a7791-porter.dts
> index c14e6fe..055a7f1 100644
> --- a/arch/arm/boot/dts/r8a7791-porter.dts
> +++ b/arch/arm/boot/dts/r8a7791-porter.dts
> @@ -391,7 +391,6 @@
> 
>  		vin0ep: endpoint {
>  			remote-endpoint = <&adv7180>;
> -			bus-width = <8>;
>  		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/r8a7793-gose.dts b/arch/arm/boot/dts/r8a7793-gose.dts
> index 9ed6961..9d3fba2 100644
> --- a/arch/arm/boot/dts/r8a7793-gose.dts
> +++ b/arch/arm/boot/dts/r8a7793-gose.dts
> @@ -759,10 +759,8 @@
> 
>  		vin0ep2: endpoint {
>  			remote-endpoint = <&adv7612_out>;
> -			bus-width = <24>;
>  			hsync-active = <0>;
>  			vsync-active = <0>;
> -			pclk-sample = <1>;
>  			data-active = <1>;
>  		};
>  	};
> @@ -781,7 +779,6 @@
> 
>  		vin1ep: endpoint {
>  			remote-endpoint = <&adv7180_out>;
> -			bus-width = <8>;
>  		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/r8a7794-alt.dts b/arch/arm/boot/dts/r8a7794-alt.dts
> index 26a8834..4bbb9cc 100644
> --- a/arch/arm/boot/dts/r8a7794-alt.dts
> +++ b/arch/arm/boot/dts/r8a7794-alt.dts
> @@ -380,7 +380,6 @@
> 
>  		vin0ep: endpoint {
>  			remote-endpoint = <&adv7180>;
> -			bus-width = <8>;
>  		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/r8a7794-silk.dts b/arch/arm/boot/dts/r8a7794-silk.dts
> index 351cb3b..c0c5d31 100644
> --- a/arch/arm/boot/dts/r8a7794-silk.dts
> +++ b/arch/arm/boot/dts/r8a7794-silk.dts
> @@ -480,7 +480,6 @@
> 
>  		vin0ep: endpoint {
>  			remote-endpoint = <&adv7180>;
> -			bus-width = <8>;
>  		};
>  	};
>  };
> --
> 2.7.4
> 

-- 
Regards,
Niklas S?derlund

Re: [PATCH 6/6] ARM: dts: rcar-gen2: Add 'data-active' property

[Niklas Söderlund]

Hi Jacopo,

Thanks for your work.

On 2018-05-16 18:32:32 +0200, Jacopo Mondi wrote:
> The 'data-active' property needs to be specified when using embedded
> synchronization. Add it to the Gen-2 boards using composite video input.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
>  arch/arm/boot/dts/r8a7790-lager.dts   | 1 +
>  arch/arm/boot/dts/r8a7791-koelsch.dts | 1 +
>  arch/arm/boot/dts/r8a7791-porter.dts  | 1 +
>  arch/arm/boot/dts/r8a7793-gose.dts    | 1 +
>  arch/arm/boot/dts/r8a7794-alt.dts     | 1 +
>  arch/arm/boot/dts/r8a7794-silk.dts    | 1 +
>  6 files changed, 6 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/r8a7790-lager.dts b/arch/arm/boot/dts/r8a7790-lager.dts
> index b56b309..48fcb44 100644
> --- a/arch/arm/boot/dts/r8a7790-lager.dts
> +++ b/arch/arm/boot/dts/r8a7790-lager.dts
> @@ -893,6 +893,7 @@
> 
>  		vin1ep0: endpoint {
>  			remote-endpoint = <&adv7180>;
> +			data-active = <1>;
>  		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/r8a7791-koelsch.dts b/arch/arm/boot/dts/r8a7791-koelsch.dts
> index 9967666..fa0b25f 100644
> --- a/arch/arm/boot/dts/r8a7791-koelsch.dts
> +++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
> @@ -868,6 +868,7 @@
> 
>  		vin1ep: endpoint {
>  			remote-endpoint = <&adv7180>;
> +			data-active = <1>;

Depending on how we interpret the data-active property this can be good 
or bad. But if we interpret it as the polarity of the VIn_CLKENB pin 
this is not good as this is not connected for the adv7180 on Koelsch.

I have not checked all the Gen2 schematics as I'm still not sure how to 
interpret the property.


>  		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/r8a7791-porter.dts b/arch/arm/boot/dts/r8a7791-porter.dts
> index 055a7f1..96b9605 100644
> --- a/arch/arm/boot/dts/r8a7791-porter.dts
> +++ b/arch/arm/boot/dts/r8a7791-porter.dts
> @@ -391,6 +391,7 @@
> 
>  		vin0ep: endpoint {
>  			remote-endpoint = <&adv7180>;
> +			data-active = <1>;
>  		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/r8a7793-gose.dts b/arch/arm/boot/dts/r8a7793-gose.dts
> index 9d3fba2..80b4fa8 100644
> --- a/arch/arm/boot/dts/r8a7793-gose.dts
> +++ b/arch/arm/boot/dts/r8a7793-gose.dts
> @@ -779,6 +779,7 @@
> 
>  		vin1ep: endpoint {
>  			remote-endpoint = <&adv7180_out>;
> +			data-active = <1>;
>  		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/r8a7794-alt.dts b/arch/arm/boot/dts/r8a7794-alt.dts
> index 4bbb9cc..00df605d 100644
> --- a/arch/arm/boot/dts/r8a7794-alt.dts
> +++ b/arch/arm/boot/dts/r8a7794-alt.dts
> @@ -380,6 +380,7 @@
> 
>  		vin0ep: endpoint {
>  			remote-endpoint = <&adv7180>;
> +			data-active = <1>;
>  		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/r8a7794-silk.dts b/arch/arm/boot/dts/r8a7794-silk.dts
> index c0c5d31..ed17376 100644
> --- a/arch/arm/boot/dts/r8a7794-silk.dts
> +++ b/arch/arm/boot/dts/r8a7794-silk.dts
> @@ -480,6 +480,7 @@
> 
>  		vin0ep: endpoint {
>  			remote-endpoint = <&adv7180>;
> +			data-active = <1>;
>  		};
>  	};
>  };
> --
> 2.7.4
> 

-- 
Regards,
Niklas Söderlund

Re: [PATCH 6/6] ARM: dts: rcar-gen2: Add 'data-active' property

[Niklas Söderlund]

Hi Jacopo,

Thanks for your work.

On 2018-05-16 18:32:32 +0200, Jacopo Mondi wrote:
> The 'data-active' property needs to be specified when using embedded
> synchronization. Add it to the Gen-2 boards using composite video input.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
>  arch/arm/boot/dts/r8a7790-lager.dts   | 1 +
>  arch/arm/boot/dts/r8a7791-koelsch.dts | 1 +
>  arch/arm/boot/dts/r8a7791-porter.dts  | 1 +
>  arch/arm/boot/dts/r8a7793-gose.dts    | 1 +
>  arch/arm/boot/dts/r8a7794-alt.dts     | 1 +
>  arch/arm/boot/dts/r8a7794-silk.dts    | 1 +
>  6 files changed, 6 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/r8a7790-lager.dts b/arch/arm/boot/dts/r8a7790-lager.dts
> index b56b309..48fcb44 100644
> --- a/arch/arm/boot/dts/r8a7790-lager.dts
> +++ b/arch/arm/boot/dts/r8a7790-lager.dts
> @@ -893,6 +893,7 @@
> 
>  		vin1ep0: endpoint {
>  			remote-endpoint = <&adv7180>;
> +			data-active = <1>;
>  		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/r8a7791-koelsch.dts b/arch/arm/boot/dts/r8a7791-koelsch.dts
> index 9967666..fa0b25f 100644
> --- a/arch/arm/boot/dts/r8a7791-koelsch.dts
> +++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
> @@ -868,6 +868,7 @@
> 
>  		vin1ep: endpoint {
>  			remote-endpoint = <&adv7180>;
> +			data-active = <1>;

Depending on how we interpret the data-active property this can be good 
or bad. But if we interpret it as the polarity of the VIn_CLKENB pin 
this is not good as this is not connected for the adv7180 on Koelsch.

I have not checked all the Gen2 schematics as I'm still not sure how to 
interpret the property.


>  		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/r8a7791-porter.dts b/arch/arm/boot/dts/r8a7791-porter.dts
> index 055a7f1..96b9605 100644
> --- a/arch/arm/boot/dts/r8a7791-porter.dts
> +++ b/arch/arm/boot/dts/r8a7791-porter.dts
> @@ -391,6 +391,7 @@
> 
>  		vin0ep: endpoint {
>  			remote-endpoint = <&adv7180>;
> +			data-active = <1>;
>  		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/r8a7793-gose.dts b/arch/arm/boot/dts/r8a7793-gose.dts
> index 9d3fba2..80b4fa8 100644
> --- a/arch/arm/boot/dts/r8a7793-gose.dts
> +++ b/arch/arm/boot/dts/r8a7793-gose.dts
> @@ -779,6 +779,7 @@
> 
>  		vin1ep: endpoint {
>  			remote-endpoint = <&adv7180_out>;
> +			data-active = <1>;
>  		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/r8a7794-alt.dts b/arch/arm/boot/dts/r8a7794-alt.dts
> index 4bbb9cc..00df605d 100644
> --- a/arch/arm/boot/dts/r8a7794-alt.dts
> +++ b/arch/arm/boot/dts/r8a7794-alt.dts
> @@ -380,6 +380,7 @@
> 
>  		vin0ep: endpoint {
>  			remote-endpoint = <&adv7180>;
> +			data-active = <1>;
>  		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/r8a7794-silk.dts b/arch/arm/boot/dts/r8a7794-silk.dts
> index c0c5d31..ed17376 100644
> --- a/arch/arm/boot/dts/r8a7794-silk.dts
> +++ b/arch/arm/boot/dts/r8a7794-silk.dts
> @@ -480,6 +480,7 @@
> 
>  		vin0ep: endpoint {
>  			remote-endpoint = <&adv7180>;
> +			data-active = <1>;
>  		};
>  	};
>  };
> --
> 2.7.4
> 

-- 
Regards,
Niklas Söderlund

Re: [PATCH 6/6] ARM: dts: rcar-gen2: Add 'data-active' property

[Niklas Söderlund]

Hi Jacopo,

Thanks for your work.

On 2018-05-16 18:32:32 +0200, Jacopo Mondi wrote:
> The 'data-active' property needs to be specified when using embedded
> synchronization. Add it to the Gen-2 boards using composite video input.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
>  arch/arm/boot/dts/r8a7790-lager.dts   | 1 +
>  arch/arm/boot/dts/r8a7791-koelsch.dts | 1 +
>  arch/arm/boot/dts/r8a7791-porter.dts  | 1 +
>  arch/arm/boot/dts/r8a7793-gose.dts    | 1 +
>  arch/arm/boot/dts/r8a7794-alt.dts     | 1 +
>  arch/arm/boot/dts/r8a7794-silk.dts    | 1 +
>  6 files changed, 6 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/r8a7790-lager.dts b/arch/arm/boot/dts/r8a7790-lager.dts
> index b56b309..48fcb44 100644
> --- a/arch/arm/boot/dts/r8a7790-lager.dts
> +++ b/arch/arm/boot/dts/r8a7790-lager.dts
> @@ -893,6 +893,7 @@
> 
>  		vin1ep0: endpoint {
>  			remote-endpoint = <&adv7180>;
> +			data-active = <1>;
>  		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/r8a7791-koelsch.dts b/arch/arm/boot/dts/r8a7791-koelsch.dts
> index 9967666..fa0b25f 100644
> --- a/arch/arm/boot/dts/r8a7791-koelsch.dts
> +++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
> @@ -868,6 +868,7 @@
> 
>  		vin1ep: endpoint {
>  			remote-endpoint = <&adv7180>;
> +			data-active = <1>;

Depending on how we interpret the data-active property this can be good 
or bad. But if we interpret it as the polarity of the VIn_CLKENB pin 
this is not good as this is not connected for the adv7180 on Koelsch.

I have not checked all the Gen2 schematics as I'm still not sure how to 
interpret the property.


>  		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/r8a7791-porter.dts b/arch/arm/boot/dts/r8a7791-porter.dts
> index 055a7f1..96b9605 100644
> --- a/arch/arm/boot/dts/r8a7791-porter.dts
> +++ b/arch/arm/boot/dts/r8a7791-porter.dts
> @@ -391,6 +391,7 @@
> 
>  		vin0ep: endpoint {
>  			remote-endpoint = <&adv7180>;
> +			data-active = <1>;
>  		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/r8a7793-gose.dts b/arch/arm/boot/dts/r8a7793-gose.dts
> index 9d3fba2..80b4fa8 100644
> --- a/arch/arm/boot/dts/r8a7793-gose.dts
> +++ b/arch/arm/boot/dts/r8a7793-gose.dts
> @@ -779,6 +779,7 @@
> 
>  		vin1ep: endpoint {
>  			remote-endpoint = <&adv7180_out>;
> +			data-active = <1>;
>  		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/r8a7794-alt.dts b/arch/arm/boot/dts/r8a7794-alt.dts
> index 4bbb9cc..00df605d 100644
> --- a/arch/arm/boot/dts/r8a7794-alt.dts
> +++ b/arch/arm/boot/dts/r8a7794-alt.dts
> @@ -380,6 +380,7 @@
> 
>  		vin0ep: endpoint {
>  			remote-endpoint = <&adv7180>;
> +			data-active = <1>;
>  		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/r8a7794-silk.dts b/arch/arm/boot/dts/r8a7794-silk.dts
> index c0c5d31..ed17376 100644
> --- a/arch/arm/boot/dts/r8a7794-silk.dts
> +++ b/arch/arm/boot/dts/r8a7794-silk.dts
> @@ -480,6 +480,7 @@
> 
>  		vin0ep: endpoint {
>  			remote-endpoint = <&adv7180>;
> +			data-active = <1>;
>  		};
>  	};
>  };
> --
> 2.7.4
> 

-- 
Regards,
Niklas S�derlund

[PATCH 6/6] ARM: dts: rcar-gen2: Add 'data-active' property

[Niklas Söderlund]

Hi Jacopo,

Thanks for your work.

On 2018-05-16 18:32:32 +0200, Jacopo Mondi wrote:
> The 'data-active' property needs to be specified when using embedded
> synchronization. Add it to the Gen-2 boards using composite video input.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
>  arch/arm/boot/dts/r8a7790-lager.dts   | 1 +
>  arch/arm/boot/dts/r8a7791-koelsch.dts | 1 +
>  arch/arm/boot/dts/r8a7791-porter.dts  | 1 +
>  arch/arm/boot/dts/r8a7793-gose.dts    | 1 +
>  arch/arm/boot/dts/r8a7794-alt.dts     | 1 +
>  arch/arm/boot/dts/r8a7794-silk.dts    | 1 +
>  6 files changed, 6 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/r8a7790-lager.dts b/arch/arm/boot/dts/r8a7790-lager.dts
> index b56b309..48fcb44 100644
> --- a/arch/arm/boot/dts/r8a7790-lager.dts
> +++ b/arch/arm/boot/dts/r8a7790-lager.dts
> @@ -893,6 +893,7 @@
> 
>  		vin1ep0: endpoint {
>  			remote-endpoint = <&adv7180>;
> +			data-active = <1>;
>  		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/r8a7791-koelsch.dts b/arch/arm/boot/dts/r8a7791-koelsch.dts
> index 9967666..fa0b25f 100644
> --- a/arch/arm/boot/dts/r8a7791-koelsch.dts
> +++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
> @@ -868,6 +868,7 @@
> 
>  		vin1ep: endpoint {
>  			remote-endpoint = <&adv7180>;
> +			data-active = <1>;

Depending on how we interpret the data-active property this can be good 
or bad. But if we interpret it as the polarity of the VIn_CLKENB pin 
this is not good as this is not connected for the adv7180 on Koelsch.

I have not checked all the Gen2 schematics as I'm still not sure how to 
interpret the property.


>  		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/r8a7791-porter.dts b/arch/arm/boot/dts/r8a7791-porter.dts
> index 055a7f1..96b9605 100644
> --- a/arch/arm/boot/dts/r8a7791-porter.dts
> +++ b/arch/arm/boot/dts/r8a7791-porter.dts
> @@ -391,6 +391,7 @@
> 
>  		vin0ep: endpoint {
>  			remote-endpoint = <&adv7180>;
> +			data-active = <1>;
>  		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/r8a7793-gose.dts b/arch/arm/boot/dts/r8a7793-gose.dts
> index 9d3fba2..80b4fa8 100644
> --- a/arch/arm/boot/dts/r8a7793-gose.dts
> +++ b/arch/arm/boot/dts/r8a7793-gose.dts
> @@ -779,6 +779,7 @@
> 
>  		vin1ep: endpoint {
>  			remote-endpoint = <&adv7180_out>;
> +			data-active = <1>;
>  		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/r8a7794-alt.dts b/arch/arm/boot/dts/r8a7794-alt.dts
> index 4bbb9cc..00df605d 100644
> --- a/arch/arm/boot/dts/r8a7794-alt.dts
> +++ b/arch/arm/boot/dts/r8a7794-alt.dts
> @@ -380,6 +380,7 @@
> 
>  		vin0ep: endpoint {
>  			remote-endpoint = <&adv7180>;
> +			data-active = <1>;
>  		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/r8a7794-silk.dts b/arch/arm/boot/dts/r8a7794-silk.dts
> index c0c5d31..ed17376 100644
> --- a/arch/arm/boot/dts/r8a7794-silk.dts
> +++ b/arch/arm/boot/dts/r8a7794-silk.dts
> @@ -480,6 +480,7 @@
> 
>  		vin0ep: endpoint {
>  			remote-endpoint = <&adv7180>;
> +			data-active = <1>;
>  		};
>  	};
>  };
> --
> 2.7.4
> 

-- 
Regards,
Niklas S?derlund

Re: [PATCH 2/6] dt-bindings: media: rcar-vin: Document data-active

[jacopo mondi]

[-- Attachment #1.1: Type: text/plain, Size: 3473 bytes --]

Hi Niklas,

On Wed, May 16, 2018 at 11:55:38PM +0200, Niklas Söderlund wrote:
> Hi Jacopo,
>
> Thanks for your work.
>
> On 2018-05-16 18:32:28 +0200, Jacopo Mondi wrote:
> > Document 'data-active' property in R-Car VIN device tree bindings.
> > The property is optional when running with explicit synchronization
> > (eg. BT.601) but mandatory when embedded synchronization is in use (eg.
> > BT.656) as specified by the hardware manual.
> >
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > ---
> >  Documentation/devicetree/bindings/media/rcar_vin.txt | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt
> > index c53ce4e..17eac8a 100644
> > --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
> > +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
> > @@ -63,6 +63,11 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
> >  	If both HSYNC and VSYNC polarities are not specified, embedded
> >  	synchronization is selected.
> >
> > +        - data-active: active state of data enable signal (CLOCKENB pin).
>
> I'm not sure what you mean by active state here. video-interfaces.txt
> defines data-active as 'similar to HSYNC and VSYNC, specifies data line
> polarity' so I assume this is the polarity of the CLOCKENB pin?

Yes, I can change this if it feels confusing to you.
>
> > +          0/1 for LOW/HIGH respectively. If not specified, use HSYNC as
> > +          data enable signal. When using embedded synchronization this
> > +          property is mandatory.
>
> I'm confused, why is this mandatory if we have no embedded sync (that is
> hsync-active and vsync-active not defined)? I can't find any reference
> to this in the Gen2 datasheet but I'm sure I'm just missing it :-)
>

Not exactly, it becomes mandatory IF we have embedded sync.
Here it is my reasoning:

In the documentation of CHS bit of Vn_DMR2 register [1] the following
is specified:

"When using ITU-R BT.601, BT.709, BT.1358 interface, and the
VIn_CLKENB pin is unused, the CHS bit must be set to 1."

And setting the CHS bit to 1:

"HSYNC signal (VIn_HSYNC#) input from the pin is internally used
as the clock enable signal"

So, if 'data-active' property is not specified I assume CLCKENB is not
used, and set the CHS bit. What if we are using BT656 and there is no
HSYNC? Then specifying 'data-active' becomes mandatory, as otherwise we
set the CHS bit and wait for HSYNC pin transitions that won't happen.

This is probably wrong, as in the Koelsch case, there is no guarantee
that CLKENB is connected, and what I should have done is probably set
the CHS bit only when running on V4L2_MBUS_PARALLEL, and leave CHS
(and CES, if 'data-active' is not specified) untouched, as we're doing
today when running on V4L2_MBUS_BT656. Does this work better in your
opinion?

This also makes patch [6/6] (where I was adding 'data-active' to Gen-2
boards) not required.

Thanks
   j


[1] 26.2.18 Video n Data Mode Register 2 (VnDMR2) Datasheet version,
R19UH0105EJ0100 Rev.1.00 Apr 30, 2018

> > +
> >      - port 1 - sub-nodes describing one or more endpoints connected to
> >        the VIN from local SoC CSI-2 receivers. The endpoint numbers must
> >        use the following schema.
> > --
> > 2.7.4
> >
>
> --
> Regards,
> Niklas Söderlund

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

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/6] dt-bindings: media: rcar-vin: Document data-active

[jacopo mondi]

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

Hi Niklas,

On Wed, May 16, 2018 at 11:55:38PM +0200, Niklas Söderlund wrote:
> Hi Jacopo,
>
> Thanks for your work.
>
> On 2018-05-16 18:32:28 +0200, Jacopo Mondi wrote:
> > Document 'data-active' property in R-Car VIN device tree bindings.
> > The property is optional when running with explicit synchronization
> > (eg. BT.601) but mandatory when embedded synchronization is in use (eg.
> > BT.656) as specified by the hardware manual.
> >
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > ---
> >  Documentation/devicetree/bindings/media/rcar_vin.txt | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt
> > index c53ce4e..17eac8a 100644
> > --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
> > +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
> > @@ -63,6 +63,11 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
> >  	If both HSYNC and VSYNC polarities are not specified, embedded
> >  	synchronization is selected.
> >
> > +        - data-active: active state of data enable signal (CLOCKENB pin).
>
> I'm not sure what you mean by active state here. video-interfaces.txt
> defines data-active as 'similar to HSYNC and VSYNC, specifies data line
> polarity' so I assume this is the polarity of the CLOCKENB pin?

Yes, I can change this if it feels confusing to you.
>
> > +          0/1 for LOW/HIGH respectively. If not specified, use HSYNC as
> > +          data enable signal. When using embedded synchronization this
> > +          property is mandatory.
>
> I'm confused, why is this mandatory if we have no embedded sync (that is
> hsync-active and vsync-active not defined)? I can't find any reference
> to this in the Gen2 datasheet but I'm sure I'm just missing it :-)
>

Not exactly, it becomes mandatory IF we have embedded sync.
Here it is my reasoning:

In the documentation of CHS bit of Vn_DMR2 register [1] the following
is specified:

"When using ITU-R BT.601, BT.709, BT.1358 interface, and the
VIn_CLKENB pin is unused, the CHS bit must be set to 1."

And setting the CHS bit to 1:

"HSYNC signal (VIn_HSYNC#) input from the pin is internally used
as the clock enable signal"

So, if 'data-active' property is not specified I assume CLCKENB is not
used, and set the CHS bit. What if we are using BT656 and there is no
HSYNC? Then specifying 'data-active' becomes mandatory, as otherwise we
set the CHS bit and wait for HSYNC pin transitions that won't happen.

This is probably wrong, as in the Koelsch case, there is no guarantee
that CLKENB is connected, and what I should have done is probably set
the CHS bit only when running on V4L2_MBUS_PARALLEL, and leave CHS
(and CES, if 'data-active' is not specified) untouched, as we're doing
today when running on V4L2_MBUS_BT656. Does this work better in your
opinion?

This also makes patch [6/6] (where I was adding 'data-active' to Gen-2
boards) not required.

Thanks
   j


[1] 26.2.18 Video n Data Mode Register 2 (VnDMR2) Datasheet version,
R19UH0105EJ0100 Rev.1.00 Apr 30, 2018

> > +
> >      - port 1 - sub-nodes describing one or more endpoints connected to
> >        the VIN from local SoC CSI-2 receivers. The endpoint numbers must
> >        use the following schema.
> > --
> > 2.7.4
> >
>
> --
> Regards,
> Niklas Söderlund

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

[PATCH 2/6] dt-bindings: media: rcar-vin: Document data-active

[jacopo mondi]

Hi Niklas,

On Wed, May 16, 2018 at 11:55:38PM +0200, Niklas S?derlund wrote:
> Hi Jacopo,
>
> Thanks for your work.
>
> On 2018-05-16 18:32:28 +0200, Jacopo Mondi wrote:
> > Document 'data-active' property in R-Car VIN device tree bindings.
> > The property is optional when running with explicit synchronization
> > (eg. BT.601) but mandatory when embedded synchronization is in use (eg.
> > BT.656) as specified by the hardware manual.
> >
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > ---
> >  Documentation/devicetree/bindings/media/rcar_vin.txt | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt
> > index c53ce4e..17eac8a 100644
> > --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
> > +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
> > @@ -63,6 +63,11 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
> >  	If both HSYNC and VSYNC polarities are not specified, embedded
> >  	synchronization is selected.
> >
> > +        - data-active: active state of data enable signal (CLOCKENB pin).
>
> I'm not sure what you mean by active state here. video-interfaces.txt
> defines data-active as 'similar to HSYNC and VSYNC, specifies data line
> polarity' so I assume this is the polarity of the CLOCKENB pin?

Yes, I can change this if it feels confusing to you.
>
> > +          0/1 for LOW/HIGH respectively. If not specified, use HSYNC as
> > +          data enable signal. When using embedded synchronization this
> > +          property is mandatory.
>
> I'm confused, why is this mandatory if we have no embedded sync (that is
> hsync-active and vsync-active not defined)? I can't find any reference
> to this in the Gen2 datasheet but I'm sure I'm just missing it :-)
>

Not exactly, it becomes mandatory IF we have embedded sync.
Here it is my reasoning:

In the documentation of CHS bit of Vn_DMR2 register [1] the following
is specified:

"When using ITU-R BT.601, BT.709, BT.1358 interface, and the
VIn_CLKENB pin is unused, the CHS bit must be set to 1."

And setting the CHS bit to 1:

"HSYNC signal (VIn_HSYNC#) input from the pin is internally used
as the clock enable signal"

So, if 'data-active' property is not specified I assume CLCKENB is not
used, and set the CHS bit. What if we are using BT656 and there is no
HSYNC? Then specifying 'data-active' becomes mandatory, as otherwise we
set the CHS bit and wait for HSYNC pin transitions that won't happen.

This is probably wrong, as in the Koelsch case, there is no guarantee
that CLKENB is connected, and what I should have done is probably set
the CHS bit only when running on V4L2_MBUS_PARALLEL, and leave CHS
(and CES, if 'data-active' is not specified) untouched, as we're doing
today when running on V4L2_MBUS_BT656. Does this work better in your
opinion?

This also makes patch [6/6] (where I was adding 'data-active' to Gen-2
boards) not required.

Thanks
   j


[1] 26.2.18 Video n Data Mode Register 2 (VnDMR2) Datasheet version,
R19UH0105EJ0100 Rev.1.00 Apr 30, 2018

> > +
> >      - port 1 - sub-nodes describing one or more endpoints connected to
> >        the VIN from local SoC CSI-2 receivers. The endpoint numbers must
> >        use the following schema.
> > --
> > 2.7.4
> >
>
> --
> Regards,
> Niklas S?derlund
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180517/6971752f/attachment.sig>

Re: [PATCH 3/6] media: rcar-vin: Handle data-active property

[jacopo mondi]

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

Hi Niklas,

On Wed, May 16, 2018 at 11:58:47PM +0200, Niklas Söderlund wrote:
> Hi Jacopo,
>
> Thanks for your work.
>
> On 2018-05-16 18:32:29 +0200, Jacopo Mondi wrote:
> > The data-active property has to be specified when running with embedded
> > synchronization. The VIN peripheral can use HSYNC in place of CLOCKENB
> > when the CLOCKENB pin is not connected, this requires explicit
> > synchronization to be in use.
>
> Is this really the intent of the data-active property? I read the
> video-interfaces.txt document as such as if no hsync-active,
> vsync-active and data-active we should use the embedded synchronization
> else set the polarity for the requested pins. What am I not
> understanding here?

Almost correct.

The presence of hsync-active, vsync-active and field-evev-active
properties determinate the bus type we're running on. If none of the
is specified, the bus is marked 'BT656' and we assume the system is
using embedded synchronization.

data-active does not take part in the bus identification, and my
reasoning was the other way around as explained in reply to your
comment on [2/6], and as explained there my reasoning is probably
wrong, and we should set CHS -only- when running with explicit
synchronization, instead of making it mandatory when running with
embedded syncs.

Thanks and sorry for my confusion.

    j
>
> >
> > Now that the driver supports 'data-active' property, it makes not sense
> > to zero the mbus configuration flags when running with implicit synch
> > (V4L2_MBUS_BT656).
> >
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > ---
> >  drivers/media/platform/rcar-vin/rcar-core.c | 10 ++++++++--
> >  1 file changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
> > index d3072e1..075d08f 100644
> > --- a/drivers/media/platform/rcar-vin/rcar-core.c
> > +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> > @@ -531,15 +531,21 @@ static int rvin_digital_parse_v4l2(struct device *dev,
> >  		return -ENOTCONN;
> >
> >  	vin->mbus_cfg.type = vep->bus_type;
> > +	vin->mbus_cfg.flags = vep->bus.parallel.flags;
> >
> >  	switch (vin->mbus_cfg.type) {
> >  	case V4L2_MBUS_PARALLEL:
> >  		vin_dbg(vin, "Found PARALLEL media bus\n");
> > -		vin->mbus_cfg.flags = vep->bus.parallel.flags;
> >  		break;
> >  	case V4L2_MBUS_BT656:
> >  		vin_dbg(vin, "Found BT656 media bus\n");
> > -		vin->mbus_cfg.flags = 0;
> > +
> > +		if (!(vin->mbus_cfg.flags & V4L2_MBUS_DATA_ACTIVE_HIGH) &&
> > +		    !(vin->mbus_cfg.flags & V4L2_MBUS_DATA_ACTIVE_LOW)) {
> > +			vin_err(vin,
> > +				"Missing data enable signal polarity property\n");
>
> I fear this can't be an error as that would break backward comp ability
> with existing dtb's.
>
> > +			return -EINVAL;
> > +		}
> >  		break;
> >  	default:
> >  		vin_err(vin, "Unknown media bus type\n");
> > --
> > 2.7.4
> >
>
> --
> Regards,
> Niklas Söderlund

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

Re: [PATCH 4/6] media: rcar-vin: Handle CLOCKENB pin polarity

[jacopo mondi]

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

Hi Niklas,

On Thu, May 17, 2018 at 12:11:03AM +0200, Niklas Söderlund wrote:
> Hi Jacopo,
>
> Thanks for your patch.
>
> I'm happy that you dig into this as it clearly needs doing!
>
> On 2018-05-16 18:32:30 +0200, Jacopo Mondi wrote:
> > Handle CLOCKENB pin polarity, or use HSYNC in its place if polarity is
> > not specified.
> >
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > ---
> >  drivers/media/platform/rcar-vin/rcar-dma.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> >
> > diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c
> > index ac07f99..7a84eae 100644
> > --- a/drivers/media/platform/rcar-vin/rcar-dma.c
> > +++ b/drivers/media/platform/rcar-vin/rcar-dma.c
> > @@ -123,6 +123,8 @@
> >  /* Video n Data Mode Register 2 bits */
> >  #define VNDMR2_VPS		(1 << 30)
> >  #define VNDMR2_HPS		(1 << 29)
> > +#define VNDMR2_CES		(1 << 28)
> > +#define VNDMR2_CHS		(1 << 23)
> >  #define VNDMR2_FTEV		(1 << 17)
> >  #define VNDMR2_VLV(n)		((n & 0xf) << 12)
> >
> > @@ -691,6 +693,15 @@ static int rvin_setup(struct rvin_dev *vin)
> >  		dmr2 |= VNDMR2_VPS;
> >
> >  	/*
> > +	 * Clock-enable active level select.
> > +	 * Use HSYNC as enable if not specified
> > +	 */
> > +	if (vin->mbus_cfg.flags & V4L2_MBUS_DATA_ACTIVE_LOW)
> > +		dmr2 |= VNDMR2_CES;
> > +	else if (!(vin->mbus_cfg.flags & V4L2_MBUS_DATA_ACTIVE_HIGH))
> > +		dmr2 |= VNDMR2_CHS;
>
> After studying the datasheet for a while I'm getting more and more
> convinced this should be (with context leftout in this patch context)
> something like this:
>
> 	/* Hsync Signal Polarity Select */
> 	if (!(vin->mbus_cfg.flags & V4L2_MBUS_HSYNC_ACTIVE_LOW))
> 	        dmr2 |= VNDMR2_HPS;
>
> 	/* Vsync Signal Polarity Select */
> 	if (!(vin->mbus_cfg.flags & V4L2_MBUS_VSYNC_ACTIVE_LOW))
> 	        dmr2 |= VNDMR2_VPS;
>
> 	/* Clock Enable Signal Polarity Select */
> 	if (!(vin->mbus_cfg.flags & V4L2_MBUS_DATA_ACTIVE_LOW))
> 	        dmr2 |= VNDMR2_CES;

No, set CES if V4L2_MBUS_DATA_ACTIVE_LOW is specified, not the other
way around. See the CES bit description:

        Clock Enable Signal Polarity Select
        This bit specifies the polarity of the input clock enable signal in the ITU-
        R BT.601.
        0: Active high
        1: Active low
>
> 	/* Use HSYNC as clock enable if VIn_CLKENB is not available. */
> 	if (!(vin->mbus_cfg.flags & (V4L2_MBUS_DATA_ACTIVE_LOW | V4L2_MBUS_DATA_ACTIVE_HIGH)))
> 		dmr2 |= VNDMR2_CHS;
>
> Or am I misunderstanding something?

Isn't that what my code is doing?

if (flags & LOW)
        dmr2 |= CES;
else if (!(flags & HIGH)) // if we get here, LOW is not set too
        dmr2 |= CHS;

Anyway, if you agree with my previous replies, we should set CHS only
when running with explicit syncs (V4L2_MBUS_PARALLEL).

Thanks
  j
>
> > +
> > +	/*
> >  	 * Output format
> >  	 */
> >  	switch (vin->format.pixelformat) {
> > --
> > 2.7.4
> >
>
> --
> Regards,
> Niklas Söderlund

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

Re: [PATCH 3/6] media: rcar-vin: Handle data-active property

[Sergei Shtylyov]

Hello!

On 5/16/2018 7:32 PM, Jacopo Mondi wrote:

> The data-active property has to be specified when running with embedded

    Prop names are typically enclosed in "".

> synchronization. The VIN peripheral can use HSYNC in place of CLOCKENB

    CLKENB, maybe?

> when the CLOCKENB pin is not connected, this requires explicit
> synchronization to be in use.
> 
> Now that the driver supports 'data-active' property, it makes not sense

    "data-active", s/not/no/.

> to zero the mbus configuration flags when running with implicit synch

    Sync is better. :-)

> (V4L2_MBUS_BT656).
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
[...]

MBR, Sergei

Re: [PATCH 5/6] ARM: dts: rcar-gen2: Remove unused VIN properties

[jacopo mondi]

[-- Attachment #1.1: Type: text/plain, Size: 4603 bytes --]

Hi Niklas,

On Thu, May 17, 2018 at 12:13:07AM +0200, Niklas Söderlund wrote:
> Hi Jacopo,
>
> Thanks for your work.
>
> On 2018-05-16 18:32:31 +0200, Jacopo Mondi wrote:
> > The 'bus-width' and 'pclk-sample' properties are not parsed by the VIN
> > driver and only confuse users. Remove them in all Gen2 SoC that used
> > them.
> >
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > ---
> >  arch/arm/boot/dts/r8a7790-lager.dts   | 3 ---
> >  arch/arm/boot/dts/r8a7791-koelsch.dts | 3 ---
> >  arch/arm/boot/dts/r8a7791-porter.dts  | 1 -
> >  arch/arm/boot/dts/r8a7793-gose.dts    | 3 ---
> >  arch/arm/boot/dts/r8a7794-alt.dts     | 1 -
> >  arch/arm/boot/dts/r8a7794-silk.dts    | 1 -
> >  6 files changed, 12 deletions(-)
> >
> > diff --git a/arch/arm/boot/dts/r8a7790-lager.dts b/arch/arm/boot/dts/r8a7790-lager.dts
> > index 063fdb6..b56b309 100644
> > --- a/arch/arm/boot/dts/r8a7790-lager.dts
> > +++ b/arch/arm/boot/dts/r8a7790-lager.dts
> > @@ -873,10 +873,8 @@
> >  	port {
> >  		vin0ep2: endpoint {
> >  			remote-endpoint = <&adv7612_out>;
> > -			bus-width = <24>;
>
> I can't really make up my mind if this is a good thing or not. Device
> tree describes the hardware and not what the drivers make use of. And
> the fact is that this bus is 24 bits wide. So I'm not sure we should
> remove these properties. But I would love to hear what others think
> about this.
>

Just to point out those properties are not even documented in rcar-vin
bindings (actually, none of them was).

I feel it's wrong to have them here, as someone may think that
changing their value should actually change the VIN interface behavior,
which it's not true, leading to massive confusion and quite some code
digging for no reason (and they will get mad at us at some point, probably :)

Thanks
   j

> >  			hsync-active = <0>;
> >  			vsync-active = <0>;
> > -			pclk-sample = <1>;
> >  			data-active = <1>;
> >  		};
> >  	};
> > @@ -895,7 +893,6 @@
> >
> >  		vin1ep0: endpoint {
> >  			remote-endpoint = <&adv7180>;
> > -			bus-width = <8>;
> >  		};
> >  	};
> >  };
> > diff --git a/arch/arm/boot/dts/r8a7791-koelsch.dts b/arch/arm/boot/dts/r8a7791-koelsch.dts
> > index f40321a..9967666 100644
> > --- a/arch/arm/boot/dts/r8a7791-koelsch.dts
> > +++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
> > @@ -849,10 +849,8 @@
> >
> >  		vin0ep2: endpoint {
> >  			remote-endpoint = <&adv7612_out>;
> > -			bus-width = <24>;
> >  			hsync-active = <0>;
> >  			vsync-active = <0>;
> > -			pclk-sample = <1>;
> >  			data-active = <1>;
> >  		};
> >  	};
> > @@ -870,7 +868,6 @@
> >
> >  		vin1ep: endpoint {
> >  			remote-endpoint = <&adv7180>;
> > -			bus-width = <8>;
> >  		};
> >  	};
> >  };
> > diff --git a/arch/arm/boot/dts/r8a7791-porter.dts b/arch/arm/boot/dts/r8a7791-porter.dts
> > index c14e6fe..055a7f1 100644
> > --- a/arch/arm/boot/dts/r8a7791-porter.dts
> > +++ b/arch/arm/boot/dts/r8a7791-porter.dts
> > @@ -391,7 +391,6 @@
> >
> >  		vin0ep: endpoint {
> >  			remote-endpoint = <&adv7180>;
> > -			bus-width = <8>;
> >  		};
> >  	};
> >  };
> > diff --git a/arch/arm/boot/dts/r8a7793-gose.dts b/arch/arm/boot/dts/r8a7793-gose.dts
> > index 9ed6961..9d3fba2 100644
> > --- a/arch/arm/boot/dts/r8a7793-gose.dts
> > +++ b/arch/arm/boot/dts/r8a7793-gose.dts
> > @@ -759,10 +759,8 @@
> >
> >  		vin0ep2: endpoint {
> >  			remote-endpoint = <&adv7612_out>;
> > -			bus-width = <24>;
> >  			hsync-active = <0>;
> >  			vsync-active = <0>;
> > -			pclk-sample = <1>;
> >  			data-active = <1>;
> >  		};
> >  	};
> > @@ -781,7 +779,6 @@
> >
> >  		vin1ep: endpoint {
> >  			remote-endpoint = <&adv7180_out>;
> > -			bus-width = <8>;
> >  		};
> >  	};
> >  };
> > diff --git a/arch/arm/boot/dts/r8a7794-alt.dts b/arch/arm/boot/dts/r8a7794-alt.dts
> > index 26a8834..4bbb9cc 100644
> > --- a/arch/arm/boot/dts/r8a7794-alt.dts
> > +++ b/arch/arm/boot/dts/r8a7794-alt.dts
> > @@ -380,7 +380,6 @@
> >
> >  		vin0ep: endpoint {
> >  			remote-endpoint = <&adv7180>;
> > -			bus-width = <8>;
> >  		};
> >  	};
> >  };
> > diff --git a/arch/arm/boot/dts/r8a7794-silk.dts b/arch/arm/boot/dts/r8a7794-silk.dts
> > index 351cb3b..c0c5d31 100644
> > --- a/arch/arm/boot/dts/r8a7794-silk.dts
> > +++ b/arch/arm/boot/dts/r8a7794-silk.dts
> > @@ -480,7 +480,6 @@
> >
> >  		vin0ep: endpoint {
> >  			remote-endpoint = <&adv7180>;
> > -			bus-width = <8>;
> >  		};
> >  	};
> >  };
> > --
> > 2.7.4
> >
>
> --
> Regards,
> Niklas Söderlund

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

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 5/6] ARM: dts: rcar-gen2: Remove unused VIN properties

[jacopo mondi]

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

Hi Niklas,

On Thu, May 17, 2018 at 12:13:07AM +0200, Niklas Söderlund wrote:
> Hi Jacopo,
>
> Thanks for your work.
>
> On 2018-05-16 18:32:31 +0200, Jacopo Mondi wrote:
> > The 'bus-width' and 'pclk-sample' properties are not parsed by the VIN
> > driver and only confuse users. Remove them in all Gen2 SoC that used
> > them.
> >
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > ---
> >  arch/arm/boot/dts/r8a7790-lager.dts   | 3 ---
> >  arch/arm/boot/dts/r8a7791-koelsch.dts | 3 ---
> >  arch/arm/boot/dts/r8a7791-porter.dts  | 1 -
> >  arch/arm/boot/dts/r8a7793-gose.dts    | 3 ---
> >  arch/arm/boot/dts/r8a7794-alt.dts     | 1 -
> >  arch/arm/boot/dts/r8a7794-silk.dts    | 1 -
> >  6 files changed, 12 deletions(-)
> >
> > diff --git a/arch/arm/boot/dts/r8a7790-lager.dts b/arch/arm/boot/dts/r8a7790-lager.dts
> > index 063fdb6..b56b309 100644
> > --- a/arch/arm/boot/dts/r8a7790-lager.dts
> > +++ b/arch/arm/boot/dts/r8a7790-lager.dts
> > @@ -873,10 +873,8 @@
> >  	port {
> >  		vin0ep2: endpoint {
> >  			remote-endpoint = <&adv7612_out>;
> > -			bus-width = <24>;
>
> I can't really make up my mind if this is a good thing or not. Device
> tree describes the hardware and not what the drivers make use of. And
> the fact is that this bus is 24 bits wide. So I'm not sure we should
> remove these properties. But I would love to hear what others think
> about this.
>

Just to point out those properties are not even documented in rcar-vin
bindings (actually, none of them was).

I feel it's wrong to have them here, as someone may think that
changing their value should actually change the VIN interface behavior,
which it's not true, leading to massive confusion and quite some code
digging for no reason (and they will get mad at us at some point, probably :)

Thanks
   j

> >  			hsync-active = <0>;
> >  			vsync-active = <0>;
> > -			pclk-sample = <1>;
> >  			data-active = <1>;
> >  		};
> >  	};
> > @@ -895,7 +893,6 @@
> >
> >  		vin1ep0: endpoint {
> >  			remote-endpoint = <&adv7180>;
> > -			bus-width = <8>;
> >  		};
> >  	};
> >  };
> > diff --git a/arch/arm/boot/dts/r8a7791-koelsch.dts b/arch/arm/boot/dts/r8a7791-koelsch.dts
> > index f40321a..9967666 100644
> > --- a/arch/arm/boot/dts/r8a7791-koelsch.dts
> > +++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
> > @@ -849,10 +849,8 @@
> >
> >  		vin0ep2: endpoint {
> >  			remote-endpoint = <&adv7612_out>;
> > -			bus-width = <24>;
> >  			hsync-active = <0>;
> >  			vsync-active = <0>;
> > -			pclk-sample = <1>;
> >  			data-active = <1>;
> >  		};
> >  	};
> > @@ -870,7 +868,6 @@
> >
> >  		vin1ep: endpoint {
> >  			remote-endpoint = <&adv7180>;
> > -			bus-width = <8>;
> >  		};
> >  	};
> >  };
> > diff --git a/arch/arm/boot/dts/r8a7791-porter.dts b/arch/arm/boot/dts/r8a7791-porter.dts
> > index c14e6fe..055a7f1 100644
> > --- a/arch/arm/boot/dts/r8a7791-porter.dts
> > +++ b/arch/arm/boot/dts/r8a7791-porter.dts
> > @@ -391,7 +391,6 @@
> >
> >  		vin0ep: endpoint {
> >  			remote-endpoint = <&adv7180>;
> > -			bus-width = <8>;
> >  		};
> >  	};
> >  };
> > diff --git a/arch/arm/boot/dts/r8a7793-gose.dts b/arch/arm/boot/dts/r8a7793-gose.dts
> > index 9ed6961..9d3fba2 100644
> > --- a/arch/arm/boot/dts/r8a7793-gose.dts
> > +++ b/arch/arm/boot/dts/r8a7793-gose.dts
> > @@ -759,10 +759,8 @@
> >
> >  		vin0ep2: endpoint {
> >  			remote-endpoint = <&adv7612_out>;
> > -			bus-width = <24>;
> >  			hsync-active = <0>;
> >  			vsync-active = <0>;
> > -			pclk-sample = <1>;
> >  			data-active = <1>;
> >  		};
> >  	};
> > @@ -781,7 +779,6 @@
> >
> >  		vin1ep: endpoint {
> >  			remote-endpoint = <&adv7180_out>;
> > -			bus-width = <8>;
> >  		};
> >  	};
> >  };
> > diff --git a/arch/arm/boot/dts/r8a7794-alt.dts b/arch/arm/boot/dts/r8a7794-alt.dts
> > index 26a8834..4bbb9cc 100644
> > --- a/arch/arm/boot/dts/r8a7794-alt.dts
> > +++ b/arch/arm/boot/dts/r8a7794-alt.dts
> > @@ -380,7 +380,6 @@
> >
> >  		vin0ep: endpoint {
> >  			remote-endpoint = <&adv7180>;
> > -			bus-width = <8>;
> >  		};
> >  	};
> >  };
> > diff --git a/arch/arm/boot/dts/r8a7794-silk.dts b/arch/arm/boot/dts/r8a7794-silk.dts
> > index 351cb3b..c0c5d31 100644
> > --- a/arch/arm/boot/dts/r8a7794-silk.dts
> > +++ b/arch/arm/boot/dts/r8a7794-silk.dts
> > @@ -480,7 +480,6 @@
> >
> >  		vin0ep: endpoint {
> >  			remote-endpoint = <&adv7180>;
> > -			bus-width = <8>;
> >  		};
> >  	};
> >  };
> > --
> > 2.7.4
> >
>
> --
> Regards,
> Niklas Söderlund

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

[PATCH 5/6] ARM: dts: rcar-gen2: Remove unused VIN properties

[jacopo mondi]

Hi Niklas,

On Thu, May 17, 2018 at 12:13:07AM +0200, Niklas S?derlund wrote:
> Hi Jacopo,
>
> Thanks for your work.
>
> On 2018-05-16 18:32:31 +0200, Jacopo Mondi wrote:
> > The 'bus-width' and 'pclk-sample' properties are not parsed by the VIN
> > driver and only confuse users. Remove them in all Gen2 SoC that used
> > them.
> >
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > ---
> >  arch/arm/boot/dts/r8a7790-lager.dts   | 3 ---
> >  arch/arm/boot/dts/r8a7791-koelsch.dts | 3 ---
> >  arch/arm/boot/dts/r8a7791-porter.dts  | 1 -
> >  arch/arm/boot/dts/r8a7793-gose.dts    | 3 ---
> >  arch/arm/boot/dts/r8a7794-alt.dts     | 1 -
> >  arch/arm/boot/dts/r8a7794-silk.dts    | 1 -
> >  6 files changed, 12 deletions(-)
> >
> > diff --git a/arch/arm/boot/dts/r8a7790-lager.dts b/arch/arm/boot/dts/r8a7790-lager.dts
> > index 063fdb6..b56b309 100644
> > --- a/arch/arm/boot/dts/r8a7790-lager.dts
> > +++ b/arch/arm/boot/dts/r8a7790-lager.dts
> > @@ -873,10 +873,8 @@
> >  	port {
> >  		vin0ep2: endpoint {
> >  			remote-endpoint = <&adv7612_out>;
> > -			bus-width = <24>;
>
> I can't really make up my mind if this is a good thing or not. Device
> tree describes the hardware and not what the drivers make use of. And
> the fact is that this bus is 24 bits wide. So I'm not sure we should
> remove these properties. But I would love to hear what others think
> about this.
>

Just to point out those properties are not even documented in rcar-vin
bindings (actually, none of them was).

I feel it's wrong to have them here, as someone may think that
changing their value should actually change the VIN interface behavior,
which it's not true, leading to massive confusion and quite some code
digging for no reason (and they will get mad at us at some point, probably :)

Thanks
   j

> >  			hsync-active = <0>;
> >  			vsync-active = <0>;
> > -			pclk-sample = <1>;
> >  			data-active = <1>;
> >  		};
> >  	};
> > @@ -895,7 +893,6 @@
> >
> >  		vin1ep0: endpoint {
> >  			remote-endpoint = <&adv7180>;
> > -			bus-width = <8>;
> >  		};
> >  	};
> >  };
> > diff --git a/arch/arm/boot/dts/r8a7791-koelsch.dts b/arch/arm/boot/dts/r8a7791-koelsch.dts
> > index f40321a..9967666 100644
> > --- a/arch/arm/boot/dts/r8a7791-koelsch.dts
> > +++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
> > @@ -849,10 +849,8 @@
> >
> >  		vin0ep2: endpoint {
> >  			remote-endpoint = <&adv7612_out>;
> > -			bus-width = <24>;
> >  			hsync-active = <0>;
> >  			vsync-active = <0>;
> > -			pclk-sample = <1>;
> >  			data-active = <1>;
> >  		};
> >  	};
> > @@ -870,7 +868,6 @@
> >
> >  		vin1ep: endpoint {
> >  			remote-endpoint = <&adv7180>;
> > -			bus-width = <8>;
> >  		};
> >  	};
> >  };
> > diff --git a/arch/arm/boot/dts/r8a7791-porter.dts b/arch/arm/boot/dts/r8a7791-porter.dts
> > index c14e6fe..055a7f1 100644
> > --- a/arch/arm/boot/dts/r8a7791-porter.dts
> > +++ b/arch/arm/boot/dts/r8a7791-porter.dts
> > @@ -391,7 +391,6 @@
> >
> >  		vin0ep: endpoint {
> >  			remote-endpoint = <&adv7180>;
> > -			bus-width = <8>;
> >  		};
> >  	};
> >  };
> > diff --git a/arch/arm/boot/dts/r8a7793-gose.dts b/arch/arm/boot/dts/r8a7793-gose.dts
> > index 9ed6961..9d3fba2 100644
> > --- a/arch/arm/boot/dts/r8a7793-gose.dts
> > +++ b/arch/arm/boot/dts/r8a7793-gose.dts
> > @@ -759,10 +759,8 @@
> >
> >  		vin0ep2: endpoint {
> >  			remote-endpoint = <&adv7612_out>;
> > -			bus-width = <24>;
> >  			hsync-active = <0>;
> >  			vsync-active = <0>;
> > -			pclk-sample = <1>;
> >  			data-active = <1>;
> >  		};
> >  	};
> > @@ -781,7 +779,6 @@
> >
> >  		vin1ep: endpoint {
> >  			remote-endpoint = <&adv7180_out>;
> > -			bus-width = <8>;
> >  		};
> >  	};
> >  };
> > diff --git a/arch/arm/boot/dts/r8a7794-alt.dts b/arch/arm/boot/dts/r8a7794-alt.dts
> > index 26a8834..4bbb9cc 100644
> > --- a/arch/arm/boot/dts/r8a7794-alt.dts
> > +++ b/arch/arm/boot/dts/r8a7794-alt.dts
> > @@ -380,7 +380,6 @@
> >
> >  		vin0ep: endpoint {
> >  			remote-endpoint = <&adv7180>;
> > -			bus-width = <8>;
> >  		};
> >  	};
> >  };
> > diff --git a/arch/arm/boot/dts/r8a7794-silk.dts b/arch/arm/boot/dts/r8a7794-silk.dts
> > index 351cb3b..c0c5d31 100644
> > --- a/arch/arm/boot/dts/r8a7794-silk.dts
> > +++ b/arch/arm/boot/dts/r8a7794-silk.dts
> > @@ -480,7 +480,6 @@
> >
> >  		vin0ep: endpoint {
> >  			remote-endpoint = <&adv7180>;
> > -			bus-width = <8>;
> >  		};
> >  	};
> >  };
> > --
> > 2.7.4
> >
>
> --
> Regards,
> Niklas S?derlund
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180517/c1578eef/attachment-0001.sig>

Re: [PATCH 5/6] ARM: dts: rcar-gen2: Remove unused VIN properties

[Simon Horman]

On Thu, May 17, 2018 at 11:01:10AM +0200, jacopo mondi wrote:
> Hi Niklas,
> 
> On Thu, May 17, 2018 at 12:13:07AM +0200, Niklas Söderlund wrote:
> > Hi Jacopo,
> >
> > Thanks for your work.
> >
> > On 2018-05-16 18:32:31 +0200, Jacopo Mondi wrote:
> > > The 'bus-width' and 'pclk-sample' properties are not parsed by the VIN
> > > driver and only confuse users. Remove them in all Gen2 SoC that used
> > > them.
> > >
> > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > > ---
> > >  arch/arm/boot/dts/r8a7790-lager.dts   | 3 ---
> > >  arch/arm/boot/dts/r8a7791-koelsch.dts | 3 ---
> > >  arch/arm/boot/dts/r8a7791-porter.dts  | 1 -
> > >  arch/arm/boot/dts/r8a7793-gose.dts    | 3 ---
> > >  arch/arm/boot/dts/r8a7794-alt.dts     | 1 -
> > >  arch/arm/boot/dts/r8a7794-silk.dts    | 1 -
> > >  6 files changed, 12 deletions(-)
> > >
> > > diff --git a/arch/arm/boot/dts/r8a7790-lager.dts b/arch/arm/boot/dts/r8a7790-lager.dts
> > > index 063fdb6..b56b309 100644
> > > --- a/arch/arm/boot/dts/r8a7790-lager.dts
> > > +++ b/arch/arm/boot/dts/r8a7790-lager.dts
> > > @@ -873,10 +873,8 @@
> > >  	port {
> > >  		vin0ep2: endpoint {
> > >  			remote-endpoint = <&adv7612_out>;
> > > -			bus-width = <24>;
> >
> > I can't really make up my mind if this is a good thing or not. Device
> > tree describes the hardware and not what the drivers make use of. And
> > the fact is that this bus is 24 bits wide. So I'm not sure we should
> > remove these properties. But I would love to hear what others think
> > about this.
> >
> 
> Just to point out those properties are not even documented in rcar-vin
> bindings (actually, none of them was).
> 
> I feel it's wrong to have them here, as someone may think that
> changing their value should actually change the VIN interface behavior,
> which it's not true, leading to massive confusion and quite some code
> digging for no reason (and they will get mad at us at some point, probably :)

I think its fine that the driver doesn't implement something described in
DT - we are describing the hardware not the implementation. But I think its
not fine that DT includes properties not described in the binding.

So I think we should either
a) Fix the binding documentation, but perhaps it is already correct
   in which case we should;
b) Apply this patch

Once we have decided what is the correct description of the hardware we
can consider implications on the driver implementation.



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 5/6] ARM: dts: rcar-gen2: Remove unused VIN properties

[Simon Horman]

On Thu, May 17, 2018 at 11:01:10AM +0200, jacopo mondi wrote:
> Hi Niklas,
> 
> On Thu, May 17, 2018 at 12:13:07AM +0200, Niklas Söderlund wrote:
> > Hi Jacopo,
> >
> > Thanks for your work.
> >
> > On 2018-05-16 18:32:31 +0200, Jacopo Mondi wrote:
> > > The 'bus-width' and 'pclk-sample' properties are not parsed by the VIN
> > > driver and only confuse users. Remove them in all Gen2 SoC that used
> > > them.
> > >
> > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > > ---
> > >  arch/arm/boot/dts/r8a7790-lager.dts   | 3 ---
> > >  arch/arm/boot/dts/r8a7791-koelsch.dts | 3 ---
> > >  arch/arm/boot/dts/r8a7791-porter.dts  | 1 -
> > >  arch/arm/boot/dts/r8a7793-gose.dts    | 3 ---
> > >  arch/arm/boot/dts/r8a7794-alt.dts     | 1 -
> > >  arch/arm/boot/dts/r8a7794-silk.dts    | 1 -
> > >  6 files changed, 12 deletions(-)
> > >
> > > diff --git a/arch/arm/boot/dts/r8a7790-lager.dts b/arch/arm/boot/dts/r8a7790-lager.dts
> > > index 063fdb6..b56b309 100644
> > > --- a/arch/arm/boot/dts/r8a7790-lager.dts
> > > +++ b/arch/arm/boot/dts/r8a7790-lager.dts
> > > @@ -873,10 +873,8 @@
> > >  	port {
> > >  		vin0ep2: endpoint {
> > >  			remote-endpoint = <&adv7612_out>;
> > > -			bus-width = <24>;
> >
> > I can't really make up my mind if this is a good thing or not. Device
> > tree describes the hardware and not what the drivers make use of. And
> > the fact is that this bus is 24 bits wide. So I'm not sure we should
> > remove these properties. But I would love to hear what others think
> > about this.
> >
> 
> Just to point out those properties are not even documented in rcar-vin
> bindings (actually, none of them was).
> 
> I feel it's wrong to have them here, as someone may think that
> changing their value should actually change the VIN interface behavior,
> which it's not true, leading to massive confusion and quite some code
> digging for no reason (and they will get mad at us at some point, probably :)

I think its fine that the driver doesn't implement something described in
DT - we are describing the hardware not the implementation. But I think its
not fine that DT includes properties not described in the binding.

So I think we should either
a) Fix the binding documentation, but perhaps it is already correct
   in which case we should;
b) Apply this patch

Once we have decided what is the correct description of the hardware we
can consider implications on the driver implementation.

[PATCH 5/6] ARM: dts: rcar-gen2: Remove unused VIN properties

[Simon Horman]

On Thu, May 17, 2018 at 11:01:10AM +0200, jacopo mondi wrote:
> Hi Niklas,
> 
> On Thu, May 17, 2018 at 12:13:07AM +0200, Niklas S?derlund wrote:
> > Hi Jacopo,
> >
> > Thanks for your work.
> >
> > On 2018-05-16 18:32:31 +0200, Jacopo Mondi wrote:
> > > The 'bus-width' and 'pclk-sample' properties are not parsed by the VIN
> > > driver and only confuse users. Remove them in all Gen2 SoC that used
> > > them.
> > >
> > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > > ---
> > >  arch/arm/boot/dts/r8a7790-lager.dts   | 3 ---
> > >  arch/arm/boot/dts/r8a7791-koelsch.dts | 3 ---
> > >  arch/arm/boot/dts/r8a7791-porter.dts  | 1 -
> > >  arch/arm/boot/dts/r8a7793-gose.dts    | 3 ---
> > >  arch/arm/boot/dts/r8a7794-alt.dts     | 1 -
> > >  arch/arm/boot/dts/r8a7794-silk.dts    | 1 -
> > >  6 files changed, 12 deletions(-)
> > >
> > > diff --git a/arch/arm/boot/dts/r8a7790-lager.dts b/arch/arm/boot/dts/r8a7790-lager.dts
> > > index 063fdb6..b56b309 100644
> > > --- a/arch/arm/boot/dts/r8a7790-lager.dts
> > > +++ b/arch/arm/boot/dts/r8a7790-lager.dts
> > > @@ -873,10 +873,8 @@
> > >  	port {
> > >  		vin0ep2: endpoint {
> > >  			remote-endpoint = <&adv7612_out>;
> > > -			bus-width = <24>;
> >
> > I can't really make up my mind if this is a good thing or not. Device
> > tree describes the hardware and not what the drivers make use of. And
> > the fact is that this bus is 24 bits wide. So I'm not sure we should
> > remove these properties. But I would love to hear what others think
> > about this.
> >
> 
> Just to point out those properties are not even documented in rcar-vin
> bindings (actually, none of them was).
> 
> I feel it's wrong to have them here, as someone may think that
> changing their value should actually change the VIN interface behavior,
> which it's not true, leading to massive confusion and quite some code
> digging for no reason (and they will get mad at us at some point, probably :)

I think its fine that the driver doesn't implement something described in
DT - we are describing the hardware not the implementation. But I think its
not fine that DT includes properties not described in the binding.

So I think we should either
a) Fix the binding documentation, but perhaps it is already correct
   in which case we should;
b) Apply this patch

Once we have decided what is the correct description of the hardware we
can consider implications on the driver implementation.

Re: [PATCH 1/6] dt-bindings: media: rcar-vin: Describe optional ep properties

[Rob Herring]

On Wed, May 16, 2018 at 06:32:27PM +0200, Jacopo Mondi wrote:
> Describe the optional endpoint properties for endpoint nodes of port@0
> and port@1 of the R-Car VIN driver device tree bindings documentation.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
>  Documentation/devicetree/bindings/media/rcar_vin.txt | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt
> index a19517e1..c53ce4e 100644
> --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
> +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
> @@ -53,6 +53,16 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
>        from external SoC pins described in video-interfaces.txt[1].
>        Describing more then one endpoint in port 0 is invalid. Only VIN
>        instances that are connected to external pins should have port 0.
> +
> +      - Optional properties for endpoint nodes of port@0:
> +        - hsync-active: active state of the HSYNC signal, 0/1 for LOW/HIGH
> +	  respectively. Default is active high.
> +        - vsync-active: active state of the VSYNC signal, 0/1 for LOW/HIGH
> +	  respectively. Default is active high.
> +
> +	If both HSYNC and VSYNC polarities are not specified, embedded
> +	synchronization is selected.

No need to copy-n-paste from video-interfaces.txt. Just "see 
video-interfaces.txt" for the description is fine.

> +
>      - port 1 - sub-nodes describing one or more endpoints connected to
>        the VIN from local SoC CSI-2 receivers. The endpoint numbers must
>        use the following schema.
> @@ -62,6 +72,8 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
>          - Endpoint 2 - sub-node describing the endpoint connected to CSI40
>          - Endpoint 3 - sub-node describing the endpoint connected to CSI41
> 
> +      Endpoint nodes of port@1 do not support any optional endpoint property.
> +
>  Device node example for Gen2 platforms
>  --------------------------------------
> 
> @@ -112,7 +124,6 @@ Board setup example for Gen2 platforms (vin1 composite video input)
> 
>                  vin1ep0: endpoint {
>                          remote-endpoint = <&adv7180>;
> -                        bus-width = <8>;
>                  };
>          };
>  };
> --
> 2.7.4
> 

Re: [PATCH 1/6] dt-bindings: media: rcar-vin: Describe optional ep properties

[Rob Herring]

On Wed, May 16, 2018 at 06:32:27PM +0200, Jacopo Mondi wrote:
> Describe the optional endpoint properties for endpoint nodes of port@0
> and port@1 of the R-Car VIN driver device tree bindings documentation.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
>  Documentation/devicetree/bindings/media/rcar_vin.txt | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt
> index a19517e1..c53ce4e 100644
> --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
> +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
> @@ -53,6 +53,16 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
>        from external SoC pins described in video-interfaces.txt[1].
>        Describing more then one endpoint in port 0 is invalid. Only VIN
>        instances that are connected to external pins should have port 0.
> +
> +      - Optional properties for endpoint nodes of port@0:
> +        - hsync-active: active state of the HSYNC signal, 0/1 for LOW/HIGH
> +	  respectively. Default is active high.
> +        - vsync-active: active state of the VSYNC signal, 0/1 for LOW/HIGH
> +	  respectively. Default is active high.
> +
> +	If both HSYNC and VSYNC polarities are not specified, embedded
> +	synchronization is selected.

No need to copy-n-paste from video-interfaces.txt. Just "see 
video-interfaces.txt" for the description is fine.

> +
>      - port 1 - sub-nodes describing one or more endpoints connected to
>        the VIN from local SoC CSI-2 receivers. The endpoint numbers must
>        use the following schema.
> @@ -62,6 +72,8 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
>          - Endpoint 2 - sub-node describing the endpoint connected to CSI40
>          - Endpoint 3 - sub-node describing the endpoint connected to CSI41
> 
> +      Endpoint nodes of port@1 do not support any optional endpoint property.
> +
>  Device node example for Gen2 platforms
>  --------------------------------------
> 
> @@ -112,7 +124,6 @@ Board setup example for Gen2 platforms (vin1 composite video input)
> 
>                  vin1ep0: endpoint {
>                          remote-endpoint = <&adv7180>;
> -                        bus-width = <8>;
>                  };
>          };
>  };
> --
> 2.7.4
> 

[PATCH 1/6] dt-bindings: media: rcar-vin: Describe optional ep properties

[Rob Herring]

On Wed, May 16, 2018 at 06:32:27PM +0200, Jacopo Mondi wrote:
> Describe the optional endpoint properties for endpoint nodes of port at 0
> and port at 1 of the R-Car VIN driver device tree bindings documentation.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
>  Documentation/devicetree/bindings/media/rcar_vin.txt | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt
> index a19517e1..c53ce4e 100644
> --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
> +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
> @@ -53,6 +53,16 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
>        from external SoC pins described in video-interfaces.txt[1].
>        Describing more then one endpoint in port 0 is invalid. Only VIN
>        instances that are connected to external pins should have port 0.
> +
> +      - Optional properties for endpoint nodes of port at 0:
> +        - hsync-active: active state of the HSYNC signal, 0/1 for LOW/HIGH
> +	  respectively. Default is active high.
> +        - vsync-active: active state of the VSYNC signal, 0/1 for LOW/HIGH
> +	  respectively. Default is active high.
> +
> +	If both HSYNC and VSYNC polarities are not specified, embedded
> +	synchronization is selected.

No need to copy-n-paste from video-interfaces.txt. Just "see 
video-interfaces.txt" for the description is fine.

> +
>      - port 1 - sub-nodes describing one or more endpoints connected to
>        the VIN from local SoC CSI-2 receivers. The endpoint numbers must
>        use the following schema.
> @@ -62,6 +72,8 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
>          - Endpoint 2 - sub-node describing the endpoint connected to CSI40
>          - Endpoint 3 - sub-node describing the endpoint connected to CSI41
> 
> +      Endpoint nodes of port at 1 do not support any optional endpoint property.
> +
>  Device node example for Gen2 platforms
>  --------------------------------------
> 
> @@ -112,7 +124,6 @@ Board setup example for Gen2 platforms (vin1 composite video input)
> 
>                  vin1ep0: endpoint {
>                          remote-endpoint = <&adv7180>;
> -                        bus-width = <8>;
>                  };
>          };
>  };
> --
> 2.7.4
> 

Re: [PATCH 5/6] ARM: dts: rcar-gen2: Remove unused VIN properties

[Rob Herring]

On Thu, May 17, 2018 at 12:13:07AM +0200, Niklas Söderlund wrote:
> Hi Jacopo,
> 
> Thanks for your work.
> 
> On 2018-05-16 18:32:31 +0200, Jacopo Mondi wrote:
> > The 'bus-width' and 'pclk-sample' properties are not parsed by the VIN
> > driver and only confuse users. Remove them in all Gen2 SoC that used
> > them.
> > 
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > ---
> >  arch/arm/boot/dts/r8a7790-lager.dts   | 3 ---
> >  arch/arm/boot/dts/r8a7791-koelsch.dts | 3 ---
> >  arch/arm/boot/dts/r8a7791-porter.dts  | 1 -
> >  arch/arm/boot/dts/r8a7793-gose.dts    | 3 ---
> >  arch/arm/boot/dts/r8a7794-alt.dts     | 1 -
> >  arch/arm/boot/dts/r8a7794-silk.dts    | 1 -
> >  6 files changed, 12 deletions(-)
> > 
> > diff --git a/arch/arm/boot/dts/r8a7790-lager.dts b/arch/arm/boot/dts/r8a7790-lager.dts
> > index 063fdb6..b56b309 100644
> > --- a/arch/arm/boot/dts/r8a7790-lager.dts
> > +++ b/arch/arm/boot/dts/r8a7790-lager.dts
> > @@ -873,10 +873,8 @@
> >  	port {
> >  		vin0ep2: endpoint {
> >  			remote-endpoint = <&adv7612_out>;
> > -			bus-width = <24>;
> 
> I can't really make up my mind if this is a good thing or not. Device 
> tree describes the hardware and not what the drivers make use of. And 
> the fact is that this bus is 24 bits wide. So I'm not sure we should 
> remove these properties. But I would love to hear what others think 
> about this.

IMO, this property should only be present when all the pins are not 
connected. And by "all", I mean the minimum of what each end of the 
graph can support.

Rob

Re: [PATCH 5/6] ARM: dts: rcar-gen2: Remove unused VIN properties

[Rob Herring]

On Thu, May 17, 2018 at 12:13:07AM +0200, Niklas Söderlund wrote:
> Hi Jacopo,
> 
> Thanks for your work.
> 
> On 2018-05-16 18:32:31 +0200, Jacopo Mondi wrote:
> > The 'bus-width' and 'pclk-sample' properties are not parsed by the VIN
> > driver and only confuse users. Remove them in all Gen2 SoC that used
> > them.
> > 
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > ---
> >  arch/arm/boot/dts/r8a7790-lager.dts   | 3 ---
> >  arch/arm/boot/dts/r8a7791-koelsch.dts | 3 ---
> >  arch/arm/boot/dts/r8a7791-porter.dts  | 1 -
> >  arch/arm/boot/dts/r8a7793-gose.dts    | 3 ---
> >  arch/arm/boot/dts/r8a7794-alt.dts     | 1 -
> >  arch/arm/boot/dts/r8a7794-silk.dts    | 1 -
> >  6 files changed, 12 deletions(-)
> > 
> > diff --git a/arch/arm/boot/dts/r8a7790-lager.dts b/arch/arm/boot/dts/r8a7790-lager.dts
> > index 063fdb6..b56b309 100644
> > --- a/arch/arm/boot/dts/r8a7790-lager.dts
> > +++ b/arch/arm/boot/dts/r8a7790-lager.dts
> > @@ -873,10 +873,8 @@
> >  	port {
> >  		vin0ep2: endpoint {
> >  			remote-endpoint = <&adv7612_out>;
> > -			bus-width = <24>;
> 
> I can't really make up my mind if this is a good thing or not. Device 
> tree describes the hardware and not what the drivers make use of. And 
> the fact is that this bus is 24 bits wide. So I'm not sure we should 
> remove these properties. But I would love to hear what others think 
> about this.

IMO, this property should only be present when all the pins are not 
connected. And by "all", I mean the minimum of what each end of the 
graph can support.

Rob

Re: [PATCH 5/6] ARM: dts: rcar-gen2: Remove unused VIN properties

[Rob Herring]

On Thu, May 17, 2018 at 12:13:07AM +0200, Niklas S�derlund wrote:
> Hi Jacopo,
> 
> Thanks for your work.
> 
> On 2018-05-16 18:32:31 +0200, Jacopo Mondi wrote:
> > The 'bus-width' and 'pclk-sample' properties are not parsed by the VIN
> > driver and only confuse users. Remove them in all Gen2 SoC that used
> > them.
> > 
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > ---
> >  arch/arm/boot/dts/r8a7790-lager.dts   | 3 ---
> >  arch/arm/boot/dts/r8a7791-koelsch.dts | 3 ---
> >  arch/arm/boot/dts/r8a7791-porter.dts  | 1 -
> >  arch/arm/boot/dts/r8a7793-gose.dts    | 3 ---
> >  arch/arm/boot/dts/r8a7794-alt.dts     | 1 -
> >  arch/arm/boot/dts/r8a7794-silk.dts    | 1 -
> >  6 files changed, 12 deletions(-)
> > 
> > diff --git a/arch/arm/boot/dts/r8a7790-lager.dts b/arch/arm/boot/dts/r8a7790-lager.dts
> > index 063fdb6..b56b309 100644
> > --- a/arch/arm/boot/dts/r8a7790-lager.dts
> > +++ b/arch/arm/boot/dts/r8a7790-lager.dts
> > @@ -873,10 +873,8 @@
> >  	port {
> >  		vin0ep2: endpoint {
> >  			remote-endpoint = <&adv7612_out>;
> > -			bus-width = <24>;
> 
> I can't really make up my mind if this is a good thing or not. Device 
> tree describes the hardware and not what the drivers make use of. And 
> the fact is that this bus is 24 bits wide. So I'm not sure we should 
> remove these properties. But I would love to hear what others think 
> about this.

IMO, this property should only be present when all the pins are not 
connected. And by "all", I mean the minimum of what each end of the 
graph can support.

Rob

[PATCH 5/6] ARM: dts: rcar-gen2: Remove unused VIN properties

[Rob Herring]

On Thu, May 17, 2018 at 12:13:07AM +0200, Niklas S?derlund wrote:
> Hi Jacopo,
> 
> Thanks for your work.
> 
> On 2018-05-16 18:32:31 +0200, Jacopo Mondi wrote:
> > The 'bus-width' and 'pclk-sample' properties are not parsed by the VIN
> > driver and only confuse users. Remove them in all Gen2 SoC that used
> > them.
> > 
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > ---
> >  arch/arm/boot/dts/r8a7790-lager.dts   | 3 ---
> >  arch/arm/boot/dts/r8a7791-koelsch.dts | 3 ---
> >  arch/arm/boot/dts/r8a7791-porter.dts  | 1 -
> >  arch/arm/boot/dts/r8a7793-gose.dts    | 3 ---
> >  arch/arm/boot/dts/r8a7794-alt.dts     | 1 -
> >  arch/arm/boot/dts/r8a7794-silk.dts    | 1 -
> >  6 files changed, 12 deletions(-)
> > 
> > diff --git a/arch/arm/boot/dts/r8a7790-lager.dts b/arch/arm/boot/dts/r8a7790-lager.dts
> > index 063fdb6..b56b309 100644
> > --- a/arch/arm/boot/dts/r8a7790-lager.dts
> > +++ b/arch/arm/boot/dts/r8a7790-lager.dts
> > @@ -873,10 +873,8 @@
> >  	port {
> >  		vin0ep2: endpoint {
> >  			remote-endpoint = <&adv7612_out>;
> > -			bus-width = <24>;
> 
> I can't really make up my mind if this is a good thing or not. Device 
> tree describes the hardware and not what the drivers make use of. And 
> the fact is that this bus is 24 bits wide. So I'm not sure we should 
> remove these properties. But I would love to hear what others think 
> about this.

IMO, this property should only be present when all the pins are not 
connected. And by "all", I mean the minimum of what each end of the 
graph can support.

Rob

Re: [PATCH 2/6] dt-bindings: media: rcar-vin: Document data-active

[Rob Herring]

On Wed, May 16, 2018 at 06:32:28PM +0200, Jacopo Mondi wrote:
> Document 'data-active' property in R-Car VIN device tree bindings.
> The property is optional when running with explicit synchronization
> (eg. BT.601) but mandatory when embedded synchronization is in use (eg.
> BT.656) as specified by the hardware manual.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
>  Documentation/devicetree/bindings/media/rcar_vin.txt | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt
> index c53ce4e..17eac8a 100644
> --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
> +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
> @@ -63,6 +63,11 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
>  	If both HSYNC and VSYNC polarities are not specified, embedded
>  	synchronization is selected.
> 
> +        - data-active: active state of data enable signal (CLOCKENB pin).
> +          0/1 for LOW/HIGH respectively. If not specified, use HSYNC as
> +          data enable signal. When using embedded synchronization this
> +          property is mandatory.

This doesn't match the common property's definition which AIUI is for 
the data lines' polarity. You need a new property. Perhaps a common one.

> +
>      - port 1 - sub-nodes describing one or more endpoints connected to
>        the VIN from local SoC CSI-2 receivers. The endpoint numbers must
>        use the following schema.
> --
> 2.7.4
> 

Re: [PATCH 2/6] dt-bindings: media: rcar-vin: Document data-active

[Rob Herring]

On Wed, May 16, 2018 at 06:32:28PM +0200, Jacopo Mondi wrote:
> Document 'data-active' property in R-Car VIN device tree bindings.
> The property is optional when running with explicit synchronization
> (eg. BT.601) but mandatory when embedded synchronization is in use (eg.
> BT.656) as specified by the hardware manual.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
>  Documentation/devicetree/bindings/media/rcar_vin.txt | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt
> index c53ce4e..17eac8a 100644
> --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
> +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
> @@ -63,6 +63,11 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
>  	If both HSYNC and VSYNC polarities are not specified, embedded
>  	synchronization is selected.
> 
> +        - data-active: active state of data enable signal (CLOCKENB pin).
> +          0/1 for LOW/HIGH respectively. If not specified, use HSYNC as
> +          data enable signal. When using embedded synchronization this
> +          property is mandatory.

This doesn't match the common property's definition which AIUI is for 
the data lines' polarity. You need a new property. Perhaps a common one.

> +
>      - port 1 - sub-nodes describing one or more endpoints connected to
>        the VIN from local SoC CSI-2 receivers. The endpoint numbers must
>        use the following schema.
> --
> 2.7.4
> 

[PATCH 2/6] dt-bindings: media: rcar-vin: Document data-active

[Rob Herring]

On Wed, May 16, 2018 at 06:32:28PM +0200, Jacopo Mondi wrote:
> Document 'data-active' property in R-Car VIN device tree bindings.
> The property is optional when running with explicit synchronization
> (eg. BT.601) but mandatory when embedded synchronization is in use (eg.
> BT.656) as specified by the hardware manual.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
>  Documentation/devicetree/bindings/media/rcar_vin.txt | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt
> index c53ce4e..17eac8a 100644
> --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
> +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
> @@ -63,6 +63,11 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
>  	If both HSYNC and VSYNC polarities are not specified, embedded
>  	synchronization is selected.
> 
> +        - data-active: active state of data enable signal (CLOCKENB pin).
> +          0/1 for LOW/HIGH respectively. If not specified, use HSYNC as
> +          data enable signal. When using embedded synchronization this
> +          property is mandatory.

This doesn't match the common property's definition which AIUI is for 
the data lines' polarity. You need a new property. Perhaps a common one.

> +
>      - port 1 - sub-nodes describing one or more endpoints connected to
>        the VIN from local SoC CSI-2 receivers. The endpoint numbers must
>        use the following schema.
> --
> 2.7.4
> 

Re: [PATCH 1/6] dt-bindings: media: rcar-vin: Describe optional ep properties

[Laurent Pinchart]

Hi Rob,

On Wednesday, 23 May 2018 19:29:47 EEST Rob Herring wrote:
> On Wed, May 16, 2018 at 06:32:27PM +0200, Jacopo Mondi wrote:
> > Describe the optional endpoint properties for endpoint nodes of port@0
> > and port@1 of the R-Car VIN driver device tree bindings documentation.
> > 
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > ---
> > 
> >  Documentation/devicetree/bindings/media/rcar_vin.txt | 13 ++++++++++++-
> >  1 file changed, 12 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt
> > b/Documentation/devicetree/bindings/media/rcar_vin.txt index
> > a19517e1..c53ce4e 100644
> > --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
> > +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
> > @@ -53,6 +53,16 @@ from local SoC CSI-2 receivers (port1) depending on
> > SoC.
> > 
> >        from external SoC pins described in video-interfaces.txt[1].
> >        Describing more then one endpoint in port 0 is invalid. Only VIN
> >        instances that are connected to external pins should have port 0.
> > 
> > +
> > +      - Optional properties for endpoint nodes of port@0:
> > +        - hsync-active: active state of the HSYNC signal, 0/1 for
> > LOW/HIGH
> > +	  respectively. Default is active high.
> > +        - vsync-active: active state of the VSYNC signal, 0/1 for
> > LOW/HIGH
> > +	  respectively. Default is active high.
> > +
> > +	If both HSYNC and VSYNC polarities are not specified, embedded
> > +	synchronization is selected.
> 
> No need to copy-n-paste from video-interfaces.txt. Just "see
> video-interfaces.txt" for the description is fine.

I would still explicitly list the properties that apply to this binding. I 
agree that there's no need to copy & paste the description of those properties 
though.

> > +
> > 
> >      - port 1 - sub-nodes describing one or more endpoints connected to
> >      
> >        the VIN from local SoC CSI-2 receivers. The endpoint numbers must
> >        use the following schema.
> > 
> > @@ -62,6 +72,8 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
> > 
> >          - Endpoint 2 - sub-node describing the endpoint connected to
> >          CSI40
> >          - Endpoint 3 - sub-node describing the endpoint connected to
> >          CSI41
> > 
> > +      Endpoint nodes of port@1 do not support any optional endpoint
> > property. +
> > 
> >  Device node example for Gen2 platforms
> >  --------------------------------------
> > 
> > @@ -112,7 +124,6 @@ Board setup example for Gen2 platforms (vin1 composite
> > video input)> 
> >                  vin1ep0: endpoint {
> >                  
> >                          remote-endpoint = <&adv7180>;
> > 
> > -                        bus-width = <8>;
> > 
> >                  };
> >          
> >          };
> >  
> >  };

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/6] dt-bindings: media: rcar-vin: Describe optional ep properties

[Laurent Pinchart]

Hi Rob,

On Wednesday, 23 May 2018 19:29:47 EEST Rob Herring wrote:
> On Wed, May 16, 2018 at 06:32:27PM +0200, Jacopo Mondi wrote:
> > Describe the optional endpoint properties for endpoint nodes of port@0
> > and port@1 of the R-Car VIN driver device tree bindings documentation.
> > 
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > ---
> > 
> >  Documentation/devicetree/bindings/media/rcar_vin.txt | 13 ++++++++++++-
> >  1 file changed, 12 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt
> > b/Documentation/devicetree/bindings/media/rcar_vin.txt index
> > a19517e1..c53ce4e 100644
> > --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
> > +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
> > @@ -53,6 +53,16 @@ from local SoC CSI-2 receivers (port1) depending on
> > SoC.
> > 
> >        from external SoC pins described in video-interfaces.txt[1].
> >        Describing more then one endpoint in port 0 is invalid. Only VIN
> >        instances that are connected to external pins should have port 0.
> > 
> > +
> > +      - Optional properties for endpoint nodes of port@0:
> > +        - hsync-active: active state of the HSYNC signal, 0/1 for
> > LOW/HIGH
> > +	  respectively. Default is active high.
> > +        - vsync-active: active state of the VSYNC signal, 0/1 for
> > LOW/HIGH
> > +	  respectively. Default is active high.
> > +
> > +	If both HSYNC and VSYNC polarities are not specified, embedded
> > +	synchronization is selected.
> 
> No need to copy-n-paste from video-interfaces.txt. Just "see
> video-interfaces.txt" for the description is fine.

I would still explicitly list the properties that apply to this binding. I 
agree that there's no need to copy & paste the description of those properties 
though.

> > +
> > 
> >      - port 1 - sub-nodes describing one or more endpoints connected to
> >      
> >        the VIN from local SoC CSI-2 receivers. The endpoint numbers must
> >        use the following schema.
> > 
> > @@ -62,6 +72,8 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
> > 
> >          - Endpoint 2 - sub-node describing the endpoint connected to
> >          CSI40
> >          - Endpoint 3 - sub-node describing the endpoint connected to
> >          CSI41
> > 
> > +      Endpoint nodes of port@1 do not support any optional endpoint
> > property. +
> > 
> >  Device node example for Gen2 platforms
> >  --------------------------------------
> > 
> > @@ -112,7 +124,6 @@ Board setup example for Gen2 platforms (vin1 composite
> > video input)> 
> >                  vin1ep0: endpoint {
> >                  
> >                          remote-endpoint = <&adv7180>;
> > 
> > -                        bus-width = <8>;
> > 
> >                  };
> >          
> >          };
> >  
> >  };

-- 
Regards,

Laurent Pinchart

[PATCH 1/6] dt-bindings: media: rcar-vin: Describe optional ep properties

[Laurent Pinchart]

Hi Rob,

On Wednesday, 23 May 2018 19:29:47 EEST Rob Herring wrote:
> On Wed, May 16, 2018 at 06:32:27PM +0200, Jacopo Mondi wrote:
> > Describe the optional endpoint properties for endpoint nodes of port at 0
> > and port at 1 of the R-Car VIN driver device tree bindings documentation.
> > 
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > ---
> > 
> >  Documentation/devicetree/bindings/media/rcar_vin.txt | 13 ++++++++++++-
> >  1 file changed, 12 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt
> > b/Documentation/devicetree/bindings/media/rcar_vin.txt index
> > a19517e1..c53ce4e 100644
> > --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
> > +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
> > @@ -53,6 +53,16 @@ from local SoC CSI-2 receivers (port1) depending on
> > SoC.
> > 
> >        from external SoC pins described in video-interfaces.txt[1].
> >        Describing more then one endpoint in port 0 is invalid. Only VIN
> >        instances that are connected to external pins should have port 0.
> > 
> > +
> > +      - Optional properties for endpoint nodes of port at 0:
> > +        - hsync-active: active state of the HSYNC signal, 0/1 for
> > LOW/HIGH
> > +	  respectively. Default is active high.
> > +        - vsync-active: active state of the VSYNC signal, 0/1 for
> > LOW/HIGH
> > +	  respectively. Default is active high.
> > +
> > +	If both HSYNC and VSYNC polarities are not specified, embedded
> > +	synchronization is selected.
> 
> No need to copy-n-paste from video-interfaces.txt. Just "see
> video-interfaces.txt" for the description is fine.

I would still explicitly list the properties that apply to this binding. I 
agree that there's no need to copy & paste the description of those properties 
though.

> > +
> > 
> >      - port 1 - sub-nodes describing one or more endpoints connected to
> >      
> >        the VIN from local SoC CSI-2 receivers. The endpoint numbers must
> >        use the following schema.
> > 
> > @@ -62,6 +72,8 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
> > 
> >          - Endpoint 2 - sub-node describing the endpoint connected to
> >          CSI40
> >          - Endpoint 3 - sub-node describing the endpoint connected to
> >          CSI41
> > 
> > +      Endpoint nodes of port at 1 do not support any optional endpoint
> > property. +
> > 
> >  Device node example for Gen2 platforms
> >  --------------------------------------
> > 
> > @@ -112,7 +124,6 @@ Board setup example for Gen2 platforms (vin1 composite
> > video input)> 
> >                  vin1ep0: endpoint {
> >                  
> >                          remote-endpoint = <&adv7180>;
> > 
> > -                        bus-width = <8>;
> > 
> >                  };
> >          
> >          };
> >  
> >  };

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/6] dt-bindings: media: rcar-vin: Describe optional ep properties

[Rob Herring]

On Wed, May 23, 2018 at 2:38 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Rob,
>
> On Wednesday, 23 May 2018 19:29:47 EEST Rob Herring wrote:
>> On Wed, May 16, 2018 at 06:32:27PM +0200, Jacopo Mondi wrote:
>> > Describe the optional endpoint properties for endpoint nodes of port@0
>> > and port@1 of the R-Car VIN driver device tree bindings documentation.
>> >
>> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
>> > ---
>> >
>> >  Documentation/devicetree/bindings/media/rcar_vin.txt | 13 ++++++++++++-
>> >  1 file changed, 12 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt
>> > b/Documentation/devicetree/bindings/media/rcar_vin.txt index
>> > a19517e1..c53ce4e 100644
>> > --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
>> > +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
>> > @@ -53,6 +53,16 @@ from local SoC CSI-2 receivers (port1) depending on
>> > SoC.
>> >
>> >        from external SoC pins described in video-interfaces.txt[1].
>> >        Describing more then one endpoint in port 0 is invalid. Only VIN
>> >        instances that are connected to external pins should have port 0.
>> >
>> > +
>> > +      - Optional properties for endpoint nodes of port@0:
>> > +        - hsync-active: active state of the HSYNC signal, 0/1 for
>> > LOW/HIGH
>> > +     respectively. Default is active high.
>> > +        - vsync-active: active state of the VSYNC signal, 0/1 for
>> > LOW/HIGH
>> > +     respectively. Default is active high.
>> > +
>> > +   If both HSYNC and VSYNC polarities are not specified, embedded
>> > +   synchronization is selected.
>>
>> No need to copy-n-paste from video-interfaces.txt. Just "see
>> video-interfaces.txt" for the description is fine.
>
> I would still explicitly list the properties that apply to this binding. I
> agree that there's no need to copy & paste the description of those properties
> though.

Yes, that's what I meant. List each property with "see
video-interfaces.txt" for the description of each.

Rob

Re: [PATCH 1/6] dt-bindings: media: rcar-vin: Describe optional ep properties

[Rob Herring]

On Wed, May 23, 2018 at 2:38 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Rob,
>
> On Wednesday, 23 May 2018 19:29:47 EEST Rob Herring wrote:
>> On Wed, May 16, 2018 at 06:32:27PM +0200, Jacopo Mondi wrote:
>> > Describe the optional endpoint properties for endpoint nodes of port@0
>> > and port@1 of the R-Car VIN driver device tree bindings documentation.
>> >
>> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
>> > ---
>> >
>> >  Documentation/devicetree/bindings/media/rcar_vin.txt | 13 ++++++++++++-
>> >  1 file changed, 12 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt
>> > b/Documentation/devicetree/bindings/media/rcar_vin.txt index
>> > a19517e1..c53ce4e 100644
>> > --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
>> > +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
>> > @@ -53,6 +53,16 @@ from local SoC CSI-2 receivers (port1) depending on
>> > SoC.
>> >
>> >        from external SoC pins described in video-interfaces.txt[1].
>> >        Describing more then one endpoint in port 0 is invalid. Only VIN
>> >        instances that are connected to external pins should have port 0.
>> >
>> > +
>> > +      - Optional properties for endpoint nodes of port@0:
>> > +        - hsync-active: active state of the HSYNC signal, 0/1 for
>> > LOW/HIGH
>> > +     respectively. Default is active high.
>> > +        - vsync-active: active state of the VSYNC signal, 0/1 for
>> > LOW/HIGH
>> > +     respectively. Default is active high.
>> > +
>> > +   If both HSYNC and VSYNC polarities are not specified, embedded
>> > +   synchronization is selected.
>>
>> No need to copy-n-paste from video-interfaces.txt. Just "see
>> video-interfaces.txt" for the description is fine.
>
> I would still explicitly list the properties that apply to this binding. I
> agree that there's no need to copy & paste the description of those properties
> though.

Yes, that's what I meant. List each property with "see
video-interfaces.txt" for the description of each.

Rob

[PATCH 1/6] dt-bindings: media: rcar-vin: Describe optional ep properties

[Rob Herring]

On Wed, May 23, 2018 at 2:38 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Rob,
>
> On Wednesday, 23 May 2018 19:29:47 EEST Rob Herring wrote:
>> On Wed, May 16, 2018 at 06:32:27PM +0200, Jacopo Mondi wrote:
>> > Describe the optional endpoint properties for endpoint nodes of port at 0
>> > and port at 1 of the R-Car VIN driver device tree bindings documentation.
>> >
>> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
>> > ---
>> >
>> >  Documentation/devicetree/bindings/media/rcar_vin.txt | 13 ++++++++++++-
>> >  1 file changed, 12 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt
>> > b/Documentation/devicetree/bindings/media/rcar_vin.txt index
>> > a19517e1..c53ce4e 100644
>> > --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
>> > +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
>> > @@ -53,6 +53,16 @@ from local SoC CSI-2 receivers (port1) depending on
>> > SoC.
>> >
>> >        from external SoC pins described in video-interfaces.txt[1].
>> >        Describing more then one endpoint in port 0 is invalid. Only VIN
>> >        instances that are connected to external pins should have port 0.
>> >
>> > +
>> > +      - Optional properties for endpoint nodes of port at 0:
>> > +        - hsync-active: active state of the HSYNC signal, 0/1 for
>> > LOW/HIGH
>> > +     respectively. Default is active high.
>> > +        - vsync-active: active state of the VSYNC signal, 0/1 for
>> > LOW/HIGH
>> > +     respectively. Default is active high.
>> > +
>> > +   If both HSYNC and VSYNC polarities are not specified, embedded
>> > +   synchronization is selected.
>>
>> No need to copy-n-paste from video-interfaces.txt. Just "see
>> video-interfaces.txt" for the description is fine.
>
> I would still explicitly list the properties that apply to this binding. I
> agree that there's no need to copy & paste the description of those properties
> though.

Yes, that's what I meant. List each property with "see
video-interfaces.txt" for the description of each.

Rob