[U-Boot] [PATCH] ARM1136: add cache flush and invalidate operations
Since commit 5c1ad3e6f8ae578bbe30e09652f1531e9bc22031 (net: fec_mxc: allow use with cache enabled) the FEC_MXC driver uses flush_dcache_range() and invalidate_dcache_range() functions. This driver is also configured for ARM1136 based 'flea3' and 'mx35pdk' boards which currently do not build as there are no ARM1136 specific flush_dcache_range() and invalidate_dcache_range() functions. Add various ARM1136 cache functions to fix building for 'flea3' and 'mx35pdk'.
Signed-off-by: Anatolij Gustschin agust@denx.de Cc: Stefano Babic sbabic@denx.de Cc: Fabio Estevam fabio.estevam@freescale.com --- This patch is completely untested, I have not the HW to test on. Could someone test this patch? The mentioned commit to the FEC_MXC driver is in u-boot-arm.git master.
Thanks, Anatolij
arch/arm/cpu/arm1136/cpu.c | 78 ++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 78 insertions(+), 0 deletions(-)
diff --git a/arch/arm/cpu/arm1136/cpu.c b/arch/arm/cpu/arm1136/cpu.c index 2b91631..43b4f5f 100644 --- a/arch/arm/cpu/arm1136/cpu.c +++ b/arch/arm/cpu/arm1136/cpu.c @@ -75,3 +75,81 @@ static void cache_flush(void) asm ("mcr p15, 0, %0, c7, c7, 0": :"r" (i)); /* invalidate both caches and flush btb */ asm ("mcr p15, 0, %0, c7, c10, 4": :"r" (i)); /* mem barrier to sync things */ } + +#ifndef CONFIG_SYS_DCACHE_OFF + +#ifndef CONFIG_SYS_CACHELINE_SIZE +#define CONFIG_SYS_CACHELINE_SIZE 32 +#endif + +void invalidate_dcache_all(void) +{ + asm ("mcr p15, 0, %0, c7, c6, 0" : : "r" (0)); +} + +void flush_dcache_all(void) +{ + asm ("mcr p15, 0, %0, c7, c10, 0" : : "r" (0)); + asm ("mcr p15, 0, %0, c7, c10, 4" : : "r" (0)); +} + +static inline int bad_cache_range(unsigned long start, unsigned long stop) +{ + if ((start | stop) & (CONFIG_SYS_CACHELINE_SIZE - 1)) { + printf("Misaligned cache operation [%08lx, %08lx]\n", + start, stop); + return 1; + } + + return 0; +} + +void invalidate_dcache_range(unsigned long start, unsigned long stop) +{ + if (bad_cache_range(start, stop)) + return; + + while (start < stop) { + asm ("mcr p15, 0, %0, c7, c6, 1" : : "r" (start)); + start += CONFIG_SYS_CACHELINE_SIZE; + } +} + +void flush_dcache_range(unsigned long start, unsigned long stop) +{ + if (bad_cache_range(start, stop)) + return; + + while (start < stop) { + asm ("mcr p15, 0, %0, c7, c14, 1" : : "r" (start)); + start += CONFIG_SYS_CACHELINE_SIZE; + } + + asm ("mcr p15, 0, %0, c7, c10, 4" : : "r" (0)); +} + +void flush_cache(unsigned long start, unsigned long size) +{ + flush_dcache_range(start, start + size); +} +#else /* #ifndef CONFIG_SYS_DCACHE_OFF */ +void invalidate_dcache_all(void) +{ +} + +void flush_dcache_all(void) +{ +} + +void invalidate_dcache_range(unsigned long start, unsigned long stop) +{ +} + +void flush_dcache_range(unsigned long start, unsigned long stop) +{ +} + +void flush_cache(unsigned long start, unsigned long size) +{ +} +#endif /* #ifndef CONFIG_SYS_DCACHE_OFF */
On 30/03/2012 16:02, Anatolij Gustschin wrote:
Since commit 5c1ad3e6f8ae578bbe30e09652f1531e9bc22031
Hi Antolji,
(net: fec_mxc: allow use with cache enabled) the FEC_MXC driver uses flush_dcache_range() and invalidate_dcache_range() functions. This driver is also configured for ARM1136 based 'flea3' and 'mx35pdk' boards which currently do not build as there are no ARM1136 specific flush_dcache_range() and
The issue is known - that is one reason why I marked the cache patches for the -next. I do not know if we can run enough tests before release.
Patches for M28 / MX5 / MX6 are not part of u-boot-imx, neither are yet merged into u-boot mainline by Wolfgang. On which tree have you seen that the patch was already merged ?
Stefano
Hi Stefano,
On Fri, 30 Mar 2012 16:20:19 +0200 Stefano Babic sbabic@denx.de wrote:
On 30/03/2012 16:02, Anatolij Gustschin wrote:
Since commit 5c1ad3e6f8ae578bbe30e09652f1531e9bc22031
Hi Antolji,
(net: fec_mxc: allow use with cache enabled) the FEC_MXC driver uses flush_dcache_range() and invalidate_dcache_range() functions. This driver is also configured for ARM1136 based 'flea3' and 'mx35pdk' boards which currently do not build as there are no ARM1136 specific flush_dcache_range() and
The issue is known - that is one reason why I marked the cache patches for the -next. I do not know if we can run enough tests before release.
Patches for M28 / MX5 / MX6 are not part of u-boot-imx, neither are yet merged into u-boot mainline by Wolfgang. On which tree have you seen that the patch was already merged ?
I pulled u-boot-arm.git master for build tests and see this change on the FEC driver in resulting tree.
Thanks,
Anatolij
On 30/03/2012 16:35, Anatolij Gustschin wrote:
Hi Stefano,
On Fri, 30 Mar 2012 16:20:19 +0200 Stefano Babic sbabic@denx.de wrote:
On 30/03/2012 16:02, Anatolij Gustschin wrote:
Since commit 5c1ad3e6f8ae578bbe30e09652f1531e9bc22031
Hi Antolji,
(net: fec_mxc: allow use with cache enabled) the FEC_MXC driver uses flush_dcache_range() and invalidate_dcache_range() functions. This driver is also configured for ARM1136 based 'flea3' and 'mx35pdk' boards which currently do not build as there are no ARM1136 specific flush_dcache_range() and
The issue is known - that is one reason why I marked the cache patches for the -next. I do not know if we can run enough tests before release.
Patches for M28 / MX5 / MX6 are not part of u-boot-imx, neither are yet merged into u-boot mainline by Wolfgang. On which tree have you seen that the patch was already merged ?
I pulled u-boot-arm.git master for build tests and see this change on the FEC driver in resulting tree.
However, Albert has sent a report http://www.mail-archive.com/u-boot@lists.denx.de/msg80566.html
a none of these boards was broken. But I see now that other boards are affected (the mx28evk does not compile due to missing CONFIG_APBH_DMA).
Albert, are these patches part of your pull-request to Wolfgang ?
Stefano
Dear Stefano Babic,
On 30/03/2012 16:35, Anatolij Gustschin wrote:
Hi Stefano,
On Fri, 30 Mar 2012 16:20:19 +0200
Stefano Babic sbabic@denx.de wrote:
On 30/03/2012 16:02, Anatolij Gustschin wrote:
Since commit 5c1ad3e6f8ae578bbe30e09652f1531e9bc22031
Hi Antolji,
(net: fec_mxc: allow use with cache enabled) the FEC_MXC driver uses flush_dcache_range() and invalidate_dcache_range() functions. This driver is also configured for ARM1136 based 'flea3' and 'mx35pdk' boards which currently do not build as there are no ARM1136 specific flush_dcache_range() and
The issue is known - that is one reason why I marked the cache patches for the -next. I do not know if we can run enough tests before release.
Patches for M28 / MX5 / MX6 are not part of u-boot-imx, neither are yet merged into u-boot mainline by Wolfgang. On which tree have you seen that the patch was already merged ?
I pulled u-boot-arm.git master for build tests and see this change on the FEC driver in resulting tree.
However, Albert has sent a report http://www.mail-archive.com/u-boot@lists.denx.de/msg80566.html
a none of these boards was broken. But I see now that other boards are affected (the mx28evk does not compile due to missing CONFIG_APBH_DMA).
Fabio, can you fix please? This is trivial.
Albert, are these patches part of your pull-request to Wolfgang ?
I believe the pullRQ isn't cooked yet. The fix for this issue right now would be to merge a patch that implements blank dcache-management functions for arm1136 -- like is in AG's patch. So I'm all for merging AG's patch into AA's tree.
It's a good thing this stirred a wave of response including patches. We now know very well which boards are maintained ;-)
Also, once any such breaking patch lands into mainline, we'll know in _less_than_24_hours_ that something got broken. (this is handled by DENX CI machine).
Finally, we can't really run physical (HW) tests indeed, but did we ever run physical tests with each and every patch? (and to conclude this -- these patches were tested on M28 and MX6Q-board)
Stefano
Best regards, Marek Vasut
Hi,
On Fri, 30 Mar 2012 17:28:03 +0200 Marek Vasut marex@denx.de wrote: ...
I pulled u-boot-arm.git master for build tests and see this change on the FEC driver in resulting tree.
However, Albert has sent a report http://www.mail-archive.com/u-boot@lists.denx.de/msg80566.html
a none of these boards was broken. But I see now that other boards are affected (the mx28evk does not compile due to missing CONFIG_APBH_DMA).
Fabio, can you fix please? This is trivial.
I've a patch to fix mx28evk build already. Will send it shortly.
Thanks,
Anatolij
On 30/03/2012 17:28, Marek Vasut wrote:
However, Albert has sent a report http://www.mail-archive.com/u-boot@lists.denx.de/msg80566.html
a none of these boards was broken. But I see now that other boards are affected (the mx28evk does not compile due to missing CONFIG_APBH_DMA).
Fabio, can you fix please? This is trivial.
Albert, are these patches part of your pull-request to Wolfgang ?
I believe the pullRQ isn't cooked yet. The fix for this issue right now would be to merge a patch that implements blank dcache-management functions for arm1136 -- like is in AG's patch. So I'm all for merging AG's patch into AA's tree.
I am testing Anatolij's patch on mx35pdk.
TFTP from server 192.168.2.14; our IP address is 192.168.2.97 Filename 'mx35pdk/uImage'. Load address: 0x80800000 Loading: Misaligned cache operation [8fe726e8, 8fe72728]
However, data is correctly loaded. I will check mmc on the "flea" board.
It's a good thing this stirred a wave of response including patches. We now know very well which boards are maintained ;-)
Also, once any such breaking patch lands into mainline, we'll know in _less_than_24_hours_ that something got broken. (this is handled by DENX CI machine).
Well, this is a good thing - my worries are about that patches for imx were already merged, and
Finally, we can't really run physical (HW) tests indeed, but did we ever run physical tests with each and every patch?
Not every patch, but a patchset that can have influence on several SOCs, yes.
(and to conclude this -- these patches were tested on M28 and MX6Q-board)
mmmh...I suppose the following patches must be merged, too (I had merged into u-boot-next, really):
Author: Eric Nelson eric.nelson@boundarydevices.com Date: Sun Mar 4 11:47:37 2012 +0000
i.MX6: define CACHELINE_SIZE
and also even if not mandatory: commit 1b2150b0770d8d019a41993d8692e4a29bf70a9e Author: Eric Nelson eric.nelson@boundarydevices.com Date: Sun Mar 4 11:47:38 2012 +0000
i.MX6: implement enable_caches()
disabled by default until drivers are fixed
Signed-off-by: Eric Nelson eric.nelson@boundarydevices.com Acked-by: Marek Vasut marex@denx.de
Best regards, Stefano Babic
Dear Stefano Babic,
On 30/03/2012 17:28, Marek Vasut wrote:
However, Albert has sent a report
http://www.mail-archive.com/u-boot@lists.denx.de/msg80566.html
a none of these boards was broken. But I see now that other boards are affected (the mx28evk does not compile due to missing CONFIG_APBH_DMA).
Fabio, can you fix please? This is trivial.
Albert, are these patches part of your pull-request to Wolfgang ?
I believe the pullRQ isn't cooked yet. The fix for this issue right now would be to merge a patch that implements blank dcache-management functions for arm1136 -- like is in AG's patch. So I'm all for merging AG's patch into AA's tree.
I am testing Anatolij's patch on mx35pdk.
TFTP from server 192.168.2.14; our IP address is 192.168.2.97 Filename 'mx35pdk/uImage'. Load address: 0x80800000 Loading: Misaligned cache operation [8fe726e8, 8fe72728]
However, data is correctly loaded. I will check mmc on the "flea" board.
You get this from FEC? Will have to recheck, this is weird, will recheck once I get back to my m28.
It's a good thing this stirred a wave of response including patches. We now know very well which boards are maintained ;-)
Also, once any such breaking patch lands into mainline, we'll know in _less_than_24_hours_ that something got broken. (this is handled by DENX CI machine).
Well, this is a good thing - my worries are about that patches for imx were already merged, and
Finally, we can't really run physical (HW) tests indeed, but did we ever run physical tests with each and every patch?
Not every patch, but a patchset that can have influence on several SOCs, yes.
Agreed.
(and to conclude this -- these patches were tested on M28 and MX6Q-board)
mmmh...I suppose the following patches must be merged, too (I had merged into u-boot-next, really):
Author: Eric Nelson eric.nelson@boundarydevices.com Date: Sun Mar 4 11:47:37 2012 +0000
i.MX6: define CACHELINE_SIZEand also even if not mandatory: commit 1b2150b0770d8d019a41993d8692e4a29bf70a9e Author: Eric Nelson eric.nelson@boundarydevices.com Date: Sun Mar 4 11:47:38 2012 +0000
i.MX6: implement enable_caches() disabled by default until drivers are fixed Signed-off-by: Eric Nelson <eric.nelson@boundarydevices.com> Acked-by: Marek Vasut <marex@denx.de>
They weren't already? Hm.
Best regards, Stefano Babic
Best regards, Marek Vasut
On 30/03/2012 18:05, Marek Vasut wrote:
I am testing Anatolij's patch on mx35pdk.
TFTP from server 192.168.2.14; our IP address is 192.168.2.97 Filename 'mx35pdk/uImage'. Load address: 0x80800000 Loading: Misaligned cache operation [8fe726e8, 8fe72728]
However, data is correctly loaded. I will check mmc on the "flea" board.
You get this from FEC?
Yes, I must set explicitely ethact=FEC, because on the mx35pdk ethprime is set to SMC911.
Will have to recheck, this is weird, will recheck once I get back to my m28.
I will check also on the flea3 board later.
It's a good thing this stirred a wave of response including patches. We now know very well which boards are maintained ;-)
Also, once any such breaking patch lands into mainline, we'll know in _less_than_24_hours_ that something got broken. (this is handled by DENX CI machine).
Well, this is a good thing - my worries are about that patches for imx were already merged, and
Finally, we can't really run physical (HW) tests indeed, but did we ever run physical tests with each and every patch?
Not every patch, but a patchset that can have influence on several SOCs, yes.
Agreed.
(and to conclude this -- these patches were tested on M28 and MX6Q-board)
mmmh...I suppose the following patches must be merged, too (I had merged into u-boot-next, really):
Author: Eric Nelson eric.nelson@boundarydevices.com Date: Sun Mar 4 11:47:37 2012 +0000
i.MX6: define CACHELINE_SIZEand also even if not mandatory: commit 1b2150b0770d8d019a41993d8692e4a29bf70a9e Author: Eric Nelson eric.nelson@boundarydevices.com Date: Sun Mar 4 11:47:38 2012 +0000
i.MX6: implement enable_caches() disabled by default until drivers are fixed Signed-off-by: Eric Nelson <eric.nelson@boundarydevices.com> Acked-by: Marek Vasut <marex@denx.de>They weren't already? Hm.
They are not - I thought to push all patches together, and for this reason they are on the -next branch. I merge both into -master, and they will part of my next pull.
Best regards, Stefano Babic
From: Anatolij Gustschin agust@denx.de
Since commit 5c1ad3e6f8ae578bbe30e09652f1531e9bc22031 (net: fec_mxc: allow use with cache enabled) the FEC_MXC driver uses flush_dcache_range() and invalidate_dcache_range() functions. This driver is also configured for ARM1136 based 'flea3' and 'mx35pdk' boards which currently do not build as there are no ARM1136 specific flush_dcache_range() and invalidate_dcache_range() functions. Add various ARM1136 cache functions to fix building for 'flea3' and 'mx35pdk'.
Signed-off-by: Anatolij Gustschin agust@denx.de Signed-off-by: Stefano Babic sbabic@denx.de Cc: Fabio Estevam fabio.estevam@freescale.com --- Changes since V1:
- use the same routine as in ARM926ejs to check range to easy detect misalignment (S. Babic) - cache are still disable - add enable_caches (S. Babic)
arch/arm/cpu/arm1136/cpu.c | 95 ++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 95 insertions(+), 0 deletions(-)
diff --git a/arch/arm/cpu/arm1136/cpu.c b/arch/arm/cpu/arm1136/cpu.c index 2b91631..8edcb59 100644 --- a/arch/arm/cpu/arm1136/cpu.c +++ b/arch/arm/cpu/arm1136/cpu.c @@ -75,3 +75,98 @@ static void cache_flush(void) asm ("mcr p15, 0, %0, c7, c7, 0": :"r" (i)); /* invalidate both caches and flush btb */ asm ("mcr p15, 0, %0, c7, c10, 4": :"r" (i)); /* mem barrier to sync things */ } + +#ifndef CONFIG_SYS_DCACHE_OFF + +#ifndef CONFIG_SYS_CACHELINE_SIZE +#define CONFIG_SYS_CACHELINE_SIZE 32 +#endif + +void invalidate_dcache_all(void) +{ + asm ("mcr p15, 0, %0, c7, c6, 0" : : "r" (0)); +} + +void flush_dcache_all(void) +{ + asm ("mcr p15, 0, %0, c7, c10, 0" : : "r" (0)); + asm ("mcr p15, 0, %0, c7, c10, 4" : : "r" (0)); +} + +static inline int bad_cache_range(unsigned long start, unsigned long stop) +{ + int ok = 1; + + if (start & (CONFIG_SYS_CACHELINE_SIZE - 1)) + ok = 0; + + if (stop & (CONFIG_SYS_CACHELINE_SIZE - 1)) + ok = 0; + + if (!ok) + printf("CACHE: Misaligned operation at range [%08lx, %08lx]\n", + start, stop); + + return ok; +} + +void invalidate_dcache_range(unsigned long start, unsigned long stop) +{ + if (bad_cache_range(start, stop)) + return; + + while (start < stop) { + asm ("mcr p15, 0, %0, c7, c6, 1" : : "r" (start)); + start += CONFIG_SYS_CACHELINE_SIZE; + } +} + +void flush_dcache_range(unsigned long start, unsigned long stop) +{ + if (bad_cache_range(start, stop)) + return; + + while (start < stop) { + asm ("mcr p15, 0, %0, c7, c14, 1" : : "r" (start)); + start += CONFIG_SYS_CACHELINE_SIZE; + } + + asm ("mcr p15, 0, %0, c7, c10, 4" : : "r" (0)); +} + +void flush_cache(unsigned long start, unsigned long size) +{ + flush_dcache_range(start, start + size); +} + +void enable_caches(void) +{ +#ifndef CONFIG_SYS_ICACHE_OFF + icache_enable(); +#endif +#ifndef CONFIG_SYS_DCACHE_OFF + dcache_enable(); +#endif +} + +#else /* #ifndef CONFIG_SYS_DCACHE_OFF */ +void invalidate_dcache_all(void) +{ +} + +void flush_dcache_all(void) +{ +} + +void invalidate_dcache_range(unsigned long start, unsigned long stop) +{ +} + +void flush_dcache_range(unsigned long start, unsigned long stop) +{ +} + +void flush_cache(unsigned long start, unsigned long size) +{ +} +#endif /* #ifndef CONFIG_SYS_DCACHE_OFF */
If the range passed to flush_cache is not multiple of ARCH_DMA_MINALIGN, a warning due to mislaignment is printed. Detected with fec_mxc, mx35 boards:
CACHE: Misaligned operation at range [80800000, 8083c310]
Signed-off-by: Stefano Babic sbabic@denx.de CC: Marek Vasut marex@denx.de CC: Joe Hershberger joe.hershberger@gmail.com Cc: Wolfgang Denk wd@denx.de --- common/cmd_net.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/common/cmd_net.c b/common/cmd_net.c index 65f32bc..a500919 100644 --- a/common/cmd_net.c +++ b/common/cmd_net.c @@ -256,7 +256,7 @@ static int netboot_common(enum proto_t proto, cmd_tbl_t *cmdtp, int argc, }
/* flush cache */ - flush_cache(load_addr, size); + flush_cache(load_addr, roundup(size, ARCH_DMA_MINALIGN));
bootstage_mark(BOOTSTAGE_ID_NET_LOADED);
Dear Stefano Babic,
If the range passed to flush_cache is not multiple of ARCH_DMA_MINALIGN, a warning due to mislaignment is printed. Detected with fec_mxc, mx35 boards:
CACHE: Misaligned operation at range [80800000, 8083c310]
Signed-off-by: Stefano Babic sbabic@denx.de CC: Marek Vasut marex@denx.de CC: Joe Hershberger joe.hershberger@gmail.com Cc: Wolfgang Denk wd@denx.de
common/cmd_net.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/common/cmd_net.c b/common/cmd_net.c index 65f32bc..a500919 100644 --- a/common/cmd_net.c +++ b/common/cmd_net.c @@ -256,7 +256,7 @@ static int netboot_common(enum proto_t proto, cmd_tbl_t *cmdtp, int argc, }
/* flush cache */
- flush_cache(load_addr, size);
- flush_cache(load_addr, roundup(size, ARCH_DMA_MINALIGN));
This ain't gonna slide. You might overwrite something, even though this is just loading into memory, right? I'm not quite sure how to handle this kind of unaligned access.
But adding at least if (unaligned) debug(...); to aid people easily finding these trouble would be nice ;-)
bootstage_mark(BOOTSTAGE_ID_NET_LOADED);
Best regards, Marek Vasut
On 01/04/2012 15:46, Marek Vasut wrote:
Dear Stefano Babic,
Hi Marek,
If the range passed to flush_cache is not multiple of ARCH_DMA_MINALIGN, a warning due to mislaignment is printed. Detected with fec_mxc, mx35 boards:
CACHE: Misaligned operation at range [80800000, 8083c310]
Signed-off-by: Stefano Babic sbabic@denx.de CC: Marek Vasut marex@denx.de CC: Joe Hershberger joe.hershberger@gmail.com Cc: Wolfgang Denk wd@denx.de
common/cmd_net.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/common/cmd_net.c b/common/cmd_net.c index 65f32bc..a500919 100644 --- a/common/cmd_net.c +++ b/common/cmd_net.c @@ -256,7 +256,7 @@ static int netboot_common(enum proto_t proto, cmd_tbl_t *cmdtp, int argc, }
/* flush cache */
- flush_cache(load_addr, size);
- flush_cache(load_addr, roundup(size, ARCH_DMA_MINALIGN));
This ain't gonna slide. You might overwrite something, even though this is just loading into memory, right?
Really we do not reserve space before a network transfer and we pass only the start address (load address) where the file is copied. It is not known its size before the transfer. So maybe it is not important up to further 31 bytes are flushed ;-)
Anyway, for my understanding: we are calling a function to flush caches. This means that if the size is something more as required there will be a cache hit misseing, and the SOC will load data again - but without corrupt data, right ?
I'm not quite sure how to handle this kind of unaligned access.
Well, my first goal was to explain which is the cause of the misalignment I found last friday testing Anatolij's patch, proofing that it is not due to last FEC patches ;-)
But adding at least if (unaligned) debug(...); to aid people easily finding these trouble would be nice ;-)
Not sure - where should be inserted or what do you exactly mean? size is the length of the transfered bytes, and can assueme any value, so it is often not aligned.
Best regards, Stefano Babic
Dear Stefano Babic,
On 01/04/2012 15:46, Marek Vasut wrote:
Dear Stefano Babic,
Hi Marek,
If the range passed to flush_cache is not multiple of ARCH_DMA_MINALIGN, a warning due to mislaignment is printed. Detected with fec_mxc, mx35 boards:
CACHE: Misaligned operation at range [80800000, 8083c310]
Signed-off-by: Stefano Babic sbabic@denx.de CC: Marek Vasut marex@denx.de CC: Joe Hershberger joe.hershberger@gmail.com Cc: Wolfgang Denk wd@denx.de
common/cmd_net.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/common/cmd_net.c b/common/cmd_net.c index 65f32bc..a500919 100644 --- a/common/cmd_net.c +++ b/common/cmd_net.c @@ -256,7 +256,7 @@ static int netboot_common(enum proto_t proto, cmd_tbl_t *cmdtp, int argc, }
/* flush cache */
- flush_cache(load_addr, size);
- flush_cache(load_addr, roundup(size, ARCH_DMA_MINALIGN));
This ain't gonna slide. You might overwrite something, even though this is just loading into memory, right?
Really we do not reserve space before a network transfer and we pass only the start address (load address) where the file is copied. It is not known its size before the transfer. So maybe it is not important up to further 31 bytes are flushed ;-)
Anyway, for my understanding: we are calling a function to flush caches. This means that if the size is something more as required there will be a cache hit misseing, and the SOC will load data again - but without corrupt data, right ?
Sure, but you can flush data into the RAM that should have never been flushed to the ram in the first place and that might confuse some controller -- or I might be overly paranoid.
I'm not quite sure how to handle this kind of unaligned access.
Well, my first goal was to explain which is the cause of the misalignment I found last friday testing Anatolij's patch, proofing that it is not due to last FEC patches ;-)
But adding at least if (unaligned) debug(...); to aid people easily finding these trouble would be nice ;-)
Not sure - where should be inserted or what do you exactly mean? size is the length of the transfered bytes, and can assueme any value, so it is often not aligned.
Well just before the cache-line aligning.
Best regards, Stefano Babic
Best regards, Marek Vasut
On Sunday 01 April 2012 09:22:59 Stefano Babic wrote:
If the range passed to flush_cache is not multiple of ARCH_DMA_MINALIGN, a warning due to mislaignment is printed. Detected with fec_mxc, mx35 boards:
CACHE: Misaligned operation at range [80800000, 8083c310]
warning on flushing is broken. the arch/arm/cpu/arm926ejs/cache.c code should probably be fixed instead. -mike
Dear Mike Frysinger,
On Sunday 01 April 2012 09:22:59 Stefano Babic wrote:
If the range passed to flush_cache is not multiple of ARCH_DMA_MINALIGN, a warning due to mislaignment is printed. Detected with fec_mxc, mx35 boards:
CACHE: Misaligned operation at range [80800000, 8083c310]
warning on flushing is broken. the arch/arm/cpu/arm926ejs/cache.c code should probably be fixed instead.
Why exactly?
-mike
Best regards, Marek Vasut
On Sunday 01 April 2012 17:00:56 Marek Vasut wrote:
Dear Mike Frysinger,
On Sunday 01 April 2012 09:22:59 Stefano Babic wrote:
If the range passed to flush_cache is not multiple of ARCH_DMA_MINALIGN, a warning due to mislaignment is printed. Detected with fec_mxc, mx35 boards:
CACHE: Misaligned operation at range [80800000, 8083c310]
warning on flushing is broken. the arch/arm/cpu/arm926ejs/cache.c code should probably be fixed instead.
Why exactly?
the flush isn't harmful (ignoring the fact that a few extra bytes might get written back to external memory), and the data isn't evicted from cache. after all, we aren't talking about invalidate here, we're talking about flush.
plus, no other arch (linux or u-boot) does this.
so the better question is, why exactly should you be warning ? you should provide justification when doing something unusual ... -mike
Dear Mike Frysinger,
On Sunday 01 April 2012 17:00:56 Marek Vasut wrote:
Dear Mike Frysinger,
On Sunday 01 April 2012 09:22:59 Stefano Babic wrote:
If the range passed to flush_cache is not multiple of ARCH_DMA_MINALIGN, a warning due to mislaignment is printed. Detected with fec_mxc, mx35 boards:
CACHE: Misaligned operation at range [80800000, 8083c310]
warning on flushing is broken. the arch/arm/cpu/arm926ejs/cache.c code should probably be fixed instead.
Why exactly?
the flush isn't harmful (ignoring the fact that a few extra bytes might get written back to external memory), and the data isn't evicted from cache. after all, we aren't talking about invalidate here, we're talking about flush.
Right ... and can you be sure nothing important is overwritten in RAM?
plus, no other arch (linux or u-boot) does this.
so the better question is, why exactly should you be warning ? you should provide justification when doing something unusual ...
Because you can destroy data in DRAM that arrived there by DMA transfer for example?
-mike
Best regards, Marek Vasut
On Sunday 01 April 2012 21:44:39 Marek Vasut wrote:
Dear Mike Frysinger,
On Sunday 01 April 2012 17:00:56 Marek Vasut wrote:
Dear Mike Frysinger,
On Sunday 01 April 2012 09:22:59 Stefano Babic wrote:
If the range passed to flush_cache is not multiple of ARCH_DMA_MINALIGN, a warning due to mislaignment is printed. Detected with fec_mxc, mx35 boards:
CACHE: Misaligned operation at range [80800000, 8083c310]
warning on flushing is broken. the arch/arm/cpu/arm926ejs/cache.c code should probably be fixed instead.
Why exactly?
the flush isn't harmful (ignoring the fact that a few extra bytes might get written back to external memory), and the data isn't evicted from cache. after all, we aren't talking about invalidate here, we're talking about flush.
Right ... and can you be sure nothing important is overwritten in RAM?
except i'd bet money you're already running dcache in writethrough mode, so the flush is largely irrelevant to this line of reasoning
plus, no other arch (linux or u-boot) does this.
so the better question is, why exactly should you be warning ? you should provide justification when doing something unusual ...
Because you can destroy data in DRAM that arrived there by DMA transfer for example?
that isn't the problem of the flush functions. there would have been an invalidate call at some point with misaligned addresses, iff it actually mattered. you could argue for invalidation triggering a warning, but that isn't what we're talking about. and still, linux doesn't trigger warnings, and i think only one other arm soc does atm in u-boot.
as already shown here, the flush call was perfectly fine, and adding roundup to that call site is a waste of code space to "fix" something that isn't a problem. -mike
Dear Mike Frysinger,
On Sunday 01 April 2012 21:44:39 Marek Vasut wrote:
Dear Mike Frysinger,
On Sunday 01 April 2012 17:00:56 Marek Vasut wrote:
Dear Mike Frysinger,
On Sunday 01 April 2012 09:22:59 Stefano Babic wrote:
If the range passed to flush_cache is not multiple of ARCH_DMA_MINALIGN, a warning due to mislaignment is printed. Detected with fec_mxc, mx35 boards:
CACHE: Misaligned operation at range [80800000, 8083c310]
warning on flushing is broken. the arch/arm/cpu/arm926ejs/cache.c code should probably be fixed instead.
Why exactly?
the flush isn't harmful (ignoring the fact that a few extra bytes might get written back to external memory), and the data isn't evicted from cache. after all, we aren't talking about invalidate here, we're talking about flush.
Right ... and can you be sure nothing important is overwritten in RAM?
except i'd bet money you're already running dcache in writethrough mode, so the flush is largely irrelevant to this line of reasoning
Sure, but you can write your data, then the DMA happens and then you flush your stuff again. This is when you're screwed (all right, it's 5:30am here, I'm not confident anymore).
plus, no other arch (linux or u-boot) does this.
so the better question is, why exactly should you be warning ? you should provide justification when doing something unusual ...
Because you can destroy data in DRAM that arrived there by DMA transfer for example?
that isn't the problem of the flush functions. there would have been an invalidate call at some point with misaligned addresses, iff it actually mattered. you could argue for invalidation triggering a warning, but that isn't what we're talking about. and still, linux doesn't trigger warnings, and i think only one other arm soc does atm in u-boot.
Ain't that because linux uses aligned buffers ?
as already shown here, the flush call was perfectly fine, and adding roundup to that call site is a waste of code space to "fix" something that isn't a problem.
I believe unaligned flush in uboot should always trigger a warning, not do alignment for the programmer. The programmer knows the best how the buffer looks (aka. if you embed the buffer in some structure, it might be problem).
-mike
Best regards, Marek Vasut
On Sunday 01 April 2012 23:34:28 Marek Vasut wrote:
Dear Mike Frysinger,
On Sunday 01 April 2012 21:44:39 Marek Vasut wrote:
Dear Mike Frysinger,
On Sunday 01 April 2012 17:00:56 Marek Vasut wrote:
Dear Mike Frysinger,
On Sunday 01 April 2012 09:22:59 Stefano Babic wrote: > If the range passed to flush_cache is not multiple > of ARCH_DMA_MINALIGN, a warning due to mislaignment > is printed. > Detected with fec_mxc, mx35 boards: > > CACHE: Misaligned operation at range [80800000, 8083c310]
warning on flushing is broken. the arch/arm/cpu/arm926ejs/cache.c code should probably be fixed instead.
Why exactly?
the flush isn't harmful (ignoring the fact that a few extra bytes might get written back to external memory), and the data isn't evicted from cache. after all, we aren't talking about invalidate here, we're talking about flush.
Right ... and can you be sure nothing important is overwritten in RAM?
except i'd bet money you're already running dcache in writethrough mode, so the flush is largely irrelevant to this line of reasoning
Sure, but you can write your data, then the DMA happens and then you flush your stuff again. This is when you're screwed (all right, it's 5:30am here, I'm not confident anymore).
that's not how cache/dma works. if you're going to dma into external memory, you *invalidate* it first, then dma it in. there is no flush before/after that. if you're going to dma from external memory, then you *flush* it, then dma it out. even then, the extra flushing is rarely a problem.
plus, no other arch (linux or u-boot) does this.
so the better question is, why exactly should you be warning ? you should provide justification when doing something unusual ...
Because you can destroy data in DRAM that arrived there by DMA transfer for example?
that isn't the problem of the flush functions. there would have been an invalidate call at some point with misaligned addresses, iff it actually mattered. you could argue for invalidation triggering a warning, but that isn't what we're talking about. and still, linux doesn't trigger warnings, and i think only one other arm soc does atm in u-boot.
Ain't that because linux uses aligned buffers ?
not always because it isn't always necessary. and even if they aren't aligned, linux doesn't check in the cache functions for alignment.
as already shown here, the flush call was perfectly fine, and adding roundup to that call site is a waste of code space to "fix" something that isn't a problem.
I believe unaligned flush in uboot should always trigger a warning, not do alignment for the programmer.
the hardware is going to do the alignment already. you can't flush explicit bytes, only cache lines. the API says that you must do the fix ups because you're required to operate on at least the range given to you. if the hardware means you end up doing a few bytes before or after, then so be it. if that behavior is incorrect in the calling code, then the calling code should be fixed, but *only* if it's actually a bad thing.
The programmer knows the best how the buffer looks (aka. if you embed the buffer in some structure, it might be problem).
this statement goes against your previous one. as the caller, the programmer *does* know best as to when the alignment is required, or flushing too much is OK. in this case, the proposed patch is useless overhead.
i don't know what problem you're trying to solve here, but it doesn't seem to be a real one. other arches have been running just fine in linux/u-boot -- Blackfin certainly has been running with i/d cache for as long as i've been working on it (which is probably over 5 years at this point). -mike
On 01/04/2012 21:23, Mike Frysinger wrote:
On Sunday 01 April 2012 09:22:59 Stefano Babic wrote:
If the range passed to flush_cache is not multiple of ARCH_DMA_MINALIGN, a warning due to mislaignment is printed. Detected with fec_mxc, mx35 boards:
CACHE: Misaligned operation at range [80800000, 8083c310]
warning on flushing is broken. the arch/arm/cpu/arm926ejs/cache.c code should probably be fixed instead.
mmhhh...I agree with you. Marek, really I am expecting that the same message appears on MX28, not only on arm1136. Only if the size of the downloaded file is a multiple of 32, the warning will not appear.
Anyway, the message is misleading. Do we need such kind of warning or can we simply silently ignore it ?
Stefano
Dear Stefano Babic,
On 01/04/2012 21:23, Mike Frysinger wrote:
On Sunday 01 April 2012 09:22:59 Stefano Babic wrote:
If the range passed to flush_cache is not multiple of ARCH_DMA_MINALIGN, a warning due to mislaignment is printed. Detected with fec_mxc, mx35 boards:
CACHE: Misaligned operation at range [80800000, 8083c310]
warning on flushing is broken. the arch/arm/cpu/arm926ejs/cache.c code should probably be fixed instead.
mmhhh...I agree with you. Marek, really I am expecting that the same message appears on MX28, not only on arm1136. Only if the size of the downloaded file is a multiple of 32, the warning will not appear.
Anyway, the message is misleading. Do we need such kind of warning or can we simply silently ignore it ?
Well, it was there to catch bugs in drivers. We can probably turn it into debug() now ?
Stefano
Best regards, Marek Vasut
On 02/04/2012 16:03, Marek Vasut wrote:
Well, it was there to catch bugs in drivers. We can probably turn it into debug() now ?
Agree, I set it into next version.
Best regards, Stefano Babic
Signed-off-by: Stefano Babic sbabic@denx.de --- include/configs/flea3.h | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/include/configs/flea3.h b/include/configs/flea3.h index 649e272..f046a58 100644 --- a/include/configs/flea3.h +++ b/include/configs/flea3.h @@ -34,6 +34,7 @@ #define CONFIG_MX35_HCLK_FREQ 24000000
#define CONFIG_SYS_DCACHE_OFF +#define CONFIG_SYS_CACHELINE_SIZE 32
#define CONFIG_DISPLAY_CPUINFO
@@ -98,6 +99,7 @@ #define CONFIG_BOOTP_DNS
#define CONFIG_CMD_NAND +#define CONFIG_CMD_CACHE
#define CONFIG_CMD_I2C #define CONFIG_CMD_SPI
Signed-off-by: Stefano Babic sbabic@denx.de --- include/configs/mx35pdk.h | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/include/configs/mx35pdk.h b/include/configs/mx35pdk.h index 0c62b9f..1e03639 100644 --- a/include/configs/mx35pdk.h +++ b/include/configs/mx35pdk.h @@ -38,6 +38,7 @@
/* Set TEXT at the beginning of the NOR flash */ #define CONFIG_SYS_TEXT_BASE 0xA0000000 +#define CONFIG_SYS_CACHELINE_SIZE 32
#define CONFIG_SYS_64BIT_VSPRINTF
@@ -106,6 +107,7 @@ #define CONFIG_BOOTP_DNS
#define CONFIG_CMD_NAND +#define CONFIG_CMD_CACHE
#define CONFIG_CMD_I2C #define CONFIG_CMD_SPI
From: Anatolij Gustschin agust@denx.de
Since commit 5c1ad3e6f8ae578bbe30e09652f1531e9bc22031 (net: fec_mxc: allow use with cache enabled) the FEC_MXC driver uses flush_dcache_range() and invalidate_dcache_range() functions. This driver is also configured for ARM1136 based 'flea3' and 'mx35pdk' boards which currently do not build as there are no ARM1136 specific flush_dcache_range() and invalidate_dcache_range() functions. Add various ARM1136 cache functions to fix building for 'flea3' and 'mx35pdk'.
Signed-off-by: Anatolij Gustschin agust@denx.de Signed-off-by: Stefano Babic sbabic@denx.de Cc: Fabio Estevam fabio.estevam@freescale.com CC: Mike Frysinger vapier@gentoo.org CC: Marek Vasut marex@denx.de --- arch/arm/cpu/arm1136/cpu.c | 95 ++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 95 insertions(+), 0 deletions(-)
Changes since V2: - use debug instead of printf in case of misalignment (M. Frysinger, M. Vasut)
Changes since V1:
- use the same routine as in ARM926ejs to check range to easy detect misalignment (S. Babic) - cache are still disable - add enable_caches (S. Babic)
diff --git a/arch/arm/cpu/arm1136/cpu.c b/arch/arm/cpu/arm1136/cpu.c index 2b91631..f2e30b5 100644 --- a/arch/arm/cpu/arm1136/cpu.c +++ b/arch/arm/cpu/arm1136/cpu.c @@ -75,3 +75,98 @@ static void cache_flush(void) asm ("mcr p15, 0, %0, c7, c7, 0": :"r" (i)); /* invalidate both caches and flush btb */ asm ("mcr p15, 0, %0, c7, c10, 4": :"r" (i)); /* mem barrier to sync things */ } + +#ifndef CONFIG_SYS_DCACHE_OFF + +#ifndef CONFIG_SYS_CACHELINE_SIZE +#define CONFIG_SYS_CACHELINE_SIZE 32 +#endif + +void invalidate_dcache_all(void) +{ + asm ("mcr p15, 0, %0, c7, c6, 0" : : "r" (0)); +} + +void flush_dcache_all(void) +{ + asm ("mcr p15, 0, %0, c7, c10, 0" : : "r" (0)); + asm ("mcr p15, 0, %0, c7, c10, 4" : : "r" (0)); +} + +static inline int bad_cache_range(unsigned long start, unsigned long stop) +{ + int ok = 1; + + if (start & (CONFIG_SYS_CACHELINE_SIZE - 1)) + ok = 0; + + if (stop & (CONFIG_SYS_CACHELINE_SIZE - 1)) + ok = 0; + + if (!ok) + debug("CACHE: Misaligned operation at range [%08lx, %08lx]\n", + start, stop); + + return ok; +} + +void invalidate_dcache_range(unsigned long start, unsigned long stop) +{ + if (bad_cache_range(start, stop)) + return; + + while (start < stop) { + asm ("mcr p15, 0, %0, c7, c6, 1" : : "r" (start)); + start += CONFIG_SYS_CACHELINE_SIZE; + } +} + +void flush_dcache_range(unsigned long start, unsigned long stop) +{ + if (bad_cache_range(start, stop)) + return; + + while (start < stop) { + asm ("mcr p15, 0, %0, c7, c14, 1" : : "r" (start)); + start += CONFIG_SYS_CACHELINE_SIZE; + } + + asm ("mcr p15, 0, %0, c7, c10, 4" : : "r" (0)); +} + +void flush_cache(unsigned long start, unsigned long size) +{ + flush_dcache_range(start, start + size); +} + +void enable_caches(void) +{ +#ifndef CONFIG_SYS_ICACHE_OFF + icache_enable(); +#endif +#ifndef CONFIG_SYS_DCACHE_OFF + dcache_enable(); +#endif +} + +#else /* #ifndef CONFIG_SYS_DCACHE_OFF */ +void invalidate_dcache_all(void) +{ +} + +void flush_dcache_all(void) +{ +} + +void invalidate_dcache_range(unsigned long start, unsigned long stop) +{ +} + +void flush_dcache_range(unsigned long start, unsigned long stop) +{ +} + +void flush_cache(unsigned long start, unsigned long size) +{ +} +#endif /* #ifndef CONFIG_SYS_DCACHE_OFF */
Dear Stefano Babic,
From: Anatolij Gustschin agust@denx.de
Since commit 5c1ad3e6f8ae578bbe30e09652f1531e9bc22031 (net: fec_mxc: allow use with cache enabled) the FEC_MXC driver uses flush_dcache_range() and invalidate_dcache_range() functions. This driver is also configured for ARM1136 based 'flea3' and 'mx35pdk' boards which currently do not build as there are no ARM1136 specific flush_dcache_range() and invalidate_dcache_range() functions. Add various ARM1136 cache functions to fix building for 'flea3' and 'mx35pdk'.
Signed-off-by: Anatolij Gustschin agust@denx.de Signed-off-by: Stefano Babic sbabic@denx.de Cc: Fabio Estevam fabio.estevam@freescale.com CC: Mike Frysinger vapier@gentoo.org CC: Marek Vasut marex@denx.de
arch/arm/cpu/arm1136/cpu.c | 95 ++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 95 insertions(+), 0 deletions(-)
Changes since V2:
- use debug instead of printf in case of misalignment (M. Frysinger, M.
Vasut)
Changes since V1:
- use the same routine as in ARM926ejs to check range to easy detect
misalignment (S. Babic) - cache are still disable - add enable_caches (S. Babic)
Acked-by: Marek Vasut marex@denx.de
diff --git a/arch/arm/cpu/arm1136/cpu.c b/arch/arm/cpu/arm1136/cpu.c index 2b91631..f2e30b5 100644 --- a/arch/arm/cpu/arm1136/cpu.c +++ b/arch/arm/cpu/arm1136/cpu.c @@ -75,3 +75,98 @@ static void cache_flush(void) asm ("mcr p15, 0, %0, c7, c7, 0": :"r" (i)); /* invalidate both caches and flush btb */ asm ("mcr p15, 0, %0, c7, c10, 4": :"r" (i)); /* mem barrier to sync things */ }
+#ifndef CONFIG_SYS_DCACHE_OFF
+#ifndef CONFIG_SYS_CACHELINE_SIZE +#define CONFIG_SYS_CACHELINE_SIZE 32 +#endif
+void invalidate_dcache_all(void) +{
- asm ("mcr p15, 0, %0, c7, c6, 0" : : "r" (0));
+}
+void flush_dcache_all(void) +{
- asm ("mcr p15, 0, %0, c7, c10, 0" : : "r" (0));
- asm ("mcr p15, 0, %0, c7, c10, 4" : : "r" (0));
+}
+static inline int bad_cache_range(unsigned long start, unsigned long stop) +{
- int ok = 1;
- if (start & (CONFIG_SYS_CACHELINE_SIZE - 1))
ok = 0;- if (stop & (CONFIG_SYS_CACHELINE_SIZE - 1))
ok = 0;- if (!ok)
debug("CACHE: Misaligned operation at range [%08lx, %08lx]\n",start, stop);- return ok;
+}
+void invalidate_dcache_range(unsigned long start, unsigned long stop) +{
- if (bad_cache_range(start, stop))
return;- while (start < stop) {
asm ("mcr p15, 0, %0, c7, c6, 1" : : "r" (start));start += CONFIG_SYS_CACHELINE_SIZE;- }
+}
+void flush_dcache_range(unsigned long start, unsigned long stop) +{
- if (bad_cache_range(start, stop))
return;- while (start < stop) {
asm ("mcr p15, 0, %0, c7, c14, 1" : : "r" (start));start += CONFIG_SYS_CACHELINE_SIZE;- }
- asm ("mcr p15, 0, %0, c7, c10, 4" : : "r" (0));
+}
+void flush_cache(unsigned long start, unsigned long size) +{
- flush_dcache_range(start, start + size);
+}
+void enable_caches(void) +{ +#ifndef CONFIG_SYS_ICACHE_OFF
- icache_enable();
+#endif +#ifndef CONFIG_SYS_DCACHE_OFF
- dcache_enable();
+#endif +}
+#else /* #ifndef CONFIG_SYS_DCACHE_OFF */ +void invalidate_dcache_all(void) +{ +}
+void flush_dcache_all(void) +{ +}
+void invalidate_dcache_range(unsigned long start, unsigned long stop) +{ +}
+void flush_dcache_range(unsigned long start, unsigned long stop) +{ +}
+void flush_cache(unsigned long start, unsigned long size) +{ +} +#endif /* #ifndef CONFIG_SYS_DCACHE_OFF */
On 02/04/2012 18:18, Stefano Babic wrote:
From: Anatolij Gustschin agust@denx.de
Since commit 5c1ad3e6f8ae578bbe30e09652f1531e9bc22031 (net: fec_mxc: allow use with cache enabled) the FEC_MXC driver uses flush_dcache_range() and invalidate_dcache_range() functions. This driver is also configured for ARM1136 based 'flea3' and 'mx35pdk' boards which currently do not build as there are no ARM1136 specific flush_dcache_range() and invalidate_dcache_range() functions. Add various ARM1136 cache functions to fix building for 'flea3' and 'mx35pdk'.
Signed-off-by: Anatolij Gustschin agust@denx.de Signed-off-by: Stefano Babic sbabic@denx.de Cc: Fabio Estevam fabio.estevam@freescale.com CC: Mike Frysinger vapier@gentoo.org CC: Marek Vasut marex@denx.de
Applied to u-boot-imx (fix), thanks.
Best regards, Stefano Babic
Misaligned warnings are useful to debug faulty drivers. A misaligned warning is printed also when the driver is correct - use debug() instead of printf().
Signed-off-by: Stefano Babic sbabic@denx.de CC: Albert Aribaud albert.u.boot@aribaud.net CC: Mike Frysinger vapier@gentoo.org CC: Marek Vasut marex@denx.de ---
Changes since V1:
This substitutes [PATCH 2/4] net: round up before calling flush_cache - change warning in cache.c instead of fixing cmd_net.c (M. Frysinger, M. Vasut)
arch/arm/cpu/arm926ejs/cache.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/arch/arm/cpu/arm926ejs/cache.c b/arch/arm/cpu/arm926ejs/cache.c index 5b23e3a..4430578 100644 --- a/arch/arm/cpu/arm926ejs/cache.c +++ b/arch/arm/cpu/arm926ejs/cache.c @@ -55,7 +55,7 @@ static int check_cache_range(unsigned long start, unsigned long stop) ok = 0;
if (!ok) - printf("CACHE: Misaligned operation at range [%08lx, %08lx]\n", + debug("CACHE: Misaligned operation at range [%08lx, %08lx]\n", start, stop);
return ok;
Dear Stefano Babic,
Misaligned warnings are useful to debug faulty drivers. A misaligned warning is printed also when the driver is correct - use debug() instead of printf().
Signed-off-by: Stefano Babic sbabic@denx.de CC: Albert Aribaud albert.u.boot@aribaud.net CC: Mike Frysinger vapier@gentoo.org CC: Marek Vasut marex@denx.de
Acked-by: Marek Vasut marex@denx.de
Changes since V1:
This substitutes [PATCH 2/4] net: round up before calling flush_cache
- change warning in cache.c instead of fixing cmd_net.c (M. Frysinger, M. Vasut)
arch/arm/cpu/arm926ejs/cache.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/arch/arm/cpu/arm926ejs/cache.c b/arch/arm/cpu/arm926ejs/cache.c index 5b23e3a..4430578 100644 --- a/arch/arm/cpu/arm926ejs/cache.c +++ b/arch/arm/cpu/arm926ejs/cache.c @@ -55,7 +55,7 @@ static int check_cache_range(unsigned long start, unsigned long stop) ok = 0;
if (!ok)
printf("CACHE: Misaligned operation at range [%08lx, %08lx]\n",
debug("CACHE: Misaligned operation at range [%08lx, %08lx]\n", start, stop);return ok;
Best regards, Marek Vasut
On Mon, Apr 2, 2012 at 12:18, Stefano Babic wrote:
Misaligned warnings are useful to debug faulty drivers. A misaligned warning is printed also when the driver is correct - use debug() instead of printf().
unfortunately, this turns the failure into a silent one. if i read the code correctly, you still return an error in this code path which means things don't actually get flushed/invalidated.
to the original concept, i have no problem with cache funcs all warning on misalignment via debug() so developers can quickly see if things need to be reviewed. -mike
Dear Mike Frysinger,
On Mon, Apr 2, 2012 at 12:18, Stefano Babic wrote:
Misaligned warnings are useful to debug faulty drivers. A misaligned warning is printed also when the driver is correct - use debug() instead of printf().
unfortunately, this turns the failure into a silent one. if i read the code correctly, you still return an error in this code path which means things don't actually get flushed/invalidated.
You certainly do return an error, yes. And you don't do the op ... but then, what's the whole point of this check?
You can just ignore the return value in the flush() op, but I still don't like how this is ignored.
to the original concept, i have no problem with cache funcs all warning on misalignment via debug() so developers can quickly see if things need to be reviewed. -mike
Best regards, Marek Vasut
On Mon, Apr 2, 2012 at 14:42, Marek Vasut wrote:
On Mon, Apr 2, 2012 at 12:18, Stefano Babic wrote:
Misaligned warnings are useful to debug faulty drivers. A misaligned warning is printed also when the driver is correct - use debug() instead of printf().
unfortunately, this turns the failure into a silent one. if i read the code correctly, you still return an error in this code path which means things don't actually get flushed/invalidated.
You certainly do return an error, yes. And you don't do the op ...
You can just ignore the return value in the flush() op, but I still don't like how this is ignored.
this is all internal to this soc's cache code. the common API provides no support for returning an "error" because there is no such thing with these funcs. you do the operation on the region requested regardless of implicit side-effects.
but then, what's the whole point of this check?
my point all along :p -mike
participants (5)
-
Anatolij Gustschin -
Marek Vasut -
Marek Vasut -
Mike Frysinger -
Stefano Babic