[U-Boot] [PATCH] env: Merge Rockchip, Sunxi, Zynq and ZynqMP

[Michal Simek]

On 19.7.2018 13:13, Maxime Ripard wrote:
> Hi,
> 
> On Thu, Jul 19, 2018 at 08:45:45AM +0200, Michal Simek wrote:
>> There is no reason to have the same Kconfig options for different SoCs
>> separately. The patch is merging them together.
>>
>> Signed-off-by: Michal Simek <michal.simek at xilinx.com>
>> ---
>>
>> Patch is based on
>> https://lists.denx.de/pipermail/u-boot/2018-July/335126.html
>>
>> I have ENV_SECT_SIZE just for zynq/zynqmp because rockchip and sunxi
>> have this in their configs. When they decide to move then can enable
>> that option for them too.
>> I expect when more platforms extend this we will have less constrain
>> Kconfig setup.
>>
>> ---
>>  env/Kconfig | 66 ++++++++++++++++---------------------------------------------
>>  1 file changed, 17 insertions(+), 49 deletions(-)
>>
>> diff --git a/env/Kconfig b/env/Kconfig
>> index b37dcd78eb75..0ded003d7d41 100644
>> --- a/env/Kconfig
>> +++ b/env/Kconfig
>> @@ -431,23 +431,37 @@ config ENV_EXT4_FILE
>>  	  It's a string of the EXT4 file name. This file use to store the
>>  	  environment (explicit path to the file)
>>  
>> -if ARCH_SUNXI
>> +if ARCH_ROCKCHIP || ARCH_SUNXI || ARCH_ZYNQ || ARCH_ZYNQMP
> 
> Can we have a depends on instead? That would be more flexible.

In what sense? If depends is used below then the same 4 platforms will
be listed on all options below. (I want to also add ZYNQMP_R5 there too)
And changing this in one place seems to me better then on four.


>>  config ENV_OFFSET
>>  	hex "Environment Offset"
>>  	depends on !ENV_IS_IN_UBI
>>  	depends on !ENV_IS_NOWHERE
>> +	default 0x3f8000 if ARCH_ROCKCHIP
>>  	default 0x88000 if ARCH_SUNXI
>> +	default 0xE0000 if ARCH_ZYNQ
>> +	default 0x1E00000 if ARCH_ZYNQMP
>>  	help
>>  	  Offset from the start of the device (or partition)
>>  
>>  config ENV_SIZE
>>  	hex "Environment Size"
>> -	depends on !ENV_IS_NOWHERE
>> -	default 0x20000 if ARCH_SUNXI
>> +	default 0x8000 if ARCH_ROCKCHIP && !ENV_IS_NOWHERE
>> +	default 0x20000 if ARCH_SUNXI && !ENV_IS_NOWHERE
> 
> I'm not sure why you removed the depends on !ENV_IS_NOWHERE. Do you
> have a case where the environment is not store anywhere but still need
> a size?

yes, I had a compilation warning for that case.

in include/environment.h at line 145 it is written this
#define ENV_SIZE (CONFIG_ENV_SIZE - ENV_HEADER_SIZE)

ENV_SIZE is also used in typedef struct environment_s some lines below.
And this structure is used a lot.

How did you find out that this can't be used for ENV_IS_NOWHERE?

Thanks,
Michal