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

[Alexander Sverdlin <alexander.sverdlin@gmail.com>]

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

-- 
Alexander Sverdlin.