[alsa-devel] [PATCH] ALSA: hda - Enable mute/mic-mute LEDs for more Thinkpads with Conexant codec
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.
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)
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;@@ -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)
On 11/27/2013 01:17 PM, David Henningsson wrote:
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.
OK, got it, I will send a V2 patch according to your suggestion.
Regards, Hui.
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(-)
At Wed, 27 Nov 2013 06:17:16 +0100, David Henningsson wrote:
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.
If you'd like it to be included in 3.13, please just copy the small code snippet.
For 3.14, we can move the thinkpad-related code into another file, but I have some bad feeling to put it into hda_local.h or whatever the common code base. Maybe just create hda_thinkpad.c or such and include from both patch_realtek.c and patch_conexant.c. As long as the code size is small enough and they are all static, it should be fine.
thanks,
Takashi
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;@@ -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
On 11/27/2013 02:44 PM, Takashi Iwai wrote:
At Wed, 27 Nov 2013 06:17:16 +0100, David Henningsson wrote:
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.
If you'd like it to be included in 3.13, please just copy the small code snippet.
Yes we hope it can be merged to 3.13, so as your request, i just copy the small code snippet in the V2 patch.
For 3.14, we can move the thinkpad-related code into another file, but I have some bad feeling to put it into hda_local.h or whatever the common code base. Maybe just create hda_thinkpad.c or such and include from both patch_realtek.c and patch_conexant.c. As long as the code size is small enough and they are all static, it should be fine.
Good suggestion, if we have more machines to test and the testing expose more problems, we should think about moving the common functions to the hda_thinkpad.c.
Regards, Hui.
thanks,
Takashi
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;@@ -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
At Wed, 27 Nov 2013 15:11:41 +0800, Hui Wang wrote:
On 11/27/2013 02:44 PM, Takashi Iwai wrote:
At Wed, 27 Nov 2013 06:17:16 +0100, David Henningsson wrote:
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.
If you'd like it to be included in 3.13, please just copy the small code snippet.
Yes we hope it can be merged to 3.13, so as your request, i just copy the small code snippet in the V2 patch.
For 3.14, we can move the thinkpad-related code into another file, but I have some bad feeling to put it into hda_local.h or whatever the common code base. Maybe just create hda_thinkpad.c or such and include from both patch_realtek.c and patch_conexant.c. As long as the code size is small enough and they are all static, it should be fine.
Good suggestion, if we have more machines to test and the testing expose more problems, we should think about moving the common functions to the hda_thinkpad.c.
Hrm, this comment makes me worry a bit to include into 3.13. Hasn't been the patch tested fully? The merge window was already closed, and it means that we really need only fixes, the ones that won't regress obviously...
Takashi
Regards, Hui.
thanks,
Takashi
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;@@ -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
On 11/27/2013 03:23 PM, Takashi Iwai wrote:
At Wed, 27 Nov 2013 15:11:41 +0800, Hui Wang wrote:
On 11/27/2013 02:44 PM, Takashi Iwai wrote:
At Wed, 27 Nov 2013 06:17:16 +0100, David Henningsson wrote:
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.
If you'd like it to be included in 3.13, please just copy the small code snippet.
Yes we hope it can be merged to 3.13, so as your request, i just copy the small code snippet in the V2 patch.
For 3.14, we can move the thinkpad-related code into another file, but I have some bad feeling to put it into hda_local.h or whatever the common code base. Maybe just create hda_thinkpad.c or such and include from both patch_realtek.c and patch_conexant.c. As long as the code size is small enough and they are all static, it should be fine.
Good suggestion, if we have more machines to test and the testing expose more problems, we should think about moving the common functions to the hda_thinkpad.c.
Hrm, this comment makes me worry a bit to include into 3.13. Hasn't been the patch tested fully? The merge window was already closed, and it means that we really need only fixes, the ones that won't regress obviously...
Sorry to make you misunderstand, this patch definitely works well on the machines with Conexant codec (cxt5066 family). I have tested this patch on 9 laptops whose codecs all belong to cxt5066 family, they all worked well.
The problem i pointed to was that, assuming we meet a machine with conexant codec in future, but this codec doesn't belong to cxt5066 family, this patch will not work on this machine, but this patch will not introduce regress on this machine as well.
Regards, Hui.
Takashi
At Wed, 27 Nov 2013 15:39:19 +0800, Hui Wang wrote:
On 11/27/2013 03:23 PM, Takashi Iwai wrote:
At Wed, 27 Nov 2013 15:11:41 +0800, Hui Wang wrote:
On 11/27/2013 02:44 PM, Takashi Iwai wrote:
At Wed, 27 Nov 2013 06:17:16 +0100, David Henningsson wrote:
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.
If you'd like it to be included in 3.13, please just copy the small code snippet.
Yes we hope it can be merged to 3.13, so as your request, i just copy the small code snippet in the V2 patch.
For 3.14, we can move the thinkpad-related code into another file, but I have some bad feeling to put it into hda_local.h or whatever the common code base. Maybe just create hda_thinkpad.c or such and include from both patch_realtek.c and patch_conexant.c. As long as the code size is small enough and they are all static, it should be fine.
Good suggestion, if we have more machines to test and the testing expose more problems, we should think about moving the common functions to the hda_thinkpad.c.
Hrm, this comment makes me worry a bit to include into 3.13. Hasn't been the patch tested fully? The merge window was already closed, and it means that we really need only fixes, the ones that won't regress obviously...
Sorry to make you misunderstand, this patch definitely works well on the machines with Conexant codec (cxt5066 family). I have tested this patch on 9 laptops whose codecs all belong to cxt5066 family, they all worked well.
The problem i pointed to was that, assuming we meet a machine with conexant codec in future, but this codec doesn't belong to cxt5066 family, this patch will not work on this machine, but this patch will not introduce regress on this machine as well.
OK, now applied. Thanks.
Takashi
participants (4)
-
David Henningsson -
Hui Wang -
Hui Wang -
Takashi Iwai