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

[Michal Simek]

On 19.7.2018 17:50, Maxime Ripard wrote:
> On Thu, Jul 19, 2018 at 10:53:44AM -0400, Tom Rini wrote:
>> On Thu, Jul 19, 2018 at 03:45:11PM +0200, Michal Simek wrote:
>>> 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.
>>
>> For now I like the "if" method for now as we can't (or couldn't a while
>> ago) globally migrate everyone over.  I think trying to move everyone
>> over again is something I should give another try.
> 
> Ack.
> 
>>>>>  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?
>>
>> I would have sworn that ENV_SIZE is used for ENV_IS_NOWHERE as that's
>> how much space we have for environment when it's in memory as well.
> 
> Argh, sorry for that I was abused by sunxi-common still having that:
> https://git.denx.de/?p=u-boot.git;a=blob;f=include/configs/sunxi-common.h#l161
> 
> While i was convinced that we were relying solely on Kconfig. I'll
> send a subsequent patch, that one works for me.

Ok. Can you please convert this to any official tag which I can include?

Thanks,
Michal