[PATCH 1/3] ASoC: cs4271: Fix cs4271 I2C and SPI drivers automatic module loading

[PATCH 1/3] ASoC: cs4271: Fix cs4271 I2C and SPI drivers automatic module loading

[Herve Codina]

In commit c973b8a7dc50 ("ASoC: cs4271: Split SPI and I2C code into
different modules") the driver was slit into a core, an SPI and an I2C
part.

However, the MODULE_DEVICE_TABLE(of, cs4271_dt_ids) was in the core part
and so, module loading based on module.alias (based on DT compatible
string matching) loads the core part but not the SPI or I2C parts.

In order to have the I2C or the SPI module loaded automatically, move
the MODULE_DEVICE_TABLE(of, ...) the core to I2C and SPI parts.
Also move cs4271_dt_ids itself from the core part to I2C and SPI parts
as both the call to MODULE_DEVICE_TABLE(of, ...) and the cs4271_dt_ids
table itself need to be in the same file.

Fixes: c973b8a7dc50 ("ASoC: cs4271: Split SPI and I2C code into different modules")
Cc: stable@vger.kernel.org
Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 sound/soc/codecs/cs4271-i2c.c | 6 ++++++
 sound/soc/codecs/cs4271-spi.c | 6 ++++++
 sound/soc/codecs/cs4271.c     | 9 ---------
 sound/soc/codecs/cs4271.h     | 1 -
 4 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/sound/soc/codecs/cs4271-i2c.c b/sound/soc/codecs/cs4271-i2c.c
index 1d210b969173..cefb8733fc61 100644
--- a/sound/soc/codecs/cs4271-i2c.c
+++ b/sound/soc/codecs/cs4271-i2c.c
@@ -28,6 +28,12 @@ static const struct i2c_device_id cs4271_i2c_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, cs4271_i2c_id);
 
+static const struct of_device_id cs4271_dt_ids[] = {
+	{ .compatible = "cirrus,cs4271", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, cs4271_dt_ids);
+
 static struct i2c_driver cs4271_i2c_driver = {
 	.driver = {
 		.name = "cs4271",
diff --git a/sound/soc/codecs/cs4271-spi.c b/sound/soc/codecs/cs4271-spi.c
index 4feb80436bd9..82abc654293c 100644
--- a/sound/soc/codecs/cs4271-spi.c
+++ b/sound/soc/codecs/cs4271-spi.c
@@ -23,6 +23,12 @@ static int cs4271_spi_probe(struct spi_device *spi)
 	return cs4271_probe(&spi->dev, devm_regmap_init_spi(spi, &config));
 }
 
+static const struct of_device_id cs4271_dt_ids[] = {
+	{ .compatible = "cirrus,cs4271", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, cs4271_dt_ids);
+
 static struct spi_driver cs4271_spi_driver = {
 	.driver = {
 		.name	= "cs4271",
diff --git a/sound/soc/codecs/cs4271.c b/sound/soc/codecs/cs4271.c
index 6a3cca3d26c7..ff9c6628224c 100644
--- a/sound/soc/codecs/cs4271.c
+++ b/sound/soc/codecs/cs4271.c
@@ -543,15 +543,6 @@ static int cs4271_soc_resume(struct snd_soc_component *component)
 #define cs4271_soc_resume	NULL
 #endif /* CONFIG_PM */
 
-#ifdef CONFIG_OF
-const struct of_device_id cs4271_dt_ids[] = {
-	{ .compatible = "cirrus,cs4271", },
-	{ }
-};
-MODULE_DEVICE_TABLE(of, cs4271_dt_ids);
-EXPORT_SYMBOL_GPL(cs4271_dt_ids);
-#endif
-
 static int cs4271_component_probe(struct snd_soc_component *component)
 {
 	struct cs4271_private *cs4271 = snd_soc_component_get_drvdata(component);
diff --git a/sound/soc/codecs/cs4271.h b/sound/soc/codecs/cs4271.h
index 290283a9149e..4965ce085875 100644
--- a/sound/soc/codecs/cs4271.h
+++ b/sound/soc/codecs/cs4271.h
@@ -4,7 +4,6 @@
 
 #include <linux/regmap.h>
 
-extern const struct of_device_id cs4271_dt_ids[];
 extern const struct regmap_config cs4271_regmap_config;
 
 int cs4271_probe(struct device *dev, struct regmap *regmap);
-- 
2.51.0

Re: [PATCH 1/3] ASoC: cs4271: Fix cs4271 I2C and SPI drivers automatic module loading

[Alexander Sverdlin]

Hello Herve,

On Thu, 2025-10-16 at 15:03 +0200, Herve Codina wrote:
> In commit c973b8a7dc50 ("ASoC: cs4271: Split SPI and I2C code into
> different modules") the driver was slit into a core, an SPI and an I2C
> part.
> 
> However, the MODULE_DEVICE_TABLE(of, cs4271_dt_ids) was in the core part
> and so, module loading based on module.alias (based on DT compatible
> string matching) loads the core part but not the SPI or I2C parts.
> 
> In order to have the I2C or the SPI module loaded automatically, move
> the MODULE_DEVICE_TABLE(of, ...) the core to I2C and SPI parts.
> Also move cs4271_dt_ids itself from the core part to I2C and SPI parts
> as both the call to MODULE_DEVICE_TABLE(of, ...) and the cs4271_dt_ids
> table itself need to be in the same file.

I'm a bit confused by this change.
What do you have in SYSFS "uevent" entry for the real device?

If you consider spi_uevent() and i2c_device_uevent(), "MODALIAS=" in the
"uevent" should be prefixed with either "spi:" or "i2c:".
And this isn't what you adress in your patch.

You provide [identical] "of:" prefixed modalias to two different modules
(not sure, how this should work), but cs4271 is not an MMIO device,
so it should not generate an "of:" prefixed uevent.

Could you please show the relevant DT snippet for the affected HW?

> Fixes: c973b8a7dc50 ("ASoC: cs4271: Split SPI and I2C code into different modules")
> Cc: stable@vger.kernel.org
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> ---
>  sound/soc/codecs/cs4271-i2c.c | 6 ++++++
>  sound/soc/codecs/cs4271-spi.c | 6 ++++++
>  sound/soc/codecs/cs4271.c     | 9 ---------
>  sound/soc/codecs/cs4271.h     | 1 -
>  4 files changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/sound/soc/codecs/cs4271-i2c.c b/sound/soc/codecs/cs4271-i2c.c
> index 1d210b969173..cefb8733fc61 100644
> --- a/sound/soc/codecs/cs4271-i2c.c
> +++ b/sound/soc/codecs/cs4271-i2c.c
> @@ -28,6 +28,12 @@ static const struct i2c_device_id cs4271_i2c_id[] = {
>  };
>  MODULE_DEVICE_TABLE(i2c, cs4271_i2c_id);
>  
> +static const struct of_device_id cs4271_dt_ids[] = {
> +	{ .compatible = "cirrus,cs4271", },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, cs4271_dt_ids);
> +
>  static struct i2c_driver cs4271_i2c_driver = {
>  	.driver = {
>  		.name = "cs4271",
> diff --git a/sound/soc/codecs/cs4271-spi.c b/sound/soc/codecs/cs4271-spi.c
> index 4feb80436bd9..82abc654293c 100644
> --- a/sound/soc/codecs/cs4271-spi.c
> +++ b/sound/soc/codecs/cs4271-spi.c
> @@ -23,6 +23,12 @@ static int cs4271_spi_probe(struct spi_device *spi)
>  	return cs4271_probe(&spi->dev, devm_regmap_init_spi(spi, &config));
>  }
>  
> +static const struct of_device_id cs4271_dt_ids[] = {
> +	{ .compatible = "cirrus,cs4271", },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, cs4271_dt_ids);
> +
>  static struct spi_driver cs4271_spi_driver = {
>  	.driver = {
>  		.name	= "cs4271",
> diff --git a/sound/soc/codecs/cs4271.c b/sound/soc/codecs/cs4271.c
> index 6a3cca3d26c7..ff9c6628224c 100644
> --- a/sound/soc/codecs/cs4271.c
> +++ b/sound/soc/codecs/cs4271.c
> @@ -543,15 +543,6 @@ static int cs4271_soc_resume(struct snd_soc_component *component)
>  #define cs4271_soc_resume	NULL
>  #endif /* CONFIG_PM */
>  
> -#ifdef CONFIG_OF
> -const struct of_device_id cs4271_dt_ids[] = {
> -	{ .compatible = "cirrus,cs4271", },
> -	{ }
> -};
> -MODULE_DEVICE_TABLE(of, cs4271_dt_ids);
> -EXPORT_SYMBOL_GPL(cs4271_dt_ids);
> -#endif
> -
>  static int cs4271_component_probe(struct snd_soc_component *component)
>  {
>  	struct cs4271_private *cs4271 = snd_soc_component_get_drvdata(component);
> diff --git a/sound/soc/codecs/cs4271.h b/sound/soc/codecs/cs4271.h
> index 290283a9149e..4965ce085875 100644
> --- a/sound/soc/codecs/cs4271.h
> +++ b/sound/soc/codecs/cs4271.h
> @@ -4,7 +4,6 @@
>  
>  #include <linux/regmap.h>
>  
> -extern const struct of_device_id cs4271_dt_ids[];
>  extern const struct regmap_config cs4271_regmap_config;
>  
>  int cs4271_probe(struct device *dev, struct regmap *regmap);

-- 
Alexander Sverdlin.

Re: [PATCH 1/3] ASoC: cs4271: Fix cs4271 I2C and SPI drivers automatic module loading

[Herve Codina]

Hi Alexander,

On Thu, 16 Oct 2025 20:40:34 +0200
Alexander Sverdlin <alexander.sverdlin@gmail.com> wrote:

...

> > In order to have the I2C or the SPI module loaded automatically, move
> > the MODULE_DEVICE_TABLE(of, ...) the core to I2C and SPI parts.
> > Also move cs4271_dt_ids itself from the core part to I2C and SPI parts
> > as both the call to MODULE_DEVICE_TABLE(of, ...) and the cs4271_dt_ids
> > table itself need to be in the same file.  
> 
> I'm a bit confused by this change.
> What do you have in SYSFS "uevent" entry for the real device?

Here is my uevent content:
--- 8<---
# cat /sys/bus/i2c/devices/3-0010/uevent 
DRIVER=cs4271
OF_NAME=cs4271
OF_FULLNAME=/i2c@ff130000/cs4271@10
OF_COMPATIBLE_0=cirrus,cs4271
OF_COMPATIBLE_N=1
MODALIAS=of:Ncs4271T(null)Ccirrus,cs4271
# 
--- 8< ---

> 
> If you consider spi_uevent() and i2c_device_uevent(), "MODALIAS=" in the
> "uevent" should be prefixed with either "spi:" or "i2c:".
> And this isn't what you adress in your patch.
> 
> You provide [identical] "of:" prefixed modalias to two different modules
> (not sure, how this should work), but cs4271 is not an MMIO device,
> so it should not generate an "of:" prefixed uevent.
> 
> Could you please show the relevant DT snippet for the affected HW?

And this is the related DT part:
--- 8< ---
&i2c3 {
	status = "okay";

	cs4271@10 {
		compatible = "cirrus,cs4271";
		reg = <0x10>;
		clocks = <&cru SCLK_I2S_8CH_OUT>;
		clock-names = "mclk";

		...
	};
};
--- 8< ---

i2c3 is the following node:
https://elixir.bootlin.com/linux/v6.17.1/source/arch/arm64/boot/dts/rockchip/rk3399-base.dtsi#L732

About the related module, I have the following:
--- 8< ---
# modinfo snd_soc_cs4271_i2c
filename:       /lib/modules/6.18.0-rc1xxxx-00050-g4fa36970abe5-dirty/kernel/sound/soc/codecs/snd-soc-cs4271-i2c.ko
license:        GPL
author:         Alexander Sverdlin <subaparts@yandex.ru>
description:    ASoC CS4271 I2C Driver
alias:          i2c:cs4271
alias:          of:N*T*Ccirrus,cs4271C*
alias:          of:N*T*Ccirrus,cs4271
depends:        snd-soc-cs4271
intree:         Y
name:           snd_soc_cs4271_i2c
vermagic:       6.18.0-rc1xxxx-00050-g4fa36970abe5-dirty SMP preempt mod_unload aarch64
# 
--- 8< ---

Best regards,
Hervé