Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LPSPI Fix Timeout waiting for transfer complete #87548

Merged
merged 3 commits into from
Mar 26, 2025
+15 −8
Merged

LPSPI Fix Timeout waiting for transfer complete #87548

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
981db84
spi_nxp_lpspi: Fix ISR handler filling TX
PetervdPerk-NXP Mar 24, 2025
5c7218e
spi_nxp_lpspi: Reset peripheral on startup
PetervdPerk-NXP Mar 24, 2025
7aa4c56
spi_nxp_lpspi: Fix DMA not releasing lock on error
PetervdPerk-NXP Mar 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 10 additions & 6 deletions drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi.c
View file
Original file line number Diff line number Diff line change
@@ -26,7 +26,6 @@ static inline uint8_t tx_fifo_cur_len(LPSPI_Type *base)
return (base->FSR & LPSPI_FSR_TXCOUNT_MASK) >> LPSPI_FSR_TXCOUNT_SHIFT;
}


/* Reads a word from the RX fifo and handles writing it into the RX spi buf */
static inline void lpspi_rx_word_write_bytes(const struct device *dev, size_t offset)
{
@@ -201,9 +200,7 @@ static void lpspi_isr(const struct device *dev)
return;
}

if (spi_context_rx_len_left(ctx) == 1) {
base->TCR &= ~LPSPI_TCR_CONT_MASK;
} else if (spi_context_rx_on(ctx)) {
if (spi_context_rx_on(ctx)) {
size_t rx_fifo_len = rx_fifo_cur_len(base);
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);
@@ -213,7 +210,11 @@ static void lpspi_isr(const struct device *dev)
max_fill - tx_current_fifo_len : 0;

lpspi_fill_tx_fifo_nop(dev);
} else {
}

if (spi_context_rx_len_left(ctx) == 1) {
base->TCR &= ~LPSPI_TCR_CONT_MASK;
} else if (spi_context_rx_len_left(ctx) == 0) {
spi_context_complete(ctx, dev, 0);
NVIC_ClearPendingIRQ(config->irqn);
base->TCR &= ~LPSPI_TCR_CONT_MASK;
@@ -270,7 +271,10 @@ static int transceive(const struct device *dev, const struct spi_config *spi_cfg
LPSPI_EnableInterrupts(base, (uint32_t)kLPSPI_TxInterruptEnable |
(uint32_t)kLPSPI_RxInterruptEnable);

return spi_context_wait_for_completion(ctx);
ret = spi_context_wait_for_completion(ctx);
if (ret >= 0) {
return ret;
}

error:
spi_context_release(ctx, ret);
5 changes: 4 additions & 1 deletion drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi_common.c
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2018, 2024 NXP
* Copyright 2018, 2024-2025 NXP
*
* SPDX-License-Identifier: Apache-2.0
*/
@@ -130,6 +130,7 @@ int spi_mcux_configure(const struct device *dev, const struct spi_config *spi_cf

int spi_nxp_init_common(const struct device *dev)
{
LPSPI_Type *base = (LPSPI_Type *)DEVICE_MMIO_NAMED_GET(dev, reg_base);
const struct spi_mcux_config *config = dev->config;
struct spi_mcux_data *data = dev->data;
int err = 0;
@@ -153,6 +154,8 @@ int spi_nxp_init_common(const struct device *dev)
return err;
}

LPSPI_Reset(base);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why was this needed ? just wondering.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the IMX95 we load code into ITCM and execute from there due to that there's no POR.
Thus we can't guarantee that the LPSPI in the right state on startup, so I prefer to reset a peripheral on initialization.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, just FYI, I was planning to hardware reset peripheral at module level, in the PR I linked above, but we can do software peripheral reset also

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well not every chip has a RESET_ReleasePeripheralReset interface whereas each LPSPI has a LPSPI_Reset.
So if you really want to use RESET_ReleasePeripheralReset you would have to implement a fallback anyhow.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I know, but actually the hardware signal reset is required on some platforms anyways before interfacing to the peripheral, even to do a software reset. But like I said I have no issue with this


config->irq_config_func(dev);

return err;
2 changes: 1 addition & 1 deletion drivers/spi/spi_nxp_lpspi/spi_nxp_lpspi_dma.c
View file
Original file line number Diff line number Diff line change
@@ -224,7 +224,7 @@ static int transceive_dma(const struct device *dev, const struct spi_config *spi

ret = spi_mcux_dma_next_fill(dev);
if (ret) {
return ret;
goto out;
}

LPSPI_FlushFifo(base, true, true);