[PATCH v2 0/5] imx9{4,5}: Add Quickboot support

[PATCH v2 0/5] imx9{4,5}: Add Quickboot support

[Simona Toaca (OSS)]

From: Simona Toaca <simona.toaca@nxp.com>

This patch series adds support for saving DDR training
data to non-volatile memory on iMX94 and iMX95 platforms.
The purpose is running DDR Quickboot flow on next reboot.

The process is as follows:
- OEI runs Training flow for the DDRPHY
- OEI saves the data from training to volatile memory
- U-Boot can then save it to non-volatile memory (e.g. SD)
- OEI loads the data from NVM at cold reboot and runs Quickboot flow

By skipping training, a much lower boot time is achieved.

Changes for v2:
- Improved documentation to clarify the questions asked
- Detailed log messages for all commits
- Detailed Kconfig options for SPL_IMX_QB and CMD_IMX_QB
- Fixed the mentioned coding style issues

Simona Toaca (5):
  imx9: Add support for saving DDR training data to NVM
  arm: mach-imx: Add command to expose QB functionality
  imx9: Enable QB data saving for iMX9{4,5} EVK
  board: nxp: imx9{4,5}_evk: Add qb save option in SPL
  doc: board: nxp: Add Quickboot documentation

 arch/arm/include/asm/arch-imx9/ddr.h |  52 +++-
 arch/arm/include/asm/mach-imx/qb.h   |  15 +
 arch/arm/mach-imx/Kconfig            |  19 ++
 arch/arm/mach-imx/Makefile           |   1 +
 arch/arm/mach-imx/cmd_qb.c           | 132 ++++++++
 arch/arm/mach-imx/imx9/Makefile      |  12 +-
 arch/arm/mach-imx/imx9/qb.c          | 439 +++++++++++++++++++++++++++
 arch/arm/mach-imx/imx9/scmi/soc.c    |   9 +
 board/nxp/imx94_evk/spl.c            |   6 +-
 board/nxp/imx95_evk/spl.c            |   6 +-
 configs/imx943_evk_defconfig         |   1 +
 configs/imx95_15x15_evk_defconfig    |   1 +
 configs/imx95_evk.config             |   1 +
 doc/board/nxp/index.rst              |   1 +
 doc/board/nxp/qb.rst                 |  49 +++
 drivers/ddr/imx/imx9/Kconfig         |   8 +
 drivers/ddr/imx/phy/Kconfig          |   7 +
 17 files changed, 754 insertions(+), 5 deletions(-)
 create mode 100644 arch/arm/include/asm/mach-imx/qb.h
 create mode 100644 arch/arm/mach-imx/cmd_qb.c
 create mode 100644 arch/arm/mach-imx/imx9/qb.c
 create mode 100644 doc/board/nxp/qb.rst

-- 
2.43.0

[PATCH v2 1/5] imx9: Add support for saving DDR training data to NVM

[Simona Toaca (OSS)]

From: Simona Toaca <simona.toaca@nxp.com>

DDR training data can be saved to NVM and be available
to OEI at boot time, which will trigger QuickBoot flow.

U-Boot only checks for data integrity (CRC32), while
OEI is in charge of authentication when it tries to
load the data from NVM.

On iMX95 A0/A1, 'authentication' is done via another
CRC32. On the other boards, authentication is done by
using ELE to check the MAC stored in the ddrphy_qb_state
structure.

Supported platforms: iMX94, iMX95, .. (using OEI)
Supported storage types: eMMC, SD, SPI flash.

Signed-off-by: Viorel Suman <viorel.suman@nxp.com>
Signed-off-by: Ye Li <ye.li@nxp.com>
Signed-off-by: Simona Toaca <simona.toaca@nxp.com>
---
 arch/arm/include/asm/arch-imx9/ddr.h |  52 +++-
 arch/arm/include/asm/mach-imx/qb.h   |  13 +
 arch/arm/mach-imx/imx9/Makefile      |   8 +-
 arch/arm/mach-imx/imx9/qb.c          | 430 +++++++++++++++++++++++++++
 arch/arm/mach-imx/imx9/scmi/soc.c    |   9 +
 drivers/ddr/imx/imx9/Kconfig         |   8 +
 drivers/ddr/imx/phy/Kconfig          |   7 +
 7 files changed, 524 insertions(+), 3 deletions(-)
 create mode 100644 arch/arm/include/asm/mach-imx/qb.h
 create mode 100644 arch/arm/mach-imx/imx9/qb.c

diff --git a/arch/arm/include/asm/arch-imx9/ddr.h b/arch/arm/include/asm/arch-imx9/ddr.h
index a8e3f7354c7..d2afbe59ffb 100644
--- a/arch/arm/include/asm/arch-imx9/ddr.h
+++ b/arch/arm/include/asm/arch-imx9/ddr.h
@@ -1,6 +1,6 @@
 /* SPDX-License-Identifier: GPL-2.0+ */
 /*
- * Copyright 2022 NXP
+ * Copyright 2022-2025 NXP
  */
 
 #ifndef __ASM_ARCH_IMX8M_DDR_H
@@ -100,6 +100,56 @@ struct dram_timing_info {
 
 extern struct dram_timing_info dram_timing;
 
+#if IS_ENABLED(CONFIG_IMX95) || IS_ENABLED(CONFIG_IMX94) /* CONFIG_IMX95 || CONFIG_IMX94 */
+#if IS_ENABLED(CONFIG_IMX_SNPS_DDR_PHY_QB_GEN)
+/* Quick Boot related */
+#define DDRPHY_QB_CSR_SIZE	5168
+#define DDRPHY_QB_ACSM_SIZE	4 * 1024
+#define DDRPHY_QB_MSB_SIZE	0x200
+#define DDRPHY_QB_PSTATES	0
+#define DDRPHY_QB_PST_SIZE	DDRPHY_QB_PSTATES * 4 * 1024
+
+/**
+ * This structure needs to be aligned with the one in OEI.
+ */
+struct ddrphy_qb_state {
+	u32 crc;		  /* Used for ensuring integrity in DRAM */
+#define MAC_LENGTH              8 /* 256 bits, 32-bit aligned */
+	u32 mac[MAC_LENGTH];	  /* For 95A0/1 use mac[0] to keep CRC32 value */
+	u8 TrainedVREFCA_A0;
+	u8 TrainedVREFCA_A1;
+	u8 TrainedVREFCA_B0;
+	u8 TrainedVREFCA_B1;
+	u8 TrainedVREFDQ_A0;
+	u8 TrainedVREFDQ_A1;
+	u8 TrainedVREFDQ_B0;
+	u8 TrainedVREFDQ_B1;
+	u8 TrainedVREFDQU_A0;
+	u8 TrainedVREFDQU_A1;
+	u8 TrainedVREFDQU_B0;
+	u8 TrainedVREFDQU_B1;
+	u8 TrainedDRAMDFE_A0;
+	u8 TrainedDRAMDFE_A1;
+	u8 TrainedDRAMDFE_B0;
+	u8 TrainedDRAMDFE_B1;
+	u8 TrainedDRAMDCA_A0;
+	u8 TrainedDRAMDCA_A1;
+	u8 TrainedDRAMDCA_B0;
+	u8 TrainedDRAMDCA_B1;
+	u16 QBPllUPllProg0;
+	u16 QBPllUPllProg1;
+	u16 QBPllUPllProg2;
+	u16 QBPllUPllProg3;
+	u16 QBPllCtrl1;
+	u16 QBPllCtrl4;
+	u16 QBPllCtrl5;
+	u16 csr[DDRPHY_QB_CSR_SIZE];
+	u16 acsm[DDRPHY_QB_ACSM_SIZE];
+	u16 pst[DDRPHY_QB_PST_SIZE];
+};
+#endif /* #if IS_ENABLED(CONFIG_IMX_SNPS_DDR_PHY_QB_GEN)  */
+#endif /* #if IS_ENABLED(CONFIG_IMX95) || IS_ENABLED(CONFIG_IMX94) */
+
 void ddr_load_train_firmware(enum fw_type type);
 int ddr_init(struct dram_timing_info *timing_info);
 int ddr_cfg_phy(struct dram_timing_info *timing_info);
diff --git a/arch/arm/include/asm/mach-imx/qb.h b/arch/arm/include/asm/mach-imx/qb.h
new file mode 100644
index 00000000000..5efe68f0a60
--- /dev/null
+++ b/arch/arm/include/asm/mach-imx/qb.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright 2026 NXP
+ */
+
+#ifndef __IMX_QB_H__
+#define __IMX_QB_H__
+
+#include <stdbool.h>
+
+bool qb_check(void);
+int  qb(int qb_dev, int qb_bootdev, bool save);
+#endif
diff --git a/arch/arm/mach-imx/imx9/Makefile b/arch/arm/mach-imx/imx9/Makefile
index 53cc97c6b47..3018d128a36 100644
--- a/arch/arm/mach-imx/imx9/Makefile
+++ b/arch/arm/mach-imx/imx9/Makefile
@@ -1,6 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0+
 #
-# Copyright 2022 NXP
+# Copyright 2022,2026 NXP
 
 obj-y += lowlevel_init.o
 
@@ -12,4 +12,8 @@ endif
 
 ifneq ($(CONFIG_SPL_BUILD),y)
 obj-y += imx_bootaux.o
-endif
\ No newline at end of file
+endif
+
+ifeq ($(CONFIG_IMX_SNPS_DDR_PHY_QB_GEN),y)
+obj-y += qb.o
+endif
diff --git a/arch/arm/mach-imx/imx9/qb.c b/arch/arm/mach-imx/imx9/qb.c
new file mode 100644
index 00000000000..9987c57b16a
--- /dev/null
+++ b/arch/arm/mach-imx/imx9/qb.c
@@ -0,0 +1,430 @@
+// SPDX-License-Identifier: GPL-2.0+
+/**
+ * Copyright 2024-2026 NXP
+ */
+#include <dm/device-internal.h>
+#include <errno.h>
+#include <imx_container.h>
+#include <linux/bitfield.h>
+#include <mmc.h>
+#include <spi_flash.h>
+#include <spl.h>
+#include <stdlib.h>
+
+#include <asm/arch/ddr.h>
+#include <asm/mach-imx/boot_mode.h>
+#include <asm/mach-imx/sys_proto.h>
+
+#define QB_STATE_LOAD_SIZE    SZ_64K
+
+#define MMC_DEV		0
+#define QSPI_DEV	1
+#define NAND_DEV	2
+#define QSPI_NOR_DEV	3
+#define ROM_API_DEV	4
+#define RAM_DEV		5
+
+#if CONFIG_IS_ENABLED(DM_MMC) && CONFIG_IS_ENABLED(MMC_WRITE)
+#define QB_MMC_EN	1
+#endif /* DM_MMC && MMC_WRITE */
+#if CONFIG_IS_ENABLED(SPI)
+#define QB_SPI_EN	1
+#endif /* SPI */
+
+#define IMG_FLAGS_IMG_TYPE_MASK   0xFU
+#define IMG_FLAGS_IMG_TYPE(x)     FIELD_GET(IMG_FLAGS_IMG_TYPE_MASK, x)
+
+#define IMG_TYPE_DDR_TDATA_DUMMY  0xDU   /* dummy DDR training data image */
+
+/**
+ * Table used to implement half-byte CRC
+ * Polynomial: 0xEDB88320
+ */
+static const u32 p_table[] = {
+	0x00000000, 0x1db71064, 0x3b6e20c8, 0x26d930ac,
+	0x76dc4190, 0x6b6b51f4, 0x4db26158, 0x5005713c,
+	0xedb88320, 0xf00f9344, 0xd6d6a3e8, 0xcb61b38c,
+	0x9b64c2b0, 0x86d3d2d4, 0xa00ae278, 0xbdbdf21c,
+};
+
+/**
+ * Implement half-byte CRC algorithm
+ */
+static u32 qb_crc32(const void *addr, u32 len)
+{
+	u32 crc = ~0x00, idx, i, val;
+	const u8 *chr = (const u8 *)addr;
+
+	for (i = 0; i < len; i++, chr++) {
+		val = (u32)(*chr);
+
+		idx = (crc ^ (val >> 0)) & 0x0F;
+		crc = p_table[idx] ^ (crc >> 4);
+		idx = (crc ^ (val >> 4)) & 0x0F;
+		crc = p_table[idx] ^ (crc >> 4);
+	}
+
+	return ~crc;
+}
+
+bool qb_check(void)
+{
+	struct ddrphy_qb_state *qb_state;
+	u32 size, crc;
+
+	/**
+	 * Ensure CRC is not empty, the reason is that
+	 * the data is invalidated after first save run
+	 * or after it is overwritten.
+	 */
+	qb_state = (struct ddrphy_qb_state *)CONFIG_SAVED_QB_STATE_BASE;
+	size = sizeof(struct ddrphy_qb_state) - sizeof(qb_state->crc);
+	crc = qb_crc32(qb_state->mac, size);
+
+	if (!qb_state->crc || crc != qb_state->crc)
+		return false;
+
+	return true;
+}
+
+static unsigned long get_boot_device_offset(void *dev, int dev_type, bool bootdev)
+{
+	unsigned long offset = 0;
+	struct mmc *mmc;
+
+	switch (dev_type) {
+	case ROM_API_DEV:
+		offset = (unsigned long)dev;
+		break;
+	case MMC_DEV:
+		mmc = (struct mmc *)dev;
+
+		if (IS_SD(mmc) || mmc->part_config == MMCPART_NOAVAILABLE) {
+			offset = CONTAINER_HDR_MMCSD_OFFSET;
+		} else {
+			u8 part = EXT_CSD_EXTRACT_BOOT_PART(mmc->part_config);
+
+			if (part == EMMC_BOOT_PART_BOOT1 || part == EMMC_BOOT_PART_BOOT2)
+				offset = CONTAINER_HDR_EMMC_OFFSET;
+			else
+				offset = CONTAINER_HDR_MMCSD_OFFSET;
+		}
+		break;
+	case QSPI_DEV:
+		offset = CONTAINER_HDR_QSPI_OFFSET;
+		break;
+	case NAND_DEV:
+		offset = CONTAINER_HDR_NAND_OFFSET;
+		break;
+	case QSPI_NOR_DEV:
+		offset = CONTAINER_HDR_QSPI_OFFSET + 0x08000000;
+		break;
+	case RAM_DEV:
+		offset = (unsigned long)dev + CONTAINER_HDR_MMCSD_OFFSET;
+		break;
+	}
+
+	return offset;
+}
+
+static int parse_container(void *addr, u32 *qb_data_off)
+{
+	struct container_hdr *phdr;
+	struct boot_img_t *img_entry;
+	u8 i = 0;
+	u32 img_type, img_end;
+
+	phdr = (struct container_hdr *)addr;
+	if (phdr->tag != 0x87 || (phdr->version != 0x0 && phdr->version != 0x2))
+		return -1;
+
+	img_entry = (struct boot_img_t *)(addr + sizeof(struct container_hdr));
+	for (i = 0; i < phdr->num_images; i++) {
+		img_type = IMG_FLAGS_IMG_TYPE(img_entry->hab_flags);
+		if (img_type == IMG_TYPE_DDR_TDATA_DUMMY && img_entry->size == 0) {
+			/* Image entry pointing to DDR Training Data */
+			(*qb_data_off) = img_entry->offset;
+			return 0;
+		}
+
+		img_end = img_entry->offset + img_entry->size;
+		if (i + 1 < phdr->num_images) {
+			img_entry++;
+			if (img_end + QB_STATE_LOAD_SIZE == img_entry->offset) {
+				/* hole detected */
+				(*qb_data_off) = img_end;
+				return 0;
+			}
+		}
+	}
+
+	return -1;
+}
+
+static int get_dev_qbdata_offset(void *dev, int dev_type, unsigned long offset, u32 *qbdata_offset)
+{
+	int ret = 0;
+	u16 ctnr_hdr_align = CONTAINER_HDR_ALIGNMENT;
+	void *buf = kmalloc(ctnr_hdr_align, GFP_KERNEL);
+
+	if (!buf) {
+		printf("kmalloc buffer failed\n");
+		return -ENOMEM;
+	}
+
+	switch (dev_type) {
+#ifdef QB_MMC_EN
+	case MMC_DEV:
+		unsigned long count = 0;
+		struct mmc *mmc = (struct mmc *)dev;
+
+		count = blk_dread(mmc_get_blk_desc(mmc),
+				  offset / mmc->read_bl_len,
+				  ctnr_hdr_align / mmc->read_bl_len,
+				  buf);
+		if (count == 0) {
+			printf("Read container image from MMC/SD failed\n");
+			free(buf);
+			return -EIO;
+		}
+		break;
+#endif /* QB_MMC_EN */
+#ifdef QB_SPI_EN
+	case QSPI_DEV:
+		struct spi_flash *flash = (struct spi_flash *)dev;
+
+		ret = spi_flash_read(flash, offset,
+				     ctnr_hdr_align, buf);
+		if (ret) {
+			printf("Read container header from QSPI failed\n");
+			free(buf);
+			return -EIO;
+		}
+		break;
+#endif /* QB_SPI_EN */
+	case QSPI_NOR_DEV:
+	case RAM_DEV:
+		memcpy(buf, (const void *)offset, ctnr_hdr_align);
+		break;
+	default:
+		printf("Support for device %d not enabled\n", dev_type);
+		free(buf);
+		return -EIO;
+	}
+
+	ret = parse_container(buf, qbdata_offset);
+
+	free(buf);
+
+	return ret;
+}
+
+static int get_qbdata_offset(void *dev, int dev_type, u32 *qbdata_offset, bool bootdev)
+{
+	u32 offset = get_boot_device_offset(dev, dev_type, bootdev);
+	u16 ctnr_hdr_align = CONTAINER_HDR_ALIGNMENT;
+	u32 cont_offset;
+	int ret, i;
+
+	for (i = 0; i < 3; i++) {
+		cont_offset = offset + i * ctnr_hdr_align;
+		ret = get_dev_qbdata_offset(dev, dev_type, cont_offset, qbdata_offset);
+		if (ret == 0) {
+			(*qbdata_offset) += cont_offset;
+			break;
+		}
+	}
+
+	return ret;
+}
+
+#ifdef QB_MMC_EN
+static int mmc_get_device_index(u32 dev)
+{
+	switch (dev) {
+	case BOOT_DEVICE_MMC1:
+		return 0;
+	case BOOT_DEVICE_MMC2:
+	case BOOT_DEVICE_MMC2_2:
+		return 1;
+	}
+
+	return -ENODEV;
+}
+
+static int mmc_find_device(struct mmc **mmcp, int mmc_dev)
+{
+	int err;
+
+	err = mmc_init_device(mmc_dev);
+	if (err)
+		return err;
+
+	*mmcp = find_mmc_device(mmc_dev);
+
+	return (*mmcp) ? 0 : -ENODEV;
+}
+
+static int do_qb_mmc(int dev, bool save, bool is_bootdev)
+{
+	struct mmc *mmc;
+	int ret = 0, mmc_dev;
+	bool has_hw_part;
+	u8 orig_part, part;
+	u32 offset;
+	void *buf;
+
+	mmc_dev = mmc_get_device_index(dev);
+	if (mmc_dev < 0)
+		return mmc_dev;
+
+	ret = mmc_find_device(&mmc, mmc_dev);
+	if (ret)
+		return ret;
+
+	if (!mmc->has_init)
+		ret = mmc_init(mmc);
+
+	if (ret)
+		return ret;
+
+	has_hw_part = !IS_SD(mmc) && mmc->part_config != MMCPART_NOAVAILABLE;
+
+	if (has_hw_part) {
+		orig_part = mmc_get_blk_desc(mmc)->hwpart;
+		part = EXT_CSD_EXTRACT_BOOT_PART(mmc->part_config);
+
+		/* Select the partition */
+		ret = mmc_switch_part(mmc, part);
+		if (ret)
+			return ret;
+	}
+
+	ret = get_qbdata_offset(mmc, MMC_DEV, &offset, is_bootdev);
+	if (ret)
+		return ret;
+
+	if (save) {
+		/* QB data is stored in DDR -> can use it as buf */
+		buf = (void *)CONFIG_SAVED_QB_STATE_BASE;
+		ret = blk_dwrite(mmc_get_blk_desc(mmc),
+				 offset / mmc->write_bl_len,
+				 QB_STATE_LOAD_SIZE / mmc->write_bl_len,
+				 (const void *)buf);
+	} else {
+		/* erase */
+		ret = blk_derase(mmc_get_blk_desc(mmc),
+				 offset / mmc->write_bl_len,
+				 QB_STATE_LOAD_SIZE / mmc->write_bl_len);
+	}
+
+	ret = (ret > 0) ? 0 : -1;
+
+	/* Return to original partition */
+	if (has_hw_part)
+		ret |= mmc_switch_part(mmc, orig_part);
+
+	return ret;
+}
+#else
+static int do_qb_mmc(int dev, bool save, bool is_bootdev)
+{
+	printf("Please enable MMC and MMC_WRITE\n");
+
+	return -EOPNOTSUPP;
+}
+#endif /* QB_MMC_EN */
+
+#ifdef QB_SPI_EN
+static int spi_find_device(struct spi_flash **dev)
+{
+	unsigned int sf_bus = CONFIG_SF_DEFAULT_BUS;
+	unsigned int sf_cs = CONFIG_SF_DEFAULT_CS;
+	struct spi_flash *flash;
+	int ret = 0;
+
+	flash = spi_flash_probe(sf_bus, sf_cs,
+				CONFIG_SF_DEFAULT_SPEED,
+				CONFIG_SF_DEFAULT_MODE);
+
+	if (!flash) {
+		puts("SPI probe failed.\n");
+		return -ENODEV;
+	}
+
+	*dev = flash;
+
+	return ret;
+}
+
+static int do_qb_spi(int dev, bool save, bool is_bootdev)
+{
+	int ret = 0;
+	u32 offset;
+	void *buf;
+	struct spi_flash *flash;
+
+	ret = spi_find_device(&flash);
+	if (ret)
+		return -ENODEV;
+
+	ret = get_qbdata_offset(flash, QSPI_DEV, &offset, is_bootdev);
+	if (ret)
+		return ret;
+
+	ret = spi_flash_erase(flash, offset,
+			      QB_STATE_LOAD_SIZE);
+
+	if (!ret && save) {
+		/* QB data is stored in DDR -> can use it as buf */
+		buf = (void *)CONFIG_SAVED_QB_STATE_BASE;
+		ret = spi_flash_write(flash, offset,
+				      QB_STATE_LOAD_SIZE, buf);
+	}
+
+	return ret;
+}
+#else
+static int do_qb_spi(int dev, bool save, bool is_bootdev)
+{
+	printf("Please enable SPI\n");
+
+	return -EOPNOTSUPP;
+}
+#endif /* QB_SPI_EN */
+
+int qb(int qb_dev, int qb_bootdev, bool save)
+{
+	int ret = -1;
+
+	if (save && !qb_check())
+		return ret;
+
+	switch (qb_dev) {
+	case BOOT_DEVICE_MMC1:
+	case BOOT_DEVICE_MMC2:
+	case BOOT_DEVICE_MMC2_2:
+		ret = do_qb_mmc(qb_dev, save, !!(qb_dev == qb_bootdev));
+		break;
+	case BOOT_DEVICE_SPI:
+		ret = do_qb_spi(qb_dev, save, !!(qb_dev == qb_bootdev));
+		break;
+	default:
+		printf("Unsupported quickboot device\n");
+		break;
+	}
+
+	if (ret)
+		return ret;
+
+	if (!save)
+		return 0;
+
+	/**
+	 * invalidate qb_state mem so that at next boot
+	 * the check function will fail and save won't happen
+	 */
+	memset((void *)CONFIG_SAVED_QB_STATE_BASE, 0, sizeof(struct ddrphy_qb_state));
+
+	return 0;
+}
diff --git a/arch/arm/mach-imx/imx9/scmi/soc.c b/arch/arm/mach-imx/imx9/scmi/soc.c
index 17269ddd2fc..eb9bfe19a69 100644
--- a/arch/arm/mach-imx/imx9/scmi/soc.c
+++ b/arch/arm/mach-imx/imx9/scmi/soc.c
@@ -281,6 +281,15 @@ static struct mm_region imx9_mem_map[] = {
 			 PTE_BLOCK_NON_SHARE |
 			 PTE_BLOCK_PXN | PTE_BLOCK_UXN
 	}, {
+#if IS_ENABLED(CONFIG_IMX_SNPS_DDR_PHY_QB_GEN)
+		/* QB data */
+		.virt = CONFIG_SAVED_QB_STATE_BASE,
+		.phys = CONFIG_SAVED_QB_STATE_BASE,
+		.size = 0x200000UL,	/* 2M */
+		.attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) |
+			 PTE_BLOCK_OUTER_SHARE
+	}, {
+#endif /* CONFIG_IMX_SNPS_DDR_PHY_QB_GEN */
 		/* empty entry to split table entry 5 if needed when TEEs are used */
 		0,
 	}, {
diff --git a/drivers/ddr/imx/imx9/Kconfig b/drivers/ddr/imx/imx9/Kconfig
index 0a45340ffb6..7c244ddb5dd 100644
--- a/drivers/ddr/imx/imx9/Kconfig
+++ b/drivers/ddr/imx/imx9/Kconfig
@@ -29,4 +29,12 @@ config SAVED_DRAM_TIMING_BASE
 	  info into memory for low power use.
 	default 0x2051C000
 
+config SAVED_QB_STATE_BASE
+	hex "Define the base address for saved QuickBoot state"
+	depends on IMX_SNPS_DDR_PHY_QB_GEN
+	help
+	  Once DRAM is trained, need to save the dram related timing
+	  info into memory in order to be reachable from U-Boot.
+	default 0x8fe00000
+
 endmenu
diff --git a/drivers/ddr/imx/phy/Kconfig b/drivers/ddr/imx/phy/Kconfig
index d3e589b23c4..e8d0c005689 100644
--- a/drivers/ddr/imx/phy/Kconfig
+++ b/drivers/ddr/imx/phy/Kconfig
@@ -2,3 +2,10 @@ config IMX_SNPS_DDR_PHY
 	bool "i.MX Snopsys DDR PHY"
 	help
 	  Select the DDR PHY driver support on i.MX8M and i.MX9 SOC.
+
+config IMX_SNPS_DDR_PHY_QB_GEN
+	bool "i.MX Synopsys DDR PHY training data saving for QuickBoot mode"
+	depends on IMX94 || IMX95
+	help
+	  Select the DDR PHY training data saving for
+	  QuickBoot support on i.MX9 SOC.
-- 
2.43.0

Re: [PATCH v2 1/5] imx9: Add support for saving DDR training data to NVM

[Marek Vasut]

On 3/16/26 9:15 AM, Simona Toaca (OSS) wrote:

> diff --git a/arch/arm/include/asm/arch-imx9/ddr.h b/arch/arm/include/asm/arch-imx9/ddr.h
> index a8e3f7354c7..d2afbe59ffb 100644
> --- a/arch/arm/include/asm/arch-imx9/ddr.h
> +++ b/arch/arm/include/asm/arch-imx9/ddr.h
> @@ -1,6 +1,6 @@
>   /* SPDX-License-Identifier: GPL-2.0+ */
>   /*
> - * Copyright 2022 NXP
> + * Copyright 2022-2025 NXP

2026

>    */
>   
>   #ifndef __ASM_ARCH_IMX8M_DDR_H
> @@ -100,6 +100,56 @@ struct dram_timing_info {
>   
>   extern struct dram_timing_info dram_timing;
>   
> +#if IS_ENABLED(CONFIG_IMX95) || IS_ENABLED(CONFIG_IMX94) /* CONFIG_IMX95 || CONFIG_IMX94 */
> +#if IS_ENABLED(CONFIG_IMX_SNPS_DDR_PHY_QB_GEN)

These are only macros and structure definitions, do they need to be 
conditionally compiled ?

> +/* Quick Boot related */
> +#define DDRPHY_QB_CSR_SIZE	5168
> +#define DDRPHY_QB_ACSM_SIZE	4 * 1024

(4 * 1024) to avoid side effects

> +#define DDRPHY_QB_MSB_SIZE	0x200
> +#define DDRPHY_QB_PSTATES	0
> +#define DDRPHY_QB_PST_SIZE	DDRPHY_QB_PSTATES * 4 * 1024

Add ( )

> +/**
> + * This structure needs to be aligned with the one in OEI.
> + */
> +struct ddrphy_qb_state {
> +	u32 crc;		  /* Used for ensuring integrity in DRAM */
> +#define MAC_LENGTH              8 /* 256 bits, 32-bit aligned */
> +	u32 mac[MAC_LENGTH];	  /* For 95A0/1 use mac[0] to keep CRC32 value */
> +	u8 TrainedVREFCA_A0;

Avoid camel case please

> +	u8 TrainedVREFCA_A1;
> +	u8 TrainedVREFCA_B0;
> +	u8 TrainedVREFCA_B1;
> +	u8 TrainedVREFDQ_A0;

[...]

> diff --git a/arch/arm/include/asm/mach-imx/qb.h b/arch/arm/include/asm/mach-imx/qb.h
> new file mode 100644
> index 00000000000..5efe68f0a60
> --- /dev/null
> +++ b/arch/arm/include/asm/mach-imx/qb.h

[...]

> +bool qb_check(void);
> +int  qb(int qb_dev, int qb_bootdev, bool save);

One space after 'int' is enough.

Also, please rename the function with some more descriptive name .

[...]

> diff --git a/arch/arm/mach-imx/imx9/qb.c b/arch/arm/mach-imx/imx9/qb.c
> new file mode 100644
> index 00000000000..9987c57b16a
> --- /dev/null
> +++ b/arch/arm/mach-imx/imx9/qb.c

[...]

> +#if CONFIG_IS_ENABLED(DM_MMC) && CONFIG_IS_ENABLED(MMC_WRITE)
> +#define QB_MMC_EN	1

Place these conditionals directly into the code to maintain code 
coverage, i.e.:

if (CONFIG_IS_ENABLED(DM_MMC) && CONFIG_IS_ENABLED(MMC_WRITE)) {
  ... do_stuff();
}

> +#endif /* DM_MMC && MMC_WRITE */
> +#if CONFIG_IS_ENABLED(SPI)
> +#define QB_SPI_EN	1
> +#endif /* SPI */
> +
> +#define IMG_FLAGS_IMG_TYPE_MASK   0xFU
> +#define IMG_FLAGS_IMG_TYPE(x)     FIELD_GET(IMG_FLAGS_IMG_TYPE_MASK, x)

Add ( ) around x in the macro to avoid side effects , i.e. (x)

> +#define IMG_TYPE_DDR_TDATA_DUMMY  0xDU   /* dummy DDR training data image */

0xd , drop the U

> +/**
> + * Table used to implement half-byte CRC
> + * Polynomial: 0xEDB88320
> + */
> +static const u32 p_table[] = {
> +	0x00000000, 0x1db71064, 0x3b6e20c8, 0x26d930ac,
> +	0x76dc4190, 0x6b6b51f4, 0x4db26158, 0x5005713c,
> +	0xedb88320, 0xf00f9344, 0xd6d6a3e8, 0xcb61b38c,
> +	0x9b64c2b0, 0x86d3d2d4, 0xa00ae278, 0xbdbdf21c,
> +};
> +
> +/**
> + * Implement half-byte CRC algorithm
> + */
> +static u32 qb_crc32(const void *addr, u32 len)
> +{
> +	u32 crc = ~0x00, idx, i, val;
> +	const u8 *chr = (const u8 *)addr;
> +
> +	for (i = 0; i < len; i++, chr++) {
> +		val = (u32)(*chr);
> +
> +		idx = (crc ^ (val >> 0)) & 0x0F;
> +		crc = p_table[idx] ^ (crc >> 4);
> +		idx = (crc ^ (val >> 4)) & 0x0F;
> +		crc = p_table[idx] ^ (crc >> 4);
> +	}
> +
> +	return ~crc;
> +}

Why not simply call crc32() U-Boot function ?

> +bool qb_check(void)
> +{
> +	struct ddrphy_qb_state *qb_state;
> +	u32 size, crc;
> +
> +	/**
> +	 * Ensure CRC is not empty, the reason is that
> +	 * the data is invalidated after first save run
> +	 * or after it is overwritten.
> +	 */
> +	qb_state = (struct ddrphy_qb_state *)CONFIG_SAVED_QB_STATE_BASE;
> +	size = sizeof(struct ddrphy_qb_state) - sizeof(qb_state->crc);
> +	crc = qb_crc32(qb_state->mac, size);

"
   size = sizeof(struct ddrphy_qb_state) - MAC_LENGTH * sizeof(u32);
   crc = crc32(0, &qb_state->TrainedVREFCA_A0, size);
"

Does this work ?

> +	if (!qb_state->crc || crc != qb_state->crc)
> +		return false;
> +
> +	return true;
> +}
> +
> +static unsigned long get_boot_device_offset(void *dev, int dev_type, bool bootdev)
> +{
> +	unsigned long offset = 0;
> +	struct mmc *mmc;
> +
> +	switch (dev_type) {
> +	case ROM_API_DEV:
> +		offset = (unsigned long)dev;

Please get rid of this void * casting horribleness.

> +		break;
> +	case MMC_DEV:
> +		mmc = (struct mmc *)dev;

Also here.

[...]

> +static int parse_container(void *addr, u32 *qb_data_off)
> +{
> +	struct container_hdr *phdr;
> +	struct boot_img_t *img_entry;
> +	u8 i = 0;

int i;

> +	u32 img_type, img_end;
> +
> +	phdr = (struct container_hdr *)addr;
> +	if (phdr->tag != 0x87 || (phdr->version != 0x0 && phdr->version != 0x2))
> +		return -1;

Use errno.h return codes.

> +	img_entry = (struct boot_img_t *)(addr + sizeof(struct container_hdr));
> +	for (i = 0; i < phdr->num_images; i++) {
> +		img_type = IMG_FLAGS_IMG_TYPE(img_entry->hab_flags);
> +		if (img_type == IMG_TYPE_DDR_TDATA_DUMMY && img_entry->size == 0) {
> +			/* Image entry pointing to DDR Training Data */
> +			(*qb_data_off) = img_entry->offset;

() are not needed

> +			return 0;
> +		}
> +
> +		img_end = img_entry->offset + img_entry->size;
> +		if (i + 1 < phdr->num_images) {
> +			img_entry++;
> +			if (img_end + QB_STATE_LOAD_SIZE == img_entry->offset) {
> +				/* hole detected */
> +				(*qb_data_off) = img_end;

() are not needed
> +				return 0;
> +			}
> +		}
> +	}
> +
> +	return -1;

errno.h

> +}
> +
> +static int get_dev_qbdata_offset(void *dev, int dev_type, unsigned long offset, u32 *qbdata_offset)
> +{
> +	int ret = 0;
> +	u16 ctnr_hdr_align = CONTAINER_HDR_ALIGNMENT;
> +	void *buf = kmalloc(ctnr_hdr_align, GFP_KERNEL);
> +
> +	if (!buf) {
> +		printf("kmalloc buffer failed\n");
> +		return -ENOMEM;
> +	}
> +
> +	switch (dev_type) {
> +#ifdef QB_MMC_EN

Remove this ...

> +	case MMC_DEV:

... and use this:

if (!(CONFIG_IS_ENABLED(DM_MMC) && CONFIG_IS_ENABLED(MMC_WRITE)))
   return -EOPNOTSUPP;

> +		unsigned long count = 0;
> +		struct mmc *mmc = (struct mmc *)dev;

The void * casting has to go ...

> +		count = blk_dread(mmc_get_blk_desc(mmc),
> +				  offset / mmc->read_bl_len,
> +				  ctnr_hdr_align / mmc->read_bl_len,
> +				  buf);
> +		if (count == 0) {
> +			printf("Read container image from MMC/SD failed\n");
> +			free(buf);
> +			return -EIO;
> +		}
> +		break;
> +#endif /* QB_MMC_EN */
> +#ifdef QB_SPI_EN
> +	case QSPI_DEV:
> +		struct spi_flash *flash = (struct spi_flash *)dev;
> +
> +		ret = spi_flash_read(flash, offset,
> +				     ctnr_hdr_align, buf);
> +		if (ret) {
> +			printf("Read container header from QSPI failed\n");
> +			free(buf);
> +			return -EIO;
> +		}
> +		break;
> +#endif /* QB_SPI_EN */
> +	case QSPI_NOR_DEV:
> +	case RAM_DEV:
> +		memcpy(buf, (const void *)offset, ctnr_hdr_align);
> +		break;
> +	default:
> +		printf("Support for device %d not enabled\n", dev_type);
> +		free(buf);
> +		return -EIO;
> +	}
> +
> +	ret = parse_container(buf, qbdata_offset);

Ideally, prefix all those functions with some universal prefix, e.g. 
qb_...(), it makes it easier to look them up in disassembly dump.

> +
> +	free(buf);
> +
> +	return ret;
> +}
> +
> +static int get_qbdata_offset(void *dev, int dev_type, u32 *qbdata_offset, bool bootdev)
> +{
> +	u32 offset = get_boot_device_offset(dev, dev_type, bootdev);
> +	u16 ctnr_hdr_align = CONTAINER_HDR_ALIGNMENT;
> +	u32 cont_offset;
> +	int ret, i;
> +
> +	for (i = 0; i < 3; i++) {
> +		cont_offset = offset + i * ctnr_hdr_align;
> +		ret = get_dev_qbdata_offset(dev, dev_type, cont_offset, qbdata_offset);
> +		if (ret == 0) {
> +			(*qbdata_offset) += cont_offset;
> +			break;
> +		}
> +	}
> +
> +	return ret;
> +}
> +
> +#ifdef QB_MMC_EN

Use if (!(CONFIG_IS_ENABLED(DM_MMC) && CONFIG_IS_ENABLED(MMC_WRITE))) in 
code and you won't need this ifdeffery.

> +static int mmc_get_device_index(u32 dev)
> +{
> +	switch (dev) {
> +	case BOOT_DEVICE_MMC1:
> +		return 0;
> +	case BOOT_DEVICE_MMC2:
> +	case BOOT_DEVICE_MMC2_2:
> +		return 1;
> +	}
> +
> +	return -ENODEV;
> +}
> +
> +static int mmc_find_device(struct mmc **mmcp, int mmc_dev)
> +{
> +	int err;

Stop mixing err and ret, pick one or the other and use it consistently 
in the whole file as a return value variable;

> +	err = mmc_init_device(mmc_dev);
> +	if (err)
> +		return err;
> +
> +	*mmcp = find_mmc_device(mmc_dev);
> +
> +	return (*mmcp) ? 0 : -ENODEV;

() are unnecessary

> +}
> +
> +static int do_qb_mmc(int dev, bool save, bool is_bootdev)
> +{
> +	struct mmc *mmc;
> +	int ret = 0, mmc_dev;
> +	bool has_hw_part;
> +	u8 orig_part, part;
> +	u32 offset;
> +	void *buf;
> +
> +	mmc_dev = mmc_get_device_index(dev);
> +	if (mmc_dev < 0)
> +		return mmc_dev;
> +
> +	ret = mmc_find_device(&mmc, mmc_dev);
> +	if (ret)
> +		return ret;
> +
> +	if (!mmc->has_init)
> +		ret = mmc_init(mmc);

mmc_init() already handles the mmc->has_init conditional, drop it.

> +	if (ret)
> +		return ret;
> +
> +	has_hw_part = !IS_SD(mmc) && mmc->part_config != MMCPART_NOAVAILABLE;
> +
> +	if (has_hw_part) {
> +		orig_part = mmc_get_blk_desc(mmc)->hwpart;
> +		part = EXT_CSD_EXTRACT_BOOT_PART(mmc->part_config);
> +
> +		/* Select the partition */
> +		ret = mmc_switch_part(mmc, part);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	ret = get_qbdata_offset(mmc, MMC_DEV, &offset, is_bootdev);
> +	if (ret)
> +		return ret;
> +
> +	if (save) {
> +		/* QB data is stored in DDR -> can use it as buf */
> +		buf = (void *)CONFIG_SAVED_QB_STATE_BASE;
> +		ret = blk_dwrite(mmc_get_blk_desc(mmc),
> +				 offset / mmc->write_bl_len,
> +				 QB_STATE_LOAD_SIZE / mmc->write_bl_len,
> +				 (const void *)buf);
> +	} else {
> +		/* erase */
> +		ret = blk_derase(mmc_get_blk_desc(mmc),
> +				 offset / mmc->write_bl_len,
> +				 QB_STATE_LOAD_SIZE / mmc->write_bl_len);
> +	}
> +
> +	ret = (ret > 0) ? 0 : -1;

Use CMD_RET_SUCCESS and co.

> +	/* Return to original partition */
> +	if (has_hw_part)
> +		ret |= mmc_switch_part(mmc, orig_part);

$ret is signed integer, you cannot do bit operations on it.

> +	return ret;
> +}
> +#else
> +static int do_qb_mmc(int dev, bool save, bool is_bootdev)
> +{
> +	printf("Please enable MMC and MMC_WRITE\n");

Use if (!(CONFIG_IS_ENABLED(DM_MMC) && CONFIG_IS_ENABLED(MMC_WRITE))) 
and avoid the ifdeffery.

> +	return -EOPNOTSUPP;
> +}
> +#endif /* QB_MMC_EN */
> +
> +#ifdef QB_SPI_EN

Ifdeffery, code coverage, if (CONFIG...), same as above.

> +static int spi_find_device(struct spi_flash **dev)
> +{
> +	unsigned int sf_bus = CONFIG_SF_DEFAULT_BUS;
> +	unsigned int sf_cs = CONFIG_SF_DEFAULT_CS;
> +	struct spi_flash *flash;
> +	int ret = 0;
> +
> +	flash = spi_flash_probe(sf_bus, sf_cs,
> +				CONFIG_SF_DEFAULT_SPEED,
> +				CONFIG_SF_DEFAULT_MODE);
> +
> +	if (!flash) {
> +		puts("SPI probe failed.\n");
> +		return -ENODEV;
> +	}
> +
> +	*dev = flash;
> +
> +	return ret;
> +}
> +
> +static int do_qb_spi(int dev, bool save, bool is_bootdev)
> +{
> +	int ret = 0;
> +	u32 offset;
> +	void *buf;
> +	struct spi_flash *flash;
> +
> +	ret = spi_find_device(&flash);
> +	if (ret)
> +		return -ENODEV;
> +
> +	ret = get_qbdata_offset(flash, QSPI_DEV, &offset, is_bootdev);
> +	if (ret)
> +		return ret;
> +
> +	ret = spi_flash_erase(flash, offset,
> +			      QB_STATE_LOAD_SIZE);

if (ret)
   return ret;

if (!save)
   return 0;

Reduce indent ...

> +	if (!ret && save) {
> +		/* QB data is stored in DDR -> can use it as buf */
> +		buf = (void *)CONFIG_SAVED_QB_STATE_BASE;
> +		ret = spi_flash_write(flash, offset,
> +				      QB_STATE_LOAD_SIZE, buf);
> +	}
> +
> +	return ret;
> +}
> +#else
> +static int do_qb_spi(int dev, bool save, bool is_bootdev)
> +{
> +	printf("Please enable SPI\n");
> +
> +	return -EOPNOTSUPP;
> +}
> +#endif /* QB_SPI_EN */
> +
> +int qb(int qb_dev, int qb_bootdev, bool save)
> +{
> +	int ret = -1;

Use errno.h return values.

> +	if (save && !qb_check())
> +		return ret;
> +
> +	switch (qb_dev) {
> +	case BOOT_DEVICE_MMC1:
> +	case BOOT_DEVICE_MMC2:
> +	case BOOT_DEVICE_MMC2_2:
> +		ret = do_qb_mmc(qb_dev, save, !!(qb_dev == qb_bootdev));
> +		break;
> +	case BOOT_DEVICE_SPI:
> +		ret = do_qb_spi(qb_dev, save, !!(qb_dev == qb_bootdev));
> +		break;
> +	default:
> +		printf("Unsupported quickboot device\n");
> +		break;
> +	}
> +
> +	if (ret)
> +		return ret;
> +
> +	if (!save)
> +		return 0;
> +
> +	/**
> +	 * invalidate qb_state mem so that at next boot
> +	 * the check function will fail and save won't happen
> +	 */
> +	memset((void *)CONFIG_SAVED_QB_STATE_BASE, 0, sizeof(struct ddrphy_qb_state));
> +
> +	return 0;
> +}
> diff --git a/arch/arm/mach-imx/imx9/scmi/soc.c b/arch/arm/mach-imx/imx9/scmi/soc.c
> index 17269ddd2fc..eb9bfe19a69 100644
> --- a/arch/arm/mach-imx/imx9/scmi/soc.c
> +++ b/arch/arm/mach-imx/imx9/scmi/soc.c
> @@ -281,6 +281,15 @@ static struct mm_region imx9_mem_map[] = {
>   			 PTE_BLOCK_NON_SHARE |
>   			 PTE_BLOCK_PXN | PTE_BLOCK_UXN
>   	}, {
> +#if IS_ENABLED(CONFIG_IMX_SNPS_DDR_PHY_QB_GEN)
> +		/* QB data */

Is the ifdeffery necessary ?

Where are the QB data located in this case, DRAM or SRAM ?

> +		.virt = CONFIG_SAVED_QB_STATE_BASE,
> +		.phys = CONFIG_SAVED_QB_STATE_BASE,
> +		.size = 0x200000UL,	/* 2M */
> +		.attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) |
> +			 PTE_BLOCK_OUTER_SHARE
> +	}, {
> +#endif /* CONFIG_IMX_SNPS_DDR_PHY_QB_GEN */
>   		/* empty entry to split table entry 5 if needed when TEEs are used */
>   		0,
>   	}, {
> diff --git a/drivers/ddr/imx/imx9/Kconfig b/drivers/ddr/imx/imx9/Kconfig
> index 0a45340ffb6..7c244ddb5dd 100644
> --- a/drivers/ddr/imx/imx9/Kconfig
> +++ b/drivers/ddr/imx/imx9/Kconfig
> @@ -29,4 +29,12 @@ config SAVED_DRAM_TIMING_BASE
>   	  info into memory for low power use.
>   	default 0x2051C000

Please fix ^ that one in separate patch, default goes after depends.

>   
> +config SAVED_QB_STATE_BASE

IMX_SNPS_DDR_PHY_QB_SAVED_STATE_ADDRESS ... to make the prefix of this 
Kconfig symbol aligned with IMX_SNPS_DDR_PHY_QB_GEN below .

> +	hex "Define the base address for saved QuickBoot state"
> +	depends on IMX_SNPS_DDR_PHY_QB_GEN

"default" goes here.

> +	help
> +	  Once DRAM is trained, need to save the dram related timing
> +	  info into memory in order to be reachable from U-Boot.
> +	default 0x8fe00000
> +
>   endmenu
> diff --git a/drivers/ddr/imx/phy/Kconfig b/drivers/ddr/imx/phy/Kconfig
> index d3e589b23c4..e8d0c005689 100644
> --- a/drivers/ddr/imx/phy/Kconfig
> +++ b/drivers/ddr/imx/phy/Kconfig
> @@ -2,3 +2,10 @@ config IMX_SNPS_DDR_PHY
>   	bool "i.MX Snopsys DDR PHY"
>   	help
>   	  Select the DDR PHY driver support on i.MX8M and i.MX9 SOC.
> +
> +config IMX_SNPS_DDR_PHY_QB_GEN
> +	bool "i.MX Synopsys DDR PHY training data saving for QuickBoot mode"
> +	depends on IMX94 || IMX95
> +	help
> +	  Select the DDR PHY training data saving for
> +	  QuickBoot support on i.MX9 SOC.

Re: [PATCH v2 1/5] imx9: Add support for saving DDR training data to NVM

[Simona Toaca]

Hi Marek,

On Mon, Mar 16, 2026 at 01:13:02PM +0100, Marek Vasut wrote:
> On 3/16/26 9:15 AM, Simona Toaca (OSS) wrote:
> 
> > diff --git a/arch/arm/include/asm/arch-imx9/ddr.h b/arch/arm/include/asm/arch-imx9/ddr.h
> > index a8e3f7354c7..d2afbe59ffb 100644
> > --- a/arch/arm/include/asm/arch-imx9/ddr.h
> > +++ b/arch/arm/include/asm/arch-imx9/ddr.h
> > @@ -1,6 +1,6 @@
> >   /* SPDX-License-Identifier: GPL-2.0+ */
> >   /*
> > - * Copyright 2022 NXP
> > + * Copyright 2022-2025 NXP
> 
> 2026
> 
> >    */
> >   #ifndef __ASM_ARCH_IMX8M_DDR_H
> > @@ -100,6 +100,56 @@ struct dram_timing_info {
> >   extern struct dram_timing_info dram_timing;
> > +#if IS_ENABLED(CONFIG_IMX95) || IS_ENABLED(CONFIG_IMX94) /* CONFIG_IMX95 || CONFIG_IMX94 */
> > +#if IS_ENABLED(CONFIG_IMX_SNPS_DDR_PHY_QB_GEN)
> 
> These are only macros and structure definitions, do they need to be
> conditionally compiled ?
> 

I agree that this might be redundant, especially since IMX_SNPS_DDR_PHY_QB_GEN
depends on IMX94 || IMX95. I would leave the IMX_SNPS_DDR_PHY_QB_GEN
ifdef so that it is clear in which context this structure is used,
if it is also ok with you. If not, I will remove it.

> > +/* Quick Boot related */
> > +#define DDRPHY_QB_CSR_SIZE	5168
> > +#define DDRPHY_QB_ACSM_SIZE	4 * 1024
> 
> (4 * 1024) to avoid side effects
> 
> > +#define DDRPHY_QB_MSB_SIZE	0x200
> > +#define DDRPHY_QB_PSTATES	0
> > +#define DDRPHY_QB_PST_SIZE	DDRPHY_QB_PSTATES * 4 * 1024
> 
> Add ( )

Should I add () to all the numbers so it is consistent, or
just to the macros that contain computations?

[ ... ]

> > +static int do_qb_mmc(int dev, bool save, bool is_bootdev)
> > +{
> > +	struct mmc *mmc;
> > +	int ret = 0, mmc_dev;
> > +	bool has_hw_part;
> > +	u8 orig_part, part;
> > +	u32 offset;
> > +	void *buf;
> > +
> > +	mmc_dev = mmc_get_device_index(dev);
> > +	if (mmc_dev < 0)
> > +		return mmc_dev;
> > +
> > +	ret = mmc_find_device(&mmc, mmc_dev);
> > +	if (ret)
> > +		return ret;
> > +
> > +	if (!mmc->has_init)
> > +		ret = mmc_init(mmc);
> 
> mmc_init() already handles the mmc->has_init conditional, drop it.
> 
> > +	if (ret)
> > +		return ret;
> > +
> > +	has_hw_part = !IS_SD(mmc) && mmc->part_config != MMCPART_NOAVAILABLE;
> > +
> > +	if (has_hw_part) {
> > +		orig_part = mmc_get_blk_desc(mmc)->hwpart;
> > +		part = EXT_CSD_EXTRACT_BOOT_PART(mmc->part_config);
> > +
> > +		/* Select the partition */
> > +		ret = mmc_switch_part(mmc, part);
> > +		if (ret)
> > +			return ret;
> > +	}
> > +
> > +	ret = get_qbdata_offset(mmc, MMC_DEV, &offset, is_bootdev);
> > +	if (ret)
> > +		return ret;
> > +
> > +	if (save) {
> > +		/* QB data is stored in DDR -> can use it as buf */
> > +		buf = (void *)CONFIG_SAVED_QB_STATE_BASE;
> > +		ret = blk_dwrite(mmc_get_blk_desc(mmc),
> > +				 offset / mmc->write_bl_len,
> > +				 QB_STATE_LOAD_SIZE / mmc->write_bl_len,
> > +				 (const void *)buf);
> > +	} else {
> > +		/* erase */
> > +		ret = blk_derase(mmc_get_blk_desc(mmc),
> > +				 offset / mmc->write_bl_len,
> > +				 QB_STATE_LOAD_SIZE / mmc->write_bl_len);
> > +	}
> > +
> > +	ret = (ret > 0) ? 0 : -1;
> 
> Use CMD_RET_SUCCESS and co.
> 

I am using these enum values in cmd_qb. However, here I would have
do this 'if' at every return statement, since the other methods
return 0 or an errno code. Should I still change this?

> > +	/* Return to original partition */
> > +	if (has_hw_part)
> > +		ret |= mmc_switch_part(mmc, orig_part);

[ ... ]

> > diff --git a/arch/arm/mach-imx/imx9/scmi/soc.c b/arch/arm/mach-imx/imx9/scmi/soc.c
> > index 17269ddd2fc..eb9bfe19a69 100644
> > --- a/arch/arm/mach-imx/imx9/scmi/soc.c
> > +++ b/arch/arm/mach-imx/imx9/scmi/soc.c
> > @@ -281,6 +281,15 @@ static struct mm_region imx9_mem_map[] = {
> >   			 PTE_BLOCK_NON_SHARE |
> >   			 PTE_BLOCK_PXN | PTE_BLOCK_UXN
> >   	}, {
> > +#if IS_ENABLED(CONFIG_IMX_SNPS_DDR_PHY_QB_GEN)
> > +		/* QB data */
> 
> Is the ifdeffery necessary ?
> 
> Where are the QB data located in this case, DRAM or SRAM ?
> 

It is necessary because CONFIG_SAVED_QB_STATE_BASE is only
defined when CONFIG_IMX_SNPS_DDR_PHY_QB_GEN is enabled. So it
throws a compilation error without this. And this is DRAM.

> > +		.virt = CONFIG_SAVED_QB_STATE_BASE,
> > +		.phys = CONFIG_SAVED_QB_STATE_BASE,
> > +		.size = 0x200000UL,	/* 2M */
> > +		.attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) |
> > +			 PTE_BLOCK_OUTER_SHARE
> > +	}, {
> > +#endif /* CONFIG_IMX_SNPS_DDR_PHY_QB_GEN */
> >   		/* empty entry to split table entry 5 if needed when TEEs are used */
> >   		0,
> >   	}, {
> > diff --git a/drivers/ddr/imx/imx9/Kconfig b/drivers/ddr/imx/imx9/Kconfig
> > index 0a45340ffb6..7c244ddb5dd 100644
> > --- a/drivers/ddr/imx/imx9/Kconfig
> > +++ b/drivers/ddr/imx/imx9/Kconfig
> > @@ -29,4 +29,12 @@ config SAVED_DRAM_TIMING_BASE
> >   	  info into memory for low power use.
> >   	default 0x2051C000
> 
> Please fix ^ that one in separate patch, default goes after depends.
> 

Yes, will do.

> > +config SAVED_QB_STATE_BASE
> 
> IMX_SNPS_DDR_PHY_QB_SAVED_STATE_ADDRESS ... to make the prefix of this
> Kconfig symbol aligned with IMX_SNPS_DDR_PHY_QB_GEN below .
> 
> > +	hex "Define the base address for saved QuickBoot state"
> > +	depends on IMX_SNPS_DDR_PHY_QB_GEN
> 
> "default" goes here.
> 
> > +	help
> > +	  Once DRAM is trained, need to save the dram related timing
> > +	  info into memory in order to be reachable from U-Boot.
> > +	default 0x8fe00000
> > +
> >   endmenu
> > diff --git a/drivers/ddr/imx/phy/Kconfig b/drivers/ddr/imx/phy/Kconfig
> > index d3e589b23c4..e8d0c005689 100644
> > --- a/drivers/ddr/imx/phy/Kconfig
> > +++ b/drivers/ddr/imx/phy/Kconfig
> > @@ -2,3 +2,10 @@ config IMX_SNPS_DDR_PHY
> >   	bool "i.MX Snopsys DDR PHY"
> >   	help
> >   	  Select the DDR PHY driver support on i.MX8M and i.MX9 SOC.
> > +
> > +config IMX_SNPS_DDR_PHY_QB_GEN
> > +	bool "i.MX Synopsys DDR PHY training data saving for QuickBoot mode"
> > +	depends on IMX94 || IMX95
> > +	help
> > +	  Select the DDR PHY training data saving for
> > +	  QuickBoot support on i.MX9 SOC.

Thank you. Will fix these issues in the next version.

Simona

Re: [PATCH v2 1/5] imx9: Add support for saving DDR training data to NVM

[Marek Vasut]

On 3/17/26 1:36 PM, Simona Toaca wrote:

Hi,

>>> @@ -100,6 +100,56 @@ struct dram_timing_info {
>>>    extern struct dram_timing_info dram_timing;
>>> +#if IS_ENABLED(CONFIG_IMX95) || IS_ENABLED(CONFIG_IMX94) /* CONFIG_IMX95 || CONFIG_IMX94 */
>>> +#if IS_ENABLED(CONFIG_IMX_SNPS_DDR_PHY_QB_GEN)
>>
>> These are only macros and structure definitions, do they need to be
>> conditionally compiled ?
>>
> 
> I agree that this might be redundant, especially since IMX_SNPS_DDR_PHY_QB_GEN
> depends on IMX94 || IMX95. I would leave the IMX_SNPS_DDR_PHY_QB_GEN
> ifdef so that it is clear in which context this structure is used,
> if it is also ok with you. If not, I will remove it.

Remove it, it is superfluous. If you need to denote context, add code 
comment.

>>> +/* Quick Boot related */
>>> +#define DDRPHY_QB_CSR_SIZE	5168
>>> +#define DDRPHY_QB_ACSM_SIZE	4 * 1024
>>
>> (4 * 1024) to avoid side effects
>>
>>> +#define DDRPHY_QB_MSB_SIZE	0x200
>>> +#define DDRPHY_QB_PSTATES	0
>>> +#define DDRPHY_QB_PST_SIZE	DDRPHY_QB_PSTATES * 4 * 1024
>>
>> Add ( )
> 
> Should I add () to all the numbers so it is consistent, or
> just to the macros that contain computations?

The ones which contain more than plain integer and can have side effect 
if used in the wrong place.

> [ ... ]
> 
>>> +static int do_qb_mmc(int dev, bool save, bool is_bootdev)
>>> +{
>>> +	struct mmc *mmc;
>>> +	int ret = 0, mmc_dev;
>>> +	bool has_hw_part;
>>> +	u8 orig_part, part;
>>> +	u32 offset;
>>> +	void *buf;
>>> +
>>> +	mmc_dev = mmc_get_device_index(dev);
>>> +	if (mmc_dev < 0)
>>> +		return mmc_dev;
>>> +
>>> +	ret = mmc_find_device(&mmc, mmc_dev);
>>> +	if (ret)
>>> +		return ret;
>>> +
>>> +	if (!mmc->has_init)
>>> +		ret = mmc_init(mmc);
>>
>> mmc_init() already handles the mmc->has_init conditional, drop it.
>>
>>> +	if (ret)
>>> +		return ret;
>>> +
>>> +	has_hw_part = !IS_SD(mmc) && mmc->part_config != MMCPART_NOAVAILABLE;
>>> +
>>> +	if (has_hw_part) {
>>> +		orig_part = mmc_get_blk_desc(mmc)->hwpart;
>>> +		part = EXT_CSD_EXTRACT_BOOT_PART(mmc->part_config);
>>> +
>>> +		/* Select the partition */
>>> +		ret = mmc_switch_part(mmc, part);
>>> +		if (ret)
>>> +			return ret;
>>> +	}
>>> +
>>> +	ret = get_qbdata_offset(mmc, MMC_DEV, &offset, is_bootdev);
>>> +	if (ret)
>>> +		return ret;
>>> +
>>> +	if (save) {
>>> +		/* QB data is stored in DDR -> can use it as buf */
>>> +		buf = (void *)CONFIG_SAVED_QB_STATE_BASE;
>>> +		ret = blk_dwrite(mmc_get_blk_desc(mmc),
>>> +				 offset / mmc->write_bl_len,
>>> +				 QB_STATE_LOAD_SIZE / mmc->write_bl_len,
>>> +				 (const void *)buf);
>>> +	} else {
>>> +		/* erase */
>>> +		ret = blk_derase(mmc_get_blk_desc(mmc),
>>> +				 offset / mmc->write_bl_len,
>>> +				 QB_STATE_LOAD_SIZE / mmc->write_bl_len);
>>> +	}
>>> +
>>> +	ret = (ret > 0) ? 0 : -1;
>>
>> Use CMD_RET_SUCCESS and co.
>>
> 
> I am using these enum values in cmd_qb. However, here I would have
> do this 'if' at every return statement, since the other methods
> return 0 or an errno code. Should I still change this?

If this function does not implement a command, then it should be renamed 
to something less confusing.

>>> +	/* Return to original partition */
>>> +	if (has_hw_part)
>>> +		ret |= mmc_switch_part(mmc, orig_part);
> 
> [ ... ]
> 
>>> diff --git a/arch/arm/mach-imx/imx9/scmi/soc.c b/arch/arm/mach-imx/imx9/scmi/soc.c
>>> index 17269ddd2fc..eb9bfe19a69 100644
>>> --- a/arch/arm/mach-imx/imx9/scmi/soc.c
>>> +++ b/arch/arm/mach-imx/imx9/scmi/soc.c
>>> @@ -281,6 +281,15 @@ static struct mm_region imx9_mem_map[] = {
>>>    			 PTE_BLOCK_NON_SHARE |
>>>    			 PTE_BLOCK_PXN | PTE_BLOCK_UXN
>>>    	}, {
>>> +#if IS_ENABLED(CONFIG_IMX_SNPS_DDR_PHY_QB_GEN)
>>> +		/* QB data */
>>
>> Is the ifdeffery necessary ?
>>
>> Where are the QB data located in this case, DRAM or SRAM ?
>>
> 
> It is necessary because CONFIG_SAVED_QB_STATE_BASE is only
> defined when CONFIG_IMX_SNPS_DDR_PHY_QB_GEN is enabled. So it
> throws a compilation error without this. And this is DRAM.

Are there going to be any users that will actually want ot disable this 
functionality ? Why would they do that ?

>>> +		.virt = CONFIG_SAVED_QB_STATE_BASE,
>>> +		.phys = CONFIG_SAVED_QB_STATE_BASE,
>>> +		.size = 0x200000UL,	/* 2M */
>>> +		.attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) |
>>> +			 PTE_BLOCK_OUTER_SHARE
>>> +	}, {

[...]

[PATCH v2 2/5] arm: mach-imx: Add command to expose QB functionality

[Simona Toaca (OSS)]

From: Simona Toaca <simona.toaca@nxp.com>

This command exposes 3 methods:
- check -> checks if the data in volatile memory is valid
	   (integrity check)
- save  -> saves the data to non-volatile memory and
	   erases the data in volatile memory
- erase	-> erases the data in non-volatile memory

cmd_qb can be used either directly in the U-Boot console
or in an uuu script to save the QB data during flashing.
It supports specifying a different boot medium than the
current boot device for saving the data.

Signed-off-by: Viorel Suman <viorel.suman@nxp.com>
Signed-off-by: Ye Li <ye.li@nxp.com>
Signed-off-by: Simona Toaca <simona.toaca@nxp.com>
---
 arch/arm/mach-imx/Kconfig       |  11 +++
 arch/arm/mach-imx/Makefile      |   1 +
 arch/arm/mach-imx/cmd_qb.c      | 132 ++++++++++++++++++++++++++++++++
 arch/arm/mach-imx/imx9/Makefile |   4 +-
 4 files changed, 147 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/mach-imx/cmd_qb.c

diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
index e4014226582..b199a0419fe 100644
--- a/arch/arm/mach-imx/Kconfig
+++ b/arch/arm/mach-imx/Kconfig
@@ -71,6 +71,17 @@ config CSF_SIZE
 	  Define the maximum size for Command Sequence File (CSF) binary
 	  this information is used to define the image boot data.
 
+config CMD_IMX_QB
+	bool "Support the 'qb' command"
+	default y
+	depends on IMX_SNPS_DDR_PHY_QB_GEN && (IMX95 || IMX94)
+	help
+	  Enable qb command to write/erase DDR quick boot training
+	  data to/from a chosen boot device. Using 'qb save/erase'
+	  without args implies using the current boot device. For
+	  use in uuu scripts, the boot device must be specified
+	  explicitly.
+
 config CMD_BMODE
 	bool "Support the 'bmode' command"
 	default y
diff --git a/arch/arm/mach-imx/Makefile b/arch/arm/mach-imx/Makefile
index 0f6e737c0b9..dfa9eca43eb 100644
--- a/arch/arm/mach-imx/Makefile
+++ b/arch/arm/mach-imx/Makefile
@@ -82,6 +82,7 @@ obj-$(CONFIG_CMD_BMODE) += cmd_bmode.o
 obj-$(CONFIG_CMD_HDMIDETECT) += cmd_hdmidet.o
 obj-$(CONFIG_CMD_DEKBLOB) += cmd_dek.o
 obj-$(CONFIG_CMD_NANDBCB) += cmd_nandbcb.o
+obj-$(CONFIG_CMD_IMX_QB) += cmd_qb.o
 endif
 
 ifeq ($(CONFIG_XPL_BUILD),y)
diff --git a/arch/arm/mach-imx/cmd_qb.c b/arch/arm/mach-imx/cmd_qb.c
new file mode 100644
index 00000000000..54733c1ad88
--- /dev/null
+++ b/arch/arm/mach-imx/cmd_qb.c
@@ -0,0 +1,132 @@
+// SPDX-License-Identifier: GPL-2.0+
+/**
+ * Copyright 2024-2026 NXP
+ */
+#include <command.h>
+#include <spl.h>
+#include <stdlib.h>
+
+#include <asm/mach-imx/boot_mode.h>
+#include <asm/mach-imx/sys_proto.h>
+#include <asm/mach-imx/qb.h>
+
+static int get_board_boot_device(enum boot_device dev)
+{
+	switch (dev) {
+	case SD1_BOOT:
+	case MMC1_BOOT:
+		return BOOT_DEVICE_MMC1;
+	case SD2_BOOT:
+	case MMC2_BOOT:
+		return BOOT_DEVICE_MMC2;
+	case USB_BOOT:
+		return BOOT_DEVICE_BOARD;
+	case QSPI_BOOT:
+		return BOOT_DEVICE_SPI;
+	default:
+		return BOOT_DEVICE_NONE;
+	}
+}
+
+static void parse_qb_args(int argc, char * const argv[],
+			  int *qb_dev, int qb_bootdev)
+{
+	long dev = -1;
+	char *interface = "";
+
+	if (argc >= 2) {
+		interface = argv[1];
+	} else {
+		/* qb save -> use boot device */
+		*qb_dev = qb_bootdev;
+	}
+
+	if (argc == 3)
+		dev = simple_strtol(argv[2], NULL, 10);
+
+	if (!strcmp(interface, "mmc") && dev >= 0 &&
+	    dev <= (BOOT_DEVICE_MMC2_2 - BOOT_DEVICE_MMC1))
+		*qb_dev = BOOT_DEVICE_MMC1 + dev;
+
+	if (!strcmp(interface, "spi"))
+		*qb_dev = BOOT_DEVICE_SPI;
+}
+
+static int do_qb(struct cmd_tbl *cmdtp, int flag, int argc,
+		 char * const argv[], bool save)
+{
+	int ret = CMD_RET_FAILURE;
+	enum boot_device boot_dev = UNKNOWN_BOOT;
+	int qb_dev = BOOT_DEVICE_NONE, qb_bootdev;
+
+	boot_dev = get_boot_device();
+	qb_bootdev = get_board_boot_device(boot_dev);
+
+	parse_qb_args(argc, argv, &qb_dev, qb_bootdev);
+
+	ret = qb(qb_dev, qb_bootdev, save);
+
+	return ret ? CMD_RET_FAILURE : CMD_RET_SUCCESS;
+}
+
+static int do_qb_check(struct cmd_tbl *cmdtp, int flag,
+		       int argc, char * const argv[])
+{
+	return qb_check() ? CMD_RET_SUCCESS : CMD_RET_FAILURE;
+}
+
+static int do_qb_save(struct cmd_tbl *cmdtp, int flag,
+		      int argc, char * const argv[])
+{
+	return do_qb(cmdtp, flag, argc, argv, true);
+}
+
+static int do_qb_erase(struct cmd_tbl *cmdtp, int flag,
+		       int argc, char * const argv[])
+{
+	return do_qb(cmdtp, flag, argc, argv, false);
+}
+
+static struct cmd_tbl cmd_qb[] = {
+	U_BOOT_CMD_MKENT(check, 1, 1, do_qb_check, "", ""),
+	U_BOOT_CMD_MKENT(save,  3, 1, do_qb_save,  "", ""),
+	U_BOOT_CMD_MKENT(erase, 3, 1, do_qb_erase, "", ""),
+};
+
+static int do_qbops(struct cmd_tbl *cmdtp, int flag, int argc,
+		    char *const argv[])
+{
+	struct cmd_tbl *cp;
+
+	cp = find_cmd_tbl(argv[1], cmd_qb, ARRAY_SIZE(cmd_qb));
+
+	/* Drop the qb command */
+	argc--;
+	argv++;
+
+	if (!cp) {
+		printf("%s cp is null\n", __func__);
+		return CMD_RET_USAGE;
+	}
+
+	if (argc > cp->maxargs) {
+		printf("%s argc(%d) > cp->maxargs(%d)\n", __func__, argc, cp->maxargs);
+		return CMD_RET_USAGE;
+	}
+
+	if (flag == CMD_FLAG_REPEAT && !cmd_is_repeatable(cp)) {
+		printf("%s %s repeat flag set  but command is not repeatable\n",
+		       __func__, cp->name);
+		return CMD_RET_SUCCESS;
+	}
+
+	return cp->cmd(cmdtp, flag, argc, argv);
+}
+
+U_BOOT_CMD(
+	qb, 4, 1, do_qbops,
+	"DDR Quick Boot sub system",
+	"check - check if quick boot data is stored in mem by training flow\n"
+	"qb save [interface] [dev]  - save quick boot data in NVM location    => trigger quick boot flow\n"
+	"qb erase [interface] [dev] - erase quick boot data from NVM location => trigger training flow\n"
+);
diff --git a/arch/arm/mach-imx/imx9/Makefile b/arch/arm/mach-imx/imx9/Makefile
index 3018d128a36..7dee144e0f4 100644
--- a/arch/arm/mach-imx/imx9/Makefile
+++ b/arch/arm/mach-imx/imx9/Makefile
@@ -15,5 +15,7 @@ obj-y += imx_bootaux.o
 endif
 
 ifeq ($(CONFIG_IMX_SNPS_DDR_PHY_QB_GEN),y)
-obj-y += qb.o
+ifneq ($(CONFIG_XPL_BUILD),y)
+obj-$(CONFIG_CMD_IMX_QB) += qb.o
+endif
 endif
-- 
2.43.0

Re: [PATCH v2 2/5] arm: mach-imx: Add command to expose QB functionality

[Marek Vasut]

On 3/16/26 9:15 AM, Simona Toaca (OSS) wrote:

[...]

> +++ b/arch/arm/mach-imx/Kconfig
> @@ -71,6 +71,17 @@ config CSF_SIZE
>   	  Define the maximum size for Command Sequence File (CSF) binary
>   	  this information is used to define the image boot data.
>   
> +config CMD_IMX_QB
> +	bool "Support the 'qb' command"
> +	default y
> +	depends on IMX_SNPS_DDR_PHY_QB_GEN && (IMX95 || IMX94)

94 should be before 95

> +	help
> +	  Enable qb command to write/erase DDR quick boot training
> +	  data to/from a chosen boot device. Using 'qb save/erase'
> +	  without args

arguments ... please avoid abbreviations

> implies using the current boot device. For
> +	  use in uuu scripts, the boot device must be specified
> +	  explicitly.
> +
>   config CMD_BMODE
>   	bool "Support the 'bmode' command"
>   	default y
> diff --git a/arch/arm/mach-imx/Makefile b/arch/arm/mach-imx/Makefile
> index 0f6e737c0b9..dfa9eca43eb 100644
> --- a/arch/arm/mach-imx/Makefile
> +++ b/arch/arm/mach-imx/Makefile
> @@ -82,6 +82,7 @@ obj-$(CONFIG_CMD_BMODE) += cmd_bmode.o
>   obj-$(CONFIG_CMD_HDMIDETECT) += cmd_hdmidet.o
>   obj-$(CONFIG_CMD_DEKBLOB) += cmd_dek.o
>   obj-$(CONFIG_CMD_NANDBCB) += cmd_nandbcb.o
> +obj-$(CONFIG_CMD_IMX_QB) += cmd_qb.o

Keep the list sorted

>   endif
>   
>   ifeq ($(CONFIG_XPL_BUILD),y)
> diff --git a/arch/arm/mach-imx/cmd_qb.c b/arch/arm/mach-imx/cmd_qb.c
> new file mode 100644
> index 00000000000..54733c1ad88
> --- /dev/null
> +++ b/arch/arm/mach-imx/cmd_qb.c
> @@ -0,0 +1,132 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/**
> + * Copyright 2024-2026 NXP
> + */
> +#include <command.h>
> +#include <spl.h>
> +#include <stdlib.h>
> +
> +#include <asm/mach-imx/boot_mode.h>
> +#include <asm/mach-imx/sys_proto.h>
> +#include <asm/mach-imx/qb.h>
> +
> +static int get_board_boot_device(enum boot_device dev)
> +{
> +	switch (dev) {
> +	case SD1_BOOT:
> +	case MMC1_BOOT:
> +		return BOOT_DEVICE_MMC1;
> +	case SD2_BOOT:
> +	case MMC2_BOOT:
> +		return BOOT_DEVICE_MMC2;
> +	case USB_BOOT:
> +		return BOOT_DEVICE_BOARD;
> +	case QSPI_BOOT:
> +		return BOOT_DEVICE_SPI;
> +	default:
> +		return BOOT_DEVICE_NONE;
> +	}
> +}
> +
> +static void parse_qb_args(int argc, char * const argv[],
> +			  int *qb_dev, int qb_bootdev)
> +{
> +	long dev = -1;
> +	char *interface = "";
> +
> +	if (argc >= 2) {
> +		interface = argv[1];
> +	} else {
> +		/* qb save -> use boot device */
> +		*qb_dev = qb_bootdev;

Maybe invert the conditional, if (args < 2) qb_dev = ... to make it more 
obvious what is going on here ?

> +	}
> +
> +	if (argc == 3)
> +		dev = simple_strtol(argv[2], NULL, 10);
> +
> +	if (!strcmp(interface, "mmc") && dev >= 0 &&
> +	    dev <= (BOOT_DEVICE_MMC2_2 - BOOT_DEVICE_MMC1))
> +		*qb_dev = BOOT_DEVICE_MMC1 + dev;
> +
> +	if (!strcmp(interface, "spi"))

Shouldn't this be "else if" here, since interface can not be both mmc 
and spi ?

> +		*qb_dev = BOOT_DEVICE_SPI;
> +}
> +
> +static int do_qb(struct cmd_tbl *cmdtp, int flag, int argc,
> +		 char * const argv[], bool save)
> +{
> +	int ret = CMD_RET_FAILURE;
> +	enum boot_device boot_dev = UNKNOWN_BOOT;
> +	int qb_dev = BOOT_DEVICE_NONE, qb_bootdev;
> +
> +	boot_dev = get_boot_device();
> +	qb_bootdev = get_board_boot_device(boot_dev);
> +
> +	parse_qb_args(argc, argv, &qb_dev, qb_bootdev);
> +
> +	ret = qb(qb_dev, qb_bootdev, save);
> +
> +	return ret ? CMD_RET_FAILURE : CMD_RET_SUCCESS;
> +}
> +
> +static int do_qb_check(struct cmd_tbl *cmdtp, int flag,
> +		       int argc, char * const argv[])
> +{
> +	return qb_check() ? CMD_RET_SUCCESS : CMD_RET_FAILURE;
> +}
> +
> +static int do_qb_save(struct cmd_tbl *cmdtp, int flag,
> +		      int argc, char * const argv[])
> +{
> +	return do_qb(cmdtp, flag, argc, argv, true);
> +}
> +
> +static int do_qb_erase(struct cmd_tbl *cmdtp, int flag,
> +		       int argc, char * const argv[])
> +{
> +	return do_qb(cmdtp, flag, argc, argv, false);
> +}
> +
> +static struct cmd_tbl cmd_qb[] = {
> +	U_BOOT_CMD_MKENT(check, 1, 1, do_qb_check, "", ""),
> +	U_BOOT_CMD_MKENT(save,  3, 1, do_qb_save,  "", ""),
> +	U_BOOT_CMD_MKENT(erase, 3, 1, do_qb_erase, "", ""),
> +};
> +
> +static int do_qbops(struct cmd_tbl *cmdtp, int flag, int argc,
> +		    char *const argv[])
> +{
> +	struct cmd_tbl *cp;
> +
> +	cp = find_cmd_tbl(argv[1], cmd_qb, ARRAY_SIZE(cmd_qb));
> +
> +	/* Drop the qb command */
> +	argc--;
> +	argv++;
> +
> +	if (!cp) {
> +		printf("%s cp is null\n", __func__);

What does this error output even mean ? The user will be confused, 
please reword it.

> +		return CMD_RET_USAGE;
> +	}
> +
> +	if (argc > cp->maxargs) {
> +		printf("%s argc(%d) > cp->maxargs(%d)\n", __func__, argc, cp->maxargs);

Reword this too please.

> +		return CMD_RET_USAGE;
> +	}
> +
> +	if (flag == CMD_FLAG_REPEAT && !cmd_is_repeatable(cp)) {
> +		printf("%s %s repeat flag set  but command is not repeatable\n",

One space is enough.

> +		       __func__, cp->name);
> +		return CMD_RET_SUCCESS;
> +	}
> +
> +	return cp->cmd(cmdtp, flag, argc, argv);
> +}
> +
> +U_BOOT_CMD(
> +	qb, 4, 1, do_qbops,
> +	"DDR Quick Boot sub system",
> +	"check - check if quick boot data is stored in mem by training flow\n"
> +	"qb save [interface] [dev]  - save quick boot data in NVM location    => trigger quick boot flow\n"
> +	"qb erase [interface] [dev] - erase quick boot data from NVM location => trigger training flow\n"
> +);
> diff --git a/arch/arm/mach-imx/imx9/Makefile b/arch/arm/mach-imx/imx9/Makefile
> index 3018d128a36..7dee144e0f4 100644
> --- a/arch/arm/mach-imx/imx9/Makefile
> +++ b/arch/arm/mach-imx/imx9/Makefile
> @@ -15,5 +15,7 @@ obj-y += imx_bootaux.o
>   endif
>   
>   ifeq ($(CONFIG_IMX_SNPS_DDR_PHY_QB_GEN),y)
> -obj-y += qb.o
> +ifneq ($(CONFIG_XPL_BUILD),y)
> +obj-$(CONFIG_CMD_IMX_QB) += qb.o
> +endif

This shoudld be part of 1/5 , should it not ?

Re: [PATCH v2 2/5] arm: mach-imx: Add command to expose QB functionality

[Simona Toaca]

On Mon, Mar 16, 2026 at 01:20:06PM +0100, Marek Vasut wrote:
> On 3/16/26 9:15 AM, Simona Toaca (OSS) wrote:
> 
> [...]
> 
> > +++ b/arch/arm/mach-imx/Kconfig
> > @@ -71,6 +71,17 @@ config CSF_SIZE
> >   	  Define the maximum size for Command Sequence File (CSF) binary
> >   	  this information is used to define the image boot data.
> > +config CMD_IMX_QB
> > +	bool "Support the 'qb' command"
> > +	default y
> > +	depends on IMX_SNPS_DDR_PHY_QB_GEN && (IMX95 || IMX94)
> 
> 94 should be before 95
> 

[ ... ]

> > +static struct cmd_tbl cmd_qb[] = {
> > +	U_BOOT_CMD_MKENT(check, 1, 1, do_qb_check, "", ""),
> > +	U_BOOT_CMD_MKENT(save,  3, 1, do_qb_save,  "", ""),
> > +	U_BOOT_CMD_MKENT(erase, 3, 1, do_qb_erase, "", ""),
> > +};
> > +
> > +static int do_qbops(struct cmd_tbl *cmdtp, int flag, int argc,
> > +		    char *const argv[])
> > +{
> > +	struct cmd_tbl *cp;
> > +
> > +	cp = find_cmd_tbl(argv[1], cmd_qb, ARRAY_SIZE(cmd_qb));
> > +
> > +	/* Drop the qb command */
> > +	argc--;
> > +	argv++;
> > +
> > +	if (!cp) {
> > +		printf("%s cp is null\n", __func__);
> 
> What does this error output even mean ? The user will be confused, please
> reword it.
> 

Looking at this, these error messages might be redundant, as U-Boot displays
the command usage anyway if CMD_RET_USAGE is returned. The repeatable error is
also redundant because the command is declared repeatable.

I have a suggestion: I could use the U-Boot API for commands from
include/command.h (U_BOOT_CMD_WITH_SUBCMDS), which declares and registers
do_<cmdname> with the default method body (which is almost the same as this
version). This would imply no actual error messages, just printing the usage,
and a cleaner code.

If it's not a good approach, I will just modify the error message here as you
suggested.

> > +		return CMD_RET_USAGE;
> > +	}
> > +
> > +	if (argc > cp->maxargs) {
> > +		printf("%s argc(%d) > cp->maxargs(%d)\n", __func__, argc, cp->maxargs);
> 
> Reword this too please.
> 
> > +		return CMD_RET_USAGE;
> > +	}
> > +
> > +	if (flag == CMD_FLAG_REPEAT && !cmd_is_repeatable(cp)) {
> > +		printf("%s %s repeat flag set  but command is not repeatable\n",
> 
> One space is enough.
> 
> > +		       __func__, cp->name);
> > +		return CMD_RET_SUCCESS;
> > +	}
> > +
> > +	return cp->cmd(cmdtp, flag, argc, argv);
> > +}
> > +
> > +U_BOOT_CMD(
> > +	qb, 4, 1, do_qbops,
> > +	"DDR Quick Boot sub system",
> > +	"check - check if quick boot data is stored in mem by training flow\n"
> > +	"qb save [interface] [dev]  - save quick boot data in NVM location    => trigger quick boot flow\n"
> > +	"qb erase [interface] [dev] - erase quick boot data from NVM location => trigger training flow\n"
> > +);
> > diff --git a/arch/arm/mach-imx/imx9/Makefile b/arch/arm/mach-imx/imx9/Makefile
> > index 3018d128a36..7dee144e0f4 100644
> > --- a/arch/arm/mach-imx/imx9/Makefile
> > +++ b/arch/arm/mach-imx/imx9/Makefile
> > @@ -15,5 +15,7 @@ obj-y += imx_bootaux.o
> >   endif
> >   ifeq ($(CONFIG_IMX_SNPS_DDR_PHY_QB_GEN),y)
> > -obj-y += qb.o
> > +ifneq ($(CONFIG_XPL_BUILD),y)
> > +obj-$(CONFIG_CMD_IMX_QB) += qb.o
> > +endif
> 
> This shoudld be part of 1/5 , should it not ?

And squash commits 1 and 2, as this is cmd_qb related?
Can't I keep them separated, so there is not a lot of code
in one patch? I also find that this way I can explain each
piece of code in the commit message.

Re: [PATCH v2 2/5] arm: mach-imx: Add command to expose QB functionality

[Marek Vasut]

On 3/17/26 4:28 PM, Simona Toaca wrote:

[...]

>>> +	if (!cp) {
>>> +		printf("%s cp is null\n", __func__);
>>
>> What does this error output even mean ? The user will be confused, please
>> reword it.
>>
> 
> Looking at this, these error messages might be redundant, as U-Boot displays
> the command usage anyway if CMD_RET_USAGE is returned. The repeatable error is
> also redundant because the command is declared repeatable.
> 
> I have a suggestion: I could use the U-Boot API for commands from
> include/command.h (U_BOOT_CMD_WITH_SUBCMDS), which declares and registers
> do_<cmdname> with the default method body (which is almost the same as this
> version). This would imply no actual error messages, just printing the usage,
> and a cleaner code.

Aren't the error messages helpful to the user to determine what went 
wrong during the command execution ?

[...]

>>> +++ b/arch/arm/mach-imx/imx9/Makefile
>>> @@ -15,5 +15,7 @@ obj-y += imx_bootaux.o
>>>    endif
>>>    ifeq ($(CONFIG_IMX_SNPS_DDR_PHY_QB_GEN),y)
>>> -obj-y += qb.o
>>> +ifneq ($(CONFIG_XPL_BUILD),y)
>>> +obj-$(CONFIG_CMD_IMX_QB) += qb.o
>>> +endif
>>
>> This shoudld be part of 1/5 , should it not ?
> 
> And squash commits 1 and 2, as this is cmd_qb related?

No, only add the qb.o Makefile entry in 1/5 , since qb.c is added in 1/5 
too.

[PATCH v2 3/5] imx9: Enable QB data saving for iMX9{4,5} EVK

[Simona Toaca (OSS)]

From: Simona Toaca <simona.toaca@nxp.com>

Add the necessary options for enabling DDR training
data saving to NVM and running QuickBoot flow. This
makes Quickboot commands available in the U-Boot console,
as CMD_IMX_QB is enabled by default.

Signed-off-by: Viorel Suman <viorel.suman@nxp.com>
Signed-off-by: Simona Toaca <simona.toaca@nxp.com>
---
 configs/imx943_evk_defconfig      | 1 +
 configs/imx95_15x15_evk_defconfig | 1 +
 configs/imx95_evk.config          | 1 +
 3 files changed, 3 insertions(+)

diff --git a/configs/imx943_evk_defconfig b/configs/imx943_evk_defconfig
index ef4f9a8fcbc..57d4240ba26 100644
--- a/configs/imx943_evk_defconfig
+++ b/configs/imx943_evk_defconfig
@@ -96,6 +96,7 @@ CONFIG_SPL_CLK_CCF=y
 CONFIG_CLK_CCF=y
 CONFIG_CLK_SCMI=y
 CONFIG_SPL_CLK_SCMI=y
+CONFIG_IMX_SNPS_DDR_PHY_QB_GEN=y
 CONFIG_SPL_FIRMWARE=y
 # CONFIG_SCMI_AGENT_SMCCC is not set
 CONFIG_IMX_RGPIO2P=y
diff --git a/configs/imx95_15x15_evk_defconfig b/configs/imx95_15x15_evk_defconfig
index 38a855417d0..03c9b71f8bd 100644
--- a/configs/imx95_15x15_evk_defconfig
+++ b/configs/imx95_15x15_evk_defconfig
@@ -100,6 +100,7 @@ CONFIG_SPL_CLK_CCF=y
 CONFIG_CLK_SCMI=y
 CONFIG_SPL_CLK_SCMI=y
 CONFIG_CLK_IMX95_BLKCTRL=y
+CONFIG_IMX_SNPS_DDR_PHY_QB_GEN=y
 CONFIG_DFU_MMC=y
 CONFIG_DFU_RAM=y
 CONFIG_SPL_FIRMWARE=y
diff --git a/configs/imx95_evk.config b/configs/imx95_evk.config
index 3db583cce59..b34d3f9c935 100644
--- a/configs/imx95_evk.config
+++ b/configs/imx95_evk.config
@@ -101,6 +101,7 @@ CONFIG_SPL_CLK_CCF=y
 CONFIG_CLK_CCF=y
 CONFIG_CLK_SCMI=y
 CONFIG_SPL_CLK_SCMI=y
+CONFIG_IMX_SNPS_DDR_PHY_QB_GEN=y
 CONFIG_DFU_MMC=y
 CONFIG_DFU_RAM=y
 CONFIG_SPL_FIRMWARE=y
-- 
2.43.0

[PATCH v2 4/5] board: nxp: imx9{4,5}_evk: Add qb save option in SPL

[Simona Toaca (OSS)]

From: Simona Toaca <simona.toaca@nxp.com>

Call qb save automatically in the board-specific
spl_board_init(), if SPL_IMX_QB option is enabled.
This makes sure qb_save is called before any image
loading is done by the SPL. This option is also
suitable for the case where U-Boot proper is
missing (Falcon mode).

qb save refers to saving DDR training data to NVM,
so that OEI runs Quickboot flow on next reboot,
skipping full training and achieveing a lower boot
time.

Signed-off-by: Simona Toaca <simona.toaca@nxp.com>
---
 arch/arm/include/asm/mach-imx/qb.h | 2 ++
 arch/arm/mach-imx/Kconfig          | 8 ++++++++
 arch/arm/mach-imx/imx9/Makefile    | 4 +++-
 arch/arm/mach-imx/imx9/qb.c        | 9 +++++++++
 board/nxp/imx94_evk/spl.c          | 6 +++++-
 board/nxp/imx95_evk/spl.c          | 6 +++++-
 6 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/arch/arm/include/asm/mach-imx/qb.h b/arch/arm/include/asm/mach-imx/qb.h
index 5efe68f0a60..4f923d79e7a 100644
--- a/arch/arm/include/asm/mach-imx/qb.h
+++ b/arch/arm/include/asm/mach-imx/qb.h
@@ -10,4 +10,6 @@
 
 bool qb_check(void);
 int  qb(int qb_dev, int qb_bootdev, bool save);
+void spl_qb_save(void);
+
 #endif
diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
index b199a0419fe..614829d1587 100644
--- a/arch/arm/mach-imx/Kconfig
+++ b/arch/arm/mach-imx/Kconfig
@@ -71,6 +71,14 @@ config CSF_SIZE
 	  Define the maximum size for Command Sequence File (CSF) binary
 	  this information is used to define the image boot data.
 
+config SPL_IMX_QB
+	bool "Run qb save during SPL"
+	depends on SPL && IMX_SNPS_DDR_PHY_QB_GEN && (IMX95 || IMX94)
+	help
+	  Automatically save DDR training data (Quickboot data)
+	  to current boot device when needed (when OEI runs Training
+	  flow and saves qb data to volatile memory).
+
 config CMD_IMX_QB
 	bool "Support the 'qb' command"
 	default y
diff --git a/arch/arm/mach-imx/imx9/Makefile b/arch/arm/mach-imx/imx9/Makefile
index 7dee144e0f4..3207013812a 100644
--- a/arch/arm/mach-imx/imx9/Makefile
+++ b/arch/arm/mach-imx/imx9/Makefile
@@ -15,7 +15,9 @@ obj-y += imx_bootaux.o
 endif
 
 ifeq ($(CONFIG_IMX_SNPS_DDR_PHY_QB_GEN),y)
-ifneq ($(CONFIG_XPL_BUILD),y)
+ifeq ($(CONFIG_XPL_BUILD),y)
+obj-$(CONFIG_SPL_IMX_QB) += qb.o
+else
 obj-$(CONFIG_CMD_IMX_QB) += qb.o
 endif
 endif
diff --git a/arch/arm/mach-imx/imx9/qb.c b/arch/arm/mach-imx/imx9/qb.c
index 9987c57b16a..e00d2fd7512 100644
--- a/arch/arm/mach-imx/imx9/qb.c
+++ b/arch/arm/mach-imx/imx9/qb.c
@@ -428,3 +428,12 @@ int qb(int qb_dev, int qb_bootdev, bool save)
 
 	return 0;
 }
+
+void spl_qb_save(void)
+{
+	int dev = spl_boot_device();
+
+	/* Save QB data on current boot device */
+	if (qb(dev, dev, true))
+		printf("QB save failed\n");
+}
diff --git a/board/nxp/imx94_evk/spl.c b/board/nxp/imx94_evk/spl.c
index cc5b7f9ef0f..1d25795eb17 100644
--- a/board/nxp/imx94_evk/spl.c
+++ b/board/nxp/imx94_evk/spl.c
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0+
 /*
- * Copyright 2025 NXP
+ * Copyright 2025-2026 NXP
  */
 
 #include <hang.h>
@@ -14,6 +14,7 @@
 #include <asm/arch/sys_proto.h>
 #include <asm/mach-imx/boot_mode.h>
 #include <asm/mach-imx/ele_api.h>
+#include <asm/mach-imx/qb.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -44,6 +45,9 @@ void spl_board_init(void)
 	ret = ele_start_rng();
 	if (ret)
 		printf("Fail to start RNG: %d\n", ret);
+
+	if (IS_ENABLED(CONFIG_SPL_IMX_QB))
+		spl_qb_save();
 }
 
 /* SCMI support by default */
diff --git a/board/nxp/imx95_evk/spl.c b/board/nxp/imx95_evk/spl.c
index 761a1a4a0f6..35e4458f2b7 100644
--- a/board/nxp/imx95_evk/spl.c
+++ b/board/nxp/imx95_evk/spl.c
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0+
 /*
- * Copyright 2025 NXP
+ * Copyright 2025-2026 NXP
  */
 
 #include <hang.h>
@@ -13,6 +13,7 @@
 #include <asm/arch/sys_proto.h>
 #include <asm/mach-imx/boot_mode.h>
 #include <asm/mach-imx/ele_api.h>
+#include <asm/mach-imx/qb.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -41,6 +42,9 @@ void spl_board_init(void)
 	ret = ele_start_rng();
 	if (ret)
 		printf("Fail to start RNG: %d\n", ret);
+
+	if (IS_ENABLED(CONFIG_SPL_IMX_QB))
+		spl_qb_save();
 }
 
 void board_init_f(ulong dummy)
-- 
2.43.0

Re: [PATCH v2 4/5] board: nxp: imx9{4,5}_evk: Add qb save option in SPL

[Marek Vasut]

On 3/16/26 9:15 AM, Simona Toaca (OSS) wrote:

[...]

> +++ b/arch/arm/mach-imx/imx9/qb.c
> @@ -428,3 +428,12 @@ int qb(int qb_dev, int qb_bootdev, bool save)
>   
>   	return 0;
>   }
> +
> +void spl_qb_save(void)
> +{
> +	int dev = spl_boot_device();
> +
> +	/* Save QB data on current boot device */
> +	if (qb(dev, dev, true))
> +		printf("QB save failed\n");
> +}

Should be in 1/5 ?

> diff --git a/board/nxp/imx94_evk/spl.c b/board/nxp/imx94_evk/spl.c
> index cc5b7f9ef0f..1d25795eb17 100644
> --- a/board/nxp/imx94_evk/spl.c
> +++ b/board/nxp/imx94_evk/spl.c
> @@ -1,6 +1,6 @@
>   // SPDX-License-Identifier: GPL-2.0+
>   /*
> - * Copyright 2025 NXP
> + * Copyright 2025-2026 NXP
>    */
>   
>   #include <hang.h>
> @@ -14,6 +14,7 @@
>   #include <asm/arch/sys_proto.h>
>   #include <asm/mach-imx/boot_mode.h>
>   #include <asm/mach-imx/ele_api.h>
> +#include <asm/mach-imx/qb.h>
>   
>   DECLARE_GLOBAL_DATA_PTR;
>   
> @@ -44,6 +45,9 @@ void spl_board_init(void)
>   	ret = ele_start_rng();
>   	if (ret)
>   		printf("Fail to start RNG: %d\n", ret);

Use CONFIG_IS_ENABLED() if this is meant to be enabled in different 
U-Boot stages.

> +	if (IS_ENABLED(CONFIG_SPL_IMX_QB))
> +		spl_qb_save();
>   }
>   
>   /* SCMI support by default */
> diff --git a/board/nxp/imx95_evk/spl.c b/board/nxp/imx95_evk/spl.c
> index 761a1a4a0f6..35e4458f2b7 100644
> --- a/board/nxp/imx95_evk/spl.c
> +++ b/board/nxp/imx95_evk/spl.c
> @@ -1,6 +1,6 @@
>   // SPDX-License-Identifier: GPL-2.0+
>   /*
> - * Copyright 2025 NXP
> + * Copyright 2025-2026 NXP
>    */
>   
>   #include <hang.h>
> @@ -13,6 +13,7 @@
>   #include <asm/arch/sys_proto.h>
>   #include <asm/mach-imx/boot_mode.h>
>   #include <asm/mach-imx/ele_api.h>
> +#include <asm/mach-imx/qb.h>
>   
>   DECLARE_GLOBAL_DATA_PTR;
>   
> @@ -41,6 +42,9 @@ void spl_board_init(void)
>   	ret = ele_start_rng();
>   	if (ret)
>   		printf("Fail to start RNG: %d\n", ret);
> +
> +	if (IS_ENABLED(CONFIG_SPL_IMX_QB))

CONFIG_IS_ENABLED() ?

> +		spl_qb_save();
>   }
>   
>   void board_init_f(ulong dummy)

Re: [PATCH v2 4/5] board: nxp: imx9{4,5}_evk: Add qb save option in SPL

[Simona Toaca]

On Mon, Mar 16, 2026 at 01:22:55PM +0100, Marek Vasut wrote:
> On 3/16/26 9:15 AM, Simona Toaca (OSS) wrote:
> 
> [...]
> 
> > +++ b/arch/arm/mach-imx/imx9/qb.c
> > @@ -428,3 +428,12 @@ int qb(int qb_dev, int qb_bootdev, bool save)
> >   	return 0;
> >   }
> > +
> > +void spl_qb_save(void)
> > +{
> > +	int dev = spl_boot_device();
> > +
> > +	/* Save QB data on current boot device */
> > +	if (qb(dev, dev, true))
> > +		printf("QB save failed\n");
> > +}
> 
> Should be in 1/5 ?
>

I thought it would make sense that all the SPL-related changes should be in
a separate patch.

> > diff --git a/board/nxp/imx94_evk/spl.c b/board/nxp/imx94_evk/spl.c
> > index cc5b7f9ef0f..1d25795eb17 100644
> > --- a/board/nxp/imx94_evk/spl.c
> > +++ b/board/nxp/imx94_evk/spl.c
> > @@ -1,6 +1,6 @@
> >   // SPDX-License-Identifier: GPL-2.0+
> >   /*
> > - * Copyright 2025 NXP
> > + * Copyright 2025-2026 NXP
> >    */
> >   #include <hang.h>
> > @@ -14,6 +14,7 @@
> >   #include <asm/arch/sys_proto.h>
> >   #include <asm/mach-imx/boot_mode.h>
> >   #include <asm/mach-imx/ele_api.h>
> > +#include <asm/mach-imx/qb.h>
> >   DECLARE_GLOBAL_DATA_PTR;
> > @@ -44,6 +45,9 @@ void spl_board_init(void)
> >   	ret = ele_start_rng();
> >   	if (ret)
> >   		printf("Fail to start RNG: %d\n", ret);
> 
> Use CONFIG_IS_ENABLED() if this is meant to be enabled in different U-Boot
> stages.
> 

As far as I know, spl.o is only compiled if the U-Boot stage is SPL, so
there can't be any other stage for this file.

> > +	if (IS_ENABLED(CONFIG_SPL_IMX_QB))
> > +		spl_qb_save();

[ ... ]

Re: [PATCH v2 4/5] board: nxp: imx9{4,5}_evk: Add qb save option in SPL

[Marek Vasut]

On 3/17/26 1:40 PM, Simona Toaca wrote:
> On Mon, Mar 16, 2026 at 01:22:55PM +0100, Marek Vasut wrote:
>> On 3/16/26 9:15 AM, Simona Toaca (OSS) wrote:
>>
>> [...]
>>
>>> +++ b/arch/arm/mach-imx/imx9/qb.c
>>> @@ -428,3 +428,12 @@ int qb(int qb_dev, int qb_bootdev, bool save)
>>>    	return 0;
>>>    }
>>> +
>>> +void spl_qb_save(void)
>>> +{
>>> +	int dev = spl_boot_device();
>>> +
>>> +	/* Save QB data on current boot device */
>>> +	if (qb(dev, dev, true))
>>> +		printf("QB save failed\n");
>>> +}
>>
>> Should be in 1/5 ?
>>
> 
> I thought it would make sense that all the SPL-related changes should be in
> a separate patch.

This is a board patch, which modifies common code. That shouldn't be 
happening.

[...]

[PATCH v2 5/5] doc: board: nxp: Add Quickboot documentation

[Simona Toaca (OSS)]

From: Simona Toaca <simona.toaca@nxp.com>

Add instructions on how to use U-Boot to save
DDR training data to NVM and explain the saving
process.

Signed-off-by: Simona Toaca <simona.toaca@nxp.com>
---
 doc/board/nxp/index.rst |  1 +
 doc/board/nxp/qb.rst    | 49 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 50 insertions(+)
 create mode 100644 doc/board/nxp/qb.rst

diff --git a/doc/board/nxp/index.rst b/doc/board/nxp/index.rst
index 01d3468a47d..7e3e5e29880 100644
--- a/doc/board/nxp/index.rst
+++ b/doc/board/nxp/index.rst
@@ -29,3 +29,4 @@ NXP Semiconductors
    mx6ullevk
    rproc
    psb
+   qb
diff --git a/doc/board/nxp/qb.rst b/doc/board/nxp/qb.rst
new file mode 100644
index 00000000000..5a1e913ea11
--- /dev/null
+++ b/doc/board/nxp/qb.rst
@@ -0,0 +1,49 @@
+.. SPDX-License-Identifier: GPL-2.0+
+   Copyright 2026 NXP
+
+DDR QuickBoot flow
+------------------
+
+Some NXP boards (which use OEI - iMX943, iMX95, etc.) support saving DDR
+training data (collected by OEI during Training flow) from volatile
+to non-volatile memory, which is then available to OEI at next cold reboot.
+OEI uses the saved data to run Quickboot flow and avoid training the DDR again.
+This significantly reduces the boot time.
+
+U-Boot provides no authentication for qb data, only its integrity
+is verified via the CRC32. The authentication is done in OEI. With
+the exception of iMX95 A0/A1, which use CRC32 as well for verifying
+the data, the rest of the boards use ELE to verify the MAC stored
+in the ddrphy_qb_state structure.
+
+If the quickboot data in memory is not valid (CRC32 check fails),
+U-Boot does not save it to NVM. So, if OEI runs Quickboot flow -> no
+data is written to volatile memory -> invalid data -> no saving happens
+(qb save fails during qb check).
+
+After successful saving, U-Boot clears the data in volatile memory so
+that qb check fails at next reboot and the NVM isn't accessed again.
+
+There are 2 ways to save this data (both can be enabled):
+
+1. automatically, in SPL (by enabling CONFIG_SPL_IMX_QB)
+
+- this will save the data on the current boot device (e.g. SD)
+- other configs specific to the boot device need to be enabled (CONFIG_SPL_MMC_WRITE for saving to eMMC/SD)
+- use for: automating qb save / saving qb data if using Falcon mode (skipping U-Boot proper)
+
+2. using qb command in U-Boot console (by enabling CONFIG_CMD_IMX_QB)
+
+- supports saving on the current boot device, or on another, specified device.
+- if flashing via uuu, the command can be added in an uuu script (boot device needs to be specified)
+- use for: saving the qb data during flashing / controlling the NVM to save to
+
+::
+
+        # To save/erase on current boot device
+        => qb save/erase
+
+        # To save/erase on other boot device
+        => qb save/erase mmc 0 # eMMC
+        => qb save/erase mmc 1 # SD
+        => qb save/erase spi   # NOR SPI
-- 
2.43.0

Re: [PATCH v2 5/5] doc: board: nxp: Add Quickboot documentation

[Marek Vasut]

On 3/16/26 9:15 AM, Simona Toaca (OSS) wrote:

> +After successful saving, U-Boot clears the data in volatile memory so
> +that qb check fails at next reboot and the NVM isn't accessed again.
> +
> +There are 2 ways to save this data (both can be enabled):

Replace () with , .

> +1. automatically, in SPL (by enabling CONFIG_SPL_IMX_QB)
> +
> +- this will save the data on the current boot device (e.g. SD)
> +- other configs specific to the boot device need to be enabled (CONFIG_SPL_MMC_WRITE for saving to eMMC/SD)
> +- use for: automating qb save / saving qb data if using Falcon mode (skipping U-Boot proper)
> +
> +2. using qb command in U-Boot console (by enabling CONFIG_CMD_IMX_QB)
> +
> +- supports saving on the current boot device, or on another, specified device.
> +- if flashing via uuu, the command can be added in an uuu script (boot device needs to be specified)
> +- use for: saving the qb data during flashing / controlling the NVM to save to
> +
> +::
> +
> +        # To save/erase on current boot device
> +        => qb save/erase
> +
> +        # To save/erase on other boot device
> +        => qb save/erase mmc 0 # eMMC
> +        => qb save/erase mmc 1 # SD
> +        => qb save/erase spi   # NOR SPI

Can QB data be saved to arbitrary SPI NOR ?

Looking at this, why can this QB not use the same interface to select 
arbitrary block storage device/partition/... that e.g. FS_GENERIC does use ?

Re: [PATCH v2 5/5] doc: board: nxp: Add Quickboot documentation

[Simona Toaca]

On Mon, Mar 16, 2026 at 01:32:48PM +0100, Marek Vasut wrote:
> On 3/16/26 9:15 AM, Simona Toaca (OSS) wrote:
> 
> > +After successful saving, U-Boot clears the data in volatile memory so
> > +that qb check fails at next reboot and the NVM isn't accessed again.
> > +
> > +There are 2 ways to save this data (both can be enabled):
> 
> Replace () with , .
> 
> > +1. automatically, in SPL (by enabling CONFIG_SPL_IMX_QB)
> > +
> > +- this will save the data on the current boot device (e.g. SD)
> > +- other configs specific to the boot device need to be enabled (CONFIG_SPL_MMC_WRITE for saving to eMMC/SD)
> > +- use for: automating qb save / saving qb data if using Falcon mode (skipping U-Boot proper)
> > +
> > +2. using qb command in U-Boot console (by enabling CONFIG_CMD_IMX_QB)
> > +
> > +- supports saving on the current boot device, or on another, specified device.
> > +- if flashing via uuu, the command can be added in an uuu script (boot device needs to be specified)
> > +- use for: saving the qb data during flashing / controlling the NVM to save to
> > +
> > +::
> > +
> > +        # To save/erase on current boot device
> > +        => qb save/erase
> > +
> > +        # To save/erase on other boot device
> > +        => qb save/erase mmc 0 # eMMC
> > +        => qb save/erase mmc 1 # SD
> > +        => qb save/erase spi   # NOR SPI
> 
> Can QB data be saved to arbitrary SPI NOR ?
> 

No, this SPI NOR refers to the SPI controller selected by
the CONFIG_SF_DEFAULT_BUS and CONFIG_SF_DEFAULT_CS options.

The point is that the device we save the data to is also an
option to boot from, otherwise the qb data is useless. And the
board will boot from the selected configs if one boots from spi.

> Looking at this, why can this QB not use the same interface to select
> arbitrary block storage device/partition/... that e.g. FS_GENERIC does use ?

There is a limited list of boot devices. By keeping things the way they are
we limit the usage of this command to the actually useful boot devices (and
that we also test this command on).

Re: [PATCH v2 5/5] doc: board: nxp: Add Quickboot documentation

[Marek Vasut]

On 3/17/26 1:53 PM, Simona Toaca wrote:
> On Mon, Mar 16, 2026 at 01:32:48PM +0100, Marek Vasut wrote:
>> On 3/16/26 9:15 AM, Simona Toaca (OSS) wrote:
>>
>>> +After successful saving, U-Boot clears the data in volatile memory so
>>> +that qb check fails at next reboot and the NVM isn't accessed again.
>>> +
>>> +There are 2 ways to save this data (both can be enabled):
>>
>> Replace () with , .
>>
>>> +1. automatically, in SPL (by enabling CONFIG_SPL_IMX_QB)
>>> +
>>> +- this will save the data on the current boot device (e.g. SD)
>>> +- other configs specific to the boot device need to be enabled (CONFIG_SPL_MMC_WRITE for saving to eMMC/SD)
>>> +- use for: automating qb save / saving qb data if using Falcon mode (skipping U-Boot proper)
>>> +
>>> +2. using qb command in U-Boot console (by enabling CONFIG_CMD_IMX_QB)
>>> +
>>> +- supports saving on the current boot device, or on another, specified device.
>>> +- if flashing via uuu, the command can be added in an uuu script (boot device needs to be specified)
>>> +- use for: saving the qb data during flashing / controlling the NVM to save to
>>> +
>>> +::
>>> +
>>> +        # To save/erase on current boot device
>>> +        => qb save/erase
>>> +
>>> +        # To save/erase on other boot device
>>> +        => qb save/erase mmc 0 # eMMC
>>> +        => qb save/erase mmc 1 # SD
>>> +        => qb save/erase spi   # NOR SPI
>>
>> Can QB data be saved to arbitrary SPI NOR ?
>>
> 
> No, this SPI NOR refers to the SPI controller selected by
> the CONFIG_SF_DEFAULT_BUS and CONFIG_SF_DEFAULT_CS options.

Maybe the user would want to make a backup of the QB data to another SPI 
NOR (e.g. if the device has two, which is not unheard of) ? They should 
not be prevented from doing so.

> The point is that the device we save the data to is also an
> option to boot from, otherwise the qb data is useless.

The QB data could be stored for backup purposes, that is not useless.

> And the
> board will boot from the selected configs if one boots from spi.
Please see above.

>> Looking at this, why can this QB not use the same interface to select
>> arbitrary block storage device/partition/... that e.g. FS_GENERIC does use ?
> 
> There is a limited list of boot devices. By keeping things the way they are
> we limit the usage of this command to the actually useful boot devices (and
> that we also test this command on).

I would argue that it is better to use generic code instead of 
hand-writing local ad-hoc implementation which is a limited version of 
the same.

Re: [PATCH v2 5/5] doc: board: nxp: Add Quickboot documentation

[Simona Toaca]

Hi Marek,

On Tue, Mar 17, 2026 at 06:10:47PM +0100, Marek Vasut wrote:
> On 3/17/26 1:53 PM, Simona Toaca wrote:
> > On Mon, Mar 16, 2026 at 01:32:48PM +0100, Marek Vasut wrote:
> > > On 3/16/26 9:15 AM, Simona Toaca (OSS) wrote:
> > > 
> > > > +After successful saving, U-Boot clears the data in volatile memory so
> > > > +that qb check fails at next reboot and the NVM isn't accessed again.
> > > > +
> > > > +There are 2 ways to save this data (both can be enabled):
> > > 
> > > Replace () with , .
> > > 
> > > > +1. automatically, in SPL (by enabling CONFIG_SPL_IMX_QB)
> > > > +
> > > > +- this will save the data on the current boot device (e.g. SD)
> > > > +- other configs specific to the boot device need to be enabled (CONFIG_SPL_MMC_WRITE for saving to eMMC/SD)
> > > > +- use for: automating qb save / saving qb data if using Falcon mode (skipping U-Boot proper)
> > > > +
> > > > +2. using qb command in U-Boot console (by enabling CONFIG_CMD_IMX_QB)
> > > > +
> > > > +- supports saving on the current boot device, or on another, specified device.
> > > > +- if flashing via uuu, the command can be added in an uuu script (boot device needs to be specified)
> > > > +- use for: saving the qb data during flashing / controlling the NVM to save to
> > > > +
> > > > +::
> > > > +
> > > > +        # To save/erase on current boot device
> > > > +        => qb save/erase
> > > > +
> > > > +        # To save/erase on other boot device
> > > > +        => qb save/erase mmc 0 # eMMC
> > > > +        => qb save/erase mmc 1 # SD
> > > > +        => qb save/erase spi   # NOR SPI
> > > 
> > > Can QB data be saved to arbitrary SPI NOR ?
> > > 
> > 
> > No, this SPI NOR refers to the SPI controller selected by
> > the CONFIG_SF_DEFAULT_BUS and CONFIG_SF_DEFAULT_CS options.
> 
> Maybe the user would want to make a backup of the QB data to another SPI NOR
> (e.g. if the device has two, which is not unheard of) ? They should not be
> prevented from doing so.
> 
> > The point is that the device we save the data to is also an
> > option to boot from, otherwise the qb data is useless.
> 
> The QB data could be stored for backup purposes, that is not useless.
> 
There is one thing that might make this clearer, and I forgot to
mention it explicitly:

The get_qbdata_offset method searches for a 64KB hole in the bootloader,
so it can save the data there. OEI knows that's where the data
should be saved. On an arbitrary non-boot device, or, on a filesystem
partition, the hole would not exist. It is mkimage that puts that
hole in the bootloader, and U-Boot explicitly checks for container
header tags.

If one wants to store the data elsewhere, they could use
the generic fs methods to make the backup wherever they want, and re-load
it themselves to the QB_SAVED_STATE_BASE address, so that U-Boot can save
it from there.

> > And the
> > board will boot from the selected configs if one boots from spi.
> Please see above.
> 
> > > Looking at this, why can this QB not use the same interface to select
> > > arbitrary block storage device/partition/... that e.g. FS_GENERIC does use ?
> > 
> > There is a limited list of boot devices. By keeping things the way they are
> > we limit the usage of this command to the actually useful boot devices (and
> > that we also test this command on).
> 
> I would argue that it is better to use generic code instead of hand-writing
> local ad-hoc implementation which is a limited version of the same.

In the above context, using filesystem methods might not
be feasible.

To not spam you with replies, I want to thank you here for your review on
all the patches. I will fix the issues in the next version.

Best regards,
Simona

Re: [PATCH v2 5/5] doc: board: nxp: Add Quickboot documentation

[Marek Vasut]

On 3/18/26 3:20 PM, Simona Toaca wrote:

>>>> Can QB data be saved to arbitrary SPI NOR ?
>>>>
>>>
>>> No, this SPI NOR refers to the SPI controller selected by
>>> the CONFIG_SF_DEFAULT_BUS and CONFIG_SF_DEFAULT_CS options.
>>
>> Maybe the user would want to make a backup of the QB data to another SPI NOR
>> (e.g. if the device has two, which is not unheard of) ? They should not be
>> prevented from doing so.
>>
>>> The point is that the device we save the data to is also an
>>> option to boot from, otherwise the qb data is useless.
>>
>> The QB data could be stored for backup purposes, that is not useless.
>>
> There is one thing that might make this clearer, and I forgot to
> mention it explicitly:
> 
> The get_qbdata_offset method searches for a 64KB hole in the bootloader,
> so it can save the data there. OEI knows that's where the data
> should be saved. On an arbitrary non-boot device, or, on a filesystem
> partition, the hole would not exist. It is mkimage that puts that
> hole in the bootloader, and U-Boot explicitly checks for container
> header tags.

Uh, is there any danger that this might accidentally corrupt the 
surrounding bootloader ?

> If one wants to store the data elsewhere, they could use
> the generic fs methods to make the backup wherever they want, and re-load
> it themselves to the QB_SAVED_STATE_BASE address, so that U-Boot can save
> it from there.

It would be good to document this mechanism, esp. how to locate the QB 
data in RAM and how much data have to be saved into backup storage.

>>> And the
>>> board will boot from the selected configs if one boots from spi.
>> Please see above.
>>
>>>> Looking at this, why can this QB not use the same interface to select
>>>> arbitrary block storage device/partition/... that e.g. FS_GENERIC does use ?
>>>
>>> There is a limited list of boot devices. By keeping things the way they are
>>> we limit the usage of this command to the actually useful boot devices (and
>>> that we also test this command on).
>>
>> I would argue that it is better to use generic code instead of hand-writing
>> local ad-hoc implementation which is a limited version of the same.
> 
> In the above context, using filesystem methods might not
> be feasible.
Look at cmd/blk_common.c , that is not filesystem, that is generic 
interface for block devices.

Re: [PATCH v2 5/5] doc: board: nxp: Add Quickboot documentation

[Simona Toaca]

Hi,

On Wed, Mar 18, 2026 at 09:32:33PM +0100, Marek Vasut wrote:
> On 3/18/26 3:20 PM, Simona Toaca wrote:
> 
> > > > > Can QB data be saved to arbitrary SPI NOR ?
> > > > > 
> > > > 
> > > > No, this SPI NOR refers to the SPI controller selected by
> > > > the CONFIG_SF_DEFAULT_BUS and CONFIG_SF_DEFAULT_CS options.
> > > 
> > > Maybe the user would want to make a backup of the QB data to another SPI NOR
> > > (e.g. if the device has two, which is not unheard of) ? They should not be
> > > prevented from doing so.
> > > 
> > > > The point is that the device we save the data to is also an
> > > > option to boot from, otherwise the qb data is useless.
> > > 
> > > The QB data could be stored for backup purposes, that is not useless.
> > > 
> > There is one thing that might make this clearer, and I forgot to
> > mention it explicitly:
> > 
> > The get_qbdata_offset method searches for a 64KB hole in the bootloader,
> > so it can save the data there. OEI knows that's where the data
> > should be saved. On an arbitrary non-boot device, or, on a filesystem
> > partition, the hole would not exist. It is mkimage that puts that
> > hole in the bootloader, and U-Boot explicitly checks for container
> > header tags.
> 
> Uh, is there any danger that this might accidentally corrupt the surrounding
> bootloader ?
> 

No, the space in the bootloader is exactly 64K, and memcpy uses a fixed size,
sizeof(struct ddrphy_qb_state), which is less that 64K.

> > If one wants to store the data elsewhere, they could use
> > the generic fs methods to make the backup wherever they want, and re-load
> > it themselves to the QB_SAVED_STATE_BASE address, so that U-Boot can save
> > it from there.
> 
> It would be good to document this mechanism, esp. how to locate the QB data
> in RAM and how much data have to be saved into backup storage.
> 

I don't recommend anyone does this sort of backup, although it is possible.

I don't see why one would do that since OEI runs training and the data can be
saved then. The data is board, ddr configuration and firmare specific,
and I wouldn't want to encourage someone to think they could save it and re-use it
just like that. Why not re-run OEI and save the data (by using SPL_IMX_QB, for
example, which does it automatically)? Or run the training multiple times and use
the cmdline qb command to save to all the boot devices?

> > > > And the
> > > > board will boot from the selected configs if one boots from spi.
> > > Please see above.
> > > 
> > > > > Looking at this, why can this QB not use the same interface to select
> > > > > arbitrary block storage device/partition/... that e.g. FS_GENERIC does use ?
> > > > 
> > > > There is a limited list of boot devices. By keeping things the way they are
> > > > we limit the usage of this command to the actually useful boot devices (and
> > > > that we also test this command on).
> > > 
> > > I would argue that it is better to use generic code instead of hand-writing
> > > local ad-hoc implementation which is a limited version of the same.
> > 
> > In the above context, using filesystem methods might not
> > be feasible.
> Look at cmd/blk_common.c , that is not filesystem, that is generic interface
> for block devices.

I looked through the code and it seems that the driver for the storage device
has to call blk_create_device in order to be able to use the blk_* commands
later. MMC indeed does that, and some other drivers. For SPI NAND I could find
that mtd_bind is called and that calls blk_create_device. But for SPI NOR there
is no such thing.

Is there something I missed?

My implementation is also aligned with what happens in SPL for parsing the
image containers (see arch/arm/mach-imx/image-container.c).

Regards,
Simona

Re: [PATCH v2 5/5] doc: board: nxp: Add Quickboot documentation

[Marek Vasut]

On 3/19/26 4:17 PM, Simona Toaca wrote:

Hi,

>>> If one wants to store the data elsewhere, they could use
>>> the generic fs methods to make the backup wherever they want, and re-load
>>> it themselves to the QB_SAVED_STATE_BASE address, so that U-Boot can save
>>> it from there.
>>
>> It would be good to document this mechanism, esp. how to locate the QB data
>> in RAM and how much data have to be saved into backup storage.
>>
> 
> I don't recommend anyone does this sort of backup, although it is possible.

I think I know of at least one such use case.

> I don't see why one would do that since OEI runs training and the data can be
> saved then. The data is board, ddr configuration and firmare specific,
> and I wouldn't want to encourage someone to think they could save it and re-use it
> just like that. Why not re-run OEI and save the data (by using SPL_IMX_QB, for
> example, which does it automatically)? Or run the training multiple times and use
> the cmdline qb command to save to all the boot devices?

Consider a device that is trained in factory, at 25C. The training data 
are saved, and backed up. Then, due to some sort of training data 
corruption, a slow retraining happens in field at unknown and possibly 
very high or very low temperature, which could lead to low quality QB 
data. Worse, the next time the device starts, the conditions can be the 
exact opposite, very low or very high temperature and the DRAM may start 
to suffer from bitflips because of that. In this deployment, it is 
better to use the factory QB data from backup than to rely on the 
in-field training.

>>>>> And the
>>>>> board will boot from the selected configs if one boots from spi.
>>>> Please see above.
>>>>
>>>>>> Looking at this, why can this QB not use the same interface to select
>>>>>> arbitrary block storage device/partition/... that e.g. FS_GENERIC does use ?
>>>>>
>>>>> There is a limited list of boot devices. By keeping things the way they are
>>>>> we limit the usage of this command to the actually useful boot devices (and
>>>>> that we also test this command on).
>>>>
>>>> I would argue that it is better to use generic code instead of hand-writing
>>>> local ad-hoc implementation which is a limited version of the same.
>>>
>>> In the above context, using filesystem methods might not
>>> be feasible.
>> Look at cmd/blk_common.c , that is not filesystem, that is generic interface
>> for block devices.
> 
> I looked through the code and it seems that the driver for the storage device
> has to call blk_create_device in order to be able to use the blk_* commands
> later. MMC indeed does that, and some other drivers. For SPI NAND I could find
> that mtd_bind is called and that calls blk_create_device. But for SPI NOR there
> is no such thing.
> 
> Is there something I missed?

All block devices can (and should) use the unified block device 
accessors, otherwise this will only add yet another ad-hoc block device 
handling. MTD devices may have to be special cased.

Re: [PATCH v2 5/5] doc: board: nxp: Add Quickboot documentation

[Simona Toaca]

Hi,

On Thu, Mar 19, 2026 at 07:02:58PM +0100, Marek Vasut wrote:
> On 3/19/26 4:17 PM, Simona Toaca wrote:
> 
> Hi,
> 
> > > > If one wants to store the data elsewhere, they could use
> > > > the generic fs methods to make the backup wherever they want, and re-load
> > > > it themselves to the QB_SAVED_STATE_BASE address, so that U-Boot can save
> > > > it from there.
> > > 
> > > It would be good to document this mechanism, esp. how to locate the QB data
> > > in RAM and how much data have to be saved into backup storage.
> > > 
> > 
> > I don't recommend anyone does this sort of backup, although it is possible.
> 
> I think I know of at least one such use case.
> 
> > I don't see why one would do that since OEI runs training and the data can be
> > saved then. The data is board, ddr configuration and firmare specific,
> > and I wouldn't want to encourage someone to think they could save it and re-use it
> > just like that. Why not re-run OEI and save the data (by using SPL_IMX_QB, for
> > example, which does it automatically)? Or run the training multiple times and use
> > the cmdline qb command to save to all the boot devices?
> 
> Consider a device that is trained in factory, at 25C. The training data are
> saved, and backed up. Then, due to some sort of training data corruption, a
> slow retraining happens in field at unknown and possibly very high or very
> low temperature, which could lead to low quality QB data. Worse, the next
> time the device starts, the conditions can be the exact opposite, very low
> or very high temperature and the DRAM may start to suffer from bitflips
> because of that. In this deployment, it is better to use the factory QB data
> from backup than to rely on the in-field training.
>
Low/High temperatures don't generate 'low quality' data, just data
more suitable for that temperature point. If there are issues with the
DRAM, one can erase the data themselves, using 'qb erase', to force a
new training. This way, the data is collected under the current operating
conditions.

If there are any PHY firmware updates, or changes in DDR configuration,
performing training again is necessary, cannot use the 'backup' data.

If the concern also includes the training time, if the data gets
corrupted, the training will happen regardless.

I am not saying it can't be done, I am just saying that for things
to work the proper way, I encourage the usage of 'qb save/erase',
as I said in my previous reply.

[ ... ]

Kind regards,
Simona

Re: [PATCH v2 5/5] doc: board: nxp: Add Quickboot documentation

[Marek Vasut]

On 3/20/26 4:29 PM, Simona Toaca wrote:

Hello Simona,

>> Consider a device that is trained in factory, at 25C. The training data are
>> saved, and backed up. Then, due to some sort of training data corruption, a
>> slow retraining happens in field at unknown and possibly very high or very
>> low temperature, which could lead to low quality QB data. Worse, the next
>> time the device starts, the conditions can be the exact opposite, very low
>> or very high temperature and the DRAM may start to suffer from bitflips
>> because of that. In this deployment, it is better to use the factory QB data
>> from backup than to rely on the in-field training.
>>
> Low/High temperatures don't generate 'low quality' data, just data
> more suitable for that temperature point. If there are issues with the
> DRAM, one can erase the data themselves, using 'qb erase', to force a
> new training. This way, the data is collected under the current operating
> conditions.

This may be something a developer may do with a singular devkit, but 
this is not something one can do on a fleet of deployed devices.

In deployment, if the device breaks and cannot automatically recover 
itself, it is broken and useless, and that makes users unhappy. If the 
device becomes unstable, e.g. because it was moved from hot environment 
to cold environment which is the normal mode of operation for some of 
the devices, then it is broken, see above.

The solution is simple -- keep factory produced training data around.

> If there are any PHY firmware updates, or changes in DDR configuration,
> performing training again is necessary, cannot use the 'backup' data.

Not all of these devices are connected to the internet, this is not 
applicable.

> If the concern also includes the training time, if the data gets
> corrupted, the training will happen regardless.

This is also not applicable.

> I am not saying it can't be done, I am just saying that for things
> to work the proper way, I encourage the usage of 'qb save/erase',
> as I said in my previous reply.
What I am saying is, that the user should have a way to save QB data to 
arbitrary location of their choosing. If they then decide to restore 
those QB data in OEI because the original QB data were corrupted, that 
is up to them to patch the OEI. But that option should be available to 
the users, because there is a use case for that. And there is a generic 
way to specify the target storage device, at least for block devices.

[...]