[openib-general] [PATCH v3 2/7] IB/ipath - Implement new verbs DMA mapping functions

[Or Gerlitz]

Ralph Campbell wrote:
> This version of the patch adds support for ib_dma_alloc_coherent()
> and ib_dma_free_coherent().  It also fixes the bug Or found in
> ipath_sync_single_for_cpu() and ipath_sync_single_for_device().

> This patch implements the interposing DMA mapping functions to allow
> support for IOMMUs and remove the dependence on phys_to_virt().

Haven't you said that the ipath driver uses bus_to_virt ?

> diff -r c76ed2f1387b drivers/infiniband/hw/ipath/ipath_dma.c
> --- /dev/null	Thu Jan 01 00:00:00 1970 +0000
> +++ b/drivers/infiniband/hw/ipath/ipath_dma.c	Tue Dec 05 16:04:53 2006 -0800
> +/**
> + * ipath_dma_map_single - Map a kernel virtual address to DMA address
> + * @dev: The device for which the dma_addr is to be created
> + * @cpu_addr: The kernel virtual address
> + * @size: The size of the region in bytes
> + * @direction: The direction of the DMA
> + */
> +static u64 ipath_dma_map_single(struct ib_device *dev,
> +			        void *cpu_addr, size_t size,
> +			        enum dma_data_direction direction)
> +{
> +	BUG_ON(!valid_dma_direction(direction));
> +	return (u64) cpu_addr;
> +}

The documentation is both over kill in its volume and worse, simply 
tells a whole different story then what this code is doing. It does not 
generate DMA address, it does not care about the ib device nor the size 
or dma direction. Same for all the documentation below.

> +/**
> + * ipath_sg_dma_address - Return the DMA address from a scatter/gather entry
> + * @dev: The device for which the DMA addresses were created
> + * @sg: The scatter/gather entry
> + */
> +static u64 ipath_sg_dma_address(struct ib_device *dev, struct scatterlist *sg)
> +{
> +	return (u64) page_address(sg->page);
> +}

this is a bug, you need to add sg->offset

Or.