* [PATCH] net: macb: Clean 64b dma addresses if they are not detected
@ 2018-09-19 16:08 Michal Simek
2018-09-19 17:11 ` [U-Boot] " Joe Hershberger
2018-09-19 18:08 ` Edgar E. Iglesias
0 siblings, 2 replies; 10+ messages in thread
From: Michal Simek @ 2018-09-19 16:08 UTC (permalink / raw)
To: linux-kernel, monstr, Edgar E. Iglesias
Cc: David S. Miller, netdev, u-boot, Joe Hershberger, Nicolas Ferre
Clear ADDR64 dma bit in DMACFG register in case that HW_DMA_CAP_64B
is not detected on 64bit system.
The issue was observed when bootloader(u-boot) does not check macb
feature at DCFG6 register (DAW64_OFFSET) and enabling 64bit dma support
by default. Then macb driver is reading DMACFG register back and only
adding 64bit dma configuration but not cleaning it out.
This is also align with other features which are also cleared if they are not
present.
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
drivers/net/ethernet/cadence/macb_main.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 16e4ef7d7185..79707dff3f13 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -2163,6 +2163,8 @@ static void macb_configure_dma(struct macb *bp)
#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
if (bp->hw_dma_cap & HW_DMA_CAP_64B)
dmacfg |= GEM_BIT(ADDR64);
+ else
+ dmacfg &= ~GEM_BIT(ADDR64);
#endif
#ifdef CONFIG_MACB_USE_HWSTAMP
if (bp->hw_dma_cap & HW_DMA_CAP_PTP)
--
1.9.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [U-Boot] [PATCH] net: macb: Clean 64b dma addresses if they are not detected
2018-09-19 17:11 ` [U-Boot] " Joe Hershberger
@ 2018-09-19 17:11 ` Joe Hershberger
2018-09-19 17:11 ` Joe Hershberger
` (2 subsequent siblings)
3 siblings, 0 replies; 10+ messages in thread
From: Joe Hershberger @ 2018-09-19 17:11 UTC (permalink / raw)
To: Michal Simek
Cc: linux-kernel, Michal Simek, edgari, Joseph Hershberger, netdev,
nicolas.ferre, davem, u-boot
On Wed, Sep 19, 2018 at 11:08 AM Michal Simek <michal.simek@xilinx.com> wrote:
>
> Clear ADDR64 dma bit in DMACFG register in case that HW_DMA_CAP_64B
> is not detected on 64bit system.
> The issue was observed when bootloader(u-boot) does not check macb
> feature at DCFG6 register (DAW64_OFFSET) and enabling 64bit dma support
> by default. Then macb driver is reading DMACFG register back and only
> adding 64bit dma configuration but not cleaning it out.
>
> This is also align with other features which are also cleared if they are not
> present.
>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Acked-by: Joe Hershberger <joe.hershberger@ni.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [U-Boot] [PATCH] net: macb: Clean 64b dma addresses if they are not detected
2018-09-19 16:08 [PATCH] net: macb: Clean 64b dma addresses if they are not detected Michal Simek
@ 2018-09-19 17:11 ` Joe Hershberger
2018-09-19 17:11 ` Joe Hershberger
` (3 more replies)
2018-09-19 18:08 ` Edgar E. Iglesias
1 sibling, 4 replies; 10+ messages in thread
From: Joe Hershberger @ 2018-09-19 17:11 UTC (permalink / raw)
To: Michal Simek
Cc: linux-kernel, Michal Simek, edgari, Joseph Hershberger, netdev,
nicolas.ferre, davem, u-boot
On Wed, Sep 19, 2018 at 11:08 AM Michal Simek <michal.simek@xilinx.com> wrote:
>
> Clear ADDR64 dma bit in DMACFG register in case that HW_DMA_CAP_64B
> is not detected on 64bit system.
> The issue was observed when bootloader(u-boot) does not check macb
> feature at DCFG6 register (DAW64_OFFSET) and enabling 64bit dma support
> by default. Then macb driver is reading DMACFG register back and only
> adding 64bit dma configuration but not cleaning it out.
>
> This is also align with other features which are also cleared if they are not
> present.
>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Acked-by: Joe Hershberger <joe.hershberger@ni.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [U-Boot] [PATCH] net: macb: Clean 64b dma addresses if they are not detected
2018-09-19 17:11 ` [U-Boot] " Joe Hershberger
2018-09-19 17:11 ` Joe Hershberger
@ 2018-09-19 17:11 ` Joe Hershberger
2018-09-19 17:17 ` Joe Hershberger
2018-09-19 17:17 ` Joe Hershberger
3 siblings, 0 replies; 10+ messages in thread
From: Joe Hershberger @ 2018-09-19 17:11 UTC (permalink / raw)
To: Michal Simek
Cc: linux-kernel, Michal Simek, edgari, Joseph Hershberger, netdev,
nicolas.ferre, davem, u-boot
On Wed, Sep 19, 2018 at 11:08 AM Michal Simek <michal.simek@xilinx.com> wrote:
>
> Clear ADDR64 dma bit in DMACFG register in case that HW_DMA_CAP_64B
> is not detected on 64bit system.
> The issue was observed when bootloader(u-boot) does not check macb
> feature at DCFG6 register (DAW64_OFFSET) and enabling 64bit dma support
> by default. Then macb driver is reading DMACFG register back and only
> adding 64bit dma configuration but not cleaning it out.
>
> This is also align with other features which are also cleared if they are not
> present.
>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Acked-by: Joe Hershberger <joe.hershberger@ni.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [U-Boot] [PATCH] net: macb: Clean 64b dma addresses if they are not detected
2018-09-19 17:11 ` [U-Boot] " Joe Hershberger
2018-09-19 17:11 ` Joe Hershberger
2018-09-19 17:11 ` Joe Hershberger
@ 2018-09-19 17:17 ` Joe Hershberger
2018-09-19 17:17 ` Joe Hershberger
3 siblings, 0 replies; 10+ messages in thread
From: Joe Hershberger @ 2018-09-19 17:17 UTC (permalink / raw)
To: Michal Simek
Cc: linux-kernel, Michal Simek, edgari, Joseph Hershberger, netdev,
nicolas.ferre, davem, u-boot
On Wed, Sep 19, 2018 at 11:08 AM Michal Simek <michal.simek@xilinx.com> wrote:
>
> Clear ADDR64 dma bit in DMACFG register in case that HW_DMA_CAP_64B
> is not detected on 64bit system.
> The issue was observed when bootloader(u-boot) does not check macb
> feature at DCFG6 register (DAW64_OFFSET) and enabling 64bit dma support
> by default. Then macb driver is reading DMACFG register back and only
> adding 64bit dma configuration but not cleaning it out.
>
> This is also align with other features which are also cleared if they are not
> present.
>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Acked-by: Joe Hershberger <joe.hershberger@ni.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [U-Boot] [PATCH] net: macb: Clean 64b dma addresses if they are not detected
2018-09-19 17:11 ` [U-Boot] " Joe Hershberger
` (2 preceding siblings ...)
2018-09-19 17:17 ` Joe Hershberger
@ 2018-09-19 17:17 ` Joe Hershberger
3 siblings, 0 replies; 10+ messages in thread
From: Joe Hershberger @ 2018-09-19 17:17 UTC (permalink / raw)
To: Michal Simek
Cc: linux-kernel, Michal Simek, edgari, Joseph Hershberger, netdev,
nicolas.ferre, davem, u-boot
On Wed, Sep 19, 2018 at 11:08 AM Michal Simek <michal.simek@xilinx.com> wrote:
>
> Clear ADDR64 dma bit in DMACFG register in case that HW_DMA_CAP_64B
> is not detected on 64bit system.
> The issue was observed when bootloader(u-boot) does not check macb
> feature at DCFG6 register (DAW64_OFFSET) and enabling 64bit dma support
> by default. Then macb driver is reading DMACFG register back and only
> adding 64bit dma configuration but not cleaning it out.
>
> This is also align with other features which are also cleared if they are not
> present.
>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Acked-by: Joe Hershberger <joe.hershberger@ni.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] net: macb: Clean 64b dma addresses if they are not detected
2018-09-19 16:08 [PATCH] net: macb: Clean 64b dma addresses if they are not detected Michal Simek
2018-09-19 17:11 ` [U-Boot] " Joe Hershberger
@ 2018-09-19 18:08 ` Edgar E. Iglesias
2018-09-20 6:23 ` Michal Simek
1 sibling, 1 reply; 10+ messages in thread
From: Edgar E. Iglesias @ 2018-09-19 18:08 UTC (permalink / raw)
To: Michal Simek
Cc: linux-kernel, monstr, Edgar E. Iglesias, David S. Miller, netdev,
u-boot, Joe Hershberger, Nicolas Ferre
On Wed, Sep 19, 2018 at 06:08:18PM +0200, Michal Simek wrote:
> Clear ADDR64 dma bit in DMACFG register in case that HW_DMA_CAP_64B
> is not detected on 64bit system.
> The issue was observed when bootloader(u-boot) does not check macb
> feature at DCFG6 register (DAW64_OFFSET) and enabling 64bit dma support
> by default. Then macb driver is reading DMACFG register back and only
> adding 64bit dma configuration but not cleaning it out.
>
> This is also align with other features which are also cleared if they are not
> present.
Hi Michal,
>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
>
> drivers/net/ethernet/cadence/macb_main.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index 16e4ef7d7185..79707dff3f13 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -2163,6 +2163,8 @@ static void macb_configure_dma(struct macb *bp)
> #ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
> if (bp->hw_dma_cap & HW_DMA_CAP_64B)
> dmacfg |= GEM_BIT(ADDR64);
> + else
> + dmacfg &= ~GEM_BIT(ADDR64);
> #endif
I think you might want to do this clearing outside of the #ifdef.
If CONFIG_ARCH_DMA_ADDR_T_64BIT is not defined, we'd want to make
sure the ADDR64 is cleared. E.g something like:
dmacfg &= ~GEM_BIT(ADDR64);
#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
if (bp->hw_dma_cap & HW_DMA_CAP_64B)
dmacfg |= GEM_BIT(ADDR64);
#endif
Same thing for the USE_HWSTAMP/PTP flags below.
Best regards,
Edgar
> #ifdef CONFIG_MACB_USE_HWSTAMP
> if (bp->hw_dma_cap & HW_DMA_CAP_PTP)
> --
> 1.9.1
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] net: macb: Clean 64b dma addresses if they are not detected
2018-09-19 18:08 ` Edgar E. Iglesias
@ 2018-09-20 6:23 ` Michal Simek
2018-09-21 12:38 ` Nicolas Ferre
0 siblings, 1 reply; 10+ messages in thread
From: Michal Simek @ 2018-09-20 6:23 UTC (permalink / raw)
To: Edgar E. Iglesias, Michal Simek
Cc: linux-kernel, monstr, Edgar E. Iglesias, David S. Miller, netdev,
u-boot, Joe Hershberger, Nicolas Ferre
On 19.9.2018 20:08, Edgar E. Iglesias wrote:
> On Wed, Sep 19, 2018 at 06:08:18PM +0200, Michal Simek wrote:
>> Clear ADDR64 dma bit in DMACFG register in case that HW_DMA_CAP_64B
>> is not detected on 64bit system.
>> The issue was observed when bootloader(u-boot) does not check macb
>> feature at DCFG6 register (DAW64_OFFSET) and enabling 64bit dma support
>> by default. Then macb driver is reading DMACFG register back and only
>> adding 64bit dma configuration but not cleaning it out.
>>
>> This is also align with other features which are also cleared if they are not
>> present.
>
> Hi Michal,
>
>>
>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>> ---
>>
>> drivers/net/ethernet/cadence/macb_main.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
>> index 16e4ef7d7185..79707dff3f13 100644
>> --- a/drivers/net/ethernet/cadence/macb_main.c
>> +++ b/drivers/net/ethernet/cadence/macb_main.c
>> @@ -2163,6 +2163,8 @@ static void macb_configure_dma(struct macb *bp)
>> #ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
>> if (bp->hw_dma_cap & HW_DMA_CAP_64B)
>> dmacfg |= GEM_BIT(ADDR64);
>> + else
>> + dmacfg &= ~GEM_BIT(ADDR64);
>> #endif
>
> I think you might want to do this clearing outside of the #ifdef.
> If CONFIG_ARCH_DMA_ADDR_T_64BIT is not defined, we'd want to make
> sure the ADDR64 is cleared. E.g something like:
>
> dmacfg &= ~GEM_BIT(ADDR64);
> #ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
> if (bp->hw_dma_cap & HW_DMA_CAP_64B)
> dmacfg |= GEM_BIT(ADDR64);
> #endif
>
>
> Same thing for the USE_HWSTAMP/PTP flags below.
Origin patch, which introduce this read with mask,
macfg = gem_readl(bp, DMACFG) & ~GEM_BF(RXBS, -1L);
was done in 2011 and from that time this function was extended a little
bit. I am even not quite sure if make sense to read this reg and apply
setting on the top of it.
Nicolas: Isn't it better simply compose that reg from scratch?
Thanks,
Michal
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] net: macb: Clean 64b dma addresses if they are not detected
2018-09-20 6:23 ` Michal Simek
@ 2018-09-21 12:38 ` Nicolas Ferre
2018-09-25 6:34 ` Michal Simek
0 siblings, 1 reply; 10+ messages in thread
From: Nicolas Ferre @ 2018-09-21 12:38 UTC (permalink / raw)
To: Michal Simek, Edgar E. Iglesias
Cc: linux-kernel, monstr, Edgar E. Iglesias, David S. Miller, netdev,
u-boot, Joe Hershberger
Michal,
On 20/09/2018 at 08:23, Michal Simek wrote:
> On 19.9.2018 20:08, Edgar E. Iglesias wrote:
>> On Wed, Sep 19, 2018 at 06:08:18PM +0200, Michal Simek wrote:
>>> Clear ADDR64 dma bit in DMACFG register in case that HW_DMA_CAP_64B
>>> is not detected on 64bit system.
>>> The issue was observed when bootloader(u-boot) does not check macb
>>> feature at DCFG6 register (DAW64_OFFSET) and enabling 64bit dma support
>>> by default. Then macb driver is reading DMACFG register back and only
>>> adding 64bit dma configuration but not cleaning it out.
>>>
>>> This is also align with other features which are also cleared if they are not
>>> present.
>>
>> Hi Michal,
>>
>>>
>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>>> ---
>>>
>>> drivers/net/ethernet/cadence/macb_main.c | 2 ++
>>> 1 file changed, 2 insertions(+)
>>>
>>> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
>>> index 16e4ef7d7185..79707dff3f13 100644
>>> --- a/drivers/net/ethernet/cadence/macb_main.c
>>> +++ b/drivers/net/ethernet/cadence/macb_main.c
>>> @@ -2163,6 +2163,8 @@ static void macb_configure_dma(struct macb *bp)
>>> #ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
>>> if (bp->hw_dma_cap & HW_DMA_CAP_64B)
>>> dmacfg |= GEM_BIT(ADDR64);
>>> + else
>>> + dmacfg &= ~GEM_BIT(ADDR64);
>>> #endif
>>
>> I think you might want to do this clearing outside of the #ifdef.
>> If CONFIG_ARCH_DMA_ADDR_T_64BIT is not defined, we'd want to make
>> sure the ADDR64 is cleared. E.g something like:
>>
>> dmacfg &= ~GEM_BIT(ADDR64);
>> #ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
>> if (bp->hw_dma_cap & HW_DMA_CAP_64B)
>> dmacfg |= GEM_BIT(ADDR64);
>> #endif
>>
>>
>> Same thing for the USE_HWSTAMP/PTP flags below.
>
> Origin patch, which introduce this read with mask,
> macfg = gem_readl(bp, DMACFG) & ~GEM_BF(RXBS, -1L);
> was done in 2011 and from that time this function was extended a little
> bit. I am even not quite sure if make sense to read this reg and apply
> setting on the top of it.
>
> Nicolas: Isn't it better simply compose that reg from scratch?
I have several arguments against composing this register from scratch:
1/ the reset value of this register is non-null for both of our
platforms and it could be meaningful to keep some of these values.
2/ one bitfield could use different values between Zynq and AT91: RXBMS
(1kB to 8kB for Zynq and 512 to 4KB for AT91), with same encoding.
3/ and well, this is the type of register with multiple bits that are
marked as "reserved" and that experience tells that they might be
connected to something...
So, I'm all for correcting the code like what Edgar suggests.
Best regards,
--
Nicolas Ferre
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] net: macb: Clean 64b dma addresses if they are not detected
2018-09-21 12:38 ` Nicolas Ferre
@ 2018-09-25 6:34 ` Michal Simek
0 siblings, 0 replies; 10+ messages in thread
From: Michal Simek @ 2018-09-25 6:34 UTC (permalink / raw)
To: Nicolas Ferre, Michal Simek, Edgar E. Iglesias
Cc: linux-kernel, monstr, Edgar E. Iglesias, David S. Miller, netdev,
u-boot, Joe Hershberger
On 21.9.2018 14:38, Nicolas Ferre wrote:
> Michal,
>
> On 20/09/2018 at 08:23, Michal Simek wrote:
>> On 19.9.2018 20:08, Edgar E. Iglesias wrote:
>>> On Wed, Sep 19, 2018 at 06:08:18PM +0200, Michal Simek wrote:
>>>> Clear ADDR64 dma bit in DMACFG register in case that HW_DMA_CAP_64B
>>>> is not detected on 64bit system.
>>>> The issue was observed when bootloader(u-boot) does not check macb
>>>> feature at DCFG6 register (DAW64_OFFSET) and enabling 64bit dma support
>>>> by default. Then macb driver is reading DMACFG register back and only
>>>> adding 64bit dma configuration but not cleaning it out.
>>>>
>>>> This is also align with other features which are also cleared if
>>>> they are not
>>>> present.
>>>
>>> Hi Michal,
>>>
>>>>
>>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>>>> ---
>>>>
>>>> drivers/net/ethernet/cadence/macb_main.c | 2 ++
>>>> 1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/drivers/net/ethernet/cadence/macb_main.c
>>>> b/drivers/net/ethernet/cadence/macb_main.c
>>>> index 16e4ef7d7185..79707dff3f13 100644
>>>> --- a/drivers/net/ethernet/cadence/macb_main.c
>>>> +++ b/drivers/net/ethernet/cadence/macb_main.c
>>>> @@ -2163,6 +2163,8 @@ static void macb_configure_dma(struct macb *bp)
>>>> #ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
>>>> if (bp->hw_dma_cap & HW_DMA_CAP_64B)
>>>> dmacfg |= GEM_BIT(ADDR64);
>>>> + else
>>>> + dmacfg &= ~GEM_BIT(ADDR64);
>>>> #endif
>>>
>>> I think you might want to do this clearing outside of the #ifdef.
>>> If CONFIG_ARCH_DMA_ADDR_T_64BIT is not defined, we'd want to make
>>> sure the ADDR64 is cleared. E.g something like:
>>>
>>> dmacfg &= ~GEM_BIT(ADDR64);
>>> #ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
>>> if (bp->hw_dma_cap & HW_DMA_CAP_64B)
>>> dmacfg |= GEM_BIT(ADDR64);
>>> #endif
>>>
>>>
>>> Same thing for the USE_HWSTAMP/PTP flags below.
>>
>> Origin patch, which introduce this read with mask,
>> macfg = gem_readl(bp, DMACFG) & ~GEM_BF(RXBS, -1L);
>> was done in 2011 and from that time this function was extended a little
>> bit. I am even not quite sure if make sense to read this reg and apply
>> setting on the top of it.
>>
>> Nicolas: Isn't it better simply compose that reg from scratch?
>
> I have several arguments against composing this register from scratch:
>
> 1/ the reset value of this register is non-null for both of our
> platforms and it could be meaningful to keep some of these values.
>
> 2/ one bitfield could use different values between Zynq and AT91: RXBMS
> (1kB to 8kB for Zynq and 512 to 4KB for AT91), with same encoding.
>
> 3/ and well, this is the type of register with multiple bits that are
> marked as "reserved" and that experience tells that they might be
> connected to something...
>
> So, I'm all for correcting the code like what Edgar suggests.
ok. I have sent v2.
M
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2018-09-25 6:35 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-19 16:08 [PATCH] net: macb: Clean 64b dma addresses if they are not detected Michal Simek
2018-09-19 17:11 ` [U-Boot] " Joe Hershberger
2018-09-19 17:11 ` Joe Hershberger
2018-09-19 17:11 ` Joe Hershberger
2018-09-19 17:17 ` Joe Hershberger
2018-09-19 17:17 ` Joe Hershberger
2018-09-19 18:08 ` Edgar E. Iglesias
2018-09-20 6:23 ` Michal Simek
2018-09-21 12:38 ` Nicolas Ferre
2018-09-25 6:34 ` Michal Simek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).