On 09.08.2013 13:44, ext Mark Brown wrote:
> On Fri, Aug 09, 2013 at 12:09:11PM +0200, Ionut Nicu wrote:
>> As opposed to the other regmap cache implementations,
>> regcache_flat didn't use the cache_present bitmap for
>> figuring out whether a register was cached or not, nor
>> did it mark a register as present in the cache when
>> regcache_flat_write() was called.
>
>> This caused incorrect behaviour, such as returning
>> value 0 for non-volatile registers without first reading
>> their value from the device and storing it in the cache.
>
> The goal with the flat cache is to do as little work as possible for
> things like memory mapped devices where the cache operations are
> actually noticable in comparison with the I/O costs. I would therefore
> exapect that anything using the flat cache would want to ensure that the
> cache is fully populated at init time, reading back from the device if
> nothing else (by setting num_reg_defaults_raw but not providing values),
> rather than do additional operations in the data path.
>
Ok, I get your point. I've tried using this approach, but the thing is
that I have a device which supports only single rw operations. The
regcache_hw_init() function calls regmap_raw_read which in turn calls
_regmap_raw_read() when cache_bypass is enabled. But _regmap_raw_read()
doesn't take the use_single_rw flag into account and tries to read
num_reg_defaults_raw registers in a single bus transfer. Unfortunately
this doesn't work with my device.
I thought about changing regcache_hw_init() to check for this flag in
the same way regmap_bulk_read() does, but I think this is pretty ugly.
What do you think about moving the check to use_single_rw inside
_regmap_raw_read(), right where the bus accesses are done?
Something like in the following patch:
diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index 42b45ac..de7f353 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -1486,14 +1486,16 @@ static int _regmap_raw_read(struct regmap *map,
unsigned int reg, void *val,
{
struct regmap_range_node *range;
u8 *u8 = map->work_buf;
- int ret;
+ size_t val_bytes = map->format.val_bytes;
+ size_t reg_size = map->format.reg_bytes + map->format.pad_bytes;
+ int ret, i;
WARN_ON(!map->bus);
range = _regmap_range_lookup(map, reg);
if (range) {
ret = _regmap_select_page(map, ®, range,
- val_len / map->format.val_bytes);
+ val_len / val_bytes);
if (ret != 0)
return ret;
}
@@ -1508,15 +1510,41 @@ static int _regmap_raw_read(struct regmap *map,
unsigned int reg, void *val,
*/
u8[0] |= map->read_flag_mask;
- trace_regmap_hw_read_start(map->dev, reg,
- val_len / map->format.val_bytes);
+ /*
+ * Some devices does not support bulk read, for
+ * them we have a series of single read operations.
+ */
+ if (map->use_single_rw) {
+ for (i = 0; i < val_len; i++) {
+ trace_regmap_hw_read_start(map->dev,
+ reg + (i * map->reg_stride), 1);
+
+ map->format.format_reg(map->work_buf,
+ reg + (i * map->reg_stride),
+ map->reg_shift);
+ u8[0] |= map->read_flag_mask;
+
+ ret = map->bus->read(map->bus_context,
+ map->work_buf,
+ reg_size,
+ val + (i * val_bytes),
+ val_bytes);
+ if (ret != 0)
+ return ret;
- ret = map->bus->read(map->bus_context, map->work_buf,
- map->format.reg_bytes + map->format.pad_bytes,
- val, val_len);
+ trace_regmap_hw_read_done(map->dev,
+ reg + (i * map->reg_stride), 1);
+ }
+ } else {
+ trace_regmap_hw_read_start(map->dev, reg, val_len / val_bytes);
- trace_regmap_hw_read_done(map->dev, reg,
- val_len / map->format.val_bytes);
+ ret = map->bus->read(map->bus_context, map->work_buf,
+ reg_size, val, val_len);
+ if (ret != 0)
+ return ret;
+
+ trace_regmap_hw_read_done(map->dev, reg, val_len / val_bytes);
+ }
return ret;
}
@@ -1702,25 +1730,9 @@ int regmap_bulk_read(struct regmap *map, unsigned int
reg, void *val,
return -EINVAL;
if (vol || map->cache_type == REGCACHE_NONE) {
- /*
- * Some devices does not support bulk read, for
- * them we have a series of single read operations.
- */
- if (map->use_single_rw) {
- for (i = 0; i < val_count; i++) {
- ret = regmap_raw_read(map,
- reg + (i * map->reg_stride),
- val + (i * val_bytes),
- val_bytes);
- if (ret != 0)
- return ret;
- }
- } else {
- ret = regmap_raw_read(map, reg, val,
- val_bytes * val_count);
- if (ret != 0)
- return ret;
- }
+ ret = regmap_raw_read(map, reg, val, val_bytes * val_count);
+ if (ret != 0)
+ return ret;
for (i = 0; i < val_count * val_bytes; i += val_bytes)
map->format.parse_inplace(val + i);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
