inte_adsp: ipc: prevent ipc message send during Device power transition

When CONFIG_PM_DEVICE is enabled IPC Device may be during power transition
during a call to intel_adsp_ipc_send_message function.
Changed signatures of intel_adsp_ipc_send_message and its sync version
to return int and negative error codes on error.
On attempt to send IPC message during Device power transition
-ECANCELED error code is returned, on busy state -EBUSY.
Updated all function references.

Signed-off-by: Andrey Borisovich <andrey.borisovich@intel.com>

drivers/ipm/ipm_cavs_host.c

@@ -78,7 +78,7 @@ static int send(const struct device *dev, int wait, uint32_t id,
 
 	memcpy(buf, data, size);
 
-	bool ok = intel_adsp_ipc_send_message(INTEL_ADSP_IPC_HOST_DEV, id, ext_data);
+	int ret = intel_adsp_ipc_send_message(INTEL_ADSP_IPC_HOST_DEV, id, ext_data);
 
 	/* The IPM docs call for "busy waiting" here, but in fact
 	 * there's a blocking synchronous call available that might be
@@ -87,13 +87,13 @@ static int send(const struct device *dev, int wait, uint32_t id,
 	 * benefit anyway as all its usage is async. This is OK for
 	 * now.
 	 */
-	if (ok && wait) {
+	if (ret == -EBUSY && wait) {
 		while (!intel_adsp_ipc_is_complete(INTEL_ADSP_IPC_HOST_DEV)) {
 			k_busy_wait(1);
 		}
 	}
 
-	return ok ? 0 : -EBUSY;
+	return ret;
 }
 
 static bool ipc_handler(const struct device *dev, void *arg,

soc/xtensa/intel_adsp/common/include/intel_adsp_ipc.h

@@ -139,30 +139,32 @@ bool intel_adsp_ipc_is_complete(const struct device *dev);
  *
  * Sends a message to the other side of an IPC link. The data and
  * ext_data parameters are passed using the IDR/IDD registers.
- * Returns true if the message was sent, false if a current message is
- * in progress (in the sense of intel_adsp_ipc_is_complete()).
+ * Returns 0 if the message was sent, negative error values:
+ * -EBUSY if there is already IPC message processed (intel_adsp_ipc_is_complete returns false).
+ * -ECANCELED if IPC device will not send the message as it undergoes power
+ * transition.
  *
  * @param dev IPC device.
  * @param data 30 bits value to transmit with the message (IDR register).
  * @param ext_data Extended value to transmit with the message (IDD register).
  * @return message successfully transmitted.
  */
-bool intel_adsp_ipc_send_message(const struct device *dev,
+int intel_adsp_ipc_send_message(const struct device *dev,
 	uint32_t data, uint32_t ext_data);
 
 /** @brief Send an IPC message, block until completion.
  *
  * As for intel_adsp_ipc_send_message(), but blocks the current thread until
  * the completion of the message or the expiration of the provided
- * timeout.
+ * timeout. Returns immediately if IPC device is during power transition.
  *
  * @param dev IPC device
  * @param data 30 bits value to transmit with the message (IDR register)
  * @param ext_data Extended value to transmit with the message (IDD register)
  * @param timeout Maximum time to wait, or K_FOREVER, or K_NO_WAIT
- * @return message successfully transmitted
+ * @return returns 0 if message successfully transmited, otherwise error code.
  */
-bool intel_adsp_ipc_send_message_sync(const struct device *dev,
+int intel_adsp_ipc_send_message_sync(const struct device *dev,
 	uint32_t data, uint32_t ext_data, k_timeout_t timeout);
 
 

soc/xtensa/intel_adsp/common/ipc.c

@@ -132,17 +132,20 @@ bool intel_adsp_ipc_is_complete(const struct device *dev)
 	return not_busy && !devdata->tx_ack_pending;
 }
 
-bool intel_adsp_ipc_send_message(const struct device *dev,
+int intel_adsp_ipc_send_message(const struct device *dev,
 			   uint32_t data, uint32_t ext_data)
 {
+	if (pm_device_state_is_locked(INTEL_ADSP_IPC_HOST_DEV))
+		return -ECANCELED;
+
 	pm_device_busy_set(dev);
 	const struct intel_adsp_ipc_config *config = dev->config;
 	struct intel_adsp_ipc_data *devdata = dev->data;
 	k_spinlock_key_t key = k_spin_lock(&devdata->lock);
 
 	if ((config->regs->idr & INTEL_ADSP_IPC_BUSY) != 0 || devdata->tx_ack_pending) {
 		k_spin_unlock(&devdata->lock, key);
-		return false;
+		return -EBUSY;
 	}
 
 	k_sem_init(&devdata->sem, 0, 1);
@@ -153,15 +156,15 @@ bool intel_adsp_ipc_send_message(const struct device *dev,
 	return true;
 }
 
-bool intel_adsp_ipc_send_message_sync(const struct device *dev,
+int intel_adsp_ipc_send_message_sync(const struct device *dev,
 				uint32_t data, uint32_t ext_data,
 				k_timeout_t timeout)
 {
 	struct intel_adsp_ipc_data *devdata = dev->data;
 
-	bool ret = intel_adsp_ipc_send_message(dev, data, ext_data);
+	int ret = intel_adsp_ipc_send_message(dev, data, ext_data);
 
-	if (ret) {
+	if (ret != 0 && ret != -ECANCELED) {
 		k_sem_take(&devdata->sem, timeout);
 	}
 	return ret;

subsys/logging/backends/log_backend_adsp_hda.c

@@ -332,8 +332,10 @@ void adsp_hda_log_init(adsp_hda_log_hook_t fn, uint32_t channel)
 static inline void hda_ipc_msg(const struct device *dev, uint32_t data,
 			       uint32_t ext, k_timeout_t timeout)
 {
-	__ASSERT(intel_adsp_ipc_send_message_sync(dev, data, ext, timeout),
-		"Unexpected ipc send message failure, try increasing IPC_TIMEOUT");
+	int ret = intel_adsp_ipc_send_message_sync(dev, data, ext, timeout);
+
+	__ASSERT(!ret, "Unexpected ipc send message failure, error code: %d",
+		ret);
 }
 
 
@@ -342,20 +344,20 @@ void adsp_hda_log_cavstool_hook(uint32_t written)
 	/* We *must* send this, but we may be in a timer ISR, so we are
 	 * forced into a retry loop without timeouts and such.
 	 */
-	bool done = false;
+	int ret = -1;
 
 	/*  Send IPC message notifying log data has been written */
 	do {
-		done = intel_adsp_ipc_send_message(INTEL_ADSP_IPC_HOST_DEV, IPCCMD_HDA_PRINT,
+		ret = intel_adsp_ipc_send_message(INTEL_ADSP_IPC_HOST_DEV, IPCCMD_HDA_PRINT,
 					     (written << 8) | CHANNEL);
-	} while (!done);
-
+		if (ret == -ECANCELED) {
+			return;
+		}
+	} while (ret);
 
 	/* Wait for confirmation log data has been received */
-	do {
-		done = intel_adsp_ipc_is_complete(INTEL_ADSP_IPC_HOST_DEV);
-	} while (!done);
-
+	while (!intel_adsp_ipc_is_complete(INTEL_ADSP_IPC_HOST_DEV))
+		;
 }
 
 int adsp_hda_log_cavstool_init(void)

tests/boards/intel_adsp/hda/src/tests.h

@@ -31,8 +31,9 @@
 static inline void hda_ipc_msg(const struct device *dev, uint32_t data,
 			       uint32_t ext, k_timeout_t timeout)
 {
-	zassert_true(intel_adsp_ipc_send_message_sync(dev, data, ext, timeout),
-		"Unexpected ipc send message failure, try increasing IPC_TIMEOUT");
+	int ret = intel_adsp_ipc_send_message_sync(dev, data, ext, timeout);
+
+	zassert_true(!ret, "Unexpected ipc send message failure, error code: %d", ret);
 }
 
 #endif /* ZEPHYR_TESTS_INTEL_ADSP_TESTS_H */

tests/boards/intel_adsp/hda_log/src/tests.h

@@ -13,8 +13,9 @@
 static inline void hda_ipc_msg(const struct device *dev, uint32_t data,
 			       uint32_t ext, k_timeout_t timeout)
 {
-	zassert_true(intel_adsp_ipc_send_message_sync(dev, data, ext, timeout),
-		"Unexpected ipc send message failure, try increasing IPC_TIMEOUT");
+	int ret = intel_adsp_ipc_send_message_sync(dev, data, ext, timeout);
+
+	zassert_true(!ret, "Unexpected ipc send message failure, error code: %d", ret);
 }
 
 #endif /* ZEPHYR_TESTS_INTEL_ADSP_TESTS_H */

tests/boards/intel_adsp/smoke/src/cpus.c

@@ -162,7 +162,7 @@ static void halt_and_restart(int cpu)
 	int ret;
 
 	/* On older hardware we need to get the host to turn the core
-	 * off.  Construct an ADSPCS with only this core disabled
+	 * off. Construct an ADSPCS with only this core disabled
 	 */
 	if (!IS_ENABLED(CONFIG_SOC_INTEL_CAVS_V25)) {
 		intel_adsp_ipc_send_message(INTEL_ADSP_IPC_HOST_DEV, IPCCMD_ADSPCS,

tests/boards/intel_adsp/smoke/src/hostipc.c

@@ -32,7 +32,7 @@ static bool ipc_done(const struct device *dev, void *arg)
 
 ZTEST(intel_adsp, test_host_ipc)
 {
-	bool ret;
+	int ret;
 
 	intel_adsp_ipc_set_message_handler(INTEL_ADSP_IPC_HOST_DEV, ipc_message, NULL);
 	intel_adsp_ipc_set_done_handler(INTEL_ADSP_IPC_HOST_DEV, ipc_done, NULL);
@@ -41,7 +41,7 @@ ZTEST(intel_adsp, test_host_ipc)
 	printk("Simple message send...\n");
 	done_flag = false;
 	ret = intel_adsp_ipc_send_message(INTEL_ADSP_IPC_HOST_DEV, IPCCMD_SIGNAL_DONE, 0);
-	zassert_true(ret, "send failed");
+	zassert_true(!ret, "send failed");
 	AWAIT(intel_adsp_ipc_is_complete(INTEL_ADSP_IPC_HOST_DEV));
 	AWAIT(done_flag);
 
@@ -53,7 +53,7 @@ ZTEST(intel_adsp, test_host_ipc)
 	msg_flag = false;
 	ret = intel_adsp_ipc_send_message(INTEL_ADSP_IPC_HOST_DEV, IPCCMD_RETURN_MSG,
 				RETURN_MSG_SYNC_VAL);
-	zassert_true(ret, "send failed");
+	zassert_true(!ret, "send failed");
 	AWAIT(done_flag);
 	AWAIT(intel_adsp_ipc_is_complete(INTEL_ADSP_IPC_HOST_DEV));
 	AWAIT(msg_flag);
@@ -66,7 +66,7 @@ ZTEST(intel_adsp, test_host_ipc)
 	msg_flag = false;
 	ret = intel_adsp_ipc_send_message(INTEL_ADSP_IPC_HOST_DEV, IPCCMD_RETURN_MSG,
 				RETURN_MSG_SYNC_VAL);
-	zassert_true(ret, "send failed");
+	zassert_true(!ret, "send failed");
 	AWAIT(done_flag);
 	AWAIT(intel_adsp_ipc_is_complete(INTEL_ADSP_IPC_HOST_DEV));
 	AWAIT(msg_flag);
@@ -77,7 +77,7 @@ ZTEST(intel_adsp, test_host_ipc)
 	msg_flag = false;
 	ret = intel_adsp_ipc_send_message(INTEL_ADSP_IPC_HOST_DEV, IPCCMD_RETURN_MSG,
 				RETURN_MSG_ASYNC_VAL);
-	zassert_true(ret, "send failed");
+	zassert_true(!ret, "send failed");
 	AWAIT(done_flag);
 	AWAIT(intel_adsp_ipc_is_complete(INTEL_ADSP_IPC_HOST_DEV));
 	AWAIT(msg_flag);
@@ -92,7 +92,7 @@ ZTEST(intel_adsp, test_host_ipc)
 	done_flag = false;
 	ret = intel_adsp_ipc_send_message_sync(INTEL_ADSP_IPC_HOST_DEV, IPCCMD_ASYNC_DONE_DELAY,
 					 0, K_FOREVER);
-	zassert_true(ret, "send failed");
+	zassert_true(!ret, "send failed");
 	zassert_true(done_flag, "done interrupt failed to fire");
 	zassert_true(intel_adsp_ipc_is_complete(INTEL_ADSP_IPC_HOST_DEV),
 		"sync message incomplete");