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

[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 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 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

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 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 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 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 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

[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 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 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 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

[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