From 893e4861944740b27a5cc87edbaedfbe754bff8a Mon Sep 17 00:00:00 2001 From: robbederks Date: Wed, 4 Dec 2019 20:42:57 -0800 Subject: [PATCH] Register readback on most modules. Still need to convert the other ones (#396) * Added an initial working implementation of the register readback and applied it to the LLCAN registers. Also fixed an init bug in CAN. * Locally disable pylint import error * Reverted change to CAN-obj->FA1R setting * Fixed misra compliance * Changed hash function name * Fixed CAN filter setting * Added voltage health test * Converted ADC to register functions * Converted clock to use register setters * Added check to see if fault status is zero after running test * Converted DAC to use register setters * Converted fan to use register setters * Converted gmlan bitbanging to use register setters * Changed over interrupt driver to use register setters. Also moved some includes and definition places for critical sections * Fixed build * Converted LLGPIO to use register setters * Converted pwm to use register setters * Fixed cold boot * Fixed health fault check in automated tests * Added comment for the future (issue #367) * Converted RTC driver to use register setters * Converted SPI to use register setters * Converted timer driver to use register setters * Fixed register fault on white pandas and disabled showing of fault for release * Bump version --- VERSION | 2 +- board/bootstub.c | 5 +- board/critical.h | 23 ++++++ board/drivers/adc.h | 22 ++--- board/drivers/clock.h | 24 +++--- board/drivers/dac.h | 17 ++-- board/drivers/fan.h | 8 +- board/drivers/gmlan_alt.h | 16 ++-- board/drivers/interrupts.h | 34 +------- board/drivers/llcan.h | 36 +++++---- board/drivers/llgpio.h | 14 ++-- board/drivers/pwm.h | 31 +++---- board/drivers/registers.h | 81 +++++++++++++++++++ board/drivers/rtc.h | 20 ++--- board/drivers/spi.h | 38 ++++----- board/drivers/timer.h | 6 +- board/faults.h | 1 + board/gpio.h | 3 + board/main.c | 7 ++ board/main_declarations.h | 2 +- board/pedal/main.c | 4 + board/power_saving.h | 3 + ...{2_ignition_orientation.py => 2_health.py} | 7 ++ tests/automated/helpers.py | 5 ++ tests/development/register_hashmap_spread.py | 50 ++++++++++++ 25 files changed, 306 insertions(+), 153 deletions(-) create mode 100644 board/critical.h create mode 100644 board/drivers/registers.h rename tests/automated/{2_ignition_orientation.py => 2_health.py} (89%) create mode 100755 tests/development/register_hashmap_spread.py diff --git a/VERSION b/VERSION index 1a6afeda67a7bc..62f6ecd02c2992 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -v1.6.8 \ No newline at end of file +v1.6.9 diff --git a/board/bootstub.c b/board/bootstub.c index 78376d91584189..ac75c9c072ec5f 100644 --- a/board/bootstub.c +++ b/board/bootstub.c @@ -29,8 +29,10 @@ const board *current_board; // ********************* Includes ********************* #include "libc.h" #include "provision.h" +#include "critical.h" #include "faults.h" +#include "drivers/registers.h" #include "drivers/interrupts.h" #include "drivers/clock.h" #include "drivers/llgpio.h" @@ -67,7 +69,8 @@ extern void *_app_start[]; // BOUNTY: $200 coupon on shop.comma.ai or $100 check. int main(void) { - init_interrupts(false); + // Init interrupt table + init_interrupts(true); disable_interrupts(); clock_init(); diff --git a/board/critical.h b/board/critical.h new file mode 100644 index 00000000000000..c8cf52c7a11533 --- /dev/null +++ b/board/critical.h @@ -0,0 +1,23 @@ +// ********************* Critical section helpers ********************* +volatile bool interrupts_enabled = false; + +void enable_interrupts(void) { + interrupts_enabled = true; + __enable_irq(); +} + +void disable_interrupts(void) { + interrupts_enabled = false; + __disable_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(); \ + } diff --git a/board/drivers/adc.h b/board/drivers/adc.h index 2a91fef8dcd64e..358497adbfdcda 100644 --- a/board/drivers/adc.h +++ b/board/drivers/adc.h @@ -9,26 +9,16 @@ #define ADCCHAN_CURRENT 13 void adc_init(void) { - // global setup - ADC->CCR = ADC_CCR_TSVREFE | ADC_CCR_VBATE; - //ADC1->CR2 = ADC_CR2_ADON | ADC_CR2_EOCS | ADC_CR2_DDS; - ADC1->CR2 = ADC_CR2_ADON; - - // long - //ADC1->SMPR1 = ADC_SMPR1_SMP10 | ADC_SMPR1_SMP11 | ADC_SMPR1_SMP12 | ADC_SMPR1_SMP13; - ADC1->SMPR1 = ADC_SMPR1_SMP12 | ADC_SMPR1_SMP13; + register_set(&(ADC->CCR), ADC_CCR_TSVREFE | ADC_CCR_VBATE, 0xC30000U); + register_set(&(ADC1->CR2), ADC_CR2_ADON, 0xFF7F0F03U); + register_set(&(ADC1->SMPR1), ADC_SMPR1_SMP12 | ADC_SMPR1_SMP13, 0x7FFFFFFU); } uint32_t adc_get(unsigned int channel) { - // includes length - //ADC1->SQR1 = 0; - - // select channel - ADC1->JSQR = channel << 15; - - //ADC1->CR1 = ADC_CR1_DISCNUM_0; - //ADC1->CR1 = ADC_CR1_EOCIE; + // Select channel + register_set(&(ADC1->JSQR), (channel << 15U), 0x3FFFFFU); + // Start conversion ADC1->SR &= ~(ADC_SR_JEOC); ADC1->CR2 |= ADC_CR2_JSWSTART; while (!(ADC1->SR & ADC_SR_JEOC)); diff --git a/board/drivers/clock.h b/board/drivers/clock.h index d564c7f01db28d..b75692438ec63a 100644 --- a/board/drivers/clock.h +++ b/board/drivers/clock.h @@ -1,25 +1,24 @@ void clock_init(void) { // enable external oscillator - RCC->CR |= RCC_CR_HSEON; + register_set_bits(&(RCC->CR), RCC_CR_HSEON); while ((RCC->CR & RCC_CR_HSERDY) == 0); // divide things - RCC->CFGR = RCC_CFGR_HPRE_DIV1 | RCC_CFGR_PPRE2_DIV2 | RCC_CFGR_PPRE1_DIV4; + register_set(&(RCC->CFGR), RCC_CFGR_HPRE_DIV1 | RCC_CFGR_PPRE2_DIV2 | RCC_CFGR_PPRE1_DIV4, 0xFF7FFCF3U); // 16mhz crystal - RCC->PLLCFGR = RCC_PLLCFGR_PLLQ_2 | RCC_PLLCFGR_PLLM_3 | - RCC_PLLCFGR_PLLN_6 | RCC_PLLCFGR_PLLN_5 | RCC_PLLCFGR_PLLSRC_HSE; + register_set(&(RCC->PLLCFGR), RCC_PLLCFGR_PLLQ_2 | RCC_PLLCFGR_PLLM_3 | RCC_PLLCFGR_PLLN_6 | RCC_PLLCFGR_PLLN_5 | RCC_PLLCFGR_PLLSRC_HSE, 0x7F437FFFU); // start PLL - RCC->CR |= RCC_CR_PLLON; + register_set_bits(&(RCC->CR), RCC_CR_PLLON); while ((RCC->CR & RCC_CR_PLLRDY) == 0); // Configure Flash prefetch, Instruction cache, Data cache and wait state // *** without this, it breaks *** - FLASH->ACR = FLASH_ACR_ICEN | FLASH_ACR_DCEN | FLASH_ACR_LATENCY_5WS; + register_set(&(FLASH->ACR), FLASH_ACR_ICEN | FLASH_ACR_DCEN | FLASH_ACR_LATENCY_5WS, 0x1F0FU); // switch to PLL - RCC->CFGR |= RCC_CFGR_SW_PLL; + register_set_bits(&(RCC->CFGR), RCC_CFGR_SW_PLL); while ((RCC->CFGR & RCC_CFGR_SWS) != RCC_CFGR_SWS_PLL); // *** running on PLL *** @@ -27,14 +26,15 @@ void clock_init(void) { void watchdog_init(void) { // setup watchdog - IWDG->KR = 0x5555; - IWDG->PR = 0; // divider /4 + IWDG->KR = 0x5555U; + register_set(&(IWDG->PR), 0x0U, 0x7U); // divider/4 + // 0 = 0.125 ms, let's have a 50ms watchdog - IWDG->RLR = 400 - 1; - IWDG->KR = 0xCCCC; + register_set(&(IWDG->RLR), (400U-1U), 0xFFFU); + IWDG->KR = 0xCCCCU; } void watchdog_feed(void) { - IWDG->KR = 0xAAAA; + IWDG->KR = 0xAAAAU; } diff --git a/board/drivers/dac.h b/board/drivers/dac.h index ac565eb221cd05..4bdb16100c7bb6 100644 --- a/board/drivers/dac.h +++ b/board/drivers/dac.h @@ -2,22 +2,19 @@ void puth(unsigned int i); void puts(const char *a); void dac_init(void) { - // no buffers required since we have an opamp - //DAC->CR = DAC_CR_EN1 | DAC_CR_BOFF1 | DAC_CR_EN2 | DAC_CR_BOFF2; - DAC->DHR12R1 = 0; - DAC->DHR12R2 = 0; - DAC->CR = DAC_CR_EN1 | DAC_CR_EN2; + // No buffers required since we have an opamp + register_set(&(DAC->DHR12R1), 0U, 0xFFFU); + register_set(&(DAC->DHR12R2), 0U, 0xFFFU); + register_set(&(DAC->CR), DAC_CR_EN1 | DAC_CR_EN2, 0x3FFF3FFFU); } void dac_set(int channel, uint32_t value) { if (channel == 0) { - DAC->DHR12R1 = value; + register_set(&(DAC->DHR12R1), value, 0xFFFU); } else if (channel == 1) { - DAC->DHR12R2 = value; + register_set(&(DAC->DHR12R2), value, 0xFFFU); } else { - puts("Failed to set DAC: invalid channel value: "); - puth(value); - puts("\n"); + puts("Failed to set DAC: invalid channel value: 0x"); puth(value); puts("\n"); } } diff --git a/board/drivers/fan.h b/board/drivers/fan.h index 89b4bd34314e59..2f10e5ca8d7b51 100644 --- a/board/drivers/fan.h +++ b/board/drivers/fan.h @@ -31,9 +31,9 @@ void fan_init(void){ pwm_init(TIM3, 3); // Init TACH interrupt - SYSCFG->EXTICR[0] = SYSCFG_EXTICR1_EXTI2_PD; - EXTI->IMR |= (1U << 2); - EXTI->RTSR |= (1U << 2); - EXTI->FTSR |= (1U << 2); + register_set(&(SYSCFG->EXTICR[0]), SYSCFG_EXTICR1_EXTI2_PD, 0xF00U); + register_set_bits(&(EXTI->IMR), (1U << 2)); + register_set_bits(&(EXTI->RTSR), (1U << 2)); + register_set_bits(&(EXTI->FTSR), (1U << 2)); NVIC_EnableIRQ(EXTI2_IRQn); } \ No newline at end of file diff --git a/board/drivers/gmlan_alt.h b/board/drivers/gmlan_alt.h index 8fd32476f62002..6d4ba12b8c1069 100644 --- a/board/drivers/gmlan_alt.h +++ b/board/drivers/gmlan_alt.h @@ -124,15 +124,15 @@ int get_bit_message(char *out, CAN_FIFOMailBox_TypeDef *to_bang) { void setup_timer4(void) { // setup - TIM4->PSC = 48-1; // tick on 1 us - TIM4->CR1 = TIM_CR1_CEN; // enable - TIM4->ARR = 30-1; // 33.3 kbps + register_set(&(TIM4->PSC), (48-1), 0xFFFFU); // Tick on 1 us + register_set(&(TIM4->CR1), TIM_CR1_CEN, 0x3FU); // Enable + register_set(&(TIM4->ARR), (30-1), 0xFFFFU); // 33.3 kbps // in case it's disabled NVIC_EnableIRQ(TIM4_IRQn); // run the interrupt - TIM4->DIER = TIM_DIER_UIE; // update interrupt + register_set(&(TIM4->DIER), TIM_DIER_UIE, 0x5F5FU); // Update interrupt TIM4->SR = 0; } @@ -171,9 +171,9 @@ void reset_gmlan_switch_timeout(void) { void set_bitbanged_gmlan(int val) { if (val != 0) { - GPIOB->ODR |= (1U << 13); + register_set_bits(&(GPIOB->ODR), (1U << 13)); } else { - GPIOB->ODR &= ~(1U << 13); + register_clear_bits(&(GPIOB->ODR), (1U << 13)); } } @@ -231,8 +231,8 @@ void TIM4_IRQ_Handler(void) { if ((gmlan_sending == gmlan_sendmax) || (gmlan_fail_count == MAX_FAIL_COUNT)) { set_bitbanged_gmlan(1); // recessive set_gpio_mode(GPIOB, 13, MODE_INPUT); - TIM4->DIER = 0; // no update interrupt - TIM4->CR1 = 0; // disable timer + register_clear_bits(&(TIM4->DIER), TIM_DIER_UIE); // No update interrupt + register_set(&(TIM4->CR1), 0U, 0x3FU); // Disable timer gmlan_sendmax = -1; // exit } } diff --git a/board/drivers/interrupts.h b/board/drivers/interrupts.h index d0a138edb5e0ba..f15c441ab04dfe 100644 --- a/board/drivers/interrupts.h +++ b/board/drivers/interrupts.h @@ -1,29 +1,3 @@ -// ********************* Interrupt helpers ********************* -volatile bool interrupts_enabled = false; - -void enable_interrupts(void) { - interrupts_enabled = true; - __enable_irq(); -} - -void disable_interrupts(void) { - interrupts_enabled = false; - __disable_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(); \ - } - -// ********************* Interrupt handling ********************* - typedef struct interrupt { IRQn_Type irq_type; void (*handler)(void); @@ -79,11 +53,11 @@ void init_interrupts(bool check_rate_limit){ } // Init timer 10 for a 1s interval - RCC->APB1ENR |= RCC_APB1ENR_TIM6EN; // enable interrupt timer peripheral + register_set_bits(&(RCC->APB1ENR), RCC_APB1ENR_TIM6EN); // Enable interrupt timer peripheral REGISTER_INTERRUPT(TIM6_DAC_IRQn, TIM6_DAC_IRQ_Handler, 1, FAULT_INTERRUPT_RATE_INTERRUPTS) - TIM6->PSC = 732-1; - TIM6->DIER = TIM_DIER_UIE; - TIM6->CR1 = TIM_CR1_CEN; + register_set(&(TIM6->PSC), (732-1), 0xFFFFU); + register_set(&(TIM6->DIER), TIM_DIER_UIE, 0x5F5FU); + register_set(&(TIM6->CR1), TIM_CR1_CEN, 0x3FU); TIM6->SR = 0; NVIC_EnableIRQ(TIM6_DAC_IRQn); } diff --git a/board/drivers/llcan.h b/board/drivers/llcan.h index 0a698d4e8d9b93..4cd9b4b5abd416 100644 --- a/board/drivers/llcan.h +++ b/board/drivers/llcan.h @@ -19,25 +19,24 @@ void puts(const char *a); bool llcan_set_speed(CAN_TypeDef *CAN_obj, uint32_t speed, bool loopback, bool silent) { // initialization mode - CAN_obj->MCR = CAN_MCR_TTCM | CAN_MCR_INRQ; + register_set(&(CAN_obj->MCR), CAN_MCR_TTCM | CAN_MCR_INRQ, 0x180FFU); while((CAN_obj->MSR & CAN_MSR_INAK) != CAN_MSR_INAK); // set time quanta from defines - CAN_obj->BTR = (CAN_BTR_TS1_0 * (CAN_SEQ1-1)) | + register_set(&(CAN_obj->BTR), ((CAN_BTR_TS1_0 * (CAN_SEQ1-1)) | (CAN_BTR_TS2_0 * (CAN_SEQ2-1)) | - (can_speed_to_prescaler(speed) - 1U); + (can_speed_to_prescaler(speed) - 1U)), 0xC37F03FFU); // silent loopback mode for debugging if (loopback) { - CAN_obj->BTR |= CAN_BTR_SILM | CAN_BTR_LBKM; + register_set_bits(&(CAN_obj->BTR), CAN_BTR_SILM | CAN_BTR_LBKM); } if (silent) { - CAN_obj->BTR |= CAN_BTR_SILM; + register_set_bits(&(CAN_obj->BTR), CAN_BTR_SILM); } // reset - // cppcheck-suppress redundantAssignment ; it's a register - CAN_obj->MCR = CAN_MCR_TTCM | CAN_MCR_ABOM; + register_set(&(CAN_obj->MCR), CAN_MCR_TTCM | CAN_MCR_ABOM, 0x180FFU); #define CAN_TIMEOUT 1000000 int tmp = 0; @@ -51,20 +50,25 @@ bool llcan_set_speed(CAN_TypeDef *CAN_obj, uint32_t speed, bool loopback, bool s } void llcan_init(CAN_TypeDef *CAN_obj) { - // accept all filter - CAN_obj->FMR |= CAN_FMR_FINIT; + // Enter init mode + register_set_bits(&(CAN_obj->FMR), CAN_FMR_FINIT); + + // Wait for INAK bit to be set + while(((CAN_obj->MSR & CAN_MSR_INAK) == CAN_MSR_INAK)) {} // no mask - CAN_obj->sFilterRegister[0].FR1 = 0; - CAN_obj->sFilterRegister[0].FR2 = 0; - CAN_obj->sFilterRegister[14].FR1 = 0; - CAN_obj->sFilterRegister[14].FR2 = 0; + // For some weird reason some of these registers do not want to set properly on CAN2 and CAN3. Probably something to do with the single/dual mode and their different filters. + CAN_obj->sFilterRegister[0].FR1 = 0U; + CAN_obj->sFilterRegister[0].FR2 = 0U; + CAN_obj->sFilterRegister[14].FR1 = 0U; + CAN_obj->sFilterRegister[14].FR2 = 0U; CAN_obj->FA1R |= 1U | (1U << 14); - CAN_obj->FMR &= ~(CAN_FMR_FINIT); + // Exit init mode, do not wait + register_clear_bits(&(CAN_obj->FMR), CAN_FMR_FINIT); // enable certain CAN interrupts - CAN_obj->IER |= CAN_IER_TMEIE | CAN_IER_FMPIE0 | CAN_IER_WKUIE; + register_set_bits(&(CAN_obj->IER), CAN_IER_TMEIE | CAN_IER_FMPIE0 | CAN_IER_WKUIE); if (CAN_obj == CAN1) { NVIC_EnableIRQ(CAN1_TX_IRQn); @@ -87,7 +91,7 @@ void llcan_init(CAN_TypeDef *CAN_obj) { void llcan_clear_send(CAN_TypeDef *CAN_obj) { CAN_obj->TSR |= CAN_TSR_ABRQ0; - CAN_obj->MSR &= ~(CAN_MSR_ERRI); + register_clear_bits(&(CAN_obj->MSR), CAN_MSR_ERRI); // cppcheck-suppress selfAssignment ; needed to clear the register CAN_obj->MSR = CAN_obj->MSR; } diff --git a/board/drivers/llgpio.h b/board/drivers/llgpio.h index 9304cbe0105e22..0bd58c3b8a5316 100644 --- a/board/drivers/llgpio.h +++ b/board/drivers/llgpio.h @@ -15,16 +15,16 @@ void set_gpio_mode(GPIO_TypeDef *GPIO, unsigned int pin, unsigned int mode) { uint32_t tmp = GPIO->MODER; tmp &= ~(3U << (pin * 2U)); tmp |= (mode << (pin * 2U)); - GPIO->MODER = tmp; + register_set(&(GPIO->MODER), tmp, 0xFFFFFFFFU); EXIT_CRITICAL(); } void set_gpio_output(GPIO_TypeDef *GPIO, unsigned int pin, bool enabled) { ENTER_CRITICAL(); if (enabled) { - GPIO->ODR |= (1U << pin); + register_set_bits(&(GPIO->ODR), (1U << pin)); } else { - GPIO->ODR &= ~(1U << pin); + register_clear_bits(&(GPIO->ODR), (1U << pin)); } set_gpio_mode(GPIO, pin, MODE_OUTPUT); EXIT_CRITICAL(); @@ -33,9 +33,9 @@ void set_gpio_output(GPIO_TypeDef *GPIO, unsigned int pin, bool enabled) { 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); + register_set_bits(&(GPIO->OTYPER), (1U << pin)); } else { - GPIO->OTYPER &= ~(1U << pin); + register_clear_bits(&(GPIO->OTYPER), (1U << pin)); } EXIT_CRITICAL(); } @@ -45,7 +45,7 @@ void set_gpio_alternate(GPIO_TypeDef *GPIO, unsigned int pin, unsigned int mode) uint32_t tmp = GPIO->AFR[pin >> 3U]; tmp &= ~(0xFU << ((pin & 7U) * 4U)); tmp |= mode << ((pin & 7U) * 4U); - GPIO->AFR[pin >> 3] = tmp; + register_set(&(GPIO->AFR[pin >> 3]), tmp, 0xFFFFFFFFU); set_gpio_mode(GPIO, pin, MODE_ALTERNATE); EXIT_CRITICAL(); } @@ -55,7 +55,7 @@ void set_gpio_pullup(GPIO_TypeDef *GPIO, unsigned int pin, unsigned int mode) { uint32_t tmp = GPIO->PUPDR; tmp &= ~(3U << (pin * 2U)); tmp |= (mode << (pin * 2U)); - GPIO->PUPDR = tmp; + register_set(&(GPIO->PUPDR), tmp, 0xFFFFFFFFU); EXIT_CRITICAL(); } diff --git a/board/drivers/pwm.h b/board/drivers/pwm.h index d2e1652c1ce4ff..c3709200c1ce1e 100644 --- a/board/drivers/pwm.h +++ b/board/drivers/pwm.h @@ -1,53 +1,54 @@ #define PWM_COUNTER_OVERFLOW 2000U // To get ~50kHz +// TODO: Implement for 32-bit timers + void pwm_init(TIM_TypeDef *TIM, uint8_t channel){ // Enable timer and auto-reload - TIM->CR1 = TIM_CR1_CEN | TIM_CR1_ARPE; + register_set(&(TIM->CR1), TIM_CR1_CEN | TIM_CR1_ARPE, 0x3FU); // Set channel as PWM mode 1 and enable output switch(channel){ case 1U: - TIM->CCMR1 |= (TIM_CCMR1_OC1M_2 | TIM_CCMR1_OC1M_1 | TIM_CCMR1_OC1PE); - TIM->CCER |= TIM_CCER_CC1E; + register_set_bits(&(TIM->CCMR1), (TIM_CCMR1_OC1M_2 | TIM_CCMR1_OC1M_1 | TIM_CCMR1_OC1PE)); + register_set_bits(&(TIM->CCER), TIM_CCER_CC1E); break; case 2U: - TIM->CCMR1 |= (TIM_CCMR1_OC2M_2 | TIM_CCMR1_OC2M_1 | TIM_CCMR1_OC2PE); - TIM->CCER |= TIM_CCER_CC2E; + register_set_bits(&(TIM->CCMR1), (TIM_CCMR1_OC2M_2 | TIM_CCMR1_OC2M_1 | TIM_CCMR1_OC2PE)); + register_set_bits(&(TIM->CCER), TIM_CCER_CC2E); break; case 3U: - TIM->CCMR2 |= (TIM_CCMR2_OC3M_2 | TIM_CCMR2_OC3M_1 | TIM_CCMR2_OC3PE); - TIM->CCER |= TIM_CCER_CC3E; + register_set_bits(&(TIM->CCMR2), (TIM_CCMR2_OC3M_2 | TIM_CCMR2_OC3M_1 | TIM_CCMR2_OC3PE)); + register_set_bits(&(TIM->CCER), TIM_CCER_CC3E); break; case 4U: - TIM->CCMR2 |= (TIM_CCMR2_OC4M_2 | TIM_CCMR2_OC4M_1 | TIM_CCMR2_OC4PE); - TIM->CCER |= TIM_CCER_CC4E; + register_set_bits(&(TIM->CCMR2), (TIM_CCMR2_OC4M_2 | TIM_CCMR2_OC4M_1 | TIM_CCMR2_OC4PE)); + register_set_bits(&(TIM->CCER), TIM_CCER_CC4E); break; default: break; } // Set max counter value - TIM->ARR = PWM_COUNTER_OVERFLOW; + register_set(&(TIM->ARR), PWM_COUNTER_OVERFLOW, 0xFFFFU); // Update registers and clear counter TIM->EGR |= TIM_EGR_UG; } -// TODO: Implement for 32-bit timers void pwm_set(TIM_TypeDef *TIM, uint8_t channel, uint8_t percentage){ uint16_t comp_value = (((uint16_t) percentage * PWM_COUNTER_OVERFLOW) / 100U); switch(channel){ case 1U: - TIM->CCR1 = comp_value; + register_set(&(TIM->CCR1), comp_value, 0xFFFFU); break; case 2U: - TIM->CCR2 = comp_value; + register_set(&(TIM->CCR2), comp_value, 0xFFFFU); break; case 3U: - TIM->CCR3 = comp_value; + register_set(&(TIM->CCR3), comp_value, 0xFFFFU); break; case 4U: - TIM->CCR4 = comp_value; + register_set(&(TIM->CCR4), comp_value, 0xFFFFU); break; default: break; diff --git a/board/drivers/registers.h b/board/drivers/registers.h new file mode 100644 index 00000000000000..76748295d9f7be --- /dev/null +++ b/board/drivers/registers.h @@ -0,0 +1,81 @@ + +typedef struct reg { + volatile uint32_t *address; + uint32_t value; + uint32_t check_mask; +} reg; + +// 10 bit hash with 23 as a prime +#define REGISTER_MAP_SIZE 0x3FFU +#define HASHING_PRIME 23U +#define CHECK_COLLISION(hash, addr) (((uint32_t) register_map[hash].address != 0U) && (register_map[hash].address != addr)) + +reg register_map[REGISTER_MAP_SIZE]; + +// Hash spread in first and second iterations seems to be reasonable. +// See: tests/development/register_hashmap_spread.py +// Also, check the collision warnings in the debug output, and minimize those. +uint16_t hash_addr(uint32_t input){ + return (((input >> 16U) ^ ((((input + 1U) & 0xFFFFU) * HASHING_PRIME) & 0xFFFFU)) & REGISTER_MAP_SIZE); +} + +// Do not put bits in the check mask that get changed by the hardware +void register_set(volatile uint32_t *addr, uint32_t val, uint32_t mask){ + ENTER_CRITICAL() + // Set bits in register that are also in the mask + (*addr) = ((*addr) & (~mask)) | (val & mask); + + // Add these values to the map + uint16_t hash = hash_addr((uint32_t) addr); + uint16_t tries = REGISTER_MAP_SIZE; + while(CHECK_COLLISION(hash, addr) && (tries > 0U)) { hash = hash_addr((uint32_t) hash); tries--;} + if (tries != 0U){ + register_map[hash].address = addr; + register_map[hash].value = (register_map[hash].value & (~mask)) | (val & mask); + register_map[hash].check_mask |= mask; + } else { + #ifdef DEBUG_FAULTS + puts("Hash collision: address 0x"); puth((uint32_t) addr); puts("!\n"); + #endif + } + EXIT_CRITICAL() +} + +// Set individual bits. Also add them to the check_mask. +// Do not use this to change bits that get reset by the hardware +void register_set_bits(volatile uint32_t *addr, uint32_t val) { + return register_set(addr, val, val); +} + +// Clear individual bits. Also add them to the check_mask. +// Do not use this to clear bits that get set by the hardware +void register_clear_bits(volatile uint32_t *addr, uint32_t val) { + return register_set(addr, (~val), val); +} + +// To be called periodically +void check_registers(void){ + for(uint16_t i=0U; iBDCR & RCC_BDCR_MASK) != RCC_BDCR_OPTIONS){ puts("Initializing RTC\n"); // Reset backup domain - RCC->BDCR |= RCC_BDCR_BDRST; + register_set_bits(&(RCC->BDCR), RCC_BDCR_BDRST); // Disable write protection - PWR->CR |= PWR_CR_DBP; + register_set_bits(&(PWR->CR), PWR_CR_DBP); // Clear backup domain reset - RCC->BDCR &= ~(RCC_BDCR_BDRST); + register_clear_bits(&(RCC->BDCR), RCC_BDCR_BDRST); // Set RTC options - RCC->BDCR = RCC_BDCR_OPTIONS | (RCC->BDCR & (~RCC_BDCR_MASK)); + register_set(&(RCC->BDCR), RCC_BDCR_OPTIONS, RCC_BDCR_MASK); // Enable write protection - PWR->CR &= ~(PWR_CR_DBP); + register_clear_bits(&(PWR->CR), PWR_CR_DBP); } } } @@ -49,12 +49,12 @@ void rtc_set_time(timestamp_t time){ puts("Setting RTC time\n"); // Disable write protection - PWR->CR |= PWR_CR_DBP; + register_set_bits(&(PWR->CR), PWR_CR_DBP); RTC->WPR = 0xCA; RTC->WPR = 0x53; // Enable initialization mode - RTC->ISR |= RTC_ISR_INIT; + register_set_bits(&(RTC->ISR), RTC_ISR_INIT); while((RTC->ISR & RTC_ISR_INITF) == 0){} // Set time @@ -62,17 +62,17 @@ void rtc_set_time(timestamp_t time){ RTC->DR = (to_bcd(time.year - YEAR_OFFSET) << RTC_DR_YU_Pos) | (time.weekday << RTC_DR_WDU_Pos) | (to_bcd(time.month) << RTC_DR_MU_Pos) | (to_bcd(time.day) << RTC_DR_DU_Pos); // Set options - RTC->CR = 0U; + register_set(&(RTC->CR), 0U, 0xFCFFFFU); // Disable initalization mode - RTC->ISR &= ~(RTC_ISR_INIT); + register_clear_bits(&(RTC->ISR), RTC_ISR_INIT); // Wait for synchronization while((RTC->ISR & RTC_ISR_RSF) == 0){} // Re-enable write protection RTC->WPR = 0x00; - PWR->CR &= ~(PWR_CR_DBP); + register_clear_bits(&(PWR->CR), PWR_CR_DBP); } } diff --git a/board/drivers/spi.h b/board/drivers/spi.h index 86e795805a2d4c..29963b6bdfce77 100644 --- a/board/drivers/spi.h +++ b/board/drivers/spi.h @@ -12,20 +12,20 @@ int spi_total_count = 0; void spi_tx_dma(void *addr, int len) { // disable DMA - SPI1->CR2 &= ~SPI_CR2_TXDMAEN; - DMA2_Stream3->CR &= ~DMA_SxCR_EN; + register_clear_bits(&(SPI1->CR2), SPI_CR2_TXDMAEN); + register_clear_bits(&(DMA2_Stream3->CR), DMA_SxCR_EN); // DMA2, stream 3, channel 3 - DMA2_Stream3->M0AR = (uint32_t)addr; + register_set(&(DMA2_Stream3->M0AR), (uint32_t)addr, 0xFFFFFFFFU); DMA2_Stream3->NDTR = len; - DMA2_Stream3->PAR = (uint32_t)&(SPI1->DR); + register_set(&(DMA2_Stream3->PAR), (uint32_t)&(SPI1->DR), 0xFFFFFFFFU); // channel3, increment memory, memory -> periph, enable - DMA2_Stream3->CR = DMA_SxCR_CHSEL_1 | DMA_SxCR_CHSEL_0 | DMA_SxCR_MINC | DMA_SxCR_DIR_0 | DMA_SxCR_EN; + register_set(&(DMA2_Stream3->CR), (DMA_SxCR_CHSEL_1 | DMA_SxCR_CHSEL_0 | DMA_SxCR_MINC | DMA_SxCR_DIR_0 | DMA_SxCR_EN), 0x1E077EFEU); delay(0); - DMA2_Stream3->CR |= DMA_SxCR_TCIE; + register_set_bits(&(DMA2_Stream3->CR), DMA_SxCR_TCIE); - SPI1->CR2 |= SPI_CR2_TXDMAEN; + register_set_bits(&(SPI1->CR2), SPI_CR2_TXDMAEN); // signal data is ready by driving low // esp must be configured as input by this point @@ -34,24 +34,24 @@ void spi_tx_dma(void *addr, int len) { void spi_rx_dma(void *addr, int len) { // disable DMA - SPI1->CR2 &= ~SPI_CR2_RXDMAEN; - DMA2_Stream2->CR &= ~DMA_SxCR_EN; + register_clear_bits(&(SPI1->CR2), SPI_CR2_RXDMAEN); + register_clear_bits(&(DMA2_Stream2->CR), DMA_SxCR_EN); // drain the bus volatile uint8_t dat = SPI1->DR; (void)dat; // DMA2, stream 2, channel 3 - DMA2_Stream2->M0AR = (uint32_t)addr; + register_set(&(DMA2_Stream2->M0AR), (uint32_t)addr, 0xFFFFFFFFU); DMA2_Stream2->NDTR = len; - DMA2_Stream2->PAR = (uint32_t)&(SPI1->DR); + register_set(&(DMA2_Stream2->PAR), (uint32_t)&(SPI1->DR), 0xFFFFFFFFU); // channel3, increment memory, periph -> memory, enable - DMA2_Stream2->CR = DMA_SxCR_CHSEL_1 | DMA_SxCR_CHSEL_0 | DMA_SxCR_MINC | DMA_SxCR_EN; + register_set(&(DMA2_Stream2->CR), (DMA_SxCR_CHSEL_1 | DMA_SxCR_CHSEL_0 | DMA_SxCR_MINC | DMA_SxCR_EN), 0x1E077EFEU); delay(0); - DMA2_Stream2->CR |= DMA_SxCR_TCIE; + register_set_bits(&(DMA2_Stream2->CR), DMA_SxCR_TCIE); - SPI1->CR2 |= SPI_CR2_RXDMAEN; + register_set_bits(&(SPI1->CR2), SPI_CR2_RXDMAEN); } // ***************************** SPI IRQs ***************************** @@ -109,11 +109,11 @@ void spi_init(void) { REGISTER_INTERRUPT(EXTI4_IRQn, EXTI4_IRQ_Handler, 50000U, FAULT_INTERRUPT_RATE_SPI_CS) // TODO: Figure out if this is a reasonable limit //puts("SPI init\n"); - SPI1->CR1 = SPI_CR1_SPE; + register_set(&(SPI1->CR1), SPI_CR1_SPE, 0xFFFFU); // enable SPI interrupts //SPI1->CR2 = SPI_CR2_RXNEIE | SPI_CR2_ERRIE | SPI_CR2_TXEIE; - SPI1->CR2 = SPI_CR2_RXNEIE; + register_set(&(SPI1->CR2), SPI_CR2_RXNEIE, 0xF7U); NVIC_EnableIRQ(DMA2_Stream2_IRQn); NVIC_EnableIRQ(DMA2_Stream3_IRQn); @@ -124,8 +124,8 @@ void spi_init(void) { set_gpio_pullup(GPIOB, 0, PULL_UP); // setup interrupt on falling edge of SPI enable (on PA4) - SYSCFG->EXTICR[2] = SYSCFG_EXTICR2_EXTI4_PA; - EXTI->IMR |= (1U << 4); - EXTI->FTSR |= (1U << 4); + register_set(&(SYSCFG->EXTICR[2]), SYSCFG_EXTICR2_EXTI4_PA, 0xFFFFU); + register_set_bits(&(EXTI->IMR), (1U << 4)); + register_set_bits(&(EXTI->FTSR), (1U << 4)); NVIC_EnableIRQ(EXTI4_IRQn); } \ No newline at end of file diff --git a/board/drivers/timer.h b/board/drivers/timer.h index a14b619e4b81e3..d7aa7e8811093a 100644 --- a/board/drivers/timer.h +++ b/board/drivers/timer.h @@ -1,7 +1,7 @@ void timer_init(TIM_TypeDef *TIM, int psc) { - TIM->PSC = psc-1; - TIM->DIER = TIM_DIER_UIE; - TIM->CR1 = TIM_CR1_CEN; + register_set(&(TIM->PSC), (psc-1), 0xFFFFU); + register_set(&(TIM->DIER), TIM_DIER_UIE, 0x5F5FU); + register_set(&(TIM->CR1), TIM_CR1_CEN, 0x3FU); TIM->SR = 0; } diff --git a/board/faults.h b/board/faults.h index 9acba95f85214c..aa5db341b5c21b 100644 --- a/board/faults.h +++ b/board/faults.h @@ -21,6 +21,7 @@ #define FAULT_INTERRUPT_RATE_USB (1U << 15) #define FAULT_INTERRUPT_RATE_TIM1 (1U << 16) #define FAULT_INTERRUPT_RATE_TIM3 (1U << 17) +#define FAULT_REGISTER_DIVERGENT (1U << 18) // Permanent faults #define PERMANENT_FAULTS 0U diff --git a/board/gpio.h b/board/gpio.h index 322b0be9d06abe..2b937eced4cfdd 100644 --- a/board/gpio.h +++ b/board/gpio.h @@ -23,6 +23,9 @@ void early(void) { // Reset global critical depth global_critical_depth = 0; + // Init register and interrupt tables + init_registers(); + // neccesary for DFU flashing on a non-power cycled white panda enable_interrupts(); diff --git a/board/main.c b/board/main.c index f27e79e8d98c50..95203b9983cf09 100644 --- a/board/main.c +++ b/board/main.c @@ -6,11 +6,13 @@ #include "obj/gitversion.h" #include "main_declarations.h" +#include "critical.h" #include "libc.h" #include "provision.h" #include "faults.h" +#include "drivers/registers.h" #include "drivers/interrupts.h" #include "drivers/llcan.h" @@ -699,6 +701,9 @@ void TIM1_BRK_TIM9_IRQ_Handler(void) { } #endif + // check registers + check_registers(); + // set ignition_can to false after 2s of no CAN seen if (ignition_can_cnt > 2U) { ignition_can = false; @@ -713,7 +718,9 @@ void TIM1_BRK_TIM9_IRQ_Handler(void) { } int main(void) { + // Init interrupt table init_interrupts(true); + // 1s timer REGISTER_INTERRUPT(TIM1_BRK_TIM9_IRQn, TIM1_BRK_TIM9_IRQ_Handler, 2U, FAULT_INTERRUPT_RATE_TIM1) diff --git a/board/main_declarations.h b/board/main_declarations.h index f1e161da525188..363a992dbfe8a7 100644 --- a/board/main_declarations.h +++ b/board/main_declarations.h @@ -12,4 +12,4 @@ uint8_t hw_type = 0; const board *current_board; bool is_enumerated = 0; uint32_t heartbeat_counter = 0; -uint32_t uptime_cnt = 0; \ No newline at end of file +uint32_t uptime_cnt = 0; diff --git a/board/pedal/main.c b/board/pedal/main.c index 13becced96957a..667d27bf0f8d29 100644 --- a/board/pedal/main.c +++ b/board/pedal/main.c @@ -3,8 +3,10 @@ #include "libc.h" #include "main_declarations.h" +#include "critical.h" #include "faults.h" +#include "drivers/registers.h" #include "drivers/interrupts.h" #include "drivers/llcan.h" #include "drivers/llgpio.h" @@ -294,7 +296,9 @@ void pedal(void) { } int main(void) { + // Init interrupt table init_interrupts(true); + REGISTER_INTERRUPT(CAN1_TX_IRQn, CAN1_TX_IRQ_Handler, CAN_INTERRUPT_RATE, FAULT_INTERRUPT_RATE_CAN_1) REGISTER_INTERRUPT(CAN1_RX0_IRQn, CAN1_RX0_IRQ_Handler, CAN_INTERRUPT_RATE, FAULT_INTERRUPT_RATE_CAN_1) REGISTER_INTERRUPT(CAN1_SCE_IRQn, CAN1_SCE_IRQ_Handler, CAN_INTERRUPT_RATE, FAULT_INTERRUPT_RATE_CAN_1) diff --git a/board/power_saving.h b/board/power_saving.h index d397884f6e6a8f..3ee2170d71cf6a 100644 --- a/board/power_saving.h +++ b/board/power_saving.h @@ -1,3 +1,6 @@ +// WARNING: To stay in compliance with the SIL2 rules laid out in STM UM1840, we should never implement any of the available hardware low power modes. +// See rule: CoU_3 + #define POWER_SAVE_STATUS_DISABLED 0 #define POWER_SAVE_STATUS_ENABLED 1 diff --git a/tests/automated/2_ignition_orientation.py b/tests/automated/2_health.py similarity index 89% rename from tests/automated/2_ignition_orientation.py rename to tests/automated/2_health.py index 3537f7e0e3b6d2..da5eaec9d995ba 100644 --- a/tests/automated/2_ignition_orientation.py +++ b/tests/automated/2_health.py @@ -35,3 +35,10 @@ def test_orientation_detection(p): if (i == 0 and detected_harness_orientation != 0) or detected_harness_orientation in seen_orientations: assert False seen_orientations.append(detected_harness_orientation) + + +@test_all_pandas +@panda_connect_and_init +def test_voltage(p): + voltage = p.health()['voltage'] + assert ((voltage > 10000) and (voltage < 14000)) \ No newline at end of file diff --git a/tests/automated/helpers.py b/tests/automated/helpers.py index e9413c6ede2c8d..e6408b35099189 100644 --- a/tests/automated/helpers.py +++ b/tests/automated/helpers.py @@ -272,6 +272,11 @@ def wrapper(panda_serials=None, **kwargs): try: run_with_timeout(TIMEOUT, fn, *pandas, **kwargs) + + # Check if the pandas did not throw any faults while running test + for panda in pandas: + panda.reconnect() + assert panda.health()['fault_status'] == 0 except Exception as e: raise e finally: diff --git a/tests/development/register_hashmap_spread.py b/tests/development/register_hashmap_spread.py new file mode 100755 index 00000000000000..ba0a0eb29d8fde --- /dev/null +++ b/tests/development/register_hashmap_spread.py @@ -0,0 +1,50 @@ +#!/usr/bin/env python3 +import matplotlib.pyplot as plt # pylint: disable=import-error + +HASHING_PRIME = 23 +REGISTER_MAP_SIZE = 0x3FF +BYTES_PER_REG = 4 + +# From ST32F413 datasheet +REGISTER_ADDRESS_REGIONS = [ + (0x40000000, 0x40007FFF), + (0x40010000, 0x400107FF), + (0x40011000, 0x400123FF), + (0x40012C00, 0x40014BFF), + (0x40015000, 0x400153FF), + (0x40015800, 0x40015BFF), + (0x40016000, 0x400167FF), + (0x40020000, 0x40021FFF), + (0x40023000, 0x400233FF), + (0x40023800, 0x40023FFF), + (0x40026000, 0x400267FF), + (0x50000000, 0x5003FFFF), + (0x50060000, 0x500603FF), + (0x50060800, 0x50060BFF), + (0x50060800, 0x50060BFF), + (0xE0000000, 0xE00FFFFF) +] + +def hash(reg_addr): + return (((reg_addr >> 16) ^ ((((reg_addr + 1) & 0xFFFF) * HASHING_PRIME) & 0xFFFF)) & REGISTER_MAP_SIZE) + +# Calculate hash for each address +hashes = [] +double_hashes = [] +for (start_addr, stop_addr) in REGISTER_ADDRESS_REGIONS: + for addr in range(start_addr, stop_addr+1, BYTES_PER_REG): + h = hash(addr) + hashes.append(h) + double_hashes.append(hash(h)) + +# Make histograms +plt.subplot(2, 1, 1) +plt.hist(hashes, bins=REGISTER_MAP_SIZE) +plt.title("Number of collisions per hash") +plt.xlabel("Address") + +plt.subplot(2, 1, 2) +plt.hist(double_hashes, bins=REGISTER_MAP_SIZE) +plt.title("Number of collisions per double hash") +plt.xlabel("Address") +plt.show()