From 7cb0bf6923fe613016cd83c511187539e0709af1 Mon Sep 17 00:00:00 2001 From: RJ Ryan Date: Sun, 18 Sep 2011 15:39:28 -0400 Subject: Fix memory alignment of USB descriptor structures. A variety of USB descriptor structures have been manually "unpacked". Instead of using the struct, their members were unpacked into the struct they were nested in. Additionally sizeof()'s were commented in favor of manual calculation of structure sizes. After uncommenting these changes, the USB CDC peripheral stopped correctly configuring with the host. The root problem with the structures is that GCC is padding them. By applying __attribute__((__packed__)), these problems are fixed. I removed all the instances of the workaround I saw within the USB code. Signed-off-by: RJ Ryan --- libmaple/libmaple_types.h | 1 + libmaple/usb/descriptors.c | 125 ++++++++++++++++++++++----------------------- libmaple/usb/descriptors.h | 118 ++++++++++-------------------------------- 3 files changed, 89 insertions(+), 155 deletions(-) diff --git a/libmaple/libmaple_types.h b/libmaple/libmaple_types.h index 17bcfb1..08adaff 100644 --- a/libmaple/libmaple_types.h +++ b/libmaple/libmaple_types.h @@ -47,6 +47,7 @@ typedef void (*voidFuncPtr)(void); #define __io volatile #define __attr_flash __attribute__((section (".USER_FLASH"))) +#define __packed __attribute__((__packed__)) #ifndef NULL #define NULL 0 diff --git a/libmaple/usb/descriptors.c b/libmaple/usb/descriptors.c index 8dd9521..a0bc046 100644 --- a/libmaple/usb/descriptors.c +++ b/libmaple/usb/descriptors.c @@ -46,32 +46,34 @@ const USB_Descriptor_Device usbVcomDescriptor_Device = { }; const USB_Descriptor_Config usbVcomDescriptor_Config = { - bLength: 0x09,//sizeof(USB_Descriptor_Config_Header), - bDescriptorType: USB_DESCRIPTOR_TYPE_CONFIGURATION, - wTotalLength: 0x43,//sizeof(USB_Descriptor_Config), - bNumInterfaces: 0x02, - bConfigurationValue: 0x01, - iConfiguration: 0x00, - bmAttributes: (USB_CONFIG_ATTR_BUSPOWERED | - USB_CONFIG_ATTR_SELF_POWERED), - bMaxPower: USB_CONFIG_MAX_POWER, + Config_Header: { + bLength: sizeof(USB_Descriptor_Config_Header), + bDescriptorType: USB_DESCRIPTOR_TYPE_CONFIGURATION, + wTotalLength: sizeof(USB_Descriptor_Config), + bNumInterfaces: 0x02, + bConfigurationValue: 0x01, + iConfiguration: 0x00, + bmAttributes: (USB_CONFIG_ATTR_BUSPOWERED | + USB_CONFIG_ATTR_SELF_POWERED), + bMaxPower: USB_CONFIG_MAX_POWER, + }, CCI_Interface: { - bLength: 0x09,//sizeof(USB_Descriptor_Interface), + bLength: sizeof(USB_Descriptor_Interface), bDescriptorType: USB_DESCRIPTOR_TYPE_INTERFACE, bInterfaceNumber: 0x00, bAlternateSetting: 0x00, bNumEndpoints: 0x01, - bInterfaceClass: 0x02, - bInterfaceSubClass: 0x02, - bInterfaceProtocol: 0x01, + bInterfaceClass: USB_INTERFACE_CLASS_CDC, + bInterfaceSubClass: USB_INTERFACE_SUBCLASS_CDC_ACM, + bInterfaceProtocol: 0x01, /* Common AT Commands */ iInterface: 0x00 }, CDC_Functional_IntHeader: { - bLength: 0x05,//sizeof(CDC_FUNCTIONAL_DESCRIPTOR(2)), + bLength: CDC_FUNCTIONAL_DESCRIPTOR_SIZE(2), bDescriptorType: 0x24, SubType: 0x00, Data: {0x01, 0x10} @@ -79,7 +81,7 @@ const USB_Descriptor_Config usbVcomDescriptor_Config = { CDC_Functional_CallManagement: { - bLength: 0x05,//sizeof(CDC_FUNCTIONAL_DESCRIPTOR(2)), + bLength: CDC_FUNCTIONAL_DESCRIPTOR_SIZE(2), bDescriptorType: 0x24, SubType: 0x01, Data: {0x03, 0x01} @@ -87,7 +89,7 @@ const USB_Descriptor_Config usbVcomDescriptor_Config = { CDC_Functional_ACM: { - bLength: 0x04,//sizeof(CDC_FUNCTIONAL_DESCRIPTOR(1)), + bLength: CDC_FUNCTIONAL_DESCRIPTOR_SIZE(1), bDescriptorType: 0x24, SubType: 0x02, Data: {0x06} @@ -95,59 +97,54 @@ const USB_Descriptor_Config usbVcomDescriptor_Config = { CDC_Functional_Union: { - bLength: 0x05,//sizeof(CDC_FUNCTIONAL_DESCRIPTOR(2)), + bLength: CDC_FUNCTIONAL_DESCRIPTOR_SIZE(2), bDescriptorType: 0x24, SubType: 0x06, Data: {0x00, 0x01} }, - // ManagementEndpoint: - // { - EP1_bLength: 0x07,//sizeof(USB_Descriptor_Endpoint), - EP1_bDescriptorType: USB_DESCRIPTOR_TYPE_ENDPOINT, - EP1_bEndpointAddress: (USB_DESCRIPTOR_ENDPOINT_IN | VCOM_NOTIFICATION_EPNUM), - EP1_bmAttributes: EP_TYPE_INTERRUPT, - EP1_wMaxPacketSize0: VCOM_NOTIFICATION_EPSIZE, - EP1_wMaxPacketSize1: 0x00, - EP1_bInterval: 0xFF, - // }, - - // DCI_Interface: - // { - DCI_bLength: 0x09,//sizeof(USB_Descriptor_Interface), - DCI_bDescriptorType: USB_DESCRIPTOR_TYPE_INTERFACE, - DCI_bInterfaceNumber: 0x01, - DCI_bAlternateSetting: 0x00, - DCI_bNumEndpoints: 0x02, - DCI_bInterfaceClass: 0x0A, - DCI_bInterfaceSubClass: 0x00, - DCI_bInterfaceProtocol: 0x00, - DCI_iInterface: 0x00, - // }, - - //DataOutEndpoint: - // { - // }, - EP2_bLength: 0x07,//sizeof(USB_Descriptor_Endpoint), - EP2_bDescriptorType: USB_DESCRIPTOR_TYPE_ENDPOINT, - EP2_bEndpointAddress: (USB_DESCRIPTOR_ENDPOINT_OUT | VCOM_RX_EPNUM), - EP2_bmAttributes: EP_TYPE_BULK, - EP2_wMaxPacketSize0: VCOM_RX_EPSIZE, - EP2_wMaxPacketSize1: 0x00, - EP2_bInterval: 0x00, - - - // DataInEndpoint: - // { - EP3_bLength: 0x07,//sizeof(USB_Descriptor_Endpoint), - EP3_bDescriptorType: USB_DESCRIPTOR_TYPE_ENDPOINT, - EP3_bEndpointAddress: (USB_DESCRIPTOR_ENDPOINT_IN | VCOM_TX_EPNUM), - EP3_bmAttributes: EP_TYPE_BULK, - EP3_wMaxPacketSize0: VCOM_TX_EPSIZE, - EP3_wMaxPacketSize1: 0x00, - EP3_bInterval: 0x00 - - // } + ManagementEndpoint: + { + bLength: sizeof(USB_Descriptor_Endpoint), + bDescriptorType: USB_DESCRIPTOR_TYPE_ENDPOINT, + bEndpointAddress: (USB_DESCRIPTOR_ENDPOINT_IN | VCOM_NOTIFICATION_EPNUM), + bmAttributes: EP_TYPE_INTERRUPT, + wMaxPacketSize: VCOM_NOTIFICATION_EPSIZE, + bInterval: 0xFF, + }, + + DCI_Interface: + { + bLength: sizeof(USB_Descriptor_Interface), + bDescriptorType: USB_DESCRIPTOR_TYPE_INTERFACE, + bInterfaceNumber: 0x01, + bAlternateSetting: 0x00, + bNumEndpoints: 0x02, + bInterfaceClass: USB_INTERFACE_CLASS_DIC, + bInterfaceSubClass: 0x00, /* None */ + bInterfaceProtocol: 0x00, /* None */ + iInterface: 0x00, + }, + + DataOutEndpoint: + { + bLength: sizeof(USB_Descriptor_Endpoint), + bDescriptorType: USB_DESCRIPTOR_TYPE_ENDPOINT, + bEndpointAddress: (USB_DESCRIPTOR_ENDPOINT_OUT | VCOM_RX_EPNUM), + bmAttributes: EP_TYPE_BULK, + wMaxPacketSize: VCOM_RX_EPSIZE, + bInterval: 0x00, + }, + + DataInEndpoint: + { + bLength: sizeof(USB_Descriptor_Endpoint), + bDescriptorType: USB_DESCRIPTOR_TYPE_ENDPOINT, + bEndpointAddress: (USB_DESCRIPTOR_ENDPOINT_IN | VCOM_TX_EPNUM), + bmAttributes: EP_TYPE_BULK, + wMaxPacketSize: VCOM_TX_EPSIZE, + bInterval: 0x00 + } }; /***************************************************************************** diff --git a/libmaple/usb/descriptors.h b/libmaple/usb/descriptors.h index 460a88c..13aef29 100644 --- a/libmaple/usb/descriptors.h +++ b/libmaple/usb/descriptors.h @@ -37,6 +37,10 @@ #define USB_DEVICE_CLASS_CDC 0x02 #define USB_DEVICE_SUBCLASS_CDC 0x00 +#define USB_INTERFACE_CLASS_CDC 0x02 +/* CDC Abstract Control Model */ +#define USB_INTERFACE_SUBCLASS_CDC_ACM 0x02 +#define USB_INTERFACE_CLASS_DIC 0x0A #define USB_CONFIG_ATTR_BUSPOWERED 0b10000000 #define USB_CONFIG_ATTR_SELF_POWERED 0b11000000 @@ -59,8 +63,9 @@ extern "C" { uint8 bLength; \ uint8 bDescriptorType; \ uint16 bString[len]; \ - } + } __packed +#define CDC_FUNCTIONAL_DESCRIPTOR_SIZE(DataSize) (3 + DataSize) #define CDC_FUNCTIONAL_DESCRIPTOR(DataSize) \ struct \ { \ @@ -68,8 +73,9 @@ extern "C" { uint8 bDescriptorType; \ uint8 SubType; \ uint8 Data[DataSize]; \ - } + } __packed +/* See http://www.beyondlogic.org/usbnutshell/usb5.shtml#DeviceDescriptors */ typedef struct { uint8 bLength; uint8 bDescriptorType; @@ -85,8 +91,9 @@ typedef struct { uint8 iProduct; uint8 iSerialNumber; uint8 bNumConfigurations; -} USB_Descriptor_Device; +} __packed USB_Descriptor_Device; +/* http://www.beyondlogic.org/usbnutshell/usb5.shtml#ConfigurationDescriptors */ typedef struct { uint8 bLength; uint8 bDescriptorType; @@ -96,8 +103,9 @@ typedef struct { uint8 iConfiguration; uint8 bmAttributes; uint8 bMaxPower; -} USB_Descriptor_Config_Header; +} __packed USB_Descriptor_Config_Header; +/* See http://www.beyondlogic.org/usbnutshell/usb5.shtml#InterfaceDescriptors */ typedef struct { uint8 bLength; uint8 bDescriptorType; @@ -108,7 +116,8 @@ typedef struct { uint8 bInterfaceSubClass; uint8 bInterfaceProtocol; uint8 iInterface; -} USB_Descriptor_Interface; +} __packed USB_Descriptor_Interface; + typedef struct { uint8 bLength; @@ -117,99 +126,26 @@ typedef struct { uint8 bmAttributes; uint16 wMaxPacketSize; uint8 bInterval; -} USB_Descriptor_Endpoint; +} __packed USB_Descriptor_Endpoint; typedef struct { - /* config header */ - uint8 bLength; - uint8 bDescriptorType; - uint16 wTotalLength; - uint8 bNumInterfaces; - uint8 bConfigurationValue; - uint8 iConfiguration; - uint8 bmAttributes; - uint8 bMaxPower; - + USB_Descriptor_Config_Header Config_Header; USB_Descriptor_Interface CCI_Interface; - struct { - uint8 bLength; - uint8 bDescriptorType; - uint8 SubType; - uint8 Data[2]; - } CDC_Functional_IntHeader; - struct { - uint8 bLength; - uint8 bDescriptorType; - uint8 SubType; - uint8 Data[2]; - } CDC_Functional_CallManagement; - struct { - uint8 bLength; - uint8 bDescriptorType; - uint8 SubType; - uint8 Data[1]; - } CDC_Functional_ACM; - struct { - uint8 bLength; - uint8 bDescriptorType; - uint8 SubType; - uint8 Data[2]; - } CDC_Functional_Union; - - /* + CDC_FUNCTIONAL_DESCRIPTOR(2) CDC_Functional_IntHeader; + CDC_FUNCTIONAL_DESCRIPTOR(2) CDC_Functional_CallManagement; + CDC_FUNCTIONAL_DESCRIPTOR(1) CDC_Functional_ACM; + CDC_FUNCTIONAL_DESCRIPTOR(2) CDC_Functional_Union; USB_Descriptor_Endpoint ManagementEndpoint; - */ - uint8 EP1_bLength; - uint8 EP1_bDescriptorType; - uint8 EP1_bEndpointAddress; - uint8 EP1_bmAttributes; - uint8 EP1_wMaxPacketSize0; - uint8 EP1_wMaxPacketSize1; - uint8 EP1_bInterval; - - /* USB_Descriptor_Interface DCI_Interface; - */ - - uint8 DCI_bLength; - uint8 DCI_bDescriptorType; - uint8 DCI_bInterfaceNumber; - uint8 DCI_bAlternateSetting; - uint8 DCI_bNumEndpoints; - uint8 DCI_bInterfaceClass; - uint8 DCI_bInterfaceSubClass; - uint8 DCI_bInterfaceProtocol; - uint8 DCI_iInterface; - - /* USB_Descriptor_Endpoint DataOutEndpoint; USB_Descriptor_Endpoint DataInEndpoint; - */ - - uint8 EP2_bLength; - uint8 EP2_bDescriptorType; - uint8 EP2_bEndpointAddress; - uint8 EP2_bmAttributes; - uint8 EP2_wMaxPacketSize0; - uint8 EP2_wMaxPacketSize1; - uint8 EP2_bInterval; - - uint8 EP3_bLength; - uint8 EP3_bDescriptorType; - uint8 EP3_bEndpointAddress; - uint8 EP3_bmAttributes; - uint8 EP3_wMaxPacketSize0; - uint8 EP3_wMaxPacketSize1; - uint8 EP3_bInterval; - - -}USB_Descriptor_Config; - - typedef struct { - uint8 bLength; - uint8 bDescriptorType; - uint16 bString[]; - } USB_Descriptor_String; +} __packed USB_Descriptor_Config; + +typedef struct { + uint8 bLength; + uint8 bDescriptorType; + uint16 bString[]; +} USB_Descriptor_String; extern const USB_Descriptor_Device usbVcomDescriptor_Device; extern const USB_Descriptor_Config usbVcomDescriptor_Config; -- cgit v1.2.3 From a434d6b99b140ee3f489370d01e12cb1db210376 Mon Sep 17 00:00:00 2001 From: RJ Ryan Date: Sun, 18 Sep 2011 19:17:54 -0400 Subject: Missed one hard-coded structure size. Signed-off-by: RJ Ryan --- libmaple/usb/usb_callbacks.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libmaple/usb/usb_callbacks.c b/libmaple/usb/usb_callbacks.c index 9694942..890a97f 100644 --- a/libmaple/usb/usb_callbacks.c +++ b/libmaple/usb/usb_callbacks.c @@ -14,7 +14,7 @@ ONE_DESCRIPTOR Device_Descriptor = { ONE_DESCRIPTOR Config_Descriptor = { (uint8*)&usbVcomDescriptor_Config, - 0x43//sizeof(USB_Descriptor_Config) + sizeof(USB_Descriptor_Config) }; ONE_DESCRIPTOR String_Descriptor[3] = { -- cgit v1.2.3