Re: iMX6q PCIe phy link never came up on kernel v4.4.x

[Fabio Estevam <festevam@gmail.com>]

On Wed, Mar 16, 2016 at 6:33 PM, Tim Harvey <tharvey@gateworks.com> wrote:

> Fabio,
>
> The board combination I have where an XIO2001 is connected directly to
> an IMX6 is a bit different from Roberto's setup. In our configuration
> the XIO2001 is on an 'expansion' board that its own local PCI clock
> generation. So, in my case the XIO2001 always has a valid clock
> before/during/after its reset. This is different from Roberto's
> scenario. I do recall running into an issue with the XIO2001 on
> another product with a different host controller that had to do with
> noise on the clk prior to its reset being asserted so I am not too
> surprised at what Roberto has found.
>
> I don't specifically see an issue with a change that asserts PCI_RST#
> before the CLK gets enabled then de-asserts it after at least 100ms
> has expired from clock enable - I think that actually follows the
> specs wording closer than what we currently do (turning o the clock
> prior to assert/de-assert reset). However I get very nervous at any
> change to the IMX6 PCIe init. We have found it to be very finicky
> because of the lack of a proper reset.

Would this be the minimal change to get Roberto's setup working with 4.5?

--- a/drivers/pci/host/pci-imx6.c
+++ b/drivers/pci/host/pci-imx6.c
@@ -232,6 +232,15 @@ static int imx6_pcie_assert_core_reset(struct
pcie_port *pp)
        u32 val, gpr1, gpr12;

        /*
+        * Some PCI bridges such as TI XIO2001 require PERST to be
+        * asserted before enabling the PCIe ref_clk, otherwise it will
+        * get "confused" and the PCIe link state will stuck in POLL_STATE
+        */
+
+       if (gpio_is_valid(imx6_pcie->reset_gpio))
+               gpiod_set_value_cansleep(imx6_pcie->reset_gpio, 0);
+
+       /*
         * If the bootloader already enabled the link we need some special
         * handling to get the core back into a state where it is safe to
         * touch it for configuration.  As there is no dedicated reset signal
@@ -305,7 +314,6 @@ 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);
        }

> Roberto,
>
> Did you require the changes regarding Gen2 negotiation? My
> IMX6+XIO2001 links reliably at Gen1 which makes sense for that chip.

Yes, it would be nice if Roberto could clarify if the Gen2 changes are
needed or not.

Also, is this change also really required?

     regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
             IMX6Q_GPR1_PCIE_TEST_PD, 1 << 18);
+    udelay(10);
     regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
             IMX6Q_GPR1_PCIE_REF_CLK_EN, 0 << 16);