>> 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. >> 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 <[email protected]> a écrit : > > Hi, > > On 03.11.25 05:38, [email protected] wrote: > > From: Chali Anis <[email protected]> > > > > 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 |
