[PATCH V3 3/3] board: dhelectronics: Sync env variable dh_som_serial_number with SN

[Marek Vasut]

On 11/24/24 10:10 PM, Marek Vasut wrote:
> On 11/21/24 6:21 PM, Christoph Niedermaier wrote:
>> The env variable "SN" is used to store the serial number on DH 
>> electronics
>> SoMs. New SoMs will use the variable "dh_som_serial_number". To ensure
>> compatibility, these env variables are synchronized. This is achieved
>> using callback functions. To avoid recursive calls, these are locked
>> against each other.
> 
> What kind of recursive calls?
> 
> It would be good to expand the commit message and elaborate on the 
> recursive calls problem .
> 
>> diff --git a/board/dhelectronics/common/dh_common.c b/board/ 
>> dhelectronics/common/dh_common.c
>> index a7b0472a09..2e3e5c483b 100644
>> --- a/board/dhelectronics/common/dh_common.c
>> +++ b/board/dhelectronics/common/dh_common.c
>> @@ -45,6 +45,32 @@ struct eeprom_id_page {
>>   #define DH_EEPROM_ID_PAGE_V1_0_DATA_LEN    (sizeof(struct 
>> eeprom_id_page) - \
>>                       offsetof(struct eeprom_id_page, mac0))
>> +static bool in_dh_som_serial_number;
>> +static bool in_SN;
>> +
>> +static int on_dh_som_serial_number(const char *name, const char 
>> *value, enum env_op op,
>> +                   int flags)
>> +{
>> +    in_dh_som_serial_number = true;
>> +    if (!in_SN)
>> +        env_set("SN", value);
>> +    in_dh_som_serial_number = false;
>> +    return 0;
>> +}
Maybe a more generic solution is:

diff --git a/lib/hashtable.c b/lib/hashtable.c
index e8a59e2dcac..75c263b5053 100644
--- a/lib/hashtable.c
+++ b/lib/hashtable.c
@@ -221,11 +221,32 @@ static int
  do_callback(const struct env_entry *e, const char *name, const char 
*value,
             enum env_op op, int flags)
  {
+       int ret = 0;
+
  #ifndef CONFIG_XPL_BUILD
-       if (e->callback)
-               return e->callback(name, value, op, flags);
+       static bool in_callback;
+
+       if (!e->callback || in_callback)
+               return 0;
+
+       /*
+        * In case there are two variables which each implement env callback
+        * that performs env_set() on the other variable, the callbacks will
+        * call each other recursively until the stack runs out. Prevent 
such
+        * a recursion from happening.
+        *
+        * Example which triggers this behavior:
+        * static int on_foo(...) { env_set("bar", 0); ... }
+        * static int on_bar(...) { env_set("foo", 0); ... }
+        * U_BOOT_ENV_CALLBACK(foo, on_foo);
+        * U_BOOT_ENV_CALLBACK(bar, on_bar);
+        */
+       in_callback = true;
+       ret = e->callback(name, value, op, flags);
+       in_callback = false;
  #endif
-       return 0;
+
+       return ret;
  }

  /*