[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID:
<SN7PR12MB72012E58AAD2E8C2D7E068B28BF3A@SN7PR12MB7201.namprd12.prod.outlook.com>
Date: Wed, 22 Oct 2025 10:13:27 +0000
From: "Havalige, Thippeswamy" <thippeswamy.havalige@....com>
To: Stefan Roese <stefan.roese@...lbox.org>, "mani@...nel.org"
<mani@...nel.org>
CC: Bjorn Helgaas <helgaas@...nel.org>, "Bandi, Ravi Kumar"
<ravib@...zon.com>, "lpieralisi@...nel.org" <lpieralisi@...nel.org>,
"bhelgaas@...gle.com" <bhelgaas@...gle.com>, "linux-pci@...r.kernel.org"
<linux-pci@...r.kernel.org>, "kwilczynski@...nel.org"
<kwilczynski@...nel.org>, "robh@...nel.org" <robh@...nel.org>, "Simek,
Michal" <michal.simek@....com>, "linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>, "stable@...r.kernel.org"
<stable@...r.kernel.org>, Sean Anderson <sean.anderson@...ux.dev>,
"Yeleswarapu, Nagaradhesh" <nagaradhesh.yeleswarapu@....com>
Subject: RE: [PATCH v2] PCI: xilinx-xdma: Enable INTx interrupts
[AMD Official Use Only - AMD Internal Distribution Only]
> -----Original Message-----
> From: Stefan Roese <stefan.roese@...lbox.org>
> Sent: Wednesday, October 22, 2025 3:41 PM
> To: Havalige, Thippeswamy <thippeswamy.havalige@....com>;
> mani@...nel.org
> Cc: Bjorn Helgaas <helgaas@...nel.org>; Bandi, Ravi Kumar
> <ravib@...zon.com>; lpieralisi@...nel.org; bhelgaas@...gle.com; linux-
> pci@...r.kernel.org; kwilczynski@...nel.org; robh@...nel.org; Simek, Michal
> <michal.simek@....com>; linux-arm-kernel@...ts.infradead.org; linux-
> kernel@...r.kernel.org; stable@...r.kernel.org; Sean Anderson
> <sean.anderson@...ux.dev>; Yeleswarapu, Nagaradhesh
> <nagaradhesh.yeleswarapu@....com>
> Subject: Re: [PATCH v2] PCI: xilinx-xdma: Enable INTx interrupts
>
> On 10/22/25 12:04, Havalige, Thippeswamy wrote:
> > [AMD Official Use Only - AMD Internal Distribution Only]
> >
> > Hi Mani,
> >
> >> -----Original Message-----
> >> From: mani@...nel.org <mani@...nel.org>
> >> Sent: Wednesday, October 22, 2025 3:25 PM
> >> To: Stefan Roese <stefan.roese@...lbox.org>
> >> Cc: Bjorn Helgaas <helgaas@...nel.org>; Bandi, Ravi Kumar
> >> <ravib@...zon.com>; Havalige, Thippeswamy
> >> <thippeswamy.havalige@....com>; lpieralisi@...nel.org;
> >> bhelgaas@...gle.com; linux-pci@...r.kernel.org;
> >> kwilczynski@...nel.org; robh@...nel.org; Simek, Michal
> >> <michal.simek@....com>; linux-arm- kernel@...ts.infradead.org;
> >> linux-kernel@...r.kernel.org; stable@...r.kernel.org; Sean Anderson
> >> <sean.anderson@...ux.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@...nel.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@...r.kernel.org
> >>>>>>>>> Signed-off-by: Ravi Kumar Bandi <ravib@...zon.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@...lbox.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.
Yes, you need to manually fix this auto-generated DT for now. Will take this discussion offline.
>
> Thanks,
> Stefan
Powered by blists - more mailing lists