diff mbox

[U-Boot,v3,1/8] da850: indicate cache usage disable in config file

Message ID 1313590455-32737-2-git-send-email-nagabhushana.netagunte@ti.com
State Accepted
Headers show

Commit Message

nagabhushana.netagunte@ti.com Aug. 17, 2011, 2:14 p.m. UTC
From: Nagabhushana Netagunte <nagabhushana.netagunte@ti.com>

there are cache coherency issues when using the DAVINCI Ethernet driver,
hence caches cant be used for da850 u-boot. As per new cache management
framework,if the caches are not used in u-boot, it needs to be explicitly
indicated through macros in config file. CACHE disable is  indicated by
the following macro definitions in config file,

1. CONFIG_SYS_ICACHE_OFF
2. CONFIG_SYS_DCACHE_OFF
3. CONFIG_SYS_L2CACHE_OFF

Signed-off-by: Nagabhushana Netagunte <nagabhushana.netagunte@ti.com>
---
 include/configs/da850evm.h |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

Comments

Ben Gardiner Aug. 18, 2011, 3:13 p.m. UTC | #1
Hi,

On Wed, Aug 17, 2011 at 10:14 AM,  <nagabhushana.netagunte@ti.com> wrote:
> From: Nagabhushana Netagunte <nagabhushana.netagunte@ti.com>
>
> there are cache coherency issues when using the DAVINCI Ethernet driver,
> hence caches cant be used for da850 u-boot. As per new cache management
> framework,if the caches are not used in u-boot, it needs to be explicitly
> indicated through macros in config file. CACHE disable is  indicated by
> the following macro definitions in config file,
>
> 1. CONFIG_SYS_ICACHE_OFF
> 2. CONFIG_SYS_DCACHE_OFF
> 3. CONFIG_SYS_L2CACHE_OFF
>
> Signed-off-by: Nagabhushana Netagunte <nagabhushana.netagunte@ti.com>
> ---
>  include/configs/da850evm.h |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/include/configs/da850evm.h b/include/configs/da850evm.h
> index bbb5a9b..fdcc6e3 100644
> --- a/include/configs/da850evm.h
> +++ b/include/configs/da850evm.h
> @@ -42,6 +42,9 @@
>  #define CONFIG_SYS_HZ                  1000
>  #define CONFIG_SKIP_LOWLEVEL_INIT
>  #define CONFIG_SYS_TEXT_BASE           0xc1080000
> +#define CONFIG_SYS_ICACHE_OFF
> +#define CONFIG_SYS_DCACHE_OFF
> +#define CONFIG_SYS_L2CACHE_OFF

I've done plenty of tftp transfers of rootfs and kernel images and
flashed these to the da850evm with the caches enabled and I can't say
I've ever encountered a fatal issue here.

I understand that both Laurence and Stefan (cc'd) have confirmed that
there dcache issues with the EMA; I am assuming that the 'issue'
results in a delay in tftp'ing...

Disabling the caches will slow down decompression which will slow down
boot overall when booting from flash.

Rather than masking the issue by disabling caches and slowing down
u-boot for it's users perhaps TI should be fixing the EMAC drivers'
cache bugs instead?

Best Regards,
Ben Gardiner

---
Nanometrics Inc.
http://www.nanometrics.ca
Laurence Withers Aug. 18, 2011, 3:20 p.m. UTC | #2
On Thu, Aug 18, 2011 at 11:13:19AM -0400, Ben Gardiner wrote:
> I understand that both Laurence and Stefan (cc'd) have confirmed that
> there dcache issues with the EMA; I am assuming that the 'issue'
> results in a delay in tftp'ing...

No; the issue was that packets were being sent on to the wire with incorrect
IP checksums. It twigged that the problem was cache coherency when I put a
full hexdump of each outgoing packet in and suddenly the outbound packets
started being valid.

> Disabling the caches will slow down decompression which will slow down
> boot overall when booting from flash.
> 
> Rather than masking the issue by disabling caches and slowing down
> u-boot for it's users perhaps TI should be fixing the EMAC drivers'
> cache bugs instead?

Indeed, the correct solution is to properly manage the net buffers with
respect to the cache, although I have not attempted that change myself. I
have seen some patches start to flow that make changes in drivers to work
correctly with caches enabled, such as
http://lists.denx.de/pipermail/u-boot/2011-August/098484.html .

Bye for now,
nagabhushana.netagunte@ti.com Aug. 29, 2011, 10:56 a.m. UTC | #3
Gentlemen,

We will address cache coherency issues soon after these patches.
Earlier also, chache was disabled. Only due to new cache management
Framework which was added recently, it is explicitly needed to be 
Indicated to turn off cache. 

Since fixing the cache coherency issues with EMAC will take some time,
I want this patch to go in mainline so that issue doesn't crop up for
People who use u-boot.

Regards,
Nag
-----Original Message-----
From: Laurence Withers [mailto:lwithers@guralp.com] 
Sent: Thursday, August 18, 2011 8:50 PM
To: Ben Gardiner
Cc: Netagunte, Nagabhushana; u-boot@lists.denx.de; Rajashekhara, Sudhakar; Hadli, Manjunath; Stefan Roese
Subject: Re: [U-Boot] [PATCH v3 1/8] da850: indicate cache usage disable in config file

On Thu, Aug 18, 2011 at 11:13:19AM -0400, Ben Gardiner wrote:
> I understand that both Laurence and Stefan (cc'd) have confirmed that
> there dcache issues with the EMA; I am assuming that the 'issue'
> results in a delay in tftp'ing...

No; the issue was that packets were being sent on to the wire with incorrect
IP checksums. It twigged that the problem was cache coherency when I put a
full hexdump of each outgoing packet in and suddenly the outbound packets
started being valid.

> Disabling the caches will slow down decompression which will slow down
> boot overall when booting from flash.
> 
> Rather than masking the issue by disabling caches and slowing down
> u-boot for it's users perhaps TI should be fixing the EMAC drivers'
> cache bugs instead?

Indeed, the correct solution is to properly manage the net buffers with
respect to the cache, although I have not attempted that change myself. I
have seen some patches start to flow that make changes in drivers to work
correctly with caches enabled, such as
http://lists.denx.de/pipermail/u-boot/2011-August/098484.html .

Bye for now,
nagabhushana.netagunte@ti.com Aug. 31, 2011, 5:39 a.m. UTC | #4
Gentlemen,

If you have no further comments, can you please ACK patches?

Regards,
Nag

On Mon, Aug 29, 2011 at 16:26:57, Netagunte, Nagabhushana wrote:
> Gentlemen,
> 
> We will address cache coherency issues soon after these patches.
> Earlier also, chache was disabled. Only due to new cache management Framework which was added recently, it is explicitly needed to be Indicated to turn off cache. 
> 
> Since fixing the cache coherency issues with EMAC will take some time, I want this patch to go in mainline so that issue doesn't crop up for People who use u-boot.
> 
> Regards,
> Nag
> -----Original Message-----
> From: Laurence Withers [mailto:lwithers@guralp.com]
> Sent: Thursday, August 18, 2011 8:50 PM
> To: Ben Gardiner
> Cc: Netagunte, Nagabhushana; u-boot@lists.denx.de; Rajashekhara, Sudhakar; Hadli, Manjunath; Stefan Roese
> Subject: Re: [U-Boot] [PATCH v3 1/8] da850: indicate cache usage disable in config file
> 
> On Thu, Aug 18, 2011 at 11:13:19AM -0400, Ben Gardiner wrote:
> > I understand that both Laurence and Stefan (cc'd) have confirmed that 
> > there dcache issues with the EMA; I am assuming that the 'issue'
> > results in a delay in tftp'ing...
> 
> No; the issue was that packets were being sent on to the wire with incorrect IP checksums. It twigged that the problem was cache coherency when I put a full hexdump of each outgoing packet in and suddenly the outbound packets started being valid.
> 
> > Disabling the caches will slow down decompression which will slow down 
> > boot overall when booting from flash.
> > 
> > Rather than masking the issue by disabling caches and slowing down 
> > u-boot for it's users perhaps TI should be fixing the EMAC drivers'
> > cache bugs instead?
> 
> Indeed, the correct solution is to properly manage the net buffers with respect to the cache, although I have not attempted that change myself. I have seen some patches start to flow that make changes in drivers to work correctly with caches enabled, such as http://lists.denx.de/pipermail/u-boot/2011-August/098484.html .
> 
> Bye for now,
> -- 
> Laurence Withers, <lwithers@guralp.com>                http://www.guralp.com/
> Direct tel:+447753988197 or tel:+443333408643               Software Engineer
> General support queries: <support@guralp.com>         CMG-DCM CMG-EAM CMG-NAM
>
Ben Gardiner Aug. 31, 2011, 1:23 p.m. UTC | #5
Nagabhushana,

On Mon, Aug 29, 2011 at 6:56 AM, Netagunte, Nagabhushana
<nagabhushana.netagunte@ti.com> wrote:
>
> Gentlemen,
>
> We will address cache coherency issues soon after these patches.

Ok -- that's great news; we greatly anticipate U-boot improvements for
da850 from TI.

> Earlier also, chache was disabled. Only due to new cache management
> Framework which was added recently, it is explicitly needed to be
> Indicated to turn off cache.

My experience with the ARM-relocation changes on da8xx (which enabled
caches as I understood it) was very positive. I'm sorry I missed the
patch that disabled cache for davinci;

git log --decorate u-boot/master -- board/davinci/
arch/arm/cpu/arm926ejs/davinci/ include/configs/davinci_*
arch/arm/include/asm/arch-davinci doesn't turn up anything related to
caches so the patch (re: 'Earlier also, chache was disabled') is still
in mailing lists?

> Since fixing the cache coherency issues with EMAC will take some time,
> I want this patch to go in mainline so that issue doesn't crop up for
> People who use u-boot.

I certainly don't want anything less than the best user experience for
da850 u-boot.

It's possible that I have not been using caches in u-boot as I thought
I was -- considering I did not experience corrupted frames from the
EMAC.

On Wed, Aug 31, 2011 at 1:39 AM, Netagunte, Nagabhushana
<nagabhushana.netagunte@ti.com> wrote:
> If you have no further comments, can you please ACK patches?

for what it's worth:

Acked-by: Ben Gardiner <bengardiner@nanometrics.ca>


Best Regards,
Ben Gardiner

---
Nanometrics Inc.
+1 (613) 592-6776 x239
http://www.nanometrics.ca
diff mbox

Patch

diff --git a/include/configs/da850evm.h b/include/configs/da850evm.h
index bbb5a9b..fdcc6e3 100644
--- a/include/configs/da850evm.h
+++ b/include/configs/da850evm.h
@@ -42,6 +42,9 @@ 
 #define CONFIG_SYS_HZ			1000
 #define CONFIG_SKIP_LOWLEVEL_INIT
 #define CONFIG_SYS_TEXT_BASE		0xc1080000
+#define CONFIG_SYS_ICACHE_OFF
+#define CONFIG_SYS_DCACHE_OFF
+#define CONFIG_SYS_L2CACHE_OFF
 
 /*
  * Memory Info