Re: [PATCH v2] PCI: xilinx-xdma: Enable INTx interrupts

[Stefan Roese <stefan.roese@mailbox.org>]

On 10/22/25 12:04, Havalige, Thippeswamy wrote:
> [AMD Official Use Only - AMD Internal Distribution Only]
> 
> Hi Mani,
> 
>> -----Original Message-----
>> From: mani@kernel.org <mani@kernel.org>
>> Sent: Wednesday, October 22, 2025 3:25 PM
>> To: Stefan Roese <stefan.roese@mailbox.org>
>> Cc: Bjorn Helgaas <helgaas@kernel.org>; Bandi, Ravi Kumar
>> <ravib@amazon.com>; Havalige, Thippeswamy
>> <thippeswamy.havalige@amd.com>; lpieralisi@kernel.org;
>> bhelgaas@google.com; linux-pci@vger.kernel.org; kwilczynski@kernel.org;
>> robh@kernel.org; Simek, Michal <michal.simek@amd.com>; linux-arm-
>> kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
>> stable@vger.kernel.org; Sean Anderson <sean.anderson@linux.dev>
>> Subject: Re: [PATCH v2] PCI: xilinx-xdma: Enable INTx interrupts
>>
>> On Wed, Oct 22, 2025 at 08:59:19AM +0200, Stefan Roese wrote:
>>> Hi Bjorn,
>>> Hi Ravi,
>>>
>>> On 10/21/25 23:28, Bjorn Helgaas wrote:
>>>> On Tue, Oct 21, 2025 at 08:55:41PM +0000, Bandi, Ravi Kumar wrote:
>>>>>> On Tue, Oct 21, 2025 at 05:46:17PM +0000, Bandi, Ravi Kumar wrote:
>>>>>>>> On Oct 21, 2025, at 10:23 AM, Bjorn Helgaas <helgaas@kernel.org>
>> wrote:
>>>>>>>> On Sat, Sep 20, 2025 at 10:52:32PM +0000, Ravi Kumar Bandi
>> wrote:
>>>>>>>>> The pcie-xilinx-dma-pl driver does not enable INTx
>>>>>>>>> interrupts after initializing the port, preventing INTx
>>>>>>>>> interrupts from PCIe endpoints from flowing through the
>>>>>>>>> Xilinx XDMA root port bridge. This issue affects kernel 6.6.0 and
>> later versions.
>>>>>>>>>
>>>>>>>>> This patch allows INTx interrupts generated by PCIe
>>>>>>>>> endpoints to flow through the root port. Tested the fix on
>>>>>>>>> a board with two endpoints generating INTx interrupts.
>>>>>>>>> Interrupts are properly detected and serviced. The
>>>>>>>>> /proc/interrupts output
>>>>>>>>> shows:
>>>>>>>>>
>>>>>>>>> [...]
>>>>>>>>> 32:        320          0  pl_dma:RC-Event  16 Level     400000000.axi-
>> pcie, azdrv
>>>>>>>>> 52:        470          0  pl_dma:RC-Event  16 Level     500000000.axi-
>> pcie, azdrv
>>>>>>>>> [...]
>>>
>>> First a comment on this IRQ logging:
>>>
>>> These lines do NOT refer to the INTx IRQ(s) but the controller
>>> internal "events" (errors etc). Please see this log for INTx on my
>>> Versal platform with pci_irqd_intx_xlate added:
>>>
>>>   24:          0          0  pl_dma:RC-Event   0 Level     LINK_DOWN
>>>   25:          0          0  pl_dma:RC-Event   3 Level     HOT_RESET
>>>   26:          0          0  pl_dma:RC-Event   8 Level     CFG_TIMEOUT
>>>   27:          0          0  pl_dma:RC-Event   9 Level     CORRECTABLE
>>>   28:          0          0  pl_dma:RC-Event  10 Level     NONFATAL
>>>   29:          0          0  pl_dma:RC-Event  11 Level     FATAL
>>>   30:          0          0  pl_dma:RC-Event  20 Level     SLV_UNSUPP
>>>   31:          0          0  pl_dma:RC-Event  21 Level     SLV_UNEXP
>>>   32:          0          0  pl_dma:RC-Event  22 Level     SLV_COMPL
>>>   33:          0          0  pl_dma:RC-Event  23 Level     SLV_ERRP
>>>   34:          0          0  pl_dma:RC-Event  24 Level     SLV_CMPABT
>>>   35:          0          0  pl_dma:RC-Event  25 Level     SLV_ILLBUR
>>>   36:          0          0  pl_dma:RC-Event  26 Level     MST_DECERR
>>>   37:          0          0  pl_dma:RC-Event  27 Level     MST_SLVERR
>>>   38:         94          0  pl_dma:RC-Event  16 Level     84000000.axi-pcie
>>>   39:         94          0  pl_dma:INTx   0 Level     nvme0q0, nvme0q1
>>>
>>> The last line shows the INTx IRQs here ('pl_dma:INTx' vs 'pl_dma:RC-
>>> Event').
>>>
>>> More below...
>>>
>>>>>>>>>
>>>>>>>>> Changes since v1::
>>>>>>>>> - Fixed commit message per reviewer's comments
>>>>>>>>>
>>>>>>>>> Fixes: 8d786149d78c ("PCI: xilinx-xdma: Add Xilinx XDMA
>>>>>>>>> Root Port driver")
>>>>>>>>> Cc: stable@vger.kernel.org
>>>>>>>>> Signed-off-by: Ravi Kumar Bandi <ravib@amazon.com>
>>>>>>>>
>>>>>>>> Hi Ravi, obviously you tested this, but I don't know how to
>>>>>>>> reconcile this with Stefan's INTx fix at
>>>>>>>> https://lore.kernel.org/r/20251021154322.973640-1-stefan.roe
>>>>>>>> se@mailbox.org
>>>>>>>>
>>>>>>>> Does Stefan's fix need to be squashed into this patch?
>>>>>>>
>>>>>>> Sure, we can squash Stefan’s fix into this.
>>>>>>
>>>>>> I know we *can* squash them.
>>>>>>
>>>>>> I want to know why things worked for you and Stefan when they
>>>>>> *weren't* squashed:
>>>>>>
>>>>>>    - Why did INTx work for you even without Stefan's patch.  Did you
>>>>>>      get INTx interrupts but not the right ones, e.g., did the device
>>>>>>      signal INTA but it was received as INTB?
>>>>>
>>>>> I saw that interrupts were being generated by the endpoint device,
>>>>> but I didn’t specifically check if they were correctly translated
>>>>> in the controller. I noticed that the new driver wasn't explicitly
>>>>> enabling the interrupts, so my first approach was to enable them,
>>>>> which helped the interrupts flow through.
>>>>
>>>> OK, I'll assume the interrupts happened but the driver might not
>>>> have been able to handle them correctly, e.g., it was prepared for
>>>> INTA but got INTB or similar.
>>>>
>>>>>>    - Why did Stefan's patch work for him even without your patch.  How
>>>>>>      could Stefan's INTx work without the CSR writes to enable
>>>>>>      interrupts?
>>>>>
>>>>> I'm not entirely sure if there are any other dependencies in the
>>>>> FPGA bitstream. I'll investigate further and get back to you.
>>>>
>>>> Stefan clarified in a private message that he had applied your patch
>>>> first, so this mystery is solved.
>>>
>>> Yes. I applied Ravi's patch first and still got no INTx delivered to
>>> the nvme driver. That's what me triggered to dig deeper here and
>>> resulted in this v2 patch with pci_irqd_intx_xlate added.
>>>
>>> BTW:
>>> I re-tested just now w/o Ravi's patch and the INTx worked. Still I
>>> think Ravi's patch is valid and should be applied...
>>
>> How come INTx is working without the patch from Ravi which enabled INTx
>> routing in the controller? Was it enabled by default in the hardware?
> 
> Can you please cross-check the interrupt-map property in the device tree? Currently, the driver isn’t translating (pci_irqd_intx_xlate) the INTx number.
> 
> Here’s required DT property:
> 
> interrupt-map = <0 0 0 1 &pcie_intc_0 0>,
>                  <0 0 0 2 &pcie_intc_0 1>,
>                  <0 0 0 3 &pcie_intc_0 2>,
>                  <0 0 0 4 &pcie_intc_0 3>;

Here the auto-generated DT property (Vivado 2025.1) for our design:

         interrupt-map = <0 0 0 1 &psv_pcie_intc_0 1>,
                         <0 0 0 2 &psv_pcie_intc_0 2>,
                         <0 0 0 3 &psv_pcie_intc_0 3>,
                         <0 0 0 4 &psv_pcie_intc_0 4>;

So we should manually "fix" the auto-generated DT instead? I would
rather like to skip such a step, as this is error prone with frequent
updates from the FPGA bistream design.

Thanks,
Stefan