On 09/11/2026 12:45, Ruinskiy, Dima wrote:
> I think this patch might better be split into several patches.
>
> 1. Adding the missing IGC_DEV_ID_I226_LMVP to igc_is_device_id_i226().
> This was probably an accidental omission.

Agreed, I'll send this as a separate patch. Note that it also changes
the TSN code paths in igc_tsn.c for I226-LMvP (they will now get the
i226 behaviour), so it would be good if you could confirm that this
is correct for 0x5503.

> 2. Addressing the locked vs. non-locked call to pci_disable_link_state
> (the possible deadlock noted by Sashiko) - this should be looked into a
> bit more, I don't know off the top of my head what race is / isn't
> possible.

I believe the deadlock is real and already present for I226.
pcie_do_recovery() runtime-resumes the affected devices through
pci_walk_bridge(bridge, pci_pm_runtime_get_sync, NULL), i.e. with
pci_bus_sem held for read. If igc is runtime-suspended at that point,
igc_runtime_resume() -> __igc_resume() -> pci_disable_link_state()
takes pci_bus_sem for read a second time, which can deadlock if a
writer queues in between.

The L1.2 disable done in probe is sticky (link->aspm_disable) and the
PCI core honours it when restoring state on resume, so I plan to drop
the call from __igc_resume().

> 3. The actual problem with L1.2 substate this patch aims to solve. I
> checked with the HW team and there are two conclusions - (a) it is
> strange that L1.2 is enabled somehow for I225_LMVP - this is not
> expected.

You are right, and I was wrong about the mechanism. On my setup (HP
Thunderbolt Dock G4, I225-LMvP rev 03 behind a Goshen Ridge
downstream port) the NIC only advertises L1.1:

  L1SubCap: PCI-PM_L1.2- PCI-PM_L1.1+
            ASPM_L1.2- ASPM_L1.1+ L1_PM_Substates+

and the upstream port has no L1 PM Substates capability at all, so
L1.2 can never be enabled on this link. I will drop the I225-LMvP
part and investigate the dock errors separately with AER enabled.

> (b) generally it is probably best and simplest to remove any
> device ID checks and simply disable L1.2 across the board in this
> driver.

I can add a third patch doing that if you prefer. Otherwise I'd leave
it to you, since I have no I226 hardware to test it on.

Thanks,
Daniel

pw-bot: cr

Reply via email to