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 | 34 +++++++++++++++++++++++++++++++---
1 file changed, 31 insertions(+), 3 deletions(-)
diff --git a/sound/soc/codecs/cs4271.c b/sound/soc/codecs/cs4271.c
index ff9c6628224c..481a2c20b7cf 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[] = {
@@ -567,22 +569,36 @@ 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;
+ }
+
+ /*
+ * Be sure to have the clock and power-supplies stable before releasing
+ * the reset.
+ */
+ fsleep(1000);
+
/* Reset codec */
cs4271_reset(component);
ret = regcache_sync(cs4271->regmap);
if (ret < 0)
- return ret;
+ goto err_force_reset;
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_force_reset;
+
ret = regmap_update_bits(cs4271->regmap, CS4271_MODE2,
CS4271_MODE2_PDN, 0);
if (ret < 0)
- return ret;
+ goto err_force_reset;
+
/* Power-up sequence requires 85 uS */
udelay(85);
@@ -592,6 +608,13 @@ static int cs4271_component_probe(struct snd_soc_component *component)
CS4271_MODE2_MUTECAEQUB);
return 0;
+
+err_force_reset:
+ gpiod_set_value(cs4271->reset, 1);
+ clk_disable_unprepare(cs4271->clk);
+err_disable_regulators:
+ regulator_bulk_disable(ARRAY_SIZE(cs4271->supplies), cs4271->supplies);
+ return ret;
}
static void cs4271_component_remove(struct snd_soc_component *component)
@@ -603,6 +626,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 = {
@@ -637,6 +661,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.0On Thu, Oct 16, 2025 at 03:03:39PM +0200, Herve Codina wrote:
> + ret = clk_prepare_enable(cs4271->clk);
> + if (ret) {
> + dev_err(component->dev, "Failed to enable clk: %d\n", ret);
> + goto err_disable_regulators;
> + }
> +
> + /*
> + * Be sure to have the clock and power-supplies stable before releasing
> + * the reset.
> + */
> + fsleep(1000);
The regulator and clock drivers should be doing that. Any sleep here
should come from the device's own requirements for waiting after they
become available.
Hello Herve,
On Thu, 2025-10-16 at 15:03 +0200, 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.
what about suspend/resume, would it make sense to disable/enable the clock
there?
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> ---
> sound/soc/codecs/cs4271.c | 34 +++++++++++++++++++++++++++++++---
> 1 file changed, 31 insertions(+), 3 deletions(-)
>
> diff --git a/sound/soc/codecs/cs4271.c b/sound/soc/codecs/cs4271.c
> index ff9c6628224c..481a2c20b7cf 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[] = {
> @@ -567,22 +569,36 @@ 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;
> + }
> +
> + /*
> + * Be sure to have the clock and power-supplies stable before releasing
> + * the reset.
> + */
Would "if (cs4271->clk)" make sense for the fsleep() call?
> + fsleep(1000);
> +
> /* Reset codec */
> cs4271_reset(component);
>
> ret = regcache_sync(cs4271->regmap);
> if (ret < 0)
> - return ret;
> + goto err_force_reset;
>
> 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_force_reset;
> +
> ret = regmap_update_bits(cs4271->regmap, CS4271_MODE2,
> CS4271_MODE2_PDN, 0);
> if (ret < 0)
> - return ret;
> + goto err_force_reset;
> +
> /* Power-up sequence requires 85 uS */
> udelay(85);
>
> @@ -592,6 +608,13 @@ static int cs4271_component_probe(struct snd_soc_component *component)
> CS4271_MODE2_MUTECAEQUB);
>
> return 0;
> +
> +err_force_reset:
> + gpiod_set_value(cs4271->reset, 1);
Is the reset line assertion really required here?
> + clk_disable_unprepare(cs4271->clk);
> +err_disable_regulators:
> + regulator_bulk_disable(ARRAY_SIZE(cs4271->supplies), cs4271->supplies);
Would it make sense to fix the error path regarding regulators handling
in a separate patch (especially if you decide to extend suspend/resume)?
> + return ret;
> }
>
> static void cs4271_component_remove(struct snd_soc_component *component)
> @@ -603,6 +626,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 = {
> @@ -637,6 +661,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];
>
--
Alexander Sverdlin.
© 2016 - 2026 Red Hat, Inc.