drivers: dma: dma_xilinx_axi_dma: reliability fixes and cleanups #85412
Conversation
zephyrbot
requested review from
henrikbrixandersen,
ibirnbaum,
michalsimek and
teburd
robhancocksed
force-pushed
the
xilinx-axi-dma-fixes
branch
from
ec4011e to
53f7c35
Compare
|
Added another change to improve the support for non-DMA-coherent architectures in the driver by supporting explicit cache flush/invalidate operations as required, rather than fully disabling the dcache during descriptor and/or DMA access. |
|
An additional change to fix another issue noted with storing IRQ numbers for later enabling/disabling, as well as to ensure that DMA ISRs are not preempted by themselves. |
|
One more change, as I noticed that the descriptors were global and not per-instance, which caused problems when trying to use more than one instance. |
|
Ping.. any feedback? |
…tion The DMA_XILINX_AXI_DMA_DISABLE_CACHE_WHEN_ACCESSING_SG_DESCRIPTORS option is not meaningful to enable unless the platform/configuration actually supports the corresponding cache maintenance operations. Add dependencies accordingly. Signed-off-by: Robert Hancock <[email protected]>
Instead of using a packed structure to define the register map, just create an enum for the registers and use that to refer to them. This avoids the need for repeatedly disabling GCC warnings for taking the address of packed structure members. Signed-off-by: Robert Hancock <[email protected]>
Doing a reset of the DMA engine during the dma_configure operation is problematic when using the DMA core in combination with the Xilinx AXI Ethernet core, since the DMA core's reset signals are normally propagated into the Ethernet core. This means that after the Ethernet core initializes and starts a DMA operation for the first time, the DMA core is reset, wiping out all of the register settings that the Ethernet core has made. To avoid this, move the DMA core reset and other initialization which only needs to be done once into the init function, so this is done during initial driver load and not deferred until later. Signed-off-by: Robert Hancock <[email protected]>
Fix some issues in the driver's DMA interrupt handling: -Ensure that interrupts are cleared prior to handling interrupt events, so that events raised during interrupt processing will cause the hardware to raise a new interrupt -Ensure that we do not overwrite existing DMA descriptors which are incomplete (such as by trying to execute more transfers than there are slots in the descriptor ring) -Ensure that error events reported by the DMA engine are reported -Rename some of the variables to track pending and completed ring locations for better clarity Signed-off-by: Robert Hancock <[email protected]>
The driver previously had a timer to periodically check for completed TX/RX transfers in case an interrupt notification was missed. With previous changes to the driver to avoid lost interrupt wakeups, this workaround should no longer be required, so remove it. Signed-off-by: Robert Hancock <[email protected]>
This driver has a config option DMA_XILINX_AXI_DMA_DISABLE_CACHE_WHEN_ACCESSING_SG_DESCRIPTORS to allow it to be used on platforms where DMA memory access is not automatically cache coherent. However, fully disabling the dcache when accessing DMA buffers/descriptors is not necessary and is potentially problematic. This can be handled more selectively by doing explicit cache invalidate and/or flush operations on the buffers involved as required. Note that this does introduce a requirement that RX DMA buffers provided to the driver are cache line aligned, as otherwise the required cache invalidate operation could potentially corrupt unrelated data. This is explicitly checked when a DMA RX operation is started. Tested on Cortex-R5 with data cache enabled. Signed-off-by: Robert Hancock <[email protected]>
The way the driver was storing IRQ numbers for later use in the various locking modes was not correct, causing the wrong IRQs to potentially be disabled/enabled in some modes. Refactor the way this is done to be cleaner, and also the way the different locking modes are implemented in order to ensure that all modes receive compile test coverage. Also, ensure that the IRQ for the RX or TX channel is always disabled during the execution of the corresponding ISR, to prevent it from being preempted by itself if the DMA core raises another interrupt during the ISR execution. Signed-off-by: Robert Hancock <[email protected]>
Move the descriptor storage into the per-instance data structure rather than being global, as they should not be shared between instances. Signed-off-by: Robert Hancock <[email protected]>
robhancocksed
force-pushed
the
xilinx-axi-dma-fixes
branch
from
87457ea to
11cd484
Compare
|
Ping.. needs review |
|
Ping, still needs review |
|
@robhancocksed Can you try re-requesting reviews (top-right, under "Reviewers" heading)? |
I don't appear to have that option, since there have been no reviews on this MR to start with.. |
|
In this case, you can try converting to draft and back to ready to merge. That should (re)request reviews. |
This is really NOT the way to do it. Please refrain from doing any of the above in attempts to attract reviews. The recommended process for getting Zephyr PRs is documented here: https://docs.zephyrproject.org/latest/contribute/contributor_expectations.html#getting-prs-reviewed |
Various cleanups and fixes for the Xilinx AXI DMA driver to get it working reliably in our environment with the Xilinx AXI Ethernet driver being submitted by @WorldofJARcraft under #73986: