On Sat, Dec 11, 2010 at 11:38:45PM +0800, Sergei Shtylyov wrote:
> Hello.
>
> On 10-12-2010 19:40, 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>
> [...]
>
> > 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 */
>
> The preferred stly of the multi-line comments is this:
>
> /*
> * bla
> * bla
> */
>
> > 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)
> > +{
> > + 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",
>
> Forgot space after colon?
>
> > + ehci->sram_addr, ehci->sram_size);
> > + if (pci_request_region(pdev, 1, kobject_name(&pdev->dev.kobj))) {
>
> Hm, why dev_name() wasn't enough?
>
> > 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
> [...]
> > @@ -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; };
>
> *return* not needed.
Thank you for your code review, the four places you mentioned above
will be fixed in the next version of submission.
BRs,
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
