Re: [PATCH] ASoC: si476x: Remove custom register I/O implementation

[Mark Brown <broonie@kernel.org>]

[-- Attachment #1.1: Type: text/plain, Size: 1519 bytes --]

On Thu, Sep 26, 2013 at 12:33:33PM -0700, Andrey Smirnov wrote:
> On Thu, Sep 26, 2013 at 11:34 AM, Mark Brown <broonie@kernel.org> wrote:

> > A better and more idiomatic approach would be to have the MFD manage the
> > cache, making the device cache only when it powers things down. This also
> > allows ASoC to use the standard regmap helpers for the device which helps

> Correct me if my understanding is wrong but using standard helper
> function would mean using hw_write and hw_read from soc-io.c. Looking
> at the source code of those functions I don't believe the are
> performing any sort of locking(once again I may be wrong) and rely on
> regmap_* implementation to do it internally. With si476x driver it is
> not the case and it is important that codec write/read functions
> perform si476x_core_lock(core) before they call regmap functions. If
> that is not done it is possible to wreak havoc on I2C bus by trying to
> access the registers of the codec and doing some V4L2 operations on
> the corresponding radio device.

> Although I completely agree that cache managing should be moved to MFD.

What exactly is the locking you're talking about here?  Both regmap and
I/O have locking at those levels, if you try to do a bus interaction
using them it shouldn't conflict with any other register write.  If it's
something higher level then shouldn't the locking be higher level too?

The way the code is written it looks like the lock is being held to
ensure there's no race due to a power state change.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]