Re: [PATCH v2 9/9] ASoC: rt5677: restore safe state after resume errors

[Ajay Kumar Nandam <ajay.nandam@oss.qualcomm.com>]

On 10/14/2026 8:54 AM, Pengpeng Hou wrote:
> rt5677_resume() ignores regcache_sync() failure before continuing
> dependent resume operations. Restore cache-only mode, leave the cache
> dirty, and return the replay error before those operations.
> 
> First return physical regmap write errors from the normal-mode write
> wrapper, which currently discards them, so cache replay can observe bus
> failures. Restore reset and the optional LDO GPIO on replay failure.
> Remember that cache restoration is still pending independently of the
> requested DSP VAD mode: DAPM or controls can change that mode after
> failed resume. Suppress DSP work while restoration is pending and
> require replay on the next resume even if VAD has since been requested.
> After successful replay, preserve the existing MCLK settling delay
> before restarting requested DSP work. The already-running, retained VAD
> path and DSP-mode transport are unchanged.
> 
> ASoC still marks the component resumed when the callback reports failure.
> Track the suspend-owned IRQ disable so another suspend does not increment
> the disable depth again after a failed resume. A later successful resume
> releases that one disable reference. This does not add automatic retry or
> change the framework's best-effort card resume policy.
> 
> The issue was found by our static-analysis tool and manually reviewed.
> 
> Fixes: 0e826e867264 ("ASoC: add RT5677 CODEC driver")
> Assisted-by: gpt 5
> Signed-off-by: Pengpeng Hou <hppiscas@163.com>
> ---
> Changes since v1:
> - restore cache-only mode and dirty state on replay failure
> - keep suspend-owned IRQ disables balanced across failed resume cycles
> - propagate the normal-mode transport write result to cache replay
> - preserve pending cache restoration across changes to the VAD request
> - keep DSP work paused until replay succeeds, retaining the MCLK delay
> - send as one correctly threaded series, as Mark requested
> 
> Previous series:
> https://lore.kernel.org/all/20260906034321.86080-1-hppiscas@163.com/
> 
>   sound/soc/codecs/rt5677.c |   37 ++++++++++++++++++++++++++++++++-----
>   sound/soc/codecs/rt5677.h |    2 ++
>   2 files changed, 34 insertions(+), 5 deletions(-)
> 
> diff --git a/sound/soc/codecs/rt5677.c b/sound/soc/codecs/rt5677.c
> index 7b80922236e5818e846d9ecc6e23793e6ae40714..fd1f840d6794ea95b47d803ba4977fdb81e184a9 100644
> --- a/sound/soc/codecs/rt5677.c
> +++ b/sound/soc/codecs/rt5677.c
> @@ -885,6 +885,9 @@ static void rt5677_dsp_work(struct work_struct *work)
>   	bool enable = rt5677->dsp_vad_en;
>   	int i, val;
>   
> +	/* DAPM may request VAD even after component resume has failed. */
> +	if (READ_ONCE(rt5677->cache_restore_pending))
> +		return;
>   
>   	dev_info(rt5677->component->dev, "DSP VAD: enable=%d, activity=%d\n",
>   			enable, activity);
> @@ -4939,10 +4942,14 @@ static int rt5677_suspend(struct snd_soc_component *component)
>   
>   	if (rt5677->irq) {
>   		cancel_delayed_work_sync(&rt5677->resume_irq_check);
> -		disable_irq(rt5677->irq);
> +		if (!rt5677->suspend_irq_disabled) {
> +			disable_irq(rt5677->irq);
> +			rt5677->suspend_irq_disabled = true;
> +		}

Similar to the nau8825 case, should this new suspend_irq_disabled 
tracking be based on whether the IRQ was successfully requested?

rt5677_init_irq() still sets rt5677->irq = i2c->irq even when
devm_request_threaded_irq() fails, and probe continues after only 
logging that error.

Thanks
Ajay Kumar Nandam

>   	}
>   
>   	if (!rt5677->dsp_vad_en) {
> +		WRITE_ONCE(rt5677->cache_restore_pending, true);
>   		regcache_cache_only(rt5677->regmap, true);
>   		regcache_mark_dirty(rt5677->regmap);
>   
> @@ -4956,8 +4963,11 @@ static int rt5677_suspend(struct snd_soc_component *component)
>   static int rt5677_resume(struct snd_soc_component *component)
>   {
>   	struct rt5677_priv *rt5677 = snd_soc_component_get_drvdata(component);
> +	bool restore_cache = !rt5677->dsp_vad_en ||
> +			     READ_ONCE(rt5677->cache_restore_pending);
> +	int ret;
>   
> -	if (!rt5677->dsp_vad_en) {
> +	if (restore_cache) {
>   		rt5677->pll_src = 0;
>   		rt5677->pll_in = 0;
>   		rt5677->pll_out = 0;
> @@ -4967,11 +4977,28 @@ static int rt5677_resume(struct snd_soc_component *component)
>   			msleep(10);
>   
>   		regcache_cache_only(rt5677->regmap, false);
> -		regcache_sync(rt5677->regmap);
> +		ret = regcache_sync(rt5677->regmap);
> +		if (ret) {
> +			WRITE_ONCE(rt5677->cache_restore_pending, true);
> +			regcache_cache_only(rt5677->regmap, true);
> +			regcache_mark_dirty(rt5677->regmap);
> +			gpiod_set_value_cansleep(rt5677->reset_pin, 1);
> +			gpiod_set_value_cansleep(rt5677->pow_ldo2, 0);
> +			return ret;
> +		}
> +		WRITE_ONCE(rt5677->cache_restore_pending, false);
> +		if (rt5677->dsp_vad_en) {
> +			/* Allow MCLK to settle, as when leaving bias OFF. */
> +			schedule_delayed_work(&rt5677->dsp_work,
> +					     msecs_to_jiffies(1000));
> +		}
>   	}
>   
>   	if (rt5677->irq) {
> -		enable_irq(rt5677->irq);
> +		if (rt5677->suspend_irq_disabled) {
> +			rt5677->suspend_irq_disabled = false;
> +			enable_irq(rt5677->irq);
> +		}
>   		schedule_delayed_work(&rt5677->resume_irq_check, 0);
>   	}
>   
> @@ -5021,7 +5048,7 @@ static int rt5677_write(void *context, unsigned int reg, unsigned int val)
>   			rt5677_dsp_mode_i2c_write(rt5677, reg, val);
>   		}
>   	} else {
> -		regmap_write(rt5677->regmap_physical, reg, val);
> +		return regmap_write(rt5677->regmap_physical, reg, val);
>   	}
>   
>   	return 0;
> diff --git a/sound/soc/codecs/rt5677.h b/sound/soc/codecs/rt5677.h
> index 876f7674015c2c0a5421feb25a8c80313b18310a..e6e915453f2efbe78c9d2dd3e909397d9ad715ca 100644
> --- a/sound/soc/codecs/rt5677.h
> +++ b/sound/soc/codecs/rt5677.h
> @@ -1788,6 +1788,7 @@ struct rt5677_priv {
>   #endif
>   	bool dsp_vad_en_request; /* DSP VAD enable/disable request */
>   	bool dsp_vad_en; /* dsp_work parameter */
> +	bool cache_restore_pending;
>   	bool is_dsp_mode;
>   	bool is_vref_slow;
>   	struct delayed_work dsp_work;
> @@ -1798,6 +1799,7 @@ struct rt5677_priv {
>   	unsigned int irq_en;
>   	struct delayed_work resume_irq_check;
>   	int irq;
> +	bool suspend_irq_disabled;
>   
>   	int (*set_dsp_vad)(struct snd_soc_component *component, bool on);
>   };
> 
> base-commit: 8ef28aef33f2d5da6959ad6b579ec3cfccc009f6
> 
>