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

[Maxim Polyakov]

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 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);
>  
> 

Hi all. Any news about this patch?

-- 
Regards,
Maxim Polyakov 
Software Engineer, YADRO.