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

[Srinivas Kandagatla]

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@xxxxxxxxx>
> ---
>   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@xxxxxxxxx>");
> +MODULE_DESCRIPTION("FIXME");
> +MODULE_LICENSE("GPL v2");