[PATCH v10] PCI: Add pcie_link_is_active() function

[PATCH v10] PCI: Add pcie_link_is_active() function

[Timothy Pearson]

Add pcie_link_is_active() function to check if the physical PCIe link is
active, replacing duplicate code in multiple locations.

Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
Signed-off-by: Shawn Anastasio <sanastasio@raptorengineering.com>
Signed-off-by: Timothy Pearson <tpearson@raptorengineering.com>
---
 drivers/pci/hotplug/pciehp.h      |  1 -
 drivers/pci/hotplug/pciehp_ctrl.c |  2 +-
 drivers/pci/hotplug/pciehp_hpc.c  | 33 +++----------------------------
 drivers/pci/pci.c                 | 31 ++++++++++++++++++++++++++---
 drivers/pci/pci.h                 |  1 +
 5 files changed, 33 insertions(+), 35 deletions(-)

diff --git a/drivers/pci/hotplug/pciehp.h b/drivers/pci/hotplug/pciehp.h
index debc79b0adfb..79df49cc9946 100644
--- a/drivers/pci/hotplug/pciehp.h
+++ b/drivers/pci/hotplug/pciehp.h
@@ -186,7 +186,6 @@ int pciehp_query_power_fault(struct controller *ctrl);
 int pciehp_card_present(struct controller *ctrl);
 int pciehp_card_present_or_link_active(struct controller *ctrl);
 int pciehp_check_link_status(struct controller *ctrl);
-int pciehp_check_link_active(struct controller *ctrl);
 bool pciehp_device_replaced(struct controller *ctrl);
 void pciehp_release_ctrl(struct controller *ctrl);
 
diff --git a/drivers/pci/hotplug/pciehp_ctrl.c b/drivers/pci/hotplug/pciehp_ctrl.c
index bcc938d4420f..6cc1b27b3b11 100644
--- a/drivers/pci/hotplug/pciehp_ctrl.c
+++ b/drivers/pci/hotplug/pciehp_ctrl.c
@@ -260,7 +260,7 @@ void pciehp_handle_presence_or_link_change(struct controller *ctrl, u32 events)
 	/* Turn the slot on if it's occupied or link is up */
 	mutex_lock(&ctrl->state_lock);
 	present = pciehp_card_present(ctrl);
-	link_active = pciehp_check_link_active(ctrl);
+	link_active = pcie_link_is_active(ctrl->pcie->port);
 	if (present <= 0 && link_active <= 0) {
 		if (ctrl->state == BLINKINGON_STATE) {
 			ctrl->state = OFF_STATE;
diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
index ebd342bda235..d29ce3715a44 100644
--- a/drivers/pci/hotplug/pciehp_hpc.c
+++ b/drivers/pci/hotplug/pciehp_hpc.c
@@ -221,33 +221,6 @@ static void pcie_write_cmd_nowait(struct controller *ctrl, u16 cmd, u16 mask)
 	pcie_do_write_cmd(ctrl, cmd, mask, false);
 }
 
-/**
- * pciehp_check_link_active() - Is the link active
- * @ctrl: PCIe hotplug controller
- *
- * Check whether the downstream link is currently active. Note it is
- * possible that the card is removed immediately after this so the
- * caller may need to take it into account.
- *
- * If the hotplug controller itself is not available anymore returns
- * %-ENODEV.
- */
-int pciehp_check_link_active(struct controller *ctrl)
-{
-	struct pci_dev *pdev = ctrl_dev(ctrl);
-	u16 lnk_status;
-	int ret;
-
-	ret = pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &lnk_status);
-	if (ret == PCIBIOS_DEVICE_NOT_FOUND || PCI_POSSIBLE_ERROR(lnk_status))
-		return -ENODEV;
-
-	ret = !!(lnk_status & PCI_EXP_LNKSTA_DLLLA);
-	ctrl_dbg(ctrl, "%s: lnk_status = %x\n", __func__, lnk_status);
-
-	return ret;
-}
-
 static bool pci_bus_check_dev(struct pci_bus *bus, int devfn)
 {
 	u32 l;
@@ -467,7 +440,7 @@ int pciehp_card_present_or_link_active(struct controller *ctrl)
 	if (ret)
 		return ret;
 
-	return pciehp_check_link_active(ctrl);
+	return pcie_link_is_active(ctrl_dev(ctrl));
 }
 
 int pciehp_query_power_fault(struct controller *ctrl)
@@ -614,7 +587,7 @@ static void pciehp_ignore_link_change(struct controller *ctrl,
 	 * Synthesize it to ensure that it is acted on.
 	 */
 	down_read_nested(&ctrl->reset_lock, ctrl->depth);
-	if (!pciehp_check_link_active(ctrl) || pciehp_device_replaced(ctrl))
+	if (!pcie_link_is_active(ctrl_dev(ctrl)) || pciehp_device_replaced(ctrl))
 		pciehp_request(ctrl, ignored_events);
 	up_read(&ctrl->reset_lock);
 }
@@ -921,7 +894,7 @@ int pciehp_slot_reset(struct pcie_device *dev)
 	pcie_capability_write_word(dev->port, PCI_EXP_SLTSTA,
 				   PCI_EXP_SLTSTA_DLLSC);
 
-	if (!pciehp_check_link_active(ctrl))
+	if (!pcie_link_is_active(ctrl_dev(ctrl)))
 		pciehp_request(ctrl, PCI_EXP_SLTSTA_DLLSC);
 
 	return 0;
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index e9448d55113b..4e96ff8ee5ec 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -4908,7 +4908,6 @@ int pci_bridge_wait_for_secondary_bus(struct pci_dev *dev, char *reset_type)
 		return 0;
 
 	if (pcie_get_speed_cap(dev) <= PCIE_SPEED_5_0GT) {
-		u16 status;
 
 		pci_dbg(dev, "waiting %d ms for downstream link\n", delay);
 		msleep(delay);
@@ -4924,8 +4923,7 @@ int pci_bridge_wait_for_secondary_bus(struct pci_dev *dev, char *reset_type)
 		if (!dev->link_active_reporting)
 			return -ENOTTY;
 
-		pcie_capability_read_word(dev, PCI_EXP_LNKSTA, &status);
-		if (!(status & PCI_EXP_LNKSTA_DLLLA))
+		if (pcie_link_is_active(dev) <= 0)
 			return -ENOTTY;
 
 		return pci_dev_wait(child, reset_type,
@@ -6230,6 +6228,33 @@ void pcie_print_link_status(struct pci_dev *dev)
 }
 EXPORT_SYMBOL(pcie_print_link_status);
 
+/**
+ * pcie_link_is_active() - Checks if the link is active or not
+ * @pdev: PCI device to query
+ *
+ * Check whether the physical link is active or not. Note it is
+ * possible that the card is removed immediately after this so the
+ * caller may need to take it into account.
+ *
+ * If the PCI device itself is not available anymore returns
+ * %-ENODEV.
+ *
+ * Return: link state, or -ENODEV if the config read failes.
+ */
+int pcie_link_is_active(struct pci_dev *pdev)
+{
+	u16 lnk_status;
+	int ret;
+
+	ret = pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &lnk_status);
+	if (ret == PCIBIOS_DEVICE_NOT_FOUND || PCI_POSSIBLE_ERROR(lnk_status))
+		return -ENODEV;
+
+	pci_dbg(pdev, "lnk_status = %#06x\n", lnk_status);
+	return !!(lnk_status & PCI_EXP_LNKSTA_DLLLA);
+}
+EXPORT_SYMBOL(pcie_link_is_active);
+
 /**
  * pci_select_bars - Make BAR mask from the type of resource
  * @dev: the PCI device for which BAR mask is made
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 12215ee72afb..cf1afb718f8a 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -231,6 +231,7 @@ static inline int pci_proc_detach_bus(struct pci_bus *bus) { return 0; }
 /* Functions for PCI Hotplug drivers to use */
 int pci_hp_add_bridge(struct pci_dev *dev);
 bool pci_hp_spurious_link_change(struct pci_dev *pdev);
+int pcie_link_is_active(struct pci_dev *dev);
 
 #if defined(CONFIG_SYSFS) && defined(HAVE_PCI_LEGACY)
 void pci_create_legacy_files(struct pci_bus *bus);
-- 
2.39.5

Re: [PATCH v10] PCI: Add pcie_link_is_active() function

[Bjorn Helgaas]

On Tue, Jun 17, 2025 at 10:41:58AM -0500, Timothy Pearson wrote:
> Add pcie_link_is_active() function to check if the physical PCIe link is
> active, replacing duplicate code in multiple locations.
> 
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> Signed-off-by: Shawn Anastasio <sanastasio@raptorengineering.com>
> Signed-off-by: Timothy Pearson <tpearson@raptorengineering.com>

Whoa, whoa, slow down.  This doesn't address all the things I
mentioned (EXPORT_SYMBOL, for example).  But four postings in 90
minutes is way too much.  There's no hurry, everybody has other things
to do, and we can only assimilate a reposting every few days.  That
way others have a chance to respond with additional feedback, and you
can address it all at once.

When you do post an updated version, consider adding a brief changelog
(e.g., what changed between v8 and v9) below the "---" line so we know
what to look for.

> +EXPORT_SYMBOL(pcie_link_is_active);

Re: [PATCH v10] PCI: Add pcie_link_is_active() function

[Timothy Pearson]

----- Original Message -----
> From: "Bjorn Helgaas" <helgaas@kernel.org>
> To: "Timothy Pearson" <tpearson@raptorengineering.com>
> Cc: "linux-pci" <linux-pci@vger.kernel.org>, "mahesh" <mahesh@linux.ibm.com>, "Oliver" <oohall@gmail.com>, "Madhavan
> Srinivasan" <maddy@linux.ibm.com>, "Michael Ellerman" <mpe@ellerman.id.au>, "Lukas Wunner" <lukas@wunner.de>
> Sent: Tuesday, June 17, 2025 11:04:28 AM
> Subject: Re: [PATCH v10] PCI: Add pcie_link_is_active() function

> On Tue, Jun 17, 2025 at 10:41:58AM -0500, Timothy Pearson wrote:
>> Add pcie_link_is_active() function to check if the physical PCIe link is
>> active, replacing duplicate code in multiple locations.
>> 
>> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
>> Signed-off-by: Shawn Anastasio <sanastasio@raptorengineering.com>
>> Signed-off-by: Timothy Pearson <tpearson@raptorengineering.com>
> 
> Whoa, whoa, slow down.  This doesn't address all the things I
> mentioned (EXPORT_SYMBOL, for example).  But four postings in 90
> minutes is way too much.  There's no hurry, everybody has other things
> to do, and we can only assimilate a reposting every few days.  That
> way others have a chance to respond with additional feedback, and you
> can address it all at once.

Understood, my apologies.  I realized I had sent a couple of incorrect versions in, and wanted to avoid the known commentary on what was done wrong.

> When you do post an updated version, consider adding a brief changelog
> (e.g., what changed between v8 and v9) below the "---" line so we know
> what to look for.
> 
> > +EXPORT_SYMBOL(pcie_link_is_active);

Will do in the future.  There is some urgency on this overall patchset as we have had hotplug support broken for many years now, and it's causing continued problems on customer deployed machines.  While we can continue to point customers at the patchset and have them compile their own kernels, patience from our customer side is wearing a bit thin for that.  I will continue to try to push this forward (along with the associated ppc patch set) at a more reasonable pace.

Thanks!

Re: [PATCH v10] PCI: Add pcie_link_is_active() function

[Timothy Pearson]

----- Original Message -----
> From: "Timothy Pearson" <tpearson@raptorengineeringinc.com>
> To: "Bjorn Helgaas" <helgaas@kernel.org>
> Cc: "linux-pci" <linux-pci@vger.kernel.org>, "mahesh" <mahesh@linux.ibm.com>, "Oliver" <oohall@gmail.com>, "Madhavan
> Srinivasan" <maddy@linux.ibm.com>, "Michael Ellerman" <mpe@ellerman.id.au>, "Lukas Wunner" <lukas@wunner.de>
> Sent: Tuesday, June 17, 2025 10:41:58 AM
> Subject: [PATCH v10] PCI: Add pcie_link_is_active() function

> Add pcie_link_is_active() function to check if the physical PCIe link is
> active, replacing duplicate code in multiple locations.

Note I have re-added EXPORT_SYMBOL here, since on further inspection it looks like we need to export the symbol after all.  The problem is that some hotplug drivers (pnv-php in particular) both a.) need this functionality and b.) may be built as a module.

I think we should still keep the definition in the PCI include directory, but the symbol itself needs to be exported.

Re: [PATCH v10] PCI: Add pcie_link_is_active() function

[Bjorn Helgaas]

On Tue, Jun 17, 2025 at 11:10:50AM -0500, Timothy Pearson wrote:
> ----- Original Message -----
> > From: "Bjorn Helgaas" <helgaas@kernel.org>
> > To: "Timothy Pearson" <tpearson@raptorengineering.com>
> > Cc: "linux-pci" <linux-pci@vger.kernel.org>, "mahesh" <mahesh@linux.ibm.com>, "Oliver" <oohall@gmail.com>, "Madhavan
> > Srinivasan" <maddy@linux.ibm.com>, "Michael Ellerman" <mpe@ellerman.id.au>, "Lukas Wunner" <lukas@wunner.de>
> > Sent: Tuesday, June 17, 2025 11:04:28 AM
> > Subject: Re: [PATCH v10] PCI: Add pcie_link_is_active() function
> 
> > On Tue, Jun 17, 2025 at 10:41:58AM -0500, Timothy Pearson wrote:
> >> Add pcie_link_is_active() function to check if the physical PCIe link is
> >> active, replacing duplicate code in multiple locations.

> Will do in the future.  There is some urgency on this overall
> patchset as we have had hotplug support broken for many years now,
> and it's causing continued problems on customer deployed machines.
> While we can continue to point customers at the patchset and have
> them compile their own kernels, patience from our customer side is
> wearing a bit thin for that.  I will continue to try to push this
> forward (along with the associated ppc patch set) at a more
> reasonable pace.

Apparently this got pulled out of a series where the urgency might
have been obvious.  As a standalone patch, AFAICT it's a cleanup that
doesn't actually fix any problems.

Bjorn

Re: [PATCH v10] PCI: Add pcie_link_is_active() function

[Timothy Pearson]

----- Original Message -----
> From: "Bjorn Helgaas" <helgaas@kernel.org>
> To: "Timothy Pearson" <tpearson@raptorengineering.com>
> Cc: "linux-pci" <linux-pci@vger.kernel.org>, "mahesh" <mahesh@linux.ibm.com>, "Oliver" <oohall@gmail.com>, "Madhavan
> Srinivasan" <maddy@linux.ibm.com>, "Michael Ellerman" <mpe@ellerman.id.au>, "Lukas Wunner" <lukas@wunner.de>
> Sent: Tuesday, June 17, 2025 1:59:38 PM
> Subject: Re: [PATCH v10] PCI: Add pcie_link_is_active() function

> On Tue, Jun 17, 2025 at 11:10:50AM -0500, Timothy Pearson wrote:
>> ----- Original Message -----
>> > From: "Bjorn Helgaas" <helgaas@kernel.org>
>> > To: "Timothy Pearson" <tpearson@raptorengineering.com>
>> > Cc: "linux-pci" <linux-pci@vger.kernel.org>, "mahesh" <mahesh@linux.ibm.com>,
>> > "Oliver" <oohall@gmail.com>, "Madhavan
>> > Srinivasan" <maddy@linux.ibm.com>, "Michael Ellerman" <mpe@ellerman.id.au>,
>> > "Lukas Wunner" <lukas@wunner.de>
>> > Sent: Tuesday, June 17, 2025 11:04:28 AM
>> > Subject: Re: [PATCH v10] PCI: Add pcie_link_is_active() function
>> 
>> > On Tue, Jun 17, 2025 at 10:41:58AM -0500, Timothy Pearson wrote:
>> >> Add pcie_link_is_active() function to check if the physical PCIe link is
>> >> active, replacing duplicate code in multiple locations.
> 
>> Will do in the future.  There is some urgency on this overall
>> patchset as we have had hotplug support broken for many years now,
>> and it's causing continued problems on customer deployed machines.
>> While we can continue to point customers at the patchset and have
>> them compile their own kernels, patience from our customer side is
>> wearing a bit thin for that.  I will continue to try to push this
>> forward (along with the associated ppc patch set) at a more
>> reasonable pace.
> 
> Apparently this got pulled out of a series where the urgency might
> have been obvious.  As a standalone patch, AFAICT it's a cleanup that
> doesn't actually fix any problems.
> 
> Bjorn

Correct -- my apologies for not making that clear.  The original patch set for ppc (PowerNV) hotplug fixes was blocked on this pending patch by the ppc maintainers -- logicially it makes some sense, but I can't really submit a series that requires both groups of maintainers to approve.

Re: [PATCH v10] PCI: Add pcie_link_is_active() function

[Bjorn Helgaas]

On Tue, Jun 17, 2025 at 02:19:15PM -0500, Timothy Pearson wrote:
> ----- Original Message -----
> > From: "Bjorn Helgaas" <helgaas@kernel.org>
> > To: "Timothy Pearson" <tpearson@raptorengineering.com>
> > Cc: "linux-pci" <linux-pci@vger.kernel.org>, "mahesh" <mahesh@linux.ibm.com>, "Oliver" <oohall@gmail.com>, "Madhavan
> > Srinivasan" <maddy@linux.ibm.com>, "Michael Ellerman" <mpe@ellerman.id.au>, "Lukas Wunner" <lukas@wunner.de>
> > Sent: Tuesday, June 17, 2025 1:59:38 PM
> > Subject: Re: [PATCH v10] PCI: Add pcie_link_is_active() function
> 
> > On Tue, Jun 17, 2025 at 11:10:50AM -0500, Timothy Pearson wrote:
> >> ----- Original Message -----
> >> > From: "Bjorn Helgaas" <helgaas@kernel.org>
> >> > To: "Timothy Pearson" <tpearson@raptorengineering.com>
> >> > Cc: "linux-pci" <linux-pci@vger.kernel.org>, "mahesh" <mahesh@linux.ibm.com>,
> >> > "Oliver" <oohall@gmail.com>, "Madhavan
> >> > Srinivasan" <maddy@linux.ibm.com>, "Michael Ellerman" <mpe@ellerman.id.au>,
> >> > "Lukas Wunner" <lukas@wunner.de>
> >> > Sent: Tuesday, June 17, 2025 11:04:28 AM
> >> > Subject: Re: [PATCH v10] PCI: Add pcie_link_is_active() function
> >> 
> >> > On Tue, Jun 17, 2025 at 10:41:58AM -0500, Timothy Pearson wrote:
> >> >> Add pcie_link_is_active() function to check if the physical PCIe link is
> >> >> active, replacing duplicate code in multiple locations.
> > 
> >> Will do in the future.  There is some urgency on this overall
> >> patchset as we have had hotplug support broken for many years now,
> >> and it's causing continued problems on customer deployed machines.
> >> While we can continue to point customers at the patchset and have
> >> them compile their own kernels, patience from our customer side is
> >> wearing a bit thin for that.  I will continue to try to push this
> >> forward (along with the associated ppc patch set) at a more
> >> reasonable pace.
> > 
> > Apparently this got pulled out of a series where the urgency might
> > have been obvious.  As a standalone patch, AFAICT it's a cleanup that
> > doesn't actually fix any problems.
> 
> Correct -- my apologies for not making that clear.  The original
> patch set for ppc (PowerNV) hotplug fixes was blocked on this
> pending patch by the ppc maintainers -- logicially it makes some
> sense, but I can't really submit a series that requires both groups
> of maintainers to approve.

Two paths:

  - Best choice: since this is a cleanup that doesn't fix anything,
    pull it out of the ppc series, fix the problem, and add the
    cleanup to your TODO list for resolution in the future.  AFAICT
    this patch, at least v10, doesn't touch anything remotely
    ppc-related, so I don't see the connection with PowerNV.

  - Alternatively: send the patch to both sets of maintainers (ppc and
    pci), and maintainer less affected can ack it and the other can
    appy it.

Re: [PATCH v10] PCI: Add pcie_link_is_active() function

[Lukas Wunner]

[cc += Krishna, Shawn]

On Tue, Jun 17, 2025 at 10:41:58AM -0500, Timothy Pearson wrote:
> Add pcie_link_is_active() function to check if the physical PCIe link is
> active, replacing duplicate code in multiple locations.
> 
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> Signed-off-by: Shawn Anastasio <sanastasio@raptorengineering.com>
> Signed-off-by: Timothy Pearson <tpearson@raptorengineering.com>

I note that there's a Signed-off-by tag for Krishna here,
but Krishna isn't cc'ed.  In future revisions please cc
everyone you're tagging.

Also, if the vast majority of the patch has originally
been authored by Krishna, please give credit to Krishna
by including a From: header at the top of the message body
(as you've done in v9).

Thanks,

Lukas

Re: [PATCH v10] PCI: Add pcie_link_is_active() function

[Krishna Chaitanya Chundru]

On 6/17/2025 9:11 PM, Timothy Pearson wrote:
> Add pcie_link_is_active() function to check if the physical PCIe link is
> active, replacing duplicate code in multiple locations.
> 
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> Signed-off-by: Shawn Anastasio <sanastasio@raptorengineering.com>
> Signed-off-by: Timothy Pearson <tpearson@raptorengineering.com>
> ---
Hi Timothy,

Are you going to respin this patch.
If not I can take this patch in my series which I am going to post next
week.

- Krishna Chaitanya.
>   drivers/pci/hotplug/pciehp.h      |  1 -
>   drivers/pci/hotplug/pciehp_ctrl.c |  2 +-
>   drivers/pci/hotplug/pciehp_hpc.c  | 33 +++----------------------------
>   drivers/pci/pci.c                 | 31 ++++++++++++++++++++++++++---
>   drivers/pci/pci.h                 |  1 +
>   5 files changed, 33 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/pci/hotplug/pciehp.h b/drivers/pci/hotplug/pciehp.h
> index debc79b0adfb..79df49cc9946 100644
> --- a/drivers/pci/hotplug/pciehp.h
> +++ b/drivers/pci/hotplug/pciehp.h
> @@ -186,7 +186,6 @@ int pciehp_query_power_fault(struct controller *ctrl);
>   int pciehp_card_present(struct controller *ctrl);
>   int pciehp_card_present_or_link_active(struct controller *ctrl);
>   int pciehp_check_link_status(struct controller *ctrl);
> -int pciehp_check_link_active(struct controller *ctrl);
>   bool pciehp_device_replaced(struct controller *ctrl);
>   void pciehp_release_ctrl(struct controller *ctrl);
>   
> diff --git a/drivers/pci/hotplug/pciehp_ctrl.c b/drivers/pci/hotplug/pciehp_ctrl.c
> index bcc938d4420f..6cc1b27b3b11 100644
> --- a/drivers/pci/hotplug/pciehp_ctrl.c
> +++ b/drivers/pci/hotplug/pciehp_ctrl.c
> @@ -260,7 +260,7 @@ void pciehp_handle_presence_or_link_change(struct controller *ctrl, u32 events)
>   	/* Turn the slot on if it's occupied or link is up */
>   	mutex_lock(&ctrl->state_lock);
>   	present = pciehp_card_present(ctrl);
> -	link_active = pciehp_check_link_active(ctrl);
> +	link_active = pcie_link_is_active(ctrl->pcie->port);
>   	if (present <= 0 && link_active <= 0) {
>   		if (ctrl->state == BLINKINGON_STATE) {
>   			ctrl->state = OFF_STATE;
> diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
> index ebd342bda235..d29ce3715a44 100644
> --- a/drivers/pci/hotplug/pciehp_hpc.c
> +++ b/drivers/pci/hotplug/pciehp_hpc.c
> @@ -221,33 +221,6 @@ static void pcie_write_cmd_nowait(struct controller *ctrl, u16 cmd, u16 mask)
>   	pcie_do_write_cmd(ctrl, cmd, mask, false);
>   }
>   
> -/**
> - * pciehp_check_link_active() - Is the link active
> - * @ctrl: PCIe hotplug controller
> - *
> - * Check whether the downstream link is currently active. Note it is
> - * possible that the card is removed immediately after this so the
> - * caller may need to take it into account.
> - *
> - * If the hotplug controller itself is not available anymore returns
> - * %-ENODEV.
> - */
> -int pciehp_check_link_active(struct controller *ctrl)
> -{
> -	struct pci_dev *pdev = ctrl_dev(ctrl);
> -	u16 lnk_status;
> -	int ret;
> -
> -	ret = pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &lnk_status);
> -	if (ret == PCIBIOS_DEVICE_NOT_FOUND || PCI_POSSIBLE_ERROR(lnk_status))
> -		return -ENODEV;
> -
> -	ret = !!(lnk_status & PCI_EXP_LNKSTA_DLLLA);
> -	ctrl_dbg(ctrl, "%s: lnk_status = %x\n", __func__, lnk_status);
> -
> -	return ret;
> -}
> -
>   static bool pci_bus_check_dev(struct pci_bus *bus, int devfn)
>   {
>   	u32 l;
> @@ -467,7 +440,7 @@ int pciehp_card_present_or_link_active(struct controller *ctrl)
>   	if (ret)
>   		return ret;
>   
> -	return pciehp_check_link_active(ctrl);
> +	return pcie_link_is_active(ctrl_dev(ctrl));
>   }
>   
>   int pciehp_query_power_fault(struct controller *ctrl)
> @@ -614,7 +587,7 @@ static void pciehp_ignore_link_change(struct controller *ctrl,
>   	 * Synthesize it to ensure that it is acted on.
>   	 */
>   	down_read_nested(&ctrl->reset_lock, ctrl->depth);
> -	if (!pciehp_check_link_active(ctrl) || pciehp_device_replaced(ctrl))
> +	if (!pcie_link_is_active(ctrl_dev(ctrl)) || pciehp_device_replaced(ctrl))
>   		pciehp_request(ctrl, ignored_events);
>   	up_read(&ctrl->reset_lock);
>   }
> @@ -921,7 +894,7 @@ int pciehp_slot_reset(struct pcie_device *dev)
>   	pcie_capability_write_word(dev->port, PCI_EXP_SLTSTA,
>   				   PCI_EXP_SLTSTA_DLLSC);
>   
> -	if (!pciehp_check_link_active(ctrl))
> +	if (!pcie_link_is_active(ctrl_dev(ctrl)))
>   		pciehp_request(ctrl, PCI_EXP_SLTSTA_DLLSC);
>   
>   	return 0;
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index e9448d55113b..4e96ff8ee5ec 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -4908,7 +4908,6 @@ int pci_bridge_wait_for_secondary_bus(struct pci_dev *dev, char *reset_type)
>   		return 0;
>   
>   	if (pcie_get_speed_cap(dev) <= PCIE_SPEED_5_0GT) {
> -		u16 status;
>   
>   		pci_dbg(dev, "waiting %d ms for downstream link\n", delay);
>   		msleep(delay);
> @@ -4924,8 +4923,7 @@ int pci_bridge_wait_for_secondary_bus(struct pci_dev *dev, char *reset_type)
>   		if (!dev->link_active_reporting)
>   			return -ENOTTY;
>   
> -		pcie_capability_read_word(dev, PCI_EXP_LNKSTA, &status);
> -		if (!(status & PCI_EXP_LNKSTA_DLLLA))
> +		if (pcie_link_is_active(dev) <= 0)
>   			return -ENOTTY;
>   
>   		return pci_dev_wait(child, reset_type,
> @@ -6230,6 +6228,33 @@ void pcie_print_link_status(struct pci_dev *dev)
>   }
>   EXPORT_SYMBOL(pcie_print_link_status);
>   
> +/**
> + * pcie_link_is_active() - Checks if the link is active or not
> + * @pdev: PCI device to query
> + *
> + * Check whether the physical link is active or not. Note it is
> + * possible that the card is removed immediately after this so the
> + * caller may need to take it into account.
> + *
> + * If the PCI device itself is not available anymore returns
> + * %-ENODEV.
> + *
> + * Return: link state, or -ENODEV if the config read failes.
> + */
> +int pcie_link_is_active(struct pci_dev *pdev)
> +{
> +	u16 lnk_status;
> +	int ret;
> +
> +	ret = pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &lnk_status);
> +	if (ret == PCIBIOS_DEVICE_NOT_FOUND || PCI_POSSIBLE_ERROR(lnk_status))
> +		return -ENODEV;
> +
> +	pci_dbg(pdev, "lnk_status = %#06x\n", lnk_status);
> +	return !!(lnk_status & PCI_EXP_LNKSTA_DLLLA);
> +}
> +EXPORT_SYMBOL(pcie_link_is_active);
> +
>   /**
>    * pci_select_bars - Make BAR mask from the type of resource
>    * @dev: the PCI device for which BAR mask is made
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index 12215ee72afb..cf1afb718f8a 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -231,6 +231,7 @@ static inline int pci_proc_detach_bus(struct pci_bus *bus) { return 0; }
>   /* Functions for PCI Hotplug drivers to use */
>   int pci_hp_add_bridge(struct pci_dev *dev);
>   bool pci_hp_spurious_link_change(struct pci_dev *pdev);
> +int pcie_link_is_active(struct pci_dev *dev);
>   
>   #if defined(CONFIG_SYSFS) && defined(HAVE_PCI_LEGACY)
>   void pci_create_legacy_files(struct pci_bus *bus);

Re: [PATCH v10] PCI: Add pcie_link_is_active() function

[Timothy Pearson]

----- Original Message -----
> From: "Krishna Chaitanya Chundru" <krishna.chundru@oss.qualcomm.com>
> To: "Timothy Pearson" <tpearson@raptorengineering.com>, "Bjorn Helgaas" <helgaas@kernel.org>
> Cc: "linux-pci" <linux-pci@vger.kernel.org>, "mahesh" <mahesh@linux.ibm.com>, "Oliver" <oohall@gmail.com>, "Madhavan
> Srinivasan" <maddy@linux.ibm.com>, "Michael Ellerman" <mpe@ellerman.id.au>, "Lukas Wunner" <lukas@wunner.de>
> Sent: Tuesday, August 26, 2025 2:04:56 AM
> Subject: Re: [PATCH v10] PCI: Add pcie_link_is_active() function

> On 6/17/2025 9:11 PM, Timothy Pearson wrote:
>> Add pcie_link_is_active() function to check if the physical PCIe link is
>> active, replacing duplicate code in multiple locations.
>> 
>> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
>> Signed-off-by: Shawn Anastasio <sanastasio@raptorengineering.com>
>> Signed-off-by: Timothy Pearson <tpearson@raptorengineering.com>
>> ---
> Hi Timothy,
> 
> Are you going to respin this patch.
> If not I can take this patch in my series which I am going to post next
> week.
> 
> - Krishna Chaitanya.

If you'd like to take it please go ahead.  Thanks!