Thankyou all for providing inputs and comments on previous versions of this patchset.
Here is the v5 of the patchset addressing all the issues raised as
part of previous versions review.
This patchset adds a new simple NVMEM framework to kernel.
Up until now, NVMEM drivers were stored in drivers/misc, where they all had to
duplicate pretty much the same code to register a sysfs file, allow in-kernel
users to access the content of the devices they were driving, etc.
This was also a problem as far as other in-kernel users were involved, since
the solutions used were pretty much different from on driver to another, there
was a rather big abstraction leak.
Introduction of this framework aims at solving this. It also introduces DT
representation for consumer devices to go get the data they require (MAC
Addresses, SoC/Revision ID, part numbers, and so on) from the NVMEMs.
After learning few things about QCOM qfprom and other eeprom/efuses, which
has packed fields at bit level. Which makes it important to add support to
such memories. This version adds support to this type of non volatile
memories by adding support to bit level nvmem-cells.
Having regmap interface to this framework would give much better
abstraction for nvmems on different buses.
patch 1-2 Introduces two regmap helper functions.
patch 3-6 Introduces the NVMEM framework.
Patch 7 Adds helper functions for nvmems based on mmio.
Patch 8 migrates an existing driver to nvmem framework.
Patch 9-10 Adds Qualcomm specific qfprom driver.
Patch 11 adds entry in MAINTAINERS.
Its also possible to migrate other nvmem drivers to this framework.
Providers APIs:
nvmem_register/unregister();
Consumers APIs:
Cell based apis for both DT/Non-DT:
nvmem_cell_get()/nvmem_cell_put();
nvmem_cell_read()/nvmem_cell_write();
Raw byte access apis for both DT/non-DT.
nvmem_device_get()/nvmem_device_put()
nvmem_device_read()/nvmem_device_write();
nvmem_device_cell_read()/nvmem_device_cell_write();
Device Tree:
/* Provider */
qfprom: qfprom@00700000 {
...
/* Data cells */
tsens_calibration: calib@404 {
reg = <0x404 0x10>;
};
tsens_calibration_bckp: calib_bckp@504 {
reg = <0x504 0x11>;
bit-offset = 6;
nbits = 128;
};
pvs_version: pvs-version@6 {
reg = <0x6 0x2>
bit-offset = 7;
nbits = 2;
};
speed_bin: speed-bin@c{
reg = <0xc 0x1>;
bit-offset = 2;
nbits = 3;
};
...
};
userspace interface: binary file in /sys/class/nvmem/*/nvmem
ex:
hexdump /sys/class/nvmem/qfprom0/nvmem
0000000 0000 0000 0000 0000 0000 0000 0000 0000
*
00000a0 db10 2240 0000 e000 0c00 0c00 0000 0c00
0000000 0000 0000 0000 0000 0000 0000 0000 0000
...
*
0001000
Changes since v4(https://lkml.org/lkml/2015/3/30/725)
* rename eeprom to nvmem suggested by Matt Porter
* added bit level support to nvmem cells, if not framework is
not usable for qcom platforms.
* removed ternary operator shortcut suggested by Mark B.
* unified consumer/provider apis for both DT and non-DT.
* added name support for cell.
* added bit level bindings.
* added read-only support suggested by Matt Porter and others.
* added new nvmem_device based consumer apis.
Changes since v3(https://lkml.org/lkml/2015/3/24/1066)
* simplified logic in bin_attr_eeprom_read/write spotted by Mark Brown.
* moved from using regmap_bulk_read/write to regmap_raw_read/write
spotted by Mark Brown
* Fixed return error codes for the dummy wrappers spotted by Sascha Hauer
* Fixed various error code checks in core spotted by Sascha Hauer.
* Simplified consumer bindings suggested by Sascha Hauer.
* Added eeprom-mmio helper functions.
Changes since v2(https://lkml.org/lkml/2015/3/13/168)
* Fixed error handling in eeprom_register spotted by Mark Brown
* Added new regmap_get_max_register() and regmap_get_reg_stride().
* Fixed module build errors reported by kbuild robot.
* recycle the ids when eeprom provider is released.
Changes since v1(https://lkml.org/lkml/2015/3/5/153)
* Fix various Licencing issues spotted by Paul Bolle and Mark Brown
* Allow eeprom core to build as module spotted by Paul Bolle.
* Fix various kconfig issues spotted by Paul Bolle.
* remove unessary atomic varible spotted by Mark Brown.
* Few cleanups and common up some of the code in core.
* Add qfprom bindings.
Changes since RFC(https://lkml.org/lkml/2015/2/19/307)
* Fix documentation and error checks in read/write spotted by Andrew Lunn
* Kconfig fix suggested by Stephen Boyd.
* Add module owner suggested by Stephen Boyd and others.
* Fix unsafe handling of eeprom in unregister spotted by Russell and Mark Brown.
* seperate bindings patch as suggested by Rob.
* Add MAINTAINERS as suggested by Rob.
* Added support to allow reading eeprom for things like serial number which
* canbe scatters across.
* Added eeprom data using reg property suggested by Sascha and Stephen.
* Added non-DT support.
* Move kerneldoc to the src files spotted by Mark Brown.
* Remove local list and do eeprom lookup by using class_find_device()
Thanks,
srini
Maxime Ripard (1):
nvmem: sunxi: Move the SID driver to the nvmem framework
Srinivas Kandagatla (10):
regmap: Introduce regmap_get_max_register.
regmap: Introduce regmap_get_reg_stride.
nvmem: Add a simple NVMEM framework for nvmem providers
nvmem: Add a simple NVMEM framework for consumers
nvmem: Add nvmem_device based consumer apis.
nvmem: Add bindings for simple nvmem framework
nvmem: Add simple nvmem-mmio consumer helper functions.
nvmem: qfprom: Add Qualcomm QFPROM support.
nvmem: qfprom: Add bindings for qfprom
nvmem: Add to MAINTAINERS for nvmem framework
Documentation/ABI/testing/sysfs-driver-sunxi-sid | 22 -
.../bindings/misc/allwinner,sunxi-sid.txt | 17 -
.../bindings/nvmem/allwinner,sunxi-sid.txt | 21 +
Documentation/devicetree/bindings/nvmem/nvmem.txt | 84 ++
Documentation/devicetree/bindings/nvmem/qfprom.txt | 35 +
MAINTAINERS | 9 +
drivers/Kconfig | 2 +
drivers/Makefile | 1 +
drivers/base/regmap/regmap.c | 24 +
drivers/misc/eeprom/Kconfig | 13 -
drivers/misc/eeprom/Makefile | 1 -
drivers/misc/eeprom/sunxi_sid.c | 156 ----
drivers/nvmem/Kconfig | 36 +
drivers/nvmem/Makefile | 13 +
drivers/nvmem/core.c | 864 +++++++++++++++++++++
drivers/nvmem/nvmem-mmio.c | 69 ++
drivers/nvmem/nvmem-mmio.h | 41 +
drivers/nvmem/qfprom.c | 51 ++
drivers/nvmem/sunxi-sid.c | 64 ++
include/linux/nvmem-consumer.h | 106 +++
include/linux/nvmem-provider.h | 47 ++
include/linux/regmap.h | 14 +
22 files changed, 1481 insertions(+), 209 deletions(-)
delete mode 100644 Documentation/ABI/testing/sysfs-driver-sunxi-sid
delete mode 100644 Documentation/devicetree/bindings/misc/allwinner,sunxi-sid.txt
create mode 100644 Documentation/devicetree/bindings/nvmem/allwinner,sunxi-sid.txt
create mode 100644 Documentation/devicetree/bindings/nvmem/nvmem.txt
create mode 100644 Documentation/devicetree/bindings/nvmem/qfprom.txt
delete mode 100644 drivers/misc/eeprom/sunxi_sid.c
create mode 100644 drivers/nvmem/Kconfig
create mode 100644 drivers/nvmem/Makefile
create mode 100644 drivers/nvmem/core.c
create mode 100644 drivers/nvmem/nvmem-mmio.c
create mode 100644 drivers/nvmem/nvmem-mmio.h
create mode 100644 drivers/nvmem/qfprom.c
create mode 100644 drivers/nvmem/sunxi-sid.c
create mode 100644 include/linux/nvmem-consumer.h
create mode 100644 include/linux/nvmem-provider.h
--
1.9.1
This patch introduces regmap_get_max_register() function which would be
used by the infrastructures like nvmem framework built on top of
regmap.
Signed-off-by: Srinivas Kandagatla <[email protected]>
---
drivers/base/regmap/regmap.c | 12 ++++++++++++
include/linux/regmap.h | 7 +++++++
2 files changed, 19 insertions(+)
diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index 6273ff0..d6c8404 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -2613,6 +2613,18 @@ int regmap_get_val_bytes(struct regmap *map)
}
EXPORT_SYMBOL_GPL(regmap_get_val_bytes);
+/**
+ * regmap_get_max_register(): Report the max register value
+ *
+ * Report the max register value, mainly intended to for use by
+ * generic infrastructure built on top of regmap.
+ */
+int regmap_get_max_register(struct regmap *map)
+{
+ return map->max_register ? map->max_register : -EINVAL;
+}
+EXPORT_SYMBOL_GPL(regmap_get_max_register);
+
int regmap_parse_val(struct regmap *map, const void *buf,
unsigned int *val)
{
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index 116655d..2d87ded 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -433,6 +433,7 @@ int regmap_update_bits_check_async(struct regmap *map, unsigned int reg,
unsigned int mask, unsigned int val,
bool *change);
int regmap_get_val_bytes(struct regmap *map);
+int regmap_get_max_register(struct regmap *map);
int regmap_async_complete(struct regmap *map);
bool regmap_can_raw_write(struct regmap *map);
@@ -676,6 +677,12 @@ static inline int regmap_get_val_bytes(struct regmap *map)
return -EINVAL;
}
+static inline int regmap_get_max_register(struct regmap *map)
+{
+ WARN_ONCE(1, "regmap API is disabled");
+ return -EINVAL;
+}
+
static inline int regcache_sync(struct regmap *map)
{
WARN_ONCE(1, "regmap API is disabled");
--
1.9.1
This patch introduces regmap_get_reg_stride() function which would
be used by the infrastructures like nvmem framework built on top of
regmap. Mostly this function would be used for sanity checks on inputs
within such infrastructure.
Signed-off-by: Srinivas Kandagatla <[email protected]>
---
drivers/base/regmap/regmap.c | 12 ++++++++++++
include/linux/regmap.h | 7 +++++++
2 files changed, 19 insertions(+)
diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index d6c8404..e5eef6f 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -2625,6 +2625,18 @@ int regmap_get_max_register(struct regmap *map)
}
EXPORT_SYMBOL_GPL(regmap_get_max_register);
+/**
+ * regmap_get_reg_stride(): Report the register address stride
+ *
+ * Report the register address stride, mainly intended to for use by
+ * generic infrastructure built on top of regmap.
+ */
+int regmap_get_reg_stride(struct regmap *map)
+{
+ return map->reg_stride;
+}
+EXPORT_SYMBOL_GPL(regmap_get_reg_stride);
+
int regmap_parse_val(struct regmap *map, const void *buf,
unsigned int *val)
{
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index 2d87ded..59c55ea 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -434,6 +434,7 @@ int regmap_update_bits_check_async(struct regmap *map, unsigned int reg,
bool *change);
int regmap_get_val_bytes(struct regmap *map);
int regmap_get_max_register(struct regmap *map);
+int regmap_get_reg_stride(struct regmap *map);
int regmap_async_complete(struct regmap *map);
bool regmap_can_raw_write(struct regmap *map);
@@ -683,6 +684,12 @@ static inline int regmap_get_max_register(struct regmap *map)
return -EINVAL;
}
+static inline int regmap_get_reg_stride(struct regmap *map)
+{
+ WARN_ONCE(1, "regmap API is disabled");
+ return -EINVAL;
+}
+
static inline int regcache_sync(struct regmap *map)
{
WARN_ONCE(1, "regmap API is disabled");
--
1.9.1
This patch adds just providers part of the framework just to enable easy
review.
Up until now, NVMEM drivers like eeprom were stored in drivers/misc,
where they all had to duplicate pretty much the same code to register
a sysfs file, allow in-kernel users to access the content of the devices
they were driving, etc.
This was also a problem as far as other in-kernel users were involved, since
the solutions used were pretty much different from on driver to another,
there was a rather big abstraction leak.
This introduction of this framework aims at solving this. It also
introduces DT representation for consumer devices to go get the data
they require (MAC Addresses, SoC/Revision ID, part numbers, and so on)
from the nvmems.
Having regmap interface to this framework would give much better
abstraction for nvmems on different buses.
Signed-off-by: Maxime Ripard <[email protected]>
[Maxime Ripard: intial version of eeprom framework]
Signed-off-by: Srinivas Kandagatla <[email protected]>
---
drivers/Kconfig | 2 +
drivers/Makefile | 1 +
drivers/nvmem/Kconfig | 10 ++
drivers/nvmem/Makefile | 6 +
drivers/nvmem/core.c | 398 +++++++++++++++++++++++++++++++++++++++++
include/linux/nvmem-provider.h | 53 ++++++
6 files changed, 470 insertions(+)
create mode 100644 drivers/nvmem/Kconfig
create mode 100644 drivers/nvmem/Makefile
create mode 100644 drivers/nvmem/core.c
create mode 100644 include/linux/nvmem-provider.h
diff --git a/drivers/Kconfig b/drivers/Kconfig
index c0cc96b..69d7305 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -182,4 +182,6 @@ source "drivers/thunderbolt/Kconfig"
source "drivers/android/Kconfig"
+source "drivers/nvmem/Kconfig"
+
endmenu
diff --git a/drivers/Makefile b/drivers/Makefile
index 46d2554..f86b897 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -165,3 +165,4 @@ obj-$(CONFIG_RAS) += ras/
obj-$(CONFIG_THUNDERBOLT) += thunderbolt/
obj-$(CONFIG_CORESIGHT) += hwtracing/coresight/
obj-$(CONFIG_ANDROID) += android/
+obj-$(CONFIG_NVMEM) += nvmem/
diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
new file mode 100644
index 0000000..f157b6d
--- /dev/null
+++ b/drivers/nvmem/Kconfig
@@ -0,0 +1,10 @@
+menuconfig NVMEM
+ tristate "NVMEM Support"
+ select REGMAP
+ help
+ Support for NVMEM devices.
+
+ This framework is designed to provide a generic interface to NVMEM
+ from both the Linux Kernel and the userspace.
+
+ If unsure, say no.
diff --git a/drivers/nvmem/Makefile b/drivers/nvmem/Makefile
new file mode 100644
index 0000000..6df2c69
--- /dev/null
+++ b/drivers/nvmem/Makefile
@@ -0,0 +1,6 @@
+#
+# Makefile for nvmem drivers.
+#
+
+obj-$(CONFIG_NVMEM) += nvmem_core.o
+nvmem_core-y := core.o
diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
new file mode 100644
index 0000000..6c2f0b1
--- /dev/null
+++ b/drivers/nvmem/core.c
@@ -0,0 +1,398 @@
+/*
+ * nvmem framework core.
+ *
+ * Copyright (C) 2015 Srinivas Kandagatla <[email protected]>
+ * Copyright (C) 2013 Maxime Ripard <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * 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/nvmem-provider.h>
+#include <linux/export.h>
+#include <linux/fs.h>
+#include <linux/idr.h>
+#include <linux/init.h>
+#include <linux/regmap.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/slab.h>
+#include <linux/uaccess.h>
+
+struct nvmem_device {
+ const char *name;
+ struct regmap *regmap;
+ struct module *owner;
+ struct device dev;
+ int stride;
+ int word_size;
+ int ncells;
+ int id;
+ int users;
+ size_t size;
+ bool read_only;
+};
+
+struct nvmem_cell {
+ const char *name;
+ int offset;
+ int bytes;
+ int bit_offset;
+ int nbits;
+ struct nvmem_device *nvmem;
+ struct list_head node;
+};
+
+static DEFINE_MUTEX(nvmem_mutex);
+static DEFINE_IDA(nvmem_ida);
+
+static LIST_HEAD(nvmem_cells);
+static DEFINE_MUTEX(nvmem_cells_mutex);
+
+#define to_nvmem_device(d) container_of(d, struct nvmem_device, dev)
+
+static ssize_t bin_attr_nvmem_read(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *attr,
+ char *buf, loff_t pos, size_t count)
+{
+ struct device *dev = container_of(kobj, struct device, kobj);
+ struct nvmem_device *nvmem = to_nvmem_device(dev);
+ int rc;
+
+ /* Stop the user from reading */
+ if (pos > nvmem->size)
+ return 0;
+
+ if (pos + count > nvmem->size)
+ count = nvmem->size - pos;
+
+ count = count/nvmem->word_size * nvmem->word_size;
+
+ rc = regmap_raw_read(nvmem->regmap, pos, buf, count);
+
+ if (IS_ERR_VALUE(rc))
+ return rc;
+
+ return count;
+}
+
+static ssize_t bin_attr_nvmem_write(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *attr,
+ char *buf, loff_t pos, size_t count)
+{
+ struct device *dev = container_of(kobj, struct device, kobj);
+ struct nvmem_device *nvmem = to_nvmem_device(dev);
+ int rc;
+
+ /* Stop the user from writing */
+ if (pos > nvmem->size)
+ return 0;
+
+ if (pos + count > nvmem->size)
+ count = nvmem->size - pos;
+
+ count = count/nvmem->word_size * nvmem->word_size;
+
+ rc = regmap_raw_write(nvmem->regmap, pos, buf, count);
+
+ if (IS_ERR_VALUE(rc))
+ return rc;
+
+ return count;
+}
+
+/* default read/write permissions */
+static struct bin_attribute bin_attr_nvmem = {
+ .attr = {
+ .name = "nvmem",
+ .mode = S_IWUSR | S_IRUGO,
+ },
+ .read = bin_attr_nvmem_read,
+ .write = bin_attr_nvmem_write,
+};
+
+static struct bin_attribute *nvmem_bin_attributes[] = {
+ &bin_attr_nvmem,
+ NULL,
+};
+
+static const struct attribute_group nvmem_bin_group = {
+ .bin_attrs = nvmem_bin_attributes,
+};
+
+static const struct attribute_group *nvmem_dev_groups[] = {
+ &nvmem_bin_group,
+ NULL,
+};
+
+/* read only permission */
+static struct bin_attribute bin_attr_ro_nvmem = {
+ .attr = {
+ .name = "nvmem",
+ .mode = S_IRUGO,
+ },
+ .read = bin_attr_nvmem_read,
+};
+
+static struct bin_attribute *nvmem_bin_ro_attributes[] = {
+ &bin_attr_ro_nvmem,
+ NULL,
+};
+static const struct attribute_group nvmem_bin_ro_group = {
+ .bin_attrs = nvmem_bin_ro_attributes,
+};
+
+static void nvmem_release(struct device *dev)
+{
+ struct nvmem_device *nvmem = to_nvmem_device(dev);
+
+ ida_simple_remove(&nvmem_ida, nvmem->id);
+ kfree(nvmem);
+}
+
+static struct class nvmem_class = {
+ .name = "nvmem",
+ .dev_groups = nvmem_dev_groups,
+ .dev_release = nvmem_release,
+};
+
+static int of_nvmem_match(struct device *dev, const void *nvmem_np)
+{
+ return dev->of_node == nvmem_np;
+}
+
+static struct nvmem_device *of_nvmem_find(struct device_node *nvmem_np)
+{
+ struct device *d;
+
+ if (!nvmem_np)
+ return NULL;
+
+ d = class_find_device(&nvmem_class, NULL, nvmem_np, of_nvmem_match);
+
+ return d ? to_nvmem_device(d) : NULL;
+}
+
+static struct nvmem_cell *nvmem_find_cell(const char *cell_id)
+{
+ struct nvmem_cell *p;
+
+ list_for_each_entry(p, &nvmem_cells, node) {
+ if (p && !strcmp(p->name, cell_id))
+ return p;
+ }
+
+ return NULL;
+}
+
+static void nvmem_cell_drop(struct nvmem_cell *cell)
+{
+ mutex_lock(&nvmem_cells_mutex);
+ list_del(&cell->node);
+ mutex_unlock(&nvmem_cells_mutex);
+ kfree(cell);
+}
+
+static void nvmem_device_remove_all_cells(struct nvmem_device *nvmem)
+{
+ struct nvmem_cell *cell = NULL;
+ struct list_head *p, *n;
+
+ list_for_each_safe(p, n, &nvmem_cells) {
+ cell = list_entry(p, struct nvmem_cell, node);
+ if (cell->nvmem == nvmem)
+ nvmem_cell_drop(cell);
+ }
+}
+
+static void nvmem_cell_add(struct nvmem_cell *cell)
+{
+ mutex_lock(&nvmem_cells_mutex);
+ list_add_tail(&cell->node, &nvmem_cells);
+ mutex_unlock(&nvmem_cells_mutex);
+}
+
+static int nvmem_cell_info_to_nvmem_cell(struct nvmem_device *nvmem,
+ struct nvmem_cell_info *info,
+ struct nvmem_cell *cell)
+{
+ cell->nvmem = nvmem;
+ cell->offset = info->offset;
+ cell->bytes = info->bytes;
+ cell->name = info->name;
+
+ cell->bit_offset = info->bit_offset;
+ cell->nbits = info->nbits;
+
+ if (cell->nbits)
+ cell->bytes = DIV_ROUND_UP(cell->nbits + cell->bit_offset,
+ BITS_PER_BYTE);
+
+ if (!IS_ALIGNED(cell->offset, nvmem->stride)) {
+ dev_err(&nvmem->dev,
+ "cell %s unaligned to nvmem stride %d\n",
+ cell->name, nvmem->stride);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int nvmem_add_cells(struct nvmem_device *nvmem,
+ struct nvmem_config *cfg)
+{
+ struct nvmem_cell **cells;
+ struct nvmem_cell_info *info = cfg->cells;
+ int i, rval;
+
+ cells = kzalloc(sizeof(*cells) * cfg->ncells, GFP_KERNEL);
+ if (!cells)
+ return -ENOMEM;
+
+ for (i = 0; i < cfg->ncells; i++) {
+ cells[i] = kzalloc(sizeof(struct nvmem_cell), GFP_KERNEL);
+ if (!cells[i]) {
+ rval = -ENOMEM;
+ goto err;
+ }
+
+ rval = nvmem_cell_info_to_nvmem_cell(nvmem, &info[i], cells[i]);
+ if (IS_ERR_VALUE(rval)) {
+ kfree(cells[i]);
+ goto err;
+ }
+
+ nvmem_cell_add(cells[i]);
+ }
+
+ nvmem->ncells = cfg->ncells;
+ /* remove tmp array */
+ kfree(cells);
+
+ return 0;
+err:
+ while (--i)
+ nvmem_cell_drop(cells[i]);
+
+ return rval;
+}
+
+/**
+ * nvmem_register(): Register a nvmem device for given nvmem.
+ * Also creates an binary entry in /sys/class/nvmem/dev-name/nvmem
+ *
+ * @nvmem: nvmem device that needs to be created
+ *
+ * The return value will be an ERR_PTR() on error or a valid pointer
+ * to nvmem_device.
+ */
+
+struct nvmem_device *nvmem_register(struct nvmem_config *config)
+{
+ struct nvmem_device *nvmem;
+ struct regmap *rm;
+ int rval;
+
+ if (!config->dev)
+ return ERR_PTR(-EINVAL);
+
+ rm = dev_get_regmap(config->dev, NULL);
+ if (!rm) {
+ dev_err(config->dev, "Regmap not found\n");
+ return ERR_PTR(-EINVAL);
+ }
+
+ nvmem = kzalloc(sizeof(*nvmem), GFP_KERNEL);
+ if (!nvmem)
+ return ERR_PTR(-ENOMEM);
+
+ nvmem->id = ida_simple_get(&nvmem_ida, 0, 0, GFP_KERNEL);
+ if (nvmem->id < 0) {
+ kfree(nvmem);
+ return ERR_PTR(nvmem->id);
+ }
+
+ nvmem->regmap = rm;
+ nvmem->owner = config->owner;
+ nvmem->stride = regmap_get_reg_stride(rm);
+ nvmem->word_size = regmap_get_val_bytes(rm);
+ nvmem->size = regmap_get_max_register(rm) + nvmem->stride;
+ nvmem->dev.class = &nvmem_class;
+ nvmem->dev.parent = config->dev;
+ nvmem->dev.of_node = config->dev->of_node;
+ dev_set_name(&nvmem->dev, "%s%d",
+ config->name ? : "nvmem", config->id);
+
+ nvmem->read_only = of_property_read_bool(nvmem->dev.of_node,
+ "read-only");
+
+ device_initialize(&nvmem->dev);
+
+ dev_dbg(&nvmem->dev, "Registering nvmem device %s\n",
+ dev_name(&nvmem->dev));
+
+ rval = device_add(&nvmem->dev);
+ if (rval) {
+ ida_simple_remove(&nvmem_ida, nvmem->id);
+ kfree(nvmem);
+ return ERR_PTR(rval);
+ }
+
+ /* update sysfs attributes */
+ if (nvmem->read_only)
+ sysfs_update_group(&nvmem->dev.kobj, &nvmem_bin_ro_group);
+
+ if (config->cells)
+ nvmem_add_cells(nvmem, config);
+
+ return nvmem;
+}
+EXPORT_SYMBOL_GPL(nvmem_register);
+
+/**
+ * nvmem_unregister(): Unregister previously registered nvmem device
+ *
+ * @nvmem: Pointer to previously registered nvmem device.
+ *
+ * The return value will be an non zero on error or a zero on success.
+ */
+int nvmem_unregister(struct nvmem_device *nvmem)
+{
+ mutex_lock(&nvmem_mutex);
+ if (nvmem->users) {
+ mutex_unlock(&nvmem_mutex);
+ return -EBUSY;
+ }
+ mutex_unlock(&nvmem_mutex);
+
+ nvmem_device_remove_all_cells(nvmem);
+ device_del(&nvmem->dev);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(nvmem_unregister);
+
+static int nvmem_init(void)
+{
+ return class_register(&nvmem_class);
+}
+
+static void nvmem_exit(void)
+{
+ class_unregister(&nvmem_class);
+}
+
+subsys_initcall(nvmem_init);
+module_exit(nvmem_exit);
+
+MODULE_AUTHOR("Srinivas Kandagatla <[email protected]");
+MODULE_AUTHOR("Maxime Ripard <[email protected]");
+MODULE_DESCRIPTION("nvmem Driver Core");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/nvmem-provider.h b/include/linux/nvmem-provider.h
new file mode 100644
index 0000000..4908b37
--- /dev/null
+++ b/include/linux/nvmem-provider.h
@@ -0,0 +1,53 @@
+/*
+ * nvmem framework provider.
+ *
+ * Copyright (C) 2015 Srinivas Kandagatla <[email protected]>
+ * Copyright (C) 2013 Maxime Ripard <[email protected]>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#ifndef _LINUX_NVMEM_PROVIDER_H
+#define _LINUX_NVMEM_PROVIDER_H
+
+struct nvmem_device;
+
+struct nvmem_cell_info {
+ const char *name;
+ int offset;
+ int bytes;
+ int bit_offset;
+ int nbits;
+};
+
+struct nvmem_config {
+ struct device *dev;
+ const char *name;
+ int id;
+ struct module *owner;
+ struct nvmem_cell_info *cells;
+ int ncells;
+};
+
+#if IS_ENABLED(CONFIG_NVMEM)
+
+struct nvmem_device *nvmem_register(struct nvmem_config *cfg);
+int nvmem_unregister(struct nvmem_device *nvmem);
+
+#else
+
+static inline struct nvmem_device *nvmem_register(struct nvmem_config *cfg)
+{
+ return ERR_PTR(-ENOSYS);
+}
+
+static inline int nvmem_unregister(struct nvmem_device *nvmem)
+{
+ return -ENOSYS;
+}
+
+#endif /* CONFIG_NVMEM */
+
+#endif /* ifndef _LINUX_NVMEM_PROVIDER_H */
--
1.9.1
This patch adds just consumers part of the framework just to enable easy
review.
Up until now, nvmem drivers were stored in drivers/misc, where they all had to
duplicate pretty much the same code to register a sysfs file, allow in-kernel
users to access the content of the devices they were driving, etc.
This was also a problem as far as other in-kernel users were involved, since
the solutions used were pretty much different from on driver to another, there
was a rather big abstraction leak.
This introduction of this framework aims at solving this. It also introduces DT
representation for consumer devices to go get the data they require (MAC
Addresses, SoC/Revision ID, part numbers, and so on) from the nvmems.
Having regmap interface to this framework would give much better
abstraction for nvmems on different buses.
Signed-off-by: Maxime Ripard <[email protected]>
[Maxime Ripard: intial version of the framework]
Signed-off-by: Srinivas Kandagatla <[email protected]>
---
drivers/nvmem/core.c | 346 +++++++++++++++++++++++++++++++++++++++++
include/linux/nvmem-consumer.h | 49 ++++++
2 files changed, 395 insertions(+)
create mode 100644 include/linux/nvmem-consumer.h
diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index 6c2f0b1..8a4b358 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -16,6 +16,7 @@
#include <linux/device.h>
#include <linux/nvmem-provider.h>
+#include <linux/nvmem-consumer.h>
#include <linux/export.h>
#include <linux/fs.h>
#include <linux/idr.h>
@@ -379,6 +380,351 @@ int nvmem_unregister(struct nvmem_device *nvmem)
}
EXPORT_SYMBOL_GPL(nvmem_unregister);
+static struct nvmem_device *__nvmem_device_get(struct device_node *np,
+ struct nvmem_cell **cellp,
+ const char *cell_id)
+{
+ struct nvmem_device *nvmem = NULL;
+
+ mutex_lock(&nvmem_mutex);
+
+ if (np) {
+ nvmem = of_nvmem_find(np);
+ if (!nvmem) {
+ mutex_unlock(&nvmem_mutex);
+ return ERR_PTR(-EPROBE_DEFER);
+ }
+ } else {
+ struct nvmem_cell *cell = nvmem_find_cell(cell_id);
+
+ if (cell) {
+ nvmem = cell->nvmem;
+ *cellp = cell;
+ }
+
+ if (!nvmem) {
+ mutex_unlock(&nvmem_mutex);
+ return ERR_PTR(-ENOENT);
+ }
+ }
+
+ nvmem->users++;
+ mutex_unlock(&nvmem_mutex);
+
+ if (!try_module_get(nvmem->owner)) {
+ dev_err(&nvmem->dev,
+ "could not increase module refcount for cell %s\n",
+ nvmem->name);
+
+ mutex_lock(&nvmem_mutex);
+ nvmem->users--;
+ mutex_unlock(&nvmem_mutex);
+
+ return ERR_PTR(-EINVAL);
+ }
+
+ return nvmem;
+}
+
+static int __nvmem_device_put(struct nvmem_device *nvmem)
+{
+ module_put(nvmem->owner);
+ mutex_lock(&nvmem_mutex);
+ nvmem->users--;
+ mutex_unlock(&nvmem_mutex);
+
+ return 0;
+}
+
+static struct nvmem_cell *nvmem_cell_get_from_list(const char *cell_id)
+{
+ struct nvmem_cell *cell = NULL;
+ struct nvmem_device *nvmem;
+
+ nvmem = __nvmem_device_get(NULL, &cell, cell_id);
+ if (IS_ERR(nvmem))
+ return (struct nvmem_cell *)nvmem;
+
+
+ return cell;
+
+}
+
+static struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
+ const char *name)
+{
+ struct device_node *cell_np, *nvmem_np;
+ struct nvmem_cell *cell;
+ struct nvmem_device *nvmem;
+ const __be32 *addr;
+ int rval, len, index;
+
+ index = of_property_match_string(np, "nvmem-cell-names", name);
+
+ cell_np = of_parse_phandle(np, "nvmem-cell", index);
+ if (!cell_np)
+ return ERR_PTR(-EINVAL);
+
+ nvmem_np = of_get_next_parent(cell_np);
+ if (!nvmem_np)
+ return ERR_PTR(-EINVAL);
+
+ nvmem = __nvmem_device_get(nvmem_np, NULL, NULL);
+ if (IS_ERR(nvmem))
+ return (struct nvmem_cell *)nvmem;
+
+ addr = of_get_property(cell_np, "reg", &len);
+ if (!addr || (len < 2 * sizeof(int))) {
+ dev_err(&nvmem->dev, "of_i2c: invalid reg on %s\n",
+ cell_np->full_name);
+ rval = -EINVAL;
+ goto err_mem;
+ }
+
+ cell = kzalloc(sizeof(*cell), GFP_KERNEL);
+ if (!cell) {
+ rval = -ENOMEM;
+ goto err_mem;
+ }
+
+ cell->nvmem = nvmem;
+ cell->offset = be32_to_cpup(addr++);
+ cell->bytes = be32_to_cpup(addr);
+ cell->name = cell_np->name;
+
+ of_property_read_u32(cell_np, "bit-offset", &cell->bit_offset);
+ of_property_read_u32(cell_np, "nbits", &cell->nbits);
+
+ if (cell->nbits)
+ cell->bytes = DIV_ROUND_UP(cell->nbits + cell->bit_offset,
+ BITS_PER_BYTE);
+
+ if (!IS_ALIGNED(cell->offset, nvmem->stride)) {
+ dev_err(&nvmem->dev,
+ "cell %s unaligned to nvmem stride %d\n",
+ cell->name, nvmem->stride);
+ rval = -EINVAL;
+ goto err_sanity;
+ }
+
+ nvmem_cell_add(cell);
+
+ return cell;
+
+err_sanity:
+ kfree(cell);
+
+err_mem:
+ __nvmem_device_put(nvmem);
+
+ return ERR_PTR(rval);
+
+}
+
+/**
+ * nvmem_cell_get(): Get nvmem cell of device form a given index
+ *
+ * @dev node: Device tree node that uses the nvmem cell
+ * @index: nvmem index in nvmems property.
+ *
+ * The return value will be an ERR_PTR() on error or a valid pointer
+ * to a struct nvmem_cell. The nvmem_cell will be freed by the
+ * nvmem_cell_put().
+ */
+struct nvmem_cell *nvmem_cell_get(struct device *dev, const char *cell_id)
+{
+ struct nvmem_cell *cell;
+
+ if (dev->of_node) { /* try dt first */
+ cell = of_nvmem_cell_get(dev->of_node, cell_id);
+ if (!IS_ERR(cell) || PTR_ERR(cell) == -EPROBE_DEFER)
+ return cell;
+ }
+
+ return nvmem_cell_get_from_list(cell_id);
+
+}
+EXPORT_SYMBOL_GPL(nvmem_cell_get);
+
+/**
+ * nvmem_cell_put(): Release previously allocated nvmem cell.
+ *
+ * @cell: Previously allocated nvmem cell by nvmem_cell_get()
+ * or nvmem_cell_get().
+ */
+void nvmem_cell_put(struct nvmem_cell *cell)
+{
+ struct nvmem_device *nvmem = cell->nvmem;
+
+ __nvmem_device_put(nvmem);
+ nvmem_cell_drop(cell);
+}
+EXPORT_SYMBOL_GPL(nvmem_cell_put);
+
+static inline void nvmem_shift_read_buffer_in_place(struct nvmem_cell *cell,
+ void *buf)
+{
+ u8 *p, *b;
+ int i, bit_offset = cell->bit_offset;
+
+ p = b = buf;
+ if (bit_offset) {
+ /* First shift */
+ *b++ >>= bit_offset;
+
+ /* setup rest of the bytes if any */
+ for (i = 1; i < cell->bytes; i++) {
+ /* Get bits from next byte and shift them towards msb */
+ *p |= *b << (BITS_PER_BYTE - bit_offset);
+
+ p = b;
+ *b++ >>= bit_offset;
+ }
+
+ /* result fits in less bytes */
+ if (cell->bytes != DIV_ROUND_UP(cell->nbits, BITS_PER_BYTE))
+ *p-- = 0;
+ }
+ /* clear msb bits if any leftover in the last byte */
+ *p &= GENMASK((cell->nbits%BITS_PER_BYTE) - 1, 0);
+}
+
+static int __nvmem_cell_read(struct nvmem_device *nvmem,
+ struct nvmem_cell *cell,
+ void *buf, ssize_t *len)
+{
+ int rc;
+
+ rc = regmap_raw_read(nvmem->regmap, cell->offset, buf, cell->bytes);
+
+ if (IS_ERR_VALUE(rc))
+ return rc;
+
+ /* shift bits in-place */
+ if (cell->bit_offset || cell->bit_offset)
+ nvmem_shift_read_buffer_in_place(cell, buf);
+
+ *len = cell->bytes;
+
+ return *len;
+}
+/**
+ * nvmem_cell_read(): Read a given nvmem cell
+ *
+ * @cell: nvmem cell to be read.
+ * @len: pointer to length of cell which will be populated on successful read.
+ *
+ * The return value will be an ERR_PTR() on error or a valid pointer
+ * to a char * bufffer. The buffer should be freed by the consumer with a
+ * kfree().
+ */
+void *nvmem_cell_read(struct nvmem_cell *cell, ssize_t *len)
+{
+ struct nvmem_device *nvmem = cell->nvmem;
+ u8 *buf;
+ int rc;
+
+ if (!nvmem || !nvmem->regmap)
+ return ERR_PTR(-EINVAL);
+
+ buf = kzalloc(cell->bytes, GFP_KERNEL);
+ if (!buf)
+ return ERR_PTR(-ENOMEM);
+
+ rc = __nvmem_cell_read(nvmem, cell, buf, len);
+ if (IS_ERR_VALUE(rc)) {
+ kfree(buf);
+ return ERR_PTR(rc);
+ }
+
+ return buf;
+}
+EXPORT_SYMBOL_GPL(nvmem_cell_read);
+
+static inline void *nvmem_cell_prepare_write_buffer(struct nvmem_cell *cell,
+ u8 *_buf, int len)
+{
+ struct nvmem_device *nvmem = cell->nvmem;
+ int i, rc, nbits, bit_offset = cell->bit_offset;
+ u8 v, *p, *buf, *b, pbyte, pbits;
+
+ nbits = cell->nbits;
+ buf = kzalloc(cell->bytes, GFP_KERNEL);
+ if (!buf)
+ return ERR_PTR(-ENOMEM);
+
+ memcpy(buf, _buf, len);
+ p = b = buf;
+
+ if (bit_offset) {
+ pbyte = *b;
+ *b <<= bit_offset;
+
+ /* setup the first byte with lsb bits from nvmem */
+ rc = regmap_raw_read(nvmem->regmap, cell->offset, &v, 1);
+ *b++ |= GENMASK(bit_offset - 1, 0) & v;
+
+ /* setup rest of the byte if any */
+ for (i = 1; i < cell->bytes; i++) {
+ /* Get last byte bits and shift them towards lsb */
+ pbits = pbyte >> (BITS_PER_BYTE - 1 - bit_offset);
+ pbyte = *b;
+ p = b;
+ *b <<= bit_offset;
+ *b++ |= pbits;
+ }
+ }
+
+ /* if it's not end on byte boundary */
+ if ((nbits + bit_offset) % BITS_PER_BYTE) {
+ /* setup the last byte with msb bits from nvmem */
+ rc = regmap_raw_read(nvmem->regmap,
+ cell->offset + cell->bytes - 1, &v, 1);
+ *p |= GENMASK(7, (nbits + bit_offset) % BITS_PER_BYTE) & v;
+
+ }
+
+ return buf;
+}
+
+/**
+ * nvmem_cell_write(): Write to a given nvmem cell
+ *
+ * @cell: nvmem cell to be written.
+ * @buf: Buffer to be written.
+ * @len: length of buffer to be written to nvmem cell.
+ *
+ * The return value will be an length of bytes written or non zero on failure.
+ */
+int nvmem_cell_write(struct nvmem_cell *cell, void *buf, ssize_t len)
+{
+ struct nvmem_device *nvmem = cell->nvmem;
+ int rc;
+ void *wbuf = buf;
+
+ if (!nvmem || !nvmem->regmap || nvmem->read_only ||
+ (cell->bit_offset == 0 && len != cell->bytes))
+ return -EINVAL;
+
+ if (cell->bit_offset || cell->nbits) {
+ wbuf = nvmem_cell_prepare_write_buffer(cell, buf, len);
+ if (IS_ERR(wbuf))
+ return PTR_ERR(wbuf);
+ }
+
+ rc = regmap_raw_write(nvmem->regmap, cell->offset, wbuf, cell->bytes);
+
+ /* free the tmp buffer */
+ if (cell->bit_offset)
+ kfree(wbuf);
+
+ if (IS_ERR_VALUE(rc))
+ return rc;
+
+ return len;
+}
+EXPORT_SYMBOL_GPL(nvmem_cell_write);
+
static int nvmem_init(void)
{
return class_register(&nvmem_class);
diff --git a/include/linux/nvmem-consumer.h b/include/linux/nvmem-consumer.h
new file mode 100644
index 0000000..c3fa8c7
--- /dev/null
+++ b/include/linux/nvmem-consumer.h
@@ -0,0 +1,49 @@
+/*
+ * nvmem framework consumer.
+ *
+ * Copyright (C) 2015 Srinivas Kandagatla <[email protected]>
+ * Copyright (C) 2013 Maxime Ripard <[email protected]>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#ifndef _LINUX_NVMEM_CONSUMER_H
+#define _LINUX_NVMEM_CONSUMER_H
+
+/* consumer cookie */
+struct nvmem_cell;
+
+#if IS_ENABLED(CONFIG_NVMEM)
+
+/* Cell based interface */
+struct nvmem_cell *nvmem_cell_get(struct device *dev, const char *name);
+void nvmem_cell_put(struct nvmem_cell *cell);
+void *nvmem_cell_read(struct nvmem_cell *cell, ssize_t *len);
+int nvmem_cell_write(struct nvmem_cell *cell, void *buf, ssize_t len);
+
+#else
+
+struct nvmem_cell *nvmem_cell_get(struct device *dev, const char *name)
+{
+ return ERR_PTR(-ENOSYS);
+}
+
+static inline void nvmem_cell_put(struct nvmem_cell *cell)
+{
+}
+
+static inline char *nvmem_cell_read(struct nvmem_cell *cell, ssize_t *len)
+{
+ return ERR_PTR(-ENOSYS);
+}
+
+static inline int nvmem_cell_write(struct nvmem_cell *cell,
+ const char *buf, ssize_t len)
+{
+ return -ENOSYS;
+}
+#endif /* CONFIG_NVMEM */
+
+#endif /* ifndef _LINUX_NVMEM_CONSUMER_H */
--
1.9.1
This patch adds read/write apis which are based on nvmem_device. It is
common that the drivers like omap cape manager or qcom cpr driver to access
bytes directly at particular offset in the eeprom and not from nvmem
cell info in DT. These driver would need to get access to the nvmem
directly, which is what these new APIS provide.
These wrapper apis would help such users to avoid code duplication in
there drivers and also avoid them reading a big eeprom blob and parsing
it internally in there driver.
Signed-off-by: Srinivas Kandagatla <[email protected]>
---
drivers/nvmem/core.c | 120 +++++++++++++++++++++++++++++++++++++++++
include/linux/nvmem-consumer.h | 57 ++++++++++++++++++++
include/linux/nvmem-provider.h | 10 +---
3 files changed, 179 insertions(+), 8 deletions(-)
diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index 8a4b358..68ee8d1 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -436,6 +436,51 @@ static int __nvmem_device_put(struct nvmem_device *nvmem)
return 0;
}
+static int nvmem_match(struct device *dev, const void *data)
+{
+ return !strcmp(dev_name(dev), (const char *)data);
+}
+
+static struct nvmem_device *nvmem_find(const char *name)
+{
+ struct device *d;
+
+ d = class_find_device(&nvmem_class, NULL, (void *)name, nvmem_match);
+
+ return d ? to_nvmem_device(d) : NULL;
+}
+
+struct nvmem_device *nvmem_device_get(struct device *dev, const char *dev_name)
+{
+ struct device_node *nvmem_np, *np = dev->of_node;
+ struct nvmem_device *nvmem;
+ int index;
+
+ if (np) { /* try dt first */
+ index = of_property_match_string(np, "nvmem-names", dev_name);
+
+ nvmem_np = of_parse_phandle(np, "nvmem", index);
+ if (!nvmem_np)
+ return ERR_PTR(-EINVAL);
+
+ nvmem = __nvmem_device_get(nvmem_np, NULL, NULL);
+
+ if (!IS_ERR(nvmem) || PTR_ERR(nvmem) == -EPROBE_DEFER)
+ return nvmem;
+
+ }
+
+ return nvmem_find(dev_name);
+
+}
+EXPORT_SYMBOL_GPL(nvmem_device_get);
+
+void nvmem_device_put(struct nvmem_device *nvmem)
+{
+ __nvmem_device_put(nvmem);
+}
+EXPORT_SYMBOL_GPL(nvmem_device_put);
+
static struct nvmem_cell *nvmem_cell_get_from_list(const char *cell_id)
{
struct nvmem_cell *cell = NULL;
@@ -725,6 +770,81 @@ int nvmem_cell_write(struct nvmem_cell *cell, void *buf, ssize_t len)
}
EXPORT_SYMBOL_GPL(nvmem_cell_write);
+int nvmem_device_cell_read(struct nvmem_device *nvmem,
+ struct nvmem_cell_info *info, void *buf)
+{
+ struct nvmem_cell cell;
+ int rc, len;
+
+ if (!nvmem || !nvmem->regmap)
+ return -EINVAL;
+
+ rc = nvmem_cell_info_to_nvmem_cell(nvmem, info, &cell);
+ if (IS_ERR_VALUE(rc))
+ return rc;
+
+ rc = __nvmem_cell_read(nvmem, &cell, buf, &len);
+ if (IS_ERR_VALUE(rc))
+ return rc;
+
+ return len;
+}
+EXPORT_SYMBOL_GPL(nvmem_device_cell_read);
+
+int nvmem_device_cell_write(struct nvmem_device *nvmem,
+ struct nvmem_cell_info *info, void *buf)
+{
+ struct nvmem_cell cell;
+ int rc;
+
+ if (!nvmem || !nvmem->regmap)
+ return -EINVAL;
+
+ rc = nvmem_cell_info_to_nvmem_cell(nvmem, info, &cell);
+ if (IS_ERR_VALUE(rc))
+ return rc;
+
+ return nvmem_cell_write(&cell, buf, cell.bytes);
+}
+EXPORT_SYMBOL_GPL(nvmem_device_cell_write);
+
+int nvmem_device_read(struct nvmem_device *nvmem,
+ unsigned int offset,
+ size_t bytes, void *buf)
+{
+ int rc;
+
+ if (!nvmem || !nvmem->regmap)
+ return -EINVAL;
+
+ rc = regmap_raw_read(nvmem->regmap, offset, buf, bytes);
+
+ if (IS_ERR_VALUE(rc))
+ return rc;
+
+ return bytes;
+}
+EXPORT_SYMBOL_GPL(nvmem_device_read);
+
+int nvmem_device_write(struct nvmem_device *nvmem,
+ unsigned int offset,
+ size_t bytes, void *buf)
+{
+ int rc;
+
+ if (!nvmem || !nvmem->regmap)
+ return -EINVAL;
+
+ rc = regmap_raw_write(nvmem->regmap, offset, buf, bytes);
+
+ if (IS_ERR_VALUE(rc))
+ return rc;
+
+
+ return bytes;
+}
+EXPORT_SYMBOL_GPL(nvmem_device_write);
+
static int nvmem_init(void)
{
return class_register(&nvmem_class);
diff --git a/include/linux/nvmem-consumer.h b/include/linux/nvmem-consumer.h
index c3fa8c7..66c67ba 100644
--- a/include/linux/nvmem-consumer.h
+++ b/include/linux/nvmem-consumer.h
@@ -14,6 +14,15 @@
/* consumer cookie */
struct nvmem_cell;
+struct nvmem_device;
+
+struct nvmem_cell_info {
+ const char *name;
+ int offset;
+ int bytes;
+ int bit_offset;
+ int nbits;
+};
#if IS_ENABLED(CONFIG_NVMEM)
@@ -23,6 +32,18 @@ void nvmem_cell_put(struct nvmem_cell *cell);
void *nvmem_cell_read(struct nvmem_cell *cell, ssize_t *len);
int nvmem_cell_write(struct nvmem_cell *cell, void *buf, ssize_t len);
+/* direct nvmem device read/write interface */
+struct nvmem_device *nvmem_device_get(struct device *dev, const char *name);
+void nvmem_device_put(struct nvmem_device *nvmem);
+int nvmem_device_read(struct nvmem_device *nvmem, unsigned int offset,
+ size_t bytes, void *buf);
+int nvmem_device_write(struct nvmem_device *nvmem, unsigned int offset,
+ size_t bytes, void *buf);
+int nvmem_device_cell_read(struct nvmem_device *nvmem,
+ struct nvmem_cell_info *info, void *buf);
+int nvmem_device_cell_write(struct nvmem_device *nvmem,
+ struct nvmem_cell_info *info, void *buf);
+
#else
struct nvmem_cell *nvmem_cell_get(struct device *dev, const char *name)
@@ -44,6 +65,42 @@ static inline int nvmem_cell_write(struct nvmem_cell *cell,
{
return -ENOSYS;
}
+
+static inline struct nvmem_device *nvmem_device_get(struct device *dev,
+ const char *name)
+{
+ return ERR_PTR(-ENOSYS);
+}
+
+static inline void nvmem_device_put(struct nvmem_device *nvmem)
+{
+}
+
+static inline int nvmem_device_cell_read(struct nvmem_device *nvmem,
+ struct nvmem_cell_info *info,
+ void *buf)
+{
+ return -ENOSYS;
+}
+
+static inline int nvmem_device_cell_write(struct nvmem_device *nvmem,
+ struct nvmem_cell_info *info,
+ void *buf)
+{
+ return -ENOSYS;
+}
+
+static inline int nvmem_device_read(struct nvmem_device *nvmem,
+ unsigned int offset, size_t bytes, void *buf)
+{
+ return -ENOSYS;
+}
+
+static inline int nvmem_device_write(struct nvmem_device *nvmem,
+ unsigned int offset, size_t bytes, void *buf)
+{
+ return -ENOSYS;
+}
#endif /* CONFIG_NVMEM */
#endif /* ifndef _LINUX_NVMEM_CONSUMER_H */
diff --git a/include/linux/nvmem-provider.h b/include/linux/nvmem-provider.h
index 4908b37..7a982cd 100644
--- a/include/linux/nvmem-provider.h
+++ b/include/linux/nvmem-provider.h
@@ -12,15 +12,9 @@
#ifndef _LINUX_NVMEM_PROVIDER_H
#define _LINUX_NVMEM_PROVIDER_H
-struct nvmem_device;
+#include <linux/nvmem-consumer.h>
-struct nvmem_cell_info {
- const char *name;
- int offset;
- int bytes;
- int bit_offset;
- int nbits;
-};
+struct nvmem_device;
struct nvmem_config {
struct device *dev;
--
1.9.1
This patch adds bindings for simple nvmem framework which allows nvmem
consumers to talk to nvmem providers to get access to nvmem cell data.
Signed-off-by: Maxime Ripard <[email protected]>
[Maxime Ripard: intial version of eeprom framework]
Signed-off-by: Srinivas Kandagatla <[email protected]>
---
Documentation/devicetree/bindings/nvmem/nvmem.txt | 84 +++++++++++++++++++++++
1 file changed, 84 insertions(+)
create mode 100644 Documentation/devicetree/bindings/nvmem/nvmem.txt
diff --git a/Documentation/devicetree/bindings/nvmem/nvmem.txt b/Documentation/devicetree/bindings/nvmem/nvmem.txt
new file mode 100644
index 0000000..ecea654
--- /dev/null
+++ b/Documentation/devicetree/bindings/nvmem/nvmem.txt
@@ -0,0 +1,84 @@
+= NVMEM Data Device Tree Bindings =
+
+This binding is intended to represent the location of hardware
+configuration data stored in NVMEMs.
+
+On a significant proportion of boards, the manufacturer has stored
+some data on NVMEM, for the OS to be able to retrieve these information
+and act upon it. Obviously, the OS has to know about where to retrieve
+these data from, and where they are stored on the storage device.
+
+This document is here to document this.
+
+= Data providers =
+Contains bindings specific to provider drivers and data cells as children
+to this node.
+
+Optional properties:
+ read-only: Mark the provider as read only.
+
+= Data cells =
+These are the child nodes of the provider which contain data cell
+information like offset and size in nvmem provider.
+
+Required properties:
+reg: specifies the offset in byte within that storage device, start bit
+ in the byte and the length in bits of the data we care about.
+ There could be more then one offset-length pairs in this property.
+
+Optional properties:
+
+bit-offset: specifies the offset in bit within the address range specified
+ by reg property. Can take values from 0-7.
+nbits: specifies number of bits this cell occupies starting from bit-offset.
+
+For example:
+
+ /* Provider */
+ qfprom: qfprom@00700000 {
+ ...
+
+ /* Data cells */
+ tsens_calibration: calib@404 {
+ reg = <0x404 0x10>;
+ };
+
+ tsens_calibration_bckp: calib_bckp@504 {
+ reg = <0x504 0x11>;
+ bit-offset = 6;
+ nbits = 128;
+ };
+
+ pvs_version: pvs-version@6 {
+ reg = <0x6 0x2>
+ bit-offset = 7;
+ nbits = 2;
+ };
+
+ speed_bin: speed-bin@c{
+ reg = <0xc 0x1>;
+ bit-offset = 2;
+ nbits = 3;
+
+ };
+ ...
+ };
+
+= Data consumers =
+Are device nodes which consume nvmem data cells/providers.
+
+Required-properties:
+nvmem-cell: list of phandle to the nvmem data cells.
+nvmem-cell-names: names for the each nvmem-cell specified
+
+Optional-properties:
+nvmem : list of phandles to nvmem providers.
+nvmem-names: names for the each nvmem provider.
+
+For example:
+
+ tsens {
+ ...
+ nvmem-cell = <&tsens_calibration>;
+ nvmem-cell-names = "calibration";
+ };
--
1.9.1
This patch adds probe and remove helper functions for nvmems which are
mmio based, With these helper function new nvmem consumer drivers need
very little code add its driver.
This code is currently used for qfprom and sunxi-sid nvmem consumer
drivers.
Signed-off-by: Srinivas Kandagatla <[email protected]>
---
drivers/nvmem/Makefile | 1 +
drivers/nvmem/nvmem-mmio.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++
drivers/nvmem/nvmem-mmio.h | 41 +++++++++++++++++++++++++++
3 files changed, 111 insertions(+)
create mode 100644 drivers/nvmem/nvmem-mmio.c
create mode 100644 drivers/nvmem/nvmem-mmio.h
diff --git a/drivers/nvmem/Makefile b/drivers/nvmem/Makefile
index 6df2c69..f694cfc 100644
--- a/drivers/nvmem/Makefile
+++ b/drivers/nvmem/Makefile
@@ -4,3 +4,4 @@
obj-$(CONFIG_NVMEM) += nvmem_core.o
nvmem_core-y := core.o
+nvmem_core-y += nvmem-mmio.o
diff --git a/drivers/nvmem/nvmem-mmio.c b/drivers/nvmem/nvmem-mmio.c
new file mode 100644
index 0000000..0d8131f
--- /dev/null
+++ b/drivers/nvmem/nvmem-mmio.c
@@ -0,0 +1,69 @@
+/*
+ * Copyright (C) 2015 Srinivas Kandagatla <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * 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/module.h>
+#include <linux/err.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include "nvmem-mmio.h"
+
+int nvmem_mmio_remove(struct platform_device *pdev)
+{
+ struct nvmem_device *nvmem = platform_get_drvdata(pdev);
+
+ return nvmem_unregister(nvmem);
+}
+EXPORT_SYMBOL_GPL(nvmem_mmio_remove);
+
+int nvmem_mmio_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct resource *res;
+ const struct nvmem_mmio_data *data;
+ struct nvmem_device *nvmem;
+ struct regmap *regmap;
+ const struct of_device_id *match;
+ void __iomem *base;
+
+ if (!dev || !dev->driver)
+ return -ENODEV;
+
+ match = of_match_device(dev->driver->of_match_table, dev);
+ if (!match || !match->data)
+ return -EINVAL;
+
+ data = match->data;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ base = devm_ioremap_resource(dev, res);
+ if (IS_ERR(base))
+ return PTR_ERR(base);
+
+ data->regmap_config->max_register = resource_size(res) - 1;
+
+ regmap = devm_regmap_init_mmio(dev, base, data->regmap_config);
+ if (IS_ERR(regmap)) {
+ dev_err(dev, "regmap init failed\n");
+ return PTR_ERR(regmap);
+ }
+ data->nvmem_config->dev = dev;
+ nvmem = nvmem_register(data->nvmem_config);
+ if (IS_ERR(nvmem))
+ return PTR_ERR(nvmem);
+
+ platform_set_drvdata(pdev, nvmem);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(nvmem_mmio_probe);
diff --git a/drivers/nvmem/nvmem-mmio.h b/drivers/nvmem/nvmem-mmio.h
new file mode 100644
index 0000000..a2ad4e5
--- /dev/null
+++ b/drivers/nvmem/nvmem-mmio.h
@@ -0,0 +1,41 @@
+/*
+ * MMIO based nvmem providers.
+ *
+ * Copyright (C) 2015 Srinivas Kandagatla <[email protected]>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#ifndef _LINUX_NVMEM_MMIO_H
+#define _LINUX_NVMEM_MMIO_H
+
+#include <linux/platform_device.h>
+#include <linux/nvmem-provider.h>
+#include <linux/regmap.h>
+
+struct nvmem_mmio_data {
+ struct regmap_config *regmap_config;
+ struct nvmem_config *nvmem_config;
+};
+
+#if IS_ENABLED(CONFIG_NVMEM)
+
+int nvmem_mmio_probe(struct platform_device *pdev);
+int nvmem_mmio_remove(struct platform_device *pdev);
+
+#else
+
+static inline int nvmem_mmio_probe(struct platform_device *pdev)
+{
+ return -ENOSYS;
+}
+
+static inline int nvmem_mmio_remove(struct platform_device *pdev)
+{
+ return -ENOSYS;
+}
+#endif
+
+#endif /* ifndef _LINUX_NVMEM_MMIO_H */
--
1.9.1
This patch adds QFPROM support driver which is used by other drivers
like thermal sensor and cpufreq.
On MSM parts there are some efuses (called qfprom) these fuses store
things like calibration data, speed bins.. etc. Drivers like cpufreq,
thermal sensors would read out this data for configuring the driver.
Signed-off-by: Srinivas Kandagatla <[email protected]>
---
drivers/nvmem/Kconfig | 15 +++++++++++++++
drivers/nvmem/Makefile | 4 ++++
drivers/nvmem/qfprom.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 70 insertions(+)
create mode 100644 drivers/nvmem/qfprom.c
diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
index f157b6d..e665e23 100644
--- a/drivers/nvmem/Kconfig
+++ b/drivers/nvmem/Kconfig
@@ -8,3 +8,18 @@ menuconfig NVMEM
from both the Linux Kernel and the userspace.
If unsure, say no.
+
+if NVMEM
+
+config QCOM_QFPROM
+ tristate "QCOM QFPROM Support"
+ depends on ARCH_QCOM
+ select REGMAP_MMIO
+ help
+ Say y here to enable QFPROM support. The QFPROM provides access
+ functions for QFPROM data to rest of the drivers via nvmem interface.
+
+ This driver can also be built as a module. If so, the module
+ will be called nvmem-qfprom.
+
+endif
diff --git a/drivers/nvmem/Makefile b/drivers/nvmem/Makefile
index f694cfc..caea611 100644
--- a/drivers/nvmem/Makefile
+++ b/drivers/nvmem/Makefile
@@ -5,3 +5,7 @@
obj-$(CONFIG_NVMEM) += nvmem_core.o
nvmem_core-y := core.o
nvmem_core-y += nvmem-mmio.o
+
+# Devices
+obj-$(CONFIG_QCOM_QFPROM) += nvmem_qfprom.o
+nvmem_qfprom-y := qfprom.o
diff --git a/drivers/nvmem/qfprom.c b/drivers/nvmem/qfprom.c
new file mode 100644
index 0000000..5ea84bb
--- /dev/null
+++ b/drivers/nvmem/qfprom.c
@@ -0,0 +1,51 @@
+/*
+ * Copyright (C) 2015 Srinivas Kandagatla <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * 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/module.h>
+#include <linux/of.h>
+#include "nvmem-mmio.h"
+
+static struct regmap_config qfprom_regmap_config = {
+ .reg_bits = 32,
+ .val_bits = 8,
+ .reg_stride = 1,
+};
+
+static struct nvmem_config econfig = {
+ .name = "qfprom",
+ .owner = THIS_MODULE,
+};
+
+static struct nvmem_mmio_data qfprom_data = {
+ .nvmem_config = &econfig,
+ .regmap_config = &qfprom_regmap_config,
+};
+
+static const struct of_device_id qfprom_of_match[] = {
+ { .compatible = "qcom,qfprom", .data = &qfprom_data},
+ {/* sentinel */},
+};
+MODULE_DEVICE_TABLE(of, qfprom_of_match);
+
+static struct platform_driver qfprom_driver = {
+ .probe = nvmem_mmio_probe,
+ .remove = nvmem_mmio_remove,
+ .driver = {
+ .name = "qcom,qfprom",
+ .of_match_table = qfprom_of_match,
+ },
+};
+module_platform_driver(qfprom_driver);
+MODULE_AUTHOR("Srinivas Kandagatla <[email protected]>");
+MODULE_DESCRIPTION("Qualcomm QFPROM driver");
+MODULE_LICENSE("GPL v2");
--
1.9.1
This patch adds bindings for qfprom found in QCOM SOCs. QFPROM driver
is based on simple nvmem framework.
Signed-off-by: Srinivas Kandagatla <[email protected]>
---
Documentation/devicetree/bindings/nvmem/qfprom.txt | 35 ++++++++++++++++++++++
1 file changed, 35 insertions(+)
create mode 100644 Documentation/devicetree/bindings/nvmem/qfprom.txt
diff --git a/Documentation/devicetree/bindings/nvmem/qfprom.txt b/Documentation/devicetree/bindings/nvmem/qfprom.txt
new file mode 100644
index 0000000..8a8d55f
--- /dev/null
+++ b/Documentation/devicetree/bindings/nvmem/qfprom.txt
@@ -0,0 +1,35 @@
+= Qualcomm QFPROM device tree bindings =
+
+This binding is intended to represent QFPROM which is found in most QCOM SOCs.
+
+Required properties:
+- compatible: should be "qcom,qfprom"
+- reg: Should contain registers location and length
+
+= Data cells =
+Are child nodes of qfprom, bindings of which as described in
+bindings/nvmem/nvmem.txt
+
+Example:
+
+ qfprom: qfprom@00700000 {
+ compatible = "qcom,qfprom";
+ reg = <0x00700000 0x8000>;
+ ...
+ /* Data cells */
+ tsens_calibration: calib@404 {
+ reg = <0x4404 0x10>;
+ };
+ };
+
+
+= Data consumers =
+Are device nodes which consume nvmem data cells.
+
+For example:
+
+ tsens {
+ ...
+ nvmem-cell = <&tsens_calibration>;
+ nvmem-cell-names = "calibration";
+ };
--
1.9.1
This patch adds MAINTAINERS to nvmem framework.
Signed-off-by: Srinivas Kandagatla <[email protected]>
---
MAINTAINERS | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index b2ef613..26e1829 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6993,6 +6993,15 @@ S: Supported
F: drivers/block/nvme*
F: include/linux/nvme.h
+NVMEM FRAMEWORK
+M: Srinivas Kandagatla <[email protected]>
+M: Maxime Ripard <[email protected]>
+S: Maintained
+F: drivers/nvmem/
+F: Documentation/devicetree/bindings/nvmem/
+F: include/linux/nvmem-provider.h
+F: include/linux/nvmem-consumer.h
+
NXP-NCI NFC DRIVER
M: Clément Perrochaud <[email protected]>
R: Charles Gorand <[email protected]>
--
1.9.1
From: Maxime Ripard <[email protected]>
Now that we have the nvmem framework, we can consolidate the common driver
code. Move the driver to the framework, and hopefully, it will fix the sysfs
file creation race.
Signed-off-by: Maxime Ripard <[email protected]>
[srinivas.kandagatla: Moved to regmap based EEPROM framework]
Signed-off-by: Srinivas Kandagatla <[email protected]>
---
Documentation/ABI/testing/sysfs-driver-sunxi-sid | 22 ---
.../bindings/misc/allwinner,sunxi-sid.txt | 17 ---
.../bindings/nvmem/allwinner,sunxi-sid.txt | 21 +++
drivers/misc/eeprom/Kconfig | 13 --
drivers/misc/eeprom/Makefile | 1 -
drivers/misc/eeprom/sunxi_sid.c | 156 ---------------------
drivers/nvmem/Kconfig | 11 ++
drivers/nvmem/Makefile | 2 +
drivers/nvmem/sunxi-sid.c | 64 +++++++++
9 files changed, 98 insertions(+), 209 deletions(-)
delete mode 100644 Documentation/ABI/testing/sysfs-driver-sunxi-sid
delete mode 100644 Documentation/devicetree/bindings/misc/allwinner,sunxi-sid.txt
create mode 100644 Documentation/devicetree/bindings/nvmem/allwinner,sunxi-sid.txt
delete mode 100644 drivers/misc/eeprom/sunxi_sid.c
create mode 100644 drivers/nvmem/sunxi-sid.c
diff --git a/Documentation/ABI/testing/sysfs-driver-sunxi-sid b/Documentation/ABI/testing/sysfs-driver-sunxi-sid
deleted file mode 100644
index ffb9536..0000000
--- a/Documentation/ABI/testing/sysfs-driver-sunxi-sid
+++ /dev/null
@@ -1,22 +0,0 @@
-What: /sys/devices/*/<our-device>/eeprom
-Date: August 2013
-Contact: Oliver Schinagl <[email protected]>
-Description: read-only access to the SID (Security-ID) on current
- A-series SoC's from Allwinner. Currently supports A10, A10s, A13
- and A20 CPU's. The earlier A1x series of SoCs exports 16 bytes,
- whereas the newer A20 SoC exposes 512 bytes split into sections.
- Besides the 16 bytes of SID, there's also an SJTAG area,
- HDMI-HDCP key and some custom keys. Below a quick overview, for
- details see the user manual:
- 0x000 128 bit root-key (sun[457]i)
- 0x010 128 bit boot-key (sun7i)
- 0x020 64 bit security-jtag-key (sun7i)
- 0x028 16 bit key configuration (sun7i)
- 0x02b 16 bit custom-vendor-key (sun7i)
- 0x02c 320 bit low general key (sun7i)
- 0x040 32 bit read-control access (sun7i)
- 0x064 224 bit low general key (sun7i)
- 0x080 2304 bit HDCP-key (sun7i)
- 0x1a0 768 bit high general key (sun7i)
-Users: any user space application which wants to read the SID on
- Allwinner's A-series of CPU's.
diff --git a/Documentation/devicetree/bindings/misc/allwinner,sunxi-sid.txt b/Documentation/devicetree/bindings/misc/allwinner,sunxi-sid.txt
deleted file mode 100644
index fabdf64..0000000
--- a/Documentation/devicetree/bindings/misc/allwinner,sunxi-sid.txt
+++ /dev/null
@@ -1,17 +0,0 @@
-Allwinner sunxi-sid
-
-Required properties:
-- compatible: "allwinner,sun4i-a10-sid" or "allwinner,sun7i-a20-sid"
-- reg: Should contain registers location and length
-
-Example for sun4i:
- sid@01c23800 {
- compatible = "allwinner,sun4i-a10-sid";
- reg = <0x01c23800 0x10>
- };
-
-Example for sun7i:
- sid@01c23800 {
- compatible = "allwinner,sun7i-a20-sid";
- reg = <0x01c23800 0x200>
- };
diff --git a/Documentation/devicetree/bindings/nvmem/allwinner,sunxi-sid.txt b/Documentation/devicetree/bindings/nvmem/allwinner,sunxi-sid.txt
new file mode 100644
index 0000000..d543ed3
--- /dev/null
+++ b/Documentation/devicetree/bindings/nvmem/allwinner,sunxi-sid.txt
@@ -0,0 +1,21 @@
+Allwinner sunxi-sid
+
+Required properties:
+- compatible: "allwinner,sun4i-a10-sid" or "allwinner,sun7i-a20-sid"
+- reg: Should contain registers location and length
+
+= Data cells =
+Are child nodes of qfprom, bindings of which as described in
+bindings/nvmem/nvmem.txt
+
+Example for sun4i:
+ sid@01c23800 {
+ compatible = "allwinner,sun4i-a10-sid";
+ reg = <0x01c23800 0x10>
+ };
+
+Example for sun7i:
+ sid@01c23800 {
+ compatible = "allwinner,sun7i-a20-sid";
+ reg = <0x01c23800 0x200>
+ };
diff --git a/drivers/misc/eeprom/Kconfig b/drivers/misc/eeprom/Kconfig
index 9536852f..04f2e1f 100644
--- a/drivers/misc/eeprom/Kconfig
+++ b/drivers/misc/eeprom/Kconfig
@@ -96,17 +96,4 @@ config EEPROM_DIGSY_MTC_CFG
If unsure, say N.
-config EEPROM_SUNXI_SID
- tristate "Allwinner sunxi security ID support"
- depends on ARCH_SUNXI && SYSFS
- help
- This is a driver for the 'security ID' available on various Allwinner
- devices.
-
- Due to the potential risks involved with changing e-fuses,
- this driver is read-only.
-
- This driver can also be built as a module. If so, the module
- will be called sunxi_sid.
-
endmenu
diff --git a/drivers/misc/eeprom/Makefile b/drivers/misc/eeprom/Makefile
index 9507aec..fc1e81d 100644
--- a/drivers/misc/eeprom/Makefile
+++ b/drivers/misc/eeprom/Makefile
@@ -4,5 +4,4 @@ obj-$(CONFIG_EEPROM_LEGACY) += eeprom.o
obj-$(CONFIG_EEPROM_MAX6875) += max6875.o
obj-$(CONFIG_EEPROM_93CX6) += eeprom_93cx6.o
obj-$(CONFIG_EEPROM_93XX46) += eeprom_93xx46.o
-obj-$(CONFIG_EEPROM_SUNXI_SID) += sunxi_sid.o
obj-$(CONFIG_EEPROM_DIGSY_MTC_CFG) += digsy_mtc_eeprom.o
diff --git a/drivers/misc/eeprom/sunxi_sid.c b/drivers/misc/eeprom/sunxi_sid.c
deleted file mode 100644
index 8385177..0000000
--- a/drivers/misc/eeprom/sunxi_sid.c
+++ /dev/null
@@ -1,156 +0,0 @@
-/*
- * Copyright (c) 2013 Oliver Schinagl <[email protected]>
- * http://www.linux-sunxi.org
- *
- * 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.
- *
- * 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.
- *
- * This driver exposes the Allwinner security ID, efuses exported in byte-
- * sized chunks.
- */
-
-#include <linux/compiler.h>
-#include <linux/device.h>
-#include <linux/err.h>
-#include <linux/export.h>
-#include <linux/fs.h>
-#include <linux/io.h>
-#include <linux/kernel.h>
-#include <linux/kobject.h>
-#include <linux/module.h>
-#include <linux/of_device.h>
-#include <linux/platform_device.h>
-#include <linux/random.h>
-#include <linux/slab.h>
-#include <linux/stat.h>
-#include <linux/sysfs.h>
-#include <linux/types.h>
-
-#define DRV_NAME "sunxi-sid"
-
-struct sunxi_sid_data {
- void __iomem *reg_base;
- unsigned int keysize;
-};
-
-/* We read the entire key, due to a 32 bit read alignment requirement. Since we
- * want to return the requested byte, this results in somewhat slower code and
- * uses 4 times more reads as needed but keeps code simpler. Since the SID is
- * only very rarely probed, this is not really an issue.
- */
-static u8 sunxi_sid_read_byte(const struct sunxi_sid_data *sid_data,
- const unsigned int offset)
-{
- u32 sid_key;
-
- if (offset >= sid_data->keysize)
- return 0;
-
- sid_key = ioread32be(sid_data->reg_base + round_down(offset, 4));
- sid_key >>= (offset % 4) * 8;
-
- return sid_key; /* Only return the last byte */
-}
-
-static ssize_t sid_read(struct file *fd, struct kobject *kobj,
- struct bin_attribute *attr, char *buf,
- loff_t pos, size_t size)
-{
- struct platform_device *pdev;
- struct sunxi_sid_data *sid_data;
- int i;
-
- pdev = to_platform_device(kobj_to_dev(kobj));
- sid_data = platform_get_drvdata(pdev);
-
- if (pos < 0 || pos >= sid_data->keysize)
- return 0;
- if (size > sid_data->keysize - pos)
- size = sid_data->keysize - pos;
-
- for (i = 0; i < size; i++)
- buf[i] = sunxi_sid_read_byte(sid_data, pos + i);
-
- return i;
-}
-
-static struct bin_attribute sid_bin_attr = {
- .attr = { .name = "eeprom", .mode = S_IRUGO, },
- .read = sid_read,
-};
-
-static int sunxi_sid_remove(struct platform_device *pdev)
-{
- device_remove_bin_file(&pdev->dev, &sid_bin_attr);
- dev_dbg(&pdev->dev, "driver unloaded\n");
-
- return 0;
-}
-
-static const struct of_device_id sunxi_sid_of_match[] = {
- { .compatible = "allwinner,sun4i-a10-sid", .data = (void *)16},
- { .compatible = "allwinner,sun7i-a20-sid", .data = (void *)512},
- {/* sentinel */},
-};
-MODULE_DEVICE_TABLE(of, sunxi_sid_of_match);
-
-static int sunxi_sid_probe(struct platform_device *pdev)
-{
- struct sunxi_sid_data *sid_data;
- struct resource *res;
- const struct of_device_id *of_dev_id;
- u8 *entropy;
- unsigned int i;
-
- sid_data = devm_kzalloc(&pdev->dev, sizeof(struct sunxi_sid_data),
- GFP_KERNEL);
- if (!sid_data)
- return -ENOMEM;
-
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- sid_data->reg_base = devm_ioremap_resource(&pdev->dev, res);
- if (IS_ERR(sid_data->reg_base))
- return PTR_ERR(sid_data->reg_base);
-
- of_dev_id = of_match_device(sunxi_sid_of_match, &pdev->dev);
- if (!of_dev_id)
- return -ENODEV;
- sid_data->keysize = (int)of_dev_id->data;
-
- platform_set_drvdata(pdev, sid_data);
-
- sid_bin_attr.size = sid_data->keysize;
- if (device_create_bin_file(&pdev->dev, &sid_bin_attr))
- return -ENODEV;
-
- entropy = kzalloc(sizeof(u8) * sid_data->keysize, GFP_KERNEL);
- for (i = 0; i < sid_data->keysize; i++)
- entropy[i] = sunxi_sid_read_byte(sid_data, i);
- add_device_randomness(entropy, sid_data->keysize);
- kfree(entropy);
-
- dev_dbg(&pdev->dev, "loaded\n");
-
- return 0;
-}
-
-static struct platform_driver sunxi_sid_driver = {
- .probe = sunxi_sid_probe,
- .remove = sunxi_sid_remove,
- .driver = {
- .name = DRV_NAME,
- .of_match_table = sunxi_sid_of_match,
- },
-};
-module_platform_driver(sunxi_sid_driver);
-
-MODULE_AUTHOR("Oliver Schinagl <[email protected]>");
-MODULE_DESCRIPTION("Allwinner sunxi security id driver");
-MODULE_LICENSE("GPL");
diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
index e665e23..17f1a57 100644
--- a/drivers/nvmem/Kconfig
+++ b/drivers/nvmem/Kconfig
@@ -22,4 +22,15 @@ config QCOM_QFPROM
This driver can also be built as a module. If so, the module
will be called nvmem-qfprom.
+config NVMEM_SUNXI_SID
+ tristate "Allwinner SoCs SID support"
+ depends on ARCH_SUNXI
+ select REGMAP_MMIO
+ help
+ This is a driver for the 'security ID' available on various Allwinner
+ devices.
+
+ This driver can also be built as a module. If so, the module
+ will be called eeprom-sunxi-sid.
+
endif
diff --git a/drivers/nvmem/Makefile b/drivers/nvmem/Makefile
index caea611..cc46791 100644
--- a/drivers/nvmem/Makefile
+++ b/drivers/nvmem/Makefile
@@ -9,3 +9,5 @@ nvmem_core-y += nvmem-mmio.o
# Devices
obj-$(CONFIG_QCOM_QFPROM) += nvmem_qfprom.o
nvmem_qfprom-y := qfprom.o
+obj-$(CONFIG_NVMEM_SUNXI_SID) += nvmem-sunxi-sid.o
+nvmem-sunxi-sid-y := sunxi-sid.o
diff --git a/drivers/nvmem/sunxi-sid.c b/drivers/nvmem/sunxi-sid.c
new file mode 100644
index 0000000..5bfce35
--- /dev/null
+++ b/drivers/nvmem/sunxi-sid.c
@@ -0,0 +1,64 @@
+/*
+ * Allwinner sunXi SoCs Security ID support.
+ *
+ * Copyright (c) 2013 Oliver Schinagl <[email protected]>
+ * Copyright (C) 2014 Maxime Ripard <[email protected]>
+ *
+ * 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.
+ *
+ * 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/module.h>
+#include <linux/of.h>
+#include "nvmem-mmio.h"
+
+static bool sunxi_sid_writeable_reg(struct device *dev, unsigned int reg)
+{
+ return false;
+}
+
+static struct nvmem_config econfig = {
+ .name = "sunix-sid",
+ .owner = THIS_MODULE,
+};
+
+static struct regmap_config sunxi_sid_regmap_config = {
+ .reg_bits = 32,
+ .val_bits = 32,
+ .reg_stride = 4,
+ .writeable_reg = sunxi_sid_writeable_reg,
+};
+
+static struct nvmem_mmio_data sunxi_data = {
+ .nvmem_config = &econfig,
+ .regmap_config = &sunxi_sid_regmap_config,
+};
+
+static const struct of_device_id sunxi_sid_of_match[] = {
+ { .compatible = "allwinner,sun4i-a10-sid", .data = &sunxi_data},
+ { .compatible = "allwinner,sun7i-a20-sid", .data = &sunxi_data},
+ {/* sentinel */},
+};
+MODULE_DEVICE_TABLE(of, sunxi_sid_of_match);
+
+static struct platform_driver sunxi_sid_driver = {
+ .probe = nvmem_mmio_probe,
+ .remove = nvmem_mmio_remove,
+ .driver = {
+ .name = "eeprom-sunxi-sid",
+ .of_match_table = sunxi_sid_of_match,
+ },
+};
+module_platform_driver(sunxi_sid_driver);
+
+MODULE_AUTHOR("Oliver Schinagl <[email protected]>");
+MODULE_DESCRIPTION("Allwinner sunxi security id driver");
+MODULE_LICENSE("GPL");
--
1.9.1
On Thu, May 21, 2015 at 05:42:43PM +0100, Srinivas Kandagatla wrote:
> This patch introduces regmap_get_max_register() function which would be
> used by the infrastructures like nvmem framework built on top of
> regmap.
Applied, thanks. If the rest of the framework is ready to get merged
somewhere before it reaches Linus' tree let me know and we can handle it
then.
On Thu, May 21, 2015 at 05:42:54PM +0100, Srinivas Kandagatla wrote:
> This patch introduces regmap_get_reg_stride() function which would
> be used by the infrastructures like nvmem framework built on top of
> regmap. Mostly this function would be used for sanity checks on inputs
> within such infrastructure.
Applied, thanks.
Hi Srinivas,
> On May 21, 2015, at 19:42 , Srinivas Kandagatla <[email protected]> wrote:
>
> Thankyou all for providing inputs and comments on previous versions of this patchset.
> Here is the v5 of the patchset addressing all the issues raised as
> part of previous versions review.
>
>
[snip]
I tried to use the updated patchset with my at24 & beaglebone capemanager patches.
I have a big problem with the removal of the raw of_* access APIs.
Take for instance the case where you have multiple slot accessing different EEPROMs.
> slots {
> slot@0 {
> eeprom = <&cape0_data>;
> };
>
> slot@1 {
> eeprom = <&cape1_data>;
> };
> };
In that case there is no per-device node mapping; it’s a per-sub node.
For now I’m exporting the of_* accessors again, please consider exposing the of_* API again.
> --
> 1.9.1
>
Regards
— Pantelis
Hi Pantelis,
On 25/05/15 17:51, Pantelis Antoniou wrote:
> Hi Srinivas,
>
>> On May 21, 2015, at 19:42 , Srinivas Kandagatla <[email protected]> wrote:
>>
>> Thankyou all for providing inputs and comments on previous versions of this patchset.
>> Here is the v5 of the patchset addressing all the issues raised as
>> part of previous versions review.
>>
>
>>
>
> [snip]
>
> I tried to use the updated patchset with my at24 & beaglebone capemanager patches.
Thanks for trying it out and migrating at24 to it.
>
> I have a big problem with the removal of the raw of_* access APIs.
Ok,
>
> Take for instance the case where you have multiple slot accessing different EEPROMs.
>
>> slots {
>> slot@0 {
>> eeprom = <&cape0_data>;
>> };
>>
>> slot@1 {
>> eeprom = <&cape1_data>;
>> };
>> };
Can I ask you why should the slots be in sub-nodes?
Do you expect to have more properties associated with each slot in future?
Or is it just to get hold of eeprom data?
>
> In that case there is no per-device node mapping; it’s a per-sub node.
>
> For now I’m exporting the of_* accessors again, please consider exposing the of_* API again.
Sure, we can export of_nvmem_cell_get symbol for usecases like this.
Having said that, I got one comment on the way the nvmem is used in your
case. You should try to use nvmem_device_get() and then use
nvmem_device_read() apis, These apis are for consumers like this one.
The advantage of this would be you do not need read and store all data
in the driver and parse them internally. Basically your ee_field_get
would just do nvmem_device_read(); Does it make sense?
We can work on how to get the of_*based once you decide to move to this api.
--srini
>
>> --
>> 1.9.1
>>
>
> Regards
>
> — Pantelis
>
Hi Srinivas,
> On May 26, 2015, at 12:12 , Srinivas Kandagatla <[email protected]> wrote:
>
> Hi Pantelis,
>
> On 25/05/15 17:51, Pantelis Antoniou wrote:
>> Hi Srinivas,
>>
>>> On May 21, 2015, at 19:42 , Srinivas Kandagatla <[email protected]> wrote:
>>>
>>> Thankyou all for providing inputs and comments on previous versions of this patchset.
>>> Here is the v5 of the patchset addressing all the issues raised as
>>> part of previous versions review.
>>>
>>
>>>
>>
>> [snip]
>>
>> I tried to use the updated patchset with my at24 & beaglebone capemanager patches.
> Thanks for trying it out and migrating at24 to it.
>
Don’t mention it…
>>
>> I have a big problem with the removal of the raw of_* access APIs.
> Ok,
>>
>> Take for instance the case where you have multiple slot accessing different EEPROMs.
>>
>>> slots {
>>> slot@0 {
>>> eeprom = <&cape0_data>;
>>> };
>>>
>>> slot@1 {
>>> eeprom = <&cape1_data>;
>>> };
>>> };
>
> Can I ask you why should the slots be in sub-nodes?
> Do you expect to have more properties associated with each slot in future?
> Or is it just to get hold of eeprom data?
>
For now I don’t have any more properties besides the eeprom phandle.
I’ve reworked capemanager to work with the API as it is, but it’s not very intuitive IMHO.
The problem is that I have both the baseboard and the slot eeproms in a single property list.
If more per-slot properties are required, I’ll have to add again the slots node and
then the nvmem eeprom handles would stick out like a sore thumb.
>>
>> In that case there is no per-device node mapping; it’s a per-sub node.
>>
>> For now I’m exporting the of_* accessors again, please consider exposing the of_* API again.
> Sure, we can export of_nvmem_cell_get symbol for usecases like this.
>
> Having said that, I got one comment on the way the nvmem is used in your case. You should try to use nvmem_device_get() and then use nvmem_device_read() apis, These apis are for consumers like this one. The advantage of this would be you do not need read and store all data in the driver and parse them internally. Basically your ee_field_get would just do nvmem_device_read(); Does it make sense?
>
Hmm, good idea; I’ll give it a shot.
> We can work on how to get the of_*based once you decide to move to this api.
>
>
OK, thanks a lot for taking the time to think about this Srinivas.
>
> --srini
>>
Regards
— Pantelis
>>> --
>>> 1.9.1
>>>
>>
>> Regards
>>
>> — Pantelis
>>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
On 05/21/2015 09:43 AM, Srinivas Kandagatla wrote:
> @@ -379,6 +380,351 @@ int nvmem_unregister(struct nvmem_device *nvmem)
[...]
> +
> + return nvmem;
> +}
> +
> +static int __nvmem_device_put(struct nvmem_device *nvmem)
Why does this return int? It's not used anywhere.
> +{
> + module_put(nvmem->owner);
> + mutex_lock(&nvmem_mutex);
> + nvmem->users--;
> + mutex_unlock(&nvmem_mutex);
> +
> + return 0;
> +}
> +
> +static struct nvmem_cell *nvmem_cell_get_from_list(const char *cell_id)
> +{
> + struct nvmem_cell *cell = NULL;
> + struct nvmem_device *nvmem;
> +
> + nvmem = __nvmem_device_get(NULL, &cell, cell_id);
> + if (IS_ERR(nvmem))
> + return (struct nvmem_cell *)nvmem;
ERR_CAST?
> +
> +
> + return cell;
> +
> +}
> +
> +static struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
> + const char *name)
> +{
> + struct device_node *cell_np, *nvmem_np;
> + struct nvmem_cell *cell;
> + struct nvmem_device *nvmem;
> + const __be32 *addr;
> + int rval, len, index;
> +
> + index = of_property_match_string(np, "nvmem-cell-names", name);
> +
> + cell_np = of_parse_phandle(np, "nvmem-cell", index);
> + if (!cell_np)
> + return ERR_PTR(-EINVAL);
> +
> + nvmem_np = of_get_next_parent(cell_np);
> + if (!nvmem_np)
> + return ERR_PTR(-EINVAL);
> +
> + nvmem = __nvmem_device_get(nvmem_np, NULL, NULL);
> + if (IS_ERR(nvmem))
> + return (struct nvmem_cell *)nvmem;
ERR_CAST?
> +
> + addr = of_get_property(cell_np, "reg", &len);
> + if (!addr || (len < 2 * sizeof(int))) {
> + dev_err(&nvmem->dev, "of_i2c: invalid reg on %s\n",
huh? of_i2c?
> +
> + /* if it's not end on byte boundary */
> + if ((nbits + bit_offset) % BITS_PER_BYTE) {
> + /* setup the last byte with msb bits from nvmem */
> + rc = regmap_raw_read(nvmem->regmap,
> + cell->offset + cell->bytes - 1, &v, 1);
> + *p |= GENMASK(7, (nbits + bit_offset) % BITS_PER_BYTE) & v;
> +
> + }
> +
> + return buf;
> +}
> +
> +/**
> + * nvmem_cell_write(): Write to a given nvmem cell
This isn't kernel doc notation. It should be like
nvmem_cell_write - Write to a given nvmem cell
> + *
> + * @cell: nvmem cell to be written.
> + * @buf: Buffer to be written.
> + * @len: length of buffer to be written to nvmem cell.
> + *
> + * The return value will be an length of bytes written or non zero on failure.
> + */
> +int nvmem_cell_write(struct nvmem_cell *cell, void *buf, ssize_t len)
[..]
> +
> static int nvmem_init(void)
> {
> return class_register(&nvmem_class);
> diff --git a/include/linux/nvmem-consumer.h b/include/linux/nvmem-consumer.h
> new file mode 100644
> index 0000000..c3fa8c7
> --- /dev/null
> +++ b/include/linux/nvmem-consumer.h
> @@ -0,0 +1,49 @@
> +/*
> + * nvmem framework consumer.
> + *
> + * Copyright (C) 2015 Srinivas Kandagatla <[email protected]>
> + * Copyright (C) 2013 Maxime Ripard <[email protected]>
> + *
> + * This file is licensed under the terms of the GNU General Public
> + * License version 2. This program is licensed "as is" without any
> + * warranty of any kind, whether express or implied.
> + */
> +
> +#ifndef _LINUX_NVMEM_CONSUMER_H
> +#define _LINUX_NVMEM_CONSUMER_H
> +
> +/* consumer cookie */
> +struct nvmem_cell;
> +
> +#if IS_ENABLED(CONFIG_NVMEM)
> +
> +/* Cell based interface */
> +struct nvmem_cell *nvmem_cell_get(struct device *dev, const char *name);
We should probably forward declare struct device in this file too.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
On 05/21/2015 09:43 AM, Srinivas Kandagatla wrote:
>
> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
> new file mode 100644
> index 0000000..6c2f0b1
> --- /dev/null
> +++ b/drivers/nvmem/core.c
> @@ -0,0 +1,398 @@
> +/*
> + * nvmem framework core.
> + *
> + * Copyright (C) 2015 Srinivas Kandagatla <[email protected]>
> + * Copyright (C) 2013 Maxime Ripard <[email protected]>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * 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/nvmem-provider.h>
> +#include <linux/export.h>
> +#include <linux/fs.h>
> +#include <linux/idr.h>
> +#include <linux/init.h>
> +#include <linux/regmap.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/slab.h>
> +#include <linux/uaccess.h>
Is this include used?
> +
> +static int of_nvmem_match(struct device *dev, const void *nvmem_np)
> +{
> + return dev->of_node == nvmem_np;
> +}
> +
> +static struct nvmem_device *of_nvmem_find(struct device_node *nvmem_np)
const?
> +{
> + struct device *d;
> +
> + if (!nvmem_np)
> + return NULL;
> +
> + d = class_find_device(&nvmem_class, NULL, nvmem_np, of_nvmem_match);
> +
> + return d ? to_nvmem_device(d) : NULL;
> +}
> +
> +static struct nvmem_cell *nvmem_find_cell(const char *cell_id)
> +{
> + struct nvmem_cell *p;
> +
> + list_for_each_entry(p, &nvmem_cells, node) {
Unnecessary braces.
> + if (p && !strcmp(p->name, cell_id))
> + return p;
> + }
> +
> + return NULL;
> +}
> +
> +static void nvmem_cell_drop(struct nvmem_cell *cell)
> +{
> + mutex_lock(&nvmem_cells_mutex);
> + list_del(&cell->node);
> + mutex_unlock(&nvmem_cells_mutex);
> + kfree(cell);
> +}
> +
> +static void nvmem_device_remove_all_cells(struct nvmem_device *nvmem)
> +{
> + struct nvmem_cell *cell = NULL;
Unnecessary initialization
> + struct list_head *p, *n;
> +
> + list_for_each_safe(p, n, &nvmem_cells) {
> + cell = list_entry(p, struct nvmem_cell, node);
> + if (cell->nvmem == nvmem)
> + nvmem_cell_drop(cell);
> + }
[..]
> +
> +static int nvmem_add_cells(struct nvmem_device *nvmem,
> + struct nvmem_config *cfg)
> +{
> + struct nvmem_cell **cells;
> + struct nvmem_cell_info *info = cfg->cells;
> + int i, rval;
> +
> + cells = kzalloc(sizeof(*cells) * cfg->ncells, GFP_KERNEL);
kcalloc
> + if (!cells)
> + return -ENOMEM;
> +
> + for (i = 0; i < cfg->ncells; i++) {
> + cells[i] = kzalloc(sizeof(struct nvmem_cell), GFP_KERNEL);
sizeof(**cells) ?
> + if (!cells[i]) {
> + rval = -ENOMEM;
> + goto err;
> + }
> +
> + rval = nvmem_cell_info_to_nvmem_cell(nvmem, &info[i], cells[i]);
> + if (IS_ERR_VALUE(rval)) {
> + kfree(cells[i]);
> + goto err;
> + }
> +
> + nvmem_cell_add(cells[i]);
> + }
> +
> + nvmem->ncells = cfg->ncells;
> + /* remove tmp array */
> + kfree(cells);
> +
> + return 0;
> +err:
> + while (--i)
> + nvmem_cell_drop(cells[i]);
> +
> + return rval;
> +}
> +
> +/**
> + * nvmem_register(): Register a nvmem device for given nvmem.
> + * Also creates an binary entry in /sys/class/nvmem/dev-name/nvmem
> + *
> + * @nvmem: nvmem device that needs to be created
You mean @config?
> + *
> + * The return value will be an ERR_PTR() on error or a valid pointer
> + * to nvmem_device.
> + */
> +
> +struct nvmem_device *nvmem_register(struct nvmem_config *config)
> +{
> + struct nvmem_device *nvmem;
> + struct regmap *rm;
> + int rval;
> +
> + if (!config->dev)
> + return ERR_PTR(-EINVAL);
> +
> + rm = dev_get_regmap(config->dev, NULL);
> + if (!rm) {
> + dev_err(config->dev, "Regmap not found\n");
> + return ERR_PTR(-EINVAL);
> + }
> +
> + nvmem = kzalloc(sizeof(*nvmem), GFP_KERNEL);
> + if (!nvmem)
> + return ERR_PTR(-ENOMEM);
> +
> + nvmem->id = ida_simple_get(&nvmem_ida, 0, 0, GFP_KERNEL);
> + if (nvmem->id < 0) {
> + kfree(nvmem);
> + return ERR_PTR(nvmem->id);
> + }
> +
> + nvmem->regmap = rm;
> + nvmem->owner = config->owner;
> + nvmem->stride = regmap_get_reg_stride(rm);
> + nvmem->word_size = regmap_get_val_bytes(rm);
> + nvmem->size = regmap_get_max_register(rm) + nvmem->stride;
> + nvmem->dev.class = &nvmem_class;
> + nvmem->dev.parent = config->dev;
> + nvmem->dev.of_node = config->dev->of_node;
> + dev_set_name(&nvmem->dev, "%s%d",
> + config->name ? : "nvmem", config->id);
It may be better to always name it nvmem%d so that we don't allow the
possibility of conflicts.
> +
> + nvmem->read_only = of_property_read_bool(nvmem->dev.of_node,
> + "read-only");
What if we're not using DT? How would we specify read_only?
> +
> + device_initialize(&nvmem->dev);
> +
> + dev_dbg(&nvmem->dev, "Registering nvmem device %s\n",
> + dev_name(&nvmem->dev));
> +
> + rval = device_add(&nvmem->dev);
> + if (rval) {
> + ida_simple_remove(&nvmem_ida, nvmem->id);
> + kfree(nvmem);
> + return ERR_PTR(rval);
> + }
> +
> + /* update sysfs attributes */
> + if (nvmem->read_only)
> + sysfs_update_group(&nvmem->dev.kobj, &nvmem_bin_ro_group);
It would be nice if this could be done before the device was registered.
Perhaps have two device_types, one for read-only and one for read/write?
> +
> + if (config->cells)
> + nvmem_add_cells(nvmem, config);
> +
> + return nvmem;
> +}
> +EXPORT_SYMBOL_GPL(nvmem_register);
> +
> +/**
> + * nvmem_unregister(): Unregister previously registered nvmem device
> + *
> + * @nvmem: Pointer to previously registered nvmem device.
> + *
> + * The return value will be an non zero on error or a zero on success.
> + */
> +int nvmem_unregister(struct nvmem_device *nvmem)
> +{
> + mutex_lock(&nvmem_mutex);
> + if (nvmem->users) {
> + mutex_unlock(&nvmem_mutex);
> + return -EBUSY;
Hmm... that doesn't seem nice. Typically when something is unregistered
we have to pull the rug out from underneath the users and start
returning errors to them. The provider needs to be free to unregister
because it's been forcibly removed. So really this function should
return void.
> + }
> + mutex_unlock(&nvmem_mutex);
> +
> + nvmem_device_remove_all_cells(nvmem);
> + device_del(&nvmem->dev);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(nvmem_unregister);
> +
> +static int nvmem_init(void)
__init? And __exit on nvmem_exit?
> +{
> + return class_register(&nvmem_class);
I thought class was on the way out? Aren't we supposed to use bus types
for new stuff?
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
On 05/21/2015 09:43 AM, Srinivas Kandagatla wrote:
> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
> index 8a4b358..68ee8d1 100644
> --- a/drivers/nvmem/core.c
> +++ b/drivers/nvmem/core.c
> @@ -436,6 +436,51 @@ static int __nvmem_device_put(struct nvmem_device *nvmem)
> return 0;
> }
>
> +static int nvmem_match(struct device *dev, const void *data)
> +{
> + return !strcmp(dev_name(dev), (const char *)data);
Unnecessary cast.
> +}
> +
> +static struct nvmem_device *nvmem_find(const char *name)
> +{
> + struct device *d;
> +
> + d = class_find_device(&nvmem_class, NULL, (void *)name, nvmem_match);
Unnecessary cast
> +
> + return d ? to_nvmem_device(d) : NULL;
> +}
[...]
> +
> +void nvmem_device_put(struct nvmem_device *nvmem)
> +{
> + __nvmem_device_put(nvmem);
> +}
> +EXPORT_SYMBOL_GPL(nvmem_device_put);
>
> +int nvmem_device_cell_read(struct nvmem_device *nvmem,
> + struct nvmem_cell_info *info, void *buf)
> +{
> +}
> +EXPORT_SYMBOL_GPL(nvmem_device_cell_read);
> +
> +int nvmem_device_cell_write(struct nvmem_device *nvmem,
> + struct nvmem_cell_info *info, void *buf)
> +{
> +}
> +EXPORT_SYMBOL_GPL(nvmem_device_cell_write);
> +
> +int nvmem_device_read(struct nvmem_device *nvmem,
> + unsigned int offset,
> + size_t bytes, void *buf)
> +{
> +}
> +EXPORT_SYMBOL_GPL(nvmem_device_read);
> +
> +int nvmem_device_write(struct nvmem_device *nvmem,
> + unsigned int offset,
> + size_t bytes, void *buf)
> +{
> +}
> +EXPORT_SYMBOL_GPL(nvmem_device_write);
Can you please add kernel-doc on these exported APIs?
> +
> static int nvmem_init(void)
> {
> return class_register(&nvmem_class);
> diff --git a/include/linux/nvmem-consumer.h b/include/linux/nvmem-consumer.h
> index c3fa8c7..66c67ba 100644
> --- a/include/linux/nvmem-consumer.h
> +++ b/include/linux/nvmem-consumer.h
> @@ -23,6 +32,18 @@ void nvmem_cell_put(struct nvmem_cell *cell);
> void *nvmem_cell_read(struct nvmem_cell *cell, ssize_t *len);
> int nvmem_cell_write(struct nvmem_cell *cell, void *buf, ssize_t len);
>
> +/* direct nvmem device read/write interface */
> +struct nvmem_device *nvmem_device_get(struct device *dev, const char *name);
> +void nvmem_device_put(struct nvmem_device *nvmem);
> +int nvmem_device_read(struct nvmem_device *nvmem, unsigned int offset,
> + size_t bytes, void *buf);
> +int nvmem_device_write(struct nvmem_device *nvmem, unsigned int offset,
> + size_t bytes, void *buf);
> +int nvmem_device_cell_read(struct nvmem_device *nvmem,
> + struct nvmem_cell_info *info, void *buf);
> +int nvmem_device_cell_write(struct nvmem_device *nvmem,
> + struct nvmem_cell_info *info, void *buf);
> +
Can we also have devm_nvmem_*_get() APIs please?
> diff --git a/include/linux/nvmem-provider.h b/include/linux/nvmem-provider.h
> index 4908b37..7a982cd 100644
> --- a/include/linux/nvmem-provider.h
> +++ b/include/linux/nvmem-provider.h
> @@ -12,15 +12,9 @@
> #ifndef _LINUX_NVMEM_PROVIDER_H
> #define _LINUX_NVMEM_PROVIDER_H
>
> -struct nvmem_device;
> +#include <linux/nvmem-consumer.h>
>
> -struct nvmem_cell_info {
> - const char *name;
> - int offset;
> - int bytes;
> - int bit_offset;
> - int nbits;
> -};
> +struct nvmem_device;
Should this diff be part of an earlier patch?
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
On 05/21/2015 09:44 AM, Srinivas Kandagatla wrote:
> diff --git a/Documentation/devicetree/bindings/nvmem/nvmem.txt b/Documentation/devicetree/bindings/nvmem/nvmem.txt
> new file mode 100644
> index 0000000..ecea654
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/nvmem/nvmem.txt
> @@ -0,0 +1,84 @@
> += NVMEM Data Device Tree Bindings =
> +
> +This binding is intended to represent the location of hardware
> +configuration data stored in NVMEMs.
It would be worthwhile spelling out what NVMEM stands for.
> +
> +On a significant proportion of boards, the manufacturer has stored
> +some data on NVMEM, for the OS to be able to retrieve these information
> +and act upon it. Obviously, the OS has to know about where to retrieve
> +these data from, and where they are stored on the storage device.
> +
> +This document is here to document this.
> +
> += Data providers =
> +Contains bindings specific to provider drivers and data cells as children
> +to this node.
children of this node?
> +
> +Optional properties:
> + read-only: Mark the provider as read only.
> +
> += Data cells =
> +These are the child nodes of the provider which contain data cell
> +information like offset and size in nvmem provider.
> +
> +Required properties:
> +reg: specifies the offset in byte within that storage device, start bit
> + in the byte and the length in bits of the data we care about.
> + There could be more then one offset-length pairs in this property.
s/then/than/
> +
> +Optional properties:
> +
> +bit-offset: specifies the offset in bit within the address range specified
> + by reg property. Can take values from 0-7.
> +nbits: specifies number of bits this cell occupies starting from bit-offset.
> +
Hopefully the consumer knows the endianness of the data stored.
> +For example:
> +
> + /* Provider */
> + qfprom: qfprom@00700000 {
> + ...
> +
> + /* Data cells */
> + tsens_calibration: calib@404 {
> + reg = <0x404 0x10>;
> + };
> +
> + tsens_calibration_bckp: calib_bckp@504 {
> + reg = <0x504 0x11>;
> + bit-offset = 6;
> + nbits = 128;
> + };
> +
> + pvs_version: pvs-version@6 {
> + reg = <0x6 0x2>
> + bit-offset = 7;
> + nbits = 2;
> + };
> +
> + speed_bin: speed-bin@c{
> + reg = <0xc 0x1>;
> + bit-offset = 2;
> + nbits = 3;
> +
> + };
> + ...
> + };
> +
> += Data consumers =
> +Are device nodes which consume nvmem data cells/providers.
> +
> +Required-properties:
> +nvmem-cell: list of phandle to the nvmem data cells.
> +nvmem-cell-names: names for the each nvmem-cell specified
> +
> +Optional-properties:
> +nvmem : list of phandles to nvmem providers.
> +nvmem-names: names for the each nvmem provider.
Is nvmem-names required if nvmem is used?
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
On 05/21/2015 09:44 AM, Srinivas Kandagatla wrote:
> diff --git a/drivers/nvmem/nvmem-mmio.c b/drivers/nvmem/nvmem-mmio.c
> new file mode 100644
> index 0000000..0d8131f
> --- /dev/null
> +++ b/drivers/nvmem/nvmem-mmio.c
> @@ -0,0 +1,69 @@
> +/*
> + * Copyright (C) 2015 Srinivas Kandagatla <[email protected]>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * 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/module.h>
> +#include <linux/err.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
#include <linux/platform_device.h>
#include <linux/nvmem_provider.h>
#include <linux/regmap.h>
> +#include "nvmem-mmio.h"
> +
> +int nvmem_mmio_remove(struct platform_device *pdev)
> +{
> + struct nvmem_device *nvmem = platform_get_drvdata(pdev);
> +
> + return nvmem_unregister(nvmem);
> +}
> +EXPORT_SYMBOL_GPL(nvmem_mmio_remove);
> +
> +int nvmem_mmio_probe(struct platform_device *pdev)
> +{
> +
[...]
> +
> + platform_set_drvdata(pdev, nvmem);
It may be better to return the nvmem device instead so that the one
drvdata member is usable by the calling driver.
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(nvmem_mmio_probe);
Kernel-doc on these exported functions?
> diff --git a/drivers/nvmem/nvmem-mmio.h b/drivers/nvmem/nvmem-mmio.h
> new file mode 100644
> index 0000000..a2ad4e5
> --- /dev/null
> +++ b/drivers/nvmem/nvmem-mmio.h
> @@ -0,0 +1,41 @@
> +/*
> + * MMIO based nvmem providers.
> + *
> + * Copyright (C) 2015 Srinivas Kandagatla <[email protected]>
> + *
> + * This file is licensed under the terms of the GNU General Public
> + * License version 2. This program is licensed "as is" without any
> + * warranty of any kind, whether express or implied.
> + */
> +
> +#ifndef _LINUX_NVMEM_MMIO_H
> +#define _LINUX_NVMEM_MMIO_H
> +
> +#include <linux/platform_device.h>
Forward declare struct platform_device instead.
> +#include <linux/nvmem-provider.h>
Forward declare nvmem_config instead.
> +#include <linux/regmap.h>
Forward declare regmap_config instead.
> +
> +struct nvmem_mmio_data {
> + struct regmap_config *regmap_config;
> + struct nvmem_config *nvmem_config;
> +};
> +
> +#if IS_ENABLED(CONFIG_NVMEM)
> +
> +int nvmem_mmio_probe(struct platform_device *pdev);
> +int nvmem_mmio_remove(struct platform_device *pdev);
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
On 05/21/2015 09:44 AM, Srinivas Kandagatla wrote:
> diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
> index f157b6d..e665e23 100644
> --- a/drivers/nvmem/Kconfig
> +++ b/drivers/nvmem/Kconfig
> @@ -8,3 +8,18 @@ menuconfig NVMEM
> from both the Linux Kernel and the userspace.
>
> If unsure, say no.
> +
> +if NVMEM
> +
> +config QCOM_QFPROM
> + tristate "QCOM QFPROM Support"
> + depends on ARCH_QCOM
|| COMPILE_TEST?
> + select REGMAP_MMIO
> + help
> + Say y here to enable QFPROM support. The QFPROM provides access
> + functions for QFPROM data to rest of the drivers via nvmem interface.
> +
> + This driver can also be built as a module. If so, the module
> + will be called nvmem-qfprom.
> +
> +endif
> diff --git a/drivers/nvmem/Makefile b/drivers/nvmem/Makefile
> index f694cfc..caea611 100644
> --- a/drivers/nvmem/Makefile
> +++ b/drivers/nvmem/Makefile
> @@ -5,3 +5,7 @@
> obj-$(CONFIG_NVMEM) += nvmem_core.o
> nvmem_core-y := core.o
> nvmem_core-y += nvmem-mmio.o
> +
> +# Devices
> +obj-$(CONFIG_QCOM_QFPROM) += nvmem_qfprom.o
> +nvmem_qfprom-y := qfprom.o
Why not just
obj-$(CONFIG_QCOM_QFPROM) += nvmem_qfprom.o
?
> diff --git a/drivers/nvmem/qfprom.c b/drivers/nvmem/qfprom.c
> new file mode 100644
> index 0000000..5ea84bb
> --- /dev/null
> +++ b/drivers/nvmem/qfprom.c
> @@ -0,0 +1,51 @@
> +/*
> + * Copyright (C) 2015 Srinivas Kandagatla <[email protected]>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * 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/module.h>
> +#include <linux/of.h>
> +#include "nvmem-mmio.h"
> +
> +static struct regmap_config qfprom_regmap_config = {
const?
> + .reg_bits = 32,
> + .val_bits = 8,
> + .reg_stride = 1,
> +};
> +
> +static struct nvmem_config econfig = {
const?
> + .name = "qfprom",
> + .owner = THIS_MODULE,
> +};
> +
> +static struct nvmem_mmio_data qfprom_data = {
const?
> + .nvmem_config = &econfig,
> + .regmap_config = &qfprom_regmap_config,
> +};
>
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
On 05/21/2015 09:44 AM, Srinivas Kandagatla wrote:
> This patch adds bindings for qfprom found in QCOM SOCs. QFPROM driver
> is based on simple nvmem framework.
>
> Signed-off-by: Srinivas Kandagatla <[email protected]>
> ---
Reviewed-by: Stephen Boyd <[email protected]>
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
On 05/21/2015 09:45 AM, Srinivas Kandagatla wrote:
> diff --git a/drivers/nvmem/Makefile b/drivers/nvmem/Makefile
> index caea611..cc46791 100644
> --- a/drivers/nvmem/Makefile
> +++ b/drivers/nvmem/Makefile
> @@ -9,3 +9,5 @@ nvmem_core-y += nvmem-mmio.o
> # Devices
> obj-$(CONFIG_QCOM_QFPROM) += nvmem_qfprom.o
> nvmem_qfprom-y := qfprom.o
> +obj-$(CONFIG_NVMEM_SUNXI_SID) += nvmem-sunxi-sid.o
> +nvmem-sunxi-sid-y := sunxi-sid.o
Oh I see, so the module has nvmem- in the name. Isn't there some way to
add a rule to do that for all provider drivers?
> diff --git a/drivers/nvmem/sunxi-sid.c b/drivers/nvmem/sunxi-sid.c
> new file mode 100644
> index 0000000..5bfce35
> --- /dev/null
> +++ b/drivers/nvmem/sunxi-sid.c
> @@ -0,0 +1,64 @@
> +/*
> + * Allwinner sunXi SoCs Security ID support.
> + *
> + * Copyright (c) 2013 Oliver Schinagl <[email protected]>
> + * Copyright (C) 2014 Maxime Ripard <[email protected]>
> + *
> + * 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.
> + *
> + * 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/module.h>
> +#include <linux/of.h>
> +#include "nvmem-mmio.h"
> +
> +static bool sunxi_sid_writeable_reg(struct device *dev, unsigned int reg)
> +{
> + return false;
> +}
> +
> +static struct nvmem_config econfig = {
const?
> + .name = "sunix-sid",
> + .owner = THIS_MODULE,
> +};
> +
> +static struct regmap_config sunxi_sid_regmap_config = {
const?
> + .reg_bits = 32,
> + .val_bits = 32,
> + .reg_stride = 4,
> + .writeable_reg = sunxi_sid_writeable_reg,
> +};
> +
> +static struct nvmem_mmio_data sunxi_data = {
const?
> + .nvmem_config = &econfig,
> + .regmap_config = &sunxi_sid_regmap_config,
> +};
> +
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
On Tue, Jun 16, 2015 at 03:29:25PM -0700, Stephen Boyd wrote:
> On 05/21/2015 09:43 AM, Srinivas Kandagatla wrote:
> > + /* if it's not end on byte boundary */
> > + if ((nbits + bit_offset) % BITS_PER_BYTE) {
> > + /* setup the last byte with msb bits from nvmem */
> > + rc = regmap_raw_read(nvmem->regmap,
> > + cell->offset + cell->bytes - 1, &v, 1);
> > + *p |= GENMASK(7, (nbits + bit_offset) % BITS_PER_BYTE) & v;
> > +
> > + }
> > +
> > + return buf;
> > +}
> > +
> > +/**
> > + * nvmem_cell_write(): Write to a given nvmem cell
>
> This isn't kernel doc notation. It should be like
>
> nvmem_cell_write - Write to a given nvmem cell
Almost. Should be:
nvmem_cell_write() - Write to a given nvmem cell
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
Many thanks for review.
On 16/06/15 23:43, Stephen Boyd wrote:
> On 05/21/2015 09:43 AM, Srinivas Kandagatla wrote:
>>
>> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
>> new file mode 100644
>> index 0000000..6c2f0b1
>> --- /dev/null
>> +++ b/drivers/nvmem/core.c
>> @@ -0,0 +1,398 @@
>> +/*
>> + * nvmem framework core.
>> + *
>> + * Copyright (C) 2015 Srinivas Kandagatla <[email protected]>
>> + * Copyright (C) 2013 Maxime Ripard <[email protected]>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 and
>> + * only version 2 as published by the Free Software Foundation.
>> + *
>> + * 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/nvmem-provider.h>
>> +#include <linux/export.h>
>> +#include <linux/fs.h>
>> +#include <linux/idr.h>
>> +#include <linux/init.h>
>> +#include <linux/regmap.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/slab.h>
>> +#include <linux/uaccess.h>
>
> Is this include used?
>
Yep, not required.
>> +
>> +static int of_nvmem_match(struct device *dev, const void *nvmem_np)
>> +{
>> + return dev->of_node == nvmem_np;
>> +}
>> +
>> +static struct nvmem_device *of_nvmem_find(struct device_node *nvmem_np)
>
> const?
Sure,
>
>> +{
>> + struct device *d;
>> +
>> + if (!nvmem_np)
>> + return NULL;
>> +
>> + d = class_find_device(&nvmem_class, NULL, nvmem_np, of_nvmem_match);
>> +
>> + return d ? to_nvmem_device(d) : NULL;
>> +}
>> +
>> +static struct nvmem_cell *nvmem_find_cell(const char *cell_id)
>> +{
>> + struct nvmem_cell *p;
>> +
>> + list_for_each_entry(p, &nvmem_cells, node) {
>
> Unnecessary braces.
Yep, Will remove it.
>
>> + if (p && !strcmp(p->name, cell_id))
>> + return p;
>> + }
>> +
>> + return NULL;
>> +}
>> +
>> +static void nvmem_cell_drop(struct nvmem_cell *cell)
>> +{
>> + mutex_lock(&nvmem_cells_mutex);
>> + list_del(&cell->node);
>> + mutex_unlock(&nvmem_cells_mutex);
>> + kfree(cell);
>> +}
>> +
>> +static void nvmem_device_remove_all_cells(struct nvmem_device *nvmem)
>> +{
>> + struct nvmem_cell *cell = NULL;
>
> Unnecessary initialization
Yes.
>
>> + struct list_head *p, *n;
>> +
>> + list_for_each_safe(p, n, &nvmem_cells) {
>> + cell = list_entry(p, struct nvmem_cell, node);
>> + if (cell->nvmem == nvmem)
>> + nvmem_cell_drop(cell);
>> + }
> [..]
>> +
>> +static int nvmem_add_cells(struct nvmem_device *nvmem,
>> + struct nvmem_config *cfg)
>> +{
>> + struct nvmem_cell **cells;
>> + struct nvmem_cell_info *info = cfg->cells;
>> + int i, rval;
>> +
>> + cells = kzalloc(sizeof(*cells) * cfg->ncells, GFP_KERNEL);
>
> kcalloc
This is temporary array, I did this on intention, to make it easy to
kfree cells which are found invalid at runtime.
>
>> + if (!cells)
>> + return -ENOMEM;
>> +
>> + for (i = 0; i < cfg->ncells; i++) {
>> + cells[i] = kzalloc(sizeof(struct nvmem_cell), GFP_KERNEL);
>
> sizeof(**cells) ?
Yep.
>
>> +
>> +/**
>> + * nvmem_register(): Register a nvmem device for given nvmem.
>> + * Also creates an binary entry in /sys/class/nvmem/dev-name/nvmem
>> + *
>> + * @nvmem: nvmem device that needs to be created
>
> You mean @config?
>
Yes, I will fix it.
>> + *
>> + * The return value will be an ERR_PTR() on error or a valid pointer
>> + nvmem->dev.of_node = config->dev->of_node;
>> + dev_set_name(&nvmem->dev, "%s%d",
>> + config->name ? : "nvmem", config->id);
>
> It may be better to always name it nvmem%d so that we don't allow the
> possibility of conflicts.
We can do that, but I wanted to make the sysfs and dev entries more
readable than just nvmem0, nvmem1...
>
>> +
>> + nvmem->read_only = of_property_read_bool(nvmem->dev.of_node,
>> + "read-only");
>
> What if we're not using DT? How would we specify read_only?
>
Thanks for spotting, you are correct, I need to add read_only flag to
nvmem_config too.
>> +
>> + device_initialize(&nvmem->dev);
>> +
>> + dev_dbg(&nvmem->dev, "Registering nvmem device %s\n",
>> + dev_name(&nvmem->dev));
>> +
>> + rval = device_add(&nvmem->dev);
>> + if (rval) {
>> + ida_simple_remove(&nvmem_ida, nvmem->id);
>> + kfree(nvmem);
>> + return ERR_PTR(rval);
>> + }
>> +
>> + /* update sysfs attributes */
>> + if (nvmem->read_only)
>> + sysfs_update_group(&nvmem->dev.kobj, &nvmem_bin_ro_group);
>
> It would be nice if this could be done before the device was registered.
> Perhaps have two device_types, one for read-only and one for read/write?
The attributes are actually coming from the class object, so we have no
choice to update the attributes before the device is registered.
May it would be more safe to have default as read-only and modify it to
read/write based on read-only flag?
>
>> +
>> + */
>> +int nvmem_unregister(struct nvmem_device *nvmem)
>> +{
>> + mutex_lock(&nvmem_mutex);
>> + if (nvmem->users) {
>> + mutex_unlock(&nvmem_mutex);
>> + return -EBUSY;
>
> Hmm... that doesn't seem nice. Typically when something is unregistered
> we have to pull the rug out from underneath the users and start
> returning errors to them. The provider needs to be free to unregister
> because it's been forcibly removed. So really this function should
> return void.
>
The consumer api is get/put style, so consumers who already have
references to the provider, removing provider forcefully might lead to
dangling pointer. Having said that I can give a try and see how it looks.
>> + }
>> + mutex_unlock(&nvmem_mutex);
>> +
>> + nvmem_device_remove_all_cells(nvmem);
>> + device_del(&nvmem->dev);
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(nvmem_unregister);
>> +
>> +static int nvmem_init(void)
>
> __init? And __exit on nvmem_exit?
yep, will do that.
>
>> +{
>> + return class_register(&nvmem_class);
>
> I thought class was on the way out? Aren't we supposed to use bus types
> for new stuff?
Do you remember any conversation on the list about this? I could not
find it on web.
on the other hand, nvmem is not really a bus, making it a bus type
sounds incorrect to me.
--srini
>
On 16/06/15 23:29, Stephen Boyd wrote:
> On 05/21/2015 09:43 AM, Srinivas Kandagatla wrote:
>> @@ -379,6 +380,351 @@ int nvmem_unregister(struct nvmem_device *nvmem)
> [...]
>> +
>> + return nvmem;
>> +}
>> +
>> +static int __nvmem_device_put(struct nvmem_device *nvmem)
>
> Why does this return int? It's not used anywhere.
>
I can remove it.
>> +{
>> + module_put(nvmem->owner);
>> + mutex_lock(&nvmem_mutex);
>> + nvmem->users--;
>> + mutex_unlock(&nvmem_mutex);
>> +
>> + return 0;
>> +}
>> +
>> +static struct nvmem_cell *nvmem_cell_get_from_list(const char *cell_id)
>> +{
>> + struct nvmem_cell *cell = NULL;
>> + struct nvmem_device *nvmem;
>> +
>> + nvmem = __nvmem_device_get(NULL, &cell, cell_id);
>> + if (IS_ERR(nvmem))
>> + return (struct nvmem_cell *)nvmem;
>
> ERR_CAST?
Will fix it.
>
>> +
>> +
>> + return cell;
>> +
>> +}
>> +
>> +static struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
>> + const char *name)
>> +{
>> + struct device_node *cell_np, *nvmem_np;
>> + struct nvmem_cell *cell;
>> + struct nvmem_device *nvmem;
>> + const __be32 *addr;
>> + int rval, len, index;
>> +
>> + index = of_property_match_string(np, "nvmem-cell-names", name);
>> +
>> + cell_np = of_parse_phandle(np, "nvmem-cell", index);
>> + if (!cell_np)
>> + return ERR_PTR(-EINVAL);
>> +
>> + nvmem_np = of_get_next_parent(cell_np);
>> + if (!nvmem_np)
>> + return ERR_PTR(-EINVAL);
>> +
>> + nvmem = __nvmem_device_get(nvmem_np, NULL, NULL);
>> + if (IS_ERR(nvmem))
>> + return (struct nvmem_cell *)nvmem;
>
> ERR_CAST?
Will fix it.
>
>> +
>> + addr = of_get_property(cell_np, "reg", &len);
>> + if (!addr || (len < 2 * sizeof(int))) {
>> + dev_err(&nvmem->dev, "of_i2c: invalid reg on %s\n",
>
> huh? of_i2c?
>
:-)
Will fix it.
>
>> +
>> + /* if it's not end on byte boundary */
>> + if ((nbits + bit_offset) % BITS_PER_BYTE) {
>> + /* setup the last byte with msb bits from nvmem */
>> + rc = regmap_raw_read(nvmem->regmap,
>> + cell->offset + cell->bytes - 1, &v, 1);
>> + *p |= GENMASK(7, (nbits + bit_offset) % BITS_PER_BYTE) & v;
>> +
>> + }
>> +
>> + return buf;
>> +}
>> +
>> +/**
>> + * nvmem_cell_write(): Write to a given nvmem cell
>
> This isn't kernel doc notation. It should be like
>
> nvmem_cell_write - Write to a given nvmem cell
>
Will fix as suggested by Sascha as
foobar() - short function description of foobar
Looks correct according to Documentation/kernel-doc-nano-HOWTO.txt
>> + *
>> + * @cell: nvmem cell to be written.
>> + * @buf: Buffer to be written.
>> + * @len: length of buffer to be written to nvmem cell.
>> + *
>> + * The return value will be an length of bytes written or non zero on failure.
>> + */
>> +int nvmem_cell_write(struct nvmem_cell *cell, void *buf, ssize_t len)
> [..]
>> +
>> static int nvmem_init(void)
>> {
>> return class_register(&nvmem_class);
>> diff --git a/include/linux/nvmem-consumer.h b/include/linux/nvmem-consumer.h
>> new file mode 100644
>> index 0000000..c3fa8c7
>> --- /dev/null
>> +++ b/include/linux/nvmem-consumer.h
>> @@ -0,0 +1,49 @@
>> +/*
>> + * nvmem framework consumer.
>> + *
>> + * Copyright (C) 2015 Srinivas Kandagatla <[email protected]>
>> + * Copyright (C) 2013 Maxime Ripard <[email protected]>
>> + *
>> + * This file is licensed under the terms of the GNU General Public
>> + * License version 2. This program is licensed "as is" without any
>> + * warranty of any kind, whether express or implied.
>> + */
>> +
>> +#ifndef _LINUX_NVMEM_CONSUMER_H
>> +#define _LINUX_NVMEM_CONSUMER_H
>> +
>> +/* consumer cookie */
>> +struct nvmem_cell;
>> +
>> +#if IS_ENABLED(CONFIG_NVMEM)
>> +
>> +/* Cell based interface */
>> +struct nvmem_cell *nvmem_cell_get(struct device *dev, const char *name);
>
> We should probably forward declare struct device in this file too.
Yep I will do it.
>
On 16/06/15 23:49, Stephen Boyd wrote:
> On 05/21/2015 09:43 AM, Srinivas Kandagatla wrote:
>> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
>> index 8a4b358..68ee8d1 100644
>> --- a/drivers/nvmem/core.c
>> +++ b/drivers/nvmem/core.c
>> @@ -436,6 +436,51 @@ static int __nvmem_device_put(struct nvmem_device *nvmem)
>> return 0;
>> }
>>
>> +static int nvmem_match(struct device *dev, const void *data)
>> +{
>> + return !strcmp(dev_name(dev), (const char *)data);
>
> Unnecessary cast.
Sure, will fix it.
>
>> +}
>> +
>> +static struct nvmem_device *nvmem_find(const char *name)
>> +{
>> + struct device *d;
>> +
>> + d = class_find_device(&nvmem_class, NULL, (void *)name, nvmem_match);
>
> Unnecessary cast
Will fix it.
>
>> +
>> + return d ? to_nvmem_device(d) : NULL;
>> +}
>
> [...]
>> +
>> +void nvmem_device_put(struct nvmem_device *nvmem)
>> +{
>> + __nvmem_device_put(nvmem);
>> +}
>> +EXPORT_SYMBOL_GPL(nvmem_device_put);
>>
>> +int nvmem_device_cell_read(struct nvmem_device *nvmem,
>> + struct nvmem_cell_info *info, void *buf)
>> +{
>> +}
>> +EXPORT_SYMBOL_GPL(nvmem_device_cell_read);
>> +
>> +int nvmem_device_cell_write(struct nvmem_device *nvmem,
>> + struct nvmem_cell_info *info, void *buf)
>> +{
>> +}
>> +EXPORT_SYMBOL_GPL(nvmem_device_cell_write);
>> +
>> +int nvmem_device_read(struct nvmem_device *nvmem,
>> + unsigned int offset,
>> + size_t bytes, void *buf)
>> +{
>> +}
>> +EXPORT_SYMBOL_GPL(nvmem_device_read);
>> +
>> +int nvmem_device_write(struct nvmem_device *nvmem,
>> + unsigned int offset,
>> + size_t bytes, void *buf)
>> +{
>> +}
>> +EXPORT_SYMBOL_GPL(nvmem_device_write);
>
> Can you please add kernel-doc on these exported APIs?
>
Thanks for spotting, I will add them.
>> +
>> static int nvmem_init(void)
>> {
>> return class_register(&nvmem_class);
>> diff --git a/include/linux/nvmem-consumer.h b/include/linux/nvmem-consumer.h
>> index c3fa8c7..66c67ba 100644
>> --- a/include/linux/nvmem-consumer.h
>> +++ b/include/linux/nvmem-consumer.h
>> @@ -23,6 +32,18 @@ void nvmem_cell_put(struct nvmem_cell *cell);
>> void *nvmem_cell_read(struct nvmem_cell *cell, ssize_t *len);
>> int nvmem_cell_write(struct nvmem_cell *cell, void *buf, ssize_t len);
>>
>> +/* direct nvmem device read/write interface */
>> +struct nvmem_device *nvmem_device_get(struct device *dev, const char *name);
>> +void nvmem_device_put(struct nvmem_device *nvmem);
>> +int nvmem_device_read(struct nvmem_device *nvmem, unsigned int offset,
>> + size_t bytes, void *buf);
>> +int nvmem_device_write(struct nvmem_device *nvmem, unsigned int offset,
>> + size_t bytes, void *buf);
>> +int nvmem_device_cell_read(struct nvmem_device *nvmem,
>> + struct nvmem_cell_info *info, void *buf);
>> +int nvmem_device_cell_write(struct nvmem_device *nvmem,
>> + struct nvmem_cell_info *info, void *buf);
>> +
>
> Can we also have devm_nvmem_*_get() APIs please?
Sure, I will spin it in next version.
>
>> diff --git a/include/linux/nvmem-provider.h b/include/linux/nvmem-provider.h
>> index 4908b37..7a982cd 100644
>> --- a/include/linux/nvmem-provider.h
>> +++ b/include/linux/nvmem-provider.h
>> @@ -12,15 +12,9 @@
>> #ifndef _LINUX_NVMEM_PROVIDER_H
>> #define _LINUX_NVMEM_PROVIDER_H
>>
>> -struct nvmem_device;
>> +#include <linux/nvmem-consumer.h>
>>
>> -struct nvmem_cell_info {
>> - const char *name;
>> - int offset;
>> - int bytes;
>> - int bit_offset;
>> - int nbits;
>> -};
>> +struct nvmem_device;
>
> Should this diff be part of an earlier patch?
Possibly, something wrong with this diff, I will fix it.
>
On 16/06/15 23:53, Stephen Boyd wrote:
> On 05/21/2015 09:44 AM, Srinivas Kandagatla wrote:
>> diff --git a/Documentation/devicetree/bindings/nvmem/nvmem.txt b/Documentation/devicetree/bindings/nvmem/nvmem.txt
>> new file mode 100644
>> index 0000000..ecea654
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/nvmem/nvmem.txt
>> @@ -0,0 +1,84 @@
>> += NVMEM Data Device Tree Bindings =
>> +
>> +This binding is intended to represent the location of hardware
>> +configuration data stored in NVMEMs.
>
> It would be worthwhile spelling out what NVMEM stands for.
>
>> +
>> +On a significant proportion of boards, the manufacturer has stored
>> +some data on NVMEM, for the OS to be able to retrieve these information
>> +and act upon it. Obviously, the OS has to know about where to retrieve
>> +these data from, and where they are stored on the storage device.
>> +
>> +This document is here to document this.
>> +
>> += Data providers =
>> +Contains bindings specific to provider drivers and data cells as children
>> +to this node.
>
> children of this node?
>
Yep, will fix the text
>> +
>> +Optional properties:
>> + read-only: Mark the provider as read only.
>> +
>> += Data cells =
>> +These are the child nodes of the provider which contain data cell
>> +information like offset and size in nvmem provider.
>> +
>> +Required properties:
>> +reg: specifies the offset in byte within that storage device, start bit
>> + in the byte and the length in bits of the data we care about.
>> + There could be more then one offset-length pairs in this property.
>
> s/then/than/
Yep.
>
>> +
>> +Optional properties:
>> +
>> +bit-offset: specifies the offset in bit within the address range specified
>> + by reg property. Can take values from 0-7.
>> +nbits: specifies number of bits this cell occupies starting from bit-offset.
>> +
>
> Hopefully the consumer knows the endianness of the data stored.
As we read byte-byte, does it matter, as long as consumer gets them in
the same order as its stored.
>
>> +For example:
>> +
>> + /* Provider */
>> + qfprom: qfprom@00700000 {
>> + ...
>> +
>> + /* Data cells */
>> + tsens_calibration: calib@404 {
>> + reg = <0x404 0x10>;
>> + };
>> +
>> + tsens_calibration_bckp: calib_bckp@504 {
>> + reg = <0x504 0x11>;
>> + bit-offset = 6;
>> + nbits = 128;
>> + };
>> +
>> + pvs_version: pvs-version@6 {
>> + reg = <0x6 0x2>
>> + bit-offset = 7;
>> + nbits = 2;
>> + };
>> +
>> + speed_bin: speed-bin@c{
>> + reg = <0xc 0x1>;
>> + bit-offset = 2;
>> + nbits = 3;
>> +
>> + };
>> + ...
>> + };
>> +
>> += Data consumers =
>> +Are device nodes which consume nvmem data cells/providers.
>> +
>> +Required-properties:
>> +nvmem-cell: list of phandle to the nvmem data cells.
>> +nvmem-cell-names: names for the each nvmem-cell specified
>> +
>> +Optional-properties:
>> +nvmem : list of phandles to nvmem providers.
>> +nvmem-names: names for the each nvmem provider.
>
> Is nvmem-names required if nvmem is used?
Yes, will fix it.
>
On 16/06/15 23:58, Stephen Boyd wrote:
> On 05/21/2015 09:44 AM, Srinivas Kandagatla wrote:
>> diff --git a/drivers/nvmem/nvmem-mmio.c b/drivers/nvmem/nvmem-mmio.c
>> new file mode 100644
>> index 0000000..0d8131f
>> --- /dev/null
>> +++ b/drivers/nvmem/nvmem-mmio.c
>> @@ -0,0 +1,69 @@
>> +/*
>> + * Copyright (C) 2015 Srinivas Kandagatla <[email protected]>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 and
>> + * only version 2 as published by the Free Software Foundation.
>> + *
>> + * 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/module.h>
>> +#include <linux/err.h>
>> +#include <linux/of.h>
>> +#include <linux/of_device.h>
>
> #include <linux/platform_device.h>
> #include <linux/nvmem_provider.h>
> #include <linux/regmap.h>
>
These are included in nvmem-mmio.h, however for clarity sake I could add
them here too.
>> +#include "nvmem-mmio.h"
>> +
>> +int nvmem_mmio_remove(struct platform_device *pdev)
>> +{
>> + struct nvmem_device *nvmem = platform_get_drvdata(pdev);
>> +
>> + return nvmem_unregister(nvmem);
>> +}
>> +EXPORT_SYMBOL_GPL(nvmem_mmio_remove);
>> +
>> +int nvmem_mmio_probe(struct platform_device *pdev)
>> +{
>> +
> [...]
>> +
>> + platform_set_drvdata(pdev, nvmem);
>
> It may be better to return the nvmem device instead so that the one
> drvdata member is usable by the calling driver.
Makes sense.. I will give it a try.
>
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(nvmem_mmio_probe);
>
> Kernel-doc on these exported functions?
Yep, thanks for spotting, I will fix it.
>
>> diff --git a/drivers/nvmem/nvmem-mmio.h b/drivers/nvmem/nvmem-mmio.h
>> new file mode 100644
>> index 0000000..a2ad4e5
>> --- /dev/null
>> +++ b/drivers/nvmem/nvmem-mmio.h
>> @@ -0,0 +1,41 @@
>> +/*
>> + * MMIO based nvmem providers.
>> + *
>> + * Copyright (C) 2015 Srinivas Kandagatla <[email protected]>
>> + *
>> + * This file is licensed under the terms of the GNU General Public
>> + * License version 2. This program is licensed "as is" without any
>> + * warranty of any kind, whether express or implied.
>> + */
>> +
>> +#ifndef _LINUX_NVMEM_MMIO_H
>> +#define _LINUX_NVMEM_MMIO_H
>> +
>> +#include <linux/platform_device.h>
>
> Forward declare struct platform_device instead.
>
>> +#include <linux/nvmem-provider.h>
>
> Forward declare nvmem_config instead.
>
>> +#include <linux/regmap.h>
>
> Forward declare regmap_config instead.
>
>> +
>> +struct nvmem_mmio_data {
>> + struct regmap_config *regmap_config;
>> + struct nvmem_config *nvmem_config;
>> +};
>> +
>> +#if IS_ENABLED(CONFIG_NVMEM)
>> +
>> +int nvmem_mmio_probe(struct platform_device *pdev);
>> +int nvmem_mmio_remove(struct platform_device *pdev);
>
On 17/06/15 00:04, Stephen Boyd wrote:
> On 05/21/2015 09:45 AM, Srinivas Kandagatla wrote:
>> diff --git a/drivers/nvmem/Makefile b/drivers/nvmem/Makefile
>> index caea611..cc46791 100644
>> --- a/drivers/nvmem/Makefile
>> +++ b/drivers/nvmem/Makefile
>> @@ -9,3 +9,5 @@ nvmem_core-y += nvmem-mmio.o
>> # Devices
>> obj-$(CONFIG_QCOM_QFPROM) += nvmem_qfprom.o
>> nvmem_qfprom-y := qfprom.o
>> +obj-$(CONFIG_NVMEM_SUNXI_SID) += nvmem-sunxi-sid.o
>> +nvmem-sunxi-sid-y := sunxi-sid.o
>
> Oh I see, so the module has nvmem- in the name. Isn't there some way to
> add a rule to do that for all provider drivers?
>
I will give it a try, and also fix the other comments on this patch.
>> diff --git a/drivers/nvmem/sunxi-sid.c b/drivers/nvmem/sunxi-sid.c
>> new file mode 100644
>> index 0000000..5bfce35
>> --- /dev/null
>> +++ b/drivers/nvmem/sunxi-sid.c
>> @@ -0,0 +1,64 @@
>> +/*
>> + * Allwinner sunXi SoCs Security ID support.
>> + *
>> + * Copyright (c) 2013 Oliver Schinagl <[email protected]>
>> + * Copyright (C) 2014 Maxime Ripard <[email protected]>
>> + *
>> + * 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.
>> + *
>> + * 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/module.h>
>> +#include <linux/of.h>
>> +#include "nvmem-mmio.h"
>> +
>> +static bool sunxi_sid_writeable_reg(struct device *dev, unsigned int reg)
>> +{
>> + return false;
>> +}
>> +
>> +static struct nvmem_config econfig = {
>
> const?
>
>> + .name = "sunix-sid",
>> + .owner = THIS_MODULE,
>> +};
>> +
>> +static struct regmap_config sunxi_sid_regmap_config = {
>
> const?
>
>> + .reg_bits = 32,
>> + .val_bits = 32,
>> + .reg_stride = 4,
>> + .writeable_reg = sunxi_sid_writeable_reg,
>> +};
>> +
>> +static struct nvmem_mmio_data sunxi_data = {
>
> const?
>
>> + .nvmem_config = &econfig,
>> + .regmap_config = &sunxi_sid_regmap_config,
>> +};
>> +
>
>
On 17/06/15 00:00, Stephen Boyd wrote:
> On 05/21/2015 09:44 AM, Srinivas Kandagatla wrote:
>> diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
>> index f157b6d..e665e23 100644
>> --- a/drivers/nvmem/Kconfig
>> +++ b/drivers/nvmem/Kconfig
>> @@ -8,3 +8,18 @@ menuconfig NVMEM
>> from both the Linux Kernel and the userspace.
>>
>> If unsure, say no.
>> +
>> +if NVMEM
>> +
>> +config QCOM_QFPROM
>> + tristate "QCOM QFPROM Support"
>> + depends on ARCH_QCOM
>
> || COMPILE_TEST?
>
Yes, makes sense. I will add it.
>> + select REGMAP_MMIO
>> + help
>> + Say y here to enable QFPROM support. The QFPROM provides access
>> + functions for QFPROM data to rest of the drivers via nvmem interface.
>> +
>> + This driver can also be built as a module. If so, the module
>> + will be called nvmem-qfprom.
>> +
>> +endif
>> diff --git a/drivers/nvmem/Makefile b/drivers/nvmem/Makefile
>> index f694cfc..caea611 100644
>> --- a/drivers/nvmem/Makefile
>> +++ b/drivers/nvmem/Makefile
>> @@ -5,3 +5,7 @@
>> obj-$(CONFIG_NVMEM) += nvmem_core.o
>> nvmem_core-y := core.o
>> nvmem_core-y += nvmem-mmio.o
>> +
>> +# Devices
>> +obj-$(CONFIG_QCOM_QFPROM) += nvmem_qfprom.o
>> +nvmem_qfprom-y := qfprom.o
>
> Why not just
>
> obj-$(CONFIG_QCOM_QFPROM) += nvmem_qfprom.o
>
> ?
I will recheck on this.
>
>> diff --git a/drivers/nvmem/qfprom.c b/drivers/nvmem/qfprom.c
>> new file mode 100644
>> index 0000000..5ea84bb
>> --- /dev/null
>> +++ b/drivers/nvmem/qfprom.c
>> @@ -0,0 +1,51 @@
>> +/*
>> + * Copyright (C) 2015 Srinivas Kandagatla <[email protected]>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 and
>> + * only version 2 as published by the Free Software Foundation.
>> + *
>> + * 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/module.h>
>> +#include <linux/of.h>
>> +#include "nvmem-mmio.h"
>> +
>> +static struct regmap_config qfprom_regmap_config = {
>
> const?
Will fix all the comments.
>
>> + .reg_bits = 32,
>> + .val_bits = 8,
>> + .reg_stride = 1,
>> +};
>> +
>> +static struct nvmem_config econfig = {
>
> const?
>
>> + .name = "qfprom",
>> + .owner = THIS_MODULE,
>> +};
>> +
>> +static struct nvmem_mmio_data qfprom_data = {
>
> const?
>
>> + .nvmem_config = &econfig,
>> + .regmap_config = &qfprom_regmap_config,
>> +};
>>
>
Hello Srinivas,
On 15-05-21 17:44:12, Srinivas Kandagatla wrote:
> This patch adds bindings for simple nvmem framework which allows nvmem
> consumers to talk to nvmem providers to get access to nvmem cell data.
>
> Signed-off-by: Maxime Ripard <[email protected]>
> [Maxime Ripard: intial version of eeprom framework]
> Signed-off-by: Srinivas Kandagatla <[email protected]>
> ---
> Documentation/devicetree/bindings/nvmem/nvmem.txt | 84 +++++++++++++++++++++++
> 1 file changed, 84 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/nvmem/nvmem.txt
>
> diff --git a/Documentation/devicetree/bindings/nvmem/nvmem.txt b/Documentation/devicetree/bindings/nvmem/nvmem.txt
> new file mode 100644
> index 0000000..ecea654
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/nvmem/nvmem.txt
> @@ -0,0 +1,84 @@
> += NVMEM Data Device Tree Bindings =
> +
> +This binding is intended to represent the location of hardware
> +configuration data stored in NVMEMs.
> +
> +On a significant proportion of boards, the manufacturer has stored
> +some data on NVMEM, for the OS to be able to retrieve these information
> +and act upon it. Obviously, the OS has to know about where to retrieve
> +these data from, and where they are stored on the storage device.
> +
> +This document is here to document this.
> +
> += Data providers =
> +Contains bindings specific to provider drivers and data cells as children
> +to this node.
> +
> +Optional properties:
> + read-only: Mark the provider as read only.
> +
> += Data cells =
> +These are the child nodes of the provider which contain data cell
> +information like offset and size in nvmem provider.
> +
> +Required properties:
> +reg: specifies the offset in byte within that storage device, start bit
> + in the byte and the length in bits of the data we care about.
> + There could be more then one offset-length pairs in this property.
> +
> +Optional properties:
> +
> +bit-offset: specifies the offset in bit within the address range specified
> + by reg property. Can take values from 0-7.
> +nbits: specifies number of bits this cell occupies starting from bit-offset.
> +
> +For example:
> +
> + /* Provider */
> + qfprom: qfprom@00700000 {
> + ...
> +
> + /* Data cells */
> + tsens_calibration: calib@404 {
> + reg = <0x404 0x10>;
> + };
> +
> + tsens_calibration_bckp: calib_bckp@504 {
> + reg = <0x504 0x11>;
> + bit-offset = 6;
> + nbits = 128;
> + };
> +
> + pvs_version: pvs-version@6 {
> + reg = <0x6 0x2>
> + bit-offset = 7;
> + nbits = 2;
> + };
> +
> + speed_bin: speed-bin@c{
> + reg = <0xc 0x1>;
> + bit-offset = 2;
> + nbits = 3;
> +
> + };
> + ...
> + };
We have a need of exposing information like SoC ID, revision and such
which is what this nvmem framework proposes to be suitable for. Till
now I was trying a different approach for the same [1].
The On Chip One Time Programmable block on the Vybrid can be handled
nicely with this nvmem framework however I had a few queries with
regards to the framework after trying it on a Vybrid VF61 SoC.
1. From what I understand, getting the information in hex is the only
way possible as of now? Would it be possible to expose the information
as strings from different paths under /sys/class/nvmem/*/nvmem/?
For example, if I have a sub node as below
ocotp: ocotp@400a5000 {
compatible = "fsl,vf610-ocotp";
reg = <0x400a5000 0x1000>;
ocotp_cfg0: cfg0@410 {
reg = <0x410 0x1>;
};
ocotp_cfg1: cfg1@420 {
reg = <0x420 0x1>;
};
};
The values from the above register locations represented by the two
sub nodes above can perhaps be exposed as strings? Even if they are
not exposed as strings, making them available in separate paths, is
that something which can be done? So depending on the sub node as
above, /sys/class/nvmem/ocotp0/nvmem/cfg0/ would give values from
the registers specified.
Basically the output of /sys/class/nvmem/*/nvmem being restricted
to only the subnodes specified is what I was hoping to get along
with separate paths.
2. What if the required information is scattered across different memory
regions? In my case, the SoC ID is available from one OCOTP peripheral
block, the revision is in a separate ROM area at the start of the memory
map and some CPU information I am interested in is in another memory
region. I am not sure what would be the right way to approach it with
the nvmem framework.
[1] https://lkml.org/lkml/2015/6/5/189
Thanks & Regards,
Sanchayan Maity.
<snip>
On 19/06/15 11:36, [email protected] wrote:
> Hello Srinivas,
>
> On 15-05-21 17:44:12, Srinivas Kandagatla wrote:
>> This patch adds bindings for simple nvmem framework which allows nvmem
>> consumers to talk to nvmem providers to get access to nvmem cell data.
>>
>> Signed-off-by: Maxime Ripard <[email protected]>
>> [Maxime Ripard: intial version of eeprom framework]
>> Signed-off-by: Srinivas Kandagatla <[email protected]>
>> ---
>> Documentation/devicetree/bindings/nvmem/nvmem.txt | 84 +++++++++++++++++++++++
>> 1 file changed, 84 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/nvmem/nvmem.txt
>>
>> diff --git a/Documentation/devicetree/bindings/nvmem/nvmem.txt b/Documentation/devicetree/bindings/nvmem/nvmem.txt
>> new file mode 100644
>> index 0000000..ecea654
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/nvmem/nvmem.txt
>> @@ -0,0 +1,84 @@
>> += NVMEM Data Device Tree Bindings =
>> +
>> +This binding is intended to represent the location of hardware
>> +configuration data stored in NVMEMs.
>> +
>> +On a significant proportion of boards, the manufacturer has stored
>> +some data on NVMEM, for the OS to be able to retrieve these information
>> +and act upon it. Obviously, the OS has to know about where to retrieve
>> +these data from, and where they are stored on the storage device.
>> +
>> +This document is here to document this.
>> +
>> += Data providers =
>> +Contains bindings specific to provider drivers and data cells as children
>> +to this node.
>> +
>> +Optional properties:
>> + read-only: Mark the provider as read only.
>> +
>> += Data cells =
>> +These are the child nodes of the provider which contain data cell
>> +information like offset and size in nvmem provider.
>> +
>> +Required properties:
>> +reg: specifies the offset in byte within that storage device, start bit
>> + in the byte and the length in bits of the data we care about.
>> + There could be more then one offset-length pairs in this property.
>> +
>> +Optional properties:
>> +
>> +bit-offset: specifies the offset in bit within the address range specified
>> + by reg property. Can take values from 0-7.
>> +nbits: specifies number of bits this cell occupies starting from bit-offset.
>> +
>> +For example:
>> +
>> + /* Provider */
>> + qfprom: qfprom@00700000 {
>> + ...
>> +
>> + /* Data cells */
>> + tsens_calibration: calib@404 {
>> + reg = <0x404 0x10>;
>> + };
>> +
>> + tsens_calibration_bckp: calib_bckp@504 {
>> + reg = <0x504 0x11>;
>> + bit-offset = 6;
>> + nbits = 128;
>> + };
>> +
>> + pvs_version: pvs-version@6 {
>> + reg = <0x6 0x2>
>> + bit-offset = 7;
>> + nbits = 2;
>> + };
>> +
>> + speed_bin: speed-bin@c{
>> + reg = <0xc 0x1>;
>> + bit-offset = 2;
>> + nbits = 3;
>> +
>> + };
>> + ...
>> + };
>
> We have a need of exposing information like SoC ID, revision and such
> which is what this nvmem framework proposes to be suitable for. Till
> now I was trying a different approach for the same [1].
>
> The On Chip One Time Programmable block on the Vybrid can be handled
> nicely with this nvmem framework however I had a few queries with
> regards to the framework after trying it on a Vybrid VF61 SoC.
>
> 1. From what I understand, getting the information in hex is the only
> way possible as of now? Would it be possible to expose the information
nvmem file in the /sys/ is just a binary file. hexdump is one of the
ways to dump the data, the user can read the binary file and interpret
the data in the way he wants it.
> as strings from different paths under /sys/class/nvmem/*/nvmem/?
>
> For example, if I have a sub node as below
>
> ocotp: ocotp@400a5000 {
> compatible = "fsl,vf610-ocotp";
> reg = <0x400a5000 0x1000>;
>
> ocotp_cfg0: cfg0@410 {
> reg = <0x410 0x1>;
> };
>
> ocotp_cfg1: cfg1@420 {
> reg = <0x420 0x1>;
> };
> };
>
> The values from the above register locations represented by the two
> sub nodes above can perhaps be exposed as strings? Even if they are
> not exposed as strings, making them available in separate paths, is
> that something which can be done? So depending on the sub node as
> above, /sys/class/nvmem/ocotp0/nvmem/cfg0/ would give values from
> the registers specified.
I was thinking of add similar support to show cells in
/sys/class/nvmem/*/cells/cfg0 in future once the framework is merged.
For now I want to keep it simple. This would be binary content again.
you can dump it using strings any program which wants to interpret it
differently.
>
> Basically the output of /sys/class/nvmem/*/nvmem being restricted
> to only the subnodes specified is what I was hoping to get along
> with separate paths.
>
> 2. What if the required information is scattered across different memory
> regions? In my case, the SoC ID is available from one OCOTP peripheral
> block, the revision is in a separate ROM area at the start of the memory
> map and some CPU information I am interested in is in another memory
> region. I am not sure what would be the right way to approach it with
> the nvmem framework.
I think you would have two providers in that case one is ocotp and other
is ROM.
DT would look something like this:
/* Provider */
ocotp {
...
soc-id {
reg = ...;
};
};
rom {
...
soc-reg {
reg = ...;
};
};
/* consumer */
consumer {
nvmem-cell = <&soc_id>, <&soc_rev>;
nvmem-cell-names = "soc-id", "soc-revision";
};
--srini
>
> [1] https://lkml.org/lkml/2015/6/5/189
>
> Thanks & Regards,
> Sanchayan Maity.
>
> <snip>
>
On 06/18/2015 05:46 AM, Srinivas Kandagatla wrote:
> Many thanks for review.
>
> On 16/06/15 23:43, Stephen Boyd wrote:
>> On 05/21/2015 09:43 AM, Srinivas Kandagatla wrote:
>>>
>>>> +
>>>> +static int nvmem_add_cells(struct nvmem_device *nvmem,
>>>> + struct nvmem_config *cfg)
>>>> +{
>>>> + struct nvmem_cell **cells;
>>>> + struct nvmem_cell_info *info = cfg->cells;
>>>> + int i, rval;
>>>> +
>>>> + cells = kzalloc(sizeof(*cells) * cfg->ncells, GFP_KERNEL);
>>>
>>> kcalloc
> This is temporary array, I did this on intention, to make it easy to
> kfree cells which are found invalid at runtime.
Ok, but how does that change using kcalloc over kzalloc? I must have
missed something.
>
>
>>> + *
>>> + * The return value will be an ERR_PTR() on error or a valid pointer
>>> + nvmem->dev.of_node = config->dev->of_node;
>>> + dev_set_name(&nvmem->dev, "%s%d",
>>> + config->name ? : "nvmem", config->id);
>>
>> It may be better to always name it nvmem%d so that we don't allow the
>> possibility of conflicts.
> We can do that, but I wanted to make the sysfs and dev entries more
> readable than just nvmem0, nvmem1...
Well sysfs is not really for humans. It's for machines. The nvmem
devices could have a name property so that a more human readable string
is present.
>
>>> +
>>> + device_initialize(&nvmem->dev);
>>> +
>>> + dev_dbg(&nvmem->dev, "Registering nvmem device %s\n",
>>> + dev_name(&nvmem->dev));
>>> +
>>> + rval = device_add(&nvmem->dev);
>>> + if (rval) {
>>> + ida_simple_remove(&nvmem_ida, nvmem->id);
>>> + kfree(nvmem);
>>> + return ERR_PTR(rval);
>>> + }
>>> +
>>> + /* update sysfs attributes */
>>> + if (nvmem->read_only)
>>> + sysfs_update_group(&nvmem->dev.kobj, &nvmem_bin_ro_group);
>>
>> It would be nice if this could be done before the device was registered.
>> Perhaps have two device_types, one for read-only and one for read/write?
>
> The attributes are actually coming from the class object, so we have
> no choice to update the attributes before the device is registered.
>
> May it would be more safe to have default as read-only and modify it
> to read/write based on read-only flag?
>
>
Can you assign the attributes to the device_type in the nvmem::struct
device? I don't see why these attributes need to be part of the class.
>>
>>> +{
>>> + return class_register(&nvmem_class);
>>
>> I thought class was on the way out? Aren't we supposed to use bus types
>> for new stuff?
> Do you remember any conversation on the list about this? I could not
> find it on web.
>
> on the other hand, nvmem is not really a bus, making it a bus type
> sounds incorrect to me.
>
I found this post on the cpu class that Sudeep tried to introduce[1].
And there's this post from Kay that alludes to a unification of busses
and classes[2]. And some other post where Kay says class is dead [3].
[1] https://lkml.org/lkml/2014/8/21/191
[2] https://lwn.net/Articles/471821/
[3] https://lkml.org/lkml/2010/11/11/17
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
On 24/06/15 01:24, Stephen Boyd wrote:
> Can you assign the attributes to the device_type in the nvmem::struct
> device? I don't see why these attributes need to be part of the class.
>
I will fix this.
>>> >>
>>>> >>>+{
>>>> >>>+ return class_register(&nvmem_class);
>>> >>
>>> >>I thought class was on the way out? Aren't we supposed to use bus types
>>> >>for new stuff?
>> >Do you remember any conversation on the list about this? I could not
>> >find it on web.
>> >
>> >on the other hand, nvmem is not really a bus, making it a bus type
>> >sounds incorrect to me.
>> >
> I found this post on the cpu class that Sudeep tried to introduce[1].
> And there's this post from Kay that alludes to a unification of busses
> and classes[2]. And some other post where Kay says class is dead [3].
Thanks for the links,
Yep, looks like Class is dead, I will change the code to use bus type
instead.
>
> [1]https://lkml.org/lkml/2014/8/21/191
> [2]https://lwn.net/Articles/471821/
> [3]https://lkml.org/lkml/2010/11/11/17
--srini