On Mon, Dec 11, 2023 at 10:52:12AM -0700, Simon Glass wrote:
Hi,
On Sat, 9 Dec 2023 at 15:19, Shantur Rathore i@shantur.com wrote:
On Sat, Dec 9, 2023 at 8:56 PM Tom Rini trini@konsulko.com wrote:
On Fri, Dec 08, 2023 at 01:59:26PM +0000, Shantur Rathore wrote:
Hi Peter,
On Fri, Dec 8, 2023 at 12:59 PM Peter Robinson pbrobinson@gmail.com wrote:
On Fri, Dec 8, 2023 at 12:52 PM Shantur Rathore i@shantur.com wrote:
Hi Jagan,
On Fri, Dec 8, 2023 at 11:13 AM Jagan Teki jagan@amarulasolutions.com wrote: > > On Sun, Nov 19, 2023 at 10:54 PM Shantur Rathore i@shantur.com wrote: > > > > Rockchip SoCs can support wide range of bootflows. > > Without full bootflow commands, it can be difficult to > > figure out issues if any, hence enable by default. > > > > Reviewed-by: Simon Glass sjg@chromium.org > > > > Signed-off-by: Shantur Rathore i@shantur.com > > --- > > > > (no changes since v1) > > > > arch/arm/Kconfig | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig > > index d812685c98..fca6ef6d7e 100644 > > --- a/arch/arm/Kconfig > > +++ b/arch/arm/Kconfig > > @@ -1986,6 +1986,7 @@ config ARCH_ROCKCHIP > > imply CMD_DM > > imply DEBUG_UART_BOARD_INIT > > imply BOOTSTD_DEFAULTS > > + imply BOOTSTD_FULL if BOOTSTD_DEFAULTS > > Yes, but better to give this option to specific board vendors as > defaults are enough to boot 1st bootflow and what ever media's it has. >
Yes, that's correct it is enough to boot but by default there is no option to choose what to boot from. This was discussed in an earlier version of this patch [0] where I was explicitly enabling only for RP64.
What actual extra functionality does this provide, what is the impact on the size of images? You've not provided reasonable justification outside of very vague statements, it would be useful to know what the added options actually solves.
BOOTSTD_FULL enables all the options in bootflow commands. This is needed if
- you want to choose between multiple available bootflows rather than
just boot one default.
- if you need to list the available bootflows that bootstd has found
- if you need to select and boot any bootflow other than default.
By default all other commands in U-boot come with options to show details For example, nvme info, nvme detail, usb info, usb tree but with bootstd no way to know anything.
Image size - u-boot.itd without BOOTSTD_FULL - 1193984 bytes Image size - u-boot.itb with BOOTSTD_FULL - 1214976 bytes Difference - 20992 bytes
According to binman for RK3399 u-boot can take upto 4M [1] so we have ample space.
This was discussed in the previous patch in the link below [0]
[0] - https://patchwork.ozlabs.org/project/uboot/patch/20231111001329.537704-1-i@s... [1] - https://github.com/shantur/u-boot/blob/master/arch/arm/dts/rk3399-u-boot.dts...
If I'm recalling everything right, this also brings "bootflow" as a command more in line with what could be done with distro_bootcmd in terms of "cover every possible case and let the user override things"
Yes, That's correct.
U_BOOT_LONGHELP(bootflow, #ifdef CONFIG_CMD_BOOTFLOW_FULL "scan [-abeGl] [bdev] - scan for valid bootflows (-l list, -a all, -e errors, -b boot, -G no global)\n" "bootflow list [-e] - list scanned bootflows (-e errors)\n" "bootflow select [<num>|<name>] - select a bootflow\n" "bootflow info [-ds] - show info on current bootflow (-d dump bootflow)\n" "bootflow read - read all current-bootflow files\n" "bootflow boot - boot current bootflow\n" "bootflow menu [-t] - show a menu of available bootflows\n" "bootflow cmdline [set|get|clear|delete|auto] <param> [<value>] - update cmdline" #else "scan - boot first available bootflow\n" #endif );
I suggest we keep it off for now, as we did put in quite a bit of effort to reduce code size. It would be a shame to throw it all away.
That said, I can imagine this becoming a pain for people over time, as the 'bootflow' command becomes the common way to interact with U-Boot. I just wonder if it is too early to make the switch?
I think in hind-sight too much stuff is omitted without BOOTSTD_FULL. The option itself then enables other stuff too by default, but some parts of the bootflow command itself should be visible even without FULL to make things easier on the user.