[PATCH 22/26 v6] spl: spl_legacy: Add cache flush after reading U-Boot image

[Daniel Schwierzeck]

Am 09.04.20 um 09:43 schrieb Stefan Roese:
> On 09.04.20 09:29, Simon Goldschmidt wrote:
>> Am 08.04.2020 um 10:09 schrieb Stefan Roese:
>>> From: Weijie Gao <weijie.gao at mediatek.com>
>>>
>>> Flush the cache after reading of the U-Boot proper into SDRAM so that
>>> it can be started.
>>>
>>> This is needed on some platforms, e.g. MT76x8.
>>>
>>> Signed-off-by: Weijie Gao <weijie.gao at mediatek.com>
>>> Signed-off-by: Stefan Roese <sr at denx.de>
>>> Cc: Weijie Gao <weijie.gao at mediatek.com>
>>> Cc: Simon Goldschmidt <simon.k.r.goldschmidt at gmail.com>
>>> ---
>>> Changes in v6:
>>> - New patch
>>>
>>>   common/spl/spl_legacy.c | 4 ++++
>>>   1 file changed, 4 insertions(+)
>>>
>>> diff --git a/common/spl/spl_legacy.c b/common/spl/spl_legacy.c
>>> index 2cd2a74a4c..e320206098 100644
>>> --- a/common/spl/spl_legacy.c
>>> +++ b/common/spl/spl_legacy.c
>>> @@ -4,6 +4,7 @@
>>>    */
>>>     #include <common.h>
>>> +#include <cpu_func.h>
>>>   #include <malloc.h>
>>>   #include <spl.h>
>>>   @@ -108,5 +109,8 @@ int spl_load_legacy_img(struct spl_image_info
>>> *spl_image,
>>>           return -EINVAL;
>>>       }
>>>   +    /* Flush cache of loaded U-Boot image */
>>> +    flush_cache((unsigned long)spl_image->load_addr, spl_image->size);
>>> +
>>
>> I failed to find the mail, but haven't we discussed moving this cache
>> flush to your arch before starting a binary?
> 
> I don't remember such an agreement. But I don't object in general.
> 
>> I cannot see this being required or implemented for non-legacy images,
>> and it still seems wrong here.
> 
> Its pretty common when an OS image is loaded and booted, that the cache
> is flushed before running code from it.
> 
> But I can rework this to add some empty weak function (I don't see an
> easy better way) to do this platform specific image handling before its
> booted. Or do you have a better idea on how to handle this?
> 
> Thanks,
> Stefan

actually all MIPS platforms with non-coherent cache need that flush
before jumping to another U-Boot or OS. Thus currently random crashes
can occur on all MIPS boards with generic SPL not just mtmips.

But jump_to_image_no_args() in spl.c is already declared as weak, so we
should implement that unconditionally for MIPS similar to your patch for
do_go_exec(). It would be great if you could prepare a patch to replace
this one, thanks.

-- 
- Daniel