[alsa-devel] [PATCH v2 0/2] ALSA: Implement core jack support for kcontrol
From: "Felipe F. Tonello" eu@felipetonello.com
On this second version I made sure that the patch series can be bisected. Also, since HDA codec has it's own jack infrastructure, I just disabled the support of the core ALSA jack in it. Why? Because it's causing duplication of controls, although the input events works fine. A HDA codec developer might give us more information about it.
Patch 1) Is the actual implementation and fixes for users that uses the API. Patch 2) Name fixes. Just to make sure no jack has " Jack" duplicated at the end of its name.
Felipe F. Tonello (2): ALSA: Added jack detection KControl support ALSA: SoC: Fix jack names according new API
include/sound/control.h | 2 +- include/sound/jack.h | 8 ++-- include/sound/soc.h | 2 +- sound/core/Kconfig | 1 + sound/core/ctljack.c | 15 ++++++- sound/core/jack.c | 88 +++++++++++++++++++++++++++++++++++--- sound/pci/hda/Kconfig | 8 ---- sound/pci/oxygen/xonar_wm87x6.c | 2 +- sound/soc/fsl/wm1133-ev1.c | 4 +- sound/soc/mid-x86/mfld_machine.c | 6 +-- sound/soc/omap/ams-delta.c | 2 +- sound/soc/omap/omap-abe-twl6040.c | 4 +- sound/soc/omap/omap-twl4030.c | 4 +- sound/soc/omap/rx51.c | 6 +-- sound/soc/pxa/hx4700.c | 4 +- sound/soc/pxa/palm27x.c | 4 +- sound/soc/pxa/ttc-dkb.c | 10 ++--- sound/soc/pxa/z2.c | 4 +- sound/soc/samsung/goni_wm8994.c | 7 +-- sound/soc/samsung/h1940_uda1380.c | 4 +- sound/soc/samsung/littlemill.c | 10 ++--- sound/soc/samsung/lowland.c | 6 +-- sound/soc/samsung/rx1950_uda1380.c | 4 +- sound/soc/samsung/smartq_wm8987.c | 4 +- sound/soc/samsung/speyside.c | 6 +-- sound/soc/samsung/tobermory.c | 4 +- sound/soc/soc-jack.c | 5 ++- sound/soc/tegra/tegra_alc5632.c | 4 +- sound/soc/tegra/tegra_rt5640.c | 4 +- sound/soc/tegra/tegra_wm8903.c | 8 ++-- 30 files changed, 163 insertions(+), 77 deletions(-)
From: "Felipe F. Tonello" eu@felipetonello.com
This patch adds jack support for ALSA KControl.
This support is necessary since the new kcontrol is used by user-space daemons, such as PulseAudio(>=2.0), to do jack detection.)
Also, HDA's CONFIG_SND_HDA_INPUT_JACK is disabled because it causes to use standard snd_jack_new() to create jacks. This can cause conflict since this codec creates jack controls directly.
It makes sure that all codecs using ALSA jack API are updated to use the new API.
Signed-off-by: Felipe F. Tonello eu@felipetonello.com --- include/sound/control.h | 2 +- include/sound/jack.h | 8 ++-- include/sound/soc.h | 2 +- sound/core/Kconfig | 1 + sound/core/ctljack.c | 15 ++++++- sound/core/jack.c | 88 +++++++++++++++++++++++++++++++++++--- sound/pci/hda/Kconfig | 8 ---- sound/pci/oxygen/xonar_wm87x6.c | 2 +- sound/soc/fsl/wm1133-ev1.c | 4 +- sound/soc/mid-x86/mfld_machine.c | 4 +- sound/soc/omap/ams-delta.c | 2 +- sound/soc/omap/omap-abe-twl6040.c | 2 +- sound/soc/omap/omap-twl4030.c | 2 +- sound/soc/omap/rx51.c | 4 +- sound/soc/pxa/hx4700.c | 2 +- sound/soc/pxa/palm27x.c | 2 +- sound/soc/pxa/ttc-dkb.c | 6 +-- sound/soc/pxa/z2.c | 2 +- sound/soc/samsung/goni_wm8994.c | 5 ++- sound/soc/samsung/h1940_uda1380.c | 2 +- sound/soc/samsung/littlemill.c | 10 ++--- sound/soc/samsung/lowland.c | 6 +-- sound/soc/samsung/rx1950_uda1380.c | 2 +- sound/soc/samsung/smartq_wm8987.c | 2 +- sound/soc/samsung/speyside.c | 6 +-- sound/soc/samsung/tobermory.c | 4 +- sound/soc/soc-jack.c | 5 ++- sound/soc/tegra/tegra_alc5632.c | 2 +- sound/soc/tegra/tegra_rt5640.c | 2 +- sound/soc/tegra/tegra_wm8903.c | 4 +- 30 files changed, 146 insertions(+), 60 deletions(-)
diff --git a/include/sound/control.h b/include/sound/control.h index 5358892..fec7d2b 100644 --- a/include/sound/control.h +++ b/include/sound/control.h @@ -242,6 +242,6 @@ void snd_ctl_sync_vmaster(struct snd_kcontrol *kctl, bool hook_only); struct snd_kcontrol * snd_kctl_jack_new(const char *name, int idx, void *private_data); void snd_kctl_jack_report(struct snd_card *card, - struct snd_kcontrol *kctl, bool status); + struct snd_kcontrol *kctl, int status);
#endif /* __SOUND_CONTROL_H */ diff --git a/include/sound/jack.h b/include/sound/jack.h index 5891657..0d36f20 100644 --- a/include/sound/jack.h +++ b/include/sound/jack.h @@ -26,6 +26,7 @@ #include <sound/core.h>
struct input_dev; +struct snd_kcontrol;
/** * Jack types which can be reported. These values are used as a @@ -58,11 +59,12 @@ enum snd_jack_types {
struct snd_jack { struct input_dev *input_dev; + struct snd_kcontrol *kctl[SND_JACK_SWITCH_TYPES]; /* control for each key */ int registered; int type; const char *id; char name[100]; - unsigned int key[6]; /* Keep in sync with definitions above */ + unsigned int key[SND_JACK_SWITCH_TYPES]; /* Keep in sync with definitions above */ void *private_data; void (*private_free)(struct snd_jack *); }; @@ -70,7 +72,7 @@ struct snd_jack { #ifdef CONFIG_SND_JACK
int snd_jack_new(struct snd_card *card, const char *id, int type, - struct snd_jack **jack); + int idx, struct snd_jack **jack); void snd_jack_set_parent(struct snd_jack *jack, struct device *parent); int snd_jack_set_key(struct snd_jack *jack, enum snd_jack_types type, int keytype); @@ -80,7 +82,7 @@ void snd_jack_report(struct snd_jack *jack, int status); #else
static inline int snd_jack_new(struct snd_card *card, const char *id, int type, - struct snd_jack **jack) + int idx, struct snd_jack **jack) { return 0; } diff --git a/include/sound/soc.h b/include/sound/soc.h index 6eabee7..31bea52 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -436,7 +436,7 @@ int snd_soc_platform_trigger(struct snd_pcm_substream *substream,
/* Jack reporting */ int snd_soc_jack_new(struct snd_soc_codec *codec, const char *id, int type, - struct snd_soc_jack *jack); + int idx, struct snd_soc_jack *jack); void snd_soc_jack_report(struct snd_soc_jack *jack, int status, int mask); int snd_soc_jack_add_pins(struct snd_soc_jack *jack, int count, struct snd_soc_jack_pin *pins); diff --git a/sound/core/Kconfig b/sound/core/Kconfig index c0c2f57..8167615 100644 --- a/sound/core/Kconfig +++ b/sound/core/Kconfig @@ -20,6 +20,7 @@ config SND_COMPRESS_OFFLOAD # to avoid having to force INPUT on. config SND_JACK bool + select SND_KCTL_JACK
config SND_SEQUENCER tristate "Sequencer support" diff --git a/sound/core/ctljack.c b/sound/core/ctljack.c index e4b38fb..be1f738 100644 --- a/sound/core/ctljack.c +++ b/sound/core/ctljack.c @@ -14,7 +14,17 @@ #include <sound/core.h> #include <sound/control.h>
-#define jack_detect_kctl_info snd_ctl_boolean_mono_info +#define jack_detect_kctl_info jack_ctl_integer_info + +int jack_ctl_integer_info(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_info *uinfo) +{ + uinfo->type = SNDRV_CTL_ELEM_TYPE_INTEGER; + uinfo->count = 0x10000U; + uinfo->value.integer.min = 0; + uinfo->value.integer.max = 0xffff; + return 0; +}
static int jack_detect_kctl_get(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol) @@ -38,6 +48,7 @@ snd_kctl_jack_new(const char *name, int idx, void *private_data) kctl = snd_ctl_new1(&jack_detect_kctl, private_data); if (!kctl) return NULL; + snprintf(kctl->id.name, sizeof(kctl->id.name), "%s Jack", name); kctl->id.index = idx; kctl->private_value = 0; @@ -46,7 +57,7 @@ snd_kctl_jack_new(const char *name, int idx, void *private_data) EXPORT_SYMBOL_GPL(snd_kctl_jack_new);
void snd_kctl_jack_report(struct snd_card *card, - struct snd_kcontrol *kctl, bool status) + struct snd_kcontrol *kctl, int status) { if (kctl->private_value == status) return; diff --git a/sound/core/jack.c b/sound/core/jack.c index b35fe73..128154b 100644 --- a/sound/core/jack.c +++ b/sound/core/jack.c @@ -24,6 +24,7 @@ #include <linux/module.h> #include <sound/jack.h> #include <sound/core.h> +#include <sound/control.h>
static int jack_switch_types[SND_JACK_SWITCH_TYPES] = { SW_HEADPHONE_INSERT, @@ -37,6 +38,7 @@ static int jack_switch_types[SND_JACK_SWITCH_TYPES] = { static int snd_jack_dev_free(struct snd_device *device) { struct snd_jack *jack = device->device_data; + int i;
if (jack->private_free) jack->private_free(jack); @@ -48,19 +50,54 @@ static int snd_jack_dev_free(struct snd_device *device) else input_free_device(jack->input_dev);
+ /* Free available KControls*/ + for (i = 0; i < SND_JACK_SWITCH_TYPES; ++i) + if (jack->kctl[i]) + snd_ctl_remove(device->card, jack->kctl[i]); + kfree(jack->id); kfree(jack);
return 0; }
+const char * jack_get_name_by_key(const char *name, int key) +{ + char *jack_name; + size_t jack_name_size; + const char *key_name; + + switch(key) { + case SW_HEADPHONE_INSERT: key_name = "Headphone"; break; + case SW_MICROPHONE_INSERT: key_name = "Mic"; break; + case SW_LINEOUT_INSERT: key_name = "Line Out"; break; + case SW_JACK_PHYSICAL_INSERT: key_name = "Mechanical"; break; + case SW_VIDEOOUT_INSERT: key_name = "Video Out"; break; + case SW_LINEIN_INSERT: key_name = "Line In"; break; + default: key_name = "Unknown"; + } + + /* Avoid duplicate name in KControl */ + if (strcmp(name, key_name) != 0) { + /* allocate necessary memory space only */ + jack_name_size = strlen(name) + strlen(key_name) + 4; + jack_name = kmalloc(jack_name_size, GFP_KERNEL); + + snprintf(jack_name, jack_name_size, "%s (%s)", name, key_name); + } else { + jack_name = (char *)name; + } + + return jack_name; +} + static int snd_jack_dev_register(struct snd_device *device) { struct snd_jack *jack = device->device_data; struct snd_card *card = device->card; int err, i;
- snprintf(jack->name, sizeof(jack->name), "%s %s", + snprintf(jack->name, sizeof(jack->name), "%s %s Jack", card->shortname, jack->id); jack->input_dev->name = jack->name;
@@ -81,6 +118,18 @@ static int snd_jack_dev_register(struct snd_device *device) input_set_capability(jack->input_dev, EV_KEY, jack->key[i]); }
+ /* We don't need to free the control, it's freed by snd_ctl_add itself + if an error occur */ + for (i = 0; i < SND_JACK_SWITCH_TYPES; ++i) + if (jack->kctl[i]) { + err = snd_ctl_add(card, jack->kctl[i]); + if (err < 0) { + pr_notice("%s: ALSA Jack Control not available for '%s'\n", __func__, + jack_get_name_by_key(jack->id, jack_switch_types[i])); + jack->kctl[i] = NULL; + } + } + err = input_register_device(jack->input_dev); if (err == 0) jack->registered = 1; @@ -91,22 +140,31 @@ static int snd_jack_dev_register(struct snd_device *device) /** * snd_jack_new - Create a new jack * @card: the card instance - * @id: an identifying string for this jack + * @id: an identifying string for this jack, " Jack" is appended to the + * string * @type: a bitmask of enum snd_jack_type values that can be detected by * this jack + * @idx: The index of the ALSA control created to represent the jack. * @jjack: Used to provide the allocated jack object to the caller. * * Creates a new jack object. * + * This function creates a Jack Kcontrol for each @type id as "@id @type Jack". + * Eg.: A jack with @type SND_JACK_HEADSET will have two KControls created, + * "@id Headphone Jack" and "@id Mic Jack". If @id and @type strings are + * equals, then this KControl will be named "@id Jack" only. + * * Return: Zero if successful, or a negative error code on failure. * On success @jjack will be initialised. */ int snd_jack_new(struct snd_card *card, const char *id, int type, - struct snd_jack **jjack) + int idx, struct snd_jack **jjack) { struct snd_jack *jack; + struct snd_kcontrol *kctl; int err; int i; + static struct snd_device_ops ops = { .dev_free = snd_jack_dev_free, .dev_register = snd_jack_dev_register, @@ -129,10 +187,20 @@ int snd_jack_new(struct snd_card *card, const char *id, int type, jack->type = type;
for (i = 0; i < SND_JACK_SWITCH_TYPES; i++) - if (type & (1 << i)) + if (type & (1 << i)) { input_set_capability(jack->input_dev, EV_SW, jack_switch_types[i]);
+ /* card is the private_data */ + kctl = snd_kctl_jack_new(jack_get_name_by_key(id, jack_switch_types[i]), idx, card); + if (!kctl) { + err = -ENOMEM; + goto fail_kctl; + } + + jack->kctl[i] = kctl; + } + err = snd_device_new(card, SNDRV_DEV_JACK, jack, &ops); if (err < 0) goto fail_input; @@ -141,6 +209,11 @@ int snd_jack_new(struct snd_card *card, const char *id, int type,
return 0;
+fail_kctl: + for (i = 0; i < SND_JACK_SWITCH_TYPES; ++i) + if (jack->kctl[i]) + snd_ctl_remove(card, jack->kctl[i]); + fail_input: input_free_device(jack->input_dev); kfree(jack->id); @@ -232,10 +305,15 @@ void snd_jack_report(struct snd_jack *jack, int status)
for (i = 0; i < ARRAY_SIZE(jack_switch_types); i++) { int testbit = 1 << i; - if (jack->type & testbit) + if (jack->type & testbit) { input_report_switch(jack->input_dev, jack_switch_types[i], status & testbit); + + /* Update ALSA KControl interface */ + snd_kctl_jack_report((struct snd_card *)jack->kctl[i]->private_data, + jack->kctl[i], status & testbit); + } }
input_sync(jack->input_dev); diff --git a/sound/pci/hda/Kconfig b/sound/pci/hda/Kconfig index 59c5e9c..561abc7 100644 --- a/sound/pci/hda/Kconfig +++ b/sound/pci/hda/Kconfig @@ -65,14 +65,6 @@ config SND_HDA_INPUT_BEEP_MODE Set 1 to always enable the digital beep interface for HD-audio by default.
-config SND_HDA_INPUT_JACK - bool "Support jack plugging notification via input layer" - depends on INPUT=y || INPUT=SND - select SND_JACK - help - Say Y here to enable the jack plugging notification via - input layer. - config SND_HDA_PATCH_LOADER bool "Support initialization patch loading for HD-audio" select FW_LOADER diff --git a/sound/pci/oxygen/xonar_wm87x6.c b/sound/pci/oxygen/xonar_wm87x6.c index 6ce6860..d3816b1 100644 --- a/sound/pci/oxygen/xonar_wm87x6.c +++ b/sound/pci/oxygen/xonar_wm87x6.c @@ -286,7 +286,7 @@ static void xonar_ds_init(struct oxygen *chip) xonar_enable_output(chip);
snd_jack_new(chip->card, "Headphone", - SND_JACK_HEADPHONE, &data->hp_jack); + SND_JACK_HEADPHONE, 0, &data->hp_jack); xonar_ds_handle_hp_jack(chip);
snd_component_add(chip->card, "WM8776"); diff --git a/sound/soc/fsl/wm1133-ev1.c b/sound/soc/fsl/wm1133-ev1.c index fce6325..50f96d4 100644 --- a/sound/soc/fsl/wm1133-ev1.c +++ b/sound/soc/fsl/wm1133-ev1.c @@ -221,14 +221,14 @@ static int wm1133_ev1_init(struct snd_soc_pcm_runtime *rtd) ARRAY_SIZE(wm1133_ev1_map));
/* Headphone jack detection */ - snd_soc_jack_new(codec, "Headphone", SND_JACK_HEADPHONE, &hp_jack); + snd_soc_jack_new(codec, "Headphone", SND_JACK_HEADPHONE, 0, &hp_jack); snd_soc_jack_add_pins(&hp_jack, ARRAY_SIZE(hp_jack_pins), hp_jack_pins); wm8350_hp_jack_detect(codec, WM8350_JDR, &hp_jack, SND_JACK_HEADPHONE);
/* Microphone jack detection */ snd_soc_jack_new(codec, "Microphone", - SND_JACK_MICROPHONE | SND_JACK_BTN_0, &mic_jack); + SND_JACK_MICROPHONE | SND_JACK_BTN_0, 0, &mic_jack); snd_soc_jack_add_pins(&mic_jack, ARRAY_SIZE(mic_jack_pins), mic_jack_pins); wm8350_mic_jack_detect(codec, &mic_jack, SND_JACK_MICROPHONE, diff --git a/sound/soc/mid-x86/mfld_machine.c b/sound/soc/mid-x86/mfld_machine.c index ee36384..e52c836 100644 --- a/sound/soc/mid-x86/mfld_machine.c +++ b/sound/soc/mid-x86/mfld_machine.c @@ -254,8 +254,8 @@ static int mfld_init(struct snd_soc_pcm_runtime *runtime)
/* Headset and button jack detection */ ret_val = snd_soc_jack_new(codec, "Intel(R) MID Audio Jack", - SND_JACK_HEADSET | SND_JACK_BTN_0 | - SND_JACK_BTN_1, &mfld_jack); + SND_JACK_HEADSET | SND_JACK_BTN_0 | + SND_JACK_BTN_1, 0, &mfld_jack); if (ret_val) { pr_err("jack creation failed\n"); return ret_val; diff --git a/sound/soc/omap/ams-delta.c b/sound/soc/omap/ams-delta.c index 6294464..4ffa38e 100644 --- a/sound/soc/omap/ams-delta.c +++ b/sound/soc/omap/ams-delta.c @@ -491,7 +491,7 @@ static int ams_delta_cx20442_init(struct snd_soc_pcm_runtime *rtd) /* Add hook switch - can be used to control the codec from userspace * even if line discipline fails */ ret = snd_soc_jack_new(rtd->codec, "hook_switch", - SND_JACK_HEADSET, &ams_delta_hook_switch); + SND_JACK_HEADSET, 0, &ams_delta_hook_switch); if (ret) dev_warn(card->dev, "Failed to allocate resources for hook switch, " diff --git a/sound/soc/omap/omap-abe-twl6040.c b/sound/soc/omap/omap-abe-twl6040.c index 70cd5c7..a63038b 100644 --- a/sound/soc/omap/omap-abe-twl6040.c +++ b/sound/soc/omap/omap-abe-twl6040.c @@ -194,7 +194,7 @@ static int omap_abe_twl6040_init(struct snd_soc_pcm_runtime *rtd) /* Headset jack detection only if it is supported */ if (priv->jack_detection) { ret = snd_soc_jack_new(codec, "Headset Jack", - SND_JACK_HEADSET, &hs_jack); + SND_JACK_HEADSET, 0, &hs_jack); if (ret) return ret;
diff --git a/sound/soc/omap/omap-twl4030.c b/sound/soc/omap/omap-twl4030.c index 2a9324f..07ba4d3 100644 --- a/sound/soc/omap/omap-twl4030.c +++ b/sound/soc/omap/omap-twl4030.c @@ -190,7 +190,7 @@ static int omap_twl4030_init(struct snd_soc_pcm_runtime *rtd) hs_jack_gpios[0].gpio = priv->jack_detect;
ret = snd_soc_jack_new(codec, "Headset Jack", SND_JACK_HEADSET, - &priv->hs_jack); + 0, &priv->hs_jack); if (ret) return ret;
diff --git a/sound/soc/omap/rx51.c b/sound/soc/omap/rx51.c index 611179c..2cec19a 100644 --- a/sound/soc/omap/rx51.c +++ b/sound/soc/omap/rx51.c @@ -318,8 +318,8 @@ static int rx51_aic34_init(struct snd_soc_pcm_runtime *rtd)
/* AV jack detection */ err = snd_soc_jack_new(codec, "AV Jack", - SND_JACK_HEADSET | SND_JACK_VIDEOOUT, - &rx51_av_jack); + SND_JACK_HEADSET | SND_JACK_VIDEOOUT, + 0, &rx51_av_jack); if (err) return err; err = snd_soc_jack_add_gpios(&rx51_av_jack, diff --git a/sound/soc/pxa/hx4700.c b/sound/soc/pxa/hx4700.c index dcc9b04..82fd557 100644 --- a/sound/soc/pxa/hx4700.c +++ b/sound/soc/pxa/hx4700.c @@ -138,7 +138,7 @@ static int hx4700_ak4641_init(struct snd_soc_pcm_runtime *rtd)
/* Jack detection API stuff */ err = snd_soc_jack_new(codec, "Headphone Jack", - SND_JACK_HEADPHONE, &hs_jack); + SND_JACK_HEADPHONE, 0, &hs_jack); if (err) return err;
diff --git a/sound/soc/pxa/palm27x.c b/sound/soc/pxa/palm27x.c index e1ffcdd..264be5e 100644 --- a/sound/soc/pxa/palm27x.c +++ b/sound/soc/pxa/palm27x.c @@ -98,7 +98,7 @@ static int palm27x_ac97_init(struct snd_soc_pcm_runtime *rtd)
/* Jack detection API stuff */ err = snd_soc_jack_new(codec, "Headphone Jack", - SND_JACK_HEADPHONE, &hs_jack); + SND_JACK_HEADPHONE, 0, &hs_jack); if (err) return err;
diff --git a/sound/soc/pxa/ttc-dkb.c b/sound/soc/pxa/ttc-dkb.c index f4ea4f6..1a462cb 100644 --- a/sound/soc/pxa/ttc-dkb.c +++ b/sound/soc/pxa/ttc-dkb.c @@ -87,12 +87,12 @@ static int ttc_pm860x_init(struct snd_soc_pcm_runtime *rtd)
/* Headset jack detection */ snd_soc_jack_new(codec, "Headphone Jack", SND_JACK_HEADPHONE - | SND_JACK_BTN_0 | SND_JACK_BTN_1 | SND_JACK_BTN_2, - &hs_jack); + | SND_JACK_BTN_0 | SND_JACK_BTN_1 | SND_JACK_BTN_2, + 0, &hs_jack); snd_soc_jack_add_pins(&hs_jack, ARRAY_SIZE(hs_jack_pins), hs_jack_pins); snd_soc_jack_new(codec, "Microphone Jack", SND_JACK_MICROPHONE, - &mic_jack); + 0, &mic_jack); snd_soc_jack_add_pins(&mic_jack, ARRAY_SIZE(mic_jack_pins), mic_jack_pins);
diff --git a/sound/soc/pxa/z2.c b/sound/soc/pxa/z2.c index 76ccb17..082fd24 100644 --- a/sound/soc/pxa/z2.c +++ b/sound/soc/pxa/z2.c @@ -144,7 +144,7 @@ static int z2_wm8750_init(struct snd_soc_pcm_runtime *rtd)
/* Jack detection API stuff */ ret = snd_soc_jack_new(codec, "Headset Jack", SND_JACK_HEADSET, - &hs_jack); + 0, &hs_jack); if (ret) goto err;
diff --git a/sound/soc/samsung/goni_wm8994.c b/sound/soc/samsung/goni_wm8994.c index 415ad81..bdd70a9 100644 --- a/sound/soc/samsung/goni_wm8994.c +++ b/sound/soc/samsung/goni_wm8994.c @@ -115,8 +115,9 @@ static int goni_wm8994_init(struct snd_soc_pcm_runtime *rtd)
/* Headset jack detection */ ret = snd_soc_jack_new(codec, "Headset Jack", - SND_JACK_HEADSET | SND_JACK_MECHANICAL | SND_JACK_AVOUT, - &jack); + SND_JACK_HEADSET | SND_JACK_MECHANICAL | + SND_JACK_AVOUT, + 0, &jack); if (ret) return ret;
diff --git a/sound/soc/samsung/h1940_uda1380.c b/sound/soc/samsung/h1940_uda1380.c index fa91376..66b5a33 100644 --- a/sound/soc/samsung/h1940_uda1380.c +++ b/sound/soc/samsung/h1940_uda1380.c @@ -187,7 +187,7 @@ static int h1940_uda1380_init(struct snd_soc_pcm_runtime *rtd) snd_soc_dapm_enable_pin(dapm, "Mic Jack");
snd_soc_jack_new(codec, "Headphone Jack", SND_JACK_HEADPHONE, - &hp_jack); + 0, &hp_jack);
snd_soc_jack_add_pins(&hp_jack, ARRAY_SIZE(hp_jack_pins), hp_jack_pins); diff --git a/sound/soc/samsung/littlemill.c b/sound/soc/samsung/littlemill.c index bfb91f3..46344f3 100644 --- a/sound/soc/samsung/littlemill.c +++ b/sound/soc/samsung/littlemill.c @@ -261,11 +261,11 @@ static int littlemill_late_probe(struct snd_soc_card *card) return ret;
ret = snd_soc_jack_new(codec, "Headset", - SND_JACK_HEADSET | SND_JACK_MECHANICAL | - SND_JACK_BTN_0 | SND_JACK_BTN_1 | - SND_JACK_BTN_2 | SND_JACK_BTN_3 | - SND_JACK_BTN_4 | SND_JACK_BTN_5, - &littlemill_headset); + SND_JACK_HEADSET | SND_JACK_MECHANICAL | + SND_JACK_BTN_0 | SND_JACK_BTN_1 | + SND_JACK_BTN_2 | SND_JACK_BTN_3 | + SND_JACK_BTN_4 | SND_JACK_BTN_5, + 0, &littlemill_headset); if (ret) return ret;
diff --git a/sound/soc/samsung/lowland.c b/sound/soc/samsung/lowland.c index 570cf52..1e6a3df 100644 --- a/sound/soc/samsung/lowland.c +++ b/sound/soc/samsung/lowland.c @@ -57,9 +57,9 @@ static int lowland_wm5100_init(struct snd_soc_pcm_runtime *rtd) }
ret = snd_soc_jack_new(codec, "Headset", - SND_JACK_LINEOUT | SND_JACK_HEADSET | - SND_JACK_BTN_0, - &lowland_headset); + SND_JACK_LINEOUT | SND_JACK_HEADSET | + SND_JACK_BTN_0, + 0, &lowland_headset); if (ret) return ret;
diff --git a/sound/soc/samsung/rx1950_uda1380.c b/sound/soc/samsung/rx1950_uda1380.c index 704460a..588ad72 100644 --- a/sound/soc/samsung/rx1950_uda1380.c +++ b/sound/soc/samsung/rx1950_uda1380.c @@ -232,7 +232,7 @@ static int rx1950_uda1380_init(struct snd_soc_pcm_runtime *rtd) snd_soc_dapm_enable_pin(dapm, "Mic Jack");
snd_soc_jack_new(codec, "Headphone Jack", SND_JACK_HEADPHONE, - &hp_jack); + 0, &hp_jack);
snd_soc_jack_add_pins(&hp_jack, ARRAY_SIZE(hp_jack_pins), hp_jack_pins); diff --git a/sound/soc/samsung/smartq_wm8987.c b/sound/soc/samsung/smartq_wm8987.c index 58ae323..71a17a5 100644 --- a/sound/soc/samsung/smartq_wm8987.c +++ b/sound/soc/samsung/smartq_wm8987.c @@ -167,7 +167,7 @@ static int smartq_wm8987_init(struct snd_soc_pcm_runtime *rtd)
/* Headphone jack detection */ err = snd_soc_jack_new(codec, "Headphone Jack", - SND_JACK_HEADPHONE, &smartq_jack); + SND_JACK_HEADPHONE, 0, &smartq_jack); if (err) return err;
diff --git a/sound/soc/samsung/speyside.c b/sound/soc/samsung/speyside.c index 57df90d..feff3fb 100644 --- a/sound/soc/samsung/speyside.c +++ b/sound/soc/samsung/speyside.c @@ -154,9 +154,9 @@ static int speyside_wm8996_init(struct snd_soc_pcm_runtime *rtd) gpio_direction_output(WM8996_HPSEL_GPIO, speyside_jack_polarity);
ret = snd_soc_jack_new(codec, "Headset", - SND_JACK_LINEOUT | SND_JACK_HEADSET | - SND_JACK_BTN_0, - &speyside_headset); + SND_JACK_LINEOUT | SND_JACK_HEADSET | + SND_JACK_BTN_0, + 0, &speyside_headset); if (ret) return ret;
diff --git a/sound/soc/samsung/tobermory.c b/sound/soc/samsung/tobermory.c index f21ff60..e498e7c 100644 --- a/sound/soc/samsung/tobermory.c +++ b/sound/soc/samsung/tobermory.c @@ -178,8 +178,8 @@ static int tobermory_late_probe(struct snd_soc_card *card) return ret;
ret = snd_soc_jack_new(codec, "Headset", - SND_JACK_HEADSET | SND_JACK_BTN_0, - &tobermory_headset); + SND_JACK_HEADSET | SND_JACK_BTN_0, + 0, &tobermory_headset); if (ret) return ret;
diff --git a/sound/soc/soc-jack.c b/sound/soc/soc-jack.c index 0bb5ccc..3275ab9 100644 --- a/sound/soc/soc-jack.c +++ b/sound/soc/soc-jack.c @@ -26,6 +26,7 @@ * @id: an identifying string for this jack * @type: a bitmask of enum snd_jack_type values that can be detected by * this jack + * @idx: The index of the ALSA control created to represent the jack. * @jack: structure to use for the jack * * Creates a new jack object. @@ -34,7 +35,7 @@ * On success jack will be initialised. */ int snd_soc_jack_new(struct snd_soc_codec *codec, const char *id, int type, - struct snd_soc_jack *jack) + int idx, struct snd_soc_jack *jack) { mutex_init(&jack->mutex); jack->codec = codec; @@ -42,7 +43,7 @@ int snd_soc_jack_new(struct snd_soc_codec *codec, const char *id, int type, INIT_LIST_HEAD(&jack->jack_zones); BLOCKING_INIT_NOTIFIER_HEAD(&jack->notifier);
- return snd_jack_new(codec->card->snd_card, id, type, &jack->jack); + return snd_jack_new(codec->card->snd_card, id, type, idx, &jack->jack); } EXPORT_SYMBOL_GPL(snd_soc_jack_new);
diff --git a/sound/soc/tegra/tegra_alc5632.c b/sound/soc/tegra/tegra_alc5632.c index 48d05d9..1a44af9 100644 --- a/sound/soc/tegra/tegra_alc5632.c +++ b/sound/soc/tegra/tegra_alc5632.c @@ -110,7 +110,7 @@ static int tegra_alc5632_asoc_init(struct snd_soc_pcm_runtime *rtd) struct tegra_alc5632 *machine = snd_soc_card_get_drvdata(codec->card);
snd_soc_jack_new(codec, "Headset Jack", SND_JACK_HEADSET, - &tegra_alc5632_hs_jack); + 0, &tegra_alc5632_hs_jack); snd_soc_jack_add_pins(&tegra_alc5632_hs_jack, ARRAY_SIZE(tegra_alc5632_hs_jack_pins), tegra_alc5632_hs_jack_pins); diff --git a/sound/soc/tegra/tegra_rt5640.c b/sound/soc/tegra/tegra_rt5640.c index 08794f9..aad20f2 100644 --- a/sound/soc/tegra/tegra_rt5640.c +++ b/sound/soc/tegra/tegra_rt5640.c @@ -112,7 +112,7 @@ static int tegra_rt5640_asoc_init(struct snd_soc_pcm_runtime *rtd) struct tegra_rt5640 *machine = snd_soc_card_get_drvdata(codec->card);
snd_soc_jack_new(codec, "Headphones", SND_JACK_HEADPHONE, - &tegra_rt5640_hp_jack); + 0, &tegra_rt5640_hp_jack); snd_soc_jack_add_pins(&tegra_rt5640_hp_jack, ARRAY_SIZE(tegra_rt5640_hp_jack_pins), tegra_rt5640_hp_jack_pins); diff --git a/sound/soc/tegra/tegra_wm8903.c b/sound/soc/tegra/tegra_wm8903.c index 4ac7373..3f38d31 100644 --- a/sound/soc/tegra/tegra_wm8903.c +++ b/sound/soc/tegra/tegra_wm8903.c @@ -179,7 +179,7 @@ static int tegra_wm8903_init(struct snd_soc_pcm_runtime *rtd) if (gpio_is_valid(machine->gpio_hp_det)) { tegra_wm8903_hp_jack_gpio.gpio = machine->gpio_hp_det; snd_soc_jack_new(codec, "Headphone Jack", SND_JACK_HEADPHONE, - &tegra_wm8903_hp_jack); + 0, &tegra_wm8903_hp_jack); snd_soc_jack_add_pins(&tegra_wm8903_hp_jack, ARRAY_SIZE(tegra_wm8903_hp_jack_pins), tegra_wm8903_hp_jack_pins); @@ -189,7 +189,7 @@ static int tegra_wm8903_init(struct snd_soc_pcm_runtime *rtd) }
snd_soc_jack_new(codec, "Mic Jack", SND_JACK_MICROPHONE, - &tegra_wm8903_mic_jack); + 0, &tegra_wm8903_mic_jack); snd_soc_jack_add_pins(&tegra_wm8903_mic_jack, ARRAY_SIZE(tegra_wm8903_mic_jack_pins), tegra_wm8903_mic_jack_pins);
On Fri, Aug 02, 2013 at 12:59:24AM -0700, Felipe F. Tonello wrote:
+int jack_ctl_integer_info(struct snd_kcontrol *kcontrol,
struct snd_ctl_elem_info *uinfo)+{
- uinfo->type = SNDRV_CTL_ELEM_TYPE_INTEGER;
- uinfo->count = 0x10000U;
- uinfo->value.integer.min = 0;
- uinfo->value.integer.max = 0xffff;
- return 0;
+}
I'd expected to see us creating multiple boolean controls here. I don't have a great problem with doing things this way but it's surprising and I worry about confusing existing userspace, Takashi?
Hi Mark,
On Wed, Aug 7, 2013 at 9:45 AM, Mark Brown broonie@kernel.org wrote:
On Fri, Aug 02, 2013 at 12:59:24AM -0700, Felipe F. Tonello wrote:
+int jack_ctl_integer_info(struct snd_kcontrol *kcontrol,
struct snd_ctl_elem_info *uinfo)+{
uinfo->type = SNDRV_CTL_ELEM_TYPE_INTEGER;uinfo->count = 0x10000U;uinfo->value.integer.min = 0;uinfo->value.integer.max = 0xffff;return 0;+}
I'd expected to see us creating multiple boolean controls here. I don't have a great problem with doing things this way but it's surprising and I worry about confusing existing userspace, Takashi?
Yes, it makes more sense. I got confused with another talk we had previously, that's why I end up doing as an int.
I will wait for more comments about these patches and will submit a v3.
Thanks,
Felipe Tonello
At Wed, 7 Aug 2013 17:45:09 +0100, Mark Brown wrote:
On Fri, Aug 02, 2013 at 12:59:24AM -0700, Felipe F. Tonello wrote:
+int jack_ctl_integer_info(struct snd_kcontrol *kcontrol,
struct snd_ctl_elem_info *uinfo)+{
- uinfo->type = SNDRV_CTL_ELEM_TYPE_INTEGER;
- uinfo->count = 0x10000U;
- uinfo->value.integer.min = 0;
- uinfo->value.integer.max = 0xffff;
- return 0;
+}
I'd expected to see us creating multiple boolean controls here. I don't have a great problem with doing things this way but it's surprising and I worry about confusing existing userspace, Takashi?
Yes, multiple boolean would make more sense.
thanks,
Takashi
From: "Felipe F. Tonello" eu@felipetonello.com
This patch adds jack support for ALSA KControl.
This support is necessary since the new kcontrol is used by user-space daemons, such as PulseAudio(>=2.0), to do jack detection.)
Also, HDA's CONFIG_SND_HDA_INPUT_JACK is disabled because it causes to use standard snd_jack_new() to create jacks. This can cause conflict since this codec creates jack controls directly.
It makes sure that all codecs using ALSA jack API are updated to use the new API.
Signed-off-by: Felipe F. Tonello eu@felipetonello.com --- include/sound/control.h | 2 +- include/sound/jack.h | 8 ++-- include/sound/soc.h | 2 +- sound/core/Kconfig | 1 + sound/core/ctljack.c | 3 +- sound/core/jack.c | 88 +++++++++++++++++++++++++++++++++++--- sound/pci/hda/Kconfig | 8 ---- sound/pci/oxygen/xonar_wm87x6.c | 2 +- sound/soc/fsl/wm1133-ev1.c | 4 +- sound/soc/mid-x86/mfld_machine.c | 4 +- sound/soc/omap/ams-delta.c | 2 +- sound/soc/omap/omap-abe-twl6040.c | 2 +- sound/soc/omap/omap-twl4030.c | 2 +- sound/soc/omap/rx51.c | 4 +- sound/soc/pxa/hx4700.c | 2 +- sound/soc/pxa/palm27x.c | 2 +- sound/soc/pxa/ttc-dkb.c | 6 +-- sound/soc/pxa/z2.c | 2 +- sound/soc/samsung/goni_wm8994.c | 5 ++- sound/soc/samsung/h1940_uda1380.c | 2 +- sound/soc/samsung/littlemill.c | 10 ++--- sound/soc/samsung/lowland.c | 6 +-- sound/soc/samsung/rx1950_uda1380.c | 2 +- sound/soc/samsung/smartq_wm8987.c | 2 +- sound/soc/samsung/speyside.c | 6 +-- sound/soc/samsung/tobermory.c | 4 +- sound/soc/soc-jack.c | 5 ++- sound/soc/tegra/tegra_alc5632.c | 2 +- sound/soc/tegra/tegra_rt5640.c | 2 +- sound/soc/tegra/tegra_wm8903.c | 4 +- 30 files changed, 135 insertions(+), 59 deletions(-)
diff --git a/include/sound/control.h b/include/sound/control.h index 5358892..ffeb6b6 100644 --- a/include/sound/control.h +++ b/include/sound/control.h @@ -242,6 +242,6 @@ void snd_ctl_sync_vmaster(struct snd_kcontrol *kctl, bool hook_only); struct snd_kcontrol * snd_kctl_jack_new(const char *name, int idx, void *private_data); void snd_kctl_jack_report(struct snd_card *card, - struct snd_kcontrol *kctl, bool status); + struct snd_kcontrol *kctl, bool status);
#endif /* __SOUND_CONTROL_H */ diff --git a/include/sound/jack.h b/include/sound/jack.h index 5891657..0d36f20 100644 --- a/include/sound/jack.h +++ b/include/sound/jack.h @@ -26,6 +26,7 @@ #include <sound/core.h>
struct input_dev; +struct snd_kcontrol;
/** * Jack types which can be reported. These values are used as a @@ -58,11 +59,12 @@ enum snd_jack_types {
struct snd_jack { struct input_dev *input_dev; + struct snd_kcontrol *kctl[SND_JACK_SWITCH_TYPES]; /* control for each key */ int registered; int type; const char *id; char name[100]; - unsigned int key[6]; /* Keep in sync with definitions above */ + unsigned int key[SND_JACK_SWITCH_TYPES]; /* Keep in sync with definitions above */ void *private_data; void (*private_free)(struct snd_jack *); }; @@ -70,7 +72,7 @@ struct snd_jack { #ifdef CONFIG_SND_JACK
int snd_jack_new(struct snd_card *card, const char *id, int type, - struct snd_jack **jack); + int idx, struct snd_jack **jack); void snd_jack_set_parent(struct snd_jack *jack, struct device *parent); int snd_jack_set_key(struct snd_jack *jack, enum snd_jack_types type, int keytype); @@ -80,7 +82,7 @@ void snd_jack_report(struct snd_jack *jack, int status); #else
static inline int snd_jack_new(struct snd_card *card, const char *id, int type, - struct snd_jack **jack) + int idx, struct snd_jack **jack) { return 0; } diff --git a/include/sound/soc.h b/include/sound/soc.h index 6eabee7..31bea52 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -436,7 +436,7 @@ int snd_soc_platform_trigger(struct snd_pcm_substream *substream,
/* Jack reporting */ int snd_soc_jack_new(struct snd_soc_codec *codec, const char *id, int type, - struct snd_soc_jack *jack); + int idx, struct snd_soc_jack *jack); void snd_soc_jack_report(struct snd_soc_jack *jack, int status, int mask); int snd_soc_jack_add_pins(struct snd_soc_jack *jack, int count, struct snd_soc_jack_pin *pins); diff --git a/sound/core/Kconfig b/sound/core/Kconfig index c0c2f57..8167615 100644 --- a/sound/core/Kconfig +++ b/sound/core/Kconfig @@ -20,6 +20,7 @@ config SND_COMPRESS_OFFLOAD # to avoid having to force INPUT on. config SND_JACK bool + select SND_KCTL_JACK
config SND_SEQUENCER tristate "Sequencer support" diff --git a/sound/core/ctljack.c b/sound/core/ctljack.c index e4b38fb..441158d 100644 --- a/sound/core/ctljack.c +++ b/sound/core/ctljack.c @@ -14,7 +14,7 @@ #include <sound/core.h> #include <sound/control.h>
-#define jack_detect_kctl_info snd_ctl_boolean_mono_info +#define jack_detect_kctl_info snd_ctl_boolean_mono_info
static int jack_detect_kctl_get(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol) @@ -38,6 +38,7 @@ snd_kctl_jack_new(const char *name, int idx, void *private_data) kctl = snd_ctl_new1(&jack_detect_kctl, private_data); if (!kctl) return NULL; + snprintf(kctl->id.name, sizeof(kctl->id.name), "%s Jack", name); kctl->id.index = idx; kctl->private_value = 0; diff --git a/sound/core/jack.c b/sound/core/jack.c index b35fe73..128154b 100644 --- a/sound/core/jack.c +++ b/sound/core/jack.c @@ -24,6 +24,7 @@ #include <linux/module.h> #include <sound/jack.h> #include <sound/core.h> +#include <sound/control.h>
static int jack_switch_types[SND_JACK_SWITCH_TYPES] = { SW_HEADPHONE_INSERT, @@ -37,6 +38,7 @@ static int jack_switch_types[SND_JACK_SWITCH_TYPES] = { static int snd_jack_dev_free(struct snd_device *device) { struct snd_jack *jack = device->device_data; + int i;
if (jack->private_free) jack->private_free(jack); @@ -48,19 +50,54 @@ static int snd_jack_dev_free(struct snd_device *device) else input_free_device(jack->input_dev);
+ /* Free available KControls*/ + for (i = 0; i < SND_JACK_SWITCH_TYPES; ++i) + if (jack->kctl[i]) + snd_ctl_remove(device->card, jack->kctl[i]); + kfree(jack->id); kfree(jack);
return 0; }
+const char * jack_get_name_by_key(const char *name, int key) +{ + char *jack_name; + size_t jack_name_size; + const char *key_name; + + switch(key) { + case SW_HEADPHONE_INSERT: key_name = "Headphone"; break; + case SW_MICROPHONE_INSERT: key_name = "Mic"; break; + case SW_LINEOUT_INSERT: key_name = "Line Out"; break; + case SW_JACK_PHYSICAL_INSERT: key_name = "Mechanical"; break; + case SW_VIDEOOUT_INSERT: key_name = "Video Out"; break; + case SW_LINEIN_INSERT: key_name = "Line In"; break; + default: key_name = "Unknown"; + } + + /* Avoid duplicate name in KControl */ + if (strcmp(name, key_name) != 0) { + /* allocate necessary memory space only */ + jack_name_size = strlen(name) + strlen(key_name) + 4; + jack_name = kmalloc(jack_name_size, GFP_KERNEL); + + snprintf(jack_name, jack_name_size, "%s (%s)", name, key_name); + } else { + jack_name = (char *)name; + } + + return jack_name; +} + static int snd_jack_dev_register(struct snd_device *device) { struct snd_jack *jack = device->device_data; struct snd_card *card = device->card; int err, i;
- snprintf(jack->name, sizeof(jack->name), "%s %s", + snprintf(jack->name, sizeof(jack->name), "%s %s Jack", card->shortname, jack->id); jack->input_dev->name = jack->name;
@@ -81,6 +118,18 @@ static int snd_jack_dev_register(struct snd_device *device) input_set_capability(jack->input_dev, EV_KEY, jack->key[i]); }
+ /* We don't need to free the control, it's freed by snd_ctl_add itself + if an error occur */ + for (i = 0; i < SND_JACK_SWITCH_TYPES; ++i) + if (jack->kctl[i]) { + err = snd_ctl_add(card, jack->kctl[i]); + if (err < 0) { + pr_notice("%s: ALSA Jack Control not available for '%s'\n", __func__, + jack_get_name_by_key(jack->id, jack_switch_types[i])); + jack->kctl[i] = NULL; + } + } + err = input_register_device(jack->input_dev); if (err == 0) jack->registered = 1; @@ -91,22 +140,31 @@ static int snd_jack_dev_register(struct snd_device *device) /** * snd_jack_new - Create a new jack * @card: the card instance - * @id: an identifying string for this jack + * @id: an identifying string for this jack, " Jack" is appended to the + * string * @type: a bitmask of enum snd_jack_type values that can be detected by * this jack + * @idx: The index of the ALSA control created to represent the jack. * @jjack: Used to provide the allocated jack object to the caller. * * Creates a new jack object. * + * This function creates a Jack Kcontrol for each @type id as "@id @type Jack". + * Eg.: A jack with @type SND_JACK_HEADSET will have two KControls created, + * "@id Headphone Jack" and "@id Mic Jack". If @id and @type strings are + * equals, then this KControl will be named "@id Jack" only. + * * Return: Zero if successful, or a negative error code on failure. * On success @jjack will be initialised. */ int snd_jack_new(struct snd_card *card, const char *id, int type, - struct snd_jack **jjack) + int idx, struct snd_jack **jjack) { struct snd_jack *jack; + struct snd_kcontrol *kctl; int err; int i; + static struct snd_device_ops ops = { .dev_free = snd_jack_dev_free, .dev_register = snd_jack_dev_register, @@ -129,10 +187,20 @@ int snd_jack_new(struct snd_card *card, const char *id, int type, jack->type = type;
for (i = 0; i < SND_JACK_SWITCH_TYPES; i++) - if (type & (1 << i)) + if (type & (1 << i)) { input_set_capability(jack->input_dev, EV_SW, jack_switch_types[i]);
+ /* card is the private_data */ + kctl = snd_kctl_jack_new(jack_get_name_by_key(id, jack_switch_types[i]), idx, card); + if (!kctl) { + err = -ENOMEM; + goto fail_kctl; + } + + jack->kctl[i] = kctl; + } + err = snd_device_new(card, SNDRV_DEV_JACK, jack, &ops); if (err < 0) goto fail_input; @@ -141,6 +209,11 @@ int snd_jack_new(struct snd_card *card, const char *id, int type,
return 0;
+fail_kctl: + for (i = 0; i < SND_JACK_SWITCH_TYPES; ++i) + if (jack->kctl[i]) + snd_ctl_remove(card, jack->kctl[i]); + fail_input: input_free_device(jack->input_dev); kfree(jack->id); @@ -232,10 +305,15 @@ void snd_jack_report(struct snd_jack *jack, int status)
for (i = 0; i < ARRAY_SIZE(jack_switch_types); i++) { int testbit = 1 << i; - if (jack->type & testbit) + if (jack->type & testbit) { input_report_switch(jack->input_dev, jack_switch_types[i], status & testbit); + + /* Update ALSA KControl interface */ + snd_kctl_jack_report((struct snd_card *)jack->kctl[i]->private_data, + jack->kctl[i], status & testbit); + } }
input_sync(jack->input_dev); diff --git a/sound/pci/hda/Kconfig b/sound/pci/hda/Kconfig index 59c5e9c..561abc7 100644 --- a/sound/pci/hda/Kconfig +++ b/sound/pci/hda/Kconfig @@ -65,14 +65,6 @@ config SND_HDA_INPUT_BEEP_MODE Set 1 to always enable the digital beep interface for HD-audio by default.
-config SND_HDA_INPUT_JACK - bool "Support jack plugging notification via input layer" - depends on INPUT=y || INPUT=SND - select SND_JACK - help - Say Y here to enable the jack plugging notification via - input layer. - config SND_HDA_PATCH_LOADER bool "Support initialization patch loading for HD-audio" select FW_LOADER diff --git a/sound/pci/oxygen/xonar_wm87x6.c b/sound/pci/oxygen/xonar_wm87x6.c index 6ce6860..d3816b1 100644 --- a/sound/pci/oxygen/xonar_wm87x6.c +++ b/sound/pci/oxygen/xonar_wm87x6.c @@ -286,7 +286,7 @@ static void xonar_ds_init(struct oxygen *chip) xonar_enable_output(chip);
snd_jack_new(chip->card, "Headphone", - SND_JACK_HEADPHONE, &data->hp_jack); + SND_JACK_HEADPHONE, 0, &data->hp_jack); xonar_ds_handle_hp_jack(chip);
snd_component_add(chip->card, "WM8776"); diff --git a/sound/soc/fsl/wm1133-ev1.c b/sound/soc/fsl/wm1133-ev1.c index fce6325..50f96d4 100644 --- a/sound/soc/fsl/wm1133-ev1.c +++ b/sound/soc/fsl/wm1133-ev1.c @@ -221,14 +221,14 @@ static int wm1133_ev1_init(struct snd_soc_pcm_runtime *rtd) ARRAY_SIZE(wm1133_ev1_map));
/* Headphone jack detection */ - snd_soc_jack_new(codec, "Headphone", SND_JACK_HEADPHONE, &hp_jack); + snd_soc_jack_new(codec, "Headphone", SND_JACK_HEADPHONE, 0, &hp_jack); snd_soc_jack_add_pins(&hp_jack, ARRAY_SIZE(hp_jack_pins), hp_jack_pins); wm8350_hp_jack_detect(codec, WM8350_JDR, &hp_jack, SND_JACK_HEADPHONE);
/* Microphone jack detection */ snd_soc_jack_new(codec, "Microphone", - SND_JACK_MICROPHONE | SND_JACK_BTN_0, &mic_jack); + SND_JACK_MICROPHONE | SND_JACK_BTN_0, 0, &mic_jack); snd_soc_jack_add_pins(&mic_jack, ARRAY_SIZE(mic_jack_pins), mic_jack_pins); wm8350_mic_jack_detect(codec, &mic_jack, SND_JACK_MICROPHONE, diff --git a/sound/soc/mid-x86/mfld_machine.c b/sound/soc/mid-x86/mfld_machine.c index ee36384..e52c836 100644 --- a/sound/soc/mid-x86/mfld_machine.c +++ b/sound/soc/mid-x86/mfld_machine.c @@ -254,8 +254,8 @@ static int mfld_init(struct snd_soc_pcm_runtime *runtime)
/* Headset and button jack detection */ ret_val = snd_soc_jack_new(codec, "Intel(R) MID Audio Jack", - SND_JACK_HEADSET | SND_JACK_BTN_0 | - SND_JACK_BTN_1, &mfld_jack); + SND_JACK_HEADSET | SND_JACK_BTN_0 | + SND_JACK_BTN_1, 0, &mfld_jack); if (ret_val) { pr_err("jack creation failed\n"); return ret_val; diff --git a/sound/soc/omap/ams-delta.c b/sound/soc/omap/ams-delta.c index 6294464..4ffa38e 100644 --- a/sound/soc/omap/ams-delta.c +++ b/sound/soc/omap/ams-delta.c @@ -491,7 +491,7 @@ static int ams_delta_cx20442_init(struct snd_soc_pcm_runtime *rtd) /* Add hook switch - can be used to control the codec from userspace * even if line discipline fails */ ret = snd_soc_jack_new(rtd->codec, "hook_switch", - SND_JACK_HEADSET, &ams_delta_hook_switch); + SND_JACK_HEADSET, 0, &ams_delta_hook_switch); if (ret) dev_warn(card->dev, "Failed to allocate resources for hook switch, " diff --git a/sound/soc/omap/omap-abe-twl6040.c b/sound/soc/omap/omap-abe-twl6040.c index 70cd5c7..a63038b 100644 --- a/sound/soc/omap/omap-abe-twl6040.c +++ b/sound/soc/omap/omap-abe-twl6040.c @@ -194,7 +194,7 @@ static int omap_abe_twl6040_init(struct snd_soc_pcm_runtime *rtd) /* Headset jack detection only if it is supported */ if (priv->jack_detection) { ret = snd_soc_jack_new(codec, "Headset Jack", - SND_JACK_HEADSET, &hs_jack); + SND_JACK_HEADSET, 0, &hs_jack); if (ret) return ret;
diff --git a/sound/soc/omap/omap-twl4030.c b/sound/soc/omap/omap-twl4030.c index 2a9324f..07ba4d3 100644 --- a/sound/soc/omap/omap-twl4030.c +++ b/sound/soc/omap/omap-twl4030.c @@ -190,7 +190,7 @@ static int omap_twl4030_init(struct snd_soc_pcm_runtime *rtd) hs_jack_gpios[0].gpio = priv->jack_detect;
ret = snd_soc_jack_new(codec, "Headset Jack", SND_JACK_HEADSET, - &priv->hs_jack); + 0, &priv->hs_jack); if (ret) return ret;
diff --git a/sound/soc/omap/rx51.c b/sound/soc/omap/rx51.c index 611179c..2cec19a 100644 --- a/sound/soc/omap/rx51.c +++ b/sound/soc/omap/rx51.c @@ -318,8 +318,8 @@ static int rx51_aic34_init(struct snd_soc_pcm_runtime *rtd)
/* AV jack detection */ err = snd_soc_jack_new(codec, "AV Jack", - SND_JACK_HEADSET | SND_JACK_VIDEOOUT, - &rx51_av_jack); + SND_JACK_HEADSET | SND_JACK_VIDEOOUT, + 0, &rx51_av_jack); if (err) return err; err = snd_soc_jack_add_gpios(&rx51_av_jack, diff --git a/sound/soc/pxa/hx4700.c b/sound/soc/pxa/hx4700.c index dcc9b04..82fd557 100644 --- a/sound/soc/pxa/hx4700.c +++ b/sound/soc/pxa/hx4700.c @@ -138,7 +138,7 @@ static int hx4700_ak4641_init(struct snd_soc_pcm_runtime *rtd)
/* Jack detection API stuff */ err = snd_soc_jack_new(codec, "Headphone Jack", - SND_JACK_HEADPHONE, &hs_jack); + SND_JACK_HEADPHONE, 0, &hs_jack); if (err) return err;
diff --git a/sound/soc/pxa/palm27x.c b/sound/soc/pxa/palm27x.c index e1ffcdd..264be5e 100644 --- a/sound/soc/pxa/palm27x.c +++ b/sound/soc/pxa/palm27x.c @@ -98,7 +98,7 @@ static int palm27x_ac97_init(struct snd_soc_pcm_runtime *rtd)
/* Jack detection API stuff */ err = snd_soc_jack_new(codec, "Headphone Jack", - SND_JACK_HEADPHONE, &hs_jack); + SND_JACK_HEADPHONE, 0, &hs_jack); if (err) return err;
diff --git a/sound/soc/pxa/ttc-dkb.c b/sound/soc/pxa/ttc-dkb.c index f4ea4f6..1a462cb 100644 --- a/sound/soc/pxa/ttc-dkb.c +++ b/sound/soc/pxa/ttc-dkb.c @@ -87,12 +87,12 @@ static int ttc_pm860x_init(struct snd_soc_pcm_runtime *rtd)
/* Headset jack detection */ snd_soc_jack_new(codec, "Headphone Jack", SND_JACK_HEADPHONE - | SND_JACK_BTN_0 | SND_JACK_BTN_1 | SND_JACK_BTN_2, - &hs_jack); + | SND_JACK_BTN_0 | SND_JACK_BTN_1 | SND_JACK_BTN_2, + 0, &hs_jack); snd_soc_jack_add_pins(&hs_jack, ARRAY_SIZE(hs_jack_pins), hs_jack_pins); snd_soc_jack_new(codec, "Microphone Jack", SND_JACK_MICROPHONE, - &mic_jack); + 0, &mic_jack); snd_soc_jack_add_pins(&mic_jack, ARRAY_SIZE(mic_jack_pins), mic_jack_pins);
diff --git a/sound/soc/pxa/z2.c b/sound/soc/pxa/z2.c index 76ccb17..082fd24 100644 --- a/sound/soc/pxa/z2.c +++ b/sound/soc/pxa/z2.c @@ -144,7 +144,7 @@ static int z2_wm8750_init(struct snd_soc_pcm_runtime *rtd)
/* Jack detection API stuff */ ret = snd_soc_jack_new(codec, "Headset Jack", SND_JACK_HEADSET, - &hs_jack); + 0, &hs_jack); if (ret) goto err;
diff --git a/sound/soc/samsung/goni_wm8994.c b/sound/soc/samsung/goni_wm8994.c index 415ad81..bdd70a9 100644 --- a/sound/soc/samsung/goni_wm8994.c +++ b/sound/soc/samsung/goni_wm8994.c @@ -115,8 +115,9 @@ static int goni_wm8994_init(struct snd_soc_pcm_runtime *rtd)
/* Headset jack detection */ ret = snd_soc_jack_new(codec, "Headset Jack", - SND_JACK_HEADSET | SND_JACK_MECHANICAL | SND_JACK_AVOUT, - &jack); + SND_JACK_HEADSET | SND_JACK_MECHANICAL | + SND_JACK_AVOUT, + 0, &jack); if (ret) return ret;
diff --git a/sound/soc/samsung/h1940_uda1380.c b/sound/soc/samsung/h1940_uda1380.c index fa91376..66b5a33 100644 --- a/sound/soc/samsung/h1940_uda1380.c +++ b/sound/soc/samsung/h1940_uda1380.c @@ -187,7 +187,7 @@ static int h1940_uda1380_init(struct snd_soc_pcm_runtime *rtd) snd_soc_dapm_enable_pin(dapm, "Mic Jack");
snd_soc_jack_new(codec, "Headphone Jack", SND_JACK_HEADPHONE, - &hp_jack); + 0, &hp_jack);
snd_soc_jack_add_pins(&hp_jack, ARRAY_SIZE(hp_jack_pins), hp_jack_pins); diff --git a/sound/soc/samsung/littlemill.c b/sound/soc/samsung/littlemill.c index bfb91f3..46344f3 100644 --- a/sound/soc/samsung/littlemill.c +++ b/sound/soc/samsung/littlemill.c @@ -261,11 +261,11 @@ static int littlemill_late_probe(struct snd_soc_card *card) return ret;
ret = snd_soc_jack_new(codec, "Headset", - SND_JACK_HEADSET | SND_JACK_MECHANICAL | - SND_JACK_BTN_0 | SND_JACK_BTN_1 | - SND_JACK_BTN_2 | SND_JACK_BTN_3 | - SND_JACK_BTN_4 | SND_JACK_BTN_5, - &littlemill_headset); + SND_JACK_HEADSET | SND_JACK_MECHANICAL | + SND_JACK_BTN_0 | SND_JACK_BTN_1 | + SND_JACK_BTN_2 | SND_JACK_BTN_3 | + SND_JACK_BTN_4 | SND_JACK_BTN_5, + 0, &littlemill_headset); if (ret) return ret;
diff --git a/sound/soc/samsung/lowland.c b/sound/soc/samsung/lowland.c index 570cf52..1e6a3df 100644 --- a/sound/soc/samsung/lowland.c +++ b/sound/soc/samsung/lowland.c @@ -57,9 +57,9 @@ static int lowland_wm5100_init(struct snd_soc_pcm_runtime *rtd) }
ret = snd_soc_jack_new(codec, "Headset", - SND_JACK_LINEOUT | SND_JACK_HEADSET | - SND_JACK_BTN_0, - &lowland_headset); + SND_JACK_LINEOUT | SND_JACK_HEADSET | + SND_JACK_BTN_0, + 0, &lowland_headset); if (ret) return ret;
diff --git a/sound/soc/samsung/rx1950_uda1380.c b/sound/soc/samsung/rx1950_uda1380.c index 704460a..588ad72 100644 --- a/sound/soc/samsung/rx1950_uda1380.c +++ b/sound/soc/samsung/rx1950_uda1380.c @@ -232,7 +232,7 @@ static int rx1950_uda1380_init(struct snd_soc_pcm_runtime *rtd) snd_soc_dapm_enable_pin(dapm, "Mic Jack");
snd_soc_jack_new(codec, "Headphone Jack", SND_JACK_HEADPHONE, - &hp_jack); + 0, &hp_jack);
snd_soc_jack_add_pins(&hp_jack, ARRAY_SIZE(hp_jack_pins), hp_jack_pins); diff --git a/sound/soc/samsung/smartq_wm8987.c b/sound/soc/samsung/smartq_wm8987.c index 58ae323..71a17a5 100644 --- a/sound/soc/samsung/smartq_wm8987.c +++ b/sound/soc/samsung/smartq_wm8987.c @@ -167,7 +167,7 @@ static int smartq_wm8987_init(struct snd_soc_pcm_runtime *rtd)
/* Headphone jack detection */ err = snd_soc_jack_new(codec, "Headphone Jack", - SND_JACK_HEADPHONE, &smartq_jack); + SND_JACK_HEADPHONE, 0, &smartq_jack); if (err) return err;
diff --git a/sound/soc/samsung/speyside.c b/sound/soc/samsung/speyside.c index 57df90d..feff3fb 100644 --- a/sound/soc/samsung/speyside.c +++ b/sound/soc/samsung/speyside.c @@ -154,9 +154,9 @@ static int speyside_wm8996_init(struct snd_soc_pcm_runtime *rtd) gpio_direction_output(WM8996_HPSEL_GPIO, speyside_jack_polarity);
ret = snd_soc_jack_new(codec, "Headset", - SND_JACK_LINEOUT | SND_JACK_HEADSET | - SND_JACK_BTN_0, - &speyside_headset); + SND_JACK_LINEOUT | SND_JACK_HEADSET | + SND_JACK_BTN_0, + 0, &speyside_headset); if (ret) return ret;
diff --git a/sound/soc/samsung/tobermory.c b/sound/soc/samsung/tobermory.c index f21ff60..e498e7c 100644 --- a/sound/soc/samsung/tobermory.c +++ b/sound/soc/samsung/tobermory.c @@ -178,8 +178,8 @@ static int tobermory_late_probe(struct snd_soc_card *card) return ret;
ret = snd_soc_jack_new(codec, "Headset", - SND_JACK_HEADSET | SND_JACK_BTN_0, - &tobermory_headset); + SND_JACK_HEADSET | SND_JACK_BTN_0, + 0, &tobermory_headset); if (ret) return ret;
diff --git a/sound/soc/soc-jack.c b/sound/soc/soc-jack.c index 0bb5ccc..3275ab9 100644 --- a/sound/soc/soc-jack.c +++ b/sound/soc/soc-jack.c @@ -26,6 +26,7 @@ * @id: an identifying string for this jack * @type: a bitmask of enum snd_jack_type values that can be detected by * this jack + * @idx: The index of the ALSA control created to represent the jack. * @jack: structure to use for the jack * * Creates a new jack object. @@ -34,7 +35,7 @@ * On success jack will be initialised. */ int snd_soc_jack_new(struct snd_soc_codec *codec, const char *id, int type, - struct snd_soc_jack *jack) + int idx, struct snd_soc_jack *jack) { mutex_init(&jack->mutex); jack->codec = codec; @@ -42,7 +43,7 @@ int snd_soc_jack_new(struct snd_soc_codec *codec, const char *id, int type, INIT_LIST_HEAD(&jack->jack_zones); BLOCKING_INIT_NOTIFIER_HEAD(&jack->notifier);
- return snd_jack_new(codec->card->snd_card, id, type, &jack->jack); + return snd_jack_new(codec->card->snd_card, id, type, idx, &jack->jack); } EXPORT_SYMBOL_GPL(snd_soc_jack_new);
diff --git a/sound/soc/tegra/tegra_alc5632.c b/sound/soc/tegra/tegra_alc5632.c index 48d05d9..1a44af9 100644 --- a/sound/soc/tegra/tegra_alc5632.c +++ b/sound/soc/tegra/tegra_alc5632.c @@ -110,7 +110,7 @@ static int tegra_alc5632_asoc_init(struct snd_soc_pcm_runtime *rtd) struct tegra_alc5632 *machine = snd_soc_card_get_drvdata(codec->card);
snd_soc_jack_new(codec, "Headset Jack", SND_JACK_HEADSET, - &tegra_alc5632_hs_jack); + 0, &tegra_alc5632_hs_jack); snd_soc_jack_add_pins(&tegra_alc5632_hs_jack, ARRAY_SIZE(tegra_alc5632_hs_jack_pins), tegra_alc5632_hs_jack_pins); diff --git a/sound/soc/tegra/tegra_rt5640.c b/sound/soc/tegra/tegra_rt5640.c index 08794f9..aad20f2 100644 --- a/sound/soc/tegra/tegra_rt5640.c +++ b/sound/soc/tegra/tegra_rt5640.c @@ -112,7 +112,7 @@ static int tegra_rt5640_asoc_init(struct snd_soc_pcm_runtime *rtd) struct tegra_rt5640 *machine = snd_soc_card_get_drvdata(codec->card);
snd_soc_jack_new(codec, "Headphones", SND_JACK_HEADPHONE, - &tegra_rt5640_hp_jack); + 0, &tegra_rt5640_hp_jack); snd_soc_jack_add_pins(&tegra_rt5640_hp_jack, ARRAY_SIZE(tegra_rt5640_hp_jack_pins), tegra_rt5640_hp_jack_pins); diff --git a/sound/soc/tegra/tegra_wm8903.c b/sound/soc/tegra/tegra_wm8903.c index 4ac7373..3f38d31 100644 --- a/sound/soc/tegra/tegra_wm8903.c +++ b/sound/soc/tegra/tegra_wm8903.c @@ -179,7 +179,7 @@ static int tegra_wm8903_init(struct snd_soc_pcm_runtime *rtd) if (gpio_is_valid(machine->gpio_hp_det)) { tegra_wm8903_hp_jack_gpio.gpio = machine->gpio_hp_det; snd_soc_jack_new(codec, "Headphone Jack", SND_JACK_HEADPHONE, - &tegra_wm8903_hp_jack); + 0, &tegra_wm8903_hp_jack); snd_soc_jack_add_pins(&tegra_wm8903_hp_jack, ARRAY_SIZE(tegra_wm8903_hp_jack_pins), tegra_wm8903_hp_jack_pins); @@ -189,7 +189,7 @@ static int tegra_wm8903_init(struct snd_soc_pcm_runtime *rtd) }
snd_soc_jack_new(codec, "Mic Jack", SND_JACK_MICROPHONE, - &tegra_wm8903_mic_jack); + 0, &tegra_wm8903_mic_jack); snd_soc_jack_add_pins(&tegra_wm8903_mic_jack, ARRAY_SIZE(tegra_wm8903_mic_jack_pins), tegra_wm8903_mic_jack_pins);
At Thu, 8 Aug 2013 23:21:55 -0700, Felipe F. Tonello wrote:
From: "Felipe F. Tonello" eu@felipetonello.com
This patch adds jack support for ALSA KControl.
This support is necessary since the new kcontrol is used by user-space daemons, such as PulseAudio(>=2.0), to do jack detection.)
Also, HDA's CONFIG_SND_HDA_INPUT_JACK is disabled because it causes to use standard snd_jack_new() to create jacks. This can cause conflict since this codec creates jack controls directly.
It makes sure that all codecs using ALSA jack API are updated to use the new API.
Well, while this is a good move forward, this patch is a bit too intrusive as a single patch. It breaks way too many. "Break then fix" is no good attitude, especially when it's just something for future.
So, in general, I prefer a way with more gradual changes. Let's see in details below:
Signed-off-by: Felipe F. Tonello eu@felipetonello.com
include/sound/control.h | 2 +- include/sound/jack.h | 8 ++-- include/sound/soc.h | 2 +- sound/core/Kconfig | 1 + sound/core/ctljack.c | 3 +- sound/core/jack.c | 88 +++++++++++++++++++++++++++++++++++--- sound/pci/hda/Kconfig | 8 ---- sound/pci/oxygen/xonar_wm87x6.c | 2 +- sound/soc/fsl/wm1133-ev1.c | 4 +- sound/soc/mid-x86/mfld_machine.c | 4 +- sound/soc/omap/ams-delta.c | 2 +- sound/soc/omap/omap-abe-twl6040.c | 2 +- sound/soc/omap/omap-twl4030.c | 2 +- sound/soc/omap/rx51.c | 4 +- sound/soc/pxa/hx4700.c | 2 +- sound/soc/pxa/palm27x.c | 2 +- sound/soc/pxa/ttc-dkb.c | 6 +-- sound/soc/pxa/z2.c | 2 +- sound/soc/samsung/goni_wm8994.c | 5 ++- sound/soc/samsung/h1940_uda1380.c | 2 +- sound/soc/samsung/littlemill.c | 10 ++--- sound/soc/samsung/lowland.c | 6 +-- sound/soc/samsung/rx1950_uda1380.c | 2 +- sound/soc/samsung/smartq_wm8987.c | 2 +- sound/soc/samsung/speyside.c | 6 +-- sound/soc/samsung/tobermory.c | 4 +- sound/soc/soc-jack.c | 5 ++- sound/soc/tegra/tegra_alc5632.c | 2 +- sound/soc/tegra/tegra_rt5640.c | 2 +- sound/soc/tegra/tegra_wm8903.c | 4 +- 30 files changed, 135 insertions(+), 59 deletions(-)
diff --git a/include/sound/control.h b/include/sound/control.h index 5358892..ffeb6b6 100644 --- a/include/sound/control.h +++ b/include/sound/control.h @@ -242,6 +242,6 @@ void snd_ctl_sync_vmaster(struct snd_kcontrol *kctl, bool hook_only); struct snd_kcontrol * snd_kctl_jack_new(const char *name, int idx, void *private_data); void snd_kctl_jack_report(struct snd_card *card,
struct snd_kcontrol *kctl, bool status);
struct snd_kcontrol *kctl, bool status);
Please don't include space or coding-fix changes.
#endif /* __SOUND_CONTROL_H */ diff --git a/include/sound/jack.h b/include/sound/jack.h index 5891657..0d36f20 100644 --- a/include/sound/jack.h +++ b/include/sound/jack.h @@ -26,6 +26,7 @@ #include <sound/core.h>
struct input_dev; +struct snd_kcontrol;
/**
- Jack types which can be reported. These values are used as a
@@ -58,11 +59,12 @@ enum snd_jack_types {
struct snd_jack { struct input_dev *input_dev;
- struct snd_kcontrol *kctl[SND_JACK_SWITCH_TYPES]; /* control for each key */ int registered; int type; const char *id; char name[100];
- unsigned int key[6]; /* Keep in sync with definitions above */
- unsigned int key[SND_JACK_SWITCH_TYPES]; /* Keep in sync with definitions above */ void *private_data; void (*private_free)(struct snd_jack *);
}; @@ -70,7 +72,7 @@ struct snd_jack { #ifdef CONFIG_SND_JACK
int snd_jack_new(struct snd_card *card, const char *id, int type,
struct snd_jack **jack);
int idx, struct snd_jack **jack);
The biggest point here is that the patch changes the API function, from both API and behavioral POV. That's why you need to modify so many patches in a single patch.
Try to create a new function doing this and keep the old API and behavior as is. Then adapt / replace with the new API gradually. And in the last step, you can obsolete the former API.
Or, at least keep snd_sock_jack_new() as is without additional index but just use idx=0. Then a half of the whole patch can be reduced.
Above all, the multiple indices don't work anyway with the snd_jack stuff in the current form. The index was introduced only for kjack, and for HD-audio, snd_jack is provided just for a compatibility reason, thus it doesn't matter much even if the multiple indices don't work with it.
That being said, before going further, we need to consider how to handle the input jack stuff with multiple indices.
void snd_jack_set_parent(struct snd_jack *jack, struct device *parent); int snd_jack_set_key(struct snd_jack *jack, enum snd_jack_types type, int keytype); @@ -80,7 +82,7 @@ void snd_jack_report(struct snd_jack *jack, int status); #else
static inline int snd_jack_new(struct snd_card *card, const char *id, int type,
struct snd_jack **jack)
int idx, struct snd_jack **jack){ return 0; } diff --git a/include/sound/soc.h b/include/sound/soc.h index 6eabee7..31bea52 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -436,7 +436,7 @@ int snd_soc_platform_trigger(struct snd_pcm_substream *substream,
/* Jack reporting */ int snd_soc_jack_new(struct snd_soc_codec *codec, const char *id, int type,
struct snd_soc_jack *jack);
int idx, struct snd_soc_jack *jack);void snd_soc_jack_report(struct snd_soc_jack *jack, int status, int mask); int snd_soc_jack_add_pins(struct snd_soc_jack *jack, int count, struct snd_soc_jack_pin *pins); diff --git a/sound/core/Kconfig b/sound/core/Kconfig index c0c2f57..8167615 100644 --- a/sound/core/Kconfig +++ b/sound/core/Kconfig @@ -20,6 +20,7 @@ config SND_COMPRESS_OFFLOAD # to avoid having to force INPUT on. config SND_JACK bool
- select SND_KCTL_JACK
config SND_SEQUENCER tristate "Sequencer support" diff --git a/sound/core/ctljack.c b/sound/core/ctljack.c index e4b38fb..441158d 100644 --- a/sound/core/ctljack.c +++ b/sound/core/ctljack.c @@ -14,7 +14,7 @@ #include <sound/core.h> #include <sound/control.h>
-#define jack_detect_kctl_info snd_ctl_boolean_mono_info +#define jack_detect_kctl_info snd_ctl_boolean_mono_info
static int jack_detect_kctl_get(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol) @@ -38,6 +38,7 @@ snd_kctl_jack_new(const char *name, int idx, void *private_data) kctl = snd_ctl_new1(&jack_detect_kctl, private_data); if (!kctl) return NULL;
- snprintf(kctl->id.name, sizeof(kctl->id.name), "%s Jack", name); kctl->id.index = idx; kctl->private_value = 0;
No unnecessary space changes please.
diff --git a/sound/core/jack.c b/sound/core/jack.c index b35fe73..128154b 100644 --- a/sound/core/jack.c +++ b/sound/core/jack.c @@ -24,6 +24,7 @@ #include <linux/module.h> #include <sound/jack.h> #include <sound/core.h> +#include <sound/control.h>
static int jack_switch_types[SND_JACK_SWITCH_TYPES] = { SW_HEADPHONE_INSERT, @@ -37,6 +38,7 @@ static int jack_switch_types[SND_JACK_SWITCH_TYPES] = { static int snd_jack_dev_free(struct snd_device *device) { struct snd_jack *jack = device->device_data;
int i;
if (jack->private_free) jack->private_free(jack);
@@ -48,19 +50,54 @@ static int snd_jack_dev_free(struct snd_device *device) else input_free_device(jack->input_dev);
/* Free available KControls*/
for (i = 0; i < SND_JACK_SWITCH_TYPES; ++i)
if (jack->kctl[i])snd_ctl_remove(device->card, jack->kctl[i]);kfree(jack->id); kfree(jack);
return 0;
}
+const char * jack_get_name_by_key(const char *name, int key) +{
- char *jack_name;
- size_t jack_name_size;
- const char *key_name;
- switch(key) {
- case SW_HEADPHONE_INSERT: key_name = "Headphone"; break;
- case SW_MICROPHONE_INSERT: key_name = "Mic"; break;
- case SW_LINEOUT_INSERT: key_name = "Line Out"; break;
- case SW_JACK_PHYSICAL_INSERT: key_name = "Mechanical"; break;
- case SW_VIDEOOUT_INSERT: key_name = "Video Out"; break;
- case SW_LINEIN_INSERT: key_name = "Line In"; break;
- default: key_name = "Unknown";
- }
- /* Avoid duplicate name in KControl */
- if (strcmp(name, key_name) != 0) {
Better to check via strstr() or such. The name can be like "Front Headphone".
/* allocate necessary memory space only */jack_name_size = strlen(name) + strlen(key_name) + 4;jack_name = kmalloc(jack_name_size, GFP_KERNEL);
This leads to a memory leak. Make this function to put a string on the given buffer instead of returning a string pointer.
snprintf(jack_name, jack_name_size, "%s (%s)", name, key_name);- } else {
jack_name = (char *)name;- }
- return jack_name;
+}
static int snd_jack_dev_register(struct snd_device *device) { struct snd_jack *jack = device->device_data; struct snd_card *card = device->card; int err, i;
- snprintf(jack->name, sizeof(jack->name), "%s %s",
- snprintf(jack->name, sizeof(jack->name), "%s %s Jack", card->shortname, jack->id);
This breaks the compatibility with the existing code. You must not change the name of the existing input jack element. Some drivers create "Headphone" and some do "Headphone Jack", yes. It's bad, but too late to change.
jack->input_dev->name = jack->name;
@@ -81,6 +118,18 @@ static int snd_jack_dev_register(struct snd_device *device) input_set_capability(jack->input_dev, EV_KEY, jack->key[i]); }
- /* We don't need to free the control, it's freed by snd_ctl_add itself
if an error occur */- for (i = 0; i < SND_JACK_SWITCH_TYPES; ++i)
if (jack->kctl[i]) {err = snd_ctl_add(card, jack->kctl[i]);if (err < 0) {pr_notice("%s: ALSA Jack Control not available for '%s'\n", __func__,jack_get_name_by_key(jack->id, jack_switch_types[i]));jack->kctl[i] = NULL;}}- err = input_register_device(jack->input_dev); if (err == 0) jack->registered = 1;
@@ -91,22 +140,31 @@ static int snd_jack_dev_register(struct snd_device *device) /**
- snd_jack_new - Create a new jack
- @card: the card instance
- @id: an identifying string for this jack
- @id: an identifying string for this jack, " Jack" is appended to the
string- @type: a bitmask of enum snd_jack_type values that can be detected by
this jack
- @idx: The index of the ALSA control created to represent the jack.
- @jjack: Used to provide the allocated jack object to the caller.
- Creates a new jack object.
- This function creates a Jack Kcontrol for each @type id as "@id @type Jack".
- Eg.: A jack with @type SND_JACK_HEADSET will have two KControls created,
- "@id Headphone Jack" and "@id Mic Jack". If @id and @type strings are
- equals, then this KControl will be named "@id Jack" only.
*/
- Return: Zero if successful, or a negative error code on failure.
- On success @jjack will be initialised.
int snd_jack_new(struct snd_card *card, const char *id, int type,
struct snd_jack **jjack)
int idx, struct snd_jack **jjack){ struct snd_jack *jack;
- struct snd_kcontrol *kctl; int err; int i;
- static struct snd_device_ops ops = { .dev_free = snd_jack_dev_free, .dev_register = snd_jack_dev_register,
@@ -129,10 +187,20 @@ int snd_jack_new(struct snd_card *card, const char *id, int type, jack->type = type;
for (i = 0; i < SND_JACK_SWITCH_TYPES; i++)
if (type & (1 << i))
if (type & (1 << i)) { input_set_capability(jack->input_dev, EV_SW, jack_switch_types[i]);/* card is the private_data */kctl = snd_kctl_jack_new(jack_get_name_by_key(id, jack_switch_types[i]), idx, card);if (!kctl) {err = -ENOMEM;goto fail_kctl;}jack->kctl[i] = kctl;}err = snd_device_new(card, SNDRV_DEV_JACK, jack, &ops); if (err < 0) goto fail_input;
@@ -141,6 +209,11 @@ int snd_jack_new(struct snd_card *card, const char *id, int type,
return 0;
+fail_kctl:
- for (i = 0; i < SND_JACK_SWITCH_TYPES; ++i)
if (jack->kctl[i])snd_ctl_remove(card, jack->kctl[i]);fail_input: input_free_device(jack->input_dev); kfree(jack->id); @@ -232,10 +305,15 @@ void snd_jack_report(struct snd_jack *jack, int status)
for (i = 0; i < ARRAY_SIZE(jack_switch_types); i++) { int testbit = 1 << i;
if (jack->type & testbit)
if (jack->type & testbit) { input_report_switch(jack->input_dev, jack_switch_types[i], status & testbit);/* Update ALSA KControl interface */snd_kctl_jack_report((struct snd_card *)jack->kctl[i]->private_data,jack->kctl[i], status & testbit);
Better to do a NULL check. In this patch, snd_ctl_add() error isn't handled as a fatal error, thus jack->kctl[i] can be NULL.
}}
input_sync(jack->input_dev);
diff --git a/sound/pci/hda/Kconfig b/sound/pci/hda/Kconfig index 59c5e9c..561abc7 100644 --- a/sound/pci/hda/Kconfig +++ b/sound/pci/hda/Kconfig @@ -65,14 +65,6 @@ config SND_HDA_INPUT_BEEP_MODE Set 1 to always enable the digital beep interface for HD-audio by default.
-config SND_HDA_INPUT_JACK
- bool "Support jack plugging notification via input layer"
- depends on INPUT=y || INPUT=SND
- select SND_JACK
- help
Say Y here to enable the jack plugging notification viainput layer.
I understand why you remove this Kconfig. But by this action, you introduced an obvious regression, i.e. the input jack control is no longer created for HD-audio.
thanks,
Takashi
On Fri, Aug 09, 2013 at 03:52:04PM +0200, Takashi Iwai wrote:
Above all, the multiple indices don't work anyway with the snd_jack stuff in the current form. The index was introduced only for kjack, and for HD-audio, snd_jack is provided just for a compatibility reason, thus it doesn't matter much even if the multiple indices don't work with it.
That being said, before going further, we need to consider how to handle the input jack stuff with multiple indices.
What's the big problem with indexes and input (hopefully also extcon...) reporting?
- snprintf(jack->name, sizeof(jack->name), "%s %s",
- snprintf(jack->name, sizeof(jack->name), "%s %s Jack", card->shortname, jack->id);
This breaks the compatibility with the existing code. You must not change the name of the existing input jack element. Some drivers create "Headphone" and some do "Headphone Jack", yes. It's bad, but too late to change.
We can probably do something cheap like just check if there's a "Jack" already in the name?
At Fri, 9 Aug 2013 17:39:47 +0100, Mark Brown wrote:
On Fri, Aug 09, 2013 at 03:52:04PM +0200, Takashi Iwai wrote:
Above all, the multiple indices don't work anyway with the snd_jack stuff in the current form. The index was introduced only for kjack, and for HD-audio, snd_jack is provided just for a compatibility reason, thus it doesn't matter much even if the multiple indices don't work with it.
That being said, before going further, we need to consider how to handle the input jack stuff with multiple indices.
What's the big problem with indexes and input (hopefully also extcon...) reporting?
No big problem, but we haven't defined it. Should the index be embedded in the name string or handled in a different way?
- snprintf(jack->name, sizeof(jack->name), "%s %s",
- snprintf(jack->name, sizeof(jack->name), "%s %s Jack", card->shortname, jack->id);
This breaks the compatibility with the existing code. You must not change the name of the existing input jack element. Some drivers create "Headphone" and some do "Headphone Jack", yes. It's bad, but too late to change.
We can probably do something cheap like just check if there's a "Jack" already in the name?
Yes, I thought of it, too. But there are some funky names like "hook_switch". Also, there is "Headphones", too.
So, IMO, the function should accept a special name for the input-jack for compatibility reason while creating standard names for kjack, or so...
Takashi
Hi Takashi,
On Fri, Aug 9, 2013 at 6:52 AM, Takashi Iwai tiwai@suse.de wrote:
At Thu, 8 Aug 2013 23:21:55 -0700, Felipe F. Tonello wrote:
From: "Felipe F. Tonello" eu@felipetonello.com
This patch adds jack support for ALSA KControl.
This support is necessary since the new kcontrol is used by user-space daemons, such as PulseAudio(>=2.0), to do jack detection.)
Also, HDA's CONFIG_SND_HDA_INPUT_JACK is disabled because it causes to use standard snd_jack_new() to create jacks. This can cause conflict since this codec creates jack controls directly.
It makes sure that all codecs using ALSA jack API are updated to use the new API.
Well, while this is a good move forward, this patch is a bit too intrusive as a single patch. It breaks way too many. "Break then fix" is no good attitude, especially when it's just something for future.
I agree with you, but unfortunately I had to do that due the non-standard way that jacks are been handled in the kernel and reported to user-space.
I believe this is a classic case where we need a well defined kernel API to user-space. I'm not necessarily saying about internal kernel API/ABI, but the one which is exported to user-space. And to be honest, it's kind common to see internal API's been changed.
And that's the reason jack detection only work, out of the box, with Intel's HD-audio. Which I think it's pretty bad in the stage we are. More and more embedded running PulseAudio and other core user-space daemons.
[...]
diff --git a/include/sound/control.h b/include/sound/control.h index 5358892..ffeb6b6 100644 --- a/include/sound/control.h +++ b/include/sound/control.h @@ -242,6 +242,6 @@ void snd_ctl_sync_vmaster(struct snd_kcontrol *kctl, bool hook_only); struct snd_kcontrol * snd_kctl_jack_new(const char *name, int idx, void *private_data); void snd_kctl_jack_report(struct snd_card *card,
struct snd_kcontrol *kctl, bool status);
struct snd_kcontrol *kctl, bool status);Please don't include space or coding-fix changes.
True. I changed back to bool instead of checking out the source file :P
#endif /* __SOUND_CONTROL_H */ diff --git a/include/sound/jack.h b/include/sound/jack.h index 5891657..0d36f20 100644 --- a/include/sound/jack.h +++ b/include/sound/jack.h @@ -26,6 +26,7 @@ #include <sound/core.h>
struct input_dev; +struct snd_kcontrol;
/**
- Jack types which can be reported. These values are used as a
@@ -58,11 +59,12 @@ enum snd_jack_types {
struct snd_jack { struct input_dev *input_dev;
struct snd_kcontrol *kctl[SND_JACK_SWITCH_TYPES]; /* control for each key */ int registered; int type; const char *id; char name[100];
unsigned int key[6]; /* Keep in sync with definitions above */
unsigned int key[SND_JACK_SWITCH_TYPES]; /* Keep in sync with definitions above */ void *private_data; void (*private_free)(struct snd_jack *);}; @@ -70,7 +72,7 @@ struct snd_jack { #ifdef CONFIG_SND_JACK
int snd_jack_new(struct snd_card *card, const char *id, int type,
struct snd_jack **jack);
int idx, struct snd_jack **jack);The biggest point here is that the patch changes the API function, from both API and behavioral POV. That's why you need to modify so many patches in a single patch.
Try to create a new function doing this and keep the old API and behavior as is. Then adapt / replace with the new API gradually. And in the last step, you can obsolete the former API.
Or, at least keep snd_sock_jack_new() as is without additional index but just use idx=0. Then a half of the whole patch can be reduced.
Above all, the multiple indices don't work anyway with the snd_jack stuff in the current form. The index was introduced only for kjack, and for HD-audio, snd_jack is provided just for a compatibility reason, thus it doesn't matter much even if the multiple indices don't work with it.
That being said, before going further, we need to consider how to handle the input jack stuff with multiple indices.
[...]
diff --git a/sound/core/ctljack.c b/sound/core/ctljack.c index e4b38fb..441158d 100644 --- a/sound/core/ctljack.c +++ b/sound/core/ctljack.c @@ -14,7 +14,7 @@ #include <sound/core.h> #include <sound/control.h>
-#define jack_detect_kctl_info snd_ctl_boolean_mono_info +#define jack_detect_kctl_info snd_ctl_boolean_mono_info
static int jack_detect_kctl_get(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol) @@ -38,6 +38,7 @@ snd_kctl_jack_new(const char *name, int idx, void *private_data) kctl = snd_ctl_new1(&jack_detect_kctl, private_data); if (!kctl) return NULL;
snprintf(kctl->id.name, sizeof(kctl->id.name), "%s Jack", name); kctl->id.index = idx; kctl->private_value = 0;No unnecessary space changes please.
Ok.
diff --git a/sound/core/jack.c b/sound/core/jack.c index b35fe73..128154b 100644 --- a/sound/core/jack.c +++ b/sound/core/jack.c
[...]
+const char * jack_get_name_by_key(const char *name, int key) +{
char *jack_name;size_t jack_name_size;const char *key_name;switch(key) {case SW_HEADPHONE_INSERT: key_name = "Headphone"; break;case SW_MICROPHONE_INSERT: key_name = "Mic"; break;case SW_LINEOUT_INSERT: key_name = "Line Out"; break;case SW_JACK_PHYSICAL_INSERT: key_name = "Mechanical"; break;case SW_VIDEOOUT_INSERT: key_name = "Video Out"; break;case SW_LINEIN_INSERT: key_name = "Line In"; break;default: key_name = "Unknown";}/* Avoid duplicate name in KControl */if (strcmp(name, key_name) != 0) {Better to check via strstr() or such. The name can be like "Front Headphone".
Ok.
/* allocate necessary memory space only */jack_name_size = strlen(name) + strlen(key_name) + 4;jack_name = kmalloc(jack_name_size, GFP_KERNEL);This leads to a memory leak. Make this function to put a string on the given buffer instead of returning a string pointer.
snprintf(jack_name, jack_name_size, "%s (%s)", name, key_name);} else {jack_name = (char *)name;}return jack_name;+}
static int snd_jack_dev_register(struct snd_device *device) { struct snd_jack *jack = device->device_data; struct snd_card *card = device->card; int err, i;
snprintf(jack->name, sizeof(jack->name), "%s %s",
snprintf(jack->name, sizeof(jack->name), "%s %s Jack", card->shortname, jack->id);This breaks the compatibility with the existing code. You must not change the name of the existing input jack element. Some drivers create "Headphone" and some do "Headphone Jack", yes. It's bad, but too late to change.
Why? Can't we just fix those names in those codecs? As You see in my second patch, only few of them are using weird names. I know that this will potentially break user-space, but the trade off is not to standardize the Jack API. What do you think?
[...]
@@ -232,10 +305,15 @@ void snd_jack_report(struct snd_jack *jack, int status)
for (i = 0; i < ARRAY_SIZE(jack_switch_types); i++) { int testbit = 1 << i;
if (jack->type & testbit)
if (jack->type & testbit) { input_report_switch(jack->input_dev, jack_switch_types[i], status & testbit);/* Update ALSA KControl interface */snd_kctl_jack_report((struct snd_card *)jack->kctl[i]->private_data,jack->kctl[i], status & testbit);Better to do a NULL check. In this patch, snd_ctl_add() error isn't handled as a fatal error, thus jack->kctl[i] can be NULL.
True.
} } input_sync(jack->input_dev);diff --git a/sound/pci/hda/Kconfig b/sound/pci/hda/Kconfig index 59c5e9c..561abc7 100644 --- a/sound/pci/hda/Kconfig +++ b/sound/pci/hda/Kconfig @@ -65,14 +65,6 @@ config SND_HDA_INPUT_BEEP_MODE Set 1 to always enable the digital beep interface for HD-audio by default.
-config SND_HDA_INPUT_JACK
bool "Support jack plugging notification via input layer"depends on INPUT=y || INPUT=SNDselect SND_JACKhelpSay Y here to enable the jack plugging notification viainput layer.I understand why you remove this Kconfig. But by this action, you introduced an obvious regression, i.e. the input jack control is no longer created for HD-audio.
I did this just to see what some HD-audio developers whould say. Because what I would like to see is HD-audio codec also using snd_jack and not export those kct jack functions anymore.
BTW, who uses these input events anyway? Woudn't be better just to have standard way (ALSA KControl) to report it?
Thanks for the comments,
Felipe Tonello
At Fri, 9 Aug 2013 10:36:04 -0700, Felipe Tonello wrote:
Hi Takashi,
On Fri, Aug 9, 2013 at 6:52 AM, Takashi Iwai tiwai@suse.de wrote:
At Thu, 8 Aug 2013 23:21:55 -0700, Felipe F. Tonello wrote:
From: "Felipe F. Tonello" eu@felipetonello.com
This patch adds jack support for ALSA KControl.
This support is necessary since the new kcontrol is used by user-space daemons, such as PulseAudio(>=2.0), to do jack detection.)
Also, HDA's CONFIG_SND_HDA_INPUT_JACK is disabled because it causes to use standard snd_jack_new() to create jacks. This can cause conflict since this codec creates jack controls directly.
It makes sure that all codecs using ALSA jack API are updated to use the new API.
Well, while this is a good move forward, this patch is a bit too intrusive as a single patch. It breaks way too many. "Break then fix" is no good attitude, especially when it's just something for future.
I agree with you, but unfortunately I had to do that due the non-standard way that jacks are been handled in the kernel and reported to user-space.
I believe this is a classic case where we need a well defined kernel API to user-space. I'm not necessarily saying about internal kernel API/ABI, but the one which is exported to user-space. And to be honest, it's kind common to see internal API's been changed.
And that's the reason jack detection only work, out of the box, with Intel's HD-audio. Which I think it's pretty bad in the stage we are. More and more embedded running PulseAudio and other core user-space daemons.
I don't mean about the additional support of kctl jack ctl on ASoC. It's a damn good thing.
The problem is that you're trying to break the existing stuff, and it's done in a single shot without describing details what to do and what breaks. In other words, the proper "process" is missing in your approach.
[...]
diff --git a/include/sound/control.h b/include/sound/control.h index 5358892..ffeb6b6 100644 --- a/include/sound/control.h +++ b/include/sound/control.h @@ -242,6 +242,6 @@ void snd_ctl_sync_vmaster(struct snd_kcontrol *kctl, bool hook_only); struct snd_kcontrol * snd_kctl_jack_new(const char *name, int idx, void *private_data); void snd_kctl_jack_report(struct snd_card *card,
struct snd_kcontrol *kctl, bool status);
struct snd_kcontrol *kctl, bool status);Please don't include space or coding-fix changes.
True. I changed back to bool instead of checking out the source file :P
#endif /* __SOUND_CONTROL_H */ diff --git a/include/sound/jack.h b/include/sound/jack.h index 5891657..0d36f20 100644 --- a/include/sound/jack.h +++ b/include/sound/jack.h @@ -26,6 +26,7 @@ #include <sound/core.h>
struct input_dev; +struct snd_kcontrol;
/**
- Jack types which can be reported. These values are used as a
@@ -58,11 +59,12 @@ enum snd_jack_types {
struct snd_jack { struct input_dev *input_dev;
struct snd_kcontrol *kctl[SND_JACK_SWITCH_TYPES]; /* control for each key */ int registered; int type; const char *id; char name[100];
unsigned int key[6]; /* Keep in sync with definitions above */
unsigned int key[SND_JACK_SWITCH_TYPES]; /* Keep in sync with definitions above */ void *private_data; void (*private_free)(struct snd_jack *);}; @@ -70,7 +72,7 @@ struct snd_jack { #ifdef CONFIG_SND_JACK
int snd_jack_new(struct snd_card *card, const char *id, int type,
struct snd_jack **jack);
int idx, struct snd_jack **jack);The biggest point here is that the patch changes the API function, from both API and behavioral POV. That's why you need to modify so many patches in a single patch.
Try to create a new function doing this and keep the old API and behavior as is. Then adapt / replace with the new API gradually. And in the last step, you can obsolete the former API.
Or, at least keep snd_sock_jack_new() as is without additional index but just use idx=0. Then a half of the whole patch can be reduced.
Above all, the multiple indices don't work anyway with the snd_jack stuff in the current form. The index was introduced only for kjack, and for HD-audio, snd_jack is provided just for a compatibility reason, thus it doesn't matter much even if the multiple indices don't work with it.
That being said, before going further, we need to consider how to handle the input jack stuff with multiple indices.
[...]
diff --git a/sound/core/ctljack.c b/sound/core/ctljack.c index e4b38fb..441158d 100644 --- a/sound/core/ctljack.c +++ b/sound/core/ctljack.c @@ -14,7 +14,7 @@ #include <sound/core.h> #include <sound/control.h>
-#define jack_detect_kctl_info snd_ctl_boolean_mono_info +#define jack_detect_kctl_info snd_ctl_boolean_mono_info
static int jack_detect_kctl_get(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol) @@ -38,6 +38,7 @@ snd_kctl_jack_new(const char *name, int idx, void *private_data) kctl = snd_ctl_new1(&jack_detect_kctl, private_data); if (!kctl) return NULL;
snprintf(kctl->id.name, sizeof(kctl->id.name), "%s Jack", name); kctl->id.index = idx; kctl->private_value = 0;No unnecessary space changes please.
Ok.
diff --git a/sound/core/jack.c b/sound/core/jack.c index b35fe73..128154b 100644 --- a/sound/core/jack.c +++ b/sound/core/jack.c
[...]
+const char * jack_get_name_by_key(const char *name, int key) +{
char *jack_name;size_t jack_name_size;const char *key_name;switch(key) {case SW_HEADPHONE_INSERT: key_name = "Headphone"; break;case SW_MICROPHONE_INSERT: key_name = "Mic"; break;case SW_LINEOUT_INSERT: key_name = "Line Out"; break;case SW_JACK_PHYSICAL_INSERT: key_name = "Mechanical"; break;case SW_VIDEOOUT_INSERT: key_name = "Video Out"; break;case SW_LINEIN_INSERT: key_name = "Line In"; break;default: key_name = "Unknown";}/* Avoid duplicate name in KControl */if (strcmp(name, key_name) != 0) {Better to check via strstr() or such. The name can be like "Front Headphone".
Ok.
/* allocate necessary memory space only */jack_name_size = strlen(name) + strlen(key_name) + 4;jack_name = kmalloc(jack_name_size, GFP_KERNEL);This leads to a memory leak. Make this function to put a string on the given buffer instead of returning a string pointer.
snprintf(jack_name, jack_name_size, "%s (%s)", name, key_name);} else {jack_name = (char *)name;}return jack_name;+}
static int snd_jack_dev_register(struct snd_device *device) { struct snd_jack *jack = device->device_data; struct snd_card *card = device->card; int err, i;
snprintf(jack->name, sizeof(jack->name), "%s %s",
snprintf(jack->name, sizeof(jack->name), "%s %s Jack", card->shortname, jack->id);This breaks the compatibility with the existing code. You must not change the name of the existing input jack element. Some drivers create "Headphone" and some do "Headphone Jack", yes. It's bad, but too late to change.
Why? Can't we just fix those names in those codecs?
No. The name string is the only identifier in this case, so if you change it, it leads to a regression.
As You see in my second patch, only few of them are using weird names.
You broke the things and fixed at next. This is a bad attitude, as already mentioned.
I know that this will potentially break user-space, but the trade off is not to standardize the Jack API. What do you think?
No. You cannot break. This is a general golden rule.
The only exception would be if the user-space side will adapt the change accordingly together with the kernel change.
[...]
@@ -232,10 +305,15 @@ void snd_jack_report(struct snd_jack *jack, int status)
for (i = 0; i < ARRAY_SIZE(jack_switch_types); i++) { int testbit = 1 << i;
if (jack->type & testbit)
if (jack->type & testbit) { input_report_switch(jack->input_dev, jack_switch_types[i], status & testbit);/* Update ALSA KControl interface */snd_kctl_jack_report((struct snd_card *)jack->kctl[i]->private_data,jack->kctl[i], status & testbit);Better to do a NULL check. In this patch, snd_ctl_add() error isn't handled as a fatal error, thus jack->kctl[i] can be NULL.
True.
} } input_sync(jack->input_dev);diff --git a/sound/pci/hda/Kconfig b/sound/pci/hda/Kconfig index 59c5e9c..561abc7 100644 --- a/sound/pci/hda/Kconfig +++ b/sound/pci/hda/Kconfig @@ -65,14 +65,6 @@ config SND_HDA_INPUT_BEEP_MODE Set 1 to always enable the digital beep interface for HD-audio by default.
-config SND_HDA_INPUT_JACK
bool "Support jack plugging notification via input layer"depends on INPUT=y || INPUT=SNDselect SND_JACKhelpSay Y here to enable the jack plugging notification viainput layer.I understand why you remove this Kconfig. But by this action, you introduced an obvious regression, i.e. the input jack control is no longer created for HD-audio.
I did this just to see what some HD-audio developers whould say. Because what I would like to see is HD-audio codec also using snd_jack and not export those kct jack functions anymore.
Even if you would like so, you don't rule the world yet, so it can't be a reason to disable the whole thing out of sudden :)
BTW, who uses these input events anyway? Woudn't be better just to have standard way (ALSA KControl) to report it?
Felipe, why wouldn't you drop the whole input jack code for ASoC even after you implement kctl jack controls, then? The same logic can be applied to HD-audio input jack controls, too.
But, don't get me wrong: I'm not against the action itself, the removal of input jack support in HD-audio. I myself did propose this once ago. Again, what's missing in your approach is the proper process.
An easier (or lazier) way to manage this problem would be:
- Think whether removal of input-jack support is really needed for HD-audio; for example, if you integrate snd_jack stuff to support both input-jack and kctl jack, HD-audio driver can use it solely instead of calling snd_kctl stuff. Then both input and kctl jacks will be supported automagically.
- If it's still easier to handle kctl jacks without input jacks in HD-audio, propose the removal at first on the list, get the general consensus. Then put the removal patch in your series at first.
- Try to keep snd_jack_new() intact but create a new API function for creating both input and kctl jacks. This would accept two different name strings, one for input jack and one for kctl, with an additional index, if needed. The different names are needed not to break the things.
- Replace snd_soc_jack_new() with the new function, but you don't have to add the index argument yet at this point. The handling of multiple input-jack instances with indices isn't defined yet, so the simplest solution would be just to skip the multiple indices. It should be good enough for ASoC.
- Replace snd_jack_new() in the rest.
- If wanted, obsolete snd_jack_new(), but keep only the new API.
Takashi
On Mon, Aug 12, 2013 at 12:39:10PM +0200, Takashi Iwai wrote:
But, don't get me wrong: I'm not against the action itself, the removal of input jack support in HD-audio. I myself did propose this once ago. Again, what's missing in your approach is the proper process.
- Think whether removal of input-jack support is really needed for HD-audio; for example, if you integrate snd_jack stuff to support both input-jack and kctl jack, HD-audio driver can use it solely instead of calling snd_kctl stuff. Then both input and kctl jacks will be supported automagically.
I think this is the best approach - just get everything using jack.c for all types of jack and deal with things there. We've also got extcon based reporting to consider, having a single place where all the userspace interfaces are implemented will mean we can just add that.
This is why I put everything in jack.c in the first place :(
Hi Takashi,
On Mon, Aug 12, 2013 at 3:39 AM, Takashi Iwai tiwai@suse.de wrote:
At Fri, 9 Aug 2013 10:36:04 -0700, Felipe Tonello wrote:
Hi Takashi,
On Fri, Aug 9, 2013 at 6:52 AM, Takashi Iwai tiwai@suse.de wrote:
At Thu, 8 Aug 2013 23:21:55 -0700, Felipe F. Tonello wrote:
From: "Felipe F. Tonello" eu@felipetonello.com
This patch adds jack support for ALSA KControl.
This support is necessary since the new kcontrol is used by user-space daemons, such as PulseAudio(>=2.0), to do jack detection.)
Also, HDA's CONFIG_SND_HDA_INPUT_JACK is disabled because it causes to use standard snd_jack_new() to create jacks. This can cause conflict since this codec creates jack controls directly.
It makes sure that all codecs using ALSA jack API are updated to use the new API.
Well, while this is a good move forward, this patch is a bit too intrusive as a single patch. It breaks way too many. "Break then fix" is no good attitude, especially when it's just something for future.
I agree with you, but unfortunately I had to do that due the non-standard way that jacks are been handled in the kernel and reported to user-space.
I believe this is a classic case where we need a well defined kernel API to user-space. I'm not necessarily saying about internal kernel API/ABI, but the one which is exported to user-space. And to be honest, it's kind common to see internal API's been changed.
And that's the reason jack detection only work, out of the box, with Intel's HD-audio. Which I think it's pretty bad in the stage we are. More and more embedded running PulseAudio and other core user-space daemons.
I don't mean about the additional support of kctl jack ctl on ASoC. It's a damn good thing.
The problem is that you're trying to break the existing stuff, and it's done in a single shot without describing details what to do and what breaks. In other words, the proper "process" is missing in your approach.
Ok. I will try to follow your instructions.
[...]
I know that this will potentially break user-space, but the trade off is not to standardize the Jack API. What do you think?
No. You cannot break. This is a general golden rule.
The only exception would be if the user-space side will adapt the change accordingly together with the kernel change.
Got it.
[...]
} } input_sync(jack->input_dev);diff --git a/sound/pci/hda/Kconfig b/sound/pci/hda/Kconfig index 59c5e9c..561abc7 100644 --- a/sound/pci/hda/Kconfig +++ b/sound/pci/hda/Kconfig @@ -65,14 +65,6 @@ config SND_HDA_INPUT_BEEP_MODE Set 1 to always enable the digital beep interface for HD-audio by default.
-config SND_HDA_INPUT_JACK
bool "Support jack plugging notification via input layer"depends on INPUT=y || INPUT=SNDselect SND_JACKhelpSay Y here to enable the jack plugging notification viainput layer.I understand why you remove this Kconfig. But by this action, you introduced an obvious regression, i.e. the input jack control is no longer created for HD-audio.
I did this just to see what some HD-audio developers whould say. Because what I would like to see is HD-audio codec also using snd_jack and not export those kct jack functions anymore.
Even if you would like so, you don't rule the world yet, so it can't be a reason to disable the whole thing out of sudden :)
BTW, who uses these input events anyway? Woudn't be better just to have standard way (ALSA KControl) to report it?
Felipe, why wouldn't you drop the whole input jack code for ASoC even after you implement kctl jack controls, then? The same logic can be applied to HD-audio input jack controls, too.
Because I tried to maintain this back compatibility. But now I see that it didn't maintain a lot anyway, because of the jack names.
But, don't get me wrong: I'm not against the action itself, the removal of input jack support in HD-audio. I myself did propose this once ago. Again, what's missing in your approach is the proper process.
I see that my patches were kind radical. But at least I'm getting things clearer now.
An easier (or lazier) way to manage this problem would be:
Think whether removal of input-jack support is really needed for HD-audio; for example, if you integrate snd_jack stuff to support both input-jack and kctl jack, HD-audio driver can use it solely instead of calling snd_kctl stuff. Then both input and kctl jacks will be supported automagically.
If it's still easier to handle kctl jacks without input jacks in HD-audio, propose the removal at first on the list, get the general consensus. Then put the removal patch in your series at first.
Try to keep snd_jack_new() intact but create a new API function for creating both input and kctl jacks. This would accept two different name strings, one for input jack and one for kctl, with an additional index, if needed. The different names are needed not to break the things.
Replace snd_soc_jack_new() with the new function, but you don't have to add the index argument yet at this point. The handling of multiple input-jack instances with indices isn't defined yet, so the simplest solution would be just to skip the multiple indices. It should be good enough for ASoC.
Replace snd_jack_new() in the rest.
If wanted, obsolete snd_jack_new(), but keep only the new API.
Ok. Nice.
Thanks for all the comments. I appreciate very much.
Felipe Tonello
From: "Felipe F. Tonello" eu@felipetonello.com
The new ALSA jack API ensures that " Jack" is appended by default.
It's also good practice that all codecs name jacks based on its type.
Signed-off-by: Felipe F. Tonello eu@felipetonello.com --- sound/soc/mid-x86/mfld_machine.c | 2 +- sound/soc/omap/omap-abe-twl6040.c | 2 +- sound/soc/omap/omap-twl4030.c | 2 +- sound/soc/omap/rx51.c | 2 +- sound/soc/pxa/hx4700.c | 2 +- sound/soc/pxa/palm27x.c | 2 +- sound/soc/pxa/ttc-dkb.c | 4 ++-- sound/soc/pxa/z2.c | 2 +- sound/soc/samsung/goni_wm8994.c | 2 +- sound/soc/samsung/h1940_uda1380.c | 2 +- sound/soc/samsung/rx1950_uda1380.c | 2 +- sound/soc/samsung/smartq_wm8987.c | 2 +- sound/soc/tegra/tegra_alc5632.c | 2 +- sound/soc/tegra/tegra_rt5640.c | 2 +- sound/soc/tegra/tegra_wm8903.c | 4 ++-- 15 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/sound/soc/mid-x86/mfld_machine.c b/sound/soc/mid-x86/mfld_machine.c index e52c836..e2c7978 100644 --- a/sound/soc/mid-x86/mfld_machine.c +++ b/sound/soc/mid-x86/mfld_machine.c @@ -253,7 +253,7 @@ static int mfld_init(struct snd_soc_pcm_runtime *runtime) snd_soc_dapm_disable_pin(dapm, "LINEINR");
/* Headset and button jack detection */ - ret_val = snd_soc_jack_new(codec, "Intel(R) MID Audio Jack", + ret_val = snd_soc_jack_new(codec, "Intel(R) MID Audio", SND_JACK_HEADSET | SND_JACK_BTN_0 | SND_JACK_BTN_1, 0, &mfld_jack); if (ret_val) { diff --git a/sound/soc/omap/omap-abe-twl6040.c b/sound/soc/omap/omap-abe-twl6040.c index a63038b..45ff3bc 100644 --- a/sound/soc/omap/omap-abe-twl6040.c +++ b/sound/soc/omap/omap-abe-twl6040.c @@ -193,7 +193,7 @@ static int omap_abe_twl6040_init(struct snd_soc_pcm_runtime *rtd)
/* Headset jack detection only if it is supported */ if (priv->jack_detection) { - ret = snd_soc_jack_new(codec, "Headset Jack", + ret = snd_soc_jack_new(codec, "Headset", SND_JACK_HEADSET, 0, &hs_jack); if (ret) return ret; diff --git a/sound/soc/omap/omap-twl4030.c b/sound/soc/omap/omap-twl4030.c index 07ba4d3..0619ecb 100644 --- a/sound/soc/omap/omap-twl4030.c +++ b/sound/soc/omap/omap-twl4030.c @@ -189,7 +189,7 @@ static int omap_twl4030_init(struct snd_soc_pcm_runtime *rtd) if (priv->jack_detect > 0) { hs_jack_gpios[0].gpio = priv->jack_detect;
- ret = snd_soc_jack_new(codec, "Headset Jack", SND_JACK_HEADSET, + ret = snd_soc_jack_new(codec, "Headset", SND_JACK_HEADSET, 0, &priv->hs_jack); if (ret) return ret; diff --git a/sound/soc/omap/rx51.c b/sound/soc/omap/rx51.c index 2cec19a..46fe697 100644 --- a/sound/soc/omap/rx51.c +++ b/sound/soc/omap/rx51.c @@ -317,7 +317,7 @@ static int rx51_aic34_init(struct snd_soc_pcm_runtime *rtd) return err;
/* AV jack detection */ - err = snd_soc_jack_new(codec, "AV Jack", + err = snd_soc_jack_new(codec, "AV", SND_JACK_HEADSET | SND_JACK_VIDEOOUT, 0, &rx51_av_jack); if (err) diff --git a/sound/soc/pxa/hx4700.c b/sound/soc/pxa/hx4700.c index 82fd557..3ffe48b 100644 --- a/sound/soc/pxa/hx4700.c +++ b/sound/soc/pxa/hx4700.c @@ -137,7 +137,7 @@ static int hx4700_ak4641_init(struct snd_soc_pcm_runtime *rtd) snd_soc_dapm_nc_pin(dapm, "AUX");
/* Jack detection API stuff */ - err = snd_soc_jack_new(codec, "Headphone Jack", + err = snd_soc_jack_new(codec, "Headphone", SND_JACK_HEADPHONE, 0, &hs_jack); if (err) return err; diff --git a/sound/soc/pxa/palm27x.c b/sound/soc/pxa/palm27x.c index 264be5e..7caba46 100644 --- a/sound/soc/pxa/palm27x.c +++ b/sound/soc/pxa/palm27x.c @@ -97,7 +97,7 @@ static int palm27x_ac97_init(struct snd_soc_pcm_runtime *rtd) snd_soc_dapm_nc_pin(dapm, "MIC2");
/* Jack detection API stuff */ - err = snd_soc_jack_new(codec, "Headphone Jack", + err = snd_soc_jack_new(codec, "Headphone", SND_JACK_HEADPHONE, 0, &hs_jack); if (err) return err; diff --git a/sound/soc/pxa/ttc-dkb.c b/sound/soc/pxa/ttc-dkb.c index 1a462cb..880c01f 100644 --- a/sound/soc/pxa/ttc-dkb.c +++ b/sound/soc/pxa/ttc-dkb.c @@ -86,12 +86,12 @@ static int ttc_pm860x_init(struct snd_soc_pcm_runtime *rtd) snd_soc_dapm_disable_pin(dapm, "Headset Stereophone");
/* Headset jack detection */ - snd_soc_jack_new(codec, "Headphone Jack", SND_JACK_HEADPHONE + snd_soc_jack_new(codec, "Headphone", SND_JACK_HEADPHONE | SND_JACK_BTN_0 | SND_JACK_BTN_1 | SND_JACK_BTN_2, 0, &hs_jack); snd_soc_jack_add_pins(&hs_jack, ARRAY_SIZE(hs_jack_pins), hs_jack_pins); - snd_soc_jack_new(codec, "Microphone Jack", SND_JACK_MICROPHONE, + snd_soc_jack_new(codec, "Microphone", SND_JACK_MICROPHONE, 0, &mic_jack); snd_soc_jack_add_pins(&mic_jack, ARRAY_SIZE(mic_jack_pins), mic_jack_pins); diff --git a/sound/soc/pxa/z2.c b/sound/soc/pxa/z2.c index 082fd24..2bbb2de 100644 --- a/sound/soc/pxa/z2.c +++ b/sound/soc/pxa/z2.c @@ -143,7 +143,7 @@ static int z2_wm8750_init(struct snd_soc_pcm_runtime *rtd) snd_soc_dapm_disable_pin(dapm, "MONO1");
/* Jack detection API stuff */ - ret = snd_soc_jack_new(codec, "Headset Jack", SND_JACK_HEADSET, + ret = snd_soc_jack_new(codec, "Headset", SND_JACK_HEADSET, 0, &hs_jack); if (ret) goto err; diff --git a/sound/soc/samsung/goni_wm8994.c b/sound/soc/samsung/goni_wm8994.c index bdd70a9..2bfa4c9 100644 --- a/sound/soc/samsung/goni_wm8994.c +++ b/sound/soc/samsung/goni_wm8994.c @@ -114,7 +114,7 @@ static int goni_wm8994_init(struct snd_soc_pcm_runtime *rtd) }
/* Headset jack detection */ - ret = snd_soc_jack_new(codec, "Headset Jack", + ret = snd_soc_jack_new(codec, "Headset", SND_JACK_HEADSET | SND_JACK_MECHANICAL | SND_JACK_AVOUT, 0, &jack); diff --git a/sound/soc/samsung/h1940_uda1380.c b/sound/soc/samsung/h1940_uda1380.c index 66b5a33..f8cf0cf 100644 --- a/sound/soc/samsung/h1940_uda1380.c +++ b/sound/soc/samsung/h1940_uda1380.c @@ -186,7 +186,7 @@ static int h1940_uda1380_init(struct snd_soc_pcm_runtime *rtd) snd_soc_dapm_enable_pin(dapm, "Speaker"); snd_soc_dapm_enable_pin(dapm, "Mic Jack");
- snd_soc_jack_new(codec, "Headphone Jack", SND_JACK_HEADPHONE, + snd_soc_jack_new(codec, "Headphone", SND_JACK_HEADPHONE, 0, &hp_jack);
snd_soc_jack_add_pins(&hp_jack, ARRAY_SIZE(hp_jack_pins), diff --git a/sound/soc/samsung/rx1950_uda1380.c b/sound/soc/samsung/rx1950_uda1380.c index 588ad72..f62b845 100644 --- a/sound/soc/samsung/rx1950_uda1380.c +++ b/sound/soc/samsung/rx1950_uda1380.c @@ -231,7 +231,7 @@ static int rx1950_uda1380_init(struct snd_soc_pcm_runtime *rtd) snd_soc_dapm_enable_pin(dapm, "Speaker"); snd_soc_dapm_enable_pin(dapm, "Mic Jack");
- snd_soc_jack_new(codec, "Headphone Jack", SND_JACK_HEADPHONE, + snd_soc_jack_new(codec, "Headphone", SND_JACK_HEADPHONE, 0, &hp_jack);
snd_soc_jack_add_pins(&hp_jack, ARRAY_SIZE(hp_jack_pins), diff --git a/sound/soc/samsung/smartq_wm8987.c b/sound/soc/samsung/smartq_wm8987.c index 71a17a5..053ceea 100644 --- a/sound/soc/samsung/smartq_wm8987.c +++ b/sound/soc/samsung/smartq_wm8987.c @@ -166,7 +166,7 @@ static int smartq_wm8987_init(struct snd_soc_pcm_runtime *rtd) snd_soc_dapm_disable_pin(dapm, "Headphone Jack");
/* Headphone jack detection */ - err = snd_soc_jack_new(codec, "Headphone Jack", + err = snd_soc_jack_new(codec, "Headphone", SND_JACK_HEADPHONE, 0, &smartq_jack); if (err) return err; diff --git a/sound/soc/tegra/tegra_alc5632.c b/sound/soc/tegra/tegra_alc5632.c index 1a44af9..cfacc27 100644 --- a/sound/soc/tegra/tegra_alc5632.c +++ b/sound/soc/tegra/tegra_alc5632.c @@ -109,7 +109,7 @@ static int tegra_alc5632_asoc_init(struct snd_soc_pcm_runtime *rtd) struct snd_soc_dapm_context *dapm = &codec->dapm; struct tegra_alc5632 *machine = snd_soc_card_get_drvdata(codec->card);
- snd_soc_jack_new(codec, "Headset Jack", SND_JACK_HEADSET, + snd_soc_jack_new(codec, "Headset", SND_JACK_HEADSET, 0, &tegra_alc5632_hs_jack); snd_soc_jack_add_pins(&tegra_alc5632_hs_jack, ARRAY_SIZE(tegra_alc5632_hs_jack_pins), diff --git a/sound/soc/tegra/tegra_rt5640.c b/sound/soc/tegra/tegra_rt5640.c index aad20f2..149b12c 100644 --- a/sound/soc/tegra/tegra_rt5640.c +++ b/sound/soc/tegra/tegra_rt5640.c @@ -111,7 +111,7 @@ static int tegra_rt5640_asoc_init(struct snd_soc_pcm_runtime *rtd) struct snd_soc_codec *codec = codec_dai->codec; struct tegra_rt5640 *machine = snd_soc_card_get_drvdata(codec->card);
- snd_soc_jack_new(codec, "Headphones", SND_JACK_HEADPHONE, + snd_soc_jack_new(codec, "Headphone", SND_JACK_HEADPHONE, 0, &tegra_rt5640_hp_jack); snd_soc_jack_add_pins(&tegra_rt5640_hp_jack, ARRAY_SIZE(tegra_rt5640_hp_jack_pins), diff --git a/sound/soc/tegra/tegra_wm8903.c b/sound/soc/tegra/tegra_wm8903.c index 3f38d31..cd297f5 100644 --- a/sound/soc/tegra/tegra_wm8903.c +++ b/sound/soc/tegra/tegra_wm8903.c @@ -178,7 +178,7 @@ static int tegra_wm8903_init(struct snd_soc_pcm_runtime *rtd)
if (gpio_is_valid(machine->gpio_hp_det)) { tegra_wm8903_hp_jack_gpio.gpio = machine->gpio_hp_det; - snd_soc_jack_new(codec, "Headphone Jack", SND_JACK_HEADPHONE, + snd_soc_jack_new(codec, "Headphone", SND_JACK_HEADPHONE, 0, &tegra_wm8903_hp_jack); snd_soc_jack_add_pins(&tegra_wm8903_hp_jack, ARRAY_SIZE(tegra_wm8903_hp_jack_pins), @@ -188,7 +188,7 @@ static int tegra_wm8903_init(struct snd_soc_pcm_runtime *rtd) &tegra_wm8903_hp_jack_gpio); }
- snd_soc_jack_new(codec, "Mic Jack", SND_JACK_MICROPHONE, + snd_soc_jack_new(codec, "Mic", SND_JACK_MICROPHONE, 0, &tegra_wm8903_mic_jack); snd_soc_jack_add_pins(&tegra_wm8903_mic_jack, ARRAY_SIZE(tegra_wm8903_mic_jack_pins),
participants (4)
-
Felipe F. Tonello -
Felipe Tonello
-
Mark Brown -
Takashi Iwai