At Wed, 16 Oct 2013 14:15:36 +0200,
David Henningsson wrote:
>
> Notes/questions:
>
> For mute LEDs we can follow master like we already do on HP machines, but for
> mic mutes, we might have several capture switches. Honestly, on these laptops
> which almost always have autoswitched mics, there is only one recording source
> anyway. Can't we simply remove the extra capture PCMs, that seems easiest?
The fixup would be applied only to specific machines, so they are
without extra capture PCMs, no?
> For some reason CONFIG_THINKPAD_ACPI did not work here (it was not defined),
> but maybe it's due to the way I'm testing, or I'm missing something obvious.
> Not sure.
It can be a module. Use IS_ENABLED() macro.
#if IS_ENABLED(CONFIG_THINKPAD_ACPI)
> There are Thinkpad's with Realtek codecs as well which have mute/micmute.
> Maybe we should consider a more generic solution instead of copy-pasting
> between differen patch_* files?
Maybe. But it's not that wide spread. Let's make this one working at
first. The further code sharing (involving module split eventually)
can be discussed later.
> And also, we're missing a symbol_put in this version. Should we add a new
> "free" fixup action where we can add a call to symbol_put, or do you have a
> better suggestion?
Hm, yes, a new free fixup action is a feasible option.
thanks,
Takashi
>
> Signed-off-by: David Henningsson <david.henningsson@xxxxxxxxxxxxx>
> ---
> sound/pci/hda/patch_conexant.c | 78 ++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 78 insertions(+)
>
> diff --git a/sound/pci/hda/patch_conexant.c b/sound/pci/hda/patch_conexant.c
> index ec68eae..237df7f 100644
> --- a/sound/pci/hda/patch_conexant.c
> +++ b/sound/pci/hda/patch_conexant.c
> @@ -3232,8 +3232,80 @@ enum {
> CXT_FIXUP_HEADPHONE_MIC_PIN,
> CXT_FIXUP_HEADPHONE_MIC,
> CXT_FIXUP_GPIO1,
> + CXT_FIXUP_THINKPAD_ACPI,
> };
>
> +/* FIXME: figure out why #ifdef CONFIG_THINKPAD_ACPI does not work */
> +
> +#if 1 /* #ifdef CONFIG_THINKPAD_ACPI */
> +
> +#include <linux/thinkpad_acpi.h>
> +
> +static int (*mute_led_set_func)(int);
> +static int (*micmute_led_set_func)(int);
> +
> +static void update_tpacpi_mute_led(void *private_data, int enabled)
> +{
> + struct hda_codec *codec = private_data;
> + struct conexant_spec *spec = codec->spec;
> +
> + if (spec->dynamic_eapd)
> + cx_auto_vmaster_hook(private_data, enabled);
> +
> + if (mute_led_set_func)
> + mute_led_set_func(!enabled);
> +}
> +
> +static void update_tpacpi_micmute_led(struct hda_codec *codec,
> + struct snd_ctl_elem_value *ucontrol)
> +{
> + int val;
> + if (!ucontrol || !micmute_led_set_func)
> + return;
> + /* TODO: if this is a stereo switch, better check both channels? What about additional capture switches with indices, on other ADCs? */
> + val = ucontrol->value.integer.value[0];
> + if (strcmp("Capture Switch", ucontrol->id.name) == 0 && ucontrol->id.index == 0)
> + micmute_led_set_func(!val);
> +
> +}
> +
> +static void cxt_fixup_thinkpad_acpi(struct hda_codec *codec,
> + const struct hda_fixup *fix, int action)
> +{
> + struct conexant_spec *spec = codec->spec;
> + if (action == HDA_FIXUP_ACT_PRE_PROBE) {
> + mute_led_set_func = symbol_request(tpacpi_mute_led_set);
> + if (!mute_led_set_func)
> + snd_printk(KERN_WARNING "Failed to find thinkpad-acpi symbol tpacpi_mute_led_set\n");
> + else if (mute_led_set_func(0) < 0) {
> + symbol_put(mute_led_set_func);
> + mute_led_set_func = NULL;
> + }
> +
> + micmute_led_set_func = symbol_request(tpacpi_micmute_led_set);
> + if (!micmute_led_set_func)
> + snd_printk(KERN_WARNING "Failed to find thinkpad-acpi symbol tpacpi_micmute_led_set\n");
> + else if (micmute_led_set_func(0) < 0) {
> + symbol_put(micmute_led_set_func);
> + micmute_led_set_func = NULL;
> + }
> + /* FIXME: when do we call symbol_put? In a new FREE fixup action? */
> + }
> + if (action == HDA_FIXUP_ACT_PROBE && micmute_led_set_func)
> + spec->gen.cap_sync_hook = update_tpacpi_micmute_led;
> + if (action == HDA_FIXUP_ACT_PROBE && mute_led_set_func)
> + spec->gen.vmaster_mute.hook = update_tpacpi_mute_led;
> +}
> +
> +#else
> +
> +static void cxt_fixup_thinkpad_acpi(struct hda_codec *codec,
> + const struct hda_fixup *fix, int action)
> +{
> +}
> +
> +#endif
> +
> static void cxt_fixup_stereo_dmic(struct hda_codec *codec,
> const struct hda_fixup *fix, int action)
> {
> @@ -3344,6 +3416,8 @@ static const struct hda_fixup cxt_fixups[] = {
> [CXT_PINCFG_LENOVO_TP410] = {
> .type = HDA_FIXUP_PINS,
> .v.pins = cxt_pincfg_lenovo_tp410,
> + .chained = true,
> + .chain_id = CXT_FIXUP_THINKPAD_ACPI,
> },
> [CXT_PINCFG_LEMOTE_A1004] = {
> .type = HDA_FIXUP_PINS,
> @@ -3385,6 +3459,10 @@ static const struct hda_fixup cxt_fixups[] = {
> { }
> },
> },
> + [CXT_FIXUP_THINKPAD_ACPI] = {
> + .type = HDA_FIXUP_FUNC,
> + .v.func = cxt_fixup_thinkpad_acpi,
> + },
> };
>
> static const struct snd_pci_quirk cxt5051_fixups[] = {
> --
> 1.7.9.5
>
--
To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
