[PATCH v2 0/3] Add support for AW8898 Amplifier

[PATCH v2 0/3] Add support for AW8898 Amplifier

[Luca Weiss]

Add devicetree bindings and a driver for the AW8898 Amplifier, and add
it to the devicetree for the Fairphone 3 smartphone.

Note, that this is the first audio driver I've worked on, so please
complain if something doesn't quite look right, it probably wasn't done
on purpose.

Unfortunately we can't wire up the full audio stack yet since the SoC
side is not upstream yet, the last revision from July 2024 can be found
here:

https://lore.kernel.org/linux-arm-msm/20240731-msm8953-msm8976-asoc-v3-0-163f23c3a28d@gmail.com/

Signed-off-by: Luca Weiss <luca@lucaweiss.eu>
---
Changes in v2:
- Make supplies required & use unevaluatedProperties in dt-schema
  (Krzysztof)
- Implement many suggestions from Mark in the driver, quite a major
  refactor, most of it is unraveling the initialization and using the
  proper callbacks
- Pick up tags
- Link to v1: https://lore.kernel.org/r/20250406-aw8898-v1-0-58a2d554693f@lucaweiss.eu

---
Luca Weiss (3):
      ASoC: dt-bindings: Document Awinic AW8898 amplifier
      ASoC: codecs: Add Awinic AW8898 amplifier driver
      arm64: dts: qcom: sdm632-fairphone-fp3: Add AW8898 amplifier

 .../devicetree/bindings/sound/awinic,aw8898.yaml   |  78 +++
 arch/arm64/boot/dts/qcom/sdm632-fairphone-fp3.dts  |  20 +
 sound/soc/codecs/Kconfig                           |  10 +
 sound/soc/codecs/Makefile                          |   1 +
 sound/soc/codecs/snd-soc-aw8898.c                  | 561 +++++++++++++++++++++
 5 files changed, 670 insertions(+)
---
base-commit: 1f5c752d94a7c3388dd562f5dd82f102ac7a3ffc
change-id: 20250406-aw8898-06988582d2e0

Best regards,
-- 
Luca Weiss <luca@lucaweiss.eu>

[PATCH v2 1/3] ASoC: dt-bindings: Document Awinic AW8898 amplifier

[Luca Weiss]

Add the bindings for the Awinic AW8898 amplifier.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Luca Weiss <luca@lucaweiss.eu>
---
 .../devicetree/bindings/sound/awinic,aw8898.yaml   | 78 ++++++++++++++++++++++
 1 file changed, 78 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/awinic,aw8898.yaml b/Documentation/devicetree/bindings/sound/awinic,aw8898.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..38eb0ba506527ed9e4420f3dfa13283580e56d27
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/awinic,aw8898.yaml
@@ -0,0 +1,78 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/sound/awinic,aw8898.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Awinic AW8898 Audio Amplifier
+
+maintainers:
+  - Luca Weiss <luca@lucaweiss.eu>
+
+description:
+  The AW8898 is an I2S/TDM input, high efficiency digital Smart K audio
+  amplifier with an integrated 9.5V smart boost converter, sound quality
+  enhancement algorithms and speaker protection.
+
+allOf:
+  - $ref: dai-common.yaml#
+
+properties:
+  compatible:
+    const: awinic,aw8898
+
+  reg:
+    maxItems: 1
+
+  reset-gpios:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  dvdd-supply:
+    description:
+      Digital power supply
+
+  vddio-supply:
+    description:
+      Digital IO power supply
+
+  vdd-supply:
+    description:
+      Battery power supply
+
+  "#sound-dai-cells":
+    const: 0
+
+required:
+  - compatible
+  - reg
+  - reset-gpios
+  - dvdd-supply
+  - vddio-supply
+  - vdd-supply
+  - "#sound-dai-cells"
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        audio-codec@34 {
+            compatible = "awinic,aw8898";
+            reg = <0x34>;
+            reset-gpios = <&tlmm 21 GPIO_ACTIVE_LOW>;
+            interrupts-extended = <&tlmm 20 IRQ_TYPE_LEVEL_HIGH>;
+            dvdd-supply = <&pm8953_l5>;
+            vddio-supply = <&pm8953_l5>;
+            vdd-supply = <&vph_pwr>;
+            #sound-dai-cells = <0>;
+        };
+    };

-- 
2.50.0

[PATCH v2 2/3] ASoC: codecs: Add Awinic AW8898 amplifier driver

[Luca Weiss]

Add a driver for the AW8898 Audio Amplifier.

Signed-off-by: Luca Weiss <luca@lucaweiss.eu>
---
 sound/soc/codecs/Kconfig          |  10 +
 sound/soc/codecs/Makefile         |   1 +
 sound/soc/codecs/snd-soc-aw8898.c | 561 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 572 insertions(+)

diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
index 6d7e4725d89cd33647770e4f2c4e81445b8335ce..18b0d356fdd2a085ce3adaded671ae639e925718 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -61,6 +61,7 @@ config SND_SOC_ALL_CODECS
 	imply SND_SOC_AW88166
 	imply SND_SOC_AW88261
 	imply SND_SOC_AW88399
+	imply SND_SOC_AW8898
 	imply SND_SOC_BT_SCO
 	imply SND_SOC_BD28623
 	imply SND_SOC_CHV3_CODEC
@@ -744,6 +745,15 @@ config SND_SOC_AW88399
 	  digital Smart K audio amplifier and SKTune speaker
 	  protection algorithms.
 
+config SND_SOC_AW8898
+	tristate "Awinic AW8898 Audio Amplifier"
+	depends on I2C
+	select REGMAP_I2C
+	select GPIOLIB
+	help
+	  Enable support for the Awinic AW8898 audio amplifier with I2C
+	  control.
+
 config SND_SOC_BD28623
 	tristate "ROHM BD28623 CODEC"
 	help
diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
index a68c3d192a1b6ccec513c6bc447c29be532ea70c..1006b7c3d94e3be3402e3c6b1d81eecfe828f94e 100644
--- a/sound/soc/codecs/Makefile
+++ b/sound/soc/codecs/Makefile
@@ -481,6 +481,7 @@ obj-$(CONFIG_SND_SOC_AW88395)	+=snd-soc-aw88395.o
 obj-$(CONFIG_SND_SOC_AW88166)	+=snd-soc-aw88166.o
 obj-$(CONFIG_SND_SOC_AW88261)	+=snd-soc-aw88261.o
 obj-$(CONFIG_SND_SOC_AW88399)	+= snd-soc-aw88399.o
+obj-$(CONFIG_SND_SOC_AW8898)	+= snd-soc-aw8898.o
 obj-$(CONFIG_SND_SOC_BD28623)	+= snd-soc-bd28623.o
 obj-$(CONFIG_SND_SOC_BT_SCO)	+= snd-soc-bt-sco.o
 obj-$(CONFIG_SND_SOC_CHV3_CODEC) += snd-soc-chv3-codec.o
diff --git a/sound/soc/codecs/snd-soc-aw8898.c b/sound/soc/codecs/snd-soc-aw8898.c
new file mode 100644
index 0000000000000000000000000000000000000000..6d60719777de9017a5419ec548a586352e86b60b
--- /dev/null
+++ b/sound/soc/codecs/snd-soc-aw8898.c
@@ -0,0 +1,561 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+//
+// Copyright (c) 2018 AWINIC Technology CO., LTD
+// Author: Nick Li <liweilei@awinic.com.cn>
+//
+// Copyright (c) 2025 Luca Weiss <luca@lucaweiss.eu>
+
+#include <linux/device.h>
+#include <linux/firmware.h>
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <sound/pcm.h>
+#include <sound/pcm_params.h>
+#include <sound/soc.h>
+#include <sound/tlv.h>
+
+/* Chip ID */
+#define AW8898_ID				0x00
+#define AW8898_CHIP_ID				0x1702
+
+/* System Status */
+#define AW8898_SYSST				0x01
+#define AW8898_SYSST_PLLS			GENMASK(0, 0)
+
+/* System Control */
+#define AW8898_SYSCTRL				0x04
+#define AW8898_SYSCTRL_MODE_MASK		GENMASK(7, 7)
+#define AW8898_SYSCTRL_RCV_MODE			(1)
+#define AW8898_SYSCTRL_SPK_MODE			(0)
+#define AW8898_SYSCTRL_PW_MASK			GENMASK(0, 0)
+#define AW8898_SYSCTRL_PW_PDN			(1)
+#define AW8898_SYSCTRL_PW_ACTIVE		(0)
+
+/* I2S Interface Control */
+#define AW8898_I2SCTRL				0x05
+#define AW8898_I2SCTRL_FMS_MASK			GENMASK(7, 6)
+#define AW8898_I2SCTRL_FMS_32BIT		(3)
+#define AW8898_I2SCTRL_FMS_24BIT		(2)
+#define AW8898_I2SCTRL_FMS_20BIT		(1)
+#define AW8898_I2SCTRL_FMS_16BIT		(0)
+#define AW8898_I2SCTRL_SR_MASK			GENMASK(3, 0)
+#define AW8898_I2SCTRL_SR_48K			(8)
+#define AW8898_I2SCTRL_SR_44P1K			(7)
+#define AW8898_I2SCTRL_SR_32K			(6)
+#define AW8898_I2SCTRL_SR_16K			(3)
+#define AW8898_I2SCTRL_SR_8K			(0)
+
+/* PWM Control */
+#define AW8898_PWMCTRL				0x08
+#define AW8898_PWMCTRL_HMUTE_MASK		GENMASK(0, 0)
+#define AW8898_PWMCTRL_HMUTE_ENABLE		(1)
+#define AW8898_PWMCTRL_HMUTE_DISABLE		(0)
+
+/* Hardware AGC Configuration 7 */
+#define AW8898_HAGCCFG7				0x0f
+#define AW8898_HAGCCFG7_VOL_REG_SHIFT		(8) /* bits 15-8 */
+#define AW8898_VOLUME_MAX			(255)
+#define AW8898_VOLUME_MIN			(0)
+
+#define AW8898_MAX_REGISTER			0xff
+
+#define AW8898_CFG_NAME				"aw8898_cfg.bin"
+
+#define AW8898_NUM_SUPPLIES	3
+static const char *aw8898_supply_names[AW8898_NUM_SUPPLIES] = {
+	"vdd",		/* Battery power */
+	"vddio",	/* Digital IO power */
+	"dvdd",		/* Digital power */
+};
+
+static const char * const aw8898_dev_mode_text[] = {
+	"Speaker",
+	"Receiver",
+};
+
+enum aw8898_mode {
+	AW8898_SPEAKER,
+	AW8898_RECEIVER,
+};
+
+static const struct soc_enum aw8898_dev_mode_enum =
+	SOC_ENUM_SINGLE_EXT(ARRAY_SIZE(aw8898_dev_mode_text), aw8898_dev_mode_text);
+
+struct aw8898 {
+	struct snd_soc_component *component;
+	struct regmap *regmap;
+	struct i2c_client *client;
+	struct gpio_desc *reset;
+	struct regulator_bulk_data supplies[AW8898_NUM_SUPPLIES];
+	enum aw8898_mode dev_mode;
+	bool cfg_loaded;
+};
+
+struct aw8898_cfg {
+	int len;
+	struct {
+		__le16 addr;
+		__le16 val;
+	} __packed data[];
+};
+
+static void aw8898_set_power(struct aw8898 *aw8898, bool on)
+{
+	unsigned int val = AW8898_SYSCTRL_PW_PDN;
+
+	if (on)
+		val = AW8898_SYSCTRL_PW_ACTIVE;
+
+	regmap_update_bits(aw8898->regmap, AW8898_SYSCTRL,
+			   AW8898_SYSCTRL_PW_MASK,
+			   FIELD_PREP(AW8898_SYSCTRL_PW_MASK, val));
+}
+
+static void aw8898_update_dev_mode(struct aw8898 *aw8898)
+{
+	unsigned int mode = AW8898_SYSCTRL_SPK_MODE;
+
+	if (aw8898->dev_mode == AW8898_RECEIVER)
+		mode = AW8898_SYSCTRL_RCV_MODE;
+
+	regmap_update_bits(aw8898->regmap, AW8898_SYSCTRL,
+			   AW8898_SYSCTRL_MODE_MASK,
+			   FIELD_PREP(AW8898_SYSCTRL_MODE_MASK, mode));
+}
+
+static void aw8898_cfg_write(struct aw8898 *aw8898,
+			      struct aw8898_cfg *aw8898_cfg)
+{
+	for (int i = 0; i < aw8898_cfg->len; i++) {
+		unsigned int addr = __le32_to_cpu(aw8898_cfg->data[i].addr);
+		unsigned int val = __le32_to_cpu(aw8898_cfg->data[i].val);
+
+		dev_dbg(&aw8898->client->dev, "cfg reg = 0x%04x, val = 0x%04x\n", addr, val);
+		regmap_write(aw8898->regmap, addr, val);
+	}
+}
+
+static void aw8898_fw_loaded(const struct firmware *fw, void *context)
+{
+	struct aw8898 *aw8898 = context;
+	struct aw8898_cfg *aw8898_cfg;
+
+	if (!fw) {
+		dev_err(&aw8898->client->dev, "Failed to load firmware\n");
+		return;
+	}
+
+	dev_dbg(&aw8898->client->dev, "Loaded %s - size: %zu\n", AW8898_CFG_NAME, fw->size);
+
+	if (fw->size % 4 != 0) {
+		dev_err(&aw8898->client->dev, "Invalid firmware size %zu\n", fw->size);
+		release_firmware(fw);
+		return;
+	}
+
+	aw8898_cfg = kzalloc(sizeof(int) + fw->size, GFP_KERNEL);
+	if (!aw8898_cfg) {
+		release_firmware(fw);
+		return;
+	}
+
+	aw8898_cfg->len = fw->size / 4;
+	memcpy(aw8898_cfg->data, fw->data, fw->size);
+
+	release_firmware(fw);
+
+	aw8898_cfg_write(aw8898, aw8898_cfg);
+
+	kfree(aw8898_cfg);
+
+	aw8898->cfg_loaded = true;
+
+	aw8898_update_dev_mode(aw8898);
+}
+
+static void aw8898_cold_start(struct aw8898 *aw8898)
+{
+	int err;
+
+	err = request_firmware_nowait(THIS_MODULE, FW_ACTION_UEVENT,
+				      AW8898_CFG_NAME, &aw8898->client->dev, GFP_KERNEL,
+				      aw8898, aw8898_fw_loaded);
+	if (err)
+		dev_err(&aw8898->client->dev, "Firmware load request failed: %d\n", err);
+}
+
+static int aw8898_dev_mode_get(struct snd_kcontrol *kcontrol,
+			       struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_component *component = snd_soc_kcontrol_component(kcontrol);
+	struct aw8898 *aw8898 = snd_soc_component_get_drvdata(component);
+
+	ucontrol->value.enumerated.item[0] = aw8898->dev_mode;
+
+	return 0;
+}
+
+static int aw8898_dev_mode_put(struct snd_kcontrol *kcontrol,
+			       struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_component *component = snd_soc_kcontrol_component(kcontrol);
+	struct aw8898 *aw8898 = snd_soc_component_get_drvdata(component);
+
+	if (ucontrol->value.enumerated.item[0] > 1)
+		return -EINVAL;
+
+	if (aw8898->dev_mode == ucontrol->value.enumerated.item[0])
+		return 0;
+
+	aw8898->dev_mode = ucontrol->value.enumerated.item[0];
+
+	aw8898_update_dev_mode(aw8898);
+
+	return 1;
+}
+
+/*
+ * -127.5 dB min, 0.5 dB steps, no mute
+ * Note: The official datasheet claims to be able to attenuate between 0 dB and
+ * -96 dB with 0.5 dB/step, but the register values are 0-255 so this doesn't
+ * really line up. It's a best guess.
+ */
+static const DECLARE_TLV_DB_SCALE(vol_tlv, -12750, 50, 0);
+
+static struct snd_kcontrol_new aw8898_controls[] = {
+	SOC_ENUM_EXT("Amp Mode", aw8898_dev_mode_enum,
+		     aw8898_dev_mode_get, aw8898_dev_mode_put),
+	SOC_SINGLE_RANGE_TLV("RX Volume", AW8898_HAGCCFG7,
+			     AW8898_HAGCCFG7_VOL_REG_SHIFT,
+			     AW8898_VOLUME_MIN, AW8898_VOLUME_MAX, 1, vol_tlv),
+};
+
+static int aw8898_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
+{
+	struct snd_soc_component *component = dai->component;
+
+	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
+	case SND_SOC_DAIFMT_I2S:
+		if ((fmt & SND_SOC_DAIFMT_CLOCK_PROVIDER_MASK)
+				!= SND_SOC_DAIFMT_CBC_CFC) {
+			dev_err(component->dev, "Invalid codec master mode: %d\n",
+				fmt & SND_SOC_DAIFMT_CLOCK_PROVIDER_MASK);
+			return -EINVAL;
+		}
+		break;
+	default:
+		dev_err(component->dev, "Unsupported DAI format %d\n",
+			fmt & SND_SOC_DAIFMT_FORMAT_MASK);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int aw8898_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 aw8898 *aw8898 = snd_soc_component_get_drvdata(dai->component);
+	unsigned int val;
+
+	switch (params_rate(params)) {
+	case 8000:
+		val = AW8898_I2SCTRL_SR_8K;
+		break;
+	case 16000:
+		val = AW8898_I2SCTRL_SR_16K;
+		break;
+	case 32000:
+		val = AW8898_I2SCTRL_SR_32K;
+		break;
+	case 44100:
+		val = AW8898_I2SCTRL_SR_44P1K;
+		break;
+	case 48000:
+		val = AW8898_I2SCTRL_SR_48K;
+		break;
+	default:
+		dev_err(component->dev, "Unsupported sample rate: %d\n",
+			params_rate(params));
+		return -EINVAL;
+	}
+
+	regmap_update_bits(aw8898->regmap, AW8898_I2SCTRL,
+			   AW8898_I2SCTRL_SR_MASK,
+			   FIELD_PREP(AW8898_I2SCTRL_SR_MASK, val));
+
+	switch (params_width(params)) {
+	case 16:
+		val = AW8898_I2SCTRL_FMS_16BIT;
+		break;
+	case 20:
+		val = AW8898_I2SCTRL_FMS_20BIT;
+		break;
+	case 24:
+		val = AW8898_I2SCTRL_FMS_24BIT;
+		break;
+	case 32:
+		val = AW8898_I2SCTRL_FMS_32BIT;
+		break;
+	default:
+		dev_err(component->dev, "Unsupported sample size: %d\n",
+			params_width(params));
+		return -EINVAL;
+	}
+
+	regmap_update_bits(aw8898->regmap, AW8898_I2SCTRL,
+			   AW8898_I2SCTRL_FMS_MASK,
+			   FIELD_PREP(AW8898_I2SCTRL_FMS_MASK, val));
+
+	return 0;
+}
+
+static int aw8898_mute(struct snd_soc_dai *dai, int mute, int stream)
+{
+	struct aw8898 *aw8898 = snd_soc_component_get_drvdata(dai->component);
+	unsigned int val = AW8898_PWMCTRL_HMUTE_DISABLE;
+
+	if (mute)
+		val = AW8898_PWMCTRL_HMUTE_ENABLE;
+
+	regmap_update_bits(aw8898->regmap, AW8898_PWMCTRL,
+			   AW8898_PWMCTRL_HMUTE_MASK,
+			   FIELD_PREP(AW8898_PWMCTRL_HMUTE_MASK, val));
+
+	return 0;
+}
+
+static int aw8898_startup(struct snd_pcm_substream *substream,
+               struct snd_soc_dai *dai)
+{
+	struct aw8898 *aw8898 = snd_soc_component_get_drvdata(dai->component);
+	unsigned int val;
+	int err;
+
+	err = regmap_read_poll_timeout(aw8898->regmap, AW8898_SYSST,
+				       val, val & AW8898_SYSST_PLLS,
+				       2000, 1 * USEC_PER_SEC);
+	if (err) {
+		dev_err(&aw8898->client->dev, "iis signal check error: %d\n", err);
+		aw8898_set_power(aw8898, false);
+		return err;
+	}
+
+	return 0;
+}
+
+static const struct snd_soc_dai_ops aw8898_dai_ops = {
+	.startup	= aw8898_startup,
+	.set_fmt	= aw8898_set_fmt,
+	.hw_params	= aw8898_hw_params,
+	.mute_stream	= aw8898_mute,
+};
+
+#define AW8898_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | \
+			SNDRV_PCM_FMTBIT_S24_LE | \
+			SNDRV_PCM_FMTBIT_S32_LE)
+
+static struct snd_soc_dai_driver aw8898_dai[] = {
+	{
+		.name = "aw8898-amplifier",
+		.playback = {
+			.stream_name = "Playback",
+			.channels_min = 1,
+			.channels_max = 2,
+			.rates = SNDRV_PCM_RATE_8000_48000,
+			.formats = AW8898_FORMATS,
+		},
+		.capture = {
+			.stream_name = "Capture",
+			.channels_min = 1,
+			.channels_max = 2,
+			.rates = SNDRV_PCM_RATE_8000_48000,
+			.formats = AW8898_FORMATS,
+		},
+		.ops = &aw8898_dai_ops,
+		.symmetric_rate = 1,
+		.symmetric_channels = 1,
+		.symmetric_sample_bits = 1,
+	},
+};
+
+static int aw8898_component_probe(struct snd_soc_component *component)
+{
+	struct aw8898 *aw8898 = snd_soc_component_get_drvdata(component);
+
+	aw8898->component = component;
+
+	return 0;
+}
+
+static int aw8898_drv_event(struct snd_soc_dapm_widget *w,
+				struct snd_kcontrol *kcontrol, int event)
+{
+	struct snd_soc_component *component = snd_soc_dapm_to_component(w->dapm);
+	struct aw8898 *aw8898 = snd_soc_component_get_drvdata(component);
+	int ret;
+
+	switch (event) {
+	case SND_SOC_DAPM_PRE_PMU:
+		aw8898_set_power(aw8898, true);
+
+		if (!aw8898->cfg_loaded)
+			aw8898_cold_start(aw8898);
+
+		ret = 0;
+		break;
+	case SND_SOC_DAPM_POST_PMD:
+		aw8898_set_power(aw8898, false);
+		ret = 0;
+		break;
+	default:
+		dev_err(component->dev, "%s: invalid event %d\n", __func__, event);
+		ret = -EINVAL;
+	}
+
+	return ret;
+}
+
+static const struct snd_soc_dapm_widget aw8898_dapm_widgets[] = {
+	SND_SOC_DAPM_INPUT("IN"),
+	SND_SOC_DAPM_PGA_E("SPK PA", SND_SOC_NOPM, 0, 0, NULL, 0, aw8898_drv_event,
+			       SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD),
+	SND_SOC_DAPM_OUTPUT("OUT"),
+};
+
+static const struct snd_soc_dapm_route aw8898_dapm_routes[] = {
+	{ "SPK PA", NULL, "IN" },
+	{ "OUT", NULL, "SPK PA" },
+};
+
+static const struct snd_soc_component_driver soc_component_dev_aw8898 = {
+	.probe			= aw8898_component_probe,
+	.dapm_widgets		= aw8898_dapm_widgets,
+	.num_dapm_widgets	= ARRAY_SIZE(aw8898_dapm_widgets),
+	.dapm_routes		= aw8898_dapm_routes,
+	.num_dapm_routes	= ARRAY_SIZE(aw8898_dapm_routes),
+	.controls		= aw8898_controls,
+	.num_controls		= ARRAY_SIZE(aw8898_controls),
+};
+
+static const struct regmap_config aw8898_regmap = {
+	.reg_bits = 8,
+	.val_bits = 16,
+
+	.max_register = AW8898_MAX_REGISTER,
+	.cache_type = REGCACHE_MAPLE,
+};
+
+static void aw8898_reset(struct aw8898 *aw8898)
+{
+	gpiod_set_value_cansleep(aw8898->reset, 1);
+	fsleep(1000);
+	gpiod_set_value_cansleep(aw8898->reset, 0);
+	fsleep(1000);
+}
+
+static int aw8898_check_chipid(struct aw8898 *aw8898)
+{
+	unsigned int reg;
+	int ret;
+
+	ret = regmap_read(aw8898->regmap, AW8898_ID, &reg);
+	if (ret < 0) {
+		dev_err(&aw8898->client->dev,
+			"Failed to read register AW8898_ID: %d\n", ret);
+		return ret;
+	}
+
+	dev_dbg(&aw8898->client->dev, "Read chip ID 0x%x\n", reg);
+
+	if (reg != AW8898_CHIP_ID) {
+		dev_err(&aw8898->client->dev, "Unexpected chip ID: 0x%x\n",
+			reg);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int aw8898_probe(struct i2c_client *client)
+{
+	struct aw8898 *aw8898;
+	int ret;
+
+	aw8898 = devm_kzalloc(&client->dev, sizeof(*aw8898), GFP_KERNEL);
+	if (!aw8898)
+		return -ENOMEM;
+
+	i2c_set_clientdata(client, aw8898);
+	aw8898->client = client;
+
+	aw8898->regmap = devm_regmap_init_i2c(client, &aw8898_regmap);
+	if (IS_ERR(aw8898->regmap))
+		return dev_err_probe(&client->dev, PTR_ERR(aw8898->regmap),
+				     "Failed to allocate register map\n");
+
+	for (int i = 0; i < ARRAY_SIZE(aw8898->supplies); i++)
+		aw8898->supplies[i].supply = aw8898_supply_names[i];
+
+	ret = devm_regulator_bulk_get(&client->dev, ARRAY_SIZE(aw8898->supplies),
+				      aw8898->supplies);
+	if (ret)
+		return dev_err_probe(&client->dev, ret,
+				     "Failed to get regulators\n");
+
+	ret = regulator_bulk_enable(ARRAY_SIZE(aw8898->supplies),
+				    aw8898->supplies);
+	if (ret) {
+		dev_err(&client->dev, "Failed to enable supplies: %d\n",
+			ret);
+		return ret;
+	}
+
+	aw8898->reset = devm_gpiod_get(&client->dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(aw8898->reset))
+		return dev_err_probe(&client->dev, PTR_ERR(aw8898->reset),
+				     "Failed to get reset GPIO\n");
+
+	aw8898_reset(aw8898);
+
+	ret = aw8898_check_chipid(aw8898);
+	if (ret)
+		return dev_err_probe(&client->dev, ret, "Chip ID check failed\n");
+
+	dev_set_drvdata(&client->dev, aw8898);
+
+	ret = devm_snd_soc_register_component(&client->dev, &soc_component_dev_aw8898,
+					      aw8898_dai, ARRAY_SIZE(aw8898_dai));
+	if (ret < 0)
+		return dev_err_probe(&client->dev, ret, "Failed to register component\n");
+
+	return 0;
+}
+
+static const struct i2c_device_id aw8898_id[] = {
+	{ "aw8898" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(i2c, aw8898_id);
+
+static const struct of_device_id aw8898_of_match[] = {
+	{ .compatible = "awinic,aw8898" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, aw8898_of_match);
+
+static struct i2c_driver aw8898_driver = {
+	.driver = {
+		.name = "aw8898",
+		.of_match_table = aw8898_of_match,
+	},
+	.probe = aw8898_probe,
+	.id_table = aw8898_id,
+};
+
+module_i2c_driver(aw8898_driver);
+
+MODULE_DESCRIPTION("AW8898 Audio amplifier driver");
+MODULE_LICENSE("GPL");

-- 
2.50.0

[PATCH v2 3/3] arm64: dts: qcom: sdm632-fairphone-fp3: Add AW8898 amplifier

[Luca Weiss]

Add a node for the amplifier found on Fairphone 3, receiving sound via
I2S from the SoC and being connected to the speaker.

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Luca Weiss <luca@lucaweiss.eu>
---
 arch/arm64/boot/dts/qcom/sdm632-fairphone-fp3.dts | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sdm632-fairphone-fp3.dts b/arch/arm64/boot/dts/qcom/sdm632-fairphone-fp3.dts
index 55a45b528bd3f1bf9b6fe7882753338b43c62271..053ed5cb6b2301cdf3f5e62a74bee535062bf845 100644
--- a/arch/arm64/boot/dts/qcom/sdm632-fairphone-fp3.dts
+++ b/arch/arm64/boot/dts/qcom/sdm632-fairphone-fp3.dts
@@ -97,6 +97,26 @@ nfc@28 {
 	};
 };
 
+&i2c_6 {
+	status = "okay";
+
+	aw8898: audio-amplifier@34 {
+		compatible = "awinic,aw8898";
+		reg = <0x34>;
+
+		reset-gpios = <&tlmm 21 GPIO_ACTIVE_LOW>;
+		interrupts-extended = <&tlmm 20 IRQ_TYPE_LEVEL_HIGH>;
+
+		dvdd-supply = <&pm8953_l5>;
+		vddio-supply = <&pm8953_l5>;
+		vdd-supply = <&vph_pwr>;
+
+		#sound-dai-cells = <0>;
+	};
+
+	/* Fairphone 3+ only: Speaker amp TAS2557 @ 4c */
+};
+
 &lpass {
 	firmware-name = "qcom/msm8953/fairphone/fp3/adsp.mbn";
 

-- 
2.50.0

Re: [PATCH v2 2/3] ASoC: codecs: Add Awinic AW8898 amplifier driver

[Christophe JAILLET]

Le 05/07/2025 à 14:03, Luca Weiss a écrit :
> Add a driver for the AW8898 Audio Amplifier.
> 
> Signed-off-by: Luca Weiss <luca@lucaweiss.eu>

Hi,

...

> +#define AW8898_CFG_NAME				"aw8898_cfg.bin"
> +
> +#define AW8898_NUM_SUPPLIES	3

See the probe below, but if simplified, AW8898_NUM_SUPPLIES would be 
useless and could be removed.

> +static const char *aw8898_supply_names[AW8898_NUM_SUPPLIES] = {

static const char * const ?

> +	"vdd",		/* Battery power */
> +	"vddio",	/* Digital IO power */
> +	"dvdd",		/* Digital power */
> +};
> +
> +static const char * const aw8898_dev_mode_text[] = {
> +	"Speaker",
> +	"Receiver",
> +};

...

> +static int aw8898_drv_event(struct snd_soc_dapm_widget *w,
> +				struct snd_kcontrol *kcontrol, int event)
> +{
> +	struct snd_soc_component *component = snd_soc_dapm_to_component(w->dapm);
> +	struct aw8898 *aw8898 = snd_soc_component_get_drvdata(component);
> +	int ret;

Maybe ret = 0; to simplify the code below?

Or, as done in aw8898_hw_params(), return -EINVAL; in the default case, 
and a plain return 0; at the end of the function?

> +
> +	switch (event) {
> +	case SND_SOC_DAPM_PRE_PMU:
> +		aw8898_set_power(aw8898, true);
> +
> +		if (!aw8898->cfg_loaded)
> +			aw8898_cold_start(aw8898);
> +
> +		ret = 0;
> +		break;
> +	case SND_SOC_DAPM_POST_PMD:
> +		aw8898_set_power(aw8898, false);
> +		ret = 0;
> +		break;
> +	default:
> +		dev_err(component->dev, "%s: invalid event %d\n", __func__, event);
> +		ret = -EINVAL;

Even if useless, having a break is more standard.

> +	}
> +
> +	return ret;
> +}

...

> +static int aw8898_check_chipid(struct aw8898 *aw8898)
> +{
> +	unsigned int reg;
> +	int ret;
> +
> +	ret = regmap_read(aw8898->regmap, AW8898_ID, &reg);
> +	if (ret < 0) {
> +		dev_err(&aw8898->client->dev,
> +			"Failed to read register AW8898_ID: %d\n", ret);

aw8898_check_chipid() is only called from the probe, so using return 
dev_err_probe() should be fine.

> +		return ret;
> +	}
> +
> +	dev_dbg(&aw8898->client->dev, "Read chip ID 0x%x\n", reg);
> +
> +	if (reg != AW8898_CHIP_ID) {
> +		dev_err(&aw8898->client->dev, "Unexpected chip ID: 0x%x\n",
> +			reg);

Same.

> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int aw8898_probe(struct i2c_client *client)
> +{
> +	struct aw8898 *aw8898;
> +	int ret;
> +
> +	aw8898 = devm_kzalloc(&client->dev, sizeof(*aw8898), GFP_KERNEL);
> +	if (!aw8898)
> +		return -ENOMEM;
> +
> +	i2c_set_clientdata(client, aw8898);
> +	aw8898->client = client;
> +
> +	aw8898->regmap = devm_regmap_init_i2c(client, &aw8898_regmap);
> +	if (IS_ERR(aw8898->regmap))
> +		return dev_err_probe(&client->dev, PTR_ERR(aw8898->regmap),
> +				     "Failed to allocate register map\n");
> +
> +	for (int i = 0; i < ARRAY_SIZE(aw8898->supplies); i++)
> +		aw8898->supplies[i].supply = aw8898_supply_names[i];
> +
> +	ret = devm_regulator_bulk_get(&client->dev, ARRAY_SIZE(aw8898->supplies),
> +				      aw8898->supplies);

devm_regulator_bulk_get_enable() would simplify the code and 'struct 
aw8898'.

> +	if (ret)
> +		return dev_err_probe(&client->dev, ret,
> +				     "Failed to get regulators\n");
> +
> +	ret = regulator_bulk_enable(ARRAY_SIZE(aw8898->supplies),
> +				    aw8898->supplies);
> +	if (ret) {
> +		dev_err(&client->dev, "Failed to enable supplies: %d\n",
> +			ret);

If dev_err_probe() to be consistent with the code below and above.
But this would be removed if devm_regulator_bulk_get_enable() is used.

> +		return ret;
> +	}
> +
> +	aw8898->reset = devm_gpiod_get(&client->dev, "reset", GPIOD_OUT_HIGH);
> +	if (IS_ERR(aw8898->reset))
> +		return dev_err_probe(&client->dev, PTR_ERR(aw8898->reset),
> +				     "Failed to get reset GPIO\n");
>

...

CJ

Re: [PATCH v2 2/3] ASoC: codecs: Add Awinic AW8898 amplifier driver

[Mark Brown]

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

On Sat, Jul 05, 2025 at 02:03:06PM +0200, Luca Weiss wrote:

> +static void aw8898_update_dev_mode(struct aw8898 *aw8898)
> +{
> +	unsigned int mode = AW8898_SYSCTRL_SPK_MODE;
> +
> +	if (aw8898->dev_mode == AW8898_RECEIVER)
> +		mode = AW8898_SYSCTRL_RCV_MODE;
> +
> +	regmap_update_bits(aw8898->regmap, AW8898_SYSCTRL,
> +			   AW8898_SYSCTRL_MODE_MASK,
> +			   FIELD_PREP(AW8898_SYSCTRL_MODE_MASK, mode));
> +}

Why is this open coded rather than just being a standard enum?  AFAICT
we never reference dev_mode outside of here or the _get() and put()
callbacks.  You might be looking for a _VALUE_ENUM?

> +	if (!fw) {
> +		dev_err(&aw8898->client->dev, "Failed to load firmware\n");
> +		return;
> +	}
> +
> +	dev_dbg(&aw8898->client->dev, "Loaded %s - size: %zu\n", AW8898_CFG_NAME, fw->size);

We print the firmware name we were looking for if we loaded it, but not
if we failed to load it when it's probably more useful.

> +	aw8898_cfg_write(aw8898, aw8898_cfg);

The "firmware" here is just a list of arbatrary register writes with no
validation of addresses or anything...

> +	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
> +	case SND_SOC_DAIFMT_I2S:
> +		if ((fmt & SND_SOC_DAIFMT_CLOCK_PROVIDER_MASK)
> +				!= SND_SOC_DAIFMT_CBC_CFC) {
> +			dev_err(component->dev, "Invalid codec master mode: %d\n",

Clock provider mode.

> +static int aw8898_startup(struct snd_pcm_substream *substream,
> +               struct snd_soc_dai *dai)
> +{
> +	struct aw8898 *aw8898 = snd_soc_component_get_drvdata(dai->component);
> +	unsigned int val;
> +	int err;
> +
> +	err = regmap_read_poll_timeout(aw8898->regmap, AW8898_SYSST,
> +				       val, val & AW8898_SYSST_PLLS,
> +				       2000, 1 * USEC_PER_SEC);

What's this actually checking?  You shouldn't rely on I2S being clocked
prior to trigger...

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

Re: [PATCH v2 2/3] ASoC: codecs: Add Awinic AW8898 amplifier driver

[Luca Weiss]

Hi Mark,

On 07-07-2025 2:02 p.m., Mark Brown wrote:
> On Sat, Jul 05, 2025 at 02:03:06PM +0200, Luca Weiss wrote:
> 
>> +static void aw8898_update_dev_mode(struct aw8898 *aw8898)
>> +{
>> +	unsigned int mode = AW8898_SYSCTRL_SPK_MODE;
>> +
>> +	if (aw8898->dev_mode == AW8898_RECEIVER)
>> +		mode = AW8898_SYSCTRL_RCV_MODE;
>> +
>> +	regmap_update_bits(aw8898->regmap, AW8898_SYSCTRL,
>> +			   AW8898_SYSCTRL_MODE_MASK,
>> +			   FIELD_PREP(AW8898_SYSCTRL_MODE_MASK, mode));
>> +}
> 
> Why is this open coded rather than just being a standard enum?  AFAICT
> we never reference dev_mode outside of here or the _get() and put()
> callbacks.  You might be looking for a _VALUE_ENUM?

Thanks for the reference, I'll take a look!

> 
>> +	if (!fw) {
>> +		dev_err(&aw8898->client->dev, "Failed to load firmware\n");
>> +		return;
>> +	}
>> +
>> +	dev_dbg(&aw8898->client->dev, "Loaded %s - size: %zu\n", AW8898_CFG_NAME, fw->size);
> 
> We print the firmware name we were looking for if we loaded it, but not
> if we failed to load it when it's probably more useful.

I can remove the debug print, I think if the firmware fails to load 
we'll also get an error print from the driver core with the file name? 
But I can double check it.

> 
>> +	aw8898_cfg_write(aw8898, aw8898_cfg);
> 
> The "firmware" here is just a list of arbatrary register writes with no
> validation of addresses or anything...

Yes... Got any suggestions how to make it better? This "firmware" file 
is the one that's also usable with the original driver from the 
amplifier vendor.

I honestly haven't really checked whether all the registers that are 
written there are documented well enough in the datasheet I have, so 
that this sequence could be replaced by proper DT values. But for sure 
already I know that some registers which are used and functional, are 
not documented at all unfortunately.

> 
>> +	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
>> +	case SND_SOC_DAIFMT_I2S:
>> +		if ((fmt & SND_SOC_DAIFMT_CLOCK_PROVIDER_MASK)
>> +				!= SND_SOC_DAIFMT_CBC_CFC) {
>> +			dev_err(component->dev, "Invalid codec master mode: %d\n",
> 
> Clock provider mode.

Sure, will update.

> 
>> +static int aw8898_startup(struct snd_pcm_substream *substream,
>> +               struct snd_soc_dai *dai)
>> +{
>> +	struct aw8898 *aw8898 = snd_soc_component_get_drvdata(dai->component);
>> +	unsigned int val;
>> +	int err;
>> +
>> +	err = regmap_read_poll_timeout(aw8898->regmap, AW8898_SYSST,
>> +				       val, val & AW8898_SYSST_PLLS,
>> +				       2000, 1 * USEC_PER_SEC);
> 
> What's this actually checking?  You shouldn't rely on I2S being clocked
> prior to trigger...

I've also taken this from the original driver, so I do not know the 
original purpose of it.

The register description is "System Status Register" "PLL locked status. 
1: locked, 0: unlocked", so presumably waiting for the amplifier itself 
to get ready for playing audio.

Regards
Luca

Re: [PATCH v2 2/3] ASoC: codecs: Add Awinic AW8898 amplifier driver

[Mark Brown]

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

On Mon, Jul 07, 2025 at 05:52:08PM +0200, Luca Weiss wrote:
> On 07-07-2025 2:02 p.m., Mark Brown wrote:
> > On Sat, Jul 05, 2025 at 02:03:06PM +0200, Luca Weiss wrote:

> > > +	aw8898_cfg_write(aw8898, aw8898_cfg);

> > The "firmware" here is just a list of arbatrary register writes with no
> > validation of addresses or anything...

> Yes... Got any suggestions how to make it better? This "firmware" file is
> the one that's also usable with the original driver from the amplifier
> vendor.

> I honestly haven't really checked whether all the registers that are written
> there are documented well enough in the datasheet I have, so that this
> sequence could be replaced by proper DT values. But for sure already I know
> that some registers which are used and functional, are not documented at all
> unfortunately.

So this probably isn't actually firmware but rather tuning.  At the very
least it'd be good to check for and handle attempts to configure
registers that the driver manages itself.

You'll also need to make the naming system specific so people can have
different tunings for different machines.

> > > +	err = regmap_read_poll_timeout(aw8898->regmap, AW8898_SYSST,
> > > +				       val, val & AW8898_SYSST_PLLS,
> > > +				       2000, 1 * USEC_PER_SEC);

> > What's this actually checking?  You shouldn't rely on I2S being clocked
> > prior to trigger...

> I've also taken this from the original driver, so I do not know the original
> purpose of it.

> The register description is "System Status Register" "PLL locked status. 1:
> locked, 0: unlocked", so presumably waiting for the amplifier itself to get
> ready for playing audio.

That sounds like it needs to be much later in proceedings then, you
can't rely on all the clocks being ready that early in proceedings.
Some devices will only start clocking when they start audio.  I'm going
to guess the clocking configuration is part of the "firmware" here which
is rather a mess.

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