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

[Ralph Campbell]

On Thu, 2006-12-07 at 09:22 +0200, Or Gerlitz wrote:
> 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 ?

It did, this patch removes that too.

> > 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.

OK.  I have removed the comments and added the following at the top:

/*
 * The following functions implement driver specific replacements
 * for the ib_dma_*() functions.
 *
 * These functions return kernel virtual addresses instead of
 * device bus addresses since the driver uses the CPU to copy
 * data instead of using hardware DMA.
 */

> > +/**
> > + * 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.

Thanks, applied.