Re: [PATCH] thinkpad_acpi: Add support for HKEY version 0x200

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>

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

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




[Index of Archives]     [Linux ACPI]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Photo]     [Yosemite Photos]     [Yosemite Advice]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]

  Powered by Linux