From d7a40ba5d7f6d63c4c5f5400a448403b6d3af170 Mon Sep 17 00:00:00 2001 From: Carlo Caione Date: Mon, 15 Nov 2021 15:45:43 +0100 Subject: [PATCH] ipc_service: Extend RPMsg structs and misc fixes Extend the RPMsg structs to accommodate for the introduction of new backends and contextually fix the ipc_rpmsg_static_vrings_mi backend (the only user). Rework also some comments and ipc_service glue code. Signed-off-by: Carlo Caione --- include/ipc/ipc_rpmsg.h | 8 +++++++- include/ipc/ipc_service.h | 19 ++++++++++++++----- include/ipc/ipc_service_backend.h | 15 +++++++++++++-- include/ipc/ipc_static_vrings.h | 3 +++ .../backends/ipc_rpmsg_static_vrings_mi.c | 5 +++-- subsys/ipc/ipc_service/ipc_service.c | 5 +++++ subsys/ipc/ipc_service/lib/ipc_rpmsg.c | 10 ++++++++++ .../ipc/ipc_service/lib/ipc_static_vrings.c | 2 -- 8 files changed, 55 insertions(+), 12 deletions(-) diff --git a/include/ipc/ipc_rpmsg.h b/include/ipc/ipc_rpmsg.h index 67786732d42..2a6abaf38da 100644 --- a/include/ipc/ipc_rpmsg.h +++ b/include/ipc/ipc_rpmsg.h @@ -46,7 +46,10 @@ struct ipc_rpmsg_ept { struct rpmsg_endpoint ep; /** Name of the endpoint. */ - const char *name; + char name[RPMSG_NAME_SIZE]; + + /** Destination endpoint. */ + uint32_t dest; /** Bound flag. */ volatile bool bound; @@ -77,6 +80,9 @@ struct ipc_rpmsg_instance { /** EPT (instance) callback. */ rpmsg_ept_cb cb; + + /** Mutex for the instance. */ + struct k_mutex mtx; }; /** @brief Init an RPMsg instance. diff --git a/include/ipc/ipc_service.h b/include/ipc/ipc_service.h index 0cf631aa188..c5320668c61 100644 --- a/include/ipc/ipc_service.h +++ b/include/ipc/ipc_service.h @@ -136,15 +136,18 @@ int ipc_service_open_instance(const struct device *instance); * Registers IPC endpoint onto an instance to enable communication with a * remote device. * - * The same function registers endpoints for both master and slave devices. + * The same function registers endpoints for both host and remote devices. * * @param instance Instance to register the endpoint onto. * @param ept Endpoint object. * @param cfg Endpoint configuration. * * @retval -EIO when no backend is registered. - * @retval -EINVAL when instance or endpoint configuration is invalid. - * @retval Other errno codes depending on the implementation of the backend. + * @retval -EINVAL when instance, endpoint or configuration is invalid. + * @retval -EBUSY when the instance is busy. + * + * @retval 0 on success. + * @retval other errno codes depending on the implementation of the backend. */ int ipc_service_register_endpoint(const struct device *instance, struct ipc_ept *ept, @@ -156,8 +159,14 @@ int ipc_service_register_endpoint(const struct device *instance, * @param data Pointer to the buffer to send. * @param len Number of bytes to send. * - * @retval -EIO when no backend is registered. - * @retval Other errno codes depending on the implementation of the backend. + * @retval -EIO when no backend is registered or send hook is missing from + * backend. + * @retval -EINVAL when instance or endpoint is invalid. + * @retval -EBADMSG when the message is invalid. + * @retval -EBUSY when the instance is busy. + * + * @retval 0 on success. + * @retval other errno codes depending on the implementation of the backend. */ int ipc_service_send(struct ipc_ept *ept, const void *data, size_t len); diff --git a/include/ipc/ipc_service_backend.h b/include/ipc/ipc_service_backend.h index 53e546d49cf..a62a9864342 100644 --- a/include/ipc/ipc_service_backend.h +++ b/include/ipc/ipc_service_backend.h @@ -44,7 +44,13 @@ struct ipc_service_backend { * @param data Pointer to the buffer to send. * @param len Number of bytes to send. * - * @retval Status code. + * @retval -EINVAL when instance is invalid. + * @retval -EBADMSG when the message is invalid. + * @retval -EBUSY when the instance is busy or not ready. + * + * @retval 0 on success + * @retval other errno codes depending on the implementation of the + * backend. */ int (*send)(const struct device *instance, void *token, const void *data, size_t len); @@ -55,7 +61,12 @@ struct ipc_service_backend { * @param token Backend-specific token. * @param cfg Endpoint configuration. * - * @retval Status code. + * @retval -EINVAL when the endpoint configuration or instance is invalid. + * @retval -EBUSY when the instance is busy or not ready. + * + * @retval 0 on success + * @retval other errno codes depending on the implementation of the + * backend. */ int (*register_endpoint)(const struct device *instance, void **token, diff --git a/include/ipc/ipc_static_vrings.h b/include/ipc/ipc_static_vrings.h index 6a635eedb97..c8eb9bd07d3 100644 --- a/include/ipc/ipc_static_vrings.h +++ b/include/ipc/ipc_static_vrings.h @@ -24,6 +24,9 @@ extern "C" { /** Number of used VRING buffers. */ #define VRING_COUNT (2) +/** VRING alignment. */ +#define VRING_ALIGNMENT CONFIG_IPC_SERVICE_STATIC_VRINGS_ALIGNMENT + /** * @typedef ipc_notify_cb * @brief Define the notify callback. diff --git a/subsys/ipc/ipc_service/backends/ipc_rpmsg_static_vrings_mi.c b/subsys/ipc/ipc_service/backends/ipc_rpmsg_static_vrings_mi.c index 376230349df..3c264135ad6 100644 --- a/subsys/ipc/ipc_service/backends/ipc_rpmsg_static_vrings_mi.c +++ b/subsys/ipc/ipc_service/backends/ipc_rpmsg_static_vrings_mi.c @@ -100,7 +100,7 @@ static struct rpmsg_mi_instance *get_available_instance(const struct ipc_ept_cfg static struct ipc_rpmsg_ept *get_available_ept_slot(struct ipc_rpmsg_instance *rpmsg_instance) { for (size_t i = 0; i < NUM_ENDPOINTS; i++) { - if (rpmsg_instance->endpoint[i].name == NULL) { + if (rpmsg_instance->endpoint[i].name[0] == '\0') { return &rpmsg_instance->endpoint[i]; } } @@ -299,7 +299,8 @@ static int register_ept(const struct device *dev, return -ENODEV; } - rpmsg_ept->name = cfg->name; + strncpy(rpmsg_ept->name, cfg->name ? cfg->name : "", sizeof(rpmsg_ept->name)); + rpmsg_ept->cb = &cfg->cb; rpmsg_ept->priv = cfg->priv; rpmsg_ept->bound = false; diff --git a/subsys/ipc/ipc_service/ipc_service.c b/subsys/ipc/ipc_service/ipc_service.c index ca3efb34ac5..0334ebe347b 100644 --- a/subsys/ipc/ipc_service/ipc_service.c +++ b/subsys/ipc/ipc_service/ipc_service.c @@ -67,6 +67,11 @@ int ipc_service_send(struct ipc_ept *ept, const void *data, size_t len) { const struct ipc_service_backend *backend; + if (!ept) { + LOG_ERR("Invalid endpoint"); + return -EINVAL; + } + backend = ept->instance->api; if (!backend || !backend->send) { diff --git a/subsys/ipc/ipc_service/lib/ipc_rpmsg.c b/subsys/ipc/ipc_service/lib/ipc_rpmsg.c index 52331927c26..a0c07c15c1c 100644 --- a/subsys/ipc/ipc_service/lib/ipc_rpmsg.c +++ b/subsys/ipc/ipc_service/lib/ipc_rpmsg.c @@ -26,6 +26,12 @@ static void ns_bind_cb(struct rpmsg_device *rdev, const char *name, uint32_t des ept = &instance->endpoint[i]; if (strcmp(name, ept->name) == 0) { + /* + * The destination address is 'dest' so ns_bind_cb() is + * *NOT* called on the REMOTE side. The bound_cb() + * function will eventually take care of notifying the + * REMOTE side if needed. + */ err = rpmsg_create_ept(&ept->ep, rdev, name, RPMSG_ADDR_ANY, dest, instance->cb, rpmsg_service_unbind); if (err != 0) { @@ -52,6 +58,10 @@ int ipc_rpmsg_register_ept(struct ipc_rpmsg_instance *instance, unsigned int rol rdev = rpmsg_virtio_get_rpmsg_device(&instance->rvdev); if (role == RPMSG_REMOTE) { + /* + * The destination address is RPMSG_ADDR_ANY, this will trigger + * the ns_bind_cb() callback function on the HOST side. + */ return rpmsg_create_ept(&ept->ep, rdev, ept->name, RPMSG_ADDR_ANY, RPMSG_ADDR_ANY, instance->cb, rpmsg_service_unbind); } diff --git a/subsys/ipc/ipc_service/lib/ipc_static_vrings.c b/subsys/ipc/ipc_service/lib/ipc_static_vrings.c index a6372f5fd2a..ad9121e8851 100644 --- a/subsys/ipc/ipc_service/lib/ipc_static_vrings.c +++ b/subsys/ipc/ipc_service/lib/ipc_static_vrings.c @@ -9,8 +9,6 @@ #define SHM_DEVICE_NAME "sram0.shm" -#define VRING_ALIGNMENT (4) /* Alignment of vring buffer */ - #define RPMSG_VQ_0 (0) /* TX virtqueue queue index */ #define RPMSG_VQ_1 (1) /* RX virtqueue queue index */