| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <1465318938.7166.23.camel@redhat.com>
Date: Tue, 07 Jun 2016 13:02:18 -0400
From: Lyude Paul <cpaul@...hat.com>
To: "ibm-acpi-devel@...ts.sourceforge.net"
<ibm-acpi-devel@...ts.sourceforge.net>,
Dennis Wassenberg <dennis.wassenberg@...unet.com>
Cc: Henrique de Moraes Holschuh <ibm-acpi@....eng.br>,
Darren Hart <dvhart@...radead.org>,
platform-driver-x86@...r.kernel.org,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] thinkpad_acpi: Add support for HKEY version 0x200
Since nothing's really happened with this patch for a while I figured I'd take
over trying to get this upstream.
Regarding testing: This seems to work fine on the 60 series laptops, and works
fine on previous generations. The one thing I haven't been able to test is an X1
carbon with an adaptive keyboard since I don't seem to have one readily
available here. I'm doing a search around the office to try to find someone who
didn't throw theirs away yet so hopefully I should be able to get back to you on
that soon.
To Dennis: I took the liberty of doing a review of your patch and some testing.
There's a few things that need changing, I've outlined them below: (for the
future, it's recommended to send patches for the kernel inline in emails to make
them easier to review).
> From 8a67f5db1d2918c46b7fa2168e3d0aab2ba92731 Mon Sep 17 00:00:00 2001
> From: Dennis Wassenberg <dennis.wassenberg@...unet.com>
> Date: Wed, 13 Apr 2016 13:46:55 +0200
> Subject: [PATCH] thinkpad_acpi: Add support for HKEY version 0x200
>
> Lenovo Thinkpad devices T460, T460s, T460p, T560, X260 use
> HKEY version 0x200 without adaptive keyboard.
>
> HKEY version 0x200 has method MHKA with one parameter value.
> Passing parameter value 1 will get hotkey_all_mask (the same like
> HKEY version 0x100 without parameter). Passing parameter value 2 to
> MHKA method will retrieve hotkey_all_adaptive_mask. If 0 is returned in
> that case there is no adaptive keyboard available.
>
> Signed-off-by: Dennis Wassenberg <dennis.wassenberg@...unet.com>
> ---
> drivers/platform/x86/thinkpad_acpi.c | 88 ++++++++++++++++++++++++++---------
> -
> 1 file changed, 64 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/platform/x86/thinkpad_acpi.c
> b/drivers/platform/x86/thinkpad_acpi.c
> index a268a7a..0e72857 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -2043,6 +2043,7 @@ static int hotkey_autosleep_ack;
>
> static u32 hotkey_orig_mask; /* events the BIOS had enabled */
> static u32 hotkey_all_mask; /* all events supported in fw */
> +static u32 hotkey_adaptive_all_mask; /* all adaptive events supported
> in fw */
> static u32 hotkey_reserved_mask; /* events better left disabled */
> static u32 hotkey_driver_mask; /* events needed by the driver
> */
> static u32 hotkey_user_mask; /* events visible to userspace */
> @@ -2742,6 +2743,17 @@ static ssize_t hotkey_all_mask_show(struct device *dev,
>
> static DEVICE_ATTR_RO(hotkey_all_mask);
>
> +/* sysfs hotkey all_mask ----------------------------------------------- */
> +static ssize_t hotkey_adaptive_all_mask_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + return snprintf(buf, PAGE_SIZE, "0x%08x\n",
> + hotkey_adaptive_all_mask | hotkey_source_mask);
Make sure when you're indent function calls that split like this onto another
line you indent it like this:
return snprintf(buf, PAGE_SIZE, "0x%08x\n",
hotkey_adaptive_all_mask | hotkey_source_mask);
So that "hotkey_adaptive_all_mask" starts on the column after the starting
paranthesis.
> +}
> +
> +static DEVICE_ATTR_RO(hotkey_adaptive_all_mask);
> +
> /* sysfs hotkey recommended_mask --------------------------------------- */
> static ssize_t hotkey_recommended_mask_show(struct device *dev,
> struct device_attribute *attr,
> @@ -2985,6 +2997,7 @@ static struct attribute *hotkey_attributes[] __initdata
> = {
> &dev_attr_wakeup_hotunplug_complete.attr,
> &dev_attr_hotkey_mask.attr,
> &dev_attr_hotkey_all_mask.attr,
> + &dev_attr_hotkey_adaptive_all_mask.attr,
> &dev_attr_hotkey_recommended_mask.attr,
> #ifdef CONFIG_THINKPAD_ACPI_HOTKEY_POLL
> &dev_attr_hotkey_source_mask.attr,
> @@ -3321,20 +3334,6 @@ static int __init hotkey_init(struct ibm_init_struct
> *iibm)
> if (!tp_features.hotkey)
> return 1;
>
> - /*
> - * Check if we have an adaptive keyboard, like on the
> - * Lenovo Carbon X1 2014 (2nd Gen).
> - */
> - if (acpi_evalf(hkey_handle, &hkeyv, "MHKV", "qd")) {
> - if ((hkeyv >> 8) == 2) {
> - tp_features.has_adaptive_kbd = true;
> - res = sysfs_create_group(&tpacpi_pdev->dev.kobj,
> - &adaptive_kbd_attr_group);
> - if (res)
> - goto err_exit;
> - }
> - }
> -
> quirks = tpacpi_check_quirks(tpacpi_hotkey_qtable,
> ARRAY_SIZE(tpacpi_hotkey_qtable));
>
> @@ -3357,30 +3356,71 @@ 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 {
> + vdbg_printk(TPACPI_DBG_INIT | TPACPI_DBG_HKEY,
> + "firmware HKEY interface version: 0x%x\n",
> + hkeyv);
The indenting here wasn't correct to begin with, but since we're changing it we
might as well fix it.
> + switch (hkeyv >> 8) {
> + case 1:
> /*
> * MHKV 0x100 in A31, R40, R40e,
> * T4x, X31, and later
> */
> - 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", "qd")) {
> pr_err("missing MHKA handler, "
> - "please report this to %s\n",
> - TPACPI_MAIL);
> + "please report this to %s\n",
> + TPACPI_MAIL);
The indenting here doesn't need to be changed
> + /* Fallback: pre-init for FN+F3,F4,F12 */
> + hotkey_all_mask = 0x080cU;
> + } else {
> + tp_features.hotkey_mask = 1;
> + }
> + break;
> +
> + case 2:
> + /*
> + * MHKV 0x200 in X1, T460s, X260, T560, X1 Tablet (2016)
> + */
> +
> + /* 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);
This indenting needs to be fixed as well
Once all those fixes are made and I get that extra testing done we should be
ablew to send it upstream, assuming it doesn't break the X1…
Cheers,
Lyude
> /* Fallback: pre-init for FN+F3,F4,F12 */
> hotkey_all_mask = 0x080cU;
> } else {
> tp_features.hotkey_mask = 1;
> }
> +
> + /*
> + * Check if we have an adaptive keyboard, like on the
> + * Lenovo Carbon X1 2014 (2nd Gen).
> + */
> + if (acpi_evalf(hkey_handle,&hotkey_adaptive_all_mask,
> + "MHKA", "dd", 2)) {
Indentation needs to be fixed here as well
> + if (hotkey_adaptive_all_mask != 0) {
> + tp_features.has_adaptive_kbd = true;
> + res = sysfs_create_group(
> + &tpacpi_pdev->dev.kobj,
> + &adaptive_kbd_attr_group);
> + if (res)
> + goto err_exit;
> + }
> + } else {
> + tp_features.has_adaptive_kbd = false;
> + hotkey_adaptive_all_mask = 0x0U;
> + }
> + 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;
> }
> }
>
> --
> 2.1.4
Powered by blists - more mailing lists