Re: [ibm-acpi-devel] [PATCH v2 1/3] thinkpad_acpi: Move tablet detection into separate function
On Sat, 05 Nov 2016, Darren Hart wrote: > On Mon, Oct 31, 2016 at 07:56:40PM -0400, Lyude wrote: > > Suggested by Daniel Martin > > > > 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. > > > > Changes since v1: > > - Don't use bool for in_tablet_mode (fixes complaints from kbuild test > > robot) > > > > This series doesn't apply cleanly now (simple fuzz). > > Once we hear back from Henrique on his enum preference and thoughts on the > refactoring (which looks reasonable to me), please resubmit this series and I don't mind the enum usage, as long as it is correct. As you said, the driver is not consistent there. I also don't mind the refactoring. -- Henrique Holschuh -- Developer Access Program for Intel Xeon Phi Processors Access to Intel Xeon Phi processor-based developer platforms. With one year of Intel Parallel Studio XE. Training and support from Colfax. Order your platform today. http://sdm.link/xeonphi ___ ibm-acpi-devel mailing list ibm-acpi-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel
Re: [ibm-acpi-devel] [PATCH v2 1/3] thinkpad_acpi: Move tablet detection into separate function
On Mon, Oct 31, 2016 at 07:56:40PM -0400, Lyude wrote: > Suggested by Daniel Martin > > 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. > > Changes since v1: > - Don't use bool for in_tablet_mode (fixes complaints from kbuild test > robot) > This series doesn't apply cleanly now (simple fuzz). Once we hear back from Henrique on his enum preference and thoughts on the refactoring (which looks reasonable to me), please resubmit this series and review Documentation/SubmittingPatches for the changelog (below ---) and please be consistent in your placement of "v2" in the subject [PATCH vX N/M] prefix. Most all of my feedback here is minor, but there were enough little things that added up and I'd like to see this resubmitted as a series with those addressed that applies cleanly - largely to make sure I haven't missed context and somehow merged the wrong bits. Thanks Lyude, -- Darren Hart Intel Open Source Technology Center -- Developer Access Program for Intel Xeon Phi Processors Access to Intel Xeon Phi processor-based developer platforms. With one year of Intel Parallel Studio XE. Training and support from Colfax. Order your platform today. http://sdm.link/xeonphi ___ ibm-acpi-devel mailing list ibm-acpi-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel
[ibm-acpi-devel] [PATCH v2 1/3] thinkpad_acpi: Move tablet detection into separate function
Suggested by Daniel Martin
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.
Changes since v1:
- Don't use bool for in_tablet_mode (fixes complaints from kbuild test
robot)
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..5209a1d 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;
+ int 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
--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel
