[OSS-Tools] [PATCH 0/8] state: allow lookup of barebox state partition by Type GUID

[OSS-Tools] [PATCH 0/8] state: allow lookup of barebox state partition by Type GUID

[Ahmad Fatoum]

This implements the binding extension introduced to barebox here:
https://lore.barebox.org/barebox/20230531145927.1399282-1-a.fatoum@pengutronix.de/T/#t

With this, barebox,state backend can optionally point at a device
instead of a partition. If this device is GPT-partitioned and has
a partition with a specific partition type GUID of

  4778ed65-bf42-45fa-9c5b-287a1dc4aab1

It will be taken.

This series also fixes an annoying issue of barebox-state triggering
udev on every access, because the root block device corresponding
to the device tree node was opened r/w.

barebox-state will now open the disk read-only if possible and if
a partition exists that fits the barebox state location, it will
be opened instead.

Ahmad Fatoum (8):
  state: backend: direct: open block device in read-only mode if
    possible
  libdt: factor out u64 sysattr parsing into helper
  libdt: drop broken if-branch
  libdt: factor out __of_cdev_find helper
  libdt: use block device partition instead of parent if found
  state: align with barebox use of of_cdev_find
  libdt: use of_find_device_by_uuid for partuuid lookup
  state: allow lookup of barebox state partition by Type GUID

 src/barebox-state/backend_bucket_direct.c |   5 +-
 src/barebox-state/backend_storage.c       |   2 +-
 src/barebox-state/state.c                 |  58 +++-
 src/barebox-state/state.h                 |   3 +-
 src/dt/common.h                           |   8 +
 src/dt/dt.h                               |   8 +
 src/libdt-utils.sym                       |   5 +
 src/libdt.c                               | 341 ++++++++++++++++++----
 src/linux/uuid.h                          |  24 ++
 src/state.h                               |   4 +
 10 files changed, 380 insertions(+), 78 deletions(-)
 create mode 100644 src/linux/uuid.h

-- 
2.39.2

[OSS-Tools] [PATCH 1/8] state: backend: direct: open block device in read-only mode if possible

[Ahmad Fatoum]

We unconditionally open the device backing a direct bucket in read-write
mode. The barebox_state utility already populates struct
state_backend_storage::readonly though, which we could consult at device
open time. Do so. This could possibly be done for MTD as well, but until
that's tested, for now, we do it only for the direct backend meant for
use with block devices.

Tested-by: Ulrich Ölmann <u.oelmann@pengutronix.de>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 src/barebox-state/backend_bucket_direct.c | 5 +++--
 src/barebox-state/backend_storage.c       | 2 +-
 src/barebox-state/state.c                 | 6 +++---
 src/barebox-state/state.h                 | 3 ++-
 4 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/src/barebox-state/backend_bucket_direct.c b/src/barebox-state/backend_bucket_direct.c
index 4522f0170f3d..48c1596c9add 100644
--- a/src/barebox-state/backend_bucket_direct.c
+++ b/src/barebox-state/backend_bucket_direct.c
@@ -164,12 +164,13 @@ static void state_backend_bucket_direct_free(struct
 
 int state_backend_bucket_direct_create(struct device_d *dev, const char *path,
 				       struct state_backend_storage_bucket **bucket,
-				       off_t offset, ssize_t max_size)
+				       off_t offset, ssize_t max_size,
+				       bool readonly)
 {
 	int fd;
 	struct state_backend_storage_bucket_direct *direct;
 
-	fd = open(path, O_RDWR);
+	fd = open(path, readonly ? O_RDONLY : O_RDWR);
 	if (fd < 0) {
 		dev_err(dev, "Failed to open file '%s', %d\n", path, -errno);
 		return -errno;
diff --git a/src/barebox-state/backend_storage.c b/src/barebox-state/backend_storage.c
index 458f2a91552b..4c5e1783c199 100644
--- a/src/barebox-state/backend_storage.c
+++ b/src/barebox-state/backend_storage.c
@@ -326,7 +326,7 @@ static int state_storage_file_buckets_init(struct state_backend_storage *storage
 		offset = storage->offset + n * stridesize;
 		ret = state_backend_bucket_direct_create(storage->dev, storage->path,
 							 &bucket, offset,
-							 stridesize);
+							 stridesize, storage->readonly);
 		if (ret) {
 			dev_warn(storage->dev, "Failed to create direct bucket at '%s' offset %lld\n",
 				 storage->path, (long long) offset);
diff --git a/src/barebox-state/state.c b/src/barebox-state/state.c
index 4779574d2ee9..3174c84b8ded 100644
--- a/src/barebox-state/state.c
+++ b/src/barebox-state/state.c
@@ -649,14 +649,14 @@ struct state *state_new_from_node(struct device_node *node, bool readonly)
 	if (ret)
 		goto out_release_state;
 
+	if (readonly)
+		state_backend_set_readonly(state);
+
 	ret = state_storage_init(state, state->backend_path, offset,
 				 size, stridesize, storage_type);
 	if (ret)
 		goto out_release_state;
 
-	if (readonly)
-		state_backend_set_readonly(state);
-
 	ret = state_from_node(state, node, 1);
 	if (ret) {
 		goto out_release_state;
diff --git a/src/barebox-state/state.h b/src/barebox-state/state.h
index 719f7e43c94c..4abfa84285c3 100644
--- a/src/barebox-state/state.h
+++ b/src/barebox-state/state.h
@@ -225,7 +225,8 @@ void state_backend_set_readonly(struct state *state);
 void state_storage_free(struct state_backend_storage *storage);
 int state_backend_bucket_direct_create(struct device_d *dev, const char *path,
 				       struct state_backend_storage_bucket **bucket,
-				       off_t offset, ssize_t max_size);
+				       off_t offset, ssize_t max_size,
+				       bool readonly);
 int state_storage_write(struct state_backend_storage *storage,
 			const void * buf, ssize_t len);
 int state_storage_read(struct state_backend_storage *storage,
-- 
2.39.2

[OSS-Tools] [PATCH 2/8] libdt: factor out u64 sysattr parsing into helper

[Ahmad Fatoum]

We will need to parse two more sysattrs in a follow-up patch, so factor
this out for reuse. While at it switch to 64-bit strtoull: This may
be more useful for future users and unlike atol doesn't invoke undefined
behavior when parsing fails.

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

diff --git a/src/libdt.c b/src/libdt.c
index 2c994c647ac9..580b0b0ba769 100644
--- a/src/libdt.c
+++ b/src/libdt.c
@@ -2179,6 +2179,33 @@ static struct udev_device *device_find_mtd_partition(struct udev_device *dev,
 	return NULL;
 }
 
+/*
+ * udev_device_parse_sysattr_u64 - parse sysattr value as unsigned 64-bit integer
+ * @dev:	the udev_device to extract the attribute from
+ * @attr:	name of the attribute to lookup
+ * @outvalue:	returns the value parsed out of the attribute
+ *
+ * returns 0 for success or negative error value on failure.
+ */
+static int udev_device_parse_sysattr_u64(struct udev_device *dev, const char *attr,
+		u64 *outvalue)
+{
+	char *endptr;
+	u64 value;
+	const char *str;
+
+	str = udev_device_get_sysattr_value(dev, attr);
+	if (!str)
+		return -EINVAL;
+
+	value = strtoull(str, &endptr, 0);
+	if (!*str || *endptr)
+		return -EINVAL;
+
+	*outvalue = value;
+	return 0;
+}
+
 /*
  * device_find_block_device - extract device path from udev block device
  *
@@ -2305,24 +2332,25 @@ static int udev_device_is_eeprom(struct udev_device *dev)
  * udev_parse_mtd - get information from a mtd udev_device
  * @dev:	the udev_device to extract information from
  * @devpath:	returns the devicepath under which the mtd device is accessible
- * @size:	returns the size of the mtd device
+ * @outsize:	returns the size of the mtd device
  *
  * returns 0 for success or negative error value on failure. *devpath
  * will be valid on success and must be freed after usage.
  */
-static int udev_parse_mtd(struct udev_device *dev, char **devpath, size_t *size)
+static int udev_parse_mtd(struct udev_device *dev, char **devpath, size_t *outsize)
 {
-	const char *sizestr;
 	const char *outpath;
+	u64 size;
+	int ret;
 
 	if (!udev_device_is_mtd(dev))
 		return -EINVAL;
 
-	sizestr = udev_device_get_sysattr_value(dev, "size");
-	if (!sizestr)
-		return -EINVAL;
+	ret = udev_device_parse_sysattr_u64(dev, "size", &size);
+	if (ret)
+		return ret;
 
-	*size = atol(sizestr);
+	*outsize = size;
 
 	outpath = udev_device_get_devnode(dev);
 	if (!outpath)
-- 
2.39.2

[OSS-Tools] [PATCH 3/8] libdt: drop broken if-branch

[Ahmad Fatoum]

device_find_block_device returns 0 on success, so the way the else if
clause is now, only if there is a block device, the code falls through.
If there is none, a 0 is returned, but devpath is not populated breaking
the contract of the function. Just drop the branch for now and add back
it later in a way that works.

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 580b0b0ba769..12d692d2b2cf 100644
--- a/src/libdt.c
+++ b/src/libdt.c
@@ -2519,13 +2519,10 @@ int of_get_devicepath(struct device_node *partition_node, char **devpath, off_t
 	 */
 	dev = of_find_device_by_node_path(partition_node->full_name);
 	if (dev) {
-		if (udev_device_is_eeprom(dev)) {
+		if (udev_device_is_eeprom(dev))
 			return udev_parse_eeprom(dev, devpath);
-		} else if (!udev_parse_mtd(dev, devpath, size)) {
+		if (!udev_parse_mtd(dev, devpath, size))
 			return 0;
-		} else if (device_find_block_device(dev, devpath)) {
-			return of_parse_partition(partition_node, offset, size);
-		}
 
 		/*
 		 * If we found a device but couldn't classify it above, we fall
-- 
2.39.2

[OSS-Tools] [PATCH 4/8] libdt: factor out __of_cdev_find helper

[Ahmad Fatoum]

of_get_devicepath is an exported symbol by libdt, so we shouldn't change
its signature or its semantics. Yet, we will want to export more
information out of the udev'ification code in future. <dt/dt.h> already
declares an opaque struct cdev, so let's add a __of_cdev_find helper
that can populate it with the same information that of_get_devicepath
would return. In later commits we will define helpers that return
and process a struct cdev placed in dynamic memory.

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

diff --git a/src/libdt.c b/src/libdt.c
index 12d692d2b2cf..440fcbd32fb4 100644
--- a/src/libdt.c
+++ b/src/libdt.c
@@ -32,6 +32,12 @@
 #include <libudev.h>
 #include <dt.h>
 
+struct cdev {
+	char *devpath;
+	off_t offset;
+	size_t size;
+};
+
 static int pr_level = 5;
 
 void pr_level_set(int level)
@@ -2482,33 +2488,14 @@ static struct udev_device *of_find_device_by_uuid(struct udev_device *parent,
 	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
- * @devpath:		Returns the devicepath under which the device is accessible
- * @offset:		Returns the offset in the device
- * @size:		Returns the size of the device
- *
- * This function takes a device_node which represents a partition.
- * For this partition the function returns the device path and the offset
- * and size in the device. For mtd devices the path will be /dev/mtdx, for
- * EEPROMs it will be /sys/.../eeprom and for block devices it will be /dev/...
- * For mtd devices the device path returned will be the partition itself.
- * Since EEPROMs do not have partitions under Linux @offset and @size will
- * describe the offset and size inside the full device. The same applies to
- * block devices.
- *
- * returns 0 for success or negative error value on failure.
- */
-int of_get_devicepath(struct device_node *partition_node, char **devpath, off_t *offset,
-		size_t *size)
+static int __of_cdev_find(struct device_node *partition_node, struct cdev *cdev)
 {
 	struct device_node *node;
 	struct udev_device *dev, *partdev, *mtd;
 	int ret;
 
-	*offset = 0;
-	*size = 0;
+	cdev->offset = 0;
+	cdev->size = 0;
 
 	/*
 	 * simplest case: This nodepath can directly be translated into
@@ -2520,8 +2507,8 @@ int of_get_devicepath(struct device_node *partition_node, char **devpath, off_t
 	dev = of_find_device_by_node_path(partition_node->full_name);
 	if (dev) {
 		if (udev_device_is_eeprom(dev))
-			return udev_parse_eeprom(dev, devpath);
-		if (!udev_parse_mtd(dev, devpath, size))
+			return udev_parse_eeprom(dev, &cdev->devpath);
+		if (!udev_parse_mtd(dev, &cdev->devpath, &cdev->size))
 			return 0;
 
 		/*
@@ -2551,7 +2538,7 @@ int of_get_devicepath(struct device_node *partition_node, char **devpath, off_t
 			while (*uuid)
 				*s++ = tolower(*uuid++);
 
-			*devpath = lc_uuid;
+			cdev->devpath = lc_uuid;
 
 			return 0;
 		}
@@ -2607,21 +2594,56 @@ int of_get_devicepath(struct device_node *partition_node, char **devpath, off_t
 			return -ENODEV;
 
 		/* ...find the desired information by mtd udev_device */
-		return udev_parse_mtd(partdev, devpath, size);
+		return udev_parse_mtd(partdev, &cdev->devpath, &cdev->size);
 	}
 
 	if (udev_device_is_eeprom(dev)) {
-		ret = udev_parse_eeprom(dev, devpath);
+		ret = udev_parse_eeprom(dev, &cdev->devpath);
 		if (ret)
 			return ret;
 
-		return of_parse_partition(partition_node, offset, size);
+		return of_parse_partition(partition_node, &cdev->offset, &cdev->size);
 	} else {
-		ret = device_find_block_device(dev, devpath);
+		ret = device_find_block_device(dev, &cdev->devpath);
 		if (ret)
 			return ret;
-		return of_parse_partition(partition_node, offset, size);
+		return of_parse_partition(partition_node, &cdev->offset, &cdev->size);
 	}
 
 	return -EINVAL;
 }
+
+/*
+ * of_get_devicepath - get information how to access device corresponding to a device_node
+ * @partition_node:	The device_node which shall be accessed
+ * @devpath:		Returns the devicepath under which the device is accessible
+ * @offset:		Returns the offset in the device
+ * @size:		Returns the size of the device
+ *
+ * This function takes a device_node which represents a partition.
+ * For this partition the function returns the device path and the offset
+ * and size in the device. For mtd devices the path will be /dev/mtdx, for
+ * EEPROMs it will be /sys/.../eeprom and for block devices it will be /dev/...
+ * For mtd devices the device path returned will be the partition itself.
+ * Since EEPROMs do not have partitions under Linux @offset and @size will
+ * describe the offset and size inside the full device. The same applies to
+ * block devices.
+ *
+ * returns 0 for success or negative error value on failure.
+ */
+int of_get_devicepath(struct device_node *partition_node, char **devpath, off_t *offset,
+		size_t *size)
+{
+	struct cdev cdev = {};
+	int ret;
+
+	ret = __of_cdev_find(partition_node, &cdev);
+	if (ret)
+		return ret;
+
+	*offset = cdev.offset;
+	*size = cdev.size;
+	*devpath = cdev.devpath;
+
+	return 0;
+}
-- 
2.39.2

[OSS-Tools] [PATCH 5/8] libdt: use block device partition instead of parent if found

[Ahmad Fatoum]

Linux doesn't parse MTD fixed-partitions described in the device tree
for block devices and EEPROMs. For block devices, The dt-utils work around
this by doing the lookup in userspace and then arrive at the parent block
device along with an offset that is passed along. We can do better
though: If there's a partition block device that contains the region
we are interested in, we should just be using that.

This avoids the issue of triggering a reread of the partition table
after writing barebox state because udev is notified that we had
been opening the whole block device writable.

Tested-by: Ulrich Ölmann <u.oelmann@pengutronix.de>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 src/libdt.c | 60 +++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 45 insertions(+), 15 deletions(-)

diff --git a/src/libdt.c b/src/libdt.c
index 440fcbd32fb4..e90ac5e26273 100644
--- a/src/libdt.c
+++ b/src/libdt.c
@@ -2212,23 +2212,29 @@ static int udev_device_parse_sysattr_u64(struct udev_device *dev, const char *at
 	return 0;
 }
 
+/* True if A completely contains B */
+static bool region_contains(loff_t starta, loff_t enda,
+			    loff_t startb, loff_t endb)
+{
+        return starta <= startb && enda >= endb;
+}
+
 /*
- * device_find_block_device - extract device path from udev block device
+ * cdev_from_block_device - Populate cdev from udev block device
  *
  * @dev:	the udev_device to extract information from
- * @devpath:	returns the devicepath under which the block device is accessible
+ * @cdev:	the cdev output parameter to populate
  *
  * returns 0 for success or negative error value on failure.
  */
-int device_find_block_device(struct udev_device *dev,
-		char **devpath)
+static int cdev_from_block_device(struct udev_device *dev,
+				  struct cdev *cdev)
 {
 
 	struct udev *udev;
 	struct udev_enumerate *enumerate;
 	struct udev_list_entry *devices, *dev_list_entry;
-	struct udev_device *part;
-	const char *outpath;
+	struct udev_device *part, *best_match = NULL;
 	int ret;
 
 	udev = udev_new();
@@ -2252,19 +2258,43 @@ int device_find_block_device(struct udev_device *dev,
 		if (!devtype)
 			continue;
 		if (!strcmp(devtype, "disk")) {
-			outpath = udev_device_get_devnode(part);
-			*devpath = strdup(outpath);
-			ret = 0;
-			goto out;
+			best_match = part;
+
+			/* Should we try to find  a matching partition first? */
+			if (!cdev->size)
+				break;
+		} else if (cdev->size && !strcmp(devtype, "partition")) {
+			u64 partstart, partsize;
+
+			ret = udev_device_parse_sysattr_u64(part, "start", &partstart);
+			if (ret)
+				continue;
+
+			ret = udev_device_parse_sysattr_u64(part, "size", &partsize);
+			if (ret)
+				continue;
+
+			/* start/size sys attributes are always in 512-byte units */
+			partstart *= 512;
+			partsize *= 512;
+
+			if (!region_contains(partstart, partstart + partsize,
+					     cdev->offset, cdev->offset + cdev->size))
+				continue;
+
+			best_match = part;
+			cdev->offset -= partstart;
+			break;
 		}
 	}
-	ret = -ENODEV;
 
-out:
+	if (best_match)
+		cdev->devpath = strdup(udev_device_get_devnode(best_match));
+
 	udev_enumerate_unref(enumerate);
 	udev_unref(udev);
 
-	return ret;
+	return best_match ? 0 : -ENODEV;
 }
 
 /*
@@ -2604,10 +2634,10 @@ static int __of_cdev_find(struct device_node *partition_node, struct cdev *cdev)
 
 		return of_parse_partition(partition_node, &cdev->offset, &cdev->size);
 	} else {
-		ret = device_find_block_device(dev, &cdev->devpath);
+		ret = of_parse_partition(partition_node, &cdev->offset, &cdev->size);
 		if (ret)
 			return ret;
-		return of_parse_partition(partition_node, &cdev->offset, &cdev->size);
+		return cdev_from_block_device(dev, cdev);
 	}
 
 	return -EINVAL;
-- 
2.39.2

[OSS-Tools] [PATCH 6/8] state: align with barebox use of of_cdev_find

[Ahmad Fatoum]

As part of barebox-state by GPT partition type GUID lookup support,
state code in barebox needs direct interaction with the cdev, so it can
enumerate the child partitions. We have recently added __of_cdev_find
with internal linkage, so lets export of_cdev_find with external linkage
for outside use. Unlike __of_cdev_find, the new external function will
return dynamically allocated memory to avoid consumer code having to
know the struct cdev layout.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 src/barebox-state/state.c | 26 +++++++++++++++++++-------
 src/dt/common.h           |  8 ++++++++
 src/dt/dt.h               |  3 +++
 src/libdt-utils.sym       |  2 ++
 src/libdt.c               | 37 +++++++++++++++++++++++++++++++++++++
 5 files changed, 69 insertions(+), 7 deletions(-)

diff --git a/src/barebox-state/state.c b/src/barebox-state/state.c
index 3174c84b8ded..29e04c3d7ea8 100644
--- a/src/barebox-state/state.c
+++ b/src/barebox-state/state.c
@@ -578,6 +578,20 @@ void state_release(struct state *state)
 	free(state);
 }
 
+#ifdef __BAREBOX__
+static char *cdev_to_devpath(struct cdev *cdev, off_t *offset, size_t *size)
+{
+	/*
+	 * We only accept partitions exactly mapping the barebox-state,
+	 * but dt-utils may need to set non-zero values here
+	 */
+	*offset = 0;
+	*size = 0;
+
+	return basprintf("/dev/%s", cdev->name);
+}
+#endif
+
 /*
  * state_new_from_node - create a new state instance from a device_node
  *
@@ -594,8 +608,9 @@ struct state *state_new_from_node(struct device_node *node, bool readonly)
 	const char *alias;
 	uint32_t stridesize;
 	struct device_node *partition_node;
-	off_t offset = 0;
-	size_t size = 0;
+	struct cdev *cdev;
+	off_t offset;
+	size_t size;
 
 	alias = of_alias_get(node);
 	if (!alias) {
@@ -614,11 +629,8 @@ struct state *state_new_from_node(struct device_node *node, bool readonly)
 		goto out_release_state;
 	}
 
-#ifdef __BAREBOX__
-	ret = of_find_path_by_node(partition_node, &state->backend_path, 0);
-#else
-	ret = of_get_devicepath(partition_node, &state->backend_path, &offset, &size);
-#endif
+	cdev = of_cdev_find(partition_node);
+	ret = PTR_ERR_OR_ZERO(cdev);
 	if (ret) {
 		if (ret != -EPROBE_DEFER)
 			dev_err(&state->dev, "state failed to parse path to backend: %s\n",
diff --git a/src/dt/common.h b/src/dt/common.h
index d16f1193fbe3..38dc61cd65fe 100644
--- a/src/dt/common.h
+++ b/src/dt/common.h
@@ -166,6 +166,14 @@ static inline void *ERR_CAST(const void *ptr)
 	return (void *) ptr;
 }
 
+static inline int PTR_ERR_OR_ZERO(const void *ptr)
+{
+	if (IS_ERR(ptr))
+		return PTR_ERR(ptr);
+	else
+		return 0;
+}
+
 static inline char *barebox_asprintf(const char *fmt, ...) __attribute__ ((format(__printf__, 1, 2)));
 static inline char *barebox_asprintf(const char *fmt, ...)
 {
diff --git a/src/dt/dt.h b/src/dt/dt.h
index d5e49eb32df9..f8bd3e56efed 100644
--- a/src/dt/dt.h
+++ b/src/dt/dt.h
@@ -231,6 +231,9 @@ void of_add_memory_bank(struct device_node *node, bool dump, int r,
 int of_get_devicepath(struct device_node *partition_node, char **devnode, off_t *offset,
 		size_t *size);
 
+struct cdev *of_cdev_find(struct device_node *partition_node);
+char *cdev_to_devpath(struct cdev *cdev, off_t *offset, size_t *size);
+
 #define for_each_node_by_name(dn, name) \
 	for (dn = of_find_node_by_name(NULL, name); dn; \
 	     dn = of_find_node_by_name(dn, name))
diff --git a/src/libdt-utils.sym b/src/libdt-utils.sym
index a749317fa024..f95c74305fb0 100644
--- a/src/libdt-utils.sym
+++ b/src/libdt-utils.sym
@@ -34,6 +34,8 @@ global:
 	of_get_child_by_name;
 	of_get_child_count;
 	of_get_devicepath;
+	of_cdev_find;
+	cdev_to_devpath;
 	of_get_next_available_child;
 	of_get_parent;
 	of_get_property;
diff --git a/src/libdt.c b/src/libdt.c
index e90ac5e26273..1cfca40f9f79 100644
--- a/src/libdt.c
+++ b/src/libdt.c
@@ -2677,3 +2677,40 @@ int of_get_devicepath(struct device_node *partition_node, char **devpath, off_t
 
 	return 0;
 }
+
+/*
+ * of_cdev_find - get information how to access device corresponding to a device_node
+ * @partition_node:	The device_node which shall be accessed
+ * @devpath:		Returns the devicepath under which the device is accessible
+ * @offset:		Returns the offset in the device
+ * @size:		Returns the size of the device
+ *
+ * This function takes a device_node which represents a partition.
+ * For this partition the function returns the device path and the offset
+ * and size in the device. For mtd devices the path will be /dev/mtdx, for
+ * EEPROMs it will be /sys/.../eeprom and for block devices it will be /dev/...
+ * For mtd devices the device path returned will be the partition itself.
+ * Since EEPROMs do not have partitions under Linux @offset and @size will
+ * describe the offset and size inside the full device. The same applies to
+ * block devices.
+ *
+ * returns 0 for success or negative error value on failure.
+ */
+struct cdev *of_cdev_find(struct device_node *partition_node)
+{
+	struct cdev cdev = {};
+	int ret;
+
+	ret = __of_cdev_find(partition_node, &cdev);
+	if (ret)
+		return ERR_PTR(ret);
+
+	return xmemdup(&cdev, sizeof(cdev));
+}
+
+char *cdev_to_devpath(struct cdev *cdev, off_t *offset, size_t *size)
+{
+	*offset = cdev->offset;
+	*size = cdev->size;
+	return cdev->devpath;
+}
-- 
2.39.2

[OSS-Tools] [PATCH 7/8] libdt: use of_find_device_by_uuid for partuuid lookup

[Ahmad Fatoum]

With the addition of of_find_device_by_uuid as part of the support for
barebox,storage-by-uuid, we don't need to rely on the device symlinks
created by 60-persistent-storage.rules and instead can just use libudev
directly, which we do elsewhere anyway. This has the added benefit
that we unify with the other code paths, where we determine the device
path through the struct udev_device.

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

diff --git a/src/libdt.c b/src/libdt.c
index 1cfca40f9f79..4c000919d305 100644
--- a/src/libdt.c
+++ b/src/libdt.c
@@ -2558,18 +2558,22 @@ static int __of_cdev_find(struct device_node *partition_node, struct cdev *cdev)
 		/* when partuuid is specified short-circuit the search for the cdev */
 		ret = of_property_read_string(partition_node, "partuuid", &uuid);
 		if (!ret) {
-			const char prefix[] = "/dev/disk/by-partuuid/";
-			size_t prefix_len = sizeof(prefix) - 1;
-			char *lc_uuid, *s;
+			u64 partsize;
+			int ret;
 
-			lc_uuid = xzalloc(prefix_len + strlen(uuid) + 1);
-			s = memcpy(lc_uuid, prefix, prefix_len) + prefix_len;
+			dev = of_find_device_by_uuid(NULL, uuid, false);
+			if (!dev) {
+				fprintf(stderr, "%s: cannot find device for uuid %s\n", __func__,
+					uuid);
+				return -ENODEV;
+			}
 
-			while (*uuid)
-				*s++ = tolower(*uuid++);
-
-			cdev->devpath = lc_uuid;
+			ret = udev_device_parse_sysattr_u64(dev, "size", &partsize);
+			if (ret)
+				return -EINVAL;
 
+			cdev->size = partsize * 512;
+			cdev->devpath = strdup(udev_device_get_devnode(dev));
 			return 0;
 		}
 	}
-- 
2.39.2

[OSS-Tools] [PATCH 8/8] state: allow lookup of barebox state partition by Type GUID

[Ahmad Fatoum]

The backend device tree property so far always pointed at a partition.
Let's allow pointing it at GPT storage devices directly and lookup
the correct barebox state partition by the well-known type GUID:

  4778ed65-bf42-45fa-9c5b-287a1dc4aab1

The implementation is not as neat as hoped for, because lifetime for
barebox cdev and libudev udev_device are quite different and libdt
doesn't yet get the latter right. Once we make sure not to leak
libudev objects and add cdev_unref stubs to barebox, this can be
further simplified by sticking the struct udev_device into struct
cdev and determining extra information on demand.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 src/barebox-state/state.c |  26 ++++++++++
 src/dt/dt.h               |   5 ++
 src/libdt-utils.sym       |   3 ++
 src/libdt.c               | 101 ++++++++++++++++++++++++++++++++++++--
 src/linux/uuid.h          |  24 +++++++++
 src/state.h               |   4 ++
 6 files changed, 160 insertions(+), 3 deletions(-)
 create mode 100644 src/linux/uuid.h

diff --git a/src/barebox-state/state.c b/src/barebox-state/state.c
index 29e04c3d7ea8..42ce88d3f161 100644
--- a/src/barebox-state/state.c
+++ b/src/barebox-state/state.c
@@ -22,6 +22,7 @@
 #include <crc.h>
 #include <linux/err.h>
 #include <linux/list.h>
+#include <linux/uuid.h>
 
 #include <linux/mtd/mtd-abi.h>
 #include <malloc.h>
@@ -592,6 +593,8 @@ static char *cdev_to_devpath(struct cdev *cdev, off_t *offset, size_t *size)
 }
 #endif
 
+static guid_t barebox_state_partition_guid = BAREBOX_STATE_PARTITION_GUID;
+
 /*
  * state_new_from_node - create a new state instance from a device_node
  *
@@ -638,6 +641,29 @@ struct state *state_new_from_node(struct device_node *node, bool readonly)
 		goto out_release_state;
 	}
 
+	/* Is the backend referencing an on-disk partitonable block device? */
+	if (cdev_is_block_disk(cdev)) {
+		struct cdev *partcdev = NULL;
+
+		if (cdev_is_gpt_partitioned(cdev))
+			partcdev = cdev_find_child_by_typeuuid(cdev, &barebox_state_partition_guid);
+
+		if (!partcdev) {
+			ret = -EINVAL;
+			goto out_release_state;
+		}
+
+		pr_debug("%s: backend GPT partition looked up via PartitionTypeGUID\n",
+			 node->full_name);
+
+		cdev = partcdev;
+	}
+
+	state->backend_path = cdev_to_devpath(cdev, &offset, &size);
+
+	pr_debug("%s: backend resolved to %s %lld %zu\n", node->full_name,
+		 state->backend_path, (long long)offset, size);
+
 	state->backend_reproducible_name = of_get_reproducible_name(partition_node);
 
 	ret = of_property_read_string(node, "backend-type", &backend_type);
diff --git a/src/dt/dt.h b/src/dt/dt.h
index f8bd3e56efed..a4213d49606a 100644
--- a/src/dt/dt.h
+++ b/src/dt/dt.h
@@ -5,6 +5,7 @@
 #include <dt/list.h>
 #include <dt/common.h>
 #include <asm/byteorder.h>
+#include <linux/uuid.h>
 
 /* Default string compare functions */
 #define of_compat_cmp(s1, s2, l)	strcasecmp((s1), (s2))
@@ -234,6 +235,10 @@ int of_get_devicepath(struct device_node *partition_node, char **devnode, off_t
 struct cdev *of_cdev_find(struct device_node *partition_node);
 char *cdev_to_devpath(struct cdev *cdev, off_t *offset, size_t *size);
 
+bool cdev_is_block_disk(struct cdev *cdev);
+bool cdev_is_gpt_partitioned(struct cdev *cdev);
+struct cdev *cdev_find_child_by_typeuuid(struct cdev *cdev, guid_t *typeuuid);
+
 #define for_each_node_by_name(dn, name) \
 	for (dn = of_find_node_by_name(NULL, name); dn; \
 	     dn = of_find_node_by_name(dn, name))
diff --git a/src/libdt-utils.sym b/src/libdt-utils.sym
index f95c74305fb0..63536224e518 100644
--- a/src/libdt-utils.sym
+++ b/src/libdt-utils.sym
@@ -36,6 +36,9 @@ global:
 	of_get_devicepath;
 	of_cdev_find;
 	cdev_to_devpath;
+	cdev_is_block_disk;
+	cdev_is_gpt_partitioned;
+	cdev_find_child_by_typeuuid;
 	of_get_next_available_child;
 	of_get_parent;
 	of_get_property;
diff --git a/src/libdt.c b/src/libdt.c
index 4c000919d305..af28de6f17c6 100644
--- a/src/libdt.c
+++ b/src/libdt.c
@@ -30,12 +30,15 @@
 #include <fcntl.h>
 #include <unistd.h>
 #include <libudev.h>
+#include <sys/sysmacros.h>
 #include <dt.h>
 
 struct cdev {
 	char *devpath;
 	off_t offset;
 	size_t size;
+	bool is_gpt_partitioned;
+	bool is_block_disk;
 };
 
 static int pr_level = 5;
@@ -2230,7 +2233,7 @@ static bool region_contains(loff_t starta, loff_t enda,
 static int cdev_from_block_device(struct udev_device *dev,
 				  struct cdev *cdev)
 {
-
+	const char *devtype;
 	struct udev *udev;
 	struct udev_enumerate *enumerate;
 	struct udev_list_entry *devices, *dev_list_entry;
@@ -2250,7 +2253,7 @@ static int cdev_from_block_device(struct udev_device *dev,
 	udev_enumerate_scan_devices(enumerate);
 	devices = udev_enumerate_get_list_entry(enumerate);
 	udev_list_entry_foreach(dev_list_entry, devices) {
-		const char *path, *devtype;
+		const char *path;
 		path = udev_list_entry_get_name(dev_list_entry);
 		part = udev_device_new_from_syspath(udev, path);
 		/* distinguish device (disk) from partitions */
@@ -2288,8 +2291,15 @@ static int cdev_from_block_device(struct udev_device *dev,
 		}
 	}
 
-	if (best_match)
+	if (best_match) {
 		cdev->devpath = strdup(udev_device_get_devnode(best_match));
+		if (!strcmp(devtype, "disk")) {
+			const char *part_type;
+
+			part_type = udev_device_get_property_value(best_match, "ID_PART_TABLE_TYPE");
+			cdev->is_gpt_partitioned = part_type && !strcmp(part_type, "gpt");
+		}
+	}
 
 	udev_enumerate_unref(enumerate);
 	udev_unref(udev);
@@ -2526,6 +2536,8 @@ static int __of_cdev_find(struct device_node *partition_node, struct cdev *cdev)
 
 	cdev->offset = 0;
 	cdev->size = 0;
+	cdev->is_gpt_partitioned = false;
+	cdev->is_block_disk = false;
 
 	/*
 	 * simplest case: This nodepath can directly be translated into
@@ -2541,6 +2553,11 @@ static int __of_cdev_find(struct device_node *partition_node, struct cdev *cdev)
 		if (!udev_parse_mtd(dev, &cdev->devpath, &cdev->size))
 			return 0;
 
+		if (!cdev_from_block_device(dev, cdev)) {
+			cdev->is_block_disk = true;
+			return 0;
+		}
+
 		/*
 		 * If we found a device but couldn't classify it above, we fall
 		 * through.
@@ -2718,3 +2735,81 @@ char *cdev_to_devpath(struct cdev *cdev, off_t *offset, size_t *size)
 	*size = cdev->size;
 	return cdev->devpath;
 }
+
+static struct udev_device *udev_from_devpath(struct udev *udev,
+					     const char *devpath)
+{
+	char syspath[64];
+	struct stat st;
+	const char *type;
+	int ret;
+
+	ret = stat(devpath, &st);
+	if (ret)
+		return NULL;
+
+	if (S_ISBLK(st.st_mode))
+		type = "block";
+	else if (S_ISCHR(st.st_mode))
+		type = "char";
+	else
+		return NULL;
+
+	if (major(st.st_rdev) == 0)
+		return NULL;
+
+	snprintf(syspath, sizeof(syspath), "/sys/dev/%s/%u:%u",
+		 type, major(st.st_rdev), minor(st.st_rdev));
+
+	return udev_device_new_from_syspath(udev, syspath);
+}
+
+struct cdev *cdev_find_child_by_typeuuid(struct cdev *cdev, guid_t *typeuuid)
+{
+	struct cdev *new = NULL;
+	struct udev_device *parent, *child;
+	struct udev *udev;
+	u64 size;
+	int ret;
+
+	udev = udev_new();
+	if (!udev)
+		return NULL;
+
+	parent = udev_from_devpath(udev, cdev->devpath);
+	if (!parent)
+		goto udev_unref;
+
+	child = of_find_device_by_uuid(parent, typeuuid->str, true);
+	if (!child)
+		goto udev_unref_parent;
+
+	ret = udev_device_parse_sysattr_u64(child, "size", &size);
+	if (ret)
+		goto udev_unref_child;
+
+	new = xzalloc(sizeof(*new));
+
+	new->offset = 0;
+	new->size = size * 512;
+	new->devpath = strdup(udev_device_get_devnode(child));
+
+udev_unref_child:
+	udev_device_unref(child);
+udev_unref_parent:
+	udev_device_unref(parent);
+udev_unref:
+	udev_unref(udev);
+
+	return new;
+}
+
+bool cdev_is_block_disk(struct cdev *cdev)
+{
+	return cdev->is_block_disk;
+}
+
+bool cdev_is_gpt_partitioned(struct cdev *cdev)
+{
+	return cdev->is_gpt_partitioned;
+}
diff --git a/src/linux/uuid.h b/src/linux/uuid.h
new file mode 100644
index 000000000000..1bc7cfc94040
--- /dev/null
+++ b/src/linux/uuid.h
@@ -0,0 +1,24 @@
+/*
+ * Copyright (C) 2023 Pengutronix, Ahmad Fatoum <a.fatoum@pengutronix.de>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+#ifndef _LINUX_UUID_H_
+#define _LINUX_UUID_H_
+
+#define   UUID_STRING_LEN         36
+
+/* We only do string comparisons anyway */
+typedef struct { const char str[UUID_STRING_LEN + 1]; } guid_t;
+
+#define GUID_INIT(str) { str }
+
+#endif
diff --git a/src/state.h b/src/state.h
index d98b781c2089..889df43b5780 100644
--- a/src/state.h
+++ b/src/state.h
@@ -2,6 +2,7 @@
 #define __STATE_H
 
 #include <of.h>
+#include <linux/uuid.h>
 
 struct state;
 
@@ -55,4 +56,7 @@ static inline int state_read_mac(struct state *state, const char *name, u8 *buf)
 
 #endif /* #if IS_ENABLED(CONFIG_STATE) / #else */
 
+#define BAREBOX_STATE_PARTITION_GUID \
+	GUID_INIT("4778ed65-bf42-45fa-9c5b-287a1dc4aab1")
+
 #endif /* __STATE_H */
-- 
2.39.2

Re: [OSS-Tools] [PATCH 2/8] libdt: factor out u64 sysattr parsing into helper

[Roland Hieber]

On Wed, May 31, 2023 at 05:22:47PM +0200, Ahmad Fatoum wrote:
> We will need to parse two more sysattrs in a follow-up patch, so factor
> this out for reuse. While at it switch to 64-bit strtoull: This may
> be more useful for future users and unlike atol doesn't invoke undefined
> behavior when parsing fails.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  src/libdt.c | 42 +++++++++++++++++++++++++++++++++++-------
>  1 file changed, 35 insertions(+), 7 deletions(-)
> 
> diff --git a/src/libdt.c b/src/libdt.c
> index 2c994c647ac9..580b0b0ba769 100644
> --- a/src/libdt.c
> +++ b/src/libdt.c
> @@ -2179,6 +2179,33 @@ static struct udev_device *device_find_mtd_partition(struct udev_device *dev,
>  	return NULL;
>  }
>  
> +/*

Use /** here so API doc generators could potentially  pick it up (even
if we don't build API docs right now, but all other functions are
documented the same way).

 - Roland

> + * udev_device_parse_sysattr_u64 - parse sysattr value as unsigned 64-bit integer
> + * @dev:	the udev_device to extract the attribute from
> + * @attr:	name of the attribute to lookup
> + * @outvalue:	returns the value parsed out of the attribute
> + *
> + * returns 0 for success or negative error value on failure.
> + */
> +static int udev_device_parse_sysattr_u64(struct udev_device *dev, const char *attr,
> +		u64 *outvalue)
> +{
> +	char *endptr;
> +	u64 value;
> +	const char *str;
> +
> +	str = udev_device_get_sysattr_value(dev, attr);
> +	if (!str)
> +		return -EINVAL;
> +
> +	value = strtoull(str, &endptr, 0);
> +	if (!*str || *endptr)
> +		return -EINVAL;
> +
> +	*outvalue = value;
> +	return 0;
> +}
> +
>  /*
>   * device_find_block_device - extract device path from udev block device
>   *
> @@ -2305,24 +2332,25 @@ static int udev_device_is_eeprom(struct udev_device *dev)
>   * udev_parse_mtd - get information from a mtd udev_device
>   * @dev:	the udev_device to extract information from
>   * @devpath:	returns the devicepath under which the mtd device is accessible
> - * @size:	returns the size of the mtd device
> + * @outsize:	returns the size of the mtd device
>   *
>   * returns 0 for success or negative error value on failure. *devpath
>   * will be valid on success and must be freed after usage.
>   */
> -static int udev_parse_mtd(struct udev_device *dev, char **devpath, size_t *size)
> +static int udev_parse_mtd(struct udev_device *dev, char **devpath, size_t *outsize)
>  {
> -	const char *sizestr;
>  	const char *outpath;
> +	u64 size;
> +	int ret;
>  
>  	if (!udev_device_is_mtd(dev))
>  		return -EINVAL;
>  
> -	sizestr = udev_device_get_sysattr_value(dev, "size");
> -	if (!sizestr)
> -		return -EINVAL;
> +	ret = udev_device_parse_sysattr_u64(dev, "size", &size);
> +	if (ret)
> +		return ret;
>  
> -	*size = atol(sizestr);
> +	*outsize = size;
>  
>  	outpath = udev_device_get_devnode(dev);
>  	if (!outpath)
> -- 
> 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 2/8] libdt: factor out u64 sysattr parsing into helper

[Roland Hieber]

On Fri, Jun 02, 2023 at 03:16:13PM +0200, Roland Hieber wrote:
> On Wed, May 31, 2023 at 05:22:47PM +0200, Ahmad Fatoum wrote:
> > We will need to parse two more sysattrs in a follow-up patch, so factor
> > this out for reuse. While at it switch to 64-bit strtoull: This may
> > be more useful for future users and unlike atol doesn't invoke undefined
> > behavior when parsing fails.
> > 
> > Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> > ---
> >  src/libdt.c | 42 +++++++++++++++++++++++++++++++++++-------
> >  1 file changed, 35 insertions(+), 7 deletions(-)
> > 
> > diff --git a/src/libdt.c b/src/libdt.c
> > index 2c994c647ac9..580b0b0ba769 100644
> > --- a/src/libdt.c
> > +++ b/src/libdt.c
> > @@ -2179,6 +2179,33 @@ static struct udev_device *device_find_mtd_partition(struct udev_device *dev,
> >  	return NULL;
> >  }
> >  
> > +/*
> 
> Use /** here so API doc generators could potentially  pick it up (even
> if we don't build API docs right now, but all other functions are
> documented the same way).

OK disregard that, not all functions have /** comments */ above them, so
this is a separate issue.

  - Roland

> > + * udev_device_parse_sysattr_u64 - parse sysattr value as unsigned 64-bit integer
> > + * @dev:	the udev_device to extract the attribute from
> > + * @attr:	name of the attribute to lookup
> > + * @outvalue:	returns the value parsed out of the attribute
> > + *
> > + * returns 0 for success or negative error value on failure.
> > + */
> > +static int udev_device_parse_sysattr_u64(struct udev_device *dev, const char *attr,
> > +		u64 *outvalue)
> > +{
> > +	char *endptr;
> > +	u64 value;
> > +	const char *str;
> > +
> > +	str = udev_device_get_sysattr_value(dev, attr);
> > +	if (!str)
> > +		return -EINVAL;
> > +
> > +	value = strtoull(str, &endptr, 0);
> > +	if (!*str || *endptr)
> > +		return -EINVAL;
> > +
> > +	*outvalue = value;
> > +	return 0;
> > +}
> > +
> >  /*
> >   * device_find_block_device - extract device path from udev block device
> >   *
> > @@ -2305,24 +2332,25 @@ static int udev_device_is_eeprom(struct udev_device *dev)
> >   * udev_parse_mtd - get information from a mtd udev_device
> >   * @dev:	the udev_device to extract information from
> >   * @devpath:	returns the devicepath under which the mtd device is accessible
> > - * @size:	returns the size of the mtd device
> > + * @outsize:	returns the size of the mtd device
> >   *
> >   * returns 0 for success or negative error value on failure. *devpath
> >   * will be valid on success and must be freed after usage.
> >   */
> > -static int udev_parse_mtd(struct udev_device *dev, char **devpath, size_t *size)
> > +static int udev_parse_mtd(struct udev_device *dev, char **devpath, size_t *outsize)
> >  {
> > -	const char *sizestr;
> >  	const char *outpath;
> > +	u64 size;
> > +	int ret;
> >  
> >  	if (!udev_device_is_mtd(dev))
> >  		return -EINVAL;
> >  
> > -	sizestr = udev_device_get_sysattr_value(dev, "size");
> > -	if (!sizestr)
> > -		return -EINVAL;
> > +	ret = udev_device_parse_sysattr_u64(dev, "size", &size);
> > +	if (ret)
> > +		return ret;
> >  
> > -	*size = atol(sizestr);
> > +	*outsize = size;
> >  
> >  	outpath = udev_device_get_devnode(dev);
> >  	if (!outpath)
> > -- 
> > 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 |
> 
> 

-- 
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 5/8] libdt: use block device partition instead of parent if found

[Roland Hieber]

On Wed, May 31, 2023 at 05:22:50PM +0200, Ahmad Fatoum wrote:
> Linux doesn't parse MTD fixed-partitions described in the device tree
> for block devices and EEPROMs. For block devices, The dt-utils work around
> this by doing the lookup in userspace and then arrive at the parent block
> device along with an offset that is passed along. We can do better
> though: If there's a partition block device that contains the region
> we are interested in, we should just be using that.
> 
> This avoids the issue of triggering a reread of the partition table
> after writing barebox state because udev is notified that we had
> been opening the whole block device writable.
> 
> Tested-by: Ulrich Ölmann <u.oelmann@pengutronix.de>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  src/libdt.c | 60 +++++++++++++++++++++++++++++++++++++++--------------
>  1 file changed, 45 insertions(+), 15 deletions(-)
> 
> diff --git a/src/libdt.c b/src/libdt.c
> index 440fcbd32fb4..e90ac5e26273 100644
> --- a/src/libdt.c
> +++ b/src/libdt.c
> @@ -2212,23 +2212,29 @@ static int udev_device_parse_sysattr_u64(struct udev_device *dev, const char *at
>  	return 0;
>  }
>  
> +/* True if A completely contains B */
> +static bool region_contains(loff_t starta, loff_t enda,
> +			    loff_t startb, loff_t endb)
> +{
> +        return starta <= startb && enda >= endb;
> +}
> +
>  /*
> - * device_find_block_device - extract device path from udev block device
> + * cdev_from_block_device - Populate cdev from udev block device
>   *
>   * @dev:	the udev_device to extract information from
> - * @devpath:	returns the devicepath under which the block device is accessible
> + * @cdev:	the cdev output parameter to populate
>   *
>   * returns 0 for success or negative error value on failure.
>   */
> -int device_find_block_device(struct udev_device *dev,
> -		char **devpath)
> +static int cdev_from_block_device(struct udev_device *dev,
> +				  struct cdev *cdev)
>  {
>  
>  	struct udev *udev;
>  	struct udev_enumerate *enumerate;
>  	struct udev_list_entry *devices, *dev_list_entry;
> -	struct udev_device *part;
> -	const char *outpath;
> +	struct udev_device *part, *best_match = NULL;
>  	int ret;
>  
>  	udev = udev_new();
> @@ -2252,19 +2258,43 @@ int device_find_block_device(struct udev_device *dev,
>  		if (!devtype)
>  			continue;
>  		if (!strcmp(devtype, "disk")) {
> -			outpath = udev_device_get_devnode(part);
> -			*devpath = strdup(outpath);
> -			ret = 0;
> -			goto out;
> +			best_match = part;
> +
> +			/* Should we try to find  a matching partition first? */

Nitpick: double space ;-)

 - Roland

> +			if (!cdev->size)
> +				break;
> +		} else if (cdev->size && !strcmp(devtype, "partition")) {
> +			u64 partstart, partsize;
> +
> +			ret = udev_device_parse_sysattr_u64(part, "start", &partstart);
> +			if (ret)
> +				continue;
> +
> +			ret = udev_device_parse_sysattr_u64(part, "size", &partsize);
> +			if (ret)
> +				continue;
> +
> +			/* start/size sys attributes are always in 512-byte units */
> +			partstart *= 512;
> +			partsize *= 512;
> +
> +			if (!region_contains(partstart, partstart + partsize,
> +					     cdev->offset, cdev->offset + cdev->size))
> +				continue;
> +
> +			best_match = part;
> +			cdev->offset -= partstart;
> +			break;
>  		}
>  	}
> -	ret = -ENODEV;
>  
> -out:
> +	if (best_match)
> +		cdev->devpath = strdup(udev_device_get_devnode(best_match));
> +
>  	udev_enumerate_unref(enumerate);
>  	udev_unref(udev);
>  
> -	return ret;
> +	return best_match ? 0 : -ENODEV;
>  }
>  
>  /*
> @@ -2604,10 +2634,10 @@ static int __of_cdev_find(struct device_node *partition_node, struct cdev *cdev)
>  
>  		return of_parse_partition(partition_node, &cdev->offset, &cdev->size);
>  	} else {
> -		ret = device_find_block_device(dev, &cdev->devpath);
> +		ret = of_parse_partition(partition_node, &cdev->offset, &cdev->size);
>  		if (ret)
>  			return ret;
> -		return of_parse_partition(partition_node, &cdev->offset, &cdev->size);
> +		return cdev_from_block_device(dev, cdev);
>  	}
>  
>  	return -EINVAL;
> -- 
> 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 3/8] libdt: drop broken if-branch

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 679 bytes --]

On Wed, May 31, 2023 at 05:22:48PM +0200, Ahmad Fatoum wrote:
> device_find_block_device returns 0 on success, so the way the else if
> clause is now, only if there is a block device, the code falls through.
> If there is none, a 0 is returned, but devpath is not populated breaking
> the contract of the function. Just drop the branch for now and add back
> it later in a way that works.

s/add back it/add it back/

Otherwise:

Reviewed-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Thanks
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]