RE: [PATCH V5 4/9] dt-bindings: usb: renesas_usb3: Add renesas,usb-role-switch property

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxxxx>
> > ---
> >  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




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux