On 11/11/21 16:35, Marek Behún wrote:
From: Pali Rohár <p...@kernel.org>

Function mvebu_pcie_probe() configures PCIe controller and sometimes it
takes some time for PCIe card to bring link up. Delay mvebu_pcie_probe()
for 100ms to ensure that PCIe link is up after function finish. In the
case when no card is connected to the PCIe slot, this will delay probe
time by 100ms, which should not be problematic.

Where does this 100ms come from? From tests with your PCIe devices /
card?

Please see another comment below...

This change fixes detection and initialization of some QCA98xx cards on
the first serdes when configured in x1 mode. Default configuration of
the first serdes on A385 is x4 mode, so it looks as if some delay is
required when x4 is changed to x1 and card correctly links with A385.
Other PCIe serdes ports on A385 are x1-only, and so they don't have this
problem.

(We need this patch because in the following patch we are moving the
  configuration change from x4 to x1 from serdes driver to PCIe mvebu
  driver, because the corresponding register lives in the address space
  of the PCIe controller. Without that this explicit timeout is not
  needed, because there is an implicit timeout between change from x4 to
  x1 and probe of PCIe mvebu driver, due to the first being run in SPL
  and the second in U-Boot proper.)

Signed-off-by: Pali Rohár <p...@kernel.org>
Signed-off-by: Marek Behún <marek.be...@nic.cz>
---
  drivers/pci/pci_mvebu.c | 23 +++++++++++++++++++++++
  1 file changed, 23 insertions(+)

diff --git a/drivers/pci/pci_mvebu.c b/drivers/pci/pci_mvebu.c
index 14cd82db6f..a3364d5a59 100644
--- a/drivers/pci/pci_mvebu.c
+++ b/drivers/pci/pci_mvebu.c
@@ -22,6 +22,7 @@
  #include <asm/arch/cpu.h>
  #include <asm/arch/soc.h>
  #include <linux/bitops.h>
+#include <linux/delay.h>
  #include <linux/errno.h>
  #include <linux/ioport.h>
  #include <linux/mbus.h>
@@ -70,6 +71,9 @@ DECLARE_GLOBAL_DATA_PTR;
  #define PCIE_DEBUG_CTRL                       0x1a60
  #define  PCIE_DEBUG_SOFT_RESET                BIT(20)
+#define LINK_WAIT_RETRIES 100
+#define LINK_WAIT_TIMEOUT      1000

Wouldn't it be easier read, if this was defines like this:

#define LINK_TIMEOUT_MS         100
#define LINK_WAIT_TIMEOUT_US    1000
#define LINK_WAIT_RETRIES       ((LINK_TIMEOUT_MS * 1000) / 
LINK_WAIT_TIMEOUT_US)

Other than this:

Reviewed-by: Stefan Roese <s...@denx.de>

Thanks,
Stefan

+
  struct mvebu_pcie {
        struct pci_controller hose;
        void __iomem *base;
@@ -106,6 +110,23 @@ static inline bool mvebu_pcie_link_up(struct mvebu_pcie 
*pcie)
        return !(val & PCIE_STAT_LINK_DOWN);
  }
+static void mvebu_pcie_wait_for_link(struct mvebu_pcie *pcie)
+{
+       int retries;
+
+       /* check if the link is up or not */
+       for (retries = 0; retries < LINK_WAIT_RETRIES; retries++) {
+               if (mvebu_pcie_link_up(pcie)) {
+                       printf("%s: Link up\n", pcie->name);
+                       return;
+               }
+
+               udelay(LINK_WAIT_TIMEOUT);
+       }
+
+       printf("%s: Link down\n", pcie->name);
+}
+
  static void mvebu_pcie_set_local_bus_nr(struct mvebu_pcie *pcie, int busno)
  {
        u32 stat;
@@ -477,6 +498,8 @@ static int mvebu_pcie_probe(struct udevice *dev)
        pcie->cfgcache[(PCI_PREF_MEMORY_BASE - 0x10) / 4] =
                PCI_PREF_RANGE_TYPE_64 | (PCI_PREF_RANGE_TYPE_64 << 16);
+ mvebu_pcie_wait_for_link(pcie);
+
        return 0;
  }

Viele Grüße,
Stefan Roese

--
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de

Reply via email to