Thanks for adding the comment to explain that quirk to future patch
developers...
My ack is below; I still haven't got time to closely eyeball it, but it
seems clean enough, and any issues would look to be easily shaken out.
SB700 -- that's current stuff, so this would count as a bugfix that
should merge soonish ... correct?
On Mon, 2010-11-08 at 17:58 +0800, Andiry Xu wrote:
> >From 021f633dc60f89f8fb94783b3494e06e8c496ebb Mon Sep 17 00:00:00 2001
> From: Andiry Xu <andiry.xu@xxxxxxx>
> Date: Mon, 8 Nov 2010 15:32:20 +0800
> Subject: [PATCH] EHCI: AMD periodic frame list table quirk
>
> On AMD SB700/SB800/Hudson-2/3 platforms, USB EHCI controller may read/write
> to memory space not allocated to USB controller if there is longer than
> normal latency on DMA read encountered. In this condition the exposure will
> be encountered only if the driver has following format of Periodic Frame
> List link pointer structure:
>
> For any idle periodic schedule, the Frame List link pointers that have the
> T-bit set to 1 intending to terminate the use of frame list link pointer
> as a physical memory pointer.
>
> Idle periodic schedule Frame List Link pointer shoule be in the following
> format to avoid the issue:
>
> Frame list link pointer should be always contains a valid pointer to a
> inactive QHead with T-bit set to 0.
>
> Signed-off-by: Andiry Xu <andiry.xu@xxxxxxx>
Acked-by: David Brownell <dbrownell@xxxxxxxxxxxxxxxxxxxxx>
> ---
> drivers/usb/host/ehci-mem.c | 26 ++++++++++++++++++++++++--
> drivers/usb/host/ehci-pci.c | 13 +++++++++++++
> drivers/usb/host/ehci-sched.c | 21 ++++++++++++++++++---
> drivers/usb/host/ehci.h | 2 ++
> 4 files changed, 57 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/usb/host/ehci-mem.c b/drivers/usb/host/ehci-mem.c
> index d36e4e7..12f70c3 100644
> --- a/drivers/usb/host/ehci-mem.c
> +++ b/drivers/usb/host/ehci-mem.c
> @@ -141,6 +141,10 @@ static void ehci_mem_cleanup (struct ehci_hcd *ehci)
> qh_put (ehci->async);
> ehci->async = NULL;
>
> + if (ehci->dummy)
> + qh_put(ehci->dummy);
> + ehci->dummy = NULL;
> +
> /* DMA consistent memory and pools */
> if (ehci->qtd_pool)
> dma_pool_destroy (ehci->qtd_pool);
> @@ -227,8 +231,26 @@ static int ehci_mem_init (struct ehci_hcd *ehci, gfp_t flags)
> if (ehci->periodic == NULL) {
> goto fail;
> }
> - for (i = 0; i < ehci->periodic_size; i++)
> - ehci->periodic [i] = EHCI_LIST_END(ehci);
> +
> + if (ehci->use_dummy_qh) {
> + struct ehci_qh_hw *hw;
> + ehci->dummy = ehci_qh_alloc(ehci, flags);
> + if (!ehci->dummy)
> + goto fail;
> +
> + hw = ehci->dummy->hw;
> + hw->hw_next = EHCI_LIST_END(ehci);
> + hw->hw_qtd_next = EHCI_LIST_END(ehci);
> + hw->hw_alt_next = EHCI_LIST_END(ehci);
> + hw->hw_token &= ~QTD_STS_ACTIVE;
> + ehci->dummy->hw = hw;
> +
> + for (i = 0; i < ehci->periodic_size; i++)
> + ehci->periodic[i] = ehci->dummy->qh_dma;
> + } else {
> + for (i = 0; i < ehci->periodic_size; i++)
> + ehci->periodic[i] = EHCI_LIST_END(ehci);
> + }
>
> /* software shadow of hardware table */
> ehci->pshadow = kcalloc(ehci->periodic_size, sizeof(void *), flags);
> diff --git a/drivers/usb/host/ehci-pci.c b/drivers/usb/host/ehci-pci.c
> index a1e8d27..01bb72b 100644
> --- a/drivers/usb/host/ehci-pci.c
> +++ b/drivers/usb/host/ehci-pci.c
> @@ -103,6 +103,19 @@ static int ehci_pci_setup(struct usb_hcd *hcd)
> if (retval)
> return retval;
>
> + if ((pdev->vendor == PCI_VENDOR_ID_AMD && pdev->device == 0x7808) ||
> + (pdev->vendor == PCI_VENDOR_ID_ATI && pdev->device == 0x4396)) {
> + /* EHCI controller on AMD SB700/SB800/Hudson-2/3 platforms may
> + * read/write memory space which does not belong to it when
> + * there is NULL pointer with T-bit set to 1 in the frame list
> + * table. To avoid the issue, the frame list link pointer
> + * should always contain a valid pointer to a inactive qh.
> + */
> + ehci->use_dummy_qh = 1;
> + ehci_info(ehci, "applying AMD SB700/SB800/Hudson-2/3 EHCI "
> + "dummy qh workaround\n");
> + }
> +
> /* data structure init */
> retval = ehci_init(hcd);
> if (retval)
> diff --git a/drivers/usb/host/ehci-sched.c b/drivers/usb/host/ehci-sched.c
> index a92526d..d9f78eb 100644
> --- a/drivers/usb/host/ehci-sched.c
> +++ b/drivers/usb/host/ehci-sched.c
> @@ -98,7 +98,14 @@ static void periodic_unlink (struct ehci_hcd *ehci, unsigned frame, void *ptr)
> */
> *prev_p = *periodic_next_shadow(ehci, &here,
> Q_NEXT_TYPE(ehci, *hw_p));
> - *hw_p = *shadow_next_periodic(ehci, &here, Q_NEXT_TYPE(ehci, *hw_p));
> +
> + if (!ehci->use_dummy_qh ||
> + *shadow_next_periodic(ehci, &here, Q_NEXT_TYPE(ehci, *hw_p))
> + != EHCI_LIST_END(ehci))
> + *hw_p = *shadow_next_periodic(ehci, &here,
> + Q_NEXT_TYPE(ehci, *hw_p));
> + else
> + *hw_p = ehci->dummy->qh_dma;
> }
>
> /* how many of the uframe's 125 usecs are allocated? */
> @@ -2335,7 +2342,11 @@ restart:
> * pointer for much longer, if at all.
> */
> *q_p = q.itd->itd_next;
> - *hw_p = q.itd->hw_next;
> + if (!ehci->use_dummy_qh ||
> + q.itd->hw_next != EHCI_LIST_END(ehci))
> + *hw_p = q.itd->hw_next;
> + else
> + *hw_p = ehci->dummy->qh_dma;
> type = Q_NEXT_TYPE(ehci, q.itd->hw_next);
> wmb();
> modified = itd_complete (ehci, q.itd);
> @@ -2368,7 +2379,11 @@ restart:
> * URB completion.
> */
> *q_p = q.sitd->sitd_next;
> - *hw_p = q.sitd->hw_next;
> + if (!ehci->use_dummy_qh ||
> + q.sitd->hw_next != EHCI_LIST_END(ehci))
> + *hw_p = q.sitd->hw_next;
> + else
> + *hw_p = ehci->dummy->qh_dma;
> type = Q_NEXT_TYPE(ehci, q.sitd->hw_next);
> wmb();
> modified = sitd_complete (ehci, q.sitd);
> diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
> index bde823f..ba8eab3 100644
> --- a/drivers/usb/host/ehci.h
> +++ b/drivers/usb/host/ehci.h
> @@ -73,6 +73,7 @@ struct ehci_hcd { /* one per controller */
>
> /* async schedule support */
> struct ehci_qh *async;
> + struct ehci_qh *dummy; /* For AMD quirk use */
> struct ehci_qh *reclaim;
> unsigned scanning : 1;
>
> @@ -131,6 +132,7 @@ struct ehci_hcd { /* one per controller */
> unsigned need_io_watchdog:1;
> unsigned broken_periodic:1;
> unsigned fs_i_thresh:1; /* Intel iso scheduling */
> + unsigned use_dummy_qh:1; /* AMD Frame List table quirk*/
>
> /* required for usb32 quirk */
> #define OHCI_CTRL_HCFS (3 << 6)
--
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
