Sorry for late answer, The problem appears during calculation of md5sum of eMMC (16GB) in the loop. After some time (e.g. 1-2 hours), md5sum may be different For example: 5c2b3c7a6d69a2f6c4c1ddfdd3bf1ed5 /dev/mmcblk0 time 746 [s] 5c2b3c7a6d69a2f6c4c1ddfdd3bf1ed5 /dev/mmcblk0 time 738 [s] c5f2bb8e9d83744d4087450d6274208e /dev/mmcblk0 time 691 [s] ... > It looks very dubious whether this is actually the right thing to do. > > Just considering this driver, edma has an complementary sync_sg call in > > its .start method, so if idma needed this one, logically shouldn't it > > also need the other one as well? If there is some coherency issue on Cortex-A9 - Probably yes, I have to test it > However, from a DMA API point of view, these syncs make no sense either > > way - the very next thing we do here is call host->dma_ops->cleanup(), > > which calls dma_unmap_sg(), which will perform the appropriate cache > > maintenance anyway. Thus I can't see why this code is even here to begin > > with. Similarly on the request path - the sg list really shouldn't have > > been touched since being mapped in dw_mci_pre_dma_transfer(), so that > > sync should also be an effective no-op unless it's papering over some > > race condition elsewhere. I agree, But if there is some coherency issue with RK31xx SoCs (Cortex-A9, EDMA) and Altera SoC FPGA (Cortex-A9, IDMA) ? Because I see positive results, after applying mentioned patch (Altera SoC FPGA - Cortex-A9, IDMA). Probably using sync-ops functions before DMA transaction and after DMA transaction in this case make sense Shawn Could you please also share some details, that you remember (sync-ops and RK31xx SoCs) ? I would really appreciate it. Thanks On Di, 2018-11-27 at 08:43 +0800, Shawn Lin wrote: > On 2018/11/23 23:29, Robin Murphy wrote: > > Hi Jan, > > > > [repeating some of the discussion from your other thread for the benefit > > of the MMC audience] > > > > On 21/11/2018 07:42, JABLONSKY Jan wrote: > >> CPU may not see most up-to-date and correct copy of DMA buffer, when > >> internal DMA controller is in use. > >> Problem appears on The Altera SoC FPGA (uses integrated DMA controller), > >> during higher CPU and system memory load > >> > >> Signed-off-by: Jan Jablonsky <jan.jablonsky@xxxxxxxxxxxxxxx> > >> --- > >> drivers/mmc/host/dw_mmc.c | 3 +-- > >> 1 file changed, 1 insertion(+), 2 deletions(-) > >> > >> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c > >> index 80dc2fd..63873d9 100644 > >> --- a/drivers/mmc/host/dw_mmc.c > >> +++ b/drivers/mmc/host/dw_mmc.c > >> @@ -499,8 +499,7 @@ static void dw_mci_dmac_complete_dma(void *arg) > >> dev_vdbg(host->dev, "DMA complete\n"); > >> - if ((host->use_dma == TRANS_MODE_EDMAC) && > >> - data && (data->flags & MMC_DATA_READ)) > >> + if (data && (data->flags & MMC_DATA_READ)) > >> /* Invalidate cache after read */ > >> dma_sync_sg_for_cpu(mmc_dev(host->slot->mmc), > >> data->sg, > > > > It looks very dubious whether this is actually the right thing to do. > > Just considering this driver, edma has an complementary sync_sg call in > > its .start method, so if idma needed this one, logically shouldn't it > > also need the other one as well? > > > > However, from a DMA API point of view, these syncs make no sense either > > way - the very next thing we do here is call host->dma_ops->cleanup(), > > which calls dma_unmap_sg(), which will perform the appropriate cache > > maintenance anyway. Thus I can't see why this code is even here to begin > > with. Similarly on the request path - the sg list really shouldn't have > > been touched since being mapped in dw_mci_pre_dma_transfer(), so that > > sync should also be an effective no-op unless it's papering over some > > race condition elsewhere. > > > > Shawn - do you remember why these syncs were added in 3fc7eaef44dbc? > > Were you seeing actual coherency issues on RK31xx SoCs, or was it > > perhaps just some leftover or misunderstanding which missed getting > > cleaned up? > > I can't remember too much details but looking at the dma-mapping code > again, it seems the complemetary sync-op here is useless. > > > > > Robin. > > > > > > > >
