[PATCH v12 3/3] mtd: Add driver for concatenating devices

[Mahapatra, Amit Kumar]

[AMD Official Use Only - AMD Internal Distribution Only]

Hello Miquel,

> -----Original Message-----
> From: Miquel Raynal <miquel.raynal at bootlin.com>
> Sent: Tuesday, March 18, 2025 9:23 PM
> To: Mahapatra, Amit Kumar <amit.kumar-mahapatra at amd.com>
> Cc: richard at nod.at; vigneshr at ti.com; robh at kernel.org; krzk+dt at kernel.org;
> conor+dt at kernel.org; linux-mtd at lists.infradead.org; devicetree at vger.kernel.org;
> linux-kernel at vger.kernel.org; git (AMD-Xilinx) <git at amd.com>;
> amitrkcian2002 at gmail.com; Bernhard Frauendienst <kernel at nospam.obeliks.de>
> Subject: Re: [PATCH v12 3/3] mtd: Add driver for concatenating devices
>
> On 05/02/2025 at 19:07:30 +0530, Amit Kumar Mahapatra <amit.kumar-
> mahapatra at amd.com> wrote:
>
>
...

> > +static int __init mtd_virt_concat_create_join(void) {
> > +   struct mtd_virt_concat_node *item;
> > +   struct mtd_concat *concat;
> > +   struct mtd_info *mtd;
> > +   ssize_t name_sz;
> > +   char *name;
> > +   int ret;
> > +
> > +   list_for_each_entry(item, &concat_node_list, head) {
> > +           concat = item->concat;
> > +           mtd = &concat->mtd;
> > +           /* Create the virtual device */
> > +           name_sz = snprintf(NULL, 0, "%s-%s%s-concat",
> > +                              concat->subdev[0]->name,
> > +                              concat->subdev[1]->name,
> > +                              concat->num_subdev > MIN_DEV_PER_CONCAT
> ?
> > +                              "-+" : "");
> > +           name = kmalloc(name_sz + 1, GFP_KERNEL);
> > +           if (!name) {
> > +                   mtd_virt_concat_put_mtd_devices(concat);
> > +                   return -ENOMEM;
> > +           }
> > +
> > +           sprintf(name, "%s-%s%s-concat",
> > +                   concat->subdev[0]->name,
> > +                   concat->subdev[1]->name,
> > +                   concat->num_subdev > MIN_DEV_PER_CONCAT ?
> > +                   "-+" : "");
> > +
> > +           mtd = mtd_concat_create(concat->subdev, concat->num_subdev,
> name);
> > +           if (!mtd) {
> > +                   kfree(name);
> > +                   return -ENXIO;
> > +           }
> > +
> > +           /* Arbitrary set the first device as parent */
>
> Here we may face runtime PM issues. At some point the device model expects a
> single parent per struct device, but here we have two. I do not have any hints at the
> moment on how we could solve that.
>
> > +           mtd->dev.parent = concat->subdev[0]->dev.parent;
> > +           mtd->dev = concat->subdev[0]->dev;
> > +
> > +           /* Register the platform device */
> > +           ret = mtd_device_register(mtd, NULL, 0);
> > +           if (ret)
> > +                   goto destroy_concat;
> > +   }
> > +
> > +   return 0;
> > +
> > +destroy_concat:
> > +   mtd_concat_destroy(mtd);
> > +
> > +   return ret;
> > +}
> > +
> > +late_initcall(mtd_virt_concat_create_join);
>
> The current implementation does not support probe deferrals, I believe it should be
> handled.

I see that the parse_mtd_partitions() API can return -EPROBE_DEFER during
MTD device registration, but this behavior is specific to the
parse_qcomsmem_part parser. None of the other parsers appear to support
probe deferral. As discussed in RFC [1], the virtual concat feature is
purely a fixed-partition capability, and based on my understanding, the
fixed-partition parser does not support probe deferral.
Please let me know if you can think of any other probe deferral scenarios
that might impact the virtual concat driver.

[1] https://lore.kernel.org/all/87sermxme1.fsf@bootlin.com/

Regards,
Amit
...
> > +
> >     /* Prefer parsed partitions over driver-provided fallback */
> >     ret = parse_mtd_partitions(mtd, types, parser_data);
> >     if (ret == -EPROBE_DEFER)
>
> Thanks,
> Miquèl