[PATCH] arm: stm32mp1: activate data cache in SPL and before relocation
Marek Vasut
marex at denx.de
Fri Apr 3 23:37:22 CEST 2020
On 4/3/20 10:03 AM, Patrick DELAUNAY wrote:
> Hi Marek,
Hi,
>> From: Marek Vasut <marex at 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.
More information about the U-Boot
mailing list