Hi,
Bin Liu <b-liu@xxxxxx> writes:
> Here are a few changes in musb_h_tx_flush_fifo().
>
> - It has been observed that sometimes (if not always) musb is unable
> to flush tx fifo during urb dequeue when disconnect a device. But
> it seems to be harmless, since the tx fifo flush is done again in
> musb_ep_program() when re-use the hw_ep.
>
> But the WARN() floods the console in the case when multiple tx urbs
> are queued, so change it to dev_WARN_ONCE().
>
> - applications could queue up many tx urbs, then the 1ms delay could
> causes minutes of delay in device disconnect. So remove it to get
> better user experience. The 1ms delay does not help the flushing
> anyway.
>
> - cleanup the debug code - related to lastcsr.
>
> Signed-off-by: Bin Liu <b-liu@xxxxxx>
> ---
> drivers/usb/musb/musb_host.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
> index 883a9ad..1c7f858 100644
> --- a/drivers/usb/musb/musb_host.c
> +++ b/drivers/usb/musb/musb_host.c
> @@ -112,22 +112,22 @@ static void musb_h_tx_flush_fifo(struct musb_hw_ep *ep)
> struct musb *musb = ep->musb;
> void __iomem *epio = ep->regs;
> u16 csr;
> - u16 lastcsr = 0;
> int retries = 1000;
>
> csr = musb_readw(epio, MUSB_TXCSR);
> while (csr & MUSB_TXCSR_FIFONOTEMPTY) {
are you not going to check that TXPKTRDY is still set ? I guess that
could be another WARN(), actually, since it should never happen. But
maybe it's subject to a separate patch:
dev_WARN_ONCE(dev, !(csr & MUSB_TXCSR_TXPKTRDY),
"TxPktRdy should still be set!);
> - if (csr != lastcsr)
> - dev_dbg(musb->controller, "Host TX FIFONOTEMPTY csr: %02x\n", csr);
> - lastcsr = csr;
> csr |= MUSB_TXCSR_FLUSHFIFO | MUSB_TXCSR_TXPKTRDY;
> musb_writew(epio, MUSB_TXCSR, csr);
> csr = musb_readw(epio, MUSB_TXCSR);
> - if (WARN(retries-- < 1,
> +
> + /*
> + * FIXME: sometimes the tx fifo flush failed, it has been
> + * seeing during device disconnect.
seen ?
also, you probably want to describe here how you reproduced
this. Something along the lines of:
Reproduced at least with AM335x Evaluation Module when an input
device (using $device - VID:PID - here) is attached to its host
port.
That way, it's clear that the thing has really been reproduced and it
gives people instructions on how to reproduce.
--
balbi
Attachment:
signature.asc
Description: PGP signature
