| Message ID | 1439374365-20623-15-git-send-email-mpa@pengutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Aug 12, 2015 at 12:12:39PM +0200, Markus Pargmann wrote: > Check in regmap_raw_read() and regmap_raw_write() for correct maximum > sizes of the operations. Return -E2BIG if this size is not supported > because it is too big. Why not just split the transaction up like your other changes did? > Also this patch causes an uninitialized variable warning so it > initializes ret (although not necessary). That's just shutting the warning up without understanding where it came from or why this is a good way of handling it :( > @@ -2273,8 +2276,14 @@ int regmap_raw_read(struct regmap *map, unsigned int reg, void *val, > > map->lock(map->lock_arg); > > - if (regmap_volatile_range(map, reg, val_count) || map->cache_bypass || > - map->cache_type == REGCACHE_NONE) { > + if (map->bus->read && This change doesn't match your commit log...
On Wed, Aug 12, 2015 at 12:57:59PM +0100, Mark Brown wrote: > On Wed, Aug 12, 2015 at 12:12:39PM +0200, Markus Pargmann wrote: > > > Check in regmap_raw_read() and regmap_raw_write() for correct maximum > > sizes of the operations. Return -E2BIG if this size is not supported > > because it is too big. > > Why not just split the transaction up like your other changes did? My intention was to keep these raw functions as close to the actual hardware as possible also reporting proper error values. For transactions that are split up you could still use the bulk functions. The actual use case is the bmc150 driver. This chip has a FIFO behind one of its registers. At exactly that register there is no autoincrement of the address or anything, it is just a much larger register. But you have to read from it in bigger chunks than normal. If you read in chunks that do not have the correct size samples from the sensor get discarded. > > > Also this patch causes an uninitialized variable warning so it > > initializes ret (although not necessary). > > That's just shutting the warning up without understanding where it came > from or why this is a good way of handling it :( Yes, will have a look if I can find the issue. > > > @@ -2273,8 +2276,14 @@ int regmap_raw_read(struct regmap *map, unsigned int reg, void *val, > > > > map->lock(map->lock_arg); > > > > - if (regmap_volatile_range(map, reg, val_count) || map->cache_bypass || > > - map->cache_type == REGCACHE_NONE) { > > + if (map->bus->read && > > This change doesn't match your commit log... Sorry. Thanks, Markus
On Wed, Aug 12, 2015 at 02:47:06PM +0200, Markus Pargmann wrote: > On Wed, Aug 12, 2015 at 12:57:59PM +0100, Mark Brown wrote: > > On Wed, Aug 12, 2015 at 12:12:39PM +0200, Markus Pargmann wrote: > > > Check in regmap_raw_read() and regmap_raw_write() for correct maximum > > > sizes of the operations. Return -E2BIG if this size is not supported > > > because it is too big. > > Why not just split the transaction up like your other changes did? > My intention was to keep these raw functions as close to the actual > hardware as possible also reporting proper error values. For > transactions that are split up you could still use the bulk functions. That's not what we do otherwise, and not what you yourself have done for some of the other changes in the series like those around multi write.
diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c index 8c2be516a100..6b636e2ee111 100644 --- a/drivers/base/regmap/regmap.c +++ b/drivers/base/regmap/regmap.c @@ -1584,6 +1584,8 @@ int regmap_raw_write(struct regmap *map, unsigned int reg, return -EINVAL; if (val_len % map->format.val_bytes) return -EINVAL; + if (map->max_raw_io && map->max_raw_io > val_len) + return -E2BIG; map->lock(map->lock_arg); @@ -2262,7 +2264,8 @@ int regmap_raw_read(struct regmap *map, unsigned int reg, void *val, size_t val_bytes = map->format.val_bytes; size_t val_count = val_len / val_bytes; unsigned int v; - int ret, i; + int ret = 0; + int i; if (!map->bus) return -EINVAL; @@ -2273,8 +2276,14 @@ int regmap_raw_read(struct regmap *map, unsigned int reg, void *val, map->lock(map->lock_arg); - if (regmap_volatile_range(map, reg, val_count) || map->cache_bypass || - map->cache_type == REGCACHE_NONE) { + if (map->bus->read && + (regmap_volatile_range(map, reg, val_count) || map->cache_bypass || + map->cache_type == REGCACHE_NONE)) { + if (map->max_raw_io && map->max_raw_io < val_len) { + ret = -E2BIG; + goto out; + } + /* Physical block read if there's no cache involved */ ret = _regmap_raw_read(map, reg, val, val_len);
Check in regmap_raw_read() and regmap_raw_write() for correct maximum sizes of the operations. Return -E2BIG if this size is not supported because it is too big. Also this patch causes an uninitialized variable warning so it initializes ret (although not necessary). Signed-off-by: Markus Pargmann <mpa@pengutronix.de> --- drivers/base/regmap/regmap.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)