[PATCH 0/4] ALSA: Implement core jack support for kcontrol

[PATCH 0/4] ALSA: Implement core jack support for kcontrol

[Felipe F. Tonello]

From: "Felipe F. Tonello" <eu@felipetonello.com>

This patch series enable KControl support for core Jack and update current
codecs. This will will enable moderns user-space daemons, such as
PulseAudio(>=2.0), to have jack detection support via newer ALSA KControl
interface.

Patch 1: Actually implementation using KControl.

Patch 2: It also ensures HDA drivers to use properly this new ALSA Jack
implementation.

Patches 3-4: This patch makes sure that all users of ALSA Jack uses this
new standard way to report detections to user-space.

Felipe F. Tonello (4):
  ALSA: Added jack detection kcontrol support
  ALSA: HDA: Updating jack implementation according new ALSA Jacks
  ALSA: SoC: Updating jack implementation according new ALSA Jacks
  ALSA: oxygen: Updating jack implementation according new ALSA Jacks

 include/sound/jack.h               |  6 ++++--
 include/sound/soc.h                |  2 +-
 sound/core/Kconfig                 |  1 +
 sound/core/ctljack.c               |  3 ++-
 sound/core/jack.c                  | 29 +++++++++++++++++++++++++++--
 sound/pci/hda/Kconfig              |  8 --------
 sound/pci/hda/hda_codec.h          |  2 --
 sound/pci/hda/hda_jack.c           | 38 +++++++++++++++++---------------------
 sound/pci/hda/hda_jack.h           |  4 +---
 sound/pci/oxygen/xonar_wm87x6.c    |  2 +-
 sound/soc/fsl/wm1133-ev1.c         |  4 ++--
 sound/soc/mid-x86/mfld_machine.c   |  6 +++---
 sound/soc/omap/ams-delta.c         |  2 +-
 sound/soc/omap/omap-abe-twl6040.c  |  4 ++--
 sound/soc/omap/omap-twl4030.c      |  4 ++--
 sound/soc/omap/rx51.c              |  6 +++---
 sound/soc/pxa/hx4700.c             |  4 ++--
 sound/soc/pxa/palm27x.c            |  4 ++--
 sound/soc/pxa/ttc-dkb.c            |  8 ++++----
 sound/soc/pxa/z2.c                 |  4 ++--
 sound/soc/samsung/goni_wm8994.c    |  4 ++--
 sound/soc/samsung/h1940_uda1380.c  |  4 ++--
 sound/soc/samsung/littlemill.c     | 10 +++++-----
 sound/soc/samsung/lowland.c        |  6 +++---
 sound/soc/samsung/rx1950_uda1380.c |  4 ++--
 sound/soc/samsung/smartq_wm8987.c  |  4 ++--
 sound/soc/samsung/speyside.c       |  6 +++---
 sound/soc/samsung/tobermory.c      |  4 ++--
 sound/soc/soc-jack.c               |  4 ++--
 sound/soc/tegra/tegra_alc5632.c    |  4 ++--
 sound/soc/tegra/tegra_rt5640.c     |  2 +-
 sound/soc/tegra/tegra_wm8903.c     |  8 ++++----
 32 files changed, 107 insertions(+), 94 deletions(-)

-- 
1.8.1.4

[PATCH 1/4] ALSA: Added jack detection kcontrol support

[Felipe F. Tonello]

From: "Felipe F. Tonello" <eu@felipetonello.com>

This patch adds jack support for alsa kcontrol.

This support is necessary since the new kcontrol is used by user-space
daemons, such as PulseAudio(>=2.0), to do jack detection.)

Signed-off-by: Felipe F. Tonello <eu@felipetonello.com>
---
 include/sound/jack.h |  6 ++++--
 sound/core/Kconfig   |  1 +
 sound/core/ctljack.c |  3 ++-
 sound/core/jack.c    | 29 +++++++++++++++++++++++++++--
 4 files changed, 34 insertions(+), 5 deletions(-)

diff --git a/include/sound/jack.h b/include/sound/jack.h
index 5891657..dc62b74 100644
--- a/include/sound/jack.h
+++ b/include/sound/jack.h
@@ -26,6 +26,7 @@
 #include <sound/core.h>
 
 struct input_dev;
+struct snd_kcontrol;
 
 /**
  * Jack types which can be reported.  These values are used as a
@@ -58,6 +59,7 @@ enum snd_jack_types {
 
 struct snd_jack {
 	struct input_dev *input_dev;
+	struct snd_kcontrol *kctl;
 	int registered;
 	int type;
 	const char *id;
@@ -70,7 +72,7 @@ struct snd_jack {
 #ifdef CONFIG_SND_JACK
 
 int snd_jack_new(struct snd_card *card, const char *id, int type,
-		 struct snd_jack **jack);
+                 int idx, struct snd_jack **jack);
 void snd_jack_set_parent(struct snd_jack *jack, struct device *parent);
 int snd_jack_set_key(struct snd_jack *jack, enum snd_jack_types type,
 		     int keytype);
@@ -80,7 +82,7 @@ void snd_jack_report(struct snd_jack *jack, int status);
 #else
 
 static inline int snd_jack_new(struct snd_card *card, const char *id, int type,
-			       struct snd_jack **jack)
+                               int idx, struct snd_jack **jack)
 {
 	return 0;
 }
diff --git a/sound/core/Kconfig b/sound/core/Kconfig
index c0c2f57..8167615 100644
--- a/sound/core/Kconfig
+++ b/sound/core/Kconfig
@@ -20,6 +20,7 @@ config SND_COMPRESS_OFFLOAD
 # to avoid having to force INPUT on.
 config SND_JACK
 	bool
+	select SND_KCTL_JACK
 
 config SND_SEQUENCER
 	tristate "Sequencer support"
diff --git a/sound/core/ctljack.c b/sound/core/ctljack.c
index e4b38fb..59aa6d0 100644
--- a/sound/core/ctljack.c
+++ b/sound/core/ctljack.c
@@ -38,7 +38,8 @@ snd_kctl_jack_new(const char *name, int idx, void *private_data)
 	kctl = snd_ctl_new1(&jack_detect_kctl, private_data);
 	if (!kctl)
 		return NULL;
-	snprintf(kctl->id.name, sizeof(kctl->id.name), "%s Jack", name);
+
+	strlcpy(kctl->id.name, name, sizeof(kctl->id.name));
 	kctl->id.index = idx;
 	kctl->private_value = 0;
 	return kctl;
diff --git a/sound/core/jack.c b/sound/core/jack.c
index b35fe73..b2757b1 100644
--- a/sound/core/jack.c
+++ b/sound/core/jack.c
@@ -24,6 +24,7 @@
 #include <linux/module.h>
 #include <sound/jack.h>
 #include <sound/core.h>
+#include <sound/control.h>
 
 static int jack_switch_types[SND_JACK_SWITCH_TYPES] = {
 	SW_HEADPHONE_INSERT,
@@ -48,6 +49,7 @@ static int snd_jack_dev_free(struct snd_device *device)
 	else
 		input_free_device(jack->input_dev);
 
+	snd_ctl_remove(device->card, jack->kctl);
 	kfree(jack->id);
 	kfree(jack);
 
@@ -85,26 +87,36 @@ static int snd_jack_dev_register(struct snd_device *device)
 	if (err == 0)
 		jack->registered = 1;
 
+	/* We don't need to free the control, it's freed by snd_ctl_add itself
+	   if an error occur */
+	err = snd_ctl_add(card, jack->kctl);
+
 	return err;
 }
 
 /**
  * snd_jack_new - Create a new jack
  * @card:  the card instance
- * @id:    an identifying string for this jack
+ * @id:    an identifying string for this jack, " Jack" is appended to the
+ *         string
  * @type:  a bitmask of enum snd_jack_type values that can be detected by
  *         this jack
+ * @idx:   index of this control item
  * @jjack: Used to provide the allocated jack object to the caller.
  *
  * Creates a new jack object.
  *
+ * This function creates a Jack Kcontrol, which is exported to user space via
+ * ALSA Controls.
+ *
  * Return: Zero if successful, or a negative error code on failure.
  * On success @jjack will be initialised.
  */
 int snd_jack_new(struct snd_card *card, const char *id, int type,
-		 struct snd_jack **jjack)
+                 int idx, struct snd_jack **jjack)
 {
 	struct snd_jack *jack;
+	struct snd_kcontrol *kctl;
 	int err;
 	int i;
 	static struct snd_device_ops ops = {
@@ -117,6 +129,7 @@ int snd_jack_new(struct snd_card *card, const char *id, int type,
 		return -ENOMEM;
 
 	jack->id = kstrdup(id, GFP_KERNEL);
+	sprintf((char *)jack->id, "%s Jack", jack->id);
 
 	jack->input_dev = input_allocate_device();
 	if (jack->input_dev == NULL) {
@@ -137,6 +150,15 @@ int snd_jack_new(struct snd_card *card, const char *id, int type,
 	if (err < 0)
 		goto fail_input;
 
+	/* card is the private_data */
+	kctl = snd_kctl_jack_new(jack->id, idx, card);
+	if (!kctl) {
+		err = -ENOMEM;
+		goto fail_input;
+	}
+
+	jack->kctl = kctl;
+
 	*jjack = jack;
 
 	return 0;
@@ -239,6 +261,9 @@ void snd_jack_report(struct snd_jack *jack, int status)
 	}
 
 	input_sync(jack->input_dev);
+
+	/* Update ALSA KControl interface */
+	snd_kctl_jack_report((struct snd_card *)jack->kctl->private_data, jack->kctl, !!status);
 }
 EXPORT_SYMBOL(snd_jack_report);
 
-- 
1.8.1.4

[PATCH 2/4] ALSA: HDA: Updating jack implementation according new ALSA Jacks

[Felipe F. Tonello]

From: "Felipe F. Tonello" <eu@felipetonello.com>

ALSA standard jacks already are implemented using ALSA KControl. So there is
no need implement that itself or to use snd_jack for input events only.

Signed-off-by: Felipe F. Tonello <eu@felipetonello.com>
---
 sound/pci/hda/Kconfig     |  8 --------
 sound/pci/hda/hda_codec.h |  2 --
 sound/pci/hda/hda_jack.c  | 38 +++++++++++++++++---------------------
 sound/pci/hda/hda_jack.h  |  4 +---
 4 files changed, 18 insertions(+), 34 deletions(-)

diff --git a/sound/pci/hda/Kconfig b/sound/pci/hda/Kconfig
index 59c5e9c..561abc7 100644
--- a/sound/pci/hda/Kconfig
+++ b/sound/pci/hda/Kconfig
@@ -65,14 +65,6 @@ config SND_HDA_INPUT_BEEP_MODE
 	  Set 1 to always enable the digital beep interface for HD-audio by
 	  default.
 
-config SND_HDA_INPUT_JACK
-	bool "Support jack plugging notification via input layer"
-	depends on INPUT=y || INPUT=SND
-	select SND_JACK
-	help
-	  Say Y here to enable the jack plugging notification via
-	  input layer.
-
 config SND_HDA_PATCH_LOADER
 	bool "Support initialization patch loading for HD-audio"
 	select FW_LOADER
diff --git a/sound/pci/hda/hda_codec.h b/sound/pci/hda/hda_codec.h
index 701c2e0..ca7be59 100644
--- a/sound/pci/hda/hda_codec.h
+++ b/sound/pci/hda/hda_codec.h
@@ -912,10 +912,8 @@ struct hda_codec {
 	unsigned long jackpoll_interval; /* In jiffies. Zero means no poll, rely on unsol events */
 	struct delayed_work jackpoll_work;
 
-#ifdef CONFIG_SND_HDA_INPUT_JACK
 	/* jack detection */
 	struct snd_array jacks;
-#endif
 
 	/* fix-up list */
 	int fixup_id;
diff --git a/sound/pci/hda/hda_jack.c b/sound/pci/hda/hda_jack.c
index 3fd2973..6be1a0c 100644
--- a/sound/pci/hda/hda_jack.c
+++ b/sound/pci/hda/hda_jack.c
@@ -112,7 +112,6 @@ EXPORT_SYMBOL_HDA(snd_hda_jack_tbl_new);
 
 void snd_hda_jack_tbl_clear(struct hda_codec *codec)
 {
-#ifdef CONFIG_SND_HDA_INPUT_JACK
 	/* free jack instances manually when clearing/reconfiguring */
 	if (!codec->bus->shutdown && codec->jacktbl.list) {
 		struct hda_jack_tbl *jack = codec->jacktbl.list;
@@ -122,7 +121,6 @@ void snd_hda_jack_tbl_clear(struct hda_codec *codec)
 				snd_device_free(codec->bus->card, jack->jack);
 		}
 	}
-#endif
 	snd_array_free(&codec->jacktbl);
 }
 
@@ -283,17 +281,15 @@ void snd_hda_jack_report_sync(struct hda_codec *codec)
 			if (!jack->kctl)
 				continue;
 			state = get_jack_plug_state(jack->pin_sense);
-			snd_kctl_jack_report(codec->bus->card, jack->kctl, state);
-#ifdef CONFIG_SND_HDA_INPUT_JACK
-			if (jack->jack)
+			if (jack->phantom_jack)
+				snd_kctl_jack_report(codec->bus->card, jack->kctl, state);
+			else if (jack->jack)
 				snd_jack_report(jack->jack,
 						state ? jack->type : 0);
-#endif
 		}
 }
 EXPORT_SYMBOL_HDA(snd_hda_jack_report_sync);
 
-#ifdef CONFIG_SND_HDA_INPUT_JACK
 /* guess the jack type from the pin-config */
 static int get_input_jack_type(struct hda_codec *codec, hda_nid_t nid)
 {
@@ -320,7 +316,6 @@ static void hda_free_jack_priv(struct snd_jack *jack)
 	jacks->nid = 0;
 	jacks->jack = NULL;
 }
-#endif
 
 /**
  * snd_hda_jack_add_kctl - Add a kctl for the given pin
@@ -340,29 +335,30 @@ static int __snd_hda_jack_add_kctl(struct hda_codec *codec, hda_nid_t nid,
 		return 0;
 	if (jack->kctl)
 		return 0; /* already created */
-	kctl = snd_kctl_jack_new(name, idx, codec);
-	if (!kctl)
-		return -ENOMEM;
-	err = snd_hda_ctl_add(codec, nid, kctl);
-	if (err < 0)
-		return err;
-	jack->kctl = kctl;
+
 	jack->phantom_jack = !!phantom_jack;
 
-	state = snd_hda_jack_detect(codec, nid);
-	snd_kctl_jack_report(codec->bus->card, kctl, state);
-#ifdef CONFIG_SND_HDA_INPUT_JACK
-	if (!phantom_jack) {
+	/* If it's phantom jack only creates kcontrol jack elem */
+	if (jack->phantom_jack) {
+		kctl = snd_kctl_jack_new(name, idx, codec);
+		if (!kctl)
+			return -ENOMEM;
+		err = snd_hda_ctl_add(codec, nid, kctl);
+		if (err < 0)
+			return err;
+		jack->kctl = kctl;
+	} else {
+		state = snd_hda_jack_detect(codec, nid);
 		jack->type = get_input_jack_type(codec, nid);
 		err = snd_jack_new(codec->bus->card, name, jack->type,
-				   &jack->jack);
+		                   idx, &jack->jack);
 		if (err < 0)
 			return err;
+		jack->kctl = jack->jack->kctl;
 		jack->jack->private_data = jack;
 		jack->jack->private_free = hda_free_jack_priv;
 		snd_jack_report(jack->jack, state ? jack->type : 0);
 	}
-#endif
 	return 0;
 }
 
diff --git a/sound/pci/hda/hda_jack.h b/sound/pci/hda/hda_jack.h
index ec12abd..d4701d2 100644
--- a/sound/pci/hda/hda_jack.h
+++ b/sound/pci/hda/hda_jack.h
@@ -30,11 +30,9 @@ struct hda_jack_tbl {
 	unsigned int phantom_jack:1;    /* a fixed, always present port? */
 	hda_nid_t gating_jack;		/* valid when gating jack plugged */
 	hda_nid_t gated_jack;		/* gated is dependent on this jack */
-	struct snd_kcontrol *kctl;	/* assigned kctl for jack-detection */
-#ifdef CONFIG_SND_HDA_INPUT_JACK
+	struct snd_kcontrol *kctl;      /* assigned kctl for jack-detection */
 	int type;
 	struct snd_jack *jack;
-#endif
 };
 
 struct hda_jack_tbl *
-- 
1.8.1.4

[PATCH 3/4] ALSA: SoC: Updating jack implementation according new ALSA Jacks

[Felipe F. Tonello]

From: "Felipe F. Tonello" <eu@felipetonello.com>

ALSA standard jacks already are implemented using ALSA KControl.
So there is no need implement that itself or to use snd_jack for input
events only.

Signed-off-by: Felipe F. Tonello <eu@felipetonello.com>
---
 include/sound/soc.h                |  2 +-
 sound/soc/fsl/wm1133-ev1.c         |  4 ++--
 sound/soc/mid-x86/mfld_machine.c   |  6 +++---
 sound/soc/omap/ams-delta.c         |  2 +-
 sound/soc/omap/omap-abe-twl6040.c  |  4 ++--
 sound/soc/omap/omap-twl4030.c      |  4 ++--
 sound/soc/omap/rx51.c              |  6 +++---
 sound/soc/pxa/hx4700.c             |  4 ++--
 sound/soc/pxa/palm27x.c            |  4 ++--
 sound/soc/pxa/ttc-dkb.c            |  8 ++++----
 sound/soc/pxa/z2.c                 |  4 ++--
 sound/soc/samsung/goni_wm8994.c    |  4 ++--
 sound/soc/samsung/h1940_uda1380.c  |  4 ++--
 sound/soc/samsung/littlemill.c     | 10 +++++-----
 sound/soc/samsung/lowland.c        |  6 +++---
 sound/soc/samsung/rx1950_uda1380.c |  4 ++--
 sound/soc/samsung/smartq_wm8987.c  |  4 ++--
 sound/soc/samsung/speyside.c       |  6 +++---
 sound/soc/samsung/tobermory.c      |  4 ++--
 sound/soc/soc-jack.c               |  4 ++--
 sound/soc/tegra/tegra_alc5632.c    |  4 ++--
 sound/soc/tegra/tegra_rt5640.c     |  2 +-
 sound/soc/tegra/tegra_wm8903.c     |  8 ++++----
 23 files changed, 54 insertions(+), 54 deletions(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h
index 6eabee7..31bea52 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -436,7 +436,7 @@ int snd_soc_platform_trigger(struct snd_pcm_substream *substream,
 
 /* Jack reporting */
 int snd_soc_jack_new(struct snd_soc_codec *codec, const char *id, int type,
-		     struct snd_soc_jack *jack);
+                     int idx, struct snd_soc_jack *jack);
 void snd_soc_jack_report(struct snd_soc_jack *jack, int status, int mask);
 int snd_soc_jack_add_pins(struct snd_soc_jack *jack, int count,
 			  struct snd_soc_jack_pin *pins);
diff --git a/sound/soc/fsl/wm1133-ev1.c b/sound/soc/fsl/wm1133-ev1.c
index fce6325..50f96d4 100644
--- a/sound/soc/fsl/wm1133-ev1.c
+++ b/sound/soc/fsl/wm1133-ev1.c
@@ -221,14 +221,14 @@ static int wm1133_ev1_init(struct snd_soc_pcm_runtime *rtd)
 				ARRAY_SIZE(wm1133_ev1_map));
 
 	/* Headphone jack detection */
-	snd_soc_jack_new(codec, "Headphone", SND_JACK_HEADPHONE, &hp_jack);
+	snd_soc_jack_new(codec, "Headphone", SND_JACK_HEADPHONE, 0, &hp_jack);
 	snd_soc_jack_add_pins(&hp_jack, ARRAY_SIZE(hp_jack_pins),
 			      hp_jack_pins);
 	wm8350_hp_jack_detect(codec, WM8350_JDR, &hp_jack, SND_JACK_HEADPHONE);
 
 	/* Microphone jack detection */
 	snd_soc_jack_new(codec, "Microphone",
-			 SND_JACK_MICROPHONE | SND_JACK_BTN_0, &mic_jack);
+	                 SND_JACK_MICROPHONE | SND_JACK_BTN_0, 0, &mic_jack);
 	snd_soc_jack_add_pins(&mic_jack, ARRAY_SIZE(mic_jack_pins),
 			      mic_jack_pins);
 	wm8350_mic_jack_detect(codec, &mic_jack, SND_JACK_MICROPHONE,
diff --git a/sound/soc/mid-x86/mfld_machine.c b/sound/soc/mid-x86/mfld_machine.c
index ee36384..e2c7978 100644
--- a/sound/soc/mid-x86/mfld_machine.c
+++ b/sound/soc/mid-x86/mfld_machine.c
@@ -253,9 +253,9 @@ static int mfld_init(struct snd_soc_pcm_runtime *runtime)
 	snd_soc_dapm_disable_pin(dapm, "LINEINR");
 
 	/* Headset and button jack detection */
-	ret_val = snd_soc_jack_new(codec, "Intel(R) MID Audio Jack",
-			SND_JACK_HEADSET | SND_JACK_BTN_0 |
-			SND_JACK_BTN_1, &mfld_jack);
+	ret_val = snd_soc_jack_new(codec, "Intel(R) MID Audio",
+	                           SND_JACK_HEADSET | SND_JACK_BTN_0 |
+	                           SND_JACK_BTN_1, 0, &mfld_jack);
 	if (ret_val) {
 		pr_err("jack creation failed\n");
 		return ret_val;
diff --git a/sound/soc/omap/ams-delta.c b/sound/soc/omap/ams-delta.c
index 6294464..4ffa38e 100644
--- a/sound/soc/omap/ams-delta.c
+++ b/sound/soc/omap/ams-delta.c
@@ -491,7 +491,7 @@ static int ams_delta_cx20442_init(struct snd_soc_pcm_runtime *rtd)
 	/* Add hook switch - can be used to control the codec from userspace
 	 * even if line discipline fails */
 	ret = snd_soc_jack_new(rtd->codec, "hook_switch",
-				SND_JACK_HEADSET, &ams_delta_hook_switch);
+	                       SND_JACK_HEADSET, 0, &ams_delta_hook_switch);
 	if (ret)
 		dev_warn(card->dev,
 				"Failed to allocate resources for hook switch, "
diff --git a/sound/soc/omap/omap-abe-twl6040.c b/sound/soc/omap/omap-abe-twl6040.c
index 70cd5c7..45ff3bc 100644
--- a/sound/soc/omap/omap-abe-twl6040.c
+++ b/sound/soc/omap/omap-abe-twl6040.c
@@ -193,8 +193,8 @@ static int omap_abe_twl6040_init(struct snd_soc_pcm_runtime *rtd)
 
 	/* Headset jack detection only if it is supported */
 	if (priv->jack_detection) {
-		ret = snd_soc_jack_new(codec, "Headset Jack",
-					SND_JACK_HEADSET, &hs_jack);
+		ret = snd_soc_jack_new(codec, "Headset",
+		                       SND_JACK_HEADSET, 0, &hs_jack);
 		if (ret)
 			return ret;
 
diff --git a/sound/soc/omap/omap-twl4030.c b/sound/soc/omap/omap-twl4030.c
index 2a9324f..0619ecb 100644
--- a/sound/soc/omap/omap-twl4030.c
+++ b/sound/soc/omap/omap-twl4030.c
@@ -189,8 +189,8 @@ static int omap_twl4030_init(struct snd_soc_pcm_runtime *rtd)
 	if (priv->jack_detect > 0) {
 		hs_jack_gpios[0].gpio = priv->jack_detect;
 
-		ret = snd_soc_jack_new(codec, "Headset Jack", SND_JACK_HEADSET,
-				       &priv->hs_jack);
+		ret = snd_soc_jack_new(codec, "Headset", SND_JACK_HEADSET,
+		                       0, &priv->hs_jack);
 		if (ret)
 			return ret;
 
diff --git a/sound/soc/omap/rx51.c b/sound/soc/omap/rx51.c
index 611179c..46fe697 100644
--- a/sound/soc/omap/rx51.c
+++ b/sound/soc/omap/rx51.c
@@ -317,9 +317,9 @@ static int rx51_aic34_init(struct snd_soc_pcm_runtime *rtd)
 		return err;
 
 	/* AV jack detection */
-	err = snd_soc_jack_new(codec, "AV Jack",
-			       SND_JACK_HEADSET | SND_JACK_VIDEOOUT,
-			       &rx51_av_jack);
+	err = snd_soc_jack_new(codec, "AV",
+	                       SND_JACK_HEADSET | SND_JACK_VIDEOOUT,
+	                       0, &rx51_av_jack);
 	if (err)
 		return err;
 	err = snd_soc_jack_add_gpios(&rx51_av_jack,
diff --git a/sound/soc/pxa/hx4700.c b/sound/soc/pxa/hx4700.c
index dcc9b04..3ffe48b 100644
--- a/sound/soc/pxa/hx4700.c
+++ b/sound/soc/pxa/hx4700.c
@@ -137,8 +137,8 @@ static int hx4700_ak4641_init(struct snd_soc_pcm_runtime *rtd)
 	snd_soc_dapm_nc_pin(dapm, "AUX");
 
 	/* Jack detection API stuff */
-	err = snd_soc_jack_new(codec, "Headphone Jack",
-				SND_JACK_HEADPHONE, &hs_jack);
+	err = snd_soc_jack_new(codec, "Headphone",
+	                       SND_JACK_HEADPHONE, 0, &hs_jack);
 	if (err)
 		return err;
 
diff --git a/sound/soc/pxa/palm27x.c b/sound/soc/pxa/palm27x.c
index e1ffcdd..7caba46 100644
--- a/sound/soc/pxa/palm27x.c
+++ b/sound/soc/pxa/palm27x.c
@@ -97,8 +97,8 @@ static int palm27x_ac97_init(struct snd_soc_pcm_runtime *rtd)
 	snd_soc_dapm_nc_pin(dapm, "MIC2");
 
 	/* Jack detection API stuff */
-	err = snd_soc_jack_new(codec, "Headphone Jack",
-				SND_JACK_HEADPHONE, &hs_jack);
+	err = snd_soc_jack_new(codec, "Headphone",
+	                       SND_JACK_HEADPHONE, 0, &hs_jack);
 	if (err)
 		return err;
 
diff --git a/sound/soc/pxa/ttc-dkb.c b/sound/soc/pxa/ttc-dkb.c
index f4ea4f6..1220a51 100644
--- a/sound/soc/pxa/ttc-dkb.c
+++ b/sound/soc/pxa/ttc-dkb.c
@@ -86,13 +86,13 @@ static int ttc_pm860x_init(struct snd_soc_pcm_runtime *rtd)
 	snd_soc_dapm_disable_pin(dapm, "Headset Stereophone");
 
 	/* Headset jack detection */
-	snd_soc_jack_new(codec, "Headphone Jack", SND_JACK_HEADPHONE
+	snd_soc_jack_new(codec, "Headphone", SND_JACK_HEADPHONE
 			| SND_JACK_BTN_0 | SND_JACK_BTN_1 | SND_JACK_BTN_2,
-			&hs_jack);
+	                 0, &hs_jack);
 	snd_soc_jack_add_pins(&hs_jack, ARRAY_SIZE(hs_jack_pins),
 			      hs_jack_pins);
-	snd_soc_jack_new(codec, "Microphone Jack", SND_JACK_MICROPHONE,
-			 &mic_jack);
+	snd_soc_jack_new(codec, "Microphone", SND_JACK_MICROPHONE,
+	                 0, &mic_jack);
 	snd_soc_jack_add_pins(&mic_jack, ARRAY_SIZE(mic_jack_pins),
 			      mic_jack_pins);
 
diff --git a/sound/soc/pxa/z2.c b/sound/soc/pxa/z2.c
index 76ccb17..2bbb2de 100644
--- a/sound/soc/pxa/z2.c
+++ b/sound/soc/pxa/z2.c
@@ -143,8 +143,8 @@ static int z2_wm8750_init(struct snd_soc_pcm_runtime *rtd)
 	snd_soc_dapm_disable_pin(dapm, "MONO1");
 
 	/* Jack detection API stuff */
-	ret = snd_soc_jack_new(codec, "Headset Jack", SND_JACK_HEADSET,
-				&hs_jack);
+	ret = snd_soc_jack_new(codec, "Headset", SND_JACK_HEADSET,
+	                       0, &hs_jack);
 	if (ret)
 		goto err;
 
diff --git a/sound/soc/samsung/goni_wm8994.c b/sound/soc/samsung/goni_wm8994.c
index 415ad81..d76072d 100644
--- a/sound/soc/samsung/goni_wm8994.c
+++ b/sound/soc/samsung/goni_wm8994.c
@@ -115,8 +115,8 @@ static int goni_wm8994_init(struct snd_soc_pcm_runtime *rtd)
 
 	/* Headset jack detection */
 	ret = snd_soc_jack_new(codec, "Headset Jack",
-			SND_JACK_HEADSET | SND_JACK_MECHANICAL | SND_JACK_AVOUT,
-			&jack);
+	                       SND_JACK_HEADSET | SND_JACK_MECHANICAL | SND_JACK_AVOUT,
+	                       0, &jack);
 	if (ret)
 		return ret;
 
diff --git a/sound/soc/samsung/h1940_uda1380.c b/sound/soc/samsung/h1940_uda1380.c
index fa91376..f8cf0cf 100644
--- a/sound/soc/samsung/h1940_uda1380.c
+++ b/sound/soc/samsung/h1940_uda1380.c
@@ -186,8 +186,8 @@ static int h1940_uda1380_init(struct snd_soc_pcm_runtime *rtd)
 	snd_soc_dapm_enable_pin(dapm, "Speaker");
 	snd_soc_dapm_enable_pin(dapm, "Mic Jack");
 
-	snd_soc_jack_new(codec, "Headphone Jack", SND_JACK_HEADPHONE,
-		&hp_jack);
+	snd_soc_jack_new(codec, "Headphone", SND_JACK_HEADPHONE,
+	                 0, &hp_jack);
 
 	snd_soc_jack_add_pins(&hp_jack, ARRAY_SIZE(hp_jack_pins),
 		hp_jack_pins);
diff --git a/sound/soc/samsung/littlemill.c b/sound/soc/samsung/littlemill.c
index bfb91f3..46344f3 100644
--- a/sound/soc/samsung/littlemill.c
+++ b/sound/soc/samsung/littlemill.c
@@ -261,11 +261,11 @@ static int littlemill_late_probe(struct snd_soc_card *card)
 		return ret;
 
 	ret = snd_soc_jack_new(codec, "Headset",
-			       SND_JACK_HEADSET | SND_JACK_MECHANICAL |
-			       SND_JACK_BTN_0 | SND_JACK_BTN_1 |
-			       SND_JACK_BTN_2 | SND_JACK_BTN_3 |
-			       SND_JACK_BTN_4 | SND_JACK_BTN_5,
-			       &littlemill_headset);
+	                       SND_JACK_HEADSET | SND_JACK_MECHANICAL |
+	                       SND_JACK_BTN_0 | SND_JACK_BTN_1 |
+	                       SND_JACK_BTN_2 | SND_JACK_BTN_3 |
+	                       SND_JACK_BTN_4 | SND_JACK_BTN_5,
+	                       0, &littlemill_headset);
 	if (ret)
 		return ret;
 
diff --git a/sound/soc/samsung/lowland.c b/sound/soc/samsung/lowland.c
index 570cf52..1e6a3df 100644
--- a/sound/soc/samsung/lowland.c
+++ b/sound/soc/samsung/lowland.c
@@ -57,9 +57,9 @@ static int lowland_wm5100_init(struct snd_soc_pcm_runtime *rtd)
 	}
 
 	ret = snd_soc_jack_new(codec, "Headset",
-			       SND_JACK_LINEOUT | SND_JACK_HEADSET |
-			       SND_JACK_BTN_0,
-			       &lowland_headset);
+	                       SND_JACK_LINEOUT | SND_JACK_HEADSET |
+	                       SND_JACK_BTN_0,
+	                       0, &lowland_headset);
 	if (ret)
 		return ret;
 
diff --git a/sound/soc/samsung/rx1950_uda1380.c b/sound/soc/samsung/rx1950_uda1380.c
index 704460a..f62b845 100644
--- a/sound/soc/samsung/rx1950_uda1380.c
+++ b/sound/soc/samsung/rx1950_uda1380.c
@@ -231,8 +231,8 @@ static int rx1950_uda1380_init(struct snd_soc_pcm_runtime *rtd)
 	snd_soc_dapm_enable_pin(dapm, "Speaker");
 	snd_soc_dapm_enable_pin(dapm, "Mic Jack");
 
-	snd_soc_jack_new(codec, "Headphone Jack", SND_JACK_HEADPHONE,
-		&hp_jack);
+	snd_soc_jack_new(codec, "Headphone", SND_JACK_HEADPHONE,
+	                 0, &hp_jack);
 
 	snd_soc_jack_add_pins(&hp_jack, ARRAY_SIZE(hp_jack_pins),
 		hp_jack_pins);
diff --git a/sound/soc/samsung/smartq_wm8987.c b/sound/soc/samsung/smartq_wm8987.c
index 58ae323..053ceea 100644
--- a/sound/soc/samsung/smartq_wm8987.c
+++ b/sound/soc/samsung/smartq_wm8987.c
@@ -166,8 +166,8 @@ static int smartq_wm8987_init(struct snd_soc_pcm_runtime *rtd)
 	snd_soc_dapm_disable_pin(dapm, "Headphone Jack");
 
 	/* Headphone jack detection */
-	err = snd_soc_jack_new(codec, "Headphone Jack",
-			       SND_JACK_HEADPHONE, &smartq_jack);
+	err = snd_soc_jack_new(codec, "Headphone",
+	                       SND_JACK_HEADPHONE, 0, &smartq_jack);
 	if (err)
 		return err;
 
diff --git a/sound/soc/samsung/speyside.c b/sound/soc/samsung/speyside.c
index 57df90d..feff3fb 100644
--- a/sound/soc/samsung/speyside.c
+++ b/sound/soc/samsung/speyside.c
@@ -154,9 +154,9 @@ static int speyside_wm8996_init(struct snd_soc_pcm_runtime *rtd)
 	gpio_direction_output(WM8996_HPSEL_GPIO, speyside_jack_polarity);
 
 	ret = snd_soc_jack_new(codec, "Headset",
-			       SND_JACK_LINEOUT | SND_JACK_HEADSET |
-			       SND_JACK_BTN_0,
-			       &speyside_headset);
+	                       SND_JACK_LINEOUT | SND_JACK_HEADSET |
+	                       SND_JACK_BTN_0,
+	                       0, &speyside_headset);
 	if (ret)
 		return ret;
 
diff --git a/sound/soc/samsung/tobermory.c b/sound/soc/samsung/tobermory.c
index f21ff60..e498e7c 100644
--- a/sound/soc/samsung/tobermory.c
+++ b/sound/soc/samsung/tobermory.c
@@ -178,8 +178,8 @@ static int tobermory_late_probe(struct snd_soc_card *card)
 		return ret;
 
 	ret = snd_soc_jack_new(codec, "Headset",
-			       SND_JACK_HEADSET | SND_JACK_BTN_0,
-			       &tobermory_headset);
+	                       SND_JACK_HEADSET | SND_JACK_BTN_0,
+	                       0, &tobermory_headset);
 	if (ret)
 		return ret;
 
diff --git a/sound/soc/soc-jack.c b/sound/soc/soc-jack.c
index 0bb5ccc..ecd1601 100644
--- a/sound/soc/soc-jack.c
+++ b/sound/soc/soc-jack.c
@@ -34,7 +34,7 @@
  * On success jack will be initialised.
  */
 int snd_soc_jack_new(struct snd_soc_codec *codec, const char *id, int type,
-		     struct snd_soc_jack *jack)
+                     int idx, struct snd_soc_jack *jack)
 {
 	mutex_init(&jack->mutex);
 	jack->codec = codec;
@@ -42,7 +42,7 @@ int snd_soc_jack_new(struct snd_soc_codec *codec, const char *id, int type,
 	INIT_LIST_HEAD(&jack->jack_zones);
 	BLOCKING_INIT_NOTIFIER_HEAD(&jack->notifier);
 
-	return snd_jack_new(codec->card->snd_card, id, type, &jack->jack);
+	return snd_jack_new(codec->card->snd_card, id, type, idx, &jack->jack);
 }
 EXPORT_SYMBOL_GPL(snd_soc_jack_new);
 
diff --git a/sound/soc/tegra/tegra_alc5632.c b/sound/soc/tegra/tegra_alc5632.c
index 48d05d9..cfacc27 100644
--- a/sound/soc/tegra/tegra_alc5632.c
+++ b/sound/soc/tegra/tegra_alc5632.c
@@ -109,8 +109,8 @@ static int tegra_alc5632_asoc_init(struct snd_soc_pcm_runtime *rtd)
 	struct snd_soc_dapm_context *dapm = &codec->dapm;
 	struct tegra_alc5632 *machine = snd_soc_card_get_drvdata(codec->card);
 
-	snd_soc_jack_new(codec, "Headset Jack", SND_JACK_HEADSET,
-			 &tegra_alc5632_hs_jack);
+	snd_soc_jack_new(codec, "Headset", SND_JACK_HEADSET,
+	                 0, &tegra_alc5632_hs_jack);
 	snd_soc_jack_add_pins(&tegra_alc5632_hs_jack,
 			ARRAY_SIZE(tegra_alc5632_hs_jack_pins),
 			tegra_alc5632_hs_jack_pins);
diff --git a/sound/soc/tegra/tegra_rt5640.c b/sound/soc/tegra/tegra_rt5640.c
index 08794f9..aad20f2 100644
--- a/sound/soc/tegra/tegra_rt5640.c
+++ b/sound/soc/tegra/tegra_rt5640.c
@@ -112,7 +112,7 @@ static int tegra_rt5640_asoc_init(struct snd_soc_pcm_runtime *rtd)
 	struct tegra_rt5640 *machine = snd_soc_card_get_drvdata(codec->card);
 
 	snd_soc_jack_new(codec, "Headphones", SND_JACK_HEADPHONE,
-			 &tegra_rt5640_hp_jack);
+	                 0, &tegra_rt5640_hp_jack);
 	snd_soc_jack_add_pins(&tegra_rt5640_hp_jack,
 			ARRAY_SIZE(tegra_rt5640_hp_jack_pins),
 			tegra_rt5640_hp_jack_pins);
diff --git a/sound/soc/tegra/tegra_wm8903.c b/sound/soc/tegra/tegra_wm8903.c
index 4ac7373..cd297f5 100644
--- a/sound/soc/tegra/tegra_wm8903.c
+++ b/sound/soc/tegra/tegra_wm8903.c
@@ -178,8 +178,8 @@ static int tegra_wm8903_init(struct snd_soc_pcm_runtime *rtd)
 
 	if (gpio_is_valid(machine->gpio_hp_det)) {
 		tegra_wm8903_hp_jack_gpio.gpio = machine->gpio_hp_det;
-		snd_soc_jack_new(codec, "Headphone Jack", SND_JACK_HEADPHONE,
-				&tegra_wm8903_hp_jack);
+		snd_soc_jack_new(codec, "Headphone", SND_JACK_HEADPHONE,
+		                 0, &tegra_wm8903_hp_jack);
 		snd_soc_jack_add_pins(&tegra_wm8903_hp_jack,
 					ARRAY_SIZE(tegra_wm8903_hp_jack_pins),
 					tegra_wm8903_hp_jack_pins);
@@ -188,8 +188,8 @@ static int tegra_wm8903_init(struct snd_soc_pcm_runtime *rtd)
 					&tegra_wm8903_hp_jack_gpio);
 	}
 
-	snd_soc_jack_new(codec, "Mic Jack", SND_JACK_MICROPHONE,
-			 &tegra_wm8903_mic_jack);
+	snd_soc_jack_new(codec, "Mic", SND_JACK_MICROPHONE,
+	                 0, &tegra_wm8903_mic_jack);
 	snd_soc_jack_add_pins(&tegra_wm8903_mic_jack,
 			      ARRAY_SIZE(tegra_wm8903_mic_jack_pins),
 			      tegra_wm8903_mic_jack_pins);
-- 
1.8.1.4

[PATCH 4/4] ALSA: oxygen: Updating jack implementation according new ALSA Jacks

[Felipe F. Tonello]

From: "Felipe F. Tonello" <eu@felipetonello.com>

ALSA standard jacks already are implemented using ALSA KControl.
So there is no need implement that itself or to use snd_jack
for input events only.

Signed-off-by: Felipe F. Tonello <eu@felipetonello.com>
---
 sound/pci/oxygen/xonar_wm87x6.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/pci/oxygen/xonar_wm87x6.c b/sound/pci/oxygen/xonar_wm87x6.c
index 6ce6860..d3816b1 100644
--- a/sound/pci/oxygen/xonar_wm87x6.c
+++ b/sound/pci/oxygen/xonar_wm87x6.c
@@ -286,7 +286,7 @@ static void xonar_ds_init(struct oxygen *chip)
 	xonar_enable_output(chip);
 
 	snd_jack_new(chip->card, "Headphone",
-		     SND_JACK_HEADPHONE, &data->hp_jack);
+	             SND_JACK_HEADPHONE, 0, &data->hp_jack);
 	xonar_ds_handle_hp_jack(chip);
 
 	snd_component_add(chip->card, "WM8776");
-- 
1.8.1.4

Re: [PATCH 1/4] ALSA: Added jack detection kcontrol support

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 581 bytes --]

On Fri, Jul 26, 2013 at 11:25:30AM -0700, Felipe F. Tonello wrote:

> -	snprintf(kctl->id.name, sizeof(kctl->id.name), "%s Jack", name);
> +
> +	strlcpy(kctl->id.name, name, sizeof(kctl->id.name));

This looks like it'd break existing users unless they are updated to
remove the "Jack" from their calls?

>   * @type:  a bitmask of enum snd_jack_type values that can be detected by
>   *         this jack
> + * @idx:   index of this control item
>   * @jjack: Used to provide the allocated jack object to the caller.

"The index of the ALSA control created to represent the jack.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 3/4] ALSA: SoC: Updating jack implementation according new ALSA Jacks

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 606 bytes --]

On Fri, Jul 26, 2013 at 11:25:32AM -0700, Felipe F. Tonello wrote:
> From: "Felipe F. Tonello" <eu@felipetonello.com>
> 
> ALSA standard jacks already are implemented using ALSA KControl.
> So there is no need implement that itself or to use snd_jack for input
> events only.

This changelog doesn't correspond to the code change - what this is
doing is updating the ASoC jack support to add the ability to specify a
jack index when creating a jack.  There's nothing to do with removing
duplicate kcontrol creation here at all.

The change itself seems fine but it's not accurately described.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 3/4] ALSA: SoC: Updating jack implementation according new ALSA Jacks

[Mark Brown]

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

On Fri, Jul 26, 2013 at 11:25:32AM -0700, Felipe F. Tonello wrote:
> From: "Felipe F. Tonello" <eu@felipetonello.com>
> 
> ALSA standard jacks already are implemented using ALSA KControl.
> So there is no need implement that itself or to use snd_jack for input
> events only.

This changelog doesn't correspond to the code change - what this is
doing is updating the ASoC jack support to add the ability to specify a
jack index when creating a jack.  There's nothing to do with removing
duplicate kcontrol creation here at all.

The change itself seems fine but it's not accurately described.

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

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

Re: [PATCH 1/4] ALSA: Added jack detection kcontrol support

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 691 bytes --]

On Fri, Jul 26, 2013 at 11:25:30AM -0700, Felipe F. Tonello wrote:

> +
> +	/* Update ALSA KControl interface */
> +	snd_kctl_jack_report((struct snd_card *)jack->kctl->private_data, jack->kctl, !!status);

This isn't ideal for multi-function jacks like headsets - it will report
a single boolean value for the jack regardless of what's plugged in
meaning userpace can't do things like figure out if a headset or
headphone is present.  It's probably OK for any realistic input button
since you're not going to get an input button without other things being
present.

What I'd expect to happen here is that for multi function jacks we
create a control per function if the controls are valid.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/4] ALSA: Added jack detection kcontrol support

[Mark Brown]

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

On Fri, Jul 26, 2013 at 11:25:30AM -0700, Felipe F. Tonello wrote:

> +
> +	/* Update ALSA KControl interface */
> +	snd_kctl_jack_report((struct snd_card *)jack->kctl->private_data, jack->kctl, !!status);

This isn't ideal for multi-function jacks like headsets - it will report
a single boolean value for the jack regardless of what's plugged in
meaning userpace can't do things like figure out if a headset or
headphone is present.  It's probably OK for any realistic input button
since you're not going to get an input button without other things being
present.

What I'd expect to happen here is that for multi function jacks we
create a control per function if the controls are valid.

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

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

Re: [PATCH 4/4] ALSA: oxygen: Updating jack implementation according new ALSA Jacks

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 662 bytes --]

On Fri, Jul 26, 2013 at 11:25:33AM -0700, Felipe F. Tonello wrote:
> From: "Felipe F. Tonello" <eu@felipetonello.com>
> 
> ALSA standard jacks already are implemented using ALSA KControl.
> So there is no need implement that itself or to use snd_jack
> for input events only.

Similar changlog comment as on the ASoC patch and...

>  	snd_jack_new(chip->card, "Headphone",
> -		     SND_JACK_HEADPHONE, &data->hp_jack);
> +	             SND_JACK_HEADPHONE, 0, &data->hp_jack);
>  	xonar_ds_handle_hp_jack(chip);

...this really ought to be done as part of the commit that adds the
parameter since it breaks the build until this patch is applied.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 4/4] ALSA: oxygen: Updating jack implementation according new ALSA Jacks

[Mark Brown]

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

On Fri, Jul 26, 2013 at 11:25:33AM -0700, Felipe F. Tonello wrote:
> From: "Felipe F. Tonello" <eu@felipetonello.com>
> 
> ALSA standard jacks already are implemented using ALSA KControl.
> So there is no need implement that itself or to use snd_jack
> for input events only.

Similar changlog comment as on the ASoC patch and...

>  	snd_jack_new(chip->card, "Headphone",
> -		     SND_JACK_HEADPHONE, &data->hp_jack);
> +	             SND_JACK_HEADPHONE, 0, &data->hp_jack);
>  	xonar_ds_handle_hp_jack(chip);

...this really ought to be done as part of the commit that adds the
parameter since it breaks the build until this patch is applied.

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

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

Re: [PATCH 4/4] ALSA: oxygen: Updating jack implementation according new ALSA Jacks

[Felipe Tonello]

Hi Mark,

On Fri, Jul 26, 2013 at 11:56 AM, Mark Brown <broonie@kernel.org> wrote:
> On Fri, Jul 26, 2013 at 11:25:33AM -0700, Felipe F. Tonello wrote:
>> From: "Felipe F. Tonello" <eu@felipetonello.com>
>>
>> ALSA standard jacks already are implemented using ALSA KControl.
>> So there is no need implement that itself or to use snd_jack
>> for input events only.
>
> Similar changlog comment as on the ASoC patch and...

Ok.

>
>>       snd_jack_new(chip->card, "Headphone",
>> -                  SND_JACK_HEADPHONE, &data->hp_jack);
>> +                  SND_JACK_HEADPHONE, 0, &data->hp_jack);
>>       xonar_ds_handle_hp_jack(chip);
>
> ...this really ought to be done as part of the commit that adds the
> parameter since it breaks the build until this patch is applied.

But that's why is a patch series. But as you say, are you suggesting
me to propose this changes in one patch only?

Felipe Tonello

Re: [PATCH 4/4] ALSA: oxygen: Updating jack implementation according new ALSA Jacks

[Felipe Tonello]

Hi Mark,

On Fri, Jul 26, 2013 at 11:56 AM, Mark Brown <broonie@kernel.org> wrote:
> On Fri, Jul 26, 2013 at 11:25:33AM -0700, Felipe F. Tonello wrote:
>> From: "Felipe F. Tonello" <eu@felipetonello.com>
>>
>> ALSA standard jacks already are implemented using ALSA KControl.
>> So there is no need implement that itself or to use snd_jack
>> for input events only.
>
> Similar changlog comment as on the ASoC patch and...

Ok.

>
>>       snd_jack_new(chip->card, "Headphone",
>> -                  SND_JACK_HEADPHONE, &data->hp_jack);
>> +                  SND_JACK_HEADPHONE, 0, &data->hp_jack);
>>       xonar_ds_handle_hp_jack(chip);
>
> ...this really ought to be done as part of the commit that adds the
> parameter since it breaks the build until this patch is applied.

But that's why is a patch series. But as you say, are you suggesting
me to propose this changes in one patch only?

Felipe Tonello

Re: [PATCH 1/4] ALSA: Added jack detection kcontrol support

[Felipe Tonello]

Hi Mark,

On Fri, Jul 26, 2013 at 11:46 AM, Mark Brown <broonie@kernel.org> wrote:
> On Fri, Jul 26, 2013 at 11:25:30AM -0700, Felipe F. Tonello wrote:
>
>> -     snprintf(kctl->id.name, sizeof(kctl->id.name), "%s Jack", name);
>> +
>> +     strlcpy(kctl->id.name, name, sizeof(kctl->id.name));
>
> This looks like it'd break existing users unless they are updated to
> remove the "Jack" from their calls?

Yes, that's why I appended the " Jack" in snd_jack_new().

>
>>   * @type:  a bitmask of enum snd_jack_type values that can be detected by
>>   *         this jack
>> + * @idx:   index of this control item
>>   * @jjack: Used to provide the allocated jack object to the caller.
>
> "The index of the ALSA control created to represent the jack.

Ok.

Felipe Tonello

Re: [PATCH 1/4] ALSA: Added jack detection kcontrol support

[Felipe Tonello]

Hi Mark,

On Fri, Jul 26, 2013 at 11:46 AM, Mark Brown <broonie@kernel.org> wrote:
> On Fri, Jul 26, 2013 at 11:25:30AM -0700, Felipe F. Tonello wrote:
>
>> -     snprintf(kctl->id.name, sizeof(kctl->id.name), "%s Jack", name);
>> +
>> +     strlcpy(kctl->id.name, name, sizeof(kctl->id.name));
>
> This looks like it'd break existing users unless they are updated to
> remove the "Jack" from their calls?

Yes, that's why I appended the " Jack" in snd_jack_new().

>
>>   * @type:  a bitmask of enum snd_jack_type values that can be detected by
>>   *         this jack
>> + * @idx:   index of this control item
>>   * @jjack: Used to provide the allocated jack object to the caller.
>
> "The index of the ALSA control created to represent the jack.

Ok.

Felipe Tonello

Re: [PATCH 1/4] ALSA: Added jack detection kcontrol support

[Felipe Tonello]

Mark,

On Fri, Jul 26, 2013 at 11:54 AM, Mark Brown <broonie@kernel.org> wrote:
> On Fri, Jul 26, 2013 at 11:25:30AM -0700, Felipe F. Tonello wrote:
>
>> +
>> +     /* Update ALSA KControl interface */
>> +     snd_kctl_jack_report((struct snd_card *)jack->kctl->private_data, jack->kctl, !!status);
>
> This isn't ideal for multi-function jacks like headsets - it will report
> a single boolean value for the jack regardless of what's plugged in
> meaning userpace can't do things like figure out if a headset or
> headphone is present.  It's probably OK for any realistic input button
> since you're not going to get an input button without other things being
> present.

The KControl for Jack is boolean anyway. You can check it with "amixer
contents". user-space can figure out based on the control name. At
least PulseAudio does that way.

>
> What I'd expect to happen here is that for multi function jacks we
> create a control per function if the controls are valid.

Do you mean based on snd_jack_types?

Felipe Tonello

Re: [PATCH 3/4] ALSA: SoC: Updating jack implementation according new ALSA Jacks

[Felipe Tonello]

On Fri, Jul 26, 2013 at 11:50 AM, Mark Brown <broonie@kernel.org> wrote:
> On Fri, Jul 26, 2013 at 11:25:32AM -0700, Felipe F. Tonello wrote:
>> From: "Felipe F. Tonello" <eu@felipetonello.com>
>>
>> ALSA standard jacks already are implemented using ALSA KControl.
>> So there is no need implement that itself or to use snd_jack for input
>> events only.
>
> This changelog doesn't correspond to the code change - what this is
> doing is updating the ASoC jack support to add the ability to specify a
> jack index when creating a jack.  There's nothing to do with removing
> duplicate kcontrol creation here at all.
>
> The change itself seems fine but it's not accurately described.

Ok.

[PATCH v2 0/3] ALSA: Implement core jack support for kcontrol

[Felipe F. Tonello]

From: "Felipe F. Tonello" <eu@felipetonello.com>

Sending series again.

Difference is that previous Patch 4 was squashed with Patch 2. Also fixed
comment on Patche 3.

Felipe F. Tonello (3):
  ALSA: Added jack detection kcontrol support
  ALSA: pci: HDA/oxygen: Updating jack implementation according new ALSA
    Jacks
  ALSA: SoC: Updating jack implementation according new ALSA Jacks

 include/sound/jack.h               |  6 ++++--
 include/sound/soc.h                |  2 +-
 sound/core/Kconfig                 |  1 +
 sound/core/ctljack.c               |  3 ++-
 sound/core/jack.c                  | 29 +++++++++++++++++++++++++++--
 sound/pci/hda/Kconfig              |  8 --------
 sound/pci/hda/hda_codec.h          |  2 --
 sound/pci/hda/hda_jack.c           | 38 +++++++++++++++++---------------------
 sound/pci/hda/hda_jack.h           |  4 +---
 sound/pci/oxygen/xonar_wm87x6.c    |  2 +-
 sound/soc/fsl/wm1133-ev1.c         |  4 ++--
 sound/soc/mid-x86/mfld_machine.c   |  6 +++---
 sound/soc/omap/ams-delta.c         |  2 +-
 sound/soc/omap/omap-abe-twl6040.c  |  4 ++--
 sound/soc/omap/omap-twl4030.c      |  4 ++--
 sound/soc/omap/rx51.c              |  6 +++---
 sound/soc/pxa/hx4700.c             |  4 ++--
 sound/soc/pxa/palm27x.c            |  4 ++--
 sound/soc/pxa/ttc-dkb.c            |  8 ++++----
 sound/soc/pxa/z2.c                 |  4 ++--
 sound/soc/samsung/goni_wm8994.c    |  4 ++--
 sound/soc/samsung/h1940_uda1380.c  |  4 ++--
 sound/soc/samsung/littlemill.c     | 10 +++++-----
 sound/soc/samsung/lowland.c        |  6 +++---
 sound/soc/samsung/rx1950_uda1380.c |  4 ++--
 sound/soc/samsung/smartq_wm8987.c  |  4 ++--
 sound/soc/samsung/speyside.c       |  6 +++---
 sound/soc/samsung/tobermory.c      |  4 ++--
 sound/soc/soc-jack.c               |  4 ++--
 sound/soc/tegra/tegra_alc5632.c    |  4 ++--
 sound/soc/tegra/tegra_rt5640.c     |  2 +-
 sound/soc/tegra/tegra_wm8903.c     |  8 ++++----
 32 files changed, 107 insertions(+), 94 deletions(-)

-- 
1.8.1.4

[PATCH v2 1/3] ALSA: Added jack detection kcontrol support

[Felipe F. Tonello]

From: "Felipe F. Tonello" <eu@felipetonello.com>

This patch adds jack support for alsa kcontrol.

This support is necessary since the new kcontrol is used by user-space
daemons, such as PulseAudio(>=2.0), to do jack detection.)

Signed-off-by: Felipe F. Tonello <eu@felipetonello.com>
---
 include/sound/jack.h |  6 ++++--
 sound/core/Kconfig   |  1 +
 sound/core/ctljack.c |  3 ++-
 sound/core/jack.c    | 29 +++++++++++++++++++++++++++--
 4 files changed, 34 insertions(+), 5 deletions(-)

diff --git a/include/sound/jack.h b/include/sound/jack.h
index 5891657..dc62b74 100644
--- a/include/sound/jack.h
+++ b/include/sound/jack.h
@@ -26,6 +26,7 @@
 #include <sound/core.h>
 
 struct input_dev;
+struct snd_kcontrol;
 
 /**
  * Jack types which can be reported.  These values are used as a
@@ -58,6 +59,7 @@ enum snd_jack_types {
 
 struct snd_jack {
 	struct input_dev *input_dev;
+	struct snd_kcontrol *kctl;
 	int registered;
 	int type;
 	const char *id;
@@ -70,7 +72,7 @@ struct snd_jack {
 #ifdef CONFIG_SND_JACK
 
 int snd_jack_new(struct snd_card *card, const char *id, int type,
-		 struct snd_jack **jack);
+                 int idx, struct snd_jack **jack);
 void snd_jack_set_parent(struct snd_jack *jack, struct device *parent);
 int snd_jack_set_key(struct snd_jack *jack, enum snd_jack_types type,
 		     int keytype);
@@ -80,7 +82,7 @@ void snd_jack_report(struct snd_jack *jack, int status);
 #else
 
 static inline int snd_jack_new(struct snd_card *card, const char *id, int type,
-			       struct snd_jack **jack)
+                               int idx, struct snd_jack **jack)
 {
 	return 0;
 }
diff --git a/sound/core/Kconfig b/sound/core/Kconfig
index c0c2f57..8167615 100644
--- a/sound/core/Kconfig
+++ b/sound/core/Kconfig
@@ -20,6 +20,7 @@ config SND_COMPRESS_OFFLOAD
 # to avoid having to force INPUT on.
 config SND_JACK
 	bool
+	select SND_KCTL_JACK
 
 config SND_SEQUENCER
 	tristate "Sequencer support"
diff --git a/sound/core/ctljack.c b/sound/core/ctljack.c
index e4b38fb..59aa6d0 100644
--- a/sound/core/ctljack.c
+++ b/sound/core/ctljack.c
@@ -38,7 +38,8 @@ snd_kctl_jack_new(const char *name, int idx, void *private_data)
 	kctl = snd_ctl_new1(&jack_detect_kctl, private_data);
 	if (!kctl)
 		return NULL;
-	snprintf(kctl->id.name, sizeof(kctl->id.name), "%s Jack", name);
+
+	strlcpy(kctl->id.name, name, sizeof(kctl->id.name));
 	kctl->id.index = idx;
 	kctl->private_value = 0;
 	return kctl;
diff --git a/sound/core/jack.c b/sound/core/jack.c
index b35fe73..b2757b1 100644
--- a/sound/core/jack.c
+++ b/sound/core/jack.c
@@ -24,6 +24,7 @@
 #include <linux/module.h>
 #include <sound/jack.h>
 #include <sound/core.h>
+#include <sound/control.h>
 
 static int jack_switch_types[SND_JACK_SWITCH_TYPES] = {
 	SW_HEADPHONE_INSERT,
@@ -48,6 +49,7 @@ static int snd_jack_dev_free(struct snd_device *device)
 	else
 		input_free_device(jack->input_dev);
 
+	snd_ctl_remove(device->card, jack->kctl);
 	kfree(jack->id);
 	kfree(jack);
 
@@ -85,26 +87,36 @@ static int snd_jack_dev_register(struct snd_device *device)
 	if (err == 0)
 		jack->registered = 1;
 
+	/* We don't need to free the control, it's freed by snd_ctl_add itself
+	   if an error occur */
+	err = snd_ctl_add(card, jack->kctl);
+
 	return err;
 }
 
 /**
  * snd_jack_new - Create a new jack
  * @card:  the card instance
- * @id:    an identifying string for this jack
+ * @id:    an identifying string for this jack, " Jack" is appended to the
+ *         string
  * @type:  a bitmask of enum snd_jack_type values that can be detected by
  *         this jack
+ * @idx:   index of this control item
  * @jjack: Used to provide the allocated jack object to the caller.
  *
  * Creates a new jack object.
  *
+ * This function creates a Jack Kcontrol, which is exported to user space via
+ * ALSA Controls.
+ *
  * Return: Zero if successful, or a negative error code on failure.
  * On success @jjack will be initialised.
  */
 int snd_jack_new(struct snd_card *card, const char *id, int type,
-		 struct snd_jack **jjack)
+                 int idx, struct snd_jack **jjack)
 {
 	struct snd_jack *jack;
+	struct snd_kcontrol *kctl;
 	int err;
 	int i;
 	static struct snd_device_ops ops = {
@@ -117,6 +129,7 @@ int snd_jack_new(struct snd_card *card, const char *id, int type,
 		return -ENOMEM;
 
 	jack->id = kstrdup(id, GFP_KERNEL);
+	sprintf((char *)jack->id, "%s Jack", jack->id);
 
 	jack->input_dev = input_allocate_device();
 	if (jack->input_dev == NULL) {
@@ -137,6 +150,15 @@ int snd_jack_new(struct snd_card *card, const char *id, int type,
 	if (err < 0)
 		goto fail_input;
 
+	/* card is the private_data */
+	kctl = snd_kctl_jack_new(jack->id, idx, card);
+	if (!kctl) {
+		err = -ENOMEM;
+		goto fail_input;
+	}
+
+	jack->kctl = kctl;
+
 	*jjack = jack;
 
 	return 0;
@@ -239,6 +261,9 @@ void snd_jack_report(struct snd_jack *jack, int status)
 	}
 
 	input_sync(jack->input_dev);
+
+	/* Update ALSA KControl interface */
+	snd_kctl_jack_report((struct snd_card *)jack->kctl->private_data, jack->kctl, !!status);
 }
 EXPORT_SYMBOL(snd_jack_report);
 
-- 
1.8.1.4

[PATCH v2 2/3] ALSA: pci: HDA/oxygen: Updating jack implementation according new ALSA Jacks

[Felipe F. Tonello]

From: "Felipe F. Tonello" <eu@felipetonello.com>

ALSA standard jacks already are implemented using ALSA KControl. So there is
no need implement that itself or to use snd_jack for input events only.

Also updating oxygen codec jack implementation to support new jack API.

Signed-off-by: Felipe F. Tonello <eu@felipetonello.com>
---
 sound/pci/hda/Kconfig           |  8 --------
 sound/pci/hda/hda_codec.h       |  2 --
 sound/pci/hda/hda_jack.c        | 38 +++++++++++++++++---------------------
 sound/pci/hda/hda_jack.h        |  4 +---
 sound/pci/oxygen/xonar_wm87x6.c |  2 +-
 5 files changed, 19 insertions(+), 35 deletions(-)

diff --git a/sound/pci/hda/Kconfig b/sound/pci/hda/Kconfig
index 59c5e9c..561abc7 100644
--- a/sound/pci/hda/Kconfig
+++ b/sound/pci/hda/Kconfig
@@ -65,14 +65,6 @@ config SND_HDA_INPUT_BEEP_MODE
 	  Set 1 to always enable the digital beep interface for HD-audio by
 	  default.
 
-config SND_HDA_INPUT_JACK
-	bool "Support jack plugging notification via input layer"
-	depends on INPUT=y || INPUT=SND
-	select SND_JACK
-	help
-	  Say Y here to enable the jack plugging notification via
-	  input layer.
-
 config SND_HDA_PATCH_LOADER
 	bool "Support initialization patch loading for HD-audio"
 	select FW_LOADER
diff --git a/sound/pci/hda/hda_codec.h b/sound/pci/hda/hda_codec.h
index 701c2e0..ca7be59 100644
--- a/sound/pci/hda/hda_codec.h
+++ b/sound/pci/hda/hda_codec.h
@@ -912,10 +912,8 @@ struct hda_codec {
 	unsigned long jackpoll_interval; /* In jiffies. Zero means no poll, rely on unsol events */
 	struct delayed_work jackpoll_work;
 
-#ifdef CONFIG_SND_HDA_INPUT_JACK
 	/* jack detection */
 	struct snd_array jacks;
-#endif
 
 	/* fix-up list */
 	int fixup_id;
diff --git a/sound/pci/hda/hda_jack.c b/sound/pci/hda/hda_jack.c
index 3fd2973..6be1a0c 100644
--- a/sound/pci/hda/hda_jack.c
+++ b/sound/pci/hda/hda_jack.c
@@ -112,7 +112,6 @@ EXPORT_SYMBOL_HDA(snd_hda_jack_tbl_new);
 
 void snd_hda_jack_tbl_clear(struct hda_codec *codec)
 {
-#ifdef CONFIG_SND_HDA_INPUT_JACK
 	/* free jack instances manually when clearing/reconfiguring */
 	if (!codec->bus->shutdown && codec->jacktbl.list) {
 		struct hda_jack_tbl *jack = codec->jacktbl.list;
@@ -122,7 +121,6 @@ void snd_hda_jack_tbl_clear(struct hda_codec *codec)
 				snd_device_free(codec->bus->card, jack->jack);
 		}
 	}
-#endif
 	snd_array_free(&codec->jacktbl);
 }
 
@@ -283,17 +281,15 @@ void snd_hda_jack_report_sync(struct hda_codec *codec)
 			if (!jack->kctl)
 				continue;
 			state = get_jack_plug_state(jack->pin_sense);
-			snd_kctl_jack_report(codec->bus->card, jack->kctl, state);
-#ifdef CONFIG_SND_HDA_INPUT_JACK
-			if (jack->jack)
+			if (jack->phantom_jack)
+				snd_kctl_jack_report(codec->bus->card, jack->kctl, state);
+			else if (jack->jack)
 				snd_jack_report(jack->jack,
 						state ? jack->type : 0);
-#endif
 		}
 }
 EXPORT_SYMBOL_HDA(snd_hda_jack_report_sync);
 
-#ifdef CONFIG_SND_HDA_INPUT_JACK
 /* guess the jack type from the pin-config */
 static int get_input_jack_type(struct hda_codec *codec, hda_nid_t nid)
 {
@@ -320,7 +316,6 @@ static void hda_free_jack_priv(struct snd_jack *jack)
 	jacks->nid = 0;
 	jacks->jack = NULL;
 }
-#endif
 
 /**
  * snd_hda_jack_add_kctl - Add a kctl for the given pin
@@ -340,29 +335,30 @@ static int __snd_hda_jack_add_kctl(struct hda_codec *codec, hda_nid_t nid,
 		return 0;
 	if (jack->kctl)
 		return 0; /* already created */
-	kctl = snd_kctl_jack_new(name, idx, codec);
-	if (!kctl)
-		return -ENOMEM;
-	err = snd_hda_ctl_add(codec, nid, kctl);
-	if (err < 0)
-		return err;
-	jack->kctl = kctl;
+
 	jack->phantom_jack = !!phantom_jack;
 
-	state = snd_hda_jack_detect(codec, nid);
-	snd_kctl_jack_report(codec->bus->card, kctl, state);
-#ifdef CONFIG_SND_HDA_INPUT_JACK
-	if (!phantom_jack) {
+	/* If it's phantom jack only creates kcontrol jack elem */
+	if (jack->phantom_jack) {
+		kctl = snd_kctl_jack_new(name, idx, codec);
+		if (!kctl)
+			return -ENOMEM;
+		err = snd_hda_ctl_add(codec, nid, kctl);
+		if (err < 0)
+			return err;
+		jack->kctl = kctl;
+	} else {
+		state = snd_hda_jack_detect(codec, nid);
 		jack->type = get_input_jack_type(codec, nid);
 		err = snd_jack_new(codec->bus->card, name, jack->type,
-				   &jack->jack);
+		                   idx, &jack->jack);
 		if (err < 0)
 			return err;
+		jack->kctl = jack->jack->kctl;
 		jack->jack->private_data = jack;
 		jack->jack->private_free = hda_free_jack_priv;
 		snd_jack_report(jack->jack, state ? jack->type : 0);
 	}
-#endif
 	return 0;
 }
 
diff --git a/sound/pci/hda/hda_jack.h b/sound/pci/hda/hda_jack.h
index ec12abd..d4701d2 100644
--- a/sound/pci/hda/hda_jack.h
+++ b/sound/pci/hda/hda_jack.h
@@ -30,11 +30,9 @@ struct hda_jack_tbl {
 	unsigned int phantom_jack:1;    /* a fixed, always present port? */
 	hda_nid_t gating_jack;		/* valid when gating jack plugged */
 	hda_nid_t gated_jack;		/* gated is dependent on this jack */
-	struct snd_kcontrol *kctl;	/* assigned kctl for jack-detection */
-#ifdef CONFIG_SND_HDA_INPUT_JACK
+	struct snd_kcontrol *kctl;      /* assigned kctl for jack-detection */
 	int type;
 	struct snd_jack *jack;
-#endif
 };
 
 struct hda_jack_tbl *
diff --git a/sound/pci/oxygen/xonar_wm87x6.c b/sound/pci/oxygen/xonar_wm87x6.c
index 6ce6860..d3816b1 100644
--- a/sound/pci/oxygen/xonar_wm87x6.c
+++ b/sound/pci/oxygen/xonar_wm87x6.c
@@ -286,7 +286,7 @@ static void xonar_ds_init(struct oxygen *chip)
 	xonar_enable_output(chip);
 
 	snd_jack_new(chip->card, "Headphone",
-		     SND_JACK_HEADPHONE, &data->hp_jack);
+	             SND_JACK_HEADPHONE, 0, &data->hp_jack);
 	xonar_ds_handle_hp_jack(chip);
 
 	snd_component_add(chip->card, "WM8776");
-- 
1.8.1.4

[PATCH v2 3/3] ALSA: SoC: Updating jack implementation according new ALSA Jacks

[Felipe F. Tonello]

From: "Felipe F. Tonello" <eu@felipetonello.com>

Updating the ASoC jack support to add the ability to specify a
jack index when creating it.

Signed-off-by: Felipe F. Tonello <eu@felipetonello.com>
---
 include/sound/soc.h                |  2 +-
 sound/soc/fsl/wm1133-ev1.c         |  4 ++--
 sound/soc/mid-x86/mfld_machine.c   |  6 +++---
 sound/soc/omap/ams-delta.c         |  2 +-
 sound/soc/omap/omap-abe-twl6040.c  |  4 ++--
 sound/soc/omap/omap-twl4030.c      |  4 ++--
 sound/soc/omap/rx51.c              |  6 +++---
 sound/soc/pxa/hx4700.c             |  4 ++--
 sound/soc/pxa/palm27x.c            |  4 ++--
 sound/soc/pxa/ttc-dkb.c            |  8 ++++----
 sound/soc/pxa/z2.c                 |  4 ++--
 sound/soc/samsung/goni_wm8994.c    |  4 ++--
 sound/soc/samsung/h1940_uda1380.c  |  4 ++--
 sound/soc/samsung/littlemill.c     | 10 +++++-----
 sound/soc/samsung/lowland.c        |  6 +++---
 sound/soc/samsung/rx1950_uda1380.c |  4 ++--
 sound/soc/samsung/smartq_wm8987.c  |  4 ++--
 sound/soc/samsung/speyside.c       |  6 +++---
 sound/soc/samsung/tobermory.c      |  4 ++--
 sound/soc/soc-jack.c               |  4 ++--
 sound/soc/tegra/tegra_alc5632.c    |  4 ++--
 sound/soc/tegra/tegra_rt5640.c     |  2 +-
 sound/soc/tegra/tegra_wm8903.c     |  8 ++++----
 23 files changed, 54 insertions(+), 54 deletions(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h
index 6eabee7..31bea52 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -436,7 +436,7 @@ int snd_soc_platform_trigger(struct snd_pcm_substream *substream,
 
 /* Jack reporting */
 int snd_soc_jack_new(struct snd_soc_codec *codec, const char *id, int type,
-		     struct snd_soc_jack *jack);
+                     int idx, struct snd_soc_jack *jack);
 void snd_soc_jack_report(struct snd_soc_jack *jack, int status, int mask);
 int snd_soc_jack_add_pins(struct snd_soc_jack *jack, int count,
 			  struct snd_soc_jack_pin *pins);
diff --git a/sound/soc/fsl/wm1133-ev1.c b/sound/soc/fsl/wm1133-ev1.c
index fce6325..50f96d4 100644
--- a/sound/soc/fsl/wm1133-ev1.c
+++ b/sound/soc/fsl/wm1133-ev1.c
@@ -221,14 +221,14 @@ static int wm1133_ev1_init(struct snd_soc_pcm_runtime *rtd)
 				ARRAY_SIZE(wm1133_ev1_map));
 
 	/* Headphone jack detection */
-	snd_soc_jack_new(codec, "Headphone", SND_JACK_HEADPHONE, &hp_jack);
+	snd_soc_jack_new(codec, "Headphone", SND_JACK_HEADPHONE, 0, &hp_jack);
 	snd_soc_jack_add_pins(&hp_jack, ARRAY_SIZE(hp_jack_pins),
 			      hp_jack_pins);
 	wm8350_hp_jack_detect(codec, WM8350_JDR, &hp_jack, SND_JACK_HEADPHONE);
 
 	/* Microphone jack detection */
 	snd_soc_jack_new(codec, "Microphone",
-			 SND_JACK_MICROPHONE | SND_JACK_BTN_0, &mic_jack);
+	                 SND_JACK_MICROPHONE | SND_JACK_BTN_0, 0, &mic_jack);
 	snd_soc_jack_add_pins(&mic_jack, ARRAY_SIZE(mic_jack_pins),
 			      mic_jack_pins);
 	wm8350_mic_jack_detect(codec, &mic_jack, SND_JACK_MICROPHONE,
diff --git a/sound/soc/mid-x86/mfld_machine.c b/sound/soc/mid-x86/mfld_machine.c
index ee36384..e2c7978 100644
--- a/sound/soc/mid-x86/mfld_machine.c
+++ b/sound/soc/mid-x86/mfld_machine.c
@@ -253,9 +253,9 @@ static int mfld_init(struct snd_soc_pcm_runtime *runtime)
 	snd_soc_dapm_disable_pin(dapm, "LINEINR");
 
 	/* Headset and button jack detection */
-	ret_val = snd_soc_jack_new(codec, "Intel(R) MID Audio Jack",
-			SND_JACK_HEADSET | SND_JACK_BTN_0 |
-			SND_JACK_BTN_1, &mfld_jack);
+	ret_val = snd_soc_jack_new(codec, "Intel(R) MID Audio",
+	                           SND_JACK_HEADSET | SND_JACK_BTN_0 |
+	                           SND_JACK_BTN_1, 0, &mfld_jack);
 	if (ret_val) {
 		pr_err("jack creation failed\n");
 		return ret_val;
diff --git a/sound/soc/omap/ams-delta.c b/sound/soc/omap/ams-delta.c
index 6294464..4ffa38e 100644
--- a/sound/soc/omap/ams-delta.c
+++ b/sound/soc/omap/ams-delta.c
@@ -491,7 +491,7 @@ static int ams_delta_cx20442_init(struct snd_soc_pcm_runtime *rtd)
 	/* Add hook switch - can be used to control the codec from userspace
 	 * even if line discipline fails */
 	ret = snd_soc_jack_new(rtd->codec, "hook_switch",
-				SND_JACK_HEADSET, &ams_delta_hook_switch);
+	                       SND_JACK_HEADSET, 0, &ams_delta_hook_switch);
 	if (ret)
 		dev_warn(card->dev,
 				"Failed to allocate resources for hook switch, "
diff --git a/sound/soc/omap/omap-abe-twl6040.c b/sound/soc/omap/omap-abe-twl6040.c
index 70cd5c7..45ff3bc 100644
--- a/sound/soc/omap/omap-abe-twl6040.c
+++ b/sound/soc/omap/omap-abe-twl6040.c
@@ -193,8 +193,8 @@ static int omap_abe_twl6040_init(struct snd_soc_pcm_runtime *rtd)
 
 	/* Headset jack detection only if it is supported */
 	if (priv->jack_detection) {
-		ret = snd_soc_jack_new(codec, "Headset Jack",
-					SND_JACK_HEADSET, &hs_jack);
+		ret = snd_soc_jack_new(codec, "Headset",
+		                       SND_JACK_HEADSET, 0, &hs_jack);
 		if (ret)
 			return ret;
 
diff --git a/sound/soc/omap/omap-twl4030.c b/sound/soc/omap/omap-twl4030.c
index 2a9324f..0619ecb 100644
--- a/sound/soc/omap/omap-twl4030.c
+++ b/sound/soc/omap/omap-twl4030.c
@@ -189,8 +189,8 @@ static int omap_twl4030_init(struct snd_soc_pcm_runtime *rtd)
 	if (priv->jack_detect > 0) {
 		hs_jack_gpios[0].gpio = priv->jack_detect;
 
-		ret = snd_soc_jack_new(codec, "Headset Jack", SND_JACK_HEADSET,
-				       &priv->hs_jack);
+		ret = snd_soc_jack_new(codec, "Headset", SND_JACK_HEADSET,
+		                       0, &priv->hs_jack);
 		if (ret)
 			return ret;
 
diff --git a/sound/soc/omap/rx51.c b/sound/soc/omap/rx51.c
index 611179c..46fe697 100644
--- a/sound/soc/omap/rx51.c
+++ b/sound/soc/omap/rx51.c
@@ -317,9 +317,9 @@ static int rx51_aic34_init(struct snd_soc_pcm_runtime *rtd)
 		return err;
 
 	/* AV jack detection */
-	err = snd_soc_jack_new(codec, "AV Jack",
-			       SND_JACK_HEADSET | SND_JACK_VIDEOOUT,
-			       &rx51_av_jack);
+	err = snd_soc_jack_new(codec, "AV",
+	                       SND_JACK_HEADSET | SND_JACK_VIDEOOUT,
+	                       0, &rx51_av_jack);
 	if (err)
 		return err;
 	err = snd_soc_jack_add_gpios(&rx51_av_jack,
diff --git a/sound/soc/pxa/hx4700.c b/sound/soc/pxa/hx4700.c
index dcc9b04..3ffe48b 100644
--- a/sound/soc/pxa/hx4700.c
+++ b/sound/soc/pxa/hx4700.c
@@ -137,8 +137,8 @@ static int hx4700_ak4641_init(struct snd_soc_pcm_runtime *rtd)
 	snd_soc_dapm_nc_pin(dapm, "AUX");
 
 	/* Jack detection API stuff */
-	err = snd_soc_jack_new(codec, "Headphone Jack",
-				SND_JACK_HEADPHONE, &hs_jack);
+	err = snd_soc_jack_new(codec, "Headphone",
+	                       SND_JACK_HEADPHONE, 0, &hs_jack);
 	if (err)
 		return err;
 
diff --git a/sound/soc/pxa/palm27x.c b/sound/soc/pxa/palm27x.c
index e1ffcdd..7caba46 100644
--- a/sound/soc/pxa/palm27x.c
+++ b/sound/soc/pxa/palm27x.c
@@ -97,8 +97,8 @@ static int palm27x_ac97_init(struct snd_soc_pcm_runtime *rtd)
 	snd_soc_dapm_nc_pin(dapm, "MIC2");
 
 	/* Jack detection API stuff */
-	err = snd_soc_jack_new(codec, "Headphone Jack",
-				SND_JACK_HEADPHONE, &hs_jack);
+	err = snd_soc_jack_new(codec, "Headphone",
+	                       SND_JACK_HEADPHONE, 0, &hs_jack);
 	if (err)
 		return err;
 
diff --git a/sound/soc/pxa/ttc-dkb.c b/sound/soc/pxa/ttc-dkb.c
index f4ea4f6..1220a51 100644
--- a/sound/soc/pxa/ttc-dkb.c
+++ b/sound/soc/pxa/ttc-dkb.c
@@ -86,13 +86,13 @@ static int ttc_pm860x_init(struct snd_soc_pcm_runtime *rtd)
 	snd_soc_dapm_disable_pin(dapm, "Headset Stereophone");
 
 	/* Headset jack detection */
-	snd_soc_jack_new(codec, "Headphone Jack", SND_JACK_HEADPHONE
+	snd_soc_jack_new(codec, "Headphone", SND_JACK_HEADPHONE
 			| SND_JACK_BTN_0 | SND_JACK_BTN_1 | SND_JACK_BTN_2,
-			&hs_jack);
+	                 0, &hs_jack);
 	snd_soc_jack_add_pins(&hs_jack, ARRAY_SIZE(hs_jack_pins),
 			      hs_jack_pins);
-	snd_soc_jack_new(codec, "Microphone Jack", SND_JACK_MICROPHONE,
-			 &mic_jack);
+	snd_soc_jack_new(codec, "Microphone", SND_JACK_MICROPHONE,
+	                 0, &mic_jack);
 	snd_soc_jack_add_pins(&mic_jack, ARRAY_SIZE(mic_jack_pins),
 			      mic_jack_pins);
 
diff --git a/sound/soc/pxa/z2.c b/sound/soc/pxa/z2.c
index 76ccb17..2bbb2de 100644
--- a/sound/soc/pxa/z2.c
+++ b/sound/soc/pxa/z2.c
@@ -143,8 +143,8 @@ static int z2_wm8750_init(struct snd_soc_pcm_runtime *rtd)
 	snd_soc_dapm_disable_pin(dapm, "MONO1");
 
 	/* Jack detection API stuff */
-	ret = snd_soc_jack_new(codec, "Headset Jack", SND_JACK_HEADSET,
-				&hs_jack);
+	ret = snd_soc_jack_new(codec, "Headset", SND_JACK_HEADSET,
+	                       0, &hs_jack);
 	if (ret)
 		goto err;
 
diff --git a/sound/soc/samsung/goni_wm8994.c b/sound/soc/samsung/goni_wm8994.c
index 415ad81..d76072d 100644
--- a/sound/soc/samsung/goni_wm8994.c
+++ b/sound/soc/samsung/goni_wm8994.c
@@ -115,8 +115,8 @@ static int goni_wm8994_init(struct snd_soc_pcm_runtime *rtd)
 
 	/* Headset jack detection */
 	ret = snd_soc_jack_new(codec, "Headset Jack",
-			SND_JACK_HEADSET | SND_JACK_MECHANICAL | SND_JACK_AVOUT,
-			&jack);
+	                       SND_JACK_HEADSET | SND_JACK_MECHANICAL | SND_JACK_AVOUT,
+	                       0, &jack);
 	if (ret)
 		return ret;
 
diff --git a/sound/soc/samsung/h1940_uda1380.c b/sound/soc/samsung/h1940_uda1380.c
index fa91376..f8cf0cf 100644
--- a/sound/soc/samsung/h1940_uda1380.c
+++ b/sound/soc/samsung/h1940_uda1380.c
@@ -186,8 +186,8 @@ static int h1940_uda1380_init(struct snd_soc_pcm_runtime *rtd)
 	snd_soc_dapm_enable_pin(dapm, "Speaker");
 	snd_soc_dapm_enable_pin(dapm, "Mic Jack");
 
-	snd_soc_jack_new(codec, "Headphone Jack", SND_JACK_HEADPHONE,
-		&hp_jack);
+	snd_soc_jack_new(codec, "Headphone", SND_JACK_HEADPHONE,
+	                 0, &hp_jack);
 
 	snd_soc_jack_add_pins(&hp_jack, ARRAY_SIZE(hp_jack_pins),
 		hp_jack_pins);
diff --git a/sound/soc/samsung/littlemill.c b/sound/soc/samsung/littlemill.c
index bfb91f3..46344f3 100644
--- a/sound/soc/samsung/littlemill.c
+++ b/sound/soc/samsung/littlemill.c
@@ -261,11 +261,11 @@ static int littlemill_late_probe(struct snd_soc_card *card)
 		return ret;
 
 	ret = snd_soc_jack_new(codec, "Headset",
-			       SND_JACK_HEADSET | SND_JACK_MECHANICAL |
-			       SND_JACK_BTN_0 | SND_JACK_BTN_1 |
-			       SND_JACK_BTN_2 | SND_JACK_BTN_3 |
-			       SND_JACK_BTN_4 | SND_JACK_BTN_5,
-			       &littlemill_headset);
+	                       SND_JACK_HEADSET | SND_JACK_MECHANICAL |
+	                       SND_JACK_BTN_0 | SND_JACK_BTN_1 |
+	                       SND_JACK_BTN_2 | SND_JACK_BTN_3 |
+	                       SND_JACK_BTN_4 | SND_JACK_BTN_5,
+	                       0, &littlemill_headset);
 	if (ret)
 		return ret;
 
diff --git a/sound/soc/samsung/lowland.c b/sound/soc/samsung/lowland.c
index 570cf52..1e6a3df 100644
--- a/sound/soc/samsung/lowland.c
+++ b/sound/soc/samsung/lowland.c
@@ -57,9 +57,9 @@ static int lowland_wm5100_init(struct snd_soc_pcm_runtime *rtd)
 	}
 
 	ret = snd_soc_jack_new(codec, "Headset",
-			       SND_JACK_LINEOUT | SND_JACK_HEADSET |
-			       SND_JACK_BTN_0,
-			       &lowland_headset);
+	                       SND_JACK_LINEOUT | SND_JACK_HEADSET |
+	                       SND_JACK_BTN_0,
+	                       0, &lowland_headset);
 	if (ret)
 		return ret;
 
diff --git a/sound/soc/samsung/rx1950_uda1380.c b/sound/soc/samsung/rx1950_uda1380.c
index 704460a..f62b845 100644
--- a/sound/soc/samsung/rx1950_uda1380.c
+++ b/sound/soc/samsung/rx1950_uda1380.c
@@ -231,8 +231,8 @@ static int rx1950_uda1380_init(struct snd_soc_pcm_runtime *rtd)
 	snd_soc_dapm_enable_pin(dapm, "Speaker");
 	snd_soc_dapm_enable_pin(dapm, "Mic Jack");
 
-	snd_soc_jack_new(codec, "Headphone Jack", SND_JACK_HEADPHONE,
-		&hp_jack);
+	snd_soc_jack_new(codec, "Headphone", SND_JACK_HEADPHONE,
+	                 0, &hp_jack);
 
 	snd_soc_jack_add_pins(&hp_jack, ARRAY_SIZE(hp_jack_pins),
 		hp_jack_pins);
diff --git a/sound/soc/samsung/smartq_wm8987.c b/sound/soc/samsung/smartq_wm8987.c
index 58ae323..053ceea 100644
--- a/sound/soc/samsung/smartq_wm8987.c
+++ b/sound/soc/samsung/smartq_wm8987.c
@@ -166,8 +166,8 @@ static int smartq_wm8987_init(struct snd_soc_pcm_runtime *rtd)
 	snd_soc_dapm_disable_pin(dapm, "Headphone Jack");
 
 	/* Headphone jack detection */
-	err = snd_soc_jack_new(codec, "Headphone Jack",
-			       SND_JACK_HEADPHONE, &smartq_jack);
+	err = snd_soc_jack_new(codec, "Headphone",
+	                       SND_JACK_HEADPHONE, 0, &smartq_jack);
 	if (err)
 		return err;
 
diff --git a/sound/soc/samsung/speyside.c b/sound/soc/samsung/speyside.c
index 57df90d..feff3fb 100644
--- a/sound/soc/samsung/speyside.c
+++ b/sound/soc/samsung/speyside.c
@@ -154,9 +154,9 @@ static int speyside_wm8996_init(struct snd_soc_pcm_runtime *rtd)
 	gpio_direction_output(WM8996_HPSEL_GPIO, speyside_jack_polarity);
 
 	ret = snd_soc_jack_new(codec, "Headset",
-			       SND_JACK_LINEOUT | SND_JACK_HEADSET |
-			       SND_JACK_BTN_0,
-			       &speyside_headset);
+	                       SND_JACK_LINEOUT | SND_JACK_HEADSET |
+	                       SND_JACK_BTN_0,
+	                       0, &speyside_headset);
 	if (ret)
 		return ret;
 
diff --git a/sound/soc/samsung/tobermory.c b/sound/soc/samsung/tobermory.c
index f21ff60..e498e7c 100644
--- a/sound/soc/samsung/tobermory.c
+++ b/sound/soc/samsung/tobermory.c
@@ -178,8 +178,8 @@ static int tobermory_late_probe(struct snd_soc_card *card)
 		return ret;
 
 	ret = snd_soc_jack_new(codec, "Headset",
-			       SND_JACK_HEADSET | SND_JACK_BTN_0,
-			       &tobermory_headset);
+	                       SND_JACK_HEADSET | SND_JACK_BTN_0,
+	                       0, &tobermory_headset);
 	if (ret)
 		return ret;
 
diff --git a/sound/soc/soc-jack.c b/sound/soc/soc-jack.c
index 0bb5ccc..ecd1601 100644
--- a/sound/soc/soc-jack.c
+++ b/sound/soc/soc-jack.c
@@ -34,7 +34,7 @@
  * On success jack will be initialised.
  */
 int snd_soc_jack_new(struct snd_soc_codec *codec, const char *id, int type,
-		     struct snd_soc_jack *jack)
+                     int idx, struct snd_soc_jack *jack)
 {
 	mutex_init(&jack->mutex);
 	jack->codec = codec;
@@ -42,7 +42,7 @@ int snd_soc_jack_new(struct snd_soc_codec *codec, const char *id, int type,
 	INIT_LIST_HEAD(&jack->jack_zones);
 	BLOCKING_INIT_NOTIFIER_HEAD(&jack->notifier);
 
-	return snd_jack_new(codec->card->snd_card, id, type, &jack->jack);
+	return snd_jack_new(codec->card->snd_card, id, type, idx, &jack->jack);
 }
 EXPORT_SYMBOL_GPL(snd_soc_jack_new);
 
diff --git a/sound/soc/tegra/tegra_alc5632.c b/sound/soc/tegra/tegra_alc5632.c
index 48d05d9..cfacc27 100644
--- a/sound/soc/tegra/tegra_alc5632.c
+++ b/sound/soc/tegra/tegra_alc5632.c
@@ -109,8 +109,8 @@ static int tegra_alc5632_asoc_init(struct snd_soc_pcm_runtime *rtd)
 	struct snd_soc_dapm_context *dapm = &codec->dapm;
 	struct tegra_alc5632 *machine = snd_soc_card_get_drvdata(codec->card);
 
-	snd_soc_jack_new(codec, "Headset Jack", SND_JACK_HEADSET,
-			 &tegra_alc5632_hs_jack);
+	snd_soc_jack_new(codec, "Headset", SND_JACK_HEADSET,
+	                 0, &tegra_alc5632_hs_jack);
 	snd_soc_jack_add_pins(&tegra_alc5632_hs_jack,
 			ARRAY_SIZE(tegra_alc5632_hs_jack_pins),
 			tegra_alc5632_hs_jack_pins);
diff --git a/sound/soc/tegra/tegra_rt5640.c b/sound/soc/tegra/tegra_rt5640.c
index 08794f9..aad20f2 100644
--- a/sound/soc/tegra/tegra_rt5640.c
+++ b/sound/soc/tegra/tegra_rt5640.c
@@ -112,7 +112,7 @@ static int tegra_rt5640_asoc_init(struct snd_soc_pcm_runtime *rtd)
 	struct tegra_rt5640 *machine = snd_soc_card_get_drvdata(codec->card);
 
 	snd_soc_jack_new(codec, "Headphones", SND_JACK_HEADPHONE,
-			 &tegra_rt5640_hp_jack);
+	                 0, &tegra_rt5640_hp_jack);
 	snd_soc_jack_add_pins(&tegra_rt5640_hp_jack,
 			ARRAY_SIZE(tegra_rt5640_hp_jack_pins),
 			tegra_rt5640_hp_jack_pins);
diff --git a/sound/soc/tegra/tegra_wm8903.c b/sound/soc/tegra/tegra_wm8903.c
index 4ac7373..cd297f5 100644
--- a/sound/soc/tegra/tegra_wm8903.c
+++ b/sound/soc/tegra/tegra_wm8903.c
@@ -178,8 +178,8 @@ static int tegra_wm8903_init(struct snd_soc_pcm_runtime *rtd)
 
 	if (gpio_is_valid(machine->gpio_hp_det)) {
 		tegra_wm8903_hp_jack_gpio.gpio = machine->gpio_hp_det;
-		snd_soc_jack_new(codec, "Headphone Jack", SND_JACK_HEADPHONE,
-				&tegra_wm8903_hp_jack);
+		snd_soc_jack_new(codec, "Headphone", SND_JACK_HEADPHONE,
+		                 0, &tegra_wm8903_hp_jack);
 		snd_soc_jack_add_pins(&tegra_wm8903_hp_jack,
 					ARRAY_SIZE(tegra_wm8903_hp_jack_pins),
 					tegra_wm8903_hp_jack_pins);
@@ -188,8 +188,8 @@ static int tegra_wm8903_init(struct snd_soc_pcm_runtime *rtd)
 					&tegra_wm8903_hp_jack_gpio);
 	}
 
-	snd_soc_jack_new(codec, "Mic Jack", SND_JACK_MICROPHONE,
-			 &tegra_wm8903_mic_jack);
+	snd_soc_jack_new(codec, "Mic", SND_JACK_MICROPHONE,
+	                 0, &tegra_wm8903_mic_jack);
 	snd_soc_jack_add_pins(&tegra_wm8903_mic_jack,
 			      ARRAY_SIZE(tegra_wm8903_mic_jack_pins),
 			      tegra_wm8903_mic_jack_pins);
-- 
1.8.1.4

Re: [PATCH 4/4] ALSA: oxygen: Updating jack implementation according new ALSA Jacks

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1201 bytes --]

On Fri, Jul 26, 2013 at 12:02:51PM -0700, Felipe Tonello wrote:
> On Fri, Jul 26, 2013 at 11:56 AM, Mark Brown <broonie@kernel.org> wrote:

> >>       snd_jack_new(chip->card, "Headphone",
> >> -                  SND_JACK_HEADPHONE, &data->hp_jack);
> >> +                  SND_JACK_HEADPHONE, 0, &data->hp_jack);
> >>       xonar_ds_handle_hp_jack(chip);

> > ...this really ought to be done as part of the commit that adds the
> > parameter since it breaks the build until this patch is applied.

> But that's why is a patch series. But as you say, are you suggesting
> me to propose this changes in one patch only?

This one should be squashed in, as should the part of the ASoC change
that adjusts for the call into the core API.  The general idea with a
patch series is to split things into smaller chunks so they're easier to
understand and review but still keep things working with each change so
that things like git bisect continue to be usable.

So I guess something like one patch that changes the core jack API to
add the index and support jack creation, one to remove the HDA custom
implementation and one to add support for specifying the index to the
ASoC API and adjust all its users.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 4/4] ALSA: oxygen: Updating jack implementation according new ALSA Jacks

[Mark Brown]

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

On Fri, Jul 26, 2013 at 12:02:51PM -0700, Felipe Tonello wrote:
> On Fri, Jul 26, 2013 at 11:56 AM, Mark Brown <broonie@kernel.org> wrote:

> >>       snd_jack_new(chip->card, "Headphone",
> >> -                  SND_JACK_HEADPHONE, &data->hp_jack);
> >> +                  SND_JACK_HEADPHONE, 0, &data->hp_jack);
> >>       xonar_ds_handle_hp_jack(chip);

> > ...this really ought to be done as part of the commit that adds the
> > parameter since it breaks the build until this patch is applied.

> But that's why is a patch series. But as you say, are you suggesting
> me to propose this changes in one patch only?

This one should be squashed in, as should the part of the ASoC change
that adjusts for the call into the core API.  The general idea with a
patch series is to split things into smaller chunks so they're easier to
understand and review but still keep things working with each change so
that things like git bisect continue to be usable.

So I guess something like one patch that changes the core jack API to
add the index and support jack creation, one to remove the HDA custom
implementation and one to add support for specifying the index to the
ASoC API and adjust all its users.

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

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

Re: [PATCH 1/4] ALSA: Added jack detection kcontrol support

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1396 bytes --]

On Fri, Jul 26, 2013 at 12:10:27PM -0700, Felipe Tonello wrote:
> On Fri, Jul 26, 2013 at 11:54 AM, Mark Brown <broonie@kernel.org> wrote:

> > This isn't ideal for multi-function jacks like headsets - it will report
> > a single boolean value for the jack regardless of what's plugged in
> > meaning userpace can't do things like figure out if a headset or
> > headphone is present.  It's probably OK for any realistic input button
> > since you're not going to get an input button without other things being
> > present.

> The KControl for Jack is boolean anyway. You can check it with "amixer
> contents". user-space can figure out based on the control name. At
> least PulseAudio does that way.

No, it can't do that for headset jacks - these will be created with a
single jack reporting multiple states, there's a state for headphone and
a state for microphone.  The system can generally distinguish between
having a headset or just plain headphones inserted and act accordingly
(for example, recording from the built in microphone on a phone when
used with normal headpones).

> > What I'd expect to happen here is that for multi function jacks we
> > create a control per function if the controls are valid.

> Do you mean based on snd_jack_types?

Yes.  If there's only one function supported the current code is fine
but for multiple functions it's going to discard useful information.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 4/4] ALSA: oxygen: Updating jack implementation according new ALSA Jacks

[Felipe Tonello]

Mark,

On Fri, Jul 26, 2013 at 3:45 PM, Mark Brown <broonie@kernel.org> wrote:
> On Fri, Jul 26, 2013 at 12:02:51PM -0700, Felipe Tonello wrote:
>> On Fri, Jul 26, 2013 at 11:56 AM, Mark Brown <broonie@kernel.org> wrote:
>
>> >>       snd_jack_new(chip->card, "Headphone",
>> >> -                  SND_JACK_HEADPHONE, &data->hp_jack);
>> >> +                  SND_JACK_HEADPHONE, 0, &data->hp_jack);
>> >>       xonar_ds_handle_hp_jack(chip);
>
>> > ...this really ought to be done as part of the commit that adds the
>> > parameter since it breaks the build until this patch is applied.
>
>> But that's why is a patch series. But as you say, are you suggesting
>> me to propose this changes in one patch only?
>
> This one should be squashed in, as should the part of the ASoC change
> that adjusts for the call into the core API.  The general idea with a
> patch series is to split things into smaller chunks so they're easier to
> understand and review but still keep things working with each change so
> that things like git bisect continue to be usable.

Sure.

>
> So I guess something like one patch that changes the core jack API to
> add the index and support jack creation, one to remove the HDA custom
> implementation and one to add support for specifying the index to the
> ASoC API and adjust all its users.

I squashed into the HDA one. Perhaps I should send it again?

I don't know why, but my gmail is messing with the thread. If you
think it's better I can send a v3 with no --in-reply-to option.

Felipe Tonello

Re: [PATCH 1/4] ALSA: Added jack detection kcontrol support

[Felipe Tonello]

Mark,

On Fri, Jul 26, 2013 at 3:48 PM, Mark Brown <broonie@kernel.org> wrote:
> On Fri, Jul 26, 2013 at 12:10:27PM -0700, Felipe Tonello wrote:
>> On Fri, Jul 26, 2013 at 11:54 AM, Mark Brown <broonie@kernel.org> wrote:
>
>> > This isn't ideal for multi-function jacks like headsets - it will report
>> > a single boolean value for the jack regardless of what's plugged in
>> > meaning userpace can't do things like figure out if a headset or
>> > headphone is present.  It's probably OK for any realistic input button
>> > since you're not going to get an input button without other things being
>> > present.
>
>> The KControl for Jack is boolean anyway. You can check it with "amixer
>> contents". user-space can figure out based on the control name. At
>> least PulseAudio does that way.
>
> No, it can't do that for headset jacks - these will be created with a
> single jack reporting multiple states, there's a state for headphone and
> a state for microphone.  The system can generally distinguish between
> having a headset or just plain headphones inserted and act accordingly
> (for example, recording from the built in microphone on a phone when
> used with normal headpones).
>
>> > What I'd expect to happen here is that for multi function jacks we
>> > create a control per function if the controls are valid.

Ok, so the idea is just to change the control to type integer instead
of boolean, right?
Because as you say, the user will be able to check the type of jack
based on the status value, right?

>
>> Do you mean based on snd_jack_types?
>
> Yes.  If there's only one function supported the current code is fine
> but for multiple functions it's going to discard useful information.

So, what do you suggest to do that? I'm not sure if I understand what
you are saying.
When you mean function, do you mean the SND_JACK_BTN_n or the the jack
types, such as SND_JACK_HEADPHONE, and so on?

If a codec creates a jack type SND_JACK_HEADSET (= SND_JACK_HEADPHONE
| SND_JACK_MICROPHONE). It should be created two controls, name +
"Headphone Jack" and name + "Microphone Jack"? If so, what about the
status to report? How to know which control to report?

Felipe Tonello

Re: [PATCH 1/4] ALSA: Added jack detection kcontrol support

[Felipe Tonello]

Mark,

On Fri, Jul 26, 2013 at 3:48 PM, Mark Brown <broonie@kernel.org> wrote:
> On Fri, Jul 26, 2013 at 12:10:27PM -0700, Felipe Tonello wrote:
>> On Fri, Jul 26, 2013 at 11:54 AM, Mark Brown <broonie@kernel.org> wrote:
>
>> > This isn't ideal for multi-function jacks like headsets - it will report
>> > a single boolean value for the jack regardless of what's plugged in
>> > meaning userpace can't do things like figure out if a headset or
>> > headphone is present.  It's probably OK for any realistic input button
>> > since you're not going to get an input button without other things being
>> > present.
>
>> The KControl for Jack is boolean anyway. You can check it with "amixer
>> contents". user-space can figure out based on the control name. At
>> least PulseAudio does that way.
>
> No, it can't do that for headset jacks - these will be created with a
> single jack reporting multiple states, there's a state for headphone and
> a state for microphone.  The system can generally distinguish between
> having a headset or just plain headphones inserted and act accordingly
> (for example, recording from the built in microphone on a phone when
> used with normal headpones).
>
>> > What I'd expect to happen here is that for multi function jacks we
>> > create a control per function if the controls are valid.

Ok, so the idea is just to change the control to type integer instead
of boolean, right?
Because as you say, the user will be able to check the type of jack
based on the status value, right?

>
>> Do you mean based on snd_jack_types?
>
> Yes.  If there's only one function supported the current code is fine
> but for multiple functions it's going to discard useful information.

So, what do you suggest to do that? I'm not sure if I understand what
you are saying.
When you mean function, do you mean the SND_JACK_BTN_n or the the jack
types, such as SND_JACK_HEADPHONE, and so on?

If a codec creates a jack type SND_JACK_HEADSET (= SND_JACK_HEADPHONE
| SND_JACK_MICROPHONE). It should be created two controls, name +
"Headphone Jack" and name + "Microphone Jack"? If so, what about the
status to report? How to know which control to report?

Felipe Tonello

Re: [PATCH 1/4] ALSA: Added jack detection kcontrol support

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1416 bytes --]

On Fri, Jul 26, 2013 at 04:13:40PM -0700, Felipe Tonello wrote:
> On Fri, Jul 26, 2013 at 3:48 PM, Mark Brown <broonie@kernel.org> wrote:

> >> > What I'd expect to happen here is that for multi function jacks we
> >> > create a control per function if the controls are valid.

> Ok, so the idea is just to change the control to type integer instead
> of boolean, right?
> Because as you say, the user will be able to check the type of jack
> based on the status value, right?

It might be more idiomatic and more compatible with userspace to create
multiple controls for the jack, there was some discussion of this in the
past I think but I can't remember the result.

> > Yes.  If there's only one function supported the current code is fine
> > but for multiple functions it's going to discard useful information.

> So, what do you suggest to do that? I'm not sure if I understand what
> you are saying.
> When you mean function, do you mean the SND_JACK_BTN_n or the the jack
> types, such as SND_JACK_HEADPHONE, and so on?

The jack types, the buttons definitely sohuld be going up as input
events.

> If a codec creates a jack type SND_JACK_HEADSET (= SND_JACK_HEADPHONE
> | SND_JACK_MICROPHONE). It should be created two controls, name +
> "Headphone Jack" and name + "Microphone Jack"? If so, what about the
> status to report? How to know which control to report?

The drivers report a bitmask for status.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/4] ALSA: Added jack detection kcontrol support

[Mark Brown]

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

On Fri, Jul 26, 2013 at 04:13:40PM -0700, Felipe Tonello wrote:
> On Fri, Jul 26, 2013 at 3:48 PM, Mark Brown <broonie@kernel.org> wrote:

> >> > What I'd expect to happen here is that for multi function jacks we
> >> > create a control per function if the controls are valid.

> Ok, so the idea is just to change the control to type integer instead
> of boolean, right?
> Because as you say, the user will be able to check the type of jack
> based on the status value, right?

It might be more idiomatic and more compatible with userspace to create
multiple controls for the jack, there was some discussion of this in the
past I think but I can't remember the result.

> > Yes.  If there's only one function supported the current code is fine
> > but for multiple functions it's going to discard useful information.

> So, what do you suggest to do that? I'm not sure if I understand what
> you are saying.
> When you mean function, do you mean the SND_JACK_BTN_n or the the jack
> types, such as SND_JACK_HEADPHONE, and so on?

The jack types, the buttons definitely sohuld be going up as input
events.

> If a codec creates a jack type SND_JACK_HEADSET (= SND_JACK_HEADPHONE
> | SND_JACK_MICROPHONE). It should be created two controls, name +
> "Headphone Jack" and name + "Microphone Jack"? If so, what about the
> status to report? How to know which control to report?

The drivers report a bitmask for status.

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

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

Re: [PATCH 1/4] ALSA: Added jack detection kcontrol support

[Takashi Iwai]

At Sat, 27 Jul 2013 13:25:24 +0100,
Mark Brown wrote:
> 
> On Fri, Jul 26, 2013 at 04:13:40PM -0700, Felipe Tonello wrote:
> > On Fri, Jul 26, 2013 at 3:48 PM, Mark Brown <broonie@kernel.org> wrote:
> 
> > >> > What I'd expect to happen here is that for multi function jacks we
> > >> > create a control per function if the controls are valid.
> 
> > Ok, so the idea is just to change the control to type integer instead
> > of boolean, right?
> > Because as you say, the user will be able to check the type of jack
> > based on the status value, right?
> 
> It might be more idiomatic and more compatible with userspace to create
> multiple controls for the jack, there was some discussion of this in the
> past I think but I can't remember the result.

I also forget the end result :)

Another option would be to make an enum control, but multiple boolean
controls are easier in the end.  The headset controls of HD-audio are
represented in that way, too (because HD-audio specification can't
tell better).


Takashi

Re: [PATCH 4/4] ALSA: oxygen: Updating jack implementation according new ALSA Jacks

[Takashi Iwai]

At Fri, 26 Jul 2013 12:02:51 -0700,
Felipe Tonello wrote:
> 
> Hi Mark,
> 
> On Fri, Jul 26, 2013 at 11:56 AM, Mark Brown <broonie@kernel.org> wrote:
> > On Fri, Jul 26, 2013 at 11:25:33AM -0700, Felipe F. Tonello wrote:
> >> From: "Felipe F. Tonello" <eu@felipetonello.com>
> >>
> >> ALSA standard jacks already are implemented using ALSA KControl.
> >> So there is no need implement that itself or to use snd_jack
> >> for input events only.
> >
> > Similar changlog comment as on the ASoC patch and...
> 
> Ok.
> 
> >
> >>       snd_jack_new(chip->card, "Headphone",
> >> -                  SND_JACK_HEADPHONE, &data->hp_jack);
> >> +                  SND_JACK_HEADPHONE, 0, &data->hp_jack);
> >>       xonar_ds_handle_hp_jack(chip);
> >
> > ...this really ought to be done as part of the commit that adds the
> > parameter since it breaks the build until this patch is applied.
> 
> But that's why is a patch series. But as you say, are you suggesting
> me to propose this changes in one patch only?

The basic rule of the patch series is:
- they are split in a logical manner
- each commit must not break the build

The second rule is important especially for bisection and
backporting.  You could have done in a different way (e.g. adding a
new function like snd_jack_new_with_index() instead of changing
snd_jack_new() itself).  Which way is better, depends on the
implementation details.


Takashi

Re: [PATCH v2 1/3] ALSA: Added jack detection kcontrol support

[Takashi Iwai]

At Fri, 26 Jul 2013 15:45:11 -0700,
Felipe F. Tonello wrote:
> 
> From: "Felipe F. Tonello" <eu@felipetonello.com>
> 
> This patch adds jack support for alsa kcontrol.
> 
> This support is necessary since the new kcontrol is used by user-space
> daemons, such as PulseAudio(>=2.0), to do jack detection.)
> 
> Signed-off-by: Felipe F. Tonello <eu@felipetonello.com>

This patch breaks the build.


Takashi

> ---
>  include/sound/jack.h |  6 ++++--
>  sound/core/Kconfig   |  1 +
>  sound/core/ctljack.c |  3 ++-
>  sound/core/jack.c    | 29 +++++++++++++++++++++++++++--
>  4 files changed, 34 insertions(+), 5 deletions(-)
> 
> diff --git a/include/sound/jack.h b/include/sound/jack.h
> index 5891657..dc62b74 100644
> --- a/include/sound/jack.h
> +++ b/include/sound/jack.h
> @@ -26,6 +26,7 @@
>  #include <sound/core.h>
>  
>  struct input_dev;
> +struct snd_kcontrol;
>  
>  /**
>   * Jack types which can be reported.  These values are used as a
> @@ -58,6 +59,7 @@ enum snd_jack_types {
>  
>  struct snd_jack {
>  	struct input_dev *input_dev;
> +	struct snd_kcontrol *kctl;
>  	int registered;
>  	int type;
>  	const char *id;
> @@ -70,7 +72,7 @@ struct snd_jack {
>  #ifdef CONFIG_SND_JACK
>  
>  int snd_jack_new(struct snd_card *card, const char *id, int type,
> -		 struct snd_jack **jack);
> +                 int idx, struct snd_jack **jack);
>  void snd_jack_set_parent(struct snd_jack *jack, struct device *parent);
>  int snd_jack_set_key(struct snd_jack *jack, enum snd_jack_types type,
>  		     int keytype);
> @@ -80,7 +82,7 @@ void snd_jack_report(struct snd_jack *jack, int status);
>  #else
>  
>  static inline int snd_jack_new(struct snd_card *card, const char *id, int type,
> -			       struct snd_jack **jack)
> +                               int idx, struct snd_jack **jack)
>  {
>  	return 0;
>  }
> diff --git a/sound/core/Kconfig b/sound/core/Kconfig
> index c0c2f57..8167615 100644
> --- a/sound/core/Kconfig
> +++ b/sound/core/Kconfig
> @@ -20,6 +20,7 @@ config SND_COMPRESS_OFFLOAD
>  # to avoid having to force INPUT on.
>  config SND_JACK
>  	bool
> +	select SND_KCTL_JACK
>  
>  config SND_SEQUENCER
>  	tristate "Sequencer support"
> diff --git a/sound/core/ctljack.c b/sound/core/ctljack.c
> index e4b38fb..59aa6d0 100644
> --- a/sound/core/ctljack.c
> +++ b/sound/core/ctljack.c
> @@ -38,7 +38,8 @@ snd_kctl_jack_new(const char *name, int idx, void *private_data)
>  	kctl = snd_ctl_new1(&jack_detect_kctl, private_data);
>  	if (!kctl)
>  		return NULL;
> -	snprintf(kctl->id.name, sizeof(kctl->id.name), "%s Jack", name);
> +
> +	strlcpy(kctl->id.name, name, sizeof(kctl->id.name));
>  	kctl->id.index = idx;
>  	kctl->private_value = 0;
>  	return kctl;
> diff --git a/sound/core/jack.c b/sound/core/jack.c
> index b35fe73..b2757b1 100644
> --- a/sound/core/jack.c
> +++ b/sound/core/jack.c
> @@ -24,6 +24,7 @@
>  #include <linux/module.h>
>  #include <sound/jack.h>
>  #include <sound/core.h>
> +#include <sound/control.h>
>  
>  static int jack_switch_types[SND_JACK_SWITCH_TYPES] = {
>  	SW_HEADPHONE_INSERT,
> @@ -48,6 +49,7 @@ static int snd_jack_dev_free(struct snd_device *device)
>  	else
>  		input_free_device(jack->input_dev);
>  
> +	snd_ctl_remove(device->card, jack->kctl);
>  	kfree(jack->id);
>  	kfree(jack);
>  
> @@ -85,26 +87,36 @@ static int snd_jack_dev_register(struct snd_device *device)
>  	if (err == 0)
>  		jack->registered = 1;
>  
> +	/* We don't need to free the control, it's freed by snd_ctl_add itself
> +	   if an error occur */
> +	err = snd_ctl_add(card, jack->kctl);
> +
>  	return err;
>  }
>  
>  /**
>   * snd_jack_new - Create a new jack
>   * @card:  the card instance
> - * @id:    an identifying string for this jack
> + * @id:    an identifying string for this jack, " Jack" is appended to the
> + *         string
>   * @type:  a bitmask of enum snd_jack_type values that can be detected by
>   *         this jack
> + * @idx:   index of this control item
>   * @jjack: Used to provide the allocated jack object to the caller.
>   *
>   * Creates a new jack object.
>   *
> + * This function creates a Jack Kcontrol, which is exported to user space via
> + * ALSA Controls.
> + *
>   * Return: Zero if successful, or a negative error code on failure.
>   * On success @jjack will be initialised.
>   */
>  int snd_jack_new(struct snd_card *card, const char *id, int type,
> -		 struct snd_jack **jjack)
> +                 int idx, struct snd_jack **jjack)
>  {
>  	struct snd_jack *jack;
> +	struct snd_kcontrol *kctl;
>  	int err;
>  	int i;
>  	static struct snd_device_ops ops = {
> @@ -117,6 +129,7 @@ int snd_jack_new(struct snd_card *card, const char *id, int type,
>  		return -ENOMEM;
>  
>  	jack->id = kstrdup(id, GFP_KERNEL);
> +	sprintf((char *)jack->id, "%s Jack", jack->id);
>  
>  	jack->input_dev = input_allocate_device();
>  	if (jack->input_dev == NULL) {
> @@ -137,6 +150,15 @@ int snd_jack_new(struct snd_card *card, const char *id, int type,
>  	if (err < 0)
>  		goto fail_input;
>  
> +	/* card is the private_data */
> +	kctl = snd_kctl_jack_new(jack->id, idx, card);
> +	if (!kctl) {
> +		err = -ENOMEM;
> +		goto fail_input;
> +	}
> +
> +	jack->kctl = kctl;
> +
>  	*jjack = jack;
>  
>  	return 0;
> @@ -239,6 +261,9 @@ void snd_jack_report(struct snd_jack *jack, int status)
>  	}
>  
>  	input_sync(jack->input_dev);
> +
> +	/* Update ALSA KControl interface */
> +	snd_kctl_jack_report((struct snd_card *)jack->kctl->private_data, jack->kctl, !!status);
>  }
>  EXPORT_SYMBOL(snd_jack_report);
>  
> -- 
> 1.8.1.4
> 

Re: [PATCH v2 1/3] ALSA: Added jack detection kcontrol support

[Takashi Iwai]

At Fri, 26 Jul 2013 15:45:11 -0700,
Felipe F. Tonello wrote:
> 
> From: "Felipe F. Tonello" <eu@felipetonello.com>
> 
> This patch adds jack support for alsa kcontrol.
> 
> This support is necessary since the new kcontrol is used by user-space
> daemons, such as PulseAudio(>=2.0), to do jack detection.)
> 
> Signed-off-by: Felipe F. Tonello <eu@felipetonello.com>

This patch breaks the build.


Takashi

> ---
>  include/sound/jack.h |  6 ++++--
>  sound/core/Kconfig   |  1 +
>  sound/core/ctljack.c |  3 ++-
>  sound/core/jack.c    | 29 +++++++++++++++++++++++++++--
>  4 files changed, 34 insertions(+), 5 deletions(-)
> 
> diff --git a/include/sound/jack.h b/include/sound/jack.h
> index 5891657..dc62b74 100644
> --- a/include/sound/jack.h
> +++ b/include/sound/jack.h
> @@ -26,6 +26,7 @@
>  #include <sound/core.h>
>  
>  struct input_dev;
> +struct snd_kcontrol;
>  
>  /**
>   * Jack types which can be reported.  These values are used as a
> @@ -58,6 +59,7 @@ enum snd_jack_types {
>  
>  struct snd_jack {
>  	struct input_dev *input_dev;
> +	struct snd_kcontrol *kctl;
>  	int registered;
>  	int type;
>  	const char *id;
> @@ -70,7 +72,7 @@ struct snd_jack {
>  #ifdef CONFIG_SND_JACK
>  
>  int snd_jack_new(struct snd_card *card, const char *id, int type,
> -		 struct snd_jack **jack);
> +                 int idx, struct snd_jack **jack);
>  void snd_jack_set_parent(struct snd_jack *jack, struct device *parent);
>  int snd_jack_set_key(struct snd_jack *jack, enum snd_jack_types type,
>  		     int keytype);
> @@ -80,7 +82,7 @@ void snd_jack_report(struct snd_jack *jack, int status);
>  #else
>  
>  static inline int snd_jack_new(struct snd_card *card, const char *id, int type,
> -			       struct snd_jack **jack)
> +                               int idx, struct snd_jack **jack)
>  {
>  	return 0;
>  }
> diff --git a/sound/core/Kconfig b/sound/core/Kconfig
> index c0c2f57..8167615 100644
> --- a/sound/core/Kconfig
> +++ b/sound/core/Kconfig
> @@ -20,6 +20,7 @@ config SND_COMPRESS_OFFLOAD
>  # to avoid having to force INPUT on.
>  config SND_JACK
>  	bool
> +	select SND_KCTL_JACK
>  
>  config SND_SEQUENCER
>  	tristate "Sequencer support"
> diff --git a/sound/core/ctljack.c b/sound/core/ctljack.c
> index e4b38fb..59aa6d0 100644
> --- a/sound/core/ctljack.c
> +++ b/sound/core/ctljack.c
> @@ -38,7 +38,8 @@ snd_kctl_jack_new(const char *name, int idx, void *private_data)
>  	kctl = snd_ctl_new1(&jack_detect_kctl, private_data);
>  	if (!kctl)
>  		return NULL;
> -	snprintf(kctl->id.name, sizeof(kctl->id.name), "%s Jack", name);
> +
> +	strlcpy(kctl->id.name, name, sizeof(kctl->id.name));
>  	kctl->id.index = idx;
>  	kctl->private_value = 0;
>  	return kctl;
> diff --git a/sound/core/jack.c b/sound/core/jack.c
> index b35fe73..b2757b1 100644
> --- a/sound/core/jack.c
> +++ b/sound/core/jack.c
> @@ -24,6 +24,7 @@
>  #include <linux/module.h>
>  #include <sound/jack.h>
>  #include <sound/core.h>
> +#include <sound/control.h>
>  
>  static int jack_switch_types[SND_JACK_SWITCH_TYPES] = {
>  	SW_HEADPHONE_INSERT,
> @@ -48,6 +49,7 @@ static int snd_jack_dev_free(struct snd_device *device)
>  	else
>  		input_free_device(jack->input_dev);
>  
> +	snd_ctl_remove(device->card, jack->kctl);
>  	kfree(jack->id);
>  	kfree(jack);
>  
> @@ -85,26 +87,36 @@ static int snd_jack_dev_register(struct snd_device *device)
>  	if (err == 0)
>  		jack->registered = 1;
>  
> +	/* We don't need to free the control, it's freed by snd_ctl_add itself
> +	   if an error occur */
> +	err = snd_ctl_add(card, jack->kctl);
> +
>  	return err;
>  }
>  
>  /**
>   * snd_jack_new - Create a new jack
>   * @card:  the card instance
> - * @id:    an identifying string for this jack
> + * @id:    an identifying string for this jack, " Jack" is appended to the
> + *         string
>   * @type:  a bitmask of enum snd_jack_type values that can be detected by
>   *         this jack
> + * @idx:   index of this control item
>   * @jjack: Used to provide the allocated jack object to the caller.
>   *
>   * Creates a new jack object.
>   *
> + * This function creates a Jack Kcontrol, which is exported to user space via
> + * ALSA Controls.
> + *
>   * Return: Zero if successful, or a negative error code on failure.
>   * On success @jjack will be initialised.
>   */
>  int snd_jack_new(struct snd_card *card, const char *id, int type,
> -		 struct snd_jack **jjack)
> +                 int idx, struct snd_jack **jjack)
>  {
>  	struct snd_jack *jack;
> +	struct snd_kcontrol *kctl;
>  	int err;
>  	int i;
>  	static struct snd_device_ops ops = {
> @@ -117,6 +129,7 @@ int snd_jack_new(struct snd_card *card, const char *id, int type,
>  		return -ENOMEM;
>  
>  	jack->id = kstrdup(id, GFP_KERNEL);
> +	sprintf((char *)jack->id, "%s Jack", jack->id);
>  
>  	jack->input_dev = input_allocate_device();
>  	if (jack->input_dev == NULL) {
> @@ -137,6 +150,15 @@ int snd_jack_new(struct snd_card *card, const char *id, int type,
>  	if (err < 0)
>  		goto fail_input;
>  
> +	/* card is the private_data */
> +	kctl = snd_kctl_jack_new(jack->id, idx, card);
> +	if (!kctl) {
> +		err = -ENOMEM;
> +		goto fail_input;
> +	}
> +
> +	jack->kctl = kctl;
> +
>  	*jjack = jack;
>  
>  	return 0;
> @@ -239,6 +261,9 @@ void snd_jack_report(struct snd_jack *jack, int status)
>  	}
>  
>  	input_sync(jack->input_dev);
> +
> +	/* Update ALSA KControl interface */
> +	snd_kctl_jack_report((struct snd_card *)jack->kctl->private_data, jack->kctl, !!status);
>  }
>  EXPORT_SYMBOL(snd_jack_report);
>  
> -- 
> 1.8.1.4
> 

Re: [PATCH v2 3/3] ALSA: SoC: Updating jack implementation according new ALSA Jacks

[Stephen Warren]

On 07/26/2013 04:45 PM, Felipe F. Tonello wrote:
> Updating the ASoC jack support to add the ability to specify a
> jack index when creating it.

This also appears to rename a number of jacks. If that's intentional,
presumably it should be mentioned in the commit description?

Re: [PATCH 1/4] ALSA: Added jack detection kcontrol support

[Felipe Ferreri Tonello]

On 07/27/2013 05:25 AM, Mark Brown wrote:
> On Fri, Jul 26, 2013 at 04:13:40PM -0700, Felipe Tonello wrote:
>> On Fri, Jul 26, 2013 at 3:48 PM, Mark Brown <broonie@kernel.org> wrote:
>
>>>>> What I'd expect to happen here is that for multi function jacks we
>>>>> create a control per function if the controls are valid.
>
>> Ok, so the idea is just to change the control to type integer instead
>> of boolean, right?
>> Because as you say, the user will be able to check the type of jack
>> based on the status value, right?
>
> It might be more idiomatic and more compatible with userspace to create
> multiple controls for the jack, there was some discussion of this in the
> past I think but I can't remember the result.
>
>>> Yes.  If there's only one function supported the current code is fine
>>> but for multiple functions it's going to discard useful information.
>
>> So, what do you suggest to do that? I'm not sure if I understand what
>> you are saying.
>> When you mean function, do you mean the SND_JACK_BTN_n or the the jack
>> types, such as SND_JACK_HEADPHONE, and so on?
>
> The jack types, the buttons definitely sohuld be going up as input
> events.
>
>> If a codec creates a jack type SND_JACK_HEADSET (= SND_JACK_HEADPHONE
>> | SND_JACK_MICROPHONE). It should be created two controls, name +
>> "Headphone Jack" and name + "Microphone Jack"? If so, what about the
>> status to report? How to know which control to report?
>
> The drivers report a bitmask for status.
>

I did that but I'm not happy with the control name. Usually drivers add 
jacks like: "Headset" for a headset, "Headphone" for a headphone and so on.
I did the following: control name is jack name + (jack type) + Jack. If 
jack type == jack name, don't add (jack type) to the name.

Any suggestion how it should be?

Re: [PATCH 1/4] ALSA: Added jack detection kcontrol support

[Felipe Ferreri Tonello]

On 07/27/2013 05:25 AM, Mark Brown wrote:
> On Fri, Jul 26, 2013 at 04:13:40PM -0700, Felipe Tonello wrote:
>> On Fri, Jul 26, 2013 at 3:48 PM, Mark Brown <broonie@kernel.org> wrote:
>
>>>>> What I'd expect to happen here is that for multi function jacks we
>>>>> create a control per function if the controls are valid.
>
>> Ok, so the idea is just to change the control to type integer instead
>> of boolean, right?
>> Because as you say, the user will be able to check the type of jack
>> based on the status value, right?
>
> It might be more idiomatic and more compatible with userspace to create
> multiple controls for the jack, there was some discussion of this in the
> past I think but I can't remember the result.
>
>>> Yes.  If there's only one function supported the current code is fine
>>> but for multiple functions it's going to discard useful information.
>
>> So, what do you suggest to do that? I'm not sure if I understand what
>> you are saying.
>> When you mean function, do you mean the SND_JACK_BTN_n or the the jack
>> types, such as SND_JACK_HEADPHONE, and so on?
>
> The jack types, the buttons definitely sohuld be going up as input
> events.
>
>> If a codec creates a jack type SND_JACK_HEADSET (= SND_JACK_HEADPHONE
>> | SND_JACK_MICROPHONE). It should be created two controls, name +
>> "Headphone Jack" and name + "Microphone Jack"? If so, what about the
>> status to report? How to know which control to report?
>
> The drivers report a bitmask for status.
>

I did that but I'm not happy with the control name. Usually drivers add 
jacks like: "Headset" for a headset, "Headphone" for a headphone and so on.
I did the following: control name is jack name + (jack type) + Jack. If 
jack type == jack name, don't add (jack type) to the name.

Any suggestion how it should be?

Re: [PATCH 1/4] ALSA: Added jack detection kcontrol support

[Felipe Ferreri Tonello]

Hi Takashi,

On 07/29/2013 05:05 AM, Takashi Iwai wrote:
> At Sat, 27 Jul 2013 13:25:24 +0100,
> Mark Brown wrote:
>>
>> On Fri, Jul 26, 2013 at 04:13:40PM -0700, Felipe Tonello wrote:
>>> On Fri, Jul 26, 2013 at 3:48 PM, Mark Brown <broonie@kernel.org> wrote:
>>
>>>>>> What I'd expect to happen here is that for multi function jacks we
>>>>>> create a control per function if the controls are valid.
>>
>>> Ok, so the idea is just to change the control to type integer instead
>>> of boolean, right?
>>> Because as you say, the user will be able to check the type of jack
>>> based on the status value, right?
>>
>> It might be more idiomatic and more compatible with userspace to create
>> multiple controls for the jack, there was some discussion of this in the
>> past I think but I can't remember the result.
>
> I also forget the end result :)
>
> Another option would be to make an enum control, but multiple boolean
> controls are easier in the end.  The headset controls of HD-audio are
> represented in that way, too (because HD-audio specification can't
> tell better).

Yes. Thanks for the comment.

Felipe Tonello

Re: [PATCH 4/4] ALSA: oxygen: Updating jack implementation according new ALSA Jacks

[Felipe Ferreri Tonello]

Hi Takashi,

On 07/29/2013 05:10 AM, Takashi Iwai wrote:
> At Fri, 26 Jul 2013 12:02:51 -0700,
> Felipe Tonello wrote:
>>
>> Hi Mark,
>>
>> On Fri, Jul 26, 2013 at 11:56 AM, Mark Brown <broonie@kernel.org> wrote:
>>> On Fri, Jul 26, 2013 at 11:25:33AM -0700, Felipe F. Tonello wrote:
>>>> From: "Felipe F. Tonello" <eu@felipetonello.com>
>>>>
>>>> ALSA standard jacks already are implemented using ALSA KControl.
>>>> So there is no need implement that itself or to use snd_jack
>>>> for input events only.
>>>
>>> Similar changlog comment as on the ASoC patch and...
>>
>> Ok.
>>
>>>
>>>>        snd_jack_new(chip->card, "Headphone",
>>>> -                  SND_JACK_HEADPHONE, &data->hp_jack);
>>>> +                  SND_JACK_HEADPHONE, 0, &data->hp_jack);
>>>>        xonar_ds_handle_hp_jack(chip);
>>>
>>> ...this really ought to be done as part of the commit that adds the
>>> parameter since it breaks the build until this patch is applied.
>>
>> But that's why is a patch series. But as you say, are you suggesting
>> me to propose this changes in one patch only?
>
> The basic rule of the patch series is:
> - they are split in a logical manner
> - each commit must not break the build
>
> The second rule is important especially for bisection and
> backporting.  You could have done in a different way (e.g. adding a
> new function like snd_jack_new_with_index() instead of changing
> snd_jack_new() itself).  Which way is better, depends on the
> implementation details.

In this case I think this will not solve the issue. Because the my 
propose is to make the control creation standard for jacks.

Felipe Tonello

Re: [PATCH v2 1/3] ALSA: Added jack detection kcontrol support

[Felipe Ferreri Tonello]

On 07/29/2013 07:19 AM, Takashi Iwai wrote:
> At Fri, 26 Jul 2013 15:45:11 -0700,
> Felipe F. Tonello wrote:
>>
>> From: "Felipe F. Tonello" <eu@felipetonello.com>
>>
>> This patch adds jack support for alsa kcontrol.
>>
>> This support is necessary since the new kcontrol is used by user-space
>> daemons, such as PulseAudio(>=2.0), to do jack detection.)
>>
>> Signed-off-by: Felipe F. Tonello <eu@felipetonello.com>
>
> This patch breaks the build.

I'm planing to send a v3 in few days.

Felipe Tonello

Re: [PATCH 1/4] ALSA: Added jack detection kcontrol support

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 551 bytes --]

On Wed, Jul 31, 2013 at 08:48:27PM -0700, Felipe Ferreri Tonello wrote:

> I did that but I'm not happy with the control name. Usually drivers
> add jacks like: "Headset" for a headset, "Headphone" for a headphone
> and so on.
> I did the following: control name is jack name + (jack type) + Jack.
> If jack type == jack name, don't add (jack type) to the name.

> Any suggestion how it should be?

That sounds reasonable as a first pass, might be worth doing a survey of
the existing users to make sure it doesn't come up with anything too
horrible.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/4] ALSA: Added jack detection kcontrol support

[Mark Brown]

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

On Wed, Jul 31, 2013 at 08:48:27PM -0700, Felipe Ferreri Tonello wrote:

> I did that but I'm not happy with the control name. Usually drivers
> add jacks like: "Headset" for a headset, "Headphone" for a headphone
> and so on.
> I did the following: control name is jack name + (jack type) + Jack.
> If jack type == jack name, don't add (jack type) to the name.

> Any suggestion how it should be?

That sounds reasonable as a first pass, might be worth doing a survey of
the existing users to make sure it doesn't come up with anything too
horrible.

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

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