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

[Shantur Rathore]

Hi Jonas / Simon,

I have submitted a patch based on Jonas's suggestion.
Apologies, I am new to email based patch management and it went as new
thread (at least for me)

It's here - https://lore.kernel.org/u-boot/20231114203309.852289-1-i@shantur.com/

Kind regards,
Shantur

On Mon, Nov 13, 2023 at 12:07 AM Shantur Rathore <i at shantur.com> wrote:
>
> 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
> > >>>>
> > >>
> >