[PATCH 0/2] ASoC: es8328: enable capture on the ES8388

[PATCH 0/2] ASoC: es8328: enable capture on the ES8388

[Bob Hui]

The es8328 driver never touches a register at probe time: it enables the
supplies and the clock and leaves the codec in whatever state it is in.
On a board where this driver is the only thing that configures the codec,
userspace ends up dealing with the reset values, and one of them makes the
capture path unusable.

Patch 1 describes the documented power-on values in the regmap, so the
cache matches the chip instead of being filled in lazily on first access.

Patch 2 sets SLRCK on the ES8388.  The chip has a single LRCK pin, and
without SLRCK the ADC never gets a frame clock: capture records digital
silence while playback works, which makes it look like a hardware fault.

Both were found while bringing up a QuarkPi-CA2 (RK3588S, ES8388) on v7.3.
Playback was fine from the start; capture delivered digital silence with no
noise floor at all, which turned out to be the missing frame clock.  The
same board records normally on the vendor 5.10 stack.

The ADC digital volumes still power up at -96 dB.  That is deliberately
left to userspace (UCM or an alsactl state file) rather than baked into the
driver, since the level is a system policy decision.

One documented value does not match the silicon: registers 0x1a/0x1b (DAC
digital volume) are documented to reset to 0xc0, but the ES8388 on this
board reads back 0x28 after a cold boot, and the driver never writes them.
Patch 1 keeps the documented values; if the measured ones are preferred,
those two entries can simply be dropped.

Assisted-by: Hermes:deepseek-v4-flash
Signed-off-by: Bob Hui <bg9oxa@163.com>
---
Bob Hui (2):
      ASoC: es8328: describe the power-on register values in the regmap
      ASoC: es8328: set SLRCK for the ES8388

 sound/soc/codecs/es8328.c | 77 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 77 insertions(+)
---
base-commit: c03c1a14507dfaf8b856c2608f9ac216021fc222
change-id: 20261024-es8328-vol-init-6a52c17897cd

Best regards,
--  
Bob Hui <bg9oxa@163.com>

[PATCH 1/2] ASoC: es8328: describe the power-on register values in the regmap

[Bob Hui]

The driver uses REGCACHE_MAPLE but does not provide reg_defaults, so the
cache starts out empty: a register that has not been written yet is read
back from the hardware and cached at that point. The two then drift
apart whenever something else, such as the bootloader or a previous
kernel, has left the codec in a non-default state, and the control
values reported to userspace no longer describe the chip.

Fill in the reset values from the ES8328 datasheet (Revision 1.0, the
per-register sections 6.1.1 to 6.3.30) so the cache matches what the
codec comes up with on power-up.

Assisted-by: Hermes:deepseek-v4-flash
Signed-off-by: Bob Hui <bg9oxa@163.com>
---
 sound/soc/codecs/es8328.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)

diff --git a/sound/soc/codecs/es8328.c b/sound/soc/codecs/es8328.c
index aaa6646ad4c5e..d7f93de82f69a 100644
--- a/sound/soc/codecs/es8328.c
+++ b/sound/soc/codecs/es8328.c
@@ -856,11 +856,69 @@ static void es8328_remove(struct snd_soc_component *component)
 			       es8328->supplies);
 }
 
+static const struct reg_default es8328_reg_defaults[] = {
+	{ ES8328_CONTROL1,	0x06 },
+	{ ES8328_CONTROL2,	0x1c },
+	{ ES8328_CHIPPOWER,	0xc3 },
+	{ ES8328_ADCPOWER,	0xfc },
+	{ ES8328_DACPOWER,	0xc0 },
+	{ ES8328_CHIPLOPOW1,	0x00 },
+	{ ES8328_CHIPLOPOW2,	0x00 },
+	{ ES8328_ANAVOLMANAG,	0x7c },
+	{ ES8328_MASTERMODE,	0x80 },
+	{ ES8328_ADCCONTROL1,	0x00 },
+	{ ES8328_ADCCONTROL2,	0x00 },
+	{ ES8328_ADCCONTROL3,	0x06 },
+	{ ES8328_ADCCONTROL4,	0x00 },
+	{ ES8328_ADCCONTROL5,	0x06 },
+	{ ES8328_ADCCONTROL6,	0x30 },
+	{ ES8328_ADCCONTROL7,	0x30 },
+	{ ES8328_ADCCONTROL8,	0xc0 },
+	{ ES8328_ADCCONTROL9,	0xc0 },
+	{ ES8328_ADCCONTROL10,	0x38 },
+	{ ES8328_ADCCONTROL11,	0xb0 },
+	{ ES8328_ADCCONTROL12,	0x32 },
+	{ ES8328_ADCCONTROL13,	0x06 },
+	{ ES8328_ADCCONTROL14,	0x00 },
+	{ ES8328_DACCONTROL1,	0x00 },
+	{ ES8328_DACCONTROL2,	0x06 },
+	{ ES8328_DACCONTROL3,	0x32 },
+	{ ES8328_DACCONTROL4,	0xc0 },
+	{ ES8328_DACCONTROL5,	0xc0 },
+	{ ES8328_DACCONTROL6,	0x08 },
+	{ ES8328_DACCONTROL7,	0x06 },
+	{ ES8328_DACCONTROL8,	0x1f },
+	{ ES8328_DACCONTROL9,	0xf7 },
+	{ ES8328_DACCONTROL10,	0xfd },
+	{ ES8328_DACCONTROL11,	0xff },
+	{ ES8328_DACCONTROL12,	0x1f },
+	{ ES8328_DACCONTROL13,	0xf7 },
+	{ ES8328_DACCONTROL14,	0xfd },
+	{ ES8328_DACCONTROL15,	0xff },
+	{ ES8328_DACCONTROL16,	0x00 },
+	{ ES8328_DACCONTROL17,	0x38 },
+	{ ES8328_DACCONTROL18,	0x38 },
+	{ ES8328_DACCONTROL19,	0x38 },
+	{ ES8328_DACCONTROL20,	0x38 },
+	{ ES8328_DACCONTROL21,	0x38 },
+	{ ES8328_DACCONTROL22,	0x38 },
+	{ ES8328_DACCONTROL23,	0x00 },
+	{ ES8328_LOUT1VOL,	0x00 },
+	{ ES8328_ROUT1VOL,	0x00 },
+	{ ES8328_LOUT2VOL,	0x00 },
+	{ ES8328_ROUT2VOL,	0x00 },
+	{ ES8328_MONOOUTVOL,	0x00 },
+	{ ES8328_DACCONTROL29,	0x00 },
+	{ ES8328_DACCONTROL30,	0x00 },
+};
+
 const struct regmap_config es8328_regmap_config = {
 	.reg_bits	= 8,
 	.val_bits	= 8,
 	.max_register	= ES8328_REG_MAX,
 	.cache_type	= REGCACHE_MAPLE,
+	.reg_defaults		= es8328_reg_defaults,
+	.num_reg_defaults	= ARRAY_SIZE(es8328_reg_defaults),
 	.use_single_read = true,
 	.use_single_write = true,
 };

-- 
2.34.1

Re: [PATCH 1/2] ASoC: es8328: describe the power-on register values in the regmap

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] ASoC: es8328: cache/hardware mismatch from reg_defaults without reset
--

commit ba4a54ba4fc04177b18a7410bdd82d04a07933b8
Author: Bob Hui <bg9oxa@163.com>

ASoC: es8328: describe the power-on register values in the regmap

This commit populates es8328_reg_defaults with datasheet power-on reset values
for registers ES8328_CONTROL1 through ES8328_DACCONTROL30 and hooks them up to
es8328_regmap_config.

> diff --git a/sound/soc/codecs/es8328.c b/sound/soc/codecs/es8328.c
> index aaa6646ad4c5e..d7f93de82f69a 100644
> --- a/sound/soc/codecs/es8328.c
> +++ b/sound/soc/codecs/es8328.c
> @@ -856,11 +856,69 @@ static void es8328_remove(struct snd_soc_component *component)
>  			       es8328->supplies);
>  }
>  
> +static const struct reg_default es8328_reg_defaults[] = {
> +	{ ES8328_CONTROL1,	0x06 },
> +	{ ES8328_CONTROL2,	0x1c },
> +	{ ES8328_CHIPPOWER,	0xc3 },
> +	{ ES8328_ADCPOWER,	0xfc },
> +	{ ES8328_DACPOWER,	0xc0 },
> +	{ ES8328_CHIPLOPOW1,	0x00 },
> +	{ ES8328_CHIPLOPOW2,	0x00 },
> +	{ ES8328_ANAVOLMANAG,	0x7c },
> +	{ ES8328_MASTERMODE,	0x80 },
> +	{ ES8328_ADCCONTROL1,	0x00 },
> +	{ ES8328_ADCCONTROL2,	0x00 },
> +	{ ES8328_ADCCONTROL3,	0x06 },
> +	{ ES8328_ADCCONTROL4,	0x00 },
> +	{ ES8328_ADCCONTROL5,	0x06 },
> +	{ ES8328_ADCCONTROL6,	0x30 },
> +	{ ES8328_ADCCONTROL7,	0x30 },
> +	{ ES8328_ADCCONTROL8,	0xc0 },
> +	{ ES8328_ADCCONTROL9,	0xc0 },
> +	{ ES8328_ADCCONTROL10,	0x38 },
> +	{ ES8328_ADCCONTROL11,	0xb0 },
> +	{ ES8328_ADCCONTROL12,	0x32 },
> +	{ ES8328_ADCCONTROL13,	0x06 },
> +	{ ES8328_ADCCONTROL14,	0x00 },
> +	{ ES8328_DACCONTROL1,	0x00 },
> +	{ ES8328_DACCONTROL2,	0x06 },
> +	{ ES8328_DACCONTROL3,	0x32 },
> +	{ ES8328_DACCONTROL4,	0xc0 },
> +	{ ES8328_DACCONTROL5,	0xc0 },
> +	{ ES8328_DACCONTROL6,	0x08 },
> +	{ ES8328_DACCONTROL7,	0x06 },
> +	{ ES8328_DACCONTROL8,	0x1f },
> +	{ ES8328_DACCONTROL9,	0xf7 },
> +	{ ES8328_DACCONTROL10,	0xfd },
> +	{ ES8328_DACCONTROL11,	0xff },
> +	{ ES8328_DACCONTROL12,	0x1f },
> +	{ ES8328_DACCONTROL13,	0xf7 },
> +	{ ES8328_DACCONTROL14,	0xfd },
> +	{ ES8328_DACCONTROL15,	0xff },
> +	{ ES8328_DACCONTROL16,	0x00 },
> +	{ ES8328_DACCONTROL17,	0x38 },
> +	{ ES8328_DACCONTROL18,	0x38 },
> +	{ ES8328_DACCONTROL19,	0x38 },
> +	{ ES8328_DACCONTROL20,	0x38 },
> +	{ ES8328_DACCONTROL21,	0x38 },
> +	{ ES8328_DACCONTROL22,	0x38 },
> +	{ ES8328_DACCONTROL23,	0x00 },
> +	{ ES8328_LOUT1VOL,	0x00 },
> +	{ ES8328_ROUT1VOL,	0x00 },
> +	{ ES8328_LOUT2VOL,	0x00 },
> +	{ ES8328_ROUT2VOL,	0x00 },
> +	{ ES8328_MONOOUTVOL,	0x00 },
> +	{ ES8328_DACCONTROL29,	0x00 },
> +	{ ES8328_DACCONTROL30,	0x00 },
> +};
> +
>  const struct regmap_config es8328_regmap_config = {
>  	.reg_bits	= 8,
>  	.val_bits	= 8,
>  	.max_register	= ES8328_REG_MAX,
>  	.cache_type	= REGCACHE_MAPLE,
> +	.reg_defaults		= es8328_reg_defaults,
> +	.num_reg_defaults	= ARRAY_SIZE(es8328_reg_defaults),

[Severity: High]
Won't adding es8328_reg_defaults to es8328_regmap_config in
sound/soc/codecs/es8328.c without resetting the codec on probe cause the
regmap cache and hardware state to drift apart when the chip is left in a
non-default state?

Without reg_defaults, an empty REGCACHE_MAPLE cache causes _regmap_read() to
miss in regcache_read(), read the live register value from the chip via
map->reg_read(), and cache that value. With es8328_reg_defaults provided,
regcache_init() pre-populates the maple tree cache with power-on reset values
for every register without reading from or writing to the hardware.

However, neither es8328_probe() nor es8328_component_probe() issues a software
reset (such as ES8328_CONTROL1_SCP_RESET) or syncs the cache to hardware, and
regulator_bulk_enable() will not power-cycle the codec if the supplies were
already enabled (for example, by a bootloader, across a warm reboot/kexec, or
across driver unbind/rebind with always-on regulators):

sound/soc/codecs/es8328.c:es8328_component_probe() {
	es8328 = snd_soc_component_get_drvdata(component);

	ret = regulator_bulk_enable(ARRAY_SIZE(es8328->supplies),
				    es8328->supplies);
	...
}

Because all registers now hit in the pre-populated cache, _regmap_read() will
return the datasheet reset values instead of the actual hardware state during
ALSA control reads and initial DAPM widget setup. In addition,
snd_soc_component_update_bits() compares new values against the cached reset
defaults and elides the hardware write whenever they match.

For example, ES8328_DACCONTROL3 is cached as 0x32 (with
ES8328_DACCONTROL3_DACMUTE cleared). If the codec was left muted (0x36) prior
to probe, unmuting in es8328_mute() with mute == 0:

sound/soc/codecs/es8328.c:es8328_mute() {
	return snd_soc_component_update_bits(dai->component, ES8328_DACCONTROL3,
			ES8328_DACCONTROL3_DACMUTE,
			mute ? ES8328_DACCONTROL3_DACMUTE : 0);
}

will see ES8328_DACCONTROL3_DACMUTE already cleared in the cache and skip the
register write, leaving the DAC muted in hardware.

Similarly, in es8328_set_dai_fmt() and es8328_hw_params(), updates to
ES8328_MASTERMODE (cached as 0x80, ES8328_MASTERMODE_MSC), ES8328_DACCONTROL1
(cached as 0x00), ES8328_ADCCONTROL4 (cached as 0x00), and ES8328_DACCONTROL6
(cached as 0x08) will be skipped as no-ops when the requested mode, I2S
format, 24-bit word length, or de-emphasis setting matches the cached default
rather than the state left in the hardware.

Should the driver reset the hardware in es8328_component_probe() so the chip
is guaranteed to match es8328_reg_defaults?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261024-es8328-vol-init-v1-0-efbd24de9639@163.com?part=1

Re: [PATCH 1/2] ASoC: es8328: describe the power-on register values in the regmap

[Bob Hui]

Yes, that can happen, and it is a deliberate trade-off rather than
something this patch alone can rule out.

The values are the reset values from the ES8328 datasheet (Revision 1.0,
sections 6.1.1 to 6.3.30).  They are correct whenever the codec has been
through a power cycle, which is the normal case: probe enables the
supplies and the codec comes up in its reset state.

Where the codec is not power cycled - a warm reboot, kexec, or a driver
rebind on a board whose supplies are always on - the cache can describe a
state the hardware is not in.  The driver does not reset the codec at
probe time and so cannot guarantee otherwise.  In those cases the mixer
state that userspace installs (alsactl or UCM) is what brings the codec
back to a usable state.  Without the register cache the same board still
comes up in whatever state it was left in, the only difference is that
the driver reads that state back instead of assuming the reset values.

The suspend/resume path already forces the two back together:
es8328_resume() calls regcache_mark_dirty() and regcache_sync(), so after
the first suspend/resume cycle the hardware matches the cache again.

One thing worth stating explicitly: the driver also binds
"everest,es8388", and the ES8388 comes up with different reset values in
a handful of registers, so on those parts the cached values do not
describe the hardware.  I can split the table per chip if that is
preferred; it needs the device type plumbed through the i2c/spi match
data.

Re: [PATCH 1/2] ASoC: es8328: describe the power-on register values in the regmap

[Mark Brown]

> The driver uses REGCACHE_MAPLE but does not provide reg_defaults, so the
> cache starts out empty: a register that has not been written yet is read
> back from the hardware and cached at that point. The two then drift
> apart whenever something else, such as the bootloader or a previous
> kernel, has left the codec in a non-default state, and the control
> values reported to userspace no longer describe the chip.

> Fill in the reset values from the ES8328 datasheet (Revision 1.0, the
> per-register sections 6.1.1 to 6.3.30) so the cache matches what the
> codec comes up with on power-up.

The driver should also add a reset of the hardware during I2C probe so
that the device is in a known state always, it looks like we have
ES8328_CONTROL1_SCP_RESET available for that, if not explicitly writing
out the defaults is probably the least bad option.

> +	{ ES8328_CONTROL1,	0x06 },
> +	{ ES8328_CONTROL2,	0x1c },

https://m5stack-doc.oss-cn-shenzhen.aliyuncs.com/1141/ES8388.pdf says
says this is 0x5c.

> +	{ ES8328_DACCONTROL3,	0x32 },

0x22.

> +	{ ES8328_DACCONTROL21,	0x38 },

0x0.  I'd check the other registers too.

>  const struct regmap_config es8328_regmap_config = {
>  	.reg_bits	= 8,
>  	.val_bits	= 8,
>  	.max_register	= ES8328_REG_MAX,
>  	.cache_type	= REGCACHE_MAPLE,
> +	.reg_defaults		= es8328_reg_defaults,

Please keep the indentation consistent.

-- 
Mark Brown <broonie@kernel.org>

[PATCH 2/2] ASoC: es8328: set SLRCK for the ES8388

[Bob Hui]

The ES8388 has a single LRCK pin, so the DAC and the ADC have to take
their frame clock from that one pin.  With SLRCK clear, which is the
power-on default, the capture path never gets a frame clock and records
digital silence while playback keeps working, so boards appear to have a
dead microphone.

Bit 7 of DAC Control 21 (register 0x2b) is SLRCK on the ES8388, but it
is LD2MO, left DAC to the mono mixer, on the ES8328, so only set it for
the ES8388.

Tested on a QuarkPi-CA2 (RK3588S, ES8388): capture goes from digital
silence to working audio.

Assisted-by: Hermes:deepseek-v4-flash
Signed-off-by: Bob Hui <bg9oxa@163.com>
---
 sound/soc/codecs/es8328.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/sound/soc/codecs/es8328.c b/sound/soc/codecs/es8328.c
index d7f93de82f69..415f8c849271 100644
--- a/sound/soc/codecs/es8328.c
+++ b/sound/soc/codecs/es8328.c
@@ -13,6 +13,7 @@
 #include <linux/pm.h>
 #include <linux/regmap.h>
 #include <linux/slab.h>
+#include <linux/of.h>
 #include <linux/regulator/consumer.h>
 #include <sound/core.h>
 #include <sound/initval.h>
@@ -836,8 +837,26 @@ static int es8328_component_probe(struct snd_soc_component *component)
 		goto clk_fail;
 	}
 
+	/*
+	 * The ES8388 has a single LRCK pin, so the DAC and the ADC must
+	 * share it: without SLRCK the capture path never gets a frame
+	 * clock and records digital silence while playback still works.
+	 * Bit 7 of DAC Control 21 is SLRCK on the ES8388 and LD2MO (left
+	 * DAC to the mono mixer) on the ES8328, so write it only here.
+	 */
+	if (of_device_is_compatible(component->dev->of_node,
+				    "everest,es8388")) {
+		ret = snd_soc_component_write(component, ES8328_DACCONTROL21,
+					      0x80);
+		if (ret)
+			goto err_slrck;
+	}
+
 	return 0;
 
+err_slrck:
+	clk_disable_unprepare(es8328->clk);
+
 clk_fail:
 	regulator_bulk_disable(ARRAY_SIZE(es8328->supplies),
 			       es8328->supplies);

-- 
2.34.1

Re: [PATCH 2/2] ASoC: es8328: set SLRCK for the ES8388

[Mark Brown]

> The ES8388 has a single LRCK pin, so the DAC and the ADC have to take
> their frame clock from that one pin.  With SLRCK clear, which is the
> power-on default, the capture path never gets a frame clock and records
> digital silence while playback keeps working, so boards appear to have a
> dead microphone.

> @@ -13,6 +13,7 @@
>  #include <linux/pm.h>
>  #include <linux/regmap.h>
>  #include <linux/slab.h>
> +#include <linux/of.h>

Please keep the includes sorted.

>  
> +	/*
> +	 * The ES8388 has a single LRCK pin, so the DAC and the ADC must
> +	 * share it: without SLRCK the capture path never gets a frame
> +	 * clock and records digital silence while playback still works.
> +	 * Bit 7 of DAC Control 21 is SLRCK on the ES8388 and LD2MO (left
> +	 * DAC to the mono mixer) on the ES8328, so write it only here.
> +	 */
> +	if (of_device_is_compatible(component->dev->of_node,

It'd be better to put an ID in the match tables and then use
i2c_get_match_data() to look up the variant, that way the driver will
work on ACPI systems as well.

> +		ret = snd_soc_component_write(component, ES8328_DACCONTROL21,
> +					      0x80);

It'd be better to add a define for the bit.

-- 
Mark Brown <broonie@kernel.org>