[RFC PATCH 1/3] eeprom: Add a simple EEPROM framework
From: andrew@lunn.ch (Andrew Lunn)
Date: 2015-02-19 18:15:31
Also in:
linux-api, linux-devicetree, lkml
On Thu, Feb 19, 2015 at 05:08:28PM +0000, Srinivas Kandagatla wrote:
quoted hunk ↗ jump to hunk
From: Maxime Ripard <redacted> Up until now, EEPROM 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 EEPROMs. Having regmap interface to this framework would give much better abstraction for eeproms on different buses. Signed-off-by: Maxime Ripard <redacted> [srinivas.kandagatla: Moved to regmap based and cleanedup apis] Signed-off-by: Srinivas Kandagatla <redacted> --- .../devicetree/bindings/eeprom/eeprom.txt | 48 ++++ drivers/Kconfig | 2 + drivers/Makefile | 1 + drivers/eeprom/Kconfig | 19 ++ drivers/eeprom/Makefile | 9 + drivers/eeprom/core.c | 290 +++++++++++++++++++++ include/linux/eeprom-consumer.h | 73 ++++++ include/linux/eeprom-provider.h | 51 ++++ 8 files changed, 493 insertions(+) create mode 100644 Documentation/devicetree/bindings/eeprom/eeprom.txt create mode 100644 drivers/eeprom/Kconfig create mode 100644 drivers/eeprom/Makefile create mode 100644 drivers/eeprom/core.c create mode 100644 include/linux/eeprom-consumer.h create mode 100644 include/linux/eeprom-provider.hdiff --git a/Documentation/devicetree/bindings/eeprom/eeprom.txt b/Documentation/devicetree/bindings/eeprom/eeprom.txt new file mode 100644 index 0000000..9ec1ec2 --- /dev/null +++ b/Documentation/devicetree/bindings/eeprom/eeprom.txt@@ -0,0 +1,48 @@ += EEPROM Data Device Tree Bindings = + +This binding is intended to represent the location of hardware +configuration data stored in EEPROMs. + +On a significant proportion of boards, the manufacturer has stored +some data on an EEPROM-like device, 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 = + +Required properties: +#eeprom-cells: Number of cells in an eeprom specifier; The common + case is 2. + +For example: + + at24: eeprom at 42 { + #eeprom-cells = <2>; + }; + += Data consumers = + +Required properties: + +eeproms: List of phandle and data cell specifier triplet, one triplet + for each data cell the device might be interested in. The + triplet consists of the phandle to the eeprom provider, then + the offset in byte within that storage device, and the length
bytes
+ in byte of the data we care about.
bytes
+ +Optional properties: + +eeprom-names: List of data cell name strings sorted in the same order + as the resets property. Consumers drivers will use
resets? I guess this text was cut/paste from the reset documentation?\
+ eeprom-names to differentiate between multiple cells,
+ and hence being able to know what these cells are for.
+
+For example:
+
+ device {
+ eeproms = <&at24 14 42>;I like to use 42, but is it realistic to have a soc-rev-id which is 42 bytes long? How about using 42 as the offset and a sensible length of say 4?
quoted hunk ↗ jump to hunk
+ eeprom-names = "soc-rev-id"; + };diff --git a/drivers/Kconfig b/drivers/Kconfig index c70d6e4..d7afc82 100644 --- a/drivers/Kconfig +++ b/drivers/Kconfig@@ -184,4 +184,6 @@ source "drivers/thunderbolt/Kconfig" source "drivers/android/Kconfig" +source "drivers/eeprom/Kconfig" + endmenudiff --git a/drivers/Makefile b/drivers/Makefile index 527a6da..57eb5b0 100644 --- a/drivers/Makefile +++ b/drivers/Makefile@@ -165,3 +165,4 @@ obj-$(CONFIG_RAS) += ras/ obj-$(CONFIG_THUNDERBOLT) += thunderbolt/ obj-$(CONFIG_CORESIGHT) += coresight/ obj-$(CONFIG_ANDROID) += android/ +obj-$(CONFIG_EEPROM) += eeprom/diff --git a/drivers/eeprom/Kconfig b/drivers/eeprom/Kconfig new file mode 100644 index 0000000..2c5452a --- /dev/null +++ b/drivers/eeprom/Kconfig@@ -0,0 +1,19 @@ +menuconfig EEPROM + bool "EEPROM Support" + depends on OF + help + Support for EEPROM alike devices.
like.
+ + This framework is designed to provide a generic interface to EEPROM
EPROMs
quoted hunk ↗ jump to hunk
+ from both the Linux Kernel and the userspace. + + If unsure, say no. + +if EEPROM + +config EEPROM_DEBUG + bool "EEPROM debug support" + help + Say yes here to enable debugging support. + +endifdiff --git a/drivers/eeprom/Makefile b/drivers/eeprom/Makefile new file mode 100644 index 0000000..e130079 --- /dev/null +++ b/drivers/eeprom/Makefile@@ -0,0 +1,9 @@ +# +# Makefile for eeprom drivers. +# + +ccflags-$(CONFIG_EEPROM_DEBUG) += -DDEBUG + +obj-$(CONFIG_EEPROM) += core.o + +# Devicesdiff --git a/drivers/eeprom/core.c b/drivers/eeprom/core.c new file mode 100644 index 0000000..bc877a6 --- /dev/null +++ b/drivers/eeprom/core.c@@ -0,0 +1,290 @@ +/* + * EEPROM framework core. + * + * Copyright (C) 2015 Srinivas Kandagatla <srinivas.kandagatla@linaro.org> + * Copyright (C) 2013 Maxime Ripard <maxime.ripard@free-electrons.com> + * + * 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. + */ + +#include <linux/device.h> +#include <linux/eeprom-provider.h> +#include <linux/eeprom-consumer.h> +#include <linux/export.h> +#include <linux/fs.h> +#include <linux/idr.h> +#include <linux/init.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/slab.h> +#include <linux/uaccess.h> + +struct eeprom_cell { + struct eeprom_device *eeprom; + loff_t offset; + size_t count; +}; + +static DEFINE_MUTEX(eeprom_list_mutex); +static LIST_HEAD(eeprom_list); +static DEFINE_IDA(eeprom_ida); + +static ssize_t bin_attr_eeprom_read_write(struct kobject *kobj, + char *buf, loff_t offset, + size_t count, bool read) +{ + struct device *dev = container_of(kobj, struct device, kobj); + struct eeprom_device *eeprom = container_of(dev, struct eeprom_device, + edev); + int rc; + + if (offset > eeprom->size) + return 0; + + if (offset + count > eeprom->size) + count = eeprom->size - offset; + + if (read) + rc = regmap_bulk_read(eeprom->regmap, offset, + buf, count/eeprom->stride);
This division will round down, so you could get one less byte than what you expected, and the value you actually return. It seems like there should be a check here, the count is a multiple of stride and return an error if it is not.
+ else + rc = regmap_bulk_write(eeprom->regmap, offset, + buf, count/eeprom->stride); + + if (IS_ERR_VALUE(rc)) + return 0; +
I don't think returning 0 here, and above is the best thing to do. Return the real error code from regmap, or EINVAL or some other error code for going off the end of the eerpom.
+ return count;
+}
+
+static ssize_t bin_attr_eeprom_read(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *attr,
+ char *buf, loff_t offset, size_t count)
+{
+ return bin_attr_eeprom_read_write(kobj, buf, offset, count, true);
+}
+
+static ssize_t bin_attr_eeprom_write(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *attr,
+ char *buf, loff_t offset, size_t count)
+{
+ return bin_attr_eeprom_read_write(kobj, buf, offset, count, false);
+}These two functions seem to be identical. So just have one of them? +
+static struct bin_attribute bin_attr_eeprom = {
+ .attr = {
+ .name = "eeprom",
+ .mode = 0660,Symbolic values like S_IRUGO | S_IWUSR would be better. Are you also sure you want group write?
+ },
+ .read = bin_attr_eeprom_read,
+ .write = bin_attr_eeprom_write,
+};
+
+static struct bin_attribute *eeprom_bin_attributes[] = {
+ &bin_attr_eeprom,
+ NULL,
+};
+
+static const struct attribute_group eeprom_bin_group = {
+ .bin_attrs = eeprom_bin_attributes,
+};
+
+static const struct attribute_group *eeprom_dev_groups[] = {
+ &eeprom_bin_group,
+ NULL,
+};
+
+static struct class eeprom_class = {
+ .name = "eeprom",
+ .dev_groups = eeprom_dev_groups,
+};
+
+int eeprom_register(struct eeprom_device *eeprom)
+{
+ int rval;
+
+ if (!eeprom->regmap || !eeprom->size) {
+ dev_err(eeprom->dev, "Regmap not found\n");
+ return -EINVAL;
+ }
+
+ eeprom->id = ida_simple_get(&eeprom_ida, 0, 0, GFP_KERNEL);
+ if (eeprom->id < 0)
+ return eeprom->id;
+
+ eeprom->edev.class = &eeprom_class;
+ eeprom->edev.parent = eeprom->dev;
+ eeprom->edev.of_node = eeprom->dev ? eeprom->dev->of_node : NULL;
+ dev_set_name(&eeprom->edev, "eeprom%d", eeprom->id);
+
+ device_initialize(&eeprom->edev);
+
+ dev_dbg(&eeprom->edev, "Registering eeprom device %s\n",
+ dev_name(&eeprom->edev));
+
+ rval = device_add(&eeprom->edev);
+ if (rval)
+ return rval;
+
+ mutex_lock(&eeprom_list_mutex);
+ list_add(&eeprom->list, &eeprom_list);
+ mutex_unlock(&eeprom_list_mutex);
+
+ return 0;
+}
+EXPORT_SYMBOL(eeprom_register);
+
+int eeprom_unregister(struct eeprom_device *eeprom)
+{
+ device_del(&eeprom->edev);
+
+ mutex_lock(&eeprom_list_mutex);
+ list_del(&eeprom->list);
+ mutex_unlock(&eeprom_list_mutex);
+
+ return 0;
+}
+EXPORT_SYMBOL(eeprom_unregister);
+
+static struct eeprom_cell *__eeprom_cell_get(struct device_node *node,
+ int index)
+{
+ struct of_phandle_args args;
+ struct eeprom_cell *cell;
+ struct eeprom_device *e, *eeprom = NULL;
+ int ret;
+
+ ret = of_parse_phandle_with_args(node, "eeproms",
+ "#eeprom-cells", index, &args);
+ if (ret)
+ return ERR_PTR(ret);
+
+ if (args.args_count != 2)
+ return ERR_PTR(-EINVAL);
+
+ mutex_lock(&eeprom_list_mutex);
+
+ list_for_each_entry(e, &eeprom_list, list) {
+ if (args.np == e->edev.of_node) {
+ eeprom = e;
+ break;
+ }
+ }
+ mutex_unlock(&eeprom_list_mutex);Shouldn't you increment a reference count to the eeprom here? You are going to have trouble if the eeprom is unregistered and there is a call still referring to it.
+
+ if (!eeprom)
+ return ERR_PTR(-EPROBE_DEFER);
+
+ cell = kzalloc(sizeof(*cell), GFP_KERNEL);
+ if (!cell)
+ return ERR_PTR(-ENOMEM);
+
+ cell->eeprom = eeprom;
+ cell->offset = args.args[0];
+ cell->count = args.args[1];
+
+ return cell;
+}
+
+static struct eeprom_cell *__eeprom_cell_get_byname(struct device_node *node,
+ const char *id)
+{
+ int index = 0;
+
+ if (id)
+ index = of_property_match_string(node,
+ "eeprom-names",
+ id);
+ return __eeprom_cell_get(node, index);
+
+}
+
+struct eeprom_cell *eeprom_cell_get(struct device *dev, int index)
+{
+ if (!dev)
+ return ERR_PTR(-EINVAL);
+
+ /* First, attempt to retrieve the cell through the DT */
+ if (dev->of_node)
+ return __eeprom_cell_get(dev->of_node, index);
+
+ /* We don't support anything else yet */
+ return ERR_PTR(-ENODEV);
+}
+EXPORT_SYMBOL(eeprom_cell_get);
+
+struct eeprom_cell *eeprom_cell_get_byname(struct device *dev, const char *id)
+{
+ if (!dev)
+ return ERR_PTR(-EINVAL);
+
+ if (id && dev->of_node)
+ return __eeprom_cell_get_byname(dev->of_node, id);
+
+ /* We don't support anything else yet */
+ return ERR_PTR(-ENODEV);
+}
+EXPORT_SYMBOL(eeprom_cell_get_byname);
+
+void eeprom_cell_put(struct eeprom_cell *cell)
+{
+ kfree(cell);
+}
+EXPORT_SYMBOL(eeprom_cell_put);
+
+char *eeprom_cell_read(struct eeprom_cell *cell, ssize_t *len)
+{
+ struct eeprom_device *eeprom = cell->eeprom;
+ char *buf;
+ int rc;
+
+ if (!eeprom || !eeprom->regmap)
+ return ERR_PTR(-EINVAL);
+
+ buf = kzalloc(cell->count, GFP_KERNEL);
+ if (!buf)
+ return ERR_PTR(-ENOMEM);
+
+ rc = regmap_bulk_read(eeprom->regmap, cell->offset,
+ buf, cell->count/eeprom->stride);Same comment as above.
quoted hunk ↗ jump to hunk
+ if (IS_ERR_VALUE(rc)) { + kfree(buf); + return ERR_PTR(rc); + } + + *len = cell->count; + + return buf; +} +EXPORT_SYMBOL(eeprom_cell_read); + +int eeprom_cell_write(struct eeprom_cell *cell, const char *buf, ssize_t len) +{ + struct eeprom_device *eeprom = cell->eeprom; + + if (!eeprom || !eeprom->regmap) + return -EINVAL; + + return regmap_bulk_write(eeprom->regmap, cell->offset, + buf, cell->count/eeprom->stride); +} +EXPORT_SYMBOL(eeprom_cell_write); + +static int eeprom_init(void) +{ + return class_register(&eeprom_class); +} + +static void eeprom_exit(void) +{ + class_unregister(&eeprom_class); +} + +subsys_initcall(eeprom_init); +module_exit(eeprom_exit); + +MODULE_AUTHOR("Maxime Ripard <maxime.ripard@free-electrons.com"); +MODULE_DESCRIPTION("EEPROM Driver Core"); +MODULE_LICENSE("GPL");diff --git a/include/linux/eeprom-consumer.h b/include/linux/eeprom-consumer.h new file mode 100644 index 0000000..706ae9d --- /dev/null +++ b/include/linux/eeprom-consumer.h@@ -0,0 +1,73 @@ +/* + * EEPROM framework consumer. + * + * Copyright (C) 2015 Srinivas Kandagatla <srinivas.kandagatla@linaro.org> + * Copyright (C) 2013 Maxime Ripard <maxime.ripard@free-electrons.com> + * + * 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_EEPROM_CONSUMER_H +#define _LINUX_EEPROM_CONSUMER_H + +struct eeprom_cell; + +/** + * eeprom_cell_get(): Get eeprom cell of device form a given index.
of a device for a
+ * + * @dev: Device that will be interacted with + * @index: Index of the eeprom cell. + * + * The return value will be an ERR_PTR() on error or a valid pointer + * to a struct eeprom_cell. The eeprom_cell will be freed by the + * eeprom_cell_put(). + */ +struct eeprom_cell *eeprom_cell_get(struct device *dev, int index); + +/** + * eeprom_cell_get(): Get eeprom cell of device form a given name.
same again
+ * + * @dev: Device that will be interacted with + * @name: Name of the eeprom cell. + * + * The return value will be an ERR_PTR() on error or a valid pointer + * to a struct eeprom_cell. The eeprom_cell will be freed by the + * eeprom_cell_put(). + */ +struct eeprom_cell *eeprom_cell_get_byname(struct device *dev, + const char *name); + +/** + * eeprom_cell_put(): Release previously allocated eeprom cell. + * + * @cell: Previously allocated eeprom cell by eeprom_cell_get() + * or eeprom_cell_get_byname(). + */ +void eeprom_cell_put(struct eeprom_cell *cell); + +/** + * eeprom_cell_read(): Read a given eeprom cell + * + * @cell: eeprom 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(). + */ +char *eeprom_cell_read(struct eeprom_cell *cell, ssize_t *len);
Would void * be better than char *? I guess the contents is mostly
data, not strings.
Andrew
quoted hunk ↗ jump to hunk
+ +/** + * eeprom_cell_write(): Write to a given eeprom cell + * + * @cell: eeprom cell to be written. + * @buf: Buffer to be written. + * @len: length of buffer to be written to eeprom cell. + * + * The return value will be an non zero on error or a zero on successful write. + */ +int eeprom_cell_write(struct eeprom_cell *cell, const char *buf, ssize_t len); + +#endif /* ifndef _LINUX_EEPROM_CONSUMER_H */diff --git a/include/linux/eeprom-provider.h b/include/linux/eeprom-provider.h new file mode 100644 index 0000000..3943c2f --- /dev/null +++ b/include/linux/eeprom-provider.h@@ -0,0 +1,51 @@ +/* + * EEPROM framework provider. + * + * Copyright (C) 2015 Srinivas Kandagatla <srinivas.kandagatla@linaro.org> + * Copyright (C) 2013 Maxime Ripard <maxime.ripard@free-electrons.com> + * + * 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_EEPROM_PROVIDER_H +#define _LINUX_EEPROM_PROVIDER_H + +#include <linux/device.h> +#include <linux/regmap.h> +#include <linux/list.h> + +struct eeprom_device { + struct regmap *regmap; + int stride; + size_t size; + struct device *dev; + + /* Internal to framework */ + struct device edev; + int id; + struct list_head list; +}; + +/** + * eeprom_register(): Register a eeprom device for given eeprom. + * Also creates an binary entry in /sys/class/eeprom/eeprom[id]/eeprom + * + * @eeprom: eeprom device that needs to be created + * + * The return value will be an error code on error or a zero on success. + * The eeprom_device and sysfs entery will be freed by the eeprom_unregister(). + */ +int eeprom_register(struct eeprom_device *eeprom); + +/** + * eeprom_unregister(): Unregister previously registered eeprom device + * + * @eeprom: Pointer to previously registered eeprom device. + * + * The return value will be an non zero on error or a zero on success. + */ +int eeprom_unregister(struct eeprom_device *eeprom); + +#endif /* ifndef _LINUX_EEPROM_PROVIDER_H */-- 1.9.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel at lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel