* [PATCH 0/5] unconfuse get_unmapped_area and point/unpoint driver methods
@ 2017-10-11 3:26 Nicolas Pitre
2017-10-11 3:26 ` [PATCH 1/5] MTD: mtdram: properly handle the phys argument in the point method Nicolas Pitre
` (4 more replies)
0 siblings, 5 replies; 15+ messages in thread
From: Nicolas Pitre @ 2017-10-11 3:26 UTC (permalink / raw)
To: Boris Brezillon; +Cc: linux-mtd
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(-)
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/5] MTD: mtdram: properly handle the phys argument in the point method
2017-10-11 3:26 [PATCH 0/5] unconfuse get_unmapped_area and point/unpoint driver methods Nicolas Pitre
@ 2017-10-11 3:26 ` Nicolas Pitre
2017-10-11 20:47 ` Richard Weinberger
2017-10-11 3:26 ` [PATCH 2/5] MTD: chips/map_ram.c: implement point and unpoint methods Nicolas Pitre
` (3 subsequent siblings)
4 siblings, 1 reply; 15+ messages in thread
From: Nicolas Pitre @ 2017-10-11 3:26 UTC (permalink / raw)
To: Boris Brezillon; +Cc: linux-mtd
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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 2/5] MTD: chips/map_ram.c: implement point and unpoint methods
2017-10-11 3:26 [PATCH 0/5] unconfuse get_unmapped_area and point/unpoint driver methods Nicolas Pitre
2017-10-11 3:26 ` [PATCH 1/5] MTD: mtdram: properly handle the phys argument in the point method Nicolas Pitre
@ 2017-10-11 3:26 ` Nicolas Pitre
2017-10-11 20:48 ` Richard Weinberger
2017-10-11 3:26 ` [PATCH 3/5] MTD: chips/map_rom.c: " Nicolas Pitre
` (2 subsequent siblings)
4 siblings, 1 reply; 15+ messages in thread
From: Nicolas Pitre @ 2017-10-11 3:26 UTC (permalink / raw)
To: Boris Brezillon; +Cc: linux-mtd
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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 3/5] MTD: chips/map_rom.c: implement point and unpoint methods
2017-10-11 3:26 [PATCH 0/5] unconfuse get_unmapped_area and point/unpoint driver methods Nicolas Pitre
2017-10-11 3:26 ` [PATCH 1/5] MTD: mtdram: properly handle the phys argument in the point method Nicolas Pitre
2017-10-11 3:26 ` [PATCH 2/5] MTD: chips/map_ram.c: implement point and unpoint methods Nicolas Pitre
@ 2017-10-11 3:26 ` Nicolas Pitre
2017-10-11 20:49 ` Richard Weinberger
2017-10-11 3:26 ` [PATCH 4/5] MTD: implement mtd_get_unmapped_area() using the point method Nicolas Pitre
2017-10-11 3:26 ` [PATCH 5/5] MTD: remove the get_unmapped_area method Nicolas Pitre
4 siblings, 1 reply; 15+ messages in thread
From: Nicolas Pitre @ 2017-10-11 3:26 UTC (permalink / raw)
To: Boris Brezillon; +Cc: linux-mtd
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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 4/5] MTD: implement mtd_get_unmapped_area() using the point method
2017-10-11 3:26 [PATCH 0/5] unconfuse get_unmapped_area and point/unpoint driver methods Nicolas Pitre
` (2 preceding siblings ...)
2017-10-11 3:26 ` [PATCH 3/5] MTD: chips/map_rom.c: " Nicolas Pitre
@ 2017-10-11 3:26 ` Nicolas Pitre
2017-10-11 20:51 ` Richard Weinberger
2017-10-11 3:26 ` [PATCH 5/5] MTD: remove the get_unmapped_area method Nicolas Pitre
4 siblings, 1 reply; 15+ messages in thread
From: Nicolas Pitre @ 2017-10-11 3:26 UTC (permalink / raw)
To: Boris Brezillon; +Cc: linux-mtd
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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 5/5] MTD: remove the get_unmapped_area method
2017-10-11 3:26 [PATCH 0/5] unconfuse get_unmapped_area and point/unpoint driver methods Nicolas Pitre
` (3 preceding siblings ...)
2017-10-11 3:26 ` [PATCH 4/5] MTD: implement mtd_get_unmapped_area() using the point method Nicolas Pitre
@ 2017-10-11 3:26 ` Nicolas Pitre
2017-10-11 21:02 ` Richard Weinberger
4 siblings, 1 reply; 15+ messages in thread
From: Nicolas Pitre @ 2017-10-11 3:26 UTC (permalink / raw)
To: Boris Brezillon; +Cc: linux-mtd
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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 1/5] MTD: mtdram: properly handle the phys argument in the point method
2017-10-11 3:26 ` [PATCH 1/5] MTD: mtdram: properly handle the phys argument in the point method Nicolas Pitre
@ 2017-10-11 20:47 ` Richard Weinberger
2017-10-11 21:32 ` Nicolas Pitre
0 siblings, 1 reply; 15+ messages in thread
From: Richard Weinberger @ 2017-10-11 20:47 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Boris Brezillon, linux-mtd
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/5] MTD: chips/map_ram.c: implement point and unpoint methods
2017-10-11 3:26 ` [PATCH 2/5] MTD: chips/map_ram.c: implement point and unpoint methods Nicolas Pitre
@ 2017-10-11 20:48 ` Richard Weinberger
0 siblings, 0 replies; 15+ messages in thread
From: Richard Weinberger @ 2017-10-11 20:48 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Boris Brezillon, linux-mtd
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/5] MTD: chips/map_rom.c: implement point and unpoint methods
2017-10-11 3:26 ` [PATCH 3/5] MTD: chips/map_rom.c: " Nicolas Pitre
@ 2017-10-11 20:49 ` Richard Weinberger
2017-10-11 20:56 ` Nicolas Pitre
0 siblings, 1 reply; 15+ messages in thread
From: Richard Weinberger @ 2017-10-11 20:49 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Boris Brezillon, linux-mtd
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 4/5] MTD: implement mtd_get_unmapped_area() using the point method
2017-10-11 3:26 ` [PATCH 4/5] MTD: implement mtd_get_unmapped_area() using the point method Nicolas Pitre
@ 2017-10-11 20:51 ` Richard Weinberger
0 siblings, 0 replies; 15+ messages in thread
From: Richard Weinberger @ 2017-10-11 20:51 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Boris Brezillon, linux-mtd
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/5] MTD: chips/map_rom.c: implement point and unpoint methods
2017-10-11 20:49 ` Richard Weinberger
@ 2017-10-11 20:56 ` Nicolas Pitre
0 siblings, 0 replies; 15+ messages in thread
From: Nicolas Pitre @ 2017-10-11 20:56 UTC (permalink / raw)
To: Richard Weinberger; +Cc: Boris Brezillon, linux-mtd
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 5/5] MTD: remove the get_unmapped_area method
2017-10-11 3:26 ` [PATCH 5/5] MTD: remove the get_unmapped_area method Nicolas Pitre
@ 2017-10-11 21:02 ` Richard Weinberger
0 siblings, 0 replies; 15+ messages in thread
From: Richard Weinberger @ 2017-10-11 21:02 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Boris Brezillon, linux-mtd
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/5] MTD: mtdram: properly handle the phys argument in the point method
2017-10-11 20:47 ` Richard Weinberger
@ 2017-10-11 21:32 ` Nicolas Pitre
2017-10-11 21:40 ` Richard Weinberger
0 siblings, 1 reply; 15+ messages in thread
From: Nicolas Pitre @ 2017-10-11 21:32 UTC (permalink / raw)
To: Richard Weinberger; +Cc: Boris Brezillon, linux-mtd
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/5] MTD: mtdram: properly handle the phys argument in the point method
2017-10-11 21:32 ` Nicolas Pitre
@ 2017-10-11 21:40 ` Richard Weinberger
2017-10-11 21:53 ` Nicolas Pitre
0 siblings, 1 reply; 15+ messages in thread
From: Richard Weinberger @ 2017-10-11 21:40 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Boris Brezillon, linux-mtd
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/5] MTD: mtdram: properly handle the phys argument in the point method
2017-10-11 21:40 ` Richard Weinberger
@ 2017-10-11 21:53 ` Nicolas Pitre
0 siblings, 0 replies; 15+ messages in thread
From: Nicolas Pitre @ 2017-10-11 21:53 UTC (permalink / raw)
To: Richard Weinberger; +Cc: Boris Brezillon, linux-mtd
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
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2017-10-11 21:53 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-11 3:26 [PATCH 0/5] unconfuse get_unmapped_area and point/unpoint driver methods Nicolas Pitre
2017-10-11 3:26 ` [PATCH 1/5] MTD: mtdram: properly handle the phys argument in the point method Nicolas Pitre
2017-10-11 20:47 ` Richard Weinberger
2017-10-11 21:32 ` Nicolas Pitre
2017-10-11 21:40 ` Richard Weinberger
2017-10-11 21:53 ` Nicolas Pitre
2017-10-11 3:26 ` [PATCH 2/5] MTD: chips/map_ram.c: implement point and unpoint methods Nicolas Pitre
2017-10-11 20:48 ` Richard Weinberger
2017-10-11 3:26 ` [PATCH 3/5] MTD: chips/map_rom.c: " Nicolas Pitre
2017-10-11 20:49 ` Richard Weinberger
2017-10-11 20:56 ` Nicolas Pitre
2017-10-11 3:26 ` [PATCH 4/5] MTD: implement mtd_get_unmapped_area() using the point method Nicolas Pitre
2017-10-11 20:51 ` Richard Weinberger
2017-10-11 3:26 ` [PATCH 5/5] MTD: remove the get_unmapped_area method Nicolas Pitre
2017-10-11 21:02 ` Richard Weinberger
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.