* [PATCH v3 0/2] rockchip rk3399 port initialization fixes @ 2024-04-12 2:37 Damien Le Moal 2024-04-12 2:37 ` [PATCH v3 1/2] PCI: rockchip-host: Fix rockchip_pcie_host_init_port() PERST# handling Damien Le Moal 2024-04-12 2:37 ` [PATCH v3 2/2] PCI: rockchip-host: Wait 100ms after reset before starting configuration Damien Le Moal 0 siblings, 2 replies; 7+ messages in thread From: Damien Le Moal @ 2024-04-12 2:37 UTC (permalink / raw) To: Shawn Lin, Bjorn Helgaas, Heiko Stuebner, linux-pci Cc: Lorenzo Pieralisi, Krzysztof Wilczyński, linux-rockchip, linux-arm-kernel A couple of patches to have the rockchip rk3399 host controller port initialization follow the PCI specifications around PERST# signal timing handling. Changes from v2: - Use PCIE_T_PVPERL_MS macro in patch 1 (and remove useless comments). - Split second msleep() addition into patch 2 as suggested by Bjorn. Changes from v1: - Add more specification details to the commit message. - Add missing msleep(100) after PERST# is deasserted. Damien Le Moal (2): PCI: rockchip-host: Fix rockchip_pcie_host_init_port() PERST# handling PCI: rockchip-host: Wait 100ms after reset before starting configuration drivers/pci/controller/pcie-rockchip-host.c | 3 +++ drivers/pci/pci.h | 7 +++++++ 2 files changed, 10 insertions(+) -- 2.44.0 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v3 1/2] PCI: rockchip-host: Fix rockchip_pcie_host_init_port() PERST# handling 2024-04-12 2:37 [PATCH v3 0/2] rockchip rk3399 port initialization fixes Damien Le Moal @ 2024-04-12 2:37 ` Damien Le Moal 2024-04-12 21:26 ` Bjorn Helgaas 2024-04-15 6:48 ` Manivannan Sadhasivam 2024-04-12 2:37 ` [PATCH v3 2/2] PCI: rockchip-host: Wait 100ms after reset before starting configuration Damien Le Moal 1 sibling, 2 replies; 7+ messages in thread From: Damien Le Moal @ 2024-04-12 2:37 UTC (permalink / raw) To: Shawn Lin, Bjorn Helgaas, Heiko Stuebner, linux-pci Cc: Lorenzo Pieralisi, Krzysztof Wilczyński, linux-rockchip, linux-arm-kernel 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), and for at least 100 msec after the power is stable (Tpvperl, defined by the macro PCIE_T_PVPERL_MS). Modify rockchip_pcie_host_init_port() to satisfy these constraints by adding a sleep period before bringing back PESRT# signal to high using the ep_gpio GPIO. Since Tperst-clk is the shorter wait time, add an msleep() call for the longer PCIE_T_PVPERL_MS milliseconds to handle both timing requirements. Signed-off-by: Damien Le Moal <dlemoal@kernel.org> --- drivers/pci/controller/pcie-rockchip-host.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/pci/controller/pcie-rockchip-host.c b/drivers/pci/controller/pcie-rockchip-host.c index 300b9dc85ecc..fc868251e570 100644 --- a/drivers/pci/controller/pcie-rockchip-host.c +++ b/drivers/pci/controller/pcie-rockchip-host.c @@ -322,6 +322,7 @@ static int rockchip_pcie_host_init_port(struct rockchip_pcie *rockchip) rockchip_pcie_write(rockchip, PCIE_CLIENT_LINK_TRAIN_ENABLE, PCIE_CLIENT_CONFIG); + msleep(PCIE_T_PVPERL_MS); gpiod_set_value_cansleep(rockchip->ep_gpio, 1); /* 500ms timeout value should be enough for Gen1/2 training */ -- 2.44.0 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v3 1/2] PCI: rockchip-host: Fix rockchip_pcie_host_init_port() PERST# handling 2024-04-12 2:37 ` [PATCH v3 1/2] PCI: rockchip-host: Fix rockchip_pcie_host_init_port() PERST# handling Damien Le Moal @ 2024-04-12 21:26 ` Bjorn Helgaas 2024-04-15 6:48 ` Manivannan Sadhasivam 1 sibling, 0 replies; 7+ messages in thread From: Bjorn Helgaas @ 2024-04-12 21:26 UTC (permalink / raw) To: Damien Le Moal Cc: Shawn Lin, Bjorn Helgaas, Heiko Stuebner, linux-pci, Lorenzo Pieralisi, Krzysztof Wilczyński, linux-rockchip, linux-arm-kernel On Fri, Apr 12, 2024 at 11:37:20AM +0900, 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 "PCIe CEM r5.1, sec 2.9.2" > for at least 100 usec (Tperst-clk) after the PCIe reference clock > becomes stable (if a reference clock is supplied), and for at least > 100 msec after the power is stable (Tpvperl, defined by the macro > PCIE_T_PVPERL_MS). > > Modify rockchip_pcie_host_init_port() to satisfy these constraints by > adding a sleep period before bringing back PESRT# signal to high using s/PESRT#/PERST#/ s/bringing back PERST# to high/deasserting PERST#/ Whoever applies this can probably fix these up for you. > the ep_gpio GPIO. Since Tperst-clk is the shorter wait time, add an > msleep() call for the longer PCIE_T_PVPERL_MS milliseconds to handle > both timing requirements. > > Signed-off-by: Damien Le Moal <dlemoal@kernel.org> > --- > drivers/pci/controller/pcie-rockchip-host.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/pci/controller/pcie-rockchip-host.c b/drivers/pci/controller/pcie-rockchip-host.c > index 300b9dc85ecc..fc868251e570 100644 > --- a/drivers/pci/controller/pcie-rockchip-host.c > +++ b/drivers/pci/controller/pcie-rockchip-host.c > @@ -322,6 +322,7 @@ static int rockchip_pcie_host_init_port(struct rockchip_pcie *rockchip) > rockchip_pcie_write(rockchip, PCIE_CLIENT_LINK_TRAIN_ENABLE, > PCIE_CLIENT_CONFIG); > > + msleep(PCIE_T_PVPERL_MS); Looks good, thanks! > gpiod_set_value_cansleep(rockchip->ep_gpio, 1); > > /* 500ms timeout value should be enough for Gen1/2 training */ > -- > 2.44.0 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 1/2] PCI: rockchip-host: Fix rockchip_pcie_host_init_port() PERST# handling 2024-04-12 2:37 ` [PATCH v3 1/2] PCI: rockchip-host: Fix rockchip_pcie_host_init_port() PERST# handling Damien Le Moal 2024-04-12 21:26 ` Bjorn Helgaas @ 2024-04-15 6:48 ` Manivannan Sadhasivam 1 sibling, 0 replies; 7+ messages in thread From: Manivannan Sadhasivam @ 2024-04-15 6:48 UTC (permalink / raw) To: Damien Le Moal Cc: Shawn Lin, Bjorn Helgaas, Heiko Stuebner, linux-pci, Lorenzo Pieralisi, Krzysztof Wilczyński, linux-rockchip, linux-arm-kernel On Fri, Apr 12, 2024 at 11:37:20AM +0900, 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), and for at least > 100 msec after the power is stable (Tpvperl, defined by the macro > PCIE_T_PVPERL_MS). > > Modify rockchip_pcie_host_init_port() to satisfy these constraints by > adding a sleep period before bringing back PESRT# signal to high using > the ep_gpio GPIO. Since Tperst-clk is the shorter wait time, add an > msleep() call for the longer PCIE_T_PVPERL_MS milliseconds to handle > both timing requirements. > > Signed-off-by: Damien Le Moal <dlemoal@kernel.org> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> - Mani > --- > drivers/pci/controller/pcie-rockchip-host.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/pci/controller/pcie-rockchip-host.c b/drivers/pci/controller/pcie-rockchip-host.c > index 300b9dc85ecc..fc868251e570 100644 > --- a/drivers/pci/controller/pcie-rockchip-host.c > +++ b/drivers/pci/controller/pcie-rockchip-host.c > @@ -322,6 +322,7 @@ static int rockchip_pcie_host_init_port(struct rockchip_pcie *rockchip) > rockchip_pcie_write(rockchip, PCIE_CLIENT_LINK_TRAIN_ENABLE, > PCIE_CLIENT_CONFIG); > > + msleep(PCIE_T_PVPERL_MS); > gpiod_set_value_cansleep(rockchip->ep_gpio, 1); > > /* 500ms timeout value should be enough for Gen1/2 training */ > -- > 2.44.0 > > -- மணிவண்ணன் சதாசிவம் _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v3 2/2] PCI: rockchip-host: Wait 100ms after reset before starting configuration 2024-04-12 2:37 [PATCH v3 0/2] rockchip rk3399 port initialization fixes Damien Le Moal 2024-04-12 2:37 ` [PATCH v3 1/2] PCI: rockchip-host: Fix rockchip_pcie_host_init_port() PERST# handling Damien Le Moal @ 2024-04-12 2:37 ` Damien Le Moal 2024-04-12 21:29 ` Bjorn Helgaas 1 sibling, 1 reply; 7+ messages in thread From: Damien Le Moal @ 2024-04-12 2:37 UTC (permalink / raw) To: Shawn Lin, Bjorn Helgaas, Heiko Stuebner, linux-pci Cc: Lorenzo Pieralisi, Krzysztof Wilczyński, linux-rockchip, linux-arm-kernel 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(+) 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. -- 2.44.0 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v3 2/2] PCI: rockchip-host: Wait 100ms after reset before starting configuration 2024-04-12 2:37 ` [PATCH v3 2/2] PCI: rockchip-host: Wait 100ms after reset before starting configuration Damien Le Moal @ 2024-04-12 21:29 ` Bjorn Helgaas 2024-04-13 1:50 ` Damien Le Moal 0 siblings, 1 reply; 7+ messages in thread From: Bjorn Helgaas @ 2024-04-12 21:29 UTC (permalink / raw) To: Damien Le Moal Cc: Shawn Lin, Bjorn Helgaas, Heiko Stuebner, linux-pci, Lorenzo Pieralisi, Krzysztof Wilczyński, linux-rockchip, linux-arm-kernel 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 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 2/2] PCI: rockchip-host: Wait 100ms after reset before starting configuration 2024-04-12 21:29 ` Bjorn Helgaas @ 2024-04-13 1:50 ` Damien Le Moal 0 siblings, 0 replies; 7+ messages in thread From: Damien Le Moal @ 2024-04-13 1:50 UTC (permalink / raw) To: Bjorn Helgaas Cc: Shawn Lin, Bjorn Helgaas, Heiko Stuebner, linux-pci, Lorenzo Pieralisi, Krzysztof Wilczyński, linux-rockchip, linux-arm-kernel 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 >> -- Damien Le Moal Western Digital Research _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-04-15 6:48 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-04-12 2:37 [PATCH v3 0/2] rockchip rk3399 port initialization fixes Damien Le Moal 2024-04-12 2:37 ` [PATCH v3 1/2] PCI: rockchip-host: Fix rockchip_pcie_host_init_port() PERST# handling Damien Le Moal 2024-04-12 21:26 ` Bjorn Helgaas 2024-04-15 6:48 ` Manivannan Sadhasivam 2024-04-12 2:37 ` [PATCH v3 2/2] PCI: rockchip-host: Wait 100ms after reset before starting configuration Damien Le Moal 2024-04-12 21:29 ` Bjorn Helgaas 2024-04-13 1:50 ` Damien Le Moal
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).