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