Skip to content

Commit 2b5e210

Browse files
committed
Remove support for busy-wait millisecond delays with IRQs disabled
The current implementation of Pybricks does not disable IRQs for any significant length of time, which means that we can remove clock driver code supporting it.
1 parent 625e80b commit 2b5e210

File tree

8 files changed

+12
-106
lines changed

8 files changed

+12
-106
lines changed

bricks/_common_stm32/mphalport.c

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -22,22 +22,17 @@
2222
extern volatile uint32_t pbdrv_clock_ticks;
2323

2424
// Core delay function that does an efficient sleep and may switch thread context.
25-
// If IRQs are enabled then we must have the GIL.
25+
// We must have the GIL.
2626
void mp_hal_delay_ms(mp_uint_t Delay) {
27-
if (pbdrv_clock_is_ticking()) {
28-
// IRQs enabled, so can use systick counter to do the delay
29-
uint32_t start = pbdrv_clock_ticks;
30-
// Wraparound of tick is taken care of by 2's complement arithmetic.
31-
do {
32-
// This macro will execute the necessary idle behaviour. It may
33-
// raise an exception, switch threads or enter sleep mode (waiting for
34-
// (at least) the SysTick interrupt).
35-
MICROPY_EVENT_POLL_HOOK
36-
} while (pbdrv_clock_ticks - start < Delay);
37-
} else {
38-
// IRQs disabled, so need to use a busy loop for the delay.
39-
pbdrv_clock_busy_delay_ms(Delay);
40-
}
27+
// Use systick counter to do the delay
28+
uint32_t start = pbdrv_clock_ticks;
29+
// Wraparound of tick is taken care of by 2's complement arithmetic.
30+
do {
31+
// This macro will execute the necessary idle behaviour. It may
32+
// raise an exception, switch threads or enter sleep mode (waiting for
33+
// (at least) the SysTick interrupt).
34+
MICROPY_EVENT_POLL_HOOK
35+
} while (pbdrv_clock_ticks - start < Delay);
4136
}
4237

4338
uintptr_t mp_hal_stdio_poll(uintptr_t poll_flags) {

lib/pbio/drv/clock/clock_ev3.c

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -119,13 +119,4 @@ uint32_t pbdrv_clock_get_100us(void) {
119119
return pbdrv_clock_get_ms() * 10;
120120
}
121121

122-
void pbdrv_clock_busy_delay_ms(uint32_t ms) {
123-
// TODO
124-
}
125-
126-
bool pbdrv_clock_is_ticking(void) {
127-
// TODO
128-
return true;
129-
}
130-
131122
#endif // PBDRV_CONFIG_CLOCK_TIAM1808

lib/pbio/drv/clock/clock_linux.c

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -112,14 +112,4 @@ uint32_t pbdrv_clock_get_us(void) {
112112
return time_val.tv_sec * 1000000 + time_val.tv_nsec / 1000;
113113
}
114114

115-
void pbdrv_clock_busy_delay_ms(uint32_t ms) {
116-
uint32_t start = pbdrv_clock_get_ms();
117-
while (pbdrv_clock_get_ms() - start < ms) {
118-
}
119-
}
120-
121-
bool pbdrv_clock_is_ticking(void) {
122-
return true;
123-
}
124-
125115
#endif // PBDRV_CONFIG_CLOCK_LINUX

lib/pbio/drv/clock/clock_none.c

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,4 @@ uint32_t pbdrv_clock_get_100us(void) {
2222
return 0;
2323
}
2424

25-
void pbdrv_clock_busy_delay_ms(uint32_t ms) {
26-
}
27-
28-
bool pbdrv_clock_is_ticking(void) {
29-
return false;
30-
}
31-
3225
#endif // PBDRV_CONFIG_CLOCK_NONE

lib/pbio/drv/clock/clock_nxt.c

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -87,32 +87,4 @@ uint32_t pbdrv_clock_get_us(void) {
8787
return pbdrv_clock_ticks * 1000;
8888
}
8989

90-
// TODO: we really should get rid of blocking waits if possible
91-
92-
void nx_systick_wait_ms(uint32_t ms) {
93-
// FIXME: this does not currently handle overflow (pbdrv_clock_ticks + ms > UINT32_MAX)
94-
uint32_t final = pbdrv_clock_ticks + ms;
95-
96-
while (pbdrv_clock_ticks < final) {
97-
;
98-
}
99-
}
100-
101-
void pbdrv_clock_busy_delay_ms(uint32_t ms) {
102-
nx_systick_wait_ms(ms);
103-
}
104-
105-
void nx_systick_wait_ns(uint32_t ns) {
106-
volatile uint32_t x = (ns >> 7) + 1;
107-
108-
while (x--) {
109-
;
110-
}
111-
}
112-
113-
bool pbdrv_clock_is_ticking(void) {
114-
// TODO
115-
return true;
116-
}
117-
11890
#endif // PBDRV_CONFIG_CLOCK_NXT

lib/pbio/drv/clock/clock_stm32.c

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,8 @@ uint32_t HAL_GetTick(void) {
9292
// We provide our own version of HAL_Delay that calls __WFI while waiting,
9393
// and works when interrupts are disabled. This function is intended to be
9494
// used only by the ST HAL functions.
95+
//
96+
// TODO: Is anything actually still calling this?
9597
void HAL_Delay(uint32_t Delay) {
9698
if (__get_PRIMASK() == 0) {
9799
// IRQs enabled, so can use systick counter to do the delay
@@ -112,14 +114,4 @@ void HAL_Delay(uint32_t Delay) {
112114
}
113115
}
114116

115-
void pbdrv_clock_busy_delay_ms(uint32_t ms) {
116-
HAL_Delay(ms);
117-
}
118-
119-
bool pbdrv_clock_is_ticking(void) {
120-
// Init already completed in SystemInit(), so we just need to check if
121-
// interrupts are enabled.
122-
return __get_PRIMASK() == 0;
123-
}
124-
125117
#endif // PBDRV_CONFIG_CLOCK_STM32

lib/pbio/drv/clock/clock_test.c

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -41,12 +41,5 @@ uint32_t pbdrv_clock_get_us(void) {
4141
return clock_ticks * 1000;
4242
}
4343

44-
void pbdrv_clock_busy_delay_ms(uint32_t ms) {
45-
}
46-
47-
bool pbdrv_clock_is_ticking(void) {
48-
return true;
49-
}
50-
5144

5245
#endif // PBDRV_CONFIG_CLOCK_TEST

lib/pbio/include/pbdrv/clock.h

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -37,26 +37,6 @@ uint32_t pbdrv_clock_get_us(void);
3737
*/
3838
void pbdrv_clock_busy_delay_us(uint32_t us);
3939

40-
/**
41-
* Busy wait delay for several milliseconds. May not be accurate.
42-
*
43-
* NB: Should not be used in any driver code. This exists only as a hook for
44-
* APIs that need a blocking delay to work when interrupts could be disabled.
45-
*
46-
* @param [in] ms The number of milliseconds to delay.
47-
*/
48-
void pbdrv_clock_busy_delay_ms(uint32_t ms);
49-
50-
/**
51-
* Tests if the millisecond clock is ticking.
52-
*
53-
* On embedded systems, this means that the clock was initialized and IRQs are
54-
* enabled.
55-
*
56-
* @return True if the clock is ticking, false otherwise.
57-
*/
58-
bool pbdrv_clock_is_ticking(void);
59-
6040
#endif /* _PBDRV_CLOCK_H_ */
6141

6242
/** @} */

0 commit comments

Comments
 (0)