Re: regcache_sync() errors for read-only registers cache

[Mark Brown <broonie@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2237 bytes --]

On Tue, Mar 03, 2015 at 10:22:59AM +0100, Takashi Iwai wrote:
> Mark Brown wrote:

> > > The --scissors option of git am is your friend.

> > That's still pain.

> But it's still better than sending two mails even if you don't know
> whether it's the right patch.  It's even not tag as an RFC.  The patch
> was there just as a reference.

I actually find it harder to work with TBH - it breaks up the mail to
have the extra stuff around the code in there and it's harder to apply
if it's OK.  During a discussion it feels more natural to just have the
diff hunk with the mail text around it instead.

> > Well, it's either that or adding the values read back from the chip to
> > the defaults.

> For fixing the single rw, it's easy in either way (although the latter
> sounds bad from the performance POV).  But what about the block rw?

Why should adding something to the defaults hurt performance (it should
just be a one time cost to insert the default which we've got a
reasonable chance of making back later)?  I guess if there's a lot of
these registers it'll add up but they're pretty rare, usually it's a few
ID and revision registers and anything else is volatile so wouldn't get
cached at all.

Block I/O can just get the same fix I think, the logic is basically the
same it's just what we do with differences that changes.

> > > regmap_wrietable() call in _regmap_write().

> > It's superfluous with respect to what?  Still a bit confused, sorry.

> regmap_writeable() is called twice in that code path with my patch.
> First before calling _regmap_write() and again in _regmap_write().
> The second call is superfluous in this code path although it's needed
> for other paths.

> regmap_writeable() isn't usually that heavy, but it's still
> suboptimal.

Oh, right.  The two checks are logically distinct to me - the check in
_write() is more of an assert than something that's expected to go off,
anything relying on it is in trouble, while the one in the cache sync is
there as part of normal operation.  If anyone cared about performance to
that extent it probably ought to be a build option to even check though
since the I/O is generally so slow and it's rare to implement writeable
at all it doesn't normally matter.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]