From da7922de23953cbee214e7cb33f8f51f462de8a1 Mon Sep 17 00:00:00 2001 From: Luis Ubieda Date: Thu, 15 Aug 2024 18:45:11 -0400 Subject: [PATCH 1/4] spi: mcux_lpspi: Refactor driver to extract common RTIO functionality As a step to make them common code: spi_rtio.c. Verified this refactorization builds and passes spi_loopback, both with CONFIG_SPI_RTIO enabled, as well as disabled. Tested on mimxrt1010_evk. Signed-off-by: Luis Ubieda --- drivers/spi/spi_mcux_lpspi.c | 137 +++++++++++++++++++++++++++++------ 1 file changed, 114 insertions(+), 23 deletions(-) diff --git a/drivers/spi/spi_mcux_lpspi.c b/drivers/spi/spi_mcux_lpspi.c index 4cbf21d77eb9b7..ba3eb2cd7aaec9 100644 --- a/drivers/spi/spi_mcux_lpspi.c +++ b/drivers/spi/spi_mcux_lpspi.c @@ -574,6 +574,62 @@ static int transceive_dma(const struct device *dev, } #endif +#ifdef CONFIG_SPI_RTIO + +int spi_rtio_transceive(const struct device *dev, + const struct spi_config *config, + const struct spi_buf_set *tx_bufs, + const struct spi_buf_set *rx_bufs) +{ + struct spi_mcux_data *data = dev->data; + struct spi_dt_spec *dt_spec = &data->dt_spec; + struct rtio_sqe *sqe; + struct rtio_cqe *cqe; + int err = 0; + int ret; + + dt_spec->config = *config; + + ret = spi_rtio_copy(data->r, &data->iodev, tx_bufs, rx_bufs, &sqe); + if (ret < 0) { + return ret; + } + + /** Submit request and wait */ + rtio_submit(data->r, ret); + + while (ret > 0) { + cqe = rtio_cqe_consume(data->r); + if (cqe->result < 0) { + err = cqe->result; + } + + rtio_cqe_release(data->r, cqe); + ret--; + } + + return err; +} + +static inline int transceive_rtio(const struct device *dev, + const struct spi_config *spi_cfg, + const struct spi_buf_set *tx_bufs, + const struct spi_buf_set *rx_bufs) +{ + struct spi_mcux_data *data = dev->data; + int ret; + + spi_context_lock(&data->ctx, false, NULL, NULL, spi_cfg); + + ret = spi_rtio_transceive(dev, spi_cfg, tx_bufs, rx_bufs); + + spi_context_release(&data->ctx, ret); + + return ret; +} + +#else + static int transceive(const struct device *dev, const struct spi_config *spi_cfg, const struct spi_buf_set *tx_bufs, @@ -608,12 +664,16 @@ static int transceive(const struct device *dev, return ret; } +#endif /* CONFIG_SPI_RTIO */ static int spi_mcux_transceive(const struct device *dev, const struct spi_config *spi_cfg, const struct spi_buf_set *tx_bufs, const struct spi_buf_set *rx_bufs) { +#ifdef CONFIG_SPI_RTIO + return transceive_rtio(dev, spi_cfg, tx_bufs, rx_bufs); +#else #ifdef CONFIG_SPI_MCUX_LPSPI_DMA const struct spi_mcux_data *data = dev->data; @@ -623,6 +683,7 @@ static int spi_mcux_transceive(const struct device *dev, #endif /* CONFIG_SPI_MCUX_LPSPI_DMA */ return transceive(dev, spi_cfg, tx_bufs, rx_bufs, false, NULL, NULL); +#endif /* CONFIG_SPI_RTIO */ } #ifdef CONFIG_SPI_ASYNC @@ -731,8 +792,20 @@ static inline void spi_spin_unlock(const struct device *dev, k_spinlock_key_t ke k_spin_unlock(&data->lock, key); } +static inline void spi_mcux_iodev_prepare_start(const struct device *dev) +{ + struct spi_mcux_data *data = dev->data; + struct spi_dt_spec *spi_dt_spec = data->txn_curr->sqe.iodev->data; + struct spi_config *spi_config = &spi_dt_spec->config; + int err; + + err = spi_mcux_configure(dev, spi_config); + __ASSERT(!err, "%d", err); -static void spi_mcux_iodev_next(const struct device *dev, bool completion); + spi_context_cs_control(&data->ctx, true); +} + +static bool spi_rtio_next(const struct device *dev, bool completion); static void spi_mcux_iodev_start(const struct device *dev) { @@ -773,10 +846,7 @@ static void spi_mcux_iodev_start(const struct device *dev) break; default: LOG_ERR("Invalid op code %d for submission %p\n", sqe->op, (void *)sqe); - - spi_mcux_iodev_next(dev, true); - rtio_iodev_sqe_err(txn_head, -EINVAL); - spi_mcux_iodev_complete(dev, 0); + spi_mcux_iodev_complete(dev, -EINVAL); return; } @@ -793,7 +863,7 @@ static void spi_mcux_iodev_start(const struct device *dev) } } -static void spi_mcux_iodev_next(const struct device *dev, bool completion) +static bool spi_rtio_next(const struct device *dev, bool completion) { struct spi_mcux_data *data = dev->data; @@ -801,7 +871,7 @@ static void spi_mcux_iodev_next(const struct device *dev, bool completion) if (!completion && data->txn_curr != NULL) { spi_spin_unlock(dev, key); - return; + return false; } struct mpsc_node *next = mpsc_pop(&data->io_q); @@ -818,42 +888,63 @@ static void spi_mcux_iodev_next(const struct device *dev, bool completion) spi_spin_unlock(dev, key); - if (data->txn_curr != NULL) { - struct spi_dt_spec *spi_dt_spec = data->txn_curr->sqe.iodev->data; - struct spi_config *spi_cfg = &spi_dt_spec->config; + return (data->txn_curr != NULL); +} - spi_mcux_configure(dev, spi_cfg); - spi_context_cs_control(&data->ctx, true); - spi_mcux_iodev_start(dev); +static bool spi_rtio_submit(const struct device *dev, + struct rtio_iodev_sqe *iodev_sqe) +{ + struct spi_mcux_data *data = dev->data; + + mpsc_push(&data->io_q, &iodev_sqe->q); + + return spi_rtio_next(dev, false); +} + +static bool spi_rtio_complete(const struct device *dev, int status) +{ + struct spi_mcux_data *data = dev->data; + struct rtio_iodev_sqe *txn_head = data->txn_head; + bool result; + + result = spi_rtio_next(dev, true); + + if (status < 0) { + rtio_iodev_sqe_err(txn_head, status); + } else { + rtio_iodev_sqe_ok(txn_head, status); } + + return result; } static void spi_mcux_iodev_submit(const struct device *dev, struct rtio_iodev_sqe *iodev_sqe) { - struct spi_mcux_data *data = dev->data; - - mpsc_push(&data->io_q, &iodev_sqe->q); - spi_mcux_iodev_next(dev, false); + if (spi_rtio_submit(dev, iodev_sqe)) { + spi_mcux_iodev_prepare_start(dev); + spi_mcux_iodev_start(dev); + } } static void spi_mcux_iodev_complete(const struct device *dev, int status) { struct spi_mcux_data *data = dev->data; - if (data->txn_curr->sqe.flags & RTIO_SQE_TRANSACTION) { + if (!status && data->txn_curr->sqe.flags & RTIO_SQE_TRANSACTION) { data->txn_curr = rtio_txn_next(data->txn_curr); spi_mcux_iodev_start(dev); } else { - struct rtio_iodev_sqe *txn_head = data->txn_head; - + /** De-assert CS-line to space from next transaction */ spi_context_cs_control(&data->ctx, false); - spi_mcux_iodev_next(dev, true); - rtio_iodev_sqe_ok(txn_head, status); + + if (spi_rtio_complete(dev, status)) { + spi_mcux_iodev_prepare_start(dev); + spi_mcux_iodev_start(dev); + } } } - #endif From f628a157b48b54f7ef946d4e8d3af15955d24ae2 Mon Sep 17 00:00:00 2001 From: Luis Ubieda Date: Fri, 16 Aug 2024 15:53:20 -0400 Subject: [PATCH 2/4] spi: spi_mcux_lpspi: Removed spin lock from iodev_start Does not seem to be required. This allows hiding away the spin lock APIs. Signed-off-by: Luis Ubieda --- drivers/spi/spi_mcux_lpspi.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/spi/spi_mcux_lpspi.c b/drivers/spi/spi_mcux_lpspi.c index ba3eb2cd7aaec9..2257794dbee099 100644 --- a/drivers/spi/spi_mcux_lpspi.c +++ b/drivers/spi/spi_mcux_lpspi.c @@ -852,11 +852,8 @@ static void spi_mcux_iodev_start(const struct device *dev) data->transfer_len = transfer.dataSize; - k_spinlock_key_t key = spi_spin_lock(dev); - status = LPSPI_MasterTransferNonBlocking(base, &data->handle, &transfer); - spi_spin_unlock(dev, key); if (status != kStatus_Success) { LOG_ERR("Transfer could not start"); rtio_iodev_sqe_err(txn_head, -EIO); From 37b6ad112dd5123fd8203fbc9fa63b2adece5a94 Mon Sep 17 00:00:00 2001 From: Luis Ubieda Date: Wed, 14 Aug 2024 21:42:56 -0400 Subject: [PATCH 3/4] spi: rtio: Extract common APIs into separate file Extracted common SPI RTIO operations from the spi_mcux_lpspi driver into spi_rtio, which should be common across RTIO drivers. Tested with spi_loopback with and without CONFIG_SPI_RTIO. Ran on mimxrt1010_evk. Also, verified the other SPI RTIO driver (spi_sam) is not broken by these changes (tested building for target: robokit1 with the same conditions as above). Signed-off-by: Luis Ubieda --- drivers/spi/spi_mcux_lpspi.c | 163 +++++------------------------- drivers/spi/spi_rtio.c | 131 ++++++++++++++++++++++++ include/zephyr/drivers/spi/rtio.h | 77 ++++++++++++++ 3 files changed, 236 insertions(+), 135 deletions(-) diff --git a/drivers/spi/spi_mcux_lpspi.c b/drivers/spi/spi_mcux_lpspi.c index 2257794dbee099..10b72c9ba0fe04 100644 --- a/drivers/spi/spi_mcux_lpspi.c +++ b/drivers/spi/spi_mcux_lpspi.c @@ -21,6 +21,7 @@ #include #ifdef CONFIG_SPI_RTIO #include +#include #include #endif @@ -74,13 +75,7 @@ struct spi_mcux_data { size_t transfer_len; #ifdef CONFIG_SPI_RTIO - struct rtio *r; - struct mpsc io_q; - struct rtio_iodev iodev; - struct rtio_iodev_sqe *txn_head; - struct rtio_iodev_sqe *txn_curr; - struct spi_dt_spec dt_spec; - struct k_spinlock lock; + struct spi_rtio *rtio_ctx; #endif #ifdef CONFIG_SPI_MCUX_LPSPI_DMA @@ -181,7 +176,9 @@ static void spi_mcux_master_transfer_callback(LPSPI_Type *base, struct spi_mcux_data *data = userData; #ifdef CONFIG_SPI_RTIO - if (data->txn_head != NULL) { + struct spi_rtio *rtio_ctx = data->rtio_ctx; + + if (rtio_ctx->txn_head != NULL) { spi_mcux_iodev_complete(data->dev, status); return; } @@ -576,59 +573,25 @@ static int transceive_dma(const struct device *dev, #ifdef CONFIG_SPI_RTIO -int spi_rtio_transceive(const struct device *dev, - const struct spi_config *config, - const struct spi_buf_set *tx_bufs, - const struct spi_buf_set *rx_bufs) -{ - struct spi_mcux_data *data = dev->data; - struct spi_dt_spec *dt_spec = &data->dt_spec; - struct rtio_sqe *sqe; - struct rtio_cqe *cqe; - int err = 0; - int ret; - - dt_spec->config = *config; - - ret = spi_rtio_copy(data->r, &data->iodev, tx_bufs, rx_bufs, &sqe); - if (ret < 0) { - return ret; - } - - /** Submit request and wait */ - rtio_submit(data->r, ret); - - while (ret > 0) { - cqe = rtio_cqe_consume(data->r); - if (cqe->result < 0) { - err = cqe->result; - } - - rtio_cqe_release(data->r, cqe); - ret--; - } - - return err; -} - static inline int transceive_rtio(const struct device *dev, const struct spi_config *spi_cfg, const struct spi_buf_set *tx_bufs, const struct spi_buf_set *rx_bufs) { struct spi_mcux_data *data = dev->data; + struct spi_rtio *rtio_ctx = data->rtio_ctx; int ret; spi_context_lock(&data->ctx, false, NULL, NULL, spi_cfg); - ret = spi_rtio_transceive(dev, spi_cfg, tx_bufs, rx_bufs); + ret = spi_rtio_transceive(rtio_ctx, spi_cfg, tx_bufs, rx_bufs); spi_context_release(&data->ctx, ret); return ret; } -#else +#endif /* CONFIG_SPI_RTIO */ static int transceive(const struct device *dev, const struct spi_config *spi_cfg, @@ -664,8 +627,6 @@ static int transceive(const struct device *dev, return ret; } -#endif /* CONFIG_SPI_RTIO */ - static int spi_mcux_transceive(const struct device *dev, const struct spi_config *spi_cfg, const struct spi_buf_set *tx_bufs, @@ -673,7 +634,8 @@ static int spi_mcux_transceive(const struct device *dev, { #ifdef CONFIG_SPI_RTIO return transceive_rtio(dev, spi_cfg, tx_bufs, rx_bufs); -#else +#endif /* CONFIG_SPI_RTIO */ + #ifdef CONFIG_SPI_MCUX_LPSPI_DMA const struct spi_mcux_data *data = dev->data; @@ -683,7 +645,6 @@ static int spi_mcux_transceive(const struct device *dev, #endif /* CONFIG_SPI_MCUX_LPSPI_DMA */ return transceive(dev, spi_cfg, tx_bufs, rx_bufs, false, NULL, NULL); -#endif /* CONFIG_SPI_RTIO */ } #ifdef CONFIG_SPI_ASYNC @@ -761,10 +722,7 @@ static int spi_mcux_init(const struct device *dev) #endif /* CONFIG_SPI_MCUX_LPSPI_DMA */ #ifdef CONFIG_SPI_RTIO - data->dt_spec.bus = dev; - data->iodev.api = &spi_iodev_api; - data->iodev.data = &data->dt_spec; - mpsc_init(&data->io_q); + spi_rtio_init(data->rtio_ctx, dev); #endif err = pinctrl_apply_state(config->pincfg, PINCTRL_STATE_DEFAULT); @@ -778,24 +736,12 @@ static int spi_mcux_init(const struct device *dev) } #ifdef CONFIG_SPI_RTIO -static inline k_spinlock_key_t spi_spin_lock(const struct device *dev) -{ - struct spi_mcux_data *data = dev->data; - - return k_spin_lock(&data->lock); -} - -static inline void spi_spin_unlock(const struct device *dev, k_spinlock_key_t key) -{ - struct spi_mcux_data *data = dev->data; - - k_spin_unlock(&data->lock, key); -} static inline void spi_mcux_iodev_prepare_start(const struct device *dev) { struct spi_mcux_data *data = dev->data; - struct spi_dt_spec *spi_dt_spec = data->txn_curr->sqe.iodev->data; + struct spi_rtio *rtio_ctx = data->rtio_ctx; + struct spi_dt_spec *spi_dt_spec = rtio_ctx->txn_curr->sqe.iodev->data; struct spi_config *spi_config = &spi_dt_spec->config; int err; @@ -805,16 +751,14 @@ static inline void spi_mcux_iodev_prepare_start(const struct device *dev) spi_context_cs_control(&data->ctx, true); } -static bool spi_rtio_next(const struct device *dev, bool completion); static void spi_mcux_iodev_start(const struct device *dev) { - /* const struct spi_mcux_config *config = dev->config; */ struct spi_mcux_data *data = dev->data; - struct rtio_sqe *sqe = &data->txn_curr->sqe; + struct spi_rtio *rtio_ctx = data->rtio_ctx; + struct rtio_sqe *sqe = &rtio_ctx->txn_curr->sqe; struct spi_dt_spec *spi_dt_spec = sqe->iodev->data; struct spi_config *spi_cfg = &spi_dt_spec->config; - struct rtio_iodev_sqe *txn_head = data->txn_head; LPSPI_Type *base = (LPSPI_Type *)DEVICE_MMIO_NAMED_GET(dev, reg_base); lpspi_transfer_t transfer; @@ -856,69 +800,17 @@ static void spi_mcux_iodev_start(const struct device *dev) &transfer); if (status != kStatus_Success) { LOG_ERR("Transfer could not start"); - rtio_iodev_sqe_err(txn_head, -EIO); + spi_mcux_iodev_complete(dev, -EIO); } } -static bool spi_rtio_next(const struct device *dev, bool completion) -{ - struct spi_mcux_data *data = dev->data; - - k_spinlock_key_t key = spi_spin_lock(dev); - - if (!completion && data->txn_curr != NULL) { - spi_spin_unlock(dev, key); - return false; - } - - struct mpsc_node *next = mpsc_pop(&data->io_q); - - if (next != NULL) { - struct rtio_iodev_sqe *next_sqe = CONTAINER_OF(next, struct rtio_iodev_sqe, q); - - data->txn_head = next_sqe; - data->txn_curr = next_sqe; - } else { - data->txn_head = NULL; - data->txn_curr = NULL; - } - - spi_spin_unlock(dev, key); - - return (data->txn_curr != NULL); -} - -static bool spi_rtio_submit(const struct device *dev, - struct rtio_iodev_sqe *iodev_sqe) -{ - struct spi_mcux_data *data = dev->data; - - mpsc_push(&data->io_q, &iodev_sqe->q); - - return spi_rtio_next(dev, false); -} - -static bool spi_rtio_complete(const struct device *dev, int status) -{ - struct spi_mcux_data *data = dev->data; - struct rtio_iodev_sqe *txn_head = data->txn_head; - bool result; - - result = spi_rtio_next(dev, true); - - if (status < 0) { - rtio_iodev_sqe_err(txn_head, status); - } else { - rtio_iodev_sqe_ok(txn_head, status); - } - - return result; -} - static void spi_mcux_iodev_submit(const struct device *dev, struct rtio_iodev_sqe *iodev_sqe) { - if (spi_rtio_submit(dev, iodev_sqe)) { + struct spi_mcux_data *data = dev->data; + struct spi_rtio *rtio_ctx = data->rtio_ctx; + + if (spi_rtio_submit(rtio_ctx, iodev_sqe)) { spi_mcux_iodev_prepare_start(dev); spi_mcux_iodev_start(dev); } @@ -927,15 +819,16 @@ static void spi_mcux_iodev_submit(const struct device *dev, static void spi_mcux_iodev_complete(const struct device *dev, int status) { struct spi_mcux_data *data = dev->data; + struct spi_rtio *rtio_ctx = data->rtio_ctx; - if (!status && data->txn_curr->sqe.flags & RTIO_SQE_TRANSACTION) { - data->txn_curr = rtio_txn_next(data->txn_curr); + if (!status && rtio_ctx->txn_curr->sqe.flags & RTIO_SQE_TRANSACTION) { + rtio_ctx->txn_curr = rtio_txn_next(rtio_ctx->txn_curr); spi_mcux_iodev_start(dev); } else { /** De-assert CS-line to space from next transaction */ spi_context_cs_control(&data->ctx, false); - if (spi_rtio_complete(dev, status)) { + if (spi_rtio_complete(rtio_ctx, status)) { spi_mcux_iodev_prepare_start(dev); spi_mcux_iodev_start(dev); } @@ -956,9 +849,9 @@ static const struct spi_driver_api spi_mcux_driver_api = { .release = spi_mcux_release, }; - -#define SPI_MCUX_RTIO_DEFINE(n) RTIO_DEFINE(spi_mcux_rtio_##n, CONFIG_SPI_MCUX_RTIO_SQ_SIZE, \ - CONFIG_SPI_MCUX_RTIO_SQ_SIZE) +#define SPI_MCUX_RTIO_DEFINE(n) SPI_RTIO_DEFINE(spi_mcux_rtio_##n, \ + CONFIG_SPI_MCUX_RTIO_SQ_SIZE, \ + CONFIG_SPI_MCUX_RTIO_SQ_SIZE) #ifdef CONFIG_SPI_MCUX_LPSPI_DMA #define SPI_DMA_CHANNELS(n) \ @@ -1050,7 +943,7 @@ static const struct spi_driver_api spi_mcux_driver_api = { SPI_CONTEXT_CS_GPIOS_INITIALIZE(DT_DRV_INST(n), ctx) \ SPI_DMA_CHANNELS(n) \ IF_ENABLED(CONFIG_SPI_RTIO, \ - (.r = &spi_mcux_rtio_##n,)) \ + (.rtio_ctx = &spi_mcux_rtio_##n,)) \ \ }; \ \ diff --git a/drivers/spi/spi_rtio.c b/drivers/spi/spi_rtio.c index 4d4589026a1790..2852eb9df6e09c 100644 --- a/drivers/spi/spi_rtio.c +++ b/drivers/spi/spi_rtio.c @@ -8,6 +8,8 @@ #include #include #include +#include +#include #include LOG_MODULE_DECLARE(spi_rtio, CONFIG_SPI_LOG_LEVEL); @@ -106,3 +108,132 @@ void spi_rtio_iodev_default_submit(const struct device *dev, rtio_work_req_submit(req, iodev_sqe, spi_rtio_iodev_default_submit_sync); } + +/** + * @brief Lock the SPI RTIO spinlock + * + * This is used internally for controlling the SPI RTIO context, and is + * exposed to the user as it's required for safely implementing + * iodev_start API, specific to each driver. + * + * @param ctx SPI RTIO context + * + * @retval Spinlock key + */ +static inline k_spinlock_key_t spi_spin_lock(struct spi_rtio *ctx) +{ + return k_spin_lock(&ctx->lock); +} + +/** + * @brief Unlock the previously obtained SPI RTIO spinlock + * + * @param ctx SPI RTIO context + * @param key Spinlock key + */ +static inline void spi_spin_unlock(struct spi_rtio *ctx, k_spinlock_key_t key) +{ + k_spin_unlock(&ctx->lock, key); +} + +void spi_rtio_init(struct spi_rtio *ctx, + const struct device *dev) +{ + mpsc_init(&ctx->io_q); + ctx->txn_head = NULL; + ctx->txn_curr = NULL; + ctx->dt_spec.bus = dev; + ctx->iodev.data = &ctx->dt_spec; + ctx->iodev.api = &spi_iodev_api; +} + +/** + * @private + * @brief Setup the next transaction (could be a single op) if needed + * + * @retval true New transaction to start with the hardware is setup + * @retval false No new transaction to start + */ +static bool spi_rtio_next(struct spi_rtio *ctx, bool completion) +{ + k_spinlock_key_t key = spi_spin_lock(ctx); + + if (!completion && ctx->txn_curr != NULL) { + spi_spin_unlock(ctx, key); + return false; + } + + struct mpsc_node *next = mpsc_pop(&ctx->io_q); + + if (next != NULL) { + struct rtio_iodev_sqe *next_sqe = CONTAINER_OF(next, struct rtio_iodev_sqe, q); + + ctx->txn_head = next_sqe; + ctx->txn_curr = next_sqe; + } else { + ctx->txn_head = NULL; + ctx->txn_curr = NULL; + } + + spi_spin_unlock(ctx, key); + + return (ctx->txn_curr != NULL); +} + +bool spi_rtio_complete(struct spi_rtio *ctx, int status) +{ + struct rtio_iodev_sqe *txn_head = ctx->txn_head; + bool result; + + result = spi_rtio_next(ctx, true); + + if (status < 0) { + rtio_iodev_sqe_err(txn_head, status); + } else { + rtio_iodev_sqe_ok(txn_head, status); + } + + return result; +} + +bool spi_rtio_submit(struct spi_rtio *ctx, + struct rtio_iodev_sqe *iodev_sqe) +{ + /** Done */ + mpsc_push(&ctx->io_q, &iodev_sqe->q); + return spi_rtio_next(ctx, false); +} + +int spi_rtio_transceive(struct spi_rtio *ctx, + const struct spi_config *config, + const struct spi_buf_set *tx_bufs, + const struct spi_buf_set *rx_bufs) +{ + struct spi_dt_spec *dt_spec = &ctx->dt_spec; + struct rtio_sqe *sqe; + struct rtio_cqe *cqe; + int err = 0; + int ret; + + dt_spec->config = *config; + + ret = spi_rtio_copy(ctx->r, &ctx->iodev, tx_bufs, rx_bufs, &sqe); + if (ret < 0) { + return ret; + } + + /** Submit request and wait */ + rtio_submit(ctx->r, ret); + + while (ret > 0) { + cqe = rtio_cqe_consume(ctx->r); + if (cqe->result < 0) { + err = cqe->result; + } + + rtio_cqe_release(ctx->r, cqe); + ret--; + } + + return err; +} diff --git a/include/zephyr/drivers/spi/rtio.h b/include/zephyr/drivers/spi/rtio.h index b5e2629a4924df..91726655c85e3a 100644 --- a/include/zephyr/drivers/spi/rtio.h +++ b/include/zephyr/drivers/spi/rtio.h @@ -7,6 +7,79 @@ #ifndef ZEPHYR_DRIVERS_SPI_RTIO_H_ #define ZEPHYR_DRIVERS_SPI_RTIO_H_ +#include +#include +#include + +#ifdef __cplusplus +extern "C" { +#endif + +/** + * @brief Driver context for implementing SPI with RTIO + */ +struct spi_rtio { + struct k_spinlock lock; + struct rtio *r; + struct mpsc io_q; + struct rtio_iodev iodev; + struct rtio_iodev_sqe *txn_head; + struct rtio_iodev_sqe *txn_curr; + struct spi_dt_spec dt_spec; +}; + +/** + * @brief Statically define a spi_rtio context + * + * @param _name Symbolic name of the context + * @param _sq_sz Submission queue entry pool size + * @param _cq_sz Completion queue entry pool size + */ +#define SPI_RTIO_DEFINE(_name, _sq_sz, _cq_sz) \ + RTIO_DEFINE(CONCAT(_name, _r), _sq_sz, _cq_sz); \ + static struct spi_rtio _name = { \ + .r = &CONCAT(_name, _r), \ + }; + +/** + * @brief Initialize a SPI RTIO context + * + * @param ctx SPI RTIO driver context + * @param dev SPI bus + */ +void spi_rtio_init(struct spi_rtio *ctx, const struct device *dev); + +/** + * @brief Signal that the current (ctx->txn_curr) submission has been completed + * + * @param ctx SPI RTIO driver context + * @param status Completion status, negative values are errors + * + * @retval true Next submission is ready to start + * @retval false No more submissions to work on + */ +bool spi_rtio_complete(struct spi_rtio *ctx, int status); + +/** + * @brief Submit, atomically, a submission to work on at some point + * + * @retval true Next submission is ready to start + * @retval false No new submission to start or submissions are in progress already + */ +bool spi_rtio_submit(struct spi_rtio *ctx, struct rtio_iodev_sqe *iodev_sqe); + +/** + * @brief Perform a SPI Transfer (transceive) in a blocking call + * + * Provides a compatible API for the existing spi_transceive API by blocking + * the caller until the operation is complete. + * For details see @ref spi_transceive. + */ +int spi_rtio_transceive(struct spi_rtio *ctx, + const struct spi_config *config, + const struct spi_buf_set *tx_bufs, + const struct spi_buf_set *rx_bufs); + /** * @brief Fallback SPI RTIO submit implementation. * @@ -16,4 +89,8 @@ void spi_rtio_iodev_default_submit(const struct device *dev, struct rtio_iodev_sqe *iodev_sqe); +#ifdef __cplusplus +} +#endif + #endif /* ZEPHYR_DRIVERS_SPI_RTIO_H_ */ From 48146dca408c4d86cef04c4b675dad6d6899ac1d Mon Sep 17 00:00:00 2001 From: Luis Ubieda Date: Sat, 17 Aug 2024 08:36:52 -0400 Subject: [PATCH 4/4] spi: rtio: Move spi_rtio_copy to spi_rtio To group all common APIs for SPI RTIO. Signed-off-by: Luis Ubieda --- drivers/spi/spi_rtio.c | 158 ++++++++++++++++++++++++++++++ drivers/spi/spi_sam.c | 1 + include/zephyr/drivers/spi.h | 158 ------------------------------ include/zephyr/drivers/spi/rtio.h | 18 ++++ 4 files changed, 177 insertions(+), 158 deletions(-) diff --git a/drivers/spi/spi_rtio.c b/drivers/spi/spi_rtio.c index 2852eb9df6e09c..c514b5acad63e2 100644 --- a/drivers/spi/spi_rtio.c +++ b/drivers/spi/spi_rtio.c @@ -109,6 +109,164 @@ void spi_rtio_iodev_default_submit(const struct device *dev, rtio_work_req_submit(req, iodev_sqe, spi_rtio_iodev_default_submit_sync); } +/** + * @brief Copy the tx_bufs and rx_bufs into a set of RTIO requests + * + * @param[in] r rtio context + * @param[in] iodev iodev to transceive with + * @param[in] tx_bufs transmit buffer set + * @param[in] rx_bufs receive buffer set + * @param[out] last_sqe last sqe submitted, NULL if not enough memory + * + * @retval Number of submission queue entries + * @retval -ENOMEM out of memory + */ +int spi_rtio_copy(struct rtio *r, + struct rtio_iodev *iodev, + const struct spi_buf_set *tx_bufs, + const struct spi_buf_set *rx_bufs, + struct rtio_sqe **last_sqe) +{ + int ret = 0; + size_t tx_count = tx_bufs ? tx_bufs->count : 0; + size_t rx_count = rx_bufs ? rx_bufs->count : 0; + + uint32_t tx = 0, tx_len = 0; + uint32_t rx = 0, rx_len = 0; + uint8_t *tx_buf, *rx_buf; + + struct rtio_sqe *sqe = NULL; + + if (tx < tx_count) { + tx_buf = tx_bufs->buffers[tx].buf; + tx_len = tx_bufs->buffers[tx].len; + } else { + tx_buf = NULL; + tx_len = rx_bufs->buffers[rx].len; + } + + if (rx < rx_count) { + rx_buf = rx_bufs->buffers[rx].buf; + rx_len = rx_bufs->buffers[rx].len; + } else { + rx_buf = NULL; + rx_len = tx_bufs->buffers[tx].len; + } + + + while ((tx < tx_count || rx < rx_count) && (tx_len > 0 || rx_len > 0)) { + sqe = rtio_sqe_acquire(r); + + if (sqe == NULL) { + ret = -ENOMEM; + rtio_sqe_drop_all(r); + goto out; + } + + ret++; + + /* If tx/rx len are same, we can do a simple transceive */ + if (tx_len == rx_len) { + if (tx_buf == NULL) { + rtio_sqe_prep_read(sqe, iodev, RTIO_PRIO_NORM, + rx_buf, rx_len, NULL); + } else if (rx_buf == NULL) { + rtio_sqe_prep_write(sqe, iodev, RTIO_PRIO_NORM, + tx_buf, tx_len, NULL); + } else { + rtio_sqe_prep_transceive(sqe, iodev, RTIO_PRIO_NORM, + tx_buf, rx_buf, rx_len, NULL); + } + tx++; + rx++; + if (rx < rx_count) { + rx_buf = rx_bufs->buffers[rx].buf; + rx_len = rx_bufs->buffers[rx].len; + } else { + rx_buf = NULL; + rx_len = 0; + } + if (tx < tx_count) { + tx_buf = tx_bufs->buffers[tx].buf; + tx_len = tx_bufs->buffers[tx].len; + } else { + tx_buf = NULL; + tx_len = 0; + } + } else if (tx_len == 0) { + rtio_sqe_prep_read(sqe, iodev, RTIO_PRIO_NORM, + (uint8_t *)rx_buf, + (uint32_t)rx_len, + NULL); + rx++; + if (rx < rx_count) { + rx_buf = rx_bufs->buffers[rx].buf; + rx_len = rx_bufs->buffers[rx].len; + } else { + rx_buf = NULL; + rx_len = 0; + } + } else if (rx_len == 0) { + rtio_sqe_prep_write(sqe, iodev, RTIO_PRIO_NORM, + (uint8_t *)tx_buf, + (uint32_t)tx_len, + NULL); + tx++; + if (tx < tx_count) { + tx_buf = rx_bufs->buffers[rx].buf; + tx_len = rx_bufs->buffers[rx].len; + } else { + tx_buf = NULL; + tx_len = 0; + } + } else if (tx_len > rx_len) { + rtio_sqe_prep_transceive(sqe, iodev, RTIO_PRIO_NORM, + (uint8_t *)tx_buf, + (uint8_t *)rx_buf, + (uint32_t)rx_len, + NULL); + tx_len -= rx_len; + tx_buf += rx_len; + rx++; + if (rx < rx_count) { + rx_buf = rx_bufs->buffers[rx].buf; + rx_len = rx_bufs->buffers[rx].len; + } else { + rx_buf = NULL; + rx_len = tx_len; + } + } else if (rx_len > tx_len) { + rtio_sqe_prep_transceive(sqe, iodev, RTIO_PRIO_NORM, + (uint8_t *)tx_buf, + (uint8_t *)rx_buf, + (uint32_t)tx_len, + NULL); + rx_len -= tx_len; + rx_buf += tx_len; + tx++; + if (tx < tx_count) { + tx_buf = tx_bufs->buffers[tx].buf; + tx_len = tx_bufs->buffers[tx].len; + } else { + tx_buf = NULL; + tx_len = rx_len; + } + } else { + __ASSERT(false, "Invalid %s state", __func__); + } + + sqe->flags = RTIO_SQE_TRANSACTION; + } + + if (sqe != NULL) { + sqe->flags = 0; + *last_sqe = sqe; + } + +out: + return ret; +} + /** * @brief Lock the SPI RTIO spinlock * diff --git a/drivers/spi/spi_sam.c b/drivers/spi/spi_sam.c index fc8e296194ef0c..e8fd42375ab082 100644 --- a/drivers/spi/spi_sam.c +++ b/drivers/spi/spi_sam.c @@ -17,6 +17,7 @@ LOG_MODULE_REGISTER(spi_sam); #include #include #include +#include #include #include #include diff --git a/include/zephyr/drivers/spi.h b/include/zephyr/drivers/spi.h index 56d2cd3d925382..958211e8294d03 100644 --- a/include/zephyr/drivers/spi.h +++ b/include/zephyr/drivers/spi.h @@ -1085,164 +1085,6 @@ static inline bool spi_is_ready_iodev(const struct rtio_iodev *spi_iodev) return spi_is_ready_dt(spec); } -/** - * @brief Copy the tx_bufs and rx_bufs into a set of RTIO requests - * - * @param[in] r rtio context - * @param[in] iodev iodev to transceive with - * @param[in] tx_bufs transmit buffer set - * @param[in] rx_bufs receive buffer set - * @param[out] last_sqe last sqe submitted, NULL if not enough memory - * - * @retval Number of submission queue entries - * @retval -ENOMEM out of memory - */ -static inline int spi_rtio_copy(struct rtio *r, - struct rtio_iodev *iodev, - const struct spi_buf_set *tx_bufs, - const struct spi_buf_set *rx_bufs, - struct rtio_sqe **last_sqe) -{ - int ret = 0; - size_t tx_count = tx_bufs ? tx_bufs->count : 0; - size_t rx_count = rx_bufs ? rx_bufs->count : 0; - - uint32_t tx = 0, tx_len = 0; - uint32_t rx = 0, rx_len = 0; - uint8_t *tx_buf, *rx_buf; - - struct rtio_sqe *sqe = NULL; - - if (tx < tx_count) { - tx_buf = tx_bufs->buffers[tx].buf; - tx_len = tx_bufs->buffers[tx].len; - } else { - tx_buf = NULL; - tx_len = rx_bufs->buffers[rx].len; - } - - if (rx < rx_count) { - rx_buf = rx_bufs->buffers[rx].buf; - rx_len = rx_bufs->buffers[rx].len; - } else { - rx_buf = NULL; - rx_len = tx_bufs->buffers[tx].len; - } - - - while ((tx < tx_count || rx < rx_count) && (tx_len > 0 || rx_len > 0)) { - sqe = rtio_sqe_acquire(r); - - if (sqe == NULL) { - ret = -ENOMEM; - rtio_sqe_drop_all(r); - goto out; - } - - ret++; - - /* If tx/rx len are same, we can do a simple transceive */ - if (tx_len == rx_len) { - if (tx_buf == NULL) { - rtio_sqe_prep_read(sqe, iodev, RTIO_PRIO_NORM, - rx_buf, rx_len, NULL); - } else if (rx_buf == NULL) { - rtio_sqe_prep_write(sqe, iodev, RTIO_PRIO_NORM, - tx_buf, tx_len, NULL); - } else { - rtio_sqe_prep_transceive(sqe, iodev, RTIO_PRIO_NORM, - tx_buf, rx_buf, rx_len, NULL); - } - tx++; - rx++; - if (rx < rx_count) { - rx_buf = rx_bufs->buffers[rx].buf; - rx_len = rx_bufs->buffers[rx].len; - } else { - rx_buf = NULL; - rx_len = 0; - } - if (tx < tx_count) { - tx_buf = tx_bufs->buffers[tx].buf; - tx_len = tx_bufs->buffers[tx].len; - } else { - tx_buf = NULL; - tx_len = 0; - } - } else if (tx_len == 0) { - rtio_sqe_prep_read(sqe, iodev, RTIO_PRIO_NORM, - (uint8_t *)rx_buf, - (uint32_t)rx_len, - NULL); - rx++; - if (rx < rx_count) { - rx_buf = rx_bufs->buffers[rx].buf; - rx_len = rx_bufs->buffers[rx].len; - } else { - rx_buf = NULL; - rx_len = 0; - } - } else if (rx_len == 0) { - rtio_sqe_prep_write(sqe, iodev, RTIO_PRIO_NORM, - (uint8_t *)tx_buf, - (uint32_t)tx_len, - NULL); - tx++; - if (tx < tx_count) { - tx_buf = rx_bufs->buffers[rx].buf; - tx_len = rx_bufs->buffers[rx].len; - } else { - tx_buf = NULL; - tx_len = 0; - } - } else if (tx_len > rx_len) { - rtio_sqe_prep_transceive(sqe, iodev, RTIO_PRIO_NORM, - (uint8_t *)tx_buf, - (uint8_t *)rx_buf, - (uint32_t)rx_len, - NULL); - tx_len -= rx_len; - tx_buf += rx_len; - rx++; - if (rx < rx_count) { - rx_buf = rx_bufs->buffers[rx].buf; - rx_len = rx_bufs->buffers[rx].len; - } else { - rx_buf = NULL; - rx_len = tx_len; - } - } else if (rx_len > tx_len) { - rtio_sqe_prep_transceive(sqe, iodev, RTIO_PRIO_NORM, - (uint8_t *)tx_buf, - (uint8_t *)rx_buf, - (uint32_t)tx_len, - NULL); - rx_len -= tx_len; - rx_buf += tx_len; - tx++; - if (tx < tx_count) { - tx_buf = tx_bufs->buffers[tx].buf; - tx_len = tx_bufs->buffers[tx].len; - } else { - tx_buf = NULL; - tx_len = rx_len; - } - } else { - __ASSERT_NO_MSG("Invalid spi_rtio_copy state"); - } - - sqe->flags = RTIO_SQE_TRANSACTION; - } - - if (sqe != NULL) { - sqe->flags = 0; - *last_sqe = sqe; - } - -out: - return ret; -} - #endif /* CONFIG_SPI_RTIO */ /** diff --git a/include/zephyr/drivers/spi/rtio.h b/include/zephyr/drivers/spi/rtio.h index 91726655c85e3a..04d5bad8e69119 100644 --- a/include/zephyr/drivers/spi/rtio.h +++ b/include/zephyr/drivers/spi/rtio.h @@ -41,6 +41,24 @@ struct spi_rtio { .r = &CONCAT(_name, _r), \ }; +/** + * @brief Copy the tx_bufs and rx_bufs into a set of RTIO requests + * + * @param[in] r rtio context + * @param[in] iodev iodev to transceive with + * @param[in] tx_bufs transmit buffer set + * @param[in] rx_bufs receive buffer set + * @param[out] last_sqe last sqe submitted, NULL if not enough memory + * + * @retval Number of submission queue entries + * @retval -ENOMEM out of memory + */ +int spi_rtio_copy(struct rtio *r, + struct rtio_iodev *iodev, + const struct spi_buf_set *tx_bufs, + const struct spi_buf_set *rx_bufs, + struct rtio_sqe **last_sqe); + /** * @brief Initialize a SPI RTIO context *