On 08.09.22 16:06, Marek Behún wrote:

From: Pali Rohár pali@kernel.org

Commit 3308933d2fe9 ("arm: mvebu: Avoid reading MVEBU_REG_PCIE_DEVID register too many times") broke support for caches on all Armada SoCs.

Before that commit there was code:

 if (mvebu_soc_family() != MVEBU_SOC_A375) {
     dcache_enable();
 }

And after that commit there is code:

 if (IS_ENABLED(CONFIG_ARMADA_375)) {
     dcache_enable();
 }

Comment above this code says that d-cache should be disabled on Armada 375. But new code inverted logic and broke Armada 375 and slowed down all other Armada SoCs (including A38x).

Fix this issue by changing logic to:

 if (!IS_ENABLED(CONFIG_ARMADA_375)) {
     dcache_enable();
 }

Which matches behavior prior that commit.

Fixes: 3308933d2fe9 ("arm: mvebu: Avoid reading MVEBU_REG_PCIE_DEVID register too many times") 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 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-mvebu/cpu.c b/arch/arm/mach-mvebu/cpu.c index 1457af1d6a..b512ccc501 100644 --- a/arch/arm/mach-mvebu/cpu.c +++ b/arch/arm/mach-mvebu/cpu.c @@ -663,7 +663,7 @@ void enable_caches(void) * ethernet driver (mvpp2). So lets keep the d-cache disabled * until this is solved. */

• if (IS_ENABLED(CONFIG_ARMADA_375)) {

• if (!IS_ENABLED(CONFIG_ARMADA_375)) { /* Enable D-cache. I-cache is already enabled in start.S */ dcache_enable(); }

Viele Grüße, Stefan Roese

On 08.09.22 16:06, Marek Behún wrote:

From: Pali Rohár pali@kernel.org

Commit c86d53fd88df ("arm: mvebu: Don't disable cache at startup on Armada XP at all") introduced branch for non-AXP code which was guarded by A38X condition. Fix this issue by checking for AXP platform, not by A38X.

Fixes: c86d53fd88df ("arm: mvebu: Don't disable cache at startup on Armada XP at all") 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 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-mvebu/cpu.c b/arch/arm/mach-mvebu/cpu.c index b512ccc501..8e5d1ba21e 100644 --- a/arch/arm/mach-mvebu/cpu.c +++ b/arch/arm/mach-mvebu/cpu.c @@ -448,7 +448,7 @@ int arch_cpu_init(void) struct pl310_regs *const pl310 = (struct pl310_regs *)CONFIG_SYS_PL310_BASE;

• if (IS_ENABLED(CONFIG_ARMADA_38X)) {

• if (!IS_ENABLED(CONFIG_ARMADA_XP)) { /*
  • To fully release / unlock this area from cache, we need
  • to flush all caches and disable the L2 cache.

Viele Grüße, Stefan Roese

On 08.09.22 16:06, Marek Behún wrote:

From: Pali Rohár pali@kernel.org

Replace magic constant 1 when disabling MMU by macro CR_M from include header file asm/system.h.

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/lowlevel.S | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-mvebu/lowlevel.S b/arch/arm/mach-mvebu/lowlevel.S index 2491310eb0..b460382c6b 100644 --- a/arch/arm/mach-mvebu/lowlevel.S +++ b/arch/arm/mach-mvebu/lowlevel.S @@ -2,6 +2,7 @@

#include <config.h> #include <linux/linkage.h> +#include <asm/system.h>

ENTRY(arch_very_early_init) #ifdef CONFIG_ARMADA_38X @@ -12,7 +13,7 @@ ENTRY(arch_very_early_init) * still locked to cache. */ mrc p15, 0, r0, c1, c0, 0

• bic r0, #1

• bic r0, #CR_M mcr p15, 0, r0, c1, c0, 0 #endif

Viele Grüße, Stefan Roese

On 08.09.22 16:06, Marek Behún wrote:

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)

Viele Grüße, Stefan Roese

On 08.09.22 16:06, Marek Behún wrote:

From: Pali Rohár pali@kernel.org

Commit 5bb2c550b11e ("arm: mvebu: Move internal registers in arch_very_early_init() function") moved code from file cpu.c to lowlevel.c, which moves Marvell internal registers from address INTREG_BASE_ADDR_REG to SOC_REGS_PHY_BASE.

But the steps describing how to do it correctly were documented only in older U-Boot versions and commit cefd764222ee ("arm: mvebu: Fix internal register config on A38x") probably unintentionally removed important details about MMU from code comments around.

Commit 5bb2c550b11e ("arm: mvebu: Move internal registers in arch_very_early_init() function") implemented code movement according to (now incomplete) comments which resulted in semi-broken code.

The result is that I-cache is currently disabled for all Armada 38x boards and maybe there are some other (unreported / undetected) issues.

Reimplement it correctly. First flush all caches, then disable MMU and L2 cache and then move Marvell internal registers. There is no need to explicitly disable I-cache.

After this change lzmadec command with lzma image of 0x7000000 bytes is doing decompression just 5 seconds. Before this change it was 30 seconds.

To make lowlevel.S code more readable, extend asm/pl310.h header file to be compatible with assembler and use macros from this file.

Fixes: 5bb2c550b11e ("arm: mvebu: Move internal registers in arch_very_early_init() function") 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/include/asm/pl310.h | 9 +++++++-- arch/arm/mach-mvebu/cpu.c | 13 ------------- arch/arm/mach-mvebu/lowlevel.S | 27 +++++++++++++++++++++++++++ 3 files changed, 34 insertions(+), 15 deletions(-)

diff --git a/arch/arm/include/asm/pl310.h b/arch/arm/include/asm/pl310.h index f69e9e45f8..9d4cd68ee4 100644 --- a/arch/arm/include/asm/pl310.h +++ b/arch/arm/include/asm/pl310.h @@ -7,13 +7,12 @@ #ifndef _PL310_H_ #define _PL310_H_

-#include <linux/types.h>

• /* Register bit fields */ #define PL310_AUX_CTRL_ASSOCIATIVITY_MASK (1 << 16) #define L2X0_DYNAMIC_CLK_GATING_EN (1 << 1) #define L2X0_STNDBY_MODE_EN (1 << 0) #define L2X0_CTRL_EN 1

+#define L2X0_CTRL_OFF 0x100

#define L310_SHARED_ATT_OVERRIDE_ENABLE (1 << 22) #define L310_AUX_CTRL_DATA_PREFETCH_MASK (1 << 28) @@ -27,6 +26,10 @@ #define L2X0_CACHE_ID_RTL_MASK 0x3f #define L2X0_CACHE_ID_RTL_R3P2 0x8

+#ifndef __ASSEMBLY__

+#include <linux/types.h>

• struct pl310_regs { u32 pl310_cache_id; u32 pl310_cache_type;

@@ -87,3 +90,5 @@ void pl310_inval_range(u32 start, u32 end); void pl310_clean_inval_range(u32 start, u32 end);

#endif

+#endif diff --git a/arch/arm/mach-mvebu/cpu.c b/arch/arm/mach-mvebu/cpu.c index d410b87171..9a80440d1a 100644 --- a/arch/arm/mach-mvebu/cpu.c +++ b/arch/arm/mach-mvebu/cpu.c @@ -445,19 +445,6 @@ static void setup_usb_phys(void) */ int arch_cpu_init(void) {

• struct pl310_regs *const pl310 =

• (struct pl310_regs *)CONFIG_SYS_PL310_BASE;
  

• if (!IS_ENABLED(CONFIG_ARMADA_XP)) {

• /*
  

•  * To fully release / unlock this area from cache, we need
  

•  * to flush all caches and disable the L2 cache.
  

•  */
  

• icache_disable();
  

• dcache_disable();
  

• clrbits_le32(&pl310->pl310_ctrl, L2X0_CTRL_EN);
  

• }
• /*
  • We need to call mvebu_mbus_probe() before calling
  • update_sdram_window_sizes() as it disables all previously

diff --git a/arch/arm/mach-mvebu/lowlevel.S b/arch/arm/mach-mvebu/lowlevel.S index b460382c6b..60c2072c35 100644 --- a/arch/arm/mach-mvebu/lowlevel.S +++ b/arch/arm/mach-mvebu/lowlevel.S @@ -3,6 +3,7 @@ #include <config.h> #include <linux/linkage.h> #include <asm/system.h> +#include <asm/pl310.h>

ENTRY(arch_very_early_init) #ifdef CONFIG_ARMADA_38X @@ -11,10 +12,36 @@ ENTRY(arch_very_early_init) * register address on Armada 38x. Without this the SDRAM * located at >= 0x4000.0000 is also not accessible, as its * still locked to cache.

• *
  

• * So to fully release / unlock this area from cache, we need
  

• * to first flush all caches, then disable the MMU and
  

• * disable the L2 cache.
  

  */

• /* Invalidate L1 I/D */

• mov r0, #0 @ set up for MCR

• mcr p15, 0, r0, c8, c7, 0 @ invalidate TLBs

• mcr p15, 0, r0, c7, c5, 0 @ invalidate icache

• mcr p15, 0, r0, c7, c5, 6 @ invalidate BP array

• mcr p15, 0, r0, c7, c10, 4 @ DSB

• mcr p15, 0, r0, c7, c5, 4 @ ISB

• /* Disable MMU */ mrc p15, 0, r0, c1, c0, 0 bic r0, #CR_M mcr p15, 0, r0, c1, c0, 0

• /*

• * Disable L2 cache
  

• *
  

• * NOTE: Internal registers are still at address INTREG_BASE_ADDR_REG
  

• *       but CONFIG_SYS_PL310_BASE is already calculated from base
  

• *       address SOC_REGS_PHY_BASE.
  

• */
  

• ldr r1, =(CONFIG_SYS_PL310_BASE - SOC_REGS_PHY_BASE + INTREG_BASE_ADDR_REG)

• ldr r0, [r1, #L2X0_CTRL_OFF]

• bic r0, #L2X0_CTRL_EN

• str r0, [r1, #L2X0_CTRL_OFF] #endif

  /* Move internal registers from INTREG_BASE_ADDR_REG to SOC_REGS_PHY_BASE */

Viele Grüße, Stefan Roese