Skip to content

[WIP][RFC]Device control features #44693

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 4 commits into from
Closed

[WIP][RFC]Device control features #44693

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
46d3e9c
device: Create a dedicated runtime context
Apr 8, 2022
48f7bc8
device: Introduce device lock for concurrent access mitigation
May 28, 2020
529a848
device: Make the concurrent access mechanism DTS based
May 4, 2022
bdaaf3d
device: Introduce a generic context synchronization mechanism
Mar 30, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 22 additions & 32 deletions include/zephyr/device.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Comment on lines -429 to +416
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my opinion this should be dropped. I think the existing name is OK and I think there should be a high bar to changing the core device model APIs.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

device_state is semantically quite off once you add more features in it. Plus, the change is currently minimal, there is only the statistics in i²c and can that directly use it. It's unrelated to pm_device_state

/** Non-negative result of initializing the device.
*
* The absolute value returned when the device initialization
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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) \
Expand All @@ -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) \
Expand Down
15 changes: 9 additions & 6 deletions include/zephyr/linker/common-ram.ld
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
17 changes: 14 additions & 3 deletions kernel/device.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down Expand Up @@ -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.
*/
Expand All @@ -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;
}
}
}
Expand Down Expand Up @@ -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.
Expand All @@ -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,
Expand Down