[PATCH] PCI: Add pcie_get_link_endpoints() helper

[PATCH] PCI: Add pcie_get_link_endpoints() helper

[Priyank Rathod]

In PCIe topologies, physical links are point-to-point connections
between an Upstream Component (Downstream Port, such as a Root Port
or Switch Downstream Port) and a Downstream Component (Upstream Port,
such as an Endpoint or Switch Upstream Port), per PCIe Base Specification
r7.0/r6.0 sec 1.3.1.

Currently, various drivers across drivers/pci/ independently infer the
two ends of a PCIe link using varied ad-hoc methods:
  - ASPM (drivers/pci/pcie/aspm.c): pcie_aspm_get_link(),
    alloc_pcie_link_state(), and pci_configure_ltr() resolve parent
    bridges and subordinate function 0.
  - Precision Time Measurement (drivers/pci/pcie/ptm.c):
    pci_upstream_ptm() traverses pci_upstream_bridge() to validate
    upstream link partner capabilities.
  - PCIe Link Retrain (drivers/pci/pci.c): pcie_retrain_link() and
    pcie_update_link_speed() coordinate link retraining from the
    Downstream Port across the subordinate bus.
  - AER/DPC Recovery (drivers/pci/pcie/err.c): pcie_do_recovery()
    identifies the parent bridge via pci_upstream_bridge() to trigger
    secondary bus reset and broadcast driver recovery callbacks.
  - Lane Margining at Receiver (LMR): margin_enable_write() requires
    both link endpoints to establish hierarchical locking and runtime PM
    pinning.

Ad-hoc subordinate bus iteration risks races with concurrent device
removal or hotplug if pci_bus_sem is omitted.

Introduce pcie_get_link_endpoints() and pcie_put_link_endpoints() in the
PCI core to provide a standardized, symmetric, and race-safe helper:
  - For Endpoints: resolves the parent Downstream Port via
    pci_upstream_bridge(pdev).
  - For Downstream Ports: safely inspects the subordinate bus under
    down_read(&pci_bus_sem) and acquires a reference via pci_dev_get()
    on the child device.

Callers release the acquired reference using pcie_put_link_endpoints().

Signed-off-by: Priyank Rathod <rathodpriyank@google.com>
---
Hi Bjorn, Ilpo, and PCI maintainers,

During the review of the PCIe Lane Margining at Receiver (LMR) patch series
(v7: https://lore.kernel.org/linux-pci/20260828-pcie-lmt-v7-1-6012e9e0940a@google.com/),
Ilpo Järvinen pointed out that feature drivers (such as LMR) resolving link
partners duplicate link traversal logic that is already present in drivers such
as ASPM:

  "This feels like duplicating similar functionality with the aspm driver
   that also wants to infer ends of the link when giving a pci_dev in.
   The aspm driver currently does that within, but it kind of duplicating
   pci_bus. It would be nice to avoid the duplication and have something
   similar for this in PCI core."

In PCIe topologies, physical links are point-to-point interconnects
between an Upstream Component (Downstream Port, such as a Root Port or Switch
Downstream Port) and a Downstream Component (Upstream Port, such as an Endpoint
or Switch Upstream Port), per PCIe Base Specification Revision 7.0 / 6.0
Section 1.3.1.

Several subsystems across drivers/pci/ independently infer and coordinate both
ends of a PCIe link:
  1. ASPM (drivers/pci/pcie/aspm.c): pcie_aspm_get_link(),
     alloc_pcie_link_state(), and pci_configure_ltr() resolve parent bridges
     and subordinate function 0 to configure ASPMC and L1SS.
  2. Precision Time Measurement (drivers/pci/pcie/ptm.c): pci_upstream_ptm()
     traverses pci_upstream_bridge() to validate upstream link partner
     capabilities.
  3. PCIe Link Retrain (drivers/pci/pci.c): pcie_retrain_link() and
     pcie_update_link_speed() coordinate link retraining from the Downstream
     Port across the subordinate bus.
  4. AER & DPC Recovery (drivers/pci/pcie/err.c): pcie_do_recovery() identifies
     the parent bridge via pci_upstream_bridge() to trigger secondary bus
     resets and broadcast driver error callbacks.
  5. Lane Margining at Receiver (LMR): margin_enable_write() requires both
     link endpoints to establish hierarchical locking (pci_dev_lock) and
     runtime PM pinning.

Currently, these drivers independently implement ad-hoc traversals via
pci_upstream_bridge() or subordinate bus device iteration. Ad-hoc subordinate
bus iteration is error-prone and risks races with concurrent hot-unplug or
device removal if pci_bus_sem is omitted.

This patch introduces pcie_get_link_endpoints() and pcie_put_link_endpoints()
in the PCI core (drivers/pci/pci.c and include/linux/pci.h) as a standalone
helper:
  - For Endpoints: resolves the parent Downstream Port via
    pci_upstream_bridge(pdev).
  - For Downstream Ports: safely inspects the subordinate bus under
    down_read(&pci_bus_sem) and acquires a reference via pci_dev_get() on the
    child device.

Callers release the acquired reference with pcie_put_link_endpoints().

Validation:
  - Compiled clean on x86_64 defconfig (0 warnings, 0 errors).
  - Multi-architecture build validated for ARM64 and x86_64 targets.
  - Passed checkpatch.pl (0 warnings, 0 errors).
  - Passed Sashiko pre-commit test runner (ID: 46b5f8a3da1ca0056407b3db6c82b001264f86b7).
---
 drivers/pci/pci.c   | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/pci.h |  6 ++++++
 2 files changed, 56 insertions(+)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 77b17b13ee61..7bdfe7e5ab3a 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -4618,6 +4618,56 @@ int pcie_retrain_link(struct pci_dev *pdev, bool use_lt)
 	return rc;
 }
 
+/**
+ * pcie_get_link_endpoints - Identify Upstream and Downstream ends of a PCIe link
+ * @pdev: Any PCIe device on the link (Downstream Port or Endpoint)
+ * @downstream_port: Output pointer to Downstream Port (Upstream Component)
+ * @upstream_port: Output pointer to Upstream Port (Downstream Component)
+ *
+ * Identifies both ends of a point-to-point PCIe link. Increments reference count
+ * on @upstream_port if dynamically discovered on a downstream port. Callers must
+ * release with pcie_put_link_endpoints().
+ *
+ * Return: 0 on success, or -EINVAL if @pdev is NULL or not PCIe.
+ */
+int pcie_get_link_endpoints(struct pci_dev *pdev,
+			    struct pci_dev **downstream_port,
+			    struct pci_dev **upstream_port)
+{
+	if (!pdev || !pci_is_pcie(pdev))
+		return -EINVAL;
+
+	if (pcie_downstream_port(pdev)) {
+		*downstream_port = pdev;
+		down_read(&pci_bus_sem);
+		*upstream_port = pdev->subordinate ?
+			pci_dev_get(list_first_entry_or_null(&pdev->subordinate->devices,
+							     struct pci_dev, bus_list)) : NULL;
+		up_read(&pci_bus_sem);
+	} else {
+		*downstream_port = pci_upstream_bridge(pdev);
+		*upstream_port = pdev;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(pcie_get_link_endpoints);
+
+/**
+ * pcie_put_link_endpoints - Release references acquired by pcie_get_link_endpoints
+ * @pdev: Device passed to pcie_get_link_endpoints()
+ * @downstream_port: Downstream Port pointer
+ * @upstream_port: Upstream Port pointer
+ */
+void pcie_put_link_endpoints(struct pci_dev *pdev,
+			     struct pci_dev *downstream_port,
+			     struct pci_dev *upstream_port)
+{
+	if (pdev && pcie_downstream_port(pdev) && upstream_port)
+		pci_dev_put(upstream_port);
+}
+EXPORT_SYMBOL_GPL(pcie_put_link_endpoints);
+
 /**
  * pcie_wait_for_link_delay - Wait until link is active or inactive
  * @pdev: Bridge device
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 64b308b6e61c..d30bf056a7ce 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1264,6 +1264,12 @@ struct resource *pci_find_parent_resource(const struct pci_dev *dev,
 u8 pci_swizzle_interrupt_pin(const struct pci_dev *dev, u8 pin);
 int pci_get_interrupt_pin(struct pci_dev *dev, struct pci_dev **bridge);
 u8 pci_common_swizzle(struct pci_dev *dev, u8 *pinp);
+int pcie_get_link_endpoints(struct pci_dev *pdev,
+			    struct pci_dev **downstream_port,
+			    struct pci_dev **upstream_port);
+void pcie_put_link_endpoints(struct pci_dev *pdev,
+			     struct pci_dev *downstream_port,
+			     struct pci_dev *upstream_port);
 struct pci_dev *pci_dev_get(struct pci_dev *dev);
 void pci_dev_put(struct pci_dev *dev);
 DEFINE_FREE(pci_dev_put, struct pci_dev *, if (_T) pci_dev_put(_T))

---
base-commit: 0f23d56f17fdfc7db69d51f64c8b91bbab947aa9
change-id: 20260831-pcie-link-endpoints-978e100d5d06

Best regards,
-- 
Priyank Rathod <rathodpriyank@google.com>

Re: [PATCH] PCI: Add pcie_get_link_endpoints() helper

[Ilpo Järvinen]

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

On Mon, 31 Aug 2026, Priyank Rathod wrote:

> In PCIe topologies, physical links are point-to-point connections
> between an Upstream Component (Downstream Port, such as a Root Port
> or Switch Downstream Port) and a Downstream Component (Upstream Port,
> such as an Endpoint or Switch Upstream Port), per PCIe Base Specification
> r7.0/r6.0 sec 1.3.1.
> 
> Currently, various drivers across drivers/pci/ independently infer the
> two ends of a PCIe link using varied ad-hoc methods:
>   - ASPM (drivers/pci/pcie/aspm.c): pcie_aspm_get_link(),
>     alloc_pcie_link_state(), and pci_configure_ltr() resolve parent
>     bridges and subordinate function 0.

This is not the full picture. The aspm driver has to write the same config 
to all functions (except L1SS that is only in func 0).

>   - Precision Time Measurement (drivers/pci/pcie/ptm.c):
>     pci_upstream_ptm() traverses pci_upstream_bridge() to validate
>     upstream link partner capabilities.

This looks something entirely different, and it's pure get (a certain) 
device upstream, not the get-my-link-partner query.

>   - PCIe Link Retrain (drivers/pci/pci.c): pcie_retrain_link() and
>     pcie_update_link_speed() coordinate link retraining from the
>     Downstream Port across the subordinate bus.

I don't entirely agree with this assessment. pcie_retrain_link() is given 
a downstream port, mere looking into ->subordinate doesn't constitute as a 
need for having access to the struct of the other end of the link.

>   - AER/DPC Recovery (drivers/pci/pcie/err.c): pcie_do_recovery()
>     identifies the parent bridge via pci_upstream_bridge() to trigger
>     secondary bus reset and broadcast driver recovery callbacks.

This seems to have some validity, though it must cover RCiEPs.

>   - Lane Margining at Receiver (LMR): margin_enable_write() requires
>     both link endpoints to establish hierarchical locking and runtime PM
>     pinning.
> 
> Ad-hoc subordinate bus iteration risks races with concurrent device
> removal or hotplug if pci_bus_sem is omitted.
> 
> Introduce pcie_get_link_endpoints() and pcie_put_link_endpoints() in the
> PCI core to provide a standardized, symmetric, and race-safe helper:
>   - For Endpoints: resolves the parent Downstream Port via
>     pci_upstream_bridge(pdev).
>   - For Downstream Ports: safely inspects the subordinate bus under
>     down_read(&pci_bus_sem) and acquires a reference via pci_dev_get()
>     on the child device.
> 
> Callers release the acquired reference using pcie_put_link_endpoints().
> 
> Signed-off-by: Priyank Rathod <rathodpriyank@google.com>
> ---
> Hi Bjorn, Ilpo, and PCI maintainers,
> 
> During the review of the PCIe Lane Margining at Receiver (LMR) patch series
> (v7: https://lore.kernel.org/linux-pci/20260828-pcie-lmt-v7-1-6012e9e0940a@google.com/),
> Ilpo Järvinen pointed out that feature drivers (such as LMR) resolving link
> partners duplicate link traversal logic that is already present in drivers such
> as ASPM:
> 
>   "This feels like duplicating similar functionality with the aspm driver
>    that also wants to infer ends of the link when giving a pci_dev in.
>    The aspm driver currently does that within, but it kind of duplicating
>    pci_bus. It would be nice to avoid the duplication and have something
>    similar for this in PCI core."
> 
> In PCIe topologies, physical links are point-to-point interconnects
> between an Upstream Component (Downstream Port, such as a Root Port or Switch
> Downstream Port) and a Downstream Component (Upstream Port, such as an Endpoint
> or Switch Upstream Port), per PCIe Base Specification Revision 7.0 / 6.0
> Section 1.3.1.
> 
> Several subsystems across drivers/pci/ independently infer and coordinate both
> ends of a PCIe link:
>   1. ASPM (drivers/pci/pcie/aspm.c): pcie_aspm_get_link(),
>      alloc_pcie_link_state(), and pci_configure_ltr() resolve parent bridges
>      and subordinate function 0 to configure ASPMC and L1SS.
>   2. Precision Time Measurement (drivers/pci/pcie/ptm.c): pci_upstream_ptm()
>      traverses pci_upstream_bridge() to validate upstream link partner
>      capabilities.
>   3. PCIe Link Retrain (drivers/pci/pci.c): pcie_retrain_link() and
>      pcie_update_link_speed() coordinate link retraining from the Downstream
>      Port across the subordinate bus.
>   4. AER & DPC Recovery (drivers/pci/pcie/err.c): pcie_do_recovery() identifies
>      the parent bridge via pci_upstream_bridge() to trigger secondary bus
>      resets and broadcast driver error callbacks.
>   5. Lane Margining at Receiver (LMR): margin_enable_write() requires both
>      link endpoints to establish hierarchical locking (pci_dev_lock) and
>      runtime PM pinning.
> 
> Currently, these drivers independently implement ad-hoc traversals via
> pci_upstream_bridge() or subordinate bus device iteration. Ad-hoc subordinate
> bus iteration is error-prone and risks races with concurrent hot-unplug or
> device removal if pci_bus_sem is omitted.
> 
> This patch introduces pcie_get_link_endpoints() and pcie_put_link_endpoints()
> in the PCI core (drivers/pci/pci.c and include/linux/pci.h) as a standalone
> helper:
>   - For Endpoints: resolves the parent Downstream Port via
>     pci_upstream_bridge(pdev).
>   - For Downstream Ports: safely inspects the subordinate bus under
>     down_read(&pci_bus_sem) and acquires a reference via pci_dev_get() on the
>     child device.
> 
> Callers release the acquired reference with pcie_put_link_endpoints().
> 
> Validation:
>   - Compiled clean on x86_64 defconfig (0 warnings, 0 errors).
>   - Multi-architecture build validated for ARM64 and x86_64 targets.
>   - Passed checkpatch.pl (0 warnings, 0 errors).
>   - Passed Sashiko pre-commit test runner (ID: 46b5f8a3da1ca0056407b3db6c82b001264f86b7).
> ---
>  drivers/pci/pci.c   | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/pci.h |  6 ++++++
>  2 files changed, 56 insertions(+)
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 77b17b13ee61..7bdfe7e5ab3a 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -4618,6 +4618,56 @@ int pcie_retrain_link(struct pci_dev *pdev, bool use_lt)
>  	return rc;
>  }
>  
> +/**
> + * pcie_get_link_endpoints - Identify Upstream and Downstream ends of a PCIe link
> + * @pdev: Any PCIe device on the link (Downstream Port or Endpoint)
> + * @downstream_port: Output pointer to Downstream Port (Upstream Component)
> + * @upstream_port: Output pointer to Upstream Port (Downstream Component)
> + *
> + * Identifies both ends of a point-to-point PCIe link. Increments reference count
> + * on @upstream_port if dynamically discovered on a downstream port. Callers must
> + * release with pcie_put_link_endpoints().
> + *
> + * Return: 0 on success, or -EINVAL if @pdev is NULL or not PCIe.
> + */
> +int pcie_get_link_endpoints(struct pci_dev *pdev,
> +			    struct pci_dev **downstream_port,
> +			    struct pci_dev **upstream_port)
> +{
> +	if (!pdev || !pci_is_pcie(pdev))
> +		return -EINVAL;
> +
> +	if (pcie_downstream_port(pdev)) {
> +		*downstream_port = pdev;
> +		down_read(&pci_bus_sem);
> +		*upstream_port = pdev->subordinate ?
> +			pci_dev_get(list_first_entry_or_null(&pdev->subordinate->devices,
> +							     struct pci_dev, bus_list)) : NULL;

Too much is crammed into one statement.

> +		up_read(&pci_bus_sem);
> +	} else {
> +		*downstream_port = pci_upstream_bridge(pdev);
> +		*upstream_port = pdev;

Why complicate things with the unbalance?

> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(pcie_get_link_endpoints);
> +
> +/**
> + * pcie_put_link_endpoints - Release references acquired by pcie_get_link_endpoints
> + * @pdev: Device passed to pcie_get_link_endpoints()
> + * @downstream_port: Downstream Port pointer
> + * @upstream_port: Upstream Port pointer
> + */
> +void pcie_put_link_endpoints(struct pci_dev *pdev,
> +			     struct pci_dev *downstream_port,
> +			     struct pci_dev *upstream_port)
> +{
> +	if (pdev && pcie_downstream_port(pdev) && upstream_port)
> +		pci_dev_put(upstream_port);
> +}
> +EXPORT_SYMBOL_GPL(pcie_put_link_endpoints);

--
 i.

Re: [PATCH] PCI: Add pcie_get_link_endpoints() helper

[Priyank Rathod]

On Tue, 1 Sep 2026, Ilpo Järvinen wrote:
> On Mon, 31 Aug 2026, Priyank Rathod wrote:
>
> >   - ASPM (drivers/pci/pcie/aspm.c): pcie_aspm_get_link(),
> >     alloc_pcie_link_state(), and pci_configure_ltr() resolve parent
> >     bridges and subordinate function 0.
>
> This is not the full picture. The aspm driver has to write the same config
> to all functions (except L1SS that is only in func 0).

Hi Ilpo,

Sorry for the slow reply, and for posting LMR v3 before answering this.

You're right. The helper only returns Function 0, which isn't what the
ASPM driver needs, so I shouldn't have listed it as a user.

> >   - Precision Time Measurement (drivers/pci/pcie/ptm.c):
> >     pci_upstream_ptm() traverses pci_upstream_bridge() to validate
> >     upstream link partner capabilities.
>
> This looks something entirely different, and it's pure get (a certain)
> device upstream, not the get-my-link-partner query.

Agreed, PTM comes off the list.

> >   - PCIe Link Retrain (drivers/pci/pci.c): pcie_retrain_link() and
> >     pcie_update_link_speed() coordinate link retraining from the
> >     Downstream Port across the subordinate bus.
>
> I don't entirely agree with this assessment. pcie_retrain_link() is given
> a downstream port, mere looking into ->subordinate doesn't constitute as a
> need for having access to the struct of the other end of the link.

Agreed, that one goes too.

> >   - AER/DPC Recovery (drivers/pci/pcie/err.c): pcie_do_recovery()
> >     identifies the parent bridge via pci_upstream_bridge() to trigger
> >     secondary bus reset and broadcast driver recovery callbacks.
>
> This seems to have some validity, though it must cover RCiEPs.

The helper doesn't handle RCiEPs, and I don't have a patch converting
err.c, so I'll drop it as well.

That leaves LMR as the only user, so for v4 I dropped the exported
helper and kept the lookup private to margin.c. LMR only needs
Function 0 on the Upstream Port side (pcilmr makes the same
assumption).

On v7 you said you'd like to avoid duplicating what the ASPM driver
does. If you'd rather have a shared helper in the PCI core, I can do
that as a separate series that also converts aspm.c, so it has a
second user from the start. Let me know which you prefer.

> > +		*upstream_port = pdev->subordinate ?
> > +			pci_dev_get(list_first_entry_or_null(&pdev->subordinate->devices,
> > +							     struct pci_dev, bus_list)) : NULL;
>
> Too much is crammed into one statement.
>
> > +	} else {
> > +		*downstream_port = pci_upstream_bridge(pdev);
> > +		*upstream_port = pdev;
>
> Why complicate things with the unbalance?

Both were fixed in the copy in LMR v3 (patch 1/3), which takes a
reference on both ends. This code goes away in v4 anyway.

On ASPM, since you asked about it on v6 and v7: v4 drops
pci_aspm_inhibit() and doesn't change aspm.c. It uses the existing
API the way ath10k, ath11k and ath12k do: save with
pcie_aspm_enabled(), call pci_disable_link_state() before margining
and pci_force_enable_link_state() afterwards. Reading aspm.c, that
round trip doesn't restore everything, so v4 adds a few things in
margin.c:

 - If pcie_aspm_enabled() returns 0 (firmware left ASPM off,
   pcie_aspm=off, or the "performance" policy), v4 calls neither
   function.
 - It disables only the states pcie_aspm_enabled() reported, not
   PCIE_LINK_STATE_ASPM_ALL, so the restore clears the same
   aspm_disable bits (except the L1 substates, see below).
 - __pci_enable_link_state() sets clkpm_default from
   PCIE_LINK_STATE_CLKPM, which pcie_aspm_enabled() never reports, so
   with the default policy the restore would turn Clock PM off. v4
   adds PCIE_LINK_STATE_CLKPM to the restore mask if CLKREQ_EN was
   set at the start. The ath drivers hit this too.
 - It reads ASPM Control in Link Control on both ends (read-only) at
   the start and before each margining command, and fails with -EBUSY
   if it's set. That covers firmware-enabled ASPM with pcie_aspm=off,
   and ASPM coming back on mid-session through the link/*_aspm sysfs
   files or another driver calling pci_force_enable_link_state().

The rest is documented in pcie-lmr.rst:

 - If another driver disables a state that LMR also disabled, the
   restore turns it back on.
 - aspm_default ends up as the saved set.
 - Disabling L1 also disables the L1 substates, so substates that
   were off at the start stay disabled.
 - Without OS control of ASPM, the disable works but the restore
   returns -EPERM, so ASPM stays off until reboot.

Counting disables per state in aspm.c, as you suggested on v7, would
fix most of these, but v4 doesn't depend on it.

v3 also brought back the MSampleMultipleReceivers bit that you couldn't
find in r7.0 on v5. That was my mistake, and v4 removes it. Some
section numbers in v3 were wrong too (Link Control 2, for one), so v4
refers to registers and fields by name instead of section numbers, and
keeps comments within 80 columns.

Thanks,
Priyank