[PATCH RFC 0/7] Allow multiple callbacks for hda_jack

[PATCH RFC 0/7] Allow multiple callbacks for hda_jack

[Takashi Iwai]

Hi,

this is a series of patches I quickly cooked up after the discussion
in this morning: the support of multiple callbacks per jack.

The series is applied on top of the previous fix patch (ALSA: hda -
Fix invalid pin powermap without jack detection).  It begins with
a couple of cleanups, then introduces the new hda_jack_callback
struct and the changes along with it, then ends with another
couple of cleanup patches based on the new infrastructure.

I've tested only with a small set of devices, so far.


Takashi

[PATCH RFC 1/7] ALSA: hda - Get rid of action field from struct hda_jack_tbl

[Takashi Iwai]

The action value assigned to each hda_jack_tbl entry is mostly
superfluous.  The actually used values are either the widget NID or a
value specific to the callback.

The former case can be simply replaced by a reference to widget NID
itself.  The only place doing the latter is STAC/IDT codec driver for
the powermap handling.  But, the code doesn't need to check the action
field at all -- the function jack_update_power() is called either with
a specific pin or with NULL.  So the check of jack->action can be
removed completely there, too.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/pci/hda/hda_generic.c    |  4 +---
 sound/pci/hda/hda_generic.h    |  6 ------
 sound/pci/hda/hda_jack.c       | 10 +++-------
 sound/pci/hda/hda_jack.h       | 22 +---------------------
 sound/pci/hda/patch_ca0132.c   | 16 +++++++++-------
 sound/pci/hda/patch_cirrus.c   |  3 ---
 sound/pci/hda/patch_hdmi.c     |  4 ++--
 sound/pci/hda/patch_realtek.c  |  9 +++------
 sound/pci/hda/patch_sigmatel.c | 18 ++++--------------
 sound/pci/hda/patch_via.c      |  4 ----
 10 files changed, 23 insertions(+), 73 deletions(-)

diff --git a/sound/pci/hda/hda_generic.c b/sound/pci/hda/hda_generic.c
index 95121e818b4d..4d605e4ac41c 100644
--- a/sound/pci/hda/hda_generic.c
+++ b/sound/pci/hda/hda_generic.c
@@ -4180,7 +4180,7 @@ static int check_auto_mute_availability(struct hda_codec *codec)
 		if (!is_jack_detectable(codec, nid))
 			continue;
 		codec_dbg(codec, "Enable HP auto-muting on NID 0x%x\n", nid);
-		snd_hda_jack_detect_enable_callback(codec, nid, HDA_GEN_HP_EVENT,
+		snd_hda_jack_detect_enable_callback(codec, nid,
 						    call_hp_automute);
 		spec->detect_hp = 1;
 	}
@@ -4193,7 +4193,6 @@ static int check_auto_mute_availability(struct hda_codec *codec)
 					continue;
 				codec_dbg(codec, "Enable Line-Out auto-muting on NID 0x%x\n", nid);
 				snd_hda_jack_detect_enable_callback(codec, nid,
-								    HDA_GEN_FRONT_EVENT,
 								    call_line_automute);
 				spec->detect_lo = 1;
 			}
@@ -4235,7 +4234,6 @@ static bool auto_mic_check_imux(struct hda_codec *codec)
 	for (i = 1; i < spec->am_num_entries; i++)
 		snd_hda_jack_detect_enable_callback(codec,
 						    spec->am_entry[i].pin,
-						    HDA_GEN_MIC_EVENT,
 						    call_mic_autoswitch);
 	return true;
 }
diff --git a/sound/pci/hda/hda_generic.h b/sound/pci/hda/hda_generic.h
index 3f95f1d3f1f8..72f5624125fb 100644
--- a/sound/pci/hda/hda_generic.h
+++ b/sound/pci/hda/hda_generic.h
@@ -12,12 +12,6 @@
 #ifndef __SOUND_HDA_GENERIC_H
 #define __SOUND_HDA_GENERIC_H
 
-/* unsol event tags */
-enum {
-	HDA_GEN_HP_EVENT = 1, HDA_GEN_FRONT_EVENT, HDA_GEN_MIC_EVENT,
-	HDA_GEN_LAST_EVENT = HDA_GEN_MIC_EVENT
-};
-
 /* table entry for multi-io paths */
 struct hda_multi_io {
 	hda_nid_t pin;		/* multi-io widget pin NID */
diff --git a/sound/pci/hda/hda_jack.c b/sound/pci/hda/hda_jack.c
index 9746d73cec52..9c8f24f2d56b 100644
--- a/sound/pci/hda/hda_jack.c
+++ b/sound/pci/hda/hda_jack.c
@@ -217,7 +217,6 @@ EXPORT_SYMBOL_GPL(snd_hda_jack_detect_state);
  * snd_hda_jack_detect_enable - enable the jack-detection
  */
 int snd_hda_jack_detect_enable_callback(struct hda_codec *codec, hda_nid_t nid,
-					unsigned char action,
 					hda_jack_callback cb)
 {
 	struct hda_jack_tbl *jack = snd_hda_jack_tbl_new(codec, nid);
@@ -226,8 +225,6 @@ int snd_hda_jack_detect_enable_callback(struct hda_codec *codec, hda_nid_t nid,
 	if (jack->jack_detect)
 		return 0; /* already registered */
 	jack->jack_detect = 1;
-	if (action)
-		jack->action = action;
 	if (cb)
 		jack->callback = cb;
 	if (codec->jackpoll_interval > 0)
@@ -238,10 +235,9 @@ int snd_hda_jack_detect_enable_callback(struct hda_codec *codec, hda_nid_t nid,
 }
 EXPORT_SYMBOL_GPL(snd_hda_jack_detect_enable_callback);
 
-int snd_hda_jack_detect_enable(struct hda_codec *codec, hda_nid_t nid,
-			       unsigned char action)
+int snd_hda_jack_detect_enable(struct hda_codec *codec, hda_nid_t nid)
 {
-	return snd_hda_jack_detect_enable_callback(codec, nid, action, NULL);
+	return snd_hda_jack_detect_enable_callback(codec, nid, NULL);
 }
 EXPORT_SYMBOL_GPL(snd_hda_jack_detect_enable);
 
@@ -431,7 +427,7 @@ static int add_jack_kctl(struct hda_codec *codec, hda_nid_t nid,
 		return err;
 
 	if (!phantom_jack)
-		return snd_hda_jack_detect_enable(codec, nid, 0);
+		return snd_hda_jack_detect_enable(codec, nid);
 	return 0;
 }
 
diff --git a/sound/pci/hda/hda_jack.h b/sound/pci/hda/hda_jack.h
index 46e1ea83ce3c..c1abc7324d68 100644
--- a/sound/pci/hda/hda_jack.h
+++ b/sound/pci/hda/hda_jack.h
@@ -19,7 +19,6 @@ typedef void (*hda_jack_callback) (struct hda_codec *, struct hda_jack_tbl *);
 
 struct hda_jack_tbl {
 	hda_nid_t nid;
-	unsigned char action;		/* event action (0 = none) */
 	unsigned char tag;		/* unsol event tag */
 	unsigned int private_data;	/* arbitrary data */
 	hda_jack_callback callback;
@@ -47,29 +46,10 @@ struct hda_jack_tbl *
 snd_hda_jack_tbl_new(struct hda_codec *codec, hda_nid_t nid);
 void snd_hda_jack_tbl_clear(struct hda_codec *codec);
 
-/**
- * snd_hda_jack_get_action - get jack-tbl entry for the tag
- *
- * Call this from the unsol event handler to get the assigned action for the
- * event.  This will mark the dirty flag for the later reporting, too.
- */
-static inline unsigned char
-snd_hda_jack_get_action(struct hda_codec *codec, unsigned int tag)
-{
-	struct hda_jack_tbl *jack = snd_hda_jack_tbl_get_from_tag(codec, tag);
-	if (jack) {
-		jack->jack_dirty = 1;
-		return jack->action;
-	}
-	return 0;
-}
-
 void snd_hda_jack_set_dirty_all(struct hda_codec *codec);
 
-int snd_hda_jack_detect_enable(struct hda_codec *codec, hda_nid_t nid,
-			       unsigned char action);
+int snd_hda_jack_detect_enable(struct hda_codec *codec, hda_nid_t nid);
 int snd_hda_jack_detect_enable_callback(struct hda_codec *codec, hda_nid_t nid,
-					unsigned char action,
 					hda_jack_callback cb);
 
 int snd_hda_jack_set_gating_jack(struct hda_codec *codec, hda_nid_t gated_nid,
diff --git a/sound/pci/hda/patch_ca0132.c b/sound/pci/hda/patch_ca0132.c
index 5d8455e2dacd..39fae52258f0 100644
--- a/sound/pci/hda/patch_ca0132.c
+++ b/sound/pci/hda/patch_ca0132.c
@@ -4116,8 +4116,8 @@ static void init_input(struct hda_codec *codec, hda_nid_t pin, hda_nid_t adc)
 
 static void ca0132_init_unsol(struct hda_codec *codec)
 {
-	snd_hda_jack_detect_enable(codec, UNSOL_TAG_HP, UNSOL_TAG_HP);
-	snd_hda_jack_detect_enable(codec, UNSOL_TAG_AMIC1, UNSOL_TAG_AMIC1);
+	snd_hda_jack_detect_enable(codec, UNSOL_TAG_HP);
+	snd_hda_jack_detect_enable(codec, UNSOL_TAG_AMIC1);
 }
 
 static void refresh_amp_caps(struct hda_codec *codec, hda_nid_t nid, int dir)
@@ -4406,16 +4406,18 @@ static void ca0132_process_dsp_response(struct hda_codec *codec)
 static void ca0132_unsol_event(struct hda_codec *codec, unsigned int res)
 {
 	struct ca0132_spec *spec = codec->spec;
+	unsigned int tag = (res >> AC_UNSOL_RES_TAG_SHIFT) & 0x3f;
 
-	if (((res >> AC_UNSOL_RES_TAG_SHIFT) & 0x3f) == UNSOL_TAG_DSP) {
+	if (tag == UNSOL_TAG_DSP) {
 		ca0132_process_dsp_response(codec);
 	} else {
-		res = snd_hda_jack_get_action(codec,
-				(res >> AC_UNSOL_RES_TAG_SHIFT) & 0x3f);
+		struct hda_jack_tbl *jack;
 
 		codec_dbg(codec, "snd_hda_jack_get_action: 0x%x\n", res);
-
-		switch (res) {
+		jack = snd_hda_jack_tbl_get_from_tag(codec, tag);
+		if (!jack)
+			return;
+		switch (jack->nid) {
 		case UNSOL_TAG_HP:
 			/* Delay enabling the HP amp, to let the mic-detection
 			 * state machine run.
diff --git a/sound/pci/hda/patch_cirrus.c b/sound/pci/hda/patch_cirrus.c
index 3db724eaa53c..69b0ffc55a51 100644
--- a/sound/pci/hda/patch_cirrus.c
+++ b/sound/pci/hda/patch_cirrus.c
@@ -135,8 +135,6 @@ enum {
 #define CS421X_IDX_DAC_CFG	0x03
 #define CS421X_IDX_SPK_CTL	0x04
 
-#define SPDIF_EVENT		0x04
-
 /* Cirrus Logic CS4213 is like CS4210 but does not have SPDIF input/output */
 #define CS4213_VENDOR_NID	0x09
 
@@ -1019,7 +1017,6 @@ static void parse_cs421x_digital(struct hda_codec *codec)
 		if (get_wcaps(codec, nid) & AC_WCAP_UNSOL_CAP) {
 			spec->spdif_detect = 1;
 			snd_hda_jack_detect_enable_callback(codec, nid,
-							    SPDIF_EVENT,
 							    cs4210_spdif_automute);
 		}
 	}
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index 99d7d7fecaad..8f94527f1890 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -2165,7 +2165,7 @@ static int generic_hdmi_init(struct hda_codec *codec)
 		hda_nid_t pin_nid = per_pin->pin_nid;
 
 		hdmi_init_pin(codec, pin_nid);
-		snd_hda_jack_detect_enable_callback(codec, pin_nid, pin_nid,
+		snd_hda_jack_detect_enable_callback(codec, pin_nid,
 			codec->jackpoll_interval > 0 ? jack_callback : NULL);
 	}
 	return 0;
@@ -2428,7 +2428,7 @@ static int simple_playback_init(struct hda_codec *codec)
 	if (get_wcaps(codec, pin) & AC_WCAP_OUT_AMP)
 		snd_hda_codec_write(codec, pin, 0, AC_VERB_SET_AMP_GAIN_MUTE,
 				    AMP_OUT_UNMUTE);
-	snd_hda_jack_detect_enable(codec, pin, pin);
+	snd_hda_jack_detect_enable(codec, pin);
 	return 0;
 }
 
diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
index 6b1a5de07e35..ac00420e59ff 100644
--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -40,9 +40,6 @@
 /* keep halting ALC5505 DSP, for power saving */
 #define HALT_REALTEK_ALC5505
 
-/* unsol event tags */
-#define ALC_DCVOL_EVENT		0x08
-
 /* for GPIO Poll */
 #define GPIO_MASK	0x03
 
@@ -1130,7 +1127,8 @@ static void alc880_fixup_vol_knob(struct hda_codec *codec,
 				  const struct hda_fixup *fix, int action)
 {
 	if (action == HDA_FIXUP_ACT_PROBE)
-		snd_hda_jack_detect_enable_callback(codec, 0x21, ALC_DCVOL_EVENT, alc_update_knob_master);
+		snd_hda_jack_detect_enable_callback(codec, 0x21,
+						    alc_update_knob_master);
 }
 
 static const struct hda_fixup alc880_fixups[] = {
@@ -1593,7 +1591,7 @@ static void alc260_fixup_gpio1_toggle(struct hda_codec *codec,
 		spec->gen.detect_hp = 1;
 		spec->gen.automute_speaker = 1;
 		spec->gen.autocfg.hp_pins[0] = 0x0f; /* copy it for automute */
-		snd_hda_jack_detect_enable_callback(codec, 0x0f, HDA_GEN_HP_EVENT,
+		snd_hda_jack_detect_enable_callback(codec, 0x0f,
 						    snd_hda_gen_hp_automute);
 		snd_hda_add_verbs(codec, alc_gpio1_init_verbs);
 	}
@@ -4254,7 +4252,6 @@ static void alc282_fixup_asus_tx300(struct hda_codec *codec,
 		spec->gen.auto_mute_via_amp = 1;
 		spec->gen.automute_hook = asus_tx300_automute;
 		snd_hda_jack_detect_enable_callback(codec, 0x1b,
-						    HDA_GEN_HP_EVENT,
 						    snd_hda_gen_hp_automute);
 		break;
 	case HDA_FIXUP_ACT_BUILD:
diff --git a/sound/pci/hda/patch_sigmatel.c b/sound/pci/hda/patch_sigmatel.c
index 60aebd0f5e56..bc371cfb5d84 100644
--- a/sound/pci/hda/patch_sigmatel.c
+++ b/sound/pci/hda/patch_sigmatel.c
@@ -40,11 +40,6 @@
 #include "hda_generic.h"
 
 enum {
-	STAC_VREF_EVENT	= 8,
-	STAC_PWR_EVENT,
-};
-
-enum {
 	STAC_REF,
 	STAC_9200_OQO,
 	STAC_9200_DELL_D21,
@@ -505,13 +500,11 @@ static void jack_update_power(struct hda_codec *codec,
 	for (i = 0; i < spec->num_pwrs; i++) {
 		hda_nid_t nid = spec->pwr_nids[i];
 		jack = snd_hda_jack_tbl_get(codec, nid);
-		if (!jack || !jack->action)
+		if (!jack)
 			continue;
-		if (jack->action == STAC_PWR_EVENT ||
-		    jack->action <= HDA_GEN_LAST_EVENT)
-			stac_toggle_power_map(codec, nid,
-					      snd_hda_jack_detect(codec, nid),
-					      false);
+		stac_toggle_power_map(codec, nid,
+				      snd_hda_jack_detect(codec, nid),
+				      false);
 	}
 
 	snd_hda_codec_write(codec, codec->afg, 0, AC_VERB_IDT_SET_POWER_MAP,
@@ -568,7 +561,6 @@ static void stac_init_power_map(struct hda_codec *codec)
 		    spec->vref_mute_led_nid != nid &&
 		    is_jack_detectable(codec, nid)) {
 			snd_hda_jack_detect_enable_callback(codec, nid,
-							    STAC_PWR_EVENT,
 							    jack_update_power);
 		} else {
 			if (def_conf == AC_JACK_PORT_NONE)
@@ -3028,7 +3020,6 @@ static void stac92hd71bxx_fixup_hp_m4(struct hda_codec *codec,
 	snd_hda_codec_write_cache(codec, codec->afg, 0,
 				  AC_VERB_SET_GPIO_UNSOLICITED_RSP_MASK, 0x02);
 	snd_hda_jack_detect_enable_callback(codec, codec->afg,
-					    STAC_VREF_EVENT,
 					    stac_vref_event);
 	jack = snd_hda_jack_tbl_get(codec, codec->afg);
 	if (jack)
@@ -4052,7 +4043,6 @@ static void stac9205_fixup_dell_m43(struct hda_codec *codec,
 		snd_hda_codec_write_cache(codec, codec->afg, 0,
 			AC_VERB_SET_GPIO_UNSOLICITED_RSP_MASK, 0x10);
 		snd_hda_jack_detect_enable_callback(codec, codec->afg,
-						    STAC_VREF_EVENT,
 						    stac_vref_event);
 		jack = snd_hda_jack_tbl_get(codec, codec->afg);
 		if (jack)
diff --git a/sound/pci/hda/patch_via.c b/sound/pci/hda/patch_via.c
index 778166259b3e..2a8be5a5da15 100644
--- a/sound/pci/hda/patch_via.c
+++ b/sound/pci/hda/patch_via.c
@@ -592,8 +592,6 @@ static void via_jack_powerstate_event(struct hda_codec *codec, struct hda_jack_t
 	set_widgets_power_state(codec);
 }
 
-#define VIA_JACK_EVENT	(HDA_GEN_LAST_EVENT + 1)
-
 static void via_set_jack_unsol_events(struct hda_codec *codec)
 {
 	struct via_spec *spec = codec->spec;
@@ -610,7 +608,6 @@ static void via_set_jack_unsol_events(struct hda_codec *codec)
 		if (pin && !snd_hda_jack_tbl_get(codec, pin) &&
 		    is_jack_detectable(codec, pin))
 			snd_hda_jack_detect_enable_callback(codec, pin,
-							    VIA_JACK_EVENT,
 							    via_jack_powerstate_event);
 	}
 
@@ -619,7 +616,6 @@ static void via_set_jack_unsol_events(struct hda_codec *codec)
 		if (pin && !snd_hda_jack_tbl_get(codec, pin) &&
 		    is_jack_detectable(codec, pin))
 			snd_hda_jack_detect_enable_callback(codec, pin,
-							    VIA_JACK_EVENT,
 							    via_jack_powerstate_event);
 	}
 }
-- 
2.1.0

[PATCH RFC 2/7] ALSA: hda - Make snd_hda_jack_tbl_new() static

[Takashi Iwai]

It's called only in hda_jack.c, so make it local.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/pci/hda/hda_jack.c | 3 +--
 sound/pci/hda/hda_jack.h | 2 --
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/sound/pci/hda/hda_jack.c b/sound/pci/hda/hda_jack.c
index 9c8f24f2d56b..7f332794993f 100644
--- a/sound/pci/hda/hda_jack.c
+++ b/sound/pci/hda/hda_jack.c
@@ -94,7 +94,7 @@ EXPORT_SYMBOL_GPL(snd_hda_jack_tbl_get_from_tag);
 /**
  * snd_hda_jack_tbl_new - create a jack-table entry for the given NID
  */
-struct hda_jack_tbl *
+static struct hda_jack_tbl *
 snd_hda_jack_tbl_new(struct hda_codec *codec, hda_nid_t nid)
 {
 	struct hda_jack_tbl *jack = snd_hda_jack_tbl_get(codec, nid);
@@ -108,7 +108,6 @@ snd_hda_jack_tbl_new(struct hda_codec *codec, hda_nid_t nid)
 	jack->tag = codec->jacktbl.used;
 	return jack;
 }
-EXPORT_SYMBOL_GPL(snd_hda_jack_tbl_new);
 
 void snd_hda_jack_tbl_clear(struct hda_codec *codec)
 {
diff --git a/sound/pci/hda/hda_jack.h b/sound/pci/hda/hda_jack.h
index c1abc7324d68..67f42db9c89c 100644
--- a/sound/pci/hda/hda_jack.h
+++ b/sound/pci/hda/hda_jack.h
@@ -42,8 +42,6 @@ snd_hda_jack_tbl_get(struct hda_codec *codec, hda_nid_t nid);
 struct hda_jack_tbl *
 snd_hda_jack_tbl_get_from_tag(struct hda_codec *codec, unsigned char tag);
 
-struct hda_jack_tbl *
-snd_hda_jack_tbl_new(struct hda_codec *codec, hda_nid_t nid);
 void snd_hda_jack_tbl_clear(struct hda_codec *codec);
 
 void snd_hda_jack_set_dirty_all(struct hda_codec *codec);
-- 
2.1.0

[PATCH RFC 3/7] ALSA: hda - Make snd_hda_jack_detect_enable_callbac() returning the jack object

[Takashi Iwai]

STAC/IDT driver calls snd_hda_jack_tbl_get() again after calling
snd_hda_jack_detect_enable_callback().  For simplifying this, let's
make snd_hda_jack_detect_enable_callback() returning the pointer while
handling the error with the standard IS_ERR() & co.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/pci/hda/hda_jack.c       | 20 +++++++++++++-------
 sound/pci/hda/hda_jack.h       |  5 +++--
 sound/pci/hda/patch_sigmatel.c | 14 ++++++--------
 3 files changed, 22 insertions(+), 17 deletions(-)

diff --git a/sound/pci/hda/hda_jack.c b/sound/pci/hda/hda_jack.c
index 7f332794993f..27fccd2c8d41 100644
--- a/sound/pci/hda/hda_jack.c
+++ b/sound/pci/hda/hda_jack.c
@@ -215,28 +215,34 @@ EXPORT_SYMBOL_GPL(snd_hda_jack_detect_state);
 /**
  * snd_hda_jack_detect_enable - enable the jack-detection
  */
-int snd_hda_jack_detect_enable_callback(struct hda_codec *codec, hda_nid_t nid,
-					hda_jack_callback cb)
+struct hda_jack_tbl *
+snd_hda_jack_detect_enable_callback(struct hda_codec *codec, hda_nid_t nid,
+				    hda_jack_callback cb)
 {
 	struct hda_jack_tbl *jack = snd_hda_jack_tbl_new(codec, nid);
+	int err;
+
 	if (!jack)
-		return -ENOMEM;
+		return ERR_PTR(-ENOMEM);
 	if (jack->jack_detect)
-		return 0; /* already registered */
+		return jack; /* already registered */
 	jack->jack_detect = 1;
 	if (cb)
 		jack->callback = cb;
 	if (codec->jackpoll_interval > 0)
-		return 0; /* No unsol if we're polling instead */
-	return snd_hda_codec_write_cache(codec, nid, 0,
+		return jack; /* No unsol if we're polling instead */
+	err = snd_hda_codec_write_cache(codec, nid, 0,
 					 AC_VERB_SET_UNSOLICITED_ENABLE,
 					 AC_USRSP_EN | jack->tag);
+	if (err < 0)
+		return ERR_PTR(err);
+	return jack;
 }
 EXPORT_SYMBOL_GPL(snd_hda_jack_detect_enable_callback);
 
 int snd_hda_jack_detect_enable(struct hda_codec *codec, hda_nid_t nid)
 {
-	return snd_hda_jack_detect_enable_callback(codec, nid, NULL);
+	return PTR_ERR_OR_ZERO(snd_hda_jack_detect_enable_callback(codec, nid, NULL));
 }
 EXPORT_SYMBOL_GPL(snd_hda_jack_detect_enable);
 
diff --git a/sound/pci/hda/hda_jack.h b/sound/pci/hda/hda_jack.h
index 67f42db9c89c..668669ce3e52 100644
--- a/sound/pci/hda/hda_jack.h
+++ b/sound/pci/hda/hda_jack.h
@@ -47,8 +47,9 @@ void snd_hda_jack_tbl_clear(struct hda_codec *codec);
 void snd_hda_jack_set_dirty_all(struct hda_codec *codec);
 
 int snd_hda_jack_detect_enable(struct hda_codec *codec, hda_nid_t nid);
-int snd_hda_jack_detect_enable_callback(struct hda_codec *codec, hda_nid_t nid,
-					hda_jack_callback cb);
+struct hda_jack_tbl *
+snd_hda_jack_detect_enable_callback(struct hda_codec *codec, hda_nid_t nid,
+				    hda_jack_callback cb);
 
 int snd_hda_jack_set_gating_jack(struct hda_codec *codec, hda_nid_t gated_nid,
 				 hda_nid_t gating_nid);
diff --git a/sound/pci/hda/patch_sigmatel.c b/sound/pci/hda/patch_sigmatel.c
index bc371cfb5d84..4b338beb9449 100644
--- a/sound/pci/hda/patch_sigmatel.c
+++ b/sound/pci/hda/patch_sigmatel.c
@@ -3019,10 +3019,9 @@ static void stac92hd71bxx_fixup_hp_m4(struct hda_codec *codec,
 	/* Enable VREF power saving on GPIO1 detect */
 	snd_hda_codec_write_cache(codec, codec->afg, 0,
 				  AC_VERB_SET_GPIO_UNSOLICITED_RSP_MASK, 0x02);
-	snd_hda_jack_detect_enable_callback(codec, codec->afg,
-					    stac_vref_event);
-	jack = snd_hda_jack_tbl_get(codec, codec->afg);
-	if (jack)
+	jack = snd_hda_jack_detect_enable_callback(codec, codec->afg,
+						   stac_vref_event);
+	if (!IS_ERR(jack))
 		jack->private_data = 0x02;
 
 	spec->gpio_mask |= 0x02;
@@ -4042,10 +4041,9 @@ static void stac9205_fixup_dell_m43(struct hda_codec *codec,
 		/* Enable unsol response for GPIO4/Dock HP connection */
 		snd_hda_codec_write_cache(codec, codec->afg, 0,
 			AC_VERB_SET_GPIO_UNSOLICITED_RSP_MASK, 0x10);
-		snd_hda_jack_detect_enable_callback(codec, codec->afg,
-						    stac_vref_event);
-		jack = snd_hda_jack_tbl_get(codec, codec->afg);
-		if (jack)
+		jack = snd_hda_jack_detect_enable_callback(codec, codec->afg,
+							   stac_vref_event);
+		if (!IS_ERR(jack))
 			jack->private_data = 0x01;
 
 		spec->gpio_dir = 0x0b;
-- 
2.1.0

[PATCH RFC 4/7] ALSA: hda - Allow multiple callbacks for jack

[Takashi Iwai]

So far, hda_jack infrastructure allows only one callback per jack, and
this makes things slightly complicated when a driver wants to assign
multiple tasks to a jack, e.g. the standard auto-mute with a power
up/down sequence.  This can be simplified if the hda_jack accepts
multiple callbacks.

This patch is such an extension: the callback-specific part (the
function and private_data) is split to another struct from
hda_jack_tbl, and multiple such objects can be assigned to a single
hda_jack_tbl entry.

The new struct hda_jack_callback is passed to each callback function
now, thus the patch became bigger than expected.  But these changes
are mostly trivial.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/pci/hda/hda_generic.c    | 19 ++++++++++++-------
 sound/pci/hda/hda_generic.h    | 12 ++++++------
 sound/pci/hda/hda_jack.c       | 43 +++++++++++++++++++++++++++++-------------
 sound/pci/hda/hda_jack.h       | 17 ++++++++++++-----
 sound/pci/hda/patch_cirrus.c   |  2 +-
 sound/pci/hda/patch_conexant.c |  3 ++-
 sound/pci/hda/patch_hdmi.c     | 14 ++++++++++----
 sound/pci/hda/patch_realtek.c  | 12 +++++++-----
 sound/pci/hda/patch_sigmatel.c | 24 +++++++++++------------
 sound/pci/hda/patch_via.c      | 11 +++++++----
 10 files changed, 99 insertions(+), 58 deletions(-)

diff --git a/sound/pci/hda/hda_generic.c b/sound/pci/hda/hda_generic.c
index 4d605e4ac41c..32a85f9cac4b 100644
--- a/sound/pci/hda/hda_generic.c
+++ b/sound/pci/hda/hda_generic.c
@@ -2032,7 +2032,8 @@ static int create_speaker_out_ctls(struct hda_codec *codec)
  * independent HP controls
  */
 
-static void call_hp_automute(struct hda_codec *codec, struct hda_jack_tbl *jack);
+static void call_hp_automute(struct hda_codec *codec,
+			     struct hda_jack_callback *jack);
 static int indep_hp_info(struct snd_kcontrol *kcontrol,
 			 struct snd_ctl_elem_info *uinfo)
 {
@@ -3948,7 +3949,8 @@ static void call_update_outputs(struct hda_codec *codec)
 }
 
 /* standard HP-automute helper */
-void snd_hda_gen_hp_automute(struct hda_codec *codec, struct hda_jack_tbl *jack)
+void snd_hda_gen_hp_automute(struct hda_codec *codec,
+			     struct hda_jack_callback *jack)
 {
 	struct hda_gen_spec *spec = codec->spec;
 	hda_nid_t *pins = spec->autocfg.hp_pins;
@@ -3968,7 +3970,8 @@ void snd_hda_gen_hp_automute(struct hda_codec *codec, struct hda_jack_tbl *jack)
 EXPORT_SYMBOL_GPL(snd_hda_gen_hp_automute);
 
 /* standard line-out-automute helper */
-void snd_hda_gen_line_automute(struct hda_codec *codec, struct hda_jack_tbl *jack)
+void snd_hda_gen_line_automute(struct hda_codec *codec,
+			       struct hda_jack_callback *jack)
 {
 	struct hda_gen_spec *spec = codec->spec;
 
@@ -3988,7 +3991,8 @@ void snd_hda_gen_line_automute(struct hda_codec *codec, struct hda_jack_tbl *jac
 EXPORT_SYMBOL_GPL(snd_hda_gen_line_automute);
 
 /* standard mic auto-switch helper */
-void snd_hda_gen_mic_autoswitch(struct hda_codec *codec, struct hda_jack_tbl *jack)
+void snd_hda_gen_mic_autoswitch(struct hda_codec *codec,
+				struct hda_jack_callback *jack)
 {
 	struct hda_gen_spec *spec = codec->spec;
 	int i;
@@ -4011,7 +4015,8 @@ void snd_hda_gen_mic_autoswitch(struct hda_codec *codec, struct hda_jack_tbl *ja
 EXPORT_SYMBOL_GPL(snd_hda_gen_mic_autoswitch);
 
 /* call appropriate hooks */
-static void call_hp_automute(struct hda_codec *codec, struct hda_jack_tbl *jack)
+static void call_hp_automute(struct hda_codec *codec,
+			     struct hda_jack_callback *jack)
 {
 	struct hda_gen_spec *spec = codec->spec;
 	if (spec->hp_automute_hook)
@@ -4021,7 +4026,7 @@ static void call_hp_automute(struct hda_codec *codec, struct hda_jack_tbl *jack)
 }
 
 static void call_line_automute(struct hda_codec *codec,
-			       struct hda_jack_tbl *jack)
+			       struct hda_jack_callback *jack)
 {
 	struct hda_gen_spec *spec = codec->spec;
 	if (spec->line_automute_hook)
@@ -4031,7 +4036,7 @@ static void call_line_automute(struct hda_codec *codec,
 }
 
 static void call_mic_autoswitch(struct hda_codec *codec,
-				struct hda_jack_tbl *jack)
+				struct hda_jack_callback *jack)
 {
 	struct hda_gen_spec *spec = codec->spec;
 	if (spec->mic_autoswitch_hook)
diff --git a/sound/pci/hda/hda_generic.h b/sound/pci/hda/hda_generic.h
index 72f5624125fb..61dd5153f512 100644
--- a/sound/pci/hda/hda_generic.h
+++ b/sound/pci/hda/hda_generic.h
@@ -284,11 +284,11 @@ struct hda_gen_spec {
 
 	/* automute / autoswitch hooks */
 	void (*hp_automute_hook)(struct hda_codec *codec,
-				 struct hda_jack_tbl *tbl);
+				 struct hda_jack_callback *cb);
 	void (*line_automute_hook)(struct hda_codec *codec,
-				   struct hda_jack_tbl *tbl);
+				   struct hda_jack_callback *cb);
 	void (*mic_autoswitch_hook)(struct hda_codec *codec,
-				    struct hda_jack_tbl *tbl);
+				    struct hda_jack_callback *cb);
 };
 
 int snd_hda_gen_spec_init(struct hda_gen_spec *spec);
@@ -320,11 +320,11 @@ int snd_hda_gen_build_pcms(struct hda_codec *codec);
 
 /* standard jack event callbacks */
 void snd_hda_gen_hp_automute(struct hda_codec *codec,
-			     struct hda_jack_tbl *jack);
+			     struct hda_jack_callback *jack);
 void snd_hda_gen_line_automute(struct hda_codec *codec,
-			       struct hda_jack_tbl *jack);
+			       struct hda_jack_callback *jack);
 void snd_hda_gen_mic_autoswitch(struct hda_codec *codec,
-				struct hda_jack_tbl *jack);
+				struct hda_jack_callback *jack);
 void snd_hda_gen_update_outputs(struct hda_codec *codec);
 
 #ifdef CONFIG_PM
diff --git a/sound/pci/hda/hda_jack.c b/sound/pci/hda/hda_jack.c
index 27fccd2c8d41..13a9e6796379 100644
--- a/sound/pci/hda/hda_jack.c
+++ b/sound/pci/hda/hda_jack.c
@@ -117,8 +117,13 @@ void snd_hda_jack_tbl_clear(struct hda_codec *codec)
 		struct hda_jack_tbl *jack = codec->jacktbl.list;
 		int i;
 		for (i = 0; i < codec->jacktbl.used; i++, jack++) {
+			struct hda_jack_callback *cb, *next;
 			if (jack->jack)
 				snd_device_free(codec->bus->card, jack->jack);
+			for (cb = jack->callback; cb; cb = next) {
+				next = cb->next;
+				kfree(cb);
+			}
 		}
 	}
 #endif
@@ -215,28 +220,36 @@ EXPORT_SYMBOL_GPL(snd_hda_jack_detect_state);
 /**
  * snd_hda_jack_detect_enable - enable the jack-detection
  */
-struct hda_jack_tbl *
+struct hda_jack_callback *
 snd_hda_jack_detect_enable_callback(struct hda_codec *codec, hda_nid_t nid,
-				    hda_jack_callback cb)
+				    hda_jack_callback_fn func)
 {
-	struct hda_jack_tbl *jack = snd_hda_jack_tbl_new(codec, nid);
+	struct hda_jack_tbl *jack;
+	struct hda_jack_callback *callback = NULL;
 	int err;
 
+	jack = snd_hda_jack_tbl_new(codec, nid);
 	if (!jack)
 		return ERR_PTR(-ENOMEM);
-	if (jack->jack_detect)
-		return jack; /* already registered */
+	if (func) {
+		callback = kzalloc(sizeof(*callback), GFP_KERNEL);
+		if (!callback)
+			return ERR_PTR(-ENOMEM);
+		callback->func = func;
+		callback->tbl = jack;
+		callback->next = jack->callback;
+		jack->callback = callback;
+	}
+
 	jack->jack_detect = 1;
-	if (cb)
-		jack->callback = cb;
 	if (codec->jackpoll_interval > 0)
-		return jack; /* No unsol if we're polling instead */
+		return callback; /* No unsol if we're polling instead */
 	err = snd_hda_codec_write_cache(codec, nid, 0,
 					 AC_VERB_SET_UNSOLICITED_ENABLE,
 					 AC_USRSP_EN | jack->tag);
 	if (err < 0)
 		return ERR_PTR(err);
-	return jack;
+	return callback;
 }
 EXPORT_SYMBOL_GPL(snd_hda_jack_detect_enable_callback);
 
@@ -499,13 +512,17 @@ EXPORT_SYMBOL_GPL(snd_hda_jack_add_kctls);
 static void call_jack_callback(struct hda_codec *codec,
 			       struct hda_jack_tbl *jack)
 {
-	if (jack->callback)
-		jack->callback(codec, jack);
+	struct hda_jack_callback *cb;
+
+	for (cb = jack->callback; cb; cb = cb->next)
+		cb->func(codec, cb);
 	if (jack->gated_jack) {
 		struct hda_jack_tbl *gated =
 			snd_hda_jack_tbl_get(codec, jack->gated_jack);
-		if (gated && gated->callback)
-			gated->callback(codec, gated);
+		if (gated) {
+			for (cb = gated->callback; cb; cb = cb->next)
+				cb->func(codec, cb);
+		}
 	}
 }
 
diff --git a/sound/pci/hda/hda_jack.h b/sound/pci/hda/hda_jack.h
index 668669ce3e52..b41e0a3ea1fb 100644
--- a/sound/pci/hda/hda_jack.h
+++ b/sound/pci/hda/hda_jack.h
@@ -14,14 +14,21 @@
 
 struct auto_pin_cfg;
 struct hda_jack_tbl;
+struct hda_jack_callback;
 
-typedef void (*hda_jack_callback) (struct hda_codec *, struct hda_jack_tbl *);
+typedef void (*hda_jack_callback_fn) (struct hda_codec *, struct hda_jack_callback *);
+
+struct hda_jack_callback {
+	struct hda_jack_tbl *tbl;
+	hda_jack_callback_fn func;
+	unsigned int private_data;	/* arbitrary data */
+	struct hda_jack_callback *next;
+};
 
 struct hda_jack_tbl {
 	hda_nid_t nid;
 	unsigned char tag;		/* unsol event tag */
-	unsigned int private_data;	/* arbitrary data */
-	hda_jack_callback callback;
+	struct hda_jack_callback *callback;
 	/* jack-detection stuff */
 	unsigned int pin_sense;		/* cached pin-sense value */
 	unsigned int jack_detect:1;	/* capable of jack-detection? */
@@ -47,9 +54,9 @@ void snd_hda_jack_tbl_clear(struct hda_codec *codec);
 void snd_hda_jack_set_dirty_all(struct hda_codec *codec);
 
 int snd_hda_jack_detect_enable(struct hda_codec *codec, hda_nid_t nid);
-struct hda_jack_tbl *
+struct hda_jack_callback *
 snd_hda_jack_detect_enable_callback(struct hda_codec *codec, hda_nid_t nid,
-				    hda_jack_callback cb);
+				    hda_jack_callback_fn cb);
 
 int snd_hda_jack_set_gating_jack(struct hda_codec *codec, hda_nid_t gated_nid,
 				 hda_nid_t gating_nid);
diff --git a/sound/pci/hda/patch_cirrus.c b/sound/pci/hda/patch_cirrus.c
index 69b0ffc55a51..1589c9bcce3e 100644
--- a/sound/pci/hda/patch_cirrus.c
+++ b/sound/pci/hda/patch_cirrus.c
@@ -982,7 +982,7 @@ static void cs4210_pinmux_init(struct hda_codec *codec)
 }
 
 static void cs4210_spdif_automute(struct hda_codec *codec,
-				  struct hda_jack_tbl *tbl)
+				  struct hda_jack_callback *tbl)
 {
 	struct cs_spec *spec = codec->spec;
 	bool spdif_present = false;
diff --git a/sound/pci/hda/patch_conexant.c b/sound/pci/hda/patch_conexant.c
index e0c5bc1d671b..d5b0582daaf0 100644
--- a/sound/pci/hda/patch_conexant.c
+++ b/sound/pci/hda/patch_conexant.c
@@ -393,7 +393,8 @@ static void olpc_xo_update_mic_pins(struct hda_codec *codec)
 }
 
 /* mic_autoswitch hook */
-static void olpc_xo_automic(struct hda_codec *codec, struct hda_jack_tbl *jack)
+static void olpc_xo_automic(struct hda_codec *codec,
+			    struct hda_jack_callback *jack)
 {
 	struct conexant_spec *spec = codec->spec;
 	int saved_cached_write = codec->cached_write;
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index 8f94527f1890..39862e98551c 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -1163,17 +1163,23 @@ static void hdmi_setup_audio_infoframe(struct hda_codec *codec,
 
 static bool hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll);
 
-static void jack_callback(struct hda_codec *codec, struct hda_jack_tbl *jack)
+static void check_presence_and_report(struct hda_codec *codec, hda_nid_t nid)
 {
 	struct hdmi_spec *spec = codec->spec;
-	int pin_idx = pin_nid_to_pin_index(codec, jack->nid);
+	int pin_idx = pin_nid_to_pin_index(codec, nid);
+
 	if (pin_idx < 0)
 		return;
-
 	if (hdmi_present_sense(get_pin(spec, pin_idx), 1))
 		snd_hda_jack_report_sync(codec);
 }
 
+static void jack_callback(struct hda_codec *codec,
+			  struct hda_jack_callback *jack)
+{
+	check_presence_and_report(codec, jack->tbl->nid);
+}
+
 static void hdmi_intrinsic_event(struct hda_codec *codec, unsigned int res)
 {
 	int tag = res >> AC_UNSOL_RES_TAG_SHIFT;
@@ -1190,7 +1196,7 @@ static void hdmi_intrinsic_event(struct hda_codec *codec, unsigned int res)
 		codec->addr, jack->nid, dev_entry, !!(res & AC_UNSOL_RES_IA),
 		!!(res & AC_UNSOL_RES_PD), !!(res & AC_UNSOL_RES_ELDV));
 
-	jack_callback(codec, jack);
+	check_presence_and_report(codec, jack->nid);
 }
 
 static void hdmi_non_intrinsic_event(struct hda_codec *codec, unsigned int res)
diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
index ac00420e59ff..a109fdb085f9 100644
--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -264,7 +264,8 @@ static void alc_fix_pll_init(struct hda_codec *codec, hda_nid_t nid,
 }
 
 /* update the master volume per volume-knob's unsol event */
-static void alc_update_knob_master(struct hda_codec *codec, struct hda_jack_tbl *jack)
+static void alc_update_knob_master(struct hda_codec *codec,
+				   struct hda_jack_callback *jack)
 {
 	unsigned int val;
 	struct snd_kcontrol *kctl;
@@ -276,7 +277,7 @@ static void alc_update_knob_master(struct hda_codec *codec, struct hda_jack_tbl
 	uctl = kzalloc(sizeof(*uctl), GFP_KERNEL);
 	if (!uctl)
 		return;
-	val = snd_hda_codec_read(codec, jack->nid, 0,
+	val = snd_hda_codec_read(codec, jack->tbl->nid, 0,
 				 AC_VERB_GET_VOLUME_KNOB_CONTROL, 0);
 	val &= HDA_AMP_VOLMASK;
 	uctl->value.integer.value[0] = val;
@@ -3272,7 +3273,7 @@ static void alc269_fixup_quanta_mute(struct hda_codec *codec,
 }
 
 static void alc269_x101_hp_automute_hook(struct hda_codec *codec,
-					 struct hda_jack_tbl *jack)
+					 struct hda_jack_callback *jack)
 {
 	struct alc_spec *spec = codec->spec;
 	int vref;
@@ -3926,7 +3927,8 @@ static void alc_update_headset_mode_hook(struct hda_codec *codec,
 	alc_update_headset_mode(codec);
 }
 
-static void alc_update_headset_jack_cb(struct hda_codec *codec, struct hda_jack_tbl *jack)
+static void alc_update_headset_jack_cb(struct hda_codec *codec,
+				       struct hda_jack_callback *jack)
 {
 	struct alc_spec *spec = codec->spec;
 	spec->current_headset_type = ALC_HEADSET_TYPE_UNKNOWN;
@@ -4166,7 +4168,7 @@ static void alc269_fixup_limit_int_mic_boost(struct hda_codec *codec,
 }
 
 static void alc283_hp_automute_hook(struct hda_codec *codec,
-				    struct hda_jack_tbl *jack)
+				    struct hda_jack_callback *jack)
 {
 	struct alc_spec *spec = codec->spec;
 	int vref;
diff --git a/sound/pci/hda/patch_sigmatel.c b/sound/pci/hda/patch_sigmatel.c
index 4b338beb9449..3193529607f2 100644
--- a/sound/pci/hda/patch_sigmatel.c
+++ b/sound/pci/hda/patch_sigmatel.c
@@ -481,7 +481,7 @@ static void stac_toggle_power_map(struct hda_codec *codec, hda_nid_t nid,
 
 /* update power bit per jack plug/unplug */
 static void jack_update_power(struct hda_codec *codec,
-			      struct hda_jack_tbl *jack)
+			      struct hda_jack_callback *jack)
 {
 	struct sigmatel_spec *spec = codec->spec;
 	int i;
@@ -489,9 +489,9 @@ static void jack_update_power(struct hda_codec *codec,
 	if (!spec->num_pwrs)
 		return;
 
-	if (jack && jack->nid) {
-		stac_toggle_power_map(codec, jack->nid,
-				      snd_hda_jack_detect(codec, jack->nid),
+	if (jack && jack->tbl->nid) {
+		stac_toggle_power_map(codec, jack->tbl->nid,
+				      snd_hda_jack_detect(codec, jack->tbl->nid),
 				      true);
 		return;
 	}
@@ -499,8 +499,7 @@ static void jack_update_power(struct hda_codec *codec,
 	/* update all jacks */
 	for (i = 0; i < spec->num_pwrs; i++) {
 		hda_nid_t nid = spec->pwr_nids[i];
-		jack = snd_hda_jack_tbl_get(codec, nid);
-		if (!jack)
+		if (!snd_hda_jack_tbl_get(codec, nid))
 			continue;
 		stac_toggle_power_map(codec, nid,
 				      snd_hda_jack_detect(codec, nid),
@@ -512,27 +511,28 @@ static void jack_update_power(struct hda_codec *codec,
 }
 
 static void stac_hp_automute(struct hda_codec *codec,
-				 struct hda_jack_tbl *jack)
+				 struct hda_jack_callback *jack)
 {
 	snd_hda_gen_hp_automute(codec, jack);
 	jack_update_power(codec, jack);
 }
 
 static void stac_line_automute(struct hda_codec *codec,
-				   struct hda_jack_tbl *jack)
+				   struct hda_jack_callback *jack)
 {
 	snd_hda_gen_line_automute(codec, jack);
 	jack_update_power(codec, jack);
 }
 
 static void stac_mic_autoswitch(struct hda_codec *codec,
-				struct hda_jack_tbl *jack)
+				struct hda_jack_callback *jack)
 {
 	snd_hda_gen_mic_autoswitch(codec, jack);
 	jack_update_power(codec, jack);
 }
 
-static void stac_vref_event(struct hda_codec *codec, struct hda_jack_tbl *event)
+static void stac_vref_event(struct hda_codec *codec,
+			    struct hda_jack_callback *event)
 {
 	unsigned int data;
 
@@ -3011,7 +3011,7 @@ static void stac92hd71bxx_fixup_hp_m4(struct hda_codec *codec,
 				      const struct hda_fixup *fix, int action)
 {
 	struct sigmatel_spec *spec = codec->spec;
-	struct hda_jack_tbl *jack;
+	struct hda_jack_callback *jack;
 
 	if (action != HDA_FIXUP_ACT_PRE_PROBE)
 		return;
@@ -4033,7 +4033,7 @@ static void stac9205_fixup_dell_m43(struct hda_codec *codec,
 				    const struct hda_fixup *fix, int action)
 {
 	struct sigmatel_spec *spec = codec->spec;
-	struct hda_jack_tbl *jack;
+	struct hda_jack_callback *jack;
 
 	if (action == HDA_FIXUP_ACT_PRE_PROBE) {
 		snd_hda_apply_pincfgs(codec, dell_9205_m43_pin_configs);
diff --git a/sound/pci/hda/patch_via.c b/sound/pci/hda/patch_via.c
index 2a8be5a5da15..8d234ab9f06b 100644
--- a/sound/pci/hda/patch_via.c
+++ b/sound/pci/hda/patch_via.c
@@ -118,7 +118,7 @@ static void via_playback_pcm_hook(struct hda_pcm_stream *hinfo,
 				  struct hda_codec *codec,
 				  struct snd_pcm_substream *substream,
 				  int action);
-static void via_hp_automute(struct hda_codec *codec, struct hda_jack_tbl *tbl);
+static void via_hp_automute(struct hda_codec *codec, struct hda_jack_callback *tbl);
 
 static struct via_spec *via_new_spec(struct hda_codec *codec)
 {
@@ -575,19 +575,22 @@ static const struct snd_kcontrol_new vt1708_jack_detect_ctl[] = {
 	{} /* terminator */
 };
 
-static void via_hp_automute(struct hda_codec *codec, struct hda_jack_tbl *tbl)
+static void via_hp_automute(struct hda_codec *codec,
+			    struct hda_jack_callback *tbl)
 {
 	set_widgets_power_state(codec);
 	snd_hda_gen_hp_automute(codec, tbl);
 }
 
-static void via_line_automute(struct hda_codec *codec, struct hda_jack_tbl *tbl)
+static void via_line_automute(struct hda_codec *codec,
+			      struct hda_jack_callback *tbl)
 {
 	set_widgets_power_state(codec);
 	snd_hda_gen_line_automute(codec, tbl);
 }
 
-static void via_jack_powerstate_event(struct hda_codec *codec, struct hda_jack_tbl *tbl)
+static void via_jack_powerstate_event(struct hda_codec *codec,
+				      struct hda_jack_callback *tbl)
 {
 	set_widgets_power_state(codec);
 }
-- 
2.1.0

[PATCH RFC 5/7] ALSA: hda - Remove superfluous callbacks from STAC/IDT codecs

[Takashi Iwai]

Now we can register multiple callbacks to each jack, most of hooks
used in STAC/IDT codecs can be removed by enabling the powermap update
callback for all relevant pins.  Along with this, the call of
stac_init_power_map() can be moved back to stac_parse_auto_config()
and the own build_controls callback can be removed, too.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/pci/hda/patch_sigmatel.c | 38 ++------------------------------------
 1 file changed, 2 insertions(+), 36 deletions(-)

diff --git a/sound/pci/hda/patch_sigmatel.c b/sound/pci/hda/patch_sigmatel.c
index 3193529607f2..4f6413e01c13 100644
--- a/sound/pci/hda/patch_sigmatel.c
+++ b/sound/pci/hda/patch_sigmatel.c
@@ -510,27 +510,6 @@ static void jack_update_power(struct hda_codec *codec,
 			    spec->power_map_bits);
 }
 
-static void stac_hp_automute(struct hda_codec *codec,
-				 struct hda_jack_callback *jack)
-{
-	snd_hda_gen_hp_automute(codec, jack);
-	jack_update_power(codec, jack);
-}
-
-static void stac_line_automute(struct hda_codec *codec,
-				   struct hda_jack_callback *jack)
-{
-	snd_hda_gen_line_automute(codec, jack);
-	jack_update_power(codec, jack);
-}
-
-static void stac_mic_autoswitch(struct hda_codec *codec,
-				struct hda_jack_callback *jack)
-{
-	snd_hda_gen_mic_autoswitch(codec, jack);
-	jack_update_power(codec, jack);
-}
-
 static void stac_vref_event(struct hda_codec *codec,
 			    struct hda_jack_callback *event)
 {
@@ -555,8 +534,6 @@ static void stac_init_power_map(struct hda_codec *codec)
 		hda_nid_t nid = spec->pwr_nids[i];
 		unsigned int def_conf = snd_hda_codec_get_pincfg(codec, nid);
 		def_conf = get_defcfg_connect(def_conf);
-		if (snd_hda_jack_tbl_get(codec, nid))
-			continue;
 		if (def_conf == AC_JACK_PORT_COMPLEX &&
 		    spec->vref_mute_led_nid != nid &&
 		    is_jack_detectable(codec, nid)) {
@@ -4206,9 +4183,6 @@ static int stac_parse_auto_config(struct hda_codec *codec)
 	spec->gen.pcm_capture_hook = stac_capture_pcm_hook;
 
 	spec->gen.automute_hook = stac_update_outputs;
-	spec->gen.hp_automute_hook = stac_hp_automute;
-	spec->gen.line_automute_hook = stac_line_automute;
-	spec->gen.mic_autoswitch_hook = stac_mic_autoswitch;
 
 	err = snd_hda_gen_parse_auto_config(codec, &spec->gen.autocfg);
 	if (err < 0)
@@ -4260,16 +4234,8 @@ static int stac_parse_auto_config(struct hda_codec *codec)
 			return err;
 	}
 
-	return 0;
-}
-
-static int stac_build_controls(struct hda_codec *codec)
-{
-	int err = snd_hda_gen_build_controls(codec);
-
-	if (err < 0)
-		return err;
 	stac_init_power_map(codec);
+
 	return 0;
 }
 
@@ -4383,7 +4349,7 @@ static int stac_suspend(struct hda_codec *codec)
 #endif /* CONFIG_PM */
 
 static const struct hda_codec_ops stac_patch_ops = {
-	.build_controls = stac_build_controls,
+	.build_controls = snd_hda_gen_build_controls,
 	.build_pcms = snd_hda_gen_build_pcms,
 	.init = stac_init,
 	.free = stac_free,
-- 
2.1.0

[PATCH RFC 6/7] ALSA: hda - Remove superfluous hooks from VIA driver

[Takashi Iwai]

Like the previous fix for STAC/IDT codecs, the automute hooks in VIA
driver can be also removed by enabling the power control callback for
all pins.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/pci/hda/patch_via.c | 25 ++-----------------------
 1 file changed, 2 insertions(+), 23 deletions(-)

diff --git a/sound/pci/hda/patch_via.c b/sound/pci/hda/patch_via.c
index 8d234ab9f06b..6c206b6c8d65 100644
--- a/sound/pci/hda/patch_via.c
+++ b/sound/pci/hda/patch_via.c
@@ -118,7 +118,6 @@ static void via_playback_pcm_hook(struct hda_pcm_stream *hinfo,
 				  struct hda_codec *codec,
 				  struct snd_pcm_substream *substream,
 				  int action);
-static void via_hp_automute(struct hda_codec *codec, struct hda_jack_callback *tbl);
 
 static struct via_spec *via_new_spec(struct hda_codec *codec)
 {
@@ -575,20 +574,6 @@ static const struct snd_kcontrol_new vt1708_jack_detect_ctl[] = {
 	{} /* terminator */
 };
 
-static void via_hp_automute(struct hda_codec *codec,
-			    struct hda_jack_callback *tbl)
-{
-	set_widgets_power_state(codec);
-	snd_hda_gen_hp_automute(codec, tbl);
-}
-
-static void via_line_automute(struct hda_codec *codec,
-			      struct hda_jack_callback *tbl)
-{
-	set_widgets_power_state(codec);
-	snd_hda_gen_line_automute(codec, tbl);
-}
-
 static void via_jack_powerstate_event(struct hda_codec *codec,
 				      struct hda_jack_callback *tbl)
 {
@@ -602,22 +587,16 @@ static void via_set_jack_unsol_events(struct hda_codec *codec)
 	hda_nid_t pin;
 	int i;
 
-	spec->gen.hp_automute_hook = via_hp_automute;
-	if (cfg->speaker_pins[0])
-		spec->gen.line_automute_hook = via_line_automute;
-
 	for (i = 0; i < cfg->line_outs; i++) {
 		pin = cfg->line_out_pins[i];
-		if (pin && !snd_hda_jack_tbl_get(codec, pin) &&
-		    is_jack_detectable(codec, pin))
+		if (pin && is_jack_detectable(codec, pin))
 			snd_hda_jack_detect_enable_callback(codec, pin,
 							    via_jack_powerstate_event);
 	}
 
 	for (i = 0; i < cfg->num_inputs; i++) {
 		pin = cfg->line_out_pins[i];
-		if (pin && !snd_hda_jack_tbl_get(codec, pin) &&
-		    is_jack_detectable(codec, pin))
+		if (pin && is_jack_detectable(codec, pin))
 			snd_hda_jack_detect_enable_callback(codec, pin,
 							    via_jack_powerstate_event);
 	}
-- 
2.1.0

[PATCH RFC 7/7] ALSA: hda - Use standard hda_jack infrastructure for CA0132 driver

[Takashi Iwai]

For its headphone, mic and DSP responses, we can use the standard
hda_jack infrastructure in CA0132 driver, too.  The only point to
handle carefully is the delayed headphone jack handling.  It tries to
react after a certain delay.  Here we use the existing block_report
flag in hda_jack_tbl (that was implemented for HDMI).

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/pci/hda/patch_ca0132.c | 76 ++++++++++++++++++++------------------------
 1 file changed, 34 insertions(+), 42 deletions(-)

diff --git a/sound/pci/hda/patch_ca0132.c b/sound/pci/hda/patch_ca0132.c
index 39fae52258f0..4f7ffa8c4a0d 100644
--- a/sound/pci/hda/patch_ca0132.c
+++ b/sound/pci/hda/patch_ca0132.c
@@ -3224,8 +3224,14 @@ static void ca0132_unsol_hp_delayed(struct work_struct *work)
 {
 	struct ca0132_spec *spec = container_of(
 		to_delayed_work(work), struct ca0132_spec, unsol_hp_work);
+	struct hda_jack_tbl *jack;
+
 	ca0132_select_out(spec->codec);
-	snd_hda_jack_report_sync(spec->codec);
+	jack = snd_hda_jack_tbl_get(spec->codec, UNSOL_TAG_HP);
+	if (jack) {
+		jack->block_report = 0;
+		snd_hda_jack_report_sync(spec->codec);
+	}
 }
 
 static void ca0132_set_dmic(struct hda_codec *codec, int enable);
@@ -4114,12 +4120,6 @@ static void init_input(struct hda_codec *codec, hda_nid_t pin, hda_nid_t adc)
 	}
 }
 
-static void ca0132_init_unsol(struct hda_codec *codec)
-{
-	snd_hda_jack_detect_enable(codec, UNSOL_TAG_HP);
-	snd_hda_jack_detect_enable(codec, UNSOL_TAG_AMIC1);
-}
-
 static void refresh_amp_caps(struct hda_codec *codec, hda_nid_t nid, int dir)
 {
 	unsigned int caps;
@@ -4390,7 +4390,8 @@ static void ca0132_download_dsp(struct hda_codec *codec)
 		ca0132_set_dsp_msr(codec, true);
 }
 
-static void ca0132_process_dsp_response(struct hda_codec *codec)
+static void ca0132_process_dsp_response(struct hda_codec *codec,
+					struct hda_jack_callback *callback)
 {
 	struct ca0132_spec *spec = codec->spec;
 
@@ -4403,38 +4404,31 @@ static void ca0132_process_dsp_response(struct hda_codec *codec)
 	dspio_clear_response_queue(codec);
 }
 
-static void ca0132_unsol_event(struct hda_codec *codec, unsigned int res)
+static void hp_callback(struct hda_codec *codec, struct hda_jack_callback *cb)
 {
 	struct ca0132_spec *spec = codec->spec;
-	unsigned int tag = (res >> AC_UNSOL_RES_TAG_SHIFT) & 0x3f;
 
-	if (tag == UNSOL_TAG_DSP) {
-		ca0132_process_dsp_response(codec);
-	} else {
-		struct hda_jack_tbl *jack;
-
-		codec_dbg(codec, "snd_hda_jack_get_action: 0x%x\n", res);
-		jack = snd_hda_jack_tbl_get_from_tag(codec, tag);
-		if (!jack)
-			return;
-		switch (jack->nid) {
-		case UNSOL_TAG_HP:
-			/* Delay enabling the HP amp, to let the mic-detection
-			 * state machine run.
-			 */
-			cancel_delayed_work_sync(&spec->unsol_hp_work);
-			queue_delayed_work(codec->bus->workq,
-					   &spec->unsol_hp_work,
-					   msecs_to_jiffies(500));
-			break;
-		case UNSOL_TAG_AMIC1:
-			ca0132_select_mic(codec);
-			snd_hda_jack_report_sync(codec);
-			break;
-		default:
-			break;
-		}
-	}
+	/* Delay enabling the HP amp, to let the mic-detection
+	 * state machine run.
+	 */
+	cancel_delayed_work_sync(&spec->unsol_hp_work);
+	queue_delayed_work(codec->bus->workq, &spec->unsol_hp_work,
+			   msecs_to_jiffies(500));
+	cb->tbl->block_report = 1;
+}
+
+static void amic_callback(struct hda_codec *codec, struct hda_jack_callback *cb)
+{
+	ca0132_select_mic(codec);
+}
+
+static void ca0132_init_unsol(struct hda_codec *codec)
+{
+	snd_hda_jack_detect_enable_callback(codec, UNSOL_TAG_HP, hp_callback);
+	snd_hda_jack_detect_enable_callback(codec, UNSOL_TAG_AMIC1,
+					    amic_callback);
+	snd_hda_jack_detect_enable_callback(codec, UNSOL_TAG_DSP,
+					    ca0132_process_dsp_response);
 }
 
 /*
@@ -4445,8 +4439,6 @@ static void ca0132_unsol_event(struct hda_codec *codec, unsigned int res)
 static struct hda_verb ca0132_base_init_verbs[] = {
 	/*enable ct extension*/
 	{0x15, VENDOR_CHIPIO_CT_EXTENSIONS_ENABLE, 0x1},
-	/*enable DSP node unsol, needed for DSP download*/
-	{0x16, AC_VERB_SET_UNSOLICITED_ENABLE, AC_USRSP_EN | UNSOL_TAG_DSP},
 	{}
 };
 
@@ -4563,6 +4555,8 @@ static int ca0132_init(struct hda_codec *codec)
 
 	snd_hda_power_up(codec);
 
+	ca0132_init_unsol(codec);
+
 	ca0132_init_params(codec);
 	ca0132_init_flags(codec);
 	snd_hda_sequence_write(codec, spec->base_init_verbs);
@@ -4585,8 +4579,6 @@ static int ca0132_init(struct hda_codec *codec)
 	for (i = 0; i < spec->num_init_verbs; i++)
 		snd_hda_sequence_write(codec, spec->init_verbs[i]);
 
-	ca0132_init_unsol(codec);
-
 	ca0132_select_out(codec);
 	ca0132_select_mic(codec);
 
@@ -4614,7 +4606,7 @@ static struct hda_codec_ops ca0132_patch_ops = {
 	.build_pcms = ca0132_build_pcms,
 	.init = ca0132_init,
 	.free = ca0132_free,
-	.unsol_event = ca0132_unsol_event,
+	.unsol_event = snd_hda_jack_unsol_event,
 };
 
 static void ca0132_config(struct hda_codec *codec)
-- 
2.1.0

Re: [PATCH RFC 3/7] ALSA: hda - Make snd_hda_jack_detect_enable_callbac() returning the jack object

[David Henningsson]

Nitpick: s/callbac/callback and s/returning/return in subject

On 2014-09-11 16:19, Takashi Iwai wrote:
> STAC/IDT driver calls snd_hda_jack_tbl_get() again after calling
> snd_hda_jack_detect_enable_callback().  For simplifying this, let's
> make snd_hda_jack_detect_enable_callback() returning the pointer while
> handling the error with the standard IS_ERR() & co.
>
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>   sound/pci/hda/hda_jack.c       | 20 +++++++++++++-------
>   sound/pci/hda/hda_jack.h       |  5 +++--
>   sound/pci/hda/patch_sigmatel.c | 14 ++++++--------
>   3 files changed, 22 insertions(+), 17 deletions(-)
>
> diff --git a/sound/pci/hda/hda_jack.c b/sound/pci/hda/hda_jack.c
> index 7f332794993f..27fccd2c8d41 100644
> --- a/sound/pci/hda/hda_jack.c
> +++ b/sound/pci/hda/hda_jack.c
> @@ -215,28 +215,34 @@ EXPORT_SYMBOL_GPL(snd_hda_jack_detect_state);
>   /**
>    * snd_hda_jack_detect_enable - enable the jack-detection
>    */
> -int snd_hda_jack_detect_enable_callback(struct hda_codec *codec, hda_nid_t nid,
> -					hda_jack_callback cb)
> +struct hda_jack_tbl *
> +snd_hda_jack_detect_enable_callback(struct hda_codec *codec, hda_nid_t nid,
> +				    hda_jack_callback cb)

I have not seen the ERR_PTR usage before and it's not used commonly in 
the hda driver, so I think it's important to document that it can return 
a non-null pointer that is not a valid hda_jack_tbl. Looks dangerous 
otherwise.

-- 
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic

Re: [PATCH RFC 4/7] ALSA: hda - Allow multiple callbacks for jack

[David Henningsson]

On 2014-09-11 16:19, Takashi Iwai wrote:
> So far, hda_jack infrastructure allows only one callback per jack, and
> this makes things slightly complicated when a driver wants to assign
> multiple tasks to a jack, e.g. the standard auto-mute with a power
> up/down sequence.  This can be simplified if the hda_jack accepts
> multiple callbacks.
>
> This patch is such an extension: the callback-specific part (the
> function and private_data) is split to another struct from
> hda_jack_tbl, and multiple such objects can be assigned to a single
> hda_jack_tbl entry.
>
> The new struct hda_jack_callback is passed to each callback function
> now, thus the patch became bigger than expected.  But these changes
> are mostly trivial.
>
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>   sound/pci/hda/hda_generic.c    | 19 ++++++++++++-------
>   sound/pci/hda/hda_generic.h    | 12 ++++++------
>   sound/pci/hda/hda_jack.c       | 43 +++++++++++++++++++++++++++++-------------
>   sound/pci/hda/hda_jack.h       | 17 ++++++++++++-----
>   sound/pci/hda/patch_cirrus.c   |  2 +-
>   sound/pci/hda/patch_conexant.c |  3 ++-
>   sound/pci/hda/patch_hdmi.c     | 14 ++++++++++----
>   sound/pci/hda/patch_realtek.c  | 12 +++++++-----
>   sound/pci/hda/patch_sigmatel.c | 24 +++++++++++------------
>   sound/pci/hda/patch_via.c      | 11 +++++++----
>   10 files changed, 99 insertions(+), 58 deletions(-)
>
> diff --git a/sound/pci/hda/hda_generic.c b/sound/pci/hda/hda_generic.c
> index 4d605e4ac41c..32a85f9cac4b 100644
> --- a/sound/pci/hda/hda_generic.c
> +++ b/sound/pci/hda/hda_generic.c
> @@ -2032,7 +2032,8 @@ static int create_speaker_out_ctls(struct hda_codec *codec)
>    * independent HP controls
>    */
>
> -static void call_hp_automute(struct hda_codec *codec, struct hda_jack_tbl *jack);
> +static void call_hp_automute(struct hda_codec *codec,
> +			     struct hda_jack_callback *jack);
>   static int indep_hp_info(struct snd_kcontrol *kcontrol,
>   			 struct snd_ctl_elem_info *uinfo)
>   {
> @@ -3948,7 +3949,8 @@ static void call_update_outputs(struct hda_codec *codec)
>   }
>
>   /* standard HP-automute helper */
> -void snd_hda_gen_hp_automute(struct hda_codec *codec, struct hda_jack_tbl *jack)
> +void snd_hda_gen_hp_automute(struct hda_codec *codec,
> +			     struct hda_jack_callback *jack)
>   {
>   	struct hda_gen_spec *spec = codec->spec;
>   	hda_nid_t *pins = spec->autocfg.hp_pins;
> @@ -3968,7 +3970,8 @@ void snd_hda_gen_hp_automute(struct hda_codec *codec, struct hda_jack_tbl *jack)
>   EXPORT_SYMBOL_GPL(snd_hda_gen_hp_automute);
>
>   /* standard line-out-automute helper */
> -void snd_hda_gen_line_automute(struct hda_codec *codec, struct hda_jack_tbl *jack)
> +void snd_hda_gen_line_automute(struct hda_codec *codec,
> +			       struct hda_jack_callback *jack)
>   {
>   	struct hda_gen_spec *spec = codec->spec;
>
> @@ -3988,7 +3991,8 @@ void snd_hda_gen_line_automute(struct hda_codec *codec, struct hda_jack_tbl *jac
>   EXPORT_SYMBOL_GPL(snd_hda_gen_line_automute);
>
>   /* standard mic auto-switch helper */
> -void snd_hda_gen_mic_autoswitch(struct hda_codec *codec, struct hda_jack_tbl *jack)
> +void snd_hda_gen_mic_autoswitch(struct hda_codec *codec,
> +				struct hda_jack_callback *jack)
>   {
>   	struct hda_gen_spec *spec = codec->spec;
>   	int i;
> @@ -4011,7 +4015,8 @@ void snd_hda_gen_mic_autoswitch(struct hda_codec *codec, struct hda_jack_tbl *ja
>   EXPORT_SYMBOL_GPL(snd_hda_gen_mic_autoswitch);
>
>   /* call appropriate hooks */
> -static void call_hp_automute(struct hda_codec *codec, struct hda_jack_tbl *jack)
> +static void call_hp_automute(struct hda_codec *codec,
> +			     struct hda_jack_callback *jack)
>   {
>   	struct hda_gen_spec *spec = codec->spec;
>   	if (spec->hp_automute_hook)
> @@ -4021,7 +4026,7 @@ static void call_hp_automute(struct hda_codec *codec, struct hda_jack_tbl *jack)
>   }
>
>   static void call_line_automute(struct hda_codec *codec,
> -			       struct hda_jack_tbl *jack)
> +			       struct hda_jack_callback *jack)
>   {
>   	struct hda_gen_spec *spec = codec->spec;
>   	if (spec->line_automute_hook)
> @@ -4031,7 +4036,7 @@ static void call_line_automute(struct hda_codec *codec,
>   }
>
>   static void call_mic_autoswitch(struct hda_codec *codec,
> -				struct hda_jack_tbl *jack)
> +				struct hda_jack_callback *jack)
>   {
>   	struct hda_gen_spec *spec = codec->spec;
>   	if (spec->mic_autoswitch_hook)
> diff --git a/sound/pci/hda/hda_generic.h b/sound/pci/hda/hda_generic.h
> index 72f5624125fb..61dd5153f512 100644
> --- a/sound/pci/hda/hda_generic.h
> +++ b/sound/pci/hda/hda_generic.h
> @@ -284,11 +284,11 @@ struct hda_gen_spec {
>
>   	/* automute / autoswitch hooks */
>   	void (*hp_automute_hook)(struct hda_codec *codec,
> -				 struct hda_jack_tbl *tbl);
> +				 struct hda_jack_callback *cb);
>   	void (*line_automute_hook)(struct hda_codec *codec,
> -				   struct hda_jack_tbl *tbl);
> +				   struct hda_jack_callback *cb);
>   	void (*mic_autoswitch_hook)(struct hda_codec *codec,
> -				    struct hda_jack_tbl *tbl);
> +				    struct hda_jack_callback *cb);
>   };
>
>   int snd_hda_gen_spec_init(struct hda_gen_spec *spec);
> @@ -320,11 +320,11 @@ int snd_hda_gen_build_pcms(struct hda_codec *codec);
>
>   /* standard jack event callbacks */
>   void snd_hda_gen_hp_automute(struct hda_codec *codec,
> -			     struct hda_jack_tbl *jack);
> +			     struct hda_jack_callback *jack);
>   void snd_hda_gen_line_automute(struct hda_codec *codec,
> -			       struct hda_jack_tbl *jack);
> +			       struct hda_jack_callback *jack);
>   void snd_hda_gen_mic_autoswitch(struct hda_codec *codec,
> -				struct hda_jack_tbl *jack);
> +				struct hda_jack_callback *jack);
>   void snd_hda_gen_update_outputs(struct hda_codec *codec);
>
>   #ifdef CONFIG_PM
> diff --git a/sound/pci/hda/hda_jack.c b/sound/pci/hda/hda_jack.c
> index 27fccd2c8d41..13a9e6796379 100644
> --- a/sound/pci/hda/hda_jack.c
> +++ b/sound/pci/hda/hda_jack.c
> @@ -117,8 +117,13 @@ void snd_hda_jack_tbl_clear(struct hda_codec *codec)
>   		struct hda_jack_tbl *jack = codec->jacktbl.list;
>   		int i;
>   		for (i = 0; i < codec->jacktbl.used; i++, jack++) {
> +			struct hda_jack_callback *cb, *next;
>   			if (jack->jack)
>   				snd_device_free(codec->bus->card, jack->jack);
> +			for (cb = jack->callback; cb; cb = next) {
> +				next = cb->next;
> +				kfree(cb);
> +			}
>   		}
>   	}
>   #endif
> @@ -215,28 +220,36 @@ EXPORT_SYMBOL_GPL(snd_hda_jack_detect_state);
>   /**
>    * snd_hda_jack_detect_enable - enable the jack-detection
>    */
> -struct hda_jack_tbl *
> +struct hda_jack_callback *
>   snd_hda_jack_detect_enable_callback(struct hda_codec *codec, hda_nid_t nid,
> -				    hda_jack_callback cb)
> +				    hda_jack_callback_fn func)
>   {
> -	struct hda_jack_tbl *jack = snd_hda_jack_tbl_new(codec, nid);
> +	struct hda_jack_tbl *jack;
> +	struct hda_jack_callback *callback = NULL;
>   	int err;
>
> +	jack = snd_hda_jack_tbl_new(codec, nid);
>   	if (!jack)
>   		return ERR_PTR(-ENOMEM);

> -	if (jack->jack_detect)
> -		return jack; /* already registered */

This check is an optimisation (do not write unsol verbs to the codec 
more than once, even if many callbacks are added) and should not be 
removed. The "if (func) " part should just move above the check.

> +	if (func) {
> +		callback = kzalloc(sizeof(*callback), GFP_KERNEL);
> +		if (!callback)
> +			return ERR_PTR(-ENOMEM);
> +		callback->func = func;
> +		callback->tbl = jack;
> +		callback->next = jack->callback;
> +		jack->callback = callback;
> +	}
> +
>   	jack->jack_detect = 1;
> -	if (cb)
> -		jack->callback = cb;
>   	if (codec->jackpoll_interval > 0)
> -		return jack; /* No unsol if we're polling instead */
> +		return callback; /* No unsol if we're polling instead */
>   	err = snd_hda_codec_write_cache(codec, nid, 0,
>   					 AC_VERB_SET_UNSOLICITED_ENABLE,
>   					 AC_USRSP_EN | jack->tag);
>   	if (err < 0)
>   		return ERR_PTR(err);
> -	return jack;
> +	return callback;
>   }
>   EXPORT_SYMBOL_GPL(snd_hda_jack_detect_enable_callback);
>
> @@ -499,13 +512,17 @@ EXPORT_SYMBOL_GPL(snd_hda_jack_add_kctls);
>   static void call_jack_callback(struct hda_codec *codec,
>   			       struct hda_jack_tbl *jack)
>   {
> -	if (jack->callback)
> -		jack->callback(codec, jack);
> +	struct hda_jack_callback *cb;
> +
> +	for (cb = jack->callback; cb; cb = cb->next)
> +		cb->func(codec, cb);
>   	if (jack->gated_jack) {
>   		struct hda_jack_tbl *gated =
>   			snd_hda_jack_tbl_get(codec, jack->gated_jack);
> -		if (gated && gated->callback)
> -			gated->callback(codec, gated);
> +		if (gated) {
> +			for (cb = gated->callback; cb; cb = cb->next)
> +				cb->func(codec, cb);
> +		}
>   	}
>   }
>
> diff --git a/sound/pci/hda/hda_jack.h b/sound/pci/hda/hda_jack.h
> index 668669ce3e52..b41e0a3ea1fb 100644
> --- a/sound/pci/hda/hda_jack.h
> +++ b/sound/pci/hda/hda_jack.h
> @@ -14,14 +14,21 @@
>
>   struct auto_pin_cfg;
>   struct hda_jack_tbl;
> +struct hda_jack_callback;
>
> -typedef void (*hda_jack_callback) (struct hda_codec *, struct hda_jack_tbl *);
> +typedef void (*hda_jack_callback_fn) (struct hda_codec *, struct hda_jack_callback *);
> +
> +struct hda_jack_callback {
> +	struct hda_jack_tbl *tbl;
> +	hda_jack_callback_fn func;
> +	unsigned int private_data;	/* arbitrary data */
> +	struct hda_jack_callback *next;
> +};
>
>   struct hda_jack_tbl {
>   	hda_nid_t nid;
>   	unsigned char tag;		/* unsol event tag */
> -	unsigned int private_data;	/* arbitrary data */
> -	hda_jack_callback callback;
> +	struct hda_jack_callback *callback;
>   	/* jack-detection stuff */
>   	unsigned int pin_sense;		/* cached pin-sense value */
>   	unsigned int jack_detect:1;	/* capable of jack-detection? */
> @@ -47,9 +54,9 @@ void snd_hda_jack_tbl_clear(struct hda_codec *codec);
>   void snd_hda_jack_set_dirty_all(struct hda_codec *codec);
>
>   int snd_hda_jack_detect_enable(struct hda_codec *codec, hda_nid_t nid);
> -struct hda_jack_tbl *
> +struct hda_jack_callback *
>   snd_hda_jack_detect_enable_callback(struct hda_codec *codec, hda_nid_t nid,
> -				    hda_jack_callback cb);
> +				    hda_jack_callback_fn cb);
>
>   int snd_hda_jack_set_gating_jack(struct hda_codec *codec, hda_nid_t gated_nid,
>   				 hda_nid_t gating_nid);
> diff --git a/sound/pci/hda/patch_cirrus.c b/sound/pci/hda/patch_cirrus.c
> index 69b0ffc55a51..1589c9bcce3e 100644
> --- a/sound/pci/hda/patch_cirrus.c
> +++ b/sound/pci/hda/patch_cirrus.c
> @@ -982,7 +982,7 @@ static void cs4210_pinmux_init(struct hda_codec *codec)
>   }
>
>   static void cs4210_spdif_automute(struct hda_codec *codec,
> -				  struct hda_jack_tbl *tbl)
> +				  struct hda_jack_callback *tbl)
>   {
>   	struct cs_spec *spec = codec->spec;
>   	bool spdif_present = false;
> diff --git a/sound/pci/hda/patch_conexant.c b/sound/pci/hda/patch_conexant.c
> index e0c5bc1d671b..d5b0582daaf0 100644
> --- a/sound/pci/hda/patch_conexant.c
> +++ b/sound/pci/hda/patch_conexant.c
> @@ -393,7 +393,8 @@ static void olpc_xo_update_mic_pins(struct hda_codec *codec)
>   }
>
>   /* mic_autoswitch hook */
> -static void olpc_xo_automic(struct hda_codec *codec, struct hda_jack_tbl *jack)
> +static void olpc_xo_automic(struct hda_codec *codec,
> +			    struct hda_jack_callback *jack)
>   {
>   	struct conexant_spec *spec = codec->spec;
>   	int saved_cached_write = codec->cached_write;
> diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
> index 8f94527f1890..39862e98551c 100644
> --- a/sound/pci/hda/patch_hdmi.c
> +++ b/sound/pci/hda/patch_hdmi.c
> @@ -1163,17 +1163,23 @@ static void hdmi_setup_audio_infoframe(struct hda_codec *codec,
>
>   static bool hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll);
>
> -static void jack_callback(struct hda_codec *codec, struct hda_jack_tbl *jack)
> +static void check_presence_and_report(struct hda_codec *codec, hda_nid_t nid)
>   {
>   	struct hdmi_spec *spec = codec->spec;
> -	int pin_idx = pin_nid_to_pin_index(codec, jack->nid);
> +	int pin_idx = pin_nid_to_pin_index(codec, nid);
> +
>   	if (pin_idx < 0)
>   		return;
> -
>   	if (hdmi_present_sense(get_pin(spec, pin_idx), 1))
>   		snd_hda_jack_report_sync(codec);
>   }
>
> +static void jack_callback(struct hda_codec *codec,
> +			  struct hda_jack_callback *jack)
> +{
> +	check_presence_and_report(codec, jack->tbl->nid);
> +}
> +
>   static void hdmi_intrinsic_event(struct hda_codec *codec, unsigned int res)
>   {
>   	int tag = res >> AC_UNSOL_RES_TAG_SHIFT;
> @@ -1190,7 +1196,7 @@ static void hdmi_intrinsic_event(struct hda_codec *codec, unsigned int res)
>   		codec->addr, jack->nid, dev_entry, !!(res & AC_UNSOL_RES_IA),
>   		!!(res & AC_UNSOL_RES_PD), !!(res & AC_UNSOL_RES_ELDV));
>
> -	jack_callback(codec, jack);
> +	check_presence_and_report(codec, jack->nid);
>   }
>
>   static void hdmi_non_intrinsic_event(struct hda_codec *codec, unsigned int res)
> diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
> index ac00420e59ff..a109fdb085f9 100644
> --- a/sound/pci/hda/patch_realtek.c
> +++ b/sound/pci/hda/patch_realtek.c
> @@ -264,7 +264,8 @@ static void alc_fix_pll_init(struct hda_codec *codec, hda_nid_t nid,
>   }
>
>   /* update the master volume per volume-knob's unsol event */
> -static void alc_update_knob_master(struct hda_codec *codec, struct hda_jack_tbl *jack)
> +static void alc_update_knob_master(struct hda_codec *codec,
> +				   struct hda_jack_callback *jack)
>   {
>   	unsigned int val;
>   	struct snd_kcontrol *kctl;
> @@ -276,7 +277,7 @@ static void alc_update_knob_master(struct hda_codec *codec, struct hda_jack_tbl
>   	uctl = kzalloc(sizeof(*uctl), GFP_KERNEL);
>   	if (!uctl)
>   		return;
> -	val = snd_hda_codec_read(codec, jack->nid, 0,
> +	val = snd_hda_codec_read(codec, jack->tbl->nid, 0,
>   				 AC_VERB_GET_VOLUME_KNOB_CONTROL, 0);
>   	val &= HDA_AMP_VOLMASK;
>   	uctl->value.integer.value[0] = val;
> @@ -3272,7 +3273,7 @@ static void alc269_fixup_quanta_mute(struct hda_codec *codec,
>   }
>
>   static void alc269_x101_hp_automute_hook(struct hda_codec *codec,
> -					 struct hda_jack_tbl *jack)
> +					 struct hda_jack_callback *jack)
>   {
>   	struct alc_spec *spec = codec->spec;
>   	int vref;
> @@ -3926,7 +3927,8 @@ static void alc_update_headset_mode_hook(struct hda_codec *codec,
>   	alc_update_headset_mode(codec);
>   }
>
> -static void alc_update_headset_jack_cb(struct hda_codec *codec, struct hda_jack_tbl *jack)
> +static void alc_update_headset_jack_cb(struct hda_codec *codec,
> +				       struct hda_jack_callback *jack)
>   {
>   	struct alc_spec *spec = codec->spec;
>   	spec->current_headset_type = ALC_HEADSET_TYPE_UNKNOWN;
> @@ -4166,7 +4168,7 @@ static void alc269_fixup_limit_int_mic_boost(struct hda_codec *codec,
>   }
>
>   static void alc283_hp_automute_hook(struct hda_codec *codec,
> -				    struct hda_jack_tbl *jack)
> +				    struct hda_jack_callback *jack)
>   {
>   	struct alc_spec *spec = codec->spec;
>   	int vref;
> diff --git a/sound/pci/hda/patch_sigmatel.c b/sound/pci/hda/patch_sigmatel.c
> index 4b338beb9449..3193529607f2 100644
> --- a/sound/pci/hda/patch_sigmatel.c
> +++ b/sound/pci/hda/patch_sigmatel.c
> @@ -481,7 +481,7 @@ static void stac_toggle_power_map(struct hda_codec *codec, hda_nid_t nid,
>
>   /* update power bit per jack plug/unplug */
>   static void jack_update_power(struct hda_codec *codec,
> -			      struct hda_jack_tbl *jack)
> +			      struct hda_jack_callback *jack)
>   {
>   	struct sigmatel_spec *spec = codec->spec;
>   	int i;
> @@ -489,9 +489,9 @@ static void jack_update_power(struct hda_codec *codec,
>   	if (!spec->num_pwrs)
>   		return;
>
> -	if (jack && jack->nid) {
> -		stac_toggle_power_map(codec, jack->nid,
> -				      snd_hda_jack_detect(codec, jack->nid),
> +	if (jack && jack->tbl->nid) {
> +		stac_toggle_power_map(codec, jack->tbl->nid,
> +				      snd_hda_jack_detect(codec, jack->tbl->nid),
>   				      true);
>   		return;
>   	}
> @@ -499,8 +499,7 @@ static void jack_update_power(struct hda_codec *codec,
>   	/* update all jacks */
>   	for (i = 0; i < spec->num_pwrs; i++) {
>   		hda_nid_t nid = spec->pwr_nids[i];
> -		jack = snd_hda_jack_tbl_get(codec, nid);
> -		if (!jack)
> +		if (!snd_hda_jack_tbl_get(codec, nid))
>   			continue;
>   		stac_toggle_power_map(codec, nid,
>   				      snd_hda_jack_detect(codec, nid),
> @@ -512,27 +511,28 @@ static void jack_update_power(struct hda_codec *codec,
>   }
>
>   static void stac_hp_automute(struct hda_codec *codec,
> -				 struct hda_jack_tbl *jack)
> +				 struct hda_jack_callback *jack)
>   {
>   	snd_hda_gen_hp_automute(codec, jack);
>   	jack_update_power(codec, jack);
>   }
>
>   static void stac_line_automute(struct hda_codec *codec,
> -				   struct hda_jack_tbl *jack)
> +				   struct hda_jack_callback *jack)
>   {
>   	snd_hda_gen_line_automute(codec, jack);
>   	jack_update_power(codec, jack);
>   }
>
>   static void stac_mic_autoswitch(struct hda_codec *codec,
> -				struct hda_jack_tbl *jack)
> +				struct hda_jack_callback *jack)
>   {
>   	snd_hda_gen_mic_autoswitch(codec, jack);
>   	jack_update_power(codec, jack);
>   }
>
> -static void stac_vref_event(struct hda_codec *codec, struct hda_jack_tbl *event)
> +static void stac_vref_event(struct hda_codec *codec,
> +			    struct hda_jack_callback *event)
>   {
>   	unsigned int data;
>
> @@ -3011,7 +3011,7 @@ static void stac92hd71bxx_fixup_hp_m4(struct hda_codec *codec,
>   				      const struct hda_fixup *fix, int action)
>   {
>   	struct sigmatel_spec *spec = codec->spec;
> -	struct hda_jack_tbl *jack;
> +	struct hda_jack_callback *jack;
>
>   	if (action != HDA_FIXUP_ACT_PRE_PROBE)
>   		return;
> @@ -4033,7 +4033,7 @@ static void stac9205_fixup_dell_m43(struct hda_codec *codec,
>   				    const struct hda_fixup *fix, int action)
>   {
>   	struct sigmatel_spec *spec = codec->spec;
> -	struct hda_jack_tbl *jack;
> +	struct hda_jack_callback *jack;
>
>   	if (action == HDA_FIXUP_ACT_PRE_PROBE) {
>   		snd_hda_apply_pincfgs(codec, dell_9205_m43_pin_configs);
> diff --git a/sound/pci/hda/patch_via.c b/sound/pci/hda/patch_via.c
> index 2a8be5a5da15..8d234ab9f06b 100644
> --- a/sound/pci/hda/patch_via.c
> +++ b/sound/pci/hda/patch_via.c
> @@ -118,7 +118,7 @@ static void via_playback_pcm_hook(struct hda_pcm_stream *hinfo,
>   				  struct hda_codec *codec,
>   				  struct snd_pcm_substream *substream,
>   				  int action);
> -static void via_hp_automute(struct hda_codec *codec, struct hda_jack_tbl *tbl);
> +static void via_hp_automute(struct hda_codec *codec, struct hda_jack_callback *tbl);
>
>   static struct via_spec *via_new_spec(struct hda_codec *codec)
>   {
> @@ -575,19 +575,22 @@ static const struct snd_kcontrol_new vt1708_jack_detect_ctl[] = {
>   	{} /* terminator */
>   };
>
> -static void via_hp_automute(struct hda_codec *codec, struct hda_jack_tbl *tbl)
> +static void via_hp_automute(struct hda_codec *codec,
> +			    struct hda_jack_callback *tbl)
>   {
>   	set_widgets_power_state(codec);
>   	snd_hda_gen_hp_automute(codec, tbl);
>   }
>
> -static void via_line_automute(struct hda_codec *codec, struct hda_jack_tbl *tbl)
> +static void via_line_automute(struct hda_codec *codec,
> +			      struct hda_jack_callback *tbl)
>   {
>   	set_widgets_power_state(codec);
>   	snd_hda_gen_line_automute(codec, tbl);
>   }
>
> -static void via_jack_powerstate_event(struct hda_codec *codec, struct hda_jack_tbl *tbl)
> +static void via_jack_powerstate_event(struct hda_codec *codec,
> +				      struct hda_jack_callback *tbl)
>   {
>   	set_widgets_power_state(codec);
>   }
>

-- 
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic

Re: [PATCH RFC 0/7] Allow multiple callbacks for hda_jack

[David Henningsson]

On 2014-09-11 16:19, Takashi Iwai wrote:
> Hi,
>
> this is a series of patches I quickly cooked up after the discussion
> in this morning: the support of multiple callbacks per jack.
>
> The series is applied on top of the previous fix patch (ALSA: hda -
> Fix invalid pin powermap without jack detection).  It begins with
> a couple of cleanups, then introduces the new hda_jack_callback
> struct and the changes along with it, then ends with another
> couple of cleanup patches based on the new infrastructure.
>
> I've tested only with a small set of devices, so far.

In general I like this idea and I remember thinking along the same lines.

I'm pondering whether we could use a more memory efficient layout for 
the callback list. Like allocating a snd_array on codec level and have 
indices to that list instead of pointers. Then the kernel would have 
less memory blocks to worry about. What do you think?

-- 
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic

Re: [PATCH RFC 3/7] ALSA: hda - Make snd_hda_jack_detect_enable_callbac() returning the jack object

[Takashi Iwai]

At Thu, 11 Sep 2014 16:44:40 +0200,
David Henningsson wrote:
> 
> 
> Nitpick: s/callbac/callback and s/returning/return in subject
> 
> On 2014-09-11 16:19, Takashi Iwai wrote:
> > STAC/IDT driver calls snd_hda_jack_tbl_get() again after calling
> > snd_hda_jack_detect_enable_callback().  For simplifying this, let's
> > make snd_hda_jack_detect_enable_callback() returning the pointer while
> > handling the error with the standard IS_ERR() & co.
> >
> > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> > ---
> >   sound/pci/hda/hda_jack.c       | 20 +++++++++++++-------
> >   sound/pci/hda/hda_jack.h       |  5 +++--
> >   sound/pci/hda/patch_sigmatel.c | 14 ++++++--------
> >   3 files changed, 22 insertions(+), 17 deletions(-)
> >
> > diff --git a/sound/pci/hda/hda_jack.c b/sound/pci/hda/hda_jack.c
> > index 7f332794993f..27fccd2c8d41 100644
> > --- a/sound/pci/hda/hda_jack.c
> > +++ b/sound/pci/hda/hda_jack.c
> > @@ -215,28 +215,34 @@ EXPORT_SYMBOL_GPL(snd_hda_jack_detect_state);
> >   /**
> >    * snd_hda_jack_detect_enable - enable the jack-detection
> >    */
> > -int snd_hda_jack_detect_enable_callback(struct hda_codec *codec, hda_nid_t nid,
> > -					hda_jack_callback cb)
> > +struct hda_jack_tbl *
> > +snd_hda_jack_detect_enable_callback(struct hda_codec *codec, hda_nid_t nid,
> > +				    hda_jack_callback cb)
> 
> I have not seen the ERR_PTR usage before and it's not used commonly in 
> the hda driver, so I think it's important to document that it can return 
> a non-null pointer that is not a valid hda_jack_tbl. Looks dangerous 
> otherwise.

Good point.  Will update the comment.


Takashi

Re: [PATCH RFC 4/7] ALSA: hda - Allow multiple callbacks for jack

[Takashi Iwai]

At Thu, 11 Sep 2014 17:01:01 +0200,
David Henningsson wrote:
> 
> 
> 
> On 2014-09-11 16:19, Takashi Iwai wrote:
> > So far, hda_jack infrastructure allows only one callback per jack, and
> > this makes things slightly complicated when a driver wants to assign
> > multiple tasks to a jack, e.g. the standard auto-mute with a power
> > up/down sequence.  This can be simplified if the hda_jack accepts
> > multiple callbacks.
> >
> > This patch is such an extension: the callback-specific part (the
> > function and private_data) is split to another struct from
> > hda_jack_tbl, and multiple such objects can be assigned to a single
> > hda_jack_tbl entry.
> >
> > The new struct hda_jack_callback is passed to each callback function
> > now, thus the patch became bigger than expected.  But these changes
> > are mostly trivial.
> >
> > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> > ---
> >   sound/pci/hda/hda_generic.c    | 19 ++++++++++++-------
> >   sound/pci/hda/hda_generic.h    | 12 ++++++------
> >   sound/pci/hda/hda_jack.c       | 43 +++++++++++++++++++++++++++++-------------
> >   sound/pci/hda/hda_jack.h       | 17 ++++++++++++-----
> >   sound/pci/hda/patch_cirrus.c   |  2 +-
> >   sound/pci/hda/patch_conexant.c |  3 ++-
> >   sound/pci/hda/patch_hdmi.c     | 14 ++++++++++----
> >   sound/pci/hda/patch_realtek.c  | 12 +++++++-----
> >   sound/pci/hda/patch_sigmatel.c | 24 +++++++++++------------
> >   sound/pci/hda/patch_via.c      | 11 +++++++----
> >   10 files changed, 99 insertions(+), 58 deletions(-)
> >
> > diff --git a/sound/pci/hda/hda_generic.c b/sound/pci/hda/hda_generic.c
> > index 4d605e4ac41c..32a85f9cac4b 100644
> > --- a/sound/pci/hda/hda_generic.c
> > +++ b/sound/pci/hda/hda_generic.c
> > @@ -2032,7 +2032,8 @@ static int create_speaker_out_ctls(struct hda_codec *codec)
> >    * independent HP controls
> >    */
> >
> > -static void call_hp_automute(struct hda_codec *codec, struct hda_jack_tbl *jack);
> > +static void call_hp_automute(struct hda_codec *codec,
> > +			     struct hda_jack_callback *jack);
> >   static int indep_hp_info(struct snd_kcontrol *kcontrol,
> >   			 struct snd_ctl_elem_info *uinfo)
> >   {
> > @@ -3948,7 +3949,8 @@ static void call_update_outputs(struct hda_codec *codec)
> >   }
> >
> >   /* standard HP-automute helper */
> > -void snd_hda_gen_hp_automute(struct hda_codec *codec, struct hda_jack_tbl *jack)
> > +void snd_hda_gen_hp_automute(struct hda_codec *codec,
> > +			     struct hda_jack_callback *jack)
> >   {
> >   	struct hda_gen_spec *spec = codec->spec;
> >   	hda_nid_t *pins = spec->autocfg.hp_pins;
> > @@ -3968,7 +3970,8 @@ void snd_hda_gen_hp_automute(struct hda_codec *codec, struct hda_jack_tbl *jack)
> >   EXPORT_SYMBOL_GPL(snd_hda_gen_hp_automute);
> >
> >   /* standard line-out-automute helper */
> > -void snd_hda_gen_line_automute(struct hda_codec *codec, struct hda_jack_tbl *jack)
> > +void snd_hda_gen_line_automute(struct hda_codec *codec,
> > +			       struct hda_jack_callback *jack)
> >   {
> >   	struct hda_gen_spec *spec = codec->spec;
> >
> > @@ -3988,7 +3991,8 @@ void snd_hda_gen_line_automute(struct hda_codec *codec, struct hda_jack_tbl *jac
> >   EXPORT_SYMBOL_GPL(snd_hda_gen_line_automute);
> >
> >   /* standard mic auto-switch helper */
> > -void snd_hda_gen_mic_autoswitch(struct hda_codec *codec, struct hda_jack_tbl *jack)
> > +void snd_hda_gen_mic_autoswitch(struct hda_codec *codec,
> > +				struct hda_jack_callback *jack)
> >   {
> >   	struct hda_gen_spec *spec = codec->spec;
> >   	int i;
> > @@ -4011,7 +4015,8 @@ void snd_hda_gen_mic_autoswitch(struct hda_codec *codec, struct hda_jack_tbl *ja
> >   EXPORT_SYMBOL_GPL(snd_hda_gen_mic_autoswitch);
> >
> >   /* call appropriate hooks */
> > -static void call_hp_automute(struct hda_codec *codec, struct hda_jack_tbl *jack)
> > +static void call_hp_automute(struct hda_codec *codec,
> > +			     struct hda_jack_callback *jack)
> >   {
> >   	struct hda_gen_spec *spec = codec->spec;
> >   	if (spec->hp_automute_hook)
> > @@ -4021,7 +4026,7 @@ static void call_hp_automute(struct hda_codec *codec, struct hda_jack_tbl *jack)
> >   }
> >
> >   static void call_line_automute(struct hda_codec *codec,
> > -			       struct hda_jack_tbl *jack)
> > +			       struct hda_jack_callback *jack)
> >   {
> >   	struct hda_gen_spec *spec = codec->spec;
> >   	if (spec->line_automute_hook)
> > @@ -4031,7 +4036,7 @@ static void call_line_automute(struct hda_codec *codec,
> >   }
> >
> >   static void call_mic_autoswitch(struct hda_codec *codec,
> > -				struct hda_jack_tbl *jack)
> > +				struct hda_jack_callback *jack)
> >   {
> >   	struct hda_gen_spec *spec = codec->spec;
> >   	if (spec->mic_autoswitch_hook)
> > diff --git a/sound/pci/hda/hda_generic.h b/sound/pci/hda/hda_generic.h
> > index 72f5624125fb..61dd5153f512 100644
> > --- a/sound/pci/hda/hda_generic.h
> > +++ b/sound/pci/hda/hda_generic.h
> > @@ -284,11 +284,11 @@ struct hda_gen_spec {
> >
> >   	/* automute / autoswitch hooks */
> >   	void (*hp_automute_hook)(struct hda_codec *codec,
> > -				 struct hda_jack_tbl *tbl);
> > +				 struct hda_jack_callback *cb);
> >   	void (*line_automute_hook)(struct hda_codec *codec,
> > -				   struct hda_jack_tbl *tbl);
> > +				   struct hda_jack_callback *cb);
> >   	void (*mic_autoswitch_hook)(struct hda_codec *codec,
> > -				    struct hda_jack_tbl *tbl);
> > +				    struct hda_jack_callback *cb);
> >   };
> >
> >   int snd_hda_gen_spec_init(struct hda_gen_spec *spec);
> > @@ -320,11 +320,11 @@ int snd_hda_gen_build_pcms(struct hda_codec *codec);
> >
> >   /* standard jack event callbacks */
> >   void snd_hda_gen_hp_automute(struct hda_codec *codec,
> > -			     struct hda_jack_tbl *jack);
> > +			     struct hda_jack_callback *jack);
> >   void snd_hda_gen_line_automute(struct hda_codec *codec,
> > -			       struct hda_jack_tbl *jack);
> > +			       struct hda_jack_callback *jack);
> >   void snd_hda_gen_mic_autoswitch(struct hda_codec *codec,
> > -				struct hda_jack_tbl *jack);
> > +				struct hda_jack_callback *jack);
> >   void snd_hda_gen_update_outputs(struct hda_codec *codec);
> >
> >   #ifdef CONFIG_PM
> > diff --git a/sound/pci/hda/hda_jack.c b/sound/pci/hda/hda_jack.c
> > index 27fccd2c8d41..13a9e6796379 100644
> > --- a/sound/pci/hda/hda_jack.c
> > +++ b/sound/pci/hda/hda_jack.c
> > @@ -117,8 +117,13 @@ void snd_hda_jack_tbl_clear(struct hda_codec *codec)
> >   		struct hda_jack_tbl *jack = codec->jacktbl.list;
> >   		int i;
> >   		for (i = 0; i < codec->jacktbl.used; i++, jack++) {
> > +			struct hda_jack_callback *cb, *next;
> >   			if (jack->jack)
> >   				snd_device_free(codec->bus->card, jack->jack);
> > +			for (cb = jack->callback; cb; cb = next) {
> > +				next = cb->next;
> > +				kfree(cb);
> > +			}
> >   		}
> >   	}
> >   #endif
> > @@ -215,28 +220,36 @@ EXPORT_SYMBOL_GPL(snd_hda_jack_detect_state);
> >   /**
> >    * snd_hda_jack_detect_enable - enable the jack-detection
> >    */
> > -struct hda_jack_tbl *
> > +struct hda_jack_callback *
> >   snd_hda_jack_detect_enable_callback(struct hda_codec *codec, hda_nid_t nid,
> > -				    hda_jack_callback cb)
> > +				    hda_jack_callback_fn func)
> >   {
> > -	struct hda_jack_tbl *jack = snd_hda_jack_tbl_new(codec, nid);
> > +	struct hda_jack_tbl *jack;
> > +	struct hda_jack_callback *callback = NULL;
> >   	int err;
> >
> > +	jack = snd_hda_jack_tbl_new(codec, nid);
> >   	if (!jack)
> >   		return ERR_PTR(-ENOMEM);
> 
> > -	if (jack->jack_detect)
> > -		return jack; /* already registered */
> 
> This check is an optimisation (do not write unsol verbs to the codec 
> more than once, even if many callbacks are added) and should not be 
> removed. The "if (func) " part should just move above the check.

Right, it makes sense.


Takashi

Re: [PATCH RFC 0/7] Allow multiple callbacks for hda_jack

[Takashi Iwai]

At Thu, 11 Sep 2014 17:14:00 +0200,
David Henningsson wrote:
> 
> 
> 
> On 2014-09-11 16:19, Takashi Iwai wrote:
> > Hi,
> >
> > this is a series of patches I quickly cooked up after the discussion
> > in this morning: the support of multiple callbacks per jack.
> >
> > The series is applied on top of the previous fix patch (ALSA: hda -
> > Fix invalid pin powermap without jack detection).  It begins with
> > a couple of cleanups, then introduces the new hda_jack_callback
> > struct and the changes along with it, then ends with another
> > couple of cleanup patches based on the new infrastructure.
> >
> > I've tested only with a small set of devices, so far.
> 
> In general I like this idea and I remember thinking along the same lines.
> 
> I'm pondering whether we could use a more memory efficient layout for 
> the callback list. Like allocating a snd_array on codec level and have 
> indices to that list instead of pointers. Then the kernel would have 
> less memory blocks to worry about. What do you think?

I don't think the memory usage would be any problem in this case as
it's just a few numbers of small blocks.  The only question is which
is better manageable in the source code level.  Let's see...


Takashi

Re: [PATCH RFC 0/7] Allow multiple callbacks for hda_jack

[Takashi Iwai]

At Mon, 15 Sep 2014 10:42:21 +0200,
Takashi Iwai wrote:
> 
> At Thu, 11 Sep 2014 17:14:00 +0200,
> David Henningsson wrote:
> > 
> > 
> > 
> > On 2014-09-11 16:19, Takashi Iwai wrote:
> > > Hi,
> > >
> > > this is a series of patches I quickly cooked up after the discussion
> > > in this morning: the support of multiple callbacks per jack.
> > >
> > > The series is applied on top of the previous fix patch (ALSA: hda -
> > > Fix invalid pin powermap without jack detection).  It begins with
> > > a couple of cleanups, then introduces the new hda_jack_callback
> > > struct and the changes along with it, then ends with another
> > > couple of cleanup patches based on the new infrastructure.
> > >
> > > I've tested only with a small set of devices, so far.
> > 
> > In general I like this idea and I remember thinking along the same lines.
> > 
> > I'm pondering whether we could use a more memory efficient layout for 
> > the callback list. Like allocating a snd_array on codec level and have 
> > indices to that list instead of pointers. Then the kernel would have 
> > less memory blocks to worry about. What do you think?
> 
> I don't think the memory usage would be any problem in this case as
> it's just a few numbers of small blocks.  The only question is which
> is better manageable in the source code level.  Let's see...

I tried hacking with snd_array, but this ended up more complexity in
the code (either adding an extra stuff into struct hda_codec or
obviously more overhead than the simple kmalloc).  So, I decided to
keep the code as it was.

If you find a better solution, let me know.  In anyway, I'll submit v2
patches.


thanks,

Takashi

Re: [PATCH RFC 0/7] Allow multiple callbacks for hda_jack

[David Henningsson]

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



On 2014-09-15 14:11, Takashi Iwai wrote:
> At Mon, 15 Sep 2014 10:42:21 +0200,
> Takashi Iwai wrote:
>>
>> At Thu, 11 Sep 2014 17:14:00 +0200,
>> David Henningsson wrote:
>>>
>>>
>>>
>>> On 2014-09-11 16:19, Takashi Iwai wrote:
>>>> Hi,
>>>>
>>>> this is a series of patches I quickly cooked up after the discussion
>>>> in this morning: the support of multiple callbacks per jack.
>>>>
>>>> The series is applied on top of the previous fix patch (ALSA: hda -
>>>> Fix invalid pin powermap without jack detection).  It begins with
>>>> a couple of cleanups, then introduces the new hda_jack_callback
>>>> struct and the changes along with it, then ends with another
>>>> couple of cleanup patches based on the new infrastructure.
>>>>
>>>> I've tested only with a small set of devices, so far.
>>>
>>> In general I like this idea and I remember thinking along the same lines.
>>>
>>> I'm pondering whether we could use a more memory efficient layout for
>>> the callback list. Like allocating a snd_array on codec level and have
>>> indices to that list instead of pointers. Then the kernel would have
>>> less memory blocks to worry about. What do you think?
>>
>> I don't think the memory usage would be any problem in this case as
>> it's just a few numbers of small blocks.  The only question is which
>> is better manageable in the source code level.  Let's see...
>
> I tried hacking with snd_array, but this ended up more complexity in
> the code (either adding an extra stuff into struct hda_codec or
> obviously more overhead than the simple kmalloc).  So, I decided to
> keep the code as it was.
>
> If you find a better solution, let me know.  In anyway, I'll submit v2
> patches.

The attached version is a quick write-up of what I was thinking. 
(Untested, apply on top of patch 4/7.) Whether its better or worse is a 
matter of taste I suppose - the iterating over the callbacks is a little 
less compact, but OTOH the freeing is slightly simpler, and there are 
also less calls to malloc.

Btw, in your version, the current free in snd_hda_jack_tbl_clear is 
within CONFIG_SND_HDA_INPUT_JACK ifdef. Looks like a memory leak if 
CONFIG_SND_HDA_INPUT_JACK is not defined.


-- 
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic

[-- Attachment #2: jack.diff --]
[-- Type: text/x-patch, Size: 3954 bytes --]

diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
index 0aa2e1e..5a88776 100644
--- a/sound/pci/hda/hda_codec.c
+++ b/sound/pci/hda/hda_codec.c
@@ -1471,6 +1471,7 @@ int snd_hda_codec_new(struct hda_bus *bus,
 	snd_array_init(&codec->cvt_setups, sizeof(struct hda_cvt_setup), 8);
 	snd_array_init(&codec->spdif_out, sizeof(struct hda_spdif_out), 16);
 	snd_array_init(&codec->jacktbl, sizeof(struct hda_jack_tbl), 16);
+	snd_array_init(&codec->jack_cb_tbl, sizeof(struct hda_jack_callback), 16);
 	snd_array_init(&codec->verbs, sizeof(struct hda_verb *), 8);
 	INIT_LIST_HEAD(&codec->conn_list);
 
diff --git a/sound/pci/hda/hda_codec.h b/sound/pci/hda/hda_codec.h
index 9c8820f..746cb7c 100644
--- a/sound/pci/hda/hda_codec.h
+++ b/sound/pci/hda/hda_codec.h
@@ -390,6 +390,7 @@ struct hda_codec {
 
 	/* jack detection */
 	struct snd_array jacktbl;
+	struct snd_array jack_cb_tbl;
 	unsigned long jackpoll_interval; /* In jiffies. Zero means no poll, rely on unsol events */
 	struct delayed_work jackpoll_work;
 
diff --git a/sound/pci/hda/hda_jack.c b/sound/pci/hda/hda_jack.c
index cbaa5bf..9336f41 100644
--- a/sound/pci/hda/hda_jack.c
+++ b/sound/pci/hda/hda_jack.c
@@ -105,6 +105,7 @@ snd_hda_jack_tbl_new(struct hda_codec *codec, hda_nid_t nid)
 		return NULL;
 	jack->nid = nid;
 	jack->jack_dirty = 1;
+	jack->callback_idx = -1;
 	jack->tag = codec->jacktbl.used;
 	return jack;
 }
@@ -118,17 +119,13 @@ void snd_hda_jack_tbl_clear(struct hda_codec *codec)
 		struct hda_jack_tbl *jack = codec->jacktbl.list;
 		int i;
 		for (i = 0; i < codec->jacktbl.used; i++, jack++) {
-			struct hda_jack_callback *cb, *next;
 			if (jack->jack)
 				snd_device_free(codec->bus->card, jack->jack);
-			for (cb = jack->callback; cb; cb = next) {
-				next = cb->next;
-				kfree(cb);
-			}
 		}
 	}
 #endif
 	snd_array_free(&codec->jacktbl);
+	snd_array_free(&codec->jack_cb_tbl);
 }
 
 #define get_jack_plug_state(sense) !!(sense & AC_PINSENSE_PRESENCE)
@@ -237,13 +234,13 @@ snd_hda_jack_detect_enable_callback(struct hda_codec *codec, hda_nid_t nid,
 	if (!jack)
 		return ERR_PTR(-ENOMEM);
 	if (func) {
-		callback = kzalloc(sizeof(*callback), GFP_KERNEL);
+		callback = snd_array_new(&codec->jack_cb_tbl);
 		if (!callback)
 			return ERR_PTR(-ENOMEM);
 		callback->func = func;
 		callback->tbl = jack;
-		callback->next = jack->callback;
-		jack->callback = callback;
+		callback->next = jack->callback_idx;
+		jack->callback_idx = snd_array_index(&codec->jack_cb_tbl, callback);
 	}
 
 	if (jack->jack_detect)
@@ -519,16 +516,24 @@ EXPORT_SYMBOL_GPL(snd_hda_jack_add_kctls);
 static void call_jack_callback(struct hda_codec *codec,
 			       struct hda_jack_tbl *jack)
 {
+	int cb_idx = jack->callback_idx;
 	struct hda_jack_callback *cb;
 
-	for (cb = jack->callback; cb; cb = cb->next)
+	while (cb_idx != -1) {
+		cb = snd_array_elem(&codec->jack_cb_tbl, cb_idx);
 		cb->func(codec, cb);
+		cb_idx = cb->next;
+	}
 	if (jack->gated_jack) {
 		struct hda_jack_tbl *gated =
 			snd_hda_jack_tbl_get(codec, jack->gated_jack);
 		if (gated) {
-			for (cb = gated->callback; cb; cb = cb->next)
+			cb_idx = gated->callback_idx;
+			while (cb_idx != -1) {
+				cb = snd_array_elem(&codec->jack_cb_tbl, cb_idx);
 				cb->func(codec, cb);
+				cb_idx = cb->next;
+			}
 		}
 	}
 }
diff --git a/sound/pci/hda/hda_jack.h b/sound/pci/hda/hda_jack.h
index 2784cf5..c634f4e 100644
--- a/sound/pci/hda/hda_jack.h
+++ b/sound/pci/hda/hda_jack.h
@@ -22,13 +22,13 @@ struct hda_jack_callback {
 	struct hda_jack_tbl *tbl;
 	hda_jack_callback_fn func;
 	unsigned int private_data;	/* arbitrary data */
-	struct hda_jack_callback *next;
+	int next;
 };
 
 struct hda_jack_tbl {
 	hda_nid_t nid;
 	unsigned char tag;		/* unsol event tag */
-	struct hda_jack_callback *callback;
+	int callback_idx;
 	/* jack-detection stuff */
 	unsigned int pin_sense;		/* cached pin-sense value */
 	unsigned int jack_detect:1;	/* capable of jack-detection? */

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

Re: [PATCH RFC 0/7] Allow multiple callbacks for hda_jack

[Takashi Iwai]

At Tue, 16 Sep 2014 10:33:30 +0200,
David Henningsson wrote:
> 
> 
> 
> On 2014-09-15 14:11, Takashi Iwai wrote:
> > At Mon, 15 Sep 2014 10:42:21 +0200,
> > Takashi Iwai wrote:
> >>
> >> At Thu, 11 Sep 2014 17:14:00 +0200,
> >> David Henningsson wrote:
> >>>
> >>>
> >>>
> >>> On 2014-09-11 16:19, Takashi Iwai wrote:
> >>>> Hi,
> >>>>
> >>>> this is a series of patches I quickly cooked up after the discussion
> >>>> in this morning: the support of multiple callbacks per jack.
> >>>>
> >>>> The series is applied on top of the previous fix patch (ALSA: hda -
> >>>> Fix invalid pin powermap without jack detection).  It begins with
> >>>> a couple of cleanups, then introduces the new hda_jack_callback
> >>>> struct and the changes along with it, then ends with another
> >>>> couple of cleanup patches based on the new infrastructure.
> >>>>
> >>>> I've tested only with a small set of devices, so far.
> >>>
> >>> In general I like this idea and I remember thinking along the same lines.
> >>>
> >>> I'm pondering whether we could use a more memory efficient layout for
> >>> the callback list. Like allocating a snd_array on codec level and have
> >>> indices to that list instead of pointers. Then the kernel would have
> >>> less memory blocks to worry about. What do you think?
> >>
> >> I don't think the memory usage would be any problem in this case as
> >> it's just a few numbers of small blocks.  The only question is which
> >> is better manageable in the source code level.  Let's see...
> >
> > I tried hacking with snd_array, but this ended up more complexity in
> > the code (either adding an extra stuff into struct hda_codec or
> > obviously more overhead than the simple kmalloc).  So, I decided to
> > keep the code as it was.
> >
> > If you find a better solution, let me know.  In anyway, I'll submit v2
> > patches.
> 
> The attached version is a quick write-up of what I was thinking. 
> (Untested, apply on top of patch 4/7.) Whether its better or worse is a 
> matter of taste I suppose - the iterating over the callbacks is a little 
> less compact, but OTOH the freeing is slightly simpler, and there are 
> also less calls to malloc.

Thanks, it's surprisingly identical with what I've wrote for
comparison ;)  And I didn't like to put a new field to hda_codec.

> Btw, in your version, the current free in snd_hda_jack_tbl_clear is 
> within CONFIG_SND_HDA_INPUT_JACK ifdef. Looks like a memory leak if 
> CONFIG_SND_HDA_INPUT_JACK is not defined.

Good catch.  I'll fix that.


thanks,

Takashi