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

[Sit, Michael Wei Hong]

> -----Original Message-----
> From: Mark Brown <broonie at kernel.org>
> Sent: Thursday, 30 July, 2020 8:34 PM
> To: Sit, Michael Wei Hong <michael.wei.hong.sit at intel.com>
> Cc: Dan Murphy <dmurphy at ti.com>; alsa-devel at alsa-project.org;
> tiwai at suse.com; pierre-louis.bossart at linux.intel.com; Rojewski, Cezary
> <cezary.rojewski at intel.com>; Shevchenko, Andriy
> <andriy.shevchenko at intel.com>; liam.r.girdwood at linux.intel.com; Sia,
> Jee Heng <jee.heng.sia at intel.com>; a-estrada at ti.com; zakkaye at ti.com
> Subject: Re: [PATCH 3/3] ASoC: codec: tlv3204: Moving GPIO reset and
> add ADC reset
> 
> On Thu, Jul 30, 2020 at 05:46:20AM +0000, Sit, Michael Wei Hong wrote:
> > 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?
> 
> You need to submit code against the current development branch unless
> it's a bug fix for the current release branch, sometimes interfaces change
> in the development version (as is the case here).
> 
> Please don't top post, reply in line with needed context.  This allows
> readers to readily follow the flow of conversation and understand what
> you are talking about and also helps ensure that everything in the
> discussion is being addressed.
> 
> Please fix your mail client to word wrap within paragraphs at something
> substantially less than 80 columns.  Doing this makes your messages much
> easier to read and reply to.

> -----Original Message-----
> From: Alsa-devel <alsa-devel-bounces at alsa-project.org> On Behalf Of Sit,
> Michael Wei Hong
> Sent: Thursday, 30 July, 2020 1:46 PM
> To: Dan Murphy <dmurphy at ti.com>; alsa-devel at alsa-project.org
> Cc: Rojewski, Cezary <cezary.rojewski at intel.com>; a-estrada at ti.com;
> Shevchenko, Andriy <andriy.shevchenko at intel.com>; tiwai at suse.com;
> Sia, Jee Heng <jee.heng.sia at intel.com>; pierre-
> louis.bossart at linux.intel.com; liam.r.girdwood at linux.intel.com;
> broonie at kernel.org; zakkaye at ti.com
> Subject: RE: [PATCH 3/3] ASoC: codec: tlv3204: Moving GPIO reset and
> add ADC reset
> 
> 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 at ti.com>
> Sent: Wednesday, 29 July, 2020 8:31 PM
> To: Sit, Michael Wei Hong <michael.wei.hong.sit at intel.com>; alsa-
> devel at alsa-project.org
> Cc: tiwai at suse.com; broonie at kernel.org; pierre-
> louis.bossart at linux.intel.com; Rojewski, Cezary
> <cezary.rojewski at intel.com>; Shevchenko, Andriy
> <andriy.shevchenko at intel.com>; liam.r.girdwood at linux.intel.com; Sia,
> Jee Heng <jee.heng.sia at intel.com>; a-estrada at ti.com; zakkaye at 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 at intel.com>
> > Reviewed-by: Sia Jee Heng <jee.heng.sia at intel.com>
> > Reviewed-by: Pierre-Louis Bossart
> > <pierre-louis.bossart at 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
> 

Hi Dan,

This code patch is created based on the kernel version 5.8-rc4.
The register reading and writing are copied from the function aic32x4_component_probe.
May I know the version of the kernel you are applying the patch on?

Thanks,
Regards,
Michael