On Thu, Aug 20, 2015 at 05:03:46PM -0400, Alan Stern wrote: > On Thu, 20 Aug 2015, Felipe Balbi wrote: > > > > > > - When using pattern = 1 as module parameters to compare the data, the > > > > > packet size must be same between host and device's. > > > > > > > > why ? > > > > > > The gadget stores the pattern data starting from 0 for each packet it > > > sends. But the host tests the pattern data starting from 0 only at the > > > beginning of the transfer; the host expects the pattern to continue > > > without resetting at packet boundaries (if the transfer length is > > > larger than the maxpacket size). > > > > then that's another bug which needs to be fixed :-) > > Here's my attempt at a fix. Completely untested, but it does compile > without errors. Peter, do you mind trying this out?
After adding related changes at gadget side, it works. In fact, the gadget stores the pattern data starting from 0 to transfer length (not the packet length). Besides, you may need to consider high bandwidth ISO transfer, you can send the formal patch and I will change gadget side and have a test. Besides, if I use vary < length, the test 4/7/8 have failed (still not check why) Besides, considering let vary equals to length for control transfer? In that case, we can use one line command for all tests. diff --git a/drivers/usb/gadget/function/f_sourcesink.c b/drivers/usb/gadget/function/f_sourcesink.c index cbfaf86..414046b 100644 --- a/drivers/usb/gadget/function/f_sourcesink.c +++ b/drivers/usb/gadget/function/f_sourcesink.c @@ -510,7 +510,8 @@ static int check_read_data(struct f_sourcesink *ss, struct usb_request *req) * stutter for any reason, including buffer duplication...) */ case 1: - if (*buf == (u8)(i % 63)) + + if (*buf == (u8)((i % ss->out_ep->desc->wMaxPacketSize) % 63)) continue; break; } @@ -525,14 +526,14 @@ static void reinit_write_data(struct usb_ep *ep, struct usb_request *req) { unsigned i; u8 *buf = req->buf; - + switch (pattern) { case 0: memset(req->buf, 0, req->length); break; case 1: for (i = 0; i < req->length; i++) - *buf++ = (u8) (i % 63); + *buf++ = (u8) ((i % ep->desc->wMaxPacketSize) % 63); break; case 2: break; > > Alan Stern > > > > Index: usb-4.2/drivers/usb/misc/usbtest.c > =================================================================== > --- usb-4.2.orig/drivers/usb/misc/usbtest.c > +++ usb-4.2/drivers/usb/misc/usbtest.c > @@ -303,11 +303,20 @@ static unsigned mod_pattern; > module_param_named(pattern, mod_pattern, uint, S_IRUGO | S_IWUSR); > MODULE_PARM_DESC(mod_pattern, "i/o pattern (0 == zeroes)"); > > -static inline void simple_fill_buf(struct urb *urb) > +static unsigned get_maxpacket(struct usb_device *udev, int pipe) > +{ > + struct usb_host_endpoint *ep; > + > + ep = usb_pipe_endpoint(udev, pipe); > + return le16_to_cpup(&ep->desc.wMaxPacketSize); > +} > + > +static void simple_fill_buf(struct urb *urb) > { > unsigned i; > u8 *buf = urb->transfer_buffer; > unsigned len = urb->transfer_buffer_length; > + unsigned maxpacket; > > switch (pattern) { > default: > @@ -316,8 +325,9 @@ static inline void simple_fill_buf(struc > memset(buf, 0, len); > break; > case 1: /* mod63 */ > + maxpacket = get_maxpacket(urb->dev, urb->pipe); > for (i = 0; i < len; i++) > - *buf++ = (u8) (i % 63); > + *buf++ = (u8) ((i % maxpacket) % 63); > break; > } > } > @@ -349,6 +359,7 @@ static int simple_check_buf(struct usbte > u8 expected; > u8 *buf = urb->transfer_buffer; > unsigned len = urb->actual_length; > + unsigned maxpacket = get_maxpacket(urb->dev, urb->pipe); > > int ret = check_guard_bytes(tdev, urb); > if (ret) > @@ -366,7 +377,7 @@ static int simple_check_buf(struct usbte > * with set_interface or set_config. > */ > case 1: /* mod63 */ > - expected = i % 63; > + expected = (i % maxpacket) % 63; > break; > /* always fail unsupported patterns */ > default: > @@ -478,11 +489,12 @@ static void free_sglist(struct scatterli > } > > static struct scatterlist * > -alloc_sglist(int nents, int max, int vary) > +alloc_sglist(int nents, int max, int vary, struct usbtest_dev *dev, int pipe) > { > struct scatterlist *sg; > unsigned i; > unsigned size = max; > + unsigned maxpacket = > get_maxpacket(interface_to_usbdev(dev->intf), pipe); > > if (max == 0) > return NULL; > @@ -511,7 +523,7 @@ alloc_sglist(int nents, int max, int var > break; > case 1: > for (j = 0; j < size; j++) > - *buf++ = (u8) (j % 63); > + *buf++ = (u8) ((j % maxpacket) % 63); > break; > } > > @@ -2175,7 +2187,7 @@ usbtest_ioctl(struct usb_interface *intf > "TEST 5: write %d sglists %d entries of %d bytes\n", > param->iterations, > param->sglen, param->length); > - sg = alloc_sglist(param->sglen, param->length, 0); > + sg = alloc_sglist(param->sglen, param->length, 0, dev, > dev->out_pipe); > if (!sg) { > retval = -ENOMEM; > break; > @@ -2193,7 +2205,7 @@ usbtest_ioctl(struct usb_interface *intf > "TEST 6: read %d sglists %d entries of %d bytes\n", > param->iterations, > param->sglen, param->length); > - sg = alloc_sglist(param->sglen, param->length, 0); > + sg = alloc_sglist(param->sglen, param->length, 0, dev, > dev->in_pipe); > if (!sg) { > retval = -ENOMEM; > break; > @@ -2210,7 +2222,7 @@ usbtest_ioctl(struct usb_interface *intf > "TEST 7: write/%d %d sglists %d entries 0..%d bytes\n", > param->vary, param->iterations, > param->sglen, param->length); > - sg = alloc_sglist(param->sglen, param->length, param->vary); > + sg = alloc_sglist(param->sglen, param->length, param->vary, > dev, dev->out_pipe); > if (!sg) { > retval = -ENOMEM; > break; > @@ -2227,7 +2239,7 @@ usbtest_ioctl(struct usb_interface *intf > "TEST 8: read/%d %d sglists %d entries 0..%d bytes\n", > param->vary, param->iterations, > param->sglen, param->length); > - sg = alloc_sglist(param->sglen, param->length, param->vary); > + sg = alloc_sglist(param->sglen, param->length, param->vary, > dev, dev->in_pipe); > if (!sg) { > retval = -ENOMEM; > break; > -- Best Regards, Peter Chen -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html