From 9a4618b05b9f1093908c2153c719c5eb5d4a79ee Mon Sep 17 00:00:00 2001 From: Joshua Diamond Date: Mon, 1 Feb 2021 19:12:41 -0500 Subject: [PATCH] Address wake from sleep instability (#11450) * resolve race condition between suspend and wake in LUFA * avoid multiple calls to suspend_power_down() / suspend_wakeup_init() * Remove duplicate suspend_power_down_kb() call * pause on wakeup to wait for USB state to settle * need the repeated suspend_power_down() (that's where the sleep is) * more efficient implementation * fine tune the pause after sending wakeup * speculative chibios version of pause-after-wake * make wakeup delay configurable, and adjust value * better location for wakeup delay --- docs/config_options.md | 2 ++ tmk_core/common/avr/suspend.c | 1 - tmk_core/common/suspend.h | 4 ++++ tmk_core/protocol/chibios/usb_main.c | 11 +++++++++++ tmk_core/protocol/lufa/lufa.c | 24 ++++++++++++++++++++---- 5 files changed, 37 insertions(+), 5 deletions(-) diff --git a/docs/config_options.md b/docs/config_options.md index a3262b418b..9a64b9b3d2 100644 --- a/docs/config_options.md +++ b/docs/config_options.md @@ -97,6 +97,8 @@ This is a C header file that is one of the first things included, and will persi * sets the maximum power (in mA) over USB for the device (default: 500) * `#define USB_POLLING_INTERVAL_MS 10` * sets the USB polling rate in milliseconds for the keyboard, mouse, and shared (NKRO/media keys) interfaces +* `#define USB_SUSPEND_WAKEUP_DELAY 200` + * set the number of milliseconde to pause after sending a wakeup packet * `#define F_SCL 100000L` * sets the I2C clock rate speed for keyboards using I2C. The default is `400000L`, except for keyboards using `split_common`, where the default is `100000L`. diff --git a/tmk_core/common/avr/suspend.c b/tmk_core/common/avr/suspend.c index 9db0e0064a..b784a0835d 100644 --- a/tmk_core/common/avr/suspend.c +++ b/tmk_core/common/avr/suspend.c @@ -102,7 +102,6 @@ static void power_down(uint8_t wdto) { # if defined(RGBLIGHT_SLEEP) && defined(RGBLIGHT_ENABLE) rgblight_suspend(); # endif - suspend_power_down_kb(); // TODO: more power saving // See PicoPower application note diff --git a/tmk_core/common/suspend.h b/tmk_core/common/suspend.h index 766df95aa1..9d17d984ed 100644 --- a/tmk_core/common/suspend.h +++ b/tmk_core/common/suspend.h @@ -12,3 +12,7 @@ void suspend_wakeup_init_user(void); void suspend_wakeup_init_kb(void); void suspend_power_down_user(void); void suspend_power_down_kb(void); + +#ifndef USB_SUSPEND_WAKEUP_DELAY +# define USB_SUSPEND_WAKEUP_DELAY 200 +#endif diff --git a/tmk_core/protocol/chibios/usb_main.c b/tmk_core/protocol/chibios/usb_main.c index cb7aeb23b2..13b1e34d28 100644 --- a/tmk_core/protocol/chibios/usb_main.c +++ b/tmk_core/protocol/chibios/usb_main.c @@ -708,6 +708,17 @@ void init_usb_driver(USBDriver *usbp) { void restart_usb_driver(USBDriver *usbp) { usbStop(usbp); usbDisconnectBus(usbp); + +#if USB_SUSPEND_WAKEUP_DELAY > 0 + // Some hubs, kvm switches, and monitors do + // weird things, with USB device state bouncing + // around wildly on wakeup, yielding race + // conditions that can corrupt the keyboard state. + // + // Pause for a while to let things settle... + wait_ms(USB_SUSPEND_WAKEUP_DELAY); +#endif + usbStart(usbp, &usbcfg); usbConnectBus(usbp); } diff --git a/tmk_core/protocol/lufa/lufa.c b/tmk_core/protocol/lufa/lufa.c index 623aa33ff5..74e48222d0 100644 --- a/tmk_core/protocol/lufa/lufa.c +++ b/tmk_core/protocol/lufa/lufa.c @@ -435,7 +435,9 @@ void EVENT_USB_Device_Suspend() { */ void EVENT_USB_Device_WakeUp() { print("[W]"); +#if defined(NO_USB_STARTUP_CHECK) suspend_wakeup_init(); +#endif #ifdef SLEEP_LED_ENABLE sleep_led_disable(); @@ -1073,12 +1075,26 @@ int main(void) { print("Keyboard start.\n"); while (1) { #if !defined(NO_USB_STARTUP_CHECK) - while (USB_DeviceState == DEVICE_STATE_Suspended) { + if (USB_DeviceState == DEVICE_STATE_Suspended) { print("[s]"); - suspend_power_down(); - if (USB_Device_RemoteWakeupEnabled && suspend_wakeup_condition()) { - USB_Device_SendRemoteWakeup(); + while (USB_DeviceState == DEVICE_STATE_Suspended) { + suspend_power_down(); + if (USB_Device_RemoteWakeupEnabled && suspend_wakeup_condition()) { + USB_Device_SendRemoteWakeup(); + clear_keyboard(); + +# if USB_SUSPEND_WAKEUP_DELAY > 0 + // Some hubs, kvm switches, and monitors do + // weird things, with USB device state bouncing + // around wildly on wakeup, yielding race + // conditions that can corrupt the keyboard state. + // + // Pause for a while to let things settle... + wait_ms(USB_SUSPEND_WAKEUP_DELAY); +# endif + } } + suspend_wakeup_init(); } #endif