[PATCH 1/3] arm: caches: protect dram_bank_mmu_setup access to bi_dram

[Marek Vasut]

On 4/8/20 7:54 PM, Patrick DELAUNAY wrote:
> Dear Marek,
> 
>> From: Marek Vasut <marex at denx.de>
>> Sent: vendredi 3 avril 2020 23:27
>>
>> On 4/3/20 10:28 AM, Patrick Delaunay wrote:
>>> Add protection in dram_bank_mmu_setup() to avoid access to bd->bi_dram
>>> before relocation.
>>>
>>> This patch allow to use the generic weak function dram_bank_mmu_setup
>>> to activate the MMU and the data cache in SPL or in U-Boot before
>>> relocation, when bd->bi_dram is not yet initialized.
>>>
>>> In this cases, the MMU must be initialized explicitly with
>>> mmu_set_region_dcache_behaviour function.
>>>
>>> Signed-off-by: Patrick Delaunay <patrick.delaunay at st.com>
>>> ---
>>>
>>>  arch/arm/lib/cache-cp15.c | 4 ++++
>>>  1 file changed, 4 insertions(+)
>>>
>>> diff --git a/arch/arm/lib/cache-cp15.c b/arch/arm/lib/cache-cp15.c
>>> index f8d20960da..54509f11c3 100644
>>> --- a/arch/arm/lib/cache-cp15.c
>>> +++ b/arch/arm/lib/cache-cp15.c
>>> @@ -91,6 +91,10 @@ __weak void dram_bank_mmu_setup(int bank)
>>>  	bd_t *bd = gd->bd;
>>>  	int	i;
>>>
>>> +	/* bd->bi_dram is available only after relocation */
>>> +	if ((gd->flags & GD_FLG_RELOC) == 0)
>>> +		return;
>>
>> Why not just set the bd->bi_dram correctly before this is called ?
> 
> Just set "bd->bi_dram" seens as a hack.
> 
> For me the bd struct can be updated only in  common/board_f.c
> after reserve_board() for U-Boot 
> Or other spl_set_bd() called in board_init_r() for SPL. 
> 
> And that can cause issue if CONFIG_NR_DRAM_BANKS > 1
> (even it is not the case today for STM32MP1).
> 
> But if this kind of protection is not correct here I prefer come back
> to overidde of the weak fucntio dram_bank_mmu_setup in stm32mp arch
> (it is the reason this weak definition)

I'd say, let's wait for feedback from the others.

I would be inclined to set bd->bi_dram, but maybe others have other
opinions.