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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
