Re: [PATCH v2] USB: Gadget, fsl_qe: Fixed send of zero byte packets

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

 



Hello.

On 22-11-2010 13:02, Holger brunck wrote:

From: Stefan Bigler<stefan.bigler@xxxxxxxxxxx>

Some usb serial host drivers expect a short packet before they forward
the data to the application. This is caused by them trying to read more
than one packet at a time. So when the gadget sends an exact multiple
of the maximum packet size, it should append a zero-length packet.
The gadget driver for fsl_qe was not handling this case correctly.
This patch adds sending of a null packet if it is the last packet of
a frame and the request length is USB_EP0_MAX_SIZE (64 Byte).

See also commit 2e25134122c25ebb0679b4bbd536fb46c669f9d7

    Linus has asked to also specify the commit summary in parens.

Ok I will do this if the patch is with regards to its content ok.

   It's does not seem OK currently...

Signed-off-by: Stefan Bigler<stefan.bigler@xxxxxxxxxxx>
Signed-off-by: Holger Brunck<holger.brunck@xxxxxxxxxxx>
Cc: Li Yang<leoli@xxxxxxxxxxxxx>
Cc: David Brownell<dbrownell@xxxxxxxxxxxxxxxxxxxxx>

---
  drivers/usb/gadget/fsl_qe_udc.c |    3 +++
  1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/usb/gadget/fsl_qe_udc.c
b/drivers/usb/gadget/fsl_qe_udc.c
index 7881f12..279454a 100644
--- a/drivers/usb/gadget/fsl_qe_udc.c
+++ b/drivers/usb/gadget/fsl_qe_udc.c
@@ -1229,6 +1229,9 @@ static int frame_create_tx(struct qe_ep *ep,
struct qe_frame *frame)
         else
                 reval = sendnulldata(ep, frame, 0);

    No, really you shouldn't send null data in every case when
req->req.length - ep->sent<= 0, only if req->req.length == 0 or
req->req.zero == 1...

Hm, yes this piece of code looks a bit odd to me too. And I did not understand
what it should solve. But I let it untouched, because the bug my patch should
fix is in my eyes a different topic.

   No, this is actually the same topic.

+       if (req->req.zero&&  (req->req.length == USB_EP0_MAX_SIZE))

    Not (req->req.length % USB_EP0_MAX_SIZE == 0)? Also, you should
probably check that ep->sent == req->req.length here...

No not (req->req.length % USB_EP0_MAX_SIZE == 0), because the nulldata which is
missing should only be send if it is the last packet of a frame and not every

   But your code will only send nulldata is the transfer is *exactly* 64 bytes.
req->req.length shouldn't change.

time a 64byte packet is send.

You've misunderstood what I proposed -- there's no way it can send nulldata for each 64 packet. That's why we should check that (ep->sent == req->req.length).

Best regards
Holger

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