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.

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.

Cc: Minkyu Kang mk7.kang@samsung.com Cc: Remy Bohmer linux@bohmer.net

OpenRISC: Tested-by: Stefan Kristiansson stefan.kristiansson@saunalahti.fi

Beagleboard xM, Pandaboard run-tested, s5p_goni build-tested. Signed-off-by: Tom Rini trini@ti.com --- common/cmd_usb.c | 3 ++- common/usb.c | 27 +++++++++++++++++++-------- 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, 39 insertions(+), 21 deletions(-)

diff --git a/common/cmd_usb.c b/common/cmd_usb.c index d4ec2a2..320667f 100644 --- a/common/cmd_usb.c +++ b/common/cmd_usb.c @@ -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(&epdesc->wMaxPacketSize)); if ((epdesc->bmAttributes & 0x03) == 0x3) printf(" Interval %dms", epdesc->bInterval); printf("\n"); diff --git a/common/usb.c b/common/usb.c index 4418c70..63a11c8 100644 --- a/common/usb.c +++ b/common/usb.c @@ -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(&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,15 @@ 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(&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); USB_PRINTF("if %d, ep %d\n", ifno, epno); break; default: diff --git a/drivers/serial/usbtty.c b/drivers/serial/usbtty.c index e2e87fe..a263b2c 100644 --- a/drivers/serial/usbtty.c +++ b/drivers/serial/usbtty.c @@ -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(\ + &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; diff --git a/drivers/usb/gadget/epautoconf.c b/drivers/usb/gadget/epautoconf.c index 1896489..5b8776e 100644 --- a/drivers/usb/gadget/epautoconf.c +++ b/drivers/usb/gadget/epautoconf.c @@ -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 at 512 bytes. */ - max = 0x7ff & le16_to_cpu(desc->wMaxPacketSize); + max = 0x7ff & le16_to_cpu(get_unaligned(&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(&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(cpu_to_le16(size), &desc->wMaxPacketSize); } return 1; } diff --git a/drivers/usb/gadget/s3c_udc_otg.c b/drivers/usb/gadget/s3c_udc_otg.c index 5a3ac78..901fac9 100644 --- a/drivers/usb/gadget/s3c_udc_otg.c +++ b/drivers/usb/gadget/s3c_udc_otg.c @@ -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(&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(&desc->wMaxPacketSize)) != + ep_maxpacket(ep)) || !get_unaligned(&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(&desc->wMaxPacketSize));

/* Reset halt state */ s3c_udc_set_nak(ep); diff --git a/include/usbdescriptors.h b/include/usbdescriptors.h index 392fcf5..2dec3b9 100644 --- a/include/usbdescriptors.h +++ b/include/usbdescriptors.h @@ -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;