[PATCH 1/5] ASoC: wm8962: Enable oscillator if selecting WM8962_FLL_OSC

[PATCH 1/5] ASoC: wm8962: Enable oscillator if selecting WM8962_FLL_OSC

[Stuart Henderson]

Signed-off-by: Stuart Henderson <stuarth@opensource.cirrus.com>
---
 sound/soc/codecs/wm8962.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/sound/soc/codecs/wm8962.c b/sound/soc/codecs/wm8962.c
index fb90ae6a8a34..6d7bb696b135 100644
--- a/sound/soc/codecs/wm8962.c
+++ b/sound/soc/codecs/wm8962.c
@@ -2914,8 +2914,12 @@ static int wm8962_set_fll(struct snd_soc_component *component, int fll_id, int s
 	switch (fll_id) {
 	case WM8962_FLL_MCLK:
 	case WM8962_FLL_BCLK:
+		fll1 |= (fll_id - 1) << WM8962_FLL_REFCLK_SRC_SHIFT;
+		break;
 	case WM8962_FLL_OSC:
 		fll1 |= (fll_id - 1) << WM8962_FLL_REFCLK_SRC_SHIFT;
+		snd_soc_component_update_bits(component, WM8962_PLL2,
+					      WM8962_OSC_ENA, WM8962_OSC_ENA);
 		break;
 	case WM8962_FLL_INT:
 		snd_soc_component_update_bits(component, WM8962_FLL_CONTROL_1,
-- 
2.39.2

[PATCH 2/5] ASoC: wm8962: Enable both SPKOUTR_ENA and SPKOUTL_ENA in mono mode

[Stuart Henderson]

Signed-off-by: Stuart Henderson <stuarth@opensource.cirrus.com>
---
 sound/soc/codecs/wm8962.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/sound/soc/codecs/wm8962.c b/sound/soc/codecs/wm8962.c
index 6d7bb696b135..2256cc0a37eb 100644
--- a/sound/soc/codecs/wm8962.c
+++ b/sound/soc/codecs/wm8962.c
@@ -2229,6 +2229,9 @@ SND_SOC_DAPM_PGA_E("HPOUT", SND_SOC_NOPM, 0, 0, NULL, 0, hp_event,
 
 SND_SOC_DAPM_OUTPUT("HPOUTL"),
 SND_SOC_DAPM_OUTPUT("HPOUTR"),
+
+SND_SOC_DAPM_PGA("SPKOUTL Output", WM8962_CLASS_D_CONTROL_1, 6, 0, NULL, 0),
+SND_SOC_DAPM_PGA("SPKOUTR Output", WM8962_CLASS_D_CONTROL_1, 7, 0, NULL, 0),
 };
 
 static const struct snd_soc_dapm_widget wm8962_dapm_spk_mono_widgets[] = {
@@ -2236,7 +2239,6 @@ SND_SOC_DAPM_MIXER("Speaker Mixer", WM8962_MIXER_ENABLES, 1, 0,
 		   spkmixl, ARRAY_SIZE(spkmixl)),
 SND_SOC_DAPM_MUX_E("Speaker PGA", WM8962_PWR_MGMT_2, 4, 0, &spkoutl_mux,
 		   out_pga_event, SND_SOC_DAPM_POST_PMU),
-SND_SOC_DAPM_PGA("Speaker Output", WM8962_CLASS_D_CONTROL_1, 7, 0, NULL, 0),
 SND_SOC_DAPM_OUTPUT("SPKOUT"),
 };
 
@@ -2251,9 +2253,6 @@ SND_SOC_DAPM_MUX_E("SPKOUTL PGA", WM8962_PWR_MGMT_2, 4, 0, &spkoutl_mux,
 SND_SOC_DAPM_MUX_E("SPKOUTR PGA", WM8962_PWR_MGMT_2, 3, 0, &spkoutr_mux,
 		   out_pga_event, SND_SOC_DAPM_POST_PMU),
 
-SND_SOC_DAPM_PGA("SPKOUTR Output", WM8962_CLASS_D_CONTROL_1, 7, 0, NULL, 0),
-SND_SOC_DAPM_PGA("SPKOUTL Output", WM8962_CLASS_D_CONTROL_1, 6, 0, NULL, 0),
-
 SND_SOC_DAPM_OUTPUT("SPKOUTL"),
 SND_SOC_DAPM_OUTPUT("SPKOUTR"),
 };
@@ -2366,12 +2365,18 @@ static const struct snd_soc_dapm_route wm8962_spk_mono_intercon[] = {
 	{ "Speaker PGA", "Mixer", "Speaker Mixer" },
 	{ "Speaker PGA", "DAC", "DACL" },
 
-	{ "Speaker Output", NULL, "Speaker PGA" },
-	{ "Speaker Output", NULL, "SYSCLK" },
-	{ "Speaker Output", NULL, "TOCLK" },
-	{ "Speaker Output", NULL, "TEMP_SPK" },
+	{ "SPKOUTL Output", NULL, "Speaker PGA" },
+	{ "SPKOUTL Output", NULL, "SYSCLK" },
+	{ "SPKOUTL Output", NULL, "TOCLK" },
+	{ "SPKOUTL Output", NULL, "TEMP_SPK" },
+
+	{ "SPKOUTR Output", NULL, "Speaker PGA" },
+	{ "SPKOUTR Output", NULL, "SYSCLK" },
+	{ "SPKOUTR Output", NULL, "TOCLK" },
+	{ "SPKOUTR Output", NULL, "TEMP_SPK" },
 
-	{ "SPKOUT", NULL, "Speaker Output" },
+	{ "SPKOUT", NULL, "SPKOUTL Output" },
+	{ "SPKOUT", NULL, "SPKOUTR Output" },
 };
 
 static const struct snd_soc_dapm_route wm8962_spk_stereo_intercon[] = {
-- 
2.39.2

Re: [PATCH 2/5] ASoC: wm8962: Enable both SPKOUTR_ENA and SPKOUTL_ENA in mono mode

[Mark Brown]

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

On Wed, Mar 06, 2024 at 04:14:36PM +0000, Stuart Henderson wrote:
> Signed-off-by: Stuart Henderson <stuarth@opensource.cirrus.com>

Because...?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH 3/5] ASoC: fsl: Fix up mclk_id for fsl,imx-audio-wm8962

[Stuart Henderson]

wm8962_set_fll is currently incorrect, and largely ignores the source
parameter.  This patch fixes this use of wm8962_set_fll in preparation
for fixing this.  Previously we were using WM8962_SYSCLK_MCLK (0), but
wm8962_set_fll ends up using the fll_id (in this case WM8962_FLL = 1).
Change this to WM8962_FLL_MCLK (1) instead.

Signed-off-by: Stuart Henderson <stuarth@opensource.cirrus.com>
---
 sound/soc/fsl/fsl-asoc-card.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/fsl/fsl-asoc-card.c b/sound/soc/fsl/fsl-asoc-card.c
index bc07f26ba303..2781fd8d198e 100644
--- a/sound/soc/fsl/fsl-asoc-card.c
+++ b/sound/soc/fsl/fsl-asoc-card.c
@@ -656,7 +656,7 @@ static int fsl_asoc_card_probe(struct platform_device *pdev)
 		priv->card.num_dapm_routes = ARRAY_SIZE(audio_map_tx);
 	} else if (of_device_is_compatible(np, "fsl,imx-audio-wm8962")) {
 		codec_dai_name = "wm8962";
-		priv->codec_priv.mclk_id = WM8962_SYSCLK_MCLK;
+		priv->codec_priv.mclk_id = WM8962_FLL_MCLK;
 		priv->codec_priv.fll_id = WM8962_SYSCLK_FLL;
 		priv->codec_priv.pll_id = WM8962_FLL;
 		priv->dai_fmt |= SND_SOC_DAIFMT_CBP_CFP;
-- 
2.39.2

[PATCH 4/5] ASoC: wm8962: Fix wm8962_set_fll to use source instead of fll_id

[Stuart Henderson]

Previously wm8962_set_fll was using fll_id to configure the source.
This change is problematic, but it looks like there's limited
users of this driver, and luckily they all seem to be intending to
use WM8962_FLL_MCLK as the source which happens to have the same
value as WM8962_FLL.

Signed-off-by: Stuart Henderson <stuarth@opensource.cirrus.com>
---
 sound/soc/codecs/wm8962.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/sound/soc/codecs/wm8962.c b/sound/soc/codecs/wm8962.c
index 2256cc0a37eb..24bd818c3345 100644
--- a/sound/soc/codecs/wm8962.c
+++ b/sound/soc/codecs/wm8962.c
@@ -2890,6 +2890,14 @@ static int wm8962_set_fll(struct snd_soc_component *component, int fll_id, int s
 	int ret;
 	int fll1 = 0;
 
+	switch (fll_id) {
+	case WM8962_FLL:
+		break;
+	default:
+		dev_err(component->dev, "Unknown FLL ID %d\n", fll_id);
+		return -EINVAL;
+	}
+
 	/* Any change? */
 	if (source == wm8962->fll_src && Fref == wm8962->fll_fref &&
 	    Fout == wm8962->fll_fout)
@@ -2916,13 +2924,13 @@ static int wm8962_set_fll(struct snd_soc_component *component, int fll_id, int s
 	/* Parameters good, disable so we can reprogram */
 	snd_soc_component_update_bits(component, WM8962_FLL_CONTROL_1, WM8962_FLL_ENA, 0);
 
-	switch (fll_id) {
+	switch (source) {
 	case WM8962_FLL_MCLK:
 	case WM8962_FLL_BCLK:
-		fll1 |= (fll_id - 1) << WM8962_FLL_REFCLK_SRC_SHIFT;
+		fll1 |= (source - 1) << WM8962_FLL_REFCLK_SRC_SHIFT;
 		break;
 	case WM8962_FLL_OSC:
-		fll1 |= (fll_id - 1) << WM8962_FLL_REFCLK_SRC_SHIFT;
+		fll1 |= (source - 1) << WM8962_FLL_REFCLK_SRC_SHIFT;
 		snd_soc_component_update_bits(component, WM8962_PLL2,
 					      WM8962_OSC_ENA, WM8962_OSC_ENA);
 		break;
-- 
2.39.2

Re: [PATCH 4/5] ASoC: wm8962: Fix wm8962_set_fll to use source instead of fll_id

[Mark Brown]

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

On Wed, Mar 06, 2024 at 04:14:38PM +0000, Stuart Henderson wrote:

> Previously wm8962_set_fll was using fll_id to configure the source.

Which was a problem because...?

> This change is problematic, but it looks like there's limited
> users of this driver, and luckily they all seem to be intending to
> use WM8962_FLL_MCLK as the source which happens to have the same
> value as WM8962_FLL.

If the change is problematic why make it at all?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH 5/5] ASoC: wm8962: Fix up incorrect error message in wm8962_set_fll

[Stuart Henderson]

Use source instead of ret, which seems to be unrelated and will always
be zero.

Signed-off-by: Stuart Henderson <stuarth@opensource.cirrus.com>
---
 sound/soc/codecs/wm8962.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/codecs/wm8962.c b/sound/soc/codecs/wm8962.c
index 24bd818c3345..5ad6850c591f 100644
--- a/sound/soc/codecs/wm8962.c
+++ b/sound/soc/codecs/wm8962.c
@@ -2941,7 +2941,7 @@ static int wm8962_set_fll(struct snd_soc_component *component, int fll_id, int s
 				    WM8962_FLL_FRC_NCO, WM8962_FLL_FRC_NCO);
 		break;
 	default:
-		dev_err(component->dev, "Unknown FLL source %d\n", ret);
+		dev_err(component->dev, "Unknown FLL source %d\n", source);
 		return -EINVAL;
 	}
 
-- 
2.39.2

Re: [PATCH 1/5] ASoC: wm8962: Enable oscillator if selecting WM8962_FLL_OSC

[Mark Brown]

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

On Wed, Mar 06, 2024 at 04:14:35PM +0000, Stuart Henderson wrote:

Why?

> Signed-off-by: Stuart Henderson <stuarth@opensource.cirrus.com>

Please don't send patch serieses without cover letters, having a cover
letter makes it easier to tell why the series is a series and makes it
easier for tooling to work with the series.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: (subset) [PATCH 1/5] ASoC: wm8962: Enable oscillator if selecting WM8962_FLL_OSC

[Mark Brown]

On Wed, 06 Mar 2024 16:14:35 +0000, Stuart Henderson wrote:
> 


Applied to

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

Thanks!

[1/5] ASoC: wm8962: Enable oscillator if selecting WM8962_FLL_OSC
      commit: 03c7874106ca5032a312626b927b1c35f07b1f35
[2/5] ASoC: wm8962: Enable both SPKOUTR_ENA and SPKOUTL_ENA in mono mode
      commit: 6fa849e4d78b880e878138bf238e4fd2bac3c4fa
[5/5] ASoC: wm8962: Fix up incorrect error message in wm8962_set_fll
      commit: 96e202f8c52ac49452f83317cf3b34cd1ad81e18

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