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


Groups > linux.kernel > #1346758 > unrolled thread

[RESEND RFC 0/3] Proposed extensions for NVMEM

Started byAndrey Smirnov <andrew.smirnov@gmail.com>
First post2016-03-01 18:00 +0100
Last post2016-03-02 18:30 +0100
Articles 4 — 2 participants

Back to article view | Back to linux.kernel


Contents

  [RESEND RFC 0/3] Proposed extensions for NVMEM Andrey Smirnov <andrew.smirnov@gmail.com> - 2016-03-01 18:00 +0100
    [RESEND RFC 2/3] nvmem: Add 'nvmem-blob' driver Andrey Smirnov <andrew.smirnov@gmail.com> - 2016-03-01 18:10 +0100
      Re: [RESEND RFC 2/3] nvmem: Add 'nvmem-blob' driver Srinivas Kandagatla <srinivas.kandagatla@linaro.org> - 2016-03-02 15:00 +0100
        Re: [RESEND RFC 2/3] nvmem: Add 'nvmem-blob' driver Andrey Smirnov <andrew.smirnov@gmail.com> - 2016-03-02 18:30 +0100

#1346758 — [RESEND RFC 0/3] Proposed extensions for NVMEM

FromAndrey Smirnov <andrew.smirnov@gmail.com>
Date2016-03-01 18:00 +0100
Subject[RESEND RFC 0/3] Proposed extensions for NVMEM
Message-ID<r7VOq-3gD-5@gated-at.bofh.it>
Hello,

This RFC introduces two new drivers to NVMEM subsytem. First driver,
'nvmem-blob', serves the purpose of exposing data, embedded in DTB,
via NVMEM consumer API. Second, 'nvmem-composite', allows the user to
combin a number of NVMEM cells (or parts of them) into a single
continuos "blob" of data presented to the rest of the system as a
regular NVMEM device.

The intent of this RFC is to solicit feedback about the approach,
binding or if these features should be accepted upstream in general.

I originally proposed this extentions as a part of Barebox project (Barebox
borrows a lot of concepts from Linux kernel and adopting NVMEM
subsystem was being discussed), in order to facilitate usage of NVMEM
to initialize source of MAC address for a paticular Ethernet adapter
(Barebox would read that value and fixup DT blob with appropriate
value as a part of booting Linux). However the drivers should be
generic enough to not be tied to that case.

Please bear in mind the the code included is a very rough draft and a
lot of error checking/handling code in is was stubbed out.

Any feedback is welcome.

Thank you,
Andrey Smirnov

Andrey Smirnov (3):
  nvmem: Add 'of_nvmem_cell_from_device_node()'
  nvmem: Add 'nvmem-blob' driver
  nvmem: Add 'nvmem-composite' driver

 Documentation/devicetree/bindings/nvmem/blob.txt   |  35 ++++
 .../devicetree/bindings/nvmem/composite.txt        |  44 ++++
 drivers/nvmem/Makefile                             |   2 +
 drivers/nvmem/blob.c                               | 132 ++++++++++++
 drivers/nvmem/composite.c                          | 228 +++++++++++++++++++++
 drivers/nvmem/core.c                               |  44 ++--
 include/linux/nvmem-consumer.h                     |   7 +
 7 files changed, 479 insertions(+), 13 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/nvmem/blob.txt
 create mode 100644 Documentation/devicetree/bindings/nvmem/composite.txt
 create mode 100644 drivers/nvmem/blob.c
 create mode 100644 drivers/nvmem/composite.c

-- 
2.5.0

[toc] | [next] | [standalone]


#1346766 — [RESEND RFC 2/3] nvmem: Add 'nvmem-blob' driver

FromAndrey Smirnov <andrew.smirnov@gmail.com>
Date2016-03-01 18:10 +0100
Subject[RESEND RFC 2/3] nvmem: Add 'nvmem-blob' driver
Message-ID<r7VY6-3zV-23@gated-at.bofh.it>
In reply to#1346758
Add 'nvmem-blob' driver, which allows to access device tree embedded
data via NVMEM subsystem API.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 Documentation/devicetree/bindings/nvmem/blob.txt |  35 ++++++
 drivers/nvmem/Makefile                           |   1 +
 drivers/nvmem/blob.c                             | 132 +++++++++++++++++++++++
 3 files changed, 168 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/nvmem/blob.txt
 create mode 100644 drivers/nvmem/blob.c

diff --git a/Documentation/devicetree/bindings/nvmem/blob.txt b/Documentation/devicetree/bindings/nvmem/blob.txt
new file mode 100644
index 0000000..b299576
--- /dev/null
+++ b/Documentation/devicetree/bindings/nvmem/blob.txt
@@ -0,0 +1,35 @@
+= Deviece Tree Embedded Blob As NVMEM Device =
+
+This binding is designed to provide a way for a developer to reference
+data, built-in into device tree blob file, as NVMEM provider and acccess
+certain portions of that data as NVMEM cells using NVMEM consumer API.
+
+Required properties:
+- compatible: should be "nvmem-blob"
+- data: specifies data contained in nvmem device
+
+= Data cells =
+Are child nodes of nvmem-composite, bindings of which as described in
+bindings/nvmem/nvmem.txt
+
+Example:
+
+	a-blob {
+		compatible = "nvmem-blob";
+		data = [aa bb cc dd ee];
+
+		cell1: cell@0 {
+			reg = <0 5>;
+		};
+	};
+
+= Data consumers =
+Are device nodes which consume nvmem data cells.
+
+Example:
+
+	a-node {
+		...
+		nvmem-cells = <&cell1>;
+		nvmem-cell-names = "some-data";
+	};
diff --git a/drivers/nvmem/Makefile b/drivers/nvmem/Makefile
index 95dde3f..1a1adba 100644
--- a/drivers/nvmem/Makefile
+++ b/drivers/nvmem/Makefile
@@ -18,3 +18,4 @@ obj-$(CONFIG_NVMEM_SUNXI_SID)	+= nvmem_sunxi_sid.o
 nvmem_sunxi_sid-y		:= sunxi_sid.o
 obj-$(CONFIG_NVMEM_VF610_OCOTP)	+= nvmem-vf610-ocotp.o
 nvmem-vf610-ocotp-y		:= vf610-ocotp.o
+obj-y += blob.o
diff --git a/drivers/nvmem/blob.c b/drivers/nvmem/blob.c
new file mode 100644
index 0000000..3f2296b
--- /dev/null
+++ b/drivers/nvmem/blob.c
@@ -0,0 +1,132 @@
+#define DEBUG 1
+
+#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>
+
+struct nvmem_blob {
+	const u8 *data;
+	size_t data_size;
+};
+
+static int nvmem_blob_write(void *context, const void *data,
+				size_t count)
+{
+	return -ENOTSUPP;
+}
+
+static int nvmem_blob_read(void *context,
+			   const void *reg, size_t reg_size,
+			   void *val, size_t val_size)
+{
+	struct nvmem_blob *nblob = context;
+	const unsigned int offset = *(u32 *)reg;
+	
+	memcpy(val, nblob->data + offset,
+	       min(val_size, nblob->data_size - offset));
+	return 0;
+}
+
+static const struct regmap_bus nvmem_blob_regmap_bus = {
+	.write = nvmem_blob_write,
+	.read  = nvmem_blob_read,
+};
+
+
+static int nvmem_blob_validate_dt(struct device_node *np)
+{
+	return 0;
+}
+
+static int nvmem_blob_probe(struct platform_device *pdev)
+{
+	int ret;
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	struct regmap *map;
+	struct nvmem_device *nvmem;
+	struct nvmem_blob *nblob;
+	struct property *pp;
+	struct nvmem_config  nv_cnf = {0};
+	struct regmap_config rm_cnf = {0};
+
+	ret = nvmem_blob_validate_dt(np);
+	if (ret < 0) {
+		dev_dbg(dev, "Device tree validation failed\n");
+		return ret;
+	}
+
+	nblob = devm_kzalloc(dev, sizeof(*nblob), GFP_KERNEL);
+	if (!nblob) {
+		dev_dbg(dev, "Not enough memory to allocate a blob\n");
+		return -ENOMEM;
+	}
+
+	pp = of_find_property(np, "data", NULL);
+	BUG_ON(!pp);
+	
+	nblob->data = pp->value;
+	nblob->data_size = pp->length;
+
+	rm_cnf.reg_bits = 32;
+	rm_cnf.val_bits = 8;
+	rm_cnf.reg_stride = 1;
+	rm_cnf.name = "nvmem-blob";
+	rm_cnf.max_register = nblob->data_size - 1;
+
+	map = devm_regmap_init(dev,
+			       &nvmem_blob_regmap_bus,
+			       nblob,
+			       &rm_cnf);
+	if (IS_ERR(map)) {
+		dev_dbg(dev, "Failed to initilize regmap\n");
+		return PTR_ERR(map);
+	}
+
+	nv_cnf.name = "nvmem-blob";
+	nv_cnf.read_only = true;
+	nv_cnf.dev = dev;
+	nv_cnf.owner = THIS_MODULE;
+
+	nvmem = nvmem_register(&nv_cnf);
+	if (IS_ERR(nvmem)) {
+		dev_dbg(dev, "Filed to register nvmem device\n");
+		return PTR_ERR(nvmem);
+	}
+
+	platform_set_drvdata(pdev, nblob);
+	return 0;
+}
+
+static int nvmem_blob_remove(struct platform_device *pdev)
+{
+	struct nvmem_device *nvmem = platform_get_drvdata(pdev);
+
+	return nvmem_unregister(nvmem);
+}
+
+static const struct of_device_id nvmem_blob_dt_ids[] = {
+	{ .compatible = "nvmem-blob", },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, nvmem_blob_dt_ids);
+
+static struct platform_driver nvmem_blob_driver = {
+	.probe	= nvmem_blob_probe,
+	.remove	= nvmem_blob_remove,
+	.driver = {
+		.name	= "nvmem-blob",
+		.of_match_table = nvmem_blob_dt_ids,
+	},
+};
+module_platform_driver(nvmem_blob_driver);
+
+MODULE_AUTHOR("Andrey Smirnov <andrew.smirnov@gmail.com>");
+MODULE_DESCRIPTION("FIXME");
+MODULE_LICENSE("GPL v2");
-- 
2.5.0

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


#1348041 — Re: [RESEND RFC 2/3] nvmem: Add 'nvmem-blob' driver

FromSrinivas Kandagatla <srinivas.kandagatla@linaro.org>
Date2016-03-02 15:00 +0100
SubjectRe: [RESEND RFC 2/3] nvmem: Add 'nvmem-blob' driver
Message-ID<r8ftM-8on-3@gated-at.bofh.it>
In reply to#1346766

On 01/03/16 16:59, Andrey Smirnov wrote:
> Add 'nvmem-blob' driver, which allows to access device tree embedded
> data via NVMEM subsystem API.

Patch itself looks simple. Before we review it further could you provide 
more details on the exact usecase or some background of this.


Incase you resend the patches, could you split the dt bindings into 
separate patch and add dt guys in to the loop, They would have different 
opinion on adding dummy stuff into DT itself.

thanks,
srini
>
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> ---
>   Documentation/devicetree/bindings/nvmem/blob.txt |  35 ++++++
>   drivers/nvmem/Makefile                           |   1 +
>   drivers/nvmem/blob.c                             | 132 +++++++++++++++++++++++
>   3 files changed, 168 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/nvmem/blob.txt
>   create mode 100644 drivers/nvmem/blob.c
>
> diff --git a/Documentation/devicetree/bindings/nvmem/blob.txt b/Documentation/devicetree/bindings/nvmem/blob.txt
> new file mode 100644
> index 0000000..b299576
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/nvmem/blob.txt
> @@ -0,0 +1,35 @@
> += Deviece Tree Embedded Blob As NVMEM Device =
> +
> +This binding is designed to provide a way for a developer to reference
> +data, built-in into device tree blob file, as NVMEM provider and acccess
> +certain portions of that data as NVMEM cells using NVMEM consumer API.
> +
> +Required properties:
> +- compatible: should be "nvmem-blob"
> +- data: specifies data contained in nvmem device
> +
> += Data cells =
> +Are child nodes of nvmem-composite, bindings of which as described in
> +bindings/nvmem/nvmem.txt
> +
> +Example:
> +
> +	a-blob {
> +		compatible = "nvmem-blob";
> +		data = [aa bb cc dd ee];
> +
> +		cell1: cell@0 {
> +			reg = <0 5>;
> +		};
> +	};
> +
> += Data consumers =
> +Are device nodes which consume nvmem data cells.
> +
> +Example:
> +
> +	a-node {
> +		...
> +		nvmem-cells = <&cell1>;
> +		nvmem-cell-names = "some-data";
> +	};
> diff --git a/drivers/nvmem/Makefile b/drivers/nvmem/Makefile
> index 95dde3f..1a1adba 100644
> --- a/drivers/nvmem/Makefile
> +++ b/drivers/nvmem/Makefile
> @@ -18,3 +18,4 @@ obj-$(CONFIG_NVMEM_SUNXI_SID)	+= nvmem_sunxi_sid.o
>   nvmem_sunxi_sid-y		:= sunxi_sid.o
>   obj-$(CONFIG_NVMEM_VF610_OCOTP)	+= nvmem-vf610-ocotp.o
>   nvmem-vf610-ocotp-y		:= vf610-ocotp.o
> +obj-y += blob.o
> diff --git a/drivers/nvmem/blob.c b/drivers/nvmem/blob.c
> new file mode 100644
> index 0000000..3f2296b
> --- /dev/null
> +++ b/drivers/nvmem/blob.c
> @@ -0,0 +1,132 @@
> +#define DEBUG 1
> +
> +#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>
> +
> +struct nvmem_blob {
> +	const u8 *data;
> +	size_t data_size;
> +};
> +
> +static int nvmem_blob_write(void *context, const void *data,
> +				size_t count)
> +{
> +	return -ENOTSUPP;
> +}
> +
> +static int nvmem_blob_read(void *context,
> +			   const void *reg, size_t reg_size,
> +			   void *val, size_t val_size)
> +{
> +	struct nvmem_blob *nblob = context;
> +	const unsigned int offset = *(u32 *)reg;
> +	
> +	memcpy(val, nblob->data + offset,
> +	       min(val_size, nblob->data_size - offset));
> +	return 0;
> +}
> +
> +static const struct regmap_bus nvmem_blob_regmap_bus = {
> +	.write = nvmem_blob_write,
> +	.read  = nvmem_blob_read,
> +};
> +
> +
> +static int nvmem_blob_validate_dt(struct device_node *np)
> +{
> +	return 0;
> +}
> +
> +static int nvmem_blob_probe(struct platform_device *pdev)
> +{
> +	int ret;
> +	struct device *dev = &pdev->dev;
> +	struct device_node *np = dev->of_node;
> +	struct regmap *map;
> +	struct nvmem_device *nvmem;
> +	struct nvmem_blob *nblob;
> +	struct property *pp;
> +	struct nvmem_config  nv_cnf = {0};
> +	struct regmap_config rm_cnf = {0};
> +
> +	ret = nvmem_blob_validate_dt(np);
> +	if (ret < 0) {
> +		dev_dbg(dev, "Device tree validation failed\n");
> +		return ret;
> +	}
> +
> +	nblob = devm_kzalloc(dev, sizeof(*nblob), GFP_KERNEL);
> +	if (!nblob) {
> +		dev_dbg(dev, "Not enough memory to allocate a blob\n");
> +		return -ENOMEM;
> +	}
> +
> +	pp = of_find_property(np, "data", NULL);
> +	BUG_ON(!pp);
> +	
> +	nblob->data = pp->value;
> +	nblob->data_size = pp->length;
> +
> +	rm_cnf.reg_bits = 32;
> +	rm_cnf.val_bits = 8;
> +	rm_cnf.reg_stride = 1;
> +	rm_cnf.name = "nvmem-blob";
> +	rm_cnf.max_register = nblob->data_size - 1;
> +
> +	map = devm_regmap_init(dev,
> +			       &nvmem_blob_regmap_bus,
> +			       nblob,
> +			       &rm_cnf);
> +	if (IS_ERR(map)) {
> +		dev_dbg(dev, "Failed to initilize regmap\n");
> +		return PTR_ERR(map);
> +	}
> +
> +	nv_cnf.name = "nvmem-blob";
> +	nv_cnf.read_only = true;
> +	nv_cnf.dev = dev;
> +	nv_cnf.owner = THIS_MODULE;
> +
> +	nvmem = nvmem_register(&nv_cnf);
> +	if (IS_ERR(nvmem)) {
> +		dev_dbg(dev, "Filed to register nvmem device\n");
> +		return PTR_ERR(nvmem);
> +	}
> +
> +	platform_set_drvdata(pdev, nblob);
> +	return 0;
> +}
> +
> +static int nvmem_blob_remove(struct platform_device *pdev)
> +{
> +	struct nvmem_device *nvmem = platform_get_drvdata(pdev);
> +
> +	return nvmem_unregister(nvmem);
> +}
> +
> +static const struct of_device_id nvmem_blob_dt_ids[] = {
> +	{ .compatible = "nvmem-blob", },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, nvmem_blob_dt_ids);
> +
> +static struct platform_driver nvmem_blob_driver = {
> +	.probe	= nvmem_blob_probe,
> +	.remove	= nvmem_blob_remove,
> +	.driver = {
> +		.name	= "nvmem-blob",
> +		.of_match_table = nvmem_blob_dt_ids,
> +	},
> +};
> +module_platform_driver(nvmem_blob_driver);
> +
> +MODULE_AUTHOR("Andrey Smirnov <andrew.smirnov@gmail.com>");
> +MODULE_DESCRIPTION("FIXME");
> +MODULE_LICENSE("GPL v2");
>

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


#1348349 — Re: [RESEND RFC 2/3] nvmem: Add 'nvmem-blob' driver

FromAndrey Smirnov <andrew.smirnov@gmail.com>
Date2016-03-02 18:30 +0100
SubjectRe: [RESEND RFC 2/3] nvmem: Add 'nvmem-blob' driver
Message-ID<r8iL1-2in-31@gated-at.bofh.it>
In reply to#1348041
On Wed, Mar 2, 2016 at 5:58 AM, Srinivas Kandagatla
<srinivas.kandagatla@linaro.org> wrote:
>
>
> On 01/03/16 16:59, Andrey Smirnov wrote:
>>
>> Add 'nvmem-blob' driver, which allows to access device tree embedded
>> data via NVMEM subsystem API.
>
>
> Patch itself looks simple. Before we review it further could you provide
> more details on the exact usecase or some background of this.

The discussion on this topic originated on mailing list of Barebox
project(which borrows very heavily from Linux designs). Barebox
operates on two device tree blobs, one is used for its internal
initialization, whereas second one is passed to Linux kernel when
booting it. The problem I was trying to solve was to make possible to
specify in the first DT blob what data would be used for MAC address
fixup of the second DT blob(the one passed to Linux).

My first approach was to implement a very limited DT code, however in
discussing it the consensus was that porting 'nvmem' subsystem from
the kernel and using for the same purpose would be a better approach.
First pass adoption of that subsystem revealed that there were two
use-cases that current design didn't allow us to handle:

- Depending on the version i.MX SoC MAC address data stored in ROM
would have different layout so as a possible solution to that I
implemented "composite" driver(patch #3)

- On i.MX28, part of the MAC address is hard-coded in
arch/arm/mach-mxs/mach-mxs.c and only a portion of it is read from
ROM, this patch in combination with the aforementioned one should
allow us to encode all needed info in DT.

Ideally, since all of the above is as applicable to Linux as it is to
Barebox it would be good for BB not to invent its own custom 'nvmem'
flavor, so hence me trying to start a conversation about adding this
upstream.

Hope this clarifies things a bit.

>
>
> Incase you resend the patches, could you split the dt bindings into separate
> patch and add dt guys in to the loop, They would have different opinion on
> adding dummy stuff into DT itself.

Will do.


Thanks,
Andrey

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web