| Message ID | 20190905105042.27526-4-oohall@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v3,1/4] hw/psi: Add chip ID to interrupt names | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch master (7b12d5489fcfd73ef7ec0cb27eff7f8a5f13b238) |
| snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot | success | Test snowpatch/job/snowpatch-skiboot on branch master |
| snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot-dco | success | Signed-off-by present |
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@gmail.com> P9 returns an extra IRQ_ATTR_TYPE_LSI attribute but that's minor. Reviewed-by: Cédric Le Goater <clg@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(); > >
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();
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@gmail.com> --- 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(-)