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

On 2025-11-21 9:41 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

Please CC the reviewers who actively review your patches, makes it easier not miss the follow ups.

Not talking about the tag-area, just the email Cc :)

---

sound/soc/codecs/Kconfig | 10 + sound/soc/codecs/Makefile | 4 + sound/soc/codecs/rt5575-spi.c | 86 ++++++++ sound/soc/codecs/rt5575-spi.h | 16 ++ sound/soc/codecs/rt5575.c | 370 ++++++++++++++++++++++++++++++++++ sound/soc/codecs/rt5575.h | 54 +++++ 6 files changed, 540 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 a0dfef57200c..a3ea5febd1e0 100644 --- a/sound/soc/codecs/Kconfig +++ b/sound/soc/codecs/Kconfig @@ -211,6 +211,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

@@ -1767,6 +1768,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 39138d96a720..82f660cbe8ec 100644 --- a/sound/soc/codecs/Makefile +++ b/sound/soc/codecs/Makefile @@ -252,6 +252,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 @@ -684,6 +686,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..9dbc8170fb76 --- /dev/null +++ b/sound/soc/codecs/rt5575-spi.c @@ -0,0 +1,86 @@ +// 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;

+bool rt5575_spi_ready;

I believe the patch is dead on arrival, unfortunately as it won't compile as =m (module) when CONFIG_SND_SOC_RT5575_SPI is enabled as =m simultaneously. EXPORT_SYMBOL_GPL() or a friend of his is missing.

+static struct spi_device *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;
• rt5575_spi_ready = true;

I still do not understand the logic behind having rt5575_spi _and_ rt5575_spi_ready. The latter is used just one, in the I2C-device probe. rt5575_spi is static and will be by default initialized to NULL. Statement 'if (!rt5575_spi)' is enough to cover that single occurrence in your code.

• return 0;

+}

...

diff --git a/sound/soc/codecs/rt5575.c b/sound/soc/codecs/rt5575.c new file mode 100644 index 000000000000..58508d643273 --- /dev/null +++ b/sound/soc/codecs/rt5575.c

...

+static int rt5575_probe(struct snd_soc_component *component) +{

• struct rt5575_priv *rt5575 = snd_soc_component_get_drvdata(component);
• rt5575->component = component;
• dev_info(component->dev, "Private ID: %llx\n", rt5575_get_priv_id(rt5575));

+#if IS_ENABLED(CONFIG_SND_SOC_RT5575_SPI)

• request_firmware_nowait(THIS_MODULE, FW_ACTION_UEVENT, "realtek/rt5575/rt5575_fw1.bin",

• component->dev, GFP_KERNEL, rt5575, rt5575_fw_load_by_spi);
  

I do not believe this is OK. If the SPI component exists, from the code-perspective it looks like the firmware-loading procedure is essential and failing to load the firmware should halt driver's operations.

See below as this touches on the driver design which looks off and I haven't found any valid argumentation in your previous message that would clarify the situation.

+#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;
• int ret, val;

+#if IS_ENABLED(CONFIG_SND_SOC_RT5575_SPI)

• if (!rt5575_spi_ready)

• return -EPROBE_DEFER;
  

+#endif

I finally understood what's off in the design, and this part is crucial to the subject.

If CONFIG_SND_SOC_RT5575_SPI is enabled, and the SPI kept failing for whatever reason, the I2C device probe would get deferred continuously. As the kconfig looks optional, the question is: Does rt5575 codec exist in a "without SPI flash" version or, is it always shipped with SPI flash?

If it does exist as a standalone component - without SPI - then enabling CONFIG_SND_SOC_RT5575_SPI effectively renders the driver dysfunctional.

• rt5575 = devm_kzalloc(&i2c->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(&i2c->dev, "Failed to allocate register map: %d\n", ret);
  

• return ret;
  

• }
• rt5575->regmap = devm_regmap_init(&i2c->dev, NULL, i2c, &rt5575_regmap);
• if (IS_ERR(rt5575->regmap)) {

• ret = PTR_ERR(rt5575->regmap);
  

• dev_err(&i2c->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(&i2c->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(&i2c->dev, "This is not formal version\n");
  

• return -ENODEV;
  

• }
• return devm_snd_soc_register_component(&i2c->dev, &rt5575_soc_component_dev, rt5575_dai,

• ARRAY_SIZE(rt5575_dai));
  

+}

+static const struct of_device_id rt5575_of_match[] = {

• { .compatible = "realtek,rt5575" },
• { }

+}; +MODULE_DEVICE_TABLE(of, rt5575_of_match);

+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");