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

[Tom Rini]

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.

> >>  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.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180719/28a1d927/attachment.sig>