[U-Boot] [PATCH] usb: gadget: ci_udc: fix suspend/resume of USB Mass Storage

[Marek Vasut]

On 08/24/2016 03:30 AM, John Tobias wrote:
> Hi Marek,

Hi,

> On Tue, Aug 23, 2016 at 5:18 PM, Marek Vasut <marex at denx.de> wrote:
>> On 08/24/2016 12:49 AM, Fabio Estevam wrote:
>>> Adding Marek on Cc
>>
>> Thanks
>>
>>> On Mon, Aug 22, 2016 at 4:38 PM, John Tobias <john.tobias.ph at gmail.com> wrote:
>>>> When the host machine went to suspend mode and then wake it up, it send
>>>> a USB_REQ_GET_STATUS.
>>
>> Can you expand on this ? It is not clear what the conditions to
>> replicate this are. Please do so in detail.
> 
> 
> For MacOS Yosemite:
> 
> Connect your board into your host machine and run UMS. Once, the icon
> pop-up on your host machine (MacOS), put it in sleep mode (by clicking
> sleep in the menu under apple icon on the upper left corner of the
> screen). Then, once, you're sure that the host machine is in sleep
> mode, just wake it up.
> 
>     1.0 If you load the uboot without the said patch, you should see a
> message "Disk Not Ejected Properly", the icon disappeared and the
> drive is no longer available.
>     1.1 If you load the uboot with the said patch, you should be able
> to access the drive after the host woke-up.
> 
> For Windows (I've only tested in Windows 7):
> Similar procedures, after the icon pop-up on the screen, put the host
> machine in sleep mode (by clicking sleep under "Shut down" menu). The
> behavior should similar to 1.0 and 1.1.

OK, I see, now it's clearer . This really should be in the commit message.

> Btw, if you are testing it in Windows, connect your device directly
> into the USB port of your host machine, not in the HUB.

I think I'll have to trust you on all this.

Do you know which usb request arrives which throws the code off ?
I don't think it's correct to remove the bRequestType checking altogether.

Thanks

>>>> The existing case condition below, never
>>>> become true and it causes the host machine to reset the connection. Once,
>>>> it reset you will see an error "Disk Not Ejected Properly".
>>>>
>>>> case SETUP(USB_DIR_IN | USB_RECIP_DEVICE, USB_REQ_GET_STATUS):
>>>>
>>>> By applying this patch, the device could respond to the USB_REQ_GET_STATUS
>>>> command correctly.
>>>> ---
>>>>  drivers/usb/gadget/ci_udc.c | 8 ++++----
>>>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/usb/gadget/ci_udc.c b/drivers/usb/gadget/ci_udc.c
>>>> index d36bcf6..fdec613 100644
>>>> --- a/drivers/usb/gadget/ci_udc.c
>>>> +++ b/drivers/usb/gadget/ci_udc.c
>>>> @@ -703,8 +703,8 @@ static void handle_setup(void)
>>>>         list_del_init(&ci_req->queue);
>>>>         ci_ep->req_primed = false;
>>>>
>>>> -       switch (SETUP(r.bRequestType, r.bRequest)) {
>>>> -       case SETUP(USB_RECIP_ENDPOINT, USB_REQ_CLEAR_FEATURE):
>>>> +       switch (r.bRequest) {
>>>> +       case USB_REQ_CLEAR_FEATURE:
>>>>                 _num = r.wIndex & 15;
>>>>                 _in = !!(r.wIndex & 0x80);
>>>>
>>>> @@ -729,7 +729,7 @@ static void handle_setup(void)
>>>>                 }
>>>>                 return;
>>>>
>>>> -       case SETUP(USB_RECIP_DEVICE, USB_REQ_SET_ADDRESS):
>>>> +       case USB_REQ_SET_ADDRESS:
>>>>                 /*
>>>>                  * write address delayed (will take effect
>>>>                  * after the next IN txn)
>>>> @@ -739,7 +739,7 @@ static void handle_setup(void)
>>>>                 usb_ep_queue(controller.gadget.ep0, req, 0);
>>>>                 return;
>>>>
>>>> -       case SETUP(USB_DIR_IN | USB_RECIP_DEVICE, USB_REQ_GET_STATUS):
>>>> +       case USB_REQ_GET_STATUS:
>>>>                 req->length = 2;
>>>>                 buf = (char *)req->buf;
>>>>                 buf[0] = 1 << USB_DEVICE_SELF_POWERED;
>>>> --
>>>> 2.9.3
>>>>
>>
>>
>> --
>> Best regards,
>> Marek Vasut
> 
> Regards,
> 
> John Tobias
> 


-- 
Best regards,
Marek Vasut