[PATCH 1/2] drivers: misc: external_state: add a barebox external state.

[Ahmad Fatoum]

Hi Anis,

On 11/3/25 3:09 PM, anis chali wrote:
>>> strlen can't return negative numbers.
> ack, right it is size_t (unsigned)
> 
>>> The original function had a number of safety checks that are missing here:
>>>
>>> - It only ran the code when in EFI payload mode, as the same barebox
>>>  binary could be chainloaded without EFI
>>>
>>> - It checked that /boot is mounted and gave a useful error message to hint
>>>  at e.g., EFIFS missing, which we are missing out on here.
>>>
>>> I am not yet convinced this is strictly an improvement.
> 
> Yes you are right but I didn't reintegrate the code in generic driver
> since it will just use Kconfigs to reference a state dtb file.
> In case this code was accepted there is no context to do the check,
> but I ensured that barebox gives a behaves in way that the user
> will quickly see that there is no /boot directory with the message
> telling No such file or directory, anyway you suggestion to add -f to
> state command
> is more elegant, I will do some experimentations. the aim of this
> patch is to support a state.dtb given in runtime which permits
> userspace interaction with
> native board because I didn't figure out how barebox state command
> could communicate with a barebox intree state.

I didn't quite understand what you are hoping to achieve with this patch
set, but if the command would serve the purpose, then that would be
best, I think.

Cheers,
Ahmad

> 
>>> If you have the need to customize where your state comes from, how about adding
>>> a state -f /path/to/state.dtb command option and calling it from your init script?
> right, more elegant suggestion, I will try it.
> 
> Thank's for feedback
> 
> Anis.
> 
> Le lun. 3 nov. 2025 à 01:46, Ahmad Fatoum <a.fatoum at pengutronix.de> a écrit :
>>
>> Hi,
>>
>> On 03.11.25 05:38, chalianis1 at gmail.com wrote:
>>> From: Chali Anis <chalianis1 at gmail.com>
>>>
>>> Add a driver to use an external state dtb, gives the ability to define
>>> an external state at compile time. useful for yocto or buildroot defining
>>> a state.dtb that will be passed to barebox at compile time vi a defconfig
>>> fragment.
>>
>> We already had code that did this unconditionally for EFI.
>> Can you explain why this needs to be customizable?
>>
>>> +config EXTERNAL_STATE
>>> +     tristate "Use external barebox state dtb"
>>> +     depends on OFDEVICE
>>> +     depends on STATE
>>> +     help
>>> +       This permits the use of an extranl dtb state blob
>>
>> external
>>
>>> +       which permits to dynamicly at compile time specify
>>
>> dynamically
>>
>>> +       an external blob vi EXTERNAL_STATE_DTB_PATH
>>> +
>>> +config EXTERNAL_STATE_DTB_PATH
>>> +     string "the external barebox state dtb path"
>>> +     depends on EXTERNAL_STATE
>>
>> Really needs help text as EXTERNAL makes me think of a file on the
>> build host.
>>
>>> +static int state_external_init(void)
>>> +{
>>> +     const char *dt_path = CONFIG_EXTERNAL_STATE_DTB_PATH;
>>> +     struct device_node *state_root = NULL;
>>> +     size_t size;
>>> +     void *fdt;
>>> +     int ret;
>>> +
>>> +     if (strlen(dt_path) <= 0)
>>> +             return -EINVAL;
>>
>> strlen can't return negative numbers.
>>
>> The original function had a number of safety checks that are missing here:
>>
>> - It only ran the code when in EFI payload mode, as the same barebox
>>   binary could be chainloaded without EFI
>>
>> - It checked that /boot is mounted and gave a useful error message to hint
>>   at e.g., EFIFS missing, which we are missing out on here.
>>
>>
>> I am not yet convinced this is strictly an improvement.
>>
>> If you have the need to customize where your state comes from, how about adding
>> a state -f /path/to/state.dtb command option and calling it from your init script?
>>
>> Cheers,
>> Ahmad
>>
>>> +
>>> +     fdt = read_file(dt_path, &size);
>>> +     if (!fdt) {
>>> +             pr_info("unable to read %s: %m\n", dt_path);
>>> +             return 0;
>>> +     }
>>> +
>>> +     state_root = of_unflatten_dtb(fdt, size);
>>> +     if (!IS_ERR(state_root)) {
>>> +             struct device_node *np = NULL;
>>> +             struct state *state;
>>> +
>>> +             ret = barebox_register_of(state_root);
>>> +             if (ret)
>>> +                     pr_warn("Failed to register device-tree: %pe\n", ERR_PTR(ret));
>>> +
>>> +             np = of_find_node_by_alias(state_root, "state");
>>> +
>>> +             state = state_new_from_node(np, false);
>>> +             if (IS_ERR(state))
>>> +                     return PTR_ERR(state);
>>> +
>>> +             ret = state_load(state);
>>> +             if (ret != -ENOMEDIUM)
>>> +                     pr_warn("Failed to load persistent state, continuing with defaults, %d\n",
>>> +                             ret);
>>> +
>>> +             return 0;
>>> +     }
>>> +
>>> +     return -EINVAL;
>>> +}
>>> +
>>> +late_initcall(state_external_init);
>>
>>
>> --
>> Pengutronix e.K.                           |                             |
>> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
>> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
>> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> 

-- 
Pengutronix e.K.                  |                             |
Steuerwalder Str. 21              | http://www.pengutronix.de/  |
31137 Hildesheim, Germany         | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |