[PATCH v2] nvme: Look for StorageD3Enable on companion ACPI device instead

Christoph Hellwig hch at lst.de
Thu May 27 10:28:58 PDT 2021


[adding Rafael]

On Thu, May 27, 2021 at 09:58:44AM -0700, Keith Busch wrote:
>  EXPORT_SYMBOL_GPL(acpi_dev_pm_attach);
> +
> +bool acpi_storage_d3(struct device *dev)

This need a really good kerneldoc comment.  Not like pci_pr3_present
which somehow made it into the PCI core, but has not explanation
whatsoever..

> +{
> +	struct acpi_device *adev;
> +	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.
> +	 */

This is part of what should got into the kerneldoc

> +	adev = ACPI_COMPANION(dev);
> +	if (!adev)
> +		return false;
> +	if (fwnode_property_read_u8(acpi_fwnode_handle(adev), "StorageD3Enable",
> +			&val))
> +		return false;
> +	return val == 1;
> +}

adev could be initialized at declaration time.

Also any chance we could have acpi_fwnode_property_read_* helpers
that include the NULL check and the acpi_fwnode_handle conversion which
seems to be boilerplated all over?

Also I wonder if having this only in ACPI is a good idea.  What when
we get the same thing in OF?  Isn't this something that the PM layer
should tell the device when calling ->suspend?



More information about the Linux-nvme mailing list