Patchwork [RFC] mtd: allow mmap for ram devices in memory address space.

login
register
mail settings
Submitter Philippe De Muyter
Date March 6, 2014, 7:32 a.m.
Message ID <1394091146-26122-1-git-send-email-phdm@macqel.be>
Download mbox | patch
Permalink /patch/327328/
State New
Headers show

Comments

Philippe De Muyter - March 6, 2014, 7:32 a.m.
Implement mmap for mtd-ram, by using mtd->_get_unmapped_area, as asked by
an ancient comment, to get the physical address (if any) in memory of the
mtd device.

Therefore, mapram_unmapped_area had to be changed to return the physical
address, not the virtual one, For the NOMMU case, that makes no difference,
but for the MMU case, this is needed by mapram_unmapped_area to implement mmap.

Signed-off-by: Philippe De Muyter <phdm@macqel.be>
Cc: David Howells <dhowells@redhat.com>
Cc: Markus Niebel <list-09@tqsc.de>
---
 drivers/mtd/chips/map_ram.c |    2 +-
 drivers/mtd/mtdchar.c       |   12 +++++++++++-
 2 files changed, 12 insertions(+), 2 deletions(-)
Brian Norris - March 6, 2014, 9:39 p.m.
On Thu, Mar 06, 2014 at 08:32:26AM +0100, Philippe De Muyter wrote:
> Implement mmap for mtd-ram, by using mtd->_get_unmapped_area, as asked by
> an ancient comment, to get the physical address (if any) in memory of the
> mtd device.
> 
> Therefore, mapram_unmapped_area had to be changed to return the physical
> address, not the virtual one, For the NOMMU case, that makes no difference,
> but for the MMU case, this is needed by mapram_unmapped_area to implement mmap.
> 
> Signed-off-by: Philippe De Muyter <phdm@macqel.be>
> Cc: David Howells <dhowells@redhat.com>
> Cc: Markus Niebel <list-09@tqsc.de>
> ---
>  drivers/mtd/chips/map_ram.c |    2 +-
>  drivers/mtd/mtdchar.c       |   12 +++++++++++-
>  2 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mtd/chips/map_ram.c b/drivers/mtd/chips/map_ram.c
> index 991c2a1..e93d147 100644
> --- a/drivers/mtd/chips/map_ram.c
> +++ b/drivers/mtd/chips/map_ram.c
> @@ -92,7 +92,7 @@ static unsigned long mapram_unmapped_area(struct mtd_info *mtd,
>  					  unsigned long flags)
>  {
>  	struct map_info *map = mtd->priv;
> -	return (unsigned long) map->virt + offset;
> +	return (unsigned long) map->phys + offset;
>  }
>  
>  static int mapram_read (struct mtd_info *mtd, loff_t from, size_t len, size_t *retlen, u_char *buf)
> diff --git a/drivers/mtd/mtdchar.c b/drivers/mtd/mtdchar.c
> index 2147e73..00eac7a 100644
> --- a/drivers/mtd/mtdchar.c
> +++ b/drivers/mtd/mtdchar.c
> @@ -1112,13 +1112,23 @@ static int mtdchar_mmap(struct file *file, struct vm_area_struct *vma)
>  #ifdef CONFIG_MMU
>  	struct mtd_file_info *mfi = file->private_data;
>  	struct mtd_info *mtd = mfi->mtd;
> -	struct map_info *map = mtd->priv;
> +	loff_t offset = vma->vm_pgoff << PAGE_SHIFT;
> +	loff_t len = vma->vm_end - vma->vm_start;
>  
> +	if (offset > mtd->size - len)
> +		return (unsigned long) -EINVAL;

What's the cast doing? This function returns 'int'.

And anyway, you're duplicating the checks that are already in
mtd_get_unmapped_area(). See below.

> +	if (mtd->_get_unmapped_area) {
> +		phys_addr_t start;
> +
> +		start = mtd->_get_unmapped_area(mtd, len, offset, vma->vm_flags);

You should be using mtd_get_unmapped_area(), as the comment below says.
The underscore in front of the function pointer is to prevent people
from using them directly. You can check for -EOPNOTSUPP after calling
mtd_get_unmapped_area().

> +		return vm_iomap_memory(vma, start, len);
> +	}
>          /* This is broken because it assumes the MTD device is map-based
>  	   and that mtd->priv is a valid struct map_info.  It should be
>  	   replaced with something that uses the mtd_get_unmapped_area()
>  	   operation properly. */
>  	if (0 /*mtd->type == MTD_RAM || mtd->type == MTD_ROM*/) {
> +		struct map_info *map = mtd->priv;

You're adding dead code. It's behind an 'if (0)'. Could you perhaps take
a harder look at this code and fix up or kill any cruft?

>  #ifdef pgprot_noncached
>  		if (file->f_flags & O_DSYNC || map->phys >= __pa(high_memory))
>  			vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);

Brian

Patch

diff --git a/drivers/mtd/chips/map_ram.c b/drivers/mtd/chips/map_ram.c
index 991c2a1..e93d147 100644
--- a/drivers/mtd/chips/map_ram.c
+++ b/drivers/mtd/chips/map_ram.c
@@ -92,7 +92,7 @@  static unsigned long mapram_unmapped_area(struct mtd_info *mtd,
 					  unsigned long flags)
 {
 	struct map_info *map = mtd->priv;
-	return (unsigned long) map->virt + offset;
+	return (unsigned long) map->phys + offset;
 }
 
 static int mapram_read (struct mtd_info *mtd, loff_t from, size_t len, size_t *retlen, u_char *buf)
diff --git a/drivers/mtd/mtdchar.c b/drivers/mtd/mtdchar.c
index 2147e73..00eac7a 100644
--- a/drivers/mtd/mtdchar.c
+++ b/drivers/mtd/mtdchar.c
@@ -1112,13 +1112,23 @@  static int mtdchar_mmap(struct file *file, struct vm_area_struct *vma)
 #ifdef CONFIG_MMU
 	struct mtd_file_info *mfi = file->private_data;
 	struct mtd_info *mtd = mfi->mtd;
-	struct map_info *map = mtd->priv;
+	loff_t offset = vma->vm_pgoff << PAGE_SHIFT;
+	loff_t len = vma->vm_end - vma->vm_start;
 
+	if (offset > mtd->size - len)
+		return (unsigned long) -EINVAL;
+	if (mtd->_get_unmapped_area) {
+		phys_addr_t start;
+
+		start = mtd->_get_unmapped_area(mtd, len, offset, vma->vm_flags);
+		return vm_iomap_memory(vma, start, len);
+	}
         /* This is broken because it assumes the MTD device is map-based
 	   and that mtd->priv is a valid struct map_info.  It should be
 	   replaced with something that uses the mtd_get_unmapped_area()
 	   operation properly. */
 	if (0 /*mtd->type == MTD_RAM || mtd->type == MTD_ROM*/) {
+		struct map_info *map = mtd->priv;
 #ifdef pgprot_noncached
 		if (file->f_flags & O_DSYNC || map->phys >= __pa(high_memory))
 			vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);