2010/5/15 Felix Fietkau <nbd@xxxxxxxxxxx>: > On 2010-05-14 5:27 PM, Ming Lei wrote: >> 2010/5/14 Felix Fietkau <nbd@xxxxxxxxxxx>: >>> On 2010-05-14 3:16 PM, tom.leiming@xxxxxxxxx wrote: >>>> From: Ming Lei <tom.leiming@xxxxxxxxx> >>>> >>>> If buffer is to be accessed by cpu after dma transfer is over, but >>>> between dma mapping and dma unmapping, we should use >>>> dma_sync_single_for_cpu to sync the buffer between cpu with >>>> device. And dma_sync_single_for_device is used to let >>>> device gain the buffer again. >>> I think this patch is wrong. On most MIPS devices, >>> dma_sync_single_for_cpu is a no-op. In fact, with this patch, the rx >>> path fails very quickly. >> >> Sorry for my bad email client. >> >> On most MIPS devices, dma_sync_single_for_cpu does same things >> almost with dma_unmap_single(plat_unmap_dma_mem is no-op). If >> dma_unmap_single is enough, dma_sync_single_for_cpu is certainly >> enough, >> isn't it? > Because I did some testing with these functions while writing the code, > I already know that dma_sync_single_for_cpu is not enough in this case. In theory, dma_sync_single_for_cpu is enough in this case, as explained in the commit log. On MIPS, cache invalidation is done in dma mapping for DMA_FROM_DEVICE direction, so it is correct that dma_sync_single_for_cpu or dma unmapping is only a no-op since access from CPU will trigger a refetch of the buffer from memory into cache. Also dma_sync_single_for_device still does cache invalidation for DMA_FROM_DEVICE direction, if it has to be done in this case to avoid rx failure, I guess there is some code which does touch the dma buffer after dma mapping but before dma_sync_single_for_device(should be dma_sync_single_for_cpu), maybe before dma transfer is over. If so, it should be a bug since it violates the dma api constraint, maybe we should have a careful review to check the dma api rule. > > Maybe we need to place the dma_sync_single_for_device call elsewhere and > then move the dma_sync_single_for_cpu call there afterwads, but simply > replacing this instance as is done in your patch *will* cause regressions. > > - Felix > -- Lei Ming -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
