[PATCH u-boot-marvell 0/5] mvebu important fixes

[PATCH u-boot-marvell 0/5] mvebu important fixes

[Marek Behún]

Hello Stefan,

could you please test and send these patches to Tom for the next rc,
if possible?

I discovered that loadb does not work with higher speeds and lzmadec
is way too slow, bisected to one of Pali's commits, and then Pali
investigated that there are some issues with cache settings.

Marek

Pali Rohár (5):
  arm: mvebu: Fix function enable_caches
  arm: mvebu: Guard non-AXP code by checking for AXP
  arm: mvebu: lowlevel.S: Use CR_M from asm/system.h
  arm: mvebu: Enable L2 cache also on Armada 38x
  arm: mvebu: Fix moving internal registers

 arch/arm/include/asm/pl310.h   |  9 +++++++--
 arch/arm/mach-mvebu/cpu.c      | 37 +++++++++++++++-------------------
 arch/arm/mach-mvebu/lowlevel.S | 30 ++++++++++++++++++++++++++-
 3 files changed, 52 insertions(+), 24 deletions(-)

-- 
2.35.1

[PATCH u-boot-marvell 1/5] arm: mvebu: Fix function enable_caches

[Marek Behún]

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>
---
 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();
 	}
-- 
2.35.1

[PATCH u-boot-marvell 2/5] arm: mvebu: Guard non-AXP code by checking for AXP

[Marek Behún]

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>
---
 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.
-- 
2.35.1

[PATCH u-boot-marvell 3/5] arm: mvebu: lowlevel.S: Use CR_M from asm/system.h

[Marek Behún]

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>
---
 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
 
-- 
2.35.1

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

[Marek Behún]

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>
---
 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)
-- 
2.35.1

[PATCH u-boot-marvell 5/5] arm: mvebu: Fix moving internal registers

[Marek Behún]

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>
---
 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 */
-- 
2.35.1

Re: [PATCH u-boot-marvell 1/5] arm: mvebu: Fix function enable_caches

[Stefan Roese]

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

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de

Re: [PATCH u-boot-marvell 2/5] arm: mvebu: Guard non-AXP code by checking for AXP

[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

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de

Re: [PATCH u-boot-marvell 3/5] arm: mvebu: lowlevel.S: Use CR_M from asm/system.h

[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

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de

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

[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

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de

Re: [PATCH u-boot-marvell 5/5] arm: mvebu: Fix moving internal registers

[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

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de

Re: [PATCH u-boot-marvell 0/5] mvebu important fixes

[Stefan Roese]

Hi Marek,

On 08.09.22 16:06, Marek Behún wrote:
> Hello Stefan,
> 
> could you please test and send these patches to Tom for the next rc,
> if possible?
> 
> I discovered that loadb does not work with higher speeds and lzmadec
> is way too slow, bisected to one of Pali's commits, and then Pali
> investigated that there are some issues with cache settings.

Just back from a short vacation. Let me take a look, if this is
possible. I'll do some basic tests on my Armada XP platform as well.

Thanks,
Stefan

> Marek
> 
> Pali Rohár (5):
>    arm: mvebu: Fix function enable_caches
>    arm: mvebu: Guard non-AXP code by checking for AXP
>    arm: mvebu: lowlevel.S: Use CR_M from asm/system.h
>    arm: mvebu: Enable L2 cache also on Armada 38x
>    arm: mvebu: Fix moving internal registers
> 
>   arch/arm/include/asm/pl310.h   |  9 +++++++--
>   arch/arm/mach-mvebu/cpu.c      | 37 +++++++++++++++-------------------
>   arch/arm/mach-mvebu/lowlevel.S | 30 ++++++++++++++++++++++++++-
>   3 files changed, 52 insertions(+), 24 deletions(-)
> 

Viele Grüße,
Stefan Roese

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de