[U-Boot] [RFC 1/3] dm: blk: add UCLASS_PARTITION

[AKASHI Takahiro]

On Tue, Jan 29, 2019 at 11:20:01PM +0100, Heinrich Schuchardt wrote:
> On 1/29/19 3:59 AM, AKASHI Takahiro wrote:
> > UCLASS_PARTITION device will be created as a child node of
> > UCLASS_BLK device.
> > 
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org>
> > ---
> >  drivers/block/blk-uclass.c | 52 ++++++++++++++++++++++++++++++++++++++
> >  include/dm/uclass-id.h     |  1 +
> >  2 files changed, 53 insertions(+)
> > 
> > diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c
> > index baaf431e5e0c..d4ca30f23fc1 100644
> > --- a/drivers/block/blk-uclass.c
> > +++ b/drivers/block/blk-uclass.c
> > @@ -10,6 +10,8 @@
> >  #include <dm/device-internal.h>
> >  #include <dm/lists.h>
> >  #include <dm/uclass-internal.h>
> > +#include <part.h>
> > +#include <string.h>
> >  
> >  static const char *if_typename_str[IF_TYPE_COUNT] = {
> >  	[IF_TYPE_IDE]		= "ide",
> > @@ -654,3 +656,53 @@ UCLASS_DRIVER(blk) = {
> >  	.post_probe	= blk_post_probe,
> >  	.per_device_platdata_auto_alloc_size = sizeof(struct blk_desc),
> >  };
> > +
> > +U_BOOT_DRIVER(blk_partition) = {
> > +	.name		= "blk_partition",
> > +	.id		= UCLASS_PARTITION,
> > +	.platdata_auto_alloc_size = sizeof(struct disk_part),
> > +};
> > +
> > +UCLASS_DRIVER(partition) = {
> > +	.id		= UCLASS_PARTITION,
> > +	.name		= "partition",
> > +};
> > +
> > +#if defined(CONFIG_PARTITIONS) && defined(CONFIG_HAVE_BLOCK_DEVICE)
> > +int blk_create_partitions(struct udevice *parent)
> > +{
> > +	int part;
> > +	struct blk_desc *desc = dev_get_uclass_platdata(parent);
> > +	disk_partition_t info;
> > +	struct disk_part *part_data;
> > +	char devname[32];
> > +	struct udevice *dev;
> > +	int disks = 0, ret;
> > +
> > +	/* Add devices for each partition */
> > +	for (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)
> 
> This looks like a memory leak for the output of strdup().

Yes, I'm aware of that, but please note that this is a prototype
and so I haven't paid much attention to failure cases (error recovery).
First of all, even in the current implementation, we don't support
*unplugging* (or unbind in EFI jargon?) devices.
It's a more fundamental issue.

> > +			return ret;
> 
> Why would we leave here if one partition fails?
> Does this imply that all further partitions will fail?
> Should we use continue here?

Ditto. Please be patient for the time being :)

-Takahiro Akashi

> Best regards
> 
> Heinrich
> 
> > +
> > +		part_data = dev_get_uclass_platdata(dev);
> > +		part_data->partnum = part;
> > +		part_data->gpt_part_info = info;
> > +
> > +		disks++;
> > +	}
> > +
> > +	return disks;
> > +}
> > +#else
> > +int blk_create_partitions(struct udevice *dev)
> > +{
> > +	return 0;
> > +}
> > +#endif
> > diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
> > index f3bafb3c6353..e02b5f8fda42 100644
> > --- a/include/dm/uclass-id.h
> > +++ b/include/dm/uclass-id.h
> > @@ -65,6 +65,7 @@ enum uclass_id {
> >  	UCLASS_NVME,		/* NVM Express device */
> >  	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_GENERIC,	/* Generic PCI bus device */
> > 
>