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 | 13 +++++++++++++ sound/soc/codecs/cs4271.c | 9 --------- sound/soc/codecs/cs4271.h | 1 - 4 files changed, 19 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..28dd7b8f3507 100644 --- a/sound/soc/codecs/cs4271-spi.c +++ b/sound/soc/codecs/cs4271-spi.c @@ -23,11 +23,24 @@ static int cs4271_spi_probe(struct spi_device *spi) return cs4271_probe(&spi->dev, devm_regmap_init_spi(spi, &config)); }
+static const struct spi_device_id cs4271_id_spi[] = { + { "cs4271", 0 }, + {} +}; +MODULE_DEVICE_TABLE(spi, cs4271_id_spi); + +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", .of_match_table = of_match_ptr(cs4271_dt_ids), }, + .id_table = cs4271_id_spi, .probe = cs4271_spi_probe, }; module_spi_driver(cs4271_spi_driver); 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);
Hi Herve,
On Wed, 2025-10-29 at 10:39 +0100, 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.
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 | 13 +++++++++++++ sound/soc/codecs/cs4271.c | 9 --------- sound/soc/codecs/cs4271.h | 1 - 4 files changed, 19 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..28dd7b8f3507 100644 --- a/sound/soc/codecs/cs4271-spi.c +++ b/sound/soc/codecs/cs4271-spi.c @@ -23,11 +23,24 @@ static int cs4271_spi_probe(struct spi_device *spi) return cs4271_probe(&spi->dev, devm_regmap_init_spi(spi, &config)); } +static const struct spi_device_id cs4271_id_spi[] = {
- { "cs4271", 0 },
- {}
+}; +MODULE_DEVICE_TABLE(spi, cs4271_id_spi);
+static const struct of_device_id cs4271_dt_ids[] = {
- { .compatible = "cirrus,cs4271", },
- { }
+}; +MODULE_DEVICE_TABLE(of, cs4271_dt_ids);
So currently SPI core doesn't generate "of:" prefixed uevents, therefore this currently doesn't help? However, imagine, you'd have both backends enabled as modules, -spi and -i2c. udev/modprobe currently load just one module it finds first. What is the guarantee that the loaded module for the "of:" prefixed I2C uevent would be the -i2c module?
static struct spi_driver cs4271_spi_driver = {
Hi Alexander,
On Wed, 29 Oct 2025 12:20:27 +0100 Alexander Sverdlin alexander.sverdlin@gmail.com wrote:
...
diff --git a/sound/soc/codecs/cs4271-spi.c b/sound/soc/codecs/cs4271-spi.c index 4feb80436bd9..28dd7b8f3507 100644 --- a/sound/soc/codecs/cs4271-spi.c +++ b/sound/soc/codecs/cs4271-spi.c @@ -23,11 +23,24 @@ static int cs4271_spi_probe(struct spi_device *spi) return cs4271_probe(&spi->dev, devm_regmap_init_spi(spi, &config)); } +static const struct spi_device_id cs4271_id_spi[] = {
- { "cs4271", 0 },
- {}
+}; +MODULE_DEVICE_TABLE(spi, cs4271_id_spi);
+static const struct of_device_id cs4271_dt_ids[] = {
- { .compatible = "cirrus,cs4271", },
- { }
+}; +MODULE_DEVICE_TABLE(of, cs4271_dt_ids);
So currently SPI core doesn't generate "of:" prefixed uevents, therefore this currently doesn't help? However, imagine, you'd have both backends enabled as modules, -spi and -i2c. udev/modprobe currently load just one module it finds first. What is the guarantee that the loaded module for the "of:" prefixed I2C uevent would be the -i2c module?
I hesitate to fully remove cs4271_dt_ids in the SPI part.
I understood having it could lead to issues if both SPI and I2C parts are compiled as modules but this is the pattern used in quite a lot of drivers.
Maybe this could be handle globally out of this series instead of introducing a specific pattern in this series.
But well, if you and Mark are ok to fully remove the cs4271_dt_ids from the SPI part and so unset the of_match_table from the cs4271_spi_driver, I can do the modification.
Let me know if I should send a new iteration with cs4271_dt_ids fully removed from the SPI part.
Also, last point, I don't have any cs4271 connected to a SPI bus. I use only the I2C version and will not be able to check for correct modifications on the SPI part.
Best regards, Hervé
On Thu, Oct 30, 2025 at 02:43:19PM +0100, Herve Codina wrote:
I hesitate to fully remove cs4271_dt_ids in the SPI part.
...
Maybe this could be handle globally out of this series instead of introducing a specific pattern in this series.
Yes, it feels sensible to be consistant and if there's practical issues fix everyone rather than have one driver that works randomly differently.
Hi Herve,
On Thu, 2025-10-30 at 14:43 +0100, Herve Codina wrote:
--- a/sound/soc/codecs/cs4271-spi.c +++ b/sound/soc/codecs/cs4271-spi.c @@ -23,11 +23,24 @@ static int cs4271_spi_probe(struct spi_device *spi) return cs4271_probe(&spi->dev, devm_regmap_init_spi(spi, &config)); } +static const struct spi_device_id cs4271_id_spi[] = {
- { "cs4271", 0 },
- {}
+}; +MODULE_DEVICE_TABLE(spi, cs4271_id_spi);
+static const struct of_device_id cs4271_dt_ids[] = {
- { .compatible = "cirrus,cs4271", },
- { }
+}; +MODULE_DEVICE_TABLE(of, cs4271_dt_ids);
So currently SPI core doesn't generate "of:" prefixed uevents, therefore this currently doesn't help? However, imagine, you'd have both backends enabled as modules, -spi and -i2c. udev/modprobe currently load just one module it finds first. What is the guarantee that the loaded module for the "of:" prefixed I2C uevent would be the -i2c module?
I hesitate to fully remove cs4271_dt_ids in the SPI part.
I understood having it could lead to issues if both SPI and I2C parts are compiled as modules but this is the pattern used in quite a lot of drivers.
Maybe this could be handle globally out of this series instead of introducing a specific pattern in this series.
But well, if you and Mark are ok to fully remove the cs4271_dt_ids from the SPI part and so unset the of_match_table from the cs4271_spi_driver, I can do the modification.
Let me know if I should send a new iteration with cs4271_dt_ids fully removed from the SPI part.
Also, last point, I don't have any cs4271 connected to a SPI bus. I use only the I2C version and will not be able to check for correct modifications on the SPI part.
I'd propose to drop SPI modifications in this case, because by doing this you actually introduce yet another problem for the I2C case you are interested in (namely if you'd enable both modules).
Hi Herve,
On Thu, 2025-10-30 at 14:54 +0100, Alexander Sverdlin wrote:
--- a/sound/soc/codecs/cs4271-spi.c +++ b/sound/soc/codecs/cs4271-spi.c @@ -23,11 +23,24 @@ static int cs4271_spi_probe(struct spi_device *spi) return cs4271_probe(&spi->dev, devm_regmap_init_spi(spi, &config)); } +static const struct spi_device_id cs4271_id_spi[] = {
- { "cs4271", 0 },
- {}
+}; +MODULE_DEVICE_TABLE(spi, cs4271_id_spi);
+static const struct of_device_id cs4271_dt_ids[] = {
- { .compatible = "cirrus,cs4271", },
- { }
+}; +MODULE_DEVICE_TABLE(of, cs4271_dt_ids);
So currently SPI core doesn't generate "of:" prefixed uevents, therefore this currently doesn't help? However, imagine, you'd have both backends enabled as modules, -spi and -i2c. udev/modprobe currently load just one module it finds first. What is the guarantee that the loaded module for the "of:" prefixed I2C uevent would be the -i2c module?
I hesitate to fully remove cs4271_dt_ids in the SPI part.
I understood having it could lead to issues if both SPI and I2C parts are compiled as modules but this is the pattern used in quite a lot of drivers.
Maybe this could be handle globally out of this series instead of introducing a specific pattern in this series.
But well, if you and Mark are ok to fully remove the cs4271_dt_ids from the SPI part and so unset the of_match_table from the cs4271_spi_driver, I can do the modification.
Let me know if I should send a new iteration with cs4271_dt_ids fully removed from the SPI part.
Also, last point, I don't have any cs4271 connected to a SPI bus. I use only the I2C version and will not be able to check for correct modifications on the SPI part.
I'd propose to drop SPI modifications in this case, because by doing this you actually introduce yet another problem for the I2C case you are interested in (namely if you'd enable both modules).
sorry again for the confusion, I meant only to drop "MODULE_DEVICE_TABLE(of, " from SPI...
But seems that Mark actually has different opinion... Indeed
$ grep -F "MODULE_DEVICE_TABLE(of, " *spi*.c | wc -l
currently reports "17" out of 23 SPI-capable CODECs. I just don't see how you are helping the I2C use case by doing this...
linux-stable-mirror@lists.linaro.org
-
Alexander Sverdlin -
Herve Codina -
Mark Brown