by Srinivas Kandagatla

[permalink] [raw]

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

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, [email protected] wrote:
>>>> From: Srinivas Kandagatla <[email protected]>
>>>>
>>>> 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,
>

2017-08-21 18:34:23

by Mark Brown

[permalink] [raw]

Subject: Applied "ASoC: qcom: apq8016-sbc: Add support to Headset JACK" to the asoc tree

The patch

ASoC: qcom: apq8016-sbc: Add support to Headset JACK

has been applied to the asoc tree at

git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From b47b91c8504c62f95ddff2876620d68697927bd4 Mon Sep 17 00:00:00 2001
From: Srinivas Kandagatla <[email protected]>
Date: Thu, 17 Aug 2017 10:02:11 +0200
Subject: [PATCH] ASoC: qcom: apq8016-sbc: Add support to Headset JACK

This patch adds support to Headset JACK, also provides board specific
vref ranges for mbhc buttons to be detected.

This headset supports both 3 pole and 4 pole headset type and 5 buttons.

Signed-off-by: Srinivas Kandagatla <[email protected]>
Signed-off-by: Mark Brown <[email protected]>
---
sound/soc/qcom/apq8016_sbc.c | 34 ++++++++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)

diff --git a/sound/soc/qcom/apq8016_sbc.c b/sound/soc/qcom/apq8016_sbc.c
index 96a079d9f697..d49adc822a11 100644
--- a/sound/soc/qcom/apq8016_sbc.c
+++ b/sound/soc/qcom/apq8016_sbc.c
@@ -21,12 +21,16 @@
#include <linux/platform_device.h>
#include <sound/pcm.h>
#include <sound/pcm_params.h>
+#include <sound/jack.h>
#include <sound/soc.h>
+#include <uapi/linux/input-event-codes.h>
#include <dt-bindings/sound/apq8016-lpass.h>

struct apq8016_sbc_data {
void __iomem *mic_iomux;
void __iomem *spkr_iomux;
+ struct snd_soc_jack jack;
+ bool jack_setup;
struct snd_soc_dai_link dai_link[]; /* dynamically allocated */
};

@@ -70,6 +74,31 @@ static int apq8016_sbc_dai_init(struct snd_soc_pcm_runtime *rtd)

}

+ if (!pdata->jack_setup) {
+ struct snd_jack *jack;
+
+ rval = snd_soc_card_jack_new(card, "Headset Jack",
+ SND_JACK_HEADSET |
+ SND_JACK_HEADPHONE |
+ SND_JACK_BTN_0 | SND_JACK_BTN_1 |
+ SND_JACK_BTN_2 | SND_JACK_BTN_3 |
+ SND_JACK_BTN_4,
+ &pdata->jack, NULL, 0);
+
+ if (rval < 0) {
+ dev_err(card->dev, "Unable to add Headphone Jack\n");
+ return rval;
+ }
+
+ jack = pdata->jack.jack;
+
+ snd_jack_set_key(jack, SND_JACK_BTN_0, KEY_MEDIA);
+ snd_jack_set_key(jack, SND_JACK_BTN_1, KEY_VOICECOMMAND);
+ snd_jack_set_key(jack, SND_JACK_BTN_2, KEY_VOLUMEUP);
+ snd_jack_set_key(jack, SND_JACK_BTN_3, KEY_VOLUMEDOWN);
+ pdata->jack_setup = true;
+ }
+
for (i = 0 ; i < dai_link->num_codecs; i++) {
struct snd_soc_dai *dai = rtd->codec_dais[i];

@@ -81,6 +110,11 @@ static int apq8016_sbc_dai_init(struct snd_soc_pcm_runtime *rtd)
dev_warn(card->dev, "Failed to set mclk: %d\n", rval);
return rval;
}
+ rval = snd_soc_codec_set_jack(codec, &pdata->jack, NULL);
+ if (rval != 0 && rval != -ENOTSUPP) {
+ dev_warn(card->dev, "Failed to set jack: %d\n", rval);
+ return rval;
+ }
}

return 0;
--
2.13.3