[PATCH] env: Remove all dependencies for SYS_REDUNDAND_ENVIRONMENT

[PATCH] env: Remove all dependencies for SYS_REDUNDAND_ENVIRONMENT

[Michal Simek]

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(-)

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

[PATCH] env: Remove all dependencies for SYS_REDUNDAND_ENVIRONMENT

[Tom Rini]

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?

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210113/c636ce33/attachment.sig>

[PATCH] env: Remove all dependencies for SYS_REDUNDAND_ENVIRONMENT

[Michal Simek]

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

[PATCH] env: Remove all dependencies for SYS_REDUNDAND_ENVIRONMENT

[Tom Rini]

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!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210113/1c9701d8/attachment.sig>

[PATCH] env: Remove all dependencies for SYS_REDUNDAND_ENVIRONMENT

[Michal Simek]

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

[PATCH] env: Remove all dependencies for SYS_REDUNDAND_ENVIRONMENT

[Tom Rini]

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!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210114/77562f68/attachment.sig>