[PATCH 0/7] ASoC: codecs: ES8389: Modify the ES8389 driver

[PATCH 0/7] ASoC: codecs: ES8389: Modify the ES8389 driver

[Zhang Yi]

v0:
  - Modify volatile_register
  - Add MCLK_MASK to resolve the issue related to mclk_src
  - Modify configuration about clock
  - Add private members about HPF at driver and devicetree
  - Add INPUT MUX at route

Zhang Yi (7):
  ASoC: codecs: ES8389: Modify volatile_register
  ASoC: codecs: ES8389: Fix the issue about mclk_src
  ASoC: codecs: ES8389: Modify the clock table
  ASoC: codecs: ES8389: Modify the initial configuration
  ASoC: codecs: ES8389: Add private members related to HPF
  ASoC: codecs: ES8389: Add INPUTL MUX and INPUTR MUX
  ASoC: dt-bindings: ES8389: Add members about HPF

 .../bindings/sound/everest,es8389.yaml        |  23 +++
 sound/soc/codecs/es8389.c                     | 136 +++++++++++++-----
 sound/soc/codecs/es8389.h                     |   6 +-
 3 files changed, 125 insertions(+), 40 deletions(-)

-- 
2.17.1

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

[Zhang Yi]

Mark some registers that are not volatile as false

Signed-off-by: Zhang Yi <zhangyi@everest-semi.com>
---
 sound/soc/codecs/es8389.c | 25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/sound/soc/codecs/es8389.c b/sound/soc/codecs/es8389.c
index 449d9574b..9c8164916 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:
+	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;
+	}
 }
 
 static const DECLARE_TLV_DB_SCALE(dac_vol_tlv, -9550, 50, 0);
-- 
2.17.1

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

[sashiko-bot]

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

[PATCH 2/7] ASoC: codecs: ES8389: Fix the issue about mclk_src

[Zhang Yi]

Fix the issue with incorrect modifications to mclk_src

Signed-off-by: Zhang Yi <zhangyi@everest-semi.com>
---
 sound/soc/codecs/es8389.c | 12 ++++--------
 sound/soc/codecs/es8389.h |  5 ++---
 2 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/sound/soc/codecs/es8389.c b/sound/soc/codecs/es8389.c
index 9c8164916..7a9d5d42a 100644
--- a/sound/soc/codecs/es8389.c
+++ b/sound/soc/codecs/es8389.c
@@ -36,8 +36,8 @@ struct	es8389_private {
 	unsigned int sysclk;
 	int mastermode;
 
-	u8 mclk_src;
 	u8 vddd;
+	bool mclk_src;
 	int version;
 	enum snd_soc_bias_level bias_level;
 };
@@ -607,9 +607,9 @@ static int es8389_pcm_hw_params(struct snd_pcm_substream *substream,
 	regmap_update_bits(es8389->regmap, ES8389_ADC_FORMAT_MUTE, ES8389_DATA_LEN_MASK, state);
 	regmap_update_bits(es8389->regmap, ES8389_DAC_FORMAT_MUTE, ES8389_DATA_LEN_MASK, state);
 
-	if (es8389->mclk_src == ES8389_SCLK_PIN) {
+	if (es8389->mclk_src) {
 		regmap_update_bits(es8389->regmap, ES8389_MASTER_CLK,
-					ES8389_MCLK_SOURCE, es8389->mclk_src);
+					ES8389_MCLK_MASK, ES8389_MCLK_FROM_SCLK);
 		es8389->sysclk = params_channels(params) * params_width(params) * params_rate(params);
 	}
 
@@ -897,11 +897,7 @@ static int es8389_probe(struct snd_soc_component *component)
 	int ret, i;
 	struct es8389_private *es8389 = snd_soc_component_get_drvdata(component);
 
-	ret = device_property_read_u8(component->dev, "everest,mclk-src", &es8389->mclk_src);
-	if (ret != 0) {
-		dev_dbg(component->dev, "mclk-src return %d", ret);
-		es8389->mclk_src = ES8389_MCLK_SOURCE;
-	}
+	es8389->mclk_src = device_property_read_bool(component->dev, "everest,mclk-from-sclk");
 
 	for (i = 0; i < ARRAY_SIZE(es8389_core_supplies); i++)
 		es8389->core_supply[i].supply = es8389_core_supplies[i];
diff --git a/sound/soc/codecs/es8389.h b/sound/soc/codecs/es8389.h
index d21e72f87..7f1212219 100644
--- a/sound/soc/codecs/es8389.h
+++ b/sound/soc/codecs/es8389.h
@@ -116,9 +116,8 @@
 #define ES8389_TDM_SLOT               (0x70 << 0)
 #define ES8389_TDM_SHIFT              4
 
-#define ES8389_MCLK_SOURCE            (1 << 6)
-#define ES8389_MCLK_PIN               (1 << 6)
-#define ES8389_SCLK_PIN               (0 << 6)
+#define ES8389_MCLK_MASK              (3 << 6)
+#define ES8389_MCLK_FROM_SCLK         (1 << 6)
 
 /* ES8389_FMT */
 #define ES8389_S24_LE                 (0 << 5)
-- 
2.17.1

Re: [PATCH 2/7] ASoC: codecs: ES8389: Fix the issue about mclk_src

[sashiko-bot]

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

Pre-existing issues:
- [High] Incorrect sysclk calculation for SCLK mode due to using params_width() instead of params_physical_width() or snd_soc_params_to_bclk().
- [High] Component probe function ties devm_ allocations to the I2C device, causing memory and resource leaks on ASoC card unbind/rebind.
- [High] Regulators are enabled in component probe but never disabled in component remove, leading to regulator enable count leaks.
- [High] Silent failure when valid clock coefficients are not found.
--

commit 96650b0d78cc8e7757fca690cc1cc18f2d5796b3
Author: Zhang Yi <zhangyi@everest-semi.com>

ASoC: codecs: ES8389: Fix the issue about mclk_src

Fix the issue with incorrect modifications to mclk_src

> diff --git a/sound/soc/codecs/es8389.c b/sound/soc/codecs/es8389.c
> index 9c816491648c7..7a9d5d42a05d0 100644
> --- a/sound/soc/codecs/es8389.c
> +++ b/sound/soc/codecs/es8389.c
[ ... ]
> @@ -607,9 +607,9 @@ static int es8389_pcm_hw_params(struct snd_pcm_substream *substream,
>  	regmap_update_bits(es8389->regmap, ES8389_ADC_FORMAT_MUTE, ES8389_DATA_LEN_MASK, state);
>  	regmap_update_bits(es8389->regmap, ES8389_DAC_FORMAT_MUTE, ES8389_DATA_LEN_MASK, state);
>  
> -	if (es8389->mclk_src == ES8389_SCLK_PIN) {
> +	if (es8389->mclk_src) {
>  		regmap_update_bits(es8389->regmap, ES8389_MASTER_CLK,
> -					ES8389_MCLK_SOURCE, es8389->mclk_src);
> +					ES8389_MCLK_MASK, ES8389_MCLK_FROM_SCLK);
>  		es8389->sysclk = params_channels(params) * params_width(params) * params_rate(params);

[Severity: High]
This isn't a bug introduced by this patch, but does this calculate the
sysclk correctly for SCLK mode?

It looks like params_width() returns the semantic sample width (e.g., 24
for S24_LE), rather than the physical slot width on the I2S bus
(typically 32 bits for 24-bit samples).

Could this result in an incorrectly calculated sysclk, causing get_coeff()
to fail or configure incorrect clock dividers? Should this use
params_physical_width() or snd_soc_params_to_bclk() instead?

Later in this same function:

sound/soc/codecs/es8389.c:es8389_pcm_hw_params() {
    ...
	coeff = get_coeff(es8389->vddd, dmic_enable, es8389->sysclk, params_rate(params));
	if (coeff >= 0) {
        ...
	} else {
		dev_warn(component->dev, "Clock coefficients do not match");
	}

	return 0;
}

[Severity: High]
This is a pre-existing issue, but does this safely handle errors?

If get_coeff() fails to find valid clock coefficients and returns -EINVAL,
the driver logs a warning but unconditionally returns 0 (success) to the
ALSA core.

Will this cause the audio framework to proceed with an unconfigured codec,
resulting in silent failures or garbled audio? Should it propagate the error
code instead?

[ ... ]
> @@ -897,11 +897,7 @@ static int es8389_probe(struct snd_soc_component *component)
>  	int ret, i;
>  	struct es8389_private *es8389 = snd_soc_component_get_drvdata(component);
>  
> -	ret = device_property_read_u8(component->dev, "everest,mclk-src", &es8389->mclk_src);
> -	if (ret != 0) {
> -		dev_dbg(component->dev, "mclk-src return %d", ret);
> -		es8389->mclk_src = ES8389_MCLK_SOURCE;
> -	}
> +	es8389->mclk_src = device_property_read_bool(component->dev, "everest,mclk-from-sclk");
>  
>  	for (i = 0; i < ARRAY_SIZE(es8389_core_supplies); i++)
>  		es8389->core_supply[i].supply = es8389_core_supplies[i];

[Severity: High]
This isn't a bug introduced by this patch, but does es8389_probe() leak
resources during ASoC card unbind/rebind?

This is an ASoC component probe callback (executed when the sound card
binds), not an I2C driver probe callback. However, later in this
function, it allocates devm resources tied to the I2C device:

sound/soc/codecs/es8389.c:es8389_probe() {
    ...
	ret = devm_regulator_bulk_get(component->dev, ARRAY_SIZE(es8389_core_supplies), es8389->core_supply);
    ...
	es8389->mclk = devm_clk_get_optional(component->dev, "mclk");
    ...
}

Since the I2C device's lifecycle spans beyond the sound card's lifecycle,
every time the sound card is unbound and rebound, won't es8389_probe()
execute again, allocating new regulators and clock handles on the I2C
device's devres list without freeing the previous ones?

Furthermore, this probe function enables the regulators:

sound/soc/codecs/es8389.c:es8389_probe() {
    ...
	ret = regulator_bulk_enable(ARRAY_SIZE(es8389_core_supplies), es8389->core_supply);
    ...
}

[Severity: High]
This is a pre-existing issue, but does this leak the regulator enable count?

The corresponding es8389_remove() function does not call
regulator_bulk_disable():

sound/soc/codecs/es8389.c:es8389_remove() {
	struct es8389_private *es8389 = snd_soc_component_get_drvdata(component);

	regmap_write(es8389->regmap, ES8389_MASTER_MODE, 0x28);
	regmap_write(es8389->regmap, ES8389_HPSW, 0x00);
    ...
}

Every time the ASoC component is bound and then unbound, won't the
regulator's internal enable count be incremented but never decremented?
Could this prevent the regulator from ever being powered off by the system,
leading to increased power consumption?

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

[PATCH 3/7] ASoC: codecs: ES8389: Modify the clock table

[Zhang Yi]

Updated the configuration for certain frequencies

Signed-off-by: Zhang Yi <zhangyi@everest-semi.com>
---
 sound/soc/codecs/es8389.c | 46 ++++++++++++++++++++-------------------
 1 file changed, 24 insertions(+), 22 deletions(-)

diff --git a/sound/soc/codecs/es8389.c b/sound/soc/codecs/es8389.c
index 7a9d5d42a..6ad0b5800 100644
--- a/sound/soc/codecs/es8389.c
+++ b/sound/soc/codecs/es8389.c
@@ -434,52 +434,54 @@ static const struct _coeff_div  coeff_div[] = {
 	{36, 576000, 16000, 0x00, 0x55, 0x84, 0xD0, 0x01, 0xC1, 0x90, 0x00, 0x00, 0x23, 0x8F, 0xBF, 0xC0, 0x1F, 0x8F, 0x01, 0x12, 0x00, 0x12, 0x31, 0x0E, 2, 2},
 	{48, 768000, 16000, 0x02, 0x57, 0x04, 0xC0, 0x01, 0xC1, 0x90, 0x00, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0xFF, 0x7F, 0x00, 0x12, 0x00, 0x12, 0x31, 0x0E, 2, 2},
 	{50, 800000, 16000, 0x00, 0x7E, 0x01, 0xD9, 0x00, 0xC2, 0x80, 0x00, 0x00, 0x18, 0x95, 0xD0, 0xC0, 0xC7, 0x95, 0x00, 0x12, 0x00, 0x12, 0x31, 0x0E, 2, 2},
-	{64, 1024000, 16000, 0x00, 0x45, 0x24, 0xC0, 0x01, 0xD1, 0x90, 0x00, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0xFF, 0x7F, 0x00, 0x12, 0x00, 0x12, 0x31, 0x0E, 2, 2},
-	{72, 1152000, 16000, 0x00, 0x45, 0x24, 0xC0, 0x01, 0xD1, 0x90, 0x00, 0x00, 0x23, 0x8F, 0xBF, 0xC0, 0x1F, 0x8F, 0x01, 0x12, 0x00, 0x12, 0x31, 0x0E, 2, 2},
+	{64, 1024000, 16000, 0x00, 0x45, 0x24, 0xC0, 0x01, 0xC1, 0x90, 0x00, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0xFF, 0x7F, 0x00, 0x12, 0x00, 0x12, 0x31, 0x0E, 2, 2},
+	{72, 1152000, 16000, 0x00, 0x45, 0x24, 0xC0, 0x01, 0xC1, 0x90, 0x00, 0x00, 0x23, 0x8F, 0xBF, 0xC0, 0x1F, 0x8F, 0x01, 0x12, 0x00, 0x12, 0x31, 0x0E, 2, 2},
 	{96, 1536000, 16000, 0x02, 0x55, 0x84, 0xD0, 0x01, 0xC1, 0x90, 0x00, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0xFF, 0x7F, 0x00, 0x12, 0x00, 0x12, 0x31, 0x0E, 2, 2},
 	{128, 2048000, 16000, 0x00, 0x51, 0x04, 0xD0, 0x01, 0xC1, 0x90, 0x00, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0xFF, 0x7F, 0x00, 0x12, 0x00, 0x12, 0x31, 0x0E, 2, 2},
 	{144, 2304000, 16000, 0x00, 0x51, 0x00, 0xC0, 0x01, 0xC1, 0x90, 0x00, 0x00, 0x23, 0x8F, 0xBF, 0xC0, 0x1F, 0x8F, 0x01, 0x12, 0x00, 0x12, 0x31, 0x0E, 2, 2},
+	{150, 2400000, 16000, 0x02, 0x7E, 0x01, 0xC9, 0x00, 0xC2, 0x80, 0x40, 0x00, 0x18, 0x95, 0xD0, 0xC0, 0xC7, 0x95, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
 	{192, 3072000, 16000, 0x02, 0x65, 0x25, 0xE0, 0x00, 0xE1, 0x90, 0x00, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0xFF, 0x7F, 0x00, 0x12, 0x00, 0x12, 0x31, 0x0E, 2, 2},
-	{256, 4096000, 16000, 0x00, 0x41, 0x04, 0xC0, 0x01, 0xD1, 0x90, 0x00, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0xFF, 0x7F, 0x00, 0x12, 0x00, 0x12, 0x31, 0x0E, 2, 2},
+	{256, 4096000, 16000, 0x00, 0x41, 0x04, 0xC0, 0x01, 0xC1, 0x90, 0x00, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0xFF, 0x7F, 0x00, 0x12, 0x00, 0x12, 0x31, 0x0E, 2, 2},
 	{300, 4800000, 16000, 0x02, 0x66, 0x01, 0xD9, 0x00, 0xC2, 0x80, 0x00, 0x00, 0x18, 0x95, 0xD0, 0xC0, 0xC7, 0x95, 0x00, 0x12, 0x00, 0x12, 0x31, 0x0E, 2, 2},
 	{384, 6144000, 16000, 0x02, 0x51, 0x04, 0xD0, 0x01, 0xC1, 0x90, 0x00, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0xFF, 0x7F, 0x00, 0x12, 0x00, 0x12, 0x31, 0x0E, 2, 2},
-	{512, 8192000, 16000, 0x01, 0x41, 0x04, 0xC0, 0x01, 0xD1, 0x90, 0x00, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0xFF, 0x7F, 0x00, 0x12, 0x00, 0x12, 0x31, 0x0E, 2, 2},
+	{512, 8192000, 16000, 0x01, 0x41, 0x04, 0xC0, 0x01, 0xC1, 0x90, 0x00, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0xFF, 0x7F, 0x00, 0x12, 0x00, 0x12, 0x31, 0x0E, 2, 2},
 	{750, 12000000, 16000, 0x0E, 0x7E, 0x01, 0xC9, 0x00, 0xC2, 0x80, 0x40, 0x00, 0x18, 0x95, 0xD0, 0xC0, 0xC7, 0x95, 0x00, 0x12, 0x00, 0x12, 0x31, 0x0E, 2, 2},
-	{768, 12288000, 16000, 0x02, 0x41, 0x04, 0xC0, 0x01, 0xD1, 0x90, 0x40, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0xFF, 0x7F, 0x00, 0x12, 0x00, 0x12, 0x31, 0x0E, 2, 2},
-	{1024, 16384000, 16000, 0x03, 0x41, 0x04, 0xC0, 0x01, 0xD1, 0x90, 0x40, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0xFF, 0x7F, 0x00, 0x12, 0x00, 0x12, 0x31, 0x0E, 2, 2},
+	{768, 12288000, 16000, 0x02, 0x41, 0x04, 0xC0, 0x01, 0xC1, 0x90, 0x40, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0xFF, 0x7F, 0x00, 0x12, 0x00, 0x12, 0x31, 0x0E, 2, 2},
+	{1024, 16384000, 16000, 0x03, 0x41, 0x04, 0xC0, 0x01, 0xC1, 0x90, 0x40, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0xFF, 0x7F, 0x00, 0x12, 0x00, 0x12, 0x31, 0x0E, 2, 2},
 	{1152, 18432000, 16000, 0x08, 0x51, 0x04, 0xD0, 0x01, 0xC1, 0x90, 0x40, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0xFF, 0x7F, 0x00, 0x12, 0x00, 0x12, 0x31, 0x0E, 2, 2},
 	{1200, 19200000, 16000, 0x0B, 0x66, 0x01, 0xD9, 0x00, 0xC2, 0x80, 0x40, 0x00, 0x18, 0x95, 0xD0, 0xC0, 0xC7, 0x95, 0x00, 0x12, 0x00, 0x12, 0x31, 0x0E, 2, 2},
 	{1500, 24000000, 16000, 0x0E, 0x26, 0x01, 0xD9, 0x00, 0xC2, 0x80, 0xC0, 0x00, 0x18, 0x95, 0xD0, 0xC0, 0xC7, 0x95, 0x00, 0x12, 0x00, 0x12, 0x31, 0x0E, 2, 2},
-	{1536, 24576000, 16000, 0x05, 0x41, 0x04, 0xC0, 0x01, 0xD1, 0x90, 0xC0, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0xFF, 0x7F, 0x00, 0x12, 0x00, 0x12, 0x31, 0x0E, 2, 2},
+	{1536, 24576000, 16000, 0x05, 0x41, 0x04, 0xC0, 0x01, 0xC1, 0x90, 0xC0, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0xFF, 0x7F, 0x00, 0x12, 0x00, 0x12, 0x31, 0x0E, 2, 2},
 	{1625, 26000000, 16000, 0x40, 0x6E, 0x05, 0xC8, 0x01, 0xC2, 0x90, 0xC0, 0x00, 0x18, 0x95, 0xD0, 0xC0, 0x63, 0x95, 0x00, 0x12, 0x00, 0x12, 0x31, 0x0E, 2, 2},
 	{800, 19200000, 24000, 0x07, 0x66, 0x01, 0xD9, 0x00, 0xC2, 0x80, 0x40, 0x00, 0x18, 0x95, 0xD0, 0xC0, 0xC7, 0x95, 0x00, 0x12, 0x00, 0x1A, 0x49, 0x14, 2, 2},
 	{375, 12000000, 32000, 0x0E, 0x2E, 0x05, 0xC8, 0x00, 0xC2, 0x80, 0x40, 0x01, 0x18, 0x95, 0xD0, 0xC0, 0x63, 0x95, 0x00, 0x12, 0x00, 0x23, 0x61, 0x1B, 2, 0},
-	{600, 19200000, 32000, 0x05, 0x46, 0x01, 0xD8, 0x10, 0xD2, 0x80, 0x40, 0x00, 0x18, 0x95, 0xD0, 0xC0, 0x63, 0x95, 0x00, 0x12, 0x00, 0x23, 0x61, 0x1B, 2, 2},
-	{32, 1411200, 44100, 0x00, 0x45, 0xA4, 0xD0, 0x10, 0xD1, 0x80, 0x00, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0x7F, 0x7F, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
+	{600, 19200000, 32000, 0x05, 0x46, 0x01, 0xD8, 0x10, 0xC2, 0x80, 0x40, 0x00, 0x18, 0x95, 0xD0, 0xC0, 0x63, 0x95, 0x00, 0x12, 0x00, 0x23, 0x61, 0x1B, 2, 2},
+	{32, 1411200, 44100, 0x00, 0x45, 0xA4, 0xD0, 0x10, 0xC1, 0x80, 0x00, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0x7F, 0x7F, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
 	{64, 2822400, 44100, 0x00, 0x51, 0x00, 0xC0, 0x10, 0xC1, 0x80, 0x00, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0x7F, 0x7F, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
-	{128, 5644800, 44100, 0x00, 0x41, 0x04, 0xD0, 0x10, 0xD1, 0x80, 0x00, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0x7F, 0x7F, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
-	{256, 11289600, 44100, 0x01, 0x41, 0x04, 0xD0, 0x10, 0xD1, 0x80, 0x40, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0x7F, 0x7F, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
-	{512, 22579200, 44100, 0x03, 0x41, 0x04, 0xD0, 0x10, 0xD1, 0x80, 0xC0, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0x7F, 0x7F, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
-	{32, 1536000, 48000, 0x00, 0x45, 0xA4, 0xD0, 0x10, 0xD1, 0x80, 0x00, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0x7F, 0x7F, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
+	{128, 5644800, 44100, 0x00, 0x41, 0x04, 0xD0, 0x10, 0xC1, 0x80, 0x00, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0x7F, 0x7F, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
+	{256, 11289600, 44100, 0x01, 0x41, 0x04, 0xD0, 0x10, 0xC1, 0x80, 0x40, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0x7F, 0x7F, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
+	{512, 22579200, 44100, 0x03, 0x41, 0x04, 0xD0, 0x10, 0xC1, 0x80, 0xC0, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0x7F, 0x7F, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
+	{32, 1536000, 48000, 0x00, 0x45, 0xA4, 0xD0, 0x10, 0xC1, 0x80, 0x00, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0x7F, 0x7F, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
 	{48, 2304000, 48000, 0x02, 0x55, 0x04, 0xC0, 0x10, 0xC1, 0x80, 0x00, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0x7F, 0x7F, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
 	{50, 2400000, 48000, 0x00, 0x76, 0x01, 0xC8, 0x10, 0xC2, 0x80, 0x00, 0x00, 0x18, 0x95, 0xD0, 0xC0, 0x63, 0x95, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
 	{64, 3072000, 48000, 0x00, 0x51, 0x04, 0xC0, 0x10, 0xC1, 0x80, 0x00, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0x7F, 0x7F, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
-	{100, 4800000, 48000, 0x00, 0x46, 0x01, 0xD8, 0x10, 0xD2, 0x80, 0x00, 0x00, 0x18, 0x95, 0xD0, 0xC0, 0x63, 0x95, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
+	{100, 4800000, 48000, 0x00, 0x46, 0x01, 0xD8, 0x10, 0xC2, 0x80, 0x00, 0x00, 0x18, 0x95, 0xD0, 0xC0, 0x63, 0x95, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
 	{125, 6000000, 48000, 0x04, 0x6E, 0x05, 0xC8, 0x10, 0xC2, 0x80, 0x00, 0x01, 0x18, 0x95, 0xD0, 0xC0, 0x63, 0x95, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
-	{128, 6144000, 48000, 0x00, 0x41, 0x04, 0xD0, 0x10, 0xD1, 0x80, 0x00, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0x7F, 0x7F, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
-	{200, 9600000, 48000, 0x01, 0x46, 0x01, 0xD8, 0x10, 0xD2, 0x80, 0x00, 0x00, 0x18, 0x95, 0xD0, 0xC0, 0x63, 0x95, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
+	{128, 6144000, 48000, 0x00, 0x41, 0x04, 0xD0, 0x10, 0xC1, 0x80, 0x00, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0x7F, 0x7F, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
+	{200, 9600000, 48000, 0x01, 0x46, 0x01, 0xD8, 0x10, 0xC2, 0x80, 0x00, 0x00, 0x18, 0x95, 0xD0, 0xC0, 0x63, 0x95, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
 	{250, 12000000, 48000, 0x04, 0x76, 0x01, 0xC8, 0x10, 0xC2, 0x80, 0x40, 0x00, 0x18, 0x95, 0xD0, 0xC0, 0x63, 0x95, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
-	{256, 12288000, 48000, 0x01, 0x41, 0x04, 0xD0, 0x10, 0xD1, 0x80, 0x40, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0x7F, 0x7F, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
-	{384, 18432000, 48000, 0x02, 0x41, 0x04, 0xD0, 0x10, 0xD1, 0x80, 0x40, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0x7F, 0x7F, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
-	{400, 19200000, 48000, 0x03, 0x46, 0x01, 0xD8, 0x10, 0xD2, 0x80, 0x40, 0x00, 0x18, 0x95, 0xD0, 0xC0, 0x63, 0x95, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
-	{500, 24000000, 48000, 0x04, 0x46, 0x01, 0xD8, 0x10, 0xD2, 0x80, 0xC0, 0x00, 0x18, 0x95, 0xD0, 0xC0, 0x63, 0x95, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
-	{512, 24576000, 48000, 0x03, 0x41, 0x04, 0xD0, 0x10, 0xD1, 0x80, 0xC0, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0x7F, 0x7F, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
+	{256, 12288000, 48000, 0x01, 0x41, 0x04, 0xD0, 0x10, 0xC1, 0x80, 0x40, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0x7F, 0x7F, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
+	{384, 18432000, 48000, 0x02, 0x41, 0x04, 0xD0, 0x10, 0xC1, 0x80, 0x40, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0x7F, 0x7F, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
+	{400, 19200000, 48000, 0x03, 0x46, 0x01, 0xD8, 0x10, 0xC2, 0x80, 0x40, 0x00, 0x18, 0x95, 0xD0, 0xC0, 0x63, 0x95, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
+	{500, 24000000, 48000, 0x04, 0x46, 0x01, 0xD8, 0x10, 0xC2, 0x80, 0xC0, 0x00, 0x18, 0x95, 0xD0, 0xC0, 0x63, 0x95, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
+	{512, 24576000, 48000, 0x03, 0x41, 0x04, 0xD0, 0x10, 0xC1, 0x80, 0xC0, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0x7F, 0x7F, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
 	{800, 38400000, 48000, 0x18, 0x45, 0x04, 0xC0, 0x10, 0xC1, 0x80, 0xC0, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0x7F, 0x7F, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
 	{128, 11289600, 88200, 0x00, 0x50, 0x00, 0xC0, 0x10, 0xC1, 0x80, 0x40, 0x00, 0x9F, 0x7F, 0xBF, 0xC0, 0x7F, 0x7F, 0x80, 0x12, 0xC0, 0x32, 0x89, 0x25, 2, 2},
-	{64, 6144000, 96000, 0x00, 0x41, 0x00, 0xD0, 0x10, 0xD1, 0x80, 0x00, 0x00, 0x9F, 0x7F, 0xBF, 0xC0, 0x7F, 0x7F, 0x80, 0x12, 0xC0, 0x35, 0x91, 0x28, 2, 2},
+	{64, 6144000, 96000, 0x00, 0x41, 0x00, 0xD0, 0x10, 0xC1, 0x80, 0x00, 0x00, 0x9F, 0x7F, 0xBF, 0xC0, 0x7F, 0x7F, 0x80, 0x12, 0xC0, 0x35, 0x91, 0x28, 2, 2},
 	{96, 9216000, 96000, 0x02, 0x43, 0x00, 0xC0, 0x10, 0xC0, 0x80, 0x00, 0x00, 0x9F, 0x7F, 0xBF, 0xC0, 0x7F, 0x7F, 0x80, 0x12, 0xC0, 0x35, 0x91, 0x28, 2, 2},
 	{256, 24576000, 96000, 0x00, 0x40, 0x00, 0xC0, 0x10, 0xC1, 0x80, 0xC0, 0x00, 0x9F, 0x7F, 0xBF, 0xC0, 0x7F, 0x7F, 0x80, 0x12, 0xC0, 0x35, 0x91, 0x28, 2, 2},
 	{128, 24576000, 192000, 0x00, 0x50, 0x00, 0xC0, 0x18, 0xC1, 0x81, 0xC0, 0x00, 0x8F, 0x7F, 0xBF, 0xC0, 0x3F, 0x7F, 0x80, 0x12, 0xC0, 0x3F, 0xF9, 0x3F, 2, 2},
+	{64, 12288000, 192000, 0x00, 0x41, 0x00, 0xC0, 0x18, 0xC1, 0x80, 0x00, 0x00, 0x8F, 0x7F, 0xEF, 0xC0, 0x7F, 0x7F, 0x80, 0x12, 0xC0, 0x3F, 0xF9, 0x3F, 1, 0},
 };
 
 static inline int get_coeff(u8 vddd, u8 dmic, int mclk, int rate)
-- 
2.17.1

Re: [PATCH 3/7] ASoC: codecs: ES8389: Modify the clock table

[sashiko-bot]

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

New issues:
- [Medium] Newly added coefficient row for `mclk=12288000`, `rate=192000` is artificially restricted to a single voltage.

Pre-existing issues:
- [High] Modulo arithmetic bug in `get_coeff()` causes `0` to act as a wildcard, rendering specific configuration rows unreachable.
- [High] Silent failure in `es8389_pcm_hw_params()` when clock coefficients are not found.
--

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

    ASoC: codecs: ES8389: Modify the clock table
    
    Updated the configuration for certain frequencies

> diff --git a/sound/soc/codecs/es8389.c b/sound/soc/codecs/es8389.c
> index 7a9d5d42a05d0..6ad0b580068cf 100644
> --- a/sound/soc/codecs/es8389.c
> +++ b/sound/soc/codecs/es8389.c
> @@ -434,52 +434,54 @@ static const struct _coeff_div  coeff_div[] = {

[ ... ]

>  	{128, 24576000, 192000, 0x00, 0x50, 0x00, 0xC0, 0x18, 0xC1, 0x81, 0xC0, 0x00, 0x8F, 0x7F, 0xBF, 0xC0, 0x3F, 0x7F, 0x80, 0x12, 0xC0, 0x3F, 0xF9, 0x3F, 2, 2},
> +	{64, 12288000, 192000, 0x00, 0x41, 0x00, 0xC0, 0x18, 0xC1, 0x80, 0x00, 0x00, 0x8F, 0x7F, 0xEF, 0xC0, 0x7F, 0x7F, 0x80, 0x12, 0xC0, 0x3F, 0xF9, 0x3F, 1, 0},

[Severity: Medium]
Is it intentional to restrict this new row to dvdd_vol=1 and dmic_sel=0, rather
than using the wildcard values (2, 2) like the other 192000Hz row?

According to the logic in get_coeff(), dvdd_vol=1 strictly requires vddd=1
(ES8389_3V3). If the device is running on 1.8V (vddd=0), get_coeff() will
reject this row and return -EINVAL. Could this break 192kHz playback on 1.8V
systems?

>  };
>  
>  static inline int get_coeff(u8 vddd, u8 dmic, int mclk, int rate)

[Severity: High]
This is a pre-existing issue, but does the bitwise and modulo arithmetic here
unintentionally cause 0 to act as a wildcard?

In get_coeff():

	vddd_det |= ~(coeff_div[i].dvdd_vol % 2) & 0x01;
	dmic_det |= ~(coeff_div[i].dmic_sel % 2) & 0x01;

Because 0 % 2 == 0, the expression ~0 & 1 evaluates to 1. This means any row
with dmic_sel == 0 or dvdd_vol == 0 acts as a wildcard and matches regardless
of the actual requested dmic or vddd values.

For configurations where a 0 row precedes a 1 row for the same frequency, like:

	{50, 400000, 8000, ..., 2, 0},
	{50, 400000, 8000, ..., 2, 1},

get_coeff() will always return the first row (the 0 configuration), making the
second row completely unreachable and preventing correct hardware setup when
dmic=1 is requested.

[Severity: High]
This is also a pre-existing issue, but when get_coeff() fails to find a
matching row and returns -EINVAL, es8389_pcm_hw_params() does not propagate the
error:

	coeff = get_coeff(es8389->vddd, dmic_enable, es8389->sysclk, params_rate(params));
	if (coeff >= 0) {
		regmap_write(...);
		...
	} else {
		dev_warn(component->dev, "Clock coefficients do not match");
	}

	return 0;

Returning 0 causes ALSA to continue preparing and triggering the PCM stream
without configuring the codec hardware properly. Could this lead to undefined
behavior or silent audio failures?

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

[PATCH 4/7] ASoC: codecs: ES8389: Modify the initial configuration

[Zhang Yi]

Modify the initial configuration

Signed-off-by: Zhang Yi <zhangyi@everest-semi.com>
---
 sound/soc/codecs/es8389.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/soc/codecs/es8389.c b/sound/soc/codecs/es8389.c
index 6ad0b5800..8e47b4e32 100644
--- a/sound/soc/codecs/es8389.c
+++ b/sound/soc/codecs/es8389.c
@@ -792,7 +792,7 @@ static void es8389_init(struct snd_soc_component *component)
 
 	regmap_read(es8389->regmap, ES8389_MAX_REGISTER, &reg);
 	es8389->version = reg;
-	regmap_write(es8389->regmap, ES8389_ISO_CTL, 0x00);
+	regmap_write(es8389->regmap, ES8389_ISO_CTL, 0x56);
 	regmap_write(es8389->regmap, ES8389_RESET, 0x7E);
 	regmap_write(es8389->regmap, ES8389_ISO_CTL, 0x38);
 	regmap_write(es8389->regmap, ES8389_ADC_HPF1, 0x64);
@@ -844,7 +844,7 @@ static void es8389_init(struct snd_soc_component *component)
 	regmap_write(es8389->regmap, ES8389_SCLK_DIV, 0x04);
 	regmap_write(es8389->regmap, ES8389_LRCK_DIV1, 0x01);
 	regmap_write(es8389->regmap, ES8389_LRCK_DIV2, 0x00);
-	regmap_write(es8389->regmap, ES8389_OSC_CLK, 0x00);
+	regmap_write(es8389->regmap, ES8389_OSC_CLK, 0x10);
 	regmap_write(es8389->regmap, ES8389_ADC_OSR, 0x1F);
 	regmap_write(es8389->regmap, ES8389_ADC_DSP, 0x7F);
 	regmap_write(es8389->regmap, ES8389_ADC_MUTE, 0xC0);
-- 
2.17.1

[PATCH 5/7] ASoC: codecs: ES8389: Add private members related to HPF

[Zhang Yi]

Add private members related to HPF.
And defines ES8389_HPF_DEFAULT

Signed-off-by: Zhang Yi <zhangyi@everest-semi.com>
---
 sound/soc/codecs/es8389.c | 18 ++++++++++++++++--
 sound/soc/codecs/es8389.h |  1 +
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/sound/soc/codecs/es8389.c b/sound/soc/codecs/es8389.c
index 8e47b4e32..d1bcbbfcd 100644
--- a/sound/soc/codecs/es8389.c
+++ b/sound/soc/codecs/es8389.c
@@ -36,6 +36,8 @@ struct	es8389_private {
 	unsigned int sysclk;
 	int mastermode;
 
+	u8 hpfl;
+	u8 hpfr;
 	u8 vddd;
 	bool mclk_src;
 	int version;
@@ -742,8 +744,8 @@ static int es8389_mute(struct snd_soc_dai *dai, int mute, int direction)
 			regmap_update_bits(es8389->regmap, ES8389_DAC_FORMAT_MUTE,
 						0x03, 0x00);
 		} else {
-			regmap_update_bits(es8389->regmap, ES8389_ADC_HPF1, 0x0f, 0x0a);
-			regmap_update_bits(es8389->regmap, ES8389_ADC_HPF2, 0x0f, 0x0a);
+			regmap_update_bits(es8389->regmap, ES8389_ADC_HPF1, 0x0f, es8389->hpfl);
+			regmap_update_bits(es8389->regmap, ES8389_ADC_HPF2, 0x0f, es8389->hpfr);
 			regmap_update_bits(es8389->regmap, ES8389_ADC_FORMAT_MUTE,
 						0x03, 0x00);
 		}
@@ -901,6 +903,18 @@ static int es8389_probe(struct snd_soc_component *component)
 
 	es8389->mclk_src = device_property_read_bool(component->dev, "everest,mclk-from-sclk");
 
+	ret = device_property_read_u8(component->dev, "everest,hpfl", &es8389->hpfl);
+	if (ret != 0) {
+		dev_dbg(component->dev, "hpfl return %d\n", ret);
+		es8389->hpfl = ES8389_HPF_DEFAULT;
+	}
+
+	ret = device_property_read_u8(component->dev, "everest,hpfr", &es8389->hpfr);
+	if (ret != 0) {
+		dev_dbg(component->dev, "hpfr return %d\n", ret);
+		es8389->hpfr = ES8389_HPF_DEFAULT;
+	}
+
 	for (i = 0; i < ARRAY_SIZE(es8389_core_supplies); i++)
 		es8389->core_supply[i].supply = es8389_core_supplies[i];
 	ret = devm_regulator_bulk_get(component->dev, ARRAY_SIZE(es8389_core_supplies), es8389->core_supply);
diff --git a/sound/soc/codecs/es8389.h b/sound/soc/codecs/es8389.h
index 7f1212219..81353dcec 100644
--- a/sound/soc/codecs/es8389.h
+++ b/sound/soc/codecs/es8389.h
@@ -105,6 +105,7 @@
 
 #define ES8389_MIC_SEL_MASK           (7 << 4)
 #define ES8389_MIC_DEFAULT            (1 << 4)
+#define ES8389_HPF_DEFAULT            (0x0A << 0)
 
 #define ES8389_MASTER_MODE_EN         (1 << 0)
 
-- 
2.17.1

Re: [PATCH 5/7] ASoC: codecs: ES8389: Add private members related to HPF

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] User-configured ALSA controls are silently clobbered with hardcoded defaults during resume from suspend, leading to state desynchronization.
--

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

ASoC: codecs: ES8389: Add private members related to HPF
    
Add private members related to HPF.
And defines ES8389_HPF_DEFAULT

> diff --git a/sound/soc/codecs/es8389.c b/sound/soc/codecs/es8389.c
> index 8e47b4e3214e0..d1bcbbfcd4754 100644
> --- a/sound/soc/codecs/es8389.c
> +++ b/sound/soc/codecs/es8389.c

[ ... ]

> @@ -742,8 +744,8 @@ static int es8389_mute(struct snd_soc_dai *dai, int mute, int direction)
>  			regmap_update_bits(es8389->regmap, ES8389_DAC_FORMAT_MUTE,
>  						0x03, 0x00);
>  		} else {
> -			regmap_update_bits(es8389->regmap, ES8389_ADC_HPF1, 0x0f, 0x0a);
> -			regmap_update_bits(es8389->regmap, ES8389_ADC_HPF2, 0x0f, 0x0a);
> +			regmap_update_bits(es8389->regmap, ES8389_ADC_HPF1, 0x0f, es8389->hpfl);
> +			regmap_update_bits(es8389->regmap, ES8389_ADC_HPF2, 0x0f, es8389->hpfr);

[Severity: High]
Does this interact safely with the resume path?

If the "ASoC: codecs: ES8389: Modify volatile_register" patch in this series
marks control registers (like ES8389_ADC_HPF2, ES8389_DAC_INV, and
ES8389_ADC_MUTE) as non-volatile, they are placed into the regmap cache for
restoration across suspend/resume.

However, in es8389_resume(), regcache_cache_bypass() is disabled before
calling es8389_init():

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

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

es8389_init() then uses regmap_write() to forcefully initialize these
registers with hardcoded defaults. Because the cache is active, these writes
will overwrite the user's previously set values in the regcache:

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

When regcache_sync() executes immediately afterward in es8389_resume(), it
syncs this clobbered cache to the hardware.

Could this cause user-configured ALSA controls (such as silent unmute of the
ADC OSR Volume Switch, clearing output inversions) to be permanently lost on
resume without any ALSA events being generated?

>  			regmap_update_bits(es8389->regmap, ES8389_ADC_FORMAT_MUTE,
>  						0x03, 0x00);
>  		}

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

[PATCH 6/7] ASoC: codecs: ES8389: Add INPUTL MUX and INPUTR MUX

[Zhang Yi]

Add INPUTL MUX and INPUTR MUX in route

Signed-off-by: Zhang Yi <zhangyi@everest-semi.com>
---
 sound/soc/codecs/es8389.c | 35 +++++++++++++++++++++++++++++++++--
 1 file changed, 33 insertions(+), 2 deletions(-)

diff --git a/sound/soc/codecs/es8389.c b/sound/soc/codecs/es8389.c
index d1bcbbfcd..5e4afafd9 100644
--- a/sound/soc/codecs/es8389.c
+++ b/sound/soc/codecs/es8389.c
@@ -164,6 +164,16 @@ static const struct soc_enum alc_ramprate =
 static const struct soc_enum alc_winsize =
 	SOC_ENUM_SINGLE(ES8389_ALC_CTL, 0, 16, winsize);
 
+static const char *const es8389_adcl_mux_txt[] = {
+	"Normal",
+	"ADC2 channel to ADC1 channel",
+};
+
+static const char *const es8389_adcr_mux_txt[] = {
+	"Normal",
+	"ADC1 channel to ADC2 channel",
+};
+
 static const char *const es8389_outl_mux_txt[] = {
 	"Normal",
 	"DAC2 channel to DAC1 channel",
@@ -191,6 +201,20 @@ static const unsigned int es8389_pga_values[] = {
 	1, 5, 6
 };
 
+static const struct soc_enum es8389_adcl_mux_enum =
+	SOC_ENUM_SINGLE(ES8389_ADC_MODE, 5,
+			ARRAY_SIZE(es8389_adcl_mux_txt), es8389_adcl_mux_txt);
+
+static const struct snd_kcontrol_new es8389_adcl_mux_controls =
+	SOC_DAPM_ENUM("INPUTL MUX", es8389_adcl_mux_enum);
+
+static const struct soc_enum es8389_adcr_mux_enum =
+	SOC_ENUM_SINGLE(ES8389_ADC_MODE, 4,
+			ARRAY_SIZE(es8389_adcr_mux_txt), es8389_adcr_mux_txt);
+
+static const struct snd_kcontrol_new es8389_adcr_mux_controls =
+	SOC_DAPM_ENUM("INPUTR MUX", es8389_adcr_mux_enum);
+
 static const struct soc_enum es8389_outl_mux_enum =
 	SOC_ENUM_SINGLE(ES8389_DAC_MIX, 5,
 			ARRAY_SIZE(es8389_outl_mux_txt), es8389_outl_mux_txt);
@@ -319,6 +343,8 @@ static const struct snd_soc_dapm_widget es8389_dapm_widgets[] = {
 			   &es8389_adc_mixer_controls[0],
 			   ARRAY_SIZE(es8389_adc_mixer_controls)),
 	SND_SOC_DAPM_MUX("ADC MUX", SND_SOC_NOPM, 0, 0, &es8389_dmic_mux_controls),
+	SND_SOC_DAPM_MUX("INPUTL MUX", SND_SOC_NOPM, 0, 0, &es8389_adcl_mux_controls),
+	SND_SOC_DAPM_MUX("INPUTR MUX", SND_SOC_NOPM, 0, 0, &es8389_adcr_mux_controls),
 
 	SND_SOC_DAPM_MUX("OUTL MUX", SND_SOC_NOPM, 0, 0, &es8389_outl_mux_controls),
 	SND_SOC_DAPM_MUX("OUTR MUX", SND_SOC_NOPM, 0, 0, &es8389_outr_mux_controls),
@@ -332,10 +358,15 @@ static const struct snd_soc_dapm_route es8389_dapm_routes[] = {
 	{"ADCL", NULL, "PGAL"},
 	{"ADCR", NULL, "PGAR"},
 
+	{"INPUTL MUX", "Normal", "ADCL"},
+	{"INPUTL MUX", "ADC2 channel to ADC1 channel", "ADCR"},
+	{"INPUTR MUX", "Normal", "ADCR"},
+	{"INPUTR MUX", "ADC1 channel to ADC2 channel", "ADCL"},
+
 	{"ADC Mixer", "DACL ADCL Mixer", "DACL"},
 	{"ADC Mixer", "DACR ADCR Mixer", "DACR"},
-	{"ADC Mixer", NULL, "ADCL"},
-	{"ADC Mixer", NULL, "ADCR"},
+	{"ADC Mixer", NULL, "INPUTL MUX"},
+	{"ADC Mixer", NULL, "INPUTR MUX"},
 
 	{"ADC MUX", "AMIC", "ADC Mixer"},
 	{"ADC MUX", "DMIC", "DMIC"},
-- 
2.17.1

Re: [PATCH 6/7] ASoC: codecs: ES8389: Add INPUTL MUX and INPUTR MUX

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The newly added INPUTL MUX and INPUTR MUX controls will lose their user-configured state upon system resume, defaulting back to 'Normal' and causing the ALSA DAPM graph to desync from the physical hardware state.
--

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

ASoC: codecs: ES8389: Add INPUTL MUX and INPUTR MUX

This commit adds INPUTL MUX and INPUTR MUX controls to the audio routing
graph for the ES8389 codec.

> diff --git a/sound/soc/codecs/es8389.c b/sound/soc/codecs/es8389.c
> index d1bcbbfcd4754..5e4afafd9be63 100644
> --- a/sound/soc/codecs/es8389.c
> +++ b/sound/soc/codecs/es8389.c

[ ... ]

> @@ -191,6 +201,20 @@ static const unsigned int es8389_pga_values[] = {
>  	1, 5, 6
>  };
>  
> +static const struct soc_enum es8389_adcl_mux_enum =
> +	SOC_ENUM_SINGLE(ES8389_ADC_MODE, 5,
> +			ARRAY_SIZE(es8389_adcl_mux_txt), es8389_adcl_mux_txt);
> +
> +static const struct snd_kcontrol_new es8389_adcl_mux_controls =
> +	SOC_DAPM_ENUM("INPUTL MUX", es8389_adcl_mux_enum);
> +
> +static const struct soc_enum es8389_adcr_mux_enum =
> +	SOC_ENUM_SINGLE(ES8389_ADC_MODE, 4,
> +			ARRAY_SIZE(es8389_adcr_mux_txt), es8389_adcr_mux_txt);

[Severity: High]
Will the user's MUX selection be lost during system suspend and resume?

These MUX controls are backed by the ES8389_ADC_MODE register. If this
register is treated as volatile by the regmap subsystem, its value will not
be saved in the cache.

During system resume, if power was lost, es8389_init() unconditionally
clears bits 4 and 5 of ES8389_ADC_MODE, forcing the MUX back to 'Normal'.
Because the register is volatile, regcache_sync() will skip it and fail to
restore the user's selected configuration.

Does this cause the ALSA DAPM graph to desynchronize from the physical
hardware state, potentially resulting in incorrect audio routing until the
control is manually toggled again?

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

[PATCH 7/7] ASoC: dt-bindings: ES8389: Add members about HPF

[Zhang Yi]

Add members related to HPF

Signed-off-by: Zhang Yi <zhangyi@everest-semi.com>
---
 .../bindings/sound/everest,es8389.yaml        | 23 +++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/everest,es8389.yaml b/Documentation/devicetree/bindings/sound/everest,es8389.yaml
index 75ce0bc48..be92014c0 100644
--- a/Documentation/devicetree/bindings/sound/everest,es8389.yaml
+++ b/Documentation/devicetree/bindings/sound/everest,es8389.yaml
@@ -38,6 +38,27 @@ properties:
     description:
       Interface power supply.
 
+  everest,mclk-from-sclk:
+    $ref: /schemas/types.yaml#/definitions/flag
+    description:
+      Indicates that SCLK is used as the internal clock.
+
+  everest,hpfl:
+    $ref: /schemas/types.yaml#/definitions/uint8
+    description:
+      the HPF value of ADCL.
+    minimum: 0x00
+    maximum: 0x0f
+    default: 0x0a
+
+  everest,hpfr:
+    $ref: /schemas/types.yaml#/definitions/uint8
+    description:
+      the HPF value of ADCR.
+    minimum: 0x00
+    maximum: 0x0f
+    default: 0x0a
+
 required:
   - compatible
   - reg
@@ -58,5 +79,7 @@ examples:
         #sound-dai-cells = <0>;
         vddd-supply = <&vdd3v3>;
         vdda-supply = <&vdd3v3>;
+        everest,hpfl = [0a];
+        everest,hpfr = [0a];
       };
     };
-- 
2.17.1

Re: [PATCH 7/7] ASoC: dt-bindings: ES8389: Add members about HPF

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The patch adds a clock-related property (`everest,mclk-from-sclk`) without documenting it in the commit message. The commit message explicitly states the intent is to 'Add members about HPF' (High-Pass Filter), completely omitting the new clock configuration property.
--

commit 1602f24c0f54f1d184d3693108b41af9cf3c92cb
Author: Zhang Yi <zhangyi@everest-semi.com>

ASoC: dt-bindings: ES8389: Add members about HPF

This commit updates the ES8389 device tree binding to add new properties
related to the High-Pass Filter (HPF).

> diff --git a/Documentation/devicetree/bindings/sound/everest,es8389.yaml b/Documentation/devicetree/bindings/sound/everest,es8389.yaml
> index 75ce0bc489045..be92014c0c597 100644
> --- a/Documentation/devicetree/bindings/sound/everest,es8389.yaml
> +++ b/Documentation/devicetree/bindings/sound/everest,es8389.yaml
> @@ -38,6 +38,27 @@ properties:
>      description:
>        Interface power supply.
>  
> +  everest,mclk-from-sclk:
> +    $ref: /schemas/types.yaml#/definitions/flag
> +    description:
> +      Indicates that SCLK is used as the internal clock.
> +

[Severity: Medium]
The commit message notes that this patch adds members related to HPF, but
this property appears to configure the clock. Is it intentional to include
these clock configuration changes in this patch, or should the commit message
be updated to reflect the addition of everest,mclk-from-sclk?

[ ... ]

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

[PATCH 0/7] ASoC: codecs: ES8389: Modify the ES8389 driver

[Zhang Yi]

v0:
  - Modify volatile_register
  - Add MCLK_MASK to resolve the issue related to mclk_src
  - Modify configuration about clock
  - Add private members about HPF at driver and devicetree
  - Add INPUT MUX at route

Zhang Yi (7):
  ASoC: codecs: ES8389: Modify volatile_register
  ASoC: codecs: ES8389: Fix the issue about mclk_src
  ASoC: codecs: ES8389: Modify the clock table
  ASoC: codecs: ES8389: Modify the initial configuration
  ASoC: codecs: ES8389: Add private members related to HPF
  ASoC: codecs: ES8389: Add INPUTL MUX and INPUTR MUX
  ASoC: dt-bindings: ES8389: Add members about HPF and clock

 .../bindings/sound/everest,es8389.yaml        |  23 +++
 sound/soc/codecs/es8389.c                     | 136 +++++++++++++-----
 sound/soc/codecs/es8389.h                     |   6 +-
 3 files changed, 125 insertions(+), 40 deletions(-)

-- 
2.17.1

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

[Zhang Yi]

Mark some registers that are not volatile as false

Signed-off-by: Zhang Yi <zhangyi@everest-semi.com>
---
 sound/soc/codecs/es8389.c | 25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/sound/soc/codecs/es8389.c b/sound/soc/codecs/es8389.c
index 449d9574b..9c8164916 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:
+	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;
+	}
 }
 
 static const DECLARE_TLV_DB_SCALE(dac_vol_tlv, -9550, 50, 0);
-- 
2.17.1

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

[sashiko-bot]

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

New issues:
- [High] The regmap cache for non-volatile registers is overwritten with hardcoded default values during resume if the device lost power, neutralizing the patch's fix.

Pre-existing issues:
- [High] Critical configuration registers modified during stream setup are marked as volatile, causing them to be lost upon suspend/resume.
- [High] The variable `regv` is evaluated uninitialized if `regmap_read` fails during resume.
--

commit 146c46405cf59e3e0362628efb42fd4f52cff71a
Author: Zhang Yi <zhangyi@everest-semi.com>

ASoC: codecs: ES8389: Modify volatile_register

This patch modifies es8389_volatile_register() to mark a specific set of 
registers as non-volatile, enabling regcache for them. All other registers 
now default to being volatile.

> 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_ADC_RESET:
>  		return false;
> +	default:
> +		return true;

[Severity: High]
This is a pre-existing issue, but does this fall-through prevent critical
configuration registers from being restored by regcache_sync()?

Critical configuration registers modified during stream setup like
ES8389_DMIC_EN, ES8389_ADC_MODE, ES8389_ADC_FORMAT_MUTE, and ES8389_ADC_OSR
remain uncached.

Since the ALSA framework does not re-invoke hw_params on resume, these
registers must be cached and restored by regcache_sync(). Because they are
volatile, active audio streams will fail or lose routing after a
suspend/resume cycle.

> +	}
>  }

[Severity: High]
By newly marking these registers as non-volatile, does this cause the
regcache to be overwritten with defaults during a power-loss resume?

In sound/soc/codecs/es8389.c:es8389_resume(), if the device lost power
(regv == 0xff), es8389_init() is called without regcache_cache_bypass being
enabled:

    regmap_read(es8389->regmap, ES8389_RESET, &regv);
    regcache_cache_bypass(es8389->regmap, false);

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

Inside es8389_init(), direct regmap_write() calls are made:

    regmap_write(es8389->regmap, ES8389_ADC_HPF1, 0x64);

Because bypass is disabled, these writes clobber the newly added regmap cache
with hardcoded default values. The subsequent regcache_sync() then syncs these
defaults instead of the user's prior settings, breaking state restoration.

[Severity: High]
This is a pre-existing issue, but can regv be evaluated uninitialized if the
regmap_read fails during resume?

In sound/soc/codecs/es8389.c:es8389_resume():

    regmap_read(es8389->regmap, ES8389_RESET, &regv);
    regcache_cache_bypass(es8389->regmap, false);

    if (regv == 0xff)

If regmap_read() fails (which can occur during resume due to bus readiness
issues), regv contains uninitialized stack memory. The subsequent evaluation
against 0xff produces undefined behavior, potentially skipping necessary
device initialization and leading to silent failure.

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