Messages in this thread | | | Date | Thu, 15 Sep 2011 17:11:17 +0200 | From | Lars-Peter Clausen <> | Subject | Re: [PATCH 1/6 v3] regmap: Introduce caching support |
| |
Tested-by: Lars-Peter Clausen <lars@metafoo.de> (for REGCACHE_NONE and REGCACHE_RBTREE)
Some minor comments though:
> - > +/** > + * regcache_read: Fetch the value of a given register from the cache. > + * > + * @map: map to configure. > + * @reg: The register index. > + * @value: The value to be returned. > + * > + * Return a negative value on failure, 0 on success. > + */ > +int regcache_read(struct regmap *map, > + unsigned int reg, unsigned int *value) > +{ > + unsigned int max_reg; > + > + if (map->cache_type == REGCACHE_NONE) > + return -ENOSYS; > + > + BUG_ON(!map->cache_ops); > + > + if (!regmap_readable(map, reg)) > + return -EIO; > + > + if (map->max_register) > + max_reg = map->max_register; > + else > + max_reg = map->num_cache_defaults_raw;
In my opinion we should initialize max_register to num_cache_defaults_raw if it is not set during initialization instead of doing this check each time. Also regmap_readable already checks whether the register is in the register range, so there is no need to repeat the check here.
> + > + > + if (reg < max_reg && !regmap_volatile(map, reg)) > + return map->cache_ops->read(map, reg, value); > + return -EINVAL; > +} > +EXPORT_SYMBOL_GPL(regcache_read); > +
| |