Hi Russell,

From: ext Russell King - ARM Linux <li...@arm.linux.org.uk>
Subject: Re: [PATCH 4/6] omap iommu: simple virtual address space management
Date: Sat, 16 May 2009 11:22:48 +0200

> On Tue, May 05, 2009 at 03:47:06PM +0300, Hiroshi DOYU wrote:
> > +int ioremap_page(unsigned long virt, unsigned long phys, unsigned int 
> > mtype)
> > +{
> > +   const struct mem_type *type;
> > +
> > +   type = get_mem_type(mtype);
> > +   if (!type)
> > +           return -EINVAL;
> 
> I think it would make more sense to move this lookup into the caller
> for this - you're going to be making quite a lot of calls into
> ioremap_page() so you really don't want to be doing the same lookup
> every time.
> 
> > +/* map 'sglist' to a contiguous mpu virtual area and return 'va' */
> > +static void *vmap_sg(const struct sg_table *sgt)
> > +{
> > +   u32 va;
> > +   size_t total;
> > +   unsigned int i;
> > +   struct scatterlist *sg;
> > +   struct vm_struct *new;
> > +
> > +   total = sgtable_len(sgt);
> > +   if (!total)
> > +           return ERR_PTR(-EINVAL);
> > +
> > +   new = __get_vm_area(total, VM_IOREMAP, VMALLOC_START, VMALLOC_END);
> > +   if (!new)
> > +           return ERR_PTR(-ENOMEM);
> > +   va = (u32)new->addr;
> 
> In other words, move it here.

Right. In this change for better performance, I have to expose "struct
mem_type" and "get_mem_type()" a bit more as below. Is this change
acceptable, or do you have a better idea?

diff --git a/arch/arm/include/asm/io.h b/arch/arm/include/asm/io.h
index 85a49e2..ad1cbc4 100644
--- a/arch/arm/include/asm/io.h
+++ b/arch/arm/include/asm/io.h
@@ -77,8 +77,17 @@ extern void __iounmap(volatile void __iomem *addr);
 /*
  * external interface to remap single page with appropriate type
  */
+struct mem_type {
+       unsigned int prot_pte;
+       unsigned int prot_l1;
+       unsigned int prot_sect;
+       unsigned int domain;
+};
+
+const struct mem_type *get_mem_type(unsigned int type);
+
 extern int ioremap_page(unsigned long virt, unsigned long phys,
-                       unsigned int mtype);
+                       const struct mem_type *mtype);
 
 /*
  * Bad read/write accesses...
diff --git a/arch/arm/mm/ioremap.c b/arch/arm/mm/ioremap.c
index 8441351..0ab75c6 100644
--- a/arch/arm/mm/ioremap.c
+++ b/arch/arm/mm/ioremap.c
@@ -110,15 +110,10 @@ static int remap_area_pages(unsigned long start, unsigned 
long pfn,
        return err;
 }
 
-int ioremap_page(unsigned long virt, unsigned long phys, unsigned int mtype)
+int ioremap_page(unsigned long virt, unsigned long phys,
+                const struct mem_type *mtype)
 {
-       const struct mem_type *type;
-
-       type = get_mem_type(mtype);
-       if (!type)
-               return -EINVAL;
-
-       return remap_area_pages(virt, __phys_to_pfn(phys), PAGE_SIZE, type);
+       return remap_area_pages(virt, __phys_to_pfn(phys), PAGE_SIZE, mtype);
 }
 EXPORT_SYMBOL(ioremap_page);
 
diff --git a/arch/arm/mm/mm.h b/arch/arm/mm/mm.h
index 5d9f539..57f10aa 100644
--- a/arch/arm/mm/mm.h
+++ b/arch/arm/mm/mm.h
@@ -16,15 +16,6 @@ static inline pmd_t *pmd_off_k(unsigned long virt)
        return pmd_off(pgd_offset_k(virt), virt);
 }
 
-struct mem_type {
-       unsigned int prot_pte;
-       unsigned int prot_l1;
-       unsigned int prot_sect;
-       unsigned int domain;
-};
-
-const struct mem_type *get_mem_type(unsigned int type);
-
 #endif
 
 struct map_desc;
diff --git a/arch/arm/plat-omap/iovmm.c b/arch/arm/plat-omap/iovmm.c
index a539051..020f5af 100644
--- a/arch/arm/plat-omap/iovmm.c
+++ b/arch/arm/plat-omap/iovmm.c
@@ -167,6 +167,11 @@ static void *vmap_sg(const struct sg_table *sgt)
        unsigned int i;
        struct scatterlist *sg;
        struct vm_struct *new;
+       const struct mem_type *mtype;
+
+       mtype = get_mem_type(MT_DEVICE);
+       if (!type)
+               return -EINVAL;
 
        total = sgtable_len(sgt);
        if (!total)
@@ -187,7 +192,7 @@ static void *vmap_sg(const struct sg_table *sgt)
 
                BUG_ON(bytes != PAGE_SIZE);
 
-               err = ioremap_page(va,  pa, MT_DEVICE);
+               err = ioremap_page(va,  pa, mtype);
                if (err)
                        goto err_out;
 
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" 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