Dear Sebastian,
On 11/25/2014 10:52 PM, Sebastian Andrzej Siewior wrote:
The max packet size within the FS descriptor has to be highest possible
value and _not_ the one that is (or will be) used on FS.
The current code sets wMaxPacketSize of FS interrupt endpoint descriptor
as 64, which is in accordance with the usb 2.0 specification, section 5.7.3
The maximum allowable interrupt data payload size is 64 bytes
or less for full-speed. High-speed endpoints are allowed
maximum data payload sizes up to 1024 bytes.
The way the code works (since day 1) is that usb_ep_autoconfig() is
invoked _only_ on the FS endpoint and then the endpoint address is
copies over to HS/SS endpoints. If the any of the "critical" options are
different between FS and HS then we have to pass the HS value and
correct later.
What now happens is that we look for an INT-OUT endpoint of 64bytes. The
code will return an endpoint matching this category. Further the
sourcesink will take this endpoint and increase the MPS to 1024. On
This is valid only for HS and SS interrupt endpoints. Right?
net2280 for instance the code tries to be clever to return an endpoint
which can only do 64 MPS. The same happens on musb where we mostlike get
an endpoint which can only do 512. The result is then on the host side:
What is the speed at which your device is operating?
|~# testusb -a -t 9 -c 2
|unknown speed /dev/bus/usb/002/045 0
|usbtest 2-1:3.0: TEST 9: ch9 (subset) control tests, 2 times
|usbtest 2-1:3.0: can't set_interface = 2, -32
|usbtest 2-1:3.0: ch9 subset failed, iterations left 1
|/dev/bus/usb/002/045 test 9 --> 32 (Broken pipe)
because the on the gadget side we can't enable the endpoint because
desc->size > ep->max_size.
Fixes: ef11982dd7a6 ("usb: gadget: zero: Add support for interrupt EP")
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx>
---
drivers/usb/gadget/function/f_sourcesink.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/drivers/usb/gadget/function/f_sourcesink.c b/drivers/usb/gadget/function/f_sourcesink.c
index 80be25b32cd7..7d8f0216e1a6 100644
--- a/drivers/usb/gadget/function/f_sourcesink.c
+++ b/drivers/usb/gadget/function/f_sourcesink.c
@@ -161,7 +161,7 @@ static struct usb_endpoint_descriptor fs_int_source_desc = {
.bEndpointAddress = USB_DIR_IN,
.bmAttributes = USB_ENDPOINT_XFER_INT,
- .wMaxPacketSize = cpu_to_le16(64),
+ .wMaxPacketSize = cpu_to_le16(1024),
.bInterval = GZERO_INT_INTERVAL,
};
@@ -171,7 +171,7 @@ static struct usb_endpoint_descriptor fs_int_sink_desc = {
.bEndpointAddress = USB_DIR_OUT,
.bmAttributes = USB_ENDPOINT_XFER_INT,
- .wMaxPacketSize = cpu_to_le16(64),
+ .wMaxPacketSize = cpu_to_le16(1024),
.bInterval = GZERO_INT_INTERVAL,
};
This change in wMAxPacketSize of FS interrupt descriptors is violation
of the specs.
@@ -556,16 +556,6 @@ sourcesink_bind(struct usb_configuration *c, struct usb_function *f)
if (int_maxburst > 15)
int_maxburst = 15;
- /* fill in the FS interrupt descriptors from the module parameters */
- fs_int_source_desc.wMaxPacketSize = int_maxpacket > 64 ?
- 64 : int_maxpacket;
- fs_int_source_desc.bInterval = int_interval > 255 ?
- 255 : int_interval;
- fs_int_sink_desc.wMaxPacketSize = int_maxpacket > 64 ?
- 64 : int_maxpacket;
- fs_int_sink_desc.bInterval = int_interval > 255 ?
- 255 : int_interval;
-
/* allocate int endpoints */
ss->int_in_ep = usb_ep_autoconfig(cdev->gadget, &fs_int_source_desc);
if (!ss->int_in_ep)
@@ -587,6 +577,16 @@ sourcesink_bind(struct usb_configuration *c, struct usb_function *f)
if (int_maxpacket > 1024)
int_maxpacket = 1024;
+ /* fill in the FS interrupt descriptors from the module parameters */
+ fs_int_source_desc.wMaxPacketSize = int_maxpacket > 64 ?
+ 64 : int_maxpacket;
+ fs_int_source_desc.bInterval = int_interval > 255 ?
+ 255 : int_interval;
+ fs_int_sink_desc.wMaxPacketSize = int_maxpacket > 64 ?
+ 64 : int_maxpacket;
+ fs_int_sink_desc.bInterval = int_interval > 255 ?
+ 255 : int_interval;
+
/* support high speed hardware */
hs_source_desc.bEndpointAddress = fs_source_desc.bEndpointAddress;
hs_sink_desc.bEndpointAddress = fs_sink_desc.bEndpointAddress;
Things might be working for you but this is not the correct fix, IMO.
Looking into the patch I feel it shall introduce other regressions.
Did you try inserting g_zero with module parameters for musb?
Regards
Amit Virdi
--
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
