Re: [PATCH] PCI: pcie-xilinx-dma-pl: Fix off-by-one INTx IRQ handling

[Sean Anderson <sean.anderson@linux.dev>]

On 10/21/25 09:39, Stefan Roese wrote:
> While testing with NVMe drives connected to the Versal QDMA PL PCIe RP
> on our platform I noticed that with MSI disabled (e.g. via pci=nomsi)
> the NVMe interrupts are not delivered to the host CPU resulting in
> timeouts while probing.
> 
> Debugging has shown, that the hwirq numbers passed to this device driver
> (1...4, 1=INTA etc) need to get adjusted to match the numbers in the
> controller registers bits (0...3). This patch now correctly matches the
> hwirq number to the PCIe controller register bits.
> 
> Signed-off-by: Stefan Roese <stefan.roese@mailbox.org>
> Cc: Sean Anderson <sean.anderson@linux.dev>

I assume I'm CC'd because of commit 02a370c4fc0f ("PCI: xilinx-nwl: Fix
off-by-one in INTx IRQ handler"), which does the exact opposite of this
patch.

And I'm rather confused as to how you determined that hwirq is
one-based, since it seems to come directly from

	for_each_set_bit(i, &val, PCI_NUM_INTX)
		generic_handle_domain_irq(port->intx_domain, i);

which is zero-based.

--Sean

> Cc: Manivannan Sadhasivam <mani@kernel.org>
> Cc: Ravi Kumar Bandi <ravib@amazon.com>
> Cc: Thippeswamy Havalige <thippeswamy.havalige@amd.com>
> Cc: Michal Simek <michal.simek@amd.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> ---
>  drivers/pci/controller/pcie-xilinx-dma-pl.c | 21 ++++++++++++++++++---
>  1 file changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pci/controller/pcie-xilinx-dma-pl.c b/drivers/pci/controller/pcie-xilinx-dma-pl.c
> index 84888eda990b2..5cca9d018bc89 100644
> --- a/drivers/pci/controller/pcie-xilinx-dma-pl.c
> +++ b/drivers/pci/controller/pcie-xilinx-dma-pl.c
> @@ -331,7 +331,12 @@ static void xilinx_mask_intx_irq(struct irq_data *data)
>  	unsigned long flags;
>  	u32 mask, val;
>  
> -	mask = BIT(data->hwirq + XILINX_PCIE_DMA_IDRN_SHIFT);
> +	/*
> +	 * INTx hwirq: 1=INTA, 2=INTB, 3=INTC, 4=INTD
> +	 * In the controller regs this is represented in bits 0...3, so we need
> +	 * to subtract 1 here
> +	 */
> +	mask = BIT(data->hwirq + XILINX_PCIE_DMA_IDRN_SHIFT - 1);
>  	raw_spin_lock_irqsave(&port->lock, flags);
>  	val = pcie_read(port, XILINX_PCIE_DMA_REG_IDRN_MASK);
>  	pcie_write(port, (val & (~mask)), XILINX_PCIE_DMA_REG_IDRN_MASK);
> @@ -344,7 +349,12 @@ static void xilinx_unmask_intx_irq(struct irq_data *data)
>  	unsigned long flags;
>  	u32 mask, val;
>  
> -	mask = BIT(data->hwirq + XILINX_PCIE_DMA_IDRN_SHIFT);
> +	/*
> +	 * INTx hwirq: 1=INTA, 2=INTB, 3=INTC, 4=INTD
> +	 * In the controller regs this is represented in bits 0...3, so we need
> +	 * to subtract 1 here
> +	 */
> +	mask = BIT(data->hwirq + XILINX_PCIE_DMA_IDRN_SHIFT - 1);
>  	raw_spin_lock_irqsave(&port->lock, flags);
>  	val = pcie_read(port, XILINX_PCIE_DMA_REG_IDRN_MASK);
>  	pcie_write(port, (val | mask), XILINX_PCIE_DMA_REG_IDRN_MASK);
> @@ -620,8 +630,13 @@ static irqreturn_t xilinx_pl_dma_pcie_intx_flow(int irq, void *args)
>  	val = FIELD_GET(XILINX_PCIE_DMA_IDRN_MASK,
>  			pcie_read(port, XILINX_PCIE_DMA_REG_IDRN));
>  
> +	/*
> +	 * INTx hwirq: 1=INTA, 2=INTB, 3=INTC, 4=INTD
> +	 * In the controller regs this is represented in bits 0...3, so we need
> +	 * to add 1 here again for the registered handler
> +	 */
>  	for_each_set_bit(i, &val, PCI_NUM_INTX)
> -		generic_handle_domain_irq(port->intx_domain, i);
> +		generic_handle_domain_irq(port->intx_domain, i + 1);
>  	return IRQ_HANDLED;
>  }
>