Hi Biju-san, > From: Biju Das, Sent: Tuesday, April 23, 2019 6:08 PM > > 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@xxxxxxxxxxxxxx> > > > --- > > > 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. Thank you for the explanation. > > 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. Oops. Thank you for the pointed it out. > 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 Since the forced_b_device is related to debug purpose (controlled by debugfs), I don't want to use the value for type-c. > 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? I'd like to change usb3_check_id() somehow. How about the following conditions? In type-c environment, since usb3->usb_role_switch_property is true, it should be OK for it. if ((!usb3->usb_role_switch_property && 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); Best regards, Yoshihiro Shimoda > Note:- > I have done only sanity testing with this changes. > > Regards, > Biju > > > > > >
