Module Name:    src
Committed By:   nonaka
Date:           Wed Apr 18 10:05:59 UTC 2018

Modified Files:
        src/sys/dev/ic: nvme.c nvmevar.h
        src/sys/dev/pci: nvme_pci.c

Log Message:
nvme(4): Added some delay before check RDY bit quirk when disabling device.

Pick from FreeBSD nvme(4) r326937.


To generate a diff of this commit:
cvs rdiff -u -r1.37 -r1.38 src/sys/dev/ic/nvme.c
cvs rdiff -u -r1.15 -r1.16 src/sys/dev/ic/nvmevar.h
cvs rdiff -u -r1.19 -r1.20 src/sys/dev/pci/nvme_pci.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/dev/ic/nvme.c
diff -u src/sys/dev/ic/nvme.c:1.37 src/sys/dev/ic/nvme.c:1.38
--- src/sys/dev/ic/nvme.c:1.37	Sat Mar 17 09:45:44 2018
+++ src/sys/dev/ic/nvme.c	Wed Apr 18 10:05:59 2018
@@ -1,4 +1,4 @@
-/*	$NetBSD: nvme.c,v 1.37 2018/03/17 09:45:44 jdolecek Exp $	*/
+/*	$NetBSD: nvme.c,v 1.38 2018/04/18 10:05:59 nonaka Exp $	*/
 /*	$OpenBSD: nvme.c,v 1.49 2016/04/18 05:59:50 dlg Exp $ */
 
 /*
@@ -18,7 +18,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: nvme.c,v 1.37 2018/03/17 09:45:44 jdolecek Exp $");
+__KERNEL_RCSID(0, "$NetBSD: nvme.c,v 1.38 2018/04/18 10:05:59 nonaka Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -42,6 +42,8 @@ __KERNEL_RCSID(0, "$NetBSD: nvme.c,v 1.3
 
 #include "ioconf.h"
 
+#define	B4_CHK_RDY_DELAY_MS	2300	/* workaround controller bug */
+
 int nvme_adminq_size = 32;
 int nvme_ioq_size = 1024;
 
@@ -220,15 +222,6 @@ static int
 nvme_ready(struct nvme_softc *sc, uint32_t rdy)
 {
 	u_int i = 0;
-	uint32_t cc;
-
-	cc = nvme_read4(sc, NVME_CC);
-	if (((cc & NVME_CC_EN) != 0) != (rdy != 0)) {
-		aprint_error_dev(sc->sc_dev,
-		    "controller enabled status expected %d, found to be %d\n",
-		    (rdy != 0), ((cc & NVME_CC_EN) != 0));
-		return ENXIO;
-	}
 
 	while ((nvme_read4(sc, NVME_CSTS) & NVME_CSTS_RDY) != rdy) {
 		if (i++ > sc->sc_rdy_to)
@@ -245,17 +238,24 @@ static int
 nvme_enable(struct nvme_softc *sc, u_int mps)
 {
 	uint32_t cc, csts;
+	int error;
 
 	cc = nvme_read4(sc, NVME_CC);
 	csts = nvme_read4(sc, NVME_CSTS);
-	
-	if (ISSET(cc, NVME_CC_EN)) {
-		aprint_error_dev(sc->sc_dev, "controller unexpectedly enabled, failed to stay disabled\n");
 
+	/*
+	 * See note in nvme_disable. Short circuit if we're already enabled.
+	 */
+	if (ISSET(cc, NVME_CC_EN)) {
 		if (ISSET(csts, NVME_CSTS_RDY))
-			return 1;
+			return 0;
 
 		goto waitready;
+	} else {
+		/* EN == 0 already wait for RDY == 0 or fail */
+		error = nvme_ready(sc, 0);
+		if (error)
+			return error;
 	}
 
 	nvme_write8(sc, NVME_ASQ, NVME_DMA_DVA(sc->sc_admin_q->q_sq_dmamem));
@@ -282,7 +282,6 @@ nvme_enable(struct nvme_softc *sc, u_int
 	nvme_write4(sc, NVME_CC, cc);
 	nvme_barrier(sc, 0, sc->sc_ios,
 	    BUS_SPACE_BARRIER_READ | BUS_SPACE_BARRIER_WRITE);
-	delay(5000);
 
     waitready:
 	return nvme_ready(sc, NVME_CSTS_RDY);
@@ -292,20 +291,44 @@ static int
 nvme_disable(struct nvme_softc *sc)
 {
 	uint32_t cc, csts;
+	int error;
 
 	cc = nvme_read4(sc, NVME_CC);
 	csts = nvme_read4(sc, NVME_CSTS);
 
-	if (ISSET(cc, NVME_CC_EN) && !ISSET(csts, NVME_CSTS_RDY))
-		nvme_ready(sc, NVME_CSTS_RDY);
+	/*
+	 * Per 3.1.5 in NVME 1.3 spec, transitioning CC.EN from 0 to 1
+	 * when CSTS.RDY is 1 or transitioning CC.EN from 1 to 0 when
+	 * CSTS.RDY is 0 "has undefined results" So make sure that CSTS.RDY
+	 * isn't the desired value. Short circuit if we're already disabled.
+	 */
+	if (ISSET(cc, NVME_CC_EN)) {
+		if (!ISSET(csts, NVME_CSTS_RDY)) {
+			/* EN == 1, wait for RDY == 1 or fail */
+			error = nvme_ready(sc, NVME_CSTS_RDY);
+			if (error)
+				return error;
+		}
+	} else {
+		/* EN == 0 already wait for RDY == 0 */
+		if (!ISSET(csts, NVME_CSTS_RDY))
+			return 0;
 
-	CLR(cc, NVME_CC_EN);
+		goto waitready;
+	}
 
+	CLR(cc, NVME_CC_EN);
 	nvme_write4(sc, NVME_CC, cc);
 	nvme_barrier(sc, 0, sc->sc_ios, BUS_SPACE_BARRIER_READ);
-	
-	delay(5000);
 
+	/*
+	 * Some drives have issues with accessing the mmio after we disable,
+	 * so delay for a bit after we write the bit to cope with these issues.
+	 */
+	if (ISSET(sc->sc_quirks, NVME_QUIRK_DELAY_B4_CHK_RDY))
+		delay(B4_CHK_RDY_DELAY_MS);
+
+    waitready:
 	return nvme_ready(sc, 0);
 }
 

Index: src/sys/dev/ic/nvmevar.h
diff -u src/sys/dev/ic/nvmevar.h:1.15 src/sys/dev/ic/nvmevar.h:1.16
--- src/sys/dev/ic/nvmevar.h:1.15	Fri Mar 16 23:31:19 2018
+++ src/sys/dev/ic/nvmevar.h	Wed Apr 18 10:05:59 2018
@@ -1,4 +1,4 @@
-/*	$NetBSD: nvmevar.h,v 1.15 2018/03/16 23:31:19 jdolecek Exp $	*/
+/*	$NetBSD: nvmevar.h,v 1.16 2018/04/18 10:05:59 nonaka Exp $	*/
 /*	$OpenBSD: nvmevar.h,v 1.8 2016/04/14 11:18:32 dlg Exp $ */
 
 /*
@@ -136,6 +136,9 @@ struct nvme_softc {
 	uint32_t		sc_flags;
 #define	NVME_F_ATTACHED	__BIT(0)
 #define	NVME_F_OPEN	__BIT(1)
+
+	uint32_t		sc_quirks;
+#define	NVME_QUIRK_DELAY_B4_CHK_RDY	__BIT(0)
 };
 
 #define	lemtoh16(p)	le16toh(*((uint16_t *)(p)))

Index: src/sys/dev/pci/nvme_pci.c
diff -u src/sys/dev/pci/nvme_pci.c:1.19 src/sys/dev/pci/nvme_pci.c:1.20
--- src/sys/dev/pci/nvme_pci.c:1.19	Thu Jun  1 02:45:11 2017
+++ src/sys/dev/pci/nvme_pci.c	Wed Apr 18 10:05:59 2018
@@ -1,4 +1,4 @@
-/*	$NetBSD: nvme_pci.c,v 1.19 2017/06/01 02:45:11 chs Exp $	*/
+/*	$NetBSD: nvme_pci.c,v 1.20 2018/04/18 10:05:59 nonaka Exp $	*/
 /*	$OpenBSD: nvme_pci.c,v 1.3 2016/04/14 11:18:32 dlg Exp $ */
 
 /*
@@ -43,7 +43,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: nvme_pci.c,v 1.19 2017/06/01 02:45:11 chs Exp $");
+__KERNEL_RCSID(0, "$NetBSD: nvme_pci.c,v 1.20 2018/04/18 10:05:59 nonaka Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -59,6 +59,7 @@ __KERNEL_RCSID(0, "$NetBSD: nvme_pci.c,v
 
 #include <dev/pci/pcireg.h>
 #include <dev/pci/pcivar.h>
+#include <dev/pci/pcidevs.h>
 
 #include <dev/ic/nvmereg.h>
 #include <dev/ic/nvmevar.h>
@@ -92,6 +93,39 @@ static int	nvme_pci_intr_disestablish(st
 static int	nvme_pci_setup_intr(struct pci_attach_args *,
 		    struct nvme_pci_softc *);
 
+static const struct nvme_pci_quirk {
+	pci_vendor_id_t		vendor;
+	pci_product_id_t	product;
+	uint32_t		quirks;
+} nvme_pci_quirks[] = {
+	{ PCI_VENDOR_HGST, PCI_PRODUCT_HGST_SN100,
+	    NVME_QUIRK_DELAY_B4_CHK_RDY },
+	{ PCI_VENDOR_HGST, PCI_PRODUCT_HGST_SN200,
+	    NVME_QUIRK_DELAY_B4_CHK_RDY },
+	{ PCI_VENDOR_BEIJING_MEMBLAZE, PCI_PRODUCT_BEIJING_MEMBLAZE_PBLAZE4,
+	    NVME_QUIRK_DELAY_B4_CHK_RDY },
+	{ PCI_VENDOR_SAMSUNGELEC3, PCI_PRODUCT_SAMSUNGELEC3_172X,
+	    NVME_QUIRK_DELAY_B4_CHK_RDY },
+	{ PCI_VENDOR_SAMSUNGELEC3, PCI_PRODUCT_SAMSUNGELEC3_172XAB,
+	    NVME_QUIRK_DELAY_B4_CHK_RDY },
+};
+
+static const struct nvme_pci_quirk *
+nvme_pci_lookup_quirk(struct pci_attach_args *pa)
+{
+	const struct nvme_pci_quirk *q;
+	int i;
+
+	for (i = 0; i < __arraycount(nvme_pci_quirks); i++) {
+		q = &nvme_pci_quirks[i];
+
+		if (PCI_VENDOR(pa->pa_id) == q->vendor &&
+		    PCI_PRODUCT(pa->pa_id) == q->product)
+			return q;
+	}
+	return NULL;
+}
+
 static int
 nvme_pci_match(device_t parent, cfdata_t match, void *aux)
 {
@@ -111,6 +145,7 @@ nvme_pci_attach(device_t parent, device_
 	struct nvme_pci_softc *psc = device_private(self);
 	struct nvme_softc *sc = &psc->psc_nvme;
 	struct pci_attach_args *pa = aux;
+	const struct nvme_pci_quirk *quirk;
 	pcireg_t memtype, reg;
 	bus_addr_t memaddr;
 	int flags, error;
@@ -179,6 +214,11 @@ nvme_pci_attach(device_t parent, device_
 	sc->sc_ih = kmem_zalloc(sizeof(*sc->sc_ih) * psc->psc_nintrs, KM_SLEEP);
 	sc->sc_softih = kmem_zalloc(
 	    sizeof(*sc->sc_softih) * psc->psc_nintrs, KM_SLEEP);
+
+	quirk = nvme_pci_lookup_quirk(pa);
+	if (quirk != NULL)
+		sc->sc_quirks = quirk->quirks;
+
 	if (nvme_attach(sc) != 0) {
 		/* error printed by nvme_attach() */
 		goto softintr_free;

Reply via email to