[PATCH] powerpc/opal-irqchip: Use interrupt names if present

Recent versions of OPAL will be able to provide names for the various
OPAL interrupts via a new "opal-interrupt-names" property. So let's
use them to make /proc/interrupts more informative.

This also modernises the code that fetches the interrupt array to use
the helpers provided by the generic code instead of hand-parsing the
property.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 arch/powerpc/platforms/powernv/opal-irqchip.c | 45 ++++++++++++++++++++-------
 1 file changed, 33 insertions(+), 12 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/opal-irqchip.c b/arch/powerpc/platforms/powernv/opal-irqchip.c
index 998316b..fe9b029 100644
--- a/arch/powerpc/platforms/powernv/opal-irqchip.c
+++ b/arch/powerpc/platforms/powernv/opal-irqchip.c

@@ -183,8 +183,9 @@ void opal_event_shutdown(void)
 int __init opal_event_init(void)
 {
 	struct device_node *dn, *opal_node;
-	const __be32 *irqs;
-	int i, irqlen, rc = 0;
+	const char **names;
+	u32 *irqs;
+	int i, rc = 0;
 
 	opal_node = of_find_node_by_path("/ibm,opal");
 	if (!opal_node) {

@@ -209,37 +210,57 @@ int __init opal_event_init(void)
 		goto out;
 	}
 
-	/* Get interrupt property */
-	irqs = of_get_property(opal_node, "opal-interrupts", &irqlen);
-	opal_irq_count = irqs ? (irqlen / 4) : 0;
+	/* Get opal-interrupts property and names if present */
+	rc = of_property_count_u32_elems(opal_node, "opal-interrupts");
+	if (rc < 0)
+		goto out;
+	opal_irq_count = rc;
 	pr_debug("Found %d interrupts reserved for OPAL\n", opal_irq_count);
+	irqs = kzalloc(rc * sizeof(u32), GFP_KERNEL);
+	if (WARN_ON(!irqs))
+		goto out;
+	rc = of_property_read_u32_array(opal_node, "opal-interrupts",
+					irqs, opal_irq_count);
+	if (rc < 0) {
+		pr_err("Error %d reading opal-interrupts array\n", rc);
+		goto out;
+	}
+	names = kzalloc(opal_irq_count * sizeof(char *), GFP_KERNEL);
+	of_property_read_string_array(opal_node, "opal-interrupts-names",
+				      names, opal_irq_count);
 
 	/* Install interrupt handlers */
 	opal_irqs = kcalloc(opal_irq_count, sizeof(*opal_irqs), GFP_KERNEL);
-	for (i = 0; irqs && i < opal_irq_count; i++, irqs++) {
-		unsigned int irq, virq;
+	for (i = 0; i < opal_irq_count; i++) {
+		unsigned int virq;
+		char *name;
 
 		/* Get hardware and virtual IRQ */
-		irq = be32_to_cpup(irqs);
-		virq = irq_create_mapping(NULL, irq);
+		virq = irq_create_mapping(NULL, irqs[i]);
 		if (!virq) {
-			pr_warn("Failed to map irq 0x%x\n", irq);
+			pr_warn("Failed to map irq 0x%x\n", irqs[i]);
 			continue;
 		}
+		if (names && names[i] && strlen(names[i]))
+			name = kasprintf(GFP_KERNEL, "opal-%s", names[i]);
+		else
+			name = kasprintf(GFP_KERNEL, "opal");
 
 		/* Install interrupt handler */
 		rc = request_irq(virq, opal_interrupt, IRQF_TRIGGER_LOW,
-				 "opal", NULL);
+				 name, NULL);
 		if (rc) {
 			irq_dispose_mapping(virq);
 			pr_warn("Error %d requesting irq %d (0x%x)\n",
-				 rc, virq, irq);
+				 rc, virq, irqs[i]);
 			continue;
 		}
 
 		/* Cache IRQ */
 		opal_irqs[i] = virq;
 	}
+	kfree(irqs);
+	kfree(names);
 
 out:
 	of_node_put(opal_node);

Benjamin Herrenschmidt [off-list ref] writes:

Recent versions of OPAL will be able to provide names for the various
OPAL interrupts via a new "opal-interrupt-names" property. So let's
use them to make /proc/interrupts more informative.

I guess there's no point asking whether there's a generic device tree
spec for this sort of thing, and if so whether we should use it?

cheers

On Wed, 2016-11-30 at 19:07 +1100, Michael Ellerman wrote:

Benjamin Herrenschmidt [off-list ref] writes:

quoted

Recent versions of OPAL will be able to provide names for the
various
OPAL interrupts via a new "opal-interrupt-names" property. So let's
use them to make /proc/interrupts more informative.

I guess there's no point asking whether there's a generic device tree
spec for this sort of thing, and if so whether we should use it?

There isn't one. The existing "opal-interrupts" from day one was a bit
weird anyway, it's not a proper "interrupts" property to begin with,
but it's unfixable now. Also I don't think there's a generic way to
name interrupts either.

So for this specific case, just adding a new prop with a string-list
matching 1:1 the entries in "opal-interrupts" is the most logical
choice.

Cheers,
Ben.

On Wed, 2016-11-30 at 19:21 +1100, Benjamin Herrenschmidt wrote:

There isn't one. The existing "opal-interrupts" from day one was a
bit
weird anyway, it's not a proper "interrupts" property to begin with,
but it's unfixable now. Also I don't think there's a generic way to
name interrupts either.

So for this specific case, just adding a new prop with a string-list
matching 1:1 the entries in "opal-interrupts" is the most logical
choice.

I spoke too soon... people have come up with an "interrupt-names"
property that matches a corresponding standard "interrupts" property.

The format is the same, a string-list with an entry per entry in
"interrupts".

So we have choices here... we could use the standard for both, making
OPAL generate both the old property and the new property "pair", then
make the code in linux look for the new one(s) and fallback, or we can
just add opal-interrupt-names, which is less churn but keeps it non-
standard.

I do have a soft-spot for going down the standard path even if it means
effectively duplicating the information between two properties. We'll
probably never be able to get rid of the old one, unless we backport
enough to make the new one mandatory for P9 in which case skiboot can
drop the "legacy" one on P9 and later.

Cheers,
Ben