On Thu, May 15, 2014 at 03:11:48PM +0100, Alan Stern wrote:
> On Wed, 14 May 2014, Mark Brown wrote:
>
> > From: Liviu Dudau <Liviu.Dudau@xxxxxxx>
> >
> > arm64 architecture handles correctly 64bit DMAs and can enable support
> > for 64bit EHCI host controllers.
> >
> > Signed-off-by: Liviu Dudau <Liviu.Dudau@xxxxxxx>
> > Signed-off-by: Ryan Harkin <ryan.harkin@xxxxxxxxxx>
> > Signed-off-by: Mark Brown <broonie@xxxxxxxxxx>
>
> Did you folks tested this for all sorts of host controllers? I have no
> way to verify that it works, and last I heard, many (or even most)
> controllers don't work right with 64-bit DMA.
I have tested it with a host controller that is capable of 64-bit DMA and
without this change it doesn't work. At the moment it is the only known
USB host controller enabled for arm64. And 64-bit DMA works fine on arm64.
>
> > diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
> > index 81cda09b47e3..e704d403beae 100644
> > --- a/drivers/usb/host/ehci-hcd.c
> > +++ b/drivers/usb/host/ehci-hcd.c
> > @@ -590,11 +590,17 @@ static int ehci_run (struct usb_hcd *hcd)
> > */
> > hcc_params = ehci_readl(ehci, &ehci->caps->hcc_params);
> > if (HCC_64BIT_ADDR(hcc_params)) {
> > - ehci_writel(ehci, 0, &ehci->regs->segment);
> > -#if 0
> > -// this is deeply broken on almost all architectures
> > +#if CONFIG_ARM64
> > + ehci_writel(ehci, ehci->periodic_dma >> 32,
> > + &ehci->regs->segment);
> > + /*
> > + * this is deeply broken on almost all architectures
> > + * but arm64 can use it so enable it
> > + */
> > if (!dma_set_mask(hcd->self.controller, DMA_BIT_MASK(64)))
> > ehci_info(ehci, "enabled 64bit DMA\n");
> > +#else
> > + ehci_writel(ehci, 0, &ehci->regs->segment);
>
> It's silly to put this line in a separate #else section. The upper 32
> bits of ehci->periodic_dma are bound to be 0 anyway, because it was
> allocated before the DMA mask was changed.
Well, I don't want to enable 64-bit DMA for *all* the platforms, so there
needs to be an #else. While it is true that ehci->periodic_dma variable
has the top 32 bits zero, that cannot be guaranteed to be true for the
physical register holding that value, so I guess the write is not superfluous.
Best regards,
Liviu
>
> > #endif
> > }
>
> Alan Stern
>
>
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯
--
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
