Skip to contentSkip to conversation

ARM: cache-l2x0: Manage the errata at run time

PATCH Superseded srinidhi kasagar 4 participants 7 messages Jan 21, 2013 – Jan 22, 2013 Arm (32-bit)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

PATCH 1/4 ARM: cache-l2x0: Manage the errata at run time

Make it possible to manage the errata by its own by using the l2x0 ID register. This relieves the platforms from choosing the Errata's at compile time

---
 arch/arm/include/asm/hardware/cache-l2x0.h |    2 +
 arch/arm/mm/cache-l2x0.c                   |   77 +++++++++++++++-------------
 2 files changed, 43 insertions(+), 36 deletions(-)
arch/arm/include/asm/hardware/cache-l2x0.h +2 -0
index 3b2c40b..d5994ac 100644
@@ -117,6 +117,8 @@ static inline int l2x0_of_init(u32 aux_val, u32 aux_mask)
 }
 #endif
 
+asmlinkage u32 l2x0_get_rtl_release(void);
+
 struct l2x0_regs {
 	unsigned long phy_base;
 	unsigned long aux_ctrl;
arch/arm/mm/cache-l2x0.c +41 -36

125 lines not on this page · raw message

Signature · 1 line
1.7.2.dirty
#1 lore raw

PATCH 1/4 ARM: cache-l2x0: Manage the errata at run time

Quoted 30 lines · srinidhi kasagar

Make it possible to manage the errata by its own by using the l2x0 ID register. This relieves the platforms from choosing the Errata's at compile time

---
 arch/arm/include/asm/hardware/cache-l2x0.h |    2 +
 arch/arm/mm/cache-l2x0.c                   |   77 +++++++++++++++-------------
 2 files changed, 43 insertions(+), 36 deletions(-)
arch/arm/include/asm/hardware/cache-l2x0.h +2 -0
index 3b2c40b..d5994ac 100644
@@ -117,6 +117,8 @@ static inline int l2x0_of_init(u32 aux_val, u32 aux_mask)
 }
 #endif
 
+asmlinkage u32 l2x0_get_rtl_release(void);
+
 struct l2x0_regs {
 	unsigned long phy_base;
 	unsigned long aux_ctrl;
arch/arm/mm/cache-l2x0.c +0 -0
index c2f3739..49058ac 100644
@@ -49,6 +49,16 @@ struct l2x0_of_data {
 
 static bool of_init = false;

arch/arm/mm/cache-l2x0.c struct l2x0_of_data { · line 60

Quoted 10 lines
arch/arm/mm/cache-l2x0.c +9 -0
 
+/*
+ * Identify ther RTL releases of l2x0 - This might help in applying
+ * the l2x0 errata's dynamically rather compile time options
+ */
+asmlinkage u32 l2x0_get_rtl_release(void)
+{
+	return readl_relaxed(l2x0_base + L2X0_CACHE_ID) &
+			L2X0_CACHE_ID_RTL_MASK;
+}

You're calling this function all over the place, including from the flush code. Can you read the RTL release during probe and stash it somewhere instead please?

Will

#2 lore raw

PATCH 1/4 ARM: cache-l2x0: Manage the errata at run time

Quoted 32 lines · Will Deacon
Quoted 31 lines · srinidhi kasagar

Make it possible to manage the errata by its own by using the l2x0 ID register. This relieves the platforms from choosing the Errata's at compile time

---
 arch/arm/include/asm/hardware/cache-l2x0.h |    2 +
 arch/arm/mm/cache-l2x0.c                   |   77 +++++++++++++++-------------
 2 files changed, 43 insertions(+), 36 deletions(-)
arch/arm/include/asm/hardware/cache-l2x0.h +2 -0
index 3b2c40b..d5994ac 100644
@@ -117,6 +117,8 @@ static inline int l2x0_of_init(u32 aux_val, u32 aux_mask)
 }
 #endif
 
+asmlinkage u32 l2x0_get_rtl_release(void);
+
 struct l2x0_regs {
 	unsigned long phy_base;
 	unsigned long aux_ctrl;
arch/arm/mm/cache-l2x0.c +0 -0
index c2f3739..49058ac 100644
@@ -49,6 +49,16 @@ struct l2x0_of_data {
 
 static bool of_init = false;
 
Quoted 13 lines

arch/arm/mm/cache-l2x0.c struct l2x0_of_data { · line 60

Quoted 9 lines
arch/arm/mm/cache-l2x0.c +9 -0
+/*
+ * Identify ther RTL releases of l2x0 - This might help in applying
+ * the l2x0 errata's dynamically rather compile time options
+ */
+asmlinkage u32 l2x0_get_rtl_release(void)
+{
+	return readl_relaxed(l2x0_base + L2X0_CACHE_ID) &
+			L2X0_CACHE_ID_RTL_MASK;
+}

You're calling this function all over the place, including from the flush code. Can you read the RTL release during probe and stash it somewhere instead please?

I thought of doing that, however TI omap's suspend is the only one which needs this API as well, So, I had to make it global. Refer my patch 3/4. I can duplicate this for omap if you think so..

srinidhi

#3 lore raw

PATCH 1/4 ARM: cache-l2x0: Manage the errata at run time

Quoted 12 lines · Srinidhi Kasagar
Quoted 10 lines · Will Deacon

arch/arm/mm/cache-l2x0.c struct l2x0_of_data { · line 60

Quoted 5 lines · srinidhi kasagar
arch/arm/mm/cache-l2x0.c +5 -0
+asmlinkage u32 l2x0_get_rtl_release(void)
+{
+	return readl_relaxed(l2x0_base + L2X0_CACHE_ID) &
+			L2X0_CACHE_ID_RTL_MASK;
+}

You're calling this function all over the place, including from the flush code. Can you read the RTL release during probe and stash it somewhere instead please?

Quoted 3 lines

I thought of doing that, however TI omap's suspend is the only one which needs this API as well, So, I had to make it global. Refer my patch 3/4. I can duplicate this for omap if you think so..

Just make the get_rtl_release_function return the stashed value if it's been probed and get the driver to use the stashed value directly.

Will

#4 lore raw

PATCH 1/4 ARM: cache-l2x0: Manage the errata at run time

In reply to srinidhi kasagar

arch/arm/mm/cache-l2x0.c struct l2x0_of_data { · line 56

Quoted 5 lines · srinidhi kasagar
arch/arm/mm/cache-l2x0.c +5 -0
+/*
+ * Identify ther RTL releases of l2x0 - This might help in applying
+ * the l2x0 errata's dynamically rather compile time options
+ */
+asmlinkage u32 l2x0_get_rtl_release(void)

Why asmlinkage?

Quoted 16 lines
arch/arm/mm/cache-l2x0.c +6 -2
+{
+	return readl_relaxed(l2x0_base + L2X0_CACHE_ID) &
+			L2X0_CACHE_ID_RTL_MASK;
+}
+
 static inline void cache_wait_way(void __iomem *reg, unsigned long mask)
 {
 	/* wait for cache operation by line or way to complete */
@@ -87,46 +97,41 @@ static inline void l2x0_inv_line(unsigned long addr)
 	writel_relaxed(addr, base + L2X0_INV_LINE_PA);
 }
 
-#if defined(CONFIG_PL310_ERRATA_588369) || defined(CONFIG_PL310_ERRATA_727915)
-static inline void debug_writel(unsigned long val)
+static void debug_writel(unsigned long val)
 {

arch/arm/mm/cache-l2x0.c static inline void l2x0_inv_line(unsigned long addr) · line 109

Quoted 10 lines
arch/arm/mm/cache-l2x0.c +8 -2
-	if (outer_cache.set_debug)
-		outer_cache.set_debug(val);
+	u32 l2x0_revision = l2x0_get_rtl_release();
+
+	if (l2x0_revision == L2X0_CACHE_ID_RTL_R3P0 ||
+		l2x0_revision == L2X0_CACHE_ID_RTL_R2P0 ||
+		l2x0_revision == L2X0_CACHE_ID_RTL_R1P0 ||
+		l2x0_revision == L2X0_CACHE_ID_RTL_R0P0)
+			if (outer_cache.set_debug)
+				outer_cache.set_debug(val);

This needs comments from the TI folk. Also, change this around - if there's no setting for 'set_debug' there's no point reading the rtl release and checking it against a set of values. Added Santosh.

arch/arm/mm/cache-l2x0.c static inline void l2x0_inv_line(unsigned long addr) · line 124

Quoted 10 lines
arch/arm/mm/cache-l2x0.c +5 -2
 static inline void l2x0_flush_line(unsigned long addr)
 {
 	void __iomem *base = l2x0_base;
-	cache_wait(base + L2X0_CLEAN_INV_LINE_PA, 1);
-	writel_relaxed(addr, base + L2X0_CLEAN_INV_LINE_PA);
+	u32 l2x0_revision = l2x0_get_rtl_release();
+
+	if (l2x0_revision == L2X0_CACHE_ID_RTL_R0P0 ||
+		l2x0_revision == L2X0_CACHE_ID_RTL_R1P0)
+	{

Coding standards.

	if (l2x0_revision == L2X0_CACHE_ID_RTL_R0P0 ||
	    l2x0_revision == L2X0_CACHE_ID_RTL_R1P0) {
#5 lore raw

PATCH 1/4 ARM: cache-l2x0: Manage the errata at run time

Quoted 8 lines · Russell King - ARM Linux

arch/arm/mm/cache-l2x0.c struct l2x0_of_data { · line 56

Quoted 5 lines · srinidhi kasagar
arch/arm/mm/cache-l2x0.c +5 -0
+/*
+ * Identify ther RTL releases of l2x0 - This might help in applying
+ * the l2x0 errata's dynamically rather compile time options
+ */
+asmlinkage u32 l2x0_get_rtl_release(void)

Why asmlinkage?

Because I found that TI's omap suspend code also need this. arch/arm/mach-omap2/sleep44xx.S

Quoted 32 lines
Quoted 17 lines
arch/arm/mm/cache-l2x0.c +6 -2
+{
+	return readl_relaxed(l2x0_base + L2X0_CACHE_ID) &
+			L2X0_CACHE_ID_RTL_MASK;
+}
+
 static inline void cache_wait_way(void __iomem *reg, unsigned long mask)
 {
 	/* wait for cache operation by line or way to complete */
@@ -87,46 +97,41 @@ static inline void l2x0_inv_line(unsigned long addr)
 	writel_relaxed(addr, base + L2X0_INV_LINE_PA);
 }
 
-#if defined(CONFIG_PL310_ERRATA_588369) || defined(CONFIG_PL310_ERRATA_727915)
-static inline void debug_writel(unsigned long val)
+static void debug_writel(unsigned long val)
 {

arch/arm/mm/cache-l2x0.c static inline void l2x0_inv_line(unsigned long addr) · line 109

Quoted 10 lines
arch/arm/mm/cache-l2x0.c +8 -2
-	if (outer_cache.set_debug)
-		outer_cache.set_debug(val);
+	u32 l2x0_revision = l2x0_get_rtl_release();
+
+	if (l2x0_revision == L2X0_CACHE_ID_RTL_R3P0 ||
+		l2x0_revision == L2X0_CACHE_ID_RTL_R2P0 ||
+		l2x0_revision == L2X0_CACHE_ID_RTL_R1P0 ||
+		l2x0_revision == L2X0_CACHE_ID_RTL_R0P0)
+			if (outer_cache.set_debug)
+				outer_cache.set_debug(val);

This needs comments from the TI folk. Also, change this around - if there's no setting for 'set_debug' there's no point reading the rtl release and checking it against a set of values. Added Santosh.

Quoted 12 lines

arch/arm/mm/cache-l2x0.c static inline void l2x0_inv_line(unsigned long addr) · line 124

Quoted 10 lines
arch/arm/mm/cache-l2x0.c +5 -2
 static inline void l2x0_flush_line(unsigned long addr)
 {
 	void __iomem *base = l2x0_base;
-	cache_wait(base + L2X0_CLEAN_INV_LINE_PA, 1);
-	writel_relaxed(addr, base + L2X0_CLEAN_INV_LINE_PA);
+	u32 l2x0_revision = l2x0_get_rtl_release();
+
+	if (l2x0_revision == L2X0_CACHE_ID_RTL_R0P0 ||
+		l2x0_revision == L2X0_CACHE_ID_RTL_R1P0)
+	{

Coding standards.

OK.

regards/srinidhi

#6 lore raw

PATCH 1/4 ARM: cache-l2x0: Manage the errata at run time

In reply to Russell King - ARM Linux
Quoted 31 lines · Russell King - ARM Linux

arch/arm/mm/cache-l2x0.c struct l2x0_of_data { · line 56

Quoted 5 lines · srinidhi kasagar
arch/arm/mm/cache-l2x0.c +5 -0
+/*
+ * Identify ther RTL releases of l2x0 - This might help in applying
+ * the l2x0 errata's dynamically rather compile time options
+ */
+asmlinkage u32 l2x0_get_rtl_release(void)

Why asmlinkage?

Quoted 22 lines
arch/arm/mm/cache-l2x0.c +10 -4
+{
+	return readl_relaxed(l2x0_base + L2X0_CACHE_ID) &
+			L2X0_CACHE_ID_RTL_MASK;
+}
+
  static inline void cache_wait_way(void __iomem *reg, unsigned long mask)
  {
  	/* wait for cache operation by line or way to complete */
@@ -87,46 +97,41 @@ static inline void l2x0_inv_line(unsigned long addr)
  	writel_relaxed(addr, base + L2X0_INV_LINE_PA);
  }
 
-#if defined(CONFIG_PL310_ERRATA_588369) || defined(CONFIG_PL310_ERRATA_727915)
-static inline void debug_writel(unsigned long val)
+static void debug_writel(unsigned long val)
  {
-	if (outer_cache.set_debug)
-		outer_cache.set_debug(val);
+	u32 l2x0_revision = l2x0_get_rtl_release();
+
+	if (l2x0_revision == L2X0_CACHE_ID_RTL_R3P0 ||
+		l2x0_revision == L2X0_CACHE_ID_RTL_R2P0 ||
Quoted 9 lines

arch/arm/mm/cache-l2x0.c static inline void l2x0_inv_line(unsigned long addr) · line 109

Quoted 4 lines
arch/arm/mm/cache-l2x0.c +4 -0
+		l2x0_revision == L2X0_CACHE_ID_RTL_R1P0 ||
+		l2x0_revision == L2X0_CACHE_ID_RTL_R0P0)
+			if (outer_cache.set_debug)
+				outer_cache.set_debug(val);

This needs comments from the TI folk. Also, change this around - if there's no setting for 'set_debug' there's no point reading the rtl release and checking it against a set of values. Added Santosh.

Thanks Russell for looping me. 588369 is fixed in r3p0 but 727915 is present. Both has same work around so the version checks are fine. I agree that moving the version check inside .set_debug check.

Regards
Santosh