[PATCH v2] usb: dwc3: support 64 bit DMA in platform driver

[PATCH v2] usb: dwc3: support 64 bit DMA in platform driver

[Sven Peter]

Currently, the dwc3 platform driver does not explicitly ask for
a DMA mask. This makes it fall back to the default 32-bit mask which
breaks the driver on systems that only have RAM starting above the
first 4G like the Apple M1 SoC.

Fix this by using the same logic already present in xhci-plat.c:
First, try to set a coherent dma mask for 64-bit, and then attempt
again with a 32-bit mask if this fails.

Signed-off-by: Sven Peter <sven@svenpeter.dev>
---
I have taken the code directly from the xhci-plat.c driver so
I think this change should be fairly low risk.
Unfortunately I only have the Apple M1 to test this on but here
the driver still works with the iommu enabled which limits the
address space to 32 bit. It also enables to use this with the iommu
in bypass mode which requires 64 bit addresses.

I believe this has been working fine so far since the dwc3 driver
only uses a few very small buffers in host mode which might still
fit within the first 4G of address space on many devices. The
majority of DMA buffers are allocated inside the xhci driver which
will already call dma_set_mask_and_coherent.

Best,

Sven

changes from v1:
 - removed WARN_ON around !dwc->sysdev->dma_mask; pointed out by greg k-h

 drivers/usb/dwc3/core.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index b6e53d8212cd..4930541a8984 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -1545,6 +1545,21 @@ static int dwc3_probe(struct platform_device *pdev)
 
 	dwc3_get_properties(dwc);
 
+	/* Try to set 64-bit DMA first */
+	if (!dwc->sysdev->dma_mask)
+		/* Platform did not initialize dma_mask */
+		ret = dma_coerce_mask_and_coherent(dwc->sysdev,
+						   DMA_BIT_MASK(64));
+	else
+		ret = dma_set_mask_and_coherent(dwc->sysdev, DMA_BIT_MASK(64));
+
+	/* If seting 64-bit DMA mask fails, fall back to 32-bit DMA mask */
+	if (ret) {
+		ret = dma_set_mask_and_coherent(dwc->sysdev, DMA_BIT_MASK(32));
+		if (ret)
+			return ret;
+	}
+
 	dwc->reset = devm_reset_control_array_get_optional_shared(dev);
 	if (IS_ERR(dwc->reset))
 		return PTR_ERR(dwc->reset);
-- 
2.25.1

Re: [PATCH v2] usb: dwc3: support 64 bit DMA in platform driver

[Arnd Bergmann]

On Sun, Jun 6, 2021 at 12:36 PM Sven Peter <sven@svenpeter.dev> wrote:
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index b6e53d8212cd..4930541a8984 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -1545,6 +1545,21 @@ static int dwc3_probe(struct platform_device *pdev)
>
>         dwc3_get_properties(dwc);
>
> +       /* Try to set 64-bit DMA first */
> +       if (!dwc->sysdev->dma_mask)
> +               /* Platform did not initialize dma_mask */
> +               ret = dma_coerce_mask_and_coherent(dwc->sysdev,
> +                                                  DMA_BIT_MASK(64));
> +       else
> +               ret = dma_set_mask_and_coherent(dwc->sysdev, DMA_BIT_MASK(64));
> +
> +       /* If seting 64-bit DMA mask fails, fall back to 32-bit DMA mask */
> +       if (ret) {
> +               ret = dma_set_mask_and_coherent(dwc->sysdev, DMA_BIT_MASK(32));
> +               if (ret)
> +                       return ret;
> +       }

Please drop the dma_coerce_mask_and_coherent() code path as well: if
the device is marked as non-DMA capable in the platform, it's better have
it not be usable at all than to assume a particular bus property that may
or may not be present on that bus.

The 32-bit mask is the default on all buses you might see a dwc3 controller
on, so you can drop that as well, and just leave the
dma_set_mask_and_coherent().

        Arnd

Re: [PATCH v2] usb: dwc3: support 64 bit DMA in platform driver

[Sven Peter]

On Sun, Jun 6, 2021, at 17:43, Arnd Bergmann wrote:
> On Sun, Jun 6, 2021 at 12:36 PM Sven Peter <sven@svenpeter.dev> wrote:
> > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> > index b6e53d8212cd..4930541a8984 100644
> > --- a/drivers/usb/dwc3/core.c
> > +++ b/drivers/usb/dwc3/core.c
> > @@ -1545,6 +1545,21 @@ static int dwc3_probe(struct platform_device *pdev)
> >
> >         dwc3_get_properties(dwc);
> >
> > +       /* Try to set 64-bit DMA first */
> > +       if (!dwc->sysdev->dma_mask)
> > +               /* Platform did not initialize dma_mask */
> > +               ret = dma_coerce_mask_and_coherent(dwc->sysdev,
> > +                                                  DMA_BIT_MASK(64));
> > +       else
> > +               ret = dma_set_mask_and_coherent(dwc->sysdev, DMA_BIT_MASK(64));
> > +
> > +       /* If seting 64-bit DMA mask fails, fall back to 32-bit DMA mask */
> > +       if (ret) {
> > +               ret = dma_set_mask_and_coherent(dwc->sysdev, DMA_BIT_MASK(32));
> > +               if (ret)
> > +                       return ret;
> > +       }
> 
> Please drop the dma_coerce_mask_and_coherent() code path as well: if
> the device is marked as non-DMA capable in the platform, it's better have
> it not be usable at all than to assume a particular bus property that may
> or may not be present on that bus.
> 
> The 32-bit mask is the default on all buses you might see a dwc3 controller
> on, so you can drop that as well, and just leave the
> dma_set_mask_and_coherent().
> 
>         Arnd
> 

Makes sense, will drop the two things for v3.

Thanks,

Sven