* [PATCH V1] regmap: Support for caching in reg_raw_write()
@ 2012-02-10 11:32 Laxman Dewangan
2012-02-10 11:45 ` Mark Brown
2012-02-10 12:01 ` Lars-Peter Clausen
0 siblings, 2 replies; 7+ messages in thread
From: Laxman Dewangan @ 2012-02-10 11:32 UTC (permalink / raw)
To: broonie, gregkh, linux-kernel; +Cc: linux-tegra, ldewangan
Adding support for caching of data into the
non-volatile register from the call of reg_raw_write().
This will allow the larger block of data write into multiple
register without worrying whether register is cached or not
through reg_raw_write().
Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
drivers/base/regmap/regmap.c | 25 ++++++++++++++++++++-----
1 files changed, 20 insertions(+), 5 deletions(-)
diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index 80129c0..e5986ab 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -323,6 +323,25 @@ static int _regmap_raw_write(struct regmap *map, unsigned int reg,
if (!map->writeable_reg(map->dev, reg + i))
return -EINVAL;
+ if (!map->cache_bypass && map->format.parse_val) {
+ int val_bytes = map->format.val_bytes;
+ unsigned int ival;
+ for (i = 0; i < val_len / map->format.val_bytes; i++) {
+ memcpy(map->work_buf, val + (i * val_bytes), val_bytes);
+ ival = map->format.parse_val(map->work_buf);
+ ret = regcache_write(map, reg + i, ival);
+ if (ret) {
+ dev_warn(map->dev,
+ "Error in caching of register\n");
+ return ret;
+ }
+ }
+ if (map->cache_only) {
+ map->cache_dirty = true;
+ return 0;
+ }
+ }
+
map->format.format_reg(map->work_buf, reg);
u8[0] |= map->write_flag_mask;
@@ -373,7 +392,7 @@ int _regmap_write(struct regmap *map, unsigned int reg,
int ret;
BUG_ON(!map->format.format_write && !map->format.format_val);
- if (!map->cache_bypass) {
+ if (!map->cache_bypass && map->format.format_write) {
ret = regcache_write(map, reg, val);
if (ret != 0)
return ret;
@@ -450,12 +469,8 @@ EXPORT_SYMBOL_GPL(regmap_write);
int regmap_raw_write(struct regmap *map, unsigned int reg,
const void *val, size_t val_len)
{
- size_t val_count = val_len / map->format.val_bytes;
int ret;
- WARN_ON(!regmap_volatile_range(map, reg, val_count) &&
- map->cache_type != REGCACHE_NONE);
-
mutex_lock(&map->lock);
ret = _regmap_raw_write(map, reg, val, val_len);
--
1.7.1.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH V1] regmap: Support for caching in reg_raw_write()
2012-02-10 11:32 [PATCH V1] regmap: Support for caching in reg_raw_write() Laxman Dewangan
@ 2012-02-10 11:45 ` Mark Brown
2012-02-10 12:01 ` Lars-Peter Clausen
1 sibling, 0 replies; 7+ messages in thread
From: Mark Brown @ 2012-02-10 11:45 UTC (permalink / raw)
To: Laxman Dewangan; +Cc: gregkh, linux-kernel, linux-tegra
[-- Attachment #1: Type: text/plain, Size: 811 bytes --]
On Fri, Feb 10, 2012 at 05:02:52PM +0530, Laxman Dewangan wrote:
This looks good, but could you make the few small improvements below
please?
> + if (!map->cache_bypass && map->format.parse_val) {
> + int val_bytes = map->format.val_bytes;
> + unsigned int ival;
> + for (i = 0; i < val_len / map->format.val_bytes; i++) {
A bit nit picky but since you've got a local val_bytes you may as well
use it.
> + memcpy(map->work_buf, val + (i * val_bytes), val_bytes);
> + ival = map->format.parse_val(map->work_buf);
> + ret = regcache_write(map, reg + i, ival);
> + if (ret) {
> + dev_warn(map->dev,
> + "Error in caching of register\n");
Say which register and the return value here - knowing the register
might be very helpful for debug. This should probably also be at least
a dev_err().
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH V1] regmap: Support for caching in reg_raw_write()
2012-02-10 11:32 [PATCH V1] regmap: Support for caching in reg_raw_write() Laxman Dewangan
2012-02-10 11:45 ` Mark Brown
@ 2012-02-10 12:01 ` Lars-Peter Clausen
2012-02-10 12:07 ` Laxman Dewangan
2012-02-10 12:14 ` Mark Brown
1 sibling, 2 replies; 7+ messages in thread
From: Lars-Peter Clausen @ 2012-02-10 12:01 UTC (permalink / raw)
To: Laxman Dewangan; +Cc: broonie, gregkh, linux-kernel, linux-tegra
On 02/10/2012 12:32 PM, Laxman Dewangan wrote:
> Adding support for caching of data into the
> non-volatile register from the call of reg_raw_write().
>
> This will allow the larger block of data write into multiple
> register without worrying whether register is cached or not
> through reg_raw_write().
>
In my opinion it makes more sense to introduce a regmap_bulk_write function
for this. regmap_raw_write was not really meant to write registers but
rather binary blobs, like firmware. Also this keeps things consistent with
the read part of the regmap API.
> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
> ---
> drivers/base/regmap/regmap.c | 25 ++++++++++++++++++++-----
> 1 files changed, 20 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
> index 80129c0..e5986ab 100644
> --- a/drivers/base/regmap/regmap.c
> +++ b/drivers/base/regmap/regmap.c
> @@ -323,6 +323,25 @@ static int _regmap_raw_write(struct regmap *map, unsigned int reg,
> if (!map->writeable_reg(map->dev, reg + i))
> return -EINVAL;
>
> + if (!map->cache_bypass && map->format.parse_val) {
> + int val_bytes = map->format.val_bytes;
> + unsigned int ival;
> + for (i = 0; i < val_len / map->format.val_bytes; i++) {
> + memcpy(map->work_buf, val + (i * val_bytes), val_bytes);
> + ival = map->format.parse_val(map->work_buf);
> + ret = regcache_write(map, reg + i, ival);
> + if (ret) {
> + dev_warn(map->dev,
> + "Error in caching of register\n");
> + return ret;
> + }
> + }
> + if (map->cache_only) {
> + map->cache_dirty = true;
> + return 0;
> + }
> + }
> +
> map->format.format_reg(map->work_buf, reg);
>
> u8[0] |= map->write_flag_mask;
> @@ -373,7 +392,7 @@ int _regmap_write(struct regmap *map, unsigned int reg,
> int ret;
> BUG_ON(!map->format.format_write && !map->format.format_val);
>
> - if (!map->cache_bypass) {
> + if (!map->cache_bypass && map->format.format_write) {
> ret = regcache_write(map, reg, val);
> if (ret != 0)
> return ret;
> @@ -450,12 +469,8 @@ EXPORT_SYMBOL_GPL(regmap_write);
> int regmap_raw_write(struct regmap *map, unsigned int reg,
> const void *val, size_t val_len)
> {
> - size_t val_count = val_len / map->format.val_bytes;
> int ret;
>
> - WARN_ON(!regmap_volatile_range(map, reg, val_count) &&
> - map->cache_type != REGCACHE_NONE);
> -
> mutex_lock(&map->lock);
>
> ret = _regmap_raw_write(map, reg, val, val_len);
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH V1] regmap: Support for caching in reg_raw_write()
2012-02-10 12:01 ` Lars-Peter Clausen
@ 2012-02-10 12:07 ` Laxman Dewangan
2012-02-10 12:14 ` Mark Brown
1 sibling, 0 replies; 7+ messages in thread
From: Laxman Dewangan @ 2012-02-10 12:07 UTC (permalink / raw)
To: Lars-Peter Clausen; +Cc: broonie, gregkh, linux-kernel, linux-tegra
On Friday 10 February 2012 05:31 PM, Lars-Peter Clausen wrote:
> On 02/10/2012 12:32 PM, Laxman Dewangan wrote:
>> Adding support for caching of data into the
>> non-volatile register from the call of reg_raw_write().
>>
>> This will allow the larger block of data write into multiple
>> register without worrying whether register is cached or not
>> through reg_raw_write().
>>
> In my opinion it makes more sense to introduce a regmap_bulk_write function
> for this.
Yes, I am going to push another change which will have the
regmap_bulk_write() which will be in sync with read part.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH V1] regmap: Support for caching in reg_raw_write()
2012-02-10 12:01 ` Lars-Peter Clausen
2012-02-10 12:07 ` Laxman Dewangan
@ 2012-02-10 12:14 ` Mark Brown
2012-02-10 12:33 ` Lars-Peter Clausen
1 sibling, 1 reply; 7+ messages in thread
From: Mark Brown @ 2012-02-10 12:14 UTC (permalink / raw)
To: Lars-Peter Clausen; +Cc: Laxman Dewangan, gregkh, linux-kernel, linux-tegra
[-- Attachment #1: Type: text/plain, Size: 886 bytes --]
On Fri, Feb 10, 2012 at 01:01:26PM +0100, Lars-Peter Clausen wrote:
> In my opinion it makes more sense to introduce a regmap_bulk_write function
> for this. regmap_raw_write was not really meant to write registers but
This isn't exclusive to that, I think Laxman planned to send a patch
doing that on top of this one which would be what most users would end
up using.
> rather binary blobs, like firmware. Also this keeps things consistent with
> the read part of the regmap API.
See the previous discussion on this in the past day or so - bulk_write()
is more complicated to implement by itself since it's going to end up
boling down to a raw_write() internally anyway (as does reg_write()) and
it seems nicer to just do the right thing if people ask for it.
We can always refactor later to improve performance if needed but this
is a simple starting point.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH V1] regmap: Support for caching in reg_raw_write()
2012-02-10 12:14 ` Mark Brown
@ 2012-02-10 12:33 ` Lars-Peter Clausen
2012-02-10 15:58 ` Mark Brown
0 siblings, 1 reply; 7+ messages in thread
From: Lars-Peter Clausen @ 2012-02-10 12:33 UTC (permalink / raw)
To: Mark Brown; +Cc: Laxman Dewangan, gregkh, linux-kernel, linux-tegra
On 02/10/2012 01:14 PM, Mark Brown wrote:
> On Fri, Feb 10, 2012 at 01:01:26PM +0100, Lars-Peter Clausen wrote:
>
>> In my opinion it makes more sense to introduce a regmap_bulk_write function
>> for this. regmap_raw_write was not really meant to write registers but
>
> This isn't exclusive to that, I think Laxman planned to send a patch
> doing that on top of this one which would be what most users would end
> up using.
>
>> rather binary blobs, like firmware. Also this keeps things consistent with
>> the read part of the regmap API.
>
> See the previous discussion on this in the past day or so - bulk_write()
> is more complicated to implement by itself since it's going to end up
> boling down to a raw_write() internally anyway (as does reg_write()) and
> it seems nicer to just do the right thing if people ask for it.
>
regmap_write doesn't always go the raw_write path. Also with this approach
we end up formatting the value into the raw format, only to parse it again
in the next step.
Furthermore I don't think it makes sense to cache raw values as the cache
operates on a register level, not on a byte level.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH V1] regmap: Support for caching in reg_raw_write()
2012-02-10 12:33 ` Lars-Peter Clausen
@ 2012-02-10 15:58 ` Mark Brown
0 siblings, 0 replies; 7+ messages in thread
From: Mark Brown @ 2012-02-10 15:58 UTC (permalink / raw)
To: Lars-Peter Clausen; +Cc: Laxman Dewangan, gregkh, linux-kernel, linux-tegra
[-- Attachment #1: Type: text/plain, Size: 1031 bytes --]
On Fri, Feb 10, 2012 at 01:33:16PM +0100, Lars-Peter Clausen wrote:
> regmap_write doesn't always go the raw_write path. Also with this approach
We can ignore the non-byte formats as a wart for this; they're cut out
of this area of the subsystem functionality and just get single register
write operatons with single register cache access.
> we end up formatting the value into the raw format, only to parse it again
> in the next step.
Yes, like I say there's room for optimisation. For 8 bit values there
would be win from working out that we don't need to do anything at all,
though I doubt it's ever going to be performance critical.
> Furthermore I don't think it makes sense to cache raw values as the cache
> operates on a register level, not on a byte level.
The entire API operates on registers really. The only reason
raw_write() takes a length is because it's what most of the users are
actually likely to have to hand, trying to write to less than a register
isn't particularly sensible.
[-- 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-02-10 15:58 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-02-10 11:32 [PATCH V1] regmap: Support for caching in reg_raw_write() Laxman Dewangan
2012-02-10 11:45 ` Mark Brown
2012-02-10 12:01 ` Lars-Peter Clausen
2012-02-10 12:07 ` Laxman Dewangan
2012-02-10 12:14 ` Mark Brown
2012-02-10 12:33 ` Lars-Peter Clausen
2012-02-10 15:58 ` 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).