[PATCH] PCI: Stop waiting for link status after config read failure

[PATCH] PCI: Stop waiting for link status after config read failure

[Yury Murashka]

With a nested PCIe topology with multiple layers of hotplug, a link can go
down near the bottom of the topology shortly before a link above it goes
down. In that case, pcie_wait_for_link_status() can wait for the full
timeout while every read of the link status register fails because the
device has disappeared.

Return immediately when reading the link status fails so event processing
can continue.

Signed-off-by: Yury Murashka <yurypm@arista.com>
Co-authored-by: James Sewart <jamessewart@arista.com>
---
 drivers/pci/pci.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index b2879a6be..a568d5ac1 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -4565,8 +4565,9 @@ static int pci_pm_reset(struct pci_dev *dev, bool probe)
  * @use_lt: Use the LT bit if TRUE, or the DLLLA bit if FALSE.
  * @active: Waiting for active or inactive?
  *
- * Return 0 if successful, or -ETIMEDOUT if status has not changed within
- * PCIE_LINK_RETRAIN_TIMEOUT_MS milliseconds.
+ * Return 0 if successful, -ENODEV if the link status cannot be read, or
+ * -ETIMEDOUT if status has not changed within PCIE_LINK_RETRAIN_TIMEOUT_MS
+ * milliseconds.
  */
 static int pcie_wait_for_link_status(struct pci_dev *pdev,
 				     bool use_lt, bool active)
@@ -4580,7 +4581,8 @@ static int pcie_wait_for_link_status(struct pci_dev *pdev,
 
 	end_jiffies = jiffies + msecs_to_jiffies(PCIE_LINK_RETRAIN_TIMEOUT_MS);
 	do {
-		pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &lnksta);
+		if (pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &lnksta))
+			return -ENODEV;
 		if ((lnksta & lnksta_mask) == lnksta_match)
 			return 0;
 		msleep(1);
@@ -4603,8 +4605,9 @@ static int pcie_wait_for_link_status(struct pci_dev *pdev,
  * according to @use_lt.  It is not verified whether the use of the DLLLA
  * bit is valid.
  *
- * Return 0 if successful, or -ETIMEDOUT if training has not completed
- * within PCIE_LINK_RETRAIN_TIMEOUT_MS milliseconds.
+ * Return 0 if successful, -ENODEV if the link status cannot be read, or
+ * -ETIMEDOUT if training has not completed within
+ * PCIE_LINK_RETRAIN_TIMEOUT_MS milliseconds.
  */
 int pcie_retrain_link(struct pci_dev *pdev, bool use_lt)
 {

base-commit: a500db7819c50db59e55f1b4fa1c3baa5a2616f3
-- 
2.51.0

Re: [PATCH] PCI: Stop waiting for link status after config read failure

[sashiko-bot]

> With a nested PCIe topology with multiple layers of hotplug, a link can go
> down near the bottom of the topology shortly before a link above it goes
> down. In that case, pcie_wait_for_link_status() can wait for the full
> timeout while every read of the link status register fails because the
> device has disappeared.
> 
> Return immediately when reading the link status fails so event processing
> can continue.
> 
> Signed-off-by: Yury Murashka <yurypm@arista.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904111318.1063858-1-yurypm@arista.com?part=1

Re: [PATCH] PCI: Stop waiting for link status after config read failure

[Ilpo Järvinen]

On Fri, 4 Sep 2026, Yury Murashka wrote:

> With a nested PCIe topology with multiple layers of hotplug, a link can go
> down near the bottom of the topology shortly before a link above it goes
> down. In that case, pcie_wait_for_link_status() can wait for the full
> timeout while every read of the link status register fails because the
> device has disappeared.
> 
> Return immediately when reading the link status fails so event processing
> can continue.
> 
> Signed-off-by: Yury Murashka <yurypm@arista.com>
> Co-authored-by: James Sewart <jamessewart@arista.com>
> ---
>  drivers/pci/pci.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index b2879a6be..a568d5ac1 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -4565,8 +4565,9 @@ static int pci_pm_reset(struct pci_dev *dev, bool probe)
>   * @use_lt: Use the LT bit if TRUE, or the DLLLA bit if FALSE.
>   * @active: Waiting for active or inactive?
>   *
> - * Return 0 if successful, or -ETIMEDOUT if status has not changed within
> - * PCIE_LINK_RETRAIN_TIMEOUT_MS milliseconds.
> + * Return 0 if successful, -ENODEV if the link status cannot be read, or
> + * -ETIMEDOUT if status has not changed within PCIE_LINK_RETRAIN_TIMEOUT_MS
> + * milliseconds.
>   */
>  static int pcie_wait_for_link_status(struct pci_dev *pdev,
>  				     bool use_lt, bool active)
> @@ -4580,7 +4581,8 @@ static int pcie_wait_for_link_status(struct pci_dev *pdev,
>  
>  	end_jiffies = jiffies + msecs_to_jiffies(PCIE_LINK_RETRAIN_TIMEOUT_MS);
>  	do {
> -		pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &lnksta);
> +		if (pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &lnksta))
> +			return -ENODEV;

Wouldn't it be better to base such checks on PCI_POSSIBLE_ERROR()?

If you keep the check for the case where pcie_capability_read_word() 
returns error, its return value should be converted with 
pcibios_err_to_errno(), not just return -ENODEV.

>  		if ((lnksta & lnksta_mask) == lnksta_match)
>  			return 0;
>  		msleep(1);
> @@ -4603,8 +4605,9 @@ static int pcie_wait_for_link_status(struct pci_dev *pdev,
>   * according to @use_lt.  It is not verified whether the use of the DLLLA
>   * bit is valid.
>   *
> - * Return 0 if successful, or -ETIMEDOUT if training has not completed
> - * within PCIE_LINK_RETRAIN_TIMEOUT_MS milliseconds.
> + * Return 0 if successful, -ENODEV if the link status cannot be read, or

Kerneldoc wants this formatting:

Return:

> + * -ETIMEDOUT if training has not completed within
> + * PCIE_LINK_RETRAIN_TIMEOUT_MS milliseconds.
>   */
>  int pcie_retrain_link(struct pci_dev *pdev, bool use_lt)
>  {
> 
> base-commit: a500db7819c50db59e55f1b4fa1c3baa5a2616f3
> 

-- 
 i.

Re: [PATCH] PCI: Stop waiting for link status after config read failure

[Lukas Wunner]

On Fri, Sep 04, 2026 at 11:13:18AM +0000, Yury Murashka wrote:
> With a nested PCIe topology with multiple layers of hotplug, a link can go
> down near the bottom of the topology shortly before a link above it goes
> down. In that case, pcie_wait_for_link_status() can wait for the full
> timeout while every read of the link status register fails because the
> device has disappeared.
> 
> Return immediately when reading the link status fails so event processing
> can continue.
[...]
> +++ b/drivers/pci/pci.c
> @@ -4580,7 +4581,8 @@ static int pcie_wait_for_link_status(struct pci_dev *pdev,
>  
>  	end_jiffies = jiffies + msecs_to_jiffies(PCIE_LINK_RETRAIN_TIMEOUT_MS);
>  	do {
> -		pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &lnksta);
> +		if (pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &lnksta))
> +			return -ENODEV;
>  		if ((lnksta & lnksta_mask) == lnksta_match)
>  			return 0;
>  		msleep(1);

It might be clearer if you check for pci_dev_is_disconnected() directly
instead of relying on a PCIBIOS_DEVICE_NOT_FOUND return value which is
generated as a side effect of the device being gone.

Thanks,

Lukas

Re: [PATCH] PCI: Stop waiting for link status after config read failure

[Yury M.]

what to you think about this check:
if ((pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &lnksta) ||  
PCI_POSSIBLE_ERROR(lnksta)) && pci_dev_is_disconnected(pdev))
     return -ENODEV;

On 9/4/26 13:20, Lukas Wunner wrote:
> On Fri, Sep 04, 2026 at 11:13:18AM +0000, Yury Murashka wrote:
>> With a nested PCIe topology with multiple layers of hotplug, a link can go
>> down near the bottom of the topology shortly before a link above it goes
>> down. In that case, pcie_wait_for_link_status() can wait for the full
>> timeout while every read of the link status register fails because the
>> device has disappeared.
>>
>> Return immediately when reading the link status fails so event processing
>> can continue.
> [...]
>> +++ b/drivers/pci/pci.c
>> @@ -4580,7 +4581,8 @@ static int pcie_wait_for_link_status(struct pci_dev *pdev,
>>   
>>   	end_jiffies = jiffies + msecs_to_jiffies(PCIE_LINK_RETRAIN_TIMEOUT_MS);
>>   	do {
>> -		pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &lnksta);
>> +		if (pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &lnksta))
>> +			return -ENODEV;
>>   		if ((lnksta & lnksta_mask) == lnksta_match)
>>   			return 0;
>>   		msleep(1);
> It might be clearer if you check for pci_dev_is_disconnected() directly
> instead of relying on a PCIBIOS_DEVICE_NOT_FOUND return value which is
> generated as a side effect of the device being gone.
>
> Thanks,
>
> Lukas

Re: [PATCH] PCI: Stop waiting for link status after config read failure

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 1626 bytes --]

On Fri, 4 Sep 2026, Yury M. wrote:

> what to you think about this check:
> if ((pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &lnksta) || 
> PCI_POSSIBLE_ERROR(lnksta)) && pci_dev_is_disconnected(pdev))
>     return -ENODEV;

Apparently you're ignoring my feedback. :-(

--
 i.
 
> On 9/4/26 13:20, Lukas Wunner wrote:
> > On Fri, Sep 04, 2026 at 11:13:18AM +0000, Yury Murashka wrote:
> > > With a nested PCIe topology with multiple layers of hotplug, a link can go
> > > down near the bottom of the topology shortly before a link above it goes
> > > down. In that case, pcie_wait_for_link_status() can wait for the full
> > > timeout while every read of the link status register fails because the
> > > device has disappeared.
> > > 
> > > Return immediately when reading the link status fails so event processing
> > > can continue.
> > [...]
> > > +++ b/drivers/pci/pci.c
> > > @@ -4580,7 +4581,8 @@ static int pcie_wait_for_link_status(struct pci_dev
> > > *pdev,
> > >     	end_jiffies = jiffies +
> > > msecs_to_jiffies(PCIE_LINK_RETRAIN_TIMEOUT_MS);
> > >   	do {
> > > -		pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &lnksta);
> > > +		if (pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &lnksta))
> > > +			return -ENODEV;
> > >   		if ((lnksta & lnksta_mask) == lnksta_match)
> > >   			return 0;
> > >   		msleep(1);
> > It might be clearer if you check for pci_dev_is_disconnected() directly
> > instead of relying on a PCIBIOS_DEVICE_NOT_FOUND return value which is
> > generated as a side effect of the device being gone.
> > 
> > Thanks,
> > 
> > Lukas
> 
> 

Re: [PATCH] PCI: Stop waiting for link status after config read failure

[Lukas Wunner]

On Fri, Sep 04, 2026 at 04:36:12PM +0100, Yury M. wrote:
> On 9/4/26 13:20, Lukas Wunner wrote:
> > On Fri, Sep 04, 2026 at 11:13:18AM +0000, Yury Murashka wrote:
> > > With a nested PCIe topology with multiple layers of hotplug, a link
> > > can go down near the bottom of the topology shortly before a link
> > > above it goes down. In that case, pcie_wait_for_link_status() can
> > > wait for the full timeout while every read of the link status register
> > > fails because the device has disappeared.
> > > 
> > > Return immediately when reading the link status fails so event processing
> > > can continue.
> > [...]
> > > +++ b/drivers/pci/pci.c
> > > @@ -4580,7 +4581,8 @@ static int pcie_wait_for_link_status(struct pci_dev *pdev,
> > >   	end_jiffies = jiffies + msecs_to_jiffies(PCIE_LINK_RETRAIN_TIMEOUT_MS);
> > >   	do {
> > > -		pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &lnksta);
> > > +		if (pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &lnksta))
> > > +			return -ENODEV;
> > >   		if ((lnksta & lnksta_mask) == lnksta_match)
> > >   			return 0;
> > >   		msleep(1);
> > 
> > It might be clearer if you check for pci_dev_is_disconnected() directly
> > instead of relying on a PCIBIOS_DEVICE_NOT_FOUND return value which is
> > generated as a side effect of the device being gone.
> 
> what to you think about this check:
> 
> if ((pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &lnksta) ||
>     PCI_POSSIBLE_ERROR(lnksta)) && pci_dev_is_disconnected(pdev))
> 	return -ENODEV;

It's repetitive because pcie_capability_read_word() already checks
internally for pci_dev_is_disconnected():

  pcie_capability_read_word()
    pci_read_config_word()
      pci_dev_is_disconnected()

I just thought that since you specifically want to bail out in the
hot-removal case, it might be clearer to check pci_dev_is_disconnected()
in pcie_wait_for_link_status() before performing the config space read.

But I don't feel strongly either way and checking the return value of
pcie_capability_read_word() is a viable approach as well.

Thanks,

Lukas