diff mbox series

[resent,RFC,08/22] dm: blk: add UCLASS_PARTITION

Message ID 20211004034430.41355-9-takahiro.akashi@linaro.org
State New
Headers show
Series efi_loader: more tightly integrate UEFI disks to device model | expand

Commit Message

AKASHI Takahiro Oct. 4, 2021, 3:44 a.m. UTC
UCLASS_PARTITION device will be created as a child node of
UCLASS_BLK device.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>

---
 drivers/block/blk-uclass.c | 111 +++++++++++++++++++++++++++++++++++++
 include/blk.h              |   9 +++
 include/dm/uclass-id.h     |   1 +
 3 files changed, 121 insertions(+)

-- 
2.33.0

Comments

Ilias Apalodimas Oct. 4, 2021, 6:40 p.m. UTC | #1
Hi Akashi-san,

>  


[...]

> +int blk_create_partitions(struct udevice *parent)

> +{

> +	int part, count;

> +	struct blk_desc *desc = dev_get_uclass_plat(parent);

> +	struct disk_partition info;

> +	struct disk_part *part_data;

> +	char devname[32];

> +	struct udevice *dev;

> +	int ret;

> +

> +	if (!CONFIG_IS_ENABLED(PARTITIONS) ||

> +	    !CONFIG_IS_ENABLED(HAVE_BLOCK_DEVICE))

> +		return 0;


Would it make more sense to return an error here?

> +

> +	/* Add devices for each partition */

> +	for (count = 0, part = 1; part <= MAX_SEARCH_PARTITIONS; part++) {

> +		if (part_get_info(desc, part, &info))

> +			continue;

> +		snprintf(devname, sizeof(devname), "%s:%d", parent->name,

> +			 part);

> +

> +		ret = device_bind_driver(parent, "blk_partition",

> +					 strdup(devname), &dev);

> +		if (ret)

> +			return ret;

> +

> +		part_data = dev_get_uclass_plat(dev);

> +		part_data->partnum = part;

> +		part_data->gpt_part_info = info;

> +		count++;

> +

> +		device_probe(dev);


Probe can fail. 

> +	}

> +	debug("%s: %d partitions found in %s\n", __func__, count, parent->name);

> +

> +	return 0;

> +}

> +

>  static int blk_post_probe(struct udevice *dev)

>  {

>  	if (IS_ENABLED(CONFIG_PARTITIONS) &&

> @@ -713,3 +752,75 @@ UCLASS_DRIVER(blk) = {

>  	.post_probe	= blk_post_probe,

>  	.per_device_plat_auto	= sizeof(struct blk_desc),

>  };

[...]

Regards
/Ilias
AKASHI Takahiro Oct. 5, 2021, 1:30 a.m. UTC | #2
On Mon, Oct 04, 2021 at 09:40:21PM +0300, Ilias Apalodimas wrote:
> Hi Akashi-san,

> 

> >  

> 

> [...]

> 

> > +int blk_create_partitions(struct udevice *parent)

> > +{

> > +	int part, count;

> > +	struct blk_desc *desc = dev_get_uclass_plat(parent);

> > +	struct disk_partition info;

> > +	struct disk_part *part_data;

> > +	char devname[32];

> > +	struct udevice *dev;

> > +	int ret;

> > +

> > +	if (!CONFIG_IS_ENABLED(PARTITIONS) ||

> > +	    !CONFIG_IS_ENABLED(HAVE_BLOCK_DEVICE))

> > +		return 0;

> 

> Would it make more sense to return an error here?


!CONFIG_IS_ENABLED(PARTITIONS) means that a user doesn't want to
use partitions on their system. So simply returning 0 would be fine, I think.
This is kinda equivalence at the caller site:

	if (CONFIG_IS_ENABLED(PARTITIONS) &&
	    CONFIG_IS_ENABLED(HAVE_BLOCK_DEVICE))
                ret = blk_create_partitions(dev);
        else
                debug("We don't care about partitions.");

> > +

> > +	/* Add devices for each partition */

> > +	for (count = 0, part = 1; part <= MAX_SEARCH_PARTITIONS; part++) {

> > +		if (part_get_info(desc, part, &info))

> > +			continue;

> > +		snprintf(devname, sizeof(devname), "%s:%d", parent->name,

> > +			 part);

> > +

> > +		ret = device_bind_driver(parent, "blk_partition",

> > +					 strdup(devname), &dev);

> > +		if (ret)

> > +			return ret;

> > +

> > +		part_data = dev_get_uclass_plat(dev);

> > +		part_data->partnum = part;

> > +		part_data->gpt_part_info = info;

> > +		count++;

> > +

> > +		device_probe(dev);

> 

> Probe can fail. 


Theoretically, yes. But as a matter of fact, device_probe() does almost nothing
for UCLASS_PARTITION devices under the proposed implementation here and
I don't expect it will ever fail.
Please note that, as I commented in blk_part_post_probe(), we may want to
call blk_create_partitions() in the future so that we will support
"nested" partitioning in a partition :)

-Takahiro Akashi


> > +	}

> > +	debug("%s: %d partitions found in %s\n", __func__, count, parent->name);

> > +

> > +	return 0;

> > +}

> > +

> >  static int blk_post_probe(struct udevice *dev)

> >  {

> >  	if (IS_ENABLED(CONFIG_PARTITIONS) &&

> > @@ -713,3 +752,75 @@ UCLASS_DRIVER(blk) = {

> >  	.post_probe	= blk_post_probe,

> >  	.per_device_plat_auto	= sizeof(struct blk_desc),

> >  };

> [...]

> 

> Regards

> /Ilias
diff mbox series

Patch

diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c
index 83682dcc181a..dd7f3c0fe31e 100644
--- a/drivers/block/blk-uclass.c
+++ b/drivers/block/blk-uclass.c
@@ -12,6 +12,7 @@ 
 #include <log.h>
 #include <malloc.h>
 #include <part.h>
+#include <string.h>
 #include <dm/device-internal.h>
 #include <dm/lists.h>
 #include <dm/uclass-internal.h>
@@ -695,6 +696,44 @@  int blk_unbind_all(int if_type)
 	return 0;
 }
 
+int blk_create_partitions(struct udevice *parent)
+{
+	int part, count;
+	struct blk_desc *desc = dev_get_uclass_plat(parent);
+	struct disk_partition info;
+	struct disk_part *part_data;
+	char devname[32];
+	struct udevice *dev;
+	int ret;
+
+	if (!CONFIG_IS_ENABLED(PARTITIONS) ||
+	    !CONFIG_IS_ENABLED(HAVE_BLOCK_DEVICE))
+		return 0;
+
+	/* Add devices for each partition */
+	for (count = 0, part = 1; part <= MAX_SEARCH_PARTITIONS; part++) {
+		if (part_get_info(desc, part, &info))
+			continue;
+		snprintf(devname, sizeof(devname), "%s:%d", parent->name,
+			 part);
+
+		ret = device_bind_driver(parent, "blk_partition",
+					 strdup(devname), &dev);
+		if (ret)
+			return ret;
+
+		part_data = dev_get_uclass_plat(dev);
+		part_data->partnum = part;
+		part_data->gpt_part_info = info;
+		count++;
+
+		device_probe(dev);
+	}
+	debug("%s: %d partitions found in %s\n", __func__, count, parent->name);
+
+	return 0;
+}
+
 static int blk_post_probe(struct udevice *dev)
 {
 	if (IS_ENABLED(CONFIG_PARTITIONS) &&
@@ -713,3 +752,75 @@  UCLASS_DRIVER(blk) = {
 	.post_probe	= blk_post_probe,
 	.per_device_plat_auto	= sizeof(struct blk_desc),
 };
+
+static ulong blk_part_read(struct udevice *dev, lbaint_t start,
+			   lbaint_t blkcnt, void *buffer)
+{
+	struct udevice *parent;
+	struct disk_part *part;
+	const struct blk_ops *ops;
+
+	parent = dev_get_parent(dev);
+	ops = blk_get_ops(parent);
+	if (!ops->read)
+		return -ENOSYS;
+
+	part = dev_get_uclass_plat(dev);
+	start += part->gpt_part_info.start;
+
+	return ops->read(parent, start, blkcnt, buffer);
+}
+
+static ulong blk_part_write(struct udevice *dev, lbaint_t start,
+			    lbaint_t blkcnt, const void *buffer)
+{
+	struct udevice *parent;
+	struct disk_part *part;
+	const struct blk_ops *ops;
+
+	parent = dev_get_parent(dev);
+	ops = blk_get_ops(parent);
+	if (!ops->write)
+		return -ENOSYS;
+
+	part = dev_get_uclass_plat(dev);
+	start += part->gpt_part_info.start;
+
+	return ops->write(parent, start, blkcnt, buffer);
+}
+
+static ulong blk_part_erase(struct udevice *dev, lbaint_t start,
+			    lbaint_t blkcnt)
+{
+	struct udevice *parent;
+	struct disk_part *part;
+	const struct blk_ops *ops;
+
+	parent = dev_get_parent(dev);
+	ops = blk_get_ops(parent);
+	if (!ops->erase)
+		return -ENOSYS;
+
+	part = dev_get_uclass_plat(dev);
+	start += part->gpt_part_info.start;
+
+	return ops->erase(parent, start, blkcnt);
+}
+
+static const struct blk_ops blk_part_ops = {
+	.read	= blk_part_read,
+	.write	= blk_part_write,
+	.erase	= blk_part_erase,
+};
+
+U_BOOT_DRIVER(blk_partition) = {
+	.name		= "blk_partition",
+	.id		= UCLASS_PARTITION,
+	.ops		= &blk_part_ops,
+};
+
+UCLASS_DRIVER(partition) = {
+	.id		= UCLASS_PARTITION,
+	.per_device_plat_auto	= sizeof(struct disk_part),
+	.name		= "partition",
+};
diff --git a/include/blk.h b/include/blk.h
index 19bab081c2cd..3d883eb1db64 100644
--- a/include/blk.h
+++ b/include/blk.h
@@ -366,6 +366,15 @@  int blk_create_devicef(struct udevice *parent, const char *drv_name,
 		       const char *name, int if_type, int devnum, int blksz,
 		       lbaint_t lba, struct udevice **devp);
 
+/**
+ * blk_create_partitions - Create block devices for disk partitions
+ *
+ * Create UCLASS_PARTITION udevices for each of disk partitions in @parent
+ *
+ * @parent:	Whole disk device
+ */
+int blk_create_partitions(struct udevice *parent);
+
 /**
  * blk_unbind_all() - Unbind all device of the given interface type
  *
diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
index e7edd409f307..30892d01ce13 100644
--- a/include/dm/uclass-id.h
+++ b/include/dm/uclass-id.h
@@ -80,6 +80,7 @@  enum uclass_id {
 	UCLASS_P2SB,		/* (x86) Primary-to-Sideband Bus */
 	UCLASS_PANEL,		/* Display panel, such as an LCD */
 	UCLASS_PANEL_BACKLIGHT,	/* Backlight controller for panel */
+	UCLASS_PARTITION,	/* Logical disk partition device */
 	UCLASS_PCH,		/* x86 platform controller hub */
 	UCLASS_PCI,		/* PCI bus */
 	UCLASS_PCI_EP,		/* PCI endpoint device */