Messages in this thread | | | Date | Thu, 15 Sep 2011 16:32:27 +0100 | From | Dimitris Papastamos <> | Subject | Re: [PATCH 1/6 v3] regmap: Introduce caching support |
| |
On Thu, Sep 15, 2011 at 05:11:17PM +0200, Lars-Peter Clausen wrote: > 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.
Yes, that makes sense.
> > + > > + > > + if (reg < max_reg && !regmap_volatile(map, reg)) > > + return map->cache_ops->read(map, reg, value); > > + return -EINVAL; > > +} > > +EXPORT_SYMBOL_GPL(regcache_read); > > +
| |