[PATCH 0/5] unconfuse get_unmapped_area and point/unpoint driver methods

[PATCH 0/5] unconfuse get_unmapped_area and point/unpoint driver methods

[Nicolas Pitre]

This is my first MTD contribution after more than 10 years.
Feels like Back to the Future!  :-)

This series improves or implements the point and unpoint driver methods
and reimplements mtd_get_unmapped_area() in terms of those. Because
mtd->_point() provides a superset of what mtd->_get_unmapped_area() does
then there is no reason for having both. Or worse: having one but not
the other.

This is motivated by my work on XIP cramfs for both MMU and !MMU systems
where I do need the more complete point functionality. And it just looks
like the right thing to do. Existing users won't see a difference.

diffstat:

 drivers/mtd/chips/map_ram.c  | 34 ++++++++++++++++++++--------------
 drivers/mtd/chips/map_rom.c  | 34 +++++++++++++++++++++-------------
 drivers/mtd/devices/mtdram.c | 36 ++++++++++++++++++++++--------------
 drivers/mtd/mtdconcat.c      | 27 ---------------------------
 drivers/mtd/mtdcore.c        | 17 ++++++++++++-----
 drivers/mtd/mtdpart.c        | 14 --------------
 include/linux/mtd/mtd.h      |  4 ----
 7 files changed, 75 insertions(+), 91 deletions(-)

[PATCH 1/5] MTD: mtdram: properly handle the phys argument in the point method

[Nicolas Pitre]

When the phys pointer is non null, the point method is expected to return
the physical address for the pointed area. In the case of the mtdram
driver we have to retrieve the physical address for the corresponding
vmalloc area. However, there is no guarantee that the vmalloc area is
made of physically contiguous pages. In that case we simply limit retlen
to the actually contiguous pages.

Signed-off-by: Nicolas Pitre <nico@linaro.org>
---
 drivers/mtd/devices/mtdram.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/mtd/devices/mtdram.c b/drivers/mtd/devices/mtdram.c
index cbd8547d7a..7dbee8e62f 100644
--- a/drivers/mtd/devices/mtdram.c
+++ b/drivers/mtd/devices/mtdram.c
@@ -13,6 +13,7 @@
 #include <linux/slab.h>
 #include <linux/ioport.h>
 #include <linux/vmalloc.h>
+#include <linux/mm.h>
 #include <linux/init.h>
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/mtdram.h>
@@ -69,6 +70,27 @@ static int ram_point(struct mtd_info *mtd, loff_t from, size_t len,
 {
 	*virt = mtd->priv + from;
 	*retlen = len;
+
+	if (phys) {
+		/* limit retlen to the number of contiguous physical pages */
+		unsigned long page_ofs = offset_in_page(*virt);
+		void *addr = *virt - page_ofs;
+		unsigned long pfn1, pfn0 = vmalloc_to_pfn(addr);
+
+		*phys = __pfn_to_phys(pfn0) + page_ofs;
+		len += page_ofs;
+		while (len > PAGE_SIZE) {
+			len -= PAGE_SIZE;
+			addr += PAGE_SIZE;
+			pfn0++;
+			pfn1 = vmalloc_to_pfn(addr);
+			if (pfn1 != pfn0) {
+				*retlen = *virt - addr;
+				break;
+			}
+		}
+	}
+
 	return 0;
 }
 
-- 
2.9.5

Re: [PATCH 1/5] MTD: mtdram: properly handle the phys argument in the point method

[Richard Weinberger]

On Wed, Oct 11, 2017 at 5:26 AM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> When the phys pointer is non null, the point method is expected to return
> the physical address for the pointed area. In the case of the mtdram
> driver we have to retrieve the physical address for the corresponding
> vmalloc area. However, there is no guarantee that the vmalloc area is
> made of physically contiguous pages. In that case we simply limit retlen
> to the actually contiguous pages.
>
> Signed-off-by: Nicolas Pitre <nico@linaro.org>
> ---
>  drivers/mtd/devices/mtdram.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
>
> diff --git a/drivers/mtd/devices/mtdram.c b/drivers/mtd/devices/mtdram.c
> index cbd8547d7a..7dbee8e62f 100644
> --- a/drivers/mtd/devices/mtdram.c
> +++ b/drivers/mtd/devices/mtdram.c
> @@ -13,6 +13,7 @@
>  #include <linux/slab.h>
>  #include <linux/ioport.h>
>  #include <linux/vmalloc.h>
> +#include <linux/mm.h>
>  #include <linux/init.h>
>  #include <linux/mtd/mtd.h>
>  #include <linux/mtd/mtdram.h>
> @@ -69,6 +70,27 @@ static int ram_point(struct mtd_info *mtd, loff_t from, size_t len,
>  {
>         *virt = mtd->priv + from;
>         *retlen = len;
> +
> +       if (phys) {
> +               /* limit retlen to the number of contiguous physical pages */
> +               unsigned long page_ofs = offset_in_page(*virt);
> +               void *addr = *virt - page_ofs;
> +               unsigned long pfn1, pfn0 = vmalloc_to_pfn(addr);
> +
> +               *phys = __pfn_to_phys(pfn0) + page_ofs;
> +               len += page_ofs;
> +               while (len > PAGE_SIZE) {
> +                       len -= PAGE_SIZE;
> +                       addr += PAGE_SIZE;
> +                       pfn0++;
> +                       pfn1 = vmalloc_to_pfn(addr);
> +                       if (pfn1 != pfn0) {
> +                               *retlen = *virt - addr;
> +                               break;
> +                       }
> +               }
> +       }
> +
>         return 0;
>  }

Reviewed-by: Richard Weinberger <richard@nod.at>

-- 
Thanks,
//richard

Re: [PATCH 1/5] MTD: mtdram: properly handle the phys argument in the point method

[Nicolas Pitre]

On Wed, 11 Oct 2017, Richard Weinberger wrote:

> On Wed, Oct 11, 2017 at 5:26 AM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> > When the phys pointer is non null, the point method is expected to return
> > the physical address for the pointed area. In the case of the mtdram
> > driver we have to retrieve the physical address for the corresponding
> > vmalloc area. However, there is no guarantee that the vmalloc area is
> > made of physically contiguous pages. In that case we simply limit retlen
> > to the actually contiguous pages.
> >
> > Signed-off-by: Nicolas Pitre <nico@linaro.org>
> > ---
> >  drivers/mtd/devices/mtdram.c | 22 ++++++++++++++++++++++
> >  1 file changed, 22 insertions(+)
> >
> > diff --git a/drivers/mtd/devices/mtdram.c b/drivers/mtd/devices/mtdram.c
> > index cbd8547d7a..7dbee8e62f 100644
> > --- a/drivers/mtd/devices/mtdram.c
> > +++ b/drivers/mtd/devices/mtdram.c
> > @@ -13,6 +13,7 @@
> >  #include <linux/slab.h>
> >  #include <linux/ioport.h>
> >  #include <linux/vmalloc.h>
> > +#include <linux/mm.h>
> >  #include <linux/init.h>
> >  #include <linux/mtd/mtd.h>
> >  #include <linux/mtd/mtdram.h>
> > @@ -69,6 +70,27 @@ static int ram_point(struct mtd_info *mtd, loff_t from, size_t len,
> >  {
> >         *virt = mtd->priv + from;
> >         *retlen = len;
> > +
> > +       if (phys) {
> > +               /* limit retlen to the number of contiguous physical pages */
> > +               unsigned long page_ofs = offset_in_page(*virt);
> > +               void *addr = *virt - page_ofs;
> > +               unsigned long pfn1, pfn0 = vmalloc_to_pfn(addr);
> > +
> > +               *phys = __pfn_to_phys(pfn0) + page_ofs;
> > +               len += page_ofs;
> > +               while (len > PAGE_SIZE) {
> > +                       len -= PAGE_SIZE;
> > +                       addr += PAGE_SIZE;
> > +                       pfn0++;
> > +                       pfn1 = vmalloc_to_pfn(addr);
> > +                       if (pfn1 != pfn0) {
> > +                               *retlen = *virt - addr;

In fact the above should be: *retlen = addr - *virt;

Let me know if I should repost.


Nicolas

Re: [PATCH 1/5] MTD: mtdram: properly handle the phys argument in the point method

[Richard Weinberger]

On Wed, Oct 11, 2017 at 11:32 PM, Nicolas Pitre
<nicolas.pitre@linaro.org> wrote:
> In fact the above should be: *retlen = addr - *virt;

Hmm, yes.

While we are here, how did you test that code?

> Let me know if I should repost.

Yes, please. But wait a few days, hopefully we get more reviews.

-- 
Thanks,
//richard

Re: [PATCH 1/5] MTD: mtdram: properly handle the phys argument in the point method

[Nicolas Pitre]

On Wed, 11 Oct 2017, Richard Weinberger wrote:

> On Wed, Oct 11, 2017 at 11:32 PM, Nicolas Pitre
> <nicolas.pitre@linaro.org> wrote:
> > In fact the above should be: *retlen = addr - *virt;
> 
> Hmm, yes.
> 
> While we are here, how did you test that code?

I didn't actually manage to trigger it as physical pages were always 
contiguous in my testing. So this is from inspection i.e. I just re-read 
the code from your reply.

> > Let me know if I should repost.
> 
> Yes, please. But wait a few days, hopefully we get more reviews.

Sure.


Nicolas

[PATCH 2/5] MTD: chips/map_ram.c: implement point and unpoint methods

[Nicolas Pitre]

Signed-off-by: Nicolas Pitre <nico@linaro.org>
---
 drivers/mtd/chips/map_ram.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/mtd/chips/map_ram.c b/drivers/mtd/chips/map_ram.c
index afb43d5e17..c3939dd230 100644
--- a/drivers/mtd/chips/map_ram.c
+++ b/drivers/mtd/chips/map_ram.c
@@ -22,6 +22,9 @@ static void mapram_nop (struct mtd_info *);
 static struct mtd_info *map_ram_probe(struct map_info *map);
 static unsigned long mapram_unmapped_area(struct mtd_info *, unsigned long,
 					  unsigned long, unsigned long);
+static int mapram_point (struct mtd_info *mtd, loff_t from, size_t len,
+			 size_t *retlen, void **virt, resource_size_t *phys);
+static int mapram_unpoint(struct mtd_info *mtd, loff_t from, size_t len);
 
 
 static struct mtd_chip_driver mapram_chipdrv = {
@@ -69,7 +72,9 @@ static struct mtd_info *map_ram_probe(struct map_info *map)
 	mtd->_read = mapram_read;
 	mtd->_write = mapram_write;
 	mtd->_panic_write = mapram_write;
+	mtd->_point = mapram_point;
 	mtd->_sync = mapram_nop;
+	mtd->_unpoint = mapram_unpoint;
 	mtd->flags = MTD_CAP_RAM;
 	mtd->writesize = 1;
 
@@ -96,6 +101,25 @@ static unsigned long mapram_unmapped_area(struct mtd_info *mtd,
 	return (unsigned long) map->virt + offset;
 }
 
+static int mapram_point(struct mtd_info *mtd, loff_t from, size_t len,
+			size_t *retlen, void **virt, resource_size_t *phys)
+{
+	struct map_info *map = mtd->priv;
+
+	if (!map->virt)
+		return -EINVAL;
+	*virt = map->virt + from;
+	if (phys)
+		*phys = map->phys + from;
+	*retlen = len;
+	return 0;
+}
+
+static int mapram_unpoint(struct mtd_info *mtd, loff_t from, size_t len)
+{
+	return 0;
+}
+
 static int mapram_read (struct mtd_info *mtd, loff_t from, size_t len, size_t *retlen, u_char *buf)
 {
 	struct map_info *map = mtd->priv;
-- 
2.9.5

Re: [PATCH 2/5] MTD: chips/map_ram.c: implement point and unpoint methods

[Richard Weinberger]

On Wed, Oct 11, 2017 at 5:26 AM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> Signed-off-by: Nicolas Pitre <nico@linaro.org>
> ---
>  drivers/mtd/chips/map_ram.c | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
>
> diff --git a/drivers/mtd/chips/map_ram.c b/drivers/mtd/chips/map_ram.c
> index afb43d5e17..c3939dd230 100644
> --- a/drivers/mtd/chips/map_ram.c
> +++ b/drivers/mtd/chips/map_ram.c
> @@ -22,6 +22,9 @@ static void mapram_nop (struct mtd_info *);
>  static struct mtd_info *map_ram_probe(struct map_info *map);
>  static unsigned long mapram_unmapped_area(struct mtd_info *, unsigned long,
>                                           unsigned long, unsigned long);
> +static int mapram_point (struct mtd_info *mtd, loff_t from, size_t len,
> +                        size_t *retlen, void **virt, resource_size_t *phys);
> +static int mapram_unpoint(struct mtd_info *mtd, loff_t from, size_t len);
>
>
>  static struct mtd_chip_driver mapram_chipdrv = {
> @@ -69,7 +72,9 @@ static struct mtd_info *map_ram_probe(struct map_info *map)
>         mtd->_read = mapram_read;
>         mtd->_write = mapram_write;
>         mtd->_panic_write = mapram_write;
> +       mtd->_point = mapram_point;
>         mtd->_sync = mapram_nop;
> +       mtd->_unpoint = mapram_unpoint;
>         mtd->flags = MTD_CAP_RAM;
>         mtd->writesize = 1;
>
> @@ -96,6 +101,25 @@ static unsigned long mapram_unmapped_area(struct mtd_info *mtd,
>         return (unsigned long) map->virt + offset;
>  }
>
> +static int mapram_point(struct mtd_info *mtd, loff_t from, size_t len,
> +                       size_t *retlen, void **virt, resource_size_t *phys)
> +{
> +       struct map_info *map = mtd->priv;
> +
> +       if (!map->virt)
> +               return -EINVAL;
> +       *virt = map->virt + from;
> +       if (phys)
> +               *phys = map->phys + from;
> +       *retlen = len;
> +       return 0;
> +}
> +
> +static int mapram_unpoint(struct mtd_info *mtd, loff_t from, size_t len)
> +{
> +       return 0;
> +}
> +

Reviewed-by: Richard Weinberger <richard@nod.at>

-- 
Thanks,
//richard

[PATCH 3/5] MTD: chips/map_rom.c: implement point and unpoint methods

[Nicolas Pitre]

Signed-off-by: Nicolas Pitre <nico@linaro.org>
---
 drivers/mtd/chips/map_rom.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/mtd/chips/map_rom.c b/drivers/mtd/chips/map_rom.c
index e67f73ab44..722afb1cf4 100644
--- a/drivers/mtd/chips/map_rom.c
+++ b/drivers/mtd/chips/map_rom.c
@@ -22,6 +22,10 @@ static struct mtd_info *map_rom_probe(struct map_info *map);
 static int maprom_erase (struct mtd_info *mtd, struct erase_info *info);
 static unsigned long maprom_unmapped_area(struct mtd_info *, unsigned long,
 					  unsigned long, unsigned long);
+static int maprom_point (struct mtd_info *mtd, loff_t from, size_t len,
+			 size_t *retlen, void **virt, resource_size_t *phys);
+static int maprom_unpoint(struct mtd_info *mtd, loff_t from, size_t len);
+
 
 static struct mtd_chip_driver maprom_chipdrv = {
 	.probe	= map_rom_probe,
@@ -52,6 +56,8 @@ static struct mtd_info *map_rom_probe(struct map_info *map)
 	mtd->type = MTD_ROM;
 	mtd->size = map->size;
 	mtd->_get_unmapped_area = maprom_unmapped_area;
+	mtd->_point = maprom_point;
+	mtd->_unpoint = maprom_unpoint;
 	mtd->_read = maprom_read;
 	mtd->_write = maprom_write;
 	mtd->_sync = maprom_nop;
@@ -78,6 +84,24 @@ static unsigned long maprom_unmapped_area(struct mtd_info *mtd,
 {
 	struct map_info *map = mtd->priv;
 	return (unsigned long) map->virt + offset;
+
+static int maprom_point(struct mtd_info *mtd, loff_t from, size_t len,
+			size_t *retlen, void **virt, resource_size_t *phys)
+{
+	struct map_info *map = mtd->priv;
+
+	if (!map->virt)
+		return -EINVAL;
+	*virt = map->virt + from;
+	if (phys)
+		*phys = map->phys + from;
+	*retlen = len;
+	return 0;
+}
+
+static int maprom_unpoint(struct mtd_info *mtd, loff_t from, size_t len)
+{
+	return 0;
 }
 
 static int maprom_read (struct mtd_info *mtd, loff_t from, size_t len, size_t *retlen, u_char *buf)
-- 
2.9.5

Re: [PATCH 3/5] MTD: chips/map_rom.c: implement point and unpoint methods

[Richard Weinberger]

On Wed, Oct 11, 2017 at 5:26 AM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> Signed-off-by: Nicolas Pitre <nico@linaro.org>
> ---
>  drivers/mtd/chips/map_rom.c | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
>
> diff --git a/drivers/mtd/chips/map_rom.c b/drivers/mtd/chips/map_rom.c
> index e67f73ab44..722afb1cf4 100644
> --- a/drivers/mtd/chips/map_rom.c
> +++ b/drivers/mtd/chips/map_rom.c
> @@ -22,6 +22,10 @@ static struct mtd_info *map_rom_probe(struct map_info *map);
>  static int maprom_erase (struct mtd_info *mtd, struct erase_info *info);
>  static unsigned long maprom_unmapped_area(struct mtd_info *, unsigned long,
>                                           unsigned long, unsigned long);
> +static int maprom_point (struct mtd_info *mtd, loff_t from, size_t len,
> +                        size_t *retlen, void **virt, resource_size_t *phys);
> +static int maprom_unpoint(struct mtd_info *mtd, loff_t from, size_t len);
> +
>
>  static struct mtd_chip_driver maprom_chipdrv = {
>         .probe  = map_rom_probe,
> @@ -52,6 +56,8 @@ static struct mtd_info *map_rom_probe(struct map_info *map)
>         mtd->type = MTD_ROM;
>         mtd->size = map->size;
>         mtd->_get_unmapped_area = maprom_unmapped_area;
> +       mtd->_point = maprom_point;
> +       mtd->_unpoint = maprom_unpoint;
>         mtd->_read = maprom_read;
>         mtd->_write = maprom_write;
>         mtd->_sync = maprom_nop;
> @@ -78,6 +84,24 @@ static unsigned long maprom_unmapped_area(struct mtd_info *mtd,
>  {
>         struct map_info *map = mtd->priv;
>         return (unsigned long) map->virt + offset;
> +
> +static int maprom_point(struct mtd_info *mtd, loff_t from, size_t len,
> +                       size_t *retlen, void **virt, resource_size_t *phys)
> +{
> +       struct map_info *map = mtd->priv;
> +
> +       if (!map->virt)
> +               return -EINVAL;
> +       *virt = map->virt + from;
> +       if (phys)
> +               *phys = map->phys + from;
> +       *retlen = len;
> +       return 0;
> +}
> +
> +static int maprom_unpoint(struct mtd_info *mtd, loff_t from, size_t len)
> +{
> +       return 0;
>  }

Can we please have a generic helper function for un/point instead of
copy&pasting?

-- 
Thanks,
//richard

Re: [PATCH 3/5] MTD: chips/map_rom.c: implement point and unpoint methods

[Nicolas Pitre]

On Wed, 11 Oct 2017, Richard Weinberger wrote:

> On Wed, Oct 11, 2017 at 5:26 AM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> > Signed-off-by: Nicolas Pitre <nico@linaro.org>
> > ---
> >  drivers/mtd/chips/map_rom.c | 24 ++++++++++++++++++++++++
> >  1 file changed, 24 insertions(+)
> >
> > diff --git a/drivers/mtd/chips/map_rom.c b/drivers/mtd/chips/map_rom.c
> > index e67f73ab44..722afb1cf4 100644
> > --- a/drivers/mtd/chips/map_rom.c
> > +++ b/drivers/mtd/chips/map_rom.c
> > @@ -22,6 +22,10 @@ static struct mtd_info *map_rom_probe(struct map_info *map);
> >  static int maprom_erase (struct mtd_info *mtd, struct erase_info *info);
> >  static unsigned long maprom_unmapped_area(struct mtd_info *, unsigned long,
> >                                           unsigned long, unsigned long);
> > +static int maprom_point (struct mtd_info *mtd, loff_t from, size_t len,
> > +                        size_t *retlen, void **virt, resource_size_t *phys);
> > +static int maprom_unpoint(struct mtd_info *mtd, loff_t from, size_t len);
> > +
> >
> >  static struct mtd_chip_driver maprom_chipdrv = {
> >         .probe  = map_rom_probe,
> > @@ -52,6 +56,8 @@ static struct mtd_info *map_rom_probe(struct map_info *map)
> >         mtd->type = MTD_ROM;
> >         mtd->size = map->size;
> >         mtd->_get_unmapped_area = maprom_unmapped_area;
> > +       mtd->_point = maprom_point;
> > +       mtd->_unpoint = maprom_unpoint;
> >         mtd->_read = maprom_read;
> >         mtd->_write = maprom_write;
> >         mtd->_sync = maprom_nop;
> > @@ -78,6 +84,24 @@ static unsigned long maprom_unmapped_area(struct mtd_info *mtd,
> >  {
> >         struct map_info *map = mtd->priv;
> >         return (unsigned long) map->virt + offset;
> > +
> > +static int maprom_point(struct mtd_info *mtd, loff_t from, size_t len,
> > +                       size_t *retlen, void **virt, resource_size_t *phys)
> > +{
> > +       struct map_info *map = mtd->priv;
> > +
> > +       if (!map->virt)
> > +               return -EINVAL;
> > +       *virt = map->virt + from;
> > +       if (phys)
> > +               *phys = map->phys + from;
> > +       *retlen = len;
> > +       return 0;
> > +}
> > +
> > +static int maprom_unpoint(struct mtd_info *mtd, loff_t from, size_t len)
> > +{
> > +       return 0;
> >  }
> 
> Can we please have a generic helper function for un/point instead of
> copy&pasting?

I'd prefer not. In this case (and the mapram case) the point/unpoint 
code is unusuably simple.  I don't want people to think that such 
simpleness is expected by making this into a common or generic function. 
Normally this ought to be more involved (see cfi_cmdset_0001 for 
a real example).


Nicolas

[PATCH 4/5] MTD: implement mtd_get_unmapped_area() using the point method

[Nicolas Pitre]

The mtd->_point method is a superset of mtd->_get_unmapped_area.
Especially in the NOR flash case, the point method ensures the flash
memory is in array (data) mode and that it will stay that way which
is precisely what callers of mtd_get_unmapped_area() would expect.

Implement mtd_get_unmapped_area() in terms of mtd->_point now that all
drivers that provided a _get_unmapped_area method also have the _point
method implemented.

Signed-off-by: Nicolas Pitre <nico@linaro.org>
---
 drivers/mtd/mtdcore.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index e7ea842ba3..ecb0380158 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -1022,11 +1022,18 @@ EXPORT_SYMBOL_GPL(mtd_unpoint);
 unsigned long mtd_get_unmapped_area(struct mtd_info *mtd, unsigned long len,
 				    unsigned long offset, unsigned long flags)
 {
-	if (!mtd->_get_unmapped_area)
-		return -EOPNOTSUPP;
-	if (offset >= mtd->size || len > mtd->size - offset)
-		return -EINVAL;
-	return mtd->_get_unmapped_area(mtd, len, offset, flags);
+	size_t retlen;
+	void *virt;
+	int ret;
+
+	ret = mtd_point(mtd, offset, len, &retlen, &virt, NULL);
+	if (ret)
+		return ret;
+	if (retlen != len) {
+		mtd_unpoint(mtd, offset, retlen);
+		return -ENOSYS;
+	}
+	return (unsigned long)virt;
 }
 EXPORT_SYMBOL_GPL(mtd_get_unmapped_area);
 
-- 
2.9.5

Re: [PATCH 4/5] MTD: implement mtd_get_unmapped_area() using the point method

[Richard Weinberger]

On Wed, Oct 11, 2017 at 5:26 AM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> The mtd->_point method is a superset of mtd->_get_unmapped_area.
> Especially in the NOR flash case, the point method ensures the flash
> memory is in array (data) mode and that it will stay that way which
> is precisely what callers of mtd_get_unmapped_area() would expect.
>
> Implement mtd_get_unmapped_area() in terms of mtd->_point now that all
> drivers that provided a _get_unmapped_area method also have the _point
> method implemented.
>
> Signed-off-by: Nicolas Pitre <nico@linaro.org>
> ---
>  drivers/mtd/mtdcore.c | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> index e7ea842ba3..ecb0380158 100644
> --- a/drivers/mtd/mtdcore.c
> +++ b/drivers/mtd/mtdcore.c
> @@ -1022,11 +1022,18 @@ EXPORT_SYMBOL_GPL(mtd_unpoint);
>  unsigned long mtd_get_unmapped_area(struct mtd_info *mtd, unsigned long len,
>                                     unsigned long offset, unsigned long flags)
>  {
> -       if (!mtd->_get_unmapped_area)
> -               return -EOPNOTSUPP;
> -       if (offset >= mtd->size || len > mtd->size - offset)
> -               return -EINVAL;
> -       return mtd->_get_unmapped_area(mtd, len, offset, flags);
> +       size_t retlen;
> +       void *virt;
> +       int ret;
> +
> +       ret = mtd_point(mtd, offset, len, &retlen, &virt, NULL);
> +       if (ret)
> +               return ret;
> +       if (retlen != len) {
> +               mtd_unpoint(mtd, offset, retlen);
> +               return -ENOSYS;
> +       }
> +       return (unsigned long)virt;
>  }
>  EXPORT_SYMBOL_GPL(mtd_get_unmapped_area);

Reviewed-by: Richard Weinberger <richard@nod.at>

-- 
Thanks,
//richard

[PATCH 5/5] MTD: remove the get_unmapped_area method

[Nicolas Pitre]

It is now unused.

Signed-off-by: Nicolas Pitre <nico@linaro.org>
---
 drivers/mtd/chips/map_ram.c  | 18 ------------------
 drivers/mtd/chips/map_rom.c  | 16 ----------------
 drivers/mtd/devices/mtdram.c | 14 --------------
 drivers/mtd/mtdconcat.c      | 27 ---------------------------
 drivers/mtd/mtdpart.c        | 14 --------------
 include/linux/mtd/mtd.h      |  4 ----
 6 files changed, 93 deletions(-)

diff --git a/drivers/mtd/chips/map_ram.c b/drivers/mtd/chips/map_ram.c
index c3939dd230..1cd0fff0e9 100644
--- a/drivers/mtd/chips/map_ram.c
+++ b/drivers/mtd/chips/map_ram.c
@@ -20,8 +20,6 @@ static int mapram_write (struct mtd_info *, loff_t, size_t, size_t *, const u_ch
 static int mapram_erase (struct mtd_info *, struct erase_info *);
 static void mapram_nop (struct mtd_info *);
 static struct mtd_info *map_ram_probe(struct map_info *map);
-static unsigned long mapram_unmapped_area(struct mtd_info *, unsigned long,
-					  unsigned long, unsigned long);
 static int mapram_point (struct mtd_info *mtd, loff_t from, size_t len,
 			 size_t *retlen, void **virt, resource_size_t *phys);
 static int mapram_unpoint(struct mtd_info *mtd, loff_t from, size_t len);
@@ -68,7 +66,6 @@ static struct mtd_info *map_ram_probe(struct map_info *map)
 	mtd->type = MTD_RAM;
 	mtd->size = map->size;
 	mtd->_erase = mapram_erase;
-	mtd->_get_unmapped_area = mapram_unmapped_area;
 	mtd->_read = mapram_read;
 	mtd->_write = mapram_write;
 	mtd->_panic_write = mapram_write;
@@ -86,21 +83,6 @@ static struct mtd_info *map_ram_probe(struct map_info *map)
 	return mtd;
 }
 
-
-/*
- * Allow NOMMU mmap() to directly map the device (if not NULL)
- * - return the address to which the offset maps
- * - return -ENOSYS to indicate refusal to do the mapping
- */
-static unsigned long mapram_unmapped_area(struct mtd_info *mtd,
-					  unsigned long len,
-					  unsigned long offset,
-					  unsigned long flags)
-{
-	struct map_info *map = mtd->priv;
-	return (unsigned long) map->virt + offset;
-}
-
 static int mapram_point(struct mtd_info *mtd, loff_t from, size_t len,
 			size_t *retlen, void **virt, resource_size_t *phys)
 {
diff --git a/drivers/mtd/chips/map_rom.c b/drivers/mtd/chips/map_rom.c
index 722afb1cf4..20e3604b4d 100644
--- a/drivers/mtd/chips/map_rom.c
+++ b/drivers/mtd/chips/map_rom.c
@@ -20,8 +20,6 @@ static int maprom_write (struct mtd_info *, loff_t, size_t, size_t *, const u_ch
 static void maprom_nop (struct mtd_info *);
 static struct mtd_info *map_rom_probe(struct map_info *map);
 static int maprom_erase (struct mtd_info *mtd, struct erase_info *info);
-static unsigned long maprom_unmapped_area(struct mtd_info *, unsigned long,
-					  unsigned long, unsigned long);
 static int maprom_point (struct mtd_info *mtd, loff_t from, size_t len,
 			 size_t *retlen, void **virt, resource_size_t *phys);
 static int maprom_unpoint(struct mtd_info *mtd, loff_t from, size_t len);
@@ -55,7 +53,6 @@ static struct mtd_info *map_rom_probe(struct map_info *map)
 	mtd->name = map->name;
 	mtd->type = MTD_ROM;
 	mtd->size = map->size;
-	mtd->_get_unmapped_area = maprom_unmapped_area;
 	mtd->_point = maprom_point;
 	mtd->_unpoint = maprom_unpoint;
 	mtd->_read = maprom_read;
@@ -72,19 +69,6 @@ static struct mtd_info *map_rom_probe(struct map_info *map)
 }
 
 
-/*
- * Allow NOMMU mmap() to directly map the device (if not NULL)
- * - return the address to which the offset maps
- * - return -ENOSYS to indicate refusal to do the mapping
- */
-static unsigned long maprom_unmapped_area(struct mtd_info *mtd,
-					  unsigned long len,
-					  unsigned long offset,
-					  unsigned long flags)
-{
-	struct map_info *map = mtd->priv;
-	return (unsigned long) map->virt + offset;
-
 static int maprom_point(struct mtd_info *mtd, loff_t from, size_t len,
 			size_t *retlen, void **virt, resource_size_t *phys)
 {
diff --git a/drivers/mtd/devices/mtdram.c b/drivers/mtd/devices/mtdram.c
index 7dbee8e62f..d4c94bd46d 100644
--- a/drivers/mtd/devices/mtdram.c
+++ b/drivers/mtd/devices/mtdram.c
@@ -99,19 +99,6 @@ static int ram_unpoint(struct mtd_info *mtd, loff_t from, size_t len)
 	return 0;
 }
 
-/*
- * Allow NOMMU mmap() to directly map the device (if not NULL)
- * - return the address to which the offset maps
- * - return -ENOSYS to indicate refusal to do the mapping
- */
-static unsigned long ram_get_unmapped_area(struct mtd_info *mtd,
-					   unsigned long len,
-					   unsigned long offset,
-					   unsigned long flags)
-{
-	return (unsigned long) mtd->priv + offset;
-}
-
 static int ram_read(struct mtd_info *mtd, loff_t from, size_t len,
 		size_t *retlen, u_char *buf)
 {
@@ -156,7 +143,6 @@ int mtdram_init_device(struct mtd_info *mtd, void *mapped_address,
 	mtd->_erase = ram_erase;
 	mtd->_point = ram_point;
 	mtd->_unpoint = ram_unpoint;
-	mtd->_get_unmapped_area = ram_get_unmapped_area;
 	mtd->_read = ram_read;
 	mtd->_write = ram_write;
 
diff --git a/drivers/mtd/mtdconcat.c b/drivers/mtd/mtdconcat.c
index d573606b91..60bf53df54 100644
--- a/drivers/mtd/mtdconcat.c
+++ b/drivers/mtd/mtdconcat.c
@@ -644,32 +644,6 @@ static int concat_block_markbad(struct mtd_info *mtd, loff_t ofs)
 }
 
 /*
- * try to support NOMMU mmaps on concatenated devices
- * - we don't support subdev spanning as we can't guarantee it'll work
- */
-static unsigned long concat_get_unmapped_area(struct mtd_info *mtd,
-					      unsigned long len,
-					      unsigned long offset,
-					      unsigned long flags)
-{
-	struct mtd_concat *concat = CONCAT(mtd);
-	int i;
-
-	for (i = 0; i < concat->num_subdev; i++) {
-		struct mtd_info *subdev = concat->subdev[i];
-
-		if (offset >= subdev->size) {
-			offset -= subdev->size;
-			continue;
-		}
-
-		return mtd_get_unmapped_area(subdev, len, offset, flags);
-	}
-
-	return (unsigned long) -ENOSYS;
-}
-
-/*
  * This function constructs a virtual MTD device by concatenating
  * num_devs MTD devices. A pointer to the new device object is
  * stored to *new_dev upon success. This function does _not_
@@ -790,7 +764,6 @@ struct mtd_info *mtd_concat_create(struct mtd_info *subdev[],	/* subdevices to c
 	concat->mtd._unlock = concat_unlock;
 	concat->mtd._suspend = concat_suspend;
 	concat->mtd._resume = concat_resume;
-	concat->mtd._get_unmapped_area = concat_get_unmapped_area;
 
 	/*
 	 * Combine the erase block size info of the subdevices:
diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index a308e70739..be088bccd5 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -101,18 +101,6 @@ static int part_unpoint(struct mtd_info *mtd, loff_t from, size_t len)
 	return part->parent->_unpoint(part->parent, from + part->offset, len);
 }
 
-static unsigned long part_get_unmapped_area(struct mtd_info *mtd,
-					    unsigned long len,
-					    unsigned long offset,
-					    unsigned long flags)
-{
-	struct mtd_part *part = mtd_to_part(mtd);
-
-	offset += part->offset;
-	return part->parent->_get_unmapped_area(part->parent, len, offset,
-						flags);
-}
-
 static int part_read_oob(struct mtd_info *mtd, loff_t from,
 		struct mtd_oob_ops *ops)
 {
@@ -458,8 +446,6 @@ static struct mtd_part *allocate_partition(struct mtd_info *parent,
 		slave->mtd._unpoint = part_unpoint;
 	}
 
-	if (parent->_get_unmapped_area)
-		slave->mtd._get_unmapped_area = part_get_unmapped_area;
 	if (parent->_read_oob)
 		slave->mtd._read_oob = part_read_oob;
 	if (parent->_write_oob)
diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
index 6cd0f6b765..0a0678a5e6 100644
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -297,10 +297,6 @@ struct mtd_info {
 	int (*_point) (struct mtd_info *mtd, loff_t from, size_t len,
 		       size_t *retlen, void **virt, resource_size_t *phys);
 	int (*_unpoint) (struct mtd_info *mtd, loff_t from, size_t len);
-	unsigned long (*_get_unmapped_area) (struct mtd_info *mtd,
-					     unsigned long len,
-					     unsigned long offset,
-					     unsigned long flags);
 	int (*_read) (struct mtd_info *mtd, loff_t from, size_t len,
 		      size_t *retlen, u_char *buf);
 	int (*_write) (struct mtd_info *mtd, loff_t to, size_t len,
-- 
2.9.5

Re: [PATCH 5/5] MTD: remove the get_unmapped_area method

[Richard Weinberger]

On Wed, Oct 11, 2017 at 5:26 AM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> It is now unused.
>
> Signed-off-by: Nicolas Pitre <nico@linaro.org>
> ---
>  drivers/mtd/chips/map_ram.c  | 18 ------------------
>  drivers/mtd/chips/map_rom.c  | 16 ----------------
>  drivers/mtd/devices/mtdram.c | 14 --------------
>  drivers/mtd/mtdconcat.c      | 27 ---------------------------
>  drivers/mtd/mtdpart.c        | 14 --------------
>  include/linux/mtd/mtd.h      |  4 ----
>  6 files changed, 93 deletions(-)

Yay, less code. :-)

Reviewed-by: Richard Weinberger <richard@nod.at>

-- 
Thanks,
//richard