Message ID | c7145c00-b2b9-780c-0bfd-15f7d8a08dd6@gmail.com |
---|---|
State | New |
Headers | show |
Series | [GIT,PULL] Mediatek: 32-bit DT update for v4.15 | expand |
On Mon, Oct 23, 2017 at 12:06 AM, Matthias Brugger <matthias.bgg@gmail.com> wrote: > > ---------------------------------------------------------------- > - mt76233 add PCIe node Could you clarify what the subnodes in the PCI node are for? It seems odd to have "interrupt-map" properties in both the pcie controller and its child nodes, and I want to ensure this is following the standard PCIe binding before I pull it. Arnd
Hi Arnd, We have 3 root ports in MT7623, but this is a bug in this chip where the HW designers wired the IRQs in a nonstandard way. We've tried to statically assign the bus portion of the address part in the parent interrupt-map before, but this approach cannot handle the case - if we attach the device in random order. Thanks. On Mon, 2017-10-30 at 13:42 +0100, Arnd Bergmann wrote: > On Mon, Oct 23, 2017 at 12:06 AM, Matthias Brugger > <matthias.bgg@gmail.com> wrote: > > > > ---------------------------------------------------------------- > > - mt76233 add PCIe node > > Could you clarify what the subnodes in the PCI node are for? It seems odd > to have "interrupt-map" properties in both the pcie controller and its child > nodes, and I want to ensure this is following the standard PCIe binding before > I pull it. > > Arnd >
Hi Arnd, On 10/31/2017 05:19 AM, Ryder Lee wrote: > Hi Arnd, > > We have 3 root ports in MT7623, but this is a bug in this chip where the > HW designers wired the IRQs in a nonstandard way. We've tried to > statically assign the bus portion of the address part in the parent > interrupt-map before, but this approach cannot handle the case - if we > attach the device in random order. > Ryder, please don't top post :) > Thanks. > > On Mon, 2017-10-30 at 13:42 +0100, Arnd Bergmann wrote: >> On Mon, Oct 23, 2017 at 12:06 AM, Matthias Brugger >> <matthias.bgg@gmail.com> wrote: >>> >>> ---------------------------------------------------------------- >>> - mt76233 add PCIe node >> >> Could you clarify what the subnodes in the PCI node are for? It seems odd >> to have "interrupt-map" properties in both the pcie controller and its child >> nodes, and I want to ensure this is following the standard PCIe binding before >> I pull it. >> Arnd, I didn't found the pull request in your next/dt branch. Is there more clarification needed from our side? Regards, Matthias
On Thu, Nov 2, 2017 at 12:47 PM, Matthias Brugger <matthias.bgg@gmail.com> wrote: > Hi Arnd, > > On 10/31/2017 05:19 AM, Ryder Lee wrote: >> Hi Arnd, >> >> We have 3 root ports in MT7623, but this is a bug in this chip where the >> HW designers wired the IRQs in a nonstandard way. We've tried to >> statically assign the bus portion of the address part in the parent >> interrupt-map before, but this approach cannot handle the case - if we >> attach the device in random order. >> > > Ryder, please don't top post :) > >> Thanks. >> >> On Mon, 2017-10-30 at 13:42 +0100, Arnd Bergmann wrote: >>> On Mon, Oct 23, 2017 at 12:06 AM, Matthias Brugger >>> <matthias.bgg@gmail.com> wrote: >>>> >>>> ---------------------------------------------------------------- >>>> - mt76233 add PCIe node >>> >>> Could you clarify what the subnodes in the PCI node are for? It seems odd >>> to have "interrupt-map" properties in both the pcie controller and its child >>> nodes, and I want to ensure this is following the standard PCIe binding before >>> I pull it. >>> > > Arnd, I didn't found the pull request in your next/dt branch. > Is there more clarification needed from our side? I'm still unsure about it., this looks like exactly the thing that the top-level interrupt-map is supposed to handle fine. Can you send a pull request for the series without the pci child nodes for the moment while we figure out what the exact problem is? We can take whatever fix we come up with during the v4.15 -rc cycle then. I would assume that either the parent interrupt-map is wrong, or something broke the parser, but that it's not actually something that's wrong in the hardware design in a way that we can't already handle in a standard way. Ryder, can you be more specific how the interrupts are wired up? Is there one IRQ per slot that is connected to all of IntA/IntB/IntC/IntD and gets propagated through the bridges like that, or is it something else? Arnd
On 11/02/2017 05:05 PM, Arnd Bergmann wrote: > On Thu, Nov 2, 2017 at 12:47 PM, Matthias Brugger > <matthias.bgg@gmail.com> wrote: >> Hi Arnd, >> >> On 10/31/2017 05:19 AM, Ryder Lee wrote: >>> Hi Arnd, >>> >>> We have 3 root ports in MT7623, but this is a bug in this chip where the >>> HW designers wired the IRQs in a nonstandard way. We've tried to >>> statically assign the bus portion of the address part in the parent >>> interrupt-map before, but this approach cannot handle the case - if we >>> attach the device in random order. >>> >> >> Ryder, please don't top post :) >> >>> Thanks. >>> >>> On Mon, 2017-10-30 at 13:42 +0100, Arnd Bergmann wrote: >>>> On Mon, Oct 23, 2017 at 12:06 AM, Matthias Brugger >>>> <matthias.bgg@gmail.com> wrote: >>>>> >>>>> ---------------------------------------------------------------- >>>>> - mt76233 add PCIe node >>>> >>>> Could you clarify what the subnodes in the PCI node are for? It seems odd >>>> to have "interrupt-map" properties in both the pcie controller and its child >>>> nodes, and I want to ensure this is following the standard PCIe binding before >>>> I pull it. >>>> >> >> Arnd, I didn't found the pull request in your next/dt branch. >> Is there more clarification needed from our side? > > I'm still unsure about it., this looks like exactly the thing that the top-level > interrupt-map is supposed to handle fine. > > Can you send a pull request for the series without the pci child nodes for the > moment while we figure out what the exact problem is? We can take > whatever fix we come up with during the v4.15 -rc cycle then. Sure. I'll provide a new tag v4.14-next-dts32-2 without the patch. I will send a pull request shortly. Regards, Matthias > > I would assume that either the parent interrupt-map is wrong, or something > broke the parser, but that it's not actually something that's wrong in the > hardware design in a way that we can't already handle in a standard way. > > Ryder, can you be more specific how the interrupts are wired up? > Is there one IRQ per slot that is connected to all of IntA/IntB/IntC/IntD > and gets propagated through the bridges like that, or is it something else? > > Arnd >
On Thu, Nov 2, 2017 at 7:47 PM, Matthias Brugger <matthias.bgg@gmail.com> wrote: > >>> >>> Arnd, I didn't found the pull request in your next/dt branch. >>> Is there more clarification needed from our side? >> >> I'm still unsure about it., this looks like exactly the thing that the top-level >> interrupt-map is supposed to handle fine. >> >> Can you send a pull request for the series without the pci child nodes for the >> moment while we figure out what the exact problem is? We can take >> whatever fix we come up with during the v4.15 -rc cycle then. > > Sure. I'll provide a new tag v4.14-next-dts32-2 without the patch. > I will send a pull request shortly. I was thinking with the patch but without the duplicated interrupt-map properties (or without the extra child nodes) but dropping the patch works for me too. I was hoping that we could figure out whether it's a code bug that prevents us from just parsing the parent interrupt-map correctly, or whether there is something wrong with that map itself. Arnd
Hi, On Thu, 2017-11-02 at 17:05 +0100, Arnd Bergmann wrote: > On Thu, Nov 2, 2017 at 12:47 PM, Matthias Brugger > <matthias.bgg@gmail.com> wrote: > > Hi Arnd, > > > > On 10/31/2017 05:19 AM, Ryder Lee wrote: > >> Hi Arnd, > >> > >> We have 3 root ports in MT7623, but this is a bug in this chip where the > >> HW designers wired the IRQs in a nonstandard way. We've tried to > >> statically assign the bus portion of the address part in the parent > >> interrupt-map before, but this approach cannot handle the case - if we > >> attach the device in random order. > >> > > > > Ryder, please don't top post :) Oh. My bad! > >> Thanks. > >> > >> On Mon, 2017-10-30 at 13:42 +0100, Arnd Bergmann wrote: > >>> On Mon, Oct 23, 2017 at 12:06 AM, Matthias Brugger > >>> <matthias.bgg@gmail.com> wrote: > >>>> > >>>> ---------------------------------------------------------------- > >>>> - mt76233 add PCIe node > >>> > >>> Could you clarify what the subnodes in the PCI node are for? It seems odd > >>> to have "interrupt-map" properties in both the pcie controller and its child > >>> nodes, and I want to ensure this is following the standard PCIe binding before > >>> I pull it. > >>> > > > > Arnd, I didn't found the pull request in your next/dt branch. > > Is there more clarification needed from our side? > > I'm still unsure about it., this looks like exactly the thing that the top-level > interrupt-map is supposed to handle fine. > > Can you send a pull request for the series without the pci child nodes for the > moment while we figure out what the exact problem is? We can take > whatever fix we come up with during the v4.15 -rc cycle then. > > I would assume that either the parent interrupt-map is wrong, or something > broke the parser, but that it's not actually something that's wrong in the > hardware design in a way that we can't already handle in a standard way. > > Ryder, can you be more specific how the interrupts are wired up? > Is there one IRQ per slot that is connected to all of IntA/IntB/IntC/IntD > and gets propagated through the bridges like that, or is it something else? Yes, that's what I mean - we only have one IRQ which is connected to all INTx for each slot, and I'm not sure if there is any better way to solve this problem. > Arnd >
On Fri, Nov 3, 2017 at 2:37 AM, Ryder Lee <ryder.lee@mediatek.com> wrote: >> Ryder, can you be more specific how the interrupts are wired up? >> Is there one IRQ per slot that is connected to all of IntA/IntB/IntC/IntD >> and gets propagated through the bridges like that, or is it something else? > > Yes, that's what I mean - we only have one IRQ which is connected to all > INTx for each slot, and I'm not sure if there is any better way to solve > this problem. Ok. Your parent interrupt-map seems entirely reasonable for that case as far as I can tell (maybe someone else can find a problem): + interrupt-map-mask = <0xf800 0 0 0>; + interrupt-map = <0x0000 0 0 0 &sysirq GIC_SPI 193 IRQ_TYPE_LEVEL_LOW>, + <0x0800 0 0 0 &sysirq GIC_SPI 194 IRQ_TYPE_LEVEL_LOW>, + <0x1000 0 0 0 &sysirq GIC_SPI 195 IRQ_TYPE_LEVEL_LOW>; However, I can't find any other example of a machine using interrupt-map-mask = <0xf800 0 0 0>; in the kernel tree, so it's possible that we have a parser bug. We do have other boards that list all four interrupts for each slot, and that seems to work fine. Can you try this map in the parent while leaving out the chilren? interrupt-map-mask = <0xf800 0 0 7>; interrupt-map = <0x0000 0 0 1 &sysirq GIC_SPI 193 IRQ_TYPE_LEVEL_LOW>, <0x0000 0 0 2 &sysirq GIC_SPI 193 IRQ_TYPE_LEVEL_LOW>, <0x0000 0 0 3 &sysirq GIC_SPI 193 IRQ_TYPE_LEVEL_LOW>, <0x0000 0 0 4 &sysirq GIC_SPI 193 IRQ_TYPE_LEVEL_LOW>, <0x0800 0 0 1 &sysirq GIC_SPI 194 IRQ_TYPE_LEVEL_LOW>, <0x0800 0 0 2 &sysirq GIC_SPI 194 IRQ_TYPE_LEVEL_LOW>, <0x0800 0 0 3 &sysirq GIC_SPI 194 IRQ_TYPE_LEVEL_LOW>, <0x0800 0 0 4 &sysirq GIC_SPI 194 IRQ_TYPE_LEVEL_LOW>, <0x1000 0 0 1 &sysirq GIC_SPI 195 IRQ_TYPE_LEVEL_LOW>, <0x1000 0 0 2 &sysirq GIC_SPI 195 IRQ_TYPE_LEVEL_LOW>, <0x1000 0 0 3 &sysirq GIC_SPI 195 IRQ_TYPE_LEVEL_LOW>, <0x1000 0 0 4 &sysirq GIC_SPI 195 IRQ_TYPE_LEVEL_LOW>; This should have the exact same effect as what you have in your tree, but if that works, we can merge that version and try to figure out why the kernel thinks they are different. Arnd
On Fri, 2017-11-03 at 10:40 +0100, Arnd Bergmann wrote: > On Fri, Nov 3, 2017 at 2:37 AM, Ryder Lee <ryder.lee@mediatek.com> wrote: > >> Ryder, can you be more specific how the interrupts are wired up? > >> Is there one IRQ per slot that is connected to all of IntA/IntB/IntC/IntD > >> and gets propagated through the bridges like that, or is it something else? > > > > Yes, that's what I mean - we only have one IRQ which is connected to all > > INTx for each slot, and I'm not sure if there is any better way to solve > > this problem. > > Ok. Your parent interrupt-map seems entirely reasonable for that case as far > as I can tell (maybe someone else can find a problem): > > + interrupt-map-mask = <0xf800 0 0 0>; > + interrupt-map = <0x0000 0 0 0 &sysirq GIC_SPI 193 > IRQ_TYPE_LEVEL_LOW>, > + <0x0800 0 0 0 &sysirq GIC_SPI 194 > IRQ_TYPE_LEVEL_LOW>, > + <0x1000 0 0 0 &sysirq GIC_SPI 195 > IRQ_TYPE_LEVEL_LOW>; > > However, I can't find any other example of a machine using > interrupt-map-mask = <0xf800 0 0 0>; in the kernel tree, so it's possible > that we have a parser bug. We do have other boards that list all four > interrupts for each slot, and that seems to work fine. Can you try this > map in the parent while leaving out the chilren? > > interrupt-map-mask = <0xf800 0 0 7>; > interrupt-map = <0x0000 0 0 1 &sysirq GIC_SPI 193 > IRQ_TYPE_LEVEL_LOW>, > <0x0000 0 0 2 &sysirq GIC_SPI > 193 IRQ_TYPE_LEVEL_LOW>, > <0x0000 0 0 3 &sysirq GIC_SPI > 193 IRQ_TYPE_LEVEL_LOW>, > <0x0000 0 0 4 &sysirq GIC_SPI > 193 IRQ_TYPE_LEVEL_LOW>, > <0x0800 0 0 1 &sysirq GIC_SPI > 194 IRQ_TYPE_LEVEL_LOW>, > <0x0800 0 0 2 &sysirq GIC_SPI > 194 IRQ_TYPE_LEVEL_LOW>, > <0x0800 0 0 3 &sysirq GIC_SPI > 194 IRQ_TYPE_LEVEL_LOW>, > <0x0800 0 0 4 &sysirq GIC_SPI > 194 IRQ_TYPE_LEVEL_LOW>, > <0x1000 0 0 1 &sysirq GIC_SPI > 195 IRQ_TYPE_LEVEL_LOW>, > <0x1000 0 0 2 &sysirq GIC_SPI > 195 IRQ_TYPE_LEVEL_LOW>, > <0x1000 0 0 3 &sysirq GIC_SPI > 195 IRQ_TYPE_LEVEL_LOW>, > <0x1000 0 0 4 &sysirq GIC_SPI > 195 IRQ_TYPE_LEVEL_LOW>; > > This should have the exact same effect as what you have in your tree, > but if that works, > we can merge that version and try to figure out why the kernel thinks > they are different. I've tried this approach by using the 4-ports network card and got the result: pcieport 0000:00:01.0: assign IRQ: got 220 pcieport 0000:00:01.0: assigning IRQ 220 pcieport 0000:00:01.0: enabling device (0140 -> 0142) pcieport 0000:00:01.0: enabling bus mastering pcieport 0000:00:01.0: Signaling PME with IRQ 220 .... igb 0000:01:00.0: assign IRQ: got 221 igb 0000:01:00.0: assigning IRQ 221 igb 0000:01:00.1: assign IRQ: got 221 igb 0000:01:00.1: assigning IRQ 221 igb 0000:01:00.2: assign IRQ: got 221 igb 0000:01:00.2: assigning IRQ 221 igb 0000:01:00.3: assign IRQ: got 221 igb 0000:01:00.3: assigning IRQ 221 I think slot 1 should share its IRQ (220) with every device in the hierarchy below this root port. Ryder.
On Fri, Nov 3, 2017 at 12:52 PM, Ryder Lee <ryder.lee@mediatek.com> wrote: > On Fri, 2017-11-03 at 10:40 +0100, Arnd Bergmann wrote: >> On Fri, Nov 3, 2017 at 2:37 AM, Ryder Lee <ryder.lee@mediatek.com> wrote: >> This should have the exact same effect as what you have in your tree, >> but if that works, >> we can merge that version and try to figure out why the kernel thinks >> they are different. > > I've tried this approach by using the 4-ports network card and got the > result: > pcieport 0000:00:01.0: assign IRQ: got 220 > pcieport 0000:00:01.0: assigning IRQ 220 > pcieport 0000:00:01.0: enabling device (0140 -> 0142) > pcieport 0000:00:01.0: enabling bus mastering > pcieport 0000:00:01.0: Signaling PME with IRQ 220 > .... > igb 0000:01:00.0: assign IRQ: got 221 > igb 0000:01:00.0: assigning IRQ 221 > igb 0000:01:00.1: assign IRQ: got 221 > igb 0000:01:00.1: assigning IRQ 221 > igb 0000:01:00.2: assign IRQ: got 221 > igb 0000:01:00.2: assigning IRQ 221 > igb 0000:01:00.3: assign IRQ: got 221 > igb 0000:01:00.3: assigning IRQ 221 > > I think slot 1 should share its IRQ (220) with every device in the > hierarchy below this root port. Agreed, that is what I expected, too. I've looked at the of_pci_irq.c in more detail but couldn't come up with what happened. Can you try running with this patch or something like that to figure out where it takes the wrong turn? https://pastebin.com/MrMzwcmw Arnd
On Fri, 2017-11-03 at 16:21 +0100, Arnd Bergmann wrote: > On Fri, Nov 3, 2017 at 12:52 PM, Ryder Lee <ryder.lee@mediatek.com> wrote: > > On Fri, 2017-11-03 at 10:40 +0100, Arnd Bergmann wrote: > >> On Fri, Nov 3, 2017 at 2:37 AM, Ryder Lee <ryder.lee@mediatek.com> wrote: > > >> This should have the exact same effect as what you have in your tree, > >> but if that works, > >> we can merge that version and try to figure out why the kernel thinks > >> they are different. > > > > I've tried this approach by using the 4-ports network card and got the > > result: > > pcieport 0000:00:01.0: assign IRQ: got 220 > > pcieport 0000:00:01.0: assigning IRQ 220 > > pcieport 0000:00:01.0: enabling device (0140 -> 0142) > > pcieport 0000:00:01.0: enabling bus mastering > > pcieport 0000:00:01.0: Signaling PME with IRQ 220 > > .... > > igb 0000:01:00.0: assign IRQ: got 221 > > igb 0000:01:00.0: assigning IRQ 221 > > igb 0000:01:00.1: assign IRQ: got 221 > > igb 0000:01:00.1: assigning IRQ 221 > > igb 0000:01:00.2: assign IRQ: got 221 > > igb 0000:01:00.2: assigning IRQ 221 > > igb 0000:01:00.3: assign IRQ: got 221 > > igb 0000:01:00.3: assigning IRQ 221 > > > > I think slot 1 should share its IRQ (220) with every device in the > > hierarchy below this root port. > > Agreed, that is what I expected, too. I've looked at the of_pci_irq.c > in more detail but couldn't come up with what happened. Can you > try running with this patch or something like that to figure out where > it takes the wrong turn? > > https://pastebin.com/MrMzwcmw > [ 4.466527] pcieport 0000:00:01.0: swizzle found slot 1 pin 1 [ 4.472252] pcieport 0000:00:01.0: direct parse found irq 3747445820:3747445848 [ 4.479515] pcieport 0000:00:01.0: parse pin 1 [ 4.483920] null ppdev, node dfff3744, pin 1 [ 4.488151] null ppdev, parse node dfff3744 pin 1 [ 4.492825] of_irq_parse_raw: /pcie-controller@1a140000:00000001 [ 4.498885] OF: of_irq_parse_raw: ipar=/pcie-controller@1a140000, size=1 [ 4.505530] OF: -> addrsize=3 [ 4.508569] OF: -> match=0 (imaplen=92) [ 4.512481] OF: -> newintsize=3, newaddrsize=0 [ 4.516969] OF: -> imaplen=88 [ 4.520007] OF: -> match=0 (imaplen=84) [ 4.523899] OF: -> newintsize=3, newaddrsize=0 [ 4.528400] OF: -> imaplen=80 [ 4.531425] OF: -> match=0 (imaplen=76) [ 4.535317] OF: -> newintsize=3, newaddrsize=0 [ 4.539817] OF: -> imaplen=72 [ 4.542842] OF: -> match=0 (imaplen=68) [ 4.546734] OF: -> newintsize=3, newaddrsize=0 [ 4.551234] OF: -> imaplen=64 [ 4.554259] OF: -> match=1 (imaplen=60) [ 4.558151] OF: -> newintsize=3, newaddrsize=0 [ 4.562651] OF: -> imaplen=56 [ 4.565676] OF: -> new parent: /interrupt-controller@10200100 [ 4.571469] OF: -> got it ! [ 4.574322] null ppdev, parse node dfff3744 irq 0:194 [ 4.579413] pcieport 0000:00:01.0: assign IRQ: got 220 [ 4.613400] igb 0000:01:00.0: swizzle pin 1 [ 4.617551] pcieport 0000:00:01.0: swizzle found slot 1 pin 1 [ 4.623276] igb 0000:01:00.0: parse pin 1 [ 4.627251] pcieport 0000:00:01.0: node dfff3e6c pin 1 [ 4.632359] pcieport 0000:00:01.0: parse node dfff3e6c pin 1 [ 4.637970] of_irq_parse_raw: /pcie-controller@1a140000/pcie@1,0:00000001 [ 4.644807] OF: of_irq_parse_raw: ipar=/pcie-controller@1a140000, size=1 [ 4.651463] OF: -> addrsize=3 [ 4.654491] OF: -> match=1 (imaplen=92) [ 4.658404] OF: -> newintsize=3, newaddrsize=0 [ 4.662893] OF: -> imaplen=88 [ 4.665917] OF: -> new parent: /interrupt-controller@10200100 [ 4.671711] OF: -> got it ! [ 4.674567] pcieport 0000:00:01.0: parse node dfff3e6c irq 0:193 [ 4.680596] igb 0000:01:00.0: assign IRQ: got 221 [ 4.806020] igb 0000:01:00.1: swizzle pin 2 [ 4.810201] pcieport 0000:00:01.0: swizzle found slot 1 pin 2 [ 4.815902] igb 0000:01:00.1: parse pin 2 [ 4.819891] pcieport 0000:00:01.0: node dfff3e6c pin 2 [ 4.824987] pcieport 0000:00:01.0: parse node dfff3e6c pin 2 [ 4.830609] of_irq_parse_raw: /pcie-controller@1a140000/pcie@1,0:00000002 [ 4.837437] OF: of_irq_parse_raw: ipar=/pcie-controller@1a140000, size=1 [ 4.844093] OF: -> addrsize=3 [ 4.847121] OF: -> match=0 (imaplen=92) [ 4.851042] OF: -> newintsize=3, newaddrsize=0 [ 4.855532] OF: -> imaplen=88 [ 4.858570] OF: -> match=1 (imaplen=84) [ 4.862463] OF: -> newintsize=3, newaddrsize=0 [ 4.866952] OF: -> imaplen=80 [ 4.869988] OF: -> new parent: /interrupt-controller@10200100 [ 4.875769] OF: -> got it ! [ 4.878637] pcieport 0000:00:01.0: parse node dfff3e6c irq 0:193 [ 4.884601] igb 0000:01:00.1: assign IRQ: got 221 But if I add "interrupt-map properties" in both the pcie controller and its child node: [ 4.466979] pcieport 0000:00:01.0: swizzle found slot 1 pin 1 [ 4.472705] pcieport 0000:00:01.0: direct parse found irq 3747437628:3747437656 [ 4.479969] pcieport 0000:00:01.0: parse pin 1 [ 4.484372] null ppdev, node dfff3570, pin 1 [ 4.488603] null ppdev, parse node dfff3570 pin 1 [ 4.493278] of_irq_parse_raw: /pcie-controller@1a140000:00000001 [ 4.499338] OF: of_irq_parse_raw: ipar=/pcie-controller@1a140000, size=1 [ 4.505983] OF: -> addrsize=3 [ 4.509023] OF: -> match=0 (imaplen=20) [ 4.512935] OF: -> newintsize=3, newaddrsize=0 [ 4.517424] OF: -> imaplen=16 [ 4.520464] OF: -> match=1 (imaplen=12) [ 4.524356] OF: -> newintsize=3, newaddrsize=0 [ 4.528857] OF: -> imaplen=8 [ 4.531797] OF: -> new parent: /interrupt-controller@10200100 [ 4.537578] OF: -> got it ! [ 4.540442] null ppdev, parse node dfff3570 irq 0:194 [ 4.545534] pcieport 0000:00:01.0: assign IRQ: got 220 [ 4.579510] igb 0000:01:00.0: swizzle pin 1 [ 4.583660] pcieport 0000:00:01.0: swizzle found slot 1 pin 1 [ 4.589384] igb 0000:01:00.0: parse pin 1 [ 4.593359] pcieport 0000:00:01.0: node dfff3d34 pin 1 [ 4.598453] pcieport 0000:00:01.0: parse node dfff3d34 pin 1 [ 4.604080] of_irq_parse_raw: /pcie-controller@1a140000/pcie@1,0:00000001 [ 4.610918] OF: of_irq_parse_raw: ipar=/pcie-controller@1a140000/pcie@1,0, size=1 [ 4.618337] OF: -> addrsize=3 [ 4.621377] OF: -> match=1 (imaplen=4) [ 4.625193] OF: -> newintsize=3, newaddrsize=0 [ 4.629695] OF: -> imaplen=0 [ 4.632633] OF: -> new parent: /interrupt-controller@10200100 [ 4.638414] OF: -> got it ! [ 4.641282] pcieport 0000:00:01.0: parse node dfff3d34 irq 0:194 [ 4.647242] igb 0000:01:00.0: assign IRQ: got 220 [ 4.772710] igb 0000:01:00.1: swizzle pin 2 [ 4.776860] pcieport 0000:00:01.0: swizzle found slot 1 pin 2 [ 4.782583] igb 0000:01:00.1: parse pin 2 [ 4.786559] pcieport 0000:00:01.0: node dfff3d34 pin 2 [ 4.791668] pcieport 0000:00:01.0: parse node dfff3d34 pin 2 [ 4.834485] pcieport 0000:00:01.0: parse node dfff3d34 irq 0:194 [ 4.840460] igb 0000:01:00.1: assign IRQ: got 220 I'm trying to understand what happened in the walking function. Ryder
On Fri, 2017-11-03 at 16:21 +0100, Arnd Bergmann wrote: > On Fri, Nov 3, 2017 at 12:52 PM, Ryder Lee <ryder.lee@mediatek.com> wrote: > > On Fri, 2017-11-03 at 10:40 +0100, Arnd Bergmann wrote: > >> On Fri, Nov 3, 2017 at 2:37 AM, Ryder Lee <ryder.lee@mediatek.com> wrote: > > >> This should have the exact same effect as what you have in your tree, > >> but if that works, > >> we can merge that version and try to figure out why the kernel thinks > >> they are different. > > > > I've tried this approach by using the 4-ports network card and got the > > result: > > pcieport 0000:00:01.0: assign IRQ: got 220 > > pcieport 0000:00:01.0: assigning IRQ 220 > > pcieport 0000:00:01.0: enabling device (0140 -> 0142) > > pcieport 0000:00:01.0: enabling bus mastering > > pcieport 0000:00:01.0: Signaling PME with IRQ 220 > > .... > > igb 0000:01:00.0: assign IRQ: got 221 > > igb 0000:01:00.0: assigning IRQ 221 > > igb 0000:01:00.1: assign IRQ: got 221 > > igb 0000:01:00.1: assigning IRQ 221 > > igb 0000:01:00.2: assign IRQ: got 221 > > igb 0000:01:00.2: assigning IRQ 221 > > igb 0000:01:00.3: assign IRQ: got 221 > > igb 0000:01:00.3: assigning IRQ 221 > > > > I think slot 1 should share its IRQ (220) with every device in the > > hierarchy below this root port. > > Agreed, that is what I expected, too. I've looked at the of_pci_irq.c > in more detail but couldn't come up with what happened. Can you > try running with this patch or something like that to figure out where > it takes the wrong turn? > > https://pastebin.com/MrMzwcmw > Hi Arnd, I guess the problem is the first device attached to downstream side of a link must be device 0 - it means that the 'slot' portion of pdev->devfn should be 0 when PCIe enumeration. That's why we always mismatch to interrupt-map = <0x0000 0 0 ... GIC_SPI 193 ...> However, if we want to list all four interrupts for each slot, that could work if fallback to device tree parsing: https://elixir.free-electrons.com/linux/v4.15-rc7/source/drivers/of/of_pci_irq.c#L89 Then replace 'pdev->devfn' with 'of_pci_get_devfn(out_irq->np)' But I still not sure the proper way to solve it. Ryder.