Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1591430 > unrolled thread

[PATCH 2/3] mtd: Add support for reading MTD devices via the nvmem API

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 2/3] mtd: Add support for reading MTD devices via the nvmem API Alban <albeu@free.fr> - 2017-03-02 21:00 +0100
    Re: [PATCH 2/3] mtd: Add support for reading MTD devices via the  nvmem API Boris Brezillon <boris.brezillon@free-electrons.com> - 2017-03-02 22:20 +0100
      Re: [PATCH 2/3] mtd: Add support for reading MTD devices via the  nvmem API Alban <albeu@free.fr> - 2017-03-03 13:40 +0100
        Re: [PATCH 2/3] mtd: Add support for reading MTD devices via the  nvmem API Boris Brezillon <boris.brezillon@free-electrons.com> - 2017-03-03 14:50 +0100
          Re: [PATCH 2/3] mtd: Add support for reading MTD devices via the  nvmem API Boris Brezillon <boris.brezillon@free-electrons.com> - 2017-03-03 15:20 +0100
            Re: [PATCH 2/3] mtd: Add support for reading MTD devices via the  nvmem API Richard Weinberger <richard@nod.at> - 2017-03-03 23:30 +0100
              Re: [PATCH 2/3] mtd: Add support for reading MTD devices via the  nvmem API Alban <albeu@free.fr> - 2017-03-06 18:30 +0100
                Re: [PATCH 2/3] mtd: Add support for reading MTD devices via the  nvmem API Richard Weinberger <richard@nod.at> - 2017-03-06 20:10 +0100
                  Re: [PATCH 2/3] mtd: Add support for reading MTD devices via the  nvmem API Boris Brezillon <boris.brezillon@free-electrons.com> - 2017-03-06 22:10 +0100
          Re: [PATCH 2/3] mtd: Add support for reading MTD devices via the  nvmem API Alban <albeu@free.fr> - 2017-03-03 21:00 +0100
    Re: [PATCH 2/3] mtd: Add support for reading MTD devices via the  nvmem API Srinivas Kandagatla <srinivas.kandagatla@linaro.org> - 2017-03-03 12:30 +0100
      Re: [PATCH 2/3] mtd: Add support for reading MTD devices via the  nvmem API Boris Brezillon <boris.brezillon@free-electrons.com> - 2017-03-03 13:50 +0100
        Re: [PATCH 2/3] mtd: Add support for reading MTD devices via the  nvmem API Alban <albeu@free.fr> - 2017-03-03 14:40 +0100
          Re: [PATCH 2/3] mtd: Add support for reading MTD devices via the  nvmem API Boris Brezillon <boris.brezillon@free-electrons.com> - 2017-03-03 15:20 +0100

#1591430 — [PATCH 2/3] mtd: Add support for reading MTD devices via the nvmem API

Allow drivers that use the nvmem API to read data stored on MTD devices.
This add a simple mtd user that register itself as a read-only nvmem
device.

Signed-off-by: Alban <albeu@free.fr>
---
 drivers/mtd/Kconfig    |   9 ++++
 drivers/mtd/Makefile   |   1 +
 drivers/mtd/mtdnvmem.c | 121 +++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 131 insertions(+)
 create mode 100644 drivers/mtd/mtdnvmem.c

diff --git a/drivers/mtd/Kconfig b/drivers/mtd/Kconfig
index e83a279..9cad86c 100644
--- a/drivers/mtd/Kconfig
+++ b/drivers/mtd/Kconfig
@@ -322,6 +322,15 @@ config MTD_PARTITIONED_MASTER
 	  the parent of the partition device be the master device, rather than
 	  what lies behind the master.
 
+config MTD_NVMEM
+	tristate "Read config data from MTD devices"
+	default y
+	depends on NVMEM
+	help
+	  Provides support for reading config data from MTD devices. This can
+	  be used by drivers to read device specific data such as MAC addresses
+	  or calibration results.
+
 source "drivers/mtd/chips/Kconfig"
 
 source "drivers/mtd/maps/Kconfig"
diff --git a/drivers/mtd/Makefile b/drivers/mtd/Makefile
index 99bb9a1..f62f50b 100644
--- a/drivers/mtd/Makefile
+++ b/drivers/mtd/Makefile
@@ -26,6 +26,7 @@ obj-$(CONFIG_SSFDC)		+= ssfdc.o
 obj-$(CONFIG_SM_FTL)		+= sm_ftl.o
 obj-$(CONFIG_MTD_OOPS)		+= mtdoops.o
 obj-$(CONFIG_MTD_SWAP)		+= mtdswap.o
+obj-$(CONFIG_MTD_NVMEM)		+= mtdnvmem.o
 
 nftl-objs		:= nftlcore.o nftlmount.o
 inftl-objs		:= inftlcore.o inftlmount.o
diff --git a/drivers/mtd/mtdnvmem.c b/drivers/mtd/mtdnvmem.c
new file mode 100644
index 0000000..6eb4216
--- /dev/null
+++ b/drivers/mtd/mtdnvmem.c
@@ -0,0 +1,121 @@
+/*
+ * Copyright (C) 2017 Alban Bedel <albeu@free.fr>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include <linux/module.h>
+#include <linux/mtd/mtd.h>
+#include <linux/nvmem-provider.h>
+#include <linux/nvmem-consumer.h>
+#include <linux/slab.h>
+#include <linux/of.h>
+
+struct mtd_nvmem {
+	struct list_head list;
+	struct mtd_info *mtd;
+	struct nvmem_device *nvmem;
+};
+
+static DEFINE_MUTEX(mtd_nvmem_list_lock);
+static LIST_HEAD(mtd_nvmem_list);
+
+static int mtd_nvmem_reg_read(void *priv, unsigned int offset,
+			      void *val, size_t bytes)
+{
+	struct mtd_info *mtd = priv;
+	size_t retlen;
+	int err;
+
+	err = mtd_read(mtd, offset, bytes, &retlen, val);
+	if (err && err != -EUCLEAN)
+		return err;
+
+	return retlen == bytes ? 0 : -EIO;
+}
+
+static void mtd_nvmem_add(struct mtd_info *mtd)
+{
+	struct device *dev = &mtd->dev;
+	struct device_node *np = dev_of_node(dev);
+	struct nvmem_config config = {};
+	struct mtd_nvmem *mtd_nvmem;
+
+	/* OF devices have to provide the nvmem node */
+	if (np && !of_property_read_bool(np, "nvmem-provider"))
+		return;
+
+	config.dev = dev;
+	config.owner = THIS_MODULE;
+	config.reg_read = mtd_nvmem_reg_read;
+	config.size = mtd->size;
+	config.word_size = 1;
+	config.stride = 1;
+	config.read_only = true;
+	config.priv = mtd;
+
+	/* Alloc our struct to keep track of the MTD NVMEM devices */
+	mtd_nvmem = kzalloc(sizeof(*mtd_nvmem), GFP_KERNEL);
+	if (!mtd_nvmem)
+		return;
+
+	mtd_nvmem->mtd = mtd;
+	mtd_nvmem->nvmem = nvmem_register(&config);
+	if (IS_ERR(mtd_nvmem->nvmem)) {
+		dev_err(dev, "Failed to register NVMEM device\n");
+		kfree(mtd_nvmem);
+		return;
+	}
+
+	mutex_lock(&mtd_nvmem_list_lock);
+	list_add_tail(&mtd_nvmem->list, &mtd_nvmem_list);
+	mutex_unlock(&mtd_nvmem_list_lock);
+}
+
+static void mtd_nvmem_remove(struct mtd_info *mtd)
+{
+	struct mtd_nvmem *mtd_nvmem;
+	bool found = false;
+
+	mutex_lock(&mtd_nvmem_list_lock);
+	list_for_each_entry(mtd_nvmem, &mtd_nvmem_list, list) {
+		if (mtd_nvmem->mtd == mtd) {
+			list_del(&mtd_nvmem->list);
+			found = true;
+			break;
+		}
+	}
+	mutex_unlock(&mtd_nvmem_list_lock);
+
+	if (found) {
+		if (nvmem_unregister(mtd_nvmem->nvmem))
+			dev_err(&mtd->dev,
+				"Failed to unregister NVMEM device\n");
+		kfree(mtd_nvmem);
+	}
+}
+
+static struct mtd_notifier mtd_nvmem_notifier = {
+	.add = mtd_nvmem_add,
+	.remove = mtd_nvmem_remove,
+};
+
+static int __init mtd_nvmem_init(void)
+{
+	register_mtd_user(&mtd_nvmem_notifier);
+	return 0;
+}
+module_init(mtd_nvmem_init);
+
+static void __exit mtd_nvmem_exit(void)
+{
+	unregister_mtd_user(&mtd_nvmem_notifier);
+}
+module_exit(mtd_nvmem_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Alban Bedel <albeu@free.fr>");
+MODULE_DESCRIPTION("Driver to read config data from MTD devices");
-- 
2.7.4

[toc] | [next] | [standalone]

#1591460 — Re: [PATCH 2/3] mtd: Add support for reading MTD devices via the nvmem API

On Thu,  2 Mar 2017 20:50:22 +0100
Alban <albeu@free.fr> wrote:

> Allow drivers that use the nvmem API to read data stored on MTD devices.
> This add a simple mtd user that register itself as a read-only nvmem
> device.
> 
> Signed-off-by: Alban <albeu@free.fr>

Just a few comments, but it looks pretty good already.

> ---
>  drivers/mtd/Kconfig    |   9 ++++
>  drivers/mtd/Makefile   |   1 +
>  drivers/mtd/mtdnvmem.c | 121 +++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 131 insertions(+)
>  create mode 100644 drivers/mtd/mtdnvmem.c
> 
> diff --git a/drivers/mtd/Kconfig b/drivers/mtd/Kconfig
> index e83a279..9cad86c 100644
> --- a/drivers/mtd/Kconfig
> +++ b/drivers/mtd/Kconfig
> @@ -322,6 +322,15 @@ config MTD_PARTITIONED_MASTER
>  	  the parent of the partition device be the master device, rather than
>  	  what lies behind the master.
>  
> +config MTD_NVMEM
> +	tristate "Read config data from MTD devices"
> +	default y
> +	depends on NVMEM
> +	help
> +	  Provides support for reading config data from MTD devices. This can
> +	  be used by drivers to read device specific data such as MAC addresses
> +	  or calibration results.
> +
>  source "drivers/mtd/chips/Kconfig"
>  
>  source "drivers/mtd/maps/Kconfig"
> diff --git a/drivers/mtd/Makefile b/drivers/mtd/Makefile
> index 99bb9a1..f62f50b 100644
> --- a/drivers/mtd/Makefile
> +++ b/drivers/mtd/Makefile
> @@ -26,6 +26,7 @@ obj-$(CONFIG_SSFDC)		+= ssfdc.o
>  obj-$(CONFIG_SM_FTL)		+= sm_ftl.o
>  obj-$(CONFIG_MTD_OOPS)		+= mtdoops.o
>  obj-$(CONFIG_MTD_SWAP)		+= mtdswap.o
> +obj-$(CONFIG_MTD_NVMEM)		+= mtdnvmem.o
>  
>  nftl-objs		:= nftlcore.o nftlmount.o
>  inftl-objs		:= inftlcore.o inftlmount.o
> diff --git a/drivers/mtd/mtdnvmem.c b/drivers/mtd/mtdnvmem.c
> new file mode 100644
> index 0000000..6eb4216
> --- /dev/null
> +++ b/drivers/mtd/mtdnvmem.c
> @@ -0,0 +1,121 @@
> +/*
> + * Copyright (C) 2017 Alban Bedel <albeu@free.fr>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/mtd/mtd.h>
> +#include <linux/nvmem-provider.h>
> +#include <linux/nvmem-consumer.h>
> +#include <linux/slab.h>
> +#include <linux/of.h>
> +
> +struct mtd_nvmem {
> +	struct list_head list;
> +	struct mtd_info *mtd;
> +	struct nvmem_device *nvmem;
> +};
> +
> +static DEFINE_MUTEX(mtd_nvmem_list_lock);
> +static LIST_HEAD(mtd_nvmem_list);

I was wondering if we should have the nvmem pointer directly embedded
in the mtd_info struct. Your approach has the benefit of keeping then
nvmem wrapper completely independent, which is a good thing, but you'll
see below that there's a problem with the MTD notifier approach.

> +
> +static int mtd_nvmem_reg_read(void *priv, unsigned int offset,
> +			      void *val, size_t bytes)
> +{
> +	struct mtd_info *mtd = priv;
> +	size_t retlen;
> +	int err;
> +
> +	err = mtd_read(mtd, offset, bytes, &retlen, val);
> +	if (err && err != -EUCLEAN)
> +		return err;
> +
> +	return retlen == bytes ? 0 : -EIO;
> +}
> +
> +static void mtd_nvmem_add(struct mtd_info *mtd)
> +{
> +	struct device *dev = &mtd->dev;
> +	struct device_node *np = dev_of_node(dev);
> +	struct nvmem_config config = {};
> +	struct mtd_nvmem *mtd_nvmem;
> +
> +	/* OF devices have to provide the nvmem node */
> +	if (np && !of_property_read_bool(np, "nvmem-provider"))
> +		return;

Might have to be adapted according to the DT binding if we decide to
add an extra subnode, but then, I'm not sure the nvmem cells creation
will work correctly, because the framework expect nvmem cells to be
direct children of the nvmem device, which will no longer be the case
if you add an intermediate node between the MTD device node and the
nvmem cell nodes.

> +
> +	config.dev = dev;
> +	config.owner = THIS_MODULE;
> +	config.reg_read = mtd_nvmem_reg_read;
> +	config.size = mtd->size;
> +	config.word_size = 1;
> +	config.stride = 1;
> +	config.read_only = true;
> +	config.priv = mtd;
> +
> +	/* Alloc our struct to keep track of the MTD NVMEM devices */
> +	mtd_nvmem = kzalloc(sizeof(*mtd_nvmem), GFP_KERNEL);
> +	if (!mtd_nvmem)
> +		return;
> +
> +	mtd_nvmem->mtd = mtd;
> +	mtd_nvmem->nvmem = nvmem_register(&config);
> +	if (IS_ERR(mtd_nvmem->nvmem)) {
> +		dev_err(dev, "Failed to register NVMEM device\n");
> +		kfree(mtd_nvmem);
> +		return;
> +	}
> +
> +	mutex_lock(&mtd_nvmem_list_lock);
> +	list_add_tail(&mtd_nvmem->list, &mtd_nvmem_list);
> +	mutex_unlock(&mtd_nvmem_list_lock);
> +}
> +
> +static void mtd_nvmem_remove(struct mtd_info *mtd)
> +{
> +	struct mtd_nvmem *mtd_nvmem;
> +	bool found = false;
> +
> +	mutex_lock(&mtd_nvmem_list_lock);
> +	list_for_each_entry(mtd_nvmem, &mtd_nvmem_list, list) {
> +		if (mtd_nvmem->mtd == mtd) {
> +			list_del(&mtd_nvmem->list);
> +			found = true;
> +			break;
> +		}
> +	}
> +	mutex_unlock(&mtd_nvmem_list_lock);
> +
> +	if (found) {
> +		if (nvmem_unregister(mtd_nvmem->nvmem))
> +			dev_err(&mtd->dev,
> +				"Failed to unregister NVMEM device\n");

Ouch! You failed to unregister the NVMEM device but you have no way to
stop MTD dev removal, which means you have a potential use-after-free
bug. Not sure this can happen in real life, but I don't like that.

Maybe we should let notifiers return an error if they want to cancel
the removal, or maybe this is a good reason to put the nvmem pointer
directly in mtd_info and call mtd_nvmem_add/remove() directly from
add/del_mtd_device() and allow them to return an error.

Not that, if you go for this solution, you'll also get rid of the
global mtd_nvmem_list list and the associated lock.

> +		kfree(mtd_nvmem);
> +	}
> +}
> +
> +static struct mtd_notifier mtd_nvmem_notifier = {
> +	.add = mtd_nvmem_add,
> +	.remove = mtd_nvmem_remove,
> +};
> +
> +static int __init mtd_nvmem_init(void)
> +{
> +	register_mtd_user(&mtd_nvmem_notifier);
> +	return 0;
> +}
> +module_init(mtd_nvmem_init);
> +
> +static void __exit mtd_nvmem_exit(void)
> +{
> +	unregister_mtd_user(&mtd_nvmem_notifier);
> +}
> +module_exit(mtd_nvmem_exit);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Alban Bedel <albeu@free.fr>");
> +MODULE_DESCRIPTION("Driver to read config data from MTD devices");

[toc] | [prev] | [next] | [standalone]

#1591911 — Re: [PATCH 2/3] mtd: Add support for reading MTD devices via the nvmem API

[Multipart message — attachments visible in raw view] — view raw

On Thu, 2 Mar 2017 22:18:03 +0100
Boris Brezillon <boris.brezillon@free-electrons.com> wrote:

> On Thu,  2 Mar 2017 20:50:22 +0100
> Alban <albeu@free.fr> wrote:
> 
> [snip]
>
> > +static void mtd_nvmem_add(struct mtd_info *mtd)
> > +{
> > +	struct device *dev = &mtd->dev;
> > +	struct device_node *np = dev_of_node(dev);
> > +	struct nvmem_config config = {};
> > +	struct mtd_nvmem *mtd_nvmem;
> > +
> > +	/* OF devices have to provide the nvmem node */
> > +	if (np && !of_property_read_bool(np, "nvmem-provider"))
> > +		return;  
> 
> Might have to be adapted according to the DT binding if we decide to
> add an extra subnode, but then, I'm not sure the nvmem cells creation
> will work correctly, because the framework expect nvmem cells to be
> direct children of the nvmem device, which will no longer be the case
> if you add an intermediate node between the MTD device node and the
> nvmem cell nodes.

Yes to support such a binding we would have to fix of_nvmem_cell_get(),
but that should be quiet simple to have it support both the new and old
binding.

>
> [snip]
>
> > +static void mtd_nvmem_remove(struct mtd_info *mtd)
> > +{
> > +	struct mtd_nvmem *mtd_nvmem;
> > +	bool found = false;
> > +
> > +	mutex_lock(&mtd_nvmem_list_lock);
> > +	list_for_each_entry(mtd_nvmem, &mtd_nvmem_list, list) {
> > +		if (mtd_nvmem->mtd == mtd) {
> > +			list_del(&mtd_nvmem->list);
> > +			found = true;
> > +			break;
> > +		}
> > +	}
> > +	mutex_unlock(&mtd_nvmem_list_lock);
> > +
> > +	if (found) {
> > +		if (nvmem_unregister(mtd_nvmem->nvmem))
> > +			dev_err(&mtd->dev,
> > +				"Failed to unregister NVMEM device\n");  
> 
> Ouch! You failed to unregister the NVMEM device but you have no way to
> stop MTD dev removal, which means you have a potential use-after-free
> bug. Not sure this can happen in real life, but I don't like that.

Yes, I'm aware of this problem. Sorry, I forgot to mention this in the
cover letter.

> Maybe we should let notifiers return an error if they want to cancel
> the removal, or maybe this is a good reason to put the nvmem pointer
> directly in mtd_info and call mtd_nvmem_add/remove() directly from
> add/del_mtd_device() and allow them to return an error.
> 
> Not that, if you go for this solution, you'll also get rid of the
> global mtd_nvmem_list list and the associated lock.

IMHO the MTD users framework has to be re-worked to be useful. First
both the add and remove callbacks should have return values. Users where
the add failed shouldn't be removed later and users where the remove
fails should block the removal of the MTD.

Furthermore only passing the MTD device to the add/remove callback
force the users to keep their own list, which is annoying to say the
least. A simple fix would be to have the add callback return a pointer
that would be passed back to the remove callback. Trivial to implement
and the MTD user wouldn't have to keep any list. I will look into this
in the next days.

Alban

[toc] | [prev] | [next] | [standalone]

#1591957 — Re: [PATCH 2/3] mtd: Add support for reading MTD devices via the nvmem API

On Fri, 3 Mar 2017 13:36:29 +0100
Alban <albeu@free.fr> wrote:

> On Thu, 2 Mar 2017 22:18:03 +0100
> Boris Brezillon <boris.brezillon@free-electrons.com> wrote:
> 
> > On Thu,  2 Mar 2017 20:50:22 +0100
> > Alban <albeu@free.fr> wrote:
> > 
> > [snip]
> >  
> > > +static void mtd_nvmem_add(struct mtd_info *mtd)
> > > +{
> > > +	struct device *dev = &mtd->dev;
> > > +	struct device_node *np = dev_of_node(dev);
> > > +	struct nvmem_config config = {};
> > > +	struct mtd_nvmem *mtd_nvmem;
> > > +
> > > +	/* OF devices have to provide the nvmem node */
> > > +	if (np && !of_property_read_bool(np, "nvmem-provider"))
> > > +		return;    
> > 
> > Might have to be adapted according to the DT binding if we decide to
> > add an extra subnode, but then, I'm not sure the nvmem cells creation
> > will work correctly, because the framework expect nvmem cells to be
> > direct children of the nvmem device, which will no longer be the case
> > if you add an intermediate node between the MTD device node and the
> > nvmem cell nodes.  
> 
> Yes to support such a binding we would have to fix of_nvmem_cell_get(),
> but that should be quiet simple to have it support both the new and old
> binding.
> 
> >
> > [snip]
> >  
> > > +static void mtd_nvmem_remove(struct mtd_info *mtd)
> > > +{
> > > +	struct mtd_nvmem *mtd_nvmem;
> > > +	bool found = false;
> > > +
> > > +	mutex_lock(&mtd_nvmem_list_lock);
> > > +	list_for_each_entry(mtd_nvmem, &mtd_nvmem_list, list) {
> > > +		if (mtd_nvmem->mtd == mtd) {
> > > +			list_del(&mtd_nvmem->list);
> > > +			found = true;
> > > +			break;
> > > +		}
> > > +	}
> > > +	mutex_unlock(&mtd_nvmem_list_lock);
> > > +
> > > +	if (found) {
> > > +		if (nvmem_unregister(mtd_nvmem->nvmem))
> > > +			dev_err(&mtd->dev,
> > > +				"Failed to unregister NVMEM device\n");    
> > 
> > Ouch! You failed to unregister the NVMEM device but you have no way to
> > stop MTD dev removal, which means you have a potential use-after-free
> > bug. Not sure this can happen in real life, but I don't like that.  
> 
> Yes, I'm aware of this problem. Sorry, I forgot to mention this in the
> cover letter.

No problem.

> 
> > Maybe we should let notifiers return an error if they want to cancel
> > the removal, or maybe this is a good reason to put the nvmem pointer
> > directly in mtd_info and call mtd_nvmem_add/remove() directly from
> > add/del_mtd_device() and allow them to return an error.
> > 
> > Not that, if you go for this solution, you'll also get rid of the
> > global mtd_nvmem_list list and the associated lock.  
> 
> IMHO the MTD users framework has to be re-worked to be useful. First
> both the add and remove callbacks should have return values. Users where
> the add failed shouldn't be removed later and users where the remove
> fails should block the removal of the MTD.

As said in my previous reply, it's not just about returning an error. I
had a closer look at the code, and it seems that using
__get_mtd_device() properly should prevent the problem we are talking
about (call __get_mtd_device() after your nvmem_register() and call
__put_mtd_device() only if nvmem_unregister() succeed).

> 
> Furthermore only passing the MTD device to the add/remove callback
> force the users to keep their own list, which is annoying to say the
> least. A simple fix would be to have the add callback return a pointer
> that would be passed back to the remove callback. Trivial to implement
> and the MTD user wouldn't have to keep any list. I will look into this
> in the next days.

That's a different problem, and I'm not sure I like the idea of
changing the ->add() prototype into

	void *(*add)(struct mtd_info *);

If we want to do that, I'd rather see an API extension allowing one to
attach/detach/query/update user data to an MTD device.

[toc] | [prev] | [next] | [standalone]

#1591974 — Re: [PATCH 2/3] mtd: Add support for reading MTD devices via the nvmem API

On Fri, 3 Mar 2017 14:57:26 +0100
Alban <albeu@free.fr> wrote:

> On Fri, 3 Mar 2017 14:36:58 +0100
> Boris Brezillon <boris.brezillon@free-electrons.com> wrote:
> 
> > On Fri, 3 Mar 2017 13:36:29 +0100
> > Alban <albeu@free.fr> wrote:
> >   
> > > On Thu, 2 Mar 2017 22:18:03 +0100
> > > Boris Brezillon <boris.brezillon@free-electrons.com> wrote:
> > >     
> > > > On Thu,  2 Mar 2017 20:50:22 +0100
> > > > Alban <albeu@free.fr> wrote:
> > > > 
> > > > [snip]
> > > >      
> > [snip]
> >     
> > > > Maybe we should let notifiers return an error if they want to cancel
> > > > the removal, or maybe this is a good reason to put the nvmem pointer
> > > > directly in mtd_info and call mtd_nvmem_add/remove() directly from
> > > > add/del_mtd_device() and allow them to return an error.
> > > > 
> > > > Not that, if you go for this solution, you'll also get rid of the
> > > > global mtd_nvmem_list list and the associated lock.      
> > > 
> > > IMHO the MTD users framework has to be re-worked to be useful. First
> > > both the add and remove callbacks should have return values. Users where
> > > the add failed shouldn't be removed later and users where the remove
> > > fails should block the removal of the MTD.    
> > 
> > As said in my previous reply, it's not just about returning an error. I
> > had a closer look at the code, and it seems that using
> > __get_mtd_device() properly should prevent the problem we are talking
> > about (call __get_mtd_device() after your nvmem_register() and call
> > __put_mtd_device() only if nvmem_unregister() succeed).  
> 
> That's not going to work. If the notifier add increase the MTD reference
> count it can never be removed again.

That's not true, see the del_mtd_device() function [1], it's calling
the ->remove() notifiers before even testing ->usecount, so, if you
call __put_mtd_device() in your ->remove() hook you should be fine.

> What could work would be to
> propagate the nvmem device ref counting down to the MTD device, but that
> sound complex and would require some non-trivial locking to still allow
> for an "always succeed" removal.
> 
> > > 
> > > Furthermore only passing the MTD device to the add/remove callback
> > > force the users to keep their own list, which is annoying to say the
> > > least. A simple fix would be to have the add callback return a pointer
> > > that would be passed back to the remove callback. Trivial to implement
> > > and the MTD user wouldn't have to keep any list. I will look into this
> > > in the next days.    
> > 
> > That's a different problem, and I'm not sure I like the idea of
> > changing the ->add() prototype into
> > 
> > 	void *(*add)(struct mtd_info *);
> > 
> > If we want to do that, I'd rather see an API extension allowing one to
> > attach/detach/query/update user data to an MTD device.  
> 
> Under which condition would these be triggered? That sound more than is
> needed. I would just use the above add along with:
> 
>  int (*remove)(struct mtd_info *, void *);
> 
> And add a list of successfully added notifiers, along with their
> data pointer, to the MTD device. That's simple and would also remove
> the need for notifier to have a private list of their instances as I
> had to do here.

And then you're abusing the notifier concept. As said earlier, a
notifier is not necessarily using the device, and thus, don't
necessarily need private data.
It's not only about what is the simplest solution for your use case,
but also what other users want/need.


[1]http://lxr.free-electrons.com/source/drivers/mtd/mtdcore.c#L592

[toc] | [prev] | [next] | [standalone]

#1592318 — Re: [PATCH 2/3] mtd: Add support for reading MTD devices via the nvmem API

Am 03.03.2017 um 15:11 schrieb Boris Brezillon:
>> And add a list of successfully added notifiers, along with their
>> data pointer, to the MTD device. That's simple and would also remove
>> the need for notifier to have a private list of their instances as I
>> had to do here.
> 
> And then you're abusing the notifier concept. As said earlier, a
> notifier is not necessarily using the device, and thus, don't
> necessarily need private data.
> It's not only about what is the simplest solution for your use case,
> but also what other users want/need.

Yes, please don't use the mtd_notifier.
I strongly vote to embed the nvmem pointer into struct mtd_info.

Thanks,
//richard

[toc] | [prev] | [next] | [standalone]

#1593543 — Re: [PATCH 2/3] mtd: Add support for reading MTD devices via the nvmem API

[Multipart message — attachments visible in raw view] — view raw

On Fri, 3 Mar 2017 23:21:29 +0100
Richard Weinberger <richard@nod.at> wrote:

> Am 03.03.2017 um 15:11 schrieb Boris Brezillon:
> >> And add a list of successfully added notifiers, along with their
> >> data pointer, to the MTD device. That's simple and would also remove
> >> the need for notifier to have a private list of their instances as I
> >> had to do here.  
> > 
> > And then you're abusing the notifier concept. As said earlier, a
> > notifier is not necessarily using the device, and thus, don't
> > necessarily need private data.
> > It's not only about what is the simplest solution for your use case,
> > but also what other users want/need.  
> 
> Yes, please don't use the mtd_notifier.
> I strongly vote to embed the nvmem pointer into struct mtd_info.

Ok, I'll do that. However it mean it will have to stays in
drivers/mtd as it then become part of the MTD core.

Alban

[toc] | [prev] | [next] | [standalone]

#1593604 — Re: [PATCH 2/3] mtd: Add support for reading MTD devices via the nvmem API

Am 06.03.2017 um 18:21 schrieb Alban:
> On Fri, 3 Mar 2017 23:21:29 +0100
> Richard Weinberger <richard@nod.at> wrote:
> 
>> Am 03.03.2017 um 15:11 schrieb Boris Brezillon:
>>>> And add a list of successfully added notifiers, along with their
>>>> data pointer, to the MTD device. That's simple and would also remove
>>>> the need for notifier to have a private list of their instances as I
>>>> had to do here.  
>>>
>>> And then you're abusing the notifier concept. As said earlier, a
>>> notifier is not necessarily using the device, and thus, don't
>>> necessarily need private data.
>>> It's not only about what is the simplest solution for your use case,
>>> but also what other users want/need.  
>>
>> Yes, please don't use the mtd_notifier.
>> I strongly vote to embed the nvmem pointer into struct mtd_info.
> 
> Ok, I'll do that. However it mean it will have to stays in
> drivers/mtd as it then become part of the MTD core.

Brian, are you fine with this?
I know, refcounting in MTD is tricky. :(

Thanks,
//richard

[toc] | [prev] | [next] | [standalone]

#1593731 — Re: [PATCH 2/3] mtd: Add support for reading MTD devices via the nvmem API

On Mon, 6 Mar 2017 20:03:28 +0100
Richard Weinberger <richard@nod.at> wrote:

> Am 06.03.2017 um 18:21 schrieb Alban:
> > On Fri, 3 Mar 2017 23:21:29 +0100
> > Richard Weinberger <richard@nod.at> wrote:
> >   
> >> Am 03.03.2017 um 15:11 schrieb Boris Brezillon:  
> >>>> And add a list of successfully added notifiers, along with their
> >>>> data pointer, to the MTD device. That's simple and would also remove
> >>>> the need for notifier to have a private list of their instances as I
> >>>> had to do here.    
> >>>
> >>> And then you're abusing the notifier concept. As said earlier, a
> >>> notifier is not necessarily using the device, and thus, don't
> >>> necessarily need private data.
> >>> It's not only about what is the simplest solution for your use case,
> >>> but also what other users want/need.    
> >>
> >> Yes, please don't use the mtd_notifier.
> >> I strongly vote to embed the nvmem pointer into struct mtd_info.  
> > 
> > Ok, I'll do that. However it mean it will have to stays in
> > drivers/mtd as it then become part of the MTD core.  
> 
> Brian, are you fine with this?

Same question to Srinivas.

[toc] | [prev] | [next] | [standalone]

#1592260 — Re: [PATCH 2/3] mtd: Add support for reading MTD devices via the nvmem API

[Multipart message — attachments visible in raw view] — view raw

On Fri, 3 Mar 2017 14:36:58 +0100
Boris Brezillon <boris.brezillon@free-electrons.com> wrote:

> On Fri, 3 Mar 2017 13:36:29 +0100
> Alban <albeu@free.fr> wrote:
> 
> > On Thu, 2 Mar 2017 22:18:03 +0100
> > Boris Brezillon <boris.brezillon@free-electrons.com> wrote:
> >   
> > > On Thu,  2 Mar 2017 20:50:22 +0100
> > > Alban <albeu@free.fr> wrote:
> > > 
> > > [snip]
> > >    
> [snip]
>   
> > > Maybe we should let notifiers return an error if they want to cancel
> > > the removal, or maybe this is a good reason to put the nvmem pointer
> > > directly in mtd_info and call mtd_nvmem_add/remove() directly from
> > > add/del_mtd_device() and allow them to return an error.
> > > 
> > > Not that, if you go for this solution, you'll also get rid of the
> > > global mtd_nvmem_list list and the associated lock.    
> > 
> > IMHO the MTD users framework has to be re-worked to be useful. First
> > both the add and remove callbacks should have return values. Users where
> > the add failed shouldn't be removed later and users where the remove
> > fails should block the removal of the MTD.  
> 
> As said in my previous reply, it's not just about returning an error. I
> had a closer look at the code, and it seems that using
> __get_mtd_device() properly should prevent the problem we are talking
> about (call __get_mtd_device() after your nvmem_register() and call
> __put_mtd_device() only if nvmem_unregister() succeed).

That's not going to work. If the notifier add increase the MTD reference
count it can never be removed again. What could work would be to
propagate the nvmem device ref counting down to the MTD device, but that
sound complex and would require some non-trivial locking to still allow
for an "always succeed" removal.

> > 
> > Furthermore only passing the MTD device to the add/remove callback
> > force the users to keep their own list, which is annoying to say the
> > least. A simple fix would be to have the add callback return a pointer
> > that would be passed back to the remove callback. Trivial to implement
> > and the MTD user wouldn't have to keep any list. I will look into this
> > in the next days.  
> 
> That's a different problem, and I'm not sure I like the idea of
> changing the ->add() prototype into
> 
> 	void *(*add)(struct mtd_info *);
> 
> If we want to do that, I'd rather see an API extension allowing one to
> attach/detach/query/update user data to an MTD device.

Under which condition would these be triggered? That sound more than is
needed. I would just use the above add along with:

 int (*remove)(struct mtd_info *, void *);

And add a list of successfully added notifiers, along with their
data pointer, to the MTD device. That's simple and would also remove
the need for notifier to have a private list of their instances as I
had to do here.

Alban

[toc] | [prev] | [next] | [standalone]

#1591873 — Re: [PATCH 2/3] mtd: Add support for reading MTD devices via the nvmem API

On 02/03/17 19:50, Alban wrote:
> Allow drivers that use the nvmem API to read data stored on MTD devices.
> This add a simple mtd user that register itself as a read-only nvmem
> device.
>
Good stuff!! and useful for MAC addresses.

Am not going to repeat the same comments as Boris, but I totally agree 
with his comments.

> Signed-off-by: Alban <albeu@free.fr>
> ---
>  drivers/mtd/Kconfig    |   9 ++++
>  drivers/mtd/Makefile   |   1 +
>  drivers/mtd/mtdnvmem.c | 121 +++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 131 insertions(+)
>  create mode 100644 drivers/mtd/mtdnvmem.c

May be we should move this driver to drivers/nvmem/
>
> diff --git a/drivers/mtd/Kconfig b/drivers/mtd/Kconfig
> index e83a279..9cad86c 100644
> --- a/drivers/mtd/Kconfig
> +++ b/drivers/mtd/Kconfig
> @@ -322,6 +322,15 @@ config MTD_PARTITIONED_MASTER
>  	  the parent of the partition device be the master device, rather than
>  	  what lies behind the master.
>
> +config MTD_NVMEM
> +	tristate "Read config data from MTD devices"

May be..

"Read config data from MTD devices via NVMEM API".

Or

"MTD NVMEM Provider"

> +	default y

Do you really want it be ON by default?
> +	depends on NVMEM

Adding COMPILE_TEST would give us good test coverage.

> +	help
> +	  Provides support for reading config data from MTD devices. This can
> +	  be used by drivers to read device specific data such as MAC addresses
> +	  or calibration results.
> +
>  source "drivers/mtd/chips/Kconfig"
>
>  source "drivers/mtd/maps/Kconfig"
> diff --git a/drivers/mtd/Makefile b/drivers/mtd/Makefile
> index 99bb9a1..f62f50b 100644
> --- a/drivers/mtd/Makefile
> +++ b/drivers/mtd/Makefile
> @@ -26,6 +26,7 @@ obj-$(CONFIG_SSFDC)		+= ssfdc.o
>  obj-$(CONFIG_SM_FTL)		+= sm_ftl.o
>  obj-$(CONFIG_MTD_OOPS)		+= mtdoops.o
>  obj-$(CONFIG_MTD_SWAP)		+= mtdswap.o
> +obj-$(CONFIG_MTD_NVMEM)		+= mtdnvmem.o
>
>  nftl-objs		:= nftlcore.o nftlmount.o
>  inftl-objs		:= inftlcore.o inftlmount.o
> diff --git a/drivers/mtd/mtdnvmem.c b/drivers/mtd/mtdnvmem.c
> new file mode 100644
> index 0000000..6eb4216
> --- /dev/null
> +++ b/drivers/mtd/mtdnvmem.c
> @@ -0,0 +1,121 @@
> +/*
> + * Copyright (C) 2017 Alban Bedel <albeu@free.fr>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/mtd/mtd.h>
> +#include <linux/nvmem-provider.h>
> +#include <linux/nvmem-consumer.h>
??

> +#include <linux/slab.h>
> +#include <linux/of.h>
> +
> +struct mtd_nvmem {
> +	struct list_head list;
> +	struct mtd_info *mtd;
> +	struct nvmem_device *nvmem;
> +};
> +
> +static DEFINE_MUTEX(mtd_nvmem_list_lock);
> +static LIST_HEAD(mtd_nvmem_list);
> +
> +static int mtd_nvmem_reg_read(void *priv, unsigned int offset,
> +			      void *val, size_t bytes)
> +{
> +	struct mtd_info *mtd = priv;
> +	size_t retlen;
> +	int err;
> +
> +	err = mtd_read(mtd, offset, bytes, &retlen, val);
> +	if (err && err != -EUCLEAN)
> +		return err;
> +
> +	return retlen == bytes ? 0 : -EIO;
> +}
> +
> +static void mtd_nvmem_add(struct mtd_info *mtd)
> +{
> +	struct device *dev = &mtd->dev;
> +	struct device_node *np = dev_of_node(dev);
> +	struct nvmem_config config = {};
> +	struct mtd_nvmem *mtd_nvmem;
> +
> +	/* OF devices have to provide the nvmem node */
> +	if (np && !of_property_read_bool(np, "nvmem-provider"))
> +		return;

we should prefix the property with mtd to make to more explicit that 
this is very much specific to MTD.


> +
> +	config.dev = dev;
> +	config.owner = THIS_MODULE;
> +	config.reg_read = mtd_nvmem_reg_read;
> +	config.size = mtd->size;
> +	config.word_size = 1;
> +	config.stride = 1;
> +	config.read_only = true;
> +	config.priv = mtd;
> +
> +	/* Alloc our struct to keep track of the MTD NVMEM devices */
> +	mtd_nvmem = kzalloc(sizeof(*mtd_nvmem), GFP_KERNEL);
> +	if (!mtd_nvmem)
> +		return;
> +
> +	mtd_nvmem->mtd = mtd;
> +	mtd_nvmem->nvmem = nvmem_register(&config);
> +	if (IS_ERR(mtd_nvmem->nvmem)) {
> +		dev_err(dev, "Failed to register NVMEM device\n");
> +		kfree(mtd_nvmem);
> +		return;
> +	}
> +
> +	mutex_lock(&mtd_nvmem_list_lock);
> +	list_add_tail(&mtd_nvmem->list, &mtd_nvmem_list);
> +	mutex_unlock(&mtd_nvmem_list_lock);
> +}
> +
> +static void mtd_nvmem_remove(struct mtd_info *mtd)
> +{
> +	struct mtd_nvmem *mtd_nvmem;
> +	bool found = false;
> +

May be we can use of_nvmem_find() directly here and avoid all this list 
and lock thingy. It should make the driver much simpler.

Am sure we can add exception to make of_nvmem_find() symbol public if 
its helping providers like this.


> +	mutex_lock(&mtd_nvmem_list_lock);
> +	list_for_each_entry(mtd_nvmem, &mtd_nvmem_list, list) {
> +		if (mtd_nvmem->mtd == mtd) {
> +			list_del(&mtd_nvmem->list);
> +			found = true;
> +			break;
> +		}
> +	}
> +	mutex_unlock(&mtd_nvmem_list_lock);
> +
> +	if (found) {
> +		if (nvmem_unregister(mtd_nvmem->nvmem))
> +			dev_err(&mtd->dev,
> +				"Failed to unregister NVMEM device\n");

I will be nice to feedback error to top layer, as it does not make sense 
to remove providers if there are active consumers using it.

del_mtd_device(), unregister_mtd_user() have return values, I see no 
reason why notifiers  should not return errors.
May be if we should fix the remove() call backs to handle and return errors.


> +		kfree(mtd_nvmem);
> +	}

> +}
> +
> +static struct mtd_notifier mtd_nvmem_notifier = {
> +	.add = mtd_nvmem_add,
> +	.remove = mtd_nvmem_remove,
> +};
> +
> +static int __init mtd_nvmem_init(void)
> +{
> +	register_mtd_user(&mtd_nvmem_notifier);
> +	return 0;
> +}
> +module_init(mtd_nvmem_init);
> +
> +static void __exit mtd_nvmem_exit(void)
> +{
> +	unregister_mtd_user(&mtd_nvmem_notifier);
> +}
> +module_exit(mtd_nvmem_exit);

> +
> +MODULE_LICENSE("GPL");

GPL V2  ??


Thanks,
srini
> +MODULE_AUTHOR("Alban Bedel <albeu@free.fr>");
> +MODULE_DESCRIPTION("Driver to read config data from MTD devices");
>

[toc] | [prev] | [next] | [standalone]

#1591918 — Re: [PATCH 2/3] mtd: Add support for reading MTD devices via the nvmem API

On Fri, 3 Mar 2017 11:23:16 +0000
Srinivas Kandagatla <srinivas.kandagatla@linaro.org> wrote:


> 
> > +	mutex_lock(&mtd_nvmem_list_lock);
> > +	list_for_each_entry(mtd_nvmem, &mtd_nvmem_list, list) {
> > +		if (mtd_nvmem->mtd == mtd) {
> > +			list_del(&mtd_nvmem->list);
> > +			found = true;
> > +			break;
> > +		}
> > +	}
> > +	mutex_unlock(&mtd_nvmem_list_lock);
> > +
> > +	if (found) {
> > +		if (nvmem_unregister(mtd_nvmem->nvmem))
> > +			dev_err(&mtd->dev,
> > +				"Failed to unregister NVMEM device\n");  
> 
> I will be nice to feedback error to top layer, as it does not make sense 
> to remove providers if there are active consumers using it.
> 
> del_mtd_device(), unregister_mtd_user() have return values, I see no 
> reason why notifiers  should not return errors.
> May be if we should fix the remove() call backs to handle and return errors.

It's more complicated than that. What should you do if one of the
->remove() notifier in the middle of the list is returning an error?
Some of them have already taken the remove notification into account.
Should we call ->add() back on those notifiers? Also, I'm not sure they
are all safe against double ->remove() calls, so if we might be in
trouble when the removal is retried.

[toc] | [prev] | [next] | [standalone]

#1591950 — Re: [PATCH 2/3] mtd: Add support for reading MTD devices via the nvmem API

[Multipart message — attachments visible in raw view] — view raw

On Fri, 3 Mar 2017 13:34:19 +0100
Boris Brezillon <boris.brezillon@free-electrons.com> wrote:

> On Fri, 3 Mar 2017 11:23:16 +0000
> Srinivas Kandagatla <srinivas.kandagatla@linaro.org> wrote:
> 
> 
> >   
> > > +	mutex_lock(&mtd_nvmem_list_lock);
> > > +	list_for_each_entry(mtd_nvmem, &mtd_nvmem_list, list) {
> > > +		if (mtd_nvmem->mtd == mtd) {
> > > +			list_del(&mtd_nvmem->list);
> > > +			found = true;
> > > +			break;
> > > +		}
> > > +	}
> > > +	mutex_unlock(&mtd_nvmem_list_lock);
> > > +
> > > +	if (found) {
> > > +		if (nvmem_unregister(mtd_nvmem->nvmem))
> > > +			dev_err(&mtd->dev,
> > > +				"Failed to unregister NVMEM device\n");    
> > 
> > I will be nice to feedback error to top layer, as it does not make sense 
> > to remove providers if there are active consumers using it.
> > 
> > del_mtd_device(), unregister_mtd_user() have return values, I see no 
> > reason why notifiers  should not return errors.
> > May be if we should fix the remove() call backs to handle and return errors.  
> 
> It's more complicated than that. What should you do if one of the
> ->remove() notifier in the middle of the list is returning an error?  
> Some of them have already taken the remove notification into account.
> Should we call ->add() back on those notifiers? Also, I'm not sure they
> are all safe against double ->remove() calls, so if we might be in
> trouble when the removal is retried.

Re-adding make no sense as that could also fails. Keep it simple,
remove the notifier from the list when remove() succeed, abort when one
fails. In such a scenario that mean there is a dependency, the sys
admin should then solve this dependency and re-trigger the MTD removal.

Alban

[toc] | [prev] | [next] | [standalone]

#1591978 — Re: [PATCH 2/3] mtd: Add support for reading MTD devices via the nvmem API

On Fri, 3 Mar 2017 14:30:21 +0100
Alban <albeu@free.fr> wrote:

> On Fri, 3 Mar 2017 13:34:19 +0100
> Boris Brezillon <boris.brezillon@free-electrons.com> wrote:
> 
> > On Fri, 3 Mar 2017 11:23:16 +0000
> > Srinivas Kandagatla <srinivas.kandagatla@linaro.org> wrote:
> > 
> >   
> > >     
> > > > +	mutex_lock(&mtd_nvmem_list_lock);
> > > > +	list_for_each_entry(mtd_nvmem, &mtd_nvmem_list, list) {
> > > > +		if (mtd_nvmem->mtd == mtd) {
> > > > +			list_del(&mtd_nvmem->list);
> > > > +			found = true;
> > > > +			break;
> > > > +		}
> > > > +	}
> > > > +	mutex_unlock(&mtd_nvmem_list_lock);
> > > > +
> > > > +	if (found) {
> > > > +		if (nvmem_unregister(mtd_nvmem->nvmem))
> > > > +			dev_err(&mtd->dev,
> > > > +				"Failed to unregister NVMEM device\n");      
> > > 
> > > I will be nice to feedback error to top layer, as it does not make sense 
> > > to remove providers if there are active consumers using it.
> > > 
> > > del_mtd_device(), unregister_mtd_user() have return values, I see no 
> > > reason why notifiers  should not return errors.
> > > May be if we should fix the remove() call backs to handle and return errors.    
> > 
> > It's more complicated than that. What should you do if one of the  
> > ->remove() notifier in the middle of the list is returning an error?    
> > Some of them have already taken the remove notification into account.
> > Should we call ->add() back on those notifiers? Also, I'm not sure they
> > are all safe against double ->remove() calls, so if we might be in
> > trouble when the removal is retried.  
> 
> Re-adding make no sense as that could also fails.

I agree.

> Keep it simple,
> remove the notifier from the list when remove() succeed, abort when one
> fails. In such a scenario that mean there is a dependency, the sys
> admin should then solve this dependency and re-trigger the MTD removal.

Except notifiers are by definition not attached to a specific MTD
device. I get your point, but I think we should clarify the different
concepts.

An mtd_notifier (which seems to also be called a user in a few places)
is something that should be called each time you have an MTD
creation/removal event (or when you add a notifier to the list). You
could have notifiers that don't do anything special with the MTD
device, hence they don't require private data.

I think we should add the mtd_user concept, which would be a specific
user of an MTD device that can contain private data and is likely to be
attached to the MTD device after the notifier's ->add() method is
called.

struct mtd_user_ops {
	int (*remove)(struct mtd_user *);
};

struct mtd_user {
	struct list_node node;
	const struct mtd_user_ops *ops;
}

int mtd_attach_user(struct mtd_info *mtd, struct mtd_user *user);
int mtd_detach_user(struct mtd_info *mtd, struct mtd_user *user);

and then inside the del_mtd_device() function, before you iterate over
all notifiers, you could iterate over all attached users and call their
->remove() method. If one fails, then you stop the removal procedure.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web