| Message ID | 1556097743-12717-5-git-send-email-biju.das@bp.renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | Add USB3.0 and TI HD3SS3220 driver support | expand |
On Wed, Apr 24, 2019 at 10:22:18AM +0100, Biju Das wrote: > Add an optional property renesas,usb-role-switch to support > dual role switch for USB Type-C DRP port controller devices > using USB role switch class framework. > > Signed-off-by: Biju Das <biju.das@bp.renesas.com> > --- > 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 | 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..f1cb06a 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" > + - renesas,usb-role-switch: use USB role switch to handle role switch events Mediatek and HiSilicon both have same or similar properties in patches under review. Please coordinate and document a common property. Really, I'm wondering why this is needed. Can't you walk the graph to the connector and determine if dual role is supported by the connector type? Rob
Hi Rob, Thanks for the feedback. > Subject: Re: [PATCH V5 4/9] dt-bindings: usb: renesas_usb3: Add > renesas,usb-role-switch property > > On Wed, Apr 24, 2019 at 10:22:18AM +0100, Biju Das wrote: > > Add an optional property renesas,usb-role-switch to support dual role > > switch for USB Type-C DRP port controller devices using USB role > > switch class framework. > > > > Signed-off-by: Biju Das <biju.das@bp.renesas.com> > > --- > > 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 | 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..f1cb06a 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" > > + - renesas,usb-role-switch: use USB role switch to handle role > > + switch events > > Mediatek and HiSilicon both have same or similar properties in patches under > review. Please coordinate and document a common property. As per R-Car Gen3 boards design. The USB3.0 port on the boards (Salvator-xs and Ebisu) has a USB3.0 Type-A receptor. For debug purpose , the same port can be used as peripheral using force_b_device mode on debugfs. Ie, we can use force_b_device to switch the role on this boards. Where as RZ/G2E board is having USB 3.0 type-C connector. So the driver needs to know whether to use debugfs based dual role switch or non-debugfs based dual role switch(type-C). That is the reason I have added this property. > Really, I'm wondering why this is needed. Can't you walk the graph to the > connector and determine if dual role is supported by the connector type? Yes, Basically we don't need this property. I could walk through the graph and determine the role supported by connector type Please find the example code which will be used in the driver. +static bool is_ext_dual_role_usb_connector_available(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; +} Regards, Biju
Hi Rob, > Subject: RE: [PATCH V5 4/9] dt-bindings: usb: renesas_usb3: Add > renesas,usb-role-switch property > > On Wed, Apr 24, 2019 at 10:22:18AM +0100, Biju Das wrote: > > > Add an optional property renesas,usb-role-switch to support dual > > > role switch for USB Type-C DRP port controller devices using USB > > > role switch class framework. > > > > > > Signed-off-by: Biju Das <biju.das@bp.renesas.com> > > > --- > > > 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 | 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..f1cb06a 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" > > > + - renesas,usb-role-switch: use USB role switch to handle role > > > + switch events > > > > Mediatek and HiSilicon both have same or similar properties in patches > > under review. Please coordinate and document a common property. > > As per R-Car Gen3 boards design. The USB3.0 port on the boards (Salvator-xs > and Ebisu) has a USB3.0 Type-A receptor. > For debug purpose , the same port can be used as peripheral using > force_b_device mode on debugfs. > Ie, we can use force_b_device to switch the role on this boards. > > Where as RZ/G2E board is having USB 3.0 type-C connector. So the driver > needs to know whether to use debugfs based dual role switch or non- > debugfs based dual role switch(type-C). That is the reason I have added this > property. > > > Really, I'm wondering why this is needed. Can't you walk the graph to > > the connector and determine if dual role is supported by the connector > type? > > Yes, Basically we don't need this property. I could walk through the graph and > determine the role supported by connector type > > Please find the example code which will be used in the driver. > > +static bool is_ext_dual_role_usb_connector_available(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; > +} Since we are introducing "usb-role-switch " common property[1], I feel using the common-property make things simpler compared to walking through the graph. Are you happy with using the common property? Or still prefer walk through the graph solution? Please let me know. [1] https://patchwork.kernel.org/patch/10920909/ Regards, Biju
diff --git a/Documentation/devicetree/bindings/usb/renesas_usb3.txt b/Documentation/devicetree/bindings/usb/renesas_usb3.txt index 35039e7..f1cb06a 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" + - renesas,usb-role-switch: use USB role switch to handle role switch events 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>; + renesas,usb-role-switch; + + port { + #address-cells = <1>; + #size-cells = <0>; + + usb3peri_role_switch: endpoint@0 { + reg = <0>; + remote-endpoint = <&hd3ss3220_ep>; + }; + }; + };
Add an optional property renesas,usb-role-switch to support dual role switch for USB Type-C DRP port controller devices using USB role switch class framework. Signed-off-by: Biju Das <biju.das@bp.renesas.com> --- 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 | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)