[PATCH v2 0/4] Add support for an external Master Clock in the Cirrus CS4271 codec

[PATCH v2 0/4] Add support for an external Master Clock in the Cirrus CS4271 codec

[Herve Codina]

The Cirrus CS4271 codec can have its Master Clock provided by an
external clock when no crystal is used.

This series adds support for this external Master clock.

The first patch in the series is not related to the clock but fixes an
issue related to module loading and MODULE_DEVICE_TABLE() due to a
driver split between i2c part and spi part.

The next patch fixes regulator handling in component_probe() error path.

The last two patches document the Master clock in the binding and
implement this clock handling in the existing driver.

Best regards,
Hervé

Changes v1 -> v2
  v1: https://lore.kernel.org/lkml/20251016130340.1442090-1-herve.codina@bootlin.com/

  - Patch 1:
    Add missing MODULE_DEVICE_TABLE(spi, ...) in spi part.

  - Patch 2 (new in v2)
    Disable regulators in component_probe() error path

  - Patch 3 (2 in v1)
    Add 'Reviewed-by: Krzysztof Kozlowski'

  - Patch 4 (3 in v1):
    Remove fsleep() call.
    Disable/enable the clock at suspend/resume.
    Remove the reset line assertion on errors (not needed).

Herve Codina (4):
  ASoC: cs4271: Fix cs4271 I2C and SPI drivers automatic module loading
  ASoC: cs4271: Disable regulators in component_probe() error path
  ASoC: dt-bindings: cirrus,cs4271: Document mclk clock
  ASoC: cs4271: Add support for the external mclk

 .../bindings/sound/cirrus,cs4271.yaml         | 10 +++++
 sound/soc/codecs/cs4271-i2c.c                 |  6 +++
 sound/soc/codecs/cs4271-spi.c                 | 13 ++++++
 sound/soc/codecs/cs4271.c                     | 42 +++++++++++++------
 sound/soc/codecs/cs4271.h                     |  1 -
 5 files changed, 59 insertions(+), 13 deletions(-)

-- 
2.51.0

[PATCH v2 1/4] 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 | 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);
-- 
2.51.0

[PATCH v2 2/4] ASoC: cs4271: Disable regulators in component_probe() error path

[Herve Codina]

The commit 9a397f473657 ("ASoC: cs4271: add regulator consumer support")
has introduced regulators in the driver.

Regulators are enabled at the beginning of component_probe() but they
are not disabled on errors. This can lead to unbalanced enable/disable.

Fix the error path to disable regulators on errors.

Fixes: 9a397f473657 ("ASoC: cs4271: add regulator consumer support")
Cc: stable@vger.kernel.org
Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 sound/soc/codecs/cs4271.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/sound/soc/codecs/cs4271.c b/sound/soc/codecs/cs4271.c
index ff9c6628224c..a9d333e6c723 100644
--- a/sound/soc/codecs/cs4271.c
+++ b/sound/soc/codecs/cs4271.c
@@ -572,17 +572,17 @@ static int cs4271_component_probe(struct snd_soc_component *component)
 
 	ret = regcache_sync(cs4271->regmap);
 	if (ret < 0)
-		return ret;
+		goto err_disable_regulators;
 
 	ret = regmap_update_bits(cs4271->regmap, CS4271_MODE2,
 				 CS4271_MODE2_PDN | CS4271_MODE2_CPEN,
 				 CS4271_MODE2_PDN | CS4271_MODE2_CPEN);
 	if (ret < 0)
-		return ret;
+		goto err_disable_regulators;
 	ret = regmap_update_bits(cs4271->regmap, CS4271_MODE2,
 				 CS4271_MODE2_PDN, 0);
 	if (ret < 0)
-		return ret;
+		goto err_disable_regulators;
 	/* Power-up sequence requires 85 uS */
 	udelay(85);
 
@@ -592,6 +592,10 @@ static int cs4271_component_probe(struct snd_soc_component *component)
 				   CS4271_MODE2_MUTECAEQUB);
 
 	return 0;
+
+err_disable_regulators:
+	regulator_bulk_disable(ARRAY_SIZE(cs4271->supplies), cs4271->supplies);
+	return ret;
 }
 
 static void cs4271_component_remove(struct snd_soc_component *component)
-- 
2.51.0

[PATCH v2 3/4] ASoC: dt-bindings: cirrus,cs4271: Document mclk clock

[Herve Codina]

The Cirrus CS4271 codec can use an external clock as an input Master
Clock. When no crystal is used, the CS4271 component considers its MCLK
pin as an input pin and expects the external clock connected to provide
the Master Clock.

This clock is not documented in the binding.

Add the missing clock.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 .../devicetree/bindings/sound/cirrus,cs4271.yaml       | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/cirrus,cs4271.yaml b/Documentation/devicetree/bindings/sound/cirrus,cs4271.yaml
index 68fbf5cc208f..d286eb169915 100644
--- a/Documentation/devicetree/bindings/sound/cirrus,cs4271.yaml
+++ b/Documentation/devicetree/bindings/sound/cirrus,cs4271.yaml
@@ -25,6 +25,16 @@ properties:
   reg:
     maxItems: 1
 
+  clocks:
+    items:
+      - description:
+          Master clock connected to the MCLK pin if MCLK is an input (i.e. no
+          crystal used).
+
+  clock-names:
+    items:
+      - const: mclk
+
   spi-cpha: true
 
   spi-cpol: true
-- 
2.51.0

[PATCH v2 4/4] ASoC: cs4271: Add support for the external mclk

[Herve Codina]

The mclk (master clock) of the cs4271 codec can be an input clock.

In this case the connected clock needs to be enabled outside of any
audio stream. Indeed, this clock is needed for i2c communication.

Add support of this clock and enable it before the first i2c transfer.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 sound/soc/codecs/cs4271.c | 29 ++++++++++++++++++++++++++---
 1 file changed, 26 insertions(+), 3 deletions(-)

diff --git a/sound/soc/codecs/cs4271.c b/sound/soc/codecs/cs4271.c
index a9d333e6c723..b8a50b9001e1 100644
--- a/sound/soc/codecs/cs4271.c
+++ b/sound/soc/codecs/cs4271.c
@@ -10,6 +10,7 @@
  * DAPM support not implemented.
  */
 
+#include <linux/clk.h>
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <linux/delay.h>
@@ -163,6 +164,7 @@ struct cs4271_private {
 	/* enable soft reset workaround */
 	bool				enable_soft_reset;
 	struct regulator_bulk_data      supplies[ARRAY_SIZE(supply_names)];
+	struct clk *clk;
 };
 
 static const struct snd_soc_dapm_widget cs4271_dapm_widgets[] = {
@@ -505,6 +507,7 @@ static int cs4271_soc_suspend(struct snd_soc_component *component)
 		return ret;
 
 	regcache_mark_dirty(cs4271->regmap);
+	clk_disable_unprepare(cs4271->clk);
 	regulator_bulk_disable(ARRAY_SIZE(cs4271->supplies), cs4271->supplies);
 
 	return 0;
@@ -522,6 +525,12 @@ static int cs4271_soc_resume(struct snd_soc_component *component)
 		return ret;
 	}
 
+	ret = clk_prepare_enable(cs4271->clk);
+	if (ret) {
+		dev_err(component->dev, "Failed to enable clk: %d\n", ret);
+		return ret;
+	}
+
 	/* Do a proper reset after power up */
 	cs4271_reset(component);
 
@@ -567,22 +576,29 @@ static int cs4271_component_probe(struct snd_soc_component *component)
 		cs4271->enable_soft_reset = cs4271plat->enable_soft_reset;
 	}
 
+	ret = clk_prepare_enable(cs4271->clk);
+	if (ret) {
+		dev_err(component->dev, "Failed to enable clk: %d\n", ret);
+		goto err_disable_regulators;
+	}
+
 	/* Reset codec */
 	cs4271_reset(component);
 
 	ret = regcache_sync(cs4271->regmap);
 	if (ret < 0)
-		goto err_disable_regulators;
+		goto err_disable_clk;
 
 	ret = regmap_update_bits(cs4271->regmap, CS4271_MODE2,
 				 CS4271_MODE2_PDN | CS4271_MODE2_CPEN,
 				 CS4271_MODE2_PDN | CS4271_MODE2_CPEN);
 	if (ret < 0)
-		goto err_disable_regulators;
+		goto err_disable_clk;
 	ret = regmap_update_bits(cs4271->regmap, CS4271_MODE2,
 				 CS4271_MODE2_PDN, 0);
 	if (ret < 0)
-		goto err_disable_regulators;
+		goto err_disable_clk;
+
 	/* Power-up sequence requires 85 uS */
 	udelay(85);
 
@@ -593,6 +609,8 @@ static int cs4271_component_probe(struct snd_soc_component *component)
 
 	return 0;
 
+err_disable_clk:
+	clk_disable_unprepare(cs4271->clk);
 err_disable_regulators:
 	regulator_bulk_disable(ARRAY_SIZE(cs4271->supplies), cs4271->supplies);
 	return ret;
@@ -607,6 +625,7 @@ static void cs4271_component_remove(struct snd_soc_component *component)
 
 	regcache_mark_dirty(cs4271->regmap);
 	regulator_bulk_disable(ARRAY_SIZE(cs4271->supplies), cs4271->supplies);
+	clk_disable_unprepare(cs4271->clk);
 };
 
 static const struct snd_soc_component_driver soc_component_dev_cs4271 = {
@@ -641,6 +660,10 @@ static int cs4271_common_probe(struct device *dev,
 				     "error retrieving RESET GPIO\n");
 	gpiod_set_consumer_name(cs4271->reset, "CS4271 Reset");
 
+	cs4271->clk = devm_clk_get_optional(dev, "mclk");
+	if (IS_ERR(cs4271->clk))
+		return dev_err_probe(dev, PTR_ERR(cs4271->clk), "Failed to get mclk\n");
+
 	for (i = 0; i < ARRAY_SIZE(supply_names); i++)
 		cs4271->supplies[i].supply = supply_names[i];
 
-- 
2.51.0

Re: [PATCH v2 2/4] ASoC: cs4271: Disable regulators in component_probe() error path

[Alexander Sverdlin]

Hi!

On Wed, 2025-10-29 at 10:39 +0100, Herve Codina wrote:
> The commit 9a397f473657 ("ASoC: cs4271: add regulator consumer support")
> has introduced regulators in the driver.
> 
> Regulators are enabled at the beginning of component_probe() but they
> are not disabled on errors. This can lead to unbalanced enable/disable.
> 
> Fix the error path to disable regulators on errors.
> 
> Fixes: 9a397f473657 ("ASoC: cs4271: add regulator consumer support")
> Cc: stable@vger.kernel.org
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>

Reviewed-by: Alexander Sverdlin <alexander.sverdlin@gmail.com>

-- 
Alexander Sverdlin.

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@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 = {

-- 
Alexander Sverdlin.

Re: [PATCH v2 4/4] ASoC: cs4271: Add support for the external mclk

[Alexander Sverdlin]

Hi!

On Wed, 2025-10-29 at 10:39 +0100, Herve Codina wrote:
> The mclk (master clock) of the cs4271 codec can be an input clock.
> 
> In this case the connected clock needs to be enabled outside of any
> audio stream. Indeed, this clock is needed for i2c communication.
> 
> Add support of this clock and enable it before the first i2c transfer.
> 
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> ---
>  sound/soc/codecs/cs4271.c | 29 ++++++++++++++++++++++++++---
>  1 file changed, 26 insertions(+), 3 deletions(-)

Reviewed-by: Alexander Sverdlin <alexander.sverdlin@gmail.com>

-- 
Alexander Sverdlin.

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

[Herve Codina]

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é

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

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 393 bytes --]

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Alexander Sverdlin]

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.

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

[Alexander Sverdlin]

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

-- 
Alexander Sverdlin.

Re: (subset) [PATCH v2 0/4] Add support for an external Master Clock in the Cirrus CS4271 codec

[Mark Brown]

On Wed, 29 Oct 2025 10:39:16 +0100, Herve Codina wrote:
> The Cirrus CS4271 codec can have its Master Clock provided by an
> external clock when no crystal is used.
> 
> This series adds support for this external Master clock.
> 
> The first patch in the series is not related to the clock but fixes an
> issue related to module loading and MODULE_DEVICE_TABLE() due to a
> driver split between i2c part and spi part.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[2/4] ASoC: cs4271: Disable regulators in component_probe() error path
      commit: 1e5351ba60f5355809f30c61bbd27e97611d2be9
[3/4] ASoC: dt-bindings: cirrus,cs4271: Document mclk clock
      commit: 3cd523ba270665861647304aa30500f238ebf26e
[4/4] ASoC: cs4271: Add support for the external mclk
      commit: cf6bf51b53252284bafc7377a4d8dbf10f048b4d

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark