From efebce681105507713d35edd1b6d55ea2dccff00 Mon Sep 17 00:00:00 2001 From: Ken Van Hoeylandt Date: Fri, 30 Jan 2026 23:04:36 +0100 Subject: [PATCH] Early commit to trigger code review due to file limit on upcoming changes --- .../DevicetreeCompiler/source/generator.py | 12 +- Devices/lilygo-tdeck/Source/module.cpp | 22 +++ .../Source/drivers/Register.cpp | 10 - Devices/lilygo-tlora-pager/Source/module.cpp | 30 +++ Firmware/Source/Main.cpp | 16 +- .../Include/tactility/drivers/esp32_i2c.h | 2 - .../Source/drivers/esp32_gpio.cpp | 5 +- .../Source/drivers/esp32_i2c.cpp | 13 +- .../PlatformEsp32/Source/drivers/register.cpp | 13 -- Platforms/PlatformEsp32/Source/module.cpp | 33 ++++ Platforms/PlatformPosix/Source/Register.cpp | 10 - Platforms/PlatformPosix/Source/module.cpp | 25 +++ Tactility/Include/Tactility/Tactility.h | 5 +- Tactility/Source/Tactility.cpp | 11 +- .../tactility/concurrent/recursive_mutex.h | 1 + TactilityKernel/Include/tactility/device.h | 10 + TactilityKernel/Include/tactility/driver.h | 7 +- TactilityKernel/Include/tactility/error.h | 1 + .../Include/tactility/kernel_init.h | 22 +++ TactilityKernel/Include/tactility/module.h | 112 +++++++++++ TactilityKernel/Source/driver.cpp | 44 +++-- TactilityKernel/Source/drivers/register.cpp | 12 -- TactilityKernel/Source/drivers/root.cpp | 3 +- TactilityKernel/Source/kernel_init.cpp | 58 ++++++ TactilityKernel/Source/module.cpp | 120 ++++++++++++ Tests/TactilityKernel/DeviceTest.cpp | 10 +- .../TactilityKernel/DriverIntegrationTest.cpp | 10 +- Tests/TactilityKernel/DriverTest.cpp | 28 ++- Tests/TactilityKernel/ModuleTest.cpp | 174 ++++++++++++++++++ 29 files changed, 714 insertions(+), 105 deletions(-) create mode 100644 Devices/lilygo-tdeck/Source/module.cpp delete mode 100644 Devices/lilygo-tlora-pager/Source/drivers/Register.cpp create mode 100644 Devices/lilygo-tlora-pager/Source/module.cpp delete mode 100644 Platforms/PlatformEsp32/Source/drivers/register.cpp create mode 100644 Platforms/PlatformEsp32/Source/module.cpp delete mode 100644 Platforms/PlatformPosix/Source/Register.cpp create mode 100644 Platforms/PlatformPosix/Source/module.cpp create mode 100644 TactilityKernel/Include/tactility/kernel_init.h create mode 100644 TactilityKernel/Include/tactility/module.h delete mode 100644 TactilityKernel/Source/drivers/register.cpp create mode 100644 TactilityKernel/Source/kernel_init.cpp create mode 100644 TactilityKernel/Source/module.cpp create mode 100644 Tests/TactilityKernel/ModuleTest.cpp diff --git a/Buildscripts/DevicetreeCompiler/source/generator.py b/Buildscripts/DevicetreeCompiler/source/generator.py index 49d88d3a..795077f0 100644 --- a/Buildscripts/DevicetreeCompiler/source/generator.py +++ b/Buildscripts/DevicetreeCompiler/source/generator.py @@ -135,7 +135,7 @@ def write_device_init(file, device: Device, bindings: list[Binding], verbose: bo identifier = get_device_identifier_safe(device) device_variable = identifier # Write device struct - file.write(f"\tif (device_construct_add_start(&{device_variable}, \"{compatible_property.value}\") != ERROR_NONE) return ERROR_RESOURCE;\n") + file.write("\t{ " f"&{device_variable}, \"{compatible_property.value}\"" " },\n") # Write children for child_device in device.devices: write_device_init(file, child_device, bindings, verbose) @@ -145,8 +145,6 @@ def generate_devicetree_c(filename: str, items: list[object], bindings: list[Bin file.write(dedent('''\ // Default headers #include - #include - #include // DTS headers ''')) @@ -161,14 +159,14 @@ def generate_devicetree_c(filename: str, items: list[object], bindings: list[Bin if type(item) is Device: write_device_structs(file, item, None, bindings, verbose) # Init function body start - file.write("error_t devices_builtin_init() {\n") + file.write("struct CompatibleDevice devicetree_devices[] = {\n") # Init function body logic for item in items: if type(item) is Device: write_device_init(file, item, bindings, verbose) - file.write("\treturn ERROR_NONE;\n") # Init function body end - file.write("}\n") + file.write("\t{ NULL, NULL },\n") + file.write("};\n") def generate_devicetree_h(filename: str): with open(filename, "w") as file: @@ -180,7 +178,7 @@ def generate_devicetree_h(filename: str): extern "C" { #endif - extern error_t devices_builtin_init(); + extern struct CompatibleDevice devicetree_devices[]; #ifdef __cplusplus } diff --git a/Devices/lilygo-tdeck/Source/module.cpp b/Devices/lilygo-tdeck/Source/module.cpp new file mode 100644 index 00000000..78373614 --- /dev/null +++ b/Devices/lilygo-tdeck/Source/module.cpp @@ -0,0 +1,22 @@ +#include + +extern "C" { + +static error_t start() { + // Empty for now + return ERROR_NONE; +} + +static error_t stop() { + // Empty for now + return ERROR_NONE; +} + +/** @warn The variable name must be exactly "platform_module" */ +struct Module device_module = { + .name = "LilyGO T-Deck", + .start = start, + .stop = stop +}; + +} diff --git a/Devices/lilygo-tlora-pager/Source/drivers/Register.cpp b/Devices/lilygo-tlora-pager/Source/drivers/Register.cpp deleted file mode 100644 index 3da59c43..00000000 --- a/Devices/lilygo-tlora-pager/Source/drivers/Register.cpp +++ /dev/null @@ -1,10 +0,0 @@ -#include - -extern "C" { - -extern void register_device_drivers() { - extern Driver tlora_pager_driver; - driver_construct(&tlora_pager_driver); -} - -} diff --git a/Devices/lilygo-tlora-pager/Source/module.cpp b/Devices/lilygo-tlora-pager/Source/module.cpp new file mode 100644 index 00000000..cae58099 --- /dev/null +++ b/Devices/lilygo-tlora-pager/Source/module.cpp @@ -0,0 +1,30 @@ +#include +#include +#include + +extern "C" { + +extern Driver tlora_pager_driver; + +static error_t start() { + /* We crash when destruct fails, because if a single driver fails to construct, + * there is no guarantee that the previously constructed drivers can be destroyed */ + check(driver_construct(&tlora_pager_driver) == ERROR_NONE); + return ERROR_NONE; +} + +static error_t stop() { + /* We crash when destruct fails, because if a single driver fails to destruct, + * there is no guarantee that the previously destroyed drivers can be recovered */ + check(driver_destruct(&tlora_pager_driver) == ERROR_NONE); + return ERROR_NONE; +} + +/** @warn The variable name must be exactly "platform_module" */ +struct Module device_module = { + .name = "LilyGO T-Lora Pager", + .start = start, + .stop = stop +}; + +} diff --git a/Firmware/Source/Main.cpp b/Firmware/Source/Main.cpp index aa48542d..e9f78253 100644 --- a/Firmware/Source/Main.cpp +++ b/Firmware/Source/Main.cpp @@ -3,6 +3,11 @@ #include #include +// From the relevant platform +extern struct Module platform_module; +// From the relevant device +extern struct Module device_module; + #ifdef ESP_PLATFORM #include #else @@ -14,10 +19,6 @@ extern const tt::hal::Configuration hardwareConfiguration; extern "C" { -extern void register_kernel_drivers(); -extern void register_platform_drivers(); -extern void register_device_drivers(); - void app_main() { static const tt::Configuration config = { /** @@ -31,12 +32,7 @@ void app_main() { tt_init_tactility_c(); // ELF bindings for side-loading on ESP32 #endif - register_kernel_drivers(); - register_platform_drivers(); - register_device_drivers(); - - devices_builtin_init(); - tt::run(config); + tt::run(config, &platform_module, &device_module, devicetree_devices); } } // extern \ No newline at end of file diff --git a/Platforms/PlatformEsp32/Include/tactility/drivers/esp32_i2c.h b/Platforms/PlatformEsp32/Include/tactility/drivers/esp32_i2c.h index e210ddcd..67fc6d83 100644 --- a/Platforms/PlatformEsp32/Include/tactility/drivers/esp32_i2c.h +++ b/Platforms/PlatformEsp32/Include/tactility/drivers/esp32_i2c.h @@ -18,8 +18,6 @@ struct Esp32I2cConfig { }; error_t esp32_i2c_get_port(struct Device* device, i2c_port_t* port); -void esp32_i2c_lock(struct Device* device); -void esp32_i2c_unlock(struct Device* device); #ifdef __cplusplus } diff --git a/Platforms/PlatformEsp32/Source/drivers/esp32_gpio.cpp b/Platforms/PlatformEsp32/Source/drivers/esp32_gpio.cpp index 13f86132..d2e55482 100644 --- a/Platforms/PlatformEsp32/Source/drivers/esp32_gpio.cpp +++ b/Platforms/PlatformEsp32/Source/drivers/esp32_gpio.cpp @@ -107,6 +107,8 @@ const static GpioControllerApi esp32_gpio_api = { .get_options = get_options }; +extern struct Module platform_module; + Driver esp32_gpio_driver = { .name = "esp32_gpio", .compatible = (const char*[]) { "espressif,esp32-gpio", nullptr }, @@ -114,7 +116,8 @@ Driver esp32_gpio_driver = { .stopDevice = stop, .api = (void*)&esp32_gpio_api, .deviceType = nullptr, - .internal = { 0 } + .owner = &platform_module, + .internal = nullptr }; } // extern "C" diff --git a/Platforms/PlatformEsp32/Source/drivers/esp32_i2c.cpp b/Platforms/PlatformEsp32/Source/drivers/esp32_i2c.cpp index c78b824f..2e22f4e9 100644 --- a/Platforms/PlatformEsp32/Source/drivers/esp32_i2c.cpp +++ b/Platforms/PlatformEsp32/Source/drivers/esp32_i2c.cpp @@ -150,14 +150,6 @@ error_t esp32_i2c_get_port(struct Device* device, i2c_port_t* port) { return ERROR_NONE; } -void esp32_i2c_lock(struct Device* device) { - mutex_lock(&GET_DATA(device)->mutex); -} - -void esp32_i2c_unlock(struct Device* device) { - mutex_unlock(&GET_DATA(device)->mutex); -} - static error_t start(Device* device) { ESP_LOGI(TAG, "start %s", device->name); auto dts_config = GET_CONFIG(device); @@ -214,6 +206,8 @@ const static I2cControllerApi esp32_i2c_api = { .write_register = write_register }; +extern struct Module platform_module; + Driver esp32_i2c_driver = { .name = "esp32_i2c", .compatible = (const char*[]) { "espressif,esp32-i2c", nullptr }, @@ -221,7 +215,8 @@ Driver esp32_i2c_driver = { .stopDevice = stop, .api = (void*)&esp32_i2c_api, .deviceType = &I2C_CONTROLLER_TYPE, - .internal = { 0 } + .owner = &platform_module, + .internal = nullptr }; } // extern "C" diff --git a/Platforms/PlatformEsp32/Source/drivers/register.cpp b/Platforms/PlatformEsp32/Source/drivers/register.cpp deleted file mode 100644 index 4bdba9e4..00000000 --- a/Platforms/PlatformEsp32/Source/drivers/register.cpp +++ /dev/null @@ -1,13 +0,0 @@ -// SPDX-License-Identifier: Apache-2.0 -#include - -extern "C" { - -extern void register_platform_drivers() { - extern Driver esp32_gpio_driver; - driver_construct(&esp32_gpio_driver); - extern Driver esp32_i2c_driver; - driver_construct(&esp32_i2c_driver); -} - -} diff --git a/Platforms/PlatformEsp32/Source/module.cpp b/Platforms/PlatformEsp32/Source/module.cpp new file mode 100644 index 00000000..c165d202 --- /dev/null +++ b/Platforms/PlatformEsp32/Source/module.cpp @@ -0,0 +1,33 @@ +#include +#include +#include + +extern "C" { + +extern Driver esp32_gpio_driver; +extern Driver esp32_i2c_driver; + +static error_t start() { + /* We crash when destruct fails, because if a single driver fails to construct, + * there is no guarantee that the previously constructed drivers can be destroyed */ + check(driver_construct(&esp32_gpio_driver) == ERROR_NONE); + check(driver_construct(&esp32_i2c_driver) == ERROR_NONE); + return ERROR_NONE; +} + +static error_t stop() { + /* We crash when destruct fails, because if a single driver fails to destruct, + * there is no guarantee that the previously destroyed drivers can be recovered */ + check(driver_destruct(&esp32_gpio_driver) == ERROR_NONE); + check(driver_destruct(&esp32_i2c_driver) == ERROR_NONE); + return ERROR_NONE; +} + +// The name must be exactly "platform_module" +struct Module platform_module = { + .name = "ESP32 Platform", + .start = start, + .stop = stop +}; + +} diff --git a/Platforms/PlatformPosix/Source/Register.cpp b/Platforms/PlatformPosix/Source/Register.cpp deleted file mode 100644 index ff1f5e83..00000000 --- a/Platforms/PlatformPosix/Source/Register.cpp +++ /dev/null @@ -1,10 +0,0 @@ -// SPDX-License-Identifier: Apache-2.0 -#include - -extern "C" { - -extern void register_platform_drivers() { - /* Placeholder */ -} - -} diff --git a/Platforms/PlatformPosix/Source/module.cpp b/Platforms/PlatformPosix/Source/module.cpp new file mode 100644 index 00000000..f1b83b79 --- /dev/null +++ b/Platforms/PlatformPosix/Source/module.cpp @@ -0,0 +1,25 @@ +#include + +extern "C" { + +extern Driver esp32_gpio_driver; +extern Driver esp32_i2c_driver; + +static error_t start() { + /* NO-OP for now */ + return ERROR_NONE; +} + +static error_t stop() { + /* NO-OP for now */ + return ERROR_NONE; +} + +// The name must be exactly "platform_module" +struct Module platform_module = { + .name = "POSIX Platform", + .start = start, + .stop = stop +}; + +} diff --git a/Tactility/Include/Tactility/Tactility.h b/Tactility/Include/Tactility/Tactility.h index b0fc801e..44bfd720 100644 --- a/Tactility/Include/Tactility/Tactility.h +++ b/Tactility/Include/Tactility/Tactility.h @@ -1,5 +1,7 @@ #pragma once +#include "tactility/device.h" +#include "tactility/module.h" #include #include #include @@ -17,9 +19,8 @@ struct Configuration { /** * Attempts to initialize Tactility and all configured hardware. - * @param[in] config */ -void run(const Configuration& config); +void run(const Configuration& config, Module* platformModule, Module* deviceModule, CompatibleDevice devicetreeDevices[]); /** * While technically nullable, this instance is always set if tt_init() succeeds. diff --git a/Tactility/Source/Tactility.cpp b/Tactility/Source/Tactility.cpp index 83a1e83b..365ad839 100644 --- a/Tactility/Source/Tactility.cpp +++ b/Tactility/Source/Tactility.cpp @@ -22,6 +22,8 @@ #include #include +#include + #include #include @@ -317,10 +319,17 @@ void registerApps() { registerInstalledAppsFromSdCards(); } -void run(const Configuration& config) { +void run(const Configuration& config, Module* platformModule, Module* deviceModule, CompatibleDevice devicetreeDevices[]) { LOGGER.info("Tactility v{} on {} ({})", TT_VERSION, CONFIG_TT_DEVICE_NAME, CONFIG_TT_DEVICE_ID); assert(config.hardware); + + LOGGER.info(R"(Calling kernel_init with modules: "{}" and "{}")", platformModule->name, deviceModule->name); + if (kernel_init(platformModule, deviceModule, devicetreeDevices) != ERROR_NONE) { + LOGGER.error("Failed to initialize kernel"); + return; + } + const hal::Configuration& hardware = *config.hardware; // Assign early so starting services can use it diff --git a/TactilityKernel/Include/tactility/concurrent/recursive_mutex.h b/TactilityKernel/Include/tactility/concurrent/recursive_mutex.h index afa6d8b2..0863f81e 100644 --- a/TactilityKernel/Include/tactility/concurrent/recursive_mutex.h +++ b/TactilityKernel/Include/tactility/concurrent/recursive_mutex.h @@ -15,6 +15,7 @@ struct RecursiveMutex { }; inline static void recursive_mutex_construct(struct RecursiveMutex* mutex) { + check(mutex->handle == NULL); mutex->handle = xSemaphoreCreateRecursiveMutex(); } diff --git a/TactilityKernel/Include/tactility/device.h b/TactilityKernel/Include/tactility/device.h index a9560972..ef6d0f5f 100644 --- a/TactilityKernel/Include/tactility/device.h +++ b/TactilityKernel/Include/tactility/device.h @@ -50,6 +50,16 @@ struct Device { } internal; }; +/** + * Holds a device pointer and a compatible string. + * The device must not be constructed, added or started yet. + * This is mused by the devicetree code generator and the application init sequence. + */ +struct CompatibleDevice { + struct Device* device; + const char* compatible; +}; + /** * Initialize the properties of a device. * diff --git a/TactilityKernel/Include/tactility/driver.h b/TactilityKernel/Include/tactility/driver.h index f90ca2e3..4b52f9a3 100644 --- a/TactilityKernel/Include/tactility/driver.h +++ b/TactilityKernel/Include/tactility/driver.h @@ -25,11 +25,10 @@ struct Driver { const void* api; /** Which type of devices this driver creates (can be NULL) */ const struct DeviceType* deviceType; + /** The module that owns this driver. When it is NULL, the system owns the driver and it cannot be removed from registration. */ + const struct Module* owner; /** Internal data */ - struct { - /** Contains private data */ - void* data; - } internal; + void* internal; }; error_t driver_construct(struct Driver* driver); diff --git a/TactilityKernel/Include/tactility/error.h b/TactilityKernel/Include/tactility/error.h index 00aae3bf..f01ee259 100644 --- a/TactilityKernel/Include/tactility/error.h +++ b/TactilityKernel/Include/tactility/error.h @@ -22,6 +22,7 @@ typedef int error_t; #define ERROR_TIMEOUT 8 #define ERROR_OUT_OF_MEMORY 9 #define ERROR_NOT_SUPPORTED 10 +#define ERROR_NOT_ALLOWED 11 /** Convert an error_t to a human-readable text. Useful for logging. */ const char* error_to_string(error_t error); diff --git a/TactilityKernel/Include/tactility/kernel_init.h b/TactilityKernel/Include/tactility/kernel_init.h new file mode 100644 index 00000000..421761a6 --- /dev/null +++ b/TactilityKernel/Include/tactility/kernel_init.h @@ -0,0 +1,22 @@ +#pragma once + +#ifdef __cplusplus +extern "C" { +#endif + +#include +#include +#include + +/** + * Initialize the kernel with platform and device modules, and a device tree. + * @param platform_module The platform module to start. This module should not be constructed yet. + * @param device_module The device module to start. This module should not be constructed yet. + * @param devicetree_devices The list of generated devices from the devicetree. + * @return ERROR_NONE on success, otherwise an error code + */ +error_t kernel_init(struct Module* platform_module, struct Module* device_module, struct CompatibleDevice devicetree_devices[]); + +#ifdef __cplusplus +} +#endif diff --git a/TactilityKernel/Include/tactility/module.h b/TactilityKernel/Include/tactility/module.h new file mode 100644 index 00000000..c41a740d --- /dev/null +++ b/TactilityKernel/Include/tactility/module.h @@ -0,0 +1,112 @@ +#pragma once + +#ifdef __cplusplus +extern "C" { +#endif + +#include "error.h" +#include + +struct ModuleParent; + +/** + * A module is a collection of drivers or other functionality that can be loaded and unloaded at runtime. + */ +struct Module { + /** + * The name of the module, for logging/debugging purposes + * Should never be NULL. + */ + const char* name; + + /** + * A function to initialize the module. + * Should never be NULL. + * This can be used to load drivers, initialize hardware, etc. + * @return ERROR_NONE if successful + */ + error_t (*start)(void); + + /** + * Deinitializes the module. + * Should never be NULL. + * @return ERROR_NONE if successful + */ + error_t (*stop)(void); + + struct { + bool started; + struct ModuleParent* parent; + } internal; +}; + +/** + * A module parent is a collection of modules that can be loaded and unloaded at runtime. + */ +struct ModuleParent { + /** The name of the parent module, for logging/debugging purposes */ + const char* name; + struct { + void* data; + } internal; +}; + +/** + * @brief Initialize the module parent. + * @warn This function does no validation on input or state. + * @param parent parent module + * @return ERROR_NONE if successful, ERROR_OUT_OF_MEMORY if allocation fails + */ +error_t module_parent_construct(struct ModuleParent* parent); + +/** + * @brief Deinitialize the module parent. Must have no children when calling this. + * @warn This function does no validation on input. + * @param parent parent module + * @return ERROR_NONE if successful or ERROR_INVALID_STATE if the parent has children + */ +error_t module_parent_destruct(struct ModuleParent* parent); + +/** + * @brief Set the parent of the module. + * @warning must call before module_start() + * @param module module + * @param parent nullable parent module + * @return ERROR_NONE if successful, ERROR_INVALID_STATE if the module is already started + */ +error_t module_set_parent(struct Module* module, struct ModuleParent* parent); + +/** + * @brief Start the module. + * @param module module + * @return ERROR_NONE if already started, ERROR_INVALID_STATE if the module doesn't have a parent, or otherwise it returns the result of the module's start function + */ +error_t module_start(struct Module* module); + +/** + * @brief Check if the module is started. + * @param module module to check + * @return true if the module is started, false otherwise + */ +bool module_is_started(struct Module* module); + +/** + * @brief Stop the module. + * @param module module + * @return ERROR_NONE if successful or if the module wasn't started, or otherwise it returns the result of the module's stop function + */ +error_t module_stop(struct Module* module); + +error_t kernel_module_parent_construct(void); + +error_t kernel_module_parent_destruct(void); + +/** + * @brief Get the kernel module parent. This parent is the root parent for kernel modules. + * @return the kernel module parent + */ +struct ModuleParent* get_kernel_module_parent(void); + +#ifdef __cplusplus +} +#endif diff --git a/TactilityKernel/Source/driver.cpp b/TactilityKernel/Source/driver.cpp index e32ffa92..0f96ad15 100644 --- a/TactilityKernel/Source/driver.cpp +++ b/TactilityKernel/Source/driver.cpp @@ -12,16 +12,16 @@ #define TAG LOG_TAG(driver) -struct DriverInternalData { +struct DriverInternal { Mutex mutex { 0 }; int use_count = 0; bool destroying = false; - DriverInternalData() { + DriverInternal() { mutex_construct(&mutex); } - ~DriverInternalData() { + ~DriverInternal() { mutex_destruct(&mutex); } }; @@ -54,9 +54,9 @@ static DriverLedger& get_ledger() { #define ledger get_ledger() -#define driver_internal_data(driver) static_cast(driver->internal.data) -#define driver_lock(driver) mutex_lock(&driver_internal_data(driver)->mutex); -#define driver_unlock(driver) mutex_unlock(&driver_internal_data(driver)->mutex); +#define get_internal(driver) static_cast(driver->internal) +#define driver_lock(driver) mutex_lock(&get_internal(driver)->mutex); +#define driver_unlock(driver) mutex_unlock(&get_internal(driver)->mutex); static void driver_add(Driver* driver) { LOG_I(TAG, "add %s", driver->name); @@ -83,8 +83,8 @@ static error_t driver_remove(Driver* driver) { extern "C" { error_t driver_construct(Driver* driver) { - driver->internal.data = new(std::nothrow) DriverInternalData; - if (driver->internal.data == nullptr) { + driver->internal = new(std::nothrow) DriverInternal; + if (driver->internal == nullptr) { return ERROR_OUT_OF_MEMORY; } driver_add(driver); @@ -93,19 +93,29 @@ error_t driver_construct(Driver* driver) { error_t driver_destruct(Driver* driver) { driver_lock(driver); - if (driver_internal_data(driver)->use_count != 0 || driver_internal_data(driver)->destroying) { + // No module means the system owns it and it cannot be destroyed + if (driver->owner == nullptr) { + driver_unlock(driver); + return ERROR_NOT_ALLOWED; + } + if (get_internal(driver)->use_count != 0 || get_internal(driver)->destroying) { driver_unlock(driver); return ERROR_INVALID_STATE; } - driver_internal_data(driver)->destroying = true; - driver_unlock(driver); + get_internal(driver)->destroying = true; if (driver_remove(driver) != ERROR_NONE) { LOG_W(TAG, "Failed to remove driver from ledger: %s", driver->name); } - delete driver_internal_data(driver); - driver->internal.data = nullptr; + // Copy the mutex so we can free the driver's memory and unlock the mutex later + struct Mutex mutex_copy; + memcpy(&mutex_copy, &get_internal(driver)->mutex, sizeof(Mutex)); + + delete get_internal(driver); + driver->internal = nullptr; + + mutex_unlock(&mutex_copy); return ERROR_NONE; } @@ -140,7 +150,7 @@ error_t driver_bind(Driver* driver, Device* device) { driver_lock(driver); error_t error = ERROR_NONE; - if (driver_internal_data(driver)->destroying || !device_is_added(device)) { + if (get_internal(driver)->destroying || !device_is_added(device)) { error = ERROR_INVALID_STATE; goto error; } @@ -152,7 +162,7 @@ error_t driver_bind(Driver* driver, Device* device) { } } - driver_internal_data(driver)->use_count++; + get_internal(driver)->use_count++; driver_unlock(driver); LOG_I(TAG, "bound %s to %s", driver->name, device->name); @@ -168,7 +178,7 @@ error_t driver_unbind(Driver* driver, Device* device) { driver_lock(driver); error_t error = ERROR_NONE; - if (driver_internal_data(driver)->destroying || !device_is_added(device)) { + if (get_internal(driver)->destroying || !device_is_added(device)) { error = ERROR_INVALID_STATE; goto error; } @@ -180,7 +190,7 @@ error_t driver_unbind(Driver* driver, Device* device) { } } - driver_internal_data(driver)->use_count--; + get_internal(driver)->use_count--; driver_unlock(driver); LOG_I(TAG, "unbound %s from %s", driver->name, device->name); diff --git a/TactilityKernel/Source/drivers/register.cpp b/TactilityKernel/Source/drivers/register.cpp deleted file mode 100644 index baec0594..00000000 --- a/TactilityKernel/Source/drivers/register.cpp +++ /dev/null @@ -1,12 +0,0 @@ -// SPDX-License-Identifier: Apache-2.0 - -#include - -extern "C" { - -extern void register_kernel_drivers() { - extern Driver root_driver; - driver_construct(&root_driver); -} - -} diff --git a/TactilityKernel/Source/drivers/root.cpp b/TactilityKernel/Source/drivers/root.cpp index e8fef4fb..1be6b474 100644 --- a/TactilityKernel/Source/drivers/root.cpp +++ b/TactilityKernel/Source/drivers/root.cpp @@ -12,7 +12,8 @@ Driver root_driver = { .stopDevice = nullptr, .api = nullptr, .deviceType = nullptr, - .internal = { 0 } + .owner = nullptr, + .internal = nullptr }; } diff --git a/TactilityKernel/Source/kernel_init.cpp b/TactilityKernel/Source/kernel_init.cpp new file mode 100644 index 00000000..da46d446 --- /dev/null +++ b/TactilityKernel/Source/kernel_init.cpp @@ -0,0 +1,58 @@ +#include +#include + +#ifdef __cplusplus +extern "C" { +#endif + +#define TAG LOG_TAG(kernel) + +struct ModuleParent kernel_module_parent; + +static error_t init_kernel_drivers() { + extern Driver root_driver; + if (driver_construct(&root_driver) != ERROR_NONE) return ERROR_RESOURCE; + return ERROR_NONE; +} + +error_t kernel_init(struct Module* platform_module, struct Module* device_module, struct CompatibleDevice devicetree_devices[]) { + LOG_I(TAG, "init"); + + if (module_parent_construct(&kernel_module_parent) != ERROR_NONE) { + LOG_E(TAG, "init failed to create kernel module parent"); + return ERROR_RESOURCE; + } + + if (init_kernel_drivers() != ERROR_NONE) { + LOG_E(TAG, "init failed to init kernel drivers"); + return ERROR_RESOURCE; + } + + module_set_parent(platform_module, &kernel_module_parent); + if (module_start(platform_module) != ERROR_NONE) { + LOG_E(TAG, "init failed to start platform module"); + return ERROR_RESOURCE; + } + + module_set_parent(device_module, &kernel_module_parent); + if (module_start(device_module) != ERROR_NONE) { + LOG_E(TAG, "init failed to start device module"); + return ERROR_RESOURCE; + } + + CompatibleDevice* compatible_device = devicetree_devices; + while (compatible_device->device != nullptr) { + if (device_construct_add_start(compatible_device->device, compatible_device->compatible) != ERROR_NONE) { + LOG_E(TAG, "kernel_init failed to construct device: %s (%s)", compatible_device->device->name, compatible_device->compatible); + return ERROR_RESOURCE; + } + compatible_device++; + } + + LOG_I(TAG, "init done"); + return ERROR_NONE; +}; + +#ifdef __cplusplus +} +#endif diff --git a/TactilityKernel/Source/module.cpp b/TactilityKernel/Source/module.cpp new file mode 100644 index 00000000..89747d88 --- /dev/null +++ b/TactilityKernel/Source/module.cpp @@ -0,0 +1,120 @@ +#include +#include +#include +#include + +struct ModuleParentInternal { + std::vector modules; + struct Mutex mutex = { 0 }; +}; + +extern "C" { + +#pragma region module_parent + +error_t module_parent_construct(struct ModuleParent* parent) { + parent->internal.data = new (std::nothrow) ModuleParentInternal(); + if (!parent->internal.data) return ERROR_OUT_OF_MEMORY; + + auto* data = static_cast(parent->internal.data); + mutex_construct(&data->mutex); + + return ERROR_NONE; +} + +error_t module_parent_destruct(struct ModuleParent* parent) { + auto* data = static_cast(parent->internal.data); + if (data == nullptr) return ERROR_NONE; + + mutex_lock(&data->mutex); + if (!data->modules.empty()) { + mutex_unlock(&data->mutex); + return ERROR_INVALID_STATE; + } + mutex_unlock(&data->mutex); + + mutex_destruct(&data->mutex); + delete data; + parent->internal.data = nullptr; + return ERROR_NONE; +} + +#pragma endregion + +#pragma region module + +error_t module_set_parent(struct Module* module, struct ModuleParent* parent) { + if (module->internal.started) return ERROR_INVALID_STATE; + if (module->internal.parent == parent) return ERROR_NONE; + + // Remove from old parent + if (module->internal.parent && module->internal.parent->internal.data) { + auto* old_data = static_cast(module->internal.parent->internal.data); + mutex_lock(&old_data->mutex); + auto it = std::find(old_data->modules.begin(), old_data->modules.end(), module); + if (it != old_data->modules.end()) { + old_data->modules.erase(it); + } + mutex_unlock(&old_data->mutex); + } + + module->internal.parent = parent; + + // Add to new parent + if (parent && parent->internal.data) { + auto* new_data = static_cast(parent->internal.data); + mutex_lock(&new_data->mutex); + new_data->modules.push_back(module); + mutex_unlock(&new_data->mutex); + } + + return ERROR_NONE; +} + +error_t module_start(struct Module* module) { + if (module->internal.started) return ERROR_NONE; + if (!module->internal.parent) return ERROR_INVALID_STATE; + + error_t error = module->start(); + module->internal.started = (error == ERROR_NONE); + return error; +} + +bool module_is_started(struct Module* module) { + return module->internal.started; +} + +error_t module_stop(struct Module* module) { + if (!module->internal.started) return ERROR_NONE; + + error_t error = module->stop(); + if (error != ERROR_NONE) { + return error; + } + + module->internal.started = false; + return error; +} + +#pragma endregion + +#pragma region kernel_module_parent; + +static struct ModuleParent kernel_module_parent = { "kernel", { nullptr } }; + +error_t kernel_module_parent_construct(void) { + return module_parent_construct(&kernel_module_parent); +} + +error_t kernel_module_parent_destruct(void) { + return module_parent_destruct(&kernel_module_parent); +} + +struct ModuleParent* get_kernel_module_parent(void) { + return &kernel_module_parent; +} + +#pragma endregion + +} + diff --git a/Tests/TactilityKernel/DeviceTest.cpp b/Tests/TactilityKernel/DeviceTest.cpp index b0060d03..06acb6fa 100644 --- a/Tests/TactilityKernel/DeviceTest.cpp +++ b/Tests/TactilityKernel/DeviceTest.cpp @@ -4,6 +4,13 @@ #include #include +#include + +static Module module = { + .name = "test_module", + .start = nullptr, + .stop = nullptr +}; TEST_CASE("device_construct and device_destruct should set and unset the correct fields") { Device device = { 0 }; @@ -164,7 +171,8 @@ TEST_CASE("device_is_ready should return true only when it is started") { .stopDevice = nullptr, .api = nullptr, .deviceType = nullptr, - .internal = { 0 } + .owner = &module, + .internal = nullptr }; Device device = { 0 }; diff --git a/Tests/TactilityKernel/DriverIntegrationTest.cpp b/Tests/TactilityKernel/DriverIntegrationTest.cpp index 1a30de27..70fae766 100644 --- a/Tests/TactilityKernel/DriverIntegrationTest.cpp +++ b/Tests/TactilityKernel/DriverIntegrationTest.cpp @@ -1,6 +1,13 @@ #include "doctest.h" #include #include +#include + +static Module module = { + .name = "test_module", + .start = nullptr, + .stop = nullptr +}; struct IntegrationDriverConfig { int startResult; @@ -30,7 +37,8 @@ static Driver integration_driver = { .stopDevice = stop, .api = nullptr, .deviceType = nullptr, - .internal = { 0 } + .owner = &module, + .internal = nullptr, }; TEST_CASE("driver with with start success and stop success should start and stop a device") { diff --git a/Tests/TactilityKernel/DriverTest.cpp b/Tests/TactilityKernel/DriverTest.cpp index df326295..5661f8f1 100644 --- a/Tests/TactilityKernel/DriverTest.cpp +++ b/Tests/TactilityKernel/DriverTest.cpp @@ -1,13 +1,31 @@ #include "doctest.h" + #include +#include + +static Module module = { + .name = "test_module", + .start = nullptr, + .stop = nullptr +}; TEST_CASE("driver_construct and driver_destruct should set and unset the correct fields") { Driver driver = { 0 }; + driver.owner = &module; CHECK_EQ(driver_construct(&driver), ERROR_NONE); - CHECK_NE(driver.internal.data, nullptr); + CHECK_NE(driver.internal, nullptr); + CHECK_EQ(driver_destruct(&driver), ERROR_NONE); + CHECK_EQ(driver.internal, nullptr); +} + +TEST_CASE("a driver without a module should not be destrucable") { + Driver driver = { 0 }; + + CHECK_EQ(driver_construct(&driver), ERROR_NONE); + CHECK_EQ(driver_destruct(&driver), ERROR_NOT_ALLOWED); + driver.owner = &module; CHECK_EQ(driver_destruct(&driver), ERROR_NONE); - CHECK_EQ(driver.internal.data, nullptr); } TEST_CASE("driver_is_compatible should return true if a compatible value is found") { @@ -19,7 +37,8 @@ TEST_CASE("driver_is_compatible should return true if a compatible value is foun .stopDevice = nullptr, .api = nullptr, .deviceType = nullptr, - .internal = { 0 } + .owner = &module, + .internal = nullptr }; CHECK_EQ(driver_is_compatible(&driver, "test_compatible"), true); CHECK_EQ(driver_is_compatible(&driver, "nope"), false); @@ -35,7 +54,8 @@ TEST_CASE("driver_find should only find a compatible driver when the driver was .stopDevice = nullptr, .api = nullptr, .deviceType = nullptr, - .internal = { 0 } + .owner = &module, + .internal = nullptr }; Driver* found_driver = driver_find_compatible("test_compatible"); diff --git a/Tests/TactilityKernel/ModuleTest.cpp b/Tests/TactilityKernel/ModuleTest.cpp new file mode 100644 index 00000000..97651870 --- /dev/null +++ b/Tests/TactilityKernel/ModuleTest.cpp @@ -0,0 +1,174 @@ +#include "doctest.h" +#include + +static error_t test_start_result = ERROR_NONE; +static bool start_called = false; +static error_t test_start() { + start_called = true; + return test_start_result; +} + +static error_t test_stop_result = ERROR_NONE; +static bool stop_called = false; +static error_t test_stop() { + stop_called = true; + return test_stop_result; +} + +TEST_CASE("ModuleParent construction and destruction") { + struct ModuleParent parent = { "test_parent", { nullptr } }; + + // Test successful construction + CHECK_EQ(module_parent_construct(&parent), ERROR_NONE); + CHECK_NE(parent.internal.data, nullptr); + + // Test successful destruction + CHECK_EQ(module_parent_destruct(&parent), ERROR_NONE); + CHECK_EQ(parent.internal.data, nullptr); +} + +TEST_CASE("ModuleParent destruction with children") { + struct ModuleParent parent = { "parent", { nullptr } }; + REQUIRE_EQ(module_parent_construct(&parent), ERROR_NONE); + + struct Module module = { + .name = "test", + .start = test_start, + .stop = test_stop, + .internal = {.started = false, .parent = nullptr} + }; + + REQUIRE_EQ(module_set_parent(&module, &parent), ERROR_NONE); + + // Should fail to destruct because it has a child + CHECK_EQ(module_parent_destruct(&parent), ERROR_INVALID_STATE); + CHECK_NE(parent.internal.data, nullptr); + + // Remove child + REQUIRE_EQ(module_set_parent(&module, nullptr), ERROR_NONE); + + // Now it should succeed + CHECK_EQ(module_parent_destruct(&parent), ERROR_NONE); + CHECK_EQ(parent.internal.data, nullptr); +} + +TEST_CASE("Module parent management") { + struct ModuleParent parent1 = { "parent1", { nullptr } }; + struct ModuleParent parent2 = { "parent2", { nullptr } }; + REQUIRE_EQ(module_parent_construct(&parent1), ERROR_NONE); + REQUIRE_EQ(module_parent_construct(&parent2), ERROR_NONE); + + struct Module module = { + .name = "test", + .start = test_start, + .stop = test_stop, + .internal = {.started = false, .parent = nullptr} + }; + + // Set parent + CHECK_EQ(module_set_parent(&module, &parent1), ERROR_NONE); + CHECK_EQ(module.internal.parent, &parent1); + + // Change parent + CHECK_EQ(module_set_parent(&module, &parent2), ERROR_NONE); + CHECK_EQ(module.internal.parent, &parent2); + + // Clear parent + CHECK_EQ(module_set_parent(&module, nullptr), ERROR_NONE); + CHECK_EQ(module.internal.parent, nullptr); + + // Set same parent (should be NOOP and return ERROR_NONE) + CHECK_EQ(module_set_parent(&module, &parent1), ERROR_NONE); + CHECK_EQ(module_set_parent(&module, &parent1), ERROR_NONE); + CHECK_EQ(module.internal.parent, &parent1); + + module_parent_destruct(&parent1); + module_parent_destruct(&parent2); +} + +TEST_CASE("Module lifecycle") { + struct ModuleParent parent = { "parent", { nullptr } }; + REQUIRE_EQ(module_parent_construct(&parent), ERROR_NONE); + + start_called = false; + stop_called = false; + test_start_result = ERROR_NONE; + test_stop_result = ERROR_NONE; + + struct Module module = { + .name = "test", + .start = test_start, + .stop = test_stop, + .internal = {.started = false, .parent = nullptr} + }; + + // 1. Cannot start without parent + CHECK_EQ(module_start(&module), ERROR_INVALID_STATE); + CHECK_EQ(module_is_started(&module), false); + CHECK_EQ(start_called, false); + + CHECK_EQ(module_set_parent(&module, &parent), ERROR_NONE); + + // 2. Successful start + CHECK_EQ(module_start(&module), ERROR_NONE); + CHECK_EQ(module_is_started(&module), true); + CHECK_EQ(start_called, true); + + // 3. Start when already started (should return ERROR_NONE) + start_called = false; + CHECK_EQ(module_start(&module), ERROR_NONE); + CHECK_EQ(start_called, false); // start() function should NOT be called again + + // 4. Cannot change parent while started + CHECK_EQ(module_set_parent(&module, nullptr), ERROR_INVALID_STATE); + + // 5. Successful stop + CHECK_EQ(module_stop(&module), ERROR_NONE); + CHECK_EQ(module_is_started(&module), false); + CHECK_EQ(stop_called, true); + + // 6. Stop when already stopped (should return ERROR_NONE) + stop_called = false; + CHECK_EQ(module_stop(&module), ERROR_NONE); + CHECK_EQ(stop_called, false); // stop() function should NOT be called again + + // 7. Test failed start + test_start_result = ERROR_NOT_FOUND; + start_called = false; + CHECK_EQ(module_start(&module), ERROR_NOT_FOUND); + CHECK_EQ(module_is_started(&module), false); + CHECK_EQ(start_called, true); + + // 8. Test failed stop + CHECK_EQ(module_set_parent(&module, &parent), ERROR_NONE); + test_start_result = ERROR_NONE; + CHECK_EQ(module_start(&module), ERROR_NONE); + + test_stop_result = ERROR_NOT_SUPPORTED; + stop_called = false; + CHECK_EQ(module_stop(&module), ERROR_NOT_SUPPORTED); + CHECK_EQ(module_is_started(&module), true); // Should still be started if stop failed + CHECK_EQ(stop_called, true); + + // Clean up: fix stop result so we can stop it + test_stop_result = ERROR_NONE; + CHECK_EQ(module_stop(&module), ERROR_NONE); + + module_parent_destruct(&parent); +} + +TEST_CASE("Kernel module parent") { + // Ensure it's constructed + REQUIRE_EQ(kernel_module_parent_construct(), ERROR_NONE); + + struct ModuleParent* kernel_parent = get_kernel_module_parent(); + CHECK_NE(kernel_parent, nullptr); + CHECK_NE(kernel_parent->internal.data, nullptr); + + // Multiple calls return the same + CHECK_EQ(get_kernel_module_parent(), kernel_parent); + + // Test destruction + CHECK_EQ(kernel_module_parent_destruct(), ERROR_NONE); + CHECK_EQ(kernel_parent->internal.data, nullptr); +}