Re: [PATCH net-next 02/15] igc: Let the PCI core deal with the PM resume flow

[Bjorn Helgaas <helgaas@kernel.org>]

On Mon, Mar 30, 2026 at 04:02:31PM -0700, Tony Nguyen wrote:
> From: Mika Westerberg <mika.westerberg@linux.intel.com>
> 
> 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>
> Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
> Tested-by: Avigail Dahan <avigailx.dahan@intel.com>
> Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>

I love it, thanks a lot for doing this!

> ---
>  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 98024b46789f..0e785af0a3a3 100644
> --- a/drivers/net/ethernet/intel/igc/igc_main.c
> +++ b/drivers/net/ethernet/intel/igc/igc_main.c
> @@ -7544,9 +7544,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);

__igc_shutdown() calls pci_disable_device() in the suspend paths,
which disables bus mastering.  There are only a relative handful of
drivers that do this, mostly for NICs, so I'm not sure if it's really
necessary or if it's been cargo-culted.

Here in the resume path we call pci_enable_device_mem(),
pci_set_master(), and conditionally call pci_disable_link_state().  I
suspect all those are unnecessary because pci_pm_resume() should have
called pci_restore_state() before we get here.

> @@ -7556,9 +7553,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.47.1
>