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

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

 



Hi,

On 11/05/2015 02:04 PM, Sergei Shtylyov wrote:
On 11/05/2015 10:54 PM, Bin Liu wrote:

Here are a few changes in musb_h_tx_flush_fifo().

- Refering to 2ccc6d30a (usb: musb: fix bit mask for CSR in
   musb_h_tx_flush_fifo()), the datasheet says that
MUSB_TXCSR_FLUSHFIFO
   is only valid when MUSB_TXCSR_TXPKTRDY is set as well. It means
   MUSB_TXCSR_TXPKTRDY should be checked, not set.

    Hum, my copy of the MUSBHDRC  programmer's guide says about
TXCSR.FlushFIFO in host mode:

<<
The CPU writes a 1 to this bit to flush the latest packet from the
endpoint Tx FIFO. The FIFO pointer is reset, the TxPktRdy bit (below) is
cleared and an interrupt is generated. May be set simultaneously with
TxPktRdy to abort the packet that is currently being loaded into the
FIFO. Note: FlushFIFO should only be used when TxPktRdy is set. At other
times, it may cause data to be corrupted. Also note that, if the FIFO is
double-buffered, FlushFIFO may need to be set twice to completely clear
the FIFO.
 >>

So how to interpret this message? FLUSHFIFO and TXPKTRDY should be set
at the
same time? If so, I will drop this change in V3.

    Yes, I think it's rather clear in this respect.

In either case, musb is unable to flush the tx fifo in urb dequque for
device
disconnect, but harmless, so the next two changes are important to
give better
user experience.

    Hm, looks like some errata... and hiding this from users while
there's no workaround doesn't seem a good policy.

Well, based on the programmer's guide, the driver should set the flushFIFO bit, and wait for the interrupt.

However, the driver does 1000-times retry loop to probe the FIFONOTEMPTY bit. This is there in the very first version of the driver back in 2008. So I would think musb just behaves like that, rather than an errata. Also the WARN() was not there in the early version of the driver.

Please guide what we will do with this patch? Should I send V3 with the first change removed, or just drop this patch completely?

Thanks,
-Bin.


Thanks,
-Bin.


    So I don't think this change is valid.

- It seems to be common that sometimes (if not always) musb is unable
   to flush tx fifo during urb dequeue. 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() scares many end users, so change it to dev_dbg().

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

[...]

MBR, Sergei

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