[PATCH v1 0/6] introduce NVM XIP block storage emulation

[PATCH v1 0/6] introduce NVM XIP block storage emulation

[abdellatif.elkhlifi]

From: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>

Adding block storage emulation for NVM XIP flash devices.

Some paltforms such as Corstone-1000 need to see NVM XIP raw flash
as a block storage device with read only capability.

Here NVM flash devices are devices with addressable
memory (e.g: QSPI NOR flash).

The NVM XIP block storage emulation provides the following features:

- Emulate NVM XIP raw flash as a block storage device with read only capability
- Being generic by design and can be used by any platform
- Device tree node
- Platforms can use multiple NVM XIP devices at the same time by defining a
  DT node for each one of them
- A generic NVMXIP block driver allowing to read from the XIP flash
- A generic NVMXIP QSPI driver
- Implemented on top of memory-mapped IO (using readq macro)
- Enabling NVMXIP in sandbox64
- A sandbox test case
- Enabling NVMXIP in Corstone1000 platform as a use case

For more details please refer to the readme [A].

[A]: doc/develop/driver-model/nvmxip.rst

Cheers,
Abdellatif

Cc: Tom Rini <trini@konsulko.com>
Cc: Simon Glass <sjg@chromium.org>
Cc: Drew Reed <Drew.Reed@arm.com>
Cc: Xueliang Zhong <Xueliang.Zhong@arm.com>

Abdellatif El Khlifi (6):
  drivers/nvmxip: introduce NVM XIP block storage emulation
  sandbox64: fix: return unsigned long in readq()
  sandbox64: add support for NVMXIP QSPI
  corstone1000: add NVM XIP QSPI device tree node
  corstone1000: enable NVM XIP QSPI flash
  sandbox64: add a test case for UCLASS_NVMXIP

 MAINTAINERS                                |   8 ++
 arch/arm/dts/corstone1000.dtsi             |   9 +-
 arch/sandbox/cpu/cpu.c                     |   2 +-
 arch/sandbox/dts/sandbox64.dts             |  13 ++
 arch/sandbox/dts/test.dts                  |  14 +++
 arch/sandbox/include/asm/io.h              |   2 +-
 configs/corstone1000_defconfig             |   1 +
 configs/sandbox64_defconfig                |   1 +
 doc/develop/driver-model/index.rst         |   1 +
 doc/develop/driver-model/nvmxip.rst        |  70 +++++++++++
 doc/device-tree-bindings/nvmxip/nvmxip.txt |  56 +++++++++
 drivers/Kconfig                            |   2 +
 drivers/Makefile                           |   1 +
 drivers/block/blk-uclass.c                 |   1 +
 drivers/nvmxip/Kconfig                     |  19 +++
 drivers/nvmxip/Makefile                    |   8 ++
 drivers/nvmxip/nvmxip-uclass.c             |  15 +++
 drivers/nvmxip/nvmxip.c                    | 133 +++++++++++++++++++++
 drivers/nvmxip/nvmxip.h                    |  51 ++++++++
 drivers/nvmxip/nvmxip_qspi.c               |  67 +++++++++++
 include/dm/uclass-id.h                     |   1 +
 test/dm/Makefile                           |   5 +
 test/dm/nvmxip.c                           | 117 ++++++++++++++++++
 23 files changed, 594 insertions(+), 3 deletions(-)
 create mode 100644 doc/develop/driver-model/nvmxip.rst
 create mode 100644 doc/device-tree-bindings/nvmxip/nvmxip.txt
 create mode 100644 drivers/nvmxip/Kconfig
 create mode 100644 drivers/nvmxip/Makefile
 create mode 100644 drivers/nvmxip/nvmxip-uclass.c
 create mode 100644 drivers/nvmxip/nvmxip.c
 create mode 100644 drivers/nvmxip/nvmxip.h
 create mode 100644 drivers/nvmxip/nvmxip_qspi.c
 create mode 100644 test/dm/nvmxip.c

-- 
2.17.1

[PATCH v1 1/6] drivers/nvmxip: introduce NVM XIP block storage emulation

[abdellatif.elkhlifi]

From: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>

add block storage emulation for NVM XIP flash devices

Some paltforms such as Corstone-1000 need to see NVM XIP raw flash
as a block storage device with read only capability.

Here NVM flash devices are devices with addressable
memory (e.g: QSPI NOR flash).

The implementation is generic and can be used by different platforms.

Two drivers are provided as follows.

  nvmxip-blk :

    a generic block driver allowing to read from the XIP flash

  nvmxip_qspi :

    The driver probed with the DT and parent of the nvmxip-blk device.
    nvmxip_qspi can be reused by other platforms. If the platform
    has custom settings to apply before using the flash, then the platform
    can provide its own parent driver belonging to UCLASS_NVMXIP and reuse
    nvmxip-blk. The custom driver can be implmented like nvmxip_qspi in
    addition to the platform custom settings.

Platforms can use multiple NVM XIP devices at the same time by defining a
DT node for each one of them.

For more details please refer to doc/develop/driver-model/nvmxip.rst

Signed-off-by: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
---
 MAINTAINERS                                |   7 ++
 doc/develop/driver-model/index.rst         |   1 +
 doc/develop/driver-model/nvmxip.rst        |  70 +++++++++++
 doc/device-tree-bindings/nvmxip/nvmxip.txt |  56 +++++++++
 drivers/Kconfig                            |   2 +
 drivers/Makefile                           |   1 +
 drivers/block/blk-uclass.c                 |   1 +
 drivers/nvmxip/Kconfig                     |  19 +++
 drivers/nvmxip/Makefile                    |   8 ++
 drivers/nvmxip/nvmxip-uclass.c             |  15 +++
 drivers/nvmxip/nvmxip.c                    | 129 +++++++++++++++++++++
 drivers/nvmxip/nvmxip.h                    |  48 ++++++++
 drivers/nvmxip/nvmxip_qspi.c               |  67 +++++++++++
 include/dm/uclass-id.h                     |   1 +
 14 files changed, 425 insertions(+)
 create mode 100644 doc/develop/driver-model/nvmxip.rst
 create mode 100644 doc/device-tree-bindings/nvmxip/nvmxip.txt
 create mode 100644 drivers/nvmxip/Kconfig
 create mode 100644 drivers/nvmxip/Makefile
 create mode 100644 drivers/nvmxip/nvmxip-uclass.c
 create mode 100644 drivers/nvmxip/nvmxip.c
 create mode 100644 drivers/nvmxip/nvmxip.h
 create mode 100644 drivers/nvmxip/nvmxip_qspi.c

diff --git a/MAINTAINERS b/MAINTAINERS
index b2de50ccfc..539d01f68c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1193,6 +1193,13 @@ F:	cmd/nvme.c
 F:	include/nvme.h
 F:	doc/develop/driver-model/nvme.rst
 
+NVMXIP
+M:	Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
+S:	Maintained
+F:	doc/develop/driver-model/nvmxip.rst
+F:	doc/device-tree-bindings/nvmxip/nvmxip.txt
+F:	drivers/nvmxip/
+
 NVMEM
 M:	Sean Anderson <seanga2@gmail.com>
 S:	Maintained
diff --git a/doc/develop/driver-model/index.rst b/doc/develop/driver-model/index.rst
index 7366ef818c..8e12bbd936 100644
--- a/doc/develop/driver-model/index.rst
+++ b/doc/develop/driver-model/index.rst
@@ -20,6 +20,7 @@ subsystems
    livetree
    migration
    nvme
+   nvmxip
    of-plat
    pci-info
    pmic-framework
diff --git a/doc/develop/driver-model/nvmxip.rst b/doc/develop/driver-model/nvmxip.rst
new file mode 100644
index 0000000000..91b24e4e50
--- /dev/null
+++ b/doc/develop/driver-model/nvmxip.rst
@@ -0,0 +1,70 @@
+.. SPDX-License-Identifier: GPL-2.0+
+
+NVM XIP Block Storage Emulation Driver
+=======================================
+
+Summary
+-------
+
+Non-Volatile Memory devices with addressable memory (e.g: QSPI NOR flash) could
+be used for block storage needs (e.g: parsing a GPT layout in a raw QSPI NOR flash).
+
+The NVMXIP class provides this functionality and can be used for any 64-bit platform.
+
+The NVMXIP class provides the following drivers:
+
+      nvmxip-blk :
+
+        A generic block driver allowing to read from the XIP flash.
+	The driver belongs to UCLASS_BLK.
+	The driver implemented by drivers/nvmxip/nvmxip.c
+
+      nvmxip_qspi :
+
+        The driver probed with the DT and parent of the nvmxip-blk device.
+        nvmxip_qspi can be reused by other platforms. If the platform
+        has custom settings to apply before using the flash, then the platform
+        can provide its own parent driver belonging to UCLASS_NVMXIP and reuse
+        nvmxip-blk. The custom driver can be implmented like nvmxip_qspi in
+        addition to the platform custom settings.
+	The nvmxip_qspi driver belongs to UCLASS_NVMXIP.
+	The driver implemented by drivers/nvmxip/nvmxip_qspi.c
+
+    The implementation is generic and can be used by different platforms.
+
+Supported hardware
+--------------------------------
+
+Any 64-bit plaform.
+
+Configuration
+----------------------
+
+config NVMXIP
+	  This option allows the emulation of a block storage device
+	  on top of a direct access non volatile memory XIP flash devices.
+	  This support provides the read operation.
+	  This option provides the block storage driver nvmxip-blk which
+	  handles the read operation. This driver is HW agnostic and can support
+	  multiple flash devices at the same time.
+
+config NVMXIP_QSPI
+	  This option allows the emulation of a block storage device on top of a QSPI XIP flash.
+	  Any platform that needs to emulate one or multiple XIP flash devices can turn this
+	  option on to enable the functionality. NVMXIP config is selected automatically.
+	  Platforms that need to add custom treatments before accessing to the flash, can
+	  write their own driver (same as nvmxip_qspi in addition to the custom settings).
+
+Device Tree nodes
+--------------------
+
+Multiple XIP flash devices can be used at the same time by describing them through DT
+nodes.
+
+Please refer to the documentation of the DT binding at:
+
+doc/device-tree-bindings/nvmxip/nvmxip.txt
+
+Contributors
+------------
+   * Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
diff --git a/doc/device-tree-bindings/nvmxip/nvmxip.txt b/doc/device-tree-bindings/nvmxip/nvmxip.txt
new file mode 100644
index 0000000000..7c4b03f66b
--- /dev/null
+++ b/doc/device-tree-bindings/nvmxip/nvmxip.txt
@@ -0,0 +1,56 @@
+Specifying NVMXIP information for devices
+======================================
+
+NVM XIP flash device nodes
+---------------------------
+
+Each flash device should have its own node.
+
+Each node must specify the following fields:
+
+1)
+		compatible = "nvmxip,qspi";
+
+This allows to bind the flash device with the nvmxip_qspi driver
+If a platform has its own driver, please provide your own compatible
+string.
+
+2)
+		reg = <0x0 0x08000000 0x0 0x00200000>;
+
+The start address and size of the flash device. The values give here are an
+example (when the cell size is 2).
+
+When cell size is 1, the reg field looks like this:
+
+		reg = <0x08000000 0x00200000>;
+
+3)
+
+		lba_shift = <9>;
+
+The number of bit shifts used to calculate the size in bytes of one block.
+In this example the block size is 1 << 9 = 2 ^ 9 = 512 bytes
+
+4)
+
+		lba = <4096>;
+
+The number of blocks.
+
+Example of multiple flash devices
+----------------------------------------------------
+
+	nvmxip-qspi1@08000000 {
+		compatible = "nvmxip,qspi";
+		reg = <0x0 0x08000000 0x0 0x00200000>;
+		lba_shift = <9>;
+		lba = <4096>;
+	};
+
+	nvmxip-qspi2@08200000 {
+		compatible = "nvmxip,qspi";
+		reg = <0x0 0x08200000 0x0 0x00100000>;
+		lba_shift = <9>;
+		lba = <2048>;
+	};
diff --git a/drivers/Kconfig b/drivers/Kconfig
index 9101e538b0..ef321bee94 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -76,6 +76,8 @@ source "drivers/net/Kconfig"
 
 source "drivers/nvme/Kconfig"
 
+source "drivers/nvmxip/Kconfig"
+
 source "drivers/pci/Kconfig"
 
 source "drivers/pci_endpoint/Kconfig"
diff --git a/drivers/Makefile b/drivers/Makefile
index 83b14ef1fd..5e7fd6dab5 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -89,6 +89,7 @@ obj-$(CONFIG_FWU_MDATA) += fwu-mdata/
 obj-y += misc/
 obj-$(CONFIG_MMC) += mmc/
 obj-$(CONFIG_NVME) += nvme/
+obj-$(CONFIG_NVMXIP) += nvmxip/
 obj-$(CONFIG_PCI_ENDPOINT) += pci_endpoint/
 obj-y += dfu/
 obj-$(CONFIG_PCH) += pch/
diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c
index c69fc4d518..e8ab576c32 100644
--- a/drivers/block/blk-uclass.c
+++ b/drivers/block/blk-uclass.c
@@ -28,6 +28,7 @@ static struct {
 	{ UCLASS_AHCI, "sata" },
 	{ UCLASS_HOST, "host" },
 	{ UCLASS_NVME, "nvme" },
+	{ UCLASS_NVMXIP, "nvmxip" },
 	{ UCLASS_EFI_MEDIA, "efi" },
 	{ UCLASS_EFI_LOADER, "efiloader" },
 	{ UCLASS_VIRTIO, "virtio" },
diff --git a/drivers/nvmxip/Kconfig b/drivers/nvmxip/Kconfig
new file mode 100644
index 0000000000..3ef7105026
--- /dev/null
+++ b/drivers/nvmxip/Kconfig
@@ -0,0 +1,19 @@
+# SPDX-License-Identifier: GPL-2.0+
+#
+# Copyright 2023 Arm Limited and/or its affiliates <open-source-office@arm.com>
+# Authors:
+#   Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
+
+config NVMXIP
+	bool "NVM XIP devices support"
+	select BLK
+	help
+	  This option allows the emulation of a block storage device
+	  on top of a direct access non volatile memory XIP flash devices.
+	  This support provides the read operation.
+
+config NVMXIP_QSPI
+	bool "QSPI XIP  support"
+	select NVMXIP
+	help
+	  This option allows the emulation of a block storage device on top of a QSPI XIP flash
diff --git a/drivers/nvmxip/Makefile b/drivers/nvmxip/Makefile
new file mode 100644
index 0000000000..54eacc102e
--- /dev/null
+++ b/drivers/nvmxip/Makefile
@@ -0,0 +1,8 @@
+# SPDX-License-Identifier: GPL-2.0+
+#
+# Copyright 2023 Arm Limited and/or its affiliates <open-source-office@arm.com>
+# Authors:
+#   Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
+
+obj-y += nvmxip-uclass.o nvmxip.o
+obj-$(CONFIG_NVMXIP_QSPI) += nvmxip_qspi.o
diff --git a/drivers/nvmxip/nvmxip-uclass.c b/drivers/nvmxip/nvmxip-uclass.c
new file mode 100644
index 0000000000..aed8e163d2
--- /dev/null
+++ b/drivers/nvmxip/nvmxip-uclass.c
@@ -0,0 +1,15 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2023 Arm Limited and/or its affiliates <open-source-office@arm.com>
+ *
+ * Authors:
+ *   Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
+ */
+
+#include <common.h>
+#include <dm.h>
+
+UCLASS_DRIVER(nvmxip) = {
+	.name	= "nvmxip",
+	.id	= UCLASS_NVMXIP,
+};
diff --git a/drivers/nvmxip/nvmxip.c b/drivers/nvmxip/nvmxip.c
new file mode 100644
index 0000000000..c276fb147e
--- /dev/null
+++ b/drivers/nvmxip/nvmxip.c
@@ -0,0 +1,129 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2023 Arm Limited and/or its affiliates <open-source-office@arm.com>
+ *
+ * Authors:
+ *   Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <dm/device-internal.h>
+#include "nvmxip.h"
+
+static u32 nvmxip_bdev_max_devs;
+
+static int nvmxip_mmio_rawread(const phys_addr_t address, u64 *value)
+{
+	*value = readq(address);
+	return 0;
+}
+
+static ulong nvmxip_blk_read(struct udevice *udev, lbaint_t blknr, lbaint_t blkcnt, void *buffer)
+{
+	struct nvmxip_blk_priv *bpriv_data = dev_get_priv(udev);
+	struct blk_desc *desc = dev_get_uclass_plat(udev);
+
+	/* size of 1 block */
+	/* number of the u64 words to read */
+	u32 qwords = (blkcnt * desc->blksz) / sizeof(u64);
+	/* physical address of the first block to read */
+	phys_addr_t blkaddr = bpriv_data->pplat_data->phys_base + blknr * desc->blksz;
+	u64 *virt_blkaddr;
+	u64 *pdst = buffer;
+	u32 qdata_idx;
+
+	if (!pdst)
+		return -EINVAL;
+
+	pr_debug("[%s]: reading from blknr: %lu , blkcnt: %lu\n", udev->name, blknr, blkcnt);
+
+	virt_blkaddr = map_sysmem(blkaddr, 0);
+
+	/* assumption: the data is virtually contiguous */
+
+	for (qdata_idx = 0 ; qdata_idx < qwords ; qdata_idx++)
+		nvmxip_mmio_rawread((phys_addr_t)(virt_blkaddr + qdata_idx), pdst++);
+
+	pr_debug("[%s]:     src[0]: 0x%llx , dst[0]: 0x%llx , src[-1]: 0x%llx , dst[-1]: 0x%llx\n",
+		 udev->name,
+		 *virt_blkaddr,
+		 *(u64 *)buffer,
+		 *(u64 *)((u8 *)virt_blkaddr + desc->blksz * blkcnt - sizeof(u64)),
+		 *(u64 *)((u8 *)buffer + desc->blksz * blkcnt - sizeof(u64)));
+
+	unmap_sysmem(virt_blkaddr);
+
+	return blkcnt;
+}
+
+static int nvmxip_blk_probe(struct udevice *udev)
+{
+	struct nvmxip_priv *ppriv_data = dev_get_priv(udev->parent);
+	struct blk_desc *desc = dev_get_uclass_plat(udev);
+	struct nvmxip_blk_priv *bpriv_data = dev_get_priv(udev);
+
+	bpriv_data->bdev = udev;
+	bpriv_data->pplat_data = ppriv_data->plat_data;
+	desc->lba = bpriv_data->pplat_data->lba;
+	desc->log2blksz = bpriv_data->pplat_data->lba_shift;
+	desc->blksz = 1 << bpriv_data->pplat_data->lba_shift;
+	desc->bdev = bpriv_data->bdev;
+
+	pr_debug("[%s]: block storage layout\n    lbas: %lu , log2blksz: %d, blksz: %lu\n",
+		 udev->name, desc->lba, desc->log2blksz, desc->blksz);
+
+	return 0;
+}
+
+int nvmxip_init(struct udevice *udev)
+{
+	struct nvmxip_plat *plat_data = dev_get_plat(udev);
+	struct nvmxip_priv *priv_data = dev_get_priv(udev);
+	int ret;
+	struct udevice *bdev = NULL;
+	char bdev_name[NVMXIP_BLKDEV_NAME_SZ + 1] = {0};
+
+	priv_data->udev = udev;
+	priv_data->plat_data = plat_data;
+
+	nvmxip_bdev_max_devs++;
+
+	snprintf(bdev_name, NVMXIP_BLKDEV_NAME_SZ, "nvmxip-blk#%d", nvmxip_bdev_max_devs);
+
+	ret = blk_create_devicef(udev, NVMXIP_BLKDRV_NAME, bdev_name, UCLASS_NVMXIP,
+				 nvmxip_bdev_max_devs, NVMXIP_DEFAULT_LBA_SZ,
+				 NVMXIP_DEFAULT_LBA_COUNT, &bdev);
+	if (ret) {
+		pr_err("[%s]: failure during creation of the block device %s, error %d\n",
+		       udev->name, bdev_name, ret);
+		goto blkdev_setup_error;
+	}
+
+	ret = blk_probe_or_unbind(bdev);
+	if (ret) {
+		pr_err("[%s]: failure during probing the block device %s, error %d\n",
+		       udev->name, bdev_name, ret);
+		goto blkdev_setup_error;
+	}
+
+	pr_info("[%s]: the block device %s ready for use\n", udev->name, bdev_name);
+
+	return 0;
+
+blkdev_setup_error:
+	nvmxip_bdev_max_devs--;
+	return ret;
+}
+
+static const struct blk_ops nvmxip_blk_ops = {
+	.read	= nvmxip_blk_read,
+};
+
+U_BOOT_DRIVER(nvmxip_blk) = {
+	.name	= NVMXIP_BLKDRV_NAME,
+	.id	= UCLASS_BLK,
+	.probe	= nvmxip_blk_probe,
+	.ops	= &nvmxip_blk_ops,
+	.priv_auto	= sizeof(struct nvmxip_blk_priv),
+};
diff --git a/drivers/nvmxip/nvmxip.h b/drivers/nvmxip/nvmxip.h
new file mode 100644
index 0000000000..47ae3bd8ab
--- /dev/null
+++ b/drivers/nvmxip/nvmxip.h
@@ -0,0 +1,48 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright 2023 Arm Limited and/or its affiliates <open-source-office@arm.com>
+ *
+ * Authors:
+ *   Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
+ */
+
+#ifndef __DRIVER_NVMXIP_H__
+#define __DRIVER_NVMXIP_H__
+
+#include <asm/io.h>
+#include <blk.h>
+#include <linux/bitops.h>
+#include <linux/compat.h>
+#include <mapmem.h>
+
+#define NVMXIP_BLKDRV_NAME    "nvmxip-blk"
+
+#define NVMXIP_BLKDEV_NAME_SZ 20
+
+#define NVMXIP_DEFAULT_LBA_SHIFT 10 /* 1024 bytes per block */
+#define NVMXIP_DEFAULT_LBA_COUNT 1024 /* block count */
+
+#define NVMXIP_DEFAULT_LBA_SZ BIT(NVMXIP_DEFAULT_LBA_SHIFT)
+
+/* NVM XIP device platform data */
+struct nvmxip_plat {
+	phys_addr_t phys_base; /* NVM XIP device base address */
+	u32 lba_shift; /* block size shift count (read from device tree) */
+	lbaint_t lba; /* number of blocks (read from device tree) */
+};
+
+/* NVM XIP device private data */
+struct nvmxip_priv {
+	struct udevice *udev;
+	struct nvmxip_plat *plat_data;
+};
+
+/* Private data of the block device associated with the NVM XIP device (the parent) */
+struct nvmxip_blk_priv {
+	struct udevice *bdev;
+	struct nvmxip_plat *pplat_data; /* parent device platform data */
+};
+
+int nvmxip_init(struct udevice *udev);
+
+#endif /* __DRIVER_NVMXIP_H__ */
diff --git a/drivers/nvmxip/nvmxip_qspi.c b/drivers/nvmxip/nvmxip_qspi.c
new file mode 100644
index 0000000000..d0ce4c6633
--- /dev/null
+++ b/drivers/nvmxip/nvmxip_qspi.c
@@ -0,0 +1,67 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2023 Arm Limited and/or its affiliates <open-source-office@arm.com>
+ *
+ * Authors:
+ *   Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <fdt_support.h>
+#include "nvmxip.h"
+
+#include <asm/global_data.h>
+DECLARE_GLOBAL_DATA_PTR;
+
+#define NVMXIP_QSPI_DRV_NAME "nvmxip_qspi"
+
+static int nvmxip_qspi_probe(struct udevice *dev)
+{
+	pr_debug("[%s][%s]\n", __func__, dev->name);
+	return nvmxip_init(dev);
+}
+
+static int nvmxip_qspi_of_to_plat(struct udevice *dev)
+{
+	struct nvmxip_plat *plat_data = dev_get_plat(dev);
+	int ret;
+
+	plat_data->phys_base = (phys_addr_t)dev_read_addr(dev);
+	if (plat_data->phys_base == FDT_ADDR_T_NONE) {
+		pr_err("[%s]: can not get base address from device tree\n", dev->name);
+		return -EINVAL;
+	}
+
+	ret = dev_read_u32(dev, "lba_shift", &plat_data->lba_shift);
+	if (ret) {
+		pr_err("[%s]: can not get lba_shift from device tree\n", dev->name);
+		return -EINVAL;
+	}
+
+	ret = dev_read_u32(dev, "lba", (u32 *)&plat_data->lba);
+	if (ret) {
+		pr_err("[%s]: can not get lba from device tree\n", dev->name);
+		return -EINVAL;
+	}
+
+	pr_debug("[%s]: XIP device base addr: 0x%llx , lba_shift: %d , lbas: %lu\n",
+		 dev->name, plat_data->phys_base, plat_data->lba_shift, plat_data->lba);
+
+	return 0;
+}
+
+static const struct udevice_id nvmxip_qspi_ids[] = {
+	{ .compatible = "nvmxip,qspi" },
+	{ /* sentinel */ }
+};
+
+U_BOOT_DRIVER(nvmxip_qspi) = {
+	.name = NVMXIP_QSPI_DRV_NAME,
+	.id = UCLASS_NVMXIP,
+	.of_match = nvmxip_qspi_ids,
+	.of_to_plat = nvmxip_qspi_of_to_plat,
+	.priv_auto = sizeof(struct nvmxip_priv),
+	.plat_auto = sizeof(struct nvmxip_plat),
+	.probe = nvmxip_qspi_probe,
+};
diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
index 376f741cc2..dcfe8cfe2c 100644
--- a/include/dm/uclass-id.h
+++ b/include/dm/uclass-id.h
@@ -88,6 +88,7 @@ enum uclass_id {
 	UCLASS_NOP,		/* No-op devices */
 	UCLASS_NORTHBRIDGE,	/* Intel Northbridge / SDRAM controller */
 	UCLASS_NVME,		/* NVM Express device */
+	UCLASS_NVMXIP,		/* NVM XIP devices */
 	UCLASS_P2SB,		/* (x86) Primary-to-Sideband Bus */
 	UCLASS_PANEL,		/* Display panel, such as an LCD */
 	UCLASS_PANEL_BACKLIGHT,	/* Backlight controller for panel */
-- 
2.17.1

[PATCH v1 2/6] sandbox64: fix: return unsigned long in readq()

[abdellatif.elkhlifi]

From: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>

make readq return unsigned long

readq should return 64-bit data

Signed-off-by: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
---
 arch/sandbox/cpu/cpu.c        | 2 +-
 arch/sandbox/include/asm/io.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/sandbox/cpu/cpu.c b/arch/sandbox/cpu/cpu.c
index 636d3545b9..248d17a85c 100644
--- a/arch/sandbox/cpu/cpu.c
+++ b/arch/sandbox/cpu/cpu.c
@@ -230,7 +230,7 @@ phys_addr_t map_to_sysmem(const void *ptr)
 	return mentry->tag;
 }
 
-unsigned int sandbox_read(const void *addr, enum sandboxio_size_t size)
+unsigned long sandbox_read(const void *addr, enum sandboxio_size_t size)
 {
 	struct sandbox_state *state = state_get_current();
 
diff --git a/arch/sandbox/include/asm/io.h b/arch/sandbox/include/asm/io.h
index ad6c29a4e2..31ab7289b4 100644
--- a/arch/sandbox/include/asm/io.h
+++ b/arch/sandbox/include/asm/io.h
@@ -45,7 +45,7 @@ static inline void unmap_sysmem(const void *vaddr)
 /* Map from a pointer to our RAM buffer */
 phys_addr_t map_to_sysmem(const void *ptr);
 
-unsigned int sandbox_read(const void *addr, enum sandboxio_size_t size);
+unsigned long sandbox_read(const void *addr, enum sandboxio_size_t size);
 void sandbox_write(void *addr, unsigned int val, enum sandboxio_size_t size);
 
 #define readb(addr) sandbox_read((const void *)addr, SB_SIZE_8)
-- 
2.17.1

[PATCH v1 3/6] sandbox64: add support for NVMXIP QSPI

[abdellatif.elkhlifi]

From: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>

enable NVMXIP QSPI for sandbox 64-bit

Adding two NVM XIP QSPI storage devices.

Signed-off-by: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
---
 arch/sandbox/dts/sandbox64.dts | 13 +++++++++++++
 arch/sandbox/dts/test.dts      | 14 ++++++++++++++
 configs/sandbox64_defconfig    |  1 +
 drivers/nvmxip/nvmxip.c        |  4 ++++
 drivers/nvmxip/nvmxip.h        |  3 +++
 5 files changed, 35 insertions(+)

diff --git a/arch/sandbox/dts/sandbox64.dts b/arch/sandbox/dts/sandbox64.dts
index a9cd7908f8..aed3801af8 100644
--- a/arch/sandbox/dts/sandbox64.dts
+++ b/arch/sandbox/dts/sandbox64.dts
@@ -89,6 +89,19 @@
 		cs-gpios = <0>, <&gpio_a 0>;
 	};
 
+	nvmxip-qspi1@08000000 {
+		compatible = "nvmxip,qspi";
+		reg = <0x0 0x08000000 0x0 0x00200000>;
+		lba_shift = <9>;
+		lba = <4096>;
+	};
+
+	nvmxip-qspi2@08200000 {
+		compatible = "nvmxip,qspi";
+		reg = <0x0 0x08200000 0x0 0x00100000>;
+		lba_shift = <9>;
+		lba = <2048>;
+	};
 };
 
 #include "sandbox.dtsi"
diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index dffe10adbf..c3ba0a225e 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -1745,6 +1745,20 @@
 		compatible = "u-boot,fwu-mdata-gpt";
 		fwu-mdata-store = <&mmc0>;
 	};
+
+	nvmxip-qspi1@08000000 {
+		compatible = "nvmxip,qspi";
+		reg = <0x08000000 0x00200000>;
+		lba_shift = <9>;
+		lba = <4096>;
+	};
+
+	nvmxip-qspi2@08200000 {
+		compatible = "nvmxip,qspi";
+		reg = <0x08200000 0x00100000>;
+		lba_shift = <9>;
+		lba = <2048>;
+	};
 };
 
 #include "sandbox_pmic.dtsi"
diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig
index ba45ac0b71..cd4dadb190 100644
--- a/configs/sandbox64_defconfig
+++ b/configs/sandbox64_defconfig
@@ -259,3 +259,4 @@ CONFIG_FWU_MULTI_BANK_UPDATE=y
 CONFIG_UNIT_TEST=y
 CONFIG_UT_TIME=y
 CONFIG_UT_DM=y
+CONFIG_NVMXIP_QSPI=y
\ No newline at end of file
diff --git a/drivers/nvmxip/nvmxip.c b/drivers/nvmxip/nvmxip.c
index c276fb147e..ea9b9bfa48 100644
--- a/drivers/nvmxip/nvmxip.c
+++ b/drivers/nvmxip/nvmxip.c
@@ -87,6 +87,10 @@ int nvmxip_init(struct udevice *udev)
 	priv_data->udev = udev;
 	priv_data->plat_data = plat_data;
 
+#if CONFIG_IS_ENABLED(SANDBOX64)
+	sandbox_set_enable_memio(true);
+#endif
+
 	nvmxip_bdev_max_devs++;
 
 	snprintf(bdev_name, NVMXIP_BLKDEV_NAME_SZ, "nvmxip-blk#%d", nvmxip_bdev_max_devs);
diff --git a/drivers/nvmxip/nvmxip.h b/drivers/nvmxip/nvmxip.h
index 47ae3bd8ab..0cb8e511f0 100644
--- a/drivers/nvmxip/nvmxip.h
+++ b/drivers/nvmxip/nvmxip.h
@@ -10,6 +10,9 @@
 #define __DRIVER_NVMXIP_H__
 
 #include <asm/io.h>
+#if CONFIG_IS_ENABLED(SANDBOX64)
+#include <asm/test.h>
+#endif
 #include <blk.h>
 #include <linux/bitops.h>
 #include <linux/compat.h>
-- 
2.17.1

[PATCH v1 4/6] corstone1000: add NVM XIP QSPI device tree node

[abdellatif.elkhlifi]

From: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>

add QSPI flash device node for block storage access

Signed-off-by: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
---
 arch/arm/dts/corstone1000.dtsi | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/arm/dts/corstone1000.dtsi b/arch/arm/dts/corstone1000.dtsi
index 4e46826f88..533dfdf8e1 100644
--- a/arch/arm/dts/corstone1000.dtsi
+++ b/arch/arm/dts/corstone1000.dtsi
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0 or MIT
 /*
- * Copyright (c) 2022, Arm Limited. All rights reserved.
+ * Copyright 2022-2023 Arm Limited and/or its affiliates <open-source-office@arm.com>
  * Copyright (c) 2022, Linaro Limited. All rights reserved.
  *
  */
@@ -38,6 +38,13 @@
 		reg = <0x88200000 0x77e00000>;
 	};
 
+	nvmxip-qspi@08000000 {
+		compatible = "nvmxip,qspi";
+		reg = <0x08000000 0x2000000>;
+		lba_shift = <9>;
+		lba = <65536>;
+	};
+
 	gic: interrupt-controller@1c000000 {
 		compatible = "arm,gic-400";
 		#interrupt-cells = <3>;
-- 
2.17.1

[PATCH v1 5/6] corstone1000: enable NVM XIP QSPI flash

[abdellatif.elkhlifi]

From: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>

add the QSPI flash device with block storage capability

Signed-off-by: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
---
 configs/corstone1000_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configs/corstone1000_defconfig b/configs/corstone1000_defconfig
index dddfa27507..815355a544 100644
--- a/configs/corstone1000_defconfig
+++ b/configs/corstone1000_defconfig
@@ -52,3 +52,4 @@ CONFIG_DM_SERIAL=y
 CONFIG_USB=y
 CONFIG_USB_ISP1760=y
 CONFIG_ERRNO_STR=y
+CONFIG_NVMXIP_QSPI=y
-- 
2.17.1

[PATCH v1 6/6] sandbox64: add a test case for UCLASS_NVMXIP

[abdellatif.elkhlifi]

From: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>

provide a test for NVM XIP devices

The test case allows to make sure of the following:

- The NVM XIP QSPI devices are probed
- The DT entries are read correctly
- the data read from the flash by the NVMXIP block driver is correct

Signed-off-by: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
---
 MAINTAINERS      |   1 +
 test/dm/Makefile |   5 ++
 test/dm/nvmxip.c | 117 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 123 insertions(+)
 create mode 100644 test/dm/nvmxip.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 539d01f68c..e92898e462 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1199,6 +1199,7 @@ S:	Maintained
 F:	doc/develop/driver-model/nvmxip.rst
 F:	doc/device-tree-bindings/nvmxip/nvmxip.txt
 F:	drivers/nvmxip/
+F:	test/dm/nvmxip.c
 
 NVMEM
 M:	Sean Anderson <seanga2@gmail.com>
diff --git a/test/dm/Makefile b/test/dm/Makefile
index 7a79b6e1a2..7f9d0b3c38 100644
--- a/test/dm/Makefile
+++ b/test/dm/Makefile
@@ -1,6 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0+
 #
 # Copyright (c) 2013 Google, Inc
+# Copyright 2023 Arm Limited and/or its affiliates <open-source-office@arm.com>
 
 obj-$(CONFIG_UT_DM) += test-dm.o
 
@@ -17,6 +18,10 @@ obj-$(CONFIG_UT_DM) += test-uclass.o
 obj-$(CONFIG_UT_DM) += core.o
 obj-$(CONFIG_UT_DM) += read.o
 obj-$(CONFIG_UT_DM) += phys2bus.o
+ifeq ($(CONFIG_NVMXIP_QSPI)$(CONFIG_SANDBOX64),yy)
+obj-y += nvmxip.o
+endif
+
 ifneq ($(CONFIG_SANDBOX),)
 ifeq ($(CONFIG_ACPIGEN),y)
 obj-y += acpi.o
diff --git a/test/dm/nvmxip.c b/test/dm/nvmxip.c
new file mode 100644
index 0000000000..b65154d125
--- /dev/null
+++ b/test/dm/nvmxip.c
@@ -0,0 +1,117 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Functional tests for UCLASS_FFA  class
+ *
+ * Copyright 2023 Arm Limited and/or its affiliates <open-source-office@arm.com>
+ *
+ * Authors:
+ *   Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
+ */
+
+#include <common.h>
+#include <console.h>
+#include <blk.h>
+#include <dm.h>
+#include <dm/test.h>
+#include "../../drivers/nvmxip/nvmxip.h"
+#include <test/test.h>
+#include <test/ut.h>
+
+/* NVMXIP devices described in the device tree  */
+#define SANDBOX_NVMXIP_DEVICES 2
+
+/* reference device tree data for the probed devices */
+static struct nvmxip_plat nvmqspi_refdata[SANDBOX_NVMXIP_DEVICES] = {
+	{0x08000000, 9, 4096}, {0x08200000, 9, 2048}
+};
+
+#define NVMXIP_BLK_START_PATTERN 0x1122334455667788ULL
+#define NVMXIP_BLK_END_PATTERN 0xa1a2a3a4a5a6a7a8ULL
+
+static int dm_nvmxip_flash_sanity(u8 device_idx, void *buffer)
+{
+	int i;
+	u64 *ptr = NULL;
+	u8 *base = NULL;
+	unsigned long blksz;
+
+	blksz = 1 << nvmqspi_refdata[device_idx].lba_shift;
+
+	/* if buffer not NULL, init the flash with the pattern data*/
+	if (!buffer)
+		base = map_sysmem(nvmqspi_refdata[device_idx].phys_base, 0);
+	else
+		base = buffer;
+
+	for (i = 0; i < nvmqspi_refdata[device_idx].lba ; i++) {
+		ptr = (u64 *)(base + i * blksz);
+
+		/* write an 8 bytes pattern at the start of the current block*/
+		if (!buffer)
+			*ptr = NVMXIP_BLK_START_PATTERN;
+		else if (*ptr != NVMXIP_BLK_START_PATTERN)
+			return -EINVAL;
+
+		ptr = (u64 *)((u8 *)ptr + blksz - sizeof(u64));
+
+		/* write an 8 bytes pattern at the end of the current block*/
+		if (!buffer)
+			*ptr = NVMXIP_BLK_END_PATTERN;
+		else if (*ptr != NVMXIP_BLK_END_PATTERN)
+			return -EINVAL;
+	}
+
+	if (!buffer)
+		unmap_sysmem(base);
+
+	return 0;
+}
+
+static int dm_test_nvmxip(struct unit_test_state *uts)
+{
+	struct nvmxip_plat *plat_data = NULL;
+	struct udevice *dev = NULL, *bdev = NULL;
+	u8 device_idx;
+	void *buffer = NULL;
+	unsigned long flashsz;
+
+	/* set the flash content first for both devices */
+	dm_nvmxip_flash_sanity(0, NULL);
+	dm_nvmxip_flash_sanity(1, NULL);
+
+	/*  probing all NVM XIP QSPI devices */
+	for (device_idx = 0, uclass_first_device(UCLASS_NVMXIP, &dev);
+	     dev;
+	     uclass_next_device(&dev), device_idx++) {
+		plat_data = dev_get_plat(dev);
+
+		/* device tree entries checks */
+		ut_assertok(nvmqspi_refdata[device_idx].phys_base != plat_data->phys_base);
+		ut_assertok(nvmqspi_refdata[device_idx].lba_shift != plat_data->lba_shift);
+		ut_assertok(nvmqspi_refdata[device_idx].lba != plat_data->lba);
+
+		/* before reading all the flash blocks, let's calculate the flash size */
+		flashsz = plat_data->lba << plat_data->lba_shift;
+
+		/* allocate the user buffer where to copy the blocks data to */
+		buffer = calloc(flashsz, 1);
+		ut_assertok(!buffer);
+
+		/* the block device is the child of the parent device probed with DT*/
+		ut_assertok(device_find_first_child(dev, &bdev));
+
+		/* reading all the flash blocks*/
+		ut_asserteq(plat_data->lba, blk_read(bdev, 0, plat_data->lba, buffer));
+
+		/* compare the data read from flash with the expected data */
+		ut_assertok(dm_nvmxip_flash_sanity(device_idx, buffer));
+
+		free(buffer);
+	}
+
+	ut_assertok(device_idx != SANDBOX_NVMXIP_DEVICES);
+
+	return CMD_RET_SUCCESS;
+}
+
+DM_TEST(dm_test_nvmxip, UT_TESTF_SCAN_FDT | UT_TESTF_CONSOLE_REC);
-- 
2.17.1

Re: [PATCH v1 0/6] introduce NVM XIP block storage emulation

[Abdellatif El Khlifi]

On Mon, Jan 16, 2023 at 05:28:26PM +0000, abdellatif.elkhlifi@arm.com wrote:
> From: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
> 
> Adding block storage emulation for NVM XIP flash devices.
> 
> Some paltforms such as Corstone-1000 need to see NVM XIP raw flash
> as a block storage device with read only capability.
> 
> Here NVM flash devices are devices with addressable
> memory (e.g: QSPI NOR flash).
> 
> The NVM XIP block storage emulation provides the following features:
> 
> - Emulate NVM XIP raw flash as a block storage device with read only capability
> - Being generic by design and can be used by any platform
> - Device tree node
> - Platforms can use multiple NVM XIP devices at the same time by defining a
>   DT node for each one of them
> - A generic NVMXIP block driver allowing to read from the XIP flash
> - A generic NVMXIP QSPI driver
> - Implemented on top of memory-mapped IO (using readq macro)
> - Enabling NVMXIP in sandbox64
> - A sandbox test case
> - Enabling NVMXIP in Corstone1000 platform as a use case
> 
> For more details please refer to the readme [A].
> 
> [A]: doc/develop/driver-model/nvmxip.rst
> 
> Cheers,
> Abdellatif
> 
> Cc: Tom Rini <trini@konsulko.com>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Drew Reed <Drew.Reed@arm.com>
> Cc: Xueliang Zhong <Xueliang.Zhong@arm.com>
> 
> Abdellatif El Khlifi (6):
>   drivers/nvmxip: introduce NVM XIP block storage emulation
>   sandbox64: fix: return unsigned long in readq()
>   sandbox64: add support for NVMXIP QSPI
>   corstone1000: add NVM XIP QSPI device tree node
>   corstone1000: enable NVM XIP QSPI flash
>   sandbox64: add a test case for UCLASS_NVMXIP
> 
>  MAINTAINERS                                |   8 ++
>  arch/arm/dts/corstone1000.dtsi             |   9 +-
>  arch/sandbox/cpu/cpu.c                     |   2 +-
>  arch/sandbox/dts/sandbox64.dts             |  13 ++
>  arch/sandbox/dts/test.dts                  |  14 +++
>  arch/sandbox/include/asm/io.h              |   2 +-
>  configs/corstone1000_defconfig             |   1 +
>  configs/sandbox64_defconfig                |   1 +
>  doc/develop/driver-model/index.rst         |   1 +
>  doc/develop/driver-model/nvmxip.rst        |  70 +++++++++++
>  doc/device-tree-bindings/nvmxip/nvmxip.txt |  56 +++++++++
>  drivers/Kconfig                            |   2 +
>  drivers/Makefile                           |   1 +
>  drivers/block/blk-uclass.c                 |   1 +
>  drivers/nvmxip/Kconfig                     |  19 +++
>  drivers/nvmxip/Makefile                    |   8 ++
>  drivers/nvmxip/nvmxip-uclass.c             |  15 +++
>  drivers/nvmxip/nvmxip.c                    | 133 +++++++++++++++++++++
>  drivers/nvmxip/nvmxip.h                    |  51 ++++++++
>  drivers/nvmxip/nvmxip_qspi.c               |  67 +++++++++++
>  include/dm/uclass-id.h                     |   1 +
>  test/dm/Makefile                           |   5 +
>  test/dm/nvmxip.c                           | 117 ++++++++++++++++++
>  23 files changed, 594 insertions(+), 3 deletions(-)
>  create mode 100644 doc/develop/driver-model/nvmxip.rst
>  create mode 100644 doc/device-tree-bindings/nvmxip/nvmxip.txt
>  create mode 100644 drivers/nvmxip/Kconfig
>  create mode 100644 drivers/nvmxip/Makefile
>  create mode 100644 drivers/nvmxip/nvmxip-uclass.c
>  create mode 100644 drivers/nvmxip/nvmxip.c
>  create mode 100644 drivers/nvmxip/nvmxip.h
>  create mode 100644 drivers/nvmxip/nvmxip_qspi.c
>  create mode 100644 test/dm/nvmxip.c
> 
> -- 
> 2.17.1
>

Hi guys, a gentle reminder. Any feedback please ?

Cheers,
Abdellatif

Re: [PATCH v1 3/6] sandbox64: add support for NVMXIP QSPI

[Simon Glass]

On Mon, 16 Jan 2023 at 10:28, <abdellatif.elkhlifi@arm.com> wrote:
>
> From: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
>
> enable NVMXIP QSPI for sandbox 64-bit
>
> Adding two NVM XIP QSPI storage devices.
>
> Signed-off-by: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
> ---
>  arch/sandbox/dts/sandbox64.dts | 13 +++++++++++++
>  arch/sandbox/dts/test.dts      | 14 ++++++++++++++
>  configs/sandbox64_defconfig    |  1 +
>  drivers/nvmxip/nvmxip.c        |  4 ++++
>  drivers/nvmxip/nvmxip.h        |  3 +++
>  5 files changed, 35 insertions(+)

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

Nit below

>
> diff --git a/arch/sandbox/dts/sandbox64.dts b/arch/sandbox/dts/sandbox64.dts
> index a9cd7908f8..aed3801af8 100644
> --- a/arch/sandbox/dts/sandbox64.dts
> +++ b/arch/sandbox/dts/sandbox64.dts
> @@ -89,6 +89,19 @@
>                 cs-gpios = <0>, <&gpio_a 0>;
>         };
>
> +       nvmxip-qspi1@08000000 {
> +               compatible = "nvmxip,qspi";
> +               reg = <0x0 0x08000000 0x0 0x00200000>;
> +               lba_shift = <9>;
> +               lba = <4096>;
> +       };
> +
> +       nvmxip-qspi2@08200000 {
> +               compatible = "nvmxip,qspi";
> +               reg = <0x0 0x08200000 0x0 0x00100000>;
> +               lba_shift = <9>;
> +               lba = <2048>;
> +       };
>  };
>
>  #include "sandbox.dtsi"
> diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
> index dffe10adbf..c3ba0a225e 100644
> --- a/arch/sandbox/dts/test.dts
> +++ b/arch/sandbox/dts/test.dts
> @@ -1745,6 +1745,20 @@
>                 compatible = "u-boot,fwu-mdata-gpt";
>                 fwu-mdata-store = <&mmc0>;
>         };
> +
> +       nvmxip-qspi1@08000000 {
> +               compatible = "nvmxip,qspi";
> +               reg = <0x08000000 0x00200000>;
> +               lba_shift = <9>;
> +               lba = <4096>;
> +       };
> +
> +       nvmxip-qspi2@08200000 {
> +               compatible = "nvmxip,qspi";
> +               reg = <0x08200000 0x00100000>;
> +               lba_shift = <9>;
> +               lba = <2048>;
> +       };
>  };
>
>  #include "sandbox_pmic.dtsi"
> diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig
> index ba45ac0b71..cd4dadb190 100644
> --- a/configs/sandbox64_defconfig
> +++ b/configs/sandbox64_defconfig
> @@ -259,3 +259,4 @@ CONFIG_FWU_MULTI_BANK_UPDATE=y
>  CONFIG_UNIT_TEST=y
>  CONFIG_UT_TIME=y
>  CONFIG_UT_DM=y
> +CONFIG_NVMXIP_QSPI=y
> \ No newline at end of file
> diff --git a/drivers/nvmxip/nvmxip.c b/drivers/nvmxip/nvmxip.c
> index c276fb147e..ea9b9bfa48 100644
> --- a/drivers/nvmxip/nvmxip.c
> +++ b/drivers/nvmxip/nvmxip.c
> @@ -87,6 +87,10 @@ int nvmxip_init(struct udevice *udev)
>         priv_data->udev = udev;
>         priv_data->plat_data = plat_data;
>
> +#if CONFIG_IS_ENABLED(SANDBOX64)

if (IS_ENABLED(CONFIG_SANDBOX64))


> +       sandbox_set_enable_memio(true);
> +#endif
> +
>         nvmxip_bdev_max_devs++;
>
>         snprintf(bdev_name, NVMXIP_BLKDEV_NAME_SZ, "nvmxip-blk#%d", nvmxip_bdev_max_devs);
> diff --git a/drivers/nvmxip/nvmxip.h b/drivers/nvmxip/nvmxip.h
> index 47ae3bd8ab..0cb8e511f0 100644
> --- a/drivers/nvmxip/nvmxip.h
> +++ b/drivers/nvmxip/nvmxip.h
> @@ -10,6 +10,9 @@
>  #define __DRIVER_NVMXIP_H__
>
>  #include <asm/io.h>
> +#if CONFIG_IS_ENABLED(SANDBOX64)
> +#include <asm/test.h>
> +#endif
>  #include <blk.h>
>  #include <linux/bitops.h>
>  #include <linux/compat.h>
> --
> 2.17.1
>

Re: [PATCH v1 6/6] sandbox64: add a test case for UCLASS_NVMXIP

[Simon Glass]

Hi,

On Mon, 16 Jan 2023 at 10:29, <abdellatif.elkhlifi@arm.com> wrote:
>
> From: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
>
> provide a test for NVM XIP devices
>
> The test case allows to make sure of the following:
>
> - The NVM XIP QSPI devices are probed
> - The DT entries are read correctly
> - the data read from the flash by the NVMXIP block driver is correct
>
> Signed-off-by: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
> ---
>  MAINTAINERS      |   1 +
>  test/dm/Makefile |   5 ++
>  test/dm/nvmxip.c | 117 +++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 123 insertions(+)
>  create mode 100644 test/dm/nvmxip.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 539d01f68c..e92898e462 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1199,6 +1199,7 @@ S:        Maintained
>  F:     doc/develop/driver-model/nvmxip.rst
>  F:     doc/device-tree-bindings/nvmxip/nvmxip.txt
>  F:     drivers/nvmxip/
> +F:     test/dm/nvmxip.c
>
>  NVMEM
>  M:     Sean Anderson <seanga2@gmail.com>
> diff --git a/test/dm/Makefile b/test/dm/Makefile
> index 7a79b6e1a2..7f9d0b3c38 100644
> --- a/test/dm/Makefile
> +++ b/test/dm/Makefile
> @@ -1,6 +1,7 @@
>  # SPDX-License-Identifier: GPL-2.0+
>  #
>  # Copyright (c) 2013 Google, Inc
> +# Copyright 2023 Arm Limited and/or its affiliates <open-source-office@arm.com>
>
>  obj-$(CONFIG_UT_DM) += test-dm.o
>
> @@ -17,6 +18,10 @@ obj-$(CONFIG_UT_DM) += test-uclass.o
>  obj-$(CONFIG_UT_DM) += core.o
>  obj-$(CONFIG_UT_DM) += read.o
>  obj-$(CONFIG_UT_DM) += phys2bus.o
> +ifeq ($(CONFIG_NVMXIP_QSPI)$(CONFIG_SANDBOX64),yy)
> +obj-y += nvmxip.o
> +endif
> +
>  ifneq ($(CONFIG_SANDBOX),)
>  ifeq ($(CONFIG_ACPIGEN),y)
>  obj-y += acpi.o
> diff --git a/test/dm/nvmxip.c b/test/dm/nvmxip.c
> new file mode 100644
> index 0000000000..b65154d125
> --- /dev/null
> +++ b/test/dm/nvmxip.c
> @@ -0,0 +1,117 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Functional tests for UCLASS_FFA  class
> + *
> + * Copyright 2023 Arm Limited and/or its affiliates <open-source-office@arm.com>
> + *
> + * Authors:
> + *   Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
> + */
> +
> +#include <common.h>
> +#include <console.h>
> +#include <blk.h>
> +#include <dm.h>
> +#include <dm/test.h>
> +#include "../../drivers/nvmxip/nvmxip.h"

move to end

> +#include <test/test.h>
> +#include <test/ut.h>
> +
> +/* NVMXIP devices described in the device tree  */
> +#define SANDBOX_NVMXIP_DEVICES 2
> +
> +/* reference device tree data for the probed devices */
> +static struct nvmxip_plat nvmqspi_refdata[SANDBOX_NVMXIP_DEVICES] = {
> +       {0x08000000, 9, 4096}, {0x08200000, 9, 2048}
> +};
> +
> +#define NVMXIP_BLK_START_PATTERN 0x1122334455667788ULL
> +#define NVMXIP_BLK_END_PATTERN 0xa1a2a3a4a5a6a7a8ULL
> +
> +static int dm_nvmxip_flash_sanity(u8 device_idx, void *buffer)
> +{
> +       int i;
> +       u64 *ptr = NULL;
> +       u8 *base = NULL;
> +       unsigned long blksz;
> +
> +       blksz = 1 << nvmqspi_refdata[device_idx].lba_shift;

BIT() or BITUL() ?

> +
> +       /* if buffer not NULL, init the flash with the pattern data*/

space before */   (please fix globally)

can you explain why, in the comment?

> +       if (!buffer)
> +               base = map_sysmem(nvmqspi_refdata[device_idx].phys_base, 0);
> +       else
> +               base = buffer;
> +
> +       for (i = 0; i < nvmqspi_refdata[device_idx].lba ; i++) {
> +               ptr = (u64 *)(base + i * blksz);
> +
> +               /* write an 8 bytes pattern at the start of the current block*/
> +               if (!buffer)
> +                       *ptr = NVMXIP_BLK_START_PATTERN;
> +               else if (*ptr != NVMXIP_BLK_START_PATTERN)
> +                       return -EINVAL;
> +
> +               ptr = (u64 *)((u8 *)ptr + blksz - sizeof(u64));
> +
> +               /* write an 8 bytes pattern at the end of the current block*/
> +               if (!buffer)
> +                       *ptr = NVMXIP_BLK_END_PATTERN;
> +               else if (*ptr != NVMXIP_BLK_END_PATTERN)
> +                       return -EINVAL;

You can use ut_assert...() here so that people can see which line failed

> +       }
> +
> +       if (!buffer)
> +               unmap_sysmem(base);
> +
> +       return 0;
> +}
> +
> +static int dm_test_nvmxip(struct unit_test_state *uts)
> +{
> +       struct nvmxip_plat *plat_data = NULL;
> +       struct udevice *dev = NULL, *bdev = NULL;
> +       u8 device_idx;
> +       void *buffer = NULL;
> +       unsigned long flashsz;
> +
> +       /* set the flash content first for both devices */
> +       dm_nvmxip_flash_sanity(0, NULL);
> +       dm_nvmxip_flash_sanity(1, NULL);
> +
> +       /*  probing all NVM XIP QSPI devices */
> +       for (device_idx = 0, uclass_first_device(UCLASS_NVMXIP, &dev);
> +            dev;
> +            uclass_next_device(&dev), device_idx++) {
> +               plat_data = dev_get_plat(dev);
> +
> +               /* device tree entries checks */
> +               ut_assertok(nvmqspi_refdata[device_idx].phys_base != plat_data->phys_base);
> +               ut_assertok(nvmqspi_refdata[device_idx].lba_shift != plat_data->lba_shift);
> +               ut_assertok(nvmqspi_refdata[device_idx].lba != plat_data->lba);
> +
> +               /* before reading all the flash blocks, let's calculate the flash size */
> +               flashsz = plat_data->lba << plat_data->lba_shift;
> +
> +               /* allocate the user buffer where to copy the blocks data to */
> +               buffer = calloc(flashsz, 1);
> +               ut_assertok(!buffer);
> +
> +               /* the block device is the child of the parent device probed with DT*/
> +               ut_assertok(device_find_first_child(dev, &bdev));
> +
> +               /* reading all the flash blocks*/
> +               ut_asserteq(plat_data->lba, blk_read(bdev, 0, plat_data->lba, buffer));
> +
> +               /* compare the data read from flash with the expected data */
> +               ut_assertok(dm_nvmxip_flash_sanity(device_idx, buffer));
> +
> +               free(buffer);
> +       }
> +
> +       ut_assertok(device_idx != SANDBOX_NVMXIP_DEVICES);
> +
> +       return CMD_RET_SUCCESS;
> +}
> +
> +DM_TEST(dm_test_nvmxip, UT_TESTF_SCAN_FDT | UT_TESTF_CONSOLE_REC);
> --
> 2.17.1
>

It seems like you are using the same driver for sandbox as for the
real hardware. Is that right? There is nothing really wrong with that,
it's just unusual.

Re: [PATCH v1 0/6] introduce NVM XIP block storage emulation

[Simon Glass]

Hi Abdellatif,

On Mon, 6 Feb 2023 at 07:17, Abdellatif El Khlifi
<abdellatif.elkhlifi@arm.com> wrote:
>
> On Mon, Jan 16, 2023 at 05:28:26PM +0000, abdellatif.elkhlifi@arm.com wrote:
> > From: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
> >
> > Adding block storage emulation for NVM XIP flash devices.
> >
> > Some paltforms such as Corstone-1000 need to see NVM XIP raw flash
> > as a block storage device with read only capability.
> >
> > Here NVM flash devices are devices with addressable
> > memory (e.g: QSPI NOR flash).
> >
> > The NVM XIP block storage emulation provides the following features:
> >
> > - Emulate NVM XIP raw flash as a block storage device with read only capability
> > - Being generic by design and can be used by any platform
> > - Device tree node
> > - Platforms can use multiple NVM XIP devices at the same time by defining a
> >   DT node for each one of them
> > - A generic NVMXIP block driver allowing to read from the XIP flash
> > - A generic NVMXIP QSPI driver
> > - Implemented on top of memory-mapped IO (using readq macro)
> > - Enabling NVMXIP in sandbox64
> > - A sandbox test case
> > - Enabling NVMXIP in Corstone1000 platform as a use case
> >
> > For more details please refer to the readme [A].
> >
> > [A]: doc/develop/driver-model/nvmxip.rst
> >
> > Cheers,
> > Abdellatif
> >
> > Cc: Tom Rini <trini@konsulko.com>
> > Cc: Simon Glass <sjg@chromium.org>
> > Cc: Drew Reed <Drew.Reed@arm.com>
> > Cc: Xueliang Zhong <Xueliang.Zhong@arm.com>
> >
> > Abdellatif El Khlifi (6):
> >   drivers/nvmxip: introduce NVM XIP block storage emulation
> >   sandbox64: fix: return unsigned long in readq()
> >   sandbox64: add support for NVMXIP QSPI
> >   corstone1000: add NVM XIP QSPI device tree node
> >   corstone1000: enable NVM XIP QSPI flash
> >   sandbox64: add a test case for UCLASS_NVMXIP
> >
> >  MAINTAINERS                                |   8 ++
> >  arch/arm/dts/corstone1000.dtsi             |   9 +-
> >  arch/sandbox/cpu/cpu.c                     |   2 +-
> >  arch/sandbox/dts/sandbox64.dts             |  13 ++
> >  arch/sandbox/dts/test.dts                  |  14 +++
> >  arch/sandbox/include/asm/io.h              |   2 +-
> >  configs/corstone1000_defconfig             |   1 +
> >  configs/sandbox64_defconfig                |   1 +
> >  doc/develop/driver-model/index.rst         |   1 +
> >  doc/develop/driver-model/nvmxip.rst        |  70 +++++++++++
> >  doc/device-tree-bindings/nvmxip/nvmxip.txt |  56 +++++++++
> >  drivers/Kconfig                            |   2 +
> >  drivers/Makefile                           |   1 +
> >  drivers/block/blk-uclass.c                 |   1 +
> >  drivers/nvmxip/Kconfig                     |  19 +++
> >  drivers/nvmxip/Makefile                    |   8 ++
> >  drivers/nvmxip/nvmxip-uclass.c             |  15 +++
> >  drivers/nvmxip/nvmxip.c                    | 133 +++++++++++++++++++++
> >  drivers/nvmxip/nvmxip.h                    |  51 ++++++++
> >  drivers/nvmxip/nvmxip_qspi.c               |  67 +++++++++++
> >  include/dm/uclass-id.h                     |   1 +
> >  test/dm/Makefile                           |   5 +
> >  test/dm/nvmxip.c                           | 117 ++++++++++++++++++
> >  23 files changed, 594 insertions(+), 3 deletions(-)
> >  create mode 100644 doc/develop/driver-model/nvmxip.rst
> >  create mode 100644 doc/device-tree-bindings/nvmxip/nvmxip.txt
> >  create mode 100644 drivers/nvmxip/Kconfig
> >  create mode 100644 drivers/nvmxip/Makefile
> >  create mode 100644 drivers/nvmxip/nvmxip-uclass.c
> >  create mode 100644 drivers/nvmxip/nvmxip.c
> >  create mode 100644 drivers/nvmxip/nvmxip.h
> >  create mode 100644 drivers/nvmxip/nvmxip_qspi.c
> >  create mode 100644 test/dm/nvmxip.c
> >
> > --
> > 2.17.1
> >
>
> Hi guys, a gentle reminder. Any feedback please ?

From what I can tell, I wasn't copied...I mostly don't see patches
without a cc, sorry.

Regards,
Simon

Re: [PATCH v1 3/6] sandbox64: add support for NVMXIP QSPI

[Tom Rini]

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

On Mon, Feb 06, 2023 at 09:02:38PM -0700, Simon Glass wrote:
> On Mon, 16 Jan 2023 at 10:28, <abdellatif.elkhlifi@arm.com> wrote:
> >
> > From: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
> >
> > enable NVMXIP QSPI for sandbox 64-bit
> >
> > Adding two NVM XIP QSPI storage devices.
> >
> > Signed-off-by: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
> > ---
> >  arch/sandbox/dts/sandbox64.dts | 13 +++++++++++++
> >  arch/sandbox/dts/test.dts      | 14 ++++++++++++++
> >  configs/sandbox64_defconfig    |  1 +
> >  drivers/nvmxip/nvmxip.c        |  4 ++++
> >  drivers/nvmxip/nvmxip.h        |  3 +++
> >  5 files changed, 35 insertions(+)
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> 
> Nit below
> 
> >
> > diff --git a/arch/sandbox/dts/sandbox64.dts b/arch/sandbox/dts/sandbox64.dts
> > index a9cd7908f8..aed3801af8 100644
> > --- a/arch/sandbox/dts/sandbox64.dts
> > +++ b/arch/sandbox/dts/sandbox64.dts
> > @@ -89,6 +89,19 @@
> >                 cs-gpios = <0>, <&gpio_a 0>;
> >         };
> >
> > +       nvmxip-qspi1@08000000 {
> > +               compatible = "nvmxip,qspi";
> > +               reg = <0x0 0x08000000 0x0 0x00200000>;
> > +               lba_shift = <9>;
> > +               lba = <4096>;
> > +       };
> > +
> > +       nvmxip-qspi2@08200000 {
> > +               compatible = "nvmxip,qspi";
> > +               reg = <0x0 0x08200000 0x0 0x00100000>;
> > +               lba_shift = <9>;
> > +               lba = <2048>;
> > +       };
> >  };
> >
> >  #include "sandbox.dtsi"
> > diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
> > index dffe10adbf..c3ba0a225e 100644
> > --- a/arch/sandbox/dts/test.dts
> > +++ b/arch/sandbox/dts/test.dts
> > @@ -1745,6 +1745,20 @@
> >                 compatible = "u-boot,fwu-mdata-gpt";
> >                 fwu-mdata-store = <&mmc0>;
> >         };
> > +
> > +       nvmxip-qspi1@08000000 {
> > +               compatible = "nvmxip,qspi";
> > +               reg = <0x08000000 0x00200000>;
> > +               lba_shift = <9>;
> > +               lba = <4096>;
> > +       };
> > +
> > +       nvmxip-qspi2@08200000 {
> > +               compatible = "nvmxip,qspi";
> > +               reg = <0x08200000 0x00100000>;
> > +               lba_shift = <9>;
> > +               lba = <2048>;
> > +       };
> >  };
> >
> >  #include "sandbox_pmic.dtsi"
> > diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig
> > index ba45ac0b71..cd4dadb190 100644
> > --- a/configs/sandbox64_defconfig
> > +++ b/configs/sandbox64_defconfig
> > @@ -259,3 +259,4 @@ CONFIG_FWU_MULTI_BANK_UPDATE=y
> >  CONFIG_UNIT_TEST=y
> >  CONFIG_UT_TIME=y
> >  CONFIG_UT_DM=y
> > +CONFIG_NVMXIP_QSPI=y
> > \ No newline at end of file
> > diff --git a/drivers/nvmxip/nvmxip.c b/drivers/nvmxip/nvmxip.c
> > index c276fb147e..ea9b9bfa48 100644
> > --- a/drivers/nvmxip/nvmxip.c
> > +++ b/drivers/nvmxip/nvmxip.c
> > @@ -87,6 +87,10 @@ int nvmxip_init(struct udevice *udev)
> >         priv_data->udev = udev;
> >         priv_data->plat_data = plat_data;
> >
> > +#if CONFIG_IS_ENABLED(SANDBOX64)
> 
> if (IS_ENABLED(CONFIG_SANDBOX64))
> 
> 
> > +       sandbox_set_enable_memio(true);
> > +#endif

Isn't that just going to introduce a warning about undeclared function
on non-sandbox64?

-- 
Tom

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

Re: [PATCH v1 3/6] sandbox64: add support for NVMXIP QSPI

[Simon Glass]

Hi,

On Tue, 7 Feb 2023 at 09:30, Tom Rini <trini@konsulko.com> wrote:
>
> On Mon, Feb 06, 2023 at 09:02:38PM -0700, Simon Glass wrote:
> > On Mon, 16 Jan 2023 at 10:28, <abdellatif.elkhlifi@arm.com> wrote:
> > >
> > > From: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
> > >
> > > enable NVMXIP QSPI for sandbox 64-bit
> > >
> > > Adding two NVM XIP QSPI storage devices.
> > >
> > > Signed-off-by: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
> > > ---
> > >  arch/sandbox/dts/sandbox64.dts | 13 +++++++++++++
> > >  arch/sandbox/dts/test.dts      | 14 ++++++++++++++
> > >  configs/sandbox64_defconfig    |  1 +
> > >  drivers/nvmxip/nvmxip.c        |  4 ++++
> > >  drivers/nvmxip/nvmxip.h        |  3 +++
> > >  5 files changed, 35 insertions(+)
> >
> > Reviewed-by: Simon Glass <sjg@chromium.org>
> >
> > Nit below
> >
> > >
> > > diff --git a/arch/sandbox/dts/sandbox64.dts b/arch/sandbox/dts/sandbox64.dts
> > > index a9cd7908f8..aed3801af8 100644
> > > --- a/arch/sandbox/dts/sandbox64.dts
> > > +++ b/arch/sandbox/dts/sandbox64.dts
> > > @@ -89,6 +89,19 @@
> > >                 cs-gpios = <0>, <&gpio_a 0>;
> > >         };
> > >
> > > +       nvmxip-qspi1@08000000 {
> > > +               compatible = "nvmxip,qspi";
> > > +               reg = <0x0 0x08000000 0x0 0x00200000>;
> > > +               lba_shift = <9>;
> > > +               lba = <4096>;
> > > +       };
> > > +
> > > +       nvmxip-qspi2@08200000 {
> > > +               compatible = "nvmxip,qspi";
> > > +               reg = <0x0 0x08200000 0x0 0x00100000>;
> > > +               lba_shift = <9>;
> > > +               lba = <2048>;
> > > +       };
> > >  };
> > >
> > >  #include "sandbox.dtsi"
> > > diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
> > > index dffe10adbf..c3ba0a225e 100644
> > > --- a/arch/sandbox/dts/test.dts
> > > +++ b/arch/sandbox/dts/test.dts
> > > @@ -1745,6 +1745,20 @@
> > >                 compatible = "u-boot,fwu-mdata-gpt";
> > >                 fwu-mdata-store = <&mmc0>;
> > >         };
> > > +
> > > +       nvmxip-qspi1@08000000 {
> > > +               compatible = "nvmxip,qspi";
> > > +               reg = <0x08000000 0x00200000>;
> > > +               lba_shift = <9>;
> > > +               lba = <4096>;
> > > +       };
> > > +
> > > +       nvmxip-qspi2@08200000 {
> > > +               compatible = "nvmxip,qspi";
> > > +               reg = <0x08200000 0x00100000>;
> > > +               lba_shift = <9>;
> > > +               lba = <2048>;
> > > +       };
> > >  };
> > >
> > >  #include "sandbox_pmic.dtsi"
> > > diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig
> > > index ba45ac0b71..cd4dadb190 100644
> > > --- a/configs/sandbox64_defconfig
> > > +++ b/configs/sandbox64_defconfig
> > > @@ -259,3 +259,4 @@ CONFIG_FWU_MULTI_BANK_UPDATE=y
> > >  CONFIG_UNIT_TEST=y
> > >  CONFIG_UT_TIME=y
> > >  CONFIG_UT_DM=y
> > > +CONFIG_NVMXIP_QSPI=y
> > > \ No newline at end of file
> > > diff --git a/drivers/nvmxip/nvmxip.c b/drivers/nvmxip/nvmxip.c
> > > index c276fb147e..ea9b9bfa48 100644
> > > --- a/drivers/nvmxip/nvmxip.c
> > > +++ b/drivers/nvmxip/nvmxip.c
> > > @@ -87,6 +87,10 @@ int nvmxip_init(struct udevice *udev)
> > >         priv_data->udev = udev;
> > >         priv_data->plat_data = plat_data;
> > >
> > > +#if CONFIG_IS_ENABLED(SANDBOX64)
> >
> > if (IS_ENABLED(CONFIG_SANDBOX64))
> >
> >
> > > +       sandbox_set_enable_memio(true);
> > > +#endif
>
> Isn't that just going to introduce a warning about undeclared function
> on non-sandbox64?

Well, the header file should always be included, so it shouldn't!

Please also add a newline to the defconfig.

Regards,
Simon

Re: [PATCH v1 1/6] drivers/nvmxip: introduce NVM XIP block storage emulation

[Simon Glass]

Hi,

On Mon, 16 Jan 2023 at 10:28, <abdellatif.elkhlifi@arm.com> wrote:
>
> From: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
>
> add block storage emulation for NVM XIP flash devices
>
> Some paltforms such as Corstone-1000 need to see NVM XIP raw flash
> as a block storage device with read only capability.

As mentioned I didn't see this patch originally. It generally looks OK
but have made some comments below.

>
> Here NVM flash devices are devices with addressable
> memory (e.g: QSPI NOR flash).
>
> The implementation is generic and can be used by different platforms.
>
> Two drivers are provided as follows.
>
>   nvmxip-blk :
>
>     a generic block driver allowing to read from the XIP flash
>
>   nvmxip_qspi :
>
>     The driver probed with the DT and parent of the nvmxip-blk device.
>     nvmxip_qspi can be reused by other platforms. If the platform
>     has custom settings to apply before using the flash, then the platform
>     can provide its own parent driver belonging to UCLASS_NVMXIP and reuse
>     nvmxip-blk. The custom driver can be implmented like nvmxip_qspi in
>     addition to the platform custom settings.
>
> Platforms can use multiple NVM XIP devices at the same time by defining a
> DT node for each one of them.
>
> For more details please refer to doc/develop/driver-model/nvmxip.rst
>
> Signed-off-by: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
> ---
>  MAINTAINERS                                |   7 ++
>  doc/develop/driver-model/index.rst         |   1 +
>  doc/develop/driver-model/nvmxip.rst        |  70 +++++++++++
>  doc/device-tree-bindings/nvmxip/nvmxip.txt |  56 +++++++++
>  drivers/Kconfig                            |   2 +
>  drivers/Makefile                           |   1 +
>  drivers/block/blk-uclass.c                 |   1 +
>  drivers/nvmxip/Kconfig                     |  19 +++
>  drivers/nvmxip/Makefile                    |   8 ++
>  drivers/nvmxip/nvmxip-uclass.c             |  15 +++
>  drivers/nvmxip/nvmxip.c                    | 129 +++++++++++++++++++++
>  drivers/nvmxip/nvmxip.h                    |  48 ++++++++
>  drivers/nvmxip/nvmxip_qspi.c               |  67 +++++++++++
>  include/dm/uclass-id.h                     |   1 +
>  14 files changed, 425 insertions(+)
>  create mode 100644 doc/develop/driver-model/nvmxip.rst
>  create mode 100644 doc/device-tree-bindings/nvmxip/nvmxip.txt
>  create mode 100644 drivers/nvmxip/Kconfig
>  create mode 100644 drivers/nvmxip/Makefile
>  create mode 100644 drivers/nvmxip/nvmxip-uclass.c
>  create mode 100644 drivers/nvmxip/nvmxip.c
>  create mode 100644 drivers/nvmxip/nvmxip.h
>  create mode 100644 drivers/nvmxip/nvmxip_qspi.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index b2de50ccfc..539d01f68c 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1193,6 +1193,13 @@ F:       cmd/nvme.c
>  F:     include/nvme.h
>  F:     doc/develop/driver-model/nvme.rst
>
> +NVMXIP
> +M:     Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
> +S:     Maintained
> +F:     doc/develop/driver-model/nvmxip.rst
> +F:     doc/device-tree-bindings/nvmxip/nvmxip.txt
> +F:     drivers/nvmxip/
> +
>  NVMEM
>  M:     Sean Anderson <seanga2@gmail.com>
>  S:     Maintained
> diff --git a/doc/develop/driver-model/index.rst b/doc/develop/driver-model/index.rst
> index 7366ef818c..8e12bbd936 100644
> --- a/doc/develop/driver-model/index.rst
> +++ b/doc/develop/driver-model/index.rst
> @@ -20,6 +20,7 @@ subsystems
>     livetree
>     migration
>     nvme
> +   nvmxip
>     of-plat
>     pci-info
>     pmic-framework
> diff --git a/doc/develop/driver-model/nvmxip.rst b/doc/develop/driver-model/nvmxip.rst
> new file mode 100644
> index 0000000000..91b24e4e50
> --- /dev/null
> +++ b/doc/develop/driver-model/nvmxip.rst
> @@ -0,0 +1,70 @@
> +.. SPDX-License-Identifier: GPL-2.0+
> +
> +NVM XIP Block Storage Emulation Driver
> +=======================================
> +
> +Summary
> +-------
> +
> +Non-Volatile Memory devices with addressable memory (e.g: QSPI NOR flash) could
> +be used for block storage needs (e.g: parsing a GPT layout in a raw QSPI NOR flash).
> +
> +The NVMXIP class provides this functionality and can be used for any 64-bit platform.
> +
> +The NVMXIP class provides the following drivers:
> +
> +      nvmxip-blk :
> +
> +        A generic block driver allowing to read from the XIP flash.
> +       The driver belongs to UCLASS_BLK.
> +       The driver implemented by drivers/nvmxip/nvmxip.c
> +
> +      nvmxip_qspi :
> +
> +        The driver probed with the DT and parent of the nvmxip-blk device.
> +        nvmxip_qspi can be reused by other platforms. If the platform
> +        has custom settings to apply before using the flash, then the platform
> +        can provide its own parent driver belonging to UCLASS_NVMXIP and reuse
> +        nvmxip-blk. The custom driver can be implmented like nvmxip_qspi in
> +        addition to the platform custom settings.
> +       The nvmxip_qspi driver belongs to UCLASS_NVMXIP.
> +       The driver implemented by drivers/nvmxip/nvmxip_qspi.c
> +
> +    The implementation is generic and can be used by different platforms.
> +
> +Supported hardware
> +--------------------------------
> +
> +Any 64-bit plaform.

platform

Why not 64-bit ? Please mention that.

> +
> +Configuration
> +----------------------
> +
> +config NVMXIP
> +         This option allows the emulation of a block storage device
> +         on top of a direct access non volatile memory XIP flash devices.
> +         This support provides the read operation.
> +         This option provides the block storage driver nvmxip-blk which
> +         handles the read operation. This driver is HW agnostic and can support
> +         multiple flash devices at the same time.
> +
> +config NVMXIP_QSPI
> +         This option allows the emulation of a block storage device on top of a QSPI XIP flash.
> +         Any platform that needs to emulate one or multiple XIP flash devices can turn this
> +         option on to enable the functionality. NVMXIP config is selected automatically.
> +         Platforms that need to add custom treatments before accessing to the flash, can
> +         write their own driver (same as nvmxip_qspi in addition to the custom settings).
> +
> +Device Tree nodes
> +--------------------
> +
> +Multiple XIP flash devices can be used at the same time by describing them through DT
> +nodes.
> +
> +Please refer to the documentation of the DT binding at:
> +
> +doc/device-tree-bindings/nvmxip/nvmxip.txt
> +
> +Contributors
> +------------
> +   * Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
> diff --git a/doc/device-tree-bindings/nvmxip/nvmxip.txt b/doc/device-tree-bindings/nvmxip/nvmxip.txt
> new file mode 100644
> index 0000000000..7c4b03f66b
> --- /dev/null
> +++ b/doc/device-tree-bindings/nvmxip/nvmxip.txt
> @@ -0,0 +1,56 @@
> +Specifying NVMXIP information for devices
> +======================================
> +
> +NVM XIP flash device nodes
> +---------------------------
> +
> +Each flash device should have its own node.
> +
> +Each node must specify the following fields:
> +
> +1)
> +               compatible = "nvmxip,qspi";
> +
> +This allows to bind the flash device with the nvmxip_qspi driver
> +If a platform has its own driver, please provide your own compatible
> +string.
> +
> +2)
> +               reg = <0x0 0x08000000 0x0 0x00200000>;

Do we not use regs = /bits/ 64 <...? ?

> +
> +The start address and size of the flash device. The values give here are an
> +example (when the cell size is 2).
> +
> +When cell size is 1, the reg field looks like this:
> +
> +               reg = <0x08000000 0x00200000>;
> +
> +3)
> +
> +               lba_shift = <9>;
> +
> +The number of bit shifts used to calculate the size in bytes of one block.
> +In this example the block size is 1 << 9 = 2 ^ 9 = 512 bytes
> +
> +4)
> +
> +               lba = <4096>;
> +
> +The number of blocks.
> +
> +Example of multiple flash devices
> +----------------------------------------------------
> +
> +       nvmxip-qspi1@08000000 {
> +               compatible = "nvmxip,qspi";
> +               reg = <0x0 0x08000000 0x0 0x00200000>;
> +               lba_shift = <9>;
> +               lba = <4096>;
> +       };
> +
> +       nvmxip-qspi2@08200000 {
> +               compatible = "nvmxip,qspi";
> +               reg = <0x0 0x08200000 0x0 0x00100000>;
> +               lba_shift = <9>;
> +               lba = <2048>;
> +       };
> diff --git a/drivers/Kconfig b/drivers/Kconfig
> index 9101e538b0..ef321bee94 100644
> --- a/drivers/Kconfig
> +++ b/drivers/Kconfig
> @@ -76,6 +76,8 @@ source "drivers/net/Kconfig"
>
>  source "drivers/nvme/Kconfig"
>
> +source "drivers/nvmxip/Kconfig"
> +
>  source "drivers/pci/Kconfig"
>
>  source "drivers/pci_endpoint/Kconfig"
> diff --git a/drivers/Makefile b/drivers/Makefile
> index 83b14ef1fd..5e7fd6dab5 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -89,6 +89,7 @@ obj-$(CONFIG_FWU_MDATA) += fwu-mdata/
>  obj-y += misc/
>  obj-$(CONFIG_MMC) += mmc/
>  obj-$(CONFIG_NVME) += nvme/
> +obj-$(CONFIG_NVMXIP) += nvmxip/

Do you think this justifies its own directory? Do we expect more
drivers? Another option would be drivers/mtd

Is there an equivalent Linux driver?

>  obj-$(CONFIG_PCI_ENDPOINT) += pci_endpoint/
>  obj-y += dfu/
>  obj-$(CONFIG_PCH) += pch/
> diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c
> index c69fc4d518..e8ab576c32 100644
> --- a/drivers/block/blk-uclass.c
> +++ b/drivers/block/blk-uclass.c
> @@ -28,6 +28,7 @@ static struct {
>         { UCLASS_AHCI, "sata" },
>         { UCLASS_HOST, "host" },
>         { UCLASS_NVME, "nvme" },
> +       { UCLASS_NVMXIP, "nvmxip" },
>         { UCLASS_EFI_MEDIA, "efi" },
>         { UCLASS_EFI_LOADER, "efiloader" },
>         { UCLASS_VIRTIO, "virtio" },
> diff --git a/drivers/nvmxip/Kconfig b/drivers/nvmxip/Kconfig
> new file mode 100644
> index 0000000000..3ef7105026
> --- /dev/null
> +++ b/drivers/nvmxip/Kconfig
> @@ -0,0 +1,19 @@
> +# SPDX-License-Identifier: GPL-2.0+
> +#
> +# Copyright 2023 Arm Limited and/or its affiliates <open-source-office@arm.com>
> +# Authors:
> +#   Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
> +
> +config NVMXIP
> +       bool "NVM XIP devices support"
> +       select BLK
> +       help
> +         This option allows the emulation of a block storage device
> +         on top of a direct access non volatile memory XIP flash devices.
> +         This support provides the read operation.
> +
> +config NVMXIP_QSPI
> +       bool "QSPI XIP  support"
> +       select NVMXIP
> +       help
> +         This option allows the emulation of a block storage device on top of a QSPI XIP flash
> diff --git a/drivers/nvmxip/Makefile b/drivers/nvmxip/Makefile
> new file mode 100644
> index 0000000000..54eacc102e
> --- /dev/null
> +++ b/drivers/nvmxip/Makefile
> @@ -0,0 +1,8 @@
> +# SPDX-License-Identifier: GPL-2.0+
> +#
> +# Copyright 2023 Arm Limited and/or its affiliates <open-source-office@arm.com>
> +# Authors:
> +#   Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
> +
> +obj-y += nvmxip-uclass.o nvmxip.o
> +obj-$(CONFIG_NVMXIP_QSPI) += nvmxip_qspi.o
> diff --git a/drivers/nvmxip/nvmxip-uclass.c b/drivers/nvmxip/nvmxip-uclass.c
> new file mode 100644
> index 0000000000..aed8e163d2
> --- /dev/null
> +++ b/drivers/nvmxip/nvmxip-uclass.c
> @@ -0,0 +1,15 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2023 Arm Limited and/or its affiliates <open-source-office@arm.com>
> + *
> + * Authors:
> + *   Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
> + */
> +
> +#include <common.h>
> +#include <dm.h>
> +
> +UCLASS_DRIVER(nvmxip) = {
> +       .name   = "nvmxip",
> +       .id     = UCLASS_NVMXIP,
> +};
> diff --git a/drivers/nvmxip/nvmxip.c b/drivers/nvmxip/nvmxip.c
> new file mode 100644
> index 0000000000..c276fb147e
> --- /dev/null
> +++ b/drivers/nvmxip/nvmxip.c
> @@ -0,0 +1,129 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2023 Arm Limited and/or its affiliates <open-source-office@arm.com>
> + *
> + * Authors:
> + *   Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
> + */
> +
> +#include <common.h>
> +#include <dm.h>
> +#include <dm/device-internal.h>
> +#include "nvmxip.h"
> +
> +static u32 nvmxip_bdev_max_devs;

Please put that in the uclass-private data. We don't want statics,
etc. with driver model.

> +
> +static int nvmxip_mmio_rawread(const phys_addr_t address, u64 *value)
> +{
> +       *value = readq(address);
> +       return 0;
> +}
> +
> +static ulong nvmxip_blk_read(struct udevice *udev, lbaint_t blknr, lbaint_t blkcnt, void *buffer)
> +{
> +       struct nvmxip_blk_priv *bpriv_data = dev_get_priv(udev);
> +       struct blk_desc *desc = dev_get_uclass_plat(udev);
> +

drop blank line, and can you combine the two following comments:

> +       /* size of 1 block */
> +       /* number of the u64 words to read */
> +       u32 qwords = (blkcnt * desc->blksz) / sizeof(u64);
> +       /* physical address of the first block to read */
> +       phys_addr_t blkaddr = bpriv_data->pplat_data->phys_base + blknr * desc->blksz;
> +       u64 *virt_blkaddr;
> +       u64 *pdst = buffer;
> +       u32 qdata_idx;

Why is this u32? It should be uint. Try to use natural types for loops
and function parameters unless there is a very good reason.

> +
> +       if (!pdst)
> +               return -EINVAL;
> +
> +       pr_debug("[%s]: reading from blknr: %lu , blkcnt: %lu\n", udev->name, blknr, blkcnt);
> +
> +       virt_blkaddr = map_sysmem(blkaddr, 0);
> +
> +       /* assumption: the data is virtually contiguous */
> +
[..]

> +
> +int nvmxip_init(struct udevice *udev)

Why is this function exported? It should probably be called
nvmxip_post_bind() and be called by the uclass when a new device is
bound?

> +{
> +       struct nvmxip_plat *plat_data = dev_get_plat(udev);

plat

> +       struct nvmxip_priv *priv_data = dev_get_priv(udev);

priv

(please use these names throughout as that is the convention; it makes
it easier for people to understand your code)

> +       int ret;
> +       struct udevice *bdev = NULL;
> +       char bdev_name[NVMXIP_BLKDEV_NAME_SZ + 1] = {0};

You don't need to init it as you write to it below.

> +
> +       priv_data->udev = udev;
> +       priv_data->plat_data = plat_data;
> +
> +       nvmxip_bdev_max_devs++;
> +
> +       snprintf(bdev_name, NVMXIP_BLKDEV_NAME_SZ, "nvmxip-blk#%d", nvmxip_bdev_max_devs);
> +
> +       ret = blk_create_devicef(udev, NVMXIP_BLKDRV_NAME, bdev_name, UCLASS_NVMXIP,
> +                                nvmxip_bdev_max_devs, NVMXIP_DEFAULT_LBA_SZ,
> +                                NVMXIP_DEFAULT_LBA_COUNT, &bdev);
> +       if (ret) {
> +               pr_err("[%s]: failure during creation of the block device %s, error %d\n",
> +                      udev->name, bdev_name, ret);
> +               goto blkdev_setup_error;
> +       }
> +
> +       ret = blk_probe_or_unbind(bdev);
> +       if (ret) {
> +               pr_err("[%s]: failure during probing the block device %s, error %d\n",
> +                      udev->name, bdev_name, ret);
> +               goto blkdev_setup_error;
> +       }
> +
> +       pr_info("[%s]: the block device %s ready for use\n", udev->name, bdev_name);
> +
> +       return 0;
> +
> +blkdev_setup_error:
> +       nvmxip_bdev_max_devs--;

I'm not sure if it helps, but there is a uclass_id_count() you can use.

> +       return ret;
> +}
> +
> +static const struct blk_ops nvmxip_blk_ops = {
> +       .read   = nvmxip_blk_read,
> +};
> +
> +U_BOOT_DRIVER(nvmxip_blk) = {
> +       .name   = NVMXIP_BLKDRV_NAME,
> +       .id     = UCLASS_BLK,
> +       .probe  = nvmxip_blk_probe,
> +       .ops    = &nvmxip_blk_ops,
> +       .priv_auto      = sizeof(struct nvmxip_blk_priv),
> +};
> diff --git a/drivers/nvmxip/nvmxip.h b/drivers/nvmxip/nvmxip.h
> new file mode 100644
> index 0000000000..47ae3bd8ab
> --- /dev/null
> +++ b/drivers/nvmxip/nvmxip.h
> @@ -0,0 +1,48 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright 2023 Arm Limited and/or its affiliates <open-source-office@arm.com>
> + *
> + * Authors:
> + *   Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
> + */
> +
> +#ifndef __DRIVER_NVMXIP_H__
> +#define __DRIVER_NVMXIP_H__
> +
> +#include <asm/io.h>
> +#include <blk.h>
> +#include <linux/bitops.h>
> +#include <linux/compat.h>
> +#include <mapmem.h>

Please put blk and mapmem in the C file, at least. Check if you need
the others here, too.


> +
> +#define NVMXIP_BLKDRV_NAME    "nvmxip-blk"
> +
> +#define NVMXIP_BLKDEV_NAME_SZ 20
> +
> +#define NVMXIP_DEFAULT_LBA_SHIFT 10 /* 1024 bytes per block */
> +#define NVMXIP_DEFAULT_LBA_COUNT 1024 /* block count */
> +
> +#define NVMXIP_DEFAULT_LBA_SZ BIT(NVMXIP_DEFAULT_LBA_SHIFT)

This is a private header so you don't need the long names - e.g. drop
the NVMXIP_
> +
> +/* NVM XIP device platform data */

I suggest using sphinx style right from the start, since this is a new file.

> +struct nvmxip_plat {
> +       phys_addr_t phys_base; /* NVM XIP device base address */
> +       u32 lba_shift; /* block size shift count (read from device tree) */
> +       lbaint_t lba; /* number of blocks (read from device tree) */
> +};
> +
> +/* NVM XIP device private data */
> +struct nvmxip_priv {
> +       struct udevice *udev;
> +       struct nvmxip_plat *plat_data;

Please don't store private data from devices. You can use
dev_get_plat() or whatever on a stored device. It just adds confusion.

> +};
> +
> +/* Private data of the block device associated with the NVM XIP device (the parent) */
> +struct nvmxip_blk_priv {
> +       struct udevice *bdev;

What is that?

> +       struct nvmxip_plat *pplat_data; /* parent device platform data */

same here

> +};
> +
> +int nvmxip_init(struct udevice *udev);

should not be exported

> +
> +#endif /* __DRIVER_NVMXIP_H__ */
> diff --git a/drivers/nvmxip/nvmxip_qspi.c b/drivers/nvmxip/nvmxip_qspi.c
> new file mode 100644
> index 0000000000..d0ce4c6633
> --- /dev/null
> +++ b/drivers/nvmxip/nvmxip_qspi.c

Can you put the driver in a separate patch? It is a good idea to have
the uclass in a patch by itself.

> @@ -0,0 +1,67 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2023 Arm Limited and/or its affiliates <open-source-office@arm.com>
> + *
> + * Authors:
> + *   Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
> + */
> +
> +#include <common.h>
> +#include <dm.h>
> +#include <fdt_support.h>
> +#include "nvmxip.h"
> +
> +#include <asm/global_data.h>
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +#define NVMXIP_QSPI_DRV_NAME "nvmxip_qspi"
> +
> +static int nvmxip_qspi_probe(struct udevice *dev)
> +{
> +       pr_debug("[%s][%s]\n", __func__, dev->name);
> +       return nvmxip_init(dev);
> +}
> +
> +static int nvmxip_qspi_of_to_plat(struct udevice *dev)
> +{
> +       struct nvmxip_plat *plat_data = dev_get_plat(dev);
> +       int ret;
> +
> +       plat_data->phys_base = (phys_addr_t)dev_read_addr(dev);
> +       if (plat_data->phys_base == FDT_ADDR_T_NONE) {
> +               pr_err("[%s]: can not get base address from device tree\n", dev->name);

This is adding a lot to code size...do you want to use debug instead?

> +               return -EINVAL;
> +       }
> +
> +       ret = dev_read_u32(dev, "lba_shift", &plat_data->lba_shift);
> +       if (ret) {
> +               pr_err("[%s]: can not get lba_shift from device tree\n", dev->name);
> +               return -EINVAL;
> +       }
> +
> +       ret = dev_read_u32(dev, "lba", (u32 *)&plat_data->lba);
> +       if (ret) {
> +               pr_err("[%s]: can not get lba from device tree\n", dev->name);
> +               return -EINVAL;
> +       }
> +
> +       pr_debug("[%s]: XIP device base addr: 0x%llx , lba_shift: %d , lbas: %lu\n",
> +                dev->name, plat_data->phys_base, plat_data->lba_shift, plat_data->lba);
> +
> +       return 0;
> +}
> +
> +static const struct udevice_id nvmxip_qspi_ids[] = {
> +       { .compatible = "nvmxip,qspi" },
> +       { /* sentinel */ }
> +};
> +
> +U_BOOT_DRIVER(nvmxip_qspi) = {
> +       .name = NVMXIP_QSPI_DRV_NAME,
> +       .id = UCLASS_NVMXIP,
> +       .of_match = nvmxip_qspi_ids,
> +       .of_to_plat = nvmxip_qspi_of_to_plat,
> +       .priv_auto = sizeof(struct nvmxip_priv),
> +       .plat_auto = sizeof(struct nvmxip_plat),
> +       .probe = nvmxip_qspi_probe,
> +};
> diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
> index 376f741cc2..dcfe8cfe2c 100644
> --- a/include/dm/uclass-id.h
> +++ b/include/dm/uclass-id.h
> @@ -88,6 +88,7 @@ enum uclass_id {
>         UCLASS_NOP,             /* No-op devices */
>         UCLASS_NORTHBRIDGE,     /* Intel Northbridge / SDRAM controller */
>         UCLASS_NVME,            /* NVM Express device */
> +       UCLASS_NVMXIP,          /* NVM XIP devices */
>         UCLASS_P2SB,            /* (x86) Primary-to-Sideband Bus */
>         UCLASS_PANEL,           /* Display panel, such as an LCD */
>         UCLASS_PANEL_BACKLIGHT, /* Backlight controller for panel */
> --
> 2.17.1
>

Regards,
Simon

[PATCH v2 0/7] introduce NVM XIP block storage emulation

[Abdellatif El Khlifi]

Adding block storage emulation for NVM XIP flash devices.

Some paltforms such as Corstone-1000 need to see NVM XIP raw flash
as a block storage device with read only capability.

Here NVM flash devices are devices with addressable
memory (e.g: QSPI NOR flash).

The NVM XIP block storage emulation provides the following features:

- Emulate NVM XIP raw flash as a block storage device with read only capability
- Being generic by design and can be used by any platform
- Device tree node
- Platforms can use multiple NVM XIP devices at the same time by defining a
  DT node for each one of them
- A generic NVMXIP block driver allowing to read from the XIP flash
- A generic NVMXIP Uclass driver for binding the block device
- A generic NVMXIP QSPI driver
- Implemented on top of memory-mapped IO (using readq macro)
- Enabling NVMXIP in sandbox64
- A sandbox test case
- Enabling NVMXIP in Corstone1000 platform as a use case

For more details please refer to the readme [A].

Changelog of the major changes:
===========================

v2:

* move the drivers under drivers/mtd
* shorten the block device name
* rename nvmxip_init() to nvmxip_post_bind(), call it from the uclass when
   a new device is bound
* use uclass_id_count() in place of nvmxip_bdev_max_devs
* moving the NVMXIP QSPI driver to a seperate commit
* address nits

v1: [1]

* introduce the new feature

Cheers,
Abdellatif

List of previous patches:

[1]: https://lore.kernel.org/all/20230116172832.1946-1-abdellatif.elkhlifi@arm.com/

More details:

[A]: doc/develop/driver-model/nvmxip.rst

Cc: Tom Rini <trini@konsulko.com>
Cc: Simon Glass <sjg@chromium.org>
Cc: Drew Reed <Drew.Reed@arm.com>
Cc: Xueliang Zhong <Xueliang.Zhong@arm.com>
Cc: Rui Miguel Silva <rui.silva@linaro.org>

Abdellatif El Khlifi (7):
  drivers/mtd/nvmxip: introduce NVM XIP block storage emulation
  drivers/mtd/nvmxip: introduce QSPI XIP driver
  sandbox64: fix: return unsigned long in readq()
  sandbox64: add support for NVMXIP QSPI
  corstone1000: add NVM XIP QSPI device tree node
  corstone1000: enable NVM XIP QSPI flash
  sandbox64: add a test case for UCLASS_NVMXIP

 MAINTAINERS                                   |   8 +
 arch/arm/dts/corstone1000.dtsi                |   9 +-
 arch/sandbox/cpu/cpu.c                        |   2 +-
 arch/sandbox/dts/sandbox64.dts                |  13 ++
 arch/sandbox/dts/test.dts                     |  14 ++
 arch/sandbox/include/asm/io.h                 |   2 +-
 configs/corstone1000_defconfig                |   1 +
 configs/sandbox64_defconfig                   |   1 +
 doc/develop/driver-model/index.rst            |   1 +
 doc/develop/driver-model/nvmxip.rst           |  91 +++++++++++
 .../nvmxip/nvmxip_qspi.txt                    |  56 +++++++
 drivers/block/blk-uclass.c                    |   1 +
 drivers/mtd/Kconfig                           |   2 +
 drivers/mtd/Makefile                          |   1 +
 drivers/mtd/nvmxip/Kconfig                    |  19 +++
 drivers/mtd/nvmxip/Makefile                   |   8 +
 drivers/mtd/nvmxip/nvmxip-uclass.c            |  74 +++++++++
 drivers/mtd/nvmxip/nvmxip.c                   | 119 ++++++++++++++
 drivers/mtd/nvmxip/nvmxip.h                   |  32 ++++
 drivers/mtd/nvmxip/nvmxip_qspi.c              |  70 +++++++++
 include/dm/uclass-id.h                        |   1 +
 test/dm/Makefile                              |   5 +
 test/dm/nvmxip.c                              | 145 ++++++++++++++++++
 23 files changed, 672 insertions(+), 3 deletions(-)
 create mode 100644 doc/develop/driver-model/nvmxip.rst
 create mode 100644 doc/device-tree-bindings/nvmxip/nvmxip_qspi.txt
 create mode 100644 drivers/mtd/nvmxip/Kconfig
 create mode 100644 drivers/mtd/nvmxip/Makefile
 create mode 100644 drivers/mtd/nvmxip/nvmxip-uclass.c
 create mode 100644 drivers/mtd/nvmxip/nvmxip.c
 create mode 100644 drivers/mtd/nvmxip/nvmxip.h
 create mode 100644 drivers/mtd/nvmxip/nvmxip_qspi.c
 create mode 100644 test/dm/nvmxip.c

-- 
2.25.1

[PATCH v2 1/7] drivers/mtd/nvmxip: introduce NVM XIP block storage emulation

[Abdellatif El Khlifi]

add block storage emulation for NVM XIP flash devices

Some paltforms such as Corstone-1000 need to see NVM XIP raw flash
as a block storage device with read only capability.

Here NVM flash devices are devices with addressable
memory (e.g: QSPI NOR flash).

The implementation is generic and can be used by different platforms.

Two drivers are provided as follows.

  nvmxip-blk :

    a generic block driver allowing to read from the XIP flash

  nvmxip Uclass driver :

        When a device is described in the DT and associated with
        UCLASS_NVMXIP, the Uclass creates a block device and binds it with
	 the nvmxip-blk.

Platforms can use multiple NVM XIP devices at the same time by defining a
DT node for each one of them.

Signed-off-by: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>

---

Changelog:
===============

v2:

* move the drivers under drivers/mtd
* shorten the block device name
* rename nvmxip_init() to nvmxip_post_bind(), call it from the uclass when
   a new device is bound
* use uclass_id_count() in place of nvmxip_bdev_max_devs
* moving the NVMXIP QSPI driver to a seperate commit
* address nits

 MAINTAINERS                         |   6 ++
 doc/develop/driver-model/index.rst  |   1 +
 doc/develop/driver-model/nvmxip.rst |  48 +++++++++++
 drivers/block/blk-uclass.c          |   1 +
 drivers/mtd/Kconfig                 |   2 +
 drivers/mtd/Makefile                |   1 +
 drivers/mtd/nvmxip/Kconfig          |  13 +++
 drivers/mtd/nvmxip/Makefile         |   7 ++
 drivers/mtd/nvmxip/nvmxip-uclass.c  |  67 ++++++++++++++++
 drivers/mtd/nvmxip/nvmxip.c         | 119 ++++++++++++++++++++++++++++
 drivers/mtd/nvmxip/nvmxip.h         |  32 ++++++++
 include/dm/uclass-id.h              |   1 +
 12 files changed, 298 insertions(+)
 create mode 100644 doc/develop/driver-model/nvmxip.rst
 create mode 100644 drivers/mtd/nvmxip/Kconfig
 create mode 100644 drivers/mtd/nvmxip/Makefile
 create mode 100644 drivers/mtd/nvmxip/nvmxip-uclass.c
 create mode 100644 drivers/mtd/nvmxip/nvmxip.c
 create mode 100644 drivers/mtd/nvmxip/nvmxip.h

diff --git a/MAINTAINERS b/MAINTAINERS
index d2e245e5e9..39f143e60b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1202,6 +1202,12 @@ F:	cmd/nvme.c
 F:	include/nvme.h
 F:	doc/develop/driver-model/nvme.rst
 
+NVMXIP
+M:	Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
+S:	Maintained
+F:	doc/develop/driver-model/nvmxip.rst
+F:	drivers/mtd/nvmxip/
+
 NVMEM
 M:	Sean Anderson <seanga2@gmail.com>
 S:	Maintained
diff --git a/doc/develop/driver-model/index.rst b/doc/develop/driver-model/index.rst
index 7366ef818c..8e12bbd936 100644
--- a/doc/develop/driver-model/index.rst
+++ b/doc/develop/driver-model/index.rst
@@ -20,6 +20,7 @@ subsystems
    livetree
    migration
    nvme
+   nvmxip
    of-plat
    pci-info
    pmic-framework
diff --git a/doc/develop/driver-model/nvmxip.rst b/doc/develop/driver-model/nvmxip.rst
new file mode 100644
index 0000000000..fe087b13d2
--- /dev/null
+++ b/doc/develop/driver-model/nvmxip.rst
@@ -0,0 +1,48 @@
+.. SPDX-License-Identifier: GPL-2.0+
+
+NVM XIP Block Storage Emulation Driver
+=======================================
+
+Summary
+-------
+
+Non-Volatile Memory devices with addressable memory (e.g: QSPI NOR flash) could
+be used for block storage needs (e.g: parsing a GPT layout in a raw QSPI NOR flash).
+
+The NVMXIP Uclass provides this functionality and can be used for any 64-bit platform.
+
+The NVMXIP Uclass provides the following drivers:
+
+      nvmxip-blk block driver:
+
+        A generic block driver allowing to read from the XIP flash.
+	The driver belongs to UCLASS_BLK.
+	The driver implemented by drivers/mtd/nvmxip/nvmxip.c
+
+      nvmxip Uclass driver:
+
+        When a device is described in the DT and associated with UCLASS_NVMXIP,
+        the Uclass creates a block device and binds it with the nvmxip-blk.
+	The Uclass driver implemented by drivers/mtd/nvmxip/nvmxip-uclass.c
+
+    The implementation is generic and can be used by different platforms.
+
+Supported hardware
+--------------------------------
+
+Any 64-bit plaform.
+
+Configuration
+----------------------
+
+config NVMXIP
+	  This option allows the emulation of a block storage device
+	  on top of a direct access non volatile memory XIP flash devices.
+	  This support provides the read operation.
+	  This option provides the block storage driver nvmxip-blk which
+	  handles the read operation. This driver is HW agnostic and can support
+	  multiple flash devices at the same time.
+
+Contributors
+------------
+   * Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c
index c69fc4d518..e8ab576c32 100644
--- a/drivers/block/blk-uclass.c
+++ b/drivers/block/blk-uclass.c
@@ -28,6 +28,7 @@ static struct {
 	{ UCLASS_AHCI, "sata" },
 	{ UCLASS_HOST, "host" },
 	{ UCLASS_NVME, "nvme" },
+	{ UCLASS_NVMXIP, "nvmxip" },
 	{ UCLASS_EFI_MEDIA, "efi" },
 	{ UCLASS_EFI_LOADER, "efiloader" },
 	{ UCLASS_VIRTIO, "virtio" },
diff --git a/drivers/mtd/Kconfig b/drivers/mtd/Kconfig
index af45ef00da..5fa88dae5f 100644
--- a/drivers/mtd/Kconfig
+++ b/drivers/mtd/Kconfig
@@ -270,4 +270,6 @@ source "drivers/mtd/spi/Kconfig"
 
 source "drivers/mtd/ubi/Kconfig"
 
+source "drivers/mtd/nvmxip/Kconfig"
+
 endmenu
diff --git a/drivers/mtd/Makefile b/drivers/mtd/Makefile
index 3a78590aaa..c638980ea2 100644
--- a/drivers/mtd/Makefile
+++ b/drivers/mtd/Makefile
@@ -25,6 +25,7 @@ obj-y += nand/
 obj-y += onenand/
 obj-y += spi/
 obj-$(CONFIG_MTD_UBI) += ubi/
+obj-$(CONFIG_NVMXIP) += nvmxip/
 
 #SPL/TPL build
 else
diff --git a/drivers/mtd/nvmxip/Kconfig b/drivers/mtd/nvmxip/Kconfig
new file mode 100644
index 0000000000..ef53fc3c79
--- /dev/null
+++ b/drivers/mtd/nvmxip/Kconfig
@@ -0,0 +1,13 @@
+# SPDX-License-Identifier: GPL-2.0+
+#
+# Copyright 2023 Arm Limited and/or its affiliates <open-source-office@arm.com>
+# Authors:
+#   Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
+
+config NVMXIP
+	bool "NVM XIP devices support"
+	select BLK
+	help
+	  This option allows the emulation of a block storage device
+	  on top of a direct access non volatile memory XIP flash devices.
+	  This support provides the read operation.
diff --git a/drivers/mtd/nvmxip/Makefile b/drivers/mtd/nvmxip/Makefile
new file mode 100644
index 0000000000..07890982c7
--- /dev/null
+++ b/drivers/mtd/nvmxip/Makefile
@@ -0,0 +1,7 @@
+# SPDX-License-Identifier: GPL-2.0+
+#
+# Copyright 2023 Arm Limited and/or its affiliates <open-source-office@arm.com>
+# Authors:
+#   Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
+
+obj-y += nvmxip-uclass.o nvmxip.o
diff --git a/drivers/mtd/nvmxip/nvmxip-uclass.c b/drivers/mtd/nvmxip/nvmxip-uclass.c
new file mode 100644
index 0000000000..9f96041e3d
--- /dev/null
+++ b/drivers/mtd/nvmxip/nvmxip-uclass.c
@@ -0,0 +1,67 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2023 Arm Limited and/or its affiliates <open-source-office@arm.com>
+ *
+ * Authors:
+ *   Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <log.h>
+#include <linux/bitops.h>
+#include "nvmxip.h"
+
+/* LBA Macros */
+
+#define DEFAULT_LBA_SHIFT 10 /* 1024 bytes per block */
+#define DEFAULT_LBA_COUNT 1024 /* block count */
+
+#define DEFAULT_LBA_SZ BIT(DEFAULT_LBA_SHIFT)
+
+/**
+ * nvmxip_post_bind() - post binding treatments
+ * @dev:	the NVMXIP device
+ *
+ * Create and probe a child block device.
+ *
+ * Return:
+ *
+ * 0 on success. Otherwise, failure
+ */
+static int nvmxip_post_bind(struct udevice *udev)
+{
+	int ret;
+	struct udevice *bdev = NULL;
+	char bdev_name[NVMXIP_BLKDEV_NAME_SZ + 1];
+	int devnum;
+
+	devnum = uclass_id_count(UCLASS_NVMXIP);
+	snprintf(bdev_name, NVMXIP_BLKDEV_NAME_SZ, "blk#%d", devnum);
+
+	ret = blk_create_devicef(udev, NVMXIP_BLKDRV_NAME, bdev_name, UCLASS_NVMXIP,
+				 devnum, DEFAULT_LBA_SZ,
+				 DEFAULT_LBA_COUNT, &bdev);
+	if (ret) {
+		log_err("[%s]: failure during creation of the block device %s, error %d\n",
+			udev->name, bdev_name, ret);
+		return ret;
+	}
+
+	ret = blk_probe_or_unbind(bdev);
+	if (ret) {
+		log_err("[%s]: failure during probing the block device %s, error %d\n",
+			udev->name, bdev_name, ret);
+		return ret;
+	}
+
+	log_info("[%s]: the block device %s ready for use\n", udev->name, bdev_name);
+
+	return 0;
+}
+
+UCLASS_DRIVER(nvmxip) = {
+	.name	   = "nvmxip",
+	.id	   = UCLASS_NVMXIP,
+	.post_bind = nvmxip_post_bind,
+};
diff --git a/drivers/mtd/nvmxip/nvmxip.c b/drivers/mtd/nvmxip/nvmxip.c
new file mode 100644
index 0000000000..a359e3b482
--- /dev/null
+++ b/drivers/mtd/nvmxip/nvmxip.c
@@ -0,0 +1,119 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2023 Arm Limited and/or its affiliates <open-source-office@arm.com>
+ *
+ * Authors:
+ *   Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <log.h>
+#include <mapmem.h>
+#include <asm/io.h>
+#include <linux/bitops.h>
+#include <linux/errno.h>
+#include "nvmxip.h"
+
+/**
+ * nvmxip_mmio_rawread() - read from the XIP flash
+ * @address:	address of the data
+ * @value:	pointer to where storing the value read
+ *
+ * Read raw data from the XIP flash.
+ *
+ * Return:
+ *
+ * Always return 0.
+ */
+static int nvmxip_mmio_rawread(const phys_addr_t address, u64 *value)
+{
+	*value = readq(address);
+	return 0;
+}
+
+/**
+ * nvmxip_blk_read() - block device read operation
+ * @dev:	the block device
+ * @blknr:	first block number to read from
+ * @blkcnt:	number of blocks to read
+ * @buffer:	destination buffer
+ *
+ * Read data from the block storage device.
+ *
+ * Return:
+ *
+ * number of blocks read on success. Otherwise, failure
+ */
+static ulong nvmxip_blk_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt, void *buffer)
+{
+	struct nvmxip_plat *plat = dev_get_plat(dev->parent);
+	struct blk_desc *desc = dev_get_uclass_plat(dev);
+	/* number of the u64 words to read */
+	u32 qwords = (blkcnt * desc->blksz) / sizeof(u64);
+	/* physical address of the first block to read */
+	phys_addr_t blkaddr = plat->phys_base + blknr * desc->blksz;
+	u64 *virt_blkaddr;
+	u64 *pdst = buffer;
+	uint qdata_idx;
+
+	if (!pdst)
+		return -EINVAL;
+
+	log_debug("[%s]: reading from blknr: %lu , blkcnt: %lu\n", dev->name, blknr, blkcnt);
+
+	virt_blkaddr = map_sysmem(blkaddr, 0);
+
+	/* assumption: the data is virtually contiguous */
+
+	for (qdata_idx = 0 ; qdata_idx < qwords ; qdata_idx++)
+		nvmxip_mmio_rawread((phys_addr_t)(virt_blkaddr + qdata_idx), pdst++);
+
+	log_debug("[%s]:     src[0]: 0x%llx , dst[0]: 0x%llx , src[-1]: 0x%llx , dst[-1]: 0x%llx\n",
+		  dev->name,
+		  *virt_blkaddr,
+		  *(u64 *)buffer,
+		  *(u64 *)((u8 *)virt_blkaddr + desc->blksz * blkcnt - sizeof(u64)),
+		  *(u64 *)((u8 *)buffer + desc->blksz * blkcnt - sizeof(u64)));
+
+	unmap_sysmem(virt_blkaddr);
+
+	return blkcnt;
+}
+
+/**
+ * nvmxip_blk_probe() - block storage device probe
+ * @dev:	the block storage device
+ *
+ * Initialize the block storage descriptor.
+ *
+ * Return:
+ *
+ * Always return 0.
+ */
+static int nvmxip_blk_probe(struct udevice *dev)
+{
+	struct nvmxip_plat *plat = dev_get_plat(dev->parent);
+	struct blk_desc *desc = dev_get_uclass_plat(dev);
+
+	desc->lba = plat->lba;
+	desc->log2blksz = plat->lba_shift;
+	desc->blksz = BIT(plat->lba_shift);
+	desc->bdev = dev;
+
+	log_debug("[%s]: block storage layout\n    lbas: %lu , log2blksz: %d, blksz: %lu\n",
+		  dev->name, desc->lba, desc->log2blksz, desc->blksz);
+
+	return 0;
+}
+
+static const struct blk_ops nvmxip_blk_ops = {
+	.read	= nvmxip_blk_read,
+};
+
+U_BOOT_DRIVER(nvmxip_blk) = {
+	.name	= NVMXIP_BLKDRV_NAME,
+	.id	= UCLASS_BLK,
+	.probe	= nvmxip_blk_probe,
+	.ops	= &nvmxip_blk_ops,
+};
diff --git a/drivers/mtd/nvmxip/nvmxip.h b/drivers/mtd/nvmxip/nvmxip.h
new file mode 100644
index 0000000000..f4ef37725d
--- /dev/null
+++ b/drivers/mtd/nvmxip/nvmxip.h
@@ -0,0 +1,32 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright 2023 Arm Limited and/or its affiliates <open-source-office@arm.com>
+ *
+ * Authors:
+ *   Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
+ */
+
+#ifndef __DRIVER_NVMXIP_H__
+#define __DRIVER_NVMXIP_H__
+
+#include <blk.h>
+
+#define NVMXIP_BLKDRV_NAME    "nvmxip-blk"
+#define NVMXIP_BLKDEV_NAME_SZ 20
+
+/**
+ * struct nvmxip_plat - the NVMXIP driver plat
+ *
+ * @phys_base:	NVM XIP device base address
+ * @lba_shift:	block size shift count
+ * @lba:	number of blocks
+ *
+ * The NVMXIP information read from the DT.
+ */
+struct nvmxip_plat {
+	phys_addr_t phys_base;
+	u32 lba_shift;
+	lbaint_t lba;
+};
+
+#endif /* __DRIVER_NVMXIP_H__ */
diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
index 33e43c20db..a900c24363 100644
--- a/include/dm/uclass-id.h
+++ b/include/dm/uclass-id.h
@@ -88,6 +88,7 @@ enum uclass_id {
 	UCLASS_NOP,		/* No-op devices */
 	UCLASS_NORTHBRIDGE,	/* Intel Northbridge / SDRAM controller */
 	UCLASS_NVME,		/* NVM Express device */
+	UCLASS_NVMXIP,		/* NVM XIP devices */
 	UCLASS_P2SB,		/* (x86) Primary-to-Sideband Bus */
 	UCLASS_PANEL,		/* Display panel, such as an LCD */
 	UCLASS_PANEL_BACKLIGHT,	/* Backlight controller for panel */
-- 
2.25.1

[PATCH v2 2/7] drivers/mtd/nvmxip: introduce QSPI XIP driver

[Abdellatif El Khlifi]

add nvmxip_qspi driver under UCLASS_NVMXIP

The device associated with this driver is the parent of the blk#<id> device
nvmxip_qspi can be reused by other platforms. If the platform
has custom settings to apply before using the flash, then the platform
can provide its own parent driver belonging to UCLASS_NVMXIP and reuse
nvmxip-blk driver. The custom driver can be implemented like nvmxip_qspi in
addition to the platform custom settings.

Platforms can use multiple NVM XIP devices at the same time by defining a
DT node for each one of them.

For more details please refer to doc/develop/driver-model/nvmxip_qspi.rst

Signed-off-by: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
---
 MAINTAINERS                                   |  1 +
 doc/develop/driver-model/nvmxip.rst           | 45 +++++++++++-
 .../nvmxip/nvmxip_qspi.txt                    | 56 +++++++++++++++
 drivers/mtd/nvmxip/Kconfig                    |  6 ++
 drivers/mtd/nvmxip/Makefile                   |  1 +
 drivers/mtd/nvmxip/nvmxip_qspi.c              | 70 +++++++++++++++++++
 6 files changed, 178 insertions(+), 1 deletion(-)
 create mode 100644 doc/device-tree-bindings/nvmxip/nvmxip_qspi.txt
 create mode 100644 drivers/mtd/nvmxip/nvmxip_qspi.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 39f143e60b..77acce79c5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1206,6 +1206,7 @@ NVMXIP
 M:	Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
 S:	Maintained
 F:	doc/develop/driver-model/nvmxip.rst
+F:	doc/device-tree-bindings/nvmxip/nvmxip_qspi.txt
 F:	drivers/mtd/nvmxip/
 
 NVMEM
diff --git a/doc/develop/driver-model/nvmxip.rst b/doc/develop/driver-model/nvmxip.rst
index fe087b13d2..09afdbcccf 100644
--- a/doc/develop/driver-model/nvmxip.rst
+++ b/doc/develop/driver-model/nvmxip.rst
@@ -25,7 +25,33 @@ The NVMXIP Uclass provides the following drivers:
         the Uclass creates a block device and binds it with the nvmxip-blk.
 	The Uclass driver implemented by drivers/mtd/nvmxip/nvmxip-uclass.c
 
-    The implementation is generic and can be used by different platforms.
+      nvmxip_qspi driver :
+
+        The driver probed with the DT and is the parent of the blk#<id> device.
+        nvmxip_qspi can be reused by other platforms. If the platform
+        has custom settings to apply before using the flash, then the platform
+        can provide its own parent driver belonging to UCLASS_NVMXIP and reuse
+        nvmxip-blk. The custom driver can be implemented like nvmxip_qspi in
+        addition to the platform custom settings.
+	The nvmxip_qspi driver belongs to UCLASS_NVMXIP.
+	The driver implemented by drivers/mtd/nvmxip/nvmxip_qspi.c
+
+	For example, if we have two NVMXIP devices described in the DT
+	The devices hierarchy is as follows:
+
+::
+
+   => dm tree
+
+        Class     Index  Probed  Driver                Name
+    -----------------------------------------------------------
+    ...
+     nvmxip        0  [ + ]   nvmxip_qspi           |-- nvmxip-qspi1@08000000
+     blk           3  [ + ]   nvmxip-blk                    |   `-- nvmxip-qspi1@08000000.blk#1
+     nvmxip        1  [ + ]   nvmxip_qspi           |-- nvmxip-qspi2@08200000
+     blk           4  [ + ]   nvmxip-blk                    |   `-- nvmxip-qspi2@08200000.blk#2
+
+The implementation is generic and can be used by different platforms.
 
 Supported hardware
 --------------------------------
@@ -43,6 +69,23 @@ config NVMXIP
 	  handles the read operation. This driver is HW agnostic and can support
 	  multiple flash devices at the same time.
 
+config NVMXIP_QSPI
+	  This option allows the emulation of a block storage device on top of a QSPI XIP flash.
+	  Any platform that needs to emulate one or multiple QSPI XIP flash devices can turn this
+	  option on to enable the functionality. NVMXIP config is selected automatically.
+	  Platforms that need to add custom treatments before accessing to the flash, can
+	  write their own driver (same as nvmxip_qspi in addition to the custom settings).
+
+Device Tree nodes
+--------------------
+
+Multiple QSPI XIP flash devices can be used at the same time by describing them through DT
+nodes.
+
+Please refer to the documentation of the DT binding at:
+
+doc/device-tree-bindings/nvmxip/nvmxip_qspi.txt
+
 Contributors
 ------------
    * Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
diff --git a/doc/device-tree-bindings/nvmxip/nvmxip_qspi.txt b/doc/device-tree-bindings/nvmxip/nvmxip_qspi.txt
new file mode 100644
index 0000000000..cc60e9efdc
--- /dev/null
+++ b/doc/device-tree-bindings/nvmxip/nvmxip_qspi.txt
@@ -0,0 +1,56 @@
+Specifying NVMXIP information for devices
+======================================
+
+QSPI XIP flash device nodes
+---------------------------
+
+Each flash device should have its own node.
+
+Each node must specify the following fields:
+
+1)
+		compatible = "nvmxip,qspi";
+
+This allows to bind the flash device with the nvmxip_qspi driver
+If a platform has its own driver, please provide your own compatible
+string.
+
+2)
+		reg = <0x0 0x08000000 0x0 0x00200000>;
+
+The start address and size of the flash device. The values give here are an
+example (when the cell size is 2).
+
+When cell size is 1, the reg field looks like this:
+
+		reg = <0x08000000 0x00200000>;
+
+3)
+
+		lba_shift = <9>;
+
+The number of bit shifts used to calculate the size in bytes of one block.
+In this example the block size is 1 << 9 = 2 ^ 9 = 512 bytes
+
+4)
+
+		lba = <4096>;
+
+The number of blocks.
+
+Example of multiple flash devices
+----------------------------------------------------
+
+	nvmxip-qspi1@08000000 {
+		compatible = "nvmxip,qspi";
+		reg = <0x0 0x08000000 0x0 0x00200000>;
+		lba_shift = <9>;
+		lba = <4096>;
+	};
+
+	nvmxip-qspi2@08200000 {
+		compatible = "nvmxip,qspi";
+		reg = <0x0 0x08200000 0x0 0x00100000>;
+		lba_shift = <9>;
+		lba = <2048>;
+	};
diff --git a/drivers/mtd/nvmxip/Kconfig b/drivers/mtd/nvmxip/Kconfig
index ef53fc3c79..3ef7105026 100644
--- a/drivers/mtd/nvmxip/Kconfig
+++ b/drivers/mtd/nvmxip/Kconfig
@@ -11,3 +11,9 @@ config NVMXIP
 	  This option allows the emulation of a block storage device
 	  on top of a direct access non volatile memory XIP flash devices.
 	  This support provides the read operation.
+
+config NVMXIP_QSPI
+	bool "QSPI XIP  support"
+	select NVMXIP
+	help
+	  This option allows the emulation of a block storage device on top of a QSPI XIP flash
diff --git a/drivers/mtd/nvmxip/Makefile b/drivers/mtd/nvmxip/Makefile
index 07890982c7..54eacc102e 100644
--- a/drivers/mtd/nvmxip/Makefile
+++ b/drivers/mtd/nvmxip/Makefile
@@ -5,3 +5,4 @@
 #   Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
 
 obj-y += nvmxip-uclass.o nvmxip.o
+obj-$(CONFIG_NVMXIP_QSPI) += nvmxip_qspi.o
diff --git a/drivers/mtd/nvmxip/nvmxip_qspi.c b/drivers/mtd/nvmxip/nvmxip_qspi.c
new file mode 100644
index 0000000000..7221fd1cb4
--- /dev/null
+++ b/drivers/mtd/nvmxip/nvmxip_qspi.c
@@ -0,0 +1,70 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2023 Arm Limited and/or its affiliates <open-source-office@arm.com>
+ *
+ * Authors:
+ *   Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <fdt_support.h>
+#include <linux/errno.h>
+#include "nvmxip.h"
+
+#include <asm/global_data.h>
+DECLARE_GLOBAL_DATA_PTR;
+
+#define NVMXIP_QSPI_DRV_NAME "nvmxip_qspi"
+
+/**
+ * nvmxip_qspi_of_to_plat() -read from DT
+ * @dev:	the NVMXIP device
+ *
+ * Read from the DT the NVMXIP information.
+ *
+ * Return:
+ *
+ * 0 on success. Otherwise, failure
+ */
+static int nvmxip_qspi_of_to_plat(struct udevice *dev)
+{
+	struct nvmxip_plat *plat = dev_get_plat(dev);
+	int ret;
+
+	plat->phys_base = (phys_addr_t)dev_read_addr(dev);
+	if (plat->phys_base == FDT_ADDR_T_NONE) {
+		log_err("[%s]: can not get base address from device tree\n", dev->name);
+		return -EINVAL;
+	}
+
+	ret = dev_read_u32(dev, "lba_shift", &plat->lba_shift);
+	if (ret) {
+		log_err("[%s]: can not get lba_shift from device tree\n", dev->name);
+		return -EINVAL;
+	}
+
+	ret = dev_read_u32(dev, "lba", (u32 *)&plat->lba);
+	if (ret) {
+		log_err("[%s]: can not get lba from device tree\n", dev->name);
+		return -EINVAL;
+	}
+
+	log_debug("[%s]: XIP device base addr: 0x%llx , lba_shift: %d , lbas: %lu\n",
+		  dev->name, plat->phys_base, plat->lba_shift, plat->lba);
+
+	return 0;
+}
+
+static const struct udevice_id nvmxip_qspi_ids[] = {
+	{ .compatible = "nvmxip,qspi" },
+	{ /* sentinel */ }
+};
+
+U_BOOT_DRIVER(nvmxip_qspi) = {
+	.name = NVMXIP_QSPI_DRV_NAME,
+	.id = UCLASS_NVMXIP,
+	.of_match = nvmxip_qspi_ids,
+	.of_to_plat = nvmxip_qspi_of_to_plat,
+	.plat_auto = sizeof(struct nvmxip_plat),
+};
-- 
2.25.1

[PATCH v2 3/7] sandbox64: fix: return unsigned long in readq()

[Abdellatif El Khlifi]

make readq return unsigned long

readq should return 64-bit data

Signed-off-by: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
---
 arch/sandbox/cpu/cpu.c        | 2 +-
 arch/sandbox/include/asm/io.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/sandbox/cpu/cpu.c b/arch/sandbox/cpu/cpu.c
index 636d3545b9..248d17a85c 100644
--- a/arch/sandbox/cpu/cpu.c
+++ b/arch/sandbox/cpu/cpu.c
@@ -230,7 +230,7 @@ phys_addr_t map_to_sysmem(const void *ptr)
 	return mentry->tag;
 }
 
-unsigned int sandbox_read(const void *addr, enum sandboxio_size_t size)
+unsigned long sandbox_read(const void *addr, enum sandboxio_size_t size)
 {
 	struct sandbox_state *state = state_get_current();
 
diff --git a/arch/sandbox/include/asm/io.h b/arch/sandbox/include/asm/io.h
index ad6c29a4e2..31ab7289b4 100644
--- a/arch/sandbox/include/asm/io.h
+++ b/arch/sandbox/include/asm/io.h
@@ -45,7 +45,7 @@ static inline void unmap_sysmem(const void *vaddr)
 /* Map from a pointer to our RAM buffer */
 phys_addr_t map_to_sysmem(const void *ptr);
 
-unsigned int sandbox_read(const void *addr, enum sandboxio_size_t size);
+unsigned long sandbox_read(const void *addr, enum sandboxio_size_t size);
 void sandbox_write(void *addr, unsigned int val, enum sandboxio_size_t size);
 
 #define readb(addr) sandbox_read((const void *)addr, SB_SIZE_8)
-- 
2.25.1

[PATCH v2 4/7] sandbox64: add support for NVMXIP QSPI

[Abdellatif El Khlifi]

enable NVMXIP QSPI for sandbox 64-bit

Adding two NVM XIP QSPI storage devices.

Signed-off-by: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
Reviewed-by: Simon Glass <sjg@chromium.org>

---

Changelog:
===============

v2:

* address nits

 arch/sandbox/dts/sandbox64.dts                  | 13 +++++++++++++
 arch/sandbox/dts/test.dts                       | 14 ++++++++++++++
 configs/sandbox64_defconfig                     |  1 +
 doc/develop/driver-model/nvmxip.rst             |  2 +-
 doc/device-tree-bindings/nvmxip/nvmxip_qspi.txt |  6 +++---
 drivers/mtd/nvmxip/nvmxip-uclass.c              |  7 +++++++
 6 files changed, 39 insertions(+), 4 deletions(-)

diff --git a/arch/sandbox/dts/sandbox64.dts b/arch/sandbox/dts/sandbox64.dts
index f21fc181f3..195365580a 100644
--- a/arch/sandbox/dts/sandbox64.dts
+++ b/arch/sandbox/dts/sandbox64.dts
@@ -89,6 +89,19 @@
 		cs-gpios = <0>, <&gpio_a 0>;
 	};
 
+	nvmxip-qspi1@08000000 {
+		compatible = "nvmxip,qspi";
+		reg = /bits/ 64 <0x08000000 0x00200000>;
+		lba_shift = <9>;
+		lba = <4096>;
+	};
+
+	nvmxip-qspi2@08200000 {
+		compatible = "nvmxip,qspi";
+		reg = /bits/ 64 <0x08200000 0x00100000>;
+		lba_shift = <9>;
+		lba = <2048>;
+	};
 };
 
 #include "sandbox.dtsi"
diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index d72d7a567a..da08012834 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -1802,6 +1802,20 @@
 		compatible = "u-boot,fwu-mdata-gpt";
 		fwu-mdata-store = <&mmc0>;
 	};
+
+	nvmxip-qspi1@08000000 {
+		compatible = "nvmxip,qspi";
+		reg = <0x08000000 0x00200000>;
+		lba_shift = <9>;
+		lba = <4096>;
+	};
+
+	nvmxip-qspi2@08200000 {
+		compatible = "nvmxip,qspi";
+		reg = <0x08200000 0x00100000>;
+		lba_shift = <9>;
+		lba = <2048>;
+	};
 };
 
 #include "sandbox_pmic.dtsi"
diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig
index af2c56ad4c..bb877b6a58 100644
--- a/configs/sandbox64_defconfig
+++ b/configs/sandbox64_defconfig
@@ -260,3 +260,4 @@ CONFIG_FWU_MULTI_BANK_UPDATE=y
 CONFIG_UNIT_TEST=y
 CONFIG_UT_TIME=y
 CONFIG_UT_DM=y
+CONFIG_NVMXIP_QSPI=y
diff --git a/doc/develop/driver-model/nvmxip.rst b/doc/develop/driver-model/nvmxip.rst
index 09afdbcccf..e85dc220b9 100644
--- a/doc/develop/driver-model/nvmxip.rst
+++ b/doc/develop/driver-model/nvmxip.rst
@@ -56,7 +56,7 @@ The implementation is generic and can be used by different platforms.
 Supported hardware
 --------------------------------
 
-Any 64-bit plaform.
+Any plaform supporting readq().
 
 Configuration
 ----------------------
diff --git a/doc/device-tree-bindings/nvmxip/nvmxip_qspi.txt b/doc/device-tree-bindings/nvmxip/nvmxip_qspi.txt
index cc60e9efdc..882728d541 100644
--- a/doc/device-tree-bindings/nvmxip/nvmxip_qspi.txt
+++ b/doc/device-tree-bindings/nvmxip/nvmxip_qspi.txt
@@ -16,7 +16,7 @@ If a platform has its own driver, please provide your own compatible
 string.
 
 2)
-		reg = <0x0 0x08000000 0x0 0x00200000>;
+		reg = /bits/ 64 <0x08000000 0x00200000>;
 
 The start address and size of the flash device. The values give here are an
 example (when the cell size is 2).
@@ -43,14 +43,14 @@ Example of multiple flash devices
 
 	nvmxip-qspi1@08000000 {
 		compatible = "nvmxip,qspi";
-		reg = <0x0 0x08000000 0x0 0x00200000>;
+		reg = /bits/ 64 <0x08000000 0x00200000>;
 		lba_shift = <9>;
 		lba = <4096>;
 	};
 
 	nvmxip-qspi2@08200000 {
 		compatible = "nvmxip,qspi";
-		reg = <0x0 0x08200000 0x0 0x00100000>;
+		reg = /bits/ 64 <0x08200000 0x00100000>;
 		lba_shift = <9>;
 		lba = <2048>;
 	};
diff --git a/drivers/mtd/nvmxip/nvmxip-uclass.c b/drivers/mtd/nvmxip/nvmxip-uclass.c
index 9f96041e3d..6d8eb177b5 100644
--- a/drivers/mtd/nvmxip/nvmxip-uclass.c
+++ b/drivers/mtd/nvmxip/nvmxip-uclass.c
@@ -9,6 +9,9 @@
 #include <common.h>
 #include <dm.h>
 #include <log.h>
+#if CONFIG_IS_ENABLED(SANDBOX64)
+#include <asm/test.h>
+#endif
 #include <linux/bitops.h>
 #include "nvmxip.h"
 
@@ -36,6 +39,10 @@ static int nvmxip_post_bind(struct udevice *udev)
 	char bdev_name[NVMXIP_BLKDEV_NAME_SZ + 1];
 	int devnum;
 
+#if CONFIG_IS_ENABLED(SANDBOX64)
+	sandbox_set_enable_memio(true);
+#endif
+
 	devnum = uclass_id_count(UCLASS_NVMXIP);
 	snprintf(bdev_name, NVMXIP_BLKDEV_NAME_SZ, "blk#%d", devnum);
 
-- 
2.25.1

[PATCH v2 5/7] corstone1000: add NVM XIP QSPI device tree node

[Abdellatif El Khlifi]

add QSPI flash device node for block storage access

Signed-off-by: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
---
 arch/arm/dts/corstone1000.dtsi | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/arm/dts/corstone1000.dtsi b/arch/arm/dts/corstone1000.dtsi
index 4e46826f88..533dfdf8e1 100644
--- a/arch/arm/dts/corstone1000.dtsi
+++ b/arch/arm/dts/corstone1000.dtsi
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0 or MIT
 /*
- * Copyright (c) 2022, Arm Limited. All rights reserved.
+ * Copyright 2022-2023 Arm Limited and/or its affiliates <open-source-office@arm.com>
  * Copyright (c) 2022, Linaro Limited. All rights reserved.
  *
  */
@@ -38,6 +38,13 @@
 		reg = <0x88200000 0x77e00000>;
 	};
 
+	nvmxip-qspi@08000000 {
+		compatible = "nvmxip,qspi";
+		reg = <0x08000000 0x2000000>;
+		lba_shift = <9>;
+		lba = <65536>;
+	};
+
 	gic: interrupt-controller@1c000000 {
 		compatible = "arm,gic-400";
 		#interrupt-cells = <3>;
-- 
2.25.1

[PATCH v2 6/7] corstone1000: enable NVM XIP QSPI flash

[Abdellatif El Khlifi]

add the QSPI flash device with block storage capability

Signed-off-by: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
---
 configs/corstone1000_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configs/corstone1000_defconfig b/configs/corstone1000_defconfig
index 383317fefe..35f763596a 100644
--- a/configs/corstone1000_defconfig
+++ b/configs/corstone1000_defconfig
@@ -52,3 +52,4 @@ CONFIG_DM_SERIAL=y
 CONFIG_USB=y
 CONFIG_USB_ISP1760=y
 CONFIG_ERRNO_STR=y
+CONFIG_NVMXIP_QSPI=y
-- 
2.25.1

[PATCH v2 7/7] sandbox64: add a test case for UCLASS_NVMXIP

[Abdellatif El Khlifi]

provide a test for NVM XIP devices

The test case allows to make sure of the following:

- The NVM XIP QSPI devices are probed
- The DT entries are read correctly
- the data read from the flash by the NVMXIP block driver is correct

Signed-off-by: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>

---

Changelog:
===============

v2:

* address nits

 MAINTAINERS      |   1 +
 test/dm/Makefile |   5 ++
 test/dm/nvmxip.c | 145 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 151 insertions(+)
 create mode 100644 test/dm/nvmxip.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 77acce79c5..25beb8c48b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1208,6 +1208,7 @@ S:	Maintained
 F:	doc/develop/driver-model/nvmxip.rst
 F:	doc/device-tree-bindings/nvmxip/nvmxip_qspi.txt
 F:	drivers/mtd/nvmxip/
+F:	test/dm/nvmxip.c
 
 NVMEM
 M:	Sean Anderson <seanga2@gmail.com>
diff --git a/test/dm/Makefile b/test/dm/Makefile
index 7a79b6e1a2..7f9d0b3c38 100644
--- a/test/dm/Makefile
+++ b/test/dm/Makefile
@@ -1,6 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0+
 #
 # Copyright (c) 2013 Google, Inc
+# Copyright 2023 Arm Limited and/or its affiliates <open-source-office@arm.com>
 
 obj-$(CONFIG_UT_DM) += test-dm.o
 
@@ -17,6 +18,10 @@ obj-$(CONFIG_UT_DM) += test-uclass.o
 obj-$(CONFIG_UT_DM) += core.o
 obj-$(CONFIG_UT_DM) += read.o
 obj-$(CONFIG_UT_DM) += phys2bus.o
+ifeq ($(CONFIG_NVMXIP_QSPI)$(CONFIG_SANDBOX64),yy)
+obj-y += nvmxip.o
+endif
+
 ifneq ($(CONFIG_SANDBOX),)
 ifeq ($(CONFIG_ACPIGEN),y)
 obj-y += acpi.o
diff --git a/test/dm/nvmxip.c b/test/dm/nvmxip.c
new file mode 100644
index 0000000000..e934748eb5
--- /dev/null
+++ b/test/dm/nvmxip.c
@@ -0,0 +1,145 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Functional tests for UCLASS_FFA  class
+ *
+ * Copyright 2023 Arm Limited and/or its affiliates <open-source-office@arm.com>
+ *
+ * Authors:
+ *   Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
+ */
+
+#include <common.h>
+#include <blk.h>
+#include <console.h>
+#include <dm.h>
+#include <mapmem.h>
+#include <dm/test.h>
+#include <linux/bitops.h>
+#include <test/test.h>
+#include <test/ut.h>
+#include "../../drivers/mtd/nvmxip/nvmxip.h"
+
+/* NVMXIP devices described in the device tree */
+#define SANDBOX_NVMXIP_DEVICES 2
+
+/* reference device tree data for the probed devices */
+static struct nvmxip_plat nvmqspi_refdata[SANDBOX_NVMXIP_DEVICES] = {
+	{0x08000000, 9, 4096}, {0x08200000, 9, 2048}
+};
+
+#define NVMXIP_BLK_START_PATTERN 0x1122334455667788ULL
+#define NVMXIP_BLK_END_PATTERN 0xa1a2a3a4a5a6a7a8ULL
+
+/**
+ * dm_nvmxip_flash_sanity() - check flash data
+ * @uts: test state
+ * @device_idx:	the NVMXIP device index
+ * @buffer:	the user buffer where the blocks data is copied to
+ *
+ * Mode 1: When buffer is NULL, initialize the flash with pattern data at the start
+ * and at the end of each block. This pattern data will be used to check data consistency
+ * when verifying the data read.
+ * Mode 2: When the user buffer is provided in the argument (not NULL), compare the data
+ * of the start and the end of each block in the user buffer with the expected pattern data.
+ * Return an error when the check fails.
+ *
+ * Return:
+ *
+ * 0 on success. Otherwise, failure
+ */
+static int dm_nvmxip_flash_sanity(struct unit_test_state *uts, u8 device_idx, void *buffer)
+{
+	int i;
+	u64 *ptr;
+	u8 *base;
+	unsigned long blksz;
+
+	blksz = BIT(nvmqspi_refdata[device_idx].lba_shift);
+
+	if (!buffer) {
+		/* Mode 1: point at the flash start address. Pattern data will be written */
+		base = map_sysmem(nvmqspi_refdata[device_idx].phys_base, 0);
+	} else {
+		/* Mode 2: point at the user buffer containing the data read and to be verified */
+		base = buffer;
+	}
+
+	for (i = 0; i < nvmqspi_refdata[device_idx].lba ; i++) {
+		ptr = (u64 *)(base + i * blksz);
+
+		/* write an 8 bytes pattern at the start of the current block */
+		if (!buffer)
+			*ptr = NVMXIP_BLK_START_PATTERN;
+		else
+			ut_asserteq_64(NVMXIP_BLK_START_PATTERN, *ptr);
+
+		ptr = (u64 *)((u8 *)ptr + blksz - sizeof(u64));
+
+		/* write an 8 bytes pattern at the end of the current block */
+		if (!buffer)
+			*ptr = NVMXIP_BLK_END_PATTERN;
+		else
+			ut_asserteq_64(NVMXIP_BLK_END_PATTERN, *ptr);
+	}
+
+	if (!buffer)
+		unmap_sysmem(base);
+
+	return 0;
+}
+
+/**
+ * dm_test_nvmxip() - check flash data
+ * @uts: test state
+ * Return:
+ *
+ * CMD_RET_SUCCESS on success. Otherwise, failure
+ */
+static int dm_test_nvmxip(struct unit_test_state *uts)
+{
+	struct nvmxip_plat *plat_data = NULL;
+	struct udevice *dev = NULL, *bdev = NULL;
+	u8 device_idx;
+	void *buffer = NULL;
+	unsigned long flashsz;
+
+	/* set the flash content first for both devices */
+	dm_nvmxip_flash_sanity(uts, 0, NULL);
+	dm_nvmxip_flash_sanity(uts, 1, NULL);
+
+	/* probing all NVM XIP QSPI devices */
+	for (device_idx = 0, uclass_first_device(UCLASS_NVMXIP, &dev);
+	     dev;
+	     uclass_next_device(&dev), device_idx++) {
+		plat_data = dev_get_plat(dev);
+
+		/* device tree entries checks */
+		ut_assertok(nvmqspi_refdata[device_idx].phys_base != plat_data->phys_base);
+		ut_assertok(nvmqspi_refdata[device_idx].lba_shift != plat_data->lba_shift);
+		ut_assertok(nvmqspi_refdata[device_idx].lba != plat_data->lba);
+
+		/* before reading all the flash blocks, let's calculate the flash size */
+		flashsz = plat_data->lba << plat_data->lba_shift;
+
+		/* allocate the user buffer where to copy the blocks data to */
+		buffer = calloc(flashsz, 1);
+		ut_assertok(!buffer);
+
+		/* the block device is the child of the parent device probed with DT */
+		ut_assertok(device_find_first_child(dev, &bdev));
+
+		/* reading all the flash blocks */
+		ut_asserteq(plat_data->lba, blk_read(bdev, 0, plat_data->lba, buffer));
+
+		/* compare the data read from flash with the expected data */
+		dm_nvmxip_flash_sanity(uts, device_idx, buffer);
+
+		free(buffer);
+	}
+
+	ut_assertok(device_idx != SANDBOX_NVMXIP_DEVICES);
+
+	return CMD_RET_SUCCESS;
+}
+
+DM_TEST(dm_test_nvmxip, UT_TESTF_SCAN_FDT | UT_TESTF_CONSOLE_REC);
-- 
2.25.1

Re: [PATCH v1 1/6] drivers/nvmxip: introduce NVM XIP block storage emulation

[Abdellatif El Khlifi]

On Tue, Feb 07, 2023 at 11:38:57AM -0700, Simon Glass wrote:
> Hi,
> 
> On Mon, 16 Jan 2023 at 10:28, <abdellatif.elkhlifi@arm.com> wrote:
> >
> > From: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
> >
> > add block storage emulation for NVM XIP flash devices
> >
> > Some paltforms such as Corstone-1000 need to see NVM XIP raw flash
> > as a block storage device with read only capability.
> 
> As mentioned I didn't see this patch originally. It generally looks OK
> but have made some comments below.
> 
> >
> > Here NVM flash devices are devices with addressable
> > memory (e.g: QSPI NOR flash).
> >
> > The implementation is generic and can be used by different platforms.
> >
> > Two drivers are provided as follows.
> >
> >   nvmxip-blk :
> >
> >     a generic block driver allowing to read from the XIP flash
> >
> >   nvmxip_qspi :
> >
> >     The driver probed with the DT and parent of the nvmxip-blk device.
> >     nvmxip_qspi can be reused by other platforms. If the platform
> >     has custom settings to apply before using the flash, then the platform
> >     can provide its own parent driver belonging to UCLASS_NVMXIP and reuse
> >     nvmxip-blk. The custom driver can be implmented like nvmxip_qspi in
> >     addition to the platform custom settings.
> >
> > Platforms can use multiple NVM XIP devices at the same time by defining a
> > DT node for each one of them.
> >
> > For more details please refer to doc/develop/driver-model/nvmxip.rst
> >
> > Signed-off-by: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
> > ---
> >  MAINTAINERS                                |   7 ++
> >  doc/develop/driver-model/index.rst         |   1 +
> >  doc/develop/driver-model/nvmxip.rst        |  70 +++++++++++
> >  doc/device-tree-bindings/nvmxip/nvmxip.txt |  56 +++++++++
> >  drivers/Kconfig                            |   2 +
> >  drivers/Makefile                           |   1 +
> >  drivers/block/blk-uclass.c                 |   1 +
> >  drivers/nvmxip/Kconfig                     |  19 +++
> >  drivers/nvmxip/Makefile                    |   8 ++
> >  drivers/nvmxip/nvmxip-uclass.c             |  15 +++
> >  drivers/nvmxip/nvmxip.c                    | 129 +++++++++++++++++++++
> >  drivers/nvmxip/nvmxip.h                    |  48 ++++++++
> >  drivers/nvmxip/nvmxip_qspi.c               |  67 +++++++++++
> >  include/dm/uclass-id.h                     |   1 +
> >  14 files changed, 425 insertions(+)
> >  create mode 100644 doc/develop/driver-model/nvmxip.rst
> >  create mode 100644 doc/device-tree-bindings/nvmxip/nvmxip.txt
> >  create mode 100644 drivers/nvmxip/Kconfig
> >  create mode 100644 drivers/nvmxip/Makefile
> >  create mode 100644 drivers/nvmxip/nvmxip-uclass.c
> >  create mode 100644 drivers/nvmxip/nvmxip.c
> >  create mode 100644 drivers/nvmxip/nvmxip.h
> >  create mode 100644 drivers/nvmxip/nvmxip_qspi.c
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index b2de50ccfc..539d01f68c 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -1193,6 +1193,13 @@ F:       cmd/nvme.c
> >  F:     include/nvme.h
> >  F:     doc/develop/driver-model/nvme.rst
> >
> > +NVMXIP
> > +M:     Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
> > +S:     Maintained
> > +F:     doc/develop/driver-model/nvmxip.rst
> > +F:     doc/device-tree-bindings/nvmxip/nvmxip.txt
> > +F:     drivers/nvmxip/
> > +
> >  NVMEM
> >  M:     Sean Anderson <seanga2@gmail.com>
> >  S:     Maintained
> > diff --git a/doc/develop/driver-model/index.rst b/doc/develop/driver-model/index.rst
> > index 7366ef818c..8e12bbd936 100644
> > --- a/doc/develop/driver-model/index.rst
> > +++ b/doc/develop/driver-model/index.rst
> > @@ -20,6 +20,7 @@ subsystems
> >     livetree
> >     migration
> >     nvme
> > +   nvmxip
> >     of-plat
> >     pci-info
> >     pmic-framework
> > diff --git a/doc/develop/driver-model/nvmxip.rst b/doc/develop/driver-model/nvmxip.rst
> > new file mode 100644
> > index 0000000000..91b24e4e50
> > --- /dev/null
> > +++ b/doc/develop/driver-model/nvmxip.rst
> > @@ -0,0 +1,70 @@
> > +.. SPDX-License-Identifier: GPL-2.0+
> > +
> > +NVM XIP Block Storage Emulation Driver
> > +=======================================
> > +
> > +Summary
> > +-------
> > +
> > +Non-Volatile Memory devices with addressable memory (e.g: QSPI NOR flash) could
> > +be used for block storage needs (e.g: parsing a GPT layout in a raw QSPI NOR flash).
> > +
> > +The NVMXIP class provides this functionality and can be used for any 64-bit platform.
> > +
> > +The NVMXIP class provides the following drivers:
> > +
> > +      nvmxip-blk :
> > +
> > +        A generic block driver allowing to read from the XIP flash.
> > +       The driver belongs to UCLASS_BLK.
> > +       The driver implemented by drivers/nvmxip/nvmxip.c
> > +
> > +      nvmxip_qspi :
> > +
> > +        The driver probed with the DT and parent of the nvmxip-blk device.
> > +        nvmxip_qspi can be reused by other platforms. If the platform
> > +        has custom settings to apply before using the flash, then the platform
> > +        can provide its own parent driver belonging to UCLASS_NVMXIP and reuse
> > +        nvmxip-blk. The custom driver can be implmented like nvmxip_qspi in
> > +        addition to the platform custom settings.
> > +       The nvmxip_qspi driver belongs to UCLASS_NVMXIP.
> > +       The driver implemented by drivers/nvmxip/nvmxip_qspi.c
> > +
> > +    The implementation is generic and can be used by different platforms.
> > +
> > +Supported hardware
> > +--------------------------------
> > +
> > +Any 64-bit plaform.
> 
> platform
> 
> Why not 64-bit ? Please mention that.
> 
> > +
> > +Configuration
> > +----------------------
> > +
> > +config NVMXIP
> > +         This option allows the emulation of a block storage device
> > +         on top of a direct access non volatile memory XIP flash devices.
> > +         This support provides the read operation.
> > +         This option provides the block storage driver nvmxip-blk which
> > +         handles the read operation. This driver is HW agnostic and can support
> > +         multiple flash devices at the same time.
> > +
> > +config NVMXIP_QSPI
> > +         This option allows the emulation of a block storage device on top of a QSPI XIP flash.
> > +         Any platform that needs to emulate one or multiple XIP flash devices can turn this
> > +         option on to enable the functionality. NVMXIP config is selected automatically.
> > +         Platforms that need to add custom treatments before accessing to the flash, can
> > +         write their own driver (same as nvmxip_qspi in addition to the custom settings).
> > +
> > +Device Tree nodes
> > +--------------------
> > +
> > +Multiple XIP flash devices can be used at the same time by describing them through DT
> > +nodes.
> > +
> > +Please refer to the documentation of the DT binding at:
> > +
> > +doc/device-tree-bindings/nvmxip/nvmxip.txt
> > +
> > +Contributors
> > +------------
> > +   * Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
> > diff --git a/doc/device-tree-bindings/nvmxip/nvmxip.txt b/doc/device-tree-bindings/nvmxip/nvmxip.txt
> > new file mode 100644
> > index 0000000000..7c4b03f66b
> > --- /dev/null
> > +++ b/doc/device-tree-bindings/nvmxip/nvmxip.txt
> > @@ -0,0 +1,56 @@
> > +Specifying NVMXIP information for devices
> > +======================================
> > +
> > +NVM XIP flash device nodes
> > +---------------------------
> > +
> > +Each flash device should have its own node.
> > +
> > +Each node must specify the following fields:
> > +
> > +1)
> > +               compatible = "nvmxip,qspi";
> > +
> > +This allows to bind the flash device with the nvmxip_qspi driver
> > +If a platform has its own driver, please provide your own compatible
> > +string.
> > +
> > +2)
> > +               reg = <0x0 0x08000000 0x0 0x00200000>;
> 
> Do we not use regs = /bits/ 64 <...? ?
> 
> > +
> > +The start address and size of the flash device. The values give here are an
> > +example (when the cell size is 2).
> > +
> > +When cell size is 1, the reg field looks like this:
> > +
> > +               reg = <0x08000000 0x00200000>;
> > +
> > +3)
> > +
> > +               lba_shift = <9>;
> > +
> > +The number of bit shifts used to calculate the size in bytes of one block.
> > +In this example the block size is 1 << 9 = 2 ^ 9 = 512 bytes
> > +
> > +4)
> > +
> > +               lba = <4096>;
> > +
> > +The number of blocks.
> > +
> > +Example of multiple flash devices
> > +----------------------------------------------------
> > +
> > +       nvmxip-qspi1@08000000 {
> > +               compatible = "nvmxip,qspi";
> > +               reg = <0x0 0x08000000 0x0 0x00200000>;
> > +               lba_shift = <9>;
> > +               lba = <4096>;
> > +       };
> > +
> > +       nvmxip-qspi2@08200000 {
> > +               compatible = "nvmxip,qspi";
> > +               reg = <0x0 0x08200000 0x0 0x00100000>;
> > +               lba_shift = <9>;
> > +               lba = <2048>;
> > +       };
> > diff --git a/drivers/Kconfig b/drivers/Kconfig
> > index 9101e538b0..ef321bee94 100644
> > --- a/drivers/Kconfig
> > +++ b/drivers/Kconfig
> > @@ -76,6 +76,8 @@ source "drivers/net/Kconfig"
> >
> >  source "drivers/nvme/Kconfig"
> >
> > +source "drivers/nvmxip/Kconfig"
> > +
> >  source "drivers/pci/Kconfig"
> >
> >  source "drivers/pci_endpoint/Kconfig"
> > diff --git a/drivers/Makefile b/drivers/Makefile
> > index 83b14ef1fd..5e7fd6dab5 100644
> > --- a/drivers/Makefile
> > +++ b/drivers/Makefile
> > @@ -89,6 +89,7 @@ obj-$(CONFIG_FWU_MDATA) += fwu-mdata/
> >  obj-y += misc/
> >  obj-$(CONFIG_MMC) += mmc/
> >  obj-$(CONFIG_NVME) += nvme/
> > +obj-$(CONFIG_NVMXIP) += nvmxip/
> 
> Do you think this justifies its own directory? Do we expect more
> drivers? Another option would be drivers/mtd
> 
> Is there an equivalent Linux driver?

Yes we can have more drivers using the NVMXIP Uclass
(devices that need to run board specific treatment before accessing the NVM XIP flash).
Anyway, I moved the NVMXIP folder under mtd.
Regarding Linux, I'm not aware of any driver doing the same thing.

> 
> >  obj-$(CONFIG_PCI_ENDPOINT) += pci_endpoint/
> >  obj-y += dfu/
> >  obj-$(CONFIG_PCH) += pch/
> > diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c
> > index c69fc4d518..e8ab576c32 100644
> > --- a/drivers/block/blk-uclass.c
> > +++ b/drivers/block/blk-uclass.c
> > @@ -28,6 +28,7 @@ static struct {
> >         { UCLASS_AHCI, "sata" },
> >         { UCLASS_HOST, "host" },
> >         { UCLASS_NVME, "nvme" },
> > +       { UCLASS_NVMXIP, "nvmxip" },
> >         { UCLASS_EFI_MEDIA, "efi" },
> >         { UCLASS_EFI_LOADER, "efiloader" },
> >         { UCLASS_VIRTIO, "virtio" },
> > diff --git a/drivers/nvmxip/Kconfig b/drivers/nvmxip/Kconfig
> > new file mode 100644
> > index 0000000000..3ef7105026
> > --- /dev/null
> > +++ b/drivers/nvmxip/Kconfig
> > @@ -0,0 +1,19 @@
> > +# SPDX-License-Identifier: GPL-2.0+
> > +#
> > +# Copyright 2023 Arm Limited and/or its affiliates <open-source-office@arm.com>
> > +# Authors:
> > +#   Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
> > +
> > +config NVMXIP
> > +       bool "NVM XIP devices support"
> > +       select BLK
> > +       help
> > +         This option allows the emulation of a block storage device
> > +         on top of a direct access non volatile memory XIP flash devices.
> > +         This support provides the read operation.
> > +
> > +config NVMXIP_QSPI
> > +       bool "QSPI XIP  support"
> > +       select NVMXIP
> > +       help
> > +         This option allows the emulation of a block storage device on top of a QSPI XIP flash
> > diff --git a/drivers/nvmxip/Makefile b/drivers/nvmxip/Makefile
> > new file mode 100644
> > index 0000000000..54eacc102e
> > --- /dev/null
> > +++ b/drivers/nvmxip/Makefile
> > @@ -0,0 +1,8 @@
> > +# SPDX-License-Identifier: GPL-2.0+
> > +#
> > +# Copyright 2023 Arm Limited and/or its affiliates <open-source-office@arm.com>
> > +# Authors:
> > +#   Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
> > +
> > +obj-y += nvmxip-uclass.o nvmxip.o
> > +obj-$(CONFIG_NVMXIP_QSPI) += nvmxip_qspi.o
> > diff --git a/drivers/nvmxip/nvmxip-uclass.c b/drivers/nvmxip/nvmxip-uclass.c
> > new file mode 100644
> > index 0000000000..aed8e163d2
> > --- /dev/null
> > +++ b/drivers/nvmxip/nvmxip-uclass.c
> > @@ -0,0 +1,15 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright 2023 Arm Limited and/or its affiliates <open-source-office@arm.com>
> > + *
> > + * Authors:
> > + *   Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
> > + */
> > +
> > +#include <common.h>
> > +#include <dm.h>
> > +
> > +UCLASS_DRIVER(nvmxip) = {
> > +       .name   = "nvmxip",
> > +       .id     = UCLASS_NVMXIP,
> > +};
> > diff --git a/drivers/nvmxip/nvmxip.c b/drivers/nvmxip/nvmxip.c
> > new file mode 100644
> > index 0000000000..c276fb147e
> > --- /dev/null
> > +++ b/drivers/nvmxip/nvmxip.c
> > @@ -0,0 +1,129 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright 2023 Arm Limited and/or its affiliates <open-source-office@arm.com>
> > + *
> > + * Authors:
> > + *   Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
> > + */
> > +
> > +#include <common.h>
> > +#include <dm.h>
> > +#include <dm/device-internal.h>
> > +#include "nvmxip.h"
> > +
> > +static u32 nvmxip_bdev_max_devs;
> 
> Please put that in the uclass-private data. We don't want statics,
> etc. with driver model.
> 
> > +
> > +static int nvmxip_mmio_rawread(const phys_addr_t address, u64 *value)
> > +{
> > +       *value = readq(address);
> > +       return 0;
> > +}
> > +
> > +static ulong nvmxip_blk_read(struct udevice *udev, lbaint_t blknr, lbaint_t blkcnt, void *buffer)
> > +{
> > +       struct nvmxip_blk_priv *bpriv_data = dev_get_priv(udev);
> > +       struct blk_desc *desc = dev_get_uclass_plat(udev);
> > +
> 
> drop blank line, and can you combine the two following comments:
> 
> > +       /* size of 1 block */
> > +       /* number of the u64 words to read */
> > +       u32 qwords = (blkcnt * desc->blksz) / sizeof(u64);
> > +       /* physical address of the first block to read */
> > +       phys_addr_t blkaddr = bpriv_data->pplat_data->phys_base + blknr * desc->blksz;
> > +       u64 *virt_blkaddr;
> > +       u64 *pdst = buffer;
> > +       u32 qdata_idx;
> 
> Why is this u32? It should be uint. Try to use natural types for loops
> and function parameters unless there is a very good reason.
> 
> > +
> > +       if (!pdst)
> > +               return -EINVAL;
> > +
> > +       pr_debug("[%s]: reading from blknr: %lu , blkcnt: %lu\n", udev->name, blknr, blkcnt);
> > +
> > +       virt_blkaddr = map_sysmem(blkaddr, 0);
> > +
> > +       /* assumption: the data is virtually contiguous */
> > +
> [..]
> 
> > +
> > +int nvmxip_init(struct udevice *udev)
> 
> Why is this function exported? It should probably be called
> nvmxip_post_bind() and be called by the uclass when a new device is
> bound?
> 
> > +{
> > +       struct nvmxip_plat *plat_data = dev_get_plat(udev);
> 
> plat
> 
> > +       struct nvmxip_priv *priv_data = dev_get_priv(udev);
> 
> priv
> 
> (please use these names throughout as that is the convention; it makes
> it easier for people to understand your code)
> 
> > +       int ret;
> > +       struct udevice *bdev = NULL;
> > +       char bdev_name[NVMXIP_BLKDEV_NAME_SZ + 1] = {0};
> 
> You don't need to init it as you write to it below.
> 
> > +
> > +       priv_data->udev = udev;
> > +       priv_data->plat_data = plat_data;
> > +
> > +       nvmxip_bdev_max_devs++;
> > +
> > +       snprintf(bdev_name, NVMXIP_BLKDEV_NAME_SZ, "nvmxip-blk#%d", nvmxip_bdev_max_devs);
> > +
> > +       ret = blk_create_devicef(udev, NVMXIP_BLKDRV_NAME, bdev_name, UCLASS_NVMXIP,
> > +                                nvmxip_bdev_max_devs, NVMXIP_DEFAULT_LBA_SZ,
> > +                                NVMXIP_DEFAULT_LBA_COUNT, &bdev);
> > +       if (ret) {
> > +               pr_err("[%s]: failure during creation of the block device %s, error %d\n",
> > +                      udev->name, bdev_name, ret);
> > +               goto blkdev_setup_error;
> > +       }
> > +
> > +       ret = blk_probe_or_unbind(bdev);
> > +       if (ret) {
> > +               pr_err("[%s]: failure during probing the block device %s, error %d\n",
> > +                      udev->name, bdev_name, ret);
> > +               goto blkdev_setup_error;
> > +       }
> > +
> > +       pr_info("[%s]: the block device %s ready for use\n", udev->name, bdev_name);
> > +
> > +       return 0;
> > +
> > +blkdev_setup_error:
> > +       nvmxip_bdev_max_devs--;
> 
> I'm not sure if it helps, but there is a uclass_id_count() you can use.
> 
> > +       return ret;
> > +}
> > +
> > +static const struct blk_ops nvmxip_blk_ops = {
> > +       .read   = nvmxip_blk_read,
> > +};
> > +
> > +U_BOOT_DRIVER(nvmxip_blk) = {
> > +       .name   = NVMXIP_BLKDRV_NAME,
> > +       .id     = UCLASS_BLK,
> > +       .probe  = nvmxip_blk_probe,
> > +       .ops    = &nvmxip_blk_ops,
> > +       .priv_auto      = sizeof(struct nvmxip_blk_priv),
> > +};
> > diff --git a/drivers/nvmxip/nvmxip.h b/drivers/nvmxip/nvmxip.h
> > new file mode 100644
> > index 0000000000..47ae3bd8ab
> > --- /dev/null
> > +++ b/drivers/nvmxip/nvmxip.h
> > @@ -0,0 +1,48 @@
> > +/* SPDX-License-Identifier: GPL-2.0+ */
> > +/*
> > + * Copyright 2023 Arm Limited and/or its affiliates <open-source-office@arm.com>
> > + *
> > + * Authors:
> > + *   Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
> > + */
> > +
> > +#ifndef __DRIVER_NVMXIP_H__
> > +#define __DRIVER_NVMXIP_H__
> > +
> > +#include <asm/io.h>
> > +#include <blk.h>
> > +#include <linux/bitops.h>
> > +#include <linux/compat.h>
> > +#include <mapmem.h>
> 
> Please put blk and mapmem in the C file, at least. Check if you need
> the others here, too.

Done in patchset v2, except for blk which is needed by nvmxip.h (struct nvmxip_plat uses lbaint_t type )

Cheers,
Abdellatif

> 
> 
> > +
> > +#define NVMXIP_BLKDRV_NAME    "nvmxip-blk"
> > +
> > +#define NVMXIP_BLKDEV_NAME_SZ 20
> > +
> > +#define NVMXIP_DEFAULT_LBA_SHIFT 10 /* 1024 bytes per block */
> > +#define NVMXIP_DEFAULT_LBA_COUNT 1024 /* block count */
> > +
> > +#define NVMXIP_DEFAULT_LBA_SZ BIT(NVMXIP_DEFAULT_LBA_SHIFT)
> 
> This is a private header so you don't need the long names - e.g. drop
> the NVMXIP_
> > +
> > +/* NVM XIP device platform data */
> 
> I suggest using sphinx style right from the start, since this is a new file.
> 
> > +struct nvmxip_plat {
> > +       phys_addr_t phys_base; /* NVM XIP device base address */
> > +       u32 lba_shift; /* block size shift count (read from device tree) */
> > +       lbaint_t lba; /* number of blocks (read from device tree) */
> > +};
> > +
> > +/* NVM XIP device private data */
> > +struct nvmxip_priv {
> > +       struct udevice *udev;
> > +       struct nvmxip_plat *plat_data;
> 
> Please don't store private data from devices. You can use
> dev_get_plat() or whatever on a stored device. It just adds confusion.
> 
> > +};
> > +
> > +/* Private data of the block device associated with the NVM XIP device (the parent) */
> > +struct nvmxip_blk_priv {
> > +       struct udevice *bdev;
> 
> What is that?
> 
> > +       struct nvmxip_plat *pplat_data; /* parent device platform data */
> 
> same here
> 
> > +};
> > +
> > +int nvmxip_init(struct udevice *udev);
> 
> should not be exported
> 
> > +
> > +#endif /* __DRIVER_NVMXIP_H__ */
> > diff --git a/drivers/nvmxip/nvmxip_qspi.c b/drivers/nvmxip/nvmxip_qspi.c
> > new file mode 100644
> > index 0000000000..d0ce4c6633
> > --- /dev/null
> > +++ b/drivers/nvmxip/nvmxip_qspi.c
> 
> Can you put the driver in a separate patch? It is a good idea to have
> the uclass in a patch by itself.
> 
> > @@ -0,0 +1,67 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright 2023 Arm Limited and/or its affiliates <open-source-office@arm.com>
> > + *
> > + * Authors:
> > + *   Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
> > + */
> > +
> > +#include <common.h>
> > +#include <dm.h>
> > +#include <fdt_support.h>
> > +#include "nvmxip.h"
> > +
> > +#include <asm/global_data.h>
> > +DECLARE_GLOBAL_DATA_PTR;
> > +
> > +#define NVMXIP_QSPI_DRV_NAME "nvmxip_qspi"
> > +
> > +static int nvmxip_qspi_probe(struct udevice *dev)
> > +{
> > +       pr_debug("[%s][%s]\n", __func__, dev->name);
> > +       return nvmxip_init(dev);
> > +}
> > +
> > +static int nvmxip_qspi_of_to_plat(struct udevice *dev)
> > +{
> > +       struct nvmxip_plat *plat_data = dev_get_plat(dev);
> > +       int ret;
> > +
> > +       plat_data->phys_base = (phys_addr_t)dev_read_addr(dev);
> > +       if (plat_data->phys_base == FDT_ADDR_T_NONE) {
> > +               pr_err("[%s]: can not get base address from device tree\n", dev->name);
> 
> This is adding a lot to code size...do you want to use debug instead?
> 
> > +               return -EINVAL;
> > +       }
> > +
> > +       ret = dev_read_u32(dev, "lba_shift", &plat_data->lba_shift);
> > +       if (ret) {
> > +               pr_err("[%s]: can not get lba_shift from device tree\n", dev->name);
> > +               return -EINVAL;
> > +       }
> > +
> > +       ret = dev_read_u32(dev, "lba", (u32 *)&plat_data->lba);
> > +       if (ret) {
> > +               pr_err("[%s]: can not get lba from device tree\n", dev->name);
> > +               return -EINVAL;
> > +       }
> > +
> > +       pr_debug("[%s]: XIP device base addr: 0x%llx , lba_shift: %d , lbas: %lu\n",
> > +                dev->name, plat_data->phys_base, plat_data->lba_shift, plat_data->lba);
> > +
> > +       return 0;
> > +}
> > +
> > +static const struct udevice_id nvmxip_qspi_ids[] = {
> > +       { .compatible = "nvmxip,qspi" },
> > +       { /* sentinel */ }
> > +};
> > +
> > +U_BOOT_DRIVER(nvmxip_qspi) = {
> > +       .name = NVMXIP_QSPI_DRV_NAME,
> > +       .id = UCLASS_NVMXIP,
> > +       .of_match = nvmxip_qspi_ids,
> > +       .of_to_plat = nvmxip_qspi_of_to_plat,
> > +       .priv_auto = sizeof(struct nvmxip_priv),
> > +       .plat_auto = sizeof(struct nvmxip_plat),
> > +       .probe = nvmxip_qspi_probe,
> > +};
> > diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
> > index 376f741cc2..dcfe8cfe2c 100644
> > --- a/include/dm/uclass-id.h
> > +++ b/include/dm/uclass-id.h
> > @@ -88,6 +88,7 @@ enum uclass_id {
> >         UCLASS_NOP,             /* No-op devices */
> >         UCLASS_NORTHBRIDGE,     /* Intel Northbridge / SDRAM controller */
> >         UCLASS_NVME,            /* NVM Express device */
> > +       UCLASS_NVMXIP,          /* NVM XIP devices */
> >         UCLASS_P2SB,            /* (x86) Primary-to-Sideband Bus */
> >         UCLASS_PANEL,           /* Display panel, such as an LCD */
> >         UCLASS_PANEL_BACKLIGHT, /* Backlight controller for panel */
> > --
> > 2.17.1
> >
> 
> Regards,
> Simon

Re: [PATCH v1 6/6] sandbox64: add a test case for UCLASS_NVMXIP

[Abdellatif El Khlifi]

On Mon, Feb 06, 2023 at 09:02:40PM -0700, Simon Glass wrote:
> Hi,
> 
> On Mon, 16 Jan 2023 at 10:29, <abdellatif.elkhlifi@arm.com> wrote:
> >
> > From: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
> >
> > provide a test for NVM XIP devices
> >
> > The test case allows to make sure of the following:
> >
> > - The NVM XIP QSPI devices are probed
> > - The DT entries are read correctly
> > - the data read from the flash by the NVMXIP block driver is correct
> >
> > Signed-off-by: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
> > ---
> >  MAINTAINERS      |   1 +
> >  test/dm/Makefile |   5 ++
> >  test/dm/nvmxip.c | 117 +++++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 123 insertions(+)
> >  create mode 100644 test/dm/nvmxip.c
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 539d01f68c..e92898e462 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -1199,6 +1199,7 @@ S:        Maintained
> >  F:     doc/develop/driver-model/nvmxip.rst
> >  F:     doc/device-tree-bindings/nvmxip/nvmxip.txt
> >  F:     drivers/nvmxip/
> > +F:     test/dm/nvmxip.c
> >
> >  NVMEM
> >  M:     Sean Anderson <seanga2@gmail.com>
> > diff --git a/test/dm/Makefile b/test/dm/Makefile
> > index 7a79b6e1a2..7f9d0b3c38 100644
> > --- a/test/dm/Makefile
> > +++ b/test/dm/Makefile
> > @@ -1,6 +1,7 @@
> >  # SPDX-License-Identifier: GPL-2.0+
> >  #
> >  # Copyright (c) 2013 Google, Inc
> > +# Copyright 2023 Arm Limited and/or its affiliates <open-source-office@arm.com>
> >
> >  obj-$(CONFIG_UT_DM) += test-dm.o
> >
> > @@ -17,6 +18,10 @@ obj-$(CONFIG_UT_DM) += test-uclass.o
> >  obj-$(CONFIG_UT_DM) += core.o
> >  obj-$(CONFIG_UT_DM) += read.o
> >  obj-$(CONFIG_UT_DM) += phys2bus.o
> > +ifeq ($(CONFIG_NVMXIP_QSPI)$(CONFIG_SANDBOX64),yy)
> > +obj-y += nvmxip.o
> > +endif
> > +
> >  ifneq ($(CONFIG_SANDBOX),)
> >  ifeq ($(CONFIG_ACPIGEN),y)
> >  obj-y += acpi.o
> > diff --git a/test/dm/nvmxip.c b/test/dm/nvmxip.c
> > new file mode 100644
> > index 0000000000..b65154d125
> > --- /dev/null
> > +++ b/test/dm/nvmxip.c
> > @@ -0,0 +1,117 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Functional tests for UCLASS_FFA  class
> > + *
> > + * Copyright 2023 Arm Limited and/or its affiliates <open-source-office@arm.com>
> > + *
> > + * Authors:
> > + *   Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
> > + */
> > +
> > +#include <common.h>
> > +#include <console.h>
> > +#include <blk.h>
> > +#include <dm.h>
> > +#include <dm/test.h>
> > +#include "../../drivers/nvmxip/nvmxip.h"
> 
> move to end
> 
> > +#include <test/test.h>
> > +#include <test/ut.h>
> > +
> > +/* NVMXIP devices described in the device tree  */
> > +#define SANDBOX_NVMXIP_DEVICES 2
> > +
> > +/* reference device tree data for the probed devices */
> > +static struct nvmxip_plat nvmqspi_refdata[SANDBOX_NVMXIP_DEVICES] = {
> > +       {0x08000000, 9, 4096}, {0x08200000, 9, 2048}
> > +};
> > +
> > +#define NVMXIP_BLK_START_PATTERN 0x1122334455667788ULL
> > +#define NVMXIP_BLK_END_PATTERN 0xa1a2a3a4a5a6a7a8ULL
> > +
> > +static int dm_nvmxip_flash_sanity(u8 device_idx, void *buffer)
> > +{
> > +       int i;
> > +       u64 *ptr = NULL;
> > +       u8 *base = NULL;
> > +       unsigned long blksz;
> > +
> > +       blksz = 1 << nvmqspi_refdata[device_idx].lba_shift;
> 
> BIT() or BITUL() ?
> 
> > +
> > +       /* if buffer not NULL, init the flash with the pattern data*/
> 
> space before */   (please fix globally)
> 
> can you explain why, in the comment?
> 
> > +       if (!buffer)
> > +               base = map_sysmem(nvmqspi_refdata[device_idx].phys_base, 0);
> > +       else
> > +               base = buffer;
> > +
> > +       for (i = 0; i < nvmqspi_refdata[device_idx].lba ; i++) {
> > +               ptr = (u64 *)(base + i * blksz);
> > +
> > +               /* write an 8 bytes pattern at the start of the current block*/
> > +               if (!buffer)
> > +                       *ptr = NVMXIP_BLK_START_PATTERN;
> > +               else if (*ptr != NVMXIP_BLK_START_PATTERN)
> > +                       return -EINVAL;
> > +
> > +               ptr = (u64 *)((u8 *)ptr + blksz - sizeof(u64));
> > +
> > +               /* write an 8 bytes pattern at the end of the current block*/
> > +               if (!buffer)
> > +                       *ptr = NVMXIP_BLK_END_PATTERN;
> > +               else if (*ptr != NVMXIP_BLK_END_PATTERN)
> > +                       return -EINVAL;
> 
> You can use ut_assert...() here so that people can see which line failed
> 
> > +       }
> > +
> > +       if (!buffer)
> > +               unmap_sysmem(base);
> > +
> > +       return 0;
> > +}
> > +
> > +static int dm_test_nvmxip(struct unit_test_state *uts)
> > +{
> > +       struct nvmxip_plat *plat_data = NULL;
> > +       struct udevice *dev = NULL, *bdev = NULL;
> > +       u8 device_idx;
> > +       void *buffer = NULL;
> > +       unsigned long flashsz;
> > +
> > +       /* set the flash content first for both devices */
> > +       dm_nvmxip_flash_sanity(0, NULL);
> > +       dm_nvmxip_flash_sanity(1, NULL);
> > +
> > +       /*  probing all NVM XIP QSPI devices */
> > +       for (device_idx = 0, uclass_first_device(UCLASS_NVMXIP, &dev);
> > +            dev;
> > +            uclass_next_device(&dev), device_idx++) {
> > +               plat_data = dev_get_plat(dev);
> > +
> > +               /* device tree entries checks */
> > +               ut_assertok(nvmqspi_refdata[device_idx].phys_base != plat_data->phys_base);
> > +               ut_assertok(nvmqspi_refdata[device_idx].lba_shift != plat_data->lba_shift);
> > +               ut_assertok(nvmqspi_refdata[device_idx].lba != plat_data->lba);
> > +
> > +               /* before reading all the flash blocks, let's calculate the flash size */
> > +               flashsz = plat_data->lba << plat_data->lba_shift;
> > +
> > +               /* allocate the user buffer where to copy the blocks data to */
> > +               buffer = calloc(flashsz, 1);
> > +               ut_assertok(!buffer);
> > +
> > +               /* the block device is the child of the parent device probed with DT*/
> > +               ut_assertok(device_find_first_child(dev, &bdev));
> > +
> > +               /* reading all the flash blocks*/
> > +               ut_asserteq(plat_data->lba, blk_read(bdev, 0, plat_data->lba, buffer));
> > +
> > +               /* compare the data read from flash with the expected data */
> > +               ut_assertok(dm_nvmxip_flash_sanity(device_idx, buffer));
> > +
> > +               free(buffer);
> > +       }
> > +
> > +       ut_assertok(device_idx != SANDBOX_NVMXIP_DEVICES);
> > +
> > +       return CMD_RET_SUCCESS;
> > +}
> > +
> > +DM_TEST(dm_test_nvmxip, UT_TESTF_SCAN_FDT | UT_TESTF_CONSOLE_REC);
> > --
> > 2.17.1
> >
> 
> It seems like you are using the same driver for sandbox as for the
> real hardware. Is that right? There is nothing really wrong with that,
> it's just unusual.

Yes, the driver is exactly the same for real HW and sandbox because the logic is the same. No need to duplicate it.

Cheers,
Abdellatif

Re: [PATCH v1 3/6] sandbox64: add support for NVMXIP QSPI

[Abdellatif El Khlifi]

On Tue, Feb 07, 2023 at 11:38:45AM -0700, Simon Glass wrote:

Hi Simon,

> Hi,
> 
> On Tue, 7 Feb 2023 at 09:30, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Mon, Feb 06, 2023 at 09:02:38PM -0700, Simon Glass wrote:
> > > On Mon, 16 Jan 2023 at 10:28, <abdellatif.elkhlifi@arm.com> wrote:
> > > >
> > > > From: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
> > > >
> > > > enable NVMXIP QSPI for sandbox 64-bit
> > > >
> > > > Adding two NVM XIP QSPI storage devices.
> > > >
> > > > Signed-off-by: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
> > > > ---
> > > >  arch/sandbox/dts/sandbox64.dts | 13 +++++++++++++
> > > >  arch/sandbox/dts/test.dts      | 14 ++++++++++++++
> > > >  configs/sandbox64_defconfig    |  1 +
> > > >  drivers/nvmxip/nvmxip.c        |  4 ++++
> > > >  drivers/nvmxip/nvmxip.h        |  3 +++
> > > >  5 files changed, 35 insertions(+)
> > >
> > > Reviewed-by: Simon Glass <sjg@chromium.org>
> > >
> > > Nit below
> > >
> > > >
> > > > diff --git a/arch/sandbox/dts/sandbox64.dts b/arch/sandbox/dts/sandbox64.dts
> > > > index a9cd7908f8..aed3801af8 100644
> > > > --- a/arch/sandbox/dts/sandbox64.dts
> > > > +++ b/arch/sandbox/dts/sandbox64.dts
> > > > @@ -89,6 +89,19 @@
> > > >                 cs-gpios = <0>, <&gpio_a 0>;
> > > >         };
> > > >
> > > > +       nvmxip-qspi1@08000000 {
> > > > +               compatible = "nvmxip,qspi";
> > > > +               reg = <0x0 0x08000000 0x0 0x00200000>;
> > > > +               lba_shift = <9>;
> > > > +               lba = <4096>;
> > > > +       };
> > > > +
> > > > +       nvmxip-qspi2@08200000 {
> > > > +               compatible = "nvmxip,qspi";
> > > > +               reg = <0x0 0x08200000 0x0 0x00100000>;
> > > > +               lba_shift = <9>;
> > > > +               lba = <2048>;
> > > > +       };
> > > >  };
> > > >
> > > >  #include "sandbox.dtsi"
> > > > diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
> > > > index dffe10adbf..c3ba0a225e 100644
> > > > --- a/arch/sandbox/dts/test.dts
> > > > +++ b/arch/sandbox/dts/test.dts
> > > > @@ -1745,6 +1745,20 @@
> > > >                 compatible = "u-boot,fwu-mdata-gpt";
> > > >                 fwu-mdata-store = <&mmc0>;
> > > >         };
> > > > +
> > > > +       nvmxip-qspi1@08000000 {
> > > > +               compatible = "nvmxip,qspi";
> > > > +               reg = <0x08000000 0x00200000>;
> > > > +               lba_shift = <9>;
> > > > +               lba = <4096>;
> > > > +       };
> > > > +
> > > > +       nvmxip-qspi2@08200000 {
> > > > +               compatible = "nvmxip,qspi";
> > > > +               reg = <0x08200000 0x00100000>;
> > > > +               lba_shift = <9>;
> > > > +               lba = <2048>;
> > > > +       };
> > > >  };
> > > >
> > > >  #include "sandbox_pmic.dtsi"
> > > > diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig
> > > > index ba45ac0b71..cd4dadb190 100644
> > > > --- a/configs/sandbox64_defconfig
> > > > +++ b/configs/sandbox64_defconfig
> > > > @@ -259,3 +259,4 @@ CONFIG_FWU_MULTI_BANK_UPDATE=y
> > > >  CONFIG_UNIT_TEST=y
> > > >  CONFIG_UT_TIME=y
> > > >  CONFIG_UT_DM=y
> > > > +CONFIG_NVMXIP_QSPI=y
> > > > \ No newline at end of file
> > > > diff --git a/drivers/nvmxip/nvmxip.c b/drivers/nvmxip/nvmxip.c
> > > > index c276fb147e..ea9b9bfa48 100644
> > > > --- a/drivers/nvmxip/nvmxip.c
> > > > +++ b/drivers/nvmxip/nvmxip.c
> > > > @@ -87,6 +87,10 @@ int nvmxip_init(struct udevice *udev)
> > > >         priv_data->udev = udev;
> > > >         priv_data->plat_data = plat_data;
> > > >
> > > > +#if CONFIG_IS_ENABLED(SANDBOX64)
> > >
> > > if (IS_ENABLED(CONFIG_SANDBOX64))
> > >
> > >
> > > > +       sandbox_set_enable_memio(true);
> > > > +#endif
> >
> > Isn't that just going to introduce a warning about undeclared function
> > on non-sandbox64?
> 
> Well, the header file should always be included, so it shouldn't!

I tried what you suggested by including #include <asm/test.h> without #ifdef.

When compiling for Arm, the compilation fails:

drivers/mtd/nvmxip/nvmxip-uclass.c:12:10: fatal error: asm/test.h: No such
 file or directory                                                       
   12 | #include <asm/test.h>
      |          ^~~~~~~~~~~~
compilation terminated.

Reason of failure:

sandbox_set_enable_memio() prototype is declared in sandbox/include/asm/test.h, which doesn't work for platforms other than sandbox.

Even when including include/test/test.h instead of sandbox/include/asm/test.h, it will fail because there is an #ifdef CONFIG_SANDBOX in include/test/test.h:

#ifdef CONFIG_SANDBOX
#include <asm/state.h>
#include <asm/test.h>
#endif

I think better to keep the #if CONFIG_IS_ENABLED(SANDBOX64) in nvmxip-uclass.c.

Cheers,
Abdellatif

> 
> Please also add a newline to the defconfig.
> 
> Regards,
> Simon

Re: [PATCH v2 3/7] sandbox64: fix: return unsigned long in readq()

[Simon Glass]

On Mon, 17 Apr 2023 at 03:12, Abdellatif El Khlifi
<abdellatif.elkhlifi@arm.com> wrote:
>
> make readq return unsigned long
>
> readq should return 64-bit data
>
> Signed-off-by: Abdellatif El Khlifi <abdellatif.elkhlifi@arm.com>
> ---
>  arch/sandbox/cpu/cpu.c        | 2 +-
>  arch/sandbox/include/asm/io.h | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>

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

Re: [PATCH v2 0/7] introduce NVM XIP block storage emulation

[Tom Rini]

On Mon, 17 Apr 2023 10:11:51 +0100, Abdellatif El Khlifi wrote:

> Adding block storage emulation for NVM XIP flash devices.
> 
> Some paltforms such as Corstone-1000 need to see NVM XIP raw flash
> as a block storage device with read only capability.
> 
> Here NVM flash devices are devices with addressable
> memory (e.g: QSPI NOR flash).
> 
> [...]

Applied to u-boot/master, thanks!

-- 
Tom