mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] ASoC: ak4458: Check reset status after deassert
@ 2026-10-10  8:40 shengjiu.wang
  2026-10-10 19:03 ` Alvin Šipraga
  0 siblings, 1 reply; 2+ messages in thread
From: shengjiu.wang @ 2026-10-10  8:40 UTC (permalink / raw)
  To: lgirdwood, broonie, perex, tiwai, p.zabel, kuninori.morimoto.gx,
	linux-sound, linux-kernel

From: Shengjiu Wang <shengjiu.wang@nxp.com>

The reset line of the ak4458 can be shared, for example when two ak4458
devices use the same reset GPIO. In that case the two codecs may call
reset_control_deassert() concurrently from their runtime resume paths.

Only the first caller performs the actual hardware deassert, which may
sleep (for example a GPIO-expander based reset controller). A second
caller merely increments the shared deassert count and returns
immediately, so it can continue while the first caller's .deassert() has
not finished releasing the line yet, leaving the device still in reset.

Poll reset_control_status() after deassert so a caller does not continue
until the reset is really released. reset_control_status() returns a
positive value while the line is still asserted, zero once it is
deasserted, and a negative errno when the reset controller does not
implement status reporting. Stop polling on status <= 0 so controllers
without a .status callback do not spin for the full timeout, and check
the reset_control_deassert() return value as well.

Fixes: 8a0de73cf9dc ("ASoC: ak4458: add optional reset control to instead of gpio")
Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
 sound/soc/codecs/ak4458.c | 32 +++++++++++++++++++++++++++++---
 1 file changed, 29 insertions(+), 3 deletions(-)

diff --git a/sound/soc/codecs/ak4458.c b/sound/soc/codecs/ak4458.c
index a74be99e877d..64984974ff88 100644
--- a/sound/soc/codecs/ak4458.c
+++ b/sound/soc/codecs/ak4458.c
@@ -8,6 +8,7 @@
 #include <linux/delay.h>
 #include <linux/gpio/consumer.h>
 #include <linux/i2c.h>
+#include <linux/iopoll.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/pm_runtime.h>
@@ -635,11 +636,36 @@ static struct snd_soc_dai_driver ak4497_dai = {
 
 static void ak4458_reset(struct ak4458_priv *ak4458, bool active)
 {
+	int ret, status;
+
 	if (!IS_ERR_OR_NULL(ak4458->reset)) {
-		if (active)
+		if (active) {
 			reset_control_assert(ak4458->reset);
-		else
-			reset_control_deassert(ak4458->reset);
+		} else {
+			ret = reset_control_deassert(ak4458->reset);
+			if (ret) {
+				dev_warn(ak4458->dev,
+					 "failed to deassert reset: %d\n", ret);
+			} else {
+				/*
+				 * On a shared reset line another consumer may
+				 * still be in its hardware .deassert(), which can
+				 * sleep, so the line may not be released yet.
+				 * Poll the status until it reads deasserted (0).
+				 * A negative return means the controller does not
+				 * report status, so stop polling in that case.
+				 */
+				ret = read_poll_timeout(reset_control_status, status,
+							status <= 0, 100, 20000,
+							false, ak4458->reset);
+				if (ret)
+					dev_warn(ak4458->dev,
+						 "reset still asserted after deassert\n");
+				else if (status < 0)
+					dev_dbg(ak4458->dev,
+						"reset status not supported: %d\n", status);
+			}
+		}
 		usleep_range(1000, 2000);
 	}
 }
-- 
2.34.1


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] ASoC: ak4458: Check reset status after deassert
  2026-10-10  8:40 [PATCH] ASoC: ak4458: Check reset status after deassert shengjiu.wang
@ 2026-10-10 19:03 ` Alvin Šipraga
  0 siblings, 0 replies; 2+ messages in thread
From: Alvin Šipraga @ 2026-10-10 19:03 UTC (permalink / raw)
  To: shengjiu.wang
  Cc: lgirdwood, broonie, perex, tiwai, p.zabel, kuninori.morimoto.gx,
	linux-sound, linux-kernel

On Sat, Oct 10, 2026 at 04:40:58PM +0800, shengjiu.wang@oss.nxp.com wrote:
> From: Shengjiu Wang <shengjiu.wang@nxp.com>
> 
> The reset line of the ak4458 can be shared, for example when two ak4458
> devices use the same reset GPIO. In that case the two codecs may call
> reset_control_deassert() concurrently from their runtime resume paths.
> 
> Only the first caller performs the actual hardware deassert, which may
> sleep (for example a GPIO-expander based reset controller). A second
> caller merely increments the shared deassert count and returns
> immediately, so it can continue while the first caller's .deassert() has
> not finished releasing the line yet, leaving the device still in reset.

This seems like something that should be fixed in the reset controller
framework. In fact the issue you describe seems to contradict the
documentation of reset_control_deassert():

/**
 * reset_control_deassert - deasserts the reset line
 * @rstc: reset controller
 *
 * After calling this function, the reset is guaranteed to be deasserted.

^ (1) seems not to be the case as you have found

 * Consumers must not use reset_control_reset on shared reset lines when
 * reset_control_(de)assert has been used.

^ (2) isn't this rule also being broken in the suspend/resume path of
      this driver?

 *
 * If rstc is NULL it is an optional reset and the function will just
 * return 0.
 */
int reset_control_deassert(struct reset_control *rstc)
...


(1) being fixed in the framework would solve your problem.

But (2) seems a bit more intractable. Shared reset GPIOs have always
been a pain... I wonder if there is a better approach?

Kind regards,
Alvin

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-10-10 19:05 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-10  8:40 [PATCH] ASoC: ak4458: Check reset status after deassert shengjiu.wang
2026-10-10 19:03 ` Alvin Šipraga

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®