[PATCH v3 0/3] arm64: dts: Draak: Enable video inputs and VIN4

[PATCH v3 0/3] arm64: dts: Draak: Enable video inputs and VIN4

[Jacopo Mondi]

Hello,
  this series enables HDMI, CVBS and VIN4 for R8A77995 Draak board.

Compared to v2, I have added review tags and a missing Signed-off-by line.
Two small comments added to Draak DTS to describe the shared video input path
between CVBS and HDMI. As in v2, CVBS is the default video input and thus is
connected to VIN in DTS.

Switching to HDMI is shown in the patch on top of the following branch:
git://jmondi.org/linux d3/media-master/salvator-x-dts_csi2/d3-hdmi

Simon: once the series this one depends on [1] has been accepted, I guess this
one is ready to get in, right?

The series has been developed on top of media-master tree but applies cleanly
on top of latest renesas-driver.

Thanks
   j

[1] [PATCH v3 0/9] rcar-vin: Add support for parallel input on Gen3

v2 -> v3:
- Add comment to CVBS and HDMI inputs
- Add missing Signed-off-by to [2/3]
- Add Niklas' tag

Jacopo Mondi (3):
  dt-bindings: media: rcar-vin: Add R8A77995 support
  arm64: dts: renesas: draak: Describe CVBS input
  arm64: dts: renesas: draak: Describe HDMI input

 .../devicetree/bindings/media/rcar_vin.txt         |  1 +
 arch/arm64/boot/dts/renesas/r8a77995-draak.dts     | 90 ++++++++++++++++++++++
 2 files changed, 91 insertions(+)

--
2.7.4

[PATCH v3 1/3] dt-bindings: media: rcar-vin: Add R8A77995 support

[Jacopo Mondi]

Add compatible string for R-Car D3 R8A7795 to list of SoCs supported by
rcar-vin driver.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Acked-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
---
 Documentation/devicetree/bindings/media/rcar_vin.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt
index a19517e1..5c6f2a7 100644
--- a/Documentation/devicetree/bindings/media/rcar_vin.txt
+++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
@@ -22,6 +22,7 @@ on Gen3 platforms to a CSI-2 receiver.
    - "renesas,vin-r8a7795" for the R8A7795 device
    - "renesas,vin-r8a7796" for the R8A7796 device
    - "renesas,vin-r8a77970" for the R8A77970 device
+   - "renesas,vin-r8a77995" for the R8A77995 device
    - "renesas,rcar-gen2-vin" for a generic R-Car Gen2 or RZ/G1 compatible
      device.

--
2.7.4

[PATCH v3 2/3] arm64: dts: renesas: draak: Describe CVBS input

[Jacopo Mondi]

Describe CVBS video input through analog video decoder ADV7180
connected to video input interface VIN4.

The video input signal path is shared with HDMI video input, and
selected by on-board switches SW-53 and SW-54 with CVBS input selected
by the default switches configuration.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

---
v2 -> v3:
- Add comment to describe the shared input video path
- Add my SoB and Niklas' R-b tags
---
 arch/arm64/boot/dts/renesas/r8a77995-draak.dts | 42 ++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
index 9d73de8..95745fc 100644
--- a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
+++ b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
@@ -142,6 +142,11 @@
 		groups = "usb0";
 		function = "usb0";
 	};
+
+	vin4_pins_cvbs: vin4 {
+		groups = "vin4_data8", "vin4_sync", "vin4_clk";
+		function = "vin4";
+	};
 };

 &i2c0 {
@@ -154,6 +159,23 @@
 		reg = <0x50>;
 		pagesize = <8>;
 	};
+
+	analog-video@20 {
+		compatible = "adi,adv7180";
+		reg = <0x20>;
+
+		port {
+			/*
+			 * The VIN4 video input path is shared between
+			 * CVBS and HDMI inputs through SW[49-54] switches.
+			 *
+			 * CVBS is the default selection, link it to VIN4 here.
+			 */
+			adv7180_out: endpoint {
+				remote-endpoint = <&vin4_in>;
+			};
+		};
+	};
 };

 &i2c1 {
@@ -246,3 +268,23 @@
 	timeout-sec = <60>;
 	status = "okay";
 };
+
+&vin4 {
+	pinctrl-0 = <&vin4_pins_cvbs>;
+	pinctrl-names = "default";
+
+	status = "okay";
+
+	ports {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		port@0 {
+			reg = <0>;
+
+			vin4_in: endpoint {
+				remote-endpoint = <&adv7180_out>;
+			};
+		};
+	};
+};
--
2.7.4

[PATCH v3 3/3] arm64: dts: renesas: draak: Describe HDMI input

[Jacopo Mondi]

Describe HDMI input connector and ADV7612 HDMI decoder installed on
R-Car Gen3 Draak board.

The video signal routing to the HDMI decoder to the video input interface
VIN4 is multiplexed with CVBS input path, and enabled/disabled through
on-board switches SW-49, SW-50, SW-51 and SW-52.

As the default board switches configuration connects CVBS input to VIN4,
leave the HDMI decoder unconnected in DTS.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

---
v2 -> v3:
- Add comment on HDMI output port about the shared CVBS/HDMI video path
- Add Niklas' R-b tag
---
 arch/arm64/boot/dts/renesas/r8a77995-draak.dts | 48 ++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
index 95745fc..1e475a4 100644
--- a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
+++ b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
@@ -59,6 +59,17 @@
 		};
 	};

+	hdmi-in {
+		compatible = "hdmi-connector";
+		type = "a";
+
+		port {
+			hdmi_con_in: endpoint {
+				remote-endpoint = <&adv7612_in>;
+			};
+		};
+	};
+
 	memory@48000000 {
 		device_type = "memory";
 		/* first 128MB is reserved for secure area. */
@@ -176,6 +187,43 @@
 			};
 		};
 	};
+
+	hdmi-decoder@4c {
+		compatible = "adi,adv7612";
+		reg = <0x4c>;
+		default-input = <0>;
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			port@0 {
+				reg = <0>;
+
+				adv7612_in: endpoint {
+					remote-endpoint = <&hdmi_con_in>;
+				};
+			};
+
+			port@2 {
+				reg = <2>;
+
+				/*
+				 * The VIN4 video input path is shared between
+				 * CVBS and HDMI inputs through SW[49-54]
+				 * switches.
+				 *
+				 * CVBS is the default selection, leave HDMI
+				 * not connected here.
+				 */
+				adv7612_out: endpoint {
+					pclk-sample = <0>;
+					hsync-active = <0>;
+					vsync-active = <0>;
+				};
+			};
+		};
+	};
 };

 &i2c1 {
--
2.7.4

Re: [PATCH v3 2/3] arm64: dts: renesas: draak: Describe CVBS input

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Friday, 18 May 2018 17:47:57 EEST Jacopo Mondi wrote:
> Describe CVBS video input through analog video decoder ADV7180
> connected to video input interface VIN4.
> 
> The video input signal path is shared with HDMI video input, and
> selected by on-board switches SW-53 and SW-54 with CVBS input selected
> by the default switches configuration.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> 
> ---
> v2 -> v3:
> - Add comment to describe the shared input video path
> - Add my SoB and Niklas' R-b tags
> ---
>  arch/arm64/boot/dts/renesas/r8a77995-draak.dts | 42 +++++++++++++++++++++++
>  1 file changed, 42 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts index 9d73de8..95745fc
> 100644
> --- a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> +++ b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> @@ -142,6 +142,11 @@
>  		groups = "usb0";
>  		function = "usb0";
>  	};
> +
> +	vin4_pins_cvbs: vin4 {
> +		groups = "vin4_data8", "vin4_sync", "vin4_clk";
> +		function = "vin4";
> +	};
>  };
> 
>  &i2c0 {
> @@ -154,6 +159,23 @@
>  		reg = <0x50>;
>  		pagesize = <8>;
>  	};
> +
> +	analog-video@20 {
> +		compatible = "adi,adv7180";
> +		reg = <0x20>;
> +
> +		port {

The adv7180 DT bindings document the output port as 3 or 6 (respectively for 
the CP and ST versions of the chip). You should thus number the port. Apart 
from that the patch looks good.

> +			/*
> +			 * The VIN4 video input path is shared between
> +			 * CVBS and HDMI inputs through SW[49-54] switches.
> +			 *
> +			 * CVBS is the default selection, link it to VIN4 here.
> +			 */
> +			adv7180_out: endpoint {
> +				remote-endpoint = <&vin4_in>;
> +			};
> +		};
> +	};
>  };
> 
>  &i2c1 {
> @@ -246,3 +268,23 @@
>  	timeout-sec = <60>;
>  	status = "okay";
>  };
> +
> +&vin4 {
> +	pinctrl-0 = <&vin4_pins_cvbs>;
> +	pinctrl-names = "default";
> +
> +	status = "okay";
> +
> +	ports {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		port@0 {
> +			reg = <0>;
> +
> +			vin4_in: endpoint {
> +				remote-endpoint = <&adv7180_out>;
> +			};
> +		};
> +	};
> +};

-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 3/3] arm64: dts: renesas: draak: Describe HDMI input

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Friday, 18 May 2018 17:47:58 EEST Jacopo Mondi wrote:
> Describe HDMI input connector and ADV7612 HDMI decoder installed on
> R-Car Gen3 Draak board.
> 
> The video signal routing to the HDMI decoder to the video input interface
> VIN4 is multiplexed with CVBS input path, and enabled/disabled through
> on-board switches SW-49, SW-50, SW-51 and SW-52.
> 
> As the default board switches configuration connects CVBS input to VIN4,
> leave the HDMI decoder unconnected in DTS.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
> v2 -> v3:
> - Add comment on HDMI output port about the shared CVBS/HDMI video path
> - Add Niklas' R-b tag
> ---
>  arch/arm64/boot/dts/renesas/r8a77995-draak.dts | 48 +++++++++++++++++++++++
>  1 file changed, 48 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts index 95745fc..1e475a4
> 100644
> --- a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> +++ b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> @@ -59,6 +59,17 @@
>  		};
>  	};
> 
> +	hdmi-in {
> +		compatible = "hdmi-connector";
> +		type = "a";
> +
> +		port {
> +			hdmi_con_in: endpoint {
> +				remote-endpoint = <&adv7612_in>;
> +			};
> +		};
> +	};
> +
>  	memory@48000000 {
>  		device_type = "memory";
>  		/* first 128MB is reserved for secure area. */
> @@ -176,6 +187,43 @@
>  			};
>  		};
>  	};
> +
> +	hdmi-decoder@4c {
> +		compatible = "adi,adv7612";
> +		reg = <0x4c>;
> +		default-input = <0>;
> +
> +		ports {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			port@0 {
> +				reg = <0>;
> +
> +				adv7612_in: endpoint {
> +					remote-endpoint = <&hdmi_con_in>;
> +				};
> +			};
> +
> +			port@2 {
> +				reg = <2>;
> +
> +				/*
> +				 * The VIN4 video input path is shared between
> +				 * CVBS and HDMI inputs through SW[49-54]
> +				 * switches.
> +				 *
> +				 * CVBS is the default selection, leave HDMI
> +				 * not connected here.
> +				 */
> +				adv7612_out: endpoint {
> +					pclk-sample = <0>;
> +					hsync-active = <0>;
> +					vsync-active = <0>;
> +				};
> +			};
> +		};
> +	};
>  };
> 
>  &i2c1 {

-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 2/3] arm64: dts: renesas: draak: Describe CVBS input

[kbuild test robot]

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

Hi Jacopo,

I love your patch! Yet something to improve:

[auto build test ERROR on linuxtv-media/master]
[cannot apply to renesas/next v4.17-rc6]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Jacopo-Mondi/arm64-dts-Draak-Enable-video-inputs-and-VIN4/20180521-052159
base:   git://linuxtv.org/media_tree.git master
config: arm64-defconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm64 

All errors (new ones prefixed by >>):

>> Error: arch/arm64/boot/dts/renesas/r8a77995-draak.dts:272.1-6 Label or path vin4 not found
>> FATAL ERROR: Syntax error parsing input tree

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 37440 bytes --]

Re: [PATCH v3 2/3] arm64: dts: renesas: draak: Describe CVBS input

[jacopo mondi]

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

Hi Laurent,

On Fri, May 18, 2018 at 06:12:15PM +0300, Laurent Pinchart wrote:
> Hi Jacopo,
>
> Thank you for the patch.
>
> On Friday, 18 May 2018 17:47:57 EEST Jacopo Mondi wrote:
> > Describe CVBS video input through analog video decoder ADV7180
> > connected to video input interface VIN4.
> >
> > The video input signal path is shared with HDMI video input, and
> > selected by on-board switches SW-53 and SW-54 with CVBS input selected
> > by the default switches configuration.
> >
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> >
> > ---
> > v2 -> v3:
> > - Add comment to describe the shared input video path
> > - Add my SoB and Niklas' R-b tags
> > ---
> >  arch/arm64/boot/dts/renesas/r8a77995-draak.dts | 42 +++++++++++++++++++++++
> >  1 file changed, 42 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> > b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts index 9d73de8..95745fc
> > 100644
> > --- a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> > +++ b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> > @@ -142,6 +142,11 @@
> >  		groups = "usb0";
> >  		function = "usb0";
> >  	};
> > +
> > +	vin4_pins_cvbs: vin4 {
> > +		groups = "vin4_data8", "vin4_sync", "vin4_clk";
> > +		function = "vin4";
> > +	};
> >  };
> >
> >  &i2c0 {
> > @@ -154,6 +159,23 @@
> >  		reg = <0x50>;
> >  		pagesize = <8>;
> >  	};
> > +
> > +	analog-video@20 {
> > +		compatible = "adi,adv7180";
> > +		reg = <0x20>;
> > +
> > +		port {
>
> The adv7180 DT bindings document the output port as 3 or 6 (respectively for
> the CP and ST versions of the chip). You should thus number the port. Apart
> from that the patch looks good.

I admit I have barely copied this from Gen-2 boards DTS, but reading
the driver code and binding description again, I think this is
correct, as the output port numbering and mandatory input port (which
is missing here) only apply to adv7180cp/st chip versions.

Here we describe plain adv7180, no need to number output ports afaict.

Thanks
  j

>
> > +			/*
> > +			 * The VIN4 video input path is shared between
> > +			 * CVBS and HDMI inputs through SW[49-54] switches.
> > +			 *
> > +			 * CVBS is the default selection, link it to VIN4 here.
> > +			 */
> > +			adv7180_out: endpoint {
> > +				remote-endpoint = <&vin4_in>;
> > +			};
> > +		};
> > +	};
> >  };
> >
> >  &i2c1 {
> > @@ -246,3 +268,23 @@
> >  	timeout-sec = <60>;
> >  	status = "okay";
> >  };
> > +
> > +&vin4 {
> > +	pinctrl-0 = <&vin4_pins_cvbs>;
> > +	pinctrl-names = "default";
> > +
> > +	status = "okay";
> > +
> > +	ports {
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +
> > +		port@0 {
> > +			reg = <0>;
> > +
> > +			vin4_in: endpoint {
> > +				remote-endpoint = <&adv7180_out>;
> > +			};
> > +		};
> > +	};
> > +};
>
> --
> Regards,
>
> Laurent Pinchart
>
>
>

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

Re: [PATCH v3 2/3] arm64: dts: renesas: draak: Describe CVBS input

[Laurent Pinchart]

Hi Jacopo,

On Monday, 21 May 2018 12:57:05 EEST jacopo mondi wrote:
> On Fri, May 18, 2018 at 06:12:15PM +0300, Laurent Pinchart wrote:
> > On Friday, 18 May 2018 17:47:57 EEST Jacopo Mondi wrote:
> >> Describe CVBS video input through analog video decoder ADV7180
> >> connected to video input interface VIN4.
> >> 
> >> The video input signal path is shared with HDMI video input, and
> >> selected by on-board switches SW-53 and SW-54 with CVBS input selected
> >> by the default switches configuration.
> >> 
> >> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> >> Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> >> 
> >> ---
> >> v2 -> v3:
> >> - Add comment to describe the shared input video path
> >> - Add my SoB and Niklas' R-b tags
> >> ---
> >> 
> >>  arch/arm64/boot/dts/renesas/r8a77995-draak.dts | 42 +++++++++++++++++++
> >>  1 file changed, 42 insertions(+)
> >> 
> >> diff --git a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> >> b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts index 9d73de8..95745fc
> >> 100644
> >> --- a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> >> +++ b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> >> @@ -142,6 +142,11 @@
> >>  		groups = "usb0";
> >>  		function = "usb0";
> >>  	};
> >> +
> >> +	vin4_pins_cvbs: vin4 {
> >> +		groups = "vin4_data8", "vin4_sync", "vin4_clk";
> >> +		function = "vin4";
> >> +	};
> >>  };
> >>  
> >>  &i2c0 {
> >> @@ -154,6 +159,23 @@
> >>  		reg = <0x50>;
> >>  		pagesize = <8>;
> >>  	};
> >> +
> >> +	analog-video@20 {
> >> +		compatible = "adi,adv7180";
> >> +		reg = <0x20>;
> >> +
> >> +		port {
> > 
> > The adv7180 DT bindings document the output port as 3 or 6 (respectively
> > for the CP and ST versions of the chip). You should thus number the port.
> > Apart from that the patch looks good.
> 
> I admit I have barely copied this from Gen-2 boards DTS, but reading
> the driver code and binding description again, I think this is
> correct, as the output port numbering and mandatory input port (which
> is missing here) only apply to adv7180cp/st chip versions.
> 
> Here we describe plain adv7180, no need to number output ports afaict.

Indeed, my bad.

Shouldn't you however use "adi,adv7180cp" as that's the chip we are using ? 
The "adi,adv7180" has no port documented in its DT bindings, so it shouldn't 
have any port node at all.

> >> +			/*
> >> +			 * The VIN4 video input path is shared between
> >> +			 * CVBS and HDMI inputs through SW[49-54] switches.
> >> +			 *
> >> +			 * CVBS is the default selection, link it to VIN4 here.
> >> +			 */
> >> +			adv7180_out: endpoint {
> >> +				remote-endpoint = <&vin4_in>;
> >> +			};
> >> +		};
> >> +	};
> >>  };
> >>  
> >>  &i2c1 {
> >> @@ -246,3 +268,23 @@
> >>  	timeout-sec = <60>;
> >>  	status = "okay";
> >>  };
> >> +
> >> +&vin4 {
> >> +	pinctrl-0 = <&vin4_pins_cvbs>;
> >> +	pinctrl-names = "default";
> >> +
> >> +	status = "okay";
> >> +
> >> +	ports {
> >> +		#address-cells = <1>;
> >> +		#size-cells = <0>;
> >> +
> >> +		port@0 {
> >> +			reg = <0>;
> >> +
> >> +			vin4_in: endpoint {
> >> +				remote-endpoint = <&adv7180_out>;
> >> +			};
> >> +		};
> >> +	};
> >> +};

-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 2/3] arm64: dts: renesas: draak: Describe CVBS input

[jacopo mondi]

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

Hi Laurent,

On Mon, May 21, 2018 at 01:54:55PM +0300, Laurent Pinchart wrote:
> Hi Jacopo,
>
> On Monday, 21 May 2018 12:57:05 EEST jacopo mondi wrote:
> > On Fri, May 18, 2018 at 06:12:15PM +0300, Laurent Pinchart wrote:
> > > On Friday, 18 May 2018 17:47:57 EEST Jacopo Mondi wrote:
> > >> Describe CVBS video input through analog video decoder ADV7180
> > >> connected to video input interface VIN4.
> > >>
> > >> The video input signal path is shared with HDMI video input, and
> > >> selected by on-board switches SW-53 and SW-54 with CVBS input selected
> > >> by the default switches configuration.
> > >>
> > >> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > >> Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > >>
> > >> ---
> > >> v2 -> v3:
> > >> - Add comment to describe the shared input video path
> > >> - Add my SoB and Niklas' R-b tags
> > >> ---
> > >>
> > >>  arch/arm64/boot/dts/renesas/r8a77995-draak.dts | 42 +++++++++++++++++++
> > >>  1 file changed, 42 insertions(+)
> > >>
> > >> diff --git a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> > >> b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts index 9d73de8..95745fc
> > >> 100644
> > >> --- a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> > >> +++ b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> > >> @@ -142,6 +142,11 @@
> > >>  		groups = "usb0";
> > >>  		function = "usb0";
> > >>  	};
> > >> +
> > >> +	vin4_pins_cvbs: vin4 {
> > >> +		groups = "vin4_data8", "vin4_sync", "vin4_clk";
> > >> +		function = "vin4";
> > >> +	};
> > >>  };
> > >>
> > >>  &i2c0 {
> > >> @@ -154,6 +159,23 @@
> > >>  		reg = <0x50>;
> > >>  		pagesize = <8>;
> > >>  	};
> > >> +
> > >> +	analog-video@20 {
> > >> +		compatible = "adi,adv7180";
> > >> +		reg = <0x20>;
> > >> +
> > >> +		port {
> > >
> > > The adv7180 DT bindings document the output port as 3 or 6 (respectively
> > > for the CP and ST versions of the chip). You should thus number the port.
> > > Apart from that the patch looks good.
> >
> > I admit I have barely copied this from Gen-2 boards DTS, but reading
> > the driver code and binding description again, I think this is
> > correct, as the output port numbering and mandatory input port (which
> > is missing here) only apply to adv7180cp/st chip versions.
> >
> > Here we describe plain adv7180, no need to number output ports afaict.
>
> Indeed, my bad.
>
> Shouldn't you however use "adi,adv7180cp" as that's the chip we are using ?
> The "adi,adv7180" has no port documented in its DT bindings, so it shouldn't
> have any port node at all.

I'm a bit confused here.

The only Gen-2 board using the "adi,adv7180cp" compatible string is
Gose, which is also the only one I can get schematics for. That board
indeed feature an ADV7180WBCP32Z, as the Draak does. I cannot get
schematics for any other Gen-2 board, to compare the ADV7180 variant
installed there. If anyone can confirm that all other Gen-2 board uses
a different version (or that all other Gen-2 board DTS file use a
wrong compatible string value), I'll re-send this with a different
compatible value and proper port nodes numbering.

Thanks
  j
>
> > >> +			/*
> > >> +			 * The VIN4 video input path is shared between
> > >> +			 * CVBS and HDMI inputs through SW[49-54] switches.
> > >> +			 *
> > >> +			 * CVBS is the default selection, link it to VIN4 here.
> > >> +			 */
> > >> +			adv7180_out: endpoint {
> > >> +				remote-endpoint = <&vin4_in>;
> > >> +			};
> > >> +		};
> > >> +	};
> > >>  };
> > >>
> > >>  &i2c1 {
> > >> @@ -246,3 +268,23 @@
> > >>  	timeout-sec = <60>;
> > >>  	status = "okay";
> > >>  };
> > >> +
> > >> +&vin4 {
> > >> +	pinctrl-0 = <&vin4_pins_cvbs>;
> > >> +	pinctrl-names = "default";
> > >> +
> > >> +	status = "okay";
> > >> +
> > >> +	ports {
> > >> +		#address-cells = <1>;
> > >> +		#size-cells = <0>;
> > >> +
> > >> +		port@0 {
> > >> +			reg = <0>;
> > >> +
> > >> +			vin4_in: endpoint {
> > >> +				remote-endpoint = <&adv7180_out>;
> > >> +			};
> > >> +		};
> > >> +	};
> > >> +};
>
> --
> Regards,
>
> Laurent Pinchart
>
>
>

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

Re: [PATCH v3 2/3] arm64: dts: renesas: draak: Describe CVBS input

[Laurent Pinchart]

Hi Jacopo,

On Monday, 21 May 2018 15:33:40 EEST jacopo mondi wrote:
> On Mon, May 21, 2018 at 01:54:55PM +0300, Laurent Pinchart wrote:
> > On Monday, 21 May 2018 12:57:05 EEST jacopo mondi wrote:
> >> On Fri, May 18, 2018 at 06:12:15PM +0300, Laurent Pinchart wrote:
> >>> On Friday, 18 May 2018 17:47:57 EEST Jacopo Mondi wrote:
> >>>> Describe CVBS video input through analog video decoder ADV7180
> >>>> connected to video input interface VIN4.
> >>>> 
> >>>> The video input signal path is shared with HDMI video input, and
> >>>> selected by on-board switches SW-53 and SW-54 with CVBS input
> >>>> selected by the default switches configuration.
> >>>> 
> >>>> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> >>>> Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> >>>> 
> >>>> ---
> >>>> v2 -> v3:
> >>>> - Add comment to describe the shared input video path
> >>>> - Add my SoB and Niklas' R-b tags
> >>>> ---
> >>>> 
> >>>>  arch/arm64/boot/dts/renesas/r8a77995-draak.dts | 42 ++++++++++++++++++
> >>>>  1 file changed, 42 insertions(+)
> >>>> 
> >>>> diff --git a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> >>>> b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts index
> >>>> 9d73de8..95745fc
> >>>> 100644
> >>>> --- a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> >>>> +++ b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> >>>> @@ -142,6 +142,11 @@
> >>>>  		groups = "usb0";
> >>>>  		function = "usb0";
> >>>>  	};
> >>>> +
> >>>> +	vin4_pins_cvbs: vin4 {
> >>>> +		groups = "vin4_data8", "vin4_sync", "vin4_clk";
> >>>> +		function = "vin4";
> >>>> +	};
> >>>>  };
> >>>>  
> >>>>  &i2c0 {
> >>>> @@ -154,6 +159,23 @@
> >>>>  		reg = <0x50>;
> >>>>  		pagesize = <8>;
> >>>>  	};
> >>>> +
> >>>> +	analog-video@20 {
> >>>> +		compatible = "adi,adv7180";
> >>>> +		reg = <0x20>;
> >>>> +
> >>>> +		port {
> >>> 
> >>> The adv7180 DT bindings document the output port as 3 or 6
> >>> (respectively for the CP and ST versions of the chip). You should thus
> >>> number the port. Apart from that the patch looks good.
> >> 
> >> I admit I have barely copied this from Gen-2 boards DTS, but reading
> >> the driver code and binding description again, I think this is
> >> correct, as the output port numbering and mandatory input port (which
> >> is missing here) only apply to adv7180cp/st chip versions.
> >> 
> >> Here we describe plain adv7180, no need to number output ports afaict.
> > 
> > Indeed, my bad.
> > 
> > Shouldn't you however use "adi,adv7180cp" as that's the chip we are using
> > ?
> > The "adi,adv7180" has no port documented in its DT bindings, so it
> > shouldn't have any port node at all.
> 
> I'm a bit confused here.
> 
> The only Gen-2 board using the "adi,adv7180cp" compatible string is
> Gose, which is also the only one I can get schematics for. That board
> indeed feature an ADV7180WBCP32Z, as the Draak does. I cannot get
> schematics for any other Gen-2 board, to compare the ADV7180 variant
> installed there. If anyone can confirm that all other Gen-2 board uses
> a different version (or that all other Gen-2 board DTS file use a
> wrong compatible string value), I'll re-send this with a different
> compatible value and proper port nodes numbering.

Other Gen2 boards use a ADV7180WBCP32Z as well. The issue here isn't that the 
chip you're trying to support is different. The DT bindings that were 
initially written for the adi,adv7180 didn't have port nodes. When it was time 
to add them, we realized that two variants of the chip existed with different 
connectivity. We have thus added two new compatible strings to differentiate 
them, with different port numbers. The old compatible string should be 
deprecated in favour of the new ones.

> >>>> +			/*
> >>>> +			 * The VIN4 video input path is shared between
> >>>> +			 * CVBS and HDMI inputs through SW[49-54] switches.
> >>>> +			 *
> >>>> +			 * CVBS is the default selection, link it to VIN4 here.
> >>>> +			 */
> >>>> +			adv7180_out: endpoint {
> >>>> +				remote-endpoint = <&vin4_in>;
> >>>> +			};
> >>>> +		};
> >>>> +	};
> >>>>  };
> >>>>  
> >>>>  &i2c1 {
> >>>> @@ -246,3 +268,23 @@
> >>>>  	timeout-sec = <60>;
> >>>>  	status = "okay";
> >>>>  };
 >>>> +
> >>>> +&vin4 {
> >>>> +	pinctrl-0 = <&vin4_pins_cvbs>;
> >>>> +	pinctrl-names = "default";
> >>>> +
> >>>> +	status = "okay";
> >>>> +
> >>>> +	ports {
> >>>> +		#address-cells = <1>;
> >>>> +		#size-cells = <0>;
> >>>> +
> >>>> +		port@0 {
> >>>> +			reg = <0>;
> >>>> +
> >>>> +			vin4_in: endpoint {
> >>>> +				remote-endpoint = <&adv7180_out>;
> >>>> +			};
> >>>> +		};
> >>>> +	};
> >>>> +};

-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 2/3] arm64: dts: renesas: draak: Describe CVBS input

[jacopo mondi]

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

HI Laurent,

On Mon, May 21, 2018 at 04:10:34PM +0300, Laurent Pinchart wrote:
> Hi Jacopo,
>
> On Monday, 21 May 2018 15:33:40 EEST jacopo mondi wrote:
> > On Mon, May 21, 2018 at 01:54:55PM +0300, Laurent Pinchart wrote:
> > > On Monday, 21 May 2018 12:57:05 EEST jacopo mondi wrote:
> > >> On Fri, May 18, 2018 at 06:12:15PM +0300, Laurent Pinchart wrote:
> > >>> On Friday, 18 May 2018 17:47:57 EEST Jacopo Mondi wrote:
> > >>>> Describe CVBS video input through analog video decoder ADV7180
> > >>>> connected to video input interface VIN4.
> > >>>>
> > >>>> The video input signal path is shared with HDMI video input, and
> > >>>> selected by on-board switches SW-53 and SW-54 with CVBS input
> > >>>> selected by the default switches configuration.
> > >>>>
> > >>>> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > >>>> Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > >>>>
> > >>>> ---
> > >>>> v2 -> v3:
> > >>>> - Add comment to describe the shared input video path
> > >>>> - Add my SoB and Niklas' R-b tags
> > >>>> ---
> > >>>>
> > >>>>  arch/arm64/boot/dts/renesas/r8a77995-draak.dts | 42 ++++++++++++++++++
> > >>>>  1 file changed, 42 insertions(+)
> > >>>>
> > >>>> diff --git a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> > >>>> b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts index
> > >>>> 9d73de8..95745fc
> > >>>> 100644
> > >>>> --- a/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> > >>>> +++ b/arch/arm64/boot/dts/renesas/r8a77995-draak.dts
> > >>>> @@ -142,6 +142,11 @@
> > >>>>  		groups = "usb0";
> > >>>>  		function = "usb0";
> > >>>>  	};
> > >>>> +
> > >>>> +	vin4_pins_cvbs: vin4 {
> > >>>> +		groups = "vin4_data8", "vin4_sync", "vin4_clk";
> > >>>> +		function = "vin4";
> > >>>> +	};
> > >>>>  };
> > >>>>
> > >>>>  &i2c0 {
> > >>>> @@ -154,6 +159,23 @@
> > >>>>  		reg = <0x50>;
> > >>>>  		pagesize = <8>;
> > >>>>  	};
> > >>>> +
> > >>>> +	analog-video@20 {
> > >>>> +		compatible = "adi,adv7180";
> > >>>> +		reg = <0x20>;
> > >>>> +
> > >>>> +		port {
> > >>>
> > >>> The adv7180 DT bindings document the output port as 3 or 6
> > >>> (respectively for the CP and ST versions of the chip). You should thus
> > >>> number the port. Apart from that the patch looks good.
> > >>
> > >> I admit I have barely copied this from Gen-2 boards DTS, but reading
> > >> the driver code and binding description again, I think this is
> > >> correct, as the output port numbering and mandatory input port (which
> > >> is missing here) only apply to adv7180cp/st chip versions.
> > >>
> > >> Here we describe plain adv7180, no need to number output ports afaict.
> > >
> > > Indeed, my bad.
> > >
> > > Shouldn't you however use "adi,adv7180cp" as that's the chip we are using
> > > ?
> > > The "adi,adv7180" has no port documented in its DT bindings, so it
> > > shouldn't have any port node at all.
> >
> > I'm a bit confused here.
> >
> > The only Gen-2 board using the "adi,adv7180cp" compatible string is
> > Gose, which is also the only one I can get schematics for. That board
> > indeed feature an ADV7180WBCP32Z, as the Draak does. I cannot get
> > schematics for any other Gen-2 board, to compare the ADV7180 variant
> > installed there. If anyone can confirm that all other Gen-2 board uses
> > a different version (or that all other Gen-2 board DTS file use a
> > wrong compatible string value), I'll re-send this with a different
> > compatible value and proper port nodes numbering.
>
> Other Gen2 boards use a ADV7180WBCP32Z as well. The issue here isn't that the
> chip you're trying to support is different. The DT bindings that were
> initially written for the adi,adv7180 didn't have port nodes. When it was time
> to add them, we realized that two variants of the chip existed with different
> connectivity. We have thus added two new compatible strings to differentiate
> them, with different port numbers. The old compatible string should be
> deprecated in favour of the new ones.
>

Ack, thanks for explaining.

I don't have any Gen2 board here, but if someone is willing to test, I
can change the Gen2 bindings to use the new version in a follow-up
series.

Thanks
   j

> > >>>> +			/*
> > >>>> +			 * The VIN4 video input path is shared between
> > >>>> +			 * CVBS and HDMI inputs through SW[49-54] switches.
> > >>>> +			 *
> > >>>> +			 * CVBS is the default selection, link it to VIN4 here.
> > >>>> +			 */
> > >>>> +			adv7180_out: endpoint {
> > >>>> +				remote-endpoint = <&vin4_in>;
> > >>>> +			};
> > >>>> +		};
> > >>>> +	};
> > >>>>  };
> > >>>>
> > >>>>  &i2c1 {
> > >>>> @@ -246,3 +268,23 @@
> > >>>>  	timeout-sec = <60>;
> > >>>>  	status = "okay";
> > >>>>  };
>  >>>> +
> > >>>> +&vin4 {
> > >>>> +	pinctrl-0 = <&vin4_pins_cvbs>;
> > >>>> +	pinctrl-names = "default";
> > >>>> +
> > >>>> +	status = "okay";
> > >>>> +
> > >>>> +	ports {
> > >>>> +		#address-cells = <1>;
> > >>>> +		#size-cells = <0>;
> > >>>> +
> > >>>> +		port@0 {
> > >>>> +			reg = <0>;
> > >>>> +
> > >>>> +			vin4_in: endpoint {
> > >>>> +				remote-endpoint = <&adv7180_out>;
> > >>>> +			};
> > >>>> +		};
> > >>>> +	};
> > >>>> +};
>
> --
> Regards,
>
> Laurent Pinchart
>
>
>

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