USB: Fix memcpy routines

The memcpy like routines for moving to and from usb packet
RAM couldn't deal with all unaligned uses, this fixes their
behavior.  In particular, a previous caller might assume
that the packet RAM addresses were contiguous and attempt
to break up a call into two separate chunks (as the queue
insertion/removal code does).  But this can lead to invalid
pointers passed to these memcpy routines.  A much cleaner
solution is to make the packet RAM address space contiguous.
To do so the memcpy routines take packet RAM addresses
instead of AHB address space mapped addresses and
__usb_ram_start needed to change to be of type usb_uint so
that pointer arithmatic on it worked correctly on all platforms,
this also allowed the usb_sram_addr macro to be simplified.

Signed-off-by: Anton Staaf <robotboy@chromium.org>

BRANCH=None
BUG=None
TEST=make buildall -j
     Verify that USB still works on Ryu and discovery-stm32f072

Change-Id: I479461f07a3203f1e6e0cf9705f512a5a43c4646
Reviewed-on: https://chromium-review.googlesource.com/264764
Trybot-Ready: Anton Staaf <robotboy@chromium.org>
Tested-by: Anton Staaf <robotboy@chromium.org>
Reviewed-by: Anton Staaf <robotboy@chromium.org>
Commit-Queue: Anton Staaf <robotboy@chromium.org>

3 files changed, 31 insertions, 29 deletions

diff --git a/chip/stm32/usb-stream.c b/chip/stm32/usb-stream.c
index f5487372bb..2b3fe35e8f 100644
--- a/chip/stm32/usb-stream.c
+++ b/chip/stm32/usb-stream.c
@@ -17,7 +17,8 @@
 
 static size_t rx_read(struct usb_stream_config const *config)
 {
-	size_t count = btable_ep[config->endpoint].rx_count & 0x3ff;
+	uintptr_t address = btable_ep[config->endpoint].rx_addr;
+	size_t    count   = btable_ep[config->endpoint].rx_count & 0x3ff;
 
 	/*
 	 * Only read the received USB packet if there is enough space in the
@@ -27,17 +28,18 @@ static size_t rx_read(struct usb_stream_config const *config)
 		return 0;
 
 	return producer_write_memcpy(&config->producer,
-				     config->rx_ram,
+				     (void *) address,
 				     count,
 				     memcpy_from_usbram);
 }
 
 static size_t tx_write(struct usb_stream_config const *config)
 {
-	size_t count = consumer_read_memcpy(&config->consumer,
-					    config->tx_ram,
-					    config->tx_size,
-					    memcpy_to_usbram);
+	uintptr_t address = btable_ep[config->endpoint].tx_addr;
+	size_t    count   = consumer_read_memcpy(&config->consumer,
+						(void *) address,
+						config->tx_size,
+						memcpy_to_usbram);
 
 	btable_ep[config->endpoint].tx_count = count;
 
diff --git a/chip/stm32/usb.c b/chip/stm32/usb.c
index ff71b4e582..cb419d998b 100644
--- a/chip/stm32/usb.c
+++ b/chip/stm32/usb.c
@@ -77,6 +77,8 @@ struct stm32_endpoint btable_ep[USB_EP_COUNT]
 static usb_uint ep0_buf_tx[USB_MAX_PACKET_SIZE / 2] __usb_ram;
 static usb_uint ep0_buf_rx[USB_MAX_PACKET_SIZE / 2] __usb_ram;
 
+#define EP0_BUF_TX_SRAM_ADDR ((void *) usb_sram_addr(ep0_buf_tx))
+
 static int set_addr;
 /* remaining size of descriptor data to transfer */
 static int desc_left;
@@ -155,7 +157,7 @@ static void ep0_rx(void)
 			desc_ptr = desc + USB_MAX_PACKET_SIZE;
 			len = USB_MAX_PACKET_SIZE;
 		}
-		memcpy_to_usbram(ep0_buf_tx, desc, len);
+		memcpy_to_usbram(EP0_BUF_TX_SRAM_ADDR, desc, len);
 		if (type == USB_DT_CONFIGURATION)
 			/* set the real descriptor size */
 			ep0_buf_tx[1] = USB_DESC_SIZE;
@@ -166,7 +168,7 @@ static void ep0_rx(void)
 	} else if (req == (USB_DIR_IN | (USB_REQ_GET_STATUS << 8))) {
 		uint16_t zero = 0;
 		/* Get status */
-		memcpy_to_usbram(ep0_buf_tx, (void *)&zero, 2);
+		memcpy_to_usbram(EP0_BUF_TX_SRAM_ADDR, (void *)&zero, 2);
 		btable_ep[0].tx_count = 2;
 		STM32_TOGGLE_EP(0, EP_TX_RX_MASK, EP_TX_RX_VALID,
 			  EP_STATUS_OUT /*null OUT transaction */);
@@ -208,7 +210,7 @@ static void ep0_tx(void)
 	if (desc_ptr) {
 		/* we have an on-going descriptor transfer */
 		int len = MIN(desc_left, USB_MAX_PACKET_SIZE);
-		memcpy_to_usbram(ep0_buf_tx, desc_ptr, len);
+		memcpy_to_usbram(EP0_BUF_TX_SRAM_ADDR, desc_ptr, len);
 		btable_ep[0].tx_count = len;
 		desc_left -= len;
 		desc_ptr += len;
@@ -346,21 +348,15 @@ int usb_is_enabled(void)
 
 void *memcpy_to_usbram(void *dest, const void *src, size_t n)
 {
+	int       unaligned =                 (((uintptr_t) dest) & 1);
+	usb_uint *d         = &__usb_ram_start[((uintptr_t) dest) / 2];
+	uint8_t  *s         = (uint8_t *) src;
+	int       i;
+
 	/*
-	 * The d pointer needs to be volatile to prevent GCC from possibly
-	 * breaking writes to the USB packet RAM into multiple 16-bit writes,
-	 * which, due to the way the AHB2APB bridge works would clobber what
-	 * we write with 32-bit extensions of the 16-bit writes.
+	 * Handle unaligned leading byte via read/modify/write.
 	 */
-	int                i;
-	uint8_t           *s = (uint8_t *) src;
-	usb_uint volatile *d = (usb_uint volatile *)((uintptr_t) dest & ~1);
-
-	if ((((uintptr_t) dest) & 1) && n) {
-		/*
-		 * The first destination byte is not aligned, perform a read/
-		 * modify/write.
-		 */
+	if (unaligned && n) {
 		*d = (*d & ~0xff00) | (*s << 8);
 		n--;
 		s++;
@@ -382,13 +378,16 @@ void *memcpy_to_usbram(void *dest, const void *src, size_t n)
 
 void *memcpy_from_usbram(void *dest, const void *src, size_t n)
 {
-	int       i;
-	usb_uint *s = (usb_uint *)((uintptr_t) src & ~1);
-	uint8_t  *d = (uint8_t *) dest;
+	int             unaligned =                 (((uintptr_t) src) & 1);
+	usb_uint const *s         = &__usb_ram_start[((uintptr_t) src) / 2];
+	uint8_t        *d         = (uint8_t *) dest;
+	int             i;
 
-	if ((((uintptr_t) src) & 1) && n) {
-		*d++ = *s++ >> 8;
+	if (unaligned && n) {
+		*d = *s >> 8;
 		n--;
+		s++;
+		d++;
 	}
 
 	for (i = 0; i < n / 2; i++) {
diff --git a/chip/stm32/usb_hid.c b/chip/stm32/usb_hid.c
index 8b39778202..8973ba9c25 100644
--- a/chip/stm32/usb_hid.c
+++ b/chip/stm32/usb_hid.c
@@ -88,7 +88,7 @@ static usb_uint hid_ep_buf[HID_REPORT_SIZE / 2] __usb_ram;
 
 void set_keyboard_report(uint64_t rpt)
 {
-	memcpy_to_usbram(hid_ep_buf, &rpt, sizeof(rpt));
+	memcpy_to_usbram((void *) usb_sram_addr(hid_ep_buf), &rpt, sizeof(rpt));
 	/* enable TX */
 	STM32_TOGGLE_EP(USB_EP_HID, EP_TX_MASK, EP_TX_VALID, 0);
 }
@@ -124,7 +124,8 @@ static int hid_iface_request(usb_uint *ep0_buf_rx, usb_uint *ep0_buf_tx)
 			      (USB_REQ_GET_DESCRIPTOR << 8))) &&
 			      (ep0_buf_rx[1] == (USB_HID_DT_REPORT << 8))) {
 		/* Setup : HID specific : Get Report descriptor */
-		memcpy_to_usbram(ep0_buf_tx, report_desc,
+		memcpy_to_usbram((void *) usb_sram_addr(ep0_buf_tx),
+				 report_desc,
 				 sizeof(report_desc));
 		btable_ep[0].tx_count = MIN(ep0_buf_rx[3],
 				   sizeof(report_desc));