[PATCH v2 1/7] nvme: Move block dev creation from uclass post_probe() to driver probe()

[PATCH v2 1/7] nvme: Move block dev creation from uclass post_probe() to driver probe()

[Bin Meng]

At present the block device creation happens in the NVMe uclass
driver post_probe() phase. In preparation to support multiple
namespaces, we should issue namespace identify before creating
block devices but that touches the underlying hardware hence it
is not appropriate to do such in the uclass driver post_probe().
Let's move it to driver probe() phase instead.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>

---

Changes in v2:
- drop the nvme per-child platdata change

 drivers/nvme/nvme-uclass.c | 30 ------------------------------
 drivers/nvme/nvme.c        | 18 ++++++++++++++++++
 2 files changed, 18 insertions(+), 30 deletions(-)

diff --git a/drivers/nvme/nvme-uclass.c b/drivers/nvme/nvme-uclass.c
index 277e31e1f3..610166d76e 100644
--- a/drivers/nvme/nvme-uclass.c
+++ b/drivers/nvme/nvme-uclass.c
@@ -5,39 +5,9 @@
  */
 
 #include <common.h>
-#include <blk.h>
-#include <errno.h>
 #include <dm.h>
-#include <dm/device.h>
-#include "nvme.h"
-
-static int nvme_uclass_post_probe(struct udevice *udev)
-{
-	char name[20];
-	struct udevice *ns_udev;
-	int i, ret;
-	struct nvme_dev *ndev = dev_get_priv(udev);
-
-	/* Create a blk device for each namespace */
-	for (i = 0; i < ndev->nn; i++) {
-		/*
-		 * Encode the namespace id to the device name so that
-		 * we can extract it when doing the probe.
-		 */
-		sprintf(name, "blk#%d", i);
-
-		/* The real blksz and size will be set by nvme_blk_probe() */
-		ret = blk_create_devicef(udev, "nvme-blk", name, IF_TYPE_NVME,
-					 -1, 512, 0, &ns_udev);
-		if (ret)
-			return ret;
-	}
-
-	return 0;
-}
 
 UCLASS_DRIVER(nvme) = {
 	.name	= "nvme",
 	.id	= UCLASS_NVME,
-	.post_probe = nvme_uclass_post_probe,
 };
diff --git a/drivers/nvme/nvme.c b/drivers/nvme/nvme.c
index c61dab20c5..29735a6ad8 100644
--- a/drivers/nvme/nvme.c
+++ b/drivers/nvme/nvme.c
@@ -879,6 +879,24 @@ static int nvme_probe(struct udevice *udev)
 
 	nvme_get_info_from_identify(ndev);
 
+	/* Create a blk device for each namespace */
+	for (int i = 0; i < ndev->nn; i++) {
+		struct udevice *ns_udev;
+		char name[20];
+
+		/*
+		 * Encode the namespace id to the device name so that
+		 * we can extract it when doing the probe.
+		 */
+		sprintf(name, "blk#%d", i);
+
+		/* The real blksz and size will be set by nvme_blk_probe() */
+		ret = blk_create_devicef(udev, "nvme-blk", name, IF_TYPE_NVME,
+					 -1, 512, 0, &ns_udev);
+		if (ret)
+			goto free_queue;
+	}
+
 	return 0;
 
 free_queue:
-- 
2.25.1

[PATCH v2 2/7] nvme: Skip block device creation for inactive namespaces

[Bin Meng]

At present for each namespace there is a block device created for it.
There is no issue if the number of supported namespaces reported from
the NVMe device is only 1.

Since QEMU commit 7f0f1acedf15 ("hw/block/nvme: support multiple namespaces"),
the number of supported namespaces reported has been changed from 1
to 256, but not all of them are active namespaces. The actual active
one depends on the QEMU command line parameters. A common case is
that namespace 1 being active and all other 255 being inactive.

If a namespace is inactive, the namespace identify command returns a
zero filled data structure. We can use field NSZE (namespace size) to
decide whether a block device should be created for it.

Reported-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
---

(no changes since v1)

 drivers/nvme/nvme.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/nvme.c b/drivers/nvme/nvme.c
index 29735a6ad8..bb24c9cdd3 100644
--- a/drivers/nvme/nvme.c
+++ b/drivers/nvme/nvme.c
@@ -835,6 +835,7 @@ static int nvme_probe(struct udevice *udev)
 {
 	int ret;
 	struct nvme_dev *ndev = dev_get_priv(udev);
+	struct nvme_id_ns *id;
 
 	ndev->instance = trailing_strtol(udev->name);
 
@@ -880,10 +881,27 @@ static int nvme_probe(struct udevice *udev)
 	nvme_get_info_from_identify(ndev);
 
 	/* Create a blk device for each namespace */
+
+	id = memalign(ndev->page_size, sizeof(struct nvme_id_ns));
+	if (!id) {
+		ret = -ENOMEM;
+		goto free_queue;
+	}
+
 	for (int i = 0; i < ndev->nn; i++) {
 		struct udevice *ns_udev;
 		char name[20];
 
+		memset(id, 0, sizeof(*id));
+		if (nvme_identify(ndev, i + 1, 0, (dma_addr_t)(long)id)) {
+			ret = -EIO;
+			goto free_id;
+		}
+
+		/* skip inactive namespace */
+		if (!id->nsze)
+			continue;
+
 		/*
 		 * Encode the namespace id to the device name so that
 		 * we can extract it when doing the probe.
@@ -894,11 +912,14 @@ static int nvme_probe(struct udevice *udev)
 		ret = blk_create_devicef(udev, "nvme-blk", name, IF_TYPE_NVME,
 					 -1, 512, 0, &ns_udev);
 		if (ret)
-			goto free_queue;
+			goto free_id;
 	}
 
+	free(id);
 	return 0;
 
+free_id:
+	free(id);
 free_queue:
 	free((void *)ndev->queues);
 free_nvme:
-- 
2.25.1

[PATCH v2 3/7] nvme: Eliminate the offset of one during block dev creation

[Bin Meng]

At present there is an offset of one added during the creation of
block device. This can be very confusing as we wanted to encode the
namespace id in the block device name but namespae id cannot be zero.

This changes to use the namespace id directly in the block device
name, eliminating the offset of one effectively.

Suggested-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
Signed-off-by: Bin Meng <bmeng.cn@gmail.com>

---

Changes in v2:
- new patch: Eliminate the offset of one during block dev creation

 drivers/nvme/nvme.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/nvme/nvme.c b/drivers/nvme/nvme.c
index bb24c9cdd3..97c665d6a3 100644
--- a/drivers/nvme/nvme.c
+++ b/drivers/nvme/nvme.c
@@ -709,7 +709,7 @@ static int nvme_blk_probe(struct udevice *udev)
 	memset(ns, 0, sizeof(*ns));
 	ns->dev = ndev;
 	/* extract the namespace id from the block device name */
-	ns->ns_id = trailing_strtol(udev->name) + 1;
+	ns->ns_id = trailing_strtol(udev->name);
 	if (nvme_identify(ndev, ns->ns_id, 0, (dma_addr_t)(long)id)) {
 		free(id);
 		return -EIO;
@@ -888,12 +888,12 @@ static int nvme_probe(struct udevice *udev)
 		goto free_queue;
 	}
 
-	for (int i = 0; i < ndev->nn; i++) {
+	for (int i = 1; i <= ndev->nn; i++) {
 		struct udevice *ns_udev;
 		char name[20];
 
 		memset(id, 0, sizeof(*id));
-		if (nvme_identify(ndev, i + 1, 0, (dma_addr_t)(long)id)) {
+		if (nvme_identify(ndev, i, 0, (dma_addr_t)(long)id)) {
 			ret = -EIO;
 			goto free_id;
 		}
-- 
2.25.1

[PATCH v2 4/7] nvme: Drop useless members of 'struct nvme_ns'

[Bin Meng]

mode_select_num_blocks and mode_select_block_len in 'struct nvme_ns'
are not useful. Drop them.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>

---

Changes in v2:
- new patch: Drop useless members of 'struct nvme_ns'

 drivers/nvme/nvme.c | 4 +---
 drivers/nvme/nvme.h | 2 --
 2 files changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/nvme/nvme.c b/drivers/nvme/nvme.c
index 97c665d6a3..e2e13dbf66 100644
--- a/drivers/nvme/nvme.c
+++ b/drivers/nvme/nvme.c
@@ -719,11 +719,9 @@ static int nvme_blk_probe(struct udevice *udev)
 	flbas = id->flbas & NVME_NS_FLBAS_LBA_MASK;
 	ns->flbas = flbas;
 	ns->lba_shift = id->lbaf[flbas].ds;
-	ns->mode_select_num_blocks = le64_to_cpu(id->nsze);
-	ns->mode_select_block_len = 1 << ns->lba_shift;
 	list_add(&ns->list, &ndev->namespaces);
 
-	desc->lba = ns->mode_select_num_blocks;
+	desc->lba = le64_to_cpu(id->nsze);
 	desc->log2blksz = ns->lba_shift;
 	desc->blksz = 1 << ns->lba_shift;
 	desc->bdev = udev;
diff --git a/drivers/nvme/nvme.h b/drivers/nvme/nvme.h
index aa4b3bac67..c6aae4da5d 100644
--- a/drivers/nvme/nvme.h
+++ b/drivers/nvme/nvme.h
@@ -633,8 +633,6 @@ struct nvme_ns {
 	int devnum;
 	int lba_shift;
 	u8 flbas;
-	u64 mode_select_num_blocks;
-	u32 mode_select_block_len;
 };
 
 #endif /* __DRIVER_NVME_H__ */
-- 
2.25.1

[PATCH v2 5/7] nvme: Don't clear nvme blk device's priv space

[Bin Meng]

A udevice's priv space is cleared in alloc_priv() in the DM core.
Don't do it again in its probe() routine.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>

---

Changes in v2:
- new patch: Don't clear nvme blk device's priv space

 drivers/nvme/nvme.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/nvme/nvme.c b/drivers/nvme/nvme.c
index e2e13dbf66..0527198aa6 100644
--- a/drivers/nvme/nvme.c
+++ b/drivers/nvme/nvme.c
@@ -706,7 +706,6 @@ static int nvme_blk_probe(struct udevice *udev)
 	if (!id)
 		return -ENOMEM;
 
-	memset(ns, 0, sizeof(*ns));
 	ns->dev = ndev;
 	/* extract the namespace id from the block device name */
 	ns->ns_id = trailing_strtol(udev->name);
-- 
2.25.1

[PATCH v2 6/7] doc: develop: Convert README.nvme to reST

[Bin Meng]

This converts the existing README.nvme to reST, and puts it under
the develop/driver-model/ directory.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>

---

Changes in v2:
- new patch: Convert README.nvme to reST

 doc/develop/driver-model/index.rst            |  1 +
 .../driver-model/nvme.rst}                    | 25 ++++++++++++++-----
 2 files changed, 20 insertions(+), 6 deletions(-)
 rename doc/{README.nvme => develop/driver-model/nvme.rst} (88%)

diff --git a/doc/develop/driver-model/index.rst b/doc/develop/driver-model/index.rst
index 10a76256b0..7366ef818c 100644
--- a/doc/develop/driver-model/index.rst
+++ b/doc/develop/driver-model/index.rst
@@ -19,6 +19,7 @@ subsystems
    i2c-howto
    livetree
    migration
+   nvme
    of-plat
    pci-info
    pmic-framework
diff --git a/doc/README.nvme b/doc/develop/driver-model/nvme.rst
similarity index 88%
rename from doc/README.nvme
rename to doc/develop/driver-model/nvme.rst
index e8f9be149e..736c0a063d 100644
--- a/doc/README.nvme
+++ b/doc/develop/driver-model/nvme.rst
@@ -1,11 +1,13 @@
-# SPDX-License-Identifier: GPL-2.0+
-#
-# Copyright (C) 2017 NXP Semiconductors
-# Copyright (C) 2017 Bin Meng <bmeng.cn@gmail.com>
+.. SPDX-License-Identifier: GPL-2.0+
+.. Copyright (C) 2017 NXP Semiconductors
+.. Copyright (C) 2017 Bin Meng <bmeng.cn@gmail.com>
 
-What is NVMe
+NVMe Support
 ============
 
+What is NVMe
+------------
+
 NVM Express (NVMe) is a register level interface that allows host software to
 communicate with a non-volatile memory subsystem. This interface is optimized
 for enterprise and client solid state drives, typically attached to the PCI
@@ -48,6 +50,8 @@ identified.
 
 To list all of the NVMe hard disks, try:
 
+.. code-block:: none
+
   => nvme info
   Device 0: Vendor: 0x8086 Rev: 8DV10131 Prod: CVFT535600LS400BGN
 	    Type: Hard Disk
@@ -55,10 +59,14 @@ To list all of the NVMe hard disks, try:
 
 and print out detailed information for controller and namespaces via:
 
+.. code-block:: none
+
   => nvme detail
 
 Raw block read/write to can be done via the 'nvme read/write' commands:
 
+.. code-block:: none
+
   => nvme read a0000000 0 11000
 
   => tftp 80000000 /tftpboot/kernel.itb
@@ -66,6 +74,8 @@ Raw block read/write to can be done via the 'nvme read/write' commands:
 
 Of course, file system command can be used on the NVMe hard disk as well:
 
+.. code-block:: none
+
   => fatls nvme 0:1
 	32376967   kernel.itb
 	22929408   100m
@@ -81,4 +91,7 @@ QEMU supports NVMe emulation and we can test NVMe driver with QEMU x86 running
 U-Boot. Please see README.x86 for how to build u-boot.rom image for QEMU x86.
 
 Example command line to call QEMU x86 below with emulated NVMe device:
-$ ./qemu-system-i386 -drive file=nvme.img,if=none,id=drv0 -device nvme,drive=drv0,serial=QEMUNVME0001 -bios u-boot.rom
+
+.. code-block:: bash
+
+  $ ./qemu-system-i386 -drive file=nvme.img,if=none,id=drv0 -device nvme,drive=drv0,serial=QEMUNVME0001 -bios u-boot.rom
-- 
2.25.1

[PATCH v2 7/7] MAINTAINERS: Add an entry for NVMe

[Bin Meng]

This was missed when NVMe support was initially brought to U-Boot
back in 2017. Add an entry for it and list myself as the maintainer.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>

---

Changes in v2:
- new patch: Add an entry for NVMe

 MAINTAINERS | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 2accd1fb83..81190f8f8f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -913,6 +913,14 @@ S:	Maintained
 T:	git https://source.denx.de/u-boot/custodians/u-boot-nios.git
 F:	arch/nios2/
 
+NVMe
+M:	Bin Meng <bmeng.cn@gmail.com>
+S:	Maintained
+F:	drivers/nvme/
+F:	cmd/nvme.c
+F:	include/nvme.h
+F:	doc/develop/driver-model/nvme.rst
+
 ONENAND
 #M:	Lukasz Majewski <l.majewski@majess.pl>
 S:	Orphaned (Since 2017-01)
-- 
2.25.1

Re: [PATCH v2 7/7] MAINTAINERS: Add an entry for NVMe

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 316 bytes --]

On Tue, Jun 22, 2021 at 09:16:23PM +0800, Bin Meng wrote:

> This was missed when NVMe support was initially brought to U-Boot
> back in 2017. Add an entry for it and list myself as the maintainer.
> 
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> 

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v2 1/7] nvme: Move block dev creation from uclass post_probe() to driver probe()

[Bin Meng]

On Tue, Jun 22, 2021 at 9:16 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> At present the block device creation happens in the NVMe uclass
> driver post_probe() phase. In preparation to support multiple
> namespaces, we should issue namespace identify before creating
> block devices but that touches the underlying hardware hence it
> is not appropriate to do such in the uclass driver post_probe().
> Let's move it to driver probe() phase instead.
>
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>
> ---
>
> Changes in v2:
> - drop the nvme per-child platdata change
>
>  drivers/nvme/nvme-uclass.c | 30 ------------------------------
>  drivers/nvme/nvme.c        | 18 ++++++++++++++++++
>  2 files changed, 18 insertions(+), 30 deletions(-)
>

series applied to u-boot-x86, thanks!