Skip to content

Commit

Permalink
VA Fixes: USB IRQ Handling and EP configuration, Thread handler shena…
Browse files Browse the repository at this point in the history
…nigans. (#3705)

* FuriHal: properly handle high priority USB IRQ, change CDC decriptor to use separate TX/RX endpoints
* Furi: drop task handle, cleanup casts and memory corrupt in threads
* FuriHal: update max power in USB descriptors
* Furi: properly handle thread free if thread was not started
* Furi crash: meaningful interrupt name instead of id

---------

Co-authored-by: SG <who.just.the.doctor@gmail.com>
  • Loading branch information
skotopes and DrZlo13 committed Jun 12, 2024
1 parent 798e0c3 commit 70d2453
Show file tree
Hide file tree
Showing 10 changed files with 207 additions and 31 deletions.
9 changes: 8 additions & 1 deletion furi/core/check.c
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include <furi_hal_rtc.h>
#include <furi_hal_debug.h>
#include <furi_hal_bt.h>
#include <furi_hal_interrupt.h>
#include <stdio.h>

#include <FreeRTOS.h>
Expand Down Expand Up @@ -110,8 +111,14 @@ static void __furi_print_heap_info(void) {

static void __furi_print_name(bool isr) {
if(isr) {
uint8_t exception_number = __get_IPSR();
const char* name = furi_hal_interrupt_get_name(exception_number);
furi_log_puts("[ISR ");
__furi_put_uint32_as_text(__get_IPSR());
if(name) {
furi_log_puts(name);
} else {
__furi_put_uint32_as_text(__get_IPSR());
}
furi_log_puts("] ");
} else {
const char* name = pcTaskGetName(NULL);
Expand Down
38 changes: 19 additions & 19 deletions furi/core/thread.c
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ struct FuriThreadStdout {

struct FuriThread {
StaticTask_t container;
TaskHandle_t task_handle;
StackType_t* stack_buffer;

FuriThreadState state;
Expand All @@ -54,6 +53,7 @@ struct FuriThread {
// this ensures that the size of this structure is minimal
bool is_service;
bool heap_trace_enabled;
volatile bool is_active;
};

// IMPORTANT: container MUST be the FIRST struct member
Expand Down Expand Up @@ -90,9 +90,8 @@ static void furi_thread_body(void* context) {
furi_check(thread->state == FuriThreadStateStarting);
furi_thread_set_state(thread, FuriThreadStateRunning);

TaskHandle_t task_handle = xTaskGetCurrentTaskHandle();
if(thread->heap_trace_enabled == true) {
memmgr_heap_enable_thread_trace((FuriThreadId)task_handle);
memmgr_heap_enable_thread_trace(thread);
}

thread->ret = thread->callback(thread->context);
Expand All @@ -101,14 +100,14 @@ static void furi_thread_body(void* context) {

if(thread->heap_trace_enabled == true) {
furi_delay_ms(33);
thread->heap_size = memmgr_heap_get_thread_memory((FuriThreadId)task_handle);
thread->heap_size = memmgr_heap_get_thread_memory(thread);
furi_log_print_format(
thread->heap_size ? FuriLogLevelError : FuriLogLevelInfo,
TAG,
"%s allocation balance: %zu",
thread->name ? thread->name : "Thread",
thread->heap_size);
memmgr_heap_disable_thread_trace((FuriThreadId)task_handle);
memmgr_heap_disable_thread_trace(thread);
}

furi_check(thread->state == FuriThreadStateRunning);
Expand Down Expand Up @@ -197,7 +196,7 @@ void furi_thread_free(FuriThread* thread) {
furi_check(thread->is_service == false);
// Cannot free a non-joined thread
furi_check(thread->state == FuriThreadStateStopped);
furi_check(thread->task_handle == NULL);
furi_check(!thread->is_active);

furi_thread_set_name(thread, NULL);
furi_thread_set_appid(thread, NULL);
Expand Down Expand Up @@ -313,25 +312,26 @@ void furi_thread_start(FuriThread* thread) {
uint32_t stack_depth = thread->stack_size / sizeof(StackType_t);
UBaseType_t priority = thread->priority ? thread->priority : FuriThreadPriorityNormal;

thread->task_handle = xTaskCreateStatic(
furi_thread_body,
thread->name,
stack_depth,
thread,
priority,
thread->stack_buffer,
&thread->container);
thread->is_active = true;

furi_check(thread->task_handle == (TaskHandle_t)&thread->container);
furi_check(
xTaskCreateStatic(
furi_thread_body,
thread->name,
stack_depth,
thread,
priority,
thread->stack_buffer,
&thread->container) == (TaskHandle_t)thread);
}

void furi_thread_cleanup_tcb_event(TaskHandle_t task) {
FuriThread* thread = pvTaskGetThreadLocalStoragePointer(task, 0);
if(thread) {
// clear thread local storage
vTaskSetThreadLocalStoragePointer(task, 0, NULL);
furi_check(thread->task_handle == task);
thread->task_handle = NULL;
furi_check(thread == (FuriThread*)task);
thread->is_active = false;
}
}

Expand All @@ -346,7 +346,7 @@ bool furi_thread_join(FuriThread* thread) {
//
// If your thread exited, but your app stuck here: some other thread uses
// all cpu time, which delays kernel from releasing task handle
while(thread->task_handle) {
while(thread->is_active) {
furi_delay_ms(10);
}

Expand All @@ -355,7 +355,7 @@ bool furi_thread_join(FuriThread* thread) {

FuriThreadId furi_thread_get_id(FuriThread* thread) {
furi_check(thread);
return thread->task_handle;
return thread;
}

void furi_thread_enable_heap_trace(FuriThread* thread) {
Expand Down
3 changes: 2 additions & 1 deletion targets/f18/api_symbols.csv
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
entry,status,name,type,params
Version,+,64.2,,
Version,+,64.3,,
Header,+,applications/services/bt/bt_service/bt.h,,
Header,+,applications/services/bt/bt_service/bt_keys_storage.h,,
Header,+,applications/services/cli/cli.h,,
Expand Down Expand Up @@ -1271,6 +1271,7 @@ Function,+,furi_hal_info_get,void,"PropertyValueCallback, char, void*"
Function,+,furi_hal_info_get_api_version,void,"uint16_t*, uint16_t*"
Function,-,furi_hal_init,void,
Function,-,furi_hal_init_early,void,
Function,+,furi_hal_interrupt_get_name,const char*,uint8_t
Function,-,furi_hal_interrupt_init,void,
Function,+,furi_hal_interrupt_set_isr,void,"FuriHalInterruptId, FuriHalInterruptISR, void*"
Function,+,furi_hal_interrupt_set_isr_ex,void,"FuriHalInterruptId, FuriHalInterruptPriority, FuriHalInterruptISR, void*"
Expand Down
3 changes: 2 additions & 1 deletion targets/f7/api_symbols.csv
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
entry,status,name,type,params
Version,+,64.2,,
Version,+,64.3,,
Header,+,applications/drivers/subghz/cc1101_ext/cc1101_ext_interconnect.h,,
Header,+,applications/services/bt/bt_service/bt.h,,
Header,+,applications/services/bt/bt_service/bt_keys_storage.h,,
Expand Down Expand Up @@ -1396,6 +1396,7 @@ Function,+,furi_hal_infrared_is_busy,_Bool,
Function,+,furi_hal_infrared_set_tx_output,void,FuriHalInfraredTxPin
Function,-,furi_hal_init,void,
Function,-,furi_hal_init_early,void,
Function,+,furi_hal_interrupt_get_name,const char*,uint8_t
Function,-,furi_hal_interrupt_init,void,
Function,+,furi_hal_interrupt_set_isr,void,"FuriHalInterruptId, FuriHalInterruptISR, void*"
Function,+,furi_hal_interrupt_set_isr_ex,void,"FuriHalInterruptId, FuriHalInterruptPriority, FuriHalInterruptISR, void*"
Expand Down
159 changes: 159 additions & 0 deletions targets/f7/furi_hal/furi_hal_interrupt.c
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,12 @@ void USB_LP_IRQHandler(void) {
#endif
}

void USB_HP_IRQHandler(void) { //-V524
#ifndef FURI_RAM_EXEC
usbd_poll(&udev);
#endif
}

void IPCC_C1_TX_IRQHandler(void) {
HW_IPCC_Tx_Handler();
}
Expand Down Expand Up @@ -340,3 +346,156 @@ void USART1_IRQHandler(void) {
void LPUART1_IRQHandler(void) {
furi_hal_interrupt_call(FuriHalInterruptIdLpUart1);
}

const char* furi_hal_interrupt_get_name(uint8_t exception_number) {
int32_t id = (int32_t)exception_number - 16;

switch(id) {
case -14:
return "NMI";
case -13:
return "HardFault";
case -12:
return "MemMgmt";
case -11:
return "BusFault";
case -10:
return "UsageFault";
case -5:
return "SVC";
case -4:
return "DebugMon";
case -2:
return "PendSV";
case -1:
return "SysTick";
case 0:
return "WWDG";
case 1:
return "PVD_PVM";
case 2:
return "TAMP";
case 3:
return "RTC_WKUP";
case 4:
return "FLASH";
case 5:
return "RCC";
case 6:
return "EXTI0";
case 7:
return "EXTI1";
case 8:
return "EXTI2";
case 9:
return "EXTI3";
case 10:
return "EXTI4";
case 11:
return "DMA1_Ch1";
case 12:
return "DMA1_Ch2";
case 13:
return "DMA1_Ch3";
case 14:
return "DMA1_Ch4";
case 15:
return "DMA1_Ch5";
case 16:
return "DMA1_Ch6";
case 17:
return "DMA1_Ch7";
case 18:
return "ADC1";
case 19:
return "USB_HP";
case 20:
return "USB_LP";
case 21:
return "C2SEV_PWR_C2H";
case 22:
return "COMP";
case 23:
return "EXTI9_5";
case 24:
return "TIM1_BRK";
case 25:
return "TIM1_UP_TIM16";
case 26:
return "TIM1_TRG_COM_TIM17";
case 27:
return "TIM1_CC";
case 28:
return "TIM2";
case 29:
return "PKA";
case 30:
return "I2C1_EV";
case 31:
return "I2C1_ER";
case 32:
return "I2C3_EV";
case 33:
return "I2C3_ER";
case 34:
return "SPI1";
case 35:
return "SPI2";
case 36:
return "USART1";
case 37:
return "LPUART1";
case 38:
return "SAI1";
case 39:
return "TSC";
case 40:
return "EXTI15_10";
case 41:
return "RTC_Alarm";
case 42:
return "CRS";
case 43:
return "PWR_SOTF_BLE";
case 44:
return "IPCC_C1_RX";
case 45:
return "IPCC_C1_TX";
case 46:
return "HSEM";
case 47:
return "LPTIM1";
case 48:
return "LPTIM2";
case 49:
return "LCD";
case 50:
return "QUADSPI";
case 51:
return "AES1";
case 52:
return "AES2";
case 53:
return "RNG";
case 54:
return "FPU";
case 55:
return "DMA2_Ch1";
case 56:
return "DMA2_Ch2";
case 57:
return "DMA2_Ch3";
case 58:
return "DMA2_Ch4";
case 59:
return "DMA2_Ch5";
case 60:
return "DMA2_Ch6";
case 61:
return "DMA2_Ch7";
case 62:
return "DMAMUX1_OVR";
default:
return NULL;
}
}
8 changes: 8 additions & 0 deletions targets/f7/furi_hal/furi_hal_interrupt.h
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,14 @@ void furi_hal_interrupt_set_isr_ex(
FuriHalInterruptISR isr,
void* context);

/** Get interrupt name by exception number.
* Exception number can be obtained from IPSR register.
*
* @param exception_number
* @return const char* or NULL if interrupt name is not found
*/
const char* furi_hal_interrupt_get_name(uint8_t exception_number);

#ifdef __cplusplus
}
#endif
2 changes: 1 addition & 1 deletion targets/f7/furi_hal/furi_hal_usb_ccid.c
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ static const struct CcidConfigDescriptor ccid_cfg_desc = {
.bConfigurationValue = 1,
.iConfiguration = NO_DESCRIPTOR,
.bmAttributes = USB_CFG_ATTR_RESERVED | USB_CFG_ATTR_SELFPOWERED,
.bMaxPower = USB_CFG_POWER_MA(100),
.bMaxPower = USB_CFG_POWER_MA(500),
},
.intf_0 =
{
Expand Down
12 changes: 6 additions & 6 deletions targets/f7/furi_hal/furi_hal_usb_cdc.c
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,13 @@
#include "usb.h"
#include "usb_cdc.h"

#define CDC0_RXD_EP 0x02
#define CDC0_RXD_EP 0x01
#define CDC0_TXD_EP 0x82
#define CDC0_NTF_EP 0x81
#define CDC0_NTF_EP 0x83

#define CDC1_RXD_EP 0x04
#define CDC1_TXD_EP 0x84
#define CDC1_NTF_EP 0x83
#define CDC1_TXD_EP 0x85
#define CDC1_NTF_EP 0x86

#define CDC_NTF_SZ 0x08

Expand Down Expand Up @@ -75,7 +75,7 @@ static const struct CdcConfigDescriptorSingle cdc_cfg_desc_single = {
.bConfigurationValue = 1,
.iConfiguration = NO_DESCRIPTOR,
.bmAttributes = USB_CFG_ATTR_RESERVED | USB_CFG_ATTR_SELFPOWERED,
.bMaxPower = USB_CFG_POWER_MA(100),
.bMaxPower = USB_CFG_POWER_MA(500),
},
.iad_0 =
{
Expand Down Expand Up @@ -188,7 +188,7 @@ static const struct CdcConfigDescriptorDual
.bConfigurationValue = 1,
.iConfiguration = NO_DESCRIPTOR,
.bmAttributes = USB_CFG_ATTR_RESERVED | USB_CFG_ATTR_SELFPOWERED,
.bMaxPower = USB_CFG_POWER_MA(100),
.bMaxPower = USB_CFG_POWER_MA(500),
},
.iad_0 =
{
Expand Down
Loading

0 comments on commit 70d2453

Please sign in to comment.