V-USB suspend refactor (#11891)

2021-02-25 15:54:25 +11:00 · 2021-02-25 15:54:25 +11:00 · 39694d5eb0
commit 39694d5eb0
parent 46f4422a87
6 changed files with 107 additions and 85 deletions
--- a/quantum/mcu_selection.mk
+++ b/quantum/mcu_selection.mk
@ -334,9 +334,6 @@ ifneq (,$(filter $(MCU),atmega32a))
  #     calculate timings. Do NOT tack on a 'UL' at the end, this will be done
  #     automatically to create a 32-bit value in your source code.
  F_CPU ?= 12000000
-
-  # unsupported features for now
-  NO_SUSPEND_POWER_DOWN ?= yes
 endif

 ifneq (,$(filter $(MCU),atmega328p))
@ -351,9 +348,6 @@ ifneq (,$(filter $(MCU),atmega328p))
  #     calculate timings. Do NOT tack on a 'UL' at the end, this will be done
  #     automatically to create a 32-bit value in your source code.
  F_CPU ?= 16000000
-
-  # unsupported features for now
-  NO_SUSPEND_POWER_DOWN ?= yes
 endif

 ifneq (,$(filter $(MCU),atmega328))
@ -368,10 +362,6 @@ ifneq (,$(filter $(MCU),atmega328))
  #     calculate timings. Do NOT tack on a 'UL' at the end, this will be done
  #     automatically to create a 32-bit value in your source code.
  F_CPU ?= 16000000
-
-  # unsupported features for now
-  NO_UART ?= yes
-  NO_SUSPEND_POWER_DOWN ?= yes
 endif

 ifneq (,$(filter $(MCU),attiny85))
@ -383,7 +373,4 @@ ifneq (,$(filter $(MCU),attiny85))
  #     calculate timings. Do NOT tack on a 'UL' at the end, this will be done
  #     automatically to create a 32-bit value in your source code.
  F_CPU ?= 16500000
-
-  # unsupported features for now
-  NO_SUSPEND_POWER_DOWN ?= yes
 endif
--- a/tmk_core/common/avr/sleep_led.c
+++ b/tmk_core/common/avr/sleep_led.c
@ -90,7 +90,7 @@ void sleep_led_toggle(void) {
 *
 * (64[steps] * 4[duration]) / 64[PWM periods/s] = 4 second breath cycle
 *
- * http://www.wolframalpha.com/input/?i=%28sin%28+x%2F64*pi%29**8+*+255%2C+x%3D0+to+63
+ * https://www.wolframalpha.com/input/?i=sin%28x%2F64*pi%29**8+*+255%2C+x%3D0+to+63
 * (0..63).each {|x| p ((sin(x/64.0*PI)**8)*255).to_i }
 */
 static const uint8_t breathing_table[64] PROGMEM = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 2, 4, 6, 10, 15, 23, 32, 44, 58, 74, 93, 113, 135, 157, 179, 199, 218, 233, 245, 252, 255, 252, 245, 233, 218, 199, 179, 157, 135, 113, 93, 74, 58, 44, 32, 23, 15, 10, 6, 4, 2, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0};
--- a/tmk_core/common/avr/suspend.c
+++ b/tmk_core/common/avr/suspend.c
@ -4,7 +4,6 @@
 #include <avr/interrupt.h>
 #include "matrix.h"
 #include "action.h"
-#include "suspend_avr.h"
 #include "suspend.h"
 #include "timer.h"
 #include "led.h"
@ -13,6 +12,9 @@
 #ifdef PROTOCOL_LUFA
 #    include "lufa.h"
 #endif
+#ifdef PROTOCOL_VUSB
+#    include "vusb.h"
+#endif

 #ifdef BACKLIGHT_ENABLE
 #    include "backlight.h"
@ -52,7 +54,25 @@ __attribute__((weak)) void suspend_power_down_user(void) {}
 */
 __attribute__((weak)) void suspend_power_down_kb(void) { suspend_power_down_user(); }

-#ifndef NO_SUSPEND_POWER_DOWN
+#if !defined(NO_SUSPEND_POWER_DOWN) && defined(WDT_vect)
+
+// clang-format off
+#define wdt_intr_enable(value) \
+__asm__ __volatile__ ( \
+    "in __tmp_reg__,__SREG__" "\n\t" \
+    "cli" "\n\t" \
+    "wdr" "\n\t" \
+    "sts %0,%1" "\n\t" \
+    "out __SREG__,__tmp_reg__" "\n\t" \
+    "sts %0,%2" "\n\t" \
+    : /* no outputs */ \
+    : "M" (_SFR_MEM_ADDR(_WD_CONTROL_REG)), \
+    "r" (_BV(_WD_CHANGE_BIT) | _BV(WDE)), \
+    "r" ((uint8_t) ((value & 0x08 ? _WD_PS3_MASK : 0x00) | _BV(WDIE) | (value & 0x07))) \
+    : "r0" \
+)
+// clang-format on
+
 /** \brief Power down MCU with watchdog timer
 *
 * wdto: watchdog timer timeout defined in <avr/wdt.h>
@ -74,37 +94,11 @@ static uint8_t wdt_timeout = 0;
 * FIXME: needs doc
 */
 static void power_down(uint8_t wdto) {
-#    ifdef PROTOCOL_LUFA
-    if (USB_DeviceState == DEVICE_STATE_Configured) return;
-#    endif
    wdt_timeout = wdto;

    // Watchdog Interrupt Mode
    wdt_intr_enable(wdto);

-#    ifdef BACKLIGHT_ENABLE
-    backlight_set(0);
-#    endif
-
-    // Turn off LED indicators
-    uint8_t leds_off = 0;
-#    if defined(BACKLIGHT_CAPS_LOCK) && defined(BACKLIGHT_ENABLE)
-    if (is_backlight_enabled()) {
-        // Don't try to turn off Caps Lock indicator as it is backlight and backlight is already off
-        leds_off |= (1 << USB_LED_CAPS_LOCK);
-    }
-#    endif
-    led_set(leds_off);
-
-#    ifdef AUDIO_ENABLE
-    // This sometimes disables the start-up noise, so it's been disabled
-    // stop_all_notes();
-#    endif /* AUDIO_ENABLE */
-#    if defined(RGBLIGHT_SLEEP) && defined(RGBLIGHT_ENABLE)
-    rgblight_suspend();
-#    endif
-    suspend_power_down_kb();
-
    // TODO: more power saving
    // See PicoPower application note
    // - I/O port input with pullup
@ -127,10 +121,46 @@ static void power_down(uint8_t wdto) {
 * FIXME: needs doc
 */
 void suspend_power_down(void) {
+#ifdef PROTOCOL_LUFA
+    if (USB_DeviceState == DEVICE_STATE_Configured) return;
+#endif
+#ifdef PROTOCOL_VUSB
+    if (!vusb_suspended) return;
+#endif
+
    suspend_power_down_kb();

 #ifndef NO_SUSPEND_POWER_DOWN
+    // Turn off backlight
+#    ifdef BACKLIGHT_ENABLE
+    backlight_set(0);
+#    endif
+
+    // Turn off LED indicators
+    uint8_t leds_off = 0;
+#    if defined(BACKLIGHT_CAPS_LOCK) && defined(BACKLIGHT_ENABLE)
+    if (is_backlight_enabled()) {
+        // Don't try to turn off Caps Lock indicator as it is backlight and backlight is already off
+        leds_off |= (1 << USB_LED_CAPS_LOCK);
+    }
+#    endif
+    led_set(leds_off);
+
+    // Turn off audio
+#    ifdef AUDIO_ENABLE
+    // This sometimes disables the start-up noise, so it's been disabled
+    // stop_all_notes();
+#    endif
+
+    // Turn off underglow
+#    if defined(RGBLIGHT_SLEEP) && defined(RGBLIGHT_ENABLE)
+    rgblight_suspend();
+#    endif
+
+    // Enter sleep state if possible (ie, the MCU has a watchdog timeout interrupt)
+#    if defined(WDT_vect)
    power_down(WDTO_15MS);
+#    endif
 #endif
 }

@ -164,17 +194,24 @@ __attribute__((weak)) void suspend_wakeup_init_kb(void) { suspend_wakeup_init_us
 void suspend_wakeup_init(void) {
    // clear keyboard state
    clear_keyboard();
+
+    // Turn on backlight
 #ifdef BACKLIGHT_ENABLE
    backlight_init();
 #endif
+
+    // Restore LED indicators
    led_set(host_keyboard_leds());
+
+    // Wake up underglow
 #if defined(RGBLIGHT_SLEEP) && defined(RGBLIGHT_ENABLE)
    rgblight_wakeup();
 #endif
+
    suspend_wakeup_init_kb();
 }

-#ifndef NO_SUSPEND_POWER_DOWN
+#if !defined(NO_SUSPEND_POWER_DOWN) && defined(WDT_vect)
 /* watchdog timeout */
 ISR(WDT_vect) {
    // compensate timer for sleep
--- a/tmk_core/common/avr/suspend_avr.h
+++ b/tmk_core/common/avr/suspend_avr.h
@ -1,25 +0,0 @@
-#pragma once
-
-#include <stdint.h>
-#include <stdbool.h>
-#include <avr/sleep.h>
-#include <avr/wdt.h>
-#include <avr/interrupt.h>
-
-// clang-format off
-#define wdt_intr_enable(value)              \
-__asm__ __volatile__ (                      \
-    "in __tmp_reg__,__SREG__"        "\n\t" \
-    "cli"                            "\n\t" \
-    "wdr"                            "\n\t" \
-    "sts %0,%1"                      "\n\t" \
-    "out __SREG__,__tmp_reg__"       "\n\t" \
-    "sts %0,%2"                      "\n\t" \
-    : /* no outputs */                      \
-    : "M" (_SFR_MEM_ADDR(_WD_CONTROL_REG)), \
-    "r" (_BV(_WD_CHANGE_BIT) | _BV(WDE)),   \
-    "r" ((uint8_t) ((value & 0x08 ? _WD_PS3_MASK : 0x00) | \
-        _BV(WDIE) | (value & 0x07)) )       \
-    : "r0"  \
-)
-// clang-format on
--- a/tmk_core/protocol/vusb/main.c
+++ b/tmk_core/protocol/vusb/main.c
@ -53,10 +53,10 @@ static void initForUsbConnectivity(void) {
    usbDeviceConnect();
 }

-static void usb_remote_wakeup(void) {
+static void vusb_send_remote_wakeup(void) {
    cli();

-    int8_t ddr_orig = USBDDR;
+    uint8_t ddr_orig = USBDDR;
    USBOUT |= (1 << USBMINUS);
    USBDDR = ddr_orig | USBMASK;
    USBOUT ^= USBMASK;
@ -70,6 +70,27 @@ static void usb_remote_wakeup(void) {
    sei();
 }

+bool vusb_suspended = false;
+
+static void vusb_suspend(void) {
+    vusb_suspended = true;
+
+#ifdef SLEEP_LED_ENABLE
+    sleep_led_enable();
+#endif
+
+    suspend_power_down();
+}
+
+static void vusb_wakeup(void) {
+    vusb_suspended = false;
+    suspend_wakeup_init();
+
+#ifdef SLEEP_LED_ENABLE
+    sleep_led_disable();
+#endif
+}
+
 /** \brief Setup USB
 *
 * FIXME: Needs doc
@ -87,9 +108,8 @@ static void setup_usb(void) {
 */
 int main(void) __attribute__((weak));
 int main(void) {
-    bool suspended = false;
 #if USB_COUNT_SOF
-    uint16_t last_timer = timer_read();
+    uint16_t sof_timer = timer_read();
 #endif

 #ifdef CLKPR
@ -112,23 +132,24 @@ int main(void) {
    while (1) {
 #if USB_COUNT_SOF
        if (usbSofCount != 0) {
-            suspended   = false;
            usbSofCount = 0;
-            last_timer  = timer_read();
-#    ifdef SLEEP_LED_ENABLE
-            sleep_led_disable();
-#    endif
+            sof_timer   = timer_read();
+            if (vusb_suspended) {
+                vusb_wakeup();
+            }
        } else {
            // Suspend when no SOF in 3ms-10ms(7.1.7.4 Suspending of USB1.1)
-            if (timer_elapsed(last_timer) > 5) {
-                suspended = true;
-#    ifdef SLEEP_LED_ENABLE
-                sleep_led_enable();
-#    endif
+            if (!vusb_suspended && timer_elapsed(sof_timer) > 5) {
+                vusb_suspend();
            }
        }
 #endif
-        if (!suspended) {
+        if (vusb_suspended) {
+            vusb_suspend();
+            if (suspend_wakeup_condition()) {
+                vusb_send_remote_wakeup();
+            }
+        } else {
            usbPoll();

            // TODO: configuration process is inconsistent. it sometime fails.
@ -145,6 +166,7 @@ int main(void) {
                raw_hid_task();
            }
 #endif
+
 #ifdef CONSOLE_ENABLE
            usbPoll();

@ -156,8 +178,6 @@ int main(void) {
            // Run housekeeping
            housekeeping_task_kb();
            housekeeping_task_user();
-        } else if (suspend_wakeup_condition()) {
-            usb_remote_wakeup();
        }
    }
 }
--- a/tmk_core/protocol/vusb/vusb.h
+++ b/tmk_core/protocol/vusb/vusb.h
@ -18,6 +18,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #pragma once

 #include "host_driver.h"
+#include <usbdrv/usbdrv.h>

 typedef struct usbDescriptorHeader {
    uchar bLength;
@ -119,5 +120,7 @@ typedef struct usbConfigurationDescriptor {

 #define USB_STRING_LEN(s) (sizeof(usbDescriptorHeader_t) + ((s) << 1))

+extern bool vusb_suspended;
+
 host_driver_t *vusb_driver(void);
 void           vusb_transfer_keyboard(void);