[PATCH] usb: gadget: aspeed: Workaround memory ordering issue

2018-07-11 Thread Benjamin Herrenschmidt
The Aspeed SoC has a memory ordering issue that (thankfully)
only affects the USB gadget device. A read back is necessary
after writing to memory and before letting the device DMA
from it.

Signed-off-by: Benjamin Herrenschmidt 
---
 drivers/usb/gadget/udc/aspeed-vhub/ep0.c  |  2 ++
 drivers/usb/gadget/udc/aspeed-vhub/epn.c  | 14 +++---
 drivers/usb/gadget/udc/aspeed-vhub/vhub.h | 33 +++
 3 files changed, 46 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/gadget/udc/aspeed-vhub/ep0.c 
b/drivers/usb/gadget/udc/aspeed-vhub/ep0.c
index 44f2b3b53b2f..e2927fb083cf 100644
--- a/drivers/usb/gadget/udc/aspeed-vhub/ep0.c
+++ b/drivers/usb/gadget/udc/aspeed-vhub/ep0.c
@@ -219,6 +219,8 @@ static void ast_vhub_ep0_do_send(struct ast_vhub_ep *ep,
if (chunk && req->req.buf)
memcpy(ep->buf, req->req.buf + req->req.actual, chunk);
 
+   vhub_dma_workaround(ep->buf);
+
/* Remember chunk size and trigger send */
reg = VHUB_EP0_SET_TX_LEN(chunk);
writel(reg, ep->ep0.ctlstat);
diff --git a/drivers/usb/gadget/udc/aspeed-vhub/epn.c 
b/drivers/usb/gadget/udc/aspeed-vhub/epn.c
index 80c9feac5147..5939eb1e97f2 100644
--- a/drivers/usb/gadget/udc/aspeed-vhub/epn.c
+++ b/drivers/usb/gadget/udc/aspeed-vhub/epn.c
@@ -66,11 +66,16 @@ static void ast_vhub_epn_kick(struct ast_vhub_ep *ep, 
struct ast_vhub_req *req)
if (!req->req.dma) {
 
/* For IN transfers, copy data over first */
-   if (ep->epn.is_in)
+   if (ep->epn.is_in) {
memcpy(ep->buf, req->req.buf + act, chunk);
+   vhub_dma_workaround(ep->buf);
+   }
writel(ep->buf_dma, ep->epn.regs + AST_VHUB_EP_DESC_BASE);
-   } else
+   } else {
+   if (ep->epn.is_in)
+   vhub_dma_workaround(req->req.buf);
writel(req->req.dma + act, ep->epn.regs + 
AST_VHUB_EP_DESC_BASE);
+   }
 
/* Start DMA */
req->active = true;
@@ -161,6 +166,7 @@ static inline unsigned int ast_vhub_count_free_descs(struct 
ast_vhub_ep *ep)
 static void ast_vhub_epn_kick_desc(struct ast_vhub_ep *ep,
   struct ast_vhub_req *req)
 {
+   struct ast_vhub_desc *desc = NULL;
unsigned int act = req->act_count;
unsigned int len = req->req.length;
unsigned int chunk;
@@ -177,7 +183,6 @@ static void ast_vhub_epn_kick_desc(struct ast_vhub_ep *ep,
 
/* While we can create descriptors */
while (ast_vhub_count_free_descs(ep) && req->last_desc < 0) {
-   struct ast_vhub_desc *desc;
unsigned int d_num;
 
/* Grab next free descriptor */
@@ -227,6 +232,9 @@ static void ast_vhub_epn_kick_desc(struct ast_vhub_ep *ep,
req->act_count = act = act + chunk;
}
 
+   if (likely(desc))
+   vhub_dma_workaround(desc);
+
/* Tell HW about new descriptors */
writel(VHUB_EP_DMA_SET_CPU_WPTR(ep->epn.d_next),
   ep->epn.regs + AST_VHUB_EP_DESC_STATUS);
diff --git a/drivers/usb/gadget/udc/aspeed-vhub/vhub.h 
b/drivers/usb/gadget/udc/aspeed-vhub/vhub.h
index 2b040257bc1f..4ed03d33a5a9 100644
--- a/drivers/usb/gadget/udc/aspeed-vhub/vhub.h
+++ b/drivers/usb/gadget/udc/aspeed-vhub/vhub.h
@@ -462,6 +462,39 @@ enum std_req_rc {
 #define DDBG(d, fmt, ...)  do { } while(0)
 #endif
 
+static inline void vhub_dma_workaround(void *addr)
+{
+   /*
+* This works around a confirmed HW issue with the Aspeed chip.
+*
+* The core uses a different bus to memory than the AHB going to
+* the USB device controller. Due to the latter having a higher
+* priority than the core for arbitration on that bus, it's
+* possible for an MMIO to the device, followed by a DMA by the
+* device from memory to all be performed and services before
+* a previous store to memory gets completed.
+*
+* This the following scenario can happen:
+*
+*- Driver writes to a DMA descriptor (Mbus)
+*- Driver writes to the MMIO register to start the DMA (AHB)
+*- The gadget sees the second write and sends a read of the
+*  descriptor to the memory controller (Mbus)
+*- The gadget hits memory before the descriptor write
+*  causing it to read an obsolete value.
+*
+* Thankfully the problem is limited to the USB gadget device, other
+* masters in the SoC all have a lower priority than the core, thus
+* ensuring that the store by the core arrives first.
+*
+* The workaround consists of using a dummy read of the memory before
+* doing the MMIO writes. This will ensure that the previous writes
+* have been "pushed out".
+*/
+   mb();
+   (void)__raw_readl((void __iomem *)addr);
+}
+
 /* core.c */
 void 

Re: [1/5] xhci: Fix perceived dead host due to runtime suspend race with event handler

2018-07-11 Thread Kai-Heng Feng

Hi Mathias,

at 21:19, Mathias Nyman  wrote:


Don't rely on event interrupt (EINT) bit alone to detect pending port
change in resume. If no change event is detected the host may be suspended
again, oterwise roothubs are resumed.

There is a lag in xHC setting EINT. If we don't notice the pending change
in resume, and the controller is runtime suspeded again, it causes the
event handler to assume host is dead as it will fail to read xHC registers
once PCI puts the controller to D3 state.

[  268.520969] xhci_hcd: xhci_resume: starting port polling.
[  268.520985] xhci_hcd: xhci_hub_status_data: stopping port polling.
[  268.521030] xhci_hcd: xhci_suspend: stopping port polling.
[  268.521040] xhci_hcd: // Setting command ring address to 0x349bd001
[  268.521139] xhci_hcd: Port Status Change Event for port 3
[  268.521149] xhci_hcd: resume root hub
[  268.521163] xhci_hcd: port resume event for port 3
[  268.521168] xhci_hcd: xHC is not running.
[  268.521174] xhci_hcd: handle_port_status: starting port polling.
[  268.596322] xhci_hcd: xhci_hc_died: xHCI host controller not  
responding, assume dead


The EINT lag is described in a additional note in xhci specs 4.19.2:

"Due to internal xHC scheduling and system delays, there will be a lag
between a change bit being set and the Port Status Change Event that it
generated being written to the Event Ring. If SW reads the PORTSC and
sees a change bit set, there is no guarantee that the corresponding Port
Status Change Event has already been written into the Event Ring."

Cc: 



I tried to backport this patch to v4.15, and "xhci: Create new structures  
to store xhci port information" series is a dependency for this patch.


The series brings substantial changes, so I am wondering if you have any  
plan to backport this patch to older kernels?


Kai-Heng



Signed-off-by: Mathias Nyman 



--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Apply for a 3% loan...

2018-07-11 Thread Matt Adams
Hello, We offer L oans at 3% interest rate per annum. If intereted, contact me 
with amount needed and L oan duration for more details...
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html