| Message ID | m3h9fuk7ik.fsf@t19.piap.pl (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Mar 25, 2016 at 10:32 AM, Krzysztof Ha?asa <khalasa@piap.pl> wrote: > A recent commit 5c5fb40de8f14391a1238db05cef88754faf9229 stated: > Follows: linus/v4.4-rc2 > Precedes: linus/v4.5-rc1 > > PCI: imx6: Add support for active-low reset GPIO > > We previously used of_get_named_gpio(), which ignores the OF flags cell, so > the reset GPIO defaulted to "active high." This doesn't work on the Toradex > Apalis SoM with Ixora base board, which has an active-low reset GPIO. > > Use devm_gpiod_get_optional() instead so we pay attention to the active > high/low flag. This also adds support for GPIOs described via ACPI. > > The (now replaced) code doesn't support the above: > @@ -287,10 +287,10 @@ static int imx6_pcie_deassert_core_reset(struct pcie_port *pp) > usleep_range(200, 500); > > /* Some boards don't have PCIe reset GPIO. */ > - if (gpio_is_valid(imx6_pcie->reset_gpio)) { > - gpio_set_value_cansleep(imx6_pcie->reset_gpio, 0); > + if (imx6_pcie->reset_gpio) { > + gpiod_set_value_cansleep(imx6_pcie->reset_gpio, 0); > msleep(100); > - gpio_set_value_cansleep(imx6_pcie->reset_gpio, 1); > + gpiod_set_value_cansleep(imx6_pcie->reset_gpio, 1); > } > return 0; > > If the reset_gpio setup code had ignored the flags (haven't checked > that), then clearly the resets were active-low (most reset lines are, > because they can be then driven with open-drain/collector output). > The gpiod_set_value*(0) activates reset, gpiod_set_value(1) - > deactivates. > > Now we're told the setup code is now level-aware, but the above sequence > thus _deactivates_ reset for 100 ms, then _activates_ it again. It has > no chance to work, unless a board has a broken DTS file. A quick grep > shows that about half the IMX6 boards specify an active-low PCIe reset, > 4 ask for active-high, and another 4 don't bother. > > > I wonder if all boards (except maybe that Toradex set) use an active-low > PCIe reset and are now broken. Perhaps Toradex uses active-high and thus > works. > > I'm not fixing individual DTS files because I don't really know, though > perhaps we should change them all to "active-low", since it would work > the same as before the 5c5fb40de8f14391a1238db05cef88754faf9229 change. > > Confirmed to fix Gateworks Laguna GW54xx. > Without the patch, the following happens (as expected): > > PCI host bridge /soc/pcie@0x01000000 ranges: > No bus range found for /soc/pcie@0x01000000, using [bus 00-ff] > IO 0x01f80000..0x01f8ffff -> 0x00000000 > MEM 0x01000000..0x01efffff -> 0x01000000 > imx6q-pcie 1ffc000.pcie: phy link never came up > > Signed-off-by: Krzysztof Ha?asa <khalasa@piap.pl> Good catch! Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com> I will fix imx6q-sabresd.dtsi when this patch gets applied.
On Sun, Mar 27, 2016 at 11:44 AM, Fabio Estevam <festevam@gmail.com> wrote: > Good catch! > > Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com> > > I will fix imx6q-sabresd.dtsi when this patch gets applied. After thinking more about it, I think the correct fix is to revert 5c5fb40de8f143 ("PCI: imx6: Add support for active-low reset GPIO") so that we do not break old dtb's. Then a new dt property can be added if someone needs gpio active high PCI reset.
On Sun, Mar 27, 2016 at 5:26 PM, Fabio Estevam <festevam@gmail.com> wrote: > On Sun, Mar 27, 2016 at 11:44 AM, Fabio Estevam <festevam@gmail.com> wrote: > >> Good catch! >> >> Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com> >> >> I will fix imx6q-sabresd.dtsi when this patch gets applied. > > After thinking more about it, I think the correct fix is to revert > 5c5fb40de8f143 ("PCI: imx6: Add support for active-low reset GPIO") so > that we do not break old dtb's. > > Then a new dt property can be added if someone needs gpio active high PCI reset. > -- > To unsubscribe from this list: send the line "unsubscribe linux-pci" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html Fabio, I would agree with you on this - 5c5fb40de8f143 ("PCI: imx6: Add support for active-low reset GPIO") should be reverted. I just finished bisecting an issue to this specific patch only to find out Krzysztof found it a few days ago ;) Thanks Krzysztof. Tim
Hi Tim, On Mon, Mar 28, 2016 at 4:59 PM, Tim Harvey <tharvey@gateworks.com> wrote: > Fabio, > > I would agree with you on this - 5c5fb40de8f143 ("PCI: imx6: Add > support for active-low reset GPIO") should be reverted. > > I just finished bisecting an issue to this specific patch only to find > out Krzysztof found it a few days ago ;) Thanks Krzysztof. I sent the revert patch yesterday: http://marc.info/?l=linux-pci&m=145912622805757&w=2
On Mon, Mar 28, 2016 at 1:13 PM, Fabio Estevam <festevam@gmail.com> wrote: > Hi Tim, > > On Mon, Mar 28, 2016 at 4:59 PM, Tim Harvey <tharvey@gateworks.com> wrote: > >> Fabio, >> >> I would agree with you on this - 5c5fb40de8f143 ("PCI: imx6: Add >> support for active-low reset GPIO") should be reverted. >> >> I just finished bisecting an issue to this specific patch only to find >> out Krzysztof found it a few days ago ;) Thanks Krzysztof. > > I sent the revert patch yesterday: > http://marc.info/?l=linux-pci&m=145912622805757&w=2 Fabio, ok - I'll respond there as I agree with the patch but not the wording of the commit (It's Gateworks 'Ventana' using IMX6 not Laguna and we do define the polarity properly as active-low in Ventana dt's). It is the fact that the gpio polarity has the wrong logic level that breaks Ventana. However, there seems to be another regression in 4.5 that's keeping PCI working for me and I'm still bisecting that (using 802.11n access points to test). Can you confirm that PCI works in v4.5 on IMX6 boards with only 5c5fb40de8f14391a1238db05cef88754faf9229 reverted? Tim
On Mon, Mar 28, 2016 at 5:42 PM, Tim Harvey <tharvey@gateworks.com> wrote: > Fabio, > > ok - I'll respond there as I agree with the patch but not the wording > of the commit (It's Gateworks 'Ventana' using IMX6 not Laguna and we > do define the polarity properly as active-low in Ventana dt's). It is > the fact that the gpio polarity has the wrong logic level that breaks > Ventana. Ok, I will change the wording in v2. > > However, there seems to be another regression in 4.5 that's keeping > PCI working for me and I'm still bisecting that (using 802.11n access > points to test). Can you confirm that PCI works in v4.5 on IMX6 boards > with only 5c5fb40de8f14391a1238db05cef88754faf9229 reverted? On imx6qdl-sabresd.dtsi the PCI reset gpio polarity is set to high, which is not correct, so the Wifi card could be detected even with 5c5fb40de8f. So two errors in sequence and PCI still works on this board :-) I don't have access to the board at the moment, but the only test I did was to see that the Wifi card got detected. I haven't really tried to communicate via Wifi.
On Mon, Mar 28, 2016 at 2:30 PM, Fabio Estevam <festevam@gmail.com> wrote: > On Mon, Mar 28, 2016 at 5:42 PM, Tim Harvey <tharvey@gateworks.com> wrote: > >> Fabio, >> >> ok - I'll respond there as I agree with the patch but not the wording >> of the commit (It's Gateworks 'Ventana' using IMX6 not Laguna and we >> do define the polarity properly as active-low in Ventana dt's). It is >> the fact that the gpio polarity has the wrong logic level that breaks >> Ventana. > > Ok, I will change the wording in v2. > >> >> However, there seems to be another regression in 4.5 that's keeping >> PCI working for me and I'm still bisecting that (using 802.11n access >> points to test). Can you confirm that PCI works in v4.5 on IMX6 boards >> with only 5c5fb40de8f14391a1238db05cef88754faf9229 reverted? > > On imx6qdl-sabresd.dtsi the PCI reset gpio polarity is set to high, > which is not correct, so the Wifi card could be detected even with > 5c5fb40de8f. So two errors in sequence and PCI still works on this > board :-) ouch - two wrongs did make a right! It's not too easy to tell how many IMX6 boards incorrectly specify their reset-gpio polarity. I don't know what the best way to determine what boards use the IMX6 pcie host controller. Is there a dtc usage that will display the compiled dtb's then we grep out 'compatible = "fsl,imx6q-pcie"' to at least get the list of boards to inspect? I'm curious if its just one or two boards that incorrectly specify the polarity of their PCI reset. > > I don't have access to the board at the moment, but the only test I > did was to see that the Wifi card got detected. I haven't really tried > to communicate via Wifi. I figured out it was the change to enable CONFIG_PCI_MSI in v4.5 that is causing interrupts to fail for me. Lucas, the case that is failing for me is when I have 4 miniPCI radios behind a PCIe->PCI bridge. In this case the radios get legacy INTA/B/C/D mapped to them correctly from what I can tell (GIC 123/122/121/120 swizzled appropriately), but those interrupts never fire. I don't think this case is necessarily a regression, I'm not clear that it has ever worked. In fact I can't seem to come up with a scenario where the MSI irq is firing either: IMX6->ath9k radio (no bridge) with MSI doesn't fire the PCI-MSI interrupt or the GPC 123 interrupt that gets mapped to it via the driver. Any ideas what can be going on here? Regards, Tim
On Mon, Mar 28, 2016 at 7:06 PM, Tim Harvey <tharvey@gateworks.com> wrote: >> On imx6qdl-sabresd.dtsi the PCI reset gpio polarity is set to high, >> which is not correct, so the Wifi card could be detected even with >> 5c5fb40de8f. So two errors in sequence and PCI still works on this >> board :-) > > ouch - two wrongs did make a right! > > It's not too easy to tell how many IMX6 boards incorrectly specify > their reset-gpio polarity. I don't know what the best way to determine > what boards use the IMX6 pcie host controller. Is there a dtc usage > that will display the compiled dtb's then we grep out 'compatible = > "fsl,imx6q-pcie"' to at least get the list of boards to inspect? I'm > curious if its just one or two boards that incorrectly specify the > polarity of their PCI reset. In order to keep old dtb's working we should simply ignore the GPIO flags passed in the 'reset-gpio' property. That's why we need a revert. Just sent a v2, BTW.
Tim Harvey <tharvey@gateworks.com> writes: > ok - I'll respond there as I agree with the patch but not the wording > of the commit (It's Gateworks 'Ventana' using IMX6 not Laguna and we > do define the polarity properly as active-low in Ventana dt's). Right, it's Ventana of course (I had been working with Laguna boards recently). > However, there seems to be another regression in 4.5 that's keeping > PCI working for me and I'm still bisecting that (using 802.11n access > points to test). Can you confirm that PCI works in v4.5 on IMX6 boards > with only 5c5fb40de8f14391a1238db05cef88754faf9229 reverted? I will check with my frame buffer and wifi cards.
Tim Harvey <tharvey@gateworks.com> writes: > It's not too easy to tell how many IMX6 boards incorrectly specify > their reset-gpio polarity. I don't know what the best way to determine > what boards use the IMX6 pcie host controller. Is there a dtc usage > that will display the compiled dtb's then we grep out 'compatible = > "fsl,imx6q-pcie"' to at least get the list of boards to inspect? I'm > curious if its just one or two boards that incorrectly specify the > polarity of their PCI reset. I guess, maybe 8 of them. Not counting those with out-of-tree DTS/DTB files. Something like: $ grep reset-gpio arch/arm/boot/dts/imx6* | grep -v phy-reset > I figured out it was the change to enable CONFIG_PCI_MSI in v4.5 that > is causing interrupts to fail for me. Right, a long standing issue. MSI never worked for me on i.MX6.
Fabio Estevam <festevam@gmail.com> writes: > In order to keep old dtb's working we should simply ignore the GPIO > flags passed in the 'reset-gpio' property. > > That's why we need a revert. Just sent a v2, BTW. OTOH, we should fix it some day, making sure the DTS files are fixed first: imx6qdl-apf6dev.dtsi: reset-gpio = <&gpio6 2 GPIO_ACTIVE_HIGH>; imx6qdl-aristainetos2.dtsi: reset-gpio = <&gpio2 16 GPIO_ACTIVE_HIGH>; imx6qdl-hummingboard.dtsi: reset-gpio = <&gpio3 4 0>; (I think RMK already handles this) imx6qdl-phytec-pfla02.dtsi: reset-gpio = <&gpio4 17 0>; imx6qdl-sabresd.dtsi: reset-gpio = <&gpio7 12 0>; imx6q-dmo-edmqmx6.dts: reset-gpio = <&gpio4 8 0>; imx6q-novena.dts: reset-gpio = <&gpio3 29 GPIO_ACTIVE_HIGH>; imx6q-tbs2910.dts: reset-gpio = <&gpio7 12 GPIO_ACTIVE_HIGH>; The original patch was a bad implementation of a good idea.
> OTOH, we should fix it some day, making sure the DTS files are fixed > first: > > imx6qdl-apf6dev.dtsi: reset-gpio = <&gpio6 2 GPIO_ACTIVE_HIGH>; > imx6qdl-aristainetos2.dtsi: reset-gpio = <&gpio2 16 GPIO_ACTIVE_HIGH>; > imx6qdl-hummingboard.dtsi: reset-gpio = <&gpio3 4 0>; (I think RMK already handles this) > imx6qdl-phytec-pfla02.dtsi: reset-gpio = <&gpio4 17 0>; > imx6qdl-sabresd.dtsi: reset-gpio = <&gpio7 12 0>; > imx6q-dmo-edmqmx6.dts: reset-gpio = <&gpio4 8 0>; > imx6q-novena.dts: reset-gpio = <&gpio3 29 GPIO_ACTIVE_HIGH>; > imx6q-tbs2910.dts: reset-gpio = <&gpio7 12 GPIO_ACTIVE_HIGH>; Or maybe we should simply change these to *_LOW, add that short patch from me, and forget about it.
Am Montag, den 28.03.2016, 15:06 -0700 schrieb Tim Harvey: > On Mon, Mar 28, 2016 at 2:30 PM, Fabio Estevam <festevam@gmail.com> wrote: > > On Mon, Mar 28, 2016 at 5:42 PM, Tim Harvey <tharvey@gateworks.com> wrote: > > > >> Fabio, > >> > >> ok - I'll respond there as I agree with the patch but not the wording > >> of the commit (It's Gateworks 'Ventana' using IMX6 not Laguna and we > >> do define the polarity properly as active-low in Ventana dt's). It is > >> the fact that the gpio polarity has the wrong logic level that breaks > >> Ventana. > > > > Ok, I will change the wording in v2. > > > >> > >> However, there seems to be another regression in 4.5 that's keeping > >> PCI working for me and I'm still bisecting that (using 802.11n access > >> points to test). Can you confirm that PCI works in v4.5 on IMX6 boards > >> with only 5c5fb40de8f14391a1238db05cef88754faf9229 reverted? > > > > On imx6qdl-sabresd.dtsi the PCI reset gpio polarity is set to high, > > which is not correct, so the Wifi card could be detected even with > > 5c5fb40de8f. So two errors in sequence and PCI still works on this > > board :-) > > ouch - two wrongs did make a right! > > It's not too easy to tell how many IMX6 boards incorrectly specify > their reset-gpio polarity. I don't know what the best way to determine > what boards use the IMX6 pcie host controller. Is there a dtc usage > that will display the compiled dtb's then we grep out 'compatible = > "fsl,imx6q-pcie"' to at least get the list of boards to inspect? I'm > curious if its just one or two boards that incorrectly specify the > polarity of their PCI reset. > I would suspect that most boards specify the reset polarity the wrong way around. Fixing this without breaking DT stability is hard. OTOH we could just argue that the system description in DT is wrong and needs to be fixed. > > > > I don't have access to the board at the moment, but the only test I > > did was to see that the Wifi card got detected. I haven't really tried > > to communicate via Wifi. > > I figured out it was the change to enable CONFIG_PCI_MSI in v4.5 that > is causing interrupts to fail for me. > Is this working with v4.4 and PCI_MSI enabled? I'm sure I've tested MSI IRQs before enabling them in the defconfig and they have been working for me for a long time before that. Tested with i210 on Gateworks Ventana. > Lucas, the case that is failing for me is when I have 4 miniPCI radios > behind a PCIe->PCI bridge. In this case the radios get legacy > INTA/B/C/D mapped to them correctly from what I can tell (GIC > 123/122/121/120 swizzled appropriately), but those interrupts never > fire. I don't think this case is necessarily a regression, I'm not > clear that it has ever worked. In fact I can't seem to come up with a > scenario where the MSI irq is firing either: IMX6->ath9k radio (no > bridge) with MSI doesn't fire the PCI-MSI interrupt or the GPC 123 > interrupt that gets mapped to it via the driver. Any ideas what can be > going on here? > Please test if MSI IRQs work with v4.4. I'll take a look at this later today. Regards, Lucas
Lucas Stach <l.stach@pengutronix.de> writes: > Is this working with v4.4 and PCI_MSI enabled? I'm sure I've tested MSI > IRQs before enabling them in the defconfig and they have been working > for me for a long time before that. Tested with i210 on Gateworks > Ventana. MSI never worked for me on Ventana. I have been using 4.2 extensively, and now I'm switching to 4.5 (which doesn't work either). Could it be a DTS (bridge) problem(?) On 4.5, trying to use it with TW6869 frame buffer and GW5410: TW6869: PCI 0000:04:00.0, IRQ 336, MMIO 0x1100000 TW686x 0000:04:00.0: enabling device (0140 -> 0142) CPU0 CPU1 CPU2 CPU3 16: 1165 1032 1271 1591 GIC-0 29 Edge twd 17: 879 387 1404 606 GPC 55 Level i.MX Timer Tick 18: 6434 0 0 0 GPC 13 Level mxs-dma 19: 0 0 0 0 GPC 15 Level bch 21: 0 0 0 0 GPC 9 Level 130000.gpu 22: 0 0 0 0 GPC 10 Level 134000.gpu 24: 0 0 0 0 GPC 120 Level mx6-pcie-msi 26: 0 0 0 0 GPC 26 Level 2020000.serial 30: 0 0 0 0 GPC 12 Level 2040000.vpu 240: 0 0 0 0 gpio-mxc 0 Edge 2198000.usdhc cd 280: 0 0 0 0 GPC 19 Level rtc alarm 286: 0 0 0 0 GPC 2 Level sdma 287: 0 0 0 0 GPC 43 Level 2184000.usb 288: 32 0 0 0 GPC 40 Level 2184200.usb 289: 2294 0 0 0 GIC-0 150 Level 2188000.ethernet 290: 0 0 0 0 GIC-0 151 Level 2188000.ethernet 291: 0 0 0 0 GPC 24 Level mmc0 292: 0 0 0 0 GPC 36 Level 21a0000.i2c 293: 0 0 0 0 GPC 37 Level 21a4000.i2c 294: 0 0 0 0 GPC 38 Level 21a8000.i2c 296: 1422 0 0 0 GPC 27 Level 21e8000.serial 297: 0 0 0 0 GPC 30 Level 21f4000.serial 300: 0 0 0 0 GPC 39 Level ahci-imx[2200000.sata] 301: 0 0 0 0 GPC 11 Level 2204000.gpu 304: 0 0 0 0 PCI-MSI 0 Edge PCIe PME, aerdrv 336: 0 0 0 0 GPC 123 Level TW6869 339: 0 0 0 0 IPU 457 Edge (null) 340: 0 0 0 0 IPU 451 Edge (null) 341: 0 0 0 0 IPU 457 Edge (null) 342: 0 0 0 0 IPU 451 Edge (null) IPI0: 0 0 0 0 CPU wakeup interrupts IPI1: 183 111 90 57 Timer broadcast interrupts IPI2: 453 2091 6539 2088 Rescheduling interrupts IPI3: 37 32 29 23 Function call interrupts IPI4: 0 0 0 0 CPU stop interrupts IPI5: 0 0 0 1 IRQ work interrupts IPI6: 0 0 0 0 completion interrupts Err: 0 00:00.0 PCI bridge: Synopsys, Inc. Device abcd (rev 01) 01:00.0 PCI bridge: PLX Technology, Inc. PEX 8609 8-lane, 8-Port PCI Express Gen 2 (5.0 GT/s) Switch with DMA (rev ba) 01:00.1 System peripheral: PLX Technology, Inc. PEX 8609 8-lane, 8-Port PCI Express Gen 2 (5.0 GT/s) Switch with DMA (rev ba) 02:01.0 PCI bridge: PLX Technology, Inc. PEX 8609 8-lane, 8-Port PCI Express Gen 2 (5.0 GT/s) Switch with DMA (rev ba) 02:04.0 PCI bridge: PLX Technology, Inc. PEX 8609 8-lane, 8-Port PCI Express Gen 2 (5.0 GT/s) Switch with DMA (rev ba) 02:05.0 PCI bridge: PLX Technology, Inc. PEX 8609 8-lane, 8-Port PCI Express Gen 2 (5.0 GT/s) Switch with DMA (rev ba) 02:06.0 PCI bridge: PLX Technology, Inc. PEX 8609 8-lane, 8-Port PCI Express Gen 2 (5.0 GT/s) Switch with DMA (rev ba) 02:07.0 PCI bridge: PLX Technology, Inc. PEX 8609 8-lane, 8-Port PCI Express Gen 2 (5.0 GT/s) Switch with DMA (rev ba) 02:08.0 PCI bridge: PLX Technology, Inc. PEX 8609 8-lane, 8-Port PCI Express Gen 2 (5.0 GT/s) Switch with DMA (rev ba) 02:09.0 PCI bridge: PLX Technology, Inc. PEX 8609 8-lane, 8-Port PCI Express Gen 2 (5.0 GT/s) Switch with DMA (rev ba) 04:00.0 Multimedia controller: Techwell Inc. Device 6869 (rev 01) 08:00.0 Ethernet controller: Marvell Technology Group Ltd. 88E8057 PCI-E Gigabit Ethernet Controller -[0000:00]---00.0-[01]--+-00.0-[02-09]--+-01.0-[03]-- | +-04.0-[04]----00.0 | +-05.0-[05]-- | +-06.0-[06]-- | +-07.0-[07]-- | +-08.0-[08]----00.0 | \-09.0-[09]-- \-00.1
Am Dienstag, den 29.03.2016, 12:39 +0200 schrieb Krzysztof Ha?asa: > Lucas Stach <l.stach@pengutronix.de> writes: > > > Is this working with v4.4 and PCI_MSI enabled? I'm sure I've tested MSI > > IRQs before enabling them in the defconfig and they have been working > > for me for a long time before that. Tested with i210 on Gateworks > > Ventana. > > MSI never worked for me on Ventana. I have been using 4.2 extensively, > and now I'm switching to 4.5 (which doesn't work either). > > Could it be a DTS (bridge) problem(?) > > On 4.5, trying to use it with TW6869 frame buffer and GW5410: > > TW6869: PCI 0000:04:00.0, IRQ 336, MMIO 0x1100000 > TW686x 0000:04:00.0: enabling device (0140 -> 0142) > I don't see whee the device even tries to use MSI IRQs. Even if the infrastructure is enabled it opts to use legacy INTA. There is no upstream driver for this chip, so I don't know where to look to find out if the driver tries to enable MSI. Is what you are saying that if you enable MSI support in the kernel, it breaks legacy IRQs? Regards, Lucas > CPU0 CPU1 CPU2 CPU3 > 16: 1165 1032 1271 1591 GIC-0 29 Edge twd > 17: 879 387 1404 606 GPC 55 Level i.MX Timer Tick > 18: 6434 0 0 0 GPC 13 Level mxs-dma > 19: 0 0 0 0 GPC 15 Level bch > 21: 0 0 0 0 GPC 9 Level 130000.gpu > 22: 0 0 0 0 GPC 10 Level 134000.gpu > 24: 0 0 0 0 GPC 120 Level mx6-pcie-msi > 26: 0 0 0 0 GPC 26 Level 2020000.serial > 30: 0 0 0 0 GPC 12 Level 2040000.vpu > 240: 0 0 0 0 gpio-mxc 0 Edge 2198000.usdhc cd > 280: 0 0 0 0 GPC 19 Level rtc alarm > 286: 0 0 0 0 GPC 2 Level sdma > 287: 0 0 0 0 GPC 43 Level 2184000.usb > 288: 32 0 0 0 GPC 40 Level 2184200.usb > 289: 2294 0 0 0 GIC-0 150 Level 2188000.ethernet > 290: 0 0 0 0 GIC-0 151 Level 2188000.ethernet > 291: 0 0 0 0 GPC 24 Level mmc0 > 292: 0 0 0 0 GPC 36 Level 21a0000.i2c > 293: 0 0 0 0 GPC 37 Level 21a4000.i2c > 294: 0 0 0 0 GPC 38 Level 21a8000.i2c > 296: 1422 0 0 0 GPC 27 Level 21e8000.serial > 297: 0 0 0 0 GPC 30 Level 21f4000.serial > 300: 0 0 0 0 GPC 39 Level ahci-imx[2200000.sata] > 301: 0 0 0 0 GPC 11 Level 2204000.gpu > 304: 0 0 0 0 PCI-MSI 0 Edge PCIe PME, aerdrv > 336: 0 0 0 0 GPC 123 Level TW6869 > 339: 0 0 0 0 IPU 457 Edge (null) > 340: 0 0 0 0 IPU 451 Edge (null) > 341: 0 0 0 0 IPU 457 Edge (null) > 342: 0 0 0 0 IPU 451 Edge (null) > IPI0: 0 0 0 0 CPU wakeup interrupts > IPI1: 183 111 90 57 Timer broadcast interrupts > IPI2: 453 2091 6539 2088 Rescheduling interrupts > IPI3: 37 32 29 23 Function call interrupts > IPI4: 0 0 0 0 CPU stop interrupts > IPI5: 0 0 0 1 IRQ work interrupts > IPI6: 0 0 0 0 completion interrupts > Err: 0 > > 00:00.0 PCI bridge: Synopsys, Inc. Device abcd (rev 01) > 01:00.0 PCI bridge: PLX Technology, Inc. PEX 8609 8-lane, 8-Port PCI Express Gen 2 (5.0 GT/s) Switch with DMA (rev ba) > 01:00.1 System peripheral: PLX Technology, Inc. PEX 8609 8-lane, 8-Port PCI Express Gen 2 (5.0 GT/s) Switch with DMA (rev ba) > 02:01.0 PCI bridge: PLX Technology, Inc. PEX 8609 8-lane, 8-Port PCI Express Gen 2 (5.0 GT/s) Switch with DMA (rev ba) > 02:04.0 PCI bridge: PLX Technology, Inc. PEX 8609 8-lane, 8-Port PCI Express Gen 2 (5.0 GT/s) Switch with DMA (rev ba) > 02:05.0 PCI bridge: PLX Technology, Inc. PEX 8609 8-lane, 8-Port PCI Express Gen 2 (5.0 GT/s) Switch with DMA (rev ba) > 02:06.0 PCI bridge: PLX Technology, Inc. PEX 8609 8-lane, 8-Port PCI Express Gen 2 (5.0 GT/s) Switch with DMA (rev ba) > 02:07.0 PCI bridge: PLX Technology, Inc. PEX 8609 8-lane, 8-Port PCI Express Gen 2 (5.0 GT/s) Switch with DMA (rev ba) > 02:08.0 PCI bridge: PLX Technology, Inc. PEX 8609 8-lane, 8-Port PCI Express Gen 2 (5.0 GT/s) Switch with DMA (rev ba) > 02:09.0 PCI bridge: PLX Technology, Inc. PEX 8609 8-lane, 8-Port PCI Express Gen 2 (5.0 GT/s) Switch with DMA (rev ba) > 04:00.0 Multimedia controller: Techwell Inc. Device 6869 (rev 01) > 08:00.0 Ethernet controller: Marvell Technology Group Ltd. 88E8057 PCI-E Gigabit Ethernet Controller > > -[0000:00]---00.0-[01]--+-00.0-[02-09]--+-01.0-[03]-- > | +-04.0-[04]----00.0 > | +-05.0-[05]-- > | +-06.0-[06]-- > | +-07.0-[07]-- > | +-08.0-[08]----00.0 > | \-09.0-[09]-- > \-00.1 >
On Tuesday 29 March 2016 12:55:21 Lucas Stach wrote: > Am Dienstag, den 29.03.2016, 12:39 +0200 schrieb Krzysztof Ha?asa: > > Lucas Stach <l.stach@pengutronix.de> writes: > > > > > Is this working with v4.4 and PCI_MSI enabled? I'm sure I've tested MSI > > > IRQs before enabling them in the defconfig and they have been working > > > for me for a long time before that. Tested with i210 on Gateworks > > > Ventana. > > > > MSI never worked for me on Ventana. I have been using 4.2 extensively, > > and now I'm switching to 4.5 (which doesn't work either). > > > > Could it be a DTS (bridge) problem(?) > > > > On 4.5, trying to use it with TW6869 frame buffer and GW5410: > > > > TW6869: PCI 0000:04:00.0, IRQ 336, MMIO 0x1100000 > > TW686x 0000:04:00.0: enabling device (0140 -> 0142) > > > I don't see whee the device even tries to use MSI IRQs. Even if the > infrastructure is enabled it opts to use legacy INTA. It just never calls pci_enable_msi(), right? MSI is purely opt-in for the driver, but it should just work if the device supports it and you add that call. Arnd
On Tue, Mar 29, 2016 at 3:55 AM, Lucas Stach <l.stach@pengutronix.de> wrote: > Am Dienstag, den 29.03.2016, 12:39 +0200 schrieb Krzysztof Ha?asa: >> Lucas Stach <l.stach@pengutronix.de> writes: >> >> > Is this working with v4.4 and PCI_MSI enabled? I'm sure I've tested MSI >> > IRQs before enabling them in the defconfig and they have been working >> > for me for a long time before that. Tested with i210 on Gateworks >> > Ventana. >> >> MSI never worked for me on Ventana. I have been using 4.2 extensively, >> and now I'm switching to 4.5 (which doesn't work either). >> >> Could it be a DTS (bridge) problem(?) Lucas, It's not the bridge - its the fact that not all drivers support MSI interrupts. One of the most common uses of PCI on Ventana is 802.11n radios and of them the ath9k driver is very commonly used and does not request an MSI interrupt (drivers/net/wireless/ath/ath9k/pci.c) >> >> On 4.5, trying to use it with TW6869 frame buffer and GW5410: >> >> TW6869: PCI 0000:04:00.0, IRQ 336, MMIO 0x1100000 >> TW686x 0000:04:00.0: enabling device (0140 -> 0142) >> > I don't see whee the device even tries to use MSI IRQs. Even if the > infrastructure is enabled it opts to use legacy INTA. yes, that's what many drivers do. > > There is no upstream driver for this chip, so I don't know where to look > to find out if the driver tries to enable MSI. > > Is what you are saying that if you enable MSI support in the kernel, it > breaks legacy IRQs? Yes - any driver that does not support MSI will use legacy IRQ's and they never fire. The Ventana GW53xx and GW54xx boards have a Marvell PCIe GigE supported by the sky2 driver as eth1 which does support MSI and I verified it gets an MSI interrupt and does work (along ath9k devices with legacy interrupts that fail to work). root@ventana:~# cat /proc/interrupts | grep MSI 299: 0 0 PCI-MSI 0 Edge PCIe PME, aerdrv 308: 8726 0 PCI-MSI 9 Edge eth1 So it appears that MSI works for those drivers that use it, but does somehow cause legacy IRQ's to break. I sent you a GW5400 dev kit over a while back to use for through bridge testing on IMX6 that you should be able to repeat this with assuming you have a PCIe card with a driver that doesn't support MSI (or that you can tweak its driver to not support MSI). I think 31e98e0d24cd2537a63e06e235e050a06b175df7 "ARM: imx_v6_v7_defconfig: enable PCI_MSI" should be reverted as well until we figure this out. Tim
On Tue, Mar 29, 2016 at 5:55 AM, Lucas Stach <l.stach@pengutronix.de> wrote: > I would suspect that most boards specify the reset polarity the wrong > way around. Fixing this without breaking DT stability is hard. OTOH we It is not hard if we just revert the buggy commit. > could just argue that the system description in DT is wrong and needs to > be fixed. Sure, this would be a nice thing to do as well.
Lucas Stach <l.stach@pengutronix.de> writes: >> TW6869: PCI 0000:04:00.0, IRQ 336, MMIO 0x1100000 >> TW686x 0000:04:00.0: enabling device (0140 -> 0142) >> > I don't see whee the device even tries to use MSI IRQs. Even if the > infrastructure is enabled it opts to use legacy INTA. It only tries to use normal IRQ. > There is no upstream driver for this chip, so I don't know where to look > to find out if the driver tries to enable MSI. It's been posted on linux-media list... I added pci_enable_msi() to this driver and it didn't help. > Is what you are saying that if you enable MSI support in the kernel, it > breaks legacy IRQs? Precisely. However, MSI doesn't seem to work either. Could be a problem specific to this TW6869 card. Ventana GW5410 has 5 mPCIe slots, and (with MSI enabled in the system) those IRQs (non-MSI) don't work in any slot: 304: 0 0 0 0 PCI-MSI 0 Edge PCIe PME, aerdrv 336: 0 0 0 0 GPC 123 Level TW8689 in J7 slot 337: 0 0 0 0 GPC 122 Level TW8689 in J8, J10, J11 338: 0 0 0 0 GPC 121 Level TW8689 in J6) If I enable MSI on this card (adding pci_enable_msi()): 313: 0 0 0 0 PCI-MSI 9 Edge TW6869 in J7 slot The only way I can get it to work is by disabling MSI (system wide).
Krzysztof Ha?asa <khalasa@piap.pl> [2016-03-25 14:32:35]: Cze??, > I wonder if all boards (except maybe that Toradex set) use an active-low > PCIe reset and are now broken. Perhaps Toradex uses active-high and thus > works. I'm really puzzled by this :-) With your patch applied I get following on Toradex Apalis modules: DTS: reset-gpio = <&gpio1 28 GPIO_ACTIVE_LOW>; dmesg: imx6q-pcie 1ffc000.pcie: phy link never came up gpio: gpio-28 ( |reset ) out hi pin voltage: 0V DTS: reset-gpio = <&gpio1 28 GPIO_ACTIVE_HIGH>; dmesg: ath9k 0000:01:00.0: enabling device (0140 -> 0142) gpio: gpio-28 ( |reset ) out lo pin voltage: 3V3 So Toradex Apalis is actually active-high? Thanks. -- ynezz
On Wed, Mar 30, 2016 at 9:06 AM, Petr Štetiar <ynezz@true.cz> wrote: > I'm really puzzled by this :-) With your patch applied I get following on > Toradex Apalis modules: > > DTS: reset-gpio = <&gpio1 28 GPIO_ACTIVE_LOW>; > dmesg: imx6q-pcie 1ffc000.pcie: phy link never came up > gpio: gpio-28 ( |reset ) out hi > pin voltage: 0V > > DTS: reset-gpio = <&gpio1 28 GPIO_ACTIVE_HIGH>; > dmesg: ath9k 0000:01:00.0: enabling device (0140 -> 0142) > gpio: gpio-28 ( |reset ) out lo > pin voltage: 3V3 > > So Toradex Apalis is actually active-high? Thanks. Yes, exactly. That's why you need to introduce a new property to handle the active-high case, so that old dtb's are not broken.
Hi Petr On Wed, 2016-03-30 at 14:06 +0200, Petr Štetiar wrote: > Krzysztof Ha?asa <khalasa@piap.pl> [2016-03-25 14:32:35]: > > Cze??, > > > > > I wonder if all boards (except maybe that Toradex set) use an > > active-low > > PCIe reset and are now broken. Perhaps Toradex uses active-high and > > thus > > works. > I'm really puzzled by this :-) With your patch applied I get > following on > Toradex Apalis modules: > > DTS: reset-gpio = <&gpio1 28 GPIO_ACTIVE_LOW>; > dmesg: imx6q-pcie 1ffc000.pcie: phy link never came up > gpio: gpio-28 ( |reset ) > out hi > pin voltage: 0V > > DTS: reset-gpio = <&gpio1 28 GPIO_ACTIVE_HIGH>; > dmesg: ath9k 0000:01:00.0: enabling device (0140 -> 0142) > gpio: gpio-28 ( |reset ) > out lo > pin voltage: 3V3 > > So Toradex Apalis is actually active-high? Yes, I actually explained this in detail in my cover letter: http://article.gmane.org/gmane.linux.drivers.devicetree/154829 > Thanks. > > -- ynezz Cheers Marcel
On Wed, Mar 30, 2016 at 1:10 AM, Krzysztof Ha?asa <khalasa@piap.pl> wrote: > Lucas Stach <l.stach@pengutronix.de> writes: > >>> TW6869: PCI 0000:04:00.0, IRQ 336, MMIO 0x1100000 >>> TW686x 0000:04:00.0: enabling device (0140 -> 0142) >>> >> I don't see whee the device even tries to use MSI IRQs. Even if the >> infrastructure is enabled it opts to use legacy INTA. > > It only tries to use normal IRQ. > >> There is no upstream driver for this chip, so I don't know where to look >> to find out if the driver tries to enable MSI. > > It's been posted on linux-media list... I added pci_enable_msi() to this > driver and it didn't help. > >> Is what you are saying that if you enable MSI support in the kernel, it >> breaks legacy IRQs? > > Precisely. > > However, MSI doesn't seem to work either. Could be a problem specific to > this TW6869 card. > > Ventana GW5410 has 5 mPCIe slots, and (with MSI enabled in the system) > those IRQs (non-MSI) don't work in any slot: > > 304: 0 0 0 0 PCI-MSI 0 Edge PCIe PME, aerdrv > 336: 0 0 0 0 GPC 123 Level TW8689 in J7 slot > 337: 0 0 0 0 GPC 122 Level TW8689 in J8, J10, J11 > 338: 0 0 0 0 GPC 121 Level TW8689 in J6) > > If I enable MSI on this card (adding pci_enable_msi()): > 313: 0 0 0 0 PCI-MSI 9 Edge TW6869 in J7 slot > > The only way I can get it to work is by disabling MSI (system wide). Krzysztof, I have found that MSI does work on IMX6 through a bridge (as on the GW5410) and not, for devices/drivers that support MSI, but it does break devices/drivers that use legacy irqs as we've discussed. The MSI capable devices/drivers I've been able to show working with MSI enabled are: - Marvell sky2 GigE (present on GW53xx and GW54xx both through a PEX switch) - Ath10k 802.11n radio miniPCIe socket (tested on GW51xx with no switch and GW53xx with switch). So perhaps there is something else going on with the tw8689 device/driver that is causing it to not work with MSI. We have an avc8000 miniPCIe with tw8689 here and can test if you send me your patches that enable tw8689 with msi. Regardless of MSI working in our tests we still disable it because of it breaking legacy irqs and for performance reasons. Regards, Tim
Tim, > So perhaps there is something else going on with the tw8689 > device/driver that is causing it to not work with MSI. We have an > avc8000 miniPCIe with tw8689 here and can test if you send me your > patches that enable tw8689 with msi. I think you can use this: http://git.linuxtv.org/hverkuil/media_tree.git/log/?h=tw686x > Regardless of MSI working in our tests we still disable it because of > it breaking legacy irqs and for performance reasons. So do I. However I'm also using Fedora 23 and this means I have to recompile the kernel, since they build it with MSI enabled. I think we should fix it, either by disabling in run time, or making it work. Performance-wise, I found it "surprising" that one can't have multiple MSIs with separate hw vector for each.
diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c index fe60096..f17fb02 100644 --- a/drivers/pci/host/pci-imx6.c +++ b/drivers/pci/host/pci-imx6.c @@ -288,9 +288,9 @@ static int imx6_pcie_deassert_core_reset(struct pcie_port *pp) /* Some boards don't have PCIe reset GPIO. */ if (imx6_pcie->reset_gpio) { - gpiod_set_value_cansleep(imx6_pcie->reset_gpio, 0); - msleep(100); gpiod_set_value_cansleep(imx6_pcie->reset_gpio, 1); + msleep(100); + gpiod_set_value_cansleep(imx6_pcie->reset_gpio, 0); } return 0;
A recent commit 5c5fb40de8f14391a1238db05cef88754faf9229 stated: Follows: linus/v4.4-rc2 Precedes: linus/v4.5-rc1 PCI: imx6: Add support for active-low reset GPIO We previously used of_get_named_gpio(), which ignores the OF flags cell, so the reset GPIO defaulted to "active high." This doesn't work on the Toradex Apalis SoM with Ixora base board, which has an active-low reset GPIO. Use devm_gpiod_get_optional() instead so we pay attention to the active high/low flag. This also adds support for GPIOs described via ACPI. The (now replaced) code doesn't support the above: @@ -287,10 +287,10 @@ static int imx6_pcie_deassert_core_reset(struct pcie_port *pp) usleep_range(200, 500); /* Some boards don't have PCIe reset GPIO. */ - if (gpio_is_valid(imx6_pcie->reset_gpio)) { - gpio_set_value_cansleep(imx6_pcie->reset_gpio, 0); + if (imx6_pcie->reset_gpio) { + gpiod_set_value_cansleep(imx6_pcie->reset_gpio, 0); msleep(100); - gpio_set_value_cansleep(imx6_pcie->reset_gpio, 1); + gpiod_set_value_cansleep(imx6_pcie->reset_gpio, 1); } return 0; If the reset_gpio setup code had ignored the flags (haven't checked that), then clearly the resets were active-low (most reset lines are, because they can be then driven with open-drain/collector output). The gpiod_set_value*(0) activates reset, gpiod_set_value(1) - deactivates. Now we're told the setup code is now level-aware, but the above sequence thus _deactivates_ reset for 100 ms, then _activates_ it again. It has no chance to work, unless a board has a broken DTS file. A quick grep shows that about half the IMX6 boards specify an active-low PCIe reset, 4 ask for active-high, and another 4 don't bother. I wonder if all boards (except maybe that Toradex set) use an active-low PCIe reset and are now broken. Perhaps Toradex uses active-high and thus works. I'm not fixing individual DTS files because I don't really know, though perhaps we should change them all to "active-low", since it would work the same as before the 5c5fb40de8f14391a1238db05cef88754faf9229 change. Confirmed to fix Gateworks Laguna GW54xx. Without the patch, the following happens (as expected): PCI host bridge /soc/pcie@0x01000000 ranges: No bus range found for /soc/pcie@0x01000000, using [bus 00-ff] IO 0x01f80000..0x01f8ffff -> 0x00000000 MEM 0x01000000..0x01efffff -> 0x01000000 imx6q-pcie 1ffc000.pcie: phy link never came up Signed-off-by: Krzysztof Ha?asa <khalasa@piap.pl>