Hi Ohad,

Thanks for the patch.

On Friday 03 June 2011 00:27:39 Ohad Ben-Cohen wrote:
> Migrate omap's iovmm (virtual memory manager) to the generic iommu api.
> 
> This brings iovmm a step forward towards being completely non
> omap-specific (it's still assuming omap's iommu page sizes, and also
> maintaining state inside omap's internal iommu structure, but it no
> longer calls omap-specific iommu map/unmap api).
> 
> Further generalizing of iovmm (or complete removal) should take place
> together with broader plans of providing a generic virtual memory manager
> and allocation framework (de-coupled from specific mappers).
> 
> Signed-off-by: Ohad Ben-Cohen <o...@wizery.com>

[snip]

> diff --git a/arch/arm/plat-omap/iovmm.c b/arch/arm/plat-omap/iovmm.c
> index 51ef43e..80bb2b6 100644
> --- a/arch/arm/plat-omap/iovmm.c
> +++ b/arch/arm/plat-omap/iovmm.c

[snip]

> @@ -473,22 +475,22 @@ static int map_iovm_area(struct iommu *obj, struct
> iovm_struct *new, u32 pa;
>               int pgsz;
>               size_t bytes;
> -             struct iotlb_entry e;
> 
>               pa = sg_phys(sg);
>               bytes = sg_dma_len(sg);
> 
>               flags &= ~IOVMF_PGSZ_MASK;
> +
>               pgsz = bytes_to_iopgsz(bytes);
>               if (pgsz < 0)
>                       goto err_out;
> -             flags |= pgsz;

pgsz isn't used anymore, you can remove it.

> +
> +             order = get_order(bytes);

Does iommu_map() handle offsets correctly, or does it expect pa to be aligned 
to an order (or other) boundary ? Same comment for iommu_unmap() in 
unmap_iovm_area().

>               pr_debug("%s: [%d] %08x %08x(%x)\n", __func__,
>                        i, da, pa, bytes);
> 
> -             iotlb_init_entry(&e, da, pa, flags);
> -             err = iopgtable_store_entry(obj, &e);
> +             err = iommu_map(domain, da, pa, order, flags);
>               if (err)
>                       goto err_out;
> 
> @@ -502,9 +504,11 @@ err_out:
>       for_each_sg(sgt->sgl, sg, i, j) {
>               size_t bytes;
> 
> -             bytes = iopgtable_clear_entry(obj, da);
> +             bytes = sg_dma_len(sg);

As Russell pointed out, we should use sg->length instead of sg_dma_length(sg). 
sg_dma_length(sg) is only valid after the scatter list has been DMA-mapped, 
which doesn't happen in the iovmm driver. This applies to all sg_dma_len(sg) 
calls.

> +             order = get_order(bytes);
> 
> -             BUG_ON(!iopgsz_ok(bytes));
> +             /* ignore failures.. we're already handling one */
> +             iommu_unmap(domain, da, order);
> 
>               da += bytes;
>       }

-- 
Regards,

Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to