Re: [PATCH v3 1/2] ASoC: es8328: reset the codec at probe time

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] ASoC: es8328: es8328_component_probe() drops reg_defaults from maple cache
--

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

ASoC: es8328: reset the codec at probe time

This commit issues a control port reset in es8328_component_probe() so the
codec starts from a known state across warm reboots and driver rebinds. It
also adds per-variant register default tables for ES8328 and ES8388 and
selects the matching regmap_config during I2C and SPI probe.

> diff --git a/sound/soc/codecs/es8328.c b/sound/soc/codecs/es8328.c
> index aaa6646ad4c5e..5618fd7e6eb62 100644
> --- a/sound/soc/codecs/es8328.c
> +++ b/sound/soc/codecs/es8328.c
[ ... ]
> @@ -836,6 +837,37 @@ static int es8328_component_probe(struct snd_soc_component *component)
>  		goto clk_fail;
>  	}
>  
> +	/*
> +	 * Put the codec into a known state: the supplies may already be on,
> +	 * a warm reboot or a kexec keeps the registers alive, and a driver
> +	 * rebind leaves whatever the previous instance programmed.  Without
> +	 * this every read-modify-write in the driver works from a value the
> +	 * hardware never had.  SCP_RESET is not self-clearing.
> +	 */
> +	ret = snd_soc_component_write(component, ES8328_CONTROL1,
> +				      ES8328_CONTROL1_SCP_RESET);
> +	if (ret) {
> +		dev_err(component->dev, "unable to reset the codec\n");
> +		clk_disable_unprepare(es8328->clk);
> +		goto clk_fail;
> +	}
> +	usleep_range(1000, 5000);
> +	ret = snd_soc_component_write(component, ES8328_CONTROL1,
> +				      ES8328_CONTROL1_ENREF |
> +				      ES8328_CONTROL1_VMIDSEL_500k);
> +	if (ret) {
> +		dev_err(component->dev, "unable to clear the codec reset\n");
> +		clk_disable_unprepare(es8328->clk);
> +		goto clk_fail;
> +	}
> +
> +	/*
> +	 * The reset put the codec back to its power-on values, so whatever
> +	 * the cache still holds from a previous bind is stale.  Drop it and
> +	 * let the next access read the chip again.
> +	 */
> +	regcache_drop_region(es8328->regmap, 0, ES8328_REG_MAX);

[Severity: Medium]
As reported by Sashiko on v2 [1], does calling regcache_drop_region() in
es8328_component_probe() in sound/soc/codecs/es8328.c wipe out the
es8328_reg_defaults and es8388_reg_defaults tables that were just populated
during regmap initialization?

During es8328_i2c_probe() or es8328_spi_probe(), devm_regmap_init_i2c() and
devm_regmap_init_spi() call regcache_init() -> regcache_maple_populate(),
which populates map->cache with the default register values from
es8328_reg_defaults or es8388_reg_defaults.

When the ASoC card binds the component, es8328_component_probe() runs and
calls:

    regcache_drop_region(es8328->regmap, 0, ES8328_REG_MAX)
      -> regcache_maple_drop()

In drivers/base/regmap/regcache-maple.c, regcache_maple_drop() frees and
erases the maple tree entries in [0, ES8328_REG_MAX] without restoring
map->reg_defaults:

    kfree(entry);
    mas_lock(&mas);
    mas_erase(&mas);

On subsequent register accesses, _regmap_read() in
drivers/base/regmap/regmap.c calls regcache_read() -> regcache_maple_read(),
which finds no entry in map->cache and returns -ENOENT instead of falling
back to map->reg_defaults:

    if (!map->cache_bypass) {
            ret = regcache_read(map, reg, val);
            if (ret == 0)
                    return 0;
    }
    ...
    ret = map->reg_read(context, reg, val);

As a result, every register read or read-modify-write after
es8328_component_probe() misses the cache and performs a physical bus read,
leaving es8328_reg_defaults and es8388_reg_defaults unused after probe. On
3-wire SPI (sound/soc/codecs/es8328-spi.c), where the control interface has
no data-out pin, dropping the cache causes snd_soc_component_update_bits()
to issue physical SPI reads that return bus garbage instead of using the
default values.

Should es8328_component_probe() call regmap_reinit_cache() with the matching
es8388_regmap_config or es8328_regmap_config (selected via es8328->type)
instead of regcache_drop_region()?

[1] https://lore.kernel.org/r/20261031-es8328-vol-init-v2-1-d15b04c22f4f@163.com

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261031-es8328-vol-init-v3-0-7d9c4b7ab67f@163.com?part=1