[U-Boot] [PATCH] The cache flush using coprocessor must be in lib_arm/cache-cp15.c

[U-Boot] [PATCH] The cache flush using coprocessor must be in lib_arm/cache-cp15.c

[Kim, Heung Jun]

Hi? I'm about to suggest one more thing related to "change cpu.c under
cpu/arm_cortexa8 dir to common code.".

asm ("mcr p15, 0, %0, c7, c5, 0": :"r" (0));

It's the common feature all over the arm core, not only arm cortex A8.
The common cache function is defined in the lib_arm/cache-cp15.c as you
know. So, It's seems the better method that cache_flush() is moved to
lib_arm/cache-cp15.c

Signed-off-by: HeungJun, Kim <riverful.kim@samsung.com>
---
 cpu/arm_cortexa8/cpu.c |    4 ----
 lib_arm/cache-cp15.c   |    5 +++++
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/cpu/arm_cortexa8/cpu.c b/cpu/arm_cortexa8/cpu.c
index 3e1780b..329febb 100644
--- a/cpu/arm_cortexa8/cpu.c
+++ b/cpu/arm_cortexa8/cpu.c
@@ -154,7 +154,3 @@ void l2cache_disable()
 	}
 }

-static void cache_flush(void)
-{
-	asm ("mcr p15, 0, %0, c7, c5, 0": :"r" (0));
-}
diff --git a/lib_arm/cache-cp15.c b/lib_arm/cache-cp15.c
index 62ed54f..face9b2 100644
--- a/lib_arm/cache-cp15.c
+++ b/lib_arm/cache-cp15.c
@@ -53,6 +53,11 @@ static void cache_disable(uint32_t cache_bit)
 	cp_delay();
 	set_cr(reg & ~cache_bit);
 }
+
+static void cache_flush(void)
+{
+	asm ("mcr p15, 0, %0, c7, c5, 0": :"r" (0));
+}
 #endif

 #ifdef CONFIG_SYS_NO_ICACHE
-- 
1.5.6.3

[U-Boot] [PATCH] The cache flush using coprocessor must be in lib_arm/cache-cp15.c

[Dirk Behme]

Kim, Heung Jun wrote:
> Hi? I'm about to suggest one more thing related to "change cpu.c under
> cpu/arm_cortexa8 dir to common code.".
> 
> asm ("mcr p15, 0, %0, c7, c5, 0": :"r" (0));
> 
> It's the common feature all over the arm core, not only arm cortex A8.
> The common cache function is defined in the lib_arm/cache-cp15.c as you
> know. So, It's seems the better method that cache_flush() is moved to
> lib_arm/cache-cp15.c
> 
> Signed-off-by: HeungJun, Kim <riverful.kim@samsung.com>

Yes, this is fine. Thanks.

Do you like to update

http://lists.denx.de/pipermail/u-boot/2009-May/053386.html

regarding this and rename the functions there to e.g. 
cortexa8_l2cache_enable()/disable() (or soc_* or cpu_*)?

Best regards

Dirk

> ---
>  cpu/arm_cortexa8/cpu.c |    4 ----
>  lib_arm/cache-cp15.c   |    5 +++++
>  2 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/cpu/arm_cortexa8/cpu.c b/cpu/arm_cortexa8/cpu.c
> index 3e1780b..329febb 100644
> --- a/cpu/arm_cortexa8/cpu.c
> +++ b/cpu/arm_cortexa8/cpu.c
> @@ -154,7 +154,3 @@ void l2cache_disable()
>  	}
>  }
> 
> -static void cache_flush(void)
> -{
> -	asm ("mcr p15, 0, %0, c7, c5, 0": :"r" (0));
> -}
> diff --git a/lib_arm/cache-cp15.c b/lib_arm/cache-cp15.c
> index 62ed54f..face9b2 100644
> --- a/lib_arm/cache-cp15.c
> +++ b/lib_arm/cache-cp15.c
> @@ -53,6 +53,11 @@ static void cache_disable(uint32_t cache_bit)
>  	cp_delay();
>  	set_cr(reg & ~cache_bit);
>  }
> +
> +static void cache_flush(void)
> +{
> +	asm ("mcr p15, 0, %0, c7, c5, 0": :"r" (0));
> +}
>  #endif
> 
>  #ifdef CONFIG_SYS_NO_ICACHE

[U-Boot] [PATCH] The cache flush using coprocessor must be in lib_arm/cache-cp15.c

[Jean-Christophe PLAGNIOL-VILLARD]

On 17:51 Thu 28 May     , Kim, Heung Jun wrote:
> Hi? I'm about to suggest one more thing related to "change cpu.c under
> cpu/arm_cortexa8 dir to common code.".
> 
> asm ("mcr p15, 0, %0, c7, c5, 0": :"r" (0));
> 
> It's the common feature all over the arm core, not only arm cortex A8.
> The common cache function is defined in the lib_arm/cache-cp15.c as you
> know. So, It's seems the better method that cache_flush() is moved to
> lib_arm/cache-cp15.c
NACK

to boot linux we are suppose to clean both cache I and DCache

the I/DCache flush is armv and arch dependant (example ARM7TDMI, armv4wt, armv3,
armv7, etc...)

so for now please let it in cpu/$(ARCH)/cpu.c

Best Regards,
J.

[U-Boot] [PATCH] The cache flush using coprocessor must be in lib_arm/cache-cp15.c

[Dirk Behme]

Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 17:51 Thu 28 May     , Kim, Heung Jun wrote:
>> Hi? I'm about to suggest one more thing related to "change cpu.c under
>> cpu/arm_cortexa8 dir to common code.".
>>
>> asm ("mcr p15, 0, %0, c7, c5, 0": :"r" (0));
>>
>> It's the common feature all over the arm core, not only arm cortex A8.
>> The common cache function is defined in the lib_arm/cache-cp15.c as you
>> know. So, It's seems the better method that cache_flush() is moved to
>> lib_arm/cache-cp15.c
> NACK
> 
> to boot linux we are suppose to clean both cache I and DCache
> 
> the I/DCache flush is armv and arch dependant (example ARM7TDMI, armv4wt, armv3,
> armv7, etc...)
> 
> so for now please let it in cpu/$(ARCH)/cpu.c

Well, in

http://lists.denx.de/pipermail/u-boot/2009-May/053396.html

Kim mentioned that he shuffles around the code due to compile errors 
while adding his new code. So we have to see if we can stay with the 
current implementation or have to change something.

Kim: What compile errors do you get for cache_flush()? Could you check 
if you really don't need it? Why don't you get compile errors if you 
move it from cpu/arm_cortexa8/cpu.c to lib_arm/cache-cp15.c?

Best regards

Dirk

[U-Boot] [PATCH] The cache flush using coprocessor must be in lib_arm/cache-cp15.c

[Kim, Heung Jun]

Thanks to point. Dirk.

And call me "riverful" next time, plz :)

> Kim mentioned that he shuffles around the code due to compile errors while
> adding his new code. So we have to see if we can stay with the current
> implementation or have to change something.
Yes. I shuffles around a little now.

> Kim: What compile errors do you get for cache_flush()? Could you check if
> you really don't need it? Why don't you get compile errors if you move it
> from cpu/arm_cortexa8/cpu.c to lib_arm/cache-cp15.c?
The compile errors are two.

1. First error occured cause of omap3 dependent definition CPU_3XX_ES20.

cpu.c:99: warning: function declaration isn't a prototype
cpu.c: In function 'l2cache_enable':
cpu.c:104: warning: implicit declaration of function 'get_cpu_rev'
cpu.c:104: error: 'CPU_3XX_ES20' undeclared (first use in this function)
cpu.c:104: error: (Each undeclared identifier is reported only once
cpu.c:104: error: for each function it appears in.)
cpu.c: At top level:
cpu.c:129: warning: function declaration isn't a prototype
cpu.c: In function 'l2cache_disable':
cpu.c:134: error: 'CPU_3XX_ES20' undeclared (first use in this function)
make[1]: *** [cpu.o] Error 1


2. After upper error was resolved, linking error occured.
The definition PRM_RSTCTRL is also omap3 dependent.
It is used in the cpu/arm_cortexa8/start.S, and defined
in the include/asm-arm/arch-omap3/cpu.h

/arm_cortexa8/s5pc100/libs5pc100.a lib_arm/libarm.a
fs/cramfs/libcramfs.a fs/fat/libfat.a fs/fdos/libfdos.a
fs/jffs2/libjffs2.a fs/reiserfs/libreiserfs.a fs/ext2/libext2fs.a
fs/yaffs2/libyaffs2.a fs/ubifs/libubifs.a net/libnet.a disk/libdisk.a
drivers/bios_emulator/libatibiosemu.a drivers/block/libblock.a
drivers/dma/libdma.a drivers/fpga/libfpga.a drivers/gpio/libgpio.a
drivers/hwmon/libhwmon.a drivers/i2c/libi2c.a drivers/input/libinput.a
drivers/misc/libmisc.a drivers/mmc/libmmc.a drivers/mtd/libmtd.a
drivers/mtd/nand/libnand.a drivers/mtd/nand_legacy/libnand_legacy.a
drivers/mtd/onenand/libonenand.a drivers/mtd/ubi/libubi.a
drivers/mtd/spi/libspi_flash.a drivers/net/libnet.a
drivers/net/phy/libphy.a drivers/net/sk98lin/libsk98lin.a
drivers/pci/libpci.a drivers/pcmcia/libpcmcia.a drivers/spi/libspi.a
drivers/rtc/librtc.a drivers/serial/libserial.a
drivers/twserial/libtws.a drivers/usb/gadget/libusb_gadget.a
drivers/usb/host/libusb_host.a drivers/usb/musb/libusb_musb.a
drivers/video/libvideo.a drivers/watchdog/libwatchdog.a
common/libcommon.a libfdt/libfdt.a api/libapi.a post/libpost.a
board/samsung/tickertape/libtickertape.a --end-group -L
/opt/toolchains/arm-2007q3/bin/../lib/gcc/arm-none-linux-gnueabi/4.2.1
-lgcc \
			-Map u-boot.map -o u-boot
cpu/arm_cortexa8/start.o: In function `rstctl':
/ubuntu/archive/s5pc100/u-boot-arm/cpu/arm_cortexa8/start.S:514:
undefined reference to `PRM_RSTCTRL'

Thanks one more.

Best regards,

HeungJun, Kim(riverful)

[U-Boot] [PATCH] The cache flush using coprocessor must be in lib_arm/cache-cp15.c

[Dirk Behme]

Hi riverful,

Kim, Heung Jun wrote:
> Thanks to point. Dirk.
> 
> And call me "riverful" next time, plz :)
> 
>> Kim mentioned that he shuffles around the code due to compile errors while
>> adding his new code. So we have to see if we can stay with the current
>> implementation or have to change something.
> Yes. I shuffles around a little now.
> 
>> Kim: What compile errors do you get for cache_flush()? Could you check if
>> you really don't need it? Why don't you get compile errors if you move it
>> from cpu/arm_cortexa8/cpu.c to lib_arm/cache-cp15.c?
> The compile errors are two.
> 
> 1. First error occured cause of omap3 dependent definition CPU_3XX_ES20.
> 
> cpu.c:99: warning: function declaration isn't a prototype
> cpu.c: In function 'l2cache_enable':
> cpu.c:104: warning: implicit declaration of function 'get_cpu_rev'
> cpu.c:104: error: 'CPU_3XX_ES20' undeclared (first use in this function)
> cpu.c:104: error: (Each undeclared identifier is reported only once
> cpu.c:104: error: for each function it appears in.)
> cpu.c: At top level:
> cpu.c:129: warning: function declaration isn't a prototype
> cpu.c: In function 'l2cache_disable':
> cpu.c:134: error: 'CPU_3XX_ES20' undeclared (first use in this function)
> make[1]: *** [cpu.o] Error 1

Ok, this looks to me that we have to move l2cache_enable() and 
l2cache_disable() to cpu/arm_cortexa8/omap3/board.c and rename them to 
cortexa8_l2cache_enable()/disable() as asked in

http://lists.denx.de/pipermail/u-boot/2009-May/053405.html

But it doesn't seem that we have to touch cache_flush()? Then it can 
stay in cpu/arm_cortexa8/cpu.c as requested by Jean-Christophe?

> 2. After upper error was resolved, linking error occured.
> The definition PRM_RSTCTRL is also omap3 dependent.
> It is used in the cpu/arm_cortexa8/start.S, and defined
> in the include/asm-arm/arch-omap3/cpu.h
> 
> /arm_cortexa8/s5pc100/libs5pc100.a lib_arm/libarm.a
> fs/cramfs/libcramfs.a fs/fat/libfat.a fs/fdos/libfdos.a
> fs/jffs2/libjffs2.a fs/reiserfs/libreiserfs.a fs/ext2/libext2fs.a
> fs/yaffs2/libyaffs2.a fs/ubifs/libubifs.a net/libnet.a disk/libdisk.a
> drivers/bios_emulator/libatibiosemu.a drivers/block/libblock.a
> drivers/dma/libdma.a drivers/fpga/libfpga.a drivers/gpio/libgpio.a
> drivers/hwmon/libhwmon.a drivers/i2c/libi2c.a drivers/input/libinput.a
> drivers/misc/libmisc.a drivers/mmc/libmmc.a drivers/mtd/libmtd.a
> drivers/mtd/nand/libnand.a drivers/mtd/nand_legacy/libnand_legacy.a
> drivers/mtd/onenand/libonenand.a drivers/mtd/ubi/libubi.a
> drivers/mtd/spi/libspi_flash.a drivers/net/libnet.a
> drivers/net/phy/libphy.a drivers/net/sk98lin/libsk98lin.a
> drivers/pci/libpci.a drivers/pcmcia/libpcmcia.a drivers/spi/libspi.a
> drivers/rtc/librtc.a drivers/serial/libserial.a
> drivers/twserial/libtws.a drivers/usb/gadget/libusb_gadget.a
> drivers/usb/host/libusb_host.a drivers/usb/musb/libusb_musb.a
> drivers/video/libvideo.a drivers/watchdog/libwatchdog.a
> common/libcommon.a libfdt/libfdt.a api/libapi.a post/libpost.a
> board/samsung/tickertape/libtickertape.a --end-group -L
> /opt/toolchains/arm-2007q3/bin/../lib/gcc/arm-none-linux-gnueabi/4.2.1
> -lgcc \
> 			-Map u-boot.map -o u-boot
> cpu/arm_cortexa8/start.o: In function `rstctl':
> /ubuntu/archive/s5pc100/u-boot-arm/cpu/arm_cortexa8/start.S:514:
> undefined reference to `PRM_RSTCTRL'

This is new and not discussed anywhere on this list already?

So to summarize my understanding:

We have to:

a) move l2cache_enable() and l2cache_disable()
b) keep cache_flush() as it is
c) talk about start.S

Do you agree?

Best regards

Dirk

[U-Boot] [PATCH] The cache flush using coprocessor must be in lib_arm/cache-cp15.c

[Jean-Christophe PLAGNIOL-VILLARD]

On 09:07 Fri 29 May     , Dirk Behme wrote:
> Hi riverful,
>
> Kim, Heung Jun wrote:
>> Thanks to point. Dirk.
>>
>> And call me "riverful" next time, plz :)
>>
>>> Kim mentioned that he shuffles around the code due to compile errors while
>>> adding his new code. So we have to see if we can stay with the current
>>> implementation or have to change something.
>> Yes. I shuffles around a little now.
>>
>>> Kim: What compile errors do you get for cache_flush()? Could you check if
>>> you really don't need it? Why don't you get compile errors if you move it
>>> from cpu/arm_cortexa8/cpu.c to lib_arm/cache-cp15.c?
>> The compile errors are two.
>>
>> 1. First error occured cause of omap3 dependent definition CPU_3XX_ES20.
>>
>> cpu.c:99: warning: function declaration isn't a prototype
>> cpu.c: In function 'l2cache_enable':
>> cpu.c:104: warning: implicit declaration of function 'get_cpu_rev'
>> cpu.c:104: error: 'CPU_3XX_ES20' undeclared (first use in this function)
>> cpu.c:104: error: (Each undeclared identifier is reported only once
>> cpu.c:104: error: for each function it appears in.)
>> cpu.c: At top level:
>> cpu.c:129: warning: function declaration isn't a prototype
>> cpu.c: In function 'l2cache_disable':
>> cpu.c:134: error: 'CPU_3XX_ES20' undeclared (first use in this function)
>> make[1]: *** [cpu.o] Error 1
>
> Ok, this looks to me that we have to move l2cache_enable() and  
> l2cache_disable() to cpu/arm_cortexa8/omap3/board.c and rename them to  
> cortexa8_l2cache_enable()/disable() as asked in
<snip>
> This is new and not discussed anywhere on this list already?
>
> So to summarize my understanding:
>
> We have to:
>
> a) move l2cache_enable() and l2cache_disable()
move I agree rename no

we need to move the l2cache_XXXXX to cpu/$(ARCH)/$(SOC)/cache.c

> b) keep cache_flush() as it is

> c) talk about start.S
I'll propose a patch during the WE

Best Regards,
J.

[U-Boot] [PATCH] The cache flush using coprocessor must be in lib_arm/cache-cp15.c

[Dirk Behme]

Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 09:07 Fri 29 May     , Dirk Behme wrote:
...
>> c) talk about start.S
> I'll propose a patch during the WE

Do you have already any idea about how such a patch could look like? I 
haven't looked into it, yet.

Best regards

Dirk

[U-Boot] [PATCH] The cache flush using coprocessor must be in lib_arm/cache-cp15.c

[Jean-Christophe PLAGNIOL-VILLARD]

On 09:38 Fri 29 May     , Dirk Behme wrote:
> Jean-Christophe PLAGNIOL-VILLARD wrote:
>> On 09:07 Fri 29 May     , Dirk Behme wrote:
> ...
>>> c) talk about start.S
>> I'll propose a patch during the WE
>
> Do you have already any idea about how such a patch could look like? I  
> haven't looked into it, yet.
yes I've one, IIRC I've a patch that need to be rebase but it will clean this
today I've not the time sorry

Best Regards,
J.

[U-Boot] [PATCH] The cache flush using coprocessor must be in lib_arm/cache-cp15.c

[Dirk Behme]

Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 09:38 Fri 29 May     , Dirk Behme wrote:
>> Jean-Christophe PLAGNIOL-VILLARD wrote:
>>> On 09:07 Fri 29 May     , Dirk Behme wrote:
>> ...
>>>> c) talk about start.S
>>> I'll propose a patch during the WE
>> Do you have already any idea about how such a patch could look like? I  
>> haven't looked into it, yet.
> yes I've one, IIRC I've a patch that need to be rebase but it will clean this
> today I've not the time sorry

Do you like to send the patch as it is (should just take 1min?)?

Then, riverful or someone else maybe can help to rebase it, test it 
and we can already review/discuss about it.

Many thanks

Dirk