lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Mon, 19 Aug 2013 13:18:01 +0200 From: Alexander Sverdlin <alexander.sverdlin@....com> To: Ionut Nicu <ioan.nicu.ext@....com>, linux-kernel@...r.kernel.org CC: Mark Brown <broonie@...nel.org>, Greg Kroah-Hartman <gregkh@...uxfoundation.org> Subject: Re: Fwd: Re: [PATCH 1/3] regmap: flat: use the cache_present bitmap Hello! On 08/19/2013 10:25 AM, Ionut Nicu wrote: > -------- Original Message -------- > Subject: Re: [PATCH 1/3] regmap: flat: use the cache_present bitmap > Date: Fri, 09 Aug 2013 19:16:49 +0200 > From: Ionut Nicu <ioan.nicu.ext@....com> > To: ext Mark Brown <broonie@...nel.org> > CC: Greg Kroah-Hartman <gregkh@...uxfoundation.org>, linux-kernel@...r.kernel.org > > 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: Acked-by: Alexander Sverdlin <alexander.sverdlin@....com> > > 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); > > > > > -- Best regards, Alexander Sverdlin. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists