From 15159d9a606654efa573efe048890ddc936ba884 Mon Sep 17 00:00:00 2001 From: Tomasz Leman Date: Thu, 4 Dec 2025 15:26:01 +0100 Subject: [PATCH 1/4] ipc_service: Add ipc_service_send_critical function Add ipc_service_send_critical() as a dedicated API for sending critical high-priority messages over an IPC endpoint. Introduce an optional send_critical() callback in struct ipc_service_backend so backends can implement a special fast path that bypasses normal state and busy checks for critical notifications such as crash reports or fatal errors. The ipc_service_send_critical() wrapper mirrors ipc_service_send() on the service side and delegates the actual behavior to the backend-specific send_critical() implementation. Signed-off-by: Tomasz Leman --- include/zephyr/ipc/ipc_service.h | 30 ++++++++++++++++++++++++ include/zephyr/ipc/ipc_service_backend.h | 23 ++++++++++++++++++ subsys/ipc/ipc_service/ipc_service.c | 24 +++++++++++++++++++ 3 files changed, 77 insertions(+) diff --git a/include/zephyr/ipc/ipc_service.h b/include/zephyr/ipc/ipc_service.h index dbc0b8b7ee48a..7bcf6c4ca4218 100644 --- a/include/zephyr/ipc/ipc_service.h +++ b/include/zephyr/ipc/ipc_service.h @@ -315,6 +315,36 @@ int ipc_service_deregister_endpoint(struct ipc_ept *ept); */ int ipc_service_send(struct ipc_ept *ept, const void *data, size_t len); +/** @brief Send a high-priority message bypassing normal state checks. + * + * This function sends a high-priority message using the backend in a + * special fast path that skips normal state and busy checking. It is + * intended for critical system notifications such as crash reports, + * fatal errors, or emergency shutdown signals that must be delivered + * even when the IPC channel is otherwise considered busy or blocked. + * + * WARNING: This function should only be used for critical system notifications. + * Misuse can lead to data corruption or system instability. The backend must + * support this operation. + * + * @param[in] ept Registered endpoint by @ref ipc_service_register_endpoint. + * @param[in] data Pointer to the critical message buffer to send. + * @param[in] len Number of bytes to send. + * + * @retval -EIO when no backend is registered or send_critical hook is missing + * from backend. + * @retval -EINVAL when instance or endpoint is invalid. + * @retval -ENOENT when the endpoint is not registered with the instance. + * @retval -EBADMSG when the data is invalid (i.e. invalid data format, + * invalid length exceeds backend limits, ...). + * @retval -ENOTSUP when the operation is not supported by backend. + * @retval -ENOMEM when even critical path buffers are exhausted. + * + * @retval bytes number of bytes sent. + * @retval other errno codes depending on the implementation of the backend. + */ +int ipc_service_send_critical(struct ipc_ept *ept, const void *data, size_t len); + /** @brief Get the TX buffer size * * Get the maximal size of a buffer which can be obtained by @ref diff --git a/include/zephyr/ipc/ipc_service_backend.h b/include/zephyr/ipc/ipc_service_backend.h index 3e2f4b11f180a..3f02497154598 100644 --- a/include/zephyr/ipc/ipc_service_backend.h +++ b/include/zephyr/ipc/ipc_service_backend.h @@ -71,6 +71,29 @@ struct ipc_service_backend { int (*send)(const struct device *instance, void *token, const void *data, size_t len); + /** @brief Pointer to the function that sends critical messages bypassing flow control. + * + * This function sends high-priority messages directly, bypassing busy checks + * and normal queuing mechanisms. It should be used only for critical system + * notifications like crash reports or fatal errors. + * + * @param[in] instance Instance pointer. + * @param[in] token Backend-specific token. + * @param[in] data Pointer to the buffer to send. + * @param[in] len Number of bytes to send. + * + * @retval -EINVAL when instance is invalid. + * @retval -ENOENT when the endpoint is not registered with the instance. + * @retval -EBADMSG when the message is invalid. + * @retval -ENOTSUP when critical send is not supported. + * @retval -ENOMEM when even critical buffers are unavailable. + * + * @retval bytes number of bytes sent. + * @retval other errno codes depending on the implementation of the backend. + */ + int (*send_critical)(const struct device *instance, void *token, + const void *data, size_t len); + /** @brief Pointer to the function that will be used to register endpoints. * * @param[in] instance Instance to register the endpoint onto. diff --git a/subsys/ipc/ipc_service/ipc_service.c b/subsys/ipc/ipc_service/ipc_service.c index bc2e1bf7918cf..c8aa76c70d624 100644 --- a/subsys/ipc/ipc_service/ipc_service.c +++ b/subsys/ipc/ipc_service/ipc_service.c @@ -144,6 +144,30 @@ int ipc_service_send(struct ipc_ept *ept, const void *data, size_t len) return backend->send(ept->instance, ept->token, data, len); } +int ipc_service_send_critical(struct ipc_ept *ept, const void *data, size_t len) +{ + const struct ipc_service_backend *backend; + + if (!ept) { + LOG_ERR("Invalid endpoint"); + return -EINVAL; + } + + if (!ept->instance) { + LOG_ERR("Endpoint not registered\n"); + return -ENOENT; + } + + backend = ept->instance->api; + + if (!backend || !backend->send_critical) { + LOG_ERR("Invalid backend configuration"); + return -EIO; + } + + return backend->send_critical(ept->instance, ept->token, data, len); +} + int ipc_service_get_tx_buffer_size(struct ipc_ept *ept) { const struct ipc_service_backend *backend; From f165f05e69d3059913ca43bcc54ca0719fcd280a Mon Sep 17 00:00:00 2001 From: Tomasz Leman Date: Fri, 5 Dec 2025 10:49:26 +0100 Subject: [PATCH 2/4] ipc: intel_adsp: simplify host IPC service backend Rework the Intel Audio DSP host IPC service backend to use the generic ipc_service data/len conventions and the new critical send and buffer management APIs. Replace the backend-specific intel_adsp_ipc_msg and related enums with a simple two-word payload passed through the standard ipc_service_send() and ipc_service_send_critical() interfaces, and adapt the ISR and receive callback to operate on this representation. Use ipc_service_get_tx_buffer_size() as a readiness check for the host channel and provide hold_rx_buffer() / release_rx_buffer() implementations so ipc_service_release_rx_buffer() can be used to signal when the channel becomes available again. Wire the emergency send path through send_critical() to route urgent messages via the backend's emergency fast path while keeping the normal IPC flow unchanged. Move the synchronous IPC wait logic out of the Intel Audio DSP host IPC backend and into the common intel_adsp_ipc helper used by tests. Update intel_adsp_ipc_send_message_sync() to both send the IPC message through ipc_service_send() and wait on the backend semaphore, and remove the now redundant ipc_send_message_sync() helper from the host IPC backend. Document that intel_adsp_ipc_send_message_sync() is a test-only helper, not used by SOF firmware, and that it is a candidate for future removal or for replacement by an explicit synchronous send primitive in the generic IPC service API or in application code. Signed-off-by: Tomasz Leman --- .../zephyr/ipc/backends/intel_adsp_host_ipc.h | 58 +------ soc/intel/intel_adsp/common/ipc.c | 91 +++++------ .../backends/ipc_intel_adsp_host_ipc.c | 150 ++++++++++-------- 3 files changed, 125 insertions(+), 174 deletions(-) diff --git a/include/zephyr/ipc/backends/intel_adsp_host_ipc.h b/include/zephyr/ipc/backends/intel_adsp_host_ipc.h index 74938e53c86f0..7929b82900839 100644 --- a/include/zephyr/ipc/backends/intel_adsp_host_ipc.h +++ b/include/zephyr/ipc/backends/intel_adsp_host_ipc.h @@ -14,54 +14,6 @@ #include -/** Enum on IPC send length argument to indicate IPC message type. */ -enum intel_adsp_send_len { - /** Normal IPC message. */ - INTEL_ADSP_IPC_SEND_MSG, - - /** Synchronous IPC message. */ - INTEL_ADSP_IPC_SEND_MSG_SYNC, - - /** Emergency IPC message. */ - INTEL_ADSP_IPC_SEND_MSG_EMERGENCY, - - /** Send a DONE message. */ - INTEL_ADSP_IPC_SEND_DONE, - - /** Query backend to see if IPC is complete. */ - INTEL_ADSP_IPC_SEND_IS_COMPLETE, -}; - -/** Enum on callback return values. */ -enum intel_adsp_cb_ret { - /** Callback return to indicate no issue. Must be 0. */ - INTEL_ADSP_IPC_CB_RET_OKAY = 0, - - /** Callback return to signal needing external completion. */ - INTEL_ADSP_IPC_CB_RET_EXT_COMPLETE, -}; - -/** Enum on callback length argument to indicate which triggers the callback. */ -enum intel_adsp_cb_len { - /** Callback length to indicate this is an IPC message. */ - INTEL_ADSP_IPC_CB_MSG, - - /** Callback length to indicate this is a DONE message. */ - INTEL_ADSP_IPC_CB_DONE, -}; - -/** Struct for IPC message descriptor. */ -struct intel_adsp_ipc_msg { - /** Header specific to platform. */ - uint32_t data; - - /** Extension specific to platform. */ - uint32_t ext_data; - - /** Timeout for sending synchronuous message. */ - k_timeout_t timeout; -}; - #ifdef CONFIG_INTEL_ADSP_IPC_OLD_INTERFACE /** @@ -89,6 +41,7 @@ struct intel_adsp_ipc_msg { */ typedef bool (*intel_adsp_ipc_handler_t)(const struct device *dev, void *arg, uint32_t data, uint32_t ext_data); +#endif /* CONFIG_INTEL_ADSP_IPC_OLD_INTERFACE */ /** * @brief Intel ADSP IPC Message Complete Callback. @@ -114,8 +67,6 @@ typedef bool (*intel_adsp_ipc_handler_t)(const struct device *dev, void *arg, ui */ typedef bool (*intel_adsp_ipc_done_t)(const struct device *dev, void *arg); -#endif /* CONFIG_INTEL_ADSP_IPC_OLD_INTERFACE */ - #ifdef CONFIG_PM_DEVICE typedef int (*intel_adsp_ipc_resume_handler_t)(const struct device *dev, void *arg); @@ -152,13 +103,13 @@ struct intel_adsp_ipc_data { /** Argument for message handler callback. */ void *handler_arg; +#endif /* CONFIG_INTEL_ADSP_IPC_OLD_INTERFACE */ /** Callback for done notification. */ intel_adsp_ipc_done_t done_notify; /** Argument for done notification callback. */ void *done_arg; -#endif /* CONFIG_INTEL_ADSP_IPC_OLD_INTERFACE */ #ifdef CONFIG_PM_DEVICE /** Pointer to resume handler. */ @@ -179,9 +130,8 @@ struct intel_adsp_ipc_data { * Endpoint private data struct. */ struct intel_adsp_ipc_ept_priv_data { - /** Callback return value (enum intel_adsp_cb_ret). */ - int cb_ret; - + /* Message done flag. */ + bool msg_done; /** Pointer to additional private data. */ void *priv; }; diff --git a/soc/intel/intel_adsp/common/ipc.c b/soc/intel/intel_adsp/common/ipc.c index aff2c2f70ee62..981bbbecb6cf4 100644 --- a/soc/intel/intel_adsp/common/ipc.c +++ b/soc/intel/intel_adsp/common/ipc.c @@ -43,94 +43,75 @@ static void intel_adsp_ipc_receive_cb(const void *data, size_t len, void *priv) struct intel_adsp_ipc_data *devdata = dev->data; struct intel_adsp_ipc_ept_priv_data *priv_data = (struct intel_adsp_ipc_ept_priv_data *)priv; - bool done = true; - - if (len == INTEL_ADSP_IPC_CB_MSG) { - const struct intel_adsp_ipc_msg *msg = (const struct intel_adsp_ipc_msg *)data; - - if (devdata->handle_message != NULL) { - done = devdata->handle_message(dev, devdata->handler_arg, msg->data, - msg->ext_data); - } - - if (done) { - priv_data->cb_ret = INTEL_ADSP_IPC_CB_RET_OKAY; - } else { - priv_data->cb_ret = -EBADMSG; - } - } else if (len == INTEL_ADSP_IPC_CB_DONE) { - bool external_completion = false; - - if (devdata->done_notify != NULL) { - external_completion = devdata->done_notify(dev, devdata->done_arg); - } - - if (external_completion) { - priv_data->cb_ret = INTEL_ADSP_IPC_CB_RET_EXT_COMPLETE; - } else { - priv_data->cb_ret = INTEL_ADSP_IPC_CB_RET_OKAY; - } + + __ASSERT(len == sizeof(uint32_t) * 2, "Unexpected IPC payload length: %zu", len); + __ASSERT(data != NULL, "IPC payload pointer is NULL"); + + const uint32_t *msg = (const uint32_t *)data; + + if (devdata->handle_message != NULL) { + priv_data->msg_done = devdata->handle_message(dev, devdata->handler_arg, msg[0], + msg[1]); } } void intel_adsp_ipc_complete(const struct device *dev) { - int ret; - - ret = ipc_service_send(&intel_adsp_ipc_ept, NULL, INTEL_ADSP_IPC_SEND_DONE); + ARG_UNUSED(dev); + int ret = ipc_service_release_rx_buffer(&intel_adsp_ipc_ept, NULL); ARG_UNUSED(ret); + __ASSERT(ret == 0, "ipc_service_release_rx_buffer() failed: %d", ret); } bool intel_adsp_ipc_is_complete(const struct device *dev) { - int ret; - - ret = ipc_service_send(&intel_adsp_ipc_ept, NULL, INTEL_ADSP_IPC_SEND_IS_COMPLETE); - - return ret == 0; + ARG_UNUSED(dev); + return ipc_service_get_tx_buffer_size(&intel_adsp_ipc_ept) > 0; } int intel_adsp_ipc_send_message(const struct device *dev, uint32_t data, uint32_t ext_data) { - struct intel_adsp_ipc_msg msg = {.data = data, .ext_data = ext_data}; - int ret; - - ret = ipc_service_send(&intel_adsp_ipc_ept, &msg, INTEL_ADSP_IPC_SEND_MSG); + ARG_UNUSED(dev); + uint32_t msg[2] = {data, ext_data}; - if (ret < 0) { - return ret; - } - - return 0; + return ipc_service_send(&intel_adsp_ipc_ept, &msg, sizeof(msg)); } +/* + * This helper sends an IPC message and then waits synchronously for completion using the backend + * semaphore. It is currently only used by tests, SOF firmware does not rely on it. + * + * The long‑term plan is to either: + * - remove this helper entirely, + * - move the synchronous wait logic to the application layer (SOF), or + * - extend the generic IPC service API with an explicit synchronous send primitive. + * + * Until that decision is made, the function is kept here only to support existing test code and + * should not be used by new callers. + */ 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_msg msg = { - .data = data, - .ext_data = ext_data, - .timeout = timeout, - }; - int ret; + uint32_t msg[2] = {data, ext_data}; + struct intel_adsp_ipc_data *devdata = dev->data; - ret = ipc_service_send(&intel_adsp_ipc_ept, &msg, INTEL_ADSP_IPC_SEND_MSG_SYNC); + int ret = ipc_service_send(&intel_adsp_ipc_ept, &msg, sizeof(msg)); if (ret < 0) { - return ret; + k_sem_take(&devdata->sem, timeout); } - return 0; + return ret; } void intel_adsp_ipc_send_message_emergency(const struct device *dev, uint32_t data, uint32_t ext_data) { - struct intel_adsp_ipc_msg msg = {.data = data, .ext_data = ext_data}; int ret; + uint32_t msg[2] = {data, ext_data}; - ret = ipc_service_send(&intel_adsp_ipc_ept, &msg, INTEL_ADSP_IPC_SEND_MSG_EMERGENCY); + ret = ipc_service_send_critical(&intel_adsp_ipc_ept, &msg, sizeof(msg)); ARG_UNUSED(ret); } diff --git a/subsys/ipc/ipc_service/backends/ipc_intel_adsp_host_ipc.c b/subsys/ipc/ipc_service/backends/ipc_intel_adsp_host_ipc.c index 99996c84c0c7f..002ac84af0e43 100644 --- a/subsys/ipc/ipc_service/backends/ipc_intel_adsp_host_ipc.c +++ b/subsys/ipc/ipc_service/backends/ipc_intel_adsp_host_ipc.c @@ -68,45 +68,32 @@ static void intel_adsp_ipc_isr(const void *devarg) k_spinlock_key_t key = k_spin_lock(&devdata->lock); if (regs->tdr & INTEL_ADSP_IPC_BUSY) { - bool done = true; - if (ept_cfg->cb.received != NULL) { - struct intel_adsp_ipc_msg cb_msg = { - .data = regs->tdr & ~INTEL_ADSP_IPC_BUSY, - .ext_data = regs->tdd, - }; - - ept_cfg->cb.received(&cb_msg, INTEL_ADSP_IPC_CB_MSG, ept_cfg->priv); + uint32_t msg[2] = {(regs->tdr & ~INTEL_ADSP_IPC_BUSY), regs->tdd}; - done = (priv_data->cb_ret == INTEL_ADSP_IPC_CB_RET_OKAY); + ept_cfg->cb.received(&msg, sizeof(msg), ept_cfg->priv); } regs->tdr = INTEL_ADSP_IPC_BUSY; - if (done) { + if (priv_data->msg_done) { #ifdef CONFIG_SOC_SERIES_INTEL_ADSP_ACE regs->tda = INTEL_ADSP_IPC_ACE1X_TDA_DONE; #else regs->tda = INTEL_ADSP_IPC_DONE; #endif + priv_data->msg_done = false; } } - /* Same signal, but on different bits in 1.5 */ - bool done = (regs->ida & INTEL_ADSP_IPC_DONE); - - if (done) { + /* Same signal, but on different bits in ACE */ + if (regs->ida & INTEL_ADSP_IPC_DONE) { bool external_completion = false; - if (ept_cfg->cb.received != NULL) { - ept_cfg->cb.received(NULL, INTEL_ADSP_IPC_CB_DONE, ept_cfg->priv); - - if (priv_data->cb_ret == INTEL_ADSP_IPC_CB_RET_EXT_COMPLETE) { - external_completion = true; - } + if (devdata->done_notify != NULL) { + external_completion = devdata->done_notify(dev, devdata->done_arg); } devdata->tx_ack_pending = false; - /* * Allow the system to enter the runtime idle state after the IPC acknowledgment * is received. @@ -114,7 +101,7 @@ static void intel_adsp_ipc_isr(const void *devarg) pm_policy_state_lock_put(PM_STATE_RUNTIME_IDLE, PM_ALL_SUBSTATES); k_sem_give(&devdata->sem); - /* IPC completion registers will be set externally. */ + /* IPC completion registers will be set externally */ if (external_completion) { k_spin_unlock(&devdata->lock, key); return; @@ -233,20 +220,6 @@ static int ipc_send_message(const struct device *dev, uint32_t data, uint32_t ex return 0; } -static int 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; - - int ret = ipc_send_message(dev, data, ext_data); - - if (ret == 0) { - k_sem_take(&devdata->sem, timeout); - } - - return ret; -} - static int ipc_send_message_emergency(const struct device *dev, uint32_t data, uint32_t ext_data) { const struct intel_adsp_ipc_config *const config = dev->config; @@ -301,51 +274,94 @@ static int ipc_send_message_emergency(const struct device *dev, uint32_t data, u */ static int intel_adsp_ipc_send(const struct device *dev, void *token, const void *data, size_t len) { - int ret; + ARG_UNUSED(token); - const struct intel_adsp_ipc_msg *msg = (const struct intel_adsp_ipc_msg *)data; - - switch (len) { - case INTEL_ADSP_IPC_SEND_MSG: { - ret = ipc_send_message(dev, msg->data, msg->ext_data); + if (dev == NULL) { + return -EINVAL; + } - break; + if (len != sizeof(uint32_t) * 2 || data == NULL) { + return -EBADMSG; } - case INTEL_ADSP_IPC_SEND_MSG_SYNC: { - ret = ipc_send_message_sync(dev, msg->data, msg->ext_data, msg->timeout); - break; + const uint32_t *msg = (const uint32_t *)data; + + return ipc_send_message(dev, msg[0], msg[1]); +} + +/* + * Report the availability of the host IPC channel. + * + * This backend uses the TX buffer size query as a way to check whether the host is ready to + * receive the next message. When the IPC channel is idle (no BUSY bit set and no pending TX + * acknowledgment), a single “buffer” of two 32-bit words is considered available and the + * function returns sizeof(uint32_t) * 2. When the channel is still busy, no buffer is + * available and the function returns 0. + * + * Return values: + * - -EINVAL if @p instance is NULL. + * - sizeof(uint32_t) * 2 when the channel is ready for a new message. + * - 0 when the previous message has not yet been fully processed. + */ +int intel_adsp_ipc_get_tx_buffer_size(const struct device *instance, void *token) +{ + ARG_UNUSED(token); + + if (instance == NULL) { + return -EINVAL; } - case INTEL_ADSP_IPC_SEND_MSG_EMERGENCY: { - ret = ipc_send_message_emergency(dev, msg->data, msg->ext_data); - break; + if (ipc_is_complete(instance)) { + return sizeof(uint32_t) * 2; } - case INTEL_ADSP_IPC_SEND_DONE: { - ipc_complete(dev); - ret = 0; + return 0; +} - break; +/* + * This backend does not need to explicitly hold RX buffers because the IPC channel is effectively + * held from the moment the message is received in the interrupt handler until the firmware + * completes the handling. However, we must still provide a hold_rx_buffer() implementation so that + * ipc_service_release_rx_buffer() can check both hold and release callbacks and allow the use of + * ipc_service_release_rx_buffer() to notify the host that the channel is available again. + */ +int intel_adsp_ipc_hold_rx_buffer(const struct device *instance, void *token, void *data) +{ + ARG_UNUSED(instance); + ARG_UNUSED(token); + ARG_UNUSED(data); + return -ENOTSUP; +} + +int intel_adsp_ipc_release_rx_buffer(const struct device *instance, void *token, void *data) +{ + ARG_UNUSED(token); + ARG_UNUSED(data); + + if (instance == NULL) { + return -EINVAL; } - case INTEL_ADSP_IPC_SEND_IS_COMPLETE: { - bool completed = ipc_is_complete(dev); - if (completed) { - ret = 0; - } else { - ret = -EAGAIN; - } + ipc_complete(instance); + return 0; +} - break; +static int intel_adsp_ipc_send_critical(const struct device *dev, void *token, + const void *data, size_t len) +{ + ARG_UNUSED(token); + + if (dev == NULL) { + return -EINVAL; } - default: - ret = -EBADMSG; - break; + if (len != sizeof(uint32_t) * 2 || data == NULL) { + return -EBADMSG; } - return ret; + const uint32_t *msg = (const uint32_t *)data; + + return ipc_send_message_emergency(dev, msg[0], msg[1]); } static int intel_adsp_ipc_dt_init(const struct device *dev) @@ -461,6 +477,10 @@ const static struct ipc_service_backend intel_adsp_ipc_backend_api = { .send = intel_adsp_ipc_send, .register_endpoint = intel_adsp_ipc_register_ept, .deregister_endpoint = intel_adsp_ipc_deregister_ept, + .get_tx_buffer_size = intel_adsp_ipc_get_tx_buffer_size, + .hold_rx_buffer = intel_adsp_ipc_hold_rx_buffer, + .release_rx_buffer = intel_adsp_ipc_release_rx_buffer, + .send_critical = intel_adsp_ipc_send_critical, }; DEVICE_DT_DEFINE(INTEL_ADSP_IPC_HOST_DTNODE, intel_adsp_ipc_dt_init, From 4972a49636fc31b7dd474c75c3b9dcec30ae5131 Mon Sep 17 00:00:00 2001 From: Tomasz Leman Date: Mon, 8 Dec 2025 12:03:21 +0100 Subject: [PATCH 3/4] intel_adsp: document host IPC backend data model and send hook This patch refreshes the documentation in the Intel Audio DSP host IPC service backend to match the current data model and backend API contracts. The updated comments clarify that ipc_ept_cfg::priv must point to intel_adsp_ipc_ept_priv_data so that the backend can carry state between the ISR and the application callbacks. They also describe that both the send and receive paths operate on a fixed two-word uint32_t IPC payload (header and extended payload) rather than a generic byte buffer, and that len must always be sizeof(uint32_t) * 2. The documentation for intel_adsp_ipc_send() is rewritten to explain the expected payload format, the length and NULL checks performed by the backend, and how the function programs the IPC registers while relying on ipc_send_message() for BUSY and TX acknowledgment handling. The parameter semantics and error returns are aligned with the generic ipc_service send() hook contract to make the backend behaviour easier to understand and reuse. Signed-off-by: Tomasz Leman --- .../backends/ipc_intel_adsp_host_ipc.c | 48 +++++++++---------- 1 file changed, 22 insertions(+), 26 deletions(-) diff --git a/subsys/ipc/ipc_service/backends/ipc_intel_adsp_host_ipc.c b/subsys/ipc/ipc_service/backends/ipc_intel_adsp_host_ipc.c index 002ac84af0e43..3a8938893db42 100644 --- a/subsys/ipc/ipc_service/backends/ipc_intel_adsp_host_ipc.c +++ b/subsys/ipc/ipc_service/backends/ipc_intel_adsp_host_ipc.c @@ -9,16 +9,14 @@ * * @brief IPC service backend for Intel Audio DSP host IPC * - * @note When declaring struct ipt_ept_cfg, the field priv must point to - * a struct intel_adsp_ipc_ept_priv_data. This is used for passing - * callback returns. + * @note When declaring struct ipc_ept_cfg, the field @p priv must point to a struct + * intel_adsp_ipc_ept_priv_data. This is used to pass backend private state between the ISR + * and the application callbacks. * - * @note For sending message and the received callback, the data and len - * arguments are not used to represent a byte array. Instead, - * the data argument points to the descriptor of data to be sent - * (of struct intel_adsp_ipc_msg). The len argument represents - * the type of message to be sent (enum intel_adsp_send_len) and - * the type of callback (enum intel_adsp_cb_len). + * @note For sending messages and in the receive callback, the @p data and @p len arguments + * represent a fixed two-word IPC payload rather than a generic byte buffer. The @p data + * pointer must reference an array of two uint32_t values (header and extended payload) and + * @p len must be sizeof(uint32_t) * 2. */ #define DT_DRV_COMPAT intel_adsp_host_ipc @@ -250,27 +248,25 @@ static int ipc_send_message_emergency(const struct device *dev, uint32_t data, u /** * @brief Send an IPC message. * - * This implements the inner working of ipc_service_send(). + * This implements the backend send() hook used by ipc_service_send() for the Intel Audio DSP host + * IPC. * - * @note Arguments @a data and @a len are not used to point to a byte buffer of data - * to be sent. Instead, @a data must point to a descriptor of data to be sent, - * struct intel_adsp_ipc_msg. And @a len indicates what type of message to send - * as described in enum intel_adsp_send_len. + * The @a data argument is expected to point to an array of two 32-bit words, where the first word + * is the IPC header and the second word is the extended payload. The @a len argument must be + * exactly sizeof(uint32_t) * 2 or the call is rejected. * - * Return values for various message types: - * - For INTEL_ADSP_IPC_SEND_MSG_*, returns 0 when message is sent. Negative errno if - * errors. - * - For INTEL_ADSP_IPC_SEND_DONE, always returns 0 for sending DONE message. - * - For INTEL_ADSP_IPC_SEND_IS_COMPLETE, returns 0 if host has processed the message. - * -EAGAIN if not. + * On success the function programs the IPC registers and starts transmission towards the host, + * enforcing the normal BUSY and TX acknowledgment checks performed by ipc_send_message(). * - * @param[in] dev Pointer to device struct. - * @param[in] token Backend-specific token. - * @param[in] data Descriptor of IPC message to be sent (as struct intel_adsp_ipc_msg). - * @param[in] len Type of message to be sent (described in enum intel_adsp_send_len). + * @param[in] dev Pointer to device struct. + * @param[in] token Backend-specific token (unused). + * @param[in] data Pointer to two-word IPC payload. + * @param[in] len Size of the payload in bytes (must be 8). * - * @return 0 if message is sent successfully or query returns okay. - * Negative errno otherwise. + * @return 0 on success, negative errno on failure. + * -EINVAL if @p dev is NULL. + * -EBADMSG if @p data is NULL or @p len is invalid. + * Propagates error codes from ipc_send_message(). */ static int intel_adsp_ipc_send(const struct device *dev, void *token, const void *data, size_t len) { From fc7ccd4de7462843f8477d39f0dc7a308a4a78e6 Mon Sep 17 00:00:00 2001 From: Tomasz Leman Date: Fri, 12 Dec 2025 13:12:30 +0100 Subject: [PATCH 4/4] tests: intel_adsp: update smoke IPC tests for new backend This patch updates the intel_adsp smoke tests to use the new host IPC backend data model and APIs. The host IPC smoke test now treats messages as a fixed two-word uint32_t payload instead of using struct intel_adsp_ipc_msg and the INTEL_ADSP_IPC_CB_* message types. The receive callback validates the payload length and pointer, checks the header/payload values against the expected RETURN_MSG_* constants, and sets msg_flag on receipt. The test-specific intel_adsp_ipc_send_message() and intel_adsp_ipc_send_message_sync() helpers are rewritten to send a two-word payload via ipc_service_send(), and the synchronous helper waits on the backend semaphore only when the send succeeds. A dedicated done callback is registered via intel_adsp_ipc_set_done_handler() to drive done_flag, and intel_adsp_ipc_complete() now completes RX by calling ipc_service_release_rx_buffer() and asserting on failure. Completion polling is aligned with the backend by using ipc_service_get_tx_buffer_size(). The clock calibration smoke test is adapted in the same way. It now includes the ipc_service and intel_adsp host IPC backend headers when using the new interface, interprets incoming data as a two-word uint32_t payload, validates the length and pointer, and stores the returned timestamp from the extended payload word into the endpoint private data. The rest of the test flow, including endpoint registration, timestamp requests and clock rate checks, remains unchanged but runs on top of the new IPC backend representation. Signed-off-by: Tomasz Leman --- tests/boards/intel_adsp/smoke/src/clock.c | 19 +++-- tests/boards/intel_adsp/smoke/src/hostipc.c | 83 ++++++++++++--------- 2 files changed, 61 insertions(+), 41 deletions(-) diff --git a/tests/boards/intel_adsp/smoke/src/clock.c b/tests/boards/intel_adsp/smoke/src/clock.c index c7d669ccfc2fc..02d16b3a057bf 100644 --- a/tests/boards/intel_adsp/smoke/src/clock.c +++ b/tests/boards/intel_adsp/smoke/src/clock.c @@ -9,6 +9,11 @@ #include #include "tests.h" +#ifndef CONFIG_INTEL_ADSP_IPC_OLD_INTERFACE +#include +#include +#endif + #ifdef CONFIG_INTEL_ADSP_IPC_OLD_INTERFACE static volatile uint32_t old_host_dt; @@ -27,15 +32,15 @@ struct intel_adsp_ipc_ept_priv_data test_priv_data; void clock_ipc_receive_cb(const void *data, size_t len, void *priv) { - if (len == INTEL_ADSP_IPC_CB_MSG) { - const struct intel_adsp_ipc_msg *msg = (const struct intel_adsp_ipc_msg *)data; - struct intel_adsp_ipc_ept_priv_data *tpd = - (struct intel_adsp_ipc_ept_priv_data *)priv; + struct intel_adsp_ipc_ept_priv_data *tpd = + (struct intel_adsp_ipc_ept_priv_data *)priv; + const uint32_t *msg = (const uint32_t *)data; - tpd->priv = (void *)msg->data; + zassert_equal(len, sizeof(uint32_t) * 2, "unexpected IPC message length"); + zassert_not_null(data, "IPC payload pointer is NULL"); - tpd->cb_ret = INTEL_ADSP_IPC_CB_RET_OKAY; - } + /* Store returned timestamp from the extended payload word. */ + tpd->priv = (void *)(uintptr_t)msg[1]; } struct ipc_ept_cfg clock_ipc_ept_cfg = { diff --git a/tests/boards/intel_adsp/smoke/src/hostipc.c b/tests/boards/intel_adsp/smoke/src/hostipc.c index 955cd21a4a223..48e4982294b5b 100644 --- a/tests/boards/intel_adsp/smoke/src/hostipc.c +++ b/tests/boards/intel_adsp/smoke/src/hostipc.c @@ -38,6 +38,8 @@ static bool ipc_done(const struct device *dev, void *arg) #include #include +typedef bool (*intel_adsp_ipc_done_t)(const struct device *dev, void *arg); + struct ipc_ept host_ipc_ept; void ipc_receive_cb(const void *data, size_t len, void *priv) @@ -45,27 +47,17 @@ void ipc_receive_cb(const void *data, size_t len, void *priv) struct intel_adsp_ipc_ept_priv_data *priv_data = (struct intel_adsp_ipc_ept_priv_data *)priv; - if (len == INTEL_ADSP_IPC_CB_MSG) { - const struct intel_adsp_ipc_msg *msg = (const struct intel_adsp_ipc_msg *)data; - - zassert_equal(msg->data, msg->ext_data, "unequal message data/ext_data"); - zassert_true(msg->data == RETURN_MSG_SYNC_VAL || - msg->data == RETURN_MSG_ASYNC_VAL, "unexpected msg data"); + const uint32_t *msg = (const uint32_t *)data; - msg_flag = true; + ARG_UNUSED(priv_data); - if (msg->data == RETURN_MSG_SYNC_VAL) { - priv_data->cb_ret = INTEL_ADSP_IPC_CB_RET_OKAY; - } else { - priv_data->cb_ret = -EINVAL; - } - } else if (len == INTEL_ADSP_IPC_CB_DONE) { - zassert_false(done_flag, "done called unexpectedly"); + zassert_equal(len, sizeof(uint32_t) * 2, "unexpected IPC message length"); + zassert_not_null(data, "IPC payload pointer is NULL"); - done_flag = true; - - priv_data->cb_ret = INTEL_ADSP_IPC_CB_RET_OKAY; - } + zassert_equal(msg[0], msg[1], "unequal message data/ext_data"); + zassert_true(msg[0] == RETURN_MSG_SYNC_VAL || + msg[0] == RETURN_MSG_ASYNC_VAL, "unexpected msg data"); + msg_flag = true; } static struct intel_adsp_ipc_ept_priv_data host_ipc_priv_data; @@ -78,41 +70,63 @@ struct ipc_ept_cfg host_ipc_ept_cfg = { .priv = &host_ipc_priv_data, }; -static void intel_adsp_ipc_complete(const struct device *dev) +static void intel_adsp_ipc_set_done_handler(const struct device *dev, + intel_adsp_ipc_done_t fn, + void *arg) { - int ret; + struct intel_adsp_ipc_data *devdata = dev->data; + k_spinlock_key_t key = k_spin_lock(&devdata->lock); - ret = ipc_service_send(&host_ipc_ept, NULL, INTEL_ADSP_IPC_SEND_DONE); + devdata->done_notify = fn; + devdata->done_arg = arg; + k_spin_unlock(&devdata->lock, key); +} + +static bool host_ipc_done(const struct device *dev, void *arg) +{ + ARG_UNUSED(dev); + + zassert_is_null(arg, "wrong done arg"); + zassert_false(done_flag, "done called unexpectedly"); + done_flag = true; - ARG_UNUSED(ret); + return false; } -static bool intel_adsp_ipc_is_complete(const struct device *dev) +static void intel_adsp_ipc_complete(const struct device *dev) { int ret; - ret = ipc_service_send(&host_ipc_ept, NULL, INTEL_ADSP_IPC_SEND_IS_COMPLETE); + ARG_UNUSED(dev); + ret = ipc_service_release_rx_buffer(&host_ipc_ept, NULL); + zassert_equal(ret, 0, "ipc_service_release_rx_buffer() failed: %d", ret); +} - return ret == 0; +static bool intel_adsp_ipc_is_complete(const struct device *dev) +{ + ARG_UNUSED(dev); + return ipc_service_get_tx_buffer_size(&host_ipc_ept) > 0; } int intel_adsp_ipc_send_message(const struct device *dev, uint32_t data, uint32_t ext_data) { - struct intel_adsp_ipc_msg msg = {.data = data, .ext_data = ext_data}; - int ret; + ARG_UNUSED(dev); + uint32_t msg[2] = {data, ext_data}; - ret = ipc_service_send(&host_ipc_ept, &msg, INTEL_ADSP_IPC_SEND_MSG); - - return ret; + return ipc_service_send(&host_ipc_ept, &msg, sizeof(msg)); } static 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_msg msg = {.data = data, .ext_data = ext_data, .timeout = timeout}; - int ret; + uint32_t msg[2] = {data, ext_data}; + struct intel_adsp_ipc_data *devdata = dev->data; - ret = ipc_service_send(&host_ipc_ept, &msg, INTEL_ADSP_IPC_SEND_MSG_SYNC); + int ret = ipc_service_send(&host_ipc_ept, &msg, sizeof(msg)); + + if (ret == 0) { + k_sem_take(&devdata->sem, timeout); + } return ret; } @@ -129,6 +143,7 @@ ZTEST(intel_adsp, test_host_ipc) ret = ipc_service_register_endpoint(INTEL_ADSP_IPC_HOST_DEV, &host_ipc_ept, &host_ipc_ept_cfg); zassert_equal(ret, 0, "cannot register IPC endpoint"); + intel_adsp_ipc_set_done_handler(INTEL_ADSP_IPC_HOST_DEV, host_ipc_done, NULL); #endif /* CONFIG_INTEL_ADSP_IPC_OLD_INTERFACE */ /* Just send a message and wait for it to complete */ @@ -191,10 +206,10 @@ ZTEST(intel_adsp, test_host_ipc) zassert_true(intel_adsp_ipc_is_complete(INTEL_ADSP_IPC_HOST_DEV), "sync message incomplete"); + intel_adsp_ipc_set_done_handler(INTEL_ADSP_IPC_HOST_DEV, NULL, NULL); #ifdef CONFIG_INTEL_ADSP_IPC_OLD_INTERFACE /* Clean up. Further tests might want to use IPC */ intel_adsp_ipc_set_message_handler(INTEL_ADSP_IPC_HOST_DEV, NULL, NULL); - intel_adsp_ipc_set_done_handler(INTEL_ADSP_IPC_HOST_DEV, NULL, NULL); #else /* CONFIG_INTEL_ADSP_IPC_OLD_INTERFACE */ ret = ipc_service_deregister_endpoint(&host_ipc_ept); zassert_equal(ret, 0, "cannot de-register IPC endpoint");