[PATCH] configs: rockpro64: Enable SPI command and full BOOTSTD

[Shantur Rathore]

Hey Jonas,

This sounds like a good idea.
Please implement this.

Kind regards,
Shantur

On Sun, Nov 12, 2023 at 3:01 PM Jonas Karlman <jonas at kwiboo.se> wrote:
>
> Hi Shantur,
>
> On 2023-11-12 15:21, Shantur Rathore wrote:
> > Hey Jonas,
> >
> >> For normal generic use the full bootstd commands should not be needed,
> >> do you have special scripting requirements that require access to full
> >> bootstd commands?
> >>
> >> All rockchip boards use standard boot, this only enables full commands
> >> for one particular board, why is this board special? Or
> >
> > This board isn't special.
> >
> >> is there merit
> >> to imply enable of full commands for all rockchip boards?
> >
> > Maybe not only on rockchip boards but all boards.
> >
> > Do we need full commands in general ? - Maybe not
> >
> > Then why did you need to enable full commands ?
> > Bootstd works when the distro used is supporting it. In my case when I
> > installed the distro (Armbian), EFI was installed to
> > /boot/efi/EFI/debian by default instead of /boot/efi/EFI/boot as
> > expected by Bootstd.
> > Bootstd couldn't find the EFI and failed to boot. In this scenario
> > there is no way to figure out what's going on, what bootmethods /
> > bootflows is bootstd able to find or try to manually boot some.
> > To be able to support the smallest bit of debugging you end up
> > building U-Boot from scratch to enable bootstd full for bootstd info
> > commands.
> > All other commands in U-boot by default come with options to display
> > more information ( for ex. nvme info, nvme detail, usb tree, usb info,
> > usb dev, fatinfo and many more ) then why is bootstd restricted?
> > Thus, in my opinion we should enable BOOTSTD_FULL for users to be able
> > to see more information.
> > Restricting might be beneficial for end user devices but atleast
> > development devices like RockPro64 should come with BOOTSTD_FULL by
> > default.
>
> Thanks for clarifying the need! And I also see the benefit of having
> full feature commands enabled.
>
> For RK35xx family of devices I have tried to ensure all boards have same
> features and sounds like something like below could benefit all rk
> boards, or at least for ROCKCHIP_RK3568 and ROCKCHIP_RK3588 boards.
>
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index c39ae40335a0..07e93a9c7ad5 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -1987,6 +1987,7 @@ config ARCH_ROCKCHIP
>         imply CMD_DM
>         imply DEBUG_UART_BOARD_INIT
>         imply BOOTSTD_DEFAULTS
> +       imply BOOTSTD_FULL if BOOTSTD_DEFAULTS
>         imply FAT_WRITE
>         imply SARADC_ROCKCHIP
>         imply SPL_SYSRESET
>
>
> Regards,
> Jonas
>
> >
> > Kind regards,
> > Shantur
> >
> > On Sun, Nov 12, 2023 at 12:50 PM Jonas Karlman <jonas at kwiboo.se> wrote:
> >>
> >> Hi Shantur,
> >>
> >> On 2023-11-12 13:34, Shantur Rathore wrote:
> >>> Hi Jonas,
> >>>
> >>>> The CMD_SPI is not used to interact with the SPI flash.
> >>>>
> >>>> CMD_SF is already enabled and you can use "sf probe" and any other sf
> >>>> related action to interact with the SPI flash on this board.
> >>>>
> >>>
> >>> You are right, this is not needed, thanks for correcting me.
> >>> I will update my patch.
> >>>
> >>>> What is the reasoning behind enabling full bootstd commands? Especially
> >>>> why just this board need it enabled by default.
> >>>>
> >>>> Standard boot is already fully functional, it just does not have full
> >>>> features commands enabled by default.
> >>>
> >>> By default standard boot only allows you to boot from the first
> >>> available boot device.
> >>> This board supports SDCard, NVME (via PCIe port), USB and network boot
> >>> and with BOOTSTD_FULL we can choose what to exactly boot.
> >>> The boot_targets environment variable only allows you to choose which
> >>> device to boot, not which boot method / flow to use.
> >>> We have ample space in SPI Flash, so why not let it have the full
> >>> potential of U-Boot by default.
> >>
> >> You can control boot targets using the boot_targets env var and boot
> >> methods using the bootmeths env var.
> >>
> >> https://docs.u-boot.org/en/latest/develop/bootstd.html#controlling-ordering
> >>
> >> For normal generic use the full bootstd commands should not be needed,
> >> do you have special scripting requirements that require access to full
> >> bootstd commands?
> >>
> >> All rockchip boards use standard boot, this only enables full commands
> >> for one particular board, why is this board special? Or is there merit
> >> to imply enable of full commands for all rockchip boards?
> >>
> >> Regards,
> >> Jonas
> >>
> >>>
> >>> Kind regards,
> >>> Shantur
> >>>
> >>> On Sun, Nov 12, 2023 at 10:39 AM Jonas Karlman <jonas at kwiboo.se> wrote:
> >>>>
> >>>> On 2023-11-11 01:13, Shantur Rathore wrote:
> >>>>> RockPro64 has a 16MB onboard SPI chip and current u-boot takes
> >>>>> around 2MB, we can enable more features.
> >>>>> Updating config to enable SPI commands and full BootSTD support.
> >>>>> ---
> >>>>>  configs/rockpro64-rk3399_defconfig | 2 ++
> >>>>>  1 file changed, 2 insertions(+)
> >>>>>
> >>>>> diff --git a/configs/rockpro64-rk3399_defconfig b/configs/rockpro64-rk3399_defconfig
> >>>>> index 4cd6b76665..cad0b8a5d7 100644
> >>>>> --- a/configs/rockpro64-rk3399_defconfig
> >>>>> +++ b/configs/rockpro64-rk3399_defconfig
> >>>>> @@ -46,10 +46,12 @@ CONFIG_CMD_BOOTZ=y
> >>>>>  CONFIG_CMD_GPT=y
> >>>>>  CONFIG_CMD_MMC=y
> >>>>>  CONFIG_CMD_PCI=y
> >>>>> +CONFIG_CMD_SPI=y
> >>>>
> >>>> The CMD_SPI is not used to interact with the SPI flash.
> >>>>
> >>>> CMD_SF is already enabled and you can use "sf probe" and any other sf
> >>>> related action to interact with the SPI flash on this board.
> >>>>
> >>>>>  CONFIG_CMD_USB=y
> >>>>>  # CONFIG_CMD_SETEXPR is not set
> >>>>>  CONFIG_CMD_TIME=y
> >>>>>  CONFIG_CMD_BOOTSTAGE=y
> >>>>> +CONFIG_BOOTSTD_FULL=y
> >>>>
> >>>> What is the reasoning behind enabling full bootstd commands? Especially
> >>>> why just this board need it enabled by default.
> >>>>
> >>>> Standard boot is already fully functional, it just does not have full
> >>>> features commands enabled by default.
> >>>>
> >>>> Regards,
> >>>> Jonas
> >>>>
> >>>>>  CONFIG_SPL_OF_CONTROL=y
> >>>>>  CONFIG_OF_SPL_REMOVE_PROPS="pinctrl-0 pinctrl-names clock-names interrupt-parent assigned-clocks assigned-clock-rates assigned-clock-parents"
> >>>>>  CONFIG_ENV_IS_IN_SPI_FLASH=y
> >>>>
> >>
>