[PATCH v8 0/2] ASoC: rt5575: Add the codec driver for the ALC5575

[PATCH v8 0/2] ASoC: rt5575: Add the codec driver for the ALC5575

[Oder Chiou]

Hi all,

This patch series adds support for the Realtek ALC5575 audio codec.

Changes in v8:
- Patch 1/2:
  - remove the variable rt5575_spi_ready
  - use the multiple compatible names to distinguish between w/wo flash
- Patch 2/2
  - add compatible enum "realtek,rt5575-with-spi"

Changes in v7:
- Patch 1/2:
  - add a caption for the tristates
  - remove the redundant enum of the SPI command
  - add the error log in the request firmware failure
  - change the function name rt5575_spi_fw_loaded to rt5575_fw_load_by_spi
  - minor fixes
- Patch 2/2
  - modify commit message
- Link to v7: https://lore.kernel.org/all/20251121084112.743518-1-oder_chiou@realtek.com/

Changes in v6:
- Patch 1/2:
  - modify commit message
  - add select SND_SOC_RT5575 to config SND_SOC_RT5575_SPI in the Kconfig
  - revise the boiler plate in the head of the file
  - sort the include files
  - use a structure to transfer the spi data
  - use the poll() related function instead the for-loop
  - revise the UUID to the private ID
  - minor fixes
- Patch 2/2
  - modify description
- Link to v6: https://lore.kernel.org/all/20251031073245.3629060-1-oder_chiou@realtek.com/

Changes in v2 to v5:
- Patch 1/2:
  - move the firmware to the subdirectory
  - remove the empty functions
  - remove the cache_type in the regmap_config
  - add the error log in the run firmware failure
- Patch 2/2:
  - nothing
- Link to v5: https://lore.kernel.org/all/20251015103404.3075684-1-oder_chiou@realtek.com/

Oder Chiou (2):
  ASoC: rt5575: Add the codec driver for the ALC5575
  ASoC: dt-bindings: realtek,rt5575: add bindings for ALC5575

 .../bindings/sound/realtek,rt5575.yaml        |  44 ++
 sound/soc/codecs/Kconfig                      |  10 +
 sound/soc/codecs/Makefile                     |   4 +
 sound/soc/codecs/rt5575-spi.c                 |  84 ++++
 sound/soc/codecs/rt5575-spi.h                 |  16 +
 sound/soc/codecs/rt5575.c                     | 375 ++++++++++++++++++
 sound/soc/codecs/rt5575.h                     |  54 +++
 7 files changed, 587 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/realtek,rt5575.yaml
 create mode 100644 sound/soc/codecs/rt5575-spi.c
 create mode 100644 sound/soc/codecs/rt5575-spi.h
 create mode 100644 sound/soc/codecs/rt5575.c
 create mode 100644 sound/soc/codecs/rt5575.h

-- 
2.52.0

[PATCH v8 1/2] ASoC: rt5575: Add the codec driver for the ALC5575

[Oder Chiou]

The ALC5575 integrates an audio DSP that typically loads its firmware
from an external flash via its own SPI host interface. In certain
hardware configurations, the firmware can alternatively be loaded
through the SPI client interface. The driver provides basic mute and
volume control functions. When the SPI client interface is enabled,
firmware loading is handled by the SPI driver.

Signed-off-by: Oder Chiou <oder_chiou@realtek.com>
---
 sound/soc/codecs/Kconfig      |  10 +
 sound/soc/codecs/Makefile     |   4 +
 sound/soc/codecs/rt5575-spi.c |  84 ++++++++
 sound/soc/codecs/rt5575-spi.h |  16 ++
 sound/soc/codecs/rt5575.c     | 375 ++++++++++++++++++++++++++++++++++
 sound/soc/codecs/rt5575.h     |  54 +++++
 6 files changed, 543 insertions(+)
 create mode 100644 sound/soc/codecs/rt5575-spi.c
 create mode 100644 sound/soc/codecs/rt5575-spi.h
 create mode 100644 sound/soc/codecs/rt5575.c
 create mode 100644 sound/soc/codecs/rt5575.h

diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
index 6087ebde9523..484618cf3cdd 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -212,6 +212,7 @@ config SND_SOC_ALL_CODECS
 	imply SND_SOC_RT1305
 	imply SND_SOC_RT1308
 	imply SND_SOC_RT5514
+	imply SND_SOC_RT5575
 	imply SND_SOC_RT5616
 	imply SND_SOC_RT5631
 	imply SND_SOC_RT5640
@@ -1784,6 +1785,15 @@ config SND_SOC_RT5514_SPI_BUILTIN
 	bool # force RT5514_SPI to be built-in to avoid link errors
 	default SND_SOC_RT5514=y && SND_SOC_RT5514_SPI=m
 
+config SND_SOC_RT5575
+	tristate "Realtek ALC5575 Codec - I2C"
+	depends on I2C
+
+config SND_SOC_RT5575_SPI
+	tristate "Realtek ALC5575 Codec - SPI"
+	depends on SPI_MASTER
+	select SND_SOC_RT5575
+
 config SND_SOC_RT5616
 	tristate "Realtek RT5616 CODEC"
 	depends on I2C
diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
index d687d4f74363..1b9642318f77 100644
--- a/sound/soc/codecs/Makefile
+++ b/sound/soc/codecs/Makefile
@@ -253,6 +253,8 @@ snd-soc-rt286-y := rt286.o
 snd-soc-rt298-y := rt298.o
 snd-soc-rt5514-y := rt5514.o
 snd-soc-rt5514-spi-y := rt5514-spi.o
+snd-soc-rt5575-y := rt5575.o
+snd-soc-rt5575-spi-y := rt5575-spi.o
 snd-soc-rt5616-y := rt5616.o
 snd-soc-rt5631-y := rt5631.o
 snd-soc-rt5640-y := rt5640.o
@@ -686,6 +688,8 @@ obj-$(CONFIG_SND_SOC_RT298)	+= snd-soc-rt298.o
 obj-$(CONFIG_SND_SOC_RT5514)	+= snd-soc-rt5514.o
 obj-$(CONFIG_SND_SOC_RT5514_SPI)	+= snd-soc-rt5514-spi.o
 obj-$(CONFIG_SND_SOC_RT5514_SPI_BUILTIN)	+= snd-soc-rt5514-spi.o
+obj-$(CONFIG_SND_SOC_RT5575)	+= snd-soc-rt5575.o
+obj-$(CONFIG_SND_SOC_RT5575_SPI)	+= snd-soc-rt5575-spi.o
 obj-$(CONFIG_SND_SOC_RT5616)	+= snd-soc-rt5616.o
 obj-$(CONFIG_SND_SOC_RT5631)	+= snd-soc-rt5631.o
 obj-$(CONFIG_SND_SOC_RT5640)	+= snd-soc-rt5640.o
diff --git a/sound/soc/codecs/rt5575-spi.c b/sound/soc/codecs/rt5575-spi.c
new file mode 100644
index 000000000000..12c2379a061e
--- /dev/null
+++ b/sound/soc/codecs/rt5575-spi.c
@@ -0,0 +1,84 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * rt5575-spi.c  --  ALC5575 SPI driver
+ *
+ * Copyright(c) 2025 Realtek Semiconductor Corp.
+ *
+ */
+
+#include <linux/of.h>
+#include <linux/spi/spi.h>
+
+#include "rt5575-spi.h"
+
+#define RT5575_SPI_CMD_BURST_WRITE	5
+#define RT5575_SPI_BUF_LEN		240
+
+struct rt5575_spi_burst_write {
+	u8 cmd;
+	u32 addr;
+	u8 data[RT5575_SPI_BUF_LEN];
+	u8 dummy;
+} __packed;
+
+struct spi_device *rt5575_spi;
+EXPORT_SYMBOL_GPL(rt5575_spi);
+
+/**
+ * rt5575_spi_burst_write - Write data to SPI by rt5575 address.
+ * @addr: Start address.
+ * @txbuf: Data buffer for writing.
+ * @len: Data length.
+ *
+ */
+int rt5575_spi_burst_write(u32 addr, const u8 *txbuf, size_t len)
+{
+	struct rt5575_spi_burst_write buf = {
+		.cmd = RT5575_SPI_CMD_BURST_WRITE
+	};
+	unsigned int end, offset = 0;
+
+	while (offset < len) {
+		if (offset + RT5575_SPI_BUF_LEN <= len)
+			end = RT5575_SPI_BUF_LEN;
+		else
+			end = len % RT5575_SPI_BUF_LEN;
+
+		buf.addr = cpu_to_le32(addr + offset);
+
+		memcpy(&buf.data, &txbuf[offset], end);
+
+		spi_write(rt5575_spi, &buf, sizeof(buf));
+
+		offset += RT5575_SPI_BUF_LEN;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(rt5575_spi_burst_write);
+
+static int rt5575_spi_probe(struct spi_device *spi)
+{
+	rt5575_spi = spi;
+
+	return 0;
+}
+
+static const struct of_device_id rt5575_of_match[] = {
+	{ .compatible = "realtek,rt5575" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, rt5575_of_match);
+
+static struct spi_driver rt5575_spi_driver = {
+	.driver = {
+		.name = "rt5575",
+		.of_match_table = of_match_ptr(rt5575_of_match),
+	},
+	.probe = rt5575_spi_probe,
+};
+module_spi_driver(rt5575_spi_driver);
+
+MODULE_DESCRIPTION("ALC5575 SPI driver");
+MODULE_AUTHOR("Oder Chiou <oder_chiou@realtek.com>");
+MODULE_LICENSE("GPL");
diff --git a/sound/soc/codecs/rt5575-spi.h b/sound/soc/codecs/rt5575-spi.h
new file mode 100644
index 000000000000..b364b49bb43e
--- /dev/null
+++ b/sound/soc/codecs/rt5575-spi.h
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * rt5575-spi.h  --  ALC5575 SPI driver
+ *
+ * Copyright(c) 2025 Realtek Semiconductor Corp.
+ *
+ */
+
+#ifndef __RT5575_SPI_H__
+#define __RT5575_SPI_H__
+
+extern struct spi_device *rt5575_spi;
+
+int rt5575_spi_burst_write(u32 addr, const u8 *txbuf, size_t len);
+
+#endif /* __RT5575_SPI_H__ */
diff --git a/sound/soc/codecs/rt5575.c b/sound/soc/codecs/rt5575.c
new file mode 100644
index 000000000000..c7e8f5a606bc
--- /dev/null
+++ b/sound/soc/codecs/rt5575.c
@@ -0,0 +1,375 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * rt5575.c  --  ALC5575 ALSA SoC audio component driver
+ *
+ * Copyright(c) 2025 Realtek Semiconductor Corp.
+ *
+ */
+
+#include <linux/firmware.h>
+#include <linux/i2c.h>
+#include <sound/soc.h>
+#include <sound/tlv.h>
+
+#include "rt5575.h"
+#if IS_ENABLED(CONFIG_SND_SOC_RT5575_SPI)
+#include "rt5575-spi.h"
+#endif
+
+static bool rt5575_readable_register(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case RT5575_ID:
+	case RT5575_ID_1:
+	case RT5575_MIXL_VOL:
+	case RT5575_MIXR_VOL:
+	case RT5575_PROMPT_VOL:
+	case RT5575_SPK01_VOL:
+	case RT5575_SPK23_VOL:
+	case RT5575_MIC1_VOL:
+	case RT5575_MIC2_VOL:
+	case RT5575_WNC_CTRL:
+	case RT5575_MODE_CTRL:
+	case RT5575_I2S_RATE_CTRL:
+	case RT5575_SLEEP_CTRL:
+	case RT5575_ALG_BYPASS_CTRL:
+	case RT5575_PINMUX_CTRL_2:
+	case RT5575_GPIO_CTRL_1:
+	case RT5575_DSP_BUS_CTRL:
+	case RT5575_SW_INT:
+	case RT5575_DSP_BOOT_ERR:
+	case RT5575_DSP_READY:
+	case RT5575_DSP_CMD_ADDR:
+	case RT5575_EFUSE_DATA_2:
+	case RT5575_EFUSE_DATA_3:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static const DECLARE_TLV_DB_SCALE(ob_tlv, -9525, 75, 0);
+
+#if IS_ENABLED(CONFIG_SND_SOC_RT5575_SPI)
+static void rt5575_fw_load_by_spi(const struct firmware *fw, void *context)
+{
+	struct rt5575_priv *rt5575 = context;
+	struct i2c_client *i2c = rt5575->i2c;
+	const struct firmware *firmware;
+	static const char * const fw_path[] = {
+		"realtek/rt5575/rt5575_fw2.bin",
+		"realtek/rt5575/rt5575_fw3.bin",
+		"realtek/rt5575/rt5575_fw4.bin"
+	};
+	static const u32 fw_addr[] = { 0x5f600000, 0x5f7fe000, 0x5f7ff000 };
+	int i, ret;
+
+	regmap_write(rt5575->dsp_regmap, 0xfafafafa, 0x00000004);
+	regmap_write(rt5575->dsp_regmap, 0x18008064, 0x00000000);
+	regmap_write(rt5575->dsp_regmap, 0x18008068, 0x0002ffff);
+
+	rt5575_spi_burst_write(0x5f400000, fw->data, fw->size);
+	release_firmware(fw);
+
+	for (i = 0; i < ARRAY_SIZE(fw_addr); i++) {
+		ret = request_firmware(&firmware, fw_path[i], &i2c->dev);
+		if (!ret) {
+			rt5575_spi_burst_write(fw_addr[i], firmware->data, firmware->size);
+			release_firmware(firmware);
+		} else {
+			dev_err(&i2c->dev, "Request firmware failure: %d\n", ret);
+			return;
+		}
+	}
+
+	regmap_write(rt5575->dsp_regmap, 0x18000000, 0x00000000);
+
+	regmap_update_bits(rt5575->regmap, RT5575_SW_INT, 1, 1);
+
+	regmap_read_poll_timeout(rt5575->regmap, RT5575_SW_INT, ret, !ret, 100000, 10000000);
+
+	if (ret)
+		dev_err(&i2c->dev, "Run firmware failure: %d\n", ret);
+}
+#endif
+
+static const struct snd_kcontrol_new rt5575_snd_controls[] = {
+	SOC_DOUBLE("Speaker CH-01 Playback Switch", RT5575_SPK01_VOL, 31, 15, 1, 1),
+	SOC_DOUBLE_TLV("Speaker CH-01 Playback Volume", RT5575_SPK01_VOL, 17, 1, 167, 0, ob_tlv),
+	SOC_DOUBLE("Speaker CH-23 Playback Switch", RT5575_SPK23_VOL, 31, 15, 1, 1),
+	SOC_DOUBLE_TLV("Speaker CH-23 Playback Volume", RT5575_SPK23_VOL, 17, 1, 167, 0, ob_tlv),
+	SOC_DOUBLE("Mic1 Capture Switch", RT5575_MIC1_VOL, 31, 15, 1, 1),
+	SOC_DOUBLE_TLV("Mic1 Capture Volume", RT5575_MIC1_VOL, 17, 1, 167, 0, ob_tlv),
+	SOC_DOUBLE("Mic2 Capture Switch", RT5575_MIC2_VOL, 31, 15, 1, 1),
+	SOC_DOUBLE_TLV("Mic2 Capture Volume", RT5575_MIC2_VOL, 17, 1, 167, 0, ob_tlv),
+	SOC_DOUBLE_R("Mix Playback Switch", RT5575_MIXL_VOL, RT5575_MIXR_VOL, 31, 1, 1),
+	SOC_DOUBLE_R_TLV("Mix Playback Volume", RT5575_MIXL_VOL, RT5575_MIXR_VOL, 1, 127, 0,
+		ob_tlv),
+	SOC_DOUBLE("Prompt Playback Switch", RT5575_PROMPT_VOL, 31, 15, 1, 1),
+	SOC_DOUBLE_TLV("Prompt Playback Volume", RT5575_PROMPT_VOL, 17, 1, 167, 0, ob_tlv),
+};
+
+static const struct snd_soc_dapm_widget rt5575_dapm_widgets[] = {
+	SND_SOC_DAPM_AIF_IN("AIF1RX", "AIF1 Playback", 0, SND_SOC_NOPM, 0, 0),
+	SND_SOC_DAPM_AIF_OUT("AIF1TX", "AIF1 Capture", 0, SND_SOC_NOPM, 0, 0),
+	SND_SOC_DAPM_AIF_IN("AIF2RX", "AIF2 Playback", 0, SND_SOC_NOPM, 0, 0),
+	SND_SOC_DAPM_AIF_OUT("AIF2TX", "AIF2 Capture", 0, SND_SOC_NOPM, 0, 0),
+	SND_SOC_DAPM_AIF_IN("AIF3RX", "AIF3 Playback", 0, SND_SOC_NOPM, 0, 0),
+	SND_SOC_DAPM_AIF_OUT("AIF3TX", "AIF3 Capture", 0, SND_SOC_NOPM, 0, 0),
+	SND_SOC_DAPM_AIF_IN("AIF4RX", "AIF4 Playback", 0, SND_SOC_NOPM, 0, 0),
+	SND_SOC_DAPM_AIF_OUT("AIF4TX", "AIF4 Capture", 0, SND_SOC_NOPM, 0, 0),
+
+	SND_SOC_DAPM_INPUT("INPUT"),
+	SND_SOC_DAPM_OUTPUT("OUTPUT"),
+};
+
+static const struct snd_soc_dapm_route rt5575_dapm_routes[] = {
+	{ "AIF1TX", NULL, "INPUT" },
+	{ "AIF2TX", NULL, "INPUT" },
+	{ "AIF3TX", NULL, "INPUT" },
+	{ "AIF4TX", NULL, "INPUT" },
+	{ "OUTPUT", NULL, "AIF1RX" },
+	{ "OUTPUT", NULL, "AIF2RX" },
+	{ "OUTPUT", NULL, "AIF3RX" },
+	{ "OUTPUT", NULL, "AIF4RX" },
+};
+
+static long long rt5575_get_priv_id(struct rt5575_priv *rt5575)
+{
+	int priv_id_low, priv_id_high;
+
+	regmap_write(rt5575->regmap, RT5575_EFUSE_PID, 0xa0000000);
+	regmap_read(rt5575->regmap, RT5575_EFUSE_DATA_2, &priv_id_low);
+	regmap_read(rt5575->regmap, RT5575_EFUSE_DATA_3, &priv_id_high);
+	regmap_write(rt5575->regmap, RT5575_EFUSE_PID, 0);
+
+	return ((long long)priv_id_high << 32) | (long long)priv_id_low;
+}
+
+static const struct of_device_id rt5575_of_match[] = {
+	{ .compatible = "realtek,rt5575" },
+	{ .compatible = "realtek,rt5575-with-spi" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, rt5575_of_match);
+
+static int rt5575_probe(struct snd_soc_component *component)
+{
+	struct rt5575_priv *rt5575 = snd_soc_component_get_drvdata(component);
+	struct device *dev = component->dev;
+
+	rt5575->component = component;
+
+	dev_info(dev, "Private ID: %llx\n", rt5575_get_priv_id(rt5575));
+
+#if IS_ENABLED(CONFIG_SND_SOC_RT5575_SPI)
+	if (of_device_is_compatible(dev->of_node, rt5575_of_match[1].compatible))
+		request_firmware_nowait(THIS_MODULE, FW_ACTION_UEVENT,
+		"realtek/rt5575/rt5575_fw1.bin", component->dev, GFP_KERNEL, rt5575,
+		rt5575_fw_load_by_spi);
+#endif
+
+	return 0;
+}
+
+#define RT5575_STEREO_RATES SNDRV_PCM_RATE_8000_192000
+#define RT5575_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S20_3LE | \
+			SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S8 | \
+			SNDRV_PCM_FMTBIT_S32_LE)
+
+static struct snd_soc_dai_driver rt5575_dai[] = {
+	{
+		.name = "rt5575-aif1",
+		.id = RT5575_AIF1,
+		.playback = {
+			.stream_name = "AIF1 Playback",
+			.channels_min = 1,
+			.channels_max = 8,
+			.rates = RT5575_STEREO_RATES,
+			.formats = RT5575_FORMATS,
+		},
+		.capture = {
+			.stream_name = "AIF1 Capture",
+			.channels_min = 1,
+			.channels_max = 8,
+			.rates = RT5575_STEREO_RATES,
+			.formats = RT5575_FORMATS,
+		},
+	},
+	{
+		.name = "rt5575-aif2",
+		.id = RT5575_AIF2,
+		.playback = {
+			.stream_name = "AIF2 Playback",
+			.channels_min = 1,
+			.channels_max = 8,
+			.rates = RT5575_STEREO_RATES,
+			.formats = RT5575_FORMATS,
+		},
+		.capture = {
+			.stream_name = "AIF2 Capture",
+			.channels_min = 1,
+			.channels_max = 8,
+			.rates = RT5575_STEREO_RATES,
+			.formats = RT5575_FORMATS,
+		},
+	},
+	{
+		.name = "rt5575-aif3",
+		.id = RT5575_AIF3,
+		.playback = {
+			.stream_name = "AIF3 Playback",
+			.channels_min = 1,
+			.channels_max = 8,
+			.rates = RT5575_STEREO_RATES,
+			.formats = RT5575_FORMATS,
+		},
+		.capture = {
+			.stream_name = "AIF3 Capture",
+			.channels_min = 1,
+			.channels_max = 8,
+			.rates = RT5575_STEREO_RATES,
+			.formats = RT5575_FORMATS,
+		},
+	},
+	{
+		.name = "rt5575-aif4",
+		.id = RT5575_AIF4,
+		.playback = {
+			.stream_name = "AIF4 Playback",
+			.channels_min = 1,
+			.channels_max = 8,
+			.rates = RT5575_STEREO_RATES,
+			.formats = RT5575_FORMATS,
+		},
+		.capture = {
+			.stream_name = "AIF4 Capture",
+			.channels_min = 1,
+			.channels_max = 8,
+			.rates = RT5575_STEREO_RATES,
+			.formats = RT5575_FORMATS,
+		},
+	},
+};
+
+static const struct snd_soc_component_driver rt5575_soc_component_dev = {
+	.probe = rt5575_probe,
+	.controls = rt5575_snd_controls,
+	.num_controls = ARRAY_SIZE(rt5575_snd_controls),
+	.dapm_widgets = rt5575_dapm_widgets,
+	.num_dapm_widgets = ARRAY_SIZE(rt5575_dapm_widgets),
+	.dapm_routes = rt5575_dapm_routes,
+	.num_dapm_routes = ARRAY_SIZE(rt5575_dapm_routes),
+	.use_pmdown_time = 1,
+	.endianness = 1,
+};
+
+static const struct regmap_config rt5575_dsp_regmap = {
+	.name = "dsp",
+	.reg_bits = 32,
+	.val_bits = 32,
+	.reg_stride = 2,
+};
+
+static int rt5575_i2c_read(void *context, unsigned int reg, unsigned int *val)
+{
+	struct i2c_client *client = context;
+	struct rt5575_priv *rt5575 = i2c_get_clientdata(client);
+
+	regmap_read(rt5575->dsp_regmap, reg | RT5575_DSP_MAPPING, val);
+
+	return 0;
+}
+
+static int rt5575_i2c_write(void *context, unsigned int reg, unsigned int val)
+{
+	struct i2c_client *client = context;
+	struct rt5575_priv *rt5575 = i2c_get_clientdata(client);
+
+	regmap_write(rt5575->dsp_regmap, reg | RT5575_DSP_MAPPING, val);
+
+	return 0;
+}
+
+static const struct regmap_config rt5575_regmap = {
+	.reg_bits = 16,
+	.val_bits = 32,
+	.reg_stride = 4,
+	.max_register = 0xfffc,
+	.readable_reg = rt5575_readable_register,
+	.reg_read = rt5575_i2c_read,
+	.reg_write = rt5575_i2c_write,
+	.use_single_read = true,
+	.use_single_write = true,
+};
+
+static const struct i2c_device_id rt5575_i2c_id[] = {
+	{ "rt5575" },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, rt5575_i2c_id);
+
+static int rt5575_i2c_probe(struct i2c_client *i2c)
+{
+	struct rt5575_priv *rt5575;
+	struct device *dev = &i2c->dev;
+	int ret, val;
+
+#if IS_ENABLED(CONFIG_SND_SOC_RT5575_SPI)
+	if (!rt5575_spi && of_device_is_compatible(dev->of_node, rt5575_of_match[1].compatible))
+		return -EPROBE_DEFER;
+#endif
+
+	rt5575 = devm_kzalloc(dev, sizeof(struct rt5575_priv),
+				GFP_KERNEL);
+	if (rt5575 == NULL)
+		return -ENOMEM;
+
+	i2c_set_clientdata(i2c, rt5575);
+
+	rt5575->i2c = i2c;
+
+	rt5575->dsp_regmap = devm_regmap_init_i2c(i2c, &rt5575_dsp_regmap);
+	if (IS_ERR(rt5575->dsp_regmap)) {
+		ret = PTR_ERR(rt5575->dsp_regmap);
+		dev_err(dev, "Failed to allocate register map: %d\n", ret);
+		return ret;
+	}
+
+	rt5575->regmap = devm_regmap_init(dev, NULL, i2c, &rt5575_regmap);
+	if (IS_ERR(rt5575->regmap)) {
+		ret = PTR_ERR(rt5575->regmap);
+		dev_err(dev, "Failed to allocate register map: %d\n", ret);
+		return ret;
+	}
+
+	regmap_read(rt5575->regmap, RT5575_ID, &val);
+	if (val != RT5575_DEVICE_ID) {
+		dev_err(dev, "Device with ID register %08x is not rt5575\n", val);
+		return -ENODEV;
+	}
+
+	regmap_read(rt5575->regmap, RT5575_ID_1, &val);
+	if (!val) {
+		dev_err(dev, "This is not formal version\n");
+		return -ENODEV;
+	}
+
+	return devm_snd_soc_register_component(dev, &rt5575_soc_component_dev, rt5575_dai,
+		ARRAY_SIZE(rt5575_dai));
+}
+
+static struct i2c_driver rt5575_i2c_driver = {
+	.driver = {
+		.name = "rt5575",
+		.owner = THIS_MODULE,
+		.of_match_table = of_match_ptr(rt5575_of_match),
+	},
+	.probe = rt5575_i2c_probe,
+	.id_table = rt5575_i2c_id,
+};
+module_i2c_driver(rt5575_i2c_driver);
+
+MODULE_DESCRIPTION("ASoC ALC5575 driver");
+MODULE_AUTHOR("Oder Chiou <oder_chiou@realtek.com>");
+MODULE_LICENSE("GPL");
diff --git a/sound/soc/codecs/rt5575.h b/sound/soc/codecs/rt5575.h
new file mode 100644
index 000000000000..11149612274a
--- /dev/null
+++ b/sound/soc/codecs/rt5575.h
@@ -0,0 +1,54 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * rt5575.h  --  ALC5575 ALSA SoC audio driver
+ *
+ * Copyright(c) 2025 Realtek Semiconductor Corp.
+ *
+ */
+
+#ifndef __RT5575_H__
+#define __RT5575_H__
+
+#define RT5575_DEVICE_ID	0x10ec5575
+#define RT5575_DSP_MAPPING	0x18000000
+
+#define RT5575_ID		0x8008
+#define RT5575_ID_1		0x800c
+#define RT5575_MIXL_VOL		0x8a14
+#define RT5575_MIXR_VOL		0x8a18
+#define RT5575_PROMPT_VOL	0x8a84
+#define RT5575_SPK01_VOL	0x8a88
+#define RT5575_SPK23_VOL	0x8a8c
+#define RT5575_MIC1_VOL		0x8a98
+#define RT5575_MIC2_VOL		0x8a9c
+#define RT5575_WNC_CTRL		0x80ec
+#define RT5575_MODE_CTRL	0x80f0
+#define RT5575_I2S_RATE_CTRL	0x80f4
+#define RT5575_SLEEP_CTRL	0x80f8
+#define RT5575_ALG_BYPASS_CTRL	0x80fc
+#define RT5575_PINMUX_CTRL_2	0x81a4
+#define RT5575_GPIO_CTRL_1	0x8208
+#define RT5575_DSP_BUS_CTRL	0x880c
+#define RT5575_SW_INT		0x0018
+#define RT5575_DSP_BOOT_ERR	0x8e14
+#define RT5575_DSP_READY	0x8e24
+#define RT5575_DSP_CMD_ADDR	0x8e28
+#define RT5575_EFUSE_DATA_2	0xc638
+#define RT5575_EFUSE_DATA_3	0xc63c
+#define RT5575_EFUSE_PID	0xc660
+
+enum {
+	RT5575_AIF1,
+	RT5575_AIF2,
+	RT5575_AIF3,
+	RT5575_AIF4,
+	RT5575_AIFS,
+};
+
+struct rt5575_priv {
+	struct i2c_client *i2c;
+	struct snd_soc_component *component;
+	struct regmap *dsp_regmap, *regmap;
+};
+
+#endif /* __RT5575_H__ */
-- 
2.52.0

Re: [PATCH v8 1/2] ASoC: rt5575: Add the codec driver for the ALC5575

[Krzysztof Kozlowski]

On 01/12/2025 11:59, Oder Chiou wrote:
> +
> +static int rt5575_i2c_read(void *context, unsigned int reg, unsigned int *val)
> +{
> +	struct i2c_client *client = context;
> +	struct rt5575_priv *rt5575 = i2c_get_clientdata(client);
> +
> +	regmap_read(rt5575->dsp_regmap, reg | RT5575_DSP_MAPPING, val);
> +
> +	return 0;
> +}
> +
> +static int rt5575_i2c_write(void *context, unsigned int reg, unsigned int val)
> +{
> +	struct i2c_client *client = context;
> +	struct rt5575_priv *rt5575 = i2c_get_clientdata(client);
> +
> +	regmap_write(rt5575->dsp_regmap, reg | RT5575_DSP_MAPPING, val);
> +
> +	return 0;
> +}
> +
> +static const struct regmap_config rt5575_regmap = {
> +	.reg_bits = 16,
> +	.val_bits = 32,
> +	.reg_stride = 4,
> +	.max_register = 0xfffc,
> +	.readable_reg = rt5575_readable_register,
> +	.reg_read = rt5575_i2c_read,
> +	.reg_write = rt5575_i2c_write,
> +	.use_single_read = true,
> +	.use_single_write = true,
> +};

OF device ID table goes around here - together with I2C.
> +
> +static const struct i2c_device_id rt5575_i2c_id[] = {
> +	{ "rt5575" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, rt5575_i2c_id);
> +
> +static int rt5575_i2c_probe(struct i2c_client *i2c)
> +{
> +	struct rt5575_priv *rt5575;
> +	struct device *dev = &i2c->dev;
> +	int ret, val;
> +
> +#if IS_ENABLED(CONFIG_SND_SOC_RT5575_SPI)

No ifdefs in driver code.

> +	if (!rt5575_spi && of_device_is_compatible(dev->of_node, rt5575_of_match[1].compatible))

No, use driver match data if ever, but this is just wrong. You said it
depends on SPI flash, not SPI interface.

> +		return -EPROBE_DEFER;
> +#endif
> +
> +	rt5575 = devm_kzalloc(dev, sizeof(struct rt5575_priv),
> +				GFP_KERNEL);
> +	if (rt5575 == NULL)

This is not Linux coding style. Open existing drivers. It's everywhere
written (!foo)

> +		return -ENOMEM;
> +
> +	i2c_set_clientdata(i2c, rt5575);
> +
> +	rt5575->i2c = i2c;
> +
> +	rt5575->dsp_regmap = devm_regmap_init_i2c(i2c, &rt5575_dsp_regmap);
> +	if (IS_ERR(rt5575->dsp_regmap)) {
> +		ret = PTR_ERR(rt5575->dsp_regmap);
> +		dev_err(dev, "Failed to allocate register map: %d\n", ret);
> +		return ret;
> +	}
> +
> +	rt5575->regmap = devm_regmap_init(dev, NULL, i2c, &rt5575_regmap);
> +	if (IS_ERR(rt5575->regmap)) {
> +		ret = PTR_ERR(rt5575->regmap);
> +		dev_err(dev, "Failed to allocate register map: %d\n", ret);
> +		return ret;
> +	}
> +
> +	regmap_read(rt5575->regmap, RT5575_ID, &val);
> +	if (val != RT5575_DEVICE_ID) {
> +		dev_err(dev, "Device with ID register %08x is not rt5575\n", val);
> +		return -ENODEV;
> +	}
> +
> +	regmap_read(rt5575->regmap, RT5575_ID_1, &val);
> +	if (!val) {
> +		dev_err(dev, "This is not formal version\n");
> +		return -ENODEV;
> +	}
> +
> +	return devm_snd_soc_register_component(dev, &rt5575_soc_component_dev, rt5575_dai,
> +		ARRAY_SIZE(rt5575_dai));
> +}
> +
> +static struct i2c_driver rt5575_i2c_driver = {
> +	.driver = {
> +		.name = "rt5575",
> +		.owner = THIS_MODULE,

Please drop it.

Please run standard kernel tools for static analysis, like coccinelle,
smatch and sparse, and fix reported warnings. Also please check for
warnings when building with W=1 for gcc and clang. Most of these
commands (checks or W=1 build) can build specific targets, like some
directory, to narrow the scope to only your code. The code here looks
like it needs a fix. Feel free to get in touch if the warning is not clear.

> +		.of_match_table = of_match_ptr(rt5575_of_match),

You have warning here. Drop of_match_ptr.

> +	},
> +	.probe = rt5575_i2c_probe,
> +	.id_table = rt5575_i2c_id,
> +};
> +module_i2c_driver(rt5575_i2c_driver)
Best regards,
Krzysztof

RE: [PATCH v8 1/2] ASoC: rt5575: Add the codec driver for the ALC5575

[Oder Chiou]

> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: Monday, December 8, 2025 2:05 PM
> To: Oder Chiou <oder_chiou@realtek.com>; cezary.rojewski@intel.com;
> broonie@kernel.org; lgirdwood@gmail.com; robh@kernel.org;
> krzk+dt@kernel.org; conor+dt@kernel.org
> Cc: perex@perex.cz; linux-sound@vger.kernel.org; devicetree@vger.kernel.org;
> alsa-devel@alsa-project.org; Flove(HsinFu) <flove@realtek.com>; Shuming [范
> 書銘] <shumingf@realtek.com>; Jack Yu <jack.yu@realtek.com>; Derek [方德
> 義] <derek.fang@realtek.com>
> Subject: Re: [PATCH v8 1/2] ASoC: rt5575: Add the codec driver for the
> ALC5575
> 
> 
> External mail : This email originated from outside the organization. Do not
> reply, click links, or open attachments unless you recognize the sender and
> know the content is safe.
> 
> 
> 
> On 01/12/2025 11:59, Oder Chiou wrote:
> > +
> > +static int rt5575_i2c_read(void *context, unsigned int reg, unsigned int
> *val)
> > +{
> > +     struct i2c_client *client = context;
> > +     struct rt5575_priv *rt5575 = i2c_get_clientdata(client);
> > +
> > +     regmap_read(rt5575->dsp_regmap, reg | RT5575_DSP_MAPPING,
> val);
> > +
> > +     return 0;
> > +}
> > +
> > +static int rt5575_i2c_write(void *context, unsigned int reg, unsigned int val)
> > +{
> > +     struct i2c_client *client = context;
> > +     struct rt5575_priv *rt5575 = i2c_get_clientdata(client);
> > +
> > +     regmap_write(rt5575->dsp_regmap, reg | RT5575_DSP_MAPPING,
> val);
> > +
> > +     return 0;
> > +}
> > +
> > +static const struct regmap_config rt5575_regmap = {
> > +     .reg_bits = 16,
> > +     .val_bits = 32,
> > +     .reg_stride = 4,
> > +     .max_register = 0xfffc,
> > +     .readable_reg = rt5575_readable_register,
> > +     .reg_read = rt5575_i2c_read,
> > +     .reg_write = rt5575_i2c_write,
> > +     .use_single_read = true,
> > +     .use_single_write = true,
> > +};
> 
> OF device ID table goes around here - together with I2C.
I will correct it.

> > +
> > +static const struct i2c_device_id rt5575_i2c_id[] = {
> > +     { "rt5575" },
> > +     { }
> > +};
> > +MODULE_DEVICE_TABLE(i2c, rt5575_i2c_id);
> > +
> > +static int rt5575_i2c_probe(struct i2c_client *i2c)
> > +{
> > +     struct rt5575_priv *rt5575;
> > +     struct device *dev = &i2c->dev;
> > +     int ret, val;
> > +
> > +#if IS_ENABLED(CONFIG_SND_SOC_RT5575_SPI)
> 
> No ifdefs in driver code.

I am not understanding exactly.
If the machine is without SPI interface and the codec with flash, the
CONFIG_SND_SOC_RT5575_SPI can be disabled.

> > +     if (!rt5575_spi && of_device_is_compatible(dev->of_node,
> rt5575_of_match[1].compatible))
> 
> No, use driver match data if ever, but this is just wrong. You said it
> depends on SPI flash, not SPI interface.

I will modify it to use the match data as following.
static const struct of_device_id rt5575_of_match[] = {
	{ .compatible = "realtek,rt5575", .data = (void *)RT5575_WITH_FLASH },
	{ .compatible = "realtek,rt5575-use-spi", .data = (void *)RT5575_WITHOUT_FLASH },
	{ }
};

> > +             return -EPROBE_DEFER;
> > +#endif
> > +
> > +     rt5575 = devm_kzalloc(dev, sizeof(struct rt5575_priv),
> > +                             GFP_KERNEL);
> > +     if (rt5575 == NULL)
> 
> This is not Linux coding style. Open existing drivers. It's everywhere
> written (!foo)
I will correct it.

> > +             return -ENOMEM;
> > +
> > +     i2c_set_clientdata(i2c, rt5575);
> > +
> > +     rt5575->i2c = i2c;
> > +
> > +     rt5575->dsp_regmap = devm_regmap_init_i2c(i2c,
> &rt5575_dsp_regmap);
> > +     if (IS_ERR(rt5575->dsp_regmap)) {
> > +             ret = PTR_ERR(rt5575->dsp_regmap);
> > +             dev_err(dev, "Failed to allocate register map: %d\n", ret);
> > +             return ret;
> > +     }
> > +
> > +     rt5575->regmap = devm_regmap_init(dev, NULL, i2c,
> &rt5575_regmap);
> > +     if (IS_ERR(rt5575->regmap)) {
> > +             ret = PTR_ERR(rt5575->regmap);
> > +             dev_err(dev, "Failed to allocate register map: %d\n", ret);
> > +             return ret;
> > +     }
> > +
> > +     regmap_read(rt5575->regmap, RT5575_ID, &val);
> > +     if (val != RT5575_DEVICE_ID) {
> > +             dev_err(dev, "Device with ID register %08x is not rt5575\n",
> val);
> > +             return -ENODEV;
> > +     }
> > +
> > +     regmap_read(rt5575->regmap, RT5575_ID_1, &val);
> > +     if (!val) {
> > +             dev_err(dev, "This is not formal version\n");
> > +             return -ENODEV;
> > +     }
> > +
> > +     return devm_snd_soc_register_component(dev,
> &rt5575_soc_component_dev, rt5575_dai,
> > +             ARRAY_SIZE(rt5575_dai));
> > +}
> > +
> > +static struct i2c_driver rt5575_i2c_driver = {
> > +     .driver = {
> > +             .name = "rt5575",
> > +             .owner = THIS_MODULE,
> 
> Please drop it.
> 
> Please run standard kernel tools for static analysis, like coccinelle,
> smatch and sparse, and fix reported warnings. Also please check for
> warnings when building with W=1 for gcc and clang. Most of these
> commands (checks or W=1 build) can build specific targets, like some
> directory, to narrow the scope to only your code. The code here looks
> like it needs a fix. Feel free to get in touch if the warning is not clear.
> 
> > +             .of_match_table = of_match_ptr(rt5575_of_match),
> 
> You have warning here. Drop of_match_ptr.
I will remove it.

Thanks,
Oder

Re: [PATCH v8 1/2] ASoC: rt5575: Add the codec driver for the ALC5575

[Krzysztof Kozlowski]

On 08/12/2025 08:29, Oder Chiou wrote:
>> -----Original Message-----
>> From: Krzysztof Kozlowski <krzk@kernel.org>
>> Sent: Monday, December 8, 2025 2:05 PM
>> To: Oder Chiou <oder_chiou@realtek.com>; cezary.rojewski@intel.com;
>> broonie@kernel.org; lgirdwood@gmail.com; robh@kernel.org;
>> krzk+dt@kernel.org; conor+dt@kernel.org
>> Cc: perex@perex.cz; linux-sound@vger.kernel.org; devicetree@vger.kernel.org;
>> alsa-devel@alsa-project.org; Flove(HsinFu) <flove@realtek.com>; Shuming [范
>> 書銘] <shumingf@realtek.com>; Jack Yu <jack.yu@realtek.com>; Derek [方德
>> 義] <derek.fang@realtek.com>
>> Subject: Re: [PATCH v8 1/2] ASoC: rt5575: Add the codec driver for the
>> ALC5575
>>
>>
>> External mail : This email originated from outside the organization. Do not
>> reply, click links, or open attachments unless you recognize the sender and
>> know the content is safe.
>>
>>
>>
>> On 01/12/2025 11:59, Oder Chiou wrote:
>>> +
>>> +static int rt5575_i2c_read(void *context, unsigned int reg, unsigned int
>> *val)
>>> +{
>>> +     struct i2c_client *client = context;
>>> +     struct rt5575_priv *rt5575 = i2c_get_clientdata(client);
>>> +
>>> +     regmap_read(rt5575->dsp_regmap, reg | RT5575_DSP_MAPPING,
>> val);
>>> +
>>> +     return 0;
>>> +}
>>> +
>>> +static int rt5575_i2c_write(void *context, unsigned int reg, unsigned int val)
>>> +{
>>> +     struct i2c_client *client = context;
>>> +     struct rt5575_priv *rt5575 = i2c_get_clientdata(client);
>>> +
>>> +     regmap_write(rt5575->dsp_regmap, reg | RT5575_DSP_MAPPING,
>> val);
>>> +
>>> +     return 0;
>>> +}
>>> +
>>> +static const struct regmap_config rt5575_regmap = {
>>> +     .reg_bits = 16,
>>> +     .val_bits = 32,
>>> +     .reg_stride = 4,
>>> +     .max_register = 0xfffc,
>>> +     .readable_reg = rt5575_readable_register,
>>> +     .reg_read = rt5575_i2c_read,
>>> +     .reg_write = rt5575_i2c_write,
>>> +     .use_single_read = true,
>>> +     .use_single_write = true,
>>> +};
>>
>> OF device ID table goes around here - together with I2C.
> I will correct it.
> 
>>> +
>>> +static const struct i2c_device_id rt5575_i2c_id[] = {
>>> +     { "rt5575" },
>>> +     { }
>>> +};
>>> +MODULE_DEVICE_TABLE(i2c, rt5575_i2c_id);
>>> +
>>> +static int rt5575_i2c_probe(struct i2c_client *i2c)
>>> +{
>>> +     struct rt5575_priv *rt5575;
>>> +     struct device *dev = &i2c->dev;
>>> +     int ret, val;
>>> +
>>> +#if IS_ENABLED(CONFIG_SND_SOC_RT5575_SPI)
>>
>> No ifdefs in driver code.
> 
> I am not understanding exactly.
> If the machine is without SPI interface and the codec with flash, the
> CONFIG_SND_SOC_RT5575_SPI can be disabled.

But you still should not use #ifdef. Coding style gives you alternative,
please look at the doc.

> 
>>> +     if (!rt5575_spi && of_device_is_compatible(dev->of_node,
>> rt5575_of_match[1].compatible))
>>
>> No, use driver match data if ever, but this is just wrong. You said it
>> depends on SPI flash, not SPI interface.
> 
> I will modify it to use the match data as following.
> static const struct of_device_id rt5575_of_match[] = {
> 	{ .compatible = "realtek,rt5575", .data = (void *)RT5575_WITH_FLASH },
> 	{ .compatible = "realtek,rt5575-use-spi", .data = (void *)RT5575_WITHOUT_FLASH },

What is still wrong is that why you defer probe if there is no flash. I
really do not get it...



Best regards,
Krzysztof

RE: [PATCH v8 1/2] ASoC: rt5575: Add the codec driver for the ALC5575

[Oder Chiou]

> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: Monday, December 8, 2025 4:02 PM
> To: Oder Chiou <oder_chiou@realtek.com>; cezary.rojewski@intel.com;
> broonie@kernel.org; lgirdwood@gmail.com; robh@kernel.org;
> krzk+dt@kernel.org; conor+dt@kernel.org
> Cc: perex@perex.cz; linux-sound@vger.kernel.org; devicetree@vger.kernel.org;
> alsa-devel@alsa-project.org; Flove(HsinFu) <flove@realtek.com>; Shuming [范
> 書銘] <shumingf@realtek.com>; Jack Yu <jack.yu@realtek.com>; Derek [方德
> 義] <derek.fang@realtek.com>
> Subject: Re: [PATCH v8 1/2] ASoC: rt5575: Add the codec driver for the
> ALC5575
> 
> 
> External mail : This email originated from outside the organization. Do not
> reply, click links, or open attachments unless you recognize the sender and
> know the content is safe.
> 
> 
> 
> On 08/12/2025 08:29, Oder Chiou wrote:
> >> -----Original Message-----
> >> From: Krzysztof Kozlowski <krzk@kernel.org>
> >> Sent: Monday, December 8, 2025 2:05 PM
> >> To: Oder Chiou <oder_chiou@realtek.com>; cezary.rojewski@intel.com;
> >> broonie@kernel.org; lgirdwood@gmail.com; robh@kernel.org;
> >> krzk+dt@kernel.org; conor+dt@kernel.org
> >> Cc: perex@perex.cz; linux-sound@vger.kernel.org;
> >> devicetree@vger.kernel.org; alsa-devel@alsa-project.org;
> >> Flove(HsinFu) <flove@realtek.com>; Shuming [范
> >> 書銘] <shumingf@realtek.com>; Jack Yu <jack.yu@realtek.com>; Derek [方
> 德
> >> 義] <derek.fang@realtek.com>
> >> Subject: Re: [PATCH v8 1/2] ASoC: rt5575: Add the codec driver for
> >> the
> >> ALC5575
> >>
> >>
> >> External mail : This email originated from outside the organization.
> >> Do not reply, click links, or open attachments unless you recognize
> >> the sender and know the content is safe.
> >>
> >>
> >>
> >> On 01/12/2025 11:59, Oder Chiou wrote:
> >>> +
> >>> +static int rt5575_i2c_read(void *context, unsigned int reg,
> >>> +unsigned int
> >> *val)
> >>> +{
> >>> +     struct i2c_client *client = context;
> >>> +     struct rt5575_priv *rt5575 = i2c_get_clientdata(client);
> >>> +
> >>> +     regmap_read(rt5575->dsp_regmap, reg | RT5575_DSP_MAPPING,
> >> val);
> >>> +
> >>> +     return 0;
> >>> +}
> >>> +
> >>> +static int rt5575_i2c_write(void *context, unsigned int reg,
> >>> +unsigned int val) {
> >>> +     struct i2c_client *client = context;
> >>> +     struct rt5575_priv *rt5575 = i2c_get_clientdata(client);
> >>> +
> >>> +     regmap_write(rt5575->dsp_regmap, reg | RT5575_DSP_MAPPING,
> >> val);
> >>> +
> >>> +     return 0;
> >>> +}
> >>> +
> >>> +static const struct regmap_config rt5575_regmap = {
> >>> +     .reg_bits = 16,
> >>> +     .val_bits = 32,
> >>> +     .reg_stride = 4,
> >>> +     .max_register = 0xfffc,
> >>> +     .readable_reg = rt5575_readable_register,
> >>> +     .reg_read = rt5575_i2c_read,
> >>> +     .reg_write = rt5575_i2c_write,
> >>> +     .use_single_read = true,
> >>> +     .use_single_write = true,
> >>> +};
> >>
> >> OF device ID table goes around here - together with I2C.
> > I will correct it.
> >
> >>> +
> >>> +static const struct i2c_device_id rt5575_i2c_id[] = {
> >>> +     { "rt5575" },
> >>> +     { }
> >>> +};
> >>> +MODULE_DEVICE_TABLE(i2c, rt5575_i2c_id);
> >>> +
> >>> +static int rt5575_i2c_probe(struct i2c_client *i2c) {
> >>> +     struct rt5575_priv *rt5575;
> >>> +     struct device *dev = &i2c->dev;
> >>> +     int ret, val;
> >>> +
> >>> +#if IS_ENABLED(CONFIG_SND_SOC_RT5575_SPI)
> >>
> >> No ifdefs in driver code.
> >
> > I am not understanding exactly.
> > If the machine is without SPI interface and the codec with flash, the
> > CONFIG_SND_SOC_RT5575_SPI can be disabled.
> 
> But you still should not use #ifdef. Coding style gives you alternative, please
> look at the doc.
> 
> >
> >>> +     if (!rt5575_spi && of_device_is_compatible(dev->of_node,
> >> rt5575_of_match[1].compatible))
> >>
> >> No, use driver match data if ever, but this is just wrong. You said
> >> it depends on SPI flash, not SPI interface.
> >
> > I will modify it to use the match data as following.
> > static const struct of_device_id rt5575_of_match[] = {
> >       { .compatible = "realtek,rt5575", .data = (void
> *)RT5575_WITH_FLASH },
> >       { .compatible = "realtek,rt5575-use-spi", .data = (void
> > *)RT5575_WITHOUT_FLASH },
> 
> What is still wrong is that why you defer probe if there is no flash. I really do
> not get it...

If the codec has flash, the flash is connected to the codec's SPI host
interface.

If the codec has no flash, the codec SPI driver should load the firmware
from the codec's SPI slave interface. The I2C driver must wait until the
SPI driver is ready to ensure the firmware is loaded correctly.

Re: [PATCH v8 1/2] ASoC: rt5575: Add the codec driver for the ALC5575

[Krzysztof Kozlowski]

On 08/12/2025 10:16, Oder Chiou wrote:
>> -----Original Message-----
>> From: Krzysztof Kozlowski <krzk@kernel.org>
>> Sent: Monday, December 8, 2025 4:02 PM
>> To: Oder Chiou <oder_chiou@realtek.com>; cezary.rojewski@intel.com;
>> broonie@kernel.org; lgirdwood@gmail.com; robh@kernel.org;
>> krzk+dt@kernel.org; conor+dt@kernel.org
>> Cc: perex@perex.cz; linux-sound@vger.kernel.org; devicetree@vger.kernel.org;
>> alsa-devel@alsa-project.org; Flove(HsinFu) <flove@realtek.com>; Shuming [范
>> 書銘] <shumingf@realtek.com>; Jack Yu <jack.yu@realtek.com>; Derek [方德
>> 義] <derek.fang@realtek.com>
>> Subject: Re: [PATCH v8 1/2] ASoC: rt5575: Add the codec driver for the
>> ALC5575
>>
>>
>> External mail : This email originated from outside the organization. Do not
>> reply, click links, or open attachments unless you recognize the sender and
>> know the content is safe.
>>
>>
>>
>> On 08/12/2025 08:29, Oder Chiou wrote:
>>>> -----Original Message-----
>>>> From: Krzysztof Kozlowski <krzk@kernel.org>
>>>> Sent: Monday, December 8, 2025 2:05 PM
>>>> To: Oder Chiou <oder_chiou@realtek.com>; cezary.rojewski@intel.com;
>>>> broonie@kernel.org; lgirdwood@gmail.com; robh@kernel.org;
>>>> krzk+dt@kernel.org; conor+dt@kernel.org
>>>> Cc: perex@perex.cz; linux-sound@vger.kernel.org;
>>>> devicetree@vger.kernel.org; alsa-devel@alsa-project.org;
>>>> Flove(HsinFu) <flove@realtek.com>; Shuming [范
>>>> 書銘] <shumingf@realtek.com>; Jack Yu <jack.yu@realtek.com>; Derek [方
>> 德
>>>> 義] <derek.fang@realtek.com>
>>>> Subject: Re: [PATCH v8 1/2] ASoC: rt5575: Add the codec driver for
>>>> the
>>>> ALC5575
>>>>
>>>>
>>>> External mail : This email originated from outside the organization.
>>>> Do not reply, click links, or open attachments unless you recognize
>>>> the sender and know the content is safe.
>>>>
>>>>
>>>>
>>>> On 01/12/2025 11:59, Oder Chiou wrote:
>>>>> +
>>>>> +static int rt5575_i2c_read(void *context, unsigned int reg,
>>>>> +unsigned int
>>>> *val)
>>>>> +{
>>>>> +     struct i2c_client *client = context;
>>>>> +     struct rt5575_priv *rt5575 = i2c_get_clientdata(client);
>>>>> +
>>>>> +     regmap_read(rt5575->dsp_regmap, reg | RT5575_DSP_MAPPING,
>>>> val);
>>>>> +
>>>>> +     return 0;
>>>>> +}
>>>>> +
>>>>> +static int rt5575_i2c_write(void *context, unsigned int reg,
>>>>> +unsigned int val) {
>>>>> +     struct i2c_client *client = context;
>>>>> +     struct rt5575_priv *rt5575 = i2c_get_clientdata(client);
>>>>> +
>>>>> +     regmap_write(rt5575->dsp_regmap, reg | RT5575_DSP_MAPPING,
>>>> val);
>>>>> +
>>>>> +     return 0;
>>>>> +}
>>>>> +
>>>>> +static const struct regmap_config rt5575_regmap = {
>>>>> +     .reg_bits = 16,
>>>>> +     .val_bits = 32,
>>>>> +     .reg_stride = 4,
>>>>> +     .max_register = 0xfffc,
>>>>> +     .readable_reg = rt5575_readable_register,
>>>>> +     .reg_read = rt5575_i2c_read,
>>>>> +     .reg_write = rt5575_i2c_write,
>>>>> +     .use_single_read = true,
>>>>> +     .use_single_write = true,
>>>>> +};
>>>>
>>>> OF device ID table goes around here - together with I2C.
>>> I will correct it.
>>>
>>>>> +
>>>>> +static const struct i2c_device_id rt5575_i2c_id[] = {
>>>>> +     { "rt5575" },
>>>>> +     { }
>>>>> +};
>>>>> +MODULE_DEVICE_TABLE(i2c, rt5575_i2c_id);
>>>>> +
>>>>> +static int rt5575_i2c_probe(struct i2c_client *i2c) {
>>>>> +     struct rt5575_priv *rt5575;
>>>>> +     struct device *dev = &i2c->dev;
>>>>> +     int ret, val;
>>>>> +
>>>>> +#if IS_ENABLED(CONFIG_SND_SOC_RT5575_SPI)
>>>>
>>>> No ifdefs in driver code.
>>>
>>> I am not understanding exactly.
>>> If the machine is without SPI interface and the codec with flash, the
>>> CONFIG_SND_SOC_RT5575_SPI can be disabled.
>>
>> But you still should not use #ifdef. Coding style gives you alternative, please
>> look at the doc.
>>
>>>
>>>>> +     if (!rt5575_spi && of_device_is_compatible(dev->of_node,
>>>> rt5575_of_match[1].compatible))
>>>>
>>>> No, use driver match data if ever, but this is just wrong. You said
>>>> it depends on SPI flash, not SPI interface.
>>>
>>> I will modify it to use the match data as following.
>>> static const struct of_device_id rt5575_of_match[] = {
>>>       { .compatible = "realtek,rt5575", .data = (void
>> *)RT5575_WITH_FLASH },
>>>       { .compatible = "realtek,rt5575-use-spi", .data = (void
>>> *)RT5575_WITHOUT_FLASH },
>>
>> What is still wrong is that why you defer probe if there is no flash. I really do
>> not get it...
> 
> If the codec has flash, the flash is connected to the codec's SPI host
> interface.
> 
> If the codec has no flash, the codec SPI driver should load the firmware
> from the codec's SPI slave interface. The I2C driver must wait until the
> SPI driver is ready to ensure the firmware is loaded correctly.
> 

Ah, so for that reason you created that singleton, exported it and you
wait for it? Singletons are pretty no-go anyway, how do you handle to
codecs in the system? Fragile design.

Best regards,
Krzysztof

RE: [PATCH v8 1/2] ASoC: rt5575: Add the codec driver for the ALC5575

[Oder Chiou]

> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: Tuesday, December 9, 2025 2:08 PM
> To: Oder Chiou <oder_chiou@realtek.com>; cezary.rojewski@intel.com;
> broonie@kernel.org; lgirdwood@gmail.com; robh@kernel.org;
> krzk+dt@kernel.org; conor+dt@kernel.org
> Cc: perex@perex.cz; linux-sound@vger.kernel.org; devicetree@vger.kernel.org;
> alsa-devel@alsa-project.org; Flove(HsinFu) <flove@realtek.com>; Shuming [范
> 書銘] <shumingf@realtek.com>; Jack Yu <jack.yu@realtek.com>; Derek [方德
> 義] <derek.fang@realtek.com>
> Subject: Re: [PATCH v8 1/2] ASoC: rt5575: Add the codec driver for the
> ALC5575
> 
> 
> External mail : This email originated from outside the organization. Do not
> reply, click links, or open attachments unless you recognize the sender and
> know the content is safe.
> 
> 
> 
> On 08/12/2025 10:16, Oder Chiou wrote:
> >> -----Original Message-----
> >> From: Krzysztof Kozlowski <krzk@kernel.org>
> >> Sent: Monday, December 8, 2025 4:02 PM
> >> To: Oder Chiou <oder_chiou@realtek.com>; cezary.rojewski@intel.com;
> >> broonie@kernel.org; lgirdwood@gmail.com; robh@kernel.org;
> >> krzk+dt@kernel.org; conor+dt@kernel.org
> >> Cc: perex@perex.cz; linux-sound@vger.kernel.org;
> devicetree@vger.kernel.org;
> >> alsa-devel@alsa-project.org; Flove(HsinFu) <flove@realtek.com>; Shuming
> [范
> >> 書銘] <shumingf@realtek.com>; Jack Yu <jack.yu@realtek.com>; Derek [方
> 德
> >> 義] <derek.fang@realtek.com>
> >> Subject: Re: [PATCH v8 1/2] ASoC: rt5575: Add the codec driver for the
> >> ALC5575
> >>
> >>
> >> External mail : This email originated from outside the organization. Do not
> >> reply, click links, or open attachments unless you recognize the sender and
> >> know the content is safe.
> >>
> >>
> >>
> >> On 08/12/2025 08:29, Oder Chiou wrote:
> >>>> -----Original Message-----
> >>>> From: Krzysztof Kozlowski <krzk@kernel.org>
> >>>> Sent: Monday, December 8, 2025 2:05 PM
> >>>> To: Oder Chiou <oder_chiou@realtek.com>; cezary.rojewski@intel.com;
> >>>> broonie@kernel.org; lgirdwood@gmail.com; robh@kernel.org;
> >>>> krzk+dt@kernel.org; conor+dt@kernel.org
> >>>> Cc: perex@perex.cz; linux-sound@vger.kernel.org;
> >>>> devicetree@vger.kernel.org; alsa-devel@alsa-project.org;
> >>>> Flove(HsinFu) <flove@realtek.com>; Shuming [范
> >>>> 書銘] <shumingf@realtek.com>; Jack Yu <jack.yu@realtek.com>; Derek
> [方
> >> 德
> >>>> 義] <derek.fang@realtek.com>
> >>>> Subject: Re: [PATCH v8 1/2] ASoC: rt5575: Add the codec driver for
> >>>> the
> >>>> ALC5575
> >>>>
> >>>>
> >>>> External mail : This email originated from outside the organization.
> >>>> Do not reply, click links, or open attachments unless you recognize
> >>>> the sender and know the content is safe.
> >>>>
> >>>>
> >>>>
> >>>> On 01/12/2025 11:59, Oder Chiou wrote:
> >>>>> +
> >>>>> +static int rt5575_i2c_read(void *context, unsigned int reg,
> >>>>> +unsigned int
> >>>> *val)
> >>>>> +{
> >>>>> +     struct i2c_client *client = context;
> >>>>> +     struct rt5575_priv *rt5575 = i2c_get_clientdata(client);
> >>>>> +
> >>>>> +     regmap_read(rt5575->dsp_regmap, reg |
> RT5575_DSP_MAPPING,
> >>>> val);
> >>>>> +
> >>>>> +     return 0;
> >>>>> +}
> >>>>> +
> >>>>> +static int rt5575_i2c_write(void *context, unsigned int reg,
> >>>>> +unsigned int val) {
> >>>>> +     struct i2c_client *client = context;
> >>>>> +     struct rt5575_priv *rt5575 = i2c_get_clientdata(client);
> >>>>> +
> >>>>> +     regmap_write(rt5575->dsp_regmap, reg |
> RT5575_DSP_MAPPING,
> >>>> val);
> >>>>> +
> >>>>> +     return 0;
> >>>>> +}
> >>>>> +
> >>>>> +static const struct regmap_config rt5575_regmap = {
> >>>>> +     .reg_bits = 16,
> >>>>> +     .val_bits = 32,
> >>>>> +     .reg_stride = 4,
> >>>>> +     .max_register = 0xfffc,
> >>>>> +     .readable_reg = rt5575_readable_register,
> >>>>> +     .reg_read = rt5575_i2c_read,
> >>>>> +     .reg_write = rt5575_i2c_write,
> >>>>> +     .use_single_read = true,
> >>>>> +     .use_single_write = true,
> >>>>> +};
> >>>>
> >>>> OF device ID table goes around here - together with I2C.
> >>> I will correct it.
> >>>
> >>>>> +
> >>>>> +static const struct i2c_device_id rt5575_i2c_id[] = {
> >>>>> +     { "rt5575" },
> >>>>> +     { }
> >>>>> +};
> >>>>> +MODULE_DEVICE_TABLE(i2c, rt5575_i2c_id);
> >>>>> +
> >>>>> +static int rt5575_i2c_probe(struct i2c_client *i2c) {
> >>>>> +     struct rt5575_priv *rt5575;
> >>>>> +     struct device *dev = &i2c->dev;
> >>>>> +     int ret, val;
> >>>>> +
> >>>>> +#if IS_ENABLED(CONFIG_SND_SOC_RT5575_SPI)
> >>>>
> >>>> No ifdefs in driver code.
> >>>
> >>> I am not understanding exactly.
> >>> If the machine is without SPI interface and the codec with flash, the
> >>> CONFIG_SND_SOC_RT5575_SPI can be disabled.
> >>
> >> But you still should not use #ifdef. Coding style gives you alternative, please
> >> look at the doc.
> >>
> >>>
> >>>>> +     if (!rt5575_spi && of_device_is_compatible(dev->of_node,
> >>>> rt5575_of_match[1].compatible))
> >>>>
> >>>> No, use driver match data if ever, but this is just wrong. You said
> >>>> it depends on SPI flash, not SPI interface.
> >>>
> >>> I will modify it to use the match data as following.
> >>> static const struct of_device_id rt5575_of_match[] = {
> >>>       { .compatible = "realtek,rt5575", .data = (void
> >> *)RT5575_WITH_FLASH },
> >>>       { .compatible = "realtek,rt5575-use-spi", .data = (void
> >>> *)RT5575_WITHOUT_FLASH },
> >>
> >> What is still wrong is that why you defer probe if there is no flash. I really do
> >> not get it...
> >
> > If the codec has flash, the flash is connected to the codec's SPI host
> > interface.
> >
> > If the codec has no flash, the codec SPI driver should load the firmware
> > from the codec's SPI slave interface. The I2C driver must wait until the
> > SPI driver is ready to ensure the firmware is loaded correctly.
> >
> 
> Ah, so for that reason you created that singleton, exported it and you
> wait for it? Singletons are pretty no-go anyway, how do you handle to
> codecs in the system? Fragile design.

The code will be implemented in the top of the i2c_probe() as following.

if (dev_type == RT5575_WITHOUT_FLASH) {
	if (IS_ENABLED(CONFIG_SND_SOC_RT5575_SPI)) {
		if (!rt5575_spi) {
			dev_err(dev, "Wait SPI driver ready\n");
			return -EPROBE_DEFER;
		}
	} else {
		dev_err(dev, "This dev type should enable CONFIG_SND_SOC_RT5575_SPI\n");
		return -ENODEV;
	}
}

Thanks,
Oder

Re: [PATCH v8 1/2] ASoC: rt5575: Add the codec driver for the ALC5575

[Cezary Rojewski]

On 2025-12-10 6:26 AM, Oder Chiou wrote:
>> -----Original Message-----
>> From: Krzysztof Kozlowski <krzk@kernel.org>
>> Sent: Tuesday, December 9, 2025 2:08 PM
>> To: Oder Chiou <oder_chiou@realtek.com>; cezary.rojewski@intel.com;
>> broonie@kernel.org; lgirdwood@gmail.com; robh@kernel.org;
>> krzk+dt@kernel.org; conor+dt@kernel.org
>> Cc: perex@perex.cz; linux-sound@vger.kernel.org; devicetree@vger.kernel.org;
>> alsa-devel@alsa-project.org; Flove(HsinFu) <flove@realtek.com>; Shuming [范
>> 書銘] <shumingf@realtek.com>; Jack Yu <jack.yu@realtek.com>; Derek [方德
>> 義] <derek.fang@realtek.com>
>> Subject: Re: [PATCH v8 1/2] ASoC: rt5575: Add the codec driver for the
>> ALC5575

...

>> Ah, so for that reason you created that singleton, exported it and you
>> wait for it? Singletons are pretty no-go anyway, how do you handle to
>> codecs in the system? Fragile design.
> 
> The code will be implemented in the top of the i2c_probe() as following.
> 
> if (dev_type == RT5575_WITHOUT_FLASH) {
> 	if (IS_ENABLED(CONFIG_SND_SOC_RT5575_SPI)) {
> 		if (!rt5575_spi) {
> 			dev_err(dev, "Wait SPI driver ready\n");
> 			return -EPROBE_DEFER;
> 		}
> 	} else {
> 		dev_err(dev, "This dev type should enable CONFIG_SND_SOC_RT5575_SPI\n");
> 		return -ENODEV;
> 	}
> }

The idea is to get rid of 'rt5575_spi' global. Removing 
'rt5575_spi_ready' was just one of the steps to improve the flow. 
Removal of 'rt5575_spi' is the next one.

Re: [PATCH v8 1/2] ASoC: rt5575: Add the codec driver for the ALC5575

[Cezary Rojewski]

On 2025-12-01 11:59 AM, Oder Chiou wrote:
> The ALC5575 integrates an audio DSP that typically loads its firmware
> from an external flash via its own SPI host interface. In certain
> hardware configurations, the firmware can alternatively be loaded
> through the SPI client interface. The driver provides basic mute and
> volume control functions. When the SPI client interface is enabled,
> firmware loading is handled by the SPI driver.
> 
> Signed-off-by: Oder Chiou <oder_chiou@realtek.com>

...

> diff --git a/sound/soc/codecs/rt5575-spi.c b/sound/soc/codecs/rt5575-spi.c
> new file mode 100644
> index 000000000000..12c2379a061e
> --- /dev/null
> +++ b/sound/soc/codecs/rt5575-spi.c
> @@ -0,0 +1,84 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * rt5575-spi.c  --  ALC5575 SPI driver
> + *
> + * Copyright(c) 2025 Realtek Semiconductor Corp.
> + *
> + */
> +
> +#include <linux/of.h>
> +#include <linux/spi/spi.h>
> +
> +#include "rt5575-spi.h"
> +
> +#define RT5575_SPI_CMD_BURST_WRITE	5
> +#define RT5575_SPI_BUF_LEN		240
> +
> +struct rt5575_spi_burst_write {
> +	u8 cmd;
> +	u32 addr;
> +	u8 data[RT5575_SPI_BUF_LEN];
> +	u8 dummy;
> +} __packed;
> +
> +struct spi_device *rt5575_spi;
> +EXPORT_SYMBOL_GPL(rt5575_spi);
> +
> +/**
> + * rt5575_spi_burst_write - Write data to SPI by rt5575 address.
> + * @addr: Start address.
> + * @txbuf: Data buffer for writing.
> + * @len: Data length.
> + *
> + */
> +int rt5575_spi_burst_write(u32 addr, const u8 *txbuf, size_t len)
> +{
> +	struct rt5575_spi_burst_write buf = {
> +		.cmd = RT5575_SPI_CMD_BURST_WRITE
> +	};
> +	unsigned int end, offset = 0;
> +
> +	while (offset < len) {
> +		if (offset + RT5575_SPI_BUF_LEN <= len)
> +			end = RT5575_SPI_BUF_LEN;
> +		else
> +			end = len % RT5575_SPI_BUF_LEN;
> +
> +		buf.addr = cpu_to_le32(addr + offset);
> +
> +		memcpy(&buf.data, &txbuf[offset], end);
> +
> +		spi_write(rt5575_spi, &buf, sizeof(buf));
> +
> +		offset += RT5575_SPI_BUF_LEN;

Make it cohesive by proper spacing - it's a logical block, and logical 
blocks are easier to read if grouped together e.g.:

<assuming newline after the if-else-statement>

		buf.addr = cpu_to_le32(addr + offset);
		memcpy(&buf.data, &txbuf[offset], end);
		spi_write(rt5575_spi, &buf, sizeof(buf));

		offset += RT5575_SPI_BUF_LEN;

See?  Clear separation of operations leading to spi_write() and the 
offset increment.

> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(rt5575_spi_burst_write);
> +
> +static int rt5575_spi_probe(struct spi_device *spi)
> +{
> +	rt5575_spi = spi;
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id rt5575_of_match[] = {
> +	{ .compatible = "realtek,rt5575" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, rt5575_of_match);
> +
> +static struct spi_driver rt5575_spi_driver = {
> +	.driver = {
> +		.name = "rt5575",
> +		.of_match_table = of_match_ptr(rt5575_of_match),
> +	},
> +	.probe = rt5575_spi_probe,
> +};
> +module_spi_driver(rt5575_spi_driver);
> +
> +MODULE_DESCRIPTION("ALC5575 SPI driver");
> +MODULE_AUTHOR("Oder Chiou <oder_chiou@realtek.com>");
> +MODULE_LICENSE("GPL");
> diff --git a/sound/soc/codecs/rt5575-spi.h b/sound/soc/codecs/rt5575-spi.h
> new file mode 100644
> index 000000000000..b364b49bb43e
> --- /dev/null
> +++ b/sound/soc/codecs/rt5575-spi.h
> @@ -0,0 +1,16 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * rt5575-spi.h  --  ALC5575 SPI driver
> + *
> + * Copyright(c) 2025 Realtek Semiconductor Corp.
> + *
> + */
> +
> +#ifndef __RT5575_SPI_H__
> +#define __RT5575_SPI_H__
> +
> +extern struct spi_device *rt5575_spi;
> +
> +int rt5575_spi_burst_write(u32 addr, const u8 *txbuf, size_t len);
> +
> +#endif /* __RT5575_SPI_H__ */

The entire header can be dropped and its content moved to rt5575.h. 
It's included in both C-files plus we want to get rid of the global 
rt5575_spi anyway.  I do not think rt5575_spi_burst_write() belongs here 
either, see my comments regarding rt5575_fw_load_by_spi().

> diff --git a/sound/soc/codecs/rt5575.c b/sound/soc/codecs/rt5575.c
> new file mode 100644
> index 000000000000..c7e8f5a606bc
> --- /dev/null
> +++ b/sound/soc/codecs/rt5575.c
> @@ -0,0 +1,375 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * rt5575.c  --  ALC5575 ALSA SoC audio component driver
> + *
> + * Copyright(c) 2025 Realtek Semiconductor Corp.
> + *
> + */
> +
> +#include <linux/firmware.h>
> +#include <linux/i2c.h>
> +#include <sound/soc.h>
> +#include <sound/tlv.h>
> +
> +#include "rt5575.h"
> +#if IS_ENABLED(CONFIG_SND_SOC_RT5575_SPI)
> +#include "rt5575-spi.h"
> +#endif
> +
> +static bool rt5575_readable_register(struct device *dev, unsigned int reg)
> +{
> +	switch (reg) {
> +	case RT5575_ID:
> +	case RT5575_ID_1:
> +	case RT5575_MIXL_VOL:
> +	case RT5575_MIXR_VOL:
> +	case RT5575_PROMPT_VOL:
> +	case RT5575_SPK01_VOL:
> +	case RT5575_SPK23_VOL:
> +	case RT5575_MIC1_VOL:
> +	case RT5575_MIC2_VOL:
> +	case RT5575_WNC_CTRL:
> +	case RT5575_MODE_CTRL:
> +	case RT5575_I2S_RATE_CTRL:
> +	case RT5575_SLEEP_CTRL:
> +	case RT5575_ALG_BYPASS_CTRL:
> +	case RT5575_PINMUX_CTRL_2:
> +	case RT5575_GPIO_CTRL_1:
> +	case RT5575_DSP_BUS_CTRL:
> +	case RT5575_SW_INT:
> +	case RT5575_DSP_BOOT_ERR:
> +	case RT5575_DSP_READY:
> +	case RT5575_DSP_CMD_ADDR:
> +	case RT5575_EFUSE_DATA_2:
> +	case RT5575_EFUSE_DATA_3:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
> +static const DECLARE_TLV_DB_SCALE(ob_tlv, -9525, 75, 0);
> +
> +#if IS_ENABLED(CONFIG_SND_SOC_RT5575_SPI)
> +static void rt5575_fw_load_by_spi(const struct firmware *fw, void *context)

So, rt5575_spi_burst_write(), a wrapper for spi_write(), is exported to 
be available outside of the rt5575-spi module while its only user, 
rt5575_fw_load_by_spi(), which performs no I2C-specific tasks, is found 
with the common, rt5575.c file?

We can do better.  There are couple options here, one of them consists of:

1) privatize rt5575_spi_burst_write()
2) make rt5575_fw_load_by_spi() public
3) change rt5575_fw_load_by_spi() from void to int and return 
request_firmware_xxx() result
4) reword to rt5575_spi_fw_load() to match its friends

In regard to 1), have a #if-else preproc added to rt5575.h that governs 
the implementation of said function.  If xxx_RT5575_SPI is disabled, let 
is be a stub that returns 0.

In regard to 2), please do not ignore failures from firmware loading, 
that's a recurring point in this review and keeps being ignored.  No, 
async-firmware loading in not the answer why potential errors are left 
unhandled.

Another option, perhaps a better one is to have both 
rt5575_spi_burst_write() and rt5575_spi_fw_load() privatized and move 
the firmware-loading to the SPI-device probe.  See my comments targeting 
rt5575_probe().

> +{
> +	struct rt5575_priv *rt5575 = context;
> +	struct i2c_client *i2c = rt5575->i2c;

The whole reason this function needs i2c_client is ->dev retrieval. 
Let's simplify by listing 'struct device *dev' as an argument instead. 
With that, your function's argument list is also more explicit.

> +	const struct firmware *firmware;
> +	static const char * const fw_path[] = {
> +		"realtek/rt5575/rt5575_fw2.bin",
> +		"realtek/rt5575/rt5575_fw3.bin",
> +		"realtek/rt5575/rt5575_fw4.bin"
> +	};
> +	static const u32 fw_addr[] = { 0x5f600000, 0x5f7fe000, 0x5f7ff000 };
> +	int i, ret;
> +
> +	regmap_write(rt5575->dsp_regmap, 0xfafafafa, 0x00000004);
> +	regmap_write(rt5575->dsp_regmap, 0x18008064, 0x00000000);
> +	regmap_write(rt5575->dsp_regmap, 0x18008068, 0x0002ffff);
> +
> +	rt5575_spi_burst_write(0x5f400000, fw->data, fw->size);
> +	release_firmware(fw);
> +
> +	for (i = 0; i < ARRAY_SIZE(fw_addr); i++) {
> +		ret = request_firmware(&firmware, fw_path[i], &i2c->dev);
> +		if (!ret) {
> +			rt5575_spi_burst_write(fw_addr[i], firmware->data, firmware->size);
> +			release_firmware(firmware);
> +		} else {
> +			dev_err(&i2c->dev, "Request firmware failure: %d\n", ret);
> +			return;
> +		}
> +	}
> +
> +	regmap_write(rt5575->dsp_regmap, 0x18000000, 0x00000000);
> +
> +	regmap_update_bits(rt5575->regmap, RT5575_SW_INT, 1, 1);
> +
> +	regmap_read_poll_timeout(rt5575->regmap, RT5575_SW_INT, ret, !ret, 100000, 10000000);
> +
> +	if (ret)
> +		dev_err(&i2c->dev, "Run firmware failure: %d\n", ret);
> +}
> +#endif
> +
> +static const struct snd_kcontrol_new rt5575_snd_controls[] = {
> +	SOC_DOUBLE("Speaker CH-01 Playback Switch", RT5575_SPK01_VOL, 31, 15, 1, 1),
> +	SOC_DOUBLE_TLV("Speaker CH-01 Playback Volume", RT5575_SPK01_VOL, 17, 1, 167, 0, ob_tlv),
> +	SOC_DOUBLE("Speaker CH-23 Playback Switch", RT5575_SPK23_VOL, 31, 15, 1, 1),
> +	SOC_DOUBLE_TLV("Speaker CH-23 Playback Volume", RT5575_SPK23_VOL, 17, 1, 167, 0, ob_tlv),
> +	SOC_DOUBLE("Mic1 Capture Switch", RT5575_MIC1_VOL, 31, 15, 1, 1),
> +	SOC_DOUBLE_TLV("Mic1 Capture Volume", RT5575_MIC1_VOL, 17, 1, 167, 0, ob_tlv),
> +	SOC_DOUBLE("Mic2 Capture Switch", RT5575_MIC2_VOL, 31, 15, 1, 1),
> +	SOC_DOUBLE_TLV("Mic2 Capture Volume", RT5575_MIC2_VOL, 17, 1, 167, 0, ob_tlv),
> +	SOC_DOUBLE_R("Mix Playback Switch", RT5575_MIXL_VOL, RT5575_MIXR_VOL, 31, 1, 1),
> +	SOC_DOUBLE_R_TLV("Mix Playback Volume", RT5575_MIXL_VOL, RT5575_MIXR_VOL, 1, 127, 0,
> +		ob_tlv),
> +	SOC_DOUBLE("Prompt Playback Switch", RT5575_PROMPT_VOL, 31, 15, 1, 1),
> +	SOC_DOUBLE_TLV("Prompt Playback Volume", RT5575_PROMPT_VOL, 17, 1, 167, 0, ob_tlv),
> +};
> +
> +static const struct snd_soc_dapm_widget rt5575_dapm_widgets[] = {
> +	SND_SOC_DAPM_AIF_IN("AIF1RX", "AIF1 Playback", 0, SND_SOC_NOPM, 0, 0),
> +	SND_SOC_DAPM_AIF_OUT("AIF1TX", "AIF1 Capture", 0, SND_SOC_NOPM, 0, 0),
> +	SND_SOC_DAPM_AIF_IN("AIF2RX", "AIF2 Playback", 0, SND_SOC_NOPM, 0, 0),
> +	SND_SOC_DAPM_AIF_OUT("AIF2TX", "AIF2 Capture", 0, SND_SOC_NOPM, 0, 0),
> +	SND_SOC_DAPM_AIF_IN("AIF3RX", "AIF3 Playback", 0, SND_SOC_NOPM, 0, 0),
> +	SND_SOC_DAPM_AIF_OUT("AIF3TX", "AIF3 Capture", 0, SND_SOC_NOPM, 0, 0),
> +	SND_SOC_DAPM_AIF_IN("AIF4RX", "AIF4 Playback", 0, SND_SOC_NOPM, 0, 0),
> +	SND_SOC_DAPM_AIF_OUT("AIF4TX", "AIF4 Capture", 0, SND_SOC_NOPM, 0, 0),
> +
> +	SND_SOC_DAPM_INPUT("INPUT"),
> +	SND_SOC_DAPM_OUTPUT("OUTPUT"),
> +};
> +
> +static const struct snd_soc_dapm_route rt5575_dapm_routes[] = {
> +	{ "AIF1TX", NULL, "INPUT" },
> +	{ "AIF2TX", NULL, "INPUT" },
> +	{ "AIF3TX", NULL, "INPUT" },
> +	{ "AIF4TX", NULL, "INPUT" },
> +	{ "OUTPUT", NULL, "AIF1RX" },
> +	{ "OUTPUT", NULL, "AIF2RX" },
> +	{ "OUTPUT", NULL, "AIF3RX" },
> +	{ "OUTPUT", NULL, "AIF4RX" },
> +};
> +
> +static long long rt5575_get_priv_id(struct rt5575_priv *rt5575)
> +{
> +	int priv_id_low, priv_id_high;
> +
> +	regmap_write(rt5575->regmap, RT5575_EFUSE_PID, 0xa0000000);
> +	regmap_read(rt5575->regmap, RT5575_EFUSE_DATA_2, &priv_id_low);
> +	regmap_read(rt5575->regmap, RT5575_EFUSE_DATA_3, &priv_id_high);
> +	regmap_write(rt5575->regmap, RT5575_EFUSE_PID, 0);
> +
> +	return ((long long)priv_id_high << 32) | (long long)priv_id_low;
> +}
> +
> +static const struct of_device_id rt5575_of_match[] = {
> +	{ .compatible = "realtek,rt5575" },
> +	{ .compatible = "realtek,rt5575-with-spi" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, rt5575_of_match);
> +
> +static int rt5575_probe(struct snd_soc_component *component)
> +{
> +	struct rt5575_priv *rt5575 = snd_soc_component_get_drvdata(component);
> +	struct device *dev = component->dev;
> +
> +	rt5575->component = component;
> +
> +	dev_info(dev, "Private ID: %llx\n", rt5575_get_priv_id(rt5575));
> +
> +#if IS_ENABLED(CONFIG_SND_SOC_RT5575_SPI)
> +	if (of_device_is_compatible(dev->of_node, rt5575_of_match[1].compatible))
> +		request_firmware_nowait(THIS_MODULE, FW_ACTION_UEVENT,


An ASoC card is typically a mariage of platform-component (e.g.: SoC 
level DSP) and a codec-component (e.g.: rt5575).  If for any reason the 
platform-component becomes unloaded, codec-component will also be.  When 
the platform-component becomes available again, the 
codec-component->probe() would be invoked again - firmware would be 
loaded again.  Is this the desired behaviour?

To answer that, the follow up question is:  Is the firmware-loading for 
this particular solution, a chip -level operation or, a sound-card 
-level operation?  Typically firmware-loading is the former.  Once 
succeeeds, an ASoC component can be registered.  There is no reason to 
register ASoC component, which is mainly used for PCM/streaming reasons, 
if the preliminary setup - firmware-loading - is unsuccefull.

Perhaps a good approach is to move the firmware loading to the 
SPI-device's probe()!

> +		"realtek/rt5575/rt5575_fw1.bin", component->dev, GFP_KERNEL, rt5575,
> +		rt5575_fw_load_by_spi);
> +#endif
> +
> +	return 0;
> +}

...

> +
> +static const struct i2c_device_id rt5575_i2c_id[] = {
> +	{ "rt5575" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, rt5575_i2c_id);
> +
> +static int rt5575_i2c_probe(struct i2c_client *i2c)
> +{
> +	struct rt5575_priv *rt5575;
> +	struct device *dev = &i2c->dev;
> +	int ret, val;
> +
> +#if IS_ENABLED(CONFIG_SND_SOC_RT5575_SPI)
> +	if (!rt5575_spi && of_device_is_compatible(dev->of_node, rt5575_of_match[1].compatible))
> +		return -EPROBE_DEFER;
> +#endif
> +
> +	rt5575 = devm_kzalloc(dev, sizeof(struct rt5575_priv),
> +				GFP_KERNEL);
> +	if (rt5575 == NULL)
> +		return -ENOMEM;
> +
> +	i2c_set_clientdata(i2c, rt5575);
> +
> +	rt5575->i2c = i2c;
> +
> +	rt5575->dsp_regmap = devm_regmap_init_i2c(i2c, &rt5575_dsp_regmap);
> +	if (IS_ERR(rt5575->dsp_regmap)) {
> +		ret = PTR_ERR(rt5575->dsp_regmap);
> +		dev_err(dev, "Failed to allocate register map: %d\n", ret);

"Failed to allocate DSP register map:" so it differs from the one below?

> +		return ret;
> +	}
> +
> +	rt5575->regmap = devm_regmap_init(dev, NULL, i2c, &rt5575_regmap);
> +	if (IS_ERR(rt5575->regmap)) {
> +		ret = PTR_ERR(rt5575->regmap);
> +		dev_err(dev, "Failed to allocate register map: %d\n", ret);
> +		return ret;
> +	}
> +
> +	regmap_read(rt5575->regmap, RT5575_ID, &val);
> +	if (val != RT5575_DEVICE_ID) {
> +		dev_err(dev, "Device with ID register %08x is not rt5575\n", val);
> +		return -ENODEV;
> +	}
> +
> +	regmap_read(rt5575->regmap, RT5575_ID_1, &val);
> +	if (!val) {
> +		dev_err(dev, "This is not formal version\n");
> +		return -ENODEV;
> +	}
> +
> +	return devm_snd_soc_register_component(dev, &rt5575_soc_component_dev, rt5575_dai,
> +		ARRAY_SIZE(rt5575_dai));

Alignment.

> +}
> +
> +static struct i2c_driver rt5575_i2c_driver = {
> +	.driver = {
> +		.name = "rt5575",
> +		.owner = THIS_MODULE,
> +		.of_match_table = of_match_ptr(rt5575_of_match),
> +	},
> +	.probe = rt5575_i2c_probe,
> +	.id_table = rt5575_i2c_id,
> +};
> +module_i2c_driver(rt5575_i2c_driver);
> +
> +MODULE_DESCRIPTION("ASoC ALC5575 driver");
> +MODULE_AUTHOR("Oder Chiou <oder_chiou@realtek.com>");
> +MODULE_LICENSE("GPL");

RE: [PATCH v8 1/2] ASoC: rt5575: Add the codec driver for the ALC5575

[Oder Chiou]

> -----Original Message-----
> From: Cezary Rojewski <cezary.rojewski@intel.com>
> Sent: Tuesday, December 9, 2025 4:06 AM
> To: Oder Chiou <oder_chiou@realtek.com>
> Cc: perex@perex.cz; linux-sound@vger.kernel.org; devicetree@vger.kernel.org;
> alsa-devel@alsa-project.org; Flove(HsinFu) <flove@realtek.com>; Shuming [范
> 書銘] <shumingf@realtek.com>; Jack Yu <jack.yu@realtek.com>; Derek [方德
> 義] <derek.fang@realtek.com>; broonie@kernel.org; lgirdwood@gmail.com;
> robh@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org
> Subject: Re: [PATCH v8 1/2] ASoC: rt5575: Add the codec driver for the
> ALC5575
> 
> 
> External mail : This email originated from outside the organization. Do not
> reply, click links, or open attachments unless you recognize the sender and
> know the content is safe.
> 
> 
> 
> On 2025-12-01 11:59 AM, Oder Chiou wrote:
> > The ALC5575 integrates an audio DSP that typically loads its firmware
> > from an external flash via its own SPI host interface. In certain
> > hardware configurations, the firmware can alternatively be loaded
> > through the SPI client interface. The driver provides basic mute and
> > volume control functions. When the SPI client interface is enabled,
> > firmware loading is handled by the SPI driver.
> >
> > Signed-off-by: Oder Chiou <oder_chiou@realtek.com>
> 
> ...
> 
> > diff --git a/sound/soc/codecs/rt5575-spi.c b/sound/soc/codecs/rt5575-spi.c
> > new file mode 100644
> > index 000000000000..12c2379a061e
> > --- /dev/null
> > +++ b/sound/soc/codecs/rt5575-spi.c
> > @@ -0,0 +1,84 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * rt5575-spi.c  --  ALC5575 SPI driver
> > + *
> > + * Copyright(c) 2025 Realtek Semiconductor Corp.
> > + *
> > + */
> > +
> > +#include <linux/of.h>
> > +#include <linux/spi/spi.h>
> > +
> > +#include "rt5575-spi.h"
> > +
> > +#define RT5575_SPI_CMD_BURST_WRITE   5
> > +#define RT5575_SPI_BUF_LEN           240
> > +
> > +struct rt5575_spi_burst_write {
> > +     u8 cmd;
> > +     u32 addr;
> > +     u8 data[RT5575_SPI_BUF_LEN];
> > +     u8 dummy;
> > +} __packed;
> > +
> > +struct spi_device *rt5575_spi;
> > +EXPORT_SYMBOL_GPL(rt5575_spi);
> > +
> > +/**
> > + * rt5575_spi_burst_write - Write data to SPI by rt5575 address.
> > + * @addr: Start address.
> > + * @txbuf: Data buffer for writing.
> > + * @len: Data length.
> > + *
> > + */
> > +int rt5575_spi_burst_write(u32 addr, const u8 *txbuf, size_t len)
> > +{
> > +     struct rt5575_spi_burst_write buf = {
> > +             .cmd = RT5575_SPI_CMD_BURST_WRITE
> > +     };
> > +     unsigned int end, offset = 0;
> > +
> > +     while (offset < len) {
> > +             if (offset + RT5575_SPI_BUF_LEN <= len)
> > +                     end = RT5575_SPI_BUF_LEN;
> > +             else
> > +                     end = len % RT5575_SPI_BUF_LEN;
> > +
> > +             buf.addr = cpu_to_le32(addr + offset);
> > +
> > +             memcpy(&buf.data, &txbuf[offset], end);
> > +
> > +             spi_write(rt5575_spi, &buf, sizeof(buf));
> > +
> > +             offset += RT5575_SPI_BUF_LEN;
> 
> Make it cohesive by proper spacing - it's a logical block, and logical
> blocks are easier to read if grouped together e.g.:
> 
> <assuming newline after the if-else-statement>
> 
>                 buf.addr = cpu_to_le32(addr + offset);
>                 memcpy(&buf.data, &txbuf[offset], end);
>                 spi_write(rt5575_spi, &buf, sizeof(buf));
> 
>                 offset += RT5575_SPI_BUF_LEN;
> 
> See?  Clear separation of operations leading to spi_write() and the
> offset increment.
> 
> > +     }
> > +
> > +     return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(rt5575_spi_burst_write);
> > +
> > +static int rt5575_spi_probe(struct spi_device *spi)
> > +{
> > +     rt5575_spi = spi;
> > +
> > +     return 0;
> > +}
> > +
> > +static const struct of_device_id rt5575_of_match[] = {
> > +     { .compatible = "realtek,rt5575" },
> > +     { }
> > +};
> > +MODULE_DEVICE_TABLE(of, rt5575_of_match);
> > +
> > +static struct spi_driver rt5575_spi_driver = {
> > +     .driver = {
> > +             .name = "rt5575",
> > +             .of_match_table = of_match_ptr(rt5575_of_match),
> > +     },
> > +     .probe = rt5575_spi_probe,
> > +};
> > +module_spi_driver(rt5575_spi_driver);
> > +
> > +MODULE_DESCRIPTION("ALC5575 SPI driver");
> > +MODULE_AUTHOR("Oder Chiou <oder_chiou@realtek.com>");
> > +MODULE_LICENSE("GPL");
> > diff --git a/sound/soc/codecs/rt5575-spi.h b/sound/soc/codecs/rt5575-spi.h
> > new file mode 100644
> > index 000000000000..b364b49bb43e
> > --- /dev/null
> > +++ b/sound/soc/codecs/rt5575-spi.h
> > @@ -0,0 +1,16 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +/*
> > + * rt5575-spi.h  --  ALC5575 SPI driver
> > + *
> > + * Copyright(c) 2025 Realtek Semiconductor Corp.
> > + *
> > + */
> > +
> > +#ifndef __RT5575_SPI_H__
> > +#define __RT5575_SPI_H__
> > +
> > +extern struct spi_device *rt5575_spi;
> > +
> > +int rt5575_spi_burst_write(u32 addr, const u8 *txbuf, size_t len);
> > +
> > +#endif /* __RT5575_SPI_H__ */
> 
> The entire header can be dropped and its content moved to rt5575.h.
> It's included in both C-files plus we want to get rid of the global
> rt5575_spi anyway.  I do not think rt5575_spi_burst_write() belongs here
> either, see my comments regarding rt5575_fw_load_by_spi().
> 
> > diff --git a/sound/soc/codecs/rt5575.c b/sound/soc/codecs/rt5575.c
> > new file mode 100644
> > index 000000000000..c7e8f5a606bc
> > --- /dev/null
> > +++ b/sound/soc/codecs/rt5575.c
> > @@ -0,0 +1,375 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * rt5575.c  --  ALC5575 ALSA SoC audio component driver
> > + *
> > + * Copyright(c) 2025 Realtek Semiconductor Corp.
> > + *
> > + */
> > +
> > +#include <linux/firmware.h>
> > +#include <linux/i2c.h>
> > +#include <sound/soc.h>
> > +#include <sound/tlv.h>
> > +
> > +#include "rt5575.h"
> > +#if IS_ENABLED(CONFIG_SND_SOC_RT5575_SPI)
> > +#include "rt5575-spi.h"
> > +#endif
> > +
> > +static bool rt5575_readable_register(struct device *dev, unsigned int reg)
> > +{
> > +     switch (reg) {
> > +     case RT5575_ID:
> > +     case RT5575_ID_1:
> > +     case RT5575_MIXL_VOL:
> > +     case RT5575_MIXR_VOL:
> > +     case RT5575_PROMPT_VOL:
> > +     case RT5575_SPK01_VOL:
> > +     case RT5575_SPK23_VOL:
> > +     case RT5575_MIC1_VOL:
> > +     case RT5575_MIC2_VOL:
> > +     case RT5575_WNC_CTRL:
> > +     case RT5575_MODE_CTRL:
> > +     case RT5575_I2S_RATE_CTRL:
> > +     case RT5575_SLEEP_CTRL:
> > +     case RT5575_ALG_BYPASS_CTRL:
> > +     case RT5575_PINMUX_CTRL_2:
> > +     case RT5575_GPIO_CTRL_1:
> > +     case RT5575_DSP_BUS_CTRL:
> > +     case RT5575_SW_INT:
> > +     case RT5575_DSP_BOOT_ERR:
> > +     case RT5575_DSP_READY:
> > +     case RT5575_DSP_CMD_ADDR:
> > +     case RT5575_EFUSE_DATA_2:
> > +     case RT5575_EFUSE_DATA_3:
> > +             return true;
> > +     default:
> > +             return false;
> > +     }
> > +}
> > +
> > +static const DECLARE_TLV_DB_SCALE(ob_tlv, -9525, 75, 0);
> > +
> > +#if IS_ENABLED(CONFIG_SND_SOC_RT5575_SPI)
> > +static void rt5575_fw_load_by_spi(const struct firmware *fw, void *context)
> 
> So, rt5575_spi_burst_write(), a wrapper for spi_write(), is exported to
> be available outside of the rt5575-spi module while its only user,
> rt5575_fw_load_by_spi(), which performs no I2C-specific tasks, is found
> with the common, rt5575.c file?

There are few I2C r/w in rt5575_fw_load_by_spi().

But the SPI load firmware function can move to rt5575-spi.c.
The rt5575_spi_fw_load() will do all firmware load related works, and it
will return request_firmware_xxx() result.
The rt5575_fw_load_by_spi() will be implement as following, and it will put
into the bottom of rt5575_i2c_probe().

rt5575_fw_load_by_spi()
{
	...

	regmap_write(rt5575->dsp_regmap, 0xfafafafa, 0x00000004);
	regmap_write(rt5575->dsp_regmap, 0x18008064, 0x00000000);
	regmap_write(rt5575->dsp_regmap, 0x18008068, 0x0002ffff);

	ret = rt5575_spi_fw_load();
	if (ret) {
		dev_err(dev, "Load firmware failure: %d\n", ret);
		return ENODEV;
	}

	regmap_write(rt5575->dsp_regmap, 0x18000000, 0x00000000);
	regmap_update_bits(rt5575->regmap, RT5575_SW_INT, 1, 1);

	regmap_read_poll_timeout();
	if (ret) {
		dev_err(dev, "Run firmware failure: %d\n", ret);
		return ENODEV;
	}

	...
}

> We can do better.  There are couple options here, one of them consists of:
> 
> 1) privatize rt5575_spi_burst_write()
> 2) make rt5575_fw_load_by_spi() public
> 3) change rt5575_fw_load_by_spi() from void to int and return
> request_firmware_xxx() result
> 4) reword to rt5575_spi_fw_load() to match its friends
> 
> In regard to 1), have a #if-else preproc added to rt5575.h that governs
> the implementation of said function.  If xxx_RT5575_SPI is disabled, let
> is be a stub that returns 0.
> 
> In regard to 2), please do not ignore failures from firmware loading,
> that's a recurring point in this review and keeps being ignored.  No,
> async-firmware loading in not the answer why potential errors are left
> unhandled.
> 
> Another option, perhaps a better one is to have both
> rt5575_spi_burst_write() and rt5575_spi_fw_load() privatized and move
> the firmware-loading to the SPI-device probe.  See my comments targeting
> rt5575_probe().
> 
> > +{
> > +     struct rt5575_priv *rt5575 = context;
> > +     struct i2c_client *i2c = rt5575->i2c;
> 
> The whole reason this function needs i2c_client is ->dev retrieval.
> Let's simplify by listing 'struct device *dev' as an argument instead.
> With that, your function's argument list is also more explicit.
> 
> > +     const struct firmware *firmware;
> > +     static const char * const fw_path[] = {
> > +             "realtek/rt5575/rt5575_fw2.bin",
> > +             "realtek/rt5575/rt5575_fw3.bin",
> > +             "realtek/rt5575/rt5575_fw4.bin"
> > +     };
> > +     static const u32 fw_addr[] = { 0x5f600000, 0x5f7fe000, 0x5f7ff000 };
> > +     int i, ret;
> > +
> > +     regmap_write(rt5575->dsp_regmap, 0xfafafafa, 0x00000004);
> > +     regmap_write(rt5575->dsp_regmap, 0x18008064, 0x00000000);
> > +     regmap_write(rt5575->dsp_regmap, 0x18008068, 0x0002ffff);
> > +
> > +     rt5575_spi_burst_write(0x5f400000, fw->data, fw->size);
> > +     release_firmware(fw);
> > +
> > +     for (i = 0; i < ARRAY_SIZE(fw_addr); i++) {
> > +             ret = request_firmware(&firmware, fw_path[i], &i2c->dev);
> > +             if (!ret) {
> > +                     rt5575_spi_burst_write(fw_addr[i],
> firmware->data, firmware->size);
> > +                     release_firmware(firmware);
> > +             } else {
> > +                     dev_err(&i2c->dev, "Request firmware failure:
> %d\n", ret);
> > +                     return;
> > +             }
> > +     }
> > +
> > +     regmap_write(rt5575->dsp_regmap, 0x18000000, 0x00000000);
> > +
> > +     regmap_update_bits(rt5575->regmap, RT5575_SW_INT, 1, 1);
> > +
> > +     regmap_read_poll_timeout(rt5575->regmap, RT5575_SW_INT,
> ret, !ret, 100000, 10000000);
> > +
> > +     if (ret)
> > +             dev_err(&i2c->dev, "Run firmware failure: %d\n", ret);
> > +}
> > +#endif
> > +
> > +static const struct snd_kcontrol_new rt5575_snd_controls[] = {
> > +     SOC_DOUBLE("Speaker CH-01 Playback Switch", RT5575_SPK01_VOL,
> 31, 15, 1, 1),
> > +     SOC_DOUBLE_TLV("Speaker CH-01 Playback Volume",
> RT5575_SPK01_VOL, 17, 1, 167, 0, ob_tlv),
> > +     SOC_DOUBLE("Speaker CH-23 Playback Switch", RT5575_SPK23_VOL,
> 31, 15, 1, 1),
> > +     SOC_DOUBLE_TLV("Speaker CH-23 Playback Volume",
> RT5575_SPK23_VOL, 17, 1, 167, 0, ob_tlv),
> > +     SOC_DOUBLE("Mic1 Capture Switch", RT5575_MIC1_VOL, 31, 15, 1,
> 1),
> > +     SOC_DOUBLE_TLV("Mic1 Capture Volume", RT5575_MIC1_VOL, 17,
> 1, 167, 0, ob_tlv),
> > +     SOC_DOUBLE("Mic2 Capture Switch", RT5575_MIC2_VOL, 31, 15, 1,
> 1),
> > +     SOC_DOUBLE_TLV("Mic2 Capture Volume", RT5575_MIC2_VOL, 17,
> 1, 167, 0, ob_tlv),
> > +     SOC_DOUBLE_R("Mix Playback Switch", RT5575_MIXL_VOL,
> RT5575_MIXR_VOL, 31, 1, 1),
> > +     SOC_DOUBLE_R_TLV("Mix Playback Volume", RT5575_MIXL_VOL,
> RT5575_MIXR_VOL, 1, 127, 0,
> > +             ob_tlv),
> > +     SOC_DOUBLE("Prompt Playback Switch", RT5575_PROMPT_VOL, 31,
> 15, 1, 1),
> > +     SOC_DOUBLE_TLV("Prompt Playback Volume",
> RT5575_PROMPT_VOL, 17, 1, 167, 0, ob_tlv),
> > +};
> > +
> > +static const struct snd_soc_dapm_widget rt5575_dapm_widgets[] = {
> > +     SND_SOC_DAPM_AIF_IN("AIF1RX", "AIF1 Playback", 0,
> SND_SOC_NOPM, 0, 0),
> > +     SND_SOC_DAPM_AIF_OUT("AIF1TX", "AIF1 Capture", 0,
> SND_SOC_NOPM, 0, 0),
> > +     SND_SOC_DAPM_AIF_IN("AIF2RX", "AIF2 Playback", 0,
> SND_SOC_NOPM, 0, 0),
> > +     SND_SOC_DAPM_AIF_OUT("AIF2TX", "AIF2 Capture", 0,
> SND_SOC_NOPM, 0, 0),
> > +     SND_SOC_DAPM_AIF_IN("AIF3RX", "AIF3 Playback", 0,
> SND_SOC_NOPM, 0, 0),
> > +     SND_SOC_DAPM_AIF_OUT("AIF3TX", "AIF3 Capture", 0,
> SND_SOC_NOPM, 0, 0),
> > +     SND_SOC_DAPM_AIF_IN("AIF4RX", "AIF4 Playback", 0,
> SND_SOC_NOPM, 0, 0),
> > +     SND_SOC_DAPM_AIF_OUT("AIF4TX", "AIF4 Capture", 0,
> SND_SOC_NOPM, 0, 0),
> > +
> > +     SND_SOC_DAPM_INPUT("INPUT"),
> > +     SND_SOC_DAPM_OUTPUT("OUTPUT"),
> > +};
> > +
> > +static const struct snd_soc_dapm_route rt5575_dapm_routes[] = {
> > +     { "AIF1TX", NULL, "INPUT" },
> > +     { "AIF2TX", NULL, "INPUT" },
> > +     { "AIF3TX", NULL, "INPUT" },
> > +     { "AIF4TX", NULL, "INPUT" },
> > +     { "OUTPUT", NULL, "AIF1RX" },
> > +     { "OUTPUT", NULL, "AIF2RX" },
> > +     { "OUTPUT", NULL, "AIF3RX" },
> > +     { "OUTPUT", NULL, "AIF4RX" },
> > +};
> > +
> > +static long long rt5575_get_priv_id(struct rt5575_priv *rt5575)
> > +{
> > +     int priv_id_low, priv_id_high;
> > +
> > +     regmap_write(rt5575->regmap, RT5575_EFUSE_PID, 0xa0000000);
> > +     regmap_read(rt5575->regmap, RT5575_EFUSE_DATA_2,
> &priv_id_low);
> > +     regmap_read(rt5575->regmap, RT5575_EFUSE_DATA_3,
> &priv_id_high);
> > +     regmap_write(rt5575->regmap, RT5575_EFUSE_PID, 0);
> > +
> > +     return ((long long)priv_id_high << 32) | (long long)priv_id_low;
> > +}
> > +
> > +static const struct of_device_id rt5575_of_match[] = {
> > +     { .compatible = "realtek,rt5575" },
> > +     { .compatible = "realtek,rt5575-with-spi" },
> > +     { }
> > +};
> > +MODULE_DEVICE_TABLE(of, rt5575_of_match);
> > +
> > +static int rt5575_probe(struct snd_soc_component *component)
> > +{
> > +     struct rt5575_priv *rt5575 =
> snd_soc_component_get_drvdata(component);
> > +     struct device *dev = component->dev;
> > +
> > +     rt5575->component = component;
> > +
> > +     dev_info(dev, "Private ID: %llx\n", rt5575_get_priv_id(rt5575));
> > +
> > +#if IS_ENABLED(CONFIG_SND_SOC_RT5575_SPI)
> > +     if (of_device_is_compatible(dev->of_node,
> rt5575_of_match[1].compatible))
> > +             request_firmware_nowait(THIS_MODULE,
> FW_ACTION_UEVENT,
> 
> 
> An ASoC card is typically a mariage of platform-component (e.g.: SoC
> level DSP) and a codec-component (e.g.: rt5575).  If for any reason the
> platform-component becomes unloaded, codec-component will also be.
> When
> the platform-component becomes available again, the
> codec-component->probe() would be invoked again - firmware would be
> loaded again.  Is this the desired behaviour?
> 
> To answer that, the follow up question is:  Is the firmware-loading for
> this particular solution, a chip -level operation or, a sound-card
> -level operation?  Typically firmware-loading is the former.  Once
> succeeeds, an ASoC component can be registered.  There is no reason to
> register ASoC component, which is mainly used for PCM/streaming reasons,
> if the preliminary setup - firmware-loading - is unsuccefull.
> 
> Perhaps a good approach is to move the firmware loading to the
> SPI-device's probe()!

But before the firmware-loading, I2C commands are needed. The SPI
firmware-loading cannot be independent by I2C.

Thanks,
Oder

Re: [PATCH v8 1/2] ASoC: rt5575: Add the codec driver for the ALC5575

[Cezary Rojewski]

On 2025-12-09 6:35 AM, Oder Chiou wrote:
>> -----Original Message-----
>> From: Cezary Rojewski <cezary.rojewski@intel.com>
>> Sent: Tuesday, December 9, 2025 4:06 AM
>> To: Oder Chiou <oder_chiou@realtek.com>
>> Cc: perex@perex.cz; linux-sound@vger.kernel.org; devicetree@vger.kernel.org;
>> alsa-devel@alsa-project.org; Flove(HsinFu) <flove@realtek.com>; Shuming [范
>> 書銘] <shumingf@realtek.com>; Jack Yu <jack.yu@realtek.com>; Derek [方德
>> 義] <derek.fang@realtek.com>; broonie@kernel.org; lgirdwood@gmail.com;
>> robh@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org
>> Subject: Re: [PATCH v8 1/2] ASoC: rt5575: Add the codec driver for the
>> ALC5575

...

>>> +#if IS_ENABLED(CONFIG_SND_SOC_RT5575_SPI)
>>> +static void rt5575_fw_load_by_spi(const struct firmware *fw, void *context)
>>
>> So, rt5575_spi_burst_write(), a wrapper for spi_write(), is exported to
>> be available outside of the rt5575-spi module while its only user,
>> rt5575_fw_load_by_spi(), which performs no I2C-specific tasks, is found
>> with the common, rt5575.c file?
> 
> There are few I2C r/w in rt5575_fw_load_by_spi().

It's all abstracted away by regmap.

> 
> But the SPI load firmware function can move to rt5575-spi.c.
> The rt5575_spi_fw_load() will do all firmware load related works, and it
> will return request_firmware_xxx() result.
> The rt5575_fw_load_by_spi() will be implement as following, and it will put
> into the bottom of rt5575_i2c_probe().

Did you try my suggestions, compiled them and tested before following 
up? The above looks like an evasion-tactic.

> 
> rt5575_fw_load_by_spi()
> {
> 	...
> 
> 	regmap_write(rt5575->dsp_regmap, 0xfafafafa, 0x00000004);
> 	regmap_write(rt5575->dsp_regmap, 0x18008064, 0x00000000);
> 	regmap_write(rt5575->dsp_regmap, 0x18008068, 0x0002ffff);
> 
> 	ret = rt5575_spi_fw_load();
> 	if (ret) {
> 		dev_err(dev, "Load firmware failure: %d\n", ret);
> 		return ENODEV;
> 	}
> 
> 	regmap_write(rt5575->dsp_regmap, 0x18000000, 0x00000000);
> 	regmap_update_bits(rt5575->regmap, RT5575_SW_INT, 1, 1);
> 
> 	regmap_read_poll_timeout();
> 	if (ret) {
> 		dev_err(dev, "Run firmware failure: %d\n", ret);
> 		return ENODEV;
> 	}
> 
> 	...
> }
> 
>> We can do better.  There are couple options here, one of them consists of:
>>
>> 1) privatize rt5575_spi_burst_write()
>> 2) make rt5575_fw_load_by_spi() public
>> 3) change rt5575_fw_load_by_spi() from void to int and return
>> request_firmware_xxx() result
>> 4) reword to rt5575_spi_fw_load() to match its friends
>>
>> In regard to 1), have a #if-else preproc added to rt5575.h that governs
>> the implementation of said function.  If xxx_RT5575_SPI is disabled, let
>> is be a stub that returns 0.
>>
>> In regard to 2), please do not ignore failures from firmware loading,
>> that's a recurring point in this review and keeps being ignored.  No,
>> async-firmware loading in not the answer why potential errors are left
>> unhandled.
>>
>> Another option, perhaps a better one is to have both
>> rt5575_spi_burst_write() and rt5575_spi_fw_load() privatized and move
>> the firmware-loading to the SPI-device probe.  See my comments targeting
>> rt5575_probe().
>>
>>> +{
>>> +     struct rt5575_priv *rt5575 = context;
>>> +     struct i2c_client *i2c = rt5575->i2c;
>>
>> The whole reason this function needs i2c_client is ->dev retrieval.
>> Let's simplify by listing 'struct device *dev' as an argument instead.
>> With that, your function's argument list is also more explicit.
>>
>>> +     const struct firmware *firmware;
>>> +     static const char * const fw_path[] = {
>>> +             "realtek/rt5575/rt5575_fw2.bin",
>>> +             "realtek/rt5575/rt5575_fw3.bin",
>>> +             "realtek/rt5575/rt5575_fw4.bin"
>>> +     };
>>> +     static const u32 fw_addr[] = { 0x5f600000, 0x5f7fe000, 0x5f7ff000 };
>>> +     int i, ret;
>>> +
>>> +     regmap_write(rt5575->dsp_regmap, 0xfafafafa, 0x00000004);
>>> +     regmap_write(rt5575->dsp_regmap, 0x18008064, 0x00000000);
>>> +     regmap_write(rt5575->dsp_regmap, 0x18008068, 0x0002ffff);
>>> +
>>> +     rt5575_spi_burst_write(0x5f400000, fw->data, fw->size);
>>> +     release_firmware(fw);
>>> +
>>> +     for (i = 0; i < ARRAY_SIZE(fw_addr); i++) {
>>> +             ret = request_firmware(&firmware, fw_path[i], &i2c->dev);
>>> +             if (!ret) {
>>> +                     rt5575_spi_burst_write(fw_addr[i],
>> firmware->data, firmware->size);
>>> +                     release_firmware(firmware);
>>> +             } else {
>>> +                     dev_err(&i2c->dev, "Request firmware failure:
>> %d\n", ret);
>>> +                     return;
>>> +             }
>>> +     }
>>> +
>>> +     regmap_write(rt5575->dsp_regmap, 0x18000000, 0x00000000);
>>> +
>>> +     regmap_update_bits(rt5575->regmap, RT5575_SW_INT, 1, 1);
>>> +
>>> +     regmap_read_poll_timeout(rt5575->regmap, RT5575_SW_INT,
>> ret, !ret, 100000, 10000000);
>>> +
>>> +     if (ret)
>>> +             dev_err(&i2c->dev, "Run firmware failure: %d\n", ret);
>>> +}
>>> +#endif

...

>>> +static int rt5575_probe(struct snd_soc_component *component)
>>> +{
>>> +     struct rt5575_priv *rt5575 =
>> snd_soc_component_get_drvdata(component);
>>> +     struct device *dev = component->dev;
>>> +
>>> +     rt5575->component = component;
>>> +
>>> +     dev_info(dev, "Private ID: %llx\n", rt5575_get_priv_id(rt5575));
>>> +
>>> +#if IS_ENABLED(CONFIG_SND_SOC_RT5575_SPI)
>>> +     if (of_device_is_compatible(dev->of_node,
>> rt5575_of_match[1].compatible))
>>> +             request_firmware_nowait(THIS_MODULE,
>> FW_ACTION_UEVENT,
>>
>>
>> An ASoC card is typically a mariage of platform-component (e.g.: SoC
>> level DSP) and a codec-component (e.g.: rt5575).  If for any reason the
>> platform-component becomes unloaded, codec-component will also be.
>> When
>> the platform-component becomes available again, the
>> codec-component->probe() would be invoked again - firmware would be
>> loaded again.  Is this the desired behaviour?
>>
>> To answer that, the follow up question is:  Is the firmware-loading for
>> this particular solution, a chip -level operation or, a sound-card
>> -level operation?  Typically firmware-loading is the former.  Once
>> succeeeds, an ASoC component can be registered.  There is no reason to
>> register ASoC component, which is mainly used for PCM/streaming reasons,
>> if the preliminary setup - firmware-loading - is unsuccefull.
>>
>> Perhaps a good approach is to move the firmware loading to the
>> SPI-device's probe()!
> 
> But before the firmware-loading, I2C commands are needed. The SPI
> firmware-loading cannot be independent by I2C.

 From the design perspective, we're dealing with up to components and 
somewhat of parent (I2C) <- child (SPI) relation.

Code should make sure a child is never probed before the parent is 
operational.

1) boot I2C, init regmaps
2) boot SPI, load firmware
3) register ASoC component if 1) and 2) succeed

[PATCH v8 2/2] ASoC: dt-bindings: realtek,rt5575: add bindings for ALC5575

[Oder Chiou]

Realtek ALC5575 is a highly advanced DSP and microphone CODEC that has
been designed for AI audio technology. Its impressive features include
an advanced HiFi-5 DSP core, a Neural Network Processing Unit (NPU)
owned by Realtek, and embedded 4MB memory, which enables it to operate
highly advanced AI audio algorithms. The ALC5575 supports 4xA-mic input
and 8xD-mic input, as well as a rich set of interfaces such as I2S, I2C,
SPI, and UART.

Signed-off-by: Oder Chiou <oder_chiou@realtek.com>
---
 .../bindings/sound/realtek,rt5575.yaml        | 44 +++++++++++++++++++
 1 file changed, 44 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/realtek,rt5575.yaml

diff --git a/Documentation/devicetree/bindings/sound/realtek,rt5575.yaml b/Documentation/devicetree/bindings/sound/realtek,rt5575.yaml
new file mode 100644
index 000000000000..83ccc79e6769
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/realtek,rt5575.yaml
@@ -0,0 +1,44 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/sound/realtek,rt5575.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ALC5575 audio CODEC
+
+maintainers:
+  - Oder Chiou <oder_chiou@realtek.com>
+
+description:
+  The device supports both I2C and SPI. I2C is mandatory, while SPI is
+  optional depending on the hardware configuration.
+
+allOf:
+  - $ref: /schemas/spi/spi-peripheral-props.yaml#
+  - $ref: dai-common.yaml#
+
+properties:
+  compatible:
+    enum:
+      - realtek,rt5575
+      - realtek,rt5575-with-spi
+
+  reg:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        codec@57 {
+            compatible = "realtek,rt5575";
+            reg = <0x57>;
+        };
+    };
-- 
2.52.0

Re: [PATCH v8 2/2] ASoC: dt-bindings: realtek,rt5575: add bindings for ALC5575

[Krzysztof Kozlowski]

On Mon, Dec 01, 2025 at 06:59:26PM +0800, Oder Chiou wrote:
> Realtek ALC5575 is a highly advanced DSP and microphone CODEC that has
> been designed for AI audio technology. Its impressive features include
> an advanced HiFi-5 DSP core, a Neural Network Processing Unit (NPU)
> owned by Realtek, and embedded 4MB memory, which enables it to operate
> highly advanced AI audio algorithms. The ALC5575 supports 4xA-mic input
> and 8xD-mic input, as well as a rich set of interfaces such as I2S, I2C,
> SPI, and UART.

When I asked to describe hardware, I did not meant marketing junk! Drop
all impressive features and simply describe hardware in basic terms.
This is not advanced DSP, not designed for AI audio (AI is the easiest way to
get a grumpy review), not "highly advanced AI audio algorithms" and does
not have "as a rich set of interfaces".

Use simple terms what is this. Audio codec with I2S.... interfaces.
> 
> Signed-off-by: Oder Chiou <oder_chiou@realtek.com>
> ---
>  .../bindings/sound/realtek,rt5575.yaml        | 44 +++++++++++++++++++
>  1 file changed, 44 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/sound/realtek,rt5575.yaml
> 
> diff --git a/Documentation/devicetree/bindings/sound/realtek,rt5575.yaml b/Documentation/devicetree/bindings/sound/realtek,rt5575.yaml
> new file mode 100644
> index 000000000000..83ccc79e6769
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/realtek,rt5575.yaml
> @@ -0,0 +1,44 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/sound/realtek,rt5575.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: ALC5575 audio CODEC
> +
> +maintainers:
> +  - Oder Chiou <oder_chiou@realtek.com>
> +
> +description:
> +  The device supports both I2C and SPI. I2C is mandatory, while SPI is
> +  optional depending on the hardware configuration.
> +
> +allOf:
> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +  - $ref: dai-common.yaml#
> +
> +properties:
> +  compatible:
> +    enum:
> +      - realtek,rt5575
> +      - realtek,rt5575-with-spi

Drop the second compatible. It's the same device. Whether it supports
SPI it is already known and obvious - you cannot place non-SPI chip on
SPI bus and expect it to work.

> +
> +  reg:
> +    maxItems: 1
> +

You listed so many "impressive" and "rich" features that for sure this
is incomplete.

Please post complete bindings for "impressive" device.

You miss supplies, you miss all the AI related hype (no clue what that
would be, but for sure NPU feels like needing remoteproc or at least
some other way to communicate). We all know that AI is power hungry, so
it is impossible to run it without electricity (thus supplies).

Best regards,
Krzysztof

Re: [PATCH v8 2/2] ASoC: dt-bindings: realtek,rt5575: add bindings for ALC5575

[Krzysztof Kozlowski]

On 05/12/2025 09:31, Krzysztof Kozlowski wrote:
> 
>> +
>> +  reg:
>> +    maxItems: 1
>> +
> 
> You listed so many "impressive" and "rich" features that for sure this
> is incomplete.
> 
> Please post complete bindings for "impressive" device.
> 
> You miss supplies, you miss all the AI related hype (no clue what that
> would be, but for sure NPU feels like needing remoteproc or at least
> some other way to communicate). We all know that AI is power hungry, so
> it is impossible to run it without electricity (thus supplies).
>

Also two standard nits, in case I did not ask about them before:

A nit, subject: drop second/last, redundant "bindings". The
"dt-bindings" prefix is already stating that these are bindings.
See also:
https://elixir.bootlin.com/linux/v6.17-rc3/source/Documentation/devicetree/bindings/submitting-patches.rst#L18

Please organize the patch documenting the compatible (DT bindings)
before the patch using that compatible.
See also:
https://elixir.bootlin.com/linux/v6.14-rc6/source/Documentation/devicetree/bindings/submitting-patches.rst#L46


Best regards,
Krzysztof

RE: [PATCH v8 2/2] ASoC: dt-bindings: realtek,rt5575: add bindings for ALC5575

[Oder Chiou]

> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: Friday, December 5, 2025 4:32 PM
> To: Oder Chiou <oder_chiou@realtek.com>
> Cc: cezary.rojewski@intel.com; broonie@kernel.org; lgirdwood@gmail.com;
> robh@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org; perex@perex.cz;
> linux-sound@vger.kernel.org; devicetree@vger.kernel.org;
> alsa-devel@alsa-project.org; Flove(HsinFu) <flove@realtek.com>; Shuming [范
> 書銘] <shumingf@realtek.com>; Jack Yu <jack.yu@realtek.com>; Derek [方德
> 義] <derek.fang@realtek.com>
> Subject: Re: [PATCH v8 2/2] ASoC: dt-bindings: realtek,rt5575: add bindings for
> ALC5575
> 
> 
> External mail : This email originated from outside the organization. Do not
> reply, click links, or open attachments unless you recognize the sender and
> know the content is safe.
> 
> 
> 
> On 05/12/2025 09:31, Krzysztof Kozlowski wrote:
> >
> >> +
> >> +  reg:
> >> +    maxItems: 1
> >> +
> >
> > You listed so many "impressive" and "rich" features that for sure this
> > is incomplete.
> >
> > Please post complete bindings for "impressive" device.
> >
> > You miss supplies, you miss all the AI related hype (no clue what that
> > would be, but for sure NPU feels like needing remoteproc or at least
> > some other way to communicate). We all know that AI is power hungry,
> > so it is impossible to run it without electricity (thus supplies).
> >
> 
> Also two standard nits, in case I did not ask about them before:
> 
> A nit, subject: drop second/last, redundant "bindings". The "dt-bindings" prefix
> is already stating that these are bindings.
> See also:
> https://elixir.bootlin.com/linux/v6.17-rc3/source/Documentation/devicetree/bi
> ndings/submitting-patches.rst#L18

The subject will be change to
ASoC: dt-bindings: realtek,rt5575: add support for ALC5575

> Please organize the patch documenting the compatible (DT bindings) before
> the patch using that compatible.
> See also:
> https://elixir.bootlin.com/linux/v6.14-rc6/source/Documentation/devicetree/bi
> ndings/submitting-patches.rst#L46

I will reorder the patches so that the DT bindings come first.

Thanks.

RE: [PATCH v8 2/2] ASoC: dt-bindings: realtek,rt5575: add bindings for ALC5575

[Oder Chiou]

> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: Friday, December 5, 2025 4:31 PM
> To: Oder Chiou <oder_chiou@realtek.com>
> Cc: cezary.rojewski@intel.com; broonie@kernel.org; lgirdwood@gmail.com;
> robh@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org; perex@perex.cz;
> linux-sound@vger.kernel.org; devicetree@vger.kernel.org;
> alsa-devel@alsa-project.org; Flove(HsinFu) <flove@realtek.com>; Shuming [范
> 書銘] <shumingf@realtek.com>; Jack Yu <jack.yu@realtek.com>; Derek [方德
> 義] <derek.fang@realtek.com>
> Subject: Re: [PATCH v8 2/2] ASoC: dt-bindings: realtek,rt5575: add bindings for
> ALC5575
> 
> 
> External mail : This email originated from outside the organization. Do not
> reply, click links, or open attachments unless you recognize the sender and
> know the content is safe.
> 
> 
> 
> On Mon, Dec 01, 2025 at 06:59:26PM +0800, Oder Chiou wrote:
> > Realtek ALC5575 is a highly advanced DSP and microphone CODEC that has
> > been designed for AI audio technology. Its impressive features include
> > an advanced HiFi-5 DSP core, a Neural Network Processing Unit (NPU)
> > owned by Realtek, and embedded 4MB memory, which enables it to operate
> > highly advanced AI audio algorithms. The ALC5575 supports 4xA-mic input
> > and 8xD-mic input, as well as a rich set of interfaces such as I2S, I2C,
> > SPI, and UART.
> 
> When I asked to describe hardware, I did not meant marketing junk! Drop
> all impressive features and simply describe hardware in basic terms.
> This is not advanced DSP, not designed for AI audio (AI is the easiest way to
> get a grumpy review), not "highly advanced AI audio algorithms" and does
> not have "as a rich set of interfaces".
> 
> Use simple terms what is this. Audio codec with I2S.... interfaces.

The description will be change to
Audio codec with I2S, I2C and SPI.

> >
> > Signed-off-by: Oder Chiou <oder_chiou@realtek.com>
> > ---
> >  .../bindings/sound/realtek,rt5575.yaml        | 44
> +++++++++++++++++++
> >  1 file changed, 44 insertions(+)
> >  create mode 100644
> Documentation/devicetree/bindings/sound/realtek,rt5575.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/sound/realtek,rt5575.yaml
> b/Documentation/devicetree/bindings/sound/realtek,rt5575.yaml
> > new file mode 100644
> > index 000000000000..83ccc79e6769
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/sound/realtek,rt5575.yaml
> > @@ -0,0 +1,44 @@
> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/sound/realtek,rt5575.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: ALC5575 audio CODEC
> > +
> > +maintainers:
> > +  - Oder Chiou <oder_chiou@realtek.com>
> > +
> > +description:
> > +  The device supports both I2C and SPI. I2C is mandatory, while SPI is
> > +  optional depending on the hardware configuration.
> > +
> > +allOf:
> > +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> > +  - $ref: dai-common.yaml#
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - realtek,rt5575
> > +      - realtek,rt5575-with-spi
> 
> Drop the second compatible. It's the same device. Whether it supports
> SPI it is already known and obvious - you cannot place non-SPI chip on
> SPI bus and expect it to work.

There are two hardware configurations: with SPI flash and without SPI flash.
If the codec is shipped without an SPI flash, the SPI driver can still load
the firmware through the SPI interface. The second compatible is intended to
distinguish between the versions with and without the SPI flash.

Re: [PATCH v8 2/2] ASoC: dt-bindings: realtek,rt5575: add bindings for ALC5575

[Krzysztof Kozlowski]

On 08/12/2025 06:55, Oder Chiou wrote:
>>> +properties:
>>> +  compatible:
>>> +    enum:
>>> +      - realtek,rt5575
>>> +      - realtek,rt5575-with-spi
>>
>> Drop the second compatible. It's the same device. Whether it supports
>> SPI it is already known and obvious - you cannot place non-SPI chip on
>> SPI bus and expect it to work.
> 
> There are two hardware configurations: with SPI flash and without SPI flash.
> If the codec is shipped without an SPI flash, the SPI driver can still load
> the firmware through the SPI interface. The second compatible is intended to
> distinguish between the versions with and without the SPI flash.
> 

Ah, so this is not about the bus. I don't see that compatible referenced
in your device ID table, so this is dead code. Drop or justify - by
writing driver code using it....

Best regards,
Krzysztof

Re: [PATCH v8 2/2] ASoC: dt-bindings: realtek,rt5575: add bindings for ALC5575

[Krzysztof Kozlowski]

On 08/12/2025 07:02, Krzysztof Kozlowski wrote:
> On 08/12/2025 06:55, Oder Chiou wrote:
>>>> +properties:
>>>> +  compatible:
>>>> +    enum:
>>>> +      - realtek,rt5575
>>>> +      - realtek,rt5575-with-spi
>>>
>>> Drop the second compatible. It's the same device. Whether it supports
>>> SPI it is already known and obvious - you cannot place non-SPI chip on
>>> SPI bus and expect it to work.
>>
>> There are two hardware configurations: with SPI flash and without SPI flash.
>> If the codec is shipped without an SPI flash, the SPI driver can still load
>> the firmware through the SPI interface. The second compatible is intended to
>> distinguish between the versions with and without the SPI flash.
>>
> 
> Ah, so this is not about the bus. I don't see that compatible referenced
> in your device ID table, so this is dead code. Drop or justify - by
> writing driver code using it....
> 

Wait, I saw it in deice ID table, but there is no driver data so still
pointless.

Best regards,
Krzysztof