RE: [PATCH 3/3] ASoC: codec: tlv3204: Moving GPIO reset and add ADC reset

["Sit, Michael Wei Hong" <michael.wei.hong.sit@intel.com>]

Hi Dan,

The code snippet is taken from the aic32x4_component_probe function of the driver in 5.8.

Could you have a look at that portion and see if it is the same?

Thanks,
Regards,
Michael

-----Original Message-----
From: Dan Murphy <dmurphy@ti.com> 
Sent: Wednesday, 29 July, 2020 8:31 PM
To: Sit, Michael Wei Hong <michael.wei.hong.sit@intel.com>; alsa-devel@alsa-project.org
Cc: tiwai@suse.com; broonie@kernel.org; pierre-louis.bossart@linux.intel.com; Rojewski, Cezary <cezary.rojewski@intel.com>; Shevchenko, Andriy <andriy.shevchenko@intel.com>; liam.r.girdwood@linux.intel.com; Sia, Jee Heng <jee.heng.sia@intel.com>; a-estrada@ti.com; zakkaye@ti.com
Subject: Re: [PATCH 3/3] ASoC: codec: tlv3204: Moving GPIO reset and add ADC reset

Hello

On 7/29/20 2:32 AM, Michael Sit Wei Hong wrote:
> Moving GPIO reset to a later stage and before clock registration to 
> ensure that the host system and codec clocks are in sync. If the host 
> register clock values prior to gpio reset, the last configured codec 
> clock is registered to the host. The codec then gets gpio resetted 
> setting the codec clocks to their default value, causing a mismatch. 
> Host system will skip clock setting thinking the codec clocks are 
> already at the requested rate.
>
> ADC reset is added to ensure the next audio capture does not have 
> undesired artifacts. It is probably related to the original code where 
> the probe function resets the ADC prior to 1st record.
>
> Signed-off-by: Michael Sit Wei Hong <michael.wei.hong.sit@intel.com>
> Reviewed-by: Sia Jee Heng <jee.heng.sia@intel.com>
> Reviewed-by: Pierre-Louis Bossart 
> <pierre-louis.bossart@linux.intel.com>
> ---
>   sound/soc/codecs/tlv320aic32x4.c | 47 ++++++++++++++++++++++++--------
>   1 file changed, 35 insertions(+), 12 deletions(-)
>
> diff --git a/sound/soc/codecs/tlv320aic32x4.c 
> b/sound/soc/codecs/tlv320aic32x4.c
> index 5af438a00f95..37e14558d7c0 100644
> --- a/sound/soc/codecs/tlv320aic32x4.c
> +++ b/sound/soc/codecs/tlv320aic32x4.c
> @@ -50,6 +50,28 @@ struct aic32x4_priv {
>   	struct device *dev;
>   };
>   
> +static int aic32x4_reset_adc(struct snd_soc_dapm_widget *w,
> +			     struct snd_kcontrol *kcontrol, int event) {
> +	struct snd_soc_component *component = snd_soc_dapm_to_component(w->dapm);
> +	u32 adc_reg;
> +
> +	/*
> +	 * Workaround: the datasheet does not mention a required programming
> +	 * sequence but experiments show the ADC needs to be reset after each
> +	 * capture to avoid audible artifacts.
> +	 */
> +	switch (event) {
> +	case SND_SOC_DAPM_POST_PMD:
> +		adc_reg = snd_soc_component_read32(component, AIC32X4_ADCSETUP);

This gives me a build error

sound/soc/codecs/tlv320aic32x4.c: In function ‘aic32x4_reset_adc’:
sound/soc/codecs/tlv320aic32x4.c:66:13: error: implicit declaration of function ‘snd_soc_component_read32’; did you mean ‘snd_soc_component_read’? [-Werror=implicit-function-declaration]
    66 |   adc_reg = snd_soc_component_read32(component, AIC32X4_ADCSETUP);
       |             ^~~~~~~~~~~~~~~~~~~~~~~~
       |             snd_soc_component_read

Also you should check the return to make sure it is valid.

Dan