usb: ci_udc: fix items array size/stride calculation
authorStephen Warren <swarren@nvidia.com>
Tue, 1 Jul 2014 17:41:16 +0000 (11:41 -0600)
committerMarek Vasut <marex@denx.de>
Wed, 2 Jul 2014 13:45:38 +0000 (15:45 +0200)
2 QTDs are allocated for each EP. The current allocation scheme aligns
the first QTD in each pair, but simply adds the struct size to calculate
the second QTD's address. This will result in a non-cache-aligned
addresss IF the system's ARCH_DMA_MINALIGN is not 32 bytes (i.e. the
size of struct ept_queue_item).

Similarly, the original ilist_ent_sz calculation aligned the value to
ARCH_DMA_MINALIGN but didn't take the USB HW's 32-byte alignment
requirement into account. This doesn't cause a practical issue unless
ARCH_DMA_MINALIGN < 32 (which I suspect is quite unlikely), but we may
as well fix the code to be explicit, so it's obviously completely
correct.

The new value of ILIST_ENT_SZ takes all alignment requirements into
account, so we can simplify ci_{flush,invalidate}_qtd() by simply using
that macro rather than calling roundup().

Similarly, the calculation of controller.items[i] can be simplified,
since each QTD is evenly spaced at its individual alignment requirement,
rather than each pair being aligned, and entries within the pair being
spaced apart only by structure size.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
drivers/usb/gadget/ci_udc.c

index 3a114cf11e17cb3910cfcbc193a492e5eda98fc3..abaf8985503f31ec654b35a4a2ff6d48e13c2cad 100644 (file)
 #endif
 
 /*
- * Each qTD item must be 32-byte aligned, each qTD touple must be
- * cacheline aligned. There are two qTD items for each endpoint and
- * only one of them is used for the endpoint at time, so we can group
- * them together.
+ * Every QTD must be individually aligned, since we can program any
+ * QTD's address into HW. Cache flushing requires ARCH_DMA_MINALIGN,
+ * and the USB HW requires 32-byte alignment. Align to both:
  */
 #define ILIST_ALIGN            roundup(ARCH_DMA_MINALIGN, 32)
-#define ILIST_ENT_RAW_SZ       (2 * sizeof(struct ept_queue_item))
-#define ILIST_ENT_SZ           roundup(ILIST_ENT_RAW_SZ, ARCH_DMA_MINALIGN)
-#define ILIST_SZ               (NUM_ENDPOINTS * ILIST_ENT_SZ)
+/* Each QTD is this size */
+#define ILIST_ENT_RAW_SZ       sizeof(struct ept_queue_item)
+/*
+ * Align the size of the QTD too, so we can add this value to each
+ * QTD's address to get another aligned address.
+ */
+#define ILIST_ENT_SZ           roundup(ILIST_ENT_RAW_SZ, ILIST_ALIGN)
+/* For each endpoint, we need 2 QTDs, one for each of IN and OUT */
+#define ILIST_SZ               (NUM_ENDPOINTS * 2 * ILIST_ENT_SZ)
 
 #ifndef DEBUG
 #define DBG(x...) do {} while (0)
@@ -184,8 +189,7 @@ static void ci_flush_qtd(int ep_num)
 {
        struct ept_queue_item *item = ci_get_qtd(ep_num, 0);
        const uint32_t start = (uint32_t)item;
-       const uint32_t end_raw = start + 2 * sizeof(*item);
-       const uint32_t end = roundup(end_raw, ARCH_DMA_MINALIGN);
+       const uint32_t end = start + 2 * ILIST_ENT_SZ;
 
        flush_dcache_range(start, end);
 }
@@ -200,8 +204,7 @@ static void ci_invalidate_qtd(int ep_num)
 {
        struct ept_queue_item *item = ci_get_qtd(ep_num, 0);
        const uint32_t start = (uint32_t)item;
-       const uint32_t end_raw = start + 2 * sizeof(*item);
-       const uint32_t end = roundup(end_raw, ARCH_DMA_MINALIGN);
+       const uint32_t end = start + 2 * ILIST_ENT_SZ;
 
        invalidate_dcache_range(start, end);
 }
@@ -828,10 +831,7 @@ static int ci_udc_probe(void)
                head->next = TERMINATE;
                head->info = 0;
 
-               imem = controller.items_mem + ((i >> 1) * ILIST_ENT_SZ);
-               if (i & 1)
-                       imem += sizeof(struct ept_queue_item);
-
+               imem = controller.items_mem + (i * ILIST_ENT_SZ);
                controller.items[i] = (struct ept_queue_item *)imem;
 
                if (i & 1) {