Re: [PATCH 0/6] usb: dwc3: gadget: Rework pullup

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Thinh,

Welcome back! :)

On 5/23/2022 5:30 PM, Thinh Nguyen wrote:
Wesley Cheng wrote:
Hi Thinh,

On 4/26/2022 2:05 PM, Wesley Cheng wrote:
Hi Thinh,

On 4/21/2022 7:22 PM, Thinh Nguyen wrote:
This series cleanup and enhance dwc3 pullup() handling to cover
different
corner cases.

Would be great to have some Tested-by before picking this series up.
Thanks!


Thinh Nguyen (6):
    usb: dwc3: gadget: Prevent repeat pullup()
    usb: dwc3: gadget: Refactor pullup()
    usb: dwc3: gadget: Don't modify GEVNTCOUNT in pullup()
    usb: dwc3: ep0: Don't prepare beyond Setup stage
    usb: dwc3: gadget: Only End Transfer for ep0 data phase
    usb: dwc3: gadget: Delay issuing End Transfer

   drivers/usb/dwc3/ep0.c    |   2 +-
   drivers/usb/dwc3/gadget.c | 126 ++++++++++++++++++++------------------
   2 files changed, 69 insertions(+), 59 deletions(-)


base-commit: 5c29e864999763baec9eedb9ea5bd557aa4cbd77

Thanks for this series.  Running the tests w/ the changes now and will
debug if I run into any issues.  I will need to run the previous test
cases I had as well, since the change removes the GEVNTCOUNT clearing
during pullup disable (this was added for some controller halt failures).


Going to summarize some of the things I've found so far:
1.  DWC3_EP_DELAY_STOP flag set for EPs:
The test case being run will have usb ep dequeue running closely in
parallel to soft disconnect.  There is a chance that we run into
controller halt due to active EPs, since we are not
waiting/synchronizing for DWC3_EP_DELAY_STOP to be cleared or complete.

I sent an update. Can you test it out?


Attached thinh_newest_delayed_status_causing_ep_stop_delay_flag.txt
- Force device crash if run/stop routine fails w/ -ETIMEDOUT.

Can you clarify here? Did you force the crash or did the crash occur due
to the change?

Just injecting a kernel panic if there is an -ETIMEDOUT condition during run/stop clear. The end of the traces will be at the point of which the error occurred.

- There is a situation where a function will return delayed_status, and
we can see "timed out waiting for SETUP phase" print during pullup disable.

It should be fine that the warning gets printed. The programming guide
suggested that the driver should wait for all the control transfers to
complete. This deviates from the programming guide. If it happens often
enough, we may need to increase the timeout.

Yes, agreed.


2.  Controller halt failure due to non-zero GEVNTCOUNT
So with this patch series, and removing the GEVNTCOUNT clearing, I'm
running into controller halt failures again.  When I printed the
GEVNTCOUNT register at the time of failure, it showed that there were
several pending events.

Do you have the log for this? What's the IO delay for each register read
on your platform? I suspect that the polling for halt status is too
quick, we may need to add some delay between polls.

Will try to collect a log for you after adding the new changes (if I run into this). I tried to increase the number of loops as well, but that didn't help.

If you have the log, can you also enable register read/write to see the
delta time?

I'll need to collect a set of logs for this. Will add the register read/write log also.

Thanks
Wesley Cheng



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux