Re: [PATCH v2 4/6] ASoC: codecs: msm8916-wcd-analog: add MBHC support

[Damien Riegel]

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@xxxxxxxxxx wrote:
> > > From: Srinivas Kandagatla <srinivas.kandagatla@xxxxxxxxxx>
> > > 
> > > 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.

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.


Regards,
-- 
Damien