[PATCH v3] arch: arm: Kconfig: Enable BOOTSTD_FULL for Rockchip SoCs
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 imply FAT_WRITE imply SARADC_ROCKCHIP imply SPL_SYSRESET
Hi Tom / Kever
On Sun, Nov 19, 2023 at 5:24 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 imply FAT_WRITE imply SARADC_ROCKCHIP imply SPL_SYSRESET-- 2.40.1
Can this please be merged in ?
Kind regards, Shantur
On Fri, Dec 08, 2023 at 10:52:02AM +0000, Shantur Rathore wrote:
Hi Tom / Kever
On Sun, Nov 19, 2023 at 5:24 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 imply FAT_WRITE imply SARADC_ROCKCHIP imply SPL_SYSRESETCan this please be merged in ?
I wonder if we shouldn't really globally default to BOOTSTD_FULL if BOOTSTD_DEFAULTS for everyone.
On Sat, Dec 9, 2023 at 7:18 PM Tom Rini trini@konsulko.com wrote:
On Fri, Dec 08, 2023 at 10:52:02AM +0000, Shantur Rathore wrote:
Hi Tom / Kever
On Sun, Nov 19, 2023 at 5:24 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 imply FAT_WRITE imply SARADC_ROCKCHIP imply SPL_SYSRESETCan this please be merged in ?
I wonder if we shouldn't really globally default to BOOTSTD_FULL if BOOTSTD_DEFAULTS for everyone.
Its matter of ~21KB in size, unless any platform is really to its limits it should be alright.
Regards, Shantur
On Sat, Dec 09, 2023 at 07:49:04PM +0000, Shantur Rathore wrote:
On Sat, Dec 9, 2023 at 7:18 PM Tom Rini trini@konsulko.com wrote:
On Fri, Dec 08, 2023 at 10:52:02AM +0000, Shantur Rathore wrote:
Hi Tom / Kever
On Sun, Nov 19, 2023 at 5:24 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 imply FAT_WRITE imply SARADC_ROCKCHIP imply SPL_SYSRESETCan this please be merged in ?
I wonder if we shouldn't really globally default to BOOTSTD_FULL if BOOTSTD_DEFAULTS for everyone.
Its matter of ~21KB in size, unless any platform is really to its limits it should be alright.
Maybe I need to re-check things too, since I wonder how much of that growth is re-enabling things that were removed when dropping the DISTRO stuff, and so for platforms just migrating over now it would be smaller in size if much.
Hi Shantur, Tom,
On 2023/12/10 04:45, Tom Rini wrote:
On Sat, Dec 09, 2023 at 07:49:04PM +0000, Shantur Rathore wrote:
On Sat, Dec 9, 2023 at 7:18 PM Tom Rini trini@konsulko.com wrote:
On Fri, Dec 08, 2023 at 10:52:02AM +0000, Shantur Rathore wrote:
Hi Tom / Kever
On Sun, Nov 19, 2023 at 5:24 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 imply FAT_WRITE imply SARADC_ROCKCHIP imply SPL_SYSRESETCan this please be merged in ?
I wonder if we shouldn't really globally default to BOOTSTD_FULL if BOOTSTD_DEFAULTS for everyone.
Its matter of ~21KB in size, unless any platform is really to its limits it should be alright.
Maybe I need to re-check things too, since I wonder how much of that growth is re-enabling things that were removed when dropping the DISTRO stuff, and so for platforms just migrating over now it would be smaller in size if much.
A board maintainer is free to enable this option, but I don't agree to enable this for everyone.
Not like rk3399 and rk3588, some of other SoCs always want a clean and simple but usable U-Boot,
eg. rk3036 and rk3308 are still in the list.
Thanks, - Kever
Hi Kever,
On Tue, Jan 9, 2024 at 10:55 AM Kever Yang kever.yang@rock-chips.com wrote:
Hi Shantur, Tom,
On 2023/12/10 04:45, Tom Rini wrote:
On Sat, Dec 09, 2023 at 07:49:04PM +0000, Shantur Rathore wrote:
On Sat, Dec 9, 2023 at 7:18 PM Tom Rini trini@konsulko.com wrote:
On Fri, Dec 08, 2023 at 10:52:02AM +0000, Shantur Rathore wrote:
Hi Tom / Kever
On Sun, Nov 19, 2023 at 5:24 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 imply FAT_WRITE imply SARADC_ROCKCHIP imply SPL_SYSRESETCan this please be merged in ?
I wonder if we shouldn't really globally default to BOOTSTD_FULL if BOOTSTD_DEFAULTS for everyone.
Its matter of ~21KB in size, unless any platform is really to its limits it should be alright.
Maybe I need to re-check things too, since I wonder how much of that growth is re-enabling things that were removed when dropping the DISTRO stuff, and so for platforms just migrating over now it would be smaller in size if much.
A board maintainer is free to enable this option, but I don't agree to enable this for everyone.
Not like rk3399 and rk3588, some of other SoCs always want a clean and simple but usable U-Boot,
eg. rk3036 and rk3308 are still in the list.
The discussion is what we consider "usable U-Boot" By default bootstd doesn't have any options and not even to show what it's going to boot.
Would it be acceptable if BOOTSTD_FULL is enabled for SoCs rather than boards?
Kind regards, Shantur
Hi Kever,
On Wed, Jan 10, 2024 at 11:58 AM Shantur Rathore i@shantur.com wrote:
Hi Kever,
On Tue, Jan 9, 2024 at 10:55 AM Kever Yang kever.yang@rock-chips.com wrote:
Hi Shantur, Tom,
On 2023/12/10 04:45, Tom Rini wrote:
On Sat, Dec 09, 2023 at 07:49:04PM +0000, Shantur Rathore wrote:
On Sat, Dec 9, 2023 at 7:18 PM Tom Rini trini@konsulko.com wrote:
On Fri, Dec 08, 2023 at 10:52:02AM +0000, Shantur Rathore wrote:
Hi Tom / Kever
On Sun, Nov 19, 2023 at 5:24 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 > imply FAT_WRITE > imply SARADC_ROCKCHIP > imply SPL_SYSRESET Can this please be merged in ?
I wonder if we shouldn't really globally default to BOOTSTD_FULL if BOOTSTD_DEFAULTS for everyone.
Its matter of ~21KB in size, unless any platform is really to its limits it should be alright.
Maybe I need to re-check things too, since I wonder how much of that growth is re-enabling things that were removed when dropping the DISTRO stuff, and so for platforms just migrating over now it would be smaller in size if much.
A board maintainer is free to enable this option, but I don't agree to enable this for everyone.
Not like rk3399 and rk3588, some of other SoCs always want a clean and simple but usable U-Boot,
eg. rk3036 and rk3308 are still in the list.
The discussion is what we consider "usable U-Boot" By default bootstd doesn't have any options and not even to show what it's going to boot.
Would it be acceptable if BOOTSTD_FULL is enabled for SoCs rather than boards?
Can you please suggest the way forward? Initially the patch was for RockPro64 and then after discussion it was suggested that as BOOTSTD_DEFAULT is very very limited we should enable BOOTSTD_FULL for SoCs that can support multiple boot modes.
Kind regards, Shantur
Hi Shantur,
On 2024/1/17 14:26, Shantur Rathore wrote:
Hi Kever,
On Wed, Jan 10, 2024 at 11:58 AM Shantur Rathorei@shantur.com wrote:
Hi Kever,
On Tue, Jan 9, 2024 at 10:55 AM Kever Yangkever.yang@rock-chips.com wrote:
Hi Shantur, Tom,
On 2023/12/10 04:45, Tom Rini wrote:
On Sat, Dec 09, 2023 at 07:49:04PM +0000, Shantur Rathore wrote:
On Sat, Dec 9, 2023 at 7:18 PM Tom Rinitrini@konsulko.com wrote:
On Fri, Dec 08, 2023 at 10:52:02AM +0000, Shantur Rathore wrote:
> Hi Tom / Kever > > On Sun, Nov 19, 2023 at 5:24 PM Shantur Rathorei@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 Glasssjg@chromium.org >> >> Signed-off-by: Shantur Rathorei@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 >> imply FAT_WRITE >> imply SARADC_ROCKCHIP >> imply SPL_SYSRESET > Can this please be merged in ? I wonder if we shouldn't really globally default to BOOTSTD_FULL if BOOTSTD_DEFAULTS for everyone.
Its matter of ~21KB in size, unless any platform is really to its limits it should be alright.
Maybe I need to re-check things too, since I wonder how much of that growth is re-enabling things that were removed when dropping the DISTRO stuff, and so for platforms just migrating over now it would be smaller in size if much.
A board maintainer is free to enable this option, but I don't agree to enable this for everyone.
Not like rk3399 and rk3588, some of other SoCs always want a clean and simple but usable U-Boot,
eg. rk3036 and rk3308 are still in the list.
The discussion is what we consider "usable U-Boot" By default bootstd doesn't have any options and not even to show what it's going to boot.
Would it be acceptable if BOOTSTD_FULL is enabled for SoCs rather than boards?
Can you please suggest the way forward? Initially the patch was for RockPro64 and then after discussion it was suggested that as BOOTSTD_DEFAULT is very very limited we should enable BOOTSTD_FULL for SoCs that can support multiple boot modes.
Let's enable it for RK3588 first and then maybe other SoCs which not code size sensitive.
Thanks,
- Kever
Kind regards, Shantur
Hi Kever,
On Wed, Jan 17, 2024 at 9:18 AM Kever Yang kever.yang@rock-chips.com wrote:
Hi Shantur,
On 2024/1/17 14:26, Shantur Rathore wrote:
Hi Kever,
On Wed, Jan 10, 2024 at 11:58 AM Shantur Rathore i@shantur.com wrote:
Hi Kever,
On Tue, Jan 9, 2024 at 10:55 AM Kever Yang kever.yang@rock-chips.com wrote:
Hi Shantur, Tom,
On 2023/12/10 04:45, Tom Rini wrote:
On Sat, Dec 09, 2023 at 07:49:04PM +0000, Shantur Rathore wrote:
On Sat, Dec 9, 2023 at 7:18 PM Tom Rini trini@konsulko.com wrote:
On Fri, Dec 08, 2023 at 10:52:02AM +0000, Shantur Rathore wrote:
Hi Tom / Kever
On Sun, Nov 19, 2023 at 5:24 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 imply FAT_WRITE imply SARADC_ROCKCHIP imply SPL_SYSRESETCan this please be merged in ?
I wonder if we shouldn't really globally default to BOOTSTD_FULL if BOOTSTD_DEFAULTS for everyone.
Its matter of ~21KB in size, unless any platform is really to its limits it should be alright.
Maybe I need to re-check things too, since I wonder how much of that growth is re-enabling things that were removed when dropping the DISTRO stuff, and so for platforms just migrating over now it would be smaller in size if much.
A board maintainer is free to enable this option, but I don't agree to enable this for everyone.
Not like rk3399 and rk3588, some of other SoCs always want a clean and simple but usable U-Boot,
eg. rk3036 and rk3308 are still in the list.
The discussion is what we consider "usable U-Boot" By default bootstd doesn't have any options and not even to show what it's going to boot.
Would it be acceptable if BOOTSTD_FULL is enabled for SoCs rather than boards?
Can you please suggest the way forward? Initially the patch was for RockPro64 and then after discussion it was suggested that as BOOTSTD_DEFAULT is very very limited we should enable BOOTSTD_FULL for SoCs that can support multiple boot modes.
Let's enable it for RK3588 first and then maybe other SoCs which not code size sensitive.
My requirement is for RK3399, so I will enable BOOTSTD_FULL for it. While at it do you want me to enable it for RK3588 as well ?
Kind regards, Shantur
Hi Shantur,
On 2024/1/17 17:38, Shantur Rathore wrote:
Hi Kever,
On Wed, Jan 17, 2024 at 9:18 AM Kever Yang kever.yang@rock-chips.com wrote:
Hi Shantur,
On 2024/1/17 14:26, Shantur Rathore wrote:
Hi Kever,
On Wed, Jan 10, 2024 at 11:58 AM Shantur Rathore i@shantur.com wrote:
Hi Kever,
On Tue, Jan 9, 2024 at 10:55 AM Kever Yang kever.yang@rock-chips.com wrote:
Hi Shantur, Tom,
On 2023/12/10 04:45, Tom Rini wrote:
On Sat, Dec 09, 2023 at 07:49:04PM +0000, Shantur Rathore wrote:
On Sat, Dec 9, 2023 at 7:18 PM Tom Rini trini@konsulko.com wrote:
On Fri, Dec 08, 2023 at 10:52:02AM +0000, Shantur Rathore wrote:
Hi Tom / Kever
On Sun, Nov 19, 2023 at 5:24 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 imply FAT_WRITE imply SARADC_ROCKCHIP imply SPL_SYSRESETCan this please be merged in ?
I wonder if we shouldn't really globally default to BOOTSTD_FULL if BOOTSTD_DEFAULTS for everyone.
Its matter of ~21KB in size, unless any platform is really to its limits it should be alright.
Maybe I need to re-check things too, since I wonder how much of that growth is re-enabling things that were removed when dropping the DISTRO stuff, and so for platforms just migrating over now it would be smaller in size if much.
A board maintainer is free to enable this option, but I don't agree to enable this for everyone.
Not like rk3399 and rk3588, some of other SoCs always want a clean and simple but usable U-Boot,
eg. rk3036 and rk3308 are still in the list.
The discussion is what we consider "usable U-Boot" By default bootstd doesn't have any options and not even to show what it's going to boot.
Would it be acceptable if BOOTSTD_FULL is enabled for SoCs rather than boards?
Can you please suggest the way forward? Initially the patch was for RockPro64 and then after discussion it was suggested that as BOOTSTD_DEFAULT is very very limited we should enable BOOTSTD_FULL for SoCs that can support multiple boot modes.
Let's enable it for RK3588 first and then maybe other SoCs which not code size sensitive.
My requirement is for RK3399, so I will enable BOOTSTD_FULL for it. While at it do you want me to enable it for RK3588 as well ?
You can add for RK3399 or both RK3399 and RK3588.
Thanks,
- Kever
Kind regards, Shantur
Hi Kever,
On Thu, Jan 18, 2024 at 3:10 AM Kever Yang kever.yang@rock-chips.com wrote:
Hi Shantur,
On 2024/1/17 17:38, Shantur Rathore wrote:
Hi Kever,
On Wed, Jan 17, 2024 at 9:18 AM Kever Yang kever.yang@rock-chips.com wrote:
Hi Shantur,
On 2024/1/17 14:26, Shantur Rathore wrote:
Hi Kever,
On Wed, Jan 10, 2024 at 11:58 AM Shantur Rathore i@shantur.com wrote:
Hi Kever,
On Tue, Jan 9, 2024 at 10:55 AM Kever Yang kever.yang@rock-chips.com wrote:
Hi Shantur, Tom,
On 2023/12/10 04:45, Tom Rini wrote:
On Sat, Dec 09, 2023 at 07:49:04PM +0000, Shantur Rathore wrote:
On Sat, Dec 9, 2023 at 7:18 PM Tom Rini trini@konsulko.com wrote:
On Fri, Dec 08, 2023 at 10:52:02AM +0000, Shantur Rathore wrote:
Hi Tom / Kever
On Sun, Nov 19, 2023 at 5:24 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 imply FAT_WRITE imply SARADC_ROCKCHIP imply SPL_SYSRESETCan this please be merged in ?
I wonder if we shouldn't really globally default to BOOTSTD_FULL if BOOTSTD_DEFAULTS for everyone.
Its matter of ~21KB in size, unless any platform is really to its limits it should be alright.
Maybe I need to re-check things too, since I wonder how much of that growth is re-enabling things that were removed when dropping the DISTRO stuff, and so for platforms just migrating over now it would be smaller in size if much.
A board maintainer is free to enable this option, but I don't agree to enable this for everyone.
Not like rk3399 and rk3588, some of other SoCs always want a clean and simple but usable U-Boot,
eg. rk3036 and rk3308 are still in the list.
The discussion is what we consider "usable U-Boot" By default bootstd doesn't have any options and not even to show what it's going to boot.
Would it be acceptable if BOOTSTD_FULL is enabled for SoCs rather than boards?
Can you please suggest the way forward? Initially the patch was for RockPro64 and then after discussion it was suggested that as BOOTSTD_DEFAULT is very very limited we should enable BOOTSTD_FULL for SoCs that can support multiple boot modes.
Let's enable it for RK3588 first and then maybe other SoCs which not code size sensitive.
My requirement is for RK3399, so I will enable BOOTSTD_FULL for it. While at it do you want me to enable it for RK3588 as well ?
You can add for RK3399 or both RK3399 and RK3588.
Patch v5 [0] has been submitted with RK3399 and RK3588
[0] - https://patchwork.ozlabs.org/project/uboot/patch/20240121220447.663407-1-i@s...
Kind regards, Shantur
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.
Jagan.
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_DEFAULTSYes, 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.
[0] - https://patchwork.ozlabs.org/project/uboot/patch/20231111001329.537704-1-i@s...
Kind regards, Shantur
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_DEFAULTSYes, 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.
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_DEFAULTSYes, 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...
Kind regards, Shantur
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_DEFAULTSYes, 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"
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_DEFAULTSYes, 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 );
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?
Regards, Simon
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.
Hi Tom,
On Mon, 11 Dec 2023 at 11:19, Tom Rini trini@konsulko.com wrote:
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.
At the time the goal was to avoid growth compared to the distro scripts. We could perhaps add some more things in with BOOTSTD_FULL but still have it as an option?
Regards, Simon
On Mon, Dec 11, 2023 at 11:22:45AM -0700, Simon Glass wrote:
Hi Tom,
[snip]
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.
At the time the goal was to avoid growth compared to the distro scripts. We could perhaps add some more things in with BOOTSTD_FULL but still have it as an option?
Right. But now that we've tried this, some of the feedback has been that it's just too minimal right now. Like looking at the help message for bootflow, list and info should probably always be available. And maybe the flags for "scan" should be re-thought? Too late to change things now but "bootflow scan -b" should maybe how it's always been for "scan and boot".
Hi all,
On Mon, Dec 11, 2023 at 6:27 PM Tom Rini trini@konsulko.com wrote:
On Mon, Dec 11, 2023 at 11:22:45AM -0700, Simon Glass wrote:
Hi Tom,
[snip]
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.
At the time the goal was to avoid growth compared to the distro scripts. We could perhaps add some more things in with BOOTSTD_FULL but still have it as an option?
Right. But now that we've tried this, some of the feedback has been that it's just too minimal right now. Like looking at the help message for bootflow, list and info should probably always be available. And maybe the flags for "scan" should be re-thought? Too late to change things now but "bootflow scan -b" should maybe how it's always been for "scan and boot".
What would be the preferred approach for this patch? Is it to update the default capabilities of bootflow or we can have this patch?
Thanks Shantur
-- Tom
On Fri, Dec 22, 2023 at 12:05:39PM +0000, Shantur Rathore wrote:
Hi all,
On Mon, Dec 11, 2023 at 6:27 PM Tom Rini trini@konsulko.com wrote:
On Mon, Dec 11, 2023 at 11:22:45AM -0700, Simon Glass wrote:
Hi Tom,
[snip]
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.
At the time the goal was to avoid growth compared to the distro scripts. We could perhaps add some more things in with BOOTSTD_FULL but still have it as an option?
Right. But now that we've tried this, some of the feedback has been that it's just too minimal right now. Like looking at the help message for bootflow, list and info should probably always be available. And maybe the flags for "scan" should be re-thought? Too late to change things now but "bootflow scan -b" should maybe how it's always been for "scan and boot".
What would be the preferred approach for this patch? Is it to update the default capabilities of bootflow or we can have this patch?
Well, I don't see a problem with just adding this to the platforms which enable BOOTSTD_FULL until we can rework what's included/excluded by that flag, and there's an issue filed for it now.
On Fri, Dec 22, 2023 at 12:07 PM Tom Rini trini@konsulko.com wrote:
On Fri, Dec 22, 2023 at 12:05:39PM +0000, Shantur Rathore wrote:
Hi all,
On Mon, Dec 11, 2023 at 6:27 PM Tom Rini trini@konsulko.com wrote:
On Mon, Dec 11, 2023 at 11:22:45AM -0700, Simon Glass wrote:
Hi Tom,
[snip]
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.
At the time the goal was to avoid growth compared to the distro scripts. We could perhaps add some more things in with BOOTSTD_FULL but still have it as an option?
Right. But now that we've tried this, some of the feedback has been that it's just too minimal right now. Like looking at the help message for bootflow, list and info should probably always be available. And maybe the flags for "scan" should be re-thought? Too late to change things now but "bootflow scan -b" should maybe how it's always been for "scan and boot".
What would be the preferred approach for this patch? Is it to update the default capabilities of bootflow or we can have this patch?
Well, I don't see a problem with just adding this to the platforms which enable BOOTSTD_FULL until we can rework what's included/excluded by that flag, and there's an issue filed for it now.
-- Tom
Great, can we have this merged in please then [0]
[0] - https://patchwork.ozlabs.org/project/uboot/patch/20231119172310.1307942-1-i@...
Regards, Shantur
Hi,
On Fri, Dec 22, 2023 at 12:09 PM Shantur Rathore i@shantur.com wrote:
On Fri, Dec 22, 2023 at 12:07 PM Tom Rini trini@konsulko.com wrote:
On Fri, Dec 22, 2023 at 12:05:39PM +0000, Shantur Rathore wrote:
Hi all,
On Mon, Dec 11, 2023 at 6:27 PM Tom Rini trini@konsulko.com wrote:
On Mon, Dec 11, 2023 at 11:22:45AM -0700, Simon Glass wrote:
Hi Tom,
[snip]
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.
At the time the goal was to avoid growth compared to the distro scripts. We could perhaps add some more things in with BOOTSTD_FULL but still have it as an option?
Right. But now that we've tried this, some of the feedback has been that it's just too minimal right now. Like looking at the help message for bootflow, list and info should probably always be available. And maybe the flags for "scan" should be re-thought? Too late to change things now but "bootflow scan -b" should maybe how it's always been for "scan and boot".
What would be the preferred approach for this patch? Is it to update the default capabilities of bootflow or we can have this patch?
Well, I don't see a problem with just adding this to the platforms which enable BOOTSTD_FULL until we can rework what's included/excluded by that flag, and there's an issue filed for it now.
-- Tom
Great, can we have this merged in please then [0]
[0] - https://patchwork.ozlabs.org/project/uboot/patch/20231119172310.1307942-1-i@...
I tried this patch over the next branch and it failed to build.
Sent v4 here : https://patchwork.ozlabs.org/project/uboot/patch/20231222234358.563121-1-i@s...
Regards, Shantur
participants (6)
-
Jagan Teki -
Kever Yang -
Peter Robinson -
Shantur Rathore -
Simon Glass -
Tom Rini