On Fri, Jan 16, 2026 at 6:50 PM Quentin Schulz <[email protected]> wrote: > > Hi Alexey, > > On 1/16/26 3:29 PM, Alexey Charkov wrote: > > On Fri, Jan 16, 2026 at 2:50 PM Quentin Schulz <[email protected]> > > wrote: > >> > >> Hi Alexey, > >> > >> On 1/16/26 11:08 AM, Alexey Charkov wrote: > >>> 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: > [...] > >>>>> +&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 > >> > >> This is probably the typical Rockchip BS :) It is the "same" for SD CD > >> or WP pins. They have a hardware-controlled mode for pin A which is > >> usually a black box for SW (nothing to do, it "works"). But nothing > >> prevents you from using another GPIO for that (just have to be careful > >> about the voltage level I guess, here it seems to be 1.2V for the > >> domain, which isn't that usual I guess. > > > > I bet that the boot ROM might get picky there, and only work with > > GPIO4_D0. But if the UFS chip is not meant to be used as the primary > > boot device - sure, can't see why not. On the other hand, there's not > > much direct gain for board designers from using a different pin, and > > people tend to just stick with Rockchip reference schematics unless > > something different is explicitly required for their design, so my > > money is on "it will be GPIO4 for all RK3576 boards". > > > > Extra consideration in the same stream: the pin itself is defined in > > rk3576.dtsi upstream, not in board-specific .dts files. > > > > OK, I see that rk3576.dtsi specifies the reset-gpios here. I guess it > should be seen as a default value, like we have for pinmuxes. Also, this > is lacking a pinmux for the GPIO, so there's no guarantee the pin will > be muxed into the GPIO function before you control it.
Indeed. This must have been overlooked upon addition, given that GPIO function is supposed to be the power-on default for this pin. So in the end it's either reconfigured by the boot ROM to the hardware-controlled reset mode (then it leaves the UFS controller in a magically working state), or it's left at the power-on default GPIO state - end result is a working UFS controller in both cases, but possibly with traces of dark magic. Sounds like a patch to the upstream .dtsi is warranted - will do that. > >>> controlled mode would also deviate from the upstream DT binding, as it > >>> stipulates a GPIO for device resets [3]. > >>> > >> > >> Not asking for this, rather wondering if we should really have this at > >> the SoC level rather than the board level. If it can be any GPIO why > >> forcing all boards to have GPIO4 bank "enabled" in SPL + U-Boot proper > >> before reloc. It isn't the first SoC to have that (also see all SoC > >> u-boot.dtsi having the UART controller from the reference design > >> "enabled"), and I don't think it's planned for us to have a product on > >> it, so... not a blocker from my side :) > > > > I believe that the boards which don't use UFS for early boot will just > > compile it out, together with the respective GPIO code. Then the only > > downside for them is a marginally larger SPL DTB - but not on a scale > > that matters for RK3576. And those that use UFS for early boot will > > likely want it to be on the pin expected by the boot ROM, then having > > the prerequisites already defined makes life easier for them (and it's > > not much different from having e.g. FSPI0/1 and eMMC enabled by > > default even for boards that don't use them). > > > > My hunch is the BootROM may not even attempt to reset the UFS chip and > rely on it having lost power (see different ways for the PMIC to reset > its regulators when it's itself reset in SW). But I have nothing to > prove this :) I took my JTAG out. Boot ROM at its code address 0x6448 writes 0xff0011 to 0x2604b398, where: - 0x2604b398 is VCCIO7_IOC_GPIOD4_IOMUX_SEL_L - 0xff << 16 is the write mask - 0x10 is GPIO4_D1 mux (=UFS_REFCLK, not GPIO mode) - 0x01 is GPIO4_D0 mux (=UFS_RSTn, not GPIO mode) Further down the line at its code address 0x6480 it writes 0x100010 to 0x2604b400, where: - 0x2604b400 is VCCIO7_IOC_XIN_UFS_CON - 0x10 << 16 is the write mask - 0x10 is ufs_rstn_out a.k.a. UFS reset IO configuration (presumably enabling the hardware-controlled device reset mode) This only applies to when UFS is identified as the boot storage - otherwise the registers stay at their power-on reset defaults (i.e. zeros for these fields) > >>> [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 > >>> :) > >>> > >> > >> See > >> https://lore.kernel.org/devicetree-spec/[email protected]/ > > > > Ooooh that sounds like a discussion that can potentially stretch out > > for a long time. I'd much rather use a value that works for the only > > Anything to be added to the dt spec is taking months if not year, even > when agreed upon. > > > chip where it matters now (RK3576), then solve the problem whenever it > > arises (if it does). After all, bootrom.h is not ABI, and DT is, so it > > is much easier to fix up bootrom.h without stepping on too many toes. > > > > I'm saying this will solve the problem, not that we have to rely on it > now :) > > > Furthermore, I'm not even sure that /chosen/bootsource can address the > > It's not /chosen/bootsource, this has merged already in the spec and > won't be usable for this mapping (lots to read but it's explained > somewhere in the thread). > > > issue here, because boot ROM's won't magically start passing DT > > phandles to the bootloader, so there will still be an ID->node lookup > > table somewhere in early code (with potentially per-SoC overrides). > > No, that's the point of the thread I linked. The idea is to provide in > DTS the mapping between the value in the register set by the BootROM to > a controller/flash node somehow (via phandle or something else) so that > we don't have to have this table in code at all. After all, this is > hardware description (albeit based on BootROM "API" but that cannot > change anyway, it's silicon (if it does, good luck all may the odds be > in your favor)) so there's no reason for all bootloaders to have to > reinvent their own sauce (see barebox doing something different than > U-Boot there). > > I read the BootROM register value, I read the DT property with the > mapping, then I know which device it is. Bonus point, you don't need to > keep the DT node path in sync in your table, because the mapping is in > the same DT as what it refers to. > > This is not for now (and I should probably revive the topic), just > wanted to tell you there's another way to do that and we should probably > aim at doing that in the mid/long term (and then all this in-code > mapping won't need to maintained anymore, and there won't be conflicts > between SoCs having the same value for different meanings, or differnet > values for the same meaning). Oh, I got it now. Yes, that sounds like a beautiful longer-term solution, separating data from code cleanly and avoiding if-then-else trees. Best regards, Alexey
