Skip to content

Commit 7775513

Browse files
hierophecthierophect
authored
Merge pull request #2463 from hierophect/stm32-i2c-rework
STM32: I2C fix & general busio cleanup
2 parents b5df5ce + 5aae8df commit 7775513

File tree

4 files changed

+353
-252
lines changed

4 files changed

+353
-252
lines changed

‎ports/stm32f4/common-hal/busio/I2C.c

Lines changed: 102 additions & 100 deletions
Original file line numberDiff line numberDiff line change
@@ -35,51 +35,46 @@
3535
#include "supervisor/shared/translate.h"
3636
#include "common-hal/microcontroller/Pin.h"
3737

38-
STATIC bool reserved_i2c[3];
39-
STATIC bool never_reset[3];
38+
//arrays use 0 based numbering: I2C1 is stored at index 0
39+
#define MAX_I2C 3
40+
STATIC bool reserved_i2c[MAX_I2C];
41+
STATIC bool never_reset_i2c[MAX_I2C];
4042

41-
void i2c_reset(void) {
42-
//Note: I2Cs are also forcibly reset in construct, due to silicon error
43-
#ifdef I2C1
44-
reserved_i2c[0] = false;
45-
__HAL_RCC_I2C1_CLK_DISABLE();
46-
#endif
47-
#ifdef I2C2
48-
reserved_i2c[1] = false;
49-
__HAL_RCC_I2C2_CLK_DISABLE();
50-
#endif
51-
#ifdef I2C3
52-
reserved_i2c[2] = false;
53-
__HAL_RCC_I2C3_CLK_DISABLE();
54-
#endif
55-
}
56-
57-
void common_hal_busio_i2c_never_reset(busio_i2c_obj_t *self) {
58-
for (size_t i = 0 ; i < MP_ARRAY_SIZE(mcu_i2c_banks); i++) {
59-
if (self->handle.Instance == mcu_i2c_banks[i]) {
60-
never_reset[i] = true;
43+
#define ALL_CLOCKS 0xFF
44+
STATIC void i2c_clock_enable(uint8_t mask);
45+
STATIC void i2c_clock_disable(uint8_t mask);
6146

62-
never_reset_pin_number(self->scl->pin->port, self->scl->pin->number);
63-
never_reset_pin_number(self->sda->pin->port, self->scl->pin->number);
64-
break;
47+
void i2c_reset(void) {
48+
uint16_t never_reset_mask = 0x00;
49+
for(int i = 0; i < MAX_I2C; i++) {
50+
if (!never_reset_i2c[i]) {
51+
reserved_i2c[i] = false;
52+
} else {
53+
never_reset_mask |= 1 << i;
6554
}
6655
}
56+
i2c_clock_disable(ALL_CLOCKS & ~(never_reset_mask));
6757
}
6858

69-
7059
void common_hal_busio_i2c_construct(busio_i2c_obj_t *self,
7160
const mcu_pin_obj_t* scl, const mcu_pin_obj_t* sda, uint32_t frequency, uint32_t timeout) {
7261

7362
//match pins to I2C objects
7463
I2C_TypeDef * I2Cx;
64+
uint8_t sda_len = MP_ARRAY_SIZE(mcu_i2c_sda_list);
65+
uint8_t scl_len = MP_ARRAY_SIZE(mcu_i2c_scl_list);
66+
bool i2c_taken = false;
7567

76-
uint8_t sda_len = sizeof(mcu_i2c_sda_list)/sizeof(*mcu_i2c_sda_list);
77-
uint8_t scl_len = sizeof(mcu_i2c_scl_list)/sizeof(*mcu_i2c_scl_list);
78-
for(uint i=0; i<sda_len;i++) {
68+
for (uint i = 0; i < sda_len; i++) {
7969
if (mcu_i2c_sda_list[i].pin == sda) {
80-
for(uint j=0; j<scl_len;j++) {
70+
for (uint j = 0; j < scl_len; j++) {
8171
if ((mcu_i2c_scl_list[j].pin == scl)
8272
&& (mcu_i2c_scl_list[j].i2c_index == mcu_i2c_sda_list[i].i2c_index)) {
73+
//keep looking if the I2C is taken, could be another SCL that works
74+
if (reserved_i2c[mcu_i2c_scl_list[i].i2c_index - 1]) {
75+
i2c_taken = true;
76+
continue;
77+
}
8378
self->scl = &mcu_i2c_scl_list[j];
8479
self->sda = &mcu_i2c_sda_list[i];
8580
break;
@@ -89,14 +84,14 @@ void common_hal_busio_i2c_construct(busio_i2c_obj_t *self,
8984
}
9085

9186
//handle typedef selection, errors
92-
if(self->sda!=NULL && self->scl!=NULL ) {
93-
I2Cx = mcu_i2c_banks[self->sda->i2c_index-1];
87+
if (self->sda != NULL && self->scl != NULL ) {
88+
I2Cx = mcu_i2c_banks[self->sda->i2c_index - 1];
9489
} else {
95-
mp_raise_RuntimeError(translate("Invalid I2C pin selection"));
96-
}
97-
98-
if(reserved_i2c[self->sda->i2c_index-1]) {
99-
mp_raise_RuntimeError(translate("Hardware busy, try alternative pins"));
90+
if (i2c_taken) {
91+
mp_raise_ValueError(translate("Hardware busy, try alternative pins"));
92+
} else {
93+
mp_raise_ValueError(translate("Invalid I2C pin selection"));
94+
}
10095
}
10196

10297
//Start GPIO for each pin
@@ -115,44 +110,9 @@ void common_hal_busio_i2c_construct(busio_i2c_obj_t *self,
115110
GPIO_InitStruct.Alternate = self->scl->altfn_index;
116111
HAL_GPIO_Init(pin_port(scl->port), &GPIO_InitStruct);
117112

118-
//Fix for HAL error caused by soft reboot GPIO init SDA pin voltage drop. See Eratta.
119-
//Must be in this exact spot or I2C will get stuck in infinite loop.
120-
//TODO: See git issue #2172
121-
#ifdef I2C1
122-
__HAL_RCC_I2C1_FORCE_RESET();
123-
HAL_Delay(2);
124-
__HAL_RCC_I2C1_RELEASE_RESET();
125-
#endif
126-
#ifdef I2C2
127-
__HAL_RCC_I2C2_FORCE_RESET();
128-
HAL_Delay(2);
129-
__HAL_RCC_I2C2_RELEASE_RESET();
130-
#endif
131-
#ifdef I2C3
132-
__HAL_RCC_I2C3_FORCE_RESET();
133-
HAL_Delay(2);
134-
__HAL_RCC_I2C3_RELEASE_RESET();
135-
#endif
136-
137-
//Keep separate so above hack can be cleanly replaced
138-
#ifdef I2C1
139-
if(I2Cx==I2C1) {
140-
reserved_i2c[0] = true;
141-
__HAL_RCC_I2C1_CLK_ENABLE();
142-
}
143-
#endif
144-
#ifdef I2C2
145-
if(I2Cx==I2C2) {
146-
reserved_i2c[1] = true;
147-
__HAL_RCC_I2C2_CLK_ENABLE();
148-
}
149-
#endif
150-
#ifdef I2C3
151-
if(I2Cx==I2C3) {
152-
reserved_i2c[2] = true;
153-
__HAL_RCC_I2C3_CLK_ENABLE();
154-
}
155-
#endif
113+
//Note: due to I2C soft reboot issue, do not relocate clock init.
114+
i2c_clock_enable(1 << (self->sda->i2c_index - 1));
115+
reserved_i2c[self->sda->i2c_index - 1] = true;
156116

157117
self->handle.Instance = I2Cx;
158118
self->handle.Init.ClockSpeed = 100000;
@@ -163,13 +123,26 @@ void common_hal_busio_i2c_construct(busio_i2c_obj_t *self,
163123
self->handle.Init.OwnAddress2 = 0;
164124
self->handle.Init.GeneralCallMode = I2C_GENERALCALL_DISABLE;
165125
self->handle.Init.NoStretchMode = I2C_NOSTRETCH_DISABLE;
166-
if(HAL_I2C_Init(&(self->handle)) != HAL_OK) {
126+
self->handle.State = HAL_I2C_STATE_RESET;
127+
if (HAL_I2C_Init(&(self->handle)) != HAL_OK) {
167128
mp_raise_RuntimeError(translate("I2C Init Error"));
168129
}
169130
claim_pin(sda);
170131
claim_pin(scl);
171132
}
172133

134+
void common_hal_busio_i2c_never_reset(busio_i2c_obj_t *self) {
135+
for (size_t i = 0; i < MP_ARRAY_SIZE(mcu_i2c_banks); i++) {
136+
if (self->handle.Instance == mcu_i2c_banks[i]) {
137+
never_reset_i2c[i] = true;
138+
139+
never_reset_pin_number(self->scl->pin->port, self->scl->pin->number);
140+
never_reset_pin_number(self->sda->pin->port, self->sda->pin->number);
141+
break;
142+
}
143+
}
144+
}
145+
173146
bool common_hal_busio_i2c_deinited(busio_i2c_obj_t *self) {
174147
return self->sda->pin == mp_const_none;
175148
}
@@ -178,35 +151,19 @@ void common_hal_busio_i2c_deinit(busio_i2c_obj_t *self) {
178151
if (common_hal_busio_i2c_deinited(self)) {
179152
return;
180153
}
181-
#ifdef I2C1
182-
if(self->handle.Instance==I2C1) {
183-
never_reset[0] = false;
184-
reserved_i2c[0] = false;
185-
__HAL_RCC_I2C1_CLK_DISABLE();
186-
}
187-
#endif
188-
#ifdef I2C2
189-
if(self->handle.Instance==I2C2) {
190-
never_reset[1] = false;
191-
reserved_i2c[1] = false;
192-
__HAL_RCC_I2C2_CLK_DISABLE();
193-
}
194-
#endif
195-
#ifdef I2C3
196-
if(self->handle.Instance==I2C3) {
197-
never_reset[2] = false;
198-
reserved_i2c[2] = false;
199-
__HAL_RCC_I2C3_CLK_DISABLE();
200-
}
201-
#endif
154+
155+
i2c_clock_disable(1 << (self->sda->i2c_index - 1));
156+
reserved_i2c[self->sda->i2c_index - 1] = false;
157+
never_reset_i2c[self->sda->i2c_index - 1] = false;
158+
202159
reset_pin_number(self->sda->pin->port,self->sda->pin->number);
203160
reset_pin_number(self->scl->pin->port,self->scl->pin->number);
204161
self->sda = mp_const_none;
205162
self->scl = mp_const_none;
206163
}
207164

208165
bool common_hal_busio_i2c_probe(busio_i2c_obj_t *self, uint8_t addr) {
209-
return HAL_I2C_IsDeviceReady(&(self->handle), (uint16_t)(addr<<1),2,2) == HAL_OK;
166+
return HAL_I2C_IsDeviceReady(&(self->handle), (uint16_t)(addr << 1), 2, 2) == HAL_OK;
210167
}
211168

212169
bool common_hal_busio_i2c_try_lock(busio_i2c_obj_t *self) {
@@ -238,11 +195,56 @@ void common_hal_busio_i2c_unlock(busio_i2c_obj_t *self) {
238195

239196
uint8_t common_hal_busio_i2c_write(busio_i2c_obj_t *self, uint16_t addr,
240197
const uint8_t *data, size_t len, bool transmit_stop_bit) {
241-
HAL_StatusTypeDef result = HAL_I2C_Master_Transmit(&(self->handle), (uint16_t)(addr<<1), (uint8_t *)data, (uint16_t)len, 500);
198+
HAL_StatusTypeDef result = HAL_I2C_Master_Transmit(&(self->handle), (uint16_t)(addr << 1),
199+
(uint8_t *)data, (uint16_t)len, 500);
242200
return result == HAL_OK ? 0 : MP_EIO;
243201
}
244202

245203
uint8_t common_hal_busio_i2c_read(busio_i2c_obj_t *self, uint16_t addr,
246204
uint8_t *data, size_t len) {
247-
return HAL_I2C_Master_Receive(&(self->handle), (uint16_t)(addr<<1), data, (uint16_t)len, 500) == HAL_OK ? 0 : MP_EIO;
205+
return HAL_I2C_Master_Receive(&(self->handle), (uint16_t)(addr<<1), data, (uint16_t)len, 500)
206+
== HAL_OK ? 0 : MP_EIO;
207+
}
208+
209+
STATIC void i2c_clock_enable(uint8_t mask) {
210+
//Note: hard reset required due to soft reboot issue.
211+
#ifdef I2C1
212+
if (mask & (1 << 0)) {
213+
__HAL_RCC_I2C1_CLK_ENABLE();
214+
__HAL_RCC_I2C1_FORCE_RESET();
215+
__HAL_RCC_I2C1_RELEASE_RESET();
216+
}
217+
#endif
218+
#ifdef I2C2
219+
if (mask & (1 << 1)) {
220+
__HAL_RCC_I2C2_CLK_ENABLE();
221+
__HAL_RCC_I2C2_FORCE_RESET();
222+
__HAL_RCC_I2C2_RELEASE_RESET();
223+
}
224+
#endif
225+
#ifdef I2C3
226+
if (mask & (1 << 2)) {
227+
__HAL_RCC_I2C3_CLK_ENABLE();
228+
__HAL_RCC_I2C3_FORCE_RESET();
229+
__HAL_RCC_I2C3_RELEASE_RESET();
230+
}
231+
#endif
232+
}
233+
234+
STATIC void i2c_clock_disable(uint8_t mask) {
235+
#ifdef I2C1
236+
if (mask & (1 << 0)) {
237+
__HAL_RCC_I2C1_CLK_DISABLE();
238+
}
239+
#endif
240+
#ifdef I2C2
241+
if (mask & (1 << 1)) {
242+
__HAL_RCC_I2C2_CLK_DISABLE();
243+
}
244+
#endif
245+
#ifdef I2C3
246+
if (mask & (1 << 2)) {
247+
__HAL_RCC_I2C3_CLK_DISABLE();
248+
}
249+
#endif
248250
}

0 commit comments

Comments
 (0)