[PATCH] ASoC: Intel: Enabled button jack for BSW platform with rt5650 codec

[PATCH] ASoC: Intel: Enabled button jack for BSW platform with rt5650 codec

[yang.a.fang]

From: "Fang, Yang A" <yang.a.fang@intel.com>

rt5650 codec supports 4 buttons detections so enabled it

Signed-off-by: Fang, Yang A <yang.a.fang@intel.com>
---
 sound/soc/intel/boards/cht_bsw_rt5645.c |   23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/sound/soc/intel/boards/cht_bsw_rt5645.c b/sound/soc/intel/boards/cht_bsw_rt5645.c
index 8f96c21..1468c3d 100644
--- a/sound/soc/intel/boards/cht_bsw_rt5645.c
+++ b/sound/soc/intel/boards/cht_bsw_rt5645.c
@@ -43,6 +43,7 @@ struct cht_acpi_card {
 struct cht_mc_private {
 	struct snd_soc_jack hp_jack;
 	struct snd_soc_jack mic_jack;
+	struct snd_soc_jack btn_jack;
 	struct cht_acpi_card *acpi_card;
 };
 
@@ -182,6 +183,7 @@ static int cht_codec_init(struct snd_soc_pcm_runtime *runtime)
 	struct snd_soc_codec *codec = runtime->codec;
 	struct snd_soc_dai *codec_dai = runtime->codec_dai;
 	struct cht_mc_private *ctx = snd_soc_card_get_drvdata(runtime->card);
+	struct snd_soc_jack *btn_jack;
 
 	/* Select clk_i2s1_asrc as ASRC clock source */
 	rt5645_sel_asrc_clk_src(codec,
@@ -214,7 +216,26 @@ static int cht_codec_init(struct snd_soc_pcm_runtime *runtime)
 		return ret;
 	}
 
-	rt5645_set_jack_detect(codec, &ctx->hp_jack, &ctx->mic_jack, NULL);
+	if (ctx->acpi_card->codec_type == CODEC_TYPE_RT5650) {
+
+		ret = snd_soc_card_jack_new(runtime->card, "Button Jack",
+						SND_JACK_BTN_0 |
+						SND_JACK_BTN_1 |
+						SND_JACK_BTN_2 |
+						SND_JACK_BTN_3, &ctx->btn_jack,
+						NULL, 0);
+		if (ret) {
+			dev_err(runtime->dev,
+					"Btn jack creation failed %d\n", ret);
+			return ret;
+		}
+
+		btn_jack = &ctx->btn_jack;
+	} else {
+		btn_jack = NULL;
+	}
+
+	rt5645_set_jack_detect(codec, &ctx->hp_jack, &ctx->mic_jack, btn_jack);
 
 	return ret;
 }
-- 
1.7.9.5

Re: [PATCH] ASoC: Intel: Enabled button jack for BSW platform with rt5650 codec

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 816 bytes --]

On Mon, May 04, 2015 at 05:19:20PM -0700, yang.a.fang@intel.com wrote:
> From: "Fang, Yang A" <yang.a.fang@intel.com>
> 
> rt5650 codec supports 4 buttons detections so enabled it

> @@ -43,6 +43,7 @@ struct cht_acpi_card {
>  struct cht_mc_private {
>  	struct snd_soc_jack hp_jack;
>  	struct snd_soc_jack mic_jack;
> +	struct snd_soc_jack btn_jack;
>  	struct cht_acpi_card *acpi_card;

This is a bit weird - why are we adding an additional jack here, surely
the button is attached to the mic jack here?  The jacks should
correspond to the physical jacks on the system rather than the function
so that users and applications can tie the jacks that we have in
software to those physically on the system.

Now I'm wondering if the headphone and microphone are expected to be a
single jack here?

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH] ASoC: Intel: Enabled button jack for BSW platform with rt5650 codec

[Dylan Reid]

On Tue, May 5, 2015 at 2:49 PM, Mark Brown <broonie@kernel.org> wrote:
> On Mon, May 04, 2015 at 05:19:20PM -0700, yang.a.fang@intel.com wrote:
>> From: "Fang, Yang A" <yang.a.fang@intel.com>
>>
>> rt5650 codec supports 4 buttons detections so enabled it
>
>> @@ -43,6 +43,7 @@ struct cht_acpi_card {
>>  struct cht_mc_private {
>>       struct snd_soc_jack hp_jack;
>>       struct snd_soc_jack mic_jack;
>> +     struct snd_soc_jack btn_jack;
>>       struct cht_acpi_card *acpi_card;
>
> This is a bit weird - why are we adding an additional jack here, surely
> the button is attached to the mic jack here?  The jacks should
> correspond to the physical jacks on the system rather than the function
> so that users and applications can tie the jacks that we have in
> software to those physically on the system.
>
> Now I'm wondering if the headphone and microphone are expected to be a
> single jack here?

At least the Chromebook version will have a single combo jack.  I
would expect there to be a single jack created that reports all of hp,
mic, and buttons.

>
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>

Re: [PATCH] ASoC: Intel: Enabled button jack for BSW platform with rt5650 codec

[Yang Fang]

On Tue, May 05, 2015 at 03:02:01PM -0700, Dylan Reid wrote:
> On Tue, May 5, 2015 at 2:49 PM, Mark Brown <broonie@kernel.org> wrote:
> > On Mon, May 04, 2015 at 05:19:20PM -0700, yang.a.fang@intel.com wrote:
> >> From: "Fang, Yang A" <yang.a.fang@intel.com>
> >>
> >> rt5650 codec supports 4 buttons detections so enabled it
> >
> >> @@ -43,6 +43,7 @@ struct cht_acpi_card {
> >>  struct cht_mc_private {
> >>       struct snd_soc_jack hp_jack;
> >>       struct snd_soc_jack mic_jack;
> >> +     struct snd_soc_jack btn_jack;
> >>       struct cht_acpi_card *acpi_card;
> >
> > This is a bit weird - why are we adding an additional jack here, surely
> > the button is attached to the mic jack here?  The jacks should
> > correspond to the physical jacks on the system rather than the function
> > so that users and applications can tie the jacks that we have in
> > software to those physically on the system.
> >
> > Now I'm wondering if the headphone and microphone are expected to be a
> > single jack here?
> 
> At least the Chromebook version will have a single combo jack.  I
> would expect there to be a single jack created that reports all of hp,
> mic, and buttons.
I recalled in Baytrail cras required hp and mic to be separated jack.
I beleived it is changed afterwards.

now rt5645_set_jack_detect in rt5645.c aleady split jack into 3 like
below. maybe we need Bard to combine them ?

 int rt5645_set_jack_detect(struct snd_soc_codec *codec,
     struct snd_soc_jack *hp_jack, struct snd_soc_jack *mic_jack,
     struct snd_soc_jack *btn_jack)
 {
     struct rt5645_priv *rt5645 = snd_soc_codec_get_drvdata(codec);

     rt5645->hp_jack = hp_jack;
     rt5645->mic_jack = mic_jack;
     rt5645->btn_jack = btn_jack;
     if (rt5645->btn_jack && rt5645->codec_type ==
CODEC_TYPE_RT5650) {
         rt5645->en_button_func = true;
         regmap_update_bits(rt5645->regmap, RT5645_GPIO_CTRL1,
                 RT5645_GP1_PIN_IRQ, RT5645_GP1_PIN_IRQ);
         regmap_update_bits(rt5645->regmap, RT5645_DEPOP_M1,
                 RT5645_HP_CB_MASK, RT5645_HP_CB_PU);
         regmap_update_bits(rt5645->regmap, RT5645_GEN_CTRL1,
                 RT5645_DIG_GATE_CTRL, RT5645_DIG_GATE_CTRL);
     }
     rt5645_irq_detection(rt5645);

     return 0;
 }

Yang

> 
> >
> > _______________________________________________
> > Alsa-devel mailing list
> > Alsa-devel@alsa-project.org
> > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> >

Re: [PATCH] ASoC: Intel: Enabled button jack for BSW platform with rt5650 codec

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 495 bytes --]

On Tue, May 05, 2015 at 03:02:01PM -0700, Dylan Reid wrote:
> On Tue, May 5, 2015 at 2:49 PM, Mark Brown <broonie@kernel.org> wrote:

> > Now I'm wondering if the headphone and microphone are expected to be a
> > single jack here?

> At least the Chromebook version will have a single combo jack.  I
> would expect there to be a single jack created that reports all of hp,
> mic, and buttons.

OK, so that should probably be fixed then.  For modern laptops I guess
this is the most common case.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH] ASoC: Intel: Enabled button jack for BSW platform with rt5650 codec

[Dylan Reid]

On Tue, May 5, 2015 at 3:14 PM, Yang Fang <yang.a.fang@intel.com> wrote:
> On Tue, May 05, 2015 at 03:02:01PM -0700, Dylan Reid wrote:
>> On Tue, May 5, 2015 at 2:49 PM, Mark Brown <broonie@kernel.org> wrote:
>> > On Mon, May 04, 2015 at 05:19:20PM -0700, yang.a.fang@intel.com wrote:
>> >> From: "Fang, Yang A" <yang.a.fang@intel.com>
>> >>
>> >> rt5650 codec supports 4 buttons detections so enabled it
>> >
>> >> @@ -43,6 +43,7 @@ struct cht_acpi_card {
>> >>  struct cht_mc_private {
>> >>       struct snd_soc_jack hp_jack;
>> >>       struct snd_soc_jack mic_jack;
>> >> +     struct snd_soc_jack btn_jack;
>> >>       struct cht_acpi_card *acpi_card;
>> >
>> > This is a bit weird - why are we adding an additional jack here, surely
>> > the button is attached to the mic jack here?  The jacks should
>> > correspond to the physical jacks on the system rather than the function
>> > so that users and applications can tie the jacks that we have in
>> > software to those physically on the system.
>> >
>> > Now I'm wondering if the headphone and microphone are expected to be a
>> > single jack here?
>>
>> At least the Chromebook version will have a single combo jack.  I
>> would expect there to be a single jack created that reports all of hp,
>> mic, and buttons.
> I recalled in Baytrail cras required hp and mic to be separated jack.
> I beleived it is changed afterwards.
>
> now rt5645_set_jack_detect in rt5645.c aleady split jack into 3 like
> below. maybe we need Bard to combine them ?

That would be ideal.  The ts3a227e driver already behaves this.

>
>  int rt5645_set_jack_detect(struct snd_soc_codec *codec,
>      struct snd_soc_jack *hp_jack, struct snd_soc_jack *mic_jack,
>      struct snd_soc_jack *btn_jack)
>  {
>      struct rt5645_priv *rt5645 = snd_soc_codec_get_drvdata(codec);
>
>      rt5645->hp_jack = hp_jack;
>      rt5645->mic_jack = mic_jack;
>      rt5645->btn_jack = btn_jack;
>      if (rt5645->btn_jack && rt5645->codec_type ==
> CODEC_TYPE_RT5650) {
>          rt5645->en_button_func = true;
>          regmap_update_bits(rt5645->regmap, RT5645_GPIO_CTRL1,
>                  RT5645_GP1_PIN_IRQ, RT5645_GP1_PIN_IRQ);
>          regmap_update_bits(rt5645->regmap, RT5645_DEPOP_M1,
>                  RT5645_HP_CB_MASK, RT5645_HP_CB_PU);
>          regmap_update_bits(rt5645->regmap, RT5645_GEN_CTRL1,
>                  RT5645_DIG_GATE_CTRL, RT5645_DIG_GATE_CTRL);
>      }
>      rt5645_irq_detection(rt5645);
>
>      return 0;
>  }
>
> Yang
>
>>
>> >
>> > _______________________________________________
>> > Alsa-devel mailing list
>> > Alsa-devel@alsa-project.org
>> > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>> >

Re: [PATCH] ASoC: Intel: Enabled button jack for BSW platform with rt5650 codec

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 402 bytes --]

On Tue, May 05, 2015 at 03:14:15PM -0700, Yang Fang wrote:

> now rt5645_set_jack_detect in rt5645.c aleady split jack into 3 like
> below. maybe we need Bard to combine them ?

It's fine to do that in the CODEC driver and actually good for the
headphone and microphone - the board can always pass in the same jack
for all of them but if it does have split headphone and mic jacks then
it can do that.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH] ASoC: Intel: Enabled button jack for BSW platform with rt5650 codec

[Fang, Yang A]

> -----Original Message-----
> From: Mark Brown [mailto:broonie@kernel.org]
> Sent: Tuesday, May 05, 2015 3:30 PM
> To: Fang, Yang A
> Cc: Dylan Reid; alsa-devel@alsa-project.org; Sripathi, Srinivas; Koul, Vinod;
> Jain, Praveen K; Liam Girdwood; Iriawan, Denny; Nujella, Sathyanarayana;
> kevin.strasser@linux.intel.com
> Subject: Re: [alsa-devel] [PATCH] ASoC: Intel: Enabled button jack for BSW
> platform with rt5650 codec
> 
> On Tue, May 05, 2015 at 03:14:15PM -0700, Yang Fang wrote:
> 
> > now rt5645_set_jack_detect in rt5645.c aleady split jack into 3 like
> > below. maybe we need Bard to combine them ?
> 
> It's fine to do that in the CODEC driver and actually good for the headphone
> and microphone - the board can always pass in the same jack for all of them
> but if it does have split headphone and mic jacks then it can do that.

Thanks. I will post v2 after Bard modifies the api

Yang

Re: [PATCH] ASoC: Intel: Enabled button jack for BSW platform with rt5650 codec

[Dylan Reid]

On Tue, May 5, 2015 at 3:39 PM, Fang, Yang A <yang.a.fang@intel.com> wrote:
>
>
>> -----Original Message-----
>> From: Mark Brown [mailto:broonie@kernel.org]
>> Sent: Tuesday, May 05, 2015 3:30 PM
>> To: Fang, Yang A
>> Cc: Dylan Reid; alsa-devel@alsa-project.org; Sripathi, Srinivas; Koul, Vinod;
>> Jain, Praveen K; Liam Girdwood; Iriawan, Denny; Nujella, Sathyanarayana;
>> kevin.strasser@linux.intel.com
>> Subject: Re: [alsa-devel] [PATCH] ASoC: Intel: Enabled button jack for BSW
>> platform with rt5650 codec
>>
>> On Tue, May 05, 2015 at 03:14:15PM -0700, Yang Fang wrote:
>>
>> > now rt5645_set_jack_detect in rt5645.c aleady split jack into 3 like
>> > below. maybe we need Bard to combine them ?
>>
>> It's fine to do that in the CODEC driver and actually good for the headphone
>> and microphone - the board can always pass in the same jack for all of them
>> but if it does have split headphone and mic jacks then it can do that.
>
> Thanks. I will post v2 after Bard modifies the api

If I'm interpreting Mark's comment correctly, he is suggesting that
the API remain the same and have cht_bsw_rt5645.c pass the same jack
to all three parameters.  I like this idea, as unlike the 227e driver
the 5650 could be used with separate hp and mic jacks. I'd imagine
there wouldn't be a button jack in that case since that is headset
specific, but that shouldn't matter.

>
> Yang

Re: [PATCH] ASoC: Intel: Enabled button jack for BSW platform with rt5650 codec

[Fang, Yang A]

> -----Original Message-----
> From: dgreid@google.com [mailto:dgreid@google.com] On Behalf Of Dylan
> Reid
> Sent: Tuesday, May 05, 2015 3:43 PM
> To: Fang, Yang A
> Cc: Mark Brown; alsa-devel@alsa-project.org; Sripathi, Srinivas; Koul, Vinod;
> Jain, Praveen K; Liam Girdwood; Iriawan, Denny; Nujella, Sathyanarayana;
> kevin.strasser@linux.intel.com; Bard Liao
> Subject: Re: [alsa-devel] [PATCH] ASoC: Intel: Enabled button jack for BSW
> platform with rt5650 codec
> 
> On Tue, May 5, 2015 at 3:39 PM, Fang, Yang A <yang.a.fang@intel.com>
> wrote:
> >
> >
> >> -----Original Message-----
> >> From: Mark Brown [mailto:broonie@kernel.org]
> >> Sent: Tuesday, May 05, 2015 3:30 PM
> >> To: Fang, Yang A
> >> Cc: Dylan Reid; alsa-devel@alsa-project.org; Sripathi, Srinivas;
> >> Koul, Vinod; Jain, Praveen K; Liam Girdwood; Iriawan, Denny; Nujella,
> >> Sathyanarayana; kevin.strasser@linux.intel.com
> >> Subject: Re: [alsa-devel] [PATCH] ASoC: Intel: Enabled button jack
> >> for BSW platform with rt5650 codec
> >>
> >> On Tue, May 05, 2015 at 03:14:15PM -0700, Yang Fang wrote:
> >>
> >> > now rt5645_set_jack_detect in rt5645.c aleady split jack into 3
> >> > like below. maybe we need Bard to combine them ?
> >>
> >> It's fine to do that in the CODEC driver and actually good for the
> >> headphone and microphone - the board can always pass in the same jack
> >> for all of them but if it does have split headphone and mic jacks then it can
> do that.
> >
> > Thanks. I will post v2 after Bard modifies the api
> 
> If I'm interpreting Mark's comment correctly, he is suggesting that the API
> remain the same and have cht_bsw_rt5645.c pass the same jack to all three
> parameters.  I like this idea, as unlike the 227e driver the 5650 could be used
> with separate hp and mic jacks. I'd imagine there wouldn't be a button jack in
> that case since that is headset specific, but that shouldn't matter.

Ok  will try to pass same jack from the board file

> 
> >
> > Yang

Re: [PATCH] ASoC: Intel: Enabled button jack for BSW platform with rt5650 codec

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1035 bytes --]

On Tue, May 05, 2015 at 03:42:52PM -0700, Dylan Reid wrote:
> On Tue, May 5, 2015 at 3:39 PM, Fang, Yang A <yang.a.fang@intel.com> wrote:

> >> It's fine to do that in the CODEC driver and actually good for the headphone
> >> and microphone - the board can always pass in the same jack for all of them
> >> but if it does have split headphone and mic jacks then it can do that.

> > Thanks. I will post v2 after Bard modifies the api

> If I'm interpreting Mark's comment correctly, he is suggesting that
> the API remain the same and have cht_bsw_rt5645.c pass the same jack
> to all three parameters.  I like this idea, as unlike the 227e driver
> the 5650 could be used with separate hp and mic jacks. I'd imagine
> there wouldn't be a button jack in that case since that is headset
> specific, but that shouldn't matter.

Yes, that's what I meant - I had originally interpreted the ability to
specify the buttons separately as the ability to turn button detection
on or off from the machine driver, either the same as mic or null.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

[PATCH v2] ASoC: Intel: Enabled button jack for BSW platform with rt5650 codec

[yang.a.fang]

From: "Fang, Yang A" <yang.a.fang@intel.com>

rt5650 codec supports 4 buttons detections so enabled it

Signed-off-by: Fang, Yang A <yang.a.fang@intel.com>
---
 sound/soc/intel/boards/cht_bsw_rt5645.c |   25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/sound/soc/intel/boards/cht_bsw_rt5645.c b/sound/soc/intel/boards/cht_bsw_rt5645.c
index 8f96c21..bdcaf46 100644
--- a/sound/soc/intel/boards/cht_bsw_rt5645.c
+++ b/sound/soc/intel/boards/cht_bsw_rt5645.c
@@ -41,8 +41,7 @@ struct cht_acpi_card {
 };
 
 struct cht_mc_private {
-	struct snd_soc_jack hp_jack;
-	struct snd_soc_jack mic_jack;
+	struct snd_soc_jack jack;
 	struct cht_acpi_card *acpi_card;
 };
 
@@ -179,6 +178,7 @@ static int cht_aif1_hw_params(struct snd_pcm_substream *substream,
 static int cht_codec_init(struct snd_soc_pcm_runtime *runtime)
 {
 	int ret;
+	int jack_type;
 	struct snd_soc_codec *codec = runtime->codec;
 	struct snd_soc_dai *codec_dai = runtime->codec_dai;
 	struct cht_mc_private *ctx = snd_soc_card_get_drvdata(runtime->card);
@@ -198,23 +198,22 @@ static int cht_codec_init(struct snd_soc_pcm_runtime *runtime)
 		return ret;
 	}
 
-	ret = snd_soc_card_jack_new(runtime->card, "Headphone Jack",
-				    SND_JACK_HEADPHONE, &ctx->hp_jack,
-				    NULL, 0);
-	if (ret) {
-		dev_err(runtime->dev, "HP jack creation failed %d\n", ret);
-		return ret;
-	}
+	if (ctx->acpi_card->codec_type == CODEC_TYPE_RT5650)
+		jack_type = SND_JACK_HEADPHONE | SND_JACK_MICROPHONE |
+					SND_JACK_BTN_0 | SND_JACK_BTN_1 |
+					SND_JACK_BTN_2 | SND_JACK_BTN_3;
+	else
+		jack_type = SND_JACK_HEADPHONE | SND_JACK_MICROPHONE;
 
-	ret = snd_soc_card_jack_new(runtime->card, "Mic Jack",
-				    SND_JACK_MICROPHONE, &ctx->mic_jack,
+	ret = snd_soc_card_jack_new(runtime->card, "Headset Jack",
+				    jack_type, &ctx->jack,
 				    NULL, 0);
 	if (ret) {
-		dev_err(runtime->dev, "Mic jack creation failed %d\n", ret);
+		dev_err(runtime->dev, "Headset jack creation failed %d\n", ret);
 		return ret;
 	}
 
-	rt5645_set_jack_detect(codec, &ctx->hp_jack, &ctx->mic_jack, NULL);
+	rt5645_set_jack_detect(codec, &ctx->jack, &ctx->jack, &ctx->jack);
 
 	return ret;
 }
-- 
1.7.9.5

Re: [PATCH v2] ASoC: Intel: Enabled button jack for BSW platform with rt5650 codec

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 204 bytes --]

On Tue, May 05, 2015 at 04:55:34PM -0700, yang.a.fang@intel.com wrote:
> From: "Fang, Yang A" <yang.a.fang@intel.com>
> 
> rt5650 codec supports 4 buttons detections so enabled it

Applied, thanks.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]