Skip to contentSkip to conversation

ARM: OMAP: L3 interconnect: Error reporting cleanups

PATCH In review Todd Poynor 2 participants 3 messages May 25, 2011

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 ARM: OMAP: L3 interconnect: Error reporting cleanups

* Move variable declarations from header file and make these static (the entire header file should probably go away).

* Define L3 TARG instance offsets, and read/write STDERRLOG registers relative to those offsets, rather than defining STDERRLOG_MAIN instance offsets and accessing other registers via offsets from that register.

* Use ffs() to find error source from the L3_FLAGMUX_REGERRn register.

* Remove extra l3_base[] entry.

* Modify L3 custom error message for consistency with standard error message.

* Fixup indentation in one spot.

---
 arch/arm/mach-omap2/omap_l3_noc.c |  122 +++++++++++++++++++++++++++++++------
 arch/arm/mach-omap2/omap_l3_noc.h |   88 +-------------------------
 2 files changed, 106 insertions(+), 104 deletions(-)
arch/arm/mach-omap2/omap_l3_noc.c +102 -20
index 82632c2..7e81e4c 100644
@@ -29,6 +29,88 @@
 
 #include "omap_l3_noc.h"
 
+static u32 l3_flagmux[L3_MODULES] = {
+	0x50C,
+	0x100C,
+	0X020C
+};
+
+/*
+ * L3 Target instances
+ */
+static u32 l3_targ_inst_clk1[] = {
+	0x100, /* DMM1 */
+	0x200, /* DMM2 */
+	0x300, /* ABE */
+	0x400, /* L4CFG */
+	0x600  /* CLK2 PWR DISC */
+};
+
+static u32 l3_targ_inst_clk2[] = {
+	0x500,		/* CORTEX M3 */
+	0x300,		/* DSS */
+	0x100,		/* GPMC */
+	0x400,		/* ISS */
+	0x700,		/* IVAHD */
+	0xD00,		/* missing in TRM  corresponds to AES1*/
+	0x900,		/* L4 PER0*/
+	0x200,		/* OCMRAM */
+	0x100,		/* missing in TRM corresponds to GPMC sERROR*/
+	0x600,		/* SGX */
+	0x800,		/* SL2 */
+	0x1600,		/* C2C */
+	0x1100,		/* missing in TRM corresponds PWR DISC CLK1*/
+	0xF00,		/* missing in TRM corrsponds to SHA1*/
+	0xE00,		/* missing in TRM corresponds to AES2*/
+	0xC00,		/* L4 PER3 */
+	0xA00,		/* L4 PER1*/
+	0xB00		/* L4 PER2*/
+};
+
+static u32 l3_targ_inst_clk3[] = {
+	0x0100	/* EMUSS */
+};
+
+static char *l3_targ_inst_name[L3_MODULES][18] = {
+	{
+	"DMM1",
+	"DMM2",
+	"ABE",
+	"L4CFG",
+	"CLK2 PWR DISC",
+	},
+	{
+	"CORTEX M3" ,
+	"DSS ",
+	"GPMC ",
+	"ISS ",
+	"IVAHD ",
+	"AES1",
+	"L4 PER0",
+	"OCMRAM ",
+	"GPMC sERROR",
+	"SGX ",
+	"SL2 ",
+	"C2C ",
+	"PWR DISC CLK1",
+	"SHA1",
+	"AES2",
+	"L4 PER3",
+	"L4 PER1",
+	"L4 PER2",
+	},
+	{
+	"EMUSS",
+	},
+};
+
+static u32 *l3_targ[L3_MODULES] = {
+	l3_targ_inst_clk1,
+	l3_targ_inst_clk2,
+	l3_targ_inst_clk3,
+};
+
+
 /*
  * Interrupt Handler for L3 error detection.
  *	1) Identify the L3 clockdomain partition to which the error belongs to.
@@ -56,10 +138,10 @@ static irqreturn_t l3_interrupt_handler(int irq, void *_l3)
 {
 
 	struct omap4_l3		*l3 = _l3;
-	int inttype, i, j;
+	int inttype, i;
 	int err_src = 0;
-	u32 std_err_main_addr, std_err_main, err_reg;
-	u32 base, slave_addr, clear;
+	u32 l3_targ_base, std_err_main, err_reg;
+	u32 base, clear;
 	char *source_name;
 
 	/* Get the Type of interrupt */
@@ -79,45 +161,45 @@ static irqreturn_t l3_interrupt_handler(int irq, void *_l3)
 		/* Get the corresponding error and analyse */
 		if (err_reg) {
 			/* Identify the source from control status register */
-			for (j = 0; !(err_reg & (1 << j)); j++)
-									;
-
-			err_src = j;
+			err_src = ffs(err_reg) - 1;
 			/* Read the stderrlog_main_source from clk domain */
-			std_err_main_addr = base + (*(l3_targ[i] + err_src));
-			std_err_main =  readl(std_err_main_addr);
+			l3_targ_base = base + (*(l3_targ[i] + err_src));
+			std_err_main =  readl(l3_targ_base +
+					      L3_TARG_STDERRLOG_MAIN);
 
 			switch ((std_err_main & CUSTOM_ERROR)) {
 			case STANDARD_ERROR:
 				source_name =
-				l3_targ_stderrlog_main_name[i][err_src];
-
-				slave_addr = std_err_main_addr +
-						L3_SLAVE_ADDRESS_OFFSET;
+					l3_targ_inst_name[i][err_src];
 				WARN(true, "L3 standard error: SOURCE:%s at address 0x%x\n",
-					source_name, readl(slave_addr));
+				     source_name,
+				     readl(l3_targ_base +
+					   L3_TARG_STDERRLOG_SLVOFSLSB));
 				/* clear the std error log*/
 				clear = std_err_main | CLEAR_STDERR_LOG;
-				writel(clear, std_err_main_addr);
+				writel(clear, l3_targ_base +
+					      L3_TARG_STDERRLOG_MAIN);
 				break;
 
 			case CUSTOM_ERROR:
 				source_name =
-				l3_targ_stderrlog_main_name[i][err_src];
+					l3_targ_inst_name[i][err_src];
 
-				WARN(true, "CUSTOM SRESP error with SOURCE:%s\n",
+				WARN(true, "L3 custom error: SOURCE:%s\n",
 							source_name);
 				/* clear the std error log*/
 				clear = std_err_main | CLEAR_STDERR_LOG;
-				writel(clear, std_err_main_addr);
+				writel(clear, l3_targ_base +
+					      L3_TARG_STDERRLOG_MAIN);
 				break;
 
 			default:
 				/* Nothing to be handled here as of now */
 				break;
 			}
-		/* Error found so break the for loop */
-		break;
+
+			/* Error found so break the for loop */
+			break;
 		}
 	}
 	return IRQ_HANDLED;
arch/arm/mach-omap2/omap_l3_noc.h +4 -84

111 lines not on this page · raw message

Signature · 1 line
1.7.3.1
#1 lore raw

Tod,

Quoted 3 lines · Todd Poynor

* Move variable declarations from header file and make these static (the entire header file should probably go away).

Infact the intial version posted on the list had all these structures in C file. After some comments on the list we moved them to header file. Let's not move it again.

Quoted 5 lines

* Define L3 TARG instance offsets, and read/write STDERRLOG registers relative to those offsets, rather than defining STDERRLOG_MAIN instance offsets and accessing other registers via offsets from that register.

Quoted 8 lines

* Use ffs() to find error source from the L3_FLAGMUX_REGERRn register.

* Remove extra l3_base[] entry.

* Modify L3 custom error message for consistency with standard error message.

These changes are ok.

Quoted 2 lines

* Fixup indentation in one spot.

This code indentations needs more fixing and I have patch to clean that up. Your change is already part of that patch.

Quoted 5 lines
---
  arch/arm/mach-omap2/omap_l3_noc.c |  122 +++++++++++++++++++++++++++++++------
  arch/arm/mach-omap2/omap_l3_noc.h |   88 +-------------------------
  2 files changed, 106 insertions(+), 104 deletions(-)

Can you update your patch with above. I will add that along with other L3 clean-up patch.

Thanks

regards
Santosh

#2 lore raw
Quoted 8 lines · Santosh Shilimkar

Tod,

Quoted 3 lines · Todd Poynor

* Move variable declarations from header file and make these static (the entire header file should probably go away).

Infact the intial version posted on the list had all these structures in C file. After some comments on the list we moved them to header file. Let's not move it again.

I searched (a little) but didn't find the feedback that led to this. Tell me more? Defining variables (not externs, not even statics) in header files is usually frowned upon.

Quoted 14 lines
Quoted 6 lines

* Define L3 TARG instance offsets, and read/write STDERRLOG registers relative to those offsets, rather than defining STDERRLOG_MAIN instance offsets and accessing other registers via offsets from that register.

Quoted 8 lines

* Use ffs() to find error source from the L3_FLAGMUX_REGERRn register.

* Remove extra l3_base[] entry.

* Modify L3 custom error message for consistency with standard error message.

Quoted 7 lines

These changes are ok.

Quoted 2 lines

* Fixup indentation in one spot.

This code indentations needs more fixing and I have patch to clean that up. Your change is already part of that patch.

OK.

Quoted 13 lines
Quoted 6 lines
---
 arch/arm/mach-omap2/omap_l3_noc.c |  122 +++++++++++++++++++++++++++++++------
 arch/arm/mach-omap2/omap_l3_noc.h |   88 +-------------------------
 2 files changed, 106 insertions(+), 104 deletions(-)

Can you update your patch with above. I will add that along with other L3 clean-up patch.

Thanks

regards
Santosh