Messages in this thread | | | Date | Mon, 19 Sep 2011 18:28:31 +0200 | From | Lars-Peter Clausen <> | Subject | Re: [PATCH 6/6 v5] regmap: Incorporate the regcache core into regmap |
| |
On 09/19/2011 06:13 PM, Mark Brown wrote: > On Mon, Sep 19, 2011 at 05:57:49PM +0200, Lars-Peter Clausen wrote: >> On 09/19/2011 03:34 PM, Dimitris Papastamos wrote: > >>> + if (!map->cache_bypass) { >>> + ret = regcache_write(map, reg, val); >>> + if (!ret || map->cache_only) >>> + return 0; > >> The hw write shouldn't be skipped if the cache write is successful. We should >> only exit here if cache_only is set. > > There's a couple of other issues too. I've already got the following > patch for this one locally: > > --- a/drivers/base/regmap/regmap.c > +++ b/drivers/base/regmap/regmap.c > @@ -304,7 +304,9 @@ static int _regmap_write(struct regmap *map, > unsigned int re > > if (!map->cache_bypass) { > ret = regcache_write(map, reg, val); > - if (!ret || map->cache_only) > + if (ret != 0) > + return ret;
Hm... last time we said we want to fallback to hw read/write even if the cache operation has failed. The issue is that regcache_write will check for regmap_writeable, so you'll get different behaviour if caching is enabled for registers where regmap_writeable returns false.
> + if (map->cache_only) > return 0; > } > >> I also wonder if we should pass the return value of regcache_write on to the >> caller if cache_only is set. > > Yup. > >> Btw. what should happen if both cache_bypass and cache_only are set? Or is that >> an invalid configuration? > > That's not sensible. Probably BUG_ON().
BUG_ON might be a bit to strict, if we wanted to allow cache_only to be enabled through debugfs like it is done for ASoC right now.
| |