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
