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