OctoClock bugfixes

* Bumped compatibility version to 3
* firmware: Ethernet, clkdist bugfixes
* lib: fixed invalid rev detection

2 files changed, 101 insertions, 42 deletions

diff --git a/firmware/octoclock/bootloader/CMakeLists.txt b/firmware/octoclock/bootloader/CMakeLists.txt
index 04bcfc492..7623a8698 100644
--- a/firmware/octoclock/bootloader/CMakeLists.txt
+++ b/firmware/octoclock/bootloader/CMakeLists.txt
@@ -49,7 +49,7 @@ add_custom_target(
 
 add_custom_target(
     upload_bootloader
-    ${AVRDUDE} -p atmega128 -c ${PROGRAMMER} -P usb -U efuse:w:0xFF:m -U hfuse:w:0x80:m -U lfuse:w:0xFF:m -U flash:w:octoclock_bootloader.hex:i
+    ${AVRDUDE} -p atmega128 -c ${PROGRAMMER} -P usb -U efuse:w:0xFF:m -U hfuse:w:0x80:m -U lfuse:w:0xEF:m -U flash:w:octoclock_bootloader.hex:i
     WORKING_DIRECTORY ${CMAKE_BINARY_DIR}
     DEPENDS octoclock_bootloader_hex
     COMMENT "Uploading OctoClock bootloader to device with ${PROGRAMMER}"
diff --git a/firmware/octoclock/bootloader/main.c b/firmware/octoclock/bootloader/main.c
index 5e2e6f17e..09f740d31 100644
--- a/firmware/octoclock/bootloader/main.c
+++ b/firmware/octoclock/bootloader/main.c
@@ -23,13 +23,13 @@
 #include <avr/eeprom.h>
 #include <avr/io.h>
 #include <avr/pgmspace.h>
+#include <avr/wdt.h>
 
+#include <avrlibdefs.h>
 #include <octoclock.h>
 #include <debug.h>
 #include <network.h>
 
-#include <util/delay.h>
-
 #include <net/enc28j60.h>
 
 #include "octoclock/common.h"
@@ -40,10 +40,19 @@
  */
 #define TIME_PASSED (TCNT1 > (TIMER1_ONE_SECOND*5) || (TIFR & _BV(TOV1)))
 
-//States
-static bool received_cmd = false;
-static bool done_burning = false;
+/*
+ * States
+ */
+static bool received_cmd = false; // Received "PREPARE_FW_BURN_CMD" signal
+static bool done_burning = false; // Received "FINALIZE_BURNING_CMD" signal
+static bool app_checked  = false; // Ran validation check on firmware
 
+/*
+ * After new firmware is burned onto the device, the bootloader calculates its
+ * CRC and burns it into the EEPROM. When the device boots, this CRC is used
+ * to validate the firmware before loading it. This struct represents how the
+ * information is stored in the EEPROM.
+ */
 typedef struct {
     uint16_t fw_len;
     uint16_t fw_crc;
@@ -51,15 +60,22 @@ typedef struct {
 
 static crc_info_t crc_info;
 
+/*
+ * What actually burns the firmware onto the device.
+ *
+ * Source: http://www.atmel.com/webdoc/AVRLibcReferenceManual/group__avr__boot.html
+ */
 static void boot_program_page(uint8_t *buf, uint16_t page){
-    uint16_t i;
+    // Disable interrupts
+    uint8_t sreg = SREG;
+    cli();
 
     eeprom_busy_wait();
 
     boot_page_erase(page);
     boot_spm_busy_wait(); // Wait until the memory is erased.
 
-    for(i = 0; i < SPM_PAGESIZE; i += 2){
+    for(uint16_t i = 0; i < SPM_PAGESIZE; i += 2){
         // Set up little-endian word.
         uint16_t w = *buf++;
         w += ((*buf++) << 8);
@@ -68,36 +84,31 @@ static void boot_program_page(uint8_t *buf, uint16_t page){
     }
 
     boot_page_write(page); // Store buffer in flash page.
-    boot_spm_busy_wait(); // Wait until the memory is written.
+    boot_spm_busy_wait();  // Wait until the memory is written.
 
     // Reenable RWW-section again. We need this if we want to jump back
     // to the application after bootloading.
     boot_rww_enable();
+
+    // Restore interrupt state
+    SREG = sreg;
+    sei();
 }
 
+/*
+ * Load firmware at given address into packet to send to host.
+ */
 static void read_firmware(uint16_t addr, octoclock_packet_t *pkt_out){
     for(size_t i = 0; i < SPM_PAGESIZE; i++){
         pkt_out->data[i] = pgm_read_byte(addr+i);
     }
 }
 
-void handle_udp_query_packet(
-    struct socket_address src, struct socket_address dst,
-    unsigned char *payload, int payload_len
-){
-    const octoclock_packet_t *pkt_in = (octoclock_packet_t*)payload;
-
-    //Respond to query packets
-    if(pkt_in->code == OCTOCLOCK_QUERY_CMD){
-        octoclock_packet_t pkt_out;
-        pkt_out.proto_ver = OCTOCLOCK_BOOTLOADER_PROTO_VER;
-        pkt_out.sequence = pkt_in->sequence;
-        pkt_out.code = OCTOCLOCK_QUERY_ACK;
-        pkt_out.len = 0;
-        send_udp_pkt(OCTOCLOCK_UDP_CTRL_PORT, src, (void*)&pkt_out, sizeof(octoclock_packet_t));
-    }
-}
-
+/*
+ * Calculate the CRC of the current firmware.
+ *
+ * Adapted from _crc16_update in <util/crc16.h>.
+ */
 static void calculate_crc(uint16_t *crc, uint16_t len){
     *crc = 0xFFFF;
 
@@ -110,6 +121,11 @@ static void calculate_crc(uint16_t *crc, uint16_t len){
     }
 }
 
+/*
+ * Calculate the CRC of the current firmware. If it matches the
+ * CRC burned into the EEPROM, the firmware is considered valid,
+ * and the bootloader can load it.
+ */
 static bool valid_app(){
     crc_info_t crc_eeprom_info;
     eeprom_read_block(&crc_eeprom_info, (void*)OCTOCLOCK_EEPROM_APP_LEN, 4);
@@ -118,6 +134,26 @@ static bool valid_app(){
     return (crc_info.fw_crc == crc_eeprom_info.fw_crc);
 }
 
+/*
+ * UDP handlers
+ */
+void handle_udp_query_packet(
+    struct socket_address src, struct socket_address dst,
+    unsigned char *payload, int payload_len
+){
+    const octoclock_packet_t *pkt_in = (octoclock_packet_t*)payload;
+
+    // Respond to uhd::device::find(), identify as bootloader
+    if(pkt_in->code == OCTOCLOCK_QUERY_CMD){
+        octoclock_packet_t pkt_out;
+        pkt_out.proto_ver = OCTOCLOCK_BOOTLOADER_PROTO_VER;
+        pkt_out.sequence = pkt_in->sequence;
+        pkt_out.code = OCTOCLOCK_QUERY_ACK;
+        pkt_out.len = 0;
+        send_udp_pkt(OCTOCLOCK_UDP_CTRL_PORT, src, (void*)&pkt_out, sizeof(octoclock_packet_t));
+    }
+}
+
 void handle_udp_fw_packet(
     struct socket_address src, struct socket_address dst,
     unsigned char *payload, int payload_len
@@ -132,24 +168,27 @@ void handle_udp_fw_packet(
         case PREPARE_FW_BURN_CMD:
             received_cmd = true;
             done_burning = false;
+            crc_info.fw_crc = pkt_in->crc;
             crc_info.fw_len = pkt_in->len;
             pkt_out.code = FW_BURN_READY_ACK;
             break;
 
+        // Burn firmware sent from the host
         case FILE_TRANSFER_CMD:
             boot_program_page(pkt_in->data, pkt_in->addr);
             pkt_out.code = FILE_TRANSFER_ACK;
+            pkt_out.addr = pkt_in->addr;
             break;
 
+        // Send firmware back to the host for verification
         case READ_FW_CMD:
             pkt_out.code = READ_FW_ACK;
             read_firmware(pkt_in->addr, &pkt_out);
             break;
 
+        // Calculate the CRC of the new firmware and finish
         case FINALIZE_BURNING_CMD:
-            //With stuff verified, burn CRC info into EEPROM
             done_burning = true;
-            calculate_crc(&(crc_info.fw_crc), crc_info.fw_len);
             eeprom_write_block(&crc_info, (void*)OCTOCLOCK_EEPROM_APP_LEN, 4);
             pkt_out.code = FINALIZE_BURNING_ACK;
             break;
@@ -170,11 +209,12 @@ void handle_udp_eeprom_packet(
     pkt_out.sequence = pkt_in->sequence;
     pkt_out.len = 0;
 
+    // Restore OctoClock's EEPROM to factory state
     if(pkt_in->proto_ver == OCTOCLOCK_FW_COMPAT_NUM){
         switch(pkt_in->code){
             case CLEAR_EEPROM_CMD:
                 received_cmd = true;
-                uint8_t blank_eeprom[103];
+                uint8_t blank_eeprom[103]; // 103 is offset of CRC info
                 memset(blank_eeprom, 0xFF, 103);
                 eeprom_write_block(blank_eeprom, 0, 103);
                 pkt_out.code = CLEAR_EEPROM_ACK;
@@ -189,28 +229,45 @@ void handle_udp_eeprom_packet(
 
 int main(void){
 
-    asm("cli");
+    // Disable watchdog timer
+    wdt_disable();
+
+    // Give interrupts to bootloader
+    MCUCR = (1<<IVCE);
+    MCUCR = (1<<IVSEL);
+    cli();
 
-    //Initialization
+    // Atmega128
     setup_atmel_io_ports();
+
+    // Start timer
+    TIMER1_INIT();
+
+    // Ethernet stack
     network_init();
-    init_udp_listeners();
     register_udp_listener(OCTOCLOCK_UDP_CTRL_PORT, handle_udp_query_packet);
     register_udp_listener(OCTOCLOCK_UDP_FW_PORT, handle_udp_fw_packet);
     register_udp_listener(OCTOCLOCK_UDP_EEPROM_PORT, handle_udp_eeprom_packet);
 
-    //Turn LED's on to show we're in the bootloader
+    // Turn LED's on to show we're in the bootloader
     PORTC |= 0x20;
     PORTC |= (0x20<<1);
     PORTC |= (0x20<<2);
 
-    TIMER1_INIT();
-    bool app_checked = false;
-
+    /*
+     * This loop determines whether the OctoClock will remain in its bootloader
+     * state or if it will load the main firmware. After five seconds, it will
+     * check to see if valid firmware is installed. If so, it will immediately
+     * load it. Otherwise, it will remain here until firmware is installed.
+     *
+     * This process can be stopped by an instruction from the firmware burner
+     * utility, at which point the OctoClock will remain in bootloader state until
+     * instructed by the utility to exit the loop and load the new firmware.
+     */
     while(true){
         if(done_burning){
             if(valid_app()) break;
-            else done_burning = false; //Burning somehow failed and wasn't caught
+            else done_burning = false; // Burning somehow failed and wasn't caught
         }
         if(!app_checked && !received_cmd && TIME_PASSED){
             app_checked = true;
@@ -220,16 +277,18 @@ int main(void){
         network_check();
     }
 
-    //Turn LED's off before moving to application
+    // Turn LED's off before moving to application
     PORTC &= ~0x20;
     PORTC &= ~(0x20<<1);
     PORTC &= ~(0x20<<2);
 
     /*
-     * Whether the bootloader reaches here through five seconds of inactivity
-     * or after a firmware burn just finished, it can be assumed that the application
-     * is valid.
+     * At this point, the bootloader has determined that there is valid
+     * firmware installed on the device and that it is OK to load it.
      */
+    TIMER1_DISABLE();
+    MCUCR = (1<<IVCE);
+    MCUCR = 0;
+    cli();
     asm("jmp 0000");
-    return 0; //Will never get here, but AVR-GCC needs it
 }