diff options
author | Joshua Diamond <josh@windowoffire.com> | 2021-02-01 19:12:41 -0500 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-02-02 11:12:41 +1100 |
commit | 9a4618b05b9f1093908c2153c719c5eb5d4a79ee (patch) | |
tree | 7487e6226c72d2c3d09749d60ce8df18a2c40d59 | |
parent | 85079d6a2ecfd55d0d33ce32cd1ad137f1c1df55 (diff) | |
download | qmk_firmware-9a4618b05b9f1093908c2153c719c5eb5d4a79ee.tar.gz qmk_firmware-9a4618b05b9f1093908c2153c719c5eb5d4a79ee.zip |
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
-rw-r--r-- | docs/config_options.md | 2 | ||||
-rw-r--r-- | tmk_core/common/avr/suspend.c | 1 | ||||
-rw-r--r-- | tmk_core/common/suspend.h | 4 | ||||
-rw-r--r-- | tmk_core/protocol/chibios/usb_main.c | 11 | ||||
-rw-r--r-- | 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 a3262b418..9a64b9b3d 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 | |||
97 | * sets the maximum power (in mA) over USB for the device (default: 500) | 97 | * sets the maximum power (in mA) over USB for the device (default: 500) |
98 | * `#define USB_POLLING_INTERVAL_MS 10` | 98 | * `#define USB_POLLING_INTERVAL_MS 10` |
99 | * sets the USB polling rate in milliseconds for the keyboard, mouse, and shared (NKRO/media keys) interfaces | 99 | * sets the USB polling rate in milliseconds for the keyboard, mouse, and shared (NKRO/media keys) interfaces |
100 | * `#define USB_SUSPEND_WAKEUP_DELAY 200` | ||
101 | * set the number of milliseconde to pause after sending a wakeup packet | ||
100 | * `#define F_SCL 100000L` | 102 | * `#define F_SCL 100000L` |
101 | * 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`. | 103 | * 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`. |
102 | 104 | ||
diff --git a/tmk_core/common/avr/suspend.c b/tmk_core/common/avr/suspend.c index 9db0e0064..b784a0835 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) { | |||
102 | # if defined(RGBLIGHT_SLEEP) && defined(RGBLIGHT_ENABLE) | 102 | # if defined(RGBLIGHT_SLEEP) && defined(RGBLIGHT_ENABLE) |
103 | rgblight_suspend(); | 103 | rgblight_suspend(); |
104 | # endif | 104 | # endif |
105 | suspend_power_down_kb(); | ||
106 | 105 | ||
107 | // TODO: more power saving | 106 | // TODO: more power saving |
108 | // See PicoPower application note | 107 | // See PicoPower application note |
diff --git a/tmk_core/common/suspend.h b/tmk_core/common/suspend.h index 766df95aa..9d17d984e 100644 --- a/tmk_core/common/suspend.h +++ b/tmk_core/common/suspend.h | |||
@@ -12,3 +12,7 @@ void suspend_wakeup_init_user(void); | |||
12 | void suspend_wakeup_init_kb(void); | 12 | void suspend_wakeup_init_kb(void); |
13 | void suspend_power_down_user(void); | 13 | void suspend_power_down_user(void); |
14 | void suspend_power_down_kb(void); | 14 | void suspend_power_down_kb(void); |
15 | |||
16 | #ifndef USB_SUSPEND_WAKEUP_DELAY | ||
17 | # define USB_SUSPEND_WAKEUP_DELAY 200 | ||
18 | #endif | ||
diff --git a/tmk_core/protocol/chibios/usb_main.c b/tmk_core/protocol/chibios/usb_main.c index cb7aeb23b..13b1e34d2 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) { | |||
708 | void restart_usb_driver(USBDriver *usbp) { | 708 | void restart_usb_driver(USBDriver *usbp) { |
709 | usbStop(usbp); | 709 | usbStop(usbp); |
710 | usbDisconnectBus(usbp); | 710 | usbDisconnectBus(usbp); |
711 | |||
712 | #if USB_SUSPEND_WAKEUP_DELAY > 0 | ||
713 | // Some hubs, kvm switches, and monitors do | ||
714 | // weird things, with USB device state bouncing | ||
715 | // around wildly on wakeup, yielding race | ||
716 | // conditions that can corrupt the keyboard state. | ||
717 | // | ||
718 | // Pause for a while to let things settle... | ||
719 | wait_ms(USB_SUSPEND_WAKEUP_DELAY); | ||
720 | #endif | ||
721 | |||
711 | usbStart(usbp, &usbcfg); | 722 | usbStart(usbp, &usbcfg); |
712 | usbConnectBus(usbp); | 723 | usbConnectBus(usbp); |
713 | } | 724 | } |
diff --git a/tmk_core/protocol/lufa/lufa.c b/tmk_core/protocol/lufa/lufa.c index 623aa33ff..74e48222d 100644 --- a/tmk_core/protocol/lufa/lufa.c +++ b/tmk_core/protocol/lufa/lufa.c | |||
@@ -435,7 +435,9 @@ void EVENT_USB_Device_Suspend() { | |||
435 | */ | 435 | */ |
436 | void EVENT_USB_Device_WakeUp() { | 436 | void EVENT_USB_Device_WakeUp() { |
437 | print("[W]"); | 437 | print("[W]"); |
438 | #if defined(NO_USB_STARTUP_CHECK) | ||
438 | suspend_wakeup_init(); | 439 | suspend_wakeup_init(); |
440 | #endif | ||
439 | 441 | ||
440 | #ifdef SLEEP_LED_ENABLE | 442 | #ifdef SLEEP_LED_ENABLE |
441 | sleep_led_disable(); | 443 | sleep_led_disable(); |
@@ -1073,12 +1075,26 @@ int main(void) { | |||
1073 | print("Keyboard start.\n"); | 1075 | print("Keyboard start.\n"); |
1074 | while (1) { | 1076 | while (1) { |
1075 | #if !defined(NO_USB_STARTUP_CHECK) | 1077 | #if !defined(NO_USB_STARTUP_CHECK) |
1076 | while (USB_DeviceState == DEVICE_STATE_Suspended) { | 1078 | if (USB_DeviceState == DEVICE_STATE_Suspended) { |
1077 | print("[s]"); | 1079 | print("[s]"); |
1078 | suspend_power_down(); | 1080 | while (USB_DeviceState == DEVICE_STATE_Suspended) { |
1079 | if (USB_Device_RemoteWakeupEnabled && suspend_wakeup_condition()) { | 1081 | suspend_power_down(); |
1080 | USB_Device_SendRemoteWakeup(); | 1082 | if (USB_Device_RemoteWakeupEnabled && suspend_wakeup_condition()) { |
1083 | USB_Device_SendRemoteWakeup(); | ||
1084 | clear_keyboard(); | ||
1085 | |||
1086 | # if USB_SUSPEND_WAKEUP_DELAY > 0 | ||
1087 | // Some hubs, kvm switches, and monitors do | ||
1088 | // weird things, with USB device state bouncing | ||
1089 | // around wildly on wakeup, yielding race | ||
1090 | // conditions that can corrupt the keyboard state. | ||
1091 | // | ||
1092 | // Pause for a while to let things settle... | ||
1093 | wait_ms(USB_SUSPEND_WAKEUP_DELAY); | ||
1094 | # endif | ||
1095 | } | ||
1081 | } | 1096 | } |
1097 | suspend_wakeup_init(); | ||
1082 | } | 1098 | } |
1083 | #endif | 1099 | #endif |
1084 | 1100 | ||