[U-Boot] [PATCHv2 5/5] env: Finish migration of common ENV options

[Simon Goldschmidt]

Am 19.11.2019 um 22:52 schrieb Tom Rini:
> On Tue, Nov 19, 2019 at 10:39:18PM +0100, Simon Goldschmidt wrote:
>> Am 19.11.2019 um 02:02 schrieb Tom Rini:> - In ARMv8 NXP Layerscape
>> platforms we also need to make use of
>>>     CONFIG_SYS_RELOC_GD_ENV_ADDR now, do so.
>>> - On ENV_IS_IN_REMOTE, CONFIG_ENV_OFFSET is never used, drop the define
>>>     to 0.
>>> - Add Kconfig entry for ENV_ADDR.
>>> - Make ENV_ADDR / ENV_OFFSET depend on the env locations that use it.
>>> - Add ENV_xxx_REDUND options that depend on their primary option and
>>>     SYS_REDUNDAND_ENVIRONMENT
>>> - On a number of PowerPC platforms, use SPL_ENV_ADDR not CONFIG_ENV_ADDR
>>>     for the pre-main-U-Boot environment location.
>>> - On ENV_IS_IN_SPI_FLASH, check not for CONFIG_ENV_ADDR being set but
>>>     rather it being non-zero, as it will now be zero by default.
>>> - Rework the env_offset absolute in env/embedded.o to not use
>>>     CONFIG_ENV_OFFSET as it was the only use of ENV_OFFSET within
>>>     ENV_IS_IN_FLASH.
>>> - Migrate all platforms.
>>>
>>> Cc: Wolfgang Denk <wd at denx.de>
>>> Cc: Joe Hershberger <joe.hershberger at ni.com>
>>> Cc: Patrick Delaunay <patrick.delaunay at st.com>
>>> Cc: uboot-stm32 at st-md-mailman.stormreply.com
>>> Signed-off-by: Tom Rini <trini at konsulko.com>
>>> ---
>>
>> <snip>
>>
>>> diff --git a/include/configs/socfpga_common.h
>> b/include/configs/socfpga_common.h
>>> index baa214399ff9..05bfef75c0df 100644
>>> --- a/include/configs/socfpga_common.h
>>> +++ b/include/configs/socfpga_common.h
>>> @@ -157,21 +157,13 @@ unsigned int cm_get_qspi_controller_clk_hz(void);
>>>    /*
>>>     * U-Boot environment
>>>     */
>>> -#if !defined(CONFIG_ENV_SIZE)
>>> -#define CONFIG_ENV_SIZE			(8 * 1024)
>>> -#endif
>>>
>>>    /* Environment for SDMMC boot */
>>> -#if defined(CONFIG_ENV_IS_IN_MMC) && !defined(CONFIG_ENV_OFFSET)
>>> +#if defined(CONFIG_ENV_IS_IN_MMC)
>>>    #define CONFIG_SYS_MMC_ENV_DEV		0 /* device 0 */
>>> -#define CONFIG_ENV_OFFSET		(34 * 512) /* just after the GPT */
>>>    #endif
>>>
>>>    /* Environment for QSPI boot */
>>> -#if defined(CONFIG_ENV_IS_IN_SPI_FLASH) && !defined(CONFIG_ENV_OFFSET)
>>> -#define CONFIG_ENV_OFFSET		0x00100000
>>> -#define CONFIG_ENV_SECT_SIZE		(64 * 1024)
>>> -#endif
>>
>> Removing paragraphs like this one will break configs that haven't made it to
>> a mainline defconfig. E.g. for socfpga_socrates_defconfig, you can chose for
>> the ENV to be saved in SPI instead of MMC as the config supports booting
>> from all sources.
>>
>> How do we proceed with such things? I know that might be non-mainline, but I
>> think throwing this info away is a step-back, not an improvement.
>>
>> [And don't get me wrong: this doesn't affect my downstream boards, they
>> don't save/load env due to secure boot reasons anyway.]
> 
> So, I would be happy to see follow up series that add default values for
> locations for various SoCs.  That would address the type of problem
> you're talking about, I believe.

You mean adding lines like this to env/Kconfig at config ENV_OFFSET
	default 0x00100000 if ARCH_SOCFPGA && ENV_IS_IN_SPI_FLASH

Does that even work? And how well does it scale? I know it's a more 
fundamental problem, but in my opinion, scattering those defaults for 
every arch or board throughout all Kconfig files is not the best 
solution. Although I have to admit I don't have an idea of how to solve 
it better right now...

> 
> I'd be even happier if someone looked at how Zephyr takes a dts file and
> generates a header and adapt that to a way for us to have some values be
> read from a dts* file and turned into a define we can use at build time
> (not just the OF_PLAT data).  That would be a real nice step forward :)
> 

If we have that ENV information in the devicetree, we could just use it 
and dump the defines. No need to convert dts to defines beforehand. Only 
if dts cannot be used, we need something different. But in that case 
(mostly real small SPL, I guess), do we really enable the ENV loading code?

Anyway, thinking this over, I don't see a way out to keep the SPI env 
locations for socfpga when moving to Kconfig, so for socfpga:

Reviewed-by: Simon Goldschmidt <simon.k.r.goldschmidt at gmail.com>

BTW: From looking at the include/configs/* changes, 
CONFIG_SYS_MMC_ENV_DEV could be moved to Kconfig, too...

Regards,
Simon