ASoC: Add Richtek rt9123 and rt9123p support

sound/soc/codecs/rt9123.c new file +4 -0

+static int rt9123_xhandler_get(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_component *comp = snd_soc_kcontrol_component(kcontrol);
+	struct device *dev = comp->dev;

sound/soc/codecs/rt9123.c new file +5 -0

+	/* Trigger RG reset before regmap init cache */
+	ret = i2c_smbus_write_word_data(i2c, RT9123_REG_AMPCTRL, RT9123_MASK_SWRST);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to trigger RG reset\n");
+

Quoted 3 lines · On Fri, Apr 04, 2025 at 10:22:12PM +0800, [email protected] wrote:

sound/soc/codecs/rt9123.c new file +3 -0

+static int rt9123_enable_event(struct snd_soc_dapm_widget *w, struct snd_kcontrol *kcontrol,
+			       int event)
+{

sound/soc/codecs/rt9123.c line 78

Quoted 8 lines

sound/soc/codecs/rt9123.c new file +8 -0

+	ret = pm_runtime_resume_and_get(dev);
+	if (ret)
+		return ret;
+
+	snd_soc_component_write_field(comp, RT9123_REG_AMPCTRL, RT9123_MASK_AMPON, enable);
+
+	pm_runtime_mark_last_busy(dev);
+	pm_runtime_put_autosuspend(dev);

What's going on with the runtime PM stuff here? Especially for the DAPM widget usually the ASoC core will be able to keep devices runtime PM enabled so long as they are in use so I'd expect this not to have any impact. Why not just use a normal DAPM widget?

Quoted 4 lines

sound/soc/codecs/rt9123.c new file +4 -0

+static int rt9123_xhandler_get(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_component *comp = snd_soc_kcontrol_component(kcontrol);
+	struct device *dev = comp->dev;

sound/soc/codecs/rt9123.c line 130

Quoted 10 lines

sound/soc/codecs/rt9123.c new file +10 -0

+	int ret;
+
+	ret = pm_runtime_resume_and_get(dev);
+	if (ret)
+		return ret;
+
+	if (rt9123_kcontrol_name_comp(kcontrol, "SPK Gain Volume") == 0)
+		ret = snd_soc_get_volsw(kcontrol, ucontrol);
+	else
+		ret = snd_soc_get_enum_double(kcontrol, ucontrol);

This is even more unusual - it'll runtime PM enable the device every time we write to a control, even if the device is idle. The driver does implement a register cache so it's especially confusing, we'll power up the device, resync the cache, write to the hardware then power the device off again. Usually you'd just use the standard operations and then let the register writes get synced to the cache whenever it gets enabled for actual use. Again, why not just use standard controls?

Quoted 4 lines · Mark Brown

What's going on with the runtime PM stuff here? Especially for the DAPM widget usually the ASoC core will be able to keep devices runtime PM enabled so long as they are in use so I'd expect this not to have any impact. Why not just use a normal DAPM widget?

That's because The RG 0x01 'RT9123_REG_AMPCTRL' is mixed with other volatile status bitfield like as 'SW_RST', 'SYS_STATE'. That's why I use pm_runtime to make sure the RG can really be accessed at that time. Actually, the mixed RG bitfield for 'RW' and 'RO' is a bad design.

Quoted 11 lines · ChiYuan Huang

Quoted 4 lines · Mark Brown

What's going on with the runtime PM stuff here? Especially for the DAPM widget usually the ASoC core will be able to keep devices runtime PM enabled so long as they are in use so I'd expect this not to have any impact. Why not just use a normal DAPM widget?

That's because The RG 0x01 'RT9123_REG_AMPCTRL' is mixed with other volatile status bitfield like as 'SW_RST', 'SYS_STATE'. That's why I use pm_runtime to make sure the RG can really be accessed at that time. Actually, the mixed RG bitfield for 'RW' and 'RO' is a bad design.

You need some comments explaining what's going on here. If the volatile fields are read only shouldn't you be OK, so long as the register is not cached you should be able to do a read modify write fine? Unless the status bits are clear on read.

From: ChiYuan Huang <[email protected]>

Document the ASoC Richtek rt9123p.

Signed-off-by ChiYuan Huang <[email protected]>

---
 .../bindings/sound/richtek,rt9123p.yaml       | 50 +++++++++++++++++++
 1 file changed, 50 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/richtek,rt9123p.yaml

Documentation/devicetree/bindings/sound/richtek,rt9123p.yaml new file +24 -0

new file mode 100644
index 000000000000..836cd369a68c
@@ -0,0 +1,50 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/sound/richtek,rt9123p.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Richtek RT9123P Audio Amplifier
+
+maintainers:
+  - ChiYuan Huang <[email protected]>
+
+description:
+  RT9123P is a RT9123 variant which does not support I2C control.
+
+allOf:
+  - $ref: dai-common.yaml#
+
+properties:
+  compatible:
+    enum:
+      - richtek,rt9123p
+
+  '#sound-dai-cells':
+    const: 0

Documentation/devicetree/bindings/sound/richtek,rt9123p.yaml new file +16 -0

+
+required:
+  - compatible
+  - '#sound-dai-cells'
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    amplifier {
+        compatible = "richtek,rt9123p";
+        enable-gpios = <&gpio 26 GPIO_ACTIVE_HIGH>;
+        #sound-dai-cells = <0>;
+    };

Signature · 1 line

2.34.1

From: ChiYuan Huang <[email protected]>

Add codec driver for Richtek rt9123p.

Signed-off-by ChiYuan Huang <[email protected]>

---
 sound/soc/codecs/Kconfig   |   6 ++
 sound/soc/codecs/Makefile  |   2 +
 sound/soc/codecs/rt9123p.c | 171 +++++++++++++++++++++++++++++++++++++
 3 files changed, 179 insertions(+)
 create mode 100644 sound/soc/codecs/rt9123p.c

sound/soc/codecs/Kconfig +6 -0

index c61b2d3cf284..b0fa935846c0 100644
@@ -1832,6 +1832,12 @@ config SND_SOC_RT9123
 	  Enable support for the I2C control mode of Richtek RT9123 3.2W mono
 	  Class-D audio amplifier.
 
+config SND_SOC_RT9123P
+	tristate "Richtek RT9123P Mono Class-D Amplifier"
+	help
+	  Enable support for the HW control mode of Richtek RT9123P 3.2W mono
+	  Class-D audio amplifier.
+
 config SND_SOC_RTQ9128
 	tristate "Richtek RTQ9128 45W Digital Input Amplifier"
 	depends on I2C

sound/soc/codecs/Makefile +2 -0

index d8d0bc367af8..fba699701804 100644
@@ -271,6 +271,7 @@ snd-soc-rt721-sdca-y := rt721-sdca.o rt721-sdca-sdw.o
 snd-soc-rt722-sdca-y := rt722-sdca.o rt722-sdca-sdw.o
 snd-soc-rt9120-y := rt9120.o
 snd-soc-rt9123-y := rt9123.o
+snd-soc-rt9123p-y := rt9123p.o
 snd-soc-rtq9128-y := rtq9128.o
 snd-soc-sdw-mockup-y := sdw-mockup.o
 snd-soc-sgtl5000-y := sgtl5000.o

@@ -686,6 +687,7 @@ obj-$(CONFIG_SND_SOC_RT721_SDCA_SDW)     += snd-soc-rt721-sdca.o
 obj-$(CONFIG_SND_SOC_RT722_SDCA_SDW)     += snd-soc-rt722-sdca.o
 obj-$(CONFIG_SND_SOC_RT9120)	+= snd-soc-rt9120.o
 obj-$(CONFIG_SND_SOC_RT9123)	+= snd-soc-rt9123.o
+obj-$(CONFIG_SND_SOC_RT9123P)	+= snd-soc-rt9123p.o
 obj-$(CONFIG_SND_SOC_RTQ9128)	+= snd-soc-rtq9128.o
 obj-$(CONFIG_SND_SOC_SDW_MOCKUP)     += snd-soc-sdw-mockup.o
 obj-$(CONFIG_SND_SOC_SGTL5000)  += snd-soc-sgtl5000.o

sound/soc/codecs/rt9123p.c new file +122 -0

new file mode 100644
index 000000000000..b0ff5f856e4c
@@ -0,0 +1,171 @@
+// SPDX-License-Identifier: GPL-2.0-only
+//
+// rt9123p.c -- RT9123 (HW Mode) ALSA SoC Codec driver
+//
+// Author: ChiYuan Huang <[email protected]>
+
+#include <linux/acpi.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/gpio/consumer.h>
+#include <linux/kernel.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <sound/pcm.h>
+#include <sound/soc.h>
+#include <sound/soc-dai.h>
+#include <sound/soc-dapm.h>
+
+struct rt9123p_priv {
+	struct gpio_desc *enable;
+	unsigned int enable_delay;
+	int enable_switch;
+};
+
+static int rt9123p_daiops_trigger(struct snd_pcm_substream *substream, int cmd,
+				  struct snd_soc_dai *dai)
+{
+	struct snd_soc_component *comp = dai->component;
+	struct rt9123p_priv *rt9123p = snd_soc_component_get_drvdata(comp);
+
+	if (!rt9123p->enable)
+		return 0;
+
+	switch (cmd) {
+	case SNDRV_PCM_TRIGGER_START:
+	case SNDRV_PCM_TRIGGER_RESUME:
+	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
+		mdelay(rt9123p->enable_delay);
+		if (rt9123p->enable_switch) {
+			gpiod_set_value(rt9123p->enable, 1);
+			dev_dbg(comp->dev, "set enable to 1");
+		}
+		break;
+	case SNDRV_PCM_TRIGGER_STOP:
+	case SNDRV_PCM_TRIGGER_SUSPEND:
+	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+		gpiod_set_value(rt9123p->enable, 0);
+		dev_dbg(comp->dev, "set enable to 0");
+		break;
+	default:
+		break;
+	}
+
+	return 0;
+}
+
+static int rt9123p_enable_event(struct snd_soc_dapm_widget *w, struct snd_kcontrol *kcontrol,
+				int event)
+{
+	struct snd_soc_component *comp = snd_soc_dapm_to_component(w->dapm);
+	struct rt9123p_priv *rt9123p = snd_soc_component_get_drvdata(comp);
+
+	if (event & SND_SOC_DAPM_POST_PMU)
+		rt9123p->enable_switch = 1;
+	else if (event & SND_SOC_DAPM_POST_PMD)
+		rt9123p->enable_switch = 0;
+
+	return 0;
+}
+
+static const struct snd_soc_dapm_widget rt9123p_dapm_widgets[] = {
+	SND_SOC_DAPM_OUTPUT("SPK"),
+	SND_SOC_DAPM_OUT_DRV_E("Amp Drv", SND_SOC_NOPM, 0, 0, NULL, 0, rt9123p_enable_event,
+			       SND_SOC_DAPM_POST_PMU | SND_SOC_DAPM_POST_PMD),
+};
+
+static const struct snd_soc_dapm_route rt9123p_dapm_routes[] = {
+	{"Amp Drv", NULL, "HiFi Playback"},
+	{"SPK", NULL, "Amp Drv"},
+};
+
+static const struct snd_soc_component_driver rt9123p_comp_driver = {
+	.dapm_widgets		= rt9123p_dapm_widgets,
+	.num_dapm_widgets	= ARRAY_SIZE(rt9123p_dapm_widgets),
+	.dapm_routes		= rt9123p_dapm_routes,
+	.num_dapm_routes	= ARRAY_SIZE(rt9123p_dapm_routes),
+	.idle_bias_on		= 1,
+	.use_pmdown_time	= 1,
+	.endianness		= 1,
+};
+
+static const struct snd_soc_dai_ops rt9123p_dai_ops = {
+	.trigger        = rt9123p_daiops_trigger,
+};
+
+static struct snd_soc_dai_driver rt9123p_dai_driver = {
+	.name = "HiFi",
+	.playback = {
+		.stream_name	= "HiFi Playback",
+		.formats	= SNDRV_PCM_FMTBIT_S16 | SNDRV_PCM_FMTBIT_S24 |
+				  SNDRV_PCM_FMTBIT_S32,
+		.rates		= SNDRV_PCM_RATE_8000 | SNDRV_PCM_RATE_16000 |
+				  SNDRV_PCM_RATE_22050 | SNDRV_PCM_RATE_24000 |
+				  SNDRV_PCM_RATE_32000 | SNDRV_PCM_RATE_44100 |
+				  SNDRV_PCM_RATE_48000 | SNDRV_PCM_RATE_88200 |
+				  SNDRV_PCM_RATE_96000,
+		.rate_min	= 8000,
+		.rate_max	= 96000,
+		.channels_min	= 1,
+		.channels_max	= 2,
+	},
+	.ops    = &rt9123p_dai_ops,
+};
+
+static int rt9123p_platform_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct rt9123p_priv *rt9123p;
+	int ret;

Quoted 183 lines · On Fri, Apr 04, 2025 at 10:22:14PM +0800, [email protected] wrote:

From: ChiYuan Huang <[email protected]>

Add codec driver for Richtek rt9123p.

Signed-off-by ChiYuan Huang <[email protected]>

---
 sound/soc/codecs/Kconfig   |   6 ++
 sound/soc/codecs/Makefile  |   2 +
 sound/soc/codecs/rt9123p.c | 171 +++++++++++++++++++++++++++++++++++++
 3 files changed, 179 insertions(+)
 create mode 100644 sound/soc/codecs/rt9123p.c

sound/soc/codecs/Kconfig +6 -0

index c61b2d3cf284..b0fa935846c0 100644
@@ -1832,6 +1832,12 @@ config SND_SOC_RT9123
 	  Enable support for the I2C control mode of Richtek RT9123 3.2W mono
 	  Class-D audio amplifier.
 
+config SND_SOC_RT9123P
+	tristate "Richtek RT9123P Mono Class-D Amplifier"
+	help
+	  Enable support for the HW control mode of Richtek RT9123P 3.2W mono
+	  Class-D audio amplifier.
+
 config SND_SOC_RTQ9128
 	tristate "Richtek RTQ9128 45W Digital Input Amplifier"
 	depends on I2C

sound/soc/codecs/Makefile +2 -0

index d8d0bc367af8..fba699701804 100644
@@ -271,6 +271,7 @@ snd-soc-rt721-sdca-y := rt721-sdca.o rt721-sdca-sdw.o
 snd-soc-rt722-sdca-y := rt722-sdca.o rt722-sdca-sdw.o
 snd-soc-rt9120-y := rt9120.o
 snd-soc-rt9123-y := rt9123.o
+snd-soc-rt9123p-y := rt9123p.o
 snd-soc-rtq9128-y := rtq9128.o
 snd-soc-sdw-mockup-y := sdw-mockup.o
 snd-soc-sgtl5000-y := sgtl5000.o

@@ -686,6 +687,7 @@ obj-$(CONFIG_SND_SOC_RT721_SDCA_SDW)     += snd-soc-rt721-sdca.o
 obj-$(CONFIG_SND_SOC_RT722_SDCA_SDW)     += snd-soc-rt722-sdca.o
 obj-$(CONFIG_SND_SOC_RT9120)	+= snd-soc-rt9120.o
 obj-$(CONFIG_SND_SOC_RT9123)	+= snd-soc-rt9123.o
+obj-$(CONFIG_SND_SOC_RT9123P)	+= snd-soc-rt9123p.o
 obj-$(CONFIG_SND_SOC_RTQ9128)	+= snd-soc-rtq9128.o
 obj-$(CONFIG_SND_SOC_SDW_MOCKUP)     += snd-soc-sdw-mockup.o
 obj-$(CONFIG_SND_SOC_SGTL5000)  += snd-soc-sgtl5000.o

sound/soc/codecs/rt9123p.c new file +128 -0

new file mode 100644
index 000000000000..b0ff5f856e4c
@@ -0,0 +1,171 @@
+// SPDX-License-Identifier: GPL-2.0-only
+//
+// rt9123p.c -- RT9123 (HW Mode) ALSA SoC Codec driver
+//
+// Author: ChiYuan Huang <[email protected]>
+
+#include <linux/acpi.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/gpio/consumer.h>
+#include <linux/kernel.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <sound/pcm.h>
+#include <sound/soc.h>
+#include <sound/soc-dai.h>
+#include <sound/soc-dapm.h>
+
+struct rt9123p_priv {
+	struct gpio_desc *enable;
+	unsigned int enable_delay;
+	int enable_switch;
+};
+
+static int rt9123p_daiops_trigger(struct snd_pcm_substream *substream, int cmd,
+				  struct snd_soc_dai *dai)
+{
+	struct snd_soc_component *comp = dai->component;
+	struct rt9123p_priv *rt9123p = snd_soc_component_get_drvdata(comp);
+
+	if (!rt9123p->enable)
+		return 0;
+
+	switch (cmd) {
+	case SNDRV_PCM_TRIGGER_START:
+	case SNDRV_PCM_TRIGGER_RESUME:
+	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
+		mdelay(rt9123p->enable_delay);
+		if (rt9123p->enable_switch) {
+			gpiod_set_value(rt9123p->enable, 1);
+			dev_dbg(comp->dev, "set enable to 1");
+		}
+		break;
+	case SNDRV_PCM_TRIGGER_STOP:
+	case SNDRV_PCM_TRIGGER_SUSPEND:
+	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+		gpiod_set_value(rt9123p->enable, 0);
+		dev_dbg(comp->dev, "set enable to 0");
+		break;
+	default:
+		break;
+	}
+
+	return 0;
+}
+
+static int rt9123p_enable_event(struct snd_soc_dapm_widget *w, struct snd_kcontrol *kcontrol,
+				int event)
+{
+	struct snd_soc_component *comp = snd_soc_dapm_to_component(w->dapm);
+	struct rt9123p_priv *rt9123p = snd_soc_component_get_drvdata(comp);
+
+	if (event & SND_SOC_DAPM_POST_PMU)
+		rt9123p->enable_switch = 1;
+	else if (event & SND_SOC_DAPM_POST_PMD)
+		rt9123p->enable_switch = 0;
+
+	return 0;
+}
+
+static const struct snd_soc_dapm_widget rt9123p_dapm_widgets[] = {
+	SND_SOC_DAPM_OUTPUT("SPK"),
+	SND_SOC_DAPM_OUT_DRV_E("Amp Drv", SND_SOC_NOPM, 0, 0, NULL, 0, rt9123p_enable_event,
+			       SND_SOC_DAPM_POST_PMU | SND_SOC_DAPM_POST_PMD),
+};
+
+static const struct snd_soc_dapm_route rt9123p_dapm_routes[] = {
+	{"Amp Drv", NULL, "HiFi Playback"},
+	{"SPK", NULL, "Amp Drv"},
+};
+
+static const struct snd_soc_component_driver rt9123p_comp_driver = {
+	.dapm_widgets		= rt9123p_dapm_widgets,
+	.num_dapm_widgets	= ARRAY_SIZE(rt9123p_dapm_widgets),
+	.dapm_routes		= rt9123p_dapm_routes,
+	.num_dapm_routes	= ARRAY_SIZE(rt9123p_dapm_routes),
+	.idle_bias_on		= 1,
+	.use_pmdown_time	= 1,
+	.endianness		= 1,
+};
+
+static const struct snd_soc_dai_ops rt9123p_dai_ops = {
+	.trigger        = rt9123p_daiops_trigger,
+};
+
+static struct snd_soc_dai_driver rt9123p_dai_driver = {
+	.name = "HiFi",
+	.playback = {
+		.stream_name	= "HiFi Playback",
+		.formats	= SNDRV_PCM_FMTBIT_S16 | SNDRV_PCM_FMTBIT_S24 |
+				  SNDRV_PCM_FMTBIT_S32,
+		.rates		= SNDRV_PCM_RATE_8000 | SNDRV_PCM_RATE_16000 |
+				  SNDRV_PCM_RATE_22050 | SNDRV_PCM_RATE_24000 |
+				  SNDRV_PCM_RATE_32000 | SNDRV_PCM_RATE_44100 |
+				  SNDRV_PCM_RATE_48000 | SNDRV_PCM_RATE_88200 |
+				  SNDRV_PCM_RATE_96000,
+		.rate_min	= 8000,
+		.rate_max	= 96000,
+		.channels_min	= 1,
+		.channels_max	= 2,
+	},
+	.ops    = &rt9123p_dai_ops,
+};
+
+static int rt9123p_platform_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct rt9123p_priv *rt9123p;
+	int ret;
+
+	rt9123p = devm_kzalloc(dev, sizeof(*rt9123p), GFP_KERNEL);
+	if (!rt9123p)
+		return -ENOMEM;
+
+	rt9123p->enable = devm_gpiod_get_optional(dev, "enable", GPIOD_OUT_LOW);

sound/soc/codecs/rt9123p.c line 132

Quoted 4 lines

sound/soc/codecs/rt9123p.c new file +4 -0

+	if (IS_ERR(rt9123p->enable))
+		return PTR_ERR(rt9123p->enable);
+
+	ret = device_property_read_u32(dev, "enable-delay", &rt9123p->enable_delay);

Not documented. You have a single GPIO for any sort of control. What is this delay relative to? Why does it need to be tuned per board? Properties with units have unit suffix.

Rob

Quoted 184 lines · Rob Herring

Quoted 183 lines · On Fri, Apr 04, 2025 at 10:22:14PM +0800, [email protected] wrote:

From: ChiYuan Huang <[email protected]>

Add codec driver for Richtek rt9123p.

Signed-off-by ChiYuan Huang <[email protected]>

---
 sound/soc/codecs/Kconfig   |   6 ++
 sound/soc/codecs/Makefile  |   2 +
 sound/soc/codecs/rt9123p.c | 171 +++++++++++++++++++++++++++++++++++++
 3 files changed, 179 insertions(+)
 create mode 100644 sound/soc/codecs/rt9123p.c

sound/soc/codecs/Kconfig +6 -0

index c61b2d3cf284..b0fa935846c0 100644
@@ -1832,6 +1832,12 @@ config SND_SOC_RT9123
 	  Enable support for the I2C control mode of Richtek RT9123 3.2W mono
 	  Class-D audio amplifier.
 
+config SND_SOC_RT9123P
+	tristate "Richtek RT9123P Mono Class-D Amplifier"
+	help
+	  Enable support for the HW control mode of Richtek RT9123P 3.2W mono
+	  Class-D audio amplifier.
+
 config SND_SOC_RTQ9128
 	tristate "Richtek RTQ9128 45W Digital Input Amplifier"
 	depends on I2C

sound/soc/codecs/Makefile +2 -0

index d8d0bc367af8..fba699701804 100644
@@ -271,6 +271,7 @@ snd-soc-rt721-sdca-y := rt721-sdca.o rt721-sdca-sdw.o
 snd-soc-rt722-sdca-y := rt722-sdca.o rt722-sdca-sdw.o
 snd-soc-rt9120-y := rt9120.o
 snd-soc-rt9123-y := rt9123.o
+snd-soc-rt9123p-y := rt9123p.o
 snd-soc-rtq9128-y := rtq9128.o
 snd-soc-sdw-mockup-y := sdw-mockup.o
 snd-soc-sgtl5000-y := sgtl5000.o

@@ -686,6 +687,7 @@ obj-$(CONFIG_SND_SOC_RT721_SDCA_SDW)     += snd-soc-rt721-sdca.o
 obj-$(CONFIG_SND_SOC_RT722_SDCA_SDW)     += snd-soc-rt722-sdca.o
 obj-$(CONFIG_SND_SOC_RT9120)	+= snd-soc-rt9120.o
 obj-$(CONFIG_SND_SOC_RT9123)	+= snd-soc-rt9123.o
+obj-$(CONFIG_SND_SOC_RT9123P)	+= snd-soc-rt9123p.o
 obj-$(CONFIG_SND_SOC_RTQ9128)	+= snd-soc-rtq9128.o
 obj-$(CONFIG_SND_SOC_SDW_MOCKUP)     += snd-soc-sdw-mockup.o
 obj-$(CONFIG_SND_SOC_SGTL5000)  += snd-soc-sgtl5000.o

sound/soc/codecs/rt9123p.c new file +128 -0

new file mode 100644
index 000000000000..b0ff5f856e4c
@@ -0,0 +1,171 @@
+// SPDX-License-Identifier: GPL-2.0-only
+//
+// rt9123p.c -- RT9123 (HW Mode) ALSA SoC Codec driver
+//
+// Author: ChiYuan Huang <[email protected]>
+
+#include <linux/acpi.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/gpio/consumer.h>
+#include <linux/kernel.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <sound/pcm.h>
+#include <sound/soc.h>
+#include <sound/soc-dai.h>
+#include <sound/soc-dapm.h>
+
+struct rt9123p_priv {
+	struct gpio_desc *enable;
+	unsigned int enable_delay;
+	int enable_switch;
+};
+
+static int rt9123p_daiops_trigger(struct snd_pcm_substream *substream, int cmd,
+				  struct snd_soc_dai *dai)
+{
+	struct snd_soc_component *comp = dai->component;
+	struct rt9123p_priv *rt9123p = snd_soc_component_get_drvdata(comp);
+
+	if (!rt9123p->enable)
+		return 0;
+
+	switch (cmd) {
+	case SNDRV_PCM_TRIGGER_START:
+	case SNDRV_PCM_TRIGGER_RESUME:
+	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
+		mdelay(rt9123p->enable_delay);
+		if (rt9123p->enable_switch) {
+			gpiod_set_value(rt9123p->enable, 1);
+			dev_dbg(comp->dev, "set enable to 1");
+		}
+		break;
+	case SNDRV_PCM_TRIGGER_STOP:
+	case SNDRV_PCM_TRIGGER_SUSPEND:
+	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+		gpiod_set_value(rt9123p->enable, 0);
+		dev_dbg(comp->dev, "set enable to 0");
+		break;
+	default:
+		break;
+	}
+
+	return 0;
+}
+
+static int rt9123p_enable_event(struct snd_soc_dapm_widget *w, struct snd_kcontrol *kcontrol,
+				int event)
+{
+	struct snd_soc_component *comp = snd_soc_dapm_to_component(w->dapm);
+	struct rt9123p_priv *rt9123p = snd_soc_component_get_drvdata(comp);
+
+	if (event & SND_SOC_DAPM_POST_PMU)
+		rt9123p->enable_switch = 1;
+	else if (event & SND_SOC_DAPM_POST_PMD)
+		rt9123p->enable_switch = 0;
+
+	return 0;
+}
+
+static const struct snd_soc_dapm_widget rt9123p_dapm_widgets[] = {
+	SND_SOC_DAPM_OUTPUT("SPK"),
+	SND_SOC_DAPM_OUT_DRV_E("Amp Drv", SND_SOC_NOPM, 0, 0, NULL, 0, rt9123p_enable_event,
+			       SND_SOC_DAPM_POST_PMU | SND_SOC_DAPM_POST_PMD),
+};
+
+static const struct snd_soc_dapm_route rt9123p_dapm_routes[] = {
+	{"Amp Drv", NULL, "HiFi Playback"},
+	{"SPK", NULL, "Amp Drv"},
+};
+
+static const struct snd_soc_component_driver rt9123p_comp_driver = {
+	.dapm_widgets		= rt9123p_dapm_widgets,
+	.num_dapm_widgets	= ARRAY_SIZE(rt9123p_dapm_widgets),
+	.dapm_routes		= rt9123p_dapm_routes,
+	.num_dapm_routes	= ARRAY_SIZE(rt9123p_dapm_routes),
+	.idle_bias_on		= 1,
+	.use_pmdown_time	= 1,
+	.endianness		= 1,
+};
+
+static const struct snd_soc_dai_ops rt9123p_dai_ops = {
+	.trigger        = rt9123p_daiops_trigger,
+};
+
+static struct snd_soc_dai_driver rt9123p_dai_driver = {
+	.name = "HiFi",
+	.playback = {
+		.stream_name	= "HiFi Playback",
+		.formats	= SNDRV_PCM_FMTBIT_S16 | SNDRV_PCM_FMTBIT_S24 |
+				  SNDRV_PCM_FMTBIT_S32,
+		.rates		= SNDRV_PCM_RATE_8000 | SNDRV_PCM_RATE_16000 |
+				  SNDRV_PCM_RATE_22050 | SNDRV_PCM_RATE_24000 |
+				  SNDRV_PCM_RATE_32000 | SNDRV_PCM_RATE_44100 |
+				  SNDRV_PCM_RATE_48000 | SNDRV_PCM_RATE_88200 |
+				  SNDRV_PCM_RATE_96000,
+		.rate_min	= 8000,
+		.rate_max	= 96000,
+		.channels_min	= 1,
+		.channels_max	= 2,
+	},
+	.ops    = &rt9123p_dai_ops,
+};
+
+static int rt9123p_platform_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct rt9123p_priv *rt9123p;
+	int ret;
+
+	rt9123p = devm_kzalloc(dev, sizeof(*rt9123p), GFP_KERNEL);
+	if (!rt9123p)
+		return -ENOMEM;
+
+	rt9123p->enable = devm_gpiod_get_optional(dev, "enable", GPIOD_OUT_LOW);

Quoted 9 lines

sound/soc/codecs/rt9123p.c line 132

Quoted 4 lines

sound/soc/codecs/rt9123p.c new file +4 -0

+	if (IS_ERR(rt9123p->enable))
+		return PTR_ERR(rt9123p->enable);
+
+	ret = device_property_read_u32(dev, "enable-delay", &rt9123p->enable_delay);

Not documented. You have a single GPIO for any sort of control. What is this delay relative to? Why does it need to be tuned per board? Properties with units have unit suffix.

Like as the property desciption in the document, to wait clock or data to be ready. Sometimes, there's unknown jitter signal while clock or data starts. It'll be better to have a delay for this. That's why it's optional.

Thanks.

Quoted 1 line

Rob

In last mail, I have already described what the delay for.

Another question is for this property name 'enable-delay'. Should I add a suffix like as 'enable-delay-ms'?

This patch series adds Richtek rt9123 and rt9123p support. It's a 3.2W mono Class-D audio amplifier.

ChiYuan Huang (4):
ASoC: dt-bindings: Add bindings for Richtek rt9123
ASoC: codecs: Add support for Richtek rt9123
ASoC: dt-bindings: Add bindings for Richtek rt9123p
ASoC: codecs: Add support for Richtek rt9123p

[...]