Re: [PATCH 06/13] PCI: aardvark: Do not clear status bits of masked interrupts

Lorenzo Pieralisi
2021-10-04T14:06:59Z

[+Marc - always better to have his eyes on IRQ handling code]

On Fri, Oct 01, 2021 at 09:58:49PM +0200, Marek Behún wrote:
> From: Pali Rohár <pali@kernel.org>
>
> It is incorrect to clear status bits of masked interrupts.
>
> The aardvark driver clears all status interrupt bits if no unmasked
> status bit is set. Masked bits should never be cleared.
>
> Fixes: 8c39d710363c ("PCI: aardvark: Add Aardvark PCI host controller driver")
> Signed-off-by: Pali Rohár <pali@kernel.org>
> Reviewed-by: Marek Behún <kabel@kernel.org>
> Signed-off-by: Marek Behún <kabel@kernel.org>
> Cc: stable@vger.kernel.org
> ---
> drivers/pci/controller/pci-aardvark.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
> index d5d6f92e5143..e4986806a189 100644
> --- a/drivers/pci/controller/pci-aardvark.c
> +++ b/drivers/pci/controller/pci-aardvark.c
> @@ -1295,11 +1295,8 @@ static void advk_pcie_handle_int(struct advk_pcie *pcie)
> isr1_mask = advk_readl(pcie, PCIE_ISR1_MASK_REG);
> isr1_status = isr1_val & ((~isr1_mask) & PCIE_ISR1_ALL_MASK);
>
> - if (!isr0_status && !isr1_status) {
> - advk_writel(pcie, isr0_val, PCIE_ISR0_REG);
> - advk_writel(pcie, isr1_val, PCIE_ISR1_REG);

This looks fine - on the other hand if no interrupt is set in the status
registers (that are filtered with the masks) we are dealing with a
spurious IRQ right ? Just gauging how severe this is.

Lorenzo

> + if (!isr0_status && !isr1_status)
> return;
> - }
>
> /* Process MSI interrupts */
> if (isr0_status & PCIE_ISR0_MSI_INT_PENDING)
> --
> 2.32.0
>