Re: [U-Boot] [PATCHv2 5/5] env: Finish migration of common ENV options
On Mon, Nov 18, 2019 at 08:02:10PM -0500, Tom Rini wrote: > - 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 > Cc: Joe Hershberger > Cc: Patrick Delaunay > Cc: uboot-st...@st-md-mailman.stormreply.com > Signed-off-by: Tom Rini > Acked-by: Joe Hershberger > Reviewed-by: Simon Goldschmidt Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCHv2 5/5] env: Finish migration of common ENV options
On Wed, Nov 20, 2019 at 08:39:27PM +0100, Simon Goldschmidt wrote: > 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 > > > > Cc: Joe Hershberger > > > > Cc: Patrick Delaunay > > > > Cc: uboot-st...@st-md-mailman.stormreply.com > > > > Signed-off-by: Tom Rini > > > > --- > > > > > > > > > > > > > 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 0x0010 > > > > -#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 0x0010 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... That does work, it's about what we do for other platforms today, it's not the best and doesn't scale well. > > 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? I mention the above mainly because there's a lot of other examples of this type of problem where yes, we can put a value in Kconfig and have a default but it's really not the best way forward. > 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 Thanks! > BTW: From looking at the include/configs/* changes, CONFIG_SYS_MMC_ENV_DEV > could be moved to Kconfig, too... Yes, there's a bunch
Re: [U-Boot] [PATCHv2 5/5] env: Finish migration of common ENV options
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 Cc: Joe Hershberger Cc: Patrick Delaunay Cc: uboot-st...@st-md-mailman.stormreply.com Signed-off-by: Tom Rini --- 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 0x0010 -#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 0x0010 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 BTW: From looking at the include/configs/* changes, CONFIG_SYS_MMC_ENV_DEV could be moved to Kconfig, too... Regards, Simon ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCHv2 5/5] env: Finish migration of common ENV options
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 > > Cc: Joe Hershberger > > Cc: Patrick Delaunay > > Cc: uboot-st...@st-md-mailman.stormreply.com > > Signed-off-by: Tom Rini > > --- > > > > > 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_DEV0 /* 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 0x0010 > > -#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. 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 :) -- Tom signature.asc Description: PGP signature ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCHv2 5/5] env: Finish migration of common ENV options
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 > Cc: Joe Hershberger > Cc: Patrick Delaunay > Cc: uboot-st...@st-md-mailman.stormreply.com > Signed-off-by: Tom Rini > --- > 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 0x0010 > -#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.] Regards, Simon > > /* >* SPL > diff --git a/include/configs/socfpga_sr1500.h b/include/configs/socfpga_sr1500.h > index e1a48715fb0d..ccaa050ae597 100644 > --- a/include/configs/socfpga_sr1500.h > +++ b/include/configs/socfpga_sr1500.h > @@ -28,10 +28,6 @@ > #define CONFIG_SYS_BOOTCOUNT_BE > > /* Environment setting for SPI flash */ > -#define CONFIG_ENV_SECT_SIZE (64 * 1024) > -#define CONFIG_ENV_SIZE (16 * 1024) > -#define CONFIG_ENV_OFFSET 0x000e > -#define CONFIG_ENV_OFFSET_REDUND (CONFIG_ENV_OFFSET + CONFIG_ENV_SECT_SIZE) > > /* The rest of the configuration is shared */ > #include > diff --git a/include/configs/socfpga_stratix10_socdk.h b/include/configs/socfpga_stratix10_socdk.h > index 8e6ecf4bed34..e8e66fa4ae67 100644 > --- a/include/configs/socfpga_stratix10_socdk.h > +++ b/include/configs/socfpga_stratix10_socdk.h > @@ -48,9 +48,7 @@ > /* >* U-Boot environment configurations >*/ > -#define CONFIG_ENV_SIZE 0x1000 > #define CONFIG_SYS_MMC_ENV_DEV 0 /* device 0 */ > -#define CONFIG_ENV_OFFSET 512 /* just after the MBR */ > > /* >* QSPI support > @@ -62,13 +60,6 @@ > /* Flash device info */ > > /*#define CONFIG_ENV_IS_IN_SPI_FLASH*/ > -#ifdef CONFIG_ENV_IS_IN_SPI_FLASH > -#undef CONFIG_ENV_OFFSET > -#undef CONFIG_ENV_SIZE > -#define CONFIG_ENV_OFFSET 0x71 > -#define CONFIG_ENV_SIZE (4 * 1024) > -#define CONFIG_ENV_SECT_SIZE (4 * 1024) > -#endif /* CONFIG_ENV_IS_IN_SPI_FLASH */ > > #ifndef CONFIG_SPL_BUILD > #define CONFIG_MTD_DEVICE > diff --git a/include/configs/socfpga_vining_fpga.h b/include/configs/socfpga_vining_fpga.h > index 5cc12419a692..8b97cd93a78a 100644 > --- a/include/configs/socfpga_vining_fpga.h > +++ b/include/configs/socfpga_vining_fpga.h > @@ -195,11 +195,6 @@ >"fi\0"\ >"socfpga_legacy_reset_compat=1\0" > > -#define CONFIG_ENV_SECT_SIZE (64 * 1024) > -#define CONFIG_ENV_OFFSET 0x10 > -#define CONFIG_ENV_OFFSET_REDUND \ > - (CONFIG_ENV_OFFSET + CONFIG_ENV_SECT_SIZE) > - > /* Support changing the prompt string */ > #define CONFIG_CMDLINE_PS_SUPPORT > > diff --git a/include/configs/socrates.h b/include/configs/socrates.h > index f5f3316b9030..4fe67dced850 100644 > --- a/include/configs/socrates.h > +++ b/include/configs/socrates.h > @@ -187,11 +187,6 @@ >
Re: [U-Boot] [PATCHv2 5/5] env: Finish migration of common ENV options
> -Original Message- > From: Tom Rini > Sent: Monday, November 18, 2019 7:02 PM > To: u-boot@lists.denx.de > Cc: Wolfgang Denk ; Joseph Hershberger > ; Patrick Delaunay > ; uboot-st...@st-md-mailman.stormreply.com > Subject: [EXTERNAL] [PATCHv2 5/5] env: Finish migration of common ENV > options > > - 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 > Cc: Joe Hershberger > Cc: Patrick Delaunay > Cc: uboot-st...@st-md-mailman.stormreply.com > Signed-off-by: Tom Rini Wow! That was a big one! Acked-by: Joe Hershberger ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
