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

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:

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:

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; }

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__ */

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);

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

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)

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

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!
participants (2)
-
Bin Meng
-
Tom Rini