Re: [RFC PATCH] m68k: set dma and coherent masks for Macintosh SONIC based ethernet

2018-05-26 Thread 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).

-- 
--
To unsubscribe from this list: send the line "unsubscribe linux-m68k" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC PATCH] m68k: set dma and coherent masks for Macintosh SONIC based ethernet

2018-05-26 Thread Michael Schmitz
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


--
To unsubscribe from this list: send the line "unsubscribe linux-m68k" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC PATCH] m68k: set dma and coherent masks for Macintosh SONIC based ethernet

2018-05-26 Thread Guenter Roeck
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, _addr))
return cpu_addr;
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-m68k" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC PATCH] m68k: set dma and coherent masks for Macintosh SONIC based ethernet

2018-05-26 Thread 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.

-- 

>  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 = _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(_dev);
>  
>   if (macintosh_config->expansion_type == MAC_EXP_PDS ||
>   macintosh_config->expansion_type == MAC_EXP_PDS_COMM)
> -- 
> 2.7.4
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-m68k" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[RFC PATCH] m68k: set dma and coherent masks for Macintosh SONIC based ethernet

2018-05-26 Thread Guenter Roeck
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 = _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(_dev);
 
if (macintosh_config->expansion_type == MAC_EXP_PDS ||
macintosh_config->expansion_type == MAC_EXP_PDS_COMM)
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-m68k" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] dmaengine: mcf-edma: add ColdFire mcf5441x eDMA support

2018-05-26 Thread Angelo Dureghello
Hi Vinod,

thanks for your support.

On Wed, May 23, 2018 at 11:07:06AM +0530, Vinod wrote:
> On 22-05-18, 23:28, Angelo Dureghello wrote:
> > Hi Vinod,
> > 
> > On Mon, May 07, 2018 at 07:45:35PM +0530, Vinod Koul wrote:
> > > On Fri, May 04, 2018 at 09:18:19PM +0200, Angelo Dureghello wrote:
> > > > Hi Vinod,
> > > > 
> > > > thanks for the review,
> > > > 
> > > > On Thu, May 03, 2018 at 10:18:30PM +0530, Vinod Koul wrote:
> > > > > On Wed, Apr 25, 2018 at 10:08:17PM +0200, Angelo Dureghello wrote:
> > > > > > This patch adds dma support for NXP mcf5441x (ColdFire) family.
> > > > > > 
> > > > > > ColdFire mcf5441x implements an edma hw module similar to the
> > > > > 
> > > > > Is it similar to to edma ?
> > > > > 
> > > > 
> > > > It is similar to Freescale "edma" but with a different number of
> > > > channels, a bit different register set, different interrupt
> > > > structure, no channel multiplexer.
> > > 
> > > ok
> > > 
> > > > > > one implemented in Vybrid VFxxx controllers, but with a slightly
> > > > > > different register set, more dma channels (64 instead of 32),
> > > > > > a different interrupt mechanism and some other minor differences.
> > > > > > 
> > > > > > For the above reasons, modfying fsl-edma.c was too complex and
> > > > > > likely too ugly. From here, the decision to create a different
> > > > > > driver, but starting from fsl-edma.
> > > > > 
> > > > > can the common stuff be made into a lib and shared between then two 
> > > > > rather
> > > > > than having a same driver or different drivers?
> > > > 
> > > > It should be possible to collect some common code in a kind of
> > > > mcf_edma_core.c common module, but in this case i cannot then test
> > > > the Vybrid edma after the changes since i miss that hardware.
> > > 
> > > Sure you should send the patches and folks who care about fsl driver
> > > would look it up and test
> > > 
> > > > Would be maybe possible for you to diff fsl-edma and this mcf-edma,
> > > > just to confirm if i can still stay this way, or if moving to a
> > > > library becomes mandatory ?
> > > 
> > > well since you know the IP you would make a better guess on that, best is
> > > to check register sets in drivers
> > > 
> > I fixed all the discussed points.
> > 
> > Actaully mcf-edma (ColdFire) has a slightly different register set (due to 
> > 64
> > channels in place of 16 of fsl-edma) and, for the same reason, a different
> > DMA interrupt structure.
> > Also, i simplified some parts of the driver considering ColdFire (mcf) 
> > big endian architecture.
> > 
> > So i would send a rev 2 patch with all the fixes, than eventually in a 
> > second
> > phase i may try to create some common code, but at least we have the 
> > ColdFire
> > DMA. What do you think ?
> 
> wouldn't it be easier to just make common parts and then add edma specific 
> code.
> If I was doing this it would be my apprach and that way code edma specific 
> will
> be lesser and faster review
> 

I tried to set up a common module, but couldn't reach any good point.

Issues are:
1) Edma register set between 32 and 64ch is similar, but some offsets/names 
are not matching between the 2 variants, some registers names are swapped over
the reg. address range,
2) interrupt numbers and scheme is still different, handler implementation 
comes 
different,
3) as a corollary of the above, all the common functions that needs to access 
edma registers should use same structure pointers. I could use a union
someway but points where register are accessed are many, and i should
differentiate the access in each case, referencing to a different structure
in each case.

If you have any idea on how i could reach a common module, with 2 different 
registers set, that's welcome.
I stay on the thought that a separate 64-channel module is the best
way to go here.

Currently, as Freescale "edma" variants, i know:

Vybrid VFXXX   32ch   DMA multiplexer   reg.set 1
Kynetis K70 (CortexM4) 32ch   DMA multiplexer   reg.set 1
imx8xx (coming)32ch   no multiplexerreg.set 1
MPC57xxk   32ch   DMA multiplexer   reg.set 1
ColdFire mcf5441x  64ch   no multiplexerreg.set 2 <---

There may me other cpu using this fsl edma module but not in my knowledge
right now.

So i still think at the end, to have 2 separate drivers for the 32 and 64
variant is good and probably the most ordered/clean solution.

Regards,
Angelo


> -- 
> ~Vinod
> --
> To unsubscribe from this list: send the line "unsubscribe dmaengine" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-m68k" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html