2015-02-20 14:45:42

by Bastien Nocera

[permalink] [raw]
Subject: [PATCH 1/7] thinkpad_acpi: Remember adaptive kbd presence

Rather than checking on each suspend and resume whether the laptop
has an adaptive keyboard, check when the driver is initialised.

Signed-off-by: Bastien Nocera <[email protected]>
---
drivers/platform/x86/thinkpad_acpi.c | 37 ++++++++++++++++++------------------
1 file changed, 18 insertions(+), 19 deletions(-)

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index c3d11fa..c1f4daf 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -318,6 +318,7 @@ static struct {
u32 sensors_pdrv_attrs_registered:1;
u32 sensors_pdev_attrs_registered:1;
u32 hotkey_poll_active:1;
+ u32 has_adaptive_kbd:1;
} tp_features;

static struct {
@@ -3226,6 +3227,15 @@ 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;
+ }
+
quirks = tpacpi_check_quirks(tpacpi_hotkey_qtable,
ARRAY_SIZE(tpacpi_hotkey_qtable));

@@ -3834,28 +3844,21 @@ static void hotkey_notify(struct ibm_struct *ibm, u32 event)

static void hotkey_suspend(void)
{
- int hkeyv;
-
/* Do these on suspend, we get the events on early resume! */
hotkey_wakeup_reason = TP_ACPI_WAKEUP_NONE;
hotkey_autosleep_ack = 0;

/* save previous mode of adaptive keyboard of X1 Carbon */
- if (acpi_evalf(hkey_handle, &hkeyv, "MHKV", "qd")) {
- if ((hkeyv >> 8) == 2) {
- if (!acpi_evalf(hkey_handle,
- &adaptive_keyboard_prev_mode,
- "GTRW", "dd", 0)) {
- pr_err("Cannot read adaptive keyboard mode.\n");
- }
+ if (tp_features.has_adaptive_kbd) {
+ if (!acpi_evalf(hkey_handle, &adaptive_keyboard_prev_mode,
+ "GTRW", "dd", 0)) {
+ pr_err("Cannot read adaptive keyboard mode.\n");
}
}
}

static void hotkey_resume(void)
{
- int hkeyv;
-
tpacpi_disable_brightness_delay();

if (hotkey_status_set(true) < 0 ||
@@ -3870,14 +3873,10 @@ static void hotkey_resume(void)
hotkey_poll_setup_safe(false);

/* restore previous mode of adapive keyboard of X1 Carbon */
- if (acpi_evalf(hkey_handle, &hkeyv, "MHKV", "qd")) {
- if ((hkeyv >> 8) == 2) {
- if (!acpi_evalf(hkey_handle,
- NULL,
- "STRW", "vd",
- adaptive_keyboard_prev_mode)) {
- pr_err("Cannot set adaptive keyboard mode.\n");
- }
+ if (tp_features.has_adaptive_kbd) {
+ if (!acpi_evalf(hkey_handle, NULL, "STRW", "vd",
+ adaptive_keyboard_prev_mode)) {
+ pr_err("Cannot set adaptive keyboard mode.\n");
}
}
}
--
2.1.0


2015-02-26 06:18:23

by Darren Hart

[permalink] [raw]
Subject: Re: [PATCH 1/7] thinkpad_acpi: Remember adaptive kbd presence

On Fri, Feb 20, 2015 at 03:44:10PM +0100, Bastien Nocera wrote:
> Rather than checking on each suspend and resume whether the laptop
> has an adaptive keyboard, check when the driver is initialised.

Bastien, am I awaiting another version of this from you to address comments from
Henrique?

Henrique, when you're satisfied, please provide a Reviewed-by for the series.

Thanks,

--
Darren Hart
Intel Open Source Technology Center

2015-02-26 11:22:49

by Bastien Nocera

[permalink] [raw]
Subject: Re: [PATCH 1/7] thinkpad_acpi: Remember adaptive kbd presence

On Wed, 2015-02-25 at 22:18 -0800, Darren Hart wrote:
> On Fri, Feb 20, 2015 at 03:44:10PM +0100, Bastien Nocera wrote:
> > Rather than checking on each suspend and resume whether the laptop
> > has an adaptive keyboard, check when the driver is initialised.
>
> Bastien, am I awaiting another version of this from you to address comments from
> Henrique?

I was waiting for more comments on the additional keycodes from Dmitry
and Jiri.

> Henrique, when you're satisfied, please provide a Reviewed-by for the series.

I can send the first 5 patches of that series if you want to land it
soon.

Subject: Re: [ibm-acpi-devel] [PATCH 1/7] thinkpad_acpi: Remember adaptive kbd presence

On Thu, Feb 26, 2015, at 03:18, Darren Hart wrote:
> On Fri, Feb 20, 2015 at 03:44:10PM +0100, Bastien Nocera wrote:
> > Rather than checking on each suspend and resume whether the laptop
> > has an adaptive keyboard, check when the driver is initialised.
>
> Bastien, am I awaiting another version of this from you to address
> comments from
> Henrique?
>
> Henrique, when you're satisfied, please provide a Reviewed-by for the
> series.

I usually provide a signed-off-by, as I am the thinkpad-acpi driver
maintainer... reviewed-by is implied in that case.

Let's wait a little bit to see if we get a reply on the keycodes. That
said, Bastien, feel free to send the next version at any time. I won't
object to the initial keymap being full of KEY_RESERVED/KEY_UNKNOWN and
receiving incremental patches later. It might even help merging, as you
could merge the new keycodes in a smaller patchset through the input
tree, later.

But I really want to see a patch added to the series that updates the
driver documentation before the series gets merged. It should document
any new sysfs entries, and also the relevant HKEY events related to the
adaptative keyboard, as well as the new keymap.

--
"One disk to rule them all, One disk to find them. One disk to bring
them all and in the darkness grind them. In the Land of Redmond
where the shadows lie." -- The Silicon Valley Tarot
Henrique Holschuh

Subject: Re: [PATCH 1/7] thinkpad_acpi: Remember adaptive kbd presence

On Thu, Feb 26, 2015, at 08:22, Bastien Nocera wrote:
> On Wed, 2015-02-25 at 22:18 -0800, Darren Hart wrote:
> > On Fri, Feb 20, 2015 at 03:44:10PM +0100, Bastien Nocera wrote:
> > > Rather than checking on each suspend and resume whether the laptop
> > > has an adaptive keyboard, check when the driver is initialised.
> >
> > Bastien, am I awaiting another version of this from you to address comments from
> > Henrique?
>
> I was waiting for more comments on the additional keycodes from Dmitry
> and Jiri.
>
> > Henrique, when you're satisfied, please provide a Reviewed-by for the series.
>
> I can send the first 5 patches of that series if you want to land it
> soon.

Please write the missing patch with the documentation update, and add it
to the next version of the series...

We could then merge this series without any new keycodes ASAP through
the platform-x86 tree, and add new keycodes later through either the
input tree or the platform-x86 tree, whichever Dmitry and Darren feel
best.

--
"One disk to rule them all, One disk to find them. One disk to bring
them all and in the darkness grind them. In the Land of Redmond
where the shadows lie." -- The Silicon Valley Tarot
Henrique Holschuh

Subject: Re: [ibm-acpi-devel] [PATCH 1/7] thinkpad_acpi: Remember adaptive kbd presence

On Fri, Feb 27, 2015, at 08:05, Henrique de Moraes Holschuh wrote:
> On Thu, Feb 26, 2015, at 03:18, Darren Hart wrote:
> > On Fri, Feb 20, 2015 at 03:44:10PM +0100, Bastien Nocera wrote:
> > > Rather than checking on each suspend and resume whether the laptop
> > > has an adaptive keyboard, check when the driver is initialised.
> >
> > Bastien, am I awaiting another version of this from you to address
> > comments from
> > Henrique?
> >
> > Henrique, when you're satisfied, please provide a Reviewed-by for the
> > series.
>
> I usually provide a signed-off-by, as I am the thinkpad-acpi driver
> maintainer... reviewed-by is implied in that case.

Or an Acked-by, for that matter. Which is what I'd use for this series,
since there is no reason to gatekeep it and it is being sent to you
directly.

--
"One disk to rule them all, One disk to find them. One disk to bring
them all and in the darkness grind them. In the Land of Redmond
where the shadows lie." -- The Silicon Valley Tarot
Henrique Holschuh