device: refactor to aggregate common dynamic state for devices

include/device.h

@@ -270,8 +270,11 @@ struct device_pm {
  const struct device *dev;
  /** Lock to synchronize the get/put operations */
  struct k_sem lock;
+ /* Following are packed fields protected by #lock. */
  /** Device pm enable flag */
- bool enable;
+ bool enable : 1;
+ /* Following are packed fields accessed with atomic bit operations. */
+ atomic_t atomic_flags;
  /** Device usage count */
  atomic_t usage;
  /** Device idle internal power state */
@@ -284,8 +287,42 @@ struct device_pm {
  struct k_poll_signal signal;
 };
 
+/** Bit position in device_pm::atomic_flags that records whether the
+ * device is busy.
+ */
+#define DEVICE_PM_ATOMIC_FLAGS_BUSY_BIT 0
+
 /**
- * @brief Runtime device structure (in memory) per driver instance
+ * @brief Runtime device dynamic structure (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 {
+ /** Non-negative result of initializing the device.
+ *
+ * The absolute value returned when the device initialization
+ * function was invoked, or `UINT8_MAX` if the value exceeds
+ * an 8-bit integer. If initialized is also set, a zero value
+ * indicates initialization succeeded.
+ */
+ unsigned int init_res : 8;
+
+ /** Indicates the device initialization function has been
+ * invoked.
+ */
+ bool initialized : 1;
+
+#ifdef CONFIG_PM_DEVICE
+ /* Power management data */
+ struct device_pm pm;
+#endif /* CONFIG_PM_DEVICE */
+};
+
+/**
+ * @brief Runtime device structure (in ROM) per driver instance
  */
 struct device {
  /** Name of the device instance */
@@ -294,6 +331,8 @@ 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;
 #ifdef CONFIG_PM_DEVICE
@@ -697,16 +736,22 @@ static inline int device_pm_put_sync(const struct device *dev) { return -ENOTSUP
  */
 #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
+ * dev_name.
+ */
+#define Z_DEVICE_STATE_NAME(dev_name) _CONCAT(__devstate_, dev_name)
+
 #define Z_DEVICE_DEFINE(node_id, dev_name, drv_name, init_fn, pm_control_fn, \
  data_ptr, cfg_ptr, level, prio, api_ptr) \
- Z_DEVICE_DEFINE_PM(dev_name)  \
+ static struct device_state Z_DEVICE_STATE_NAME(dev_name); \
  COND_CODE_1(DT_NODE_EXISTS(node_id), (), (static)) \
  const Z_DECL_ALIGN(struct device) \
  DEVICE_NAME_GET(dev_name) __used \
  __attribute__((__section__(".device_" #level STRINGIFY(prio)))) = { \
  .name = drv_name, \
  .config = (cfg_ptr), \
  .api = (api_ptr), \
+ .state = &Z_DEVICE_STATE_NAME(dev_name), \
  .data = (data_ptr), \
  Z_DEVICE_DEFINE_PM_INIT(dev_name, pm_control_fn) \
  }; \
@@ -716,23 +761,10 @@ static inline int device_pm_put_sync(const struct device *dev) { return -ENOTSUP
  (&DEVICE_NAME_GET(dev_name)), level, prio)
 
 #ifdef CONFIG_PM_DEVICE
-#define Z_DEVICE_DEFINE_PM(dev_name) \
- static struct device_pm _CONCAT(__pm_, dev_name) __used = { \
- .usage = ATOMIC_INIT(0), \
- .lock = Z_SEM_INITIALIZER( \
- _CONCAT(__pm_, dev_name).lock, 1, 1), \
- .signal = K_POLL_SIGNAL_INITIALIZER( \
- _CONCAT(__pm_, dev_name).signal), \
- .event = K_POLL_EVENT_INITIALIZER( \
- K_POLL_TYPE_SIGNAL, \
- K_POLL_MODE_NOTIFY_ONLY, \
- &_CONCAT(__pm_, dev_name).signal), \
- };
 #define Z_DEVICE_DEFINE_PM_INIT(dev_name, pm_control_fn) \
  .device_pm_control = (pm_control_fn), \
- .pm  = &_CONCAT(__pm_, dev_name),
+ .pm = &Z_DEVICE_STATE_NAME(dev_name).pm,
 #else
-#define Z_DEVICE_DEFINE_PM(dev_name)
 #define Z_DEVICE_DEFINE_PM_INIT(dev_name, pm_control_fn)
 #endif
 

include/linker/common-ram.ld

@@ -13,33 +13,6 @@
  } GROUP_DATA_LINK_IN(RAMABLE_REGION, ROMABLE_REGION)
 #endif
 
-/*
- * Space for storing per device init status and busy bitmap in case PM is
- * enabled. Since we do not know beforehand the number of devices,
- * we go through the below mechanism to allocate the required space.
- * Both are made of 1 bit per-device instance, so we compute the size of
- * of an entire bitfield, aligned on 32bits.
- */
-#define DEVICE_COUNT \
- ((__device_end - __device_start) / _DEVICE_STRUCT_SIZEOF)
-#define DEVICE_BITFIELD_SIZE (((DEVICE_COUNT + 31) / 32) * 4)
-
-#define DEVICE_INIT_STATUS_BITFIELD() \
- FILL(0x00); \
- __device_init_status_start = .; \
- . = . + DEVICE_BITFIELD_SIZE; \
- __device_init_status_end = .;
-
-#ifdef CONFIG_PM_DEVICE
-#define DEVICE_BUSY_BITFIELD() \
- FILL(0x00); \
- __device_busy_start = .; \
- . = . + DEVICE_BITFIELD_SIZE; \
- __device_busy_end = .;
-#else
-#define DEVICE_BUSY_BITFIELD()
-#endif
-
  SECTION_DATA_PROLOGUE(devices,,)
  {
  /* link in devices objects, which are tied to the init ones;
@@ -53,8 +26,6 @@
  CREATE_OBJ_LEVEL(device, APPLICATION)
  CREATE_OBJ_LEVEL(device, SMP)
  __device_end = .;
- DEVICE_INIT_STATUS_BITFIELD()
- DEVICE_BUSY_BITFIELD()
  } GROUP_DATA_LINK_IN(RAMABLE_REGION, ROMABLE_REGION)
 
  SECTION_DATA_PROLOGUE(initshell,,)

kernel/device.c

@@ -25,11 +25,37 @@ extern const struct device __device_end[];
 
 extern uint32_t __device_init_status_start[];
 
+static inline void device_pm_state_init(const struct device *dev)
+{
 #ifdef CONFIG_PM_DEVICE
-extern uint32_t __device_busy_start[];
-extern uint32_t __device_busy_end[];
-#define DEVICE_BUSY_SIZE (__device_busy_end - __device_busy_start)
-#endif
+ *dev->pm = (struct device_pm){
+ .usage = ATOMIC_INIT(0),
+ .lock = Z_SEM_INITIALIZER(dev->pm->lock, 1, 1),
+ .signal = K_POLL_SIGNAL_INITIALIZER(dev->pm->signal),
+ .event = K_POLL_EVENT_INITIALIZER(
+ K_POLL_TYPE_SIGNAL,
+ K_POLL_MODE_NOTIFY_ONLY,
+ &dev->pm->signal),
+ };
+#endif /* CONFIG_PM_DEVICE */
+}
+
+/**
+ * @brief Initialize state for all static devices.
+ *
+ * The state object is always zero-initialized, but this may not be
+ * sufficient.
+ */
+void z_device_state_init(void)
+{
+ const struct device *dev = __device_start;
+
+ while (dev < __device_end) {
+ device_pm_state_init(dev);
+ z_object_init(dev);
+ ++dev;
+ }
+}
 
 /**
  * @brief Execute all the init entry initialization functions at a given level
@@ -59,18 +85,22 @@ void z_sys_init_run_level(int32_t level)
 
  for (entry = levels[level]; entry < levels[level+1]; entry++) {
  const struct device *dev = entry->dev;
+ int rc = entry->init(dev);
 
  if (dev != NULL) {
- z_object_init(dev);
- }
-
- if ((entry->init(dev) != 0) && (dev != NULL)) {
- /* Initialization failed.
- * Set the init status bit so device is not declared ready.
+ /* Mark device initialized. If initialization
+ * failed, record the error condition.
  */
- sys_bitfield_set_bit(
- (mem_addr_t) __device_init_status_start,
- (dev - __device_start));
+ if (rc != 0) {
+ if (rc < 0) {
+ rc = -rc;
+ }
+ if (rc > UINT8_MAX) {
+ rc = UINT8_MAX;
+ }
+ dev->state->init_res = rc;
+ }
+ dev->state->initialized = true;
  }
  }
 }
@@ -129,9 +159,7 @@ size_t z_device_get_all_static(struct device const **devices)
 
 bool z_device_ready(const struct device *dev)
 {
- /* Set bit indicates device failed initialization */
- return !(sys_bitfield_test_bit((mem_addr_t)__device_init_status_start,
- (dev - __device_start)));
+ return dev->state->initialized && (dev->state->init_res == 0);
 }
 
 #ifdef CONFIG_PM_DEVICE
@@ -146,43 +174,46 @@ int device_pm_control_nop(const struct device *unused_device,
 
 int device_any_busy_check(void)
 {
- int i = 0;
+ const struct device *dev = __device_start;
 
- for (i = 0; i < DEVICE_BUSY_SIZE; i++) {
- if (__device_busy_start[i] != 0U) {
+ while (dev < __device_end) {
+ if (atomic_test_bit(&dev->pm->atomic_flags,
+ DEVICE_PM_ATOMIC_FLAGS_BUSY_BIT)) {
  return -EBUSY;
  }
+ ++dev;
  }
+
  return 0;
 }
 
-int device_busy_check(const struct device *chk_dev)
+int device_busy_check(const struct device *dev)
 {
- if (atomic_test_bit((const atomic_t *)__device_busy_start,
- (chk_dev - __device_start))) {
+ if (atomic_test_bit(&dev->pm->atomic_flags,
+ DEVICE_PM_ATOMIC_FLAGS_BUSY_BIT)) {
  return -EBUSY;
  }
  return 0;
 }
 
 #endif
 
-void device_busy_set(const struct device *busy_dev)
+void device_busy_set(const struct device *dev)
 {
 #ifdef CONFIG_PM_DEVICE
- atomic_set_bit((atomic_t *) __device_busy_start,
- (busy_dev - __device_start));
+ atomic_set_bit(&dev->pm->atomic_flags,
+ DEVICE_PM_ATOMIC_FLAGS_BUSY_BIT);
 #else
- ARG_UNUSED(busy_dev);
+ ARG_UNUSED(dev);
 #endif
 }
 
-void device_busy_clear(const struct device *busy_dev)
+void device_busy_clear(const struct device *dev)
 {
 #ifdef CONFIG_PM_DEVICE
- atomic_clear_bit((atomic_t *) __device_busy_start,
- (busy_dev - __device_start));
+ atomic_clear_bit(&dev->pm->atomic_flags,
+ DEVICE_PM_ATOMIC_FLAGS_BUSY_BIT);
 #else
- ARG_UNUSED(busy_dev);
+ ARG_UNUSED(dev);
 #endif
 }

kernel/include/kernel_internal.h

@@ -37,6 +37,8 @@ static inline void z_data_copy(void)
 #endif
 FUNC_NORETURN void z_cstart(void);
 
+void z_device_state_init(void);
+
 extern FUNC_NORETURN void z_thread_entry(k_thread_entry_t entry,
  void *p1, void *p2, void *p3);
 

kernel/init.c

@@ -386,6 +386,9 @@ FUNC_NORETURN void z_cstart(void)
 #if defined(CONFIG_MMU) && defined(CONFIG_USERSPACE)
  z_kernel_map_fixup();
 #endif
+ /* do any necessary initialization of static devices */
+ z_device_state_init();
+
  /* perform basic hardware initialization */
  z_sys_init_run_level(_SYS_INIT_LEVEL_PRE_KERNEL_1);
  z_sys_init_run_level(_SYS_INIT_LEVEL_PRE_KERNEL_2);