[RFC] ASoC: max98373: don't access volatile registers in bias level off

[Mark Brown]

On Mon, Nov 09, 2020 at 09:55:43PM +0800, Bard Liao wrote:
> We will set regcache_cache_only true in suspend. As a result, regmap_read
> will return error when we try to read volatile registers in suspend.
> Besides, it doesn't make sense to read feedback data when codec is not
> active. To avoid the regmap_read error, this patch try to return a fake
> value when kcontrol _get is called in suspend.

> However, the question is what is the "correct" behavior when we try to
> read a volatile register when cache only is set.
> 1. return an error code to signal codec is not available as what we have
> now.
> 2. return a fake value like what this patch do.
> 3. wake-up the codec and always return a valid value.

This depends a bit on what the value is, if a value obtained by waking
the device up is likely to be useful and what userspace is likely to
do if it gets an error.

> -SOC_SINGLE("ADC PVDD", MAX98373_R2054_MEAS_ADC_PVDD_CH_READBACK, 0, 0xFF, 0),
> -SOC_SINGLE("ADC TEMP", MAX98373_R2055_MEAS_ADC_THERM_CH_READBACK, 0, 0xFF, 0),

For things like voltage and temperature it seems like if we return zero
that's not much different from a userspace point of view than returning
an error, they're both clearly bad values so if userspace is doing any
kind of error checking based on looking at the values it's likely to get
upset by unexpected values - a clear indication that it was the read
that failed might be better than bogus data, stops someone wondering why
there's no power or the device is suddenly freezing.  Or if it's
important that we get a value for monitoring purposes then waking the
device up and reading a value might make more sense though it'd burn
power if done by some random ALSA UI on a regular basis which wouldn't
be desirable either, it'd be relatively slow too.

Another option might be for the driver to cache the last value it got
when powering down, that way it can return something "sensible" even if
it's not up to date.

TBH I'd consider moving these to hwmon, it's possibly a bit more
idiomatic to have these there than in the ALSA API where things do stuff
like go through and read all the controls I think?  Not sure that it'd
be that much happier with values that are only intermittently readable
though so the underlying problem remains.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://mailman.alsa-project.org/pipermail/alsa-devel/attachments/20201119/3623cf9a/attachment.sig>