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

[PATCH v2 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-20190306 branch.

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: add usb-role-switch property
  usb: typec: driver for TI HD3SS3220 USB Type-C DRP port controller
  usb: gadget: udc: renesas_usb3: Use usb_role_switch framework
  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       |  22 ++
 .../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              | 125 ++++++++--
 drivers/usb/typec/Kconfig                          |  10 +
 drivers/usb/typec/Makefile                         |   1 +
 drivers/usb/typec/hd3ss3220.c                      | 266 +++++++++++++++++++++
 8 files changed, 497 insertions(+), 22 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt
 create mode 100644 drivers/usb/typec/hd3ss3220.c

-- 
2.7.4

[PATCH v2 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>
---
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 v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role-switch property

[Biju Das]

Add support for renesas_usb3 to support dual role switch
using usb role switch class framework.

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

diff --git a/Documentation/devicetree/bindings/usb/renesas_usb3.txt b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
index 35039e7..eecaf4c 100644
--- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
+++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
@@ -22,6 +22,7 @@ Required properties:
 Optional properties:
   - phys: phandle + phy specifier pair
   - phy-names: must be "usb"
+  - usb-role-switch: use USB role switch to support dual-role switch
 
 Example of R-Car H3 ES1.x:
 	usb3_peri0: usb@ee020000 {
@@ -39,3 +40,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 v2 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>
---
V1-->V2
  * No change

This patch depends on commit 35ed6229c0f0d079f2
("usb: gadget: udc: renesas_usb3: Add bindings for r8a774c0")
---
 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 42c66f9..d9d55c8 100644
--- a/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts
+++ b/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts
@@ -124,6 +124,11 @@
 		function = "sdhi0";
 		power-source = <1800>;
 	};
+
+	usb30_pins: usb30 {
+		groups = "usb30", "usb30_id";
+		function = "usb30";
+	};
 };
 
 &rwdt {
@@ -151,3 +156,15 @@
 	sd-uhs-sdr104;
 	status = "okay";
 };
+
+&usb3_peri0 {
+	companion = <&xhci0>;
+	status = "okay";
+};
+
+&xhci0 {
+	pinctrl-0 = <&usb30_pins>;
+	pinctrl-names = "default";
+
+	status = "okay";
+};
-- 
2.7.4

[PATCH v2 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>
---
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 d9d55c8..ea74d21 100644
--- a/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts
+++ b/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts
@@ -80,6 +80,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";
@@ -160,6 +188,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 v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role-switch property

[Yoshihiro Shimoda]

Hi Biju-san,

> From: Biju Das, Sent: Thursday, March 14, 2019 5:40 PM
> 
> Add support for renesas_usb3 to support dual role switch
> using usb role switch class framework.
> 
> Signed-off-by: Biju Das <biju.das@bp.renesas.com>
> ---
>  V1-->V2
>   * Added usb-role-switch-property
>   * Updated the example with usb-role-switch property.
> ---
>  .../devicetree/bindings/usb/renesas_usb3.txt       | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> index 35039e7..eecaf4c 100644
> --- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> +++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> @@ -22,6 +22,7 @@ Required properties:
>  Optional properties:
>    - phys: phandle + phy specifier pair
>    - phy-names: must be "usb"
> +  - usb-role-switch: use USB role switch to support dual-role switch

I don't think we can add such a property. At least, we have to add "renesas," prefix.

Best regards,
Yoshihiro Shimoda

>  Example of R-Car H3 ES1.x:
>  	usb3_peri0: usb@ee020000 {
> @@ -39,3 +40,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

RE: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role-switch property

[Biju Das]

Hi Shimoda-San,

Thanks for the feedback.

> Subject: RE: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role-
> switch property
> 
> Hi Biju-san,
> 
> > From: Biju Das, Sent: Thursday, March 14, 2019 5:40 PM
> >
> > Add support for renesas_usb3 to support dual role switch using usb
> > role switch class framework.
> >
> > Signed-off-by: Biju Das <biju.das@bp.renesas.com>
> > ---
> >  V1-->V2
> >   * Added usb-role-switch-property
> >   * Updated the example with usb-role-switch property.
> > ---
> >  .../devicetree/bindings/usb/renesas_usb3.txt       | 22
> ++++++++++++++++++++++
> >  1 file changed, 22 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > index 35039e7..eecaf4c 100644
> > --- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > +++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > @@ -22,6 +22,7 @@ Required properties:
> >  Optional properties:
> >    - phys: phandle + phy specifier pair
> >    - phy-names: must be "usb"
> > +  - usb-role-switch: use USB role switch to support dual-role switch
> 
> I don't think we can add such a property. At least, we have to add "renesas,"
> prefix.

usb_role_switch_get   api uses  "usb-role-switch"  property to get role switch linked with the device.

HD3SS3220 port controller driver gets role switch handle linked with the device using usb_role_switch_get  api.  
That is the reason, I have added " usb-role-switch" property here.

Do you have any other suggestion to get usb role switch handle?

Please correct me if I am wrong. 
 
> >  Example of R-Car H3 ES1.x:
> >  	usb3_peri0: usb@ee020000 {
> > @@ -39,3 +40,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

Regards,
Biju

Re: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role-switch property

[Heikki Krogerus]

On Thu, Mar 14, 2019 at 10:14:12AM +0000, Biju Das wrote:
> Hi Shimoda-San,
> 
> Thanks for the feedback.
> 
> > Subject: RE: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role-
> > switch property
> > 
> > Hi Biju-san,
> > 
> > > From: Biju Das, Sent: Thursday, March 14, 2019 5:40 PM
> > >
> > > Add support for renesas_usb3 to support dual role switch using usb
> > > role switch class framework.
> > >
> > > Signed-off-by: Biju Das <biju.das@bp.renesas.com>
> > > ---
> > >  V1-->V2
> > >   * Added usb-role-switch-property
> > >   * Updated the example with usb-role-switch property.
> > > ---
> > >  .../devicetree/bindings/usb/renesas_usb3.txt       | 22
> > ++++++++++++++++++++++
> > >  1 file changed, 22 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > index 35039e7..eecaf4c 100644
> > > --- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > +++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > @@ -22,6 +22,7 @@ Required properties:
> > >  Optional properties:
> > >    - phys: phandle + phy specifier pair
> > >    - phy-names: must be "usb"
> > > +  - usb-role-switch: use USB role switch to support dual-role switch
> > 
> > I don't think we can add such a property. At least, we have to add "renesas,"
> > prefix.
> 
> usb_role_switch_get   api uses  "usb-role-switch"  property to get role switch linked with the device.
> 
> HD3SS3220 port controller driver gets role switch handle linked with the device using usb_role_switch_get  api.  
> That is the reason, I have added " usb-role-switch" property here.
> 
> Do you have any other suggestion to get usb role switch handle?

We can still change the API. Can we use the compatible for this?


thanks,

-- 
heikki

RE: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role-switch property

[Biju Das]

Hi Heikki,

Thanks for the feedback.

> Subject: Re: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role-
> switch property
> 
> On Thu, Mar 14, 2019 at 10:14:12AM +0000, Biju Das wrote:
> > Hi Shimoda-San,
> >
> > Thanks for the feedback.
> >
> > > Subject: RE: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add
> > > usb-role- switch property
> > >
> > > Hi Biju-san,
> > >
> > > > From: Biju Das, Sent: Thursday, March 14, 2019 5:40 PM
> > > >
> > > > Add support for renesas_usb3 to support dual role switch using usb
> > > > role switch class framework.
> > > >
> > > > Signed-off-by: Biju Das <biju.das@bp.renesas.com>
> > > > ---
> > > >  V1-->V2
> > > >   * Added usb-role-switch-property
> > > >   * Updated the example with usb-role-switch property.
> > > > ---
> > > >  .../devicetree/bindings/usb/renesas_usb3.txt       | 22
> > > ++++++++++++++++++++++
> > > >  1 file changed, 22 insertions(+)
> > > >
> > > > diff --git
> > > > a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > index 35039e7..eecaf4c 100644
> > > > --- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > +++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > @@ -22,6 +22,7 @@ Required properties:
> > > >  Optional properties:
> > > >    - phys: phandle + phy specifier pair
> > > >    - phy-names: must be "usb"
> > > > +  - usb-role-switch: use USB role switch to support dual-role
> > > > + switch
> > >
> > > I don't think we can add such a property. At least, we have to add
> "renesas,"
> > > prefix.
> >
> > usb_role_switch_get   api uses  "usb-role-switch"  property to get role
> switch linked with the device.
> >
> > HD3SS3220 port controller driver gets role switch handle linked with the
> device using usb_role_switch_get  api.
> > That is the reason, I have added " usb-role-switch" property here.
> >
> > Do you have any other suggestion to get usb role switch handle?
> 
> We can still change the API. Can we use the compatible for this?

Do you mean usb_role_switch_get  API needs  to handle compatible "usb-x-connector"  wherex= a,b ,c ?
Then it uses the graphs api to get the device linked with it and return the usb role switch handle.
In that case, no need to add  generic "usb-role-switch"  property here.

Can you please confirm my understanding is correct?

Regards,
Biju

Re: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role-switch property

[Heikki Krogerus]

Thanks,

On Fri, Mar 15, 2019 at 09:08:19AM +0000, Biju Das wrote:
> > Subject: Re: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role-
> > > > > a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > index 35039e7..eecaf4c 100644
> > > > > --- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > +++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > @@ -22,6 +22,7 @@ Required properties:
> > > > >  Optional properties:
> > > > >    - phys: phandle + phy specifier pair
> > > > >    - phy-names: must be "usb"
> > > > > +  - usb-role-switch: use USB role switch to support dual-role
> > > > > + switch
> > > >
> > > > I don't think we can add such a property. At least, we have to add
> > "renesas,"
> > > > prefix.
> > >
> > > usb_role_switch_get   api uses  "usb-role-switch"  property to get role
> > switch linked with the device.
> > >
> > > HD3SS3220 port controller driver gets role switch handle linked with the
> > device using usb_role_switch_get  api.
> > > That is the reason, I have added " usb-role-switch" property here.
> > >
> > > Do you have any other suggestion to get usb role switch handle?
> > 
> > We can still change the API. Can we use the compatible for this?
> 
> Do you mean usb_role_switch_get  API needs  to handle compatible "usb-x-connector"  wherex= a,b ,c ?
> Then it uses the graphs api to get the device linked with it and return the usb role switch handle.
> In that case, no need to add  generic "usb-role-switch"  property here.
> 
> Can you please confirm my understanding is correct?

No, I meant the compatible property would have the value
"usb-role-switch". Your compatible would probable look something
like this:

        compatible = "renesas,r8a774c0-usb3-peri",
                     "usb-role-switch";

So the idea would be that the device supplying USB role switch
functionality, in your case the USB controller, would need to have the
compatible property containing "usb-role-switch".

I'm really not an expert in DT, so I don't know if that is acceptable.
Rob can you comment on this?


thanks,

-- 
heikki

Re: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role-switch property

[Rob Herring]

On Fri, Mar 15, 2019 at 12:51:46PM +0200, Heikki Krogerus wrote:
> Thanks,
> 
> On Fri, Mar 15, 2019 at 09:08:19AM +0000, Biju Das wrote:
> > > Subject: Re: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role-
> > > > > > a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > > b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > > index 35039e7..eecaf4c 100644
> > > > > > --- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > > +++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > > @@ -22,6 +22,7 @@ Required properties:
> > > > > >  Optional properties:
> > > > > >    - phys: phandle + phy specifier pair
> > > > > >    - phy-names: must be "usb"
> > > > > > +  - usb-role-switch: use USB role switch to support dual-role
> > > > > > + switch
> > > > >
> > > > > I don't think we can add such a property. At least, we have to add
> > > "renesas,"
> > > > > prefix.
> > > >
> > > > usb_role_switch_get   api uses  "usb-role-switch"  property to get role
> > > switch linked with the device.
> > > >
> > > > HD3SS3220 port controller driver gets role switch handle linked with the
> > > device using usb_role_switch_get  api.
> > > > That is the reason, I have added " usb-role-switch" property here.
> > > >
> > > > Do you have any other suggestion to get usb role switch handle?
> > > 
> > > We can still change the API. Can we use the compatible for this?
> > 
> > Do you mean usb_role_switch_get  API needs  to handle compatible "usb-x-connector"  wherex= a,b ,c ?
> > Then it uses the graphs api to get the device linked with it and return the usb role switch handle.
> > In that case, no need to add  generic "usb-role-switch"  property here.
> > 
> > Can you please confirm my understanding is correct?
> 
> No, I meant the compatible property would have the value
> "usb-role-switch". Your compatible would probable look something
> like this:
> 
>         compatible = "renesas,r8a774c0-usb3-peri",
>                      "usb-role-switch";
> 
> So the idea would be that the device supplying USB role switch
> functionality, in your case the USB controller, would need to have the
> compatible property containing "usb-role-switch".

That's not really something a driver could bind to nor provides any info 
as to what the h/w is (and how to interact with it).

> 
> I'm really not an expert in DT, so I don't know if that is acceptable.
> Rob can you comment on this?

I would go with Biju's proposal.

Rob

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

[Rob Herring]

On Thu, Mar 14, 2019 at 08:39:29AM +0000, 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>
> ---
> 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

Reviewed-by: Rob Herring <robh@kernel.org>

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

Arguably, not that useful of a label as it doesn't describe which USB C 
connector.

> +		data-role = "dual";
> +	};
> +
> +	port {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		hd3ss3220_ep: endpoint@0 {
> +			reg = <0>;
> +			remote-endpoint = <&usb3peri_role_switch>;
> +		};
> +	};
> +};
> -- 
> 2.7.4
> 

Re: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role-switch property

[Heikki Krogerus]

On Thu, Mar 28, 2019 at 10:33:53AM -0500, Rob Herring wrote:
> On Fri, Mar 15, 2019 at 12:51:46PM +0200, Heikki Krogerus wrote:
> > Thanks,
> > 
> > On Fri, Mar 15, 2019 at 09:08:19AM +0000, Biju Das wrote:
> > > > Subject: Re: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role-
> > > > > > > a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > > > b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > > > index 35039e7..eecaf4c 100644
> > > > > > > --- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > > > +++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > > > @@ -22,6 +22,7 @@ Required properties:
> > > > > > >  Optional properties:
> > > > > > >    - phys: phandle + phy specifier pair
> > > > > > >    - phy-names: must be "usb"
> > > > > > > +  - usb-role-switch: use USB role switch to support dual-role
> > > > > > > + switch
> > > > > >
> > > > > > I don't think we can add such a property. At least, we have to add
> > > > "renesas,"
> > > > > > prefix.
> > > > >
> > > > > usb_role_switch_get   api uses  "usb-role-switch"  property to get role
> > > > switch linked with the device.
> > > > >
> > > > > HD3SS3220 port controller driver gets role switch handle linked with the
> > > > device using usb_role_switch_get  api.
> > > > > That is the reason, I have added " usb-role-switch" property here.
> > > > >
> > > > > Do you have any other suggestion to get usb role switch handle?
> > > > 
> > > > We can still change the API. Can we use the compatible for this?
> > > 
> > > Do you mean usb_role_switch_get  API needs  to handle compatible "usb-x-connector"  wherex= a,b ,c ?
> > > Then it uses the graphs api to get the device linked with it and return the usb role switch handle.
> > > In that case, no need to add  generic "usb-role-switch"  property here.
> > > 
> > > Can you please confirm my understanding is correct?
> > 
> > No, I meant the compatible property would have the value
> > "usb-role-switch". Your compatible would probable look something
> > like this:
> > 
> >         compatible = "renesas,r8a774c0-usb3-peri",
> >                      "usb-role-switch";
> > 
> > So the idea would be that the device supplying USB role switch
> > functionality, in your case the USB controller, would need to have the
> > compatible property containing "usb-role-switch".
> 
> That's not really something a driver could bind to nor provides any info 
> as to what the h/w is (and how to interact with it).

Fair enough. As I said, I don't know much about DT.

The problem we are trying to solve here is, how to identify the USB
role switch from graph. It's primarily the USB Type-C connector
drivers that need to be able to get a handle to the role switch device
so they can tell it what to do. Basically the caller of
usb_role_switch_get() will have the compatible "usb-c-connector", so
it's of no use to us. We need the other endpoint, the role switch.

Note: The USB role switch device will be a discrete (or integrated)
mux on platforms that have separate USB host and USB device
controllers, and on platforms with dual-role capable USB controller
(like this one) the USB controller will represent it.

At the moment the function usb_role_switch_get() walks trough the
graph of the caller (so the connector) and expects that the
remote-endpoint representing the mux will have a boolean property
"usb-role-switch".

I do agree that Documentation/devicetree/bindings/usb/renesas_usb3.txt
should not be the file describing that boolean property, but if we had
a separate file describing the bindings for just the USB role switch
devices, would using it still be OK?

If not, then can you propose something else? How do we identify the
USB role switch endpoint from the graph? Would it be OK to expect the
the endpoint subnode to have that boolean property instead?


thanks,

-- 
heikki

Re: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role-switch property

[Rob Herring]

On Thu, Mar 28, 2019 at 12:48 PM Heikki Krogerus
<heikki.krogerus@linux.intel.com> wrote:
>
> On Thu, Mar 28, 2019 at 10:33:53AM -0500, Rob Herring wrote:
> > On Fri, Mar 15, 2019 at 12:51:46PM +0200, Heikki Krogerus wrote:
> > > Thanks,
> > >
> > > On Fri, Mar 15, 2019 at 09:08:19AM +0000, Biju Das wrote:
> > > > > Subject: Re: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role-
> > > > > > > > a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > > > > b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > > > > index 35039e7..eecaf4c 100644
> > > > > > > > --- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > > > > +++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > > > > @@ -22,6 +22,7 @@ Required properties:
> > > > > > > >  Optional properties:
> > > > > > > >    - phys: phandle + phy specifier pair
> > > > > > > >    - phy-names: must be "usb"
> > > > > > > > +  - usb-role-switch: use USB role switch to support dual-role
> > > > > > > > + switch
> > > > > > >
> > > > > > > I don't think we can add such a property. At least, we have to add
> > > > > "renesas,"
> > > > > > > prefix.
> > > > > >
> > > > > > usb_role_switch_get   api uses  "usb-role-switch"  property to get role
> > > > > switch linked with the device.
> > > > > >
> > > > > > HD3SS3220 port controller driver gets role switch handle linked with the
> > > > > device using usb_role_switch_get  api.
> > > > > > That is the reason, I have added " usb-role-switch" property here.
> > > > > >
> > > > > > Do you have any other suggestion to get usb role switch handle?
> > > > >
> > > > > We can still change the API. Can we use the compatible for this?
> > > >
> > > > Do you mean usb_role_switch_get  API needs  to handle compatible "usb-x-connector"  wherex= a,b ,c ?
> > > > Then it uses the graphs api to get the device linked with it and return the usb role switch handle.
> > > > In that case, no need to add  generic "usb-role-switch"  property here.
> > > >
> > > > Can you please confirm my understanding is correct?
> > >
> > > No, I meant the compatible property would have the value
> > > "usb-role-switch". Your compatible would probable look something
> > > like this:
> > >
> > >         compatible = "renesas,r8a774c0-usb3-peri",
> > >                      "usb-role-switch";
> > >
> > > So the idea would be that the device supplying USB role switch
> > > functionality, in your case the USB controller, would need to have the
> > > compatible property containing "usb-role-switch".
> >
> > That's not really something a driver could bind to nor provides any info
> > as to what the h/w is (and how to interact with it).
>
> Fair enough. As I said, I don't know much about DT.
>
> The problem we are trying to solve here is, how to identify the USB
> role switch from graph. It's primarily the USB Type-C connector
> drivers that need to be able to get a handle to the role switch device
> so they can tell it what to do. Basically the caller of
> usb_role_switch_get() will have the compatible "usb-c-connector", so
> it's of no use to us. We need the other endpoint, the role switch.
>
> Note: The USB role switch device will be a discrete (or integrated)
> mux on platforms that have separate USB host and USB device
> controllers, and on platforms with dual-role capable USB controller
> (like this one) the USB controller will represent it.

Either way, it should just be the device connected to the connector's
USB data port in the graph. Though maybe if USB2 and USB3 are
different controllers that would complicate things.

> At the moment the function usb_role_switch_get() walks trough the
> graph of the caller (so the connector) and expects that the
> remote-endpoint representing the mux will have a boolean property
> "usb-role-switch".

Assuming we have that I think it should be in the device
(controller/mux) node rather than the endpoint node itself.

It could also be outside of DT in that the controller driver will know
the h/w can support role switch and can register that capability. IOW,
it can be implied by the compatible string.

>
> I do agree that Documentation/devicetree/bindings/usb/renesas_usb3.txt
> should not be the file describing that boolean property, but if we had
> a separate file describing the bindings for just the USB role switch
> devices, would using it still be OK?

It should be described somewhere common, yes, but the property itself
belongs in controller nodes. Generally, we still document where common
properties are used.

Though, I prefer the implied by the compatible option. This should
work unless there's a strong need to find the switch in DT without the
switch driver being probed or if this is board specific (I wouldn't
think so).

>
> If not, then can you propose something else? How do we identify the
> USB role switch endpoint from the graph? Would it be OK to expect the
> the endpoint subnode to have that boolean property instead?
>
>
> thanks,
>
> --
> heikki

RE: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role-switch property

[Biju Das]

Hi All,

Thanks for the feedback.

> -----Original Message-----
> From: Rob Herring <robh@kernel.org>
> Sent: 29 March 2019 13:58
> To: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> Cc: Biju Das <biju.das@bp.renesas.com>; Yoshihiro Shimoda
> <yoshihiro.shimoda.uh@renesas.com>; Mark Rutland
> <mark.rutland@arm.com>; Greg Kroah-Hartman
> <gregkh@linuxfoundation.org>; Felipe Balbi <balbi@kernel.org>; linux-
> usb@vger.kernel.org; devicetree@vger.kernel.org; Simon Horman
> <horms@verge.net.au>; Geert Uytterhoeven <geert+renesas@glider.be>;
> Chris Paterson <Chris.Paterson2@renesas.com>; Fabrizio Castro
> <fabrizio.castro@bp.renesas.com>; linux-renesas-soc@vger.kernel.org
> Subject: Re: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role-
> switch property
> 
> On Thu, Mar 28, 2019 at 12:48 PM Heikki Krogerus
> <heikki.krogerus@linux.intel.com> wrote:
> >
> > On Thu, Mar 28, 2019 at 10:33:53AM -0500, Rob Herring wrote:
> > > On Fri, Mar 15, 2019 at 12:51:46PM +0200, Heikki Krogerus wrote:
> > > > Thanks,
> > > >
> > > > On Fri, Mar 15, 2019 at 09:08:19AM +0000, Biju Das wrote:
> > > > > > Subject: Re: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3:
> > > > > > add usb-role-
> > > > > > > > > a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > > > > > b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > > > > > index 35039e7..eecaf4c 100644
> > > > > > > > > ---
> > > > > > > > > a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > > > > > +++
> b/Documentation/devicetree/bindings/usb/renesas_usb3
> > > > > > > > > +++ .txt
> > > > > > > > > @@ -22,6 +22,7 @@ Required properties:
> > > > > > > > >  Optional properties:
> > > > > > > > >    - phys: phandle + phy specifier pair
> > > > > > > > >    - phy-names: must be "usb"
> > > > > > > > > +  - usb-role-switch: use USB role switch to support
> > > > > > > > > + dual-role switch
> > > > > > > >
> > > > > > > > I don't think we can add such a property. At least, we
> > > > > > > > have to add
> > > > > > "renesas,"
> > > > > > > > prefix.
> > > > > > >
> > > > > > > usb_role_switch_get   api uses  "usb-role-switch"  property to get
> role
> > > > > > switch linked with the device.
> > > > > > >
> > > > > > > HD3SS3220 port controller driver gets role switch handle
> > > > > > > linked with the
> > > > > > device using usb_role_switch_get  api.
> > > > > > > That is the reason, I have added " usb-role-switch" property here.
> > > > > > >
> > > > > > > Do you have any other suggestion to get usb role switch handle?
> > > > > >
> > > > > > We can still change the API. Can we use the compatible for this?
> > > > >
> > > > > Do you mean usb_role_switch_get  API needs  to handle compatible
> "usb-x-connector"  wherex= a,b ,c ?
> > > > > Then it uses the graphs api to get the device linked with it and return
> the usb role switch handle.
> > > > > In that case, no need to add  generic "usb-role-switch"  property
> here.
> > > > >
> > > > > Can you please confirm my understanding is correct?
> > > >
> > > > No, I meant the compatible property would have the value
> > > > "usb-role-switch". Your compatible would probable look something
> > > > like this:
> > > >
> > > >         compatible = "renesas,r8a774c0-usb3-peri",
> > > >                      "usb-role-switch";
> > > >
> > > > So the idea would be that the device supplying USB role switch
> > > > functionality, in your case the USB controller, would need to have
> > > > the compatible property containing "usb-role-switch".
> > >
> > > That's not really something a driver could bind to nor provides any
> > > info as to what the h/w is (and how to interact with it).
> >
> > Fair enough. As I said, I don't know much about DT.
> >
> > The problem we are trying to solve here is, how to identify the USB
> > role switch from graph. It's primarily the USB Type-C connector
> > drivers that need to be able to get a handle to the role switch device
> > so they can tell it what to do. Basically the caller of
> > usb_role_switch_get() will have the compatible "usb-c-connector", so
> > it's of no use to us. We need the other endpoint, the role switch.
> >
> > Note: The USB role switch device will be a discrete (or integrated)
> > mux on platforms that have separate USB host and USB device
> > controllers, and on platforms with dual-role capable USB controller
> > (like this one) the USB controller will represent it.

Looks like we have a solution now. Heikki already proposed a new API
"add API to get usb_role_switch by node" https://patchwork.kernel.org/patch/10882555/

We can use this API to find remote endpoint of USB role switch device(renesas usb3) connected with type-c DRP port controller driver(TI HD3SS3220).
I believe in this case, we don't need generic Boolean property " usb-role-switch"

Apart from this, we can add Renesas specific property (renesas, usb-role-switch) to differentiate the
USB role switch associated with type C DRP controller driver from others.

I will send V3 based on this solution. Please let me know if you think otherwise.

Regards,
Biju

> Either way, it should just be the device connected to the connector's USB
> data port in the graph. Though maybe if USB2 and USB3 are different
> controllers that would complicate things.
> 
> > At the moment the function usb_role_switch_get() walks trough the
> > graph of the caller (so the connector) and expects that the
> > remote-endpoint representing the mux will have a boolean property
> > "usb-role-switch".
> 
> Assuming we have that I think it should be in the device
> (controller/mux) node rather than the endpoint node itself.
> 
> It could also be outside of DT in that the controller driver will know the h/w
> can support role switch and can register that capability. IOW, it can be implied
> by the compatible string.
> 
> >
> > I do agree that Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > should not be the file describing that boolean property, but if we had
> > a separate file describing the bindings for just the USB role switch
> > devices, would using it still be OK?
> 
> It should be described somewhere common, yes, but the property itself
> belongs in controller nodes. Generally, we still document where common
> properties are used.
> 
> Though, I prefer the implied by the compatible option. This should work
> unless there's a strong need to find the switch in DT without the switch driver
> being probed or if this is board specific (I wouldn't think so).
> 
> >
> > If not, then can you propose something else? How do we identify the
> > USB role switch endpoint from the graph? Would it be OK to expect the
> > the endpoint subnode to have that boolean property instead?
> >