| Message ID | 20240528062045.624906-2-npiggin@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ppc/pnv: LPC interrupt fixes | expand |
Reviewed-by: Glenn Miles <milesg@linux.ibm.com> Thanks, Glenn On Tue, 2024-05-28 at 16:20 +1000, Nicholas Piggin wrote: > From: Glenn Miles <milesg@linux.vnet.ibm.com> > > The LPC HC irq status register bits are set when an LPC IRQSER input > is > asserted. These irq status bits drive the PSI irq to the CPU > interrupt > controller. The LPC HC irq status bits are cleared by software > writing > to the register with 1's for the bits to clear. > > Existing register write was clearing the irq status bits even when > the > input was asserted, this results in interrupts being lost. > > This fix changes the behavior to keep track of the device IRQ status > in internal state that is separate from the irq status register, and > only allowing the irq status bits to be cleared if the associated > input is not asserted. > > [np: rebased before P9 PSI SERIRQ patch, adjust changelog/comments] > Signed-off-by: Glenn Miles <milesg@linux.ibm.com> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > --- > include/hw/ppc/pnv_lpc.h | 3 +++ > hw/ppc/pnv_lpc.c | 22 +++++++++++++++++++--- > 2 files changed, 22 insertions(+), 3 deletions(-) > > diff --git a/include/hw/ppc/pnv_lpc.h b/include/hw/ppc/pnv_lpc.h > index 5d22c45570..97c6872c3f 100644 > --- a/include/hw/ppc/pnv_lpc.h > +++ b/include/hw/ppc/pnv_lpc.h > @@ -73,6 +73,9 @@ struct PnvLpcController { > uint32_t opb_irq_pol; > uint32_t opb_irq_input; > > + /* LPC device IRQ state */ > + uint32_t lpc_hc_irq_inputs; > + > /* LPC HC registers */ > uint32_t lpc_hc_fw_seg_idsel; > uint32_t lpc_hc_fw_rd_acc_size; > diff --git a/hw/ppc/pnv_lpc.c b/hw/ppc/pnv_lpc.c > index d692858bee..252690dcaa 100644 > --- a/hw/ppc/pnv_lpc.c > +++ b/hw/ppc/pnv_lpc.c > @@ -505,7 +505,14 @@ static void lpc_hc_write(void *opaque, hwaddr > addr, uint64_t val, > pnv_lpc_eval_irqs(lpc); > break; > case LPC_HC_IRQSTAT: > - lpc->lpc_hc_irqstat &= ~val; > + /* > + * This register is write-to-clear for the IRQSER (LPC > device IRQ) > + * status. However if the device has not de-asserted its > interrupt > + * that will just raise this IRQ status bit again. Model > this by > + * keeping track of the inputs and only clearing if the > inputs are > + * deasserted. > + */ > + lpc->lpc_hc_irqstat &= ~(val & ~lpc->lpc_hc_irq_inputs); > pnv_lpc_eval_irqs(lpc); > break; > case LPC_HC_ERROR_ADDRESS: > @@ -803,11 +810,20 @@ static void pnv_lpc_isa_irq_handler_cpld(void > *opaque, int n, int level) > static void pnv_lpc_isa_irq_handler(void *opaque, int n, int level) > { > PnvLpcController *lpc = PNV_LPC(opaque); > + uint32_t irq_bit = LPC_HC_IRQ_SERIRQ0 >> n; > > - /* The Naples HW latches the 1 levels, clearing is done by SW */ > if (level) { > - lpc->lpc_hc_irqstat |= LPC_HC_IRQ_SERIRQ0 >> n; > + lpc->lpc_hc_irq_inputs |= irq_bit; > + > + /* > + * The LPC HC in Naples and later latches LPC IRQ into a bit > field in > + * the IRQSTAT register, and that drives the PSI IRQ to the IC. > + * Software clears this bit manually (see LPC_HC_IRQSTAT > handler). > + */ > + lpc->lpc_hc_irqstat |= irq_bit; > pnv_lpc_eval_irqs(lpc); > + } else { > + lpc->lpc_hc_irq_inputs &= ~irq_bit; > } > } >
diff --git a/include/hw/ppc/pnv_lpc.h b/include/hw/ppc/pnv_lpc.h index 5d22c45570..97c6872c3f 100644 --- a/include/hw/ppc/pnv_lpc.h +++ b/include/hw/ppc/pnv_lpc.h @@ -73,6 +73,9 @@ struct PnvLpcController { uint32_t opb_irq_pol; uint32_t opb_irq_input; + /* LPC device IRQ state */ + uint32_t lpc_hc_irq_inputs; + /* LPC HC registers */ uint32_t lpc_hc_fw_seg_idsel; uint32_t lpc_hc_fw_rd_acc_size; diff --git a/hw/ppc/pnv_lpc.c b/hw/ppc/pnv_lpc.c index d692858bee..252690dcaa 100644 --- a/hw/ppc/pnv_lpc.c +++ b/hw/ppc/pnv_lpc.c @@ -505,7 +505,14 @@ static void lpc_hc_write(void *opaque, hwaddr addr, uint64_t val, pnv_lpc_eval_irqs(lpc); break; case LPC_HC_IRQSTAT: - lpc->lpc_hc_irqstat &= ~val; + /* + * This register is write-to-clear for the IRQSER (LPC device IRQ) + * status. However if the device has not de-asserted its interrupt + * that will just raise this IRQ status bit again. Model this by + * keeping track of the inputs and only clearing if the inputs are + * deasserted. + */ + lpc->lpc_hc_irqstat &= ~(val & ~lpc->lpc_hc_irq_inputs); pnv_lpc_eval_irqs(lpc); break; case LPC_HC_ERROR_ADDRESS: @@ -803,11 +810,20 @@ static void pnv_lpc_isa_irq_handler_cpld(void *opaque, int n, int level) static void pnv_lpc_isa_irq_handler(void *opaque, int n, int level) { PnvLpcController *lpc = PNV_LPC(opaque); + uint32_t irq_bit = LPC_HC_IRQ_SERIRQ0 >> n; - /* The Naples HW latches the 1 levels, clearing is done by SW */ if (level) { - lpc->lpc_hc_irqstat |= LPC_HC_IRQ_SERIRQ0 >> n; + lpc->lpc_hc_irq_inputs |= irq_bit; + + /* + * The LPC HC in Naples and later latches LPC IRQ into a bit field in + * the IRQSTAT register, and that drives the PSI IRQ to the IC. + * Software clears this bit manually (see LPC_HC_IRQSTAT handler). + */ + lpc->lpc_hc_irqstat |= irq_bit; pnv_lpc_eval_irqs(lpc); + } else { + lpc->lpc_hc_irq_inputs &= ~irq_bit; } }