mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Ruinskiy, Dima" <dima.ruinskiy@intel.com>
To: "Daniel Viaño" <danividanivi@gmail.com>
Cc: <netdev-bot+sashiko@kernel.org>, <anthony.l.nguyen@intel.com>,
	<przemyslaw.kitszel@intel.com>,
	<intel-wired-lan@lists.osuosl.org>, <netdev@vger.kernel.org>,
	<bhelgaas@google.com>, <linux-pci@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>, <kuba@kernel.org>
Subject: Re: [PATCH net] igc: Disable PCIe L1.2 substate on I225-LMvP and I226-LMvP devices
Date: Wed, 11 Nov 2026 11:02:27 +0200	[thread overview]
Message-ID: <5d45a583-198c-467d-afbe-8f19398e82f3@intel.com> (raw)
In-Reply-To: <CAD6rab8LEfZcQgcZLmeiDzd-PUi56h=9aBEsuOS2qnoyZEsdxA@mail.gmail.com>

On 10/11/2026 12:04, Daniel Viaño wrote:
> 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.
I confirmed that the new I226-only TSN functionality (enabled in 
upstream commit b8897dc54e3b) applies to the I226_LMVP (0x5503) as well, 
so I see no risk adding it to the device ID list. This will make the 
L1.2 disable code function as intended.

> 
>> 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().
Great, so yes, this is good to have as a separate patch.

> 
>> 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.
Thank you for checking. Since you confirm the I225 issue you experience 
does not actually stem from L1.2, we can skip this for now.

--Dima


      reply	other threads:[~2026-11-11  9:02 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-25 11:28 Daniel Viaño
2026-09-29 11:41 ` netdev-bot+sashiko
2026-11-09 11:43   ` Ruinskiy, Dima
2026-11-10 10:04     ` Daniel Viaño
2026-11-11  9:02       ` Ruinskiy, Dima [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5d45a583-198c-467d-afbe-8f19398e82f3@intel.com \
    --to=dima.ruinskiy@intel.com \
    --cc=anthony.l.nguyen@intel.com \
    --cc=bhelgaas@google.com \
    --cc=danividanivi@gmail.com \
    --cc=intel-wired-lan@lists.osuosl.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=netdev-bot+sashiko@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=przemyslaw.kitszel@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®