| Message ID | 1461064294-2879-1-git-send-email-hadess@hadess.net (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
Bastien Nocera <hadess@hadess.net> writes: > From: Shuduo Sang <shuduo.sang@canonical.com> > > The Thinkpad X1 Carbon 2nd generation (2014) ships with BIOS that will > return HKEY interface version 0x200. It needs thinkpad-acpi support > otherwise it will be routed to wrong branch and the hotkey mask will > be wrong. > > https://bugzilla.kernel.org/show_bug.cgi?id=114731 > > Signed-off-by: Bruce Ma <bruce.ma@canonical.com> > Signed-off-by: Shuduo Sang <shuduo.sang@canonical.com> > --- > drivers/platform/x86/thinkpad_acpi.c | 35 ++++++++++++++++++++++++++++++----- > 1 file changed, 30 insertions(+), 5 deletions(-) > > diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c > index dad2984..c177936 100644 > --- a/drivers/platform/x86/thinkpad_acpi.c > +++ b/drivers/platform/x86/thinkpad_acpi.c > @@ -3357,11 +3357,8 @@ static int __init hotkey_init(struct ibm_init_struct *iibm) > A30, R30, R31, T20-22, X20-21, X22-24. Detected by checking > for HKEY interface version 0x100 */ > if (acpi_evalf(hkey_handle, &hkeyv, "MHKV", "qd")) { > - if ((hkeyv >> 8) != 1) { > - pr_err("unknown version of the HKEY interface: 0x%x\n", > - hkeyv); > - pr_err("please report this to %s\n", TPACPI_MAIL); > - } else { > + switch (hkeyv >> 8) { > + case 1: > /* > * MHKV 0x100 in A31, R40, R40e, > * T4x, X31, and later > @@ -3381,6 +3378,34 @@ static int __init hotkey_init(struct ibm_init_struct *iibm) > } else { > tp_features.hotkey_mask = 1; > } > + break; > + > + case 2: > + /* > + * MHKV 0x200 in X1 > + */ > + vdbg_printk(TPACPI_DBG_INIT | TPACPI_DBG_HKEY, > + "firmware HKEY interface version: 0x%x\n", > + hkeyv); > + > + /* Paranoia check AND init hotkey_all_mask */ > + if (!acpi_evalf(hkey_handle, &hotkey_all_mask, > + "MHKA", "dd", 1)) { > + pr_err("missing MHKA handler, " > + "please report this to %s\n", > + TPACPI_MAIL); > + /* Fallback: pre-init for FN+F3,F4,F12 */ > + hotkey_all_mask = 0x080cU; > + } else { > + tp_features.hotkey_mask = 1; > + } > + break; Why do you duplicate this code block with that single byte changed? If that was intended, then it certainly should be explained. Bjørn -- To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 2016-04-19 at 14:42 +0200, Bjørn Mork wrote: > Bastien Nocera <hadess@hadess.net> writes: > > > > > From: Shuduo Sang <shuduo.sang@canonical.com> > > > > The Thinkpad X1 Carbon 2nd generation (2014) ships with BIOS that > > will > > return HKEY interface version 0x200. It needs thinkpad-acpi support > > otherwise it will be routed to wrong branch and the hotkey mask > > will > > be wrong. > > > > https://bugzilla.kernel.org/show_bug.cgi?id=114731 > > > > Signed-off-by: Bruce Ma <bruce.ma@canonical.com> > > Signed-off-by: Shuduo Sang <shuduo.sang@canonical.com> > > --- > > drivers/platform/x86/thinkpad_acpi.c | 35 > > ++++++++++++++++++++++++++++++----- > > 1 file changed, 30 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/platform/x86/thinkpad_acpi.c > > b/drivers/platform/x86/thinkpad_acpi.c > > index dad2984..c177936 100644 > > --- a/drivers/platform/x86/thinkpad_acpi.c > > +++ b/drivers/platform/x86/thinkpad_acpi.c > > @@ -3357,11 +3357,8 @@ static int __init hotkey_init(struct > > ibm_init_struct *iibm) > > A30, R30, R31, T20-22, X20-21, X22-24. Detected by > > checking > > for HKEY interface version 0x100 */ > > if (acpi_evalf(hkey_handle, &hkeyv, "MHKV", "qd")) { > > - if ((hkeyv >> 8) != 1) { > > - pr_err("unknown version of the HKEY > > interface: 0x%x\n", > > - hkeyv); > > - pr_err("please report this to %s\n", > > TPACPI_MAIL); > > - } else { > > + switch (hkeyv >> 8) { > > + case 1: > > /* > > * MHKV 0x100 in A31, R40, R40e, > > * T4x, X31, and later > > @@ -3381,6 +3378,34 @@ static int __init hotkey_init(struct > > ibm_init_struct *iibm) > > } else { > > tp_features.hotkey_mask = 1; > > } > > + break; > > + > > + case 2: > > + /* > > + * MHKV 0x200 in X1 > > + */ > > + vdbg_printk(TPACPI_DBG_INIT | > > TPACPI_DBG_HKEY, > > + "firmware HKEY interface version: > > 0x%x\n", > > + hkeyv); > > + > > + /* Paranoia check AND init hotkey_all_mask > > */ > > + if (!acpi_evalf(hkey_handle, > > &hotkey_all_mask, > > + "MHKA", "dd", 1)) { > > + pr_err("missing MHKA handler, " > > + "please report this to > > %s\n", > > + TPACPI_MAIL); > > + /* Fallback: pre-init for > > FN+F3,F4,F12 */ > > + hotkey_all_mask = 0x080cU; > > + } else { > > + tp_features.hotkey_mask = 1; > > + } > > + break; > > Why do you duplicate this code block with that single byte changed? > If > that was intended, then it certainly should be explained. I didn't, I re-sent one of Shuduo's patches. In any case, it seems that the patch breaks a few devices that appeared after the Lenovo Carbon X1 2014. This is discussed, in a few separate threads, on the ibm-acpi list: http://news.gmane.org/gmane.linux.acpi.ibm-acpi.devel Cheers -- To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Apr 19, 2016 at 02:48:25PM +0200, Bastien Nocera wrote: > On Tue, 2016-04-19 at 14:42 +0200, Bjørn Mork wrote: > > Bastien Nocera <hadess@hadess.net> writes: > > > > > > > > From: Shuduo Sang <shuduo.sang@canonical.com> > > > > > > The Thinkpad X1 Carbon 2nd generation (2014) ships with BIOS that > > > will > > > return HKEY interface version 0x200. It needs thinkpad-acpi support > > > otherwise it will be routed to wrong branch and the hotkey mask > > > will > > > be wrong. > > > > > > https://bugzilla.kernel.org/show_bug.cgi?id=114731 > > > > > > Signed-off-by: Bruce Ma <bruce.ma@canonical.com> > > > Signed-off-by: Shuduo Sang <shuduo.sang@canonical.com> > > > --- > > > drivers/platform/x86/thinkpad_acpi.c | 35 > > > ++++++++++++++++++++++++++++++----- > > > 1 file changed, 30 insertions(+), 5 deletions(-) > > > > > > diff --git a/drivers/platform/x86/thinkpad_acpi.c > > > b/drivers/platform/x86/thinkpad_acpi.c > > > index dad2984..c177936 100644 > > > --- a/drivers/platform/x86/thinkpad_acpi.c > > > +++ b/drivers/platform/x86/thinkpad_acpi.c > > > @@ -3357,11 +3357,8 @@ static int __init hotkey_init(struct > > > ibm_init_struct *iibm) > > > A30, R30, R31, T20-22, X20-21, X22-24. Detected by > > > checking > > > for HKEY interface version 0x100 */ > > > if (acpi_evalf(hkey_handle, &hkeyv, "MHKV", "qd")) { > > > - if ((hkeyv >> 8) != 1) { > > > - pr_err("unknown version of the HKEY > > > interface: 0x%x\n", > > > - hkeyv); > > > - pr_err("please report this to %s\n", > > > TPACPI_MAIL); > > > - } else { > > > + switch (hkeyv >> 8) { > > > + case 1: > > > /* > > > * MHKV 0x100 in A31, R40, R40e, > > > * T4x, X31, and later > > > @@ -3381,6 +3378,34 @@ static int __init hotkey_init(struct > > > ibm_init_struct *iibm) > > > } else { > > > tp_features.hotkey_mask = 1; > > > } > > > + break; > > > + > > > + case 2: > > > + /* > > > + * MHKV 0x200 in X1 > > > + */ > > > + vdbg_printk(TPACPI_DBG_INIT | > > > TPACPI_DBG_HKEY, > > > + "firmware HKEY interface version: > > > 0x%x\n", > > > + hkeyv); > > > + > > > + /* Paranoia check AND init hotkey_all_mask > > > */ > > > + if (!acpi_evalf(hkey_handle, > > > &hotkey_all_mask, > > > + "MHKA", "dd", 1)) { > > > + pr_err("missing MHKA handler, " > > > + "please report this to > > > %s\n", > > > + TPACPI_MAIL); > > > + /* Fallback: pre-init for > > > FN+F3,F4,F12 */ > > > + hotkey_all_mask = 0x080cU; > > > + } else { > > > + tp_features.hotkey_mask = 1; > > > + } > > > + break; > > > > Why do you duplicate this code block with that single byte changed? > > If > > that was intended, then it certainly should be explained. It is not entirely duplicated. The calls to acpi_evalf are different. Would it be a good idea to move the call to vdbg_printk in both the switch cases to before the switch case to reduce the code duplication? Maybe also update the comment in case 2 to suggest that it's the MHKV for multiple newer models? > I didn't, I re-sent one of Shuduo's patches. > > In any case, it seems that the patch breaks a few devices that appeared > after the Lenovo Carbon X1 2014. This is discussed, in a few separate > threads, on the ibm-acpi list: > http://news.gmane.org/gmane.linux.acpi.ibm-acpi.devel This patch breaks devices? I failed to find any mention of that. Do you mean that it fixes some broken devices ..? -- To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c index dad2984..c177936 100644 --- a/drivers/platform/x86/thinkpad_acpi.c +++ b/drivers/platform/x86/thinkpad_acpi.c @@ -3357,11 +3357,8 @@ static int __init hotkey_init(struct ibm_init_struct *iibm) A30, R30, R31, T20-22, X20-21, X22-24. Detected by checking for HKEY interface version 0x100 */ if (acpi_evalf(hkey_handle, &hkeyv, "MHKV", "qd")) { - if ((hkeyv >> 8) != 1) { - pr_err("unknown version of the HKEY interface: 0x%x\n", - hkeyv); - pr_err("please report this to %s\n", TPACPI_MAIL); - } else { + switch (hkeyv >> 8) { + case 1: /* * MHKV 0x100 in A31, R40, R40e, * T4x, X31, and later @@ -3381,6 +3378,34 @@ static int __init hotkey_init(struct ibm_init_struct *iibm) } else { tp_features.hotkey_mask = 1; } + break; + + case 2: + /* + * MHKV 0x200 in X1 + */ + vdbg_printk(TPACPI_DBG_INIT | TPACPI_DBG_HKEY, + "firmware HKEY interface version: 0x%x\n", + hkeyv); + + /* Paranoia check AND init hotkey_all_mask */ + if (!acpi_evalf(hkey_handle, &hotkey_all_mask, + "MHKA", "dd", 1)) { + pr_err("missing MHKA handler, " + "please report this to %s\n", + TPACPI_MAIL); + /* Fallback: pre-init for FN+F3,F4,F12 */ + hotkey_all_mask = 0x080cU; + } else { + tp_features.hotkey_mask = 1; + } + break; + + default: + pr_err("unknown version of the HKEY interface: 0x%x\n", + hkeyv); + pr_err("please report this to %s\n", TPACPI_MAIL); + break; } }