Re: [PATCH v2] mtd: Fix mtd not the same name not registered if nvmem

Ricardo Ribalda Delgado
2020-04-14T13:47:54Z

Ping?

On Thu, Apr 2, 2020 at 8:59 AM Ricardo Ribalda Delgado
<ribalda@kernel.org> wrote:
>
> When the nvmem framework is enabled, a nvmem device is created per mtd
> device/partition.
>
> It is not uncommon that a device can have multiple mtd devices with
> partitions that have the same name. Eg, when there DT overlay is allowed
> and the same device with mtd is attached twice.
>
> Under that circumstances, the mtd fails to register due to a name
> duplication on the nvmem framework.
>
> With this patch we add a _1, _2, _X to the subsequent names if there is
> a collition, and throw a warning, instead of not starting the mtd
> device.
>
> [ 8.948991] sysfs: cannot create duplicate filename '/bus/nvmem/devices/Production Data'
> [ 8.948992] CPU: 7 PID: 246 Comm: systemd-udevd Not tainted 5.5.0-qtec-standard #13
> [ 8.948993] Hardware name: AMD Dibbler/Dibbler, BIOS 05.22.04.0019 10/26/2019
> [ 8.948994] Call Trace:
> [ 8.948996] dump_stack+0x50/0x70
> [ 8.948998] sysfs_warn_dup.cold+0x17/0x2d
> [ 8.949000] sysfs_do_create_link_sd.isra.0+0xc2/0xd0
> [ 8.949002] bus_add_device+0x74/0x140
> [ 8.949004] device_add+0x34b/0x850
> [ 8.949006] nvmem_register.part.0+0x1bf/0x640
> ...
> [ 8.948926] mtd mtd8: Failed to register NVMEM device
>
> Signed-off-by: Ricardo Ribalda Delgado <ribalda@kernel.org>
> ---
> v2: I left behind on my patch a
>
> mtd->nvmem = NULL;
>
> from my tests. Sorry.
>
> drivers/mtd/mtdcore.c | 35 ++++++++++++++++++++++++++++++++++-
> 1 file changed, 34 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> index 5fac4355b9c2..7a4b520ef3b0 100644
> --- a/drivers/mtd/mtdcore.c
> +++ b/drivers/mtd/mtdcore.c
> @@ -28,6 +28,7 @@
> #include <linux/leds.h>
> #include <linux/debugfs.h>
> #include <linux/nvmem-provider.h>
> +#include <linux/nvmem-consumer.h>
>
> #include <linux/mtd/mtd.h>
> #include <linux/mtd/partitions.h>
> @@ -545,13 +546,34 @@ static int mtd_nvmem_reg_read(void *priv, unsigned int offset,
> return retlen == bytes ? 0 : -EIO;
> }
>
> +static int nvmem_next_name(const char *init_name, char *name, size_t len)
> +{
> + unsigned int i = 0;
> + int ret = 0;
> + struct nvmem_device *dev = NULL;
> +
> + strlcpy(name, init_name, len);
> +
> + while ((ret < len) &&
> + !IS_ERR(dev = nvmem_device_find(name, device_match_name))) {
> + nvmem_device_put(dev);
> + ret = snprintf(name, len, "%s_%u", init_name, ++i);
> + }
> +
> + if (ret >= len)
> + return -ENOMEM;
> +
> + return i;
> +}
> +
> static int mtd_nvmem_add(struct mtd_info *mtd)
> {
> struct nvmem_config config = {};
> + char name[128];
> + int ret = 0;
>
> config.id = -1;
> config.dev = &mtd->dev;
> - config.name = mtd->name;
> config.owner = THIS_MODULE;
> config.reg_read = mtd_nvmem_reg_read;
> config.size = mtd->size;
> @@ -562,6 +584,13 @@ static int mtd_nvmem_add(struct mtd_info *mtd)
> config.no_of_node = true;
> config.priv = mtd;
>
> + if (mtd->name) {
> + ret = nvmem_next_name(mtd->name, name, sizeof(name));
> + if (ret < 0)
> + return ret;
> + config.name = name;
> + }
> +
> mtd->nvmem = nvmem_register(&config);
> if (IS_ERR(mtd->nvmem)) {
> /* Just ignore if there is no NVMEM support in the kernel */
> @@ -573,6 +602,10 @@ static int mtd_nvmem_add(struct mtd_info *mtd)
> }
> }
>
> + if (ret)
> + dev_warn(&mtd->dev, "mtdev %s renamed to %s due to name collision",
> + mtd->name, nvmem_dev_name(mtd->nvmem));
> +
> return 0;
> }
>
> --
> 2.25.1
>