Re: [PATCH 4/5] xhci: Fix command ring pointer corruption while aborting a command

youling 257
2021-10-25T15:01:36Z

test this patch suspend resume usb can work.

2021-10-25 19:21 GMT+08:00, Mathias Nyman <mathias.nyman@linux.intel.com>:
> Hi
>
>>> This patch cause suspend to disk resume usb not work, xhci_hcd
>>> 0000:00:14.0:
>>> Abort failed to stop command ring: -110.
>
> Thanks for the report, this is odd.
>
> Could you double check that by reverting this patch resume start working
> again.
>
> If this is the case maybe we need to write all 64bits before this xHC
> hardware reacts to
> CRCR register changes.
>
> Maybe following changes on top of current patch could help:
>
> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> index 311597bba80e..32665637d5e5 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
> @@ -366,7 +366,7 @@ static void xhci_handle_stopped_cmd_ring(struct xhci_hcd
> *xhci,
> /* Must be called with xhci->lock held, releases and aquires lock back */
> static int xhci_abort_cmd_ring(struct xhci_hcd *xhci, unsigned long flags)
> {
> - u32 temp_32;
> + u64 crcr;
> int ret;
>
> xhci_dbg(xhci, "Abort command ring\n");
> @@ -375,13 +375,15 @@ static int xhci_abort_cmd_ring(struct xhci_hcd *xhci,
> unsigned long flags)
>
> /*
> * The control bits like command stop, abort are located in lower
> - * dword of the command ring control register. Limit the write
> - * to the lower dword to avoid corrupting the command ring pointer
> - * in case if the command ring is stopped by the time upper dword
> - * is written.
> + * dword of the command ring control register. Some hw require all
> + * 64 bits to be written, starting with lower dword.
> + * Make sure the upper dword is valid to avoid corrupting the command
> + * ring pointer in case if the command ring is stopped by the time upper
> + * dword is written.
> */
> - temp_32 = readl(&xhci->op_regs->cmd_ring);
> - writel(temp_32 | CMD_RING_ABORT, &xhci->op_regs->cmd_ring);
> + crcr = xhci_trb_virt_to_dma(xhci->cmd_ring->deq_seg,
> + xhci->cmd_ring->dequeue);
> + xhci_write_64(xhci, crcr | CMD_RING_ABORT, &xhci->op_regs->cmd_ring);
>
> /* Section 4.6.1.2 of xHCI 1.0 spec says software should also time the
> * completion of the Command Abort operation. If CRR is not negated in 5
>
> -Mathias
>