On Thu, May 15, 2014 at 04:36:25PM +0100, Alan Stern wrote:
> On Thu, 15 May 2014, Liviu Dudau wrote:
>
> > 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.
>
> What do you mean it doesn't work? Can't the host controller use 32-bit
> DMA?
It could if arm64 would restrict the DMA addresses to 32-bit, but it doesn't
and I end up on my platform with USB DMA buffers allocated >4GB address.
>
> > At the moment it is the only known
> > USB host controller enabled for arm64. And 64-bit DMA works fine on arm64.
>
> What do you mean? What happens if you plug in a PCI card containing,
> say, a Sony EHCI host controller on an arm64 system?
I don't have one, so I don't know for sure. I will try to find a PCI card that
can do 32-bit and 64-bit DMA.
>
> > > > 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.
>
> No, there doesn't. Just leave the
>
> ehci_writel(ehci, 0, &ehci->regs->segment);
>
> line above the "#if CONFIG_ARM64", the way it is in the original code,
> and get rid of
>
> ehci_writel(ehci, ehci->periodic_dma >> 32,
> &ehci->regs->segment);
Actually I need this line because my period_dma addresses have top 32-bit
values non-zero.
>
> > 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.
>
> That's why you can write 0 to the register instead of writing
> ehci->periodic_dma >> 32.
>
> Don't forget, the controller uses that same ehci->regs->segment
> register for ehci->qtd_pool, ehci->qh_pool, ehci->itd_pool, and
> ehci->sitd_pool as well as ehci->periodic. If those DMA pools were
> allocated in different regions of memory (that is, regions whose upper
> 32 bits were different), the controller wouldn't be able to access
> them. The only way to insure they will all be allocated in the same
> 4-GB region is if they are allocated in the first 4 GB.
My platform creates all the USB buffers outside the 4GB zone. Need to go back
to the code to understand if that is due to design or misconfiguration.
Best regards,
Liviu
>
> 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
