From: Marek Vasut <ma...@denx.de>
Sent: Sunday, November 24, 2024 11:11 PM
> 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;
> }
>
> /*
Looks good, I will integrate it in V4.
Regards
Christoph
