From 21ac4a8e0c3aa2bf2bfbbf40e86f01b80debacde Mon Sep 17 00:00:00 2001 From: Declan Snyder Date: Fri, 20 Sep 2024 14:50:01 -0500 Subject: [PATCH 01/18] drivers: spi_mcux_lpspi: Organize top of file Organize #includes and #defines for less redundancy and more readability. Shorten some macros to avoid multi line statements. Add some comments. Signed-off-by: Declan Snyder --- drivers/spi/spi_mcux_lpspi.c | 85 ++++++++++++++++++------------------ 1 file changed, 42 insertions(+), 43 deletions(-) diff --git a/drivers/spi/spi_mcux_lpspi.c b/drivers/spi/spi_mcux_lpspi.c index 11deabce1d30d..c66d201f700c4 100644 --- a/drivers/spi/spi_mcux_lpspi.c +++ b/drivers/spi/spi_mcux_lpspi.c @@ -6,29 +6,29 @@ #define DT_DRV_COMPAT nxp_imx_lpspi -#include #include +#include #include -#include -#if CONFIG_NXP_LP_FLEXCOMM -#include -#endif -#include #include -#ifdef CONFIG_SPI_MCUX_LPSPI_DMA -#include -#endif -#include + +#include +LOG_MODULE_REGISTER(spi_mcux_lpspi, CONFIG_SPI_LOG_LEVEL); + #ifdef CONFIG_SPI_RTIO -#include #include -#include #endif -LOG_MODULE_REGISTER(spi_mcux_lpspi, CONFIG_SPI_LOG_LEVEL); - #include "spi_context.h" +#if CONFIG_NXP_LP_FLEXCOMM +#include +#endif + +#include + +/* If any hardware revisions change this, make it into a DT property. + * DONT'T make #ifdefs here by platform. + */ #define CHIP_SELECT_COUNT 4 #define MAX_DATA_WIDTH 4096 @@ -36,6 +36,23 @@ LOG_MODULE_REGISTER(spi_mcux_lpspi, CONFIG_SPI_LOG_LEVEL); #define DEV_CFG(_dev) ((const struct spi_mcux_config *)(_dev)->config) #define DEV_DATA(_dev) ((struct spi_mcux_data *)(_dev)->data) +#ifdef CONFIG_SPI_MCUX_LPSPI_DMA +#include + +/* These flags are arbitrary */ +#define LPSPI_DMA_ERROR_FLAG BIT(0) +#define LPSPI_DMA_RX_DONE_FLAG BIT(1) +#define LPSPI_DMA_TX_DONE_FLAG BIT(2) +#define LPSPI_DMA_DONE_FLAG (LPSPI_DMA_RX_DONE_FLAG | LPSPI_DMA_TX_DONE_FLAG) + +struct spi_dma_stream { + const struct device *dma_dev; + uint32_t channel; /* stores the channel for dma */ + struct dma_config dma_cfg; + struct dma_block_config dma_blk_cfg; +}; +#endif /* CONFIG_SPI_MCUX_LPSPI_DMA */ + struct spi_mcux_config { DEVICE_MMIO_NAMED_ROM(reg_base); #ifdef CONFIG_NXP_LP_FLEXCOMM @@ -51,36 +68,19 @@ struct spi_mcux_config { lpspi_pin_config_t data_pin_config; }; -#ifdef CONFIG_SPI_MCUX_LPSPI_DMA -#define SPI_MCUX_LPSPI_DMA_ERROR_FLAG 0x01 -#define SPI_MCUX_LPSPI_DMA_RX_DONE_FLAG 0x02 -#define SPI_MCUX_LPSPI_DMA_TX_DONE_FLAG 0x04 -#define SPI_MCUX_LPSPI_DMA_DONE_FLAG \ - (SPI_MCUX_LPSPI_DMA_RX_DONE_FLAG | SPI_MCUX_LPSPI_DMA_TX_DONE_FLAG) - -struct stream { - const struct device *dma_dev; - uint32_t channel; /* stores the channel for dma */ - struct dma_config dma_cfg; - struct dma_block_config dma_blk_cfg; -}; -#endif - struct spi_mcux_data { DEVICE_MMIO_NAMED_RAM(reg_base); const struct device *dev; lpspi_master_handle_t handle; struct spi_context ctx; size_t transfer_len; - #ifdef CONFIG_SPI_RTIO struct spi_rtio *rtio_ctx; #endif - #ifdef CONFIG_SPI_MCUX_LPSPI_DMA volatile uint32_t status_flags; - struct stream dma_rx; - struct stream dma_tx; + struct spi_dma_stream dma_rx; + struct spi_dma_stream dma_tx; /* dummy value used for transferring NOP when tx buf is null */ uint32_t dummy_tx_buffer; /* dummy value used to read RX data into when rx buf is null */ @@ -286,25 +286,25 @@ static void spi_mcux_dma_callback(const struct device *dev, void *arg, uint32_t if (status < 0) { LOG_ERR("DMA callback error with channel %d.", channel); - data->status_flags |= SPI_MCUX_LPSPI_DMA_ERROR_FLAG; + data->status_flags |= LPSPI_DMA_ERROR_FLAG; } else { /* identify the origin of this callback */ if (channel == data->dma_tx.channel) { /* this part of the transfer ends */ - data->status_flags |= SPI_MCUX_LPSPI_DMA_TX_DONE_FLAG; + data->status_flags |= LPSPI_DMA_TX_DONE_FLAG; LOG_DBG("DMA TX Block Complete"); } else if (channel == data->dma_rx.channel) { /* this part of the transfer ends */ - data->status_flags |= SPI_MCUX_LPSPI_DMA_RX_DONE_FLAG; + data->status_flags |= LPSPI_DMA_RX_DONE_FLAG; LOG_DBG("DMA RX Block Complete"); } else { LOG_ERR("DMA callback channel %d is not valid.", channel); - data->status_flags |= SPI_MCUX_LPSPI_DMA_ERROR_FLAG; + data->status_flags |= LPSPI_DMA_ERROR_FLAG; } } #if CONFIG_SPI_ASYNC if (data->ctx.asynchronous && - ((data->status_flags & SPI_MCUX_LPSPI_DMA_DONE_FLAG) == SPI_MCUX_LPSPI_DMA_DONE_FLAG)) { + ((data->status_flags & LPSPI_DMA_DONE_FLAG) == LPSPI_DMA_DONE_FLAG)) { /* Load dma blocks of equal length */ size_t dma_size = MIN(data->ctx.tx_len, data->ctx.rx_len); @@ -332,7 +332,7 @@ static int spi_mcux_dma_tx_load(const struct device *dev, const uint8_t *buf, si LPSPI_Type *base = (LPSPI_Type *)DEVICE_MMIO_NAMED_GET(dev, reg_base); /* remember active TX DMA channel (used in callback) */ - struct stream *stream = &data->dma_tx; + struct spi_dma_stream *stream = &data->dma_tx; blk_cfg = &stream->dma_blk_cfg; @@ -373,7 +373,7 @@ static int spi_mcux_dma_rx_load(const struct device *dev, uint8_t *buf, size_t l LPSPI_Type *base = (LPSPI_Type *)DEVICE_MMIO_NAMED_GET(dev, reg_base); /* retrieve active RX DMA channel (used in callback) */ - struct stream *stream = &data->dma_rx; + struct spi_dma_stream *stream = &data->dma_rx; blk_cfg = &stream->dma_blk_cfg; @@ -416,12 +416,11 @@ static int wait_dma_rx_tx_done(const struct device *dev) LOG_DBG("Timed out waiting for SPI context to complete"); return ret; } - if (data->status_flags & SPI_MCUX_LPSPI_DMA_ERROR_FLAG) { + if (data->status_flags & LPSPI_DMA_ERROR_FLAG) { return -EIO; } - if ((data->status_flags & SPI_MCUX_LPSPI_DMA_DONE_FLAG) == - SPI_MCUX_LPSPI_DMA_DONE_FLAG) { + if ((data->status_flags & LPSPI_DMA_DONE_FLAG) == LPSPI_DMA_DONE_FLAG) { LOG_DBG("DMA block completed"); return 0; } From 56c3a7f1d58d61c8b690d7089c2045ec42f0e1d7 Mon Sep 17 00:00:00 2001 From: Declan Snyder Date: Fri, 20 Sep 2024 15:04:58 -0500 Subject: [PATCH 02/18] drivers: spi_mcux_lpspi: Remove parent_dev field Remove parent_dev from config struct, and simplify code for the definition and use of the irq config function. Signed-off-by: Declan Snyder --- drivers/spi/spi_mcux_lpspi.c | 50 +++++++++++------------------------- 1 file changed, 15 insertions(+), 35 deletions(-) diff --git a/drivers/spi/spi_mcux_lpspi.c b/drivers/spi/spi_mcux_lpspi.c index c66d201f700c4..9e103bdfefdaf 100644 --- a/drivers/spi/spi_mcux_lpspi.c +++ b/drivers/spi/spi_mcux_lpspi.c @@ -55,9 +55,6 @@ struct spi_dma_stream { struct spi_mcux_config { DEVICE_MMIO_NAMED_ROM(reg_base); -#ifdef CONFIG_NXP_LP_FLEXCOMM - const struct device *parent_dev; -#endif const struct device *clock_dev; clock_control_subsys_t clock_subsys; void (*irq_config_func)(const struct device *dev); @@ -661,16 +658,7 @@ static int spi_mcux_init(const struct device *dev) DEVICE_MMIO_NAMED_MAP(dev, reg_base, K_MEM_CACHE_NONE | K_MEM_DIRECT_MAP); -#if CONFIG_NXP_LP_FLEXCOMM - /* When using LP Flexcomm driver, register the interrupt handler - * so we receive notification from the LP Flexcomm interrupt handler. - */ - nxp_lp_flexcomm_setirqhandler(config->parent_dev, dev, LP_FLEXCOMM_PERIPH_LPSPI, - spi_mcux_isr); -#else - /* Interrupt is managed by this driver */ config->irq_config_func(dev); -#endif err = spi_context_cs_configure_all(&data->ctx); if (err < 0) { @@ -849,31 +837,29 @@ static const struct spi_driver_api spi_mcux_driver_api = { #define SPI_DMA_CHANNELS(n) #endif /* CONFIG_SPI_MCUX_LPSPI_DMA */ -#define SPI_MCUX_LPSPI_MODULE_IRQ_CONNECT(n) \ - do { \ - IRQ_CONNECT(DT_INST_IRQN(n), DT_INST_IRQ(n, priority), spi_mcux_isr, \ - DEVICE_DT_INST_GET(n), 0); \ - irq_enable(DT_INST_IRQN(n)); \ - } while (false) - -#define SPI_MCUX_LPSPI_MODULE_IRQ(n) \ - IF_ENABLED(DT_INST_IRQ_HAS_IDX(n, 0), (SPI_MCUX_LPSPI_MODULE_IRQ_CONNECT(n))) - -#ifdef CONFIG_NXP_LP_FLEXCOMM -#define PARENT_DEV(n) .parent_dev = DEVICE_DT_GET(DT_INST_PARENT(n)), +#if defined(CONFIG_NXP_LP_FLEXCOMM) +#define SPI_MCUX_LPSPI_IRQ_FUNC(n) \ + nxp_lp_flexcomm_setirqhandler(DEVICE_DT_GET(DT_INST_PARENT(n)), DEVICE_DT_INST_GET(n), \ + LP_FLEXCOMM_PERIPH_LPSPI, spi_mcux_isr); #else -#define PARENT_DEV(n) -#endif /* CONFIG_NXP_LP_FLEXCOMM */ +#define SPI_MCUX_LPSPI_IRQ_FUNC(n) \ + IRQ_CONNECT(DT_INST_IRQN(n), DT_INST_IRQ(n, priority), spi_mcux_isr, \ + DEVICE_DT_INST_GET(n), 0); \ + irq_enable(DT_INST_IRQN(n)); +#endif #define SPI_MCUX_LPSPI_INIT(n) \ PINCTRL_DT_INST_DEFINE(n); \ COND_CODE_1(CONFIG_SPI_RTIO, (SPI_MCUX_RTIO_DEFINE(n)), ()); \ \ - static void spi_mcux_config_func_##n(const struct device *dev); \ + static void spi_mcux_config_func_##n(const struct device *dev) \ + { \ + SPI_MCUX_LPSPI_IRQ_FUNC(n) \ + } \ \ static const struct spi_mcux_config spi_mcux_config_##n = { \ DEVICE_MMIO_NAMED_ROM_INIT(reg_base, DT_DRV_INST(n)), \ - PARENT_DEV(n).clock_dev = DEVICE_DT_GET(DT_INST_CLOCKS_CTLR(n)), \ + .clock_dev = DEVICE_DT_GET(DT_INST_CLOCKS_CTLR(n)), \ .clock_subsys = (clock_control_subsys_t)DT_INST_CLOCKS_CELL(n, name), \ .irq_config_func = spi_mcux_config_func_##n, \ .pcs_sck_delay = UTIL_AND(DT_INST_NODE_HAS_PROP(n, pcs_sck_delay), \ @@ -891,15 +877,9 @@ static const struct spi_driver_api spi_mcux_driver_api = { SPI_CONTEXT_INIT_SYNC(spi_mcux_data_##n, ctx), \ SPI_CONTEXT_CS_GPIOS_INITIALIZE(DT_DRV_INST(n), ctx) SPI_DMA_CHANNELS(n) \ IF_ENABLED(CONFIG_SPI_RTIO, (.rtio_ctx = &spi_mcux_rtio_##n,)) \ - \ }; \ \ DEVICE_DT_INST_DEFINE(n, spi_mcux_init, NULL, &spi_mcux_data_##n, &spi_mcux_config_##n, \ - POST_KERNEL, CONFIG_SPI_INIT_PRIORITY, &spi_mcux_driver_api); \ - \ - static void spi_mcux_config_func_##n(const struct device *dev) \ - { \ - SPI_MCUX_LPSPI_MODULE_IRQ(n); \ - } + POST_KERNEL, CONFIG_SPI_INIT_PRIORITY, &spi_mcux_driver_api); DT_INST_FOREACH_STATUS_OKAY(SPI_MCUX_LPSPI_INIT) From 3993e44693da35509a87ef0923fd6b7820c241d3 Mon Sep 17 00:00:00 2001 From: Declan Snyder Date: Fri, 20 Sep 2024 15:13:24 -0500 Subject: [PATCH 03/18] drivers: spi_mcux_lpspi: Move init to end of file Move init function to bottom of file by custom. Signed-off-by: Declan Snyder --- drivers/spi/spi_mcux_lpspi.c | 94 ++++++++++++++++++------------------ 1 file changed, 47 insertions(+), 47 deletions(-) diff --git a/drivers/spi/spi_mcux_lpspi.c b/drivers/spi/spi_mcux_lpspi.c index 9e103bdfefdaf..38fb7120a23eb 100644 --- a/drivers/spi/spi_mcux_lpspi.c +++ b/drivers/spi/spi_mcux_lpspi.c @@ -650,53 +650,6 @@ static int spi_mcux_release(const struct device *dev, const struct spi_config *s return 0; } -static int spi_mcux_init(const struct device *dev) -{ - int err; - const struct spi_mcux_config *config = dev->config; - struct spi_mcux_data *data = dev->data; - - DEVICE_MMIO_NAMED_MAP(dev, reg_base, K_MEM_CACHE_NONE | K_MEM_DIRECT_MAP); - - config->irq_config_func(dev); - - err = spi_context_cs_configure_all(&data->ctx); - if (err < 0) { - return err; - } - - spi_context_unlock_unconditionally(&data->ctx); - - data->dev = dev; - -#ifdef CONFIG_SPI_MCUX_LPSPI_DMA - if (data->dma_tx.dma_dev && data->dma_rx.dma_dev) { - if (!device_is_ready(data->dma_tx.dma_dev)) { - LOG_ERR("%s device is not ready", data->dma_tx.dma_dev->name); - return -ENODEV; - } - - if (!device_is_ready(data->dma_rx.dma_dev)) { - LOG_ERR("%s device is not ready", data->dma_rx.dma_dev->name); - return -ENODEV; - } - } -#endif /* CONFIG_SPI_MCUX_LPSPI_DMA */ - -#ifdef CONFIG_SPI_RTIO - spi_rtio_init(data->rtio_ctx, dev); -#endif - - err = pinctrl_apply_state(config->pincfg, PINCTRL_STATE_DEFAULT); - if (err) { - return err; - } - - spi_context_unlock_unconditionally(&data->ctx); - - return 0; -} - #ifdef CONFIG_SPI_RTIO static inline void spi_mcux_iodev_prepare_start(const struct device *dev) @@ -796,6 +749,53 @@ static void spi_mcux_iodev_complete(const struct device *dev, int status) #endif +static int spi_mcux_init(const struct device *dev) +{ + int err; + const struct spi_mcux_config *config = dev->config; + struct spi_mcux_data *data = dev->data; + + DEVICE_MMIO_NAMED_MAP(dev, reg_base, K_MEM_CACHE_NONE | K_MEM_DIRECT_MAP); + + config->irq_config_func(dev); + + err = spi_context_cs_configure_all(&data->ctx); + if (err < 0) { + return err; + } + + spi_context_unlock_unconditionally(&data->ctx); + + data->dev = dev; + +#ifdef CONFIG_SPI_MCUX_LPSPI_DMA + if (data->dma_tx.dma_dev && data->dma_rx.dma_dev) { + if (!device_is_ready(data->dma_tx.dma_dev)) { + LOG_ERR("%s device is not ready", data->dma_tx.dma_dev->name); + return -ENODEV; + } + + if (!device_is_ready(data->dma_rx.dma_dev)) { + LOG_ERR("%s device is not ready", data->dma_rx.dma_dev->name); + return -ENODEV; + } + } +#endif /* CONFIG_SPI_MCUX_LPSPI_DMA */ + +#ifdef CONFIG_SPI_RTIO + spi_rtio_init(data->rtio_ctx, dev); +#endif + + err = pinctrl_apply_state(config->pincfg, PINCTRL_STATE_DEFAULT); + if (err) { + return err; + } + + spi_context_unlock_unconditionally(&data->ctx); + + return 0; +} + static const struct spi_driver_api spi_mcux_driver_api = { .transceive = spi_mcux_transceive, #ifdef CONFIG_SPI_ASYNC From f7733eedb433a457cc284b3617824c206a484a29 Mon Sep 17 00:00:00 2001 From: Declan Snyder Date: Fri, 20 Sep 2024 15:17:44 -0500 Subject: [PATCH 04/18] drivers: spi_mcux_lpspi: Clean up init function Do initializations in a more logical order, remove unnecessarily duplicated code, reorder stack variables to be in reverse christmas tree order. Signed-off-by: Declan Snyder --- drivers/spi/spi_mcux_lpspi.c | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/drivers/spi/spi_mcux_lpspi.c b/drivers/spi/spi_mcux_lpspi.c index 38fb7120a23eb..3e83c96611421 100644 --- a/drivers/spi/spi_mcux_lpspi.c +++ b/drivers/spi/spi_mcux_lpspi.c @@ -751,21 +751,12 @@ static void spi_mcux_iodev_complete(const struct device *dev, int status) static int spi_mcux_init(const struct device *dev) { - int err; const struct spi_mcux_config *config = dev->config; struct spi_mcux_data *data = dev->data; + int err; DEVICE_MMIO_NAMED_MAP(dev, reg_base, K_MEM_CACHE_NONE | K_MEM_DIRECT_MAP); - config->irq_config_func(dev); - - err = spi_context_cs_configure_all(&data->ctx); - if (err < 0) { - return err; - } - - spi_context_unlock_unconditionally(&data->ctx); - data->dev = dev; #ifdef CONFIG_SPI_MCUX_LPSPI_DMA @@ -782,15 +773,21 @@ static int spi_mcux_init(const struct device *dev) } #endif /* CONFIG_SPI_MCUX_LPSPI_DMA */ -#ifdef CONFIG_SPI_RTIO - spi_rtio_init(data->rtio_ctx, dev); -#endif + err = spi_context_cs_configure_all(&data->ctx); + if (err < 0) { + return err; + } err = pinctrl_apply_state(config->pincfg, PINCTRL_STATE_DEFAULT); if (err) { return err; } + config->irq_config_func(dev); + +#ifdef CONFIG_SPI_RTIO + spi_rtio_init(data->rtio_ctx, dev); +#endif spi_context_unlock_unconditionally(&data->ctx); return 0; From d6d2cfa709cfa122a7eda16cde38087f99081d96 Mon Sep 17 00:00:00 2001 From: Declan Snyder Date: Fri, 20 Sep 2024 15:42:21 -0500 Subject: [PATCH 05/18] drivers: spi_mcux_lpspi: Create DMA dev helpers Create helper functions for duplicated code related to checking for DMA devices, with readable names. Also remove unneeded function prototype. Signed-off-by: Declan Snyder --- drivers/spi/spi_mcux_lpspi.c | 52 ++++++++++++++++++++++++------------ 1 file changed, 35 insertions(+), 17 deletions(-) diff --git a/drivers/spi/spi_mcux_lpspi.c b/drivers/spi/spi_mcux_lpspi.c index 3e83c96611421..68e2d630dc9af 100644 --- a/drivers/spi/spi_mcux_lpspi.c +++ b/drivers/spi/spi_mcux_lpspi.c @@ -272,7 +272,10 @@ static int spi_mcux_configure(const struct device *dev, const struct spi_config } #ifdef CONFIG_SPI_MCUX_LPSPI_DMA -static int spi_mcux_dma_rxtx_load(const struct device *dev, size_t *dma_size); +static bool lpspi_inst_has_dma(const struct spi_mcux_data *data) +{ + return (data->dma_tx.dma_dev && data->dma_rx.dma_dev); +} /* This function is executed in the interrupt context */ static void spi_mcux_dma_callback(const struct device *dev, void *arg, uint32_t channel, int status) @@ -550,7 +553,9 @@ static int transceive_dma(const struct device *dev, const struct spi_config *spi return ret; } -#endif +#else +#define lpspi_inst_has_dma(arg) arg != arg +#endif /* CONFIG_SPI_MCUX_LPSPI_DMA */ #ifdef CONFIG_SPI_RTIO @@ -613,7 +618,7 @@ static int spi_mcux_transceive(const struct device *dev, const struct spi_config #ifdef CONFIG_SPI_MCUX_LPSPI_DMA const struct spi_mcux_data *data = dev->data; - if (data->dma_rx.dma_dev && data->dma_tx.dma_dev) { + if (lpspi_inst_has_dma(data)) { return transceive_dma(dev, spi_cfg, tx_bufs, rx_bufs, false, NULL, NULL); } #endif /* CONFIG_SPI_MCUX_LPSPI_DMA */ @@ -630,7 +635,7 @@ static int spi_mcux_transceive_async(const struct device *dev, const struct spi_ #ifdef CONFIG_SPI_MCUX_LPSPI_DMA struct spi_mcux_data *data = dev->data; - if (data->dma_rx.dma_dev && data->dma_tx.dma_dev) { + if (lpspi_inst_has_dma(data)) { spi_context_buffers_setup(&data->ctx, tx_bufs, rx_bufs, 1); } @@ -749,29 +754,42 @@ static void spi_mcux_iodev_complete(const struct device *dev, int status) #endif +#if defined(CONFIG_SPI_MCUX_LPSPI_DMA) +static int lpspi_dma_dev_ready(const struct device *dma_dev) +{ + if (!device_is_ready(dma_dev)) { + LOG_ERR("%s device is not ready", dma_dev->name); + return -ENODEV; + } + + return 0; +} + +static int lpspi_dma_devs_ready(struct spi_mcux_data *data) +{ + return lpspi_dma_dev_ready(data->dma_tx.dma_dev) | + lpspi_dma_dev_ready(data->dma_rx.dma_dev); +} +#else +#define lpspi_dma_devs_ready(...) 0 +#endif /* CONFIG_SPI_MCUX_LPSPI_DMA */ + static int spi_mcux_init(const struct device *dev) { const struct spi_mcux_config *config = dev->config; struct spi_mcux_data *data = dev->data; - int err; + int err = 0; DEVICE_MMIO_NAMED_MAP(dev, reg_base, K_MEM_CACHE_NONE | K_MEM_DIRECT_MAP); data->dev = dev; -#ifdef CONFIG_SPI_MCUX_LPSPI_DMA - if (data->dma_tx.dma_dev && data->dma_rx.dma_dev) { - if (!device_is_ready(data->dma_tx.dma_dev)) { - LOG_ERR("%s device is not ready", data->dma_tx.dma_dev->name); - return -ENODEV; - } - - if (!device_is_ready(data->dma_rx.dma_dev)) { - LOG_ERR("%s device is not ready", data->dma_rx.dma_dev->name); - return -ENODEV; - } + if (IS_ENABLED(CONFIG_SPI_MCUX_LPSPI_DMA) && lpspi_inst_has_dma(data)) { + err = lpspi_dma_devs_ready(data); + } + if (err < 0) { + return err; } -#endif /* CONFIG_SPI_MCUX_LPSPI_DMA */ err = spi_context_cs_configure_all(&data->ctx); if (err < 0) { From 3caf9ee4845337f0e1104612782ba564651e840d Mon Sep 17 00:00:00 2001 From: Declan Snyder Date: Fri, 20 Sep 2024 15:51:09 -0500 Subject: [PATCH 06/18] drivers: spi_mcux_lpspi: Minor RTIO cleanup Add closing comment for #ifdef and remove unnecessary indentation of an else block. Also move the transcieve_rtio function to be near the other rtio functions. And move function prototypes to top of file. Signed-off-by: Declan Snyder --- drivers/spi/spi_mcux_lpspi.c | 67 +++++++++++++++++------------------- 1 file changed, 32 insertions(+), 35 deletions(-) diff --git a/drivers/spi/spi_mcux_lpspi.c b/drivers/spi/spi_mcux_lpspi.c index 68e2d630dc9af..d9b575d66017e 100644 --- a/drivers/spi/spi_mcux_lpspi.c +++ b/drivers/spi/spi_mcux_lpspi.c @@ -85,6 +85,13 @@ struct spi_mcux_data { #endif }; +#ifdef CONFIG_SPI_RTIO +static void spi_mcux_iodev_complete(const struct device *dev, int status); +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); +#endif + static int spi_mcux_transfer_next_packet(const struct device *dev) { /* const struct spi_mcux_config *config = dev->config; */ @@ -161,10 +168,6 @@ static void spi_mcux_isr(const struct device *dev) #endif } -#ifdef CONFIG_SPI_RTIO -static void spi_mcux_iodev_complete(const struct device *dev, int status); -#endif - static void spi_mcux_master_transfer_callback(LPSPI_Type *base, lpspi_master_handle_t *handle, status_t status, void *userData) { @@ -557,27 +560,6 @@ static int transceive_dma(const struct device *dev, const struct spi_config *spi #define lpspi_inst_has_dma(arg) arg != arg #endif /* CONFIG_SPI_MCUX_LPSPI_DMA */ -#ifdef CONFIG_SPI_RTIO - -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(rtio_ctx, spi_cfg, tx_bufs, rx_bufs); - - spi_context_release(&data->ctx, ret); - - return ret; -} - -#endif /* CONFIG_SPI_RTIO */ - static int 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, bool asynchronous, spi_callback_t cb, void *userdata) @@ -614,7 +596,6 @@ static int spi_mcux_transceive(const struct device *dev, const struct spi_config #ifdef CONFIG_SPI_RTIO return transceive_rtio(dev, spi_cfg, tx_bufs, rx_bufs); #endif /* CONFIG_SPI_RTIO */ - #ifdef CONFIG_SPI_MCUX_LPSPI_DMA const struct spi_mcux_data *data = dev->data; @@ -656,6 +637,22 @@ static int spi_mcux_release(const struct device *dev, const struct spi_config *s } #ifdef CONFIG_SPI_RTIO +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(rtio_ctx, spi_cfg, tx_bufs, rx_bufs); + + spi_context_release(&data->ctx, ret); + + return ret; +} static inline void spi_mcux_iodev_prepare_start(const struct device *dev) { @@ -741,18 +738,18 @@ static void spi_mcux_iodev_complete(const struct device *dev, int status) 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); + return; + } - if (spi_rtio_complete(rtio_ctx, status)) { - spi_mcux_iodev_prepare_start(dev); - spi_mcux_iodev_start(dev); - } + /** 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_prepare_start(dev); + spi_mcux_iodev_start(dev); } } - -#endif +#endif /* CONFIG_SPI_RTIO */ #if defined(CONFIG_SPI_MCUX_LPSPI_DMA) static int lpspi_dma_dev_ready(const struct device *dma_dev) From a383f368b790ec0fc8346b65b7aba23b0c50ebcc Mon Sep 17 00:00:00 2001 From: Declan Snyder Date: Fri, 20 Sep 2024 16:16:56 -0500 Subject: [PATCH 07/18] drivers: spi_mcux_lpspi: Consolidate IRQ handle Small change to consolidate the amount of lines of code in the spi_mcux_isr by making a macro for the argument and removing a line of code that was commented out. Also move the ISR to be the first function in the file which is common in many other drivers, instead of randomly in the middle of the file. And move the isr callback to be next to the isr. Signed-off-by: Declan Snyder --- drivers/spi/spi_mcux_lpspi.c | 66 +++++++++++++++++------------------- 1 file changed, 32 insertions(+), 34 deletions(-) diff --git a/drivers/spi/spi_mcux_lpspi.c b/drivers/spi/spi_mcux_lpspi.c index d9b575d66017e..efa7fc933eddb 100644 --- a/drivers/spi/spi_mcux_lpspi.c +++ b/drivers/spi/spi_mcux_lpspi.c @@ -36,6 +36,9 @@ LOG_MODULE_REGISTER(spi_mcux_lpspi, CONFIG_SPI_LOG_LEVEL); #define DEV_CFG(_dev) ((const struct spi_mcux_config *)(_dev)->config) #define DEV_DATA(_dev) ((struct spi_mcux_data *)(_dev)->data) +/* Argument to MCUX SDK IRQ handler */ +#define LPSPI_IRQ_HANDLE_ARG COND_CODE_1(CONFIG_NXP_LP_FLEXCOMM, (LPSPI_GetInstance(base)), (base)) + #ifdef CONFIG_SPI_MCUX_LPSPI_DMA #include @@ -85,6 +88,7 @@ struct spi_mcux_data { #endif }; +static int spi_mcux_transfer_next_packet(const struct device *dev); #ifdef CONFIG_SPI_RTIO static void spi_mcux_iodev_complete(const struct device *dev, int status); static inline int transceive_rtio(const struct device *dev, const struct spi_config *spi_cfg, @@ -92,6 +96,33 @@ static inline int transceive_rtio(const struct device *dev, const struct spi_con const struct spi_buf_set *rx_bufs); #endif +static void spi_mcux_isr(const struct device *dev) +{ + struct spi_mcux_data *data = dev->data; + LPSPI_Type *base = (LPSPI_Type *)DEVICE_MMIO_NAMED_GET(dev, reg_base); + + LPSPI_MasterTransferHandleIRQ(LPSPI_IRQ_HANDLE_ARG, &data->handle); +} + +static void spi_mcux_master_callback(LPSPI_Type *base, lpspi_master_handle_t *handle, + status_t status, void *userData) +{ + struct spi_mcux_data *data = userData; + +#ifdef CONFIG_SPI_RTIO + struct spi_rtio *rtio_ctx = data->rtio_ctx; + + if (rtio_ctx->txn_head != NULL) { + spi_mcux_iodev_complete(data->dev, status); + return; + } +#endif + spi_context_update_tx(&data->ctx, 1, data->transfer_len); + spi_context_update_rx(&data->ctx, 1, data->transfer_len); + + spi_mcux_transfer_next_packet(data->dev); +} + static int spi_mcux_transfer_next_packet(const struct device *dev) { /* const struct spi_mcux_config *config = dev->config; */ @@ -155,38 +186,6 @@ static int spi_mcux_transfer_next_packet(const struct device *dev) return 0; } -static void spi_mcux_isr(const struct device *dev) -{ - /* const struct spi_mcux_config *config = dev->config; */ - struct spi_mcux_data *data = dev->data; - LPSPI_Type *base = (LPSPI_Type *)DEVICE_MMIO_NAMED_GET(dev, reg_base); - -#if CONFIG_NXP_LP_FLEXCOMM - LPSPI_MasterTransferHandleIRQ(LPSPI_GetInstance(base), &data->handle); -#else - LPSPI_MasterTransferHandleIRQ(base, &data->handle); -#endif -} - -static void spi_mcux_master_transfer_callback(LPSPI_Type *base, lpspi_master_handle_t *handle, - status_t status, void *userData) -{ - struct spi_mcux_data *data = userData; - -#ifdef CONFIG_SPI_RTIO - struct spi_rtio *rtio_ctx = data->rtio_ctx; - - if (rtio_ctx->txn_head != NULL) { - spi_mcux_iodev_complete(data->dev, status); - return; - } -#endif - spi_context_update_tx(&data->ctx, 1, data->transfer_len); - spi_context_update_rx(&data->ctx, 1, data->transfer_len); - - spi_mcux_transfer_next_packet(data->dev); -} - static int spi_mcux_configure(const struct device *dev, const struct spi_config *spi_cfg) { const struct spi_mcux_config *config = dev->config; @@ -264,8 +263,7 @@ static int spi_mcux_configure(const struct device *dev, const struct spi_config base->CR |= LPSPI_CR_DBGEN_MASK; } - LPSPI_MasterTransferCreateHandle(base, &data->handle, spi_mcux_master_transfer_callback, - data); + LPSPI_MasterTransferCreateHandle(base, &data->handle, spi_mcux_master_callback, data); LPSPI_SetDummyData(base, 0); From 086dd02ff9c1e0b80d25c87f112bd7ab71ee833c Mon Sep 17 00:00:00 2001 From: Declan Snyder Date: Fri, 20 Sep 2024 16:35:06 -0500 Subject: [PATCH 08/18] drivers: spi_mcux_lpspi: Group API functions Group the actual API functions to be next to each other. Signed-off-by: Declan Snyder --- drivers/spi/spi_mcux_lpspi.c | 140 +++++++++++++++++------------------ 1 file changed, 69 insertions(+), 71 deletions(-) diff --git a/drivers/spi/spi_mcux_lpspi.c b/drivers/spi/spi_mcux_lpspi.c index efa7fc933eddb..2859187ca0936 100644 --- a/drivers/spi/spi_mcux_lpspi.c +++ b/drivers/spi/spi_mcux_lpspi.c @@ -91,9 +91,6 @@ struct spi_mcux_data { static int spi_mcux_transfer_next_packet(const struct device *dev); #ifdef CONFIG_SPI_RTIO static void spi_mcux_iodev_complete(const struct device *dev, int status); -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); #endif static void spi_mcux_isr(const struct device *dev) @@ -588,52 +585,6 @@ static int transceive(const struct device *dev, const struct spi_config *spi_cfg return ret; } -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); -#endif /* CONFIG_SPI_RTIO */ -#ifdef CONFIG_SPI_MCUX_LPSPI_DMA - const struct spi_mcux_data *data = dev->data; - - if (lpspi_inst_has_dma(data)) { - return transceive_dma(dev, spi_cfg, tx_bufs, rx_bufs, false, NULL, NULL); - } -#endif /* CONFIG_SPI_MCUX_LPSPI_DMA */ - - return transceive(dev, spi_cfg, tx_bufs, rx_bufs, false, NULL, NULL); -} - -#ifdef CONFIG_SPI_ASYNC -static int spi_mcux_transceive_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) -{ -#ifdef CONFIG_SPI_MCUX_LPSPI_DMA - struct spi_mcux_data *data = dev->data; - - if (lpspi_inst_has_dma(data)) { - spi_context_buffers_setup(&data->ctx, tx_bufs, rx_bufs, 1); - } - - return transceive_dma(dev, spi_cfg, tx_bufs, rx_bufs, true, cb, userdata); -#else - return transceive(dev, spi_cfg, tx_bufs, rx_bufs, true, cb, userdata); -#endif /* CONFIG_SPI_MCUX_LPSPI_DMA */ -} -#endif /* CONFIG_SPI_ASYNC */ - -static int spi_mcux_release(const struct device *dev, const struct spi_config *spi_cfg) -{ - struct spi_mcux_data *data = dev->data; - - spi_context_unlock_unconditionally(&data->ctx); - - return 0; -} - #ifdef CONFIG_SPI_RTIO static inline int transceive_rtio(const struct device *dev, const struct spi_config *spi_cfg, const struct spi_buf_set *tx_bufs, @@ -717,17 +668,6 @@ static void spi_mcux_iodev_start(const struct device *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_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); - } -} - static void spi_mcux_iodev_complete(const struct device *dev, int status) { struct spi_mcux_data *data = dev->data; @@ -747,8 +687,77 @@ static void spi_mcux_iodev_complete(const struct device *dev, int 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_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); + } +} + #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); +#endif /* CONFIG_SPI_RTIO */ +#ifdef CONFIG_SPI_MCUX_LPSPI_DMA + const struct spi_mcux_data *data = dev->data; + + if (lpspi_inst_has_dma(data)) { + return transceive_dma(dev, spi_cfg, tx_bufs, rx_bufs, false, NULL, NULL); + } +#endif /* CONFIG_SPI_MCUX_LPSPI_DMA */ + + return transceive(dev, spi_cfg, tx_bufs, rx_bufs, false, NULL, NULL); +} + +#ifdef CONFIG_SPI_ASYNC +static int spi_mcux_transceive_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) +{ +#ifdef CONFIG_SPI_MCUX_LPSPI_DMA + struct spi_mcux_data *data = dev->data; + + if (lpspi_inst_has_dma(data)) { + spi_context_buffers_setup(&data->ctx, tx_bufs, rx_bufs, 1); + } + + return transceive_dma(dev, spi_cfg, tx_bufs, rx_bufs, true, cb, userdata); +#else + return transceive(dev, spi_cfg, tx_bufs, rx_bufs, true, cb, userdata); +#endif /* CONFIG_SPI_MCUX_LPSPI_DMA */ +} +#endif /* CONFIG_SPI_ASYNC */ + +static int spi_mcux_release(const struct device *dev, const struct spi_config *spi_cfg) +{ + struct spi_mcux_data *data = dev->data; + + spi_context_unlock_unconditionally(&data->ctx); + + return 0; +} + +static const struct spi_driver_api spi_mcux_driver_api = { + .transceive = spi_mcux_transceive, +#ifdef CONFIG_SPI_ASYNC + .transceive_async = spi_mcux_transceive_async, +#endif +#ifdef CONFIG_SPI_RTIO + .iodev_submit = spi_mcux_iodev_submit, +#endif + .release = spi_mcux_release, +}; + #if defined(CONFIG_SPI_MCUX_LPSPI_DMA) static int lpspi_dma_dev_ready(const struct device *dma_dev) { @@ -806,17 +815,6 @@ static int spi_mcux_init(const struct device *dev) return 0; } -static const struct spi_driver_api spi_mcux_driver_api = { - .transceive = spi_mcux_transceive, -#ifdef CONFIG_SPI_ASYNC - .transceive_async = spi_mcux_transceive_async, -#endif -#ifdef CONFIG_SPI_RTIO - .iodev_submit = spi_mcux_iodev_submit, -#endif - .release = spi_mcux_release, -}; - #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) From 05034009000f95020dbfa2624e60241be8609cc3 Mon Sep 17 00:00:00 2001 From: Declan Snyder Date: Fri, 20 Sep 2024 17:07:00 -0500 Subject: [PATCH 09/18] drivers: spi_mcux_lpspi: clean up transceive calls Move transceive funtions next to each other and clean up the wrapping of functions to eliminate the ifdefs. Signed-off-by: Declan Snyder --- drivers/spi/spi_mcux_lpspi.c | 138 ++++++++++++++++++----------------- 1 file changed, 70 insertions(+), 68 deletions(-) diff --git a/drivers/spi/spi_mcux_lpspi.c b/drivers/spi/spi_mcux_lpspi.c index 2859187ca0936..158f79ff13a18 100644 --- a/drivers/spi/spi_mcux_lpspi.c +++ b/drivers/spi/spi_mcux_lpspi.c @@ -553,56 +553,10 @@ static int transceive_dma(const struct device *dev, const struct spi_config *spi } #else #define lpspi_inst_has_dma(arg) arg != arg +#define transceive_dma(...) 0 #endif /* CONFIG_SPI_MCUX_LPSPI_DMA */ -static int 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, - bool asynchronous, spi_callback_t cb, void *userdata) -{ - struct spi_mcux_data *data = dev->data; - int ret; - - spi_context_lock(&data->ctx, asynchronous, cb, userdata, spi_cfg); - - ret = spi_mcux_configure(dev, spi_cfg); - if (ret) { - goto out; - } - - spi_context_buffers_setup(&data->ctx, tx_bufs, rx_bufs, 1); - - spi_context_cs_control(&data->ctx, true); - - ret = spi_mcux_transfer_next_packet(dev); - if (ret) { - goto out; - } - - ret = spi_context_wait_for_completion(&data->ctx); -out: - spi_context_release(&data->ctx, ret); - - return ret; -} - #ifdef CONFIG_SPI_RTIO -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(rtio_ctx, spi_cfg, tx_bufs, rx_bufs); - - spi_context_release(&data->ctx, ret); - - return ret; -} - static inline void spi_mcux_iodev_prepare_start(const struct device *dev) { struct spi_mcux_data *data = dev->data; @@ -699,23 +653,81 @@ static void spi_mcux_iodev_submit(const struct device *dev, struct rtio_iodev_sq } } +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(rtio_ctx, spi_cfg, tx_bufs, rx_bufs); + + spi_context_release(&data->ctx, ret); + + return ret; +} +#else +#define transceive_rtio(...) 0 #endif /* CONFIG_SPI_RTIO */ +static int 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, + bool asynchronous, spi_callback_t cb, void *userdata) +{ + struct spi_mcux_data *data = dev->data; + int ret; + + spi_context_lock(&data->ctx, asynchronous, cb, userdata, spi_cfg); + + ret = spi_mcux_configure(dev, spi_cfg); + if (ret) { + goto out; + } + + spi_context_buffers_setup(&data->ctx, tx_bufs, rx_bufs, 1); + + spi_context_cs_control(&data->ctx, true); + + ret = spi_mcux_transfer_next_packet(dev); + if (ret) { + goto out; + } + + ret = spi_context_wait_for_completion(&data->ctx); +out: + spi_context_release(&data->ctx, ret); + + return ret; +} + 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) + const struct spi_buf_set *tx_bufs, const struct spi_buf_set *rx_bufs, + spi_callback_t cb, void *userdata, bool async) { -#ifdef CONFIG_SPI_RTIO - return transceive_rtio(dev, spi_cfg, tx_bufs, rx_bufs); -#endif /* CONFIG_SPI_RTIO */ -#ifdef CONFIG_SPI_MCUX_LPSPI_DMA - const struct spi_mcux_data *data = dev->data; + struct spi_mcux_data *data = dev->data; if (lpspi_inst_has_dma(data)) { - return transceive_dma(dev, spi_cfg, tx_bufs, rx_bufs, false, NULL, NULL); + if (async) { + spi_context_buffers_setup(&data->ctx, tx_bufs, rx_bufs, 1); + } + return transceive_dma(dev, spi_cfg, tx_bufs, rx_bufs, async, cb, userdata); } -#endif /* CONFIG_SPI_MCUX_LPSPI_DMA */ - return transceive(dev, spi_cfg, tx_bufs, rx_bufs, false, NULL, NULL); + if (IS_ENABLED(CONFIG_SPI_RTIO)) { + return transceive_rtio(dev, spi_cfg, tx_bufs, rx_bufs); + } + + return transceive(dev, spi_cfg, tx_bufs, rx_bufs, async, cb, userdata); +} + +static int spi_mcux_transceive_sync(const struct device *dev, const struct spi_config *spi_cfg, + const struct spi_buf_set *tx_bufs, + const struct spi_buf_set *rx_bufs) +{ + return spi_mcux_transceive(dev, spi_cfg, tx_bufs, rx_bufs, NULL, NULL, false); } #ifdef CONFIG_SPI_ASYNC @@ -724,17 +736,7 @@ static int spi_mcux_transceive_async(const struct device *dev, const struct spi_ const struct spi_buf_set *rx_bufs, spi_callback_t cb, void *userdata) { -#ifdef CONFIG_SPI_MCUX_LPSPI_DMA - struct spi_mcux_data *data = dev->data; - - if (lpspi_inst_has_dma(data)) { - spi_context_buffers_setup(&data->ctx, tx_bufs, rx_bufs, 1); - } - - return transceive_dma(dev, spi_cfg, tx_bufs, rx_bufs, true, cb, userdata); -#else - return transceive(dev, spi_cfg, tx_bufs, rx_bufs, true, cb, userdata); -#endif /* CONFIG_SPI_MCUX_LPSPI_DMA */ + return spi_mcux_transceive(dev, spi_cfg, tx_bufs, rx_bufs, cb, userdata, true); } #endif /* CONFIG_SPI_ASYNC */ @@ -748,7 +750,7 @@ static int spi_mcux_release(const struct device *dev, const struct spi_config *s } static const struct spi_driver_api spi_mcux_driver_api = { - .transceive = spi_mcux_transceive, + .transceive = spi_mcux_transceive_sync, #ifdef CONFIG_SPI_ASYNC .transceive_async = spi_mcux_transceive_async, #endif From 6c3db028d48d05f2420eb3c8601d1ed85c8c716b Mon Sep 17 00:00:00 2001 From: Declan Snyder Date: Fri, 20 Sep 2024 17:39:10 -0500 Subject: [PATCH 10/18] drivers: spi_mcux_lpspi: Remove duplicate bufsetup This line doesn't need to be in multiple places. Signed-off-by: Declan Snyder --- drivers/spi/spi_mcux_lpspi.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/spi/spi_mcux_lpspi.c b/drivers/spi/spi_mcux_lpspi.c index 158f79ff13a18..8c0c0125fcc17 100644 --- a/drivers/spi/spi_mcux_lpspi.c +++ b/drivers/spi/spi_mcux_lpspi.c @@ -487,8 +487,9 @@ static int transceive_dma(const struct device *dev, const struct spi_config *spi /* DMA is fast enough watermarks are not required */ LPSPI_SetFifoWatermarks(base, 0U, 0U); + spi_context_buffers_setup(&data->ctx, tx_bufs, rx_bufs, 1); + if (!asynchronous) { - spi_context_buffers_setup(&data->ctx, tx_bufs, rx_bufs, 1); spi_context_cs_control(&data->ctx, true); /* Send each spi buf via DMA, updating context as DMA completes */ @@ -710,9 +711,6 @@ static int spi_mcux_transceive(const struct device *dev, const struct spi_config struct spi_mcux_data *data = dev->data; if (lpspi_inst_has_dma(data)) { - if (async) { - spi_context_buffers_setup(&data->ctx, tx_bufs, rx_bufs, 1); - } return transceive_dma(dev, spi_cfg, tx_bufs, rx_bufs, async, cb, userdata); } From 588f7499686c1cbb414d05613cb951b42ca30c2f Mon Sep 17 00:00:00 2001 From: Declan Snyder Date: Fri, 20 Sep 2024 17:46:07 -0500 Subject: [PATCH 11/18] drivers: spi_mcux_lpspi: Remove commented code Remove lines of code that are commented out. Signed-off-by: Declan Snyder --- drivers/spi/spi_mcux_lpspi.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/drivers/spi/spi_mcux_lpspi.c b/drivers/spi/spi_mcux_lpspi.c index 8c0c0125fcc17..477c470429f25 100644 --- a/drivers/spi/spi_mcux_lpspi.c +++ b/drivers/spi/spi_mcux_lpspi.c @@ -122,7 +122,6 @@ static void spi_mcux_master_callback(LPSPI_Type *base, lpspi_master_handle_t *ha static int spi_mcux_transfer_next_packet(const struct device *dev) { - /* const struct spi_mcux_config *config = dev->config; */ struct spi_mcux_data *data = dev->data; LPSPI_Type *base = (LPSPI_Type *)DEVICE_MMIO_NAMED_GET(dev, reg_base); struct spi_context *ctx = &data->ctx; @@ -324,7 +323,6 @@ static void spi_mcux_dma_callback(const struct device *dev, void *arg, uint32_t static int spi_mcux_dma_tx_load(const struct device *dev, const uint8_t *buf, size_t len) { - /* const struct spi_mcux_config *cfg = dev->config; */ struct spi_mcux_data *data = dev->data; struct dma_block_config *blk_cfg; LPSPI_Type *base = (LPSPI_Type *)DEVICE_MMIO_NAMED_GET(dev, reg_base); @@ -365,7 +363,6 @@ static int spi_mcux_dma_tx_load(const struct device *dev, const uint8_t *buf, si static int spi_mcux_dma_rx_load(const struct device *dev, uint8_t *buf, size_t len) { - /*const struct spi_mcux_config *cfg = dev->config; */ struct spi_mcux_data *data = dev->data; struct dma_block_config *blk_cfg; LPSPI_Type *base = (LPSPI_Type *)DEVICE_MMIO_NAMED_GET(dev, reg_base); @@ -462,7 +459,6 @@ static int transceive_dma(const struct device *dev, const struct spi_config *spi const struct spi_buf_set *tx_bufs, const struct spi_buf_set *rx_bufs, bool asynchronous, spi_callback_t cb, void *userdata) { - /* const struct spi_mcux_config *config = dev->config; */ struct spi_mcux_data *data = dev->data; LPSPI_Type *base = (LPSPI_Type *)DEVICE_MMIO_NAMED_GET(dev, reg_base); int ret; From ff946d3548a748adff393c13921f37e7897a74e9 Mon Sep 17 00:00:00 2001 From: Declan Snyder Date: Fri, 20 Sep 2024 18:03:34 -0500 Subject: [PATCH 12/18] drivers: spi_mcux_lpspi: Clean up next_packet func Next packet function is way more complicated than it needs to be. Clean it up by simplifying the code and making readable helper functions. Signed-off-by: Declan Snyder --- drivers/spi/spi_mcux_lpspi.c | 48 +++++++----------------------------- 1 file changed, 9 insertions(+), 39 deletions(-) diff --git a/drivers/spi/spi_mcux_lpspi.c b/drivers/spi/spi_mcux_lpspi.c index 477c470429f25..e4a2d8e68bbb1 100644 --- a/drivers/spi/spi_mcux_lpspi.c +++ b/drivers/spi/spi_mcux_lpspi.c @@ -125,53 +125,23 @@ static int spi_mcux_transfer_next_packet(const struct device *dev) struct spi_mcux_data *data = dev->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 ((ctx->tx_len == 0) && (ctx->rx_len == 0)) { - /* nothing left to rx or tx, we're done! */ - spi_context_cs_control(&data->ctx, false); - spi_context_complete(&data->ctx, dev, 0); + if (max_chunk == 0) { + spi_context_cs_control(ctx, false); + spi_context_complete(ctx, dev, 0); return 0; } + data->transfer_len = max_chunk; + transfer.configFlags = kLPSPI_MasterPcsContinuous | (ctx->config->slave << LPSPI_MASTER_PCS_SHIFT); - - if (ctx->tx_len == 0) { - /* rx only, nothing to tx */ - transfer.txData = NULL; - transfer.rxData = ctx->rx_buf; - transfer.dataSize = ctx->rx_len; - } else if (ctx->rx_len == 0) { - /* tx only, nothing to rx */ - transfer.txData = (uint8_t *)ctx->tx_buf; - transfer.rxData = NULL; - transfer.dataSize = ctx->tx_len; - } else if (ctx->tx_len == ctx->rx_len) { - /* rx and tx are the same length */ - transfer.txData = (uint8_t *)ctx->tx_buf; - transfer.rxData = ctx->rx_buf; - transfer.dataSize = ctx->tx_len; - } else if (ctx->tx_len > ctx->rx_len) { - /* Break up the tx into multiple transfers so we don't have to - * rx into a longer intermediate buffer. Leave chip select - * active between transfers. - */ - transfer.txData = (uint8_t *)ctx->tx_buf; - transfer.rxData = ctx->rx_buf; - transfer.dataSize = ctx->rx_len; - } else { - /* Break up the rx into multiple transfers so we don't have to - * tx from a longer intermediate buffer. Leave chip select - * active between transfers. - */ - transfer.txData = (uint8_t *)ctx->tx_buf; - transfer.rxData = ctx->rx_buf; - transfer.dataSize = ctx->tx_len; - } - - data->transfer_len = transfer.dataSize; + 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, &data->handle, &transfer); if (status != kStatus_Success) { From debbf3b89c0f54897fc81da139a93a7661ba7720 Mon Sep 17 00:00:00 2001 From: Declan Snyder Date: Fri, 20 Sep 2024 18:18:05 -0500 Subject: [PATCH 13/18] drivers: spi_mcux_lpspi: Macro for master cfg flag All the master transfer seemed to use the same config flags, reuse with a macro. Signed-off-by: Declan Snyder --- drivers/spi/spi_mcux_lpspi.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/spi/spi_mcux_lpspi.c b/drivers/spi/spi_mcux_lpspi.c index e4a2d8e68bbb1..55f28cbf9e9da 100644 --- a/drivers/spi/spi_mcux_lpspi.c +++ b/drivers/spi/spi_mcux_lpspi.c @@ -39,6 +39,10 @@ LOG_MODULE_REGISTER(spi_mcux_lpspi, CONFIG_SPI_LOG_LEVEL); /* Argument to MCUX SDK IRQ handler */ #define LPSPI_IRQ_HANDLE_ARG COND_CODE_1(CONFIG_NXP_LP_FLEXCOMM, (LPSPI_GetInstance(base)), (base)) +/* flag for SDK API for master transfers */ +#define LPSPI_MASTER_XFER_CFG_FLAGS(slave) \ + kLPSPI_MasterPcsContinuous | (slave << LPSPI_MASTER_PCS_SHIFT) + #ifdef CONFIG_SPI_MCUX_LPSPI_DMA #include @@ -137,8 +141,7 @@ static int spi_mcux_transfer_next_packet(const struct device *dev) data->transfer_len = max_chunk; - transfer.configFlags = - kLPSPI_MasterPcsContinuous | (ctx->config->slave << LPSPI_MASTER_PCS_SHIFT); + 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; @@ -550,8 +553,7 @@ static void spi_mcux_iodev_start(const struct device *dev) lpspi_transfer_t transfer; status_t status; - transfer.configFlags = - kLPSPI_MasterPcsContinuous | (spi_cfg->slave << LPSPI_MASTER_PCS_SHIFT); + transfer.configFlags = LPSPI_MASTER_XFER_CFG_FLAGS(spi_cfg->slave); switch (sqe->op) { case RTIO_OP_RX: From b006783f9ba253f37f76dc1023ac79ba99f4407a Mon Sep 17 00:00:00 2001 From: Declan Snyder Date: Fri, 20 Sep 2024 18:29:17 -0500 Subject: [PATCH 14/18] drivers: spi_mcux_lpspi: ErrLog dev in iodev_start Log the device name and status if the transfer could not start, same way as in non-RTIO path. Signed-off-by: Declan Snyder --- drivers/spi/spi_mcux_lpspi.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/spi/spi_mcux_lpspi.c b/drivers/spi/spi_mcux_lpspi.c index 55f28cbf9e9da..b953476e03207 100644 --- a/drivers/spi/spi_mcux_lpspi.c +++ b/drivers/spi/spi_mcux_lpspi.c @@ -548,7 +548,6 @@ static void spi_mcux_iodev_start(const struct device *dev) 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; @@ -586,7 +585,7 @@ static void spi_mcux_iodev_start(const struct device *dev) status = LPSPI_MasterTransferNonBlocking(base, &data->handle, &transfer); if (status != kStatus_Success) { - LOG_ERR("Transfer could not start"); + LOG_ERR("Transfer could not start on %s: %d", dev->name, status); spi_mcux_iodev_complete(dev, -EIO); } } From a04e110834527c36c11f08e9c8afd05764f086ef Mon Sep 17 00:00:00 2001 From: Declan Snyder Date: Fri, 20 Sep 2024 18:41:40 -0500 Subject: [PATCH 15/18] drivers: spi_mcux_lpspi: Remove rtio prepare_start Remove unneeded prepare_start function since all uses of it are paired with the start function. Also, probably should not mess with chip select before deciding to do the transfer. And just return on some error like the rest of the driver does instead of assert. Signed-off-by: Declan Snyder --- drivers/spi/spi_mcux_lpspi.c | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/drivers/spi/spi_mcux_lpspi.c b/drivers/spi/spi_mcux_lpspi.c index b953476e03207..65d11f735a0c8 100644 --- a/drivers/spi/spi_mcux_lpspi.c +++ b/drivers/spi/spi_mcux_lpspi.c @@ -527,20 +527,6 @@ static int transceive_dma(const struct device *dev, const struct spi_config *spi #endif /* CONFIG_SPI_MCUX_LPSPI_DMA */ #ifdef CONFIG_SPI_RTIO -static inline void spi_mcux_iodev_prepare_start(const struct device *dev) -{ - struct spi_mcux_data *data = dev->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; - - err = spi_mcux_configure(dev, spi_config); - __ASSERT(!err, "%d", err); - - spi_context_cs_control(&data->ctx, true); -} - static void spi_mcux_iodev_start(const struct device *dev) { struct spi_mcux_data *data = dev->data; @@ -552,6 +538,12 @@ static void spi_mcux_iodev_start(const struct device *dev) lpspi_transfer_t transfer; status_t status; + status = spi_mcux_configure(dev, spi_cfg); + if (status) { + LOG_ERR("Error configuring lpspi"); + return; + } + transfer.configFlags = LPSPI_MASTER_XFER_CFG_FLAGS(spi_cfg->slave); switch (sqe->op) { @@ -583,6 +575,8 @@ static void spi_mcux_iodev_start(const struct device *dev) data->transfer_len = transfer.dataSize; + spi_context_cs_control(&data->ctx, true); + status = LPSPI_MasterTransferNonBlocking(base, &data->handle, &transfer); if (status != kStatus_Success) { LOG_ERR("Transfer could not start on %s: %d", dev->name, status); @@ -605,7 +599,6 @@ static void spi_mcux_iodev_complete(const struct device *dev, int status) spi_context_cs_control(&data->ctx, false); if (spi_rtio_complete(rtio_ctx, status)) { - spi_mcux_iodev_prepare_start(dev); spi_mcux_iodev_start(dev); } } @@ -616,7 +609,6 @@ static void spi_mcux_iodev_submit(const struct device *dev, struct rtio_iodev_sq 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); } } From ba495460d9f4c030370eac729cf43b8ce5e117cd Mon Sep 17 00:00:00 2001 From: Declan Snyder Date: Fri, 20 Sep 2024 18:49:06 -0500 Subject: [PATCH 16/18] drivers: spi_mcux_lpspi: Clean up configure func Do all input validation etc BEFORE setting up configuration, instead of mixing these things everywhere. Also condense the formatting of the code. Signed-off-by: Declan Snyder --- drivers/spi/spi_mcux_lpspi.c | 51 +++++++++++++++--------------------- 1 file changed, 21 insertions(+), 30 deletions(-) diff --git a/drivers/spi/spi_mcux_lpspi.c b/drivers/spi/spi_mcux_lpspi.c index 65d11f735a0c8..6c40b18b95937 100644 --- a/drivers/spi/spi_mcux_lpspi.c +++ b/drivers/spi/spi_mcux_lpspi.c @@ -160,49 +160,25 @@ static int spi_mcux_configure(const struct device *dev, const struct spi_config const struct spi_mcux_config *config = dev->config; struct spi_mcux_data *data = dev->data; LPSPI_Type *base = (LPSPI_Type *)DEVICE_MMIO_NAMED_GET(dev, reg_base); + uint32_t word_size = SPI_WORD_SIZE_GET(spi_cfg->operation); lpspi_master_config_t master_config; uint32_t clock_freq; - uint32_t word_size; if (spi_cfg->operation & SPI_HALF_DUPLEX) { LOG_ERR("Half-duplex not supported"); return -ENOTSUP; } - LPSPI_MasterGetDefaultConfig(&master_config); - if (spi_cfg->slave > CHIP_SELECT_COUNT) { LOG_ERR("Slave %d is greater than %d", spi_cfg->slave, CHIP_SELECT_COUNT); return -EINVAL; } - word_size = SPI_WORD_SIZE_GET(spi_cfg->operation); if (word_size > MAX_DATA_WIDTH) { LOG_ERR("Word size %d is greater than %d", word_size, MAX_DATA_WIDTH); return -EINVAL; } - master_config.bitsPerFrame = word_size; - - master_config.cpol = (SPI_MODE_GET(spi_cfg->operation) & SPI_MODE_CPOL) - ? kLPSPI_ClockPolarityActiveLow - : kLPSPI_ClockPolarityActiveHigh; - - master_config.cpha = (SPI_MODE_GET(spi_cfg->operation) & SPI_MODE_CPHA) - ? kLPSPI_ClockPhaseSecondEdge - : kLPSPI_ClockPhaseFirstEdge; - - master_config.direction = - (spi_cfg->operation & SPI_TRANSFER_LSB) ? kLPSPI_LsbFirst : kLPSPI_MsbFirst; - - master_config.baudRate = spi_cfg->frequency; - - master_config.pcsToSckDelayInNanoSec = config->pcs_sck_delay; - master_config.lastSckToPcsDelayInNanoSec = config->sck_pcs_delay; - master_config.betweenTransferDelayInNanoSec = config->transfer_delay; - - master_config.pinCfg = config->data_pin_config; - if (!device_is_ready(config->clock_dev)) { LOG_ERR("clock control device not ready"); return -ENODEV; @@ -226,17 +202,32 @@ static int spi_mcux_configure(const struct device *dev, const struct spi_config } } - LPSPI_MasterInit(base, &master_config, clock_freq); - if (IS_ENABLED(CONFIG_DEBUG)) { base->CR |= LPSPI_CR_DBGEN_MASK; } - LPSPI_MasterTransferCreateHandle(base, &data->handle, spi_mcux_master_callback, data); + data->ctx.config = spi_cfg; - LPSPI_SetDummyData(base, 0); + LPSPI_MasterGetDefaultConfig(&master_config); - data->ctx.config = spi_cfg; + master_config.bitsPerFrame = word_size; + master_config.cpol = (SPI_MODE_GET(spi_cfg->operation) & SPI_MODE_CPOL) + ? kLPSPI_ClockPolarityActiveLow + : kLPSPI_ClockPolarityActiveHigh; + master_config.cpha = (SPI_MODE_GET(spi_cfg->operation) & SPI_MODE_CPHA) + ? kLPSPI_ClockPhaseSecondEdge + : kLPSPI_ClockPhaseFirstEdge; + master_config.direction = + (spi_cfg->operation & SPI_TRANSFER_LSB) ? kLPSPI_LsbFirst : kLPSPI_MsbFirst; + master_config.baudRate = spi_cfg->frequency; + master_config.pcsToSckDelayInNanoSec = config->pcs_sck_delay; + master_config.lastSckToPcsDelayInNanoSec = config->sck_pcs_delay; + master_config.betweenTransferDelayInNanoSec = config->transfer_delay; + master_config.pinCfg = config->data_pin_config; + + LPSPI_MasterInit(base, &master_config, clock_freq); + LPSPI_MasterTransferCreateHandle(base, &data->handle, spi_mcux_master_callback, data); + LPSPI_SetDummyData(base, 0); return 0; } From 700ef5e0070e2d73d18a0902b11dbc747c96e17d Mon Sep 17 00:00:00 2001 From: Declan Snyder Date: Fri, 20 Sep 2024 19:09:27 -0500 Subject: [PATCH 17/18] drivers: spi_mcux_lpspi: Optimize dma callback Optimize DMA callback by cleaning up the code and storing less debug logging strings. Signed-off-by: Declan Snyder --- drivers/spi/spi_mcux_lpspi.c | 52 +++++++++++++++++++----------------- 1 file changed, 27 insertions(+), 25 deletions(-) diff --git a/drivers/spi/spi_mcux_lpspi.c b/drivers/spi/spi_mcux_lpspi.c index 6c40b18b95937..0bec063c0a2cd 100644 --- a/drivers/spi/spi_mcux_lpspi.c +++ b/drivers/spi/spi_mcux_lpspi.c @@ -244,44 +244,46 @@ static void spi_mcux_dma_callback(const struct device *dev, void *arg, uint32_t /* arg directly holds the spi device */ const struct device *spi_dev = arg; struct spi_mcux_data *data = (struct spi_mcux_data *)spi_dev->data; + char debug_char; if (status < 0) { - LOG_ERR("DMA callback error with channel %d.", channel); - data->status_flags |= LPSPI_DMA_ERROR_FLAG; + goto error; + } + + /* identify the origin of this callback */ + if (channel == data->dma_tx.channel) { + /* this part of the transfer ends */ + data->status_flags |= LPSPI_DMA_TX_DONE_FLAG; + debug_char = 'T'; + } else if (channel == data->dma_rx.channel) { + /* this part of the transfer ends */ + data->status_flags |= LPSPI_DMA_RX_DONE_FLAG; + debug_char = 'R'; } else { - /* identify the origin of this callback */ - if (channel == data->dma_tx.channel) { - /* this part of the transfer ends */ - data->status_flags |= LPSPI_DMA_TX_DONE_FLAG; - LOG_DBG("DMA TX Block Complete"); - } else if (channel == data->dma_rx.channel) { - /* this part of the transfer ends */ - data->status_flags |= LPSPI_DMA_RX_DONE_FLAG; - LOG_DBG("DMA RX Block Complete"); - } else { - LOG_ERR("DMA callback channel %d is not valid.", channel); - data->status_flags |= LPSPI_DMA_ERROR_FLAG; - } + goto error; } + + LOG_DBG("DMA %cX Block Complete", debug_char); + #if CONFIG_SPI_ASYNC - if (data->ctx.asynchronous && - ((data->status_flags & LPSPI_DMA_DONE_FLAG) == LPSPI_DMA_DONE_FLAG)) { + if (data->ctx.asynchronous && (data->status_flags & LPSPI_DMA_DONE_FLAG)) { /* Load dma blocks of equal length */ - size_t dma_size = MIN(data->ctx.tx_len, data->ctx.rx_len); + size_t dma_size = spi_context_max_continuous_chunk(data->ctx); - if (dma_size == 0) { - dma_size = MAX(data->ctx.tx_len, data->ctx.rx_len); + if (dma_size != 0) { + return; } spi_context_update_tx(&data->ctx, 1, dma_size); spi_context_update_rx(&data->ctx, 1, dma_size); - - if (data->ctx.tx_len == 0 && data->ctx.rx_len == 0) { - spi_context_complete(&data->ctx, spi_dev, 0); - } - return; } #endif + + goto done; +error: + LOG_ERR("DMA callback error with channel %d.", channel); + data->status_flags |= LPSPI_DMA_ERROR_FLAG; +done: spi_context_complete(&data->ctx, spi_dev, 0); } From bcb563bb7e436a5da34d95ffe5d30a24d16c3566 Mon Sep 17 00:00:00 2001 From: Declan Snyder Date: Fri, 20 Sep 2024 20:11:55 -0500 Subject: [PATCH 18/18] drivers: spi_mcux_lpspi: Clean up DMA path Clean up DMA path of code. Signed-off-by: Declan Snyder --- drivers/spi/spi_mcux_lpspi.c | 269 ++++++++++++++++++----------------- 1 file changed, 140 insertions(+), 129 deletions(-) diff --git a/drivers/spi/spi_mcux_lpspi.c b/drivers/spi/spi_mcux_lpspi.c index 0bec063c0a2cd..4795340020285 100644 --- a/drivers/spi/spi_mcux_lpspi.c +++ b/drivers/spi/spi_mcux_lpspi.c @@ -86,9 +86,7 @@ struct spi_mcux_data { struct spi_dma_stream dma_rx; struct spi_dma_stream dma_tx; /* dummy value used for transferring NOP when tx buf is null */ - uint32_t dummy_tx_buffer; - /* dummy value used to read RX data into when rx buf is null */ - uint32_t dummy_rx_buffer; + uint32_t dummy_buffer; #endif }; @@ -287,231 +285,244 @@ static void spi_mcux_dma_callback(const struct device *dev, void *arg, uint32_t spi_context_complete(&data->ctx, spi_dev, 0); } -static int spi_mcux_dma_tx_load(const struct device *dev, const uint8_t *buf, size_t len) +static struct dma_block_config *spi_mcux_dma_common_load(struct spi_dma_stream *stream, + const struct device *dev, + const uint8_t *buf, size_t len) { struct spi_mcux_data *data = dev->data; - struct dma_block_config *blk_cfg; - LPSPI_Type *base = (LPSPI_Type *)DEVICE_MMIO_NAMED_GET(dev, reg_base); - - /* remember active TX DMA channel (used in callback) */ - struct spi_dma_stream *stream = &data->dma_tx; - - blk_cfg = &stream->dma_blk_cfg; + struct dma_block_config *blk_cfg = &stream->dma_blk_cfg; /* prepare the block for this TX DMA channel */ memset(blk_cfg, 0, sizeof(struct dma_block_config)); + blk_cfg->block_size = len; + if (buf == NULL) { - /* Treat the transfer as a peripheral to peripheral one, so that DMA - * reads from this address each time - */ - blk_cfg->source_address = (uint32_t)&data->dummy_tx_buffer; + blk_cfg->source_address = (uint32_t)&data->dummy_buffer; + blk_cfg->dest_address = (uint32_t)&data->dummy_buffer; + /* pretend it is peripheral xfer so DMA just xfer to dummy buf */ stream->dma_cfg.channel_direction = PERIPHERAL_TO_PERIPHERAL; } else { - /* tx direction has memory as source and periph as dest. */ blk_cfg->source_address = (uint32_t)buf; + blk_cfg->dest_address = (uint32_t)buf; + } + + /* Transfer 1 byte each DMA loop */ + stream->dma_cfg.source_burst_length = 1; + stream->dma_cfg.user_data = (void *)dev; + stream->dma_cfg.head_block = blk_cfg; + + return blk_cfg; +} + +static int spi_mcux_dma_tx_load(const struct device *dev, const uint8_t *buf, size_t len) +{ + LPSPI_Type *base = (LPSPI_Type *)DEVICE_MMIO_NAMED_GET(dev, reg_base); + struct spi_mcux_data *data = dev->data; + /* remember active TX DMA channel (used in callback) */ + struct spi_dma_stream *stream = &data->dma_tx; + struct dma_block_config *blk_cfg = spi_mcux_dma_common_load(stream, dev, buf, len); + + if (buf != NULL) { + /* tx direction has memory as source and periph as dest. */ stream->dma_cfg.channel_direction = MEMORY_TO_PERIPHERAL; } - /* Enable scatter/gather */ - blk_cfg->source_gather_en = 1; + /* Dest is LPSPI tx fifo */ blk_cfg->dest_address = LPSPI_GetTxRegisterAddress(base); - blk_cfg->block_size = len; - /* Transfer 1 byte each DMA loop */ - stream->dma_cfg.source_burst_length = 1; - stream->dma_cfg.head_block = &stream->dma_blk_cfg; /* give the client dev as arg, as the callback comes from the dma */ - stream->dma_cfg.user_data = (struct device *)dev; /* pass our client origin to the dma: data->dma_tx.dma_channel */ - return dma_config(data->dma_tx.dma_dev, data->dma_tx.channel, &stream->dma_cfg); + return dma_config(stream->dma_dev, stream->channel, &stream->dma_cfg); } static int spi_mcux_dma_rx_load(const struct device *dev, uint8_t *buf, size_t len) { - struct spi_mcux_data *data = dev->data; - struct dma_block_config *blk_cfg; LPSPI_Type *base = (LPSPI_Type *)DEVICE_MMIO_NAMED_GET(dev, reg_base); - + struct spi_mcux_data *data = dev->data; /* retrieve active RX DMA channel (used in callback) */ struct spi_dma_stream *stream = &data->dma_rx; + struct dma_block_config *blk_cfg = spi_mcux_dma_common_load(stream, dev, buf, len); - blk_cfg = &stream->dma_blk_cfg; - - /* prepare the block for this RX DMA channel */ - memset(blk_cfg, 0, sizeof(struct dma_block_config)); - - if (buf == NULL) { - /* Treat the transfer as a peripheral to peripheral one, so that DMA - * reads from this address each time - */ - blk_cfg->dest_address = (uint32_t)&data->dummy_rx_buffer; - stream->dma_cfg.channel_direction = PERIPHERAL_TO_PERIPHERAL; - } else { + if (buf != NULL) { /* rx direction has periph as source and mem as dest. */ - blk_cfg->dest_address = (uint32_t)buf; stream->dma_cfg.channel_direction = PERIPHERAL_TO_MEMORY; } - blk_cfg->block_size = len; - /* Enable scatter/gather */ - blk_cfg->dest_scatter_en = 1; + /* Source is LPSPI rx fifo */ blk_cfg->source_address = LPSPI_GetRxRegisterAddress(base); - stream->dma_cfg.source_burst_length = 1; - - stream->dma_cfg.head_block = blk_cfg; - stream->dma_cfg.user_data = (struct device *)dev; /* pass our client origin to the dma: data->dma_rx.channel */ - return dma_config(data->dma_rx.dma_dev, data->dma_rx.channel, &stream->dma_cfg); + return dma_config(stream->dma_dev, stream->channel, &stream->dma_cfg); } static int wait_dma_rx_tx_done(const struct device *dev) { struct spi_mcux_data *data = dev->data; - int ret = -1; + int ret; - while (1) { + do { ret = spi_context_wait_for_completion(&data->ctx); if (ret) { LOG_DBG("Timed out waiting for SPI context to complete"); return ret; - } - if (data->status_flags & LPSPI_DMA_ERROR_FLAG) { + } else if (data->status_flags & LPSPI_DMA_ERROR_FLAG) { return -EIO; } + } while (!((data->status_flags & LPSPI_DMA_DONE_FLAG) == LPSPI_DMA_DONE_FLAG)); - if ((data->status_flags & LPSPI_DMA_DONE_FLAG) == LPSPI_DMA_DONE_FLAG) { - LOG_DBG("DMA block completed"); - return 0; - } - } + LOG_DBG("DMA block completed"); + return 0; } static inline int spi_mcux_dma_rxtx_load(const struct device *dev, size_t *dma_size) { - struct spi_mcux_data *lpspi_data = dev->data; + struct spi_mcux_data *data = dev->data; + struct spi_context *ctx = &data->ctx; int ret = 0; /* Clear status flags */ - lpspi_data->status_flags = 0U; + data->status_flags = 0U; + /* Load dma blocks of equal length */ - *dma_size = MIN(lpspi_data->ctx.tx_len, lpspi_data->ctx.rx_len); - if (*dma_size == 0) { - *dma_size = MAX(lpspi_data->ctx.tx_len, lpspi_data->ctx.rx_len); - } + *dma_size = spi_context_max_continuous_chunk(ctx); - ret = spi_mcux_dma_tx_load(dev, lpspi_data->ctx.tx_buf, *dma_size); + ret = spi_mcux_dma_tx_load(dev, ctx->tx_buf, *dma_size); if (ret != 0) { return ret; } - ret = spi_mcux_dma_rx_load(dev, lpspi_data->ctx.rx_buf, *dma_size); + ret = spi_mcux_dma_rx_load(dev, ctx->rx_buf, *dma_size); if (ret != 0) { return ret; } /* Start DMA */ - ret = dma_start(lpspi_data->dma_tx.dma_dev, lpspi_data->dma_tx.channel); + ret = dma_start(data->dma_tx.dma_dev, data->dma_tx.channel); if (ret != 0) { return ret; } - ret = dma_start(lpspi_data->dma_rx.dma_dev, lpspi_data->dma_rx.channel); + ret = dma_start(data->dma_rx.dma_dev, data->dma_rx.channel); return ret; } -static int transceive_dma(const struct device *dev, const struct spi_config *spi_cfg, - const struct spi_buf_set *tx_bufs, const struct spi_buf_set *rx_bufs, - bool asynchronous, spi_callback_t cb, void *userdata) +#ifdef CONFIG_SPI_ASYNC +static int transceive_dma_async(const struct device *dev, spi_callback_t cb, void *userdata) { struct spi_mcux_data *data = dev->data; - LPSPI_Type *base = (LPSPI_Type *)DEVICE_MMIO_NAMED_GET(dev, reg_base); - int ret; + struct spi_context *ctx = &data->ctx; size_t dma_size; + int ret; - if (!asynchronous) { - spi_context_lock(&data->ctx, asynchronous, cb, userdata, spi_cfg); - } + ctx->asynchronous = true; + ctx->callback = cb; + ctx->callback_data = userdata; - ret = spi_mcux_configure(dev, spi_cfg); + ret = spi_mcux_dma_rxtx_load(dev, &dma_size); if (ret) { - if (!asynchronous) { - spi_context_release(&data->ctx, ret); - } return ret; } -#ifdef CONFIG_SOC_SERIES_MCXN - base->TCR |= LPSPI_TCR_CONT_MASK; -#endif + /* Enable DMA Requests */ + LPSPI_EnableDMA(base, kLPSPI_TxDmaEnable | kLPSPI_RxDmaEnable); - /* DMA is fast enough watermarks are not required */ - LPSPI_SetFifoWatermarks(base, 0U, 0U); + return 0; +} +#else +#define transceive_dma_async(...) 0 +#endif /* CONFIG_SPI_ASYNC */ - spi_context_buffers_setup(&data->ctx, tx_bufs, rx_bufs, 1); +static int transceive_dma_sync(const struct device *dev) +{ + LPSPI_Type *base = (LPSPI_Type *)DEVICE_MMIO_NAMED_GET(dev, reg_base); + struct spi_mcux_data *data = dev->data; + struct spi_context *ctx = &data->ctx; + size_t dma_size; + int ret; - if (!asynchronous) { - spi_context_cs_control(&data->ctx, true); + spi_context_cs_control(ctx, true); - /* Send each spi buf via DMA, updating context as DMA completes */ - while (data->ctx.rx_len > 0 || data->ctx.tx_len > 0) { - /* Load dma block */ - ret = spi_mcux_dma_rxtx_load(dev, &dma_size); - if (ret != 0) { - goto out; - } + /* Send each spi buf via DMA, updating context as DMA completes */ + while (ctx->rx_len > 0 || ctx->tx_len > 0) { + /* Load dma block */ + ret = spi_mcux_dma_rxtx_load(dev, &dma_size); + if (ret) { + return ret; + } #ifdef CONFIG_SOC_SERIES_MCXN - while (!(LPSPI_GetStatusFlags(base) & kLPSPI_TxDataRequestFlag)) { - /* wait until previous tx finished */ - } + while (!(LPSPI_GetStatusFlags(base) & kLPSPI_TxDataRequestFlag)) { + /* wait until previous tx finished */ + } #endif - /* Enable DMA Requests */ - LPSPI_EnableDMA(base, kLPSPI_TxDmaEnable | kLPSPI_RxDmaEnable); + /* Enable DMA Requests */ + LPSPI_EnableDMA(base, kLPSPI_TxDmaEnable | kLPSPI_RxDmaEnable); - /* Wait for DMA to finish */ - ret = wait_dma_rx_tx_done(dev); - if (ret != 0) { - goto out; - } + /* Wait for DMA to finish */ + ret = wait_dma_rx_tx_done(dev); + if (ret) { + return ret; + } #ifndef CONFIG_SOC_SERIES_MCXN - while ((LPSPI_GetStatusFlags(base) & kLPSPI_ModuleBusyFlag)) { - /* wait until module is idle */ - } + while ((LPSPI_GetStatusFlags(base) & kLPSPI_ModuleBusyFlag)) { + /* wait until module is idle */ + } #endif - /* Disable DMA */ - LPSPI_DisableDMA(base, kLPSPI_TxDmaEnable | kLPSPI_RxDmaEnable); - - /* Update SPI contexts with amount of data we just sent */ - spi_context_update_tx(&data->ctx, 1, dma_size); - spi_context_update_rx(&data->ctx, 1, dma_size); - } - spi_context_cs_control(&data->ctx, false); - base->TCR = 0; + /* Disable DMA */ + LPSPI_DisableDMA(base, kLPSPI_TxDmaEnable | kLPSPI_RxDmaEnable); -out: - spi_context_release(&data->ctx, ret); + /* Update SPI contexts with amount of data we just sent */ + spi_context_update_tx(ctx, 1, dma_size); + spi_context_update_rx(ctx, 1, dma_size); } -#if CONFIG_SPI_ASYNC - else { - data->ctx.asynchronous = asynchronous; - data->ctx.callback = cb; - data->ctx.callback_data = userdata; - ret = spi_mcux_dma_rxtx_load(dev, &dma_size); - if (ret != 0) { - goto out; - } + spi_context_cs_control(ctx, false); - /* Enable DMA Requests */ - LPSPI_EnableDMA(base, kLPSPI_TxDmaEnable | kLPSPI_RxDmaEnable); + base->TCR = 0; + + return 0; +} + +static int transceive_dma(const struct device *dev, const struct spi_config *spi_cfg, + const struct spi_buf_set *tx_bufs, const struct spi_buf_set *rx_bufs, + bool asynchronous, spi_callback_t cb, void *userdata) +{ + struct spi_mcux_data *data = dev->data; + LPSPI_Type *base = (LPSPI_Type *)DEVICE_MMIO_NAMED_GET(dev, reg_base); + int ret; + + if (!asynchronous) { + spi_context_lock(&data->ctx, asynchronous, cb, userdata, spi_cfg); + } + + ret = spi_mcux_configure(dev, spi_cfg); + if (ret && !asynchronous) { + goto out; + } else if (ret) { + return ret; } + +#ifdef CONFIG_SOC_SERIES_MCXN + base->TCR |= LPSPI_TCR_CONT_MASK; #endif + /* DMA is fast enough watermarks are not required */ + LPSPI_SetFifoWatermarks(base, 0U, 0U); + + spi_context_buffers_setup(&data->ctx, tx_bufs, rx_bufs, 1); + + if (asynchronous) { + ret = transceive_dma_async(dev, cb, userdata); + } else { + ret = transceive_dma_sync(dev); + } + +out: + spi_context_release(&data->ctx, ret); return ret; } #else