mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2 0/7] ASoC: codecs: aw88261: fixes and cleanup
@ 2026-05-18 21:44 Val Packett
  2026-05-18 21:44 ` [PATCH v2 1/7] ASoC: codecs: aw88261: support changing sample rate and bit width Val Packett
                   ` (7 more replies)
  0 siblings, 8 replies; 15+ messages in thread
From: Val Packett @ 2026-05-18 21:44 UTC (permalink / raw)
  Cc: Val Packett, Liam Girdwood, Mark Brown, Jaroslav Kysela,
	Takashi Iwai, ~postmarketos/upstreaming, phone-devel,
	linux-sound, linux-kernel

The Awinic smart speaker/amp drivers were merged in a very
"downstream-brained" state, where configuration was only really
determined by the binary "firmware" (register list) file instead
of properly participating in the ASoC system. Let's start
untangling this mess. This series makes aw88261 actually usable
on devices like fairphone-fp5, motorola-dubai and xiaomi-pipa.

v2:
- Add missing sample rates to the switch statement (turns out we do have them)
- Process the conversion from requested values to register ones directly in
  the ASoC ops to make sure EINVAL is returned early, from the right place
- Do not include untested aw88258 fixes.. (why did I think that some support
  for it was already present in the tree?)
- Do include TDM support, logspam reduction, a newly discovered mask fix
- Remove misfeatures inherited from downstream: async start, fade-in/fade-out
  (hopefully this won't be too controversial?)
- Also overhaul volume control to make it actually usable as PlaybackVolume

v1: https://lore.kernel.org/all/20260420213250.215465-2-val@packett.cool/

~val

---

Val Packett (7):
  ASoC: codecs: aw88261: support changing sample rate and bit width
  ASoC: codecs: aw88261: add TDM support
  ASoC: codecs: aw88261: reduce log spam
  ASoC: codecs: aw88261: remove fade in/out on start/stop
  ASoC: codecs: aw88261: remove async start
  ASoC: codecs: aw88261: fix incorrect masks for boost regs
  ASoC: codecs: aw88261: make volume control usable

 sound/soc/codecs/aw88261.c | 612 +++++++++++++++++++------------------
 sound/soc/codecs/aw88261.h | 179 ++++++++++-
 2 files changed, 486 insertions(+), 305 deletions(-)

-- 
2.53.0


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v2 1/7] ASoC: codecs: aw88261: support changing sample rate and bit width
  2026-05-18 21:44 [PATCH v2 0/7] ASoC: codecs: aw88261: fixes and cleanup Val Packett
@ 2026-05-18 21:44 ` Val Packett
  2026-05-18 21:44 ` [PATCH v2 2/7] ASoC: codecs: aw88261: add TDM support Val Packett
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 15+ messages in thread
From: Val Packett @ 2026-05-18 21:44 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai, Weidong Wang
  Cc: Val Packett, ~postmarketos/upstreaming, phone-devel, linux-sound,
	linux-kernel

The aw88261 driver only worked with 32-bit 48kHz streams so far due to
the lack of a proper PLL initialization sequence. Fix by selecting all
the necessary PLL settings based on what was passed to us by the
hw_params/set_fmt ops. This replaces the strange downstream routine
that tries two divider modes in sequence.

Fixes: 028a2ae25691 ("ASoC: codecs: Add aw88261 amplifier driver")
Signed-off-by: Val Packett <val@packett.cool>
---
 sound/soc/codecs/aw88261.c | 247 +++++++++++++++++++++++++++----------
 sound/soc/codecs/aw88261.h | 116 ++++++++++++++++-
 2 files changed, 299 insertions(+), 64 deletions(-)

diff --git a/sound/soc/codecs/aw88261.c b/sound/soc/codecs/aw88261.c
index a6805d5405cd..2064e72b51af 100644
--- a/sound/soc/codecs/aw88261.c
+++ b/sound/soc/codecs/aw88261.c
@@ -13,6 +13,7 @@
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
 #include <sound/soc.h>
+#include <sound/pcm_params.h>
 #include "aw88261.h"
 #include "aw88395/aw88395_data_type.h"
 #include "aw88395/aw88395_device.h"
@@ -158,7 +159,7 @@ static int aw88261_dev_get_iis_status(struct aw_device *aw_dev)
 	return ret;
 }
 
-static int aw88261_dev_check_mode1_pll(struct aw_device *aw_dev)
+static int aw88261_dev_check_pll(struct aw_device *aw_dev)
 {
 	int ret, i;
 
@@ -175,71 +176,48 @@ static int aw88261_dev_check_mode1_pll(struct aw_device *aw_dev)
 	return -EPERM;
 }
 
-static int aw88261_dev_check_mode2_pll(struct aw_device *aw_dev)
-{
-	unsigned int reg_val;
-	int ret, i;
-
-	ret = regmap_read(aw_dev->regmap, AW88261_PLLCTRL1_REG, &reg_val);
-	if (ret)
-		return ret;
-
-	reg_val &= (~AW88261_CCO_MUX_MASK);
-	if (reg_val == AW88261_CCO_MUX_DIVIDED_VALUE) {
-		dev_dbg(aw_dev->dev, "CCO_MUX is already divider");
-		return -EPERM;
-	}
-
-	/* change mode2 */
-	ret = regmap_update_bits(aw_dev->regmap, AW88261_PLLCTRL1_REG,
-			~AW88261_CCO_MUX_MASK, AW88261_CCO_MUX_DIVIDED_VALUE);
-	if (ret)
-		return ret;
-
-	for (i = 0; i < AW88261_DEV_SYSST_CHECK_MAX; i++) {
-		ret = aw88261_dev_get_iis_status(aw_dev);
-		if (ret) {
-			dev_err(aw_dev->dev, "mode2 iis signal check error");
-			usleep_range(AW88261_2000_US, AW88261_2000_US + 10);
-		} else {
-			break;
-		}
-	}
-
-	/* change mode1 */
-	ret = regmap_update_bits(aw_dev->regmap, AW88261_PLLCTRL1_REG,
-			~AW88261_CCO_MUX_MASK, AW88261_CCO_MUX_BYPASS_VALUE);
-	if (ret == 0) {
-		usleep_range(AW88261_2000_US, AW88261_2000_US + 10);
-		for (i = 0; i < AW88261_DEV_SYSST_CHECK_MAX; i++) {
-			ret = aw88261_dev_check_mode1_pll(aw_dev);
-			if (ret) {
-				dev_err(aw_dev->dev, "mode2 switch to mode1, iis signal check error");
-				usleep_range(AW88261_2000_US, AW88261_2000_US + 10);
-			} else {
-				break;
-			}
-		}
-	}
-
-	return ret;
-}
-
-static int aw88261_dev_check_syspll(struct aw_device *aw_dev)
+static int aw88261_dev_configure_syspll(struct aw88261 *aw88261)
 {
+	struct aw_device *aw_dev = aw88261->aw_pa;
 	int ret;
 
-	ret = aw88261_dev_check_mode1_pll(aw_dev);
-	if (ret) {
-		dev_dbg(aw_dev->dev, "mode1 check iis failed try switch to mode2 check");
-		ret = aw88261_dev_check_mode2_pll(aw_dev);
-		if (ret) {
-			dev_err(aw_dev->dev, "mode2 check iis failed");
-			return ret;
-		}
-	}
+	/* PLL divider must be used for 8/16/32 kHz modes */
+	ret = regmap_update_bits(aw_dev->regmap, AW88261_PLLCTRL1_REG,
+			~AW88261_CCO_MUX_MASK, aw88261->cco_mux_value);
+	if (ret)
+		return ret;
 
-	return ret;
+	/* The word clock (WCK) defines the beginning of a frame */
+	ret = regmap_update_bits(aw_dev->regmap, AW88261_I2SCTRL1_REG,
+			~AW88261_I2SSR_MASK, aw88261->sr_value);
+	if (ret)
+		return ret;
+
+	/* The bit clock (BCK) defines the length of a frame */
+	ret = regmap_update_bits(aw_dev->regmap, AW88261_I2SCTRL1_REG,
+			~AW88261_I2SBCK_MASK, aw88261->bck_value);
+	if (ret)
+		return ret;
+
+	/* The logical frame size is the width of data for 1 slot */
+	ret = regmap_update_bits(aw_dev->regmap, AW88261_I2SCTRL1_REG,
+			~AW88261_I2SFS_MASK, aw88261->fs_value);
+	if (ret)
+		return ret;
+
+	/* The I2S interface mode (Philips standard, LSB/MSB justified) */
+	ret = regmap_update_bits(aw_dev->regmap, AW88261_I2SCTRL1_REG,
+			~AW88261_I2SMD_MASK, aw88261->md_value);
+	if (ret)
+		return ret;
+
+	/* The polarity of the bit clock (BCK) */
+	ret = regmap_update_bits(aw_dev->regmap, AW88261_SYSCTRL_REG,
+			~AW88261_BCKINV_MASK, aw88261->bck_inv_value);
+	if (ret)
+		return ret;
+
+	return aw88261_dev_check_pll(aw_dev);
 }
 
 static int aw88261_dev_check_sysst(struct aw_device *aw_dev)
@@ -558,7 +536,7 @@ static int aw88261_dev_start(struct aw88261 *aw88261)
 	aw88261_dev_pwd(aw_dev, false);
 	usleep_range(AW88261_2000_US, AW88261_2000_US + 10);
 
-	ret = aw88261_dev_check_syspll(aw_dev);
+	ret = aw88261_dev_configure_syspll(aw88261);
 	if (ret) {
 		dev_err(aw_dev->dev, "pll check failed cannot start");
 		goto pll_check_fail;
@@ -712,6 +690,140 @@ static void aw88261_start(struct aw88261 *aw88261, bool sync_start)
 			AW88261_START_WORK_DELAY_MS);
 }
 
+static int aw88261_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
+{
+	struct snd_soc_component *component = dai->component;
+	struct aw88261 *aw88261 = snd_soc_component_get_drvdata(component);
+
+	switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
+	case SND_SOC_DAIFMT_NB_NF:
+		aw88261->bck_inv_value = AW88261_BCKINV_NOT_INVERT_VALUE;
+		break;
+	case SND_SOC_DAIFMT_IB_NF:
+		aw88261->bck_inv_value = AW88261_BCKINV_INVERTED_VALUE;
+		break;
+	default:
+		dev_err(aw88261->aw_pa->dev, "unsupported invert mode 0x%x\n",
+			fmt & SND_SOC_DAIFMT_INV_MASK);
+		return -EINVAL;
+	}
+
+	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
+	case SND_SOC_DAIFMT_I2S:
+		aw88261->md_value = AW88261_I2SMD_PHILIPS_STANDARD_VALUE;
+		break;
+	case SND_SOC_DAIFMT_MSB:
+		aw88261->md_value = AW88261_I2SMD_MSB_JUSTIFIED_VALUE;
+		break;
+	case SND_SOC_DAIFMT_LSB:
+		aw88261->md_value = AW88261_I2SMD_LSB_JUSTIFIED_VALUE;
+		break;
+	default:
+		dev_err(aw88261->aw_pa->dev, "unsupported DAI format 0x%x\n",
+			fmt & SND_SOC_DAIFMT_FORMAT_MASK);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int aw88261_hw_params(struct snd_pcm_substream *substream,
+	struct snd_pcm_hw_params *params,
+	struct snd_soc_dai *dai)
+{
+	struct snd_soc_component *component = dai->component;
+	struct aw88261 *aw88261 = snd_soc_component_get_drvdata(component);
+
+	if (substream->stream == SNDRV_PCM_STREAM_CAPTURE)
+		return 0;
+
+	aw88261->cco_mux_value = AW88261_CCO_MUX_BYPASS_VALUE;
+	switch (params_rate(params)) {
+	case 8000:
+		aw88261->sr_value = AW88261_I2SSR_8KHZ_VALUE;
+		aw88261->cco_mux_value = AW88261_CCO_MUX_DIVIDED_VALUE;
+		break;
+	case 11025:
+		aw88261->sr_value = AW88261_I2SSR_11P025KHZ_VALUE;
+		break;
+	case 12000:
+		aw88261->sr_value = AW88261_I2SSR_12KHZ_VALUE;
+		break;
+	case 16000:
+		aw88261->sr_value = AW88261_I2SSR_16KHZ_VALUE;
+		aw88261->cco_mux_value = AW88261_CCO_MUX_DIVIDED_VALUE;
+		break;
+	case 22050:
+		aw88261->sr_value = AW88261_I2SSR_22P05KHZ_VALUE;
+		break;
+	case 24000:
+		aw88261->sr_value = AW88261_I2SSR_24KHZ_VALUE;
+		break;
+	case 32000:
+		aw88261->sr_value = AW88261_I2SSR_32KHZ_VALUE;
+		aw88261->cco_mux_value = AW88261_CCO_MUX_DIVIDED_VALUE;
+		break;
+	case 44100:
+		aw88261->sr_value = AW88261_I2SSR_44P1KHZ_VALUE;
+		break;
+	case 48000:
+		aw88261->sr_value = AW88261_I2SSR_48KHZ_VALUE;
+		break;
+	case 96000:
+		aw88261->sr_value = AW88261_I2SSR_96KHZ_VALUE;
+		break;
+	case 192000:
+		aw88261->sr_value = AW88261_I2SSR_192KHZ_VALUE;
+		break;
+	default:
+		dev_err(aw88261->aw_pa->dev, "unsupported sample rate %d\n",
+			params_rate(params));
+		return -EINVAL;
+	}
+
+	switch (params_width(params)) {
+	case 16:
+		aw88261->fs_value = AW88261_I2SFS_16_BITS_VALUE;
+		break;
+	case 20:
+		aw88261->fs_value = AW88261_I2SFS_20_BITS_VALUE;
+		break;
+	case 24:
+		aw88261->fs_value = AW88261_I2SFS_24_BITS_VALUE;
+		break;
+	case 32:
+		aw88261->fs_value = AW88261_I2SFS_32_BITS_VALUE;
+		break;
+	default:
+		dev_err(aw88261->aw_pa->dev, "unsupported bit width %d\n",
+			params_width(params));
+		return -EINVAL;
+	}
+
+	switch (params_physical_width(params)) {
+	case 16:
+		aw88261->bck_value = AW88261_I2SBCK_32FS_VALUE;
+		break;
+	case 24:
+		aw88261->bck_value = AW88261_I2SBCK_48FS_VALUE;
+		break;
+	case 32:
+		aw88261->bck_value = AW88261_I2SBCK_64FS_VALUE;
+		break;
+	default:
+		dev_err(aw88261->aw_pa->dev, "unsupported physical bit width %d\n",
+			params_physical_width(params));
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static const struct snd_soc_dai_ops aw88261_dai_ops = {
+	.set_fmt = aw88261_set_fmt,
+	.hw_params = aw88261_hw_params,
+};
+
 static struct snd_soc_dai_driver aw88261_dai[] = {
 	{
 		.name = "aw88261-aif",
@@ -730,6 +842,7 @@ static struct snd_soc_dai_driver aw88261_dai[] = {
 			.rates = AW88261_RATES,
 			.formats = AW88261_FORMATS,
 		},
+		.ops = &aw88261_dai_ops,
 	},
 };
 
@@ -1249,6 +1362,14 @@ static int aw88261_i2c_probe(struct i2c_client *i2c)
 	if (!aw88261)
 		return -ENOMEM;
 
+	/* set defaults */
+	aw88261->sr_value = AW88261_I2SSR_48KHZ_VALUE;
+	aw88261->cco_mux_value = AW88261_CCO_MUX_BYPASS_VALUE;
+	aw88261->fs_value = AW88261_I2SFS_24_BITS_VALUE;
+	aw88261->bck_value = AW88261_I2SBCK_64FS_VALUE;
+	aw88261->bck_inv_value = AW88261_BCKINV_NOT_INVERT_VALUE;
+	aw88261->md_value = AW88261_I2SMD_PHILIPS_STANDARD_VALUE;
+
 	mutex_init(&aw88261->lock);
 
 	i2c_set_clientdata(i2c, aw88261);
diff --git a/sound/soc/codecs/aw88261.h b/sound/soc/codecs/aw88261.h
index 1fee589608d6..3d7483d625a9 100644
--- a/sound/soc/codecs/aw88261.h
+++ b/sound/soc/codecs/aw88261.h
@@ -116,6 +116,19 @@
 #define AW88261_VCALK_SHIFT		(0)
 #define AW88261_VCALKL_SHIFT		(0)
 
+#define AW88261_BCKINV_START_BIT	(4)
+#define AW88261_BCKINV_BITS_LEN	(1)
+#define AW88261_BCKINV_MASK	\
+	(~(((1<<AW88261_BCKINV_BITS_LEN)-1) << AW88261_BCKINV_START_BIT))
+
+#define AW88261_BCKINV_NOT_INVERT	(0)
+#define AW88261_BCKINV_NOT_INVERT_VALUE	\
+	(AW88261_BCKINV_NOT_INVERT << AW88261_BCKINV_START_BIT)
+
+#define AW88261_BCKINV_INVERTED	(1)
+#define AW88261_BCKINV_INVERTED_VALUE	\
+	(AW88261_BCKINV_INVERTED << AW88261_BCKINV_START_BIT)
+
 #define AW88261_AMPPD_START_BIT	(1)
 #define AW88261_AMPPD_BITS_LEN		(1)
 #define AW88261_AMPPD_MASK		\
@@ -264,7 +277,98 @@
 #define AW88261_I2STXEN_ENABLE_VALUE	\
 	(AW88261_I2STXEN_ENABLE << AW88261_I2STXEN_START_BIT)
 
-#define AW88261_CCO_MUX_START_BIT	(14)
+#define AW88261_I2SMD_START_BIT	(8)
+#define AW88261_I2SMD_BITS_LEN	(2)
+#define AW88261_I2SMD_MASK	\
+	(~(((1<<AW88261_I2SMD_BITS_LEN)-1) << AW88261_I2SMD_START_BIT))
+
+#define AW88261_I2SMD_PHILIPS_STANDARD	(0)
+#define AW88261_I2SMD_PHILIPS_STANDARD_VALUE	\
+	(AW88261_I2SMD_PHILIPS_STANDARD << AW88261_I2SMD_START_BIT)
+
+#define AW88261_I2SMD_MSB_JUSTIFIED	(1)
+#define AW88261_I2SMD_MSB_JUSTIFIED_VALUE	\
+	(AW88261_I2SMD_MSB_JUSTIFIED << AW88261_I2SMD_START_BIT)
+
+#define AW88261_I2SMD_LSB_JUSTIFIED	(2)
+#define AW88261_I2SMD_LSB_JUSTIFIED_VALUE	\
+	(AW88261_I2SMD_LSB_JUSTIFIED << AW88261_I2SMD_START_BIT)
+
+#define AW88261_I2SFS_START_BIT			(6)
+#define AW88261_I2SFS_BITS_LEN			(2)
+#define AW88261_I2SFS_MASK				\
+	(~(((1<<AW88261_I2SFS_BITS_LEN)-1)<<AW88261_I2SFS_START_BIT))
+
+#define AW88261_I2SFS_16_BITS			(0)
+#define AW88261_I2SFS_16_BITS_VALUE		\
+	(AW88261_I2SFS_16_BITS << AW88261_I2SFS_START_BIT)
+#define AW88261_I2SFS_20_BITS			(1)
+#define AW88261_I2SFS_20_BITS_VALUE		\
+	(AW88261_I2SFS_20_BITS << AW88261_I2SFS_START_BIT)
+#define AW88261_I2SFS_24_BITS			(2)
+#define AW88261_I2SFS_24_BITS_VALUE		\
+	(AW88261_I2SFS_24_BITS << AW88261_I2SFS_START_BIT)
+#define AW88261_I2SFS_32_BITS			(3)
+#define AW88261_I2SFS_32_BITS_VALUE		\
+	(AW88261_I2SFS_32_BITS << AW88261_I2SFS_START_BIT)
+
+#define AW88261_I2SBCK_START_BIT	(4)
+#define AW88261_I2SBCK_BITS_LEN	(2)
+#define AW88261_I2SBCK_MASK	\
+	(~(((1<<AW88261_I2SBCK_BITS_LEN)-1) << AW88261_I2SBCK_START_BIT))
+
+#define AW88261_I2SBCK_32FS	(0)
+#define AW88261_I2SBCK_32FS_VALUE	\
+	(AW88261_I2SBCK_32FS << AW88261_I2SBCK_START_BIT)
+
+#define AW88261_I2SBCK_48FS	(1)
+#define AW88261_I2SBCK_48FS_VALUE	\
+	(AW88261_I2SBCK_48FS << AW88261_I2SBCK_START_BIT)
+
+#define AW88261_I2SBCK_64FS	(2)
+#define AW88261_I2SBCK_64FS_VALUE	\
+	(AW88261_I2SBCK_64FS << AW88261_I2SBCK_START_BIT)
+
+#define AW88261_I2SSR_START_BIT			(0)
+#define AW88261_I2SSR_BITS_LEN			(4)
+#define AW88261_I2SSR_MASK				\
+	(~(((1<<AW88261_I2SSR_BITS_LEN)-1) << AW88261_I2SSR_START_BIT))
+
+#define AW88261_I2SSR_8KHZ				(0)
+#define AW88261_I2SSR_8KHZ_VALUE		\
+	(AW88261_I2SSR_8KHZ << AW88261_I2SSR_START_BIT)
+#define AW88261_I2SSR_11P025KHZ			(1)
+#define AW88261_I2SSR_11P025KHZ_VALUE	\
+	(AW88261_I2SSR_11P025KHZ << AW88261_I2SSR_START_BIT)
+#define AW88261_I2SSR_12KHZ				(2)
+#define AW88261_I2SSR_12KHZ_VALUE		\
+	(AW88261_I2SSR_12KHZ << AW88261_I2SSR_START_BIT)
+#define AW88261_I2SSR_16KHZ				(3)
+#define AW88261_I2SSR_16KHZ_VALUE		\
+	(AW88261_I2SSR_16KHZ << AW88261_I2SSR_START_BIT)
+#define AW88261_I2SSR_22P05KHZ			(4)
+#define AW88261_I2SSR_22P05KHZ_VALUE	\
+	(AW88261_I2SSR_22P05KHZ << AW88261_I2SSR_START_BIT)
+#define AW88261_I2SSR_24KHZ				(5)
+#define AW88261_I2SSR_24KHZ_VALUE		\
+	(AW88261_I2SSR_24KHZ << AW88261_I2SSR_START_BIT)
+#define AW88261_I2SSR_32KHZ				(6)
+#define AW88261_I2SSR_32KHZ_VALUE		\
+	(AW88261_I2SSR_32KHZ << AW88261_I2SSR_START_BIT)
+#define AW88261_I2SSR_44P1KHZ			(7)
+#define AW88261_I2SSR_44P1KHZ_VALUE	\
+	(AW88261_I2SSR_44P1KHZ << AW88261_I2SSR_START_BIT)
+#define AW88261_I2SSR_48KHZ				(8)
+#define AW88261_I2SSR_48KHZ_VALUE		\
+	(AW88261_I2SSR_48KHZ << AW88261_I2SSR_START_BIT)
+#define AW88261_I2SSR_96KHZ				(9)
+#define AW88261_I2SSR_96KHZ_VALUE		\
+	(AW88261_I2SSR_96KHZ << AW88261_I2SSR_START_BIT)
+#define AW88261_I2SSR_192KHZ			(10)
+#define AW88261_I2SSR_192KHZ_VALUE		\
+	(AW88261_I2SSR_192KHZ << AW88261_I2SSR_START_BIT)
+
+#define AW88261_CCO_MUX_START_BIT	(6)
 #define AW88261_CCO_MUX_BITS_LEN	(1)
 #define AW88261_CCO_MUX_MASK		\
 	(~(((1<<AW88261_CCO_MUX_BITS_LEN)-1) << AW88261_CCO_MUX_START_BIT))
@@ -370,7 +474,10 @@
 #define AW88261_START_RETRIES		(5)
 #define AW88261_START_WORK_DELAY_MS	(0)
 
+/* NOTE: 192000 has a reg value donwstream but not listed in datasheet */
 #define AW88261_RATES (SNDRV_PCM_RATE_8000_48000 | \
+			SNDRV_PCM_RATE_12000 | \
+			SNDRV_PCM_RATE_24000 | \
 			SNDRV_PCM_RATE_96000)
 #define AW88261_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | \
 			SNDRV_PCM_FMTBIT_S24_LE | \
@@ -451,6 +558,13 @@ struct aw88261 {
 	unsigned int mute_st;
 	unsigned int amppd_st;
 
+	unsigned int sr_value;
+	unsigned int cco_mux_value;
+	unsigned int fs_value;
+	unsigned int bck_value;
+	unsigned int bck_inv_value;
+	unsigned int md_value;
+
 	bool phase_sync;
 };
 
-- 
2.53.0


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v2 2/7] ASoC: codecs: aw88261: add TDM support
  2026-05-18 21:44 [PATCH v2 0/7] ASoC: codecs: aw88261: fixes and cleanup Val Packett
  2026-05-18 21:44 ` [PATCH v2 1/7] ASoC: codecs: aw88261: support changing sample rate and bit width Val Packett
@ 2026-05-18 21:44 ` Val Packett
  2026-05-19 16:06   ` Mark Brown
  2026-05-18 21:44 ` [PATCH v2 3/7] ASoC: codecs: aw88261: reduce log spam Val Packett
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 15+ messages in thread
From: Val Packett @ 2026-05-18 21:44 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai
  Cc: Val Packett, ~postmarketos/upstreaming, phone-devel, linux-sound,
	linux-kernel

This amp supports TDM mode, so implement the set_tdm_slot operation to
let the SoC driver configure the TDM slot number, width, and masks.

Signed-off-by: Val Packett <val@packett.cool>
---
 sound/soc/codecs/aw88261.c | 88 ++++++++++++++++++++++++++++++++++++++
 sound/soc/codecs/aw88261.h | 53 +++++++++++++++++++++++
 2 files changed, 141 insertions(+)

diff --git a/sound/soc/codecs/aw88261.c b/sound/soc/codecs/aw88261.c
index 2064e72b51af..7b0e778f5a25 100644
--- a/sound/soc/codecs/aw88261.c
+++ b/sound/soc/codecs/aw88261.c
@@ -10,6 +10,7 @@
 
 #include <linux/i2c.h>
 #include <linux/firmware.h>
+#include <linux/bitops.h>
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
 #include <sound/soc.h>
@@ -181,6 +182,30 @@ static int aw88261_dev_configure_syspll(struct aw88261 *aw88261)
 	struct aw_device *aw_dev = aw88261->aw_pa;
 	int ret;
 
+	/* Configure TDM slots (I2S is represented as no slots) */
+	ret = regmap_update_bits(aw_dev->regmap, AW88261_I2SCTRL2_REG,
+			~AW88261_SLOT_NUM_MASK, aw88261->slot_num_value);
+	if (ret)
+		return ret;
+
+	ret = regmap_update_bits(aw_dev->regmap, AW88261_I2SCTRL2_REG,
+			~AW88261_I2S_TX_SLOTVLD_MASK,
+			aw88261->tx_slotvld_mask);
+	if (ret)
+		return ret;
+
+	ret = regmap_update_bits(aw_dev->regmap, AW88261_I2SCTRL2_REG,
+			~AW88261_I2S_RXL_SLOTVLD_MASK,
+			aw88261->rxl_slotvld_mask);
+	if (ret)
+		return ret;
+
+	ret = regmap_update_bits(aw_dev->regmap, AW88261_I2SCTRL2_REG,
+			~AW88261_I2S_RXR_SLOTVLD_MASK,
+			aw88261->rxr_slotvld_mask);
+	if (ret)
+		return ret;
+
 	/* PLL divider must be used for 8/16/32 kHz modes */
 	ret = regmap_update_bits(aw_dev->regmap, AW88261_PLLCTRL1_REG,
 			~AW88261_CCO_MUX_MASK, aw88261->cco_mux_value);
@@ -710,9 +735,11 @@ static int aw88261_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
 
 	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
 	case SND_SOC_DAIFMT_I2S:
+	case SND_SOC_DAIFMT_DSP_A:
 		aw88261->md_value = AW88261_I2SMD_PHILIPS_STANDARD_VALUE;
 		break;
 	case SND_SOC_DAIFMT_MSB:
+	case SND_SOC_DAIFMT_DSP_B:
 		aw88261->md_value = AW88261_I2SMD_MSB_JUSTIFIED_VALUE;
 		break;
 	case SND_SOC_DAIFMT_LSB:
@@ -819,9 +846,68 @@ static int aw88261_hw_params(struct snd_pcm_substream *substream,
 	return 0;
 }
 
+static int aw88261_set_tdm_slot(struct snd_soc_dai *dai,
+	unsigned int tx_mask, unsigned int rx_mask, int slots, int slot_width)
+{
+	struct snd_soc_component *component = dai->component;
+	struct aw88261 *aw88261 = snd_soc_component_get_drvdata(component);
+	int bit;
+
+	switch (slots) {
+	case 0:
+		aw88261->slot_num_value = AW88261_SLOT_NUM_I2S_MODE_VALUE;
+		break;
+	case 1:
+		aw88261->slot_num_value = AW88261_SLOT_NUM_TDM1S_VALUE;
+		break;
+	case 2:
+		aw88261->slot_num_value = AW88261_SLOT_NUM_TDM2S_VALUE;
+		break;
+	case 4:
+		aw88261->slot_num_value = AW88261_SLOT_NUM_TDM4S_VALUE;
+		break;
+	case 6:
+		aw88261->slot_num_value = AW88261_SLOT_NUM_TDM6S_VALUE;
+		break;
+	case 8:
+		aw88261->slot_num_value = AW88261_SLOT_NUM_TDM8S_VALUE;
+		break;
+	case 16:
+		aw88261->slot_num_value = AW88261_SLOT_NUM_TDM16S_VALUE;
+		break;
+	default:
+		dev_err(aw88261->aw_pa->dev, "unsupported slot count %d\n", slots);
+		return -EINVAL;
+	}
+
+	if (tx_mask != 0) {
+		if ((bit = __ffs(tx_mask)) > 15)
+			return -EINVAL;
+
+		aw88261->tx_slotvld_mask = bit << AW88261_I2S_TX_SLOTVLD_START_BIT;
+	}
+
+	if (rx_mask != 0) {
+		if ((bit = __ffs(rx_mask)) > 15)
+			return -EINVAL;
+
+		aw88261->rxl_slotvld_mask = bit << AW88261_I2S_RXL_SLOTVLD_START_BIT;
+	}
+
+	if ((rx_mask & ~BIT(bit)) != 0) {
+		if ((bit = __ffs(rx_mask & ~BIT(bit))) > 15)
+			return -EINVAL;
+
+		aw88261->rxr_slotvld_mask = bit << AW88261_I2S_RXR_SLOTVLD_START_BIT;
+	}
+
+	return 0;
+}
+
 static const struct snd_soc_dai_ops aw88261_dai_ops = {
 	.set_fmt = aw88261_set_fmt,
 	.hw_params = aw88261_hw_params,
+	.set_tdm_slot	= aw88261_set_tdm_slot,
 };
 
 static struct snd_soc_dai_driver aw88261_dai[] = {
@@ -1363,12 +1449,14 @@ static int aw88261_i2c_probe(struct i2c_client *i2c)
 		return -ENOMEM;
 
 	/* set defaults */
+	aw88261->slot_num_value = AW88261_SLOT_NUM_I2S_MODE_VALUE;
 	aw88261->sr_value = AW88261_I2SSR_48KHZ_VALUE;
 	aw88261->cco_mux_value = AW88261_CCO_MUX_BYPASS_VALUE;
 	aw88261->fs_value = AW88261_I2SFS_24_BITS_VALUE;
 	aw88261->bck_value = AW88261_I2SBCK_64FS_VALUE;
 	aw88261->bck_inv_value = AW88261_BCKINV_NOT_INVERT_VALUE;
 	aw88261->md_value = AW88261_I2SMD_PHILIPS_STANDARD_VALUE;
+	aw88261->rxr_slotvld_mask = 1 << AW88261_I2S_RXR_SLOTVLD_START_BIT;
 
 	mutex_init(&aw88261->lock);
 
diff --git a/sound/soc/codecs/aw88261.h b/sound/soc/codecs/aw88261.h
index 3d7483d625a9..02820795d434 100644
--- a/sound/soc/codecs/aw88261.h
+++ b/sound/soc/codecs/aw88261.h
@@ -368,6 +368,54 @@
 #define AW88261_I2SSR_192KHZ_VALUE		\
 	(AW88261_I2SSR_192KHZ << AW88261_I2SSR_START_BIT)
 
+#define AW88261_SLOT_NUM_START_BIT	(12)
+#define AW88261_SLOT_NUM_BITS_LEN	(3)
+#define AW88261_SLOT_NUM_MASK	\
+	(~(((1<<AW88261_SLOT_NUM_BITS_LEN)-1) << AW88261_SLOT_NUM_START_BIT))
+
+#define AW88261_SLOT_NUM_I2S_MODE	(0)
+#define AW88261_SLOT_NUM_I2S_MODE_VALUE	\
+	(AW88261_SLOT_NUM_I2S_MODE << AW88261_SLOT_NUM_START_BIT)
+
+#define AW88261_SLOT_NUM_TDM1S	(1)
+#define AW88261_SLOT_NUM_TDM1S_VALUE	\
+	(AW88261_SLOT_NUM_TDM1S << AW88261_SLOT_NUM_START_BIT)
+
+#define AW88261_SLOT_NUM_TDM2S	(2)
+#define AW88261_SLOT_NUM_TDM2S_VALUE	\
+	(AW88261_SLOT_NUM_TDM2S << AW88261_SLOT_NUM_START_BIT)
+
+#define AW88261_SLOT_NUM_TDM4S	(3)
+#define AW88261_SLOT_NUM_TDM4S_VALUE	\
+	(AW88261_SLOT_NUM_TDM4S << AW88261_SLOT_NUM_START_BIT)
+
+#define AW88261_SLOT_NUM_TDM6S	(4)
+#define AW88261_SLOT_NUM_TDM6S_VALUE	\
+	(AW88261_SLOT_NUM_TDM6S << AW88261_SLOT_NUM_START_BIT)
+
+#define AW88261_SLOT_NUM_TDM8S	(5)
+#define AW88261_SLOT_NUM_TDM8S_VALUE	\
+	(AW88261_SLOT_NUM_TDM8S << AW88261_SLOT_NUM_START_BIT)
+
+#define AW88261_SLOT_NUM_TDM16S	(6)
+#define AW88261_SLOT_NUM_TDM16S_VALUE	\
+	(AW88261_SLOT_NUM_TDM16S << AW88261_SLOT_NUM_START_BIT)
+
+#define AW88261_I2S_TX_SLOTVLD_START_BIT	(8)
+#define AW88261_I2S_TX_SLOTVLD_BITS_LEN	(4)
+#define AW88261_I2S_TX_SLOTVLD_MASK	\
+	(~(((1<<AW88261_I2S_TX_SLOTVLD_BITS_LEN)-1) << AW88261_I2S_TX_SLOTVLD_START_BIT))
+
+#define AW88261_I2S_RXR_SLOTVLD_START_BIT	(4)
+#define AW88261_I2S_RXR_SLOTVLD_BITS_LEN	(4)
+#define AW88261_I2S_RXR_SLOTVLD_MASK	\
+	(~(((1<<AW88261_I2S_RXR_SLOTVLD_BITS_LEN)-1) << AW88261_I2S_RXR_SLOTVLD_START_BIT))
+
+#define AW88261_I2S_RXL_SLOTVLD_START_BIT	(0)
+#define AW88261_I2S_RXL_SLOTVLD_BITS_LEN	(4)
+#define AW88261_I2S_RXL_SLOTVLD_MASK	\
+	(~(((1<<AW88261_I2S_RXL_SLOTVLD_BITS_LEN)-1) << AW88261_I2S_RXL_SLOTVLD_START_BIT))
+
 #define AW88261_CCO_MUX_START_BIT	(6)
 #define AW88261_CCO_MUX_BITS_LEN	(1)
 #define AW88261_CCO_MUX_MASK		\
@@ -565,6 +613,11 @@ struct aw88261 {
 	unsigned int bck_inv_value;
 	unsigned int md_value;
 
+	unsigned int slot_num_value;
+	unsigned int tx_slotvld_mask;
+	unsigned int rxl_slotvld_mask;
+	unsigned int rxr_slotvld_mask;
+
 	bool phase_sync;
 };
 
-- 
2.53.0


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v2 3/7] ASoC: codecs: aw88261: reduce log spam
  2026-05-18 21:44 [PATCH v2 0/7] ASoC: codecs: aw88261: fixes and cleanup Val Packett
  2026-05-18 21:44 ` [PATCH v2 1/7] ASoC: codecs: aw88261: support changing sample rate and bit width Val Packett
  2026-05-18 21:44 ` [PATCH v2 2/7] ASoC: codecs: aw88261: add TDM support Val Packett
@ 2026-05-18 21:44 ` Val Packett
  2026-05-18 21:44 ` [PATCH v2 4/7] ASoC: codecs: aw88261: remove fade in/out on start/stop Val Packett
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 15+ messages in thread
From: Val Packett @ 2026-05-18 21:44 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai
  Cc: Val Packett, ~postmarketos/upstreaming, phone-devel, linux-sound,
	linux-kernel

This driver would create a wall of logspam during initialization due to
e.g. the PLL not being ready while waiting for it to stabilize. Change
intermediate dev_err() calls to dev_dbg() to reduce the noise.

While here, log the detected chip ID when that check fails.

Signed-off-by: Val Packett <val@packett.cool>
---
 sound/soc/codecs/aw88261.c | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/sound/soc/codecs/aw88261.c b/sound/soc/codecs/aw88261.c
index 7b0e778f5a25..d1fc3403cb5a 100644
--- a/sound/soc/codecs/aw88261.c
+++ b/sound/soc/codecs/aw88261.c
@@ -153,7 +153,7 @@ static int aw88261_dev_get_iis_status(struct aw_device *aw_dev)
 	if (ret)
 		return ret;
 	if ((reg_val & AW88261_BIT_PLL_CHECK) != AW88261_BIT_PLL_CHECK) {
-		dev_err(aw_dev->dev, "check pll lock fail,reg_val:0x%04x", reg_val);
+		dev_dbg(aw_dev->dev, "check pll lock fail,reg_val:0x%04x", reg_val);
 		return -EINVAL;
 	}
 
@@ -167,7 +167,7 @@ static int aw88261_dev_check_pll(struct aw_device *aw_dev)
 	for (i = 0; i < AW88261_DEV_SYSST_CHECK_MAX; i++) {
 		ret = aw88261_dev_get_iis_status(aw_dev);
 		if (ret) {
-			dev_err(aw_dev->dev, "mode1 iis signal check error");
+			dev_dbg(aw_dev->dev, "mode1 iis signal check error");
 			usleep_range(AW88261_2000_US, AW88261_2000_US + 10);
 		} else {
 			return ret;
@@ -259,7 +259,7 @@ static int aw88261_dev_check_sysst(struct aw_device *aw_dev)
 		check_val = reg_val & (~AW88261_BIT_SYSST_CHECK_MASK)
 							& AW88261_BIT_SYSST_CHECK;
 		if (check_val != AW88261_BIT_SYSST_CHECK) {
-			dev_err(aw_dev->dev, "check sysst fail, reg_val=0x%04x, check:0x%x",
+			dev_dbg(aw_dev->dev, "check sysst fail, reg_val=0x%04x, check:0x%x",
 				reg_val, AW88261_BIT_SYSST_CHECK);
 			usleep_range(AW88261_2000_US, AW88261_2000_US + 10);
 		} else {
@@ -553,7 +553,7 @@ static int aw88261_dev_start(struct aw88261 *aw88261)
 	int ret;
 
 	if (aw_dev->status == AW88261_DEV_PW_ON) {
-		dev_info(aw_dev->dev, "already power on");
+		dev_dbg(aw_dev->dev, "already power on");
 		return 0;
 	}
 
@@ -563,7 +563,7 @@ static int aw88261_dev_start(struct aw88261 *aw88261)
 
 	ret = aw88261_dev_configure_syspll(aw88261);
 	if (ret) {
-		dev_err(aw_dev->dev, "pll check failed cannot start");
+		dev_dbg(aw_dev->dev, "pll check failed");
 		goto pll_check_fail;
 	}
 
@@ -574,7 +574,7 @@ static int aw88261_dev_start(struct aw88261 *aw88261)
 	/* check i2s status */
 	ret = aw88261_dev_check_sysst(aw_dev);
 	if (ret) {
-		dev_err(aw_dev->dev, "sysst check failed");
+		dev_dbg(aw_dev->dev, "sysst check failed");
 		goto sysst_check_fail;
 	}
 
@@ -675,18 +675,22 @@ static void aw88261_start_pa(struct aw88261 *aw88261)
 	for (i = 0; i < AW88261_START_RETRIES; i++) {
 		ret = aw88261_reg_update(aw88261, aw88261->phase_sync);
 		if (ret) {
-			dev_err(aw88261->aw_pa->dev, "fw update failed, cnt:%d\n", i);
+			dev_dbg(aw88261->aw_pa->dev,
+				"aw88261_reg_update failed, cnt:%d, ret:%d\n", i, ret);
 			continue;
 		}
 		ret = aw88261_dev_start(aw88261);
 		if (ret) {
-			dev_err(aw88261->aw_pa->dev, "aw88261 device start failed. retry = %d", i);
+			dev_dbg(aw88261->aw_pa->dev,
+				"aw88261_dev_start failed, cnt:%d, ret:%d\n", i, ret);
 			continue;
 		} else {
-			dev_info(aw88261->aw_pa->dev, "start success\n");
+			dev_dbg(aw88261->aw_pa->dev, "start success\n");
 			break;
 		}
 	}
+	if (ret != 0)
+		dev_err(aw88261->aw_pa->dev, "start failure (%d)\n", ret);
 }
 
 static void aw88261_startup_work(struct work_struct *work)
@@ -1407,7 +1411,7 @@ static int aw88261_init(struct aw88261 *aw88261, struct i2c_client *i2c, struct
 		return ret;
 	}
 	if (chip_id != AW88261_CHIP_ID) {
-		dev_err(&i2c->dev, "unsupported device");
+		dev_err(&i2c->dev, "unsupported device id = %x", chip_id);
 		return -ENXIO;
 	}
 
-- 
2.53.0


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v2 4/7] ASoC: codecs: aw88261: remove fade in/out on start/stop
  2026-05-18 21:44 [PATCH v2 0/7] ASoC: codecs: aw88261: fixes and cleanup Val Packett
                   ` (2 preceding siblings ...)
  2026-05-18 21:44 ` [PATCH v2 3/7] ASoC: codecs: aw88261: reduce log spam Val Packett
@ 2026-05-18 21:44 ` Val Packett
  2026-05-18 21:44 ` [PATCH v2 5/7] ASoC: codecs: aw88261: remove async start Val Packett
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 15+ messages in thread
From: Val Packett @ 2026-05-18 21:44 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai
  Cc: Val Packett, ~postmarketos/upstreaming, phone-devel, linux-sound,
	linux-kernel

This "feature" was copied from downstream, but it does not belong in
the kernel at all. Remove it to simplify the driver.

Signed-off-by: Val Packett <val@packett.cool>
---
 sound/soc/codecs/aw88261.c | 158 +------------------------------------
 sound/soc/codecs/aw88261.h |   3 -
 2 files changed, 2 insertions(+), 159 deletions(-)

diff --git a/sound/soc/codecs/aw88261.c b/sound/soc/codecs/aw88261.c
index d1fc3403cb5a..ba2a0b0fc296 100644
--- a/sound/soc/codecs/aw88261.c
+++ b/sound/soc/codecs/aw88261.c
@@ -45,50 +45,6 @@ static void aw88261_dev_set_volume(struct aw_device *aw_dev, unsigned int value)
 	regmap_write(aw_dev->regmap, AW88261_SYSCTRL2_REG, real_value);
 }
 
-static void aw88261_dev_fade_in(struct aw_device *aw_dev)
-{
-	struct aw_volume_desc *desc = &aw_dev->volume_desc;
-	int fade_in_vol = desc->ctl_volume;
-	int fade_step = aw_dev->fade_step;
-	int i;
-
-	if (fade_step == 0 || aw_dev->fade_in_time == 0) {
-		aw88261_dev_set_volume(aw_dev, fade_in_vol);
-		return;
-	}
-
-	for (i = AW88261_MUTE_VOL; i >= fade_in_vol; i -= fade_step) {
-		aw88261_dev_set_volume(aw_dev, i);
-		usleep_range(aw_dev->fade_in_time,
-					aw_dev->fade_in_time + 10);
-	}
-
-	if (i != fade_in_vol)
-		aw88261_dev_set_volume(aw_dev, fade_in_vol);
-}
-
-static void aw88261_dev_fade_out(struct aw_device *aw_dev)
-{
-	struct aw_volume_desc *desc = &aw_dev->volume_desc;
-	int fade_step = aw_dev->fade_step;
-	int i;
-
-	if (fade_step == 0 || aw_dev->fade_out_time == 0) {
-		aw88261_dev_set_volume(aw_dev, AW88261_MUTE_VOL);
-		return;
-	}
-
-	for (i = desc->ctl_volume; i <= AW88261_MUTE_VOL; i += fade_step) {
-		aw88261_dev_set_volume(aw_dev, i);
-		usleep_range(aw_dev->fade_out_time, aw_dev->fade_out_time + 10);
-	}
-
-	if (i != AW88261_MUTE_VOL) {
-		aw88261_dev_set_volume(aw_dev, AW88261_MUTE_VOL);
-		usleep_range(aw_dev->fade_out_time, aw_dev->fade_out_time + 10);
-	}
-}
-
 static void aw88261_dev_i2s_tx_enable(struct aw_device *aw_dev, bool flag)
 {
 	if (flag)
@@ -122,13 +78,13 @@ static void aw88261_dev_amppd(struct aw_device *aw_dev, bool amppd)
 static void aw88261_dev_mute(struct aw_device *aw_dev, bool is_mute)
 {
 	if (is_mute) {
-		aw88261_dev_fade_out(aw_dev);
+		aw88261_dev_set_volume(aw_dev, AW88261_MUTE_VOL);
 		regmap_update_bits(aw_dev->regmap, AW88261_SYSCTRL_REG,
 				~AW88261_HMUTE_MASK, AW88261_HMUTE_ENABLE_VALUE);
 	} else {
 		regmap_update_bits(aw_dev->regmap, AW88261_SYSCTRL_REG,
 				~AW88261_HMUTE_MASK, AW88261_HMUTE_DISABLE_VALUE);
-		aw88261_dev_fade_in(aw_dev);
+		aw88261_dev_set_volume(aw_dev, aw_dev->volume_desc.ctl_volume);
 	}
 }
 
@@ -936,75 +892,6 @@ static struct snd_soc_dai_driver aw88261_dai[] = {
 	},
 };
 
-static int aw88261_get_fade_in_time(struct snd_kcontrol *kcontrol,
-	struct snd_ctl_elem_value *ucontrol)
-{
-	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
-	struct aw88261 *aw88261 = snd_soc_component_get_drvdata(component);
-	struct aw_device *aw_dev = aw88261->aw_pa;
-
-	ucontrol->value.integer.value[0] = aw_dev->fade_in_time;
-
-	return 0;
-}
-
-static int aw88261_set_fade_in_time(struct snd_kcontrol *kcontrol,
-	struct snd_ctl_elem_value *ucontrol)
-{
-	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
-	struct aw88261 *aw88261 = snd_soc_component_get_drvdata(component);
-	struct soc_mixer_control *mc =
-		(struct soc_mixer_control *)kcontrol->private_value;
-	struct aw_device *aw_dev = aw88261->aw_pa;
-	int time;
-
-	time = ucontrol->value.integer.value[0];
-
-	if (time < mc->min || time > mc->max)
-		return -EINVAL;
-
-	if (time != aw_dev->fade_in_time) {
-		aw_dev->fade_in_time = time;
-		return 1;
-	}
-
-	return 0;
-}
-
-static int aw88261_get_fade_out_time(struct snd_kcontrol *kcontrol,
-	struct snd_ctl_elem_value *ucontrol)
-{
-	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
-	struct aw88261 *aw88261 = snd_soc_component_get_drvdata(component);
-	struct aw_device *aw_dev = aw88261->aw_pa;
-
-	ucontrol->value.integer.value[0] = aw_dev->fade_out_time;
-
-	return 0;
-}
-
-static int aw88261_set_fade_out_time(struct snd_kcontrol *kcontrol,
-	struct snd_ctl_elem_value *ucontrol)
-{
-	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
-	struct aw88261 *aw88261 = snd_soc_component_get_drvdata(component);
-	struct soc_mixer_control *mc =
-		(struct soc_mixer_control *)kcontrol->private_value;
-	struct aw_device *aw_dev = aw88261->aw_pa;
-	int time;
-
-	time = ucontrol->value.integer.value[0];
-	if (time < mc->min || time > mc->max)
-		return -EINVAL;
-
-	if (time != aw_dev->fade_out_time) {
-		aw_dev->fade_out_time = time;
-		return 1;
-	}
-
-	return 0;
-}
-
 static int aw88261_dev_set_profile_index(struct aw_device *aw_dev, int index)
 {
 	/* check the index whether is valid */
@@ -1128,48 +1015,10 @@ static int aw88261_volume_set(struct snd_kcontrol *kcontrol,
 	return 0;
 }
 
-static int aw88261_get_fade_step(struct snd_kcontrol *kcontrol,
-				struct snd_ctl_elem_value *ucontrol)
-{
-	struct snd_soc_component *codec = snd_kcontrol_chip(kcontrol);
-	struct aw88261 *aw88261 = snd_soc_component_get_drvdata(codec);
-
-	ucontrol->value.integer.value[0] = aw88261->aw_pa->fade_step;
-
-	return 0;
-}
-
-static int aw88261_set_fade_step(struct snd_kcontrol *kcontrol,
-				struct snd_ctl_elem_value *ucontrol)
-{
-	struct snd_soc_component *codec = snd_kcontrol_chip(kcontrol);
-	struct aw88261 *aw88261 = snd_soc_component_get_drvdata(codec);
-	struct soc_mixer_control *mc =
-		(struct soc_mixer_control *)kcontrol->private_value;
-	int value;
-
-	value = ucontrol->value.integer.value[0];
-	if (value < mc->min || value > mc->max)
-		return -EINVAL;
-
-	if (aw88261->aw_pa->fade_step != value) {
-		aw88261->aw_pa->fade_step = value;
-		return 1;
-	}
-
-	return 0;
-}
-
 static const struct snd_kcontrol_new aw88261_controls[] = {
 	SOC_SINGLE_EXT("PCM Playback Volume", AW88261_SYSCTRL2_REG,
 		6, AW88261_MUTE_VOL, 0, aw88261_volume_get,
 		aw88261_volume_set),
-	SOC_SINGLE_EXT("Fade Step", 0, 0, AW88261_MUTE_VOL, 0,
-		aw88261_get_fade_step, aw88261_set_fade_step),
-	SOC_SINGLE_EXT("Volume Ramp Up Step", 0, 0, FADE_TIME_MAX, FADE_TIME_MIN,
-		aw88261_get_fade_in_time, aw88261_set_fade_in_time),
-	SOC_SINGLE_EXT("Volume Ramp Down Step", 0, 0, FADE_TIME_MAX, FADE_TIME_MIN,
-		aw88261_get_fade_out_time, aw88261_set_fade_out_time),
 	AW88261_PROFILE_EXT("Profile Set", aw88261_profile_info,
 		aw88261_profile_get, aw88261_profile_set),
 };
@@ -1260,8 +1109,6 @@ static int aw88261_dev_init(struct aw88261 *aw88261, struct aw_container *aw_cfg
 	if (ret)
 		return ret;
 
-	aw_dev->fade_in_time = AW88261_500_US;
-	aw_dev->fade_out_time = AW88261_500_US;
 	aw_dev->prof_cur = AW88261_INIT_PROFILE;
 	aw_dev->prof_index = AW88261_INIT_PROFILE;
 
@@ -1432,7 +1279,6 @@ static int aw88261_init(struct aw88261 *aw88261, struct i2c_client *i2c, struct
 	aw_dev->prof_info.prof_type = AW88395_DEV_NONE_TYPE_ID;
 	aw_dev->channel = 0;
 	aw_dev->fw_status = AW88261_DEV_FW_FAILED;
-	aw_dev->fade_step = AW88261_VOLUME_STEP_DB;
 	aw_dev->volume_desc.ctl_volume = AW88261_VOL_DEFAULT_VALUE;
 	aw_dev->volume_desc.mute_volume = AW88261_MUTE_VOL;
 	aw88261_parse_channel_dt(aw88261);
diff --git a/sound/soc/codecs/aw88261.h b/sound/soc/codecs/aw88261.h
index 02820795d434..12c98f01fcfe 100644
--- a/sound/soc/codecs/aw88261.h
+++ b/sound/soc/codecs/aw88261.h
@@ -531,9 +531,6 @@
 			SNDRV_PCM_FMTBIT_S24_LE | \
 			SNDRV_PCM_FMTBIT_S32_LE)
 
-#define FADE_TIME_MAX			100000
-#define FADE_TIME_MIN			0
-
 #define AW88261_DEV_DEFAULT_CH		(0)
 #define AW88261_ACF_FILE		"aw88261_acf.bin"
 #define AW88261_DEV_SYSST_CHECK_MAX	(10)
-- 
2.53.0


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v2 5/7] ASoC: codecs: aw88261: remove async start
  2026-05-18 21:44 [PATCH v2 0/7] ASoC: codecs: aw88261: fixes and cleanup Val Packett
                   ` (3 preceding siblings ...)
  2026-05-18 21:44 ` [PATCH v2 4/7] ASoC: codecs: aw88261: remove fade in/out on start/stop Val Packett
@ 2026-05-18 21:44 ` Val Packett
  2026-05-18 21:44 ` [PATCH v2 6/7] ASoC: codecs: aw88261: fix incorrect masks for boost regs Val Packett
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 15+ messages in thread
From: Val Packett @ 2026-05-18 21:44 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai
  Cc: Val Packett, ~postmarketos/upstreaming, phone-devel, linux-sound,
	linux-kernel

Codec drivers are not supposed to do anything like this. The result was
that the first second or so of playback was essentially inaudible, and
very short alert sounds could be missed entirely. Let's not do this.

Signed-off-by: Val Packett <val@packett.cool>
---
 sound/soc/codecs/aw88261.c | 33 ++++-----------------------------
 sound/soc/codecs/aw88261.h |  6 ------
 2 files changed, 4 insertions(+), 35 deletions(-)

diff --git a/sound/soc/codecs/aw88261.c b/sound/soc/codecs/aw88261.c
index ba2a0b0fc296..e0644fb445e5 100644
--- a/sound/soc/codecs/aw88261.c
+++ b/sound/soc/codecs/aw88261.c
@@ -649,17 +649,7 @@ static void aw88261_start_pa(struct aw88261 *aw88261)
 		dev_err(aw88261->aw_pa->dev, "start failure (%d)\n", ret);
 }
 
-static void aw88261_startup_work(struct work_struct *work)
-{
-	struct aw88261 *aw88261 =
-		container_of(work, struct aw88261, start_work.work);
-
-	mutex_lock(&aw88261->lock);
-	aw88261_start_pa(aw88261);
-	mutex_unlock(&aw88261->lock);
-}
-
-static void aw88261_start(struct aw88261 *aw88261, bool sync_start)
+static void aw88261_start(struct aw88261 *aw88261)
 {
 	if (aw88261->aw_pa->fw_status != AW88261_DEV_FW_OK)
 		return;
@@ -667,12 +657,7 @@ static void aw88261_start(struct aw88261 *aw88261, bool sync_start)
 	if (aw88261->aw_pa->status == AW88261_DEV_PW_ON)
 		return;
 
-	if (sync_start == AW88261_SYNC_START)
-		aw88261_start_pa(aw88261);
-	else
-		queue_delayed_work(system_dfl_wq,
-			&aw88261->start_work,
-			AW88261_START_WORK_DELAY_MS);
+	aw88261_start_pa(aw88261);
 }
 
 static int aw88261_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
@@ -970,7 +955,7 @@ static int aw88261_profile_set(struct snd_kcontrol *kcontrol,
 
 	if (aw88261->aw_pa->status) {
 		aw88261_dev_stop(aw88261->aw_pa);
-		aw88261_start(aw88261, AW88261_SYNC_START);
+		aw88261_start(aw88261);
 	}
 
 	mutex_unlock(&aw88261->lock);
@@ -1032,7 +1017,7 @@ static int aw88261_playback_event(struct snd_soc_dapm_widget *w,
 	mutex_lock(&aw88261->lock);
 	switch (event) {
 	case SND_SOC_DAPM_PRE_PMU:
-		aw88261_start(aw88261, AW88261_ASYNC_START);
+		aw88261_start(aw88261);
 		break;
 	case SND_SOC_DAPM_POST_PMD:
 		aw88261_dev_stop(aw88261->aw_pa);
@@ -1192,8 +1177,6 @@ static int aw88261_codec_probe(struct snd_soc_component *component)
 	struct aw88261 *aw88261 = snd_soc_component_get_drvdata(component);
 	int ret;
 
-	INIT_DELAYED_WORK(&aw88261->start_work, aw88261_startup_work);
-
 	ret = aw88261_request_firmware_file(aw88261);
 	if (ret)
 		return dev_err_probe(aw88261->aw_pa->dev, ret,
@@ -1217,16 +1200,8 @@ static int aw88261_codec_probe(struct snd_soc_component *component)
 	return ret;
 }
 
-static void aw88261_codec_remove(struct snd_soc_component *aw_codec)
-{
-	struct aw88261 *aw88261 = snd_soc_component_get_drvdata(aw_codec);
-
-	cancel_delayed_work_sync(&aw88261->start_work);
-}
-
 static const struct snd_soc_component_driver soc_codec_dev_aw88261 = {
 	.probe = aw88261_codec_probe,
-	.remove = aw88261_codec_remove,
 };
 
 static void aw88261_parse_channel_dt(struct aw88261 *aw88261)
diff --git a/sound/soc/codecs/aw88261.h b/sound/soc/codecs/aw88261.h
index 12c98f01fcfe..1b1beba6a26b 100644
--- a/sound/soc/codecs/aw88261.h
+++ b/sound/soc/codecs/aw88261.h
@@ -555,11 +555,6 @@
 	.put = profile_set, \
 }
 
-enum {
-	AW88261_SYNC_START = 0,
-	AW88261_ASYNC_START,
-};
-
 enum aw88261_id {
 	AW88261_CHIP_ID = 0x2113,
 };
@@ -594,7 +589,6 @@ struct aw88261 {
 	struct aw_device *aw_pa;
 	struct mutex lock;
 	struct gpio_desc *reset_gpio;
-	struct delayed_work start_work;
 	struct regmap *regmap;
 	struct aw_container *aw_cfg;
 
-- 
2.53.0


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v2 6/7] ASoC: codecs: aw88261: fix incorrect masks for boost regs
  2026-05-18 21:44 [PATCH v2 0/7] ASoC: codecs: aw88261: fixes and cleanup Val Packett
                   ` (4 preceding siblings ...)
  2026-05-18 21:44 ` [PATCH v2 5/7] ASoC: codecs: aw88261: remove async start Val Packett
@ 2026-05-18 21:44 ` Val Packett
  2026-05-18 21:44 ` [PATCH v2 7/7] ASoC: codecs: aw88261: make volume control usable Val Packett
  2026-05-22 12:36 ` [PATCH v2 0/7] ASoC: codecs: aw88261: fixes and cleanup Luca Weiss
  7 siblings, 0 replies; 15+ messages in thread
From: Val Packett @ 2026-05-18 21:44 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai, Weidong Wang
  Cc: Val Packett, ~postmarketos/upstreaming, phone-devel, linux-sound,
	linux-kernel

The boost-related register fields used in aw88261_reg_force_set use the
exact same definitions as the rest of the fields, where the mask must be
inverted when passing it to regmap_update_bits, but they weren't
inverted here.

Fixes: 028a2ae25691 ("ASoC: codecs: Add aw88261 amplifier driver")
Signed-off-by: Val Packett <val@packett.cool>
---
 sound/soc/codecs/aw88261.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/sound/soc/codecs/aw88261.c b/sound/soc/codecs/aw88261.c
index e0644fb445e5..adc728e45f57 100644
--- a/sound/soc/codecs/aw88261.c
+++ b/sound/soc/codecs/aw88261.c
@@ -243,22 +243,22 @@ static void aw88261_reg_force_set(struct aw88261 *aw88261)
 	if (aw88261->frcset_en == AW88261_FRCSET_ENABLE) {
 		/* set FORCE_PWM */
 		regmap_update_bits(aw88261->regmap, AW88261_BSTCTRL3_REG,
-				AW88261_FORCE_PWM_MASK, AW88261_FORCE_PWM_FORCEMINUS_PWM_VALUE);
+				~AW88261_FORCE_PWM_MASK, AW88261_FORCE_PWM_FORCEMINUS_PWM_VALUE);
 		/* set BOOST_OS_WIDTH */
 		regmap_update_bits(aw88261->regmap, AW88261_BSTCTRL5_REG,
-				AW88261_BST_OS_WIDTH_MASK, AW88261_BST_OS_WIDTH_50NS_VALUE);
+				~AW88261_BST_OS_WIDTH_MASK, AW88261_BST_OS_WIDTH_50NS_VALUE);
 		/* set BURST_LOOPR */
 		regmap_update_bits(aw88261->regmap, AW88261_BSTCTRL6_REG,
-				AW88261_BST_LOOPR_MASK, AW88261_BST_LOOPR_340K_VALUE);
+				~AW88261_BST_LOOPR_MASK, AW88261_BST_LOOPR_340K_VALUE);
 		/* set RSQN_DLY */
 		regmap_update_bits(aw88261->regmap, AW88261_BSTCTRL7_REG,
-				AW88261_RSQN_DLY_MASK, AW88261_RSQN_DLY_35NS_VALUE);
+				~AW88261_RSQN_DLY_MASK, AW88261_RSQN_DLY_35NS_VALUE);
 		/* set BURST_SSMODE */
 		regmap_update_bits(aw88261->regmap, AW88261_BSTCTRL8_REG,
-				AW88261_BURST_SSMODE_MASK, AW88261_BURST_SSMODE_FAST_VALUE);
+				~AW88261_BURST_SSMODE_MASK, AW88261_BURST_SSMODE_FAST_VALUE);
 		/* set BST_BURST */
 		regmap_update_bits(aw88261->regmap, AW88261_BSTCTRL9_REG,
-				AW88261_BST_BURST_MASK, AW88261_BST_BURST_30MA_VALUE);
+				~AW88261_BST_BURST_MASK, AW88261_BST_BURST_30MA_VALUE);
 	} else {
 		dev_dbg(aw88261->aw_pa->dev, "needn't set reg value");
 	}
-- 
2.53.0


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v2 7/7] ASoC: codecs: aw88261: make volume control usable
  2026-05-18 21:44 [PATCH v2 0/7] ASoC: codecs: aw88261: fixes and cleanup Val Packett
                   ` (5 preceding siblings ...)
  2026-05-18 21:44 ` [PATCH v2 6/7] ASoC: codecs: aw88261: fix incorrect masks for boost regs Val Packett
@ 2026-05-18 21:44 ` Val Packett
  2026-05-19 16:20   ` Mark Brown
  2026-05-22 12:39   ` Luca Weiss
  2026-05-22 12:36 ` [PATCH v2 0/7] ASoC: codecs: aw88261: fixes and cleanup Luca Weiss
  7 siblings, 2 replies; 15+ messages in thread
From: Val Packett @ 2026-05-18 21:44 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai, Weidong Wang
  Cc: Val Packett, ~postmarketos/upstreaming, phone-devel, linux-sound,
	linux-kernel

- Invert the value to match userspace expectations (in the hardware,
  positive numbers represent negative dB attenuation)
- Provide TLV metadata for the dB scale (and divide the raw values by 2
  as the excessive precision used by HW is not representable in TLV)
- Do not unnecessarily reset the volume while switching profiles
- Simplify aw88261_dev_set_volume using regmap_update_bits
- Do not add the initial volume from the profile to the requested volume
  as that would throw off the dB mapping (if a lower max limit is
  desired, it can be set in the UCM profile in userspace)

With this change, it's actually possible to use this hardware volume
control as PlaybackVolume in an ALSA UCM profile.

Fixes: 028a2ae25691 ("ASoC: codecs: Add aw88261 amplifier driver")
Signed-off-by: Val Packett <val@packett.cool>
---
 sound/soc/codecs/aw88261.c | 52 +++++++++++++++-----------------------
 sound/soc/codecs/aw88261.h |  1 -
 2 files changed, 21 insertions(+), 32 deletions(-)

diff --git a/sound/soc/codecs/aw88261.c b/sound/soc/codecs/aw88261.c
index adc728e45f57..0e6b2dfe5db9 100644
--- a/sound/soc/codecs/aw88261.c
+++ b/sound/soc/codecs/aw88261.c
@@ -15,6 +15,7 @@
 #include <linux/regulator/consumer.h>
 #include <sound/soc.h>
 #include <sound/pcm_params.h>
+#include <sound/tlv.h>
 #include "aw88261.h"
 #include "aw88395/aw88395_data_type.h"
 #include "aw88395/aw88395_device.h"
@@ -29,20 +30,10 @@ static const struct regmap_config aw88261_remap_config = {
 
 static void aw88261_dev_set_volume(struct aw_device *aw_dev, unsigned int value)
 {
-	struct aw_volume_desc *vol_desc = &aw_dev->volume_desc;
-	unsigned int real_value, volume;
-	unsigned int reg_value;
+	unsigned int volume = min(value, (unsigned int)AW88261_MUTE_VOL);
 
-	volume = min((value + vol_desc->init_volume), (unsigned int)AW88261_MUTE_VOL);
-	real_value = DB_TO_REG_VAL(volume);
-
-	regmap_read(aw_dev->regmap, AW88261_SYSCTRL2_REG, &reg_value);
-
-	real_value = (real_value | (reg_value & AW88261_VOL_START_MASK));
-
-	dev_dbg(aw_dev->dev, "value 0x%x , real_value:0x%x", value, real_value);
-
-	regmap_write(aw_dev->regmap, AW88261_SYSCTRL2_REG, real_value);
+	regmap_update_bits(aw_dev->regmap, AW88261_SYSCTRL2_REG,
+		~AW88261_VOL_MASK, DB_TO_REG_VAL(volume));
 }
 
 static void aw88261_dev_i2s_tx_enable(struct aw_device *aw_dev, bool flag)
@@ -424,17 +415,7 @@ static int aw88261_dev_reg_update(struct aw88261 *aw88261,
 			break;
 	}
 
-	ret = aw88261_dev_set_vcalb(aw_dev);
-	if (ret)
-		return ret;
-
-	if (aw_dev->prof_cur != aw_dev->prof_index)
-		vol_desc->ctl_volume = 0;
-
-	/* keep min volume */
-	aw88261_dev_set_volume(aw_dev, vol_desc->mute_volume);
-
-	return ret;
+	return aw88261_dev_set_vcalb(aw_dev);
 }
 
 static int aw88261_dev_get_prof_name(struct aw_device *aw_dev, int index, char **prof_name)
@@ -970,7 +951,8 @@ static int aw88261_volume_get(struct snd_kcontrol *kcontrol,
 	struct aw88261 *aw88261 = snd_soc_component_get_drvdata(codec);
 	struct aw_volume_desc *vol_desc = &aw88261->aw_pa->volume_desc;
 
-	ucontrol->value.integer.value[0] = vol_desc->ctl_volume;
+	ucontrol->value.integer.value[0] =
+		(AW88261_MUTE_VOL - vol_desc->ctl_volume) / 2;
 
 	return 0;
 }
@@ -983,13 +965,13 @@ static int aw88261_volume_set(struct snd_kcontrol *kcontrol,
 	struct aw_volume_desc *vol_desc = &aw88261->aw_pa->volume_desc;
 	struct soc_mixer_control *mc =
 		(struct soc_mixer_control *)kcontrol->private_value;
-	int value;
-
-	value = ucontrol->value.integer.value[0];
+	int value = ucontrol->value.integer.value[0];
 
 	if (value < mc->min || value > mc->max)
 		return -EINVAL;
 
+	value = AW88261_MUTE_VOL - (value * 2);
+
 	if (vol_desc->ctl_volume != value) {
 		vol_desc->ctl_volume = value;
 		aw88261_dev_set_volume(aw88261->aw_pa, vol_desc->ctl_volume);
@@ -1000,10 +982,18 @@ static int aw88261_volume_set(struct snd_kcontrol *kcontrol,
 	return 0;
 }
 
+/*
+ * The field contains 4 bits in units of 6dB + 6 bits in units of 0.125dB
+ * which is too precise for TLV (!) so we have to multiply the scale by 2.
+ *
+ * The range is clamped at -90dB to prevent overflowing the 4-bit part.
+ */
+static const DECLARE_TLV_DB_SCALE(volume_tlv, -9000, 25, 0);
+
 static const struct snd_kcontrol_new aw88261_controls[] = {
-	SOC_SINGLE_EXT("PCM Playback Volume", AW88261_SYSCTRL2_REG,
-		6, AW88261_MUTE_VOL, 0, aw88261_volume_get,
-		aw88261_volume_set),
+	SOC_SINGLE_EXT_TLV("PCM Playback Volume", AW88261_SYSCTRL2_REG,
+		6, AW88261_MUTE_VOL / 2, 1,
+		aw88261_volume_get, aw88261_volume_set, volume_tlv),
 	AW88261_PROFILE_EXT("Profile Set", aw88261_profile_info,
 		aw88261_profile_get, aw88261_profile_set),
 };
diff --git a/sound/soc/codecs/aw88261.h b/sound/soc/codecs/aw88261.h
index 1b1beba6a26b..76ef47236bb7 100644
--- a/sound/soc/codecs/aw88261.h
+++ b/sound/soc/codecs/aw88261.h
@@ -536,7 +536,6 @@
 #define AW88261_DEV_SYSST_CHECK_MAX	(10)
 #define AW88261_SOFT_RESET_VALUE	(0x55aa)
 #define AW88261_REG_TO_DB		(0x3f)
-#define AW88261_VOL_START_MASK		(0xfc00)
 #define AW88261_INIT_PROFILE		(0)
 
 #define REG_VAL_TO_DB(value)		((((value) >> AW88261_VOL_6DB_START) * \
-- 
2.53.0


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 2/7] ASoC: codecs: aw88261: add TDM support
  2026-05-18 21:44 ` [PATCH v2 2/7] ASoC: codecs: aw88261: add TDM support Val Packett
@ 2026-05-19 16:06   ` Mark Brown
  2026-05-19 22:32     ` Val Packett
  0 siblings, 1 reply; 15+ messages in thread
From: Mark Brown @ 2026-05-19 16:06 UTC (permalink / raw)
  To: Val Packett
  Cc: Liam Girdwood, Jaroslav Kysela, Takashi Iwai,
	~postmarketos/upstreaming, phone-devel, linux-sound,
	linux-kernel

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

On Mon, May 18, 2026 at 06:44:31PM -0300, Val Packett wrote:
> This amp supports TDM mode, so implement the set_tdm_slot operation to
> let the SoC driver configure the TDM slot number, width, and masks.

> +static int aw88261_set_tdm_slot(struct snd_soc_dai *dai,
> +	unsigned int tx_mask, unsigned int rx_mask, int slots, int slot_width)
> +{

In spite of the changelog this doesn't actually seem to use slot_width?
It should.

Note also that we should disable TDM if slots == 0.

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 7/7] ASoC: codecs: aw88261: make volume control usable
  2026-05-18 21:44 ` [PATCH v2 7/7] ASoC: codecs: aw88261: make volume control usable Val Packett
@ 2026-05-19 16:20   ` Mark Brown
  2026-05-19 22:36     ` Val Packett
  2026-05-22 12:39   ` Luca Weiss
  1 sibling, 1 reply; 15+ messages in thread
From: Mark Brown @ 2026-05-19 16:20 UTC (permalink / raw)
  To: Val Packett
  Cc: Liam Girdwood, Jaroslav Kysela, Takashi Iwai, Weidong Wang,
	~postmarketos/upstreaming, phone-devel, linux-sound,
	linux-kernel

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

On Mon, May 18, 2026 at 06:44:36PM -0300, Val Packett wrote:

> @@ -424,17 +415,7 @@ static int aw88261_dev_reg_update(struct aw88261 *aw88261,
>  			break;
>  	}
>  
> -	ret = aw88261_dev_set_vcalb(aw_dev);
> -	if (ret)
> -		return ret;
> -
> -	if (aw_dev->prof_cur != aw_dev->prof_index)
> -		vol_desc->ctl_volume = 0;
> -
> -	/* keep min volume */
> -	aw88261_dev_set_volume(aw_dev, vol_desc->mute_volume);
> -
> -	return ret;
> +	return aw88261_dev_set_vcalb(aw_dev);
>  }

This looks like it was intentional in order to ensure that things are
muted until we power up (there's an unmute in _dev_start())?

> @@ -983,13 +965,13 @@ static int aw88261_volume_set(struct snd_kcontrol *kcontrol,
>  	struct aw_volume_desc *vol_desc = &aw88261->aw_pa->volume_desc;
>  	struct soc_mixer_control *mc =
>  		(struct soc_mixer_control *)kcontrol->private_value;
> -	int value;
> -
> -	value = ucontrol->value.integer.value[0];
> +	int value = ucontrol->value.integer.value[0];
>  
>  	if (value < mc->min || value > mc->max)
>  		return -EINVAL;
>  
> +	value = AW88261_MUTE_VOL - (value * 2);
> +

We're doing the invert of the value scale before we store it, currently
the default value is defined as 0 in the header so we'll default to
maximum volume which is likely to lead to some distress at some point.
The default should be updated as well.

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 2/7] ASoC: codecs: aw88261: add TDM support
  2026-05-19 16:06   ` Mark Brown
@ 2026-05-19 22:32     ` Val Packett
  2026-05-20 12:18       ` Mark Brown
  0 siblings, 1 reply; 15+ messages in thread
From: Val Packett @ 2026-05-19 22:32 UTC (permalink / raw)
  To: Mark Brown
  Cc: Liam Girdwood, Jaroslav Kysela, Takashi Iwai,
	~postmarketos/upstreaming, phone-devel, linux-sound,
	linux-kernel


On 5/19/26 1:06 PM, Mark Brown wrote:
> On Mon, May 18, 2026 at 06:44:31PM -0300, Val Packett wrote:
>> This amp supports TDM mode, so implement the set_tdm_slot operation to
>> let the SoC driver configure the TDM slot number, width, and masks.
>> +static int aw88261_set_tdm_slot(struct snd_soc_dai *dai,
>> +	unsigned int tx_mask, unsigned int rx_mask, int slots, int slot_width)
>> +{
> In spite of the changelog this doesn't actually seem to use slot_width?
> It should.

Right, I hadn't exactly figured out where that slots in (ha)..

The datasheet seems to say that I2SBCK is slot width, but there are only 
3 options for it (32/48/64) and I currently select the one that matches 
2*physical_width.

Grepping the tree I'm seeing that slot_width is basically the same thing 
as physical width. Should slot_width from set_tdm_slot take precedence 
over the params_physical_width() from hw_params? Should I just have both 
callbacks set the variable and whichever was called last "wins"?

> Note also that we should disable TDM if slots == 0.

This is pretty much already the case.. but I guess I should also reset 
the masks specifically to the i2s default (2nd channel to R). Are masks 
also zero when slots == 0?


Thanks,
~val


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 7/7] ASoC: codecs: aw88261: make volume control usable
  2026-05-19 16:20   ` Mark Brown
@ 2026-05-19 22:36     ` Val Packett
  0 siblings, 0 replies; 15+ messages in thread
From: Val Packett @ 2026-05-19 22:36 UTC (permalink / raw)
  To: Mark Brown
  Cc: Liam Girdwood, Jaroslav Kysela, Takashi Iwai, Weidong Wang,
	~postmarketos/upstreaming, phone-devel, linux-sound,
	linux-kernel


On 5/19/26 1:20 PM, Mark Brown wrote:
> On Mon, May 18, 2026 at 06:44:36PM -0300, Val Packett wrote:
>
>> @@ -424,17 +415,7 @@ static int aw88261_dev_reg_update(struct aw88261 *aw88261,
>>   			break;
>>   	}
>>   
>> -	ret = aw88261_dev_set_vcalb(aw_dev);
>> -	if (ret)
>> -		return ret;
>> -
>> -	if (aw_dev->prof_cur != aw_dev->prof_index)
>> -		vol_desc->ctl_volume = 0;
>> -
>> -	/* keep min volume */
>> -	aw88261_dev_set_volume(aw_dev, vol_desc->mute_volume);
>> -
>> -	return ret;
>> +	return aw88261_dev_set_vcalb(aw_dev);
>>   }
> This looks like it was intentional in order to ensure that things are
> muted until we power up (there's an unmute in _dev_start())?
Sure.. that doesn't seem required but I can keep it to err on the side 
of safety / keeping what works.
>> @@ -983,13 +965,13 @@ static int aw88261_volume_set(struct snd_kcontrol *kcontrol,
>>   	struct aw_volume_desc *vol_desc = &aw88261->aw_pa->volume_desc;
>>   	struct soc_mixer_control *mc =
>>   		(struct soc_mixer_control *)kcontrol->private_value;
>> -	int value;
>> -
>> -	value = ucontrol->value.integer.value[0];
>> +	int value = ucontrol->value.integer.value[0];
>>   
>>   	if (value < mc->min || value > mc->max)
>>   		return -EINVAL;
>>   
>> +	value = AW88261_MUTE_VOL - (value * 2);
>> +
> We're doing the invert of the value scale before we store it, currently
> the default value is defined as 0 in the header so we'll default to
> maximum volume which is likely to lead to some distress at some point.
> The default should be updated as well.

Ack


~val


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 2/7] ASoC: codecs: aw88261: add TDM support
  2026-05-19 22:32     ` Val Packett
@ 2026-05-20 12:18       ` Mark Brown
  0 siblings, 0 replies; 15+ messages in thread
From: Mark Brown @ 2026-05-20 12:18 UTC (permalink / raw)
  To: Val Packett
  Cc: Liam Girdwood, Jaroslav Kysela, Takashi Iwai,
	~postmarketos/upstreaming, phone-devel, linux-sound,
	linux-kernel

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

On Tue, May 19, 2026 at 07:32:33PM -0300, Val Packett wrote:
> On 5/19/26 1:06 PM, Mark Brown wrote:

> > In spite of the changelog this doesn't actually seem to use slot_width?
> > It should.

> Right, I hadn't exactly figured out where that slots in (ha)..

> The datasheet seems to say that I2SBCK is slot width, but there are only 3
> options for it (32/48/64) and I currently select the one that matches
> 2*physical_width.

> Grepping the tree I'm seeing that slot_width is basically the same thing as
> physical width. Should slot_width from set_tdm_slot take precedence over the
> params_physical_width() from hw_params? Should I just have both callbacks
> set the variable and whichever was called last "wins"?

If TDM is configured then hw_params should not cause the number of bits
on the bus to vary at all, the clocking of the bus is fixed by the TDM
configuration.

> > Note also that we should disable TDM if slots == 0.

> This is pretty much already the case.. but I guess I should also reset the
> masks specifically to the i2s default (2nd channel to R). Are masks also
> zero when slots == 0?

They should be.

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 0/7] ASoC: codecs: aw88261: fixes and cleanup
  2026-05-18 21:44 [PATCH v2 0/7] ASoC: codecs: aw88261: fixes and cleanup Val Packett
                   ` (6 preceding siblings ...)
  2026-05-18 21:44 ` [PATCH v2 7/7] ASoC: codecs: aw88261: make volume control usable Val Packett
@ 2026-05-22 12:36 ` Luca Weiss
  7 siblings, 0 replies; 15+ messages in thread
From: Luca Weiss @ 2026-05-22 12:36 UTC (permalink / raw)
  To: Val Packett
  Cc: Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai,
	~postmarketos/upstreaming, phone-devel, linux-sound,
	linux-kernel

Hi Val,

On Mon May 18, 2026 at 11:44 PM CEST, Val Packett wrote:
> The Awinic smart speaker/amp drivers were merged in a very
> "downstream-brained" state, where configuration was only really
> determined by the binary "firmware" (register list) file instead
> of properly participating in the ASoC system. Let's start
> untangling this mess. This series makes aw88261 actually usable
> on devices like fairphone-fp5, motorola-dubai and xiaomi-pipa.

Thanks a lot for working on this!

With this series, and simply hooking up the two amplifiers in dts,
speakers seem to work without any issues, getting rid of the hacks that
I've had in the tree before!

Tested-by: Luca Weiss <luca.weiss@fairphone.com> # qcm6490-fairphone-fp5

Regards
Luca

>
> v2:
> - Add missing sample rates to the switch statement (turns out we do have them)
> - Process the conversion from requested values to register ones directly in
>   the ASoC ops to make sure EINVAL is returned early, from the right place
> - Do not include untested aw88258 fixes.. (why did I think that some support
>   for it was already present in the tree?)
> - Do include TDM support, logspam reduction, a newly discovered mask fix
> - Remove misfeatures inherited from downstream: async start, fade-in/fade-out
>   (hopefully this won't be too controversial?)
> - Also overhaul volume control to make it actually usable as PlaybackVolume
>
> v1: https://lore.kernel.org/all/20260420213250.215465-2-val@packett.cool/
>
> ~val
>
> ---
>
> Val Packett (7):
>   ASoC: codecs: aw88261: support changing sample rate and bit width
>   ASoC: codecs: aw88261: add TDM support
>   ASoC: codecs: aw88261: reduce log spam
>   ASoC: codecs: aw88261: remove fade in/out on start/stop
>   ASoC: codecs: aw88261: remove async start
>   ASoC: codecs: aw88261: fix incorrect masks for boost regs
>   ASoC: codecs: aw88261: make volume control usable
>
>  sound/soc/codecs/aw88261.c | 612 +++++++++++++++++++------------------
>  sound/soc/codecs/aw88261.h | 179 ++++++++++-
>  2 files changed, 486 insertions(+), 305 deletions(-)


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 7/7] ASoC: codecs: aw88261: make volume control usable
  2026-05-18 21:44 ` [PATCH v2 7/7] ASoC: codecs: aw88261: make volume control usable Val Packett
  2026-05-19 16:20   ` Mark Brown
@ 2026-05-22 12:39   ` Luca Weiss
  1 sibling, 0 replies; 15+ messages in thread
From: Luca Weiss @ 2026-05-22 12:39 UTC (permalink / raw)
  To: Val Packett, Liam Girdwood, Mark Brown, Jaroslav Kysela,
	Takashi Iwai, Weidong Wang
  Cc: ~postmarketos/upstreaming, phone-devel, linux-sound, linux-kernel

Hi Val,

On Mon May 18, 2026 at 11:44 PM CEST, Val Packett wrote:
> - Invert the value to match userspace expectations (in the hardware,
>   positive numbers represent negative dB attenuation)
> - Provide TLV metadata for the dB scale (and divide the raw values by 2
>   as the excessive precision used by HW is not representable in TLV)
> - Do not unnecessarily reset the volume while switching profiles
> - Simplify aw88261_dev_set_volume using regmap_update_bits
> - Do not add the initial volume from the profile to the requested volume
>   as that would throw off the dB mapping (if a lower max limit is
>   desired, it can be set in the UCM profile in userspace)
>
> With this change, it's actually possible to use this hardware volume
> control as PlaybackVolume in an ALSA UCM profile.
>
> Fixes: 028a2ae25691 ("ASoC: codecs: Add aw88261 amplifier driver")
> Signed-off-by: Val Packett <val@packett.cool>
> ---
>  sound/soc/codecs/aw88261.c | 52 +++++++++++++++-----------------------
>  sound/soc/codecs/aw88261.h |  1 -
>  2 files changed, 21 insertions(+), 32 deletions(-)
>
> diff --git a/sound/soc/codecs/aw88261.c b/sound/soc/codecs/aw88261.c
> index adc728e45f57..0e6b2dfe5db9 100644
> --- a/sound/soc/codecs/aw88261.c
> +++ b/sound/soc/codecs/aw88261.c
> @@ -15,6 +15,7 @@
>  #include <linux/regulator/consumer.h>
>  #include <sound/soc.h>
>  #include <sound/pcm_params.h>
> +#include <sound/tlv.h>
>  #include "aw88261.h"
>  #include "aw88395/aw88395_data_type.h"
>  #include "aw88395/aw88395_device.h"
> @@ -29,20 +30,10 @@ static const struct regmap_config aw88261_remap_config = {
>  
>  static void aw88261_dev_set_volume(struct aw_device *aw_dev, unsigned int value)
>  {
> -	struct aw_volume_desc *vol_desc = &aw_dev->volume_desc;
> -	unsigned int real_value, volume;
> -	unsigned int reg_value;
> +	unsigned int volume = min(value, (unsigned int)AW88261_MUTE_VOL);
>  
> -	volume = min((value + vol_desc->init_volume), (unsigned int)AW88261_MUTE_VOL);
> -	real_value = DB_TO_REG_VAL(volume);
> -
> -	regmap_read(aw_dev->regmap, AW88261_SYSCTRL2_REG, &reg_value);
> -
> -	real_value = (real_value | (reg_value & AW88261_VOL_START_MASK));
> -
> -	dev_dbg(aw_dev->dev, "value 0x%x , real_value:0x%x", value, real_value);
> -
> -	regmap_write(aw_dev->regmap, AW88261_SYSCTRL2_REG, real_value);
> +	regmap_update_bits(aw_dev->regmap, AW88261_SYSCTRL2_REG,
> +		~AW88261_VOL_MASK, DB_TO_REG_VAL(volume));
>  }
>  
>  static void aw88261_dev_i2s_tx_enable(struct aw_device *aw_dev, bool flag)
> @@ -424,17 +415,7 @@ static int aw88261_dev_reg_update(struct aw88261 *aw88261,
>  			break;
>  	}
>  
> -	ret = aw88261_dev_set_vcalb(aw_dev);
> -	if (ret)
> -		return ret;
> -
> -	if (aw_dev->prof_cur != aw_dev->prof_index)
> -		vol_desc->ctl_volume = 0;
> -
> -	/* keep min volume */
> -	aw88261_dev_set_volume(aw_dev, vol_desc->mute_volume);
> -
> -	return ret;
> +	return aw88261_dev_set_vcalb(aw_dev);

These removals cause the following compile warning, when applied on
linux-next:

sound/soc/codecs/aw88261.c: In function 'aw88261_dev_reg_update':
sound/soc/codecs/aw88261.c:347:32: warning: unused variable 'vol_desc' [-Wunused-variable]
  347 |         struct aw_volume_desc *vol_desc = &aw_dev->volume_desc;
      |                                ^~~~~~~~

Regards
Luca

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2026-05-22 12:39 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-05-18 21:44 [PATCH v2 0/7] ASoC: codecs: aw88261: fixes and cleanup Val Packett
2026-05-18 21:44 ` [PATCH v2 1/7] ASoC: codecs: aw88261: support changing sample rate and bit width Val Packett
2026-05-18 21:44 ` [PATCH v2 2/7] ASoC: codecs: aw88261: add TDM support Val Packett
2026-05-19 16:06   ` Mark Brown
2026-05-19 22:32     ` Val Packett
2026-05-20 12:18       ` Mark Brown
2026-05-18 21:44 ` [PATCH v2 3/7] ASoC: codecs: aw88261: reduce log spam Val Packett
2026-05-18 21:44 ` [PATCH v2 4/7] ASoC: codecs: aw88261: remove fade in/out on start/stop Val Packett
2026-05-18 21:44 ` [PATCH v2 5/7] ASoC: codecs: aw88261: remove async start Val Packett
2026-05-18 21:44 ` [PATCH v2 6/7] ASoC: codecs: aw88261: fix incorrect masks for boost regs Val Packett
2026-05-18 21:44 ` [PATCH v2 7/7] ASoC: codecs: aw88261: make volume control usable Val Packett
2026-05-19 16:20   ` Mark Brown
2026-05-19 22:36     ` Val Packett
2026-05-22 12:39   ` Luca Weiss
2026-05-22 12:36 ` [PATCH v2 0/7] ASoC: codecs: aw88261: fixes and cleanup Luca Weiss

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®