Re: [PATCH 1/7] ASoC: codecs: ES8389: Modify volatile_register

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Incomplete volatile register list leaves critical configuration un-cached, causing state loss on resume.
- [High] regmap cache is clobbered with hardware defaults during resume, nullifying the preservation of user settings.
--

commit 18a4d58b5b0816359115dead13dc5872d2f0a08a
Author: Zhang Yi <zhangyi@everest-semi.com>

ASoC: codecs: ES8389: Modify volatile_register

Mark some registers that are not volatile as false.

> diff --git a/sound/soc/codecs/es8389.c b/sound/soc/codecs/es8389.c
> index 449d9574b03a8..9c816491648c7 100644
> --- a/sound/soc/codecs/es8389.c
> +++ b/sound/soc/codecs/es8389.c
> @@ -50,10 +50,29 @@ static const char * const es8389_core_supplies[] = {
>  static bool es8389_volatile_register(struct device *dev,
>  			unsigned int reg)
>  {
> -	if ((reg  <= 0xff))
> -		return true;
> -	else
> +	switch (reg) {
> +	case ES8389_ADC_HPF1:
> +	case ES8389_ADC_HPF2:
> +	case ES8389_ADCL_VOL:
> +	case ES8389_ADCR_VOL:
> +	case ES8389_MIC1_GAIN:
> +	case ES8389_MIC2_GAIN:

[Severity: High]
This is a pre-existing issue, but does making these registers non-volatile
cause their cached user settings to be clobbered during resume?

In es8389_resume(), regcache_cache_bypass(es8389->regmap, false) is called
before invoking es8389_init():

sound/soc/codecs/es8389.c:es8389_resume() {
    ...
	regcache_cache_bypass(es8389->regmap, false);

	if (regv == 0xff)
		es8389_init(component);
    ...
}

Because caching is enabled during es8389_init(), its explicit writes to these
registers overwrite the user's settings in the regmap cache with hardware
defaults:

sound/soc/codecs/es8389.c:es8389_init() {
    ...
	regmap_write(es8389->regmap, ES8389_ADC_MUTE, 0xC0);
    ...
}

When regcache_sync() subsequently runs, it syncs these defaults instead of the
user's configuration, which silently mutes the microphone and resets its gain
upon every resume.

> +	case ES8389_DACL_VOL:
> +	case ES8389_DACR_VOL:
> +	case ES8389_ALC_ON:
> +	case ES8389_ALC_CTL:
> +	case ES8389_ALC_TARGET:
> +	case ES8389_ALC_GAIN:
> +	case ES8389_ADC_MUTE:
> +	case ES8389_OSR_VOL:
> +	case ES8389_DAC_INV:
> +	case ES8389_MIX_VOL:
> +	case ES8389_DAC_MIX:
> +	case ES8389_ADC_RESET:
>  		return false;
> +	default:
> +		return true;

[Severity: High]
This is a pre-existing issue, but does this default case incorrectly omit
essential configuration registers?

It leaves important configuration registers like ES8389_MASTER_MODE,
ES8389_ADC_MODE, ES8389_DMIC_EN, ES8389_ADC_FORMAT_MUTE, and
ES8389_DAC_FORMAT_MUTE as volatile.

Because volatile registers bypass the regmap cache, any user changes, such as
DAI formats and DAPM MUX selections, are not cached.

When es8389_init() writes default values to these registers, regcache_sync()
skips them, so the user's MUX and format configurations are never restored,
breaking audio routing.

> +	}
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260609025605.16945-1-zhangyi@everest-semi.com?part=1