David Brownell wrote:
From: Ajay Kumar Gupta <ajay.gupta@xxxxxx>
Fixes endpoint starvation issue when more than one bulk QH is multiplexed on the reserved bulk RX endpoint, which is normal for cases like serial and ethernet adapters.
This patch sets the NAK timeout interval for such QHs, and when a timeout triggers the next QH will be scheduled. (This resembles the bulk scheduling done in hardware by EHCI, OHCI, and UHCI.)
This scheme doesn't work for devices which are connected to a high to full speed tree (transaction translator) as there is no NAK timeout interrupt from the musb controller from such devices.
Tested with PIO, Inventra DMA, CPPI DMA.
[ dbrownell@xxxxxxxxxxxxxxxxxxxxx: fold in start_urb() update; clarify only for bulk RX; don't accidentally clear WZC bits ]
I've long deferred commenting on this patch but decided to finally speak out...
Signed-off-by: Ajay Kumar Gupta <ajay.gupta@xxxxxx> Cc: Felipe Balbi <felipe.balbi@xxxxxxxxx> Signed-off-by: David Brownell <dbrownell@xxxxxxxxxxxxxxxxxxxxx>
--- a/drivers/usb/musb/musb_host.c +++ b/drivers/usb/musb/musb_host.c
[...]
@@ -201,8 +195,9 @@ musb_start_urb(struct musb *musb, int is len = urb->iso_frame_desc[0].length; break; default: /* bulk, interrupt */ - buf = urb->transfer_buffer; - len = urb->transfer_buffer_length; + /* actual_length may be nonzero on retry paths */
Well, I've already commented that retries are not handled consistently.
+ buf = urb->transfer_buffer + urb->actual_length; + len = urb->transfer_buffer_length - urb->actual_length; }DBG(4, "qh %p urb %p dev%d ep%d%s%s, hw_ep %d, %p/%d\n",
[...]
@@ -1357,6 +1354,50 @@ finish:#endif +/* Schedule next QH from musb->in_bulk and move the current qh to+ * the end; avoids starvation for other endpoints. + */ +static void musb_bulk_rx_nak_timeout(struct musb *musb, struct musb_hw_ep *ep) +{ + struct dma_channel *dma; + struct urb *urb; + void __iomem *mbase = musb->mregs; + void __iomem *epio = ep->regs; + struct musb_qh *cur_qh, *next_qh; + u16 rx_csr; + + musb_ep_select(mbase, ep->epnum); + dma = is_dma_capable() ? ep->rx_channel : NULL; + + /* clear nak timeout bit */ + rx_csr = musb_readw(epio, MUSB_RXCSR); + rx_csr |= MUSB_RXCSR_H_WZC_BITS; + rx_csr &= ~MUSB_RXCSR_DATAERROR; + musb_writew(epio, MUSB_RXCSR, rx_csr);
I don't think you want to clear the NAKTimeout bit without clearing ReqPkt as well since otherwise you're risking a race if the device decides to ACK the packet in the short window between this point and when you finally switch to another device...
+
+ cur_qh = first_qh(&musb->in_bulk);
+ if (cur_qh) {
Is the opposite even possible? I *highly* doubt it given the fact that this gets called with non-NULL URB.
+ /* set rx_reinit and schedule the next qh */ + ep->rx_reinit = 1; + musb_start_urb(musb, 1, next_qh);
Note that the NAKTimeout bit will be cleared by musb_h_flush_rxfifo() called from musb_rx_reinit() a bit later anyway.
@@ -1420,18 +1461,26 @@ void musb_host_rx(struct musb *musb, u8 } else if (rx_csr & MUSB_RXCSR_DATAERROR) { if (USB_ENDPOINT_XFER_ISOC != qh->type) {
Why not change this to switch (qh->type) instead?
@@ -1751,6 +1800,17 @@ static int musb_schedule( head = &musb->in_bulk; else head = &musb->out_bulk; + + /* Enable bulk RX NAK timeout scheme when bulk requests are + * multiplexed. This scheme doen't work in high speed to full + * speed scenario as NAK interrupts are not coming from a + * full speed device connected to a high speed device. + * NAK timeout interval is 8 (128 uframe or 16ms) for HS and + * 4 (8 frame or 8ms) for FS device.
It's not clear why the timeouts are different... WBR, 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
