Re: [PATCH] ALSA: hda - Enable mute/mic-mute LEDs for more Thinkpads with Conexant codec

[David Henningsson <david.henningsson@canonical.com>]

On 11/27/2013 04:26 AM, Hui Wang wrote:
> Most Thinkpad Edge series laptops use conexant codec, so far although
> the codecs have different minor Vendor Id and minor Subsystem Id, but
> they all belong to the cxt_5066 family, this change can make the
> mute/mic-mute LEDs support more generic among cxt_5066 family.

Hi Hui and thanks for your patch!

It's also nice that you have tested the patch on several machines and
found it to be working.

As for the actual implementation - as the rest of the thinkpad-acpi code
is copy-pasted between patch_realtek and patch_conexant, I think this
would also apply here, i e, no code in hda_local.h, instead copy from
patch_realtek to patch_conexant as necessary.

While copy-pasting code is not the most elegant of solutions, I got the
impression from Takashi that he preferred that for now.

> 
> Cc: Alex Hung <alex.hung@canonical.com>
> Cc: David Henningsson <david.henningsson@canonical.com>
> Signed-off-by: Hui Wang <hui.wang@canonical.com>
> ---
>  sound/pci/hda/hda_local.h      | 22 ++++++++++++++++++++++
>  sound/pci/hda/patch_conexant.c |  3 +++
>  sound/pci/hda/patch_realtek.c  | 20 --------------------
>  3 files changed, 25 insertions(+), 20 deletions(-)
> 
> diff --git a/sound/pci/hda/hda_local.h b/sound/pci/hda/hda_local.h
> index d398b64..1155958 100644
> --- a/sound/pci/hda/hda_local.h
> +++ b/sound/pci/hda/hda_local.h
> @@ -777,4 +777,26 @@ void snd_hdmi_write_eld_info(struct hdmi_eld *eld,
>  #define SND_PRINT_CHANNEL_ALLOCATION_ADVISED_BUFSIZE 80
>  void snd_print_channel_allocation(int spk_alloc, char *buf, int buflen);
>  
> +#if IS_ENABLED(CONFIG_THINKPAD_ACPI)
> +#include <acpi/acpi.h>
> +
> +static acpi_status acpi_check_cb(acpi_handle handle, u32 lvl, void *context,
> +				 void **rv)
> +{
> +	bool *found = context;
> +	*found = true;
> +	return AE_OK;
> +}
> +
> +static inline bool is_thinkpad(struct hda_codec *codec)
> +{
> +	bool found = false;
> +	if (codec->subsystem_id >> 16 != 0x17aa)
> +		return false;
> +	if (ACPI_SUCCESS(acpi_get_devices("LEN0068", acpi_check_cb, &found, NULL)) && found)
> +		return true;
> +	found = false;
> +	return ACPI_SUCCESS(acpi_get_devices("IBM0068", acpi_check_cb, &found, NULL)) && found;
> +}
> +#endif
>  #endif /* __SOUND_HDA_LOCAL_H */
> diff --git a/sound/pci/hda/patch_conexant.c b/sound/pci/hda/patch_conexant.c
> index c205bb1..8063ae9 100644
> --- a/sound/pci/hda/patch_conexant.c
> +++ b/sound/pci/hda/patch_conexant.c
> @@ -3279,6 +3279,8 @@ static void cxt_fixup_thinkpad_acpi(struct hda_codec *codec,
>  	bool removefunc = false;
>  
>  	if (action == HDA_FIXUP_ACT_PROBE) {
> +		if (!is_thinkpad(codec))
> +			return;
>  		if (!led_set_func)
>  			led_set_func = symbol_request(tpacpi_led_set);
>  		if (!led_set_func) {
> @@ -3494,6 +3496,7 @@ static const struct snd_pci_quirk cxt5066_fixups[] = {
>  	SND_PCI_QUIRK(0x17aa, 0x3975, "Lenovo U300s", CXT_FIXUP_STEREO_DMIC),
>  	SND_PCI_QUIRK(0x17aa, 0x3977, "Lenovo IdeaPad U310", CXT_FIXUP_STEREO_DMIC),
>  	SND_PCI_QUIRK(0x17aa, 0x397b, "Lenovo S205", CXT_FIXUP_STEREO_DMIC),
> +	SND_PCI_QUIRK_VENDOR(0x17aa, "Thinkpad", CXT_FIXUP_THINKPAD_ACPI),
>  	SND_PCI_QUIRK(0x1c06, 0x2011, "Lemote A1004", CXT_PINCFG_LEMOTE_A1004),
>  	SND_PCI_QUIRK(0x1c06, 0x2012, "Lemote A1205", CXT_PINCFG_LEMOTE_A1205),
>  	{}
> diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
> index 5e42059..bedf35f 100644
> --- a/sound/pci/hda/patch_realtek.c
> +++ b/sound/pci/hda/patch_realtek.c
> @@ -3673,29 +3673,9 @@ static void alc290_fixup_mono_speakers(struct hda_codec *codec,
>  #if IS_ENABLED(CONFIG_THINKPAD_ACPI)
>  
>  #include <linux/thinkpad_acpi.h>
> -#include <acpi/acpi.h>
>  
>  static int (*led_set_func)(int, bool);
>  
> -static acpi_status acpi_check_cb(acpi_handle handle, u32 lvl, void *context,
> -				 void **rv)
> -{
> -	bool *found = context;
> -	*found = true;
> -	return AE_OK;
> -}
> -
> -static bool is_thinkpad(struct hda_codec *codec)
> -{
> -	bool found = false;
> -	if (codec->subsystem_id >> 16 != 0x17aa)
> -		return false;
> -	if (ACPI_SUCCESS(acpi_get_devices("LEN0068", acpi_check_cb, &found, NULL)) && found)
> -		return true;
> -	found = false;
> -	return ACPI_SUCCESS(acpi_get_devices("IBM0068", acpi_check_cb, &found, NULL)) && found;
> -}
> -
>  static void update_tpacpi_mute_led(void *private_data, int enabled)
>  {
>  	if (led_set_func)
> 



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