[alsa-devel] [PATCH 0/2] *** wm8962 regmap related fix ***
This patch set aims to fix issues in wm8962 codec driver related to regmap, currently any attempt to read from ALC Coefficient register will fail when wm8962 is in suspend mode. As ALC2 register is volatile register, it can't be read when cache_only flag is set.
Another issue is, if wm8962's regulator is set to 'regulator-always-on' mode, then after wm8962 is resumed from suspend, wm8962 codec is reset, but cache_dirty flag isn't set, this cause difference between actual wm8962 HW and regmap cache.
Jiada Wang (2): ASoC: WM8962: mark cache_dirty flag after software reset in pm_resume ASoC: Codec: wm8962: declare ALC Coefficients as 4 separate registers
sound/soc/codecs/wm8962.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)
By doing software reset of wm8962 in pm_resume, all registers which have already been set will be reset to default value without regmap interface be involved, thus driver need to mark cache_dirty flag, to let regcache can be updated by regcache_sync().
Signed-off-by: Jiada Wang jiada_wang@mentor.com --- sound/soc/codecs/wm8962.c | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/sound/soc/codecs/wm8962.c b/sound/soc/codecs/wm8962.c index 293e47a..319ee38 100644 --- a/sound/soc/codecs/wm8962.c +++ b/sound/soc/codecs/wm8962.c @@ -3805,6 +3805,13 @@ static int wm8962_runtime_resume(struct device *dev)
wm8962_reset(wm8962);
+ /* All registers have been reset to default value without calling + * to regmap interface, even if reset fails, some registers + * maybe in intermediate status, so we need to mark regmap + * cache_dirty flag. + */ + regcache_mark_dirty(wm8962->regmap); + /* SYSCLK defaults to on; make sure it is off so we can safely * write to registers if the device is declocked. */
On Tue, Oct 06, 2015 at 04:06:54PM +0900, Jiada Wang wrote:
- /* All registers have been reset to default value without calling
* to regmap interface, even if reset fails, some registers* maybe in intermediate status, so we need to mark regmap* cache_dirty flag.*/- regcache_mark_dirty(wm8962->regmap);
This is a standard thing that applies to almost all devices, there should be no need for such an extensive comment (which would normally flag up that there's something weird and surprising going on).
Hi
On 10/06/2015 07:59 PM, Mark Brown wrote:
On Tue, Oct 06, 2015 at 04:06:54PM +0900, Jiada Wang wrote:
- /* All registers have been reset to default value without calling
* to regmap interface, even if reset fails, some registers* maybe in intermediate status, so we need to mark regmap* cache_dirty flag.*/- regcache_mark_dirty(wm8962->regmap);
This is a standard thing that applies to almost all devices, there should be no need for such an extensive comment (which would normally flag up that there's something weird and surprising going on).
Will remove the comment in next update
As ALC2 register is volatile, declare it as one of ALC Coefficients register together with other non-volatile registers will cause issue, in case wm8962 has enter suspend mode, and cache_only flag is set, any attempt to read from ALC2 will fail.
Instead of declaring one ALC Coefficients register which contains ALC1 ~ ALC3 and Noise Gate, this patch declares 4 separate registers, so that regmap can handle these registers differently based on their classification.
Signed-off-by: Jiada Wang jiada_wang@mentor.com --- sound/soc/codecs/wm8962.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/sound/soc/codecs/wm8962.c b/sound/soc/codecs/wm8962.c index 319ee38..1201efd 100644 --- a/sound/soc/codecs/wm8962.c +++ b/sound/soc/codecs/wm8962.c @@ -1782,8 +1782,11 @@ SND_SOC_BYTES("HD Bass Coefficients", WM8962_HDBASS_AI_1, 30),
SOC_DOUBLE("ALC Switch", WM8962_ALC1, WM8962_ALCL_ENA_SHIFT, WM8962_ALCR_ENA_SHIFT, 1, 0), -SND_SOC_BYTES_MASK("ALC Coefficients", WM8962_ALC1, 4, +SND_SOC_BYTES_MASK("ALC1", WM8962_ALC1, 1, WM8962_ALCL_ENA_MASK | WM8962_ALCR_ENA_MASK), +SND_SOC_BYTES("ALC2", WM8962_ALC2, 1), +SND_SOC_BYTES("ALC3", WM8962_ALC3, 1), +SND_SOC_BYTES("Noise Gate", WM8962_NOISE_GATE, 1), };
static const struct snd_kcontrol_new wm8962_spk_mono_controls[] = {
On Tue, Oct 06, 2015 at 04:06:55PM +0900, Jiada Wang wrote:
As ALC2 register is volatile, declare it as one of ALC Coefficients register together with other non-volatile registers will cause issue, in case wm8962 has enter suspend mode, and cache_only flag is set, any attempt to read from ALC2 will fail.
Instead of declaring one ALC Coefficients register which contains ALC1 ~ ALC3 and Noise Gate, this patch declares 4 separate registers, so that regmap can handle these registers differently based on their classification.
I don't understand this commit log. Why does regmap care how these registers are presented to userspace, and how does splitting the controls up address the problem with one of the registers being volatile? Surely that register still has the same problem?
Hi
On 10/06/2015 08:01 PM, Mark Brown wrote:
On Tue, Oct 06, 2015 at 04:06:55PM +0900, Jiada Wang wrote:
As ALC2 register is volatile, declare it as one of ALC Coefficients register together with other non-volatile registers will cause issue, in case wm8962 has enter suspend mode, and cache_only flag is set, any attempt to read from ALC2 will fail.
Instead of declaring one ALC Coefficients register which contains ALC1 ~ ALC3 and Noise Gate, this patch declares 4 separate registers, so that regmap can handle these registers differently based on their classification.
I don't understand this commit log. Why does regmap care how these registers are presented to userspace, and how does splitting the controls up address the problem with one of the registers being volatile? Surely that register still has the same problem?
.get callback function will call regmap_raw_read() to read register value from these registers, when these 4 regsters are declared as one "ALC coefficient" register, condition check of regmap_volatile_range() will return false, thus regmap will go word by word for the cache from each register of "ALC Coefficient", the failure scenario is, when wm8962 is in suspend mode (cache_only flag is set), as ALC2 doesn't have cached value, then any attempt to read from it fails,
By splitting these registers, regmap can handle ALC2 as a single volatile register, and always read from HW
Thanks, Jiada
participants (2)
-
Jiada Wang -
Mark Brown