mbox series

[v3,0/5] ASoC: codecs: msm8916-wcd-analog: Add support to MBHC

Message ID 20170809164927.19663-1-srinivas.kandagatla@linaro.org
Headers show
Series ASoC: codecs: msm8916-wcd-analog: Add support to MBHC | expand

Message

Srinivas Kandagatla Aug. 9, 2017, 4:49 p.m. UTC
From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>


This patchset adds support to MBHC(Multibutton headset control) block in PM8916
analog block. MBHC support comes from 2 blocks first mechanical headset detection
and second headset type, 5 button detection.

This patchset 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.

Damien sent a similar patchset to add support to mechanical detection,
but that patch has issues and will not work on most usecases (for example
after a playback/capture session, multicodec case). So I only picked up
the BIT mask patch from that series.

Tested it on DB410c with Audio Mezz board with 4 pole and 3 pole
and during playback and recording use cases.

Changes since v2(https://lkml.org/lkml/2017/8/2/609):
	- cleaned up code spotted by Damien
	- dropped fix snd_soc_codec_set_jack return error as it alredy applied

Srinivas Kandagatla (5):
  ASoC: codecs: msm8916-wcd-analog: move codec reset to probe
  ASoC: codecs: msm8916-wcd-analog: get micbias voltage from dt
  ASoC: codecs: msm8916-wcd-analog: add MBHC support
  ASoC: qcom: apq8016-sbc: Add support to Headset JACK
  arm64: dts: apq8016-sbc: add mbhc buttons support

 .../bindings/sound/qcom,msm8916-wcd-analog.txt     |  18 +-
 arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi          |   2 +
 sound/soc/codecs/msm8916-wcd-analog.c              | 407 +++++++++++++++++++--
 sound/soc/qcom/apq8016_sbc.c                       |  34 ++
 4 files changed, 437 insertions(+), 24 deletions(-)

-- 
2.9.3

Comments

Damien Riegel Aug. 9, 2017, 9:10 p.m. UTC | #1
Hi Srinivas,

On Wed, Aug 09, 2017 at 06:49:25PM +0200, srinivas.kandagatla@linaro.org wrote:
> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.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.


I have the same issue with this patchset, KEY_MEDIA is being reported
when unplugging a headset. I added a few traces and what I observe is
that the "button pressed" irq is fired when unplugging, but no "button
released" irq follows. To get a "button released" irq, I need to either
plug a headset, or plug-and-unplug headphones.

So basically, detection fails because we don't get the "button pressed"
irq prior to the mechanical switch irq. Do you know what could explain
the MBHC not firing the "button released" irq when unplugging headset?


Thanks,
-- 
Damien
Srinivas Kandagatla Aug. 10, 2017, 10:02 a.m. UTC | #2
Hi Damien,
Thanks for testing.

On 09/08/17 22:10, Damien Riegel wrote:
> Hi Srinivas,

> 

> On Wed, Aug 09, 2017 at 06:49:25PM +0200, srinivas.kandagatla@linaro.org wrote:

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

> 

> I have the same issue with this patchset, KEY_MEDIA is being reported

> when unplugging a headset. I added a few traces and what I observe is

Am unable to reproduce the same issue, I tried atleast 6 different mix 
of headset/headphones.
here is my evtest log:  http://paste.ubuntu.com/25282592/

Could you explain bit more about your setup, so that I can try to 
reproduce the same issue.

My setup is DB410c + Audio Mezz board 
https://www.arrow.com/en/products/audiomezz/seeed-technology-limited


> that the "button pressed" irq is fired when unplugging, but no "button

> released" irq follows. To get a "button released" irq, I need to either

> plug a headset, or plug-and-unplug headphones.

> 

> So basically, detection fails because we don't get the "button pressed"

> irq prior to the mechanical switch irq. Do you know what could explain

> the MBHC not firing the "button released" irq when unplugging headset?

> 

Can you also share output of evetest and /proc/interrupts so that i can 
see if the extra logic of masking btn0 is creating the issue, we can 
also try removing the btn0 accessory detect logic and see if we get 
correct button press/releases.

thanks,
srini


> 

> Thanks,

>
Damien Riegel Aug. 10, 2017, 1:33 p.m. UTC | #3
Hi,

On Thu, Aug 10, 2017 at 11:02:34AM +0100, Srinivas Kandagatla wrote:
> Hi Damien,

> Thanks for testing.

> 

> On 09/08/17 22:10, Damien Riegel wrote:

> > Hi Srinivas,

> > 

> > On Wed, Aug 09, 2017 at 06:49:25PM +0200, srinivas.kandagatla@linaro.org wrote:

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

> > 

> > I have the same issue with this patchset, KEY_MEDIA is being reported

> > when unplugging a headset. I added a few traces and what I observe is

> Am unable to reproduce the same issue, I tried atleast 6 different mix of

> headset/headphones.

> here is my evtest log:  http://paste.ubuntu.com/25282592/

> 

> Could you explain bit more about your setup, so that I can try to reproduce

> the same issue.

> 

> My setup is DB410c + Audio Mezz board

> https://www.arrow.com/en/products/audiomezz/seeed-technology-limited


I'm using a hardware based on Intrinsyc Open-Q 410 Development Kit:
https://www.intrinsyc.com/snapdragon-embedded-development-kits/snapdragon-410-development-kit/

We use the same SoM with a different carrier board, but connections to
the jack connector are routed the same way on both carrier boards.

> > that the "button pressed" irq is fired when unplugging, but no "button

> > released" irq follows. To get a "button released" irq, I need to either

> > plug a headset, or plug-and-unplug headphones.

> > 

> > So basically, detection fails because we don't get the "button pressed"

> > irq prior to the mechanical switch irq. Do you know what could explain

> > the MBHC not firing the "button released" irq when unplugging headset?

> > 

> Can you also share output of evetest and /proc/interrupts so that i can see

> if the extra logic of masking btn0 is creating the issue, we can also try

> removing the btn0 accessory detect logic and see if we get correct button

> press/releases.


It's definitely what's causing the headphones to be reported as headset,
but not what's causing the interrupt not to be triggered in the first
place. I don't have access to the hardware right now, so I'll provide
you with logs next week.

Thanks,
-- 
Damien
Srinivas Kandagatla Aug. 11, 2017, 3:15 p.m. UTC | #4
On 10/08/17 16:24, Mark Brown wrote:
> On Wed, Aug 09, 2017 at 06:49:24PM +0200, srinivas.kandagatla@linaro.org wrote:

> 

>> -	snd_soc_write(codec, CDC_A_MICB_1_VAL, MICB_1_VAL_MICB_OUT_VAL_V2P70V);

>> -	/*

>> -	 * Special headset needs MICBIAS as 2.7V so wait for

>> -	 * 50 msec for the MICBIAS to reach 2.7 volts.

>> -	 */

>> -	msleep(50);

>> +	if (wcd->micbias_mv)

> 

> We seem to have just completely lost this ramp time handling?

Yes, will take care of this in next version.

>
Damien Riegel Aug. 14, 2017, 2:12 p.m. UTC | #5
Hi,


On Thu, Aug 10, 2017 at 09:33:29AM -0400, Damien Riegel wrote:
> Hi,

> 

> On Thu, Aug 10, 2017 at 11:02:34AM +0100, Srinivas Kandagatla wrote:

> > Hi Damien,

> > Thanks for testing.

> > 

> > On 09/08/17 22:10, Damien Riegel wrote:

> > > Hi Srinivas,

> > > 

> > > On Wed, Aug 09, 2017 at 06:49:25PM +0200, srinivas.kandagatla@linaro.org wrote:

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

> > > 

> > > I have the same issue with this patchset, KEY_MEDIA is being reported

> > > when unplugging a headset. I added a few traces and what I observe is

> > Am unable to reproduce the same issue, I tried atleast 6 different mix of

> > headset/headphones.

> > here is my evtest log:  http://paste.ubuntu.com/25282592/

> > 

> > Could you explain bit more about your setup, so that I can try to reproduce

> > the same issue.

> > 

> > My setup is DB410c + Audio Mezz board

> > https://www.arrow.com/en/products/audiomezz/seeed-technology-limited

> 

> I'm using a hardware based on Intrinsyc Open-Q 410 Development Kit:

> https://www.intrinsyc.com/snapdragon-embedded-development-kits/snapdragon-410-development-kit/

> 

> We use the same SoM with a different carrier board, but connections to

> the jack connector are routed the same way on both carrier boards.

> 

> > > that the "button pressed" irq is fired when unplugging, but no "button

> > > released" irq follows. To get a "button released" irq, I need to either

> > > plug a headset, or plug-and-unplug headphones.

> > > 

> > > So basically, detection fails because we don't get the "button pressed"

> > > irq prior to the mechanical switch irq. Do you know what could explain

> > > the MBHC not firing the "button released" irq when unplugging headset?

> > > 

> > Can you also share output of evetest and /proc/interrupts so that i can see

> > if the extra logic of masking btn0 is creating the issue, we can also try

> > removing the btn0 accessory detect logic and see if we get correct button

> > press/releases.

> 

> It's definitely what's causing the headphones to be reported as headset,

> but not what's causing the interrupt not to be triggered in the first

> place. I don't have access to the hardware right now, so I'll provide

> you with logs next week.


I did the test, and addded the content of /proc/interrupts.

    https://paste.ubuntu.com/25312443/

As you can see, after "[INSERT HEADPHONE]", we only get "mbhc switch
irq", but not "mbhc btn press irq". Let me know if you need more
details.

As I said before, I don't see that 100% of the time, but I don't do
anything special to trigger it either (like pulling the cable extra
slowly or whatnot), it just happens.


Thank you,
-- 
Damien
Damien Riegel Aug. 16, 2017, 5:48 p.m. UTC | #6
On Mon, Aug 14, 2017 at 05:34:53PM +0100, Srinivas Kandagatla wrote:
> 

> 

> On 14/08/17 15:12, Damien Riegel wrote:

> > Hi,

> > 

> > 

> > On Thu, Aug 10, 2017 at 09:33:29AM -0400, Damien Riegel wrote:

> > > Hi,

> > > 

> > > On Thu, Aug 10, 2017 at 11:02:34AM +0100, Srinivas Kandagatla wrote:

> > > > Hi Damien,

> > > > Thanks for testing.

> > > > 

> > > > On 09/08/17 22:10, Damien Riegel wrote:

> > > > > Hi Srinivas,

> > > > > 

> > > > > On Wed, Aug 09, 2017 at 06:49:25PM +0200, srinivas.kandagatla@linaro.org wrote:

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

> > > > > 

> > > > > I have the same issue with this patchset, KEY_MEDIA is being reported

> > > > > when unplugging a headset. I added a few traces and what I observe is

> > > > Am unable to reproduce the same issue, I tried atleast 6 different mix of

> > > > headset/headphones.

> > > > here is my evtest log:  http://paste.ubuntu.com/25282592/

> > > > 

> > > > Could you explain bit more about your setup, so that I can try to reproduce

> > > > the same issue.

> > > > 

> > > > My setup is DB410c + Audio Mezz board

> > > > https://www.arrow.com/en/products/audiomezz/seeed-technology-limited

> > > 

> > > I'm using a hardware based on Intrinsyc Open-Q 410 Development Kit:

> > > https://www.intrinsyc.com/snapdragon-embedded-development-kits/snapdragon-410-development-kit/

> > > 

> > > We use the same SoM with a different carrier board, but connections to

> > > the jack connector are routed the same way on both carrier boards.

> > > 

> > > > > that the "button pressed" irq is fired when unplugging, but no "button

> > > > > released" irq follows. To get a "button released" irq, I need to either

> > > > > plug a headset, or plug-and-unplug headphones.

> > > > > 

> > > > > So basically, detection fails because we don't get the "button pressed"

> > > > > irq prior to the mechanical switch irq. Do you know what could explain

> > > > > the MBHC not firing the "button released" irq when unplugging headset?

> > > > > 

> > > > Can you also share output of evetest and /proc/interrupts so that i can see

> > > > if the extra logic of masking btn0 is creating the issue, we can also try

> > > > removing the btn0 accessory detect logic and see if we get correct button

> > > > press/releases.

> > > 

> > > It's definitely what's causing the headphones to be reported as headset,

> > > but not what's causing the interrupt not to be triggered in the first

> > > place. I don't have access to the hardware right now, so I'll provide

> > > you with logs next week.

> > 

> > I did the test, and addded the content of /proc/interrupts.

> > 

> >      https://paste.ubuntu.com/25312443/

> > 

> > As you can see, after "[INSERT HEADPHONE]", we only get "mbhc switch

> > irq", but not "mbhc btn press irq". Let me know if you need more

> > details.

> > 

> > As I said before, I don't see that 100% of the time, but I don't do

> > anything special to trigger it either (like pulling the cable extra

> > slowly or whatnot), it just happens.

> 

> Looking at the interrupts count its clear that there was a real button press

> event from the hardware, so My suspect is the voltage threshold values

> programmed for the buttons, which might not be very much correct if you are

> using same values as db410c. These values are partly board specific.

> 

> Can you tune those values and give it a try, specially btn0 values.


Correct me if I'm wrong, but if the design is the same, the values
should be more a less the same as well, right? Moreover, the driver
detects the right key press in normal use cases.

Anyway, I tried to tweak the values but ended up with the same results.
Pressing btn0 brings the DC tension to 0V, so I'm not sure changing the
detection threshold of btn0 has any impact.

Another explanation could be the jack connector. I did a few tests on
the Open-Q 410 [1] and could not reproduce the issue of the spurious key
when unplugging headset, it only happens on the in-house device. So I
guess they have slightly different jack connectors and only one kind
triggers that issue. With that conclusion:


Tested-by: Damien Riegel <damien.riegel@savoirfairelinux.com>


[1] https://www.intrinsyc.com/snapdragon-embedded-development-kits/snapdragon-410-development-kit/

Thank you,
-- 
Damien
Srinivas Kandagatla Aug. 17, 2017, 10:15 a.m. UTC | #7
On 16/08/17 18:48, Damien Riegel wrote:
> Correct me if I'm wrong, but if the design is the same, the values

> should be more a less the same as well, right? Moreover, the driver

> detects the right key press in normal use cases.

You are partly correct, but these values might change according to your 
board layout and there is button tuning procedure to derive these 
threshold values for each board according to OEM design requirements.

thanks,
srini