[PATCH] thinkpad_acpi: support HKEY interface version 0x200

[PATCH] thinkpad_acpi: support HKEY interface version 0x200

[Bastien Nocera]

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;
+
+		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.7.3

Re: [PATCH] thinkpad_acpi: support HKEY interface version 0x200

[Bjørn Mork]

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

Re: [PATCH] thinkpad_acpi: support HKEY interface version 0x200

[Bastien Nocera]

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

------------------------------------------------------------------------------
Find and fix application performance issues faster with Applications Manager
Applications Manager provides deep performance insights into multiple tiers of
your business applications. It resolves application problems quickly and
reduces your MTTR. Get your free trial!
https://ad.doubleclick.net/ddm/clk/302982198;130105516;z
_______________________________________________
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel

Re: [PATCH] thinkpad_acpi: support HKEY interface version 0x200

[Yrjan Skrimstad]

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 ..?

------------------------------------------------------------------------------
Find and fix application performance issues faster with Applications Manager
Applications Manager provides deep performance insights into multiple tiers of
your business applications. It resolves application problems quickly and
reduces your MTTR. Get your free trial!
https://ad.doubleclick.net/ddm/clk/302982198;130105516;z
_______________________________________________
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel