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?
> +
> 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?
> + 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?
>
> /* 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.
thanks,
greg k-h
--
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
