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

[Miquel Raynal <miquel.raynal@bootlin.com>]

On 21/05/2025 at 06:13:32 GMT, "Mahapatra, Amit Kumar" <amit.kumar-mahapatra@amd.com> wrote:

> [AMD Official Use Only - AMD Internal Distribution Only]
>
>> On 13/05/2025 at 14:45:39 GMT, "Mahapatra, Amit Kumar" <amit.kumar-
>> mahapatra@amd.com> wrote:
>>
>> > [AMD Official Use Only - AMD Internal Distribution Only]
>> >
>> > Hello Miquel,
>> >
>> >> >> > +           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.
>> >>
>> >> That's true, but I kind of dislike the late_initcall, I fear it might break in creative
>> ways.
>> >
>> > I understand, but since we require the partition information to be
>> > available, late_initcall seems to be the most suitable choice among
>> > the initcall levels—if we decide to proceed with using an initcall.
>> > Regarding potential failures, as far as I can tell, the operation
>> > would fail if, at the time of concatenation, one or more of the MTD
>> > devices involved in the concat are not yet available. In such a
>> > scenario, we can issue a kernel warning and exit gracefully. But,
>> > However, if you prefer to move away from using initcalls and have an
>> > alternative implementation approach in mind, please let us know.
>>
>> I am sorry but this does not work with modules, and we cannot ignore this case I
>> believe. More specifically, if a controller probe is deferred (with EPROBE_DEFER
>> or just prevented because some dependencies are not yet satisfied), you'll get
>> incorrectly defined mtd devices.
>
> Ok, an alternative solution could be to remove the initcall registration
> and instead invoke mtd_virt_concat_create_join()—which was previously
> registered as a late_initcall—directly from mtd_device_parse_register().
> I believe this approach would address both of your concerns regarding
> module support and probe deferral. Additionally, we could consider
> moving the entire code from mtd_virt_concat.c into mtdconcat.c.
> Please let us know your take on this.

What would this bring?

Maybe we should trigger some kind of notifier after registering an mtd
device and in there attempt to gather all mtd devices required for the
concatenation. Can you please propose something like that?

Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/