* [PATCH v6 1/7] dt-bindings: usb: hd3ss3220 device tree binding document
2019-05-15 12:09 [PATCH v6 0/7] Add USB3.0 and TI HD3SS3220 driver support Biju Das
@ 2019-05-15 12:09 ` Biju Das
2019-05-15 16:09 ` Sergei Shtylyov
2019-05-21 8:03 ` Yoshihiro Shimoda
2019-05-15 12:09 ` [PATCH v6 2/7] dt-bindings: usb: renesas_usb3: Document usb role switch support Biju Das
` (5 subsequent siblings)
6 siblings, 2 replies; 23+ messages in thread
From: Biju Das @ 2019-05-15 12:09 UTC (permalink / raw)
To: Rob Herring, Mark Rutland, Greg Kroah-Hartman
Cc: Biju Das, Heikki Krogerus, Felipe Balbi, linux-usb, devicetree,
Simon Horman, Yoshihiro Shimoda, Geert Uytterhoeven,
Chris Paterson, Fabrizio Castro, linux-renesas-soc
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
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v6 1/7] dt-bindings: usb: hd3ss3220 device tree binding document
2019-05-15 12:09 ` [PATCH v6 1/7] dt-bindings: usb: hd3ss3220 device tree binding document Biju Das
@ 2019-05-15 16:09 ` Sergei Shtylyov
2019-05-21 9:42 ` Biju Das
2019-05-21 8:03 ` Yoshihiro Shimoda
1 sibling, 1 reply; 23+ messages in thread
From: Sergei Shtylyov @ 2019-05-15 16:09 UTC (permalink / raw)
To: Biju Das, Rob Herring, Mark Rutland, Greg Kroah-Hartman
Cc: Heikki Krogerus, Felipe Balbi, linux-usb, devicetree,
Simon Horman, Yoshihiro Shimoda, Geert Uytterhoeven,
Chris Paterson, Fabrizio Castro, linux-renesas-soc
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: [PATCH v6 1/7] dt-bindings: usb: hd3ss3220 device tree binding document
2019-05-15 16:09 ` Sergei Shtylyov
@ 2019-05-21 9:42 ` Biju Das
0 siblings, 0 replies; 23+ messages in thread
From: Biju Das @ 2019-05-21 9:42 UTC (permalink / raw)
To: Sergei Shtylyov, Rob Herring, Mark Rutland, Greg Kroah-Hartman
Cc: Heikki Krogerus, Felipe Balbi, linux-usb, devicetree,
Simon Horman, Yoshihiro Shimoda, Geert Uytterhoeven,
Chris Paterson, Fabrizio Castro, linux-renesas-soc
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: [PATCH v6 1/7] dt-bindings: usb: hd3ss3220 device tree binding document
2019-05-15 12:09 ` [PATCH v6 1/7] dt-bindings: usb: hd3ss3220 device tree binding document Biju Das
2019-05-15 16:09 ` Sergei Shtylyov
@ 2019-05-21 8:03 ` Yoshihiro Shimoda
2019-05-21 8:08 ` Kuninori Morimoto
1 sibling, 1 reply; 23+ messages in thread
From: Yoshihiro Shimoda @ 2019-05-21 8:03 UTC (permalink / raw)
To: Biju Das, Rob Herring, Mark Rutland, Greg Kroah-Hartman
Cc: Biju Das, Heikki Krogerus, Felipe Balbi, linux-usb, devicetree,
Simon Horman, Geert Uytterhoeven, Chris Paterson,
Fabrizio Castro, linux-renesas-soc
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v6 1/7] dt-bindings: usb: hd3ss3220 device tree binding document
2019-05-21 8:03 ` Yoshihiro Shimoda
@ 2019-05-21 8:08 ` Kuninori Morimoto
2019-05-22 10:59 ` Biju Das
0 siblings, 1 reply; 23+ messages in thread
From: Kuninori Morimoto @ 2019-05-21 8:08 UTC (permalink / raw)
To: Yoshihiro Shimoda
Cc: Biju Das, Rob Herring, Mark Rutland, Greg Kroah-Hartman,
Heikki Krogerus, Felipe Balbi, linux-usb, devicetree,
Simon Horman, Geert Uytterhoeven, Chris Paterson,
Fabrizio Castro, linux-renesas-soc
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: [PATCH v6 1/7] dt-bindings: usb: hd3ss3220 device tree binding document
2019-05-21 8:08 ` Kuninori Morimoto
@ 2019-05-22 10:59 ` Biju Das
2019-05-23 5:18 ` Yoshihiro Shimoda
0 siblings, 1 reply; 23+ messages in thread
From: Biju Das @ 2019-05-22 10:59 UTC (permalink / raw)
To: Kuninori Morimoto, Yoshihiro Shimoda
Cc: Rob Herring, Mark Rutland, Greg Kroah-Hartman, Heikki Krogerus,
Felipe Balbi, linux-usb, devicetree, Simon Horman,
Geert Uytterhoeven, Chris Paterson, Fabrizio Castro,
linux-renesas-soc
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: [PATCH v6 1/7] dt-bindings: usb: hd3ss3220 device tree binding document
2019-05-22 10:59 ` Biju Das
@ 2019-05-23 5:18 ` Yoshihiro Shimoda
2019-05-23 8:56 ` Biju Das
0 siblings, 1 reply; 23+ messages in thread
From: Yoshihiro Shimoda @ 2019-05-23 5:18 UTC (permalink / raw)
To: Biju Das, Kuninori Morimoto
Cc: Rob Herring, Mark Rutland, Greg Kroah-Hartman, Heikki Krogerus,
Felipe Balbi, linux-usb, devicetree, Simon Horman,
Geert Uytterhoeven, Chris Paterson, Fabrizio Castro,
linux-renesas-soc
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: [PATCH v6 1/7] dt-bindings: usb: hd3ss3220 device tree binding document
2019-05-23 5:18 ` Yoshihiro Shimoda
@ 2019-05-23 8:56 ` Biju Das
0 siblings, 0 replies; 23+ messages in thread
From: Biju Das @ 2019-05-23 8:56 UTC (permalink / raw)
To: Yoshihiro Shimoda, Kuninori Morimoto
Cc: Rob Herring, Mark Rutland, Greg Kroah-Hartman, Heikki Krogerus,
Felipe Balbi, linux-usb, devicetree, Simon Horman,
Geert Uytterhoeven, Chris Paterson, Fabrizio Castro,
linux-renesas-soc
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.
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v6 2/7] dt-bindings: usb: renesas_usb3: Document usb role switch support
2019-05-15 12:09 [PATCH v6 0/7] Add USB3.0 and TI HD3SS3220 driver support Biju Das
2019-05-15 12:09 ` [PATCH v6 1/7] dt-bindings: usb: hd3ss3220 device tree binding document Biju Das
@ 2019-05-15 12:09 ` Biju Das
2019-05-21 8:27 ` Yoshihiro Shimoda
2019-05-15 12:09 ` [PATCH v6 3/7] usb: typec: driver for TI HD3SS3220 USB Type-C DRP port controller Biju Das
` (4 subsequent siblings)
6 siblings, 1 reply; 23+ messages in thread
From: Biju Das @ 2019-05-15 12:09 UTC (permalink / raw)
To: Rob Herring, Mark Rutland, Greg Kroah-Hartman
Cc: Biju Das, Heikki Krogerus, Felipe Balbi, linux-usb, devicetree,
Simon Horman, Yoshihiro Shimoda, Geert Uytterhoeven,
Chris Paterson, Fabrizio Castro, linux-renesas-soc
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
^ permalink raw reply related [flat|nested] 23+ messages in thread
* RE: [PATCH v6 2/7] dt-bindings: usb: renesas_usb3: Document usb role switch support
2019-05-15 12:09 ` [PATCH v6 2/7] dt-bindings: usb: renesas_usb3: Document usb role switch support Biju Das
@ 2019-05-21 8:27 ` Yoshihiro Shimoda
2019-05-28 7:25 ` Biju Das
0 siblings, 1 reply; 23+ messages in thread
From: Yoshihiro Shimoda @ 2019-05-21 8:27 UTC (permalink / raw)
To: Biju Das, Rob Herring, Mark Rutland, Greg Kroah-Hartman
Cc: Biju Das, Heikki Krogerus, Felipe Balbi, linux-usb, devicetree,
Simon Horman, Geert Uytterhoeven, Chris Paterson,
Fabrizio Castro, linux-renesas-soc
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: [PATCH v6 2/7] dt-bindings: usb: renesas_usb3: Document usb role switch support
2019-05-21 8:27 ` Yoshihiro Shimoda
@ 2019-05-28 7:25 ` Biju Das
0 siblings, 0 replies; 23+ messages in thread
From: Biju Das @ 2019-05-28 7:25 UTC (permalink / raw)
To: Yoshihiro Shimoda, Rob Herring, Mark Rutland, Greg Kroah-Hartman
Cc: Heikki Krogerus, Felipe Balbi, linux-usb, devicetree,
Simon Horman, Geert Uytterhoeven, Chris Paterson,
Fabrizio Castro, linux-renesas-soc
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v6 3/7] usb: typec: driver for TI HD3SS3220 USB Type-C DRP port controller
2019-05-15 12:09 [PATCH v6 0/7] Add USB3.0 and TI HD3SS3220 driver support Biju Das
2019-05-15 12:09 ` [PATCH v6 1/7] dt-bindings: usb: hd3ss3220 device tree binding document Biju Das
2019-05-15 12:09 ` [PATCH v6 2/7] dt-bindings: usb: renesas_usb3: Document usb role switch support Biju Das
@ 2019-05-15 12:09 ` Biju Das
2019-05-15 12:09 ` [PATCH v6 4/7] usb: gadget: udc: renesas_usb3: Add dual role switch support Biju Das
` (3 subsequent siblings)
6 siblings, 0 replies; 23+ messages in thread
From: Biju Das @ 2019-05-15 12:09 UTC (permalink / raw)
To: Heikki Krogerus, Matthias Brugger, Greg Kroah-Hartman
Cc: Biju Das, Felipe Balbi, Chunfeng Yun, linux-usb, Simon Horman,
Yoshihiro Shimoda, Geert Uytterhoeven, Chris Paterson,
Fabrizio Castro, linux-renesas-soc
Driver for TI HD3SS3220 USB Type-C DRP port controller.
The driver currently registers the port and supports data role swapping.
Signed-off-by: Biju Das <biju.das@bp.renesas.com>
Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
V5-->V6
* No change
Note: This patch depend on [1]
[1]: [v5,4/6] usb: roles: add API to get usb_role_switch by node
https://patchwork.kernel.org/patch/10942499/
V4-->V5
* Incorporated Heikki's review comment
(https://patchwork.kernel.org/patch/10902531/)
* Added Heikki's Reviewed-by tag
V3-->V4
* Incorporated Chunfeng Yun's review comment
* Used fwnode API's to get usb role switch handle.
V2-->V3
* Used the new api "usb_role_switch by node" for getting
remote endpoint associated with Type-C USB DRP port
controller devices.
V1-->V2
* Driver uses usb role class instead of extcon for dual role switch
and also handles connect/disconnect events.
---
drivers/usb/typec/Kconfig | 10 ++
drivers/usb/typec/Makefile | 1 +
drivers/usb/typec/hd3ss3220.c | 263 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 274 insertions(+)
create mode 100644 drivers/usb/typec/hd3ss3220.c
diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
index 89d9193..92a3717 100644
--- a/drivers/usb/typec/Kconfig
+++ b/drivers/usb/typec/Kconfig
@@ -50,6 +50,16 @@ source "drivers/usb/typec/tcpm/Kconfig"
source "drivers/usb/typec/ucsi/Kconfig"
+config TYPEC_HD3SS3220
+ tristate "TI HD3SS3220 Type-C DRP Port controller driver"
+ depends on I2C
+ help
+ Say Y or M here if your system has TI HD3SS3220 Type-C DRP Port
+ controller driver.
+
+ If you choose to build this driver as a dynamically linked module, the
+ module will be called hd3ss3220.ko.
+
config TYPEC_TPS6598X
tristate "TI TPS6598x USB Power Delivery controller driver"
depends on I2C
diff --git a/drivers/usb/typec/Makefile b/drivers/usb/typec/Makefile
index 6696b72..7753a5c3 100644
--- a/drivers/usb/typec/Makefile
+++ b/drivers/usb/typec/Makefile
@@ -4,5 +4,6 @@ typec-y := class.o mux.o bus.o
obj-$(CONFIG_TYPEC) += altmodes/
obj-$(CONFIG_TYPEC_TCPM) += tcpm/
obj-$(CONFIG_TYPEC_UCSI) += ucsi/
+obj-$(CONFIG_TYPEC_HD3SS3220) += hd3ss3220.o
obj-$(CONFIG_TYPEC_TPS6598X) += tps6598x.o
obj-$(CONFIG_TYPEC) += mux/
diff --git a/drivers/usb/typec/hd3ss3220.c b/drivers/usb/typec/hd3ss3220.c
new file mode 100644
index 0000000..7ef7e85
--- /dev/null
+++ b/drivers/usb/typec/hd3ss3220.c
@@ -0,0 +1,263 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * TI HD3SS3220 Type-C DRP Port Controller Driver
+ *
+ * Copyright (C) 2019 Renesas Electronics Corp.
+ */
+
+#include <linux/module.h>
+#include <linux/i2c.h>
+#include <linux/usb/role.h>
+#include <linux/irqreturn.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+#include <linux/usb/typec.h>
+#include <linux/delay.h>
+
+#define HD3SS3220_REG_CN_STAT_CTRL 0x09
+#define HD3SS3220_REG_GEN_CTRL 0x0A
+#define HD3SS3220_REG_DEV_REV 0xA0
+
+/* Register HD3SS3220_REG_CN_STAT_CTRL*/
+#define HD3SS3220_REG_CN_STAT_CTRL_ATTACHED_STATE_MASK (BIT(7) | BIT(6))
+#define HD3SS3220_REG_CN_STAT_CTRL_AS_DFP BIT(6)
+#define HD3SS3220_REG_CN_STAT_CTRL_AS_UFP BIT(7)
+#define HD3SS3220_REG_CN_STAT_CTRL_TO_ACCESSORY (BIT(7) | BIT(6))
+#define HD3SS3220_REG_CN_STAT_CTRL_INT_STATUS BIT(4)
+
+/* Register HD3SS3220_REG_GEN_CTRL*/
+#define HD3SS3220_REG_GEN_CTRL_SRC_PREF_MASK (BIT(2) | BIT(1))
+#define HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_DEFAULT 0x00
+#define HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_TRY_SNK BIT(1)
+#define HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_TRY_SRC (BIT(2) | BIT(1))
+
+struct hd3ss3220 {
+ struct device *dev;
+ struct regmap *regmap;
+ struct usb_role_switch *role_sw;
+ struct typec_port *port;
+ struct typec_capability typec_cap;
+};
+
+static int hd3ss3220_set_source_pref(struct hd3ss3220 *hd3ss3220, int src_pref)
+{
+ return regmap_update_bits(hd3ss3220->regmap, HD3SS3220_REG_GEN_CTRL,
+ HD3SS3220_REG_GEN_CTRL_SRC_PREF_MASK,
+ src_pref);
+}
+
+static enum usb_role hd3ss3220_get_attached_state(struct hd3ss3220 *hd3ss3220)
+{
+ unsigned int reg_val;
+ enum usb_role attached_state;
+ int ret;
+
+ ret = regmap_read(hd3ss3220->regmap, HD3SS3220_REG_CN_STAT_CTRL,
+ ®_val);
+ if (ret < 0)
+ return ret;
+
+ switch (reg_val & HD3SS3220_REG_CN_STAT_CTRL_ATTACHED_STATE_MASK) {
+ case HD3SS3220_REG_CN_STAT_CTRL_AS_DFP:
+ attached_state = USB_ROLE_HOST;
+ break;
+ case HD3SS3220_REG_CN_STAT_CTRL_AS_UFP:
+ attached_state = USB_ROLE_DEVICE;
+ break;
+ default:
+ attached_state = USB_ROLE_NONE;
+ break;
+ }
+
+ return attached_state;
+}
+
+static int hd3ss3220_dr_set(const struct typec_capability *cap,
+ enum typec_data_role role)
+{
+ struct hd3ss3220 *hd3ss3220 = container_of(cap, struct hd3ss3220,
+ typec_cap);
+ enum usb_role role_val;
+ int pref, ret = 0;
+
+ if (role == TYPEC_HOST) {
+ role_val = USB_ROLE_HOST;
+ pref = HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_TRY_SRC;
+ } else {
+ role_val = USB_ROLE_DEVICE;
+ pref = HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_TRY_SNK;
+ }
+
+ ret = hd3ss3220_set_source_pref(hd3ss3220, pref);
+ usleep_range(10, 100);
+
+ usb_role_switch_set_role(hd3ss3220->role_sw, role_val);
+ typec_set_data_role(hd3ss3220->port, role);
+
+ return ret;
+}
+
+static void hd3ss3220_set_role(struct hd3ss3220 *hd3ss3220)
+{
+ enum usb_role role_state = hd3ss3220_get_attached_state(hd3ss3220);
+
+ usb_role_switch_set_role(hd3ss3220->role_sw, role_state);
+ if (role_state == USB_ROLE_NONE)
+ hd3ss3220_set_source_pref(hd3ss3220,
+ HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_DEFAULT);
+
+ switch (role_state) {
+ case USB_ROLE_HOST:
+ typec_set_data_role(hd3ss3220->port, TYPEC_HOST);
+ break;
+ case USB_ROLE_DEVICE:
+ typec_set_data_role(hd3ss3220->port, TYPEC_DEVICE);
+ break;
+ default:
+ break;
+ }
+}
+
+irqreturn_t hd3ss3220_irq(struct hd3ss3220 *hd3ss3220)
+{
+ int err;
+
+ hd3ss3220_set_role(hd3ss3220);
+ err = regmap_update_bits_base(hd3ss3220->regmap,
+ HD3SS3220_REG_CN_STAT_CTRL,
+ HD3SS3220_REG_CN_STAT_CTRL_INT_STATUS,
+ HD3SS3220_REG_CN_STAT_CTRL_INT_STATUS,
+ NULL, false, true);
+ if (err < 0)
+ return IRQ_NONE;
+
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t hd3ss3220_irq_handler(int irq, void *data)
+{
+ struct i2c_client *client = to_i2c_client(data);
+ struct hd3ss3220 *hd3ss3220 = i2c_get_clientdata(client);
+
+ return hd3ss3220_irq(hd3ss3220);
+}
+
+static const struct regmap_config config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .max_register = 0x0A,
+};
+
+static int hd3ss3220_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct hd3ss3220 *hd3ss3220;
+ struct fwnode_handle *parent, *child;
+ int ret;
+ unsigned int data;
+
+ hd3ss3220 = devm_kzalloc(&client->dev, sizeof(struct hd3ss3220),
+ GFP_KERNEL);
+ if (!hd3ss3220)
+ return -ENOMEM;
+
+ i2c_set_clientdata(client, hd3ss3220);
+
+ hd3ss3220->dev = &client->dev;
+ hd3ss3220->regmap = devm_regmap_init_i2c(client, &config);
+ if (IS_ERR(hd3ss3220->regmap))
+ return PTR_ERR(hd3ss3220->regmap);
+
+ hd3ss3220_set_source_pref(hd3ss3220,
+ HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_DEFAULT);
+ child = fwnode_graph_get_next_endpoint(dev_fwnode(hd3ss3220->dev),
+ NULL);
+ parent = fwnode_graph_get_remote_port_parent(child);
+ hd3ss3220->role_sw = fwnode_usb_role_switch_get(parent);
+ if (IS_ERR_OR_NULL(hd3ss3220->role_sw)) {
+ fwnode_handle_put(child);
+ fwnode_handle_put(parent);
+ return PTR_ERR(hd3ss3220->role_sw);
+ }
+
+ fwnode_handle_put(child);
+ fwnode_handle_put(parent);
+
+ hd3ss3220->typec_cap.prefer_role = TYPEC_NO_PREFERRED_ROLE;
+ hd3ss3220->typec_cap.dr_set = hd3ss3220_dr_set;
+ hd3ss3220->typec_cap.type = TYPEC_PORT_DRP;
+ hd3ss3220->typec_cap.data = TYPEC_PORT_DRD;
+
+ hd3ss3220->port = typec_register_port(&client->dev,
+ &hd3ss3220->typec_cap);
+ if (IS_ERR(hd3ss3220->port))
+ return PTR_ERR(hd3ss3220->port);
+
+ hd3ss3220_set_role(hd3ss3220);
+ ret = regmap_read(hd3ss3220->regmap, HD3SS3220_REG_CN_STAT_CTRL, &data);
+ if (ret < 0)
+ goto error;
+
+ if (data & HD3SS3220_REG_CN_STAT_CTRL_INT_STATUS) {
+ ret = regmap_write(hd3ss3220->regmap,
+ HD3SS3220_REG_CN_STAT_CTRL,
+ data | HD3SS3220_REG_CN_STAT_CTRL_INT_STATUS);
+ if (ret < 0)
+ goto error;
+ }
+
+ if (client->irq > 0) {
+ ret = devm_request_threaded_irq(&client->dev, client->irq, NULL,
+ hd3ss3220_irq_handler,
+ IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+ "hd3ss3220", &client->dev);
+ if (ret)
+ goto error;
+ }
+
+ ret = i2c_smbus_read_byte_data(client, HD3SS3220_REG_DEV_REV);
+ if (ret < 0)
+ goto error;
+
+ dev_info(&client->dev, "probed revision=0x%x\n", ret);
+
+ return 0;
+error:
+ typec_unregister_port(hd3ss3220->port);
+ usb_role_switch_put(hd3ss3220->role_sw);
+
+ return ret;
+}
+
+static int hd3ss3220_remove(struct i2c_client *client)
+{
+ struct hd3ss3220 *hd3ss3220 = i2c_get_clientdata(client);
+
+ typec_unregister_port(hd3ss3220->port);
+ usb_role_switch_put(hd3ss3220->role_sw);
+
+ return 0;
+}
+
+static const struct of_device_id dev_ids[] = {
+ { .compatible = "ti,hd3ss3220"},
+ {}
+};
+MODULE_DEVICE_TABLE(of, dev_ids);
+
+static struct i2c_driver hd3ss3220_driver = {
+ .driver = {
+ .name = "hd3ss3220",
+ .of_match_table = of_match_ptr(dev_ids),
+ },
+ .probe = hd3ss3220_probe,
+ .remove = hd3ss3220_remove,
+};
+
+module_i2c_driver(hd3ss3220_driver);
+
+MODULE_AUTHOR("Biju Das <biju.das@bp.renesas.com>");
+MODULE_DESCRIPTION("TI HD3SS3220 DRP Port Controller Driver");
+MODULE_LICENSE("GPL");
--
2.7.4
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v6 4/7] usb: gadget: udc: renesas_usb3: Add dual role switch support
2019-05-15 12:09 [PATCH v6 0/7] Add USB3.0 and TI HD3SS3220 driver support Biju Das
` (2 preceding siblings ...)
2019-05-15 12:09 ` [PATCH v6 3/7] usb: typec: driver for TI HD3SS3220 USB Type-C DRP port controller Biju Das
@ 2019-05-15 12:09 ` Biju Das
2019-05-16 3:16 ` Chunfeng Yun
2019-05-21 5:49 ` Yoshihiro Shimoda
2019-05-15 12:09 ` [PATCH v6 5/7] arm64: defconfig: enable TYPEC_HD3SS3220 config option Biju Das
` (2 subsequent siblings)
6 siblings, 2 replies; 23+ messages in thread
From: Biju Das @ 2019-05-15 12:09 UTC (permalink / raw)
To: Felipe Balbi, Greg Kroah-Hartman, Yoshihiro Shimoda
Cc: Biju Das, Heikki Krogerus, Simon Horman, Fabrizio Castro,
Kees Cook, linux-usb, Simon Horman, Geert Uytterhoeven,
Chris Paterson, linux-renesas-soc
The RZ/G2E cat874 board has a type-c connector connected to hd3ss3220 usb
type-c drp port controller. This patch adds dual role switch support for
the type-c connector using the usb role switch class framework.
Signed-off-by: Biju Das <biju.das@bp.renesas.com>
---
V5-->V6
* Added graph api's to find the role supported by the connector.
V4-->V5
* Incorporated Shimoda-san's review comment
(https://patchwork.kernel.org/patch/10902537/)
V3-->V4
* No Change
V2-->V3
* Incorporated Shimoda-san's review comment
(https://patchwork.kernel.org/patch/10852507/)
* Used renesas,usb-role-switch property for differentiating USB
role switch associated with Type-C port controller driver.
V1-->V2
* Driver uses usb role clas for handling dual role switch and handling
connect/disconnect events instead of extcon.
---
drivers/usb/gadget/udc/renesas_usb3.c | 121 ++++++++++++++++++++++++++++++++--
1 file changed, 114 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/gadget/udc/renesas_usb3.c b/drivers/usb/gadget/udc/renesas_usb3.c
index 7dc2485..1d41998 100644
--- a/drivers/usb/gadget/udc/renesas_usb3.c
+++ b/drivers/usb/gadget/udc/renesas_usb3.c
@@ -24,6 +24,7 @@
#include <linux/usb/ch9.h>
#include <linux/usb/gadget.h>
#include <linux/usb/of.h>
+#include <linux/of_graph.h>
#include <linux/usb/role.h>
/* register definitions */
@@ -351,6 +352,8 @@ struct renesas_usb3 {
int disabled_count;
struct usb_request *ep0_req;
+
+ enum usb_role connection_state;
u16 test_mode;
u8 ep0_buf[USB3_EP0_BUF_SIZE];
bool softconnect;
@@ -359,6 +362,7 @@ struct renesas_usb3 {
bool extcon_usb; /* check vbus and set EXTCON_USB */
bool forced_b_device;
bool start_to_connect;
+ bool dual_role_sw;
};
#define gadget_to_renesas_usb3(_gadget) \
@@ -699,8 +703,10 @@ static void usb3_mode_config(struct renesas_usb3 *usb3, bool host, bool a_dev)
unsigned long flags;
spin_lock_irqsave(&usb3->lock, flags);
- usb3_set_mode_by_role_sw(usb3, host);
- usb3_vbus_out(usb3, a_dev);
+ if (!usb3->dual_role_sw || usb3->connection_state != USB_ROLE_NONE) {
+ usb3_set_mode_by_role_sw(usb3, host);
+ usb3_vbus_out(usb3, a_dev);
+ }
/* for A-Peripheral or forced B-device mode */
if ((!host && a_dev) || usb3->start_to_connect)
usb3_connect(usb3);
@@ -716,7 +722,8 @@ static void usb3_check_id(struct renesas_usb3 *usb3)
{
usb3->extcon_host = usb3_is_a_device(usb3);
- if (usb3->extcon_host && !usb3->forced_b_device)
+ if ((!usb3->dual_role_sw && usb3->extcon_host && !usb3->forced_b_device)
+ || usb3->connection_state == USB_ROLE_HOST)
usb3_mode_config(usb3, true, true);
else
usb3_mode_config(usb3, false, false);
@@ -2343,14 +2350,65 @@ static enum usb_role renesas_usb3_role_switch_get(struct device *dev)
return cur_role;
}
-static int renesas_usb3_role_switch_set(struct device *dev,
- enum usb_role role)
+static void handle_ext_role_switch_states(struct device *dev,
+ enum usb_role role)
+{
+ struct renesas_usb3 *usb3 = dev_get_drvdata(dev);
+ struct device *host = usb3->host_dev;
+ enum usb_role cur_role = renesas_usb3_role_switch_get(dev);
+
+ switch (role) {
+ case USB_ROLE_NONE:
+ usb3->connection_state = USB_ROLE_NONE;
+ if (usb3->driver)
+ usb3_disconnect(usb3);
+ usb3_vbus_out(usb3, false);
+ break;
+ case USB_ROLE_DEVICE:
+ if (usb3->connection_state == USB_ROLE_NONE) {
+ usb3->connection_state = USB_ROLE_DEVICE;
+ usb3_set_mode(usb3, false);
+ if (usb3->driver)
+ usb3_connect(usb3);
+ } else if (cur_role == USB_ROLE_HOST) {
+ device_release_driver(host);
+ usb3_set_mode(usb3, false);
+ if (usb3->driver)
+ usb3_connect(usb3);
+ }
+ usb3_vbus_out(usb3, false);
+ break;
+ case USB_ROLE_HOST:
+ if (usb3->connection_state == USB_ROLE_NONE) {
+ if (usb3->driver)
+ usb3_disconnect(usb3);
+
+ usb3->connection_state = USB_ROLE_HOST;
+ usb3_set_mode(usb3, true);
+ usb3_vbus_out(usb3, true);
+ if (device_attach(host) < 0)
+ dev_err(dev, "device_attach(host) failed\n");
+ } else if (cur_role == USB_ROLE_DEVICE) {
+ usb3_disconnect(usb3);
+ /* Must set the mode before device_attach of the host */
+ usb3_set_mode(usb3, true);
+ /* This device_attach() might sleep */
+ if (device_attach(host) < 0)
+ dev_err(dev, "device_attach(host) failed\n");
+ }
+ break;
+ default:
+ break;
+ }
+}
+
+static void handle_role_switch_states(struct device *dev,
+ enum usb_role role)
{
struct renesas_usb3 *usb3 = dev_get_drvdata(dev);
struct device *host = usb3->host_dev;
enum usb_role cur_role = renesas_usb3_role_switch_get(dev);
- pm_runtime_get_sync(dev);
if (cur_role == USB_ROLE_HOST && role == USB_ROLE_DEVICE) {
device_release_driver(host);
usb3_set_mode(usb3, false);
@@ -2361,6 +2419,20 @@ static int renesas_usb3_role_switch_set(struct device *dev,
if (device_attach(host) < 0)
dev_err(dev, "device_attach(host) failed\n");
}
+}
+
+static int renesas_usb3_role_switch_set(struct device *dev,
+ enum usb_role role)
+{
+ struct renesas_usb3 *usb3 = dev_get_drvdata(dev);
+
+ pm_runtime_get_sync(dev);
+
+ if (usb3->dual_role_sw)
+ handle_ext_role_switch_states(dev, role);
+ else
+ handle_role_switch_states(dev, role);
+
pm_runtime_put(dev);
return 0;
@@ -2650,12 +2722,41 @@ static const unsigned int renesas_usb3_cable[] = {
EXTCON_NONE,
};
-static const struct usb_role_switch_desc renesas_usb3_role_switch_desc = {
+static struct usb_role_switch_desc renesas_usb3_role_switch_desc = {
.set = renesas_usb3_role_switch_set,
.get = renesas_usb3_role_switch_get,
.allow_userspace_control = true,
};
+static bool is_usb_dual_role_switch(struct device *dev)
+{
+ struct device_node *np = dev->of_node;
+ struct device_node *parent;
+ struct device_node *child;
+ bool ret = false;
+ const char *role_type = NULL;
+
+ child = of_graph_get_endpoint_by_regs(np, -1, -1);
+ if (!child)
+ return ret;
+
+ parent = of_graph_get_remote_port_parent(child);
+ of_node_put(child);
+ child = of_get_child_by_name(parent, "connector");
+ of_node_put(parent);
+ if (!child)
+ return ret;
+
+ if (of_device_is_compatible(child, "usb-c-connector")) {
+ of_property_read_string(child, "data-role", &role_type);
+ if (role_type && (!strncmp(role_type, "dual", strlen("dual"))))
+ ret = true;
+ }
+
+ of_node_put(child);
+ return ret;
+}
+
static int renesas_usb3_probe(struct platform_device *pdev)
{
struct renesas_usb3 *usb3;
@@ -2741,6 +2842,12 @@ static int renesas_usb3_probe(struct platform_device *pdev)
if (ret < 0)
goto err_dev_create;
+ if (device_property_read_bool(&pdev->dev, "usb-role-switch") &&
+ is_usb_dual_role_switch(&pdev->dev)) {
+ usb3->dual_role_sw = true;
+ renesas_usb3_role_switch_desc.fwnode = dev_fwnode(&pdev->dev);
+ }
+
INIT_WORK(&usb3->role_work, renesas_usb3_role_work);
usb3->role_sw = usb_role_switch_register(&pdev->dev,
&renesas_usb3_role_switch_desc);
--
2.7.4
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v6 4/7] usb: gadget: udc: renesas_usb3: Add dual role switch support
2019-05-15 12:09 ` [PATCH v6 4/7] usb: gadget: udc: renesas_usb3: Add dual role switch support Biju Das
@ 2019-05-16 3:16 ` Chunfeng Yun
2019-05-16 8:00 ` Biju Das
2019-05-21 5:49 ` Yoshihiro Shimoda
1 sibling, 1 reply; 23+ messages in thread
From: Chunfeng Yun @ 2019-05-16 3:16 UTC (permalink / raw)
To: Biju Das
Cc: Felipe Balbi, Greg Kroah-Hartman, Yoshihiro Shimoda,
Heikki Krogerus, Simon Horman, Fabrizio Castro, Kees Cook,
linux-usb, Simon Horman, Geert Uytterhoeven, Chris Paterson,
linux-renesas-soc
On Wed, 2019-05-15 at 13:09 +0100, Biju Das wrote:
> The RZ/G2E cat874 board has a type-c connector connected to hd3ss3220 usb
> type-c drp port controller. This patch adds dual role switch support for
> the type-c connector using the usb role switch class framework.
>
> Signed-off-by: Biju Das <biju.das@bp.renesas.com>
> ---
> V5-->V6
> * Added graph api's to find the role supported by the connector.
> V4-->V5
> * Incorporated Shimoda-san's review comment
> (https://patchwork.kernel.org/patch/10902537/)
> V3-->V4
> * No Change
> V2-->V3
> * Incorporated Shimoda-san's review comment
> (https://patchwork.kernel.org/patch/10852507/)
> * Used renesas,usb-role-switch property for differentiating USB
> role switch associated with Type-C port controller driver.
> V1-->V2
> * Driver uses usb role clas for handling dual role switch and handling
> connect/disconnect events instead of extcon.
> ---
> drivers/usb/gadget/udc/renesas_usb3.c | 121 ++++++++++++++++++++++++++++++++--
> 1 file changed, 114 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/usb/gadget/udc/renesas_usb3.c b/drivers/usb/gadget/udc/renesas_usb3.c
> index 7dc2485..1d41998 100644
> --- a/drivers/usb/gadget/udc/renesas_usb3.c
> +++ b/drivers/usb/gadget/udc/renesas_usb3.c
> @@ -24,6 +24,7 @@
> #include <linux/usb/ch9.h>
> #include <linux/usb/gadget.h>
> #include <linux/usb/of.h>
> +#include <linux/of_graph.h>
> #include <linux/usb/role.h>
>
> /* register definitions */
> @@ -351,6 +352,8 @@ struct renesas_usb3 {
> int disabled_count;
>
> struct usb_request *ep0_req;
> +
> + enum usb_role connection_state;
> u16 test_mode;
> u8 ep0_buf[USB3_EP0_BUF_SIZE];
> bool softconnect;
> @@ -359,6 +362,7 @@ struct renesas_usb3 {
> bool extcon_usb; /* check vbus and set EXTCON_USB */
> bool forced_b_device;
> bool start_to_connect;
> + bool dual_role_sw;
> };
>
> #define gadget_to_renesas_usb3(_gadget) \
> @@ -699,8 +703,10 @@ static void usb3_mode_config(struct renesas_usb3 *usb3, bool host, bool a_dev)
> unsigned long flags;
>
> spin_lock_irqsave(&usb3->lock, flags);
> - usb3_set_mode_by_role_sw(usb3, host);
> - usb3_vbus_out(usb3, a_dev);
> + if (!usb3->dual_role_sw || usb3->connection_state != USB_ROLE_NONE) {
> + usb3_set_mode_by_role_sw(usb3, host);
> + usb3_vbus_out(usb3, a_dev);
> + }
> /* for A-Peripheral or forced B-device mode */
> if ((!host && a_dev) || usb3->start_to_connect)
> usb3_connect(usb3);
> @@ -716,7 +722,8 @@ static void usb3_check_id(struct renesas_usb3 *usb3)
> {
> usb3->extcon_host = usb3_is_a_device(usb3);
>
> - if (usb3->extcon_host && !usb3->forced_b_device)
> + if ((!usb3->dual_role_sw && usb3->extcon_host && !usb3->forced_b_device)
> + || usb3->connection_state == USB_ROLE_HOST)
> usb3_mode_config(usb3, true, true);
> else
> usb3_mode_config(usb3, false, false);
> @@ -2343,14 +2350,65 @@ static enum usb_role renesas_usb3_role_switch_get(struct device *dev)
> return cur_role;
> }
>
> -static int renesas_usb3_role_switch_set(struct device *dev,
> - enum usb_role role)
> +static void handle_ext_role_switch_states(struct device *dev,
> + enum usb_role role)
> +{
> + struct renesas_usb3 *usb3 = dev_get_drvdata(dev);
> + struct device *host = usb3->host_dev;
> + enum usb_role cur_role = renesas_usb3_role_switch_get(dev);
> +
> + switch (role) {
> + case USB_ROLE_NONE:
> + usb3->connection_state = USB_ROLE_NONE;
> + if (usb3->driver)
> + usb3_disconnect(usb3);
> + usb3_vbus_out(usb3, false);
> + break;
> + case USB_ROLE_DEVICE:
> + if (usb3->connection_state == USB_ROLE_NONE) {
> + usb3->connection_state = USB_ROLE_DEVICE;
> + usb3_set_mode(usb3, false);
> + if (usb3->driver)
> + usb3_connect(usb3);
> + } else if (cur_role == USB_ROLE_HOST) {
> + device_release_driver(host);
> + usb3_set_mode(usb3, false);
> + if (usb3->driver)
> + usb3_connect(usb3);
> + }
> + usb3_vbus_out(usb3, false);
> + break;
> + case USB_ROLE_HOST:
> + if (usb3->connection_state == USB_ROLE_NONE) {
> + if (usb3->driver)
> + usb3_disconnect(usb3);
> +
> + usb3->connection_state = USB_ROLE_HOST;
> + usb3_set_mode(usb3, true);
> + usb3_vbus_out(usb3, true);
> + if (device_attach(host) < 0)
> + dev_err(dev, "device_attach(host) failed\n");
> + } else if (cur_role == USB_ROLE_DEVICE) {
> + usb3_disconnect(usb3);
> + /* Must set the mode before device_attach of the host */
> + usb3_set_mode(usb3, true);
> + /* This device_attach() might sleep */
> + if (device_attach(host) < 0)
> + dev_err(dev, "device_attach(host) failed\n");
> + }
> + break;
> + default:
> + break;
> + }
> +}
> +
> +static void handle_role_switch_states(struct device *dev,
> + enum usb_role role)
> {
> struct renesas_usb3 *usb3 = dev_get_drvdata(dev);
> struct device *host = usb3->host_dev;
> enum usb_role cur_role = renesas_usb3_role_switch_get(dev);
>
> - pm_runtime_get_sync(dev);
> if (cur_role == USB_ROLE_HOST && role == USB_ROLE_DEVICE) {
> device_release_driver(host);
> usb3_set_mode(usb3, false);
> @@ -2361,6 +2419,20 @@ static int renesas_usb3_role_switch_set(struct device *dev,
> if (device_attach(host) < 0)
> dev_err(dev, "device_attach(host) failed\n");
> }
> +}
> +
> +static int renesas_usb3_role_switch_set(struct device *dev,
> + enum usb_role role)
> +{
> + struct renesas_usb3 *usb3 = dev_get_drvdata(dev);
> +
> + pm_runtime_get_sync(dev);
> +
> + if (usb3->dual_role_sw)
> + handle_ext_role_switch_states(dev, role);
> + else
> + handle_role_switch_states(dev, role);
> +
> pm_runtime_put(dev);
>
> return 0;
> @@ -2650,12 +2722,41 @@ static const unsigned int renesas_usb3_cable[] = {
> EXTCON_NONE,
> };
>
> -static const struct usb_role_switch_desc renesas_usb3_role_switch_desc = {
> +static struct usb_role_switch_desc renesas_usb3_role_switch_desc = {
> .set = renesas_usb3_role_switch_set,
> .get = renesas_usb3_role_switch_get,
> .allow_userspace_control = true,
> };
>
> +static bool is_usb_dual_role_switch(struct device *dev)
To me, it's not good idea to pay an attention to specific consumer of
the role switch, assume any device could assign role if it get this USB
role switch, not only type-c connector
> +{
> + struct device_node *np = dev->of_node;
> + struct device_node *parent;
> + struct device_node *child;
> + bool ret = false;
> + const char *role_type = NULL;
> +
> + child = of_graph_get_endpoint_by_regs(np, -1, -1);
> + if (!child)
> + return ret;
> +
> + parent = of_graph_get_remote_port_parent(child);
> + of_node_put(child);
> + child = of_get_child_by_name(parent, "connector");
> + of_node_put(parent);
> + if (!child)
> + return ret;
> +
> + if (of_device_is_compatible(child, "usb-c-connector")) {
> + of_property_read_string(child, "data-role", &role_type);
> + if (role_type && (!strncmp(role_type, "dual", strlen("dual"))))
> + ret = true;
> + }
> +
> + of_node_put(child);
> + return ret;
> +}
> +
> static int renesas_usb3_probe(struct platform_device *pdev)
> {
> struct renesas_usb3 *usb3;
> @@ -2741,6 +2842,12 @@ static int renesas_usb3_probe(struct platform_device *pdev)
> if (ret < 0)
> goto err_dev_create;
>
> + if (device_property_read_bool(&pdev->dev, "usb-role-switch") &&
> + is_usb_dual_role_switch(&pdev->dev)) {
> + usb3->dual_role_sw = true;
> + renesas_usb3_role_switch_desc.fwnode = dev_fwnode(&pdev->dev);
> + }
> +
> INIT_WORK(&usb3->role_work, renesas_usb3_role_work);
> usb3->role_sw = usb_role_switch_register(&pdev->dev,
> &renesas_usb3_role_switch_desc);
^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: [PATCH v6 4/7] usb: gadget: udc: renesas_usb3: Add dual role switch support
2019-05-16 3:16 ` Chunfeng Yun
@ 2019-05-16 8:00 ` Biju Das
0 siblings, 0 replies; 23+ messages in thread
From: Biju Das @ 2019-05-16 8:00 UTC (permalink / raw)
To: Chunfeng Yun, Rob Herring
Cc: Felipe Balbi, Greg Kroah-Hartman, Yoshihiro Shimoda,
Heikki Krogerus, Simon Horman, Fabrizio Castro, Kees Cook,
linux-usb, Simon Horman, Geert Uytterhoeven, Chris Paterson,
linux-renesas-soc
+ Rob
Hi Chunfeng Yun,
Thanks for the feedback.
> Subject: Re: [PATCH v6 4/7] usb: gadget: udc: renesas_usb3: Add dual role
> switch support
>
> On Wed, 2019-05-15 at 13:09 +0100, Biju Das wrote:
> > The RZ/G2E cat874 board has a type-c connector connected to hd3ss3220
> > usb type-c drp port controller. This patch adds dual role switch
> > support for the type-c connector using the usb role switch class framework.
> >
> > Signed-off-by: Biju Das <biju.das@bp.renesas.com>
> > ---
> > V5-->V6
> > * Added graph api's to find the role supported by the connector.
> > V4-->V5
> > * Incorporated Shimoda-san's review comment
> > (https://patchwork.kernel.org/patch/10902537/)
> > V3-->V4
> > * No Change
> > V2-->V3
> > * Incorporated Shimoda-san's review comment
> > (https://patchwork.kernel.org/patch/10852507/)
> > * Used renesas,usb-role-switch property for differentiating USB
> > role switch associated with Type-C port controller driver.
> > V1-->V2
> > * Driver uses usb role clas for handling dual role switch and handling
> > connect/disconnect events instead of extcon.
> > ---
> > drivers/usb/gadget/udc/renesas_usb3.c | 121
> > ++++++++++++++++++++++++++++++++--
> > 1 file changed, 114 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/usb/gadget/udc/renesas_usb3.c
> > b/drivers/usb/gadget/udc/renesas_usb3.c
> > index 7dc2485..1d41998 100644
> > --- a/drivers/usb/gadget/udc/renesas_usb3.c
> > +++ b/drivers/usb/gadget/udc/renesas_usb3.c
> > @@ -24,6 +24,7 @@
> > #include <linux/usb/ch9.h>
> > #include <linux/usb/gadget.h>
> > #include <linux/usb/of.h>
> > +#include <linux/of_graph.h>
> > #include <linux/usb/role.h>
> >
> > /* register definitions */
> > @@ -351,6 +352,8 @@ struct renesas_usb3 {
> > int disabled_count;
> >
> > struct usb_request *ep0_req;
> > +
> > + enum usb_role connection_state;
> > u16 test_mode;
> > u8 ep0_buf[USB3_EP0_BUF_SIZE];
> > bool softconnect;
> > @@ -359,6 +362,7 @@ struct renesas_usb3 {
> > bool extcon_usb; /* check vbus and set EXTCON_USB
> */
> > bool forced_b_device;
> > bool start_to_connect;
> > + bool dual_role_sw;
> > };
> >
> > #define gadget_to_renesas_usb3(_gadget) \
> > @@ -699,8 +703,10 @@ static void usb3_mode_config(struct renesas_usb3
> *usb3, bool host, bool a_dev)
> > unsigned long flags;
> >
> > spin_lock_irqsave(&usb3->lock, flags);
> > - usb3_set_mode_by_role_sw(usb3, host);
> > - usb3_vbus_out(usb3, a_dev);
> > + if (!usb3->dual_role_sw || usb3->connection_state !=
> USB_ROLE_NONE) {
> > + usb3_set_mode_by_role_sw(usb3, host);
> > + usb3_vbus_out(usb3, a_dev);
> > + }
> > /* for A-Peripheral or forced B-device mode */
> > if ((!host && a_dev) || usb3->start_to_connect)
> > usb3_connect(usb3);
> > @@ -716,7 +722,8 @@ static void usb3_check_id(struct renesas_usb3
> > *usb3) {
> > usb3->extcon_host = usb3_is_a_device(usb3);
> >
> > - if (usb3->extcon_host && !usb3->forced_b_device)
> > + if ((!usb3->dual_role_sw && usb3->extcon_host && !usb3-
> >forced_b_device)
> > + || usb3->connection_state == USB_ROLE_HOST)
> > usb3_mode_config(usb3, true, true);
> > else
> > usb3_mode_config(usb3, false, false); @@ -2343,14 +2350,65
> @@
> > static enum usb_role renesas_usb3_role_switch_get(struct device *dev)
> > return cur_role;
> > }
> >
> > -static int renesas_usb3_role_switch_set(struct device *dev,
> > - enum usb_role role)
> > +static void handle_ext_role_switch_states(struct device *dev,
> > + enum usb_role role)
> > +{
> > + struct renesas_usb3 *usb3 = dev_get_drvdata(dev);
> > + struct device *host = usb3->host_dev;
> > + enum usb_role cur_role = renesas_usb3_role_switch_get(dev);
> > +
> > + switch (role) {
> > + case USB_ROLE_NONE:
> > + usb3->connection_state = USB_ROLE_NONE;
> > + if (usb3->driver)
> > + usb3_disconnect(usb3);
> > + usb3_vbus_out(usb3, false);
> > + break;
> > + case USB_ROLE_DEVICE:
> > + if (usb3->connection_state == USB_ROLE_NONE) {
> > + usb3->connection_state = USB_ROLE_DEVICE;
> > + usb3_set_mode(usb3, false);
> > + if (usb3->driver)
> > + usb3_connect(usb3);
> > + } else if (cur_role == USB_ROLE_HOST) {
> > + device_release_driver(host);
> > + usb3_set_mode(usb3, false);
> > + if (usb3->driver)
> > + usb3_connect(usb3);
> > + }
> > + usb3_vbus_out(usb3, false);
> > + break;
> > + case USB_ROLE_HOST:
> > + if (usb3->connection_state == USB_ROLE_NONE) {
> > + if (usb3->driver)
> > + usb3_disconnect(usb3);
> > +
> > + usb3->connection_state = USB_ROLE_HOST;
> > + usb3_set_mode(usb3, true);
> > + usb3_vbus_out(usb3, true);
> > + if (device_attach(host) < 0)
> > + dev_err(dev, "device_attach(host)
> failed\n");
> > + } else if (cur_role == USB_ROLE_DEVICE) {
> > + usb3_disconnect(usb3);
> > + /* Must set the mode before device_attach of the
> host */
> > + usb3_set_mode(usb3, true);
> > + /* This device_attach() might sleep */
> > + if (device_attach(host) < 0)
> > + dev_err(dev, "device_attach(host)
> failed\n");
> > + }
> > + break;
> > + default:
> > + break;
> > + }
> > +}
> > +
> > +static void handle_role_switch_states(struct device *dev,
> > + enum usb_role role)
> > {
> > struct renesas_usb3 *usb3 = dev_get_drvdata(dev);
> > struct device *host = usb3->host_dev;
> > enum usb_role cur_role = renesas_usb3_role_switch_get(dev);
> >
> > - pm_runtime_get_sync(dev);
> > if (cur_role == USB_ROLE_HOST && role == USB_ROLE_DEVICE) {
> > device_release_driver(host);
> > usb3_set_mode(usb3, false);
> > @@ -2361,6 +2419,20 @@ static int renesas_usb3_role_switch_set(struct
> device *dev,
> > if (device_attach(host) < 0)
> > dev_err(dev, "device_attach(host) failed\n");
> > }
> > +}
> > +
> > +static int renesas_usb3_role_switch_set(struct device *dev,
> > + enum usb_role role)
> > +{
> > + struct renesas_usb3 *usb3 = dev_get_drvdata(dev);
> > +
> > + pm_runtime_get_sync(dev);
> > +
> > + if (usb3->dual_role_sw)
> > + handle_ext_role_switch_states(dev, role);
> > + else
> > + handle_role_switch_states(dev, role);
> > +
> > pm_runtime_put(dev);
> >
> > return 0;
> > @@ -2650,12 +2722,41 @@ static const unsigned int renesas_usb3_cable[]
> = {
> > EXTCON_NONE,
> > };
> >
> > -static const struct usb_role_switch_desc
> > renesas_usb3_role_switch_desc = {
> > +static struct usb_role_switch_desc renesas_usb3_role_switch_desc = {
> > .set = renesas_usb3_role_switch_set,
> > .get = renesas_usb3_role_switch_get,
> > .allow_userspace_control = true,
> > };
> >
> > +static bool is_usb_dual_role_switch(struct device *dev)
> To me, it's not good idea to pay an attention to specific consumer of the role
> switch, assume any device could assign role if it get this USB role switch, not
> only type-c connector
Yes, I agree.
I have previously posted a patch based on this [1].
[1]. https://patchwork.kernel.org/patch/10852505/
Then on the binding patch, Rob suggested to walk the graph to the connector
and determine if dual role is supported by the connector type [2]
[2]. https://patchwork.kernel.org/patch/10914379/
The purpose of "usb role switch" is to assign the roles. So looks like, we don't need this function.
Please correct me, if I am wrong.
Regards,
Biju
> > +{
> > + struct device_node *np = dev->of_node;
> > + struct device_node *parent;
> > + struct device_node *child;
> > + bool ret = false;
> > + const char *role_type = NULL;
> > +
> > + child = of_graph_get_endpoint_by_regs(np, -1, -1);
> > + if (!child)
> > + return ret;
> > +
> > + parent = of_graph_get_remote_port_parent(child);
> > + of_node_put(child);
> > + child = of_get_child_by_name(parent, "connector");
> > + of_node_put(parent);
> > + if (!child)
> > + return ret;
> > +
> > + if (of_device_is_compatible(child, "usb-c-connector")) {
> > + of_property_read_string(child, "data-role", &role_type);
> > + if (role_type && (!strncmp(role_type, "dual",
> strlen("dual"))))
> > + ret = true;
> > + }
> > +
> > + of_node_put(child);
> > + return ret;
> > +}
> > +
> > static int renesas_usb3_probe(struct platform_device *pdev) {
> > struct renesas_usb3 *usb3;
> > @@ -2741,6 +2842,12 @@ static int renesas_usb3_probe(struct
> platform_device *pdev)
> > if (ret < 0)
> > goto err_dev_create;
> >
> > + if (device_property_read_bool(&pdev->dev, "usb-role-switch") &&
> > + is_usb_dual_role_switch(&pdev->dev)) {
> > + usb3->dual_role_sw = true;
> > + renesas_usb3_role_switch_desc.fwnode =
> dev_fwnode(&pdev->dev);
> > + }
> > +
> > INIT_WORK(&usb3->role_work, renesas_usb3_role_work);
> > usb3->role_sw = usb_role_switch_register(&pdev->dev,
> > &renesas_usb3_role_switch_desc);
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: [PATCH v6 4/7] usb: gadget: udc: renesas_usb3: Add dual role switch support
2019-05-15 12:09 ` [PATCH v6 4/7] usb: gadget: udc: renesas_usb3: Add dual role switch support Biju Das
2019-05-16 3:16 ` Chunfeng Yun
@ 2019-05-21 5:49 ` Yoshihiro Shimoda
2019-05-21 7:10 ` Biju Das
1 sibling, 1 reply; 23+ messages in thread
From: Yoshihiro Shimoda @ 2019-05-21 5:49 UTC (permalink / raw)
To: Biju Das, Felipe Balbi, Greg Kroah-Hartman
Cc: Biju Das, Heikki Krogerus, Simon Horman, Fabrizio Castro,
Kees Cook, linux-usb, Simon Horman, Geert Uytterhoeven,
Chris Paterson, linux-renesas-soc
Hi Biju-san,
Thank you for the patch!
> From: Biju Das, Sent: Wednesday, May 15, 2019 9:09 PM
> Subject: [PATCH v6 4/7] usb: gadget: udc: renesas_usb3: Add dual role switch support
Now I'm confusing about the "Add dual role switch support" mean... Especially,
this driver has already supports dual role switch support by own sysfs or debugfs.
> The RZ/G2E cat874 board has a type-c connector connected to hd3ss3220 usb
> type-c drp port controller. This patch adds dual role switch support for
> the type-c connector using the usb role switch class framework.
IIUC, after this patch is applied, the hs3ss3220 type-c driver can switch
the role by using the usb role switch class framework.
> Signed-off-by: Biju Das <biju.das@bp.renesas.com>
> ---
> V5-->V6
> * Added graph api's to find the role supported by the connector.
> V4-->V5
> * Incorporated Shimoda-san's review comment
> (https://patchwork.kernel.org/patch/10902537/)
> V3-->V4
> * No Change
> V2-->V3
> * Incorporated Shimoda-san's review comment
> (https://patchwork.kernel.org/patch/10852507/)
> * Used renesas,usb-role-switch property for differentiating USB
> role switch associated with Type-C port controller driver.
> V1-->V2
> * Driver uses usb role clas for handling dual role switch and handling
> connect/disconnect events instead of extcon.
> ---
> drivers/usb/gadget/udc/renesas_usb3.c | 121 ++++++++++++++++++++++++++++++++--
> 1 file changed, 114 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/usb/gadget/udc/renesas_usb3.c b/drivers/usb/gadget/udc/renesas_usb3.c
> index 7dc2485..1d41998 100644
> --- a/drivers/usb/gadget/udc/renesas_usb3.c
> +++ b/drivers/usb/gadget/udc/renesas_usb3.c
> @@ -24,6 +24,7 @@
> #include <linux/usb/ch9.h>
> #include <linux/usb/gadget.h>
> #include <linux/usb/of.h>
> +#include <linux/of_graph.h>
> #include <linux/usb/role.h>
>
> /* register definitions */
> @@ -351,6 +352,8 @@ struct renesas_usb3 {
> int disabled_count;
>
> struct usb_request *ep0_req;
> +
> + enum usb_role connection_state;
> u16 test_mode;
> u8 ep0_buf[USB3_EP0_BUF_SIZE];
> bool softconnect;
> @@ -359,6 +362,7 @@ struct renesas_usb3 {
> bool extcon_usb; /* check vbus and set EXTCON_USB */
> bool forced_b_device;
> bool start_to_connect;
> + bool dual_role_sw;
I'm also confusing this name.
> };
>
> #define gadget_to_renesas_usb3(_gadget) \
<snip>
> @@ -699,8 +703,10 @@ static void usb3_mode_config(struct renesas_usb3 *usb3, bool host, bool a_dev)
> unsigned long flags;
>
> spin_lock_irqsave(&usb3->lock, flags);
> - usb3_set_mode_by_role_sw(usb3, host);
> - usb3_vbus_out(usb3, a_dev);
> + if (!usb3->dual_role_sw || usb3->connection_state != USB_ROLE_NONE) {
> + usb3_set_mode_by_role_sw(usb3, host);
> + usb3_vbus_out(usb3, a_dev);
> + }
> /* for A-Peripheral or forced B-device mode */
> if ((!host && a_dev) || usb3->start_to_connect)
> usb3_connect(usb3);
> @@ -716,7 +722,8 @@ static void usb3_check_id(struct renesas_usb3 *usb3)
> {
> usb3->extcon_host = usb3_is_a_device(usb3);
>
> - if (usb3->extcon_host && !usb3->forced_b_device)
> + if ((!usb3->dual_role_sw && usb3->extcon_host && !usb3->forced_b_device)
> + || usb3->connection_state == USB_ROLE_HOST)
> usb3_mode_config(usb3, true, true);
> else
> usb3_mode_config(usb3, false, false);
> @@ -2343,14 +2350,65 @@ static enum usb_role renesas_usb3_role_switch_get(struct device *dev)
> return cur_role;
> }
>
> -static int renesas_usb3_role_switch_set(struct device *dev,
> - enum usb_role role)
> +static void handle_ext_role_switch_states(struct device *dev,
> + enum usb_role role)
> +{
> + struct renesas_usb3 *usb3 = dev_get_drvdata(dev);
> + struct device *host = usb3->host_dev;
> + enum usb_role cur_role = renesas_usb3_role_switch_get(dev);
> +
> + switch (role) {
> + case USB_ROLE_NONE:
> + usb3->connection_state = USB_ROLE_NONE;
> + if (usb3->driver)
> + usb3_disconnect(usb3);
> + usb3_vbus_out(usb3, false);
> + break;
> + case USB_ROLE_DEVICE:
> + if (usb3->connection_state == USB_ROLE_NONE) {
> + usb3->connection_state = USB_ROLE_DEVICE;
> + usb3_set_mode(usb3, false);
> + if (usb3->driver)
> + usb3_connect(usb3);
> + } else if (cur_role == USB_ROLE_HOST) {
> + device_release_driver(host);
> + usb3_set_mode(usb3, false);
> + if (usb3->driver)
> + usb3_connect(usb3);
> + }
> + usb3_vbus_out(usb3, false);
> + break;
> + case USB_ROLE_HOST:
> + if (usb3->connection_state == USB_ROLE_NONE) {
> + if (usb3->driver)
> + usb3_disconnect(usb3);
> +
> + usb3->connection_state = USB_ROLE_HOST;
> + usb3_set_mode(usb3, true);
> + usb3_vbus_out(usb3, true);
> + if (device_attach(host) < 0)
> + dev_err(dev, "device_attach(host) failed\n");
> + } else if (cur_role == USB_ROLE_DEVICE) {
> + usb3_disconnect(usb3);
> + /* Must set the mode before device_attach of the host */
> + usb3_set_mode(usb3, true);
> + /* This device_attach() might sleep */
> + if (device_attach(host) < 0)
> + dev_err(dev, "device_attach(host) failed\n");
> + }
> + break;
> + default:
> + break;
> + }
> +}
> +
> +static void handle_role_switch_states(struct device *dev,
> + enum usb_role role)
> {
> struct renesas_usb3 *usb3 = dev_get_drvdata(dev);
> struct device *host = usb3->host_dev;
> enum usb_role cur_role = renesas_usb3_role_switch_get(dev);
>
> - pm_runtime_get_sync(dev);
> if (cur_role == USB_ROLE_HOST && role == USB_ROLE_DEVICE) {
> device_release_driver(host);
> usb3_set_mode(usb3, false);
> @@ -2361,6 +2419,20 @@ static int renesas_usb3_role_switch_set(struct device *dev,
> if (device_attach(host) < 0)
> dev_err(dev, "device_attach(host) failed\n");
> }
> +}
> +
> +static int renesas_usb3_role_switch_set(struct device *dev,
> + enum usb_role role)
> +{
> + struct renesas_usb3 *usb3 = dev_get_drvdata(dev);
> +
> + pm_runtime_get_sync(dev);
> +
> + if (usb3->dual_role_sw)
> + handle_ext_role_switch_states(dev, role);
> + else
> + handle_role_switch_states(dev, role);
> +
> pm_runtime_put(dev);
>
> return 0;
> @@ -2650,12 +2722,41 @@ static const unsigned int renesas_usb3_cable[] = {
> EXTCON_NONE,
> };
>
> -static const struct usb_role_switch_desc renesas_usb3_role_switch_desc = {
> +static struct usb_role_switch_desc renesas_usb3_role_switch_desc = {
> .set = renesas_usb3_role_switch_set,
> .get = renesas_usb3_role_switch_get,
> .allow_userspace_control = true,
> };
>
> +static bool is_usb_dual_role_switch(struct device *dev)
> +{
> + struct device_node *np = dev->of_node;
> + struct device_node *parent;
> + struct device_node *child;
> + bool ret = false;
> + const char *role_type = NULL;
> +
> + child = of_graph_get_endpoint_by_regs(np, -1, -1);
> + if (!child)
> + return ret;
> +
> + parent = of_graph_get_remote_port_parent(child);
> + of_node_put(child);
> + child = of_get_child_by_name(parent, "connector");
> + of_node_put(parent);
> + if (!child)
> + return ret;
> +
> + if (of_device_is_compatible(child, "usb-c-connector")) {
> + of_property_read_string(child, "data-role", &role_type);
> + if (role_type && (!strncmp(role_type, "dual", strlen("dual"))))
> + ret = true;
> + }
> +
> + of_node_put(child);
> + return ret;
> +}
> +
> static int renesas_usb3_probe(struct platform_device *pdev)
> {
> struct renesas_usb3 *usb3;
> @@ -2741,6 +2842,12 @@ static int renesas_usb3_probe(struct platform_device *pdev)
> if (ret < 0)
> goto err_dev_create;
>
> + if (device_property_read_bool(&pdev->dev, "usb-role-switch") &&
> + is_usb_dual_role_switch(&pdev->dev)) {
I think either one of the conditions is enough. (Only "usb-role-switch"
checking is enough, IIUC).
JFYI, according to the binding document [1], this "usb-role-switch" means:
---
+ - usb-role-switch: boolean, indicates that the device is capable of assigning
+ the USB data role (USB host or USB device) for a given
+ USB connector, such as Type-C, Type-B(micro).
+ see connector/usb-connector.txt.
---
So, R-Car Gen3 / Salvator-XS cannot have this property because the board
has Type-A connector.
[1] https://patchwork.kernel.org/patch/10934835/
> + usb3->dual_role_sw = true;
So, "role_sw_by_connector" matches with my image.
What do you think?
Best regards,
Yoshihiro Shimoda
> + renesas_usb3_role_switch_desc.fwnode = dev_fwnode(&pdev->dev);
> + }
> +
> INIT_WORK(&usb3->role_work, renesas_usb3_role_work);
> usb3->role_sw = usb_role_switch_register(&pdev->dev,
> &renesas_usb3_role_switch_desc);
> --
> 2.7.4
^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: [PATCH v6 4/7] usb: gadget: udc: renesas_usb3: Add dual role switch support
2019-05-21 5:49 ` Yoshihiro Shimoda
@ 2019-05-21 7:10 ` Biju Das
2019-05-21 7:34 ` Yoshihiro Shimoda
0 siblings, 1 reply; 23+ messages in thread
From: Biju Das @ 2019-05-21 7:10 UTC (permalink / raw)
To: Yoshihiro Shimoda, Felipe Balbi, Greg Kroah-Hartman
Cc: Heikki Krogerus, Simon Horman, Fabrizio Castro, Kees Cook,
linux-usb, Simon Horman, Geert Uytterhoeven, Chris Paterson,
linux-renesas-soc, Rob Herring
Hi Shimoda-San,
Thanks for the feedback.
> > From: Biju Das, Sent: Wednesday, May 15, 2019 9:09 PM
> > Subject: [PATCH v6 4/7] usb: gadget: udc: renesas_usb3: Add dual role
> > switch support
>
> Now I'm confusing about the "Add dual role switch support" mean...
> Especially, this driver has already supports dual role switch support by own
> sysfs or debugfs.
Sorry for the confusion.
What about "Enhance role switch support" ?
> > The RZ/G2E cat874 board has a type-c connector connected to hd3ss3220
> > usb type-c drp port controller. This patch adds dual role switch
> > support for the type-c connector using the usb role switch class framework.
>
> IIUC, after this patch is applied, the hs3ss3220 type-c driver can switch the
> role by using the usb role switch class framework.
Yes, That is correct. HD3SS3220 driver detects host/device connection events (attach/detach) and
It calls "usb_role_switch_set_role" to assign/switch the role.
> > Signed-off-by: Biju Das <biju.das@bp.renesas.com>
> > ---
> > V5-->V6
> > * Added graph api's to find the role supported by the connector.
> > V4-->V5
> > * Incorporated Shimoda-san's review comment
> > (https://patchwork.kernel.org/patch/10902537/)
> > V3-->V4
> > * No Change
> > V2-->V3
> > * Incorporated Shimoda-san's review comment
> > (https://patchwork.kernel.org/patch/10852507/)
> > * Used renesas,usb-role-switch property for differentiating USB
> > role switch associated with Type-C port controller driver.
> > V1-->V2
> > * Driver uses usb role clas for handling dual role switch and handling
> > connect/disconnect events instead of extcon.
> > ---
> > drivers/usb/gadget/udc/renesas_usb3.c | 121
> > ++++++++++++++++++++++++++++++++--
> > 1 file changed, 114 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/usb/gadget/udc/renesas_usb3.c
> > b/drivers/usb/gadget/udc/renesas_usb3.c
> > index 7dc2485..1d41998 100644
> > --- a/drivers/usb/gadget/udc/renesas_usb3.c
> > +++ b/drivers/usb/gadget/udc/renesas_usb3.c
> > @@ -24,6 +24,7 @@
> > #include <linux/usb/ch9.h>
> > #include <linux/usb/gadget.h>
> > #include <linux/usb/of.h>
> > +#include <linux/of_graph.h>
> > #include <linux/usb/role.h>
> >
> > /* register definitions */
> > @@ -351,6 +352,8 @@ struct renesas_usb3 {
> > int disabled_count;
> >
> > struct usb_request *ep0_req;
> > +
> > + enum usb_role connection_state;
> > u16 test_mode;
> > u8 ep0_buf[USB3_EP0_BUF_SIZE];
> > bool softconnect;
> > @@ -359,6 +362,7 @@ struct renesas_usb3 {
> > bool extcon_usb; /* check vbus and set EXTCON_USB
> */
> > bool forced_b_device;
> > bool start_to_connect;
> > + bool dual_role_sw;
>
> I'm also confusing this name.
OK. will change it to "role_sw_by_connector"
> > };
> >
> > #define gadget_to_renesas_usb3(_gadget) \
> <snip>
> > @@ -699,8 +703,10 @@ static void usb3_mode_config(struct renesas_usb3
> *usb3, bool host, bool a_dev)
> > unsigned long flags;
> >
> > spin_lock_irqsave(&usb3->lock, flags);
> > - usb3_set_mode_by_role_sw(usb3, host);
> > - usb3_vbus_out(usb3, a_dev);
> > + if (!usb3->dual_role_sw || usb3->connection_state !=
> USB_ROLE_NONE) {
> > + usb3_set_mode_by_role_sw(usb3, host);
> > + usb3_vbus_out(usb3, a_dev);
> > + }
> > /* for A-Peripheral or forced B-device mode */
> > if ((!host && a_dev) || usb3->start_to_connect)
> > usb3_connect(usb3);
> > @@ -716,7 +722,8 @@ static void usb3_check_id(struct renesas_usb3
> > *usb3) {
> > usb3->extcon_host = usb3_is_a_device(usb3);
> >
> > - if (usb3->extcon_host && !usb3->forced_b_device)
> > + if ((!usb3->dual_role_sw && usb3->extcon_host && !usb3-
> >forced_b_device)
> > + || usb3->connection_state == USB_ROLE_HOST)
> > usb3_mode_config(usb3, true, true);
> > else
> > usb3_mode_config(usb3, false, false); @@ -2343,14 +2350,65
> @@
> > static enum usb_role renesas_usb3_role_switch_get(struct device *dev)
> > return cur_role;
> > }
> >
> > -static int renesas_usb3_role_switch_set(struct device *dev,
> > - enum usb_role role)
> > +static void handle_ext_role_switch_states(struct device *dev,
> > + enum usb_role role)
> > +{
> > + struct renesas_usb3 *usb3 = dev_get_drvdata(dev);
> > + struct device *host = usb3->host_dev;
> > + enum usb_role cur_role = renesas_usb3_role_switch_get(dev);
> > +
> > + switch (role) {
> > + case USB_ROLE_NONE:
> > + usb3->connection_state = USB_ROLE_NONE;
> > + if (usb3->driver)
> > + usb3_disconnect(usb3);
> > + usb3_vbus_out(usb3, false);
> > + break;
> > + case USB_ROLE_DEVICE:
> > + if (usb3->connection_state == USB_ROLE_NONE) {
> > + usb3->connection_state = USB_ROLE_DEVICE;
> > + usb3_set_mode(usb3, false);
> > + if (usb3->driver)
> > + usb3_connect(usb3);
> > + } else if (cur_role == USB_ROLE_HOST) {
> > + device_release_driver(host);
> > + usb3_set_mode(usb3, false);
> > + if (usb3->driver)
> > + usb3_connect(usb3);
> > + }
> > + usb3_vbus_out(usb3, false);
> > + break;
> > + case USB_ROLE_HOST:
> > + if (usb3->connection_state == USB_ROLE_NONE) {
> > + if (usb3->driver)
> > + usb3_disconnect(usb3);
> > +
> > + usb3->connection_state = USB_ROLE_HOST;
> > + usb3_set_mode(usb3, true);
> > + usb3_vbus_out(usb3, true);
> > + if (device_attach(host) < 0)
> > + dev_err(dev, "device_attach(host)
> failed\n");
> > + } else if (cur_role == USB_ROLE_DEVICE) {
> > + usb3_disconnect(usb3);
> > + /* Must set the mode before device_attach of the
> host */
> > + usb3_set_mode(usb3, true);
> > + /* This device_attach() might sleep */
> > + if (device_attach(host) < 0)
> > + dev_err(dev, "device_attach(host)
> failed\n");
> > + }
> > + break;
> > + default:
> > + break;
> > + }
> > +}
> > +
> > +static void handle_role_switch_states(struct device *dev,
> > + enum usb_role role)
> > {
> > struct renesas_usb3 *usb3 = dev_get_drvdata(dev);
> > struct device *host = usb3->host_dev;
> > enum usb_role cur_role = renesas_usb3_role_switch_get(dev);
> >
> > - pm_runtime_get_sync(dev);
> > if (cur_role == USB_ROLE_HOST && role == USB_ROLE_DEVICE) {
> > device_release_driver(host);
> > usb3_set_mode(usb3, false);
> > @@ -2361,6 +2419,20 @@ static int renesas_usb3_role_switch_set(struct
> device *dev,
> > if (device_attach(host) < 0)
> > dev_err(dev, "device_attach(host) failed\n");
> > }
> > +}
> > +
> > +static int renesas_usb3_role_switch_set(struct device *dev,
> > + enum usb_role role)
> > +{
> > + struct renesas_usb3 *usb3 = dev_get_drvdata(dev);
> > +
> > + pm_runtime_get_sync(dev);
> > +
> > + if (usb3->dual_role_sw)
> > + handle_ext_role_switch_states(dev, role);
> > + else
> > + handle_role_switch_states(dev, role);
> > +
> > pm_runtime_put(dev);
> >
> > return 0;
> > @@ -2650,12 +2722,41 @@ static const unsigned int renesas_usb3_cable[]
> = {
> > EXTCON_NONE,
> > };
> >
> > -static const struct usb_role_switch_desc
> > renesas_usb3_role_switch_desc = {
> > +static struct usb_role_switch_desc renesas_usb3_role_switch_desc = {
> > .set = renesas_usb3_role_switch_set,
> > .get = renesas_usb3_role_switch_get,
> > .allow_userspace_control = true,
> > };
> >
> > +static bool is_usb_dual_role_switch(struct device *dev) {
> > + struct device_node *np = dev->of_node;
> > + struct device_node *parent;
> > + struct device_node *child;
> > + bool ret = false;
> > + const char *role_type = NULL;
> > +
> > + child = of_graph_get_endpoint_by_regs(np, -1, -1);
> > + if (!child)
> > + return ret;
> > +
> > + parent = of_graph_get_remote_port_parent(child);
> > + of_node_put(child);
> > + child = of_get_child_by_name(parent, "connector");
> > + of_node_put(parent);
> > + if (!child)
> > + return ret;
> > +
> > + if (of_device_is_compatible(child, "usb-c-connector")) {
> > + of_property_read_string(child, "data-role", &role_type);
> > + if (role_type && (!strncmp(role_type, "dual",
> strlen("dual"))))
> > + ret = true;
> > + }
> > +
> > + of_node_put(child);
> > + return ret;
> > +}
> > +
> > static int renesas_usb3_probe(struct platform_device *pdev) {
> > struct renesas_usb3 *usb3;
> > @@ -2741,6 +2842,12 @@ static int renesas_usb3_probe(struct
> platform_device *pdev)
> > if (ret < 0)
> > goto err_dev_create;
> >
> > + if (device_property_read_bool(&pdev->dev, "usb-role-switch") &&
> > + is_usb_dual_role_switch(&pdev->dev)) {
>
> I think either one of the conditions is enough. (Only "usb-role-switch"
> checking is enough, IIUC).
OK, Will remove the other check.
> JFYI, according to the binding document [1], this "usb-role-switch" means:
> ---
> + - usb-role-switch: boolean, indicates that the device is capable of assigning
> + the USB data role (USB host or USB device) for a
> given
> + USB connector, such as Type-C, Type-B(micro).
> + see connector/usb-connector.txt.
> ---
>
> So, R-Car Gen3 / Salvator-XS cannot have this property because the board
> has Type-A connector.
>
> [1] https://patchwork.kernel.org/patch/10934835/
I have updated the binding patch to cover this[1]
[1]. https://patchwork.kernel.org/patch/10944631/
Are you ok with this binding patch?
> > + usb3->dual_role_sw = true;
>
> So, "role_sw_by_connector" matches with my image.
> What do you think?
OK, will use " role_sw_by_connector"
Regards,
Biju
> > + renesas_usb3_role_switch_desc.fwnode =
> dev_fwnode(&pdev->dev);
> > + }
> > +
> > INIT_WORK(&usb3->role_work, renesas_usb3_role_work);
> > usb3->role_sw = usb_role_switch_register(&pdev->dev,
> > &renesas_usb3_role_switch_desc);
> > --
> > 2.7.4
^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: [PATCH v6 4/7] usb: gadget: udc: renesas_usb3: Add dual role switch support
2019-05-21 7:10 ` Biju Das
@ 2019-05-21 7:34 ` Yoshihiro Shimoda
0 siblings, 0 replies; 23+ messages in thread
From: Yoshihiro Shimoda @ 2019-05-21 7:34 UTC (permalink / raw)
To: Biju Das, Felipe Balbi, Greg Kroah-Hartman
Cc: Heikki Krogerus, Simon Horman, Fabrizio Castro, Kees Cook,
linux-usb, Simon Horman, Geert Uytterhoeven, Chris Paterson,
linux-renesas-soc, Rob Herring
Hi Biju-san,
> From: Biju Das, Sent: Tuesday, May 21, 2019 4:10 PM
>
> Hi Shimoda-San,
>
> Thanks for the feedback.
>
> > > From: Biju Das, Sent: Wednesday, May 15, 2019 9:09 PM
> > > Subject: [PATCH v6 4/7] usb: gadget: udc: renesas_usb3: Add dual role
> > > switch support
> >
> > Now I'm confusing about the "Add dual role switch support" mean...
> > Especially, this driver has already supports dual role switch support by own
> > sysfs or debugfs.
>
> Sorry for the confusion.
> What about "Enhance role switch support" ?
Thank you for the suggestion. It's good to me.
> > > The RZ/G2E cat874 board has a type-c connector connected to hd3ss3220
> > > usb type-c drp port controller. This patch adds dual role switch
> > > support for the type-c connector using the usb role switch class framework.
> >
> > IIUC, after this patch is applied, the hs3ss3220 type-c driver can switch the
> > role by using the usb role switch class framework.
>
> Yes, That is correct. HD3SS3220 driver detects host/device connection events (attach/detach) and
> It calls "usb_role_switch_set_role" to assign/switch the role.
I got it.
<snip>
> > JFYI, according to the binding document [1], this "usb-role-switch" means:
> > ---
> > + - usb-role-switch: boolean, indicates that the device is capable of assigning
> > + the USB data role (USB host or USB device) for a
> > given
> > + USB connector, such as Type-C, Type-B(micro).
> > + see connector/usb-connector.txt.
> > ---
> >
> > So, R-Car Gen3 / Salvator-XS cannot have this property because the board
> > has Type-A connector.
> >
> > [1] https://patchwork.kernel.org/patch/10934835/
>
> I have updated the binding patch to cover this[1]
> [1]. https://patchwork.kernel.org/patch/10944631/
> Are you ok with this binding patch?
I'm sorry, I overlooked the patch. I'll check it later.
Best regards,
Yoshihiro Shimoda
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v6 5/7] arm64: defconfig: enable TYPEC_HD3SS3220 config option
2019-05-15 12:09 [PATCH v6 0/7] Add USB3.0 and TI HD3SS3220 driver support Biju Das
` (3 preceding siblings ...)
2019-05-15 12:09 ` [PATCH v6 4/7] usb: gadget: udc: renesas_usb3: Add dual role switch support Biju Das
@ 2019-05-15 12:09 ` Biju Das
2019-05-15 12:09 ` [PATCH v6 6/7] arm64: dts: renesas: r8a774c0-cat874: Enable USB3.0 host/peripheral device node Biju Das
2019-05-15 12:09 ` [PATCH v6 7/7] arm64: dts: renesas: r8a774c0-cat874: Enable usb role switch support Biju Das
6 siblings, 0 replies; 23+ messages in thread
From: Biju Das @ 2019-05-15 12:09 UTC (permalink / raw)
To: Catalin Marinas, Will Deacon
Cc: Biju Das, Greg Kroah-Hartman, Heiko Stuebner, Simon Horman,
Andy Gross, Olof Johansson, Shawn Guo, Jagan Teki,
Bjorn Andersson, Enric Balletbo i Serra, Stefan Wahren,
Ezequiel Garcia, Marc Gonzalez, linux-arm-kernel, Simon Horman,
Geert Uytterhoeven, Chris Paterson, Fabrizio Castro,
linux-renesas-soc
Enable support for the TI HD3SS320 USB Type-C DRP Port controller driver
by turning on CONFIG_TYPEC and CONFIG_TYPEC_HD3SS3220 as modules.
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/configs/defconfig | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 8871cf7..9dc71a7 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -576,6 +576,8 @@ CONFIG_USB_ULPI=y
CONFIG_USB_GADGET=y
CONFIG_USB_RENESAS_USBHS_UDC=m
CONFIG_USB_RENESAS_USB3=m
+CONFIG_TYPEC=m
+CONFIG_TYPEC_HD3SS3220=m
CONFIG_MMC=y
CONFIG_MMC_BLOCK_MINORS=32
CONFIG_MMC_ARMMMCI=y
--
2.7.4
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v6 6/7] arm64: dts: renesas: r8a774c0-cat874: Enable USB3.0 host/peripheral device node
2019-05-15 12:09 [PATCH v6 0/7] Add USB3.0 and TI HD3SS3220 driver support Biju Das
` (4 preceding siblings ...)
2019-05-15 12:09 ` [PATCH v6 5/7] arm64: defconfig: enable TYPEC_HD3SS3220 config option Biju Das
@ 2019-05-15 12:09 ` Biju Das
2019-05-15 12:09 ` [PATCH v6 7/7] arm64: dts: renesas: r8a774c0-cat874: Enable usb role switch support Biju Das
6 siblings, 0 replies; 23+ messages in thread
From: Biju Das @ 2019-05-15 12:09 UTC (permalink / raw)
To: Rob Herring, Mark Rutland
Cc: Biju Das, Greg Kroah-Hartman, Heikki Krogerus, Felipe Balbi,
Simon Horman, Yoshihiro Shimoda, Magnus Damm, linux-renesas-soc,
devicetree, Geert Uytterhoeven, Chris Paterson, Fabrizio Castro
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
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v6 7/7] arm64: dts: renesas: r8a774c0-cat874: Enable usb role switch support
2019-05-15 12:09 [PATCH v6 0/7] Add USB3.0 and TI HD3SS3220 driver support Biju Das
` (5 preceding siblings ...)
2019-05-15 12:09 ` [PATCH v6 6/7] arm64: dts: renesas: r8a774c0-cat874: Enable USB3.0 host/peripheral device node Biju Das
@ 2019-05-15 12:09 ` Biju Das
2019-05-24 21:50 ` Rob Herring
6 siblings, 1 reply; 23+ messages in thread
From: Biju Das @ 2019-05-15 12:09 UTC (permalink / raw)
To: Rob Herring, Mark Rutland
Cc: Biju Das, Greg Kroah-Hartman, Heikki Krogerus, Felipe Balbi,
Simon Horman, Yoshihiro Shimoda, Magnus Damm, linux-renesas-soc,
devicetree, Geert Uytterhoeven, Chris Paterson, Fabrizio Castro
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
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v6 7/7] arm64: dts: renesas: r8a774c0-cat874: Enable usb role switch support
2019-05-15 12:09 ` [PATCH v6 7/7] arm64: dts: renesas: r8a774c0-cat874: Enable usb role switch support Biju Das
@ 2019-05-24 21:50 ` Rob Herring
0 siblings, 0 replies; 23+ messages in thread
From: Rob Herring @ 2019-05-24 21:50 UTC (permalink / raw)
To: Biju Das
Cc: Mark Rutland, Greg Kroah-Hartman, Heikki Krogerus, Felipe Balbi,
Simon Horman, Yoshihiro Shimoda, Magnus Damm, linux-renesas-soc,
devicetree, Geert Uytterhoeven, Chris Paterson, Fabrizio Castro
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
>
^ permalink raw reply [flat|nested] 23+ messages in thread