From: Stephen Warren swarren@nvidia.com

Fix ci_ep_submit_next_request()'s ZLP transmission code to explicitly call ci_get_qtd() to find the address of the other QTD to use. This will allow us to correctly align each QTD individually in the future, which may involve leaving a gap between the QTDs.

Signed-off-by: Stephen Warren swarren@nvidia.com --- drivers/usb/gadget/ci_udc.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/gadget/ci_udc.c b/drivers/usb/gadget/ci_udc.c index 8ba604841c47..4115cd5dab0f 100644 --- a/drivers/usb/gadget/ci_udc.c +++ b/drivers/usb/gadget/ci_udc.c @@ -404,10 +404,11 @@ static void ci_ep_submit_next_request(struct ci_ep *ci_ep) * only 1 is used at a time since either an IN or an OUT but * not both is queued. For an IN transaction, item currently * points at the second of these items, so we know that we - * can use (item - 1) to transmit the extra zero-length packet + * can use the other to transmit the extra zero-length packet. */ - item->next = (unsigned)(item - 1); - item--; + struct ept_queue_item *other_item = ci_get_qtd(num, 0); + item->next = (unsigned)other_item; + item = other_item; item->info = INFO_ACTIVE; }

From: Stephen Warren swarren@nvidia.com

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 | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/usb/gadget/ci_udc.c b/drivers/usb/gadget/ci_udc.c index 3a114cf11e17..abaf8985503f 100644 --- a/drivers/usb/gadget/ci_udc.c +++ b/drivers/usb/gadget/ci_udc.c @@ -35,15 +35,20 @@ #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) {

From: Stephen Warren swarren@nvidia.com

struct ci_req is a purely software structure, and needs no specific memory alignment. Hence, allocate it with calloc() rather than memalign(). The use of memalign() was left-over from when struct ci_req was going to hold the aligned bounce buffer, but this is now dynamically allocated.

Signed-off-by: Stephen Warren swarren@nvidia.com --- drivers/usb/gadget/ci_udc.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/ci_udc.c b/drivers/usb/gadget/ci_udc.c index 89138675c32f..b8c36523eb1d 100644 --- a/drivers/usb/gadget/ci_udc.c +++ b/drivers/usb/gadget/ci_udc.c @@ -222,12 +222,11 @@ ci_ep_alloc_request(struct usb_ep *ep, unsigned int gfp_flags) if (num == 0 && controller.ep0_req) return &controller.ep0_req->req;

- ci_req = memalign(ARCH_DMA_MINALIGN, sizeof(*ci_req)); + ci_req = calloc(1, sizeof(*ci_req)); if (!ci_req) return NULL;

INIT_LIST_HEAD(&ci_req->queue); - ci_req->b_buf = 0;

if (num == 0) controller.ep0_req = ci_req;