Re: [PATCH] chipidea: Fix issue in reconnecing gadget without insmod/rmmod

[Niranjan Dighe]

On Wed, Apr 12, 2017 at 5:21 PM, Peter Chen <hzpeterchen@xxxxxxxxx> wrote:
> On Tue, Apr 11, 2017 at 08:46:24PM +0530, Niranjan Dighe wrote:
>> Currently usb_gadget_connect() is called only through gadget
>> registration via composite_driver_probe(). As a result, after a
>> disconnection, if the role transitions to host and back to gadget,
>> the gadget is not recognized by the host anymore.
>> This is a typical scenario with an iAP device in the following
>> usecase -
>> conn iAP dev -> send roleswitch command -> roleswitch ourself
>> to gadget -> disconnect iAP device -> roleswitch back to host to
>> enumerate the device again -> reconnect device -> resend the
>> roleswitch command -> roleswitch ourself to gadget -> ISSUE.
>>
>> To workaround this, do the following -
>>
>> 1. Restart OTG FSM on SLi interrupt and on switching role to gadget
>> so that device transitions to B_IDLE state from B_PERIPHERAL state.
>> A transition from B_IDLE to B_PERIPHERAL is needed to enable
>> interrups and restore the correct state of the chipidea controller
>> so that communication with host is possible
>>
>> 2. usb_gadget_connect() after roleswitch to gadget so that
>> gadget->ops->pullup() is called and D+ line is asserted. This
>> causes host to "see" the device and enumeration can happen.
>>
>> Signed-off-by: Niranjan Dighe <niranjan.dighe@xxxxxxxxx>
>> ---
>>  drivers/usb/chipidea/debug.c | 10 +++++++++-
>>  drivers/usb/chipidea/udc.c   |  3 +++
>>  2 files changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/chipidea/debug.c b/drivers/usb/chipidea/debug.c
>> index a4f7db2..66c1485 100644
>> --- a/drivers/usb/chipidea/debug.c
>> +++ b/drivers/usb/chipidea/debug.c
>> @@ -16,7 +16,7 @@
>>  #include "udc.h"
>>  #include "bits.h"
>>  #include "otg.h"
>> -
>> +extern void ci_hdrc_otg_fsm_restart(struct ci_hdrc *);
>>  /**
>>   * ci_device_show: prints information about device capabilities and status
>>   */
>> @@ -325,6 +325,14 @@ static ssize_t ci_role_write(struct file *file, const char __user *ubuf,
>>       ci_role_stop(ci);
>>       ret = ci_role_start(ci, role);
>>       enable_irq(ci->irq);
>> +
>> +     /* REVISIT - Avoid repeated FSM restart*/
>> +
>> +     if (role == CI_ROLE_GADGET) {
>> +             ci_hdrc_otg_fsm_restart(ci);
>> +             usb_gadget_connect(&ci->gadget);
>> +     }
>> +
>>       pm_runtime_put_sync(ci->dev);
>>
>>       return ret ? ret : count;
>> diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
>> index a0accd5..1e3b827 100644
>> --- a/drivers/usb/chipidea/udc.c
>> +++ b/drivers/usb/chipidea/udc.c
>> @@ -29,6 +29,8 @@
>>  #include "otg.h"
>>  #include "otg_fsm.h"
>>
>> +extern void ci_hdrc_otg_fsm_restart(struct ci_hdrc *);
>> +
>>  /* control endpoint description */
>>  static const struct usb_endpoint_descriptor
>>  ctrl_endpt_out_desc = {
>> @@ -1881,6 +1883,7 @@ static irqreturn_t udc_irq(struct ci_hdrc *ci)
>>                               ci->driver->suspend(&ci->gadget);
>>                               usb_gadget_set_state(&ci->gadget,
>>                                               USB_STATE_SUSPENDED);
>> +                             ci_hdrc_otg_fsm_restart(ci);
>>                               spin_lock(&ci->lock);
>>                       }
>>               }
>> --
>
> Hi Niranjan,
>
> When working with iAP device, there are two role-switch methods
> - Through OTG FSM, and using sysfs entries under
> /sys/bus/platform/devices/ci_hdrc.0/inputs
> but you may need to patch code to keep vbus always on for A-device,
> it is not compliance with OTG spec.
> - Using role interface under debugfs (I move it under sysfs for v4.12).
> But you need to patch the code and let the A-device switch back to
> host after iAP is disconnected.
>
> You don't need to use above two methods together, I suggest using the
> 2nd method since OTG FSM is hard to maintain due to no mandatory use
> case for it.
>
> --
>
> Best Regards,
> Peter Chen

Thank you Peter for your response.

Yes, I will try to switch the role on disconnection of iAP device to Host. If
I understand correctly I have to do something like this -

        ci_role_stop(ci); //Gadget role stop
        ret = ci_role_start(ci, role); //Host role start

correct?

Regards,
Niranjan