Re: [PATCH] net: fec: set dma_coherent_mask

From: Greg Ungerer <gerg@linux-m68k.org>
To: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: netdev <netdev@vger.kernel.org>, Linux/m68k <linux-m68k@vger.kernel.org>
Subject: Re: [PATCH] net: fec: set dma_coherent_mask
Date: Wed, 28 Mar 2018 22:29:36 +1000	[thread overview]
Message-ID: <d00139ba-113a-9846-3b85-4d015901fbbb@linux-m68k.org> (raw)
In-Reply-To: <CAMuHMdXzySkpAi+kS+XCXR8kjY_AsV4h6t+XsuXx4o_0u2GSEA@mail.gmail.com>

Hi Geert,

On 28/03/18 17:57, Geert Uytterhoeven wrote:
[skip]
>> [PATCH] m68k: set dma and coherent masks for platform FEC ethernets
>>
>> As of commit 205e1b7f51e4 ("dma-mapping: warn when there is no
>> coherent_dma_mask") the Freescale FEC driver is issuing the following
>> warning on driver initialization on ColdFire systems:
>>
>> WARNING: CPU: 0 PID: 1 at ./include/linux/dma-mapping.h:516 0x40159e20
>> Modules linked in:
>> CPU: 0 PID: 1 Comm: swapper Not tainted 4.16.0-rc7-dirty #4
>> Stack from 41833dd8:
>>          41833dd8 40259c53 40025534 40279e26 00000003 00000000 4004e514 41827000
>>          400255de 40244e42 00000204 40159e20 00000009 00000000 00000000 4024531d
>>          40159e20 40244e42 00000204 00000000 00000000 00000000 00000007 00000000
>>          00000000 40279e26 4028d040 40226576 4003ae88 40279e26 418273f6 41833ef8
>>          7fffffff 418273f2 41867028 4003c9a2 4180ac6c 00000004 41833f8c 4013e71c
>>          40279e1c 40279e26 40226c16 4013ced2 40279e26 40279e58 4028d040 00000000
>> Call Trace:
>>          [<40025534>] 0x40025534
>>   [<4004e514>] 0x4004e514
>>   [<400255de>] 0x400255de
>>   [<40159e20>] 0x40159e20
>>   [<40159e20>] 0x40159e20
>>
>> It is not fatal, the driver and the system continue to function normally.
>>
>> 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: Greg Ungerer <gerg@linux-m68k.org>
>> ---
>>   arch/m68k/coldfire/device.c | 14 ++++++++++++--
>>   1 file changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/m68k/coldfire/device.c b/arch/m68k/coldfire/device.c
>> index 84938fd..f93e0e5 100644
>> --- a/arch/m68k/coldfire/device.c
>> +++ b/arch/m68k/coldfire/device.c
>> @@ -130,12 +130,18 @@
>>          },
>>   };
>>
>> +static u64 mcf_dma_mask = DMA_BIT_MASK(32);
>> +
>>   static struct platform_device mcf_fec0 = {
>>          .name                   = FEC_NAME,
>>          .id                     = 0,
>>          .num_resources          = ARRAY_SIZE(mcf_fec0_resources),
>>          .resource               = mcf_fec0_resources,
>> -       .dev.platform_data      = FEC_PDATA,
>> +       .dev = {
>> +               .dma_mask               = &mcf_dma_mask,
> 
> Can you make this &mcf_fec0.dev.coherent_dma_mask, removing the need for
> mcf_dma_mask, or doesn't C allow that?

Yes, sure can. Looks like some other architectures do that too.
I'll change it. Thanks.

Regards
Greg

>> +               .coherent_dma_mask      = DMA_BIT_MASK(32),
>> +               .platform_data          = FEC_PDATA,
>> +       }
>>   };
>>
>>   #ifdef MCFFEC_BASE1
>> @@ -167,7 +173,11 @@
>>          .id                     = 1,
>>          .num_resources          = ARRAY_SIZE(mcf_fec1_resources),
>>          .resource               = mcf_fec1_resources,
>> -       .dev.platform_data      = FEC_PDATA,
>> +       .dev = {
>> +               .dma_mask               = &mcf_dma_mask,
> 
> Likewise.
> 
>> +               .coherent_dma_mask      = DMA_BIT_MASK(32),
>> +               .platform_data          = FEC_PDATA,
>> +       }
> 
> Gr{oetje,eeting}s,
> 
>                          Geert
> 