From a11edd8850298180fd796c33bcb0518c2e90f08e Mon Sep 17 00:00:00 2001 From: Gerardo Rodriguez Date: Mon, 16 Aug 2021 13:15:55 -0500 Subject: [PATCH 1/5] #114 Add busy error code to I2C transactions when the response hasn't been attended Uprev I2C protocl version --- source/board/microbitv2/i2c_commands.c | 11 ++++++++++- source/board/microbitv2/i2c_commands.h | 3 ++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/source/board/microbitv2/i2c_commands.c b/source/board/microbitv2/i2c_commands.c index 521be6afa..d96415068 100644 --- a/source/board/microbitv2/i2c_commands.c +++ b/source/board/microbitv2/i2c_commands.c @@ -73,6 +73,8 @@ static void i2c_write_flash_callback(uint8_t* pData, uint8_t size); static void i2c_read_flash_callback(uint8_t* pData, uint8_t size); static void i2c_slave_callback(I2C_Type *base, i2c_slave_transfer_t *xfer, void *userData) { + i2cCommand_t i2cResponse = {0}; + switch (xfer->event) { /* Address match event */ @@ -101,6 +103,13 @@ static void i2c_slave_callback(I2C_Type *base, i2c_slave_transfer_t *xfer, void xfer->data = g_slave_RX_buff; xfer->dataSize = I2C_DATA_LENGTH; g_SlaveRxFlag = true; + + // Clear transmit Buffer in IRQ. It will be filled when the task attends the I2C event + i2c_clearBuffer(); + // Add busy error code + i2cResponse.cmdId = gErrorResponse_c; + i2cResponse.cmdData.errorRspCmd.errorCode = gErrorBusy_c; + i2c_fillBuffer((uint8_t*) &i2cResponse, 0, sizeof(i2cResponse)); break; /* Transfer done */ @@ -276,7 +285,7 @@ static void i2c_write_comms_callback(uint8_t* pData, uint8_t size) { memcpy(&i2cResponse.cmdData.readRspCmd.data, &board_id_hex, sizeof(board_id_hex)); break; case gI2CProtocolVersion_c: { - uint16_t i2c_version = 1; + uint16_t i2c_version = 2; i2cResponse.cmdData.readRspCmd.dataSize = sizeof(i2c_version); memcpy(&i2cResponse.cmdData.readRspCmd.data, &i2c_version, sizeof(i2c_version)); } diff --git a/source/board/microbitv2/i2c_commands.h b/source/board/microbitv2/i2c_commands.h index f18436210..688e48a72 100644 --- a/source/board/microbitv2/i2c_commands.h +++ b/source/board/microbitv2/i2c_commands.h @@ -77,7 +77,8 @@ typedef enum errorCode_tag { gErrorWrongPropertySize_c = 0x35, gErrorReadDisallowed_c = 0x36, gErrorWriteDisallowed_c = 0x37, - gErrorWriteFail_c = 0x38 + gErrorWriteFail_c = 0x38, + gErrorBusy_c = 0x39 } errorCode_t; typedef __PACKED_STRUCT readReqCmd_tag { From 096519dbccc60d6ca86c60dd8dede8cda195b18d Mon Sep 17 00:00:00 2001 From: Gerardo Rodriguez Date: Mon, 16 Aug 2021 13:23:29 -0500 Subject: [PATCH 2/5] #112 Add I2C Encoding window command for filesystem parser workaround --- source/board/microbitv2/i2c_commands.c | 40 ++++++++++++++++++++++++++ source/board/microbitv2/i2c_commands.h | 3 ++ source/board/microbitv2/microbitv2.c | 32 +++++++++++++++++++-- 3 files changed, 72 insertions(+), 3 deletions(-) diff --git a/source/board/microbitv2/i2c_commands.c b/source/board/microbitv2/i2c_commands.c index d96415068..05f1c922f 100644 --- a/source/board/microbitv2/i2c_commands.c +++ b/source/board/microbitv2/i2c_commands.c @@ -615,6 +615,44 @@ static void i2c_write_flash_callback(uint8_t* pData, uint8_t size) { i2c_fillBuffer((uint8_t*) pI2cCommand, 0, 1); } break; + case gFlashCfgEncWindow_c: + if (size == 1) { + /* If size is 1 (only cmd id), this means it's a read */ + uint32_t tempFileEncWindowStart = __REV(gflashConfig.fileEncWindowStart); + uint32_t tempFileEncWindowEnd = __REV(gflashConfig.fileEncWindowEnd); + i2c_fillBuffer((uint8_t*) pI2cCommand, 0, 1); + i2c_fillBuffer((uint8_t*) &tempFileEncWindowStart, 1, sizeof(gflashConfig.fileEncWindowStart)); + i2c_fillBuffer((uint8_t*) &tempFileEncWindowEnd, 5, sizeof(gflashConfig.fileEncWindowEnd)); + } else if (size == 9) { + /* If size is 9 (cmd id + 8B data), this means it's a write */ + uint32_t tempFileEncWindowStart = pI2cCommand->cmdData.data[0] << 24 | + pI2cCommand->cmdData.data[1] << 16 | + pI2cCommand->cmdData.data[2] << 8 | + pI2cCommand->cmdData.data[3] << 0; + uint32_t tempFileEncWindowEnd = pI2cCommand->cmdData.data[4] << 24 | + pI2cCommand->cmdData.data[5] << 16 | + pI2cCommand->cmdData.data[6] << 8 | + pI2cCommand->cmdData.data[7] << 0; + + /* Validate encoding window */ + if (tempFileEncWindowStart <= tempFileEncWindowEnd) { + gflashConfig.fileEncWindowStart = tempFileEncWindowStart; + tempFileEncWindowStart = __REV(gflashConfig.fileEncWindowStart); + gflashConfig.fileEncWindowEnd = tempFileEncWindowEnd; + tempFileEncWindowEnd = __REV(gflashConfig.fileEncWindowEnd); + + i2c_fillBuffer((uint8_t*) pI2cCommand, 0, 1); + i2c_fillBuffer((uint8_t*) &tempFileEncWindowStart, 1, sizeof(gflashConfig.fileEncWindowStart)); + i2c_fillBuffer((uint8_t*) &tempFileEncWindowEnd, 5, sizeof(gflashConfig.fileEncWindowEnd)); + } else { + pI2cCommand->cmdId = gFlashError_c; + i2c_fillBuffer((uint8_t*) pI2cCommand, 0, 1); + } + } else { + pI2cCommand->cmdId = gFlashError_c; + i2c_fillBuffer((uint8_t*) pI2cCommand, 0, 1); + } + break; case gFlashCfgFileVisible_c: if (size == 1) { /* If size is 1 (only cmd id), this means it's a read */ @@ -658,6 +696,8 @@ static void i2c_write_flash_callback(uint8_t* pData, uint8_t size) { memcpy(gflashConfig.fileName, FLASH_CFG_FILENAME, 11); gflashConfig.fileSize = FLASH_CFG_FILESIZE; gflashConfig.fileVisible = FLASH_CFG_FILEVISIBLE; + gflashConfig.fileEncWindowStart = 0; + gflashConfig.fileEncWindowEnd = 0; } i2c_fillBuffer((uint8_t*) pI2cCommand, 0, 1); break; diff --git a/source/board/microbitv2/i2c_commands.h b/source/board/microbitv2/i2c_commands.h index 688e48a72..5f975cca6 100644 --- a/source/board/microbitv2/i2c_commands.h +++ b/source/board/microbitv2/i2c_commands.h @@ -124,6 +124,8 @@ typedef __PACKED_STRUCT flashConfig_tag { vfs_filename_t fileName; uint32_t fileSize; bool fileVisible; + uint32_t fileEncWindowStart; + uint32_t fileEncWindowEnd; } flashConfig_t; /*! Flash interface command type */ @@ -136,6 +138,7 @@ typedef enum flashCmdId_tag { gFlashStorageSize_c = 0x06, gFlashSectorSize_c = 0x07, gFlashRemountMSD_c = 0x08, + gFlashCfgEncWindow_c = 0x09, gFlashDataRead_c = 0x0A, gFlashDataWrite_c = 0x0B, gFlashDataErase_c = 0x0C, diff --git a/source/board/microbitv2/microbitv2.c b/source/board/microbitv2/microbitv2.c index 8d9148d5d..fb15d9074 100644 --- a/source/board/microbitv2/microbitv2.c +++ b/source/board/microbitv2/microbitv2.c @@ -71,6 +71,8 @@ flashConfig_t gflashConfig = { .fileName = FLASH_CFG_FILENAME, .fileSize = FLASH_CFG_FILESIZE, .fileVisible = FLASH_CFG_FILEVISIBLE, + .fileEncWindowStart = 0, + .fileEncWindowEnd = 0, }; typedef enum { @@ -528,7 +530,8 @@ void vfs_user_build_filesystem_hook() { } } - file_size = gflashConfig.fileSize; + // Add encoding window file size. 1B encoded into 2B ASCII + file_size = gflashConfig.fileSize + (gflashConfig.fileEncWindowEnd - gflashConfig.fileEncWindowStart); if (gflashConfig.fileVisible) { vfs_create_file(gflashConfig.fileName, read_file_data_txt, 0, file_size); @@ -538,9 +541,32 @@ void vfs_user_build_filesystem_hook() { // File callback to be used with vfs_add_file to return file contents static uint32_t read_file_data_txt(uint32_t sector_offset, uint8_t *data, uint32_t num_sectors) { + uint32_t read_address = FLASH_STORAGE_ADDRESS + (VFS_SECTOR_SIZE * sector_offset); + uint32_t encoded_data_offset = (gflashConfig.fileEncWindowEnd - gflashConfig.fileEncWindowStart); + // Ignore out of bound reads - if ( (FLASH_STORAGE_ADDRESS + VFS_SECTOR_SIZE * sector_offset) < (FLASH_CONFIG_ADDRESS + FLASH_INTERFACE_SIZE) ) { - memcpy(data, (uint8_t *) (FLASH_STORAGE_ADDRESS + VFS_SECTOR_SIZE * sector_offset), VFS_SECTOR_SIZE); + if ( read_address < (FLASH_CONFIG_ADDRESS + FLASH_INTERFACE_SIZE + encoded_data_offset) ) { + for (uint32_t i = 0; i < VFS_SECTOR_SIZE; i++) { + if (i + (VFS_SECTOR_SIZE * sector_offset) < gflashConfig.fileEncWindowStart) { + // If data is before encoding window, no offset is needed + data[i] = *(uint8_t *) (read_address + i); + } else if(i + (VFS_SECTOR_SIZE * sector_offset) < (gflashConfig.fileEncWindowStart + encoded_data_offset * 2)) { + // Data inside encoding window needs to consider encoding window start and size + uint8_t enc_byte = *(uint8_t *) (FLASH_STORAGE_ADDRESS + ((VFS_SECTOR_SIZE * sector_offset) + gflashConfig.fileEncWindowStart + i ) / 2); + if (i % 2 == 0) { + // High nibble + enc_byte = 0x0F & (enc_byte >> 4); + } else { + // Low nibble + enc_byte = 0x0F & enc_byte; + } + // Encode one nibble to one ASCII byte + data[i] = enc_byte <= 9 ? enc_byte + 0x30 : enc_byte + 0x37; + } else { + // If data is after encoding window, adjustment is needed + data[i] = *(uint8_t *) (read_address + i - encoded_data_offset); + } + } } return VFS_SECTOR_SIZE; From b7e6a0eb44896c211d44cdbcf89135e4be21ffe8 Mon Sep 17 00:00:00 2001 From: Gerardo Rodriguez Date: Wed, 8 Sep 2021 17:48:23 -0500 Subject: [PATCH 3/5] #114 Fix I2C busy error code -Ignore NOP cmd when clearing TX buffer and adding busy error code --- source/board/microbitv2/i2c_commands.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/source/board/microbitv2/i2c_commands.c b/source/board/microbitv2/i2c_commands.c index 05f1c922f..1dc488bca 100644 --- a/source/board/microbitv2/i2c_commands.c +++ b/source/board/microbitv2/i2c_commands.c @@ -104,12 +104,6 @@ static void i2c_slave_callback(I2C_Type *base, i2c_slave_transfer_t *xfer, void xfer->dataSize = I2C_DATA_LENGTH; g_SlaveRxFlag = true; - // Clear transmit Buffer in IRQ. It will be filled when the task attends the I2C event - i2c_clearBuffer(); - // Add busy error code - i2cResponse.cmdId = gErrorResponse_c; - i2cResponse.cmdData.errorRspCmd.errorCode = gErrorBusy_c; - i2c_fillBuffer((uint8_t*) &i2cResponse, 0, sizeof(i2cResponse)); break; /* Transfer done */ @@ -124,6 +118,12 @@ static void i2c_slave_callback(I2C_Type *base, i2c_slave_transfer_t *xfer, void // Ignore NOP cmd in I2C Write if (!(g_SlaveRxFlag && g_slave_RX_buff[0] == 0x00)) { + // Clear transmit Buffer in IRQ. It will be filled when the task attends the I2C event + i2c_clearBuffer(); + // Add busy error code + i2cResponse.cmdId = gErrorResponse_c; + i2cResponse.cmdData.errorRspCmd.errorCode = gErrorBusy_c; + i2c_fillBuffer((uint8_t*) &i2cResponse, 0, sizeof(i2cResponse)); main_board_event(); } From 8ee56483d01d9e344ad6d74e2c5206b256fa98d9 Mon Sep 17 00:00:00 2001 From: Gerardo Rodriguez Date: Wed, 6 Oct 2021 00:37:11 -0500 Subject: [PATCH 4/5] I2C data logging fixes. -Don't clear I2C buffer at the start of flash command handling. -Keep interrupt signal asserted when busy error was read. -Don't clear I2C TX Buffer after reads. Allowing multiple reads of last result. --- source/board/microbitv2/i2c_commands.c | 39 +++++++++++++++++--------- 1 file changed, 25 insertions(+), 14 deletions(-) diff --git a/source/board/microbitv2/i2c_commands.c b/source/board/microbitv2/i2c_commands.c index 1dc488bca..f7c37449a 100644 --- a/source/board/microbitv2/i2c_commands.c +++ b/source/board/microbitv2/i2c_commands.c @@ -47,6 +47,9 @@ static volatile bool g_SlaveCompletionFlag = false; static volatile bool g_SlaveRxFlag = false; static uint8_t address_match = 0; static uint32_t transferredCount = 0; +static const uint8_t g_slave_busy_error_buff[] = {gErrorResponse_c, gErrorBusy_c}; +static uint8_t g_slave_TX_buff_ready = 0; +static uint8_t g_slave_busy = 0; static i2cWriteCallback_t pfWriteCommsCallback = NULL; static i2cReadCallback_t pfReadCommsCallback = NULL; @@ -88,8 +91,15 @@ static void i2c_slave_callback(I2C_Type *base, i2c_slave_transfer_t *xfer, void /* Transmit request */ case kI2C_SlaveTransmitEvent: /* Update information for transmit process */ - xfer->data = g_slave_TX_buff; - xfer->dataSize = I2C_DATA_LENGTH; + // Send response if the buffer is ready, otherwise send busy error + if (g_slave_TX_buff_ready) { + xfer->data = g_slave_TX_buff; + xfer->dataSize = I2C_DATA_LENGTH; + } else { + xfer->data = g_slave_busy_error_buff; + xfer->dataSize = sizeof(g_slave_busy_error_buff); + g_slave_busy = 1; + } g_SlaveRxFlag = false; break; @@ -118,13 +128,19 @@ static void i2c_slave_callback(I2C_Type *base, i2c_slave_transfer_t *xfer, void // Ignore NOP cmd in I2C Write if (!(g_SlaveRxFlag && g_slave_RX_buff[0] == 0x00)) { - // Clear transmit Buffer in IRQ. It will be filled when the task attends the I2C event + // Only process events if the busy error was not read + if (!g_slave_busy) { + main_board_event(); + } else { + g_slave_busy = 0; + } + } + + // Buffer with response is not ready after an I2C Write inside the IRQ + // It will be ready when the task attends the I2C event + if(g_SlaveRxFlag) { + g_slave_TX_buff_ready = 0; i2c_clearBuffer(); - // Add busy error code - i2cResponse.cmdId = gErrorResponse_c; - i2cResponse.cmdData.errorRspCmd.errorCode = gErrorBusy_c; - i2c_fillBuffer((uint8_t*) &i2cResponse, 0, sizeof(i2cResponse)); - main_board_event(); } i2c_allow_sleep = false; @@ -268,6 +284,7 @@ void i2c_fillBuffer (uint8_t* data, uint32_t position, uint32_t size) { for (uint32_t i = 0; i < size; i++) { g_slave_TX_buff[position + i] = data[i]; } + g_slave_TX_buff_ready = 1; } static void i2c_write_comms_callback(uint8_t* pData, uint8_t size) { @@ -422,8 +439,6 @@ static void i2c_read_comms_callback(uint8_t* pData, uint8_t size) { break; } - i2c_clearBuffer(); - // Release COMBINED_SENSOR_INT PORT_SetPinMux(COMBINED_SENSOR_INT_PORT, COMBINED_SENSOR_INT_PIN, kPORT_PinDisabledOrAnalog); } @@ -487,8 +502,6 @@ static void i2c_write_flash_callback(uint8_t* pData, uint8_t size) { uint32_t address = storage_address + FLASH_STORAGE_ADDRESS; uint32_t length = __REV(pI2cCommand->cmdData.write.length); uint32_t data = (uint32_t) pI2cCommand->cmdData.write.data; - - i2c_clearBuffer(); switch (pI2cCommand->cmdId) { case gFlashDataWrite_c: @@ -725,8 +738,6 @@ static void i2c_write_flash_callback(uint8_t* pData, uint8_t size) { } static void i2c_read_flash_callback(uint8_t* pData, uint8_t size) { - i2c_clearBuffer(); - // Release COMBINED_SENSOR_INT PORT_SetPinMux(COMBINED_SENSOR_INT_PORT, COMBINED_SENSOR_INT_PIN, kPORT_PinDisabledOrAnalog); } From 5068e9cb787903fe3264b51e877bbda676bcd455 Mon Sep 17 00:00:00 2001 From: Gerardo Rodriguez Date: Thu, 21 Oct 2021 19:06:13 -0500 Subject: [PATCH 5/5] Serial stalling fixes. -Enable UART HW overrun interrupt. -Reduce time spent in I2C IRQ handler to avoid UART overrun and data loss. --- source/board/microbitv2/i2c_commands.c | 10 ++++++---- source/hic_hal/freescale/kl27z/uart.c | 2 +- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/source/board/microbitv2/i2c_commands.c b/source/board/microbitv2/i2c_commands.c index f7c37449a..933719b69 100644 --- a/source/board/microbitv2/i2c_commands.c +++ b/source/board/microbitv2/i2c_commands.c @@ -106,12 +106,14 @@ static void i2c_slave_callback(I2C_Type *base, i2c_slave_transfer_t *xfer, void /* Receive request */ case kI2C_SlaveReceiveEvent: /* Update information for received process */ + // We don't need to clear g_slave_RX_buff because we also have the + // transferredCount to know what data is valid + xfer->data = g_slave_RX_buff; + xfer->dataSize = I2C_DATA_LENGTH; + // Hack: Default driver can't differentiate between RX or TX on // completion event, so we set a flag here. Can't process more // than I2C_DATA_LENGTH bytes on RX - memset(&g_slave_RX_buff, 0, sizeof(g_slave_RX_buff)); - xfer->data = g_slave_RX_buff; - xfer->dataSize = I2C_DATA_LENGTH; g_SlaveRxFlag = true; break; @@ -140,7 +142,6 @@ static void i2c_slave_callback(I2C_Type *base, i2c_slave_transfer_t *xfer, void // It will be ready when the task attends the I2C event if(g_SlaveRxFlag) { g_slave_TX_buff_ready = 0; - i2c_clearBuffer(); } i2c_allow_sleep = false; @@ -155,6 +156,7 @@ static void i2c_slave_callback(I2C_Type *base, i2c_slave_transfer_t *xfer, void void board_custom_event() { if (g_SlaveRxFlag) { + i2c_clearBuffer(); if (pfWriteCommsCallback && address_match == I2C_SLAVE_NRF_KL_COMMS) { pfWriteCommsCallback(&g_slave_RX_buff[0], transferredCount); } diff --git a/source/hic_hal/freescale/kl27z/uart.c b/source/hic_hal/freescale/kl27z/uart.c index da531e5ad..080d51379 100644 --- a/source/hic_hal/freescale/kl27z/uart.c +++ b/source/hic_hal/freescale/kl27z/uart.c @@ -180,7 +180,7 @@ int32_t uart_set_configuration(UART_Configuration *config) // Enable UART interrupt NVIC_ClearPendingIRQ(UART_RX_TX_IRQn); NVIC_EnableIRQ(UART_RX_TX_IRQn); - UART->CTRL |= LPUART_CTRL_RIE_MASK; + UART->CTRL |= LPUART_CTRL_RIE_MASK | LPUART_CTRL_ORIE_MASK; return 1; }