josh
/
Teacup_Firmware
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

dda_clock(): guard against *dda changes in interrupts 

dda_clock() might be interrupted by dda_step(), and dda_step might
use or modify variables also being used in dda_clock().  It is
possible for dda to be modified when a new dda becomes live during
our dda_clock().  Check the dda->id to ensure it has not changed on
us before we actually write new calculated values into the dda.

Note by Traumflug: copied some of the explanation in the commit
message directly into the code.
This commit is contained in:
Phil Hord 2016-05-20 11:41:07 -04:00 committed by Markus Hitter
parent 1eefb28a19
commit 90d519af5f
1 changed files with 14 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
dda.c
View File

@ -763,6 +763,7 @@ void dda_clock() {
int32_t move_n; int32_t move_n;
uint8_t recalc_speed; uint8_t recalc_speed;
#endif #endif
uint8_t current_id ;
dda = queue_current_movement(); dda = queue_current_movement();
if (dda != last_dda) { if (dda != last_dda) {
@ -867,6 +868,7 @@ void dda_clock() {
// http://www.embedded.com/design/mcus-processors-and-socs/4006438/Generate-stepper-motor-speed-profiles-in-real-time // http://www.embedded.com/design/mcus-processors-and-socs/4006438/Generate-stepper-motor-speed-profiles-in-real-time
// and http://www.atmel.com/images/doc8017.pdf (Atmel app note AVR446) // and http://www.atmel.com/images/doc8017.pdf (Atmel app note AVR446)
ATOMIC_START ATOMIC_START
current_id = dda->id;
move_step_no = move_state.step_no; move_step_no = move_state.step_no;
// All other variables are read-only or unused in dda_step(), // All other variables are read-only or unused in dda_step(),
// so no need for atomic operations. // so no need for atomic operations.
@ -918,8 +920,18 @@ void dda_clock() {
// Write results. // Write results.
ATOMIC_START ATOMIC_START
dda->c = move_c; /**
dda->n = move_n; Apply new n & c values only if dda didn't change underneath us. It
is possible for dda to be modified since fetching values in the
ATOMIC above, e.g. when a new dda becomes live.
In case such a change happened, values in the new dda are more
recent than our calculation here, anyways.
*/
if (current_id == dda->id) {
dda->c = move_c;
dda->n = move_n;
}
ATOMIC_END ATOMIC_END
} }
#endif #endif