linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Miquel Raynal <miquel.raynal@bootlin.com>
To: Gregory Clement <gregory.clement@bootlin.com>,
	Jason Cooper <jason@lakedaemon.net>, Andrew Lunn <andrew@lunn.ch>,
	Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>,
	Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
	Bjorn Helgaas <bhelgaas@google.com>
Cc: <devicetree@vger.kernel.org>, Rob Herring <robh+dt@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
	linux-pci@vger.kernel.org, <linux-kernel@vger.kernel.org>,
	<linux-arm-kernel@lists.infradead.org>,
	Antoine Tenart <antoine.tenart@bootlin.com>,
	Maxime Chevallier <maxime.chevallier@bootlin.com>,
	Nadav Haklai <nadavh@marvell.com>,
	Miquel Raynal <miquel.raynal@bootlin.com>
Subject: [PATCH v3 01/15] PCI: aardvark: Enlarge PIO timeout
Date: Tue,  8 Jan 2019 17:24:26 +0100	[thread overview]
Message-ID: <20190108162441.5278-2-miquel.raynal@bootlin.com> (raw)
In-Reply-To: <20190108162441.5278-1-miquel.raynal@bootlin.com>

This delay has been proven to work until now, however, there is a
reproducible way to fail the driver probe because of it.

Adding the support for the PCIe gated peripheral clock that feeds the
Aardvark IP defers the Aardvark driver probe at boot time. The probe
functions calls pci_host_probe() at its end. This is where the timeout
(sometimes) resides, leading to a kernel panic the next time a
register is accessed. Here is the function call sequence:

    advk_pcie_probe()
    pci_host_probe()
    pci_scan_root_bus_bridge()
    pci_scan_child_bus_extend()
    pci_scan_bridge_extend()
    pci_scan_child_bus_extend()
    pci_scan_slot()
    pcie_aspm_init_link_state()
    pcie_aspm_cap_init()

pcie_aspm_cap_init() is in charge of doing the initial ASPM state
setup (PCIe Active-State Power Management). The logic at the top of
the function is:

    1/ Read upstream and downstream components' register state.
    2/ Configure the common clock before checking latencies.
    3/ Read again upstream and downstream component's register state.
    4/ ...

Experimentation shows that while 1/ always work, 3/ sometimes timeouts
when reading the downstream components' register state. Each read
operation is handled by the following sequence:

    pcie_capability_read_dword()
    pci_bus_read_config_dword()
    advk_pcie_rd_conf()

advk_pcie_rd_conf() just configures a few registers and polls on the
ISR bit before reading the value that has been retrieved through the
interconnect. The polling timeout of the ISR bit is set to 1ms. While
this value seems to fit most of the situations, in our case it is
sometimes crossed when reading PCI_EXP_LNKCAP (PCI link capacities)
from the downstream component. It has been measured that most of the
time that the read lasts longer than 1ms, it rises ~24ms. In my tests,
using twice this delay always works. The root cause and the reason why
the timeout only appearing when the probe is delayed is unknown
though.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/pci/controller/pci-aardvark.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
index 750081c1cb48..18120e312ae1 100644
--- a/drivers/pci/controller/pci-aardvark.c
+++ b/drivers/pci/controller/pci-aardvark.c
@@ -175,7 +175,7 @@
 	(PCIE_CONF_BUS(bus) | PCIE_CONF_DEV(PCI_SLOT(devfn))	| \
 	 PCIE_CONF_FUNC(PCI_FUNC(devfn)) | PCIE_CONF_REG(where))
 
-#define PIO_TIMEOUT_MS			1
+#define PIO_TIMEOUT_MS			50
 
 #define LINK_WAIT_MAX_RETRIES		10
 #define LINK_WAIT_USLEEP_MIN		90000
-- 
2.19.1


  reply	other threads:[~2019-01-08 16:26 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-08 16:24 [PATCH v3 00/15] Bring suspend to RAM support to PCIe Aardvark driver Miquel Raynal
2019-01-08 16:24 ` Miquel Raynal [this message]
2019-01-08 16:24 ` [PATCH v3 02/15] PCI: aardvark: Configure more registers in the configuration helper Miquel Raynal
2019-01-08 16:24 ` [PATCH v3 03/15] PCI: aardvark: Add clock support Miquel Raynal
2019-01-08 16:24 ` [PATCH v3 04/15] PCI: aardvark: Add PHY support Miquel Raynal
2019-01-08 16:24 ` [PATCH v3 05/15] PCI: aardvark: Add PCIe warm reset support Miquel Raynal
2019-01-08 16:24 ` [PATCH v3 06/15] PCI: aardvark: Add external reset GPIO support Miquel Raynal
2019-01-08 16:24 ` [PATCH v3 07/15] PCI: aardvark: Add suspend to RAM support Miquel Raynal
2019-01-08 16:24 ` [PATCH v3 08/15] dt-bindings: PCI: aardvark: Describe the clocks property Miquel Raynal
2019-01-08 16:24 ` [PATCH v3 09/15] dt-bindings: PCI: aardvark: Describe the PHY property Miquel Raynal
2019-01-08 16:24 ` [PATCH v3 10/15] dt-bindings: PCI: aardvark: Describe the PCIe endpoint card reset pins Miquel Raynal
2019-01-15 20:13   ` Rob Herring
2019-01-08 16:24 ` [PATCH v3 11/15] dt-bindings: PCI: aardvark: Describe the reset-gpios property Miquel Raynal
2019-01-08 16:24 ` [PATCH v3 12/15] ARM64: dts: marvell: armada-37xx: declare PCIe clock Miquel Raynal
2019-01-08 16:24 ` [PATCH v3 13/15] ARM64: dts: marvell: armada-3720-espressobin: declare PCIe PHY Miquel Raynal
2019-02-06 14:17   ` Gregory CLEMENT
2019-01-08 16:24 ` [PATCH v3 14/15] ARM64: dts: marvell: armada-37xx: declare PCIe reset pin Miquel Raynal
2019-02-06 11:11   ` Gregory CLEMENT
2019-01-08 16:24 ` [PATCH v3 15/15] ARM64: dts: marvell: armada-3720-espressobin: declare PCIe warm " Miquel Raynal
2019-02-06 11:12   ` Gregory CLEMENT
2019-01-18 16:51 ` [PATCH v3 00/15] Bring suspend to RAM support to PCIe Aardvark driver Gregory CLEMENT
2019-01-20 15:16   ` Miquel Raynal
2019-01-23 17:05 ` Lorenzo Pieralisi
2019-01-25 10:05   ` Miquel Raynal
2019-01-25 12:40     ` Lorenzo Pieralisi
2019-01-25 12:57       ` Miquel Raynal
2019-01-25 17:38         ` Lorenzo Pieralisi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190108162441.5278-2-miquel.raynal@bootlin.com \
    --to=miquel.raynal@bootlin.com \
    --cc=andrew@lunn.ch \
    --cc=antoine.tenart@bootlin.com \
    --cc=bhelgaas@google.com \
    --cc=devicetree@vger.kernel.org \
    --cc=gregory.clement@bootlin.com \
    --cc=jason@lakedaemon.net \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=lorenzo.pieralisi@arm.com \
    --cc=mark.rutland@arm.com \
    --cc=maxime.chevallier@bootlin.com \
    --cc=nadavh@marvell.com \
    --cc=robh+dt@kernel.org \
    --cc=sebastian.hesselbarth@gmail.com \
    --cc=thomas.petazzoni@bootlin.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).