[PATCH 0/5] PCI / igc: Improvements related to PTM enabling

[PATCH 0/5] PCI / igc: Improvements related to PTM enabling

[Mika Westerberg]

Hi all,

There is (still) an issue with Linux PCIe PTM enabling that happens because
Linux automatically enables PTM if certain capabilities are set. However,
turns out this is not enough because once we enumerate PCIe Switch Upstream
port we also enable PTM but the Downstream Ports are not yet enumerated.
This triggers floods of AER errors like this:

      pcieport 0000:00:07.1: AER: Multiple Uncorrectable (Non-Fatal) error message received from 0000:00:07.1
      pcieport 0000:00:07.1: PCIe Bus Error: severity=Uncorrectable (Non-Fatal), type=Transaction Layer, (Receiver ID)
      pcieport 0000:00:07.1:   device [8086:d44f] error status/mask=00200000/00000000
      pcieport 0000:00:07.1:    [21] ACSViol                (First)
      pcieport 0000:00:07.1: AER:   TLP Header: 0x34000000 0x00000052 0x00000000 0x00000000
      pcieport 0000:00:07.1: AER: device recovery successful
      pcieport 0000:00:07.1: AER: Uncorrectable (Non-Fatal) error message received from 0000:00:07.1

We have ACS Source Validation enabled so Requester ID 0 which is sent by
the not-enumerated Downstream Port triggers the ACS violation AER.

This can be prevented by enabling PTM when the whole topology has been
enumerated and doing it like that seems to be reasonable anyway because we
only have a couple of drivers enabling it now so it does not make sense to
enable otherwise as it consumes bandwidth.

I did that fix and the problem went away but wanted to test with a device
and driver that actually enables PTM. I have a couple of igc NICs here that
has this support. However, when testing I noticed that during power state
transitions we still get errors like this from igc:

  igc 0000:03:00.0 enp3s0: Timeout reading IGC_PTM_STAT register

and after this PTM for the device stays disabled.

This series includes fixes for igc that deal with the issues I found and
now PTM gets succesfully enabled and works accross suspend and runtime
suspend of igc, and there are no flood of AER errors as above. While there
there is one cleanup patch in the middle that drops unused parameter.

Mika Westerberg (5):
  igc: Call netif_queue_set_napi() with rntl locked
  igc: Let the PCI core deal with the PM resume flow
  igc: Don't reset the hardware on suspend path
  PCI/PTM: Drop granularity parameter from pci_enable_ptm()
  PCI/PTM: Do not enable PTM automatically for Root and Switch Upstream Ports

 drivers/net/ethernet/intel/ice/ice_main.c     |  2 +-
 drivers/net/ethernet/intel/idpf/idpf_main.c   |  2 +-
 drivers/net/ethernet/intel/igc/igc.h          |  2 +-
 drivers/net/ethernet/intel/igc/igc_ethtool.c  |  6 +-
 drivers/net/ethernet/intel/igc/igc_main.c     | 33 ++++----
 .../net/ethernet/mellanox/mlx5/core/main.c    |  2 +-
 drivers/pci/pcie/ptm.c                        | 77 ++++++++++---------
 include/linux/pci.h                           |  6 +-
 8 files changed, 64 insertions(+), 66 deletions(-)

-- 
2.50.1

[PATCH 1/5] igc: Call netif_queue_set_napi() with rntl locked

[Mika Westerberg]

When runtime resuming igc we get:

  [  516.161666] RTNL: assertion failed at ./include/net/netdev_lock.h (72)

Happens because commit 310ae9eb2617 ("net: designate queue -> napi
linking as "ops protected"") added check for this. For this reason drop
the special case for runtime PM from __igc_resume(). This makes it take
rtnl lock unconditionally.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/net/ethernet/intel/igc/igc_main.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
index 27e5c2109138..7ffd34bfa14e 100644
--- a/drivers/net/ethernet/intel/igc/igc_main.c
+++ b/drivers/net/ethernet/intel/igc/igc_main.c
@@ -7518,7 +7518,7 @@ static void igc_deliver_wake_packet(struct net_device *netdev)
 	netif_rx(skb);
 }
 
-static int __igc_resume(struct device *dev, bool rpm)
+static int __igc_resume(struct device *dev)
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
 	struct net_device *netdev = pci_get_drvdata(pdev);
@@ -7563,11 +7563,9 @@ static int __igc_resume(struct device *dev, bool rpm)
 	wr32(IGC_WUS, ~0);
 
 	if (netif_running(netdev)) {
-		if (!rpm)
-			rtnl_lock();
+		rtnl_lock();
 		err = __igc_open(netdev, true);
-		if (!rpm)
-			rtnl_unlock();
+		rtnl_unlock();
 		if (!err)
 			netif_device_attach(netdev);
 	}
@@ -7577,12 +7575,12 @@ static int __igc_resume(struct device *dev, bool rpm)
 
 static int igc_resume(struct device *dev)
 {
-	return __igc_resume(dev, false);
+	return __igc_resume(dev);
 }
 
 static int igc_runtime_resume(struct device *dev)
 {
-	return __igc_resume(dev, true);
+	return __igc_resume(dev);
 }
 
 static int igc_suspend(struct device *dev)
-- 
2.50.1

[PATCH 2/5] igc: Let the PCI core deal with the PM resume flow

[Mika Westerberg]

Currently igc driver calls pci_set_power_state() and pci_restore_state()
and the like to bring the device back from low power states. However,
PCI core handles all this on behalf of the driver. Furthermore with PTM
enabled the PCI core re-enables it on resume but the driver calls
pci_restore_state() which ends up disabling it again.

For this reason let the PCI core handle the common PM resume flow.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
---
 drivers/net/ethernet/intel/igc/igc_main.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
index 7ffd34bfa14e..99a4c99ddd57 100644
--- a/drivers/net/ethernet/intel/igc/igc_main.c
+++ b/drivers/net/ethernet/intel/igc/igc_main.c
@@ -7526,9 +7526,6 @@ static int __igc_resume(struct device *dev)
 	struct igc_hw *hw = &adapter->hw;
 	u32 err, val;
 
-	pci_set_power_state(pdev, PCI_D0);
-	pci_restore_state(pdev);
-
 	if (!pci_device_is_present(pdev))
 		return -ENODEV;
 	err = pci_enable_device_mem(pdev);
@@ -7538,9 +7535,6 @@ static int __igc_resume(struct device *dev)
 	}
 	pci_set_master(pdev);
 
-	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);
 
-- 
2.50.1

[PATCH 3/5] igc: Don't reset the hardware on suspend path

[Mika Westerberg]

Commit c01163dbd1b8 ("PCI/PM: Always disable PTM for all devices during
suspend") made the PCI core to suspend (disable) PTM before driver
suspend hooks are called. In case of igc what happens is that on suspend
path PCI core calls pci_suspend_ptm() then igc suspend hook that calls
igc_down() that ends up calling igc_ptp_reset() (which according to the
comment is actually needed for re-enabling the device). Anyways that
function also poll IGC_PTM_STAT that will end up timing out because PTM
is already disabled:

  [  160.716119] igc 0000:03:00.0 enp3s0: Timeout reading IGC_PTM_STAT register

There should be no reason resetting the hardware on suspend path so fix
this by avoiding the reset.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/net/ethernet/intel/igc/igc.h         |  2 +-
 drivers/net/ethernet/intel/igc/igc_ethtool.c |  6 +++---
 drivers/net/ethernet/intel/igc/igc_main.c    | 13 +++++++------
 3 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/intel/igc/igc.h b/drivers/net/ethernet/intel/igc/igc.h
index a427f05814c1..21d6eba1cc09 100644
--- a/drivers/net/ethernet/intel/igc/igc.h
+++ b/drivers/net/ethernet/intel/igc/igc.h
@@ -349,7 +349,7 @@ struct igc_adapter {
 };
 
 void igc_up(struct igc_adapter *adapter);
-void igc_down(struct igc_adapter *adapter);
+void igc_down(struct igc_adapter *adapter, bool reset);
 int igc_open(struct net_device *netdev);
 int igc_close(struct net_device *netdev);
 int igc_setup_tx_resources(struct igc_ring *ring);
diff --git a/drivers/net/ethernet/intel/igc/igc_ethtool.c b/drivers/net/ethernet/intel/igc/igc_ethtool.c
index 0122009bedd0..4d1bcc19255f 100644
--- a/drivers/net/ethernet/intel/igc/igc_ethtool.c
+++ b/drivers/net/ethernet/intel/igc/igc_ethtool.c
@@ -638,7 +638,7 @@ igc_ethtool_set_ringparam(struct net_device *netdev,
 		goto clear_reset;
 	}
 
-	igc_down(adapter);
+	igc_down(adapter, true);
 
 	/* We can't just free everything and then setup again,
 	 * because the ISRs in MSI-X mode get passed pointers
@@ -737,7 +737,7 @@ static int igc_ethtool_set_pauseparam(struct net_device *netdev,
 	if (adapter->fc_autoneg == AUTONEG_ENABLE) {
 		hw->fc.requested_mode = igc_fc_default;
 		if (netif_running(adapter->netdev)) {
-			igc_down(adapter);
+			igc_down(adapter, true);
 			igc_up(adapter);
 		} else {
 			igc_reset(adapter);
@@ -2077,7 +2077,7 @@ igc_ethtool_set_link_ksettings(struct net_device *netdev,
 
 	/* reset the link */
 	if (netif_running(adapter->netdev)) {
-		igc_down(adapter);
+		igc_down(adapter, true);
 		igc_up(adapter);
 	} else {
 		igc_reset(adapter);
diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
index 99a4c99ddd57..84f5d92c81cc 100644
--- a/drivers/net/ethernet/intel/igc/igc_main.c
+++ b/drivers/net/ethernet/intel/igc/igc_main.c
@@ -5308,8 +5308,9 @@ void igc_update_stats(struct igc_adapter *adapter)
 /**
  * igc_down - Close the interface
  * @adapter: board private structure
+ * @reset: issue reset
  */
-void igc_down(struct igc_adapter *adapter)
+void igc_down(struct igc_adapter *adapter, bool reset)
 {
 	struct net_device *netdev = adapter->netdev;
 	struct igc_hw *hw = &adapter->hw;
@@ -5365,7 +5366,7 @@ void igc_down(struct igc_adapter *adapter)
 	adapter->link_speed = 0;
 	adapter->link_duplex = 0;
 
-	if (!pci_channel_offline(adapter->pdev))
+	if (reset && !pci_channel_offline(adapter->pdev))
 		igc_reset(adapter);
 
 	/* clear VLAN promisc flag so VFTA will be updated if necessary */
@@ -5383,7 +5384,7 @@ void igc_reinit_locked(struct igc_adapter *adapter)
 {
 	while (test_and_set_bit(__IGC_RESETTING, &adapter->state))
 		usleep_range(1000, 2000);
-	igc_down(adapter);
+	igc_down(adapter, true);
 	igc_up(adapter);
 	clear_bit(__IGC_RESETTING, &adapter->state);
 }
@@ -5437,7 +5438,7 @@ static int igc_change_mtu(struct net_device *netdev, int new_mtu)
 	adapter->max_frame_size = max_frame;
 
 	if (netif_running(netdev))
-		igc_down(adapter);
+		igc_down(adapter, true);
 
 	netdev_dbg(netdev, "changing MTU from %d to %d\n", netdev->mtu, new_mtu);
 	WRITE_ONCE(netdev->mtu, new_mtu);
@@ -6301,7 +6302,7 @@ static int __igc_close(struct net_device *netdev, bool suspending)
 	if (!suspending)
 		pm_runtime_get_sync(&pdev->dev);
 
-	igc_down(adapter);
+	igc_down(adapter, !suspending);
 
 	igc_release_hw_control(adapter);
 
@@ -7628,7 +7629,7 @@ static pci_ers_result_t igc_io_error_detected(struct pci_dev *pdev,
 	}
 
 	if (netif_running(netdev))
-		igc_down(adapter);
+		igc_down(adapter, true);
 	pci_disable_device(pdev);
 	rtnl_unlock();
 
-- 
2.50.1

[PATCH 4/5] PCI/PTM: Drop granularity parameter from pci_enable_ptm()

[Mika Westerberg]

This is not used by any of the existing callers so we can simplify the
function slightly and get rid of that.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_main.c      |  2 +-
 drivers/net/ethernet/intel/idpf/idpf_main.c    |  2 +-
 drivers/net/ethernet/intel/igc/igc_main.c      |  2 +-
 drivers/net/ethernet/mellanox/mlx5/core/main.c |  2 +-
 drivers/pci/pcie/ptm.c                         | 11 +++--------
 include/linux/pci.h                            |  4 ++--
 6 files changed, 9 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index ebf48feffb30..b35c4e4ecd2a 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -5028,7 +5028,7 @@ static int ice_init(struct ice_pf *pf)
 	}
 
 	if (pf->hw.mac_type == ICE_MAC_E830) {
-		err = pci_enable_ptm(pf->pdev, NULL);
+		err = pci_enable_ptm(pf->pdev);
 		if (err)
 			dev_dbg(dev, "PCIe PTM not supported by PCIe bus/controller\n");
 	}
diff --git a/drivers/net/ethernet/intel/idpf/idpf_main.c b/drivers/net/ethernet/intel/idpf/idpf_main.c
index 0dd741dcfcdb..ab3c409e587b 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_main.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_main.c
@@ -257,7 +257,7 @@ static int idpf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 		goto err_free;
 	}
 
-	err = pci_enable_ptm(pdev, NULL);
+	err = pci_enable_ptm(pdev);
 	if (err)
 		pci_dbg(pdev, "PCIe PTM is not supported by PCIe bus/controller\n");
 
diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
index 84f5d92c81cc..291222b06915 100644
--- a/drivers/net/ethernet/intel/igc/igc_main.c
+++ b/drivers/net/ethernet/intel/igc/igc_main.c
@@ -7124,7 +7124,7 @@ static int igc_probe(struct pci_dev *pdev,
 	if (err)
 		goto err_pci_reg;
 
-	err = pci_enable_ptm(pdev, NULL);
+	err = pci_enable_ptm(pdev);
 	if (err < 0)
 		dev_info(&pdev->dev, "PCIe PTM not supported by PCIe bus/controller\n");
 
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/main.c b/drivers/net/ethernet/mellanox/mlx5/core/main.c
index fdc3ba20912e..0b94d4ed0ef6 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c
@@ -960,7 +960,7 @@ static int mlx5_pci_init(struct mlx5_core_dev *dev, struct pci_dev *pdev,
 
 	mlx5_pci_vsc_init(dev);
 
-	pci_enable_ptm(pdev, NULL);
+	pci_enable_ptm(pdev);
 
 	return 0;
 
diff --git a/drivers/pci/pcie/ptm.c b/drivers/pci/pcie/ptm.c
index 91a598ed534c..2c848ae4f15f 100644
--- a/drivers/pci/pcie/ptm.c
+++ b/drivers/pci/pcie/ptm.c
@@ -88,7 +88,7 @@ void pci_ptm_init(struct pci_dev *dev)
 
 	if (pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT ||
 	    pci_pcie_type(dev) == PCI_EXP_TYPE_UPSTREAM)
-		pci_enable_ptm(dev, NULL);
+		pci_enable_ptm(dev);
 }
 
 void pci_save_ptm_state(struct pci_dev *dev)
@@ -182,15 +182,13 @@ static int __pci_enable_ptm(struct pci_dev *dev)
 /**
  * pci_enable_ptm() - Enable Precision Time Measurement
  * @dev: PCI device
- * @granularity: pointer to return granularity
  *
- * Enable Precision Time Measurement for @dev.  If successful and
- * @granularity is non-NULL, return the Effective Granularity.
+ * Enable Precision Time Measurement for @dev.
  *
  * Return: zero if successful, or -EINVAL if @dev lacks a PTM Capability or
  * is not a PTM Root and lacks an upstream path of PTM-enabled devices.
  */
-int pci_enable_ptm(struct pci_dev *dev, u8 *granularity)
+int pci_enable_ptm(struct pci_dev *dev)
 {
 	int rc;
 	char clock_desc[8];
@@ -201,9 +199,6 @@ int pci_enable_ptm(struct pci_dev *dev, u8 *granularity)
 
 	dev->ptm_enabled = 1;
 
-	if (granularity)
-		*granularity = dev->ptm_granularity;
-
 	switch (dev->ptm_granularity) {
 	case 0:
 		snprintf(clock_desc, sizeof(clock_desc), "unknown");
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 1c270f1d5123..8aaa72dcb164 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1975,11 +1975,11 @@ struct pci_ptm_debugfs {
 };
 
 #ifdef CONFIG_PCIE_PTM
-int pci_enable_ptm(struct pci_dev *dev, u8 *granularity);
+int pci_enable_ptm(struct pci_dev *dev);
 void pci_disable_ptm(struct pci_dev *dev);
 bool pcie_ptm_enabled(struct pci_dev *dev);
 #else
-static inline int pci_enable_ptm(struct pci_dev *dev, u8 *granularity)
+static inline int pci_enable_ptm(struct pci_dev *dev)
 { return -EINVAL; }
 static inline void pci_disable_ptm(struct pci_dev *dev) { }
 static inline bool pcie_ptm_enabled(struct pci_dev *dev)
-- 
2.50.1

[PATCH 5/5] PCI/PTM: Do not enable PTM automatically for Root and Switch Upstream Ports

[Mika Westerberg]

Currently we enable PTM automatically for Root and Switch Upstream Ports
if the advertised capabilities support the relevant role. However, there
are few issues with this. First of all if there is no Endpoint that
actually needs the PTM functionality, this is just wasting link
bandwidth. There are just a couple of drivers calling pci_ptm_enable()
in the tree.

Secondly we do the enablement in pci_ptm_init() that is called pretty
early for the Switch Upstream Port before Downstream Ports are even
enumerated. Since the Upstream Port configuration affects the whole
Switch enabling it this early might cause the PTM requests to be sent
already. We actually do see effect of this:

  pcieport 0000:00:07.1: pciehp: Slot(6-1): Card present
  pcieport 0000:00:07.1: pciehp: Slot(6-1): Link Up
  pci 0000:2c:00.0: [8086:5786] type 01 class 0x060400 PCIe Switch Upstream Port
  pci 0000:2c:00.0: PCI bridge to [bus 00]
  pci 0000:2c:00.0:   bridge window [io  0x0000-0x0fff]
  pci 0000:2c:00.0:   bridge window [mem 0x00000000-0x000fffff]
  pci 0000:2c:00.0:   bridge window [mem 0x00000000-0x000fffff 64bit pref]
  ...
  pci 0000:2c:00.0: PME# supported from D0 D1 D2 D3hot D3cold
  pci 0000:2c:00.0: PTM enabled, 4ns granularity

At this point we have only enumerated the Switch Upstream Port and now
PTM got enabled which immediately triggers flood of these:

  pcieport 0000:00:07.1: AER: Multiple Uncorrectable (Non-Fatal) error message received from 0000:00:07.1
  pcieport 0000:00:07.1: PCIe Bus Error: severity=Uncorrectable (Non-Fatal), type=Transaction Layer, (Receiver ID)
  pcieport 0000:00:07.1:   device [8086:d44f] error status/mask=00200000/00000000
  pcieport 0000:00:07.1:    [21] ACSViol                (First)
  pcieport 0000:00:07.1: AER:   TLP Header: 0x34000000 0x00000052 0x00000000 0x00000000
  pcieport 0000:00:07.1: AER: device recovery successful
  pcieport 0000:00:07.1: AER: Uncorrectable (Non-Fatal) error message received from 0000:00:07.1

In the above TLP Header the Requester ID is 0 which rightfully triggers
an error as we have ACS Source Validation enabled.

For this reason change the PTM enablement to happen at the time
pci_enable_ptm() is called. It will try to enable PTM first for upstream
devices before enabling for the Endpoint itself. For disable path we
need to keep count of how many times PTM has been enabled and disable
only on the last so change the dev->ptm_enabled to a counter (and rename
it to dev->ptm_enable_cnt analogous to dev->pci_enable_cnt).

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/pci/pcie/ptm.c | 68 ++++++++++++++++++++++++------------------
 include/linux/pci.h    |  2 +-
 2 files changed, 40 insertions(+), 30 deletions(-)

diff --git a/drivers/pci/pcie/ptm.c b/drivers/pci/pcie/ptm.c
index 2c848ae4f15f..a41ffd1914de 100644
--- a/drivers/pci/pcie/ptm.c
+++ b/drivers/pci/pcie/ptm.c
@@ -52,6 +52,7 @@ void pci_ptm_init(struct pci_dev *dev)
 		return;
 
 	dev->ptm_cap = ptm;
+	atomic_set(&dev->ptm_enable_cnt, 0);
 	pci_add_ext_cap_save_buffer(dev, PCI_EXT_CAP_ID_PTM, sizeof(u32));
 
 	pci_read_config_dword(dev, ptm + PCI_PTM_CAP, &cap);
@@ -85,10 +86,6 @@ void pci_ptm_init(struct pci_dev *dev)
 		dev->ptm_responder = 1;
 	if (cap & PCI_PTM_CAP_REQ)
 		dev->ptm_requester = 1;
-
-	if (pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT ||
-	    pci_pcie_type(dev) == PCI_EXP_TYPE_UPSTREAM)
-		pci_enable_ptm(dev);
 }
 
 void pci_save_ptm_state(struct pci_dev *dev)
@@ -129,26 +126,11 @@ void pci_restore_ptm_state(struct pci_dev *dev)
 static int __pci_enable_ptm(struct pci_dev *dev)
 {
 	u16 ptm = dev->ptm_cap;
-	struct pci_dev *ups;
 	u32 ctrl;
 
 	if (!ptm)
 		return -EINVAL;
 
-	/*
-	 * A device uses local PTM Messages to request time information
-	 * from a PTM Root that's farther upstream.  Every device along the
-	 * path must support PTM and have it enabled so it can handle the
-	 * messages.  Therefore, if this device is not a PTM Root, the
-	 * upstream link partner must have PTM enabled before we can enable
-	 * PTM.
-	 */
-	if (!dev->ptm_root) {
-		ups = pci_upstream_ptm(dev);
-		if (!ups || !ups->ptm_enabled)
-			return -EINVAL;
-	}
-
 	switch (pci_pcie_type(dev)) {
 	case PCI_EXP_TYPE_ROOT_PORT:
 		if (!dev->ptm_root)
@@ -193,11 +175,35 @@ int pci_enable_ptm(struct pci_dev *dev)
 	int rc;
 	char clock_desc[8];
 
+	/*
+	 * A device uses local PTM Messages to request time information
+	 * from a PTM Root that's farther upstream. Every device along
+	 * the path must support PTM and have it enabled so it can
+	 * handle the messages. Therefore, if this device is not a PTM
+	 * Root, the upstream link partner must have PTM enabled before
+	 * we can enable PTM.
+	 */
+	if (!dev->ptm_root) {
+		struct pci_dev *parent;
+
+		parent = pci_upstream_ptm(dev);
+		if (!parent)
+			return -EINVAL;
+		/* Enable PTM for the parent */
+		rc = pci_enable_ptm(parent);
+		if (rc)
+			return rc;
+	}
+
+	/* Already enabled? */
+	if (atomic_inc_return(&dev->ptm_enable_cnt) > 1)
+		return 0;
+
 	rc = __pci_enable_ptm(dev);
-	if (rc)
+	if (rc) {
+		atomic_dec(&dev->ptm_enable_cnt);
 		return rc;
-
-	dev->ptm_enabled = 1;
+	}
 
 	switch (dev->ptm_granularity) {
 	case 0:
@@ -239,27 +245,31 @@ static void __pci_disable_ptm(struct pci_dev *dev)
  */
 void pci_disable_ptm(struct pci_dev *dev)
 {
-	if (dev->ptm_enabled) {
+	struct pci_dev *parent;
+
+	if (atomic_dec_and_test(&dev->ptm_enable_cnt))
 		__pci_disable_ptm(dev);
-		dev->ptm_enabled = 0;
-	}
+
+	parent = pci_upstream_ptm(dev);
+	if (parent)
+		pci_disable_ptm(parent);
 }
 EXPORT_SYMBOL(pci_disable_ptm);
 
 /*
- * Disable PTM, but preserve dev->ptm_enabled so we silently re-enable it on
+ * Disable PTM, but preserve dev->ptm_enable_cnt so we silently re-enable it on
  * resume if necessary.
  */
 void pci_suspend_ptm(struct pci_dev *dev)
 {
-	if (dev->ptm_enabled)
+	if (atomic_read(&dev->ptm_enable_cnt))
 		__pci_disable_ptm(dev);
 }
 
 /* If PTM was enabled before suspend, re-enable it when resuming */
 void pci_resume_ptm(struct pci_dev *dev)
 {
-	if (dev->ptm_enabled)
+	if (atomic_read(&dev->ptm_enable_cnt))
 		__pci_enable_ptm(dev);
 }
 
@@ -268,7 +278,7 @@ bool pcie_ptm_enabled(struct pci_dev *dev)
 	if (!dev)
 		return false;
 
-	return dev->ptm_enabled;
+	return atomic_read(&dev->ptm_enable_cnt);
 }
 EXPORT_SYMBOL(pcie_ptm_enabled);
 
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 8aaa72dcb164..7e49d35d81a5 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -518,7 +518,7 @@ struct pci_dev {
 	unsigned int	ptm_root:1;
 	unsigned int	ptm_responder:1;
 	unsigned int	ptm_requester:1;
-	unsigned int	ptm_enabled:1;
+	atomic_t	ptm_enable_cnt;		/* pci_enable_ptm() has been called */
 	u8		ptm_granularity;
 #endif
 #ifdef CONFIG_PCI_MSI
-- 
2.50.1

Re: [PATCH 2/5] igc: Let the PCI core deal with the PM resume flow

[Bjorn Helgaas]

On Tue, Feb 24, 2026 at 12:10:41PM +0100, Mika Westerberg wrote:
> Currently igc driver calls pci_set_power_state() and pci_restore_state()
> and the like to bring the device back from low power states. However,
> PCI core handles all this on behalf of the driver. Furthermore with PTM
> enabled the PCI core re-enables it on resume but the driver calls
> pci_restore_state() which ends up disabling it again.
> 
> For this reason let the PCI core handle the common PM resume flow.
> 
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>

I love it, thanks a lot for doing this!

Do we still need the pci_enable_device_mem() and pci_set_master()
in __igc_resume()?

I suppose some of that is related to the pci_disable_device() in the
suspend path (__igc_shutdown()), but there are only a few dozen
drivers that do this, so I'm not sure it's essential.

> ---
>  drivers/net/ethernet/intel/igc/igc_main.c | 6 ------
>  1 file changed, 6 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
> index 7ffd34bfa14e..99a4c99ddd57 100644
> --- a/drivers/net/ethernet/intel/igc/igc_main.c
> +++ b/drivers/net/ethernet/intel/igc/igc_main.c
> @@ -7526,9 +7526,6 @@ static int __igc_resume(struct device *dev)
>  	struct igc_hw *hw = &adapter->hw;
>  	u32 err, val;
>  
> -	pci_set_power_state(pdev, PCI_D0);
> -	pci_restore_state(pdev);
> -
>  	if (!pci_device_is_present(pdev))
>  		return -ENODEV;
>  	err = pci_enable_device_mem(pdev);
> @@ -7538,9 +7535,6 @@ static int __igc_resume(struct device *dev)
>  	}
>  	pci_set_master(pdev);
>  
> -	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);
>  
> -- 
> 2.50.1
> 

Re: [PATCH 0/5] PCI / igc: Improvements related to PTM enabling

[Bjorn Helgaas]

On Tue, Feb 24, 2026 at 12:10:39PM +0100, Mika Westerberg wrote:
> Hi all,
> 
> There is (still) an issue with Linux PCIe PTM enabling that happens because
> Linux automatically enables PTM if certain capabilities are set. However,
> turns out this is not enough because once we enumerate PCIe Switch Upstream
> port we also enable PTM but the Downstream Ports are not yet enumerated.
> This triggers floods of AER errors like this:
> 
>       pcieport 0000:00:07.1: AER: Multiple Uncorrectable (Non-Fatal) error message received from 0000:00:07.1
>       pcieport 0000:00:07.1: PCIe Bus Error: severity=Uncorrectable (Non-Fatal), type=Transaction Layer, (Receiver ID)
>       pcieport 0000:00:07.1:   device [8086:d44f] error status/mask=00200000/00000000
>       pcieport 0000:00:07.1:    [21] ACSViol                (First)
>       pcieport 0000:00:07.1: AER:   TLP Header: 0x34000000 0x00000052 0x00000000 0x00000000
>       pcieport 0000:00:07.1: AER: device recovery successful
>       pcieport 0000:00:07.1: AER: Uncorrectable (Non-Fatal) error message received from 0000:00:07.1
> 
> We have ACS Source Validation enabled so Requester ID 0 which is sent by
> the not-enumerated Downstream Port triggers the ACS violation AER.
> 
> This can be prevented by enabling PTM when the whole topology has been
> enumerated and doing it like that seems to be reasonable anyway because we
> only have a couple of drivers enabling it now so it does not make sense to
> enable otherwise as it consumes bandwidth.
> 
> I did that fix and the problem went away but wanted to test with a device
> and driver that actually enables PTM. I have a couple of igc NICs here that
> has this support. However, when testing I noticed that during power state
> transitions we still get errors like this from igc:
> 
>   igc 0000:03:00.0 enp3s0: Timeout reading IGC_PTM_STAT register
> 
> and after this PTM for the device stays disabled.
> 
> This series includes fixes for igc that deal with the issues I found and
> now PTM gets succesfully enabled and works accross suspend and runtime
> suspend of igc, and there are no flood of AER errors as above. While there
> there is one cleanup patch in the middle that drops unused parameter.
> 
> Mika Westerberg (5):
>   igc: Call netif_queue_set_napi() with rntl locked
>   igc: Let the PCI core deal with the PM resume flow
>   igc: Don't reset the hardware on suspend path
>   PCI/PTM: Drop granularity parameter from pci_enable_ptm()
>   PCI/PTM: Do not enable PTM automatically for Root and Switch Upstream Ports

These last two don't look dependent on the igc patches, so I applied
them to pci/ptm for v7.1, thanks!

Let me know if there is some dependency and I can ack them and drop
them from the PCI tree.

>  drivers/net/ethernet/intel/ice/ice_main.c     |  2 +-
>  drivers/net/ethernet/intel/idpf/idpf_main.c   |  2 +-
>  drivers/net/ethernet/intel/igc/igc.h          |  2 +-
>  drivers/net/ethernet/intel/igc/igc_ethtool.c  |  6 +-
>  drivers/net/ethernet/intel/igc/igc_main.c     | 33 ++++----
>  .../net/ethernet/mellanox/mlx5/core/main.c    |  2 +-
>  drivers/pci/pcie/ptm.c                        | 77 ++++++++++---------
>  include/linux/pci.h                           |  6 +-
>  8 files changed, 64 insertions(+), 66 deletions(-)
> 
> -- 
> 2.50.1
> 

Re: [PATCH 1/5] igc: Call netif_queue_set_napi() with rntl locked

[Vinicius Costa Gomes]

Mika Westerberg <mika.westerberg@linux.intel.com> writes:

> When runtime resuming igc we get:
>
>   [  516.161666] RTNL: assertion failed at ./include/net/netdev_lock.h (72)
>
> Happens because commit 310ae9eb2617 ("net: designate queue -> napi
> linking as "ops protected"") added check for this. For this reason drop
> the special case for runtime PM from __igc_resume(). This makes it take
> rtnl lock unconditionally.
>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> ---

Typo in the commit subject: rntl -> rtnl

I think it's worth fixing, anyway:

Acked-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>


Cheers,
-- 
Vinicius

Re: [PATCH 2/5] igc: Let the PCI core deal with the PM resume flow

[Vinicius Costa Gomes]

Mika Westerberg <mika.westerberg@linux.intel.com> writes:

> Currently igc driver calls pci_set_power_state() and pci_restore_state()
> and the like to bring the device back from low power states. However,
> PCI core handles all this on behalf of the driver. Furthermore with PTM
> enabled the PCI core re-enables it on resume but the driver calls
> pci_restore_state() which ends up disabling it again.
>
> For this reason let the PCI core handle the common PM resume flow.
>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> ---

Acked-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>


Cheers,
-- 
Vinicius

Re: [PATCH 3/5] igc: Don't reset the hardware on suspend path

[Vinicius Costa Gomes]

Mika Westerberg <mika.westerberg@linux.intel.com> writes:

> Commit c01163dbd1b8 ("PCI/PM: Always disable PTM for all devices during
> suspend") made the PCI core to suspend (disable) PTM before driver
> suspend hooks are called. In case of igc what happens is that on suspend
> path PCI core calls pci_suspend_ptm() then igc suspend hook that calls
> igc_down() that ends up calling igc_ptp_reset() (which according to the
> comment is actually needed for re-enabling the device). Anyways that
> function also poll IGC_PTM_STAT that will end up timing out because PTM
> is already disabled:
>
>   [  160.716119] igc 0000:03:00.0 enp3s0: Timeout reading IGC_PTM_STAT register
>
> There should be no reason resetting the hardware on suspend path so fix
> this by avoiding the reset.
>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> ---

Acked-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>


Cheers,
-- 
Vinicius

Re: [Intel-wired-lan] [PATCH 4/5] PCI/PTM: Drop granularity parameter from pci_enable_ptm()

[Jacob Keller]

On 2/24/2026 3:10 AM, Mika Westerberg wrote:
> diff --git a/drivers/pci/pcie/ptm.c b/drivers/pci/pcie/ptm.c
> index 91a598ed534c..2c848ae4f15f 100644
> --- a/drivers/pci/pcie/ptm.c
> +++ b/drivers/pci/pcie/ptm.c
> @@ -88,7 +88,7 @@ void pci_ptm_init(struct pci_dev *dev)
>   
>   	if (pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT ||
>   	    pci_pcie_type(dev) == PCI_EXP_TYPE_UPSTREAM)
> -		pci_enable_ptm(dev, NULL);
> +		pci_enable_ptm(dev);
>   }
>   
>   void pci_save_ptm_state(struct pci_dev *dev)
> @@ -182,15 +182,13 @@ static int __pci_enable_ptm(struct pci_dev *dev)
>   /**
>    * pci_enable_ptm() - Enable Precision Time Measurement
>    * @dev: PCI device
> - * @granularity: pointer to return granularity
>    *
> - * Enable Precision Time Measurement for @dev.  If successful and
> - * @granularity is non-NULL, return the Effective Granularity.
> + * Enable Precision Time Measurement for @dev.
>    *
>    * Return: zero if successful, or -EINVAL if @dev lacks a PTM Capability or
>    * is not a PTM Root and lacks an upstream path of PTM-enabled devices.
>    */
> -int pci_enable_ptm(struct pci_dev *dev, u8 *granularity)
> +int pci_enable_ptm(struct pci_dev *dev)
>   {
>   	int rc;
>   	char clock_desc[8];
> @@ -201,9 +199,6 @@ int pci_enable_ptm(struct pci_dev *dev, u8 *granularity)
>   
>   	dev->ptm_enabled = 1;
>   
> -	if (granularity)
> -		*granularity = dev->ptm_granularity;
> -


Presumably, if a driver ever actually needs this ptm_granularity in the 
future, they can just read it from the device, or we can add a helper to 
access if needed. Makes sense.

Thanks,
Jake

Re: [Intel-wired-lan] [PATCH 0/5] PCI / igc: Improvements related to PTM enabling

[Jacob Keller]

On 2/24/2026 3:10 AM, Mika Westerberg wrote:
> Hi all,
> 
> There is (still) an issue with Linux PCIe PTM enabling that happens because
> Linux automatically enables PTM if certain capabilities are set. However,
> turns out this is not enough because once we enumerate PCIe Switch Upstream
> port we also enable PTM but the Downstream Ports are not yet enumerated.
> This triggers floods of AER errors like this:
> 
>        pcieport 0000:00:07.1: AER: Multiple Uncorrectable (Non-Fatal) error message received from 0000:00:07.1
>        pcieport 0000:00:07.1: PCIe Bus Error: severity=Uncorrectable (Non-Fatal), type=Transaction Layer, (Receiver ID)
>        pcieport 0000:00:07.1:   device [8086:d44f] error status/mask=00200000/00000000
>        pcieport 0000:00:07.1:    [21] ACSViol                (First)
>        pcieport 0000:00:07.1: AER:   TLP Header: 0x34000000 0x00000052 0x00000000 0x00000000
>        pcieport 0000:00:07.1: AER: device recovery successful
>        pcieport 0000:00:07.1: AER: Uncorrectable (Non-Fatal) error message received from 0000:00:07.1
> 
> We have ACS Source Validation enabled so Requester ID 0 which is sent by
> the not-enumerated Downstream Port triggers the ACS violation AER.
> 
> This can be prevented by enabling PTM when the whole topology has been
> enumerated and doing it like that seems to be reasonable anyway because we
> only have a couple of drivers enabling it now so it does not make sense to
> enable otherwise as it consumes bandwidth.
> 
> I did that fix and the problem went away but wanted to test with a device
> and driver that actually enables PTM. I have a couple of igc NICs here that
> has this support. However, when testing I noticed that during power state
> transitions we still get errors like this from igc:
> 
>    igc 0000:03:00.0 enp3s0: Timeout reading IGC_PTM_STAT register
> 
> and after this PTM for the device stays disabled.
> 
> This series includes fixes for igc that deal with the issues I found and
> now PTM gets succesfully enabled and works accross suspend and runtime
> suspend of igc, and there are no flood of AER errors as above. While there
> there is one cleanup patch in the middle that drops unused parameter.
> 
> Mika Westerberg (5):
>    igc: Call netif_queue_set_napi() with rntl locked
>    igc: Let the PCI core deal with the PM resume flow
>    igc: Don't reset the hardware on suspend path

I'm not one of the main developers for igc, but everything here looks 
good to me.

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

Re: [PATCH 2/5] igc: Let the PCI core deal with the PM resume flow

[Mika Westerberg]

On Tue, Feb 24, 2026 at 10:58:37AM -0600, Bjorn Helgaas wrote:
> On Tue, Feb 24, 2026 at 12:10:41PM +0100, Mika Westerberg wrote:
> > Currently igc driver calls pci_set_power_state() and pci_restore_state()
> > and the like to bring the device back from low power states. However,
> > PCI core handles all this on behalf of the driver. Furthermore with PTM
> > enabled the PCI core re-enables it on resume but the driver calls
> > pci_restore_state() which ends up disabling it again.
> > 
> > For this reason let the PCI core handle the common PM resume flow.
> > 
> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> 
> I love it, thanks a lot for doing this!
> 
> Do we still need the pci_enable_device_mem() and pci_set_master()
> in __igc_resume()?
>
> I suppose some of that is related to the pci_disable_device() in the
> suspend path (__igc_shutdown()), but there are only a few dozen
> drivers that do this, so I'm not sure it's essential.

I think they are just as you describe due the fact there are explicit
pci_disable_device() calls. Probably we can get rid of them as well but
that requires careful testing that nothing accidentally breaks.

@Vitaly, what do you think? I can take a look.

Re: [PATCH 0/5] PCI / igc: Improvements related to PTM enabling

[Mika Westerberg]

On Tue, Feb 24, 2026 at 11:13:08AM -0600, Bjorn Helgaas wrote:
> On Tue, Feb 24, 2026 at 12:10:39PM +0100, Mika Westerberg wrote:
> > Hi all,
> > 
> > There is (still) an issue with Linux PCIe PTM enabling that happens because
> > Linux automatically enables PTM if certain capabilities are set. However,
> > turns out this is not enough because once we enumerate PCIe Switch Upstream
> > port we also enable PTM but the Downstream Ports are not yet enumerated.
> > This triggers floods of AER errors like this:
> > 
> >       pcieport 0000:00:07.1: AER: Multiple Uncorrectable (Non-Fatal) error message received from 0000:00:07.1
> >       pcieport 0000:00:07.1: PCIe Bus Error: severity=Uncorrectable (Non-Fatal), type=Transaction Layer, (Receiver ID)
> >       pcieport 0000:00:07.1:   device [8086:d44f] error status/mask=00200000/00000000
> >       pcieport 0000:00:07.1:    [21] ACSViol                (First)
> >       pcieport 0000:00:07.1: AER:   TLP Header: 0x34000000 0x00000052 0x00000000 0x00000000
> >       pcieport 0000:00:07.1: AER: device recovery successful
> >       pcieport 0000:00:07.1: AER: Uncorrectable (Non-Fatal) error message received from 0000:00:07.1
> > 
> > We have ACS Source Validation enabled so Requester ID 0 which is sent by
> > the not-enumerated Downstream Port triggers the ACS violation AER.
> > 
> > This can be prevented by enabling PTM when the whole topology has been
> > enumerated and doing it like that seems to be reasonable anyway because we
> > only have a couple of drivers enabling it now so it does not make sense to
> > enable otherwise as it consumes bandwidth.
> > 
> > I did that fix and the problem went away but wanted to test with a device
> > and driver that actually enables PTM. I have a couple of igc NICs here that
> > has this support. However, when testing I noticed that during power state
> > transitions we still get errors like this from igc:
> > 
> >   igc 0000:03:00.0 enp3s0: Timeout reading IGC_PTM_STAT register
> > 
> > and after this PTM for the device stays disabled.
> > 
> > This series includes fixes for igc that deal with the issues I found and
> > now PTM gets succesfully enabled and works accross suspend and runtime
> > suspend of igc, and there are no flood of AER errors as above. While there
> > there is one cleanup patch in the middle that drops unused parameter.
> > 
> > Mika Westerberg (5):
> >   igc: Call netif_queue_set_napi() with rntl locked
> >   igc: Let the PCI core deal with the PM resume flow
> >   igc: Don't reset the hardware on suspend path
> >   PCI/PTM: Drop granularity parameter from pci_enable_ptm()
> >   PCI/PTM: Do not enable PTM automatically for Root and Switch Upstream Ports
> 
> These last two don't look dependent on the igc patches, so I applied
> them to pci/ptm for v7.1, thanks!

Thanks!

> Let me know if there is some dependency and I can ack them and drop
> them from the PCI tree.

They can be separate. No dependency. Just wanted to "ship" them together
due the fact that all of them are needed to get rid of the errors.

Re: [PATCH 2/5] igc: Let the PCI core deal with the PM resume flow

[Mika Westerberg]

On Wed, Feb 25, 2026 at 01:26:19PM +0100, Mika Westerberg wrote:
> On Tue, Feb 24, 2026 at 10:58:37AM -0600, Bjorn Helgaas wrote:
> > On Tue, Feb 24, 2026 at 12:10:41PM +0100, Mika Westerberg wrote:
> > > Currently igc driver calls pci_set_power_state() and pci_restore_state()
> > > and the like to bring the device back from low power states. However,
> > > PCI core handles all this on behalf of the driver. Furthermore with PTM
> > > enabled the PCI core re-enables it on resume but the driver calls
> > > pci_restore_state() which ends up disabling it again.
> > > 
> > > For this reason let the PCI core handle the common PM resume flow.
> > > 
> > > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> > 
> > I love it, thanks a lot for doing this!
> > 
> > Do we still need the pci_enable_device_mem() and pci_set_master()
> > in __igc_resume()?
> >
> > I suppose some of that is related to the pci_disable_device() in the
> > suspend path (__igc_shutdown()), but there are only a few dozen
> > drivers that do this, so I'm not sure it's essential.
> 
> I think they are just as you describe due the fact there are explicit
> pci_disable_device() calls. Probably we can get rid of them as well but
> that requires careful testing that nothing accidentally breaks.
> 
> @Vitaly, what do you think? I can take a look.

Sorry @Vitaly and @Vinicius I meant :)

Re: [PATCH 2/5] igc: Let the PCI core deal with the PM resume flow

[Vinicius Costa Gomes]

Mika Westerberg <mika.westerberg@linux.intel.com> writes:

> On Tue, Feb 24, 2026 at 10:58:37AM -0600, Bjorn Helgaas wrote:
>> On Tue, Feb 24, 2026 at 12:10:41PM +0100, Mika Westerberg wrote:
>> > Currently igc driver calls pci_set_power_state() and pci_restore_state()
>> > and the like to bring the device back from low power states. However,
>> > PCI core handles all this on behalf of the driver. Furthermore with PTM
>> > enabled the PCI core re-enables it on resume but the driver calls
>> > pci_restore_state() which ends up disabling it again.
>> > 
>> > For this reason let the PCI core handle the common PM resume flow.
>> > 
>> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
>> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
>> 
>> I love it, thanks a lot for doing this!
>> 
>> Do we still need the pci_enable_device_mem() and pci_set_master()
>> in __igc_resume()?
>>
>> I suppose some of that is related to the pci_disable_device() in the
>> suspend path (__igc_shutdown()), but there are only a few dozen
>> drivers that do this, so I'm not sure it's essential.
>
> I think they are just as you describe due the fact there are explicit
> pci_disable_device() calls. Probably we can get rid of them as well but
> that requires careful testing that nothing accidentally breaks.
>

This series is solving real problems (thank you btw), I think the
pci_disable_device() one would be better as a separate series.


Cheers,
-- 
Vinicius

Re: [PATCH 2/5] igc: Let the PCI core deal with the PM resume flow

[Mika Westerberg]

On Wed, Feb 25, 2026 at 03:56:44PM -0800, Vinicius Costa Gomes wrote:
> Mika Westerberg <mika.westerberg@linux.intel.com> writes:
> 
> > On Tue, Feb 24, 2026 at 10:58:37AM -0600, Bjorn Helgaas wrote:
> >> On Tue, Feb 24, 2026 at 12:10:41PM +0100, Mika Westerberg wrote:
> >> > Currently igc driver calls pci_set_power_state() and pci_restore_state()
> >> > and the like to bring the device back from low power states. However,
> >> > PCI core handles all this on behalf of the driver. Furthermore with PTM
> >> > enabled the PCI core re-enables it on resume but the driver calls
> >> > pci_restore_state() which ends up disabling it again.
> >> > 
> >> > For this reason let the PCI core handle the common PM resume flow.
> >> > 
> >> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> >> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> >> 
> >> I love it, thanks a lot for doing this!
> >> 
> >> Do we still need the pci_enable_device_mem() and pci_set_master()
> >> in __igc_resume()?
> >>
> >> I suppose some of that is related to the pci_disable_device() in the
> >> suspend path (__igc_shutdown()), but there are only a few dozen
> >> drivers that do this, so I'm not sure it's essential.
> >
> > I think they are just as you describe due the fact there are explicit
> > pci_disable_device() calls. Probably we can get rid of them as well but
> > that requires careful testing that nothing accidentally breaks.
> >
> 
> This series is solving real problems (thank you btw), I think the
> pci_disable_device() one would be better as a separate series.

Okay works for me :)

Re: [Intel-wired-lan] [PATCH 1/5] igc: Call netif_queue_set_napi() with rntl locked

[Dahan, AvigailX]

On 24/02/2026 13:10, Mika Westerberg wrote:
> When runtime resuming igc we get:
> 
>    [  516.161666] RTNL: assertion failed at ./include/net/netdev_lock.h (72)
> 
> Happens because commit 310ae9eb2617 ("net: designate queue -> napi
> linking as "ops protected"") added check for this. For this reason drop
> the special case for runtime PM from __igc_resume(). This makes it take
> rtnl lock unconditionally.
> 
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> ---
>   drivers/net/ethernet/intel/igc/igc_main.c | 12 +++++-------
>   1 file changed, 5 insertions(+), 7 deletions(-)
> 

Tested-by: Avigail Dahan <avigailx.dahan@intel.com>

Re: [Intel-wired-lan] [PATCH 2/5] igc: Let the PCI core deal with the PM resume flow

[Dahan, AvigailX]

On 24/02/2026 13:10, Mika Westerberg wrote:
> Currently igc driver calls pci_set_power_state() and pci_restore_state()
> and the like to bring the device back from low power states. However,
> PCI core handles all this on behalf of the driver. Furthermore with PTM
> enabled the PCI core re-enables it on resume but the driver calls
> pci_restore_state() which ends up disabling it again.
> 
> For this reason let the PCI core handle the common PM resume flow.
> 
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> ---
>   drivers/net/ethernet/intel/igc/igc_main.c | 6 ------
>   1 file changed, 6 deletions(-)
> 

Tested-by: Avigail Dahan <avigailx.dahan@intel.com>

Re: [Intel-wired-lan] [PATCH 3/5] igc: Don't reset the hardware on suspend path

[Dahan, AvigailX]

On 24/02/2026 13:10, Mika Westerberg wrote:
> Commit c01163dbd1b8 ("PCI/PM: Always disable PTM for all devices during
> suspend") made the PCI core to suspend (disable) PTM before driver
> suspend hooks are called. In case of igc what happens is that on suspend
> path PCI core calls pci_suspend_ptm() then igc suspend hook that calls
> igc_down() that ends up calling igc_ptp_reset() (which according to the
> comment is actually needed for re-enabling the device). Anyways that
> function also poll IGC_PTM_STAT that will end up timing out because PTM
> is already disabled:
> 
>    [  160.716119] igc 0000:03:00.0 enp3s0: Timeout reading IGC_PTM_STAT register
> 
> There should be no reason resetting the hardware on suspend path so fix
> this by avoiding the reset.
> 
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> ---
>   drivers/net/ethernet/intel/igc/igc.h         |  2 +-
>   drivers/net/ethernet/intel/igc/igc_ethtool.c |  6 +++---
>   drivers/net/ethernet/intel/igc/igc_main.c    | 13 +++++++------
>   3 files changed, 11 insertions(+), 10 deletions(-)
> 

Tested-by: Avigail Dahan <avigailx.dahan@intel.com>

Re: [Intel-wired-lan] [PATCH 4/5] PCI/PTM: Drop granularity parameter from pci_enable_ptm()

[Dahan, AvigailX]

On 24/02/2026 13:10, Mika Westerberg wrote:
> This is not used by any of the existing callers so we can simplify the
> function slightly and get rid of that.
> 
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> ---
>   drivers/net/ethernet/intel/ice/ice_main.c      |  2 +-
>   drivers/net/ethernet/intel/idpf/idpf_main.c    |  2 +-
>   drivers/net/ethernet/intel/igc/igc_main.c      |  2 +-
>   drivers/net/ethernet/mellanox/mlx5/core/main.c |  2 +-
>   drivers/pci/pcie/ptm.c                         | 11 +++--------
>   include/linux/pci.h                            |  4 ++--
>   6 files changed, 9 insertions(+), 14 deletions(-)
> 

Tested-by: Avigail Dahan <avigailx.dahan@intel.com>

Re: [Intel-wired-lan] [PATCH 5/5] PCI/PTM: Do not enable PTM automatically for Root and Switch Upstream Ports

[Dahan, AvigailX]

On 24/02/2026 13:10, Mika Westerberg wrote:
> Currently we enable PTM automatically for Root and Switch Upstream Ports
> if the advertised capabilities support the relevant role. However, there
> are few issues with this. First of all if there is no Endpoint that
> actually needs the PTM functionality, this is just wasting link
> bandwidth. There are just a couple of drivers calling pci_ptm_enable()
> in the tree.
> 
> Secondly we do the enablement in pci_ptm_init() that is called pretty
> early for the Switch Upstream Port before Downstream Ports are even
> enumerated. Since the Upstream Port configuration affects the whole
> Switch enabling it this early might cause the PTM requests to be sent
> already. We actually do see effect of this:
> 
>    pcieport 0000:00:07.1: pciehp: Slot(6-1): Card present
>    pcieport 0000:00:07.1: pciehp: Slot(6-1): Link Up
>    pci 0000:2c:00.0: [8086:5786] type 01 class 0x060400 PCIe Switch Upstream Port
>    pci 0000:2c:00.0: PCI bridge to [bus 00]
>    pci 0000:2c:00.0:   bridge window [io  0x0000-0x0fff]
>    pci 0000:2c:00.0:   bridge window [mem 0x00000000-0x000fffff]
>    pci 0000:2c:00.0:   bridge window [mem 0x00000000-0x000fffff 64bit pref]
>    ...
>    pci 0000:2c:00.0: PME# supported from D0 D1 D2 D3hot D3cold
>    pci 0000:2c:00.0: PTM enabled, 4ns granularity
> 
> At this point we have only enumerated the Switch Upstream Port and now
> PTM got enabled which immediately triggers flood of these:
> 
>    pcieport 0000:00:07.1: AER: Multiple Uncorrectable (Non-Fatal) error message received from 0000:00:07.1
>    pcieport 0000:00:07.1: PCIe Bus Error: severity=Uncorrectable (Non-Fatal), type=Transaction Layer, (Receiver ID)
>    pcieport 0000:00:07.1:   device [8086:d44f] error status/mask=00200000/00000000
>    pcieport 0000:00:07.1:    [21] ACSViol                (First)
>    pcieport 0000:00:07.1: AER:   TLP Header: 0x34000000 0x00000052 0x00000000 0x00000000
>    pcieport 0000:00:07.1: AER: device recovery successful
>    pcieport 0000:00:07.1: AER: Uncorrectable (Non-Fatal) error message received from 0000:00:07.1
> 
> In the above TLP Header the Requester ID is 0 which rightfully triggers
> an error as we have ACS Source Validation enabled.
> 
> For this reason change the PTM enablement to happen at the time
> pci_enable_ptm() is called. It will try to enable PTM first for upstream
> devices before enabling for the Endpoint itself. For disable path we
> need to keep count of how many times PTM has been enabled and disable
> only on the last so change the dev->ptm_enabled to a counter (and rename
> it to dev->ptm_enable_cnt analogous to dev->pci_enable_cnt).
> 
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> ---
>   drivers/pci/pcie/ptm.c | 68 ++++++++++++++++++++++++------------------
>   include/linux/pci.h    |  2 +-
>   2 files changed, 40 insertions(+), 30 deletions(-)
> 

Tested-by: Avigail Dahan <avigailx.dahan@intel.com>