On Sat, Dec 11, 2010 at 05:06:58AM +0800, Greg KH wrote:
> On Sat, Dec 11, 2010 at 12:40:48AM +0800, Hao Wu wrote:
> > From: Andy Luo <yifei.luo@xxxxxxxxx>
> >
> > The Intel Moorestown platform has MPH and OTG EHCI controllers that have
> > internal SRAM that could be used as descriptors DMA pool caching. The SRAM is
> > exposed via PCI BAR1. The limitation here is the SRAM access should be 32bit
> > aligned.
> >
> > Signed-off-by: Jacob Pan <jacob.jun.pan@xxxxxxxxx>
> > Signed-off-by: Alek Du <alek.du@xxxxxxxxx>
> > Signed-off-by: Alan Cox <alan@xxxxxxxxxxxxxxx>
> > Signed-off-by: Hao Wu <hao.wu@xxxxxxxxx>
> > Signed-off-by: Andy Luo <yifei.luo@xxxxxxxxx>
> > ---
> > drivers/usb/core/buffer.c | 10 ++++++++++
> > drivers/usb/host/ehci-hcd.c | 3 ++-
> > drivers/usb/host/ehci-pci.c | 39 +++++++++++++++++++++++++++++++++++++++
> > drivers/usb/host/ehci.h | 8 +++++++-
> > include/linux/usb/hcd.h | 2 ++
> > 5 files changed, 60 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/usb/core/buffer.c b/drivers/usb/core/buffer.c
> > index 2c69654..150d4b8 100644
> > --- a/drivers/usb/core/buffer.c
> > +++ b/drivers/usb/core/buffer.c
> > @@ -115,6 +115,11 @@ void *hcd_buffer_alloc(
> > return kmalloc(size, mem_flags);
> > }
> >
> > + /* we won't use internal SRAM as data payload, we can't get
> > + any benefits from it */
> > + if (hcd->has_sram && hcd->sram_no_payload)
> > + return dma_alloc_coherent(NULL, size, dma, mem_flags);
>
> So, there shouldn't be any problems with it either, right? Why is this
> needed?
If we have the flag of "has_sram" and "sram_no_payload" which means we don't
want to use this internal SRAM for data payload, then we use "NULL" as the
first parameter for dma_alloc_coherent, in this way the SRAM as declared region
of memory for this device won't be used by hcd_buffer_alloc().
> > +
> > for (i = 0; i < HCD_BUFFER_POOLS; i++) {
> > if (size <= pool_max [i])
> > return dma_pool_alloc(hcd->pool [i], mem_flags, dma);
> > @@ -141,6 +146,11 @@ void hcd_buffer_free(
> > return;
> > }
> >
> > + if (hcd->has_sram && hcd->sram_no_payload) {
> > + dma_free_coherent(NULL, size, addr, dma);
> > + return;
> > + }
> > +
> > for (i = 0; i < HCD_BUFFER_POOLS; i++) {
> > if (size <= pool_max [i]) {
> > dma_pool_free(hcd->pool [i], addr, dma);
> > diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
> > index 769a957..b5dd3ad 100644
> > --- a/drivers/usb/host/ehci-hcd.c
> > +++ b/drivers/usb/host/ehci-hcd.c
> > @@ -529,7 +529,8 @@ static void ehci_stop (struct usb_hcd *hcd)
> > ehci_work (ehci);
> > spin_unlock_irq (&ehci->lock);
> > ehci_mem_cleanup (ehci);
> > -
> > + if (hcd->has_sram)
> > + sram_deinit(hcd);
> > #ifdef EHCI_STATS
> > ehci_dbg (ehci, "irq normal %ld err %ld reclaim %ld (lost %ld)\n",
> > ehci->stats.normal, ehci->stats.error, ehci->stats.reclaim,
> > diff --git a/drivers/usb/host/ehci-pci.c b/drivers/usb/host/ehci-pci.c
> > index 455f0aa..a7d15ac 100644
> > --- a/drivers/usb/host/ehci-pci.c
> > +++ b/drivers/usb/host/ehci-pci.c
> > @@ -41,6 +41,39 @@ static int ehci_pci_reinit(struct ehci_hcd *ehci, struct pci_dev *pdev)
> > return 0;
> > }
> >
> > +/* enable SRAM if sram detected */
> > +static void sram_init(struct usb_hcd *hcd)
> > +{
>
> That sounds pretty specific to a single host controller, right?
> Shouldn't you name this better?
You are right, it's not so good. As it is only used by vender of intel, how about
"intel_sram_init"?
> > + struct ehci_hcd *ehci = hcd_to_ehci(hcd);
> > + struct pci_dev *pdev = to_pci_dev(hcd->self.controller);
> > +
> > + if (!hcd->has_sram)
> > + return;
> > + ehci->sram_addr = pci_resource_start(pdev, 1);
> > + ehci->sram_size = pci_resource_len(pdev, 1);
> > + ehci_info(ehci, "Found HCD SRAM at %x size:%x\n",
> > + ehci->sram_addr, ehci->sram_size);
> > + if (pci_request_region(pdev, 1, kobject_name(&pdev->dev.kobj))) {
> > + ehci_warn(ehci, "SRAM request failed\n");
> > + hcd->has_sram = 0;
> > + } else if (!dma_declare_coherent_memory(&pdev->dev, ehci->sram_addr,
> > + ehci->sram_addr, ehci->sram_size, DMA_MEMORY_MAP)) {
> > + ehci_warn(ehci, "SRAM DMA declare failed\n");
> > + pci_release_region(pdev, 1);
> > + hcd->has_sram = 0;
> > + }
> > +}
> > +
> > +static void sram_deinit(struct usb_hcd *hcd)
> > +{
> > + struct pci_dev *pdev = to_pci_dev(hcd->self.controller);
> > +
> > + if (!hcd->has_sram)
> > + return;
> > + dma_release_declared_memory(&pdev->dev);
> > + pci_release_region(pdev, 1);
> > +}
> > +
> > /* called during probe() after chip reset completes */
> > static int ehci_pci_setup(struct usb_hcd *hcd)
> > {
> > @@ -70,10 +103,16 @@ static int ehci_pci_setup(struct usb_hcd *hcd)
> > hcd->has_tt = 1;
> > ehci->has_hostpc = 1;
> > force_otg_hc_mode = 1;
> > + hcd->has_sram = 1;
> > + hcd->sram_no_payload = 1;
> > + sram_init(hcd);
> > } else if (pdev->device == 0x0806) {
> > ehci_info(ehci, "Detected Langwell MPH\n");
> > hcd->has_tt = 1;
> > ehci->has_hostpc = 1;
> > + hcd->has_sram = 1;
> > + hcd->sram_no_payload = 1;
> > + sram_init(hcd);
> > }
> > }
> >
> > diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
> > index bde823f..50f0072 100644
> > --- a/drivers/usb/host/ehci.h
> > +++ b/drivers/usb/host/ehci.h
> > @@ -144,6 +144,8 @@ struct ehci_hcd { /* one per controller */
> > unsigned has_lpm:1; /* support link power management */
> > unsigned has_ppcd:1; /* support per-port change bits */
> > u8 sbrn; /* packed release number */
> > + unsigned int sram_addr;
> > + unsigned int sram_size;
>
> Are you sure this can fit in an unsigned int? Shouldn't you make this a
> bit more explicit?
Will change it by dma_addr_t.
> >
> > /* irq statistics */
> > #ifdef EHCI_STATS
> > @@ -725,5 +727,9 @@ static inline u32 hc32_to_cpup (const struct ehci_hcd *ehci, const __hc32 *x)
> > #endif /* DEBUG */
> >
> > /*-------------------------------------------------------------------------*/
> > -
> > +#ifdef CONFIG_PCI
> > +static void sram_deinit(struct usb_hcd *hcd);
> > +#else
> > +static void sram_deinit(struct usb_hcd *hcd) { return; };
>
> No need for the return.
Will fix it in next version submission.
Thanks,
Andy
--
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
