Re: [PATCH v2] PCI: rockchip-host: Fix rockchip_pcie_host_init_port() PERST# handling

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello Damien,

Please see my comments below.

On 2024-03-30 04:50, Damien Le Moal wrote:
The PCIe specifications (PCI Express Electromechanical Specification rev
2.0, section 2.6.2) mandate that the PERST# signal must remain asserted
for at least 100 usec (Tperst-clk) after the PCIe reference clock
becomes stable (if a reference clock is supplied), for at least 100 msec
after the power is stable (Tpvperl).

In addition, the PCI Express Base SPecification Rev 2.0, section 6.6.1
state that the host should wait for at least 100 msec from the end of a
conventional reset (PERST# is de-asserted) before accessing the
configuration space of the attached device.

Modify rockchip_pcie_host_init_port() by adding two 100ms sleep, one
before and after bringing back PESRT signal to high using the ep_gpio
GPIO. Comments are also added to clarify this behavior.

Signed-off-by: Damien Le Moal <dlemoal@xxxxxxxxxx>
---

Changes from v1:
 - Add more specification details to the commit message.
 - Add missing msleep(100) after PERST# is deasserted.

 drivers/pci/controller/pcie-rockchip-host.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/pci/controller/pcie-rockchip-host.c
b/drivers/pci/controller/pcie-rockchip-host.c
index 300b9dc85ecc..ff2fa27bd883 100644
--- a/drivers/pci/controller/pcie-rockchip-host.c
+++ b/drivers/pci/controller/pcie-rockchip-host.c
@@ -294,6 +294,7 @@ static int rockchip_pcie_host_init_port(struct
rockchip_pcie *rockchip)
 	int err, i = MAX_LANE_NUM;
 	u32 status;

+	/* Assert PERST */
 	gpiod_set_value_cansleep(rockchip->ep_gpio, 0);

 	err = rockchip_pcie_init_port(rockchip);
@@ -322,8 +323,19 @@ static int rockchip_pcie_host_init_port(struct
rockchip_pcie *rockchip)
 	rockchip_pcie_write(rockchip, PCIE_CLIENT_LINK_TRAIN_ENABLE,
 			    PCIE_CLIENT_CONFIG);

+	/*
+	 * PCIe CME specifications mandate that PERST be asserted for at
+	 * least 100ms after power is stable.
+	 */
+	msleep(100);

Perhaps it would be slightly better to use usleep_range()
instead of msleep().

 	gpiod_set_value_cansleep(rockchip->ep_gpio, 1);

+	/*
+	 * PCIe base specifications rev 2.0 mandate that the host wait for
+	 * 100ms after completion of a conventional reset.
+	 */
+	msleep(100);

Obviously, the same comment as above applies here.

+
 	/* 500ms timeout value should be enough for Gen1/2 training */
err = readl_poll_timeout(rockchip->apb_base + PCIE_CLIENT_BASIC_STATUS1,
 				 status, PCIE_LINK_UP(status), 20,




[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux