The udc uses an shared dma memory space between hard and software. This
memory layout is described in ci13xxx_qh and ci13xxx_td which are marked
with the attribute ((packed)).
The packed attribute leads the compiler to generate one byte operations
for addressing the mapped memory as it believes this memory has no
alignement issues as common streaming data. This appeares on armv5
machines where the hardware does not support unaligned 32bit operations.
Compilers for newer ARMs will probably still generate 32bit operations,
as the hardware supports it.
The Datasheet of the synopsys core describes, that some operations on
the mapped memory need to be atomic double word operations. I.e. the
next pointer addressing in the qhead, as otherwise the hardware could
read wrong data and totally stuck.
This patch fixes that issue by addressing every mapped area operation
with explicit readl and writel operations where needed. It also adds an
wmb() for the prepared TD data before it gets enqueued into the qhead.
We also remove the packed attribute as other patches could not address
this memory handling correct and leak new causes for the issue to come
back.
Signed-off-by: Michael Grzeschik <m.grzeschik@xxxxxxxxxxxxxx>
---
drivers/usb/chipidea/debug.c | 2 +-
drivers/usb/chipidea/udc.c | 34 +++++++++++++++++-----------------
drivers/usb/chipidea/udc.h | 4 ++--
3 files changed, 20 insertions(+), 20 deletions(-)
diff --git a/drivers/usb/chipidea/debug.c b/drivers/usb/chipidea/debug.c
index 36a7063..768fea4 100644
--- a/drivers/usb/chipidea/debug.c
+++ b/drivers/usb/chipidea/debug.c
@@ -180,7 +180,7 @@ static int ci_requests_show(struct seq_file *s, void *data)
for (j = 0; j < qsize; j++)
seq_printf(s, " %04X: %08X\n", j,
- *((u32 *)req->ptr + j));
+ readl(&req->ptr + j));
}
spin_unlock_irqrestore(&ci->lock, flags);
diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index 4a7b356..fd702e9 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -446,6 +446,8 @@ static int _hardware_enqueue(struct ci13xxx_ep *mEp, struct ci13xxx_req *mReq)
mReq->ptr->page[i] =
(mReq->req.dma + i * CI13XXX_PAGE_SIZE) & ~TD_RESERVED_MASK;
+ wmb();
+
if (!list_empty(&mEp->qh.queue)) {
struct ci13xxx_req *mReqPrev;
int n = hw_ep_bit(mEp->num, mEp->dir);
@@ -454,9 +456,9 @@ static int _hardware_enqueue(struct ci13xxx_ep *mEp, struct ci13xxx_req *mReq)
mReqPrev = list_entry(mEp->qh.queue.prev,
struct ci13xxx_req, queue);
if (mReqPrev->zptr)
- mReqPrev->zptr->next = mReq->dma & TD_ADDR_MASK;
+ writel(mReq->dma & TD_ADDR_MASK, &mReqPrev->zptr->next);
else
- mReqPrev->ptr->next = mReq->dma & TD_ADDR_MASK;
+ writel(mReq->dma & TD_ADDR_MASK, &mReqPrev->ptr->next);
wmb();
if (hw_read(ci, OP_ENDPTPRIME, BIT(n)))
goto done;
@@ -468,11 +470,10 @@ static int _hardware_enqueue(struct ci13xxx_ep *mEp, struct ci13xxx_req *mReq)
if (tmp_stat)
goto done;
}
-
/* QH configuration */
- mEp->qh.ptr->td.next = mReq->dma; /* TERMINATE = 0 */
- mEp->qh.ptr->td.token &= ~TD_STATUS; /* clear status */
- mEp->qh.ptr->cap |= QH_ZLT;
+ writel(mReq->dma, &mEp->qh.ptr->td.next); /* TERMINATE = 0 */
+ writel(readl(&mEp->qh.ptr->td.token) & ~TD_STATUS, &mEp->qh.ptr->td.token);
+ writel(readl(&mEp->qh.ptr->cap) | QH_ZLT, &mEp->qh.ptr->cap);
wmb(); /* synchronize before ep prime */
@@ -494,11 +495,11 @@ static int _hardware_dequeue(struct ci13xxx_ep *mEp, struct ci13xxx_req *mReq)
if (mReq->req.status != -EALREADY)
return -EINVAL;
- if ((TD_STATUS_ACTIVE & mReq->ptr->token) != 0)
+ if ((TD_STATUS_ACTIVE & readl(&mReq->ptr->token)) != 0)
return -EBUSY;
if (mReq->zptr) {
- if ((TD_STATUS_ACTIVE & mReq->zptr->token) != 0)
+ if ((TD_STATUS_ACTIVE & readl(&mReq->zptr->token)) != 0)
return -EBUSY;
dma_pool_free(mEp->td_pool, mReq->zptr, mReq->zdma);
mReq->zptr = NULL;
@@ -508,7 +509,7 @@ static int _hardware_dequeue(struct ci13xxx_ep *mEp, struct ci13xxx_req *mReq)
usb_gadget_unmap_request(&mEp->ci->gadget, &mReq->req, mEp->dir);
- mReq->req.status = mReq->ptr->token & TD_STATUS;
+ mReq->req.status = readl(&mReq->ptr->token) & TD_STATUS;
if ((TD_STATUS_HALTED & mReq->req.status) != 0)
mReq->req.status = -1;
else if ((TD_STATUS_DT_ERR & mReq->req.status) != 0)
@@ -516,7 +517,7 @@ static int _hardware_dequeue(struct ci13xxx_ep *mEp, struct ci13xxx_req *mReq)
else if ((TD_STATUS_TR_ERR & mReq->req.status) != 0)
mReq->req.status = -1;
- mReq->req.actual = mReq->ptr->token & TD_TOTAL_BYTES;
+ mReq->req.actual = readl(&mReq->ptr->token) & TD_TOTAL_BYTES;
mReq->req.actual >>= __ffs(TD_TOTAL_BYTES);
mReq->req.actual = mReq->req.length - mReq->req.actual;
mReq->req.actual = mReq->req.status ? 0 : mReq->req.actual;
@@ -786,7 +787,7 @@ __acquires(mEp->lock)
if (retval < 0)
break;
list_del_init(&mReq->queue);
- trace_ci_ep_complete_req(mEp, mReq->ptr->token, retval);
+ trace_ci_ep_complete_req(mEp, readl(&mReq->ptr->token), retval);
if (mReq->req.complete != NULL) {
spin_unlock(mEp->lock);
if ((mEp->type == USB_ENDPOINT_XFER_CONTROL) &&
@@ -1026,18 +1027,17 @@ static int ep_enable(struct usb_ep *ep,
trace_ci_ep_enable(mEp, 0);
- mEp->qh.ptr->cap = 0;
+ writel(0, &mEp->qh.ptr->cap);
if (mEp->type == USB_ENDPOINT_XFER_CONTROL)
- mEp->qh.ptr->cap |= QH_IOS;
+ writel(readl(&mEp->qh.ptr->cap) | QH_IOS, &mEp->qh.ptr->cap);
else if (mEp->type == USB_ENDPOINT_XFER_ISOC)
mEp->qh.ptr->cap &= ~QH_MULT;
else
- mEp->qh.ptr->cap &= ~QH_ZLT;
+ writel(readl(&mEp->qh.ptr->cap) & ~QH_ZLT, &mEp->qh.ptr->cap);
- mEp->qh.ptr->cap |=
- (mEp->ep.maxpacket << __ffs(QH_MAX_PKT)) & QH_MAX_PKT;
- mEp->qh.ptr->td.next |= TD_TERMINATE; /* needed? */
+ writel(readl(&mEp->qh.ptr->cap) | ((mEp->ep.maxpacket << __ffs(QH_MAX_PKT)) & QH_MAX_PKT), &mEp->qh.ptr->cap);
+ writel(readl(&mEp->qh.ptr->td.next) | TD_TERMINATE, &mEp->qh.ptr->td.next); /* needed? */
/*
* Enable endpoints in the HW other than ep0 as ep0
diff --git a/drivers/usb/chipidea/udc.h b/drivers/usb/chipidea/udc.h
index d644574..be60464 100644
--- a/drivers/usb/chipidea/udc.h
+++ b/drivers/usb/chipidea/udc.h
@@ -38,7 +38,7 @@ struct ci13xxx_td {
#define TD_CURR_OFFSET (0x0FFFUL << 0)
#define TD_FRAME_NUM (0x07FFUL << 0)
#define TD_RESERVED_MASK (0x0FFFUL << 0)
-} __attribute__ ((packed));
+};
/* DMA layout of queue heads */
struct ci13xxx_qh {
@@ -55,7 +55,7 @@ struct ci13xxx_qh {
/* 9 */
u32 RESERVED;
struct usb_ctrlrequest setup;
-} __attribute__ ((packed));
+};
/**
* struct ci13xxx_req - usb request representation
--
1.8.2.rc2
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
