[Skiboot] [PATCH] core/interrupts: Clean up OPAL ICS node

Oliver O'Halloran oohall at gmail.com
Tue Apr 23 16:29:41 AEST 2019


For some reason that's lost to time the OPAL interrupt-controller source
device node has a unit address of 0. This causes dtc to emit warnings
since the node overlaps with the memory at 0 node. There is also no real need
for the OPAL ICS node to even have a unit address since it has no
registers (reflected by the zero length in reg).

This patch reworks the OPAL ICS node so that it has no unit address
which results in a bare top-level node called "interrupt-controller."
While we're here get rid of the explicit device-tree traversal in
get_ics_phandle() and use a cached copy that is setup in add_ics_node().

Suggested-by: Maxim Polyakov <m.polyakov at yadro.com>
Signed-off-by: Oliver O'Halloran <oohall at gmail.com>
---
I had a look at the xics/xive code in linux and it doesn't seem that
either make any assumptions about the @0 being in the unit address.

Boot tested on a Tuleta and a Witherspoon without issues.
---
 core/interrupts.c    | 32 +++++++++++++++++++-------------
 include/interrupts.h |  2 +-
 2 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/core/interrupts.c b/core/interrupts.c
index 5d7a68cd5eff..b4602d374ec3 100644
--- a/core/interrupts.c
+++ b/core/interrupts.c
@@ -173,18 +173,27 @@ uint32_t get_psi_interrupt(uint32_t chip_id)
 	return irq;
 }
 
+static struct dt_node *ics_node;
 
-struct dt_node *add_ics_node(void)
+void add_ics_node(void)
 {
-	struct dt_node *ics = dt_new_addr(dt_root, "interrupt-controller", 0);
+	struct dt_node *ics;
 	bool has_xive;
 
-	if (!ics)
-		return NULL;
+	/*
+	 * This should never happen, but this code is ancient so maybe there's
+	 * some obscure edge case it's covering
+	 */
+	ics = dt_new(dt_root, "interrupt-controller");
+	if (!ics) {
+		ics_node = dt_find_by_name(dt_root, "interrupt-controller");
+		prlog(PR_DEBUG, "OPAL ICS node already exists?\n");
+
+		return;
+	}
 
 	has_xive = proc_gen >= proc_gen_p9;
 
-	dt_add_property_cells(ics, "reg", 0, 0, 0, 0);
 	dt_add_property_strings(ics, "compatible",
 				has_xive ? "ibm,opal-xive-vc" : "IBM,ppc-xics",
 				"IBM,opal-xics");
@@ -194,19 +203,16 @@ struct dt_node *add_ics_node(void)
 			       "PowerPC-Interrupt-Source-Controller");
 	dt_add_property(ics, "interrupt-controller", NULL, 0);
 
-	return ics;
+	ics_node = ics;
 }
 
 uint32_t get_ics_phandle(void)
 {
-	struct dt_node *i;
 
-	for (i = dt_first(dt_root); i; i = dt_next(dt_root, i)) {
-		if (streq(i->name, "interrupt-controller at 0")) {
-			return i->phandle;
-		}
-	}
-	abort();
+	if (!ics_node)
+		abort();
+
+	return ics_node->phandle;
 }
 
 void add_opal_interrupts(void)
diff --git a/include/interrupts.h b/include/interrupts.h
index 2c4fa7e92399..4a49bbaeb601 100644
--- a/include/interrupts.h
+++ b/include/interrupts.h
@@ -312,7 +312,7 @@ extern void irq_for_each_source(void (*cb)(struct irq_source *, void *),
 
 extern uint32_t get_psi_interrupt(uint32_t chip_id);
 
-extern struct dt_node *add_ics_node(void);
+extern void add_ics_node(void);
 extern void add_opal_interrupts(void);
 extern uint32_t get_ics_phandle(void);
 
-- 
2.20.1



More information about the Skiboot mailing list