[PATCH iwl-net v2 0/2] igc: fix L1.2 disable on resume, add I226-LMvP to I226 helper

[PATCH iwl-net v2 0/2] igc: fix L1.2 disable on resume, add I226-LMvP to I226 helper

[Daniel Viaño]

This series replaces "igc: Disable PCIe L1.2 substate on I225-LMvP and
I226-LMvP devices" [1], split up as suggested by Dima Ruinskiy.

Patch 1 removes pci_disable_link_state() from __igc_resume(). That
function is also the runtime resume callback and can be reached with
pci_bus_sem already held for read (pcie_do_recovery() runtime-resumes
the affected devices from within pci_walk_bus()), so taking the lock
again risks a deadlock. The L1.2 disable done in probe persists in the
link's aspm_disable mask, so the call is not needed on resume.

Patch 2 adds the missing IGC_DEV_ID_I226_LMVP to igc_is_device_id_i226().
Besides the L1.2 disable, this also moves I226-LMvP onto the I226 TSN
code paths; Dima confirmed that this is correct for 0x5503 [2]. Patch 1
comes first so that I226-LMvP never picks up the problematic
resume-path call.

The I225-LMvP part of v1 is dropped. On the setup where I saw the
errors (HP Thunderbolt Dock G4), the I225-LMvP does not advertise L1.2
and the upstream port has no L1 PM Substates capability, so L1.2 is not
the cause; I will investigate that separately. As agreed with Dima
[2], disabling L1.2 unconditionally is not part of this series.

Testing: compile-tested only (igc builds cleanly with W=1). I have no
I226 hardware, so neither change has been runtime-tested on the
affected devices; testing on I226 / I226-LMvP would be appreciated.

The resume-path issue was reported by the Sashiko AI review of v1.
I used an AI coding assistant (Claude) to help with the locking
analysis and to draft the patches; I have reviewed them, compile-tested
them and take responsibility for them.

Changes in v2:
- Split v1 into separate patches, as suggested by Dima.
- Dropped the I225-LMvP L1.2 disable: L1.2 is not supported on that
  link, so it cannot explain the dock errors.
- New patch 1: stop calling pci_disable_link_state() from
  __igc_resume() (reported by Sashiko).
- Corrected the Fixes: tags.
- Changed the subject prefix to iwl-net.

[1] https://lore.kernel.org/all/20260925112850.189302-1-danividanivi@gmail.com/
[2] https://lore.kernel.org/all/5d45a583-198c-467d-afbe-8f19398e82f3@intel.com/

Daniel Viaño (2):
  igc: don't disable ASPM L1.2 from __igc_resume()
  igc: treat I226-LMvP as an I226 device

 drivers/net/ethernet/intel/igc/igc_base.c | 1 +
 drivers/net/ethernet/intel/igc/igc_main.c | 3 ---
 2 files changed, 1 insertion(+), 3 deletions(-)

-- 
2.53.0

[PATCH iwl-net v2 1/2] igc: don't disable ASPM L1.2 from __igc_resume()

[Daniel Viaño]

Commit 0325143b59c6 ("igc: disable L1.2 PCI-E link substate to avoid
performance issue") disables L1.2 on I226 in probe, in __igc_resume()
and in igc_io_slot_reset(). The resume path uses
pci_disable_link_state(), which takes pci_bus_sem for read.

__igc_resume() is also the runtime resume callback, and it can be
called with pci_bus_sem already held: pcie_do_recovery() runtime-
resumes all affected devices through

  pci_walk_bridge(bridge, pci_pm_runtime_get_sync, NULL)

i.e. from within pci_walk_bus(), which holds pci_bus_sem for read. If
the igc device is runtime-suspended at that point, the call chain is

  pci_walk_bus()                      down_read(&pci_bus_sem)
    pci_pm_runtime_get_sync()
      igc_runtime_resume()
        __igc_resume()
          pci_disable_link_state()    down_read(&pci_bus_sem)

A recursive down_read() deadlocks if a writer (e.g. a hot-plug event
adding or removing a device) queues on pci_bus_sem between the two
acquisitions. pci_disable_link_state_locked() cannot be used instead,
because on the system suspend/resume and ordinary runtime resume paths
the lock is not held.

The call is not needed on resume in the first place. The disable done
in igc_probe() is recorded in the link's aspm_disable mask, which is
only cleared through the ASPM sysfs interface. The PCI core honours
that mask whenever it reconfigures the link, including when the device
returns to D0 (pcie_aspm_pm_state_change()), and pci_restore_state()
restores the L1 PM Substates control registers saved with L1.2
already disabled. Drop the call from __igc_resume().

The issue was found by the Sashiko AI review of an earlier version of
this series.

Fixes: 0325143b59c6 ("igc: disable L1.2 PCI-E link substate to avoid performance issue")
Signed-off-by: Daniel Viaño <danividanivi@gmail.com>
---
 drivers/net/ethernet/intel/igc/igc_main.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
index 62a7c5178..1b5dcc166 100644
--- a/drivers/net/ethernet/intel/igc/igc_main.c
+++ b/drivers/net/ethernet/intel/igc/igc_main.c
@@ -7585,9 +7585,6 @@ static int __igc_resume(struct device *dev, bool rpm)
 	pci_enable_wake(pdev, PCI_D3hot, 0);
 	pci_enable_wake(pdev, PCI_D3cold, 0);
 
-	if (igc_is_device_id_i226(hw))
-		pci_disable_link_state(pdev, PCIE_LINK_STATE_L1_2);
-
 	if (igc_init_interrupt_scheme(adapter, true)) {
 		netdev_err(netdev, "Unable to allocate memory for queues\n");
 		return -ENOMEM;
-- 
2.53.0

[PATCH iwl-net v2 2/2] igc: treat I226-LMvP as an I226 device

[Daniel Viaño]

The I226-LMvP device ID (0x5503) was added by commit 8f20571db527
("igc: Add new device ID"), but when commit b8897dc54e3b ("igc: remove
I226 Qbv BaseTime restriction") introduced igc_is_device_id_i226(), it
was left out. As a result I226-LMvP is currently treated as neither an
I225 nor an I226 part.

Add IGC_DEV_ID_I226_LMVP to igc_is_device_id_i226(). This changes the
behaviour for I226-LMvP in the following places, all of which now
match the other I226 parts:

 - igc_probe() and igc_io_slot_reset() disable ASPM L1.2, as done for
   I226 since commit 0325143b59c6 ("igc: disable L1.2 PCI-E link
   substate to avoid performance issue").
 - igc_tsn_enable_offload() and igc_tsn_disable_offload() set and
   restore the I226-specific RETX (Qbv full) thresholds.
 - igc_tsn_enable_offload() handles the FutScdDis bit and the base
   time as on I226.

Intel confirmed that the I226 TSN functionality introduced by commit
b8897dc54e3b also applies to I226-LMvP.

Fixes: b8897dc54e3b ("igc: remove I226 Qbv BaseTime restriction")
Link: https://lore.kernel.org/all/5d45a583-198c-467d-afbe-8f19398e82f3@intel.com/
Signed-off-by: Daniel Viaño <danividanivi@gmail.com>
---
 drivers/net/ethernet/intel/igc/igc_base.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/intel/igc/igc_base.c b/drivers/net/ethernet/intel/igc/igc_base.c
index ab9120a31..6865fef40 100644
--- a/drivers/net/ethernet/intel/igc/igc_base.c
+++ b/drivers/net/ethernet/intel/igc/igc_base.c
@@ -447,6 +447,7 @@ bool igc_is_device_id_i226(struct igc_hw *hw)
 	case IGC_DEV_ID_I226_V:
 	case IGC_DEV_ID_I226_K:
 	case IGC_DEV_ID_I226_IT:
+	case IGC_DEV_ID_I226_LMVP:
 		return true;
 	default:
 		return false;
-- 
2.53.0