| Message ID | 1480463779.11342.79.camel@kernel.crashing.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
Benjamin Herrenschmidt <benh@kernel.crashing.org> 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 <benh@kernel.crashing.org> 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? 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
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);
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(-)