From 25331be3164ca609ebecbfcc727ce904e3676594 Mon Sep 17 00:00:00 2001 From: Drashna Jaelre Date: Sat, 26 Aug 2023 19:52:12 -0700 Subject: [PATCH] Revert changes to ChibiOS Suspend Code (#21830) * Partially revert #19780 * Finish * Get teensy 3.5/3.6 board files too * fix lint issue * Revert "[Bug] Restore usb suspend wakeup delay (#21676)" This reverts commit e8e989fd7ad7c10e725e50ae8b0a4426e09f7f30. * Apply suggestions from code review Co-authored-by: Joel Challis --------- Co-authored-by: Joel Challis --- .../boards/GENERIC_WB32_F3G71XX/board/board.c | 4 ++ .../boards/GENERIC_WB32_FQ95XX/board/board.c | 4 ++ .../boards/IC_TEENSY_3_1/board/board.c | 5 ++ .../boards/PJRC_TEENSY_3_5/board/board.mk | 11 ++++ .../boards/PJRC_TEENSY_3_5/board/extra.c | 7 +++ .../boards/PJRC_TEENSY_3_6/board/board.mk | 11 ++++ .../boards/PJRC_TEENSY_3_6/board/extra.c | 7 +++ platforms/chibios/suspend.c | 1 - tmk_core/protocol/chibios/chibios.c | 51 +++++++++++++------ tmk_core/protocol/chibios/usb_main.c | 27 +++++++--- 10 files changed, 105 insertions(+), 23 deletions(-) create mode 100644 platforms/chibios/boards/PJRC_TEENSY_3_5/board/board.mk create mode 100644 platforms/chibios/boards/PJRC_TEENSY_3_5/board/extra.c create mode 100644 platforms/chibios/boards/PJRC_TEENSY_3_6/board/board.mk create mode 100644 platforms/chibios/boards/PJRC_TEENSY_3_6/board/extra.c diff --git a/platforms/chibios/boards/GENERIC_WB32_F3G71XX/board/board.c b/platforms/chibios/boards/GENERIC_WB32_F3G71XX/board/board.c index e38a7e0054..f74c9e8be7 100644 --- a/platforms/chibios/boards/GENERIC_WB32_F3G71XX/board/board.c +++ b/platforms/chibios/boards/GENERIC_WB32_F3G71XX/board/board.c @@ -80,3 +80,7 @@ void __early_init(void) { void boardInit(void) { } + +void restart_usb_driver(USBDriver *usbp) { + // Do nothing. Restarting the USB driver on these boards breaks it. +} diff --git a/platforms/chibios/boards/GENERIC_WB32_FQ95XX/board/board.c b/platforms/chibios/boards/GENERIC_WB32_FQ95XX/board/board.c index 22b4ff73b5..a99537fc27 100644 --- a/platforms/chibios/boards/GENERIC_WB32_FQ95XX/board/board.c +++ b/platforms/chibios/boards/GENERIC_WB32_FQ95XX/board/board.c @@ -80,3 +80,7 @@ void __early_init(void) { void boardInit(void) { } + +void restart_usb_driver(USBDriver *usbp) { + // Do nothing. Restarting the USB driver on these boards breaks it. +} diff --git a/platforms/chibios/boards/IC_TEENSY_3_1/board/board.c b/platforms/chibios/boards/IC_TEENSY_3_1/board/board.c index 36ae8051ee..189d90952d 100644 --- a/platforms/chibios/boards/IC_TEENSY_3_1/board/board.c +++ b/platforms/chibios/boards/IC_TEENSY_3_1/board/board.c @@ -144,3 +144,8 @@ void __early_init(void) { * @todo Add your board-specific code, if any. */ void boardInit(void) {} + + +void restart_usb_driver(USBDriver *usbp) { + // Do nothing. Restarting the USB driver on these boards breaks it. +} diff --git a/platforms/chibios/boards/PJRC_TEENSY_3_5/board/board.mk b/platforms/chibios/boards/PJRC_TEENSY_3_5/board/board.mk new file mode 100644 index 0000000000..e129836b08 --- /dev/null +++ b/platforms/chibios/boards/PJRC_TEENSY_3_5/board/board.mk @@ -0,0 +1,11 @@ +include $(CHIBIOS_CONTRIB)/os/hal/boards/PJRC_TEENSY_3_5/board.mk + +# List of all the board related files. +BOARDSRC += $(BOARD_PATH)/board/extra.c + +# Required include directories +BOARDINC += $(BOARD_PATH)/board + +# Shared variables +ALLCSRC += $(BOARDSRC) +ALLINC += $(BOARDINC) diff --git a/platforms/chibios/boards/PJRC_TEENSY_3_5/board/extra.c b/platforms/chibios/boards/PJRC_TEENSY_3_5/board/extra.c new file mode 100644 index 0000000000..4940d6d99b --- /dev/null +++ b/platforms/chibios/boards/PJRC_TEENSY_3_5/board/extra.c @@ -0,0 +1,7 @@ +#include + +void restart_usb_driver(USBDriver *usbp) { + // Do nothing. Restarting the USB driver on the Teensy 3.6 breaks it, + // resulting in a keyboard which can wake up a PC from Suspend-to-RAM, but + // does not actually produce any keypresses until you un-plug and re-plug. +} diff --git a/platforms/chibios/boards/PJRC_TEENSY_3_6/board/board.mk b/platforms/chibios/boards/PJRC_TEENSY_3_6/board/board.mk new file mode 100644 index 0000000000..aba195db04 --- /dev/null +++ b/platforms/chibios/boards/PJRC_TEENSY_3_6/board/board.mk @@ -0,0 +1,11 @@ +include $(CHIBIOS_CONTRIB)/os/hal/boards/PJRC_TEENSY_3_6/board.mk + +# List of all the board related files. +BOARDSRC += $(BOARD_PATH)/board/extra.c + +# Required include directories +BOARDINC += $(BOARD_PATH)/board + +# Shared variables +ALLCSRC += $(BOARDSRC) +ALLINC += $(BOARDINC) diff --git a/platforms/chibios/boards/PJRC_TEENSY_3_6/board/extra.c b/platforms/chibios/boards/PJRC_TEENSY_3_6/board/extra.c new file mode 100644 index 0000000000..4940d6d99b --- /dev/null +++ b/platforms/chibios/boards/PJRC_TEENSY_3_6/board/extra.c @@ -0,0 +1,7 @@ +#include + +void restart_usb_driver(USBDriver *usbp) { + // Do nothing. Restarting the USB driver on the Teensy 3.6 breaks it, + // resulting in a keyboard which can wake up a PC from Suspend-to-RAM, but + // does not actually produce any keypresses until you un-plug and re-plug. +} diff --git a/platforms/chibios/suspend.c b/platforms/chibios/suspend.c index a937ccf059..ce03433e3a 100644 --- a/platforms/chibios/suspend.c +++ b/platforms/chibios/suspend.c @@ -42,7 +42,6 @@ void suspend_wakeup_init(void) { clear_keys(); #ifdef MOUSEKEY_ENABLE mousekey_clear(); - mousekey_send(); #endif /* MOUSEKEY_ENABLE */ #ifdef PROGRAMMABLE_BUTTON_ENABLE programmable_button_clear(); diff --git a/tmk_core/protocol/chibios/chibios.c b/tmk_core/protocol/chibios/chibios.c index ac39606179..4d97f1cd82 100644 --- a/tmk_core/protocol/chibios/chibios.c +++ b/tmk_core/protocol/chibios/chibios.c @@ -80,6 +80,26 @@ void console_task(void); void midi_ep_task(void); #endif +/* TESTING + * Amber LED blinker thread, times are in milliseconds. + */ +/* set this variable to non-zero anywhere to blink once */ +// static THD_WORKING_AREA(waThread1, 128); +// static THD_FUNCTION(Thread1, arg) { + +// (void)arg; +// chRegSetThreadName("blinker"); +// while (true) { +// systime_t time; + +// time = USB_DRIVER.state == USB_ACTIVE ? 250 : 500; +// palClearLine(LINE_CAPS_LOCK); +// chSysPolledDelayX(MS2RTC(STM32_HCLK, time)); +// palSetLine(LINE_CAPS_LOCK); +// chSysPolledDelayX(MS2RTC(STM32_HCLK, time)); +// } +// } + /* Early initialisation */ __attribute__((weak)) void early_hardware_init_pre(void) { @@ -115,6 +135,9 @@ void boardInit(void) { void protocol_setup(void) { usb_device_state_init(); + + // TESTING + // chThdCreateStatic(waThread1, sizeof(waThread1), NORMALPRIO, Thread1, NULL); } static host_driver_t *driver = NULL; @@ -157,32 +180,28 @@ void protocol_post_init(void) { } void protocol_pre_task(void) { + usb_event_queue_task(); + #if !defined(NO_USB_STARTUP_CHECK) if (USB_DRIVER.state == USB_SUSPENDED) { dprintln("suspending keyboard"); while (USB_DRIVER.state == USB_SUSPENDED) { - suspend_power_down(); + /* Do this in the suspended state */ + suspend_power_down(); // on AVR this deep sleeps for 15ms + /* Remote wakeup */ if ((USB_DRIVER.status & USB_GETSTATUS_REMOTE_WAKEUP_ENABLED) && suspend_wakeup_condition()) { - /* issue a remote wakeup event to the host which should resume - * the bus and get our keyboard out of suspension. */ usbWakeupHost(&USB_DRIVER); -# 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 + restart_usb_driver(&USB_DRIVER); } } - /* after a successful wakeup a USB_EVENT_WAKEUP is signaled to QMK by - * ChibiOS, which triggers a wakeup callback that restores the state of - * the keyboard. Therefore we do nothing here. */ + /* Woken up */ + // variables has been already cleared by the wakeup hook + send_keyboard_report(); +# ifdef MOUSEKEY_ENABLE + mousekey_send(); +# endif /* MOUSEKEY_ENABLE */ } #endif - - usb_event_queue_task(); } void protocol_post_task(void) { diff --git a/tmk_core/protocol/chibios/usb_main.c b/tmk_core/protocol/chibios/usb_main.c index e1327f065c..b14ca30c1a 100644 --- a/tmk_core/protocol/chibios/usb_main.c +++ b/tmk_core/protocol/chibios/usb_main.c @@ -784,19 +784,34 @@ void init_usb_driver(USBDriver *usbp) { #endif } - restart_usb_driver(usbp); + /* + * Activates the USB driver and then the USB bus pull-up on D+. + * Note, a delay is inserted in order to not have to disconnect the cable + * after a reset. + */ + usbDisconnectBus(usbp); + usbStop(usbp); + wait_ms(50); + usbStart(usbp, &usbcfg); + usbConnectBus(usbp); chVTObjectInit(&keyboard_idle_timer); } -/** @brief Restarts the USB driver and emulates a physical bus reconnection. - * Note that the bus reconnection is MCU and even board specific, so it might - * be a NOP on some hardware platforms. - */ __attribute__((weak)) void restart_usb_driver(USBDriver *usbp) { usbDisconnectBus(usbp); usbStop(usbp); - wait_ms(50); + +#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); }