| Message ID | 1363454057-7409-1-git-send-email-mathieu.poirier@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sat, Mar 16, 2013 at 10:14 AM, <mathieu.poirier@linaro.org> wrote: > From: "Mathieu J. Poirier" <mathieu.poirier@linaro.org> > > Some devices have too few buttons, which it makes it hard to have > a reset combo that won't trigger automatically. As such a > timeout functionality that requires the combination to be held for > a given amount of time before triggering is introduced. > > If a key combo is recognized and held for a 'timeout' amount of time, > the system triggers a reset. The code seems to only require one of the keys in the combo to be held for the full 'timeout' amount of time: ... > /* key release */ > - if (--state->reset_seq_cnt == 0) > + if (--state->reset_seq_cnt == 0) { > state->reset_canceled = false; > + del_timer(&state->keyreset_timeout); > + }
On 13-03-18 06:44 PM, Arve Hjønnevåg wrote: > On Sat, Mar 16, 2013 at 10:14 AM, <mathieu.poirier@linaro.org> wrote: >> From: "Mathieu J. Poirier" <mathieu.poirier@linaro.org> >> >> Some devices have too few buttons, which it makes it hard to have >> a reset combo that won't trigger automatically. As such a >> timeout functionality that requires the combination to be held for >> a given amount of time before triggering is introduced. >> >> If a key combo is recognized and held for a 'timeout' amount of time, >> the system triggers a reset. > > The code seems to only require one of the keys in the combo to be held > for the full 'timeout' amount of time: > > ... >> /* key release */ >> - if (--state->reset_seq_cnt == 0) >> + if (--state->reset_seq_cnt == 0) { >> state->reset_canceled = false; >> + del_timer(&state->keyreset_timeout); >> + } > I think the case you are referring to is when keys are released - please correct me if I'm wrong. You are correct, after the timer has been initialised (meaning that _all_ the keys have been pressed) only a single key need to be held down to carry out the reset order. The current keyreset driver works exactly the same way - I just tried it on my board. Get back to me if you want to see this corrected. Thanks, Mathieu. -- 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 Wed, Mar 20, 2013 at 8:11 AM, Mathieu Poirier <mathieu.poirier@linaro.org> wrote: > On 13-03-18 06:44 PM, Arve Hjønnevåg wrote: >> On Sat, Mar 16, 2013 at 10:14 AM, <mathieu.poirier@linaro.org> wrote: >>> From: "Mathieu J. Poirier" <mathieu.poirier@linaro.org> >>> >>> Some devices have too few buttons, which it makes it hard to have >>> a reset combo that won't trigger automatically. As such a >>> timeout functionality that requires the combination to be held for >>> a given amount of time before triggering is introduced. >>> >>> If a key combo is recognized and held for a 'timeout' amount of time, >>> the system triggers a reset. >> >> The code seems to only require one of the keys in the combo to be held >> for the full 'timeout' amount of time: >> >> ... >>> /* key release */ >>> - if (--state->reset_seq_cnt == 0) >>> + if (--state->reset_seq_cnt == 0) { >>> state->reset_canceled = false; >>> + del_timer(&state->keyreset_timeout); >>> + } >> > > I think the case you are referring to is when keys are released - please > correct me if I'm wrong. > > You are correct, after the timer has been initialised (meaning that > _all_ the keys have been pressed) only a single key need to be held down > to carry out the reset order. > > The current keyreset driver works exactly the same way - I just tried it > on my board. > > Get back to me if you want to see this corrected. > Since this feature so far has only been used with a single key, fixing the code is not a high priority. However, I don't think the current description matches the implementation you have, so one of them should change.
diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c index 7953dfa..8e62062 100644 --- a/drivers/tty/sysrq.c +++ b/drivers/tty/sysrq.c @@ -42,6 +42,7 @@ #include <linux/input.h> #include <linux/uaccess.h> #include <linux/moduleparam.h> +#include <linux/jiffies.h> #include <asm/ptrace.h> #include <asm/irq_regs.h> @@ -50,6 +51,9 @@ static int __read_mostly sysrq_enabled = SYSRQ_DEFAULT_ENABLE; static bool __read_mostly sysrq_always_enabled; +unsigned short platform_sysrq_reset_seq[] __weak = { KEY_RESERVED }; +int sysrq_downtime_ms __weak; + static bool sysrq_on(void) { return sysrq_enabled || sysrq_always_enabled; @@ -584,6 +588,7 @@ struct sysrq_state { int reset_seq_len; int reset_seq_cnt; int reset_seq_version; + struct timer_list keyreset_timeout; }; #define SYSRQ_KEY_RESET_MAX 20 /* Should be plenty */ @@ -617,7 +622,14 @@ static void sysrq_parse_reset_sequence(struct sysrq_state *state) state->reset_seq_version = sysrq_reset_seq_version; } -static bool sysrq_detect_reset_sequence(struct sysrq_state *state, +static void sysrq_handle_reset_request(struct sysrq_state *state) +{ + unsigned long expires = jiffies + msecs_to_jiffies(sysrq_downtime_ms); + state->keyreset_timeout.expires = expires; + add_timer(&state->keyreset_timeout); +} + +static void sysrq_detect_reset_sequence(struct sysrq_state *state, unsigned int code, int value) { if (!test_bit(code, state->reset_keybit)) { @@ -629,17 +641,22 @@ static bool sysrq_detect_reset_sequence(struct sysrq_state *state, state->reset_canceled = true; } else if (value == 0) { /* key release */ - if (--state->reset_seq_cnt == 0) + if (--state->reset_seq_cnt == 0) { state->reset_canceled = false; + del_timer(&state->keyreset_timeout); + } } else if (value == 1) { /* key press, not autorepeat */ if (++state->reset_seq_cnt == state->reset_seq_len && - !state->reset_canceled) { - return true; + !state->reset_canceled) { + sysrq_handle_reset_request(state); } } +} - return false; +static void sysrq_handle_reset_timeout(unsigned long data) +{ + __handle_sysrq(sysrq_xlate[KEY_B], false); } static void sysrq_reinject_alt_sysrq(struct work_struct *work) @@ -746,10 +763,8 @@ static bool sysrq_handle_keypress(struct sysrq_state *sysrq, if (was_active) schedule_work(&sysrq->reinject_work); - if (sysrq_detect_reset_sequence(sysrq, code, value)) { - /* Force emergency reboot */ - __handle_sysrq(sysrq_xlate[KEY_B], false); - } + /* Check for reset sequence */ + sysrq_detect_reset_sequence(sysrq, code, value); } else if (value == 0 && test_and_clear_bit(code, sysrq->key_down)) { /* @@ -810,6 +825,8 @@ static int sysrq_connect(struct input_handler *handler, sysrq->handle.handler = handler; sysrq->handle.name = "sysrq"; sysrq->handle.private = sysrq; + init_timer(&sysrq->keyreset_timeout); + sysrq->keyreset_timeout.function = sysrq_handle_reset_timeout; error = input_register_handle(&sysrq->handle); if (error) { @@ -839,6 +856,7 @@ static void sysrq_disconnect(struct input_handle *handle) input_close_device(handle); cancel_work_sync(&sysrq->reinject_work); + del_timer(&sysrq->keyreset_timeout); input_unregister_handle(handle); kfree(sysrq); } @@ -868,8 +886,6 @@ static struct input_handler sysrq_handler = { static bool sysrq_handler_registered; -unsigned short platform_sysrq_reset_seq[] __weak = { KEY_RESERVED }; - static inline void sysrq_register_handler(void) { unsigned short key; @@ -929,6 +945,8 @@ static struct kernel_param_ops param_ops_sysrq_reset_seq = { module_param_array_named(reset_seq, sysrq_reset_seq, sysrq_reset_seq, &sysrq_reset_seq_len, 0644); +module_param(sysrq_downtime_ms, int, 0644); + #else static inline void sysrq_register_handler(void)