From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751749AbdJCUxJ (ORCPT ); Tue, 3 Oct 2017 16:53:09 -0400 Received: from mail.savoirfairelinux.com ([208.88.110.44]:49584 "EHLO mail.savoirfairelinux.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751158AbdJCUxI (ORCPT ); Tue, 3 Oct 2017 16:53:08 -0400 Date: Tue, 3 Oct 2017 16:53:05 -0400 From: Damien Riegel To: Srinivas Kandagatla Cc: alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org, Liam Girdwood , Mark Brown , Jaroslav Kysela , Takashi Iwai , kernel@savoirfairelinux.com Subject: Re: [PATCH v2] ASoC: codecs: msm8916-wcd-analog: configure micbias in mbhc setup Message-ID: <20171003205305.zyazssrvm3boy7yb@workotop.localdomain> References: <20171003132735.29401-1-damien.riegel@savoirfairelinux.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: NeoMutt/20170714 (1.8.3) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Oct 03, 2017 at 09:42:18PM +0100, Srinivas Kandagatla wrote: > > > On 03/10/17 14:27, Damien Riegel wrote: > > The very first time a headset is plugged in, detection is unreliable > > because bias hasn't been configured yet, it's done once a mechanical > > insertion interrupt has been triggered, so following insertions (and > > thus detections) are not affected. > > > > To fix the very first detection, the bias must also be configured in the > > function that setup the MBHC. Move pm8916_wcd_setup_mbhc after > > pm8916_mbhc_configure_bias to avoid a forward declaration. > > > > Signed-off-by: Damien Riegel > > --- > > Changes in v2: > > - squash the two patches in one patch only > > I do not see the squashed patches here?? > It looks like its just the first patch. pm8916_mbhc_configure_bias is now called in pm8916_wcd_setup_mbhc. See inlined comment. > Thanks, > srini > > > > sound/soc/codecs/msm8916-wcd-analog.c | 94 +++++++++++++++++++---------------- > > 1 file changed, 50 insertions(+), 44 deletions(-) > > > > diff --git a/sound/soc/codecs/msm8916-wcd-analog.c b/sound/soc/codecs/msm8916-wcd-analog.c > > index f562f2d86907..71494e9dbdcb 100644 > > --- a/sound/soc/codecs/msm8916-wcd-analog.c > > +++ b/sound/soc/codecs/msm8916-wcd-analog.c > > @@ -443,50 +443,6 @@ static int pm8916_wcd_analog_enable_micbias_int1(struct > > wcd->micbias1_cap_mode); > > } > > -static void pm8916_wcd_setup_mbhc(struct pm8916_wcd_analog_priv *wcd) > > -{ > > - struct snd_soc_codec *codec = wcd->codec; > > - u32 plug_type = 0; > > - u32 int_en_mask; > > - > > - snd_soc_write(codec, CDC_A_MBHC_DET_CTL_1, > > - CDC_A_MBHC_DET_CTL_L_DET_EN | > > - CDC_A_MBHC_DET_CTL_MECH_DET_TYPE_INSERTION | > > - CDC_A_MBHC_DET_CTL_MIC_CLAMP_CTL_AUTO | > > - CDC_A_MBHC_DET_CTL_MBHC_BIAS_EN); > > - > > - if (wcd->hphl_jack_type_normally_open) > > - plug_type |= CDC_A_HPHL_PLUG_TYPE_NO; > > - > > - if (wcd->gnd_jack_type_normally_open) > > - plug_type |= CDC_A_GND_PLUG_TYPE_NO; > > - > > - snd_soc_write(codec, CDC_A_MBHC_DET_CTL_2, > > - CDC_A_MBHC_DET_CTL_HS_L_DET_PULL_UP_CTRL_I_3P0 | > > - CDC_A_MBHC_DET_CTL_HS_L_DET_COMPA_CTRL_V0P9_VDD | > > - plug_type | > > - CDC_A_MBHC_DET_CTL_HPHL_100K_TO_GND_EN); > > - > > - > > - snd_soc_write(codec, CDC_A_MBHC_DBNC_TIMER, > > - CDC_A_MBHC_DBNC_TIMER_INSREM_DBNC_T_256_MS | > > - CDC_A_MBHC_DBNC_TIMER_BTN_DBNC_T_16MS); > > - > > - /* enable MBHC clock */ > > - snd_soc_update_bits(codec, CDC_D_CDC_DIG_CLK_CTL, > > - DIG_CLK_CTL_D_MBHC_CLK_EN_MASK, > > - DIG_CLK_CTL_D_MBHC_CLK_EN); > > - > > - int_en_mask = MBHC_SWITCH_INT; > > - if (wcd->mbhc_btn_enabled) > > - int_en_mask |= MBHC_BUTTON_PRESS_DET | MBHC_BUTTON_RELEASE_DET; > > - > > - snd_soc_update_bits(codec, CDC_D_INT_EN_CLR, int_en_mask, 0); > > - snd_soc_update_bits(codec, CDC_D_INT_EN_SET, int_en_mask, int_en_mask); > > - wcd->mbhc_btn0_released = false; > > - wcd->detect_accessory_type = true; > > -} > > - > > static int pm8916_mbhc_configure_bias(struct pm8916_wcd_analog_priv *priv, > > bool micbias2_enabled) > > { > > @@ -534,6 +490,56 @@ static int pm8916_mbhc_configure_bias(struct pm8916_wcd_analog_priv *priv, > > return 0; > > } > > +static void pm8916_wcd_setup_mbhc(struct pm8916_wcd_analog_priv *wcd) > > +{ > > + struct snd_soc_codec *codec = wcd->codec; > > + bool micbias_enabled = false; > > + u32 plug_type = 0; > > + u32 int_en_mask; > > + > > + snd_soc_write(codec, CDC_A_MBHC_DET_CTL_1, > > + CDC_A_MBHC_DET_CTL_L_DET_EN | > > + CDC_A_MBHC_DET_CTL_MECH_DET_TYPE_INSERTION | > > + CDC_A_MBHC_DET_CTL_MIC_CLAMP_CTL_AUTO | > > + CDC_A_MBHC_DET_CTL_MBHC_BIAS_EN); > > + > > + if (wcd->hphl_jack_type_normally_open) > > + plug_type |= CDC_A_HPHL_PLUG_TYPE_NO; > > + > > + if (wcd->gnd_jack_type_normally_open) > > + plug_type |= CDC_A_GND_PLUG_TYPE_NO; > > + > > + snd_soc_write(codec, CDC_A_MBHC_DET_CTL_2, > > + CDC_A_MBHC_DET_CTL_HS_L_DET_PULL_UP_CTRL_I_3P0 | > > + CDC_A_MBHC_DET_CTL_HS_L_DET_COMPA_CTRL_V0P9_VDD | > > + plug_type | > > + CDC_A_MBHC_DET_CTL_HPHL_100K_TO_GND_EN); > > + > > + > > + snd_soc_write(codec, CDC_A_MBHC_DBNC_TIMER, > > + CDC_A_MBHC_DBNC_TIMER_INSREM_DBNC_T_256_MS | > > + CDC_A_MBHC_DBNC_TIMER_BTN_DBNC_T_16MS); > > + > > + /* enable MBHC clock */ > > + snd_soc_update_bits(codec, CDC_D_CDC_DIG_CLK_CTL, > > + DIG_CLK_CTL_D_MBHC_CLK_EN_MASK, > > + DIG_CLK_CTL_D_MBHC_CLK_EN); > > + > > + if (snd_soc_read(codec, CDC_A_MICB_2_EN) & CDC_A_MICB_2_EN_ENABLE) > > + micbias_enabled = true; > > + > > + pm8916_mbhc_configure_bias(wcd, micbias_enabled); ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Right here. I agree that it's quite easy to miss when most of the diff is a function being moved, with only a few lines added into it. Thanks, -- Damien > > + > > + int_en_mask = MBHC_SWITCH_INT; > > + if (wcd->mbhc_btn_enabled) > > + int_en_mask |= MBHC_BUTTON_PRESS_DET | MBHC_BUTTON_RELEASE_DET; > > + > > + snd_soc_update_bits(codec, CDC_D_INT_EN_CLR, int_en_mask, 0); > > + snd_soc_update_bits(codec, CDC_D_INT_EN_SET, int_en_mask, int_en_mask); > > + wcd->mbhc_btn0_released = false; > > + wcd->detect_accessory_type = true; > > +} > > + > > static int pm8916_wcd_analog_enable_micbias_int2(struct > > snd_soc_dapm_widget > > *w, struct snd_kcontrol > >