[PATCH v8 0/7] ALSA: jack: Refactoring for jack kctls

[PATCH v8 0/7] ALSA: jack: Refactoring for jack kctls

[Jie Yang]

Currently only hda will create kctls for hda jacks, for asoc, people
may need create jack kctls in specific driver.

Here we are introducing kctls for each jack, by creating kctls and add
them to jack controls list (considering exist of combo jack). At the
same time, we will report events for each control in the list.

With this new implementation, both HDA and soc jack kctls works fine,
they are compatible for old HDA jack kctls, too.

For soc,
a. snd_jack_new() with NULL jack_kctl;
b. for each pin, call snd_jack_add_kctls() to add one kctl(it will
call snd_ctl_add() to add kctl to card);

For HDA,
With snd_jack_new(), we can use phantom_jack = true for phantom jack
creating, and the jack kctl pointer will be filled with the new
created kcontrol.

Changes in v8:
1. remove snd_jack_kctl from HDA part, change snd_jack_new param
to use bool for creating kcontrol at jack creating stage;
2. create jack for phantom jack, with no input device;
3. remove HDA jack get_unique_index() and index related code.

Changes in v7:
1. move name generating and index getting part into ctljack.c;
2. remove exposing private_data and destructor to hda, then jack
core can handle kctls by itself totally.
3. some small code fixing.

Changes in v6:
1. refine kctl name generating function;
2. integrate HDA jack kctl more deeply, support phantom jack in
snd_jack_new();
3. add documentation for Jack kcontrols to explain how to use it.

Changes in v5:
1. remove SND_KCTL_JACK config item, we need jack kctl once
CONFIG_SND_JACK is selected.

Changes in v4:
1. use snd_ctl_find_id() to get avaliable index;
2. add initial kctl for snd_jack_new(compatible for HDA);
3. add struct snd_jack_kctl * field to struct hda_jack_tbl;
4. new kctls for soc jack during jack pins creating.
5. add a patch to remove exporting snd_kctl_jack_new().

Changes in v3:
1. replace bit index with bit mask in jack_kctl;
2. add exception for SND_JACK_HEADSET and SND_JACK_AVOUT, only create
one jack kctl for these two combo jacks, respectively.
3. add NULL check, mem kfree, and fix some potential risk.

Change in v2:
1. define jack_kctl struct, and put jack kctl related stuff there;
2. add a patch to remove the existing controls for HDA jack.

*** BLURB HERE ***

Jie Yang (7):
  ALSA: jack: implement kctl creating for jack device
  ALSA: Jack: refactoring snd_kctl_jack_new to support embedded kctl
  ALSA: jack: extend snd_jack_new to support phantom jack
  ALSA: hda - Update to use the new jack kctls method
  ASoC: jack: create kctls according to jack pins info
  ALSA: jack: remove exporting ctljack functions
  ALSA: Docs: Add documentation for Jack kcontrols

 Documentation/sound/alsa/Jack-Controls.txt |  48 ++++++++++
 include/sound/control.h                    |   2 +-
 include/sound/jack.h                       |  19 +++-
 sound/core/Kconfig                         |   3 -
 sound/core/Makefile                        |   3 +-
 sound/core/ctljack.c                       |  41 +++++++--
 sound/core/jack.c                          | 142 ++++++++++++++++++++++++++---
 sound/pci/hda/Kconfig                      |   1 -
 sound/pci/hda/hda_jack.c                   |  90 +++++-------------
 sound/pci/hda/hda_jack.h                   |   5 +-
 sound/pci/hda/patch_hdmi.c                 |   2 +-
 sound/pci/oxygen/xonar_wm87x6.c            |   2 +-
 sound/soc/soc-jack.c                       |   3 +-
 13 files changed, 260 insertions(+), 101 deletions(-)
 create mode 100644 Documentation/sound/alsa/Jack-Controls.txt

-- 
1.9.1

[PATCH v8 1/7] ALSA: jack: implement kctl creating for jack device

[Jie Yang]

Currently the ALSA jack core registers only input devices for each jack
registered. These jack input devices are not readable by userspace devices
that run as non root. This patch and the following series patches will
implement kctls inside the core jack part, including kctls creating, status
changing report, for both HD-Audio and ASoC jack. This allows non root
userspace to read jack status and act on it.

This patch implement snd_jack_add_new_kctl(), which will create a kcontrol,
add it to the card, and also attach it to the jack kctl list.

The patch also initial the jack kctl list after jack is newed, and report
kctl status when doing jack report.

In the following patches, We will update snd_jack_new() to support phantom
jack creating, and also enable a kcontrol creating at this jack new stage.
After that, we can remove these part from HDA jack, and leave jack kctls
handled by core part thoroughly.

Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
Modified-by: Jie Yang <yang.jie@intel.com>
Signed-off-by: Jie Yang <yang.jie@intel.com>
Reveiwed-by: Mark Brown <broonie@kernel.org>
---
 include/sound/jack.h  |  15 +++++++-
 sound/core/Kconfig    |   3 --
 sound/core/Makefile   |   3 +-
 sound/core/jack.c     | 104 +++++++++++++++++++++++++++++++++++++++++++++++++-
 sound/pci/hda/Kconfig |   1 -
 5 files changed, 118 insertions(+), 8 deletions(-)

diff --git a/include/sound/jack.h b/include/sound/jack.h
index 2182350..9781e75 100644
--- a/include/sound/jack.h
+++ b/include/sound/jack.h
@@ -73,6 +73,8 @@ enum snd_jack_types {
 
 struct snd_jack {
 	struct input_dev *input_dev;
+	struct list_head kctl_list;
+	struct snd_card *card;
 	int registered;
 	int type;
 	const char *id;
@@ -82,10 +84,17 @@ struct snd_jack {
 	void (*private_free)(struct snd_jack *);
 };
 
+struct snd_jack_kctl {
+	struct snd_kcontrol *kctl;
+	struct list_head list;		/* list of controls belong to the same jack */
+	unsigned int mask_bits;	/* one of the corresponding bits of status change will report to this kctl */
+};
+
 #ifdef CONFIG_SND_JACK
 
 int snd_jack_new(struct snd_card *card, const char *id, int type,
 		 struct snd_jack **jack);
+int snd_jack_add_new_kctl(struct snd_jack *jack, const char * name, int mask);
 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);
@@ -93,13 +102,17 @@ int snd_jack_set_key(struct snd_jack *jack, enum snd_jack_types type,
 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)
 {
 	return 0;
 }
 
+static inline int snd_jack_add_new_kctl(struct snd_jack *jack, const char * name, int mask)
+{
+	return 0;
+}
+
 static inline void snd_jack_set_parent(struct snd_jack *jack,
 				       struct device *parent)
 {
diff --git a/sound/core/Kconfig b/sound/core/Kconfig
index 313f22e..63cc2e9 100644
--- a/sound/core/Kconfig
+++ b/sound/core/Kconfig
@@ -221,9 +221,6 @@ config SND_PCM_XRUN_DEBUG
 config SND_VMASTER
 	bool
 
-config SND_KCTL_JACK
-	bool
-
 config SND_DMA_SGBUF
 	def_bool y
 	depends on X86
diff --git a/sound/core/Makefile b/sound/core/Makefile
index 4daf2f5..e041dc2 100644
--- a/sound/core/Makefile
+++ b/sound/core/Makefile
@@ -7,8 +7,7 @@ snd-y     := sound.o init.o memory.o info.o control.o misc.o device.o
 snd-$(CONFIG_ISA_DMA_API) += isadma.o
 snd-$(CONFIG_SND_OSSEMUL) += sound_oss.o info_oss.o
 snd-$(CONFIG_SND_VMASTER) += vmaster.o
-snd-$(CONFIG_SND_KCTL_JACK) += ctljack.o
-snd-$(CONFIG_SND_JACK)	  += jack.o
+snd-$(CONFIG_SND_JACK)	  += ctljack.o jack.o
 
 snd-pcm-y := pcm.o pcm_native.o pcm_lib.o pcm_timer.o pcm_misc.o \
 		pcm_memory.o memalloc.o
diff --git a/sound/core/jack.c b/sound/core/jack.c
index 8658578..741db7c 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,
@@ -54,7 +55,13 @@ static int snd_jack_dev_disconnect(struct snd_device *device)
 static int snd_jack_dev_free(struct snd_device *device)
 {
 	struct snd_jack *jack = device->device_data;
+	struct snd_card *card = device->card;
+	struct snd_jack_kctl *jack_kctl, *tmp_jack_kctl;
 
+	list_for_each_entry_safe(jack_kctl, tmp_jack_kctl, &jack->kctl_list, list) {
+		list_del_init(&jack_kctl->list);
+		snd_ctl_remove(card, jack_kctl->kctl);
+	}
 	if (jack->private_free)
 		jack->private_free(jack);
 
@@ -100,6 +107,90 @@ static int snd_jack_dev_register(struct snd_device *device)
 	return err;
 }
 
+static void snd_jack_kctl_private_free(struct snd_kcontrol *kctl)
+{
+	struct snd_jack_kctl *jack_kctl;
+
+	jack_kctl = kctl->private_data;
+	if (jack_kctl) {
+		list_del(&jack_kctl->list);
+		kfree(jack_kctl);
+	}
+}
+
+static void snd_jack_kctl_add(struct snd_jack *jack, struct snd_jack_kctl *jack_kctl)
+{
+	list_add_tail(&jack_kctl->list, &jack->kctl_list);
+}
+
+/**
+ * snd_jack_kctl_new - Create a new snd_jack_kctl and return it
+ * @card:  the card instance
+ * @kctl_name:  the name for the snd_kcontrol object
+ * @mask:  a bitmask of enum snd_jack_type values that can be detected
+ *         by this snd_jack_kctl object.
+ *
+ * Creates a new snd_kcontrol object, and assign it to the new created
+ *	     snd_jack_kctl object.
+ *
+ * Return: The new created snd_jack_kctl object, or NULL if failed.
+ */
+static struct snd_jack_kctl * snd_jack_kctl_new(struct snd_card *card, const char *name, unsigned int mask)
+{
+	struct snd_kcontrol *kctl;
+	struct snd_jack_kctl *jack_kctl;
+	int err;
+
+	kctl = snd_kctl_jack_new(name, 0, card);
+	if (!kctl)
+		return NULL;
+
+	err = snd_ctl_add(card, kctl);
+	if (err < 0)
+		return NULL;
+
+	jack_kctl = kzalloc(sizeof(*jack_kctl), GFP_KERNEL);
+
+	if (!jack_kctl)
+		goto error;
+
+	jack_kctl->kctl = kctl;
+	jack_kctl->mask_bits = mask;
+
+	kctl->private_data = jack_kctl;
+	kctl->private_free = snd_jack_kctl_private_free;
+
+	return jack_kctl;
+error:
+	snd_ctl_free_one(kctl);
+	return NULL;
+}
+
+/**
+ * snd_jack_add_new_kctl - Create a new snd_jack_kctl and add it to jack
+ * @jack:  the jack instance which the kctl will attaching to
+ * @name:  the name for the snd_kcontrol object
+ * @mask:  a bitmask of enum snd_jack_type values that can be detected
+ *         by this snd_jack_kctl object.
+ *
+ * Creates a new snd_kcontrol object, and assign it to the new created
+ *	     snd_jack_kctl object, then add it to the jack kctl_list.
+ *
+ * Return: Zero if successful, or a negative error code on failure.
+ */
+int snd_jack_add_new_kctl(struct snd_jack *jack, const char * name, int mask)
+{
+	struct snd_jack_kctl *jack_kctl;
+
+	jack_kctl = snd_jack_kctl_new(jack->card, name, mask);
+	if (!jack_kctl)
+		return -ENOMEM;
+
+	snd_jack_kctl_add(jack, jack_kctl);
+	return 0;
+}
+EXPORT_SYMBOL(snd_jack_add_new_kctl);
+
 /**
  * snd_jack_new - Create a new jack
  * @card:  the card instance
@@ -150,6 +241,9 @@ int snd_jack_new(struct snd_card *card, const char *id, int type,
 	if (err < 0)
 		goto fail_input;
 
+	jack->card = card;
+	INIT_LIST_HEAD(&jack->kctl_list);
+
 	*jjack = jack;
 
 	return 0;
@@ -230,6 +324,7 @@ EXPORT_SYMBOL(snd_jack_set_key);
  */
 void snd_jack_report(struct snd_jack *jack, int status)
 {
+	struct snd_jack_kctl *jack_kctl;
 	int i;
 
 	if (!jack)
@@ -245,13 +340,20 @@ void snd_jack_report(struct snd_jack *jack, int status)
 
 	for (i = 0; i < ARRAY_SIZE(jack_switch_types); i++) {
 		int testbit = 1 << i;
-		if (jack->type & testbit)
+		if (jack->type & testbit) {
 			input_report_switch(jack->input_dev,
 					    jack_switch_types[i],
 					    status & testbit);
+		}
 	}
 
 	input_sync(jack->input_dev);
+
+	list_for_each_entry(jack_kctl, &jack->kctl_list, list) {
+		snd_kctl_jack_report(jack->card, jack_kctl->kctl,
+					status & jack_kctl->mask_bits);
+	}
+
 }
 EXPORT_SYMBOL(snd_jack_report);
 
diff --git a/sound/pci/hda/Kconfig b/sound/pci/hda/Kconfig
index 7f0f2c5..363f365 100644
--- a/sound/pci/hda/Kconfig
+++ b/sound/pci/hda/Kconfig
@@ -4,7 +4,6 @@ config SND_HDA
 	tristate
 	select SND_PCM
 	select SND_VMASTER
-	select SND_KCTL_JACK
 
 config SND_HDA_INTEL
 	tristate "HD Audio PCI"
-- 
1.9.1

[PATCH v8 2/7] ALSA: Jack: refactoring snd_kctl_jack_new to support embedded kctl

[Jie Yang]

Move available index get part into snd_kctl_jack_new(), also add kctl
name regenerating func to remove redundant " Jack" which is passed in
wrongly in some cases.

Signed-off-by: Jie Yang <yang.jie@intel.com>
---
 include/sound/control.h  |  2 +-
 sound/core/ctljack.c     | 39 +++++++++++++++++++++++++++++++++++----
 sound/core/jack.c        |  2 +-
 sound/pci/hda/hda_jack.c |  2 +-
 4 files changed, 38 insertions(+), 7 deletions(-)

diff --git a/include/sound/control.h b/include/sound/control.h
index 75f3054..58751a0 100644
--- a/include/sound/control.h
+++ b/include/sound/control.h
@@ -252,7 +252,7 @@ void snd_ctl_sync_vmaster(struct snd_kcontrol *kctl, bool hook_only);
  * Helper functions for jack-detection controls
  */
 struct snd_kcontrol *
-snd_kctl_jack_new(const char *name, int idx, void *private_data);
+snd_kctl_jack_new(const char *name, struct snd_card *card);
 void snd_kctl_jack_report(struct snd_card *card,
 			  struct snd_kcontrol *kctl, bool status);
 
diff --git a/sound/core/ctljack.c b/sound/core/ctljack.c
index e4b38fb..b631996 100644
--- a/sound/core/ctljack.c
+++ b/sound/core/ctljack.c
@@ -31,15 +31,46 @@ static struct snd_kcontrol_new jack_detect_kctl = {
 	.get = jack_detect_kctl_get,
 };
 
+static int get_available_index(struct snd_card *card, const char *name)
+{
+	struct snd_ctl_elem_id sid;
+
+	memset(&sid, 0, sizeof(sid));
+
+	sid.index = 0;
+	sid.iface = SNDRV_CTL_ELEM_IFACE_CARD;
+	strlcpy(sid.name, name, sizeof(sid.name));
+
+	while (snd_ctl_find_id(card, &sid))
+		sid.index++;
+
+	return sid.index;
+}
+
+static void jack_kctl_name_gen(char *name, const char *src_name, int size)
+{
+	size_t count = strlen(src_name);
+	bool need_cat = true;
+
+	/* remove redundant " Jack" from src_name */
+	if (count >= 5)
+		need_cat = strncmp(&src_name[count - 5], " Jack", 5) ? true : false;
+
+	snprintf(name, size, need_cat ? "%s Jack" : "%s", src_name);
+
+}
+
 struct snd_kcontrol *
-snd_kctl_jack_new(const char *name, int idx, void *private_data)
+snd_kctl_jack_new(const char *name, struct snd_card *card)
 {
 	struct snd_kcontrol *kctl;
-	kctl = snd_ctl_new1(&jack_detect_kctl, private_data);
+
+	kctl = snd_ctl_new1(&jack_detect_kctl, card);
 	if (!kctl)
 		return NULL;
-	snprintf(kctl->id.name, sizeof(kctl->id.name), "%s Jack", name);
-	kctl->id.index = idx;
+
+	jack_kctl_name_gen(kctl->id.name, name, sizeof(kctl->id.name));
+	kctl->id.index = get_available_index(card, name);
 	kctl->private_value = 0;
 	return kctl;
 }
diff --git a/sound/core/jack.c b/sound/core/jack.c
index 741db7c..b13d0b1 100644
--- a/sound/core/jack.c
+++ b/sound/core/jack.c
@@ -141,7 +141,7 @@ static struct snd_jack_kctl * snd_jack_kctl_new(struct snd_card *card, const cha
 	struct snd_jack_kctl *jack_kctl;
 	int err;
 
-	kctl = snd_kctl_jack_new(name, 0, card);
+	kctl = snd_kctl_jack_new(name, card);
 	if (!kctl)
 		return NULL;
 
diff --git a/sound/pci/hda/hda_jack.c b/sound/pci/hda/hda_jack.c
index e664307..a046e2f 100644
--- a/sound/pci/hda/hda_jack.c
+++ b/sound/pci/hda/hda_jack.c
@@ -402,7 +402,7 @@ 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);
+	kctl = snd_kctl_jack_new(name, codec);
 	if (!kctl)
 		return -ENOMEM;
 	err = snd_hda_ctl_add(codec, nid, kctl);
-- 
1.9.1

[PATCH v8 3/7] ALSA: jack: extend snd_jack_new to support phantom jack

[Jie Yang]

For phantom jack, we won't create input device, but jack kctl
may be needed.

Here, we extend snd_jack_new() to support phantom jack creating:
pass in a bool param for [non-]phantom flag, and a bool param
initial_jack to indicate that if we need create kctl at this
jack creating stage.

We can also add kctl to a jack after the jack is created.

This make the integrating the existing HDA jack kctl and soc jack
kctl possible.

Signed-off-by: Jie Yang <yang.jie@intel.com>
---
 include/sound/jack.h            |  4 ++--
 sound/core/jack.c               | 38 ++++++++++++++++++++++++++------------
 sound/pci/hda/hda_jack.c        |  2 +-
 sound/pci/oxygen/xonar_wm87x6.c |  2 +-
 sound/soc/soc-jack.c            |  2 +-
 5 files changed, 31 insertions(+), 17 deletions(-)

diff --git a/include/sound/jack.h b/include/sound/jack.h
index 9781e75..3f51c17 100644
--- a/include/sound/jack.h
+++ b/include/sound/jack.h
@@ -93,7 +93,7 @@ struct snd_jack_kctl {
 #ifdef CONFIG_SND_JACK
 
 int snd_jack_new(struct snd_card *card, const char *id, int type,
-		 struct snd_jack **jack);
+		 struct snd_jack **jack, bool initial_kctl, bool phantom_jack);
 int snd_jack_add_new_kctl(struct snd_jack *jack, const char * name, int mask);
 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,
@@ -103,7 +103,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)
+			       struct snd_jack **jack, bool initial_kctl, bool phantom_jack)
 {
 	return 0;
 }
diff --git a/sound/core/jack.c b/sound/core/jack.c
index b13d0b1..6c729ef 100644
--- a/sound/core/jack.c
+++ b/sound/core/jack.c
@@ -198,6 +198,9 @@ EXPORT_SYMBOL(snd_jack_add_new_kctl);
  * @type:  a bitmask of enum snd_jack_type values that can be detected by
  *         this jack
  * @jjack: Used to provide the allocated jack object to the caller.
+ * @phantom_jack: for phantom jack, only create needed kctl, won't create
+ *         input device
+ * @initial_kctl: create kctl if true, also add it to the non-phantom jack kctl list
  *
  * Creates a new jack object.
  *
@@ -205,9 +208,10 @@ EXPORT_SYMBOL(snd_jack_add_new_kctl);
  * On success @jjack will be initialised.
  */
 int snd_jack_new(struct snd_card *card, const char *id, int type,
-		 struct snd_jack **jjack)
+		 struct snd_jack **jjack, bool initial_kctl, bool phantom_jack)
 {
 	struct snd_jack *jack;
+	struct snd_jack_kctl *jack_kctl = NULL;
 	int err;
 	int i;
 	static struct snd_device_ops ops = {
@@ -216,26 +220,33 @@ int snd_jack_new(struct snd_card *card, const char *id, int type,
 		.dev_disconnect = snd_jack_dev_disconnect,
 	};
 
+	if (initial_kctl)
+		jack_kctl = snd_jack_kctl_new(card, id, type);
+
 	jack = kzalloc(sizeof(struct snd_jack), GFP_KERNEL);
 	if (jack == NULL)
 		return -ENOMEM;
 
 	jack->id = kstrdup(id, GFP_KERNEL);
 
-	jack->input_dev = input_allocate_device();
-	if (jack->input_dev == NULL) {
-		err = -ENOMEM;
-		goto fail_input;
-	}
+	/* don't creat input device for phantom jack */
+	if (!phantom_jack) {
+		jack->input_dev = input_allocate_device();
+		if (jack->input_dev == NULL) {
+			err = -ENOMEM;
+			goto fail_input;
+		}
 
-	jack->input_dev->phys = "ALSA";
+		jack->input_dev->phys = "ALSA";
 
-	jack->type = type;
+		jack->type = type;
 
-	for (i = 0; i < SND_JACK_SWITCH_TYPES; i++)
-		if (type & (1 << i))
-			input_set_capability(jack->input_dev, EV_SW,
-					     jack_switch_types[i]);
+		for (i = 0; i < SND_JACK_SWITCH_TYPES; i++)
+			if (type & (1 << i))
+				input_set_capability(jack->input_dev, EV_SW,
+						     jack_switch_types[i]);
+
+	}
 
 	err = snd_device_new(card, SNDRV_DEV_JACK, jack, &ops);
 	if (err < 0)
@@ -244,6 +255,9 @@ int snd_jack_new(struct snd_card *card, const char *id, int type,
 	jack->card = card;
 	INIT_LIST_HEAD(&jack->kctl_list);
 
+	if (initial_kctl && jack_kctl)
+		snd_jack_kctl_add(jack, jack_kctl);
+
 	*jjack = jack;
 
 	return 0;
diff --git a/sound/pci/hda/hda_jack.c b/sound/pci/hda/hda_jack.c
index a046e2f..5314c73 100644
--- a/sound/pci/hda/hda_jack.c
+++ b/sound/pci/hda/hda_jack.c
@@ -417,7 +417,7 @@ static int __snd_hda_jack_add_kctl(struct hda_codec *codec, hda_nid_t nid,
 	if (!phantom_jack) {
 		jack->type = get_input_jack_type(codec, nid);
 		err = snd_jack_new(codec->bus->card, name, jack->type,
-				   &jack->jack);
+				   &jack->jack, false, false);
 		if (err < 0)
 			return err;
 		jack->jack->private_data = jack;
diff --git a/sound/pci/oxygen/xonar_wm87x6.c b/sound/pci/oxygen/xonar_wm87x6.c
index 6ce6860..90ac479 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, &data->hp_jack, false, false);
 	xonar_ds_handle_hp_jack(chip);
 
 	snd_component_add(chip->card, "WM8776");
diff --git a/sound/soc/soc-jack.c b/sound/soc/soc-jack.c
index 9f60c25..70a9bdd 100644
--- a/sound/soc/soc-jack.c
+++ b/sound/soc/soc-jack.c
@@ -48,7 +48,7 @@ int snd_soc_card_jack_new(struct snd_soc_card *card, const char *id, int type,
 	INIT_LIST_HEAD(&jack->jack_zones);
 	BLOCKING_INIT_NOTIFIER_HEAD(&jack->notifier);
 
-	ret = snd_jack_new(card->snd_card, id, type, &jack->jack);
+	ret = snd_jack_new(card->snd_card, id, type, &jack->jack, false, false);
 	if (ret)
 		return ret;
 
-- 
1.9.1

[PATCH v8 4/7] ALSA: hda - Update to use the new jack kctls method

[Jie Yang]

In the new jack kctls design, snd_kcontrol can be created
during snd_jack_new(), or calling snd_jack_add_new_kctls()
to create and attach to the created jack later.

Here we create jack kctls at the jack creating stage, for
both phantom/non-phantom jack.

Signed-off-by: Jie Yang <yang.jie@intel.com>
---
 sound/pci/hda/hda_jack.c   | 90 +++++++++++++---------------------------------
 sound/pci/hda/hda_jack.h   |  5 +--
 sound/pci/hda/patch_hdmi.c |  2 +-
 3 files changed, 27 insertions(+), 70 deletions(-)

diff --git a/sound/pci/hda/hda_jack.c b/sound/pci/hda/hda_jack.c
index 5314c73..6aeb29d 100644
--- a/sound/pci/hda/hda_jack.c
+++ b/sound/pci/hda/hda_jack.c
@@ -132,11 +132,11 @@ void snd_hda_jack_tbl_clear(struct hda_codec *codec)
 
 	for (i = 0; i < codec->jacktbl.used; i++, jack++) {
 		struct hda_jack_callback *cb, *next;
-#ifdef CONFIG_SND_HDA_INPUT_JACK
+
 		/* free jack instances manually when clearing/reconfiguring */
 		if (!codec->bus->shutdown && jack->jack)
 			snd_device_free(codec->bus->card, jack->jack);
-#endif
+
 		for (cb = jack->callback; cb; cb = next) {
 			next = cb->next;
 			kfree(cb);
@@ -337,20 +337,15 @@ void snd_hda_jack_report_sync(struct hda_codec *codec)
 	jack = codec->jacktbl.list;
 	for (i = 0; i < codec->jacktbl.used; i++, jack++)
 		if (jack->nid) {
-			if (!jack->kctl || jack->block_report)
+			if (!jack->jack || jack->block_report)
 				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)
-				snd_jack_report(jack->jack,
-						state ? jack->type : 0);
-#endif
+			snd_jack_report(jack->jack,
+					state ? jack->type : 0);
 		}
 }
 EXPORT_SYMBOL_GPL(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)
 {
@@ -377,54 +372,42 @@ 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
  * @codec: the HDA codec
  * @nid: pin NID to assign
  * @name: string name for the jack
- * @idx: index number for the jack
  * @phantom_jack: flag to deal as a phantom jack
  *
  * This assigns a jack-detection kctl to the given pin.  The kcontrol
  * will have the given name and index.
  */
 static int __snd_hda_jack_add_kctl(struct hda_codec *codec, hda_nid_t nid,
-			  const char *name, int idx, bool phantom_jack)
+			  const char *name, bool phantom_jack)
 {
 	struct hda_jack_tbl *jack;
-	struct snd_kcontrol *kctl;
-	int err, state;
+	int err, state, type;
 
 	jack = snd_hda_jack_tbl_new(codec, nid);
 	if (!jack)
 		return 0;
-	if (jack->kctl)
+	if (jack->jack)
 		return 0; /* already created */
-	kctl = snd_kctl_jack_new(name, codec);
-	if (!kctl)
-		return -ENOMEM;
-	err = snd_hda_ctl_add(codec, nid, kctl);
+
+	type = get_input_jack_type(codec, nid);
+	err = snd_jack_new(codec->bus->card, name, type,
+			   &jack->jack, true, phantom_jack);
 	if (err < 0)
 		return err;
-	jack->kctl = kctl;
-	jack->phantom_jack = !!phantom_jack;
 
+	jack->phantom_jack = !!phantom_jack;
+	jack->type = type;
+	jack->jack->private_data = jack;
+	jack->jack->private_free = hda_free_jack_priv;
 	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) {
-		jack->type = get_input_jack_type(codec, nid);
-		err = snd_jack_new(codec->bus->card, name, jack->type,
-				   &jack->jack, false, false);
-		if (err < 0)
-			return err;
-		jack->jack->private_data = jack;
-		jack->jack->private_free = hda_free_jack_priv;
-		snd_jack_report(jack->jack, state ? jack->type : 0);
-	}
-#endif
+	snd_jack_report(jack->jack, state ? jack->type : 0);
+
 	return 0;
 }
 
@@ -433,44 +416,23 @@ static int __snd_hda_jack_add_kctl(struct hda_codec *codec, hda_nid_t nid,
  * @codec: the HDA codec
  * @nid: pin NID
  * @name: the name string for the jack ctl
- * @idx: the ctl index for the jack ctl
  *
  * This is a simple helper calling __snd_hda_jack_add_kctl().
  */
 int snd_hda_jack_add_kctl(struct hda_codec *codec, hda_nid_t nid,
-			  const char *name, int idx)
+			  const char *name)
 {
-	return __snd_hda_jack_add_kctl(codec, nid, name, idx, false);
+	return __snd_hda_jack_add_kctl(codec, nid, name, false);
 }
 EXPORT_SYMBOL_GPL(snd_hda_jack_add_kctl);
 
-/* get the unique index number for the given kctl name */
-static int get_unique_index(struct hda_codec *codec, const char *name, int idx)
-{
-	struct hda_jack_tbl *jack;
-	int i, len = strlen(name);
- again:
-	jack = codec->jacktbl.list;
-	for (i = 0; i < codec->jacktbl.used; i++, jack++) {
-		/* jack->kctl.id contains "XXX Jack" name string with index */
-		if (jack->kctl &&
-		    !strncmp(name, jack->kctl->id.name, len) &&
-		    !strcmp(" Jack", jack->kctl->id.name + len) &&
-		    jack->kctl->id.index == idx) {
-			idx++;
-			goto again;
-		}
-	}
-	return idx;
-}
-
 static int add_jack_kctl(struct hda_codec *codec, hda_nid_t nid,
 			 const struct auto_pin_cfg *cfg,
 			 const char *base_name)
 {
 	unsigned int def_conf, conn;
 	char name[SNDRV_CTL_ELEM_ID_NAME_MAXLEN];
-	int idx, err;
+	int err;
 	bool phantom_jack;
 
 	if (!nid)
@@ -482,16 +444,14 @@ static int add_jack_kctl(struct hda_codec *codec, hda_nid_t nid,
 	phantom_jack = (conn != AC_JACK_PORT_COMPLEX) ||
 		       !is_jack_detectable(codec, nid);
 
-	if (base_name) {
+	if (base_name)
 		strlcpy(name, base_name, sizeof(name));
-		idx = 0;
-	} else
-		snd_hda_get_pin_label(codec, nid, cfg, name, sizeof(name), &idx);
+	else
+		snd_hda_get_pin_label(codec, nid, cfg, name, sizeof(name), NULL);
 	if (phantom_jack)
 		/* Example final name: "Internal Mic Phantom Jack" */
 		strncat(name, " Phantom", sizeof(name) - strlen(name) - 1);
-	idx = get_unique_index(codec, name, idx);
-	err = __snd_hda_jack_add_kctl(codec, nid, name, idx, phantom_jack);
+	err = __snd_hda_jack_add_kctl(codec, nid, name, phantom_jack);
 	if (err < 0)
 		return err;
 
diff --git a/sound/pci/hda/hda_jack.h b/sound/pci/hda/hda_jack.h
index b279e32..387d309 100644
--- a/sound/pci/hda/hda_jack.h
+++ b/sound/pci/hda/hda_jack.h
@@ -39,11 +39,8 @@ struct hda_jack_tbl {
 	unsigned int block_report:1;    /* in a transitional state - do not report to userspace */
 	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
 	int type;
 	struct snd_jack *jack;
-#endif
 };
 
 struct hda_jack_tbl *
@@ -85,7 +82,7 @@ static inline bool snd_hda_jack_detect(struct hda_codec *codec, hda_nid_t nid)
 bool is_jack_detectable(struct hda_codec *codec, hda_nid_t nid);
 
 int snd_hda_jack_add_kctl(struct hda_codec *codec, hda_nid_t nid,
-			  const char *name, int idx);
+			  const char *name);
 int snd_hda_jack_add_kctls(struct hda_codec *codec,
 			   const struct auto_pin_cfg *cfg);
 
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index b422e40..1679337 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -2089,7 +2089,7 @@ static int generic_hdmi_build_jack(struct hda_codec *codec, int pin_idx)
 		strncat(hdmi_str, " Phantom",
 			sizeof(hdmi_str) - strlen(hdmi_str) - 1);
 
-	return snd_hda_jack_add_kctl(codec, per_pin->pin_nid, hdmi_str, 0);
+	return snd_hda_jack_add_kctl(codec, per_pin->pin_nid, hdmi_str);
 }
 
 static int generic_hdmi_build_controls(struct hda_codec *codec)
-- 
1.9.1

[PATCH v8 5/7] ASoC: jack: create kctls according to jack pins info

[Jie Yang]

In the new jack kctls design, we can create kctls according to
pins info:

1. during jack creating, snd_jack_new() with NULL jack_kctl;
2. for each pin, call snd_jack_add_kctl() to add one kctl
(it will call snd_ctl_add() to add kctl to card);

Signed-off-by: Jie Yang <yang.jie@intel.com>
---
 sound/soc/soc-jack.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/sound/soc/soc-jack.c b/sound/soc/soc-jack.c
index 70a9bdd..87ca980 100644
--- a/sound/soc/soc-jack.c
+++ b/sound/soc/soc-jack.c
@@ -197,6 +197,7 @@ int snd_soc_jack_add_pins(struct snd_soc_jack *jack, int count,
 
 		INIT_LIST_HEAD(&pins[i].list);
 		list_add(&(pins[i].list), &jack->pins);
+		snd_jack_add_new_kctl(jack->jack, pins[i].pin, pins[i].mask);
 	}
 
 	/* Update to reflect the last reported status; canned jack
-- 
1.9.1

[PATCH v8 6/7] ALSA: jack: remove exporting ctljack functions

[Jie Yang]

After created jack embedded kctls and removed hda jack kctls, the funcs
snd_kctl_jack_new() and snd_kctl_jack_report() are merely internal called,
so here remove exporting them.

Signed-off-by: Jie Yang <yang.jie@intel.com>
---
 sound/core/ctljack.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/sound/core/ctljack.c b/sound/core/ctljack.c
index b631996..9ff955c 100644
--- a/sound/core/ctljack.c
+++ b/sound/core/ctljack.c
@@ -74,7 +74,6 @@ snd_kctl_jack_new(const char *name, struct snd_card *card)
 	kctl->private_value = 0;
 	return kctl;
 }
-EXPORT_SYMBOL_GPL(snd_kctl_jack_new);
 
 void snd_kctl_jack_report(struct snd_card *card,
 			  struct snd_kcontrol *kctl, bool status)
@@ -84,4 +83,3 @@ void snd_kctl_jack_report(struct snd_card *card,
 	kctl->private_value = status;
 	snd_ctl_notify(card, SNDRV_CTL_EVENT_MASK_VALUE, &kctl->id);
 }
-EXPORT_SYMBOL_GPL(snd_kctl_jack_report);
-- 
1.9.1

[PATCH v8 7/7] ALSA: Docs: Add documentation for Jack kcontrols

[Jie Yang]

Add documentation describing Jack embedded kcontrols, and how to
use it on HD-Audio and ASoC case.

Signed-off-by: Jie Yang <yang.jie@intel.com>
---
 Documentation/sound/alsa/Jack-Controls.txt | 48 ++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)
 create mode 100644 Documentation/sound/alsa/Jack-Controls.txt

diff --git a/Documentation/sound/alsa/Jack-Controls.txt b/Documentation/sound/alsa/Jack-Controls.txt
new file mode 100644
index 0000000..9e93947
--- /dev/null
+++ b/Documentation/sound/alsa/Jack-Controls.txt
@@ -0,0 +1,48 @@
+Why we need kcontrols for Jack?
+===============================
+
+ALSA using kcontrols to export controlling(switch, volume, Mux, ...) to
+user space, e.g. pulseaudio can switch off headphone and switch on
+speaker when no heaphone is pluged in.
+
+For each physical jack, there may be dynamic pluged in/out status which
+upper layer may be interested in. Currently the ALSA jack core registers
+only input devices for each jack registered. These jack input devices
+are not readable by userspace devices that run as non root.
+
+So we create embeded kcontrols for each jack, report jack event via
+these kcontrols, userspace switch them to ack on these event, to
+implement what they are wanna to.
+
+Combining with ucm machanism, userpace can do fantasic controlling as
+they want to.
+
+Jack and kcontrols
+==================
+
+Each jack will have a kcontrol list, we can create a kcontrol and attach
+it to the jack, at jack creating stage. Also, we can add kcontrol to an
+exist jack, at anytime when you are happy.
+
+Those kcontrols will be freed automatically when the Jack is freed.
+
+How to create kcontrol embedded jack?
+=====================================
+
+To be compatible with previous HD-Audio jack and ASoC jack, we modify
+snd_jack_new() with adding two params:
+
+ - @phantom_jack: for phantom jack, only create needed kctl, won't create
+	real jack device.
+ - @jjack_kctl: create kctl if non-NULL pointer passed in, and provide
+	it to the caller. also add it to the non-phantom jack kctl list.
+
+For HDA jack, we can use phantom_jack = true for phantom jack creating,
+and the jack kctl pointer will be filled with the new created kcontrol.
+
+For ASoC jack, usually, we create kcontrols after jack is created, with
+calling snd_hda_jack_add_kctl() for each jack pins. And, the pin name
+will be assigned to the corresponding kcontrol name. So, to make
+userspace understand and recognize it, please assign proper name for
+each jack pin.
+
-- 
1.9.1

Re: [PATCH v8 1/7] ALSA: jack: implement kctl creating for jack device

[Takashi Iwai]

At Wed, 22 Apr 2015 14:03:47 +0800,
Jie Yang wrote:
> 
> Currently the ALSA jack core registers only input devices for each jack
> registered. These jack input devices are not readable by userspace devices
> that run as non root. This patch and the following series patches will
> implement kctls inside the core jack part, including kctls creating, status
> changing report, for both HD-Audio and ASoC jack. This allows non root
> userspace to read jack status and act on it.
> 
> This patch implement snd_jack_add_new_kctl(), which will create a kcontrol,
> add it to the card, and also attach it to the jack kctl list.
> 
> The patch also initial the jack kctl list after jack is newed, and report
> kctl status when doing jack report.
> 
> In the following patches, We will update snd_jack_new() to support phantom
> jack creating, and also enable a kcontrol creating at this jack new stage.
> After that, we can remove these part from HDA jack, and leave jack kctls
> handled by core part thoroughly.
> 
> Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
> Modified-by: Jie Yang <yang.jie@intel.com>
> Signed-off-by: Jie Yang <yang.jie@intel.com>
> Reveiwed-by: Mark Brown <broonie@kernel.org>
> ---
>  include/sound/jack.h  |  15 +++++++-
>  sound/core/Kconfig    |   3 --
>  sound/core/Makefile   |   3 +-
>  sound/core/jack.c     | 104 +++++++++++++++++++++++++++++++++++++++++++++++++-
>  sound/pci/hda/Kconfig |   1 -
>  5 files changed, 118 insertions(+), 8 deletions(-)
> 
> diff --git a/include/sound/jack.h b/include/sound/jack.h
> index 2182350..9781e75 100644
> --- a/include/sound/jack.h
> +++ b/include/sound/jack.h
> @@ -73,6 +73,8 @@ enum snd_jack_types {
>  
>  struct snd_jack {
>  	struct input_dev *input_dev;
> +	struct list_head kctl_list;
> +	struct snd_card *card;
>  	int registered;
>  	int type;
>  	const char *id;
> @@ -82,10 +84,17 @@ struct snd_jack {
>  	void (*private_free)(struct snd_jack *);
>  };
>  
> +struct snd_jack_kctl {
> +	struct snd_kcontrol *kctl;
> +	struct list_head list;		/* list of controls belong to the same jack */
> +	unsigned int mask_bits;	/* one of the corresponding bits of status change will report to this kctl */
> +};

This struct isn't referred outside jack.c, so we can move it to
jack.c.  This hides the internal implementation details.


>  #ifdef CONFIG_SND_JACK
>  
>  int snd_jack_new(struct snd_card *card, const char *id, int type,
>  		 struct snd_jack **jack);
> +int snd_jack_add_new_kctl(struct snd_jack *jack, const char * name, int mask);
>  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);
> @@ -93,13 +102,17 @@ int snd_jack_set_key(struct snd_jack *jack, enum snd_jack_types type,
>  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)
>  {
>  	return 0;
>  }
>  
> +static inline int snd_jack_add_new_kctl(struct snd_jack *jack, const char * name, int mask)
> +{
> +	return 0;
> +}
> +
>  static inline void snd_jack_set_parent(struct snd_jack *jack,
>  				       struct device *parent)
>  {
> diff --git a/sound/core/Kconfig b/sound/core/Kconfig
> index 313f22e..63cc2e9 100644
> --- a/sound/core/Kconfig
> +++ b/sound/core/Kconfig
> @@ -221,9 +221,6 @@ config SND_PCM_XRUN_DEBUG
>  config SND_VMASTER
>  	bool
>  
> -config SND_KCTL_JACK
> -	bool
> -
>  config SND_DMA_SGBUF
>  	def_bool y
>  	depends on X86
> diff --git a/sound/core/Makefile b/sound/core/Makefile
> index 4daf2f5..e041dc2 100644
> --- a/sound/core/Makefile
> +++ b/sound/core/Makefile
> @@ -7,8 +7,7 @@ snd-y     := sound.o init.o memory.o info.o control.o misc.o device.o
>  snd-$(CONFIG_ISA_DMA_API) += isadma.o
>  snd-$(CONFIG_SND_OSSEMUL) += sound_oss.o info_oss.o
>  snd-$(CONFIG_SND_VMASTER) += vmaster.o
> -snd-$(CONFIG_SND_KCTL_JACK) += ctljack.o
> -snd-$(CONFIG_SND_JACK)	  += jack.o
> +snd-$(CONFIG_SND_JACK)	  += ctljack.o jack.o

I guess this breaks the build when CONFIG_SND_HDA_INPUT_JACK=n.


Takashi

Re: [PATCH v8 3/7] ALSA: jack: extend snd_jack_new to support phantom jack

[Takashi Iwai]

At Wed, 22 Apr 2015 14:03:49 +0800,
Jie Yang wrote:
> 
> diff --git a/sound/core/jack.c b/sound/core/jack.c
> index b13d0b1..6c729ef 100644
> --- a/sound/core/jack.c
> +++ b/sound/core/jack.c
> @@ -198,6 +198,9 @@ EXPORT_SYMBOL(snd_jack_add_new_kctl);
>   * @type:  a bitmask of enum snd_jack_type values that can be detected by
>   *         this jack
>   * @jjack: Used to provide the allocated jack object to the caller.
> + * @phantom_jack: for phantom jack, only create needed kctl, won't create
> + *         input device
> + * @initial_kctl: create kctl if true, also add it to the non-phantom jack kctl list

Better to align with the actual argument order.

>   *
>   * Creates a new jack object.
>   *
> @@ -205,9 +208,10 @@ EXPORT_SYMBOL(snd_jack_add_new_kctl);
>   * On success @jjack will be initialised.
>   */
>  int snd_jack_new(struct snd_card *card, const char *id, int type,
> -		 struct snd_jack **jjack)
> +		 struct snd_jack **jjack, bool initial_kctl, bool phantom_jack)
>  {
>  	struct snd_jack *jack;
> +	struct snd_jack_kctl *jack_kctl = NULL;
>  	int err;
>  	int i;
>  	static struct snd_device_ops ops = {
> @@ -216,26 +220,33 @@ int snd_jack_new(struct snd_card *card, const char *id, int type,
>  		.dev_disconnect = snd_jack_dev_disconnect,
>  	};
>  
> +	if (initial_kctl)
> +		jack_kctl = snd_jack_kctl_new(card, id, type);
> +

The function should return -ENOMEM if snd_jack_kctl_new() fails.

>  	jack = kzalloc(sizeof(struct snd_jack), GFP_KERNEL);
>  	if (jack == NULL)
>  		return -ENOMEM;
>  
>  	jack->id = kstrdup(id, GFP_KERNEL);
>  
> -	jack->input_dev = input_allocate_device();
> -	if (jack->input_dev == NULL) {
> -		err = -ENOMEM;
> -		goto fail_input;
> -	}
> +	/* don't creat input device for phantom jack */
> +	if (!phantom_jack) {
> +		jack->input_dev = input_allocate_device();
> +		if (jack->input_dev == NULL) {
> +			err = -ENOMEM;
> +			goto fail_input;
> +		}
>  
> -	jack->input_dev->phys = "ALSA";
> +		jack->input_dev->phys = "ALSA";
>  
> -	jack->type = type;
> +		jack->type = type;
>  
> -	for (i = 0; i < SND_JACK_SWITCH_TYPES; i++)
> -		if (type & (1 << i))
> -			input_set_capability(jack->input_dev, EV_SW,
> -					     jack_switch_types[i]);
> +		for (i = 0; i < SND_JACK_SWITCH_TYPES; i++)
> +			if (type & (1 << i))
> +				input_set_capability(jack->input_dev, EV_SW,
> +						     jack_switch_types[i]);
> +
> +	}
>  
>  	err = snd_device_new(card, SNDRV_DEV_JACK, jack, &ops);
>  	if (err < 0)
> @@ -244,6 +255,9 @@ int snd_jack_new(struct snd_card *card, const char *id, int type,
>  	jack->card = card;
>  	INIT_LIST_HEAD(&jack->kctl_list);
>  
> +	if (initial_kctl && jack_kctl)

... then here no need to check both.


Takashi

Re: [PATCH v8 2/7] ALSA: Jack: refactoring snd_kctl_jack_new to support embedded kctl

[Takashi Iwai]

At Wed, 22 Apr 2015 14:03:48 +0800,
Jie Yang wrote:
> 
> Move available index get part into snd_kctl_jack_new(), also add kctl
> name regenerating func to remove redundant " Jack" which is passed in
> wrongly in some cases.

The subject doesn't match with this description well.


> Signed-off-by: Jie Yang <yang.jie@intel.com>
> ---
>  include/sound/control.h  |  2 +-
>  sound/core/ctljack.c     | 39 +++++++++++++++++++++++++++++++++++----
>  sound/core/jack.c        |  2 +-
>  sound/pci/hda/hda_jack.c |  2 +-
>  4 files changed, 38 insertions(+), 7 deletions(-)
> 
> diff --git a/include/sound/control.h b/include/sound/control.h
> index 75f3054..58751a0 100644
> --- a/include/sound/control.h
> +++ b/include/sound/control.h
> @@ -252,7 +252,7 @@ void snd_ctl_sync_vmaster(struct snd_kcontrol *kctl, bool hook_only);
>   * Helper functions for jack-detection controls
>   */
>  struct snd_kcontrol *
> -snd_kctl_jack_new(const char *name, int idx, void *private_data);
> +snd_kctl_jack_new(const char *name, struct snd_card *card);
>  void snd_kctl_jack_report(struct snd_card *card,
>  			  struct snd_kcontrol *kctl, bool status);
>  
> diff --git a/sound/core/ctljack.c b/sound/core/ctljack.c
> index e4b38fb..b631996 100644
> --- a/sound/core/ctljack.c
> +++ b/sound/core/ctljack.c
> @@ -31,15 +31,46 @@ static struct snd_kcontrol_new jack_detect_kctl = {
>  	.get = jack_detect_kctl_get,
>  };
>  
> +static int get_available_index(struct snd_card *card, const char *name)
> +{
> +	struct snd_ctl_elem_id sid;
> +
> +	memset(&sid, 0, sizeof(sid));
> +
> +	sid.index = 0;
> +	sid.iface = SNDRV_CTL_ELEM_IFACE_CARD;
> +	strlcpy(sid.name, name, sizeof(sid.name));
> +
> +	while (snd_ctl_find_id(card, &sid))
> +		sid.index++;
> +
> +	return sid.index;
> +}
> +
> +static void jack_kctl_name_gen(char *name, const char *src_name, int size)
> +{
> +	size_t count = strlen(src_name);
> +	bool need_cat = true;
> +
> +	/* remove redundant " Jack" from src_name */
> +	if (count >= 5)
> +		need_cat = strncmp(&src_name[count - 5], " Jack", 5) ? true : false;
> +
> +	snprintf(name, size, need_cat ? "%s Jack" : "%s", src_name);
> +
> +}
> +
>  struct snd_kcontrol *
> -snd_kctl_jack_new(const char *name, int idx, void *private_data)
> +snd_kctl_jack_new(const char *name, struct snd_card *card)
>  {
>  	struct snd_kcontrol *kctl;
> -	kctl = snd_ctl_new1(&jack_detect_kctl, private_data);
> +
> +	kctl = snd_ctl_new1(&jack_detect_kctl, card);

Why changing the private data?  Did it really work...?


Takashi

Re: [PATCH v8 1/7] ALSA: jack: implement kctl creating for jack device

[Jie, Yang]

> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai@suse.de]
> Sent: Wednesday, April 22, 2015 7:28 PM
> To: Jie, Yang
> Cc: broonie@kernel.org; alsa-devel@alsa-project.org; Girdwood, Liam R;
> tanu.kaskinen@linux.intel.com; Liam Girdwood
> Subject: Re: [PATCH v8 1/7] ALSA: jack: implement kctl creating for jack
> device
> 
> At Wed, 22 Apr 2015 14:03:47 +0800,
> Jie Yang wrote:
> >
> > Currently the ALSA jack core registers only input devices for each
> > jack registered. These jack input devices are not readable by
> > userspace devices that run as non root. This patch and the following
> > series patches will implement kctls inside the core jack part,
> > including kctls creating, status changing report, for both HD-Audio
> > and ASoC jack. This allows non root userspace to read jack status and act on
> it.
> >
> > This patch implement snd_jack_add_new_kctl(), which will create a
> > kcontrol, add it to the card, and also attach it to the jack kctl list.
> >
> > The patch also initial the jack kctl list after jack is newed, and
> > report kctl status when doing jack report.
> >
> > In the following patches, We will update snd_jack_new() to support
> > phantom jack creating, and also enable a kcontrol creating at this jack new
> stage.
> > After that, we can remove these part from HDA jack, and leave jack
> > kctls handled by core part thoroughly.
> >
> > Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
> > Modified-by: Jie Yang <yang.jie@intel.com>
> > Signed-off-by: Jie Yang <yang.jie@intel.com>
> > Reveiwed-by: Mark Brown <broonie@kernel.org>
> > ---
> >  include/sound/jack.h  |  15 +++++++-
> >  sound/core/Kconfig    |   3 --
> >  sound/core/Makefile   |   3 +-
> >  sound/core/jack.c     | 104
> +++++++++++++++++++++++++++++++++++++++++++++++++-
> >  sound/pci/hda/Kconfig |   1 -
> >  5 files changed, 118 insertions(+), 8 deletions(-)
> >
> > diff --git a/include/sound/jack.h b/include/sound/jack.h index
> > 2182350..9781e75 100644
> > --- a/include/sound/jack.h
> > +++ b/include/sound/jack.h
> > @@ -73,6 +73,8 @@ enum snd_jack_types {
> >
> >  struct snd_jack {
> >  	struct input_dev *input_dev;
> > +	struct list_head kctl_list;
> > +	struct snd_card *card;
> >  	int registered;
> >  	int type;
> >  	const char *id;
> > @@ -82,10 +84,17 @@ struct snd_jack {
> >  	void (*private_free)(struct snd_jack *);  };
> >
> > +struct snd_jack_kctl {
> > +	struct snd_kcontrol *kctl;
> > +	struct list_head list;		/* list of controls belong to the same
> jack */
> > +	unsigned int mask_bits;	/* one of the corresponding bits of
> status change will report to this kctl */
> > +};
> 
> This struct isn't referred outside jack.c, so we can move it to jack.c.  This hides
> the internal implementation details.
 
Also thought of it, will change it soon.

> 
> 
> >  #ifdef CONFIG_SND_JACK
> >
> >  int snd_jack_new(struct snd_card *card, const char *id, int type,
> >  		 struct snd_jack **jack);
> > +int snd_jack_add_new_kctl(struct snd_jack *jack, const char * name,
> > +int mask);
> >  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);
> > @@ -93,13 +102,17 @@ int snd_jack_set_key(struct snd_jack *jack, enum
> > snd_jack_types type,  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)
> >  {
> >  	return 0;
> >  }
> >
> > +static inline int snd_jack_add_new_kctl(struct snd_jack *jack, const
> > +char * name, int mask) {
> > +	return 0;
> > +}
> > +
> >  static inline void snd_jack_set_parent(struct snd_jack *jack,
> >  				       struct device *parent)
> >  {
> > diff --git a/sound/core/Kconfig b/sound/core/Kconfig index
> > 313f22e..63cc2e9 100644
> > --- a/sound/core/Kconfig
> > +++ b/sound/core/Kconfig
> > @@ -221,9 +221,6 @@ config SND_PCM_XRUN_DEBUG  config
> SND_VMASTER
> >  	bool
> >
> > -config SND_KCTL_JACK
> > -	bool
> > -
> >  config SND_DMA_SGBUF
> >  	def_bool y
> >  	depends on X86
> > diff --git a/sound/core/Makefile b/sound/core/Makefile index
> > 4daf2f5..e041dc2 100644
> > --- a/sound/core/Makefile
> > +++ b/sound/core/Makefile
> > @@ -7,8 +7,7 @@ snd-y     := sound.o init.o memory.o info.o control.o
> misc.o device.o
> >  snd-$(CONFIG_ISA_DMA_API) += isadma.o
> >  snd-$(CONFIG_SND_OSSEMUL) += sound_oss.o info_oss.o
> >  snd-$(CONFIG_SND_VMASTER) += vmaster.o
> > -snd-$(CONFIG_SND_KCTL_JACK) += ctljack.o
> > -snd-$(CONFIG_SND_JACK)	  += jack.o
> > +snd-$(CONFIG_SND_JACK)	  += ctljack.o jack.o
> 
> I guess this breaks the build when CONFIG_SND_HDA_INPUT_JACK=n.
 
Actually not, just double confirm it.

> 
> 
> Takashi

Re: [PATCH v8 2/7] ALSA: Jack: refactoring snd_kctl_jack_new to support embedded kctl

[Jie, Yang]

> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai@suse.de]
> Sent: Wednesday, April 22, 2015 7:33 PM
> To: Jie, Yang
> Cc: broonie@kernel.org; alsa-devel@alsa-project.org; Girdwood, Liam R;
> tanu.kaskinen@linux.intel.com
> Subject: Re: [PATCH v8 2/7] ALSA: Jack: refactoring snd_kctl_jack_new to
> support embedded kctl
> 
> At Wed, 22 Apr 2015 14:03:48 +0800,
> Jie Yang wrote:
> >
> > Move available index get part into snd_kctl_jack_new(), also add kctl
> > name regenerating func to remove redundant " Jack" which is passed in
> > wrongly in some cases.
> 
> The subject doesn't match with this description well.
 
How about changing them as below:

ALSA: Jack: handle jack embedded kcontrol creating within ctljack

In ctljack.c, add function get_available_index() to allocate
index for new kcontrol, and jack_kctl_name_gen() to remove
redundant " Jack" which is passed in wrongly in some cases.

> 
> 
> > Signed-off-by: Jie Yang <yang.jie@intel.com>
> > ---
> >  include/sound/control.h  |  2 +-
> >  sound/core/ctljack.c     | 39 +++++++++++++++++++++++++++++++++++-
> ---
> >  sound/core/jack.c        |  2 +-
> >  sound/pci/hda/hda_jack.c |  2 +-
> >  4 files changed, 38 insertions(+), 7 deletions(-)
> >
> > diff --git a/include/sound/control.h b/include/sound/control.h index
> > 75f3054..58751a0 100644
> > --- a/include/sound/control.h
> > +++ b/include/sound/control.h
> > @@ -252,7 +252,7 @@ void snd_ctl_sync_vmaster(struct snd_kcontrol
> *kctl, bool hook_only);
> >   * Helper functions for jack-detection controls
> >   */
> >  struct snd_kcontrol *
> > -snd_kctl_jack_new(const char *name, int idx, void *private_data);
> > +snd_kctl_jack_new(const char *name, struct snd_card *card);
> >  void snd_kctl_jack_report(struct snd_card *card,
> >  			  struct snd_kcontrol *kctl, bool status);
> >
> > diff --git a/sound/core/ctljack.c b/sound/core/ctljack.c index
> > e4b38fb..b631996 100644
> > --- a/sound/core/ctljack.c
> > +++ b/sound/core/ctljack.c
> > @@ -31,15 +31,46 @@ static struct snd_kcontrol_new jack_detect_kctl = {
> >  	.get = jack_detect_kctl_get,
> >  };
> >
> > +static int get_available_index(struct snd_card *card, const char
> > +*name) {
> > +	struct snd_ctl_elem_id sid;
> > +
> > +	memset(&sid, 0, sizeof(sid));
> > +
> > +	sid.index = 0;
> > +	sid.iface = SNDRV_CTL_ELEM_IFACE_CARD;
> > +	strlcpy(sid.name, name, sizeof(sid.name));
> > +
> > +	while (snd_ctl_find_id(card, &sid))
> > +		sid.index++;
> > +
> > +	return sid.index;
> > +}
> > +
> > +static void jack_kctl_name_gen(char *name, const char *src_name, int
> > +size) {
> > +	size_t count = strlen(src_name);
> > +	bool need_cat = true;
> > +
> > +	/* remove redundant " Jack" from src_name */
> > +	if (count >= 5)
> > +		need_cat = strncmp(&src_name[count - 5], " Jack", 5) ? true :
> > +false;
> > +
> > +	snprintf(name, size, need_cat ? "%s Jack" : "%s", src_name);
> > +
> > +}
> > +
> >  struct snd_kcontrol *
> > -snd_kctl_jack_new(const char *name, int idx, void *private_data)
> > +snd_kctl_jack_new(const char *name, struct snd_card *card)
> >  {
> >  	struct snd_kcontrol *kctl;
> > -	kctl = snd_ctl_new1(&jack_detect_kctl, private_data);
> > +
> > +	kctl = snd_ctl_new1(&jack_detect_kctl, card);
> 
> Why changing the private data?  Did it really work...?
 
After this refactoring series, only jack.c:snd_jack_kctl_new() will call this
snd_kctl_jack_new() with struct snd_card * passed in, so I changed it
here, to make it easier for reading.

Here struct snd_card * will be cast to void * in snd_ctl_new1().

Actually, I can keep it as before, then struct snd_card * will be cast to
void * in snd_kctl_jack_new().

Seems no big difference with these two method? If you like, I can change
It back.

~Keyon

> 
> 
> Takashi

Re: [PATCH v8 3/7] ALSA: jack: extend snd_jack_new to support phantom jack

[Jie, Yang]

> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai@suse.de]
> Sent: Wednesday, April 22, 2015 7:31 PM
> To: Jie, Yang
> Cc: broonie@kernel.org; alsa-devel@alsa-project.org; Girdwood, Liam R;
> tanu.kaskinen@linux.intel.com
> Subject: Re: [PATCH v8 3/7] ALSA: jack: extend snd_jack_new to support
> phantom jack
> 
> At Wed, 22 Apr 2015 14:03:49 +0800,
> Jie Yang wrote:
> >
> > diff --git a/sound/core/jack.c b/sound/core/jack.c index
> > b13d0b1..6c729ef 100644
> > --- a/sound/core/jack.c
> > +++ b/sound/core/jack.c
> > @@ -198,6 +198,9 @@ EXPORT_SYMBOL(snd_jack_add_new_kctl);
> >   * @type:  a bitmask of enum snd_jack_type values that can be detected
> by
> >   *         this jack
> >   * @jjack: Used to provide the allocated jack object to the caller.
> > + * @phantom_jack: for phantom jack, only create needed kctl, won't
> create
> > + *         input device
> > + * @initial_kctl: create kctl if true, also add it to the non-phantom
> > + jack kctl list
> 
> Better to align with the actual argument order.
 
OK, will update it, together with that for documentation in patch 7/7.

> 
> >   *
> >   * Creates a new jack object.
> >   *
> > @@ -205,9 +208,10 @@ EXPORT_SYMBOL(snd_jack_add_new_kctl);
> >   * On success @jjack will be initialised.
> >   */
> >  int snd_jack_new(struct snd_card *card, const char *id, int type,
> > -		 struct snd_jack **jjack)
> > +		 struct snd_jack **jjack, bool initial_kctl, bool phantom_jack)
> >  {
> >  	struct snd_jack *jack;
> > +	struct snd_jack_kctl *jack_kctl = NULL;
> >  	int err;
> >  	int i;
> >  	static struct snd_device_ops ops = { @@ -216,26 +220,33 @@ int
> > snd_jack_new(struct snd_card *card, const char *id, int type,
> >  		.dev_disconnect = snd_jack_dev_disconnect,
> >  	};
> >
> > +	if (initial_kctl)
> > +		jack_kctl = snd_jack_kctl_new(card, id, type);
> > +
> 
> The function should return -ENOMEM if snd_jack_kctl_new() fails.
> 
> >  	jack = kzalloc(sizeof(struct snd_jack), GFP_KERNEL);
> >  	if (jack == NULL)
> >  		return -ENOMEM;
> >
> >  	jack->id = kstrdup(id, GFP_KERNEL);
> >
> > -	jack->input_dev = input_allocate_device();
> > -	if (jack->input_dev == NULL) {
> > -		err = -ENOMEM;
> > -		goto fail_input;
> > -	}
> > +	/* don't creat input device for phantom jack */
> > +	if (!phantom_jack) {
> > +		jack->input_dev = input_allocate_device();
> > +		if (jack->input_dev == NULL) {
> > +			err = -ENOMEM;
> > +			goto fail_input;
> > +		}
> >
> > -	jack->input_dev->phys = "ALSA";
> > +		jack->input_dev->phys = "ALSA";
> >
> > -	jack->type = type;
> > +		jack->type = type;
> >
> > -	for (i = 0; i < SND_JACK_SWITCH_TYPES; i++)
> > -		if (type & (1 << i))
> > -			input_set_capability(jack->input_dev, EV_SW,
> > -					     jack_switch_types[i]);
> > +		for (i = 0; i < SND_JACK_SWITCH_TYPES; i++)
> > +			if (type & (1 << i))
> > +				input_set_capability(jack->input_dev,
> EV_SW,
> > +						     jack_switch_types[i]);
> > +
> > +	}
> >
> >  	err = snd_device_new(card, SNDRV_DEV_JACK, jack, &ops);
> >  	if (err < 0)
> > @@ -244,6 +255,9 @@ int snd_jack_new(struct snd_card *card, const char
> *id, int type,
> >  	jack->card = card;
> >  	INIT_LIST_HEAD(&jack->kctl_list);
> >
> > +	if (initial_kctl && jack_kctl)
> 
> ... then here no need to check both.
 
Good, will change it soon.

> 
> 
> Takashi

Re: [PATCH v8 2/7] ALSA: Jack: refactoring snd_kctl_jack_new to support embedded kctl

[Takashi Iwai]

At Wed, 22 Apr 2015 12:40:43 +0000,
Jie, Yang wrote:
> 
> > -----Original Message-----
> > From: Takashi Iwai [mailto:tiwai@suse.de]
> > Sent: Wednesday, April 22, 2015 7:33 PM
> > To: Jie, Yang
> > Cc: broonie@kernel.org; alsa-devel@alsa-project.org; Girdwood, Liam R;
> > tanu.kaskinen@linux.intel.com
> > Subject: Re: [PATCH v8 2/7] ALSA: Jack: refactoring snd_kctl_jack_new to
> > support embedded kctl
> > 
> > At Wed, 22 Apr 2015 14:03:48 +0800,
> > Jie Yang wrote:
> > >
> > > Move available index get part into snd_kctl_jack_new(), also add kctl
> > > name regenerating func to remove redundant " Jack" which is passed in
> > > wrongly in some cases.
> > 
> > The subject doesn't match with this description well.
>  
> How about changing them as below:
> 
> ALSA: Jack: handle jack embedded kcontrol creating within ctljack
> 
> In ctljack.c, add function get_available_index() to allocate
> index for new kcontrol, and jack_kctl_name_gen() to remove
> redundant " Jack" which is passed in wrongly in some cases.
> 
> > 
> > 
> > > Signed-off-by: Jie Yang <yang.jie@intel.com>
> > > ---
> > >  include/sound/control.h  |  2 +-
> > >  sound/core/ctljack.c     | 39 +++++++++++++++++++++++++++++++++++-
> > ---
> > >  sound/core/jack.c        |  2 +-
> > >  sound/pci/hda/hda_jack.c |  2 +-
> > >  4 files changed, 38 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/include/sound/control.h b/include/sound/control.h index
> > > 75f3054..58751a0 100644
> > > --- a/include/sound/control.h
> > > +++ b/include/sound/control.h
> > > @@ -252,7 +252,7 @@ void snd_ctl_sync_vmaster(struct snd_kcontrol
> > *kctl, bool hook_only);
> > >   * Helper functions for jack-detection controls
> > >   */
> > >  struct snd_kcontrol *
> > > -snd_kctl_jack_new(const char *name, int idx, void *private_data);
> > > +snd_kctl_jack_new(const char *name, struct snd_card *card);
> > >  void snd_kctl_jack_report(struct snd_card *card,
> > >  			  struct snd_kcontrol *kctl, bool status);
> > >
> > > diff --git a/sound/core/ctljack.c b/sound/core/ctljack.c index
> > > e4b38fb..b631996 100644
> > > --- a/sound/core/ctljack.c
> > > +++ b/sound/core/ctljack.c
> > > @@ -31,15 +31,46 @@ static struct snd_kcontrol_new jack_detect_kctl = {
> > >  	.get = jack_detect_kctl_get,
> > >  };
> > >
> > > +static int get_available_index(struct snd_card *card, const char
> > > +*name) {
> > > +	struct snd_ctl_elem_id sid;
> > > +
> > > +	memset(&sid, 0, sizeof(sid));
> > > +
> > > +	sid.index = 0;
> > > +	sid.iface = SNDRV_CTL_ELEM_IFACE_CARD;
> > > +	strlcpy(sid.name, name, sizeof(sid.name));
> > > +
> > > +	while (snd_ctl_find_id(card, &sid))
> > > +		sid.index++;
> > > +
> > > +	return sid.index;
> > > +}
> > > +
> > > +static void jack_kctl_name_gen(char *name, const char *src_name, int
> > > +size) {
> > > +	size_t count = strlen(src_name);
> > > +	bool need_cat = true;
> > > +
> > > +	/* remove redundant " Jack" from src_name */
> > > +	if (count >= 5)
> > > +		need_cat = strncmp(&src_name[count - 5], " Jack", 5) ? true :
> > > +false;
> > > +
> > > +	snprintf(name, size, need_cat ? "%s Jack" : "%s", src_name);
> > > +
> > > +}
> > > +
> > >  struct snd_kcontrol *
> > > -snd_kctl_jack_new(const char *name, int idx, void *private_data)
> > > +snd_kctl_jack_new(const char *name, struct snd_card *card)
> > >  {
> > >  	struct snd_kcontrol *kctl;
> > > -	kctl = snd_ctl_new1(&jack_detect_kctl, private_data);
> > > +
> > > +	kctl = snd_ctl_new1(&jack_detect_kctl, card);
> > 
> > Why changing the private data?  Did it really work...?
>  
> After this refactoring series, only jack.c:snd_jack_kctl_new() will call this
> snd_kctl_jack_new() with struct snd_card * passed in, so I changed it
> here, to make it easier for reading.
> 
> Here struct snd_card * will be cast to void * in snd_ctl_new1().
> 
> Actually, I can keep it as before, then struct snd_card * will be cast to
> void * in snd_kctl_jack_new().
> 
> Seems no big difference with these two method? If you like, I can change
> It back.

You break the bisectionability in your series.

Try to build and test at *each* commit whether it works as expected.
Since snd_kctl_jack_new() is still called from hda_jack.c at this
commit, it'll certainly crash when the jack is actually handled.
Or, maybe it's even before that -- it may result in double jacks.

Especially test with several kconfigs to build (with and without
CONFIG_SND_HDA_INPUT_JACK and CONFIG_SND_JACK), and test with the
actual HD-audio driver.

Keeping things working isn't an easy task, but it's often worthwhile.
Once when you have done, you have idea how to manage a patch series.


Takashi

Re: [PATCH v8 2/7] ALSA: Jack: refactoring snd_kctl_jack_new to support embedded kctl

[Jie, Yang]

> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai@suse.de]
> Sent: Wednesday, April 22, 2015 8:58 PM
> To: Jie, Yang
> Cc: broonie@kernel.org; alsa-devel@alsa-project.org; Girdwood, Liam R;
> tanu.kaskinen@linux.intel.com
> Subject: Re: [PATCH v8 2/7] ALSA: Jack: refactoring snd_kctl_jack_new to
> support embedded kctl
> 
> At Wed, 22 Apr 2015 12:40:43 +0000,
> Jie, Yang wrote:
> >
> > > -----Original Message-----
> > > From: Takashi Iwai [mailto:tiwai@suse.de]
> > > Sent: Wednesday, April 22, 2015 7:33 PM
> > > To: Jie, Yang
> > > Cc: broonie@kernel.org; alsa-devel@alsa-project.org; Girdwood, Liam
> > > R; tanu.kaskinen@linux.intel.com
> > > Subject: Re: [PATCH v8 2/7] ALSA: Jack: refactoring
> > > snd_kctl_jack_new to support embedded kctl
> > >
> > > At Wed, 22 Apr 2015 14:03:48 +0800,
> > > Jie Yang wrote:
> > > >
> > > > Move available index get part into snd_kctl_jack_new(), also add
> > > > kctl name regenerating func to remove redundant " Jack" which is
> > > > passed in wrongly in some cases.
> > >
> > > The subject doesn't match with this description well.
> >
> > How about changing them as below:
> >
> > ALSA: Jack: handle jack embedded kcontrol creating within ctljack
> >
> > In ctljack.c, add function get_available_index() to allocate index for
> > new kcontrol, and jack_kctl_name_gen() to remove redundant " Jack"
> > which is passed in wrongly in some cases.
> >
> > >
> > >
> > > > Signed-off-by: Jie Yang <yang.jie@intel.com>
> > > > ---
> > > >  include/sound/control.h  |  2 +-
> > > >  sound/core/ctljack.c     | 39
> +++++++++++++++++++++++++++++++++++-
> > > ---
> > > >  sound/core/jack.c        |  2 +-
> > > >  sound/pci/hda/hda_jack.c |  2 +-
> > > >  4 files changed, 38 insertions(+), 7 deletions(-)
> > > >
> > > > diff --git a/include/sound/control.h b/include/sound/control.h
> > > > index
> > > > 75f3054..58751a0 100644
> > > > --- a/include/sound/control.h
> > > > +++ b/include/sound/control.h
> > > > @@ -252,7 +252,7 @@ void snd_ctl_sync_vmaster(struct snd_kcontrol
> > > *kctl, bool hook_only);
> > > >   * Helper functions for jack-detection controls
> > > >   */
> > > >  struct snd_kcontrol *
> > > > -snd_kctl_jack_new(const char *name, int idx, void *private_data);
> > > > +snd_kctl_jack_new(const char *name, struct snd_card *card);
> > > >  void snd_kctl_jack_report(struct snd_card *card,
> > > >  			  struct snd_kcontrol *kctl, bool status);
> > > >
> > > > diff --git a/sound/core/ctljack.c b/sound/core/ctljack.c index
> > > > e4b38fb..b631996 100644
> > > > --- a/sound/core/ctljack.c
> > > > +++ b/sound/core/ctljack.c
> > > > @@ -31,15 +31,46 @@ static struct snd_kcontrol_new jack_detect_kctl
> = {
> > > >  	.get = jack_detect_kctl_get,
> > > >  };
> > > >
> > > > +static int get_available_index(struct snd_card *card, const char
> > > > +*name) {
> > > > +	struct snd_ctl_elem_id sid;
> > > > +
> > > > +	memset(&sid, 0, sizeof(sid));
> > > > +
> > > > +	sid.index = 0;
> > > > +	sid.iface = SNDRV_CTL_ELEM_IFACE_CARD;
> > > > +	strlcpy(sid.name, name, sizeof(sid.name));
> > > > +
> > > > +	while (snd_ctl_find_id(card, &sid))
> > > > +		sid.index++;
> > > > +
> > > > +	return sid.index;
> > > > +}
> > > > +
> > > > +static void jack_kctl_name_gen(char *name, const char *src_name,
> > > > +int
> > > > +size) {
> > > > +	size_t count = strlen(src_name);
> > > > +	bool need_cat = true;
> > > > +
> > > > +	/* remove redundant " Jack" from src_name */
> > > > +	if (count >= 5)
> > > > +		need_cat = strncmp(&src_name[count - 5], " Jack", 5) ? true :
> > > > +false;
> > > > +
> > > > +	snprintf(name, size, need_cat ? "%s Jack" : "%s", src_name);
> > > > +
> > > > +}
> > > > +
> > > >  struct snd_kcontrol *
> > > > -snd_kctl_jack_new(const char *name, int idx, void *private_data)
> > > > +snd_kctl_jack_new(const char *name, struct snd_card *card)
> > > >  {
> > > >  	struct snd_kcontrol *kctl;
> > > > -	kctl = snd_ctl_new1(&jack_detect_kctl, private_data);
> > > > +
> > > > +	kctl = snd_ctl_new1(&jack_detect_kctl, card);
> > >
> > > Why changing the private data?  Did it really work...?
> >
> > After this refactoring series, only jack.c:snd_jack_kctl_new() will
> > call this
> > snd_kctl_jack_new() with struct snd_card * passed in, so I changed it
> > here, to make it easier for reading.
> >
> > Here struct snd_card * will be cast to void * in snd_ctl_new1().
> >
> > Actually, I can keep it as before, then struct snd_card * will be cast
> > to void * in snd_kctl_jack_new().
> >
> > Seems no big difference with these two method? If you like, I can
> > change It back.
> 
> You break the bisectionability in your series.
> 
> Try to build and test at *each* commit whether it works as expected.
> Since snd_kctl_jack_new() is still called from hda_jack.c at this commit, it'll
> certainly crash when the jack is actually handled.
> Or, maybe it's even before that -- it may result in double jacks.
 
Thanks for pointing out this, cast codec to card is really a risk here, will change
it back.

> 
> Especially test with several kconfigs to build (with and without
> CONFIG_SND_HDA_INPUT_JACK and CONFIG_SND_JACK), and test with the
> actual HD-audio driver.

Um, I tested the building for each commit with, but not run for each. Will notice
it next time.

> 
> Keeping things working isn't an easy task, but it's often worthwhile.
> Once when you have done, you have idea how to manage a patch series.
 
That's true. Thanks for guiding patiently. :)

> 
> 
> Takashi

Re: [PATCH v8 2/7] ALSA: Jack: refactoring snd_kctl_jack_new to support embedded kctl

[Jie, Yang]

> -----Original Message-----
> From: Jie, Yang
> Sent: Wednesday, April 22, 2015 9:23 PM
> To: 'Takashi Iwai'
> Cc: broonie@kernel.org; alsa-devel@alsa-project.org; Girdwood, Liam R;
> tanu.kaskinen@linux.intel.com
> Subject: RE: [PATCH v8 2/7] ALSA: Jack: refactoring snd_kctl_jack_new to
> support embedded kctl
> 
> > -----Original Message-----
> > From: Takashi Iwai [mailto:tiwai@suse.de]
> > Sent: Wednesday, April 22, 2015 8:58 PM
> > To: Jie, Yang
> > Cc: broonie@kernel.org; alsa-devel@alsa-project.org; Girdwood, Liam R;
> > tanu.kaskinen@linux.intel.com
> > Subject: Re: [PATCH v8 2/7] ALSA: Jack: refactoring snd_kctl_jack_new
> > to support embedded kctl
> >
> > At Wed, 22 Apr 2015 12:40:43 +0000,
> > Jie, Yang wrote:
> > >
> > > > -----Original Message-----
> > > > From: Takashi Iwai [mailto:tiwai@suse.de]
> > > > Sent: Wednesday, April 22, 2015 7:33 PM
> > > > To: Jie, Yang
> > > > Cc: broonie@kernel.org; alsa-devel@alsa-project.org; Girdwood,
> > > > Liam R; tanu.kaskinen@linux.intel.com
> > > > Subject: Re: [PATCH v8 2/7] ALSA: Jack: refactoring
> > > > snd_kctl_jack_new to support embedded kctl
> > > >
> > > > At Wed, 22 Apr 2015 14:03:48 +0800, Jie Yang wrote:
> > > > >
> > > > > Move available index get part into snd_kctl_jack_new(), also add
> > > > > kctl name regenerating func to remove redundant " Jack" which is
> > > > > passed in wrongly in some cases.
> > > >
> > > > The subject doesn't match with this description well.
> > >
> > > How about changing them as below:
> > >
> > > ALSA: Jack: handle jack embedded kcontrol creating within ctljack
> > >
> > > In ctljack.c, add function get_available_index() to allocate index
> > > for new kcontrol, and jack_kctl_name_gen() to remove redundant "
> Jack"
> > > which is passed in wrongly in some cases.
> > >
> > > >
> > > >
> > > > > Signed-off-by: Jie Yang <yang.jie@intel.com>
> > > > > ---
> > > > >  include/sound/control.h  |  2 +-
> > > > >  sound/core/ctljack.c     | 39
> > +++++++++++++++++++++++++++++++++++-
> > > > ---
> > > > >  sound/core/jack.c        |  2 +-
> > > > >  sound/pci/hda/hda_jack.c |  2 +-
> > > > >  4 files changed, 38 insertions(+), 7 deletions(-)
> > > > >
> > > > > diff --git a/include/sound/control.h b/include/sound/control.h
> > > > > index
> > > > > 75f3054..58751a0 100644
> > > > > --- a/include/sound/control.h
> > > > > +++ b/include/sound/control.h
> > > > > @@ -252,7 +252,7 @@ void snd_ctl_sync_vmaster(struct
> > > > > snd_kcontrol
> > > > *kctl, bool hook_only);
> > > > >   * Helper functions for jack-detection controls
> > > > >   */
> > > > >  struct snd_kcontrol *
> > > > > -snd_kctl_jack_new(const char *name, int idx, void
> > > > > *private_data);
> > > > > +snd_kctl_jack_new(const char *name, struct snd_card *card);
> > > > >  void snd_kctl_jack_report(struct snd_card *card,
> > > > >  			  struct snd_kcontrol *kctl, bool status);
> > > > >
> > > > > diff --git a/sound/core/ctljack.c b/sound/core/ctljack.c index
> > > > > e4b38fb..b631996 100644
> > > > > --- a/sound/core/ctljack.c
> > > > > +++ b/sound/core/ctljack.c
> > > > > @@ -31,15 +31,46 @@ static struct snd_kcontrol_new
> > > > > jack_detect_kctl
> > = {
> > > > >  	.get = jack_detect_kctl_get,
> > > > >  };
> > > > >
> > > > > +static int get_available_index(struct snd_card *card, const
> > > > > +char
> > > > > +*name) {
> > > > > +	struct snd_ctl_elem_id sid;
> > > > > +
> > > > > +	memset(&sid, 0, sizeof(sid));
> > > > > +
> > > > > +	sid.index = 0;
> > > > > +	sid.iface = SNDRV_CTL_ELEM_IFACE_CARD;
> > > > > +	strlcpy(sid.name, name, sizeof(sid.name));
> > > > > +
> > > > > +	while (snd_ctl_find_id(card, &sid))
> > > > > +		sid.index++;
> > > > > +
> > > > > +	return sid.index;
> > > > > +}
> > > > > +
> > > > > +static void jack_kctl_name_gen(char *name, const char
> > > > > +*src_name, int
> > > > > +size) {
> > > > > +	size_t count = strlen(src_name);
> > > > > +	bool need_cat = true;
> > > > > +
> > > > > +	/* remove redundant " Jack" from src_name */
> > > > > +	if (count >= 5)
> > > > > +		need_cat = strncmp(&src_name[count - 5], " Jack",
> 5) ? true :
> > > > > +false;
> > > > > +
> > > > > +	snprintf(name, size, need_cat ? "%s Jack" : "%s", src_name);
> > > > > +
> > > > > +}
> > > > > +
> > > > >  struct snd_kcontrol *
> > > > > -snd_kctl_jack_new(const char *name, int idx, void
> > > > > *private_data)
> > > > > +snd_kctl_jack_new(const char *name, struct snd_card *card)
> > > > >  {
> > > > >  	struct snd_kcontrol *kctl;
> > > > > -	kctl = snd_ctl_new1(&jack_detect_kctl, private_data);
> > > > > +
> > > > > +	kctl = snd_ctl_new1(&jack_detect_kctl, card);
> > > >
> > > > Why changing the private data?  Did it really work...?
 

Hi, Takashi, I recalled that why I need change it to 'card' here: we need
it in the new added function get_available_index().

when I look more into snd_ctl_new1() and the private_data
usage in hda_jack.c(private_data is not really used there),  so, I prefer
to keep this change, and change the calling in hda_jack.c as below:

-	kctl = snd_kctl_jack_new(name, codec);
+ 	kctl = snd_kctl_jack_new(name, codec->bus->card);

This should fix both issues.

What's your opinion on it?

> > >
> > > After this refactoring series, only jack.c:snd_jack_kctl_new() will
> > > call this
> > > snd_kctl_jack_new() with struct snd_card * passed in, so I changed
> > > it here, to make it easier for reading.
> > >
> > > Here struct snd_card * will be cast to void * in snd_ctl_new1().
> > >
> > > Actually, I can keep it as before, then struct snd_card * will be
> > > cast to void * in snd_kctl_jack_new().
> > >
> > > Seems no big difference with these two method? If you like, I can
> > > change It back.
> >
> > You break the bisectionability in your series.
> >
> > Try to build and test at *each* commit whether it works as expected.
> > Since snd_kctl_jack_new() is still called from hda_jack.c at this
> > commit, it'll certainly crash when the jack is actually handled.
> > Or, maybe it's even before that -- it may result in double jacks.
> 
> Thanks for pointing out this, cast codec to card is really a risk here, will change
> it back.
> 
> >
> > Especially test with several kconfigs to build (with and without
> > CONFIG_SND_HDA_INPUT_JACK and CONFIG_SND_JACK), and test with
> the
> > actual HD-audio driver.
> 
> Um, I tested the building for each commit with, but not run for each. Will
> notice it next time.
> 
> >
> > Keeping things working isn't an easy task, but it's often worthwhile.
> > Once when you have done, you have idea how to manage a patch series.
> 
> That's true. Thanks for guiding patiently. :)
> 
> >
> >
> > Takashi

Re: [PATCH v8 2/7] ALSA: Jack: refactoring snd_kctl_jack_new to support embedded kctl

[Takashi Iwai]

At Wed, 22 Apr 2015 13:56:25 +0000,
Jie, Yang wrote:
> 
> > -----Original Message-----
> > From: Jie, Yang
> > Sent: Wednesday, April 22, 2015 9:23 PM
> > To: 'Takashi Iwai'
> > Cc: broonie@kernel.org; alsa-devel@alsa-project.org; Girdwood, Liam R;
> > tanu.kaskinen@linux.intel.com
> > Subject: RE: [PATCH v8 2/7] ALSA: Jack: refactoring snd_kctl_jack_new to
> > support embedded kctl
> > 
> > > -----Original Message-----
> > > From: Takashi Iwai [mailto:tiwai@suse.de]
> > > Sent: Wednesday, April 22, 2015 8:58 PM
> > > To: Jie, Yang
> > > Cc: broonie@kernel.org; alsa-devel@alsa-project.org; Girdwood, Liam R;
> > > tanu.kaskinen@linux.intel.com
> > > Subject: Re: [PATCH v8 2/7] ALSA: Jack: refactoring snd_kctl_jack_new
> > > to support embedded kctl
> > >
> > > At Wed, 22 Apr 2015 12:40:43 +0000,
> > > Jie, Yang wrote:
> > > >
> > > > > -----Original Message-----
> > > > > From: Takashi Iwai [mailto:tiwai@suse.de]
> > > > > Sent: Wednesday, April 22, 2015 7:33 PM
> > > > > To: Jie, Yang
> > > > > Cc: broonie@kernel.org; alsa-devel@alsa-project.org; Girdwood,
> > > > > Liam R; tanu.kaskinen@linux.intel.com
> > > > > Subject: Re: [PATCH v8 2/7] ALSA: Jack: refactoring
> > > > > snd_kctl_jack_new to support embedded kctl
> > > > >
> > > > > At Wed, 22 Apr 2015 14:03:48 +0800, Jie Yang wrote:
> > > > > >
> > > > > > Move available index get part into snd_kctl_jack_new(), also add
> > > > > > kctl name regenerating func to remove redundant " Jack" which is
> > > > > > passed in wrongly in some cases.
> > > > >
> > > > > The subject doesn't match with this description well.
> > > >
> > > > How about changing them as below:
> > > >
> > > > ALSA: Jack: handle jack embedded kcontrol creating within ctljack
> > > >
> > > > In ctljack.c, add function get_available_index() to allocate index
> > > > for new kcontrol, and jack_kctl_name_gen() to remove redundant "
> > Jack"
> > > > which is passed in wrongly in some cases.
> > > >
> > > > >
> > > > >
> > > > > > Signed-off-by: Jie Yang <yang.jie@intel.com>
> > > > > > ---
> > > > > >  include/sound/control.h  |  2 +-
> > > > > >  sound/core/ctljack.c     | 39
> > > +++++++++++++++++++++++++++++++++++-
> > > > > ---
> > > > > >  sound/core/jack.c        |  2 +-
> > > > > >  sound/pci/hda/hda_jack.c |  2 +-
> > > > > >  4 files changed, 38 insertions(+), 7 deletions(-)
> > > > > >
> > > > > > diff --git a/include/sound/control.h b/include/sound/control.h
> > > > > > index
> > > > > > 75f3054..58751a0 100644
> > > > > > --- a/include/sound/control.h
> > > > > > +++ b/include/sound/control.h
> > > > > > @@ -252,7 +252,7 @@ void snd_ctl_sync_vmaster(struct
> > > > > > snd_kcontrol
> > > > > *kctl, bool hook_only);
> > > > > >   * Helper functions for jack-detection controls
> > > > > >   */
> > > > > >  struct snd_kcontrol *
> > > > > > -snd_kctl_jack_new(const char *name, int idx, void
> > > > > > *private_data);
> > > > > > +snd_kctl_jack_new(const char *name, struct snd_card *card);
> > > > > >  void snd_kctl_jack_report(struct snd_card *card,
> > > > > >  			  struct snd_kcontrol *kctl, bool status);
> > > > > >
> > > > > > diff --git a/sound/core/ctljack.c b/sound/core/ctljack.c index
> > > > > > e4b38fb..b631996 100644
> > > > > > --- a/sound/core/ctljack.c
> > > > > > +++ b/sound/core/ctljack.c
> > > > > > @@ -31,15 +31,46 @@ static struct snd_kcontrol_new
> > > > > > jack_detect_kctl
> > > = {
> > > > > >  	.get = jack_detect_kctl_get,
> > > > > >  };
> > > > > >
> > > > > > +static int get_available_index(struct snd_card *card, const
> > > > > > +char
> > > > > > +*name) {
> > > > > > +	struct snd_ctl_elem_id sid;
> > > > > > +
> > > > > > +	memset(&sid, 0, sizeof(sid));
> > > > > > +
> > > > > > +	sid.index = 0;
> > > > > > +	sid.iface = SNDRV_CTL_ELEM_IFACE_CARD;
> > > > > > +	strlcpy(sid.name, name, sizeof(sid.name));
> > > > > > +
> > > > > > +	while (snd_ctl_find_id(card, &sid))
> > > > > > +		sid.index++;
> > > > > > +
> > > > > > +	return sid.index;
> > > > > > +}
> > > > > > +
> > > > > > +static void jack_kctl_name_gen(char *name, const char
> > > > > > +*src_name, int
> > > > > > +size) {
> > > > > > +	size_t count = strlen(src_name);
> > > > > > +	bool need_cat = true;
> > > > > > +
> > > > > > +	/* remove redundant " Jack" from src_name */
> > > > > > +	if (count >= 5)
> > > > > > +		need_cat = strncmp(&src_name[count - 5], " Jack",
> > 5) ? true :
> > > > > > +false;
> > > > > > +
> > > > > > +	snprintf(name, size, need_cat ? "%s Jack" : "%s", src_name);
> > > > > > +
> > > > > > +}
> > > > > > +
> > > > > >  struct snd_kcontrol *
> > > > > > -snd_kctl_jack_new(const char *name, int idx, void
> > > > > > *private_data)
> > > > > > +snd_kctl_jack_new(const char *name, struct snd_card *card)
> > > > > >  {
> > > > > >  	struct snd_kcontrol *kctl;
> > > > > > -	kctl = snd_ctl_new1(&jack_detect_kctl, private_data);
> > > > > > +
> > > > > > +	kctl = snd_ctl_new1(&jack_detect_kctl, card);
> > > > >
> > > > > Why changing the private data?  Did it really work...?
>  
> 
> Hi, Takashi, I recalled that why I need change it to 'card' here: we need
> it in the new added function get_available_index().
> 
> when I look more into snd_ctl_new1() and the private_data
> usage in hda_jack.c(private_data is not really used there),  so, I prefer
> to keep this change, and change the calling in hda_jack.c as below:
> 
> -	kctl = snd_kctl_jack_new(name, codec);
> + 	kctl = snd_kctl_jack_new(name, codec->bus->card);
> 
> This should fix both issues.

Rather change the function to take both card and private_data
arguments temporarily.  Later on, you can drop private_data argument
once when the code is detached from hda.


Takashi

Re: [PATCH v8 2/7] ALSA: Jack: refactoring snd_kctl_jack_new to support embedded kctl

[Jie, Yang]

> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai@suse.de]
> Sent: Wednesday, April 22, 2015 10:09 PM
> To: Jie, Yang
> Cc: broonie@kernel.org; alsa-devel@alsa-project.org; Girdwood, Liam R;
> tanu.kaskinen@linux.intel.com
> Subject: Re: [PATCH v8 2/7] ALSA: Jack: refactoring snd_kctl_jack_new to
> support embedded kctl
> 
> At Wed, 22 Apr 2015 13:56:25 +0000,
> Jie, Yang wrote:
> >
> > > -----Original Message-----
> > > From: Jie, Yang
> > > Sent: Wednesday, April 22, 2015 9:23 PM
> > > To: 'Takashi Iwai'
> > > Cc: broonie@kernel.org; alsa-devel@alsa-project.org; Girdwood, Liam
> > > R; tanu.kaskinen@linux.intel.com
> > > Subject: RE: [PATCH v8 2/7] ALSA: Jack: refactoring
> > > snd_kctl_jack_new to support embedded kctl
> > >
> > > > -----Original Message-----
> > > > From: Takashi Iwai [mailto:tiwai@suse.de]
> > > > Sent: Wednesday, April 22, 2015 8:58 PM
> > > > To: Jie, Yang
> > > > Cc: broonie@kernel.org; alsa-devel@alsa-project.org; Girdwood,
> > > > Liam R; tanu.kaskinen@linux.intel.com
> > > > Subject: Re: [PATCH v8 2/7] ALSA: Jack: refactoring
> > > > snd_kctl_jack_new to support embedded kctl
> > > >
> > > > At Wed, 22 Apr 2015 12:40:43 +0000, Jie, Yang wrote:
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Takashi Iwai [mailto:tiwai@suse.de]
> > > > > > Sent: Wednesday, April 22, 2015 7:33 PM
> > > > > > To: Jie, Yang
> > > > > > Cc: broonie@kernel.org; alsa-devel@alsa-project.org; Girdwood,
> > > > > > Liam R; tanu.kaskinen@linux.intel.com
> > > > > > Subject: Re: [PATCH v8 2/7] ALSA: Jack: refactoring
> > > > > > snd_kctl_jack_new to support embedded kctl
> > > > > >
> > > > > > At Wed, 22 Apr 2015 14:03:48 +0800, Jie Yang wrote:
> > > > > > >
> > > > > > > Move available index get part into snd_kctl_jack_new(), also
> > > > > > > add kctl name regenerating func to remove redundant " Jack"
> > > > > > > which is passed in wrongly in some cases.
> > > > > >
> > > > > > The subject doesn't match with this description well.
> > > > >
> > > > > How about changing them as below:
> > > > >
> > > > > ALSA: Jack: handle jack embedded kcontrol creating within
> > > > > ctljack
> > > > >
> > > > > In ctljack.c, add function get_available_index() to allocate
> > > > > index for new kcontrol, and jack_kctl_name_gen() to remove
> redundant "
> > > Jack"
> > > > > which is passed in wrongly in some cases.
> > > > >
> > > > > >
> > > > > >
> > > > > > > Signed-off-by: Jie Yang <yang.jie@intel.com>
> > > > > > > ---
> > > > > > >  include/sound/control.h  |  2 +-
> > > > > > >  sound/core/ctljack.c     | 39
> > > > +++++++++++++++++++++++++++++++++++-
> > > > > > ---
> > > > > > >  sound/core/jack.c        |  2 +-
> > > > > > >  sound/pci/hda/hda_jack.c |  2 +-
> > > > > > >  4 files changed, 38 insertions(+), 7 deletions(-)
> > > > > > >
> > > > > > > diff --git a/include/sound/control.h
> > > > > > > b/include/sound/control.h index
> > > > > > > 75f3054..58751a0 100644
> > > > > > > --- a/include/sound/control.h
> > > > > > > +++ b/include/sound/control.h
> > > > > > > @@ -252,7 +252,7 @@ void snd_ctl_sync_vmaster(struct
> > > > > > > snd_kcontrol
> > > > > > *kctl, bool hook_only);
> > > > > > >   * Helper functions for jack-detection controls
> > > > > > >   */
> > > > > > >  struct snd_kcontrol *
> > > > > > > -snd_kctl_jack_new(const char *name, int idx, void
> > > > > > > *private_data);
> > > > > > > +snd_kctl_jack_new(const char *name, struct snd_card *card);
> > > > > > >  void snd_kctl_jack_report(struct snd_card *card,
> > > > > > >  			  struct snd_kcontrol *kctl, bool status);
> > > > > > >
> > > > > > > diff --git a/sound/core/ctljack.c b/sound/core/ctljack.c
> > > > > > > index
> > > > > > > e4b38fb..b631996 100644
> > > > > > > --- a/sound/core/ctljack.c
> > > > > > > +++ b/sound/core/ctljack.c
> > > > > > > @@ -31,15 +31,46 @@ static struct snd_kcontrol_new
> > > > > > > jack_detect_kctl
> > > > = {
> > > > > > >  	.get = jack_detect_kctl_get,  };
> > > > > > >
> > > > > > > +static int get_available_index(struct snd_card *card, const
> > > > > > > +char
> > > > > > > +*name) {
> > > > > > > +	struct snd_ctl_elem_id sid;
> > > > > > > +
> > > > > > > +	memset(&sid, 0, sizeof(sid));
> > > > > > > +
> > > > > > > +	sid.index = 0;
> > > > > > > +	sid.iface = SNDRV_CTL_ELEM_IFACE_CARD;
> > > > > > > +	strlcpy(sid.name, name, sizeof(sid.name));
> > > > > > > +
> > > > > > > +	while (snd_ctl_find_id(card, &sid))
> > > > > > > +		sid.index++;
> > > > > > > +
> > > > > > > +	return sid.index;
> > > > > > > +}
> > > > > > > +
> > > > > > > +static void jack_kctl_name_gen(char *name, const char
> > > > > > > +*src_name, int
> > > > > > > +size) {
> > > > > > > +	size_t count = strlen(src_name);
> > > > > > > +	bool need_cat = true;
> > > > > > > +
> > > > > > > +	/* remove redundant " Jack" from src_name */
> > > > > > > +	if (count >= 5)
> > > > > > > +		need_cat = strncmp(&src_name[count - 5], " Jack",
> > > 5) ? true :
> > > > > > > +false;
> > > > > > > +
> > > > > > > +	snprintf(name, size, need_cat ? "%s Jack" : "%s",
> > > > > > > +src_name);
> > > > > > > +
> > > > > > > +}
> > > > > > > +
> > > > > > >  struct snd_kcontrol *
> > > > > > > -snd_kctl_jack_new(const char *name, int idx, void
> > > > > > > *private_data)
> > > > > > > +snd_kctl_jack_new(const char *name, struct snd_card *card)
> > > > > > >  {
> > > > > > >  	struct snd_kcontrol *kctl;
> > > > > > > -	kctl = snd_ctl_new1(&jack_detect_kctl, private_data);
> > > > > > > +
> > > > > > > +	kctl = snd_ctl_new1(&jack_detect_kctl, card);
> > > > > >
> > > > > > Why changing the private data?  Did it really work...?
> >
> >
> > Hi, Takashi, I recalled that why I need change it to 'card' here: we
> > need it in the new added function get_available_index().
> >
> > when I look more into snd_ctl_new1() and the private_data usage in
> > hda_jack.c(private_data is not really used there),  so, I prefer to
> > keep this change, and change the calling in hda_jack.c as below:
> >
> > -	kctl = snd_kctl_jack_new(name, codec);
> > + 	kctl = snd_kctl_jack_new(name, codec->bus->card);
> >
> > This should fix both issues.
> 
> Rather change the function to take both card and private_data arguments
> temporarily.  Later on, you can drop private_data argument once when the
> code is detached from hda.

OK, will do like that.

> 
> 
> Takashi