From: Biju Das <biju.das@bp.renesas.com> To: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Heikki Krogerus <heikki.krogerus@linux.intel.com>, Simon Horman <horms+renesas@verge.net.au>, Fabrizio Castro <fabrizio.castro@bp.renesas.com>, Kees Cook <keescook@chromium.org>, "linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>, Simon Horman <horms@verge.net.au>, Geert Uytterhoeven <geert+renesas@glider.be>, Chris Paterson <Chris.Paterson2@renesas.com>, "linux-renesas-soc@vger.kernel.org" <linux-renesas-soc@vger.kernel.org>, Felipe Balbi <balbi@kernel.org> Subject: RE: [PATCH V4 4/7] usb: gadget: udc: renesas_usb3: Use usb_role_switch framework Date: Tue, 23 Apr 2019 09:07:46 +0000 [thread overview] Message-ID: <OSBPR01MB21037040E074CBC79E3E780EB8230@OSBPR01MB2103.jpnprd01.prod.outlook.com> (raw) In-Reply-To: <OSBPR01MB17335B483D7585AB1F27B61DD8230@OSBPR01MB1733.jpnprd01.prod.outlook.com> Hi Shimoda-San, Thanks for the feedback. > Subject: RE: [PATCH V4 4/7] usb: gadget: udc: renesas_usb3: Use > usb_role_switch framework > > Hi Biju-san, > > Thank you for the patch! > > > From: Biju Das, Sent: Tuesday, April 16, 2019 6:38 PM > > > > RZ/G2E cat874 board is capable of detecting cable connect and > > disconnect events. Add support for renesas_usb3 to receive connect and > > disconnect events and support dual-role switch using usb-role-switch > framework. > > > > Signed-off-by: Biju Das <biju.das@bp.renesas.com> > > --- > > 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. > <snip> > > > +static void usb3_check_vbus(struct renesas_usb3 *usb3) { > > + if (usb3->workaround_for_vbus) { > > + if (usb3->usb_role_switch_property) { > > + if (usb3->connection_state == USB_ROLE_DEVICE) { > > + usb3_mode_config(usb3, false, false); > > I should have pointed it out the previous version though, why does this > usb3_mode_config() calling need? > My guess is: > - renesas_usb3_start() calls renesas_usb3_init_controller(). > -- renesas_usb3_init_controller() calls usb3_check_id() and then > usb_check_vbus(). > --- usb_check_id() calls usb3_mode_config(usb3, true, true) and then the > HW acts as host mode. > ----> So, you'd like the HW to acts as peripheral mode when the > connection_state is USB_ROLE_DEVICE, > you added that the usb3_check_vbus() calls usb3_mode_config(usb3, > false, false). > > Is my guess correct? If so, I'd like to add such code into usb3_check_id() like > below: Yes, it is almost correct. The scenario I am trying is [1] USB type C cable connected to a Host Machine(TI chip identifies as Device connection. But we haven't installed Gadget module for Device operation) [2] After that trying to install gadget module. In this case, it calls usb_check_id() as mentioned above and configure it as Host mode. > if ((usb3->extcon_host && !usb3->forced_b_device) || > (usb3->usb_role_switch_property && > usb3->connection_state == USB_ROLE_HOST)) > usb3_mode_config(usb3, true, true); > else > usb3_mode_config(usb3, false, false); > > What do you think? Since as per [1], usb3->extcon_host=1 and !usb3->forced_b_device ==1 , The above code always enter into Host Mode configuration. To make it work, I need to update " usb3->forced_b_device" based on connection_state from TI chip. So the new code look like 1) There is no change in usb_check_id() call. if (usb3->extcon_host && !usb3->forced_b_device) usb3_mode_config(usb3, true, true); else usb3_mode_config(usb3, false, false); 2) Update "usb3->forced_b_device" variable based on connection_state. @@ -2370,6 +2376,7 @@ static void handle_ext_role_switch_states(struct device *dev, usb3_vbus_out(usb3, false); break; case USB_ROLE_DEVICE: + usb3->forced_b_device = true; if (usb3->connection_state == USB_ROLE_NONE) { usb3->connection_state = USB_ROLE_DEVICE; usb3_set_mode(usb3, false); @@ -2384,6 +2391,7 @@ static void handle_ext_role_switch_states(struct device *dev, usb3_vbus_out(usb3, false); break; case USB_ROLE_HOST: + usb3->forced_b_device = false; Can you please confirm are you ok with this changes? Or do you prefer the previous one? Note:- I have done only sanity testing with this changes. Regards, Biju
WARNING: multiple messages have this Message-ID (diff)
From: Biju Das <biju.das@bp.renesas.com> To: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Heikki Krogerus <heikki.krogerus@linux.intel.com>, Simon Horman <horms+renesas@verge.net.au>, Fabrizio Castro <fabrizio.castro@bp.renesas.com>, Kees Cook <keescook@chromium.org>, "linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>, Simon Horman <horms@verge.net.au>, Geert Uytterhoeven <geert+renesas@glider.be>, Chris Paterson <Chris.Paterson2@renesas.com>, "linux-renesas-soc@vger.kernel.org" <linux-renesas-soc@vger.kernel.org>, Felipe Balbi <balbi@kernel.org> Subject: [V4,4/7] usb: gadget: udc: renesas_usb3: Use usb_role_switch framework Date: Tue, 23 Apr 2019 09:07:46 +0000 [thread overview] Message-ID: <OSBPR01MB21037040E074CBC79E3E780EB8230@OSBPR01MB2103.jpnprd01.prod.outlook.com> (raw) Hi Shimoda-San, Thanks for the feedback. > Subject: RE: [PATCH V4 4/7] usb: gadget: udc: renesas_usb3: Use > usb_role_switch framework > > Hi Biju-san, > > Thank you for the patch! > > > From: Biju Das, Sent: Tuesday, April 16, 2019 6:38 PM > > > > RZ/G2E cat874 board is capable of detecting cable connect and > > disconnect events. Add support for renesas_usb3 to receive connect and > > disconnect events and support dual-role switch using usb-role-switch > framework. > > > > Signed-off-by: Biju Das <biju.das@bp.renesas.com> > > --- > > 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. > <snip> > > > +static void usb3_check_vbus(struct renesas_usb3 *usb3) { > > + if (usb3->workaround_for_vbus) { > > + if (usb3->usb_role_switch_property) { > > + if (usb3->connection_state == USB_ROLE_DEVICE) { > > + usb3_mode_config(usb3, false, false); > > I should have pointed it out the previous version though, why does this > usb3_mode_config() calling need? > My guess is: > - renesas_usb3_start() calls renesas_usb3_init_controller(). > -- renesas_usb3_init_controller() calls usb3_check_id() and then > usb_check_vbus(). > --- usb_check_id() calls usb3_mode_config(usb3, true, true) and then the > HW acts as host mode. > ----> So, you'd like the HW to acts as peripheral mode when the > connection_state is USB_ROLE_DEVICE, > you added that the usb3_check_vbus() calls usb3_mode_config(usb3, > false, false). > > Is my guess correct? If so, I'd like to add such code into usb3_check_id() like > below: Yes, it is almost correct. The scenario I am trying is [1] USB type C cable connected to a Host Machine(TI chip identifies as Device connection. But we haven't installed Gadget module for Device operation) [2] After that trying to install gadget module. In this case, it calls usb_check_id() as mentioned above and configure it as Host mode. > if ((usb3->extcon_host && !usb3->forced_b_device) || > (usb3->usb_role_switch_property && > usb3->connection_state == USB_ROLE_HOST)) > usb3_mode_config(usb3, true, true); > else > usb3_mode_config(usb3, false, false); > > What do you think? Since as per [1], usb3->extcon_host=1 and !usb3->forced_b_device ==1 , The above code always enter into Host Mode configuration. To make it work, I need to update " usb3->forced_b_device" based on connection_state from TI chip. So the new code look like 1) There is no change in usb_check_id() call. if (usb3->extcon_host && !usb3->forced_b_device) usb3_mode_config(usb3, true, true); else usb3_mode_config(usb3, false, false); 2) Update "usb3->forced_b_device" variable based on connection_state. @@ -2370,6 +2376,7 @@ static void handle_ext_role_switch_states(struct device *dev, usb3_vbus_out(usb3, false); break; case USB_ROLE_DEVICE: + usb3->forced_b_device = true; if (usb3->connection_state == USB_ROLE_NONE) { usb3->connection_state = USB_ROLE_DEVICE; usb3_set_mode(usb3, false); @@ -2384,6 +2391,7 @@ static void handle_ext_role_switch_states(struct device *dev, usb3_vbus_out(usb3, false); break; case USB_ROLE_HOST: + usb3->forced_b_device = false; Can you please confirm are you ok with this changes? Or do you prefer the previous one? Note:- I have done only sanity testing with this changes. Regards, Biju
next prev parent reply other threads:[~2019-04-23 9:07 UTC|newest] Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-04-16 9:38 [PATCH V4 0/7] Add USB3.0 and TI HD3SS3220 driver support Biju Das 2019-04-16 9:38 ` [PATCH V4 1/7] dt-bindings: usb: hd3ss3220 device tree binding document Biju Das 2019-04-16 9:38 ` [V4,1/7] " Biju Das 2019-04-16 9:38 ` [PATCH V4 2/7] dt-bindings: usb: renesas_usb3: Add renesas,usb-role-switch property Biju Das 2019-04-16 9:38 ` [V4,2/7] " Biju Das 2019-04-16 9:38 ` [PATCH V4 3/7] usb: typec: driver for TI HD3SS3220 USB Type-C DRP port controller Biju Das 2019-04-16 9:38 ` [V4,3/7] " Biju Das 2019-04-17 9:57 ` [PATCH V4 3/7] " Heikki Krogerus 2019-04-17 9:57 ` [V4,3/7] " Heikki Krogerus 2019-04-17 11:07 ` [PATCH V4 3/7] " Biju Das 2019-04-17 11:07 ` [V4,3/7] " Biju Das 2019-04-16 9:38 ` [PATCH V4 4/7] usb: gadget: udc: renesas_usb3: Use usb_role_switch framework Biju Das 2019-04-16 9:38 ` [V4,4/7] " Biju Das 2019-04-23 2:27 ` [PATCH V4 4/7] " Yoshihiro Shimoda 2019-04-23 2:27 ` [V4,4/7] " Yoshihiro Shimoda 2019-04-23 9:07 ` Biju Das [this message] 2019-04-23 9:07 ` Biju Das 2019-04-23 11:00 ` [PATCH V4 4/7] " Yoshihiro Shimoda 2019-04-23 11:00 ` [V4,4/7] " Yoshihiro Shimoda 2019-04-23 15:06 ` [PATCH V4 4/7] " Biju Das 2019-04-23 15:06 ` [V4,4/7] " Biju Das 2019-04-16 9:38 ` [PATCH V4 5/7] arm64: defconfig: enable TYPEC_HD3SS3220 config option Biju Das 2019-04-16 9:38 ` [V4,5/7] " Biju Das 2019-04-16 9:38 ` [PATCH V4 6/7] arm64: dts: renesas: r8a774c0-cat874: Enable USB3.0 host/peripheral device node Biju Das 2019-04-16 9:38 ` [PATCH V4 7/7] 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=OSBPR01MB21037040E074CBC79E3E780EB8230@OSBPR01MB2103.jpnprd01.prod.outlook.com \ --to=biju.das@bp.renesas.com \ --cc=Chris.Paterson2@renesas.com \ --cc=balbi@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+renesas@verge.net.au \ --cc=horms@verge.net.au \ --cc=keescook@chromium.org \ --cc=linux-renesas-soc@vger.kernel.org \ --cc=linux-usb@vger.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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.