diff mbox series

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

Message ID 20210607061751.89752-1-sven@svenpeter.dev (mailing list archive)
State Accepted
Commit 45d39448b4d0260743f25d88fd929451ec8296f2
Headers show
Series [v3] usb: dwc3: support 64 bit DMA in platform driver | expand

Commit Message

Sven Peter June 7, 2021, 6:17 a.m. UTC
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 calling dma_set_mask_and_coherent with a 64bit mask.

Signed-off-by: Sven Peter <sven@svenpeter.dev>
---

Third time's a charm I hope - this time much simpler :)

I still 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 v2:
 - remove both dma_coerce_mask_and_coherent and the 32 bit
   dma_set_mask_and_coherent as pointed out by Arnd Bergmann
changes from v1:
 - removed WARN_ON around !dwc->sysdev->dma_mask; pointed out by greg k-h

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

Comments

Arnd Bergmann June 7, 2021, 7:25 a.m. UTC | #1
On Mon, Jun 7, 2021 at 8:18 AM Sven Peter <sven@svenpeter.dev> wrote:
>
> 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 calling dma_set_mask_and_coherent with a 64bit mask.
>
> Signed-off-by: Sven Peter <sven@svenpeter.dev>
> ---
>
> Third time's a charm I hope - this time much simpler :)

I think this is almost good, but there is still one small issue:

> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index b6e53d8212cd..ba4792b6a98f 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -1545,6 +1545,10 @@ static int dwc3_probe(struct platform_device *pdev)
>
>         dwc3_get_properties(dwc);
>
> +       ret = dma_set_mask_and_coherent(dwc->sysdev, DMA_BIT_MASK(64));
> +       if (ret)
> +               return ret;

This will now  fail on machines with dwc3 connected to a 32-bit bus (or a
bus that is accidentally not annotated as supporting 64-bit) when there is
some memory that is not addressable through that bus.

If dma_set_mask_and_coherent() fails, the platform should just fall back to
32-bit addressing as it did before your change. dma_alloc_*() will do that
implicitly by allocating from ZONE_DMA32, while dma_map_*() fails
on any non-addressable memory, or falls back to swiotlb if that is available.

        Arnd
Sven Peter June 7, 2021, 8:01 a.m. UTC | #2
On Mon, Jun 7, 2021, at 09:25, Arnd Bergmann wrote:
> On Mon, Jun 7, 2021 at 8:18 AM Sven Peter <sven@svenpeter.dev> wrote:
> >
> > 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 calling dma_set_mask_and_coherent with a 64bit mask.
> >
> > Signed-off-by: Sven Peter <sven@svenpeter.dev>
> > ---
> >
> > Third time's a charm I hope - this time much simpler :)
> 
> I think this is almost good, but there is still one small issue:
> 
> > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> > index b6e53d8212cd..ba4792b6a98f 100644
> > --- a/drivers/usb/dwc3/core.c
> > +++ b/drivers/usb/dwc3/core.c
> > @@ -1545,6 +1545,10 @@ static int dwc3_probe(struct platform_device *pdev)
> >
> >         dwc3_get_properties(dwc);
> >
> > +       ret = dma_set_mask_and_coherent(dwc->sysdev, DMA_BIT_MASK(64));
> > +       if (ret)
> > +               return ret;
> 
> This will now  fail on machines with dwc3 connected to a 32-bit bus (or a
> bus that is accidentally not annotated as supporting 64-bit) when there is
> some memory that is not addressable through that bus.
> 
> If dma_set_mask_and_coherent() fails, the platform should just fall back to
> 32-bit addressing as it did before your change. dma_alloc_*() will do that
> implicitly by allocating from ZONE_DMA32, while dma_map_*() fails
> on any non-addressable memory, or falls back to swiotlb if that is available.


Makes sense, but just to make sure I understand this correctly:
All that needs to be done is call dma_set_mask_and_coherent with a 64 bit
mask and then just ignore the return value?


Thanks,

Sven
Arnd Bergmann June 7, 2021, 8:22 a.m. UTC | #3
On Mon, Jun 7, 2021 at 10:01 AM Sven Peter <sven@svenpeter.dev> wrote:
> On Mon, Jun 7, 2021, at 09:25, Arnd Bergmann wrote:
> > On Mon, Jun 7, 2021 at 8:18 AM Sven Peter <sven@svenpeter.dev> wrote:
> > >
> > > 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 calling dma_set_mask_and_coherent with a 64bit mask.
> > >
> > > Signed-off-by: Sven Peter <sven@svenpeter.dev>
> > > ---
> > >
> > > Third time's a charm I hope - this time much simpler :)
> >
> > I think this is almost good, but there is still one small issue:
> >
> > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> > > index b6e53d8212cd..ba4792b6a98f 100644
> > > --- a/drivers/usb/dwc3/core.c
> > > +++ b/drivers/usb/dwc3/core.c
> > > @@ -1545,6 +1545,10 @@ static int dwc3_probe(struct platform_device *pdev)
> > >
> > >         dwc3_get_properties(dwc);
> > >
> > > +       ret = dma_set_mask_and_coherent(dwc->sysdev, DMA_BIT_MASK(64));
> > > +       if (ret)
> > > +               return ret;
> >
> > This will now  fail on machines with dwc3 connected to a 32-bit bus (or a
> > bus that is accidentally not annotated as supporting 64-bit) when there is
> > some memory that is not addressable through that bus.
> >
> > If dma_set_mask_and_coherent() fails, the platform should just fall back to
> > 32-bit addressing as it did before your change. dma_alloc_*() will do that
> > implicitly by allocating from ZONE_DMA32, while dma_map_*() fails
> > on any non-addressable memory, or falls back to swiotlb if that is available.
>
>
> Makes sense, but just to make sure I understand this correctly:
> All that needs to be done is call dma_set_mask_and_coherent with a 64 bit
> mask and then just ignore the return value?

If the driver never calls dma_map_*() on the device, that is correct, otherwise
it has to be careful about what pointers it passes in there to avoid
failing later.
Since it is already working without the dma_set_mask(), I don't expect a
problem there.

I suppose in theory, the dwc3_alloc_scratch_buffers() should use GFP_DMA32
if dma_set_mask_and_coherent() failed. On arm32, it won't matter since
all kernel pointers are generall within ZONE_DMA32, and on arm64 we always
build with SWIOTLB enabled. Not sure where else you'd typically find dwc3,
or if any of them are broken without changing this.

        Arnd
Sven Peter June 7, 2021, 9:06 a.m. UTC | #4
On Mon, Jun 7, 2021, at 10:22, Arnd Bergmann wrote:
> On Mon, Jun 7, 2021 at 10:01 AM Sven Peter <sven@svenpeter.dev> wrote:
> > On Mon, Jun 7, 2021, at 09:25, Arnd Bergmann wrote:
> > > On Mon, Jun 7, 2021 at 8:18 AM Sven Peter <sven@svenpeter.dev> wrote:
> > > >
> > > > 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 calling dma_set_mask_and_coherent with a 64bit mask.
> > > >
> > > > Signed-off-by: Sven Peter <sven@svenpeter.dev>
> > > > ---
> > > >
> > > > Third time's a charm I hope - this time much simpler :)
> > >
> > > I think this is almost good, but there is still one small issue:
> > >
> > > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> > > > index b6e53d8212cd..ba4792b6a98f 100644
> > > > --- a/drivers/usb/dwc3/core.c
> > > > +++ b/drivers/usb/dwc3/core.c
> > > > @@ -1545,6 +1545,10 @@ static int dwc3_probe(struct platform_device *pdev)
> > > >
> > > >         dwc3_get_properties(dwc);
> > > >
> > > > +       ret = dma_set_mask_and_coherent(dwc->sysdev, DMA_BIT_MASK(64));
> > > > +       if (ret)
> > > > +               return ret;
> > >
> > > This will now  fail on machines with dwc3 connected to a 32-bit bus (or a
> > > bus that is accidentally not annotated as supporting 64-bit) when there is
> > > some memory that is not addressable through that bus.
> > >
> > > If dma_set_mask_and_coherent() fails, the platform should just fall back to
> > > 32-bit addressing as it did before your change. dma_alloc_*() will do that
> > > implicitly by allocating from ZONE_DMA32, while dma_map_*() fails
> > > on any non-addressable memory, or falls back to swiotlb if that is available.
> >
> >
> > Makes sense, but just to make sure I understand this correctly:
> > All that needs to be done is call dma_set_mask_and_coherent with a 64 bit
> > mask and then just ignore the return value?
> 
> If the driver never calls dma_map_*() on the device, that is correct, otherwise
> it has to be careful about what pointers it passes in there to avoid
> failing later.
> Since it is already working without the dma_set_mask(), I don't expect a
> problem there.
> 
> I suppose in theory, the dwc3_alloc_scratch_buffers() should use GFP_DMA32
> if dma_set_mask_and_coherent() failed. On arm32, it won't matter since
> all kernel pointers are generall within ZONE_DMA32, and on arm64 we always
> build with SWIOTLB enabled. Not sure where else you'd typically find dwc3,
> or if any of them are broken without changing this.
> 
>         Arnd
> 

I've looked at Documentation/core-api/dma-api-howto.rst again which mentions that

	By default, the kernel assumes that your device can address 32-bits of DMA
	addressing.  For a 64-bit capable device, this needs to be increased, and for
	a device with limitations, it needs to be decreased.
	[...]
	These calls usually return zero to indicated your device can perform DMA
	properly on the machine given the address mask you provided, but they might
	return an error if the mask is too small to be supportable on the given
	system.  If it returns non-zero, your device cannot perform DMA properly on
	this platform, and attempting to do so will result in undefined behavior.
	You must not use DMA on this device unless the dma_set_mask family of
	functions has returned success.

which, unless I'm reading this incorrectly, should mean that asking for a 64bit
mask is always fine. In the worst case the mask will just be downgraded to
32bit if the bus is correctly annotated (the places I looked at that use the mask
take the min of that one and dev->bus_dma_limit).
Only asking for a mask that is too small would be bad.

I have also found [1],[2] which made changes to that documentation and that also
seems to confirm that it's fine to just ask for a 64 bit mask either way.


So for these cases

> > > This will now  fail on machines with dwc3 connected to a 32-bit bus (or a
> > > bus that is accidentally not annotated as supporting 64-bit) when there is
> > > some memory that is not addressable through that bus.

the call should return success but the final mask used for allocations should
remain at 32bit. Before the change no memory above the 32bit limit was used by
the dwc3 core and after the change we still can't use any memory above the
32bit limit.


Now if we had a dwc3 controller with
 * a quirk that only allows 32bit DMA for the core dwc3 controller
 * but support for >32bit DMA for xhci buffers (xhci already asks for a 64bit mask) 
 * on a bus that's otherwise annotated to support 64bit
this change will break that.

But that's unrelated to the dma_set_mask_and_coherent return value since
just calling it with a 64bit mask will already cause trouble (and also be successful!).

The problem I see is that we likely wouldn't know about devices with a quirk like this
since so far everything has been working fine there. I'm not really sure how to guard
against that either since we would only notice on the first DMA transfer above the 32bit
limit. I'm also not sure how likely the existence of such a weird device is.

This hypothetical dwc3 controller should probably either be confined to a bus with a 
proper 32bit limit or get a quirk that enforces allocations from ZONE_DMA32. Doesn't
change the fact that they used to work but would now break after this patch.



[1] https://lists.linuxfoundation.org/pipermail/iommu/2019-February/033669.html
[2] https://lists.linuxfoundation.org/pipermail/iommu/2019-February/033674.html

Best,

Sven
Arnd Bergmann June 7, 2021, 9:17 a.m. UTC | #5
On Mon, Jun 7, 2021 at 11:06 AM Sven Peter <sven@svenpeter.dev> wrote:
> On Mon, Jun 7, 2021, at 10:22, Arnd Bergmann wrote:
>
> I've looked at Documentation/core-api/dma-api-howto.rst again which mentions that
>
>         By default, the kernel assumes that your device can address 32-bits of DMA
>         addressing.  For a 64-bit capable device, this needs to be increased, and for
>         a device with limitations, it needs to be decreased.
>         [...]
>         These calls usually return zero to indicated your device can perform DMA
>         properly on the machine given the address mask you provided, but they might
>         return an error if the mask is too small to be supportable on the given
>         system.  If it returns non-zero, your device cannot perform DMA properly on
>         this platform, and attempting to do so will result in undefined behavior.
>         You must not use DMA on this device unless the dma_set_mask family of
>         functions has returned success.
>
> which, unless I'm reading this incorrectly, should mean that asking for a 64bit
> mask is always fine. In the worst case the mask will just be downgraded to
> 32bit if the bus is correctly annotated (the places I looked at that use the mask
> take the min of that one and dev->bus_dma_limit).
> Only asking for a mask that is too small would be bad.
>
> I have also found [1],[2] which made changes to that documentation and that also
> seems to confirm that it's fine to just ask for a 64 bit mask either way.

Indeed, I forgot about that change, this does make it easier.

> So for these cases
>
> > > > This will now  fail on machines with dwc3 connected to a 32-bit bus (or a
> > > > bus that is accidentally not annotated as supporting 64-bit) when there is
> > > > some memory that is not addressable through that bus.
>
> the call should return success but the final mask used for allocations should
> remain at 32bit. Before the change no memory above the 32bit limit was used by
> the dwc3 core and after the change we still can't use any memory above the
> 32bit limit.

Right.

> Now if we had a dwc3 controller with
>  * a quirk that only allows 32bit DMA for the core dwc3 controller
>  * but support for >32bit DMA for xhci buffers (xhci already asks for a 64bit mask)
>  * on a bus that's otherwise annotated to support 64bit
> this change will break that.
>
> But that's unrelated to the dma_set_mask_and_coherent return value since
> just calling it with a 64bit mask will already cause trouble (and also be successful!).
>
> The problem I see is that we likely wouldn't know about devices with a quirk like this
> since so far everything has been working fine there. I'm not really sure how to guard
> against that either since we would only notice on the first DMA transfer above the 32bit
> limit. I'm also not sure how likely the existence of such a weird device is.
>
> This hypothetical dwc3 controller should probably either be confined to a bus with a
> proper 32bit limit or get a quirk that enforces allocations from ZONE_DMA32. Doesn't
> change the fact that they used to work but would now break after this patch.

Makes sense, so for your v3 patch:

Reviewed-by: Arnd Bergmann <arnd@arndb.de>
Christoph Hellwig June 7, 2021, 10:04 a.m. UTC | #6
On Mon, Jun 07, 2021 at 09:25:43AM +0200, Arnd Bergmann wrote:
> This will now  fail on machines with dwc3 connected to a 32-bit bus (or a
> bus that is accidentally not annotated as supporting 64-bit) when there is
> some memory that is not addressable through that bus.

No, it won't.  dma_set_mask_and_coherent with a 64-bit mask will never
fail if dev->dma_mask is non-NULL.
Christoph Hellwig June 7, 2021, 10:05 a.m. UTC | #7
Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>
Felipe Balbi June 10, 2021, 9:24 a.m. UTC | #8
Hi,

Sven Peter <sven@svenpeter.dev> writes:
> 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 calling dma_set_mask_and_coherent with a 64bit mask.
>
> Signed-off-by: Sven Peter <sven@svenpeter.dev>

Acked-by: Felipe Balbi <balbi@kernel.org>

> ---
>
> Third time's a charm I hope - this time much simpler :)
>
> I still think this change should be fairly low risk.
>
> Unfortunately I only have the Apple M1 to test this on but here

wait a minute. The M1 includes a dwc3? That's awesome. Mind sharing a
regdump? Should be in debugfs.
Sven Peter June 10, 2021, 3:24 p.m. UTC | #9
On Thu, Jun 10, 2021, at 11:24, Felipe Balbi wrote:
> 
> Hi,
> 
> Sven Peter <sven@svenpeter.dev> writes:
> > 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 calling dma_set_mask_and_coherent with a 64bit mask.
> >
> > Signed-off-by: Sven Peter <sven@svenpeter.dev>
> 
> Acked-by: Felipe Balbi <balbi@kernel.org>

Cheers!

> 
> > ---
> >
> > Third time's a charm I hope - this time much simpler :)
> >
> > I still think this change should be fairly low risk.
> >
> > Unfortunately I only have the Apple M1 to test this on but here
> 
> wait a minute. The M1 includes a dwc3? That's awesome. Mind sharing a
> regdump? Should be in debugfs.



Two of them, actually :-)
One for each of the USB C ports together with an unknown PHY and some weird
quirks (neither the dwc3 gadget mode nor the xhci hardware seem to receive
more than a single connect event).
I've actually written a very basic dwc3 gadget driver for our bootloader
m1n1 [1,2] loosely based on your Linux driver from before it was switched to
GPLv2-only to experiment with the hardware and load linux kernels over usb :-)

I haven't found anything in debugfs, but I can share a simple dump of the MMIO
space. This is already after the controller has been initialized in gadget mode.
If you need something else just let me know.

>>> reghexdump32(0x502280000, 0xda00)
00000000  01100020 0200047f 1c0000f1 0200000a 0238ffcd 000004e0 00000440 0000003f
00000020  00000000 00000001 00000001 00000000 00000000 00000000 00000000 00000000
00000040  00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
00000060  *
00000420  00000280 00000000 00000000 00000000 00000280 00000000 00000000 00000000
00000440  00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
00000460  00000000 00000fa0 00000000 00000000 00000000 00000000 00000000 00000000
00000480  *
000004e0  00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
00000500  *
000008e0  00000401 00000000 00000000 00000000 02000402 20425355 00180101 00000000
00000900  03100002 20425355 20000102 00000000 00050134 000a4135 00000000 00000000
00000920  00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
00000940  *
0000c100  00000001 00006f00 00f00000 01500000 30c12004 00000000 7e800020 00001908
0000c120  33313130 00000000 12345678 0a416802 00000000 00000000 00000000 00000000
0000c140  4020800a 02092486 12345678 10420086 48422019 643d0410 18cc803f 0f000e92
0000c160  00230000 9cc20006 00000000 00000000 257cf70a 00000000 00000000 00800000
0000c180  00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
0000c1a0  3139302a 736f3035 33313130 00000000 00000000 00000000 00000000 00000000
0000c1c0  00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
0000c1e0  *
0000c200  00102400 00000000 00000000 00000000 00000000 00000000 00000000 00000000
0000c220  00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
0000c240  *
0000c2c0  01021102 00000000 00000000 00000000 00000000 00000000 00000000 00000000
0000c2e0  00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
0000c300  00000023 002300c7 00ea00c7 01b180c7 027800c7 033f00c7 040600c7 04cd80c7
0000c320  059400c7 065b00c7 072200c7 07e900c7 08b000c7 097700c7 0a3e00c7 0b0500c7
0000c340  00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
0000c360  *
0000c380  000003e8 03e88809 0bf10000 0bf18000 0bf10000 0bf18000 00000000 00000000
0000c3a0  00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
0000c3c0  *
0000c400  dead0000 00000000 00000100 00000000 00000000 00000000 00000000 00000000
0000c420  00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
0000c440  *
0000c600  00001744 00000000 18094246 0085560c 00000000 00000000 00000000 00000000
0000c620  00000000 0a0a0101 18181218 20201820 0a87f020 0001ffff 00000000 00000000
0000c640  00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
0000c660  *
0000c700  00080800 80f00000 00000007 008e0001 00000000 00000005 00000000 00000000
0000c720  00000003 00000000 00000000 00000000 00000000 00000000 00000000 00000000
0000c740  00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
0000c760  *
0000c800  00000000 00000000 00000001 00010002 00000000 00000000 00000001 00020002
0000c820  00000000 00000000 00000000 00000000 00000000 00000000 00000001 00030002
0000c840  00000000 00000000 00000001 00050002 00000000 00000000 00000001 00040002
0000c860  00000000 00000000 00000000 00000000 00000000 00000000 00000001 00060002
0000c880  00000000 00000000 00000001 00080002 00000000 00000000 00000001 00070002
0000c8a0  00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
0000c8c0  *
0000cd20  00000000 00000000 00000886 00000886 00000000 00000000 00000000 00000000
0000cd40  3c20060c 0002000b 00000000 00000000 00000000 00000000 00000000 00000000
0000cd60  00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
0000cd80  *
0000d000  30302626 00940076 26261f1f 00000052 9c067d05 3fff3b15 42843603 0023287d
0000d020  4e928162 008b8080 00000c00 0047e1f4 00753546 5dc2f07d 00000ea6 f0966b32
0000d040  00026cf5 1130c850 00000000 00000000 01090650 00000000 00000000 00000000
0000d060  000034c2 00000582 00003540 00000600 00000000 00000000 00051565 00000000
0000d080  00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
0000d0a0  *


fyi, the Apple Device Tree defines the following register ranges, some of
those are related the PHY and at least two are for the dwc3 itself. No idea what
the rest is used for yet.

<0x00000005 0x02280000 0x00000000 0x0000da00> <-- DWC3 + something else at the end
<0x00000005 0x02200000 0x00000000 0x00080000>
<0x00000005 0x0228c000 0x00000000 0x00001800> <-- DWC3, just before GLOBALS_REGS
<0x00000005 0x02a84000 0x00000000 0x00004000> <-- possibly related to the PHY
<0x00000005 0x02800000 0x00000000 0x00004000>
<0x00000005 0x02a80000 0x00000000 0x00004000>
<0x00000005 0x02000000 0x00000000 0x00080000>
<0x00000005 0x02080000 0x00000000 0x00080000>
<0x00000005 0x0228d000 0x00000000 0x00000800> <-- just after DWC3


Best,


Sven


[1] https://github.com/AsahiLinux/m1n1
[2] https://github.com/AsahiLinux/m1n1/blob/main/src/usb_dwc3.c
Felipe Balbi June 11, 2021, 1:17 p.m. UTC | #10
Hi,

"Sven Peter" <sven@svenpeter.dev> writes:
> On Thu, Jun 10, 2021, at 11:24, Felipe Balbi wrote:
>> Sven Peter <sven@svenpeter.dev> writes:
>> > 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 calling dma_set_mask_and_coherent with a 64bit mask.
>> >
>> > Signed-off-by: Sven Peter <sven@svenpeter.dev>
>> 
>> Acked-by: Felipe Balbi <balbi@kernel.org>
>
> Cheers!
>
>> 
>> > ---
>> >
>> > Third time's a charm I hope - this time much simpler :)
>> >
>> > I still think this change should be fairly low risk.
>> >
>> > Unfortunately I only have the Apple M1 to test this on but here
>> 
>> wait a minute. The M1 includes a dwc3? That's awesome. Mind sharing a
>> regdump? Should be in debugfs.
>
> Two of them, actually :-)

Sweet! Now we have a mobile platform that can test dwc3 role switch in
isolation :-p Just plug one dwc3 to the other lol

> One for each of the USB C ports together with an unknown PHY and some weird

I'll bet the PHY is a generic Synopsys PHY. There's no SW control for
it.

> quirks (neither the dwc3 gadget mode nor the xhci hardware seem to receive
> more than a single connect event).


> I've actually written a very basic dwc3 gadget driver for our bootloader
> m1n1 [1,2] loosely based on your Linux driver from before it was switched to
> GPLv2-only to experiment with the hardware and load linux kernels over usb :-)

noice!

> I haven't found anything in debugfs, but I can share a simple dump of the MMIO
> space. This is already after the controller has been initialized in gadget mode.
> If you need something else just let me know.

there should be a directory with the device's name. Under which you
should find a file called regdump :-)

>>>> reghexdump32(0x502280000, 0xda00)
> 00000000  01100020 0200047f 1c0000f1 0200000a 0238ffcd 000004e0 00000440 0000003f
> 00000020  00000000 00000001 00000001 00000000 00000000 00000000 00000000 00000000
> 00000040  00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> 00000060  *
> 00000420  00000280 00000000 00000000 00000000 00000280 00000000 00000000 00000000
> 00000440  00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> 00000460  00000000 00000fa0 00000000 00000000 00000000 00000000 00000000 00000000
> 00000480  *
> 000004e0  00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> 00000500  *
> 000008e0  00000401 00000000 00000000 00000000 02000402 20425355 00180101 00000000
> 00000900  03100002 20425355 20000102 00000000 00050134 000a4135 00000000 00000000
> 00000920  00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> 00000940  *
> 0000c100  00000001 00006f00 00f00000 01500000 30c12004 00000000 7e800020 00001908
> 0000c120  33313130 00000000 12345678 0a416802 00000000 00000000 00000000 00000000

This is the revision (0xc120). This confirms the IP as dwc3.1.

> 0000c140  4020800a 02092486 12345678 10420086 48422019 643d0410 18cc803f 0f000e92
> 0000c160  00230000 9cc20006 00000000 00000000 257cf70a 00000000 00000000 00800000
> 0000c180  00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> 0000c1a0  3139302a 736f3035 33313130 00000000 00000000 00000000 00000000 00000000

And this (0xc1a0) is the version number 1.90A

> 0000c1c0  00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> 0000c1e0  *
> 0000c200  00102400 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> 0000c220  00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> 0000c240  *
> 0000c2c0  01021102 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> 0000c2e0  00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> 0000c300  00000023 002300c7 00ea00c7 01b180c7 027800c7 033f00c7 040600c7 04cd80c7
> 0000c320  059400c7 065b00c7 072200c7 07e900c7 08b000c7 097700c7 0a3e00c7 0b0500c7
> 0000c340  00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> 0000c360  *
> 0000c380  000003e8 03e88809 0bf10000 0bf18000 0bf10000 0bf18000 00000000 00000000
> 0000c3a0  00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> 0000c3c0  *
> 0000c400  dead0000 00000000 00000100 00000000 00000000 00000000 00000000 00000000
> 0000c420  00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> 0000c440  *
> 0000c600  00001744 00000000 18094246 0085560c 00000000 00000000 00000000 00000000
> 0000c620  00000000 0a0a0101 18181218 20201820 0a87f020 0001ffff 00000000 00000000
> 0000c640  00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> 0000c660  *
> 0000c700  00080800 80f00000 00000007 008e0001 00000000 00000005 00000000 00000000
> 0000c720  00000003 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> 0000c740  00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> 0000c760  *
> 0000c800  00000000 00000000 00000001 00010002 00000000 00000000 00000001 00020002
> 0000c820  00000000 00000000 00000000 00000000 00000000 00000000 00000001 00030002
> 0000c840  00000000 00000000 00000001 00050002 00000000 00000000 00000001 00040002
> 0000c860  00000000 00000000 00000000 00000000 00000000 00000000 00000001 00060002
> 0000c880  00000000 00000000 00000001 00080002 00000000 00000000 00000001 00070002
> 0000c8a0  00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> 0000c8c0  *
> 0000cd20  00000000 00000000 00000886 00000886 00000000 00000000 00000000 00000000
> 0000cd40  3c20060c 0002000b 00000000 00000000 00000000 00000000 00000000 00000000
> 0000cd60  00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> 0000cd80  *
> 0000d000  30302626 00940076 26261f1f 00000052 9c067d05 3fff3b15 42843603 0023287d
> 0000d020  4e928162 008b8080 00000c00 0047e1f4 00753546 5dc2f07d 00000ea6 f0966b32
> 0000d040  00026cf5 1130c850 00000000 00000000 01090650 00000000 00000000 00000000
> 0000d060  000034c2 00000582 00003540 00000600 00000000 00000000 00051565 00000000
> 0000d080  00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> 0000d0a0  *
>
>
> fyi, the Apple Device Tree defines the following register ranges, some of
> those are related the PHY and at least two are for the dwc3 itself. No idea what
> the rest is used for yet.

could be some debugging thing. DWC3 has this EBC feature which allows an
external piece of HW to directly control the FIFOs. Usually used for
some processor tracing capability.

> <0x00000005 0x02280000 0x00000000 0x0000da00> <-- DWC3 + something else at the end
> <0x00000005 0x02200000 0x00000000 0x00080000>
> <0x00000005 0x0228c000 0x00000000 0x00001800> <-- DWC3, just before GLOBALS_REGS
> <0x00000005 0x02a84000 0x00000000 0x00004000> <-- possibly related to the PHY
> <0x00000005 0x02800000 0x00000000 0x00004000>
> <0x00000005 0x02a80000 0x00000000 0x00004000>
> <0x00000005 0x02000000 0x00000000 0x00080000>
> <0x00000005 0x02080000 0x00000000 0x00080000>
> <0x00000005 0x0228d000 0x00000000 0x00000800> <-- just after DWC3

nice.

> [1] https://github.com/AsahiLinux/m1n1
> [2] https://github.com/AsahiLinux/m1n1/blob/main/src/usb_dwc3.c

pretty cool, this may just become the best ARM linux laptop :-p


PS: _do_ ping me if you have dwc3 issues, this is super interesting heh
diff mbox series

Patch

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index b6e53d8212cd..ba4792b6a98f 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -1545,6 +1545,10 @@  static int dwc3_probe(struct platform_device *pdev)
 
 	dwc3_get_properties(dwc);
 
+	ret = dma_set_mask_and_coherent(dwc->sysdev, DMA_BIT_MASK(64));
+	if (ret)
+		return ret;
+
 	dwc->reset = devm_reset_control_array_get_optional_shared(dev);
 	if (IS_ERR(dwc->reset))
 		return PTR_ERR(dwc->reset);