On 11/05/2015 10:46 PM, Sergei Shtylyov 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 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>
---
drivers/usb/musb/musb_host.c | 16 ++++++----------
1 file changed, 6 insertions(+), 10 deletions(-)
diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
index 883a9ad..5db24f2 100644
--- a/drivers/usb/musb/musb_host.c
+++ b/drivers/usb/musb/musb_host.c
@@ -112,22 +112,18 @@ 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) {
- if (csr != lastcsr)
- dev_dbg(musb->controller, "Host TX FIFONOTEMPTY csr: %02x\n",
csr);
- lastcsr = csr;
- csr |= MUSB_TXCSR_FLUSHFIFO | MUSB_TXCSR_TXPKTRDY;
+ while ((csr & MUSB_TXCSR_FIFONOTEMPTY) && (csr & MUSB_TXCSR_TXPKTRDY)) {
while (csr & (MUSB_TXCSR_FIFONOTEMPTY | MUSB_TXCSR_TXPKTRDY)) {
Oops, I misread your check, it had &&, not ||.
+ csr |= MUSB_TXCSR_FLUSHFIFO;
No, this clearly contradicts the programmer's guide.
Still I disagree with this change.
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
