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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <60fbaaef249e6f5af602fe4087eab31cd70905de.camel@gmail.com>
Date: Thu, 16 Oct 2025 20:40:34 +0200
From: Alexander Sverdlin <alexander.sverdlin@...il.com>
To: Herve Codina <herve.codina@...tlin.com>, David Rhodes	
 <david.rhodes@...rus.com>, Richard Fitzgerald <rf@...nsource.cirrus.com>, 
 Liam Girdwood <lgirdwood@...il.com>, Mark Brown <broonie@...nel.org>, Rob
 Herring <robh@...nel.org>,  Krzysztof Kozlowski	 <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>, Jaroslav Kysela	 <perex@...ex.cz>,
 Takashi Iwai <tiwai@...e.com>, Nikita Shubin	 <nikita.shubin@...uefel.me>,
 Axel Lin <axel.lin@...ics.com>, Brian Austin	 <brian.austin@...rus.com>
Cc: linux-sound@...r.kernel.org, patches@...nsource.cirrus.com, 
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, Thomas Petazzoni
	 <thomas.petazzoni@...tlin.com>, stable@...r.kernel.org
Subject: Re: [PATCH 1/3] ASoC: cs4271: Fix cs4271 I2C and SPI drivers
 automatic module loading

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@...r.kernel.org
> Signed-off-by: Herve Codina <herve.codina@...tlin.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.