| Message ID | 04343af5d01a74dc9c90e7e6abb354cbcd43b687.1610540785.git.michal.simek@xilinx.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | env: Remove all dependencies for SYS_REDUNDAND_ENVIRONMENT | expand |
On Wed, Jan 13, 2021 at 01:26:27PM +0100, Michal Simek wrote: > CONFIG_SYS_REDUNDAND_ENVIRONMENT is changing in env_internal.h how u-boot > works with variables. struct environment_s has one byte flags property > which also affects ENV_SIZE macro. > > I have reached the case where CONFIG_ENV_IS_NOWHERE is default setup > but custom scripts can be designed in a way that u-boot is asked to > import/export variables from/to file which can be in certain format. > That's why also for this configuration make sense to enable > CONFIG_SYS_REDUNDAND_ENVIRONMENT because it depends on environment file > format. > > The patch is removing dependency on this configuration to support selecting > environment file format without any specific dependency where variables are > stored. Are you importing a binary of the environment in, which was generated with redundant env set, is what the problem is?
On 13. 01. 21 15:02, Tom Rini wrote: > On Wed, Jan 13, 2021 at 01:26:27PM +0100, Michal Simek wrote: > >> CONFIG_SYS_REDUNDAND_ENVIRONMENT is changing in env_internal.h how u-boot >> works with variables. struct environment_s has one byte flags property >> which also affects ENV_SIZE macro. >> >> I have reached the case where CONFIG_ENV_IS_NOWHERE is default setup >> but custom scripts can be designed in a way that u-boot is asked to >> import/export variables from/to file which can be in certain format. >> That's why also for this configuration make sense to enable >> CONFIG_SYS_REDUNDAND_ENVIRONMENT because it depends on environment file >> format. >> >> The patch is removing dependency on this configuration to support selecting >> environment file format without any specific dependency where variables are >> stored. > > Are you importing a binary of the environment in, which was generated > with redundant env set, is what the problem is? > Yes exactly. Thanks, Michal
On Wed, Jan 13, 2021 at 03:24:24PM +0100, Michal Simek wrote: > On 13. 01. 21 15:02, Tom Rini wrote: > > On Wed, Jan 13, 2021 at 01:26:27PM +0100, Michal Simek wrote: > > > >> CONFIG_SYS_REDUNDAND_ENVIRONMENT is changing in env_internal.h how u-boot > >> works with variables. struct environment_s has one byte flags property > >> which also affects ENV_SIZE macro. > >> > >> I have reached the case where CONFIG_ENV_IS_NOWHERE is default setup > >> but custom scripts can be designed in a way that u-boot is asked to > >> import/export variables from/to file which can be in certain format. > >> That's why also for this configuration make sense to enable > >> CONFIG_SYS_REDUNDAND_ENVIRONMENT because it depends on environment file > >> format. > >> > >> The patch is removing dependency on this configuration to support selecting > >> environment file format without any specific dependency where variables are > >> stored. > > > > Are you importing a binary of the environment in, which was generated > > with redundant env set, is what the problem is? > > Yes exactly. OK, so env import/export -b require compatible env structs, that makes sense. I assume you've ruled out env import/export -t instead already. I would rather see the struct be identical (so, always have flags) rather than say that we can enable redundant environment in all cases (since functionally, we can't for "nowhere" and don't for some others) as it also means that for your case you would still need to know to enable the redundant feature to get what you're aiming for to work. Does that make sense? Thanks!
On 13. 01. 21 15:43, Tom Rini wrote: > On Wed, Jan 13, 2021 at 03:24:24PM +0100, Michal Simek wrote: >> On 13. 01. 21 15:02, Tom Rini wrote: >>> On Wed, Jan 13, 2021 at 01:26:27PM +0100, Michal Simek wrote: >>> >>>> CONFIG_SYS_REDUNDAND_ENVIRONMENT is changing in env_internal.h how u-boot >>>> works with variables. struct environment_s has one byte flags property >>>> which also affects ENV_SIZE macro. >>>> >>>> I have reached the case where CONFIG_ENV_IS_NOWHERE is default setup >>>> but custom scripts can be designed in a way that u-boot is asked to >>>> import/export variables from/to file which can be in certain format. >>>> That's why also for this configuration make sense to enable >>>> CONFIG_SYS_REDUNDAND_ENVIRONMENT because it depends on environment file >>>> format. >>>> >>>> The patch is removing dependency on this configuration to support selecting >>>> environment file format without any specific dependency where variables are >>>> stored. >>> >>> Are you importing a binary of the environment in, which was generated >>> with redundant env set, is what the problem is? >> >> Yes exactly. > > OK, so env import/export -b require compatible env structs, that makes > sense. I assume you've ruled out env import/export -t instead already. Yes that's not an option. > I would rather see the struct be identical (so, always have flags) > rather than say that we can enable redundant environment in all cases > (since functionally, we can't for "nowhere" and don't for some others) > as it also means that for your case you would still need to know to > enable the redundant feature to get what you're aiming for to work. > Does that make sense? Thanks! I have not a problem to enable this feature for all but when this is simply enabled for everybody boards which don't have this enabled will fail. Maybe variables without that flags can be fall back option that you can read them but when you save you will add there flag field. As of I now I know that I want to enable this feature for certain board because boot variables are generated by OS. Thanks, Michal
On Wed, Jan 13, 2021 at 03:50:17PM +0100, Michal Simek wrote: > > > On 13. 01. 21 15:43, Tom Rini wrote: > > On Wed, Jan 13, 2021 at 03:24:24PM +0100, Michal Simek wrote: > >> On 13. 01. 21 15:02, Tom Rini wrote: > >>> On Wed, Jan 13, 2021 at 01:26:27PM +0100, Michal Simek wrote: > >>> > >>>> CONFIG_SYS_REDUNDAND_ENVIRONMENT is changing in env_internal.h how u-boot > >>>> works with variables. struct environment_s has one byte flags property > >>>> which also affects ENV_SIZE macro. > >>>> > >>>> I have reached the case where CONFIG_ENV_IS_NOWHERE is default setup > >>>> but custom scripts can be designed in a way that u-boot is asked to > >>>> import/export variables from/to file which can be in certain format. > >>>> That's why also for this configuration make sense to enable > >>>> CONFIG_SYS_REDUNDAND_ENVIRONMENT because it depends on environment file > >>>> format. > >>>> > >>>> The patch is removing dependency on this configuration to support selecting > >>>> environment file format without any specific dependency where variables are > >>>> stored. > >>> > >>> Are you importing a binary of the environment in, which was generated > >>> with redundant env set, is what the problem is? > >> > >> Yes exactly. > > > > OK, so env import/export -b require compatible env structs, that makes > > sense. I assume you've ruled out env import/export -t instead already. > > Yes that's not an option. > > > I would rather see the struct be identical (so, always have flags) > > rather than say that we can enable redundant environment in all cases > > (since functionally, we can't for "nowhere" and don't for some others) > > as it also means that for your case you would still need to know to > > enable the redundant feature to get what you're aiming for to work. > > Does that make sense? Thanks! > > I have not a problem to enable this feature for all but when this is > simply enabled for everybody boards which don't have this enabled will > fail. Maybe variables without that flags can be fall back option that > you can read them but when you save you will add there flag field. > > As of I now I know that I want to enable this feature for certain board > because boot variables are generated by OS. OK, so yes, maybe we don't want to enable it globally unconditionally. Can you do a v2 where you update the help text to note that this changes the binary environment structure U-Boot uses? My main concern is that the problem you've tracked down and solved here should be easier to spot for the next person that goes down this path. Thanks!
diff --git a/env/Kconfig b/env/Kconfig index 67ce93061b7d..356d8a0605b4 100644 --- a/env/Kconfig +++ b/env/Kconfig @@ -411,8 +411,6 @@ config ENV_IS_IN_UBI config SYS_REDUNDAND_ENVIRONMENT bool "Enable redundant environment support" - depends on ENV_IS_IN_EEPROM || ENV_IS_IN_FLASH || ENV_IS_IN_MMC || \ - ENV_IS_IN_NAND || ENV_IS_IN_SPI_FLASH || ENV_IS_IN_UBI help Normally, the environemt is stored in a single location. By selecting this option, you can then define where to hold a redundant
CONFIG_SYS_REDUNDAND_ENVIRONMENT is changing in env_internal.h how u-boot works with variables. struct environment_s has one byte flags property which also affects ENV_SIZE macro. I have reached the case where CONFIG_ENV_IS_NOWHERE is default setup but custom scripts can be designed in a way that u-boot is asked to import/export variables from/to file which can be in certain format. That's why also for this configuration make sense to enable CONFIG_SYS_REDUNDAND_ENVIRONMENT because it depends on environment file format. The patch is removing dependency on this configuration to support selecting environment file format without any specific dependency where variables are stored. Signed-off-by: Michal Simek <michal.simek@xilinx.com> --- env/Kconfig | 2 -- 1 file changed, 2 deletions(-)