Skip to content

Commit

Permalink
Gpio race condition fix (commaai#263)
Browse files Browse the repository at this point in the history 
* Fixed pedal not initializing

* Interrupt changes

* More changes
  • Loading branch information
@robbederks @rbiasini
robbederks authored and rbiasini committed Aug 28, 2019
1 parent d69d05f commit d68508c
Show file tree
Hide file tree
Showing 10 changed files with 70 additions and 47 deletions.
10 changes: 7 additions & 3 deletions board/board.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,12 @@ void detect_configuration(void) {
has_external_debug_serial = detect_with_pull(GPIOA, 3, PULL_DOWN);

#ifdef PANDA
// check if the ESP is trying to put me in boot mode
is_entering_bootmode = !detect_with_pull(GPIOB, 0, PULL_UP);
if(hw_type == HW_TYPE_WHITE_PANDA) {
// check if the ESP is trying to put me in boot mode
is_entering_bootmode = !detect_with_pull(GPIOB, 0, PULL_UP);
} else {
is_entering_bootmode = 0;
}
#else
is_entering_bootmode = 0;
#endif
Expand All @@ -58,4 +62,4 @@ void detect_configuration(void) {
// ///// Board functions ///// //
bool board_has_gps(void) {
return ((hw_type == HW_TYPE_GREY_PANDA) || (hw_type == HW_TYPE_BLACK_PANDA));
}
}
2 changes: 1 addition & 1 deletion board/bootstub.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ extern void *_app_start[];
// BOUNTY: $200 coupon on shop.comma.ai or $100 check.

int main(void) {
__disable_irq();
disable_interrupts();
clock_init();
detect_configuration();
detect_board_type();
Expand Down
25 changes: 13 additions & 12 deletions board/drivers/can.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ int can_overflow_cnt = 0;
bool can_pop(can_ring *q, CAN_FIFOMailBox_TypeDef *elem) {
bool ret = 0;

enter_critical_section();
ENTER_CRITICAL();
if (q->w_ptr != q->r_ptr) {
*elem = q->elems[q->r_ptr];
if ((q->r_ptr + 1U) == q->fifo_size) {
Expand All @@ -72,7 +72,7 @@ bool can_pop(can_ring *q, CAN_FIFOMailBox_TypeDef *elem) {
}
ret = 1;
}
exit_critical_section();
EXIT_CRITICAL();

return ret;
}
Expand All @@ -81,7 +81,7 @@ bool can_push(can_ring *q, CAN_FIFOMailBox_TypeDef *elem) {
bool ret = false;
uint32_t next_w_ptr;

enter_critical_section();
ENTER_CRITICAL();
if ((q->w_ptr + 1U) == q->fifo_size) {
next_w_ptr = 0;
} else {
Expand All @@ -92,7 +92,7 @@ bool can_push(can_ring *q, CAN_FIFOMailBox_TypeDef *elem) {
q->w_ptr = next_w_ptr;
ret = true;
}
exit_critical_section();
EXIT_CRITICAL();
if (!ret) {
can_overflow_cnt++;
#ifdef DEBUG
Expand All @@ -103,10 +103,10 @@ bool can_push(can_ring *q, CAN_FIFOMailBox_TypeDef *elem) {
}

void can_clear(can_ring *q) {
enter_critical_section();
ENTER_CRITICAL();
q->w_ptr = 0;
q->r_ptr = 0;
exit_critical_section();
EXIT_CRITICAL();
}

// assign CAN numbering
Expand All @@ -124,7 +124,7 @@ uint8_t bus_lookup[] = {0,1,2};
uint8_t can_num_lookup[] = {0,1,2,-1};
int8_t can_forwarding[] = {-1,-1,-1,-1};
uint32_t can_speed[] = {5000, 5000, 5000, 333};
#define CAN_MAX 3
#define CAN_MAX 3U

#define CANIF_FROM_CAN_NUM(num) (cans[num])
#define CAN_NUM_FROM_CANIF(CAN) ((CAN)==CAN1 ? 0 : ((CAN) == CAN2 ? 1 : 2))
Expand Down Expand Up @@ -158,9 +158,10 @@ void can_init(uint8_t can_number) {
}

void can_init_all(void) {
for (int i=0; i < CAN_MAX; i++) {
for (uint8_t i=0U; i < CAN_MAX; i++) {
can_init(i);
}
current_board->enable_can_transcievers(true);
}

void can_flip_buses(uint8_t bus1, uint8_t bus2){
Expand Down Expand Up @@ -248,7 +249,7 @@ void can_set_obd(uint8_t harness_orientation, bool obd){

// CAN error
void can_sce(CAN_TypeDef *CAN) {
enter_critical_section();
ENTER_CRITICAL();

#ifdef DEBUG
if (CAN==CAN1) puts("CAN1: ");
Expand All @@ -271,15 +272,15 @@ void can_sce(CAN_TypeDef *CAN) {

can_err_cnt += 1;
llcan_clear_send(CAN);
exit_critical_section();
EXIT_CRITICAL();
}

// ***************************** CAN *****************************

void process_can(uint8_t can_number) {
if (can_number != 0xffU) {

enter_critical_section();
ENTER_CRITICAL();

CAN_TypeDef *CAN = CANIF_FROM_CAN_NUM(can_number);
uint8_t bus_number = BUS_NUM_FROM_CAN_NUM(can_number);
Expand Down Expand Up @@ -327,7 +328,7 @@ void process_can(uint8_t can_number) {
}
}

exit_critical_section();
EXIT_CRITICAL();
}
}

Expand Down
10 changes: 10 additions & 0 deletions board/drivers/llgpio.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,42 +11,52 @@
#define OUTPUT_TYPE_OPEN_DRAIN 1U

void set_gpio_mode(GPIO_TypeDef *GPIO, unsigned int pin, unsigned int mode) {
ENTER_CRITICAL();
uint32_t tmp = GPIO->MODER;
tmp &= ~(3U << (pin * 2U));
tmp |= (mode << (pin * 2U));
GPIO->MODER = tmp;
EXIT_CRITICAL();
}

void set_gpio_output(GPIO_TypeDef *GPIO, unsigned int pin, bool enabled) {
ENTER_CRITICAL();
if (enabled) {
GPIO->ODR |= (1U << pin);
} else {
GPIO->ODR &= ~(1U << pin);
}
set_gpio_mode(GPIO, pin, MODE_OUTPUT);
EXIT_CRITICAL();
}

void set_gpio_output_type(GPIO_TypeDef *GPIO, unsigned int pin, unsigned int output_type){
ENTER_CRITICAL();
if(output_type == OUTPUT_TYPE_OPEN_DRAIN) {
GPIO->OTYPER |= (1U << pin);
} else {
GPIO->OTYPER &= ~(1U << pin);
}
EXIT_CRITICAL();
}

void set_gpio_alternate(GPIO_TypeDef *GPIO, unsigned int pin, unsigned int mode) {
ENTER_CRITICAL();
uint32_t tmp = GPIO->AFR[pin >> 3U];
tmp &= ~(0xFU << ((pin & 7U) * 4U));
tmp |= mode << ((pin & 7U) * 4U);
GPIO->AFR[pin >> 3] = tmp;
set_gpio_mode(GPIO, pin, MODE_ALTERNATE);
EXIT_CRITICAL();
}

void set_gpio_pullup(GPIO_TypeDef *GPIO, unsigned int pin, unsigned int mode) {
ENTER_CRITICAL();
uint32_t tmp = GPIO->PUPDR;
tmp &= ~(3U << (pin * 2U));
tmp |= (mode << (pin * 2U));
GPIO->PUPDR = tmp;
EXIT_CRITICAL();
}

int get_gpio_input(GPIO_TypeDef *GPIO, unsigned int pin) {
Expand Down
24 changes: 12 additions & 12 deletions board/drivers/uart.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ uart_ring *get_ring_by_number(int a) {
// ***************************** serial port *****************************

void uart_ring_process(uart_ring *q) {
enter_critical_section();
ENTER_CRITICAL();
// TODO: check if external serial is connected
int sr = q->uart->SR;

Expand Down Expand Up @@ -108,7 +108,7 @@ void uart_ring_process(uart_ring *q) {
// set dropped packet flag?
}

exit_critical_section();
EXIT_CRITICAL();
}

// interrupt boilerplate
Expand All @@ -121,13 +121,13 @@ void UART5_IRQHandler(void) { uart_ring_process(&lin1_ring); }
bool getc(uart_ring *q, char *elem) {
bool ret = false;

enter_critical_section();
ENTER_CRITICAL();
if (q->w_ptr_rx != q->r_ptr_rx) {
if (elem != NULL) *elem = q->elems_rx[q->r_ptr_rx];
q->r_ptr_rx = (q->r_ptr_rx + 1U) % FIFO_SIZE;
ret = true;
}
exit_critical_section();
EXIT_CRITICAL();

return ret;
}
Expand All @@ -136,14 +136,14 @@ bool injectc(uart_ring *q, char elem) {
int ret = false;
uint16_t next_w_ptr;

enter_critical_section();
ENTER_CRITICAL();
next_w_ptr = (q->w_ptr_rx + 1U) % FIFO_SIZE;
if (next_w_ptr != q->r_ptr_rx) {
q->elems_rx[q->w_ptr_rx] = elem;
q->w_ptr_rx = next_w_ptr;
ret = true;
}
exit_critical_section();
EXIT_CRITICAL();

return ret;
}
Expand All @@ -152,14 +152,14 @@ bool putc(uart_ring *q, char elem) {
bool ret = false;
uint16_t next_w_ptr;

enter_critical_section();
ENTER_CRITICAL();
next_w_ptr = (q->w_ptr_tx + 1U) % FIFO_SIZE;
if (next_w_ptr != q->r_ptr_tx) {
q->elems_tx[q->w_ptr_tx] = elem;
q->w_ptr_tx = next_w_ptr;
ret = true;
}
exit_critical_section();
EXIT_CRITICAL();

uart_ring_process(q);

Expand All @@ -185,12 +185,12 @@ void uart_send_break(uart_ring *u) {
}

void clear_uart_buff(uart_ring *q) {
enter_critical_section();
ENTER_CRITICAL();
q->w_ptr_tx = 0;
q->r_ptr_tx = 0;
q->w_ptr_rx = 0;
q->r_ptr_rx = 0;
exit_critical_section();
EXIT_CRITICAL();
}

// ***************************** start UART code *****************************
Expand All @@ -215,7 +215,7 @@ char usart1_dma[USART1_DMA_LEN];
void uart_dma_drain(void) {
uart_ring *q = &esp_ring;

enter_critical_section();
ENTER_CRITICAL();

if ((DMA2->HISR & DMA_HISR_TCIF5) || (DMA2->HISR & DMA_HISR_HTIF5) || (DMA2_Stream5->NDTR != USART1_DMA_LEN)) {
// disable DMA
Expand Down Expand Up @@ -245,7 +245,7 @@ void uart_dma_drain(void) {
q->uart->CR3 |= USART_CR3_DMAR;
}

exit_critical_section();
EXIT_CRITICAL();
}

void DMA2_Stream5_IRQHandler(void) {
Expand Down
6 changes: 6 additions & 0 deletions board/gpio.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,12 @@ void jump_to_bootloader(void) {
}

void early(void) {
// Reset global critical depth
global_critical_depth = 0;

// neccesary for DFU flashing on a non-power cycled white panda
enable_interrupts();

// after it's been in the bootloader, things are initted differently, so we reset
if ((enter_bootloader_mode != BOOT_NORMAL) &&
(enter_bootloader_mode != ENTER_BOOTLOADER_MAGIC) &&
Expand Down
26 changes: 14 additions & 12 deletions board/libc.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,24 +42,26 @@ int memcmp(const void * ptr1, const void * ptr2, unsigned int num) {

// ********************* IRQ helpers *********************

int interrupts_enabled = 0;
volatile bool interrupts_enabled = false;

void enable_interrupts(void) {
interrupts_enabled = 1;
interrupts_enabled = true;
__enable_irq();
}

int critical_depth = 0;
void enter_critical_section(void) {
void disable_interrupts(void) {
interrupts_enabled = false;
__disable_irq();
// this is safe because interrupts are disabled
critical_depth += 1;
}

void exit_critical_section(void) {
// this is safe because interrupts are disabled
critical_depth -= 1;
if ((critical_depth == 0) && interrupts_enabled) {
__enable_irq();
uint8_t global_critical_depth = 0U;
#define ENTER_CRITICAL() \
__disable_irq(); \
global_critical_depth += 1U;

#define EXIT_CRITICAL() \
global_critical_depth -= 1U; \
if ((global_critical_depth == 0U) && interrupts_enabled) { \
__enable_irq(); \
}
}

2 changes: 1 addition & 1 deletion board/main.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -645,7 +645,7 @@ void TIM3_IRQHandler(void) {

int main(void) {
// shouldn't have interrupts here, but just in case
__disable_irq();
disable_interrupts();

// init early devices
clock_init();
Expand Down
10 changes: 5 additions & 5 deletions board/pedal/main.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,18 +55,18 @@ void debug_ring_callback(uart_ring *ring) {
}
}

int usb_cb_ep1_in(uint8_t *usbdata, int len, bool hardwired) {
int usb_cb_ep1_in(void *usbdata, int len, bool hardwired) {
UNUSED(usbdata);
UNUSED(len);
UNUSED(hardwired);
return 0;
}
void usb_cb_ep2_out(uint8_t *usbdata, int len, bool hardwired) {
void usb_cb_ep2_out(void *usbdata, int len, bool hardwired) {
UNUSED(usbdata);
UNUSED(len);
UNUSED(hardwired);
}
void usb_cb_ep3_out(uint8_t *usbdata, int len, bool hardwired) {
void usb_cb_ep3_out(void *usbdata, int len, bool hardwired) {
UNUSED(usbdata);
UNUSED(len);
UNUSED(hardwired);
Expand Down Expand Up @@ -299,7 +299,7 @@ void pedal(void) {
}

int main(void) {
__disable_irq();
disable_interrupts();

// init devices
clock_init();
Expand Down Expand Up @@ -334,7 +334,7 @@ int main(void) {
watchdog_init();

puts("**** INTERRUPTS ON ****\n");
__enable_irq();
enable_interrupts();

// main pedal loop
while (1) {
Expand Down
Loading

0 comments on commit d68508c

Please sign in to comment.