On 05/02/2025 at 19:07:30 +0530, Amit Kumar Mahapatra <amit.kumar-mahapatra@xxxxxxx> wrote:
> Introducing CONFIG_VIRT_CONCAT to separate the legacy flow from the
> new
CONFIG_MTD_VIRT_CONCAT
> approach, where individual partitions within a concatenated partition are
> not registered, as they are likely not needed by the user.
I am not a big fan of this choice. We had issues with hiding things to
the user in the first place. Could we find a way to expose both the
original mtd devices as well as the virtually concatenated partitions?
> Solution is focusing on fixed-partitions description only because it
> depends on device boundaries.
>
> Suggested-by: Bernhard Frauendienst <kernel@xxxxxxxxxxxxxxxxx>
> Suggested-by: Miquel Raynal <miquel.raynal@xxxxxxxxxxx>
> Signed-off-by: Amit Kumar Mahapatra <amit.kumar-mahapatra@xxxxxxx>
> ---
> drivers/mtd/Kconfig | 8 ++
> drivers/mtd/Makefile | 1 +
> drivers/mtd/mtd_virt_concat.c | 254 ++++++++++++++++++++++++++++++++++
> drivers/mtd/mtdcore.c | 7 +
> drivers/mtd/mtdpart.c | 6 +
> include/linux/mtd/concat.h | 24 ++++
> 6 files changed, 300 insertions(+)
> create mode 100644 drivers/mtd/mtd_virt_concat.c
>
> diff --git a/drivers/mtd/Kconfig b/drivers/mtd/Kconfig
> index 796a2eccbef0..3dade7c469df 100644
> --- a/drivers/mtd/Kconfig
> +++ b/drivers/mtd/Kconfig
> @@ -206,6 +206,14 @@ config MTD_PARTITIONED_MASTER
> the parent of the partition device be the master device, rather than
> what lies behind the master.
>
> +config MTD_VIRT_CONCAT
> + tristate "Virtual concatenated MTD devices"
> + help
> + The driver enables the creation of a virtual MTD device
of virtual MTD devices
> + by concatenating multiple physical MTD devices into a single
> + entity. This allows for the creation of partitions larger than
> + the individual physical chips, extending across chip boundaries.
> +
...
> +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.
> +static void __exit mtd_virt_concat_exit(void)
> +{
> + mtd_virt_concat_destroy_joins();
> + mtd_virt_concat_destroy_items();
> +}
> +module_exit(mtd_virt_concat_exit);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Bernhard Frauendienst <kernel@xxxxxxxxxxxxxxxxx>");
> +MODULE_AUTHOR("Amit Kumar Mahapatra <amit.kumar-mahapatra@xxxxxxx>");
> +MODULE_DESCRIPTION("Virtual concat MTD device driver");
> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> index 724f917f91ba..2264fe81810f 100644
> --- a/drivers/mtd/mtdcore.c
> +++ b/drivers/mtd/mtdcore.c
> @@ -34,6 +34,7 @@
>
> #include <linux/mtd/mtd.h>
> #include <linux/mtd/partitions.h>
> +#include <linux/mtd/concat.h>
>
> #include "mtdcore.h"
>
> @@ -1067,6 +1068,12 @@ int mtd_device_parse_register(struct mtd_info *mtd, const char * const *types,
> goto out;
> }
>
> + if (IS_ENABLED(CONFIG_MTD_VIRT_CONCAT)) {
Maybe IS_REACHABLE() is more relevant?
> + ret = mtd_virt_concat_node_create();
> + if (ret < 0)
> + goto out;
> + }
> +
> /* Prefer parsed partitions over driver-provided fallback */
> ret = parse_mtd_partitions(mtd, types, parser_data);
> if (ret == -EPROBE_DEFER)
Thanks,
Miquèl
