Reviewed-by: Glenn Miles <mil...@linux.ibm.com>
Thanks,
Glenn
On Tue, 2024-05-28 at 16:20 +1000, Nicholas Piggin wrote:
> From: Glenn Miles <mil...@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 <mil...@linux.ibm.com>
> Signed-off-by: Nicholas Piggin <npig...@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;
> }
> }
>
