[PATCH] i.MX6 PCIe: Fix imx6_pcie_deassert_core_reset() polarity

[PATCH] i.MX6 PCIe: Fix imx6_pcie_deassert_core_reset() polarity

[Krzysztof Hałasa]

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 at 0x01000000 ranges:
  No bus range found for /soc/pcie at 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>

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;
 

[PATCH] i.MX6 PCIe: Fix imx6_pcie_deassert_core_reset() polarity

[Fabio Estevam]

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 at 0x01000000 ranges:
>   No bus range found for /soc/pcie at 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.

[PATCH] i.MX6 PCIe: Fix imx6_pcie_deassert_core_reset() polarity

[Fabio Estevam]

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.

[PATCH] i.MX6 PCIe: Fix imx6_pcie_deassert_core_reset() polarity

[Tim Harvey]

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 at 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

[PATCH] i.MX6 PCIe: Fix imx6_pcie_deassert_core_reset() polarity

[Fabio Estevam]

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

[PATCH] i.MX6 PCIe: Fix imx6_pcie_deassert_core_reset() polarity

[Tim Harvey]

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

[PATCH] i.MX6 PCIe: Fix imx6_pcie_deassert_core_reset() polarity

[Fabio Estevam]

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.

[PATCH] i.MX6 PCIe: Fix imx6_pcie_deassert_core_reset() polarity

[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 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

[PATCH] i.MX6 PCIe: Fix imx6_pcie_deassert_core_reset() polarity

[Fabio Estevam]

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.

[PATCH] i.MX6 PCIe: Fix imx6_pcie_deassert_core_reset() polarity

[Krzysztof Hałasa]

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.
-- 
Krzysztof Halasa

Industrial Research Institute for Automation and Measurements PIAP
Al. Jerozolimskie 202, 02-486 Warsaw, Poland

[PATCH] i.MX6 PCIe: Fix imx6_pcie_deassert_core_reset() polarity

[Krzysztof Hałasa]

> 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.
-- 
Krzysztof Halasa

Industrial Research Institute for Automation and Measurements PIAP
Al. Jerozolimskie 202, 02-486 Warsaw, Poland

[PATCH] i.MX6 PCIe: Fix imx6_pcie_deassert_core_reset() polarity

[Krzysztof Hałasa]

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.
-- 
Krzysztof Halasa

Industrial Research Institute for Automation and Measurements PIAP
Al. Jerozolimskie 202, 02-486 Warsaw, Poland

[PATCH] i.MX6 PCIe: Fix imx6_pcie_deassert_core_reset() polarity

[Lucas Stach]

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

[PATCH] i.MX6 PCIe: Fix imx6_pcie_deassert_core_reset() polarity

[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)

           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

-- 
Krzysztof Halasa

Industrial Research Institute for Automation and Measurements PIAP
Al. Jerozolimskie 202, 02-486 Warsaw, Poland

[PATCH] i.MX6 PCIe: Fix imx6_pcie_deassert_core_reset() polarity

[Lucas Stach]

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
> 

[PATCH] i.MX6 PCIe: Fix imx6_pcie_deassert_core_reset() polarity

[Arnd Bergmann]

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

[PATCH] i.MX6 PCIe: Fix imx6_pcie_deassert_core_reset() polarity

[Tim Harvey]

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 at 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

[PATCH] i.MX6 PCIe: Fix imx6_pcie_deassert_core_reset() polarity

[Arnd Bergmann]

On Tuesday 29 March 2016 06:32:29 Tim Harvey wrote:
> >
> > 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 at 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.

That doesn't sound like a helpful solution, multi_v7_defconfig for
instance will still be broken because it enables PCI_MSI, and so
will be all major distros.

What happens if we patch the pci-imx6 driver to not make use of
its MSI support even when that is enabled in the kernel? Does that
get both devices in your GW5xxx to work with legacy interrupts or
is one or both of them still broken?

	Arnd

diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
index eb5a2755a164..d7607b2695c6 100644
--- a/drivers/pci/host/pci-imx6.c
+++ b/drivers/pci/host/pci-imx6.c
@@ -470,7 +470,7 @@ static void imx6_pcie_host_init(struct pcie_port *pp)
 
 	imx6_pcie_establish_link(pp);
 
-	if (IS_ENABLED(CONFIG_PCI_MSI))
+	if (0 && IS_ENABLED(CONFIG_PCI_MSI))
 		dw_pcie_msi_init(pp);
 }
 
@@ -490,7 +490,7 @@ static int __init imx6_add_pcie_port(struct pcie_port *pp,
 {
 	int ret;
 
-	if (IS_ENABLED(CONFIG_PCI_MSI)) {
+	if (0 && IS_ENABLED(CONFIG_PCI_MSI)) {
 		pp->msi_irq = platform_get_irq_byname(pdev, "msi");
 		if (pp->msi_irq <= 0) {
 			dev_err(&pdev->dev, "failed to get MSI irq\n");

[PATCH] i.MX6 PCIe: Fix imx6_pcie_deassert_core_reset() polarity

[Tim Harvey]

On Tue, Mar 29, 2016 at 6:52 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Tuesday 29 March 2016 06:32:29 Tim Harvey wrote:
>> >
>> > 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 at 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.
>
> That doesn't sound like a helpful solution, multi_v7_defconfig for
> instance will still be broken because it enables PCI_MSI, and so
> will be all major distros.

Arnd,

True, but keep in mind that as far as I can tell this behavior has
been around since MSI was added to the IMX6 (breakage of devices that
use legacy irq's if MSI is enabled) which was in 3.16.

>
> What happens if we patch the pci-imx6 driver to not make use of
> its MSI support even when that is enabled in the kernel? Does that
> get both devices in your GW5xxx to work with legacy interrupts or
> is one or both of them still broken?
>
>         Arnd
>
> diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
> index eb5a2755a164..d7607b2695c6 100644
> --- a/drivers/pci/host/pci-imx6.c
> +++ b/drivers/pci/host/pci-imx6.c
> @@ -470,7 +470,7 @@ static void imx6_pcie_host_init(struct pcie_port *pp)
>
>         imx6_pcie_establish_link(pp);
>
> -       if (IS_ENABLED(CONFIG_PCI_MSI))
> +       if (0 && IS_ENABLED(CONFIG_PCI_MSI))
>                 dw_pcie_msi_init(pp);
>  }
>
> @@ -490,7 +490,7 @@ static int __init imx6_add_pcie_port(struct pcie_port *pp,
>  {
>         int ret;
>
> -       if (IS_ENABLED(CONFIG_PCI_MSI)) {
> +       if (0 && IS_ENABLED(CONFIG_PCI_MSI)) {
>                 pp->msi_irq = platform_get_irq_byname(pdev, "msi");
>                 if (pp->msi_irq <= 0) {
>                         dev_err(&pdev->dev, "failed to get MSI irq\n");
>

That is not enough - we would also need to disable a couple more in
the designware core that imx6 uses, which is also used by several
other SoC's. We should probably get some feedback from people with
those SoC's regarding MSI breaking legacy irqs.

PCI_MSI was enabled in imx_v6_v7_defconfig in 4.5 and enabled in
multi_v7_defconfig back in 3.16. PCI MSI was first introduced for the
IMX6 host controller in 3.16 as well. I verified that the same issue
exists all the way back to 3.16.

I don't know if its worse to disable PCI MSI for IMX6/designware all
the way back to 3.16 or to disable it just back to 4.5 where imx_v6_v7
enabled it, or perhaps just figure out the actual issue and get that
backported?

Lucas, have you had any thoughts or time yet to think about why
enabling MSI breaks legacy IRQs?

Tim

[PATCH] i.MX6 PCIe: Fix imx6_pcie_deassert_core_reset() polarity

[Arnd Bergmann]

On Tuesday 29 March 2016 07:29:34 Tim Harvey wrote:
> On Tue, Mar 29, 2016 at 6:52 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Tuesday 29 March 2016 06:32:29 Tim Harvey wrote:

> >> I think 31e98e0d24cd2537a63e06e235e050a06b175df7 "ARM:
> >> imx_v6_v7_defconfig: enable PCI_MSI" should be reverted as well until
> >> we figure this out.
> >
> > That doesn't sound like a helpful solution, multi_v7_defconfig for
> > instance will still be broken because it enables PCI_MSI, and so
> > will be all major distros.
> 
> Arnd,
> 
> True, but keep in mind that as far as I can tell this behavior has
> been around since MSI was added to the IMX6 (breakage of devices that
> use legacy irq's if MSI is enabled) which was in 3.16.

I see. This part wasn't clear to me.

> > What happens if we patch the pci-imx6 driver to not make use of
> > its MSI support even when that is enabled in the kernel? Does that
> > get both devices in your GW5xxx to work with legacy interrupts or
> > is one or both of them still broken?
> >
> >         Arnd
> >
> > diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
> > index eb5a2755a164..d7607b2695c6 100644
> > --- a/drivers/pci/host/pci-imx6.c
> > +++ b/drivers/pci/host/pci-imx6.c
> > @@ -470,7 +470,7 @@ static void imx6_pcie_host_init(struct pcie_port *pp)
> >
> >         imx6_pcie_establish_link(pp);
> >
> > -       if (IS_ENABLED(CONFIG_PCI_MSI))
> > +       if (0 && IS_ENABLED(CONFIG_PCI_MSI))
> >                 dw_pcie_msi_init(pp);
> >  }
> >
> > @@ -490,7 +490,7 @@ static int __init imx6_add_pcie_port(struct pcie_port *pp,
> >  {
> >         int ret;
> >
> > -       if (IS_ENABLED(CONFIG_PCI_MSI)) {
> > +       if (0 && IS_ENABLED(CONFIG_PCI_MSI)) {
> >                 pp->msi_irq = platform_get_irq_byname(pdev, "msi");
> >                 if (pp->msi_irq <= 0) {
> >                         dev_err(&pdev->dev, "failed to get MSI irq\n");
> >
> 
> That is not enough - we would also need to disable a couple more in
> the designware core that imx6 uses, which is also used by several
> other SoC's. We should probably get some feedback from people with
> those SoC's regarding MSI breaking legacy irqs.

Good point. I really just meant this as an experiment, trying to
figure out what causes it to break. I'd be surprised if the
MSI support in the generic pcie-designware driver caused the same
problem on the other SoCs.

> PCI_MSI was enabled in imx_v6_v7_defconfig in 4.5 and enabled in
> multi_v7_defconfig back in 3.16. PCI MSI was first introduced for the
> IMX6 host controller in 3.16 as well. I verified that the same issue
> exists all the way back to 3.16.

Thanks for doing that research.

> I don't know if its worse to disable PCI MSI for IMX6/designware all
> the way back to 3.16 or to disable it just back to 4.5 where imx_v6_v7
> enabled it, or perhaps just figure out the actual issue and get that
> backported?

I'd like to see the problem understood better before we talk about
backports.

One thing I just noticed is that the same GPC interrupt line (GIC_SPI
120) is used for MSI and for IntD. Maybe there is something going on with
sharing an interrupt line between a nested irqchip and a device?

Could this be a bug in the generic IRQ handling code? Can you check
which interrupt the broken device(s) on your machine are using? Is
it always the 120 (IntD) line, or are the other three lines broken
as well? I don't actually know how to look it up, but the 'lspci -t'
output should let us reconstruct the swizzling manually.

	Arnd

[PATCH] i.MX6 PCIe: Fix imx6_pcie_deassert_core_reset() polarity

[Tim Harvey]

On Tue, Mar 29, 2016 at 7:50 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Tuesday 29 March 2016 07:29:34 Tim Harvey wrote:
>> On Tue, Mar 29, 2016 at 6:52 AM, Arnd Bergmann <arnd@arndb.de> wrote:
>> > On Tuesday 29 March 2016 06:32:29 Tim Harvey wrote:
>
>> >> I think 31e98e0d24cd2537a63e06e235e050a06b175df7 "ARM:
>> >> imx_v6_v7_defconfig: enable PCI_MSI" should be reverted as well until
>> >> we figure this out.
>> >
>> > That doesn't sound like a helpful solution, multi_v7_defconfig for
>> > instance will still be broken because it enables PCI_MSI, and so
>> > will be all major distros.
>>
>> Arnd,
>>
>> True, but keep in mind that as far as I can tell this behavior has
>> been around since MSI was added to the IMX6 (breakage of devices that
>> use legacy irq's if MSI is enabled) which was in 3.16.
>
> I see. This part wasn't clear to me.
>
>> > What happens if we patch the pci-imx6 driver to not make use of
>> > its MSI support even when that is enabled in the kernel? Does that
>> > get both devices in your GW5xxx to work with legacy interrupts or
>> > is one or both of them still broken?
>> >
>> >         Arnd
>> >
>> > diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
>> > index eb5a2755a164..d7607b2695c6 100644
>> > --- a/drivers/pci/host/pci-imx6.c
>> > +++ b/drivers/pci/host/pci-imx6.c
>> > @@ -470,7 +470,7 @@ static void imx6_pcie_host_init(struct pcie_port *pp)
>> >
>> >         imx6_pcie_establish_link(pp);
>> >
>> > -       if (IS_ENABLED(CONFIG_PCI_MSI))
>> > +       if (0 && IS_ENABLED(CONFIG_PCI_MSI))
>> >                 dw_pcie_msi_init(pp);
>> >  }
>> >
>> > @@ -490,7 +490,7 @@ static int __init imx6_add_pcie_port(struct pcie_port *pp,
>> >  {
>> >         int ret;
>> >
>> > -       if (IS_ENABLED(CONFIG_PCI_MSI)) {
>> > +       if (0 && IS_ENABLED(CONFIG_PCI_MSI)) {
>> >                 pp->msi_irq = platform_get_irq_byname(pdev, "msi");
>> >                 if (pp->msi_irq <= 0) {
>> >                         dev_err(&pdev->dev, "failed to get MSI irq\n");
>> >
>>
>> That is not enough - we would also need to disable a couple more in
>> the designware core that imx6 uses, which is also used by several
>> other SoC's. We should probably get some feedback from people with
>> those SoC's regarding MSI breaking legacy irqs.
>
> Good point. I really just meant this as an experiment, trying to
> figure out what causes it to break. I'd be surprised if the
> MSI support in the generic pcie-designware driver caused the same
> problem on the other SoCs.
>
>> PCI_MSI was enabled in imx_v6_v7_defconfig in 4.5 and enabled in
>> multi_v7_defconfig back in 3.16. PCI MSI was first introduced for the
>> IMX6 host controller in 3.16 as well. I verified that the same issue
>> exists all the way back to 3.16.
>
> Thanks for doing that research.
>
>> I don't know if its worse to disable PCI MSI for IMX6/designware all
>> the way back to 3.16 or to disable it just back to 4.5 where imx_v6_v7
>> enabled it, or perhaps just figure out the actual issue and get that
>> backported?
>
> I'd like to see the problem understood better before we talk about
> backports.
>
> One thing I just noticed is that the same GPC interrupt line (GIC_SPI
> 120) is used for MSI and for IntD. Maybe there is something going on with
> sharing an interrupt line between a nested irqchip and a device?
>
> Could this be a bug in the generic IRQ handling code? Can you check
> which interrupt the broken device(s) on your machine are using? Is
> it always the 120 (IntD) line, or are the other three lines broken
> as well? I don't actually know how to look it up, but the 'lspci -t'
> output should let us reconstruct the swizzling manually.
>
>         Arnd

Arnd,

Right, on the IMX the MSI interrupt is GIC-120 which is also the
legacy INTD and I do see that if I happen to put a radio in a slot
where due to swizzling its pin1 becomes INTD (GIC-120) the interrupt
does fire and the device works. Any other slot using GIC-123 (INTA),
GIC-122 (INTB), or GIC-121 (INTC) never fires so its very possible
that something in the designware core is masking out the legacy irqs.
I would also think this was something IMX specific, but I really don't
see any codepaths in pci-imx6.c that would cause that: a driver
requesting a legacy PCI would get a GIC interrupt which is handled by
the IMX6 gpc interrupt controller.

Any dra7xxx, exynos, spear13xx, keystone, layerscape, hisi, qcom SoC
users of designware PCIe core out there that can verify PCI MSI and
legacy are both working at the same time?

Lucas is the expert here and I believe he has the documentation for
the designware core that Freescale doens't provide with the IMX6
documentation so hopefully he can provide some insight. He's the one
that has authored all the MSI support and has been using it.

I typically advise our users to 'not' enable MSI because
architecturally you can spread 4 distinct legacy irq's across CPU's
better than a single shared irq.

Tim

[PATCH] i.MX6 PCIe: Fix imx6_pcie_deassert_core_reset() polarity

[Arnd Bergmann]

On Tuesday 29 March 2016 08:10:08 Tim Harvey wrote:
> Arnd,
> 
> Right, on the IMX the MSI interrupt is GIC-120 which is also the
> legacy INTD and I do see that if I happen to put a radio in a slot
> where due to swizzling its pin1 becomes INTD (GIC-120) the interrupt
> does fire and the device works. Any other slot using GIC-123 (INTA),
> GIC-122 (INTB), or GIC-121 (INTC) never fires so its very possible
> that something in the designware core is masking out the legacy irqs.

Interesting. I was actually expecting the opposite here, having the
IRQs only work if they are not IntD. 


> I typically advise our users to 'not' enable MSI because
> architecturally you can spread 4 distinct legacy irq's across CPU's
> better than a single shared irq.

That is a very good point, I never understood why we want to enable
MSI support on any PCI host bridge that just forwards all MSIs
to a single IRQ line. Originally MSI was meant as a performance
feature, but there is nothing in this setup that makes things go
faster, and several things that make it go slower.

I would still hope that with disabling MSI support in just the i.MX
driver (as in the trivial patch I suggested trying, or by
reverting Lucas' d1dc9749a5b8 patch) will make things just work.
If not, we may need to change the pcie-designware driver as well,
so it doesn't try to enable MSI on its own.

	Arnd

[PATCH] i.MX6 PCIe: Fix imx6_pcie_deassert_core_reset() polarity

[Tim Harvey]

On Tue, Mar 29, 2016 at 8:24 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Tuesday 29 March 2016 08:10:08 Tim Harvey wrote:
>> Arnd,
>>
>> Right, on the IMX the MSI interrupt is GIC-120 which is also the
>> legacy INTD and I do see that if I happen to put a radio in a slot
>> where due to swizzling its pin1 becomes INTD (GIC-120) the interrupt
>> does fire and the device works. Any other slot using GIC-123 (INTA),
>> GIC-122 (INTB), or GIC-121 (INTC) never fires so its very possible
>> that something in the designware core is masking out the legacy irqs.
>
> Interesting. I was actually expecting the opposite here, having the
> IRQs only work if they are not IntD.
>
>
>> I typically advise our users to 'not' enable MSI because
>> architecturally you can spread 4 distinct legacy irq's across CPU's
>> better than a single shared irq.
>
> That is a very good point, I never understood why we want to enable
> MSI support on any PCI host bridge that just forwards all MSIs
> to a single IRQ line. Originally MSI was meant as a performance
> feature, but there is nothing in this setup that makes things go
> faster, and several things that make it go slower.

I had a conversation once with Lucas about implementing the shared MSI
interrupt in such a way that its smp affinity could be set to other
CPU's to gain a performance benefit in certain multi-device cases.

While this is technically possible it would involve creating a softirq
glue between the different handlers but that would add overhead of a
softirq plus potentially waking up another CPU to every IRQ which
would end up adding some overhead to even the simple single-device
case.

Without any hard data it wasn't clear if this was worth it or if there
was a clean way to provide this as build-time or run-time option.

Tim

[PATCH] i.MX6 PCIe: Fix imx6_pcie_deassert_core_reset() polarity

[Arnd Bergmann]

On Tuesday 29 March 2016 10:38:16 Tim Harvey wrote:
> On Tue, Mar 29, 2016 at 8:24 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Tuesday 29 March 2016 08:10:08 Tim Harvey wrote:
> >> Arnd,
> >>
> >> Right, on the IMX the MSI interrupt is GIC-120 which is also the
> >> legacy INTD and I do see that if I happen to put a radio in a slot
> >> where due to swizzling its pin1 becomes INTD (GIC-120) the interrupt
> >> does fire and the device works. Any other slot using GIC-123 (INTA),
> >> GIC-122 (INTB), or GIC-121 (INTC) never fires so its very possible
> >> that something in the designware core is masking out the legacy irqs.
> >
> > Interesting. I was actually expecting the opposite here, having the
> > IRQs only work if they are not IntD.
> >
> >
> >> I typically advise our users to 'not' enable MSI because
> >> architecturally you can spread 4 distinct legacy irq's across CPU's
> >> better than a single shared irq.
> >
> > That is a very good point, I never understood why we want to enable
> > MSI support on any PCI host bridge that just forwards all MSIs
> > to a single IRQ line. Originally MSI was meant as a performance
> > feature, but there is nothing in this setup that makes things go
> > faster, and several things that make it go slower.
> 
> I had a conversation once with Lucas about implementing the shared MSI
> interrupt in such a way that its smp affinity could be set to other
> CPU's to gain a performance benefit in certain multi-device cases.
> 
> While this is technically possible it would involve creating a softirq
> glue between the different handlers but that would add overhead of a
> softirq plus potentially waking up another CPU to every IRQ which
> would end up adding some overhead to even the simple single-device
> case.
> 
> Without any hard data it wasn't clear if this was worth it or if there
> was a clean way to provide this as build-time or run-time option.

I think it's pretty clear that this would take things from 'somewhat silly'
to 'completely bonkers' ;-)

	Arnd

[PATCH] i.MX6 PCIe: Fix imx6_pcie_deassert_core_reset() polarity

[Marc Zyngier]

On 29/03/16 16:24, Arnd Bergmann wrote:
> On Tuesday 29 March 2016 08:10:08 Tim Harvey wrote:
>> Arnd,
>>
>> Right, on the IMX the MSI interrupt is GIC-120 which is also the
>> legacy INTD and I do see that if I happen to put a radio in a slot
>> where due to swizzling its pin1 becomes INTD (GIC-120) the interrupt
>> does fire and the device works. Any other slot using GIC-123 (INTA),
>> GIC-122 (INTB), or GIC-121 (INTC) never fires so its very possible
>> that something in the designware core is masking out the legacy irqs.
> 
> Interesting. I was actually expecting the opposite here, having the
> IRQs only work if they are not IntD. 
> 
> 
>> I typically advise our users to 'not' enable MSI because
>> architecturally you can spread 4 distinct legacy irq's across CPU's
>> better than a single shared irq.
> 
> That is a very good point, I never understood why we want to enable
> MSI support on any PCI host bridge that just forwards all MSIs
> to a single IRQ line. Originally MSI was meant as a performance
> feature, but there is nothing in this setup that makes things go
> faster, and several things that make it go slower.

Feature-ticking exercise.

"We support MSI", never mind if that negating the benefits of the
mechanism and ending up with disastrous impacts on interrupt affinity,
and a set of open questions regarding the effect of the MSI as a DMA fence.

/me stops ranting for the day...

	M.
-- 
Jazz is not dead. It just smells funny...

[PATCH] i.MX6 PCIe: Fix imx6_pcie_deassert_core_reset() polarity

[Roberto Fichera]

On 03/29/2016 05:10 PM, Tim Harvey wrote:
> Arnd,
>
> Right, on the IMX the MSI interrupt is GIC-120 which is also the
> legacy INTD and I do see that if I happen to put a radio in a slot
> where due to swizzling its pin1 becomes INTD (GIC-120) the interrupt
> does fire and the device works. Any other slot using GIC-123 (INTA),
> GIC-122 (INTB), or GIC-121 (INTC) never fires so its very possible
> that something in the designware core is masking out the legacy irqs.
> I would also think this was something IMX specific, but I really don't
> see any codepaths in pci-imx6.c that would cause that: a driver
> requesting a legacy PCI would get a GIC interrupt which is handled by
> the IMX6 gpc interrupt controller.
>
> Any dra7xxx, exynos, spear13xx, keystone, layerscape, hisi, qcom SoC
> users of designware PCIe core out there that can verify PCI MSI and
> legacy are both working at the same time?
>
> Lucas is the expert here and I believe he has the documentation for
> the designware core that Freescale doens't provide with the IMX6
> documentation so hopefully he can provide some insight. He's the one
> that has authored all the MSI support and has been using it.
>
> I typically advise our users to 'not' enable MSI because
> architecturally you can spread 4 distinct legacy irq's across CPU's
> better than a single shared irq.

Don't know if I'm facing similar problem, however devices connected in miniPCI slot behind
a PCIe-to-PCI bridge (MSI is disabled) using INTA all is working ok, including shared IRQ.
In case of INTB will not work, and the GIC irq quite often get stuck.

[PATCH] i.MX6 PCIe: Fix imx6_pcie_deassert_core_reset() polarity

[Tim Harvey]

On Tue, Mar 29, 2016 at 9:13 AM, Roberto Fichera <kernel@tekno-soft.it> wrote:
> On 03/29/2016 05:10 PM, Tim Harvey wrote:
>> Arnd,
>>
>> Right, on the IMX the MSI interrupt is GIC-120 which is also the
>> legacy INTD and I do see that if I happen to put a radio in a slot
>> where due to swizzling its pin1 becomes INTD (GIC-120) the interrupt
>> does fire and the device works. Any other slot using GIC-123 (INTA),
>> GIC-122 (INTB), or GIC-121 (INTC) never fires so its very possible
>> that something in the designware core is masking out the legacy irqs.
>> I would also think this was something IMX specific, but I really don't
>> see any codepaths in pci-imx6.c that would cause that: a driver
>> requesting a legacy PCI would get a GIC interrupt which is handled by
>> the IMX6 gpc interrupt controller.
>>
>> Any dra7xxx, exynos, spear13xx, keystone, layerscape, hisi, qcom SoC
>> users of designware PCIe core out there that can verify PCI MSI and
>> legacy are both working at the same time?
>>
>> Lucas is the expert here and I believe he has the documentation for
>> the designware core that Freescale doens't provide with the IMX6
>> documentation so hopefully he can provide some insight. He's the one
>> that has authored all the MSI support and has been using it.
>>
>> I typically advise our users to 'not' enable MSI because
>> architecturally you can spread 4 distinct legacy irq's across CPU's
>> better than a single shared irq.
>
> Don't know if I'm facing similar problem, however devices connected in miniPCI slot behind
> a PCIe-to-PCI bridge (MSI is disabled) using INTA all is working ok, including shared IRQ.
> In case of INTB will not work, and the GIC irq quite often get stuck.
>

Roberto,

What board/platform is this and what does /proc/interrupts look like?

This sounds like what would happen if the downstream interrupts on the
PCIe-to-PCI bridge are not mapped properly as was the case with a
board I support (in which case I had to work out a bootloader fixup
that placed a non-standard interrupt-map in the device-tree for the
bridge). What bridge are you using?

Tim

[PATCH] i.MX6 PCIe: Fix imx6_pcie_deassert_core_reset() polarity

[Roberto Fichera]

On 03/29/2016 06:40 PM, Tim Harvey wrote:
> On Tue, Mar 29, 2016 at 9:13 AM, Roberto Fichera <kernel@tekno-soft.it> wrote:
>> On 03/29/2016 05:10 PM, Tim Harvey wrote:
>>> Arnd,
>>>
>>> Right, on the IMX the MSI interrupt is GIC-120 which is also the
>>> legacy INTD and I do see that if I happen to put a radio in a slot
>>> where due to swizzling its pin1 becomes INTD (GIC-120) the interrupt
>>> does fire and the device works. Any other slot using GIC-123 (INTA),
>>> GIC-122 (INTB), or GIC-121 (INTC) never fires so its very possible
>>> that something in the designware core is masking out the legacy irqs.
>>> I would also think this was something IMX specific, but I really don't
>>> see any codepaths in pci-imx6.c that would cause that: a driver
>>> requesting a legacy PCI would get a GIC interrupt which is handled by
>>> the IMX6 gpc interrupt controller.
>>>
>>> Any dra7xxx, exynos, spear13xx, keystone, layerscape, hisi, qcom SoC
>>> users of designware PCIe core out there that can verify PCI MSI and
>>> legacy are both working at the same time?
>>>
>>> Lucas is the expert here and I believe he has the documentation for
>>> the designware core that Freescale doens't provide with the IMX6
>>> documentation so hopefully he can provide some insight. He's the one
>>> that has authored all the MSI support and has been using it.
>>>
>>> I typically advise our users to 'not' enable MSI because
>>> architecturally you can spread 4 distinct legacy irq's across CPU's
>>> better than a single shared irq.
>> Don't know if I'm facing similar problem, however devices connected in miniPCI slot behind
>> a PCIe-to-PCI bridge (MSI is disabled) using INTA all is working ok, including shared IRQ.
>> In case of INTB will not work, and the GIC irq quite often get stuck.
>>
> Roberto,
>
> What board/platform is this and what does /proc/interrupts look like?

It's a custom board

root at voneus-janas-imx6q:~# cat /proc/interrupts
           CPU0       CPU1       CPU2       CPU3
 16:        936        637       2057        938       GIC  29 Edge      twd
 17:          0          0          0          0       GPC  55 Level     i.MX Timer Tick
 22:        247          0          0          0       GPC  26 Level     2020000.serial
 34:          0          0          0          0  gpio-mxc   6 Edge      Factory Reset Button
267:          0          0          0          0       GPC  49 Level     imx_thermal
272:          0          0          0          0       GPC  19 Level     rtc alarm
278:          0          0          0          0       GPC   2 Level     sdma
281:        361          0          0          0       GIC 150 Level     2188000.ethernet
282:          0          0          0          0       GIC 151 Level     2188000.ethernet
283:       2882          0          0          0       GPC  25 Level     mmc0
284:         95          0          0          0       GPC  37 Level     21a4000.i2c
290:      36546          0          0          0       GPC 123 Level     PCIe PME, b4xxp
291:          2          0          0          0       GIC 137 Level     2101000.jr0
292:          0          0          0          0       GIC 138 Level     2102000.jr1
IPI0:          0          0          0          0  CPU wakeup interrupts
IPI1:          0          0          0          0  Timer broadcast interrupts
IPI2:       1642       1038       1626       1781  Rescheduling interrupts
IPI3:         95         95        122        119  Function call interrupts
IPI4:          3          0          2          0  Single function call interrupts
IPI5:          0          0          0          0  CPU stop interrupts
IPI6:          0          0          0          0  IRQ work interrupts
IPI7:          0          0          0          0  completion interrupts
Err:          0


>
> This sounds like what would happen if the downstream interrupts on the
> PCIe-to-PCI bridge are not mapped properly as was the case with a
> board I support (in which case I had to work out a bootloader fixup
> that placed a non-standard interrupt-map in the device-tree for the
> bridge). What bridge are you using?

PCIe-to-PCI bridge is a Ti XIO2001 where we are using INTA/B only wired 1:1

>
> Tim
>

[PATCH] i.MX6 PCIe: Fix imx6_pcie_deassert_core_reset() polarity

[Tim Harvey]

On Tue, Mar 29, 2016 at 9:44 AM, Roberto Fichera <kernel@tekno-soft.it> wrote:
> On 03/29/2016 06:40 PM, Tim Harvey wrote:
>> On Tue, Mar 29, 2016 at 9:13 AM, Roberto Fichera <kernel@tekno-soft.it> wrote:
>>> On 03/29/2016 05:10 PM, Tim Harvey wrote:
>>>> Arnd,
>>>>
>>>> Right, on the IMX the MSI interrupt is GIC-120 which is also the
>>>> legacy INTD and I do see that if I happen to put a radio in a slot
>>>> where due to swizzling its pin1 becomes INTD (GIC-120) the interrupt
>>>> does fire and the device works. Any other slot using GIC-123 (INTA),
>>>> GIC-122 (INTB), or GIC-121 (INTC) never fires so its very possible
>>>> that something in the designware core is masking out the legacy irqs.
>>>> I would also think this was something IMX specific, but I really don't
>>>> see any codepaths in pci-imx6.c that would cause that: a driver
>>>> requesting a legacy PCI would get a GIC interrupt which is handled by
>>>> the IMX6 gpc interrupt controller.
>>>>
>>>> Any dra7xxx, exynos, spear13xx, keystone, layerscape, hisi, qcom SoC
>>>> users of designware PCIe core out there that can verify PCI MSI and
>>>> legacy are both working at the same time?
>>>>
>>>> Lucas is the expert here and I believe he has the documentation for
>>>> the designware core that Freescale doens't provide with the IMX6
>>>> documentation so hopefully he can provide some insight. He's the one
>>>> that has authored all the MSI support and has been using it.
>>>>
>>>> I typically advise our users to 'not' enable MSI because
>>>> architecturally you can spread 4 distinct legacy irq's across CPU's
>>>> better than a single shared irq.
>>> Don't know if I'm facing similar problem, however devices connected in miniPCI slot behind
>>> a PCIe-to-PCI bridge (MSI is disabled) using INTA all is working ok, including shared IRQ.
>>> In case of INTB will not work, and the GIC irq quite often get stuck.
>>>
>> Roberto,
>>
>> What board/platform is this and what does /proc/interrupts look like?
>
> It's a custom board
>
> root at voneus-janas-imx6q:~# cat /proc/interrupts
>            CPU0       CPU1       CPU2       CPU3
>  16:        936        637       2057        938       GIC  29 Edge      twd
>  17:          0          0          0          0       GPC  55 Level     i.MX Timer Tick
>  22:        247          0          0          0       GPC  26 Level     2020000.serial
>  34:          0          0          0          0  gpio-mxc   6 Edge      Factory Reset Button
> 267:          0          0          0          0       GPC  49 Level     imx_thermal
> 272:          0          0          0          0       GPC  19 Level     rtc alarm
> 278:          0          0          0          0       GPC   2 Level     sdma
> 281:        361          0          0          0       GIC 150 Level     2188000.ethernet
> 282:          0          0          0          0       GIC 151 Level     2188000.ethernet
> 283:       2882          0          0          0       GPC  25 Level     mmc0
> 284:         95          0          0          0       GPC  37 Level     21a4000.i2c
> 290:      36546          0          0          0       GPC 123 Level     PCIe PME, b4xxp
> 291:          2          0          0          0       GIC 137 Level     2101000.jr0
> 292:          0          0          0          0       GIC 138 Level     2102000.jr1
> IPI0:          0          0          0          0  CPU wakeup interrupts
> IPI1:          0          0          0          0  Timer broadcast interrupts
> IPI2:       1642       1038       1626       1781  Rescheduling interrupts
> IPI3:         95         95        122        119  Function call interrupts
> IPI4:          3          0          2          0  Single function call interrupts
> IPI5:          0          0          0          0  CPU stop interrupts
> IPI6:          0          0          0          0  IRQ work interrupts
> IPI7:          0          0          0          0  completion interrupts
> Err:          0
>
>
>>
>> This sounds like what would happen if the downstream interrupts on the
>> PCIe-to-PCI bridge are not mapped properly as was the case with a
>> board I support (in which case I had to work out a bootloader fixup
>> that placed a non-standard interrupt-map in the device-tree for the
>> bridge). What bridge are you using?
>
> PCIe-to-PCI bridge is a Ti XIO2001 where we are using INTA/B only wired 1:1
>

Roberto,

That's right, we've talked about your bridge on IMX community.

I don't see anything in your proc/interrupts other than GPC 123 - you
probably only had one device populated when you did that. Put devices
in all for slots then show me 'cat /proc/interrupts' as well as 'lspci
-vv' (so that I can see what interrupt was given to pin1 and what
interrupt that maps to on the IMX6).

Check your XIO2001 routing and insure the following for proper IRQ mapping:
Slot12: IDSEL A28: socket INTA = XIO2001 INTA
Slot13: IDSEL A29: socket INTA = XIO2001 INTB
Slot14: IDSEL A30: socket INTA = XIO2001 INTC
Slot15: IDSEL A31: socket INTA = XIO2001 INTD

The relationship between slot number of IDSEL is based on the PCI
specification. The XIO2001 int mapping to socket mapping is based on
Table 2 in the XIO2001 implementation guide. In my case what the
hardware designer flipped the IDSEL mappings above such that slot12's
idsel was hooked to A31 (so it was really slot15) etc, which created a
non-standard mapping that required what ended up being a very time
consuming and difficult to figure out software fixup (to say the
least).

Tim

[PATCH] i.MX6 PCIe: Fix imx6_pcie_deassert_core_reset() polarity

[Roberto Fichera]

On 03/29/2016 07:31 PM, Tim Harvey wrote:
> On Tue, Mar 29, 2016 at 9:44 AM, Roberto Fichera <kernel@tekno-soft.it> wrote:
>>>
>>> Roberto,
>>>
>>> What board/platform is this and what does /proc/interrupts look like?
>> It's a custom board
>>
>> root at voneus-janas-imx6q:~# cat /proc/interrupts
>>            CPU0       CPU1       CPU2       CPU3
>>  16:        936        637       2057        938       GIC  29 Edge      twd
>>  17:          0          0          0          0       GPC  55 Level     i.MX Timer Tick
>>  22:        247          0          0          0       GPC  26 Level     2020000.serial
>>  34:          0          0          0          0  gpio-mxc   6 Edge      Factory Reset Button
>> 267:          0          0          0          0       GPC  49 Level     imx_thermal
>> 272:          0          0          0          0       GPC  19 Level     rtc alarm
>> 278:          0          0          0          0       GPC   2 Level     sdma
>> 281:        361          0          0          0       GIC 150 Level     2188000.ethernet
>> 282:          0          0          0          0       GIC 151 Level     2188000.ethernet
>> 283:       2882          0          0          0       GPC  25 Level     mmc0
>> 284:         95          0          0          0       GPC  37 Level     21a4000.i2c
>> 290:      36546          0          0          0       GPC 123 Level     PCIe PME, b4xxp
>> 291:          2          0          0          0       GIC 137 Level     2101000.jr0
>> 292:          0          0          0          0       GIC 138 Level     2102000.jr1
>> IPI0:          0          0          0          0  CPU wakeup interrupts
>> IPI1:          0          0          0          0  Timer broadcast interrupts
>> IPI2:       1642       1038       1626       1781  Rescheduling interrupts
>> IPI3:         95         95        122        119  Function call interrupts
>> IPI4:          3          0          2          0  Single function call interrupts
>> IPI5:          0          0          0          0  CPU stop interrupts
>> IPI6:          0          0          0          0  IRQ work interrupts
>> IPI7:          0          0          0          0  completion interrupts
>> Err:          0
>>
>>
>>> This sounds like what would happen if the downstream interrupts on the
>>> PCIe-to-PCI bridge are not mapped properly as was the case with a
>>> board I support (in which case I had to work out a bootloader fixup
>>> that placed a non-standard interrupt-map in the device-tree for the
>>> bridge). What bridge are you using?
>> PCIe-to-PCI bridge is a Ti XIO2001 where we are using INTA/B only wired 1:1
>>
> Roberto,
>
> That's right, we've talked about your bridge on IMX community.
>
> I don't see anything in your proc/interrupts other than GPC 123 - you
> probably only had one device populated when you did that. 

Yep! That was the case

> Put devices
> in all for slots then show me 'cat /proc/interrupts' as well as 'lspci
> -vv' (so that I can see what interrupt was given to pin1 and what
> interrupt that maps to on the IMX6).

GPC 123 is serving J2 and J1, and looks ok

root at voneus-janas-imx6q:~# cat /proc/interrupts
           CPU0       CPU1       CPU2       CPU3
 16:       7409      25043       2869      71444       GIC  29 Edge      twd
 17:          0          0          0          0       GPC  55 Level     i.MX Timer Tick
 22:        526          0          0          0       GPC  26 Level     2020000.serial
 34:          0          0          0          0  gpio-mxc   6 Edge      Factory Reset Button
267:          0          0          0          0       GPC  49 Level     imx_thermal
272:          0          0          0          0       GPC  19 Level     rtc alarm
278:          0          0          0          0       GPC   2 Level     sdma
281:       8808          0          0          0       GIC 150 Level     2188000.ethernet
282:          0          0          0          0       GIC 151 Level     2188000.ethernet
283:       2529          0          0          0       GPC  25 Level     mmc0
284:         95          0          0          0       GPC  37 Level     21a4000.i2c
290:    2391578          0          0          0       GPC 123 Level     PCIe PME, b4xxp, b4xxp
291:          2          0          0          0       GIC 137 Level     2101000.jr0
292:          0          0          0          0       GIC 138 Level     2102000.jr1
IPI0:          0          0          0          0  CPU wakeup interrupts
IPI1:          0          0          0          0  Timer broadcast interrupts
IPI2:       1122       3838       2051       9631  Rescheduling interrupts
IPI3:         60         56         48         54  Function call interrupts
IPI4:          2          1          2          3  Single function call interrupts
IPI5:          0          0          0          0  CPU stop interrupts
IPI6:          0          0          0          0  IRQ work interrupts
IPI7:          0          0          0          0  completion interrupts
Err:          0


root at voneus-janas-imx6q:~# lspci -vv -s 02:
02:00.0 ISDN controller: Cologne Chip Designs GmbH ISDN network Controller [HFC-4S] (rev 01)
        Subsystem: Cologne Chip Designs GmbH HFC-4S [OpenVox B200P / B400P]
        Control: I/O+ Mem+ BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr+ Stepping- SERR+ FastB2B- DisINTx-
        Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
        Interrupt: pin A routed to IRQ 290
        Region 0: I/O ports at 1000 [size=8]
        Region 1: Memory@01100000 (32-bit, non-prefetchable) [size=4K]
        Capabilities: [40] Power Management version 2
                Flags: PMEClk- DSI+ D1+ D2+ AuxCurrent=0mA PME(D0+,D1+,D2+,D3hot+,D3cold-)
                Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME-
        Kernel driver in use: wcb4xxp
        Kernel modules: wcb4xxp

02:04.0 ISDN controller: Cologne Chip Designs GmbH ISDN network Controller [HFC-4S] (rev 01)
        Subsystem: Cologne Chip Designs GmbH HFC-4S [OpenVox B200P / B400P]
        Control: I/O+ Mem+ BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr+ Stepping- SERR+ FastB2B- DisINTx-
        Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
        Interrupt: pin A routed to IRQ 290
        Region 0: I/O ports at 1008 [size=8]
        Region 1: Memory@01101000 (32-bit, non-prefetchable) [size=4K]
        Capabilities: [40] Power Management version 2
                Flags: PMEClk- DSI+ D1+ D2+ AuxCurrent=0mA PME(D0+,D1+,D2+,D3hot+,D3cold-)
                Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME-
        Kernel driver in use: wcb4xxp
        Kernel modules: wcb4xxp


>
> Check your XIO2001 routing and insure the following for proper IRQ mapping:
> Slot12: IDSEL A28: socket INTA = XIO2001 INTA
> Slot13: IDSEL A29: socket INTA = XIO2001 INTB
> Slot14: IDSEL A30: socket INTA = XIO2001 INTC
> Slot15: IDSEL A31: socket INTA = XIO2001 INTD

After crosschecking with our hardware designer the PCB IRQ mapping is the following:

J2 : IDSEL  A16: => Device 0 : socket INTA = XIO2001 INTA
J3 : IDSEL  A18: => Device 2 : socket INTA = XIO2001 INTA* **(This should be INTC)*         
J11: IDSEL A20: => Device 4 : socket INTA = XIO2001 INTA

The interrupt routing for J3 is wrong. The XIO2001 driver may expect Device 2 to interrupt on INTC - but it will
interrupt on INTA.

>
> The relationship between slot number of IDSEL is based on the PCI
> specification. The XIO2001 int mapping to socket mapping is based on
> Table 2 in the XIO2001 implementation guide. In my case what the
> hardware designer flipped the IDSEL mappings above such that slot12's
> idsel was hooked to A31 (so it was really slot15) etc, which created a
> non-standard mapping that required what ended up being a very time
> consuming and difficult to figure out software fixup (to say the
> least).

Yep! I have read it

>
> Tim
>

[PATCH] i.MX6 PCIe: Fix imx6_pcie_deassert_core_reset() polarity

[Arnd Bergmann]

On Wednesday 30 March 2016 10:00:33 Roberto Fichera wrote:
> >
> > Check your XIO2001 routing and insure the following for proper IRQ mapping:
> > Slot12: IDSEL A28: socket INTA = XIO2001 INTA
> > Slot13: IDSEL A29: socket INTA = XIO2001 INTB
> > Slot14: IDSEL A30: socket INTA = XIO2001 INTC
> > Slot15: IDSEL A31: socket INTA = XIO2001 INTD
> 
> After crosschecking with our hardware designer the PCB IRQ mapping is the following:
> 
> J2 : IDSEL  A16: => Device 0 : socket INTA = XIO2001 INTA
> J3 : IDSEL  A18: => Device 2 : socket INTA = XIO2001 INTA* **(This should be INTC)*         
> J11: IDSEL A20: => Device 4 : socket INTA = XIO2001 INTA
> 
> The interrupt routing for J3 is wrong. The XIO2001 driver may expect Device 2 to interrupt on INTC - but it will
> interrupt on INTA.

What does your interrupt-map property look like then? Note that you
have to override both map and map-mask in this case.

	Arnd

[PATCH] i.MX6 PCIe: Fix imx6_pcie_deassert_core_reset() polarity

[Roberto Fichera]

On 03/30/2016 12:10 PM, Arnd Bergmann wrote:
> On Wednesday 30 March 2016 10:00:33 Roberto Fichera wrote:
>>> Check your XIO2001 routing and insure the following for proper IRQ mapping:
>>> Slot12: IDSEL A28: socket INTA = XIO2001 INTA
>>> Slot13: IDSEL A29: socket INTA = XIO2001 INTB
>>> Slot14: IDSEL A30: socket INTA = XIO2001 INTC
>>> Slot15: IDSEL A31: socket INTA = XIO2001 INTD
>> After crosschecking with our hardware designer the PCB IRQ mapping is the following:
>>
>> J2 : IDSEL  A16: => Device 0 : socket INTA = XIO2001 INTA
>> J3 : IDSEL  A18: => Device 2 : socket INTA = XIO2001 INTA* **(This should be INTC)*         
>> J11: IDSEL A20: => Device 4 : socket INTA = XIO2001 INTA
>>
>> The interrupt routing for J3 is wrong. The XIO2001 driver may expect Device 2 to interrupt on INTC - but it will
>> interrupt on INTA.
> What does your interrupt-map property look like then? 

Unfortunately it seems that J3 slot doesn't work anymore. Inserting a card there, PCIe link will not come up anymore.
Likely I broke it. Looking at some spare logs I have, a card inserted in J3 will get another interrupt, was 291 however
unfortunately I don't have an usefull lspci -vv output, sorry! Will check in it against another PCB when I can.

> Note that you have to override both map and map-mask in this case.

Can you please give more details where should I have a look?

>
> 	Arnd
>

[PATCH] i.MX6 PCIe: Fix imx6_pcie_deassert_core_reset() polarity

[Tim Harvey]

On Wed, Mar 30, 2016 at 5:50 AM, Roberto Fichera <kernel@tekno-soft.it> wrote:
> On 03/30/2016 12:10 PM, Arnd Bergmann wrote:
>> On Wednesday 30 March 2016 10:00:33 Roberto Fichera wrote:
>>>> Check your XIO2001 routing and insure the following for proper IRQ mapping:
>>>> Slot12: IDSEL A28: socket INTA = XIO2001 INTA
>>>> Slot13: IDSEL A29: socket INTA = XIO2001 INTB
>>>> Slot14: IDSEL A30: socket INTA = XIO2001 INTC
>>>> Slot15: IDSEL A31: socket INTA = XIO2001 INTD
>>> After crosschecking with our hardware designer the PCB IRQ mapping is the following:
>>>
>>> J2 : IDSEL  A16: => Device 0 : socket INTA = XIO2001 INTA
>>> J3 : IDSEL  A18: => Device 2 : socket INTA = XIO2001 INTA* **(This should be INTC)*
>>> J11: IDSEL A20: => Device 4 : socket INTA = XIO2001 INTA
>>>
>>> The interrupt routing for J3 is wrong. The XIO2001 driver may expect Device 2 to interrupt on INTC - but it will
>>> interrupt on INTA.
>> What does your interrupt-map property look like then?
>
> Unfortunately it seems that J3 slot doesn't work anymore. Inserting a card there, PCIe link will not come up anymore.
> Likely I broke it. Looking at some spare logs I have, a card inserted in J3 will get another interrupt, was 291 however
> unfortunately I don't have an usefull lspci -vv output, sorry! Will check in it against another PCB when I can.
>
>> Note that you have to override both map and map-mask in this case.
>
> Can you please give more details where should I have a look?
>
>>
>>       Arnd
>>
>

Robert,

The interrupt-map / interrupt-map-mask properties are the ones that
dictate irq mapping. In most cases they are defined at the host
controller only and the kernel will assume standard interrupt
swizzling (aka barber pole'ing) through bridges and will rotate
interrupts (swizzle) for each bridge you go through. It is only if the
interrupts are 'not' mapped properly (as in your case, and as was mine
on a specific add-in card) that you need to define interrupt-map /
interrupt-map-mask for the actual bridge with the interrupt mapping
issue. So in other words, you won't have an interrupt-map/mask for
your TI XIO2001 'currently', but you need to add one to describe its
non-standard mapping.

If you look at imx6qdl.dtsi you'll see the interrupt-map/mask for
standard mapping is:

interrupt-map-mask = <0 0 0 0x7>;
interrupt-map = <0 0 0 1 &gpc GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>,
                          <0 0 0 2 &gpc GIC_SPI 122 IRQ_TYPE_LEVEL_HIGH>,
                          <0 0 0 3 &gpc GIC_SPI 121 IRQ_TYPE_LEVEL_HIGH>,
                          <0 0 0 4 &gpc GIC_SPI 120 IRQ_TYPE_LEVEL_HIGH>;

This is because on the IMX6 INTA=GIC_123, INTB=GIC_122, INTC=GIC_121,
INTD=GIC_120. The interrupt-map-mask is important because it decribes
which bits of the 'unit interrupt specifier' each map pertains to. For
the standard mapping above only the pin is important because this is
the mapping for each slot so only the last three bits of the 'unit
interrupt specifier' which has four cells is specified in the mask
(0x7).

In your case you will need to describe a map that depends not only on
pin but on slot. The first 32bit cell in the 'unit interrupt
specifier' defines the bus/domain/function (BDF) as: bus << 16 | dev
<< 11 | func <<8. This is also the same format for the first cell in
the 'reg' property that describes each PCI device.

If you are saying that your hardware did not swizzle the interrupts
for the various slots then that means the above mapping needs to be
applied to each slot the same. I

You need to nest PCI devices as they appear on the bus, specifying reg
properly. So, in your case you have a XIO2001 bridge hanging off of
the IMX6 root complex. The root complex is at BDF 0:00.0, the XIO2001
is at BDF 1:00.0, and its sockets are@bus=2. So you will need to
add the following to your device-tree (fixing pinctrl and reset-gpio
per your board specifics):

&pcie {
        pinctrl-names = "default";
        pinctrl-0 = <&pinctrl_pcie>;
        reset-gpio = <&gpio1 29 GPIO_ACTIVE_LOW>;
        status = "okay";

       /* 0:00.0 root complex */
       pcie at 0,0,0 {
               reg = <0 0 0 0 0>; /* bus=0, dev=0, func=0 */

               /* 1:00.0 TI bridge with reversed IRQ mapping */
               pcie at 1,0,0 {
                       device_type = "pci";
                       #address-cells = <3>;
                       #size-cells = <2>;
                       reg = <0x010000 0 0 0 0>; /* bus=1, dev=0, func=0 */
                       #interrupt-cells = <1>;
                       interrupt-map-mask = <0xfff00 0 0 0x7>; /*
match bus and device as well as pin */
                       interrupt-map =
                               /* MAP for INTA/B/C/D in slot 2:00.00 -
standard mapping */
                               <0x26000 0 0 1 &gpc GIC_SPI 123 IRQ_TYPE_LEVEL_H
                               <0x26000 0 0 2 &gpc GIC_SPI 122 IRQ_TYPE_LEVEL_H
                               <0x26000 0 0 3 &gpc GIC_SPI 121 IRQ_TYPE_LEVEL_H
                               <0x26000 0 0 4 &gpc GIC_SPI 120 IRQ_TYPE_LEVEL_H
                               /* MAP for INTA/B/C/D in slot 2:02.00 -
wrong mapping */
                               <0x26800 0 0 1 &gpc GIC_SPI 123 IRQ_TYPE_LEVEL_H
                               <0x26800 0 0 2 &gpc GIC_SPI 122 IRQ_TYPE_LEVEL_H
                               <0x26800 0 0 3 &gpc GIC_SPI 121 IRQ_TYPE_LEVEL_H
                               <0x26800 0 0 4 &gpc GIC_SPI 120 IRQ_TYPE_LEVEL_H
                               /* MAP for INTA/B/C/D in slot 2:04.00 -
standard mapping */
                               <0x27000 0 0 1 &gpc GIC_SPI 123 IRQ_TYPE_LEVEL_H
                               <0x27000 0 0 2 &gpc GIC_SPI 122 IRQ_TYPE_LEVEL_H
                               <0x27000 0 0 3 &gpc GIC_SPI 121 IRQ_TYPE_LEVEL_H
                               <0x27000 0 0 4 &gpc GIC_SPI 120 IRQ_TYPE_LEVEL_H
...
               };
       };
};

You will need to provide a full mapping which means you'll need to
know how INTA/B/C/D are mapped to each slot. MiniPCIe only users
INTA/B but afaik you have to specify all four. I 'think' what you are
elluding to is that the hardware engineer didn't swizzle the slots at
all so all slots are mapped with INTA->INTA, INTB->INTB, INTC->INTC,
INTD->INTD. If this is the case then you just copy the above for all
slots taking care to specify the first cell properly with b<<16 |
d<<11.

You may find http://devicetree.org/Device_Tree_Usage#Advanced_Interrupt_Mapping
to be helpful as well.

Regards,

Tim

[PATCH] i.MX6 PCIe: Fix imx6_pcie_deassert_core_reset() polarity

[Roberto Fichera]

On 03/30/2016 03:38 PM, Tim Harvey wrote:
> On Wed, Mar 30, 2016 at 5:50 AM, Roberto Fichera <kernel@tekno-soft.it> wrote:
>> On 03/30/2016 12:10 PM, Arnd Bergmann wrote:
>>> On Wednesday 30 March 2016 10:00:33 Roberto Fichera wrote:
>>>>> Check your XIO2001 routing and insure the following for proper IRQ mapping:
>>>>> Slot12: IDSEL A28: socket INTA = XIO2001 INTA
>>>>> Slot13: IDSEL A29: socket INTA = XIO2001 INTB
>>>>> Slot14: IDSEL A30: socket INTA = XIO2001 INTC
>>>>> Slot15: IDSEL A31: socket INTA = XIO2001 INTD
>>>> After crosschecking with our hardware designer the PCB IRQ mapping is the following:
>>>>
>>>> J2 : IDSEL  A16: => Device 0 : socket INTA = XIO2001 INTA
>>>> J3 : IDSEL  A18: => Device 2 : socket INTA = XIO2001 INTA* **(This should be INTC)*
>>>> J11: IDSEL A20: => Device 4 : socket INTA = XIO2001 INTA
>>>>
>>>> The interrupt routing for J3 is wrong. The XIO2001 driver may expect Device 2 to interrupt on INTC - but it will
>>>> interrupt on INTA.
>>> What does your interrupt-map property look like then?
>> Unfortunately it seems that J3 slot doesn't work anymore. Inserting a card there, PCIe link will not come up anymore.
>> Likely I broke it. Looking at some spare logs I have, a card inserted in J3 will get another interrupt, was 291 however
>> unfortunately I don't have an usefull lspci -vv output, sorry! Will check in it against another PCB when I can.
>>
>>> Note that you have to override both map and map-mask in this case.
>> Can you please give more details where should I have a look?
>>
>>>       Arnd
>>>
> Robert,
>
> The interrupt-map / interrupt-map-mask properties are the ones that
> dictate irq mapping. In most cases they are defined at the host
> controller only and the kernel will assume standard interrupt
> swizzling (aka barber pole'ing) through bridges and will rotate
> interrupts (swizzle) for each bridge you go through. It is only if the
> interrupts are 'not' mapped properly (as in your case, and as was mine
> on a specific add-in card) that you need to define interrupt-map /
> interrupt-map-mask for the actual bridge with the interrupt mapping
> issue. So in other words, you won't have an interrupt-map/mask for
> your TI XIO2001 'currently', but you need to add one to describe its
> non-standard mapping.
>
> If you look at imx6qdl.dtsi you'll see the interrupt-map/mask for
> standard mapping is:
>
> interrupt-map-mask = <0 0 0 0x7>;
> interrupt-map = <0 0 0 1 &gpc GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>,
>                           <0 0 0 2 &gpc GIC_SPI 122 IRQ_TYPE_LEVEL_HIGH>,
>                           <0 0 0 3 &gpc GIC_SPI 121 IRQ_TYPE_LEVEL_HIGH>,
>                           <0 0 0 4 &gpc GIC_SPI 120 IRQ_TYPE_LEVEL_HIGH>;
>
> This is because on the IMX6 INTA=GIC_123, INTB=GIC_122, INTC=GIC_121,
> INTD=GIC_120. The interrupt-map-mask is important because it decribes
> which bits of the 'unit interrupt specifier' each map pertains to. For
> the standard mapping above only the pin is important because this is
> the mapping for each slot so only the last three bits of the 'unit
> interrupt specifier' which has four cells is specified in the mask
> (0x7).
>
> In your case you will need to describe a map that depends not only on
> pin but on slot. The first 32bit cell in the 'unit interrupt
> specifier' defines the bus/domain/function (BDF) as: bus << 16 | dev
> << 11 | func <<8. This is also the same format for the first cell in
> the 'reg' property that describes each PCI device.
>
> If you are saying that your hardware did not swizzle the interrupts
> for the various slots then that means the above mapping needs to be
> applied to each slot the same. I
>
> You need to nest PCI devices as they appear on the bus, specifying reg
> properly. So, in your case you have a XIO2001 bridge hanging off of
> the IMX6 root complex. The root complex is at BDF 0:00.0, the XIO2001
> is at BDF 1:00.0, and its sockets are at bus=2. So you will need to
> add the following to your device-tree (fixing pinctrl and reset-gpio
> per your board specifics):
>
> &pcie {
>         pinctrl-names = "default";
>         pinctrl-0 = <&pinctrl_pcie>;
>         reset-gpio = <&gpio1 29 GPIO_ACTIVE_LOW>;
>         status = "okay";
>
>        /* 0:00.0 root complex */
>        pcie at 0,0,0 {
>                reg = <0 0 0 0 0>; /* bus=0, dev=0, func=0 */
>
>                /* 1:00.0 TI bridge with reversed IRQ mapping */
>                pcie at 1,0,0 {
>                        device_type = "pci";
>                        #address-cells = <3>;
>                        #size-cells = <2>;
>                        reg = <0x010000 0 0 0 0>; /* bus=1, dev=0, func=0 */
>                        #interrupt-cells = <1>;
>                        interrupt-map-mask = <0xfff00 0 0 0x7>; /*
> match bus and device as well as pin */
>                        interrupt-map =
>                                /* MAP for INTA/B/C/D in slot 2:00.00 -
> standard mapping */
>                                <0x26000 0 0 1 &gpc GIC_SPI 123 IRQ_TYPE_LEVEL_H
>                                <0x26000 0 0 2 &gpc GIC_SPI 122 IRQ_TYPE_LEVEL_H
>                                <0x26000 0 0 3 &gpc GIC_SPI 121 IRQ_TYPE_LEVEL_H
>                                <0x26000 0 0 4 &gpc GIC_SPI 120 IRQ_TYPE_LEVEL_H
>                                /* MAP for INTA/B/C/D in slot 2:02.00 -
> wrong mapping */
>                                <0x26800 0 0 1 &gpc GIC_SPI 123 IRQ_TYPE_LEVEL_H
>                                <0x26800 0 0 2 &gpc GIC_SPI 122 IRQ_TYPE_LEVEL_H
>                                <0x26800 0 0 3 &gpc GIC_SPI 121 IRQ_TYPE_LEVEL_H
>                                <0x26800 0 0 4 &gpc GIC_SPI 120 IRQ_TYPE_LEVEL_H
>                                /* MAP for INTA/B/C/D in slot 2:04.00 -
> standard mapping */
>                                <0x27000 0 0 1 &gpc GIC_SPI 123 IRQ_TYPE_LEVEL_H
>                                <0x27000 0 0 2 &gpc GIC_SPI 122 IRQ_TYPE_LEVEL_H
>                                <0x27000 0 0 3 &gpc GIC_SPI 121 IRQ_TYPE_LEVEL_H
>                                <0x27000 0 0 4 &gpc GIC_SPI 120 IRQ_TYPE_LEVEL_H
> ...
>                };
>        };
> };
>
> You will need to provide a full mapping which means you'll need to
> know how INTA/B/C/D are mapped to each slot. MiniPCIe only users
> INTA/B but afaik you have to specify all four. I 'think' what you are
> elluding to is that the hardware engineer didn't swizzle the slots at
> all so all slots are mapped with INTA->INTA, INTB->INTB, INTC->INTC,
> INTD->INTD. If this is the case then you just copy the above for all
> slots taking care to specify the first cell properly with b<<16 |
> d<<11.
>
> You may find http://devicetree.org/Device_Tree_Usage#Advanced_Interrupt_Mapping
> to be helpful as well.

Tim,

thanks for the detailed information. Will have a look soon.


>
> Regards,
>
> Tim
>

[PATCH] i.MX6 PCIe: Fix imx6_pcie_deassert_core_reset() polarity

[Krzysztof Hałasa]

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 Halasa

Industrial Research Institute for Automation and Measurements PIAP
Al. Jerozolimskie 202, 02-486 Warsaw, Poland

[PATCH] i.MX6 PCIe: Fix imx6_pcie_deassert_core_reset() polarity

[Tim Harvey]

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

[PATCH] i.MX6 PCIe: Fix imx6_pcie_deassert_core_reset() polarity

[Krzysztof Hałasa]

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.
-- 
Krzysztof Halasa

Industrial Research Institute for Automation and Measurements PIAP
Al. Jerozolimskie 202, 02-486 Warsaw, Poland

[PATCH] i.MX6 PCIe: Fix imx6_pcie_deassert_core_reset() polarity

[Fabio Estevam]

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.

[PATCH] i.MX6 PCIe: Fix imx6_pcie_deassert_core_reset() polarity

[Krzysztof Hałasa]

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.
-- 
Krzysztof Halasa

Industrial Research Institute for Automation and Measurements PIAP
Al. Jerozolimskie 202, 02-486 Warsaw, Poland

[PATCH] i.MX6 PCIe: Fix imx6_pcie_deassert_core_reset() polarity

[Petr Štetiar]

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

[PATCH] i.MX6 PCIe: Fix imx6_pcie_deassert_core_reset() polarity

[Fabio Estevam]

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.

[PATCH] i.MX6 PCIe: Fix imx6_pcie_deassert_core_reset() polarity

[Marcel Ziswiler]

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