From mboxrd@z Thu Jan 1 00:00:00 1970 From: Michal Simek Date: Fri, 20 Jul 2018 08:28:07 +0200 Subject: [U-Boot] [PATCH] env: Merge Rockchip, Sunxi, Zynq and ZynqMP In-Reply-To: <20180719155011.waldhsi5vbvubzcl@flea> References: <333eefe1af0b59ba782cab4c1a0a90966e7abe7a.1531982743.git.michal.simek@xilinx.com> <20180719111325.ebw5cq6aqigwc2b7@flea> <20180719145344.GM4609@bill-the-cat> <20180719155011.waldhsi5vbvubzcl@flea> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 >>>>> --- >>>>> >>>>> 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