Am 09.04.2020 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....@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....@mediatek.com> >>> Signed-off-by: Stefan Roese <s...@denx.de> >>> Cc: Weijie Gao <weijie....@mediatek.com> >>> Cc: Simon Goldschmidt <simon.k.r.goldschm...@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.
Ok, maybe I remember wrong then. > >> 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. Yes, of course. I can follow you there. I'm just curious why it's added here and only for legacy images. > > 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? I don't know. And I'm not totally opposed to this patch. I just think it's a strange thing to do here. If you need it, I think it should be in a more central place. Surely, you'd need it for non-legacy images, too? I could imagine this being added to jump_to_image_no_args(). That way, we would have the cache flush in a central place instead of stumbling accross it again in the future (e.g. for non-legacy images). OTOH, I'm not sure that would be free of side-effects...? Regards, Simon > > Thanks, > Stefan >