[PATCH v1 0/2] Add common wait_for_boot and reset_and_boot apis for cs35l41 drivers

[PATCH v1 0/2] Add common wait_for_boot and reset_and_boot apis for cs35l41 drivers

[Stefan Binding]

Both HDA and ASoC CS35L41 drivers use a similar meachnism for waiting
for the chip to boot: regmap_read_poll_timeout().
However, this method can break if the macro returns early if a
regmap_read() call fails.
Replace this macro with a loop which does not break on regmap_read()
failure.

In addition, ensure the software reset prior to boot does not fail,
and ensure it is additionally run in the ASoC driver.

Stefan Binding (2):
  ALSA: hda: cs35l41: Add common wait_for_boot api and reset_and_boot
    api
  ASoC: cs35l41: Use common wait_for_boot and reset_and_boot apis

 include/sound/cs35l41.h                    |  2 +
 sound/hda/codecs/side-codecs/cs35l41_hda.c | 33 +----------
 sound/soc/codecs/cs35l41-lib.c             | 67 ++++++++++++++++++++++
 sound/soc/codecs/cs35l41.c                 | 18 +-----
 4 files changed, 74 insertions(+), 46 deletions(-)

-- 
2.43.0

[PATCH v1 1/2] ALSA: hda: cs35l41: Add common wait_for_boot api and reset_and_boot api

[Stefan Binding]

Both the ASoC CS35L41 and HDA CS35L41 drivers currently use
regmap_read_poll_timeout() calls to wait for the chip to boot.
However, this macro will exit immediately if one of the regmap_read()
calls fail. This could be cause by the hardware not being ready yet
after a reset, which then breaks the loop early and leads to probe
and resume calls failing.

This patch replaces the existing code in the HDA driver with a
function with a loop which does not break on regmap_read() failure.
Since both drivers need to do this, make a common function in
cs35l41-lib.c to be called from both drivers.
This patch only updates the HDA driver, a separate patch will update
the ASoC driver.

In addition, before the call to wait to boot, the HDA driver also
does a software reset to ensure the device is in a known state before
probe or resume (for example, if there is no reset gpio). Since this
write could fail for the same reason, add a common api to do this
with a retry mechanism.

Signed-off-by: Stefan Binding <sbinding@opensource.cirrus.com>
---
 include/sound/cs35l41.h                    |  2 +
 sound/hda/codecs/side-codecs/cs35l41_hda.c | 33 +----------
 sound/soc/codecs/cs35l41-lib.c             | 67 ++++++++++++++++++++++
 3 files changed, 71 insertions(+), 31 deletions(-)

diff --git a/include/sound/cs35l41.h b/include/sound/cs35l41.h
index 7542cabfa726..b6b338c55ff0 100644
--- a/include/sound/cs35l41.h
+++ b/include/sound/cs35l41.h
@@ -924,5 +924,7 @@ bool cs35l41_safe_reset(struct regmap *regmap, enum cs35l41_boost_type b_type);
 int cs35l41_mdsync_up(struct regmap *regmap);
 int cs35l41_global_enable(struct device *dev, struct regmap *regmap, enum cs35l41_boost_type b_type,
 			  int enable, struct cs_dsp *dsp);
+int cs35l41_wait_boot_done(struct device *dev, struct regmap *regmap);
+int cs35l41_reset_and_boot(struct device *dev, struct regmap *regmap);
 
 #endif /* __CS35L41_H */
diff --git a/sound/hda/codecs/side-codecs/cs35l41_hda.c b/sound/hda/codecs/side-codecs/cs35l41_hda.c
index f879e3ffbb53..23582f23a880 100644
--- a/sound/hda/codecs/side-codecs/cs35l41_hda.c
+++ b/sound/hda/codecs/side-codecs/cs35l41_hda.c
@@ -974,30 +974,6 @@ static int cs35l41_system_suspend(struct device *dev)
 	return ret;
 }
 
-static int cs35l41_wait_boot_done(struct cs35l41_hda *cs35l41)
-{
-	unsigned int int_status;
-	int ret;
-
-	ret = regmap_read_poll_timeout(cs35l41->regmap, CS35L41_IRQ1_STATUS4, int_status,
-				       int_status & CS35L41_OTP_BOOT_DONE, 1000, 100000);
-	if (ret) {
-		dev_err(cs35l41->dev, "Failed waiting for OTP_BOOT_DONE\n");
-		return ret;
-	}
-
-	ret = regmap_read(cs35l41->regmap, CS35L41_IRQ1_STATUS3, &int_status);
-	if (ret || (int_status & CS35L41_OTP_BOOT_ERR)) {
-		dev_err(cs35l41->dev, "OTP Boot status %x error\n",
-			int_status & CS35L41_OTP_BOOT_ERR);
-		if (!ret)
-			ret = -EIO;
-		return ret;
-	}
-
-	return 0;
-}
-
 static int cs35l41_system_resume(struct device *dev)
 {
 	struct cs35l41_hda *cs35l41 = dev_get_drvdata(dev);
@@ -1020,10 +996,7 @@ static int cs35l41_system_resume(struct device *dev)
 
 	regcache_cache_only(cs35l41->regmap, false);
 
-	regmap_write(cs35l41->regmap, CS35L41_SFT_RESET, CS35L41_SOFTWARE_RESET);
-	usleep_range(2000, 2100);
-
-	ret = cs35l41_wait_boot_done(cs35l41);
+	ret = cs35l41_reset_and_boot(cs35l41->dev, cs35l41->regmap);
 	if (ret)
 		return ret;
 
@@ -2003,11 +1976,9 @@ int cs35l41_hda_probe(struct device *dev, const char *device_name, int id, int i
 		gpiod_set_value_cansleep(cs35l41->reset_gpio, 1);
 	}
 
-	usleep_range(2000, 2100);
-	regmap_write(cs35l41->regmap, CS35L41_SFT_RESET, CS35L41_SOFTWARE_RESET);
 	usleep_range(2000, 2100);
 
-	ret = cs35l41_wait_boot_done(cs35l41);
+	ret = cs35l41_reset_and_boot(cs35l41->dev, cs35l41->regmap);
 	if (ret)
 		goto err;
 
diff --git a/sound/soc/codecs/cs35l41-lib.c b/sound/soc/codecs/cs35l41-lib.c
index b0fa80705be7..973713626ebd 100644
--- a/sound/soc/codecs/cs35l41-lib.c
+++ b/sound/soc/codecs/cs35l41-lib.c
@@ -17,6 +17,8 @@
 #include <sound/cs35l41.h>
 
 #define CS35L41_FIRMWARE_OLD_VERSION 0x001C00 /* v0.28.0 */
+#define CS35L41_GLOBAL_BOOT_DONE_TIMEOUT_US 100000
+#define CS35L41_GLOBAL_BOOT_DONE_SLEEP_US 2000
 
 static const struct reg_default cs35l41_reg[] = {
 	{ CS35L41_PWR_CTRL1,			0x00000000 },
@@ -1586,6 +1588,71 @@ int cs35l41_exit_hibernate(struct device *dev, struct regmap *regmap)
 }
 EXPORT_SYMBOL_GPL(cs35l41_exit_hibernate);
 
+int cs35l41_wait_boot_done(struct device *dev, struct regmap *regmap)
+{
+	ktime_t timeout;
+	unsigned int val = 0;
+	int ret = 0;
+
+	/*
+	 * The chip could NAK a read request while it is booting so ignore
+	 * errors from regmap_read.
+	 */
+	timeout = ktime_add_us(ktime_get(), CS35L41_GLOBAL_BOOT_DONE_TIMEOUT_US);
+	regmap_read(regmap, CS35L41_IRQ1_STATUS4, &val);
+	while (!(val & CS35L41_OTP_BOOT_DONE) && !ktime_after(ktime_get(), timeout)) {
+		usleep_range(CS35L41_GLOBAL_BOOT_DONE_SLEEP_US / 2,
+			     CS35L41_GLOBAL_BOOT_DONE_SLEEP_US);
+		regmap_read(regmap, CS35L41_IRQ1_STATUS4, &val);
+	}
+
+	if (!(val & CS35L41_OTP_BOOT_DONE)) {
+		dev_err(dev, "Timeout waiting for OTP_BOOT_DONE\n");
+		return -ETIMEDOUT;
+	}
+
+	ret = regmap_read(regmap, CS35L41_IRQ1_STATUS3, &val);
+	if (ret || (val & CS35L41_OTP_BOOT_ERR)) {
+		dev_err(dev, "OTP Boot status %x error\n",
+			val & CS35L41_OTP_BOOT_ERR);
+		if (!ret)
+			ret = -EIO;
+		return ret;
+	}
+	return 0;
+}
+EXPORT_SYMBOL_GPL(cs35l41_wait_boot_done);
+
+int cs35l41_reset_and_boot(struct device *dev, struct regmap *regmap)
+{
+	int ret;
+
+	ret = regmap_write(regmap, CS35L41_SFT_RESET, CS35L41_SOFTWARE_RESET);
+	if (ret) {
+		dev_warn(dev, "Failed to write software reset: %d\n", ret);
+
+		/*
+		 * If the write failed, it is likely because the chip is still booting after a
+		 * reset. Wait for boot, then retry the software reset. If the initial write failed
+		 * for another reason, then either waiting for boot, or the second write will fail,
+		 * So we can return the error then.
+		 */
+		ret = cs35l41_wait_boot_done(dev, regmap);
+		if (ret)
+			return ret;
+
+		ret = regmap_write(regmap, CS35L41_SFT_RESET, CS35L41_SOFTWARE_RESET);
+		if (ret) {
+			dev_err(dev, "Failed to write software reset on retry: %d\n", ret);
+			return ret;
+		}
+	}
+	usleep_range(2000, 2100);
+
+	return cs35l41_wait_boot_done(dev, regmap);
+}
+EXPORT_SYMBOL_GPL(cs35l41_reset_and_boot);
+
 MODULE_DESCRIPTION("CS35L41 library");
 MODULE_AUTHOR("David Rhodes, Cirrus Logic Inc, <david.rhodes@cirrus.com>");
 MODULE_AUTHOR("Lucas Tanure, Cirrus Logic Inc, <tanureal@opensource.cirrus.com>");
-- 
2.43.0

Re: [PATCH v1 1/2] ALSA: hda: cs35l41: Add common wait_for_boot api and reset_and_boot api

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] ASoC: cs35l41: false timeout in cs35l41_wait_boot_done() polling loop
--

commit 1eb324e95aeb4455ca413ace17b7aba52e402175
Author: Stefan Binding <sbinding@opensource.cirrus.com>

ALSA: hda: cs35l41: Add common wait_for_boot api and reset_and_boot api

This commit introduces common cs35l41_wait_boot_done() and
cs35l41_reset_and_boot() helpers in cs35l41-lib.c and updates the HDA
CS35L41 driver to use them during probe and system resume. The new helpers
ignore transient regmap_read() failures while the chip is booting and retry
the software reset write if it initially fails.

> diff --git a/sound/soc/codecs/cs35l41-lib.c b/sound/soc/codecs/cs35l41-lib.c
> index b0fa80705be7d..973713626ebd4 100644
> --- a/sound/soc/codecs/cs35l41-lib.c
> +++ b/sound/soc/codecs/cs35l41-lib.c
[ ... ]
> @@ -1586,6 +1588,71 @@ int cs35l41_exit_hibernate(struct device *dev, struct regmap *regmap)
>  }
>  EXPORT_SYMBOL_GPL(cs35l41_exit_hibernate);
>  
> +int cs35l41_wait_boot_done(struct device *dev, struct regmap *regmap)
> +{
> +	ktime_t timeout;
> +	unsigned int val = 0;
> +	int ret = 0;
> +
> +	/*
> +	 * The chip could NAK a read request while it is booting so ignore
> +	 * errors from regmap_read.
> +	 */
> +	timeout = ktime_add_us(ktime_get(), CS35L41_GLOBAL_BOOT_DONE_TIMEOUT_US);
> +	regmap_read(regmap, CS35L41_IRQ1_STATUS4, &val);
> +	while (!(val & CS35L41_OTP_BOOT_DONE) && !ktime_after(ktime_get(), timeout)) {
> +		usleep_range(CS35L41_GLOBAL_BOOT_DONE_SLEEP_US / 2,
> +			     CS35L41_GLOBAL_BOOT_DONE_SLEEP_US);
> +		regmap_read(regmap, CS35L41_IRQ1_STATUS4, &val);
> +	}
> +
> +	if (!(val & CS35L41_OTP_BOOT_DONE)) {
> +		dev_err(dev, "Timeout waiting for OTP_BOOT_DONE\n");
> +		return -ETIMEDOUT;
> +	}

[Severity: Medium]
Could this polling loop in cs35l41_wait_boot_done() return a false
-ETIMEDOUT if the task is preempted or delayed across the timeout deadline?

When cs35l41_hda_probe() or cs35l41_system_resume() calls
cs35l41_reset_and_boot() -> cs35l41_wait_boot_done(), if a regmap_read() of
CS35L41_IRQ1_STATUS4 occurs while the chip is still booting (either
returning a value without CS35L41_OTP_BOOT_DONE set or failing with a bus
NAK and leaving val at 0), and the thread is then preempted during
usleep_range() or delayed on the I2C/SPI controller lock until after
timeout expires, the loop condition !ktime_after(ktime_get(), timeout)
evaluates to false before another register read is performed. Even if the
hardware finished booting well before the deadline, val still holds the
stale pre-preemption value and cs35l41_wait_boot_done() fails with
-ETIMEDOUT.

Would it be better to use read_poll_timeout() from include/linux/iopoll.h
(similar to cs35l56_wait_for_firmware_boot() in
sound/soc/codecs/cs35l56-shared.c), which samples the timeout before
performing the read and guarantees one final read after the deadline expires
while still ignoring regmap_read() errors?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261030171045.469944-1-sbinding@opensource.cirrus.com?part=1

[PATCH v1 2/2] ASoC: cs35l41: Use common wait_for_boot and reset_and_boot apis

[Stefan Binding]

Use the newly added common wait_for_boot and reset_and_boot apis on
probe.

This should prevent issues occurring if the chip is not ready on
before it starts reading and writing to it.

In addition, the driver now ensures the chip is reset using a software
reset, when calling reset_and_boot().

Signed-off-by: Stefan Binding <sbinding@opensource.cirrus.com>
---
 sound/soc/codecs/cs35l41.c | 18 +++---------------
 1 file changed, 3 insertions(+), 15 deletions(-)

diff --git a/sound/soc/codecs/cs35l41.c b/sound/soc/codecs/cs35l41.c
index d286025183f4..0e40444a1003 100644
--- a/sound/soc/codecs/cs35l41.c
+++ b/sound/soc/codecs/cs35l41.c
@@ -1211,7 +1211,7 @@ static int cs35l41_get_system_name(struct cs35l41_private *cs35l41)
 
 int cs35l41_probe(struct cs35l41_private *cs35l41, const struct cs35l41_hw_cfg *hw_cfg)
 {
-	u32 regid, reg_revid, i, mtl_revid, int_status, chipid_match;
+	u32 regid, reg_revid, i, mtl_revid, chipid_match;
 	int irq_pol = 0;
 	int ret;
 
@@ -1260,21 +1260,9 @@ int cs35l41_probe(struct cs35l41_private *cs35l41, const struct cs35l41_hw_cfg *
 
 	usleep_range(2000, 2100);
 
-	ret = regmap_read_poll_timeout(cs35l41->regmap, CS35L41_IRQ1_STATUS4,
-				       int_status, int_status & CS35L41_OTP_BOOT_DONE,
-				       1000, 100000);
-	if (ret) {
-		dev_err_probe(cs35l41->dev, ret,
-			      "Failed waiting for OTP_BOOT_DONE\n");
-		goto err;
-	}
-
-	regmap_read(cs35l41->regmap, CS35L41_IRQ1_STATUS3, &int_status);
-	if (int_status & CS35L41_OTP_BOOT_ERR) {
-		dev_err(cs35l41->dev, "OTP Boot error\n");
-		ret = -EINVAL;
+	ret = cs35l41_reset_and_boot(cs35l41->dev, cs35l41->regmap);
+	if (ret)
 		goto err;
-	}
 
 	ret = regmap_read(cs35l41->regmap, CS35L41_DEVID, &regid);
 	if (ret < 0) {
-- 
2.43.0

Re: [PATCH v1 2/2] ASoC: cs35l41: Use common wait_for_boot and reset_and_boot apis

[Mark Brown]

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

On Fri, Oct 30, 2026 at 05:10:15PM +0000, Stefan Binding wrote:
> Use the newly added common wait_for_boot and reset_and_boot apis on
> probe.
> 
> This should prevent issues occurring if the chip is not ready on
> before it starts reading and writing to it.

Acked-by: Mark Brown <broonie@kernel.org>

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