Hi Quentin, On 2024-06-17 16:03, Quentin Schulz wrote: > Hi Jonas, > > On 6/17/24 3:44 PM, Jonas Karlman wrote: >> Hi Quentin, >> >> On 2024-06-17 15:29, Quentin Schulz wrote: >>> Hi Jonas, >>> >>> On 6/17/24 3:26 PM, Jonas Karlman wrote: >>>> Hi Quentin, >>>> >>>> On 2024-06-17 15:10, Quentin Schulz wrote: >>>>> From: Quentin Schulz <quentin.sch...@cherry.de> >>>>> >>>>> In commit 100f489f58a6 ("rockchip: rk3399: Fix loading FIT from SD-card >>>>> when booting from eMMC"), the spi1 bootph properties were mistakenly >>>>> removed meaning, so re-add them back to fix SPI-NOR flash not being >>>>> found in U-Boot pre-reloc as required for RK3399 Puma. >>>> >>>> Good catch, for TPL/SPL the bootph props is propagated, something that >>>> is not done for pre-reloc. >>>> >>> >>> Can you tell us a bit more about this? I know that the pinctrl nodes >>> marked for pre-reloc recursively apply the same to their parent, but >>> couldn't find anything similar for other subsystems for example. What >>> did I miss in my 5m search? >> >> v2024.04-rc1 added support to propagate bootph props using fdtgrep for >> the u-boot-tpl/spl.dtb files, however pre-reloc the main u-boot.dtb (or >> possible the FDT included in FIT) that is not processed by fdtgrep. >> >> fdtgrep: Allow propagating properties up to supernodes >> https://source.denx.de/u-boot/u-boot/-/commit/7a06cc2027c0169c462da63a68fa269c0d59a950 >> >> Makefile: Use the fdtgrep -u flag >> https://source.denx.de/u-boot/u-boot/-/commit/aca95282c1b72c41d8e72984b1dceb15f396b2f8 >> > > Thanks for the pointers! > > I'm wondering if we shouldn't do the same for U-Boot proper DTB as well?
I think that would be good, and should also remove the need for the recursive pinctrl lookup for bootph props during pre-reloc. > > i.e. > > cmd_fdt_rm_props = cat $< | $(objtree)/tools/fdtgrep -r -O dtb - -o $@ > > to > > cmd_fdt_rm_props = $(objtree)/tools/fdtgrep $(fdtgrep_props) -u $< | > $(objtree)/tools/fdtgrep -r -O dtb - -o $@ > > I assume we need to add a bunch more options though. > > But that would make sense to me, the fact that there's a pre-reloc stage > in U-Boot proper is already a bit odd (to me), but if we have something > that behaves differently than the pre-reloc stage in earlier stages... > I'm not sure this is a good idea? Fully agree that part of the pre-reloc stage make little sense at least on Rockchip platform where full memory is available before pre-reloc. (I am hoping to get an initial series to remove time wasted during pre-reloc for Rockchip platform out in near future, e.g. ~250ms is currently wasted looking up e.g. CRU_BASE on rk3399, a constant that is known at compile time). > > While at it, I'm wondering if we didn't miss the removal of > bootph-some-ram in cmd_fdtgrep for the VPL/SPL/TPL? I think so too, the bootph-some-ram should be safe to remove. We can/should probably also extend fdtgrep to remove disabled nodes for the VPL/SPL/TPL. > > Additionally, we should be able to remove bootph-pre-sram and > bootph-pre-ram in U-Boot proper DT since those only apply to non-U-Boot > proper stages, don't you think? I do not fully think we can remove them, due to the dual use of ofnode_pre_reloc(). During pre-reloc it will return true if node is required, and after pre-reloc it return true if the node was required during any prior phase. Unsure if anything depend on this secondry use. Regards, Jonas > > Cheers, > Quentin