[PATCH v4 1/3] spl: Make UFS available for SPL builds

[Quentin Schulz]

Hi Alexey,

On 1/8/26 6:49 PM, Alexey Charkov wrote:
> Add minimal infrastructure to build SPL images with support for UFS
> storage devices. This also pulls in SCSI support and charset functions,
> which are dependencies of the UFS code.
> 
> With this, only a fixed offset is supported for loading the next image,
> which should be specified in CONFIG_SPL_UFS_RAW_U_BOOT_SECTOR as the
> number of 4096-byte sectors into the UFS block device.
> 
> Signed-off-by: Alexey Charkov <alchark at gmail.com>
> ---
>   MAINTAINERS                |  1 +
>   arch/arm/include/asm/spl.h |  1 +
>   common/spl/Kconfig         | 29 +++++++++++++++++++++++++++
>   common/spl/Makefile        |  1 +
>   common/spl/spl_ufs.c       | 49 ++++++++++++++++++++++++++++++++++++++++++++++
>   drivers/Makefile           |  1 +
>   drivers/scsi/Makefile      |  3 +++
>   lib/Makefile               |  1 +
>   8 files changed, 86 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 27ce73d83f48..ca31e57f018a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1861,6 +1861,7 @@ M:	Neil Armstrong <neil.armstrong at linaro.org>
>   M:	Bhupesh Sharma <bhupesh.linux at gmail.com>
>   M:	Neha Malcom Francis <n-francis at ti.com>
>   S:	Maintained
> +F:	common/spl/spl_ufs.c
>   F:	drivers/ufs/
>   
>   UPL
> diff --git a/arch/arm/include/asm/spl.h b/arch/arm/include/asm/spl.h
> index ee79a19c05c9..dd462ea6ad82 100644
> --- a/arch/arm/include/asm/spl.h
> +++ b/arch/arm/include/asm/spl.h
> @@ -30,6 +30,7 @@ enum {
>   	BOOT_DEVICE_XIP,
>   	BOOT_DEVICE_BOOTROM,
>   	BOOT_DEVICE_SMH,
> +	BOOT_DEVICE_UFS,
>   	BOOT_DEVICE_NONE
>   };
>   #endif
> diff --git a/common/spl/Kconfig b/common/spl/Kconfig
> index 4f4119f5806c..6a82260dd010 100644
> --- a/common/spl/Kconfig
> +++ b/common/spl/Kconfig
> @@ -1612,6 +1612,35 @@ config SPL_THERMAL
>   	  automatic power-off when the temperature gets too high or low. Other
>   	  devices may be discrete but connected on a suitable bus.
>   
> +config SPL_UFS_SUPPORT
> +	bool "Support loading from UFS"
> +	select SPL_LOAD_BLOCK

Doesn;t this depend on CONFIG_SCSI as well? Like it does nothing without 
it (does it even compile?), see changes you made to drivers/scsi/Makefile.

> +	help
> +	  Enable support for UFS in SPL. This allows
> +	  use of UFS devices such as hard drives and flash drivers for
> +	  loading U-Boot.
> +
> +config SPL_UFS_RAW_U_BOOT_DEVNUM
> +	int "SCSI device number of the UFS device to load U-Boot from"
> +	depends on SPL_UFS_SUPPORT
> +	default 0
> +	help
> +	  UFS devices are usually configured with multiple LUNs, which present
> +	  themselves as sequentially numbered SCSI devices. Usually one would
> +	  get a default LUN 0 taking up most of the space on the device, with
> +	  a number of smaller LUNs following it. This option controls which of
> +	  them the SPL will attempt to load U-Boot from. Note that this is the
> +	  SCSI device number, which might differ from the UFS LUN if you have
> +	  multiple SCSI devices attached and recognized by the SPL.
> +

I'm not verse in SCSI... Is the device number guaranteed to be stable 
across reboots? Is it probe order dependent? If that's the case, this is 
not a useful symbol and we should be trying to figure out a way to make 
it stable (via aliases for example, or using another mechanism).

> +config SPL_UFS_RAW_U_BOOT_SECTOR
> +	hex "Address on the UFS to load U-Boot from"
> +	depends on SPL_UFS_SUPPORT
> +	default 0x800 if ARCH_ROCKCHIP
> +	help
> +	  Address on the block device to load U-Boot from,

s/,/./

> +	  Units: UFS sectors (1 sector = 4096 bytes).
> +
>   config SPL_WATCHDOG
>   	bool "Support watchdog drivers"
>   	imply SPL_WDT if !HW_WATCHDOG
> diff --git a/common/spl/Makefile b/common/spl/Makefile
> index 4c9482bd3096..e18f3cf09484 100644
> --- a/common/spl/Makefile
> +++ b/common/spl/Makefile
> @@ -37,6 +37,7 @@ obj-$(CONFIG_$(PHASE_)DFU) += spl_dfu.o
>   obj-$(CONFIG_$(PHASE_)SPI_LOAD) += spl_spi.o
>   obj-$(CONFIG_$(PHASE_)RAM_SUPPORT) += spl_ram.o
>   obj-$(CONFIG_$(PHASE_)USB_SDP_SUPPORT) += spl_sdp.o
> +obj-$(CONFIG_$(PHASE_)UFS_SUPPORT) += spl_ufs.o
>   endif
>   
>   obj-$(CONFIG_$(PHASE_)UPL) += spl_upl.o
> diff --git a/common/spl/spl_ufs.c b/common/spl/spl_ufs.c
> new file mode 100644
> index 000000000000..6b035db13c41
> --- /dev/null
> +++ b/common/spl/spl_ufs.c
> @@ -0,0 +1,49 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * (C) Copyright 2025 Alexey Charkov <alchark at gmail.com>
> + */
> +
> +#include <spl.h>
> +#include <spl_load.h>
> +#include <scsi.h>
> +#include <errno.h>
> +#include <image.h>
> +#include <linux/compiler.h>
> +#include <log.h>
> +
> +static ulong spl_ufs_load_read(struct spl_load_info *load, ulong off, ulong size, void *buf)
> +{
> +	struct blk_desc *bd = load->priv;
> +	lbaint_t sector = off >> bd->log2blksz;
> +	lbaint_t count = size >> bd->log2blksz;
> +
> +	return blk_dread(bd, sector, count, buf) << bd->log2blksz;
> +}
> +
> +static int spl_ufs_load_image(struct spl_image_info *spl_image,
> +			      struct spl_boot_device *bootdev)
> +{
> +	int err = -ENOSYS;
> +	struct blk_desc *bd;
> +	struct spl_load_info load;
> +	int devnum = CONFIG_SPL_UFS_RAW_U_BOOT_DEVNUM;
> +	unsigned long sector = CONFIG_SPL_UFS_RAW_U_BOOT_SECTOR;

Reverse christmas tree here usually (so longest line at the top).

> +
> +	/* try to recognize storage devices immediately */
> +	scsi_scan(false);
> +	bd = blk_get_devnum_by_uclass_id(UCLASS_SCSI, devnum);
> +	if (!bd)
> +		return -ENODEV;
> +
> +	spl_load_init(&load, spl_ufs_load_read, bd, bd->blksz);
> +	err = spl_load(spl_image, bootdev, &load, 0, sector << bd->log2blksz);
> +	if (err) {
> +		puts("spl_ufs_load_image: ufs block read error\n");
> +		log_debug("(error=%d)\n", err);

Why both a print and a log_debug?

Cheers,
Quentin