[PATCH 1/2] nanddump: verify that start address is page aligned

[PATCH 1/2] nanddump: verify that start address is page aligned

[Baruch Siach]

Make nanddump consistent with nandwrite, which does not accept non page aligned
start addresses. Thus, non page aligned dumps are useless.

Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
 nanddump.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/nanddump.c b/nanddump.c
index 0fdf736..d6d1352 100644
--- a/nanddump.c
+++ b/nanddump.c
@@ -377,6 +377,12 @@ int main(int argc, char * const argv[])
 	}
 
 	/* Initialize start/end addresses and block size */
+	if (start_addr & (meminfo.writesize - 1)) {
+		fprintf(stderr, "The start address is not page-aligned !\n"
+				"The pagesize of this NAND Flash is 0x%x.\n",
+				meminfo.writesize);
+		goto closeall;
+	}
 	if (length)
 		end_addr = start_addr + length;
 	if (!length || end_addr > meminfo.size)
-- 
1.7.1

[PATCH 2/2] nanddump: always check the first erase block

[Baruch Siach]

Check the first block even when start_addr is not eraseblock aligned.

Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
 nanddump.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/nanddump.c b/nanddump.c
index d6d1352..9267a1c 100644
--- a/nanddump.c
+++ b/nanddump.c
@@ -283,7 +283,7 @@ int main(int argc, char * const argv[])
 	struct mtd_oob_buf oob = {0, 16, oobbuf};
 	mtd_info_t meminfo;
 	char pretty_buf[PRETTY_BUF_LEN];
-	int oobinfochanged = 0 ;
+	int oobinfochanged = 0, firstblock = 1;
 	struct nand_oobinfo old_oobinfo;
 	struct mtd_ecc_stats stat1, stat2;
 	bool eccstats = false;
@@ -405,8 +405,10 @@ int main(int argc, char * const argv[])
 		// new eraseblock , check for bad block
 		if (noskipbad) {
 			badblock = 0;
-		} else if (blockstart != (ofs & (~meminfo.erasesize + 1))) {
+		} else if (blockstart != (ofs & (~meminfo.erasesize + 1)) ||
+				firstblock) {
 			blockstart = ofs & (~meminfo.erasesize + 1);
+			firstblock = 0;
 			if ((badblock = ioctl(fd, MEMGETBADBLOCK, &blockstart)) < 0) {
 				perror("ioctl(MEMGETBADBLOCK)");
 				goto closeall;
-- 
1.7.1

Re: [PATCH 2/2] nanddump: always check the first erase block

[Artem Bityutskiy]

On Tue, 2010-10-12 at 10:23 +0200, Baruch Siach wrote:
> Check the first block even when start_addr is not eraseblock aligned.
> 
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> ---
>  nanddump.c |    6 ++++--
>  1 files changed, 4 insertions(+), 2 deletions(-)

Pushed to mtd-utils, thanks!

-- 
Best Regards,
Artem Bityutskiy (Битюцкий Артём)

Re: [PATCH 1/2] nanddump: verify that start address is page aligned

[Artem Bityutskiy]

On Tue, 2010-10-12 at 10:23 +0200, Baruch Siach wrote:
> Make nanddump consistent with nandwrite, which does not accept non page aligned
> start addresses. Thus, non page aligned dumps are useless.
> 
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> ---
>  nanddump.c |    6 ++++++
>  1 files changed, 6 insertions(+), 0 deletions(-)
> 
> diff --git a/nanddump.c b/nanddump.c
> index 0fdf736..d6d1352 100644
> --- a/nanddump.c
> +++ b/nanddump.c
> @@ -377,6 +377,12 @@ int main(int argc, char * const argv[])
>  	}
>  
>  	/* Initialize start/end addresses and block size */
> +	if (start_addr & (meminfo.writesize - 1)) {
> +		fprintf(stderr, "The start address is not page-aligned !\n"
> +				"The pagesize of this NAND Flash is 0x%x.\n",
> +				meminfo.writesize);
> +		goto closeall;
> +	}

I'm fine with this change in general, but theoretically we may have a
user who relies on this. Could you please instead print a warning but
preserve the old behavior. Then in after the next release we can make
remove the warning and turn it into an error, just like this patch do.
This will potentially be user-friendly.

Add a new record to the feature-removal-schedule.txt file. I'll then
turn the warning into an error at some point.

Thanks!

-- 
Best Regards,
Artem Bityutskiy (Битюцкий Артём)

[PATCH] nanddump: warn when the start address is not page aligned

[Baruch Siach]

nanddump should be consistent with nandwrite, which does not accept non page
aligned start addresses. Thus, non page aligned dumps are useless.

To ease migration only warn for now. Add the plan of making this an error to
feature-removal-schedule.txt.

Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
 feature-removal-schedule.txt |   12 +++++++++++-
 nanddump.c                   |    7 +++++++
 2 files changed, 18 insertions(+), 1 deletions(-)

diff --git a/feature-removal-schedule.txt b/feature-removal-schedule.txt
index 21bc8d4..f4ed2a3 100644
--- a/feature-removal-schedule.txt
+++ b/feature-removal-schedule.txt
@@ -27,5 +27,15 @@ So the plan is (to be done by Artem Bityutskiy):
 3.1 adding a warning to make users stop using them
 3.2 removing both options.
 ---------------------------
-2.
+2. nanddump: fail when the -s parameter is not page aligned
+
+nanddump should be consistent with nandwrite, and refuse accepting non page 
+aligned start addresses. These dumps are most likely useless, since nandwrite 
+won't write then at this location. Currently only a warning is issued to keep 
+backward compatibility during a transition period. This should become an error 
+after 1.5.0.
+
+To be done by Artem Bityutskiy.
+---------------------------
+3.
 ---------------------------
diff --git a/nanddump.c b/nanddump.c
index 15bf9c4..bb649da 100644
--- a/nanddump.c
+++ b/nanddump.c
@@ -360,6 +360,13 @@ int main(int argc, char * const argv[])
 	}
 
 	/* Initialize start/end addresses and block size */
+	if (start_addr & (meminfo.writesize - 1)) {
+		fprintf(stderr, "WARNING: The start address is not page-aligned !\n"
+				"The pagesize of this NAND Flash is 0x%x.\n"
+				"nandwrite doesn't allow writes starting at this location.\n"
+				"Future versions of nanddump will fail here.\n",
+				meminfo.writesize);
+	}
 	if (length)
 		end_addr = start_addr + length;
 	if (!length || end_addr > meminfo.size)
-- 
1.7.1

Re: [PATCH] nanddump: warn when the start address is not page aligned

[Artem Bityutskiy]

On Mon, 2010-10-18 at 09:23 +0200, Baruch Siach wrote:
> nanddump should be consistent with nandwrite, which does not accept non page
> aligned start addresses. Thus, non page aligned dumps are useless.
> 
> To ease migration only warn for now. Add the plan of making this an error to
> feature-removal-schedule.txt.
> 
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> ---
>  feature-removal-schedule.txt |   12 +++++++++++-
>  nanddump.c                   |    7 +++++++
>  2 files changed, 18 insertions(+), 1 deletions(-)
> 
> diff --git a/feature-removal-schedule.txt b/feature-removal-schedule.txt
> index 21bc8d4..f4ed2a3 100644
> --- a/feature-removal-schedule.txt
> +++ b/feature-removal-schedule.txt
> @@ -27,5 +27,15 @@ So the plan is (to be done by Artem Bityutskiy):
>  3.1 adding a warning to make users stop using them
>  3.2 removing both options.
>  ---------------------------
> -2.
> +2. nanddump: fail when the -s parameter is not page aligned
> +
> +nanddump should be consistent with nandwrite, and refuse accepting non page 
> +aligned start addresses. These dumps are most likely useless, since nandwrite 
> +won't write then at this location. Currently only a warning is issued to keep 
> +backward compatibility during a transition period. This should become an error 
> +after 1.5.0.

I tweaked this and removed 1.5.0 - it is better to not specify exact
version. Also, fixed the following trailing white-spaces issue for the
feature-removal-schedule.txt part:

Applying: nanddump: warn when the start address is not page aligned
/home/dedekind/git/mtd-utils/.git/rebase-apply/patch:17: trailing whitespace.
nanddump should be consistent with nandwrite, and refuse accepting non page 
/home/dedekind/git/mtd-utils/.git/rebase-apply/patch:18: trailing whitespace.
aligned start addresses. These dumps are most likely useless, since nandwrite 
/home/dedekind/git/mtd-utils/.git/rebase-apply/patch:19: trailing whitespace.
won't write then at this location. Currently only a warning is issued to keep 
/home/dedekind/git/mtd-utils/.git/rebase-apply/patch:20: trailing whitespace.
backward compatibility during a transition period. This should become an error 
warning: 4 lines add whitespace errors.

And pushed, thanks.

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)