diff mbox series

[v4,2/4] block: partitions: populate fwnode

Message ID 6acc459a392d562abc58f7e55c6f04dba8073257.1719520771.git.daniel@makrotopia.org (mailing list archive)
State New, archived
Headers show
Series block: preparations for NVMEM provider | expand

Commit Message

Daniel Golle June 27, 2024, 8:50 p.m. UTC
Let block partitions to be represented by a firmware node and hence
allow them to being referenced e.g. for use with blk-nvmem.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
 block/partitions/core.c | 70 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 70 insertions(+)

Comments

Christoph Hellwig June 28, 2024, 4:44 a.m. UTC | #1
On Thu, Jun 27, 2024 at 09:50:39PM +0100, Daniel Golle wrote:
> +		/*
> +		 * In case 'uuid' is defined in the partitions firmware node require
> +		 * partition meta info being present and the specified uuid to match.
> +		 */

Overly long lines, which is really annyoing for block comments.

> +		got_uuid = !fwnode_property_read_string(fw_part, "uuid", &uuid);
> +		if (got_uuid && (!bdev->bd_meta_info ||
> +				 !part_meta_match(uuid, bdev->bd_meta_info->uuid,
> +						  PARTITION_META_INFO_UUIDLTH)))

Can we please not use the crazy part_meta stuff for anything new?
We should never have merge it, and right now it is at least isolated
to the boot time root dev_t partsing, and I'd really prefer to keep it
in that corner.
Daniel Golle June 28, 2024, 12:16 p.m. UTC | #2
Hi Christoph,

thank you for reviewing.

On Thu, Jun 27, 2024 at 09:44:28PM -0700, Christoph Hellwig wrote:
> On Thu, Jun 27, 2024 at 09:50:39PM +0100, Daniel Golle wrote:
> > +		/*
> > +		 * In case 'uuid' is defined in the partitions firmware node require
> > +		 * partition meta info being present and the specified uuid to match.
> > +		 */
> 
> Overly long lines, which is really annyoing for block comments.

Should I use 80 chars as limit everywhere?

> 
> > +		got_uuid = !fwnode_property_read_string(fw_part, "uuid", &uuid);
> > +		if (got_uuid && (!bdev->bd_meta_info ||
> > +				 !part_meta_match(uuid, bdev->bd_meta_info->uuid,
> > +						  PARTITION_META_INFO_UUIDLTH)))
> 
> Can we please not use the crazy part_meta stuff for anything new?
> We should never have merge it, and right now it is at least isolated
> to the boot time root dev_t partsing, and I'd really prefer to keep it
> in that corner.
> 

At least up to my understanding there isn't any other to know a
partitions UUID or volume name.

If there is another way to access this information I'd happily make
use of it, but I couldn't find any.
Christoph Hellwig June 28, 2024, 12:41 p.m. UTC | #3
On Fri, Jun 28, 2024 at 01:16:13PM +0100, Daniel Golle wrote:
> > Overly long lines, which is really annyoing for block comments.
> 
> Should I use 80 chars as limit everywhere?

In my opinion that makes things easier.  The coding style allows to
exceed it for individual lines where it improves readability, which
is a bit of an odd case.

> > Can we please not use the crazy part_meta stuff for anything new?
> > We should never have merge it, and right now it is at least isolated
> > to the boot time root dev_t partsing, and I'd really prefer to keep it
> > in that corner.
> > 
> 
> At least up to my understanding there isn't any other to know a
> partitions UUID or volume name.
> 
> If there is another way to access this information I'd happily make
> use of it, but I couldn't find any.

That is true, but except for the early dev_t parsing we actually never
use these partition uuids in the kernel at all.  Also in all the
normal file system references either use the file system uuid,
or the device provided one for devices that support them.  Most of
that is handled entirely in userspace, though - the kernel largely
operates just on the dev_t.
diff mbox series

Patch

diff --git a/block/partitions/core.c b/block/partitions/core.c
index ab76e64f0f6c..edf582de741f 100644
--- a/block/partitions/core.c
+++ b/block/partitions/core.c
@@ -10,6 +10,8 @@ 
 #include <linux/ctype.h>
 #include <linux/vmalloc.h>
 #include <linux/raid/detect.h>
+#include <linux/property.h>
+
 #include "check.h"
 
 static int (*const check_part[])(struct parsed_partitions *) = {
@@ -281,6 +283,72 @@  static ssize_t whole_disk_show(struct device *dev,
 }
 static const DEVICE_ATTR(whole_disk, 0444, whole_disk_show, NULL);
 
+static bool part_meta_match(const char *attr, const char *member, size_t length)
+{
+	/* check if length of attr exceeds specified maximum length */
+	if (strnlen(attr, length) == length)
+		return false;
+
+	/* return true if strings match */
+	return !strncmp(attr, member, length);
+}
+
+static struct fwnode_handle *find_partition_fwnode(struct block_device *bdev)
+{
+	struct fwnode_handle *fw_parts, *fw_part;
+	struct device *ddev = disk_to_dev(bdev->bd_disk);
+	const char *partname, *uuid;
+	u32 partno;
+	bool got_uuid, got_partname, got_partno;
+
+	fw_parts = device_get_named_child_node(ddev, "partitions");
+	if (!fw_parts)
+		return NULL;
+
+	fwnode_for_each_child_node(fw_parts, fw_part) {
+		got_uuid = false;
+		got_partname = false;
+		got_partno = false;
+		/*
+		 * In case 'uuid' is defined in the partitions firmware node require
+		 * partition meta info being present and the specified uuid to match.
+		 */
+		got_uuid = !fwnode_property_read_string(fw_part, "uuid", &uuid);
+		if (got_uuid && (!bdev->bd_meta_info ||
+				 !part_meta_match(uuid, bdev->bd_meta_info->uuid,
+						  PARTITION_META_INFO_UUIDLTH)))
+			continue;
+
+		/*
+		 * In case 'partname' is defined in the partitions firmware node require
+		 * partition meta info being present and the specified volname to match.
+		 */
+		got_partname = !fwnode_property_read_string(fw_part, "partname",
+							    &partname);
+		if (got_partname && (!bdev->bd_meta_info ||
+				     !part_meta_match(partname,
+						      bdev->bd_meta_info->volname,
+						      PARTITION_META_INFO_VOLNAMELTH)))
+			continue;
+
+		/*
+		 * In case 'partno' is defined in the partitions firmware node the
+		 * specified partno needs to match.
+		 */
+		got_partno = !fwnode_property_read_u32(fw_part, "partno", &partno);
+		if (got_partno && bdev_partno(bdev) != partno)
+			continue;
+
+		/* Skip if no matching criteria is present in firmware node */
+		if (!got_uuid && !got_partname && !got_partno)
+			continue;
+
+		return fw_part;
+	}
+
+	return NULL;
+}
+
 /*
  * Must be called either with open_mutex held, before a disk can be opened or
  * after all disk users are gone.
@@ -355,6 +423,8 @@  static struct block_device *add_partition(struct gendisk *disk, int partno,
 			goto out_put;
 	}
 
+	device_set_node(pdev, find_partition_fwnode(bdev));
+
 	/* delay uevent until 'holders' subdir is created */
 	dev_set_uevent_suppress(pdev, 1);
 	err = device_add(pdev);