From 37b6ad112dd5123fd8203fbc9fa63b2adece5a94 Mon Sep 17 00:00:00 2001 From: Luis Ubieda Date: Wed, 14 Aug 2024 21:42:56 -0400 Subject: [PATCH] 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_ */