Replace the manual `do-while` polling loops with the readl_poll_timeout()
helper when checking the link DL_UP and DL_LINK_ACTIVE status bits
during link bring-up. This simplifies the code by removing the open-coded
timeout logic in favor of the standard, more robust iopoll framework.
The change improves readability and reduces code duplication.
Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: Mikko Perttunen <mperttunen@nvidia.com>
Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
v1: dropped the include <linux/iopoll.h> header file.
---
drivers/pci/controller/pci-tegra.c | 37 +++++++++++-------------------
1 file changed, 14 insertions(+), 23 deletions(-)
diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c
index 07a61d902eae..b0056818a203 100644
--- a/drivers/pci/controller/pci-tegra.c
+++ b/drivers/pci/controller/pci-tegra.c
@@ -2169,37 +2169,28 @@ static bool tegra_pcie_port_check_link(struct tegra_pcie_port *port)
value |= RP_PRIV_MISC_PRSNT_MAP_EP_PRSNT;
writel(value, port->base + RP_PRIV_MISC);
- do {
- unsigned int timeout = TEGRA_PCIE_LINKUP_TIMEOUT;
+ while (retries--) {
+ int err;
- do {
- value = readl(port->base + RP_VEND_XP);
-
- if (value & RP_VEND_XP_DL_UP)
- break;
-
- usleep_range(1000, 2000);
- } while (--timeout);
-
- if (!timeout) {
+ err = readl_poll_timeout(port->base + RP_VEND_XP, value,
+ value & RP_VEND_XP_DL_UP,
+ 1000,
+ TEGRA_PCIE_LINKUP_TIMEOUT * 1000);
+ if (err) {
dev_dbg(dev, "link %u down, retrying\n", port->index);
goto retry;
}
- timeout = TEGRA_PCIE_LINKUP_TIMEOUT;
-
- do {
- value = readl(port->base + RP_LINK_CONTROL_STATUS);
-
- if (value & RP_LINK_CONTROL_STATUS_DL_LINK_ACTIVE)
- return true;
-
- usleep_range(1000, 2000);
- } while (--timeout);
+ err = readl_poll_timeout(port->base + RP_LINK_CONTROL_STATUS,
+ value,
+ value & RP_LINK_CONTROL_STATUS_DL_LINK_ACTIVE,
+ 1000, TEGRA_PCIE_LINKUP_TIMEOUT * 1000);
+ if (!err)
+ return true;
retry:
tegra_pcie_port_reset(port);
- } while (--retries);
+ }
return false;
}
--
2.50.1On Fri, Sep 26, 2025 at 12:57:44PM +0530, Anand Moon wrote:
> Replace the manual `do-while` polling loops with the readl_poll_timeout()
> helper when checking the link DL_UP and DL_LINK_ACTIVE status bits
> during link bring-up. This simplifies the code by removing the open-coded
> timeout logic in favor of the standard, more robust iopoll framework.
> The change improves readability and reduces code duplication.
>
> Cc: Thierry Reding <thierry.reding@gmail.com>
> Cc: Mikko Perttunen <mperttunen@nvidia.com>
> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> ---
> v1: dropped the include <linux/iopoll.h> header file.
> ---
> drivers/pci/controller/pci-tegra.c | 37 +++++++++++-------------------
> 1 file changed, 14 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c
> index 07a61d902eae..b0056818a203 100644
> --- a/drivers/pci/controller/pci-tegra.c
> +++ b/drivers/pci/controller/pci-tegra.c
> @@ -2169,37 +2169,28 @@ static bool tegra_pcie_port_check_link(struct tegra_pcie_port *port)
> value |= RP_PRIV_MISC_PRSNT_MAP_EP_PRSNT;
> writel(value, port->base + RP_PRIV_MISC);
>
> - do {
> - unsigned int timeout = TEGRA_PCIE_LINKUP_TIMEOUT;
> + while (retries--) {
> + int err;
>
> - do {
> - value = readl(port->base + RP_VEND_XP);
> -
> - if (value & RP_VEND_XP_DL_UP)
> - break;
> -
> - usleep_range(1000, 2000);
> - } while (--timeout);
> -
> - if (!timeout) {
> + err = readl_poll_timeout(port->base + RP_VEND_XP, value,
> + value & RP_VEND_XP_DL_UP,
> + 1000,
The delay between the iterations had range of (1000, 2000), now it will become
(250, 1000). How can you ensure that this delay is sufficient?
- Mani
--
மணிவண்ணன் சதாசிவம்
Hi Manivannan,
Thanks for your review comment.
On Sun, 19 Oct 2025 at 13:20, Manivannan Sadhasivam <mani@kernel.org> wrote:
>
> On Fri, Sep 26, 2025 at 12:57:44PM +0530, Anand Moon wrote:
> > Replace the manual `do-while` polling loops with the readl_poll_timeout()
> > helper when checking the link DL_UP and DL_LINK_ACTIVE status bits
> > during link bring-up. This simplifies the code by removing the open-coded
> > timeout logic in favor of the standard, more robust iopoll framework.
> > The change improves readability and reduces code duplication.
> >
> > Cc: Thierry Reding <thierry.reding@gmail.com>
> > Cc: Mikko Perttunen <mperttunen@nvidia.com>
> > Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> > ---
> > v1: dropped the include <linux/iopoll.h> header file.
> > ---
> > drivers/pci/controller/pci-tegra.c | 37 +++++++++++-------------------
> > 1 file changed, 14 insertions(+), 23 deletions(-)
> >
> > diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c
> > index 07a61d902eae..b0056818a203 100644
> > --- a/drivers/pci/controller/pci-tegra.c
> > +++ b/drivers/pci/controller/pci-tegra.c
> > @@ -2169,37 +2169,28 @@ static bool tegra_pcie_port_check_link(struct tegra_pcie_port *port)
> > value |= RP_PRIV_MISC_PRSNT_MAP_EP_PRSNT;
> > writel(value, port->base + RP_PRIV_MISC);
> >
> > - do {
> > - unsigned int timeout = TEGRA_PCIE_LINKUP_TIMEOUT;
> > + while (retries--) {
> > + int err;
> >
> > - do {
> > - value = readl(port->base + RP_VEND_XP);
> > -
> > - if (value & RP_VEND_XP_DL_UP)
> > - break;
> > -
> > - usleep_range(1000, 2000);
> > - } while (--timeout);
> > -
> > - if (!timeout) {
> > + err = readl_poll_timeout(port->base + RP_VEND_XP, value,
> > + value & RP_VEND_XP_DL_UP,
> > + 1000,
>
> The delay between the iterations had range of (1000, 2000), now it will become
> (250, 1000). How can you ensure that this delay is sufficient?
>
I asked if the timeout should be increased for the loops, but Mikko
Perttunen said that 200ms delay is fine.
[1] https://lore.kernel.org/linux-tegra/CANAwSgT615R32WTBzi2-8FYntmaxbmVRLmA3yi+=4ryH43aaWQ@mail.gmail.com/#t
> - Mani
>
Thanks
-Anand
> --
> மணிவண்ணன் சதாசிவம்
On Mon, Oct 20, 2025 at 05:47:15PM +0530, Anand Moon wrote:
> Hi Manivannan,
>
> Thanks for your review comment.
>
> On Sun, 19 Oct 2025 at 13:20, Manivannan Sadhasivam <mani@kernel.org> wrote:
> >
> > On Fri, Sep 26, 2025 at 12:57:44PM +0530, Anand Moon wrote:
> > > Replace the manual `do-while` polling loops with the readl_poll_timeout()
> > > helper when checking the link DL_UP and DL_LINK_ACTIVE status bits
> > > during link bring-up. This simplifies the code by removing the open-coded
> > > timeout logic in favor of the standard, more robust iopoll framework.
> > > The change improves readability and reduces code duplication.
> > >
> > > Cc: Thierry Reding <thierry.reding@gmail.com>
> > > Cc: Mikko Perttunen <mperttunen@nvidia.com>
> > > Signed-off-by: Anand Moon <linux.amoon@gmail.com>
> > > ---
> > > v1: dropped the include <linux/iopoll.h> header file.
> > > ---
> > > drivers/pci/controller/pci-tegra.c | 37 +++++++++++-------------------
> > > 1 file changed, 14 insertions(+), 23 deletions(-)
> > >
> > > diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c
> > > index 07a61d902eae..b0056818a203 100644
> > > --- a/drivers/pci/controller/pci-tegra.c
> > > +++ b/drivers/pci/controller/pci-tegra.c
> > > @@ -2169,37 +2169,28 @@ static bool tegra_pcie_port_check_link(struct tegra_pcie_port *port)
> > > value |= RP_PRIV_MISC_PRSNT_MAP_EP_PRSNT;
> > > writel(value, port->base + RP_PRIV_MISC);
> > >
> > > - do {
> > > - unsigned int timeout = TEGRA_PCIE_LINKUP_TIMEOUT;
> > > + while (retries--) {
> > > + int err;
> > >
> > > - do {
> > > - value = readl(port->base + RP_VEND_XP);
> > > -
> > > - if (value & RP_VEND_XP_DL_UP)
> > > - break;
> > > -
> > > - usleep_range(1000, 2000);
> > > - } while (--timeout);
> > > -
> > > - if (!timeout) {
> > > + err = readl_poll_timeout(port->base + RP_VEND_XP, value,
> > > + value & RP_VEND_XP_DL_UP,
> > > + 1000,
> >
> > The delay between the iterations had range of (1000, 2000), now it will become
> > (250, 1000). How can you ensure that this delay is sufficient?
> >
> I asked if the timeout should be increased for the loops, but Mikko
> Perttunen said that 200ms delay is fine.
>
readl_poll_timeout() internally uses usleep_range(), which transforms the 1000us
delay into, usleep_range(251, 1000). So the delay *could* theoretically be 251us
* 200 = ~50ms.
So I doubt it will be sifficient, as from the old code, it looks like the
hardware could take around 200ms to complete link up.
- Mani
--
மணிவண்ணன் சதாசிவம்
© 2016 - 2026 Red Hat, Inc.