[PATCH 2/4] regmap: Make internal read/write functions reentrant from themselves.

[PATCH 2/4] regmap: Make internal read/write functions reentrant from themselves.

[Krystian Garbaciak]

Functions _regmap_update_bits() and _regmap_write() are modified
so they can be recurrently entered from the inside.

The internal reading functions need no modification for current implementation
of indirect access.

Signed-off-by: Krystian Garbaciak <krystian.garbaciak@diasemi.com>
---
 drivers/base/regmap/regmap.c |   30 ++++++++++++++++--------------
 1 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index a365aa8..7c5291e 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -515,12 +515,9 @@ int _regmap_write(struct regmap *map, unsigned int reg,
 
 		return ret;
 	} else {
-		map->format.format_val(map->work_buf + map->format.reg_bytes
-				       + map->format.pad_bytes, val);
-		return _regmap_raw_write(map, reg,
-					 map->work_buf +
-					 map->format.reg_bytes +
-					 map->format.pad_bytes,
+		/* Using stack for value data, to make function reentrant */
+		map->format.format_val(&val, val);
+		return _regmap_raw_write(map, reg, &val,
 					 map->format.val_bytes);
 	}
 }
@@ -810,11 +807,9 @@ static int _regmap_update_bits(struct regmap *map, unsigned int reg,
 	int ret;
 	unsigned int tmp, orig;
 
-	mutex_lock(&map->lock);
-
 	ret = _regmap_read(map, reg, &orig);
 	if (ret != 0)
-		goto out;
+		return ret;
 
 	tmp = orig & ~mask;
 	tmp |= val & mask;
@@ -826,9 +821,6 @@ static int _regmap_update_bits(struct regmap *map, unsigned int reg,
 		*change = false;
 	}
 
-out:
-	mutex_unlock(&map->lock);
-
 	return ret;
 }
 
@@ -846,7 +838,12 @@ int regmap_update_bits(struct regmap *map, unsigned int reg,
 		       unsigned int mask, unsigned int val)
 {
 	bool change;
-	return _regmap_update_bits(map, reg, mask, val, &change);
+	int ret;
+
+	mutex_lock(&map->lock);
+	ret = _regmap_update_bits(map, reg, mask, val, &change);
+	mutex_unlock(&map->lock);
+	return ret;
 }
 EXPORT_SYMBOL_GPL(regmap_update_bits);
 
@@ -866,7 +863,12 @@ int regmap_update_bits_check(struct regmap *map, unsigned int reg,
 			     unsigned int mask, unsigned int val,
 			     bool *change)
 {
-	return _regmap_update_bits(map, reg, mask, val, change);
+	int ret;
+
+	mutex_lock(&map->lock);
+	ret = _regmap_update_bits(map, reg, mask, val, change);
+	mutex_unlock(&map->lock);
+	return ret;
 }
 EXPORT_SYMBOL_GPL(regmap_update_bits_check);
 
-- 
1.7.0.4

Re: [PATCH 2/4] regmap: Make internal read/write functions reentrant from themselves.

[Mark Brown]

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

On Thu, May 31, 2012 at 04:19:36PM +0200, Krystian Garbaciak wrote:

> Functions _regmap_update_bits() and _regmap_write() are modified
> so they can be recurrently entered from the inside.

You should describe what you're doing here in more detail...

> The internal reading functions need no modification for current implementation
> of indirect access.

So the subject should really be "Make internal write functions
reentrant"?

> -		map->format.format_val(map->work_buf + map->format.reg_bytes
> -				       + map->format.pad_bytes, val);
> -		return _regmap_raw_write(map, reg,
> -					 map->work_buf +
> -					 map->format.reg_bytes +
> -					 map->format.pad_bytes,
> +		/* Using stack for value data, to make function reentrant */
> +		map->format.format_val(&val, val);
> +		return _regmap_raw_write(map, reg, &val,

We can't safely do this for all buses, some want to DMA and you can't
DMA from stack.  This also means that we'll no longer be able to send a
single buffer to the device as we'll never have the value immediately
following the register address and padding any more.  That'll increase
overhead on many systems, you can often see the handover on gathers on a
scope.

I don't understand why we can't take the decision to write the page
register before we start using the buffer?

Also, it seems like your mailing setup is a bit broken - these messages
aren't being threaded with each other.

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

RE: [PATCH 2/4] regmap: Make internal read/write functions reentrant from themselves.

[Krystian Garbaciak]

> > The internal reading functions need no modification for current
> > implementation of indirect access.
> 
> So the subject should really be "Make internal write functions reentrant"?

Yes, it can be.

> We can't safely do this for all buses, some want to DMA and you can't DMA
> from stack.  This also means that we'll no longer be able to send a single buffer
> to the device as we'll never have the value immediately following the register
> address and padding any more.  That'll increase overhead on many systems, you
> can often see the handover on gathers on a scope.
> 
> I don't understand why we can't take the decision to write the page register
> before we start using the buffer?

I will try to solve this on next patch revision.

> Also, it seems like your mailing setup is a bit broken - these messages aren't
> being threaded with each other.

I've sent it like a bunch of new patches with different numbering, if that might be the reason.
Legal Disclaimer: This e-mail communication (and any attachment/s) is confidential and contains proprietary information, 
some or all of which may be legally privileged. It is intended solely for the use of the individual or entity to which it
is addressed. Access to this email by anyone else is unauthorized. If you are not the intended recipient, any disclosure, 
copying, distribution or any action taken or omitted to be taken in reliance on it, is prohibited and may be unlawful.

Please consider the environment before printing this e-mail