On 4/3/20 10:03 AM, Patrick DELAUNAY wrote:
Hi Marek,
Hi,
From: Marek Vasut marex@denx.de Sent: lundi 30 mars 2020 16:04
On 3/30/20 3:49 PM, Patrick DELAUNAY wrote:
Hi Marek,
Hi,
[...]
- /* Enable D-cache. I-cache is already enabled in start.S */
- /* I-cache is already enabled in start.S */
Not needed for arm V7 (I copy this function from other platfrom ... I don't remember which one)
Maybe this needs to be de-duplicated if it's a copy ?
I don't remember.... As I mixed several references
But I found the same content in many arm arch;
arch/arm/mach-imx/mx5/soc.c:67 arch/arm/mach-rockchip/board.c:47 arch/arm/mach-tegra/board.c:271 arch/arm/mach-sunxi/board.c:347 arch/arm/mach-exynos/soc.c:30: arch/arm/mach-zynq/cpu.c:88: arch/arm/cpu/armv7/iproc-common/hwinit-common.c:1 arch/arm/mach-u8500/cache.c:14 arch/arm/mach-keystone/init.c:206
And different implementation in arch/arm/mach-socfpga/misc.c:55
On SoCFPGA, we are sure both need to be enabled, except SPL might not want to enable dcache, which is why it's implemented there that way. I dunno about the other platforms.
mach-omap2/omap-cache.c:49 void enable_caches(void) {
/* Enable I cache if not enabled */ if (!icache_status()) icache_enable();
dcache_enable(); }
the issue the weak function empty, so it is mandatory to redefine the real implementation for each platform.
arch/arm/lib/cache.c:35 /*
- Default implementation of enable_caches()
- Real implementation should be in platform code
*/ __weak void enable_caches(void) { puts("WARNING: Caches not enabled\n"); }
Hm, that's a valid point. Then I think we're stuck with re-reimplementing this one.
[...]
+static void set_mmu_dcache(u32 addr, u32 size) {
- int i;
- u32 start, end;
- start = addr >> MMU_SECTION_SHIFT;
- end = ((u64)addr + (u64)size) >> MMU_SECTION_SHIFT;
Is this a copy of dram_bank_mmu_setup() in arch/arm/lib/cache-cp15.c ? Why ?
It is not just a copy...
set__mmu_dache is only a static helper for function dram_bank_mmu_setup()
I override the default implementation of the weak functon dram_bank_mmu_setup()
Can you instead augment the original implementation to cater for this usecase or would that be too difficult ?
Have a generic behavior...
I will propose to protect the access to bd->bi_dram[bank] in dram_bank_mmu_setup
Thanks!
[...]
SYRAM content (board_f)
- SPL code
- SPL data
- SPL stack (reversed order)
- TTB
But I can move it in BSS as global apl variable, I need to think about it.... It is probably more clean.
Please do :)
I willl move it in ".data" section in V2 for SPL and U-Boot.
Even in binary size increase and the SPL load time by ROM code is increase by 30ms.
Can it be mallocated instead ? If it's in initialized data, it will add to the binary size, and I don't think you need it to be initialized data.