From 02df780eabf3390671b69771fd7e6aade7b3f3bc Mon Sep 17 00:00:00 2001 From: Declan Snyder Date: Tue, 28 Jan 2025 15:52:47 -0600 Subject: [PATCH] spi_nxp_lpspi: Use default RTIO submit For the CPU-based drivers, delete the old MCUX based RTIO driver and use the default RTIO submit implementation instead. Rationale: - 300 LOC -> 1 LOC to maintain. - MCUX SDK based driver cannot control the chip select for the transfer properly, but the new spi_nxp_lpspi.c driver can. So this fixes the bug with the PCS when using RTIO. Also enable the default RTIO implementation for DMA based driver. In the future a DMA based RTIO driver with custom implementation can be designed, but for CPU based transfer, which is already not optimal performance, code maintenance is more important. Only requirement is asynchronous submit, which is accomplished by p4wq in rtio workq. Signed-off-by: Declan Snyder --- drivers/spi/spi_nxp_lpspi/CMakeLists.txt | 1 - drivers/spi/spi_nxp_lpspi/Kconfig | 21 -- .../spi/spi_nxp_lpspi/spi_mcux_lpspi_rtio.c | 255 ------------------ drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi.c | 3 + drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi_dma.c | 3 + .../spi/spi_nxp_lpspi/spi_nxp_lpspi_priv.h | 1 + 6 files changed, 7 insertions(+), 277 deletions(-) delete mode 100644 drivers/spi/spi_nxp_lpspi/spi_mcux_lpspi_rtio.c diff --git a/drivers/spi/spi_nxp_lpspi/CMakeLists.txt b/drivers/spi/spi_nxp_lpspi/CMakeLists.txt index 59f6c69e5db4..49d161642181 100644 --- a/drivers/spi/spi_nxp_lpspi/CMakeLists.txt +++ b/drivers/spi/spi_nxp_lpspi/CMakeLists.txt @@ -3,4 +3,3 @@ zephyr_library_sources_ifdef(CONFIG_SPI_MCUX_LPSPI spi_nxp_lpspi_common.c) zephyr_library_sources_ifdef(CONFIG_SPI_MCUX_LPSPI_NORMAL spi_nxp_lpspi.c) zephyr_library_sources_ifdef(CONFIG_SPI_MCUX_LPSPI_DMA spi_nxp_lpspi_dma.c) -zephyr_library_sources_ifdef(CONFIG_SPI_MCUX_LPSPI_RTIO spi_mcux_lpspi_rtio.c) diff --git a/drivers/spi/spi_nxp_lpspi/Kconfig b/drivers/spi/spi_nxp_lpspi/Kconfig index 0cad771bc02e..5897daf3655c 100644 --- a/drivers/spi/spi_nxp_lpspi/Kconfig +++ b/drivers/spi/spi_nxp_lpspi/Kconfig @@ -12,7 +12,6 @@ config SPI_MCUX_LPSPI if SPI_MCUX_LPSPI -if !SPI_RTIO config SPI_MCUX_LPSPI_DMA bool "MCUX LPSPI SPI DMA Support" select DMA @@ -27,25 +26,5 @@ config SPI_MCUX_LPSPI_NORMAL depends on $(dt_compat_any_not_has_prop,$(DT_COMPAT_NXP_LPSPI),dmas) || !SPI_MCUX_LPSPI_DMA help Use the traditional (non-RTIO) SPI driver for NXP LPSPI. -endif # !SPI_RTIO - -if SPI_RTIO -config SPI_MCUX_LPSPI_RTIO - bool "NXP MCUX LPSPI RTIO based driver" - default y - help - Use the RTIO-based SPI driver for the NXP LPSPI. - -config SPI_MCUX_RTIO_SQ_SIZE - int "number of available submission queue entries" - default 8 # sensible default that covers most common spi transactions - help - when rtio is use with spi each driver holds a context with which blocking - api calls use to perform spi transactions. this queue needs to be as deep - as the longest set of spi_buf_sets used, where normal spi operations are - used (equal length buffers). it may need to be slightly deeper where the - spi buffer sets for transmit/receive are not always matched equally in - length as these are transformed into normal transceives. -endif # SPI_RTIO endif # SPI_MCUX_LPSPI diff --git a/drivers/spi/spi_nxp_lpspi/spi_mcux_lpspi_rtio.c b/drivers/spi/spi_nxp_lpspi/spi_mcux_lpspi_rtio.c deleted file mode 100644 index 63f4f822229f..000000000000 --- a/drivers/spi/spi_nxp_lpspi/spi_mcux_lpspi_rtio.c +++ /dev/null @@ -1,255 +0,0 @@ -/* - * Copyright 2023-2024 NXP - * - * SPDX-License-Identifier: Apache-2.0 - */ - -#define DT_DRV_COMPAT nxp_lpspi - -#include -LOG_MODULE_REGISTER(spi_mcux_lpspi_rtio, CONFIG_SPI_LOG_LEVEL); - -#include -#include "spi_nxp_lpspi_priv.h" - -struct spi_mcux_rtio_data { - lpspi_master_handle_t handle; - struct spi_rtio *rtio_ctx; - size_t transfer_len; -}; - -static int spi_mcux_transfer_next_packet(const struct device *dev) -{ - struct spi_mcux_data *data = dev->data; - struct spi_mcux_rtio_data *rtio_data = (struct spi_mcux_rtio_data *)data->driver_data; - LPSPI_Type *base = (LPSPI_Type *)DEVICE_MMIO_NAMED_GET(dev, reg_base); - struct spi_context *ctx = &data->ctx; - size_t max_chunk = spi_context_max_continuous_chunk(ctx); - lpspi_transfer_t transfer; - status_t status; - - if (max_chunk == 0) { - spi_context_cs_control(ctx, false); - spi_context_complete(ctx, dev, 0); - return 0; - } - - rtio_data->transfer_len = max_chunk; - - transfer.configFlags = LPSPI_MASTER_XFER_CFG_FLAGS(ctx->config->slave); - transfer.txData = (ctx->tx_len == 0 ? NULL : ctx->tx_buf); - transfer.rxData = (ctx->rx_len == 0 ? NULL : ctx->rx_buf); - transfer.dataSize = max_chunk; - - status = LPSPI_MasterTransferNonBlocking(base, &rtio_data->handle, &transfer); - if (status != kStatus_Success) { - LOG_ERR("Transfer could not start on %s: %d", dev->name, status); - return status == kStatus_LPSPI_Busy ? -EBUSY : -EINVAL; - } - - return 0; -} - -static void spi_mcux_iodev_complete(const struct device *dev, int status); - -static void spi_mcux_master_rtio_callback(LPSPI_Type *base, lpspi_master_handle_t *handle, - status_t status, void *userData) -{ - struct spi_mcux_data *data = userData; - struct spi_mcux_rtio_data *rtio_data = (struct spi_mcux_rtio_data *)data->driver_data; - struct spi_rtio *rtio_ctx = rtio_data->rtio_ctx; - - if (rtio_ctx->txn_head != NULL) { - spi_mcux_iodev_complete(data->dev, status); - return; - } - - spi_context_update_tx(&data->ctx, 1, rtio_data->transfer_len); - spi_context_update_rx(&data->ctx, 1, rtio_data->transfer_len); - - spi_mcux_transfer_next_packet(data->dev); -} - -static void spi_mcux_iodev_start(const struct device *dev) -{ - struct spi_mcux_data *data = dev->data; - struct spi_mcux_rtio_data *rtio_data = (struct spi_mcux_rtio_data *)data->driver_data; - struct spi_rtio *rtio_ctx = rtio_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; - LPSPI_Type *base = (LPSPI_Type *)DEVICE_MMIO_NAMED_GET(dev, reg_base); - lpspi_transfer_t transfer; - status_t status; - - status = spi_mcux_configure(dev, spi_cfg); - if (status) { - LOG_ERR("Error configuring lpspi"); - return; - } - - LPSPI_MasterTransferCreateHandle(base, &rtio_data->handle, spi_mcux_master_rtio_callback, - data); - - transfer.configFlags = LPSPI_MASTER_XFER_CFG_FLAGS(spi_cfg->slave); - - switch (sqe->op) { - case RTIO_OP_RX: - transfer.txData = NULL; - transfer.rxData = sqe->rx.buf; - transfer.dataSize = sqe->rx.buf_len; - break; - case RTIO_OP_TX: - transfer.rxData = NULL; - transfer.txData = sqe->tx.buf; - transfer.dataSize = sqe->tx.buf_len; - break; - case RTIO_OP_TINY_TX: - transfer.rxData = NULL; - transfer.txData = sqe->tiny_tx.buf; - transfer.dataSize = sqe->tiny_tx.buf_len; - break; - case RTIO_OP_TXRX: - transfer.txData = sqe->txrx.tx_buf; - transfer.rxData = sqe->txrx.rx_buf; - transfer.dataSize = sqe->txrx.buf_len; - break; - default: - LOG_ERR("Invalid op code %d for submission %p\n", sqe->op, (void *)sqe); - spi_mcux_iodev_complete(dev, -EINVAL); - return; - } - - rtio_data->transfer_len = transfer.dataSize; - - spi_context_cs_control(&data->ctx, true); - - status = LPSPI_MasterTransferNonBlocking(base, &rtio_data->handle, &transfer); - if (status != kStatus_Success) { - LOG_ERR("Transfer could not start on %s: %d", dev->name, status); - spi_mcux_iodev_complete(dev, -EIO); - } -} - -static void spi_mcux_iodev_complete(const struct device *dev, int status) -{ - struct spi_mcux_data *data = dev->data; - struct spi_mcux_rtio_data *rtio_data = (struct spi_mcux_rtio_data *)data->driver_data; - struct spi_rtio *rtio_ctx = rtio_data->rtio_ctx; - - 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); - return; - } - - /** De-assert CS-line to space from next transaction */ - spi_context_cs_control(&data->ctx, false); - - if (spi_rtio_complete(rtio_ctx, status)) { - spi_mcux_iodev_start(dev); - } -} - -static void spi_mcux_iodev_submit(const struct device *dev, struct rtio_iodev_sqe *iodev_sqe) -{ - struct spi_mcux_data *data = dev->data; - struct spi_mcux_rtio_data *rtio_data = (struct spi_mcux_rtio_data *)data->driver_data; - struct spi_rtio *rtio_ctx = rtio_data->rtio_ctx; - - if (spi_rtio_submit(rtio_ctx, iodev_sqe)) { - spi_mcux_iodev_start(dev); - } -} - -static 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_mcux_rtio_data *rtio_data = (struct spi_mcux_rtio_data *)data->driver_data; - struct spi_rtio *rtio_ctx = rtio_data->rtio_ctx; - int ret; - - spi_context_lock(&data->ctx, false, NULL, NULL, spi_cfg); - - ret = spi_rtio_transceive(rtio_ctx, spi_cfg, tx_bufs, rx_bufs); - - spi_context_release(&data->ctx, ret); - - return ret; -} - -#ifdef CONFIG_SPI_ASYNC -static int transceive_rtio_async(const struct device *dev, const struct spi_config *spi_cfg, - const struct spi_buf_set *tx_bufs, - const struct spi_buf_set *rx_bufs, spi_callback_t cb, - void *userdata) -{ - ARG_UNUSED(dev); - ARG_UNUSED(spi_cfg); - ARG_UNUSED(tx_bufs); - ARG_UNUSED(rx_bufs); - ARG_UNUSED(cb); - ARG_UNUSED(userdata); - - return -ENOTSUP; -} -#endif - -static DEVICE_API(spi, spi_mcux_rtio_driver_api) = { - .transceive = transceive_rtio, -#ifdef CONFIG_SPI_ASYNC - .transceive_async = transceive_rtio_async, -#endif - .iodev_submit = spi_mcux_iodev_submit, - .release = spi_mcux_release, -}; - -static int spi_mcux_rtio_init(const struct device *dev) -{ - struct spi_mcux_data *data = dev->data; - struct spi_mcux_rtio_data *rtio_data = (struct spi_mcux_rtio_data *)data->driver_data; - int err = 0; - - err = spi_nxp_init_common(dev); - if (err) { - return err; - } - - spi_rtio_init(rtio_data->rtio_ctx, dev); - - spi_context_unlock_unconditionally(&data->ctx); - - return 0; -} - -static void lpspi_isr(const struct device *dev) -{ - struct spi_mcux_data *data = dev->data; - struct spi_mcux_rtio_data *rtio_data = (struct spi_mcux_rtio_data *)data->driver_data; - LPSPI_Type *base = (LPSPI_Type *)DEVICE_MMIO_NAMED_GET(dev, reg_base); - - LPSPI_MasterTransferHandleIRQ(LPSPI_IRQ_HANDLE_ARG, &rtio_data->handle); -} - -#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) - -#define SPI_MCUX_LPSPI_RTIO_INIT(n) \ - SPI_MCUX_RTIO_DEFINE(n); \ - SPI_NXP_LPSPI_COMMON_INIT(n) \ - SPI_MCUX_LPSPI_CONFIG_INIT(n) \ - \ - static struct spi_mcux_rtio_data lpspi_rtio_data_##n = { \ - .rtio_ctx = &spi_mcux_rtio_##n, \ - }; \ - \ - static struct spi_mcux_data spi_mcux_data_##n = {.driver_data = &lpspi_rtio_data_##n, \ - SPI_NXP_LPSPI_COMMON_DATA_INIT(n)}; \ - \ - SPI_DEVICE_DT_INST_DEFINE(n, spi_mcux_rtio_init, NULL, &spi_mcux_data_##n, \ - &spi_mcux_config_##n, POST_KERNEL, CONFIG_SPI_INIT_PRIORITY, \ - &spi_mcux_rtio_driver_api); - -DT_INST_FOREACH_STATUS_OKAY(SPI_MCUX_LPSPI_RTIO_INIT) diff --git a/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi.c b/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi.c index 57e21bf20331..da6305e3e0bd 100644 --- a/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi.c +++ b/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi.c @@ -319,6 +319,9 @@ static DEVICE_API(spi, spi_mcux_driver_api) = { .transceive = spi_mcux_transceive_sync, #ifdef CONFIG_SPI_ASYNC .transceive_async = spi_mcux_transceive_async, +#endif +#ifdef CONFIG_SPI_RTIO + .iodev_submit = spi_rtio_iodev_default_submit, #endif .release = spi_mcux_release, }; diff --git a/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi_dma.c b/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi_dma.c index 63f27a1e8425..7ad3a3206de7 100644 --- a/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi_dma.c +++ b/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi_dma.c @@ -388,6 +388,9 @@ static DEVICE_API(spi, spi_mcux_driver_api) = { .transceive = spi_nxp_dma_transceive_sync, #ifdef CONFIG_SPI_ASYNC .transceive_async = spi_nxp_dma_transceive_async, +#endif +#ifdef CONFIG_SPI_RTIO + .iodev_submit = spi_rtio_iodev_default_submit, #endif .release = spi_mcux_release, }; diff --git a/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi_priv.h b/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi_priv.h index f1c45981ad95..5108f6ebbc9c 100644 --- a/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi_priv.h +++ b/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi_priv.h @@ -5,6 +5,7 @@ */ #include +#include #include #include #include