Hi Quentin, On Wed, Jan 14, 2026 at 10:05 PM Quentin Schulz <[email protected]> wrote: > > Hi Alexey, > > On 1/8/26 6:50 PM, Alexey Charkov wrote: > > Add the required architecture-specific lookups to enable U-boot SPL to > > load images from UFS storage devices on Rockchip RK3576, which has a > > boot ROM capable of loading the SPL image from UFS. > > > > This also requires the DM_RESET framework to be available in SPL, as > > the UFS driver uses it. > > If it is a hard dependency, why is it not specified as such in the > symbol directly instead of relying on imply's at the ARCH level? This > will also be "painful" once a new Rockchip SoC has UFS support as we'll > need to forget not to add those imply.
Fair enough, will convert it into a "select" under Rockchip UFS, thanks for pointing it out! > > Signed-off-by: Alexey Charkov <[email protected]> > > --- > > arch/arm/dts/rk3576-u-boot.dtsi | 13 ++++++++++++- > > arch/arm/include/asm/arch-rockchip/bootrom.h | 1 + > > arch/arm/mach-rockchip/Kconfig | 3 +++ > > arch/arm/mach-rockchip/rk3576/rk3576.c | 1 + > > arch/arm/mach-rockchip/spl-boot-order.c | 14 ++++++++++++++ > > drivers/reset/Kconfig | 9 +++++++++ > > drivers/reset/Makefile | 2 +- > > 7 files changed, 41 insertions(+), 2 deletions(-) > > > > diff --git a/arch/arm/dts/rk3576-u-boot.dtsi > > b/arch/arm/dts/rk3576-u-boot.dtsi > > index dc3771b556a3..84d9f39fd407 100644 > > --- a/arch/arm/dts/rk3576-u-boot.dtsi > > +++ b/arch/arm/dts/rk3576-u-boot.dtsi > > @@ -12,7 +12,7 @@ > > }; > > > > chosen { > > - u-boot,spl-boot-order = "same-as-spl", &sdmmc, &sdhci; > > + u-boot,spl-boot-order = "same-as-spl", &sdmmc, &sdhci, &ufshc; > > }; > > > > dmc { > > @@ -81,6 +81,12 @@ > > bootph-some-ram; > > }; > > > > +&gpio4 { > > + /* Used for resetting the UFS device during initialization */ > > + bootph-pre-ram; > > + bootph-some-ram; > > +}; > > + > > Is it always GPIO4 for the reset GPIO for UFS? Seems like something that > should probably be put in a board -u-boot.dtsi instead? It is in fact always GPIO4 pin D0. It's multiplexed to a somehow-hardware-controlled pin inconspicuously called UFS_RSTn within VCCIO7_IOC_GPIOD_IOMUX_SEL_L, but I couldn't find how it's supposed to operate in a hardware-controlled mode (it's not described in the TRM). Mainline Linux does the same thing as I do here (GPIO mode [1]), while Rockchip's own U-boot code switches the pin to the hardware-controlled mode [2] and never touches anything about device resets from then on - which feels like a potentially fragile black box to me. Hardware controlled mode would also deviate from the upstream DT binding, as it stipulates a GPIO for device resets [3]. [1] https://github.com/torvalds/linux/blob/master/drivers/ufs/host/ufs-rockchip.c#L220-L231 [2] https://github.com/rockchip-linux/u-boot/blob/next-dev/drivers/ufs/ufs-rockchip.c#L222 [3] https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/ufs/rockchip%2Crk3576-ufshc.yaml#L53-L58 > > &ioc_grf { > > bootph-all; > > }; > > @@ -176,6 +182,11 @@ > > bootph-pre-ram; > > }; > > > > +&ufshc { > > + bootph-pre-ram; > > + bootph-some-ram; > > +}; > > + > > &xin24m { > > bootph-all; > > }; > > diff --git a/arch/arm/include/asm/arch-rockchip/bootrom.h > > b/arch/arm/include/asm/arch-rockchip/bootrom.h > > index b15938c021d6..f9ecb6858f04 100644 > > --- a/arch/arm/include/asm/arch-rockchip/bootrom.h > > +++ b/arch/arm/include/asm/arch-rockchip/bootrom.h > > @@ -51,6 +51,7 @@ enum { > > BROM_BOOTSOURCE_SPINOR = 3, > > BROM_BOOTSOURCE_SPINAND = 4, > > BROM_BOOTSOURCE_SD = 5, > > + BROM_BOOTSOURCE_UFS = 7, > > Hopefully this will be the same value for all upcoming SoCs with UFS > (and not used for something else :) ). Well it's hard to guarantee (otherwise Jonas wouldn't have had to add a translation function [4] for the IDs that changed in RK3576). Maybe at some point we'll have to switch to a lookup table per each SoC variant if these keep changing, but thankfully that point is not today :) [4] https://source.denx.de/u-boot/u-boot/-/commit/99a9b6ba6cf1c2f376e6f0ac3e6a2ced6c6addd4 > > BROM_BOOTSOURCE_I2C = 8, > > BROM_BOOTSOURCE_SPI = 9, > > BROM_BOOTSOURCE_USB = 10, > > diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig > > index 92bb4aa62f11..483a2f7c4ab2 100644 > > --- a/arch/arm/mach-rockchip/Kconfig > > +++ b/arch/arm/mach-rockchip/Kconfig > > @@ -436,6 +436,9 @@ config ROCKCHIP_RK3576 > > imply SPL_ATF_NO_PLATFORM_PARAM if SPL_ATF > > imply SPL_CLK > > imply SPL_DM_SEQ_ALIAS > > + imply SPL_DM_GPIO if UFS_ROCKCHIP && SPL_UFS_SUPPORT > > + imply SPL_DM_RESET if UFS_ROCKCHIP && SPL_UFS_SUPPORT > > + imply SPL_GPIO if UFS_ROCKCHIP && SPL_UFS_SUPPORT > > imply SPL_FIT_SIGNATURE > > imply SPL_LOAD_FIT > > imply SPL_MMC_HS200_SUPPORT if SPL_MMC && MMC_HS200_SUPPORT > > diff --git a/arch/arm/mach-rockchip/rk3576/rk3576.c > > b/arch/arm/mach-rockchip/rk3576/rk3576.c > > index a1e8a7572fa4..46cf60dc77c7 100644 > > --- a/arch/arm/mach-rockchip/rk3576/rk3576.c > > +++ b/arch/arm/mach-rockchip/rk3576/rk3576.c > > @@ -46,6 +46,7 @@ const char * const boot_devices[BROM_LAST_BOOTSOURCE + 1] > > = { > > [BROM_BOOTSOURCE_FSPI0] = "/soc/spi@2a340000/flash@0", > > [BROM_BOOTSOURCE_FSPI1_M1] = "/soc/spi@2a300000/flash@0", > > [BROM_BOOTSOURCE_SD] = "/soc/mmc@2a310000", > > + [BROM_BOOTSOURCE_UFS] = "/soc/ufshc@2a2d0000", > > }; > > > > static struct mm_region rk3576_mem_map[] = { > > diff --git a/arch/arm/mach-rockchip/spl-boot-order.c > > b/arch/arm/mach-rockchip/spl-boot-order.c > > index 6572dde29f65..d2dd5e10935f 100644 > > --- a/arch/arm/mach-rockchip/spl-boot-order.c > > +++ b/arch/arm/mach-rockchip/spl-boot-order.c > > @@ -76,6 +76,9 @@ static int spl_node_to_boot_device(int node) > > if (!uclass_find_device_by_of_offset(UCLASS_SPI_FLASH, node, &parent)) > > return BOOT_DEVICE_SPI; > > > > + if (!uclass_find_device_by_of_offset(UCLASS_UFS, node, &parent)) > > + return BOOT_DEVICE_UFS; > > + > > return -1; > > } > > > > @@ -231,6 +234,17 @@ int spl_decode_boot_device(u32 boot_device, char *buf, > > size_t buflen) > > return -ENODEV; > > } > > > > + if (boot_device == BOOT_DEVICE_UFS) { > > + ret = uclass_find_device(UCLASS_UFS, 0, &dev); > > + if (ret) { > > + debug("%s: could not find device for UFS: %d\n", > > + __func__, ret); > > + return ret; > > + } > > + > > + return ofnode_get_path(dev_ofnode(dev), buf, buflen); > > + } > > + > > #if CONFIG_IS_ENABLED(BLK) > > dev_num = (boot_device == BOOT_DEVICE_MMC1) ? 0 : 1; > > > > diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig > > index 74c267dfc4e5..d35f3904e404 100644 > > --- a/drivers/reset/Kconfig > > +++ b/drivers/reset/Kconfig > > @@ -99,6 +99,15 @@ config RESET_ROCKCHIP > > though is that some reset signals, like I2C or MISC reset multiple > > devices. > > > > +config SPL_RESET_ROCKCHIP > > + bool "SPL reset controller driver for Rockchip SoCs" > > + depends on SPL_DM_RESET && ARCH_ROCKCHIP && CLK > > + default y > > + help > > + Support for the reset controller on Rockchip SoCs in SPL. Select > > this > > + if you observe any reset-related warnings or errors when booting > > SPL, > > + such as when using UFS storage > > + > > Separate patch for this please. Ack > > config RESET_HSDK > > bool "Synopsys HSDK Reset Driver" > > depends on DM_RESET && TARGET_HSDK > > diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile > > index ee5b009d1341..dc7d3b610e6e 100644 > > --- a/drivers/reset/Makefile > > +++ b/drivers/reset/Makefile > > @@ -16,7 +16,7 @@ obj-$(CONFIG_RESET_BCM6345) += reset-bcm6345.o > > obj-$(CONFIG_RESET_UNIPHIER) += reset-uniphier.o > > obj-$(CONFIG_RESET_AST2500) += reset-ast2500.o > > obj-$(CONFIG_RESET_AST2600) += reset-ast2600.o > > -obj-$(CONFIG_RESET_ROCKCHIP) += reset-rockchip.o rst-rk3528.o rst-rk3576.o > > rst-rk3588.o > > +obj-$(CONFIG_$(PHASE_)RESET_ROCKCHIP) += reset-rockchip.o rst-rk3528.o > > rst-rk3576.o rst-rk3588.o > > Separate patch for this please. Ack Thanks a lot, Alexey
