Hi Wolfram, On Thu, 6 Oct 2022 at 21:05, Wolfram Sang <wsa+renesas@xxxxxxxxxxxxxxxxxxxx> wrote: > > Motivation for this series from patch 5: > > === > So far, we have been relying on access_end interrupts only to mark DMA > transfers as done implying that DMA end interrupts have occurred by then > anyhow. On some SoCs under some conditions, this turned out to be not > enough. So, we enable DMA interrupts as well and make sure that both > events, DMA irq and access_end irq, have happened before finishing the > DMA transfer. The tmio/sdhi core still relies on using tasklets. I think we should strive to move away from tasklets for all mmc host drivers and to use threaded irqs instead. That said, I am worried that it might be harder to move away from tasklets beyond $subject series, for tmio/sdhi, but I might be wrong? So, I am wondering if it perhaps would be better to make that modernization/conversion as the first step instead? Kind regards Uffe > === > > The first two patches are cleanups. For the rest, the basis were BSP > patches, but they have been refactored heavily, so they look very > different now: > > * self-contained > except for the new dma_irq callback which is for the TMIO core, all > other code is self-contained in renesas_sdhi_internal_dmac now. > * concise > Less new members for the existing structs, also code duplication was > avoided by moving code to more suitable locations > * replaced the spinlock with atomic bit operators > * used quirk mechanism for the old INFO1 layout > > Tested on a Salvator-X board with M3-W (r8a77960) and a Salvator-XS > board with M3-N (r8a77965). No regression encountered in functionality > and performance. A branch can be found here: > > git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git renesas/sdhi/upport_dmaend > > > Here are excerpts of a log when booting the M3-N with patch 6 applied to > show that all combinations of incoming irqs are handled: > > === DMA first, Access second === > > <idle>-0 [000] d.h.. 0.505454: renesas_sdhi_internal_dmac_dma_irq: DMA end > <idle>-0 [000] d.h.. 0.505496: renesas_sdhi_internal_dmac_dataend_dma: Access end: 0 > <idle>-0 [000] ..s.. 0.505498: renesas_sdhi_internal_dmac_complete_tasklet_fn: Tasklet > > === Access first, DMA second === > > kworker/0:2-42 [000] d.h.. 0.510603: renesas_sdhi_internal_dmac_dataend_dma: Access end: 0 > kworker/0:2-42 [000] d.h.. 0.510605: renesas_sdhi_internal_dmac_dma_irq: DMA end > kworker/0:2-42 [000] ..s.. 0.510606: renesas_sdhi_internal_dmac_complete_tasklet_fn: Tasklet > > === Access first, Simulated DMA second === > > <idle>-0 [000] d.H.. 0.510635: renesas_sdhi_internal_dmac_dataend_dma: Access end: 0 > <idle>-0 [000] ..s.. 0.510637: renesas_sdhi_internal_dmac_issue_tasklet_fn: Simulated DMA end > <idle>-0 [000] ..s.. 0.510638: renesas_sdhi_internal_dmac_complete_tasklet_fn: Tasklet > > (I have never seen Simulated DMA (= CMD error happened) first, but it > should be handled like regular DMA end as well(tm).) > > === Access first, no DMA end needed because of DATA error (EILSEQ) === > > <idle>-0 [000] d.H.. 0.510894: renesas_sdhi_internal_dmac_dataend_dma: Access end: -84 > <idle>-0 [000] ..s.. 0.510896: renesas_sdhi_internal_dmac_complete_tasklet_fn: Tasklet > > === > > I think this is as far as I can reach alone. I'd love to get review > comments and further testing. Shimoda-san, can you kindly ask the BSP > team to do further testing? > > Thank you everyone and happy hacking, > > Wolfram > > > Wolfram Sang (6): > mmc: renesas_sdhi: remove accessor function for internal_dmac > mmc: renesas_sdhi: improve naming of DMA struct > mmc: tmio: add callback for dma irq > mmc: renesas_sdhi: add quirk for broken register layout > mmc: renesas_sdhi: take DMA end interrupts into account > DEBUG mmc: renesas_sdhi: debug end_flags for DMA > > drivers/mmc/host/renesas_sdhi.h | 14 ++- > drivers/mmc/host/renesas_sdhi_core.c | 2 +- > drivers/mmc/host/renesas_sdhi_internal_dmac.c | 86 ++++++++++++------- > drivers/mmc/host/tmio_mmc.h | 1 + > drivers/mmc/host/tmio_mmc_core.c | 3 + > 5 files changed, 72 insertions(+), 34 deletions(-) > > -- > 2.35.1 >
