Skip to content

Commit 52228aa

Browse files
ArcaneNibbledlech
authored andcommitted
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 52228aa

File tree

10 files changed

+24
-109
lines changed

10 files changed

+24
-109
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) {

bricks/ev3/mphalport.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,9 @@
1919
#include "py/stream.h"
2020

2121
// Core delay function that does an efficient sleep and may switch thread context.
22-
// If IRQs are enabled then we must have the GIL.
22+
// We must have the GIL.
2323
void mp_hal_delay_ms(mp_uint_t Delay) {
24-
// IRQs enabled, so can use systick counter to do the delay
24+
// Use systick counter to do the delay
2525
uint32_t start = pbdrv_clock_get_ms();
2626
// Wraparound of tick is taken care of by 2's complement arithmetic.
2727
do {

bricks/nxt/mphalport.c

Lines changed: 10 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -18,28 +18,18 @@
1818
#include "py/mpconfig.h"
1919
#include "py/stream.h"
2020

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

4535
// delay for given number of microseconds

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 & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -98,10 +98,6 @@ void nx_systick_wait_ms(uint32_t ms) {
9898
}
9999
}
100100

101-
void pbdrv_clock_busy_delay_ms(uint32_t ms) {
102-
nx_systick_wait_ms(ms);
103-
}
104-
105101
void nx_systick_wait_ns(uint32_t ns) {
106102
volatile uint32_t x = (ns >> 7) + 1;
107103

@@ -110,9 +106,4 @@ void nx_systick_wait_ns(uint32_t ns) {
110106
}
111107
}
112108

113-
bool pbdrv_clock_is_ticking(void) {
114-
// TODO
115-
return true;
116-
}
117-
118109
#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)