[PATCH 0/6] ASoC: codecs: aw88166: fixes and cleanup

[PATCH 0/6] ASoC: codecs: aw88166: fixes and cleanup

[Aaron Kling via B4 Relay]

The driver was merged in a very downstream-centric manner. This series
copies the changes from Val Packetts series [0] cleaning up the aw88261
driver. This makes the driver notably more usable on the AYN and Retroid
Pocket QCS8550 devices, as well as the AYN Odin 3.

[0] https://lore.kernel.org/linux-sound/20260529200550.529719-1-val@packett.cool/

---
Aaron Kling (6):
      ASoC: codecs: aw88166: support changing sample rate and bit width
      ASoC: codecs: aw88166: add TDM support
      ASoC: codecs: aw88166: reduce log spam
      ASoC: codecs: aw88166: remove fade in/out on start/stop
      ASoC: codecs: aw88166: remove async start
      ASoC: codecs: aw88166: make volume control usable

 sound/soc/codecs/aw88166.c | 590 +++++++++++++++++++++++++--------------------
 sound/soc/codecs/aw88166.h | 180 +++++++++++++-
 2 files changed, 501 insertions(+), 269 deletions(-)
---
base-commit: 4c253ac4b29b8c6cc6fdef8f92d4facde62e63b9
change-id: 20260925-aw88166-cleanup-d5a141be651a

Best regards,
-- 
Aaron Kling <webgeek1234@gmail.com>

[PATCH 1/6] ASoC: codecs: aw88166: support changing sample rate and bit width

[Aaron Kling via B4 Relay]

From: Aaron Kling <webgeek1234@gmail.com>

The aw88166 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: 94e412c28e61 ("ASoC: codecs: Add aw88166 amplifier driver")
Signed-off-by: Aaron Kling <webgeek1234@gmail.com>
---
 sound/soc/codecs/aw88166.c | 239 ++++++++++++++++++++++++++++++++++-----------
 sound/soc/codecs/aw88166.h | 108 +++++++++++++++++++-
 2 files changed, 291 insertions(+), 56 deletions(-)

diff --git a/sound/soc/codecs/aw88166.c b/sound/soc/codecs/aw88166.c
index d2a138211c596..89608118cac6c 100644
--- a/sound/soc/codecs/aw88166.c
+++ b/sound/soc/codecs/aw88166.c
@@ -15,6 +15,7 @@
 #include <linux/minmax.h>
 #include <linux/regmap.h>
 #include <sound/soc.h>
+#include <sound/pcm_params.h>
 #include "aw88166.h"
 #include "aw88395/aw88395_device.h"
 
@@ -31,6 +32,14 @@ struct aw88166 {
 	unsigned int vcalb_init_val;
 	unsigned int re_init_val;
 	unsigned int dither_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;
 };
 
@@ -99,7 +108,7 @@ static int aw_dev_get_iis_status(struct aw_device *aw_dev)
 	return 0;
 }
 
-static int aw_dev_check_mode1_pll(struct aw_device *aw_dev)
+static int aw88166_dev_check_pll(struct aw_device *aw_dev)
 {
 	int ret, i;
 
@@ -116,71 +125,48 @@ static int aw_dev_check_mode1_pll(struct aw_device *aw_dev)
 	return -EPERM;
 }
 
-static int aw_dev_check_mode2_pll(struct aw_device *aw_dev)
+static int aw88166_dev_configure_syspll(struct aw88166 *aw88261)
 {
-	unsigned int reg_val;
-	int ret, i;
+	struct aw_device *aw_dev = aw88166->aw_pa;
+	int ret;
 
-	ret = regmap_read(aw_dev->regmap, AW88166_PLLCTRL2_REG, &reg_val);
+	/* PLL divider must be used for 8/16/32 kHz modes */
+	ret = regmap_update_bits(aw_dev->regmap, AW88166_PLLCTRL1_REG,
+			~AW88166_CCO_MUX_MASK, aw88166->cco_mux_value);
 	if (ret)
 		return ret;
 
-	reg_val &= (~AW88166_CCO_MUX_MASK);
-	if (reg_val == AW88166_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, AW88166_PLLCTRL2_REG,
-			~AW88166_CCO_MUX_MASK, AW88166_CCO_MUX_DIVIDED_VALUE);
+	/* The word clock (WCK) defines the beginning of a frame */
+	ret = regmap_update_bits(aw_dev->regmap, AW88166_I2SCTRL1_REG,
+			~AW88166_I2SSR_MASK, aw88166->sr_value);
 	if (ret)
 		return ret;
 
-	for (i = 0; i < AW88166_DEV_SYSST_CHECK_MAX; i++) {
-		ret = aw_dev_get_iis_status(aw_dev);
-		if (ret) {
-			dev_err(aw_dev->dev, "mode2 iis signal check error");
-			usleep_range(AW88166_2000_US, AW88166_2000_US + 10);
-		} else {
-			break;
-		}
-	}
-
-	/* change mode1 */
-	regmap_update_bits(aw_dev->regmap, AW88166_PLLCTRL2_REG,
-			~AW88166_CCO_MUX_MASK, AW88166_CCO_MUX_BYPASS_VALUE);
-	if (ret == 0) {
-		usleep_range(AW88166_2000_US, AW88166_2000_US + 10);
-		for (i = 0; i < AW88166_DEV_SYSST_CHECK_MAX; i++) {
-			ret = aw_dev_get_iis_status(aw_dev);
-			if (ret) {
-				dev_err(aw_dev->dev, "mode2 switch to mode1, iis signal check error");
-				usleep_range(AW88166_2000_US, AW88166_2000_US + 10);
-			} else {
-				break;
-			}
-		}
-	}
+	/* The bit clock (BCK) defines the length of a frame */
+	ret = regmap_update_bits(aw_dev->regmap, AW88166_I2SCTRL1_REG,
+			~AW88166_I2SBCK_MASK, aw88166->bck_value);
+	if (ret)
+		return ret;
 
-	return ret;
-}
+	/* The logical frame size is the width of data for 1 slot */
+	ret = regmap_update_bits(aw_dev->regmap, AW88166_I2SCTRL1_REG,
+			~AW88166_I2SFS_MASK, aw88166->fs_value);
+	if (ret)
+		return ret;
 
-static int aw_dev_check_syspll(struct aw_device *aw_dev)
-{
-	int ret;
+	/* The I2S interface mode (Philips standard, LSB/MSB justified) */
+	ret = regmap_update_bits(aw_dev->regmap, AW88166_I2SCTRL1_REG,
+			~AW88166_I2SMD_MASK, aw88166->md_value);
+	if (ret)
+		return ret;
 
-	ret = aw_dev_check_mode1_pll(aw_dev);
-	if (ret) {
-		dev_dbg(aw_dev->dev, "mode1 check iis failed try switch to mode2 check");
-		ret = aw_dev_check_mode2_pll(aw_dev);
-		if (ret) {
-			dev_err(aw_dev->dev, "mode2 check iis failed");
-			return ret;
-		}
-	}
+	/* The polarity of the bit clock (BCK) */
+	ret = regmap_update_bits(aw_dev->regmap, AW88166_SYSCTRL_REG,
+			~AW88166_BCKINV_MASK, aw88166->bck_inv_value);
+	if (ret)
+		return ret;
 
-	return 0;
+	return aw88166_dev_check_pll(aw_dev);
 }
 
 static int aw_dev_check_sysst(struct aw_device *aw_dev)
@@ -733,7 +719,7 @@ static int aw88166_dev_start(struct aw88166 *aw88166)
 	aw_dev_pwd(aw_dev, false);
 	usleep_range(AW88166_2000_US, AW88166_2000_US + 10);
 
-	ret = aw_dev_check_syspll(aw_dev);
+	ret = aw88166_dev_configure_syspll(aw88166);
 	if (ret) {
 		dev_err(aw_dev->dev, "pll check failed cannot start\n");
 		goto pll_check_fail;
@@ -1254,6 +1240,140 @@ static int aw88166_stop(struct aw_device *aw_dev)
 	return 0;
 }
 
+static int aw88166_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
+{
+	struct snd_soc_component *component = dai->component;
+	struct aw88166 *aw88166 = snd_soc_component_get_drvdata(component);
+
+	switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
+	case SND_SOC_DAIFMT_NB_NF:
+		aw88166->bck_inv_value = AW88166_BCKINV_NOT_INVERT_VALUE;
+		break;
+	case SND_SOC_DAIFMT_IB_NF:
+		aw88166->bck_inv_value = AW88166_BCKINV_INVERTED_VALUE;
+		break;
+	default:
+		dev_err(aw88166->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:
+		aw88166->md_value = AW88166_I2SMD_PHILIPS_STANDARD_VALUE;
+		break;
+	case SND_SOC_DAIFMT_MSB:
+		aw88166->md_value = AW88166_I2SMD_MSB_JUSTIFIED_VALUE;
+		break;
+	case SND_SOC_DAIFMT_LSB:
+		aw88166->md_value = AW88166_I2SMD_LSB_JUSTIFIED_VALUE;
+		break;
+	default:
+		dev_err(aw88166->aw_pa->dev, "unsupported DAI format 0x%x\n",
+			fmt & SND_SOC_DAIFMT_FORMAT_MASK);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int aw88166_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 aw88166 *aw88166 = snd_soc_component_get_drvdata(component);
+
+	if (substream->stream == SNDRV_PCM_STREAM_CAPTURE)
+		return 0;
+
+	aw88166->cco_mux_value = AW88166_CCO_MUX_BYPASS_VALUE;
+	switch (params_rate(params)) {
+	case 8000:
+		aw88166->sr_value = AW88166_I2SSR_8KHZ_VALUE;
+		aw88166->cco_mux_value = AW88166_CCO_MUX_DIVIDED_VALUE;
+		break;
+	case 11025:
+		aw88166->sr_value = AW88166_I2SSR_11P025KHZ_VALUE;
+		break;
+	case 12000:
+		aw88166->sr_value = AW88166_I2SSR_12KHZ_VALUE;
+		break;
+	case 16000:
+		aw88166->sr_value = AW88166_I2SSR_16KHZ_VALUE;
+		aw88166->cco_mux_value = AW88166_CCO_MUX_DIVIDED_VALUE;
+		break;
+	case 22050:
+		aw88166->sr_value = AW88166_I2SSR_22P05KHZ_VALUE;
+		break;
+	case 24000:
+		aw88166->sr_value = AW88166_I2SSR_24KHZ_VALUE;
+		break;
+	case 32000:
+		aw88166->sr_value = AW88166_I2SSR_32KHZ_VALUE;
+		aw88166->cco_mux_value = AW88166_CCO_MUX_DIVIDED_VALUE;
+		break;
+	case 44100:
+		aw88166->sr_value = AW88166_I2SSR_44P1KHZ_VALUE;
+		break;
+	case 48000:
+		aw88166->sr_value = AW88166_I2SSR_48KHZ_VALUE;
+		break;
+	case 96000:
+		aw88166->sr_value = AW88166_I2SSR_96KHZ_VALUE;
+		break;
+	case 192000:
+		aw88166->sr_value = AW88166_I2SSR_192KHZ_VALUE;
+		break;
+	default:
+		dev_err(aw88166->aw_pa->dev, "unsupported sample rate %d\n",
+			params_rate(params));
+		return -EINVAL;
+	}
+
+	switch (params_width(params)) {
+	case 16:
+		aw88166->fs_value = AW88166_I2SFS_16_BITS_VALUE;
+		break;
+	case 20:
+		aw88166->fs_value = AW88166_I2SFS_20_BITS_VALUE;
+		break;
+	case 24:
+		aw88166->fs_value = AW88166_I2SFS_24_BITS_VALUE;
+		break;
+	case 32:
+		aw88166->fs_value = AW88166_I2SFS_32_BITS_VALUE;
+		break;
+	default:
+		dev_err(aw88166->aw_pa->dev, "unsupported bit width %d\n",
+			params_width(params));
+		return -EINVAL;
+	}
+
+	switch (params_physical_width(params)) {
+	case 16:
+		aw88166->bck_value = AW88166_I2SBCK_32FS_VALUE;
+		break;
+	case 24:
+		aw88166->bck_value = AW88166_I2SBCK_48FS_VALUE;
+		break;
+	case 32:
+		aw88166->bck_value = AW88166_I2SBCK_64FS_VALUE;
+		break;
+	default:
+		dev_err(aw88166->aw_pa->dev, "unsupported physical bit width %d\n",
+			params_physical_width(params));
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static const struct snd_soc_dai_ops aw88166_dai_ops = {
+	.set_fmt = aw88166_set_fmt,
+	.hw_params = aw88166_hw_params,
+};
+
 static struct snd_soc_dai_driver aw88166_dai[] = {
 	{
 		.name = "aw88166-aif",
@@ -1272,6 +1392,7 @@ static struct snd_soc_dai_driver aw88166_dai[] = {
 			.rates = AW88166_RATES,
 			.formats = AW88166_FORMATS,
 		},
+		.ops = &aw88166_dai_ops,
 	},
 };
 
@@ -1769,6 +1890,14 @@ static int aw88166_i2c_probe(struct i2c_client *i2c)
 	if (!aw88166)
 		return -ENOMEM;
 
+	/* set defaults */
+	aw88166->sr_value = AW88166_I2SSR_48KHZ_VALUE;
+	aw88166->cco_mux_value = AW88166_CCO_MUX_BYPASS_VALUE;
+	aw88166->fs_value = AW88166_I2SFS_24_BITS_VALUE;
+	aw88166->bck_value = AW88166_I2SBCK_64FS_VALUE;
+	aw88166->bck_inv_value = AW88166_BCKINV_NOT_INVERT_VALUE;
+	aw88166->md_value = AW88166_I2SMD_PHILIPS_STANDARD_VALUE;
+
 	mutex_init(&aw88166->lock);
 
 	i2c_set_clientdata(i2c, aw88166);
diff --git a/sound/soc/codecs/aw88166.h b/sound/soc/codecs/aw88166.h
index 9f3f47a7003e2..0aa92b3b6c3b8 100644
--- a/sound/soc/codecs/aw88166.h
+++ b/sound/soc/codecs/aw88166.h
@@ -244,6 +244,19 @@
 #define AW88166_RCV_MODE_RECEIVER_VALUE	\
 	(AW88166_RCV_MODE_RECEIVER << AW88166_RCV_MODE_START_BIT)
 
+#define AW88166_BCKINV_START_BIT	(4)
+#define AW88166_BCKINV_BITS_LEN	(1)
+#define AW88166_BCKINV_MASK	\
+	(~(((1<<AW88166_BCKINV_BITS_LEN)-1) << AW88166_BCKINV_START_BIT))
+
+#define AW88166_BCKINV_NOT_INVERT	(0)
+#define AW88166_BCKINV_NOT_INVERT_VALUE	\
+	(AW88166_BCKINV_NOT_INVERT << AW88166_BCKINV_START_BIT)
+
+#define AW88166_BCKINV_INVERTED	(1)
+#define AW88166_BCKINV_INVERTED_VALUE	\
+	(AW88166_BCKINV_INVERTED << AW88166_BCKINV_START_BIT)
+
 #define AW88166_AMPPD_START_BIT	(1)
 #define AW88166_AMPPD_BITS_LEN		(1)
 #define AW88166_AMPPD_MASK		\
@@ -372,7 +385,98 @@
 		AW88166_PLLS_LOCKED_VALUE | \
 		AW88166_SWS_SWITCHING_VALUE)
 
-#define AW88166_CCO_MUX_START_BIT	(14)
+#define AW88166_I2SMD_START_BIT	(8)
+#define AW88166_I2SMD_BITS_LEN	(2)
+#define AW88166_I2SMD_MASK	\
+	(~(((1<<AW88166_I2SMD_BITS_LEN)-1) << AW88166_I2SMD_START_BIT))
+
+#define AW88166_I2SMD_PHILIPS_STANDARD	(0)
+#define AW88166_I2SMD_PHILIPS_STANDARD_VALUE	\
+	(AW88166_I2SMD_PHILIPS_STANDARD << AW88166_I2SMD_START_BIT)
+
+#define AW88166_I2SMD_MSB_JUSTIFIED	(1)
+#define AW88166_I2SMD_MSB_JUSTIFIED_VALUE	\
+	(AW88166_I2SMD_MSB_JUSTIFIED << AW88166_I2SMD_START_BIT)
+
+#define AW88166_I2SMD_LSB_JUSTIFIED	(2)
+#define AW88166_I2SMD_LSB_JUSTIFIED_VALUE	\
+	(AW88166_I2SMD_LSB_JUSTIFIED << AW88166_I2SMD_START_BIT)
+
+#define AW88166_I2SFS_START_BIT			(6)
+#define AW88166_I2SFS_BITS_LEN			(2)
+#define AW88166_I2SFS_MASK				\
+	(~(((1<<AW88166_I2SFS_BITS_LEN)-1)<<AW88166_I2SFS_START_BIT))
+
+#define AW88166_I2SFS_16_BITS			(0)
+#define AW88166_I2SFS_16_BITS_VALUE		\
+	(AW88166_I2SFS_16_BITS << AW88166_I2SFS_START_BIT)
+#define AW88166_I2SFS_20_BITS			(1)
+#define AW88166_I2SFS_20_BITS_VALUE		\
+	(AW88166_I2SFS_20_BITS << AW88166_I2SFS_START_BIT)
+#define AW88166_I2SFS_24_BITS			(2)
+#define AW88166_I2SFS_24_BITS_VALUE		\
+	(AW88166_I2SFS_24_BITS << AW88166_I2SFS_START_BIT)
+#define AW88166_I2SFS_32_BITS			(3)
+#define AW88166_I2SFS_32_BITS_VALUE		\
+	(AW88166_I2SFS_32_BITS << AW88166_I2SFS_START_BIT)
+
+#define AW88166_I2SBCK_START_BIT	(4)
+#define AW88166_I2SBCK_BITS_LEN	(2)
+#define AW88166_I2SBCK_MASK	\
+	(~(((1<<AW88166_I2SBCK_BITS_LEN)-1) << AW88166_I2SBCK_START_BIT))
+
+#define AW88166_I2SBCK_32FS	(0)
+#define AW88166_I2SBCK_32FS_VALUE	\
+	(AW88166_I2SBCK_32FS << AW88166_I2SBCK_START_BIT)
+
+#define AW88166_I2SBCK_48FS	(1)
+#define AW88166_I2SBCK_48FS_VALUE	\
+	(AW88166_I2SBCK_48FS << AW88166_I2SBCK_START_BIT)
+
+#define AW88166_I2SBCK_64FS	(2)
+#define AW88166_I2SBCK_64FS_VALUE	\
+	(AW88166_I2SBCK_64FS << AW88166_I2SBCK_START_BIT)
+
+#define AW88166_I2SSR_START_BIT			(0)
+#define AW88166_I2SSR_BITS_LEN			(4)
+#define AW88166_I2SSR_MASK				\
+	(~(((1<<AW88166_I2SSR_BITS_LEN)-1) << AW88166_I2SSR_START_BIT))
+
+#define AW88166_I2SSR_8KHZ				(0)
+#define AW88166_I2SSR_8KHZ_VALUE		\
+	(AW88166_I2SSR_8KHZ << AW88166_I2SSR_START_BIT)
+#define AW88166_I2SSR_11P025KHZ			(1)
+#define AW88166_I2SSR_11P025KHZ_VALUE	\
+	(AW88166_I2SSR_11P025KHZ << AW88166_I2SSR_START_BIT)
+#define AW88166_I2SSR_12KHZ				(2)
+#define AW88166_I2SSR_12KHZ_VALUE		\
+	(AW88166_I2SSR_12KHZ << AW88166_I2SSR_START_BIT)
+#define AW88166_I2SSR_16KHZ				(3)
+#define AW88166_I2SSR_16KHZ_VALUE		\
+	(AW88166_I2SSR_16KHZ << AW88166_I2SSR_START_BIT)
+#define AW88166_I2SSR_22P05KHZ			(4)
+#define AW88166_I2SSR_22P05KHZ_VALUE	\
+	(AW88166_I2SSR_22P05KHZ << AW88166_I2SSR_START_BIT)
+#define AW88166_I2SSR_24KHZ				(5)
+#define AW88166_I2SSR_24KHZ_VALUE		\
+	(AW88166_I2SSR_24KHZ << AW88166_I2SSR_START_BIT)
+#define AW88166_I2SSR_32KHZ				(6)
+#define AW88166_I2SSR_32KHZ_VALUE		\
+	(AW88166_I2SSR_32KHZ << AW88166_I2SSR_START_BIT)
+#define AW88166_I2SSR_44P1KHZ			(7)
+#define AW88166_I2SSR_44P1KHZ_VALUE	\
+	(AW88166_I2SSR_44P1KHZ << AW88166_I2SSR_START_BIT)
+#define AW88166_I2SSR_48KHZ				(8)
+#define AW88166_I2SSR_48KHZ_VALUE		\
+	(AW88166_I2SSR_48KHZ << AW88166_I2SSR_START_BIT)
+#define AW88166_I2SSR_96KHZ				(9)
+#define AW88166_I2SSR_96KHZ_VALUE		\
+	(AW88166_I2SSR_96KHZ << AW88166_I2SSR_START_BIT)
+#define AW88166_I2SSR_192KHZ			(10)
+#define AW88166_I2SSR_192KHZ_VALUE		\
+	(AW88166_I2SSR_192KHZ << AW88166_I2SSR_START_BIT)
+
+#define AW88166_CCO_MUX_START_BIT	(8)
 #define AW88166_CCO_MUX_BITS_LEN	(1)
 #define AW88166_CCO_MUX_MASK		\
 	(~(((1<<AW88166_CCO_MUX_BITS_LEN)-1) << AW88166_CCO_MUX_START_BIT))
@@ -460,6 +564,8 @@
 #define AW88166_ACF_FILE			"aw88166_acf.bin"
 
 #define AW88166_RATES (SNDRV_PCM_RATE_8000_48000 | \
+			SNDRV_PCM_RATE_12000 | \
+			SNDRV_PCM_RATE_24000 | \
 			SNDRV_PCM_RATE_96000)
 #define AW88166_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | \
 			SNDRV_PCM_FMTBIT_S24_LE | \

-- 
2.54.0

[PATCH 2/6] ASoC: codecs: aw88166: add TDM support

[Aaron Kling via B4 Relay]

From: Aaron Kling <webgeek1234@gmail.com>

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: Aaron Kling <webgeek1234@gmail.com>
---
 sound/soc/codecs/aw88166.c | 123 ++++++++++++++++++++++++++++++++++++++++++++-
 sound/soc/codecs/aw88166.h |  50 ++++++++++++++++++
 2 files changed, 171 insertions(+), 2 deletions(-)

diff --git a/sound/soc/codecs/aw88166.c b/sound/soc/codecs/aw88166.c
index 89608118cac6c..cd8773e316b90 100644
--- a/sound/soc/codecs/aw88166.c
+++ b/sound/soc/codecs/aw88166.c
@@ -7,6 +7,7 @@
 // Author: Weidong Wang <wangweidong.a@awinic.com>
 //
 
+#include <linux/bitops.h>
 #include <linux/cleanup.h>
 #include <linux/crc32.h>
 #include <linux/firmware.h>
@@ -38,8 +39,14 @@ struct aw88166 {
 	unsigned int fs_value;
 	unsigned int bck_value;
 	unsigned int bck_inv_value;
+	unsigned int tdm_bck_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;
 };
 
@@ -125,11 +132,35 @@ static int aw88166_dev_check_pll(struct aw_device *aw_dev)
 	return -EPERM;
 }
 
-static int aw88166_dev_configure_syspll(struct aw88166 *aw88261)
+static int aw88166_dev_configure_syspll(struct aw88166 *aw88166)
 {
 	struct aw_device *aw_dev = aw88166->aw_pa;
 	int ret;
 
+	/* Configure TDM slots (I2S is represented as no slots) */
+	ret = regmap_update_bits(aw_dev->regmap, AW88166_I2SCTRL2_REG,
+			~AW88166_SLOT_NUM_MASK, aw88166->slot_num_value);
+	if (ret)
+		return ret;
+
+	ret = regmap_update_bits(aw_dev->regmap, AW88166_I2SCTRL2_REG,
+			~AW88166_I2S_TX_SLOTVLD_MASK,
+			aw88166->tx_slotvld_mask);
+	if (ret)
+		return ret;
+
+	ret = regmap_update_bits(aw_dev->regmap, AW88166_I2SCTRL2_REG,
+			~AW88166_I2S_RXL_SLOTVLD_MASK,
+			aw88166->rxl_slotvld_mask);
+	if (ret)
+		return ret;
+
+	ret = regmap_update_bits(aw_dev->regmap, AW88166_I2SCTRL2_REG,
+			~AW88166_I2S_RXR_SLOTVLD_MASK,
+			aw88166->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, AW88166_PLLCTRL1_REG,
 			~AW88166_CCO_MUX_MASK, aw88166->cco_mux_value);
@@ -144,7 +175,9 @@ static int aw88166_dev_configure_syspll(struct aw88166 *aw88261)
 
 	/* The bit clock (BCK) defines the length of a frame */
 	ret = regmap_update_bits(aw_dev->regmap, AW88166_I2SCTRL1_REG,
-			~AW88166_I2SBCK_MASK, aw88166->bck_value);
+			~AW88166_I2SBCK_MASK,
+			(aw88166->tdm_bck_value != AW88166_TDM_BCK_UNSET)
+			? aw88166->tdm_bck_value : aw88166->bck_value);
 	if (ret)
 		return ret;
 
@@ -1260,9 +1293,11 @@ static int aw88166_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:
 		aw88166->md_value = AW88166_I2SMD_PHILIPS_STANDARD_VALUE;
 		break;
 	case SND_SOC_DAIFMT_MSB:
+	case SND_SOC_DAIFMT_DSP_B:
 		aw88166->md_value = AW88166_I2SMD_MSB_JUSTIFIED_VALUE;
 		break;
 	case SND_SOC_DAIFMT_LSB:
@@ -1369,9 +1404,90 @@ static int aw88166_hw_params(struct snd_pcm_substream *substream,
 	return 0;
 }
 
+static int aw88166_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 aw88166 *aw88166 = snd_soc_component_get_drvdata(component);
+	int chan;
+
+	switch (slots) {
+	case 0:
+		/* Just reset everything TDM related to I2S values */
+		aw88166->slot_num_value = AW88166_SLOT_NUM_I2S_MODE_VALUE;
+		aw88166->tdm_bck_value = AW88166_TDM_BCK_UNSET;
+		aw88166->tx_slotvld_mask = 0 << AW88166_I2S_TX_SLOTVLD_START_BIT;
+		aw88166->rxl_slotvld_mask = 0 << AW88166_I2S_RXL_SLOTVLD_START_BIT;
+		aw88166->rxr_slotvld_mask = 1 << AW88166_I2S_RXR_SLOTVLD_START_BIT;
+		return 0;
+	case 1:
+		aw88166->slot_num_value = AW88166_SLOT_NUM_TDM1S_VALUE;
+		break;
+	case 2:
+		aw88166->slot_num_value = AW88166_SLOT_NUM_TDM2S_VALUE;
+		break;
+	case 4:
+		aw88166->slot_num_value = AW88166_SLOT_NUM_TDM4S_VALUE;
+		break;
+	case 6:
+		aw88166->slot_num_value = AW88166_SLOT_NUM_TDM6S_VALUE;
+		break;
+	case 8:
+		aw88166->slot_num_value = AW88166_SLOT_NUM_TDM8S_VALUE;
+		break;
+	case 16:
+		aw88166->slot_num_value = AW88166_SLOT_NUM_TDM16S_VALUE;
+		break;
+	default:
+		dev_err(aw88166->aw_pa->dev, "unsupported slot count %d\n", slots);
+		return -EINVAL;
+	}
+
+	switch (slot_width) {
+	case 16:
+		aw88166->tdm_bck_value = AW88166_I2SBCK_32FS_VALUE;
+		break;
+	case 20:
+	case 24:
+		aw88166->tdm_bck_value = AW88166_I2SBCK_48FS_VALUE;
+		break;
+	case 32:
+		aw88166->tdm_bck_value = AW88166_I2SBCK_64FS_VALUE;
+		break;
+	default:
+		dev_err(aw88166->aw_pa->dev, "unsupported slot width %d\n",
+			slot_width);
+		return -EINVAL;
+	}
+
+	if (tx_mask != 0) {
+		if ((chan = __ffs(tx_mask)) > 16)
+			return -EINVAL;
+
+		aw88166->tx_slotvld_mask = chan << AW88166_I2S_TX_SLOTVLD_START_BIT;
+	}
+
+	if (rx_mask != 0) {
+		if ((chan = __ffs(rx_mask)) > 16)
+			return -EINVAL;
+
+		aw88166->rxl_slotvld_mask = chan << AW88166_I2S_RXL_SLOTVLD_START_BIT;
+	}
+
+	if ((rx_mask & ~BIT(chan)) != 0) {
+		if ((chan = __ffs(rx_mask & ~BIT(chan))) > 16)
+			return -EINVAL;
+
+		aw88166->rxr_slotvld_mask = chan << AW88166_I2S_RXR_SLOTVLD_START_BIT;
+	}
+
+	return 0;
+}
+
 static const struct snd_soc_dai_ops aw88166_dai_ops = {
 	.set_fmt = aw88166_set_fmt,
 	.hw_params = aw88166_hw_params,
+	.set_tdm_slot = aw88166_set_tdm_slot,
 };
 
 static struct snd_soc_dai_driver aw88166_dai[] = {
@@ -1891,12 +2007,15 @@ static int aw88166_i2c_probe(struct i2c_client *i2c)
 		return -ENOMEM;
 
 	/* set defaults */
+	aw88166->slot_num_value = AW88166_SLOT_NUM_I2S_MODE_VALUE;
 	aw88166->sr_value = AW88166_I2SSR_48KHZ_VALUE;
 	aw88166->cco_mux_value = AW88166_CCO_MUX_BYPASS_VALUE;
 	aw88166->fs_value = AW88166_I2SFS_24_BITS_VALUE;
 	aw88166->bck_value = AW88166_I2SBCK_64FS_VALUE;
 	aw88166->bck_inv_value = AW88166_BCKINV_NOT_INVERT_VALUE;
+	aw88166->tdm_bck_value = AW88166_TDM_BCK_UNSET;
 	aw88166->md_value = AW88166_I2SMD_PHILIPS_STANDARD_VALUE;
+	aw88166->rxr_slotvld_mask = 1 << AW88166_I2S_RXR_SLOTVLD_START_BIT;
 
 	mutex_init(&aw88166->lock);
 
diff --git a/sound/soc/codecs/aw88166.h b/sound/soc/codecs/aw88166.h
index 0aa92b3b6c3b8..377a733447e70 100644
--- a/sound/soc/codecs/aw88166.h
+++ b/sound/soc/codecs/aw88166.h
@@ -437,6 +437,8 @@
 #define AW88166_I2SBCK_64FS_VALUE	\
 	(AW88166_I2SBCK_64FS << AW88166_I2SBCK_START_BIT)
 
+#define AW88166_TDM_BCK_UNSET			UINT_MAX
+
 #define AW88166_I2SSR_START_BIT			(0)
 #define AW88166_I2SSR_BITS_LEN			(4)
 #define AW88166_I2SSR_MASK				\
@@ -476,6 +478,54 @@
 #define AW88166_I2SSR_192KHZ_VALUE		\
 	(AW88166_I2SSR_192KHZ << AW88166_I2SSR_START_BIT)
 
+#define AW88166_SLOT_NUM_START_BIT	(12)
+#define AW88166_SLOT_NUM_BITS_LEN	(3)
+#define AW88166_SLOT_NUM_MASK	\
+	(~(((1<<AW88166_SLOT_NUM_BITS_LEN)-1) << AW88166_SLOT_NUM_START_BIT))
+
+#define AW88166_SLOT_NUM_I2S_MODE	(0)
+#define AW88166_SLOT_NUM_I2S_MODE_VALUE	\
+	(AW88166_SLOT_NUM_I2S_MODE << AW88166_SLOT_NUM_START_BIT)
+
+#define AW88166_SLOT_NUM_TDM1S	(1)
+#define AW88166_SLOT_NUM_TDM1S_VALUE	\
+	(AW88166_SLOT_NUM_TDM1S << AW88166_SLOT_NUM_START_BIT)
+
+#define AW88166_SLOT_NUM_TDM2S	(2)
+#define AW88166_SLOT_NUM_TDM2S_VALUE	\
+	(AW88166_SLOT_NUM_TDM2S << AW88166_SLOT_NUM_START_BIT)
+
+#define AW88166_SLOT_NUM_TDM4S	(3)
+#define AW88166_SLOT_NUM_TDM4S_VALUE	\
+	(AW88166_SLOT_NUM_TDM4S << AW88166_SLOT_NUM_START_BIT)
+
+#define AW88166_SLOT_NUM_TDM6S	(4)
+#define AW88166_SLOT_NUM_TDM6S_VALUE	\
+	(AW88166_SLOT_NUM_TDM6S << AW88166_SLOT_NUM_START_BIT)
+
+#define AW88166_SLOT_NUM_TDM8S	(5)
+#define AW88166_SLOT_NUM_TDM8S_VALUE	\
+	(AW88166_SLOT_NUM_TDM8S << AW88166_SLOT_NUM_START_BIT)
+
+#define AW88166_SLOT_NUM_TDM16S	(6)
+#define AW88166_SLOT_NUM_TDM16S_VALUE	\
+	(AW88166_SLOT_NUM_TDM16S << AW88166_SLOT_NUM_START_BIT)
+
+#define AW88166_I2S_TX_SLOTVLD_START_BIT	(8)
+#define AW88166_I2S_TX_SLOTVLD_BITS_LEN	(4)
+#define AW88166_I2S_TX_SLOTVLD_MASK	\
+	(~(((1<<AW88166_I2S_TX_SLOTVLD_BITS_LEN)-1) << AW88166_I2S_TX_SLOTVLD_START_BIT))
+
+#define AW88166_I2S_RXR_SLOTVLD_START_BIT	(4)
+#define AW88166_I2S_RXR_SLOTVLD_BITS_LEN	(4)
+#define AW88166_I2S_RXR_SLOTVLD_MASK	\
+	(~(((1<<AW88166_I2S_RXR_SLOTVLD_BITS_LEN)-1) << AW88166_I2S_RXR_SLOTVLD_START_BIT))
+
+#define AW88166_I2S_RXL_SLOTVLD_START_BIT	(0)
+#define AW88166_I2S_RXL_SLOTVLD_BITS_LEN	(4)
+#define AW88166_I2S_RXL_SLOTVLD_MASK	\
+	(~(((1<<AW88166_I2S_RXL_SLOTVLD_BITS_LEN)-1) << AW88166_I2S_RXL_SLOTVLD_START_BIT))
+
 #define AW88166_CCO_MUX_START_BIT	(8)
 #define AW88166_CCO_MUX_BITS_LEN	(1)
 #define AW88166_CCO_MUX_MASK		\

-- 
2.54.0

[PATCH 3/6] ASoC: codecs: aw88166: reduce log spam

[Aaron Kling via B4 Relay]

From: Aaron Kling <webgeek1234@gmail.com>

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.

Signed-off-by: Aaron Kling <webgeek1234@gmail.com>
---
 sound/soc/codecs/aw88166.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/sound/soc/codecs/aw88166.c b/sound/soc/codecs/aw88166.c
index cd8773e316b90..c8b8c70aaf356 100644
--- a/sound/soc/codecs/aw88166.c
+++ b/sound/soc/codecs/aw88166.c
@@ -108,7 +108,7 @@ static int aw_dev_get_iis_status(struct aw_device *aw_dev)
 	if (ret)
 		return ret;
 	if ((reg_val & AW88166_BIT_PLL_CHECK) != AW88166_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;
 	}
 
@@ -122,7 +122,7 @@ static int aw88166_dev_check_pll(struct aw_device *aw_dev)
 	for (i = 0; i < AW88166_DEV_SYSST_CHECK_MAX; i++) {
 		ret = aw_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(AW88166_2000_US, AW88166_2000_US + 10);
 		} else {
 			return 0;
@@ -223,7 +223,7 @@ static int aw_dev_check_sysst(struct aw_device *aw_dev)
 			return ret;
 
 		if ((reg_val & (~AW88166_BIT_SYSST_CHECK_MASK) & check_val) != check_val) {
-			dev_err(aw_dev->dev, "check sysst fail, cnt=%d, reg_val=0x%04x, check:0x%x",
+			dev_dbg(aw_dev->dev, "check sysst fail, cnt=%d, reg_val=0x%04x, check:0x%x",
 				i, reg_val, AW88166_BIT_SYSST_NOSWS_CHECK);
 			usleep_range(AW88166_2000_US, AW88166_2000_US + 10);
 		} else {
@@ -754,7 +754,7 @@ static int aw88166_dev_start(struct aw88166 *aw88166)
 
 	ret = aw88166_dev_configure_syspll(aw88166);
 	if (ret) {
-		dev_err(aw_dev->dev, "pll check failed cannot start\n");
+		dev_dbg(aw_dev->dev, "pll check failed\n");
 		goto pll_check_fail;
 	}
 
@@ -765,7 +765,7 @@ static int aw88166_dev_start(struct aw88166 *aw88166)
 	/* check i2s status */
 	ret = aw_dev_check_sysst(aw_dev);
 	if (ret) {
-		dev_err(aw_dev->dev, "sysst check failed\n");
+		dev_dbg(aw_dev->dev, "sysst check failed\n");
 		goto sysst_check_fail;
 	}
 
@@ -1175,10 +1175,11 @@ static void aw88166_start_pa(struct aw88166 *aw88166)
 	for (i = 0; i < AW88166_START_RETRIES; i++) {
 		ret = aw88166_dev_start(aw88166);
 		if (ret) {
-			dev_err(aw88166->aw_pa->dev, "aw88166 device start failed. retry = %d", i);
+			dev_dbg(aw88166->aw_pa->dev,
+				"aw88166 device start failed. cnt:%d, ret:%d", i, ret);
 			ret = aw88166_dev_fw_update(aw88166, AW88166_DSP_FW_UPDATE_ON, true);
 			if (ret) {
-				dev_err(aw88166->aw_pa->dev, "fw update failed");
+				dev_dbg(aw88166->aw_pa->dev, "fw update failed");
 				continue;
 			}
 		} else {
@@ -1186,6 +1187,8 @@ static void aw88166_start_pa(struct aw88166 *aw88166)
 			break;
 		}
 	}
+	if (ret != 0)
+		dev_err(aw88166->aw_pa->dev, "start failure (%d)\n", ret);
 }
 
 static void aw88166_startup_work(struct work_struct *work)

-- 
2.54.0

[PATCH 4/6] ASoC: codecs: aw88166: remove fade in/out on start/stop

[Aaron Kling via B4 Relay]

From: Aaron Kling <webgeek1234@gmail.com>

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: Aaron Kling <webgeek1234@gmail.com>
---
 sound/soc/codecs/aw88166.c | 157 +--------------------------------------------
 sound/soc/codecs/aw88166.h |   2 -
 2 files changed, 2 insertions(+), 157 deletions(-)

diff --git a/sound/soc/codecs/aw88166.c b/sound/soc/codecs/aw88166.c
index c8b8c70aaf356..ab7af3eaceb28 100644
--- a/sound/soc/codecs/aw88166.c
+++ b/sound/soc/codecs/aw88166.c
@@ -668,59 +668,16 @@ static int aw_dev_set_volume(struct aw_device *aw_dev, unsigned int value)
 	return ret;
 }
 
-static void aw_dev_fade_in(struct aw_device *aw_dev)
-{
-	struct aw_volume_desc *desc = &aw_dev->volume_desc;
-	u16 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) {
-		aw_dev_set_volume(aw_dev, fade_in_vol);
-		return;
-	}
-
-	for (i = AW88166_MUTE_VOL; i >= fade_in_vol; i -= fade_step) {
-		aw_dev_set_volume(aw_dev, i);
-		usleep_range(aw_dev->fade_in_time, aw_dev->fade_in_time + 10);
-	}
-
-	if (i != fade_in_vol)
-		aw_dev_set_volume(aw_dev, fade_in_vol);
-}
-
-static void aw_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) {
-		aw_dev_set_volume(aw_dev, AW88166_MUTE_VOL);
-		return;
-	}
-
-	for (i = desc->ctl_volume; i <= AW88166_MUTE_VOL; i += fade_step) {
-		aw_dev_set_volume(aw_dev, i);
-		usleep_range(aw_dev->fade_out_time, aw_dev->fade_out_time + 10);
-	}
-
-	if (i != AW88166_MUTE_VOL) {
-		aw_dev_set_volume(aw_dev, AW88166_MUTE_VOL);
-		usleep_range(aw_dev->fade_out_time, aw_dev->fade_out_time + 10);
-	}
-}
-
 static void aw88166_dev_mute(struct aw_device *aw_dev, bool is_mute)
 {
 	if (is_mute) {
-		aw_dev_fade_out(aw_dev);
+		aw_dev_set_volume(aw_dev, AW88166_MUTE_VOL);
 		regmap_update_bits(aw_dev->regmap, AW88166_SYSCTRL_REG,
 				~AW88166_HMUTE_MASK, AW88166_HMUTE_ENABLE_VALUE);
 	} else {
 		regmap_update_bits(aw_dev->regmap, AW88166_SYSCTRL_REG,
 				~AW88166_HMUTE_MASK, AW88166_HMUTE_DISABLE_VALUE);
-		aw_dev_fade_in(aw_dev);
+		aw_dev_set_volume(aw_dev, aw_dev->volume_desc.ctl_volume);
 	}
 }
 
@@ -1515,75 +1472,6 @@ static struct snd_soc_dai_driver aw88166_dai[] = {
 	},
 };
 
-static int aw88166_get_fade_in_time(struct snd_kcontrol *kcontrol,
-	struct snd_ctl_elem_value *ucontrol)
-{
-	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
-	struct aw88166 *aw88166 = snd_soc_component_get_drvdata(component);
-	struct aw_device *aw_dev = aw88166->aw_pa;
-
-	ucontrol->value.integer.value[0] = aw_dev->fade_in_time;
-
-	return 0;
-}
-
-static int aw88166_set_fade_in_time(struct snd_kcontrol *kcontrol,
-	struct snd_ctl_elem_value *ucontrol)
-{
-	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
-	struct aw88166 *aw88166 = snd_soc_component_get_drvdata(component);
-	struct soc_mixer_control *mc =
-		(struct soc_mixer_control *)kcontrol->private_value;
-	struct aw_device *aw_dev = aw88166->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 aw88166_get_fade_out_time(struct snd_kcontrol *kcontrol,
-	struct snd_ctl_elem_value *ucontrol)
-{
-	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
-	struct aw88166 *aw88166 = snd_soc_component_get_drvdata(component);
-	struct aw_device *aw_dev = aw88166->aw_pa;
-
-	ucontrol->value.integer.value[0] = aw_dev->fade_out_time;
-
-	return 0;
-}
-
-static int aw88166_set_fade_out_time(struct snd_kcontrol *kcontrol,
-	struct snd_ctl_elem_value *ucontrol)
-{
-	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
-	struct aw88166 *aw88166 = snd_soc_component_get_drvdata(component);
-	struct soc_mixer_control *mc =
-		(struct soc_mixer_control *)kcontrol->private_value;
-	struct aw_device *aw_dev = aw88166->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 aw88166_dev_set_profile_index(struct aw_device *aw_dev, int index)
 {
 	/* check the index whether is valid */
@@ -1704,38 +1592,6 @@ static int aw88166_volume_set(struct snd_kcontrol *kcontrol,
 	return 0;
 }
 
-static int aw88166_get_fade_step(struct snd_kcontrol *kcontrol,
-				struct snd_ctl_elem_value *ucontrol)
-{
-	struct snd_soc_component *codec = snd_kcontrol_chip(kcontrol);
-	struct aw88166 *aw88166 = snd_soc_component_get_drvdata(codec);
-
-	ucontrol->value.integer.value[0] = aw88166->aw_pa->fade_step;
-
-	return 0;
-}
-
-static int aw88166_set_fade_step(struct snd_kcontrol *kcontrol,
-				struct snd_ctl_elem_value *ucontrol)
-{
-	struct snd_soc_component *codec = snd_kcontrol_chip(kcontrol);
-	struct aw88166 *aw88166 = 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 (aw88166->aw_pa->fade_step != value) {
-		aw88166->aw_pa->fade_step = value;
-		return 1;
-	}
-
-	return 0;
-}
-
 static int aw88166_re_get(struct snd_kcontrol *kcontrol,
 				struct snd_ctl_elem_value *ucontrol)
 {
@@ -1780,8 +1636,6 @@ static int aw88166_dev_init(struct aw88166 *aw88166, struct aw_container *aw_cfg
 		dev_err(aw_dev->dev, "aw_dev acf parse failed\n");
 		return -EINVAL;
 	}
-	aw_dev->fade_in_time = AW88166_1000_US / 10;
-	aw_dev->fade_out_time = AW88166_1000_US >> 1;
 	aw_dev->prof_cur = aw_dev->prof_info.prof_desc[0].id;
 	aw_dev->prof_index = aw_dev->prof_info.prof_desc[0].id;
 
@@ -1856,12 +1710,6 @@ static const struct snd_kcontrol_new aw88166_controls[] = {
 	SOC_SINGLE_EXT("PCM Playback Volume", AW88166_SYSCTRL2_REG,
 		6, AW88166_MUTE_VOL, 0, aw88166_volume_get,
 		aw88166_volume_set),
-	SOC_SINGLE_EXT("Fade Step", 0, 0, AW88166_MUTE_VOL, 0,
-		aw88166_get_fade_step, aw88166_set_fade_step),
-	SOC_SINGLE_EXT("Volume Ramp Up Step", 0, 0, FADE_TIME_MAX, FADE_TIME_MIN,
-		aw88166_get_fade_in_time, aw88166_set_fade_in_time),
-	SOC_SINGLE_EXT("Volume Ramp Down Step", 0, 0, FADE_TIME_MAX, FADE_TIME_MIN,
-		aw88166_get_fade_out_time, aw88166_set_fade_out_time),
 	SOC_SINGLE_EXT("Calib", 0, 0, AW88166_CALI_RE_MAX, 0,
 		aw88166_re_get, aw88166_re_set),
 	AW88166_PROFILE_EXT("AW88166 Profile Set", aw88166_profile_info,
@@ -1989,7 +1837,6 @@ static int aw88166_init(struct aw88166 *aw88166, struct i2c_client *i2c, struct
 	aw_dev->channel = AW88166_DEV_DEFAULT_CH;
 	aw_dev->fw_status = AW88166_DEV_FW_FAILED;
 
-	aw_dev->fade_step = AW88166_VOLUME_STEP_DB;
 	aw_dev->volume_desc.ctl_volume = AW88166_VOL_DEFAULT_VALUE;
 
 	aw88166_parse_channel_dt(aw88166);
diff --git a/sound/soc/codecs/aw88166.h b/sound/soc/codecs/aw88166.h
index 377a733447e70..1a61e56f607c9 100644
--- a/sound/soc/codecs/aw88166.h
+++ b/sound/soc/codecs/aw88166.h
@@ -607,8 +607,6 @@
 #define AW88166_DEV_SYSST_CHECK_MAX		(10)
 #define AW88166_START_RETRIES			(5)
 #define AW88166_START_WORK_DELAY_MS		(0)
-#define FADE_TIME_MAX				100000
-#define FADE_TIME_MIN				0
 #define AW88166_CHIP_ID			(0x2066)
 #define AW88166_I2C_NAME			"aw88166"
 #define AW88166_ACF_FILE			"aw88166_acf.bin"

-- 
2.54.0

[PATCH 5/6] ASoC: codecs: aw88166: remove async start

[Aaron Kling via B4 Relay]

From: Aaron Kling <webgeek1234@gmail.com>

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: Aaron Kling <webgeek1234@gmail.com>
---
 sound/soc/codecs/aw88166.c | 33 ++++-----------------------------
 sound/soc/codecs/aw88166.h |  5 -----
 2 files changed, 4 insertions(+), 34 deletions(-)

diff --git a/sound/soc/codecs/aw88166.c b/sound/soc/codecs/aw88166.c
index ab7af3eaceb28..6b98864de81a9 100644
--- a/sound/soc/codecs/aw88166.c
+++ b/sound/soc/codecs/aw88166.c
@@ -24,7 +24,6 @@ struct aw88166 {
 	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;
 
@@ -1148,16 +1147,7 @@ static void aw88166_start_pa(struct aw88166 *aw88166)
 		dev_err(aw88166->aw_pa->dev, "start failure (%d)\n", ret);
 }
 
-static void aw88166_startup_work(struct work_struct *work)
-{
-	struct aw88166 *aw88166 =
-		container_of(work, struct aw88166, start_work.work);
-
-	guard(mutex)(&aw88166->lock);
-	aw88166_start_pa(aw88166);
-}
-
-static void aw88166_start(struct aw88166 *aw88166, bool sync_start)
+static void aw88166_start(struct aw88166 *aw88166)
 {
 	int ret;
 
@@ -1173,12 +1163,7 @@ static void aw88166_start(struct aw88166 *aw88166, bool sync_start)
 		return;
 	}
 
-	if (sync_start == AW88166_SYNC_START)
-		aw88166_start_pa(aw88166);
-	else
-		queue_delayed_work(system_dfl_wq,
-			&aw88166->start_work,
-			AW88166_START_WORK_DELAY_MS);
+	aw88166_start_pa(aw88166);
 }
 
 static int aw_dev_check_sysint(struct aw_device *aw_dev)
@@ -1550,7 +1535,7 @@ static int aw88166_profile_set(struct snd_kcontrol *kcontrol,
 
 	if (aw88166->aw_pa->status) {
 		aw88166_stop(aw88166->aw_pa);
-		aw88166_start(aw88166, AW88166_SYNC_START);
+		aw88166_start(aw88166);
 	}
 
 	return 1;
@@ -1725,7 +1710,7 @@ static int aw88166_playback_event(struct snd_soc_dapm_widget *w,
 	guard(mutex)(&aw88166->lock);
 	switch (event) {
 	case SND_SOC_DAPM_PRE_PMU:
-		aw88166_start(aw88166, AW88166_ASYNC_START);
+		aw88166_start(aw88166);
 		break;
 	case SND_SOC_DAPM_POST_PMD:
 		aw88166_stop(aw88166->aw_pa);
@@ -1759,8 +1744,6 @@ static int aw88166_codec_probe(struct snd_soc_component *component)
 	struct aw88166 *aw88166 = snd_soc_component_get_drvdata(component);
 	int ret;
 
-	INIT_DELAYED_WORK(&aw88166->start_work, aw88166_startup_work);
-
 	ret = aw88166_request_firmware_file(aw88166);
 	if (ret)
 		dev_err(aw88166->aw_pa->dev, "%s failed\n", __func__);
@@ -1768,16 +1751,8 @@ static int aw88166_codec_probe(struct snd_soc_component *component)
 	return ret;
 }
 
-static void aw88166_codec_remove(struct snd_soc_component *aw_codec)
-{
-	struct aw88166 *aw88166 = snd_soc_component_get_drvdata(aw_codec);
-
-	cancel_delayed_work_sync(&aw88166->start_work);
-}
-
 static const struct snd_soc_component_driver soc_codec_dev_aw88166 = {
 	.probe = aw88166_codec_probe,
-	.remove = aw88166_codec_remove,
 	.dapm_widgets = aw88166_dapm_widgets,
 	.num_dapm_widgets = ARRAY_SIZE(aw88166_dapm_widgets),
 	.dapm_routes = aw88166_audio_map,
diff --git a/sound/soc/codecs/aw88166.h b/sound/soc/codecs/aw88166.h
index 1a61e56f607c9..b8dbfe0f307c4 100644
--- a/sound/soc/codecs/aw88166.h
+++ b/sound/soc/codecs/aw88166.h
@@ -670,11 +670,6 @@ enum AW88166_DEV_DSP_CFG {
 	AW88166_DEV_DSP_BYPASS = 1,
 };
 
-enum {
-	AW88166_SYNC_START = 0,
-	AW88166_ASYNC_START,
-};
-
 enum {
 	AW88166_RECORD_SEC_DATA = 0,
 	AW88166_RECOVERY_SEC_DATA = 1,

-- 
2.54.0

[PATCH 6/6] ASoC: codecs: aw88166: make volume control usable

[Aaron Kling via B4 Relay]

From: Aaron Kling <webgeek1234@gmail.com>

- 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 aw88166_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: 94e412c28e61 ("ASoC: codecs: Add aw88166 amplifier driver")
Signed-off-by: Aaron Kling <webgeek1234@gmail.com>
---
 sound/soc/codecs/aw88166.c | 47 ++++++++++++++++++++++------------------------
 sound/soc/codecs/aw88166.h | 15 ++++++++++++++-
 2 files changed, 36 insertions(+), 26 deletions(-)

diff --git a/sound/soc/codecs/aw88166.c b/sound/soc/codecs/aw88166.c
index 6b98864de81a9..3e87be33f4908 100644
--- a/sound/soc/codecs/aw88166.c
+++ b/sound/soc/codecs/aw88166.c
@@ -17,6 +17,7 @@
 #include <linux/regmap.h>
 #include <sound/soc.h>
 #include <sound/pcm_params.h>
+#include <sound/tlv.h>
 #include "aw88166.h"
 #include "aw88395/aw88395_device.h"
 
@@ -645,26 +646,12 @@ static int aw_dev_dsp_check(struct aw_device *aw_dev)
 	return ret;
 }
 
-static int aw_dev_set_volume(struct aw_device *aw_dev, unsigned int value)
+static void aw_dev_set_volume(struct aw_device *aw_dev, unsigned int value)
 {
-	struct aw_volume_desc *vol_desc = &aw_dev->volume_desc;
-	unsigned int reg_value;
-	u16 real_value;
-	int ret;
-
-	real_value = min((value + vol_desc->init_volume), (unsigned int)AW88166_MUTE_VOL);
-
-	ret = regmap_read(aw_dev->regmap, AW88166_SYSCTRL2_REG, &reg_value);
-	if (ret)
-		return ret;
-
-	dev_dbg(aw_dev->dev, "value 0x%x , reg:0x%x", value, real_value);
+	unsigned int volume = min(value, (unsigned int)AW88166_MUTE_VOL);
 
-	real_value = (real_value << AW88166_VOL_START_BIT) | (reg_value & AW88166_VOL_MASK);
-
-	ret = regmap_write(aw_dev->regmap, AW88166_SYSCTRL2_REG, real_value);
-
-	return ret;
+	regmap_update_bits(aw_dev->regmap, AW88166_SYSCTRL2_REG,
+		~AW88166_VOL_MASK, DB_TO_REG_VAL(volume));
 }
 
 static void aw88166_dev_mute(struct aw_device *aw_dev, bool is_mute)
@@ -1548,7 +1535,8 @@ static int aw88166_volume_get(struct snd_kcontrol *kcontrol,
 	struct aw88166 *aw88166 = snd_soc_component_get_drvdata(codec);
 	struct aw_volume_desc *vol_desc = &aw88166->aw_pa->volume_desc;
 
-	ucontrol->value.integer.value[0] = vol_desc->ctl_volume;
+	ucontrol->value.integer.value[0] =
+		(AW88166_MUTE_VOL - vol_desc->ctl_volume) / 2;
 
 	return 0;
 }
@@ -1561,12 +1549,13 @@ static int aw88166_volume_set(struct snd_kcontrol *kcontrol,
 	struct aw_volume_desc *vol_desc = &aw88166->aw_pa->volume_desc;
 	struct soc_mixer_control *mc =
 		(struct soc_mixer_control *)kcontrol->private_value;
-	int value;
+	int value = ucontrol->value.integer.value[0];
 
-	value = ucontrol->value.integer.value[0];
 	if (value < mc->min || value > mc->max)
 		return -EINVAL;
 
+	value = AW88166_MUTE_VOL - (value * 2);
+
 	if (vol_desc->ctl_volume != value) {
 		vol_desc->ctl_volume = value;
 		aw_dev_set_volume(aw88166->aw_pa, vol_desc->ctl_volume);
@@ -1691,10 +1680,18 @@ static int aw88166_request_firmware_file(struct aw88166 *aw88166)
 	return ret;
 }
 
+/*
+ * 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 aw88166_controls[] = {
-	SOC_SINGLE_EXT("PCM Playback Volume", AW88166_SYSCTRL2_REG,
-		6, AW88166_MUTE_VOL, 0, aw88166_volume_get,
-		aw88166_volume_set),
+	SOC_SINGLE_EXT_TLV("PCM Playback Volume", AW88166_SYSCTRL2_REG,
+		6, AW88166_CTL_MAX_VOL, 1,
+		aw88166_volume_get, aw88166_volume_set, volume_tlv),
 	SOC_SINGLE_EXT("Calib", 0, 0, AW88166_CALI_RE_MAX, 0,
 		aw88166_re_get, aw88166_re_set),
 	AW88166_PROFILE_EXT("AW88166 Profile Set", aw88166_profile_info,
@@ -1812,7 +1809,7 @@ static int aw88166_init(struct aw88166 *aw88166, struct i2c_client *i2c, struct
 	aw_dev->channel = AW88166_DEV_DEFAULT_CH;
 	aw_dev->fw_status = AW88166_DEV_FW_FAILED;
 
-	aw_dev->volume_desc.ctl_volume = AW88166_VOL_DEFAULT_VALUE;
+	aw_dev->volume_desc.ctl_volume = AW88166_CTL_DEFAULT_VOL;
 
 	aw88166_parse_channel_dt(aw88166);
 
diff --git a/sound/soc/codecs/aw88166.h b/sound/soc/codecs/aw88166.h
index b8dbfe0f307c4..c0303340812cf 100644
--- a/sound/soc/codecs/aw88166.h
+++ b/sound/soc/codecs/aw88166.h
@@ -112,6 +112,9 @@
 #define AW88166_REG_MAX		(0x7E)
 #define AW88166_MUTE_VOL		(1023)
 
+#define AW88166_VOL_6DB_START		(6)
+
+
 #define AW88166_DSP_CFG_ADDR		(0x9B00)
 #define AW88166_DSP_REG_CFG_ADPZ_RA	(0x9B68)
 #define AW88166_DSP_FW_ADDR		(0x8980)
@@ -590,7 +593,8 @@
 #define AW88166_CALI_RE_MAX			(15000)
 #define AW88166_CALI_RE_MIN			(4000)
 #define AW88166_VOLUME_STEP_DB			(64)
-#define AW88166_VOL_DEFAULT_VALUE		(0)
+#define AW88166_CTL_MAX_VOL			(AW88166_MUTE_VOL / 2)
+#define AW88166_CTL_DEFAULT_VOL			(AW88166_CTL_MAX_VOL / 2)
 #define AW88166_DSP_RE_TO_SHOW_RE(re, shift)	(((re) * (1000)) >> (shift))
 #define AW88166_SHOW_RE_TO_DSP_RE(re, shift)	(((re) << shift) / (1000))
 
@@ -619,6 +623,15 @@
 			SNDRV_PCM_FMTBIT_S24_LE | \
 			SNDRV_PCM_FMTBIT_S32_LE)
 
+#define AW88166_REG_TO_DB		(0x3f)
+
+#define REG_VAL_TO_DB(value)		((((value) >> AW88166_VOL_6DB_START) * \
+					AW88166_VOLUME_STEP_DB) + \
+					((value) & AW88166_REG_TO_DB))
+#define DB_TO_REG_VAL(value)		((((value) / AW88166_VOLUME_STEP_DB) << \
+					AW88166_VOL_6DB_START) + \
+					((value) % AW88166_VOLUME_STEP_DB))
+
 #define AW88166_PROFILE_EXT(xname, profile_info, profile_get, profile_set) \
 { \
 	.iface = SNDRV_CTL_ELEM_IFACE_MIXER, \

-- 
2.54.0

Re: [PATCH 1/6] ASoC: codecs: aw88166: support changing sample rate and bit width

[Mark Brown]

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

On Fri, Sep 25, 2026 at 02:47:29AM -0500, Aaron Kling via B4 Relay wrote:

> The aw88166 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.

> -	ret = regmap_read(aw_dev->regmap, AW88166_PLLCTRL2_REG, &reg_val);
> +	/* PLL divider must be used for 8/16/32 kHz modes */
> +	ret = regmap_update_bits(aw_dev->regmap, AW88166_PLLCTRL1_REG,
> +			~AW88166_CCO_MUX_MASK, aw88166->cco_mux_value);

We seem to just completely drop all reference to PLLCTRL2 and switch
over to PLLCTRL1, I'm going to guess that this means that something that
used to work now no longer works...

> +static int aw88166_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 aw88166 *aw88166 = snd_soc_component_get_drvdata(component);
> +
> +	case 32:
> +		aw88166->bck_value = AW88166_I2SBCK_64FS_VALUE;
> +		break;
> +	default:
> +		dev_err(aw88166->aw_pa->dev, "unsupported physical bit width %d\n",
> +			params_physical_width(params));
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}

This doesn't touch the hardware, it just writes state so if hw_params()
is called while things are powered up nothing is going to take effect
until the next stream.

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

Re: [PATCH 4/6] ASoC: codecs: aw88166: remove fade in/out on start/stop

[Mark Brown]

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

On Fri, Sep 25, 2026 at 02:47:32AM -0500, Aaron Kling via B4 Relay wrote:
> From: Aaron Kling <webgeek1234@gmail.com>
> 
> This "feature" was copied from downstream, but it does not belong in
> the kernel at all. Remove it to simplify the driver.

Are you sure?  Some devices have really bad pop/click performance and
need ramps to mask the effects, we've got other drivers with soft ramps
in them.

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

Re: [PATCH 5/6] ASoC: codecs: aw88166: remove async start

[Mark Brown]

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

On Fri, Sep 25, 2026 at 02:47:33AM -0500, Aaron Kling via B4 Relay wrote:
> From: Aaron Kling <webgeek1234@gmail.com>
> 
> 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.

Again, are you sure this isn't a workaround for terrible hardware?

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

Re: [PATCH 6/6] ASoC: codecs: aw88166: make volume control usable

[Mark Brown]

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

On Fri, Sep 25, 2026 at 02:47:34AM -0500, Aaron Kling via B4 Relay wrote:
> From: Aaron Kling <webgeek1234@gmail.com>
> 
> - Invert the value to match userspace expectations (in the hardware,
>   positive numbers represent negative dB attenuation)

That's a very bad compatibility break, if anyone has a profile saved
they might get excessively loud volume from their existing configuration
which could be distressing.  It might be better to rename the control.

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

Re: [PATCH 5/6] ASoC: codecs: aw88166: remove async start

[Cezary Rojewski]

On 9/25/2026 9:47 AM, Aaron Kling via B4 Relay wrote:
> From: Aaron Kling <webgeek1234@gmail.com>
> 
> 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.
Do anything like what?
Let's not do what?

With all due respect, the commit message found here is so vague I'm
surprised people even review this.  Your change is not a one-liner
cleanup.  I'd expect at least a short description provided, on what is
happening _currently_ and _why_ removal improves the functional
behaviour of the codec device.


Kind regards,
Czarek

Re: [PATCH 5/6] ASoC: codecs: aw88166: remove async start

[Aaron Kling]

On Fri, Sep 25, 2026 at 9:35 AM Cezary Rojewski
<cezary.rojewski@intel.com> wrote:
>
> On 9/25/2026 9:47 AM, Aaron Kling via B4 Relay wrote:
> > From: Aaron Kling <webgeek1234@gmail.com>
> >
> > 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.
> Do anything like what?
> Let's not do what?
>
> With all due respect, the commit message found here is so vague I'm
> surprised people even review this.  Your change is not a one-liner
> cleanup.  I'd expect at least a short description provided, on what is
> happening _currently_ and _why_ removal improves the functional
> behaviour of the codec device.

As mentioned in the cover letter, this series is a direct copy of
changes to the aw88261 driver, which went through three revisions and
was accepted in this form. The matching patch is here [0]. What makes
this worse than the already merged patch?

Aaron

https://lore.kernel.org/linux-sound/20260529200550.529719-6-val@packett.cool/

Re: [PATCH 5/6] ASoC: codecs: aw88166: remove async start

[Aaron Kling]

On Fri, Sep 25, 2026 at 9:13 AM Mark Brown <broonie@kernel.org> wrote:
>
> On Fri, Sep 25, 2026 at 02:47:33AM -0500, Aaron Kling via B4 Relay wrote:
> > From: Aaron Kling <webgeek1234@gmail.com>
> >
> > 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.
>
> Again, are you sure this isn't a workaround for terrible hardware?

It's possible, I suppose. But this is copied from the already accepted
aw88261 series. And it has real effects on my devices. On an AYN Thor
without these changes, if I start a video, it will play for 1-2
seconds without audio, then I'll start hearing audio. With this
change, audio and video start at the same time.

Aaron

Re: [PATCH 1/6] ASoC: codecs: aw88166: support changing sample rate and bit width

[Aaron Kling]

On Fri, Sep 25, 2026 at 9:00 AM Mark Brown <broonie@kernel.org> wrote:
>
> On Fri, Sep 25, 2026 at 02:47:29AM -0500, Aaron Kling via B4 Relay wrote:
>
> > The aw88166 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.
>
> > -     ret = regmap_read(aw_dev->regmap, AW88166_PLLCTRL2_REG, &reg_val);
> > +     /* PLL divider must be used for 8/16/32 kHz modes */
> > +     ret = regmap_update_bits(aw_dev->regmap, AW88166_PLLCTRL1_REG,
> > +                     ~AW88166_CCO_MUX_MASK, aw88166->cco_mux_value);
>
> We seem to just completely drop all reference to PLLCTRL2 and switch
> over to PLLCTRL1, I'm going to guess that this means that something that
> used to work now no longer works...

To be honest, I only understand the audio handling at a high level.
This was copied from the aw88261 change almost entirely as-is, as
mentioned in the cover letter. I did some spot checks of the data
sheet to make sure changed addresses were the same and what I checked
did match the 88261 changes. But for the hardware specific stuff, I
unfortunately don't know. What I can say is that after this change,
the AYN Odin3 aw88166 firmware blob from the stock android image works
as-is, where-as before it required some binary patches to start a
stream. Implying that the firmware defaults to settings that didn't
match what the os was using.

> > +static int aw88166_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 aw88166 *aw88166 = snd_soc_component_get_drvdata(component);
> > +
> > +     case 32:
> > +             aw88166->bck_value = AW88166_I2SBCK_64FS_VALUE;
> > +             break;
> > +     default:
> > +             dev_err(aw88166->aw_pa->dev, "unsupported physical bit width %d\n",
> > +                     params_physical_width(params));
> > +             return -EINVAL;
> > +     }
> > +
> > +     return 0;
> > +}
>
> This doesn't touch the hardware, it just writes state so if hw_params()
> is called while things are powered up nothing is going to take effect
> until the next stream.

Mmm. As mentioned above, this copies another change, so if it's wrong,
there's more places that need fixed. If there's a better way to handle
this, I can certainly update this patch.

Aaron

Re: [PATCH 4/6] ASoC: codecs: aw88166: remove fade in/out on start/stop

[Aaron Kling]

On Fri, Sep 25, 2026 at 9:11 AM Mark Brown <broonie@kernel.org> wrote:
>
> On Fri, Sep 25, 2026 at 02:47:32AM -0500, Aaron Kling via B4 Relay wrote:
> > From: Aaron Kling <webgeek1234@gmail.com>
> >
> > This "feature" was copied from downstream, but it does not belong in
> > the kernel at all. Remove it to simplify the driver.
>
> Are you sure?  Some devices have really bad pop/click performance and
> need ramps to mask the effects, we've got other drivers with soft ramps
> in them.

Ditto replies on the other changes. This was accepted for aw88261, is
there anything specific to this driver that doesn't apply there? I
have not experienced any such noise on the AYN and Retroid Pocket
devices that use this amp since making these changes.

Aaron

Re: [PATCH 6/6] ASoC: codecs: aw88166: make volume control usable

[Aaron Kling]

On Fri, Sep 25, 2026 at 9:18 AM Mark Brown <broonie@kernel.org> wrote:
>
> On Fri, Sep 25, 2026 at 02:47:34AM -0500, Aaron Kling via B4 Relay wrote:
> > From: Aaron Kling <webgeek1234@gmail.com>
> >
> > - Invert the value to match userspace expectations (in the hardware,
> >   positive numbers represent negative dB attenuation)
>
> That's a very bad compatibility break, if anyone has a profile saved
> they might get excessively loud volume from their existing configuration
> which could be distressing.  It might be better to rename the control.

Mmm. At the risk of sounding like a broken record, is this any
different from the accepted aw88261 change? I wouldn't be specifically
opposed to a new control as my use case, Android, is very tightly
controlled and I can just update the audio config I ship. But more
standard Linux use cases that rely on a standardize control name would
be stuck with a non-standard inverted control forever.

Aaron

Re: [PATCH 5/6] ASoC: codecs: aw88166: remove async start

[Mark Brown]

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

On Fri, Sep 25, 2026 at 10:44:46AM -0500, Aaron Kling wrote:
> On Fri, Sep 25, 2026 at 9:13 AM Mark Brown <broonie@kernel.org> wrote:

> > Again, are you sure this isn't a workaround for terrible hardware?

> It's possible, I suppose. But this is copied from the already accepted
> aw88261 series. And it has real effects on my devices. On an AYN Thor
> without these changes, if I start a video, it will play for 1-2
> seconds without audio, then I'll start hearing audio. With this
> change, audio and video start at the same time.

Does the audio sound fine - are there glitches at the start of the
stream?

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

Re: [PATCH 5/6] ASoC: codecs: aw88166: remove async start

[Aaron Kling]

On Fri, Sep 25, 2026 at 11:06 AM Mark Brown <broonie@kernel.org> wrote:
>
> On Fri, Sep 25, 2026 at 10:44:46AM -0500, Aaron Kling wrote:
> > On Fri, Sep 25, 2026 at 9:13 AM Mark Brown <broonie@kernel.org> wrote:
>
> > > Again, are you sure this isn't a workaround for terrible hardware?
>
> > It's possible, I suppose. But this is copied from the already accepted
> > aw88261 series. And it has real effects on my devices. On an AYN Thor
> > without these changes, if I start a video, it will play for 1-2
> > seconds without audio, then I'll start hearing audio. With this
> > change, audio and video start at the same time.
>
> Does the audio sound fine - are there glitches at the start of the
> stream?

No glitches that I've noticed so far. Can't say I've done a crazy wide
range of tests, but I've run through most of my normal use cases and
haven't heard anything unexpected.

Aaron

Re: [PATCH 6/6] ASoC: codecs: aw88166: make volume control usable

[Mark Brown]

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

On Fri, Sep 25, 2026 at 11:05:42AM -0500, Aaron Kling wrote:
> On Fri, Sep 25, 2026 at 9:18 AM Mark Brown <broonie@kernel.org> wrote:
> > On Fri, Sep 25, 2026 at 02:47:34AM -0500, Aaron Kling via B4 Relay wrote:

> > > - Invert the value to match userspace expectations (in the hardware,
> > >   positive numbers represent negative dB attenuation)

> > That's a very bad compatibility break, if anyone has a profile saved
> > they might get excessively loud volume from their existing configuration
> > which could be distressing.  It might be better to rename the control.

> Mmm. At the risk of sounding like a broken record, is this any
> different from the accepted aw88261 change? I wouldn't be specifically
> opposed to a new control as my use case, Android, is very tightly
> controlled and I can just update the audio config I ship. But more
> standard Linux use cases that rely on a standardize control name would
> be stuck with a non-standard inverted control forever.

Why would renaming the control do that?

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

Re: [PATCH 6/6] ASoC: codecs: aw88166: make volume control usable

[Aaron Kling]

On Fri, Sep 25, 2026 at 11:18 AM Mark Brown <broonie@kernel.org> wrote:
>
> On Fri, Sep 25, 2026 at 11:05:42AM -0500, Aaron Kling wrote:
> > On Fri, Sep 25, 2026 at 9:18 AM Mark Brown <broonie@kernel.org> wrote:
> > > On Fri, Sep 25, 2026 at 02:47:34AM -0500, Aaron Kling via B4 Relay wrote:
>
> > > > - Invert the value to match userspace expectations (in the hardware,
> > > >   positive numbers represent negative dB attenuation)
>
> > > That's a very bad compatibility break, if anyone has a profile saved
> > > they might get excessively loud volume from their existing configuration
> > > which could be distressing.  It might be better to rename the control.
>
> > Mmm. At the risk of sounding like a broken record, is this any
> > different from the accepted aw88261 change? I wouldn't be specifically
> > opposed to a new control as my use case, Android, is very tightly
> > controlled and I can just update the audio config I ship. But more
> > standard Linux use cases that rely on a standardize control name would
> > be stuck with a non-standard inverted control forever.
>
> Why would renaming the control do that?

I might be misunderstanding something, but I thought there were
standardized names, like this "PCM Playback Volume" that tools and os
handling expect and will use if found. Which would cause a second
control with a different name to not get used. Is there something I'm
missing to where a new control would get used without manual
intervention?

Aaron

Re: [PATCH 4/6] ASoC: codecs: aw88166: remove fade in/out on start/stop

[Mark Brown]

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

On Fri, Sep 25, 2026 at 10:59:24AM -0500, Aaron Kling wrote:
> On Fri, Sep 25, 2026 at 9:11 AM Mark Brown <broonie@kernel.org> wrote:

> > Are you sure?  Some devices have really bad pop/click performance and
> > need ramps to mask the effects, we've got other drivers with soft ramps
> > in them.

> Ditto replies on the other changes. This was accepted for aw88261, is
> there anything specific to this driver that doesn't apply there? I
> have not experienced any such noise on the AYN and Retroid Pocket
> devices that use this amp since making these changes.

Perhaps as Cezary sugested the changelogs on the aw88261 were better and
didn't set off alarm bells in the same way.  I don't know anything in
particular about either of these devices, the drivers aren't the best
but sometimes hardware is also bad.  

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

Re: [PATCH 5/6] ASoC: codecs: aw88166: remove async start

[Mark Brown]

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

On Fri, Sep 25, 2026 at 11:16:18AM -0500, Aaron Kling wrote:
> On Fri, Sep 25, 2026 at 11:06 AM Mark Brown <broonie@kernel.org> wrote:

> > Does the audio sound fine - are there glitches at the start of the
> > stream?

> No glitches that I've noticed so far. Can't say I've done a crazy wide
> range of tests, but I've run through most of my normal use cases and
> haven't heard anything unexpected.

The change is probably fine then but explain this, ramps like that are
almost always workarounds.

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

Re: [PATCH 6/6] ASoC: codecs: aw88166: make volume control usable

[Mark Brown]

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

On Fri, Sep 25, 2026 at 11:25:06AM -0500, Aaron Kling wrote:
> On Fri, Sep 25, 2026 at 11:18 AM Mark Brown <broonie@kernel.org> wrote:
> > On Fri, Sep 25, 2026 at 11:05:42AM -0500, Aaron Kling wrote:

> > > controlled and I can just update the audio config I ship. But more
> > > standard Linux use cases that rely on a standardize control name would
> > > be stuck with a non-standard inverted control forever.

> > Why would renaming the control do that?

> I might be misunderstanding something, but I thought there were
> standardized names, like this "PCM Playback Volume" that tools and os
> handling expect and will use if found. Which would cause a second
> control with a different name to not get used. Is there something I'm
> missing to where a new control would get used without manual
> intervention?

There are standard names, but you should be able to find a new name in
the standard set (eg, adding an Analog or Digital) that's unimplemented.
At the very least they also wouldn't see the old name and therefore
would not get stuck with an inverted control.

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