On Wed, Jun 01, 2011 at 03:50:50PM +0200, Laurent Pinchart wrote: > In the specific iovmm case, the driver uses the sglist API to build a list of > page-size sg entries, and then process it in software. Is that considered as > an abuse of the sglist API, or valid usage ? > > Anyway, sglist chaining is not needed by iovmm. As iovmm just walks the sglist > manually, it's easier to allocate it in one go rather than using sglist > chaining. This of course doesn't make your patch unneeded or wrong. Well, there's a two issues here: 1. Should iovmm use sg_phys(sg) with sg_dma_len(sg) ? Probably not, because a scatterlist before DMA API mapping is defined by sg_page(sg), sg->offset, sg->length and has N entries. After DMA API mapping (n = dma_map_sg(dev, sg, N, dir)), it has n entries where n <= N, and the DMA address/lengths are sg_dma_address(sg) and sg_dma_len(sg). Both these are undefined for unmapped scatterlists. Getting this wrong means breakage when CONFIG_NEED_SG_DMA_LENGTH is enabled. 2. What would be the effect of enabling SG list chaining on iovmm? The code uses the correct SG list walking helpers (for_each_sg) so it should be able to cope with chained SG lists. So, I think there's no problem here with chained SG lists, but there is an issue with using sg_dma_len(). I'd suggest converting stuff to use sg->length with sg_page(sg) rather than sg_dma_len(sg). As for whether SG chaining is required or not, if you're running up against the maximum SG table size, then you do have a requirement for SG chaining. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
