On Thu, Jun 03, 2021 at 02:07:53PM -0500, Mario Limonciello wrote: > Although first implemented for NVME, this check may be usable by > other drivers as well. Microsoft's specification explicitly mentions > that is may be usable by SATA and AHCI devices. Google also indicates > that they have used this with SDHCI in a downstream kernel tree that > a user can plug a storage device into. > > Link: Link: https://docs.microsoft.com/en-us/windows-hardware/design/component-guidelines/power-management-for-storage-hardware-devices-intro > Suggested-by: Keith Busch <kbusch@xxxxxxxxxx> > CC: rrangel@xxxxxxxxxxxx > CC: david.e.box@xxxxxxxxxxxxxxx > CC: Shyam-sundar S-k <Shyam-sundar.S-k@xxxxxxx> > CC: Alexander Deucher <Alexander.Deucher@xxxxxxx> > CC: Rafael J. Wysocki <rjw@xxxxxxxxxxxxx> > CC: Prike Liang <prike.liang@xxxxxxx> > Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx> > --- > drivers/acpi/device_pm.c | 25 +++++++++++++++++++++++++ > drivers/nvme/host/pci.c | 28 +--------------------------- > include/linux/acpi.h | 5 +++++ > 3 files changed, 31 insertions(+), 27 deletions(-) > > Changes from v3->v4 > * Rebase on nvme-5.14 (w/ patch 1/2 from v3 of series accepted) > * Adjust commit message per Christoph's suggestions > * Adjust documentation per Christoph's suggestions > > diff --git a/drivers/acpi/device_pm.c b/drivers/acpi/device_pm.c > index d260bc1f3e6e..1edb68d00b8e 100644 > --- a/drivers/acpi/device_pm.c > +++ b/drivers/acpi/device_pm.c > @@ -1340,4 +1340,29 @@ int acpi_dev_pm_attach(struct device *dev, bool power_on) > return 1; > } > EXPORT_SYMBOL_GPL(acpi_dev_pm_attach); > + > +/** > + * acpi_storage_d3 - Check if a storage device should use D3. > + * @dev: Device to check > + * > + * Returns %true if @dev should be put into D3 when the ->suspend method is > + * called, else %false. The name of this function is somewhat misleading > + * as it has nothing to do with storage except for the name of the ACPI > + * property. On some platforms resume will not work if this hint is ignored. > + * > + */ > +bool acpi_storage_d3(struct device *dev) > +{ > + struct acpi_device *adev = ACPI_COMPANION(dev); > + u8 val; > + > + if (!adev) > + return false; > + if (fwnode_property_read_u8(acpi_fwnode_handle(adev), "StorageD3Enable", > + &val)) > + return false; > + return val == 1; > +} > +EXPORT_SYMBOL_GPL(acpi_storage_d3); > + > #endif /* CONFIG_PM */ > diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c > index 3aa7245a505f..8fbc4c87a0d8 100644 > --- a/drivers/nvme/host/pci.c > +++ b/drivers/nvme/host/pci.c > @@ -2828,32 +2828,6 @@ static unsigned long check_vendor_combination_bug(struct pci_dev *pdev) > return 0; > } > > -#ifdef CONFIG_ACPI > -static bool nvme_acpi_storage_d3(struct pci_dev *dev) > -{ > - struct acpi_device *adev = ACPI_COMPANION(&dev->dev); > - u8 val; > - > - /* > - * Look for _DSD property specifying that the storage device on the port > - * must use D3 to support deep platform power savings during > - * suspend-to-idle. > - */ > - > - if (!adev) > - return false; > - if (fwnode_property_read_u8(acpi_fwnode_handle(adev), "StorageD3Enable", > - &val)) > - return false; > - return val == 1; > -} > -#else > -static inline bool nvme_acpi_storage_d3(struct pci_dev *dev) > -{ > - return false; > -} > -#endif /* CONFIG_ACPI */ > - > static void nvme_async_probe(void *data, async_cookie_t cookie) > { > struct nvme_dev *dev = data; > @@ -2903,7 +2877,7 @@ static int nvme_probe(struct pci_dev *pdev, const struct pci_device_id *id) > > quirks |= check_vendor_combination_bug(pdev); > > - if (!noacpi && nvme_acpi_storage_d3(pdev)) { > + if (!noacpi && acpi_storage_d3(&pdev->dev)) { > /* > * Some systems use a bios work around to ask for D3 on > * platforms that support kernel managed suspend. > diff --git a/include/linux/acpi.h b/include/linux/acpi.h > index c60745f657e9..dd0dafd21e33 100644 > --- a/include/linux/acpi.h > +++ b/include/linux/acpi.h > @@ -1004,6 +1004,7 @@ int acpi_dev_resume(struct device *dev); > int acpi_subsys_runtime_suspend(struct device *dev); > int acpi_subsys_runtime_resume(struct device *dev); > int acpi_dev_pm_attach(struct device *dev, bool power_on); > +bool acpi_storage_d3(struct device *dev); > #else > static inline int acpi_subsys_runtime_suspend(struct device *dev) { return 0; } > static inline int acpi_subsys_runtime_resume(struct device *dev) { return 0; } > @@ -1011,6 +1012,10 @@ static inline int acpi_dev_pm_attach(struct device *dev, bool power_on) > { > return 0; > } > +static inline bool acpi_storage_d3(struct device *dev) > +{ > + return false; > +} > #endif > > #if defined(CONFIG_ACPI) && defined(CONFIG_PM_SLEEP) Tested on an AMD Chromebook and it still works. Reviewed-by: Raul E Rangel <rrangel@xxxxxxxxxxxx>
