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

[Marek Vasut]

On 4/2/26 11:40 AM, Simona Toaca (OSS) wrote:
> From: Simona Toaca <simona.toaca at 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

On the other ... SoC revisions (not boards) , ... ?

[...]

> +++ b/arch/arm/include/asm/arch-imx9/ddr.h
> @@ -1,6 +1,6 @@
>   /* SPDX-License-Identifier: GPL-2.0+ */
>   /*
> - * Copyright 2022 NXP
> + * Copyright 2022-2026 NXP
>    */
>   
>   #ifndef __ASM_ARCH_IMX8M_DDR_H
> @@ -100,6 +100,52 @@ struct dram_timing_info {
>   
>   extern struct dram_timing_info dram_timing;
>   
> +/* 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 */

Please indent with tabs.

> +	u32 mac[MAC_LENGTH];	  /* For 95A0/1 use mac[0] to keep CRC32 value */


[...]

> +++ b/arch/arm/include/asm/mach-imx/qb.h
> @@ -0,0 +1,15 @@
> +/* 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, bool save);
> +void spl_qb_save(void);

Please call the functions something less opaque than "qb()" , imx_qb_*() 
would alrady be so much better.

[...]

> diff --git a/arch/arm/mach-imx/imx9/qb.c b/arch/arm/mach-imx/imx9/qb.c
> new file mode 100644
> index 00000000000..220545192d4
> --- /dev/null
> +++ b/arch/arm/mach-imx/imx9/qb.c
> @@ -0,0 +1,379 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/**
> + * Copyright 2024-2026 NXP
> + */
> +#include <dm/device-internal.h>
> +#include <dm/uclass.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 <u-boot/crc.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 IMG_FLAGS_IMG_TYPE_MASK   0xF
> +#define IMG_FLAGS_IMG_TYPE(x)     FIELD_GET(IMG_FLAGS_IMG_TYPE_MASK, (x))
> +
> +#define IMG_TYPE_DDR_TDATA_DUMMY  0xD   /* dummy DDR training data image */
> +
> +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_QB_SAVED_STATE_BASE;
> +	size = sizeof(struct ddrphy_qb_state) - sizeof(qb_state->crc);
> +	crc = crc32(0, (u8 *)qb_state->mac, size);
> +
> +	if (!qb_state->crc || crc != qb_state->crc)
> +		return false;
> +
> +	return true;
> +}
> +
> +static unsigned long qb_get_boot_device_offset(void *dev, int dev_type)
> +{
> +	unsigned long offset = 0;
> +	struct mmc *mmc;
> +
> +	switch (dev_type) {
> +	case MMC_DEV:
> +		mmc = dev;
> +
> +		if (IS_SD(mmc) || mmc->part_config == MMCPART_NOAVAILABLE) {
> +			offset = CONTAINER_HDR_MMCSD_OFFSET;

Multiple return in this case will make the function simpler:

return CONTAINER_HDR_MMCSD_OFFSET;

> +		} else {

No need for } else { anymore.

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

return CONTAINER_HDR_EMMC_OFFSET;

> +			else
> +				offset = CONTAINER_HDR_MMCSD_OFFSET;

return CONTAINER_HDR_MMCSD_OFFSET;

> +		}
> +		break;
> +	case QSPI_DEV:
> +		offset = CONTAINER_HDR_QSPI_OFFSET;
> +		break;
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +
> +	return offset;
> +}

[...]

> +static int get_dev_qbdata_offset(void *dev, int dev_type, unsigned long offset,
> +				 u32 *qbdata_offset)
> +{
> +	int ret = -EOPNOTSUPP;
> +	u16 ctnr_hdr_align = CONTAINER_HDR_ALIGNMENT;
> +	void *buf = kmalloc(ctnr_hdr_align, GFP_KERNEL);

Can you use buffer on stack ? u8 buf[CONTAINER_HDR_ALIGNMENT]; ? That 
would allow you to remove all the goto statements.

If not, use malloc() not kmalloc().

> +	if (!buf) {
> +		printf("kmalloc buffer failed\n");

if malloc fails, so will printing, so drop this print and return -ENOMEM 
only.

> +		return -ENOMEM;
> +	}
> +
> +	switch (dev_type) {
> +	case MMC_DEV:
> +		unsigned long count = 0;
> +		struct mmc *mmc = dev;
> +
> +		if (!(CONFIG_IS_ENABLED(DM_MMC) && CONFIG_IS_ENABLED(MMC_WRITE)))
> +			goto get_dev_qbdata_offset_exit;
> +
> +		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");
> +			ret = -EIO;
> +			goto get_dev_qbdata_offset_exit;
> +		}
> +		break;
> +	case QSPI_DEV:
> +		if (!CONFIG_IS_ENABLED(SPI))
> +			goto get_dev_qbdata_offset_exit;
> +
> +		ret = spi_flash_read(dev, offset,
> +				     ctnr_hdr_align, buf);
> +		if (ret) {
> +			printf("Read container header from QSPI failed\n");
> +			ret = -EIO;
> +			goto get_dev_qbdata_offset_exit;
> +		}
> +		break;
> +	default:
> +		printf("Support for device %d not enabled\n", dev_type);
> +		goto get_dev_qbdata_offset_exit;
> +	}
> +
> +	ret = qb_parse_container(buf, qbdata_offset);
> +
> +get_dev_qbdata_offset_exit:
> +	free(buf);
> +
> +	return ret;
> +}

[...]

> +void spl_qb_save(void)
> +{
> +	int dev = spl_boot_device();
> +
> +	/* Save QB data on current boot device */
> +	if (qb(dev, true))

Please call this function something less opaque than "qb()" , pick 
something more descriptive.

> +		printf("QB save failed\n");
> +}
[...]