nvme/pci: Add quick suspend quirk for Toshiba drive.

[PATCH v1] nvme/pci: Add quick suspend quirk for Toshiba drive.

Posted by Nitin Rawat 4 years, 4 months ago

The Toshiba KBG40ZPZ256G SSD is having resume issues on SC7280 platform.
Hence enabling quick suspend quirks for Toshiba KBG40ZPZ256G on sc7280
platform so that nvme device is taken through shutdown path
during resume.

No issue is seen after enabling this quirks.

Signed-off-by: Nitin Rawat <quic_nitirawa@quicinc.com>
Signed-off-by: Shaik Sajida Bhanu <quic_c_sbhanu@quicinc.com>
---
 drivers/nvme/host/pci.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 6a99ed6..04c5954 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -3032,6 +3032,15 @@ 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;
+
+		/*
+		 * Toshiba KBG40ZPZ256G on Sc7280 platform is having NVMe
+		 * Resume issue. Appending quick suspend quirks for sc7280
+		 * platforms so that full NVMe device shutdown path is
+		 * executed during resume.
+		 */
+		if (of_machine_is_compatible("qcom,sc7280"))
+			return NVME_QUIRK_SIMPLE_SUSPEND;
 	}

 	return 0;
--
2.7.4

Re: [PATCH v1] nvme/pci: Add quick suspend quirk for Toshiba drive.

Posted by Keith Busch 4 years, 4 months ago

On Thu, Feb 10, 2022 at 05:51:16PM +0530, Nitin Rawat wrote:
> The Toshiba KBG40ZPZ256G SSD is having resume issues on SC7280 platform.
> Hence enabling quick suspend quirks for Toshiba KBG40ZPZ256G on sc7280
> platform so that nvme device is taken through shutdown path
> during resume.
> 
> No issue is seen after enabling this quirks.
> 
> Signed-off-by: Nitin Rawat <quic_nitirawa@quicinc.com>
> Signed-off-by: Shaik Sajida Bhanu <quic_c_sbhanu@quicinc.com>
> ---
>  drivers/nvme/host/pci.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
> index 6a99ed6..04c5954 100644
> --- a/drivers/nvme/host/pci.c
> +++ b/drivers/nvme/host/pci.c
> @@ -3032,6 +3032,15 @@ 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;
> +
> +		/*
> +		 * Toshiba KBG40ZPZ256G on Sc7280 platform is having NVMe
> +		 * Resume issue. Appending quick suspend quirks for sc7280
> +		 * platforms so that full NVMe device shutdown path is
> +		 * executed during resume.
> +		 */
> +		if (of_machine_is_compatible("qcom,sc7280"))
> +			return NVME_QUIRK_SIMPLE_SUSPEND;

Shouldn't this check be moved outside the vendor/device check? It
doesn't seem like this behavior for this platform is specific to any
particular controller, right?

Otherwise, looks fine.

RE: [PATCH v1] nvme/pci: Add quick suspend quirk for Toshiba drive.

Posted by Nitin Rawat (QUIC) 4 years, 4 months ago

Thanks keith for review.
Yes moved the condition check outside vendor/device check in Patchset 2.

-----Original Message-----
From: Keith Busch <kbusch@kernel.org> 
Sent: Thursday, February 10, 2022 8:31 PM
To: Nitin Rawat (QUIC) <quic_nitirawa@quicinc.com>
Cc: Jens Axboe <axboe@fb.com>; Sagi Grimberg <sagi@grimberg.me>; linux-nvme@lists.infradead.org; linux-kernel@vger.kernel.org; Sajida Bhanu (Temp) (QUIC) <quic_c_sbhanu@quicinc.com>
Subject: Re: [PATCH v1] nvme/pci: Add quick suspend quirk for Toshiba drive.

On Thu, Feb 10, 2022 at 05:51:16PM +0530, Nitin Rawat wrote:
> The Toshiba KBG40ZPZ256G SSD is having resume issues on SC7280 platform.
> Hence enabling quick suspend quirks for Toshiba KBG40ZPZ256G on sc7280 
> platform so that nvme device is taken through shutdown path during 
> resume.
> 
> No issue is seen after enabling this quirks.
> 
> Signed-off-by: Nitin Rawat <quic_nitirawa@quicinc.com>
> Signed-off-by: Shaik Sajida Bhanu <quic_c_sbhanu@quicinc.com>
> ---
>  drivers/nvme/host/pci.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c index 
> 6a99ed6..04c5954 100644
> --- a/drivers/nvme/host/pci.c
> +++ b/drivers/nvme/host/pci.c
> @@ -3032,6 +3032,15 @@ 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;
> +
> +		/*
> +		 * Toshiba KBG40ZPZ256G on Sc7280 platform is having NVMe
> +		 * Resume issue. Appending quick suspend quirks for sc7280
> +		 * platforms so that full NVMe device shutdown path is
> +		 * executed during resume.
> +		 */
> +		if (of_machine_is_compatible("qcom,sc7280"))
> +			return NVME_QUIRK_SIMPLE_SUSPEND;

Shouldn't this check be moved outside the vendor/device check? It doesn't seem like this behavior for this platform is specific to any particular controller, right?

Otherwise, looks fine.