Messages in this thread | | | Subject | Re: dw_mmc: IDMAC Invalidate cache after read | From | Shawn Lin <> | Date | Tue, 27 Nov 2018 08:43:11 +0800 |
| |
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@thalesgroup.com> >> --- >> 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. > > >
| |