[PATCH 0/3] Add "no-bus" configuration for regmap API

All of lore.kernel.org
 help / color / mirror / Atom feed

* [PATCH 0/3] Add "no-bus" configuration for regmap API
@ 2012-12-21  9:47 Andrey Smirnov
  2012-12-21  9:47 ` [PATCH 1/3] Add provisions to have user-defined read operation Andrey Smirnov
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Andrey Smirnov @ 2012-12-21  9:47 UTC (permalink / raw)
  To: andrey.smirnov; +Cc: broonie, linux-kernel

This patch proposes extension to the API that was suggested by Mark in
the following thread:
http://article.gmane.org/gmane.linux.kernel/1383734

The idea behind extensions is to allow the devices that expose some
register-like interface but whose protocol for reading or writing
those registers could not be simplified to serialized bytestream
writes to be used within 'regmap' framework

Original RFC thread can be found here:
https://lkml.org/lkml/2012/11/23/367

First two patches are refactoring patches that add all the necessary
plumbing to the regmap internals and move old functionality to use
that infrastracture.

Third patch introduces code that implements 'no-bus' configuration

Andrey Smirnov (3):
  Add provisions to have user-defined read operation
  Add provisions to have user-defined write operation
  Add "no-bus" option for regmap API

 drivers/base/regmap/internal.h |    5 ++
 drivers/base/regmap/regmap.c   |  156 +++++++++++++++++++++++++++++-----------
 include/linux/regmap.h         |    7 ++
 3 files changed, 126 insertions(+), 42 deletions(-)

-- 
1.7.9.5


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/3] Add provisions to have user-defined read operation
  2012-12-21  9:47 [PATCH 0/3] Add "no-bus" configuration for regmap API Andrey Smirnov
@ 2012-12-21  9:47 ` Andrey Smirnov
  2012-12-21  9:47 ` [PATCH 2/3] Add provisions to have user-defined write operation Andrey Smirnov
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Andrey Smirnov @ 2012-12-21  9:47 UTC (permalink / raw)
  To: andrey.smirnov; +Cc: broonie, linux-kernel

This commit is a preparatory commit to provide "no-bus" configuration
option for regmap API. It adds necessary plumbing needed to have the
ability to provide user define register read function.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 drivers/base/regmap/internal.h |    2 ++
 drivers/base/regmap/regmap.c   |   34 +++++++++++++++++++++++++---------
 2 files changed, 27 insertions(+), 9 deletions(-)

diff --git a/drivers/base/regmap/internal.h b/drivers/base/regmap/internal.h
index 80f9ab9..c450c1f 100644
--- a/drivers/base/regmap/internal.h
+++ b/drivers/base/regmap/internal.h
@@ -58,6 +58,8 @@ struct regmap {
 	bool (*volatile_reg)(struct device *dev, unsigned int reg);
 	bool (*precious_reg)(struct device *dev, unsigned int reg);
 
+	int (*reg_read)(struct regmap *map, unsigned int reg, unsigned int *val);
+
 	u8 read_flag_mask;
 	u8 write_flag_mask;
 
diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index c241ae2..4e45877 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -34,6 +34,9 @@ static int _regmap_update_bits(struct regmap *map, unsigned int reg,
 			       unsigned int mask, unsigned int val,
 			       bool *change);
 
+static int _regmap_bus_read(struct regmap *map, unsigned int reg,
+			    unsigned int *val);
+
 bool regmap_writeable(struct regmap *map, unsigned int reg)
 {
 	if (map->max_register && reg > map->max_register)
@@ -373,6 +376,8 @@ struct regmap *regmap_init(struct device *dev,
 		map->read_flag_mask = bus->read_flag_mask;
 	}
 
+	map->reg_read = _regmap_bus_read;
+
 	reg_endian = config->reg_format_endian;
 	if (reg_endian == REGMAP_ENDIAN_DEFAULT)
 		reg_endian = bus->reg_format_endian_default;
@@ -1090,10 +1095,26 @@ static int _regmap_raw_read(struct regmap *map, unsigned int reg, void *val,
 	return ret;
 }
 
+static int _regmap_bus_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;
+}
+
 static int _regmap_read(struct regmap *map, unsigned int reg,
 			unsigned int *val)
 {
 	int ret;
+	BUG_ON(!map->reg_read);
 
 	if (!map->cache_bypass) {
 		ret = regcache_read(map, reg, val);
@@ -1101,26 +1122,21 @@ static int _regmap_read(struct regmap *map, unsigned int reg,
 			return 0;
 	}
 
-	if (!map->format.parse_val)
-		return -EINVAL;
-
 	if (map->cache_only)
 		return -EBUSY;
 
-	ret = _regmap_raw_read(map, reg, map->work_buf, map->format.val_bytes);
+	ret = map->reg_read(map, reg, val);
 	if (ret == 0) {
-		*val = map->format.parse_val(map->work_buf);
-
 #ifdef LOG_DEVICE
 		if (strcmp(dev_name(map->dev), LOG_DEVICE) == 0)
 			dev_info(map->dev, "%x => %x\n", reg, *val);
 #endif
 
 		trace_regmap_reg_read(map->dev, reg, *val);
-	}
 
-	if (ret == 0 && !map->cache_bypass)
-		regcache_write(map, reg, *val);
+		if (!map->cache_bypass)
+			regcache_write(map, reg, *val);
+	}
 
 	return ret;
 }
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/3] Add provisions to have user-defined write operation
  2012-12-21  9:47 [PATCH 0/3] Add "no-bus" configuration for regmap API Andrey Smirnov
  2012-12-21  9:47 ` [PATCH 1/3] Add provisions to have user-defined read operation Andrey Smirnov
@ 2012-12-21  9:47 ` Andrey Smirnov
  2012-12-21  9:47 ` [PATCH 3/3] Add "no-bus" option for regmap API Andrey Smirnov
  2012-12-27 17:41 ` [PATCH 0/3] Add "no-bus" configuration " Mark Brown
  3 siblings, 0 replies; 7+ messages in thread
From: Andrey Smirnov @ 2012-12-21  9:47 UTC (permalink / raw)
  To: andrey.smirnov; +Cc: broonie, linux-kernel

This commit is a preparatory commit to provide "no-bus" configuration
option for regmap API. It adds necessary plumbing needed to have the
ability to provide user define register write function.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 drivers/base/regmap/internal.h |    1 +
 drivers/base/regmap/regmap.c   |   72 ++++++++++++++++++++++++++--------------
 2 files changed, 48 insertions(+), 25 deletions(-)

diff --git a/drivers/base/regmap/internal.h b/drivers/base/regmap/internal.h
index c450c1f..5d10c6d 100644
--- a/drivers/base/regmap/internal.h
+++ b/drivers/base/regmap/internal.h
@@ -59,6 +59,7 @@ struct regmap {
 	bool (*precious_reg)(struct device *dev, unsigned int reg);
 
 	int (*reg_read)(struct regmap *map, unsigned int reg, unsigned int *val);
+	int (*reg_write)(struct regmap *map, unsigned int reg, unsigned int val);
 
 	u8 read_flag_mask;
 	u8 write_flag_mask;
diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index 4e45877..006b50b 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -36,6 +36,11 @@ static int _regmap_update_bits(struct regmap *map, unsigned int reg,
 
 static int _regmap_bus_read(struct regmap *map, unsigned int reg,
 			    unsigned int *val);
+static int _regmap_bus_formatted_write(struct regmap *map, unsigned int reg,
+				       unsigned int val);
+static int _regmap_bus_raw_write(struct regmap *map, unsigned int reg,
+				 unsigned int val);
+
 
 bool regmap_writeable(struct regmap *map, unsigned int reg)
 {
@@ -523,6 +528,11 @@ struct regmap *regmap_init(struct device *dev,
 		goto err_map;
 	}
 
+	if (map->format.format_write)
+		map->reg_write = _regmap_bus_formatted_write;
+	else if (map->format.format_val)
+		map->reg_write = _regmap_bus_raw_write;
+
 	map->range_tree = RB_ROOT;
 	for (i = 0; i < config->n_ranges; i++) {
 		const struct regmap_range_cfg *range_cfg = &config->ranges[i];
@@ -883,11 +893,46 @@ static int _regmap_raw_write(struct regmap *map, unsigned int reg,
 	return ret;
 }
 
+static int _regmap_bus_formatted_write(struct regmap *map, unsigned int reg,
+				       unsigned int val)
+{
+	int ret;
+	BUG_ON(!map->format.format_write);
+
+	ret = _regmap_select_page(map, &reg, 1);
+	if (ret < 0)
+		return ret;
+
+	map->format.format_write(map, reg, val);
+
+	trace_regmap_hw_write_start(map->dev, reg, 1);
+
+	ret = map->bus->write(map->bus_context, map->work_buf,
+			      map->format.buf_size);
+
+	trace_regmap_hw_write_done(map->dev, reg, 1);
+
+	return ret;
+}
+
+static int _regmap_bus_raw_write(struct regmap *map, unsigned int reg,
+				 unsigned int val)
+{
+	BUG_ON(!map->format.format_val);
+
+	map->format.format_val(map->work_buf + map->format.reg_bytes
+			       + map->format.pad_bytes, val, 0);
+	return _regmap_raw_write(map, reg,
+				 map->work_buf +
+				 map->format.reg_bytes +
+				 map->format.pad_bytes,
+				 map->format.val_bytes);
+}
+
 int _regmap_write(struct regmap *map, unsigned int reg,
 		  unsigned int val)
 {
 	int ret;
-	BUG_ON(!map->format.format_write && !map->format.format_val);
 
 	if (!map->cache_bypass && map->format.format_write) {
 		ret = regcache_write(map, reg, val);
@@ -906,30 +951,7 @@ int _regmap_write(struct regmap *map, unsigned int reg,
 
 	trace_regmap_reg_write(map->dev, reg, val);
 
-	if (map->format.format_write) {
-		ret = _regmap_select_page(map, &reg, 1);
-		if (ret < 0)
-			return ret;
-
-		map->format.format_write(map, reg, val);
-
-		trace_regmap_hw_write_start(map->dev, reg, 1);
-
-		ret = map->bus->write(map->bus_context, map->work_buf,
-				      map->format.buf_size);
-
-		trace_regmap_hw_write_done(map->dev, reg, 1);
-
-		return ret;
-	} else {
-		map->format.format_val(map->work_buf + map->format.reg_bytes
-				       + map->format.pad_bytes, val, 0);
-		return _regmap_raw_write(map, reg,
-					 map->work_buf +
-					 map->format.reg_bytes +
-					 map->format.pad_bytes,
-					 map->format.val_bytes);
-	}
+	return map->reg_write(map, reg, val);
 }
 
 /**
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 3/3] Add "no-bus" option for regmap API
  2012-12-21  9:47 [PATCH 0/3] Add "no-bus" configuration for regmap API Andrey Smirnov
  2012-12-21  9:47 ` [PATCH 1/3] Add provisions to have user-defined read operation Andrey Smirnov
  2012-12-21  9:47 ` [PATCH 2/3] Add provisions to have user-defined write operation Andrey Smirnov
@ 2012-12-21  9:47 ` Andrey Smirnov
  2012-12-27 18:24   ` Mark Brown
  2012-12-27 17:41 ` [PATCH 0/3] Add "no-bus" configuration " Mark Brown
  3 siblings, 1 reply; 7+ messages in thread
From: Andrey Smirnov @ 2012-12-21  9:47 UTC (permalink / raw)
  To: andrey.smirnov; +Cc: broonie, linux-kernel

This commit adds provision for "no-bus" usage of the regmap API. In
this configuration user can provide API with two callbacks 'reg_read'
and 'reg_write' which are to be called when reads and writes to one of
device's registers is performed. This is useful for devices that
expose registers but whose register access sequence does not fit the 'bus'
abstraction.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 drivers/base/regmap/internal.h |    6 ++--
 drivers/base/regmap/regmap.c   |   78 ++++++++++++++++++++++++++++------------
 include/linux/regmap.h         |    7 ++++
 3 files changed, 67 insertions(+), 24 deletions(-)

diff --git a/drivers/base/regmap/internal.h b/drivers/base/regmap/internal.h
index 5d10c6d..644ff3d 100644
--- a/drivers/base/regmap/internal.h
+++ b/drivers/base/regmap/internal.h
@@ -58,8 +58,10 @@ struct regmap {
 	bool (*volatile_reg)(struct device *dev, unsigned int reg);
 	bool (*precious_reg)(struct device *dev, unsigned int reg);
 
-	int (*reg_read)(struct regmap *map, unsigned int reg, unsigned int *val);
-	int (*reg_write)(struct regmap *map, unsigned int reg, unsigned int val);
+	int (*reg_read)(void *context, unsigned int reg, unsigned int *val);
+	int (*reg_write)(void *context, unsigned int reg, unsigned int val);
+
+	bool cache_registers;
 
 	u8 read_flag_mask;
 	u8 write_flag_mask;
diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index 006b50b..cd0ad64 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -34,11 +34,11 @@ static int _regmap_update_bits(struct regmap *map, unsigned int reg,
 			       unsigned int mask, unsigned int val,
 			       bool *change);
 
-static int _regmap_bus_read(struct regmap *map, unsigned int reg,
+static int _regmap_bus_read(void *context, unsigned int reg,
 			    unsigned int *val);
-static int _regmap_bus_formatted_write(struct regmap *map, unsigned int reg,
+static int _regmap_bus_formatted_write(void *contex, unsigned int reg,
 				       unsigned int val);
-static int _regmap_bus_raw_write(struct regmap *map, unsigned int reg,
+static int _regmap_bus_raw_write(void *context, unsigned int reg,
 				 unsigned int val);
 
 
@@ -334,7 +334,7 @@ struct regmap *regmap_init(struct device *dev,
 	enum regmap_endian reg_endian, val_endian;
 	int i, j;
 
-	if (!bus || !config)
+	if (!config)
 		goto err;
 
 	map = kzalloc(sizeof(*map), GFP_KERNEL);
@@ -343,14 +343,14 @@ struct regmap *regmap_init(struct device *dev,
 		goto err;
 	}
 
-	if (bus->fast_io) {
-		spin_lock_init(&map->spinlock);
-		map->lock = regmap_lock_spinlock;
-		map->unlock = regmap_unlock_spinlock;
-	} else {
+	if (!bus || !bus->fast_io) {
 		mutex_init(&map->mutex);
 		map->lock = regmap_lock_mutex;
 		map->unlock = regmap_unlock_mutex;
+	} else {
+		spin_lock_init(&map->spinlock);
+		map->lock = regmap_lock_spinlock;
+		map->unlock = regmap_unlock_spinlock;
 	}
 	map->format.reg_bytes = DIV_ROUND_UP(config->reg_bits, 8);
 	map->format.pad_bytes = config->pad_bits / 8;
@@ -373,6 +373,15 @@ struct regmap *regmap_init(struct device *dev,
 	map->precious_reg = config->precious_reg;
 	map->cache_type = config->cache_type;
 	map->name = config->name;
+	map->reg_read = config->reg_read;
+	map->reg_write = config->reg_write;
+
+	if (!bus) {
+		map->cache_registers = true;
+		goto skip_format_initialization;
+	} else {
+		map->reg_read = _regmap_bus_read;
+	}
 
 	if (config->read_flag_mask || config->write_flag_mask) {
 		map->read_flag_mask = config->read_flag_mask;
@@ -381,8 +390,6 @@ struct regmap *regmap_init(struct device *dev,
 		map->read_flag_mask = bus->read_flag_mask;
 	}
 
-	map->reg_read = _regmap_bus_read;
-
 	reg_endian = config->reg_format_endian;
 	if (reg_endian == REGMAP_ENDIAN_DEFAULT)
 		reg_endian = bus->reg_format_endian_default;
@@ -528,10 +535,14 @@ struct regmap *regmap_init(struct device *dev,
 		goto err_map;
 	}
 
-	if (map->format.format_write)
+	if (map->format.format_write) {
+		map->cache_registers = true;
 		map->reg_write = _regmap_bus_formatted_write;
-	else if (map->format.format_val)
+	} else if (map->format.format_val) {
 		map->reg_write = _regmap_bus_raw_write;
+	}
+
+skip_format_initialization:
 
 	map->range_tree = RB_ROOT;
 	for (i = 0; i < config->n_ranges; i++) {
@@ -712,7 +723,7 @@ void regmap_exit(struct regmap *map)
 	regcache_exit(map);
 	regmap_debugfs_exit(map);
 	regmap_range_exit(map);
-	if (map->bus->free_context)
+	if (map->bus && map->bus->free_context)
 		map->bus->free_context(map->bus_context);
 	kfree(map->work_buf);
 	kfree(map);
@@ -817,6 +828,8 @@ static int _regmap_raw_write(struct regmap *map, unsigned int reg,
 	size_t len;
 	int i;
 
+	BUG_ON(!map->bus);
+
 	/* Check for unwritable registers before we start */
 	if (map->writeable_reg)
 		for (i = 0; i < val_len / map->format.val_bytes; i++)
@@ -893,11 +906,13 @@ static int _regmap_raw_write(struct regmap *map, unsigned int reg,
 	return ret;
 }
 
-static int _regmap_bus_formatted_write(struct regmap *map, unsigned int reg,
+static int _regmap_bus_formatted_write(void *context, unsigned int reg,
 				       unsigned int val)
 {
 	int ret;
-	BUG_ON(!map->format.format_write);
+	struct regmap *map = context;
+
+	BUG_ON(!map->bus || !map->format.format_write);
 
 	ret = _regmap_select_page(map, &reg, 1);
 	if (ret < 0)
@@ -915,10 +930,12 @@ static int _regmap_bus_formatted_write(struct regmap *map, unsigned int reg,
 	return ret;
 }
 
-static int _regmap_bus_raw_write(struct regmap *map, unsigned int reg,
+static int _regmap_bus_raw_write(void *context, unsigned int reg,
 				 unsigned int val)
 {
-	BUG_ON(!map->format.format_val);
+	struct regmap *map = context;
+
+	BUG_ON(!map->bus || !map->format.format_val);
 
 	map->format.format_val(map->work_buf + map->format.reg_bytes
 			       + map->format.pad_bytes, val, 0);
@@ -934,7 +951,7 @@ int _regmap_write(struct regmap *map, unsigned int reg,
 {
 	int ret;
 
-	if (!map->cache_bypass && map->format.format_write) {
+	if (!map->cache_bypass && map->cache_registers) {
 		ret = regcache_write(map, reg, val);
 		if (ret != 0)
 			return ret;
@@ -951,7 +968,10 @@ int _regmap_write(struct regmap *map, unsigned int reg,
 
 	trace_regmap_reg_write(map->dev, reg, val);
 
-	return map->reg_write(map, reg, val);
+	if (map->bus)
+		return map->reg_write(map, reg, val);
+	else
+		return map->reg_write(map->bus_context, reg, val);
 }
 
 /**
@@ -1002,6 +1022,8 @@ int regmap_raw_write(struct regmap *map, unsigned int reg,
 {
 	int ret;
 
+	if (!map->bus)
+		return -EINVAL;
 	if (val_len % map->format.val_bytes)
 		return -EINVAL;
 	if (reg % map->reg_stride)
@@ -1038,6 +1060,8 @@ int regmap_bulk_write(struct regmap *map, unsigned int reg, const void *val,
 	size_t val_bytes = map->format.val_bytes;
 	void *wval;
 
+	if (!map->bus)
+		return -EINVAL;
 	if (!map->format.parse_val)
 		return -EINVAL;
 	if (reg % map->reg_stride)
@@ -1090,6 +1114,8 @@ static int _regmap_raw_read(struct regmap *map, unsigned int reg, void *val,
 	u8 *u8 = map->work_buf;
 	int ret;
 
+	BUG_ON(!map->bus);
+
 	ret = _regmap_select_page(map, &reg, val_len / map->format.val_bytes);
 	if (ret < 0)
 		return ret;
@@ -1117,10 +1143,11 @@ static int _regmap_raw_read(struct regmap *map, unsigned int reg, void *val,
 	return ret;
 }
 
-static int _regmap_bus_read(struct regmap *map, unsigned int reg,
+static int _regmap_bus_read(void *context, unsigned int reg,
 			    unsigned int *val)
 {
 	int ret;
+	struct regmap *map = context;
 
 	if (!map->format.parse_val)
 		return -EINVAL;
@@ -1147,7 +1174,10 @@ static int _regmap_read(struct regmap *map, unsigned int reg,
 	if (map->cache_only)
 		return -EBUSY;
 
-	ret = map->reg_read(map, reg, val);
+	if (map->bus)
+		ret = map->reg_read(map, reg, val);
+	else
+		ret = map->reg_read(map->bus_context, reg, val);
 	if (ret == 0) {
 #ifdef LOG_DEVICE
 		if (strcmp(dev_name(map->dev), LOG_DEVICE) == 0)
@@ -1209,6 +1239,8 @@ int regmap_raw_read(struct regmap *map, unsigned int reg, void *val,
 	unsigned int v;
 	int ret, i;
 
+	if (!map->bus)
+		return -EINVAL;
 	if (val_len % map->format.val_bytes)
 		return -EINVAL;
 	if (reg % map->reg_stride)
@@ -1260,6 +1292,8 @@ int regmap_bulk_read(struct regmap *map, unsigned int reg, void *val,
 	size_t val_bytes = map->format.val_bytes;
 	bool vol = regmap_volatile_range(map, reg, val_count);
 
+	if (!map->bus)
+		return -EINVAL;
 	if (!map->format.parse_val)
 		return -EINVAL;
 	if (reg % map->reg_stride)
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index 7f7e00d..78ac1c1 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -75,6 +75,10 @@ enum regmap_endian {
  * @precious_reg: Optional callback returning true if the rgister
  *                should not be read outside of a call from the driver
  *                (eg, a clear on read interrupt status register).
+ * @reg_read: Optional callback that if filled will be used to perform
+ *            all the reads from the registers.
+ * @reg_write: Optional callback that if filled will be used to perform
+ *             all the writes to the registers.
  *
  * @max_register: Optional, specifies the maximum valid register index.
  * @reg_defaults: Power on reset values for registers (for use with
@@ -117,6 +121,9 @@ struct regmap_config {
 	bool (*volatile_reg)(struct device *dev, unsigned int reg);
 	bool (*precious_reg)(struct device *dev, unsigned int reg);
 
+	int (*reg_read)(void *context, unsigned int reg, unsigned int *val);
+	int (*reg_write)(void *context, unsigned int reg, unsigned int val);
+
 	unsigned int max_register;
 	const struct reg_default *reg_defaults;
 	unsigned int num_reg_defaults;
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 3/3] Add "no-bus" option for regmap API
  2012-12-21  9:47 ` [PATCH 3/3] Add "no-bus" option for regmap API Andrey Smirnov
@ 2012-12-27 18:24   ` Mark Brown
  2012-12-29 19:18     ` Andrey Smirnov
  0 siblings, 1 reply; 7+ messages in thread
From: Mark Brown @ 2012-12-27 18:24 UTC (permalink / raw)
  To: Andrey Smirnov; +Cc: andrey.smirnov, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1678 bytes --]

On Fri, Dec 21, 2012 at 01:47:18AM -0800, Andrey Smirnov wrote:

This looks really good, the issues and questions I have below are pretty
detailed.

> -	int (*reg_read)(struct regmap *map, unsigned int reg, unsigned int *val);
> -	int (*reg_write)(struct regmap *map, unsigned int reg, unsigned int val);
> +	int (*reg_read)(void *context, unsigned int reg, unsigned int *val);
> +	int (*reg_write)(void *context, unsigned int reg, unsigned int val);

I'd be inclined to just do this in the initial refectoring patches
rather than rerefactoring here.

> +	if (!bus || !bus->fast_io) {
>  		mutex_init(&map->mutex);
>  		map->lock = regmap_lock_mutex;
>  		map->unlock = regmap_unlock_mutex;
> +	} else {
> +		spin_lock_init(&map->spinlock);
> +		map->lock = regmap_lock_spinlock;
> +		map->unlock = regmap_unlock_spinlock;

It's not immediately obvious to me that no-bus should be forced to use
mutexes - is there any great reason for tying the two together?  I'd add
a flag to allow no-bus devices to choose, possibly as part of a separate
"bus" configuration thing that gets configured with a separate init
function.

> +	if (!bus) {
> +		map->cache_registers = true;
> +		goto skip_format_initialization;
> +	} else {
> +		map->reg_read = _regmap_bus_read;
> +	}

Not sure I understand cache_registers here.  Why has this flag been
added?

> + * @reg_read: Optional callback that if filled will be used to perform
> + *            all the reads from the registers.
> + * @reg_write: Optional callback that if filled will be used to perform
> + *             all the writes to the registers.

I'd probably add some comment about not using this in conjunction with
SPI or I2C.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 3/3] Add "no-bus" option for regmap API
  2012-12-27 18:24   ` Mark Brown
@ 2012-12-29 19:18     ` Andrey Smirnov
  0 siblings, 0 replies; 7+ messages in thread
From: Andrey Smirnov @ 2012-12-29 19:18 UTC (permalink / raw)
  To: Mark Brown; +Cc: andrey.smirnov@convergeddevices.net, linux-kernel

>> -     int (*reg_read)(struct regmap *map, unsigned int reg, unsigned int *val);
>> -     int (*reg_write)(struct regmap *map, unsigned int reg, unsigned int val);
>> +     int (*reg_read)(void *context, unsigned int reg, unsigned int *val);
>> +     int (*reg_write)(void *context, unsigned int reg, unsigned int val);
>
> I'd be inclined to just do this in the initial refectoring patches
> rather than rerefactoring here.

I agree. I realized that I would have to have those parameters as
"void *" only after I made first two commits and since I knew that the
first version of the patch would have other problems decided to
postpone fixing this till the second version of the patch.

>
>> +     if (!bus || !bus->fast_io) {
>>               mutex_init(&map->mutex);
>>               map->lock = regmap_lock_mutex;
>>               map->unlock = regmap_unlock_mutex;
>> +     } else {
>> +             spin_lock_init(&map->spinlock);
>> +             map->lock = regmap_lock_spinlock;
>> +             map->unlock = regmap_unlock_spinlock;
>
> It's not immediately obvious to me that no-bus should be forced to use
> mutexes - is there any great reason for tying the two together?

No reason. I'll add provisions for configuring.

>
>> +     if (!bus) {
>> +             map->cache_registers = true;
>> +             goto skip_format_initialization;
>> +     } else {
>> +             map->reg_read = _regmap_bus_read;
>> +     }
>
> Not sure I understand cache_registers here.  Why has this flag been
> added?

As of now when the write operation is performed("_regmap_write" is
being called) the caching is handled either in "_regmap_write" or in
"_regmap_raw_write". The decision which one of the two places it will
be done at is made based on the availability of the
"map->format.format_write". With the addition of "no-bus"
configuration "map->format.format_write" is no longer a valid
criterion for that decision and since caching writes makes sense for
"no-bus" configuration too, I added this variable to serve as flag so
that caching would be handled in "_regmap_write" for "no-bus"
configuration and for busses with "map->format.format_write" and in
"_regmap_raw_write" for all the other cases.

>> + * @reg_read: Optional callback that if filled will be used to perform
>> + *            all the reads from the registers.
>> + * @reg_write: Optional callback that if filled will be used to perform
>> + *             all the writes to the registers.
>
> I'd probably add some comment about not using this in conjunction with
> SPI or I2C.

Will do.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 0/3] Add "no-bus" configuration for regmap API
  2012-12-21  9:47 [PATCH 0/3] Add "no-bus" configuration for regmap API Andrey Smirnov
                   ` (2 preceding siblings ...)
  2012-12-21  9:47 ` [PATCH 3/3] Add "no-bus" option for regmap API Andrey Smirnov
@ 2012-12-27 17:41 ` Mark Brown
  3 siblings, 0 replies; 7+ messages in thread
From: Mark Brown @ 2012-12-27 17:41 UTC (permalink / raw)
  To: Andrey Smirnov; +Cc: andrey.smirnov, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 383 bytes --]

On Fri, Dec 21, 2012 at 01:47:15AM -0800, Andrey Smirnov wrote:
> This patch proposes extension to the API that was suggested by Mark in
> the following thread:
> http://article.gmane.org/gmane.linux.kernel/1383734

This needs rebasing against v3.8-rc1.  Please also add regmap: to the
subject line - missing that means that things are more likely to get
missed or looked at slowly.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2012-12-29 19:25 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-12-21  9:47 [PATCH 0/3] Add "no-bus" configuration for regmap API Andrey Smirnov
2012-12-21  9:47 ` [PATCH 1/3] Add provisions to have user-defined read operation Andrey Smirnov
2012-12-21  9:47 ` [PATCH 2/3] Add provisions to have user-defined write operation Andrey Smirnov
2012-12-21  9:47 ` [PATCH 3/3] Add "no-bus" option for regmap API Andrey Smirnov
2012-12-27 18:24   ` Mark Brown
2012-12-29 19:18     ` Andrey Smirnov
2012-12-27 17:41 ` [PATCH 0/3] Add "no-bus" configuration " Mark Brown

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.