From: Biju Das <biju.das@bp.renesas.com>
To: Rob Herring <robh@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Heikki Krogerus <heikki.krogerus@linux.intel.com>,
Felipe Balbi <balbi@kernel.org>,
"linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
Simon Horman <horms@verge.net.au>,
Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>,
Geert Uytterhoeven <geert+renesas@glider.be>,
Chris Paterson <Chris.Paterson2@renesas.com>,
Fabrizio Castro <fabrizio.castro@bp.renesas.com>,
"linux-renesas-soc@vger.kernel.org"
<linux-renesas-soc@vger.kernel.org>,
Chunfeng Yun <chunfeng.yun@mediatek.com>,
Yu Chen <chenyu56@huawei.com>
Subject: RE: [PATCH V5 4/9] dt-bindings: usb: renesas_usb3: Add renesas,usb-role-switch property
Date: Fri, 3 May 2019 06:39:24 +0000 [thread overview]
Message-ID: <OSAPR01MB2098D4F5C9C2D620324DD42CB8350@OSAPR01MB2098.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <OSBPR01MB2103BF796E95B788CC018CA2B8390@OSBPR01MB2103.jpnprd01.prod.outlook.com>
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
next prev parent reply other threads:[~2019-05-03 6:39 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-04-24 9:22 [PATCH V5 0/9] Add USB3.0 and TI HD3SS3220 driver support Biju Das
2019-04-24 9:22 ` [PATCH V5 1/9] usb: roles: Introduce stubs for the exiting functions in role.h Biju Das
2019-04-24 9:22 ` [PATCH V5 2/9] usb: roles: add API to get usb_role_switch by node Biju Das
2019-04-24 9:22 ` [PATCH V5 3/9] dt-bindings: usb: hd3ss3220 device tree binding document Biju Das
2019-04-24 9:22 ` [PATCH V5 4/9] dt-bindings: usb: renesas_usb3: Add renesas,usb-role-switch property Biju Das
2019-04-26 18:14 ` Rob Herring
2019-04-29 9:39 ` Biju Das
2019-05-03 6:39 ` Biju Das [this message]
2019-04-24 9:22 ` [PATCH V5 5/9] usb: typec: driver for TI HD3SS3220 USB Type-C DRP port controller Biju Das
2019-04-24 9:22 ` [PATCH V5 6/9] usb: gadget: udc: renesas_usb3: Use usb_role_switch framework Biju Das
2019-04-24 9:22 ` [PATCH V5 7/9] arm64: defconfig: enable TYPEC_HD3SS3220 config option Biju Das
2019-04-24 9:22 ` [PATCH V5 8/9] arm64: dts: renesas: r8a774c0-cat874: Enable USB3.0 host/peripheral device node Biju Das
2019-04-24 9:22 ` [PATCH V5 9/9] arm64: dts: renesas: r8a774c0-cat874: Enable usb role switch support Biju Das
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=OSAPR01MB2098D4F5C9C2D620324DD42CB8350@OSAPR01MB2098.jpnprd01.prod.outlook.com \
--to=biju.das@bp.renesas.com \
--cc=Chris.Paterson2@renesas.com \
--cc=balbi@kernel.org \
--cc=chenyu56@huawei.com \
--cc=chunfeng.yun@mediatek.com \
--cc=devicetree@vger.kernel.org \
--cc=fabrizio.castro@bp.renesas.com \
--cc=geert+renesas@glider.be \
--cc=gregkh@linuxfoundation.org \
--cc=heikki.krogerus@linux.intel.com \
--cc=horms@verge.net.au \
--cc=linux-renesas-soc@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=robh@kernel.org \
--cc=yoshihiro.shimoda.uh@renesas.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).