I3C DMA Transfer and Abort Support #2460
Draft
+619
−4
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
SwaroopPKADI
requested review from
buha,
CiprianRegus,
dbogdan,
amiclaus,
rbolboac and
danmois
as code owners
|
Is this tested on hw? can you provide a basic_example.c along-side? |
drivers/platform/stm32/stm32_i3c.c
Outdated
| int32_t stm32_i3c_dma_transfer(struct no_os_i3c_desc *desc, | ||
| struct no_os_i3c_msg *msgs, | ||
| uint32_t len, | ||
| void (*callback)(void *), |
There was a problem hiding this comment.
callback is not used anywhere in this function. Am I missing something?
drivers/platform/stm32/stm32_i3c.c
Outdated
Comment on lines
677
to
898
| else if (tx_count == 0) | ||
| hi3c->State = HAL_I3C_STATE_BUSY_RX; | ||
| else | ||
| hi3c->State = HAL_I3C_STATE_BUSY_TX_RX; | ||
|
|
||
| /* Enable Tx process (frame complete and error) interrupts */ | ||
| hi3c->Instance->IER |= (HAL_I3C_IT_FCIE | HAL_I3C_IT_ERRIE); | ||
|
|
||
| /* Enable control DMA Request */ | ||
| LL_I3C_EnableDMAReq_Control(hi3c->Instance); | ||
|
|
||
| /* Check if Rx counter different from zero */ | ||
| if (rx_count != 0U) | ||
| { | ||
| /* Enable Rx data DMA Request */ | ||
| LL_I3C_EnableDMAReq_RX(hi3c->Instance); | ||
| } | ||
|
|
||
| /* Check if Tx counter different from zero */ | ||
| if (tx_count != 0U) | ||
| { | ||
| /* Enable Tx data DMA Request */ | ||
| LL_I3C_EnableDMAReq_TX(hi3c->Instance); | ||
| } | ||
|
|
||
| /******************** DMA Configuration ************************/ | ||
| dma_ctx->desc = desc; | ||
|
|
||
| ret = no_os_dma_config_xfer(xdesc->i3c_dma_desc->dma_desc, &dma_ctx->cr_ch_xfer, 1, xdesc->i3c_dma_desc->crdma_ch); | ||
| if (ret) | ||
| goto free_tx_ch_xfer; | ||
| if (dma_ctx->rx_ch_xfer) { | ||
| ret = no_os_dma_config_xfer(xdesc->i3c_dma_desc->dma_desc, dma_ctx->rx_ch_xfer, rx_count, xdesc->i3c_dma_desc->rxdma_ch); | ||
| if (ret) | ||
| goto free_tx_ch_xfer; | ||
| } | ||
| if (dma_ctx->tx_ch_xfer) { | ||
| ret = no_os_dma_config_xfer(xdesc->i3c_dma_desc->dma_desc, dma_ctx->tx_ch_xfer, tx_count, xdesc->i3c_dma_desc->txdma_ch); | ||
| if (ret) | ||
| goto free_tx_ch_xfer; | ||
| } | ||
|
|
||
| /********************** Priority Configuration ************************/ | ||
| /* | ||
| * Note: The priority of the I3C interrupt has to lower than DMA interrupt. | ||
| * The I3C interrupt is made to wait until the DMA interrupt is received | ||
| * and then continues to write the next control word once the DMA transfer is | ||
| * configured. | ||
| */ | ||
| if (xdesc->i3c_dma_desc->txdma_ch) | ||
| no_os_irq_get_priority(xdesc->i3c_dma_desc->dma_desc->irq_ctrl, | ||
| xdesc->i3c_dma_desc->txdma_ch->irq_num, | ||
| &txdma_priority); | ||
| if (xdesc->i3c_dma_desc->rxdma_ch) | ||
| no_os_irq_get_priority(xdesc->i3c_dma_desc->dma_desc->irq_ctrl, | ||
| xdesc->i3c_dma_desc->rxdma_ch->irq_num, | ||
| &rxdma_priority); | ||
| i3c_priority = txdma_priority > rxdma_priority ? txdma_priority+1 : rxdma_priority+1; | ||
| HAL_NVIC_SetPriority(xdesc->irq_id, i3c_priority, 0); | ||
| i3c_dma_cplt[desc->bus->device_id] = false; | ||
|
|
||
| /********************* DMA Start ***********************/ | ||
| if (dma_ctx->rx_ch_xfer) { | ||
| ret = no_os_dma_xfer_start(xdesc->i3c_dma_desc->dma_desc, xdesc->i3c_dma_desc->rxdma_ch); | ||
| if (ret) | ||
| goto abort_transfer; | ||
| } | ||
| if (dma_ctx->tx_ch_xfer) { | ||
| ret = no_os_dma_xfer_start(xdesc->i3c_dma_desc->dma_desc, xdesc->i3c_dma_desc->txdma_ch); | ||
| if (ret) | ||
| goto abort_transfer; | ||
| } | ||
|
|
||
| /* Write Control buffer data to control register */ | ||
| ret = no_os_dma_xfer_start(xdesc->i3c_dma_desc->dma_desc, xdesc->i3c_dma_desc->crdma_ch); | ||
| if (ret) | ||
| goto abort_transfer; | ||
|
|
||
| /* Decrement remaining control buffer data counter */ | ||
| hi3c->ControlXferCount--; | ||
|
|
||
| /* Initiate a Start condition */ | ||
| LL_I3C_RequestTransfer(hi3c->Instance); | ||
|
|
||
| return ret; | ||
|
|
||
| abort_transfer: | ||
| no_os_dma_xfer_abort(xdesc->i3c_dma_desc->dma_desc, xdesc->i3c_dma_desc->crdma_ch); | ||
| no_os_dma_xfer_abort(xdesc->i3c_dma_desc->dma_desc, xdesc->i3c_dma_desc->txdma_ch); | ||
| no_os_dma_xfer_abort(xdesc->i3c_dma_desc->dma_desc, xdesc->i3c_dma_desc->rxdma_ch); | ||
| free_tx_ch_xfer: | ||
| no_os_free(dma_ctx->tx_ch_xfer); | ||
| free_rx_ch_xfer: | ||
| no_os_free(dma_ctx->rx_ch_xfer); | ||
| free_cr_src: | ||
| no_os_free(dma_ctx->cr_ch_xfer.src); | ||
| free_ctx: | ||
| no_os_free(dma_ctx); | ||
| return ret; | ||
| } |
There was a problem hiding this comment.
Please split this into multiple functions. 250 LOC is too much...
Added the DMA transfer and abort functionalities for i3c. This will enable the user to configure the DMA for I3C transaction asynchronously and abort it when necessary. Signed-off-by: Swaroop PrakashKumar <[email protected]>
…I3C. The driver supports usage of DMA channels for the Control register, Tx and Rx. Signed-off-by: Swaroop PrakashKumar <[email protected]>
SwaroopPKADI
force-pushed
the
i3c_dma_support
branch
from
585938f to
0ee107c
Compare
|
/AzurePipelines run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
buha
reviewed
Feb 21, 2025
| * Transfer a list of messages using DMA. Return once the first one started and | ||
| * invoke a callback when they are done. | ||
| */ | ||
| int32_t no_os_i3c_dma_transfer(struct no_os_i3c_desc *desc, |
There was a problem hiding this comment.
A word about naming inconsistency, there is some existing DMA support in SPI and I think we should follow the naming convention there:
/* Iterate over the spi_msg array and send all messages at once */
int32_t no_os_spi_transfer(struct no_os_spi_desc *desc,
struct no_os_spi_msg *msgs,
uint32_t len);
/* Transfer a list of messages using DMA. Wait until all transfers are done */
int32_t no_os_spi_transfer_dma_sync(struct no_os_spi_desc *desc,
struct no_os_spi_msg *msgs,
uint32_t len);
/*
* Transfer a list of messages using DMA. Return once the first one started and
* invoke a callback when they are done.
*/
int32_t no_os_spi_transfer_dma_async(struct no_os_spi_desc *desc,
struct no_os_spi_msg *msgs,
uint32_t len,
void (*callback)(void *),
void *ctx);
If we follow that, your function should be named no_os_i3c_transfer_dma_async. I think it would be useful to include a synchronous version too, no_os_i3c_transfer_dma which uses the async version internally but also takes care of the callback.
There was a problem hiding this comment.
I understand the mismatch in naming convention and will fix that but, I couldn't understand the usefulness of having a synchronous version of DMA transfer. Isn't it equivalent to a normal I3C transfer?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pull Request Description
I added DMA transfer and abort support for the I3C interface. This allows efficient data transfers and the ability to stop transfers when needed. The DMA transaction of multiple transfers occur sequentially and I3C data complete callback waits for the DMA callback to be completed and the next transfer to be configured before proceeding to configure the next write to control register.
PR Type
PR Checklist