Revert "Register readback on most modules. Still need to convert the …

…other ones (commaai#396)" This reverts commit 893e486.
rafcabezas · Dec 5, 2019 · 56ec215 · 56ec215
1 parent 893e486
commit 56ec215
Show file tree

Hide file tree

Showing 25 changed files with 153 additions and 306 deletions.
diff --git a/VERSION b/VERSION
@@ -1 +1 @@
-v1.6.9
+v1.6.8
diff --git a/board/bootstub.c b/board/bootstub.c
@@ -29,10 +29,8 @@ 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"
@@ -69,8 +67,7 @@ extern void *_app_start[];
 // BOUNTY: $200 coupon on shop.comma.ai or $100 check.
 
 int main(void) {
-  // Init interrupt table
-  init_interrupts(true);
+  init_interrupts(false);
 
   disable_interrupts();
   clock_init();

diff --git a/board/critical.h b/board/critical.h
diff --git a/board/drivers/adc.h b/board/drivers/adc.h
@@ -9,16 +9,26 @@
 #define ADCCHAN_CURRENT 13
 
 void adc_init(void) {
-  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);
+  // 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;
 }
 
 uint32_t adc_get(unsigned int channel) {
-  // Select channel
-  register_set(&(ADC1->JSQR), (channel << 15U), 0x3FFFFFU);
+  // includes length
+  //ADC1->SQR1 = 0;
+
+  // select channel
+  ADC1->JSQR = channel << 15;
+
+  //ADC1->CR1 = ADC_CR1_DISCNUM_0;
+  //ADC1->CR1 = ADC_CR1_EOCIE;
 
-  // 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
@@ -1,40 +1,40 @@
 void clock_init(void) {
   // enable external oscillator
-  register_set_bits(&(RCC->CR), RCC_CR_HSEON);
+  RCC->CR |= RCC_CR_HSEON;
   while ((RCC->CR & RCC_CR_HSERDY) == 0);
 
   // divide things
-  register_set(&(RCC->CFGR), RCC_CFGR_HPRE_DIV1 | RCC_CFGR_PPRE2_DIV2 | RCC_CFGR_PPRE1_DIV4, 0xFF7FFCF3U);
+  RCC->CFGR = RCC_CFGR_HPRE_DIV1 | RCC_CFGR_PPRE2_DIV2 | RCC_CFGR_PPRE1_DIV4;
 
   // 16mhz crystal
-  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);
+  RCC->PLLCFGR = RCC_PLLCFGR_PLLQ_2 | RCC_PLLCFGR_PLLM_3 |
+                 RCC_PLLCFGR_PLLN_6 | RCC_PLLCFGR_PLLN_5 | RCC_PLLCFGR_PLLSRC_HSE;
 
   // start PLL
-  register_set_bits(&(RCC->CR), RCC_CR_PLLON);
+  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 ***
-  register_set(&(FLASH->ACR), FLASH_ACR_ICEN | FLASH_ACR_DCEN | FLASH_ACR_LATENCY_5WS, 0x1F0FU);
+  FLASH->ACR = FLASH_ACR_ICEN | FLASH_ACR_DCEN | FLASH_ACR_LATENCY_5WS;
 
   // switch to PLL
-  register_set_bits(&(RCC->CFGR), RCC_CFGR_SW_PLL);
+  RCC->CFGR |= RCC_CFGR_SW_PLL;
   while ((RCC->CFGR & RCC_CFGR_SWS) != RCC_CFGR_SWS_PLL);
 
   // *** running on PLL ***
 }
 
 void watchdog_init(void) {
   // setup watchdog
-  IWDG->KR = 0x5555U;
-  register_set(&(IWDG->PR), 0x0U, 0x7U);  // divider/4
-
+  IWDG->KR = 0x5555;
+  IWDG->PR = 0;          // divider /4
   // 0 = 0.125 ms, let's have a 50ms watchdog
-  register_set(&(IWDG->RLR), (400U-1U), 0xFFFU);
-  IWDG->KR = 0xCCCCU;
+  IWDG->RLR = 400 - 1;
+  IWDG->KR = 0xCCCC;
 }
 
 void watchdog_feed(void) {
-  IWDG->KR = 0xAAAAU;
+  IWDG->KR = 0xAAAA;
 }
 
diff --git a/board/drivers/dac.h b/board/drivers/dac.h
@@ -2,19 +2,22 @@ void puth(unsigned int i);
 void puts(const char *a);
 
 void dac_init(void) {
-  // 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);
+  // 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;
 }
 
 void dac_set(int channel, uint32_t value) {
   if (channel == 0) {
-    register_set(&(DAC->DHR12R1), value, 0xFFFU);
+    DAC->DHR12R1 = value;
   } else if (channel == 1) {
-    register_set(&(DAC->DHR12R2), value, 0xFFFU);
+    DAC->DHR12R2 = value;
   } else {
-    puts("Failed to set DAC: invalid channel value: 0x"); puth(value); puts("\n");
+    puts("Failed to set DAC: invalid channel value: ");
+    puth(value);
+    puts("\n");
   }
 }
 
diff --git 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
-    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));
+    SYSCFG->EXTICR[0] = SYSCFG_EXTICR1_EXTI2_PD;
+    EXTI->IMR |= (1U << 2);
+    EXTI->RTSR |= (1U << 2);
+    EXTI->FTSR |= (1U << 2);
     NVIC_EnableIRQ(EXTI2_IRQn);
 }
diff --git 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
-  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
+  TIM4->PSC = 48-1;          // tick on 1 us
+  TIM4->CR1 = TIM_CR1_CEN;   // enable
+  TIM4->ARR = 30-1;          // 33.3 kbps
 
   // in case it's disabled
   NVIC_EnableIRQ(TIM4_IRQn);
 
   // run the interrupt
-  register_set(&(TIM4->DIER), TIM_DIER_UIE, 0x5F5FU); // Update interrupt
+  TIM4->DIER = TIM_DIER_UIE; // update interrupt
   TIM4->SR = 0;
 }
 
@@ -171,9 +171,9 @@ void reset_gmlan_switch_timeout(void) {
 
 void set_bitbanged_gmlan(int val) {
   if (val != 0) {
-    register_set_bits(&(GPIOB->ODR), (1U << 13));
+    GPIOB->ODR |= (1U << 13);
   } else {
-    register_clear_bits(&(GPIOB->ODR), (1U << 13));
+    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);
-        register_clear_bits(&(TIM4->DIER), TIM_DIER_UIE); // No update interrupt
-        register_set(&(TIM4->CR1), 0U, 0x3FU); // Disable timer
+        TIM4->DIER = 0;  // no update interrupt
+        TIM4->CR1 = 0;   // disable timer
         gmlan_sendmax = -1;   // exit
       }
     }

diff --git a/board/drivers/interrupts.h b/board/drivers/interrupts.h
@@ -1,3 +1,29 @@
+// ********************* 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);
@@ -53,11 +79,11 @@ void init_interrupts(bool check_rate_limit){
   }
 
   // Init timer 10 for a 1s interval
-  register_set_bits(&(RCC->APB1ENR), RCC_APB1ENR_TIM6EN);  // Enable interrupt timer peripheral
+  RCC->APB1ENR |= RCC_APB1ENR_TIM6EN;  // enable interrupt timer peripheral
   REGISTER_INTERRUPT(TIM6_DAC_IRQn, TIM6_DAC_IRQ_Handler, 1, FAULT_INTERRUPT_RATE_INTERRUPTS)
-  register_set(&(TIM6->PSC), (732-1), 0xFFFFU);
-  register_set(&(TIM6->DIER), TIM_DIER_UIE, 0x5F5FU);
-  register_set(&(TIM6->CR1), TIM_CR1_CEN, 0x3FU);
+  TIM6->PSC = 732-1;
+  TIM6->DIER = TIM_DIER_UIE;
+  TIM6->CR1 = TIM_CR1_CEN;
   TIM6->SR = 0;
   NVIC_EnableIRQ(TIM6_DAC_IRQn);
 }

diff --git a/board/drivers/llcan.h b/board/drivers/llcan.h
@@ -19,24 +19,25 @@ void puts(const char *a);
 
 bool llcan_set_speed(CAN_TypeDef *CAN_obj, uint32_t speed, bool loopback, bool silent) {
   // initialization mode
-  register_set(&(CAN_obj->MCR), CAN_MCR_TTCM | CAN_MCR_INRQ, 0x180FFU);
+  CAN_obj->MCR = CAN_MCR_TTCM | CAN_MCR_INRQ;
   while((CAN_obj->MSR & CAN_MSR_INAK) != CAN_MSR_INAK);
 
   // set time quanta from defines
-  register_set(&(CAN_obj->BTR), ((CAN_BTR_TS1_0 * (CAN_SEQ1-1)) |
+  CAN_obj->BTR = (CAN_BTR_TS1_0 * (CAN_SEQ1-1)) |
             (CAN_BTR_TS2_0 * (CAN_SEQ2-1)) |
-            (can_speed_to_prescaler(speed) - 1U)), 0xC37F03FFU);
+            (can_speed_to_prescaler(speed) - 1U);
 
   // silent loopback mode for debugging
   if (loopback) {
-    register_set_bits(&(CAN_obj->BTR), CAN_BTR_SILM | CAN_BTR_LBKM);
+    CAN_obj->BTR |= CAN_BTR_SILM | CAN_BTR_LBKM;
   }
   if (silent) {
-    register_set_bits(&(CAN_obj->BTR), CAN_BTR_SILM);
+    CAN_obj->BTR |= CAN_BTR_SILM;
   }
 
   // reset
-  register_set(&(CAN_obj->MCR), CAN_MCR_TTCM | CAN_MCR_ABOM, 0x180FFU);
+  // cppcheck-suppress redundantAssignment ; it's a register
+  CAN_obj->MCR = CAN_MCR_TTCM | CAN_MCR_ABOM;
 
   #define CAN_TIMEOUT 1000000
   int tmp = 0;
@@ -50,25 +51,20 @@ bool llcan_set_speed(CAN_TypeDef *CAN_obj, uint32_t speed, bool loopback, bool s
 }
 
 void llcan_init(CAN_TypeDef *CAN_obj) {
-  // 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)) {}
+  // accept all filter
+  CAN_obj->FMR |= CAN_FMR_FINIT;
 
   // no mask
-  // 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->sFilterRegister[0].FR1 = 0;
+  CAN_obj->sFilterRegister[0].FR2 = 0;
+  CAN_obj->sFilterRegister[14].FR1 = 0;
+  CAN_obj->sFilterRegister[14].FR2 = 0;
   CAN_obj->FA1R |= 1U | (1U << 14);
 
-  // Exit init mode, do not wait
-  register_clear_bits(&(CAN_obj->FMR), CAN_FMR_FINIT);
+  CAN_obj->FMR &= ~(CAN_FMR_FINIT);
 
   // enable certain CAN interrupts
-  register_set_bits(&(CAN_obj->IER), CAN_IER_TMEIE | CAN_IER_FMPIE0 |  CAN_IER_WKUIE);
+  CAN_obj->IER |= CAN_IER_TMEIE | CAN_IER_FMPIE0 |  CAN_IER_WKUIE;
 
   if (CAN_obj == CAN1) {
     NVIC_EnableIRQ(CAN1_TX_IRQn);
@@ -91,7 +87,7 @@ void llcan_init(CAN_TypeDef *CAN_obj) {
 
 void llcan_clear_send(CAN_TypeDef *CAN_obj) {
   CAN_obj->TSR |= CAN_TSR_ABRQ0;
-  register_clear_bits(&(CAN_obj->MSR), CAN_MSR_ERRI);
+  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
@@ -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));
-  register_set(&(GPIO->MODER), tmp, 0xFFFFFFFFU);
+  GPIO->MODER = tmp;
   EXIT_CRITICAL();
 }
 
 void set_gpio_output(GPIO_TypeDef *GPIO, unsigned int pin, bool enabled) {
   ENTER_CRITICAL();
   if (enabled) {
-    register_set_bits(&(GPIO->ODR), (1U << pin));
+    GPIO->ODR |= (1U << pin);
   } else {
-    register_clear_bits(&(GPIO->ODR), (1U << pin));
+    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) {
-    register_set_bits(&(GPIO->OTYPER), (1U << pin));
+    GPIO->OTYPER |= (1U << pin);
   } else {
-    register_clear_bits(&(GPIO->OTYPER), (1U << pin));
+    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);
-  register_set(&(GPIO->AFR[pin >> 3]), tmp, 0xFFFFFFFFU);
+  GPIO->AFR[pin >> 3] = tmp;
   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));
-  register_set(&(GPIO->PUPDR), tmp, 0xFFFFFFFFU);
+  GPIO->PUPDR = tmp;
   EXIT_CRITICAL();
 }