ASoC: codecs: aw88261: fix changing sample rate and bit width

PATCH Superseded Val Packett 2 participants 4 messages Apr 20 – Apr 21

Subsystems (2)

Sound ASoC

lore

Share & tools

lore

Val Packett

author

2026-04-20 21:36 UTC #0 lore raw

PATCH 1/2 ASoC: codecs: aw88261: fix changing sample rate and bit width

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 in the hw_params DAI callback. This replaces the strange downstream routine that tries two divider modes in sequence.

Signed-off-by Val Packett <[email protected]>

---

This driver is used for the speakers on devices like fairphone-fp5 and motorola-dubai, but until now only with non-upstreamable hacks (see [1]) to force the use of S32_LE because it "did not initialize with the format limited to S16_LE".

It seems like the downstream driver did not actually handle this properly either, so I presume similar hacks are done there too. There are two different versions I've found (in the same tree even): the "newer" more complex one [2] sends hw_params straight to /dev/null and has this bizzare I-don't-know-what-I'm-doing "mode1/mode2" routine that was copied into the upstream driver, while the other simpler one [3] does indeed have an hw_params callback that configures registers based on the params..

Except if anyone tried to just copy it, it wouldn't actually fix support for bit widths other than 32. The crucial thing it missed was the I2SBCK field, which is what's actually responsible for setting the "physical" frame length! Also during testing I've discovered that the value of the registers was being reset during aw88261_dev_pwd, so touching registers directly in the hw_params handler would not work.

With this patch, the aw88261 speakers in my motorola-dubai do work with the S16_LE format, avoiding the need for sm8250.c hacks.

[1]: https://github.com/sc7280-mainline/linux/commit/b9c78cb306ff069eee65213c67b7e7fba40e6221
[2]: https://github.com/LineageOS/android_kernel_motorola_sm7325/tree/lineage-23.2/techpack/audio/asoc/codecs/aw88261
[3]: https://github.com/LineageOS/android_kernel_motorola_sm7325/blob/lineage-23.2/techpack/audio/asoc/codecs/aw882xx/aw882xx.c

Thanks,
~val

---
 sound/soc/codecs/aw88261.c | 174 ++++++++++++++++++++++++-------------
 sound/soc/codecs/aw88261.h |  78 ++++++++++++++++-
 2 files changed, 189 insertions(+), 63 deletions(-)

sound/soc/codecs/aw88261.c +112 ~~-62~~

index a6805d5405cd..3c7d3e3865eb 100644
@@ -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,95 @@ 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;
+	uint32_t sr_value, fs_value, cco_mux_value, bck_value;
 	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;
-		}
+	switch (aw88261->sample_rate) {
+	case 8000:
+		sr_value = AW88261_I2SSR_8KHZ_VALUE;
+		cco_mux_value = AW88261_CCO_MUX_DIVIDED_VALUE;
+		break;
+	case 16000:
+		sr_value = AW88261_I2SSR_16KHZ_VALUE;
+		cco_mux_value = AW88261_CCO_MUX_DIVIDED_VALUE;
+		break;
+	case 32000:
+		sr_value = AW88261_I2SSR_32KHZ_VALUE;
+		cco_mux_value = AW88261_CCO_MUX_DIVIDED_VALUE;
+		break;
+	case 44100:
+		sr_value = AW88261_I2SSR_44P1KHZ_VALUE;
+		cco_mux_value = AW88261_CCO_MUX_BYPASS_VALUE;
+		break;
+	case 48000:
+		sr_value = AW88261_I2SSR_48KHZ_VALUE;
+		cco_mux_value = AW88261_CCO_MUX_BYPASS_VALUE;
+		break;
+	case 96000:
+		sr_value = AW88261_I2SSR_96KHZ_VALUE;
+		cco_mux_value = AW88261_CCO_MUX_BYPASS_VALUE;
+		break;
+	case 192000:
+		sr_value = AW88261_I2SSR_192KHZ_VALUE;
+		cco_mux_value = AW88261_CCO_MUX_BYPASS_VALUE;
+		break;
+	default:
+		dev_err(aw_dev->dev, "unsupported sample rate %d\n",
+			aw88261->sample_rate);
+		return -EINVAL;
 	}
 
-	return ret;
+	switch (aw88261->bit_width) {
+	case 16:
+		fs_value = AW88261_I2SFS_16_BITS_VALUE;
+		bck_value = AW88261_I2SBCK_32FS_VALUE;
+		break;
+	case 20:
+		fs_value = AW88261_I2SFS_20_BITS_VALUE;
+		bck_value = AW88261_I2SBCK_48FS_VALUE;
+		break;
+	case 24:
+		fs_value = AW88261_I2SFS_24_BITS_VALUE;
+		bck_value = AW88261_I2SBCK_48FS_VALUE;
+		break;
+	case 32:
+		fs_value = AW88261_I2SFS_32_BITS_VALUE;
+		bck_value = AW88261_I2SBCK_64FS_VALUE;
+		break;
+	default:
+		dev_err(aw_dev->dev, "unsupported bit width %d\n",
+			aw88261->bit_width);
+		return -EINVAL;
+	}
+
+	/* 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, cco_mux_value);
+	if (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, 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, 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, fs_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 +583,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 +737,26 @@ static void aw88261_start(struct aw88261 *aw88261, bool sync_start)
 			AW88261_START_WORK_DELAY_MS);
 }
 
+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;
+
+	/* Only store the settings as the regs do get reset when starting */
+	aw88261->sample_rate = params_rate(params);
+	aw88261->bit_width = params_width(params);
+	return 0;
+}
+
+static const struct snd_soc_dai_ops aw88261_dai_ops = {
+	.hw_params = aw88261_hw_params,
+};
+
 static struct snd_soc_dai_driver aw88261_dai[] = {
 	{
 		.name = "aw88261-aif",

@@ -730,6 +775,7 @@ static struct snd_soc_dai_driver aw88261_dai[] = {
 			.rates = AW88261_RATES,
 			.formats = AW88261_FORMATS,
 		},
+		.ops = &aw88261_dai_ops,
 	},
 };

@@ -1249,6 +1295,10 @@ static int aw88261_i2c_probe(struct i2c_client *i2c)
 	if (!aw88261)
 		return -ENOMEM;
 
+	/* set defaults */
+	aw88261->sample_rate = 48000;
+	aw88261->bit_width = 32;
+
 	mutex_init(&aw88261->lock);
 
 	i2c_set_clientdata(i2c, aw88261);

sound/soc/codecs/aw88261.h +77 -1

90 lines not on this page · raw message

Signature · 1 line

2.53.0

Val Packett

author

2026-04-20 21:37 UTC #1 lore raw

PATCH 2/2 ASoC: codecs: aw88261: use correct registers for AW88258

The AW88258 has a slightly different register map, namely the PLL ones are different and some extra boost ones are absent. Before the recent hw_params fix, the PLL CCO_MUX accesses were actually always wrong as the PLLCTRL1 address (0x54) was from the AW88261 but the CCO_MUX offset
(14) was from the AW88258. With said fix the AW88261 parameters were always used, making it work on that chip. Make the accesses correct for the AW88258 as well.

Signed-off-by Val Packett <[email protected]>

---
This one is not tested (don't have the hardware), but since I've noticed that the CCO_MUX was not even correct for any of the supported chips, I've decided to try fixing it for the other one as well.

Code style wise this is kind of a really annoying situation but hopefully this looks okay enough

~val

---
 sound/soc/codecs/aw88258.h | 34 ++++++++++++++++++++++++++++++++++
 sound/soc/codecs/aw88261.c | 37 ++++++++++++++++++++++++++-----------
 2 files changed, 60 insertions(+), 11 deletions(-)
 create mode 100644 sound/soc/codecs/aw88258.h

sound/soc/codecs/aw88258.h new file +34 -0

new file mode 100644
index 000000000000..7ae177667ae6
@@ -0,0 +1,34 @@
+// SPDX-License-Identifier: GPL-2.0-only
+//
+// aw88258.h  --  AW88258 ALSA SoC Audio driver
+//
+// Copyright (c) 2023 awinic Technology CO., LTD
+//
+// Author: Jimmy Zhang <[email protected]>
+// Author: Weidong Wang <[email protected]>
+//
+
+#ifndef __AW88258_H__
+#define __AW88258_H__
+
+/* This file contains definitions for registers that differ between
+ * the AW88261 (ID 0x2113) and the AW88258 (ID 0x1852). */
+
+#define AW88258_PLLCTRL1_REG		(0x66)
+#define AW88258_PLLCTRL2_REG		(0x67)
+#define AW88258_PLLCTRL3_REG		(0x68)
+
+#define AW88258_CCO_MUX_START_BIT	(14)
+#define AW88258_CCO_MUX_BITS_LEN	(1)
+#define AW88258_CCO_MUX_MASK		\
+	(~(((1<<AW88258_CCO_MUX_BITS_LEN)-1) << AW88258_CCO_MUX_START_BIT))
+
+#define AW88258_CCO_MUX_DIVIDED	(0)
+#define AW88258_CCO_MUX_DIVIDED_VALUE	\
+	(AW88258_CCO_MUX_DIVIDED << AW88258_CCO_MUX_START_BIT)
+
+#define AW88258_CCO_MUX_BYPASS		(1)
+#define AW88258_CCO_MUX_BYPASS_VALUE	\
+	(AW88258_CCO_MUX_BYPASS << AW88258_CCO_MUX_START_BIT)
+
+#endif

sound/soc/codecs/aw88261.c +26 ~~-11~~

index 3c7d3e3865eb..172f38a4921c 100644
@@ -14,6 +14,7 @@
 #include <linux/regulator/consumer.h>
 #include <sound/soc.h>
 #include <sound/pcm_params.h>
+#include "aw88258.h"
 #include "aw88261.h"
 #include "aw88395/aw88395_data_type.h"
 #include "aw88395/aw88395_device.h"

@@ -179,37 +180,36 @@ static int aw88261_dev_check_pll(struct aw_device *aw_dev)
 static int aw88261_dev_configure_syspll(struct aw88261 *aw88261)
 {
 	struct aw_device *aw_dev = aw88261->aw_pa;
-	uint32_t sr_value, fs_value, cco_mux_value, bck_value;
+	uint32_t sr_value, fs_value, bck_value;
+	bool bypass_divider = false;
 	int ret;
 
 	switch (aw88261->sample_rate) {
 	case 8000:
 		sr_value = AW88261_I2SSR_8KHZ_VALUE;
-		cco_mux_value = AW88261_CCO_MUX_DIVIDED_VALUE;
 		break;
 	case 16000:
 		sr_value = AW88261_I2SSR_16KHZ_VALUE;
-		cco_mux_value = AW88261_CCO_MUX_DIVIDED_VALUE;
 		break;
 	case 32000:
 		sr_value = AW88261_I2SSR_32KHZ_VALUE;
-		cco_mux_value = AW88261_CCO_MUX_DIVIDED_VALUE;
+		bypass_divider = true;
 		break;
 	case 44100:
 		sr_value = AW88261_I2SSR_44P1KHZ_VALUE;
-		cco_mux_value = AW88261_CCO_MUX_BYPASS_VALUE;
+		bypass_divider = true;
 		break;
 	case 48000:
 		sr_value = AW88261_I2SSR_48KHZ_VALUE;
-		cco_mux_value = AW88261_CCO_MUX_BYPASS_VALUE;
+		bypass_divider = true;
 		break;
 	case 96000:
 		sr_value = AW88261_I2SSR_96KHZ_VALUE;
-		cco_mux_value = AW88261_CCO_MUX_BYPASS_VALUE;
+		bypass_divider = true;
 		break;
 	case 192000:
 		sr_value = AW88261_I2SSR_192KHZ_VALUE;
-		cco_mux_value = AW88261_CCO_MUX_BYPASS_VALUE;
+		bypass_divider = true;
 		break;
 	default:
 		dev_err(aw_dev->dev, "unsupported sample rate %d\n",

@@ -241,8 +241,21 @@ static int aw88261_dev_configure_syspll(struct aw88261 *aw88261)
 	}
 
 	/* 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, cco_mux_value);
+	switch (aw_dev->chip_id) {
+	case AW88261_CHIP_ID:
+		ret = regmap_update_bits(aw_dev->regmap, AW88261_PLLCTRL1_REG,
+				~AW88261_CCO_MUX_MASK, bypass_divider ?
+				AW88261_CCO_MUX_BYPASS_VALUE : AW88261_CCO_MUX_DIVIDED);
+		break;
+	case AW88258_CHIP_ID:
+		ret = regmap_update_bits(aw_dev->regmap, AW88258_PLLCTRL1_REG,
+				~AW88258_CCO_MUX_MASK, bypass_divider ?
+				AW88258_CCO_MUX_BYPASS_VALUE : AW88258_CCO_MUX_DIVIDED);
+		break;
+	default:
+		WARN_ONCE(1, "missing PLL regs for chip %x", aw_dev->chip_id);
+		return -ENXIO;
+	}
 	if (ret)
 		return ret;

@@ -306,7 +319,9 @@ static void aw88261_dev_uls_hmute(struct aw_device *aw_dev, bool uls_hmute)
 
 static void aw88261_reg_force_set(struct aw88261 *aw88261)
 {
-	if (aw88261->frcset_en == AW88261_FRCSET_ENABLE) {
+	if (aw88261->aw_pa->chip_id == AW88258_CHIP_ID) {
+		dev_dbg(aw88261->aw_pa->dev, "force_set not supported on aw88258");
+	} else 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);

Signature · 1 line

2.53.0

Mark Brown

maintainer

2026-04-21 15:59 UTC #2 lore raw

In reply to Val Packett

Quoted 18 lines · Val Packett

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 in the hw_params DAI callback. This replaces the strange downstream routine that tries two divider modes in sequence.
sound/soc/codecs/aw88261.c +9 -0
+static int aw88261_dev_configure_syspll(struct aw88261 *aw88261)
 {
 
 
+	switch (aw88261->sample_rate) {
+	case 8000:
+	case 16000:
+	case 32000:
+	case 44100:
+	case 48000:
+	case 96000:
+	case 192000:

...

sound/soc/codecs/aw88261.c static int aw88261_dev_check_mode1_pll(struct aw_device *aw_dev) · line 218

Quoted 5 lines

sound/soc/codecs/aw88261.c +4 -0

+	default:
+		dev_err(aw_dev->dev, "unsupported sample rate %d\n",
+			aw88261->sample_rate);
+		return -EINVAL;
 	}

The driver advertises more rats than this, including 11.025kHz, 12kHz and multiples thereof. It's unlikely anything would actually *use* them but it'd be better to update the constraints.

Val Packett

author

2026-04-21 16:04 UTC #3 lore raw

Quoted 19 lines · Mark Brown

Quoted 16 lines · Val Packett
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 in the hw_params DAI callback. This replaces the strange downstream routine that tries two divider modes in sequence.
sound/soc/codecs/aw88261.c +9 -0
+static int aw88261_dev_configure_syspll(struct aw88261 *aw88261)
  {
 
+	switch (aw88261->sample_rate) {
+	case 8000:
+	case 16000:
+	case 32000:
+	case 44100:
+	case 48000:
+	case 96000:
+	case 192000:
...

Quoted 8 lines

sound/soc/codecs/aw88261.c static int aw88261_dev_check_mode1_pll(struct aw_device *aw_dev) · line 218
Quoted 5 lines
sound/soc/codecs/aw88261.c +4 -0
+	default:
+		dev_err(aw_dev->dev, "unsupported sample rate %d\n",
+			aw88261->sample_rate);
+		return -EINVAL;
  	}
The driver advertises more rats than this, including 11.025kHz, 12kHz and multiples thereof. It's unlikely anything would actually *use* them but it'd be better to update the constraints.

Oops, I never realized that SNDRV_PCM_RATE_8000_48000 includes so much "unreasonable" junk!

Ack.

I thought of updating the rates to include 192000 but I'm not sure how actually supported it is. The definition for it is present in downstream headers, but the datasheet for the AW88258 mentions 96 kHz as the largest rate, and I couldn't find a datasheet for the 88261 itself..

~val