From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756192AbcKETPr (ORCPT ); Sat, 5 Nov 2016 15:15:47 -0400 Received: from bombadil.infradead.org ([198.137.202.9]:49015 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755084AbcKETPp (ORCPT ); Sat, 5 Nov 2016 15:15:45 -0400 Date: Sat, 5 Nov 2016 12:17:17 -0700 From: Darren Hart To: Lyude Cc: ibm-acpi-devel@lists.sourceforge.net, Daniel Martin , Henrique de Moraes Holschuh , platform-driver-x86@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 1/3] thinkpad_acpi: Move tablet detection into separate function Message-ID: <20161105191717.GF100528@f23x64.localdomain> References: <1477954535-12536-1-git-send-email-lyude@redhat.com> <1477954535-12536-2-git-send-email-lyude@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1477954535-12536-2-git-send-email-lyude@redhat.com> User-Agent: Mutt/1.7.1 (2016-10-04) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Oct 31, 2016 at 06:55:32PM -0400, Lyude wrote: > Suggested by Daniel Martin 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 > Cc: Daniel Martin > --- > 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