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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20170803125355.l5m5mk5utl4nlbwk@workotop.localdomain>
Date:   Thu, 3 Aug 2017 08:53:55 -0400
From:   Damien Riegel <damien.riegel@...oirfairelinux.com>
To:     Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
Cc:     Mark Brown <broonie@...nel.org>,
        Banajit Goswami <bgoswami@...eaurora.org>,
        alsa-devel@...a-project.org, Jaroslav Kysela <perex@...ex.cz>,
        Takashi Iwai <tiwai@...e.com>,
        Patrick Lai <plai@...eaurora.org>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 4/6] ASoC: codecs: msm8916-wcd-analog: add MBHC support

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