Re: [PATCH v2] env: spi: Fix gd->env_valid for the first write

[Michal Simek <michal.simek@amd.com>]

On 9/11/25 05:28, Marek Vasut wrote:
> On 9/10/25 9:26 AM, Michal Simek wrote:
>> In case of ENV_INVALID (uninitialized variables) the first env location
>> should be used for storing variables.
>> That's why change the logic how env_valid is setup to be aligned with
>> offset calculation.
>> This will also fix the print about Valid environment is showing proper
>> location where variables are saved for the first time.
>> And also fixes behavior where the first two writes were going to the
>> first location instead of the first to the first location and second to
>> second location.
>>
>> Signed-off-by: Michal Simek <michal.simek@amd.com>
>> ---
>>
>> Changes in v2:
>> - Simplify commit message
>>
>> Origin commit message with more technical details.
>>
>> When both location for variables are not valid (or empty) gd->env_valid is
>> 0 (ENV_INVALID) which is setup by
>> env_sf_load()/env_import_redund()/env_check_redund() calls.
>>
>> When saveenv is called in case of SPI env_sf_save() is called which
>> contains logic which describes new/old variable locations.
>>
>> if (gd->env_valid == ENV_VALID) {
>>          env_new_offset = CONFIG_ENV_OFFSET_REDUND;
>>          env_offset = CONFIG_ENV_OFFSET;
>> } else {
>>          env_new_offset = CONFIG_ENV_OFFSET;
>>          env_offset = CONFIG_ENV_OFFSET_REDUND;
>> }
>>
>> In case of ENV_INVALID option the first location is used (else part)
>> and variables are saved with ENV_REDUND_ACTIVE flag.
>> The second location flag is rewritten to ENV_REDUND_OBSOLETE.
>>
>> And
>> gd->env_valid = gd->env_valid == ENV_REDUND ? ENV_VALID : ENV_REDUND;
>> is executed. env_valid is ENV_INVALID that's why ENV_REDUND is setup
>> but that's not correct because the first location has been written and not
>> the second one.
>> That is confirmed by
>> printf("Valid environment: %d\n", (int)gd->env_valid);
>> which is showing number 2 (ENV_REDUND).
>>
>> That's why change the logic how env_valid is setup to be aligned with
>> offset calculation which also cover the case where initial state is
>> ENV_INVALID.
>> Then print about Valid environment is showing proper location where
>> variables are saved for the first time.
>> But also it is fixing behavior where the first two writes are going to the
>> first location instead of the first to the first location and second to
>> second location. Alternation is happening after it.
>>
>> v1: https://lore.kernel.org/ 
>> r/03aa59f72f47fe0ecafd1e1ca52cbb8a5a8f0446.1755613540.git.michal.simek@amd.com
> Sorry for not being clear, the original commit message structure and technical 
> details were fine, it only needed slight rephrasing, in the original form it was 
> very hard to understand. Can you please reinstate the original commit message 
> and only clean it up slightly ?

Can you exactly point me to part which wasn't clear? It has a lot of details 
about flow and obviously it makes sense to me that's why I would like you to 
point me to exact part which should be improved.

Thanks,
Michal