[U-Boot] [PATCH] ARM1136: add cache flush and invalidate operations

[U-Boot] [PATCH] ARM1136: add cache flush and invalidate operations

[Anatolij Gustschin]

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 */
-- 
1.7.7.6

[U-Boot] [PATCH] ARM1136: add cache flush and invalidate operations

[Stefano Babic]

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

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

[U-Boot] [PATCH] ARM1136: add cache flush and invalidate operations

[Anatolij Gustschin]

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

[U-Boot] [PATCH] ARM1136: add cache flush and invalidate operations

[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 at 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

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

[U-Boot] [PATCH] ARM1136: add cache flush and invalidate operations

[Marek Vasut]

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 at 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

[U-Boot] [PATCH] ARM1136: add cache flush and invalidate operations

[Anatolij Gustschin]

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 at 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

[U-Boot] [PATCH] ARM1136: add cache flush and invalidate operations

[Stefano Babic]

On 30/03/2012 17:28, Marek Vasut wrote:

>>
>> However, Albert has sent a report
>>  http://www.mail-archive.com/u-boot at 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

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

[U-Boot] [PATCH] ARM1136: add cache flush and invalidate operations

[Marek Vasut]

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 at 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_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>

They weren't already? Hm.

> 
> Best regards,
> Stefano Babic

Best regards,
Marek Vasut

[U-Boot] [PATCH] ARM1136: add cache flush and invalidate operations

[Stefano Babic]

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_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>
> 
> 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

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

[U-Boot] [PATCH 1/4] ARM1136: add cache flush and invalidate operations

[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 */
-- 
1.7.5.4

[U-Boot] [PATCH 2/4] net: round up before calling flush_cache

[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));
 
 	bootstage_mark(BOOTSTAGE_ID_NET_LOADED);
 
-- 
1.7.5.4

[U-Boot] [PATCH 3/4] mx35: flea3: fix when cache functions are linked

[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
-- 
1.7.5.4

[U-Boot] [PATCH 4/4] mx35: mx35pdk: fix when cache functions are linked

[Stefano Babic]

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
-- 
1.7.5.4

[U-Boot] [PATCH 2/4] net: round up before calling flush_cache

[Marek Vasut]

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

[U-Boot] [PATCH 2/4] net: round up before calling flush_cache

[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 ?

> 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

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

[U-Boot] [PATCH 2/4] net: round up before calling flush_cache

[Marek Vasut]

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

[U-Boot] [PATCH 2/4] net: round up before calling flush_cache

[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.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120401/59e51ba6/attachment.pgp>

[U-Boot] [PATCH 2/4] net: round up before calling flush_cache

[Marek Vasut]

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

[U-Boot] [PATCH 2/4] net: round up before calling flush_cache

[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.

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120401/0d7e9214/attachment.pgp>

[U-Boot] [PATCH 2/4] net: round up before calling flush_cache

[Marek Vasut]

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

[U-Boot] [PATCH 2/4] net: round up before calling flush_cache

[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

> > 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120401/b5d4e2ac/attachment.pgp>

[U-Boot] [PATCH 2/4] net: round up before calling flush_cache

[Marek Vasut]

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

[U-Boot] [PATCH 2/4] net: round up before calling flush_cache

[Mike Frysinger]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120402/b441f677/attachment.pgp>

[U-Boot] [PATCH 2/4] net: round up before calling flush_cache

[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 ?

Stefano

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

[U-Boot] [PATCH 2/4] net: round up before calling flush_cache

[Marek Vasut]

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

[U-Boot] [PATCH 2/4] net: round up before calling flush_cache

[Stefano Babic]

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


-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

[U-Boot] [PATCH V3 1/4] ARM1136: add cache flush and invalidate operations

[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)

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 */
-- 
1.7.5.4

[U-Boot] [PATCH V2 2/4] ARM: 926ejs: use debug() for misaligned addresses

[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;
-- 
1.7.5.4

[U-Boot] [PATCH V3 1/4] ARM1136: add cache flush and invalidate operations

[Marek Vasut]

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 */

[U-Boot] [PATCH V2 2/4] ARM: 926ejs: use debug() for misaligned addresses

[Marek Vasut]

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

[U-Boot] [PATCH V3 1/4] ARM1136: add cache flush and invalidate operations

[Stefano Babic]

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

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

[U-Boot] [PATCH V2 2/4] ARM: 926ejs: use debug() for misaligned addresses

[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.

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

[U-Boot] [PATCH V2 2/4] ARM: 926ejs: use debug() for misaligned addresses

[Marek Vasut]

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

[U-Boot] [PATCH V2 2/4] ARM: 926ejs: use debug() for misaligned addresses

[Mike Frysinger]

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