[PATCH v6 0/7] Add USB3.0 and TI HD3SS3220 driver support

[PATCH v6 0/7] Add USB3.0 and TI HD3SS3220 driver support

[Biju Das]

This series adds USB 3.0 support for the CAT874 platform, including a
new driver for the TI HD3SS3220 USB Type-C DRP port controller.

This patch series supports:
1) Host hotplug operation
2) Device hot plug operation
3) USB type-C data_role switch
   (Tested with 2 RZ/G2E boards connected with a Type-C cable)

This patchset is based on linux_next next-20190514 branch.
V5-->V6
  * Updated bindings patch by using usb-role-switch property
  * Used graph api's to get role switch supported by the usb-c-connector
V4-->V5
  * Incorporated Heikki's review comment.
    (https://patchwork.kernel.org/patch/10902531/)
  * Patch 1 is a dependency patch to make the compilation successful.
    (https://patchwork.kernel.org/patch/10909971/)
  * Patch 2 is also dependency patch to make the compilation successful.
    Rebased on top of Patch1 and fixed Heikki's review comments
    (https://patchwork.kernel.org/patch/10882555/)
  * Incorporated Shimoda-San's review comment.
    (https://patchwork.kernel.org/patch/10902535/)
V3-->V4
  * Incorporated Chunfeng Yun's review comment
    (https://patchwork.kernel.org/project/linux-usb/list/?submitter=133171)
  * Used fwnode API's to get roleswitch handle

V2-->V3
  * Used the new API to usb_role_switch by node to find the remote endpoint
    (https://patchwork.kernel.org/patch/10882555/)
  * Added renesas,usb-role-switch property
  * Incorporated shimoda-san's review comment
    (https://patchwork.kernel.org/patch/10852507/)

V1-->V2
  * Use USB role class instead of extcon to receive connect and disconnect
    events and also for the dual role switch.
  * Dropped patch 6
  * Squashed patch 8 and patch 9
  * https://patchwork.kernel.org/cover/10840641/


Biju Das (7):
  dt-bindings: usb: hd3ss3220 device tree binding document
  dt-bindings: usb: renesas_usb3: Document usb role switch support
  usb: typec: driver for TI HD3SS3220 USB Type-C DRP port controller
  usb: gadget: udc: renesas_usb3: Add dual role switch support
  arm64: defconfig: enable TYPEC_HD3SS3220 config option
  arm64: dts: renesas: r8a774c0-cat874: Enable USB3.0 host/peripheral
    device node
  arm64: dts: renesas: r8a774c0-cat874: Enable usb role switch support

 .../devicetree/bindings/usb/renesas_usb3.txt       |  26 ++
 .../devicetree/bindings/usb/ti,hd3ss3220.txt       |  37 +++
 arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts    |  56 +++++
 arch/arm64/configs/defconfig                       |   2 +
 drivers/usb/gadget/udc/renesas_usb3.c              | 121 +++++++++-
 drivers/usb/typec/Kconfig                          |  10 +
 drivers/usb/typec/Makefile                         |   1 +
 drivers/usb/typec/hd3ss3220.c                      | 263 +++++++++++++++++++++
 8 files changed, 509 insertions(+), 7 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt
 create mode 100644 drivers/usb/typec/hd3ss3220.c

-- 
2.7.4

[PATCH v6 1/7] dt-bindings: usb: hd3ss3220 device tree binding document

[Biju Das]

Add device tree binding document for TI HD3SS3220 Type-C DRP port
controller driver.

Signed-off-by: Biju Das <biju.das@bp.renesas.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
V5-->V6
  * No change.
V4-->V5
  * No Change.
V3-->V4
  * No Change.
V2-->V3
  * Added Rob's Reviewed by tag.
V1-->V2
  * Added connector node.
  * updated the example with connector node.
---
 .../devicetree/bindings/usb/ti,hd3ss3220.txt       | 37 ++++++++++++++++++++++
 1 file changed, 37 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt

diff --git a/Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt b/Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt
new file mode 100644
index 0000000..7f41400
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt
@@ -0,0 +1,37 @@
+TI HD3SS3220 TypeC DRP Port Controller.
+
+Required properties:
+ - compatible: Must be "ti,hd3ss3220".
+ - reg: I2C slave address, must be 0x47 or 0x67 based on ADDR pin.
+ - interrupts: <a b> where a is the interrupt number and b represents an
+   encoding of the sense and level information for the interrupt.
+
+Required sub-node:
+ - connector : The "usb-c-connector" attached to the hd3ss3220 chip. The
+   bindings of the connector node are specified in:
+
+	Documentation/devicetree/bindings/connector/usb-connector.txt
+
+Example:
+hd3ss3220@47 {
+	compatible = "ti,hd3ss3220";
+	reg = <0x47>;
+	interrupt-parent = <&gpio6>;
+	interrupts = <3 IRQ_TYPE_LEVEL_LOW>;
+
+	usb_con: connector {
+		compatible = "usb-c-connector";
+		label = "USB-C";
+		data-role = "dual";
+	};
+
+	port {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		hd3ss3220_ep: endpoint@0 {
+			reg = <0>;
+			remote-endpoint = <&usb3peri_role_switch>;
+		};
+	};
+};
-- 
2.7.4

[PATCH v6 2/7] dt-bindings: usb: renesas_usb3: Document usb role switch support

[Biju Das]

Update the DT bindings documentation to support usb role switch
for USB Type-C connector using USB role switch class framework.

Signed-off-by: Biju Das <biju.das@bp.renesas.com>
---
 V5-->V6
  * Updated description
  * Added usb-role-switch-property
 V4-->V5
  * No Change
 V3-->V4
  * No Change
 V2-->V3
  * Added optional renesas,usb-role-switch property.
 V1-->V2
  * Added usb-role-switch-property
  * Updated the example with usb-role-switch property.
---
 .../devicetree/bindings/usb/renesas_usb3.txt       | 26 ++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/renesas_usb3.txt b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
index 35039e7..ea6c63c 100644
--- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
+++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
@@ -22,6 +22,11 @@ Required properties:
 Optional properties:
   - phys: phandle + phy specifier pair
   - phy-names: must be "usb"
+  - usb-role-switch: support role switch. see usb/generic.txt
+
+Sub-nodes:
+The port would be added as a subnode if the "usb-role-switch" property is used.
+	see graph.txt
 
 Example of R-Car H3 ES1.x:
 	usb3_peri0: usb@ee020000 {
@@ -39,3 +44,24 @@ Example of R-Car H3 ES1.x:
 		interrupts = <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&cpg CPG_MOD 327>;
 	};
+
+Example of RZ/G2E:
+	usb3_peri0: usb@ee020000 {
+		compatible = "renesas,r8a774c0-usb3-peri",
+			     "renesas,rcar-gen3-usb3-peri";
+		reg = <0 0xee020000 0 0x400>;
+		interrupts = <GIC_SPI 104 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&cpg CPG_MOD 328>;
+		companion = <&xhci0>;
+		usb-role-switch;
+
+		port {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			usb3peri_role_switch: endpoint@0 {
+				reg = <0>;
+				remote-endpoint = <&hd3ss3220_ep>;
+			};
+		};
+	};
-- 
2.7.4

[PATCH v6 6/7] arm64: dts: renesas: r8a774c0-cat874: Enable USB3.0 host/peripheral device node

[Biju Das]

This patch enables USB3.0 host/peripheral device node for r8a774c0
cat874 board.

Signed-off-by: Biju Das <biju.das@bp.renesas.com>
---
V5-->V6
  * No change
V4-->V5
  * No change
V3-->V4
  * No change
V2-->V3
  * No change
V1-->V2
  * No change
---
 arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts b/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts
index 013a48c..b9ae7db 100644
--- a/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts
+++ b/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts
@@ -134,6 +134,11 @@
 		function = "sdhi0";
 		power-source = <1800>;
 	};
+
+	usb30_pins: usb30 {
+		groups = "usb30", "usb30_id";
+		function = "usb30";
+	};
 };
 
 &rwdt {
@@ -166,3 +171,15 @@
 	renesas,no-otg-pins;
 	status = "okay";
 };
+
+&usb3_peri0 {
+	companion = <&xhci0>;
+	status = "okay";
+};
+
+&xhci0 {
+	pinctrl-0 = <&usb30_pins>;
+	pinctrl-names = "default";
+
+	status = "okay";
+};
-- 
2.7.4

[PATCH v6 7/7] arm64: dts: renesas: r8a774c0-cat874: Enable usb role switch support

[Biju Das]

This patch enables TI HD3SS3220 device and support usb role switch
for the CAT 874 platform.

Signed-off-by: Biju Das <biju.das@bp.renesas.com>
---
V5-->V6
  * No change
V4-->V5
  * No change
V3-->V4
  * No change
V2-->V3
  * Used "renesas,usb-role-switch" instead of generic "usb-role-switch"
    property
V1-->V2
  * New patch
---
 arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts | 39 +++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts b/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts
index b9ae7db..124ed58 100644
--- a/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts
+++ b/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts
@@ -85,6 +85,34 @@
 	clock-frequency = <48000000>;
 };
 
+&i2c0 {
+	status = "okay";
+	clock-frequency = <100000>;
+
+	hd3ss3220@47 {
+		compatible = "ti,hd3ss3220";
+		reg = <0x47>;
+		interrupt-parent = <&gpio6>;
+		interrupts = <3 IRQ_TYPE_LEVEL_LOW>;
+
+		usb_con: connector {
+			compatible = "usb-c-connector";
+			label = "USB-C";
+			data-role = "dual";
+		};
+
+		port {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			hd3ss3220_ep: endpoint@0 {
+				reg = <0>;
+				remote-endpoint = <&usb3peri_role_switch>;
+			};
+		};
+	};
+};
+
 &i2c1 {
 	pinctrl-0 = <&i2c1_pins>;
 	pinctrl-names = "default";
@@ -175,6 +203,17 @@
 &usb3_peri0 {
 	companion = <&xhci0>;
 	status = "okay";
+	usb-role-switch;
+
+	port {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		usb3peri_role_switch: endpoint@0 {
+			reg = <0>;
+			remote-endpoint = <&hd3ss3220_ep>;
+		};
+	};
 };
 
 &xhci0 {
-- 
2.7.4

Re: [PATCH v6 1/7] dt-bindings: usb: hd3ss3220 device tree binding document

[Sergei Shtylyov]

Hello!

On 05/15/2019 03:09 PM, Biju Das wrote:

> Add device tree binding document for TI HD3SS3220 Type-C DRP port
> controller driver.
> 
> Signed-off-by: Biju Das <biju.das@bp.renesas.com>
> Reviewed-by: Rob Herring <robh@kernel.org>
> ---
> V5-->V6
>   * No change.
> V4-->V5
>   * No Change.
> V3-->V4
>   * No Change.
> V2-->V3
>   * Added Rob's Reviewed by tag.
> V1-->V2
>   * Added connector node.
>   * updated the example with connector node.
> ---
>  .../devicetree/bindings/usb/ti,hd3ss3220.txt       | 37 ++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt
> 
> diff --git a/Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt b/Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt
> new file mode 100644
> index 0000000..7f41400
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt
> @@ -0,0 +1,37 @@
> +TI HD3SS3220 TypeC DRP Port Controller.
> +
> +Required properties:
> + - compatible: Must be "ti,hd3ss3220".
> + - reg: I2C slave address, must be 0x47 or 0x67 based on ADDR pin.
> + - interrupts: <a b> where a is the interrupt number and b represents an
> +   encoding of the sense and level information for the interrupt.

   This depends on an interrupt controller used. I'd just said "an interrupt
specifier", w/o further details.

> +
> +Required sub-node:
> + - connector : The "usb-c-connector" attached to the hd3ss3220 chip. The
> +   bindings of the connector node are specified in:
> +
> +	Documentation/devicetree/bindings/connector/usb-connector.txt
> +
[...]

MBR, Sergei

RE: [PATCH v6 1/7] dt-bindings: usb: hd3ss3220 device tree binding document

[Yoshihiro Shimoda]

Hi Biju-san,

Thank you for the patch!

> From: Biju Das, Sent: Wednesday, May 15, 2019 9:09 PM
> 
> Add device tree binding document for TI HD3SS3220 Type-C DRP port
> controller driver.
> 
> Signed-off-by: Biju Das <biju.das@bp.renesas.com>
> Reviewed-by: Rob Herring <robh@kernel.org>
> ---
> V5-->V6
>   * No change.
> V4-->V5
>   * No Change.
> V3-->V4
>   * No Change.
> V2-->V3
>   * Added Rob's Reviewed by tag.
> V1-->V2
>   * Added connector node.
>   * updated the example with connector node.
> ---
>  .../devicetree/bindings/usb/ti,hd3ss3220.txt       | 37 ++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt
> 
> diff --git a/Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt
> b/Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt
> new file mode 100644
> index 0000000..7f41400
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt
> @@ -0,0 +1,37 @@
> +TI HD3SS3220 TypeC DRP Port Controller.
> +
> +Required properties:
> + - compatible: Must be "ti,hd3ss3220".
> + - reg: I2C slave address, must be 0x47 or 0x67 based on ADDR pin.
> + - interrupts: <a b> where a is the interrupt number and b represents an
> +   encoding of the sense and level information for the interrupt.
> +
> +Required sub-node:
> + - connector : The "usb-c-connector" attached to the hd3ss3220 chip. The

s/connector :/connector:/

> +   bindings of the connector node are specified in:
> +
> +	Documentation/devicetree/bindings/connector/usb-connector.txt
> +
> +Example:
> +hd3ss3220@47 {
> +	compatible = "ti,hd3ss3220";
> +	reg = <0x47>;
> +	interrupt-parent = <&gpio6>;
> +	interrupts = <3 IRQ_TYPE_LEVEL_LOW>;
> +
> +	usb_con: connector {
> +		compatible = "usb-c-connector";
> +		label = "USB-C";
> +		data-role = "dual";
> +	};
> +
> +	port {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		hd3ss3220_ep: endpoint@0 {
> +			reg = <0>;
> +			remote-endpoint = <&usb3peri_role_switch>;
> +		};
> +	};

According to the connector/usb-connector.txt, should the connector node
have ports, port@1 and an endpoint nodes like below?

	usb_con: connector {
		compatible = "usb-c-connector";
		label = "USB-C";
		data-role = "dual";

		ports {
			#address-cells = <1>;
			#size-cells = <0>;

			port@1 {
				reg = <1>;
				hd3ss3220_ep: endpoint {
					remote-endpoint = <&usb3peri_role_switch>;
				};
			};
		};
	};

Best regards,
Yoshihiro Shimoda

Re: [PATCH v6 1/7] dt-bindings: usb: hd3ss3220 device tree binding document

[Kuninori Morimoto]

Hi

> > +Required properties:
> > + - compatible: Must be "ti,hd3ss3220".
> > + - reg: I2C slave address, must be 0x47 or 0x67 based on ADDR pin.
> > + - interrupts: <a b> where a is the interrupt number and b represents an
> > +   encoding of the sense and level information for the interrupt.
> > +
> > +Required sub-node:
> > + - connector : The "usb-c-connector" attached to the hd3ss3220 chip. The
> 
> s/connector :/connector:/

Maybe it is for alignment ?

> According to the connector/usb-connector.txt, should the connector node
> have ports, port@1 and an endpoint nodes like below?

"ports" is needed if it has multiple "port",
otherwise, single port is allowed from OF-graph point of view.

Thank you for your help !!
Best regards
---
Kuninori Morimoto

RE: [PATCH v6 2/7] dt-bindings: usb: renesas_usb3: Document usb role switch support

[Yoshihiro Shimoda]

Hi Biju-san,

Thank you for the patch!

> From: Biju Das, Sent: Wednesday, May 15, 2019 9:09 PM
> 
> Update the DT bindings documentation to support usb role switch
> for USB Type-C connector using USB role switch class framework.
> 
> Signed-off-by: Biju Das <biju.das@bp.renesas.com>
> ---
>  V5-->V6
>   * Updated description
>   * Added usb-role-switch-property
>  V4-->V5
>   * No Change
>  V3-->V4
>   * No Change
>  V2-->V3
>   * Added optional renesas,usb-role-switch property.
>  V1-->V2
>   * Added usb-role-switch-property
>   * Updated the example with usb-role-switch property.
> ---
>  .../devicetree/bindings/usb/renesas_usb3.txt       | 26 ++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> index 35039e7..ea6c63c 100644
> --- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> +++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> @@ -22,6 +22,11 @@ Required properties:
>  Optional properties:
>    - phys: phandle + phy specifier pair
>    - phy-names: must be "usb"
> +  - usb-role-switch: support role switch. see usb/generic.txt
> +
> +Sub-nodes:
> +The port would be added as a subnode if the "usb-role-switch" property is used.
> +	see graph.txt

I think we should describe which type of a subnode is needed.
I made an example below. This is based on the usb-connector.txt.
---
Sub-nodes:
- any connector to the data bus of this controller should be modeled using the OF graph
  bindings specified in bindings/graph.txt.
---

>  Example of R-Car H3 ES1.x:
>  	usb3_peri0: usb@ee020000 {
> @@ -39,3 +44,24 @@ Example of R-Car H3 ES1.x:
>  		interrupts = <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>;
>  		clocks = <&cpg CPG_MOD 327>;
>  	};
> +
> +Example of RZ/G2E:
> +	usb3_peri0: usb@ee020000 {
> +		compatible = "renesas,r8a774c0-usb3-peri",
> +			     "renesas,rcar-gen3-usb3-peri";
> +		reg = <0 0xee020000 0 0x400>;
> +		interrupts = <GIC_SPI 104 IRQ_TYPE_LEVEL_HIGH>;
> +		clocks = <&cpg CPG_MOD 328>;
> +		companion = <&xhci0>;
> +		usb-role-switch;
> +
> +		port {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			usb3peri_role_switch: endpoint@0 {
> +				reg = <0>;

I'm not sure, but I don't think this endpoint@0 and reg = <0> are needed.
In other words, can we use have following node?
			usb3peri_role_switch: endpoint {
				remote-endpoint = <&hd3ss3220_ep>;
			};

Best regards,
Yoshihiro Shimoda

RE: [PATCH v6 1/7] dt-bindings: usb: hd3ss3220 device tree binding document

[Biju Das]

Hi Sergei,

Thanks for the feedback.

> Subject: Re: [PATCH v6 1/7] dt-bindings: usb: hd3ss3220 device tree binding
> document
> 
> Hello!
> 
> On 05/15/2019 03:09 PM, Biju Das wrote:
> 
> > Add device tree binding document for TI HD3SS3220 Type-C DRP port
> > controller driver.
> >
> > Signed-off-by: Biju Das <biju.das@bp.renesas.com>
> > Reviewed-by: Rob Herring <robh@kernel.org>
> > ---
> > V5-->V6
> >   * No change.
> > V4-->V5
> >   * No Change.
> > V3-->V4
> >   * No Change.
> > V2-->V3
> >   * Added Rob's Reviewed by tag.
> > V1-->V2
> >   * Added connector node.
> >   * updated the example with connector node.
> > ---
> >  .../devicetree/bindings/usb/ti,hd3ss3220.txt       | 37
> ++++++++++++++++++++++
> >  1 file changed, 37 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt
> >
> > diff --git a/Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt
> > b/Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt
> > new file mode 100644
> > index 0000000..7f41400
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt
> > @@ -0,0 +1,37 @@
> > +TI HD3SS3220 TypeC DRP Port Controller.
> > +
> > +Required properties:
> > + - compatible: Must be "ti,hd3ss3220".
> > + - reg: I2C slave address, must be 0x47 or 0x67 based on ADDR pin.
> > + - interrupts: <a b> where a is the interrupt number and b represents an
> > +   encoding of the sense and level information for the interrupt.
> 
>    This depends on an interrupt controller used. I'd just said "an interrupt
> specifier", w/o further details.

Fine , If it is ok for everyone. 

Regards,
Biju
> > +
> > +Required sub-node:
> > + - connector : The "usb-c-connector" attached to the hd3ss3220 chip. The
> > +   bindings of the connector node are specified in:
> > +
> > +	Documentation/devicetree/bindings/connector/usb-connector.txt
> > +
> [...]
> 
> MBR, Sergei

RE: [PATCH v6 1/7] dt-bindings: usb: hd3ss3220 device tree binding document

[Biju Das]

Hi Morimoto-San and Shimoda-San,

Thanks for the feedback.

> Subject: Re: [PATCH v6 1/7] dt-bindings: usb: hd3ss3220 device tree binding
> document
> 
> 
> Hi
> 
> > > +Required properties:
> > > + - compatible: Must be "ti,hd3ss3220".
> > > + - reg: I2C slave address, must be 0x47 or 0x67 based on ADDR pin.
> > > + - interrupts: <a b> where a is the interrupt number and b represents an
> > > +   encoding of the sense and level information for the interrupt.
> > > +
> > > +Required sub-node:
> > > + - connector : The "usb-c-connector" attached to the hd3ss3220
> > > +chip. The
> >
> > s/connector :/connector:/
> 
> Maybe it is for alignment ?

Yes, I need to fix the extra space.

> > According to the connector/usb-connector.txt, should the connector
> > node have ports, port@1 and an endpoint nodes like below?
> 
> "ports" is needed if it has multiple "port", otherwise, single port is allowed
> from OF-graph point of view.

OK. I will use single port on  the next patch series.

Regards,
Biju

RE: [PATCH v6 1/7] dt-bindings: usb: hd3ss3220 device tree binding document

[Yoshihiro Shimoda]

Hi Biju-san, Morimoto-san,

> From: Biju Das, Sent: Wednesday, May 22, 2019 8:00 PM
<snip>
> > > According to the connector/usb-connector.txt, should the connector
> > > node have ports, port@1 and an endpoint nodes like below?
> >
> > "ports" is needed if it has multiple "port", otherwise, single port is allowed
> > from OF-graph point of view.
> 
> OK. I will use single port on  the next patch series.

According to the connector/usb-connector.txt [1], even if this device uses a single port,
we should describe ports node and port@1 (for SuperSpeed) subnode like usb/typec-tcpci.txt.

[1]
Required nodes:
- any data bus to the connector should be modeled using the OF graph bindings
  specified in bindings/graph.txt, unless the bus is between parent node and
  the connector. Since single connector can have multiple data buses every bus
  has assigned OF graph port number as follows:
    0: High Speed (HS), present in all connectors,
    1: Super Speed (SS), present in SS capable connectors,
    2: Sideband use (SBU), present in USB-C.

Best regards,
Yoshihiro Shimoda

RE: [PATCH v6 1/7] dt-bindings: usb: hd3ss3220 device tree binding document

[Biju Das]

Hi Shimoda-San,

Thanks for the feedback.

> Subject: RE: [PATCH v6 1/7] dt-bindings: usb: hd3ss3220 device tree binding
> document
> 
> Hi Biju-san, Morimoto-san,
> 
> > From: Biju Das, Sent: Wednesday, May 22, 2019 8:00 PM
> <snip>
> > > > According to the connector/usb-connector.txt, should the connector
> > > > node have ports, port@1 and an endpoint nodes like below?
> > >
> > > "ports" is needed if it has multiple "port", otherwise, single port
> > > is allowed from OF-graph point of view.
> >
> > OK. I will use single port on  the next patch series.
> 
> According to the connector/usb-connector.txt [1], even if this device uses a
> single port, we should describe ports node and port@1 (for SuperSpeed)
> subnode like usb/typec-tcpci.txt.

OK.  I will update the example like below.

hd3ss3220@47 {
	compatible = "ti,hd3ss3220";
	reg = <0x47>;
	interrupt-parent = <&gpio6>;
	interrupts = <3 IRQ_TYPE_LEVEL_LOW>;

	usb_con: connector {
		compatible = "usb-c-connector";
		label = "USB-C";
		data-role = "dual";

		ports {
			#address-cells = <1>;
			#size-cells = <0>;

			port@1 {
				reg = <1>;
				hd3ss3220_ep: endpoint {
					remote-endpoint = <&usb3_role_switch>;
				};
			};
		};
	};
};

Regards,
Biju

> [1]
> Required nodes:
> - any data bus to the connector should be modeled using the OF graph
> bindings
>   specified in bindings/graph.txt, unless the bus is between parent node and
>   the connector. Since single connector can have multiple data buses every
> bus
>   has assigned OF graph port number as follows:
>     0: High Speed (HS), present in all connectors,
>     1: Super Speed (SS), present in SS capable connectors,
>     2: Sideband use (SBU), present in USB-C.

Re: [PATCH v6 7/7] arm64: dts: renesas: r8a774c0-cat874: Enable usb role switch support

[Rob Herring]

On Wed, May 15, 2019 at 01:09:12PM +0100, Biju Das wrote:
> This patch enables TI HD3SS3220 device and support usb role switch
> for the CAT 874 platform.
> 
> Signed-off-by: Biju Das <biju.das@bp.renesas.com>
> ---
> V5-->V6
>   * No change
> V4-->V5
>   * No change
> V3-->V4
>   * No change
> V2-->V3
>   * Used "renesas,usb-role-switch" instead of generic "usb-role-switch"
>     property
> V1-->V2
>   * New patch
> ---
>  arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts | 39 +++++++++++++++++++++++++
>  1 file changed, 39 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts b/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts
> index b9ae7db..124ed58 100644
> --- a/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts
> +++ b/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts
> @@ -85,6 +85,34 @@
>  	clock-frequency = <48000000>;
>  };
>  
> +&i2c0 {
> +	status = "okay";
> +	clock-frequency = <100000>;
> +
> +	hd3ss3220@47 {
> +		compatible = "ti,hd3ss3220";
> +		reg = <0x47>;
> +		interrupt-parent = <&gpio6>;
> +		interrupts = <3 IRQ_TYPE_LEVEL_LOW>;
> +
> +		usb_con: connector {
> +			compatible = "usb-c-connector";
> +			label = "USB-C";
> +			data-role = "dual";
> +		};
> +
> +		port {

port should be a child of 'connector' node. It should also be port #1 if 
this is a SuperSpeed controller. Port #0 is HS.

As there are multiple ports possible, there should be a 'ports' node 
too.

> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			hd3ss3220_ep: endpoint@0 {
> +				reg = <0>;

Don't need reg when there is only 1. Build your dtb with W=1 as that 
will tell you this.

> +				remote-endpoint = <&usb3peri_role_switch>;
> +			};
> +		};
> +	};
> +};
> +
>  &i2c1 {
>  	pinctrl-0 = <&i2c1_pins>;
>  	pinctrl-names = "default";
> @@ -175,6 +203,17 @@
>  &usb3_peri0 {
>  	companion = <&xhci0>;
>  	status = "okay";
> +	usb-role-switch;
> +
> +	port {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		usb3peri_role_switch: endpoint@0 {
> +			reg = <0>;
> +			remote-endpoint = <&hd3ss3220_ep>;
> +		};
> +	};
>  };
>  
>  &xhci0 {
> -- 
> 2.7.4
> 

RE: [PATCH v6 2/7] dt-bindings: usb: renesas_usb3: Document usb role switch support

[Biju Das]

HI Shimoda-San,

Thanks for the feedback.

> Subject: RE: [PATCH v6 2/7] dt-bindings: usb: renesas_usb3: Document usb
> role switch support
> 
> Hi Biju-san,
> 
> Thank you for the patch!
> 
> > From: Biju Das, Sent: Wednesday, May 15, 2019 9:09 PM
> >
> > Update the DT bindings documentation to support usb role switch for
> > USB Type-C connector using USB role switch class framework.
> >
> > Signed-off-by: Biju Das <biju.das@bp.renesas.com>
> > ---
> >  V5-->V6
> >   * Updated description
> >   * Added usb-role-switch-property
> >  V4-->V5
> >   * No Change
> >  V3-->V4
> >   * No Change
> >  V2-->V3
> >   * Added optional renesas,usb-role-switch property.
> >  V1-->V2
> >   * Added usb-role-switch-property
> >   * Updated the example with usb-role-switch property.
> > ---
> >  .../devicetree/bindings/usb/renesas_usb3.txt       | 26
> ++++++++++++++++++++++
> >  1 file changed, 26 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > index 35039e7..ea6c63c 100644
> > --- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > +++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > @@ -22,6 +22,11 @@ Required properties:
> >  Optional properties:
> >    - phys: phandle + phy specifier pair
> >    - phy-names: must be "usb"
> > +  - usb-role-switch: support role switch. see usb/generic.txt
> > +
> > +Sub-nodes:
> > +The port would be added as a subnode if the "usb-role-switch" property is
> used.
> > +	see graph.txt
> 
> I think we should describe which type of a subnode is needed.
> I made an example below. This is based on the usb-connector.txt.
> ---
> Sub-nodes:
> - any connector to the data bus of this controller should be modeled using
> the OF graph
> bindings specified in bindings/graph.txt

OK to me. I will add the following  at the end.  ", if the "usb-role-switch" property is
used."
> ---
> 
> >  Example of R-Car H3 ES1.x:
> >  	usb3_peri0: usb@ee020000 {
> > @@ -39,3 +44,24 @@ Example of R-Car H3 ES1.x:
> >  		interrupts = <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>;
> >  		clocks = <&cpg CPG_MOD 327>;
> >  	};
> > +
> > +Example of RZ/G2E:
> > +	usb3_peri0: usb@ee020000 {
> > +		compatible = "renesas,r8a774c0-usb3-peri",
> > +			     "renesas,rcar-gen3-usb3-peri";
> > +		reg = <0 0xee020000 0 0x400>;
> > +		interrupts = <GIC_SPI 104 IRQ_TYPE_LEVEL_HIGH>;
> > +		clocks = <&cpg CPG_MOD 328>;
> > +		companion = <&xhci0>;
> > +		usb-role-switch;
> > +
> > +		port {
> > +			#address-cells = <1>;
> > +			#size-cells = <0>;
> > +
> > +			usb3peri_role_switch: endpoint@0 {
> > +				reg = <0>;
> 
> I'm not sure, but I don't think this endpoint@0 and reg = <0> are needed.
> In other words, can we use have following node?
> 			usb3peri_role_switch: endpoint {
> 				remote-endpoint = <&hd3ss3220_ep>;
> 			};

OK . Will update  the example with

        port {                                                                   
                usb3_role_switch: endpoint {                                     
                        remote-endpoint = <&hd3ss3220_ep>;                       
                 };                                                               
         };

Regards,
Biju