From: Lyude Paul <cpaul@redhat.com>
To: "ibm-acpi-devel@lists.sourceforge.net"
<ibm-acpi-devel@lists.sourceforge.net>,
Dennis Wassenberg <dennis.wassenberg@secunet.com>
Cc: Darren Hart <dvhart@infradead.org>,
Henrique de Moraes Holschuh <ibm-acpi@hmh.eng.br>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
platform-driver-x86@vger.kernel.org
Subject: Re: [ibm-acpi-devel] [PATCH] thinkpad_acpi: Add support for HKEY version 0x200
Date: Tue, 07 Jun 2016 17:10:51 -0400 [thread overview]
Message-ID: <1465333851.7166.46.camel@redhat.com> (raw)
In-Reply-To: <1465318938.7166.23.camel@redhat.com>
Managed to find someone in the office with one of these laptops and it looks
like the adaptive keyboard works perfectly with this patch, so I can give my t-
b:
Tested-by: Lyude Paul <cpaul@redhat.com>
Cheers,
Lyude
On Tue, 2016-06-07 at 13:02 -0400, Lyude Paul wrote:
> 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@secunet.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@secunet.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
> ------------------------------------------------------------------------------
> What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
> patterns at an interface-level. Reveals which users, apps, and protocols are
> consuming the most bandwidth. Provides multi-vendor support for NetFlow,
> J-Flow, sFlow and other flows. Make informed decisions using capacity
> planning reports. https://ad.doubleclick.net/ddm/clk/305295220;132659582;e
> _______________________________________________
> ibm-acpi-devel mailing list
> ibm-acpi-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel
next prev parent reply other threads:[~2016-06-07 21:10 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-06-07 17:02 [PATCH] thinkpad_acpi: Add support for HKEY version 0x200 Lyude Paul
2016-06-07 21:10 ` Lyude Paul [this message]
2016-06-07 21:43 ` [ibm-acpi-devel] " Darren Hart
2016-06-07 22:06 ` Marco Trevisan (Treviño)
2016-06-07 23:53 ` Henrique de Moraes Holschuh
2016-06-08 14:54 ` [PATCH v2] " Lyude
2016-06-08 19:30 ` Darren Hart
2016-06-09 5:05 ` [ibm-acpi-devel] " Marco Trevisan (Treviño)
2016-06-09 19:57 ` Darren Hart
2016-06-09 21:04 ` Henrique de Moraes Holschuh
2016-06-09 21:18 ` Darren Hart
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1465333851.7166.46.camel@redhat.com \
--to=cpaul@redhat.com \
--cc=dennis.wassenberg@secunet.com \
--cc=dvhart@infradead.org \
--cc=ibm-acpi-devel@lists.sourceforge.net \
--cc=ibm-acpi@hmh.eng.br \
--cc=linux-kernel@vger.kernel.org \
--cc=platform-driver-x86@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).