[RFC/PATCH 0/2] Adding support to expose mtd flash otp regions via nvmem

[RFC/PATCH 0/2] Adding support to expose mtd flash otp regions via nvmem

[Moritz Fischer]

Hi all,

attached series allows exporting mtd flash otp regions via nvmem.
This is my first stab and still needs cleanup, but I wanted to get some
general feedback on whether this can conceptually work.

In an earlier conversation with Boris (in Cc) he suggested (I think),
something similar to what I tried to implement.

If someone has suggestions (especially on how to deduplicate some of the parsing
code w.r.t to partitions I'd be happy to rework the series)

Thanks for your feedback,

Moritz

Moritz Fischer (2):
  doc: bindings: Add bindings documentation for mtd otp nvmem
  mtd: otp: Expose mtd flash otp regions as nvmem providers

 .../devicetree/bindings/mtd/otp-nvmem.txt          |  62 ++++++++++
 drivers/mtd/Kconfig                                |   8 ++
 drivers/mtd/Makefile                               |   1 +
 drivers/mtd/mtdcore.c                              |  35 ++++++
 drivers/mtd/mtdcore.h                              |   1 +
 drivers/mtd/mtdnvmem.c                             | 127 +++++++++++++++++++++
 drivers/mtd/mtdnvmem.h                             |  25 ++++
 drivers/mtd/mtdpart.c                              | 112 ++++++++++++++++++
 drivers/mtd/ofpart.c                               | 102 +++++++++++++++++
 9 files changed, 473 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/otp-nvmem.txt
 create mode 100644 drivers/mtd/mtdnvmem.c
 create mode 100644 drivers/mtd/mtdnvmem.h

-- 
2.5.5

[RFC/PATCH 1/2] doc: bindings: Add bindings documentation for mtd otp nvmem

[Moritz Fischer]

This commit adds documentation describing the bindings for
exposing mtd flash otp regions as nvmem providers via devicetree.

Signed-off-by: Moritz Fischer <moritz.fischer@ettus.com>
---
 .../devicetree/bindings/mtd/otp-nvmem.txt          | 62 ++++++++++++++++++++++
 1 file changed, 62 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mtd/otp-nvmem.txt

diff --git a/Documentation/devicetree/bindings/mtd/otp-nvmem.txt b/Documentation/devicetree/bindings/mtd/otp-nvmem.txt
new file mode 100644
index 0000000..a83a7da
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/otp-nvmem.txt
@@ -0,0 +1,62 @@
+Representing OTP regions in devicetree
+
+OTP regions can be represented by sub-nodes of an mtd device.
+
+The partition table should be a subnode of the mtd node and should be names
+'otp-partitions'. This node should have the following property:
+
+- compatible: (required) must be 'fixed-partitions'
+
+OTP regions are then defined in subnodes of the partitions node.
+
+Required properties for OTP regions:
+- reg: The region's offset and size within the mtd device
+
+Optional properties:
+- label: The label / name for this region. If ommited, the label is taken
+  from the node name (excluding the unit address).
+
+Example:
+
+flash@0 {
+	partitions {
+		compatible = "fixed-partitions";
+		#address-cells = <1>;
+		#size-cells = <1>;
+
+		partition@0 {
+			label = "uboot-spl";
+			reg = <0x0 0xe0000>;
+		};
+		partition@1 {
+			label = "uboot-env";
+			reg = <0xe0000 0x20000>;
+		};
+		partition@2 {
+			label = "uboot";
+			reg = <0x100000 0x100000>;
+		};
+	};
+
+	otp-partitions {
+		compatible = "fixed-partitions";
+		#address-cells = <1>;
+		#size-cells = <1>;
+
+		region@0 {
+			label = "factory-data";
+			reg = <0x0 0x40>;
+
+			#address-cells = <0x1>;
+			#size-cells = <0x1>;
+
+			product: nvmem@0 {
+				reg = <0x0 0x2>;
+			};
+
+			revision: nvmem@3 {
+				reg = <0x3 0x2>;
+			};
+		};
+	};
+}
-- 
2.5.5

[RFC/PATCH 2/2] mtd: otp: Expose mtd flash otp regions as nvmem providers

[Moritz Fischer]

This commit introduces support for exposing otp regions
in mtd flash devices as nvmem providers, allowing them to provide
information such as serial numbers, calibration data or
product ids.

Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Brian Norris <computersforpeace@gmail.com>
Cc: Boris Brezillon <boris.brezillon@free-electrons.com>

Signed-off-by: Moritz Fischer <moritz.fischer@ettus.com>
---
 drivers/mtd/Kconfig    |   8 ++++
 drivers/mtd/Makefile   |   1 +
 drivers/mtd/mtdcore.c  |  35 ++++++++++++++
 drivers/mtd/mtdcore.h  |   1 +
 drivers/mtd/mtdnvmem.c | 127 +++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/mtd/mtdnvmem.h |  25 ++++++++++
 drivers/mtd/mtdpart.c  | 112 +++++++++++++++++++++++++++++++++++++++++++
 drivers/mtd/ofpart.c   | 102 +++++++++++++++++++++++++++++++++++++++
 8 files changed, 411 insertions(+)
 create mode 100644 drivers/mtd/mtdnvmem.c
 create mode 100644 drivers/mtd/mtdnvmem.h

diff --git a/drivers/mtd/Kconfig b/drivers/mtd/Kconfig
index e83a279..4d99b64 100644
--- a/drivers/mtd/Kconfig
+++ b/drivers/mtd/Kconfig
@@ -309,6 +309,14 @@ config MTD_SWAP
 	  The driver provides wear leveling by storing erase counter into the
 	  OOB.
 
+config MTD_NVMEM
+	tristate "Expose OTP regions as NVMEM providers"
+	depends on MTD && NVMEM
+	help
+	  Allows exposing OTP regions in mtd devices via nvmem.
+	  This is useful for storing information like calibration data,
+	  serial numbers, or mac addresses.
+
 config MTD_PARTITIONED_MASTER
 	bool "Retain master device when partitioned"
 	default n
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/mtdcore.c b/drivers/mtd/mtdcore.c
index 8965ffa..9f187bc 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -46,6 +46,7 @@
 #include <linux/mtd/partitions.h>
 
 #include "mtdcore.h"
+#include "mtdnvmem.h"
 
 static struct backing_dev_info mtd_bdi = {
 };
@@ -566,6 +567,30 @@ static int mtd_add_device_partitions(struct mtd_info *mtd,
 	return 0;
 }
 
+#ifdef CONFIG_MTD_NVMEM
+static int mtd_add_device_otp_regions(struct mtd_info *mtd,
+				      struct mtd_partitions *parts)
+{
+	const struct mtd_partition *real_parts = parts->parts;
+	int nbparts = parts->nr_parts;
+	int ret;
+
+	if (nbparts > 0) {
+		ret = add_otp_regions(mtd, real_parts, nbparts);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+#else
+static int mtd_add_device_otp_regions(struct mtd_info *mtd,
+				      struct mtd_partitions *parts)
+{
+	return 0;
+}
+#endif
+
 /*
  * Set a few defaults based on the parent devices, if not provided by the
  * driver
@@ -582,6 +607,8 @@ static void mtd_set_dev_defaults(struct mtd_info *mtd)
 	}
 }
 
+static const char const *otp_types[] = {"ofotp", NULL};
+
 /**
  * mtd_device_parse_register - parse partitions and register an MTD device.
  *
@@ -642,6 +669,14 @@ int mtd_device_parse_register(struct mtd_info *mtd, const char * const *types,
 	if (ret)
 		goto out;
 
+	ret = parse_mtd_partitions(mtd, otp_types, &parsed, parser_data);
+	if (ret)
+		goto out;
+
+	ret = mtd_add_device_otp_regions(mtd, &parsed);
+	if (ret)
+		goto out;
+
 	/*
 	 * FIXME: some drivers unfortunately call this function more than once.
 	 * So we have to check if we've already assigned the reboot notifier.
diff --git a/drivers/mtd/mtdcore.h b/drivers/mtd/mtdcore.h
index 55fdb8e..11ccb90 100644
--- a/drivers/mtd/mtdcore.h
+++ b/drivers/mtd/mtdcore.h
@@ -9,6 +9,7 @@ struct mtd_info *__mtd_next_device(int i);
 int add_mtd_device(struct mtd_info *mtd);
 int del_mtd_device(struct mtd_info *mtd);
 int add_mtd_partitions(struct mtd_info *, const struct mtd_partition *, int);
+int add_otp_regions(struct mtd_info *, const struct mtd_partition *, int);
 int del_mtd_partitions(struct mtd_info *);
 
 struct mtd_partitions;
diff --git a/drivers/mtd/mtdnvmem.c b/drivers/mtd/mtdnvmem.c
new file mode 100644
index 0000000..e99a95e
--- /dev/null
+++ b/drivers/mtd/mtdnvmem.c
@@ -0,0 +1,127 @@
+/*
+ * Copyright (c) 2016, National Instruments Corp.
+ *
+ * Generic NVMEM support for OTP regions in MTD devices
+ *
+ * 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; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/device.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/nvmem-provider.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+#include <linux/mtd/mtd.h>
+
+struct mtd_nvmem {
+	struct mtd_info *info;
+	struct nvmem_device *dev;
+	struct regmap *regmap;
+};
+
+static int mtd_otp_regmap_read(void *context, const void *reg, size_t reg_size,
+			       void *val, size_t val_size)
+{
+	struct mtd_info *info = context;
+	const u8 *offset = reg;
+	int err;
+	size_t retlen;
+
+	if (reg_size != 1)
+		return -EINVAL;
+
+	err = mtd_read_user_prot_reg(info, *offset, val_size > info->size
+				     ? info->size : val_size, &retlen,
+				     (u_char *)val);
+
+	return 0;
+}
+
+static int mtd_otp_regmap_write(void *context, const void *data, size_t count)
+{
+	/* Not implemented */
+	return 0;
+}
+
+static const struct regmap_bus mtd_otp_bus = {
+	.read = mtd_otp_regmap_read,
+	.write = mtd_otp_regmap_write,
+	.reg_format_endian_default = REGMAP_ENDIAN_NATIVE,
+	.val_format_endian_default = REGMAP_ENDIAN_NATIVE,
+};
+
+static bool mtd_otp_nvmem_writeable_reg(struct device *dev, unsigned int reg)
+{
+	return false;
+}
+
+static struct regmap_config mtd_otp_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.reg_stride = 1,
+	.writeable_reg = mtd_otp_nvmem_writeable_reg,
+	.name = "mtd-otp",
+};
+
+static struct nvmem_config mtd_otp_nvmem_config = {
+	.read_only = true,
+	.owner = THIS_MODULE,
+};
+
+struct mtd_nvmem *mtd_otp_nvmem_register(struct mtd_info *info)
+{
+	struct mtd_nvmem *nvmem;
+	struct device *dev = &info->dev;
+
+	nvmem = kzalloc(sizeof(*nvmem), GFP_KERNEL);
+	if (!nvmem)
+		return ERR_PTR(-ENOMEM);
+
+	mtd_otp_regmap_config.max_register = info->size;
+
+	nvmem->regmap = regmap_init(dev, &mtd_otp_bus, info,
+				  &mtd_otp_regmap_config);
+	if (IS_ERR(nvmem->regmap)) {
+		dev_err(dev, "regmap init failed");
+		goto out_free;
+	}
+
+	mtd_otp_nvmem_config.dev = dev;
+	mtd_otp_nvmem_config.name = info->name;
+
+	nvmem->dev = nvmem_register(&mtd_otp_nvmem_config);
+
+	if (!nvmem->dev) {
+		dev_err(dev, "failed to register nvmem");
+		goto out_regmap;
+	}
+
+	return nvmem;
+
+out_regmap:
+	regmap_exit(nvmem->regmap);
+
+out_free:
+	kfree(nvmem);
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(mtd_otp_nvmem_register);
+
+void mtd_otp_nvmem_remove(struct mtd_nvmem *nvmem)
+{
+	nvmem_unregister(nvmem->dev);
+	regmap_exit(nvmem->regmap);
+	kfree(nvmem);
+}
+EXPORT_SYMBOL_GPL(mtd_otp_nvmem_remove);
diff --git a/drivers/mtd/mtdnvmem.h b/drivers/mtd/mtdnvmem.h
new file mode 100644
index 0000000..1003c5f
--- /dev/null
+++ b/drivers/mtd/mtdnvmem.h
@@ -0,0 +1,25 @@
+/*
+ * Copyright (c) 2016, National Instruments Corp.
+ *
+ * Generic NVMEM support for OTP regions in MTD devices
+ *
+ * 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; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef MTDNVMEM_H
+#define MTDNVMEM_H
+
+struct mtd_nvmem;
+
+struct mtd_nvmem *mtd_otp_nvmem_register(struct mtd_info *info);
+
+void mtd_otp_nvmem_remove(struct mtd_nvmem *mem);
+
+#endif /* MTDNVMEM_H */
diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index 6ef118d..7acf82bf 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -33,6 +33,7 @@
 #include <linux/kconfig.h>
 
 #include "mtdcore.h"
+#include "mtdnvmem.h"
 
 /* Our partition linked list */
 static LIST_HEAD(mtd_partitions);
@@ -44,6 +45,7 @@ struct mtd_part {
 	struct mtd_info *master;
 	uint64_t offset;
 	struct list_head list;
+	struct mtd_nvmem *nv;
 };
 
 /*
@@ -319,6 +321,10 @@ static int part_block_markbad(struct mtd_info *mtd, loff_t ofs)
 
 static inline void free_partition(struct mtd_part *p)
 {
+#ifdef CONFIG_MTD_NVMEM
+	if (p->nv)
+		mtd_otp_nvmem_remove(p->nv);
+#endif
 	kfree(p->mtd.name);
 	kfree(p);
 }
@@ -349,6 +355,72 @@ int del_mtd_partitions(struct mtd_info *master)
 	return err;
 }
 
+#ifdef CONFIG_MTD_NVMEM
+static struct mtd_part *allocate_otp_region(struct mtd_info *master,
+					    const struct mtd_partition *part,
+					    int partno, uint64_t cur_offset)
+{
+	struct mtd_part *slave;
+	char *name;
+
+	/* allocate the partition structure */
+	slave = kzalloc(sizeof(*slave), GFP_KERNEL);
+	name = kstrdup(part->name, GFP_KERNEL);
+	if (!name || !slave) {
+		pr_err("memory allocation error while creating partitions for \"%s\"\n",
+		       master->name);
+		kfree(name);
+		kfree(slave);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	/* set up the MTD object for this partition */
+	slave->mtd.type = master->type;
+	slave->mtd.flags = master->flags & ~part->mask_flags;
+	slave->mtd.size = part->size;
+	slave->mtd.writesize = 1;
+	slave->mtd.writebufsize = 1;
+	slave->mtd.erasesize = 1;
+	slave->mtd.oobsize = 0;
+	slave->mtd.oobavail = 0;
+	slave->mtd.subpage_sft = master->subpage_sft;
+
+	slave->mtd.name = name;
+	slave->mtd.owner = master->owner;
+	slave->mtd.dev.of_node = part->node;
+
+	if (master->_read_user_prot_reg)
+		slave->mtd._read_user_prot_reg = part_read_user_prot_reg;
+	if (master->_read_fact_prot_reg)
+		slave->mtd._read_fact_prot_reg = part_read_fact_prot_reg;
+	if (master->_write_user_prot_reg)
+		slave->mtd._write_user_prot_reg = part_write_user_prot_reg;
+	if (master->_lock_user_prot_reg)
+		slave->mtd._lock_user_prot_reg = part_lock_user_prot_reg;
+	if (master->_get_user_prot_info)
+		slave->mtd._get_user_prot_info = part_get_user_prot_info;
+	if (master->_get_fact_prot_info)
+		slave->mtd._get_fact_prot_info = part_get_fact_prot_info;
+
+	if (!partno && !master->dev.class && master->_suspend &&
+	    master->_resume) {
+		slave->mtd._suspend = part_suspend;
+		slave->mtd._resume = part_resume;
+	}
+
+	slave->master = master;
+	slave->offset = part->offset;
+	slave->mtd.size = part->size;
+
+	pr_notice("0x%012llx-0x%012llx : \"%s\"\n",
+		  (unsigned long long)slave->offset,
+		  (unsigned long long)(slave->offset + slave->mtd.size),
+		  slave->mtd.name);
+
+	return slave;
+}
+#endif
+
 static struct mtd_part *allocate_partition(struct mtd_info *master,
 			const struct mtd_partition *part, int partno,
 			uint64_t cur_offset)
@@ -682,6 +754,46 @@ int add_mtd_partitions(struct mtd_info *master,
 	return 0;
 }
 
+#ifdef CONFIG_MTD_NVMEM
+int add_otp_regions(struct mtd_info *master,
+		    const struct mtd_partition *parts, int nbparts)
+{
+	struct mtd_part *slave;
+	u64 cur_offset = 0;
+	int i;
+
+	pr_notice("Creating %d MTD OTP region(s) on \"%s\":\n", nbparts,
+		  master->name);
+
+	for (i = 0; i < nbparts; i++) {
+		slave = allocate_otp_region(master, parts + i, i, cur_offset);
+		if (IS_ERR(slave)) {
+			del_mtd_partitions(master);
+			return PTR_ERR(slave);
+		}
+
+		mutex_lock(&mtd_partitions_mutex);
+		list_add(&slave->list, &mtd_partitions);
+		mutex_unlock(&mtd_partitions_mutex);
+
+		add_mtd_device(&slave->mtd);
+		mtd_add_partition_attrs(slave);
+
+		slave->nv = mtd_otp_nvmem_register(&slave->mtd);
+
+		cur_offset = slave->offset + slave->mtd.size;
+	}
+
+	return 0;
+}
+#else
+int add_otp_regions(struct mtd_info *master,
+		    const struct mtd_partition *parts, int nbparts)
+{
+	return 0;
+}
+#endif
+
 static DEFINE_SPINLOCK(part_parser_lock);
 static LIST_HEAD(part_parsers);
 
diff --git a/drivers/mtd/ofpart.c b/drivers/mtd/ofpart.c
index 9a68bff..3a0d14d 100644
--- a/drivers/mtd/ofpart.c
+++ b/drivers/mtd/ofpart.c
@@ -145,6 +145,106 @@ static struct mtd_part_parser ofpart_parser = {
 	.name = "ofpart",
 };
 
+static int parse_ofotp_partitions(struct mtd_info *master,
+				  const struct mtd_partition **pparts,
+				  struct mtd_part_parser_data *data)
+{
+	struct mtd_partition *parts;
+	struct device_node *mtd_node;
+	struct device_node *ofpart_node;
+	const char *partname;
+	struct device_node *pp;
+	int nr_parts, i, ret = 0;
+
+	/* Pull of_node from the master device node */
+	mtd_node = mtd_get_of_node(master);
+	if (!mtd_node)
+		return 0;
+
+	ofpart_node = of_get_child_by_name(mtd_node, "otp-partitions");
+	if (!ofpart_node) {
+		/*
+		 * We might get here even when ofpart isn't used at all (e.g.,
+		 * when using another parser), so don't be louder than
+		 * KERN_DEBUG
+		 */
+		pr_debug("%s: 'otp-partitions' subnode not found on %s\n",
+			 master->name, mtd_node->full_name);
+		return 0;
+	} else if (!of_device_is_compatible(ofpart_node, "fixed-partitions")) {
+		/* The 'partitions' subnode might be used by another parser */
+		return 0;
+	}
+
+	/* First count the subnodes */
+	nr_parts = 0;
+	for_each_child_of_node(ofpart_node,  pp)
+		nr_parts++;
+
+	if (nr_parts == 0)
+		return 0;
+
+	parts = kcalloc(nr_parts, sizeof(*parts), GFP_KERNEL);
+	if (!parts)
+		return -ENOMEM;
+
+	i = 0;
+	for_each_child_of_node(ofpart_node,  pp) {
+		const __be32 *reg;
+		int len;
+		int a_cells, s_cells;
+
+		reg = of_get_property(pp, "reg", &len);
+		if (!reg) {
+			nr_parts--;
+			continue;
+		}
+
+		a_cells = of_n_addr_cells(pp);
+		s_cells = of_n_size_cells(pp);
+		if (len / 4 != a_cells + s_cells) {
+			pr_debug("%s: ofpart partition %s (%s) error parsing reg property.\n",
+				 master->name, pp->full_name,
+				 mtd_node->full_name);
+			goto ofpart_fail;
+		}
+
+		parts[i].offset = of_read_number(reg, a_cells);
+		parts[i].size = of_read_number(reg + a_cells, s_cells);
+		parts[i].node = pp;
+
+		partname = of_get_property(pp, "label", &len);
+		if (!partname)
+			partname = of_get_property(pp, "name", &len);
+		parts[i].name = partname;
+
+		if (of_get_property(pp, "read-only", &len))
+			parts[i].mask_flags |= MTD_WRITEABLE;
+
+		i++;
+	}
+
+	if (!nr_parts)
+		goto ofpart_none;
+
+	*pparts = parts;
+	return nr_parts;
+
+ofpart_fail:
+	pr_err("%s: error parsing ofotp partition %s (%s)\n",
+	       master->name, pp->full_name, mtd_node->full_name);
+	ret = -EINVAL;
+ofpart_none:
+	of_node_put(pp);
+	kfree(parts);
+	return ret;
+}
+
+static struct mtd_part_parser ofotp_parser = {
+	.parse_fn = parse_ofotp_partitions,
+	.name = "ofotp",
+};
+
 static int parse_ofoldpart_partitions(struct mtd_info *master,
 				      const struct mtd_partition **pparts,
 				      struct mtd_part_parser_data *data)
@@ -210,6 +310,7 @@ static int __init ofpart_parser_init(void)
 {
 	register_mtd_parser(&ofpart_parser);
 	register_mtd_parser(&ofoldpart_parser);
+	register_mtd_parser(&ofotp_parser);
 	return 0;
 }
 
@@ -217,6 +318,7 @@ static void __exit ofpart_parser_exit(void)
 {
 	deregister_mtd_parser(&ofpart_parser);
 	deregister_mtd_parser(&ofoldpart_parser);
+	deregister_mtd_parser(&ofotp_parser);
 }
 
 module_init(ofpart_parser_init);
-- 
2.5.5

Re: [RFC/PATCH 1/2] doc: bindings: Add bindings documentation for mtd otp nvmem

[Boris Brezillon]

Hi Moritz,

On Wed, 25 May 2016 14:26:46 -0700
Moritz Fischer <moritz.fischer@ettus.com> wrote:

> This commit adds documentation describing the bindings for
> exposing mtd flash otp regions as nvmem providers via devicetree.
> 
> Signed-off-by: Moritz Fischer <moritz.fischer@ettus.com>
> ---
>  .../devicetree/bindings/mtd/otp-nvmem.txt          | 62 ++++++++++++++++++++++
>  1 file changed, 62 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mtd/otp-nvmem.txt
> 
> diff --git a/Documentation/devicetree/bindings/mtd/otp-nvmem.txt b/Documentation/devicetree/bindings/mtd/otp-nvmem.txt
> new file mode 100644
> index 0000000..a83a7da
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/otp-nvmem.txt
> @@ -0,0 +1,62 @@
> +Representing OTP regions in devicetree
> +
> +OTP regions can be represented by sub-nodes of an mtd device.
> +
> +The partition table should be a subnode of the mtd node and should be names
> +'otp-partitions'. This node should have the following property:
> +
> +- compatible: (required) must be 'fixed-partitions'
> +
> +OTP regions are then defined in subnodes of the partitions node.
> +
> +Required properties for OTP regions:
> +- reg: The region's offset and size within the mtd device
> +
> +Optional properties:
> +- label: The label / name for this region. If ommited, the label is taken
> +  from the node name (excluding the unit address).
> +
> +Example:
> +
> +flash@0 {
> +	partitions {
> +		compatible = "fixed-partitions";
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +
> +		partition@0 {
> +			label = "uboot-spl";
> +			reg = <0x0 0xe0000>;
> +		};
> +		partition@1 {
> +			label = "uboot-env";
> +			reg = <0xe0000 0x20000>;
> +		};
> +		partition@2 {
> +			label = "uboot";
> +			reg = <0x100000 0x100000>;
> +		};
> +	};
> +
> +	otp-partitions {
> +		compatible = "fixed-partitions";
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +
> +		region@0 {
> +			label = "factory-data";
> +			reg = <0x0 0x40>;
> +
> +			#address-cells = <0x1>;
> +			#size-cells = <0x1>;
> +
> +			product: nvmem@0 {
> +				reg = <0x0 0x2>;
> +			};
> +
> +			revision: nvmem@3 {
> +				reg = <0x3 0x2>;
> +			};
> +		};
> +	};
> +}

I think the MTD partition -> nvmem connection could benefit to non-OTP
partitions too.

So, how about defining the nvmem regions under the partition nodes,
like that:

flash@0 {
	partitions {
		compatible = "fixed-partitions";
		#address-cells = <1>;
		#size-cells = <1>;

		partition@0 {
			label = "uboot-spl";
			reg = <0x0 0xe0000>;
		};

		/* ... */

		partition@X{
			label = "factory-data-part";
			reg = <0x200000 0x100000>;
			#address-cells = <1>;
			#size-cells = <1>;

			product: nvmem@0 {
				reg = <0x0 0x2>;
			};

			revision: nvmem@3 {
				reg = <0x3 0x2>;
			};
		};
	};

	otp-partitions {
		compatible = "fixed-partitions";
		#address-cells = <1>;
		#size-cells = <1>;

		partition@X{
			label = "factory-data-part";
			reg = <0x0 0x40>;
			#address-cells = <1>;
			#size-cells = <1>;

			product: nvmem@0 {
				reg = <0x0 0x2>;
			};

			revision: nvmem@3 {
				reg = <0x3 0x2>;
			};
		};
	};
};

I know this requires changing the implementation to select the
appropriate nvmem wrapper to use depending on whether we're interfacing
with an OTP area or a regular one, but that should be doable.

Regards,

Boris

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [RFC/PATCH 1/2] doc: bindings: Add bindings documentation for mtd otp nvmem

[Moritz Fischer]

Hi Boris,

On Thu, May 26, 2016 at 1:04 AM, Boris Brezillon
<boris.brezillon@free-electrons.com> wrote:

> I think the MTD partition -> nvmem connection could benefit to non-OTP
> partitions too.

Yeah, I thought about that, too. Would you use the _read, and _write
callbacks in that case?

> So, how about defining the nvmem regions under the partition nodes,
> like that:
>
> flash@0 {
>         partitions {
>                 compatible = "fixed-partitions";
>                 #address-cells = <1>;
>                 #size-cells = <1>;
>
>                 partition@0 {
>                         label = "uboot-spl";
>                         reg = <0x0 0xe0000>;
>                 };
>
>                 /* ... */
>
>                 partition@X{
>                         label = "factory-data-part";
>                         reg = <0x200000 0x100000>;
>                         #address-cells = <1>;
>                         #size-cells = <1>;
>
>                         product: nvmem@0 {
>                                 reg = <0x0 0x2>;
>                         };
>
>                         revision: nvmem@3 {
>                                 reg = <0x3 0x2>;
>                         };
>                 };
>         };
>
>         otp-partitions {
>                 compatible = "fixed-partitions";
>                 #address-cells = <1>;
>                 #size-cells = <1>;
>
>                 partition@X{
>                         label = "factory-data-part";
>                         reg = <0x0 0x40>;
>                         #address-cells = <1>;
>                         #size-cells = <1>;
>
>                         product: nvmem@0 {
>                                 reg = <0x0 0x2>;
>                         };
>
>                         revision: nvmem@3 {
>                                 reg = <0x3 0x2>;
>                         };
>                 };
>         };
> };
>
> I know this requires changing the implementation to select the
> appropriate nvmem wrapper to use depending on whether we're interfacing
> with an OTP area or a regular one, but that should be doable.

The implementation still needs work anyways, so I might as well add
this to my list ...
Would you do the nvmem mapping always, or conditionalize on a flag in
the dt node like 'nvmem-export'?

Thanks for the feedback,

Moritz

Re: [RFC/PATCH 1/2] doc: bindings: Add bindings documentation for mtd otp nvmem

[Boris Brezillon]

Hi Moritz,

On Thu, 26 May 2016 10:28:48 -0700
Moritz Fischer <moritz.fischer@ettus.com> wrote:

> Hi Boris,
> 
> On Thu, May 26, 2016 at 1:04 AM, Boris Brezillon
> <boris.brezillon@free-electrons.com> wrote:
> 
> > I think the MTD partition -> nvmem connection could benefit to non-OTP
> > partitions too.  
> 
> Yeah, I thought about that, too. Would you use the _read, and _write
> callbacks in that case?

Yep, this means you'll need a flag to decide what kind of partition is
exposed (OTP or normal), and then adjust the partition
->_read()/->_write() implementations.

Actually, I'm not sure what will happen if you really expose OTP
partitions, since these ones shouldn't implement norma
->_read()/_write(), or they should wrap those calls around their
master ->_read/write_prot_xx().
I must admit I haven't looked at patch 2 yet, so maybe that's what
you're doing.

> 
> > So, how about defining the nvmem regions under the partition nodes,
> > like that:
> >
> > flash@0 {
> >         partitions {
> >                 compatible = "fixed-partitions";
> >                 #address-cells = <1>;
> >                 #size-cells = <1>;
> >
> >                 partition@0 {
> >                         label = "uboot-spl";
> >                         reg = <0x0 0xe0000>;
> >                 };
> >
> >                 /* ... */
> >
> >                 partition@X{
> >                         label = "factory-data-part";
> >                         reg = <0x200000 0x100000>;
> >                         #address-cells = <1>;
> >                         #size-cells = <1>;
> >
> >                         product: nvmem@0 {
> >                                 reg = <0x0 0x2>;
> >                         };
> >
> >                         revision: nvmem@3 {
> >                                 reg = <0x3 0x2>;
> >                         };
> >                 };
> >         };
> >
> >         otp-partitions {
> >                 compatible = "fixed-partitions";
> >                 #address-cells = <1>;
> >                 #size-cells = <1>;
> >
> >                 partition@X{
> >                         label = "factory-data-part";
> >                         reg = <0x0 0x40>;
> >                         #address-cells = <1>;
> >                         #size-cells = <1>;
> >
> >                         product: nvmem@0 {
> >                                 reg = <0x0 0x2>;
> >                         };
> >
> >                         revision: nvmem@3 {
> >                                 reg = <0x3 0x2>;
> >                         };
> >                 };
> >         };
> > };
> >
> > I know this requires changing the implementation to select the
> > appropriate nvmem wrapper to use depending on whether we're interfacing
> > with an OTP area or a regular one, but that should be doable.  
> 
> The implementation still needs work anyways, so I might as well add
> this to my list ...
> Would you do the nvmem mapping always, or conditionalize on a flag in
> the dt node like 'nvmem-export'?

Well, I asked myself the same question when I proposed the binding. You
actually don't need the property since you can detect the presence of
the #address/size-cells properties or the presence of child nodes, but
maybe it's safer to make it dependent on an extra property (or use
compatible = "mtd-to-nvmem").
I honestly don't know what's the best solution. Rob, any suggestion?

Regards,

Boris

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com