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);