Hello. Ming Lei wrote:
Suppose the condition of "request->zero&& (request->actual == request->length)" is true now: - if request->length % musb_ep->packet_sz == 0, the patch can send zlp out without any problem -if request->length % musb_ep->packet_sz != 0, in the dma case we may set txpktrdy to send out short packet since the 2nd condition is triggered,
Which 2nd condition?
(is_dma&& (!dma->desired_mode || (request->actual& (musb_ep->packet_sz - 1)))
and in pio case txstate has set it already if request->actual == request->length;
It might have been cleared already by the time you'd set it again. You'll then re-trigger empty packet send.
Yes, your are right, and follows the revised version.
How about this one?
diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c index 46cf94a..fff2455 100644 --- a/drivers/usb/musb/musb_gadget.c +++ b/drivers/usb/musb/musb_gadget.c @@ -477,40 +477,39 @@ void musb_g_tx(struct musb *musb, u8 epnum) epnum, csr, musb_ep->dma->actual_len, request); } - if (is_dma || request->actual == request->length) { - /* - * First, maybe a terminating short packet. Some DMA - * engines might handle this by themselves. - */ - if ((request->zero&& request->length - && request->length % musb_ep->packet_sz == 0) + /* + * First, maybe a terminating short packet. Some DMA + * engines might handle this by themselves. + */ + if ((request->zero && request->length + && (request->length % musb_ep->packet_sz == 0) + && (request->actual == request->length))
Parens around == not necessary.
#ifdef CONFIG_USB_INVENTRA_DMA - || (is_dma && (!dma->desired_mode || - (request->actual & - (musb_ep->packet_sz - 1)))) + || (is_dma && (!dma->desired_mode || + (request->actual & + (musb_ep->packet_sz - 1)))) #endif - ) { - /* - * On DMA completion, FIFO may not be - * available yet... - */ - if (csr& MUSB_TXCSR_TXPKTRDY) - return; + ) { + /* + * On DMA completion, FIFO may not be + * available yet... + */ + if (csr& MUSB_TXCSR_TXPKTRDY) + return; - DBG(4, "sending zero pkt\n"); - musb_writew(epio, MUSB_TXCSR, MUSB_TXCSR_MODE - | MUSB_TXCSR_TXPKTRDY); - request->zero = 0; - } + DBG(4, "sending zero pkt\n"); + musb_writew(epio, MUSB_TXCSR, MUSB_TXCSR_MODE + | MUSB_TXCSR_TXPKTRDY); + request->zero = 0; + } - if (request->actual == request->length) { - musb_g_giveback(musb_ep, request, 0); - request = musb_ep->desc ? next_request(musb_ep) : NULL; - if (!request) { - DBG(4, "%s idle now\n", - musb_ep->end_point.name); - return; - } + if (request->actual == request->length) {
I don't understand why we have to check this twice when only once would suffice.
This check can handle both zero and non-zero request, so it is needed.
It's needed above all the other checks, that's all. We don't need to check it twice.
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
