[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID:
<SN7PR12MB7201C6B5B64F8847DD6D816D8BF3A@SN7PR12MB7201.namprd12.prod.outlook.com>
Date: Wed, 22 Oct 2025 10:36:28 +0000
From: "Havalige, Thippeswamy" <thippeswamy.havalige@....com>
To: "mani@...nel.org" <mani@...nel.org>
CC: Stefan Roese <stefan.roese@...lbox.org>, 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>, "Musham, Sai
Krishna" <sai.krishna.musham@....com>
Subject: RE: [PATCH v2] PCI: xilinx-xdma: Enable INTx interrupts
[AMD Official Use Only - AMD Internal Distribution Only]
Hi Mani,
> -----Original Message-----
> From: mani@...nel.org <mani@...nel.org>
> Sent: Wednesday, October 22, 2025 4:02 PM
> To: Havalige, Thippeswamy <thippeswamy.havalige@....com>
> Cc: Stefan Roese <stefan.roese@...lbox.org>; 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>
> Subject: Re: [PATCH v2] PCI: xilinx-xdma: Enable INTx interrupts
>
> On Wed, Oct 22, 2025 at 10:08:44AM +0000, Havalige, Thippeswamy wrote:
> > [AMD Official Use Only - AMD Internal Distribution Only]
> >
> > Hi Stefan,
> >
> > > -----Original Message-----
> > > From: Stefan Roese <stefan.roese@...lbox.org>
> > > Sent: Wednesday, October 22, 2025 3:29 PM
> > > To: mani@...nel.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 10/22/25 11:55, mani@...nel.org wrote:
> > > > 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.roese@m
> > > >>>>>>> ailbox.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?
> > >
> > > Yes, this is my best guess right now. I could double-check here, but
> > > IMHO it makes sense to enable it "manually" as done with Ravi's
> > > patch to not rely on this default setup at all.
> > Hardware doesn't enable this bits by default, INTx didn't work since there is a
> miss match in the DT property which doesn't require pci_irqd_intx_xlate.
> >
> > 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>;
> >
>
> Ok. This makes me believe that we do not need Stefan's patch [1] and need
> just this patch from Ravi.
>
> - Mani
>
> [1] https://lore.kernel.org/linux-pci/20251021154322.973640-1-
> stefan.roese@...lbox.org/
We even don’t need ravi patch, as we have tested this at our end it works fine by just updating interrupt-map
Property. We need to now understand the difference in design.
>
> --
> மணிவண்ணன் சதாசிவம்
Powered by blists - more mailing lists