[PATCH] mtd: core: Fix erase area alignment for non power of 2 erasesize

[PATCH] mtd: core: Fix erase area alignment for non power of 2 erasesize

[Ladislav Michl]

Devices as AT45DB161 DataFlash uses non power of two page size (528)
while present alignment algorithm relies on erasesize being power
of 2.
Fix that by introducing helper functions rounding to any multiply.
Note that logic is sligthly changed to be consistent as ending
address is moved forward to include also last byte meant to be
erased while previous implementation moved it backward.

Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
---
 drivers/mtd/core.c     | 28 ++++++++++++++++++++++------
 include/linux/kernel.h |  5 +++++
 2 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/drivers/mtd/core.c b/drivers/mtd/core.c
index d3cbe502f..f86b70176 100644
--- a/drivers/mtd/core.c
+++ b/drivers/mtd/core.c
@@ -141,15 +141,31 @@ static struct mtd_erase_region_info *mtd_find_erase_region(struct mtd_info *mtd,
 	return NULL;
 }
 
+static inline loff_t esize_up(loff_t x, uint32_t erasesize)
+{
+	uint32_t mod = DIV_ROUND_DOWN_ULL(x, erasesize);
+	if (mod == 0)
+		return x;
+	return x + erasesize - mod;
+}
+
+static inline loff_t esize_down(loff_t x, uint32_t erasesize)
+{
+	uint32_t mod = DIV_ROUND_DOWN_ULL(x, erasesize);
+	if (mod == 0)
+		return x;
+	return x - mod;
+}
+
 static int mtd_erase_align(struct mtd_info *mtd, loff_t *count, loff_t *offset)
 {
 	struct mtd_erase_region_info *e;
 	loff_t ofs;
 
 	if (mtd->numeraseregions == 0) {
-		ofs = *offset & ~(loff_t)(mtd->erasesize - 1);
-		*count += (*offset - ofs);
-		*count = ALIGN(*count, mtd->erasesize);
+		ofs = esize_down(*offset, mtd->erasesize);
+		*count += *offset - ofs;
+		*count = esize_up(*count, mtd->erasesize);
 		*offset = ofs;
 		return 0;
 	}
@@ -158,14 +174,14 @@ static int mtd_erase_align(struct mtd_info *mtd, loff_t *count, loff_t *offset)
 	if (!e)
 		return -EINVAL;
 
-	ofs = *offset & ~(e->erasesize - 1);
-	*count += (*offset - ofs);
+	ofs = esize_down(*offset, e->erasesize);
+	*count += *offset - ofs;
 
 	e = mtd_find_erase_region(mtd, *offset + *count);
 	if (!e)
 		return -EINVAL;
 
-	*count = ALIGN(*count, e->erasesize);
+	*count = esize_up(*count, e->erasesize);
 	*offset = ofs;
 
 	return 0;
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index cc6d6f746..ee0b48e08 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -55,6 +55,11 @@
 
 #define DIV_ROUND_UP(n,d) (((n) + (d) - 1) / (d))
 
+#define DIV_ROUND_DOWN_ULL(ll, d) \
+	({ unsigned long long _tmp = (ll); do_div(_tmp, d); _tmp; })
+
+#define DIV_ROUND_UP_ULL(ll, d)		DIV_ROUND_DOWN_ULL((ll) + (d) - 1, (d))
+
 #define DIV_ROUND_CLOSEST(x, divisor)(			\
 {							\
 	typeof(divisor) __divisor = divisor;		\
-- 
2.20.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] mtd: core: Fix erase area alignment for non power of 2 erasesize

[Sascha Hauer]

On Wed, Jan 09, 2019 at 12:28:14PM +0100, Ladislav Michl wrote:
> Devices as AT45DB161 DataFlash uses non power of two page size (528)
> while present alignment algorithm relies on erasesize being power
> of 2.
> Fix that by introducing helper functions rounding to any multiply.
> Note that logic is sligthly changed to be consistent as ending
> address is moved forward to include also last byte meant to be
> erased while previous implementation moved it backward.
> 
> Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
> ---
>  drivers/mtd/core.c     | 28 ++++++++++++++++++++++------
>  include/linux/kernel.h |  5 +++++
>  2 files changed, 27 insertions(+), 6 deletions(-)

Applied, thanks

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] mtd: core: Fix erase area alignment for non power of 2 erasesize

[Ladislav Michl]

On Thu, Jan 10, 2019 at 09:32:07AM +0100, Sascha Hauer wrote:
> On Wed, Jan 09, 2019 at 12:28:14PM +0100, Ladislav Michl wrote:
> > Devices as AT45DB161 DataFlash uses non power of two page size (528)
> > while present alignment algorithm relies on erasesize being power
> > of 2.
> > Fix that by introducing helper functions rounding to any multiply.
> > Note that logic is sligthly changed to be consistent as ending
> > address is moved forward to include also last byte meant to be
> > erased while previous implementation moved it backward.
> > 
> > Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
> > ---
> >  drivers/mtd/core.c     | 28 ++++++++++++++++++++++------
> >  include/linux/kernel.h |  5 +++++
> >  2 files changed, 27 insertions(+), 6 deletions(-)
> 
> Applied, thanks

Hi Sascha,

I was searching for paper bag, but was unable to find anything thick
enouh to cover behind :-/ This version is buggy and I noticed right
now when testing on another board. Corrected version follows, which
is also 192 bytes shorter for my ARM target. It seems next branch
at https://git.pengutronix.de/ is not yet updated, so perhaps my fault
won't affect anyone.

--- 8< ---
From: Ladislav Michl <ladis@linux-mips.org>
Subject: [PATCH v2] mtd: core: Fix erase area alignment for non power of 2 erasesize

Devices as AT45DB161 DataFlash uses non power of two page size (528)
while present alignment algorithm relies on erasesize being power
of 2.
Fix that by introducing helper functions rounding to any multiply.
Note that logic is sligthly changed to be consistent as ending
address is moved forward to include also last byte meant to be
erased while previous implementation moved it backward.

Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
---
 drivers/mtd/core.c | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/drivers/mtd/core.c b/drivers/mtd/core.c
index d3cbe502f..f44c6cfc6 100644
--- a/drivers/mtd/core.c
+++ b/drivers/mtd/core.c
@@ -141,15 +141,28 @@ static struct mtd_erase_region_info *mtd_find_erase_region(struct mtd_info *mtd,
 	return NULL;
 }
 
+static loff_t __mtd_erase_round(loff_t x, uint32_t esize, int up)
+{
+	uint64_t dividend = x;
+	uint32_t mod = do_div(dividend, esize);
+	if (mod == 0)
+		return x;
+	if (up)
+		x += esize;
+	return x - mod;
+}
+#define mtd_erase_round_up(x, esize)	__mtd_erase_round(x, esize, 1)
+#define mtd_erase_round_down(x, esize)	__mtd_erase_round(x, esize, 0)
+
 static int mtd_erase_align(struct mtd_info *mtd, loff_t *count, loff_t *offset)
 {
 	struct mtd_erase_region_info *e;
 	loff_t ofs;
 
 	if (mtd->numeraseregions == 0) {
-		ofs = *offset & ~(loff_t)(mtd->erasesize - 1);
-		*count += (*offset - ofs);
-		*count = ALIGN(*count, mtd->erasesize);
+		ofs = mtd_erase_round_down(*offset, mtd->erasesize);
+		*count += *offset - ofs;
+		*count = mtd_erase_round_up(*count, mtd->erasesize);
 		*offset = ofs;
 		return 0;
 	}
@@ -158,14 +171,14 @@ static int mtd_erase_align(struct mtd_info *mtd, loff_t *count, loff_t *offset)
 	if (!e)
 		return -EINVAL;
 
-	ofs = *offset & ~(e->erasesize - 1);
-	*count += (*offset - ofs);
+	ofs = mtd_erase_round_down(*offset, e->erasesize);
+	*count += *offset - ofs;
 
 	e = mtd_find_erase_region(mtd, *offset + *count);
 	if (!e)
 		return -EINVAL;
 
-	*count = ALIGN(*count, e->erasesize);
+	*count = mtd_erase_round_up(*count, e->erasesize);
 	*offset = ofs;
 
 	return 0;
-- 
2.20.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] mtd: core: Fix erase area alignment for non power of 2 erasesize

[Sascha Hauer]

On Sat, Jan 12, 2019 at 01:58:28PM +0100, Ladislav Michl wrote:
> On Thu, Jan 10, 2019 at 09:32:07AM +0100, Sascha Hauer wrote:
> > On Wed, Jan 09, 2019 at 12:28:14PM +0100, Ladislav Michl wrote:
> > > Devices as AT45DB161 DataFlash uses non power of two page size (528)
> > > while present alignment algorithm relies on erasesize being power
> > > of 2.
> > > Fix that by introducing helper functions rounding to any multiply.
> > > Note that logic is sligthly changed to be consistent as ending
> > > address is moved forward to include also last byte meant to be
> > > erased while previous implementation moved it backward.
> > > 
> > > Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
> > > ---
> > >  drivers/mtd/core.c     | 28 ++++++++++++++++++++++------
> > >  include/linux/kernel.h |  5 +++++
> > >  2 files changed, 27 insertions(+), 6 deletions(-)
> > 
> > Applied, thanks
> 
> Hi Sascha,
> 
> I was searching for paper bag, but was unable to find anything thick
> enouh to cover behind :-/ This version is buggy and I noticed right
> now when testing on another board. Corrected version follows, which
> is also 192 bytes shorter for my ARM target. It seems next branch
> at https://git.pengutronix.de/ is not yet updated, so perhaps my fault
> won't affect anyone.

I did in the meantime, but updated it now again with your updated patch.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH] mtd: core: Fix erase area alignment for non power of 2 erasesize

[Ladislav Michl]

Devices as AT45DB161 DataFlash uses non power of two page size (528)
while present alignment algorithm relies on erasesize being power
of 2.
Fix that by introducing helper functions do round to any multiply.
Note that logic is sligthly changed to be consistent as ending
address is moved forward to include also last byte meant to be
erased while previous implementation moved it backward.

Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
---
 Honestly, I do not like this way of "fixing" code. In ideal world
 this aliment should be done elsewhere, but this is just nice easy path
 of least resistance (evil).

 drivers/mtd/core.c | 31 +++++++++++++++++++++++++------
 1 file changed, 25 insertions(+), 6 deletions(-)

diff --git a/drivers/mtd/core.c b/drivers/mtd/core.c
index ae7818a18..36c0ac228 100644
--- a/drivers/mtd/core.c
+++ b/drivers/mtd/core.c
@@ -141,15 +141,34 @@ static struct mtd_erase_region_info *mtd_find_erase_region(struct mtd_info *mtd,
 	return NULL;
 }
 
+static inline loff_t __round_up(loff_t x, uint32_t multiple)
+{
+	uint64_t __x = x;
+	uint32_t mod = do_div(__x, multiple);
+	if (mod == 0)
+		return x;
+	return x + multiple - mod;
+}
+
+static inline loff_t __round_down(loff_t x, uint32_t multiple)
+{
+	uint64_t __x = x;
+	uint32_t mod = do_div(__x, multiple);
+	if (mod == 0)
+		return x;
+
+	return x - mod;
+}
+
 static int mtd_erase_align(struct mtd_info *mtd, loff_t *count, loff_t *offset)
 {
 	struct mtd_erase_region_info *e;
 	loff_t ofs;
 
 	if (mtd->numeraseregions == 0) {
-		ofs = *offset & ~(loff_t)(mtd->erasesize - 1);
-		*count += (*offset - ofs);
-		*count = ALIGN(*count, mtd->erasesize);
+		ofs = __round_down(*offset, mtd->erasesize);
+		*count += *offset - ofs;
+		*count = __round_up(*count, mtd->erasesize);
 		*offset = ofs;
 		return 0;
 	}
@@ -158,14 +177,14 @@ static int mtd_erase_align(struct mtd_info *mtd, loff_t *count, loff_t *offset)
 	if (!e)
 		return -EINVAL;
 
-	ofs = *offset & ~(e->erasesize - 1);
-	*count += (*offset - ofs);
+	ofs = __round_down(*offset, e->erasesize);
+	*count += *offset - ofs;
 
 	e = mtd_find_erase_region(mtd, *offset + *count);
 	if (!e)
 		return -EINVAL;
 
-	*count = ALIGN(*count, e->erasesize);
+	*count = __round_up(*count, e->erasesize);
 	*offset = ofs;
 
 	return 0;
-- 
2.19.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox