Re: [PATCH 14/17] spacemit: k1: Add DDR firmware support to SPL

[Yao Zi <me@ziyao.cc>]

(My mail provider complains about the huge list of recipients, so I
reduced it a little.)

On Sat, Jan 17, 2026 at 02:01:48PM -0500, Raymond Mao wrote:
> From: Raymond Mao <raymond.mao@riscstar.com>
> 
> Include DDR initialization firmware in the SPL image. The firmware
> path can be specified via the DDR_FW_FILE environment variable. If
> the firmware is not found, an empty placeholder file is created to
> allow the build to proceed without DDR initialization support.
> 
> Signed-off-by: Raymond Mao <raymond.mao@riscstar.com>
> ---
>  arch/riscv/dts/k1-spl.dts      |  34 ++++++++++++++++++++++++++++++++-
>  board/spacemit/k1/Kconfig      |   8 ++++++++
>  board/spacemit/k1/Makefile     |  19 ++++++++++++++++++
>  board/spacemit/k1/spl.c        |  30 +++++++++++++++++++++++++++++
>  include/configs/k1.h           |   3 +++
>  lib/vendor/spacemit/ddr_fw.bin | Bin 0 -> 19416 bytes

It's unusual to ship binary firmware in U-Boot repository. Please place
the firmware somewhere else, and give a clear instruction in the
board/architecture documentation explaining how to obtain/build it.

>  6 files changed, 93 insertions(+), 1 deletion(-)
>  create mode 100644 lib/vendor/spacemit/ddr_fw.bin

...

> diff --git a/board/spacemit/k1/Kconfig b/board/spacemit/k1/Kconfig
> index 9f9c806d00d..a5fa788f660 100644
> --- a/board/spacemit/k1/Kconfig
> +++ b/board/spacemit/k1/Kconfig
> @@ -15,6 +15,14 @@ config SYS_CONFIG_NAME
>  config TEXT_BASE
>  	default 0x00200000
>  
> +config SPL_DDR_FIRMWARE_OFFSET
> +	hex "DDR firmware offset in SPL image"
> +	depends on SPL
> +	default 0x20000
> +	help
> +	  Offset where DDR firmware should be placed in the SPL
> +	  image.
> +
>  config SPL_OPENSBI_LOAD_ADDR
>  	default 0x00000000
>  
> diff --git a/board/spacemit/k1/Makefile b/board/spacemit/k1/Makefile
> index 7bce47bac8c..ebe6e55867c 100644
> --- a/board/spacemit/k1/Makefile
> +++ b/board/spacemit/k1/Makefile
> @@ -5,3 +5,22 @@
>  
>  obj-y := board.o
>  obj-$(CONFIG_SPL_BUILD) += spl.o
> +
> +DDR_FW_SRC ?= $(DDR_FW_FILE)
> +FW_TARGET = $(srctree)/lib/vendor/spacemit/ddr_fw.bin
> +
> +DDR_FW_HEADER = $(objtree)/include/generated/ddr_fw_info.h
> +
> +$(obj)/spl.o: $(DDR_FW_HEADER)
> +
> +$(DDR_FW_HEADER): $(FW_TARGET)
> +	@echo "/* DDR firmware info - $$(date) */" > $@
> +	@if [ -f "$(FW_TARGET)" ]; then \
> +		SIZE=$$(stat -c%s "$(FW_TARGET)" 2>/dev/null || echo 0); \
> +	else \
> +		SIZE=0; \
> +	fi; \
> +	echo "#define DDR_FW_FILE_SIZE	$$SIZE" >> $@
> +	@echo "/* Note: Update ADDR if binman layout changes */" >> $@
> +
> +clean-files += $(FW_TARGET) $(DDR_FW_HEADER)

Please make use of existing binman[1] facibilities to avoid the extra
Kconfig and Makefile pieces. binman_sym() could be used for obtaining
both size and offset of a binman entry.

> diff --git a/board/spacemit/k1/spl.c b/board/spacemit/k1/spl.c
> index 6fe064bd430..54bad9000fe 100644
> --- a/board/spacemit/k1/spl.c
> +++ b/board/spacemit/k1/spl.c
> @@ -6,10 +6,12 @@
>  #include <asm/io.h>
>  #include <clk.h>
>  #include <clk-uclass.h>
> +#include <cpu_func.h>
>  #include <configs/k1.h>
>  #include <dm/device.h>
>  #include <dm/uclass.h>
>  #include <dt-bindings/pinctrl/k1-pinctrl.h>
> +#include <generated/ddr_fw_info.h>
>  #include <i2c.h>
>  #include <linux/delay.h>
>  #include <log.h>
> @@ -115,6 +117,33 @@ void serial_early_init(void)
>  		panic("Serial uclass init failed: %d\n", ret);
>  }
>  
> +/* Load DDR training firmware */
> +int init_ddr_firmware(void)
> +{
> +	void __iomem *src, *dst;
> +	unsigned long size;
> +
> +	src = (void __iomem *)(CONFIG_SPL_TEXT_BASE +
> +			       CONFIG_SPL_DDR_FIRMWARE_OFFSET);
> +	dst = (void __iomem *)(DDR_TRAINING_DATA_BASE);
> +	memcpy(dst, src, DDR_FW_FILE_SIZE);
> +	size = round_up(DDR_FW_FILE_SIZE, 64);
> +	flush_dcache_range((u32)(u64)dst, (u32)(u64)dst + size);
> +	return 0;
> +}
> +
> +void ddr_early_init(void)
> +{
> +	void __iomem *addr;
> +
> +	init_ddr_firmware();
> +	addr = (void __iomem *)(CONFIG_SPL_TEXT_BASE +
> +				CONFIG_SPL_DDR_FIRMWARE_OFFSET);
> +	// verify DDR firmware header

However, you don't verify it, but only print some of the information.
The comment seems misleading and extra.

> +	log_info("[0x%x]:0x%x, firmware size:%d\n",
> +		 (uint)(u64)addr, readl(addr), DDR_FW_FILE_SIZE);

Regards,
Yao Zi