* [PATCH 1/2] drivers: misc: external_state: add a barebox external state. @ 2025-11-03 4:38 chalianis1 2025-11-03 4:38 ` [PATCH 2/2] efi: payload: refactor to use the external barebox state driver chalianis1 2025-11-03 6:46 ` [PATCH 1/2] drivers: misc: external_state: add a barebox external state Ahmad Fatoum 0 siblings, 2 replies; 10+ messages in thread From: chalianis1 @ 2025-11-03 4:38 UTC (permalink / raw) To: s.hauer; +Cc: barebox, Chali Anis From: Chali Anis <chalianis1@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. Signed-off-by: Chali Anis <chalianis1@gmail.com> --- drivers/misc/Kconfig | 13 ++++++++ drivers/misc/Makefile | 1 + drivers/misc/external_state.c | 58 +++++++++++++++++++++++++++++++++++ 3 files changed, 72 insertions(+) create mode 100644 drivers/misc/external_state.c diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig index e235646ee551..91075788b5b8 100644 --- a/drivers/misc/Kconfig +++ b/drivers/misc/Kconfig @@ -21,6 +21,19 @@ config STATE_DRV depends on OFDEVICE depends on STATE +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 + which permits to dynamicly at compile time specify + an external blob vi EXTERNAL_STATE_DTB_PATH + +config EXTERNAL_STATE_DTB_PATH + string "the external barebox state dtb path" + depends on EXTERNAL_STATE + config DEV_MEM bool "Generic memory I/O device (/dev/mem)" diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile index c4b61de7b8b5..eba19f8c22c3 100644 --- a/drivers/misc/Makefile +++ b/drivers/misc/Makefile @@ -6,6 +6,7 @@ obj-$(CONFIG_JTAG) += jtag.o obj-$(CONFIG_SRAM) += sram.o obj-$(CONFIG_STATE_DRV) += state.o +obj-$(CONFIG_EXTERNAL_STATE) += external_state.o obj-$(CONFIG_DEV_MEM) += mem.o obj-$(CONFIG_DEV_PORT) += port.o obj-$(CONFIG_UBOOTVAR) += ubootvar.o diff --git a/drivers/misc/external_state.c b/drivers/misc/external_state.c new file mode 100644 index 000000000000..d392133d3370 --- /dev/null +++ b/drivers/misc/external_state.c @@ -0,0 +1,58 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Copyright (c) 2014 Sascha Hauer <s.hauer@pengutronix.de>, Pengutronix + * Copyright (C) 2025 Anis Chali <anis.chali#ro-main.com>, Ro-Main + */ + +#include <common.h> +#include <driver.h> +#include <init.h> +#include <state.h> +#include <libfile.h> + +#include <linux/err.h> + +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; + + 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); -- 2.34.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] efi: payload: refactor to use the external barebox state driver. 2025-11-03 4:38 [PATCH 1/2] drivers: misc: external_state: add a barebox external state chalianis1 @ 2025-11-03 4:38 ` chalianis1 2025-11-03 6:40 ` Ahmad Fatoum 2025-11-03 6:46 ` [PATCH 1/2] drivers: misc: external_state: add a barebox external state Ahmad Fatoum 1 sibling, 1 reply; 10+ messages in thread From: chalianis1 @ 2025-11-03 4:38 UTC (permalink / raw) To: s.hauer; +Cc: barebox, Chali Anis From: Chali Anis <chalianis1@gmail.com> use the external state config to pass the barebox state. Signed-off-by: Chali Anis <chalianis1@gmail.com> --- arch/arm/configs/efi_v8_defconfig | 2 ++ efi/payload/init.c | 50 ------------------------------- 2 files changed, 2 insertions(+), 50 deletions(-) diff --git a/arch/arm/configs/efi_v8_defconfig b/arch/arm/configs/efi_v8_defconfig index 5f946dd51d4a..26682aab304a 100644 --- a/arch/arm/configs/efi_v8_defconfig +++ b/arch/arm/configs/efi_v8_defconfig @@ -21,6 +21,8 @@ CONFIG_CONSOLE_RATP=y CONFIG_PARTITION_DISK_EFI=y CONFIG_DEFAULT_ENVIRONMENT_GENERIC_NEW=y CONFIG_STATE=y +CONFIG_EXTERNAL_STATE=y +CONFIG_EXTERNAL_STATE_DTB_PATH="/boot/EFI/barebox/state.dtb" CONFIG_BOOTCHOOSER=y CONFIG_RESET_SOURCE=y CONFIG_MACHINE_ID=y diff --git a/efi/payload/init.c b/efi/payload/init.c index 5b827c57ed1f..63d62395cf3f 100644 --- a/efi/payload/init.c +++ b/efi/payload/init.c @@ -376,56 +376,6 @@ static int efi_postcore_init(void) } postcore_efi_initcall(efi_postcore_init); -static int efi_late_init(void) -{ - const char *state_desc = "/boot/EFI/barebox/state.dtb"; - struct device_node *state_root = NULL; - size_t size; - void *fdt; - int ret; - - if (!IS_ENABLED(CONFIG_STATE)) - return 0; - - if (!get_mounted_path("/boot")) { - pr_warn("boot device couldn't be determined%s\n", - IS_ENABLED(CONFIG_FS_EFI) ? "" : " without CONFIG_FS_EFI"); - return 0; - } - - fdt = read_file(state_desc, &size); - if (!fdt) { - pr_info("unable to read %s: %m\n", state_desc); - 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 0; -} -late_efi_initcall(efi_late_init); - static int do_efiexit(int argc, char *argv[]) { if (!BS) -- 2.34.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] efi: payload: refactor to use the external barebox state driver. 2025-11-03 4:38 ` [PATCH 2/2] efi: payload: refactor to use the external barebox state driver chalianis1 @ 2025-11-03 6:40 ` Ahmad Fatoum 2025-11-03 14:10 ` anis chali 0 siblings, 1 reply; 10+ messages in thread From: Ahmad Fatoum @ 2025-11-03 6:40 UTC (permalink / raw) To: chalianis1, s.hauer; +Cc: barebox Hi, On 03.11.25 05:38, chalianis1@gmail.com wrote: > From: Chali Anis <chalianis1@gmail.com> > > use the external state config to pass the barebox state. > > Signed-off-by: Chali Anis <chalianis1@gmail.com> > --- > arch/arm/configs/efi_v8_defconfig | 2 ++ > efi/payload/init.c | 50 ------------------------------- This breaks state for x86-efi, the original user of efi_late_init(). > 2 files changed, 2 insertions(+), 50 deletions(-) > > diff --git a/arch/arm/configs/efi_v8_defconfig b/arch/arm/configs/efi_v8_defconfig > index 5f946dd51d4a..26682aab304a 100644 > --- a/arch/arm/configs/efi_v8_defconfig > +++ b/arch/arm/configs/efi_v8_defconfig > @@ -21,6 +21,8 @@ CONFIG_CONSOLE_RATP=y > CONFIG_PARTITION_DISK_EFI=y > CONFIG_DEFAULT_ENVIRONMENT_GENERIC_NEW=y > CONFIG_STATE=y > +CONFIG_EXTERNAL_STATE=y > +CONFIG_EXTERNAL_STATE_DTB_PATH="/boot/EFI/barebox/state.dtb" > CONFIG_BOOTCHOOSER=y > CONFIG_RESET_SOURCE=y > CONFIG_MACHINE_ID=y > diff --git a/efi/payload/init.c b/efi/payload/init.c > index 5b827c57ed1f..63d62395cf3f 100644 > --- a/efi/payload/init.c > +++ b/efi/payload/init.c > @@ -376,56 +376,6 @@ static int efi_postcore_init(void) > } > postcore_efi_initcall(efi_postcore_init); > > -static int efi_late_init(void) > -{ > - const char *state_desc = "/boot/EFI/barebox/state.dtb"; > - struct device_node *state_root = NULL; > - size_t size; > - void *fdt; > - int ret; > - > - if (!IS_ENABLED(CONFIG_STATE)) > - return 0; > - > - if (!get_mounted_path("/boot")) { > - pr_warn("boot device couldn't be determined%s\n", > - IS_ENABLED(CONFIG_FS_EFI) ? "" : " without CONFIG_FS_EFI"); > - return 0; > - } > - > - fdt = read_file(state_desc, &size); > - if (!fdt) { > - pr_info("unable to read %s: %m\n", state_desc); > - 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 0; > -} > -late_efi_initcall(efi_late_init); > - > static int do_efiexit(int argc, char *argv[]) > { > if (!BS) -- 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 | ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] efi: payload: refactor to use the external barebox state driver. 2025-11-03 6:40 ` Ahmad Fatoum @ 2025-11-03 14:10 ` anis chali 2025-11-03 14:24 ` Ahmad Fatoum 0 siblings, 1 reply; 10+ messages in thread From: anis chali @ 2025-11-03 14:10 UTC (permalink / raw) To: Ahmad Fatoum; +Cc: barebox >> This breaks state for x86-efi, the original user of efi_late_init(). Was not sure to add it to the efi_defconfig Le lun. 3 nov. 2025 à 01:40, Ahmad Fatoum <a.fatoum@pengutronix.de> a écrit : > > Hi, > > On 03.11.25 05:38, chalianis1@gmail.com wrote: > > From: Chali Anis <chalianis1@gmail.com> > > > > use the external state config to pass the barebox state. > > > > Signed-off-by: Chali Anis <chalianis1@gmail.com> > > --- > > arch/arm/configs/efi_v8_defconfig | 2 ++ > > efi/payload/init.c | 50 ------------------------------- > > This breaks state for x86-efi, the original user of efi_late_init(). > > > 2 files changed, 2 insertions(+), 50 deletions(-) > > > > diff --git a/arch/arm/configs/efi_v8_defconfig b/arch/arm/configs/efi_v8_defconfig > > index 5f946dd51d4a..26682aab304a 100644 > > --- a/arch/arm/configs/efi_v8_defconfig > > +++ b/arch/arm/configs/efi_v8_defconfig > > @@ -21,6 +21,8 @@ CONFIG_CONSOLE_RATP=y > > CONFIG_PARTITION_DISK_EFI=y > > CONFIG_DEFAULT_ENVIRONMENT_GENERIC_NEW=y > > CONFIG_STATE=y > > +CONFIG_EXTERNAL_STATE=y > > +CONFIG_EXTERNAL_STATE_DTB_PATH="/boot/EFI/barebox/state.dtb" > > CONFIG_BOOTCHOOSER=y > > CONFIG_RESET_SOURCE=y > > CONFIG_MACHINE_ID=y > > diff --git a/efi/payload/init.c b/efi/payload/init.c > > index 5b827c57ed1f..63d62395cf3f 100644 > > --- a/efi/payload/init.c > > +++ b/efi/payload/init.c > > @@ -376,56 +376,6 @@ static int efi_postcore_init(void) > > } > > postcore_efi_initcall(efi_postcore_init); > > > > -static int efi_late_init(void) > > -{ > > - const char *state_desc = "/boot/EFI/barebox/state.dtb"; > > - struct device_node *state_root = NULL; > > - size_t size; > > - void *fdt; > > - int ret; > > - > > - if (!IS_ENABLED(CONFIG_STATE)) > > - return 0; > > - > > - if (!get_mounted_path("/boot")) { > > - pr_warn("boot device couldn't be determined%s\n", > > - IS_ENABLED(CONFIG_FS_EFI) ? "" : " without CONFIG_FS_EFI"); > > - return 0; > > - } > > - > > - fdt = read_file(state_desc, &size); > > - if (!fdt) { > > - pr_info("unable to read %s: %m\n", state_desc); > > - 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 0; > > -} > > -late_efi_initcall(efi_late_init); > > - > > static int do_efiexit(int argc, char *argv[]) > > { > > if (!BS) > > > -- > 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 | ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] efi: payload: refactor to use the external barebox state driver. 2025-11-03 14:10 ` anis chali @ 2025-11-03 14:24 ` Ahmad Fatoum 2025-11-03 15:28 ` anis chali 0 siblings, 1 reply; 10+ messages in thread From: Ahmad Fatoum @ 2025-11-03 14:24 UTC (permalink / raw) To: anis chali; +Cc: barebox Hi, On 11/3/25 3:10 PM, anis chali wrote: >>> This breaks state for x86-efi, the original user of efi_late_init(). > > Was not sure to add it to the efi_defconfig generally speaking, we try not to break existing users and if we do, an entry should be added to the migration guide explaining what backward incompatibility was introduced. Cheers, Ahmad > > Le lun. 3 nov. 2025 à 01:40, Ahmad Fatoum <a.fatoum@pengutronix.de> a écrit : >> >> Hi, >> >> On 03.11.25 05:38, chalianis1@gmail.com wrote: >>> From: Chali Anis <chalianis1@gmail.com> >>> >>> use the external state config to pass the barebox state. >>> >>> Signed-off-by: Chali Anis <chalianis1@gmail.com> >>> --- >>> arch/arm/configs/efi_v8_defconfig | 2 ++ >>> efi/payload/init.c | 50 ------------------------------- >> >> This breaks state for x86-efi, the original user of efi_late_init(). >> >>> 2 files changed, 2 insertions(+), 50 deletions(-) >>> >>> diff --git a/arch/arm/configs/efi_v8_defconfig b/arch/arm/configs/efi_v8_defconfig >>> index 5f946dd51d4a..26682aab304a 100644 >>> --- a/arch/arm/configs/efi_v8_defconfig >>> +++ b/arch/arm/configs/efi_v8_defconfig >>> @@ -21,6 +21,8 @@ CONFIG_CONSOLE_RATP=y >>> CONFIG_PARTITION_DISK_EFI=y >>> CONFIG_DEFAULT_ENVIRONMENT_GENERIC_NEW=y >>> CONFIG_STATE=y >>> +CONFIG_EXTERNAL_STATE=y >>> +CONFIG_EXTERNAL_STATE_DTB_PATH="/boot/EFI/barebox/state.dtb" >>> CONFIG_BOOTCHOOSER=y >>> CONFIG_RESET_SOURCE=y >>> CONFIG_MACHINE_ID=y >>> diff --git a/efi/payload/init.c b/efi/payload/init.c >>> index 5b827c57ed1f..63d62395cf3f 100644 >>> --- a/efi/payload/init.c >>> +++ b/efi/payload/init.c >>> @@ -376,56 +376,6 @@ static int efi_postcore_init(void) >>> } >>> postcore_efi_initcall(efi_postcore_init); >>> >>> -static int efi_late_init(void) >>> -{ >>> - const char *state_desc = "/boot/EFI/barebox/state.dtb"; >>> - struct device_node *state_root = NULL; >>> - size_t size; >>> - void *fdt; >>> - int ret; >>> - >>> - if (!IS_ENABLED(CONFIG_STATE)) >>> - return 0; >>> - >>> - if (!get_mounted_path("/boot")) { >>> - pr_warn("boot device couldn't be determined%s\n", >>> - IS_ENABLED(CONFIG_FS_EFI) ? "" : " without CONFIG_FS_EFI"); >>> - return 0; >>> - } >>> - >>> - fdt = read_file(state_desc, &size); >>> - if (!fdt) { >>> - pr_info("unable to read %s: %m\n", state_desc); >>> - 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 0; >>> -} >>> -late_efi_initcall(efi_late_init); >>> - >>> static int do_efiexit(int argc, char *argv[]) >>> { >>> if (!BS) >> >> >> -- >> 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 | ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] efi: payload: refactor to use the external barebox state driver. 2025-11-03 14:24 ` Ahmad Fatoum @ 2025-11-03 15:28 ` anis chali 0 siblings, 0 replies; 10+ messages in thread From: anis chali @ 2025-11-03 15:28 UTC (permalink / raw) To: Ahmad Fatoum; +Cc: barebox > generally speaking, we try not to break existing users and if we do, an > entry should be added to the migration guide explaining what backward > incompatibility was introduced. Okay I understand, for people using the default efi_defconfig or just adds fragments on top of it, it will be transparent but otherwise yes there will be a breakage. Thanks. Cheers. Anis. Le lun. 3 nov. 2025 à 09:24, Ahmad Fatoum <a.fatoum@pengutronix.de> a écrit : > > Hi, > > On 11/3/25 3:10 PM, anis chali wrote: > >>> This breaks state for x86-efi, the original user of efi_late_init(). > > > > Was not sure to add it to the efi_defconfig > > generally speaking, we try not to break existing users and if we do, an > entry should be added to the migration guide explaining what backward > incompatibility was introduced. > > Cheers, > Ahmad > > > > > Le lun. 3 nov. 2025 à 01:40, Ahmad Fatoum <a.fatoum@pengutronix.de> a écrit : > >> > >> Hi, > >> > >> On 03.11.25 05:38, chalianis1@gmail.com wrote: > >>> From: Chali Anis <chalianis1@gmail.com> > >>> > >>> use the external state config to pass the barebox state. > >>> > >>> Signed-off-by: Chali Anis <chalianis1@gmail.com> > >>> --- > >>> arch/arm/configs/efi_v8_defconfig | 2 ++ > >>> efi/payload/init.c | 50 ------------------------------- > >> > >> This breaks state for x86-efi, the original user of efi_late_init(). > >> > >>> 2 files changed, 2 insertions(+), 50 deletions(-) > >>> > >>> diff --git a/arch/arm/configs/efi_v8_defconfig b/arch/arm/configs/efi_v8_defconfig > >>> index 5f946dd51d4a..26682aab304a 100644 > >>> --- a/arch/arm/configs/efi_v8_defconfig > >>> +++ b/arch/arm/configs/efi_v8_defconfig > >>> @@ -21,6 +21,8 @@ CONFIG_CONSOLE_RATP=y > >>> CONFIG_PARTITION_DISK_EFI=y > >>> CONFIG_DEFAULT_ENVIRONMENT_GENERIC_NEW=y > >>> CONFIG_STATE=y > >>> +CONFIG_EXTERNAL_STATE=y > >>> +CONFIG_EXTERNAL_STATE_DTB_PATH="/boot/EFI/barebox/state.dtb" > >>> CONFIG_BOOTCHOOSER=y > >>> CONFIG_RESET_SOURCE=y > >>> CONFIG_MACHINE_ID=y > >>> diff --git a/efi/payload/init.c b/efi/payload/init.c > >>> index 5b827c57ed1f..63d62395cf3f 100644 > >>> --- a/efi/payload/init.c > >>> +++ b/efi/payload/init.c > >>> @@ -376,56 +376,6 @@ static int efi_postcore_init(void) > >>> } > >>> postcore_efi_initcall(efi_postcore_init); > >>> > >>> -static int efi_late_init(void) > >>> -{ > >>> - const char *state_desc = "/boot/EFI/barebox/state.dtb"; > >>> - struct device_node *state_root = NULL; > >>> - size_t size; > >>> - void *fdt; > >>> - int ret; > >>> - > >>> - if (!IS_ENABLED(CONFIG_STATE)) > >>> - return 0; > >>> - > >>> - if (!get_mounted_path("/boot")) { > >>> - pr_warn("boot device couldn't be determined%s\n", > >>> - IS_ENABLED(CONFIG_FS_EFI) ? "" : " without CONFIG_FS_EFI"); > >>> - return 0; > >>> - } > >>> - > >>> - fdt = read_file(state_desc, &size); > >>> - if (!fdt) { > >>> - pr_info("unable to read %s: %m\n", state_desc); > >>> - 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 0; > >>> -} > >>> -late_efi_initcall(efi_late_init); > >>> - > >>> static int do_efiexit(int argc, char *argv[]) > >>> { > >>> if (!BS) > >> > >> > >> -- > >> 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 | > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] drivers: misc: external_state: add a barebox external state. 2025-11-03 4:38 [PATCH 1/2] drivers: misc: external_state: add a barebox external state chalianis1 2025-11-03 4:38 ` [PATCH 2/2] efi: payload: refactor to use the external barebox state driver chalianis1 @ 2025-11-03 6:46 ` Ahmad Fatoum 2025-11-03 14:09 ` anis chali 1 sibling, 1 reply; 10+ messages in thread From: Ahmad Fatoum @ 2025-11-03 6:46 UTC (permalink / raw) To: chalianis1, s.hauer; +Cc: barebox Hi, On 03.11.25 05:38, chalianis1@gmail.com wrote: > From: Chali Anis <chalianis1@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 | ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] drivers: misc: external_state: add a barebox external state. 2025-11-03 6:46 ` [PATCH 1/2] drivers: misc: external_state: add a barebox external state Ahmad Fatoum @ 2025-11-03 14:09 ` anis chali 2025-11-03 14:22 ` Ahmad Fatoum 0 siblings, 1 reply; 10+ messages in thread From: anis chali @ 2025-11-03 14:09 UTC (permalink / raw) To: Ahmad Fatoum; +Cc: barebox >> 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 <a.fatoum@pengutronix.de> a écrit : > > Hi, > > On 03.11.25 05:38, chalianis1@gmail.com wrote: > > From: Chali Anis <chalianis1@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 | ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] drivers: misc: external_state: add a barebox external state. 2025-11-03 14:09 ` anis chali @ 2025-11-03 14:22 ` Ahmad Fatoum 2025-11-03 15:26 ` anis chali 0 siblings, 1 reply; 10+ messages in thread From: Ahmad Fatoum @ 2025-11-03 14:22 UTC (permalink / raw) To: anis chali; +Cc: barebox 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@pengutronix.de> a écrit : >> >> Hi, >> >> On 03.11.25 05:38, chalianis1@gmail.com wrote: >>> From: Chali Anis <chalianis1@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 | ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] drivers: misc: external_state: add a barebox external state. 2025-11-03 14:22 ` Ahmad Fatoum @ 2025-11-03 15:26 ` anis chali 0 siblings, 0 replies; 10+ messages in thread From: anis chali @ 2025-11-03 15:26 UTC (permalink / raw) To: Ahmad Fatoum; +Cc: barebox > 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 Okay, I will come back with something soon. Thank's for support Le lun. 3 nov. 2025 à 09:22, Ahmad Fatoum <a.fatoum@pengutronix.de> a écrit : > > 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@pengutronix.de> a écrit : > >> > >> Hi, > >> > >> On 03.11.25 05:38, chalianis1@gmail.com wrote: > >>> From: Chali Anis <chalianis1@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 | > ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2025-11-03 15:29 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2025-11-03 4:38 [PATCH 1/2] drivers: misc: external_state: add a barebox external state chalianis1 2025-11-03 4:38 ` [PATCH 2/2] efi: payload: refactor to use the external barebox state driver chalianis1 2025-11-03 6:40 ` Ahmad Fatoum 2025-11-03 14:10 ` anis chali 2025-11-03 14:24 ` Ahmad Fatoum 2025-11-03 15:28 ` anis chali 2025-11-03 6:46 ` [PATCH 1/2] drivers: misc: external_state: add a barebox external state Ahmad Fatoum 2025-11-03 14:09 ` anis chali 2025-11-03 14:22 ` Ahmad Fatoum 2025-11-03 15:26 ` anis chali
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox