[PATCH 1/2] m68k: Set default dma mask for platform devices

[PATCH 1/2] m68k: Set default dma mask for platform devices

[Finn Thain]

This avoids a WARNING splat when loading the macsonic or macmace driver.
Please see commit 205e1b7f51e4 ("dma-mapping: warn when there is no
coherent_dma_mask").

This implementation of arch_setup_pdev_archdata() differs from the
powerpc one, in that this one avoids clobbering a device dma mask
which has already been initialized.

Cc: Christoph Hellwig <hch@lst.de>
Cc: Greg Ungerer <gerg@linux-m68k.org>
Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
---
 arch/m68k/kernel/dma.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/m68k/kernel/dma.c b/arch/m68k/kernel/dma.c
index c01b9b8f97bf..463572c4943f 100644
--- a/arch/m68k/kernel/dma.c
+++ b/arch/m68k/kernel/dma.c
@@ -9,6 +9,7 @@
 #include <linux/dma-mapping.h>
 #include <linux/device.h>
 #include <linux/kernel.h>
+#include <linux/platform_device.h>
 #include <linux/scatterlist.h>
 #include <linux/slab.h>
 #include <linux/vmalloc.h>
@@ -165,3 +166,12 @@ const struct dma_map_ops m68k_dma_ops = {
 	.sync_sg_for_device	= m68k_dma_sync_sg_for_device,
 };
 EXPORT_SYMBOL(m68k_dma_ops);
+
+void arch_setup_pdev_archdata(struct platform_device *pdev)
+{
+	if (pdev->dev.coherent_dma_mask == DMA_MASK_NONE &&
+	    pdev->dev.dma_mask == NULL) {
+		pdev->dev.coherent_dma_mask = DMA_BIT_MASK(32);
+		pdev->dev.dma_mask = &pdev->dev.coherent_dma_mask;
+	}
+}
-- 
2.16.1

Re: [PATCH 1/2] m68k: Set default dma mask for platform devices

[Christoph Hellwig]

> This implementation of arch_setup_pdev_archdata() differs from the
> powerpc one, in that this one avoids clobbering a device dma mask
> which has already been initialized.

I think your implementation should move into the generic implementation
in drivers/base/platform.c instead of being stuck in m68k.

Also powerpc probably wants fixing, but that's something left to the
ppc folks..

Re: [PATCH 1/2] m68k: Set default dma mask for platform devices

[Finn Thain]

On Fri, 18 May 2018, Christoph Hellwig wrote:

> > This implementation of arch_setup_pdev_archdata() differs from the 
> > powerpc one, in that this one avoids clobbering a device dma mask 
> > which has already been initialized.
> 
> I think your implementation should move into the generic implementation 
> in drivers/base/platform.c instead of being stuck in m68k.
> 
> Also powerpc probably wants fixing, but that's something left to the
> ppc folks..

On powerpc, all platform devices get a dma mask. But they don't do that in 
drivers/base/platform.c, they use arch_setup_pdev_archdata(). Why didn't 
they take the approach you suggest?

How would I support the claim that replacing an empty platform device dma 
mask with 0xffffffff is safe on all architectures and platforms?

Is there no code conditional upon dev.coherent_dma_mask or dev.dma_mask 
that could misbehave? (Didn't I cite an example in the other thread?*)

If you can convince me that it is safe, I'd be happy to submit the patch 
you asked for.

For now, I still think that patching the platform driver was the correct 
patch*.

Maybe the real problem is your commit 205e1b7f51e4 ("dma-mapping: warn 
when there is no coherent_dma_mask"), because it assumes that all dma_ops 
implementations care about coherent_dma_mask.

The dma_map_ops implementations that do use coherent_dma_mask should 
simply fail when it is unset, right?

Would it not be better to revert your patch and fix the dma_map_ops 
failure paths, than to somehow audit all the platform drivers and patch 
drivers/base/platform.c?

Thanks.

* https://lkml.kernel.org/r/alpine.LNX.2.21.1805091804290.72%40nippy.intranet

-- 

Re: [PATCH 1/2] m68k: Set default dma mask for platform devices

[Geert Uytterhoeven]

Hi Finn,

On Thu, May 17, 2018 at 12:07 PM, Finn Thain <fthain@telegraphics.com.au> wrote:
> This avoids a WARNING splat when loading the macsonic or macmace driver.
> Please see commit 205e1b7f51e4 ("dma-mapping: warn when there is no
> coherent_dma_mask").
>
> This implementation of arch_setup_pdev_archdata() differs from the
> powerpc one, in that this one avoids clobbering a device dma mask
> which has already been initialized.
>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Greg Ungerer <gerg@linux-m68k.org>
> Signed-off-by: Finn Thain <fthain@telegraphics.com.au>

Thanks, applied and queued for v4.18.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds