From 39694d5eb0b7e48e06f9544600041fbbedfff956 Mon Sep 17 00:00:00 2001 From: Ryan Date: Thu, 25 Feb 2021 15:54:25 +1100 Subject: [PATCH] V-USB suspend refactor (#11891) --- quantum/mcu_selection.mk | 13 ----- tmk_core/common/avr/sleep_led.c | 2 +- tmk_core/common/avr/suspend.c | 95 +++++++++++++++++++++---------- tmk_core/common/avr/suspend_avr.h | 25 -------- tmk_core/protocol/vusb/main.c | 54 ++++++++++++------ tmk_core/protocol/vusb/vusb.h | 3 + 6 files changed, 107 insertions(+), 85 deletions(-) delete mode 100644 tmk_core/common/avr/suspend_avr.h diff --git a/quantum/mcu_selection.mk b/quantum/mcu_selection.mk index 6b11eb4987..65c94b50f6 100644 --- 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 diff --git a/tmk_core/common/avr/sleep_led.c b/tmk_core/common/avr/sleep_led.c index 63dcc2afd9..9a3b52abe5 100644 --- 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}; diff --git a/tmk_core/common/avr/suspend.c b/tmk_core/common/avr/suspend.c index 86c3df040a..cb505ab329 100644 --- a/tmk_core/common/avr/suspend.c +++ b/tmk_core/common/avr/suspend.c @@ -4,7 +4,6 @@ #include #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 @@ -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 diff --git a/tmk_core/common/avr/suspend_avr.h b/tmk_core/common/avr/suspend_avr.h deleted file mode 100644 index 6df048f3bb..0000000000 --- a/tmk_core/common/avr/suspend_avr.h +++ /dev/null @@ -1,25 +0,0 @@ -#pragma once - -#include -#include -#include -#include -#include - -// 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 diff --git a/tmk_core/protocol/vusb/main.c b/tmk_core/protocol/vusb/main.c index 2e8bb2fbbc..af64f4e561 100644 --- 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(); } } } diff --git a/tmk_core/protocol/vusb/vusb.h b/tmk_core/protocol/vusb/vusb.h index b4c73aabae..b1ecc98f37 100644 --- 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 . #pragma once #include "host_driver.h" +#include 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);