From 50000e1eb50a1da516453ab96a3d27a110da40f2 Mon Sep 17 00:00:00 2001 From: Declan Snyder Date: Mon, 19 May 2025 11:06:25 -0500 Subject: [PATCH 01/10] spi_nxp_lpspi: Remove dev pointer from data struct The dev pointer in the data struct is not being used, so remove it. Signed-off-by: Declan Snyder --- drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi.c | 2 +- drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi_common.c | 2 -- drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi_priv.h | 1 - 3 files changed, 1 insertion(+), 4 deletions(-) diff --git a/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi.c b/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi.c index 0b9b31fa54a3..e6d932724042 100644 --- a/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi.c +++ b/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi.c @@ -215,7 +215,7 @@ static inline void lpspi_handle_tx_irq(const struct device *dev) lpspi_data->fill_len -= this_buf_words_sent; } - lpspi_next_tx_fill(data->dev); + lpspi_next_tx_fill(dev); } static inline void lpspi_end_xfer(const struct device *dev) diff --git a/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi_common.c b/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi_common.c index 5cd9d1cde5f0..2eff461a9876 100644 --- a/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi_common.c +++ b/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi_common.c @@ -204,8 +204,6 @@ int spi_nxp_init_common(const struct device *dev) DEVICE_MMIO_NAMED_MAP(dev, reg_base, K_MEM_CACHE_NONE | K_MEM_DIRECT_MAP); - data->dev = dev; - if (!device_is_ready(config->clock_dev)) { LOG_ERR("clock control device not ready"); return -ENODEV; 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 082ee79b7766..82222a0e7b8e 100644 --- a/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi_priv.h +++ b/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi_priv.h @@ -47,7 +47,6 @@ struct lpspi_config { struct lpspi_data { DEVICE_MMIO_NAMED_RAM(reg_base); - const struct device *dev; struct spi_context ctx; void *driver_data; size_t transfer_len; From bc34d1c8d44b9b28d1f435b4bd199e3a6a83e58b Mon Sep 17 00:00:00 2001 From: Declan Snyder Date: Mon, 19 May 2025 14:11:00 -0500 Subject: [PATCH 02/10] spi_nxp_lpspi: Clarify configuration function Clarify at the top of the common lpspi file what is the purpose of the file to be clear to future developers that this file is not supposed to make any assumption about a particular implementation of the zephyr API using the LPSPI, because I imagine it could be very likely that more lpspi implementation will be done in the future to make different tradeoffs than the current two. Also the current two are different enough that we should avoid making assumptions even if they currently hold for both because they might not always, as things change. We should disable interrupt events while configuring the LPSPI regardless of implementation. The specific implementation should enable the interrupts it needs on its own transceive implementation. Also clarify and simplify some code in the configure function. Namely, we no longer need to check if we are already configured to write to registers because a recent commit made it so that we clock the peripheral from the zephyr driver init instead of upon the MasterInit call on the SDK. There is also a redundant CR write which I have removed. Signed-off-by: Declan Snyder --- .../spi/spi_nxp_lpspi/spi_nxp_lpspi_common.c | 36 ++++++++++--------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi_common.c b/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi_common.c index 2eff461a9876..e4ce457a73ab 100644 --- a/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi_common.c +++ b/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi_common.c @@ -4,6 +4,13 @@ * SPDX-License-Identifier: Apache-2.0 */ +/* + * This is a collection of functions that would be useful for any driver for LPSPI. + * This function/file has no knowledge of lpspi usage model by the driver + * beyond basic configuration and should avoid making any assumptions about how + * the driver is going to achieve the zephyr API. + */ + #include LOG_MODULE_REGISTER(spi_lpspi, CONFIG_SPI_LOG_LEVEL); @@ -131,23 +138,20 @@ int spi_mcux_configure(const struct device *dev, const struct spi_config *spi_cf return ret; } - if (already_configured) { - /* Setting the baud rate in LPSPI_MasterInit requires module to be disabled. Only - * disable if already configured, otherwise the clock is not enabled and the - * CR register cannot be written. - */ - LPSPI_Enable(base, false); - while ((base->CR & LPSPI_CR_MEN_MASK) != 0U) { - /* Wait until LPSPI is disabled. Datasheet: - * After writing 0, MEN (Module Enable) remains set until the LPSPI has - * completed the current transfer and is idle. - */ - } + /* specific driver implementation should set up watermarks and interrupts. + * we reset them here to avoid any unexpected events during configuring. + */ + base->FCR = 0; + base->IER = 0; + + /* this is workaround for ERR050456 */ + base->CR |= LPSPI_CR_RST_MASK; + base->CR |= LPSPI_CR_RRF_MASK | LPSPI_CR_RTF_MASK; - /* this is workaround for ERR050456 */ - base->CR |= LPSPI_CR_RST_MASK; - base->CR |= LPSPI_CR_RRF_MASK | LPSPI_CR_RTF_MASK; - base->CR = 0x00U; + /* Setting the baud rate requires module to be disabled. */ + base->CR = 0; + while ((base->CR & LPSPI_CR_MEN_MASK) != 0) { + /* According to datasheet, should wait for this MEN bit to clear once idle */ } data->ctx.config = spi_cfg; From abd21b9520edc9f1f17e0dab46dd7efd5314328d Mon Sep 17 00:00:00 2001 From: Declan Snyder Date: Mon, 19 May 2025 14:32:07 -0500 Subject: [PATCH 03/10] spi_nxp_lpspi: Add version ID to data struct Since I expect that the drivers will need to read this version ID maybe multiple times, instead of repeatedly doing so over the peripheral bus, it is probably worth it to store a byte in RAM representing this version. The behavior of the LPSPI is fairly significantly different between versions. Not enough to warrant separate drivers but enough to need a few workarounds or different code branches depending on this. Also, the interrupt based driver is currently using a wrong macro to read this, and that is a bug. Signed-off-by: Declan Snyder --- drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi.c | 2 +- drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi_common.c | 2 ++ drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi_priv.h | 1 + 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi.c b/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi.c index e6d932724042..bedf112f9c69 100644 --- a/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi.c +++ b/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi.c @@ -276,7 +276,7 @@ static void lpspi_isr(const struct device *dev) } if ((DIV_ROUND_UP(spi_context_rx_len_left(ctx, word_size_bytes), word_size_bytes) == 1) && - (LPSPI_VERID_MAJOR(base->VERID) < 2)) { + (data->major_version < 2)) { /* Due to stalling behavior on older LPSPI, * need to end xfer in order to get last bit clocked out on bus. */ diff --git a/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi_common.c b/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi_common.c index e4ce457a73ab..078a619d2f9c 100644 --- a/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi_common.c +++ b/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi_common.c @@ -215,6 +215,8 @@ int spi_nxp_init_common(const struct device *dev) lpspi_module_system_init(base); + data->major_version = (base->VERID & LPSPI_VERID_MAJOR_MASK) >> LPSPI_VERID_MAJOR_SHIFT; + err = spi_context_cs_configure_all(&data->ctx); if (err < 0) { return err; 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 82222a0e7b8e..c236ed35a499 100644 --- a/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi_priv.h +++ b/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi_priv.h @@ -50,6 +50,7 @@ struct lpspi_data { struct spi_context ctx; void *driver_data; size_t transfer_len; + uint8_t major_version; }; /* verifies spi_cfg validity and set up configuration of hardware for xfer */ From 0197baf6fbdd50c2fe4ad516f6549fec081b8cfa Mon Sep 17 00:00:00 2001 From: Declan Snyder Date: Mon, 19 May 2025 14:43:18 -0500 Subject: [PATCH 04/10] spi_nxp_lpspi: Add maximum wait time of fifo empty Instead of waiting forever and potentially allowing infinite loop on ISR, wait some arbitrary amount of cycles to error out if it isn't happening. Still make this configurable for debugging purposes. Signed-off-by: Declan Snyder --- drivers/spi/spi_nxp_lpspi/Kconfig | 13 +++++++++++++ drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi.c | 5 ++++- .../spi/spi_nxp_lpspi/spi_nxp_lpspi_common.c | 18 +++++++++++++++--- drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi_priv.h | 2 +- 4 files changed, 33 insertions(+), 5 deletions(-) diff --git a/drivers/spi/spi_nxp_lpspi/Kconfig b/drivers/spi/spi_nxp_lpspi/Kconfig index aabb4d51ce7e..1d9ce8a770a4 100644 --- a/drivers/spi/spi_nxp_lpspi/Kconfig +++ b/drivers/spi/spi_nxp_lpspi/Kconfig @@ -36,4 +36,17 @@ config SPI_MCUX_LPSPI_CPU This has lower latency than DMA-based driver but over the longer transfers will likely have less bandwidth and use more CPU time. +config SPI_NXP_LPSPI_TXFIFO_WAIT_CYCLES + int "Number of CPU cycles to wait on TX fifo empty" + default 0 if DEBUG + default 10000 + help + This option most likely does not need changed. + The drivers tend to need to wait on confirming the transmit command + is consumed by the hardware by checking of the TX fifo is emptied. + This option gives a maximum number of CPU cycles to wait on that check. + The special value of 0 means infinite, which can be useful for debugging + for if there is some programming error that causes TX fifo not to empty. + The default of 10000 is arbitrary. + endif # SPI_MCUX_LPSPI diff --git a/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi.c b/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi.c index bedf112f9c69..8f37d90164d5 100644 --- a/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi.c +++ b/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi.c @@ -337,7 +337,10 @@ static int transceive(const struct device *dev, const struct spi_config *spi_cfg base->TCR |= LPSPI_TCR_CONT_MASK; } /* tcr is written to tx fifo */ - lpspi_wait_tx_fifo_empty(dev); + ret = lpspi_wait_tx_fifo_empty(dev); + if (ret) { + return ret; + } /* start the transfer sequence which are handled by irqs */ lpspi_next_tx_fill(dev); diff --git a/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi_common.c b/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi_common.c index 078a619d2f9c..53df267c4edf 100644 --- a/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi_common.c +++ b/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi_common.c @@ -60,12 +60,24 @@ static inline clock_ip_name_t lpspi_get_clock(LPSPI_Type *const base) } #endif -void lpspi_wait_tx_fifo_empty(const struct device *dev) +int lpspi_wait_tx_fifo_empty(const struct device *dev) { LPSPI_Type *base = (LPSPI_Type *)DEVICE_MMIO_NAMED_GET(dev, reg_base); + int arbitrary_cycle_limit = CONFIG_SPI_NXP_LPSPI_TXFIFO_WAIT_CYCLES; + bool limit_wait = arbitrary_cycle_limit > 0; - while (LPSPI_GetTxFifoCount(base) != 0) { + while (FIELD_GET(LPSPI_FSR_TXCOUNT_MASK, base->FSR) != 0) { + if (!limit_wait) { + continue; + } + + if (arbitrary_cycle_limit-- < 0) { + LOG_WRN("Failed waiting for TX fifo empty"); + return -EIO; + } } + + return 0; } int spi_lpspi_release(const struct device *dev, const struct spi_config *spi_cfg) @@ -185,7 +197,7 @@ int spi_mcux_configure(const struct device *dev, const struct spi_config *spi_cf base->CR |= LPSPI_CR_DBGEN_MASK; } - return 0; + return lpspi_wait_tx_fifo_empty(dev); } static void lpspi_module_system_init(LPSPI_Type *base) 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 c236ed35a499..e9479df6d03c 100644 --- a/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi_priv.h +++ b/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi_priv.h @@ -68,7 +68,7 @@ int spi_nxp_init_common(const struct device *dev); /* common api function for now */ int spi_lpspi_release(const struct device *dev, const struct spi_config *spi_cfg); -void lpspi_wait_tx_fifo_empty(const struct device *dev); +int lpspi_wait_tx_fifo_empty(const struct device *dev); #define SPI_LPSPI_IRQ_FUNC_LP_FLEXCOMM(n) \ nxp_lp_flexcomm_setirqhandler(DEVICE_DT_GET(DT_INST_PARENT(n)), DEVICE_DT_INST_GET(n), \ From 1677ec180c2390d2fb97552000eb2003b7971b52 Mon Sep 17 00:00:00 2001 From: Declan Snyder Date: Mon, 19 May 2025 14:25:57 -0500 Subject: [PATCH 05/10] spi_nxp_lpspi: Remove call to MasterInit For optimization purpose, remove calls to SDK. Since we know exactly what we want, this results in smaller code size. Also, this code calculates the SCK parameters more efficiently than the SDK driver did it by using a binary (instead of linear) search. Lastly, remove call to LPSPI_Reset in the init call and replace with native driver code, and remove inclusion of SDK header. Signed-off-by: Declan Snyder --- .../spi/spi_nxp_lpspi/spi_nxp_lpspi_common.c | 208 +++++++++++++++--- .../mcux/mcux-sdk-ng/drivers/drivers.cmake | 3 - 2 files changed, 173 insertions(+), 38 deletions(-) diff --git a/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi_common.c b/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi_common.c index 53df267c4edf..e41e75cbd795 100644 --- a/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi_common.c +++ b/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi_common.c @@ -15,7 +15,9 @@ LOG_MODULE_REGISTER(spi_lpspi, CONFIG_SPI_LOG_LEVEL); #include "spi_nxp_lpspi_priv.h" -#include + +/* simple macro for readability of the equations used in the clock configuring */ +#define TWO_EXP(power) BIT(power) #if defined(LPSPI_RSTS) || defined(LPSPI_CLOCKS) static LPSPI_Type *const lpspi_bases[] = LPSPI_BASE_PTRS; @@ -120,6 +122,142 @@ static inline int lpspi_validate_xfer_args(const struct spi_config *spi_cfg) return 0; } +static uint8_t lpspi_calc_delay_scaler(uint32_t desired_delay_ns, + uint32_t prescaled_clock, + uint32_t min_cycles) +{ + uint64_t delay_cycles; + + /* calculates the number of functional clock cycles needed to achieve delay */ + delay_cycles = (uint64_t)prescaled_clock * desired_delay_ns; + delay_cycles = DIV_ROUND_UP(delay_cycles, NSEC_PER_SEC); + + /* what the min_cycles parameter is about is that + * PCSSCK and SCKPSC are +1 cycles of the programmed value, + * while DBT is +2 cycles of the programmed value. + * So this calculates the value to program to the register. + */ + delay_cycles -= min_cycles; + + /* Don't overflow */ + delay_cycles = MIN(delay_cycles, UINT8_MAX); + + return (uint8_t)delay_cycles; +} + +/* returns CCR mask of the bits 8-31 */ +static inline uint32_t lpspi_set_delays(const struct device *dev, uint32_t prescaled_clock) +{ + const struct lpspi_config *config = dev->config; + + return LPSPI_CCR_PCSSCK(lpspi_calc_delay_scaler(config->pcs_sck_delay, + prescaled_clock, 1)) | + LPSPI_CCR_SCKPCS(lpspi_calc_delay_scaler(config->sck_pcs_delay, + prescaled_clock, 1)) | + LPSPI_CCR_DBT(lpspi_calc_delay_scaler(config->transfer_delay, + prescaled_clock, 2)); +} + +/* This is the equation for the sck frequency given a div and prescaler. */ +static uint32_t lpspi_calc_sck_freq(uint32_t src_clk_hz, uint16_t sckdiv, uint8_t prescaler) +{ + return (uint32_t)(src_clk_hz / (TWO_EXP(prescaler) * (sckdiv + 2))); +} + +static inline uint8_t lpspi_calc_best_div_for_prescaler(uint32_t src_clk_hz, + uint8_t prescaler, + uint32_t req_freq) +{ + uint64_t prescaled_req_freq = TWO_EXP(prescaler) * req_freq; + uint64_t ratio; + + if (prescaled_req_freq == 0) { + ratio = UINT8_MAX + 2; + } else { + ratio = DIV_ROUND_UP(src_clk_hz, prescaled_req_freq); + } + + ratio = MAX(ratio, 2); + ratio -= 2; + ratio = MIN(ratio, UINT8_MAX); + + return (uint8_t)ratio; +} + +/* This function configures the clock control register (CCR) for the desired frequency + * It does a binary search for the optimal CCR divider and TCR prescaler. + * The prescale_value parameter is changed to the best value of the prescaler, + * for use in setting the TCR outside this function. + * The return value is the mask of the CCR (bits 0-7) required to set SCKDIV for best result. + */ +static inline uint32_t lpspi_set_sckdiv(uint32_t desired_freq, + uint32_t clock_freq, uint8_t *prescale_value) +{ + uint8_t best_prescaler = 0, best_div = 0; + uint32_t best_freq = 0; + + for (int8_t prescaler = 7U; prescaler >= 0; prescaler--) { + /* if maximum freq (div = 0) won't get better than what we got with + * previous prescaler, then we can fast path exit this loop. + */ + if (lpspi_calc_sck_freq(clock_freq, 0, prescaler) < best_freq) { + break; + } + + /* the algorithm approaches the desired freq from below intentionally, + * therefore the min is our previous best and the max is the desired. + */ + uint8_t new_div = lpspi_calc_best_div_for_prescaler(clock_freq, prescaler, + desired_freq); + uint32_t new_freq = lpspi_calc_sck_freq(clock_freq, new_div, prescaler); + + if (new_freq >= best_freq && new_freq <= desired_freq) { + best_div = new_div; + best_freq = new_freq; + best_prescaler = prescaler; + } + } + + *prescale_value = best_prescaler; + + return LPSPI_CCR_SCKDIV(best_div); +} + +/* This function configures everything except the TCR and the clock scaler */ +static void lpspi_basic_config(const struct device *dev, const struct spi_config *spi_cfg) +{ + const struct lpspi_config *config = dev->config; + LPSPI_Type *base = (LPSPI_Type *)DEVICE_MMIO_NAMED_GET(dev, reg_base); + uint32_t pcs_control_bit = 1 << (LPSPI_CFGR1_PCSPOL_SHIFT + spi_cfg->slave); + uint32_t cfgr1_val = 0; + + if (spi_cfg->operation & SPI_CS_ACTIVE_HIGH) { + cfgr1_val |= pcs_control_bit; + } else { + cfgr1_val &= ~pcs_control_bit; + } + + if (SPI_OP_MODE_GET(spi_cfg->operation) == SPI_OP_MODE_MASTER) { + cfgr1_val |= LPSPI_CFGR1_MASTER_MASK; + } + + if (config->tristate_output) { + cfgr1_val |= LPSPI_CFGR1_OUTCFG_MASK; + } + + cfgr1_val |= config->data_pin_config << LPSPI_CFGR1_PINCFG_SHIFT; + + base->CFGR1 = cfgr1_val; + + if (IS_ENABLED(CONFIG_DEBUG)) { + /* DEBUG mode makes it so the lpspi does not keep + * running while debugger has halted the chip. + * This makes debugging spi transfers easier. + */ + base->CR |= LPSPI_CR_DBGEN_MASK; + } +} + int spi_mcux_configure(const struct device *dev, const struct spi_config *spi_cfg) { const struct lpspi_config *config = dev->config; @@ -128,9 +266,9 @@ int spi_mcux_configure(const struct device *dev, const struct spi_config *spi_cf bool already_configured = spi_context_configured(ctx, spi_cfg); 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; - int ret; + uint32_t clock_freq = 0; + uint8_t prescaler = 0; + int ret = 0; /* fast path to avoid reconfigure */ /* TODO: S32K3 errata ERR050456 requiring module reset before every transfer, @@ -145,10 +283,8 @@ int spi_mcux_configure(const struct device *dev, const struct spi_config *spi_cf return ret; } - ret = clock_control_get_rate(config->clock_dev, config->clock_subsys, &clock_freq); - if (ret) { - return ret; - } + /* For the purpose of configuring the LPSPI, 8 is the minimum frame size for the hardware */ + word_size = MAX(word_size, 8); /* specific driver implementation should set up watermarks and interrupts. * we reset them here to avoid any unexpected events during configuring. @@ -168,35 +304,34 @@ int spi_mcux_configure(const struct device *dev, const struct spi_config *spi_cf data->ctx.config = spi_cfg; - LPSPI_MasterGetDefaultConfig(&master_config); - - master_config.bitsPerFrame = word_size < 8 ? 8 : word_size; /* minimum FRAMSZ is 8 */ - 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.whichPcs = spi_cfg->slave + kLPSPI_Pcs0; - master_config.pcsActiveHighOrLow = (spi_cfg->operation & SPI_CS_ACTIVE_HIGH) - ? kLPSPI_PcsActiveHigh : kLPSPI_PcsActiveLow; - master_config.pinCfg = config->data_pin_config; - master_config.dataOutConfig = config->tristate_output ? kLpspiDataOutTristate : - kLpspiDataOutRetained; - - LPSPI_MasterInit(base, &master_config, clock_freq); - LPSPI_SetDummyData(base, 0); + lpspi_basic_config(dev, spi_cfg); - if (IS_ENABLED(CONFIG_DEBUG)) { - base->CR |= LPSPI_CR_DBGEN_MASK; + ret = clock_control_get_rate(config->clock_dev, config->clock_subsys, &clock_freq); + if (ret) { + return ret; } + if (SPI_OP_MODE_GET(spi_cfg->operation) == SPI_OP_MODE_MASTER) { + uint32_t ccr = 0; + + /* sckdiv algorithm must run *before* delays are set in order to know prescaler */ + ccr |= lpspi_set_sckdiv(spi_cfg->frequency, clock_freq, &prescaler); + ccr |= lpspi_set_delays(dev, clock_freq / TWO_EXP(prescaler)); + + /* note that not all bits of the register are readable on some platform, + * that's why we update it on one write + */ + base->CCR = ccr; + } + + base->CR |= LPSPI_CR_MEN_MASK; + + base->TCR = LPSPI_TCR_CPOL(!!(spi_cfg->operation & SPI_MODE_CPOL)) | + LPSPI_TCR_CPHA(!!(spi_cfg->operation & SPI_MODE_CPHA)) | + LPSPI_TCR_LSBF(!!(spi_cfg->operation & SPI_TRANSFER_LSB)) | + LPSPI_TCR_FRAMESZ(word_size - 1) | + LPSPI_TCR_PRESCALE(prescaler) | LPSPI_TCR_PCS(spi_cfg->slave); + return lpspi_wait_tx_fifo_empty(dev); } @@ -239,7 +374,10 @@ int spi_nxp_init_common(const struct device *dev) return err; } - LPSPI_Reset(base); + /* Full software reset */ + base->CR |= LPSPI_CR_RST_MASK; + base->CR |= LPSPI_CR_RRF_MASK | LPSPI_CR_RTF_MASK; + base->CR = 0x00U; config->irq_config_func(dev); diff --git a/modules/hal_nxp/mcux/mcux-sdk-ng/drivers/drivers.cmake b/modules/hal_nxp/mcux/mcux-sdk-ng/drivers/drivers.cmake index 969cf8713467..7c0595d56b78 100644 --- a/modules/hal_nxp/mcux/mcux-sdk-ng/drivers/drivers.cmake +++ b/modules/hal_nxp/mcux/mcux-sdk-ng/drivers/drivers.cmake @@ -24,12 +24,9 @@ if(CONFIG_NXP_LP_FLEXCOMM) set_variable_ifdef(CONFIG_I2C_MCUX_LPI2C CONFIG_MCUX_COMPONENT_driver.lpflexcomm_lpi2c) set_variable_ifdef(CONFIG_UART_MCUX_LPUART CONFIG_MCUX_COMPONENT_driver.lpflexcomm) set_variable_ifdef(CONFIG_UART_MCUX_LPUART CONFIG_MCUX_COMPONENT_driver.lpflexcomm_lpuart) - set_variable_ifdef(CONFIG_SPI_MCUX_LPSPI CONFIG_MCUX_COMPONENT_driver.lpflexcomm) - set_variable_ifdef(CONFIG_SPI_MCUX_LPSPI CONFIG_MCUX_COMPONENT_driver.lpflexcomm_lpspi) else() set_variable_ifdef(CONFIG_I2C_MCUX_LPI2C CONFIG_MCUX_COMPONENT_driver.lpi2c) set_variable_ifdef(CONFIG_UART_MCUX_LPUART CONFIG_MCUX_COMPONENT_driver.lpuart) - set_variable_ifdef(CONFIG_SPI_MCUX_LPSPI CONFIG_MCUX_COMPONENT_driver.lpspi) endif() set_variable_ifdef(CONFIG_DMA_MCUX_LPC CONFIG_MCUX_COMPONENT_driver.lpc_dma) From 4a207d2fac245f7d984afbbf73240fcdbfa9894e Mon Sep 17 00:00:00 2001 From: Declan Snyder Date: Tue, 20 May 2025 12:00:22 -0500 Subject: [PATCH 06/10] spi_nxp_lpspi: Remove MCUX branding Since the LPSPI drivers no long use MCUX at all, remove the MCUX branding, to avoid confusion. In the future if an implementation uses the MCUX SDK driver, it should specifically be called by MCUX in the name. Signed-off-by: Declan Snyder --- doc/releases/migration-guide-4.2.rst | 3 +++ drivers/clock_control/clock_control_mcux_ccm.c | 4 ++-- drivers/clock_control/clock_control_mcux_ccm_rev2.c | 4 ++-- drivers/clock_control/clock_control_mcux_syscon.c | 4 ++-- drivers/spi/spi_nxp_lpspi/CMakeLists.txt | 6 +++--- drivers/spi/spi_nxp_lpspi/Kconfig | 12 ++++++------ drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi.c | 4 ++-- drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi_common.c | 2 +- drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi_dma.c | 10 +++++----- drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi_priv.h | 7 +++++-- soc/nxp/imxrt/imxrt10xx/soc.c | 2 +- soc/nxp/imxrt/imxrt118x/soc.c | 4 ++-- soc/nxp/imxrt/imxrt11xx/soc.c | 2 +- tests/drivers/spi/spi_loopback/testcase.yaml | 6 +++--- 14 files changed, 38 insertions(+), 32 deletions(-) diff --git a/doc/releases/migration-guide-4.2.rst b/doc/releases/migration-guide-4.2.rst index e382db763e5d..012a22a46ef7 100644 --- a/doc/releases/migration-guide-4.2.rst +++ b/doc/releases/migration-guide-4.2.rst @@ -574,6 +574,9 @@ OpenThread SPI === +* Renamed ``CONFIG_SPI_MCUX_LPSPI`` to :kconfig:option:`CONFIG_SPI_NXP_LPSPI`, + and similar for any child configs for that driver, including + :kconfig:option:`CONFIG_SPI_NXP_LPSPI_DMA` and :kconfig:option:`CONFIG_SPI_NXP_LPSPI_CPU`. * Renamed the device tree property ``port_sel`` to ``port-sel``. * Renamed the device tree property ``chip_select`` to ``chip-select``. * The binding file for :dtcompatible:`andestech,atcspi200` has been renamed to have a name diff --git a/drivers/clock_control/clock_control_mcux_ccm.c b/drivers/clock_control/clock_control_mcux_ccm.c index 6cc104d37167..9955e2014d89 100644 --- a/drivers/clock_control/clock_control_mcux_ccm.c +++ b/drivers/clock_control/clock_control_mcux_ccm.c @@ -20,7 +20,7 @@ #include LOG_MODULE_REGISTER(clock_control); -#ifdef CONFIG_SPI_MCUX_LPSPI +#ifdef CONFIG_SPI_NXP_LPSPI static const clock_name_t lpspi_clocks[] = { kCLOCK_Usb1PllPfd1Clk, kCLOCK_Usb1PllPfd0Clk, @@ -207,7 +207,7 @@ static int mcux_ccm_get_subsys_rate(const struct device *dev, break; #endif -#ifdef CONFIG_SPI_MCUX_LPSPI +#ifdef CONFIG_SPI_NXP_LPSPI case IMX_CCM_LPSPI_CLK: { uint32_t lpspi_mux = CLOCK_GetMux(kCLOCK_LpspiMux); diff --git a/drivers/clock_control/clock_control_mcux_ccm_rev2.c b/drivers/clock_control/clock_control_mcux_ccm_rev2.c index dcca3f4eca96..9ded5edb4117 100644 --- a/drivers/clock_control/clock_control_mcux_ccm_rev2.c +++ b/drivers/clock_control/clock_control_mcux_ccm_rev2.c @@ -81,7 +81,7 @@ static int mcux_ccm_get_subsys_rate(const struct device *dev, break; #endif -#ifdef CONFIG_SPI_MCUX_LPSPI +#ifdef CONFIG_SPI_NXP_LPSPI #if defined(CONFIG_SOC_SERIES_IMXRT118X) case IMX_CCM_LPSPI0102_CLK: clock_root = kCLOCK_Root_Lpspi0102 + instance; @@ -91,7 +91,7 @@ static int mcux_ccm_get_subsys_rate(const struct device *dev, clock_root = kCLOCK_Root_Lpspi1 + instance; break; #endif /* CONFIG_SOC_SERIES_IMXRT118X */ -#endif /* CONFIG_SPI_MCUX_LPSPI */ +#endif /* CONFIG_SPI_NXP_LPSPI */ #ifdef CONFIG_UART_MCUX_LPUART #if defined(CONFIG_SOC_SERIES_IMXRT118X) diff --git a/drivers/clock_control/clock_control_mcux_syscon.c b/drivers/clock_control/clock_control_mcux_syscon.c index c7d21ebd6fed..8611bce68f99 100644 --- a/drivers/clock_control/clock_control_mcux_syscon.c +++ b/drivers/clock_control/clock_control_mcux_syscon.c @@ -555,14 +555,14 @@ static int mcux_lpc_syscon_clock_control_get_subsys_rate(const struct device *de break; #endif /* defined(CONFIG_DT_HAS_NXP_XSPI_ENABLED) */ -#if (defined(CONFIG_SPI_MCUX_LPSPI) && CONFIG_SOC_SERIES_MCXA) +#if (defined(CONFIG_SPI_NXP_LPSPI) && CONFIG_SOC_SERIES_MCXA) case MCUX_LPSPI0_CLK: *rate = CLOCK_GetLpspiClkFreq(0); break; case MCUX_LPSPI1_CLK: *rate = CLOCK_GetLpspiClkFreq(1); break; -#endif /* defined(CONFIG_SPI_MCUX_LPSPI) */ +#endif /* defined(CONFIG_SPI_NXP_LPSPI) */ } return 0; diff --git a/drivers/spi/spi_nxp_lpspi/CMakeLists.txt b/drivers/spi/spi_nxp_lpspi/CMakeLists.txt index a494fd061536..2562cfc6dc56 100644 --- a/drivers/spi/spi_nxp_lpspi/CMakeLists.txt +++ b/drivers/spi/spi_nxp_lpspi/CMakeLists.txt @@ -1,5 +1,5 @@ # Copyright 2024 NXP -zephyr_library_sources_ifdef(CONFIG_SPI_MCUX_LPSPI spi_nxp_lpspi_common.c) -zephyr_library_sources_ifdef(CONFIG_SPI_MCUX_LPSPI_CPU spi_nxp_lpspi.c) -zephyr_library_sources_ifdef(CONFIG_SPI_MCUX_LPSPI_DMA spi_nxp_lpspi_dma.c) +zephyr_library_sources_ifdef(CONFIG_SPI_NXP_LPSPI spi_nxp_lpspi_common.c) +zephyr_library_sources_ifdef(CONFIG_SPI_NXP_LPSPI_CPU spi_nxp_lpspi.c) +zephyr_library_sources_ifdef(CONFIG_SPI_NXP_LPSPI_DMA spi_nxp_lpspi_dma.c) diff --git a/drivers/spi/spi_nxp_lpspi/Kconfig b/drivers/spi/spi_nxp_lpspi/Kconfig index 1d9ce8a770a4..fa5050f71fd2 100644 --- a/drivers/spi/spi_nxp_lpspi/Kconfig +++ b/drivers/spi/spi_nxp_lpspi/Kconfig @@ -1,7 +1,7 @@ # Copyright 2018, 2024-2025 NXP # SPDX-License-Identifier: Apache-2.0 -config SPI_MCUX_LPSPI +config SPI_NXP_LPSPI bool "NXP LPSPI peripheral" default y depends on DT_HAS_NXP_LPSPI_ENABLED @@ -10,9 +10,9 @@ config SPI_MCUX_LPSPI help Enable driver support for NXP LPSPI. -if SPI_MCUX_LPSPI +if SPI_NXP_LPSPI -config SPI_MCUX_LPSPI_DMA +config SPI_NXP_LPSPI_DMA bool "NXP LPSPI DMA-based Driver" default y select DMA @@ -27,10 +27,10 @@ config SPI_MCUX_LPSPI_DMA immediately with CPU, so there could be more latency between the point of requesting a transfer and when it actually starts. -config SPI_MCUX_LPSPI_CPU +config SPI_NXP_LPSPI_CPU bool "NXP LPSPI CPU-based driver" default y - depends on $(dt_compat_any_not_has_prop,$(DT_COMPAT_NXP_LPSPI),dmas) || !SPI_MCUX_LPSPI_DMA + depends on $(dt_compat_any_not_has_prop,$(DT_COMPAT_NXP_LPSPI),dmas) || !SPI_NXP_LPSPI_DMA help Enable "normal" CPU based SPI driver for LPSPI. This has lower latency than DMA-based driver but over the @@ -49,4 +49,4 @@ config SPI_NXP_LPSPI_TXFIFO_WAIT_CYCLES for if there is some programming error that causes TX fifo not to empty. The default of 10000 is arbitrary. -endif # SPI_MCUX_LPSPI +endif # SPI_NXP_LPSPI diff --git a/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi.c b/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi.c index 8f37d90164d5..a8c2de856114 100644 --- a/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi.c +++ b/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi.c @@ -311,7 +311,7 @@ static int transceive(const struct device *dev, const struct spi_config *spi_cfg spi_context_buffers_setup(ctx, tx_bufs, rx_bufs, lpspi_data->word_size_bytes); - ret = spi_mcux_configure(dev, spi_cfg); + ret = lpspi_configure(dev, spi_cfg); if (ret) { goto error; } @@ -427,7 +427,7 @@ static int lpspi_init(const struct device *dev) #define SPI_LPSPI_INIT_IF_DMA(n) IF_DISABLED(SPI_NXP_LPSPI_HAS_DMAS(n), (LPSPI_INIT(n))) #define SPI_LPSPI_INIT(n) \ - COND_CODE_1(CONFIG_SPI_MCUX_LPSPI_DMA, \ + COND_CODE_1(CONFIG_SPI_NXP_LPSPI_DMA, \ (SPI_LPSPI_INIT_IF_DMA(n)), (LPSPI_INIT(n))) DT_INST_FOREACH_STATUS_OKAY(SPI_LPSPI_INIT) diff --git a/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi_common.c b/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi_common.c index e41e75cbd795..0d2c26959d4e 100644 --- a/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi_common.c +++ b/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi_common.c @@ -258,7 +258,7 @@ static void lpspi_basic_config(const struct device *dev, const struct spi_config } } -int spi_mcux_configure(const struct device *dev, const struct spi_config *spi_cfg) +int lpspi_configure(const struct device *dev, const struct spi_config *spi_cfg) { const struct lpspi_config *config = dev->config; struct lpspi_data *data = dev->data; 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 5b14a66fc19c..99d573847d53 100644 --- a/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi_dma.c +++ b/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi_dma.c @@ -140,7 +140,7 @@ static int lpspi_dma_next_fill(const struct device *dev) return lpspi_dma_rxtx_load(dev); } -static void spi_mcux_dma_callback(const struct device *dev, void *arg, uint32_t channel, int status) +static void lpspi_dma_callback(const struct device *dev, void *arg, uint32_t channel, int status) { const struct device *spi_dev = arg; LPSPI_Type *base = (LPSPI_Type *)DEVICE_MMIO_NAMED_GET(spi_dev, reg_base); @@ -219,7 +219,7 @@ static int transceive_dma(const struct device *dev, const struct spi_config *spi spi_context_lock(ctx, asynchronous, cb, userdata, spi_cfg); - ret = spi_mcux_configure(dev, spi_cfg); + ret = lpspi_configure(dev, spi_cfg); if (ret) { goto out; } @@ -264,7 +264,7 @@ static int lpspi_dma_dev_ready(const struct device *dma_dev) return true; } -static int spi_mcux_dma_init(const struct device *dev) +static int lpspi_dma_init(const struct device *dev) { struct lpspi_data *data = dev->data; struct spi_nxp_dma_data *dma_data = (struct spi_nxp_dma_data *)data->driver_data; @@ -319,7 +319,7 @@ static void lpspi_isr(const struct device *dev) } #define LPSPI_DMA_COMMON_CFG(n) \ - .dma_callback = spi_mcux_dma_callback, \ + .dma_callback = lpspi_dma_callback, \ .source_data_size = 1, \ .dest_data_size = 1, \ .block_count = 1 @@ -347,7 +347,7 @@ static void lpspi_isr(const struct device *dev) static struct lpspi_data lpspi_data_##n = {.driver_data = &lpspi_dma_data##n, \ SPI_NXP_LPSPI_COMMON_DATA_INIT(n)}; \ \ - SPI_DEVICE_DT_INST_DEFINE(n, spi_mcux_dma_init, NULL, &lpspi_data_##n, \ + SPI_DEVICE_DT_INST_DEFINE(n, lpspi_dma_init, NULL, &lpspi_data_##n, \ &lpspi_config_##n, POST_KERNEL, CONFIG_SPI_INIT_PRIORITY, \ &lpspi_dma_driver_api); 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 e9479df6d03c..9d3f960727d1 100644 --- a/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi_priv.h +++ b/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi_priv.h @@ -53,8 +53,11 @@ struct lpspi_data { uint8_t major_version; }; -/* verifies spi_cfg validity and set up configuration of hardware for xfer */ -int spi_mcux_configure(const struct device *dev, const struct spi_config *spi_cfg); +/* Verifies spi_cfg validity and set up configuration of hardware for xfer + * Unsets interrupt and watermark options, specific implementation should configure that. + * Sets bits in the TCR ONLY *directly* relating to what is in the spi_config struct. + */ +int lpspi_configure(const struct device *dev, const struct spi_config *spi_cfg); /* Does these things: * Set data.dev diff --git a/soc/nxp/imxrt/imxrt10xx/soc.c b/soc/nxp/imxrt/imxrt10xx/soc.c index 29529aa7b7a4..28a653e0d578 100644 --- a/soc/nxp/imxrt/imxrt10xx/soc.c +++ b/soc/nxp/imxrt/imxrt10xx/soc.c @@ -209,7 +209,7 @@ __weak void clock_init(void) CLOCK_SetDiv(kCLOCK_Lpi2cDiv, 5); /* Set I2C divider to 6 */ #endif -#ifdef CONFIG_SPI_MCUX_LPSPI +#ifdef CONFIG_SPI_NXP_LPSPI /* Configure input clock to be able to reach the datasheet specified band rate. */ CLOCK_SetMux(kCLOCK_LpspiMux, 1); /* Set SPI source to USB1 PFD0 */ CLOCK_SetDiv(kCLOCK_LpspiDiv, 0); /* Set SPI divider to 1 */ diff --git a/soc/nxp/imxrt/imxrt118x/soc.c b/soc/nxp/imxrt/imxrt118x/soc.c index 5106e6477493..5be53080f40f 100644 --- a/soc/nxp/imxrt/imxrt118x/soc.c +++ b/soc/nxp/imxrt/imxrt118x/soc.c @@ -338,7 +338,7 @@ __weak void clock_init(void) CLOCK_SetRootClock(kCLOCK_Root_Lpi2c0506, &rootCfg); #endif -#if defined(CONFIG_SPI_MCUX_LPSPI) +#if defined(CONFIG_SPI_NXP_LPSPI) #if (DT_NODE_HAS_STATUS(DT_NODELABEL(lpspi1), okay) \ || DT_NODE_HAS_STATUS(DT_NODELABEL(lpspi2), okay)) @@ -364,7 +364,7 @@ __weak void clock_init(void) CLOCK_SetRootClock(kCLOCK_Root_Lpspi0506, &rootCfg); #endif -#endif /* CONFIG_SPI_MCUX_LPSPI */ +#endif /* CONFIG_SPI_NXP_LPSPI */ #if defined(CONFIG_COUNTER_MCUX_GPT) diff --git a/soc/nxp/imxrt/imxrt11xx/soc.c b/soc/nxp/imxrt/imxrt11xx/soc.c index 4ed9df4ddfb4..b17eb95807d8 100644 --- a/soc/nxp/imxrt/imxrt11xx/soc.c +++ b/soc/nxp/imxrt/imxrt11xx/soc.c @@ -462,7 +462,7 @@ __weak void clock_init(void) CLOCK_SetRootClock(kCLOCK_Root_Enet_Timer1, &rootCfg); #endif -#ifdef CONFIG_SPI_MCUX_LPSPI +#ifdef CONFIG_SPI_NXP_LPSPI /* Configure input clock to be able to reach the datasheet specified band rate. */ rootCfg.mux = kCLOCK_LPSPI1_ClockRoot_MuxOscRc400M; rootCfg.div = 1; diff --git a/tests/drivers/spi/spi_loopback/testcase.yaml b/tests/drivers/spi/spi_loopback/testcase.yaml index 6eb098c3dcf6..7d6968528882 100644 --- a/tests/drivers/spi/spi_loopback/testcase.yaml +++ b/tests/drivers/spi/spi_loopback/testcase.yaml @@ -16,16 +16,16 @@ tests: drivers.spi.loopback.lpspi.dma: filter: DT_HAS_NXP_LPSPI_ENABLED and DT_HAS_NXP_MCUX_EDMA_ENABLED extra_configs: - - CONFIG_SPI_MCUX_LPSPI_DMA=y + - CONFIG_SPI_NXP_LPSPI_DMA=y drivers.spi.loopback.lpspi.async.unset: filter: DT_HAS_NXP_LPSPI_ENABLED and DT_HAS_NXP_MCUX_EDMA_ENABLED extra_configs: - - CONFIG_SPI_MCUX_LPSPI_DMA=n + - CONFIG_SPI_NXP_LPSPI_DMA=n - CONFIG_SPI_ASYNC=n drivers.spi.loopback.lpspi.dma.async.unset: filter: DT_HAS_NXP_LPSPI_ENABLED and DT_HAS_NXP_MCUX_EDMA_ENABLED extra_configs: - - CONFIG_SPI_MCUX_LPSPI_DMA=y + - CONFIG_SPI_NXP_LPSPI_DMA=y - CONFIG_SPI_ASYNC=n drivers.spi.loopback.rtio: extra_configs: From ddced0ab2e5643e786f44825ddf3ac9a15fe1e6b Mon Sep 17 00:00:00 2001 From: Declan Snyder Date: Tue, 3 Jun 2025 17:41:14 -0500 Subject: [PATCH 07/10] spi_nxp_lpspi: Simplify tx ctx update There is no need to update the tx context in interrupt instead of directly after the fill, this just makes the code more complex. Also, the spi context header already handled iterating over buffers so we can remove that code too. Signed-off-by: Declan Snyder --- drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi.c | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi.c b/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi.c index a8c2de856114..8d925f6ab72b 100644 --- a/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi.c +++ b/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi.c @@ -12,7 +12,6 @@ LOG_MODULE_DECLARE(spi_lpspi, CONFIG_SPI_LOG_LEVEL); #include "spi_nxp_lpspi_priv.h" struct lpspi_driver_data { - size_t fill_len; uint8_t word_size_bytes; }; @@ -126,7 +125,7 @@ static inline void lpspi_fill_tx_fifo(const struct device *dev, const uint8_t *b buf_remaining_bytes -= word_size; } - LOG_DBG("Filled TX FIFO to %d words (%d bytes)", lpspi_data->fill_len, offset); + LOG_DBG("Filled TX FIFO to %d words (%d bytes)", fill_len, offset); } /* just fills TX fifo with the specified amount of NOPS */ @@ -187,20 +186,19 @@ static void lpspi_next_tx_fill(const struct device *dev) actual_filled += next_buf_fill; } - lpspi_data->fill_len = actual_filled; + spi_context_update_tx(ctx, lpspi_data->word_size_bytes, actual_filled); } static inline void lpspi_handle_tx_irq(const struct device *dev) { LPSPI_Type *base = (LPSPI_Type *)DEVICE_MMIO_NAMED_GET(dev, reg_base); struct lpspi_data *data = dev->data; - struct lpspi_driver_data *lpspi_data = (struct lpspi_driver_data *)data->driver_data; struct spi_context *ctx = &data->ctx; base->SR = LPSPI_SR_TDF_MASK; /* If we receive a TX interrupt but no more data is available, - * we can be sure that all data has been written to the bus. + * we can be sure that all data has been written to the fifo. * Disable the interrupt to signal that we are done. */ if (!spi_context_tx_on(ctx)) { @@ -208,13 +206,6 @@ static inline void lpspi_handle_tx_irq(const struct device *dev) return; } - while (spi_context_tx_on(ctx) && lpspi_data->fill_len > 0) { - size_t this_buf_words_sent = MIN(lpspi_data->fill_len, ctx->tx_len); - - spi_context_update_tx(ctx, lpspi_data->word_size_bytes, this_buf_words_sent); - lpspi_data->fill_len -= this_buf_words_sent; - } - lpspi_next_tx_fill(dev); } @@ -272,7 +263,6 @@ static void lpspi_isr(const struct device *dev) max_fill - tx_current_fifo_len : 0; lpspi_fill_tx_fifo_nop(dev, fill_len); - lpspi_data->fill_len = fill_len; } if ((DIV_ROUND_UP(spi_context_rx_len_left(ctx, word_size_bytes), word_size_bytes) == 1) && From 7fb9c6fb94526d525d156fe153ba811efb4d091f Mon Sep 17 00:00:00 2001 From: Declan Snyder Date: Tue, 3 Jun 2025 17:52:41 -0500 Subject: [PATCH 08/10] spi_nxp_lpspi: Fix fifo fill logic Fixing some issues with the TX fifo fill logic in two places: First in the normal fill function, it didn't take into account a situation where the TX fifo is already partially filled. This currently doesn't cause a problem because the driver is written in a way that the watermark is always 0 for TDF, but in case the watermark were anything else it would cause a problem. Second, when filling the TX fifo with NOPS in order to clock the rest of the RX in from the bus, the calculation regarding the current TX fifo length was just wrong and was leading to a bug in some cases where there was a subtraction underflow and billions of NOPs were being filled. Also, there could be a problem where a few extra NOPs are put in the TX fifo if we don't count what we already have in the TX fifo, so fixing that as well. Signed-off-by: Declan Snyder --- drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi.c | 42 ++++++++++++++++++----- 1 file changed, 34 insertions(+), 8 deletions(-) diff --git a/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi.c b/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi.c index 8d925f6ab72b..0232a4c3d2a9 100644 --- a/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi.c +++ b/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi.c @@ -144,14 +144,14 @@ static void lpspi_fill_tx_fifo_nop(const struct device *dev, size_t fill_len) static void lpspi_next_tx_fill(const struct device *dev) { const struct lpspi_config *config = dev->config; + LPSPI_Type *base = (LPSPI_Type *)DEVICE_MMIO_NAMED_GET(dev, reg_base); struct lpspi_data *data = dev->data; struct lpspi_driver_data *lpspi_data = (struct lpspi_driver_data *)data->driver_data; struct spi_context *ctx = &data->ctx; - size_t fill_len; + uint8_t left_in_fifo = tx_fifo_cur_len(base); + size_t fill_len = MIN(ctx->tx_len, config->tx_fifo_size - left_in_fifo); size_t actual_filled = 0; - fill_len = MIN(ctx->tx_len, config->tx_fifo_size); - const struct spi_buf *current_buf = ctx->current_tx; const uint8_t *cur_buf_pos = ctx->tx_buf; size_t cur_buf_len_left = ctx->tx_len; @@ -254,15 +254,41 @@ static void lpspi_isr(const struct device *dev) } if (spi_context_rx_on(ctx)) { + /* capture these values because they could change during this code block */ size_t rx_fifo_len = rx_fifo_cur_len(base); + size_t tx_fifo_len = tx_fifo_cur_len(base); + + /* + * Goal here is to provide the number of TX NOPS to match the amount of RX + * we have left to receive, so that we provide the correct number of + * clocks to the bus, since the clocks only happen when TX data is being sent. + * + * The expected RX left is essentially what is left in the spi transfer + * minus what we have just got in the fifo, but prevent underflow of this + * subtraction since it is unsigned. + */ size_t expected_rx_left = rx_fifo_len < ctx->rx_len ? ctx->rx_len - rx_fifo_len : 0; - size_t max_fill = MIN(expected_rx_left, config->rx_fifo_size); - size_t tx_current_fifo_len = tx_fifo_cur_len(base); - size_t fill_len = tx_current_fifo_len < ctx->rx_len ? - max_fill - tx_current_fifo_len : 0; - lpspi_fill_tx_fifo_nop(dev, fill_len); + /* + * We know the expected amount of RX we have left but only fill up to the + * max of the RX fifo size so that we don't have some overflow of the FIFO later. + * Similarly, we shouldn't overfill the TX fifo with too many NOPs. + */ + size_t tx_fifo_space_left = config->tx_fifo_size - tx_fifo_len; + size_t rx_fifo_space_left = config->rx_fifo_size - rx_fifo_len; + size_t max_fifo_fill = MIN(tx_fifo_space_left, rx_fifo_space_left); + size_t max_fill = MIN(max_fifo_fill, expected_rx_left); + + /* If we already have some words in the tx fifo, we should count those */ + if (max_fill > tx_fifo_len) { + max_fill -= tx_fifo_len; + } else { + max_fill = 0; + } + + /* So now we want to fill the fifo with the max amount of NOPs */ + lpspi_fill_tx_fifo_nop(dev, max_fill); } if ((DIV_ROUND_UP(spi_context_rx_len_left(ctx, word_size_bytes), word_size_bytes) == 1) && From 081cdf7d439d02e9eb6319748d4cd6ddba54d772 Mon Sep 17 00:00:00 2001 From: Declan Snyder Date: Tue, 10 Jun 2025 10:20:35 -0500 Subject: [PATCH 09/10] spi_nxp_lpspi: Fix unit of buf_len in fill_tx_fifo The buf_len parameter of lpspi_fill_tx_fifo is supposed to be bytes, so we do not need to convert it. This could cause an issue if the end of the buffer is less bytes than the word size. Signed-off-by: Declan Snyder --- drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi.c b/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi.c index 0232a4c3d2a9..367d30e71165 100644 --- a/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi.c +++ b/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi.c @@ -112,17 +112,16 @@ static inline void lpspi_fill_tx_fifo(const struct device *dev, const uint8_t *b struct lpspi_data *data = dev->data; struct lpspi_driver_data *lpspi_data = (struct lpspi_driver_data *)data->driver_data; uint8_t word_size = lpspi_data->word_size_bytes; - size_t buf_remaining_bytes = buf_len * word_size; size_t offset = 0; uint32_t next_word; uint32_t next_word_bytes; for (int word_count = 0; word_count < fill_len; word_count++) { - next_word_bytes = MIN(word_size, buf_remaining_bytes); + next_word_bytes = MIN(word_size, buf_len); next_word = lpspi_next_tx_word(dev, buf, offset, next_word_bytes); base->TDR = next_word; offset += word_size; - buf_remaining_bytes -= word_size; + buf_len -= word_size; } LOG_DBG("Filled TX FIFO to %d words (%d bytes)", fill_len, offset); From ccd7222cc70a74fa173d8e6875f2595117e98952 Mon Sep 17 00:00:00 2001 From: Declan Snyder Date: Thu, 12 Jun 2025 17:38:43 -0500 Subject: [PATCH 10/10] spi_nxp_lpspi: Fix extra byte sent on v1 lpspi This stupid errata will not leave me alone, here is another bandaid to deal with an issue where an extra byte was being sent on version 1 LPSPIs due to the algorithm of filling NOPs when only RX is left was not expecting the situation where the LPSPI actually consumed everything from the fifo but is not sending it due to this ridiculous stalling errata. Signed-off-by: Declan Snyder --- drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi.c b/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi.c index 367d30e71165..ad729cd414e9 100644 --- a/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi.c +++ b/drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi.c @@ -252,6 +252,12 @@ static void lpspi_isr(const struct device *dev) return; } + /* the lpspi v1 has an errata where it doesn't clock the last bit + * in continuous mode until you write the TCR + */ + bool likely_stalling_v1 = data->major_version < 2 && + (DIV_ROUND_UP(spi_context_rx_len_left(ctx, word_size_bytes), word_size_bytes) == 1); + if (spi_context_rx_on(ctx)) { /* capture these values because they could change during this code block */ size_t rx_fifo_len = rx_fifo_cur_len(base); @@ -279,6 +285,10 @@ static void lpspi_isr(const struct device *dev) size_t max_fifo_fill = MIN(tx_fifo_space_left, rx_fifo_space_left); size_t max_fill = MIN(max_fifo_fill, expected_rx_left); + if (likely_stalling_v1 && max_fill > 0) { + max_fill -= 1; + } + /* If we already have some words in the tx fifo, we should count those */ if (max_fill > tx_fifo_len) { max_fill -= tx_fifo_len; @@ -290,8 +300,7 @@ static void lpspi_isr(const struct device *dev) lpspi_fill_tx_fifo_nop(dev, max_fill); } - if ((DIV_ROUND_UP(spi_context_rx_len_left(ctx, word_size_bytes), word_size_bytes) == 1) && - (data->major_version < 2)) { + if (likely_stalling_v1) { /* Due to stalling behavior on older LPSPI, * need to end xfer in order to get last bit clocked out on bus. */