[PATCH 0/3] ASoC: adau1372: fixes after debugging custom board

[PATCH 0/3] ASoC: adau1372: fixes after debugging custom board

[Maarten Zanders]

Maarten Zanders (3):
  ASoC: adau1372: fix mclk
  ASoC: adau1372: add support for S24_LE mode
  ASoC: adau1372: correct PGA enable & mute bit

 sound/soc/codecs/adau1372.c | 31 +++++++++++++++++--------------
 1 file changed, 17 insertions(+), 14 deletions(-)


base-commit: ef6747add3ad08a23a94b20db0b62688efb9ccd9
-- 
2.37.3

[PATCH 1/3] ASoC: adau1372: fix mclk

[Maarten Zanders]

"mclk" is retrieved from the configuration and assigned to adau1372->clk.
However adau1372->mclk (==NULL) is used for clk_prepare_enable() and
clk_disable_unprepare() which don't have any effect.

Remove .clk from struct adau1372 and use .mclk throughout.
This change ensures that the input clock is switched on/off when the
bias level is changed.

Signed-off-by: Maarten Zanders <maarten.zanders@mind.be>
---
 sound/soc/codecs/adau1372.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/sound/soc/codecs/adau1372.c b/sound/soc/codecs/adau1372.c
index a9f89e8565ec..00d0825e193b 100644
--- a/sound/soc/codecs/adau1372.c
+++ b/sound/soc/codecs/adau1372.c
@@ -25,7 +25,6 @@
 #include "adau-utils.h"
 
 struct adau1372 {
-	struct clk *clk;
 	struct regmap *regmap;
 	void (*switch_mode)(struct device *dev);
 	bool use_pll;
@@ -925,9 +924,9 @@ int adau1372_probe(struct device *dev, struct regmap *regmap,
 	if (!adau1372)
 		return -ENOMEM;
 
-	adau1372->clk = devm_clk_get(dev, "mclk");
-	if (IS_ERR(adau1372->clk))
-		return PTR_ERR(adau1372->clk);
+	adau1372->mclk = devm_clk_get(dev, "mclk");
+	if (IS_ERR(adau1372->mclk))
+		return PTR_ERR(adau1372->mclk);
 
 	adau1372->pd_gpio = devm_gpiod_get_optional(dev, "powerdown", GPIOD_OUT_HIGH);
 	if (IS_ERR(adau1372->pd_gpio))
@@ -947,7 +946,7 @@ int adau1372_probe(struct device *dev, struct regmap *regmap,
 	 * 12.288MHz. Automatically choose a valid configuration from the
 	 * external clock.
 	 */
-	rate = clk_get_rate(adau1372->clk);
+	rate = clk_get_rate(adau1372->mclk);
 
 	switch (rate) {
 	case 12288000:
-- 
2.37.3

RE: [PATCH 1/3] ASoC: adau1372: fix mclk

[Sa, Nuno]

> -----Original Message-----
> From: Maarten Zanders <maarten.zanders@mind.be>
> Sent: Friday, October 28, 2022 5:26 PM
> To: Lars-Peter Clausen <lars@metafoo.de>; Sa, Nuno
> <Nuno.Sa@analog.com>; Liam Girdwood <lgirdwood@gmail.com>; Mark
> Brown <broonie@kernel.org>; Jaroslav Kysela <perex@perex.cz>; Takashi
> Iwai <tiwai@suse.com>
> Cc: Maarten Zanders <maarten.zanders@mind.be>; alsa-devel@alsa-
> project.org; linux-kernel@vger.kernel.org
> Subject: [PATCH 1/3] ASoC: adau1372: fix mclk
> 
> [External]
> 
> "mclk" is retrieved from the configuration and assigned to adau1372->clk.
> However adau1372->mclk (==NULL) is used for clk_prepare_enable() and
> clk_disable_unprepare() which don't have any effect.
> 
> Remove .clk from struct adau1372 and use .mclk throughout.
> This change ensures that the input clock is switched on/off when the
> bias level is changed.
> 
> Signed-off-by: Maarten Zanders <maarten.zanders@mind.be>
> ---

I guess this needs a Fixes: tag?

- Nuno Sá

Re: [PATCH 1/3] ASoC: adau1372: fix mclk

[Maarten Zanders]

Hi,

On 10/31/22 12:20, Sa, Nuno wrote:
>
> I guess this needs a Fixes: tag?

Yes, you're right. I figured it'd be less relevant since we're fixing 
the initial commit, but that was wrong.

Did you get a chance to look at the fix itself?

Thanks!

RE: [PATCH 1/3] ASoC: adau1372: fix mclk

[Sa, Nuno]

> -----Original Message-----
> From: Maarten Zanders <maarten.zanders@mind.be>
> Sent: Wednesday, November 2, 2022 9:08 AM
> To: Sa, Nuno <Nuno.Sa@analog.com>; Lars-Peter Clausen
> <lars@metafoo.de>; Liam Girdwood <lgirdwood@gmail.com>; Mark Brown
> <broonie@kernel.org>; Jaroslav Kysela <perex@perex.cz>; Takashi Iwai
> <tiwai@suse.com>
> Cc: alsa-devel@alsa-project.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 1/3] ASoC: adau1372: fix mclk
> 
> [External]
> 
> Hi,
> 
> On 10/31/22 12:20, Sa, Nuno wrote:
> >
> > I guess this needs a Fixes: tag?
> 
> Yes, you're right. I figured it'd be less relevant since we're fixing
> the initial commit, but that was wrong.
> 
> Did you get a chance to look at the fix itself?
> 

Yeah, the fix looks valid to me...

- Nuno Sá

[PATCH 2/3] ASoC: adau1372: add support for S24_LE mode

[Maarten Zanders]

The ADAU1372 contains 24bit ADCs and DACs. Allow the driver to use
its native mode which uses the same settings as the current 32 bit
mode.

Signed-off-by: Maarten Zanders <maarten.zanders@mind.be>
---
 sound/soc/codecs/adau1372.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/sound/soc/codecs/adau1372.c b/sound/soc/codecs/adau1372.c
index 00d0825e193b..6b35981c8777 100644
--- a/sound/soc/codecs/adau1372.c
+++ b/sound/soc/codecs/adau1372.c
@@ -662,6 +662,7 @@ static int adau1372_hw_params(struct snd_pcm_substream *substream,
 	case 16:
 		sai1 = ADAU1372_SAI1_BCLKRATE;
 		break;
+	case 24:
 	case 32:
 		sai1 = 0;
 		break;
@@ -699,6 +700,7 @@ static int adau1372_set_tdm_slot(struct snd_soc_dai *dai, unsigned int tx_mask,
 	case 16:
 		sai1 = ADAU1372_SAI1_BCLK_TDMC;
 		break;
+	case 24:
 	case 32:
 		sai1 = 0;
 		break;
@@ -869,7 +871,9 @@ static const struct snd_soc_dai_ops adau1372_dai_ops = {
 	.startup = adau1372_startup,
 };
 
-#define ADAU1372_FORMATS (SNDRV_PCM_FMTBIT_S16_LE |	SNDRV_PCM_FMTBIT_S32_LE)
+#define ADAU1372_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | \
+			  SNDRV_PCM_FMTBIT_S24_LE | \
+			  SNDRV_PCM_FMTBIT_S32_LE)
 
 static struct snd_soc_dai_driver adau1372_dai_driver = {
 	.name = "adau1372",
-- 
2.37.3

Re: [PATCH 2/3] ASoC: adau1372: add support for S24_LE mode

[Mark Brown]

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

On Fri, Oct 28, 2022 at 05:26:24PM +0200, Maarten Zanders wrote:
> The ADAU1372 contains 24bit ADCs and DACs. Allow the driver to use
> its native mode which uses the same settings as the current 32 bit
> mode.

Normally new features like this should come at the end of the series so
that they don't get in the way of merging fixes.  No need to resend for
this though.

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

[PATCH 3/3] ASoC: adau1372: correct PGA enable & mute bit

[Maarten Zanders]

The DAPM control for PGAx uses the PGA mute bit for
power management. This bit is active high but is set to
non-inverted (ie when powering, it will mute).
The ALSA control "PGA x Capture Switch" uses the active
high PGA_ENx bit, but is set to inverted. So when
enabling this switch, the PGA gets disabled.

To correct the behaviour, invert both these bits.

Signed-off-by: Maarten Zanders <maarten.zanders@mind.be>
---
 sound/soc/codecs/adau1372.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/sound/soc/codecs/adau1372.c b/sound/soc/codecs/adau1372.c
index 6b35981c8777..c5e0f10e6982 100644
--- a/sound/soc/codecs/adau1372.c
+++ b/sound/soc/codecs/adau1372.c
@@ -206,10 +206,10 @@ static const struct snd_kcontrol_new adau1372_controls[] = {
 		       2, 1, 0, adau1372_pga_boost_tlv),
 	SOC_SINGLE_TLV("PGA 3 Boost Capture Volume", ADAU1372_REG_PGA_BOOST,
 		       3, 1, 0, adau1372_pga_boost_tlv),
-	SOC_SINGLE("PGA 0 Capture Switch", ADAU1372_REG_PGA_CTRL(0), 7, 1, 1),
-	SOC_SINGLE("PGA 1 Capture Switch", ADAU1372_REG_PGA_CTRL(1), 7, 1, 1),
-	SOC_SINGLE("PGA 2 Capture Switch", ADAU1372_REG_PGA_CTRL(2), 7, 1, 1),
-	SOC_SINGLE("PGA 3 Capture Switch", ADAU1372_REG_PGA_CTRL(3), 7, 1, 1),
+	SOC_SINGLE("PGA 0 Capture Switch", ADAU1372_REG_PGA_CTRL(0), 7, 1, 0),
+	SOC_SINGLE("PGA 1 Capture Switch", ADAU1372_REG_PGA_CTRL(1), 7, 1, 0),
+	SOC_SINGLE("PGA 2 Capture Switch", ADAU1372_REG_PGA_CTRL(2), 7, 1, 0),
+	SOC_SINGLE("PGA 3 Capture Switch", ADAU1372_REG_PGA_CTRL(3), 7, 1, 0),
 
 	SOC_SINGLE_TLV("DAC 0 Playback Volume", ADAU1372_REG_DAC_VOL(0),
 		       0, 0xff, 1, adau1372_digital_tlv),
@@ -369,10 +369,10 @@ static const struct snd_soc_dapm_widget adau1372_dapm_widgets[] = {
 	SND_SOC_DAPM_SUPPLY("MICBIAS0", ADAU1372_REG_MICBIAS, 4, 0, NULL, 0),
 	SND_SOC_DAPM_SUPPLY("MICBIAS1", ADAU1372_REG_MICBIAS, 5, 0, NULL, 0),
 
-	SND_SOC_DAPM_PGA("PGA0", ADAU1372_REG_PGA_CTRL(0), 6, 0, NULL, 0),
-	SND_SOC_DAPM_PGA("PGA1", ADAU1372_REG_PGA_CTRL(1), 6, 0, NULL, 0),
-	SND_SOC_DAPM_PGA("PGA2", ADAU1372_REG_PGA_CTRL(2), 6, 0, NULL, 0),
-	SND_SOC_DAPM_PGA("PGA3", ADAU1372_REG_PGA_CTRL(3), 6, 0, NULL, 0),
+	SND_SOC_DAPM_PGA("PGA0", ADAU1372_REG_PGA_CTRL(0), 6, 1, NULL, 0),
+	SND_SOC_DAPM_PGA("PGA1", ADAU1372_REG_PGA_CTRL(1), 6, 1, NULL, 0),
+	SND_SOC_DAPM_PGA("PGA2", ADAU1372_REG_PGA_CTRL(2), 6, 1, NULL, 0),
+	SND_SOC_DAPM_PGA("PGA3", ADAU1372_REG_PGA_CTRL(3), 6, 1, NULL, 0),
 	SND_SOC_DAPM_ADC("ADC0", NULL, ADAU1372_REG_ADC_CTRL2, 0, 0),
 	SND_SOC_DAPM_ADC("ADC1", NULL, ADAU1372_REG_ADC_CTRL2, 1, 0),
 	SND_SOC_DAPM_ADC("ADC2", NULL, ADAU1372_REG_ADC_CTRL3, 0, 0),
-- 
2.37.3

RE: [PATCH 3/3] ASoC: adau1372: correct PGA enable & mute bit

[Sa, Nuno]

> -----Original Message-----
> From: Maarten Zanders <maarten.zanders@mind.be>
> Sent: Friday, October 28, 2022 5:26 PM
> To: Lars-Peter Clausen <lars@metafoo.de>; Sa, Nuno
> <Nuno.Sa@analog.com>; Liam Girdwood <lgirdwood@gmail.com>; Mark
> Brown <broonie@kernel.org>; Jaroslav Kysela <perex@perex.cz>; Takashi
> Iwai <tiwai@suse.com>
> Cc: Maarten Zanders <maarten.zanders@mind.be>; alsa-devel@alsa-
> project.org; linux-kernel@vger.kernel.org
> Subject: [PATCH 3/3] ASoC: adau1372: correct PGA enable & mute bit
> 
> [External]
> 
> The DAPM control for PGAx uses the PGA mute bit for
> power management. This bit is active high but is set to
> non-inverted (ie when powering, it will mute).
> The ALSA control "PGA x Capture Switch" uses the active
> high PGA_ENx bit, but is set to inverted. So when
> enabling this switch, the PGA gets disabled.
> 
> To correct the behaviour, invert both these bits.
> 
> Signed-off-by: Maarten Zanders <maarten.zanders@mind.be>
> ---

Also looks like a fix so a Fixes: tag? If so, the patch should come
before ("ASoC: adau1372: add support for S24_LE mode")

- Nuno Sá

Re: [PATCH 0/3] ASoC: adau1372: fixes after debugging custom board

[Mark Brown]

On Fri, 28 Oct 2022 17:26:22 +0200, Maarten Zanders wrote:
> Maarten Zanders (3):
>   ASoC: adau1372: fix mclk
>   ASoC: adau1372: add support for S24_LE mode
>   ASoC: adau1372: correct PGA enable & mute bit
> 
>  sound/soc/codecs/adau1372.c | 31 +++++++++++++++++--------------
>  1 file changed, 17 insertions(+), 14 deletions(-)
> 
> [...]

Applied to

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

Thanks!

[1/3] ASoC: adau1372: fix mclk
      commit: 27b6fa6145215c5f49d93e322a16144b928ecd3e
[2/3] ASoC: adau1372: add support for S24_LE mode
      commit: cd887a7ba74c8378ae8b52afa04adb0d49cdf13d
[3/3] ASoC: adau1372: correct PGA enable & mute bit
      commit: dffa0df699d7c20f447e6bd797666366c6bae4b3

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