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

[Srinivas Kandagatla]

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@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.

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,