| Message ID | 1424292808.32581.45.camel@hadess.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Feb 18, 2015 at 09:53:28PM +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. Reasonable. > > Signed-off-by: Bastien Nocera <hadess@hadess.net> > --- > drivers/platform/x86/thinkpad_acpi.c | 38 ++++++++++++++++++------------------ > 1 file changed, 19 insertions(+), 19 deletions(-) > > diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c > index c3d11fa..80db3ce 100644 > --- a/drivers/platform/x86/thinkpad_acpi.c > +++ b/drivers/platform/x86/thinkpad_acpi.c ... > @@ -3226,6 +3227,13 @@ static int __init hotkey_init(struct ibm_init_struct *iibm) > if (!tp_features.hotkey) > return 1; > > + /* does it have an adaptive keyboard, like > + * the Lenovo Carbon X1 2014 (2nd gen) */ Please follow CodingStyle here: /* * Sentence formatting. * Second line. */ The above is a question, just state what you're doing: /* * 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 +3842,22 @@ 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 +3872,12 @@ 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)) { You've saved a level of indentation, please group the arguments into fewer lines, should be two instead of four. Otherwise, looks good.
On Thu, 2015-02-19 at 21:08 -0800, Darren Hart wrote: > On Wed, Feb 18, 2015 at 09:53:28PM +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. > > Reasonable. > > > > > Signed-off-by: Bastien Nocera <hadess@hadess.net> > > --- > > drivers/platform/x86/thinkpad_acpi.c | 38 ++++++++++++++++++------------------ > > 1 file changed, 19 insertions(+), 19 deletions(-) > > > > diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c > > index c3d11fa..80db3ce 100644 > > --- a/drivers/platform/x86/thinkpad_acpi.c > > +++ b/drivers/platform/x86/thinkpad_acpi.c > ... > > @@ -3226,6 +3227,13 @@ static int __init hotkey_init(struct ibm_init_struct *iibm) > > if (!tp_features.hotkey) > > return 1; > > > > + /* does it have an adaptive keyboard, like > > + * the Lenovo Carbon X1 2014 (2nd gen) */ > > Please follow CodingStyle here: > > /* > * Sentence formatting. > * Second line. > */ > > The above is a question, just state what you're doing: > > /* > * Check if we have an adaptive keyboard, like on the > * Lenovo Carbon X1 2014 (2nd Gen). > */ Done. > > + 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 +3842,22 @@ 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 +3872,12 @@ 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)) { > > You've saved a level of indentation, please group the arguments into fewer > lines, should be two instead of four. > > Otherwise, looks good. Done. -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c index c3d11fa..80db3ce 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,13 @@ static int __init hotkey_init(struct ibm_init_struct *iibm) if (!tp_features.hotkey) return 1; + /* does it have an adaptive keyboard, like + * 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 +3842,22 @@ 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 +3872,12 @@ 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"); } } }
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 <hadess@hadess.net> --- drivers/platform/x86/thinkpad_acpi.c | 38 ++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 19 deletions(-)