[PATCH v3 0/7] few fixes for NVMe and block devices

[PATCH v3 0/7] few fixes for NVMe and block devices

[dmukhin]

The series adds few fixes for NVMe subsystem and block device management.

Patch 1 adds FUA support for NVMe devices (original effort in [1]).
Patch 2 adds plumbing for blk_flush()/blk_dflush().
Patch 3 introduces NVMe flush command.
Patch 4 enables `flush` subcommand to all the block-uclass front-end
        commands (ide, nvme, pvblock, sata, scsi, usb, virtio) in one go.
Patch 5 makes nvme_shutdown() symbol public to enable use of it from
        board_quiesce_devices()
Patch 6 tiny fixup for QEMU command for manual NVMe tests.
Patch 7 updates tests for flushing block devices.

[1] https://lore.kernel.org/u-boot/20211019104049.v3.1.Ic581ec99f46b6dfa2e0b1922e670a333ac859e82@changeid/
[2] Link to v2: https://lore.kernel.org/u-boot/20260529034441.2075305-1-dmukhin@ford.com/
[3] Link to CI: https://github.com/u-boot/u-boot/pull/995

Denis Mukhin (7):
  drivers: nvme: Enable Force Unit Access (FUA)
  drivers: block: Introduce blk_flush()/blk_dflush()
  drivers: nvme: Implement flush command
  cmd: Add flush support for all blk devices
  drivers: nvme: Export nvme_shutdown()
  docs: nvme: Update QEMU command for testing
  tests: add blk_dflush() coverage

 arch/sandbox/cpu/os.c             |  5 ++++
 cmd/blk_common.c                  | 17 +++++++++++++
 cmd/blkmap.c                      |  2 ++
 cmd/ide.c                         |  1 +
 cmd/nvme.c                        |  1 +
 cmd/pvblock.c                     |  1 +
 cmd/sata.c                        |  1 +
 cmd/scsi.c                        |  1 +
 cmd/usb.c                         |  1 +
 cmd/virtio.c                      |  1 +
 disk/disk-uclass.c                |  6 +++++
 doc/develop/driver-model/nvme.rst |  2 +-
 drivers/block/blk-uclass.c        | 15 +++++++++++
 drivers/block/sandbox.c           |  9 +++++++
 drivers/nvme/nvme.c               | 42 +++++++++++++++++++++++++++++++
 drivers/nvme/nvme.h               | 12 ++++-----
 drivers/nvme/nvme_apple.c         |  1 +
 include/blk.h                     | 26 +++++++++++++++++++
 include/nvme.h                    | 11 ++++++++
 include/os.h                      |  8 ++++++
 include/part.h                    |  8 ++++++
 test/dm/host.c                    |  2 ++
 test/dm/mmc.c                     |  2 ++
 23 files changed, 167 insertions(+), 8 deletions(-)

-- 
2.54.0

[PATCH v3 1/7] drivers: nvme: Enable Force Unit Access (FUA)

[dmukhin]

From: Denis Mukhin <dmukhin@ford.com> 

Enable FUA (if present) on NVMe devices to prevent any potential data
integrity issues caused by accidental power loss or malformed device
handoff sequence to the OS.

Original FUA effort has been started in [1]

[1] https://lore.kernel.org/u-boot/20211019104049.v3.1.Ic581ec99f46b6dfa2e0b1922e670a333ac859e82@changeid/
Original-patch-by: Jon Lin <jon.lin@rock-chips.com>
Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
I mentioned Jon's work under 'Orignal-patch-by' since current
version differs quite a bit...

Changes since v2:
- new patch
---
 drivers/nvme/nvme.c | 17 +++++++++++++++++
 drivers/nvme/nvme.h |  5 +++++
 2 files changed, 22 insertions(+)

diff --git a/drivers/nvme/nvme.c b/drivers/nvme/nvme.c
index 2b14437f69c2..7bb6bb7b53f3 100644
--- a/drivers/nvme/nvme.c
+++ b/drivers/nvme/nvme.c
@@ -771,6 +771,13 @@ static ulong nvme_blk_rw(struct udevice *udev, lbaint_t blknr,
 	c.rw.appmask = 0;
 	c.rw.metadata = 0;
 
+	/*
+	 * Enable force unit access (FUA) for data integrity if volatile write
+	 * cache (VWC) is enabled.
+	 */
+	if (!read && (dev->flags & NVME_VWC_ENABLED))
+		c.rw.control |= cpu_to_le16(NVME_RW_FUA);
+
 	while (total_lbas) {
 		if (total_lbas < lbas) {
 			lbas = (u16)total_lbas;
@@ -899,6 +906,16 @@ int nvme_init(struct udevice *udev)
 		if (!id->nsze)
 			continue;
 
+		/* Check whether volatile write cache is enabled. */
+		if (ndev->vwc & NVME_CTRL_VWC_PRESENT) {
+			u32 features;
+
+			ret = nvme_get_features(ndev, NVME_FEAT_VOLATILE_WC, 0, 0,
+						&features);
+			if (!ret && (features & 0x1))
+				ndev->flags |= NVME_VWC_ENABLED;
+		}
+
 		/*
 		 * Encode the namespace id to the device name so that
 		 * we can extract it when doing the probe.
diff --git a/drivers/nvme/nvme.h b/drivers/nvme/nvme.h
index bc1d612dde40..a938da728506 100644
--- a/drivers/nvme/nvme.h
+++ b/drivers/nvme/nvme.h
@@ -594,6 +594,10 @@ enum {
 	NVME_CSTS_SHST_MASK	= 3 << 2,
 };
 
+enum {
+	NVME_VWC_ENABLED			= BIT(0),
+};
+
 /* Represents an NVM Express device. Each nvme_dev is a PCI function. */
 struct nvme_dev {
 	struct udevice *udev;
@@ -621,6 +625,7 @@ struct nvme_dev {
 	u64 *prp_pool;
 	u32 prp_entry_num;
 	u32 nn;
+	unsigned int flags;
 };
 
 /* Admin queue and a single I/O queue. */
-- 
2.54.0

[PATCH v3 2/7] drivers: block: Introduce blk_flush()/blk_dflush()

[dmukhin]

From: Denis Mukhin <dmukhin@ford.com> 

Add generic flush operations for committing dirty data to the storage
device.

This provides a common block-layer interface for flushing pending
writes, allowing callers to ensure that data buffered by the block
device or its backing implementation is written out to the underlying
storage.

Note, flushing a partition device via disk_blk_flush() flushes the entire
underlying device, so callers don't expect partition-scoped semantics.

Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
Changes since v2:
- addressed feedback:
  https://lore.kernel.org/u-boot/CAFLszTgz6ia9qgkSq=2=JT+csdo=9O8KpjJeYi1=CS2n_yua4w@mail.gmail.com/

Changes since v1:
- new patch
---
 disk/disk-uclass.c         |  6 ++++++
 drivers/block/blk-uclass.c | 15 +++++++++++++++
 include/blk.h              | 26 ++++++++++++++++++++++++++
 include/part.h             |  8 ++++++++
 4 files changed, 55 insertions(+)

diff --git a/disk/disk-uclass.c b/disk/disk-uclass.c
index ee3cc4407d76..1a87f7dffa7c 100644
--- a/disk/disk-uclass.c
+++ b/disk/disk-uclass.c
@@ -122,6 +122,11 @@ unsigned long disk_blk_erase(struct udevice *dev, lbaint_t start,
 			 blkcnt);
 }
 
+unsigned long disk_blk_flush(struct udevice *dev)
+{
+	return blk_flush(dev_get_parent(dev));
+}
+
 UCLASS_DRIVER(partition) = {
 	.id		= UCLASS_PARTITION,
 	.per_device_plat_auto	= sizeof(struct disk_part),
@@ -132,6 +137,7 @@ static const struct blk_ops blk_part_ops = {
 	.read	= disk_blk_read,
 	.write	= disk_blk_write,
 	.erase	= disk_blk_erase,
+	.flush	= disk_blk_flush,
 };
 
 U_BOOT_DRIVER(blk_partition) = {
diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c
index 73c24fd91763..16fd02ffb16a 100644
--- a/drivers/block/blk-uclass.c
+++ b/drivers/block/blk-uclass.c
@@ -514,6 +514,16 @@ long blk_erase(struct udevice *dev, lbaint_t start, lbaint_t blkcnt)
 	return ops->erase(dev, start, blkcnt);
 }
 
+long blk_flush(struct udevice *dev)
+{
+	const struct blk_ops *ops = blk_get_ops(dev);
+
+	if (ops->flush)
+		return ops->flush(dev);
+
+	return -ENOSYS;
+}
+
 ulong blk_dread(struct blk_desc *desc, lbaint_t start, lbaint_t blkcnt,
 		void *buffer)
 {
@@ -531,6 +541,11 @@ ulong blk_derase(struct blk_desc *desc, lbaint_t start, lbaint_t blkcnt)
 	return blk_erase(desc->bdev, start, blkcnt);
 }
 
+ulong blk_dflush(struct blk_desc *desc)
+{
+	return blk_flush(desc->bdev);
+}
+
 int blk_find_from_parent(struct udevice *parent, struct udevice **devp)
 {
 	struct udevice *dev;
diff --git a/include/blk.h b/include/blk.h
index 8d1b70cabd31..1004d17c5b16 100644
--- a/include/blk.h
+++ b/include/blk.h
@@ -99,6 +99,7 @@ struct blk_desc {
 	unsigned long	(*block_erase)(struct blk_desc *block_dev,
 				       lbaint_t start,
 				       lbaint_t blkcnt);
+	unsigned long	(*block_flush)(struct blk_desc *block_dev);
 	void		*priv;		/* driver private struct pointer */
 #endif
 };
@@ -239,6 +240,14 @@ struct blk_ops {
 	unsigned long (*erase)(struct udevice *dev, lbaint_t start,
 			       lbaint_t blkcnt);
 
+	/**
+	 * flush() - commit all dirty data to storage
+	 *
+	 * @dev:	Device to flush
+	 * @return 0 if OK, -ve on error
+	 */
+	unsigned long (*flush)(struct udevice *dev);
+
 	/**
 	 * select_hwpart() - select a particular hardware partition
 	 *
@@ -291,6 +300,7 @@ unsigned long blk_dwrite(struct blk_desc *block_dev, lbaint_t start,
 			 lbaint_t blkcnt, const void *buffer);
 unsigned long blk_derase(struct blk_desc *block_dev, lbaint_t start,
 			 lbaint_t blkcnt);
+unsigned long blk_dflush(struct blk_desc *block_dev);
 
 #endif /* BLK */
 
@@ -331,6 +341,14 @@ long blk_write(struct udevice *dev, lbaint_t start, lbaint_t blkcnt,
  */
 long blk_erase(struct udevice *dev, lbaint_t start, lbaint_t blkcnt);
 
+/**
+ * blk_flush() - Commit data to a block device
+ *
+ * @dev: Device to flush
+ * @return 0 if operation succeeded, or -ve on error.
+ */
+long blk_flush(struct udevice *dev);
+
 /**
  * blk_find_device() - Find a block device
  *
@@ -559,6 +577,14 @@ static inline ulong blk_derase(struct blk_desc *block_dev, lbaint_t start,
 	return block_dev->block_erase(block_dev, start, blkcnt);
 }
 
+static inline ulong blk_dflush(struct blk_desc *block_dev)
+{
+	if (block_dev->block_flush)
+		return block_dev->block_flush(block_dev);
+
+	return -ENOSYS;
+}
+
 /**
  * struct blk_driver - Driver for block interface types
  *
diff --git a/include/part.h b/include/part.h
index 6caaa6526aa4..1f3db7b076fa 100644
--- a/include/part.h
+++ b/include/part.h
@@ -454,6 +454,14 @@ ulong disk_blk_write(struct udevice *dev, lbaint_t start, lbaint_t blkcnt,
  */
 ulong disk_blk_erase(struct udevice *dev, lbaint_t start, lbaint_t blkcnt);
 
+/**
+ * disk_blk_flush() - commit data to a disk
+ *
+ * @dev:	Device to flush
+ * Return:	0 success, or -ve error number (see the IS_ERR_VALUE()) macro
+ */
+ulong disk_blk_flush(struct udevice *dev);
+
 /*
  * We don't support printing partition information in SPL and only support
  * getting partition information in a few cases.
-- 
2.54.0

[PATCH v3 3/7] drivers: nvme: Implement flush command

[dmukhin]

From: Denis Mukhin <dmukhin@ford.com> 

Implement the NVM flush command (opcode 0x00) to allow callers to
synchronize any buffered data with the backing NVMe storage.

Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
Changes since v2:
- gate flush by VWC check

Changes since v1:
- moved generic disk_blk_flush() to a separate patch
---
 drivers/nvme/nvme.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/drivers/nvme/nvme.c b/drivers/nvme/nvme.c
index 7bb6bb7b53f3..869e906396ae 100644
--- a/drivers/nvme/nvme.c
+++ b/drivers/nvme/nvme.c
@@ -821,9 +821,34 @@ static ulong nvme_blk_write(struct udevice *udev, lbaint_t blknr,
 	return nvme_blk_rw(udev, blknr, blkcnt, (void *)buffer, false);
 }
 
+/*
+ * NVM Flush command (opcode 0x00).
+ *
+ * Applies to a single namespace; the controller must commit all dirty
+ * data for that namespace to storage before completing the command.
+ */
+static ulong nvme_blk_flush(struct udevice *udev)
+{
+	struct nvme_ns *ns = dev_get_priv(udev);
+	struct nvme_dev *dev = ns->dev;
+
+	if (dev->flags & NVME_VWC_ENABLED) {
+		struct nvme_command c;
+
+		memset(&c, 0, sizeof(c));
+		c.common.opcode = nvme_cmd_flush;
+		c.common.nsid = cpu_to_le32(ns->ns_id);
+
+		return nvme_submit_sync_cmd(dev->queues[NVME_IO_Q], &c, NULL, IO_TIMEOUT);
+	}
+
+	return 0;
+}
+
 static const struct blk_ops nvme_blk_ops = {
 	.read	= nvme_blk_read,
 	.write	= nvme_blk_write,
+	.flush	= nvme_blk_flush,
 };
 
 U_BOOT_DRIVER(nvme_blk) = {
-- 
2.54.0

[PATCH v3 4/7] cmd: Add flush support for all blk devices

[dmukhin]

From: Denis Mukhin <dmukhin@ford.com> 

Introduce `flush` subcommand for all blk devices to allow committing
dirty data explicitly to the given block device.

Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
Changes since v2:
- addressed feedback:
  https://lore.kernel.org/u-boot/CAFLszTgO6Gbg+TrpSwyTffPVjd5Pv8Pw79eq7RG_gj0W-E2uqg@mail.gmail.com/

Changes since v1:
- updated commit message
- updated blk commands to enable flush
---
 cmd/blk_common.c | 17 +++++++++++++++++
 cmd/blkmap.c     |  2 ++
 cmd/ide.c        |  1 +
 cmd/nvme.c       |  1 +
 cmd/pvblock.c    |  1 +
 cmd/sata.c       |  1 +
 cmd/scsi.c       |  1 +
 cmd/usb.c        |  1 +
 cmd/virtio.c     |  1 +
 9 files changed, 26 insertions(+)

diff --git a/cmd/blk_common.c b/cmd/blk_common.c
index 56529702a470..ea6c407eb222 100644
--- a/cmd/blk_common.c
+++ b/cmd/blk_common.c
@@ -37,6 +37,23 @@ int blk_common_cmd(int argc, char *const argv[], enum uclass_id uclass_id,
 				printf("\nno %s partition table available\n",
 				       if_name);
 			return CMD_RET_SUCCESS;
+		} else if (strncmp(argv[1], "flush", 5) == 0) {
+			struct blk_desc *desc;
+			int ret;
+
+			ret = blk_get_desc(uclass_id, *cur_devnump, &desc);
+			if (ret)
+				return CMD_RET_FAILURE;
+
+			ret = blk_dflush(desc);
+			if (ret) {
+				printf("\nfailed to flush device %d (%s): %d\n",
+					   *cur_devnump, if_name, ret);
+				return CMD_RET_FAILURE;
+			}
+			printf("\nsuccess!\n");
+
+			return CMD_RET_SUCCESS;
 		}
 		return CMD_RET_USAGE;
 	case 3:
diff --git a/cmd/blkmap.c b/cmd/blkmap.c
index 65edec899e2c..828754cf6ac8 100644
--- a/cmd/blkmap.c
+++ b/cmd/blkmap.c
@@ -220,6 +220,7 @@ U_BOOT_CMD_WITH_SUBCMDS(
 	blkmap, "Composeable virtual block devices",
 	"info - list configured devices\n"
 	"blkmap part - list available partitions on current blkmap device\n"
+	"blkmap flush - commit all dirty data to current blkmap device\n"
 	"blkmap dev [<dev>] - show or set current blkmap device\n"
 	"blkmap read <addr> <blk#> <cnt>\n"
 	"blkmap write <addr> <blk#> <cnt>\n"
@@ -230,6 +231,7 @@ U_BOOT_CMD_WITH_SUBCMDS(
 	"blkmap map <label> <blk#> <cnt> mem <addr> [preserve] - memory mapping\n",
 	U_BOOT_SUBCMD_MKENT(info, 2, 1, do_blkmap_common),
 	U_BOOT_SUBCMD_MKENT(part, 2, 1, do_blkmap_common),
+	U_BOOT_SUBCMD_MKENT(flush, 2, 1, do_blkmap_common),
 	U_BOOT_SUBCMD_MKENT(dev, 4, 1, do_blkmap_common),
 	U_BOOT_SUBCMD_MKENT(read, 5, 1, do_blkmap_common),
 	U_BOOT_SUBCMD_MKENT(write, 5, 1, do_blkmap_common),
diff --git a/cmd/ide.c b/cmd/ide.c
index ed30f9468660..3f38c1e633fe 100644
--- a/cmd/ide.c
+++ b/cmd/ide.c
@@ -67,6 +67,7 @@ U_BOOT_CMD(ide, 5, 1, do_ide,
 	   "IDE sub-system",
 	   "reset - reset IDE controller\n"
 	   "ide info  - show available IDE devices\n"
+	   "ide flush - commit all dirty data to the current IDE device\n"
 	   "ide device [dev] - show or set current device\n"
 	   "ide part [dev] - print partition table of one or all IDE devices\n"
 	   "ide read  addr blk# cnt\n"
diff --git a/cmd/nvme.c b/cmd/nvme.c
index f2c9acba5c32..dbccb69042bc 100644
--- a/cmd/nvme.c
+++ b/cmd/nvme.c
@@ -47,6 +47,7 @@ U_BOOT_CMD(
 	"scan - scan NVMe devices\n"
 	"nvme detail - show details of current NVMe device\n"
 	"nvme info - show all available NVMe devices\n"
+	"nvme flush - commit all dirty data to the current NVMe device\n"
 	"nvme device [dev] - show or set current NVMe device\n"
 	"nvme part [dev] - print partition table of one or all NVMe devices\n"
 	"nvme read addr blk# cnt - read `cnt' blocks starting at block\n"
diff --git a/cmd/pvblock.c b/cmd/pvblock.c
index 3a83ac9cd92c..5e4a98a6b13b 100644
--- a/cmd/pvblock.c
+++ b/cmd/pvblock.c
@@ -20,6 +20,7 @@ int do_pvblock(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
 U_BOOT_CMD(pvblock, 5, 1, do_pvblock,
 	   "Xen para-virtualized block device",
 	   "info  - show available block devices\n"
+	   "pvblock flush - commit all dirty data to the current device\n"
 	   "pvblock device [dev] - show or set current device\n"
 	   "pvblock part [dev] - print partition table of one or all devices\n"
 	   "pvblock read  addr blk# cnt\n"
diff --git a/cmd/sata.c b/cmd/sata.c
index 8b923f9378b2..417e027cba4f 100644
--- a/cmd/sata.c
+++ b/cmd/sata.c
@@ -119,6 +119,7 @@ U_BOOT_CMD(
 	"init - init SATA sub system\n"
 	"sata stop [dev] - disable SATA sub system or device\n"
 	"sata info - show available SATA devices\n"
+	"sata flush - commit all dirty data to the current SATA device\n"
 	"sata device [dev] - show or set current device\n"
 	"sata part [dev] - print partition table\n"
 	"sata read addr blk# cnt\n"
diff --git a/cmd/scsi.c b/cmd/scsi.c
index 9f7613424e59..05850726a87a 100644
--- a/cmd/scsi.c
+++ b/cmd/scsi.c
@@ -55,6 +55,7 @@ U_BOOT_CMD(
 	"reset - reset SCSI controller\n"
 	"scsi info  - show available SCSI devices\n"
 	"scsi scan  - (re-)scan SCSI bus\n"
+	"scsi flush - commit all dirty data to the current SCSI device\n"
 	"scsi device [dev] - show or set current device\n"
 	"scsi part [dev] - print partition table of one or all SCSI devices\n"
 	"scsi read addr blk# cnt - read `cnt' blocks starting at block `blk#'\n"
diff --git a/cmd/usb.c b/cmd/usb.c
index 13a2996c1f00..3e156f54c50b 100644
--- a/cmd/usb.c
+++ b/cmd/usb.c
@@ -706,6 +706,7 @@ U_BOOT_CMD(
 	"    Available modes: J, K, S[E0_NAK], P[acket], F[orce_Enable]\n"
 #ifdef CONFIG_USB_STORAGE
 	"usb storage - show details of USB storage devices\n"
+	"usb flush - commit all dirty data to the current USB storage\n"
 	"usb dev [dev] - show or set current USB storage device\n"
 	"usb part [dev] - print partition table of one or all USB storage"
 	"    devices\n"
diff --git a/cmd/virtio.c b/cmd/virtio.c
index a42a563ab727..94f1e82cc902 100644
--- a/cmd/virtio.c
+++ b/cmd/virtio.c
@@ -44,6 +44,7 @@ U_BOOT_CMD(
 	"virtio block devices sub-system",
 	"scan - initialize virtio bus\n"
 	"virtio info - show all available virtio block devices\n"
+	"virtio flush - commit all dirty data to the current virtio block device\n"
 	"virtio device [dev] - show or set current virtio block device\n"
 	"virtio part [dev] - print partition table of one or all virtio block devices\n"
 	"virtio read addr blk# cnt - read `cnt' blocks starting at block\n"
-- 
2.54.0

[PATCH v3 5/7] drivers: nvme: Export nvme_shutdown()

[dmukhin]

From: Denis Mukhin <dmukhin@ford.com> 

Make nvme_shutdown() public so that it can be re-used in
board-specific board_quiesce_devices() before OS hand-off.

Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
Changes since v2:
- fixup for apple device

Changes since v1:
- n/a
---
 drivers/nvme/nvme.h       |  7 -------
 drivers/nvme/nvme_apple.c |  1 +
 include/nvme.h            | 11 +++++++++++
 3 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/nvme/nvme.h b/drivers/nvme/nvme.h
index a938da728506..cc1477717391 100644
--- a/drivers/nvme/nvme.h
+++ b/drivers/nvme/nvme.h
@@ -703,11 +703,4 @@ struct nvme_ops {
  */
 int nvme_init(struct udevice *udev);
 
-/**
- * nvme_shutdown() - Shutdown NVM Express device
- * @udev:	The NVM Express device
- * Return: 0 if OK, -ve on error
- */
-int nvme_shutdown(struct udevice *udev);
-
 #endif /* __DRIVER_NVME_H__ */
diff --git a/drivers/nvme/nvme_apple.c b/drivers/nvme/nvme_apple.c
index 7e7538553e3f..a36f15a3ff4e 100644
--- a/drivers/nvme/nvme_apple.c
+++ b/drivers/nvme/nvme_apple.c
@@ -6,6 +6,7 @@
 #include <dm.h>
 #include <mailbox.h>
 #include <mapmem.h>
+#include <nvme.h>
 #include "nvme.h"
 #include <reset.h>
 
diff --git a/include/nvme.h b/include/nvme.h
index 2cdf8ce320c1..c7b301300097 100644
--- a/include/nvme.h
+++ b/include/nvme.h
@@ -90,4 +90,15 @@ int nvme_print_info(struct udevice *udev);
  */
 int nvme_get_namespace_id(struct udevice *udev, u32 *ns_id, u8 *eui64);
 
+/**
+ * nvme_shutdown() - Shutdown NVM Express device
+ *
+ * Must be called before booting an OS to ensure cache is flushed
+ * and the controller is in a clean state for OS to re-initialize.
+ *
+ * @udev:	The NVM Express device
+ * Return: 0 if OK, -ve on error
+ */
+int nvme_shutdown(struct udevice *udev);
+
 #endif /* __NVME_H__ */
-- 
2.54.0

[PATCH v3 6/7] docs: nvme: Update QEMU command for testing

[dmukhin]

From: Denis Mukhin <dmukhin@ford.com> 

Switch to x86-64 QEMU machine for NVMe testing in the docs.
Add `format=raw` for empty NVMe disks QEMU command line.

Also, add -nographic for terminal based QEMU start.

Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
Changes since v2:
- corrected commit message

Changes since v1:
- n/a
---
 doc/develop/driver-model/nvme.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/develop/driver-model/nvme.rst b/doc/develop/driver-model/nvme.rst
index 755181331213..0e0624925e0d 100644
--- a/doc/develop/driver-model/nvme.rst
+++ b/doc/develop/driver-model/nvme.rst
@@ -95,4 +95,4 @@ Example command line to call QEMU x86 below with emulated NVMe device:
 
 .. code-block:: bash
 
-  $ ./qemu-system-i386 -drive file=nvme.img,if=none,id=drv0 -device nvme,drive=drv0,serial=QEMUNVME0001 -bios u-boot.rom
+  $ qemu-system-x86_64 -nographic -drive file=nvme.img,format=raw,if=none,id=drv0 -device nvme,drive=drv0,serial=QEMUNVME0001 -bios u-boot.rom
-- 
2.54.0

[PATCH v3 7/7] tests: add blk_dflush() coverage

[dmukhin]

From: Denis Mukhin <dmukhin@ford.com> 

Add minimal test invoking new blk_dflush() API.

Signed-off-by: Denis Mukhin <dmukhin@ford.com>
---
Changes since v2:
- new patch
---
 arch/sandbox/cpu/os.c   | 5 +++++
 drivers/block/sandbox.c | 9 +++++++++
 include/os.h            | 8 ++++++++
 test/dm/host.c          | 2 ++
 test/dm/mmc.c           | 2 ++
 5 files changed, 26 insertions(+)

diff --git a/arch/sandbox/cpu/os.c b/arch/sandbox/cpu/os.c
index e48eb23cdc01..a39e0fcab795 100644
--- a/arch/sandbox/cpu/os.c
+++ b/arch/sandbox/cpu/os.c
@@ -139,6 +139,11 @@ int os_close(int fd)
 	return -1;
 }
 
+int os_fsync(int fd)
+{
+    return fsync(fd);
+}
+
 int os_unlink(const char *pathname)
 {
 	return unlink(pathname);
diff --git a/drivers/block/sandbox.c b/drivers/block/sandbox.c
index 9cb27561a97f..f7e854fbf982 100644
--- a/drivers/block/sandbox.c
+++ b/drivers/block/sandbox.c
@@ -55,9 +55,18 @@ static unsigned long host_block_write(struct udevice *dev,
 	return -EIO;
 }
 
+unsigned long host_block_flush(struct udevice *dev)
+{
+	struct udevice *host_dev = dev_get_parent(dev);
+	struct host_sb_plat *plat = dev_get_plat(host_dev);
+
+	return os_fsync(plat->fd);
+}
+
 static const struct blk_ops sandbox_host_blk_ops = {
 	.read	= host_block_read,
 	.write	= host_block_write,
+	.flush	= host_block_flush,
 };
 
 U_BOOT_DRIVER(sandbox_host_blk) = {
diff --git a/include/os.h b/include/os.h
index ae3ca6d42a2a..13c7651bf537 100644
--- a/include/os.h
+++ b/include/os.h
@@ -90,6 +90,14 @@ int os_open(const char *pathname, int flags);
  */
 int os_close(int fd);
 
+/**
+ * os_fsync() - synchronize a file's in-core state with storage device
+ *
+ * @fd:	File descriptor to synchronize
+ * Return:	0 on success, negative error code - otherwise
+ */
+int os_fsync(int fd);
+
 /**
  * os_unlink() - access to the OS unlink() system call
  *
diff --git a/test/dm/host.c b/test/dm/host.c
index f577377da6a0..ed55278a701e 100644
--- a/test/dm/host.c
+++ b/test/dm/host.c
@@ -62,6 +62,8 @@ static int dm_test_host(struct unit_test_state *uts)
 	ut_assertok(fs_set_blk_dev_with_part(desc, 0));
 	ut_assertok(fs_write("/testing", 0, 0, 0x1000, &actwrite));
 
+	ut_asserteq(0, blk_dflush(desc));
+
 	ut_assertok(host_detach_file(dev));
 	ut_asserteq(0, plat->fd);
 	ut_asserteq(-ENODEV, blk_get_from_parent(dev, &blk));
diff --git a/test/dm/mmc.c b/test/dm/mmc.c
index cdebb9551967..714fcd1ea508 100644
--- a/test/dm/mmc.c
+++ b/test/dm/mmc.c
@@ -48,6 +48,8 @@ static int dm_test_mmc_blk(struct unit_test_state *uts)
 	ut_asserteq(4, blk_dread(dev_desc, 0, 4, read));
 	ut_asserteq_mem(write, read, sizeof(write));
 
+	ut_asserteq(-ENOSYS, blk_dflush(dev_desc));
+
 	return 0;
 }
 DM_TEST(dm_test_mmc_blk, UTF_SCAN_PDATA | UTF_SCAN_FDT);
-- 
2.54.0

Re: [PATCH v3 0/7] few fixes for NVMe and block devices

[neil.armstrong]

Hi,

On 7/17/26 00:33, dmukhin@ford.com wrote:
> The series adds few fixes for NVMe subsystem and block device management.
> 
> Patch 1 adds FUA support for NVMe devices (original effort in [1]).
> Patch 2 adds plumbing for blk_flush()/blk_dflush().
> Patch 3 introduces NVMe flush command.
> Patch 4 enables `flush` subcommand to all the block-uclass front-end
>          commands (ide, nvme, pvblock, sata, scsi, usb, virtio) in one go.
> Patch 5 makes nvme_shutdown() symbol public to enable use of it from
>          board_quiesce_devices()
> Patch 6 tiny fixup for QEMU command for manual NVMe tests.
> Patch 7 updates tests for flushing block devices.

Thanks for updating, but now you enable FUA, why do you still add the
flush infrastructure ?

With FUA enabled, calling flush command would be a no-op.

Neil

> 
> [1] https://lore.kernel.org/u-boot/20211019104049.v3.1.Ic581ec99f46b6dfa2e0b1922e670a333ac859e82@changeid/
> [2] Link to v2: https://lore.kernel.org/u-boot/20260529034441.2075305-1-dmukhin@ford.com/
> [3] Link to CI: https://github.com/u-boot/u-boot/pull/995
> 
> Denis Mukhin (7):
>    drivers: nvme: Enable Force Unit Access (FUA)
>    drivers: block: Introduce blk_flush()/blk_dflush()
>    drivers: nvme: Implement flush command
>    cmd: Add flush support for all blk devices
>    drivers: nvme: Export nvme_shutdown()
>    docs: nvme: Update QEMU command for testing
>    tests: add blk_dflush() coverage
> 
>   arch/sandbox/cpu/os.c             |  5 ++++
>   cmd/blk_common.c                  | 17 +++++++++++++
>   cmd/blkmap.c                      |  2 ++
>   cmd/ide.c                         |  1 +
>   cmd/nvme.c                        |  1 +
>   cmd/pvblock.c                     |  1 +
>   cmd/sata.c                        |  1 +
>   cmd/scsi.c                        |  1 +
>   cmd/usb.c                         |  1 +
>   cmd/virtio.c                      |  1 +
>   disk/disk-uclass.c                |  6 +++++
>   doc/develop/driver-model/nvme.rst |  2 +-
>   drivers/block/blk-uclass.c        | 15 +++++++++++
>   drivers/block/sandbox.c           |  9 +++++++
>   drivers/nvme/nvme.c               | 42 +++++++++++++++++++++++++++++++
>   drivers/nvme/nvme.h               | 12 ++++-----
>   drivers/nvme/nvme_apple.c         |  1 +
>   include/blk.h                     | 26 +++++++++++++++++++
>   include/nvme.h                    | 11 ++++++++
>   include/os.h                      |  8 ++++++
>   include/part.h                    |  8 ++++++
>   test/dm/host.c                    |  2 ++
>   test/dm/mmc.c                     |  2 ++
>   23 files changed, 167 insertions(+), 8 deletions(-)
> 

Re: [PATCH v3 5/7] drivers: nvme: Export nvme_shutdown()

[neil.armstrong]

On 7/17/26 00:33, dmukhin@ford.com wrote:
> From: Denis Mukhin <dmukhin@ford.com>
> 
> Make nvme_shutdown() public so that it can be re-used in
> board-specific board_quiesce_devices() before OS hand-off.
> 
> Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> ---
> Changes since v2:
> - fixup for apple device
> 
> Changes since v1:
> - n/a
> ---
>   drivers/nvme/nvme.h       |  7 -------
>   drivers/nvme/nvme_apple.c |  1 +
>   include/nvme.h            | 11 +++++++++++
>   3 files changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/nvme/nvme.h b/drivers/nvme/nvme.h
> index a938da728506..cc1477717391 100644
> --- a/drivers/nvme/nvme.h
> +++ b/drivers/nvme/nvme.h
> @@ -703,11 +703,4 @@ struct nvme_ops {
>    */
>   int nvme_init(struct udevice *udev);
>   
> -/**
> - * nvme_shutdown() - Shutdown NVM Express device
> - * @udev:	The NVM Express device
> - * Return: 0 if OK, -ve on error
> - */
> -int nvme_shutdown(struct udevice *udev);
> -
>   #endif /* __DRIVER_NVME_H__ */
> diff --git a/drivers/nvme/nvme_apple.c b/drivers/nvme/nvme_apple.c
> index 7e7538553e3f..a36f15a3ff4e 100644
> --- a/drivers/nvme/nvme_apple.c
> +++ b/drivers/nvme/nvme_apple.c
> @@ -6,6 +6,7 @@
>   #include <dm.h>
>   #include <mailbox.h>
>   #include <mapmem.h>
> +#include <nvme.h>
>   #include "nvme.h"
>   #include <reset.h>
>   
> diff --git a/include/nvme.h b/include/nvme.h
> index 2cdf8ce320c1..c7b301300097 100644
> --- a/include/nvme.h
> +++ b/include/nvme.h
> @@ -90,4 +90,15 @@ int nvme_print_info(struct udevice *udev);
>    */
>   int nvme_get_namespace_id(struct udevice *udev, u32 *ns_id, u8 *eui64);
>   
> +/**
> + * nvme_shutdown() - Shutdown NVM Express device
> + *
> + * Must be called before booting an OS to ensure cache is flushed
> + * and the controller is in a clean state for OS to re-initialize.
> + *
> + * @udev:	The NVM Express device
> + * Return: 0 if OK, -ve on error
> + */
> +int nvme_shutdown(struct udevice *udev);
> +
>   #endif /* __NVME_H__ */

Could you instead add the DM_FLAG_ACTIVE_DMA flag to the PCI NVMe driver and
then call nvme_shutdown() from a new nvem_remove in nvem_pci.c

This will automatically shutdown the NVMe devices when booting an OS and you could
trigger this manually by calling:

dm_remove_devices_flags(DM_REMOVE_ACTIVE_DMA)

Neil

Re: [PATCH v3 0/7] few fixes for NVMe and block devices

[Denis Mukhin via U-Boot]

On Mon, Jul 20, 2026 at 11:33:02AM +0200, neil.armstrong@linaro.org wrote:
> Hi,
> 
> On 7/17/26 00:33, dmukhin@ford.com wrote:
> > The series adds few fixes for NVMe subsystem and block device management.
> > 
> > Patch 1 adds FUA support for NVMe devices (original effort in [1]).
> > Patch 2 adds plumbing for blk_flush()/blk_dflush().
> > Patch 3 introduces NVMe flush command.
> > Patch 4 enables `flush` subcommand to all the block-uclass front-end
> >          commands (ide, nvme, pvblock, sata, scsi, usb, virtio) in one go.
> > Patch 5 makes nvme_shutdown() symbol public to enable use of it from
> >          board_quiesce_devices()
> > Patch 6 tiny fixup for QEMU command for manual NVMe tests.
> > Patch 7 updates tests for flushing block devices.
> 
> Thanks for updating, but now you enable FUA, why do you still add the
> flush infrastructure ?
> 
> With FUA enabled, calling flush command would be a no-op.

I forgot to plumb enabling FUA behind the Kconfig knob, since not
every bootflow needs to persist data on NVMe...

re: flush: My understanding that flush() infra in general _could_ be useful.
That's why I kept it in the series.
But I agree, w/ NVMe FUA enabled, flush is redundant.

> 
> Neil
> 
> > 
> > [1] https://lore.kernel.org/u-boot/20211019104049.v3.1.Ic581ec99f46b6dfa2e0b1922e670a333ac859e82@changeid/
> > [2] Link to v2: https://lore.kernel.org/u-boot/20260529034441.2075305-1-dmukhin@ford.com/
> > [3] Link to CI: https://github.com/u-boot/u-boot/pull/995
> > 
> > Denis Mukhin (7):
> >    drivers: nvme: Enable Force Unit Access (FUA)
> >    drivers: block: Introduce blk_flush()/blk_dflush()
> >    drivers: nvme: Implement flush command
> >    cmd: Add flush support for all blk devices
> >    drivers: nvme: Export nvme_shutdown()
> >    docs: nvme: Update QEMU command for testing
> >    tests: add blk_dflush() coverage
> > 
> >   arch/sandbox/cpu/os.c             |  5 ++++
> >   cmd/blk_common.c                  | 17 +++++++++++++
> >   cmd/blkmap.c                      |  2 ++
> >   cmd/ide.c                         |  1 +
> >   cmd/nvme.c                        |  1 +
> >   cmd/pvblock.c                     |  1 +
> >   cmd/sata.c                        |  1 +
> >   cmd/scsi.c                        |  1 +
> >   cmd/usb.c                         |  1 +
> >   cmd/virtio.c                      |  1 +
> >   disk/disk-uclass.c                |  6 +++++
> >   doc/develop/driver-model/nvme.rst |  2 +-
> >   drivers/block/blk-uclass.c        | 15 +++++++++++
> >   drivers/block/sandbox.c           |  9 +++++++
> >   drivers/nvme/nvme.c               | 42 +++++++++++++++++++++++++++++++
> >   drivers/nvme/nvme.h               | 12 ++++-----
> >   drivers/nvme/nvme_apple.c         |  1 +
> >   include/blk.h                     | 26 +++++++++++++++++++
> >   include/nvme.h                    | 11 ++++++++
> >   include/os.h                      |  8 ++++++
> >   include/part.h                    |  8 ++++++
> >   test/dm/host.c                    |  2 ++
> >   test/dm/mmc.c                     |  2 ++
> >   23 files changed, 167 insertions(+), 8 deletions(-)
> > 
> 

Re: [PATCH v3 5/7] drivers: nvme: Export nvme_shutdown()

[Denis Mukhin via U-Boot]

On Mon, Jul 20, 2026 at 11:42:31AM +0200, neil.armstrong@linaro.org wrote:
> On 7/17/26 00:33, dmukhin@ford.com wrote:
> > From: Denis Mukhin <dmukhin@ford.com>
> > 
> > Make nvme_shutdown() public so that it can be re-used in
> > board-specific board_quiesce_devices() before OS hand-off.
> > 
> > Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> > ---
> > Changes since v2:
> > - fixup for apple device
> > 
> > Changes since v1:
> > - n/a
> > ---
> >   drivers/nvme/nvme.h       |  7 -------
> >   drivers/nvme/nvme_apple.c |  1 +
> >   include/nvme.h            | 11 +++++++++++
> >   3 files changed, 12 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/nvme/nvme.h b/drivers/nvme/nvme.h
> > index a938da728506..cc1477717391 100644
> > --- a/drivers/nvme/nvme.h
> > +++ b/drivers/nvme/nvme.h
> > @@ -703,11 +703,4 @@ struct nvme_ops {
> >    */
> >   int nvme_init(struct udevice *udev);
> > -/**
> > - * nvme_shutdown() - Shutdown NVM Express device
> > - * @udev:	The NVM Express device
> > - * Return: 0 if OK, -ve on error
> > - */
> > -int nvme_shutdown(struct udevice *udev);
> > -
> >   #endif /* __DRIVER_NVME_H__ */
> > diff --git a/drivers/nvme/nvme_apple.c b/drivers/nvme/nvme_apple.c
> > index 7e7538553e3f..a36f15a3ff4e 100644
> > --- a/drivers/nvme/nvme_apple.c
> > +++ b/drivers/nvme/nvme_apple.c
> > @@ -6,6 +6,7 @@
> >   #include <dm.h>
> >   #include <mailbox.h>
> >   #include <mapmem.h>
> > +#include <nvme.h>
> >   #include "nvme.h"
> >   #include <reset.h>
> > diff --git a/include/nvme.h b/include/nvme.h
> > index 2cdf8ce320c1..c7b301300097 100644
> > --- a/include/nvme.h
> > +++ b/include/nvme.h
> > @@ -90,4 +90,15 @@ int nvme_print_info(struct udevice *udev);
> >    */
> >   int nvme_get_namespace_id(struct udevice *udev, u32 *ns_id, u8 *eui64);
> > +/**
> > + * nvme_shutdown() - Shutdown NVM Express device
> > + *
> > + * Must be called before booting an OS to ensure cache is flushed
> > + * and the controller is in a clean state for OS to re-initialize.
> > + *
> > + * @udev:	The NVM Express device
> > + * Return: 0 if OK, -ve on error
> > + */
> > +int nvme_shutdown(struct udevice *udev);
> > +
> >   #endif /* __NVME_H__ */
> 
> Could you instead add the DM_FLAG_ACTIVE_DMA flag to the PCI NVMe driver and
> then call nvme_shutdown() from a new nvem_remove in nvem_pci.c

Thanks for suggestion!
Will check that path.

> 
> This will automatically shutdown the NVMe devices when booting an OS and you could
> trigger this manually by calling:
> 
> dm_remove_devices_flags(DM_REMOVE_ACTIVE_DMA)
> 
> Neil

Re: [PATCH v3 1/7] drivers: nvme: Enable Force Unit Access (FUA)

[Simon Glass]

Hi Denis,

On 2026-07-16T22:33:54, None <dmukhin@ford.com> wrote:
> drivers: nvme: Enable Force Unit Access (FUA)
>
> Enable FUA (if present) on NVMe devices to prevent any potential data
> integrity issues caused by accidental power loss or malformed device
> handoff sequence to the OS.
>
> Original FUA effort has been started in [1]
>
> [1] https://lore.kernel.org/u-boot/20211019104049.v3.1.Ic581ec99f46b6dfa2e0b1922e670a333ac859e82@changeid/

Oddly that link doesn't work for me.

> Original-patch-by: Jon Lin <jon.lin@rock-chips.com>
> Signed-off-by: Denis Mukhin <dmukhin@ford.com>
>
> drivers/nvme/nvme.c | 17 +++++++++++++++++
>  drivers/nvme/nvme.h |  5 +++++
>  2 files changed, 22 insertions(+)

> diff --git a/drivers/nvme/nvme.c b/drivers/nvme/nvme.c
> @@ -916,6 +923,16 @@ int nvme_init(struct udevice *udev)
> +             /* Check whether volatile write cache is enabled. */
> +             if (ndev->vwc & NVME_CTRL_VWC_PRESENT) {
> +                     u32 features;
> +
> +                     ret = nvme_get_features(ndev, NVME_FEAT_VOLATILE_WC, 0, 0,
> +                                             &features);
> +                     if (!ret && (features & 0x1))
> +                             ndev->flags |= NVME_VWC_ENABLED;
> +             }

The volatile write cache is a controller-level property, not
per-namespace, but this sits inside the namespace loop, so the same Get
Features command is issued for every active namespace. Please can you
move it out of the loop, e.g. into nvme_get_info_from_identify() next
to where dev->vwc is read from the identify data?

Also, 0x1 is the WCE bit from the spec, so please add a named constant
in nvme.h rather than a magic number.

BTW this adds the first caller of nvme_get_features(), so please drop
the stale 'At the moment there is no user of this function' sentence
from the TODO comment there.

> diff --git a/drivers/nvme/nvme.c b/drivers/nvme/nvme.c
> @@ -788,6 +788,13 @@ static ulong nvme_blk_rw(struct udevice *udev, lbaint_t blknr,
> +     /*
> +      * Enable force unit access (FUA) for data integrity if volatile write
> +      * cache (VWC) is enabled.
> +      */
> +     if (!read && (dev->flags & NVME_VWC_ENABLED))
> +             c.rw.control |= cpu_to_le16(NVME_RW_FUA);

Just to check the design here: patch 3 implements the flush command and
patch 5 exports nvme_shutdown(), which already flushes the cache before
hand-off. With those in place, per-write FUA makes each write
synchronous to media, which defeats the write cache and can slow down
writing large images considerably. My understanding is that flushing at
the appropriate points (explicit flush command, shutdown) gives the
same integrity guarantee without the per-write cost. Could this patch
be dropped once the rest of the series lands? What do you think?

Regards,
Simon

Re: [PATCH v3 2/7] drivers: block: Introduce blk_flush()/blk_dflush()

[Simon Glass]

Hi Denis,

On 2026-07-16T22:33:54, None <dmukhin@ford.com> wrote:
> drivers: block: Introduce blk_flush()/blk_dflush()
>
> Add generic flush operations for committing dirty data to the storage
> device.
>
> This provides a common block-layer interface for flushing pending
> writes, allowing callers to ensure that data buffered by the block
> device or its backing implementation is written out to the underlying
> storage.
>
> Note, flushing a partition device via disk_blk_flush() flushes the entire
> underlying device, so callers don't expect partition-scoped semantics.
>
> Signed-off-by: Denis Mukhin <dmukhin@ford.com>
>
> disk/disk-uclass.c         |  6 ++++++
>  drivers/block/blk-uclass.c | 15 +++++++++++++++
>  include/blk.h              | 26 ++++++++++++++++++++++++++
>  include/part.h             |  8 ++++++++
>  4 files changed, 55 insertions(+)

> Note, flushing a partition device via disk_blk_flush() flushes the entire
> underlying device, so callers don't expect partition-scoped semantics.

I suspect you mean 'so callers should not expect partition-scoped
semantics' - please can you reword this?

> diff --git a/include/blk.h b/include/blk.h
> @@ -239,6 +240,14 @@ struct blk_ops {
> +     /**
> +      * flush() - commit all dirty data to storage
> +      *
> +      * @dev:        Device to flush
> +      * @return 0 if OK, -ve on error
> +      */
> +     unsigned long (*flush)(struct udevice *dev);

The read/write/erase ops use ulong because they return a block count,
but flush() only ever returns 0 or an error, so please can you make it
(and blk_flush(), blk_dflush() and disk_blk_flush()) return int? That
matches select_hwpart() just below, and avoids errors from drivers
(e.g. nvme_blk_flush() in patch 3) being silently converted to large
positive values at each layer.

> diff --git a/disk/disk-uclass.c b/disk/disk-uclass.c
> @@ -122,6 +122,11 @@ unsigned long disk_blk_erase(struct udevice *dev, lbaint_t start,
> +unsigned long disk_blk_flush(struct udevice *dev)
> +{
> +     return blk_flush(dev_get_parent(dev));
> +}

The other functions in this file (disk_blk_read() etc.) have a
function comment. Please can you add one here, mentioning that this
flushes the whole underlying device, not just the partition? That is
worth stating at the implementation, not just in the commit message.

> diff --git a/include/part.h b/include/part.h
> @@ -454,6 +454,14 @@ ulong disk_blk_erase(struct udevice *dev, lbaint_t start, lbaint_t blkcnt);
> + * Return:   0 success, or -ve error number (see the IS_ERR_VALUE()) macro

The closing paren is misplaced - it should be '(see the IS_ERR_VALUE()
macro)'. Also '0 on success'.

Regards,
Simon

Re: [PATCH v3 7/7] tests: add blk_dflush() coverage

[Simon Glass]

Hi Denis,

On 2026-07-16T22:33:54, None <dmukhin@ford.com> wrote:
> tests: add blk_dflush() coverage
>
> Add minimal test invoking new blk_dflush() API.
>
> Signed-off-by: Denis Mukhin <dmukhin@ford.com>
>
> arch/sandbox/cpu/os.c   | 5 +++++
>  drivers/block/sandbox.c | 9 +++++++++
>  include/os.h            | 8 ++++++++
>  test/dm/host.c          | 2 ++
>  test/dm/mmc.c           | 2 ++
>  5 files changed, 26 insertions(+)

> tests: add blk_dflush() coverage

The subject says tests, but this patch also adds os_fsync() and
implements flush support in the sandbox host block driver. Please can you
split the sandbox bits into its own sandbox: patch, before the tests?

> diff --git a/drivers/block/sandbox.c b/drivers/block/sandbox.c
> @@ -52,9 +52,18 @@ static unsigned long host_block_write(struct udevice *dev,
> +unsigned long host_block_flush(struct udevice *dev)
> +{

This should be static, to match host_block_read() and
host_block_write() - it is only referenced via the ops table.

> diff --git a/arch/sandbox/cpu/os.c b/arch/sandbox/cpu/os.c
> @@ -139,6 +139,11 @@ int os_close(int fd)
> +int os_fsync(int fd)
> +{
> +    return fsync(fd);
> +}

The function comment in os.h says this returns a negative error code,
but fsync() returns -1 with errno set. Since blk_flush() propagates
this value to callers, please can you return -errno on failure so the
caller gets a meaningful error?

> diff --git a/test/dm/host.c b/test/dm/host.c
> @@ -62,6 +62,8 @@ static int dm_test_host(struct unit_test_state *uts)
> +     ut_asserteq(0, blk_dflush(desc));
> +

This covers the direct blk_dflush() path, but nothing exercises the
partition-level disk_blk_flush() path added in patch 2, where flushing
a partition flushes the whole device. Would a test for that be
worthwhile?

Regards,
Simon

Re: [v3,0/7] few fixes for NVMe and block devices

[Simon Glass]

Hi Denis,

On 2026-07-16T22:33:54, None <dmukhin@ford.com> wrote:

> The series adds few fixes for NVMe subsystem and block device management.

This is really a new feature (block-device flush support) rather than
a set of fixes, so please can you describe it as such? It would also
help to explain the motivation in the cover letter, e.g. the OS
hand-off / power-loss scenario you mention in patch 1.

The new flush subcommand in patch 4 is user-visible, but I don't see
any doc update for it. Since you are adding a new feature you may need
to update the command documentation as well.

Regards,
Simon

Re: [PATCH v3 4/7] cmd: Add flush support for all blk devices

[Simon Glass]

Hi Denis,

On 2026-07-16T22:33:54, None <dmukhin@ford.com> wrote:
> cmd: Add flush support for all blk devices
>
> Introduce `flush` subcommand for all blk devices to allow committing
> dirty data explicitly to the given block device.
>
> Signed-off-by: Denis Mukhin <dmukhin@ford.com>
>
> cmd/blk_common.c | 17 +++++++++++++++++
>  cmd/blkmap.c     |  2 ++
>  cmd/ide.c        |  1 +
>  cmd/nvme.c       |  1 +
>  cmd/pvblock.c    |  1 +
>  cmd/sata.c       |  1 +
>  cmd/scsi.c       |  1 +
>  cmd/usb.c        |  1 +
>  cmd/virtio.c     |  1 +
>  9 files changed, 26 insertions(+)

> Introduce `flush` subcommand for all blk devices to allow committing
> dirty data explicitly to the given block device.

Since you are adding a new subcommand you may need a test for it, e.g. a
console test using run_command() and ut_assert_nextline() in test/dm/blk.c
or similar. Patch 7 covers blk_dflush() but nothing exercises the command
path in blk_common_cmd().

> diff --git a/cmd/blk_common.c b/cmd/blk_common.c
> @@ -37,6 +37,23 @@ int blk_common_cmd(int argc, char *const argv[], enum uclass_id uclass_id,
> +                     ret = blk_dflush(desc);
> +                     if (ret) {
> +                             printf("\nfailed to flush device %d (%s): %d\n",
> +                                        *cur_devnump, if_name, ret);
> +                             return CMD_RET_FAILURE;
> +                     }
> +                     printf("\nsuccess!\n");

Please can you drop the 'success!' message, or at least match the other
subcommands (the read/write handlers print '... OK')? U-Boot commands are
generally quiet on success and the return code is already available. Also
the continuation line mixes tabs and spaces rather than aligning with the
open parenthesis.

With those:

Reviewed-by: Simon Glass <sjg@chromium.org>

Regards,
Simon

Re: [PATCH v3 6/7] docs: nvme: Update QEMU command for testing

[Simon Glass]

On 2026-07-16T22:33:54, None <dmukhin@ford.com> wrote:
> docs: nvme: Update QEMU command for testing
>
> Switch to x86-64 QEMU machine for NVMe testing in the docs.
> Add `format=raw` for empty NVMe disks QEMU command line.
>
> Also, add -nographic for terminal based QEMU start.
>
> Signed-off-by: Denis Mukhin <dmukhin@ford.com>
>
> doc/develop/driver-model/nvme.rst | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Simon Glass <sjg@chromium.org>