Hi Bjorn, Thanks for the kindly reply. All these are helpful. > From: Bjorn Helgaas [mailto:helgaas@xxxxxxxxxx] > On Wed, June 08, 2016 6:47 AM > > On Tue, Jun 07, 2016 at 10:07:40AM +0000, Po Liu wrote: > > Hi Bjorn, > > > > > -----Original Message----- > > > > > > On Mon, Jun 06, 2016 at 10:01:44AM -0400, Murali Karicheri wrote: > > > > 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@xxxxxxxxxx] > >> Sent: > > > Saturday, June 04, 2016 11:49 AM > >> To: Murali Karicheri > >> > > > Cc: Po Liu; linux-pci@xxxxxxxxxxxxxxx; linux-arm- > >> > > > kernel@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; > >> > > > devicetree@xxxxxxxxxxxxxxx; 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@xxxxxxxxxx] > >>>>> Sent: Thursday, > > > June 02, 2016 > >> 11:48 AM > >>>>> To: Po Liu > >>>>> Cc: > > > > >> linux-pci@xxxxxxxxxxxxxxx; > >>>>> > >> > > > linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; > > > > >> > >>>>> linux-kernel@xxxxxxxxxxxxxxx; > > > devicetree@xxxxxxxxxxxxxxx; > >> 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. > > > > > > I'm looking at the "Error Message Controls" diagram in the PCIe > > > spec r3.0, sec 6.2.6. Does this hardware fit into the > > > platform-specific "System Error" case there? Do the Root Control > > > enable bits (in the PCIe > > > Capability) control this interrupt? If so, maybe this makes more > > > sense than I thought. > > > > It supposedly not the "System Error" case. But "the Error Interrupt" > case. > > Which means " Root Error Command register " could control the > > interrupt line we have now. (refer PCIe spec r3.0, sec 6.2.6) > > Did you actually try this out and verify that the PCIe Root Control > enable bits have no effect and the AER Root Error Command bits do > control it? The names are very similar, so there's lots of room for > misunderstanding here :) Yes, all these result were tested before reply. > > If the AER Root Error Command does control this interrupt, I think the > PCI_COMMAND_INTX_DISABLE bit in the PCI Command register should also > control it (per sec 6.2.4.1.2). Yes, I am sure the PCI_COMMAND_INTX_DISABLE bit can also control this interrupt. > > > May this kind of hardware design route broken the spec? > > If the Reporting Enable bits in the Root Port's AER Root Error Command > register control the interrupt, but the interrupt is not delivered via > the Root Port's INTx or MSI/MSI-X, I think the design is not following > the spec. > > All the information needed by the AER driver should be communicated via > the config space mechanisms described in the spec (AER capability, > MSI/MSI-X capabilities, Interrupt Pin, etc.) That way the driver works > without change on future spec-compliant hardware. > > > PME also like the AER. Hotplug is not supported. Others not known. > > Po Liu > > Per sec 6.1.6, I think PME *should* be signaled by the Root Port's INTx > or MSI/MSI-X. > > In particular, it says "Note that all other interrupt sources within the > same Function will assert the same virtual INTx wire when requesting > service." To me, that means that if we're using INTx, it will be the > same INTx for AER, PME, hotplug, etc., and it should be the one > indicated by the Interrupt Pin register. > > But I think on your Root Port: > > - There is an MSI capability, but MSI doesn't actually work at all > - Interrupt Pin contains 1, indicating INTA, which is routed to IRQ X > - AER interrupts are routed to some different IRQ Y > - PME interrupts are routed to a third IRQ Z > The descriptions are all right. > So how should we work around this? I think you should be able to get > partway there with a quirk that sets: > > dev->no_msi = 1; > dev->irq = Y; > > for this device. That should make AER work, but of course PME would not > work. > > Is there a way to set up your interrupt controller so these three > interrupts (X, Y, Z above) all map to the same Linux IRQ? If you can do > that, you could set up INTA, the AER interrupt, and the PME interrupt to > all be on the same IRQ and everything should work. > > Bjorn We'll think about all the ways. It is really helpful, thanks! ��.n��������+%������w��{.n�����{���"�)�� jg��� �����ݢj����G���� ���j:+v���w�m������w�������h�����٥
