arm: Replace test for CONFIG_ARMV7 with CONFIG_CPU_V7

PATCH New Marek Vasut 2 participants 4 messages Dec 29, 2015 – Jan 29, 2016 Arm (32-bit)

Share & tools

Marek Vasut

author

2015-12-29 18:44 UTC #0 lore raw

PATCH 1/2 arm: Replace test for CONFIG_ARMV7 with CONFIG_CPU_V7

The arch/arm/lib/cache-cp15.c checks for CONFIG_ARMV7 and if this macro is set, it configures TTBR0 register. This register must be configured for the cache on ARMv7 to operate correctly.

The problem is that noone actually sets the CONFIG_ARMV7 macro and thus the TTBR0 is not configured at all. On SoCFPGA, this produces all sorts of minor issues which are hard to replicate, for example certain USB sticks are not detected or QSPI NOR sometimes fails to write pages completely.

The solution is to replace CONFIG_ARMV7 test with CONFIG_CPU_V7 one. This is correct because the code which added the test(s) for CONFIG_ARMV7 was added shortly after CONFIG_ARMV7 was replaced by CONFIG_CPU_V7 and this code was not adjusted correctly to reflect that change.

Signed-off-by Marek Vasut <[email protected]>

Cc Tom Rini <[email protected]>

Cc Albert Aribaud <[email protected]>

Cc Simon Glass <[email protected]>

---
 arch/arm/include/asm/system.h | 4 ++--
 arch/arm/lib/cache-cp15.c     | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

arch/arm/include/asm/system.h +2 -2

index 71b3108..dec83c7 100644
@@ -220,7 +220,7 @@ static inline void set_dacr(unsigned int val)
 	isb();
 }
 
-#ifdef CONFIG_ARMV7
+#ifdef CONFIG_CPU_V7
 /* Short-Descriptor Translation Table Level 1 Bits */
 #define TTB_SECT_NS_MASK	(1 << 19)
 #define TTB_SECT_NG_MASK	(1 << 17)

@@ -257,7 +257,7 @@ enum {
 	MMU_SECTION_SIZE	= 1 << MMU_SECTION_SHIFT,
 };
 
-#ifdef CONFIG_ARMV7
+#ifdef CONFIG_CPU_V7
 /* TTBR0 bits */
 #define TTBR0_BASE_ADDR_MASK	0xFFFFC000
 #define TTBR0_RGN_NC			(0 << 3)

arch/arm/lib/cache-cp15.c +1 -1

index c65e068..8e18538 100644
@@ -96,7 +96,7 @@ static inline void mmu_setup(void)
 		dram_bank_mmu_setup(i);
 	}
 
-#ifdef CONFIG_ARMV7
+#ifdef CONFIG_CPU_V7
 	/* Set TTBR0 */
 	reg = gd->arch.tlb_addr & TTBR0_BASE_ADDR_MASK;
 #if defined(CONFIG_SYS_ARM_CACHE_WRITETHROUGH)

Signature · 1 line

2.1.4

Marek Vasut

author

2015-12-29 18:44 UTC #1 lore raw

PATCH 2/2 arm: Remove S bit from MMU section entry

Restore the old behavior of the MMU section entries configuration, which is without the S-bit.

Signed-off-by Marek Vasut <[email protected]>

Cc Tom Rini <[email protected]>

Cc Albert Aribaud <[email protected]>

Cc Simon Glass <[email protected]>

---
 arch/arm/include/asm/system.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

arch/arm/include/asm/system.h +1 -2

index dec83c7..8d64d23 100644
@@ -235,8 +235,7 @@ static inline void set_dacr(unsigned int val)
 
 /* options available for data cache on each page */
 enum dcache_option {
-	DCACHE_OFF = TTB_SECT_S_MASK | TTB_SECT_DOMAIN(0) |
-					TTB_SECT_XN_MASK | TTB_SECT,
+	DCACHE_OFF = TTB_SECT_DOMAIN(0) | TTB_SECT_XN_MASK | TTB_SECT,
 	DCACHE_WRITETHROUGH = DCACHE_OFF | TTB_SECT_C_MASK,
 	DCACHE_WRITEBACK = DCACHE_WRITETHROUGH | TTB_SECT_B_MASK,
 	DCACHE_WRITEALLOC = DCACHE_WRITEBACK | TTB_SECT_TEX(1),

Signature · 1 line

2.1.4

Albert ARIBAUD

2015-12-30 22:18 UTC #2 lore raw

PATCH 1/2 arm: Replace test for CONFIG_ARMV7 with CONFIG_CPU_V7

In reply to Marek Vasut

Hello Marek,

Quoted 9 lines · Marek Vasut

The arch/arm/lib/cache-cp15.c checks for CONFIG_ARMV7 and if this macro is set, it configures TTBR0 register. This register must be configured for the cache on ARMv7 to operate correctly.
The problem is that noone actually sets the CONFIG_ARMV7 macro and thus the TTBR0 is not configured at all. On SoCFPGA, this produces all sorts of minor issues which are hard to replicate, for example certain USB sticks are not detected or QSPI NOR sometimes fails to write pages completely.

Quoted 4 lines

The solution is to replace CONFIG_ARMV7 test with CONFIG_CPU_V7 one. This is correct because the code which added the test(s) for CONFIG_ARMV7 was added shortly after CONFIG_ARMV7 was replaced by CONFIG_CPU_V7 and this code was not adjusted correctly to reflect that change.

Analysis of the series shows that:

- it does not change the values of DCACHE_OFF, DCACHE_WRITETHROUGH and DCACHE_WRITEBACK;

- it does change the value of DCACHE_WRITEALLOC from 0x16 to 0x101E, but DCACHE_WRITEALLOC is only used when CONFIG_SYS_ARM_CACHE_WRITEALLOC is defined, which does not happen throughout U-Boot, as shown by a search for "WRITEALLOC".

- it sets inner and outer region cache control bits in TTBR0, to match the cacheability of the DDR in which the MMU table resides.

Marek performed tests of patch 1/2 only (with the S bit set) and of the whole series (wih the S bit clear). With patch 1/2 only, Marek could witness the performance hit described by Stefan Roese; with the whole series, Marek saw no performance hit any more.

I will therefore take this patch series in, since it fixes an obvious issue in the U-Boot code and does not show any adverse effect.

Amicalement,

Signature · 1 line

Albert.

Albert ARIBAUD

2016-01-29 16:22 UTC #3 lore raw

PATCH 1/2 arm: Replace test for CONFIG_ARMV7 with CONFIG_CPU_V7

In reply to Marek Vasut

Hello Marek,

Quoted 61 lines · Marek Vasut

The arch/arm/lib/cache-cp15.c checks for CONFIG_ARMV7 and if this macro is set, it configures TTBR0 register. This register must be configured for the cache on ARMv7 to operate correctly.
The problem is that noone actually sets the CONFIG_ARMV7 macro and thus the TTBR0 is not configured at all. On SoCFPGA, this produces all sorts of minor issues which are hard to replicate, for example certain USB sticks are not detected or QSPI NOR sometimes fails to write pages completely.
The solution is to replace CONFIG_ARMV7 test with CONFIG_CPU_V7 one. This is correct because the code which added the test(s) for CONFIG_ARMV7 was added shortly after CONFIG_ARMV7 was replaced by CONFIG_CPU_V7 and this code was not adjusted correctly to reflect that change.
Signed-off-by Marek Vasut <[email protected]>
Cc Tom Rini <[email protected]>
Cc Albert Aribaud <[email protected]>
Cc Simon Glass <[email protected]>
---
 arch/arm/include/asm/system.h | 4 ++--
 arch/arm/lib/cache-cp15.c     | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)
arch/arm/include/asm/system.h +2 -2
index 71b3108..dec83c7 100644
@@ -220,7 +220,7 @@ static inline void set_dacr(unsigned int val)
 	isb();
 }
 
-#ifdef CONFIG_ARMV7
+#ifdef CONFIG_CPU_V7
 /* Short-Descriptor Translation Table Level 1 Bits */
 #define TTB_SECT_NS_MASK	(1 << 19)
 #define TTB_SECT_NG_MASK	(1 << 17)
@@ -257,7 +257,7 @@ enum {
 	MMU_SECTION_SIZE	= 1 << MMU_SECTION_SHIFT,
 };
 
-#ifdef CONFIG_ARMV7
+#ifdef CONFIG_CPU_V7
 /* TTBR0 bits */
 #define TTBR0_BASE_ADDR_MASK	0xFFFFC000
 #define TTBR0_RGN_NC			(0 << 3)
arch/arm/lib/cache-cp15.c +1 -1
index c65e068..8e18538 100644
@@ -96,7 +96,7 @@ static inline void mmu_setup(void)
 		dram_bank_mmu_setup(i);
 	}
 
-#ifdef CONFIG_ARMV7
+#ifdef CONFIG_CPU_V7
 	/* Set TTBR0 */
 	reg = gd->arch.tlb_addr & TTBR0_BASE_ADDR_MASK;
 #if defined(CONFIG_SYS_ARM_CACHE_WRITETHROUGH)
Signature · 1 line
2.1.4

Series applied to u-boot-arm/master, thanks!

Amicalement,

Signature · 1 line

Albert.