[MERGE][Core] Cleanups to USB Suspend logic (21537)

platforms/chibios/suspend.c

@@ -1,4 +1,5 @@
-/* TODO */
+// Copyright 2023 QMK
+// SPDX-License-Identifier: GPL-2.0-or-later
 
 #include <ch.h>
 #include <hal.h>
@@ -32,24 +33,7 @@ void suspend_power_down(void) {
  */
 void suspend_wakeup_init(void) {
     // clear keyboard state
-    // need to do it manually, because we're running from ISR
-    //  and clear_keyboard() calls print
-    // so only clear the variables in memory
-    // the reports will be sent from main.c afterwards
-    // or if the PC asks for GET_REPORT
-    clear_mods();
-    clear_weak_mods();
-    clear_keys();
-#ifdef MOUSEKEY_ENABLE
-    mousekey_clear();
-#endif /* MOUSEKEY_ENABLE */
-#ifdef PROGRAMMABLE_BUTTON_ENABLE
-    programmable_button_clear();
-#endif /* PROGRAMMABLE_BUTTON_ENABLE */
-#ifdef EXTRAKEY_ENABLE
-    host_system_send(0);
-    host_consumer_send(0);
-#endif /* EXTRAKEY_ENABLE */
+    clear_keyboard();
 
     suspend_wakeup_init_quantum();
 }

tmk_core/protocol/chibios/chibios.c

@@ -137,7 +137,6 @@ static host_driver_t *driver = NULL;
 
 void protocol_pre_init(void) {
     /* Init USB */
-    usb_event_queue_init();
     init_usb_driver(&USB_DRIVER);
 
 #ifdef MIDI_ENABLE
@@ -177,7 +176,6 @@ void protocol_pre_task(void) {
 
 #if !defined(NO_USB_STARTUP_CHECK)
     if (USB_DRIVER.state == USB_SUSPENDED) {
-        dprintln("suspending keyboard");
         while (USB_DRIVER.state == USB_SUSPENDED) {
             /* Do this in the suspended state */
             suspend_power_down(); // on AVR this deep sleeps for 15ms
@@ -198,7 +196,10 @@ void protocol_pre_task(void) {
 #    endif
             }
         }
-        /* Woken up */
+        /* 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. We immedialty check for this after waking up. */
+        usb_event_queue_task();
     }
 #endif
 }

tmk_core/protocol/chibios/usb_main.c

@@ -95,16 +95,9 @@ static const USBDescriptor *usb_get_descriptor_cb(USBDriver *usbp, uint8_t dtype
  */
 
 #define USB_EVENT_QUEUE_SIZE 16
-usbevent_t event_queue[USB_EVENT_QUEUE_SIZE];
-uint8_t    event_queue_head;
-uint8_t    event_queue_tail;
-
-void usb_event_queue_init(void) {
-    // Initialise the event queue
-    memset(&event_queue, 0, sizeof(event_queue));
-    event_queue_head = 0;
-    event_queue_tail = 0;
-}
+static usbevent_t event_queue[USB_EVENT_QUEUE_SIZE];
+static uint8_t    event_queue_head;
+static uint8_t    event_queue_tail;
 
 static inline bool usb_event_queue_enqueue(usbevent_t event) {
     uint8_t next = (event_queue_head + 1) % USB_EVENT_QUEUE_SIZE;
@@ -117,11 +110,14 @@ static inline bool usb_event_queue_enqueue(usbevent_t event) {
 }
 
 static inline bool usb_event_queue_dequeue(usbevent_t *event) {
+    osalSysLock();
     if (event_queue_head == event_queue_tail) {
+        osalSysUnlock();
         return false;
     }
     *event           = event_queue[event_queue_tail];
     event_queue_tail = (event_queue_tail + 1) % USB_EVENT_QUEUE_SIZE;
+    osalSysUnlock();
     return true;
 }
 
@@ -142,18 +138,14 @@ static inline void usb_event_wakeup_handler(void) {
 #endif /* SLEEP_LED_ENABLE */
 }
 
-bool last_suspend_state = false;
-
 void usb_event_queue_task(void) {
     usbevent_t event;
     while (usb_event_queue_dequeue(&event)) {
         switch (event) {
             case USB_EVENT_SUSPEND:
-                last_suspend_state = true;
                 usb_event_suspend_handler();
                 break;
             case USB_EVENT_WAKEUP:
-                last_suspend_state = false;
                 usb_event_wakeup_handler();
                 break;
             case USB_EVENT_CONFIGURED:
@@ -175,55 +167,53 @@ void usb_event_queue_task(void) {
 
 /* Handles the USB driver global events. */
 static void usb_event_cb(USBDriver *usbp, usbevent_t event) {
+    osalSysLockFromISR();
+    static bool was_suspended = false;
+
     switch (event) {
         case USB_EVENT_ADDRESS:
-            return;
-
+            break;
         case USB_EVENT_CONFIGURED:
-            osalSysLockFromISR();
             for (int i = 0; i < USB_ENDPOINT_IN_COUNT; i++) {
                 usb_endpoint_in_configure_cb(&usb_endpoints_in[i]);
             }
             for (int i = 0; i < USB_ENDPOINT_OUT_COUNT; i++) {
                 usb_endpoint_out_configure_cb(&usb_endpoints_out[i]);
             }
-            osalSysUnlockFromISR();
-            if (last_suspend_state) {
+            if (was_suspended) {
                 usb_event_queue_enqueue(USB_EVENT_WAKEUP);
+                was_suspended = false;
             }
             usb_event_queue_enqueue(USB_EVENT_CONFIGURED);
-            return;
+            break;
         case USB_EVENT_SUSPEND:
-            /* Falls into.*/
+            was_suspended = true;
         case USB_EVENT_UNCONFIGURED:
-            /* Falls into.*/
         case USB_EVENT_RESET:
             usb_event_queue_enqueue(event);
-            chSysLockFromISR();
             for (int i = 0; i < USB_ENDPOINT_IN_COUNT; i++) {
                 usb_endpoint_in_suspend_cb(&usb_endpoints_in[i]);
             }
             for (int i = 0; i < USB_ENDPOINT_OUT_COUNT; i++) {
                 usb_endpoint_out_suspend_cb(&usb_endpoints_out[i]);
             }
-            chSysUnlockFromISR();
             return;
 
         case USB_EVENT_WAKEUP:
-            chSysLockFromISR();
             for (int i = 0; i < USB_ENDPOINT_IN_COUNT; i++) {
                 usb_endpoint_in_wakeup_cb(&usb_endpoints_in[i]);
             }
             for (int i = 0; i < USB_ENDPOINT_OUT_COUNT; i++) {
                 usb_endpoint_out_wakeup_cb(&usb_endpoints_out[i]);
             }
-            chSysUnlockFromISR();
             usb_event_queue_enqueue(USB_EVENT_WAKEUP);
-            return;
-
+            was_suspended = false;
+            break;
         case USB_EVENT_STALLED:
-            return;
+            break;
     }
+
+    osalSysUnlockFromISR();
 }
 
 /*

tmk_core/protocol/chibios/usb_main.h

@@ -39,9 +39,6 @@ bool send_report(usb_endpoint_in_lut_t endpoint, void *report, size_t size);
  * ---------------
  */
 
-/* Initialisation of the FIFO */
-void usb_event_queue_init(void);
-
 /* Task to dequeue and execute any handlers for the USB events on the main thread */
 void usb_event_queue_task(void);
 

tmk_core/protocol/lufa/lufa.c

@@ -820,7 +820,6 @@ void protocol_post_init(void) {
 void protocol_pre_task(void) {
 #if !defined(NO_USB_STARTUP_CHECK)
     if (USB_DeviceState == DEVICE_STATE_Suspended) {
-        dprintln("suspending keyboard");
         while (USB_DeviceState == DEVICE_STATE_Suspended) {
             suspend_power_down();
             if (suspend_wakeup_condition() && USB_Device_RemoteWakeupEnabled) {