linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] [regmap] [RESEND] Add "no-bus" configuration for regmap API
@ 2013-01-12 20:54 Andrey Smirnov
  2013-01-12 20:54 ` [PATCH 1/3] [regmap] [RESEND] Add provisions to have user-defined read operation Andrey Smirnov
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Andrey Smirnov @ 2013-01-12 20:54 UTC (permalink / raw)
  To: andrey.smirnov; +Cc: broonie, linux-kernel

Second version of the initial patchset(can be found here:
https://lkml.org/lkml/2012/12/21/98)

This version contains following improvements:
 - Provisions to switch between mutexes and spinlocks for locking in
   "no-bus" configuration
 - "reg_read" and "reg_write" have "void *context" as a first argument
   in all the patches
 - Minor improvements in documentation

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         |   18 ++++-
 3 files changed, 135 insertions(+), 44 deletions(-)

-- 
1.7.9.5


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

* [PATCH 1/3] [regmap] [RESEND] Add provisions to have user-defined read operation
  2013-01-12 20:54 [PATCH 0/3] [regmap] [RESEND] Add "no-bus" configuration for regmap API Andrey Smirnov
@ 2013-01-12 20:54 ` Andrey Smirnov
  2013-01-13 23:04   ` Mark Brown
  2013-01-12 20:54 ` [PATCH 2/3] [regmap] [RESEND] Add provisions to have user-defined write operation Andrey Smirnov
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 10+ messages in thread
From: Andrey Smirnov @ 2013-01-12 20:54 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   |   35 ++++++++++++++++++++++++++---------
 2 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/drivers/base/regmap/internal.h b/drivers/base/regmap/internal.h
index 401d191..471eb90 100644
--- a/drivers/base/regmap/internal.h
+++ b/drivers/base/regmap/internal.h
@@ -74,6 +74,8 @@ struct regmap {
 	const struct regmap_access_table *volatile_table;
 	const struct regmap_access_table *precious_table;
 
+	int (*reg_read)(void *context, 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 42d5cb0..ceaefcf 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(void *context, unsigned int reg,
+			    unsigned int *val);
+
 bool regmap_reg_in_ranges(unsigned int reg,
 			  const struct regmap_range *ranges,
 			  unsigned int nranges)
@@ -430,6 +433,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;
@@ -1202,10 +1207,27 @@ static int _regmap_raw_read(struct regmap *map, unsigned int reg, void *val,
 	return ret;
 }
 
+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;
+
+	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);
@@ -1213,26 +1235,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] 10+ messages in thread

* [PATCH 2/3] [regmap] [RESEND] Add provisions to have user-defined write operation
  2013-01-12 20:54 [PATCH 0/3] [regmap] [RESEND] Add "no-bus" configuration for regmap API Andrey Smirnov
  2013-01-12 20:54 ` [PATCH 1/3] [regmap] [RESEND] Add provisions to have user-defined read operation Andrey Smirnov
@ 2013-01-12 20:54 ` Andrey Smirnov
  2013-01-13 23:04   ` Mark Brown
  2013-01-12 20:54 ` [PATCH 3/3] [regmap] [RESEND] Add "no-bus" option for regmap API Andrey Smirnov
  2013-01-13 12:03 ` [PATCH 0/3] [regmap] [RESEND] Add "no-bus" configuration " Mark Brown
  3 siblings, 1 reply; 10+ messages in thread
From: Andrey Smirnov @ 2013-01-12 20:54 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   |   83 ++++++++++++++++++++++++++--------------
 2 files changed, 55 insertions(+), 29 deletions(-)

diff --git a/drivers/base/regmap/internal.h b/drivers/base/regmap/internal.h
index 471eb90..51f0574 100644
--- a/drivers/base/regmap/internal.h
+++ b/drivers/base/regmap/internal.h
@@ -75,6 +75,7 @@ struct regmap {
 	const struct regmap_access_table *precious_table;
 
 	int (*reg_read)(void *context, unsigned int reg, unsigned int *val);
+	int (*reg_write)(void *context, 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 ceaefcf..6845a07 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -36,6 +36,10 @@ static int _regmap_update_bits(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(void *context, unsigned int reg,
+				       unsigned int val);
+static int _regmap_bus_raw_write(void *context, unsigned int reg,
+				 unsigned int val);
 
 bool regmap_reg_in_ranges(unsigned int reg,
 			  const struct regmap_range *ranges,
@@ -580,6 +584,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->num_ranges; i++) {
 		const struct regmap_range_cfg *range_cfg = &config->ranges[i];
@@ -986,12 +995,54 @@ static int _regmap_raw_write(struct regmap *map, unsigned int reg,
 	return ret;
 }
 
+static int _regmap_bus_formatted_write(void *context, unsigned int reg,
+				       unsigned int val)
+{
+	int ret;
+	struct regmap_range_node *range;
+	struct regmap *map = context;
+
+	BUG_ON(!map->format.format_write);
+
+	range = _regmap_range_lookup(map, reg);
+	if (range) {
+		ret = _regmap_select_page(map, &reg, range, 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(void *context, unsigned int reg,
+				 unsigned int val)
+{
+	struct regmap *map = context;
+
+	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)
 {
-	struct regmap_range_node *range;
 	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);
@@ -1010,33 +1061,7 @@ int _regmap_write(struct regmap *map, unsigned int reg,
 
 	trace_regmap_reg_write(map->dev, reg, val);
 
-	if (map->format.format_write) {
-		range = _regmap_range_lookup(map, reg);
-		if (range) {
-			ret = _regmap_select_page(map, &reg, range, 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] 10+ messages in thread

* [PATCH 3/3] [regmap] [RESEND] Add "no-bus" option for regmap API
  2013-01-12 20:54 [PATCH 0/3] [regmap] [RESEND] Add "no-bus" configuration for regmap API Andrey Smirnov
  2013-01-12 20:54 ` [PATCH 1/3] [regmap] [RESEND] Add provisions to have user-defined read operation Andrey Smirnov
  2013-01-12 20:54 ` [PATCH 2/3] [regmap] [RESEND] Add provisions to have user-defined write operation Andrey Smirnov
@ 2013-01-12 20:54 ` Andrey Smirnov
  2013-01-13 23:18   ` Mark Brown
  2013-01-13 12:03 ` [PATCH 0/3] [regmap] [RESEND] Add "no-bus" configuration " Mark Brown
  3 siblings, 1 reply; 10+ messages in thread
From: Andrey Smirnov @ 2013-01-12 20:54 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 |    2 ++
 drivers/base/regmap/regmap.c   |   52 ++++++++++++++++++++++++++++++----------
 include/linux/regmap.h         |   18 +++++++++++++-
 3 files changed, 59 insertions(+), 13 deletions(-)

diff --git a/drivers/base/regmap/internal.h b/drivers/base/regmap/internal.h
index 51f0574..3b1f74b 100644
--- a/drivers/base/regmap/internal.h
+++ b/drivers/base/regmap/internal.h
@@ -77,6 +77,8 @@ struct regmap {
 	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 6845a07..4f7fcec 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -379,7 +379,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);
@@ -393,7 +393,8 @@ struct regmap *regmap_init(struct device *dev,
 		map->unlock = config->unlock;
 		map->lock_arg = config->lock_arg;
 	} else {
-		if (bus->fast_io) {
+		if ((bus && bus->fast_io) ||
+		    config->fast_io) {
 			spin_lock_init(&map->spinlock);
 			map->lock = regmap_lock_spinlock;
 			map->unlock = regmap_unlock_spinlock;
@@ -433,11 +434,19 @@ struct regmap *regmap_init(struct device *dev,
 	if (config->read_flag_mask || config->write_flag_mask) {
 		map->read_flag_mask = config->read_flag_mask;
 		map->write_flag_mask = config->write_flag_mask;
-	} else {
+	} else if (bus) {
 		map->read_flag_mask = bus->read_flag_mask;
 	}
 
-	map->reg_read = _regmap_bus_read;
+	if (!bus) {
+		map->reg_read  = config->reg_read;
+		map->reg_write = config->reg_write;
+
+		map->cache_registers = true;
+		goto skip_format_initialization;
+	} else {
+		map->reg_read  = _regmap_bus_read;
+	}
 
 	reg_endian = config->reg_format_endian;
 	if (reg_endian == REGMAP_ENDIAN_DEFAULT)
@@ -584,10 +593,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->num_ranges; i++) {
@@ -790,7 +803,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);
@@ -893,6 +906,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++)
@@ -1002,7 +1017,7 @@ static int _regmap_bus_formatted_write(void *context, unsigned int reg,
 	struct regmap_range_node *range;
 	struct regmap *map = context;
 
-	BUG_ON(!map->format.format_write);
+	BUG_ON(!map->bus || !map->format.format_write);
 
 	range = _regmap_range_lookup(map, reg);
 	if (range) {
@@ -1028,7 +1043,7 @@ static int _regmap_bus_raw_write(void *context, unsigned int reg,
 {
 	struct regmap *map = context;
 
-	BUG_ON(!map->format.format_val);
+	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);
@@ -1043,8 +1058,9 @@ int _regmap_write(struct regmap *map, unsigned int reg,
 		  unsigned int val)
 {
 	int ret;
+	void *context = (map->bus) ? map : map->bus_context;
 
-	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;
@@ -1061,7 +1077,7 @@ int _regmap_write(struct regmap *map, unsigned int reg,
 
 	trace_regmap_reg_write(map->dev, reg, val);
 
-	return map->reg_write(map, reg, val);
+	return map->reg_write(context, reg, val);
 }
 
 /**
@@ -1112,6 +1128,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)
@@ -1148,6 +1166,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)
@@ -1201,6 +1221,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);
+
 	range = _regmap_range_lookup(map, reg);
 	if (range) {
 		ret = _regmap_select_page(map, &reg, range,
@@ -1252,6 +1274,8 @@ static int _regmap_read(struct regmap *map, unsigned int reg,
 			unsigned int *val)
 {
 	int ret;
+	void *context = (map->bus) ? map : map->bus_context;
+
 	BUG_ON(!map->reg_read);
 
 	if (!map->cache_bypass) {
@@ -1263,7 +1287,7 @@ static int _regmap_read(struct regmap *map, unsigned int reg,
 	if (map->cache_only)
 		return -EBUSY;
 
-	ret = map->reg_read(map, reg, val);
+	ret = map->reg_read(context, reg, val);
 	if (ret == 0) {
 #ifdef LOG_DEVICE
 		if (strcmp(dev_name(map->dev), LOG_DEVICE) == 0)
@@ -1325,6 +1349,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)
@@ -1376,6 +1402,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 b7e95bf..28dde94 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -127,7 +127,18 @@ typedef void (*regmap_unlock)(void *);
  * @lock_arg:	  this field is passed as the only argument of lock/unlock
  *		  functions (ignored in case regular lock/unlock functions
  *		  are not overridden).
- *
+ * @reg_read:	  Optional callback that if filled will be used to perform
+ *           	  all the reads from the registers. Should only be provided for
+ *		  devices whos read operation cannot be represented as a simple read
+ *		  operation on a bus such as SPI, I2C, etc. Most of the devices do
+ * 		  not need this.
+ * @reg_write:	  Same as above for writing.
+ * @fast_io:	  Register IO is fast. Use a spinlock instead of a mutex
+ *	     	  to perform locking. This field is ignored if custom lock/unlock
+ *	     	  functions are used (see fields lock/unlock of struct regmap_config).
+ *		  This field is a duplicate of a similar file in
+ *		  'struct regmap_bus' and serves exact same purpose.
+ *		   Use it only for "no-bus" cases.
  * @max_register: Optional, specifies the maximum valid register index.
  * @wr_table:     Optional, points to a struct regmap_access_table specifying
  *                valid ranges for write access.
@@ -177,6 +188,11 @@ struct regmap_config {
 	regmap_unlock unlock;
 	void *lock_arg;
 
+	int (*reg_read)(void *context, unsigned int reg, unsigned int *val);
+	int (*reg_write)(void *context, unsigned int reg, unsigned int val);
+
+	bool fast_io;
+
 	unsigned int max_register;
 	const struct regmap_access_table *wr_table;
 	const struct regmap_access_table *rd_table;
-- 
1.7.9.5


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

* Re: [PATCH 0/3] [regmap] [RESEND] Add "no-bus" configuration for regmap API
  2013-01-12 20:54 [PATCH 0/3] [regmap] [RESEND] Add "no-bus" configuration for regmap API Andrey Smirnov
                   ` (2 preceding siblings ...)
  2013-01-12 20:54 ` [PATCH 3/3] [regmap] [RESEND] Add "no-bus" option for regmap API Andrey Smirnov
@ 2013-01-13 12:03 ` Mark Brown
  3 siblings, 0 replies; 10+ messages in thread
From: Mark Brown @ 2013-01-13 12:03 UTC (permalink / raw)
  To: Andrey Smirnov; +Cc: andrey.smirnov, linux-kernel

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

On Sat, Jan 12, 2013 at 12:54:11PM -0800, Andrey Smirnov wrote:
> Second version of the initial patchset(can be found here:
> https://lkml.org/lkml/2012/12/21/98)

Please do use subject lines appropriate for the subsystem - in general,
if your commit logs look different to others for the same subsystem
there's probably an issue.  Not doing this makes it easier for things to
get missed.  Here you want "regmap: "...

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

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

* Re: [PATCH 1/3] [regmap] [RESEND] Add provisions to have user-defined read operation
  2013-01-12 20:54 ` [PATCH 1/3] [regmap] [RESEND] Add provisions to have user-defined read operation Andrey Smirnov
@ 2013-01-13 23:04   ` Mark Brown
  0 siblings, 0 replies; 10+ messages in thread
From: Mark Brown @ 2013-01-13 23:04 UTC (permalink / raw)
  To: Andrey Smirnov; +Cc: andrey.smirnov, linux-kernel

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

On Sat, Jan 12, 2013 at 12:54:12PM -0800, Andrey Smirnov wrote:

> 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.

Applied, thanks.

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

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

* Re: [PATCH 2/3] [regmap] [RESEND] Add provisions to have user-defined write operation
  2013-01-12 20:54 ` [PATCH 2/3] [regmap] [RESEND] Add provisions to have user-defined write operation Andrey Smirnov
@ 2013-01-13 23:04   ` Mark Brown
  0 siblings, 0 replies; 10+ messages in thread
From: Mark Brown @ 2013-01-13 23:04 UTC (permalink / raw)
  To: Andrey Smirnov; +Cc: andrey.smirnov, linux-kernel

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

On Sat, Jan 12, 2013 at 12:54:13PM -0800, Andrey Smirnov wrote:
> 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.

Applied, thanks.

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

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

* Re: [PATCH 3/3] [regmap] [RESEND] Add "no-bus" option for regmap API
  2013-01-12 20:54 ` [PATCH 3/3] [regmap] [RESEND] Add "no-bus" option for regmap API Andrey Smirnov
@ 2013-01-13 23:18   ` Mark Brown
  2013-01-14 17:08     ` Andrey Smirnov
  0 siblings, 1 reply; 10+ messages in thread
From: Mark Brown @ 2013-01-13 23:18 UTC (permalink / raw)
  To: Andrey Smirnov; +Cc: andrey.smirnov, linux-kernel

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

On Sat, Jan 12, 2013 at 12:54:14PM -0800, Andrey Smirnov wrote:

> +	bool cache_registers;
> +

I'm afraid I don't quite understand this...

> -	if (!map->cache_bypass && map->format.format_write) {
> +	if (!map->cache_bypass && map->cache_registers) {
>  		ret = regcache_write(map, reg, val);

...I think it's mostly to service this check here, but looking at the
code I can't quite think why the code is doing what it's doing - I
suspect we should just remove the check for format_write() here.  I
think it was there to support potential bulk writes from the cache code
(which we don't do yet) but we're not actually doing those and it's not
clear that this should be doing that anyway.

>  	int ret;
> +	void *context = (map->bus) ? map : map->bus_context;
> +

Can you please make this a static inline regmap_map_get_context() or
something?  The same thing appears in quite a few places and the terery
operator isn't great at the best of times.

Otherwise this looks good, thanks a lot for doing this.

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

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

* Re: [PATCH 3/3] [regmap] [RESEND] Add "no-bus" option for regmap API
  2013-01-13 23:18   ` Mark Brown
@ 2013-01-14 17:08     ` Andrey Smirnov
  2013-01-15  7:02       ` Mark Brown
  0 siblings, 1 reply; 10+ messages in thread
From: Andrey Smirnov @ 2013-01-14 17:08 UTC (permalink / raw)
  To: Mark Brown; +Cc: andrey.smirnov@convergeddevices.net, linux-kernel

On Sun, Jan 13, 2013 at 3:18 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Sat, Jan 12, 2013 at 12:54:14PM -0800, Andrey Smirnov wrote:
>
>> +     bool cache_registers;
>> +
>
> I'm afraid I don't quite understand this...
>
>> -     if (!map->cache_bypass && map->format.format_write) {
>> +     if (!map->cache_bypass && map->cache_registers) {
>>               ret = regcache_write(map, reg, val);
>
> ...I think it's mostly to service this check here, but looking at the
> code I can't quite think why the code is doing what it's doing - I
> suspect we should just remove the check for format_write() here.  I
> think it was there to support potential bulk writes from the cache code
> (which we don't do yet) but we're not actually doing those and it's not
> clear that this should be doing that anyway.
>

>From my understanding of the code it is done because the caching is
handled differently for cases when format_write() and format_reg(),
format_val() are provided.
In case of 'format_write' the regcache_write() is called in
_regmap_write() directly whereas when format_reg(), format_val() are
there _regmap_write() calls _regmap_raw_write() which in turn calls
regcache_write(). If I remove that variable and corresponding check
then regcache_write() would be called twice in the case of
format_reg(), format_val(), when _regmap_write() is called, would it
not? I apologise if I miss something obvious and that is not a case(or
issue).

>>       int ret;
>> +     void *context = (map->bus) ? map : map->bus_context;
>> +
>
> Can you please make this a static inline regmap_map_get_context() or
> something?  The same thing appears in quite a few places and the terery
> operator isn't great at the best of times.
>

Will do in the next version of this patch.


Completely unrelated off-topic question: the following code in
regmap_bulk_write()

...

if (map->use_single_rw) {
    for (i = 0; i < val_count; i++) {
        ret = regmap_raw_write(map,
                                           reg + (i * map->reg_stride),
                                           val + (i * val_bytes),
                                           val_bytes);
        if (ret != 0)
            return ret;
    }
}

....

Would it not deadlock, since both regmap_bulk_write() and
regmap_raw_write() call map->lock(...)?
Once again, sorry for off-topic, I just didn't want to start a new
thread because of what very well may by my poor understanding of the
code(I'll submit a patch to change regmap_raw_write() to
_regmap_raw_write() If I am correct).

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

* Re: [PATCH 3/3] [regmap] [RESEND] Add "no-bus" option for regmap API
  2013-01-14 17:08     ` Andrey Smirnov
@ 2013-01-15  7:02       ` Mark Brown
  0 siblings, 0 replies; 10+ messages in thread
From: Mark Brown @ 2013-01-15  7:02 UTC (permalink / raw)
  To: Andrey Smirnov; +Cc: andrey.smirnov@convergeddevices.net, linux-kernel

On Mon, Jan 14, 2013 at 09:08:27AM -0800, Andrey Smirnov wrote:
> On Sun, Jan 13, 2013 at 3:18 PM, Mark Brown
> > On Sat, Jan 12, 2013 at 12:54:14PM -0800, Andrey Smirnov wrote:

> >> +     bool cache_registers;

> > I'm afraid I don't quite understand this...

> From my understanding of the code it is done because the caching is
> handled differently for cases when format_write() and format_reg(),
> format_val() are provided.

> In case of 'format_write' the regcache_write() is called in
> _regmap_write() directly whereas when format_reg(), format_val() are
> there _regmap_write() calls _regmap_raw_write() which in turn calls
> regcache_write(). If I remove that variable and corresponding check
> then regcache_write() would be called twice in the case of
> format_reg(), format_val(), when _regmap_write() is called, would it
> not? I apologise if I miss something obvious and that is not a case(or
> issue).

OK, in this case the variable is confusingly named - it has no effect on
if we're going to cache, it's about where we cache.  What's really
driving the decision here is a combination of having block I/O support
(this was done this way to support cache for block writes) and having
the ability to read (which is what limits us).  Not sure I can think of
a good name right now though...

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

end of thread, other threads:[~2013-01-15  7:02 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-01-12 20:54 [PATCH 0/3] [regmap] [RESEND] Add "no-bus" configuration for regmap API Andrey Smirnov
2013-01-12 20:54 ` [PATCH 1/3] [regmap] [RESEND] Add provisions to have user-defined read operation Andrey Smirnov
2013-01-13 23:04   ` Mark Brown
2013-01-12 20:54 ` [PATCH 2/3] [regmap] [RESEND] Add provisions to have user-defined write operation Andrey Smirnov
2013-01-13 23:04   ` Mark Brown
2013-01-12 20:54 ` [PATCH 3/3] [regmap] [RESEND] Add "no-bus" option for regmap API Andrey Smirnov
2013-01-13 23:18   ` Mark Brown
2013-01-14 17:08     ` Andrey Smirnov
2013-01-15  7:02       ` Mark Brown
2013-01-13 12:03 ` [PATCH 0/3] [regmap] [RESEND] Add "no-bus" configuration " Mark Brown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).