On 4/13/24 06:29, Bjorn Helgaas wrote: > On Fri, Apr 12, 2024 at 11:37:21AM +0900, Damien Le Moal wrote: >> The PCI Express Base Specification r6.0, section 6.6.1, states that the >> host should wait for at least 100 msec from the end of a conventional >> reset (PERST# is de-asserted) before sending a configuration request to >> ensure that the device is able to respond with a "Request Retry Status" >> completion. >> >> Add the PCIE_T_RRS_READY_MS macro to define this wait time and modify >> rockchip_pcie_host_init_port() to add this 100ms sleep after bringing >> back PESRT# signal to high using the ep_gpio GPIO. > > s/PESRT#/PERST#/ > s/bringing back PERST# signal to high/deasserting PERST#/ > >> Suggested-by: Bjorn Helgaas <helgaas@xxxxxxxxxx> >> Signed-off-by: Damien Le Moal <dlemoal@xxxxxxxxxx> >> --- >> drivers/pci/controller/pcie-rockchip-host.c | 2 ++ >> drivers/pci/pci.h | 7 +++++++ >> 2 files changed, 9 insertions(+) >> >> diff --git a/drivers/pci/controller/pcie-rockchip-host.c b/drivers/pci/controller/pcie-rockchip-host.c >> index fc868251e570..cbec71114825 100644 >> --- a/drivers/pci/controller/pcie-rockchip-host.c >> +++ b/drivers/pci/controller/pcie-rockchip-host.c >> @@ -325,6 +325,8 @@ static int rockchip_pcie_host_init_port(struct rockchip_pcie *rockchip) >> msleep(PCIE_T_PVPERL_MS); >> gpiod_set_value_cansleep(rockchip->ep_gpio, 1); >> >> + msleep(PCIE_T_RRS_READY_MS); >> + >> /* 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, >> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h >> index 17fed1846847..c93ffc5e6e1f 100644 >> --- a/drivers/pci/pci.h >> +++ b/drivers/pci/pci.h >> @@ -16,6 +16,13 @@ >> /* Power stable to PERST# inactive from PCIe card Electromechanical Spec */ >> #define PCIE_T_PVPERL_MS 100 >> >> +/* >> + * End of conventional reset (PERST# de-asserted) to first configuration >> + * request (device able to respond with a "Request Retry Status" completion), >> + * from PCI Express Base Specification r6.0, section 6.6.1. > > "PCIe r6.0, sec 6.6.1" to match typical style, e.g., the reference > just below. > > Whoever applies this can take care of this. To make it easier to apply, I sent v4 with the corrections. Thanks. > >> +#define PCIE_T_RRS_READY_MS 100 > > Thanks a lot for doing this; there are many similar places we can > update to use this #define. > >> /* >> * PCIe r6.0, sec 5.3.3.2.1 <PME Synchronization> >> * Recommends 1ms to 10ms timeout to check L2 ready. >> -- >> 2.44.0 >> -- Damien Le Moal Western Digital Research