[PATCH] Add eraseblock functions to self-check

[Carl-Daniel Hailfinger]

Add eraseblock functions to self-check. It doesn't make sense to have
different layouts for the same function on one chip.
Keep going if an error is found, we want all errors to be reported in
one fell swoop.

Signed-off-by: Carl-Daniel Hailfinger

Index: flashrom-selfcheck_eraseblocks_functions/flashrom.c
===================================================================
--- flashrom-selfcheck_eraseblocks_functions/flashrom.c	(Revision 866)
+++ flashrom-selfcheck_eraseblocks_functions/flashrom.c	(Arbeitskopie)
@@ -833,10 +833,13 @@
    return 0;
 }

-/* This function shares a lot of its structure with erase_flash(). */
+/* This function shares a lot of its structure with erase_flash().
+ * Even if an error is found, the function will keep going and check the rest.
+ */
 int selfcheck_eraseblocks(struct flashchip *flash)
 {
-	int i, k;
+	int i, j, k;
+	int ret = 0;

for (k = 0; k < NUM_ERASEFUNCTIONS; k++) {
    	unsigned int done = 0;
@@ -850,8 +853,7 @@
    				"%i region %i has size 0. Please report"
    				" a bug at [e-mail]\n",
    				flash->name, k, i);
-				return 1;
-				break;
+				ret = 1;
    		}
    		/* Blocks with zero count are bugs in flashchips.c. */
    		if (!eraser.eraseblocks[i].count &&
@@ -860,13 +862,15 @@
    				"%i region %i has count 0. Please report"
    				" a bug at [e-mail]\n",
    				flash->name, k, i);
-				return 1;
-				break;
+				ret = 1;
    		}
    		done += eraser.eraseblocks[i].count *
    			eraser.eraseblocks[i].size;
    	}
-		/* This erase function is completely empty. */
+		/* Empty eraseblock definition with erase function.  */
+		if (!done && eraser.block_erase)
+			msg_pspew("Strange: Empty eraseblock definition with "
+				"non-empty erase function. Not an error.\n");
    	if (!done)
    		continue;
    	if (done != flash->total_size * 1024) {
@@ -875,10 +879,25 @@
    			" expected 0x%06x bytes. Please report a bug at"
    			" [e-mail]\n", flash->name, k,
    			done, flash->total_size * 1024);
-			return 1;
+			ret = 1;
    	}
+		if (!eraser.block_erase)
+			continue;
+		/* Check if there are identical erase functions for different
+		 * layouts. That would imply "magic" erase functions. The
+		 * easiest way to check this is with function pointers.
+		 */
+		for (j = k + 1; j < NUM_ERASEFUNCTIONS; j++)
+			if (eraser.block_erase ==
+			    flash->block_erasers[j].block_erase) {
+				msg_gerr("ERROR: Flash chip %s erase function "
+					"%i and %i are identical. Please report"
+					" a bug at [e-mail]\n",
+					flash->name, k, j);
+				ret = 1;
+			}
    }
-	return 0;
+	return ret;
 }

int erase_flash(struct flashchip *flash)

[Sean Nelson]

On 1/18/10 6:46 PM, Carl-Daniel Hailfinger wrote:
> Add eraseblock functions to self-check. It doesn't make sense to have
> different layouts for the same function on one chip.
> Keep going if an error is found, we want all errors to be reported in
> one fell swoop.
>
> Signed-off-by: Carl-Daniel Hailfinger
> Index: flashrom-selfcheck_eraseblocks_functions/flashrom.c
>
> --- flashrom-selfcheck_eraseblocks_functions/flashrom.c	(Revision 866)
> +++ flashrom-selfcheck_eraseblocks_functions/flashrom.c	(Arbeitskopie)
> @@ -833,10 +833,13 @@
>    return 0;
>   }
>
> -/* This function shares a lot of its structure with erase_flash(). */
> +/* This function shares a lot of its structure with erase_flash().
> Even if an error is found, the function will keep going and check the rest.
> */
> int selfcheck_eraseblocks(struct flashchip *flash)
> {
> int i, k;
>
> int i, j, k;
>
> int ret = 0;
>
> for (k = 0; k<  NUM_ERASEFUNCTIONS; k++) {
> unsigned int done = 0;
>
> @@ -850,8 +853,7 @@
>    				"%i region %i has size 0. Please report"
>    				" a bug at [e-mail]\n",
>    				flash->name, k, i);
> 		return 1;
> 		break;
> 		ret = 1;
> }
> /* Blocks with zero count are bugs in flashchips.c. */
> if (!eraser.eraseblocks[i].count&&
>
> @@ -860,13 +862,15 @@
>    				"%i region %i has count 0. Please report"
>    				" a bug at [e-mail]\n",
>    				flash->name, k, i);
> 		return 1;
> 		break;
> 		ret = 1;
> }
> done += eraser.eraseblocks[i].count *
> 	eraser.eraseblocks[i].size;
> }
> /* This erase function is completely empty. */
> /* Empty eraseblock definition with erase function.  */
> if (!done&&  eraser.block_erase)
> 	msg_pspew("Strange: Empty eraseblock definition with "
> 		"non-empty erase function. Not an error.\n");
> if (!done)
> 	continue;
> if (done != flash->total_size * 1024) {
>
> @@ -875,10 +879,25 @@
>    			" expected 0x%06x bytes. Please report a bug at"
>    			" [e-mail]\n", flash->name, k,
>    			done, flash->total_size * 1024);
> 	return 1;
> 	ret = 1;
> }
> if (!eraser.block_erase)
> 	continue;
> /* Check if there are identical erase functions for different
>  * layouts. That would imply "magic" erase functions. The
>  * easiest way to check this is with function pointers.
>  */
> for (j = k + 1; j<  NUM_ERASEFUNCTIONS; j++)
> 	if (eraser.block_erase ==
> 	    flash->block_erasers[j].block_erase) {
> 		msg_gerr("ERROR: Flash chip %s erase function "
> 			"%i and %i are identical. Please report"
> 			" a bug at [e-mail]\n",
> 			flash->name, k, j);
> 		ret = 1;
> 	}
> }
> return 0;
>
> return ret;
> }
>
> int erase_flash(struct flashchip *flash)

Acked-by: Sean Nelson

[Carl-Daniel Hailfinger]

On 19.01.2010 04:40, Sean Nelson wrote:
> On 1/18/10 6:46 PM, Carl-Daniel Hailfinger wrote:
> > Add eraseblock functions to self-check.
>
> Acked-by: Sean Nelson

Thanks, r869.

Regards,
Carl-Daniel