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

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

[chalianis1]

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

[PATCH 2/2] efi: payload: refactor to use the external barebox state driver.

[chalianis1]

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

Re: [PATCH 2/2] efi: payload: refactor to use the external barebox state driver.

[Ahmad Fatoum]

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 |

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

[Ahmad Fatoum]

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 |

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

[anis chali]

>> 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 |

Re: [PATCH 2/2] efi: payload: refactor to use the external barebox state driver.

[anis chali]

>> 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 |

Re: [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@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 |

Re: [PATCH 2/2] efi: payload: refactor to use the external barebox state driver.

[Ahmad Fatoum]

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 |

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

[anis chali]

> 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 |
>

Re: [PATCH 2/2] efi: payload: refactor to use the external barebox state driver.

[anis chali]

> 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 |
>