Re: [PATCH v2 2/3] env: Add single to redundant environment upgrade path

[Marek Vasut <marek.vasut@mailbox.org>]

On 12/23/25 6:43 PM, Heinrich Schuchardt wrote:
> On 12/23/25 15:31, Marek Vasut wrote:
>> Add support for converting single-copy environment to redundant 
>> environment.
>> In case CRC checks on both redundant environment copies fail, try one 
>> more
>> CRC check on the primary environment copy and treat it as single 
>> environment.
> 
> Why would a CRC check suddenly succeed if it has failed before?

The previous CRC check was for redundant env configuration, the follow 
up test is for a single-copy environment. This is used for single-copy 
env to redundant env upgrade path.

> This needs some more explanation.

Please read the full commit message, I hope the rest of it clarifies 
what is going on here.

>> If that check does pass, rewrite the single-copy environment into 
>> redundant
>> environment format, indicate the environment is valid, and import that as
>> usual primary copy of redundant environment. Follow up 'env save' will 
>> then
>> store two environment copies and the system will continue to operate as
>> regular redundant environment system.
>>
>> Add test which validates this upgrade path. The test starts with spi.bin
>> which is pre-populated as single-copy environment and then upgrades that
>> environment to dual-copy environment.

[...]

>> +++ b/env/common.c
>> @@ -473,14 +473,24 @@ int env_import(const char *buf, int check, int 
>> flags)
>>   #ifdef CONFIG_ENV_REDUNDANT
>>   static unsigned char env_flags;
>> +#define ENV_SINGLE_HEADER_SIZE    (sizeof(uint32_t))
>> +#define ENV_SINGLE_SIZE        (CONFIG_ENV_SIZE - 
>> ENV_SINGLE_HEADER_SIZE)
>> +
>> +typedef struct {
>> +    uint32_t    crc;            /* CRC32 over data bytes */
>> +    unsigned char    data[ENV_SINGLE_SIZE];    /* Environment data */
>> +} env_single_t;
>> +
>>   int env_check_redund(const char *buf1, int buf1_read_fail,
>>                const char *buf2, int buf2_read_fail)
>>   {
>> -    int crc1_ok = 0, crc2_ok = 0;
>> +    int crc1_ok = 0, crc2_ok = 0, i;
>>       env_t *tmp_env1, *tmp_env2;
>> +    env_single_t *tmp_envs;
>>       tmp_env1 = (env_t *)buf1;
>>       tmp_env2 = (env_t *)buf2;
>> +    tmp_envs = (env_single_t *)buf1;
>>       if (buf1_read_fail && buf2_read_fail) {
>>           puts("*** Error - No Valid Environment Area found\n");
>> @@ -498,6 +508,25 @@ int env_check_redund(const char *buf1, int 
>> buf1_read_fail,
>>                   tmp_env2->crc;
>>       if (!crc1_ok && !crc2_ok) {
> 
> Do we really have a third location to copy from when both store 1 and 
> store 2 are defective? I would have expected that if a vendor provides a 
> single copy then exactly one of crc1_ok or crc2_ok is true and the other 
> is false.
> 
> Please, provide a documentation update explaining how this all works.
This implements upgrade path from single-copy env to redundant env, see 
also what Tom wrote. So no, there is no third copy, there is only 
primary copy which is single-copy env, and that primary copy is upgraded 
to redundant (dual-copy) env.