Re: [U-Boot] [PATCH 11/24] sunxi: introduce extra config option for boot0 header

2016-11-21 Thread Maxime Ripard
On Mon, Nov 21, 2016 at 09:29:50AM +, Andre Przywara wrote:
> Hi Maxime,
> 
> thanks for having a look!
> 
> On 21/11/16 07:27, Maxime Ripard wrote:
> > Hi Andre,
> > 
> > On Sun, Nov 20, 2016 at 02:57:05PM +, Andre Przywara wrote:
> >> The ENABLE_ARM_SOC_BOOT0_HOOK option is a generic option shared with
> >> other boards. To allow alternative code to be inserted, we create
> >> another, now function specific config symbol on top of it to simplify
> >> later additions. No functional change at this time.
> >>
> >> Signed-off-by: Andre Przywara 
> >> ---
> >>  board/sunxi/Kconfig   | 9 +
> >>  configs/pine64_plus_defconfig | 2 +-
> >>  2 files changed, 10 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig
> >> index e1d4ab1..0cd57a2 100644
> >> --- a/board/sunxi/Kconfig
> >> +++ b/board/sunxi/Kconfig
> >> @@ -133,6 +133,15 @@ config MACH_SUN8I
> >>bool
> >>default y if MACH_SUN8I_A23 || MACH_SUN8I_A33 || MACH_SUN8I_H3 || 
> >> MACH_SUN8I_A83T
> >>  
> >> +config RESERVE_ALLWINNER_BOOT0_HEADER
> >> +  bool "reserve space for Allwinner boot0 header"
> >> +  select ENABLE_ARM_SOC_BOOT0_HOOK
> >> +  ---help---
> >> +  Prepend a 1536 byte (empty) header to the U-Boot image file, to be
> >> +  filled with magic values post build. The Allwinner provided boot0
> >> +  blob relies on this information to load and execute U-Boot.
> >> +  Only needed on 64-bit Allwinner boards so far when using boot0.
> >> +
> > 
> > Is there a reason you can think of to disable it?
> 
> We need it only for booting from boot0, so this series actually makes
> this whole thing obsolete. Since - apart from enlarging the U-Boot
> (proper) image by 1.5KB - it doesn't hurt, though, my idea was to keep
> it in as an option for some time until we are confident that boot0 is no
> longer needed.

Then we don't need to enable it in the defconfig ?

> > If not, you should consider making this enabled by default, so that we
> > don't enable it in all the defconfig for no particular reason.
> 
> I can change the logic, make the Kconfig entry "default y if ARM64", and
> any defconfig could then choose to say "# RESERVE_... is not set".
> 
> Does that make more sense to you?
> 
> What was your major concern about this? Having pointless options in
> various defconfigs?

Yes, Hans was trying to avoid having too much duplication across
defconfig, at least for the common stuff, and I agree with him that we
should keep the defconfig as small as possible.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


signature.asc
Description: PGP signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 11/24] sunxi: introduce extra config option for boot0 header

2016-11-21 Thread Andre Przywara
Hi Maxime,

thanks for having a look!

On 21/11/16 07:27, Maxime Ripard wrote:
> Hi Andre,
> 
> On Sun, Nov 20, 2016 at 02:57:05PM +, Andre Przywara wrote:
>> The ENABLE_ARM_SOC_BOOT0_HOOK option is a generic option shared with
>> other boards. To allow alternative code to be inserted, we create
>> another, now function specific config symbol on top of it to simplify
>> later additions. No functional change at this time.
>>
>> Signed-off-by: Andre Przywara 
>> ---
>>  board/sunxi/Kconfig   | 9 +
>>  configs/pine64_plus_defconfig | 2 +-
>>  2 files changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig
>> index e1d4ab1..0cd57a2 100644
>> --- a/board/sunxi/Kconfig
>> +++ b/board/sunxi/Kconfig
>> @@ -133,6 +133,15 @@ config MACH_SUN8I
>>  bool
>>  default y if MACH_SUN8I_A23 || MACH_SUN8I_A33 || MACH_SUN8I_H3 || 
>> MACH_SUN8I_A83T
>>  
>> +config RESERVE_ALLWINNER_BOOT0_HEADER
>> +bool "reserve space for Allwinner boot0 header"
>> +select ENABLE_ARM_SOC_BOOT0_HOOK
>> +---help---
>> +Prepend a 1536 byte (empty) header to the U-Boot image file, to be
>> +filled with magic values post build. The Allwinner provided boot0
>> +blob relies on this information to load and execute U-Boot.
>> +Only needed on 64-bit Allwinner boards so far when using boot0.
>> +
> 
> Is there a reason you can think of to disable it?

We need it only for booting from boot0, so this series actually makes
this whole thing obsolete. Since - apart from enlarging the U-Boot
(proper) image by 1.5KB - it doesn't hurt, though, my idea was to keep
it in as an option for some time until we are confident that boot0 is no
longer needed.

> If not, you should consider making this enabled by default, so that we
> don't enable it in all the defconfig for no particular reason.

I can change the logic, make the Kconfig entry "default y if ARM64", and
any defconfig could then choose to say "# RESERVE_... is not set".

Does that make more sense to you?

What was your major concern about this? Having pointless options in
various defconfigs?

Cheers,
Andre.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 11/24] sunxi: introduce extra config option for boot0 header

2016-11-20 Thread Maxime Ripard
Hi Andre,

On Sun, Nov 20, 2016 at 02:57:05PM +, Andre Przywara wrote:
> The ENABLE_ARM_SOC_BOOT0_HOOK option is a generic option shared with
> other boards. To allow alternative code to be inserted, we create
> another, now function specific config symbol on top of it to simplify
> later additions. No functional change at this time.
> 
> Signed-off-by: Andre Przywara 
> ---
>  board/sunxi/Kconfig   | 9 +
>  configs/pine64_plus_defconfig | 2 +-
>  2 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig
> index e1d4ab1..0cd57a2 100644
> --- a/board/sunxi/Kconfig
> +++ b/board/sunxi/Kconfig
> @@ -133,6 +133,15 @@ config MACH_SUN8I
>   bool
>   default y if MACH_SUN8I_A23 || MACH_SUN8I_A33 || MACH_SUN8I_H3 || 
> MACH_SUN8I_A83T
>  
> +config RESERVE_ALLWINNER_BOOT0_HEADER
> + bool "reserve space for Allwinner boot0 header"
> + select ENABLE_ARM_SOC_BOOT0_HOOK
> + ---help---
> + Prepend a 1536 byte (empty) header to the U-Boot image file, to be
> + filled with magic values post build. The Allwinner provided boot0
> + blob relies on this information to load and execute U-Boot.
> + Only needed on 64-bit Allwinner boards so far when using boot0.
> +

Is there a reason you can think of to disable it?

If not, you should consider making this enabled by default, so that we
don't enable it in all the defconfig for no particular reason.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


signature.asc
Description: PGP signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH 11/24] sunxi: introduce extra config option for boot0 header

2016-11-20 Thread Andre Przywara
The ENABLE_ARM_SOC_BOOT0_HOOK option is a generic option shared with
other boards. To allow alternative code to be inserted, we create
another, now function specific config symbol on top of it to simplify
later additions. No functional change at this time.

Signed-off-by: Andre Przywara 
---
 board/sunxi/Kconfig   | 9 +
 configs/pine64_plus_defconfig | 2 +-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig
index e1d4ab1..0cd57a2 100644
--- a/board/sunxi/Kconfig
+++ b/board/sunxi/Kconfig
@@ -133,6 +133,15 @@ config MACH_SUN8I
bool
default y if MACH_SUN8I_A23 || MACH_SUN8I_A33 || MACH_SUN8I_H3 || 
MACH_SUN8I_A83T
 
+config RESERVE_ALLWINNER_BOOT0_HEADER
+   bool "reserve space for Allwinner boot0 header"
+   select ENABLE_ARM_SOC_BOOT0_HOOK
+   ---help---
+   Prepend a 1536 byte (empty) header to the U-Boot image file, to be
+   filled with magic values post build. The Allwinner provided boot0
+   blob relies on this information to load and execute U-Boot.
+   Only needed on 64-bit Allwinner boards so far when using boot0.
+
 config DRAM_TYPE
int "sunxi dram type"
depends on MACH_SUN8I_A83T
diff --git a/configs/pine64_plus_defconfig b/configs/pine64_plus_defconfig
index 6d0198f..ea53b96 100644
--- a/configs/pine64_plus_defconfig
+++ b/configs/pine64_plus_defconfig
@@ -1,5 +1,5 @@
 CONFIG_ARM=y
-CONFIG_ENABLE_ARM_SOC_BOOT0_HOOK=y
+CONFIG_RESERVE_ALLWINNER_BOOT0_HEADER=y
 CONFIG_ARCH_SUNXI=y
 CONFIG_MACH_SUN50I=y
 CONFIG_DRAM_CLK=672
-- 
2.8.2

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot