From a82e949c00aceaedd9d9a76ddcc3c88c9cad3d80 Mon Sep 17 00:00:00 2001 From: Kevin O'Connor Date: Fri, 26 May 2017 09:14:26 -0400 Subject: [PATCH] build: Use compile_time_request system for init, tasks, and shutdown Avoid using linker magic to define the init, task, and shutdown functions. Instead, use the compile_time_request system. This simplifies the build and produces more efficient code. Signed-off-by: Kevin O'Connor --- Makefile | 11 +---------- scripts/buildcommands.py | 28 ++++++++++++++++++++++++++-- scripts/checkstack.py | 9 +-------- src/adccmds.c | 4 ++-- src/avr/Makefile | 2 +- src/avr/serial.c | 2 +- src/avr/timer.c | 6 +++--- src/avr/usbserial.c | 2 +- src/avr/watchdog.c | 4 ++-- src/basecmd.c | 4 ++-- src/command.c | 4 ++-- src/declfunc.lds.S | 27 --------------------------- src/endstop.c | 2 +- src/generic/armcm_irq.c | 2 +- src/generic/timer_irq.c | 4 ++-- src/gpiocmds.c | 4 ++-- src/pru/Makefile | 11 ++++++++--- src/pru/main.c | 7 +++---- src/pwmcmds.c | 2 +- src/sam3x8e/main.c | 4 ++-- src/sam3x8e/serial.c | 2 +- src/sam3x8e/timer.c | 4 ++-- src/sched.c | 40 +++++++++------------------------------- src/sched.h | 20 ++++++-------------- src/simulator/main.c | 2 +- src/stepper.c | 2 +- 26 files changed, 82 insertions(+), 127 deletions(-) delete mode 100644 src/declfunc.lds.S diff --git a/Makefile b/Makefile index 39535898..8e1965a0 100644 --- a/Makefile +++ b/Makefile @@ -37,7 +37,6 @@ CFLAGS := -I$(OUT) -Isrc -I$(OUT)board-generic/ -O2 -MD -g \ -ffunction-sections -fdata-sections CFLAGS += -flto -fwhole-program -fno-use-linker-plugin -CFLAGS_klipper.o = $(CFLAGS) -Wl,-r -nostdlib CFLAGS_klipper.elf = $(CFLAGS) -Wl,--gc-sections CPPFLAGS = -I$(OUT) -P -MD -MT $@ @@ -84,15 +83,7 @@ $(OUT)compile_time_request.o: $(patsubst %.c, $(OUT)src/%.o.ctr,$(src-y)) ./scri $(Q)$(PYTHON) ./scripts/buildcommands.py -d $(OUT)klipper.dict $(OUT)klipper.compile_time_request $(OUT)compile_time_request.c $(Q)$(CC) $(CFLAGS) -c $(OUT)compile_time_request.c -o $@ -$(OUT)declfunc.lds: src/declfunc.lds.S - @echo " Precompiling $@" - $(Q)$(CPP) $(CPPFLAGS) -D__ASSEMBLY__ $< -o $@ - -$(OUT)klipper.o: $(patsubst %.c, $(OUT)src/%.o,$(src-y)) $(OUT)compile_time_request.o $(OUT)declfunc.lds - @echo " Linking $@" - $(Q)$(CC) $(CFLAGS_klipper.o) -Wl,-T,$(OUT)declfunc.lds $(patsubst %.c, $(OUT)src/%.o,$(src-y)) $(OUT)compile_time_request.o -o $@ - -$(OUT)klipper.elf: $(OUT)klipper.o +$(OUT)klipper.elf: $(patsubst %.c, $(OUT)src/%.o,$(src-y)) $(OUT)compile_time_request.o @echo " Linking $@" $(Q)$(CC) $(CFLAGS_klipper.elf) $^ -o $@ diff --git a/scripts/buildcommands.py b/scripts/buildcommands.py index 54a01caa..3935fa63 100644 --- a/scripts/buildcommands.py +++ b/scripts/buildcommands.py @@ -13,6 +13,7 @@ import msgproto FILEHEADER = """ /* DO NOT EDIT! This is an autogenerated file. See scripts/buildcommands.py. */ +#include "board/irq.h" #include "board/pgm.h" #include "command.h" #include "compiler.h" @@ -119,6 +120,23 @@ ctr_lookup_static_string(const char *str) """ return fmt % ("".join(code).strip(),) +def build_call_lists(call_lists): + code = [] + for funcname, funcs in call_lists.items(): + func_code = [' extern void %s(void);\n %s();' % (f, f) + for f in funcs] + if funcname == 'ctr_run_taskfuncs': + func_code = [' irq_poll();\n' + fc for fc in func_code] + fmt = """ +void +%s(void) +{ + %s +} +""" + code.append(fmt % (funcname, "\n".join(func_code).strip())) + return "".join(code) + def build_param_types(all_param_types): sorted_param_types = sorted([(i, a) for a, i in all_param_types.items()]) params = [''] @@ -271,6 +289,7 @@ def main(): encoders = [] static_strings = [] constants = {} + call_lists = {'ctr_run_initfuncs': []} # Parse request file f = open(incmdfile, 'rb') data = f.read() @@ -314,6 +333,10 @@ def main(): if name in constants and constants[name] != value: error("Conflicting definition for constant '%s'" % name) constants[name] = value + elif cmd == '_DECL_CALLLIST': + funcname, callname = parts[1:] + cl = call_lists.setdefault(funcname, []) + cl.append(callname) else: error("Unknown build time command '%s'" % cmd) # Create unique ids for each message type @@ -328,6 +351,7 @@ def main(): all_param_types = {} parsercode = build_encoders(encoders, msg_to_id, all_param_types) static_strings_code = build_static_strings(static_strings) + call_lists_code = build_call_lists(call_lists) # Create command definitions cmd_by_id = dict((msg_to_id[messages_by_name.get(msgname, msgname)], cmd) for msgname, cmd in commands.items()) @@ -342,8 +366,8 @@ def main(): , static_strings, constants, version) # Write output f = open(outcfile, 'wb') - f.write(FILEHEADER + paramcode + parsercode + static_strings_code - + cmdcode + icode) + f.write(FILEHEADER + call_lists_code + static_strings_code + + paramcode + parsercode + cmdcode + icode) f.close() # Write data dictionary diff --git a/scripts/checkstack.py b/scripts/checkstack.py index 07d88176..34314218 100755 --- a/scripts/checkstack.py +++ b/scripts/checkstack.py @@ -191,16 +191,9 @@ def main(): for info in funcs.values(): funcnameroot = info.funcname.split('.')[0] funcsbyname[funcnameroot] = info - mainfunc = funcsbyname.get('sched_main') - cmdfunc = funcsbyname.get('command_task') + cmdfunc = funcsbyname.get('sched_main') eventfunc = funcsbyname.get('__vector_13', funcsbyname.get('__vector_17')) for funcnameroot, info in funcsbyname.items(): - if (funcnameroot.startswith('_DECL_taskfuncs_') - or funcnameroot.startswith('_DECL_initfuncs_') - or funcnameroot.startswith('_DECL_shutdownfuncs_')): - funcname = funcnameroot[funcnameroot.index('_', 7)+1:] - f = funcsbyname[funcname] - mainfunc.noteCall(0, f.funcaddr, mainfunc.basic_stack_usage + 2) if funcnameroot.startswith('parser_'): f = funcsbyname.get(funcnameroot[7:]) if f is not None: diff --git a/src/adccmds.c b/src/adccmds.c index 5f1fabb4..526325ad 100644 --- a/src/adccmds.c +++ b/src/adccmds.c @@ -80,7 +80,7 @@ DECL_COMMAND(command_query_analog_in, "query_analog_in oid=%c clock=%u sample_ticks=%u sample_count=%c" " rest_ticks=%u min_value=%hu max_value=%hu"); -static void +void analog_in_task(void) { static uint16_t next; @@ -106,7 +106,7 @@ analog_in_task(void) } DECL_TASK(analog_in_task); -static void +void analog_in_shutdown(void) { uint8_t i; diff --git a/src/avr/Makefile b/src/avr/Makefile index fa9258b1..94cebfff 100644 --- a/src/avr/Makefile +++ b/src/avr/Makefile @@ -14,7 +14,7 @@ src-$(CONFIG_AVR_USBSERIAL) += avr/usbserial.c ../lib/pjrc_usb_serial/usb_serial src-$(CONFIG_AVR_SERIAL) += avr/serial.c # Suppress broken "misspelled signal handler" warnings on gcc 4.8.1 -CFLAGS_klipper.o := $(CFLAGS_klipper.o) $(if $(filter 4.8.1, $(shell $(CC) -dumpversion)), -w) +CFLAGS_klipper.elf := $(CFLAGS_klipper.elf) $(if $(filter 4.8.1, $(shell $(CC) -dumpversion)), -w) # Build the additional hex output file target-y += $(OUT)klipper.elf.hex diff --git a/src/avr/serial.c b/src/avr/serial.c index 8628b602..3aff121f 100644 --- a/src/avr/serial.c +++ b/src/avr/serial.c @@ -25,7 +25,7 @@ static uint8_t transmit_pos, transmit_max; DECL_CONSTANT(SERIAL_BAUD, CONFIG_SERIAL_BAUD); -static void +void serial_init(void) { if (CONFIG_SERIAL_BAUD_U2X) { diff --git a/src/avr/timer.c b/src/avr/timer.c index a0acac73..92459af3 100644 --- a/src/avr/timer.c +++ b/src/avr/timer.c @@ -71,7 +71,7 @@ timer_repeat_set(uint16_t next) } // Reset the timer - clear settings and dispatch next timer immediately -static void +void timer_reset(void) { uint16_t now = timer_get(); @@ -81,7 +81,7 @@ timer_reset(void) } DECL_SHUTDOWN(timer_reset); -static void +void timer_init(void) { if (CONFIG_AVR_CLKPR != -1 && (uint8_t)CONFIG_AVR_CLKPR != CLKPR) { @@ -193,7 +193,7 @@ done: // Periodic background task that temporarily boosts priority of // timers. This helps prioritize timers when tasks are idling. -static void +void timer_task(void) { irq_disable(); diff --git a/src/avr/usbserial.c b/src/avr/usbserial.c index 4cf1bd0f..3b14ea0a 100644 --- a/src/avr/usbserial.c +++ b/src/avr/usbserial.c @@ -14,7 +14,7 @@ static char receive_buf[USBSERIAL_BUFFER_SIZE]; static uint8_t receive_pos; static char transmit_buf[USBSERIAL_BUFFER_SIZE]; -static void +void usbserial_init(void) { usb_init(); diff --git a/src/avr/watchdog.c b/src/avr/watchdog.c index 306f2cd8..61b5fc5f 100644 --- a/src/avr/watchdog.c +++ b/src/avr/watchdog.c @@ -18,7 +18,7 @@ ISR(WDT_vect) shutdown("Watchdog timer!"); } -static void +void watchdog_reset(void) { wdt_reset(); @@ -29,7 +29,7 @@ watchdog_reset(void) } DECL_TASK(watchdog_reset); -static void +void watchdog_init(void) { // 0.5s timeout, interrupt and system reset diff --git a/src/basecmd.c b/src/basecmd.c index 600b8106..742a9e26 100644 --- a/src/basecmd.c +++ b/src/basecmd.c @@ -65,7 +65,7 @@ move_request_size(int size) move_item_size = size; } -static void +void move_reset(void) { if (!move_count) @@ -201,7 +201,7 @@ DECL_COMMAND_FLAGS(command_get_uptime, HF_IN_SHUTDOWN, "get_uptime"); #define SUMSQ_BASE 256 DECL_CONSTANT(STATS_SUMSQ_BASE, SUMSQ_BASE); -static void +void stats_task(void) { static uint32_t last, count, sumsq; diff --git a/src/command.c b/src/command.c index 522b0223..2a248f16 100644 --- a/src/command.c +++ b/src/command.c @@ -197,7 +197,7 @@ error: shutdown("Message encode error"); } -static void +void sendf_shutdown(void) { writeb(&in_sendf, 0); @@ -289,7 +289,7 @@ nak: } // Background task that reads commands from the board serial port -static void +void command_task(void) { // Process commands. diff --git a/src/declfunc.lds.S b/src/declfunc.lds.S deleted file mode 100644 index ba564bbf..00000000 --- a/src/declfunc.lds.S +++ /dev/null @@ -1,27 +0,0 @@ -// Linker script that defines symbols around sections. The DECL_X() -// macros need this linker script to place _start and _end symbols -// around the list of declared items. -#include "autoconf.h" // CONFIG_MACH_AVR -#include "compiler.h" // __PASTE - -#if CONFIG_MACH_AVR -#define DECL_SECTION .progmem.data. -#else -#define DECL_SECTION .rodata. -#endif - -#define DECLWRAPPER(NAME) \ - __PASTE(DECL_SECTION, NAME) : SUBALIGN(1) { \ - NAME ## _start = . ; \ - *( .rodata. ## NAME ##.pre* ) \ - *( .rodata. ## NAME ##* ) \ - *( .rodata. ## NAME ##.post* ) \ - NAME ## _end = . ; \ - } - -SECTIONS -{ - DECLWRAPPER(taskfuncs) - DECLWRAPPER(initfuncs) - DECLWRAPPER(shutdownfuncs) -} diff --git a/src/endstop.c b/src/endstop.c index 7a1fb1d9..aaaf1baa 100644 --- a/src/endstop.c +++ b/src/endstop.c @@ -112,7 +112,7 @@ command_end_stop_query(uint32_t *args) } DECL_COMMAND(command_end_stop_query, "end_stop_query oid=%c"); -static void +void end_stop_task(void) { static uint16_t next; diff --git a/src/generic/armcm_irq.c b/src/generic/armcm_irq.c index 9c01ab0d..7778c389 100644 --- a/src/generic/armcm_irq.c +++ b/src/generic/armcm_irq.c @@ -40,7 +40,7 @@ irq_poll(void) } // Clear the active irq if a shutdown happened in an irq handler -static void +void clear_active_irq(void) { uint32_t psr; diff --git a/src/generic/timer_irq.c b/src/generic/timer_irq.c index 69f4e64a..07161eee 100644 --- a/src/generic/timer_irq.c +++ b/src/generic/timer_irq.c @@ -82,7 +82,7 @@ timer_dispatch_many(void) // Periodic background task that temporarily boosts priority of // timers. This helps prioritize timers when tasks are idling. -static void +void timer_task(void) { irq_disable(); @@ -91,7 +91,7 @@ timer_task(void) } DECL_TASK(timer_task); -static void +void timer_irq_shutdown(void) { timer_repeat_until = timer_read_time() + TIMER_IDLE_REPEAT_TICKS; diff --git a/src/gpiocmds.c b/src/gpiocmds.c index a0e476b9..69f60f97 100644 --- a/src/gpiocmds.c +++ b/src/gpiocmds.c @@ -67,7 +67,7 @@ command_schedule_digital_out(uint32_t *args) DECL_COMMAND(command_schedule_digital_out, "schedule_digital_out oid=%c clock=%u value=%c"); -static void +void digital_out_shutdown(void) { uint8_t i; @@ -212,7 +212,7 @@ command_schedule_soft_pwm_out(uint32_t *args) DECL_COMMAND(command_schedule_soft_pwm_out, "schedule_soft_pwm_out oid=%c clock=%u value=%hu"); -static void +void soft_pwm_shutdown(void) { uint8_t i; diff --git a/src/pru/Makefile b/src/pru/Makefile index 4e326955..5f5739b4 100644 --- a/src/pru/Makefile +++ b/src/pru/Makefile @@ -9,9 +9,10 @@ dirs-y += lib/pru_rpmsg CFLAGS += -Os -mmcu=am335x.pru1 CFLAGS += -Ilib/pru_rpmsg/include -Ilib/pru_rpmsg/include/am335x -CFLAGS_klipper.o := $(filter-out -mmcu=%, $(CFLAGS_klipper.o)) -CFLAGS_klipper.elf := $(CFLAGS) -minrt -T src/pru/pru.lds +CFLAGS_klipper.elf := $(filter-out -mmcu=%, $(CFLAGS)) +CFLAGS_klipper.elf += -Wl,-r -nostdlib -Wl,-T,src/pru/pru.lds CFLAGS_pru0.elf := $(filter-out -mmcu=%, $(CFLAGS)) -minrt -mmcu=am335x.pru0 +CFLAGS_pru1.elf := $(CFLAGS) -minrt # Add source files src-y := $(filter-out debugcmds.c, $(src-y)) @@ -22,8 +23,12 @@ pru0-y := pru/pru0.c pru0-y += ../lib/pru_rpmsg/pru_rpmsg.c ../lib/pru_rpmsg/pru_virtqueue.c # Build the additional PRU0 binary -target-y += $(OUT)pru0.elf +target-y += $(OUT)pru0.elf $(OUT)pru1.elf $(OUT)pru0.elf: $(patsubst %.c, $(OUT)src/%.o,$(pru0-y)) @echo " Linking $@" $(Q)$(CC) $(CFLAGS_pru0.elf) $^ -o $@ + +$(OUT)pru1.elf: $(OUT)klipper.elf + @echo " Linking $@" + $(Q)$(CC) $(CFLAGS_pru1.elf) $^ -o $@ diff --git a/src/pru/main.c b/src/pru/main.c index df69a01e..46722709 100644 --- a/src/pru/main.c +++ b/src/pru/main.c @@ -75,7 +75,7 @@ irq_poll(void) _irq_poll(); } -static void +void timer_shutdown(void) { // Reenable timer irq @@ -86,12 +86,13 @@ timer_shutdown(void) } DECL_SHUTDOWN(timer_shutdown); -static void +void timer_init(void) { CT_IEP.TMR_CNT = 0; timer_shutdown(); } +DECL_INIT(timer_init); /**************************************************************** @@ -211,8 +212,6 @@ main(void) ; writel(&SHARED_MEM->signal, SIGNAL_PRU1_READY); - timer_init(); - sched_main(); return 0; } diff --git a/src/pwmcmds.c b/src/pwmcmds.c index f5a289d2..4709a91e 100644 --- a/src/pwmcmds.c +++ b/src/pwmcmds.c @@ -59,7 +59,7 @@ command_schedule_pwm_out(uint32_t *args) DECL_COMMAND(command_schedule_pwm_out, "schedule_pwm_out oid=%c clock=%u value=%hu"); -static void +void pwm_shutdown(void) { uint8_t i; diff --git a/src/sam3x8e/main.c b/src/sam3x8e/main.c index 50cc18b4..63c27cd7 100644 --- a/src/sam3x8e/main.c +++ b/src/sam3x8e/main.c @@ -16,14 +16,14 @@ DECL_CONSTANT(MCU, "sam3x8e"); * watchdog handler ****************************************************************/ -static void +void watchdog_reset(void) { WDT->WDT_CR = 0xA5000001; } DECL_TASK(watchdog_reset); -static void +void watchdog_init(void) { uint32_t timeout = 32768 / 2; // 500ms timeout diff --git a/src/sam3x8e/serial.c b/src/sam3x8e/serial.c index 9203385d..2b76a69b 100644 --- a/src/sam3x8e/serial.c +++ b/src/sam3x8e/serial.c @@ -27,7 +27,7 @@ static uint32_t transmit_pos, transmit_max; DECL_CONSTANT(SERIAL_BAUD, CONFIG_SERIAL_BAUD); -static void +void serial_init(void) { gpio_peripheral('A', PIO_PA8A_URXD, 'A', 1); diff --git a/src/sam3x8e/timer.c b/src/sam3x8e/timer.c index 498e22cb..0bc21dae 100644 --- a/src/sam3x8e/timer.c +++ b/src/sam3x8e/timer.c @@ -25,7 +25,7 @@ timer_read_time(void) return TC0->TC_CHANNEL[0].TC_CV; } -static void +void timer_init(void) { TcChannel *tc = &TC0->TC_CHANNEL[0]; @@ -44,7 +44,7 @@ timer_init(void) } DECL_INIT(timer_init); -static void +void timer_shutdown(void) { // Reenable timer irq diff --git a/src/sched.c b/src/sched.c index f82b0060..542e3747 100644 --- a/src/sched.c +++ b/src/sched.c @@ -1,6 +1,6 @@ // Basic scheduling functions and startup/shutdown code. // -// Copyright (C) 2016 Kevin O'Connor +// Copyright (C) 2016,2017 Kevin O'Connor // // This file may be distributed under the terms of the GNU GPLv3 license. @@ -167,7 +167,7 @@ sched_timer_dispatch(void) } // Shutdown all user timers on an emergency stop. -static void +void sched_timer_shutdown(void) { timer_list = &deleted_timer; @@ -209,11 +209,8 @@ run_shutdown(void) { uint32_t cur = timer_read_time(); shutdown_status = 2; - struct callback_handler *p; - foreachdecl(p, shutdownfuncs) { - void (*func)(void) = READP(p->func); - func(); - } + extern void ctr_run_shutdownfuncs(void); + ctr_run_shutdownfuncs(); shutdown_status = 1; irq_enable(); @@ -252,39 +249,20 @@ sched_shutdown(uint_fast8_t reason) * Startup and background task processing ****************************************************************/ -// Invoke all init functions (as declared by DECL_INIT) -static void -run_init(void) -{ - struct callback_handler *p; - foreachdecl(p, initfuncs) { - void (*func)(void) = READP(p->func); - func(); - } -} - -// Invoke all background task functions (as declared by DECL_TASK) -static void -run_task(void) -{ - struct callback_handler *p; - foreachdecl(p, taskfuncs) { - irq_poll(); - void (*func)(void) = READP(p->func); - func(); - } -} +// Auto-generated code in out/compile_time_requests.c +extern void ctr_run_initfuncs(void); +extern void ctr_run_taskfuncs(void); // Main loop of program void sched_main(void) { - run_init(); + ctr_run_initfuncs(); int ret = setjmp(shutdown_jmp); if (ret) run_shutdown(); for (;;) - run_task(); + ctr_run_taskfuncs(); } diff --git a/src/sched.h b/src/sched.h index 9d6411d0..ad5c2c08 100644 --- a/src/sched.h +++ b/src/sched.h @@ -2,14 +2,14 @@ #define __SCHED_H #include // uint32_t -#include "compiler.h" // __section +#include "ctr.h" // DECL_CTR // Declare an init function (called at firmware startup) -#define DECL_INIT(FUNC) _DECL_CALLBACK(initfuncs, FUNC) +#define DECL_INIT(FUNC) _DECL_CALLLIST(ctr_run_initfuncs, FUNC) // Declare a task function (called periodically during normal runtime) -#define DECL_TASK(FUNC) _DECL_CALLBACK(taskfuncs, FUNC) +#define DECL_TASK(FUNC) _DECL_CALLLIST(ctr_run_taskfuncs, FUNC) // Declare a shutdown function (called on an emergency stop) -#define DECL_SHUTDOWN(FUNC) _DECL_CALLBACK(shutdownfuncs, FUNC) +#define DECL_SHUTDOWN(FUNC) _DECL_CALLLIST(ctr_run_shutdownfuncs, FUNC) // Timer structure for scheduling timed events (see sched_add_timer() ) struct timer { @@ -33,15 +33,7 @@ void sched_report_shutdown(void); void sched_main(void); // Compiler glue for DECL_X macros above. -struct callback_handler { - void (*func)(void); -}; -#define _DECL_CALLBACK(NAME, FUNC) \ - const struct callback_handler _DECL_ ## NAME ## _ ## FUNC __visible \ - __section(".rodata." __stringify(NAME) ) = { .func = FUNC } - -#define foreachdecl(ITER, NAME) \ - extern typeof(*ITER) NAME ## _start[], NAME ## _end[]; \ - for (ITER = NAME ## _start ; ITER < NAME ## _end ; ITER ++) +#define _DECL_CALLLIST(NAME, FUNC) \ + DECL_CTR("_DECL_CALLLIST " __stringify(NAME) " " __stringify(FUNC)) #endif // sched.h diff --git a/src/simulator/main.c b/src/simulator/main.c index 72d0abaf..47dd49f4 100644 --- a/src/simulator/main.c +++ b/src/simulator/main.c @@ -117,7 +117,7 @@ console_push_output(uint8_t len) ****************************************************************/ // Periodically sleep so we don't consume all CPU -static void +void simu_pause(void) { // XXX - should check that no timers are present. diff --git a/src/stepper.c b/src/stepper.c index 258ecfda..09c615a2 100644 --- a/src/stepper.c +++ b/src/stepper.c @@ -284,7 +284,7 @@ stepper_stop(struct stepper *s) } } -static void +void stepper_shutdown(void) { uint8_t i;