[PATCH v4 0/4] SPL NVMe support

[PATCH v4 0/4] SPL NVMe support

[Mayuresh Chitale]

This patchset adds support to load images of the SPL's next booting
stage from a NVMe device.

Changes in v4:
- Drop patch 4
- Modify patch 2 to use generic fs.h APIs

Changes in v3:
- Add generic API to fetch payload from Ext or FAT filesystems
- Remove reduntant SPL_PCI_PNP config check

Changes in v2:
- Rebase on v2023.07-rc1
- Use uclass ID for blk APIs
- Add support to load FIT images from ext filesystem

Mayuresh Chitale (5):
  spl: Add Kconfig options for NVME
  spl: blk: Support loading images from fs
  nvme: pci: Enable for SPL
  spl: Support loading a FIT from ext FS
  common: spl: Add spl NVMe boot support

Mayuresh Chitale (4):
  spl: Add Kconfig options for NVME
  spl: blk: Support loading images from fs
  nvme: pci: Enable for SPL
  common: spl: Add spl NVMe boot support

 arch/riscv/include/asm/spl.h |   1 +
 common/spl/Kconfig           |  27 +++++++
 common/spl/Makefile          |   2 +
 common/spl/spl_blk_fs.c      | 134 +++++++++++++++++++++++++++++++++++
 common/spl/spl_nvme.c        |  32 +++++++++
 disk/part.c                  |  10 +--
 drivers/Makefile             |   1 +
 drivers/block/Kconfig        |   7 ++
 drivers/nvme/Makefile        |   2 +-
 drivers/pci/Kconfig          |   6 ++
 include/spl.h                |   3 +
 11 files changed, 220 insertions(+), 5 deletions(-)
 create mode 100644 common/spl/spl_blk_fs.c
 create mode 100644 common/spl/spl_nvme.c

-- 
2.34.1

[PATCH v4 1/4] spl: Add Kconfig options for NVME

[Mayuresh Chitale]

Add kconfig options to enable NVME and PCI NVMe support in SPL

Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
 common/spl/Kconfig | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/common/spl/Kconfig b/common/spl/Kconfig
index 2c042ad306..94b13f7a7f 100644
--- a/common/spl/Kconfig
+++ b/common/spl/Kconfig
@@ -1263,6 +1263,32 @@ config SPL_SATA_RAW_U_BOOT_SECTOR
 	  Sector on the SATA disk to load U-Boot from, when the SATA disk is being
 	  used in raw mode. Units: SATA disk sectors (1 sector = 512 bytes).
 
+config SPL_NVME
+	bool "NVM Express device support"
+	depends on BLK
+	select HAVE_BLOCK_DEVICE
+	select FS_LOADER
+	help
+	  This option enables support for NVM Express devices.
+	  It supports basic functions of NVMe (read/write).
+
+config SPL_NVME_PCI
+	bool "NVM Express PCI device support for SPL"
+	depends on SPL_PCI && SPL_NVME
+	help
+	  This option enables support for NVM Express PCI devices.
+	  This allows use of NVMe devices for loading u-boot.
+
+config SPL_NVME_BOOT_DEVICE
+	hex "NVMe boot device number"
+	depends on SPL_NVME
+	default 0x0
+
+config SYS_NVME_BOOT_PARTITION
+	hex "NVMe boot partition number"
+	depends on SPL_NVME
+	default	0x1
+
 config SPL_SERIAL
 	bool "Support serial"
 	select SPL_PRINTF
-- 
2.34.1

[PATCH v4 2/4] spl: blk: Support loading images from fs

[Mayuresh Chitale]

Add a generic API to support loading of SPL payload from any supported
filesystem on a given partition of a block device.

Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
---
 common/spl/Kconfig      |   1 +
 common/spl/Makefile     |   1 +
 common/spl/spl_blk_fs.c | 134 ++++++++++++++++++++++++++++++++++++++++
 drivers/block/Kconfig   |   7 +++
 include/spl.h           |   3 +
 5 files changed, 146 insertions(+)
 create mode 100644 common/spl/spl_blk_fs.c

diff --git a/common/spl/Kconfig b/common/spl/Kconfig
index 94b13f7a7f..3f705f4fb8 100644
--- a/common/spl/Kconfig
+++ b/common/spl/Kconfig
@@ -1268,6 +1268,7 @@ config SPL_NVME
 	depends on BLK
 	select HAVE_BLOCK_DEVICE
 	select FS_LOADER
+	select SPL_BLK_FS
 	help
 	  This option enables support for NVM Express devices.
 	  It supports basic functions of NVMe (read/write).
diff --git a/common/spl/Makefile b/common/spl/Makefile
index 13db3df993..5210ad0248 100644
--- a/common/spl/Makefile
+++ b/common/spl/Makefile
@@ -10,6 +10,7 @@ ifdef CONFIG_SPL_BUILD
 obj-$(CONFIG_$(SPL_TPL_)FRAMEWORK) += spl.o
 obj-$(CONFIG_$(SPL_TPL_)BOOTROM_SUPPORT) += spl_bootrom.o
 obj-$(CONFIG_$(SPL_TPL_)LOAD_FIT) += spl_fit.o
+obj-$(CONFIG_$(SPL_TPL_)BLK_FS) += spl_blk_fs.o
 obj-$(CONFIG_$(SPL_TPL_)LEGACY_IMAGE_FORMAT) += spl_legacy.o
 obj-$(CONFIG_$(SPL_TPL_)NOR_SUPPORT) += spl_nor.o
 obj-$(CONFIG_$(SPL_TPL_)XIP_SUPPORT) += spl_xip.o
diff --git a/common/spl/spl_blk_fs.c b/common/spl/spl_blk_fs.c
new file mode 100644
index 0000000000..d97adc4d39
--- /dev/null
+++ b/common/spl/spl_blk_fs.c
@@ -0,0 +1,134 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2023
+ * Ventana Micro Systems Inc.
+ *
+ */
+
+#include <common.h>
+#include <spl.h>
+#include <image.h>
+#include <fs.h>
+
+struct blk_dev {
+	const char *ifname;
+	char dev_part_str[8];
+};
+
+static ulong spl_fit_read(struct spl_load_info *load, ulong file_offset,
+			  ulong size, void *buf)
+{
+	loff_t actlen;
+	int ret;
+	struct blk_dev *dev = (struct blk_dev *)load->priv;
+
+	ret = fs_set_blk_dev(dev->ifname, dev->dev_part_str, FS_TYPE_ANY);
+	if (ret) {
+		printf("spl: unable to set blk_dev %s %s. Err - %d\n",
+		       dev->ifname, dev->dev_part_str, ret);
+		return ret;
+	}
+
+	ret = fs_read(load->filename, (ulong)buf, file_offset, size, &actlen);
+	if (ret < 0) {
+		printf("spl: error reading image %s. Err - %d\n",
+		       load->filename, ret);
+		return ret;
+	}
+
+	return actlen;
+}
+
+int spl_blk_load_image(struct spl_image_info *spl_image,
+		       struct spl_boot_device *bootdev,
+		       enum uclass_id uclass_id, int devnum, int partnum)
+{
+	const char *filename = CONFIG_SPL_PAYLOAD;
+	struct disk_partition part_info = {};
+	struct legacy_img_hdr *header;
+	struct blk_desc *blk_desc;
+	loff_t actlen, filesize;
+	struct blk_dev dev;
+	int ret;
+
+	blk_desc = blk_get_devnum_by_uclass_id(uclass_id, devnum);
+	if (!blk_desc) {
+		printf("blk desc for %d %d not found\n", uclass_id, devnum);
+		goto out;
+	}
+
+	blk_show_device(uclass_id, devnum);
+	header = spl_get_load_buffer(-sizeof(*header), sizeof(*header));
+	ret = part_get_info(blk_desc, 1, &part_info);
+	if (ret) {
+		printf("spl: no partition table found. Err - %d\n", ret);
+		goto out;
+	}
+
+	dev.ifname = blk_get_uclass_name(uclass_id);
+	snprintf(dev.dev_part_str, sizeof(dev.dev_part_str) - 1, "%d:%d",
+		 devnum, partnum);
+	ret = fs_set_blk_dev(dev.ifname, dev.dev_part_str, FS_TYPE_ANY);
+	if (ret) {
+		printf("spl: unable to set blk_dev %s %s. Err - %d\n",
+		       dev.ifname, dev.dev_part_str, ret);
+		goto out;
+	}
+
+	ret = fs_read(filename, (ulong)header, 0,
+		      sizeof(struct legacy_img_hdr), &actlen);
+	if (ret) {
+		printf("spl: unable to read file %s. Err - %d\n", filename,
+		       ret);
+		goto out;
+	}
+
+	if (IS_ENABLED(CONFIG_SPL_LOAD_FIT) &&
+	    image_get_magic(header) == FDT_MAGIC) {
+		struct spl_load_info load;
+
+		debug("Found FIT\n");
+		load.read = spl_fit_read;
+		load.bl_len = 1;
+		load.filename = (void *)filename;
+		load.priv = &dev;
+
+		return spl_load_simple_fit(spl_image, &load, 0, header);
+	}
+
+	ret = spl_parse_image_header(spl_image, bootdev, header);
+	if (ret) {
+		printf("spl: unable to parse image header. Err - %d\n",
+		       ret);
+		goto out;
+	}
+
+	ret = fs_set_blk_dev(dev.ifname, dev.dev_part_str, FS_TYPE_ANY);
+	if (ret) {
+		printf("spl: unable to set blk_dev %s %s. Err - %d\n",
+		       dev.ifname, dev.dev_part_str, ret);
+		goto out;
+	}
+
+	ret = fs_size(filename, &filesize);
+	if (ret) {
+		printf("spl: unable to get file size: %s. Err - %d\n",
+		       filename, ret);
+		goto out;
+	}
+
+	ret = fs_set_blk_dev(dev.ifname, dev.dev_part_str, FS_TYPE_ANY);
+	if (ret) {
+		printf("spl: unable to set blk_dev %s %s. Err - %d\n",
+		       dev.ifname, dev.dev_part_str, ret);
+		goto out;
+	}
+
+	ret = fs_read(filename, (ulong)spl_image->load_addr, 0, filesize,
+		      &actlen);
+	if (ret)
+		printf("spl: unable to read file %s. Err - %d\n",
+		       filename, ret);
+out:
+	return ret;
+}
diff --git a/drivers/block/Kconfig b/drivers/block/Kconfig
index 5a1aeb3d2b..6baaa6f071 100644
--- a/drivers/block/Kconfig
+++ b/drivers/block/Kconfig
@@ -107,6 +107,13 @@ config EFI_MEDIA
 
 	  For sandbox there is a test driver.
 
+config SPL_BLK_FS
+	bool "Load images from filesystems on block devices"
+	depends on SPL_BLK
+	help
+	  Use generic support to load images from fat/ext filesystems on
+	  different types of block devices such as NVMe.
+
 if EFI_MEDIA
 
 config EFI_MEDIA_SANDBOX
diff --git a/include/spl.h b/include/spl.h
index 7e0f5ac63b..20e1eb32a4 100644
--- a/include/spl.h
+++ b/include/spl.h
@@ -672,6 +672,9 @@ int spl_load_image_ext(struct spl_image_info *spl_image,
 int spl_load_image_ext_os(struct spl_image_info *spl_image,
 			  struct spl_boot_device *bootdev,
 			  struct blk_desc *block_dev, int partition);
+int spl_blk_load_image(struct spl_image_info *spl_image,
+		       struct spl_boot_device *bootdev,
+		       enum uclass_id uclass_id, int devnum, int partnum);
 
 /**
  * spl_early_init() - Set up device tree and driver model in SPL if enabled
-- 
2.34.1

[PATCH v4 3/4] nvme: pci: Enable for SPL

[Mayuresh Chitale]

Enable NVME and PCI NVMe drivers for SPL builds. Also enable PCI_PNP
for SPL which is required to auto configure the PCIe devices.

Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
---
 drivers/Makefile      | 1 +
 drivers/nvme/Makefile | 2 +-
 drivers/pci/Kconfig   | 6 ++++++
 3 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/Makefile b/drivers/Makefile
index 58be410135..dc559ea7f7 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -34,6 +34,7 @@ obj-$(CONFIG_$(SPL_)DM_MAILBOX) += mailbox/
 obj-$(CONFIG_$(SPL_)REMOTEPROC) += remoteproc/
 obj-$(CONFIG_$(SPL_)SYSINFO) += sysinfo/
 obj-$(CONFIG_$(SPL_TPL_)TPM) += tpm/
+obj-$(CONFIG_$(SPL_)NVME) += nvme/
 obj-$(CONFIG_XEN) += xen/
 obj-$(CONFIG_$(SPL_)FPGA) += fpga/
 obj-y += bus/
diff --git a/drivers/nvme/Makefile b/drivers/nvme/Makefile
index fa7b619446..fd3e68a91d 100644
--- a/drivers/nvme/Makefile
+++ b/drivers/nvme/Makefile
@@ -4,4 +4,4 @@
 
 obj-y += nvme-uclass.o nvme.o nvme_show.o
 obj-$(CONFIG_NVME_APPLE) += nvme_apple.o
-obj-$(CONFIG_NVME_PCI) += nvme_pci.o
+obj-$(CONFIG_$(SPL_)NVME_PCI) += nvme_pci.o
diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
index ef328d2652..dca71ef504 100644
--- a/drivers/pci/Kconfig
+++ b/drivers/pci/Kconfig
@@ -40,6 +40,12 @@ config PCI_PNP
 	help
 	  Enable PCI memory and I/O space resource allocation and assignment.
 
+config SPL_PCI_PNP
+	bool "Enable Plug & Play support for PCI"
+	help
+	  Enable PCI memory and I/O space resource allocation and assignment.
+	  This is required to auto configure the enumerated devices.
+
 config PCI_REGION_MULTI_ENTRY
 	bool "Enable Multiple entries of region type MEMORY in ranges for PCI"
 	help
-- 
2.34.1

[PATCH v4 4/4] common: spl: Add spl NVMe boot support

[Mayuresh Chitale]

Add support to load the next stage image from an NVMe disk which may
be formatted as an EXT or FAT filesystem. Also protect the call to
env_get in blk_get_device_part_str with CONFIG_SPL_ENV_SUPPORT macro to
avoid link error when SPL_ENV_SUPPORT is not enabled.

Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
---
 arch/riscv/include/asm/spl.h |  1 +
 common/spl/Makefile          |  1 +
 common/spl/spl_nvme.c        | 32 ++++++++++++++++++++++++++++++++
 disk/part.c                  | 10 ++++++----
 4 files changed, 40 insertions(+), 4 deletions(-)
 create mode 100644 common/spl/spl_nvme.c

diff --git a/arch/riscv/include/asm/spl.h b/arch/riscv/include/asm/spl.h
index 2898a770ee..9c0bf9755c 100644
--- a/arch/riscv/include/asm/spl.h
+++ b/arch/riscv/include/asm/spl.h
@@ -20,6 +20,7 @@ enum {
 	BOOT_DEVICE_SPI,
 	BOOT_DEVICE_USB,
 	BOOT_DEVICE_SATA,
+	BOOT_DEVICE_NVME,
 	BOOT_DEVICE_I2C,
 	BOOT_DEVICE_BOARD,
 	BOOT_DEVICE_DFU,
diff --git a/common/spl/Makefile b/common/spl/Makefile
index 5210ad0248..bad2bbf6cf 100644
--- a/common/spl/Makefile
+++ b/common/spl/Makefile
@@ -29,6 +29,7 @@ obj-$(CONFIG_$(SPL_TPL_)USB_STORAGE) += spl_usb.o
 obj-$(CONFIG_$(SPL_TPL_)FS_FAT) += spl_fat.o
 obj-$(CONFIG_$(SPL_TPL_)FS_EXT4) += spl_ext.o
 obj-$(CONFIG_$(SPL_TPL_)SATA) += spl_sata.o
+obj-$(CONFIG_$(SPL_TPL_)NVME) += spl_nvme.o
 obj-$(CONFIG_$(SPL_TPL_)SEMIHOSTING) += spl_semihosting.o
 obj-$(CONFIG_$(SPL_TPL_)DFU) += spl_dfu.o
 obj-$(CONFIG_$(SPL_TPL_)SPI_LOAD) += spl_spi.o
diff --git a/common/spl/spl_nvme.c b/common/spl/spl_nvme.c
new file mode 100644
index 0000000000..2af63f1dc8
--- /dev/null
+++ b/common/spl/spl_nvme.c
@@ -0,0 +1,32 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2023
+ * Ventana Micro Systems Inc.
+ *
+ */
+
+#include <common.h>
+#include <spl.h>
+#include <init.h>
+#include <nvme.h>
+
+static int spl_nvme_load_image(struct spl_image_info *spl_image,
+			       struct spl_boot_device *bootdev)
+{
+	int ret;
+
+	ret = pci_init();
+	if (ret < 0)
+		return ret;
+
+	ret = nvme_scan_namespace();
+	if (ret < 0)
+		return ret;
+
+	ret = spl_blk_load_image(spl_image, bootdev, UCLASS_NVME,
+				 CONFIG_SPL_NVME_BOOT_DEVICE,
+				 CONFIG_SYS_NVME_BOOT_PARTITION);
+	return ret;
+}
+
+SPL_LOAD_IMAGE_METHOD("NVME", 0, BOOT_DEVICE_NVME, spl_nvme_load_image);
diff --git a/disk/part.c b/disk/part.c
index 35300df590..d330c57ab4 100644
--- a/disk/part.c
+++ b/disk/part.c
@@ -467,10 +467,12 @@ int blk_get_device_part_str(const char *ifname, const char *dev_part_str,
 	}
 #endif
 
-	/* If no dev_part_str, use bootdevice environment variable */
-	if (!dev_part_str || !strlen(dev_part_str) ||
-	    !strcmp(dev_part_str, "-"))
-		dev_part_str = env_get("bootdevice");
+	if (IS_ENABLED(CONFIG_SPL_ENV_SUPPORT)) {
+		/* If no dev_part_str, use bootdevice environment variable */
+		if (!dev_part_str || !strlen(dev_part_str) ||
+		    !strcmp(dev_part_str, "-"))
+			dev_part_str = env_get("bootdevice");
+	}
 
 	/* If still no dev_part_str, it's an error */
 	if (!dev_part_str) {
-- 
2.34.1

Re: [PATCH v4 0/4] SPL NVMe support

[Tom Rini]

On Sat, 03 Jun 2023 19:32:52 +0530, Mayuresh Chitale wrote:

> This patchset adds support to load images of the SPL's next booting
> stage from a NVMe device.
> 
> Changes in v4:
> - Drop patch 4
> - Modify patch 2 to use generic fs.h APIs
> 
> [...]

With one change, which is that the "disk/part.c" in 4/4 were not required for
any platform in tree and also broke testcases, and so was dropped, this has now
been applied to u-boot/next. If you can explain a bit more what the problem you
had was, we can look in to it. I suspect you need to test for not
SPL_ENV_SUPPORT  but ENV_SUPPORT itself.

-- 
Tom

Re: [PATCH v4 0/4] SPL NVMe support

[mchitale]

Hi Tom,

On Tue, 2023-06-20 at 09:37 -0400, Tom Rini wrote:
> On Sat, 03 Jun 2023 19:32:52 +0530, Mayuresh Chitale wrote:
> 
> > This patchset adds support to load images of the SPL's next booting
> > stage from a NVMe device.
> > 
> > Changes in v4:
> > - Drop patch 4
> > - Modify patch 2 to use generic fs.h APIs
> > 
> > [...]
> 
> With one change, which is that the "disk/part.c" in 4/4 were not
> required for
> any platform in tree and also broke testcases, and so was dropped,
> this has now
> been applied to u-boot/next. If you can explain a bit more what the
> problem you
> had was, we can look in to it. I suspect you need to test for not
> SPL_ENV_SUPPORT  but ENV_SUPPORT itself.
> 
Thanks. 
When SPL_NVME is enabled the build breaks with the following error:
riscv64-unknown-linux-gnu-ld.bfd: disk/part.o: in function
`blk_get_device_part_str':
u-boot/disk/part.c:473: undefined reference to `env_get'
make[2]: *** [u-boot/scripts/Makefile.spl:527: spl/u-boot-spl] Error 1
make[1]: *** [/u-boot/Makefile:2053: spl/u-boot-spl] Error 2

One possible fix is:

if ((!IS_ENABLED(CONFIG_SPL) && IS_ENABLED(CONFIG_ENV_SUPPORT)) ||    
       (IS_ENABLED(CONFIG_SPL) && IS_ENABLED(CONFIG_SPL_ENV_SUPPORT)))
	if (!dev_part_str || !strlen(dev_part_str)
|| !strcmp(dev_part_str, "-"))
	dev_part_str = env_get("bootdevice");

Re: [PATCH v4 0/4] SPL NVMe support

[Heinrich Schuchardt]

On 12.07.23 15:06, mchitale@ventanamicro.com wrote:
> Hi Tom,
>
> On Tue, 2023-06-20 at 09:37 -0400, Tom Rini wrote:
>> On Sat, 03 Jun 2023 19:32:52 +0530, Mayuresh Chitale wrote:
>>
>>> This patchset adds support to load images of the SPL's next booting
>>> stage from a NVMe device.
>>>
>>> Changes in v4:
>>> - Drop patch 4
>>> - Modify patch 2 to use generic fs.h APIs
>>>
>>> [...]
>>
>> With one change, which is that the "disk/part.c" in 4/4 were not
>> required for
>> any platform in tree and also broke testcases, and so was dropped,
>> this has now
>> been applied to u-boot/next. If you can explain a bit more what the
>> problem you
>> had was, we can look in to it. I suspect you need to test for not
>> SPL_ENV_SUPPORT  but ENV_SUPPORT itself.
>>
> Thanks.
> When SPL_NVME is enabled the build breaks with the following error:
> riscv64-unknown-linux-gnu-ld.bfd: disk/part.o: in function
> `blk_get_device_part_str':
> u-boot/disk/part.c:473: undefined reference to `env_get'
> make[2]: *** [u-boot/scripts/Makefile.spl:527: spl/u-boot-spl] Error 1
> make[1]: *** [/u-boot/Makefile:2053: spl/u-boot-spl] Error 2
>
> One possible fix is:
>
> if ((!IS_ENABLED(CONFIG_SPL) && IS_ENABLED(CONFIG_ENV_SUPPORT)) ||
>         (IS_ENABLED(CONFIG_SPL) && IS_ENABLED(CONFIG_SPL_ENV_SUPPORT)))
> 	if (!dev_part_str || !strlen(dev_part_str)
> || !strcmp(dev_part_str, "-"))
> 	dev_part_str = env_get("bootdevice");
>
>

I think CONFIG_SPL_ENV_SUPPORT should depend on CONFIG_ENV_SUPPORT in
common/spl/Kconfig.

Best regards

Heinrich

Re: [PATCH v4 0/4] SPL NVMe support

[Tom Rini]

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

On Wed, Jul 12, 2023 at 03:27:45PM +0200, Heinrich Schuchardt wrote:
> On 12.07.23 15:06, mchitale@ventanamicro.com wrote:
> > Hi Tom,
> > 
> > On Tue, 2023-06-20 at 09:37 -0400, Tom Rini wrote:
> > > On Sat, 03 Jun 2023 19:32:52 +0530, Mayuresh Chitale wrote:
> > > 
> > > > This patchset adds support to load images of the SPL's next booting
> > > > stage from a NVMe device.
> > > > 
> > > > Changes in v4:
> > > > - Drop patch 4
> > > > - Modify patch 2 to use generic fs.h APIs
> > > > 
> > > > [...]
> > > 
> > > With one change, which is that the "disk/part.c" in 4/4 were not
> > > required for
> > > any platform in tree and also broke testcases, and so was dropped,
> > > this has now
> > > been applied to u-boot/next. If you can explain a bit more what the
> > > problem you
> > > had was, we can look in to it. I suspect you need to test for not
> > > SPL_ENV_SUPPORT  but ENV_SUPPORT itself.
> > > 
> > Thanks.
> > When SPL_NVME is enabled the build breaks with the following error:
> > riscv64-unknown-linux-gnu-ld.bfd: disk/part.o: in function
> > `blk_get_device_part_str':
> > u-boot/disk/part.c:473: undefined reference to `env_get'
> > make[2]: *** [u-boot/scripts/Makefile.spl:527: spl/u-boot-spl] Error 1
> > make[1]: *** [/u-boot/Makefile:2053: spl/u-boot-spl] Error 2
> > 
> > One possible fix is:
> > 
> > if ((!IS_ENABLED(CONFIG_SPL) && IS_ENABLED(CONFIG_ENV_SUPPORT)) ||
> >         (IS_ENABLED(CONFIG_SPL) && IS_ENABLED(CONFIG_SPL_ENV_SUPPORT)))
> > 	if (!dev_part_str || !strlen(dev_part_str)
> > || !strcmp(dev_part_str, "-"))
> > 	dev_part_str = env_get("bootdevice");
> > 
> > 
> 
> I think CONFIG_SPL_ENV_SUPPORT should depend on CONFIG_ENV_SUPPORT in
> common/spl/Kconfig.

Not strictly, but checking for CONFIG_IS_ENABLED(ENV_SUPPORT) should do
what's desired here?

-- 
Tom

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

Re: [PATCH v4 0/4] SPL NVMe support

[mchitale]

On Wed, 2023-07-12 at 13:12 -0400, Tom Rini wrote:
> On Wed, Jul 12, 2023 at 03:27:45PM +0200, Heinrich Schuchardt wrote:
> > On 12.07.23 15:06, mchitale@ventanamicro.com wrote:
> > > Hi Tom,
> > > 
> > > On Tue, 2023-06-20 at 09:37 -0400, Tom Rini wrote:
> > > > On Sat, 03 Jun 2023 19:32:52 +0530, Mayuresh Chitale wrote:
> > > > 
> > > > > This patchset adds support to load images of the SPL's next
> > > > > booting
> > > > > stage from a NVMe device.
> > > > > 
> > > > > Changes in v4:
> > > > > - Drop patch 4
> > > > > - Modify patch 2 to use generic fs.h APIs
> > > > > 
> > > > > [...]
> > > > 
> > > > With one change, which is that the "disk/part.c" in 4/4 were
> > > > not
> > > > required for
> > > > any platform in tree and also broke testcases, and so was
> > > > dropped,
> > > > this has now
> > > > been applied to u-boot/next. If you can explain a bit more what
> > > > the
> > > > problem you
> > > > had was, we can look in to it. I suspect you need to test for
> > > > not
> > > > SPL_ENV_SUPPORT  but ENV_SUPPORT itself.
> > > > 
> > > Thanks.
> > > When SPL_NVME is enabled the build breaks with the following
> > > error:
> > > riscv64-unknown-linux-gnu-ld.bfd: disk/part.o: in function
> > > `blk_get_device_part_str':
> > > u-boot/disk/part.c:473: undefined reference to `env_get'
> > > make[2]: *** [u-boot/scripts/Makefile.spl:527: spl/u-boot-spl]
> > > Error 1
> > > make[1]: *** [/u-boot/Makefile:2053: spl/u-boot-spl] Error 2
> > > 
> > > One possible fix is:
> > > 
> > > if ((!IS_ENABLED(CONFIG_SPL) && IS_ENABLED(CONFIG_ENV_SUPPORT))
> > > ||
> > >         (IS_ENABLED(CONFIG_SPL) &&
> > > IS_ENABLED(CONFIG_SPL_ENV_SUPPORT)))
> > > 	if (!dev_part_str || !strlen(dev_part_str)
> > > > > !strcmp(dev_part_str, "-"))
> > > 	dev_part_str = env_get("bootdevice");
> > > 
> > > 
> > 
> > I think CONFIG_SPL_ENV_SUPPORT should depend on CONFIG_ENV_SUPPORT
> > in
> > common/spl/Kconfig.
> 
> Not strictly, but checking for CONFIG_IS_ENABLED(ENV_SUPPORT) should
> do
> what's desired here?

When SPL_NVME & SPL_BLK_FS is enabled, the spl_blk_fs driver calls
fs_set_blk_dev to the set the device & partition before accessing it
and fs_set_blk_dev internally tries to get the device & partition from
the bootdevice env variable if it was not passed by the caller. However
for SPL build, when SPL_ENV_SUPPORT is not enabled nothing provides
env_get and hence the build fails.

Re: [PATCH v4 0/4] SPL NVMe support

[Tom Rini]

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

On Mon, Jul 17, 2023 at 01:39:52PM +0530, mchitale@ventanamicro.com wrote:
> On Wed, 2023-07-12 at 13:12 -0400, Tom Rini wrote:
> > On Wed, Jul 12, 2023 at 03:27:45PM +0200, Heinrich Schuchardt wrote:
> > > On 12.07.23 15:06, mchitale@ventanamicro.com wrote:
> > > > Hi Tom,
> > > > 
> > > > On Tue, 2023-06-20 at 09:37 -0400, Tom Rini wrote:
> > > > > On Sat, 03 Jun 2023 19:32:52 +0530, Mayuresh Chitale wrote:
> > > > > 
> > > > > > This patchset adds support to load images of the SPL's next
> > > > > > booting
> > > > > > stage from a NVMe device.
> > > > > > 
> > > > > > Changes in v4:
> > > > > > - Drop patch 4
> > > > > > - Modify patch 2 to use generic fs.h APIs
> > > > > > 
> > > > > > [...]
> > > > > 
> > > > > With one change, which is that the "disk/part.c" in 4/4 were
> > > > > not
> > > > > required for
> > > > > any platform in tree and also broke testcases, and so was
> > > > > dropped,
> > > > > this has now
> > > > > been applied to u-boot/next. If you can explain a bit more what
> > > > > the
> > > > > problem you
> > > > > had was, we can look in to it. I suspect you need to test for
> > > > > not
> > > > > SPL_ENV_SUPPORT  but ENV_SUPPORT itself.
> > > > > 
> > > > Thanks.
> > > > When SPL_NVME is enabled the build breaks with the following
> > > > error:
> > > > riscv64-unknown-linux-gnu-ld.bfd: disk/part.o: in function
> > > > `blk_get_device_part_str':
> > > > u-boot/disk/part.c:473: undefined reference to `env_get'
> > > > make[2]: *** [u-boot/scripts/Makefile.spl:527: spl/u-boot-spl]
> > > > Error 1
> > > > make[1]: *** [/u-boot/Makefile:2053: spl/u-boot-spl] Error 2
> > > > 
> > > > One possible fix is:
> > > > 
> > > > if ((!IS_ENABLED(CONFIG_SPL) && IS_ENABLED(CONFIG_ENV_SUPPORT))
> > > > ||
> > > >         (IS_ENABLED(CONFIG_SPL) &&
> > > > IS_ENABLED(CONFIG_SPL_ENV_SUPPORT)))
> > > > 	if (!dev_part_str || !strlen(dev_part_str)
> > > > > > !strcmp(dev_part_str, "-"))
> > > > 	dev_part_str = env_get("bootdevice");
> > > > 
> > > > 
> > > 
> > > I think CONFIG_SPL_ENV_SUPPORT should depend on CONFIG_ENV_SUPPORT
> > > in
> > > common/spl/Kconfig.
> > 
> > Not strictly, but checking for CONFIG_IS_ENABLED(ENV_SUPPORT) should
> > do
> > what's desired here?
> 
> When SPL_NVME & SPL_BLK_FS is enabled, the spl_blk_fs driver calls
> fs_set_blk_dev to the set the device & partition before accessing it
> and fs_set_blk_dev internally tries to get the device & partition from
> the bootdevice env variable if it was not passed by the caller. However
> for SPL build, when SPL_ENV_SUPPORT is not enabled nothing provides
> env_get and hence the build fails.

OK.  So in the code we should be able to test with
CONFIG_IS_ENABLED(ENV_SUPPORT) is that will be true for ((SPL and
SPL environment support) or (Main U-Boot and environment support) or
(TPL and TPL env)).

-- 
Tom

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

Re: [PATCH v4 0/4] SPL NVMe support

[mchitale]

On Mon, 2023-07-17 at 11:12 -0400, Tom Rini wrote:
> On Mon, Jul 17, 2023 at 01:39:52PM +0530, mchitale@ventanamicro.com
> wrote:
> > On Wed, 2023-07-12 at 13:12 -0400, Tom Rini wrote:
> > > On Wed, Jul 12, 2023 at 03:27:45PM +0200, Heinrich Schuchardt
> > > wrote:
> > > > On 12.07.23 15:06, mchitale@ventanamicro.com wrote:
> > > > > Hi Tom,
> > > > > 
> > > > > On Tue, 2023-06-20 at 09:37 -0400, Tom Rini wrote:
> > > > > > On Sat, 03 Jun 2023 19:32:52 +0530, Mayuresh Chitale wrote:
> > > > > > 
> > > > > > > This patchset adds support to load images of the SPL's
> > > > > > > next
> > > > > > > booting
> > > > > > > stage from a NVMe device.
> > > > > > > 
> > > > > > > Changes in v4:
> > > > > > > - Drop patch 4
> > > > > > > - Modify patch 2 to use generic fs.h APIs
> > > > > > > 
> > > > > > > [...]
> > > > > > 
> > > > > > With one change, which is that the "disk/part.c" in 4/4
> > > > > > were
> > > > > > not
> > > > > > required for
> > > > > > any platform in tree and also broke testcases, and so was
> > > > > > dropped,
> > > > > > this has now
> > > > > > been applied to u-boot/next. If you can explain a bit more
> > > > > > what
> > > > > > the
> > > > > > problem you
> > > > > > had was, we can look in to it. I suspect you need to test
> > > > > > for
> > > > > > not
> > > > > > SPL_ENV_SUPPORT  but ENV_SUPPORT itself.
> > > > > > 
> > > > > Thanks.
> > > > > When SPL_NVME is enabled the build breaks with the following
> > > > > error:
> > > > > riscv64-unknown-linux-gnu-ld.bfd: disk/part.o: in function
> > > > > `blk_get_device_part_str':
> > > > > u-boot/disk/part.c:473: undefined reference to `env_get'
> > > > > make[2]: *** [u-boot/scripts/Makefile.spl:527: spl/u-boot-
> > > > > spl]
> > > > > Error 1
> > > > > make[1]: *** [/u-boot/Makefile:2053: spl/u-boot-spl] Error 2
> > > > > 
> > > > > One possible fix is:
> > > > > 
> > > > > if ((!IS_ENABLED(CONFIG_SPL) &&
> > > > > IS_ENABLED(CONFIG_ENV_SUPPORT))
> > > > >         (IS_ENABLED(CONFIG_SPL) &&
> > > > > IS_ENABLED(CONFIG_SPL_ENV_SUPPORT)))
> > > > > 	if (!dev_part_str || !strlen(dev_part_str)
> > > > > > > !strcmp(dev_part_str, "-"))
> > > > > 	dev_part_str = env_get("bootdevice");
> > > > > 
> > > > > 
> > > > 
> > > > I think CONFIG_SPL_ENV_SUPPORT should depend on
> > > > CONFIG_ENV_SUPPORT
> > > > in
> > > > common/spl/Kconfig.
> > > 
> > > Not strictly, but checking for CONFIG_IS_ENABLED(ENV_SUPPORT)
> > > should
> > > do
> > > what's desired here?
> > 
> > When SPL_NVME & SPL_BLK_FS is enabled, the spl_blk_fs driver calls
> > fs_set_blk_dev to the set the device & partition before accessing
> > it
> > and fs_set_blk_dev internally tries to get the device & partition
> > from
> > the bootdevice env variable if it was not passed by the caller.
> > However
> > for SPL build, when SPL_ENV_SUPPORT is not enabled nothing provides
> > env_get and hence the build fails.
> 
> OK.  So in the code we should be able to test with
> CONFIG_IS_ENABLED(ENV_SUPPORT) is that will be true for ((SPL and
> SPL environment support) or (Main U-Boot and environment support) or
> (TPL and TPL env)).

Thanks! I will send the patch with this change.
>