From e3cf9e10f32ed31cafa6929339c28af6607dd4ad Mon Sep 17 00:00:00 2001 From: Rodrigo Torres Date: Thu, 20 Jan 2022 12:11:45 -0800 Subject: [PATCH 1/3] Add opt-in support for "LED_ENABLED" feature Makes LED support "opt-in" by defining "LED_ENABLED". Feature takes 6 bytes when enabled. Adds macros to handle active high and active low differences. Compared to Osamu's original implementation, moves turning on the LED to "run_bootloader", instead of "send_descriptor"; the behavior is pretty much the same, but it saves two bytes for the active-low case, since we only call `cbi` once when the bootloader becomes active (after setting T flag in SREG). Removes old and unused LED_PIN declaration. --- nanoBoot.S | 107 +++++++++++++++++++++++++++++++++++++---------------- 1 file changed, 75 insertions(+), 32 deletions(-) diff --git a/nanoBoot.S b/nanoBoot.S index fccf016..506353c 100644 --- a/nanoBoot.S +++ b/nanoBoot.S @@ -48,34 +48,76 @@ ; hfuse memory = 0xDF ; efuse memory = 0xC4 -; LED -- Configure this for the LED -; -; Teensy 2.0 compatible board (Adjust for other board) + +; ========================================================== +; LED SUPPORT START + +; Turn LED on while bootloader is active +; NOTE: This feature uses 6 bytes for active high and 8 bytes for active low + +; Uncomment the following line to enable LED feature +; #define LED_ENABLED + +; LED Configuration +; Uncomment or add a new LED configuration for your specific board + +; Adafruit's Atmega32u4 Breakout Board (Product ID: 296) - Now discontinued +; https://www.adafruit.com/product/296 +; -- LED is ON with ATmega32u4 PIN E6 HIGH +; #define LED_BIT 6 +; #define LED_CONF DDRE +; #define LED_PORT PORTE +; #define LED_ACTIVE_LEVEL 1 + +; Teensy 2.0 compatible board ; -- LED is ON with ATmega32u4 PIN D6 HIGH -#define LED_BIT 6 -#define LED_CONF DDRD -#define LED_PORT PORTD +; #define LED_BIT 6 +; #define LED_CONF DDRD +; #define LED_PORT PORTD +; #define LED_ACTIVE_LEVEL 1 -; Leonardo/Nano compatible board (Adjust for other board) +; Leonardo/Nano compatible board ; -- LED is ON with ATmega32u4 PIN C7 HIGH -; #define LED_BIT 7 -; #define LED_CONF DDRC -; #define LED_PORT PORTC +; #define LED_BIT 7 +; #define LED_CONF DDRC +; #define LED_PORT PORTC +; #define LED_ACTIVE_LEVEL 1 -; Pro Micro compatible board (Adjust for other board) +; Pro Micro compatible board ; -- LED is ON with ATmega32u4 PIN D5 LOW -; #define LED_BIT 5 -; #define LED_CONF DDRD -; #define LED_PORT PORTD +; #define LED_BIT 5 +; #define LED_CONF DDRD +; #define LED_PORT PORTD +; #define LED_ACTIVE_LEVEL 0 -; Pro Micro compatible board (Adjust for other board) +; Pro Micro compatible board ; -- LED is ON with ATmega32u4 PIN B3 LOW -; #define LED_BIT 3 -; #define LED_CONF DDRB -; #define LED_PORT PORTB +; #define LED_BIT 3 +; #define LED_CONF DDRB +; #define LED_PORT PORTB +; #define LED_ACTIVE_LEVEL 0 + +#if defined(LED_ENABLED) + #if !defined(LED_PORT) || !defined(LED_CONF) || !defined(LED_BIT) || !defined(LED_ACTIVE_LEVEL) + #error "If LED feature is enabled, the following need to be defined: LED_BIT, LED_CONF, LED_PORT, LED_ACTIVE_LEVEL" + #else + ; Set IO register as output for LED + #define ENABLE_LED_OUTPUT sbi _SFR_IO_ADDR(LED_CONF), LED_BIT + #if LED_ACTIVE_LEVEL == 1 + #define TURN_LED_ON sbi _SFR_IO_ADDR(LED_PORT), LED_BIT + #define TURN_LED_OFF cbi _SFR_IO_ADDR(LED_PORT), LED_BIT + #elif LED_ACTIVE_LEVEL == 0 + #define TURN_LED_ON cbi _SFR_IO_ADDR(LED_PORT), LED_BIT + #define TURN_LED_OFF sbi _SFR_IO_ADDR(LED_PORT), LED_BIT + #else + #error "LED_ACTIVE_LEVEL needs to be either 1 (active high) or 0 (active low)" + #endif + #endif +#endif + +; LED SUPPORT END +; ========================================================== -; Except for Pro Micro compatible board, initially LED is off. -; This code assumes Teensy 2.0 or Leonardo/Nano compatible board ; SW assumptions: ; The bootloader only "needs" endpoint 0; however, the HID spec requires any HID device to have an @@ -162,9 +204,6 @@ # define BOOT_ADDRESS 0 #endif -; For debugging purposes -.equ LED_PIN, 6 - .section .vectors ; We still want the reset vector to jump to "main" @@ -357,11 +396,16 @@ run_application: ; We get here if the cause of th jmp 0 ; Simply jump to 0x0000 (application) IMPORTANT NOTE!! This CANNOT be an 'rjmp'!! run_bootloader: - sbi _SFR_IO_ADDR(LED_CONF), LED_BIT ; Set IO register as output for LED - ; No need to trun off LED initially (non-promicro) -- MCU port is initialized as 0 - ;sbi _SFR_IO_ADDR(LED_PORT), LED_BIT ; Trun off LED initially (promicro) set ; Initialize BootLoaderActive flag (T flag in SREG) +#if defined(LED_ENABLED) + ; Set IO register as output for LED + ENABLE_LED_OUTPUT + ; Turn LED on while bootloader is active + TURN_LED_ON +#endif + + ; ================================================================= ; = Setup IRQ Vector Table @@ -551,13 +595,15 @@ exit_bootloader: ori r16, _BV(DETACH) ; Set the DETACH bit to enable the detachment std Y+oUDCON, r16 ; Store r16 to the USB Device Configuration Register (UDCON) +#if defined(LED_ENABLED) + ; Turn LED off before exiting + TURN_LED_OFF +#endif + ; ================================================================= ; = Watchdog Timer initialization ; ================================================================= - cbi _SFR_IO_ADDR(LED_PORT), LED_BIT ; Trun off LED before exiting (non-promicro) - ;sbi _SFR_IO_ADDR(LED_PORT), LED_BIT ; Trun off LED before exiting (promicro) - ; NOTE!! This part of the code assumes MCUSR has already been cleared ; Enable WDT, ~250 ms timeout (force a timeout to reset the AVR) @@ -937,9 +983,6 @@ verifyMaxDescriptorLength: send_descriptor: - sbi _SFR_IO_ADDR(LED_PORT), LED_BIT ; Trun on LED before exiting (non-promicro) - ;cbi _SFR_IO_ADDR(LED_PORT), LED_BIT ; Trun on LED before exiting (promicro) - ; Abort if RXSTPI is set ldd r17, Y+oUEINTX ; Load r17 with the value in the USB Endpoint Interrupt Register (UEINTX); sbrc r17, RXSTPI ; Skip the next instruction if the Received SETUP Interrupt Flag (RXSTPI) is cleared From c82b526f89903cea073935f6d577eeaabfa049b6 Mon Sep 17 00:00:00 2001 From: Rodrigo Torres Date: Mon, 24 Jan 2022 09:43:19 -0800 Subject: [PATCH 2/3] Address feedback from Osamu Moves "TURN_LED_ON" within "SET_CONFIGURATION" so LED is turned on ONLY when device has gone through enumeration. Adds check for "LED_ACTIVE_LEVEL" to turn LED off at the beginning of "run_bootloader" if LED is active low. This has a 2-byte penalty which makes nanoBoot go beyond 512 bytes on ProMicro-compatible boards. --- nanoBoot.S | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/nanoBoot.S b/nanoBoot.S index 506353c..d29097f 100644 --- a/nanoBoot.S +++ b/nanoBoot.S @@ -53,7 +53,8 @@ ; LED SUPPORT START ; Turn LED on while bootloader is active -; NOTE: This feature uses 6 bytes for active high and 8 bytes for active low +; NOTE: This feature uses 6 bytes for active high and 8 bytes for active low; +; see "Enable LED" in the "run_bootloader" section for details. ; Uncomment the following line to enable LED feature ; #define LED_ENABLED @@ -398,11 +399,16 @@ run_application: ; We get here if the cause of th run_bootloader: set ; Initialize BootLoaderActive flag (T flag in SREG) +; Enable LED #if defined(LED_ENABLED) ; Set IO register as output for LED ENABLE_LED_OUTPUT - ; Turn LED on while bootloader is active - TURN_LED_ON + + ; If the LED is active low, we need to turn it off here (set LED_BIT) since the MCU IO port is + ; initialized as 0. This is a 2-byte penalty when using active low LED. + #if LED_ACTIVE_LEVEL == 0 + TURN_LED_OFF + #endif #endif @@ -765,6 +771,14 @@ HANDLE_USB_STANDARD_DEVICE: ; fallthrough to SET_CONFIGURATION if equal SET_CONFIGURATION: +#if defined(LED_ENABLED) + ; Turn LED on towards the end of enumeration (SET_CONFIGURATION is done after SET_ADDRESS) + ; TODO: If we ever have space, we could add a flag here to mark the fact that we have entered + ; this state, and turn the LED on at the end of the setup request. For now this is the best we + ; can do. + TURN_LED_ON +#endif + ; Optimization by "sigprof" that saves 2 bytes ; Dirty trick: We don't need to do anything for SET_CONFIGURATION except process_Host2Device, ; so we reuse the SET_ADDRESS code by making it reload the same value to UDADDR. From 00bf03f1d46e20080704e740541654c28bf67d9e Mon Sep 17 00:00:00 2001 From: Sergey Vlasov Date: Fri, 20 Aug 2021 21:59:54 +0300 Subject: [PATCH 3/3] Save 2 bytes in the watchdog handling code `set_watchdog_timer` was called in two places, but the parameter value passed in r16 (the value used to unlock the watchdog configuration) was the same in both cases. Move the duplicated initialization of r16 into the `set_watchdog_timer` function itself, saving 2 bytes. --- nanoBoot.S | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/nanoBoot.S b/nanoBoot.S index d29097f..13dc630 100644 --- a/nanoBoot.S +++ b/nanoBoot.S @@ -383,12 +383,9 @@ main: ; We MUST disable the Watchdog Timer first, otherwise it will remain enabled and will keep resetting the system, so... ; Disable Watchdog Timer - ldi r16, _BV(WDCE) | _BV(WDE) ; Load r16 with the value needed to "unlock" the Watchdog Timer Configuration - ; Write a logic one to the Watchdog Change Enable bit (WDCE) and Watchdog System Reset Enable (WDE) - mov r17, rZERO ; Load r17 with zero to disable the Watchdog Timer completely - rcall set_watchdog_timer ; Call the subroutine that sets the watchdog timer with the values loaded in r16 and r17 + rcall set_watchdog_timer ; Call the subroutine that sets the watchdog timer with the value loaded in r17 ; check_reset_flags: sbrs rMCUSR, EXTRF ; Skip the next instruction if EXTRF is set (if External Reset Flag, skip next instruction, go to run_bootloader) @@ -613,13 +610,10 @@ exit_bootloader: ; NOTE!! This part of the code assumes MCUSR has already been cleared ; Enable WDT, ~250 ms timeout (force a timeout to reset the AVR) - ldi r16, _BV(WDCE) | _BV(WDE) ; Load r16 with the value needed to "unlock" the Watchdog Timer Configuration - ; Write a logic one to the Watchdog Change Enable bit (WDCE) and Watchdog System Reset Enable (WDE) - ldi r17, _BV(WDE) | _BV(WDP2) ; Load r17 with the value needed to set the desired Watchdog Configuration (WDCE = 0, not set!) ; Write the WDE and Watchdog prescaler bits (WDP); System Reset Mode (WDE = 1) and ~250 ms timeout (WDP2 = 1) - rcall set_watchdog_timer ; Call the subroutine that sets the wathdog timer with the values loaded in r16 and r17 + rcall set_watchdog_timer ; Call the subroutine that sets the watchdog timer with the value loaded in r17 ; for (;;); final_loop: @@ -1077,8 +1071,11 @@ EP_ISR_END: set_watchdog_timer: - ; IMPORTANT!! This function assumes the correct values for the WDTCSR register - ; configuration are already loaded onto r16 and 17. + ; IMPORTANT!! This function assumes the correct value for the WDTCSR register + ; configuration is already loaded onto r17; it also modifies r16. + + ldi r16, _BV(WDCE) | _BV(WDE) ; Load r16 with the value needed to "unlock" the Watchdog Timer Configuration + ; Write a logic one to the Watchdog Change Enable bit (WDCE) and Watchdog System Reset Enable (WDE) wdr ; Reset the Watchdog Timer