Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1512978 > unrolled thread

[PATCH 1/3] thinkpad_acpi: Move tablet detection into separate function

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 1/3] thinkpad_acpi: Move tablet detection into separate function Lyude <lyude@redhat.com> - 2016-11-01 00:00 +0100
    Re: [PATCH 1/3] thinkpad_acpi: Move tablet detection into separate  function Darren Hart <dvhart@infradead.org> - 2016-11-05 20:20 +0100

#1512978 — [PATCH 1/3] thinkpad_acpi: Move tablet detection into separate function

Suggested by Daniel Martin <consume.noise@gmail.com>

Lenovo's having a bit of fun randomly changing what hotkey events and
acpi handles they use for reporting tablet mode, so unfortunately this
means we're definitely going to need to probe for multiple types of
tablet mode support. Since the hotkey_init() is already a lot larger
then it should be, let's split up this detection into it's own function
to make things a little easier to read.

As well, since we're going to have multiple types of tablet modes, make
hotkey_tablet into an enum so we can also use it to indicate the type of
tablet mode reporting the machine supports.

Signed-off-by: Lyude <lyude@redhat.com>
Cc: Daniel Martin <consume.noise@gmail.com>
---
 drivers/platform/x86/thinkpad_acpi.c | 50 +++++++++++++++++++++++++++---------
 1 file changed, 38 insertions(+), 12 deletions(-)

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index b65ce75..369b483 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -302,7 +302,10 @@ static struct {
 	u32 hotkey:1;
 	u32 hotkey_mask:1;
 	u32 hotkey_wlsw:1;
-	u32 hotkey_tablet:1;
+	enum {
+		TP_HOTKEY_TABLET_NONE = 0,
+		TP_HOTKEY_TABLET_USES_MHKG = 1,
+	} hotkey_tablet;
 	u32 kbdlight:1;
 	u32 light:1;
 	u32 light_status:1;
@@ -3117,6 +3120,34 @@ static const struct tpacpi_quirk tpacpi_hotkey_qtable[] __initconst = {
 typedef u16 tpacpi_keymap_entry_t;
 typedef tpacpi_keymap_entry_t tpacpi_keymap_t[TPACPI_HOTKEY_MAP_LEN];
 
+static int
+hotkey_init_tablet_mode(void)
+{
+	int res;
+	char *type;
+	bool in_tablet_mode;
+
+	/* For X41t, X60t, X61t Tablets... */
+	if (acpi_evalf(hkey_handle, &res, "MHKG", "qd")) {
+		tp_features.hotkey_tablet = TP_HOTKEY_TABLET_USES_MHKG;
+		in_tablet_mode = !!(res & TP_HOTKEY_TABLET_MASK);
+		type = "MHKG";
+	}
+
+	if (!tp_features.hotkey_tablet)
+		return 0;
+
+	pr_info("Tablet mode switch found (type: %s), currently in %s mode\n",
+		type, in_tablet_mode ? "tablet" : "laptop");
+
+	res = add_to_attr_set(hotkey_dev_attributes,
+			      &dev_attr_hotkey_tablet_mode.attr);
+	if (res)
+		return -1;
+
+	return in_tablet_mode;
+}
+
 static int __init hotkey_init(struct ibm_init_struct *iibm)
 {
 	/* Requirements for changing the default keymaps:
@@ -3464,17 +3495,6 @@ static int __init hotkey_init(struct ibm_init_struct *iibm)
 		res = add_to_attr_set(hotkey_dev_attributes,
 				&dev_attr_hotkey_radio_sw.attr);
 
-	/* For X41t, X60t, X61t Tablets... */
-	if (!res && acpi_evalf(hkey_handle, &status, "MHKG", "qd")) {
-		tp_features.hotkey_tablet = 1;
-		tabletsw_state = !!(status & TP_HOTKEY_TABLET_MASK);
-		pr_info("possible tablet mode switch found; "
-			"ThinkPad in %s mode\n",
-			(tabletsw_state) ? "tablet" : "laptop");
-		res = add_to_attr_set(hotkey_dev_attributes,
-				&dev_attr_hotkey_tablet_mode.attr);
-	}
-
 	if (!res)
 		res = register_attr_set_with_sysfs(
 				hotkey_dev_attributes,
@@ -3482,6 +3502,12 @@ static int __init hotkey_init(struct ibm_init_struct *iibm)
 	if (res)
 		goto err_exit;
 
+	res = hotkey_init_tablet_mode();
+	if (res < 0)
+		goto err_exit;
+
+	tabletsw_state = res;
+
 	/* Set up key map */
 	hotkey_keycode_map = kmalloc(TPACPI_HOTKEY_MAP_SIZE,
 					GFP_KERNEL);
-- 
2.7.4

[toc] | [next] | [standalone]

#1515610 — Re: [PATCH 1/3] thinkpad_acpi: Move tablet detection into separate function

On Mon, Oct 31, 2016 at 06:55:32PM -0400, Lyude wrote:
> Suggested by Daniel Martin <consume.noise@gmail.com>

Down below with the other -by: tags please.

> 
> Lenovo's having a bit of fun randomly changing what hotkey events and

Please avoid contractions in documentation in general.

> acpi handles they use for reporting tablet mode, so unfortunately this
> means we're definitely going to need to probe for multiple types of
> tablet mode support. Since the hotkey_init() is already a lot larger
> then it should be, let's split up this detection into it's own function

s/then/than/ (nit, but since I'm commenting anyway...)
s/it's/its/

> to make things a little easier to read.
> 
> As well, since we're going to have multiple types of tablet modes, make
> hotkey_tablet into an enum so we can also use it to indicate the type of
> tablet mode reporting the machine supports.
> 
> Signed-off-by: Lyude <lyude@redhat.com>
> Cc: Daniel Martin <consume.noise@gmail.com>
> ---
>  drivers/platform/x86/thinkpad_acpi.c | 50 +++++++++++++++++++++++++++---------
>  1 file changed, 38 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> index b65ce75..369b483 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -302,7 +302,10 @@ static struct {
>  	u32 hotkey:1;
>  	u32 hotkey_mask:1;
>  	u32 hotkey_wlsw:1;
> -	u32 hotkey_tablet:1;
> +	enum {
> +		TP_HOTKEY_TABLET_NONE = 0,
> +		TP_HOTKEY_TABLET_USES_MHKG = 1,

1 will follow automatically, no need to specify explicitly.

enum usage in this driver is not very consistent, but I don't particularly care
to have it defined within the struct, but I'll leave that for Henrique to
decide.

> +	} hotkey_tablet;
>  	u32 kbdlight:1;
>  	u32 light:1;
>  	u32 light_status:1;
> @@ -3117,6 +3120,34 @@ static const struct tpacpi_quirk tpacpi_hotkey_qtable[] __initconst = {
>  typedef u16 tpacpi_keymap_entry_t;
>  typedef tpacpi_keymap_entry_t tpacpi_keymap_t[TPACPI_HOTKEY_MAP_LEN];
>  
> +static int
> +hotkey_init_tablet_mode(void)
> +{
> +	int res;
> +	char *type;
> +	bool in_tablet_mode;

Declare variables in reverse line length order please:

	bool in_tablet_mode;
	char *type;
	int res;

> +
> +	/* For X41t, X60t, X61t Tablets... */
> +	if (acpi_evalf(hkey_handle, &res, "MHKG", "qd")) {
> +		tp_features.hotkey_tablet = TP_HOTKEY_TABLET_USES_MHKG;
> +		in_tablet_mode = !!(res & TP_HOTKEY_TABLET_MASK);
> +		type = "MHKG";
> +	}
> +
> +	if (!tp_features.hotkey_tablet)
> +		return 0;
> +
> +	pr_info("Tablet mode switch found (type: %s), currently in %s mode\n",
> +		type, in_tablet_mode ? "tablet" : "laptop");
> +
> +	res = add_to_attr_set(hotkey_dev_attributes,
> +			      &dev_attr_hotkey_tablet_mode.attr);
> +	if (res)
> +		return -1;
> +
> +	return in_tablet_mode;
> +}
> +
>  static int __init hotkey_init(struct ibm_init_struct *iibm)
>  {
>  	/* Requirements for changing the default keymaps:
> @@ -3464,17 +3495,6 @@ static int __init hotkey_init(struct ibm_init_struct *iibm)
>  		res = add_to_attr_set(hotkey_dev_attributes,
>  				&dev_attr_hotkey_radio_sw.attr);
>  
> -	/* For X41t, X60t, X61t Tablets... */
> -	if (!res && acpi_evalf(hkey_handle, &status, "MHKG", "qd")) {
> -		tp_features.hotkey_tablet = 1;
> -		tabletsw_state = !!(status & TP_HOTKEY_TABLET_MASK);
> -		pr_info("possible tablet mode switch found; "
> -			"ThinkPad in %s mode\n",
> -			(tabletsw_state) ? "tablet" : "laptop");
> -		res = add_to_attr_set(hotkey_dev_attributes,
> -				&dev_attr_hotkey_tablet_mode.attr);
> -	}
> -
>  	if (!res)
>  		res = register_attr_set_with_sysfs(
>  				hotkey_dev_attributes,
> @@ -3482,6 +3502,12 @@ static int __init hotkey_init(struct ibm_init_struct *iibm)
>  	if (res)
>  		goto err_exit;
>  
> +	res = hotkey_init_tablet_mode();
> +	if (res < 0)
> +		goto err_exit;
> +
> +	tabletsw_state = res;
> +
>  	/* Set up key map */
>  	hotkey_keycode_map = kmalloc(TPACPI_HOTKEY_MAP_SIZE,
>  					GFP_KERNEL);
> -- 
> 2.7.4
> 
> 

-- 
Darren Hart
Intel Open Source Technology Center

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web