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

[Jonas Karlman]

Hi Simon,

On 2023-11-12 15:22, Simon Glass wrote:
> Hi Jonas,
> 
> On Sun, 12 Nov 2023 at 05:50, 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?
> 
> I suppose we do this in a lot of cases, where we enable commands and
> other features on various boards. So I don't have any objection to
> this. Certainly it is a pain to use bootstd for development and
> debugging of booting if you cannot use the full command set.

I have no particular objection either, I just want to understand the
need, and if this is something we should apply to more boards.

For RK35xx family of devices I have tried to ensure all boards have
similar commands and features enabled, at least for the features the
board support. So my thought now is if we should imply this Kconfig
option for ROCKCHIP_RK3568 and ROCKCHIP_RK3588.

Use of different features is also rather annoying for a distro that
wants to support a broad range of devices of a soc family, and U-Boot
for boards of the same soc family behave differently.

Regards,
Jonas

> 
> Reviewed-by: Simon Glass <sjg at chromium.org>
> 
> Regards,
> Simon
> 
>>
>> 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
>>>>
>>