From: Murali Karicheri <m-karicheri2@ti.com>
To: Po Liu <po.liu@nxp.com>, Bjorn Helgaas <helgaas@kernel.org>
Cc: "linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
Arnd Bergmann <arnd@arndb.de>, Roy Zang <roy.zang@nxp.com>,
Marc Zyngier <marc.zyngier@arm.com>,
Stuart Yoder <stuart.yoder@nxp.com>,
Yang-Leo Li <leoyang.li@nxp.com>,
Minghuan Lian <minghuan.lian@nxp.com>,
Bjorn Helgaas <bhelgaas@google.com>,
Shawn Guo <shawnguo@kernel.org>, Mingkai Hu <mingkai.hu@nxp.com>,
Rob Herring <robh@kernel.org>
Subject: Re: [PATCH 2/2] aer: add support aer interrupt with none MSI/MSI-X/INTx mode
Date: Mon, 6 Jun 2016 10:01:44 -0400 [thread overview]
Message-ID: <57558248.4010502@ti.com> (raw)
In-Reply-To: <DB5PR0401MB1702AF829693255DFC347448925C0@DB5PR0401MB1702.eurprd04.prod.outlook.com>
On 06/06/2016 03:32 AM, Po Liu wrote:
> Hi Bjorn,
> I confirm we met same problem with KeyStone base on DesignWare design.
>
>
> Best regards,
> Liu Po
>
>> -----Original Message-----
>> From: Bjorn Helgaas [mailto:helgaas@kernel.org]
>> Sent: Saturday, June 04, 2016 11:49 AM
>> To: Murali Karicheri
>> Cc: Po Liu; linux-pci@vger.kernel.org; linux-arm-
>> kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
>> devicetree@vger.kernel.org; Arnd Bergmann; Roy Zang; Marc Zyngier;
>> Stuart Yoder; Yang-Leo Li; Minghuan Lian; Bjorn Helgaas; Shawn Guo;
>> Mingkai Hu; Rob Herring
>> Subject: Re: [PATCH 2/2] aer: add support aer interrupt with none
>> MSI/MSI-X/INTx mode
>>
>> On Fri, Jun 03, 2016 at 01:31:11PM -0400, Murali Karicheri wrote:
>> > Po,
>> >
>> > Sorry to hijack your discussion, but the problem seems to be same for
>> > Keystone PCI controller which is also designware (old version) based.
>> >
>> > On 06/03/2016 12:09 AM, Bjorn Helgaas wrote:
>> > > On Thu, Jun 02, 2016 at 11:37:28AM -0400, Murali Karicheri wrote:
>> > >> On 06/02/2016 09:55 AM, Bjorn Helgaas wrote:
>> > >>> On Thu, Jun 02, 2016 at 05:01:19AM +0000, Po Liu wrote:
>> > >>>>> -----Original Message-----
>> > >>>>> From: Bjorn Helgaas [mailto:helgaas@kernel.org]
>> > >>>>> Sent: Thursday, June 02, 2016 11:48 AM
>> > >>>>> To: Po Liu
>> > >>>>> Cc: linux-pci@vger.kernel.org;
>> > >>>>> linux-arm-kernel@lists.infradead.org;
>> > >>>>> linux-kernel@vger.kernel.org; devicetree@vger.kernel.org; Arnd
>> > >>>>> Bergmann; Roy Zang; Marc Zyngier; Stuart Yoder; Yang-Leo Li;
>> > >>>>> Minghuan Lian; Bjorn Helgaas; Shawn Guo; Mingkai Hu; Rob
>> > >>>>> Herring
>> > >>>>> Subject: Re: [PATCH 2/2] aer: add support aer interrupt with
>> > >>>>> none MSI/MSI-X/INTx mode
>> > >>>>>
>> > >>>>> [+cc Rob]
>> > >>>>>
>> > >>>>> Hi Po,
>> > >>>>>
>> > >>>>> On Thu, May 26, 2016 at 02:00:06PM +0800, Po Liu wrote:
>> > >>>>> > On some platforms, root port doesn't support MSI/MSI-X/INTx
>> in RC mode.
>> > >>>>> > When chip support the aer interrupt with none MSI/MSI-X/INTx
>> > >>>>> mode, > maybe there is interrupt line for aer pme etc. Search
>> > >>>>> the interrupt > number in the fdt file.
>> > >>>>>
>> > >>>>> My understanding is that AER interrupt signaling can be done
>> > >>>>> via INTx, MSI, or MSI-X (PCIe spec r3.0, sec 6.2.4.1.2).
>> > >>>>> Apparently your device doesn't support MSI or MSI-X. Are you
>> > >>>>> saying it doesn't support INTx either? How is the interrupt
>> you're requesting here different from INTx?
>> > >>>>
>> > >>>> Layerscape use none of MSI or MSI-X or INTx to indicate the
>> > >>>> devices or root error in RC mode. But use an independent SPI
>> > >>>> interrupt(arm interrupt controller) line.
>> > >>>
>> > >>> The Root Port is a PCI device and should follow the normal PCI
>> > >>> rules for interrupts. As far as I understand, that means it
>> > >>> should use MSI, MSI-X, or INTx. If your Root Port doesn't use MSI
>> > >>> or MSI-X, it should use INTx, the PCI_INTERRUPT_PIN register
>> > >>> should tell us which (INTA/ INTB/etc.), and
>> PCI_COMMAND_INTX_DISABLE should work to disable it.
>> > >>> That's all from the PCI point of view, of course.
>> > >>
>> > >> I am faced with the same issue on Keystone PCI hardware and it has
>> > >> been on my TODO list for quite some time. Keystone PCI hardware
>> > >> also doesn't use MSI or MSI-X or INTx for reporting errors received
>> > >> at the root port, but use a platform interrupt instead (not
>> > >> complaint to PCI standard as per PCI base spec). So I would need
>> > >> similar change to have the error interrupt passed to the aer
>> > >> driver. So there are hardware out there like Keystone which
>> requires to support this through platform IRQ.
>> > >
>> > > This is not a new area of the spec, and it's hard for me to believe
>> > > that these two new PCIe controllers are both broken the same way
>> > > (although I guess both are DesignWare-based, so maybe this is the
>> > > same underlying problem in both cases?). I think it's more likely
>> > > that we just haven't figured out the right way to describe this in
>> the DT.
>> >
>> > Keystone is using an older version of the designware IP and it
>> > implements all of the interrupts in the application register space
>> > unlike other newer version of the hardware. So I assume, the version
>> > used on Layerscape is also an older version and the both have same
>> > issue in terms of non standard platform interrupt used for error
>> reporting.
>> >
>> > > I assume you have a Root Port with an AER capability, no MSI
>> > > capability, and no MSI-X capability, right?
>> >
>> > Has AER capability and both MSI and INTx (legacy) capability
>> >
>> > > What does its Interrupt
>> > > Pin register contain? If it's zero, it doesn't use INTx either, so
>> > > according to the spec it should generate no interrupts.
>> > >
>> > At address offset 0x3C by default has a value of 1, but it is writable
>> > by software. So default is INTx A.
>>
>> 0x3c is the Interrupt *Line*, which is read/write. The Interrupt
>> *Pin* is at 0x3d and should be read-only.
>>
You are right. But default is 1 at this address.
>> Does your Keystone driver support MSI? If so, since your Root Port
>> supports MSI, I would think we would use that by default, and the INTx
>> stuff wouldn't even matter.
>
> Layerscape is also shows "Both message signaled interrupts (MSI) and legacy INTx are supported."
> But both of them not work for AER interrupt when devices or root port report aer error.
> But another GIC interrupt line do.
Same with Keystone. Even though both MSI and INTx are supported
error interrupt at root port is reported on a different interrupt
line than MSI/INTx. So for Power Management event interrupt is also
different line.
Murali
>
>>
>> > > But if Interrupt Pin is non-zero, that means the Root Port should be
>> > > able to generate virtual INTx interrupts. Presumably the Root
>> > > Complex connects those interrupts to something; maybe to your
>> > > platform interrupt?
>> >
>> > Probably that is what is happening. Both Power management and Error
>> > interrupts are raised on platform interrupt lines.
>> >
>> > 12 Error Interrupts
>> >
>> > [0] System error (OR of fatal, nonfatal, correctable errors) (RC mode
>> > only) [1] PCIe fatal error (RC mode only) [2] PCIe non-fatal error (RC
>> > mode only) [3] PCIe correctable error (RC mode only) [4] AXI Error due
>> > to fatal condition in AXI bridge (EP/RC modes) [5] PCIe advanced error
>> > (RC mode only)
>> >
>> > 13 Power management and reset event interrupts
>> >
>> > [0] Power management turn-off message interrupt (EP mode only) [1]
>> > Power management ack message interrupt (RC mode only) [2] Power
>> > management event interrupt (RC mode only) [3] Link request reset
>> > interrupt (hot reset or link down) (RC mode only)
>> >
>> > > PCI doesn't say anything about an interrupt after it leaves the Root
>> > > Complex, so the fact that it's connected to a "platform interrupt"
>> > > or "SPI interrupt" or "IOAPIC interrupt" doesn't make it non-
>> compliant.
>> > > Shouldn't we be able to use the interrupt-map and related DT
>> > > properties to express the fact that Root Port virtual INTx is routed
>> > > to platform interrupt Y, even without any special-case code in
>> > > portdrv?
>> >
>> > My understanding is if RC also raise interrupt on INTx, then below map
>> > make sense, where either EP or RC can raise interrupt and the line
>> > will be muxed for interrupt from EP or RC port.
>>
>> I'm sorry, I didn't quite catch your meaning here, so let me try to
>> clarify some terminology. Maybe we'll eventually blunder into a common
>> understanding :)
>>
>> INTx is a PCI concept and only means something in the PCI hierarchy.
>> The RC would *receive* virtual INTx interrupts from the PCI hierarchy
>> and turn them into some platform-specific interrupt (not INTx) on the
>> upstream side.
>>
>> So strictly speaking, the RC might raise platform-specific interrupts
>> when it receives INTx interrupts, but it would not raise any INTx
>> interrupts itself.
>>
>> > Here is the DT entry in PCIE keystone for Legacy interrupt mapping to
>> > host interrupt.
>> > #interrupt-cells = <1>;
>> > interrupt-map-mask = <0 0 0 7>;
>> > interrupt-map = <0 0 0 1 &pcie_intc0 0>, /*
>> INT A */
>> > <0 0 0 2 &pcie_intc0 1>, /*
>> INT B */
>> > <0 0 0 3 &pcie_intc0 2>, /*
>> INT C */
>> > <0 0 0 4 &pcie_intc0 3>; /*
>> > INT D */
>>
>> If I understand correctly, this is the mapping from the PCI world (INTA,
>> INTB, etc.) to the platform-specific world (pcie_intc0 0, etc.)
>>
>> If a Root Port raises a virtual INTA, the RC should turn it into the
>> corresponding platform interrupt, i.e., GIC_SPI 48 from the description
>> below.
>>
>> > And then
>> >
>> > pcie_intc0: legacy-interrupt-controller {
>> > interrupt-controller;
>> > #interrupt-cells = <1>;
>> > interrupt-parent = <&gic>;
>> > interrupts = <GIC_SPI 48
>> IRQ_TYPE_EDGE_RISING>,
>> > <GIC_SPI 49
>> IRQ_TYPE_EDGE_RISING>,
>> > <GIC_SPI 50
>> IRQ_TYPE_EDGE_RISING>,
>> > <GIC_SPI 51
>> IRQ_TYPE_EDGE_RISING>;
>> > };
>> >
>> > So if RC interrupt for Power management and Error interrupt is a
>> > separate line, how can software describe that using the above
>> interrupt map?
>>
>> I don't know anything about interrupts the RC might generate on its own
>> behalf. A lot of the RC behavior is not specified by the PCIe spec
>> because the RC is on the border between the upstream platform-specific
>> stuff and the downstream PCIe stuff. Is there something in the PCIe
>> spec about the power management and error interrupts you're talking
>> about? Of maybe you can point me to a section of the Keystone spec that
>> talks about interrupts generated by the RC?
>
> Below is one of the PCIE controller interrupts list:
>
> 142 PEX1 INTA
> 143 PEX1 INTB
> 144 PEX1 INTC
> 145 PEX1 INTD
> 146-147 Reserved
> 148 PEX1 MSI
> 149 PEX1 PME
> 150 PEX1 CFG err interrupt
>
> Only the "150 PEX1 CFG err interrupt" routing to the aer interrupt.
>
>
>>
>> In any event, the AER interrupts we're looking for in portdrv are from
>> the Root Port, not from the RC. For INTx, my understanding is that the
>> RC *transforms* virtual INTx messages from the Root Port in the PCIe
>> domain into GIC transactions in the platform domain.
>>
>> Bjorn
--
Murali Karicheri
Linux Kernel, Keystone
next prev parent reply other threads:[~2016-06-06 14:02 UTC|newest]
Thread overview: 58+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-05-26 6:00 [PATCH 2/2] aer: add support aer interrupt with none MSI/MSI-X/INTx mode Po Liu
2016-06-02 3:48 ` Bjorn Helgaas
2016-06-02 5:01 ` Po Liu
2016-06-02 13:55 ` Bjorn Helgaas
2016-06-02 15:37 ` Murali Karicheri
2016-06-03 4:09 ` Bjorn Helgaas
2016-06-03 17:31 ` Murali Karicheri
2016-06-04 3:48 ` Bjorn Helgaas
2016-06-06 7:32 ` Po Liu
2016-06-06 14:01 ` Murali Karicheri [this message]
2016-06-06 18:10 ` Bjorn Helgaas
2016-06-07 10:07 ` Po Liu
2016-06-07 22:46 ` Bjorn Helgaas
2016-06-08 4:56 ` Po Liu
2016-06-14 6:12 ` [PATCH v2 1/2] nxp/dts: add pcie aer interrupt-name property in the dts Po Liu
2016-06-14 6:12 ` [PATCH v2 2/2] pci/aer: interrupt fixup in the quirk Po Liu
2016-06-16 13:54 ` Bjorn Helgaas
2016-06-17 3:30 ` Po Liu
2016-07-01 8:46 ` Po Liu
2016-06-14 8:24 ` [PATCH v3 1/2] nxp/dts: add pcie aer interrupt-name property in the dts Po Liu
2016-06-14 8:24 ` [PATCH v3 2/2] pci/aer: interrupt fixup in the quirk Po Liu
2016-06-23 5:43 ` Dongdong Liu
2016-07-01 8:40 ` Po Liu
2016-07-04 8:44 ` Dongdong Liu
2016-07-05 3:03 ` Po Liu
2016-07-06 8:38 ` Dongdong Liu
2016-07-29 22:41 ` Bjorn Helgaas
2016-08-22 10:09 ` Po Liu
2016-09-20 20:47 ` Bjorn Helgaas
2016-09-21 6:51 ` Po Liu
2016-09-21 21:53 ` Bjorn Helgaas
2016-08-31 6:37 ` [PATCH v4 1/2] nxp/dts: add pcie aer interrupt-name property in the dts Po Liu
2016-08-31 6:37 ` [PATCH v4 2/2] pci:aer: add support aer interrupt with none MSI/MSI-X/INTx mode Po Liu
2016-09-02 15:17 ` Rob Herring
2016-09-05 6:05 ` Po Liu
2016-09-13 4:40 ` [PATCH v5 1/3] arm/dts: add pcie aer interrupt-name property in the dts Po Liu
2016-09-13 4:40 ` [PATCH v5 2/3] arm64/dts: " Po Liu
2016-09-13 4:40 ` [PATCH v5 3/3] pci:aer: add support aer interrupt with none MSI/MSI-X/INTx mode Po Liu
2016-09-18 0:52 ` Shawn Guo
2016-09-18 3:37 ` Po Liu
2016-09-20 12:39 ` Shawn Guo
2016-09-21 6:54 ` Po Liu
2016-09-30 22:13 ` Shawn Guo
2016-09-23 13:06 ` Rob Herring
2016-09-26 8:25 ` Po Liu
2016-09-21 22:37 ` Bjorn Helgaas
2016-09-22 2:53 ` Po Liu
2016-09-30 9:11 ` [PATCH v6 1/3] arm/dts-ls1021: add pcie aer/pme interrupt-name property in the dts Po Liu
2016-09-30 9:11 ` [PATCH v6 2/3] arm64/dts-ls1043-ls2080: " Po Liu
2016-09-30 9:11 ` [PATCH v6 3/3] pci:add support aer/pme interrupts with none MSI/MSI-X/INTx mode Po Liu
2016-10-08 20:49 ` Rob Herring
2016-10-09 2:47 ` Po Liu
2016-09-05 2:25 ` [PATCH v4 1/2] nxp/dts: add pcie aer interrupt-name property in the dts Shawn Guo
2016-09-12 22:13 ` Bjorn Helgaas
2016-09-13 3:02 ` Po Liu
2016-06-16 0:36 ` [PATCH v3 " Shawn Guo
2016-06-16 10:50 ` Po Liu
2016-06-16 22:19 ` Rob Herring
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=57558248.4010502@ti.com \
--to=m-karicheri2@ti.com \
--cc=arnd@arndb.de \
--cc=bhelgaas@google.com \
--cc=devicetree@vger.kernel.org \
--cc=helgaas@kernel.org \
--cc=leoyang.li@nxp.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=marc.zyngier@arm.com \
--cc=minghuan.lian@nxp.com \
--cc=mingkai.hu@nxp.com \
--cc=po.liu@nxp.com \
--cc=robh@kernel.org \
--cc=roy.zang@nxp.com \
--cc=shawnguo@kernel.org \
--cc=stuart.yoder@nxp.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).