Re: [PATCH v3] usb: musb: fix tx fifo flush handling

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

 





On 11/06/2015 11:29 AM, Bin Liu wrote:
Hi,

On 11/06/2015 11:11 AM, Felipe Balbi wrote:

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:

Since the check does not help the flush issue, I would prefer leave it
for the future patch which does the proper fix.


        dev_WARN_ONCE(dev, !(csr & MUSB_TXCSR_TXPKTRDY),
                "TxPktRdy should still be set!);

I am not sure the WARN() here is right, it is not a error condition when
FIFONOTEMPTY is set but TXPKTRDY is not.


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

Better put this explanation in the commit message area, not here in-line?

Never mind, I will put it here inline, where I think is the better place.


Regards,
-Bin.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



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

  Powered by Linux