Re: [RFC PATCH] m68k: set dma and coherent masks for Macintosh SONIC based ethernet
Hi Geert,
On 31/05/18 05:55, Geert Uytterhoeven wrote:
On Wed, May 30, 2018 at 2:28 AM, Greg Ungerer wrote:
On 28/05/18 20:15, Geert Uytterhoeven wrote:
On Mon, May 28, 2018 at 7:26 AM, Finn Thain
wrote:
On Mon, 28 May 2018, Michael Schmitz wrote:
Am 27.05.2018 um 17:49 schrieb Finn Thain:
On Sun, 27 May 2018, Michael Schmitz wrote:
That should have fixed the warning already ...
It's still not fixed (hence my "acked-by" for Geunter's patch).
Odd - does link order still matter even though the
arch_setup_dev_archdata() function from the core platform code is
declared as a weak symbol?
I'll see what I can find out on elgar ...
Any one of the numerous patches/rfcs/suggestions that I sent will avoid
the WARN splat.
When I said "it's still not fixed", what I meant to say was, "it's still
not fixed in mainline and no proposed fix was accepted to the best of my
knowledge".
Indeed.
Do we have a consensus on the way forward? The merge window for
v4.18 will open soon.
For whatever it is worth I thought Finn's patch was the best approach
(https://lkml.org/lkml/2018/5/17/333, "m68k: Set default dma mask for
platform device").
FTR: done.
Feel free to add my acked by if you like:
Acked-by: Greg Ungerer
Regards
Greg
We seem to be hitting quite a few places (within m68k) that otherwise
need individual fixes. There is no immediate need to revert existing
changes that have already been applied if we use this now either
(like my FEC fix, commit f61e64310b75 "m68k: set dma and coherent
masks for platform FEC ethernets").
Indeed.
Gr{oetje,eeting}s,
Geert
Re: [RFC PATCH] m68k: set dma and coherent masks for Macintosh SONIC based ethernet
Hi Greg,
On Wed, May 30, 2018 at 2:28 AM, Greg Ungerer wrote:
> On 28/05/18 20:15, Geert Uytterhoeven wrote:
>> On Mon, May 28, 2018 at 7:26 AM, Finn Thain
>> wrote:
>>> On Mon, 28 May 2018, Michael Schmitz wrote:
Am 27.05.2018 um 17:49 schrieb Finn Thain:
> On Sun, 27 May 2018, Michael Schmitz wrote:
>> That should have fixed the warning already ...
>
> It's still not fixed (hence my "acked-by" for Geunter's patch).
Odd - does link order still matter even though the
arch_setup_dev_archdata() function from the core platform code is
declared as a weak symbol?
I'll see what I can find out on elgar ...
>>>
>>> Any one of the numerous patches/rfcs/suggestions that I sent will avoid
>>> the WARN splat.
>>>
>>> When I said "it's still not fixed", what I meant to say was, "it's still
>>> not fixed in mainline and no proposed fix was accepted to the best of my
>>> knowledge".
>>
>> Indeed.
>>
>> Do we have a consensus on the way forward? The merge window for
>> v4.18 will open soon.
>
> For whatever it is worth I thought Finn's patch was the best approach
> (https://lkml.org/lkml/2018/5/17/333, "m68k: Set default dma mask for
> platform device").
FTR: done.
> We seem to be hitting quite a few places (within m68k) that otherwise
> need individual fixes. There is no immediate need to revert existing
> changes that have already been applied if we use this now either
> (like my FEC fix, commit f61e64310b75 "m68k: set dma and coherent
> masks for platform FEC ethernets").
Indeed.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@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
Re: [RFC PATCH] m68k: set dma and coherent masks for Macintosh SONIC based ethernet
Hi Geert, On 28/05/18 20:15, Geert Uytterhoeven wrote: On Mon, May 28, 2018 at 7:26 AM, Finn Thain wrote: On Mon, 28 May 2018, Michael Schmitz wrote: Am 27.05.2018 um 17:49 schrieb Finn Thain: On Sun, 27 May 2018, Michael Schmitz wrote: That should have fixed the warning already ... It's still not fixed (hence my "acked-by" for Geunter's patch). Odd - does link order still matter even though the arch_setup_dev_archdata() function from the core platform code is declared as a weak symbol? I'll see what I can find out on elgar ... Any one of the numerous patches/rfcs/suggestions that I sent will avoid the WARN splat. When I said "it's still not fixed", what I meant to say was, "it's still not fixed in mainline and no proposed fix was accepted to the best of my knowledge". Indeed. Do we have a consensus on the way forward? The merge window for v4.18 will open soon. For whatever it is worth I thought Finn's patch was the best approach (https://lkml.org/lkml/2018/5/17/333, "m68k: Set default dma mask for platform device"). We seem to be hitting quite a few places (within m68k) that otherwise need individual fixes. There is no immediate need to revert existing changes that have already been applied if we use this now either (like my FEC fix, commit f61e64310b75 "m68k: set dma and coherent masks for platform FEC ethernets"). Regards Greg
Re: [RFC PATCH] m68k: set dma and coherent masks for Macintosh SONIC based ethernet
Hi Finn, On Tue, May 29, 2018 at 5:38 PM, Finn Thain wrote: > I found some patches here, > http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/generic-dma-noncoherent.2 That's the most recent IIRC. Haven't begun looking at that yet - still stuck at git://git.infradead.org/users/hch/misc.git and waiting for a SCSI disk to be installed on elgar. Christoph - any merit in testing that old repo? Cheers, Michael
Re: [RFC PATCH] m68k: set dma and coherent masks for Macintosh SONIC based ethernet
On Mon, 28 May 2018, Geert Uytterhoeven wrote: > > Do we have a consensus on the way forward? The merge window for v4.18 > will open soon. I'm content to let you and Christoph decide how best to deal with this. Don't wait for me to find consensus ;-) --
Re: [RFC PATCH] m68k: set dma and coherent masks for Macintosh SONIC based ethernet
On Tue, 29 May 2018, Christoph Hellwig wrote: > Btw, can I get a review and testing for the above? They aren't really > experimental any more, and I'd like to move architectures over as soon > as possible. When I boot the generic-dma-noncoherent.2 branch (5f4613b2dcd4) in qemu and run "ifconfig eth0 up" I get an oops (see below). The crash goes away when I add a patch to initialize the macsonic device dma masks, but it appears the new code is not as robust as the old code. Unable to handle kernel NULL pointer dereference at virtual address (null) Oops: Modules linked in: PC: [<003352e6>] dma_direct_map_page+0xc0/0xf8 SR: 2004 SP: f93954f5 a2: 1e988f30 d0: 1e9c7950d1: 1e9c7000d2: fff0b1c8d3: f4e569c7 d4: 05f0d5: 0002a0: 1e85a20aa1: Process ifconfig (pid: 46, task=87dc4b31) Frame format=7 eff addr= ssw=0505 faddr= wb 1 stat/addr/data: wb 2 stat/addr/data: wb 3 stat/addr/data: push data: Stack from 1e9b9c74: 05f0 0002 0001e9c7 003d 1e85a20a 00951ffc 1e9c7950 1e9b9cd8 00335636 1e85a20a 00951ffc 0950 05f0 0002 0017 003d38e0 0027b11e 0001e9c7 1e9783e0 1e9784d0 00381f10 1e9b9d2c 0025c11a 1e85a20a 00951ffc 0950 05f0 0002 1043 1002 0041 8000b543 1e95f00c 1e978000 0037c3a0 1e97802f 0950 1e85a20a 0007a71c 1e9b9d44 1e9b9d44 0028d4b0 Call Trace: [<0001e9c7>] resource_list_create_entry+0x11/0x46 [<00335636>] dma_noncoherent_map_page+0x32/0xfc [<0027b11e>] skb_put+0x0/0x6a [<0001e9c7>] resource_list_create_entry+0x11/0x46 [<0025c11a>] macsonic_open+0x124/0x360 [<1043>] kernel_pg_dir+0x43/0x1000 [<1002>] kernel_pg_dir+0x2/0x1000 [<0007a71c>] wb_update_bandwidth+0x40/0x4c [<0028d4b0>] __dev_open+0x8e/0x108 [<0028d026>] dev_set_rx_mode+0x0/0x3e [<0028d676>] __dev_change_flags+0x14c/0x19e [<1002>] kernel_pg_dir+0x2/0x1000 [<8914>] via_nubus_irq+0x9e/0xd0 [<00021378>] ns_capable_common+0x30/0x96 [<0028d6e8>] dev_change_flags+0x20/0x56 [<1043>] kernel_pg_dir+0x43/0x1000 [<1043>] kernel_pg_dir+0x43/0x1000 [<002fad14>] devinet_ioctl+0x610/0x754 [<1043>] kernel_pg_dir+0x43/0x1000 [<8914>] via_nubus_irq+0x9e/0xd0 [<002fc3c6>] inet_ioctl+0x1a2/0x250 [<8914>] via_nubus_irq+0x9e/0xd0 [<8914>] via_nubus_irq+0x9e/0xd0 [<00291c50>] dev_get_by_name_rcu+0x68/0xa0 [<00291c50>] dev_get_by_name_rcu+0x68/0xa0 [<8913>] via_nubus_irq+0x9d/0xd0 [<002b17ce>] dev_ioctl+0x334/0x4ba [<002b1826>] dev_ioctl+0x38c/0x4ba [<00273b44>] sock_ioctl+0x132/0x3f4 [<8914>] via_nubus_irq+0x9e/0xd0 [<1002>] kernel_pg_dir+0x2/0x1000 [<00028000>] getrusage+0x25a/0x3ee [<000c1594>] vfs_ioctl+0x20/0x36 [<8914>] via_nubus_irq+0x9e/0xd0 [<000c197e>] do_vfs_ioctl+0x70/0x6ae [<8914>] via_nubus_irq+0x9e/0xd0 [<000bf7e0>] user_path_at_empty+0x2c/0x34 [<0003471e>] __put_cred+0x2a/0x78 [<00034b2a>] put_cred_rcu+0x0/0x92 [<8914>] via_nubus_irq+0x9e/0xd0 [<000c1fec>] ksys_ioctl+0x30/0x6c [<8914>] via_nubus_irq+0x9e/0xd0 [<8914>] via_nubus_irq+0x9e/0xd0 [<000c203e>] sys_ioctl+0x16/0x1a [<8914>] via_nubus_irq+0x9e/0xd0 [<2b7c>] syscall+0x8/0xc [<8914>] via_nubus_irq+0x9e/0xd0 [] cu_norm+0x9/0x28 Code: 9781 6514 4280 4cee 18fc ffdc 4e5e 4e75 <2211> 2429 0004 60e2 2f02 2f01 2f2e 0014 486e fffc 4879 0038 1efc 4879 003e a45d Disabling lock debugging due to kernel taint --
Re: [RFC PATCH] m68k: set dma and coherent masks for Macintosh SONIC based ethernet
On Tue, May 29, 2018 at 03:38:23PM +1000, Finn Thain wrote: > On Tue, 29 May 2018, Michael Schmitz wrote: > > > > > > > Since an arch gets to apply limits in the dma ops it implements, why > > > would arch code also have to set a limit in the form of default > > > platform device masks? Powerpc seems to be the only arch that does > > > this. > > > > One of Christoph's recent patches removed most of arches' dma ops, > > replacing them by one generic implementation instead. m68k was one of > > the affected arches. I concede his patch series is experimental still > > and not in mainline, but may be included at some time. > > I found some patches here, > http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/generic-dma-noncoherent.2 > > Looks like m68k_dma_alloc() gets renamed arch_dma_alloc() and the generic > ops don't use the dma masks. This is for the 'coherent' allocations, and literatelly nothing changes for those yet. But for the mapping path this switches m68k to use the dma-direct path, wrapped by dma-noncoherent. And these do check dev->dma_mask, although only really for sanity checking. Btw, can I get a review and testing for the above? They aren't really experimental any more, and I'd like to move architectures over as soon as possible.
Re: [RFC PATCH] m68k: set dma and coherent masks for Macintosh SONIC based ethernet
On Tue, May 29, 2018 at 07:59:37AM +1200, Michael Schmitz wrote: > Hi Geert, > > my preference would be Finn's patch introducing a m68k > arch_setup_pdev_archdata(). It nicely preserves what bus code sets up > prior to registering a platform device (important for Zorro devices > using platform or mfd devices), and allows overriding by drivers that > need it. Let's go with those for now. > > If ever a kernel-wide consensus is reached to move this setup to the > generic arch_setup_pdev_archdata(), we can still back out his patch at > that time. Finn found one counterexample where absence of DMA mask > signaled 'do not use DMA', so I think moving the DMA mask setup to > generic code is unlikely to happen any time. This is the right thing to do in the long run, and I'll spend some time auditing all dma_mask users for the next merge window.
Re: [RFC PATCH] m68k: set dma and coherent masks for Macintosh SONIC based ethernet
On Tue, 29 May 2018, Michael Schmitz wrote: > > > > Since an arch gets to apply limits in the dma ops it implements, why > > would arch code also have to set a limit in the form of default > > platform device masks? Powerpc seems to be the only arch that does > > this. > > One of Christoph's recent patches removed most of arches' dma ops, > replacing them by one generic implementation instead. m68k was one of > the affected arches. I concede his patch series is experimental still > and not in mainline, but may be included at some time. I found some patches here, http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/generic-dma-noncoherent.2 Looks like m68k_dma_alloc() gets renamed arch_dma_alloc() and the generic ops don't use the dma masks. Maybe I'm looking at the wrong patches? --
Re: [RFC PATCH] m68k: set dma and coherent masks for Macintosh SONIC based ethernet
Hi Finn, Am 29.05.2018 um 14:15 schrieb Finn Thain: > > Since an arch gets to apply limits in the dma ops it implements, why would > arch code also have to set a limit in the form of default platform device > masks? Powerpc seems to be the only arch that does this. One of Christoph's recent patches removed most of arches' dma ops, replacing them by one generic implementation instead. m68k was one of the affected arches. I concede his patch series is experimental still and not in mainline, but may be included at some time. Otherwise - your patches, so if you prefer to have each driver handle this on an as-needed basis, I'm not objecting. Cheers, Michael
Re: [RFC PATCH] m68k: set dma and coherent masks for Macintosh SONIC based ethernet
On Mon, 28 May 2018, Geert Uytterhoeven wrote:
>
> Do we have a consensus on the way forward?
My prefered solution remains the two driver patches that I originally
submitted, which you objected to:
https://lkml.org/lkml/2018/5/3/10
https://lkml.org/lkml/2018/5/3/9
So there is no consensus yet.
To my mind, the existing code is pretty clear: drivers should set the
(default) mask. See also,
$ egrep -r -A1 "coherent_dma_mask.*expected" */
drivers/of/device.c: * Set default coherent_dma_mask to 32 bit. Drivers
are expected to
drivers/of/device.c- * setup the correct supported mask.
--
drivers/acpi/arm64/iort.c: * Set default coherent_dma_mask to 32 bit.
Drivers are expected to
drivers/acpi/arm64/iort.c- * setup the correct supported mask.
$
And drivers/usb/dwc2/platform.c:
/*
* Use reasonable defaults so platforms don't have to provide these.
*/
if (!dev->dev.dma_mask)
dev->dev.dma_mask = &dev->dev.coherent_dma_mask;
retval = dma_set_coherent_mask(&dev->dev, DMA_BIT_MASK(32));
And FWIW,
$ egrep -wlr
"dma_set_mask_and_coherent|dma_set_coherent_mask|dma_coerce_mask_and_coherent"
drivers/ | wc -l
196
I suppose that a driver should avoid lengthening an existing device mask.
Since an arch gets to apply limits in the dma ops it implements, why would
arch code also have to set a limit in the form of default platform device
masks? Powerpc seems to be the only arch that does this.
The same line of reasoning suggests that the problematic WARN_ON should
not appear in include/linux/ in the first place. If it is needed by
certain architectures, it should be in arch/x.
I would send a patch to revert commit 205e1b7f51e4 ("dma-mapping: warn
when there is no coherent_dma_mask") if I thought that arch code was not
somehow relying on it. But I'll leave that up to Chrisoph.
--
Re: [RFC PATCH] m68k: set dma and coherent masks for Macintosh SONIC based ethernet
Hi Geert,
my preference would be Finn's patch introducing a m68k
arch_setup_pdev_archdata(). It nicely preserves what bus code sets up
prior to registering a platform device (important for Zorro devices
using platform or mfd devices), and allows overriding by drivers that
need it.
If ever a kernel-wide consensus is reached to move this setup to the
generic arch_setup_pdev_archdata(), we can still back out his patch at
that time. Finn found one counterexample where absence of DMA mask
signaled 'do not use DMA', so I think moving the DMA mask setup to
generic code is unlikely to happen any time.
Just my $0.02 ...
Cheers,
MIchael
On Mon, May 28, 2018 at 10:15 PM, Geert Uytterhoeven
wrote:
> On Mon, May 28, 2018 at 7:26 AM, Finn Thain
> wrote:
>> On Mon, 28 May 2018, Michael Schmitz wrote:
>>> Am 27.05.2018 um 17:49 schrieb Finn Thain:
>>> > On Sun, 27 May 2018, Michael Schmitz wrote:
>>> >
>>> >> That should have fixed the warning already ...
>>> >
>>> > It's still not fixed (hence my "acked-by" for Geunter's patch).
>>> >
>>>
>>> Odd - does link order still matter even though the
>>> arch_setup_dev_archdata() function from the core platform code is
>>> declared as a weak symbol?
>>>
>>> I'll see what I can find out on elgar ...
>>>
>>
>> Any one of the numerous patches/rfcs/suggestions that I sent will avoid
>> the WARN splat.
>>
>> When I said "it's still not fixed", what I meant to say was, "it's still
>> not fixed in mainline and no proposed fix was accepted to the best of my
>> knowledge".
>
> Indeed.
>
> Do we have a consensus on the way forward? The merge window for
> v4.18 will open soon.
>
> Thanks!
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 --
> ge...@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
Re: [RFC PATCH] m68k: set dma and coherent masks for Macintosh SONIC based ethernet
On Mon, May 28, 2018 at 7:26 AM, Finn Thain wrote:
> On Mon, 28 May 2018, Michael Schmitz wrote:
>> Am 27.05.2018 um 17:49 schrieb Finn Thain:
>> > On Sun, 27 May 2018, Michael Schmitz wrote:
>> >
>> >> That should have fixed the warning already ...
>> >
>> > It's still not fixed (hence my "acked-by" for Geunter's patch).
>> >
>>
>> Odd - does link order still matter even though the
>> arch_setup_dev_archdata() function from the core platform code is
>> declared as a weak symbol?
>>
>> I'll see what I can find out on elgar ...
>>
>
> Any one of the numerous patches/rfcs/suggestions that I sent will avoid
> the WARN splat.
>
> When I said "it's still not fixed", what I meant to say was, "it's still
> not fixed in mainline and no proposed fix was accepted to the best of my
> knowledge".
Indeed.
Do we have a consensus on the way forward? The merge window for
v4.18 will open soon.
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@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
Re: [RFC PATCH] m68k: set dma and coherent masks for Macintosh SONIC based ethernet
On Sat, May 26, 2018 at 09:15:05PM -0700, Guenter Roeck wrote: > Good point. Maybe it would be better to limit the warning to SMP systems > instead of (unnecessarily) fixing drivers all over the place ? No. The coherent_dma_mask is the mask used for dma_alloc_coherent and dma_alloc_attrs. It has nothing to do with cache coherent in SMP systems.
Re: [RFC PATCH] m68k: set dma and coherent masks for Macintosh SONIC based ethernet
On Mon, 28 May 2018, Michael Schmitz wrote: > Hi Finn, > > Am 27.05.2018 um 17:49 schrieb Finn Thain: > > On Sun, 27 May 2018, Michael Schmitz wrote: > > > >> That should have fixed the warning already ... > > > > It's still not fixed (hence my "acked-by" for Geunter's patch). > > > > Odd - does link order still matter even though the > arch_setup_dev_archdata() function from the core platform code is > declared as a weak symbol? > > I'll see what I can find out on elgar ... > Any one of the numerous patches/rfcs/suggestions that I sent will avoid the WARN splat. When I said "it's still not fixed", what I meant to say was, "it's still not fixed in mainline and no proposed fix was accepted to the best of my knowledge". -- > Cheers, > > Michael >
Re: [RFC PATCH] m68k: set dma and coherent masks for Macintosh SONIC based ethernet
Hi Finn, Am 27.05.2018 um 17:49 schrieb Finn Thain: > On Sun, 27 May 2018, Michael Schmitz wrote: > >> That should have fixed the warning already ... > > It's still not fixed (hence my "acked-by" for Geunter's patch). > Odd - does link order still matter even though the arch_setup_dev_archdata() function from the core platform code is declared as a weak symbol? I'll see what I can find out on elgar ... Cheers, Michael
Re: [RFC PATCH] m68k: set dma and coherent masks for Macintosh SONIC based ethernet
On Sun, 27 May 2018, Michael Schmitz wrote: > That should have fixed the warning already ... It's still not fixed (hence my "acked-by" for Geunter's patch). --
Re: [RFC PATCH] m68k: set dma and coherent masks for Macintosh SONIC based ethernet
Hi Finn,
was your patch to implement this in arch_setup_pdev_archdata() rejected?
That should have fixed the warning already ...
Am 27.05.2018 um 15:01 schrieb Finn Thain:
> On Sat, 26 May 2018, Guenter Roeck wrote:
>
>> As of commit 205e1b7f51e4 ("dma-mapping: warn when there is no
>> coherent_dma_mask") the NatSemi SONIC Ethernet driver is issuing the
>> following warning on driver initialization on Macintosh q800 systems.
>>
>> SONIC ethernet @50f0a000, MAC 08:00:07:12:34:56, IRQ 3
>> [ cut here ]
>> WARNING: CPU: 0 PID: 1 at ./include/linux/dma-mapping.h:516
>> macsonic_init+0x6a/0x15a
>> Modules linked in:
>> CPU: 0 PID: 1 Comm: swapper Not tainted 4.17.0-rc6-mac-00286-g527f47c #1
>> Stack from 0781fdd8:
>> 0781fdd8 003615b3 000181ba 05c4 07a24cbc
>> 20e8
>> 07a24800 002c196c 0001824e 00334c06 0204 001f782a 0009
>> 003358d9 001f782a 00334c06 0204 0003 07a24800
>> 002b5cb6 000372ec 001f8b1a 07a24800 00359203 50f0a000 07a14a48 0003
>> 07845c0a 0039dcca 003c835c 003c835c 0035b924 001c19de 07845c00
>> 07845c0a 0039dcca 001c06dc 07845c0a 0781fed8 0007 0054d040 07845c0a
>> Call Trace: [<000181ba>] __warn+0xc0/0xc2
>> [<20e8>] do_one_initcall+0x0/0x140
>> [<0001824e>] warn_slowpath_null+0x26/0x2c
>> [<001f782a>] macsonic_init+0x6a/0x15a
>> [<001f782a>] macsonic_init+0x6a/0x15a
>> [<002b5cb6>] memcmp+0x0/0x2a
>> [<000372ec>] printk+0x0/0x18
>> [<001f8b1a>] mac_sonic_platform_probe+0x380/0x404
>>
>> As per the warning the coherent_dma_mask is not set on this device.
>> There is nothing special about the DMA memory coherency on this hardware
>> so we can just set the mask to 32bits in the platform data for the FEC
>> ethernet devices.
>>
>> Signed-off-by: Guenter Roeck
>
> Acked-by: Finn Thain
>
> Geert, assuming that Guenter's patch is acceptable, would you also accept
> a similar patch for the "macmace" platform device?
>
>> ---
>> Modeled after f61e64310b75 ("m68k: set dma and coherent masks for platform
>> FEC ethernets").
>>
>> RFC: Is "nothing special about the DMA memory coherency" correect ?
>>
>
> As I understand it, "cache-coherence" is meaningless unless you have
> multiple CPU cores and caches. If there's only one CPU core, its cache
> can't be said to be "coherent" or "non-coherent". Moreover, DMA (direct
> memory access) concerns an IO device and a memory, not a cache, so the
> term "coherent dma" is bogus IMHO.
DMA does concern the CPU cache insofar as (without explicit cache
maintenance) the CPU cache may hold stale data after a DMA write, or
data to be used in a DMA read may not have been written back from cache
to memory. As far as I understand it, the generic DMA API does handle
these cache maintenance aspects without a need for action at the driver
level. But you're correct that the question of coherent memory does not
arise on uniprocessor systems, so the whole warning could have been
avoided.
Unless 'coherent memory' in this context means no explicit cache
maintenance is required (CPU does bus snooping - which of our platforms
fully support that)?
Cheers,
Michael
Re: [RFC PATCH] m68k: set dma and coherent masks for Macintosh SONIC based ethernet
Hi,
On Sun, May 27, 2018 at 01:01:11PM +1000, Finn Thain wrote:
> On Sat, 26 May 2018, Guenter Roeck wrote:
>
> > As of commit 205e1b7f51e4 ("dma-mapping: warn when there is no
> > coherent_dma_mask") the NatSemi SONIC Ethernet driver is issuing the
> > following warning on driver initialization on Macintosh q800 systems.
> >
> > SONIC ethernet @50f0a000, MAC 08:00:07:12:34:56, IRQ 3
> > [ cut here ]
> > WARNING: CPU: 0 PID: 1 at ./include/linux/dma-mapping.h:516
> > macsonic_init+0x6a/0x15a
> > Modules linked in:
> > CPU: 0 PID: 1 Comm: swapper Not tainted 4.17.0-rc6-mac-00286-g527f47c #1
> > Stack from 0781fdd8:
> > 0781fdd8 003615b3 000181ba 05c4 07a24cbc
> > 20e8
> > 07a24800 002c196c 0001824e 00334c06 0204 001f782a 0009
> > 003358d9 001f782a 00334c06 0204 0003 07a24800
> > 002b5cb6 000372ec 001f8b1a 07a24800 00359203 50f0a000 07a14a48 0003
> > 07845c0a 0039dcca 003c835c 003c835c 0035b924 001c19de 07845c00
> > 07845c0a 0039dcca 001c06dc 07845c0a 0781fed8 0007 0054d040 07845c0a
> > Call Trace: [<000181ba>] __warn+0xc0/0xc2
> > [<20e8>] do_one_initcall+0x0/0x140
> > [<0001824e>] warn_slowpath_null+0x26/0x2c
> > [<001f782a>] macsonic_init+0x6a/0x15a
> > [<001f782a>] macsonic_init+0x6a/0x15a
> > [<002b5cb6>] memcmp+0x0/0x2a
> > [<000372ec>] printk+0x0/0x18
> > [<001f8b1a>] mac_sonic_platform_probe+0x380/0x404
> >
> > As per the warning the coherent_dma_mask is not set on this device.
> > There is nothing special about the DMA memory coherency on this hardware
> > so we can just set the mask to 32bits in the platform data for the FEC
> > ethernet devices.
> >
> > Signed-off-by: Guenter Roeck
>
> Acked-by: Finn Thain
>
> Geert, assuming that Guenter's patch is acceptable, would you also accept
> a similar patch for the "macmace" platform device?
>
> > ---
> > Modeled after f61e64310b75 ("m68k: set dma and coherent masks for platform
> > FEC ethernets").
> >
> > RFC: Is "nothing special about the DMA memory coherency" correect ?
> >
>
> As I understand it, "cache-coherence" is meaningless unless you have
> multiple CPU cores and caches. If there's only one CPU core, its cache
Good point. Maybe it would be better to limit the warning to SMP systems
instead of (unnecessarily) fixing drivers all over the place ?
Guenter
---
>From 9eea0e1b609b69094682757285fd7f89d3930a8e Mon Sep 17 00:00:00 2001
From: Guenter Roeck
Date: Sat, 26 May 2018 21:08:39 -0700
Subject: [PATCH] dma-mapping: Warn about coherent_dma_mask only for SMP
As of commit 205e1b7f51e4 ("dma-mapping: warn when there is no
coherent_dma_mask") is unconditional, even if the affected system
is a single-CPU system where coherence is irrelevant. Limit the
warning to SMP configurations to reduce the noise.
Fixes: 205e1b7f51e4 ("dma-mapping: warn when there is no coherent_dma_mask")
Signed-off-by: Guenter Roeck
---
include/linux/dma-mapping.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index f8ab1c0f589e..ffbb39d84797 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -513,7 +513,7 @@ static inline void *dma_alloc_attrs(struct device *dev,
size_t size,
void *cpu_addr;
BUG_ON(!ops);
- WARN_ON_ONCE(dev && !dev->coherent_dma_mask);
+ WARN_ON_ONCE(IS_ENABLED(CONFIG_SMP) && dev && !dev->coherent_dma_mask);
if (dma_alloc_from_dev_coherent(dev, size, dma_handle, &cpu_addr))
return cpu_addr;
--
2.7.4
Re: [RFC PATCH] m68k: set dma and coherent masks for Macintosh SONIC based ethernet
On Sat, 26 May 2018, Guenter Roeck wrote:
> As of commit 205e1b7f51e4 ("dma-mapping: warn when there is no
> coherent_dma_mask") the NatSemi SONIC Ethernet driver is issuing the
> following warning on driver initialization on Macintosh q800 systems.
>
> SONIC ethernet @50f0a000, MAC 08:00:07:12:34:56, IRQ 3
> [ cut here ]
> WARNING: CPU: 0 PID: 1 at ./include/linux/dma-mapping.h:516
> macsonic_init+0x6a/0x15a
> Modules linked in:
> CPU: 0 PID: 1 Comm: swapper Not tainted 4.17.0-rc6-mac-00286-g527f47c #1
> Stack from 0781fdd8:
> 0781fdd8 003615b3 000181ba 05c4 07a24cbc
> 20e8
> 07a24800 002c196c 0001824e 00334c06 0204 001f782a 0009
> 003358d9 001f782a 00334c06 0204 0003 07a24800
> 002b5cb6 000372ec 001f8b1a 07a24800 00359203 50f0a000 07a14a48 0003
> 07845c0a 0039dcca 003c835c 003c835c 0035b924 001c19de 07845c00
> 07845c0a 0039dcca 001c06dc 07845c0a 0781fed8 0007 0054d040 07845c0a
> Call Trace: [<000181ba>] __warn+0xc0/0xc2
> [<20e8>] do_one_initcall+0x0/0x140
> [<0001824e>] warn_slowpath_null+0x26/0x2c
> [<001f782a>] macsonic_init+0x6a/0x15a
> [<001f782a>] macsonic_init+0x6a/0x15a
> [<002b5cb6>] memcmp+0x0/0x2a
> [<000372ec>] printk+0x0/0x18
> [<001f8b1a>] mac_sonic_platform_probe+0x380/0x404
>
> As per the warning the coherent_dma_mask is not set on this device.
> There is nothing special about the DMA memory coherency on this hardware
> so we can just set the mask to 32bits in the platform data for the FEC
> ethernet devices.
>
> Signed-off-by: Guenter Roeck
Acked-by: Finn Thain
Geert, assuming that Guenter's patch is acceptable, would you also accept
a similar patch for the "macmace" platform device?
> ---
> Modeled after f61e64310b75 ("m68k: set dma and coherent masks for platform
> FEC ethernets").
>
> RFC: Is "nothing special about the DMA memory coherency" correect ?
>
As I understand it, "cache-coherence" is meaningless unless you have
multiple CPU cores and caches. If there's only one CPU core, its cache
can't be said to be "coherent" or "non-coherent". Moreover, DMA (direct
memory access) concerns an IO device and a memory, not a cache, so the
term "coherent dma" is bogus IMHO.
--
> arch/m68k/mac/config.c | 12 +++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/arch/m68k/mac/config.c b/arch/m68k/mac/config.c
> index 0c3275aa0197..8e0476daddb8 100644
> --- a/arch/m68k/mac/config.c
> +++ b/arch/m68k/mac/config.c
> @@ -17,6 +17,7 @@
> #include
> #include
> #include
> +#include
> /* keyb */
> #include
> #include
> @@ -971,6 +972,15 @@ static const struct resource mac_scsi_ccl_rsrc[]
> __initconst = {
> },
> };
>
> +static struct platform_device macsonic_dev = {
> + .name = "macsonic",
> + .id = -1,
> + .dev= {
> + .dma_mask = &macsonic_dev.dev.coherent_dma_mask,
> + .coherent_dma_mask = DMA_BIT_MASK(32),
> + },
> +};
> +
> int __init mac_platform_init(void)
> {
> u8 *swim_base;
> @@ -1088,7 +1098,7 @@ int __init mac_platform_init(void)
>
> if (macintosh_config->ether_type == MAC_ETHER_SONIC ||
> macintosh_config->expansion_type == MAC_EXP_PDS_COMM)
> - platform_device_register_simple("macsonic", -1, NULL, 0);
> + platform_device_register(&macsonic_dev);
>
> if (macintosh_config->expansion_type == MAC_EXP_PDS ||
> macintosh_config->expansion_type == MAC_EXP_PDS_COMM)
> --
> 2.7.4
>
[RFC PATCH] m68k: set dma and coherent masks for Macintosh SONIC based ethernet
As of commit 205e1b7f51e4 ("dma-mapping: warn when there is no
coherent_dma_mask") the NatSemi SONIC Ethernet driver is issuing the
following warning on driver initialization on Macintosh q800 systems.
SONIC ethernet @50f0a000, MAC 08:00:07:12:34:56, IRQ 3
[ cut here ]
WARNING: CPU: 0 PID: 1 at ./include/linux/dma-mapping.h:516
macsonic_init+0x6a/0x15a
Modules linked in:
CPU: 0 PID: 1 Comm: swapper Not tainted 4.17.0-rc6-mac-00286-g527f47c #1
Stack from 0781fdd8:
0781fdd8 003615b3 000181ba 05c4 07a24cbc 20e8
07a24800 002c196c 0001824e 00334c06 0204 001f782a 0009
003358d9 001f782a 00334c06 0204 0003 07a24800
002b5cb6 000372ec 001f8b1a 07a24800 00359203 50f0a000 07a14a48 0003
07845c0a 0039dcca 003c835c 003c835c 0035b924 001c19de 07845c00
07845c0a 0039dcca 001c06dc 07845c0a 0781fed8 0007 0054d040 07845c0a
Call Trace: [<000181ba>] __warn+0xc0/0xc2
[<20e8>] do_one_initcall+0x0/0x140
[<0001824e>] warn_slowpath_null+0x26/0x2c
[<001f782a>] macsonic_init+0x6a/0x15a
[<001f782a>] macsonic_init+0x6a/0x15a
[<002b5cb6>] memcmp+0x0/0x2a
[<000372ec>] printk+0x0/0x18
[<001f8b1a>] mac_sonic_platform_probe+0x380/0x404
As per the warning the coherent_dma_mask is not set on this device.
There is nothing special about the DMA memory coherency on this hardware
so we can just set the mask to 32bits in the platform data for the FEC
ethernet devices.
Signed-off-by: Guenter Roeck
---
Modeled after f61e64310b75 ("m68k: set dma and coherent masks for platform
FEC ethernets").
RFC: Is "nothing special about the DMA memory coherency" correect ?
arch/m68k/mac/config.c | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/arch/m68k/mac/config.c b/arch/m68k/mac/config.c
index 0c3275aa0197..8e0476daddb8 100644
--- a/arch/m68k/mac/config.c
+++ b/arch/m68k/mac/config.c
@@ -17,6 +17,7 @@
#include
#include
#include
+#include
/* keyb */
#include
#include
@@ -971,6 +972,15 @@ static const struct resource mac_scsi_ccl_rsrc[]
__initconst = {
},
};
+static struct platform_device macsonic_dev = {
+ .name = "macsonic",
+ .id = -1,
+ .dev= {
+ .dma_mask = &macsonic_dev.dev.coherent_dma_mask,
+ .coherent_dma_mask = DMA_BIT_MASK(32),
+ },
+};
+
int __init mac_platform_init(void)
{
u8 *swim_base;
@@ -1088,7 +1098,7 @@ int __init mac_platform_init(void)
if (macintosh_config->ether_type == MAC_ETHER_SONIC ||
macintosh_config->expansion_type == MAC_EXP_PDS_COMM)
- platform_device_register_simple("macsonic", -1, NULL, 0);
+ platform_device_register(&macsonic_dev);
if (macintosh_config->expansion_type == MAC_EXP_PDS ||
macintosh_config->expansion_type == MAC_EXP_PDS_COMM)
--
2.7.4
