| Message ID | 1424443450.22910.13.camel@hadess.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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,
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. -- 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
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.
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.
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.
On Fri, Feb 27, 2015 at 08:16:19AM -0300, Henrique de Moraes Holschuh wrote: > 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. > Henrique, I believe I may have overstepped with thinkpad-acpi and dealt with it like the other drivers in platform/drivers/x86, when instead I should have been leaving it to you. My apologies, it was not intentional. Do you typically send pull-requests for the thinkpad-acpi driver directly to Linus? Geeze, I see that the tree listed in MAINTAINERS is not even mine. Ugh, my sincere apologies. I would be more than happy to basically ignore anything to thinkpad-acpi until after you have provided a review. I can also roll up pull requests from you if you prefer to integrate into the platform-drivers-x86 tree as the path to Linus. How do you want to handle your driver?
On Tue, Mar 3, 2015, at 13:52, Darren Hart wrote: > Henrique, I believe I may have overstepped with thinkpad-acpi and dealt > with it like the other drivers in platform/drivers/x86, when instead I should > have been leaving it to you. My apologies, it was not intentional. No harm done! And I did appreciate the extra help :-) > Do you typically send pull-requests for the thinkpad-acpi driver directly to Linus? thinkpad-acpi is part of the platform-x86 subsystem, so IMHO it should continue to be merged through your tree if you are okay with that. I'd really appreciate if the typical patch approval flux would go through me first. However, I certainly don't mind if you merge patches directly should I be unresponsible for some reason, or when they are part of the usual tree-wide spot fixes for some pattern/anti-pattern, etc. > I would be more than happy to basically ignore anything to thinkpad-acpi until > after you have provided a review. I can also roll up pull requests from > you if you prefer to integrate into the platform-drivers-x86 tree as the path to > Linus. Well, if it is fine with you, I'd suggest we continue with the process used in this patchset, where I will reply to patches in this ML with a reviewed-by or acked-by before you merge them. If you want to change that at some point, just drop me a note and I will start pushing patches to you either through this ML or through signed pull requests, whichever you prefer.
On Tue, Mar 03, 2015 at 03:39:54PM -0300, Henrique de Moraes Holschuh wrote: > On Tue, Mar 3, 2015, at 13:52, Darren Hart wrote: > > Henrique, I believe I may have overstepped with thinkpad-acpi and dealt > > with it like the other drivers in platform/drivers/x86, when instead I should > > have been leaving it to you. My apologies, it was not intentional. > > No harm done! And I did appreciate the extra help :-) > > > Do you typically send pull-requests for the thinkpad-acpi driver directly to Linus? > > thinkpad-acpi is part of the platform-x86 subsystem, so IMHO it should > continue to be merged through your tree if you are okay with that. > > I'd really appreciate if the typical patch approval flux would go > through me first. However, I certainly don't mind if you merge patches > directly should I be unresponsible for some reason, or when they are > part of the usual tree-wide spot fixes for some pattern/anti-pattern, > etc. > > > I would be more than happy to basically ignore anything to thinkpad-acpi until > > after you have provided a review. I can also roll up pull requests from > > you if you prefer to integrate into the platform-drivers-x86 tree as the path to > > Linus. > > Well, if it is fine with you, I'd suggest we continue with the process > used in this patchset, where I will reply to patches in this ML with a > reviewed-by or acked-by before you merge them. > > If you want to change that at some point, just drop me a note and I will > start pushing patches to you either through this ML or through signed > pull requests, whichever you prefer. Great. Things are documented correctly in MAINTAINERS then and we can continue as before, only I will be more diligent making sure you are Cc'd and have responded to any thinkpad-acpi.c changes. I assume two weeks is sufficient, but may nag you after a week. Thanks!
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"); } } }
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 | 37 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 19 deletions(-)