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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Tue, 07 Jun 2016 17:10:51 -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:	Darren Hart <dvhart@...radead.org>,
	Henrique de Moraes Holschuh <ibm-acpi@....eng.br>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	platform-driver-x86@...r.kernel.org
Subject: Re: [ibm-acpi-devel] [PATCH] thinkpad_acpi: Add support for HKEY
 version 0x200

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@...hat.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@...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
> ------------------------------------------------------------------------------
> 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@...ts.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel

Powered by blists - more mailing lists