[PATCH 0/4] ASoC: Add Richtek rt9123 and rt9123p support

[PATCH 0/4] ASoC: Add Richtek rt9123 and rt9123p support

[cy_huang]

From: ChiYuan Huang <cy_huang@richtek.com>

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

 .../bindings/sound/richtek,rt9123.yaml        |  56 ++
 .../bindings/sound/richtek,rt9123p.yaml       |  50 ++
 sound/soc/codecs/Kconfig                      |  15 +
 sound/soc/codecs/Makefile                     |   4 +
 sound/soc/codecs/rt9123.c                     | 484 ++++++++++++++++++
 sound/soc/codecs/rt9123p.c                    | 171 +++++++
 6 files changed, 780 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/richtek,rt9123.yaml
 create mode 100644 Documentation/devicetree/bindings/sound/richtek,rt9123p.yaml
 create mode 100644 sound/soc/codecs/rt9123.c
 create mode 100644 sound/soc/codecs/rt9123p.c


base-commit: a2cc6ff5ec8f91bc463fd3b0c26b61166a07eb11
-- 
2.34.1

[PATCH 1/4] ASoC: dt-bindings: Add bindings for Richtek rt9123

[cy_huang]

From: ChiYuan Huang <cy_huang@richtek.com>

Document the ASoC Richtek rt9123.

Signed-off-by: ChiYuan Huang <cy_huang@richtek.com>
---
 .../bindings/sound/richtek,rt9123.yaml        | 56 +++++++++++++++++++
 1 file changed, 56 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/richtek,rt9123.yaml

diff --git a/Documentation/devicetree/bindings/sound/richtek,rt9123.yaml b/Documentation/devicetree/bindings/sound/richtek,rt9123.yaml
new file mode 100644
index 000000000000..5acb05cdfefd
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/richtek,rt9123.yaml
@@ -0,0 +1,56 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/sound/richtek,rt9123.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Richtek RT9123 Audio Amplifier
+
+maintainers:
+  - ChiYuan Huang <cy_huang@richtek.com>
+
+description:
+  RT9123 is a 3.2W mono Class-D audio amplifier that features high efficiency
+  and performance with ultra-low quiescent current. The digital audio interface
+  support various formats, including I2S, left-justified, right-justified, and
+  TDM formats.
+
+allOf:
+  - $ref: dai-common.yaml#
+
+properties:
+  compatible:
+    enum:
+      - richtek,rt9123
+
+  reg:
+    maxItems: 1
+
+  '#sound-dai-cells':
+    const: 0
+
+  enable-gpios:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - '#sound-dai-cells'
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        amplifier@5e {
+            compatible = "richtek,rt9123";
+            reg = <0x5e>;
+            enable-gpios = <&gpio 26 GPIO_ACTIVE_HIGH>;
+            #sound-dai-cells = <0>;
+        };
+    };
-- 
2.34.1

[PATCH 2/4] ASoC: codecs: Add support for Richtek rt9123

[cy_huang]

From: ChiYuan Huang <cy_huang@richtek.com>

Add codec driver for Richtek rt9123.

Signed-off-by: ChiYuan Huang <cy_huang@richtek.com>
---
 sound/soc/codecs/Kconfig  |   9 +
 sound/soc/codecs/Makefile |   2 +
 sound/soc/codecs/rt9123.c | 484 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 495 insertions(+)
 create mode 100644 sound/soc/codecs/rt9123.c

diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
index 40bb7a1d44bc..c61b2d3cf284 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -234,6 +234,7 @@ config SND_SOC_ALL_CODECS
 	imply SND_SOC_RT1318_SDW
 	imply SND_SOC_RT1320_SDW
 	imply SND_SOC_RT9120
+	imply SND_SOC_RT9123
 	imply SND_SOC_RTQ9128
 	imply SND_SOC_SDW_MOCKUP
 	imply SND_SOC_SGTL5000
@@ -1823,6 +1824,14 @@ config SND_SOC_RT9120
 	  Enable support for Richtek RT9120 20W, stereo, inductor-less,
 	  high-efficiency Class-D audio amplifier.
 
+config SND_SOC_RT9123
+	tristate "Richtek RT9123 Mono Class-D Amplifier"
+	depends on I2C
+	select REGMAP_I2C
+	help
+	  Enable support for the I2C control mode of Richtek RT9123 3.2W mono
+	  Class-D audio amplifier.
+
 config SND_SOC_RTQ9128
 	tristate "Richtek RTQ9128 45W Digital Input Amplifier"
 	depends on I2C
diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
index 10f726066b6c..d8d0bc367af8 100644
--- a/sound/soc/codecs/Makefile
+++ b/sound/soc/codecs/Makefile
@@ -270,6 +270,7 @@ snd-soc-rt715-sdca-y := rt715-sdca.o rt715-sdca-sdw.o
 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-rtq9128-y := rtq9128.o
 snd-soc-sdw-mockup-y := sdw-mockup.o
 snd-soc-sgtl5000-y := sgtl5000.o
@@ -684,6 +685,7 @@ obj-$(CONFIG_SND_SOC_RT715_SDCA_SDW)     += snd-soc-rt715-sdca.o
 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_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
diff --git a/sound/soc/codecs/rt9123.c b/sound/soc/codecs/rt9123.c
new file mode 100644
index 000000000000..16689c5c2db7
--- /dev/null
+++ b/sound/soc/codecs/rt9123.c
@@ -0,0 +1,484 @@
+// SPDX-License-Identifier: GPL-2.0-only
+//
+// rt9123.c -- RT9123 (SW I2C Mode) ALSA SoC Codec driver
+//
+// Author: ChiYuan Huang <cy_huang@richtek.com>
+
+#include <linux/acpi.h>
+#include <linux/bits.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/kernel.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/pm_runtime.h>
+#include <linux/property.h>
+#include <linux/regmap.h>
+#include <sound/pcm.h>
+#include <sound/pcm_params.h>
+#include <sound/soc.h>
+#include <sound/soc-dai.h>
+#include <sound/soc-dapm.h>
+#include <sound/tlv.h>
+
+#define RT9123_REG_AMPCTRL	0x01
+#define RT9123_REG_I2SOPT	0x02
+#define RT9123_REG_TDMRX	0x03
+#define RT9123_REG_SILVOLEN	0x04
+#define RT9123_REG_VOLGAIN	0x12
+#define RT9123_REG_ANAFLAG	0x36
+#define RT9123_REG_COMBOID	0xF7
+
+#define RT9123_MASK_SWRST	BIT(15)
+#define RT9123_MASK_SWMUTE	BIT(14)
+#define RT9123_MASK_AMPON	BIT(12)
+#define RT9123_MASK_AUDBIT	GENMASK(14, 12)
+#define RT9123_MASK_AUDFMT	GENMASK(11, 8)
+#define RT9123_MASK_TDMRXLOC	GENMASK(4, 0)
+#define RT9123_MASK_VENID	GENMASK(15, 4)
+
+#define RT9123_FIXED_VENID	0x340
+
+struct rt9123_priv {
+	struct gpio_desc *enable;
+	unsigned int dai_fmt;
+	int tdm_slots;
+	int tdm_slot_width;
+};
+
+static int rt9123_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 device *dev = comp->dev;
+	unsigned int enable;
+	int ret;
+
+	switch (event) {
+	case SND_SOC_DAPM_POST_PMU:
+		enable = 1;
+		break;
+	case SND_SOC_DAPM_POST_PMD:
+		enable = 0;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	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);
+
+	return 0;
+}
+
+static const struct snd_soc_dapm_widget rt9123_dapm_widgets[] = {
+	SND_SOC_DAPM_OUTPUT("SPK"),
+	SND_SOC_DAPM_OUT_DRV_E("Amp Drv", SND_SOC_NOPM, 0, 0, NULL, 0, rt9123_enable_event,
+			       SND_SOC_DAPM_POST_PMU | SND_SOC_DAPM_POST_PMD),
+};
+
+static const struct snd_soc_dapm_route rt9123_dapm_routes[] = {
+	{ "Amp Drv", NULL, "HiFi Playback" },
+	{ "SPK", NULL, "Amp Drv" },
+};
+
+static const DECLARE_TLV_DB_SCALE(dig_tlv, -10375, 25, 0);
+static const DECLARE_TLV_DB_RANGE(ana_tlv,
+				  0, 0, TLV_DB_SCALE_ITEM(-1200, 0, 0),
+				  1, 9, TLV_DB_SCALE_ITEM(0, 150, 0),
+				  10, 10, TLV_DB_SCALE_ITEM(1400, 0, 0));
+static const char * const pwmfreq_text[] = { "300KHz", "325KHz", "350KHz", "375KHz" };
+static const struct soc_enum rt9123_pwm_freq_enum =
+	SOC_ENUM_SINGLE(RT9123_REG_AMPCTRL, 4, ARRAY_SIZE(pwmfreq_text), pwmfreq_text);
+static const char * const i2sch_text[] = { "(L+R)/2", "LCH", "RCH", "(L+R)/2" };
+static const struct soc_enum rt9123_i2sch_select_enum =
+	SOC_ENUM_SINGLE(RT9123_REG_I2SOPT, 4, ARRAY_SIZE(i2sch_text), i2sch_text);
+
+static int rt9123_kcontrol_name_comp(struct snd_kcontrol *kcontrol, const char *s)
+{
+	struct snd_soc_component *comp = snd_soc_kcontrol_component(kcontrol);
+	const char *kctlname = kcontrol->id.name;
+
+	if (comp && comp->name_prefix)
+		kctlname += strlen(comp->name_prefix) + 1;
+
+	return strcmp(kctlname, s);
+}
+
+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;
+	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);
+
+	if (ret < 0)
+		dev_err(dev, "Failed to get control (%d)\n", ret);
+
+	pm_runtime_mark_last_busy(dev);
+	pm_runtime_put_autosuspend(dev);
+	return ret;
+}
+
+static int rt9123_xhandler_put(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;
+	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_put_volsw(kcontrol, ucontrol);
+	else
+		ret = snd_soc_put_enum_double(kcontrol, ucontrol);
+
+	if (ret < 0)
+		dev_err(dev, "Failed to put control (%d)\n", ret);
+
+	pm_runtime_mark_last_busy(dev);
+	pm_runtime_put_autosuspend(dev);
+	return ret;
+}
+
+static const struct snd_kcontrol_new rt9123_controls[] = {
+	SOC_SINGLE_TLV("Master Volume", RT9123_REG_VOLGAIN, 2, 511, 1, dig_tlv),
+	SOC_SINGLE_EXT_TLV("SPK Gain Volume", RT9123_REG_AMPCTRL, 0, 10, 0, rt9123_xhandler_get,
+			   rt9123_xhandler_put, ana_tlv),
+	SOC_ENUM_EXT("PWM Frequency Select", rt9123_pwm_freq_enum, rt9123_xhandler_get,
+		     rt9123_xhandler_put),
+	SOC_ENUM("I2S CH Select", rt9123_i2sch_select_enum),
+	SOC_SINGLE("Silence Detect Switch", RT9123_REG_SILVOLEN, 14, 1, 0),
+};
+
+static const struct snd_soc_component_driver rt9123_comp_driver = {
+	.controls		= rt9123_controls,
+	.num_controls		= ARRAY_SIZE(rt9123_controls),
+	.dapm_widgets		= rt9123_dapm_widgets,
+	.num_dapm_widgets	= ARRAY_SIZE(rt9123_dapm_widgets),
+	.dapm_routes		= rt9123_dapm_routes,
+	.num_dapm_routes	= ARRAY_SIZE(rt9123_dapm_routes),
+	.use_pmdown_time	= 1,
+	.endianness		= 1,
+};
+
+static int rt9123_dai_set_format(struct snd_soc_dai *dai, unsigned int fmt)
+{
+	struct rt9123_priv *rt9123 = snd_soc_dai_get_drvdata(dai);
+
+	rt9123->dai_fmt = fmt;
+	return 0;
+}
+
+static int rt9123_dai_set_tdm_slot(struct snd_soc_dai *dai, unsigned int tx_mask,
+				   unsigned int rx_mask, int slots, int slot_width)
+{
+	struct rt9123_priv *rt9123 = snd_soc_dai_get_drvdata(dai);
+	struct snd_soc_component *comp = dai->component;
+	struct device *dev = dai->dev;
+	unsigned int rx_loc;
+
+	dev_dbg(dev, "(slots, slot_width) = (%d, %d), (txmask, rxmask) = 0x%x, 0x%x\n", slots,
+		slot_width, tx_mask, rx_mask);
+
+	if (slots <= 0 || slot_width <= 0 || slots % 2 || slot_width % 8 ||
+			slots * slot_width > 256) {
+		dev_err(dev, "Invalid slot parameter (%d, %d)\n", slots, slot_width);
+		return -EINVAL;
+	}
+
+	if (!rx_mask || hweight_long(rx_mask) > 1 || ffs(rx_mask) > slots) {
+		dev_err(dev, "Invalid rx_mask 0x%08x, slots = %d\n", rx_mask, slots);
+		return -EINVAL;
+	}
+
+	/* Configure rx channel data location */
+	rx_loc = (ffs(rx_mask) - 1) * slot_width / 8;
+	snd_soc_component_write_field(comp, RT9123_REG_TDMRX, RT9123_MASK_TDMRXLOC, rx_loc);
+
+	rt9123->tdm_slots = slots;
+	rt9123->tdm_slot_width = slot_width;
+
+	return 0;
+}
+
+static int rt9123_dai_hw_params(struct snd_pcm_substream *substream,
+				struct snd_pcm_hw_params *param, struct snd_soc_dai *dai)
+{
+	struct rt9123_priv *rt9123 = snd_soc_dai_get_drvdata(dai);
+	struct snd_soc_component *comp = dai->component;
+	unsigned int fmtval, width, slot_width;
+	struct device *dev = dai->dev;
+	unsigned int audfmt, audbit;
+
+	fmtval = FIELD_GET(SND_SOC_DAIFMT_FORMAT_MASK, rt9123->dai_fmt);
+	if (rt9123->tdm_slots && fmtval != SND_SOC_DAIFMT_DSP_A && fmtval != SND_SOC_DAIFMT_DSP_B) {
+		dev_err(dev, "TDM only can support DSP_A or DSP_B format\n");
+		return -EINVAL;
+	}
+
+	switch (fmtval) {
+	case SND_SOC_DAIFMT_I2S:
+		audfmt = 0;
+		break;
+	case SND_SOC_DAIFMT_LEFT_J:
+		audfmt = 1;
+		break;
+	case SND_SOC_DAIFMT_RIGHT_J:
+		audfmt = 2;
+		break;
+	case SND_SOC_DAIFMT_DSP_B:
+		audfmt = rt9123->tdm_slots ? 4 : 3;
+		break;
+	case SND_SOC_DAIFMT_DSP_A:
+		audfmt = rt9123->tdm_slots ? 12 : 11;
+		break;
+	default:
+		dev_err(dev, "Unsupported format %d\n", fmtval);
+		return -EINVAL;
+	}
+
+	switch (width = params_width(param)) {
+	case 16:
+		audbit = 0;
+		break;
+	case 20:
+		audbit = 1;
+		break;
+	case 24:
+		audbit = 2;
+		break;
+	case 32:
+		audbit = 3;
+		break;
+	case 8:
+		audbit = 4;
+		break;
+	default:
+		dev_err(dev, "Unsupported width %d\n", width);
+		return -EINVAL;
+	}
+
+	slot_width = params_physical_width(param);
+	if (rt9123->tdm_slots && slot_width > rt9123->tdm_slot_width) {
+		dev_err(dev, "Slot width is larger than TDM slot width\n");
+		return -EINVAL;
+	}
+
+	snd_soc_component_write_field(comp, RT9123_REG_I2SOPT, RT9123_MASK_AUDFMT, audfmt);
+	snd_soc_component_write_field(comp, RT9123_REG_I2SOPT, RT9123_MASK_AUDBIT, audbit);
+
+	return 0;
+}
+
+static const struct snd_soc_dai_ops rt9123_dai_ops = {
+	.set_fmt	= rt9123_dai_set_format,
+	.set_tdm_slot	= rt9123_dai_set_tdm_slot,
+	.hw_params	= rt9123_dai_hw_params,
+};
+
+static struct snd_soc_dai_driver rt9123_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    = &rt9123_dai_ops,
+};
+
+static bool rt9123_readable_reg(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case 0x00 ... 0x05:
+	case 0x12 ... 0x13:
+	case 0x20 ... 0x21:
+	case 0x36:
+	case 0xF7:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static bool rt9123_writeable_reg(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case 0x01 ... 0x05:
+	case 0x12 ... 0x13:
+	case 0x20 ... 0x21:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static bool rt9123_volatile_reg(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case 0x01:
+	case 0x20:
+	case 0x36:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static const struct regmap_config rt9123_regmap_config = {
+	.name			= "rt9123",
+	.reg_bits		= 8,
+	.val_bits		= 16,
+	.val_format_endian	= REGMAP_ENDIAN_BIG,
+	.readable_reg		= rt9123_readable_reg,
+	.writeable_reg		= rt9123_writeable_reg,
+	.volatile_reg		= rt9123_volatile_reg,
+	.cache_type		= REGCACHE_RBTREE,
+	.num_reg_defaults_raw	= RT9123_REG_COMBOID + 1,
+};
+
+static int rt9123_i2c_probe(struct i2c_client *i2c)
+{
+	struct device *dev = &i2c->dev;
+	struct rt9123_priv *rt9123;
+	struct regmap *regmap;
+	unsigned int venid;
+	int ret;
+
+	rt9123 = devm_kzalloc(dev, sizeof(*rt9123), GFP_KERNEL);
+	if (!rt9123)
+		return -ENOMEM;
+
+	rt9123->enable = devm_gpiod_get_optional(dev, "enable", GPIOD_OUT_HIGH);
+	if (IS_ERR(rt9123->enable))
+		return PTR_ERR(rt9123->enable);
+	else if (rt9123->enable)
+		usleep_range(250, 350);
+	else
+		dev_dbg(dev, "No 'enable' GPIO specified, treat it as default on\n");
+
+	/* 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");
+
+	regmap = devm_regmap_init_i2c(i2c, &rt9123_regmap_config);
+	if (IS_ERR(regmap))
+		return dev_err_probe(dev, PTR_ERR(regmap), "Failed to init regmap\n");
+
+	ret = regmap_read(regmap, RT9123_REG_COMBOID, &venid);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to read vendor-id\n");
+
+	if ((venid & RT9123_MASK_VENID) != RT9123_FIXED_VENID)
+		return dev_err_probe(dev, -ENODEV, "Incorrect vendor-id 0x%04x\n", venid);
+
+	i2c_set_clientdata(i2c, rt9123);
+
+	pm_runtime_set_autosuspend_delay(dev, 500);
+	pm_runtime_use_autosuspend(dev);
+	pm_runtime_set_active(dev);
+	ret = devm_pm_runtime_enable(dev);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to enable pm runtime\n");
+
+	return devm_snd_soc_register_component(dev, &rt9123_comp_driver, &rt9123_dai_driver, 1);
+}
+
+#ifdef CONFIG_PM
+static int rt9123_runtime_suspend(struct device *dev)
+{
+	struct rt9123_priv *rt9123 = dev_get_drvdata(dev);
+	struct regmap *regmap = dev_get_regmap(dev, NULL);
+
+	if (rt9123->enable) {
+		regcache_cache_only(regmap, true);
+		regcache_mark_dirty(regmap);
+		gpiod_set_value(rt9123->enable, 0);
+	}
+
+	return 0;
+}
+
+static int rt9123_runtime_resume(struct device *dev)
+{
+	struct rt9123_priv *rt9123 = dev_get_drvdata(dev);
+	struct regmap *regmap = dev_get_regmap(dev, NULL);
+	int ret;
+
+	if (rt9123->enable) {
+		gpiod_set_value(rt9123->enable, 1);
+		usleep_range(250, 350);
+
+		regcache_cache_only(regmap, false);
+		ret = regcache_sync(regmap);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static const struct dev_pm_ops rt9123_dev_pm_ops = {
+	SET_RUNTIME_PM_OPS(rt9123_runtime_suspend, rt9123_runtime_resume, NULL)
+};
+#endif
+
+#ifdef CONFIG_OF
+static const struct of_device_id rt9123_device_id[] = {
+	{ .compatible = "richtek,rt9123" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, rt9123_device_id);
+#endif
+
+#ifdef CONFIG_ACPI
+static const struct acpi_device_id rt9123_acpi_match[] = {
+	{ "RT9123", 0 },
+	{}
+};
+MODULE_DEVICE_TABLE(acpi, rt9123_acpi_match);
+#endif
+
+static struct i2c_driver rt9123_i2c_driver = {
+	.driver = {
+		.name = "rt9123",
+		.of_match_table = of_match_ptr(rt9123_device_id),
+		.acpi_match_table = ACPI_PTR(rt9123_acpi_match),
+		.pm = pm_ptr(&rt9123_dev_pm_ops),
+	},
+	.probe	= rt9123_i2c_probe,
+};
+module_i2c_driver(rt9123_i2c_driver);
+
+MODULE_AUTHOR("ChiYuan Huang <cy_huang@richtek.com>");
+MODULE_DESCRIPTION("ASoC rt9123 Driver");
+MODULE_LICENSE("GPL");
-- 
2.34.1

[PATCH 3/4] ASoC: dt-bindings: Add bindings for Richtek rt9123p

[cy_huang]

From: ChiYuan Huang <cy_huang@richtek.com>

Document the ASoC Richtek rt9123p.

Signed-off-by: ChiYuan Huang <cy_huang@richtek.com>
---
 .../bindings/sound/richtek,rt9123p.yaml       | 50 +++++++++++++++++++
 1 file changed, 50 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/richtek,rt9123p.yaml

diff --git a/Documentation/devicetree/bindings/sound/richtek,rt9123p.yaml b/Documentation/devicetree/bindings/sound/richtek,rt9123p.yaml
new file mode 100644
index 000000000000..836cd369a68c
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/richtek,rt9123p.yaml
@@ -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 <cy_huang@richtek.com>
+
+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
+
+  enable-gpios:
+    maxItems: 1
+
+  enable-delay:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      Delay time for 'ENABLE' pin changes intended to make I2S clocks ready to
+      prevent speaker pop noise. The unit is in millisecond.
+    default: 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>;
+    };
-- 
2.34.1

[PATCH 4/4] ASoC: codecs: Add support for Richtek rt9123p

[cy_huang]

From: ChiYuan Huang <cy_huang@richtek.com>

Add codec driver for Richtek rt9123p.

Signed-off-by: ChiYuan Huang <cy_huang@richtek.com>
---
 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

diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
index c61b2d3cf284..b0fa935846c0 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -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
diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
index d8d0bc367af8..fba699701804 100644
--- a/sound/soc/codecs/Makefile
+++ b/sound/soc/codecs/Makefile
@@ -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
diff --git a/sound/soc/codecs/rt9123p.c b/sound/soc/codecs/rt9123p.c
new file mode 100644
index 000000000000..b0ff5f856e4c
--- /dev/null
+++ b/sound/soc/codecs/rt9123p.c
@@ -0,0 +1,171 @@
+// SPDX-License-Identifier: GPL-2.0-only
+//
+// rt9123p.c -- RT9123 (HW Mode) ALSA SoC Codec driver
+//
+// Author: ChiYuan Huang <cy_huang@richtek.com>
+
+#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);
+	if (IS_ERR(rt9123p->enable))
+		return PTR_ERR(rt9123p->enable);
+
+	ret = device_property_read_u32(dev, "enable-delay", &rt9123p->enable_delay);
+	if (ret) {
+		rt9123p->enable_delay = 0;
+		dev_dbg(dev, "no optional property 'enable-delay' found, default: no delay\n");
+	}
+
+	platform_set_drvdata(pdev, rt9123p);
+
+	return devm_snd_soc_register_component(dev, &rt9123p_comp_driver, &rt9123p_dai_driver, 1);
+}
+
+#ifdef CONFIG_OF
+static const struct of_device_id rt9123p_device_id[] = {
+	{ .compatible = "richtek,rt9123p" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, rt9123p_device_id);
+#endif
+
+#ifdef CONFIG_ACPI
+static const struct acpi_device_id rt9123p_acpi_match[] = {
+	{ "RT9123P", 0 },
+	{}
+};
+MODULE_DEVICE_TABLE(acpi, rt9123p_acpi_match);
+#endif
+
+static struct platform_driver rt9123p_platform_driver = {
+	.driver = {
+		.name = "rt9123p",
+		.of_match_table = of_match_ptr(rt9123p_device_id),
+		.acpi_match_table = ACPI_PTR(rt9123p_acpi_match),
+	},
+	.probe	= rt9123p_platform_probe,
+};
+module_platform_driver(rt9123p_platform_driver);
+
+MODULE_AUTHOR("ChiYuan Huang <cy_huang@richtek.com>");
+MODULE_DESCRIPTION("ASoC rt9123p Driver");
+MODULE_LICENSE("GPL");
-- 
2.34.1

Re: [PATCH 2/4] ASoC: codecs: Add support for Richtek rt9123

[Mark Brown]

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

On Fri, Apr 04, 2025 at 10:22:12PM +0800, cy_huang@richtek.com wrote:

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

> +	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?

> +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;
> +	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?

> +static const struct snd_kcontrol_new rt9123_controls[] = {
> +	SOC_SINGLE_TLV("Master Volume", RT9123_REG_VOLGAIN, 2, 511, 1, dig_tlv),
> +	SOC_SINGLE_EXT_TLV("SPK Gain Volume", RT9123_REG_AMPCTRL, 0, 10, 0, rt9123_xhandler_get,
> +			   rt9123_xhandler_put, ana_tlv),

Speaker Volume.

> +static const struct regmap_config rt9123_regmap_config = {
> +	.name			= "rt9123",
> +	.reg_bits		= 8,
> +	.val_bits		= 16,
> +	.val_format_endian	= REGMAP_ENDIAN_BIG,
> +	.readable_reg		= rt9123_readable_reg,
> +	.writeable_reg		= rt9123_writeable_reg,
> +	.volatile_reg		= rt9123_volatile_reg,
> +	.cache_type		= REGCACHE_RBTREE,
> +	.num_reg_defaults_raw	= RT9123_REG_COMBOID + 1,
> +};

Generally _MAPLE is a better cache type for most devices - unless you
have a strong reason to use _RBTREE it's preferred.

> +	/* 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");
> +
> +	regmap = devm_regmap_init_i2c(i2c, &rt9123_regmap_config);
> +	if (IS_ERR(regmap))
> +		return dev_err_probe(dev, PTR_ERR(regmap), "Failed to init regmap\n");
> +
> +	ret = regmap_read(regmap, RT9123_REG_COMBOID, &venid);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to read vendor-id\n");
> +
> +	if ((venid & RT9123_MASK_VENID) != RT9123_FIXED_VENID)
> +		return dev_err_probe(dev, -ENODEV, "Incorrect vendor-id 0x%04x\n", venid);

It seems nicer to verify the device ID before doing the reset in case
anything went wrong.  Who knows what some other device did with the
reset?

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

Re: [PATCH 4/4] ASoC: codecs: Add support for Richtek rt9123p

[Rob Herring]

On Fri, Apr 04, 2025 at 10:22:14PM +0800, cy_huang@richtek.com wrote:
> From: ChiYuan Huang <cy_huang@richtek.com>
> 
> Add codec driver for Richtek rt9123p.
> 
> Signed-off-by: ChiYuan Huang <cy_huang@richtek.com>
> ---
>  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
> 
> diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
> index c61b2d3cf284..b0fa935846c0 100644
> --- a/sound/soc/codecs/Kconfig
> +++ b/sound/soc/codecs/Kconfig
> @@ -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
> diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
> index d8d0bc367af8..fba699701804 100644
> --- a/sound/soc/codecs/Makefile
> +++ b/sound/soc/codecs/Makefile
> @@ -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
> diff --git a/sound/soc/codecs/rt9123p.c b/sound/soc/codecs/rt9123p.c
> new file mode 100644
> index 000000000000..b0ff5f856e4c
> --- /dev/null
> +++ b/sound/soc/codecs/rt9123p.c
> @@ -0,0 +1,171 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +//
> +// rt9123p.c -- RT9123 (HW Mode) ALSA SoC Codec driver
> +//
> +// Author: ChiYuan Huang <cy_huang@richtek.com>
> +
> +#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);
> +	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

Re: [PATCH 3/4] ASoC: dt-bindings: Add bindings for Richtek rt9123p

[Rob Herring]

On Fri, Apr 04, 2025 at 10:22:13PM +0800, cy_huang@richtek.com wrote:
> From: ChiYuan Huang <cy_huang@richtek.com>
> 
> Document the ASoC Richtek rt9123p.
> 
> Signed-off-by: ChiYuan Huang <cy_huang@richtek.com>
> ---
>  .../bindings/sound/richtek,rt9123p.yaml       | 50 +++++++++++++++++++
>  1 file changed, 50 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/sound/richtek,rt9123p.yaml
> 
> diff --git a/Documentation/devicetree/bindings/sound/richtek,rt9123p.yaml b/Documentation/devicetree/bindings/sound/richtek,rt9123p.yaml
> new file mode 100644
> index 000000000000..836cd369a68c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/richtek,rt9123p.yaml
> @@ -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 <cy_huang@richtek.com>
> +
> +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
> +
> +  enable-gpios:
> +    maxItems: 1
> +
> +  enable-delay:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      Delay time for 'ENABLE' pin changes intended to make I2S clocks ready to
> +      prevent speaker pop noise. The unit is in millisecond.
> +    default: 0

Ah, it is documented. But my questions need to be addressed (in the 
patch).

> +
> +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>;
> +    };
> -- 
> 2.34.1
> 

Re: [PATCH 2/4] ASoC: codecs: Add support for Richtek rt9123

[kernel test robot]

Hi,

kernel test robot noticed the following build errors:

[auto build test ERROR on a2cc6ff5ec8f91bc463fd3b0c26b61166a07eb11]

url:    https://github.com/intel-lab-lkp/linux/commits/cy_huang-richtek-com/ASoC-dt-bindings-Add-bindings-for-Richtek-rt9123/20250404-223054
base:   a2cc6ff5ec8f91bc463fd3b0c26b61166a07eb11
patch link:    https://lore.kernel.org/r/cff65757c4665a81397ef5f559b277f96d4236c3.1743774849.git.cy_huang%40richtek.com
patch subject: [PATCH 2/4] ASoC: codecs: Add support for Richtek rt9123
config: loongarch-randconfig-001-20250405 (https://download.01.org/0day-ci/archive/20250405/202504052206.HFqFRXUk-lkp@intel.com/config)
compiler: loongarch64-linux-gcc (GCC) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250405/202504052206.HFqFRXUk-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202504052206.HFqFRXUk-lkp@intel.com/

All errors (new ones prefixed by >>):

   sound/soc/codecs/rt9123.c: In function 'rt9123_dai_hw_params':
>> sound/soc/codecs/rt9123.c:233:18: error: implicit declaration of function 'FIELD_GET' [-Wimplicit-function-declaration]
     233 |         fmtval = FIELD_GET(SND_SOC_DAIFMT_FORMAT_MASK, rt9123->dai_fmt);
         |                  ^~~~~~~~~
   In file included from include/linux/cpumask.h:11,
                    from arch/loongarch/include/asm/processor.h:9,
                    from arch/loongarch/include/asm/thread_info.h:15,
                    from include/linux/thread_info.h:60,
                    from include/asm-generic/preempt.h:5,
                    from ./arch/loongarch/include/generated/asm/preempt.h:1,
                    from include/linux/preempt.h:79,
                    from include/linux/spinlock.h:56,
                    from include/linux/mmzone.h:8,
                    from include/linux/gfp.h:7,
                    from include/linux/slab.h:16,
                    from include/linux/resource_ext.h:11,
                    from include/linux/acpi.h:13,
                    from sound/soc/codecs/rt9123.c:7:
   sound/soc/codecs/rt9123.c: At top level:
>> sound/soc/codecs/rt9123.c:476:31: error: 'rt9123_dev_pm_ops' undeclared here (not in a function); did you mean 'rt9123_dai_ops'?
     476 |                 .pm = pm_ptr(&rt9123_dev_pm_ops),
         |                               ^~~~~~~~~~~~~~~~~
   include/linux/kernel.h:48:44: note: in definition of macro 'PTR_IF'
      48 | #define PTR_IF(cond, ptr)       ((cond) ? (ptr) : NULL)
         |                                            ^~~
   sound/soc/codecs/rt9123.c:476:23: note: in expansion of macro 'pm_ptr'
     476 |                 .pm = pm_ptr(&rt9123_dev_pm_ops),
         |                       ^~~~~~


vim +/FIELD_GET +233 sound/soc/codecs/rt9123.c

   223	
   224	static int rt9123_dai_hw_params(struct snd_pcm_substream *substream,
   225					struct snd_pcm_hw_params *param, struct snd_soc_dai *dai)
   226	{
   227		struct rt9123_priv *rt9123 = snd_soc_dai_get_drvdata(dai);
   228		struct snd_soc_component *comp = dai->component;
   229		unsigned int fmtval, width, slot_width;
   230		struct device *dev = dai->dev;
   231		unsigned int audfmt, audbit;
   232	
 > 233		fmtval = FIELD_GET(SND_SOC_DAIFMT_FORMAT_MASK, rt9123->dai_fmt);
   234		if (rt9123->tdm_slots && fmtval != SND_SOC_DAIFMT_DSP_A && fmtval != SND_SOC_DAIFMT_DSP_B) {
   235			dev_err(dev, "TDM only can support DSP_A or DSP_B format\n");
   236			return -EINVAL;
   237		}
   238	
   239		switch (fmtval) {
   240		case SND_SOC_DAIFMT_I2S:
   241			audfmt = 0;
   242			break;
   243		case SND_SOC_DAIFMT_LEFT_J:
   244			audfmt = 1;
   245			break;
   246		case SND_SOC_DAIFMT_RIGHT_J:
   247			audfmt = 2;
   248			break;
   249		case SND_SOC_DAIFMT_DSP_B:
   250			audfmt = rt9123->tdm_slots ? 4 : 3;
   251			break;
   252		case SND_SOC_DAIFMT_DSP_A:
   253			audfmt = rt9123->tdm_slots ? 12 : 11;
   254			break;
   255		default:
   256			dev_err(dev, "Unsupported format %d\n", fmtval);
   257			return -EINVAL;
   258		}
   259	
   260		switch (width = params_width(param)) {
   261		case 16:
   262			audbit = 0;
   263			break;
   264		case 20:
   265			audbit = 1;
   266			break;
   267		case 24:
   268			audbit = 2;
   269			break;
   270		case 32:
   271			audbit = 3;
   272			break;
   273		case 8:
   274			audbit = 4;
   275			break;
   276		default:
   277			dev_err(dev, "Unsupported width %d\n", width);
   278			return -EINVAL;
   279		}
   280	
   281		slot_width = params_physical_width(param);
   282		if (rt9123->tdm_slots && slot_width > rt9123->tdm_slot_width) {
   283			dev_err(dev, "Slot width is larger than TDM slot width\n");
   284			return -EINVAL;
   285		}
   286	
   287		snd_soc_component_write_field(comp, RT9123_REG_I2SOPT, RT9123_MASK_AUDFMT, audfmt);
   288		snd_soc_component_write_field(comp, RT9123_REG_I2SOPT, RT9123_MASK_AUDBIT, audbit);
   289	
   290		return 0;
   291	}
   292	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 2/4] ASoC: codecs: Add support for Richtek rt9123

[kernel test robot]

Hi,

kernel test robot noticed the following build errors:

[auto build test ERROR on a2cc6ff5ec8f91bc463fd3b0c26b61166a07eb11]

url:    https://github.com/intel-lab-lkp/linux/commits/cy_huang-richtek-com/ASoC-dt-bindings-Add-bindings-for-Richtek-rt9123/20250404-223054
base:   a2cc6ff5ec8f91bc463fd3b0c26b61166a07eb11
patch link:    https://lore.kernel.org/r/cff65757c4665a81397ef5f559b277f96d4236c3.1743774849.git.cy_huang%40richtek.com
patch subject: [PATCH 2/4] ASoC: codecs: Add support for Richtek rt9123
config: powerpc64-randconfig-003-20250405 (https://download.01.org/0day-ci/archive/20250405/202504052244.bgS5yxev-lkp@intel.com/config)
compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250405/202504052244.bgS5yxev-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202504052244.bgS5yxev-lkp@intel.com/

All errors (new ones prefixed by >>):

>> sound/soc/codecs/rt9123.c:476:17: error: use of undeclared identifier 'rt9123_dev_pm_ops'; did you mean 'rt9123_dai_ops'?
     476 |                 .pm = pm_ptr(&rt9123_dev_pm_ops),
         |                               ^~~~~~~~~~~~~~~~~
         |                               rt9123_dai_ops
   include/linux/pm.h:471:53: note: expanded from macro 'pm_ptr'
     471 | #define pm_ptr(_ptr) PTR_IF(IS_ENABLED(CONFIG_PM), (_ptr))
         |                                                     ^
   include/linux/kernel.h:48:38: note: expanded from macro 'PTR_IF'
      48 | #define PTR_IF(cond, ptr)       ((cond) ? (ptr) : NULL)
         |                                            ^
   sound/soc/codecs/rt9123.c:293:37: note: 'rt9123_dai_ops' declared here
     293 | static const struct snd_soc_dai_ops rt9123_dai_ops = {
         |                                     ^
>> sound/soc/codecs/rt9123.c:476:9: error: incompatible pointer types initializing 'const struct dev_pm_ops *' with an expression of type 'const struct snd_soc_dai_ops *' [-Werror,-Wincompatible-pointer-types]
     476 |                 .pm = pm_ptr(&rt9123_dev_pm_ops),
         |                       ^~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/pm.h:471:22: note: expanded from macro 'pm_ptr'
     471 | #define pm_ptr(_ptr) PTR_IF(IS_ENABLED(CONFIG_PM), (_ptr))
         |                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/kernel.h:48:27: note: expanded from macro 'PTR_IF'
      48 | #define PTR_IF(cond, ptr)       ((cond) ? (ptr) : NULL)
         |                                 ^~~~~~~~~~~~~~~~~~~~~~~
   2 errors generated.


vim +476 sound/soc/codecs/rt9123.c

   470	
   471	static struct i2c_driver rt9123_i2c_driver = {
   472		.driver = {
   473			.name = "rt9123",
   474			.of_match_table = of_match_ptr(rt9123_device_id),
   475			.acpi_match_table = ACPI_PTR(rt9123_acpi_match),
 > 476			.pm = pm_ptr(&rt9123_dev_pm_ops),
   477		},
   478		.probe	= rt9123_i2c_probe,
   479	};
   480	module_i2c_driver(rt9123_i2c_driver);
   481	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 2/4] ASoC: codecs: Add support for Richtek rt9123

[ChiYuan Huang]

On Fri, Apr 04, 2025 at 04:03:57PM +0100, Mark Brown wrote:
> On Fri, Apr 04, 2025 at 10:22:12PM +0800, cy_huang@richtek.com wrote:
> 
> > +static int rt9123_enable_event(struct snd_soc_dapm_widget *w, struct snd_kcontrol *kcontrol,
> > +			       int event)
> > +{
> 
> > +	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?
> 
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.
> > +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;
> > +	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?
> 
Same as the last one.

........

Others will be modified in v2.

Thanks.

Re: [PATCH 4/4] ASoC: codecs: Add support for Richtek rt9123p

[ChiYuan Huang]

On Fri, Apr 04, 2025 at 03:05:19PM -0500, Rob Herring wrote:
> On Fri, Apr 04, 2025 at 10:22:14PM +0800, cy_huang@richtek.com wrote:
> > From: ChiYuan Huang <cy_huang@richtek.com>
> > 
> > Add codec driver for Richtek rt9123p.
> > 
> > Signed-off-by: ChiYuan Huang <cy_huang@richtek.com>
> > ---
> >  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
> > 
> > diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
> > index c61b2d3cf284..b0fa935846c0 100644
> > --- a/sound/soc/codecs/Kconfig
> > +++ b/sound/soc/codecs/Kconfig
> > @@ -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
> > diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
> > index d8d0bc367af8..fba699701804 100644
> > --- a/sound/soc/codecs/Makefile
> > +++ b/sound/soc/codecs/Makefile
> > @@ -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
> > diff --git a/sound/soc/codecs/rt9123p.c b/sound/soc/codecs/rt9123p.c
> > new file mode 100644
> > index 000000000000..b0ff5f856e4c
> > --- /dev/null
> > +++ b/sound/soc/codecs/rt9123p.c
> > @@ -0,0 +1,171 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +//
> > +// rt9123p.c -- RT9123 (HW Mode) ALSA SoC Codec driver
> > +//
> > +// Author: ChiYuan Huang <cy_huang@richtek.com>
> > +
> > +#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);
> > +	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.
> Rob

Re: [PATCH 2/4] ASoC: codecs: Add support for Richtek rt9123

[Mark Brown]

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

On Mon, Apr 07, 2025 at 08:44:05AM +0800, ChiYuan Huang wrote:
> On Fri, Apr 04, 2025 at 04:03:57PM +0100, Mark Brown wrote:

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

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

Re: [PATCH 2/4] ASoC: codecs: Add support for Richtek rt9123

[ChiYuan Huang]

On Mon, Apr 07, 2025 at 01:34:29PM +0100, Mark Brown wrote:
> On Mon, Apr 07, 2025 at 08:44:05AM +0800, ChiYuan Huang wrote:
> > On Fri, Apr 04, 2025 at 04:03:57PM +0100, Mark Brown wrote:
> 
> > > 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.

Okay, I'll left some comments to make it more clear for why special handling.
And yes, Since this register cannot be cached, to use pm_runtime can guarantee
the read modify write fine.

Is my understanding correct?

Re: [PATCH 4/4] ASoC: codecs: Add support for Richtek rt9123p

[ChiYuan Huang]

On Mon, Apr 07, 2025 at 08:53:04AM +0800, ChiYuan Huang wrote:
> On Fri, Apr 04, 2025 at 03:05:19PM -0500, Rob Herring wrote:
> > On Fri, Apr 04, 2025 at 10:22:14PM +0800, cy_huang@richtek.com wrote:
> > > From: ChiYuan Huang <cy_huang@richtek.com>
> > > 
> > > Add codec driver for Richtek rt9123p.
> > > 
> > > Signed-off-by: ChiYuan Huang <cy_huang@richtek.com>
> > > ---
> > >  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
> > > 
> > > diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
> > > index c61b2d3cf284..b0fa935846c0 100644
> > > --- a/sound/soc/codecs/Kconfig
> > > +++ b/sound/soc/codecs/Kconfig
> > > @@ -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
> > > diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
> > > index d8d0bc367af8..fba699701804 100644
> > > --- a/sound/soc/codecs/Makefile
> > > +++ b/sound/soc/codecs/Makefile
> > > @@ -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
> > > diff --git a/sound/soc/codecs/rt9123p.c b/sound/soc/codecs/rt9123p.c
> > > new file mode 100644
> > > index 000000000000..b0ff5f856e4c
> > > --- /dev/null
> > > +++ b/sound/soc/codecs/rt9123p.c
> > > @@ -0,0 +1,171 @@
> > > +// SPDX-License-Identifier: GPL-2.0-only
> > > +//
> > > +// rt9123p.c -- RT9123 (HW Mode) ALSA SoC Codec driver
> > > +//
> > > +// Author: ChiYuan Huang <cy_huang@richtek.com>
> > > +
> > > +#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);
> > > +	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.
> 
Hi, Rob:
  I'm sorting out all the changes needed for v2.

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'?
> Thanks.
> > Rob

Re: [PATCH 4/4] ASoC: codecs: Add support for Richtek rt9123p

[Mark Brown]

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

On Wed, Apr 09, 2025 at 09:06:52AM +0800, ChiYuan Huang wrote:

> 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'?

Yes, if there's sensible units then adding a suffix for them would be
good.

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

Re: (subset) [PATCH 0/4] ASoC: Add Richtek rt9123 and rt9123p support

[Mark Brown]

On Fri, 04 Apr 2025 22:22:10 +0800, cy_huang@richtek.com wrote:
> 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
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[3/4] ASoC: dt-bindings: Add bindings for Richtek rt9123p
      commit: 4a046b67d2d267daf884798ee8509a502abe7a58
[4/4] ASoC: codecs: Add support for Richtek rt9123p
      commit: 38c2585c7439cc678ae105dd826f10321db29552

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark