Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:3 as permitted sender) client-ip=2620:137:e000::3:3;
Message-ID: <12d32e8e-e20b-4023-8299-39d2a69c0cd9@tuxedocomputers.com>
Date:   Tue, 5 Dec 2023 18:17:25 +0100
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] nvme-pci: Add sleep quirk for Kingston drives
Content-Language: en-US
To:     Keith Busch <kbusch@kernel.org>, Jens Axboe <axboe@kernel.dk>,
        Christoph Hellwig <hch@lst.de>,
        Sagi Grimberg <sagi@grimberg.me>
Cc:     Georg Gottleuber <ggo@tuxedocomputers.com>,
        linux-nvme@lists.infradead.org, linux-kernel@vger.kernel.org
References: <20230920085210.22573-1-wse@tuxedocomputers.com>
From:   Werner Sembach <wse@tuxedocomputers.com>
In-Reply-To: <20230920085210.22573-1-wse@tuxedocomputers.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

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.