Hi, On Wed, 7 Jan 2026 at 20:57, Peter Robinson <[email protected]> wrote: > > Hi Simon, > > > > > Each arch does something slightly different before booting the OS. Some > > > > archs even do different things depending on the CPU type. > > > > > > > > EFT_LOADER has its own special code, regardless of which arch is being > > > > booted. > > > > > > > > It is quite hard to know what actually happens in the final milliseconds > > > > before the OS boot. > > > > > > > > This series attempts to clean up U-Boot in this area. > > > > > > > > The basic approach is to create a new bootm_final() function which is > > > > called by all archs. It provides some flags for a couple of necessary > > > > variations but otherwise it is generic. > > > > > > > > RISC-V, x86 and ARM are converted over to use this new function. For > > > > consistency, EFI loader is converted as well. > > > > > > > > A noteable change is that EFI_LOADER now does bootstage processing > > > > before boot, if enabled, thus producing a report. > > > > > > Why is the change needed, I don't see it explained in the patches or > > > here and what the implications might be. > > > > At present EFI_LOADER ignores bootstage and so there is no timing > > report produced even if enabled. It was probably an oversight. > > So please include that level of detail. > > > > > There is also a patch to drop EFI_GRUB_ARM32_WORKAROUND, as discussed > > > > recently on the mailing list. > > > > > > That appears to be unrelated to this series, it should be a separate > > > standalone patch else it needs to be described here why it's a > > > dependency, and even then likely still posted separately as a > > > dependency of this series. > > > > In what way is it unrelated? In my view this is related as it is a > > pre-OS cleanup. What do you mean by dependency here? > > It is unrelated because if you remove it from the series the series > would still generally work as expected and if I apply the patch on > it's own without the rest of the series it works as expected. It is > only related by a vague association hence it should be sent separately > and discussed on it's own merit. > > > > > Future work could take this a little further: > > > > - Drop board_quiesce_devices() and rely on driver model for that > > > > - Similarly with udc_disconnect(), > > > > - cleanup_before_linux() could use more details as to what it is > > > > supposed to do, to reduce the number of arch-specific variations > > > > > > What do you mean by that statement. > > > > Well, the function does different things on differents archs and is > > mostly uncommented (apart from an ARM-specific note in a README). > > Ideally it would be commented with exactly what it should do, ideally > > at a high level so it is not just arch-specific. > > Well I would expect different arches to potentially do different > things, but your response doesn't explain to me why doing the same > thing is the right thing to do, why not doing the different things > won't affect the arrch, I want to know why the details of change is > good not a hand wavy thing (you yourself have asked others to do > similar explanations in the past too). > > > > > Note: This is the first 10 patches for an 18-part series from the > > > > Concept tree, as requested. The patches after that complete the cleanup > > > > on ARM and RISC-V. > > > > > > > > > > > > Simon Glass (10): > > > > boot: Create a function for final pre-boot steps > > > > efi: Make use of bootm_final() > > > > bootm: Add a message when booting > > > > bootstage: Add some missing dummy functions > > > > bootm: Do bootstage processing in bootm_final() > > > > x86: Call bootm_final() > > > > riscv: Drop board_quiesce_devices() > > > > riscv: Call bootm_final() > > > > efi: Drop EFI_GRUB_ARM32_WORKAROUND > > > > x86: Add cleanup_before_linux() > > > > > > > > MAINTAINERS | 1 + > > > > arch/riscv/include/asm/u-boot-riscv.h | 1 - > > > > arch/riscv/lib/bootm.c | 24 +------------------ > > > > arch/x86/cpu/cpu.c | 5 ++++ > > > > arch/x86/lib/bootm.c | 14 +---------- > > > > boot/Makefile | 2 +- > > > > boot/bootm_final.c | 34 +++++++++++++++++++++++++++ > > > > configs/mt7623n_bpir2_defconfig | 1 - > > > > include/bootm.h | 17 ++++++++++++++ > > > > include/bootstage.h | 8 +++++++ > > > > lib/efi_loader/Kconfig | 10 -------- > > > > lib/efi_loader/efi_boottime.c | 30 +---------------------- > > > > test/boot/bootflow.c | 2 +- > > > > 13 files changed, 70 insertions(+), 79 deletions(-) > > > > create mode 100644 boot/bootm_final.c > > > > > > > > -- > > > > 2.43.0 > > > > > > > > base-commit: 6cdd7597a2fbfc1572c1b0af23d3daf1cefa2de7 > > > > branch: bootma-us
Just stepping back a bit...I haven't seen any patch collect a review tag and the feedback seems less than encouraging. Here is the patchwork link: https://patchwork.ozlabs.org/project/uboot/list/?series=487409&state=* It is always hard to 'put the genie back in the bottle' once different parts of the code base have diverged. Would anyone like me to pursue this, or would it be best to drop it? Regards, Simon
