Re: [PATCH V2 2/2] usb: musb: Stop bulk endpoint while queue is rotated

From: Sergei Shtylyov
Date: Mon May 23 2016 - 09:40:26 EST


On 5/23/2016 4:26 PM, Andrew Goodbody wrote:

Ensure that the endpoint is stopped by clearing REQPKT before clearing
DATAERR_NAKTIMEOUT before rotating the queue on the dedicated bulk
endpoint.
This addresses an issue where a race could result in the endpoint
receiving data before it was reprogrammed resulting in a warning about
such data from musb_rx_reinit before it was thrown away.
The data thrown away was a valid packet that had been correctly ACKed
which meant the host and device got out of sync.

Signed-off-by: Andrew Goodbody <andrew.goodbody@xxxxxxxxxxxxxx>
Cc: stable@xxxxxxxxxxxxxxx
---
V2 added comment about clearing REQPKT before DATAERR_NAKTIMEOUT

drivers/usb/musb/musb_host.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/drivers/usb/musb/musb_host.c
b/drivers/usb/musb/musb_host.c index 2966596..676cb98 100644
--- a/drivers/usb/musb/musb_host.c
+++ b/drivers/usb/musb/musb_host.c
@@ -997,6 +997,12 @@ static void musb_bulk_nak_timeout(struct musb
*musb, struct musb_hw_ep *ep,
/* clear nak timeout bit */
rx_csr = musb_readw(epio, MUSB_RXCSR);
rx_csr |= MUSB_RXCSR_H_WZC_BITS;
+ /*
+ * Need to stop the transaction by clearing REQPKT before

Transcation? Maybe transfer?

The quote from the TRM is "If the DATAERR_NAKTIMEOUT bit is set, the controller can be directed either to continue trying this transaction (until it times out again) by clearing the DATAERR_NAKTIMEOUT bit or to abort the transaction by clearing REQPKT bit before clearing the DATAERR_NAKTIMEOUT bit."
So 'transaction' is correct.

OK.

+ * DATAERR_NAKTIMEOUT ref TRM section 16.3.8.2.2.1.2

Which TRM? Do you understand that the MUSB core is used by multiple
SoCs?
I'd recommend referring just to the "abstract" manual or the MUSB
programmer's guide (section 9.2.2 if you want an exact ref.).

That would be the AM335x Sitara Processors TRM. I don't have the MUSB programmer's guide, is it available online somewhere?

AFAIK, no.

I do not understand what you mean by the "abstract" manual.

Just saying "the manual" or "the manuals".

Would you be OK with "ref MUSB Programmer's Guide section 9.2.2"

Yes. However, that spec calls the bit just "NAK Timeout" in this context, not DATAERR_NAKTIMEOUT. Overall, this spec. uses CamelCase for the register bit names (I recall dbrownell complaining over that and uppercase all the bit names in my patches :-).

Andrew

[...]

MBR, Sergei