Many I2C and SPI based devices implement register maps on top of the raw
wire interface. This is generally done in a very standard fashion by
devices, resulting in a lot of very similar code in drivers. For some
time now ASoC has factored this code out into the subsystem but that's
only useful for audio devices. The intention with this series is to
generalise the concept so that it can be used throughout the kernel.
It's not intended that this be suitable for all devices - some devices
have things that are hard to generalise like registers with variable
size and paging which are hard to support genericly. At the minute the
code is focused on the common cases. It is likely that the same code
could be used with other buses with similar properties to I2C and SPI.
Currently only physical I/O is handled, the intention is that once this
support has been reviewed and merged the generic register cache code
that ASoC includes will also be factored out too. For devices with read
heavy workloads (especially those that need a lot of read/modify/write
cycles) or which don't support read at all this can provide a useful
performance improvement and for sparse register maps there's a lot of
benefit in relatively complex cache code.
I'm not entirely happy with the implementation currently but am fairly
happy with the external interfaces.
A semi-random set of driver conversions have been included in the series
with various degrees of testing including the subsystem wide ASoC code.
Far more drivers could use the code, these have mostly just been done as
samples in a minimally invasive fashion.
There's a git branch at:
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap.git for-next
but due to various -next dependencies this does not currently include
all of the patches posted here. I've also created a regmap-asoc branch
there which merges current ASoC code with the regmap code.
MAINTAINERS | 7 +
drivers/Kconfig | 1 +
drivers/Makefile | 1 +
drivers/mfd/Kconfig | 4 +
drivers/mfd/pcf50633-core.c | 114 ++------
drivers/mfd/wm831x-core.c | 97 +++----
drivers/mfd/wm831x-i2c.c | 56 ----
drivers/mfd/wm831x-spi.c | 48 ----
drivers/mfd/wm8994-core.c | 175 ++----------
drivers/regmap/Kconfig | 6 +
drivers/regmap/Makefile | 3 +
drivers/regmap/regmap-i2c.c | 112 ++++++++
drivers/regmap/regmap-spi.c | 75 +++++
drivers/regmap/regmap.c | 478 ++++++++++++++++++++++++++++++++
drivers/regulator/Kconfig | 1 +
drivers/regulator/tps65023-regulator.c | 98 ++-----
include/linux/mfd/pcf50633/core.h | 3 +-
include/linux/mfd/wm831x/core.h | 7 +-
include/linux/mfd/wm8994/core.h | 9 +-
include/linux/regmap.h | 75 +++++
include/sound/soc.h | 2 +
sound/soc/Kconfig | 1 +
sound/soc/soc-io.c | 316 ++-------------------
23 files changed, 920 insertions(+), 769 deletions(-)
There are many places in the tree where we implement register access for
devices on non-memory mapped buses, especially I2C and SPI. Since hardware
designers seem to have settled on a relatively consistent set of register
interfaces this can be effectively factored out into shared code. There
are a standard set of formats for marshalling data for exchange with the
device, with the actual I/O mechanisms generally being simple byte
streams.
We create an abstraction for marshaling data into formats which can be
sent on the control interfaces, and create a standard method for
plugging in actual transport underneath that.
This is mostly a refactoring and renaming of the bottom level of the
existing code for sharing register I/O which we have in ASoC. A
subsequent patch in this series converts ASoC to use this. The main
difference in interface is that reads return values by writing to a
location provided by a pointer rather than in the return value, ensuring
we can use the full range of the type for register data. We also use
unsigned types rather than ints for the same reason.
As some of the devices can have very large register maps the existing
ASoC code also contains infrastructure for managing register caches.
This cache work will be moved over in a future stage to allow for
separate review, the current patch only deals with the physical I/O.
We also only deal with access to a single register at a time initially
as this is the most common case.
Signed-off-by: Mark Brown <[email protected]>
---
MAINTAINERS | 7 +
drivers/Kconfig | 1 +
drivers/Makefile | 1 +
drivers/regmap/Kconfig | 6 +
drivers/regmap/Makefile | 2 +
drivers/regmap/regmap.c | 478 +++++++++++++++++++++++++++++++++++++++++++++++
include/linux/regmap.h | 75 ++++++++
7 files changed, 570 insertions(+), 0 deletions(-)
create mode 100644 drivers/regmap/Kconfig
create mode 100644 drivers/regmap/Makefile
create mode 100644 drivers/regmap/regmap.c
create mode 100644 include/linux/regmap.h
diff --git a/MAINTAINERS b/MAINTAINERS
index a26e9ec..bd2ec03 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5318,6 +5318,13 @@ L: [email protected]
S: Supported
F: fs/reiserfs/
+REGISTER MAP ABSTRACTION
+M: Mark Brown <[email protected]>
+T: git git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap.git
+S: Supported
+F: drivers/regmap/
+F: include/linux/regmap.h
+
RFKILL
M: Johannes Berg <[email protected]>
L: [email protected]
diff --git a/drivers/Kconfig b/drivers/Kconfig
index 3bb154d..8c55790 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -126,4 +126,5 @@ source "drivers/hwspinlock/Kconfig"
source "drivers/clocksource/Kconfig"
+source "drivers/regmap/Kconfig"
endmenu
diff --git a/drivers/Makefile b/drivers/Makefile
index 09f3232..78ab0d6 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -116,6 +116,7 @@ obj-$(CONFIG_BCMA) += bcma/
obj-$(CONFIG_VHOST_NET) += vhost/
obj-$(CONFIG_VLYNQ) += vlynq/
obj-$(CONFIG_STAGING) += staging/
+obj-$(CONFIG_REGMAP) += regmap/
obj-y += platform/
obj-y += ieee802154/
#common clk code
diff --git a/drivers/regmap/Kconfig b/drivers/regmap/Kconfig
new file mode 100644
index 0000000..fc0eb1d
--- /dev/null
+++ b/drivers/regmap/Kconfig
@@ -0,0 +1,6 @@
+# Generic register map support. There are no user servicable options here,
+# this is an API intended to be used by other kernel subsystems. These
+# subsystems should select the appropriate symbols.
+
+config REGMAP
+ bool
diff --git a/drivers/regmap/Makefile b/drivers/regmap/Makefile
new file mode 100644
index 0000000..1e5037e
--- /dev/null
+++ b/drivers/regmap/Makefile
@@ -0,0 +1,2 @@
+obj-$(CONFIG_REGMAP) += regmap.o
+
diff --git a/drivers/regmap/regmap.c b/drivers/regmap/regmap.c
new file mode 100644
index 0000000..a8610c7
--- /dev/null
+++ b/drivers/regmap/regmap.c
@@ -0,0 +1,478 @@
+/*
+ * Register map access API
+ *
+ * Copyright 2011 Wolfson Microelectronics plc
+ *
+ * Author: Mark Brown <[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 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/slab.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/err.h>
+
+#include <linux/regmap.h>
+
+struct regmap;
+
+static DEFINE_MUTEX(regmap_bus_lock);
+static LIST_HEAD(regmap_bus_list);
+
+struct regmap_format {
+ size_t buf_size;
+ size_t reg_bytes;
+ size_t val_bytes;
+ void (*format_write)(struct regmap *map,
+ unsigned int reg, unsigned int val);
+ void (*format_reg)(void *buf, unsigned int reg);
+ void (*format_val)(void *buf, unsigned int val);
+ unsigned int (*parse_val)(void *buf);
+};
+
+struct regmap {
+ struct mutex lock;
+
+ struct device *dev; /* Device we do I/O on */
+ void *work_buf; /* Scratch buffer used to format I/O */
+ struct regmap_format format; /* Buffer format */
+ struct regmap_bus *bus;
+};
+
+static void regmap_format_4_12_write(struct regmap *map,
+ unsigned int reg, unsigned int val)
+{
+ u16 *out = map->work_buf;
+ *out = cpu_to_be16((reg << 12) | val);
+}
+
+static void regmap_format_7_9_write(struct regmap *map,
+ unsigned int reg, unsigned int val)
+{
+ u16 *out = map->work_buf;
+ *out = cpu_to_be16((reg << 9) | val);
+}
+
+static void regmap_format_8(void *buf, unsigned int val)
+{
+ u8 *b = buf;
+
+ b[0] = val;
+}
+
+static void regmap_format_16(void *buf, unsigned int val)
+{
+ u16 *b = buf;
+
+ b[0] = cpu_to_be16(val);
+}
+
+static unsigned int regmap_parse_8(void *buf)
+{
+ u8 *b = buf;
+
+ return b[0];
+}
+
+static unsigned int regmap_parse_16(void *buf)
+{
+ u16 *b = buf;
+
+ return be16_to_cpu(b[0]);
+}
+
+/**
+ * remap_init: Initialise register map
+ *
+ * dev: Device that will be interacted with
+ * config: Configuration for register map
+ *
+ * The return value will be an ERR_PTR() on error or a valid pointer
+ * to a struct regmap.
+ */
+struct regmap *regmap_init(struct device *dev, struct regmap_config *config)
+{
+ struct regmap *map;
+ int ret = -EINVAL;
+
+ map = kzalloc(sizeof(*map), GFP_KERNEL);
+ if (map == NULL) {
+ ret = -ENOMEM;
+ goto err;
+ }
+
+ mutex_init(&map->lock);
+ map->format.buf_size = (config->reg_bits + config->val_bits) / 8;
+ map->format.reg_bytes = config->reg_bits / 8;
+ map->format.val_bytes = config->val_bits / 8;
+ map->dev = dev;
+
+ switch (config->reg_bits) {
+ case 4:
+ switch (config->val_bits) {
+ case 12:
+ map->format.format_write = regmap_format_4_12_write;
+ break;
+ default:
+ goto err_map;
+ }
+ break;
+
+ case 7:
+ switch (config->val_bits) {
+ case 9:
+ map->format.format_write = regmap_format_7_9_write;
+ break;
+ default:
+ goto err_map;
+ }
+ break;
+
+ case 8:
+ map->format.format_reg = regmap_format_8;
+ break;
+
+ case 16:
+ map->format.format_reg = regmap_format_16;
+ break;
+
+ default:
+ goto err_map;
+ }
+
+ switch (config->val_bits) {
+ case 8:
+ map->format.format_val = regmap_format_8;
+ map->format.parse_val = regmap_parse_8;
+ break;
+ case 16:
+ map->format.format_val = regmap_format_16;
+ map->format.parse_val = regmap_parse_16;
+ break;
+ }
+
+ if (!map->format.format_write &&
+ !(map->format.format_reg && map->format.format_val))
+ goto err_map;
+
+ /* Figure out which bus to use, and also grab a lock on the
+ * module supplying it. */
+ mutex_lock(®map_bus_lock);
+ list_for_each_entry(map->bus, ®map_bus_list, list)
+ if (map->bus->type == dev->bus &&
+ try_module_get(map->bus->owner))
+ break;
+ mutex_unlock(®map_bus_lock);
+
+ if (map->bus == NULL) {
+ ret = -EINVAL;
+ goto err_map;
+ }
+
+ map->work_buf = kmalloc(map->format.buf_size, GFP_KERNEL);
+ if (map->work_buf == NULL) {
+ ret = -ENOMEM;
+ goto err_bus;
+ }
+
+ return map;
+
+err_bus:
+ module_put(map->bus->owner);
+err_map:
+ kfree(map);
+err:
+ return ERR_PTR(ret);
+}
+EXPORT_SYMBOL_GPL(regmap_init);
+
+/**
+ * regmap_free: Free a previously allocated register map
+ */
+void regmap_free(struct regmap *map)
+{
+ kfree(map->work_buf);
+ module_put(map->bus->owner);
+ kfree(map);
+}
+EXPORT_SYMBOL_GPL(regmap_free);
+
+static int _regmap_raw_write(struct regmap *map, unsigned int reg,
+ const void *val, size_t val_len)
+{
+ void *buf;
+ int ret = -ENOTSUPP;
+ int len;
+
+ map->format.format_reg(map->work_buf, reg);
+
+ /* Try to do a gather write if we can */
+ if (map->bus->gather_write)
+ ret = map->bus->gather_write(map->dev, map->work_buf,
+ map->format.reg_bytes,
+ val, val_len);
+
+ /* Otherwise fall back on linearising by hand. */
+ if (ret == -ENOTSUPP) {
+ len = map->format.reg_bytes + val_len;
+ buf = kmalloc(len, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ memcpy(buf, map->work_buf, map->format.reg_bytes);
+ memcpy(buf + map->format.reg_bytes, val, val_len);
+ ret = map->bus->write(map->dev, buf, len);
+
+ kfree(buf);
+ }
+
+ return ret;
+}
+
+static int _regmap_write(struct regmap *map, unsigned int reg,
+ unsigned int val)
+{
+ BUG_ON(!map->format.format_write && !map->format.format_val);
+
+ if (map->format.format_write) {
+ map->format.format_write(map, reg, val);
+
+ return map->bus->write(map->dev, map->work_buf,
+ map->format.buf_size);
+ } else {
+ map->format.format_val(map->work_buf + map->format.reg_bytes,
+ val);
+ return _regmap_raw_write(map, reg,
+ map->work_buf + map->format.reg_bytes,
+ map->format.val_bytes);
+ }
+}
+
+/**
+ * regmap_write: Write a value to a single register
+ *
+ * @map: Register map to write to
+ * @reg: Register to write to
+ * @val: Value to be written
+ *
+ * A value of zero will be returned on success, a negative errno will
+ * be returned in error cases.
+ */
+int regmap_write(struct regmap *map, unsigned int reg, unsigned int val)
+{
+ int ret;
+
+ mutex_lock(&map->lock);
+
+ ret = _regmap_write(map, reg, val);
+
+ mutex_unlock(&map->lock);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(regmap_write);
+
+/**
+ * regmap_raw_write: Write raw values to one or more registers
+ *
+ * @map: Register map to write to
+ * @reg: Initial register to write to
+ * @val: Block of data to be written, laid out for direct transmission to the
+ * device
+ * @val_len: Length of data pointed to by val.
+ *
+ * This function is intended to be used for things like firmware
+ * download where a large block of data needs to be transferred to the
+ * device. No formatting will be done on the data provided.
+ *
+ * A value of zero will be returned on success, a negative errno will
+ * be returned in error cases.
+ */
+int regmap_raw_write(struct regmap *map, unsigned int reg,
+ const void *val, size_t val_len)
+{
+ int ret;
+
+ mutex_lock(&map->lock);
+
+ ret = _regmap_raw_write(map, reg, val, val_len);
+
+ mutex_unlock(&map->lock);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(regmap_raw_write);
+
+static int _regmap_raw_read(struct regmap *map, unsigned int reg, void *val,
+ unsigned int val_len)
+{
+ u8 *u8 = map->work_buf;
+ int ret;
+
+ map->format.format_reg(map->work_buf, reg);
+
+ /*
+ * Some buses flag reads by setting the high bit in the
+ * register addresss; since it's always the high bit for all
+ * current formats we can do this here rather than in
+ * formatting. This may break if we get interesting formats.
+ */
+ if (map->bus->read_flag_in_reg)
+ u8[0] |= 0x80;
+
+ ret = map->bus->read(map->dev, map->work_buf, map->format.reg_bytes,
+ val, map->format.val_bytes);
+ if (ret != 0)
+ return ret;
+
+ return 0;
+}
+
+static int _regmap_read(struct regmap *map, unsigned int reg,
+ unsigned int *val)
+{
+ int ret;
+
+ if (!map->format.parse_val)
+ return -EINVAL;
+
+ ret = _regmap_raw_read(map, reg, map->work_buf, map->format.val_bytes);
+ if (ret == 0)
+ *val = map->format.parse_val(map->work_buf);
+
+ return ret;
+}
+
+/**
+ * regmap_read: Read a value from a single register
+ *
+ * @map: Register map to write to
+ * @reg: Register to be read from
+ * @val: Pointer to store read value
+ *
+ * A value of zero will be returned on success, a negative errno will
+ * be returned in error cases.
+ */
+int regmap_read(struct regmap *map, unsigned int reg, unsigned int *val)
+{
+ int ret;
+
+ mutex_lock(&map->lock);
+
+ ret = _regmap_read(map, reg, val);
+
+ mutex_unlock(&map->lock);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(regmap_read);
+
+/**
+ * regmap_raw_read: Read raw data from the device
+ *
+ * @map: Register map to write to
+ * @reg: First register to be read from
+ * @val: Pointer to store read value
+ * @val_len: Size of data to read
+ *
+ * A value of zero will be returned on success, a negative errno will
+ * be returned in error cases.
+ */
+int regmap_raw_read(struct regmap *map, unsigned int reg, void *val,
+ size_t val_len)
+{
+ int ret;
+
+ mutex_lock(&map->lock);
+
+ ret = _regmap_raw_read(map, reg, val, val_len);
+
+ mutex_unlock(&map->lock);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(regmap_raw_read);
+
+/**
+ * regmap_bulk_read: Read multiple registers from the device
+ *
+ * @map: Register map to write to
+ * @reg: First register to be read from
+ * @val: Pointer to store read value, in native register size for device
+ * @val_count: Number of registers to read
+ *
+ * A value of zero will be returned on success, a negative errno will
+ * be returned in error cases.
+ */
+int regmap_bulk_read(struct regmap *map, unsigned int reg, void *val,
+ size_t val_count)
+{
+ int ret, i;
+ size_t val_bytes = map->format.val_bytes;
+
+ if (!map->format.parse_val)
+ return -EINVAL;
+
+ ret = regmap_raw_read(map, reg, val, val_bytes);
+ if (ret != 0)
+ return ret;
+
+ for (i = 0; i < val_count * val_bytes; i += val_bytes)
+ map->format.parse_val(val + i);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(regmap_bulk_read);
+
+/**
+ * remap_update_bits: Perform a read/modify/write cycle on the register map
+ *
+ * @map: Register map to update
+ * @reg: Register to update
+ * @mask: Bitmask to change
+ * @val: New value for bitmask
+ *
+ * Returns zero for success, a negative number on error.
+ */
+int regmap_update_bits(struct regmap *map, unsigned int reg,
+ unsigned int mask, unsigned int val)
+{
+ int ret;
+ unsigned int tmp;
+
+ mutex_lock(&map->lock);
+
+ ret = _regmap_read(map, reg, &tmp);
+ if (ret != 0)
+ goto out;
+
+ tmp &= ~mask;
+ tmp |= val;
+
+ ret = _regmap_write(map, reg, tmp);
+
+out:
+ mutex_unlock(&map->lock);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(regmap_update_bits);
+
+int regmap_add_bus(struct regmap_bus *bus)
+{
+ mutex_lock(®map_bus_lock);
+ list_add(&bus->list, ®map_bus_list);
+ mutex_unlock(®map_bus_lock);
+}
+EXPORT_SYMBOL_GPL(regmap_add_bus);
+
+int regmap_del_bus(struct regmap_bus *bus)
+{
+ mutex_lock(®map_bus_lock);
+ list_del(&bus->list);
+ mutex_unlock(®map_bus_lock);
+}
+EXPORT_SYMBOL_GPL(regmap_del_bus);
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
new file mode 100644
index 0000000..0c2e402
--- /dev/null
+++ b/include/linux/regmap.h
@@ -0,0 +1,75 @@
+#ifndef __LINUX_REGMAP_H
+#define __LINUX_REGMAP_H
+
+/*
+ * Register map access API
+ *
+ * Copyright 2011 Wolfson Microelectronics plc
+ *
+ * Author: Mark Brown <[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 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/device.h>
+#include <linux/list.h>
+
+struct regmap_config {
+ int reg_bits;
+ int val_bits;
+};
+
+typedef int (*regmap_hw_write)(struct device *dev, const void *data,
+ size_t count);
+typedef int (*regmap_hw_gather_write)(struct device *dev,
+ const void *reg, size_t reg_len,
+ const void *val, size_t val_len);
+typedef int (*regmap_hw_read)(struct device *dev,
+ const void *reg_buf, size_t reg_size,
+ void *val_buf, size_t val_size);
+
+/**
+ * Description of a hardware bus for the register map infrastructure.
+ *
+ * @list: Internal use.
+ * @type: Bus type, used to identify bus to be used for a device.
+ * @write: Write operation.
+ * @gather_write: Write operation with split register/value, return -ENOTSUPP
+ * if not implemented on a given device.
+ * @read: Read operation. Data is returned in the buffer used to transmit
+ * data.
+ * @owner: Module with the bus implementation, used to pin the implementation
+ * in memory.
+ * @read_flag_in_reg: Reads are flagged by setting the high bit in the
+ * register (frequently used by buses which don't
+ * have a bus-native way to indicate reads).
+ */
+struct regmap_bus {
+ struct list_head list;
+ struct bus_type *type;
+ regmap_hw_write write;
+ regmap_hw_gather_write gather_write;
+ regmap_hw_read read;
+ struct module *owner;
+ bool read_flag_in_reg;
+};
+
+struct regmap *regmap_init(struct device *dev, struct regmap_config *config);
+void regmap_free(struct regmap *map);
+int regmap_write(struct regmap *map, unsigned int reg, unsigned int val);
+int regmap_raw_write(struct regmap *map, unsigned int reg,
+ const void *val, size_t val_len);
+int regmap_read(struct regmap *map, unsigned int reg, unsigned int *val);
+int regmap_raw_read(struct regmap *map, unsigned int reg,
+ void *val, size_t val_len);
+int regmap_bulk_read(struct regmap *map, unsigned int reg, void *val,
+ size_t val_count);
+int regmap_update_bits(struct regmap *map, unsigned int reg,
+ unsigned int mask, unsigned int val);
+
+void regmap_add_bus(struct regmap_bus *bus);
+void regmap_del_bus(struct regmap_bus *bus);
+
+#endif
--
1.7.5.4
We initialise at postcore_initcall() so that we are available before users
- some users such as PMICs initialise very early. We won't actually try to
use any of the bus until a device initialises a register map.
Signed-off-by: Mark Brown <[email protected]>
---
drivers/regmap/Makefile | 2 +-
drivers/regmap/regmap-i2c.c | 112 +++++++++++++++++++++++++++++++++++++++++++
drivers/regmap/regmap.c | 4 +-
3 files changed, 115 insertions(+), 3 deletions(-)
create mode 100644 drivers/regmap/regmap-i2c.c
diff --git a/drivers/regmap/Makefile b/drivers/regmap/Makefile
index 1e5037e..641c20a 100644
--- a/drivers/regmap/Makefile
+++ b/drivers/regmap/Makefile
@@ -1,2 +1,2 @@
obj-$(CONFIG_REGMAP) += regmap.o
-
+obj-$(CONFIG_I2C) += regmap-i2c.o
diff --git a/drivers/regmap/regmap-i2c.c b/drivers/regmap/regmap-i2c.c
new file mode 100644
index 0000000..4a61560
--- /dev/null
+++ b/drivers/regmap/regmap-i2c.c
@@ -0,0 +1,112 @@
+/*
+ * Register map access API - I2C support
+ *
+ * Copyright 2011 Wolfson Microelectronics plc
+ *
+ * Author: Mark Brown <[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 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/regmap.h>
+#include <linux/i2c.h>
+
+static int regmap_i2c_write(struct device *dev, const void *data, size_t count)
+{
+ struct i2c_client *i2c = to_i2c_client(dev);
+ int ret;
+
+ ret = i2c_master_send(i2c, data, count);
+ if (ret == count)
+ return 0;
+ else if (ret < 0)
+ return ret;
+ else
+ return -EIO;
+}
+
+static int regmap_i2c_gather_write(struct device *dev,
+ const void *reg, size_t reg_size,
+ const void *val, size_t val_size)
+{
+ struct i2c_client *i2c = to_i2c_client(dev);
+ struct i2c_msg xfer[2];
+ int ret;
+
+ /* If the I2C controller can't do a gather tell the core, it
+ * will substitute in a linear write for us.
+ */
+ if (!i2c_check_functionality(i2c->adapter, I2C_FUNC_PROTOCOL_MANGLING))
+ return -ENOTSUPP;
+
+ xfer[0].addr = i2c->addr;
+ xfer[0].flags = 0;
+ xfer[0].len = reg_size;
+ xfer[0].buf = (void *)reg;
+
+ xfer[1].addr = i2c->addr;
+ xfer[1].flags = I2C_M_NOSTART;
+ xfer[1].len = val_size;
+ xfer[1].buf = (void *)val;
+
+ ret = i2c_transfer(i2c->adapter, xfer, 2);
+ if (ret == 2)
+ return 0;
+ if (ret < 0)
+ return ret;
+ else
+ return -EIO;
+}
+
+static int regmap_i2c_read(struct device *dev,
+ const void *reg, size_t reg_size,
+ void *val, size_t val_size)
+{
+ struct i2c_client *i2c = to_i2c_client(dev);
+ struct i2c_msg xfer[2];
+ int ret;
+
+ if (!i2c_check_functionality(i2c->adapter, I2C_FUNC_PROTOCOL_MANGLING))
+ return -ENOTSUPP;
+
+ xfer[0].addr = i2c->addr;
+ xfer[0].flags = 0;
+ xfer[0].len = reg_size;
+ xfer[0].buf = (void *)reg;
+
+ xfer[1].addr = i2c->addr;
+ xfer[1].flags = I2C_M_RD;
+ xfer[1].len = val_size;
+ xfer[1].buf = val;
+
+ ret = i2c_transfer(i2c->adapter, xfer, 2);
+ if (ret == 2)
+ return 0;
+ else if (ret < 0)
+ return ret;
+ else
+ return -EIO;
+}
+
+static struct regmap_bus regmap_i2c = {
+ .type = &i2c_bus_type,
+ .write = regmap_i2c_write,
+ .gather_write = regmap_i2c_gather_write,
+ .read = regmap_i2c_read,
+ .owner = THIS_MODULE,
+};
+
+static int __init regmap_i2c_init(void)
+{
+ regmap_add_bus(®map_i2c);
+ return 0;
+}
+postcore_initcall(regmap_i2c_init);
+
+static void __exit regmap_i2c_exit(void)
+{
+ regmap_del_bus(®map_i2c);
+}
+module_exit(regmap_i2c_exit);
diff --git a/drivers/regmap/regmap.c b/drivers/regmap/regmap.c
index a8610c7..896b710 100644
--- a/drivers/regmap/regmap.c
+++ b/drivers/regmap/regmap.c
@@ -461,7 +461,7 @@ out:
}
EXPORT_SYMBOL_GPL(regmap_update_bits);
-int regmap_add_bus(struct regmap_bus *bus)
+void regmap_add_bus(struct regmap_bus *bus)
{
mutex_lock(®map_bus_lock);
list_add(&bus->list, ®map_bus_list);
@@ -469,7 +469,7 @@ int regmap_add_bus(struct regmap_bus *bus)
}
EXPORT_SYMBOL_GPL(regmap_add_bus);
-int regmap_del_bus(struct regmap_bus *bus)
+void regmap_del_bus(struct regmap_bus *bus)
{
mutex_lock(®map_bus_lock);
list_del(&bus->list);
--
1.7.5.4
We initialise at postcore_initcall() so that we are available before users
- some users such as PMICs initialise very early. We won't actually try to
use any of the bus until a device initialises a register map.
Signed-off-by: Mark Brown <[email protected]>
---
drivers/regmap/Makefile | 1 +
drivers/regmap/regmap-spi.c | 75 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 76 insertions(+), 0 deletions(-)
create mode 100644 drivers/regmap/regmap-spi.c
diff --git a/drivers/regmap/Makefile b/drivers/regmap/Makefile
index 641c20a..74c6680 100644
--- a/drivers/regmap/Makefile
+++ b/drivers/regmap/Makefile
@@ -1,2 +1,3 @@
obj-$(CONFIG_REGMAP) += regmap.o
obj-$(CONFIG_I2C) += regmap-i2c.o
+obj-$(CONFIG_SPI) += regmap-spi.o
diff --git a/drivers/regmap/regmap-spi.c b/drivers/regmap/regmap-spi.c
new file mode 100644
index 0000000..45fe4c4
--- /dev/null
+++ b/drivers/regmap/regmap-spi.c
@@ -0,0 +1,75 @@
+/*
+ * Register map access API - SPI support
+ *
+ * Copyright 2011 Wolfson Microelectronics plc
+ *
+ * Author: Mark Brown <[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 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/regmap.h>
+#include <linux/spi/spi.h>
+
+static int regmap_spi_write(struct device *dev, const void *data, size_t count)
+{
+ struct spi_device *spi = to_spi_device(dev);
+
+ return spi_write(spi, data, count);
+}
+
+static int regmap_spi_gather_write(struct device *dev,
+ const void *reg, size_t reg_len,
+ const void *val, size_t val_len)
+{
+ struct spi_device *spi = to_spi_device(dev);
+ struct spi_message m;
+ struct spi_transfer t[2];
+
+ spi_message_init(&m);
+
+ memset(&t, 0, sizeof(t));
+
+ t[0].tx_buf = reg;
+ t[0].len = reg_len;
+ spi_message_add_tail(&t[0], &m);
+
+ t[1].tx_buf = val;
+ t[1].len = val_len;
+ spi_message_add_tail(&t[0], &m);
+
+ return spi_sync(spi, &m);
+}
+
+static int regmap_spi_read(struct device *dev,
+ const void *reg, size_t reg_size,
+ void *val, size_t val_size)
+{
+ struct spi_device *spi = to_spi_device(dev);
+
+ return spi_write_then_read(spi, reg, reg_size, val, val_size);
+}
+
+static struct regmap_bus regmap_spi = {
+ .type = &spi_bus_type,
+ .write = regmap_spi_write,
+ .gather_write = regmap_spi_gather_write,
+ .read = regmap_spi_read,
+ .owner = THIS_MODULE,
+ .read_flag_in_reg = true,
+};
+
+static int __init regmap_spi_init(void)
+{
+ regmap_add_bus(®map_spi);
+ return 0;
+}
+postcore_initcall(regmap_spi_init);
+
+static void __exit regmap_spi_exit(void)
+{
+ regmap_del_bus(®map_spi);
+}
+module_exit(regmap_spi_exit);
--
1.7.5.4
Remove all the ASoC specific physical I/O code and replace it with calls
into the regmap API. The bulk write code can only be used safely if all
regmap calls are locked with the CODEC lock, we need to add bulk support
to the regmap API or replace the code with an open coded loop (though
currently it has no users...).
Signed-off-by: Mark Brown <[email protected]>
---
include/sound/soc.h | 2 +
sound/soc/Kconfig | 1 +
sound/soc/soc-io.c | 316 ++++-----------------------------------------------
3 files changed, 28 insertions(+), 291 deletions(-)
diff --git a/include/sound/soc.h b/include/sound/soc.h
index 6424b10..40436b8 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -19,6 +19,7 @@
#include <linux/workqueue.h>
#include <linux/interrupt.h>
#include <linux/kernel.h>
+#include <linux/regmap.h>
#include <sound/core.h>
#include <sound/pcm.h>
#include <sound/control.h>
@@ -570,6 +571,7 @@ struct snd_soc_codec {
const void *reg_def_copy;
const struct snd_soc_cache_ops *cache_ops;
struct mutex cache_rw_mutex;
+ int val_bytes;
/* dapm */
struct snd_soc_dapm_context dapm;
diff --git a/sound/soc/Kconfig b/sound/soc/Kconfig
index 8224db5..32e5fd4 100644
--- a/sound/soc/Kconfig
+++ b/sound/soc/Kconfig
@@ -7,6 +7,7 @@ menuconfig SND_SOC
select SND_PCM
select AC97_BUS if SND_SOC_AC97_BUS
select SND_JACK if INPUT=y || INPUT=SND
+ select REGMAP
---help---
If you want ASoC support, you should say Y here and also to the
diff --git a/sound/soc/soc-io.c b/sound/soc/soc-io.c
index cca490c..5607cb6 100644
--- a/sound/soc/soc-io.c
+++ b/sound/soc/soc-io.c
@@ -13,26 +13,13 @@
#include <linux/i2c.h>
#include <linux/spi/spi.h>
+#include <linux/regmap.h>
#include <sound/soc.h>
#include <trace/events/asoc.h>
-#ifdef CONFIG_SPI_MASTER
-static int do_spi_write(void *control, const char *data, int len)
-{
- struct spi_device *spi = control;
- int ret;
-
- ret = spi_write(spi, data, len);
- if (ret < 0)
- return ret;
-
- return len;
-}
-#endif
-
-static int do_hw_write(struct snd_soc_codec *codec, unsigned int reg,
- unsigned int value, const void *data, int len)
+static int hw_write(struct snd_soc_codec *codec, unsigned int reg,
+ unsigned int value)
{
int ret;
@@ -49,13 +36,7 @@ static int do_hw_write(struct snd_soc_codec *codec, unsigned int reg,
return 0;
}
- ret = codec->hw_write(codec->control_data, data, len);
- if (ret == len)
- return 0;
- if (ret < 0)
- return ret;
- else
- return -EIO;
+ return regmap_write(codec->control_data, reg, value);
}
static unsigned int hw_read(struct snd_soc_codec *codec, unsigned int reg)
@@ -69,8 +50,11 @@ static unsigned int hw_read(struct snd_soc_codec *codec, unsigned int reg)
if (codec->cache_only)
return -1;
- BUG_ON(!codec->hw_read);
- return codec->hw_read(codec, reg);
+ ret = regmap_read(codec->control_data, reg, &val);
+ if (ret == 0)
+ return val;
+ else
+ return ret;
}
ret = snd_soc_cache_read(codec, reg, &val);
@@ -79,183 +63,18 @@ static unsigned int hw_read(struct snd_soc_codec *codec, unsigned int reg)
return val;
}
-static int snd_soc_4_12_write(struct snd_soc_codec *codec, unsigned int reg,
- unsigned int value)
-{
- u16 data;
-
- data = cpu_to_be16((reg << 12) | (value & 0xffffff));
-
- return do_hw_write(codec, reg, value, &data, 2);
-}
-
-static int snd_soc_7_9_write(struct snd_soc_codec *codec, unsigned int reg,
- unsigned int value)
-{
- u16 data;
-
- data = cpu_to_be16((reg << 9) | (value & 0x1ff));
-
- return do_hw_write(codec, reg, value, &data, 2);
-}
-
-static int snd_soc_8_8_write(struct snd_soc_codec *codec, unsigned int reg,
- unsigned int value)
-{
- u8 data[2];
-
- reg &= 0xff;
- data[0] = reg;
- data[1] = value & 0xff;
-
- return do_hw_write(codec, reg, value, data, 2);
-}
-
-static int snd_soc_8_16_write(struct snd_soc_codec *codec, unsigned int reg,
- unsigned int value)
-{
- u8 data[3];
- u16 val = cpu_to_be16(value);
-
- data[0] = reg;
- memcpy(&data[1], &val, sizeof(val));
-
- return do_hw_write(codec, reg, value, data, 3);
-}
-
-#if defined(CONFIG_I2C) || (defined(CONFIG_I2C_MODULE) && defined(MODULE))
-static unsigned int do_i2c_read(struct snd_soc_codec *codec,
- void *reg, int reglen,
- void *data, int datalen)
-{
- struct i2c_msg xfer[2];
- int ret;
- struct i2c_client *client = codec->control_data;
-
- /* Write register */
- xfer[0].addr = client->addr;
- xfer[0].flags = 0;
- xfer[0].len = reglen;
- xfer[0].buf = reg;
-
- /* Read data */
- xfer[1].addr = client->addr;
- xfer[1].flags = I2C_M_RD;
- xfer[1].len = datalen;
- xfer[1].buf = data;
-
- ret = i2c_transfer(client->adapter, xfer, 2);
- if (ret == 2)
- return 0;
- else if (ret < 0)
- return ret;
- else
- return -EIO;
-}
-#endif
-
-#if defined(CONFIG_I2C) || (defined(CONFIG_I2C_MODULE) && defined(MODULE))
-static unsigned int snd_soc_8_8_read_i2c(struct snd_soc_codec *codec,
- unsigned int r)
-{
- u8 reg = r;
- u8 data;
- int ret;
-
- ret = do_i2c_read(codec, ®, 1, &data, 1);
- if (ret < 0)
- return 0;
- return data;
-}
-#else
-#define snd_soc_8_8_read_i2c NULL
-#endif
-
-#if defined(CONFIG_I2C) || (defined(CONFIG_I2C_MODULE) && defined(MODULE))
-static unsigned int snd_soc_8_16_read_i2c(struct snd_soc_codec *codec,
- unsigned int r)
-{
- u8 reg = r;
- u16 data;
- int ret;
-
- ret = do_i2c_read(codec, ®, 1, &data, 2);
- if (ret < 0)
- return 0;
- return (data >> 8) | ((data & 0xff) << 8);
-}
-#else
-#define snd_soc_8_16_read_i2c NULL
-#endif
-
-#if defined(CONFIG_I2C) || (defined(CONFIG_I2C_MODULE) && defined(MODULE))
-static unsigned int snd_soc_16_8_read_i2c(struct snd_soc_codec *codec,
- unsigned int r)
-{
- u16 reg = r;
- u8 data;
- int ret;
-
- ret = do_i2c_read(codec, ®, 2, &data, 1);
- if (ret < 0)
- return 0;
- return data;
-}
-#else
-#define snd_soc_16_8_read_i2c NULL
-#endif
-
-static int snd_soc_16_8_write(struct snd_soc_codec *codec, unsigned int reg,
- unsigned int value)
-{
- u8 data[3];
- u16 rval = cpu_to_be16(reg);
-
- memcpy(data, &rval, sizeof(rval));
- data[2] = value;
-
- return do_hw_write(codec, reg, value, data, 3);
-}
-
-#if defined(CONFIG_I2C) || (defined(CONFIG_I2C_MODULE) && defined(MODULE))
-static unsigned int snd_soc_16_16_read_i2c(struct snd_soc_codec *codec,
- unsigned int r)
-{
- u16 reg = cpu_to_be16(r);
- u16 data;
- int ret;
-
- ret = do_i2c_read(codec, ®, 2, &data, 2);
- if (ret < 0)
- return 0;
- return be16_to_cpu(data);
-}
-#else
-#define snd_soc_16_16_read_i2c NULL
-#endif
-
-static int snd_soc_16_16_write(struct snd_soc_codec *codec, unsigned int reg,
- unsigned int value)
-{
- u16 data[2];
-
- data[0] = cpu_to_be16(reg);
- data[1] = cpu_to_be16(value);
-
- return do_hw_write(codec, reg, value, data, sizeof(data));
-}
-
/* Primitive bulk write support for soc-cache. The data pointed to by
- * `data' needs to already be in the form the hardware expects
- * including any leading register specific data. Any data written
- * through this function will not go through the cache as it only
- * handles writing to volatile or out of bounds registers.
+ * `data' needs to already be in the form the hardware expects. Any
+ * data written through this function will not go through the cache as
+ * it only handles writing to volatile or out of bounds registers.
+ *
+ * This is currently only supported for devices using the regmap API
+ * wrappers.
*/
-static int snd_soc_hw_bulk_write_raw(struct snd_soc_codec *codec, unsigned int reg,
+static int snd_soc_hw_bulk_write_raw(struct snd_soc_codec *codec,
+ unsigned int reg,
const void *data, size_t len)
{
- int ret;
-
/* To ensure that we don't get out of sync with the cache, check
* whether the base register is volatile or if we've directly asked
* to bypass the cache. Out of bounds registers are considered
@@ -266,66 +85,9 @@ static int snd_soc_hw_bulk_write_raw(struct snd_soc_codec *codec, unsigned int r
&& reg < codec->driver->reg_cache_size)
return -EINVAL;
- switch (codec->control_type) {
-#if defined(CONFIG_I2C) || (defined(CONFIG_I2C_MODULE) && defined(MODULE))
- case SND_SOC_I2C:
- ret = i2c_master_send(to_i2c_client(codec->dev), data, len);
- break;
-#endif
-#if defined(CONFIG_SPI_MASTER)
- case SND_SOC_SPI:
- ret = spi_write(to_spi_device(codec->dev), data, len);
- break;
-#endif
- default:
- BUG();
- }
-
- if (ret == len)
- return 0;
- if (ret < 0)
- return ret;
- else
- return -EIO;
+ return regmap_raw_write(codec->control_data, reg, data, len);
}
-static struct {
- int addr_bits;
- int data_bits;
- int (*write)(struct snd_soc_codec *codec, unsigned int, unsigned int);
- unsigned int (*read)(struct snd_soc_codec *, unsigned int);
- unsigned int (*i2c_read)(struct snd_soc_codec *, unsigned int);
-} io_types[] = {
- {
- .addr_bits = 4, .data_bits = 12,
- .write = snd_soc_4_12_write,
- },
- {
- .addr_bits = 7, .data_bits = 9,
- .write = snd_soc_7_9_write,
- },
- {
- .addr_bits = 8, .data_bits = 8,
- .write = snd_soc_8_8_write,
- .i2c_read = snd_soc_8_8_read_i2c,
- },
- {
- .addr_bits = 8, .data_bits = 16,
- .write = snd_soc_8_16_write,
- .i2c_read = snd_soc_8_16_read_i2c,
- },
- {
- .addr_bits = 16, .data_bits = 8,
- .write = snd_soc_16_8_write,
- .i2c_read = snd_soc_16_8_read_i2c,
- },
- {
- .addr_bits = 16, .data_bits = 16,
- .write = snd_soc_16_16_write,
- .i2c_read = snd_soc_16_16_read_i2c,
- },
-};
-
/**
* snd_soc_codec_set_cache_io: Set up standard I/O functions.
*
@@ -349,46 +111,18 @@ int snd_soc_codec_set_cache_io(struct snd_soc_codec *codec,
int addr_bits, int data_bits,
enum snd_soc_control_type control)
{
- int i;
+ struct regmap_config config;
- for (i = 0; i < ARRAY_SIZE(io_types); i++)
- if (io_types[i].addr_bits == addr_bits &&
- io_types[i].data_bits == data_bits)
- break;
- if (i == ARRAY_SIZE(io_types)) {
- printk(KERN_ERR
- "No I/O functions for %d bit address %d bit data\n",
- addr_bits, data_bits);
- return -EINVAL;
- }
-
- codec->write = io_types[i].write;
+ codec->write = hw_write;
codec->read = hw_read;
codec->bulk_write_raw = snd_soc_hw_bulk_write_raw;
- switch (control) {
- case SND_SOC_I2C:
-#if defined(CONFIG_I2C) || (defined(CONFIG_I2C_MODULE) && defined(MODULE))
- codec->hw_write = (hw_write_t)i2c_master_send;
-#endif
- if (io_types[i].i2c_read)
- codec->hw_read = io_types[i].i2c_read;
-
- codec->control_data = container_of(codec->dev,
- struct i2c_client,
- dev);
- break;
+ config.reg_bits = addr_bits;
+ config.val_bits = data_bits;
- case SND_SOC_SPI:
-#ifdef CONFIG_SPI_MASTER
- codec->hw_write = do_spi_write;
-#endif
-
- codec->control_data = container_of(codec->dev,
- struct spi_device,
- dev);
- break;
- }
+ codec->control_data = regmap_init(codec->dev, &config);
+ if (IS_ERR(codec->control_data))
+ return PTR_ERR(codec->control_data);
return 0;
}
--
1.7.5.4
Factor out the register read/write code to use the register map API. We
still need some wm831x specific code and locking in place to check that
the user key is handled correctly but only on the write side, reads are
not affected by the key.
Signed-off-by: Mark Brown <[email protected]>
---
drivers/mfd/Kconfig | 2 +
drivers/mfd/wm831x-core.c | 97 ++++++++++++++------------------------
drivers/mfd/wm831x-i2c.c | 56 ----------------------
drivers/mfd/wm831x-spi.c | 48 -------------------
include/linux/mfd/wm831x/core.h | 7 +--
5 files changed, 40 insertions(+), 170 deletions(-)
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 0f09c05..487c3be 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -367,6 +367,7 @@ config MFD_WM831X_I2C
bool "Support Wolfson Microelectronics WM831x/2x PMICs with I2C"
select MFD_CORE
select MFD_WM831X
+ select REGMAP
depends on I2C=y && GENERIC_HARDIRQS
help
Support for the Wolfson Microelecronics WM831x and WM832x PMICs
@@ -378,6 +379,7 @@ config MFD_WM831X_SPI
bool "Support Wolfson Microelectronics WM831x/2x PMICs with SPI"
select MFD_CORE
select MFD_WM831X
+ select REGMAP
depends on SPI_MASTER && GENERIC_HARDIRQS
help
Support for the Wolfson Microelecronics WM831x and WM832x PMICs
diff --git a/drivers/mfd/wm831x-core.c b/drivers/mfd/wm831x-core.c
index baae9e4..1d72fba 100644
--- a/drivers/mfd/wm831x-core.c
+++ b/drivers/mfd/wm831x-core.c
@@ -18,6 +18,7 @@
#include <linux/delay.h>
#include <linux/mfd/core.h>
#include <linux/slab.h>
+#include <linux/err.h>
#include <linux/mfd/wm831x/core.h>
#include <linux/mfd/wm831x/pdata.h>
@@ -160,29 +161,6 @@ int wm831x_reg_unlock(struct wm831x *wm831x)
}
EXPORT_SYMBOL_GPL(wm831x_reg_unlock);
-static int wm831x_read(struct wm831x *wm831x, unsigned short reg,
- int bytes, void *dest)
-{
- int ret, i;
- u16 *buf = dest;
-
- BUG_ON(bytes % 2);
- BUG_ON(bytes <= 0);
-
- ret = wm831x->read_dev(wm831x, reg, bytes, dest);
- if (ret < 0)
- return ret;
-
- for (i = 0; i < bytes / 2; i++) {
- buf[i] = be16_to_cpu(buf[i]);
-
- dev_vdbg(wm831x->dev, "Read %04x from R%d(0x%x)\n",
- buf[i], reg + i, reg + i);
- }
-
- return 0;
-}
-
/**
* wm831x_reg_read: Read a single WM831x register.
*
@@ -191,14 +169,10 @@ static int wm831x_read(struct wm831x *wm831x, unsigned short reg,
*/
int wm831x_reg_read(struct wm831x *wm831x, unsigned short reg)
{
- unsigned short val;
+ unsigned int val;
int ret;
- mutex_lock(&wm831x->io_lock);
-
- ret = wm831x_read(wm831x, reg, 2, &val);
-
- mutex_unlock(&wm831x->io_lock);
+ ret = regmap_read(wm831x->regmap, reg, &val);
if (ret < 0)
return ret;
@@ -218,15 +192,7 @@ EXPORT_SYMBOL_GPL(wm831x_reg_read);
int wm831x_bulk_read(struct wm831x *wm831x, unsigned short reg,
int count, u16 *buf)
{
- int ret;
-
- mutex_lock(&wm831x->io_lock);
-
- ret = wm831x_read(wm831x, reg, count * 2, buf);
-
- mutex_unlock(&wm831x->io_lock);
-
- return ret;
+ return regmap_bulk_read(wm831x->regmap, reg, buf, count);
}
EXPORT_SYMBOL_GPL(wm831x_bulk_read);
@@ -234,7 +200,7 @@ static int wm831x_write(struct wm831x *wm831x, unsigned short reg,
int bytes, void *src)
{
u16 *buf = src;
- int i;
+ int i, ret;
BUG_ON(bytes % 2);
BUG_ON(bytes <= 0);
@@ -245,11 +211,10 @@ static int wm831x_write(struct wm831x *wm831x, unsigned short reg,
dev_vdbg(wm831x->dev, "Write %04x to R%d(0x%x)\n",
buf[i], reg + i, reg + i);
-
- buf[i] = cpu_to_be16(buf[i]);
+ ret = regmap_write(wm831x->regmap, reg + i, buf[i]);
}
- return wm831x->write_dev(wm831x, reg, bytes, src);
+ return 0;
}
/**
@@ -286,20 +251,14 @@ int wm831x_set_bits(struct wm831x *wm831x, unsigned short reg,
unsigned short mask, unsigned short val)
{
int ret;
- u16 r;
mutex_lock(&wm831x->io_lock);
- ret = wm831x_read(wm831x, reg, 2, &r);
- if (ret < 0)
- goto out;
-
- r &= ~mask;
- r |= val & mask;
-
- ret = wm831x_write(wm831x, reg, 2, &r);
+ if (!wm831x_reg_locked(wm831x, reg))
+ ret = regmap_update_bits(wm831x->regmap, reg, mask, val);
+ else
+ ret = -EPERM;
-out:
mutex_unlock(&wm831x->io_lock);
return ret;
@@ -1280,6 +1239,11 @@ static struct mfd_cell backlight_devs[] = {
},
};
+static struct regmap_config wm831x_regmap_config = {
+ .reg_bits = 16,
+ .val_bits = 16,
+};
+
/*
* Instantiate the generic non-control parts of the device.
*/
@@ -1294,10 +1258,18 @@ int wm831x_device_init(struct wm831x *wm831x, unsigned long id, int irq)
mutex_init(&wm831x->key_lock);
dev_set_drvdata(wm831x->dev, wm831x);
+ wm831x->regmap = regmap_init(wm831x->dev, &wm831x_regmap_config);
+ if (IS_ERR(wm831x->regmap)) {
+ ret = PTR_ERR(wm831x->regmap);
+ dev_err(wm831x->dev, "Failed to allocate register map: %d\n",
+ ret);
+ goto err;
+ }
+
ret = wm831x_reg_read(wm831x, WM831X_PARENT_ID);
if (ret < 0) {
dev_err(wm831x->dev, "Failed to read parent ID: %d\n", ret);
- goto err;
+ goto err_regmap;
}
switch (ret) {
case 0x6204:
@@ -1306,20 +1278,20 @@ int wm831x_device_init(struct wm831x *wm831x, unsigned long id, int irq)
default:
dev_err(wm831x->dev, "Device is not a WM831x: ID %x\n", ret);
ret = -EINVAL;
- goto err;
+ goto err_regmap;
}
ret = wm831x_reg_read(wm831x, WM831X_REVISION);
if (ret < 0) {
dev_err(wm831x->dev, "Failed to read revision: %d\n", ret);
- goto err;
+ goto err_regmap;
}
rev = (ret & WM831X_PARENT_REV_MASK) >> WM831X_PARENT_REV_SHIFT;
ret = wm831x_reg_read(wm831x, WM831X_RESET_ID);
if (ret < 0) {
dev_err(wm831x->dev, "Failed to read device ID: %d\n", ret);
- goto err;
+ goto err_regmap;
}
/* Some engineering samples do not have the ID set, rely on
@@ -1394,7 +1366,7 @@ int wm831x_device_init(struct wm831x *wm831x, unsigned long id, int irq)
default:
dev_err(wm831x->dev, "Unknown WM831x device %04x\n", ret);
ret = -EINVAL;
- goto err;
+ goto err_regmap;
}
/* This will need revisiting in future but is OK for all
@@ -1408,7 +1380,7 @@ int wm831x_device_init(struct wm831x *wm831x, unsigned long id, int irq)
ret = wm831x_reg_read(wm831x, WM831X_SECURITY_KEY);
if (ret < 0) {
dev_err(wm831x->dev, "Failed to read security key: %d\n", ret);
- goto err;
+ goto err_regmap;
}
if (ret != 0) {
dev_warn(wm831x->dev, "Security key had non-zero value %x\n",
@@ -1421,7 +1393,7 @@ int wm831x_device_init(struct wm831x *wm831x, unsigned long id, int irq)
ret = pdata->pre_init(wm831x);
if (ret != 0) {
dev_err(wm831x->dev, "pre_init() failed: %d\n", ret);
- goto err;
+ goto err_regmap;
}
}
@@ -1444,7 +1416,7 @@ int wm831x_device_init(struct wm831x *wm831x, unsigned long id, int irq)
ret = wm831x_irq_init(wm831x, irq);
if (ret != 0)
- goto err;
+ goto err_regmap;
wm831x_auxadc_init(wm831x);
@@ -1540,8 +1512,10 @@ int wm831x_device_init(struct wm831x *wm831x, unsigned long id, int irq)
err_irq:
wm831x_irq_exit(wm831x);
-err:
+err_regmap:
mfd_remove_devices(wm831x->dev);
+ regmap_free(wm831x->regmap);
+err:
kfree(wm831x);
return ret;
}
@@ -1553,6 +1527,7 @@ void wm831x_device_exit(struct wm831x *wm831x)
if (wm831x->irq_base)
free_irq(wm831x->irq_base + WM831X_IRQ_AUXADC_DATA, wm831x);
wm831x_irq_exit(wm831x);
+ regmap_free(wm831x->regmap);
kfree(wm831x);
}
diff --git a/drivers/mfd/wm831x-i2c.c b/drivers/mfd/wm831x-i2c.c
index a06cbc7..4e16462 100644
--- a/drivers/mfd/wm831x-i2c.c
+++ b/drivers/mfd/wm831x-i2c.c
@@ -22,59 +22,6 @@
#include <linux/mfd/wm831x/core.h>
#include <linux/mfd/wm831x/pdata.h>
-static int wm831x_i2c_read_device(struct wm831x *wm831x, unsigned short reg,
- int bytes, void *dest)
-{
- struct i2c_client *i2c = wm831x->control_data;
- int ret;
- u16 r = cpu_to_be16(reg);
-
- ret = i2c_master_send(i2c, (unsigned char *)&r, 2);
- if (ret < 0)
- return ret;
- if (ret != 2)
- return -EIO;
-
- ret = i2c_master_recv(i2c, dest, bytes);
- if (ret < 0)
- return ret;
- if (ret != bytes)
- return -EIO;
- return 0;
-}
-
-/* Currently we allocate the write buffer on the stack; this is OK for
- * small writes - if we need to do large writes this will need to be
- * revised.
- */
-static int wm831x_i2c_write_device(struct wm831x *wm831x, unsigned short reg,
- int bytes, void *src)
-{
- struct i2c_client *i2c = wm831x->control_data;
- struct i2c_msg xfer[2];
- int ret;
-
- reg = cpu_to_be16(reg);
-
- xfer[0].addr = i2c->addr;
- xfer[0].flags = 0;
- xfer[0].len = 2;
- xfer[0].buf = (char *)®
-
- xfer[1].addr = i2c->addr;
- xfer[1].flags = I2C_M_NOSTART;
- xfer[1].len = bytes;
- xfer[1].buf = (char *)src;
-
- ret = i2c_transfer(i2c->adapter, xfer, 2);
- if (ret < 0)
- return ret;
- if (ret != 2)
- return -EIO;
-
- return 0;
-}
-
static int wm831x_i2c_probe(struct i2c_client *i2c,
const struct i2c_device_id *id)
{
@@ -86,9 +33,6 @@ static int wm831x_i2c_probe(struct i2c_client *i2c,
i2c_set_clientdata(i2c, wm831x);
wm831x->dev = &i2c->dev;
- wm831x->control_data = i2c;
- wm831x->read_dev = wm831x_i2c_read_device;
- wm831x->write_dev = wm831x_i2c_write_device;
return wm831x_device_init(wm831x, id->driver_data, i2c->irq);
}
diff --git a/drivers/mfd/wm831x-spi.c b/drivers/mfd/wm831x-spi.c
index eed8e4f..0b0730d 100644
--- a/drivers/mfd/wm831x-spi.c
+++ b/drivers/mfd/wm831x-spi.c
@@ -19,51 +19,6 @@
#include <linux/mfd/wm831x/core.h>
-static int wm831x_spi_read_device(struct wm831x *wm831x, unsigned short reg,
- int bytes, void *dest)
-{
- u16 tx_val;
- u16 *d = dest;
- int r, ret;
-
- /* Go register at a time */
- for (r = reg; r < reg + (bytes / 2); r++) {
- tx_val = r | 0x8000;
-
- ret = spi_write_then_read(wm831x->control_data,
- (u8 *)&tx_val, 2, (u8 *)d, 2);
- if (ret != 0)
- return ret;
-
- *d = be16_to_cpu(*d);
-
- d++;
- }
-
- return 0;
-}
-
-static int wm831x_spi_write_device(struct wm831x *wm831x, unsigned short reg,
- int bytes, void *src)
-{
- struct spi_device *spi = wm831x->control_data;
- u16 *s = src;
- u16 data[2];
- int ret, r;
-
- /* Go register at a time */
- for (r = reg; r < reg + (bytes / 2); r++) {
- data[0] = r;
- data[1] = *s++;
-
- ret = spi_write(spi, (char *)&data, sizeof(data));
- if (ret != 0)
- return ret;
- }
-
- return 0;
-}
-
static int __devinit wm831x_spi_probe(struct spi_device *spi)
{
struct wm831x *wm831x;
@@ -98,9 +53,6 @@ static int __devinit wm831x_spi_probe(struct spi_device *spi)
dev_set_drvdata(&spi->dev, wm831x);
wm831x->dev = &spi->dev;
- wm831x->control_data = spi;
- wm831x->read_dev = wm831x_spi_read_device;
- wm831x->write_dev = wm831x_spi_write_device;
return wm831x_device_init(wm831x, type, spi->irq);
}
diff --git a/include/linux/mfd/wm831x/core.h b/include/linux/mfd/wm831x/core.h
index 8dda8de..bd0615b 100644
--- a/include/linux/mfd/wm831x/core.h
+++ b/include/linux/mfd/wm831x/core.h
@@ -18,6 +18,7 @@
#include <linux/completion.h>
#include <linux/interrupt.h>
#include <linux/list.h>
+#include <linux/regmap.h>
/*
* Register values.
@@ -361,12 +362,8 @@ struct wm831x {
struct mutex io_lock;
struct device *dev;
- int (*read_dev)(struct wm831x *wm831x, unsigned short reg,
- int bytes, void *dest);
- int (*write_dev)(struct wm831x *wm831x, unsigned short reg,
- int bytes, void *src);
- void *control_data;
+ struct regmap *regmap;
int irq; /* Our chip IRQ */
struct mutex irq_lock;
--
1.7.5.4
Signed-off-by: Mark Brown <[email protected]>
---
drivers/mfd/Kconfig | 1 +
drivers/mfd/wm8994-core.c | 175 +++++++--------------------------------
include/linux/mfd/wm8994/core.h | 9 +--
3 files changed, 34 insertions(+), 151 deletions(-)
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 487c3be..8d788ee 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -453,6 +453,7 @@ config MFD_WM8350_I2C
config MFD_WM8994
bool "Support Wolfson Microelectronics WM8994"
select MFD_CORE
+ select REGMAP
depends on I2C=y && GENERIC_HARDIRQS
help
The WM8994 is a highly integrated hi-fi CODEC designed for
diff --git a/drivers/mfd/wm8994-core.c b/drivers/mfd/wm8994-core.c
index 7f4a78e..66bfb57 100644
--- a/drivers/mfd/wm8994-core.c
+++ b/drivers/mfd/wm8994-core.c
@@ -16,9 +16,11 @@
#include <linux/module.h>
#include <linux/slab.h>
#include <linux/i2c.h>
+#include <linux/err.h>
#include <linux/delay.h>
#include <linux/mfd/core.h>
#include <linux/pm_runtime.h>
+#include <linux/regmap.h>
#include <linux/regulator/consumer.h>
#include <linux/regulator/machine.h>
@@ -29,22 +31,7 @@
static int wm8994_read(struct wm8994 *wm8994, unsigned short reg,
int bytes, void *dest)
{
- int ret, i;
- u16 *buf = dest;
-
- BUG_ON(bytes % 2);
- BUG_ON(bytes <= 0);
-
- ret = wm8994->read_dev(wm8994, reg, bytes, dest);
- if (ret < 0)
- return ret;
-
- for (i = 0; i < bytes / 2; i++) {
- dev_vdbg(wm8994->dev, "Read %04x from R%d(0x%x)\n",
- be16_to_cpu(buf[i]), reg + i, reg + i);
- }
-
- return 0;
+ return regmap_raw_read(wm8994->regmap, reg, dest, bytes);
}
/**
@@ -55,19 +42,15 @@ static int wm8994_read(struct wm8994 *wm8994, unsigned short reg,
*/
int wm8994_reg_read(struct wm8994 *wm8994, unsigned short reg)
{
- unsigned short val;
+ unsigned int val;
int ret;
- mutex_lock(&wm8994->io_lock);
-
- ret = wm8994_read(wm8994, reg, 2, &val);
-
- mutex_unlock(&wm8994->io_lock);
+ ret = regmap_read(wm8994->regmap, reg, &val);
if (ret < 0)
return ret;
else
- return be16_to_cpu(val);
+ return val;
}
EXPORT_SYMBOL_GPL(wm8994_reg_read);
@@ -82,33 +65,13 @@ EXPORT_SYMBOL_GPL(wm8994_reg_read);
int wm8994_bulk_read(struct wm8994 *wm8994, unsigned short reg,
int count, u16 *buf)
{
- int ret;
-
- mutex_lock(&wm8994->io_lock);
-
- ret = wm8994_read(wm8994, reg, count * 2, buf);
-
- mutex_unlock(&wm8994->io_lock);
-
- return ret;
+ return regmap_bulk_read(wm8994->regmap, reg, buf, count);
}
-EXPORT_SYMBOL_GPL(wm8994_bulk_read);
static int wm8994_write(struct wm8994 *wm8994, unsigned short reg,
int bytes, const void *src)
{
- const u16 *buf = src;
- int i;
-
- BUG_ON(bytes % 2);
- BUG_ON(bytes <= 0);
-
- for (i = 0; i < bytes / 2; i++) {
- dev_vdbg(wm8994->dev, "Write %04x to R%d(0x%x)\n",
- be16_to_cpu(buf[i]), reg + i, reg + i);
- }
-
- return wm8994->write_dev(wm8994, reg, bytes, src);
+ return regmap_raw_write(wm8994->regmap, reg, src, bytes);
}
/**
@@ -121,17 +84,7 @@ static int wm8994_write(struct wm8994 *wm8994, unsigned short reg,
int wm8994_reg_write(struct wm8994 *wm8994, unsigned short reg,
unsigned short val)
{
- int ret;
-
- val = cpu_to_be16(val);
-
- mutex_lock(&wm8994->io_lock);
-
- ret = wm8994_write(wm8994, reg, 2, &val);
-
- mutex_unlock(&wm8994->io_lock);
-
- return ret;
+ return regmap_write(wm8994->regmap, reg, val);
}
EXPORT_SYMBOL_GPL(wm8994_reg_write);
@@ -146,15 +99,7 @@ EXPORT_SYMBOL_GPL(wm8994_reg_write);
int wm8994_bulk_write(struct wm8994 *wm8994, unsigned short reg,
int count, const u16 *buf)
{
- int ret;
-
- mutex_lock(&wm8994->io_lock);
-
- ret = wm8994_write(wm8994, reg, count * 2, buf);
-
- mutex_unlock(&wm8994->io_lock);
-
- return ret;
+ return regmap_raw_write(wm8994->regmap, reg, buf, count * sizeof(u16));
}
EXPORT_SYMBOL_GPL(wm8994_bulk_write);
@@ -169,28 +114,7 @@ EXPORT_SYMBOL_GPL(wm8994_bulk_write);
int wm8994_set_bits(struct wm8994 *wm8994, unsigned short reg,
unsigned short mask, unsigned short val)
{
- int ret;
- u16 r;
-
- mutex_lock(&wm8994->io_lock);
-
- ret = wm8994_read(wm8994, reg, 2, &r);
- if (ret < 0)
- goto out;
-
- r = be16_to_cpu(r);
-
- r &= ~mask;
- r |= val;
-
- r = cpu_to_be16(r);
-
- ret = wm8994_write(wm8994, reg, 2, &r);
-
-out:
- mutex_unlock(&wm8994->io_lock);
-
- return ret;
+ return regmap_update_bits(wm8994->regmap, reg, mask, val);
}
EXPORT_SYMBOL_GPL(wm8994_set_bits);
@@ -388,6 +312,11 @@ static int wm8994_ldo_in_use(struct wm8994_pdata *pdata, int ldo)
}
#endif
+static struct regmap_config wm8994_regmap_config = {
+ .reg_bits = 16,
+ .val_bits = 16,
+};
+
/*
* Instantiate the generic non-control parts of the device.
*/
@@ -397,7 +326,14 @@ static int wm8994_device_init(struct wm8994 *wm8994, int irq)
const char *devname;
int ret, i;
- mutex_init(&wm8994->io_lock);
+ wm8994->regmap = regmap_init(wm8994->dev, &wm8994_regmap_config);
+ if (IS_ERR(wm8994->regmap)) {
+ ret = PTR_ERR(wm8994->regmap);
+ dev_err(wm8994->dev, "Failed to allocate register map: %d\n",
+ ret);
+ goto err;
+ }
+
dev_set_drvdata(wm8994->dev, wm8994);
/* Add the on-chip regulators first for bootstrapping */
@@ -407,7 +343,7 @@ static int wm8994_device_init(struct wm8994 *wm8994, int irq)
NULL, 0);
if (ret != 0) {
dev_err(wm8994->dev, "Failed to add children: %d\n", ret);
- goto err;
+ goto err_regmap;
}
switch (wm8994->type) {
@@ -422,7 +358,7 @@ static int wm8994_device_init(struct wm8994 *wm8994, int irq)
break;
default:
BUG();
- goto err;
+ goto err_regmap;
}
wm8994->supplies = kzalloc(sizeof(struct regulator_bulk_data) *
@@ -430,7 +366,7 @@ static int wm8994_device_init(struct wm8994 *wm8994, int irq)
GFP_KERNEL);
if (!wm8994->supplies) {
ret = -ENOMEM;
- goto err;
+ goto err_regmap;
}
switch (wm8994->type) {
@@ -448,7 +384,7 @@ static int wm8994_device_init(struct wm8994 *wm8994, int irq)
break;
default:
BUG();
- goto err;
+ goto err_regmap;
}
ret = regulator_bulk_get(wm8994->dev, wm8994->num_supplies,
@@ -578,6 +514,8 @@ err_get:
regulator_bulk_free(wm8994->num_supplies, wm8994->supplies);
err_supplies:
kfree(wm8994->supplies);
+err_regmap:
+ regmap_free(wm8994->regmap);
err:
mfd_remove_devices(wm8994->dev);
kfree(wm8994);
@@ -593,58 +531,10 @@ static void wm8994_device_exit(struct wm8994 *wm8994)
wm8994->supplies);
regulator_bulk_free(wm8994->num_supplies, wm8994->supplies);
kfree(wm8994->supplies);
+ regmap_free(wm8994->regmap);
kfree(wm8994);
}
-static int wm8994_i2c_read_device(struct wm8994 *wm8994, unsigned short reg,
- int bytes, void *dest)
-{
- struct i2c_client *i2c = wm8994->control_data;
- int ret;
- u16 r = cpu_to_be16(reg);
-
- ret = i2c_master_send(i2c, (unsigned char *)&r, 2);
- if (ret < 0)
- return ret;
- if (ret != 2)
- return -EIO;
-
- ret = i2c_master_recv(i2c, dest, bytes);
- if (ret < 0)
- return ret;
- if (ret != bytes)
- return -EIO;
- return 0;
-}
-
-static int wm8994_i2c_write_device(struct wm8994 *wm8994, unsigned short reg,
- int bytes, const void *src)
-{
- struct i2c_client *i2c = wm8994->control_data;
- struct i2c_msg xfer[2];
- int ret;
-
- reg = cpu_to_be16(reg);
-
- xfer[0].addr = i2c->addr;
- xfer[0].flags = 0;
- xfer[0].len = 2;
- xfer[0].buf = (char *)®
-
- xfer[1].addr = i2c->addr;
- xfer[1].flags = I2C_M_NOSTART;
- xfer[1].len = bytes;
- xfer[1].buf = (char *)src;
-
- ret = i2c_transfer(i2c->adapter, xfer, 2);
- if (ret < 0)
- return ret;
- if (ret != 2)
- return -EIO;
-
- return 0;
-}
-
static int wm8994_i2c_probe(struct i2c_client *i2c,
const struct i2c_device_id *id)
{
@@ -656,9 +546,6 @@ static int wm8994_i2c_probe(struct i2c_client *i2c,
i2c_set_clientdata(i2c, wm8994);
wm8994->dev = &i2c->dev;
- wm8994->control_data = i2c;
- wm8994->read_dev = wm8994_i2c_read_device;
- wm8994->write_dev = wm8994_i2c_write_device;
wm8994->irq = i2c->irq;
wm8994->type = id->driver_data;
diff --git a/include/linux/mfd/wm8994/core.h b/include/linux/mfd/wm8994/core.h
index bfb221b..6268091 100644
--- a/include/linux/mfd/wm8994/core.h
+++ b/include/linux/mfd/wm8994/core.h
@@ -25,6 +25,7 @@ enum wm8994_type {
struct regulator_dev;
struct regulator_bulk_data;
+struct regmap;
#define WM8994_NUM_GPIO_REGS 11
#define WM8994_NUM_LDO_REGS 2
@@ -51,18 +52,12 @@ struct regulator_bulk_data;
#define WM8994_IRQ_GPIO(x) (x + WM8994_IRQ_TEMP_WARN)
struct wm8994 {
- struct mutex io_lock;
struct mutex irq_lock;
enum wm8994_type type;
struct device *dev;
- int (*read_dev)(struct wm8994 *wm8994, unsigned short reg,
- int bytes, void *dest);
- int (*write_dev)(struct wm8994 *wm8994, unsigned short reg,
- int bytes, const void *src);
-
- void *control_data;
+ struct regmap *regmap;
int gpio_base;
int irq_base;
--
1.7.5.4
Signed-off-by: Mark Brown <[email protected]>
---
drivers/mfd/Kconfig | 1 +
drivers/mfd/pcf50633-core.c | 114 +++++++++---------------------------
include/linux/mfd/pcf50633/core.h | 3 +-
3 files changed, 32 insertions(+), 86 deletions(-)
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 8d788ee..e0951c6 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -466,6 +466,7 @@ config MFD_WM8994
config MFD_PCF50633
tristate "Support for NXP PCF50633"
depends on I2C
+ select REGMAP
help
Say yes here if you have NXP PCF50633 chip on your board.
This core driver provides register access and IRQ handling
diff --git a/drivers/mfd/pcf50633-core.c b/drivers/mfd/pcf50633-core.c
index 57868416..06aa79c 100644
--- a/drivers/mfd/pcf50633-core.c
+++ b/drivers/mfd/pcf50633-core.c
@@ -23,45 +23,16 @@
#include <linux/i2c.h>
#include <linux/pm.h>
#include <linux/slab.h>
+#include <linux/regmap.h>
+#include <linux/err.h>
#include <linux/mfd/pcf50633/core.h>
-static int __pcf50633_read(struct pcf50633 *pcf, u8 reg, int num, u8 *data)
-{
- int ret;
-
- ret = i2c_smbus_read_i2c_block_data(pcf->i2c_client, reg,
- num, data);
- if (ret < 0)
- dev_err(pcf->dev, "Error reading %d regs at %d\n", num, reg);
-
- return ret;
-}
-
-static int __pcf50633_write(struct pcf50633 *pcf, u8 reg, int num, u8 *data)
-{
- int ret;
-
- ret = i2c_smbus_write_i2c_block_data(pcf->i2c_client, reg,
- num, data);
- if (ret < 0)
- dev_err(pcf->dev, "Error writing %d regs at %d\n", num, reg);
-
- return ret;
-
-}
-
/* Read a block of up to 32 regs */
int pcf50633_read_block(struct pcf50633 *pcf, u8 reg,
int nr_regs, u8 *data)
{
- int ret;
-
- mutex_lock(&pcf->lock);
- ret = __pcf50633_read(pcf, reg, nr_regs, data);
- mutex_unlock(&pcf->lock);
-
- return ret;
+ return regmap_raw_read(pcf->regmap, reg, data, nr_regs);
}
EXPORT_SYMBOL_GPL(pcf50633_read_block);
@@ -69,23 +40,18 @@ EXPORT_SYMBOL_GPL(pcf50633_read_block);
int pcf50633_write_block(struct pcf50633 *pcf , u8 reg,
int nr_regs, u8 *data)
{
- int ret;
-
- mutex_lock(&pcf->lock);
- ret = __pcf50633_write(pcf, reg, nr_regs, data);
- mutex_unlock(&pcf->lock);
-
- return ret;
+ return regmap_raw_write(pcf->regmap, reg, data, nr_regs);
}
EXPORT_SYMBOL_GPL(pcf50633_write_block);
u8 pcf50633_reg_read(struct pcf50633 *pcf, u8 reg)
{
- u8 val;
+ unsigned int val;
+ int ret;
- mutex_lock(&pcf->lock);
- __pcf50633_read(pcf, reg, 1, &val);
- mutex_unlock(&pcf->lock);
+ ret = regmap_read(pcf->regmap, reg, &val);
+ if (ret < 0)
+ return -1;
return val;
}
@@ -93,56 +59,19 @@ EXPORT_SYMBOL_GPL(pcf50633_reg_read);
int pcf50633_reg_write(struct pcf50633 *pcf, u8 reg, u8 val)
{
- int ret;
-
- mutex_lock(&pcf->lock);
- ret = __pcf50633_write(pcf, reg, 1, &val);
- mutex_unlock(&pcf->lock);
-
- return ret;
+ return regmap_write(pcf->regmap, reg, val);
}
EXPORT_SYMBOL_GPL(pcf50633_reg_write);
int pcf50633_reg_set_bit_mask(struct pcf50633 *pcf, u8 reg, u8 mask, u8 val)
{
- int ret;
- u8 tmp;
-
- val &= mask;
-
- mutex_lock(&pcf->lock);
- ret = __pcf50633_read(pcf, reg, 1, &tmp);
- if (ret < 0)
- goto out;
-
- tmp &= ~mask;
- tmp |= val;
- ret = __pcf50633_write(pcf, reg, 1, &tmp);
-
-out:
- mutex_unlock(&pcf->lock);
-
- return ret;
+ return regmap_update_bits(pcf->regmap, reg, mask, val);
}
EXPORT_SYMBOL_GPL(pcf50633_reg_set_bit_mask);
int pcf50633_reg_clear_bits(struct pcf50633 *pcf, u8 reg, u8 val)
{
- int ret;
- u8 tmp;
-
- mutex_lock(&pcf->lock);
- ret = __pcf50633_read(pcf, reg, 1, &tmp);
- if (ret < 0)
- goto out;
-
- tmp &= ~val;
- ret = __pcf50633_write(pcf, reg, 1, &tmp);
-
-out:
- mutex_unlock(&pcf->lock);
-
- return ret;
+ return regmap_update_bits(pcf->regmap, reg, val, 0);
}
EXPORT_SYMBOL_GPL(pcf50633_reg_clear_bits);
@@ -251,6 +180,11 @@ static int pcf50633_resume(struct device *dev)
static SIMPLE_DEV_PM_OPS(pcf50633_pm, pcf50633_suspend, pcf50633_resume);
+static struct regmap_config pcf50633_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+};
+
static int __devinit pcf50633_probe(struct i2c_client *client,
const struct i2c_device_id *ids)
{
@@ -272,16 +206,23 @@ static int __devinit pcf50633_probe(struct i2c_client *client,
mutex_init(&pcf->lock);
+ pcf->regmap = regmap_init(pcf->dev, &pcf50633_regmap_config);
+ if (IS_ERR(pcf->regmap)) {
+ ret = PTR_ERR(pcf->regmap);
+ dev_err(pcf->dev, "Failed to allocate register map: %d\n",
+ ret);
+ goto err_free;
+ }
+
i2c_set_clientdata(client, pcf);
pcf->dev = &client->dev;
- pcf->i2c_client = client;
version = pcf50633_reg_read(pcf, 0);
variant = pcf50633_reg_read(pcf, 1);
if (version < 0 || variant < 0) {
dev_err(pcf->dev, "Unable to probe pcf50633\n");
ret = -ENODEV;
- goto err_free;
+ goto err_regmap;
}
dev_info(pcf->dev, "Probed device version %d variant %d\n",
@@ -328,6 +269,8 @@ static int __devinit pcf50633_probe(struct i2c_client *client,
return 0;
+err_regmap:
+ regmap_free(pcf->regmap);
err_free:
kfree(pcf);
@@ -351,6 +294,7 @@ static int __devexit pcf50633_remove(struct i2c_client *client)
for (i = 0; i < PCF50633_NUM_REGULATORS; i++)
platform_device_unregister(pcf->regulator_pdev[i]);
+ regmap_free(pcf->regmap);
kfree(pcf);
return 0;
diff --git a/include/linux/mfd/pcf50633/core.h b/include/linux/mfd/pcf50633/core.h
index 50d4a04..a808407 100644
--- a/include/linux/mfd/pcf50633/core.h
+++ b/include/linux/mfd/pcf50633/core.h
@@ -21,6 +21,7 @@
#include <linux/mfd/pcf50633/backlight.h>
struct pcf50633;
+struct regmap;
#define PCF50633_NUM_REGULATORS 11
@@ -134,7 +135,7 @@ enum {
struct pcf50633 {
struct device *dev;
- struct i2c_client *i2c_client;
+ struct regmap *regmap;
struct pcf50633_platform_data *pdata;
int irq;
--
1.7.5.4
Signed-off-by: Mark Brown <[email protected]>
---
drivers/regulator/Kconfig | 1 +
drivers/regulator/tps65023-regulator.c | 98 +++++++++-----------------------
2 files changed, 28 insertions(+), 71 deletions(-)
diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index d7ed20f..c8dc5f0 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -235,6 +235,7 @@ config REGULATOR_TPS6105X
config REGULATOR_TPS65023
tristate "TI TPS65023 Power regulators"
depends on I2C
+ select REGMAP
help
This driver supports TPS65023 voltage regulator chips. TPS65023 provides
three step-down converters and two general-purpose LDO voltage regulators.
diff --git a/drivers/regulator/tps65023-regulator.c b/drivers/regulator/tps65023-regulator.c
index fbddc15..5e2c274 100644
--- a/drivers/regulator/tps65023-regulator.c
+++ b/drivers/regulator/tps65023-regulator.c
@@ -25,6 +25,7 @@
#include <linux/i2c.h>
#include <linux/delay.h>
#include <linux/slab.h>
+#include <linux/regmap.h>
/* Register definitions */
#define TPS65023_REG_VERSION 0
@@ -125,93 +126,35 @@ struct tps_pmic {
struct i2c_client *client;
struct regulator_dev *rdev[TPS65023_NUM_REGULATOR];
const struct tps_info *info[TPS65023_NUM_REGULATOR];
- struct mutex io_lock;
+ struct regmap *regmap;
};
-static inline int tps_65023_read(struct tps_pmic *tps, u8 reg)
-{
- return i2c_smbus_read_byte_data(tps->client, reg);
-}
-
-static inline int tps_65023_write(struct tps_pmic *tps, u8 reg, u8 val)
-{
- return i2c_smbus_write_byte_data(tps->client, reg, val);
-}
-
static int tps_65023_set_bits(struct tps_pmic *tps, u8 reg, u8 mask)
{
- int err, data;
-
- mutex_lock(&tps->io_lock);
-
- data = tps_65023_read(tps, reg);
- if (data < 0) {
- dev_err(&tps->client->dev, "Read from reg 0x%x failed\n", reg);
- err = data;
- goto out;
- }
-
- data |= mask;
- err = tps_65023_write(tps, reg, data);
- if (err)
- dev_err(&tps->client->dev, "Write for reg 0x%x failed\n", reg);
-
-out:
- mutex_unlock(&tps->io_lock);
- return err;
+ return regmap_update_bits(tps->regmap, reg, mask, mask);
}
static int tps_65023_clear_bits(struct tps_pmic *tps, u8 reg, u8 mask)
{
- int err, data;
-
- mutex_lock(&tps->io_lock);
-
- data = tps_65023_read(tps, reg);
- if (data < 0) {
- dev_err(&tps->client->dev, "Read from reg 0x%x failed\n", reg);
- err = data;
- goto out;
- }
-
- data &= ~mask;
-
- err = tps_65023_write(tps, reg, data);
- if (err)
- dev_err(&tps->client->dev, "Write for reg 0x%x failed\n", reg);
-
-out:
- mutex_unlock(&tps->io_lock);
- return err;
-
+ return regmap_update_bits(tps->regmap, reg, mask, 0);
}
static int tps_65023_reg_read(struct tps_pmic *tps, u8 reg)
{
- int data;
+ unsigned int val;
+ int ret;
- mutex_lock(&tps->io_lock);
+ ret = regmap_read(tps->regmap, reg, &val);
- data = tps_65023_read(tps, reg);
- if (data < 0)
- dev_err(&tps->client->dev, "Read from reg 0x%x failed\n", reg);
-
- mutex_unlock(&tps->io_lock);
- return data;
+ if (ret != 0)
+ return ret;
+ else
+ return val;
}
static int tps_65023_reg_write(struct tps_pmic *tps, u8 reg, u8 val)
{
- int err;
-
- mutex_lock(&tps->io_lock);
-
- err = tps_65023_write(tps, reg, val);
- if (err < 0)
- dev_err(&tps->client->dev, "Write for reg 0x%x failed\n", reg);
-
- mutex_unlock(&tps->io_lock);
- return err;
+ return regmap_write(tps->regmap, reg, val);
}
static int tps65023_dcdc_is_enabled(struct regulator_dev *dev)
@@ -463,6 +406,10 @@ static struct regulator_ops tps65023_ldo_ops = {
.list_voltage = tps65023_ldo_list_voltage,
};
+static struct regmap_config tps65023_regmap_config = {
+ .reg_bits = 8, .val_bits = 8,
+};
+
static int __devinit tps_65023_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
@@ -488,7 +435,13 @@ static int __devinit tps_65023_probe(struct i2c_client *client,
if (!tps)
return -ENOMEM;
- mutex_init(&tps->io_lock);
+ tps->regmap = regmap_init(&client->dev, &tps65023_regmap_config);
+ if (IS_ERR(tps->regmap)) {
+ error = PTR_ERR(tps->regmap);
+ dev_err(&client->dev, "Failed to allocate register map: %d\n",
+ error);
+ goto fail_alloc;
+ }
/* common for all regulators */
tps->client = client;
@@ -523,10 +476,12 @@ static int __devinit tps_65023_probe(struct i2c_client *client,
return 0;
- fail:
+fail:
while (--i >= 0)
regulator_unregister(tps->rdev[i]);
+ regmap_free(tps->regmap);
+fail_alloc:
kfree(tps);
return error;
}
@@ -545,6 +500,7 @@ static int __devexit tps_65023_remove(struct i2c_client *client)
for (i = 0; i < TPS65023_NUM_REGULATOR; i++)
regulator_unregister(tps->rdev[i]);
+ regmap_free(tps->regmap);
kfree(tps);
return 0;
--
1.7.5.4
Very nice!
> [...]
> diff --git a/drivers/regmap/regmap.c b/drivers/regmap/regmap.c
> new file mode 100644
> index 0000000..a8610c7
> --- /dev/null
> +++ b/drivers/regmap/regmap.c
> @@ -0,0 +1,478 @@
> +/*
> + * Register map access API
> + *
> + * Copyright 2011 Wolfson Microelectronics plc
> + *
> + * Author: Mark Brown <[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 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/slab.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/err.h>
> +
> +#include <linux/regmap.h>
> +
> +struct regmap;
> +
> +static DEFINE_MUTEX(regmap_bus_lock);
> +static LIST_HEAD(regmap_bus_list);
> +
> +struct regmap_format {
> + size_t buf_size;
> + size_t reg_bytes;
> + size_t val_bytes;
> + void (*format_write)(struct regmap *map,
> + unsigned int reg, unsigned int val);
> + void (*format_reg)(void *buf, unsigned int reg);
> + void (*format_val)(void *buf, unsigned int val);
> + unsigned int (*parse_val)(void *buf);
const void *buf. It probably also makes sense to pass the regmap to all
callbacks, so we can write generic format/parser functions which use
information for example from the regmap_format.
> +};
> +
> +struct regmap {
> + struct mutex lock;
> +
> + struct device *dev; /* Device we do I/O on */
> + void *work_buf; /* Scratch buffer used to format I/O */
> + struct regmap_format format; /* Buffer format */
> + struct regmap_bus *bus;
const struct regmap_bus
> +};
> +
> +static void regmap_format_4_12_write(struct regmap *map,
> + unsigned int reg, unsigned int val)
> +{
> + u16 *out = map->work_buf;
> + *out = cpu_to_be16((reg << 12) | val);
> +}
> +
> +static void regmap_format_7_9_write(struct regmap *map,
> + unsigned int reg, unsigned int val)
> +{
> + u16 *out = map->work_buf;
> + *out = cpu_to_be16((reg << 9) | val);
> +}
It would make sense to keep val_bits around and implement these two generically:
*out = cpu_to_be16((reg << map->format.val_bits) | val);
> [...]
> +/**
> + * remap_init: Initialise register map
> + *
> + * dev: Device that will be interacted with
> + * config: Configuration for register map
> + *
> + * The return value will be an ERR_PTR() on error or a valid pointer
> + * to a struct regmap.
> + */
> +struct regmap *regmap_init(struct device *dev, struct regmap_config *config)
regmap_alloc would in my opinion be a better name.
> [...]
> +static int _regmap_raw_read(struct regmap *map, unsigned int reg, void *val,
> + unsigned int val_len)
> +{
> + u8 *u8 = map->work_buf;
> + int ret;
> +
> + map->format.format_reg(map->work_buf, reg);
> +
> + /*
> + * Some buses flag reads by setting the high bit in the
> + * register addresss; since it's always the high bit for all
> + * current formats we can do this here rather than in
> + * formatting. This may break if we get interesting formats.
> + */
> + if (map->bus->read_flag_in_reg)
> + u8[0] |= 0x80;
This is rather specific. Making this a per device mask or bit offset, would
make more sense in my opinion. I have for example one SPI codec device which
uses the 7th bit to indicate a read. And I also have devices on a custom bus,
which needs to set the upper bit to indicate a write, so a mask for both write
and read would be good.
> +
> + ret = map->bus->read(map->dev, map->work_buf, map->format.reg_bytes,
> + val, map->format.val_bytes);
> + if (ret != 0)
> + return ret;
> +
> + return 0;
> +}
> +
> [...]
> +/**
> + * regmap_bulk_read: Read multiple registers from the device
> + *
> + * @map: Register map to write to
> + * @reg: First register to be read from
> + * @val: Pointer to store read value, in native register size for device
> + * @val_count: Number of registers to read
> + *
> + * A value of zero will be returned on success, a negative errno will
> + * be returned in error cases.
> + */
> +int regmap_bulk_read(struct regmap *map, unsigned int reg, void *val,
> + size_t val_count)
> +{
> + int ret, i;
> + size_t val_bytes = map->format.val_bytes;
> +
> + if (!map->format.parse_val)
> + return -EINVAL;
> +
> + ret = regmap_raw_read(map, reg, val, val_bytes);
val_bytes * val_count
> + if (ret != 0)
> + return ret;
> +
> + for (i = 0; i < val_count * val_bytes; i += val_bytes)
> + map->format.parse_val(val + i);
This doesn't make sense. parse_val returns the parsed value, but doesn't modify
the parsed data. It would make sense to make the interface similar to
regmap_read and make the returned values a unsigned integer array and use a
bounce buffer for reading them from the device.
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(regmap_bulk_read);
> +
> [...]
> diff --git a/include/linux/regmap.h b/include/linux/regmap.h
> new file mode 100644
> index 0000000..0c2e402
> --- /dev/null
> +++ b/include/linux/regmap.h
> @@ -0,0 +1,75 @@
> +#ifndef __LINUX_REGMAP_H
> +#define __LINUX_REGMAP_H
> +
> +/*
> + * Register map access API
> + *
> + * Copyright 2011 Wolfson Microelectronics plc
> + *
> + * Author: Mark Brown <[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 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/list.h>
#include <linux/module.h>
> +
> +struct regmap_config {
> + int reg_bits;
> + int val_bits;
size_t for both?
> +};
> +
> ...
> +
> +struct regmap *regmap_init(struct device *dev, struct regmap_config *config);
const struct regmap_config
> +void regmap_free(struct regmap *map);
> +int regmap_write(struct regmap *map, unsigned int reg, unsigned int val);
> +int regmap_raw_write(struct regmap *map, unsigned int reg,
> + const void *val, size_t val_len);
> +int regmap_read(struct regmap *map, unsigned int reg, unsigned int *val);
> +int regmap_raw_read(struct regmap *map, unsigned int reg,
> + void *val, size_t val_len);
> +int regmap_bulk_read(struct regmap *map, unsigned int reg, void *val,
> + size_t val_count);
What about bulk_write?
On 06/20/2011 02:54 PM, Mark Brown wrote:
> [...]
> +
> +#include <linux/regmap.h>
> +#include <linux/i2c.h>
#include <linux/init.h>
> [...]
> +
> +static int regmap_i2c_read(struct device *dev,
> + const void *reg, size_t reg_size,
> + void *val, size_t val_size)
> +{
> + struct i2c_client *i2c = to_i2c_client(dev);
> + struct i2c_msg xfer[2];
> + int ret;
> +
> + if (!i2c_check_functionality(i2c->adapter, I2C_FUNC_PROTOCOL_MANGLING))
> + return -ENOTSUPP;
You shouldn't need I2C_FUNC_PROTOCOL_MANGLING for reading.
> [...]
> diff --git a/drivers/regmap/regmap.c b/drivers/regmap/regmap.c
> index a8610c7..896b710 100644
> --- a/drivers/regmap/regmap.c
> +++ b/drivers/regmap/regmap.c
> @@ -461,7 +461,7 @@ out:
> [...]
I guess this should be part of the first patch.
On 06/20/2011 02:54 PM, Mark Brown wrote:
> [...]
> +
> +static int regmap_spi_read(struct device *dev,
> + const void *reg, size_t reg_size,
> + void *val, size_t val_size)
> +{
> + struct spi_device *spi = to_spi_device(dev);
> +
> + return spi_write_then_read(spi, reg, reg_size, val, val_size);
spi_write_then_read will use a bounce buffer internally, since we already have
our own bounce buffer it is probably better to use the low-level spi interface
directly in this case.
On Tue, Jun 21, 2011 at 01:22:55AM +0200, Lars-Peter Clausen wrote:
> On 06/20/2011 02:54 PM, Mark Brown wrote:
> > [...]
> > +
> > +#include <linux/regmap.h>
> > +#include <linux/i2c.h>
> #include <linux/init.h>
For...?
> > +static int regmap_i2c_read(struct device *dev,
> > + const void *reg, size_t reg_size,
> > + void *val, size_t val_size)
> > +{
> > + struct i2c_client *i2c = to_i2c_client(dev);
> > + struct i2c_msg xfer[2];
> > + int ret;
> > + if (!i2c_check_functionality(i2c->adapter, I2C_FUNC_PROTOCOL_MANGLING))
> > + return -ENOTSUPP;
> You shouldn't need I2C_FUNC_PROTOCOL_MANGLING for reading.
Hrm, yeah, probably not.
On Tue, Jun 21, 2011 at 01:26:48AM +0200, Lars-Peter Clausen wrote:
> On 06/20/2011 02:54 PM, Mark Brown wrote:
> > +static int regmap_spi_read(struct device *dev,
> > + const void *reg, size_t reg_size,
> > + void *val, size_t val_size)
> > +{
> > + struct spi_device *spi = to_spi_device(dev);
> > + return spi_write_then_read(spi, reg, reg_size, val, val_size);
> spi_write_then_read will use a bounce buffer internally, since we already have
> our own bounce buffer it is probably better to use the low-level spi interface
> directly in this case.
I've got this horrible feeling that if we try that we'll discover that
the reason the SPI API does this internally is just as valid here - if I
remember correctly it's doing this due to restrictions on DMA from the
stack and I'd strongly expect val to end up on the stack for registers.
Or to look at it from the other point of view if we don't need the
bounce buffers then why does spi_write_then_read() need them?
On 06/21/2011 01:45 AM, Mark Brown wrote:
> On Tue, Jun 21, 2011 at 01:26:48AM +0200, Lars-Peter Clausen wrote:
>> On 06/20/2011 02:54 PM, Mark Brown wrote:
>
>>> +static int regmap_spi_read(struct device *dev,
>>> + const void *reg, size_t reg_size,
>>> + void *val, size_t val_size)
>>> +{
>>> + struct spi_device *spi = to_spi_device(dev);
>
>>> + return spi_write_then_read(spi, reg, reg_size, val, val_size);
>
>> spi_write_then_read will use a bounce buffer internally, since we already have
>> our own bounce buffer it is probably better to use the low-level spi interface
>> directly in this case.
>
> I've got this horrible feeling that if we try that we'll discover that
> the reason the SPI API does this internally is just as valid here - if I
> remember correctly it's doing this due to restrictions on DMA from the
> stack and I'd strongly expect val to end up on the stack for registers.
> Or to look at it from the other point of view if we don't need the
> bounce buffers then why does spi_write_then_read() need them?
hm, right, I overlooked that val could be on the stack. I though we were always
using some kind of bounce buffer internally.
On 06/21/2011 01:41 AM, Mark Brown wrote:
> On Tue, Jun 21, 2011 at 01:22:55AM +0200, Lars-Peter Clausen wrote:
>> On 06/20/2011 02:54 PM, Mark Brown wrote:
>>> [...]
>>> +
>>> +#include <linux/regmap.h>
>>> +#include <linux/i2c.h>
>
>> #include <linux/init.h>
>
> For...?
>
postcore_initcall and module_exit
On Tue, Jun 21, 2011 at 01:15:18AM +0200, Lars-Peter Clausen wrote:
> > + void (*format_reg)(void *buf, unsigned int reg);
> > + void (*format_val)(void *buf, unsigned int val);
> > + unsigned int (*parse_val)(void *buf);
> const void *buf. It probably also makes sense to pass the regmap to all
Hrm, yeah.
> callbacks, so we can write generic format/parser functions which use
> information for example from the regmap_format.
I'd rather add it when we need it, it's not much work to refactor and
it's entirely in this file. Right now I like being sure I know what
knows about what bit of data mangling.
> > +static void regmap_format_4_12_write(struct regmap *map,
> > + unsigned int reg, unsigned int val)
> > +{
> > + u16 *out = map->work_buf;
> > + *out = cpu_to_be16((reg << 12) | val);
> > +}
> > +static void regmap_format_7_9_write(struct regmap *map,
> > + unsigned int reg, unsigned int val)
> > +{
> > + u16 *out = map->work_buf;
> > + *out = cpu_to_be16((reg << 9) | val);
> > +}
> It would make sense to keep val_bits around and implement these two generically:
> *out = cpu_to_be16((reg << map->format.val_bits) | val);
I'm not sure it's worth it for the two cases we know about, and as with
passing the map into these I like keeping the byte oriented assumption
in the interfaces as much as possible as it makes things easier to think
about.
> > +struct regmap *regmap_init(struct device *dev, struct regmap_config *config)
> regmap_alloc would in my opinion be a better name.
I like init, especially considering the plan to add cache support as
there's more work in setting that up once you start doing the advanced
caches.
> > + /*
> > + * Some buses flag reads by setting the high bit in the
> > + * register addresss; since it's always the high bit for all
> > + * current formats we can do this here rather than in
> > + * formatting. This may break if we get interesting formats.
> > + */
> > + if (map->bus->read_flag_in_reg)
> > + u8[0] |= 0x80;
> This is rather specific. Making this a per device mask or bit offset, would
> make more sense in my opinion. I have for example one SPI codec device which
> uses the 7th bit to indicate a read. And I also have devices on a custom bus,
Can you say what device this is? I'm just going on the devices we've
seen in the kernel so far here... Still easy enough to do.
> which needs to set the upper bit to indicate a write, so a mask for both write
> and read would be good.
Sure, though for stuff like this I'd say just implement them when we
need them.
> > + for (i = 0; i < val_count * val_bytes; i += val_bytes)
> > + map->format.parse_val(val + i);
> This doesn't make sense. parse_val returns the parsed value, but doesn't modify
Oh, meh. It used to.
> the parsed data. It would make sense to make the interface similar to
> regmap_read and make the returned values a unsigned integer array and use a
> bounce buffer for reading them from the device.
I thought about that but what it actually ends up meaning for most of
the current users is that they then need to either implement their own
bounce buffer or do some cross tree updates to switch from using the
native type to ints. Neither seemed terribly appealing and it did seem
reasonable to expect devices to know how big their own registers are.
> > +struct regmap_config {
> > + int reg_bits;
> > + int val_bits;
> size_t for both?
It doesn't seem particularly appropriate as we're working with a bit
count not the usual byte count where size_t gets used and I can't see it
ever making any difference.
> > +int regmap_bulk_read(struct regmap *map, unsigned int reg, void *val,
> > + size_t val_count);
> What about bulk_write?
It's painful to implement efficiently due to the need to mangle the data
on the way down, and most of the time if you're doing a bulk operation
it's because you want it to be efficient. I'm punting it until someone
really wants it so we can have a separate think about what makes sense.
On 06/21/2011 02:14 AM, Mark Brown wrote:
>
>>> +static void regmap_format_4_12_write(struct regmap *map,
>>> + unsigned int reg, unsigned int val)
>>> +{
>>> + u16 *out = map->work_buf;
>>> + *out = cpu_to_be16((reg << 12) | val);
>>> +}
>
>>> +static void regmap_format_7_9_write(struct regmap *map,
>>> + unsigned int reg, unsigned int val)
>>> +{
>>> + u16 *out = map->work_buf;
>>> + *out = cpu_to_be16((reg << 9) | val);
>>> +}
>
>> It would make sense to keep val_bits around and implement these two generically:
>> *out = cpu_to_be16((reg << map->format.val_bits) | val);
>
> I'm not sure it's worth it for the two cases we know about, and as with
> passing the map into these I like keeping the byte oriented assumption
> in the interfaces as much as possible as it makes things easier to think
> about.
Hm, ok I guess we can wait with this until there are actually other similar
formats which follow this scheme.
>
>>> +struct regmap *regmap_init(struct device *dev, struct regmap_config *config)
>
>> regmap_alloc would in my opinion be a better name.
>
> I like init, especially considering the plan to add cache support as
> there's more work in setting that up once you start doing the advanced
> caches.
If you take a look at other kernel apis _alloc is usually used if the structure
is allocated (and initialized) inside the function and _init is used when the
function initializes an already existing structure. And it also matches better
with regmap_free.
>
>>> + /*
>>> + * Some buses flag reads by setting the high bit in the
>>> + * register addresss; since it's always the high bit for all
>>> + * current formats we can do this here rather than in
>>> + * formatting. This may break if we get interesting formats.
>>> + */
>>> + if (map->bus->read_flag_in_reg)
>>> + u8[0] |= 0x80;
>
>> This is rather specific. Making this a per device mask or bit offset, would
>> make more sense in my opinion. I have for example one SPI codec device which
>> uses the 7th bit to indicate a read. And I also have devices on a custom bus,
>
> Can you say what device this is? I'm just going on the devices we've
> seen in the kernel so far here... Still easy enough to do.
>
The ad1836 for example really only uses 10 bits for data instead of 12.
The 11th bit is reserved and the 12th bit is used to indicate whether it is an
read or write. Though since gaps between register and data are not supported
data was made 12 bits wide and the upper 2 bits are always set to 0.
The adav801, which is going to be submitted upstream soon, uses a similar
scheme. First seven bits are register address, then 1 bit which indicates
read/write and then 8 bits data. We are currently using the same trick here and
made data 9 bits wide.
>>> +struct regmap_config {
>>> + int reg_bits;
>>> + int val_bits;
>
>> size_t for both?
>
> It doesn't seem particularly appropriate as we're working with a bit
> count not the usual byte count where size_t gets used and I can't see it
> ever making any difference.
I suggested it because the byte count in the regmap_format struct is size_t and
it calculated from the bit count.
On Tue, Jun 21, 2011 at 02:45:53AM +0200, Lars-Peter Clausen wrote:
> On 06/21/2011 02:14 AM, Mark Brown wrote:
> > I like init, especially considering the plan to add cache support as
> > there's more work in setting that up once you start doing the advanced
> > caches.
> If you take a look at other kernel apis _alloc is usually used if the structure
> is allocated (and initialized) inside the function and _init is used when the
> function initializes an already existing structure. And it also matches better
That more applies to split alloc/init models - here there's a single
operation that does both. To me alloc() generally means that it just
allocates a structure and leaves initialization still to be done while.
> with regmap_free.
That's true.
On Mon, Jun 20, 2011 at 01:54:42PM +0100, Mark Brown wrote:
> +static int _regmap_raw_write(struct regmap *map, unsigned int reg,
> + const void *val, size_t val_len)
> +{
> + void *buf;
> + int ret = -ENOTSUPP;
> + int len;
size_t for len?
Thanks,
Dimitris
On Tue, Jun 21, 2011 at 02:24:30AM +0100, Mark Brown wrote:
> On Tue, Jun 21, 2011 at 02:45:53AM +0200, Lars-Peter Clausen wrote:
> > On 06/21/2011 02:14 AM, Mark Brown wrote:
>
> > > I like init, especially considering the plan to add cache support as
> > > there's more work in setting that up once you start doing the advanced
> > > caches.
>
> > If you take a look at other kernel apis _alloc is usually used if the structure
> > is allocated (and initialized) inside the function and _init is used when the
> > function initializes an already existing structure. And it also matches better
>
> That more applies to split alloc/init models - here there's a single
> operation that does both. To me alloc() generally means that it just
> allocates a structure and leaves initialization still to be done while.
>
> > with regmap_free.
>
> That's true.
In the soc-cache code we use _init and _exit respectively. Might be
worth considering instead of _free?
Thanks,
Dimitris
On Tue, Jun 21, 2011 at 12:47:09PM +0100, Dimitris Papastamos wrote:
> On Tue, Jun 21, 2011 at 02:24:30AM +0100, Mark Brown wrote:
> > On Tue, Jun 21, 2011 at 02:45:53AM +0200, Lars-Peter Clausen wrote:
> > > with regmap_free.
> > That's true.
> In the soc-cache code we use _init and _exit respectively. Might be
> worth considering instead of _free?
Err, yes - that's what I said :)
On Tue, Jun 21, 2011 at 12:43:47PM +0100, Dimitris Papastamos wrote:
> On Mon, Jun 20, 2011 at 01:54:42PM +0100, Mark Brown wrote:
> > +static int _regmap_raw_write(struct regmap *map, unsigned int reg,
> > + const void *val, size_t val_len)
> > +{
> > + void *buf;
> > + int ret = -ENOTSUPP;
> > + int len;
> size_t for len?
Yes.