linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] nvme: Add quirk for LiteON CL1 devices running FW 22301111
@ 2019-08-14 20:08 Mario Limonciello
  2019-08-14 20:08 ` Keith Busch
  2019-08-14 20:14 ` Sagi Grimberg
  0 siblings, 2 replies; 8+ messages in thread
From: Mario Limonciello @ 2019-08-14 20:08 UTC (permalink / raw)
  To: Keith Busch
  Cc: Jens Axboe, Christoph Hellwig, Sagi Grimberg, linux-nvme, LKML,
	Ryan Hong, Crag Wang, sjg, Charles Hyde, Jared Dominguez,
	Mario Limonciello

One of the components in LiteON CL1 device has limitations that
can be encountered based upon boundary race conditions using the
nvme bus specific suspend to idle flow.

When this situation occurs the drive doesn't resume properly from
suspend-to-idle.

LiteON has confirmed this problem and fixed in the next firmware
version.  As this firmware is already in the field, avoid running
nvme specific suspend to idle flow.

Fixes: d916b1be94b6 ("nvme-pci: use host managed power state for suspend")
Link: http://lists.infradead.org/pipermail/linux-nvme/2019-July/thread.html
Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
Signed-off-by: Charles Hyde <charles.hyde@dellteam.com>
---
 drivers/nvme/host/core.c | 10 ++++++++++
 drivers/nvme/host/nvme.h |  5 +++++
 drivers/nvme/host/pci.c  |  3 ++-
 3 files changed, 17 insertions(+), 1 deletion(-)

This patch is the spiritual successor to the previously submitted
patch "[PATCH] drivers/nvme: save/restore HMB on suspend/resume".

After discussion with LiteON, they agreed to resolve the issue
in their next firmware release.

changes from v1:
 * Group all 3 possible CL1 strings together
 * Remove the resume code because it's
   already implied by ndev->last_ps = U32_MAX
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 8f3fbe5..02770d6 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -2251,6 +2251,16 @@ static const struct nvme_core_quirk_entry core_quirks[] = {
 		.vid = 0x1179,
 		.mn = "THNSF5256GPUK TOSHIBA",
 		.quirks = NVME_QUIRK_NO_APST,
+	},
+	/*
+	 * This LiteON CL1-3D*-Q11 firmware version has a race condition
+	 * associated with actions related to suspend to idle.  LiteON has
+	 * resolved the problem in future firmware.
+	 */
+	{
+		.vid = 0x14a4,
+		.fr = "22301111",
+		.quirks = NVME_QUIRK_SIMPLE_SUSPEND,
 	}
 };
 
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 26b563f..fe1ca0d 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -92,6 +92,11 @@ enum nvme_quirks {
 	 * Broken Write Zeroes.
 	 */
 	NVME_QUIRK_DISABLE_WRITE_ZEROES		= (1 << 9),
+
+	/*
+	 * Force simple suspend/resume path.
+	 */
+	NVME_QUIRK_SIMPLE_SUSPEND		= (1 << 10),
 };
 
 /*
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 108e109..b366f54 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -2875,7 +2875,8 @@ static int nvme_suspend(struct device *dev)
 	 * state (which may not be possible if the link is up).
 	 */
 	if (pm_suspend_via_firmware() || !ctrl->npss ||
-	    !pcie_aspm_enabled(pdev)) {
+	    !pcie_aspm_enabled(pdev) ||
+	    (ndev->ctrl.quirks & NVME_QUIRK_SIMPLE_SUSPEND)) {
 		nvme_dev_disable(ndev, true);
 		return 0;
 	}
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] nvme: Add quirk for LiteON CL1 devices running FW 22301111
  2019-08-14 20:08 [PATCH v2] nvme: Add quirk for LiteON CL1 devices running FW 22301111 Mario Limonciello
@ 2019-08-14 20:08 ` Keith Busch
  2019-08-14 20:14 ` Sagi Grimberg
  1 sibling, 0 replies; 8+ messages in thread
From: Keith Busch @ 2019-08-14 20:08 UTC (permalink / raw)
  To: Mario Limonciello
  Cc: Jens Axboe, Christoph Hellwig, Sagi Grimberg, linux-nvme, LKML,
	Ryan Hong, Crag Wang, sjg, Charles Hyde, Jared Dominguez

On Wed, Aug 14, 2019 at 01:08:24PM -0700, Mario Limonciello wrote:
> One of the components in LiteON CL1 device has limitations that
> can be encountered based upon boundary race conditions using the
> nvme bus specific suspend to idle flow.
> 
> When this situation occurs the drive doesn't resume properly from
> suspend-to-idle.
> 
> LiteON has confirmed this problem and fixed in the next firmware
> version.  As this firmware is already in the field, avoid running
> nvme specific suspend to idle flow.
> 
> Fixes: d916b1be94b6 ("nvme-pci: use host managed power state for suspend")
> Link: http://lists.infradead.org/pipermail/linux-nvme/2019-July/thread.html
> Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>
> Signed-off-by: Charles Hyde <charles.hyde@dellteam.com>

Looks fine to me.

Reviewed-by: Keith Busch <kbusch@kernel.org>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] nvme: Add quirk for LiteON CL1 devices running FW 22301111
  2019-08-14 20:08 [PATCH v2] nvme: Add quirk for LiteON CL1 devices running FW 22301111 Mario Limonciello
  2019-08-14 20:08 ` Keith Busch
@ 2019-08-14 20:14 ` Sagi Grimberg
  2019-08-14 20:19   ` Keith Busch
  2019-08-14 20:19   ` Mario.Limonciello
  1 sibling, 2 replies; 8+ messages in thread
From: Sagi Grimberg @ 2019-08-14 20:14 UTC (permalink / raw)
  To: Mario Limonciello, Keith Busch
  Cc: Jens Axboe, Christoph Hellwig, linux-nvme, LKML, Ryan Hong,
	Crag Wang, sjg, Charles Hyde, Jared Dominguez

Mario,

Can you please respin a patch that applies cleanly on nvme-5.4?

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] nvme: Add quirk for LiteON CL1 devices running FW 22301111
  2019-08-14 20:14 ` Sagi Grimberg
@ 2019-08-14 20:19   ` Keith Busch
  2019-08-15 17:19     ` Sagi Grimberg
  2019-08-14 20:19   ` Mario.Limonciello
  1 sibling, 1 reply; 8+ messages in thread
From: Keith Busch @ 2019-08-14 20:19 UTC (permalink / raw)
  To: Sagi Grimberg
  Cc: Mario Limonciello, Jens Axboe, Christoph Hellwig, linux-nvme,
	LKML, Ryan Hong, Crag Wang, sjg, Charles Hyde, Jared Dominguez

On Wed, Aug 14, 2019 at 01:14:50PM -0700, Sagi Grimberg wrote:
> Mario,
> 
> Can you please respin a patch that applies cleanly on nvme-5.4?

This fixes a regression we introduced in 5.3, so it should go in
5.3-rc. For this to apply cleanly, though, we'll need to resync to Linus'
tree to get Rafael's PCIe ASPM check after he sends his linux-pm pull
request.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH v2] nvme: Add quirk for LiteON CL1 devices running FW 22301111
  2019-08-14 20:14 ` Sagi Grimberg
  2019-08-14 20:19   ` Keith Busch
@ 2019-08-14 20:19   ` Mario.Limonciello
  1 sibling, 0 replies; 8+ messages in thread
From: Mario.Limonciello @ 2019-08-14 20:19 UTC (permalink / raw)
  To: sagi, kbusch
  Cc: axboe, hch, linux-nvme, linux-kernel, Ryan.Hong, Crag.Wang, sjg,
	Charles.Hyde, Jared.Dominguez

> -----Original Message-----
> From: Sagi Grimberg <sagi@grimberg.me>
> Sent: Wednesday, August 14, 2019 3:15 PM
> To: Limonciello, Mario; Keith Busch
> Cc: Jens Axboe; Christoph Hellwig; linux-nvme@lists.infradead.org; LKML; Hong,
> Ryan; Wang, Crag; sjg@google.com; Hyde, Charles - Dell Team; Dominguez, Jared
> Subject: Re: [PATCH v2] nvme: Add quirk for LiteON CL1 devices running FW
> 22301111
> 
> 
> [EXTERNAL EMAIL]
> 
> Mario,
> 
> Can you please respin a patch that applies cleanly on nvme-5.4?

It looks like nvme-5.4 is missing the commit coming in from Rafael's linux-next tree that it's dependent upon:
https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/commit/drivers/nvme/host?h=linux-next&id=4eaefe8c621c6195c91044396ed8060c179f7aae

Could you cherry pick that into nvme-5.4 as well?  It should apply cleanly then.  Otherwise, would you prefer
this to come in Rafael's tree too?

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] nvme: Add quirk for LiteON CL1 devices running FW 22301111
  2019-08-14 20:19   ` Keith Busch
@ 2019-08-15 17:19     ` Sagi Grimberg
  2019-08-16 19:43       ` Mario.Limonciello
  0 siblings, 1 reply; 8+ messages in thread
From: Sagi Grimberg @ 2019-08-15 17:19 UTC (permalink / raw)
  To: Keith Busch
  Cc: Mario Limonciello, Jens Axboe, Christoph Hellwig, linux-nvme,
	LKML, Ryan Hong, Crag Wang, sjg, Charles Hyde, Jared Dominguez


>> Mario,
>>
>> Can you please respin a patch that applies cleanly on nvme-5.4?
> 
> This fixes a regression we introduced in 5.3, so it should go in
> 5.3-rc. For this to apply cleanly, though, we'll need to resync to Linus'
> tree to get Rafael's PCIe ASPM check after he sends his linux-pm pull
> request.

We need to coordinate with Jens, don't think its a good idea if I'll
just randomly get stuff from linus' tree and send an rc pull request.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH v2] nvme: Add quirk for LiteON CL1 devices running FW 22301111
  2019-08-15 17:19     ` Sagi Grimberg
@ 2019-08-16 19:43       ` Mario.Limonciello
  2019-08-16 20:01         ` Keith Busch
  0 siblings, 1 reply; 8+ messages in thread
From: Mario.Limonciello @ 2019-08-16 19:43 UTC (permalink / raw)
  To: sagi, kbusch
  Cc: axboe, hch, linux-nvme, linux-kernel, Ryan.Hong, Crag.Wang, sjg,
	Charles.Hyde, Jared.Dominguez

> -----Original Message-----
> From: Sagi Grimberg <sagi@grimberg.me>
> Sent: Thursday, August 15, 2019 12:19 PM
> To: Keith Busch
> Cc: Limonciello, Mario; Jens Axboe; Christoph Hellwig; linux-
> nvme@lists.infradead.org; LKML; Hong, Ryan; Wang, Crag; sjg@google.com;
> Hyde, Charles - Dell Team; Dominguez, Jared
> Subject: Re: [PATCH v2] nvme: Add quirk for LiteON CL1 devices running FW
> 22301111
> 
> 
> [EXTERNAL EMAIL]
> 
> 
> >> Mario,
> >>
> >> Can you please respin a patch that applies cleanly on nvme-5.4?
> >
> > This fixes a regression we introduced in 5.3, so it should go in
> > 5.3-rc. For this to apply cleanly, though, we'll need to resync to Linus'
> > tree to get Rafael's PCIe ASPM check after he sends his linux-pm pull
> > request.
> 
> We need to coordinate with Jens, don't think its a good idea if I'll
> just randomly get stuff from linus' tree and send an rc pull request.

The dependent commit is in Linus' tree now.
4eaefe8c621c6195c91044396ed8060c179f7aae

Also it was reported to me after this was submitted that the comment
whitespace should have been aligned during the switchover from v1 to v2.

V1 the whitespace was further left since it was applying to 3 drives, but now
that they're combined in v2 the whitespace wasn't adjusted.

Let me know if you want me to resubmit v3 w/ whitespace modification or
if you will want to adjust that locally when you pull it in.


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] nvme: Add quirk for LiteON CL1 devices running FW 22301111
  2019-08-16 19:43       ` Mario.Limonciello
@ 2019-08-16 20:01         ` Keith Busch
  0 siblings, 0 replies; 8+ messages in thread
From: Keith Busch @ 2019-08-16 20:01 UTC (permalink / raw)
  To: Mario.Limonciello
  Cc: sagi, axboe, hch, linux-nvme, linux-kernel, Ryan.Hong, Crag.Wang,
	sjg, Charles.Hyde, Jared.Dominguez

On Fri, Aug 16, 2019 at 12:43:02PM -0700, Mario.Limonciello@dell.com wrote:
> > We need to coordinate with Jens, don't think its a good idea if I'll
> > just randomly get stuff from linus' tree and send an rc pull request.
> 
> The dependent commit is in Linus' tree now.
> 4eaefe8c621c6195c91044396ed8060c179f7aae
> 
> Also it was reported to me after this was submitted that the comment
> whitespace should have been aligned during the switchover from v1 to v2.
> 
> V1 the whitespace was further left since it was applying to 3 drives, but now
> that they're combined in v2 the whitespace wasn't adjusted.
> 
> Let me know if you want me to resubmit v3 w/ whitespace modification or
> if you will want to adjust that locally when you pull it in.

Go ahead and resend the patch with your fixes included. This is going to
have to wait till next week anyway in order to have a clean pull request
through our normal path to mainline.

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2019-08-16 20:04 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-14 20:08 [PATCH v2] nvme: Add quirk for LiteON CL1 devices running FW 22301111 Mario Limonciello
2019-08-14 20:08 ` Keith Busch
2019-08-14 20:14 ` Sagi Grimberg
2019-08-14 20:19   ` Keith Busch
2019-08-15 17:19     ` Sagi Grimberg
2019-08-16 19:43       ` Mario.Limonciello
2019-08-16 20:01         ` Keith Busch
2019-08-14 20:19   ` Mario.Limonciello

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).