Message ID | 20190423062941.14217-1-oohall@gmail.com |
---|---|
State | Under Review |
Headers | show |
Series | core/interrupts: Clean up OPAL ICS node | expand |
Context | Check | Description |
---|---|---|
snowpatch_ozlabs/apply_patch | success | Successfully applied on branch master (d318cdb3863fcf92288528bfed3b6e435cf6f0ef) |
snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot | fail | Test snowpatch/job/snowpatch-skiboot on branch master |
snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot-dco | success | Signed-off-by present |
23.04.2019 9:29, Oliver O'Halloran пишет: > 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@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@yadro.com> > Signed-off-by: Oliver O'Halloran <oohall@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@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); > > Hi all. Any news about this patch?
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@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);
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@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@yadro.com> Signed-off-by: Oliver O'Halloran <oohall@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(-)