[OSS-Tools] [PATCH v4 0/3] improve barebox-state support on EFI system

[OSS-Tools] [PATCH v4 0/3] improve barebox-state support on EFI system

[Michael Olbrich]

Hi,

so the discussion on the barebox ML resulted in a different binding for
this. Sascha has sent patches for that[1]. This is now mainline in Barebox.

So while this is 'v4' for this topic, all the patches except the last one
are actually different, so please drop the old series that is still present
in the next branch.

v2 had some Bugs that have been fixed in v3.
Added some improvements as suggested by Ahmad in v4.

In the device-tree it now looks like this:

----------------------------------------------------------------------
/ {
[...]
        state: state {
[...]
                backend = <&barebox_state>;
[...]
        };

        disk {
                compatible = "barebox,storage-by-uuid";
                uuid = "deadbeaf";

                partitions {
                        compatible = "fixed-partitions";
                        #address-cells = <2>;
                        #size-cells = <2>;

                        barebox_state: state@300000 {
                                label = "barebox-state";
                                reg = <0x0 0x300000 0x0 0x100000>;
                        };
                };
        };
};
----------------------------------------------------------------------

Regards,
Michael

[1] https://lore.barebox.org/barebox/20220207094953.949868-1-s.hauer@pengutronix.de/T/#t

Michael Olbrich (3):
  libdt: only requires a partname for mtd
  libdt: add support for barebox,storage-by-uuid
  state: automatically find state.dtb in the ESP

 src/barebox-state.c | 25 +++++++++++++
 src/libdt.c         | 91 ++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 103 insertions(+), 13 deletions(-)

-- 
2.30.2

[OSS-Tools] [PATCH v4 1/3] libdt: only requires a partname for mtd

[Michael Olbrich]

It's not used anywhere else.

Signed-off-by: Michael Olbrich <m.olbrich@pengutronix.de>
---

No changes in v3 and v4.

 src/libdt.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/src/libdt.c b/src/libdt.c
index 59e76d336d8d..48c31931e8a1 100644
--- a/src/libdt.c
+++ b/src/libdt.c
@@ -2381,7 +2381,6 @@ int of_get_devicepath(struct device_node *partition_node, char **devpath, off_t
 {
 	struct device_node *node;
 	struct udev_device *dev, *partdev, *mtd;
-	const char *partname;
 	int ret;
 
 	*offset = 0;
@@ -2451,16 +2450,18 @@ int of_get_devicepath(struct device_node *partition_node, char **devpath, off_t
 		return -ENODEV;
 	}
 
-	/* find the name of the partition... */
-	ret = of_property_read_string(partition_node, "label", &partname);
-	if (ret) {
-		fprintf(stderr, "%s: no 'label' property found in %s\n", __func__,
-			partition_node->full_name);
-		return ret;
-	}
-
 	mtd = of_find_mtd_device(dev);
 	if (mtd) {
+		const char *partname;
+
+		/* find the name of the partition... */
+		ret = of_property_read_string(partition_node, "label", &partname);
+		if (ret) {
+			fprintf(stderr, "%s: no 'label' property found in %s\n", __func__,
+				partition_node->full_name);
+			return ret;
+		}
+
 		/* ...find the udev_device by partition name... */
 		partdev = device_find_mtd_partition(dev, partname);
 		if (!partdev)
-- 
2.30.2

[OSS-Tools] [PATCH v4 2/3] libdt: add support for barebox, storage-by-uuid

[Michael Olbrich]

Signed-off-by: Michael Olbrich <m.olbrich@pengutronix.de>
---

changes in v3:
uuid comparison fixed:
 - compare the correct uuids
 - don't crash when no uuid is present

changes in v4:
use strcasecmp() to make the uuid check case insensitive.

 src/libdt.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 69 insertions(+), 5 deletions(-)

diff --git a/src/libdt.c b/src/libdt.c
index 48c31931e8a1..4076d9a2f4a3 100644
--- a/src/libdt.c
+++ b/src/libdt.c
@@ -2358,6 +2358,52 @@ out:
 	return dev;
 }
 
+static struct udev_device *of_find_device_by_uuid(const char *uuid)
+{
+	struct udev *udev;
+	struct udev_enumerate *enumerate;
+	struct udev_list_entry *devices, *dev_list_entry;
+	int ret = 0;
+
+	udev = udev_new();
+	if (!udev) {
+		  fprintf(stderr, "Can't create udev\n");
+		  return NULL;
+	}
+
+	enumerate = udev_enumerate_new(udev);
+	udev_enumerate_add_match_subsystem(enumerate, "block");
+	udev_enumerate_scan_devices(enumerate);
+	devices = udev_enumerate_get_list_entry(enumerate);
+	udev_list_entry_foreach(dev_list_entry, devices) {
+		const char *path, *devtype, *outpath, *dev_uuid;
+		struct udev_device *device;
+
+		path = udev_list_entry_get_name(dev_list_entry);
+		device = udev_device_new_from_syspath(udev, path);
+
+		/* distinguish device (disk) from partitions */
+		devtype = udev_device_get_devtype(device);
+		if (!devtype)
+			continue;
+		if (!strcmp(devtype, "disk")) {
+			dev_uuid = udev_device_get_property_value(device, "ID_PART_TABLE_UUID");
+			if (dev_uuid && !strcasecmp(dev_uuid, uuid)) {
+				outpath = udev_device_get_devnode(device);
+				return device;
+			}
+		} else if (!strcmp(devtype, "partition")) {
+			dev_uuid = udev_device_get_property_value(device, "ID_PART_ENTRY_UUID");
+			if (dev_uuid && !strcasecmp(dev_uuid, uuid)) {
+				outpath = udev_device_get_devnode(device);
+				return device;
+			}
+		}
+
+	}
+	return NULL;
+}
+
 /*
  * of_get_devicepath - get information how to access device corresponding to a device_node
  * @partition_node:	The device_node which shall be accessed
@@ -2443,11 +2489,29 @@ int of_get_devicepath(struct device_node *partition_node, char **devpath, off_t
 	if (!strcmp(node->name, "partitions"))
 		node = node->parent;
 
-	dev = of_find_device_by_node_path(node->full_name);
-	if (!dev) {
-		fprintf(stderr, "%s: cannot find device from node %s\n", __func__,
-				node->full_name);
-		return -ENODEV;
+	if (of_device_is_compatible(node, "barebox,storage-by-uuid")) {
+		const char *uuid;
+
+		ret = of_property_read_string(node, "uuid", &uuid);
+		if (ret) {
+			fprintf(stderr, "%s: missing uuid property for %s\n", __func__,
+					node->full_name);
+			return -ENODEV;
+		}
+		dev = of_find_device_by_uuid(uuid);
+		if (!dev) {
+			fprintf(stderr, "%s: cannot find device for uuid %s\n", __func__,
+				uuid);
+			return -ENODEV;
+		}
+	}
+	else {
+		dev = of_find_device_by_node_path(node->full_name);
+		if (!dev) {
+			fprintf(stderr, "%s: cannot find device from node %s\n", __func__,
+					node->full_name);
+			return -ENODEV;
+		}
 	}
 
 	mtd = of_find_mtd_device(dev);
-- 
2.30.2

[OSS-Tools] [PATCH v4 3/3] state: automatically find state.dtb in the ESP

[Michael Olbrich]

Systemd mounts the EFI System Partition (ESP) to /boot or /efi.
So look there for the state.dtb when the devicetree in sysfs/procfs is
not available.

This way barebox-state can be used on EFI systems without manually
specifying the devicetree file.

Signed-off-by: Michael Olbrich <m.olbrich@pengutronix.de>
---

changes in v4:
add /boot/efi/EFI/BAREBOX/state.dtb to the seach list.

 src/barebox-state.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/src/barebox-state.c b/src/barebox-state.c
index 334aed6f3d43..c5acd1f7780a 100644
--- a/src/barebox-state.c
+++ b/src/barebox-state.c
@@ -342,6 +342,31 @@ struct state *state_get(const char *name, const char *filename, bool readonly, b
 		}
 	} else {
 		root = of_read_proc_devicetree();
+
+		/* No device-tree in procfs / sysfs, try dtb file in the ESP */
+		if (-PTR_ERR(root) == ENOENT) {
+			const char *paths[] = {
+				/* default mount paths used by systemd */
+				"/boot/EFI/BAREBOX/state.dtb",
+				"/boot/efi/EFI/BAREBOX/state.dtb",
+				"/efi/EFI/BAREBOX/state.dtb",
+				NULL
+			};
+			void *fdt;
+			int i;
+
+			for (i = 0; paths[i]; ++i) {
+				fdt = read_file(paths[i], NULL);
+				if (fdt)
+					break;
+			}
+			if (fdt) {
+				root = of_unflatten_dtb(fdt);
+				free(fdt);
+			}
+			else
+				root = ERR_PTR(-ENOENT);
+		}
 		if (IS_ERR(root)) {
 			pr_err("Unable to read devicetree. %s\n",
 			       strerror(-PTR_ERR(root)));
-- 
2.30.2

Re: [OSS-Tools] [PATCH v4 2/3] libdt: add support for barebox, storage-by-uuid

[Roland Hieber]

On Wed, May 11, 2022 at 10:21:24AM +0200, Michael Olbrich wrote:
> Signed-off-by: Michael Olbrich <m.olbrich@pengutronix.de>
> ---
> 
> changes in v3:
> uuid comparison fixed:
>  - compare the correct uuids
>  - don't crash when no uuid is present
> 
> changes in v4:
> use strcasecmp() to make the uuid check case insensitive.
> 
>  src/libdt.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 69 insertions(+), 5 deletions(-)
> 
> diff --git a/src/libdt.c b/src/libdt.c
> index 48c31931e8a1..4076d9a2f4a3 100644
> --- a/src/libdt.c
> +++ b/src/libdt.c
> @@ -2358,6 +2358,52 @@ out:
>  	return dev;
>  }
>  
> +static struct udev_device *of_find_device_by_uuid(const char *uuid)
> +{
> +	struct udev *udev;
> +	struct udev_enumerate *enumerate;
> +	struct udev_list_entry *devices, *dev_list_entry;
> +	int ret = 0;
> +
> +	udev = udev_new();

According to the udev docs, I think this should be udev_unref()ed
somewhere, see for example in of_find_mtd_device() above.

Hmm… looking through the existing code there are more places which could
profit from an udev_unref()…

> +	if (!udev) {
> +		  fprintf(stderr, "Can't create udev\n");
> +		  return NULL;
> +	}
> +
> +	enumerate = udev_enumerate_new(udev);
> +	udev_enumerate_add_match_subsystem(enumerate, "block");
> +	udev_enumerate_scan_devices(enumerate);
> +	devices = udev_enumerate_get_list_entry(enumerate);
> +	udev_list_entry_foreach(dev_list_entry, devices) {
> +		const char *path, *devtype, *outpath, *dev_uuid;
> +		struct udev_device *device;
> +
> +		path = udev_list_entry_get_name(dev_list_entry);
> +		device = udev_device_new_from_syspath(udev, path);
> +

Same for a udev_device_unref(), somewhere…?

 - Roland

> +		/* distinguish device (disk) from partitions */
> +		devtype = udev_device_get_devtype(device);
> +		if (!devtype)
> +			continue;
> +		if (!strcmp(devtype, "disk")) {
> +			dev_uuid = udev_device_get_property_value(device, "ID_PART_TABLE_UUID");
> +			if (dev_uuid && !strcasecmp(dev_uuid, uuid)) {
> +				outpath = udev_device_get_devnode(device);
> +				return device;
> +			}
> +		} else if (!strcmp(devtype, "partition")) {
> +			dev_uuid = udev_device_get_property_value(device, "ID_PART_ENTRY_UUID");
> +			if (dev_uuid && !strcasecmp(dev_uuid, uuid)) {
> +				outpath = udev_device_get_devnode(device);
> +				return device;
> +			}
> +		}
> +
> +	}
> +	return NULL;
> +}
> +
>  /*
>   * of_get_devicepath - get information how to access device corresponding to a device_node
>   * @partition_node:	The device_node which shall be accessed
> @@ -2443,11 +2489,29 @@ int of_get_devicepath(struct device_node *partition_node, char **devpath, off_t
>  	if (!strcmp(node->name, "partitions"))
>  		node = node->parent;
>  
> -	dev = of_find_device_by_node_path(node->full_name);
> -	if (!dev) {
> -		fprintf(stderr, "%s: cannot find device from node %s\n", __func__,
> -				node->full_name);
> -		return -ENODEV;
> +	if (of_device_is_compatible(node, "barebox,storage-by-uuid")) {
> +		const char *uuid;
> +
> +		ret = of_property_read_string(node, "uuid", &uuid);
> +		if (ret) {
> +			fprintf(stderr, "%s: missing uuid property for %s\n", __func__,
> +					node->full_name);
> +			return -ENODEV;
> +		}
> +		dev = of_find_device_by_uuid(uuid);
> +		if (!dev) {
> +			fprintf(stderr, "%s: cannot find device for uuid %s\n", __func__,
> +				uuid);
> +			return -ENODEV;
> +		}
> +	}
> +	else {
> +		dev = of_find_device_by_node_path(node->full_name);
> +		if (!dev) {
> +			fprintf(stderr, "%s: cannot find device from node %s\n", __func__,
> +					node->full_name);
> +			return -ENODEV;
> +		}
>  	}
>  
>  	mtd = of_find_mtd_device(dev);
> -- 
> 2.30.2
> 
> 
> 

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

Re: [OSS-Tools] [PATCH v4 0/3] improve barebox-state support on EFI system

[Roland Hieber]

On Wed, May 11, 2022 at 10:21:22AM +0200, Michael Olbrich wrote:
> Hi,
> 
> so the discussion on the barebox ML resulted in a different binding for
> this. Sascha has sent patches for that[1]. This is now mainline in Barebox.
> 
> So while this is 'v4' for this topic, all the patches except the last one
> are actually different, so please drop the old series that is still present
> in the next branch.

I noticed that you dropped the deep probe patch from your v2 to v4 too,
but I think I will keep it to reduce the diff between barebox' and
dt-utils' state.c, even if it's a noop on the userland side.

 - Roland

> v2 had some Bugs that have been fixed in v3.
> Added some improvements as suggested by Ahmad in v4.
> 
> In the device-tree it now looks like this:
> 
> ----------------------------------------------------------------------
> / {
> [...]
>         state: state {
> [...]
>                 backend = <&barebox_state>;
> [...]
>         };
> 
>         disk {
>                 compatible = "barebox,storage-by-uuid";
>                 uuid = "deadbeaf";
> 
>                 partitions {
>                         compatible = "fixed-partitions";
>                         #address-cells = <2>;
>                         #size-cells = <2>;
> 
>                         barebox_state: state@300000 {
>                                 label = "barebox-state";
>                                 reg = <0x0 0x300000 0x0 0x100000>;
>                         };
>                 };
>         };
> };
> ----------------------------------------------------------------------
> 
> Regards,
> Michael
> 
> [1] https://lore.barebox.org/barebox/20220207094953.949868-1-s.hauer@pengutronix.de/T/#t
> 
> Michael Olbrich (3):
>   libdt: only requires a partname for mtd
>   libdt: add support for barebox,storage-by-uuid
>   state: automatically find state.dtb in the ESP
> 
>  src/barebox-state.c | 25 +++++++++++++
>  src/libdt.c         | 91 ++++++++++++++++++++++++++++++++++++++-------
>  2 files changed, 103 insertions(+), 13 deletions(-)
> 
> -- 
> 2.30.2
> 
> 
> 

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

Re: [OSS-Tools] [PATCH v4 2/3] libdt: add support for barebox, storage-by-uuid

[Michael Olbrich]

On Sun, May 15, 2022 at 10:40:55PM +0200, Roland Hieber wrote:
> On Wed, May 11, 2022 at 10:21:24AM +0200, Michael Olbrich wrote:
> > Signed-off-by: Michael Olbrich <m.olbrich@pengutronix.de>
> > ---
> > 
> > changes in v3:
> > uuid comparison fixed:
> >  - compare the correct uuids
> >  - don't crash when no uuid is present
> > 
> > changes in v4:
> > use strcasecmp() to make the uuid check case insensitive.
> > 
> >  src/libdt.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++----
> >  1 file changed, 69 insertions(+), 5 deletions(-)
> > 
> > diff --git a/src/libdt.c b/src/libdt.c
> > index 48c31931e8a1..4076d9a2f4a3 100644
> > --- a/src/libdt.c
> > +++ b/src/libdt.c
> > @@ -2358,6 +2358,52 @@ out:
> >  	return dev;
> >  }
> >  
> > +static struct udev_device *of_find_device_by_uuid(const char *uuid)
> > +{
> > +	struct udev *udev;
> > +	struct udev_enumerate *enumerate;
> > +	struct udev_list_entry *devices, *dev_list_entry;
> > +	int ret = 0;
> > +
> > +	udev = udev_new();
> 
> According to the udev docs, I think this should be udev_unref()ed
> somewhere, see for example in of_find_mtd_device() above.
> 
> Hmm… looking through the existing code there are more places which could
> profit from an udev_unref()…

That's not possible: The returned udev_device keeps a pointer udev but has
not refcount :-/.
So destroying the udev struct is probably not save until all udev devices
are destroyed.

> > +	if (!udev) {
> > +		  fprintf(stderr, "Can't create udev\n");
> > +		  return NULL;
> > +	}
> > +
> > +	enumerate = udev_enumerate_new(udev);
> > +	udev_enumerate_add_match_subsystem(enumerate, "block");
> > +	udev_enumerate_scan_devices(enumerate);
> > +	devices = udev_enumerate_get_list_entry(enumerate);
> > +	udev_list_entry_foreach(dev_list_entry, devices) {
> > +		const char *path, *devtype, *outpath, *dev_uuid;
> > +		struct udev_device *device;
> > +
> > +		path = udev_list_entry_get_name(dev_list_entry);
> > +		device = udev_device_new_from_syspath(udev, path);
> > +
> 
> Same for a udev_device_unref(), somewhere…?

Well, one device is returned so I could only unref those that don't match.
And the returned on leaks as well (that's not new. It already happens in
with the existing code).

And I just copied this loop form existing code and modified. So if you
really want to fix all memory leaks then that's a lot more work.
And I have neither the time to do it nor can I actually test all the other
code.

Michael

> > +		/* distinguish device (disk) from partitions */
> > +		devtype = udev_device_get_devtype(device);
> > +		if (!devtype)
> > +			continue;
> > +		if (!strcmp(devtype, "disk")) {
> > +			dev_uuid = udev_device_get_property_value(device, "ID_PART_TABLE_UUID");
> > +			if (dev_uuid && !strcasecmp(dev_uuid, uuid)) {
> > +				outpath = udev_device_get_devnode(device);
> > +				return device;
> > +			}
> > +		} else if (!strcmp(devtype, "partition")) {
> > +			dev_uuid = udev_device_get_property_value(device, "ID_PART_ENTRY_UUID");
> > +			if (dev_uuid && !strcasecmp(dev_uuid, uuid)) {
> > +				outpath = udev_device_get_devnode(device);
> > +				return device;
> > +			}
> > +		}
> > +
> > +	}
> > +	return NULL;
> > +}
> > +
> >  /*
> >   * of_get_devicepath - get information how to access device corresponding to a device_node
> >   * @partition_node:	The device_node which shall be accessed
> > @@ -2443,11 +2489,29 @@ int of_get_devicepath(struct device_node *partition_node, char **devpath, off_t
> >  	if (!strcmp(node->name, "partitions"))
> >  		node = node->parent;
> >  
> > -	dev = of_find_device_by_node_path(node->full_name);
> > -	if (!dev) {
> > -		fprintf(stderr, "%s: cannot find device from node %s\n", __func__,
> > -				node->full_name);
> > -		return -ENODEV;
> > +	if (of_device_is_compatible(node, "barebox,storage-by-uuid")) {
> > +		const char *uuid;
> > +
> > +		ret = of_property_read_string(node, "uuid", &uuid);
> > +		if (ret) {
> > +			fprintf(stderr, "%s: missing uuid property for %s\n", __func__,
> > +					node->full_name);
> > +			return -ENODEV;
> > +		}
> > +		dev = of_find_device_by_uuid(uuid);
> > +		if (!dev) {
> > +			fprintf(stderr, "%s: cannot find device for uuid %s\n", __func__,
> > +				uuid);
> > +			return -ENODEV;
> > +		}
> > +	}
> > +	else {
> > +		dev = of_find_device_by_node_path(node->full_name);
> > +		if (!dev) {
> > +			fprintf(stderr, "%s: cannot find device from node %s\n", __func__,
> > +					node->full_name);
> > +			return -ENODEV;
> > +		}
> >  	}
> >  
> >  	mtd = of_find_mtd_device(dev);
> > -- 
> > 2.30.2
> > 
> > 
> > 
> 
> -- 
> Roland Hieber, Pengutronix e.K.          | r.hieber@pengutronix.de     |
> Steuerwalder Str. 21                     | https://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: [OSS-Tools] [PATCH v4 0/3] improve barebox-state support on EFI system

[Michael Olbrich]

On Sun, May 15, 2022 at 11:04:39PM +0200, Roland Hieber wrote:
> On Wed, May 11, 2022 at 10:21:22AM +0200, Michael Olbrich wrote:
> > Hi,
> > 
> > so the discussion on the barebox ML resulted in a different binding for
> > this. Sascha has sent patches for that[1]. This is now mainline in Barebox.
> > 
> > So while this is 'v4' for this topic, all the patches except the last one
> > are actually different, so please drop the old series that is still present
> > in the next branch.
> 
> I noticed that you dropped the deep probe patch from your v2 to v4 too,
> but I think I will keep it to reduce the diff between barebox' and
> dt-utils' state.c, even if it's a noop on the userland side.

I added that patch to ensure that my patch applied to both files. But I
think there where other changes as well in code that I didn't touch. So if
you want to minimize the diff then you should probably pick some more
commits.

Michael

> > v2 had some Bugs that have been fixed in v3.
> > Added some improvements as suggested by Ahmad in v4.
> > 
> > In the device-tree it now looks like this:
> > 
> > ----------------------------------------------------------------------
> > / {
> > [...]
> >         state: state {
> > [...]
> >                 backend = <&barebox_state>;
> > [...]
> >         };
> > 
> >         disk {
> >                 compatible = "barebox,storage-by-uuid";
> >                 uuid = "deadbeaf";
> > 
> >                 partitions {
> >                         compatible = "fixed-partitions";
> >                         #address-cells = <2>;
> >                         #size-cells = <2>;
> > 
> >                         barebox_state: state@300000 {
> >                                 label = "barebox-state";
> >                                 reg = <0x0 0x300000 0x0 0x100000>;
> >                         };
> >                 };
> >         };
> > };
> > ----------------------------------------------------------------------
> > 
> > Regards,
> > Michael
> > 
> > [1] https://lore.barebox.org/barebox/20220207094953.949868-1-s.hauer@pengutronix.de/T/#t
> > 
> > Michael Olbrich (3):
> >   libdt: only requires a partname for mtd
> >   libdt: add support for barebox,storage-by-uuid
> >   state: automatically find state.dtb in the ESP
> > 
> >  src/barebox-state.c | 25 +++++++++++++
> >  src/libdt.c         | 91 ++++++++++++++++++++++++++++++++++++++-------
> >  2 files changed, 103 insertions(+), 13 deletions(-)
> > 
> > -- 
> > 2.30.2
> > 
> > 
> > 
> 
> -- 
> Roland Hieber, Pengutronix e.K.          | r.hieber@pengutronix.de     |
> Steuerwalder Str. 21                     | https://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 |

[OSS-Tools] [PATCH fixup 1/2] libdt: remove ultimately unused variables

[Ahmad Fatoum]

These variables were recently added, but are unused. Drop them.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 src/libdt.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/src/libdt.c b/src/libdt.c
index 44491a5e739b..302ca7a76375 100644
--- a/src/libdt.c
+++ b/src/libdt.c
@@ -2365,7 +2365,6 @@ static struct udev_device *of_find_device_by_uuid(struct udev_device *parent,
 	struct udev *udev;
 	struct udev_enumerate *enumerate;
 	struct udev_list_entry *devices, *dev_list_entry;
-	int ret = 0;
 
 	udev = udev_new();
 	if (!udev) {
@@ -2380,7 +2379,7 @@ static struct udev_device *of_find_device_by_uuid(struct udev_device *parent,
 	udev_enumerate_scan_devices(enumerate);
 	devices = udev_enumerate_get_list_entry(enumerate);
 	udev_list_entry_foreach(dev_list_entry, devices) {
-		const char *path, *devtype, *outpath, *dev_uuid;
+		const char *path, *devtype, *dev_uuid;
 		struct udev_device *device;
 		const char *property;
 
@@ -2400,10 +2399,8 @@ static struct udev_device *of_find_device_by_uuid(struct udev_device *parent,
 			property = "ID_PART_ENTRY_UUID";
 
 		dev_uuid = udev_device_get_property_value(device, property);
-		if (dev_uuid && !strcasecmp(dev_uuid, uuid)) {
-			outpath = udev_device_get_devnode(device);
+		if (dev_uuid && !strcasecmp(dev_uuid, uuid))
 			return device;
-		}
 	}
 	return NULL;
 }
-- 
2.39.2

[OSS-Tools] [PATCH fixup 2/2] libdt: fix possible use of uninitialized variable

[Ahmad Fatoum]

property may not be always defined. So let's skip trying to do something
with it in that case.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 src/libdt.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/libdt.c b/src/libdt.c
index 302ca7a76375..ca0502ac9483 100644
--- a/src/libdt.c
+++ b/src/libdt.c
@@ -2397,6 +2397,8 @@ static struct udev_device *of_find_device_by_uuid(struct udev_device *parent,
 			property = "ID_PART_TABLE_UUID";
 		else if (!strcmp(devtype, "partition"))
 			property = "ID_PART_ENTRY_UUID";
+		else
+			continue;
 
 		dev_uuid = udev_device_get_property_value(device, property);
 		if (dev_uuid && !strcasecmp(dev_uuid, uuid))
-- 
2.39.2

Re: [OSS-Tools] [PATCH v4 2/3] libdt: add support for barebox, storage-by-uuid

[Ahmad Fatoum]

Hello Michael,

On 11.05.22 10:21, Michael Olbrich wrote:
> Signed-off-by: Michael Olbrich <m.olbrich@pengutronix.de>

Tested-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

I'd squash the two fixups I just sent when I apply this.

Thanks,
Ahmad

> ---
> 
> changes in v3:
> uuid comparison fixed:
>  - compare the correct uuids
>  - don't crash when no uuid is present
> 
> changes in v4:
> use strcasecmp() to make the uuid check case insensitive.
> 
>  src/libdt.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 69 insertions(+), 5 deletions(-)
> 
> diff --git a/src/libdt.c b/src/libdt.c
> index 48c31931e8a1..4076d9a2f4a3 100644
> --- a/src/libdt.c
> +++ b/src/libdt.c
> @@ -2358,6 +2358,52 @@ out:
>  	return dev;
>  }
>  
> +static struct udev_device *of_find_device_by_uuid(const char *uuid)
> +{
> +	struct udev *udev;
> +	struct udev_enumerate *enumerate;
> +	struct udev_list_entry *devices, *dev_list_entry;
> +	int ret = 0;
> +
> +	udev = udev_new();
> +	if (!udev) {
> +		  fprintf(stderr, "Can't create udev\n");
> +		  return NULL;
> +	}
> +
> +	enumerate = udev_enumerate_new(udev);
> +	udev_enumerate_add_match_subsystem(enumerate, "block");
> +	udev_enumerate_scan_devices(enumerate);
> +	devices = udev_enumerate_get_list_entry(enumerate);
> +	udev_list_entry_foreach(dev_list_entry, devices) {
> +		const char *path, *devtype, *outpath, *dev_uuid;
> +		struct udev_device *device;
> +
> +		path = udev_list_entry_get_name(dev_list_entry);
> +		device = udev_device_new_from_syspath(udev, path);
> +
> +		/* distinguish device (disk) from partitions */
> +		devtype = udev_device_get_devtype(device);
> +		if (!devtype)
> +			continue;
> +		if (!strcmp(devtype, "disk")) {
> +			dev_uuid = udev_device_get_property_value(device, "ID_PART_TABLE_UUID");
> +			if (dev_uuid && !strcasecmp(dev_uuid, uuid)) {
> +				outpath = udev_device_get_devnode(device);
> +				return device;
> +			}
> +		} else if (!strcmp(devtype, "partition")) {
> +			dev_uuid = udev_device_get_property_value(device, "ID_PART_ENTRY_UUID");
> +			if (dev_uuid && !strcasecmp(dev_uuid, uuid)) {
> +				outpath = udev_device_get_devnode(device);
> +				return device;
> +			}
> +		}
> +
> +	}
> +	return NULL;
> +}
> +
>  /*
>   * of_get_devicepath - get information how to access device corresponding to a device_node
>   * @partition_node:	The device_node which shall be accessed
> @@ -2443,11 +2489,29 @@ int of_get_devicepath(struct device_node *partition_node, char **devpath, off_t
>  	if (!strcmp(node->name, "partitions"))
>  		node = node->parent;
>  
> -	dev = of_find_device_by_node_path(node->full_name);
> -	if (!dev) {
> -		fprintf(stderr, "%s: cannot find device from node %s\n", __func__,
> -				node->full_name);
> -		return -ENODEV;
> +	if (of_device_is_compatible(node, "barebox,storage-by-uuid")) {
> +		const char *uuid;
> +
> +		ret = of_property_read_string(node, "uuid", &uuid);
> +		if (ret) {
> +			fprintf(stderr, "%s: missing uuid property for %s\n", __func__,
> +					node->full_name);
> +			return -ENODEV;
> +		}
> +		dev = of_find_device_by_uuid(uuid);
> +		if (!dev) {
> +			fprintf(stderr, "%s: cannot find device for uuid %s\n", __func__,
> +				uuid);
> +			return -ENODEV;
> +		}
> +	}
> +	else {
> +		dev = of_find_device_by_node_path(node->full_name);
> +		if (!dev) {
> +			fprintf(stderr, "%s: cannot find device from node %s\n", __func__,
> +					node->full_name);
> +			return -ENODEV;
> +		}
>  	}
>  
>  	mtd = of_find_mtd_device(dev);

-- 
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: [OSS-Tools] [PATCH fixup 1/2] libdt: remove ultimately unused variables

[Roland Hieber]

On Wed, May 31, 2023 at 05:13:32PM +0200, Ahmad Fatoum wrote:
> These variables were recently added, but are unused. Drop them.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  src/libdt.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/src/libdt.c b/src/libdt.c
> index 44491a5e739b..302ca7a76375 100644
> --- a/src/libdt.c
> +++ b/src/libdt.c
> @@ -2365,7 +2365,6 @@ static struct udev_device *of_find_device_by_uuid(struct udev_device *parent,
>  	struct udev *udev;
>  	struct udev_enumerate *enumerate;
>  	struct udev_list_entry *devices, *dev_list_entry;
> -	int ret = 0;
>  
>  	udev = udev_new();
>  	if (!udev) {
> @@ -2380,7 +2379,7 @@ static struct udev_device *of_find_device_by_uuid(struct udev_device *parent,
>  	udev_enumerate_scan_devices(enumerate);
>  	devices = udev_enumerate_get_list_entry(enumerate);
>  	udev_list_entry_foreach(dev_list_entry, devices) {
> -		const char *path, *devtype, *outpath, *dev_uuid;
> +		const char *path, *devtype, *dev_uuid;
>  		struct udev_device *device;
>  		const char *property;
>  
> @@ -2400,10 +2399,8 @@ static struct udev_device *of_find_device_by_uuid(struct udev_device *parent,
>  			property = "ID_PART_ENTRY_UUID";

I'm not completely sure which version of the code you are referencing
here because I cannot find that context, but …

>  		dev_uuid = udev_device_get_property_value(device, property);
> -		if (dev_uuid && !strcasecmp(dev_uuid, uuid)) {
> -			outpath = udev_device_get_devnode(device);

in mol's v4 patch there's another "outpath" occurrence in the
'if (!strcmp(devtype, "disk"))' block above that.

 - Roland

> +		if (dev_uuid && !strcasecmp(dev_uuid, uuid))
>  			return device;
> -		}
>  	}
>  	return NULL;
>  }
> -- 
> 2.39.2
> 
> 
> 

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

Re: [OSS-Tools] [PATCH fixup 1/2] libdt: remove ultimately unused variables

[Ahmad Fatoum]

On 05.06.23 11:15, Roland Hieber wrote:
> On Wed, May 31, 2023 at 05:13:32PM +0200, Ahmad Fatoum wrote:
>> These variables were recently added, but are unused. Drop them.
>>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> ---
>>  src/libdt.c | 7 ++-----
>>  1 file changed, 2 insertions(+), 5 deletions(-)
>>
>> diff --git a/src/libdt.c b/src/libdt.c
>> index 44491a5e739b..302ca7a76375 100644
>> --- a/src/libdt.c
>> +++ b/src/libdt.c
>> @@ -2365,7 +2365,6 @@ static struct udev_device *of_find_device_by_uuid(struct udev_device *parent,
>>  	struct udev *udev;
>>  	struct udev_enumerate *enumerate;
>>  	struct udev_list_entry *devices, *dev_list_entry;
>> -	int ret = 0;
>>  
>>  	udev = udev_new();
>>  	if (!udev) {
>> @@ -2380,7 +2379,7 @@ static struct udev_device *of_find_device_by_uuid(struct udev_device *parent,
>>  	udev_enumerate_scan_devices(enumerate);
>>  	devices = udev_enumerate_get_list_entry(enumerate);
>>  	udev_list_entry_foreach(dev_list_entry, devices) {
>> -		const char *path, *devtype, *outpath, *dev_uuid;
>> +		const char *path, *devtype, *dev_uuid;
>>  		struct udev_device *device;
>>  		const char *property;
>>  
>> @@ -2400,10 +2399,8 @@ static struct udev_device *of_find_device_by_uuid(struct udev_device *parent,
>>  			property = "ID_PART_ENTRY_UUID";
> 
> I'm not completely sure which version of the code you are referencing
> here because I cannot find that context, but …

I replied on mol's series with two fixups that should be squashed. One of them
removes *outdir, which is written, but never read.

> 
>>  		dev_uuid = udev_device_get_property_value(device, property);
>> -		if (dev_uuid && !strcasecmp(dev_uuid, uuid)) {
>> -			outpath = udev_device_get_devnode(device);
> 
> in mol's v4 patch there's another "outpath" occurrence in the
> 'if (!strcmp(devtype, "disk"))' block above that.
> 
>  - Roland
> 
>> +		if (dev_uuid && !strcasecmp(dev_uuid, uuid))
>>  			return device;
>> -		}
>>  	}
>>  	return NULL;
>>  }
>> -- 
>> 2.39.2
>>
>>
>>
> 

-- 
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: [OSS-Tools] [PATCH fixup 1/2] libdt: remove ultimately unused variables

[Ahmad Fatoum]

On 05.06.23 11:30, Ahmad Fatoum wrote:
>> I'm not completely sure which version of the code you are referencing
>> here because I cannot find that context, but …
> 
> I replied on mol's series with two fixups that should be squashed. One of them
> removes *outdir, which is written, but never read.

Sorry, I was confused. Please find attached a patch that actually applies on v4.
The other fixup was out-of-place.


------------------ 8< ---------------------


diff --git a/src/libdt.c b/src/libdt.c
index de8209ce53ae..019a7555a6f6 100644
--- a/src/libdt.c
+++ b/src/libdt.c
@@ -2363,7 +2363,6 @@ static struct udev_device *of_find_device_by_uuid(const char *uuid)
        struct udev *udev;
        struct udev_enumerate *enumerate;
        struct udev_list_entry *devices, *dev_list_entry;
-       int ret = 0;

        udev = udev_new();
        if (!udev) {
@@ -2376,7 +2375,7 @@ static struct udev_device *of_find_device_by_uuid(const char *uuid)
        udev_enumerate_scan_devices(enumerate);
        devices = udev_enumerate_get_list_entry(enumerate);
        udev_list_entry_foreach(dev_list_entry, devices) {
-               const char *path, *devtype, *outpath, *dev_uuid;
+               const char *path, *devtype, *dev_uuid;
                struct udev_device *device;

                path = udev_list_entry_get_name(dev_list_entry);
@@ -2388,16 +2387,12 @@ static struct udev_device *of_find_device_by_uuid(const char *uuid)
                        continue;
                if (!strcmp(devtype, "disk")) {
                        dev_uuid = udev_device_get_property_value(device, "ID_PART_TABLE_UUID");
-                       if (dev_uuid && !strcasecmp(dev_uuid, uuid)) {
-                               outpath = udev_device_get_devnode(device);
+                       if (dev_uuid && !strcasecmp(dev_uuid, uuid))
                                return device;
-                       }
                } else if (!strcmp(devtype, "partition")) {
                        dev_uuid = udev_device_get_property_value(device, "ID_PART_ENTRY_UUID");
-                       if (dev_uuid && !strcasecmp(dev_uuid, uuid)) {
-                               outpath = udev_device_get_devnode(device);
+                       if (dev_uuid && !strcasecmp(dev_uuid, uuid))
                                return device;
-                       }
                }

        }


-- 
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: [OSS-Tools] [PATCH fixup 1/2] libdt: remove ultimately unused variables

[Roland Hieber]

On Mon, Jun 05, 2023 at 02:31:01PM +0200, Ahmad Fatoum wrote:
> On 05.06.23 11:30, Ahmad Fatoum wrote:
> >> I'm not completely sure which version of the code you are referencing
> >> here because I cannot find that context, but …
> > 
> > I replied on mol's series with two fixups that should be squashed. One of them
> > removes *outdir, which is written, but never read.
> 
> Sorry, I was confused. Please find attached a patch that actually applies on v4.
> The other fixup was out-of-place.

Yep, looks correct to me now.

 - Roland

> 
> 
> ------------------ 8< ---------------------
> 
> 
> diff --git a/src/libdt.c b/src/libdt.c
> index de8209ce53ae..019a7555a6f6 100644
> --- a/src/libdt.c
> +++ b/src/libdt.c
> @@ -2363,7 +2363,6 @@ static struct udev_device *of_find_device_by_uuid(const char *uuid)
>         struct udev *udev;
>         struct udev_enumerate *enumerate;
>         struct udev_list_entry *devices, *dev_list_entry;
> -       int ret = 0;
> 
>         udev = udev_new();
>         if (!udev) {
> @@ -2376,7 +2375,7 @@ static struct udev_device *of_find_device_by_uuid(const char *uuid)
>         udev_enumerate_scan_devices(enumerate);
>         devices = udev_enumerate_get_list_entry(enumerate);
>         udev_list_entry_foreach(dev_list_entry, devices) {
> -               const char *path, *devtype, *outpath, *dev_uuid;
> +               const char *path, *devtype, *dev_uuid;
>                 struct udev_device *device;
> 
>                 path = udev_list_entry_get_name(dev_list_entry);
> @@ -2388,16 +2387,12 @@ static struct udev_device *of_find_device_by_uuid(const char *uuid)
>                         continue;
>                 if (!strcmp(devtype, "disk")) {
>                         dev_uuid = udev_device_get_property_value(device, "ID_PART_TABLE_UUID");
> -                       if (dev_uuid && !strcasecmp(dev_uuid, uuid)) {
> -                               outpath = udev_device_get_devnode(device);
> +                       if (dev_uuid && !strcasecmp(dev_uuid, uuid))
>                                 return device;
> -                       }
>                 } else if (!strcmp(devtype, "partition")) {
>                         dev_uuid = udev_device_get_property_value(device, "ID_PART_ENTRY_UUID");
> -                       if (dev_uuid && !strcasecmp(dev_uuid, uuid)) {
> -                               outpath = udev_device_get_devnode(device);
> +                       if (dev_uuid && !strcasecmp(dev_uuid, uuid))
>                                 return device;
> -                       }
>                 }
> 
>         }
> 
> 
> -- 
> 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 |
> 
> 
> 

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

Re: [OSS-Tools] [PATCH v4 0/3] improve barebox-state support on EFI system

[Ahmad Fatoum]

On 11.05.22 10:21, Michael Olbrich wrote:
> Hi,
> 
> so the discussion on the barebox ML resulted in a different binding for
> this. Sascha has sent patches for that[1]. This is now mainline in Barebox.
> 
> So while this is 'v4' for this topic, all the patches except the last one
> are actually different, so please drop the old series that is still present
> in the next branch.
> 
> v2 had some Bugs that have been fixed in v3.
> Added some improvements as suggested by Ahmad in v4.
> 
> In the device-tree it now looks like this:

Thanks, applied to next branch with a small fixup as mentioned.

> 
> ----------------------------------------------------------------------
> / {
> [...]
>         state: state {
> [...]
>                 backend = <&barebox_state>;
> [...]
>         };
> 
>         disk {
>                 compatible = "barebox,storage-by-uuid";
>                 uuid = "deadbeaf";
> 
>                 partitions {
>                         compatible = "fixed-partitions";
>                         #address-cells = <2>;
>                         #size-cells = <2>;
> 
>                         barebox_state: state@300000 {
>                                 label = "barebox-state";
>                                 reg = <0x0 0x300000 0x0 0x100000>;
>                         };
>                 };
>         };
> };
> ----------------------------------------------------------------------
> 
> Regards,
> Michael
> 
> [1] https://lore.barebox.org/barebox/20220207094953.949868-1-s.hauer@pengutronix.de/T/#t
> 
> Michael Olbrich (3):
>   libdt: only requires a partname for mtd
>   libdt: add support for barebox,storage-by-uuid
>   state: automatically find state.dtb in the ESP
> 
>  src/barebox-state.c | 25 +++++++++++++
>  src/libdt.c         | 91 ++++++++++++++++++++++++++++++++++++++-------
>  2 files changed, 103 insertions(+), 13 deletions(-)
> 

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