Re: [PATCH v2] PCI: Explicitly put devices into D0 when initializing

[Mario Limonciello <superm1@kernel.org>]

On 4/24/2025 10:07 AM, Wysocki, Rafael J wrote:
> On 4/24/2025 6:31 AM, Mario Limonciello wrote:
>> From: Mario Limonciello <mario.limonciello@amd.com>
>>
>> AMD BIOS team has root caused an issue that NVME storage failed to come
>> back from suspend to a lack of a call to _REG when NVME device was 
>> probed.
>>
>> commit 112a7f9c8edbf ("PCI/ACPI: Call _REG when transitioning D-states")
>> added support for calling _REG when transitioning D-states, but this only
>> works if the device actually "transitions" D-states.
>>
>> commit 967577b062417 ("PCI/PM: Keep runtime PM enabled for unbound PCI
>> devices") added support for runtime PM on PCI devices, but never actually
>> 'explicitly' sets the device to D0.
>>
>> To make sure that devices are in D0 and that platform methods such as
>> _REG are called, explicitly set all devices into D0 during 
>> initialization.
>>
>> Fixes: 967577b062417 ("PCI/PM: Keep runtime PM enabled for unbound PCI 
>> devices")
>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>> ---
>> v2:
>>   * Move runtime PM calls after setting to D0
>>   * Use pci_pm_power_up_and_verify_state()
> 
> You could give me a credit for this suggestion.

Sorry about that, absolutely!  I believe b4 can still pick it up.

Suggested-by: Rafael J. Wysocki <rafael@kernel.org>

> 
> Anyways
> 
> Reviewed-by: Rafael J. Wysocki <rafael@kernel.org>
> 
> 
>> ---
>>   drivers/pci/pci-driver.c |  6 ------
>>   drivers/pci/pci.c        | 13 ++++++++++---
>>   drivers/pci/pci.h        |  1 +
>>   3 files changed, 11 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
>> index c8bd71a739f72..082918ce03d8a 100644
>> --- a/drivers/pci/pci-driver.c
>> +++ b/drivers/pci/pci-driver.c
>> @@ -555,12 +555,6 @@ static void pci_pm_default_resume(struct pci_dev 
>> *pci_dev)
>>       pci_enable_wake(pci_dev, PCI_D0, false);
>>   }
>> -static void pci_pm_power_up_and_verify_state(struct pci_dev *pci_dev)
>> -{
>> -    pci_power_up(pci_dev);
>> -    pci_update_current_state(pci_dev, PCI_D0);
>> -}
>> -
>>   static void pci_pm_default_resume_early(struct pci_dev *pci_dev)
>>   {
>>       pci_pm_power_up_and_verify_state(pci_dev);
>> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
>> index e77d5b53c0cec..8d125998b30b7 100644
>> --- a/drivers/pci/pci.c
>> +++ b/drivers/pci/pci.c
>> @@ -3192,6 +3192,12 @@ void pci_d3cold_disable(struct pci_dev *dev)
>>   }
>>   EXPORT_SYMBOL_GPL(pci_d3cold_disable);
>> +void pci_pm_power_up_and_verify_state(struct pci_dev *pci_dev)
>> +{
>> +    pci_power_up(pci_dev);
>> +    pci_update_current_state(pci_dev, PCI_D0);
>> +}
>> +
>>   /**
>>    * pci_pm_init - Initialize PM functions of given PCI device
>>    * @dev: PCI device to handle.
>> @@ -3202,9 +3208,6 @@ void pci_pm_init(struct pci_dev *dev)
>>       u16 status;
>>       u16 pmc;
>> -    pm_runtime_forbid(&dev->dev);
>> -    pm_runtime_set_active(&dev->dev);
>> -    pm_runtime_enable(&dev->dev);
>>       device_enable_async_suspend(&dev->dev);
>>       dev->wakeup_prepared = false;
>> @@ -3266,6 +3269,10 @@ void pci_pm_init(struct pci_dev *dev)
>>       pci_read_config_word(dev, PCI_STATUS, &status);
>>       if (status & PCI_STATUS_IMM_READY)
>>           dev->imm_ready = 1;
>> +    pci_pm_power_up_and_verify_state(dev);
>> +    pm_runtime_forbid(&dev->dev);
>> +    pm_runtime_set_active(&dev->dev);
>> +    pm_runtime_enable(&dev->dev);
>>   }
>>   static unsigned long pci_ea_flags(struct pci_dev *dev, u8 prop)
>> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
>> index b81e99cd4b62a..49165b739138b 100644
>> --- a/drivers/pci/pci.h
>> +++ b/drivers/pci/pci.h
>> @@ -148,6 +148,7 @@ void pci_dev_adjust_pme(struct pci_dev *dev);
>>   void pci_dev_complete_resume(struct pci_dev *pci_dev);
>>   void pci_config_pm_runtime_get(struct pci_dev *dev);
>>   void pci_config_pm_runtime_put(struct pci_dev *dev);
>> +void pci_pm_power_up_and_verify_state(struct pci_dev *pci_dev);
>>   void pci_pm_init(struct pci_dev *dev);
>>   void pci_ea_init(struct pci_dev *dev);
>>   void pci_msi_init(struct pci_dev *dev);