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@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;
> +}
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
