Re: [PATCH v6 0/2] PCI: Add Lane Margining at the Receiver debugfs interface

[Bjorn Helgaas <helgaas@kernel.org>]

On Tue, Oct 06, 2026 at 08:59:09PM +0000, Priyank Rathod wrote:
> Lane Margining at the Receiver lets software move the sampling point of
> a receiver in time or in voltage while the link stays up, and read back
> the errors the receiver sees, to find out how much margin a link running
> at 16.0 GT/s or faster has.  pcilmr in pciutils does this today by
> writing the capability registers directly from user space.
> 
> This series adds a debugfs interface for it.  The kernel sets the link
> up for margining and puts it back afterwards:
> 
>   - ASPM is turned off for the session with the existing ASPM API,
>     Hardware Autonomous Width/Speed Disable are set on both ends, and
>     both ends are kept runtime resumed.  All of this is undone when
>     the session ends or either end of the link is removed.
>   - Before each margining command, the link is checked against the
>     state the session started in.
>   - User space selects the receiver and the steps; the kernel does not
>     run sweeps or interpret results.
> 
> Patch 1 adds the interface and its documentation, patch 2 a kselftest.
> 
> The only PCI core changes are the init/exit hooks in probe.c and
> remove.c and their prototypes in drivers/pci/pci.h.  aspm.c, pci.c and
> include/linux/pci.h are not changed.
> 
> Signed-off-by: Priyank Rathod <rathodpriyank@google.com>
> ---
> Changes in v6:
> - Add a kref on struct pci_lmr_port and use pci_lmr_end_sessions() in
>   pci_lmr_reboot_notify() so reboot/kexec waits for in-flight commands
>   outside pci_lmr_mutex (preserving the port->lock -> pci_lmr_mutex lock
>   order) instead of skipping ports on mutex_trylock() failure.
> - Link to v5: https://lore.kernel.org/r/20261006-pcie-link-endpoints-v5-0-c1e73235746b@google.com

These reposts (v4, v5, and v6 within four hours) are a little bit too
fast.  The pace suggests this isn't fully baked yet, and it's too much
for reviewers to assimilate.

> Changes in v5:
> - Clarify in pcie_lmr.sh why the sysfs 'link/' check targets receiver 6
>   (and rename 'up' to 'child'): in PCIe terminology the Upstream Port
>   (receiver 6) is the child device below the Downstream Port, and in
>   drivers/pci/pcie/aspm.c aspm_ctrl_attrs_are_visible() looks up the
>   link via pcie_aspm_get_link(pdev) -> pci_upstream_bridge(pdev)->link_state,
>   so '/sys/bus/pci/devices/<dev>/link/' is attached to the child device
>   below the Downstream Port, not to the Downstream Port itself.
> - Link to v4: https://lore.kernel.org/r/20261006-pcie-link-endpoints-v4-0-ad5398c4260c@google.com
> 
> Changes in v4:
> - Drop the exported pcie_get_link_endpoints(); the partner lookup is
>   private to margin.c (Ilpo).
> - Drop pci_aspm_inhibit() and all aspm.c changes.  Use
>   pcie_aspm_enabled(), pci_disable_link_state() and
>   pci_force_enable_link_state(), call neither when no ASPM state is
>   enabled, and restore Clock PM when CLKREQ# was enabled.  ASPM Control
>   is checked on both ends before each command.  The remaining
>   differences after a session are documented.
> - Remove the MSampleMultipleReceivers capability bit.
> - One receiver per session, selected with a port-level 'receiver'
>   file.  A session starts with receiver 1 on a Downstream Port and 6
>   below it, as pcilmr does; receiver 0 is not used for margining.
> - Add a per-lane 'status' file with the step response and error count,
>   and a 'port' file with the Margining Port bits.  Stop writing
>   Margining Port Status.
> - Locking: take both device locks with pci_dev_trylock() only while a
>   session starts or stops, require both ends to be added, and make no
>   runtime PM calls under the LMR locks.
> - End sessions from pci_lmr_exit() for either end of the link.
> - Commands fail with -EIO after a system suspend.
> - Move debugfs to pcie_lmr_<device> at the debugfs root.
> - Keep per-device state in margin.c instead of struct pci_dev.
> - Split the selftest into its own patch, rename it pcie_lmr, use KTAP,
>   and only start a session on the port named by PCIE_LMR_DEV.
> - Refer to registers by name instead of specification section numbers.
> ...