From 85311a0b31f77cacb3fde2ad214c5cc816f1edf6 Mon Sep 17 00:00:00 2001 From: Lephe Date: Sun, 10 May 2020 16:36:21 +0200 Subject: [PATCH] drivers: update the model, replacing unload() with wait() The unload() function is not very relevant for drivers because hardware state is managed by ctx_save() and ctx_restore() and software state is managed by underlying drivers when there are dependencies. For now, it's been replaced with a wait() function that allows drivers to not be interrupted at any point. It is currently used by the DMA to wait for ongoing transfers to finish before disabling interrupts (which would prevent the transfer end from being detected) and switching in and out of gint. --- TODO | 18 +++++----- include/gint/drivers.h | 74 +++++++++++++++++++++--------------------- src/core/setup.c | 13 +++++++- src/core/start.c | 5 --- src/cpg/cpg.c | 6 ++-- src/dma/dma.c | 16 ++++----- src/keysc/getkey.c | 1 + src/keysc/keysc.c | 15 +++------ src/r61524/r61524.c | 14 ++++---- src/rtc/rtc.c | 14 ++++---- src/t6k11/t6k11.c | 14 ++++---- src/tmu/tmu.c | 16 ++++----- 12 files changed, 102 insertions(+), 104 deletions(-) diff --git a/TODO b/TODO index 4fe1a47..95c258f 100644 --- a/TODO +++ b/TODO @@ -1,14 +1,14 @@ +For the 2.0.0 release: +* bopti: remove image_t, leaving only bopti_image_t +* project: remove the compat branch +* gray: remove g*() functions + Crucial, missing things. -! core: the four basic memory functions -! core: gint_switch() with driver contexts on stack and arbitrary code +! core: the four basic memory functions (with automated tests) ! core: use gint_switch() to handle TLB misses ! core: return to menu on fxcg50 -! bopti: fxcg50 version ! syscalls: fxcg50 BFile calls -Tests to run. -* core: run the alignment/size automated tests for memory functions - Issues. * #3 make drawing functions parameterized * #5 add decent random number generation (TinyMT) @@ -18,18 +18,16 @@ Issues. Complementary elements on existing code. * gray: double-buffer gray settings and unify d* with g* * display: deprecate image_t and rename it bopti_image_t -* make fx9860g projects work out of the box on fxcg50 * topti: support unicode fonts * gray: find good values for more models than the Graph 35+E II * dma: maybe relax the 4-byte size constraint for dma_memset() -* dma: fx9860g support (need to switch it on) +* dma: fx9860g support (need to switch it on and update the Makefile) * core: try to leave add-in without reset in case of panic * hardware: fill in the HWMEM_FITTLB flag * keyboard: think of extended functions * cpg: spread spectrum on fxcg50 * display: use more of topti's assembler in drect() * core: use cmp/str for memchr() -* timer: try putting addresses in * r61524: brightness control and clean the file * t6k11: check if dupdate() can be done by the DMA @@ -44,7 +42,6 @@ Keep in mind. * core: free heap when a task-switch results in leaving the app * core: save and restore interrupt masks * timer: make sure ETMU interrupts are disabled in ctx_restore() -* timer: look for ways to improve the code again * core: document the SH7305 PFC in Future directions. @@ -53,3 +50,4 @@ Future directions. * Audio playback using Martin Poupe's method * Serial communication [SCIF] [SCIFA] * USB communication [USB] +* Make fx9860g projects work out of the box on fxcg50 diff --git a/include/gint/drivers.h b/include/gint/drivers.h index a879642..fdb4676 100644 --- a/include/gint/drivers.h +++ b/include/gint/drivers.h @@ -13,25 +13,31 @@ // // Drivers are initialized in priority order, and in linking order within // the same priority level (pretty much undefined). Make sure every -// driver's priority level is higher than those of its dependencies. +// driver's priority level is higher than those of its dependencies. In +// the description below, every function can be NULL. // // At initialization, the following functions are called: -// 1. driver_sh3() [if not NULL, SH3 fx9860g only] -// 2. ctx_save(sys_ctx) [if not NULL] -// 3. init() [if not NULL] +// 1. driver_sh3() [on SH3-based fx9860g only] +// 2. ctx_save(sys_ctx) +// 3. init() // // Then, if the on-screen boot log is enabled, the status() function is // called and the returned string is displayed (21 characters max). -// 4. status() [if not NULL, if GINT_BOOT_LOG is defined] +// 4. status() [only if GINT_BOOT_LOG is defined] // // If the gint_switch() function is called to temporarily give back // control to the operating system, the state of each driver is saved to // the stack, then restored from there. -// 5. ctx_save(stack) [if not NULL] -// 6. ctx_restore(stack) [if not NULL] +// 5. wait() +// 6. ctx_save(gint_ctx) +// 7. ctx_restore(sys_ctx) +// (stuff happening outside of gint) +// 8. ctx_save(sys_ctx) +// 9. ctx_restore(gint_ctx) // // When finally the driver is unloaded, the system context is restored. -// 7. ctx_restore(sys_ctx) [if not NULL] +// 10. wait() +// 11. ctx_restore(sys_ctx) //--- /* gint_driver_t - driver meta-information used by gint */ @@ -40,49 +46,43 @@ typedef struct /* Driver name */ char const *name; - /* driver_sh3() - rectify driver initialization on SH3 platforms - This function is called during driver initialization on SH3. It may - be NULL. */ + /* SH3-specific preinitializaton; called before init() when running on + SH3. May be NULL. */ void (*driver_sh3)(void); - /* init() - initialize the driver - This function is called after ctx_save() and needs not save the - system setting. It is typically used to initialize registers to - suitable values on startup. If there is no init function, this field - may be set to NULL */ + /* Must initialize the hardware so that the driver can start working. + This is called only once when the add-in starts, and should not save + hardware state (ctx_save() is called before). May be NULL. */ void (*init)(void); - /* unload() - deinitialize the driver - This function is called before ctx_restore() when gint is unloaded. - If there is no unload function, the field may be set to NULL */ - void (*unload)(void); + /* This function can be used to enforce a waiting period before the + driver is unloaded. It is called before returning to the OS in + gint_switch() and if the add-in exits. May be NULL. */ + void (*wait)(void); /* System's context and gint's context. These should point to enough memory to store a full driver state each. These are used when switching from the system to gint and back to the main menu. If they don't need to be initialized, put them in gint's uninitialized BSS - section using the GBSS macro of . */ + section using the GBSS macro of . May be + NULL only if both ctx_save() and ctx_restore() are NULL. */ void *sys_ctx; void *gint_ctx; - /* ctx_save() - save the driver's hardware support - This function is provided by the driver to save the state of its - hardware support (memory-mapped MPU registers, port state, etc). - @ctx A buffer of size ctx_size */ + /* Must save the state of as much driver-controlled hardware as + possible (memory-mapped MPU registers, port state, etc). This + function is called to save the system's hardware state and gint's + hardware state when moving from one into the other. The parameter + [ctx] is always either [sys_ctx] or [gint_ctx]. */ void (*ctx_save)(void *ctx); - /* ctx_restore() - restore a saved context - This function is provided by the driver to restore the state saved - by ctx_save(). It can alter the contents of the buffer freely. - @ctx A context buffer filled by ctx_save() */ + /* Must restore the state of the driver as saved by ctx_save(). */ void (*ctx_restore)(void *ctx); - /* status() - status string generation - When the boot log is defined, this function is called to print - information returned by the driver, for debugging purposes. This is - expected to be a short (max 21 bytes) string because only a few - lines are available for all drivers. - Returns a pointer to a string; a static buffer is suitable. */ + /* This function may generate a status string to display on the boot + log for debugging purposes. It should be a short (max 21 bytes) + string because all drivers' strings must fit on a few lines. May + return a pointer to a static buffer. May be NULL. */ char const * (*status)(void); } GPACKED(4) gint_driver_t; @@ -100,7 +100,7 @@ typedef struct #define GINT_DECLARE_DRIVER(level, name) \ GSECTION(".gint.drivers." #level) extern gint_driver_t name; -/* GINT_DRIVER_SH3() - declare a function for SH3-rectification +/* GINT_DRIVER_SH3() - declare a function for SH3 preinitialization This macro makes its argument NULL on fxcg50, this way the named function can be defined under #ifdef FX9860G while keeping the structure clean. */ @@ -111,7 +111,7 @@ typedef struct #endif /* GINT_DRIVER_STATUS() - declare a function for status string generation - This macro makes its argument NULL when GINT_BOOT_LOG is defuned, this way + This macro makes its argument NULL when GINT_BOOT_LOG is undefined, this way the named function can be defined under #ifdef GINT_BOOT_LOG while keeping the structure clean. */ diff --git a/src/core/setup.c b/src/core/setup.c index 4ac516d..452c0f3 100644 --- a/src/core/setup.c +++ b/src/core/setup.c @@ -126,7 +126,6 @@ static void unlock(void) to honor the dependency system */ for(gint_driver_t *drv = &edrv; (--drv) >= &bdrv;) { - if(drv->unload) drv->unload(); if(drv->ctx_restore) drv->ctx_restore(drv->sys_ctx); } @@ -136,6 +135,12 @@ static void unlock(void) /* gint_unload() - unload gint and give back control to the system */ void gint_unload(void) { + /* First wait for all the drivers to finish their current jobs */ + for(gint_driver_t *drv = &edrv; (--drv) >= &bdrv;) + { + if(drv->wait) drv->wait(); + } + gint_setvbr(system_vbr, unlock); } @@ -187,6 +192,12 @@ static void gint_switch_in(void) /* gint_switch() - temporarily switch out of gint */ void gint_switch(void (*function)(void)) { + /* Wait for all the drivers to finish their current jobs */ + for(gint_driver_t *drv = &edrv; (--drv) >= &bdrv;) + { + if(drv->wait) drv->wait(); + } + gint_setvbr(system_vbr, gint_switch_out); if(function) function(); gint_setvbr((uint32_t)&gint_vbr, gint_switch_in); diff --git a/src/core/start.c b/src/core/start.c index 8710453..09b330a 100644 --- a/src/core/start.c +++ b/src/core/start.c @@ -184,11 +184,6 @@ int start(int isappli, int optnum) /* Unload gint and give back control to the system. Driver settings will be restored while interrupts are disabled */ - #ifdef FXCG50 - /* TODO: Put that in the driver model, stupid! */ - /* TODO: Also do it in gint_switch()! */ - dma_transfer_wait(-1); - #endif gint_unload(); /* TODO: Invoke main menu instead of returning? */ diff --git a/src/cpg/cpg.c b/src/cpg/cpg.c index 7145f0b..1d4cf9b 100644 --- a/src/cpg/cpg.c +++ b/src/cpg/cpg.c @@ -151,9 +151,9 @@ static void init(void) } gint_driver_t drv_cpg = { - .name = "CPG", - .init = init, - .status = GINT_DRIVER_STATUS(cpg_status), + .name = "CPG", + .init = init, + .status = GINT_DRIVER_STATUS(cpg_status), }; GINT_DECLARE_DRIVER(1, drv_cpg); diff --git a/src/dma/dma.c b/src/dma/dma.c index 5461fc8..4d5b1e8 100644 --- a/src/dma/dma.c +++ b/src/dma/dma.c @@ -215,7 +215,7 @@ static void init(void) gint[HWDMA] = HW_LOADED; } -static void unload(void) +static void wait(void) { /* Make sure any DMA transfer is finished before leaving the app */ dma_transfer_wait(-1); @@ -286,13 +286,13 @@ static void ctx_restore(void *buf) //--- gint_driver_t drv_dma0 = { - .name = "DMA0", - .init = init, - .unload = unload, - .sys_ctx = &sys_ctx, - .gint_ctx = &gint_ctx, - .ctx_save = ctx_save, - .ctx_restore = ctx_restore, + .name = "DMA0", + .init = init, + .wait = wait, + .sys_ctx = &sys_ctx, + .gint_ctx = &gint_ctx, + .ctx_save = ctx_save, + .ctx_restore = ctx_restore, }; GINT_DECLARE_DRIVER(2, drv_dma0); diff --git a/src/keysc/getkey.c b/src/keysc/getkey.c index 651a756..0bcde25 100644 --- a/src/keysc/getkey.c +++ b/src/keysc/getkey.c @@ -3,6 +3,7 @@ //--- #include +#include #include #ifdef FX9860G diff --git a/src/keysc/keysc.c b/src/keysc/keysc.c index 0323d4d..695f0ff 100644 --- a/src/keysc/keysc.c +++ b/src/keysc/keysc.c @@ -269,6 +269,7 @@ static void init(void) /* Set the default repeat times (milliseconds) */ getkey_repeat(400, 40); + /* The timer will be stopped when the timer driver is unloaded */ timer_setup(tid, delay, 0, callback, NULL); timer_start(tid); @@ -276,13 +277,6 @@ static void init(void) gint[HWKBDSF] = KEYBOARD_SCAN_FREQUENCY; } -/* unload() - stop the support timer */ -static void unload(void) -{ - int tid = isSH3() ? 3 : 8; - timer_stop(tid); -} - #ifdef GINT_BOOT_LOG /* keysc_status() - status string of the driver */ @@ -300,10 +294,9 @@ static const char *keysc_status(void) //--- gint_driver_t drv_keysc = { - .name = "KEYSC", - .init = init, - .status = GINT_DRIVER_STATUS(keysc_status), - .unload = unload, + .name = "KEYSC", + .init = init, + .status = GINT_DRIVER_STATUS(keysc_status), }; GINT_DECLARE_DRIVER(4, drv_keysc); diff --git a/src/r61524/r61524.c b/src/r61524/r61524.c index fe26e41..e92718b 100644 --- a/src/r61524/r61524.c +++ b/src/r61524/r61524.c @@ -298,13 +298,13 @@ static const char *r61524_status(void) //--- gint_driver_t drv_r61524 = { - .name = "R61524", - .init = init, - .status = GINT_DRIVER_STATUS(r61524_status), - .sys_ctx = &sys_ctx, - .gint_ctx = &gint_ctx, - .ctx_save = ctx_save, - .ctx_restore = ctx_restore, + .name = "R61524", + .init = init, + .status = GINT_DRIVER_STATUS(r61524_status), + .sys_ctx = &sys_ctx, + .gint_ctx = &gint_ctx, + .ctx_save = ctx_save, + .ctx_restore = ctx_restore, }; GINT_DECLARE_DRIVER(5, drv_r61524); diff --git a/src/rtc/rtc.c b/src/rtc/rtc.c index bc4f437..cb3dd80 100644 --- a/src/rtc/rtc.c +++ b/src/rtc/rtc.c @@ -183,13 +183,13 @@ static void ctx_restore(void *buf) //--- gint_driver_t drv_rtc = { - .name = "RTC", - .driver_sh3 = GINT_DRIVER_SH3(driver_sh3), - .init = init, - .sys_ctx = &sys_ctx, - .gint_ctx = &gint_ctx, - .ctx_save = ctx_save, - .ctx_restore = ctx_restore, + .name = "RTC", + .driver_sh3 = GINT_DRIVER_SH3(driver_sh3), + .init = init, + .sys_ctx = &sys_ctx, + .gint_ctx = &gint_ctx, + .ctx_save = ctx_save, + .ctx_restore = ctx_restore, }; GINT_DECLARE_DRIVER(2, drv_rtc); diff --git a/src/t6k11/t6k11.c b/src/t6k11/t6k11.c index 77b0ffb..aa95868 100644 --- a/src/t6k11/t6k11.c +++ b/src/t6k11/t6k11.c @@ -251,13 +251,13 @@ static const char *t6k11_status(void) //--- gint_driver_t drv_t6k11 = { - .name = "T6K11", - .init = init, - .status = GINT_DRIVER_STATUS(t6k11_status), - .sys_ctx = &sys_ctx, - .gint_ctx = &gint_ctx, - .ctx_save = ctx_save, - .ctx_restore = ctx_restore, + .name = "T6K11", + .init = init, + .status = GINT_DRIVER_STATUS(t6k11_status), + .sys_ctx = &sys_ctx, + .gint_ctx = &gint_ctx, + .ctx_save = ctx_save, + .ctx_restore = ctx_restore, }; GINT_DECLARE_DRIVER(5, drv_t6k11); diff --git a/src/tmu/tmu.c b/src/tmu/tmu.c index 5d76498..d94bb7e 100644 --- a/src/tmu/tmu.c +++ b/src/tmu/tmu.c @@ -428,14 +428,14 @@ static void ctx_restore(void *buf) //--- gint_driver_t drv_tmu = { - .name = "TMU", - .driver_sh3 = GINT_DRIVER_SH3(driver_sh3), - .init = init, - .status = GINT_DRIVER_STATUS(tmu_status), - .sys_ctx = &sys_ctx, - .gint_ctx = &gint_ctx, - .ctx_save = ctx_save, - .ctx_restore = ctx_restore, + .name = "TMU", + .driver_sh3 = GINT_DRIVER_SH3(driver_sh3), + .init = init, + .status = GINT_DRIVER_STATUS(tmu_status), + .sys_ctx = &sys_ctx, + .gint_ctx = &gint_ctx, + .ctx_save = ctx_save, + .ctx_restore = ctx_restore, }; GINT_DECLARE_DRIVER(2, drv_tmu);