> -----Original Message----- > From: David Brownell [mailto:david-b@xxxxxxxxxxx] > Sent: Monday, November 08, 2010 11:36 PM > To: Xu, Andiry > Cc: linux-usb@xxxxxxxxxxxxxxx; gregkh@xxxxxxx > Subject: Re: [PATCH RESEND] EHCI: AMD periodic frame list table quirk > > 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? > Yes, that's correct. Thanks for ack this. > > 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
