From ae22305869f382c8a1cc088fbf3cb12bcc2d5a6c Mon Sep 17 00:00:00 2001
From: Hierophect <hierophect@gmail.com>
Date: Wed, 8 Jan 2020 15:55:53 -0500
Subject: [PATCH] Corrections to I2C, style

---
 ports/stm32f4/common-hal/busio/I2C.c      |  25 ++----
 ports/stm32f4/common-hal/busio/SPI.c      |   5 +-
 ports/stm32f4/common-hal/busio/UART.c     |   1 +
 ports/stm32f4/common-hal/pulseio/PWMOut.c | 104 +++++++++++++++-------
 4 files changed, 80 insertions(+), 55 deletions(-)

diff --git a/ports/stm32f4/common-hal/busio/I2C.c b/ports/stm32f4/common-hal/busio/I2C.c
index 46c9728a8b..37c13a9c4d 100644
--- a/ports/stm32f4/common-hal/busio/I2C.c
+++ b/ports/stm32f4/common-hal/busio/I2C.c
@@ -35,6 +35,7 @@
 #include "supervisor/shared/translate.h"
 #include "common-hal/microcontroller/Pin.h"
 
+//arrays use 0 based numbering: I2C1 is stored at index 0
 #define MAX_I2C 3
 STATIC bool reserved_i2c[MAX_I2C];
 STATIC bool never_reset_i2c[MAX_I2C];
@@ -93,10 +94,6 @@ void common_hal_busio_i2c_construct(busio_i2c_obj_t *self,
         }
     }
 
-    //Note: clock reset must be before GPIO init, due to I2C soft reboot issue
-    i2c_clock_enable(1<<(self->sda->i2c_index - 1));
-    reserved_i2c[self->sda->i2c_index - 1] = true;
-
     //Start GPIO for each pin
     GPIO_InitTypeDef GPIO_InitStruct = {0};
     GPIO_InitStruct.Pin = pin_mask(sda->number);
@@ -113,22 +110,9 @@ void common_hal_busio_i2c_construct(busio_i2c_obj_t *self,
     GPIO_InitStruct.Alternate = self->scl->altfn_index; 
     HAL_GPIO_Init(pin_port(scl->port), &GPIO_InitStruct);
 
-    //still needed? 
-    #ifdef I2C1
-    __HAL_RCC_I2C1_FORCE_RESET();
-    HAL_Delay(2);
-    __HAL_RCC_I2C1_RELEASE_RESET();
-    #endif
-    #ifdef I2C2
-    __HAL_RCC_I2C2_FORCE_RESET();
-    HAL_Delay(2);
-    __HAL_RCC_I2C2_RELEASE_RESET();
-    #endif
-    #ifdef I2C2
-    __HAL_RCC_I2C3_FORCE_RESET();
-    HAL_Delay(2);
-    __HAL_RCC_I2C3_RELEASE_RESET();
-    #endif
+    //Note: clock reset must be before GPIO init, due to I2C soft reboot issue
+    i2c_clock_enable(1<<(self->sda->i2c_index - 1));
+    reserved_i2c[self->sda->i2c_index - 1] = true;
 
     self->handle.Instance = I2Cx;
     self->handle.Init.ClockSpeed = 100000;
@@ -139,6 +123,7 @@ void common_hal_busio_i2c_construct(busio_i2c_obj_t *self,
     self->handle.Init.OwnAddress2 = 0;
     self->handle.Init.GeneralCallMode = I2C_GENERALCALL_DISABLE;
     self->handle.Init.NoStretchMode = I2C_NOSTRETCH_DISABLE;
+    self->handle.State = HAL_I2C_STATE_RESET;
     if(HAL_I2C_Init(&(self->handle)) != HAL_OK) {
         mp_raise_RuntimeError(translate("I2C Init Error"));
     }
diff --git a/ports/stm32f4/common-hal/busio/SPI.c b/ports/stm32f4/common-hal/busio/SPI.c
index 0d1bcb168e..78c6352636 100644
--- a/ports/stm32f4/common-hal/busio/SPI.c
+++ b/ports/stm32f4/common-hal/busio/SPI.c
@@ -39,13 +39,12 @@
 // Note that any bugs introduced in this file can cause crashes at startup
 // for chips using external SPI flash.
 
-#define MAX_SPI 6 //TODO; replace this as part of periph cleanup
-#define ALL_CLOCKS 0xFF
-
 //arrays use 0 based numbering: SPI1 is stored at index 0
+#define MAX_SPI 6
 STATIC bool reserved_spi[MAX_SPI];
 STATIC bool never_reset_spi[MAX_SPI];
 
+#define ALL_CLOCKS 0xFF
 STATIC void spi_clock_enable(uint8_t mask);
 STATIC void spi_clock_disable(uint8_t mask);
 
diff --git a/ports/stm32f4/common-hal/busio/UART.c b/ports/stm32f4/common-hal/busio/UART.c
index 44ef5efdd2..a3e2d96beb 100644
--- a/ports/stm32f4/common-hal/busio/UART.c
+++ b/ports/stm32f4/common-hal/busio/UART.c
@@ -40,6 +40,7 @@
 
 #define ALL_UARTS 0xFFFF
 
+//arrays use 0 based numbering: UART1 is stored at index 0
 STATIC bool reserved_uart[MAX_UART];
 int errflag; //Used to restart read halts
 
diff --git a/ports/stm32f4/common-hal/pulseio/PWMOut.c b/ports/stm32f4/common-hal/pulseio/PWMOut.c
index 2a071d0784..309848236d 100644
--- a/ports/stm32f4/common-hal/pulseio/PWMOut.c
+++ b/ports/stm32f4/common-hal/pulseio/PWMOut.c
@@ -44,10 +44,6 @@ STATIC bool never_reset_tim[TIM_BANK_ARRAY_LEN];
 STATIC void tim_clock_enable(uint16_t mask);
 STATIC void tim_clock_disable(uint16_t mask);
 
-//--------
-//STATICS
-//--------
-
 // Get the frequency (in Hz) of the source clock for the given timer.
 // On STM32F405/407/415/417 there are 2 cases for how the clock freq is set.
 // If the APB prescaler is 1, then the timer clock is equal to its respective
@@ -91,10 +87,6 @@ STATIC void timer_get_optimal_divisors(uint32_t*period, uint32_t*prescaler,
     }
 }
 
-//--------
-//COMMON HAL
-//--------
-
 void pwmout_reset(void) {
     uint16_t never_reset_mask = 0x00;
     for(int i=0;i<TIM_BANK_ARRAY_LEN;i++) {
@@ -328,80 +320,128 @@ bool common_hal_pulseio_pwmout_get_variable_frequency(pulseio_pwmout_obj_t* self
 
 STATIC void tim_clock_enable(uint16_t mask) {
     #ifdef TIM1
-    if (mask & 1<<0) __HAL_RCC_TIM1_CLK_ENABLE();
+    if (mask & 1<<0) {
+        __HAL_RCC_TIM1_CLK_ENABLE();
+    }
     #endif
     #ifdef TIM2
-    if (mask & 1<<1) __HAL_RCC_TIM2_CLK_ENABLE();
+    if (mask & 1<<1) {
+        __HAL_RCC_TIM2_CLK_ENABLE();
+    }
     #endif
     #ifdef TIM3
-    if (mask & 1<<2) __HAL_RCC_TIM3_CLK_ENABLE();
+    if (mask & 1<<2) {
+        __HAL_RCC_TIM3_CLK_ENABLE();
+    }
     #endif
     #ifdef TIM4
-    if (mask & 1<<3) __HAL_RCC_TIM4_CLK_ENABLE();
+    if (mask & 1<<3) {
+        __HAL_RCC_TIM4_CLK_ENABLE();
+    }
     #endif
     #ifdef TIM5
-    if (mask & 1<<4) __HAL_RCC_TIM5_CLK_ENABLE();
+    if (mask & 1<<4) {
+        __HAL_RCC_TIM5_CLK_ENABLE();
+    }
     #endif
     //6 and 7 are reserved ADC timers
     #ifdef TIM8
-    if (mask & 1<<7) __HAL_RCC_TIM8_CLK_ENABLE();
+    if (mask & 1<<7) {
+        __HAL_RCC_TIM8_CLK_ENABLE();
+    }
     #endif
     #ifdef TIM9
-    if (mask & 1<<8) __HAL_RCC_TIM9_CLK_ENABLE();
+    if (mask & 1<<8) {
+        __HAL_RCC_TIM9_CLK_ENABLE();
+    }
     #endif
     #ifdef TIM10
-    if (mask & 1<<9) __HAL_RCC_TIM10_CLK_ENABLE();
+    if (mask & 1<<9) {
+        __HAL_RCC_TIM10_CLK_ENABLE();
+    }
     #endif
     #ifdef TIM11
-    if (mask & 1<<10) __HAL_RCC_TIM11_CLK_ENABLE();
+    if (mask & 1<<10) {
+        __HAL_RCC_TIM11_CLK_ENABLE();
+    }
     #endif
     #ifdef TIM12
-    if (mask & 1<<11) __HAL_RCC_TIM12_CLK_ENABLE();
+    if (mask & 1<<11) {
+        __HAL_RCC_TIM12_CLK_ENABLE();
+    }
     #endif
     #ifdef TIM13
-    if (mask & 1<<12) __HAL_RCC_TIM13_CLK_ENABLE();
+    if (mask & 1<<12) {
+        __HAL_RCC_TIM13_CLK_ENABLE();
+    }
     #endif
     #ifdef TIM14
-    if (mask & 1<<13) __HAL_RCC_TIM14_CLK_ENABLE();
+    if (mask & 1<<13) {
+        __HAL_RCC_TIM14_CLK_ENABLE();
+    }
     #endif
 }
 
 STATIC void tim_clock_disable(uint16_t mask) {
     #ifdef TIM1
-    if (mask & 1<<0) __HAL_RCC_TIM1_CLK_DISABLE();
+    if (mask & 1<<0) {
+        __HAL_RCC_TIM1_CLK_DISABLE();
+    }
     #endif
     #ifdef TIM2
-    if (mask & 1<<1) __HAL_RCC_TIM2_CLK_DISABLE();
+    if (mask & 1<<1) {
+        __HAL_RCC_TIM2_CLK_DISABLE();
+    }
     #endif
     #ifdef TIM3
-    if (mask & 1<<2) __HAL_RCC_TIM3_CLK_DISABLE();
+    if (mask & 1<<2) {
+        __HAL_RCC_TIM3_CLK_DISABLE();
+    }
     #endif
     #ifdef TIM4
-    if (mask & 1<<3) __HAL_RCC_TIM4_CLK_DISABLE();
+    if (mask & 1<<3) {
+        __HAL_RCC_TIM4_CLK_DISABLE();
+    }
     #endif
     #ifdef TIM5
-    if (mask & 1<<4) __HAL_RCC_TIM5_CLK_DISABLE();
+    if (mask & 1<<4) {
+        __HAL_RCC_TIM5_CLK_DISABLE();
+    }
     #endif
     //6 and 7 are reserved ADC timers
     #ifdef TIM8
-    if (mask & 1<<7) __HAL_RCC_TIM8_CLK_DISABLE();
+    if (mask & 1<<7) {
+        __HAL_RCC_TIM8_CLK_DISABLE();
+    }
     #endif
     #ifdef TIM9
-    if (mask & 1<<8) __HAL_RCC_TIM9_CLK_DISABLE();
+    if (mask & 1<<8) {
+        __HAL_RCC_TIM9_CLK_DISABLE();
+    }
     #endif
     #ifdef TIM10
-    if (mask & 1<<9) __HAL_RCC_TIM10_CLK_DISABLE();
+    if (mask & 1<<9) {
+        __HAL_RCC_TIM10_CLK_DISABLE();
+    }
     #endif
     #ifdef TIM11
-    if (mask & 1<<10) __HAL_RCC_TIM11_CLK_DISABLE();
+    if (mask & 1<<10) {
+        __HAL_RCC_TIM11_CLK_DISABLE();
+    }
     #endif
     #ifdef TIM12
-    if (mask & 1<<11) __HAL_RCC_TIM12_CLK_DISABLE();
+    if (mask & 1<<11) {
+        __HAL_RCC_TIM12_CLK_DISABLE();
+    }
     #endif
     #ifdef TIM13
-    if (mask & 1<<12) __HAL_RCC_TIM13_CLK_DISABLE();
+    if (mask & 1<<12) {
+        __HAL_RCC_TIM13_CLK_DISABLE();
+    }
     #endif
     #ifdef TIM14
-    if (mask & 1<<13) __HAL_RCC_TIM14_CLK_DISABLE();
+    if (mask & 1<<13) {
+        __HAL_RCC_TIM14_CLK_DISABLE();
+    }
     #endif
 }