Fixes and minor features.

firmware/code/configuration_manager.c

@@ -23,9 +23,12 @@
 #include "bqf.h"
 #include "run.h"
 #ifndef TEST_TARGET
+#include "pico/multicore.h"
 #include "pico/stdlib.h"
 #include "pico/usb_device.h"
 #include "hardware/flash.h"
+#include "hardware/sync.h"
+#include "hardware/i2c.h"
 #endif
 
 /**
@@ -59,7 +62,7 @@ static const default_configuration default_config = {
 // Grab the last 4k page of flash for our configuration strutures.
 #ifndef TEST_TARGET
 static const size_t USER_CONFIGURATION_OFFSET = PICO_FLASH_SIZE_BYTES - 0x1000;
-static const uint8_t *user_configuration = (const uint8_t *) (XIP_BASE + USER_CONFIGURATION_OFFSET);
+const uint8_t *user_configuration = (const uint8_t *) (XIP_BASE + USER_CONFIGURATION_OFFSET);
 #endif
 /**
  * TODO: For now, assume we always get a complete configuration but maybe we
@@ -71,6 +74,8 @@ static uint8_t inactive_working_configuration = 0;
 static uint8_t result_buffer[256] = { U16_TO_U8S_LE(NOK), U16_TO_U8S_LE(4) };
 
 static bool reload_config = false;
+static bool save_config = false;
+static bool factory_reset = false;
 static uint16_t write_offset = 0;
 static uint16_t read_offset = 0;
 
@@ -87,7 +92,7 @@ bool validate_filter_configuration(filter_configuration_tlv *filters)
         const uint32_t type = *(uint32_t *)ptr;
         const uint16_t remaining = (uint16_t)(end - ptr);
         if (count++ > MAX_FILTER_STAGES) {
-            printf("Error! Too many filters defined.\n");
+            printf("Error! Too many filters defined. (%d)\n", count);
             return false;
         }
         switch (type) {
@@ -103,6 +108,7 @@ bool validate_filter_configuration(filter_configuration_tlv *filters)
                 printf("Error! Not enough data left for filter2 (%d)..\n", remaining);
                 return false;
             }
+            ptr += sizeof(filter2);
             break;
         }
         case PEAKING:
@@ -167,7 +173,20 @@ bool validate_configuration(tlv_header *config) {
             ptr = (uint8_t *) config->value;
             break;
         case FLASH_HEADER: {
-            ptr = (uint8_t *) ((flash_header_tlv*) config)->tlvs;
+            flash_header_tlv* header = (flash_header_tlv*) config;
+            if (header->magic != FLASH_MAGIC) {
+                printf("Unexpected magic word (%x)\n", header->magic);
+                return false;
+            }
+            if (header->version > CONFIG_VERSION) {
+                printf("Config is too new (%d > %d)\n", header->version, CONFIG_VERSION);
+                return false;
+            }
+            if (header->version < MINIMUM_CONFIG_VERSION) {
+                printf("Config is too old (%d > %d)\n", header->version, MINIMUM_CONFIG_VERSION);
+                return false;
+            }
+            ptr = (uint8_t *) header->tlvs;
             break;
         }
         default:
@@ -194,7 +213,8 @@ bool validate_configuration(tlv_header *config) {
                     printf("Preprocessing size missmatch: %u != %zu\n", tlv->length, sizeof(preprocessing_configuration_tlv));
                     return false;
                 }
-                break;}
+                break;
+            }
             default:
                 // Unknown TLVs are not invalid, just ignored.
                 break;
@@ -243,6 +263,7 @@ bool apply_configuration(tlv_header *config) {
 
 void load_config() {
 #ifndef TEST_TARGET
+    flash_header_tlv* hdr = (flash_header_tlv*) user_configuration;
     // Try to load data from flash
     if (validate_configuration((tlv_header*) user_configuration)) {
         apply_configuration((tlv_header*) user_configuration);
@@ -254,40 +275,84 @@ void load_config() {
 }
 
 #ifndef TEST_TARGET
-bool save_config() {
+// Must be in RAM
+uint8_t flash_buffer[FLASH_PAGE_SIZE];
+bool __no_inline_not_in_flash_func(save_configuration)() {
     const uint8_t active_configuration = inactive_working_configuration ? 0 : 1;
     tlv_header* config = (tlv_header*) working_configuration[active_configuration];
 
     if (validate_configuration(config)) {
         const size_t config_length = config->length - (size_t)((size_t)config->value - (size_t)config);
         // Write data to flash
-        flash_header_tlv flash_header;
+        flash_header_tlv* flash_header = (flash_header_tlv*) flash_buffer;
-        flash_header.header.type = FLASH_HEADER;
+        flash_header->header.type = FLASH_HEADER;
-        flash_header.header.length = sizeof(flash_header) + config_length;
+        flash_header->header.length = sizeof(flash_header) + config_length;
-        flash_header.magic = FLASH_MAGIC;
+        flash_header->magic = FLASH_MAGIC;
-        flash_header.version = CONFIG_VERSION;
+        flash_header->version = CONFIG_VERSION;
-        flash_range_program(USER_CONFIGURATION_OFFSET, (const uint8_t *) &flash_header, sizeof(flash_header));
+        memcpy((void*)(flash_header->tlvs), config->value, config_length);
-        flash_range_program(USER_CONFIGURATION_OFFSET + sizeof(flash_header), config->value, config_length);
+
+        power_down_dac();
+
+        uint32_t ints = save_and_disable_interrupts();
+        flash_range_erase(USER_CONFIGURATION_OFFSET, FLASH_SECTOR_SIZE);
+        flash_range_program(USER_CONFIGURATION_OFFSET, flash_buffer, FLASH_PAGE_SIZE);
+        restore_interrupts(ints);
+
+        power_up_dac();
+
         return true;
     }
     return false;
 }
 
 bool process_cmd(tlv_header* cmd) {
+    tlv_header* result = ((tlv_header*) result_buffer);
     switch (cmd->type) {
         case SET_CONFIGURATION:
             if (validate_configuration(cmd)) {
-                inactive_working_configuration = (inactive_working_configuration ? 0 : 1);
+                inactive_working_configuration = inactive_working_configuration ? 0 : 1;
-                ((tlv_header*) working_configuration[inactive_working_configuration])->length = 0;
                 reload_config = true;
+                result->type = OK;
+                result->length = 4;
                 return true;
             }
+            break;
         case SAVE_CONFIGURATION: {
             if (cmd->length == 4) {
-                return save_config();
+                save_config = true;
-            }
+                result->type = OK;
+                result->length = 4;
+                return true;
+            }
+            break;
+        }
+        case FACTORY_RESET: {
+            if (cmd->length == 4) {
+                factory_reset = true;
+                flash_header_tlv flash_header = { 0 };
+                result->type = OK;
+                result->length = 4;
+                return true;
+            }
+            break;
+        }
+        case GET_VERSION: {
+            if (cmd->length == 4) {
+                result->type = OK;
+                result->length = 4 + sizeof(version_status_tlv);
+                version_status_tlv* version = ((version_status_tlv*) result->value);
+                version->header.type = VERSION_STATUS;
+                version->header.length = sizeof(version_status_tlv);
+                version->current_version = CONFIG_VERSION;
+                version->minimum_supported_version = MINIMUM_CONFIG_VERSION;
+                version->reserved = 0;
+                return true;
+            }
+            break;
         }
     }
+    result->type = NOK;
+    result->length = 4;
     return false;
 }
 
@@ -306,20 +371,10 @@ void config_out_packet(struct usb_endpoint *ep) {
     write_offset += buffer->data_len;
 
     const uint16_t transfer_length = ((tlv_header*) working_configuration[inactive_working_configuration])->length;
-    //printf("config_length %d %d\n", transfer_length, write_offset);
     if (transfer_length && write_offset >= transfer_length) {
         // Command complete, fill the result buffer
-        tlv_header* result = ((tlv_header*) result_buffer);
         write_offset = 0;
-
+        process_cmd((tlv_header*) working_configuration[inactive_working_configuration]);
-        if (process_cmd((tlv_header*) working_configuration[inactive_working_configuration])) {
-            result->type = OK;
-            result->length = 4;
-        }
-        else {
-            result->type = NOK;
-            result->length = 4;
-        }
     }
 
     usb_grow_transfer(ep->current_transfer, 1);
@@ -357,6 +412,18 @@ void config_in_packet(struct usb_endpoint *ep) {
 }
 
 void apply_core0_config() {
+    if (save_config) {
+        save_config = false;
+        save_configuration();
+    }
+    if (factory_reset) {
+        factory_reset = false;
+        power_down_dac();
+        uint32_t ints = save_and_disable_interrupts();
+        flash_range_erase(USER_CONFIGURATION_OFFSET, FLASH_SECTOR_SIZE);
+        restore_interrupts(ints);
+        power_up_dac();
+    }
 }
 
 void apply_core1_config() {

firmware/code/configuration_types.h

@@ -18,6 +18,7 @@
 
 #define FLASH_MAGIC 0x2E8AFEDD
 #define CONFIG_VERSION 1
+#define MINIMUM_CONFIG_VERSION 1
 
 enum structure_types {
     // Commands/Responses, these are container TLVs. The Value will be a set of TLV structures.
@@ -31,6 +32,7 @@ enum structure_types {
     GET_ACTIVE_CONFIGURATION,   // Retrieves the current active configuration TLVs from RAM
     GET_STORED_CONFIGURATION,   // Retrieves the current stored configuration TLVs from Flash
     SAVE_CONFIGURATION,         // Writes the active configuration to Flash
+    FACTORY_RESET,              // Invalidates the flash memory
 
     // Configuration structures, these are returned in the body of a command/response
     PREPROCESSING_CONFIGURATION = 0x200,
@@ -98,6 +100,7 @@ typedef struct __attribute__((__packed__)) _version_status_tlv {
     tlv_header header;
     uint16_t current_version;
     uint16_t minimum_supported_version;
+    uint32_t reserved;
 } version_status_tlv;
 
 typedef struct __attribute__((__packed__)) _default_configuration {

firmware/code/run.c

@@ -49,18 +49,7 @@
 i2s_obj_t i2s_write_obj;
 static uint8_t *userbuf;
 
-static struct {
+audio_state_config audio_state = {
-    uint32_t freq;
-    union {
-        int16_t volume[2];
-        int32_t _volume;
-    };
-    union {
-        int16_t target_volume[2];
-        int32_t _target_volume;
-    };
-    bool mute;
-} audio_state = {
     .freq = 48000,
 };
 
@@ -108,6 +97,9 @@ static void _as_audio_packet(struct usb_endpoint *ep) {
     int32_t *out = (int32_t *) userbuf;
     int samples = usb_buffer->data_len / 2;
 
+    // Update filters if required
+    apply_core0_config();
+
     if (preprocessing.reverse_stereo) {
         for (int i = 0; i < samples; i+=2) {
             out[i] = in[i+1];
@@ -143,9 +135,6 @@ static void _as_audio_packet(struct usb_endpoint *ep) {
     // keep on truckin'
     usb_grow_transfer(ep->current_transfer, 1);
     usb_packet_done(ep);
-
-    // Update filters if required
-    apply_core0_config();
 }
 
 static void update_volume()
@@ -245,7 +234,7 @@ void setup() {
     // The PCM3060 supports standard mode (100kbps) or fast mode (400kbps)
     // we run in fast mode so we dont block the core for too long while
     // updating the volume.
-    i2c_init(i2c0, 400000);
+    i2c_init(i2c0, 50000);
     gpio_set_function(PCM3060_SDA_PIN, GPIO_FUNC_I2C);
     gpio_set_function(PCM3060_SCL_PIN, GPIO_FUNC_I2C);
     gpio_pull_up(PCM3060_SDA_PIN);
@@ -786,13 +775,11 @@ static struct usb_stream_transfer_funcs control_stream_funcs = {
 
 static bool ad_setup_request_handler(__unused struct usb_device *device, struct usb_setup_packet *setup) {
     setup = __builtin_assume_aligned(setup, 4);
-    printf("ad_setup_request_handler: Type %u, Request %u, Value %u, Index %u, Length %u\n", setup->bmRequestType, setup->bRequest, setup->wValue, setup->wIndex, setup->wLength);
+    //("ad_setup_request_handler: Type %u, Request %u, Value %u, Index %u, Length %u\n", setup->bmRequestType, setup->bRequest, setup->wValue, setup->wIndex, setup->wLength);
 
     if (setup->bmRequestType & USB_DIR_IN) {
         if (USB_REQ_TYPE_RECIPIENT_DEVICE == (setup->bmRequestType & USB_REQ_TYPE_TYPE_MASK)) {
-            printf("Device request %x\n", (setup->wValue >> 8));
             if ((setup->bRequest == USB_REQUEST_GET_DESCRIPTOR) && ((setup->wValue >> 8) == 0xF /* BOS */)) {
-                printf("Request BOS descriptor, idx %d\n", setup->wValue & 0xFF);
 
                 struct usb_endpoint *usb_control_in = usb_get_control_in_endpoint();
                 static struct usb_stream_transfer_funcs control_stream_funcs = {
@@ -815,6 +802,7 @@ static bool ad_setup_request_handler(__unused struct usb_device *device, struct
         // To prevent badly behaving software from accidentally triggering a reboot, e expect
         // the wValue to be equal to the Ploopy vendor id.
         if (setup->bRequest == REBOOT_BOOTLOADER && setup->wValue == 0x2E8A) {
+            power_down_dac();
             reset_usb_boot(0, 0);
             // reset_usb_boot does not return, so we will not respond to this command.
             return true;
@@ -822,9 +810,7 @@ static bool ad_setup_request_handler(__unused struct usb_device *device, struct
         else if (USB_REQ_TYPE_RECIPIENT_DEVICE == (setup->bmRequestType & USB_REQ_TYPE_RECIPIENT_MASK) && setup->bRequest == MICROSOFT_COMPATIBLE_ID_FEATURE_DESRIPTOR && setup->wIndex == 0x7)
         {
             const int length = MIN(MS_OS_20_DESC_LEN, setup->wLength);
-
+            //printf("Sending %u bytes (%u %u)\n", length, MS_OS_20_DESC_LEN, sizeof(desc_ms_os_20));
-            printf("Sending %u bytes (%u %u)\n", length, MS_OS_20_DESC_LEN, sizeof(desc_ms_os_20));
-
             struct usb_endpoint *usb_control_in = usb_get_control_in_endpoint();
             usb_stream_setup_transfer(&_control_in_stream_transfer, &control_stream_funcs, desc_ms_os_20,
                             sizeof(desc_ms_os_20), length, _tf_send_control_in_ack);
@@ -841,13 +827,13 @@ static bool ad_setup_request_handler(__unused struct usb_device *device, struct
 static struct usb_stream_transfer _config_in_stream_transfer;
 static bool configuration_interface_setup_request_handler(__unused struct usb_interface *interface, struct usb_setup_packet *setup) {
     setup = __builtin_assume_aligned(setup, 4);
-    printf("configuration_interface_setup_request_handler: Type %u, Request %u, Value %u, Index %u, Length %u\n", setup->bmRequestType, setup->bRequest, setup->wValue, setup->wIndex, setup->wLength);
+    //printf("configuration_interface_setup_request_handler: Type %u, Request %u, Value %u, Index %u, Length %u\n", setup->bmRequestType, setup->bRequest, setup->wValue, setup->wIndex, setup->wLength);
     return false;
 }
 
 static bool ac_setup_request_handler(__unused struct usb_interface *interface, struct usb_setup_packet *setup) {
     setup = __builtin_assume_aligned(setup, 4);
-    printf("ac_setup_request_handler: Type %u, Request %u, Value %u, Index %u, Length %u\n", setup->bmRequestType, setup->bRequest, setup->wValue, setup->wIndex, setup->wLength);
+    //printf("ac_setup_request_handler: Type %u, Request %u, Value %u, Index %u, Length %u\n", setup->bmRequestType, setup->bRequest, setup->wValue, setup->wIndex, setup->wLength);
 
     if (USB_REQ_TYPE_TYPE_CLASS == (setup->bmRequestType & USB_REQ_TYPE_TYPE_MASK)) {
         switch (setup->bRequest) {
@@ -875,7 +861,7 @@ static bool ac_setup_request_handler(__unused struct usb_interface *interface, s
 
 bool _as_setup_request_handler(__unused struct usb_endpoint *ep, struct usb_setup_packet *setup) {
     setup = __builtin_assume_aligned(setup, 4);
-    printf("as_setup_request_handler: Type %u, Request %u, Value %u, Index %u, Length %u\n", setup->bmRequestType, setup->bRequest, setup->wValue, setup->wIndex, setup->wLength);
+    //printf("as_setup_request_handler: Type %u, Request %u, Value %u, Index %u, Length %u\n", setup->bmRequestType, setup->bRequest, setup->wValue, setup->wIndex, setup->wLength);
 
     if (USB_REQ_TYPE_TYPE_CLASS == (setup->bmRequestType & USB_REQ_TYPE_TYPE_MASK)) {
         switch (setup->bRequest) {
@@ -950,6 +936,22 @@ void usb_sound_card_init() {
     usb_device_start();
 }
 
+// Some operations will cause popping on the audio output, temporarily
+// disabling the DAC sounds much better.
+void power_down_dac() {
+    uint8_t buf[2];
+    buf[0] = 64; // register addr
+    buf[1] = 0xF0; // DAC low power mode
+    i2c_write_blocking(i2c0, PCM_I2C_ADDR, buf, 2, false);
+}
+
+void power_up_dac() {
+    uint8_t buf[2];
+    buf[0] = 64; // register addr
+    buf[1] = 0xE0; // DAC normal mode
+    i2c_write_blocking(i2c0, PCM_I2C_ADDR, buf, 2, false);
+}
+
 /*****************************************************************************
  * USB-related code ends here.
  ****************************************************************************/

firmware/code/run.h

@@ -46,6 +46,20 @@
 #define MAX_VOLUME ENCODE_DB(0)
 #define VOLUME_RESOLUTION ENCODE_DB(0.5f)
 
+typedef struct _audio_state_config {
+    uint32_t freq;
+    union {
+        int16_t volume[2];
+        int32_t _volume;
+    };
+    union {
+        int16_t target_volume[2];
+        int32_t _target_volume;
+    };
+    bool mute;
+} audio_state_config;
+extern audio_state_config audio_state;
+
 typedef struct _audio_device_config {
     struct usb_configuration_descriptor descriptor;
     struct usb_interface_descriptor ac_interface;
@@ -144,5 +158,6 @@ static bool do_set_current(struct usb_setup_packet *);
 static bool ac_setup_request_handler(__unused struct usb_interface *, struct usb_setup_packet *);
 bool _as_setup_request_handler(__unused struct usb_endpoint *, struct usb_setup_packet *);
 void usb_sound_card_init(void);
-
+extern void power_down_dac();
+extern void power_up_dac();
 #endif