Re: [PATCH 2/4] ASoC: codecs: es8326: Fix power-up sequence
Old power-up sequence causes large pop noise during start-up. Using a new sequence instead.
static const struct regmap_range es8326_volatile_ranges[] = {
regmap_reg_range(ES8326_HP_DETECT, ES8326_HP_DETECT),
regmap_reg_range(ES8326_HPDET_TYPE, ES8326_HPDET_TYPE),
};
As well as not seeming to correspond to the patch description this seems obviously buggy - even if _HPDET_TYPE was missed from the list of volatile registers no software change should be able to make a volatile register non-volatile, that's a property of the hardware. The changes to read from HPDET_TYPE also seem unrelated to the main change here in the resume function, this should probably be multiple commits.
Just a naming space change, will create a new patch for it.
static int es8326_resume(struct snd_soc_component *component) { struct es8326_priv *es8326 = snd_soc_component_get_drvdata(component);
unsigned int reg; regcache_cache_only(es8326->regmap, false); regcache_sync(es8326->regmap);
/* reset register value to default */
regmap_write(es8326->regmap, ES8326_CSM_I2C_STA, 0x01);
usleep_range(1000, 3000);
regmap_write(es8326->regmap, ES8326_CSM_I2C_STA, 0x00);
This looks wrong, you're resyncing the cache and then start resetting registers? It feels like the ordering is off here, and some of this reset sequence might want to be done with the cache bypassed. Are you sure that there's no corruption of user visible state resulting from the power up sequence, especially around the HP driver?
Basically the chip runs through the start-up sequence every time in the resume function. Will move the reset sequence to es8326_suspend
On Thu, Jul 13, 2023 at 10:14:23AM +0800, Zhu Ning wrote:
regcache_cache_only(es8326->regmap, false); regcache_sync(es8326->regmap);
/* reset register value to default */
regmap_write(es8326->regmap, ES8326_CSM_I2C_STA, 0x01);
usleep_range(1000, 3000);
regmap_write(es8326->regmap, ES8326_CSM_I2C_STA, 0x00);
This looks wrong, you're resyncing the cache and then start resetting registers? It feels like the ordering is off here, and some of this reset sequence might want to be done with the cache bypassed. Are you sure that there's no corruption of user visible state resulting from the power up sequence, especially around the HP driver?
Basically the chip runs through the start-up sequence every time in the resume function. Will move the reset sequence to es8326_suspend
That's not going to overwrite any user visible settings?
participants (2)
-
Mark Brown
-
Zhu Ning