: USB: Fix ARM CONFIG_USB_TTY boards
cover : USB: Fix ARM CONFIG_USB_TTY boards
Hey all,
As has been reported in a few threads now, and thanks to Aneesh V's analysis we know why Pandaboard/Beagleboard (and probably a number of other ARM boards) currently hang on boot. The following patch is based on Stefan's list of possible solutions to the problem he first encountered on OpenRISC which lead to the first patch. This has been tested on Panda and Beagle and OpenRISC and I think we must have this for v2011.12 release. Thanks all!
Signature · 1 line
Tom
PATCH 1/1 USB: Use (get|put)_unaligned_le16 for accessing wMaxPacketSize
In 9792987721c7980453fe6447c3fa6593b44f8458 Stefan describes a usecase where the previous behavior of leaving wMaxPacketSize be unaligned caused fatal problems. The initial fix for this problem was incomplete however as it showed another cases of non-aligned access that previously worked implicitly. This switches to making sure that all access of wMaxPacketSize are done via (get|put)_unaligned_le16.
In order to maintain a level of readability to the code in some cases we now use a variable for the value of wMaxPacketSize and in others, a macro.
OpenRISC:
Beagleboard xM, Pandaboard run-tested, s5p_goni build-tested.
--- common/cmd_usb.c | 3 ++- common/usb.c | 23 +++++++++++++++-------- drivers/serial/usbtty.c | 10 ++++++---- drivers/usb/gadget/epautoconf.c | 8 +++++--- drivers/usb/gadget/s3c_udc_otg.c | 10 ++++++---- include/usbdescriptors.h | 2 +- 6 files changed, 35 insertions(+), 21 deletions(-)
common/cmd_usb.c +2 -1
@@ -28,6 +28,7 @@#include <common.h> #include <command.h> #include <asm/byteorder.h> +#include <asm/unaligned.h> #include <part.h> #include <usb.h>
@@ -240,7 +241,7 @@ void usb_display_ep_desc(struct usb_endpoint_descriptor *epdesc)printf ("Interrupt" );break ; } -printf (" MaxPacket %d" , epdesc->wMaxPacketSize); +printf (" MaxPacket %d" ,get_unaligned_le16 (&epdesc->wMaxPacketSize));if ((epdesc->bmAttributes &0x03 ) ==0x3 )printf (" Interval %dms" , epdesc->bInterval);printf ("\n" );
common/usb.c +15 -8
@@ -49,6 +49,7 @@#include <asm/processor.h> #include <linux/ctype.h> #include <asm/byteorder.h> +#include <asm/unaligned.h> #include <usb.h> #ifdef CONFIG_4xx
@@ -279,30 +280,32 @@ usb_set_maxpacket_ep(struct usb_device *dev, int if_idx, int ep_idx) {int b;struct usb_endpoint_descriptor *ep; +u16 ep_wMaxPacketSize; ep = &dev->config.if_desc[if_idx].ep_desc[ep_idx]; b = ep->bEndpointAddress &USB_ENDPOINT_NUMBER_MASK ; + ep_wMaxPacketSize =get_unaligned_le16 (&ep->wMaxPacketSize);if ((ep->bmAttributes &USB_ENDPOINT_XFERTYPE_MASK ) ==USB_ENDPOINT_XFER_CONTROL ) {/* Control => bidirectional */ - dev->epmaxpacketout[b] = ep->wMaxPacketSize; - dev->epmaxpacketin[b] = ep->wMaxPacketSize; + dev->epmaxpacketout[b] = ep_wMaxPacketSize; + dev->epmaxpacketin[b] = ep_wMaxPacketSize;USB_PRINTF ("##Control EP epmaxpacketout/in[%d] = %d\n" , b, dev->epmaxpacketin[b]); }else {if ((ep->bEndpointAddress &0x80 ) ==0 ) {/* OUT Endpoint */ -if (ep->wMaxPacketSize > dev->epmaxpacketout[b]) { - dev->epmaxpacketout[b] = ep->wMaxPacketSize; +if (ep_wMaxPacketSize > dev->epmaxpacketout[b]) { + dev->epmaxpacketout[b] = ep_wMaxPacketSize;USB_PRINTF ("##EP epmaxpacketout[%d] = %d\n" , b, dev->epmaxpacketout[b]); } }else {/* IN Endpoint */ -if (ep->wMaxPacketSize > dev->epmaxpacketin[b]) { - dev->epmaxpacketin[b] = ep->wMaxPacketSize; +if (ep_wMaxPacketSize > dev->epmaxpacketin[b]) { + dev->epmaxpacketin[b] = ep_wMaxPacketSize;USB_PRINTF ("##EP epmaxpacketin[%d] = %d\n" , b, dev->epmaxpacketin[b]); }
@@ -333,6 +336,7 @@ int usb_parse_config(struct usb_device *dev, unsigned char *buffer, int cfgno)struct usb_descriptor_header *head;int index, ifno, epno, curr_if_num;int i; +u16 ep_wMaxPacketSize; ifno =-1 ; epno =-1 ;
@@ -378,8 +382,11 @@ int usb_parse_config(struct usb_device *dev, unsigned char *buffer, int cfgno) dev->config.if_desc[ifno].no_of_ep++;memcpy (&dev->config.if_desc[ifno].ep_desc[epno], &buffer[index], buffer[index]); -le16_to_cpus (&(dev->config.if_desc[ifno].ep_desc[epno].\ - wMaxPacketSize)); + ep_wMaxPacketSize =get_unaligned_le16 (&dev->config.\ + if_desc[ifno].\ + ep_desc[epno].\ + wMaxPacketSize); +le16_to_cpus (&ep_wMaxPacketSize);USB_PRINTF ("if %d, ep %d\n" , ifno, epno);break ;default :
drivers/serial/usbtty.c +6 -4
@@ -25,6 +25,7 @@#include <config.h> #include <circbuf.h> #include <stdio_dev.h> +#include <asm/unaligned.h> #include "usbtty.h" #include "usb_cdc_acm.h" #include "usbdescriptors.h"
@@ -626,6 +627,9 @@ static void usbtty_init_strings (void) usb_strings = usbtty_string_table; } +#define init_wMaxPacketSize (x) le16_to_cpu(get_unaligned_le16(\ + &ep_descriptor_ptrs[(x) - 1]->wMaxPacketSize)); +static void usbtty_init_instances (void ) {int i;
@@ -688,14 +692,12 @@ static void usbtty_init_instances (void) endpoint_instance[i].rcv_attributes = ep_descriptor_ptrs[i -1 ]->bmAttributes; - endpoint_instance[i].rcv_packetSize = -le16_to_cpu (ep_descriptor_ptrs[i -1 ]->wMaxPacketSize); + endpoint_instance[i].rcv_packetSize =init_wMaxPacketSize (i); endpoint_instance[i].tx_attributes = ep_descriptor_ptrs[i -1 ]->bmAttributes; - endpoint_instance[i].tx_packetSize = -le16_to_cpu (ep_descriptor_ptrs[i -1 ]->wMaxPacketSize); + endpoint_instance[i].tx_packetSize =init_wMaxPacketSize (i); endpoint_instance[i].tx_attributes = ep_descriptor_ptrs[i -1 ]->bmAttributes;
drivers/usb/gadget/epautoconf.c +5 -3
@@ -25,6 +25,7 @@#include <linux/usb/ch9.h> #include <asm/errno.h> #include <linux/usb/gadget.h> +#include <asm/unaligned.h> #include "gadget_chips.h" #define isdigit (c) ('0' <= (c) && (c) <= '9')
@@ -127,7 +128,7 @@ static int ep_matches(* where it's an output parameter representing the full speed limit. * the usb spec fixes high speed bulk maxpacket@512 bytes. */ - max =0x7ff &le16_to_cpu (desc->wMaxPacketSize); + max =0x7ff &le16_to_cpu (get_unaligned_le16 (&desc->wMaxPacketSize));switch (type) {case USB_ENDPOINT_XFER_INT :/* INT: limit 64 bytes full speed, 1024 high speed */
@@ -143,7 +144,8 @@ static int ep_matches(return 0 ;/* BOTH: "high bandwidth" works only at high speed */ -if ((desc->wMaxPacketSize &__constant_cpu_to_le16 (3 <<11 ))) { +if ((get_unaligned_le16 (&desc->wMaxPacketSize) & +__constant_cpu_to_le16 (3 <<11 ))) {if (!gadget->is_dualspeed)return 0 ;/* configure your hardware with enough buffering!! */
@@ -176,7 +178,7 @@ static int ep_matches(/* min() doesn't work on bitfields with gcc-3.5 */ if (size >64 ) size =64 ; - desc->wMaxPacketSize =cpu_to_le16 (size); +put_unaligned_le16 (cpu_to_le16 (size), &desc->wMaxPacketSize); }return 1 ; }
drivers/usb/gadget/s3c_udc_otg.c +6 -4
@@ -40,6 +40,7 @@#include <linux/usb/gadget.h> #include <asm/byteorder.h> +#include <asm/unaligned.h> #include <asm/io.h> #include <asm/mach-types.h>
@@ -586,7 +587,8 @@ static int s3c_ep_enable(struct usb_ep *_ep,if (!_ep || !desc || ep->desc || _ep->name == ep0name || desc->bDescriptorType !=USB_DT_ENDPOINT || ep->bEndpointAddress != desc->bEndpointAddress - ||ep_maxpacket (ep) <le16_to_cpu (desc->wMaxPacketSize)) { + ||ep_maxpacket (ep) < +le16_to_cpu (get_unaligned_le16 (&desc->wMaxPacketSize))) {DEBUG ("%s: bad ep or descriptor\n" , __func__);return -EINVAL ;
@@ -603,8 +605,8 @@ static int s3c_ep_enable(struct usb_ep *_ep,/* hardware _could_ do smaller, but driver doesn't */ if ((desc->bmAttributes ==USB_ENDPOINT_XFER_BULK - &&le16_to_cpu (desc->wMaxPacketSize) !=ep_maxpacket (ep)) - || !desc->wMaxPacketSize) { + &&le16_to_cpu (get_unaligned_le16 (&desc->wMaxPacketSize)) != +ep_maxpacket (ep)) || !get_unaligned_le16 (&desc->wMaxPacketSize)) {DEBUG ("%s: bad %s maxpacket\n" , __func__, _ep->name);return -ERANGE ;
@@ -620,7 +622,7 @@ static int s3c_ep_enable(struct usb_ep *_ep, ep->stopped =0 ; ep->desc = desc; ep->pio_irqs =0 ; - ep->ep.maxpacket =le16_to_cpu (desc->wMaxPacketSize); + ep->ep.maxpacket =le16_to_cpu (get_unaligned_le16 (&desc->wMaxPacketSize));/* Reset halt state */ s3c_udc_set_nak (ep);
include/usbdescriptors.h +1 -1
@@ -199,7 +199,7 @@ struct usb_endpoint_descriptor {u8 bmAttributes;u16 wMaxPacketSize;u8 bInterval; -} __attribute__ ((packed)) __attribute__ ((aligned(2))); +} __attribute__ ((packed));struct usb_interface_descriptor {u8 bLength;
Signature · 1 line
1.7.0.4
PATCH 1/1 USB: Use (get|put)_unaligned_le16 for accessing wMaxPacketSize
Hi Tom,
after taking a second look at this a couple of things came to mind
Quoted 7 lines · Tom Rini
In
9792987721c7980453fe6447c3fa6593b44f8458Stefan describes a usecase where the previous behavior of leaving wMaxPacketSize be unaligned caused fatal problems. The initial fix for this problem was incomplete however as it showed another cases of non-aligned access that previously worked implicitly. This switches to making sure that all access of wMaxPacketSize are done via (get|put)_unaligned_le16.
Why the _le16? Shouldn't it just be (get|put)_unaligned?
common/usb.c int usb_parse_config(struct usb_device *dev, unsigned char *buffer, int cfgno) · line 392
Quoted 10 lines
common/usb.c +5
-2-le16_to_cpus (&(dev->config.if_desc[ifno].ep_desc[epno].\ - wMaxPacketSize)); + ep_wMaxPacketSize =get_unaligned_le16 (&dev->config.\ + if_desc[ifno].\ + ep_desc[epno].\ + wMaxPacketSize); +le16_to_cpus (&ep_wMaxPacketSize);USB_PRINTF ("if %d, ep %d\n" , ifno, epno);break ;default :
Since this code is changing the wMaxPacketSize, it should probably be:
ep_wMaxPacketSize = get_unaligned(&dev->config.\ if_desc[ifno].\ ep_desc[epno].\ wMaxPacketSize); put_unaligned(le16_to_cpu(ep_wMaxPacketSize), &dev->config.\ if_desc[ifno].\ ep_desc[epno].\ wMaxPacketSize);
Stefan