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

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

[Zhang Yi]

Modify the initialization configuration and routes for ES8389,
and add private members.
Describe the added private members in `devicetree/bindings`

v2 -> v1:
	-Keep the old ABI around mclk_src
	-Describe the member related to the HPF frequency in the DTS.
	-Instead of accessing HPF-related register values from DTS,
	 set a specific frequency and then match the corresponding value in the code.

v1 -> v0:
	-The binding patch has been placed before the corresponding code patch
	-Provide a more detailed description of the binding patch
	-Provide a more detailed description of the patch about HPF

Zhang Yi (7):
  ASoC: dt-bindings: ES8389: Add members about HPF and clock
  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 about HPF
  ASoC: codecs: ES8389: Add INPUTL MUX and INPUTR MUX

 .../bindings/sound/everest,es8389.yaml        |  16 ++
 sound/soc/codecs/es8389.c                     | 184 ++++++++++++++----
 sound/soc/codecs/es8389.h                     |  12 +-
 3 files changed, 174 insertions(+), 38 deletions(-)

-- 
2.17.1

[PATCH v2 1/7] ASoC: dt-bindings: ES8389: Add members about HPF and clock

[Zhang Yi]

Add members related to HPF and mclk_source

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

diff --git a/Documentation/devicetree/bindings/sound/everest,es8389.yaml b/Documentation/devicetree/bindings/sound/everest,es8389.yaml
index 7db84cf11..88fbce7b0 100644
--- a/Documentation/devicetree/bindings/sound/everest,es8389.yaml
+++ b/Documentation/devicetree/bindings/sound/everest,es8389.yaml
@@ -42,6 +42,21 @@ properties:
     description:
       Interface power supply.
 
+  everest,mclk-src:
+    $ref: /schemas/types.yaml#/definitions/uint8
+    description:
+      Indicates that SCLK is used as the internal clock.
+    minimum: 0
+    maximum: 0x01
+    default: 0x00
+
+  everest,hpf-frq:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      The frequency of HPF in Hz.
+    maximum: 1020
+    default: 16
+
 required:
   - compatible
   - reg
@@ -62,5 +77,6 @@ examples:
         #sound-dai-cells = <0>;
         vddd-supply = <&vdd3v3>;
         vdda-supply = <&vdd3v3>;
+        everest,hpf-frq = <16>;
       };
     };
-- 
2.17.1

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Property `everest,mclk-src` uses an unidiomatic `uint8` type to model a boolean flag, which is highly error-prone due to DT endianness.
- [Low] Property `everest,hpf-frq` lacks the standard `-hz` unit suffix and uses an uncommon abbreviation.
--

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

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

Add members related to HPF and mclk_source

> diff --git a/Documentation/devicetree/bindings/sound/everest,es8389.yaml b/Documentation/devicetree/bindings/sound/everest,es8389.yaml
> --- a/Documentation/devicetree/bindings/sound/everest,es8389.yaml
> +++ b/Documentation/devicetree/bindings/sound/everest,es8389.yaml
> @@ -42,6 +42,21 @@ properties:
>      description:
>        Interface power supply.
>  
> +  everest,mclk-src:
> +    $ref: /schemas/types.yaml#/definitions/uint8
> +    description:
> +      Indicates that SCLK is used as the internal clock.
> +    minimum: 0
> +    maximum: 0x01
> +    default: 0x00

[Severity: Medium]
Since this functions conceptually as a boolean toggle, should this be
defined as a standard boolean property instead of a uint8?

If a developer intuitively writes "everest,mclk-src = <1>;" in the Device
Tree, standard angle-bracket integer cells evaluate to 32-bit big-endian
integers (0x00 0x00 0x00 0x01).

When device_property_read_u8() reads this in the driver:
    ret = device_property_read_u8(component->dev, "everest,mclk-src", &es8389->mclk_src);

It will read the first byte (0x00) and silently misinterpret the flag as 0.
To correctly set the flag as a uint8, users would be forced to use the unwieldy
syntax "/bits/ 8 <1>;". Using a boolean property (where presence indicates true)
would prevent this silent configuration failure.

Also, prior discussions upstream noted a bitmask mismatch (0x40) related to this
property, but it was resolved in a subsequent patch in this series ("ASoC:
codecs: ES8389: Fix the issue about mclk_src").

> +
> +  everest,hpf-frq:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      The frequency of HPF in Hz.
> +    maximum: 1020
> +    default: 16

[Severity: Low]
Does this property name need the standard "-hz" unit suffix?

According to Device Tree schema guidelines, properties representing frequencies
should use the "-hz" suffix to be self-documenting (e.g., "everest,hpf-hz").
Additionally, "frq" is a non-standard abbreviation for frequency.

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

[PATCH v2 2/7] ASoC: codecs: ES8389: Modify volatile_register

[Zhang Yi]

Mark some registers that are not volatile as false
And modified the logic for `cache_bypass` during `8389_resume`.

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

diff --git a/sound/soc/codecs/es8389.c b/sound/soc/codecs/es8389.c
index 449d9574b..be7a36f17 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_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:
+	case ES8389_ADC_MODE:
+	case ES8389_DMIC_EN:
 		return false;
+	default:
+		return true;
+	}
 }
 
 static const DECLARE_TLV_DB_SCALE(dac_vol_tlv, -9550, 50, 0);
@@ -861,13 +880,13 @@ static int es8389_resume(struct snd_soc_component *component)
 	regcache_cache_only(es8389->regmap, false);
 	regcache_cache_bypass(es8389->regmap, true);
 	regmap_read(es8389->regmap, ES8389_RESET, &regv);
-	regcache_cache_bypass(es8389->regmap, false);
 
 	if (regv == 0xff)
 		es8389_init(component);
 	else
 		es8389_set_bias_level(component, SND_SOC_BIAS_ON);
 
+	regcache_cache_bypass(es8389->regmap, false);
 	regcache_sync(es8389->regmap);
 
 	return 0;
-- 
2.17.1

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

[sashiko-bot]

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

Pre-existing issues:
- [High] The updated `es8389_volatile_register` list misses critical configuration registers, causing their state to be permanently lost upon resume.
- [High] Variables passed to `regmap_read` are used without checking for a potential error return, which can leave them uninitialized and result in non-deterministic hardware behavior.
- [High] The return value of `es8389_set_bias_level()` is ignored in `es8389_resume()`, potentially leading to silent failures.
--

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

ASoC: codecs: ES8389: Modify volatile_register

This commit updates the volatile register definitions for the ES8389 codec,
switching to an allowlist approach for cached registers. It also adjusts
the cache bypass logic during the resume sequence.

> diff --git a/sound/soc/codecs/es8389.c b/sound/soc/codecs/es8389.c
> index 3484c87853cb4..10de2143f8d8f 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_ADCL_VOL:

[ ... ]

> +	case ES8389_ADC_MODE:
> +	case ES8389_DMIC_EN:
>  		return false;
> +	default:
> +		return true;
> +	}

[Severity: High]
This isn't a bug introduced by this patch, but by defaulting to true, do we
miss critical configuration registers like ES8389_MASTER_MODE and
ES8389_ADC_HPF2?

If these are left as volatile, they are bypassed during regcache_sync().
Since ASoC doesn't automatically re-invoke set_fmt on resume, and
es8389_init() overwrites these registers with reset defaults, does this
permanently lose the configuration across a suspend/resume cycle?

>  }
>  
>  static const DECLARE_TLV_DB_SCALE(dac_vol_tlv, -9550, 50, 0);
> @@ -861,13 +880,13 @@ static int es8389_resume(struct snd_soc_component *component)
>  	regcache_cache_only(es8389->regmap, false);
>  	regcache_cache_bypass(es8389->regmap, true);
>  	regmap_read(es8389->regmap, ES8389_RESET, &regv);

[Severity: High]
This is a pre-existing issue, but should the return value of regmap_read()
be checked here?

If the underlying I2C/SPI read fails, regv remains uninitialized. This
could result in non-deterministic hardware behavior since the resume
sequence would evaluate stack garbage in the if (regv == 0xff) check below.

This also appears in other places, for example in es8389_init():

    regmap_read(es8389->regmap, ES8389_MAX_REGISTER, &reg);
    es8389->version = reg;

> -	regcache_cache_bypass(es8389->regmap, false);
>  
>  	if (regv == 0xff)
>  		es8389_init(component);
>  	else
>  		es8389_set_bias_level(component, SND_SOC_BIAS_ON);

[Severity: High]
This is also a pre-existing issue, but what happens if es8389_set_bias_level()
encounters an error?

If clk_prepare_enable() fails inside es8389_set_bias_level(), it returns
an error code. Since that return value is ignored here, will the function
silently proceed to regcache_sync() and return success? This might leave
the hardware unpowered while the system falsely believes it resumed
correctly.

>  
> +	regcache_cache_bypass(es8389->regmap, false);
>  	regcache_sync(es8389->regmap);
>  
>  	return 0;

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

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

[Zhang Yi]

Fix the issue with incorrect modifications to mclk_src
When the system needs to be configured to use the MCLK from the SCLK pin,
the code still sets the relevant registers to use the MCLK from the MCLK pin

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

diff --git a/sound/soc/codecs/es8389.c b/sound/soc/codecs/es8389.c
index be7a36f17..7c1d61dd2 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);
 	}
 
diff --git a/sound/soc/codecs/es8389.h b/sound/soc/codecs/es8389.h
index d21e72f87..57bf7c5f8 100644
--- a/sound/soc/codecs/es8389.h
+++ b/sound/soc/codecs/es8389.h
@@ -116,9 +116,11 @@
 #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)
+#define ES8389_MCLK_SOURCE            ES8389_MCLK_PIN
+#define ES8389_MCLK_PIN               0
+#define ES8389_SCLK_PIN               1
 
 /* ES8389_FMT */
 #define ES8389_S24_LE                 (0 << 5)
-- 
2.17.1

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

[sashiko-bot]

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

New issues:
- [Critical] Modifying device tree property interpretation breaks existing configurations.

Pre-existing issues:
- [High] Incorrect BCLK (sysclk) calculation for mono streams.
--

commit 97d350d02faf890bb7862a3fb657b9cbe4613b33
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.
When the system needs to be configured to use the MCLK from the SCLK pin,
the code still sets the relevant registers to use the MCLK from the MCLK
pin.

> diff --git a/sound/soc/codecs/es8389.h b/sound/soc/codecs/es8389.h
> --- a/sound/soc/codecs/es8389.h
> +++ b/sound/soc/codecs/es8389.h
[ ... ]
> @@ -116,9 +116,11 @@
>  #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)
> +#define ES8389_MCLK_SOURCE            ES8389_MCLK_PIN
> +#define ES8389_MCLK_PIN               0
> +#define ES8389_SCLK_PIN               1

[ ... ]

> diff --git a/sound/soc/codecs/es8389.c b/sound/soc/codecs/es8389.c
> --- 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) {

[Severity: Critical]
Does changing this condition and redefining the pin macros break existing
device tree configurations?

Previously in es8389.h, ES8389_SCLK_PIN was 0. If an existing device tree
explicitly selected SCLK using <0>, it would match the old condition.
Because ES8389_SCLK_PIN is now redefined as 1, an existing legacy DT passing
<0> will evaluate as false here and incorrectly fall back to MCLK.

Similarly, if a legacy configuration used <64> for MCLK, it would now
evaluate as true and incorrectly select SCLK. Is there a need for
compatibility fallback logic to prevent ABI breakage for deployed platforms?

>  		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 is a pre-existing issue, but does this calculation result in an
incorrect sysclk for mono streams?

By using params_channels() in es8389_pcm_hw_params(), the physical clock
rate is derived from the logical channel count. For a mono stream, this
returns 1. However, on standard I2S buses, the physical frame always
contains 2 slots (left and right), meaning the actual physical bit clock on
the wire should be 2 * width * rate.

Could this cause the sysclk calculation to be halved, making it fail to
find a matching frequency and breaking mono playback?

>  	}

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

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

[Zhang Yi]

Updated the configuration for certain frequencies
If get_coeff does not find a value that matches the condition, return -EINVAL

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

diff --git a/sound/soc/codecs/es8389.c b/sound/soc/codecs/es8389.c
index 7c1d61dd2..501b3a16b 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)
@@ -657,7 +659,8 @@ static int es8389_pcm_hw_params(struct snd_pcm_substream *substream,
 		regmap_write(es8389->regmap, ES8389_SYSTEM18, coeff_div[coeff].Reg0x18);
 		regmap_write(es8389->regmap, ES8389_SYSTEM19, coeff_div[coeff].Reg0x19);
 	} else {
-		dev_warn(component->dev, "Clock coefficients do not match");
+		dev_err(component->dev, "Clock coefficients do not match");
+		return -EINVAL;
 	}
 
 	return 0;
-- 
2.17.1

[PATCH v2 5/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 501b3a16b..04b6a48b0 100644
--- a/sound/soc/codecs/es8389.c
+++ b/sound/soc/codecs/es8389.c
@@ -793,7 +793,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);
@@ -845,7 +845,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

Re: [PATCH v2 5/7] ASoC: codecs: ES8389: Modify the initial configuration

[sashiko-bot]

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

Pre-existing issues:
- [High] Regulators enabled in component probe are never disabled, leading to a refcount leak on module removal.
- [High] Unbalanced clock prepare/enable calls in bias level handling cause clock refcount underflows.
- [High] The return value of `regmap_read` is ignored, leading to the use of uninitialized stack variables if the I2C read fails.
- [High] If the codec was reset during suspend, es8389_resume re-initializes it but fails to turn on the clocks and bias.
--

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

ASoC: codecs: ES8389: Modify the initial configuration

This commit modifies the initial configuration for the ES8389 codec by
updating the values written to ES8389_ISO_CTL and ES8389_OSC_CLK during
initialization.

> diff --git a/sound/soc/codecs/es8389.c b/sound/soc/codecs/es8389.c
> index dc3410459f39d..09257687bb84c 100644
> --- a/sound/soc/codecs/es8389.c
> +++ b/sound/soc/codecs/es8389.c
> @@ -793,7 +793,7 @@ static void es8389_init(struct snd_soc_component *component)
>  
>  	regmap_read(es8389->regmap, ES8389_MAX_REGISTER, &reg);
>  	es8389->version = reg;

[Severity: High]
This is a pre-existing issue, but what happens if regmap_read() fails here?

If the underlying I2C transfer fails, regmap_read() returns an error and
leaves reg uninitialized. This uninitialized stack variable is then blindly
assigned to es8389->version.

This same pattern occurs in multiple places, such as es8389_resume():

sound/soc/codecs/es8389.c:es8389_resume() {
    ...
	regmap_read(es8389->regmap, ES8389_RESET, &regv);

	if (regv == 0xff)
    ...
}

and es8389_mute():

sound/soc/codecs/es8389.c:es8389_mute() {
    ...
		regmap_read(es8389->regmap, ES8389_CSM_STATE1, &regv);
		if (regv != ES8389_STATE_ON) {
    ...
}

Should the return value of regmap_read() be checked to prevent using
uninitialized stack variables and causing random device state
misconfigurations?

> -	regmap_write(es8389->regmap, ES8389_ISO_CTL, 0x00);
> +	regmap_write(es8389->regmap, ES8389_ISO_CTL, 0x56);

[ ... ]

[Severity: High]
This is a pre-existing issue, but does this driver leak regulator references
on module removal?

In es8389_probe(), the core regulators are enabled:

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

However, es8389_remove() fails to 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);
    ...
}

When the component is unbound, the devres framework will free the regulator
structure while its enable count is unbalanced. Does this cause a regulator
core warning and a permanent resource leak?


[Severity: High]
This is a pre-existing issue, but do we have unbalanced clock prepare/enable
calls in the bias level handling?

es8389_set_bias_level() unconditionally disables mclk in the standby case:

sound/soc/codecs/es8389.c:es8389_set_bias_level() {
    ...
	case SND_SOC_BIAS_STANDBY:
        ...
		clk_disable_unprepare(es8389->mclk);
		break;
    ...
}

Then in es8389_suspend(), the driver explicitly forces a standby transition:

sound/soc/codecs/es8389.c:es8389_suspend() {
    ...
	es8389_set_bias_level(component, SND_SOC_BIAS_STANDBY);
    ...
}

If the ASoC core had already transitioned the bias to standby prior to
component suspend, does this force a double-disable of the clock and cause a
clock underflow?


[Severity: High]
This is a pre-existing issue, but does es8389_resume() correctly bring the
device out of standby?

If the codec was reset during suspend, it re-initializes the codec via
es8389_init():

sound/soc/codecs/es8389.c:es8389_resume() {
    ...
	if (regv == 0xff)
		es8389_init(component);
	else
		es8389_set_bias_level(component, SND_SOC_BIAS_ON);
    ...
}

However, es8389_init() only configures registers without transitioning the
bias state or enabling mclk. By skipping the es8389_set_bias_level(...,
SND_SOC_BIAS_ON) call, does the device remain unclocked and in a powered-off
bias state?

If so, a subsequent ASoC state transition to standby might try to unprepare
the already disabled clock.

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

[PATCH v2 6/7] ASoC: codecs: ES8389: Add private members about HPF

[Zhang Yi]

Add private members related to HPF.
Add a helper function to match the set HPF frequency.
Access the HPF frequency set by the user from the DTS

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

diff --git a/sound/soc/codecs/es8389.c b/sound/soc/codecs/es8389.c
index 04b6a48b0..c80cc9601 100644
--- a/sound/soc/codecs/es8389.c
+++ b/sound/soc/codecs/es8389.c
@@ -36,6 +36,9 @@ struct	es8389_private {
 	unsigned int sysclk;
 	int mastermode;
 
+	u8 hpfl;
+	u8 hpfr;
+	u32 target_freq;
 	u8 mclk_src;
 	u8 vddd;
 	int version;
@@ -504,6 +507,48 @@ static inline int get_coeff(u8 vddd, u8 dmic, int mclk, int rate)
 	return -EINVAL;
 }
 
+static const u32 hpf_table[10][10] = {
+	{1020, 754, 624, 559, 527, 511, 502, 498, 497, 496},
+	{754, 495, 368, 306, 274, 259, 251, 247, 246, 244},
+	{624, 368, 243, 182, 151, 136, 128, 124, 123, 121},
+	{559, 306, 182, 120, 90, 75, 68, 63, 62, 60},
+	{527, 274, 151, 90, 60, 45, 38, 33, 32, 31},
+	{511, 259, 136, 75, 45, 30, 23, 19, 18, 17},
+	{502, 251, 128, 68, 38, 23, 16, 13, 11, 11},
+	{498, 247, 124, 63, 33, 19, 13, 10, 8, 8},
+	{497, 246, 123, 62, 32, 18, 11, 8, 8, 0},
+	{496, 244, 121, 60, 31, 17, 11, 8, 0, 0}
+};
+
+static bool find_best_hpf_freq(u32 target_hz, u8 *hpf1, u8 *hpf2)
+{
+	int best_row = -1, best_col = -1;
+	u32 min_diff = U32_MAX;
+	u32 f, diff;
+	int i, j;
+
+	if ((*hpf1 == ES8389_HPF_INVALID) | (*hpf2 == ES8389_HPF_INVALID))
+		return false;
+
+	for (i = 0; i < 10; i++) {
+		for (j = i; j < 10; j++) {
+			f = hpf_table[i][j];
+
+			diff = (target_hz > f) ? (target_hz - f) : (f - target_hz);
+			if (diff < min_diff) {
+				min_diff = diff;
+				best_row = i;
+				best_col = j;
+			}
+		}
+	}
+
+	*hpf1 = best_col + ES8389_HPF_OFFSET;
+	*hpf2 = best_row + ES8389_HPF_OFFSET;
+
+	return true;
+}
+
 /*
  * if PLL not be used, use internal clk1 for mclk,otherwise, use internal clk2 for PLL source.
  */
@@ -585,6 +630,8 @@ static int es8389_pcm_hw_params(struct snd_pcm_substream *substream,
 	int coeff, ret;
 	u8 dmic_enable, state = 0;
 	unsigned int regv;
+	u32 freq;
+	bool hpf;
 
 	switch (params_format(params)) {
 	case SNDRV_PCM_FORMAT_S16_LE:
@@ -663,6 +710,14 @@ static int es8389_pcm_hw_params(struct snd_pcm_substream *substream,
 		return -EINVAL;
 	}
 
+	freq = (es8389->target_freq * 48000) / params_rate(params);
+	hpf = find_best_hpf_freq(freq, &es8389->hpfl, &es8389->hpfr);
+	if (!hpf) {
+		dev_dbg(component->dev, "Can't find best freq. Use the default setting");
+		es8389->hpfl = ES8389_HPF_DEFAULT;
+		es8389->hpfr = ES8389_HPF_DEFAULT;
+	}
+
 	return 0;
 }
 
@@ -743,8 +798,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);
 		}
@@ -906,6 +961,12 @@ static int es8389_probe(struct snd_soc_component *component)
 		es8389->mclk_src = ES8389_MCLK_SOURCE;
 	}
 
+	ret = device_property_read_u32(component->dev, "everest,hpf-frq", &es8389->target_freq);
+	if (ret != 0) {
+		dev_dbg(component->dev, "hpf-freq return %d\n", ret);
+		es8389->target_freq = ES8389_HPF_INVALID;
+	}
+
 	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 57bf7c5f8..995a3eb58 100644
--- a/sound/soc/codecs/es8389.h
+++ b/sound/soc/codecs/es8389.h
@@ -106,6 +106,10 @@
 #define ES8389_MIC_SEL_MASK           (7 << 4)
 #define ES8389_MIC_DEFAULT            (1 << 4)
 
+#define ES8389_HPF_INVALID            U32_MAX
+#define ES8389_HPF_DEFAULT            (0x0A << 0)
+#define ES8389_HPF_OFFSET             4
+
 #define ES8389_MASTER_MODE_EN         (1 << 0)
 
 #define ES8389_TDM_OFF                (0 << 0)
-- 
2.17.1

Re: [PATCH v2 6/7] ASoC: codecs: ES8389: Add private members about HPF

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] The fallback logic for a missing `everest,hpf-frq` DT property is broken, causing an integer overflow and an impossible `u8` to `U32_MAX` comparison.
- [High] The ADC High-Pass Filter parameters `hpfl` and `hpfr` are used without being initialized during probe, leading to incorrect hardware configuration.
- [High] Playback stream operations silently corrupt the ADC (capture) High-Pass Filter state due to missing stream direction checks.
- [Medium] The newly introduced device tree property `everest,hpf-frq` contains a typo ('frq' instead of 'freq').
--

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

ASoC: codecs: ES8389: Add private members about HPF

This commit adds HPF (High-Pass Filter) related private members to the
ES8389 codec. It includes a helper function to match the set HPF frequency
and accesses the HPF frequency set by the user from the Device Tree.

> diff --git a/sound/soc/codecs/es8389.c b/sound/soc/codecs/es8389.c
> index 09257687bb84c..bcec9d9200dbe 100644
> --- a/sound/soc/codecs/es8389.c
> +++ b/sound/soc/codecs/es8389.c
[ ... ]
> @@ -504,6 +507,48 @@ static inline int get_coeff(u8 vddd, u8 dmic, int mclk, int rate)
[ ... ]
> +static bool find_best_hpf_freq(u32 target_hz, u8 *hpf1, u8 *hpf2)
> +{
> +	int best_row = -1, best_col = -1;
> +	u32 min_diff = U32_MAX;
> +	u32 f, diff;
> +	int i, j;
> +
> +	if ((*hpf1 == ES8389_HPF_INVALID) | (*hpf2 == ES8389_HPF_INVALID))

[Severity: High]
Does this code correctly evaluate the fallback condition?

hpf1 and hpf2 are pointers to u8. Dereferencing them yields a maximum value of
255, so comparing them against ES8389_HPF_INVALID (which is U32_MAX) will
always evaluate to false. This prevents the fallback path from ever executing.

> +		return false;
> +
> +	for (i = 0; i < 10; i++) {
> +		for (j = i; j < 10; j++) {
> +			f = hpf_table[i][j];
[ ... ]
> @@ -663,6 +710,14 @@ static int es8389_pcm_hw_params(struct snd_pcm_substream *substream,
>  		return -EINVAL;
>  	}
>  
> +	freq = (es8389->target_freq * 48000) / params_rate(params);

[Severity: High]
If the Device Tree property is missing, target_freq is set to U32_MAX.
Could this cause an integer overflow here, resulting in a garbage freq value?

Additionally, since the High-Pass Filter is an ADC (Capture) specific feature,
does this code safely avoid overwriting the ADC configuration during playback
stream operations?

If es8389_pcm_hw_params() is called for a playback stream, it appears it
will unconditionally recalculate the capture stream's filter configuration
using the playback rate. Could this update logic be restricted by checking
the stream direction?

> +	hpf = find_best_hpf_freq(freq, &es8389->hpfl, &es8389->hpfr);
> +	if (!hpf) {
> +		dev_dbg(component->dev, "Can't find best freq. Use the default setting");
> +		es8389->hpfl = ES8389_HPF_DEFAULT;
> +		es8389->hpfr = ES8389_HPF_DEFAULT;
> +	}
> +
>  	return 0;
>  }
>  
> @@ -743,8 +798,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]
Are hpfl and hpfr properly initialized before they are used here?

Since they are dynamically allocated and zero-initialized, if es8389_mute()
is called prior to es8389_pcm_hw_params() (for instance during stream startup
or idle states), this will write uninitialized 0x00 values to the hardware
registers instead of the previously hardcoded 0x0A default. 

Should they be explicitly initialized in es8389_probe()?

>  			regmap_update_bits(es8389->regmap, ES8389_ADC_FORMAT_MUTE,
>  						0x03, 0x00);
>  		}
> @@ -906,6 +961,12 @@ static int es8389_probe(struct snd_soc_component *component)
>  		es8389->mclk_src = ES8389_MCLK_SOURCE;
>  	}
>  
> +	ret = device_property_read_u32(component->dev, "everest,hpf-frq", &es8389->target_freq);

[Severity: Medium]
This isn't a bug, but there's a typo in the newly introduced device tree
property name ("frq" instead of "freq"). Since Device Tree bindings become
permanent ABI, could this be corrected before it becomes baked into the kernel?

> +	if (ret != 0) {
> +		dev_dbg(component->dev, "hpf-freq return %d\n", ret);
> +		es8389->target_freq = ES8389_HPF_INVALID;
> +	}
> +
>  	for (i = 0; i < ARRAY_SIZE(es8389_core_supplies); i++)
>  		es8389->core_supply[i].supply = es8389_core_supplies[i];

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

[PATCH v2 7/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 c80cc9601..19f4ced77 100644
--- a/sound/soc/codecs/es8389.c
+++ b/sound/soc/codecs/es8389.c
@@ -165,6 +165,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",
@@ -192,6 +202,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);
@@ -320,6 +344,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),
@@ -333,10 +359,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 v2 7/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:

Pre-existing issues:
- [High] Logic error in get_coeff incorrectly treats 0 as a wildcard for dmic_sel and dvdd_vol, shadowing specific coefficient configurations.
--

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

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

This commit adds INPUTL MUX and INPUTR MUX to the DAPM routing paths for
the ES8389 codec.

> diff --git a/sound/soc/codecs/es8389.c b/sound/soc/codecs/es8389.c
> index bcec9d9200dbe..294951d49432c 100644
> --- a/sound/soc/codecs/es8389.c
> +++ b/sound/soc/codecs/es8389.c

[ ... ]

[Severity: High]
This isn't a bug introduced by this patch, but looking at get_coeff() in
sound/soc/codecs/es8389.c, is there a logic error with how dmic_sel and
dvdd_vol are checked?

sound/soc/codecs/es8389.c:get_coeff() {
	...
	for (i = 0; i < ARRAY_SIZE(coeff_div); i++) {
		if (coeff_div[i].rate == rate && coeff_div[i].mclk == mclk) {
			vddd_det = ~(coeff_div[i].dvdd_vol ^ vddd) & 0x01;
			dmic_det = ~(coeff_div[i].dmic_sel ^ dmic) & 0x01;
			vddd_det |= ~(coeff_div[i].dvdd_vol % 2) & 0x01;
			dmic_det |= ~(coeff_div[i].dmic_sel % 2) & 0x01;

			if (vddd_det && dmic_det)
				return i;
		}
	}
	...
}

Because ~(0 % 2) & 0x01 evaluates to 1, doesn't this calculation cause 0 to
act as a wildcard, evaluating to true regardless of the dmic or vddd
parameters?

If entries with dmic_sel = 0 appear before dmic_sel = 1 in the coeff_div
table, could get_coeff() prematurely return the wildcard match?  This appears
to make the correct clock coefficient entries for dmic = 1 dead code,
potentially resulting in incorrect clock configurations.

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