Re: [Discussion] USB: musb-gadget: how to fix ZLP issue in musb_g_tx

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

 



Hello.

On 17-09-2010 4:00, 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.

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


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux