Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751878AbdHCNKX (ORCPT ); Thu, 3 Aug 2017 09:10:23 -0400 Received: from mail-wm0-f43.google.com ([74.125.82.43]:35982 "EHLO mail-wm0-f43.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751214AbdHCNKW (ORCPT ); Thu, 3 Aug 2017 09:10:22 -0400 Subject: Re: [PATCH v2 4/6] ASoC: codecs: msm8916-wcd-analog: add MBHC support To: Damien Riegel Cc: Mark Brown , Banajit Goswami , alsa-devel@alsa-project.org, Jaroslav Kysela , Takashi Iwai , Patrick Lai , linux-kernel@vger.kernel.org References: <20170802170930.26083-1-srinivas.kandagatla@linaro.org> <20170802170930.26083-5-srinivas.kandagatla@linaro.org> <20170802233323.blx36uutaytdexvb@workotop.localdomain> <56b54da2-0e87-a466-6184-ffbd85f631c4@linaro.org> <20170803125355.l5m5mk5utl4nlbwk@workotop.localdomain> From: Srinivas Kandagatla Message-ID: <6fd8f95f-6378-c6bf-acd7-a4e0a9d1cc5e@linaro.org> Date: Thu, 3 Aug 2017 14:10:18 +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: <20170803125355.l5m5mk5utl4nlbwk@workotop.localdomain> 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: 2965 Lines: 79 On 03/08/17 13:53, Damien Riegel wrote: > On Thu, Aug 03, 2017 at 12:11:04PM +0100, Srinivas Kandagatla wrote: >> >> >> On 03/08/17 00:33, Damien Riegel wrote: >>> Hi Srinivas, >>> >>> >>> On Wed, Aug 02, 2017 at 07:09:28PM +0200, srinivas.kandagatla@linaro.org wrote: >>>> From: Srinivas Kandagatla >>>> >>>> MBHC (MultiButton Headset Control) support is available in pm8921 in two >>>> blocks, one to detect mechanical headset insertion and removal and other >>>> block to support headset type detection and 5 button detection and othe >>>> features like impedance calculation. >>>> >>>> This patch adds support to: >>>> 1> Support to NC and NO type of headset Jacks. >>>> 2> Mechanical insertion and detection of headset jack. >>>> 3> Detect a 3 pole Headphone and a 4 pole Headset. >>>> 4> Detect 5 buttons. >>>> >>>> Tested it on DB410c with Audio Mezz board with 4 pole and 3 pole >>>> headset/headphones. >>> >> >>> Good news! This version has better results. I still have an issue where >>> a headphone is reported as a headset when I do this sequence: >> >> Hi Damien, >> >> Thanks for testing.. >> >> >>> 1. connect headset >>> 2. disconnect headset >>> 3. connect headphone >> >>> Following headphone connections/disconnections are reported correctly. >>> Note that I don't press the media key, it's wrongly detected when I pull >>> off the cable. >> >> If you do step 3 slowly there is a chance that we get KEY_MEDIA events, as >> headphone is a 3 pole, during hp inserting we would end up shorting the MIC >> and GND lines on the connector, as 3-pole connectors have longer GND >> pole/connector length, this action is same as pressing MEDIA KEY. >> >> The logic in the driver uses this MEDIA KEY press release as advantage to >> detect the accessory type before hs insertion, but this logic could break if >> we are inserting headphone slowly, which is not a normal usecase. >> >> Any solution to this issue can be always break if we are doing slow >> insertion of hs or we endup making the driver complex. I will think about >> this once again and see how other drivers handle this usecase. > > Understood, and you're right we should keep this driver as simple as > possible. > > I get the KEY_MEDIA event when doing step 2 (disconnecting headset), and > I understand why it's doing that (and I think it's okay if we don't > filter out that spurious event), but we should at least get a KEY_MEDIA > release event at the same time as SW_{MICROPHONE,HEADPHONE}_INSERT > removal. It doesn't really matter if I disconnect it fast or slowly, > sometimes I'll get the KEY_MEDIA, some other times I won't. I will see if I can fix this in a much better way and let you know. > > Shouldn't mbhc_btn0_pressed be reset to false when physical removal is > detected? I think that would solve the issue of headphone being detected > as headset. > Yes, I have considered that change to my next version. > > Regards, >