| Message ID | 20240412023721.1049303-3-dlemoal@kernel.org |
|---|---|
| State | New |
| Headers | show |
| Series | rockchip rk3399 port initialization fixes | expand |
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@kernel.org> > Signed-off-by: Damien Le Moal <dlemoal@kernel.org> > --- > 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. > +#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 >
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@kernel.org> >> Signed-off-by: Damien Le Moal <dlemoal@kernel.org> >> --- >> 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 >>
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. + */ +#define PCIE_T_RRS_READY_MS 100 + /* * PCIe r6.0, sec 5.3.3.2.1 <PME Synchronization> * Recommends 1ms to 10ms timeout to check L2 ready.
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. Suggested-by: Bjorn Helgaas <helgaas@kernel.org> Signed-off-by: Damien Le Moal <dlemoal@kernel.org> --- drivers/pci/controller/pcie-rockchip-host.c | 2 ++ drivers/pci/pci.h | 7 +++++++ 2 files changed, 9 insertions(+)