Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753122AbdIRJIJ (ORCPT ); Mon, 18 Sep 2017 05:08:09 -0400 Received: from mail-wm0-f44.google.com ([74.125.82.44]:46977 "EHLO mail-wm0-f44.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750769AbdIRJIH (ORCPT ); Mon, 18 Sep 2017 05:08:07 -0400 X-Google-Smtp-Source: AOwi7QAsQsXIi89THjoBpm+V5vVJasLMHZwXUt1DAMX1scSWbn4zTBsffiiIGAyW51yxWbkvWRVrww== Subject: Re: [RFC] ASoC: codecs: msm8916-wcd-analog: use btn0 released detection To: Damien Riegel , alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org Cc: Liam Girdwood , Mark Brown , Jaroslav Kysela , Takashi Iwai References: <20170913204355.26186-1-damien.riegel@savoirfairelinux.com> From: Srinivas Kandagatla Message-ID: <3485d646-12fd-4435-1bae-87af9a1a857e@linaro.org> Date: Mon, 18 Sep 2017 10:08:04 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <20170913204355.26186-1-damien.riegel@savoirfairelinux.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3983 Lines: 105 On 13/09/17 21:43, Damien Riegel wrote: > msm8916-wcd-analog uses button0 to differentiate between headphone and > headset. Under some circumstances, button pressed and released > interrupts are not fired as the driver expects it. > This is what we need to understand to find a right solution, I would like to understand on what is the difference in the hw layout. To start with could you share the schematics/connections differences of this area on how the lines are wired up, I would like to compare it with https://www.dropbox.com/s/zqqrqhd9d2yw7pr/Audio%C2%A0Mezzanine%C2%A0Board-SCH-0113.pdf?dl=0 thanks, srini > For instance, with some connectors, there are spurious button-pressed > interrupts when unplugging a headphone, without the corresponding > button-released interrupt. But the codec always alternates between > button pressed and released interrupts, it cannot fire two interrupts of > the same kind in a row. That means that when the headphone is plugged > back, only a button-released interrupt will be fired instead of pressed > then released. This causes the driver to report headphone as headset. > > By changing the logic and relying on button 0 release interrupt, the > driver could be made more robust for connectors that differ from the one > used on the Dragonboard's audio mezzanine. > > Signed-off-by: Damien Riegel > --- > sound/soc/codecs/msm8916-wcd-analog.c | 18 ++++++++---------- > 1 file changed, 8 insertions(+), 10 deletions(-) > > diff --git a/sound/soc/codecs/msm8916-wcd-analog.c b/sound/soc/codecs/msm8916-wcd-analog.c > index 549c269acc7d..f562f2d86907 100644 > --- a/sound/soc/codecs/msm8916-wcd-analog.c > +++ b/sound/soc/codecs/msm8916-wcd-analog.c > @@ -285,7 +285,7 @@ struct pm8916_wcd_analog_priv { > u16 codec_version; > bool mbhc_btn_enabled; > /* special event to detect accessory type */ > - bool mbhc_btn0_pressed; > + int mbhc_btn0_released; > bool detect_accessory_type; > struct clk *mclk; > struct snd_soc_codec *codec; > @@ -483,7 +483,7 @@ static void pm8916_wcd_setup_mbhc(struct pm8916_wcd_analog_priv *wcd) > > 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_pressed = false; > + wcd->mbhc_btn0_released = false; > wcd->detect_accessory_type = true; > } > > @@ -950,7 +950,7 @@ static irqreturn_t mbhc_btn_release_irq_handler(int irq, void *arg) > > /* check if its BTN0 thats released */ > if ((val != -1) && !(val & CDC_A_MBHC_RESULT_1_BTN_RESULT_MASK)) > - priv->mbhc_btn0_pressed = false; > + priv->mbhc_btn0_released = true; > > } else { > snd_soc_jack_report(priv->jack, 0, btn_mask); > @@ -983,9 +983,7 @@ static irqreturn_t mbhc_btn_press_irq_handler(int irq, void *arg) > break; > case 0x0: > /* handle BTN_0 specially for type detection */ > - if (priv->detect_accessory_type) > - priv->mbhc_btn0_pressed = true; > - else > + if (!priv->detect_accessory_type) > snd_soc_jack_report(priv->jack, > SND_JACK_BTN_0, btn_mask); > break; > @@ -1029,19 +1027,19 @@ static irqreturn_t pm8916_mbhc_switch_irq_handler(int irq, void *arg) > * both press and release event received then its > * a headset. > */ > - if (priv->mbhc_btn0_pressed) > + if (priv->mbhc_btn0_released) > snd_soc_jack_report(priv->jack, > - SND_JACK_HEADPHONE, hs_jack_mask); > + SND_JACK_HEADSET, hs_jack_mask); > else > snd_soc_jack_report(priv->jack, > - SND_JACK_HEADSET, hs_jack_mask); > + SND_JACK_HEADPHONE, hs_jack_mask); > > priv->detect_accessory_type = false; > > } else { /* removal */ > snd_soc_jack_report(priv->jack, 0, hs_jack_mask); > priv->detect_accessory_type = true; > - priv->mbhc_btn0_pressed = false; > + priv->mbhc_btn0_released = false; > } > > return IRQ_HANDLED; >