From 9beeb23c894e0a78fad78bf1ed685cdbc6acc79e Mon Sep 17 00:00:00 2001 From: George Norton Date: Tue, 6 Jun 2023 00:13:12 +0100 Subject: [PATCH] Fixes and minor features. --- firmware/code/configuration_manager.c | 119 ++++++++++++++++++++------ firmware/code/configuration_types.h | 3 + firmware/code/run.c | 52 +++++------ firmware/code/run.h | 17 +++- 4 files changed, 139 insertions(+), 52 deletions(-) diff --git a/firmware/code/configuration_manager.c b/firmware/code/configuration_manager.c index f723721..57b6a18 100644 --- a/firmware/code/configuration_manager.c +++ b/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.header.type = FLASH_HEADER; - flash_header.header.length = sizeof(flash_header) + config_length; - flash_header.magic = FLASH_MAGIC; - flash_header.version = CONFIG_VERSION; - flash_range_program(USER_CONFIGURATION_OFFSET, (const uint8_t *) &flash_header, sizeof(flash_header)); - flash_range_program(USER_CONFIGURATION_OFFSET + sizeof(flash_header), config->value, config_length); + flash_header_tlv* flash_header = (flash_header_tlv*) flash_buffer; + flash_header->header.type = FLASH_HEADER; + flash_header->header.length = sizeof(flash_header) + config_length; + flash_header->magic = FLASH_MAGIC; + flash_header->version = CONFIG_VERSION; + memcpy((void*)(flash_header->tlvs), 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); - ((tlv_header*) working_configuration[inactive_working_configuration])->length = 0; + inactive_working_configuration = inactive_working_configuration ? 0 : 1; 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; - - if (process_cmd((tlv_header*) working_configuration[inactive_working_configuration])) { - result->type = OK; - result->length = 4; - } - else { - result->type = NOK; - result->length = 4; - } + process_cmd((tlv_header*) working_configuration[inactive_working_configuration]); } 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() { diff --git a/firmware/code/configuration_types.h b/firmware/code/configuration_types.h index d1c6438..e58961a 100644 --- a/firmware/code/configuration_types.h +++ b/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 { diff --git a/firmware/code/run.c b/firmware/code/run.c index 38226e2..5cb48b1 100644 --- a/firmware/code/run.c +++ b/firmware/code/run.c @@ -49,18 +49,7 @@ i2s_obj_t i2s_write_obj; static uint8_t *userbuf; -static struct { - 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 = { +audio_state_config 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. ****************************************************************************/ diff --git a/firmware/code/run.h b/firmware/code/run.h index 22a2a49..6fb0a7e 100644 --- a/firmware/code/run.h +++ b/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 \ No newline at end of file