linux-kernel - Re: [PATCH 6/6 v3] regmap: Incorporate the regcache core into regmap

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20110915153718.GB7022@opensource.wolfsonmicro.com>
Date:	Thu, 15 Sep 2011 16:37:18 +0100
From:	Dimitris Papastamos <dp@...nsource.wolfsonmicro.com>
To:	Lars-Peter Clausen <lars@...afoo.de>
Cc:	linux-kernel@...r.kernel.org,
	Mark Brown <broonie@...nsource.wolfsonmicro.com>,
	Liam Girdwood <lrg@...com>,
	Graeme Gregory <gg@...mlogic.co.uk>,
	Samuel Oritz <sameo@...ux.intel.com>
Subject: Re: [PATCH 6/6 v3] regmap: Incorporate the regcache core into
	regmap

On Thu, Sep 15, 2011 at 05:19:32PM +0200, Lars-Peter Clausen wrote:
> > [...]
> > @@ -321,6 +333,18 @@ int regmap_write(struct regmap *map, unsigned int reg, unsigned int val)
> >  
> >  	mutex_lock(&map->lock);
> >  
> > +	if (!map->cache_bypass) {
> > +		ret = regcache_write(map, reg, val);
> > +		if (ret < 0) {
> > +			mutex_unlock(&map->lock);
> > +			return ret;
> > +		}
> > +		if (map->cache_only) {
> > +			mutex_unlock(&map->lock);
> > +			return 0;
> > +		}
> > +	}
> > +
> 
> Would it make sense to move this into _regmap_write ? In that case the code
> wouldn't have to be duplicated in regmap_update_bits and as a bonus it wouldn't
> have to deal with the mutex either.

Hmm, yes from the point of view of simplifying and avoiding code
duplication.

> >  	ret = _regmap_write(map, reg, val);
> >  
> >  	mutex_unlock(&map->lock);
> > @@ -422,6 +446,14 @@ int regmap_read(struct regmap *map, unsigned int reg, unsigned int *val)
> >  
> >  	mutex_lock(&map->lock);
> >  
> > +	if (!map->cache_bypass) {
> > +		ret = regcache_read(map, reg, val);
> > +		if (!ret) {
> > +			mutex_unlock(&map->lock);
> > +			return 0;
> > +		}
> 
> So in case regmap_readable is not true for this register regcache_read will
> return -EIO and we'll fallback to an uncached read. This doesn't make sense in
> my opinion. Or what are the except semantics regmap_readable supposed to be?

I agree the semantics are fuzzy here.

At the moment, we basically have one means of signaling that the cache
can't handle the read request, that is, by returning an error.  We
should probably extend that or simply use fixed error codes with a
specially meaning or something similar.  I guess there is a more
elegant means of doing this though.

> > +	}
> > +
> >  	ret = _regmap_read(map, reg, val);
> >  
> >  	mutex_unlock(&map->lock);
> 
--
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/