From 46d3e9cb0f14701ab1d8543ff24b79e6195ecaae Mon Sep 17 00:00:00 2001 From: Tomasz Bursztyka Date: Fri, 8 Apr 2022 13:04:33 +0200 Subject: [PATCH 1/4] device: Create a dedicated runtime context Rename existing struct device_state to struct device_context as this structure will hold more than just state stuff. Remove struct device's state attribute, saving some ROM space. Instead order the device_contexts sections the same way as the devices one so a device context can be easily retrieved from it's device pointer (same "index" in the list of device context). Signed-off-by: Tomasz Bursztyka --- include/zephyr/device.h | 54 ++++++++++++----------------- include/zephyr/linker/common-ram.ld | 15 ++++---- kernel/device.c | 17 +++++++-- 3 files changed, 45 insertions(+), 41 deletions(-) diff --git a/include/zephyr/device.h b/include/zephyr/device.h index 7820516af591..f53d5b5e33f5 100644 --- a/include/zephyr/device.h +++ b/include/zephyr/device.h @@ -120,20 +120,10 @@ typedef int16_t device_handle_t; */ #define Z_DEVICE_DT_DEV_NAME(node_id) _CONCAT(dts_ord_, DT_DEP_ORD(node_id)) -/* Synthesize a unique name for the device state associated with +/* Synthesize a unique name for the device runtime context associated with * dev_name. */ -#define Z_DEVICE_STATE_NAME(dev_name) _CONCAT(__devstate_, dev_name) - -/** - * @brief Utility macro to define and initialize the device state. - * - * @param node_id Devicetree node id of the device. - * @param dev_name Device name. - */ -#define Z_DEVICE_STATE_DEFINE(node_id, dev_name) \ - static struct device_state Z_DEVICE_STATE_NAME(dev_name) \ - __attribute__((__section__(".z_devstate"))); +#define Z_DEVICE_CONTEXT_NAME(dev_name) _CONCAT(__devctx_, dev_name) /** * @def DEVICE_DEFINE @@ -180,11 +170,10 @@ typedef int16_t device_handle_t; */ #define DEVICE_DEFINE(dev_name, drv_name, init_fn, pm_device, \ data_ptr, cfg_ptr, level, prio, api_ptr) \ - Z_DEVICE_STATE_DEFINE(DT_INVALID_NODE, dev_name) \ + Z_DEVICE_CONTEXT_DEFINE(DT_INVALID_NODE, dev_name, level, prio) \ Z_DEVICE_DEFINE(DT_INVALID_NODE, dev_name, drv_name, init_fn, \ - pm_device, \ - data_ptr, cfg_ptr, level, prio, api_ptr, \ - &Z_DEVICE_STATE_NAME(dev_name)) + pm_device, data_ptr, cfg_ptr, level, prio, \ + api_ptr) /** * @def DEVICE_DT_NAME @@ -248,14 +237,12 @@ typedef int16_t device_handle_t; #define DEVICE_DT_DEFINE(node_id, init_fn, pm_device, \ data_ptr, cfg_ptr, level, prio, \ api_ptr, ...) \ - Z_DEVICE_STATE_DEFINE(node_id, Z_DEVICE_DT_DEV_NAME(node_id)) \ + Z_DEVICE_CONTEXT_DEFINE(node_id, Z_DEVICE_DT_DEV_NAME(node_id), \ + level, prio) \ Z_DEVICE_DEFINE(node_id, Z_DEVICE_DT_DEV_NAME(node_id), \ DEVICE_DT_NAME(node_id), init_fn, \ - pm_device, \ - data_ptr, cfg_ptr, level, prio, \ - api_ptr, \ - &Z_DEVICE_STATE_NAME(Z_DEVICE_DT_DEV_NAME(node_id)), \ - __VA_ARGS__) + pm_device, data_ptr, cfg_ptr, level, prio, \ + api_ptr, __VA_ARGS__) /** * @def DEVICE_DT_INST_DEFINE @@ -419,14 +406,14 @@ typedef int16_t device_handle_t; #define DEVICE_DECLARE(name) static const struct device DEVICE_NAME_GET(name) /** - * @brief Runtime device dynamic structure (in RAM) per driver instance + * @brief Runtime device context (in RAM) per driver instance * * Fields in this are expected to be default-initialized to zero. The * kernel driver infrastructure and driver access functions are * responsible for ensuring that any non-zero initialization is done * before they are accessed. */ -struct device_state { +struct device_context { /** Non-negative result of initializing the device. * * The absolute value returned when the device initialization @@ -460,8 +447,6 @@ struct device { const void *config; /** Address of the API structure exposed by the device instance */ const void *api; - /** Address of the common device state */ - struct device_state * const state; /** Address of the device instance private data */ void * const data; /** optional pointer to handles associated with the device. @@ -829,14 +814,20 @@ __deprecated static inline int device_usable_check(const struct device *dev) FOR_EACH_NONEMPTY_TERM(IDENTITY, (,), __VA_ARGS__) /* - * Utility macro to define and initialize the device state. + * Utility macro to define and initialize the device runtime context. * * @param node_id Devicetree node id of the device. * @param dev_name Device name. + * @param level The device's initialization level. See SYS_INIT() for + * details. + * @param prio The device's priority within its initialization level. + * See SYS_INIT() for details. */ -#define Z_DEVICE_STATE_DEFINE(node_id, dev_name) \ - static struct device_state Z_DEVICE_STATE_NAME(dev_name) \ - __attribute__((__section__(".z_devstate"))); +#define Z_DEVICE_CONTEXT_DEFINE(node_id, dev_name, level, prio) \ + static Z_DECL_ALIGN(struct device_context) \ + Z_DEVICE_CONTEXT_NAME(dev_name) __used \ + __attribute__( \ + (__section__(".z_devcontext_" #level STRINGIFY(prio)"_"))); /* Construct objects that are referenced from struct device. These * include power management and dependency handles. @@ -911,7 +902,7 @@ BUILD_ASSERT(sizeof(device_handle_t) == 2, "fix the linker scripts"); * dependency handles that come from outside devicetree. */ #define Z_DEVICE_DEFINE(node_id, dev_name, drv_name, init_fn, pm_device,\ - data_ptr, cfg_ptr, level, prio, api_ptr, state_ptr, ...) \ + data_ptr, cfg_ptr, level, prio, api_ptr, ...) \ Z_DEVICE_DEFINE_PRE(node_id, dev_name, __VA_ARGS__) \ COND_CODE_1(DT_NODE_EXISTS(node_id), (), (static)) \ const Z_DECL_ALIGN(struct device) \ @@ -920,7 +911,6 @@ BUILD_ASSERT(sizeof(device_handle_t) == 2, "fix the linker scripts"); .name = drv_name, \ .config = (cfg_ptr), \ .api = (api_ptr), \ - .state = (state_ptr), \ .data = (data_ptr), \ COND_CODE_1(CONFIG_PM_DEVICE, (.pm = pm_device,), ()) \ Z_DEVICE_DEFINE_INIT(node_id, dev_name) \ diff --git a/include/zephyr/linker/common-ram.ld b/include/zephyr/linker/common-ram.ld index f36c7d8021c4..fb6d99efeeb8 100644 --- a/include/zephyr/linker/common-ram.ld +++ b/include/zephyr/linker/common-ram.ld @@ -27,13 +27,16 @@ } GROUP_DATA_LINK_IN(RAMABLE_REGION, ROMABLE_REGION) #endif - SECTION_DATA_PROLOGUE(device_states,,) + SECTION_DATA_PROLOGUE(device_contexts,,) { - /* Device states used by the device objects. */ - __device_states_start = .; - KEEP(*(".z_devstate")); - KEEP(*(".z_devstate.*")); - __device_states_end = .; + /* Device runtime context used by the device objects. */ + __device_context_start = .; + CREATE_OBJ_LEVEL(devcontext, PRE_KERNEL_1) + CREATE_OBJ_LEVEL(devcontext, PRE_KERNEL_2) + CREATE_OBJ_LEVEL(devcontext, POST_KERNEL) + CREATE_OBJ_LEVEL(devcontext, APPLICATION) + CREATE_OBJ_LEVEL(devcontext, SMP) + __device_context_end = .; } GROUP_DATA_LINK_IN(RAMABLE_REGION, ROMABLE_REGION) #if CONFIG_PM_DEVICE diff --git a/kernel/device.c b/kernel/device.c index df273209bb01..e567b983a1c5 100644 --- a/kernel/device.c +++ b/kernel/device.c @@ -24,6 +24,11 @@ extern const struct device __device_start[]; extern const struct device __device_end[]; +extern struct device_context __device_context_start[]; + +#define GET_DEV_CONTEXT(_dev) \ + &__device_context_start[_dev - __device_start]; + /** * @brief Initialize state for all static devices. * @@ -71,6 +76,8 @@ void z_sys_init_run_level(int32_t level) int rc = entry->init(dev); if (dev != NULL) { + struct device_context *dc = GET_DEV_CONTEXT(dev); + /* Mark device initialized. If initialization * failed, record the error condition. */ @@ -81,9 +88,9 @@ void z_sys_init_run_level(int32_t level) if (rc > UINT8_MAX) { rc = UINT8_MAX; } - dev->state->init_res = rc; + dc->init_res = rc; } - dev->state->initialized = true; + dc->initialized = true; } } } @@ -150,6 +157,8 @@ size_t z_device_get_all_static(struct device const **devices) bool z_device_is_ready(const struct device *dev) { + struct device_context *dc; + /* * if an invalid device pointer is passed as argument, this call * reports the `device` as not ready for usage. @@ -158,7 +167,9 @@ bool z_device_is_ready(const struct device *dev) return false; } - return dev->state->initialized && (dev->state->init_res == 0U); + dc = GET_DEV_CONTEXT(dev); + + return dc->initialized && (dc->init_res == 0U); } static int device_visitor(const device_handle_t *handles, From 48f7bc8b2681aad95b0e9377750a6f4dbef5f22f Mon Sep 17 00:00:00 2001 From: Tomasz Bursztyka Date: Thu, 28 May 2020 14:33:07 +0200 Subject: [PATCH 2/4] device: Introduce device lock for concurrent access mitigation This introduce a generic device concurrent access protection. Such feature already exist, scattered either through some driver domain API - SPI for instance - or most of the time per-driver when the maintainer has a good understanding on what to do. But not everywhere. Many drivers are not protected against concurrent access. Generalizing this will remove the burden for driver maintainers to do that on their own, but also will add it to all the drivers which are lacking this. Signed-off-by: Tomasz Bursztyka --- include/zephyr/device.h | 75 ++++++++++++++++++++++++++++++++++------- kernel/Kconfig | 2 ++ kernel/Kconfig.device | 16 +++++++++ kernel/device.c | 20 +++++++++++ 4 files changed, 101 insertions(+), 12 deletions(-) create mode 100644 kernel/Kconfig.device diff --git a/include/zephyr/device.h b/include/zephyr/device.h index f53d5b5e33f5..6943b858f23d 100644 --- a/include/zephyr/device.h +++ b/include/zephyr/device.h @@ -427,6 +427,10 @@ struct device_context { * invoked. */ bool initialized : 1; + +#if defined(CONFIG_DEVICE_CONCURRENT_ACCESS) + struct k_sem lock; +#endif }; struct pm_device; @@ -796,22 +800,55 @@ __deprecated static inline int device_usable_check(const struct device *dev) return device_is_ready(dev) ? 0 : -ENODEV; } +#if defined(CONFIG_DEVICE_CONCURRENT_ACCESS) + /** - * @} + * @brief Lock an interrupt based device structure to avoid concurrent access. + * + * Note: Needs to be the first function called when entering a device API call. + * + * @param dev A valid pointer on a struct device instance + * + * @return 0 if device got locked, a negative errno otherwise. */ +int device_lock(struct device *dev); -/* Synthesize the name of the object that holds device ordinal and - * dependency data. If the object doesn't come from a devicetree - * node, use dev_name. +/** + * @brief Release a previously locked device + * + * Note: Needs to be the last funtion called before returning a device API call. + * + * @param dev A valid pointer on a struct device instance. + * @param status The status to return + * + * @return status of the device */ -#define Z_DEVICE_HANDLE_NAME(node_id, dev_name) \ - _CONCAT(__devicehdl_, \ - COND_CODE_1(DT_NODE_EXISTS(node_id), \ - (node_id), \ - (dev_name))) +int device_release(struct device *dev, int status); -#define Z_DEVICE_EXTRA_HANDLES(...) \ - FOR_EACH_NONEMPTY_TERM(IDENTITY, (,), __VA_ARGS__) +#else /* CONFIG_DEVICE_CONCURRENT_ACCESS */ + +#define device_lock(...) (0U) +#define device_release(_dev, _status) (_status) + +#endif /* CONFIG_DEVICE_CONCURRENT_ACCESS */ + +/** + * @} + */ + +#if defined(CONFIG_DEVICE_CONCURRENT_ACCESS) + +/* Initialize structure device_context's specific attributes */ +#define Z_DEVICE_CONTEXT_INITIALIZE(_obj) \ + { \ + .lock = Z_SEM_INITIALIZER(_obj.lock, 1, 1), \ + } + +#else /* CONFIG_DEVICE_CONCURRENT_ACCESS */ + +#define Z_DEVICE_CONTEXT_INITIALIZE(_obj) {} + +#endif /* CONFIG_DEVICE_CONCURRENT_ACCESS */ /* * Utility macro to define and initialize the device runtime context. @@ -827,7 +864,21 @@ __deprecated static inline int device_usable_check(const struct device *dev) static Z_DECL_ALIGN(struct device_context) \ Z_DEVICE_CONTEXT_NAME(dev_name) __used \ __attribute__( \ - (__section__(".z_devcontext_" #level STRINGIFY(prio)"_"))); + (__section__(".z_devcontext_" #level STRINGIFY(prio)"_"))) = \ + Z_DEVICE_CONTEXT_INITIALIZE(Z_DEVICE_CONTEXT_NAME(dev_name)); + +/* Synthesize the name of the object that holds device ordinal and + * dependency data. If the object doesn't come from a devicetree + * node, use dev_name. + */ +#define Z_DEVICE_HANDLE_NAME(node_id, dev_name) \ + _CONCAT(__devicehdl_, \ + COND_CODE_1(DT_NODE_EXISTS(node_id), \ + (node_id), \ + (dev_name))) + +#define Z_DEVICE_EXTRA_HANDLES(...) \ + FOR_EACH_NONEMPTY_TERM(IDENTITY, (,), __VA_ARGS__) /* Construct objects that are referenced from struct device. These * include power management and dependency handles. diff --git a/kernel/Kconfig b/kernel/Kconfig index 267b0adb6cee..5f235456e114 100644 --- a/kernel/Kconfig +++ b/kernel/Kconfig @@ -947,3 +947,5 @@ config HAS_DYNAMIC_DEVICE_HANDLES endmenu rsource "Kconfig.vm" + +rsource "Kconfig.device" diff --git a/kernel/Kconfig.device b/kernel/Kconfig.device new file mode 100644 index 000000000000..acba6ac8474c --- /dev/null +++ b/kernel/Kconfig.device @@ -0,0 +1,16 @@ +# Device configuration options + +# Copyright (c) 2022 Intel Corporation +# SPDX-License-Identifier: Apache-2.0 + +menu "Device driver generic options" + +config DEVICE_CONCURRENT_ACCESS + bool "Concurrent access protection" + default y + help + If disabled, devices will not be protected over concurrent access. + This can be a valid possibility if you know your devices will not be + accessed by 2+ threads, and thus will reduce the memory footprint. + +endmenu diff --git a/kernel/device.c b/kernel/device.c index e567b983a1c5..216d99660e42 100644 --- a/kernel/device.c +++ b/kernel/device.c @@ -210,3 +210,23 @@ int device_supported_foreach(const struct device *dev, return device_visitor(handles, handle_count, visitor_cb, context); } + +#if defined(CONFIG_DEVICE_CONCURRENT_ACCESS) + +int device_lock(struct device *dev) +{ + struct device_context *dc = GET_DEV_CONTEXT(dev); + + return k_sem_take(&dc->lock, K_FOREVER); +} + +int device_release(struct device *dev, int status) +{ + struct device_context *dc = GET_DEV_CONTEXT(dev); + + k_sem_give(&dc->lock); + + return status; +} + +#endif /* CONFIG_DEVICE_CONCURRENT_ACCESS */ From 529a8488f87534fcd7206a3dce83999fe8842fcf Mon Sep 17 00:00:00 2001 From: Tomasz Bursztyka Date: Wed, 4 May 2022 13:23:30 +0200 Subject: [PATCH 3/4] device: Make the concurrent access mechanism DTS based Make the lock as a pointer, and optional via a DTS property. Currently, all devices get the lock or not depending on CONFIG_DEVICE_CONCURRENT_ACCESS being set or not. That's too heavy as some devices may never need such lock. Thus making it optional via a DTS property. By default, all devices will get the lock. The instances that do not want it will only have to set zephyry,no-lock = false in the relevant DTS file. The obvious drawback is that it only works for DTS based device instances. Non DTS based instance are not getting any lock anyway then. Signed-off-by: Tomasz Bursztyka --- dts/bindings/base/base.yaml | 2 +- dts/bindings/base/zephyr,device.yaml | 9 ++++++++ include/zephyr/device.h | 33 ++++++++++++++++++++++------ include/zephyr/linker/common-ram.ld | 9 ++++++++ kernel/device.c | 10 +++++++-- 5 files changed, 53 insertions(+), 10 deletions(-) create mode 100644 dts/bindings/base/zephyr,device.yaml diff --git a/dts/bindings/base/base.yaml b/dts/bindings/base/base.yaml index f99e83642ee6..8f86d2739605 100644 --- a/dts/bindings/base/base.yaml +++ b/dts/bindings/base/base.yaml @@ -1,6 +1,6 @@ # Common fields for all devices -include: [pm.yaml] +include: [pm.yaml, "zephyr,device.yaml"] properties: status: diff --git a/dts/bindings/base/zephyr,device.yaml b/dts/bindings/base/zephyr,device.yaml new file mode 100644 index 000000000000..75ebd26340ff --- /dev/null +++ b/dts/bindings/base/zephyr,device.yaml @@ -0,0 +1,9 @@ +# Common fields for zephyr devices + +properties: + zephyr,no-lock: + type: boolean + required: false + description: | + Unset by default, it enables the possibility to avoid allocating a lock + on the device instance. diff --git a/include/zephyr/device.h b/include/zephyr/device.h index 6943b858f23d..74c05699628a 100644 --- a/include/zephyr/device.h +++ b/include/zephyr/device.h @@ -170,7 +170,7 @@ typedef int16_t device_handle_t; */ #define DEVICE_DEFINE(dev_name, drv_name, init_fn, pm_device, \ data_ptr, cfg_ptr, level, prio, api_ptr) \ - Z_DEVICE_CONTEXT_DEFINE(DT_INVALID_NODE, dev_name, level, prio) \ + Z_DEVICE_CONTEXT_DEFINE(DT_INVALID_NODE, dev_name, level, prio) \ Z_DEVICE_DEFINE(DT_INVALID_NODE, dev_name, drv_name, init_fn, \ pm_device, data_ptr, cfg_ptr, level, prio, \ api_ptr) @@ -429,7 +429,7 @@ struct device_context { bool initialized : 1; #if defined(CONFIG_DEVICE_CONCURRENT_ACCESS) - struct k_sem lock; + struct k_sem *lock; #endif }; @@ -838,15 +838,33 @@ int device_release(struct device *dev, int status); #if defined(CONFIG_DEVICE_CONCURRENT_ACCESS) +/* Get device context's lock name */ +#define Z_DEVICE_CONTEXT_LOCK_NAME(dev_name) \ + _CONCAT(Z_DEVICE_CONTEXT_NAME(dev_name), _lock) + +/* Declare and initialize a lock */ +#define Z_DEVICE_CONTEXT_LOCK_DECLARE(dev_name) \ + static struct k_sem Z_DEVICE_CONTEXT_LOCK_NAME(dev_name) __used \ + __attribute__((__section__(".z_devcontext_lock"))) = \ + Z_SEM_INITIALIZER(Z_DEVICE_CONTEXT_LOCK_NAME(dev_name), 1, 1); + +/* Conditionally create a lock for the device context */ +#define Z_DEVICE_CONTEXT_LOCK_INITIALIZE(node_id, dev_name) \ + COND_CODE_0(DT_PROP(node_id, zephyr_no_lock), \ + (Z_DEVICE_CONTEXT_LOCK_DECLARE(dev_name)), ()) + /* Initialize structure device_context's specific attributes */ -#define Z_DEVICE_CONTEXT_INITIALIZE(_obj) \ - { \ - .lock = Z_SEM_INITIALIZER(_obj.lock, 1, 1), \ +#define Z_DEVICE_CONTEXT_INITIALIZE(node_id, dev_name) \ + { \ + COND_CODE_0(DT_PROP(node_id, zephyr_no_lock), \ + (.lock = &Z_DEVICE_CONTEXT_LOCK_NAME(dev_name)), \ + (.lock = NULL)) \ } #else /* CONFIG_DEVICE_CONCURRENT_ACCESS */ -#define Z_DEVICE_CONTEXT_INITIALIZE(_obj) {} +#define Z_DEVICE_CONTEXT_LOCK_INITIALIZE(node_id, dev_name) +#define Z_DEVICE_CONTEXT_INITIALIZE(node_id, dev_name) {} #endif /* CONFIG_DEVICE_CONCURRENT_ACCESS */ @@ -861,11 +879,12 @@ int device_release(struct device *dev, int status); * See SYS_INIT() for details. */ #define Z_DEVICE_CONTEXT_DEFINE(node_id, dev_name, level, prio) \ + Z_DEVICE_CONTEXT_LOCK_INITIALIZE(node_id, dev_name) \ static Z_DECL_ALIGN(struct device_context) \ Z_DEVICE_CONTEXT_NAME(dev_name) __used \ __attribute__( \ (__section__(".z_devcontext_" #level STRINGIFY(prio)"_"))) = \ - Z_DEVICE_CONTEXT_INITIALIZE(Z_DEVICE_CONTEXT_NAME(dev_name)); + Z_DEVICE_CONTEXT_INITIALIZE(node_id, dev_name); /* Synthesize the name of the object that holds device ordinal and * dependency data. If the object doesn't come from a devicetree diff --git a/include/zephyr/linker/common-ram.ld b/include/zephyr/linker/common-ram.ld index fb6d99efeeb8..83e5911b4c81 100644 --- a/include/zephyr/linker/common-ram.ld +++ b/include/zephyr/linker/common-ram.ld @@ -27,6 +27,15 @@ } GROUP_DATA_LINK_IN(RAMABLE_REGION, ROMABLE_REGION) #endif +#if defined(CONFIG_DEVICE_CONCURRENT_ACCESS) + SECTION_DATA_PROLOGUE(devcontext_locks,,) + { + __devcontext_locks_start = .; + KEEP(*(".z_devcontext_lock")); + __devcontext_locks_end = .; + } GROUP_DATA_LINK_IN(RAMABLE_REGION, ROMABLE_REGION) +#endif + SECTION_DATA_PROLOGUE(device_contexts,,) { /* Device runtime context used by the device objects. */ diff --git a/kernel/device.c b/kernel/device.c index 216d99660e42..36d4707b7525 100644 --- a/kernel/device.c +++ b/kernel/device.c @@ -217,14 +217,20 @@ int device_lock(struct device *dev) { struct device_context *dc = GET_DEV_CONTEXT(dev); - return k_sem_take(&dc->lock, K_FOREVER); + if (dc->lock == NULL) { + return 0; + } + + return k_sem_take(dc->lock, K_FOREVER); } int device_release(struct device *dev, int status) { struct device_context *dc = GET_DEV_CONTEXT(dev); - k_sem_give(&dc->lock); + if (dc->lock != NULL) { + k_sem_give(dc->lock); + } return status; } From bdaaf3d0466cb273d2820228aeed3ce0aac93baf Mon Sep 17 00:00:00 2001 From: Tomasz Bursztyka Date: Mon, 30 Mar 2020 08:18:13 +0200 Subject: [PATCH 4/4] device: Introduce a generic context synchronization mechanism This adds a synchronization semaphore to generalize call synchronization among all device drivers. This will permit to avoid for drivers maintainers to implement their own solution. Signed-off-by: Tomasz Bursztyka --- dts/bindings/base/zephyr,device.yaml | 8 +++ include/zephyr/device.h | 91 ++++++++++++++++++++++++---- include/zephyr/linker/common-ram.ld | 9 +++ kernel/Kconfig.device | 8 +++ kernel/device.c | 32 +++++++++- 5 files changed, 134 insertions(+), 14 deletions(-) diff --git a/dts/bindings/base/zephyr,device.yaml b/dts/bindings/base/zephyr,device.yaml index 75ebd26340ff..42c270e5dcb5 100644 --- a/dts/bindings/base/zephyr,device.yaml +++ b/dts/bindings/base/zephyr,device.yaml @@ -7,3 +7,11 @@ properties: description: | Unset by default, it enables the possibility to avoid allocating a lock on the device instance. + + zephyr,no-sync: + type: boolean + required: false + description: | + Unset by default, it enables the possibility to avoid allocating + a synchronization object on the device instance. This can be useful if + the device driver works on polling mode only. diff --git a/include/zephyr/device.h b/include/zephyr/device.h index 74c05699628a..ffbfbc0df798 100644 --- a/include/zephyr/device.h +++ b/include/zephyr/device.h @@ -431,6 +431,10 @@ struct device_context { #if defined(CONFIG_DEVICE_CONCURRENT_ACCESS) struct k_sem *lock; #endif +#if defined(CONFIG_DEVICE_SYNCHRONIZED_CALL) + int call_status; + struct k_sem *sync; +#endif }; struct pm_device; @@ -813,6 +817,14 @@ __deprecated static inline int device_usable_check(const struct device *dev) */ int device_lock(struct device *dev); +#else + +#define device_lock(...) (0U) + +#endif /* CONFIG_DEVICE_CONCURRENT_ACCESS */ + +#if defined(CONFIG_DEVICE_CONCURRENT_ACCESS) || \ + defined(CONFIG_DEVICE_SYNCHRONIZED_CALL) /** * @brief Release a previously locked device * @@ -825,12 +837,28 @@ int device_lock(struct device *dev); */ int device_release(struct device *dev, int status); -#else /* CONFIG_DEVICE_CONCURRENT_ACCESS */ +#else /* CONFIG_DEVICE_CONCURRENT_ACCESS || CONFIG_DEVICE_SYNCHRONIZED_CALL */ -#define device_lock(...) (0U) #define device_release(_dev, _status) (_status) -#endif /* CONFIG_DEVICE_CONCURRENT_ACCESS */ +#endif /* CONFIG_DEVICE_CONCURRENT_ACCESS || CONFIG_DEVICE_SYNCHRONIZED_CALL */ + +#if defined(CONFIG_DEVICE_SYNCHRONIZED_CALL) + +/** + * @brief Notify when a call is finished + * + * @param dev A valid pointer on a struct device instance + * @param status The status of the device, 0 on success or a negative errno + * otherwise. This function is ISR ready. + */ +void device_call_complete(struct device *dev, int status); + +#else /* CONFIG_DEVICE_SYNCHRONIZED_CALL */ + +#define device_call_complete(_dev, _status) + +#endif /* CONFIG_DEVICE_SYNCHRONIZED_CALL */ /** * @} @@ -853,21 +881,61 @@ int device_release(struct device *dev, int status); COND_CODE_0(DT_PROP(node_id, zephyr_no_lock), \ (Z_DEVICE_CONTEXT_LOCK_DECLARE(dev_name)), ()) -/* Initialize structure device_context's specific attributes */ -#define Z_DEVICE_CONTEXT_INITIALIZE(node_id, dev_name) \ - { \ - COND_CODE_0(DT_PROP(node_id, zephyr_no_lock), \ - (.lock = &Z_DEVICE_CONTEXT_LOCK_NAME(dev_name)), \ - (.lock = NULL)) \ - } +/* Initialize structure device_context's lock attribute */ +#define Z_DEVICE_CONTEXT_INITIALIZE_WITH_LOCK(node_id, dev_name) \ + COND_CODE_0(DT_PROP(node_id, zephyr_no_lock), \ + (.lock = &Z_DEVICE_CONTEXT_LOCK_NAME(dev_name),), \ + (.lock = NULL,)) #else /* CONFIG_DEVICE_CONCURRENT_ACCESS */ #define Z_DEVICE_CONTEXT_LOCK_INITIALIZE(node_id, dev_name) -#define Z_DEVICE_CONTEXT_INITIALIZE(node_id, dev_name) {} +#define Z_DEVICE_CONTEXT_INITIALIZE_WITH_LOCK(node_id, dev_name) #endif /* CONFIG_DEVICE_CONCURRENT_ACCESS */ +#if defined(CONFIG_DEVICE_SYNCHRONIZED_CALL) + +/* Get device context's sync name */ +#define Z_DEVICE_CONTEXT_SYNC_NAME(dev_name) \ + _CONCAT(Z_DEVICE_CONTEXT_NAME(dev_name), _sync) + +/* Declare and initialize a sync */ +#define Z_DEVICE_CONTEXT_SYNC_DECLARE(dev_name) \ + static struct k_sem Z_DEVICE_CONTEXT_SYNC_NAME(dev_name) __used \ + __attribute__((__section__(".z_devcontext_sync"))) = \ + Z_SEM_INITIALIZER(Z_DEVICE_CONTEXT_SYNC_NAME(dev_name), 0, 1); + +/* Conditionally create a sync for the device context */ +#define Z_DEVICE_CONTEXT_SYNC_INITIALIZE(node_id, dev_name) \ + COND_CODE_0(DT_PROP(node_id, zephyr_no_sync), \ + (Z_DEVICE_CONTEXT_SYNC_DECLARE(dev_name)), ()) + +/* Initialize structure device_context's sync attribute */ +#define Z_DEVICE_CONTEXT_INITIALIZE_WITH_SYNC(node_id, dev_name) \ + COND_CODE_0(DT_PROP(node_id, zephyr_no_sync), \ + (.sync = &Z_DEVICE_CONTEXT_SYNC_NAME(dev_name),), \ + (.sync = NULL,)) + +#else /* CONFIG_DEVICE_SYNCHRONIZED_CALL */ + +#define Z_DEVICE_CONTEXT_SYNC_INITIALIZE(node_id, dev_name) +#define Z_DEVICE_CONTEXT_INITIALIZE_WITH_SYNC(node_id, dev_name) + +#endif /* CONFIG_DEVICE_SYNCHRONIZED_CALL */ + +#define Z_DEVICE_CONTEXT_INITIALIZE(node_id, dev_name) \ + { \ + COND_CODE_1(CONFIG_DEVICE_CONCURRENT_ACCESS, \ + (Z_DEVICE_CONTEXT_INITIALIZE_WITH_LOCK( \ + node_id, dev_name)), \ + ()) \ + COND_CODE_1(CONFIG_DEVICE_SYNCHRONIZED_CALL, \ + (Z_DEVICE_CONTEXT_INITIALIZE_WITH_SYNC( \ + node_id, dev_name)), \ + ()) \ + } + /* * Utility macro to define and initialize the device runtime context. * @@ -880,6 +948,7 @@ int device_release(struct device *dev, int status); */ #define Z_DEVICE_CONTEXT_DEFINE(node_id, dev_name, level, prio) \ Z_DEVICE_CONTEXT_LOCK_INITIALIZE(node_id, dev_name) \ + Z_DEVICE_CONTEXT_SYNC_INITIALIZE(node_id, dev_name) \ static Z_DECL_ALIGN(struct device_context) \ Z_DEVICE_CONTEXT_NAME(dev_name) __used \ __attribute__( \ diff --git a/include/zephyr/linker/common-ram.ld b/include/zephyr/linker/common-ram.ld index 83e5911b4c81..9516b38d434f 100644 --- a/include/zephyr/linker/common-ram.ld +++ b/include/zephyr/linker/common-ram.ld @@ -36,6 +36,15 @@ } GROUP_DATA_LINK_IN(RAMABLE_REGION, ROMABLE_REGION) #endif +#if defined(CONFIG_DEVICE_SYNCHRONIZED_CALL) + SECTION_DATA_PROLOGUE(devcontext_syncs,,) + { + __devcontext_syncs_start = .; + KEEP(*(".z_devcontext_sync")); + __devcontext_syncs_end = .; + } GROUP_DATA_LINK_IN(RAMABLE_REGION, ROMABLE_REGION) +#endif + SECTION_DATA_PROLOGUE(device_contexts,,) { /* Device runtime context used by the device objects. */ diff --git a/kernel/Kconfig.device b/kernel/Kconfig.device index acba6ac8474c..c808aac9222f 100644 --- a/kernel/Kconfig.device +++ b/kernel/Kconfig.device @@ -13,4 +13,12 @@ config DEVICE_CONCURRENT_ACCESS This can be a valid possibility if you know your devices will not be accessed by 2+ threads, and thus will reduce the memory footprint. +config DEVICE_SYNCHRONIZED_CALL + bool "Add synchronous call generic capability" + default y + help + If disabled, device drivers will need to implement their own + synchronization mechanism when running an interrupt based API call. + Using this generic capability will avoid doing so. + endmenu diff --git a/kernel/device.c b/kernel/device.c index 36d4707b7525..82f6c834bb6f 100644 --- a/kernel/device.c +++ b/kernel/device.c @@ -211,8 +211,20 @@ int device_supported_foreach(const struct device *dev, return device_visitor(handles, handle_count, visitor_cb, context); } -#if defined(CONFIG_DEVICE_CONCURRENT_ACCESS) +#if defined(CONFIG_DEVICE_SYNCHRONIZED_CALL) +void device_call_complete(struct device *dev, int status) +{ + struct device_context *dc = GET_DEV_CONTEXT(dev) + + if (dc->sync != NULL) { + dc->call_status = status; + k_sem_give(dc->sync); + } +} +#endif /* CONFIG_DEVICE_SYNCHRONIZED_CALL */ + +#if defined(CONFIG_DEVICE_CONCURRENT_ACCESS) int device_lock(struct device *dev) { struct device_context *dc = GET_DEV_CONTEXT(dev); @@ -223,16 +235,30 @@ int device_lock(struct device *dev) return k_sem_take(dc->lock, K_FOREVER); } +#endif /* CONFIG_DEVICE_CONCURRENT_ACCESS */ +#if defined(CONFIG_DEVICE_CONCURRENT_ACCESS) || \ + defined(CONFIG_DEVICE_SYNCHRONIZED_CALL) int device_release(struct device *dev, int status) { struct device_context *dc = GET_DEV_CONTEXT(dev); +#if defined(CONFIG_DEVICE_SYNCHRONIZED_CALL) + /* If status is an error, the interrupt based + * call has not been started + */ + if (status == 0 && dc->sync != NULL) { + k_sem_take(dc->sync, K_FOREVER); + status = dc->call_status; + } +#endif /* CONFIG_DEVICE_SYNCHRONIZED_CALL */ + +#if defined(CONFIG_DEVICE_CONCURRENT_ACCESS) if (dc->lock != NULL) { k_sem_give(dc->lock); } +#endif /* CONFIG_DEVICE_CONCURRENT_ACCESS */ return status; } - -#endif /* CONFIG_DEVICE_CONCURRENT_ACCESS */ +#endif /* CONFIG_DEVICE_CONCURRENT_ACCESS || CONFIG_DEVICE_SYNCHRONIZED_CALL */