[PATCH v8 0/2] ASoC: rt5575: Add the codec driver for the ALC5575
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
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__ */
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
-----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
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
-----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.
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
-----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
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.
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;elseend = 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");
-----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;elseend = 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:
- privatize rt5575_spi_burst_write()
- make rt5575_fw_load_by_spi() public
- 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
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:
- privatize rt5575_spi_burst_write()
- make rt5575_fw_load_by_spi() public
- 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
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>; + }; + };
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
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/b...
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/b...
Best regards, Krzysztof
-----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/b... 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/b... ndings/submitting-patches.rst#L46
I will reorder the patches so that the DT bindings come first.
Thanks.
-----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-spiDrop 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.
On 08/12/2025 06:55, Oder Chiou wrote:
+properties:
- compatible:
- enum:
- realtek,rt5575- realtek,rt5575-with-spiDrop 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
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-spiDrop 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
participants (3)
-
Cezary Rojewski -
Krzysztof Kozlowski -
Oder Chiou