| Message ID | 20200812094631.4698-4-michael.wei.hong.sit@intel.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 9d4befff5a959e5f2f94357b3554a6929f596e15 |
| Headers | show |
| Series | Codec workaround | expand |
> -----Original Message----- > From: Alsa-devel <alsa-devel-bounces@alsa-project.org> On Behalf > Of Michael Sit Wei Hong > Sent: Wednesday, 12 August, 2020 5:47 PM > To: alsa-devel@alsa-project.org > Cc: Rojewski, Cezary <cezary.rojewski@intel.com>; a- > estrada@ti.com; Shevchenko, Andriy > <andriy.shevchenko@intel.com>; zakkaye@ti.com; tiwai@suse.com; > Sia, Jee Heng <jee.heng.sia@intel.com>; pierre- > louis.bossart@linux.intel.com; liam.r.girdwood@linux.intel.com; > broonie@kernel.org; dmurphy@ti.com > Subject: [PATCH v2 3/3] ASoC: codec: tlv3204: Moving GPIO reset > and add ADC reset > > 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 6c2338ea5d8d..8dcea566b375 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_read(component, > AIC32X4_ADCSETUP); > + snd_soc_component_write(component, > AIC32X4_ADCSETUP, adc_reg | > + AIC32X4_LADC_EN | > AIC32X4_RADC_EN); > + snd_soc_component_write(component, > AIC32X4_ADCSETUP, adc_reg); > + break; > + } > + return 0; > +}; > + > static int mic_bias_event(struct snd_soc_dapm_widget *w, > struct snd_kcontrol *kcontrol, int event) { @@ -434,6 > +456,7 @@ static const struct snd_soc_dapm_widget > aic32x4_dapm_widgets[] = { > SND_SOC_DAPM_SUPPLY("Mic Bias", AIC32X4_MICBIAS, 6, > 0, mic_bias_event, > SND_SOC_DAPM_POST_PMU | > SND_SOC_DAPM_PRE_PMD), > > + SND_SOC_DAPM_POST("ADC Reset", aic32x4_reset_adc), > > SND_SOC_DAPM_OUTPUT("HPL"), > SND_SOC_DAPM_OUTPUT("HPR"), > @@ -665,8 +688,8 @@ static int > aic32x4_set_processing_blocks(struct snd_soc_component > *component, } > > static int aic32x4_setup_clocks(struct snd_soc_component > *component, > - unsigned int sample_rate, unsigned int > channel, > - unsigned int bit_depth) > + unsigned int sample_rate, unsigned > int channel, > + unsigned int bit_depth) > { > u8 aosr; > u16 dosr; > @@ -958,12 +981,6 @@ static int > aic32x4_component_probe(struct snd_soc_component > *component) > if (ret) > return ret; > > - if (gpio_is_valid(aic32x4->rstn_gpio)) { > - ndelay(10); > - gpio_set_value(aic32x4->rstn_gpio, 1); > - mdelay(1); > - } > - > snd_soc_component_write(component, AIC32X4_RESET, > 0x01); > > if (aic32x4->setup) > @@ -1196,10 +1213,6 @@ int aic32x4_probe(struct device *dev, > struct regmap *regmap) > aic32x4->mclk_name = "mclk"; > } > > - ret = aic32x4_register_clocks(dev, aic32x4->mclk_name); > - if (ret) > - return ret; > - > if (gpio_is_valid(aic32x4->rstn_gpio)) { > ret = devm_gpio_request_one(dev, aic32x4- > >rstn_gpio, > GPIOF_OUT_INIT_LOW, > "tlv320aic32x4 rstn"); @@ -1221,6 +1234,16 @@ int > aic32x4_probe(struct device *dev, struct regmap *regmap) > return ret; > } > > + if (gpio_is_valid(aic32x4->rstn_gpio)) { > + ndelay(10); > + gpio_set_value_cansleep(aic32x4->rstn_gpio, 1); > + mdelay(1); > + } > + > + ret = aic32x4_register_clocks(dev, aic32x4->mclk_name); > + if (ret) > + return ret; > + > return 0; > } > EXPORT_SYMBOL(aic32x4_probe); > -- > 2.17.1 Hi everyone, Any comments on this patch set? Thanks, Regards, Michael
diff --git a/sound/soc/codecs/tlv320aic32x4.c b/sound/soc/codecs/tlv320aic32x4.c index 6c2338ea5d8d..8dcea566b375 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_read(component, AIC32X4_ADCSETUP); + snd_soc_component_write(component, AIC32X4_ADCSETUP, adc_reg | + AIC32X4_LADC_EN | AIC32X4_RADC_EN); + snd_soc_component_write(component, AIC32X4_ADCSETUP, adc_reg); + break; + } + return 0; +}; + static int mic_bias_event(struct snd_soc_dapm_widget *w, struct snd_kcontrol *kcontrol, int event) { @@ -434,6 +456,7 @@ static const struct snd_soc_dapm_widget aic32x4_dapm_widgets[] = { SND_SOC_DAPM_SUPPLY("Mic Bias", AIC32X4_MICBIAS, 6, 0, mic_bias_event, SND_SOC_DAPM_POST_PMU | SND_SOC_DAPM_PRE_PMD), + SND_SOC_DAPM_POST("ADC Reset", aic32x4_reset_adc), SND_SOC_DAPM_OUTPUT("HPL"), SND_SOC_DAPM_OUTPUT("HPR"), @@ -665,8 +688,8 @@ static int aic32x4_set_processing_blocks(struct snd_soc_component *component, } static int aic32x4_setup_clocks(struct snd_soc_component *component, - unsigned int sample_rate, unsigned int channel, - unsigned int bit_depth) + unsigned int sample_rate, unsigned int channel, + unsigned int bit_depth) { u8 aosr; u16 dosr; @@ -958,12 +981,6 @@ static int aic32x4_component_probe(struct snd_soc_component *component) if (ret) return ret; - if (gpio_is_valid(aic32x4->rstn_gpio)) { - ndelay(10); - gpio_set_value(aic32x4->rstn_gpio, 1); - mdelay(1); - } - snd_soc_component_write(component, AIC32X4_RESET, 0x01); if (aic32x4->setup) @@ -1196,10 +1213,6 @@ int aic32x4_probe(struct device *dev, struct regmap *regmap) aic32x4->mclk_name = "mclk"; } - ret = aic32x4_register_clocks(dev, aic32x4->mclk_name); - if (ret) - return ret; - if (gpio_is_valid(aic32x4->rstn_gpio)) { ret = devm_gpio_request_one(dev, aic32x4->rstn_gpio, GPIOF_OUT_INIT_LOW, "tlv320aic32x4 rstn"); @@ -1221,6 +1234,16 @@ int aic32x4_probe(struct device *dev, struct regmap *regmap) return ret; } + if (gpio_is_valid(aic32x4->rstn_gpio)) { + ndelay(10); + gpio_set_value_cansleep(aic32x4->rstn_gpio, 1); + mdelay(1); + } + + ret = aic32x4_register_clocks(dev, aic32x4->mclk_name); + if (ret) + return ret; + return 0; } EXPORT_SYMBOL(aic32x4_probe);