[Skiboot] [PATCH v3 4/4] hw/psi: Remove explicit external IRQ policy

[Cédric Le Goater]

On 05/09/2019 12:50, Oliver O'Halloran wrote:
> Rather than having an explicit policy use the presence of a platform
> defined external interrupt handler to determine whether we should direct
> the interrupt to OPAL or not. This lets us remove a pile of
> comments about why the policy is necessary and the comments about why
> we need to un-set it on P8+
> 
> Signed-off-by: Oliver O'Halloran <oohall at gmail.com>

P9 returns an extra IRQ_ATTR_TYPE_LSI attribute but that's minor.

Reviewed-by: Cédric Le Goater <clg at kaod.org>

Thanks,

C.

> ---
> v3: Replaced the policy check with this. We can't check what to set the
>     policy to in astbmc_early_init() since that's run inside the
>     platform's probe function. As a result the platform structure
>     has not been populated yet and there's no way to determine what
>     the policy should be.
> ---
>  hw/psi.c                    | 14 ++++++--------
>  include/psi.h               | 12 ------------
>  platforms/astbmc/common.c   |  3 ---
>  platforms/astbmc/garrison.c | 11 -----------
>  platforms/astbmc/p8dnu.c    | 11 -----------
>  5 files changed, 6 insertions(+), 45 deletions(-)
> 
> diff --git a/hw/psi.c b/hw/psi.c
> index d466f5a807e9..bc170bbcff13 100644
> --- a/hw/psi.c
> +++ b/hw/psi.c
> @@ -29,7 +29,6 @@ static LIST_HEAD(psis);
>  static u64 psi_link_timer;
>  static u64 psi_link_timeout;
>  static bool psi_link_poll_active;
> -static bool psi_ext_irq_policy = EXTERNAL_IRQ_POLICY_LINUX;
>  
>  static void psi_activate_phb(struct psi *psi);
>  
> @@ -471,8 +470,8 @@ static uint64_t psi_p8_irq_attributes(struct irq_source *is, uint32_t isn)
>  	if (psi->no_lpc_irqs && idx == P8_IRQ_PSI_LPC)
>  		return IRQ_ATTR_TARGET_LINUX;
>  
> -	if (idx == P8_IRQ_PSI_EXTERNAL &&
> -	    psi_ext_irq_policy == EXTERNAL_IRQ_POLICY_LINUX)
> +	/* Only direct external interrupts to OPAL if we have a handler */
> +	if (idx == P8_IRQ_PSI_EXTERNAL && !platform.external_irq)
>  		return IRQ_ATTR_TARGET_LINUX;
>  
>  	attr = IRQ_ATTR_TARGET_OPAL | IRQ_ATTR_TYPE_LSI;
> @@ -625,6 +624,10 @@ static uint64_t psi_p9_irq_attributes(struct irq_source *is __unused,
>  	 if (is_lpc_serirq)
>  		 return lpc_get_irq_policy(psi->chip_id, idx - P9_PSI_IRQ_LPC_SIRQ0);
>  
> +	/* Only direct external interrupts to OPAL if we have a handler */
> +	if (idx == P9_PSI_IRQ_EXTERNAL && !platform.external_irq)
> +		return IRQ_ATTR_TARGET_LINUX | IRQ_ATTR_TYPE_LSI;
> +
>  	return IRQ_ATTR_TARGET_OPAL | IRQ_ATTR_TYPE_LSI;
>  }
>  
> @@ -649,11 +652,6 @@ static const struct irq_source_ops psi_p9_irq_ops = {
>  	.name = psi_p9_irq_name,
>  };
>  
> -void psi_set_external_irq_policy(bool policy)
> -{
> -	psi_ext_irq_policy = policy;
> -}
> -
>  static void psi_init_p8_interrupts(struct psi *psi)
>  {
>  	uint32_t irq;
> diff --git a/include/psi.h b/include/psi.h
> index 9836e354a31b..ee1e0a7ae2ec 100644
> --- a/include/psi.h
> +++ b/include/psi.h
> @@ -247,18 +247,6 @@ extern void psi_irq_reset(void);
>  extern void psi_enable_fsp_interrupt(struct psi *psi);
>  extern void psi_fsp_link_in_use(struct psi *psi);
>  
> -/*
> - * Must be called by the platform probe() function as the policy
> - * is established before platform.init
> - *
> - * This defines whether the external interrupt should be passed to
> - * the OS or handled locally in skiboot. Return true for skiboot
> - * handling. Default if not called is Linux.
> - */
> -#define EXTERNAL_IRQ_POLICY_LINUX	false
> -#define EXTERNAL_IRQ_POLICY_SKIBOOT	true
> -extern void psi_set_external_irq_policy(bool policy);
> -
>  extern struct lock psi_lock;
>  
>  #endif /* __PSI_H */
> diff --git a/platforms/astbmc/common.c b/platforms/astbmc/common.c
> index 85043f3b91e7..15ac231fbdae 100644
> --- a/platforms/astbmc/common.c
> +++ b/platforms/astbmc/common.c
> @@ -465,9 +465,6 @@ void astbmc_early_init(void)
>  	/* Hostboot forgets to populate the PSI BAR */
>  	astbmc_fixup_psi_bar();
>  
> -	/* Send external interrupts to me */
> -	psi_set_external_irq_policy(EXTERNAL_IRQ_POLICY_SKIBOOT);
> -
>  	if (ast_sio_init()) {
>  		if (ast_io_init()) {
>  			astbmc_fixup_uart();
> diff --git a/platforms/astbmc/garrison.c b/platforms/astbmc/garrison.c
> index 1b0f865c54e0..caf6113687be 100644
> --- a/platforms/astbmc/garrison.c
> +++ b/platforms/astbmc/garrison.c
> @@ -258,17 +258,6 @@ static bool garrison_probe(void)
>  	/* Lot of common early inits here */
>  	astbmc_early_init();
>  
> -	/*
> -	 * Override external interrupt policy -> send to Linux
> -	 *
> -	 * On Naples, we get LPC interrupts via the built-in LPC
> -	 * controller demuxer, not an external CPLD. The external
> -	 * interrupt is for other uses, such as the TPM chip, we
> -	 * currently route it to Linux, but we might change that
> -	 * later if we decide we need it.
> -	 */
> -	psi_set_external_irq_policy(EXTERNAL_IRQ_POLICY_LINUX);
> -
>  	/* Fixups until HB get the NPU bindings */
>  	dt_create_npu();
>  
> diff --git a/platforms/astbmc/p8dnu.c b/platforms/astbmc/p8dnu.c
> index a76fbd9dc7bb..e223d158bd97 100644
> --- a/platforms/astbmc/p8dnu.c
> +++ b/platforms/astbmc/p8dnu.c
> @@ -307,17 +307,6 @@ static bool p8dnu_probe(void)
>  	/* Lot of common early inits here */
>  	astbmc_early_init();
>  
> -	/*
> -	 * Override external interrupt policy -> send to Linux
> -	 *
> -	 * On Naples, we get LPC interrupts via the built-in LPC
> -	 * controller demuxer, not an external CPLD. The external
> -	 * interrupt is for other uses, such as the TPM chip, we
> -	 * currently route it to Linux, but we might change that
> -	 * later if we decide we need it.
> -	 */
> -	psi_set_external_irq_policy(EXTERNAL_IRQ_POLICY_LINUX);
> -
>  	/* Fixups until HB get the NPU bindings */
>  	dt_create_npu();
>  
>