On Thu, 31 Aug 2023 at 19:01, Tom Rini <[email protected]> wrote: > > On Thu, Aug 31, 2023 at 10:24:07AM +0200, Michal Simek wrote: > > > > > > On 8/30/23 20:52, Tom Rini wrote: > > > On Tue, Aug 22, 2023 at 01:21:12PM +0530, Bhupesh Sharma wrote: > > > > > > > Ideally SYS_ICACHE_OFF and SYS_DCACHE_OFF should never be set for > > > > ARM64 (ARMv8) platforms, as it makes booting u-boot slower on these > > > > platforms. > > > > > > > > However, if some platforms require ICACHE or DCACHE to be disabled > > > > only for the smaller SPL stage, we should support such configurations > > > > in u-boot as well. > > > > I am missing the reason why this change should be accepted. > > > > > > > > > > Compile-tested for: > > > > - qemu arm64 > > > > qemu doesn't model caches that's why I don't think it is relevant here. > > > > > > - imx8 > > > > - stm32 > > > > > > > > and run-tested on: > > > > - Qualcomm RB3 platform > > > > > > > > Cc: Tom Rini <[email protected]> > > > > Cc: Simon Glass <[email protected]> > > > > Cc: Peng Fan <[email protected]> > > > > Signed-off-by: Bhupesh Sharma <[email protected]> > > > > Reviewed-by: Tom Rini <[email protected]> > > > > --- > > > > arch/arm/Kconfig | 2 ++ > > > > arch/arm/cpu/armv8/Makefile | 4 ++-- > > > > 2 files changed, 4 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig > > > > index 36ee1e9a3c..92bff715e3 100644 > > > > --- a/arch/arm/Kconfig > > > > +++ b/arch/arm/Kconfig > > > > @@ -141,6 +141,7 @@ config THUMB2_KERNEL > > > > config SYS_ICACHE_OFF > > > > bool "Do not enable icache" > > > > + depends on !ARM64 > > > > help > > > > Do not enable instruction cache in U-Boot. > > > > @@ -153,6 +154,7 @@ config SPL_SYS_ICACHE_OFF > > > > config SYS_DCACHE_OFF > > > > bool "Do not enable dcache" > > > > + depends on !ARM64 > > > > help > > > > Do not enable data cache in U-Boot. > > > > diff --git a/arch/arm/cpu/armv8/Makefile b/arch/arm/cpu/armv8/Makefile > > > > index bba4f570db..0d0c1728e4 100644 > > > > --- a/arch/arm/cpu/armv8/Makefile > > > > +++ b/arch/arm/cpu/armv8/Makefile > > > > @@ -5,13 +5,13 @@ > > > > extra-y := start.o > > > > +obj-y += cache_v8.o > > > > obj-y += cpu.o > > > > ifndef CONFIG_$(SPL_TPL_)TIMER > > > > obj-$(CONFIG_SYS_ARCH_TIMER) += generic_timer.o > > > > endif > > > > ifndef CONFIG_$(SPL_)SYS_DCACHE_OFF > > > > -obj-y += cache_v8.o > > > > -obj-y += cache.o > > > > +obj-y += cache.o > > > > endif > > > > ifdef CONFIG_SPL_BUILD > > > > obj-$(CONFIG_ARMV8_SPL_EXCEPTION_VECTORS) += exceptions.o > > > > > > I'm adding Michal here because this changes the behavior on xilinx > > > platforms that had the icache off. I was under the impression that at > > > the levels U-Boot runs at on aarch64, we just couldn't have > > > icache/dcache off, but I guess that's wrong? > > > > > > > yes. We are using icache off for mini u-boot configurations for quite a long > > time. I tried to find out any commit why we started to use it but didn't > > find any record. But will ask team to retest that use cases to see if that > > is working or not. > > In our case we are using small full U-Boot not any SPL variant. > > > > My biggest issue with the patch is that it is not clear what issue it is > > trying to solve as I have mentioned above. > > It just says that ideally icache/dcache should be on. If you want to enforce > > it on your platform you can select it when that platform is selected and > > don't need enforce it for other platforms. > > Thanks Michal. Bhupesh, I was wrong in v1, having caches off is allowed, > so you just need to fix issues like part one does where code doesn't > compile when the options are disabled, and then are there further > changes needed for your usecase?
Sure Tom. I will go back to sending v1 as v3 with some minor changes. Thanks, Bhupesh
