On 4/3/20 10:03 AM, Patrick DELAUNAY wrote: > Hi Marek, Hi,
>> From: Marek Vasut <[email protected]> >> 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.
