Revert changes to ChibiOS Suspend Code (qmk#21830)

* Partially revert qmk#19780 * Finish * Get teensy 3.5/3.6 board files too * fix lint issue * Revert "[Bug] Restore usb suspend wakeup delay (qmk#21676)" This reverts commit e8e989f. * Apply suggestions from code review Co-authored-by: Joel Challis <git@zvecr.com> --------- Co-authored-by: Joel Challis <git@zvecr.com>
jesperhellberg · Sep 9, 2023 · e86940a · e86940a
1 parent fee1053
commit e86940a
Show file tree

Hide file tree

Showing 10 changed files with 105 additions and 23 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -0,0 +1,7 @@
+#include <hal.h>
+
+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
@@ -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
@@ -0,0 +1,7 @@
+#include <hal.h>
+
+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
@@ -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
@@ -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
@@ -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);
 }