[PATCH] ASoC: wm8962: Add an event handler for TEMP_HP and TEMP_SPK

[PATCH] ASoC: wm8962: Add an event handler for TEMP_HP and TEMP_SPK

[Xiaolei Wang]

In wm8962 driver, the WM8962_ADDITIONAL_CONTROL_4 is used as a volatile
register, but this register mixes a bunch of volatile status bits and a
bunch of non-volatile control bits. The dapm widgets TEMP_HP and
TEMP_SPK leverages the control bits in this register. After the wm8962
probe, the regmap will bet set to cache only mode, then a read error
like below would be triggered when trying to read the initial power
state of the dapm widgets TEMP_HP and TEMP_SPK.
  wm8962 0-001a: ASoC: error at soc_component_read_no_lock
  on wm8962.0-001a: -16

In order to fix this issue, we add event handler to actually power
up/down these widgets. With this change, we also need to explicitly
power off these widgets in the wm8962 probe since they are enabled
by default.

Signed-off-by: Xiaolei Wang <xiaolei.wang@windriver.com>
---
 sound/soc/codecs/wm8962.c | 54 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 52 insertions(+), 2 deletions(-)

diff --git a/sound/soc/codecs/wm8962.c b/sound/soc/codecs/wm8962.c
index 398c448ea854..6df06fba4377 100644
--- a/sound/soc/codecs/wm8962.c
+++ b/sound/soc/codecs/wm8962.c
@@ -1840,6 +1840,49 @@ SOC_SINGLE_TLV("SPKOUTR Mixer DACR Volume", WM8962_SPEAKER_MIXER_5,
 	       4, 1, 0, inmix_tlv),
 };
 
+static int tp_event(struct snd_soc_dapm_widget *w,
+		    struct snd_kcontrol *kcontrol, int event)
+{
+	int ret, reg, val, mask;
+	struct snd_soc_component *component = snd_soc_dapm_to_component(w->dapm);
+
+	ret = pm_runtime_resume_and_get(component->dev);
+	if (ret < 0) {
+		dev_err(component->dev, "Failed to resume device: %d\n", ret);
+		return ret;
+	}
+
+	reg = WM8962_ADDITIONAL_CONTROL_4;
+
+	if (!strcmp(w->name, "TEMP_HP")) {
+		mask = WM8962_TEMP_ENA_HP_MASK;
+		val = WM8962_TEMP_ENA_HP;
+	} else if (!strcmp(w->name, "TEMP_SPK")) {
+		mask = WM8962_TEMP_ENA_SPK_MASK;
+		val = WM8962_TEMP_ENA_SPK;
+	} else {
+		pm_runtime_put(component->dev);
+		return -EINVAL;
+	}
+
+	switch (event) {
+	case SND_SOC_DAPM_POST_PMD:
+		val = 0;
+		fallthrough;
+	case SND_SOC_DAPM_POST_PMU:
+		ret = snd_soc_component_update_bits(component, reg, mask, val);
+		break;
+	default:
+		WARN(1, "Invalid event %d\n", event);
+		pm_runtime_put(component->dev);
+		return -EINVAL;
+	}
+
+	pm_runtime_put(component->dev);
+
+	return 0;
+}
+
 static int cp_event(struct snd_soc_dapm_widget *w,
 		    struct snd_kcontrol *kcontrol, int event)
 {
@@ -2140,8 +2183,10 @@ SND_SOC_DAPM_SUPPLY("TOCLK", WM8962_ADDITIONAL_CONTROL_1, 0, 0, NULL, 0),
 SND_SOC_DAPM_SUPPLY_S("DSP2", 1, WM8962_DSP2_POWER_MANAGEMENT,
 		      WM8962_DSP2_ENA_SHIFT, 0, dsp2_event,
 		      SND_SOC_DAPM_POST_PMU | SND_SOC_DAPM_PRE_PMD),
-SND_SOC_DAPM_SUPPLY("TEMP_HP", WM8962_ADDITIONAL_CONTROL_4, 2, 0, NULL, 0),
-SND_SOC_DAPM_SUPPLY("TEMP_SPK", WM8962_ADDITIONAL_CONTROL_4, 1, 0, NULL, 0),
+SND_SOC_DAPM_SUPPLY("TEMP_HP", SND_SOC_NOPM, 0, 0, tp_event,
+		SND_SOC_DAPM_POST_PMU|SND_SOC_DAPM_POST_PMD),
+SND_SOC_DAPM_SUPPLY("TEMP_SPK", SND_SOC_NOPM, 0, 0, tp_event,
+		SND_SOC_DAPM_POST_PMU|SND_SOC_DAPM_POST_PMD),
 
 SND_SOC_DAPM_MIXER("INPGAL", WM8962_LEFT_INPUT_PGA_CONTROL, 4, 0,
 		   inpgal, ARRAY_SIZE(inpgal)),
@@ -3763,6 +3808,11 @@ static int wm8962_i2c_probe(struct i2c_client *i2c)
 	if (ret < 0)
 		goto err_pm_runtime;
 
+	regmap_update_bits(wm8962->regmap, WM8962_ADDITIONAL_CONTROL_4,
+			    WM8962_TEMP_ENA_HP_MASK, 0);
+	regmap_update_bits(wm8962->regmap, WM8962_ADDITIONAL_CONTROL_4,
+			    WM8962_TEMP_ENA_SPK_MASK, 0);
+
 	regcache_cache_only(wm8962->regmap, true);
 
 	/* The drivers should power up as needed */
-- 
2.25.1

Re: [PATCH] ASoC: wm8962: Add an event handler for TEMP_HP and TEMP_SPK

[Charles Keepax]

On Mon, Oct 10, 2022 at 05:20:14PM +0800, Xiaolei Wang wrote:
> In wm8962 driver, the WM8962_ADDITIONAL_CONTROL_4 is used as a volatile
> register, but this register mixes a bunch of volatile status bits and a
> bunch of non-volatile control bits. The dapm widgets TEMP_HP and
> TEMP_SPK leverages the control bits in this register. After the wm8962
> probe, the regmap will bet set to cache only mode, then a read error
> like below would be triggered when trying to read the initial power
> state of the dapm widgets TEMP_HP and TEMP_SPK.
>   wm8962 0-001a: ASoC: error at soc_component_read_no_lock
>   on wm8962.0-001a: -16
> 
> In order to fix this issue, we add event handler to actually power
> up/down these widgets. With this change, we also need to explicitly
> power off these widgets in the wm8962 probe since they are enabled
> by default.
> 
> Signed-off-by: Xiaolei Wang <xiaolei.wang@windriver.com>
> ---

Acked-by: Charles Keepax <ckeepax@opensource.cirrus.com>

Thanks,
Charles

Re: [PATCH] ASoC: wm8962: Add an event handler for TEMP_HP and TEMP_SPK

[Adam Ford]

On Mon, Oct 10, 2022 at 9:57 AM Charles Keepax
<ckeepax@opensource.cirrus.com> wrote:
>
> On Mon, Oct 10, 2022 at 05:20:14PM +0800, Xiaolei Wang wrote:
> > In wm8962 driver, the WM8962_ADDITIONAL_CONTROL_4 is used as a volatile
> > register, but this register mixes a bunch of volatile status bits and a
> > bunch of non-volatile control bits. The dapm widgets TEMP_HP and
> > TEMP_SPK leverages the control bits in this register. After the wm8962
> > probe, the regmap will bet set to cache only mode, then a read error
> > like below would be triggered when trying to read the initial power
> > state of the dapm widgets TEMP_HP and TEMP_SPK.
> >   wm8962 0-001a: ASoC: error at soc_component_read_no_lock
> >   on wm8962.0-001a: -16

Thanks for this.  I saw this same error, but the audio that I use
didn't appear impacted, so I just ignored it.

> >
> > In order to fix this issue, we add event handler to actually power
> > up/down these widgets. With this change, we also need to explicitly
> > power off these widgets in the wm8962 probe since they are enabled
> > by default.
> >
> > Signed-off-by: Xiaolei Wang <xiaolei.wang@windriver.com>
> > ---
>
> Acked-by: Charles Keepax <ckeepax@opensource.cirrus.com>

Tested-by: Adam Ford <aford173@gmail.com>
>
> Thanks,
> Charles

Re: [PATCH] ASoC: wm8962: Add an event handler for TEMP_HP and TEMP_SPK

[Mark Brown]

On Mon, 10 Oct 2022 17:20:14 +0800, Xiaolei Wang wrote:
> In wm8962 driver, the WM8962_ADDITIONAL_CONTROL_4 is used as a volatile
> register, but this register mixes a bunch of volatile status bits and a
> bunch of non-volatile control bits. The dapm widgets TEMP_HP and
> TEMP_SPK leverages the control bits in this register. After the wm8962
> probe, the regmap will bet set to cache only mode, then a read error
> like below would be triggered when trying to read the initial power
> state of the dapm widgets TEMP_HP and TEMP_SPK.
>   wm8962 0-001a: ASoC: error at soc_component_read_no_lock
>   on wm8962.0-001a: -16
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/1] ASoC: wm8962: Add an event handler for TEMP_HP and TEMP_SPK
      commit: ee1aa2ae3eaa96e70229fa61deee87ef4528ffdf

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark