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

[Alexander Sverdlin]

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@xxxxxxxxxxxxxxx
> Signed-off-by: Herve Codina <herve.codina@xxxxxxxxxxx>
> ---
>  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 = {

-- 
Alexander Sverdlin.