2020-04-30 08:10:15

by Pali Rohár

[permalink] [raw]
Subject: [PATCH v4 05/12] PCI: aardvark: Issue PERST via GPIO

Add support for issuing PERST via GPIO specified in 'reset-gpios'
property (as described in PCI device tree bindings).

Some buggy cards (e.g. Compex WLE900VX or WLE1216) are not detected
after reboot when PERST is not issued during driver initialization.

If bootloader already enabled link training then issuing PERST has no
effect for some buggy cards (e.g. Compex WLE900VX) and these cards are
not detected. We therefore clear the LINK_TRAINING_EN register before.

It was observed that Compex WLE900VX card needs to be in PERST reset
for at least 10ms if bootloader enabled link training.

Tested on Turris MOX.

Signed-off-by: Pali Rohár <[email protected]>
---
drivers/pci/controller/pci-aardvark.c | 43 ++++++++++++++++++++++++++-
1 file changed, 42 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
index e202f954eb84..2ecc79c03ade 100644
--- a/drivers/pci/controller/pci-aardvark.c
+++ b/drivers/pci/controller/pci-aardvark.c
@@ -9,6 +9,7 @@
*/

#include <linux/delay.h>
+#include <linux/gpio.h>
#include <linux/interrupt.h>
#include <linux/irq.h>
#include <linux/irqdomain.h>
@@ -18,6 +19,7 @@
#include <linux/platform_device.h>
#include <linux/msi.h>
#include <linux/of_address.h>
+#include <linux/of_gpio.h>
#include <linux/of_pci.h>

#include "../pci.h"
@@ -204,6 +206,7 @@ struct advk_pcie {
int root_bus_nr;
int link_gen;
struct pci_bridge_emul bridge;
+ struct gpio_desc *reset_gpio;
};

static inline void advk_writel(struct advk_pcie *pcie, u32 val, u64 reg)
@@ -330,10 +333,31 @@ static void advk_pcie_train_link(struct advk_pcie *pcie)
dev_err(dev, "link never came up\n");
}

+static void advk_pcie_issue_perst(struct advk_pcie *pcie)
+{
+ u32 reg;
+
+ if (!pcie->reset_gpio)
+ return;
+
+ /* PERST does not work for some cards when link training is enabled */
+ reg = advk_readl(pcie, PCIE_CORE_CTRL0_REG);
+ reg &= ~LINK_TRAINING_EN;
+ advk_writel(pcie, reg, PCIE_CORE_CTRL0_REG);
+
+ /* 10ms delay is needed for some cards */
+ dev_info(&pcie->pdev->dev, "issuing PERST via reset GPIO for 10ms\n");
+ gpiod_set_value_cansleep(pcie->reset_gpio, 1);
+ usleep_range(10000, 11000);
+ gpiod_set_value_cansleep(pcie->reset_gpio, 0);
+}
+
static void advk_pcie_setup_hw(struct advk_pcie *pcie)
{
u32 reg;

+ advk_pcie_issue_perst(pcie);
+
/* Set to Direct mode */
reg = advk_readl(pcie, CTRL_CONFIG_REG);
reg &= ~(CTRL_MODE_MASK << CTRL_MODE_SHIFT);
@@ -406,7 +430,8 @@ static void advk_pcie_setup_hw(struct advk_pcie *pcie)

/*
* PERST# signal could have been asserted by pinctrl subsystem before
- * probe() callback has been called, making the endpoint going into
+ * probe() callback has been called or issued explicitly by reset gpio
+ * function advk_pcie_issue_perst(), making the endpoint going into
* fundamental reset. As required by PCI Express spec a delay for at
* least 100ms after such a reset before link training is needed.
*/
@@ -1046,6 +1071,22 @@ static int advk_pcie_probe(struct platform_device *pdev)
}
pcie->root_bus_nr = bus->start;

+ pcie->reset_gpio = devm_gpiod_get_from_of_node(dev, dev->of_node,
+ "reset-gpios", 0,
+ GPIOD_OUT_LOW,
+ "pcie1-reset");
+ ret = PTR_ERR_OR_ZERO(pcie->reset_gpio);
+ if (ret) {
+ if (ret == -ENOENT) {
+ pcie->reset_gpio = NULL;
+ } else {
+ if (ret != -EPROBE_DEFER)
+ dev_err(dev, "Failed to get reset-gpio: %i\n",
+ ret);
+ return ret;
+ }
+ }
+
ret = of_pci_get_max_link_speed(dev->of_node);
if (ret <= 0 || ret > 3)
pcie->link_gen = 3;
--
2.20.1


2020-04-30 08:25:42

by Pali Rohár

[permalink] [raw]
Subject: Re: [PATCH v4 05/12] PCI: aardvark: Issue PERST via GPIO

On Thursday 30 April 2020 10:06:18 Pali Rohár wrote:
> +static void advk_pcie_issue_perst(struct advk_pcie *pcie)
> +{
> + u32 reg;
> +
> + if (!pcie->reset_gpio)
> + return;
> +
> + /* PERST does not work for some cards when link training is enabled */
> + reg = advk_readl(pcie, PCIE_CORE_CTRL0_REG);
> + reg &= ~LINK_TRAINING_EN;
> + advk_writel(pcie, reg, PCIE_CORE_CTRL0_REG);
> +
> + /* 10ms delay is needed for some cards */
> + dev_info(&pcie->pdev->dev, "issuing PERST via reset GPIO for 10ms\n");
> + gpiod_set_value_cansleep(pcie->reset_gpio, 1);
> + usleep_range(10000, 11000);
> + gpiod_set_value_cansleep(pcie->reset_gpio, 0);
> +}

Just note about delay between changing GPIO reset:

In V2 there as only 1ms, but be figured out that it is not enough for
WLE900VX cards when they were already initialized in u-boot.

I tried to find in PCI specs if there is a defined timeout for this
operation. I found following 3 delay definitions which could be related:

TPVPERL - PERST# must remain active at least this long after power becomes valid
TPERST - When asserted, PERST# must remain asserted at least this long
TPERSTCLK - PERST# must remain active at least this long after any supplied reference clock is stable

In another spec they have defined also minimal values:

TPVPERL - Power stable to PERST# inactive - Min 100 ms
TPERST - PERST# active time - Min 100 us
TPERSTCLK - REFCLK stable before PERST# inactive - Min 100 us

After experimenting with those Compex WLE900VX cards, I know that 100us
delay is not enough. And I'm not sure if TPVPERL is really relevant for
this case. I understood that TPVPERL is needed when initializing power
again. And because delaying boot by another 100ms is does not have to be
acceptable if there is not strict reason for it, I rather decided to
stay with just 10ms delay.

If you know what is the correct timeout between changing GPIO reset,
please let me know and in future I can fix/reimplement it.

2020-05-07 21:22:33

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v4 05/12] PCI: aardvark: Issue PERST via GPIO

On Thu, Apr 30, 2020 at 10:22:45AM +0200, Pali Roh?r wrote:
> On Thursday 30 April 2020 10:06:18 Pali Roh?r wrote:
> > +static void advk_pcie_issue_perst(struct advk_pcie *pcie)
> > +{
> > + u32 reg;
> > +
> > + if (!pcie->reset_gpio)
> > + return;
> > +
> > + /* PERST does not work for some cards when link training is enabled */
> > + reg = advk_readl(pcie, PCIE_CORE_CTRL0_REG);
> > + reg &= ~LINK_TRAINING_EN;
> > + advk_writel(pcie, reg, PCIE_CORE_CTRL0_REG);
> > +
> > + /* 10ms delay is needed for some cards */
> > + dev_info(&pcie->pdev->dev, "issuing PERST via reset GPIO for 10ms\n");
> > + gpiod_set_value_cansleep(pcie->reset_gpio, 1);
> > + usleep_range(10000, 11000);
> > + gpiod_set_value_cansleep(pcie->reset_gpio, 0);
> > +}
>
> Just note about delay between changing GPIO reset:
>
> In V2 there as only 1ms, but be figured out that it is not enough for
> WLE900VX cards when they were already initialized in u-boot.
>
> I tried to find in PCI specs if there is a defined timeout for this
> operation. I found following 3 delay definitions which could be related:
>
> TPVPERL - PERST# must remain active at least this long after power becomes valid
> TPERST - When asserted, PERST# must remain asserted at least this long
> TPERSTCLK - PERST# must remain active at least this long after any supplied reference clock is stable
>
> In another spec they have defined also minimal values:
>
> TPVPERL - Power stable to PERST# inactive - Min 100 ms
> TPERST - PERST# active time - Min 100 us
> TPERSTCLK - REFCLK stable before PERST# inactive - Min 100 us
>
> After experimenting with those Compex WLE900VX cards, I know that 100us
> delay is not enough. And I'm not sure if TPVPERL is really relevant for
> this case. I understood that TPVPERL is needed when initializing power
> again. And because delaying boot by another 100ms is does not have to be
> acceptable if there is not strict reason for it, I rather decided to
> stay with just 10ms delay.
>
> If you know what is the correct timeout between changing GPIO reset,
> please let me know and in future I can fix/reimplement it.

I don't know, but seems like something each driver author shouldn't be
making up.

Rob