ASoC: rt5575: Add the codec driver for the ALC5575

sound/soc/codecs/rt5575.c new file +22 -0

+
+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 = {

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

+
+static const struct i2c_device_id rt5575_i2c_id[] = {
+	{ "rt5575" },
+	{ }

sound/soc/codecs/rt5575.c new file +30 -0

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

-----Original Message-----
From: Krzysztof Kozlowski <[email protected]>
Sent: Monday, December 8, 2025 2:05 PM
To: Oder Chiou <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Cc: [email protected]; [email protected]; [email protected]; [email protected]; Flove(HsinFu) <[email protected]>; Shuming [范 書銘] <[email protected]>; Jack Yu <[email protected]>; Derek [方德 義] <[email protected]>
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.

Review

Quoted 2 lines · Oder Chiou

(quoted diff) +2 -0

+
+static int rt5575_i2c_read(void *context, unsigned int reg, unsigned int

*val)

sound/soc/codecs/rt5575.c

Quoted 5 lines

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

+{
+     struct i2c_client *client = context;
+     struct rt5575_priv *rt5575 = i2c_get_clientdata(client);
+
+     regmap_read(rt5575->dsp_regmap, reg | RT5575_DSP_MAPPING,

val);

sound/soc/codecs/rt5575.c

Quoted 10 lines

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

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

Quoted 9 lines

sound/soc/codecs/rt5575.c new file +9 -0

+
+     return 0;
+}
+
+static const struct regmap_config rt5575_regmap = {
+     .reg_bits = 16,
+     .val_bits = 32,
+     .reg_stride = 4,
+     .max_register = 0xfffc,

sound/soc/codecs/rt5575.c line 304

Quoted 6 lines

sound/soc/codecs/rt5575.c new file +6 -0

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

Quoted 8 lines

sound/soc/codecs/rt5575.c new file +8 -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)

sound/soc/codecs/rt5575.c line 318

Quoted 6 lines

sound/soc/codecs/rt5575.c new file +6 -0

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

Quoted 1 line

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

sound/soc/codecs/rt5575.c line 325

Quoted 6 lines

sound/soc/codecs/rt5575.c new file +6 -0

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

sound/soc/codecs/rt5575.c

Quoted 7 lines

sound/soc/codecs/rt5575.c new file +7 -0

+             return -ENOMEM;
+
+     i2c_set_clientdata(i2c, rt5575);
+
+     rt5575->i2c = i2c;
+
+     rt5575->dsp_regmap = devm_regmap_init_i2c(i2c,

&rt5575_dsp_regmap);

sound/soc/codecs/rt5575.c

Quoted 7 lines

sound/soc/codecs/rt5575.c new file +7 -0

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

sound/soc/codecs/rt5575.c

Quoted 9 lines

sound/soc/codecs/rt5575.c new file +9 -0

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

sound/soc/codecs/rt5575.c

Quoted 10 lines

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

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

sound/soc/codecs/rt5575.c line 365

Quoted 7 lines

sound/soc/codecs/rt5575.c new file +7 -0

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

Quoted 1 line

+ .of_match_table = of_match_ptr(rt5575_of_match),

You have warning here. Drop of_match_ptr.

Quoted 50 lines

-----Original Message-----
From: Krzysztof Kozlowski <[email protected]>
Sent: Monday, December 8, 2025 2:05 PM
To: Oder Chiou <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Cc: [email protected]; [email protected]; [email protected]; [email protected]; Flove(HsinFu) <[email protected]>; Shuming [范 書銘] <[email protected]>; Jack Yu <[email protected]>; Derek [方德 義] <[email protected]>
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.

Review

Quoted 2 lines · Oder Chiou

(quoted diff) +2 -0

+
+static int rt5575_i2c_read(void *context, unsigned int reg, unsigned int

*val)

sound/soc/codecs/rt5575.c

Quoted 5 lines

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

+{
+     struct i2c_client *client = context;
+     struct rt5575_priv *rt5575 = i2c_get_clientdata(client);
+
+     regmap_read(rt5575->dsp_regmap, reg | RT5575_DSP_MAPPING,

val);

sound/soc/codecs/rt5575.c

Quoted 10 lines

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

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

Quoted 9 lines

sound/soc/codecs/rt5575.c new file +9 -0

+
+     return 0;
+}
+
+static const struct regmap_config rt5575_regmap = {
+     .reg_bits = 16,
+     .val_bits = 32,
+     .reg_stride = 4,
+     .max_register = 0xfffc,

Quoted 8 lines

sound/soc/codecs/rt5575.c line 304

Quoted 6 lines

sound/soc/codecs/rt5575.c new file +6 -0

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

Quoted 8 lines

Quoted 8 lines

sound/soc/codecs/rt5575.c new file +8 -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)

Quoted 8 lines

sound/soc/codecs/rt5575.c line 318

Quoted 6 lines

sound/soc/codecs/rt5575.c new file +6 -0

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

Quoted 6 lines

Quoted 2 lines

+ 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 },

-----Original Message-----
From: Krzysztof Kozlowski <[email protected]>
Sent: Monday, December 8, 2025 4:02 PM
To: Oder Chiou <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Cc: [email protected]; [email protected]; [email protected]; [email protected]; Flove(HsinFu) <[email protected]>; Shuming [范 書銘] <[email protected]>; Jack Yu <[email protected]>; Derek [方德 義] <[email protected]>
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.

Quoted 10 lines · Oder Chiou

Quoted 10 lines

-----Original Message-----
From: Krzysztof Kozlowski <[email protected]>
Sent: Monday, December 8, 2025 2:05 PM
To: Oder Chiou <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Cc: [email protected]; [email protected]; [email protected]; [email protected]; Flove(HsinFu) <[email protected]>; Shuming [范
書銘] <[email protected]>; Jack Yu <[email protected]>; Derek [方

德

Quoted 69 lines

Quoted 43 lines

義] <[email protected]>
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.

Review

Quoted 3 lines · Oder Chiou

(quoted diff) +3 -0

+
+static int rt5575_i2c_read(void *context, unsigned int reg,
+unsigned int

*val)

sound/soc/codecs/rt5575.c

Quoted 5 lines

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

+{
+     struct i2c_client *client = context;
+     struct rt5575_priv *rt5575 = i2c_get_clientdata(client);
+
+     regmap_read(rt5575->dsp_regmap, reg | RT5575_DSP_MAPPING,

val);

sound/soc/codecs/rt5575.c

Quoted 10 lines

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

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

Quoted 9 lines

sound/soc/codecs/rt5575.c new file +9 -0

+
+     return 0;
+}
+
+static const struct regmap_config rt5575_regmap = {
+     .reg_bits = 16,
+     .val_bits = 32,
+     .reg_stride = 4,
+     .max_register = 0xfffc,

Quoted 8 lines

sound/soc/codecs/rt5575.c line 304

Quoted 6 lines

sound/soc/codecs/rt5575.c new file +6 -0

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

Quoted 7 lines

Quoted 7 lines

sound/soc/codecs/rt5575.c new file +7 -0

+
+static const struct i2c_device_id rt5575_i2c_id[] = {
+     { "rt5575" },
+     { }
+};
+MODULE_DEVICE_TABLE(i2c, rt5575_i2c_id);
+

Quoted 8 lines

sound/soc/codecs/rt5575.c line 318

Quoted 6 lines

sound/soc/codecs/rt5575.c new file +6 -0

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

Quoted 3 lines

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.

Quoted 10 lines

Quoted 6 lines

Quoted 2 lines

+ 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 },

Quoted 2 lines

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

Quoted 118 lines

-----Original Message-----
From: Krzysztof Kozlowski <[email protected]>
Sent: Monday, December 8, 2025 4:02 PM
To: Oder Chiou <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Cc: [email protected]; [email protected]; [email protected]; [email protected]; Flove(HsinFu) <[email protected]>; Shuming [范 書銘] <[email protected]>; Jack Yu <[email protected]>; Derek [方德 義] <[email protected]>
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.

Quoted 10 lines · Oder Chiou

Quoted 10 lines

-----Original Message-----
From: Krzysztof Kozlowski <[email protected]>
Sent: Monday, December 8, 2025 2:05 PM
To: Oder Chiou <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Cc: [email protected]; [email protected]; [email protected]; [email protected]; Flove(HsinFu) <[email protected]>; Shuming [范
書銘] <[email protected]>; Jack Yu <[email protected]>; Derek [方

德

Quoted 69 lines

Quoted 43 lines

義] <[email protected]>
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.

Review

Quoted 3 lines · Oder Chiou

(quoted diff) +3 -0

+
+static int rt5575_i2c_read(void *context, unsigned int reg,
+unsigned int

*val)

sound/soc/codecs/rt5575.c

Quoted 5 lines

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

+{
+     struct i2c_client *client = context;
+     struct rt5575_priv *rt5575 = i2c_get_clientdata(client);
+
+     regmap_read(rt5575->dsp_regmap, reg | RT5575_DSP_MAPPING,

val);

sound/soc/codecs/rt5575.c

Quoted 10 lines

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

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

Quoted 9 lines

sound/soc/codecs/rt5575.c new file +9 -0

+
+     return 0;
+}
+
+static const struct regmap_config rt5575_regmap = {
+     .reg_bits = 16,
+     .val_bits = 32,
+     .reg_stride = 4,
+     .max_register = 0xfffc,

Quoted 8 lines

sound/soc/codecs/rt5575.c line 304

Quoted 6 lines

sound/soc/codecs/rt5575.c new file +6 -0

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

Quoted 7 lines

Quoted 7 lines

sound/soc/codecs/rt5575.c new file +7 -0

+
+static const struct i2c_device_id rt5575_i2c_id[] = {
+     { "rt5575" },
+     { }
+};
+MODULE_DEVICE_TABLE(i2c, rt5575_i2c_id);
+

Quoted 8 lines

sound/soc/codecs/rt5575.c line 318

Quoted 6 lines

sound/soc/codecs/rt5575.c new file +6 -0

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

Quoted 3 lines

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.

Quoted 10 lines

Quoted 6 lines

Quoted 2 lines

+ 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

Quoted 6 lines

*)RT5575_WITH_FLASH },

Quoted 2 lines

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

-----Original Message-----
From: Krzysztof Kozlowski <[email protected]>
Sent: Tuesday, December 9, 2025 2:08 PM
To: Oder Chiou <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Cc: [email protected]; [email protected]; [email protected]; [email protected]; Flove(HsinFu) <[email protected]>; Shuming [范 書銘] <[email protected]>; Jack Yu <[email protected]>; Derek [方德 義] <[email protected]>
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.

Quoted 7 lines · Oder Chiou

Quoted 7 lines

-----Original Message-----
From: Krzysztof Kozlowski <[email protected]>
Sent: Monday, December 8, 2025 4:02 PM
To: Oder Chiou <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Cc: [email protected]; [email protected];

[email protected];

Quoted 1 line

Quoted 1 line

[email protected]; Flove(HsinFu) <[email protected]>; Shuming

[范

Quoted 1 line

Quoted 1 line

書銘] <[email protected]>; Jack Yu <[email protected]>; Derek [方

德

Quoted 12 lines

Quoted 12 lines

義] <[email protected]>
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:

Quoted 10 lines

Quoted 10 lines

Quoted 10 lines

Quoted 10 lines

-----Original Message-----
From: Krzysztof Kozlowski <[email protected]>
Sent: Monday, December 8, 2025 2:05 PM
To: Oder Chiou <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Cc: [email protected]; [email protected]; [email protected]; [email protected]; Flove(HsinFu) <[email protected]>; Shuming [范
書銘] <[email protected]>; Jack Yu <[email protected]>; Derek

[方

Quoted 18 lines

Quoted 18 lines

德

Quoted 17 lines

Quoted 17 lines

義] <[email protected]>
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.

Review

Quoted 3 lines · Oder Chiou

(quoted diff) +3 -0

+
+static int rt5575_i2c_read(void *context, unsigned int reg,
+unsigned int

*val)

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

sound/soc/codecs/rt5575.c

Quoted 5 lines

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

+{
+     struct i2c_client *client = context;
+     struct rt5575_priv *rt5575 = i2c_get_clientdata(client);
+
+     regmap_read(rt5575->dsp_regmap, reg |

RT5575_DSP_MAPPING,

Quoted 11 lines

Quoted 11 lines

Quoted 11 lines

Quoted 11 lines

val);

sound/soc/codecs/rt5575.c

Quoted 10 lines

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

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

Quoted 63 lines

Quoted 53 lines

Quoted 36 lines

Quoted 10 lines

val);

Quoted 9 lines

sound/soc/codecs/rt5575.c new file +9 -0

+
+     return 0;
+}
+
+static const struct regmap_config rt5575_regmap = {
+     .reg_bits = 16,
+     .val_bits = 32,
+     .reg_stride = 4,
+     .max_register = 0xfffc,

Quoted 8 lines

sound/soc/codecs/rt5575.c line 304

Quoted 6 lines

sound/soc/codecs/rt5575.c new file +6 -0

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

Quoted 7 lines

Quoted 7 lines

sound/soc/codecs/rt5575.c new file +7 -0

+
+static const struct i2c_device_id rt5575_i2c_id[] = {
+     { "rt5575" },
+     { }
+};
+MODULE_DEVICE_TABLE(i2c, rt5575_i2c_id);
+

Quoted 8 lines

sound/soc/codecs/rt5575.c line 318

Quoted 6 lines

sound/soc/codecs/rt5575.c new file +6 -0

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

Quoted 3 lines

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.

Quoted 10 lines

Quoted 6 lines

Quoted 2 lines

+ 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

Quoted 6 lines

*)RT5575_WITH_FLASH },

Quoted 2 lines

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

Quoted 4 lines

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.

Quoted 12 lines

-----Original Message-----
From: Krzysztof Kozlowski <[email protected]>
Sent: Tuesday, December 9, 2025 2:08 PM
To: Oder Chiou <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Cc: [email protected]; [email protected]; [email protected]; [email protected]; Flove(HsinFu) <[email protected]>; Shuming [范 書銘] <[email protected]>; Jack Yu <[email protected]>; Derek [方德 義] <[email protected]>
Subject: Re: [PATCH v8 1/2] ASoC: rt5575: Add the codec driver for the ALC5575

Quoted 3 lines

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 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 <[email protected]>

sound/soc/codecs/rt5575-spi.c new file +43 -0

new file mode 100644
index 000000000000..12c2379a061e
@@ -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;

sound/soc/codecs/rt5575-spi.c new file +31 -0

+	}
+
+	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 <[email protected]>");
+MODULE_LICENSE("GPL");

sound/soc/codecs/rt5575-spi.h new file +6 -0

new file mode 100644
index 000000000000..b364b49bb43e
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * rt5575-spi.h  --  ALC5575 SPI driver
+ *
+ * Copyright(c) 2025 Realtek Semiconductor Corp.
+ *

sound/soc/codecs/rt5575.c new file +44 -0

new file mode 100644
index 000000000000..c7e8f5a606bc
@@ -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:

sound/soc/codecs/rt5575.c new file +100 -0

+	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)
+{

sound/soc/codecs/rt5575.c new file +6 -0

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

sound/soc/codecs/rt5575.c new file +21 -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)

sound/soc/codecs/rt5575.c new file +14 -0

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

sound/soc/codecs/rt5575.c new file +16 -0

+}
+
+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 <[email protected]>");
+MODULE_LICENSE("GPL");

-----Original Message-----
From: Cezary Rojewski <[email protected]>
Sent: Tuesday, December 9, 2025 4:06 AM
To: Oder Chiou <[email protected]>
Cc: [email protected]; [email protected]; [email protected]; [email protected]; Flove(HsinFu) <[email protected]>; Shuming [范 書銘] <[email protected]>; Jack Yu <[email protected]>; Derek [方德 義] <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
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.

Quoted 8 lines · Oder Chiou

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

Signed-off-by Oder Chiou <[email protected]>

...

Quoted 49 lines

sound/soc/codecs/rt5575-spi.c new file +43 -0

new file mode 100644
index 000000000000..12c2379a061e
@@ -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;

sound/soc/codecs/rt5575-spi.c line 53

Quoted 10 lines

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

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

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

<assuming newline after the if-else-statement>

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

                offset += RT5575_SPI_BUF_LEN;

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

Quoted 43 lines

sound/soc/codecs/rt5575-spi.c new file +31 -0

+     }
+
+     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 <[email protected]>");
+MODULE_LICENSE("GPL");

sound/soc/codecs/rt5575-spi.h new file +6 -0

new file mode 100644
index 000000000000..b364b49bb43e
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * rt5575-spi.h  --  ALC5575 SPI driver
+ *
+ * Copyright(c) 2025 Realtek Semiconductor Corp.
+ *

sound/soc/codecs/rt5575-spi.h line 16

Quoted 10 lines

sound/soc/codecs/rt5575-spi.h new file +10 -0

+ */
+
+#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().

Quoted 57 lines

sound/soc/codecs/rt5575.c new file +51 -0

new file mode 100644
index 000000000000..c7e8f5a606bc
@@ -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);

sound/soc/codecs/rt5575.c line 54

Quoted 3 lines

sound/soc/codecs/rt5575.c new file +3 -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().

sound/soc/codecs/rt5575.c line 57

Quoted 3 lines

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

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

Quoted 10 lines

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

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

sound/soc/codecs/rt5575.c

Quoted 10 lines

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

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

sound/soc/codecs/rt5575.c

Quoted 3 lines

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

+                     release_firmware(firmware);
+             } else {
+                     dev_err(&i2c->dev, "Request firmware failure:

%d\n", ret);

sound/soc/codecs/rt5575.c

Quoted 9 lines

sound/soc/codecs/rt5575.c new file +9 -0

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

sound/soc/codecs/rt5575.c

Quoted 8 lines

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

+
+     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),

sound/soc/codecs/rt5575.c line 98

Quoted 1 line

sound/soc/codecs/rt5575.c new file +1 -0

+     SOC_DOUBLE_TLV("Speaker CH-01 Playback Volume",

RT5575_SPK01_VOL, 17, 1, 167, 0, ob_tlv),

sound/soc/codecs/rt5575.c line 99

Quoted 1 line

sound/soc/codecs/rt5575.c new file +1 -0

+     SOC_DOUBLE("Speaker CH-23 Playback Switch", RT5575_SPK23_VOL,

31, 15, 1, 1),

sound/soc/codecs/rt5575.c line 100

Quoted 1 line

sound/soc/codecs/rt5575.c new file +1 -0

+     SOC_DOUBLE_TLV("Speaker CH-23 Playback Volume",

RT5575_SPK23_VOL, 17, 1, 167, 0, ob_tlv),

sound/soc/codecs/rt5575.c line 101

Quoted 1 line

sound/soc/codecs/rt5575.c new file +1 -0

+     SOC_DOUBLE("Mic1 Capture Switch", RT5575_MIC1_VOL, 31, 15, 1,

1),

sound/soc/codecs/rt5575.c line 102

Quoted 1 line

sound/soc/codecs/rt5575.c new file +1 -0

+     SOC_DOUBLE_TLV("Mic1 Capture Volume", RT5575_MIC1_VOL, 17,

1, 167, 0, ob_tlv),

sound/soc/codecs/rt5575.c line 103

Quoted 1 line

sound/soc/codecs/rt5575.c new file +1 -0

+     SOC_DOUBLE("Mic2 Capture Switch", RT5575_MIC2_VOL, 31, 15, 1,

1),

sound/soc/codecs/rt5575.c line 104

Quoted 1 line

sound/soc/codecs/rt5575.c new file +1 -0

+     SOC_DOUBLE_TLV("Mic2 Capture Volume", RT5575_MIC2_VOL, 17,

1, 167, 0, ob_tlv),

sound/soc/codecs/rt5575.c line 105

Quoted 1 line

sound/soc/codecs/rt5575.c new file +1 -0

+     SOC_DOUBLE_R("Mix Playback Switch", RT5575_MIXL_VOL,

RT5575_MIXR_VOL, 31, 1, 1),

sound/soc/codecs/rt5575.c line 106

Quoted 1 line

sound/soc/codecs/rt5575.c new file +1 -0

+     SOC_DOUBLE_R_TLV("Mix Playback Volume", RT5575_MIXL_VOL,

RT5575_MIXR_VOL, 1, 127, 0,

sound/soc/codecs/rt5575.c line 108

Quoted 2 lines

sound/soc/codecs/rt5575.c new file +2 -0

+             ob_tlv),
+     SOC_DOUBLE("Prompt Playback Switch", RT5575_PROMPT_VOL, 31,

15, 1, 1),

sound/soc/codecs/rt5575.c line 109

Quoted 1 line

sound/soc/codecs/rt5575.c new file +1 -0

+     SOC_DOUBLE_TLV("Prompt Playback Volume",

RT5575_PROMPT_VOL, 17, 1, 167, 0, ob_tlv),

sound/soc/codecs/rt5575.c

Quoted 4 lines

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

+};
+
+static const struct snd_soc_dapm_widget rt5575_dapm_widgets[] = {
+     SND_SOC_DAPM_AIF_IN("AIF1RX", "AIF1 Playback", 0,

SND_SOC_NOPM, 0, 0),

sound/soc/codecs/rt5575.c line 114

Quoted 1 line

sound/soc/codecs/rt5575.c new file +1 -0

+     SND_SOC_DAPM_AIF_OUT("AIF1TX", "AIF1 Capture", 0,

SND_SOC_NOPM, 0, 0),

sound/soc/codecs/rt5575.c line 115

Quoted 1 line

sound/soc/codecs/rt5575.c new file +1 -0

+     SND_SOC_DAPM_AIF_IN("AIF2RX", "AIF2 Playback", 0,

SND_SOC_NOPM, 0, 0),

sound/soc/codecs/rt5575.c line 116

Quoted 1 line

sound/soc/codecs/rt5575.c new file +1 -0

+     SND_SOC_DAPM_AIF_OUT("AIF2TX", "AIF2 Capture", 0,

SND_SOC_NOPM, 0, 0),

sound/soc/codecs/rt5575.c line 117

Quoted 1 line

sound/soc/codecs/rt5575.c new file +1 -0

+     SND_SOC_DAPM_AIF_IN("AIF3RX", "AIF3 Playback", 0,

SND_SOC_NOPM, 0, 0),

sound/soc/codecs/rt5575.c line 118

Quoted 1 line

sound/soc/codecs/rt5575.c new file +1 -0

+     SND_SOC_DAPM_AIF_OUT("AIF3TX", "AIF3 Capture", 0,

SND_SOC_NOPM, 0, 0),

sound/soc/codecs/rt5575.c line 119

Quoted 1 line

sound/soc/codecs/rt5575.c new file +1 -0

+     SND_SOC_DAPM_AIF_IN("AIF4RX", "AIF4 Playback", 0,

SND_SOC_NOPM, 0, 0),

sound/soc/codecs/rt5575.c line 120

Quoted 1 line

sound/soc/codecs/rt5575.c new file +1 -0

+     SND_SOC_DAPM_AIF_OUT("AIF4TX", "AIF4 Capture", 0,

SND_SOC_NOPM, 0, 0),

Quoted 12 lines

sound/soc/codecs/rt5575.c new file +12 -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" },

sound/soc/codecs/rt5575.c

Quoted 10 lines

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

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

sound/soc/codecs/rt5575.c line 143

Quoted 1 line

sound/soc/codecs/rt5575.c new file +1 -0

+     regmap_read(rt5575->regmap, RT5575_EFUSE_DATA_3,

&priv_id_high);

Quoted 5 lines

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

+     regmap_write(rt5575->regmap, RT5575_EFUSE_PID, 0);
+
+     return ((long long)priv_id_high << 32) | (long long)priv_id_low;
+}
+

sound/soc/codecs/rt5575.c

Quoted 10 lines

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

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

sound/soc/codecs/rt5575.c

Quoted 8 lines

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

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

sound/soc/codecs/rt5575.c line 167

Quoted 1 line

sound/soc/codecs/rt5575.c new file +1 -0

+             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()!

Quoted 11 lines

-----Original Message-----
From: Cezary Rojewski <[email protected]>
Sent: Tuesday, December 9, 2025 4:06 AM
To: Oder Chiou <[email protected]>
Cc: [email protected]; [email protected]; [email protected]; [email protected]; Flove(HsinFu) <[email protected]>; Shuming [范 書銘] <[email protected]>; Jack Yu <[email protected]>; Derek [方德 義] <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH v8 1/2] ASoC: rt5575: Add the codec driver for the ALC5575

Quoted 7 lines

Review

Quoted 2 lines

(quoted diff) +2 -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;
	}

	...
}

Quoted 70 lines

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

sound/soc/codecs/rt5575.c line 57

Quoted 3 lines

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

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

Quoted 10 lines

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

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

sound/soc/codecs/rt5575.c

Quoted 10 lines

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

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

sound/soc/codecs/rt5575.c

Quoted 3 lines

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

+                     release_firmware(firmware);
+             } else {
+                     dev_err(&i2c->dev, "Request firmware failure:

%d\n", ret);

sound/soc/codecs/rt5575.c

Quoted 9 lines

sound/soc/codecs/rt5575.c new file +9 -0

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

Quoted 5 lines

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

+
+     if (ret)
+             dev_err(&i2c->dev, "Run firmware failure: %d\n", ret);
+}
+#endif

Quoted 32 lines

sound/soc/codecs/rt5575.c

Quoted 3 lines

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

+static int rt5575_probe(struct snd_soc_component *component)
+{
+     struct rt5575_priv *rt5575 =

snd_soc_component_get_drvdata(component);

sound/soc/codecs/rt5575.c

Quoted 8 lines

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

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

Quoted 1 line

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

Quoted 2 lines

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.

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 <[email protected]>

---
 .../bindings/sound/realtek,rt5575.yaml        | 44 +++++++++++++++++++
 1 file changed, 44 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/realtek,rt5575.yaml

Documentation/devicetree/bindings/sound/realtek,rt5575.yaml new file +14 -0

new file mode 100644
index 000000000000..83ccc79e6769
@@ -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 <[email protected]>
+
+description:
+  The device supports both I2C and SPI. I2C is mandatory, while SPI is
+  optional depending on the hardware configuration.

Documentation/devicetree/bindings/sound/realtek,rt5575.yaml line 28

Quoted 5 lines

Documentation/devicetree/bindings/sound/realtek,rt5575.yaml new file +4 -0

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

-----Original Message-----
From: Krzysztof Kozlowski <[email protected]>
Sent: Friday, December 5, 2025 4:32 PM
To: Oder Chiou <[email protected]>
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; Flove(HsinFu) <[email protected]>; Shuming [范 書銘] <[email protected]>; Jack Yu <[email protected]>; Derek [方德 義] <[email protected]>
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.

Quoted 9 lines · Krzysztof Kozlowski

Documentation/devicetree/bindings/sound/realtek,rt5575.yaml line 28

Quoted 5 lines

Documentation/devicetree/bindings/sound/realtek,rt5575.yaml new file +4 -0

+
+  reg:
+    maxItems: 1
+

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

Quoted 7 lines

Please post complete bindings for "impressive" device.

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

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

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

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

-----Original Message-----
From: Krzysztof Kozlowski <[email protected]>
Sent: Friday, December 5, 2025 4:31 PM
To: Oder Chiou <[email protected]>
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; Flove(HsinFu) <[email protected]>; Shuming [范 書銘] <[email protected]>; Jack Yu <[email protected]>; Derek [方德 義] <[email protected]>
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.

Quoted 7 lines · Oder Chiou

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

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.

Quoted 4 lines

Signed-off-by Oder Chiou <[email protected]>

---
 .../bindings/sound/realtek,rt5575.yaml        | 44

+++++++++++++++++++

Quoted 2 lines

 1 file changed, 44 insertions(+)
 create mode 100644

Documentation/devicetree/bindings/sound/realtek,rt5575.yaml

Review

Quoted 2 lines

(quoted diff) +0 -0

b/Documentation/devicetree/bindings/sound/realtek,rt5575.yaml

Quoted 25 lines

new file mode 100644
index 000000000000..83ccc79e6769

Documentation/devicetree/bindings/sound/realtek,rt5575.yaml new file +20 -0

@@ -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 <[email protected]>
+
+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:

Documentation/devicetree/bindings/sound/realtek,rt5575.yaml line 24

Quoted 4 lines

Documentation/devicetree/bindings/sound/realtek,rt5575.yaml new file +4 -0

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

Quoted 6 lines

Quoted 5 lines

Documentation/devicetree/bindings/sound/realtek,rt5575.yaml new file +5 -0

+properties:
+  compatible:
+    enum:
+      - realtek,rt5575
+      - realtek,rt5575-with-spi

Quoted 3 lines

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

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

Quoted 6 lines · Oder Chiou

Quoted 6 lines

Quoted 5 lines

Documentation/devicetree/bindings/sound/realtek,rt5575.yaml new file +5 -0

+properties:
+  compatible:
+    enum:
+      - realtek,rt5575
+      - realtek,rt5575-with-spi

Quoted 9 lines

Quoted 3 lines

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

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

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