Re: [PATCH u-boot-marvell 4/5] arm: mvebu: Enable L2 cache also on Armada 38x

Stefan Roese
2022-09-12T06:59:11Z

> From: Pali Rohár <pali@kernel.org>
>
> For some unknown reason when L2 cache is disabled on Armada 385 then loadb,
> loadx and loady commands do not work with higher baudrates than 115200
> (they just abort transfer) and lzmadec command with lzma image of size
> 0x7000000 (maybe even smaller, we tested this one) is doing decompression
> for more than 2 minutes. After enabling L2 cache decompression takes only
> 30s and loadb, loadx and loady are stable and working fine.
>
> git bisect identified problematic commit 3308933d2fe9 ("arm: mvebu: Avoid
> reading MVEBU_REG_PCIE_DEVID register too many times"). Before this commit
> above issues were not present.
>
> But investigation showed that above issue was possible to reproduce also by
> reverting that commit and forcing compiler to do inline optimization of
> mvebu_soc_family() function. Which seems that the root of this issue is in
> caches and position of instruction of segments. So currently it is unknown
> what is or was broken, but code movement, code inlining or other compiler
> optimization triggered it.
>
> Commit 3e5ce7ceeb94 ("arm: mvebu: Enable L2 cache on Armada XP") mentioned
> that enabling L2 cache on Armada XP improved performance and that Armada
> 38x has L2 disabled (which is default state) and if needed it has to be
> enabled in separate patch. As enabling L2 cache also improve performance
> on Armada 38x, enable it.
>
> Note that Aurora cache in no outer mode is available only on Armada XP,
> hence it is not touched for Armada 38x code.
>
> Fixes: 3308933d2fe9 ("arm: mvebu: Avoid reading MVEBU_REG_PCIE_DEVID register too many times")
> Reported-by: Marek Behún <kabel@kernel.org>
> Signed-off-by: Pali Rohár <pali@kernel.org>
> Signed-off-by: Marek Behún <kabel@kernel.org>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> ---
> arch/arm/mach-mvebu/cpu.c | 22 +++++++++++++++-------
> 1 file changed, 15 insertions(+), 7 deletions(-)
>
> diff --git a/arch/arm/mach-mvebu/cpu.c b/arch/arm/mach-mvebu/cpu.c
> index 8e5d1ba21e..d410b87171 100644
> --- a/arch/arm/mach-mvebu/cpu.c
> +++ b/arch/arm/mach-mvebu/cpu.c
> @@ -671,13 +671,21 @@ void enable_caches(void)
>
> void v7_outer_cache_enable(void)
> {
> + struct pl310_regs *const pl310 =
> + (struct pl310_regs *)CONFIG_SYS_PL310_BASE;
> +
> + /* The L2 cache is already disabled at this point */
> +
> + /*
> + * For now L2 cache will be enabled only for Armada XP and Armada 38x.
> + * It can be enabled also for other SoCs after testing that it works fine.
> + */
> + if (!IS_ENABLED(CONFIG_ARMADA_XP) && !IS_ENABLED(CONFIG_ARMADA_38X))
> + return;
> +
> if (IS_ENABLED(CONFIG_ARMADA_XP)) {
> - struct pl310_regs *const pl310 =
> - (struct pl310_regs *)CONFIG_SYS_PL310_BASE;
> u32 u;
>
> - /* The L2 cache is already disabled at this point */
> -
> /*
> * For Aurora cache in no outer mode, enable via the CP15
> * coprocessor broadcasting of cache commands to L2.
> @@ -687,10 +695,10 @@ void v7_outer_cache_enable(void)
> asm volatile("mcr p15, 1, %0, c15, c2, 0" : : "r" (u));
>
> isb();
> -
> - /* Enable the L2 cache */
> - setbits_le32(&pl310->pl310_ctrl, L2X0_CTRL_EN);
> }
> +
> + /* Enable the L2 cache */
> + setbits_le32(&pl310->pl310_ctrl, L2X0_CTRL_EN);
> }
>
> void v7_outer_cache_disable(void)