Re: [PATCH] nvme-pci: Add sleep quirk for Kingston drives

Werner Sembach
2023-12-05T17:17:35Z

Hi,

Am 20.09.23 um 10:52 schrieb Werner Sembach:
> From: Georg Gottleuber <ggo@tuxedocomputers.com>
>
> Some Kingston NV1 and A2000 are wasting a lot of power on specific TUXEDO
> platforms in s2idle sleep if 'Simple Suspend' is used.
>
> This patch applies a new quirk 'Force No Simple Suspend' to achieve a
> low power sleep without 'Simple Suspend'.
>
> Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
> Signed-off-by: Georg Gottleuber <ggo@tuxedocomputers.com>
> Cc: <stable@vger.kernel.org>

Want to leave a gentle bump here. Any issues with this patch?

Kind regards,

Werner Sembach

> ---
> drivers/nvme/host/nvme.h | 5 +++++
> drivers/nvme/host/pci.c | 16 +++++++++++++++-
> 2 files changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
> index f35647c470afa..74f74b459f5fa 100644
> --- a/drivers/nvme/host/nvme.h
> +++ b/drivers/nvme/host/nvme.h
> @@ -156,6 +156,11 @@ enum nvme_quirks {
> * No temperature thresholds for channels other than 0 (Composite).
> */
> NVME_QUIRK_NO_SECONDARY_TEMP_THRESH = (1 << 19),
> +
> + /*
> + * Disables simple suspend/resume path.
> + */
> + NVME_QUIRK_FORCE_NO_SIMPLE_SUSPEND = (1 << 20),
> };
>
> /*
> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
> index 347cb5daebc3c..b4e6f0d21a44c 100644
> --- a/drivers/nvme/host/pci.c
> +++ b/drivers/nvme/host/pci.c
> @@ -2903,6 +2903,18 @@ static unsigned long check_vendor_combination_bug(struct pci_dev *pdev)
> if ((dmi_match(DMI_BOARD_VENDOR, "LENOVO")) &&
> dmi_match(DMI_BOARD_NAME, "LNVNB161216"))
> return NVME_QUIRK_SIMPLE_SUSPEND;
> + } else if (pdev->vendor == 0x2646 && (pdev->device == 0x2263 ||
> + pdev->device == 0x500f)) {
> + /*
> + * Exclude some Kingston NV1 and A2000 devices from
> + * NVME_QUIRK_SIMPLE_SUSPEND. Do a full suspend to save a
> + * lot fo energy with s2idle sleep on some TUXEDO platforms.
> + */
> + if (dmi_match(DMI_BOARD_NAME, "NS5X_NS7XAU") ||
> + dmi_match(DMI_BOARD_NAME, "NS5x_7xAU") ||
> + dmi_match(DMI_BOARD_NAME, "NS5x_7xPU") ||
> + dmi_match(DMI_BOARD_NAME, "PH4PRX1_PH6PRX1"))
> + return NVME_QUIRK_FORCE_NO_SIMPLE_SUSPEND;
> }
>
> return 0;
> @@ -2933,7 +2945,9 @@ static struct nvme_dev *nvme_pci_alloc_dev(struct pci_dev *pdev,
> dev->dev = get_device(&pdev->dev);
>
> quirks |= check_vendor_combination_bug(pdev);
> - if (!noacpi && acpi_storage_d3(&pdev->dev)) {
> + if (!noacpi &&
> + !(quirks & NVME_QUIRK_FORCE_NO_SIMPLE_SUSPEND) &&
> + acpi_storage_d3(&pdev->dev)) {
> /*
> * Some systems use a bios work around to ask for D3 on
> * platforms that support kernel managed suspend.