Add unbound DTS option and adjust pbuf for session handshake

doki-nordic · doki-nordic · commit f3f8816a39f4 · 2024-11-22T11:11:05.000+01:00
diff --git a/dts/bindings/ipc/zephyr,ipc-icmsg.yaml b/dts/bindings/ipc/zephyr,ipc-icmsg.yaml
@@ -21,6 +21,21 @@ properties:
     required: true
     type: phandle
 
+  unbound:
+    type: string
+    enum:
+      - disable
+      - enable
+      - compatibility
+    default: disable
+    description: |
+      Select unbound() callback mode. The callback can be enabled or disabled with the
+      "enable" and "disable" option respectively. This functionality requires session
+      number handshake procedure on both sides, so you cannot set "enable" on one side
+      and "disable" on the other. The "compatibility" mode detects if the remote side
+      supports handshake procedure and adjusts its behavior accordingly. The
+      "compatibility" mode is possible only if "dcache-alignment" is at least 8 bytes.
+
   dcache-alignment:
     type: int
     description: |
diff --git a/include/zephyr/ipc/icmsg.h b/include/zephyr/ipc/icmsg.h
@@ -32,6 +32,12 @@ enum icmsg_state {
 	ICMSG_STATE_READY,
 };
 
+enum icmsg_unbound_mode {
+	ICMSG_UNBOUND_MODE_DISABLE,
+	ICMSG_UNBOUND_MODE_ENABLE,
+	ICMSG_UNBOUND_MODE_COMPATIBILITY,
+};
+
 struct icmsg_config_t {
 	struct mbox_dt_spec mbox_tx;
 	struct mbox_dt_spec mbox_rx;
diff --git a/include/zephyr/ipc/pbuf.h b/include/zephyr/ipc/pbuf.h
@@ -47,20 +47,23 @@ extern "C" {
  * The structure contains configuration data.
  */
 struct pbuf_cfg {
-	volatile uint32_t *rd_idx_loc;	/* Address of the variable holding
-					 * index value of the first valid byte
-					 * in data[].
-					 */
-	volatile uint32_t *wr_idx_loc;	/* Address of the variable holding
-					 * index value of the first free byte
-					 * in data[].
-					 */
-	uint32_t dcache_alignment;	/* CPU data cache line size in bytes.
-					 * Used for validation - TODO: To be
-					 * replaced by flags.
-					 */
-	uint32_t len;			/* Length of data[] in bytes. */
-	uint8_t *data_loc;		/* Location of the data[]. */
+	volatile uint32_t *rd_idx_loc;	 /* Address of the variable holding
+					  * index value of the first valid byte
+					  * in data[].
+					  */
+	volatile uint32_t *handshake_loc;/* Address of the variable holding
+					  * handshake information.
+					  */
+	volatile uint32_t *wr_idx_loc;	 /* Address of the variable holding
+					  * index value of the first free byte
+					  * in data[].
+					  */
+	uint32_t dcache_alignment;	 /* CPU data cache line size in bytes.
+					  * Used for validation - TODO: To be
+					  * replaced by flags.
+					  */
+	uint32_t len;			 /* Length of data[] in bytes. */
+	uint8_t *data_loc;		 /* Location of the data[]. */
 };
 
 /**
@@ -112,14 +115,16 @@ struct pbuf {
  * @param size		Size of the memory.
  * @param dcache_align	Data cache alignment.
  */
-#define PBUF_CFG_INIT(mem_addr, size, dcache_align)					\
+#define PBUF_CFG_INIT(mem_addr, size, dcache_align, use_handshake)			\
 {											\
 	.rd_idx_loc = (uint32_t *)(mem_addr),						\
-	.wr_idx_loc = (uint32_t *)((uint8_t *)(mem_addr) +				\
-				MAX(dcache_align, _PBUF_IDX_SIZE)),			\
+	.handshake_loc = use_handshake ? (uint32_t *)((uint8_t *)(mem_addr) +		\
+				_PBUF_IDX_SIZE) : NULL,					\
+	.wr_idx_loc = (uint32_t *)((uint8_t *)(mem_addr) + MAX(dcache_align,		\
+				(use_handshake ? 2 : 1) * _PBUF_IDX_SIZE)),		\
 	.data_loc = (uint8_t *)((uint8_t *)(mem_addr) +					\
-				MAX(dcache_align, _PBUF_IDX_SIZE) + _PBUF_IDX_SIZE),	\
-	.len = (uint32_t)((uint32_t)(size) - MAX(dcache_align, _PBUF_IDX_SIZE) -	\
+				MAX(dcache_align, (use_handshake ? 2 : 1) * _PBUF_IDX_SIZE) + _PBUF_IDX_SIZE),	\
+	.len = (uint32_t)((uint32_t)(size) - MAX(dcache_align, (use_handshake ? 2 : 1) * _PBUF_IDX_SIZE) -	\
 				_PBUF_IDX_SIZE),					\
 	.dcache_alignment = (dcache_align),						\
 }
@@ -142,7 +147,7 @@ struct pbuf {
  * @param size			Size of the memory.
  * @param dcache_align	Data cache line size.
  */
-#define PBUF_DEFINE(name, mem_addr, size, dcache_align)					\
+#define PBUF_DEFINE(name, mem_addr, size, dcache_align, use_handshake, compatibility)	\
 	BUILD_ASSERT(dcache_align >= 0,							\
 			"Cache line size must be non negative.");			\
 	BUILD_ASSERT((size) > 0 && IS_PTR_ALIGNED_BYTES(size, _PBUF_IDX_SIZE),		\
@@ -151,8 +156,10 @@ struct pbuf {
 			"Misaligned memory.");						\
 	BUILD_ASSERT(size >= (MAX(dcache_align, _PBUF_IDX_SIZE) + _PBUF_IDX_SIZE +	\
 			_PBUF_MIN_DATA_LEN), "Insufficient size.");			\
+	BUILD_ASSERT(!(compatibility) || (dcache_align) >= 8,				\
+		"Data cache alignment must be at least 8 if compatibility is enabled.");\
 	static PBUF_MAYBE_CONST struct pbuf_cfg cfg_##name =				\
-			PBUF_CFG_INIT(mem_addr, size, dcache_align);			\
+			PBUF_CFG_INIT(mem_addr, size, dcache_align, use_handshake);	\
 	static struct pbuf name = {							\
 		.cfg = &cfg_##name,							\
 	}
@@ -223,6 +230,9 @@ int pbuf_write(struct pbuf *pb, const char *buf, uint16_t len);
  */
 int pbuf_read(struct pbuf *pb, char *buf, uint16_t len);
 
+uint32_t pbuf_handshake_read(struct pbuf *pb);
+void pbuf_handshake_write(struct pbuf *pb, uint32_t value);
+
 /**
  * @}
  */
diff --git a/samples/subsys/ipc/ipc_service/icmsg/boards/nrf5340dk_nrf5340_cpuapp.overlay b/samples/subsys/ipc/ipc_service/icmsg/boards/nrf5340dk_nrf5340_cpuapp.overlay
@@ -26,6 +26,7 @@
 
 		ipc0: ipc0 {
 			compatible = "zephyr,ipc-icmsg";
+			unbound = "enable";
 			tx-region = <&sram_tx>;
 			rx-region = <&sram_rx>;
 			mboxes = <&mbox 0>, <&mbox 1>;
diff --git a/samples/subsys/ipc/ipc_service/icmsg/remote/boards/nrf5340dk_nrf5340_cpunet.overlay b/samples/subsys/ipc/ipc_service/icmsg/remote/boards/nrf5340dk_nrf5340_cpunet.overlay
@@ -26,6 +26,7 @@
 
 		ipc0: ipc0 {
 			compatible = "zephyr,ipc-icmsg";
+			unbound = "enable";
 			tx-region = <&sram_tx>;
 			rx-region = <&sram_rx>;
 			mboxes = <&mbox 0>, <&mbox 1>;
diff --git a/subsys/ipc/ipc_service/backends/ipc_icmsg.c b/subsys/ipc/ipc_service/backends/ipc_icmsg.c
@@ -54,6 +54,8 @@ static int backend_init(const struct device *instance)
 	return 0;
 }
 
+#define UNBOUND_MODE(i) CONCAT(ICMSG_UNBOUND_MODE_, DT_INST_STRING_UPPER_TOKEN(i, unbound))
+
 #define DEFINE_BACKEND_DEVICE(i)					\
 	static const struct icmsg_config_t backend_config_##i = {	\
 		.mbox_tx = MBOX_DT_SPEC_INST_GET(i, tx),		\
@@ -63,11 +65,15 @@ static int backend_init(const struct device *instance)
 	PBUF_DEFINE(tx_pb_##i,						\
 			DT_REG_ADDR(DT_INST_PHANDLE(i, tx_region)),	\
 			DT_REG_SIZE(DT_INST_PHANDLE(i, tx_region)),	\
-			DT_INST_PROP_OR(i, dcache_alignment, 0));	\
+			DT_INST_PROP_OR(i, dcache_alignment, 0),	\
+			UNBOUND_MODE(i) != ICMSG_UNBOUND_MODE_DISABLE,	\
+			UNBOUND_MODE(i) == ICMSG_UNBOUND_MODE_COMPATIBILITY);	\
 	PBUF_DEFINE(rx_pb_##i,						\
 			DT_REG_ADDR(DT_INST_PHANDLE(i, rx_region)),	\
 			DT_REG_SIZE(DT_INST_PHANDLE(i, rx_region)),	\
-			DT_INST_PROP_OR(i, dcache_alignment, 0));	\
+			DT_INST_PROP_OR(i, dcache_alignment, 0),	\
+			UNBOUND_MODE(i) != ICMSG_UNBOUND_MODE_DISABLE,	\
+			UNBOUND_MODE(i) == ICMSG_UNBOUND_MODE_COMPATIBILITY);	\
 									\
 	static struct icmsg_data_t backend_data_##i = {			\
 		.tx_pb = &tx_pb_##i,					\
diff --git a/subsys/ipc/ipc_service/lib/icmsg.c b/subsys/ipc/ipc_service/lib/icmsg.c
@@ -15,8 +15,10 @@
 #define BOND_NOTIFY_REPEAT_TO	K_MSEC(CONFIG_IPC_SERVICE_ICMSG_BOND_NOTIFY_REPEAT_TO_MS)
 #define SHMEM_ACCESS_TO		K_MSEC(CONFIG_IPC_SERVICE_ICMSG_SHMEM_ACCESS_TO_MS)
 
-static const uint8_t magic[] = {0x45, 0x6d, 0x31, 0x6c, 0x31, 0x4b,
-				0x30, 0x72, 0x6e, 0x33, 0x6c, 0x69, 0x34};
+static const uint8_t magic[] = {0x45, 0x6d, 0x31, 0x6c, 0x31, 0x4b, 0x30,
+				0x72, 0x6e, 0x33, 0x6c, 0x69, 0x34, 0x01 };
+
+#define OLD_MAGIC_LENGTH (sizeof(magic) - 1)
 
 #ifdef CONFIG_MULTITHREADING
 #if defined(CONFIG_IPC_SERVICE_BACKEND_ICMSG_WQ_ENABLE)
diff --git a/subsys/ipc/ipc_service/lib/pbuf.c b/subsys/ipc/ipc_service/lib/pbuf.c
@@ -38,6 +38,7 @@ static int validate_cfg(const struct pbuf_cfg *cfg)
 	/* Validate pointer alignment. */
 	if (!IS_PTR_ALIGNED_BYTES(cfg->rd_idx_loc, MAX(cfg->dcache_alignment, _PBUF_IDX_SIZE)) ||
 	    !IS_PTR_ALIGNED_BYTES(cfg->wr_idx_loc, MAX(cfg->dcache_alignment, _PBUF_IDX_SIZE)) ||
+	    !IS_PTR_ALIGNED_BYTES(cfg->handshake_loc, _PBUF_IDX_SIZE) ||
 	    !IS_PTR_ALIGNED_BYTES(cfg->data_loc, _PBUF_IDX_SIZE)) {
 		return -EINVAL;
 	}
@@ -49,6 +50,8 @@ static int validate_cfg(const struct pbuf_cfg *cfg)
 
 	/* Validate pointer values. */
 	if (!(cfg->rd_idx_loc < cfg->wr_idx_loc) ||
+	    (cfg->handshake_loc && !(cfg->rd_idx_loc < cfg->handshake_loc)) ||
+	    !(cfg->handshake_loc < cfg->wr_idx_loc) ||
 	    !((uint8_t *)cfg->wr_idx_loc < cfg->data_loc) ||
 	    !(((uint8_t *)cfg->rd_idx_loc + MAX(_PBUF_IDX_SIZE, cfg->dcache_alignment)) ==
 	    (uint8_t *)cfg->wr_idx_loc)) {
@@ -253,3 +256,19 @@ int pbuf_read(struct pbuf *pb, char *buf, uint16_t len)
 
 	return len;
 }
+
+uint32_t pbuf_handshake_read(struct pbuf *pb)
+{
+	volatile uint32_t *ptr = pb->cfg->handshake_loc;
+	sys_cache_data_invd_range((void *)ptr, sizeof(*ptr));
+	__sync_synchronize();
+	return *ptr;
+}
+
+void pbuf_handshake_write(struct pbuf *pb, uint32_t value)
+{
+	volatile uint32_t *ptr = pb->cfg->handshake_loc;
+	*ptr = value;
+	__sync_synchronize();
+	sys_cache_data_flush_range((void *)ptr, sizeof(*ptr));
+}
