Re: [PATCH 1/2] regmap: Allow installing custom reg_update_bits function

[Mark Brown]

On Thu, Oct 01, 2015 at 08:29:19AM -0400, Jon Ringle wrote:
> On Thu, 1 Oct 2015, Mark Brown wrote:

> > This completely bypasses and therefore breaks the cache infrastructure.

> Right after sending the v2 patch, I realized that calling the 
> custom reg_update_bits would only be applicable for registers that are 
> marked as volatile. Would the following solution be acceptable (it would 

Well, it should still *work* with a cache, though it's certainly true
that it's unlikely to have any performance benefit with cached register
since the read part of the read/modify/write cycle is essentially free 
with the cache.

> also simplify the regmap_update_bits in the encx24j600 driver):

>   if (regmap_volatile(map, reg) && map->reg_update_bits) {
>   return map->reg_update_bits(map->bus_context, reg, mask, 
>   val, change, force_write);

> The cache state should not matter for volatile registers, right?

Right.  I see you've sent a new patch already, I'll reply to that after
I've thought about it a little.


signature.asc
Description: Digital signature

Re: [PATCH 1/2] regmap: Allow installing custom reg_update_bits function

[Jon Ringle]

On Thu, 1 Oct 2015, Mark Brown wrote:

> On Thu, Oct 01, 2015 at 02:33:06AM -0400, j...@ringle.org wrote:
> 
> > @@ -2509,6 +2510,10 @@ static int _regmap_update_bits(struct regmap *map, 
> > unsigned int reg,
> > int ret;
> > unsigned int tmp, orig;
> >  
> > +   if (map->reg_update_bits)
> > +   return map->reg_update_bits(map->bus_context, reg, mask, val,
> > +   change, force_write);
> > +
> > ret = _regmap_read(map, reg, );
> > if (ret != 0)
> > return ret;
> 
> This completely bypasses and therefore breaks the cache infrastructure.
> 

Hi Mark,

Right after sending the v2 patch, I realized that calling the 
custom reg_update_bits would only be applicable for registers that are 
marked as volatile. Would the following solution be acceptable (it would 
also simplify the regmap_update_bits in the encx24j600 driver):

if (regmap_volatile(map, reg) && map->reg_update_bits) {
return map->reg_update_bits(map->bus_context, reg, mask, 
val, change, force_write);

The cache state should not matter for volatile registers, right?

-Jon
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/2] regmap: Allow installing custom reg_update_bits function

[jon]

From: Jon Ringle 

This commit allows installing a custom reg_update_bits function for cases where
the hardware provides a mechanism to set or clear register bits without a
read/modify/write cycle. Such is the case with the Microchip ENCX24J600.

Signed-off-by: Jon Ringle 
---
 drivers/base/regmap/internal.h | 3 +++
 drivers/base/regmap/regmap.c   | 5 +
 include/linux/regmap.h | 4 
 3 files changed, 12 insertions(+)

diff --git a/drivers/base/regmap/internal.h b/drivers/base/regmap/internal.h
index cc55788..4036d7a 100644
--- a/drivers/base/regmap/internal.h
+++ b/drivers/base/regmap/internal.h
@@ -98,6 +98,9 @@ struct regmap {
 
int (*reg_read)(void *context, unsigned int reg, unsigned int *val);
int (*reg_write)(void *context, unsigned int reg, unsigned int val);
+   int (*reg_update_bits)(void *context, unsigned int reg,
+  unsigned int mask, unsigned int val,
+  bool *change, bool force_write);
 
bool defer_caching;
 
diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index afaf562..ba00d62 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -619,6 +619,7 @@ struct regmap *__regmap_init(struct device *dev,
goto skip_format_initialization;
} else {
map->reg_read  = _regmap_bus_read;
+   map->reg_update_bits = bus->reg_update_bits;
}
 
reg_endian = regmap_get_reg_endian(bus, config);
@@ -2509,6 +2510,10 @@ static int _regmap_update_bits(struct regmap *map, 
unsigned int reg,
int ret;
unsigned int tmp, orig;
 
+   if (map->reg_update_bits)
+   return map->reg_update_bits(map->bus_context, reg, mask, val,
+   change, force_write);
+
ret = _regmap_read(map, reg, );
if (ret != 0)
return ret;
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index 8fc0bfd..4d3a3b1 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -296,6 +296,9 @@ typedef int (*regmap_hw_reg_read)(void *context, unsigned 
int reg,
  unsigned int *val);
 typedef int (*regmap_hw_reg_write)(void *context, unsigned int reg,
   unsigned int val);
+typedef int (*regmap_hw_reg_update_bits)(void *context, unsigned int reg,
+unsigned int mask, unsigned int val,
+bool *change, bool force_write);
 typedef struct regmap_async *(*regmap_hw_async_alloc)(void);
 typedef void (*regmap_hw_free_context)(void *context);
 
@@ -335,6 +338,7 @@ struct regmap_bus {
regmap_hw_gather_write gather_write;
regmap_hw_async_write async_write;
regmap_hw_reg_write reg_write;
+   regmap_hw_reg_update_bits reg_update_bits;
regmap_hw_read read;
regmap_hw_reg_read reg_read;
regmap_hw_free_context free_context;
-- 
2.4.1

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] regmap: Allow installing custom reg_update_bits function

[Mark Brown]

On Thu, Oct 01, 2015 at 02:33:06AM -0400, j...@ringle.org wrote:

> @@ -2509,6 +2510,10 @@ static int _regmap_update_bits(struct regmap *map, 
> unsigned int reg,
>   int ret;
>   unsigned int tmp, orig;
>  
> + if (map->reg_update_bits)
> + return map->reg_update_bits(map->bus_context, reg, mask, val,
> + change, force_write);
> +
>   ret = _regmap_read(map, reg, );
>   if (ret != 0)
>   return ret;

This completely bypasses and therefore breaks the cache infrastructure.


signature.asc
Description: Digital signature