public inbox for linux-mtd@lists.infradead.org
 help / color / mirror / Atom feed
* [RFC/PATCH] Ubi-Utils: add compare feature
@ 2007-07-09 19:32 Alexander Schmidt
  2007-07-10  6:52 ` Frank Haverkamp
  2007-07-10  7:09 ` Artem Bityutskiy
  0 siblings, 2 replies; 7+ messages in thread
From: Alexander Schmidt @ 2007-07-09 19:32 UTC (permalink / raw)
  To: linux-mtd@lists.infradead.org; +Cc: Andreas Arnez, Frank Haverkamp

Hi,

this is a new feature for pfiflash, called "--compare". It allows the user
to simulate a pfiflash session without actually changing the flash
content. If the flash content is equal to the data in the pfif file,
pfiflash returns zero. A positive value is returned when the flash
content differs from the pfi file, which indicates that an update is
necessary. This feature is useful when a controller mounts an NFS share
during boot and has to determine if a pfi file stored on this share
contains a code update. Modified PDD values are also registered by the
compare feature.

The patch is already usable but does not contain a check for modified
volume type, size or alignment yet. These values need to be read from
sysfs and compared to the values in the pfi file. I'm sending this out
to get comments/reviews on what I've done yet.

So comments are appreciated!

Signed-off-by: Alexander Schmidt <alexs@linux.vnet.ibm.com>
---
 ubi-utils/src/bootenv.c        |   48 ++++++++
 ubi-utils/src/bootenv.h        |   10 +
 ubi-utils/src/libpfiflash.c    |  234 ++++++++++++++++++++++++++++++++++++-----
 ubi-utils/src/pfiflash.c       |   24 ++--
 ubi-utils/src/pfiflash.h       |    3 
 ubi-utils/src/pfiflash_error.h |    8 +
 6 files changed, 290 insertions(+), 37 deletions(-)

--- mtd-utils.orig/ubi-utils/src/pfiflash.c
+++ mtd-utils/ubi-utils/src/pfiflash.c
@@ -63,6 +63,9 @@ static const char *optionsstr =
 "                             'keep', 'merge' or 'overwrite'.\n"
 "  -r, --raw-flash=<dev>      Flash the raw data. Use the specified mtd device.\n"
 "  -s, --side=<seqnum>        Select the side which shall be updated.\n"
+"  -x, --compare              Only compare on-flash and pfi data, print info if\n"
+"                             an update is neccessary and return appropriate\n"
+"                             error code.\n"
 "\n"
 "  -?, --help                 Give this help list\n"
 "      --usage                Give a short usage message\n"
@@ -72,7 +75,7 @@ static const char *usage =
 "Usage: pfiflash [-cvC?V] [-l <file>] [-p <type>] [-r <dev>] [-s <seqnum>]\n"
 "            [--copyright] [--logfile=<file>] [--verbose] [--complete]\n"
 "            [--pdd-update=<type>] [--raw-flash=<dev>] [--side=<seqnum>]\n"
-"            [--help] [--usage] [--version] [pfifile]\n";
+"            [--compare] [--help] [--usage] [--version] [pfifile]\n";
 
 static const char copyright [] __attribute__((unused)) =
 	"Copyright IBM Corp 2006";
@@ -85,6 +88,7 @@ struct option long_options[] = {
 	{ .name = "pdd-update", .has_arg = 1, .flag = NULL, .val = 'p' },
 	{ .name = "raw-flash", .has_arg = 1, .flag = NULL, .val = 'r' },
 	{ .name = "side", .has_arg = 1, .flag = NULL, .val = 's' },
+	{ .name = "compare", .has_arg = 0, .flag = NULL, .val = 'x' },
 	{ .name = "help", .has_arg = 0, .flag = NULL, .val = '?' },
 	{ .name = "usage", .has_arg = 0, .flag = NULL, .val = 0 },
 	{ .name = "version", .has_arg = 0, .flag = NULL, .val = 'V' },
@@ -98,6 +102,7 @@ typedef struct myargs {
 
 	pdd_handling_t pdd_handling;
 	int seqnum;
+	int compare;
 	int complete;
 
 	FILE* fp_in;
@@ -180,6 +185,9 @@ parse_opt(int argc, char **argv, myargs 
 						 "and '1'\n", optarg);
 				}
 				break;
+			case 'x':
+				args->compare = 1;
+				break;
 			case 'r':
 				args->raw_dev = optarg;
 				break;
@@ -222,6 +230,7 @@ int main (int argc, char** argv)
 	myargs args = {
 		.verbose    = 0,
 		.seqnum	    = -1,
+		.compare    = 0,
 		.complete   = 0,
 		.logfile    = NULL, /* "/tmp/pfiflash.log", */
 		.pdd_handling = PDD_KEEP,
@@ -239,16 +248,9 @@ int main (int argc, char** argv)
 		goto err;
 	}
 
-	if (!args.raw_dev) {
-		rc = pfiflash(args.fp_in, args.complete, args.seqnum,
-			      args.pdd_handling, err_buf,
-			      PFIFLASH_MAX_ERR_BUF_SIZE);
-	} else {
-		rc = pfiflash_with_raw(args.fp_in, args.complete, args.seqnum,
-			      args.pdd_handling, args.raw_dev, err_buf,
-			      PFIFLASH_MAX_ERR_BUF_SIZE);
-	}
-
+	rc = pfiflash_with_options(args.fp_in, args.complete, args.seqnum,
+			args.compare, args.pdd_handling, args.raw_dev, err_buf,
+			PFIFLASH_MAX_ERR_BUF_SIZE);
 	if (rc != 0) {
 		goto err_fp;
 	}
--- mtd-utils.orig/ubi-utils/src/libpfiflash.c
+++ mtd-utils/ubi-utils/src/libpfiflash.c
@@ -55,6 +55,8 @@
 
 #define __unused __attribute__((unused))
 
+#define COMPARE_BUFFER_SIZE 1024
+
 static const char copyright [] __unused =
 	"Copyright (c) International Business Machines Corp., 2006";
 
@@ -82,6 +84,7 @@ static pdd_func_t pdd_funcs[PDD_HANDLING
 typedef enum ubi_update_process_t {
 	UBI_REMOVE = 0,
 	UBI_WRITE,
+	UBI_COMPARE,
 
+#define COMPARE_BUFFER_SIZE 1024
+
 static const char copyright [] __unused =
 	"Copyright (c) International Business Machines Corp., 2006";
 
@@ -82,6 +84,7 @@ static pdd_func_t pdd_funcs[PDD_HANDLING
 typedef enum ubi_update_process_t {
 	UBI_REMOVE = 0,
 	UBI_WRITE,
+	UBI_COMPARE,
 } ubi_update_process_t;
 
 
@@ -556,6 +559,170 @@ write_normal_volume(int devno, uint32_t 
 	return rc;
 }
 
+static int compare_bootenv(FILE *fp_pfi, FILE **fp_flash, uint32_t ids_size,
+		uint32_t data_size, pdd_func_t pdd_f, char *err_buf,
+		size_t err_buf_size)
+{
+	int rc, warnings = 0;
+	unsigned int i;
+	bootenv_t bootenv_pfi, bootenv_res = NULL, bootenv_flash = NULL;
+
+	rc = bootenv_create(&bootenv_pfi);
+	if (rc != 0) {
+		rc = -PFIFLASH_ERR_BOOTENV_CREATE;
+		goto err;
+	}
+
+	rc = bootenv_create(&bootenv_res);
+	if (rc != 0) {
+		rc = -PFIFLASH_ERR_BOOTENV_CREATE;
+		goto err;
+	}
+
+	rc = bootenv_read(fp_pfi, bootenv_pfi, data_size);
+	if (rc != 0) {
+		rc = -PFIFLASH_ERR_BOOTENV_READ;
+		goto err;
+	}
+
+	for (i = 0; i < ids_size; i++) {
+		rc = bootenv_create(&bootenv_flash);
+		if (rc != 0) {
+			rc = -PFIFLASH_ERR_BOOTENV_CREATE;
+			goto err;
+		}
+
+		rc = bootenv_read(fp_flash[i], bootenv_flash, BOOTENV_MAXSIZE);
+		if (rc != 0) {
+			rc = -PFIFLASH_ERR_BOOTENV_READ;
+			goto err;
+		}
+
+		rc = pdd_f(bootenv_flash, bootenv_pfi, &bootenv_res,
+				&warnings, err_buf, err_buf_size);
+		if (rc != 0) {
+			rc = -PFIFLASH_ERR_PDD_UNKNOWN;
+			goto err;
+		}
+
+		rc = bootenv_compare(bootenv_flash, bootenv_res);
+		if (rc > 0) {
+			rc = -PFIFLASH_CMP_DIFF;
+			goto err;
+		} else if (rc < 0) {
+			rc = -PFIFLASH_ERR_COMPARE;
+			goto err;
+		}
+
+		bootenv_destroy(&bootenv_flash);
+		bootenv_flash = NULL;
+	}
+
+err:
+	if (bootenv_pfi)
+		bootenv_destroy(&bootenv_pfi);
+	if (bootenv_res)
+		bootenv_destroy(&bootenv_res);
+	if (bootenv_flash)
+		bootenv_destroy(&bootenv_flash);
+
+	return rc;
+}
+
+static int compare_data(FILE *fp_pfi, FILE **fp_flash, uint32_t ids_size,
+		uint32_t bytes_left)
+{
+	unsigned int i;
+	size_t read_bytes, rc = 0;
+	char buf_pfi[COMPARE_BUFFER_SIZE];
+	char *buf_flash[ids_size];
+
+	for (i = 0; i < ids_size; i++) {
+		buf_flash[i] = malloc(COMPARE_BUFFER_SIZE);
+		if (!buf_flash[i])
+			return -PFIFLASH_ERR_COMPARE;
+	}
+
+	while (bytes_left) {
+		if (bytes_left > COMPARE_BUFFER_SIZE)
+			read_bytes = COMPARE_BUFFER_SIZE;
+		else
+			read_bytes = bytes_left;
+
+		rc = fread(buf_pfi, 1, read_bytes, fp_pfi);
+		if (rc != read_bytes) {
+			rc = -PFIFLASH_ERR_COMPARE;
+			goto err;
+		}
+
+		for (i = 0; i < ids_size; i++) {
+			rc = fread(buf_flash[i], 1, read_bytes, fp_flash[i]);
+			if (rc != read_bytes) {
+				rc = -PFIFLASH_CMP_DIFF;
+				goto err;
+			}
+
+			rc = memcmp(buf_pfi, buf_flash[i], read_bytes);
+			if (rc != 0) {
+				rc = -PFIFLASH_CMP_DIFF;
+				goto err;
+			}
+		}
+
+		bytes_left -= read_bytes;
+	}
+
+err:
+	for (i = 0; i < ids_size; i++)
+		free(buf_flash[i]);
+
+	return rc;
+}
+
+static int compare_volumes(int devno, pfi_ubi_t u, FILE *fp_pfi,
+		pdd_func_t pdd_f, char *err_buf, size_t err_buf_size)
+{
+	int rc, is_bootenv = 0;
+	unsigned int i;
+	ubi_lib_t ulib = NULL;
+	FILE *fp_flash[u->ids_size];
+
+	rc = ubi_open(&ulib);
+	if (rc != 0) {
+		rc = -PFIFLASH_ERR_UBI_OPEN;
+		goto err;
+	}
+
+	for (i = 0; i < u->ids_size; i++) {
+		if (u->ids[i] == EXAMPLE_BOOTENV_VOL_ID_1 ||
+		    u->ids[i] == EXAMPLE_BOOTENV_VOL_ID_2)
+			is_bootenv = 1;
+
+		fp_flash[i] = ubi_vol_fopen_read(ulib, devno, u->ids[i]);
+		if (fp_flash[i] == NULL) {
+			rc = -PFIFLASH_ERR_UBI_OPEN;
+			goto err;
+		}
+	}
+
+	if (is_bootenv)
+		rc = compare_bootenv(fp_pfi, fp_flash, u->ids_size,
+				u->data_size, pdd_f, err_buf, err_buf_size);
+	else
+		rc = compare_data(fp_pfi, fp_flash, u->ids_size, u->data_size);
+
+err:
+	if (rc < 0)
+		EBUF(PFIFLASH_ERRSTR[-rc]);
+
+	for (i = 0; i < u->ids_size; i++)
+		fclose(fp_flash[i]);
+	if (ulib)
+		ubi_close(&ulib);
+
+	return rc;
+}
+
 static int
 erase_mtd_region(FILE* file_p, int start, int length)
 {
@@ -865,6 +1032,15 @@ process_ubi_volumes(FILE* pfi, int seqnu
 				goto err;
 
 			break;
+		case UBI_COMPARE:
+			rc = compare_volumes(EXAMPLE_UBI_DEVICE, u, pfi, pdd_f,
+					err_buf, err_buf_size);
+			if (rc != 0) {
+				EBUF_PREPEND("compare volume");
+				goto err;
+			}
+
+			break;
 		default:
 			rc = -PFIFLASH_ERR_UBI_UNKNOWN;
 			EBUF(PFIFLASH_ERRSTR[-rc]);
@@ -949,10 +1125,11 @@ mirror_ubi_volumes(uint32_t devno, list_
 
 
 /**
- * pfiflash_with_raw - exposed func to flash memory with a PFI file
+ * pfiflash_with_options - exposed func to flash memory with a PFI file
  * @pfi			PFI data file pointer
  * @complete		flag to erase unmapped volumes
  * @seqnum		sequence number
+ * @compare		flag to compare
  * @pdd_handling	method to handle pdd (keep, merge, overwrite...)
  *
  * Error handling:
@@ -967,7 +1144,7 @@ mirror_ubi_volumes(uint32_t devno, list_
  *	- passes rc, prepends err_buf with contextual aid
  **/
 int
-pfiflash_with_raw(FILE* pfi, int complete, int seqnum,
+pfiflash_with_options(FILE* pfi, int complete, int seqnum, int compare,
 		  pdd_handling_t pdd_handling, const char* rawdev,
 		  char *err_buf, size_t err_buf_size)
 {
@@ -1001,7 +1178,7 @@ pfiflash_with_raw(FILE* pfi, int complet
 		goto err;
 	}
 
-	if (rawdev == NULL)
+	if (rawdev == NULL || compare)
 		rc = skip_raw_volumes(pfi, pfi_raws, err_buf, err_buf_size);
 	else
 		rc = process_raw_volumes(pfi, pfi_raws, rawdev, err_buf,
@@ -1011,7 +1188,7 @@ pfiflash_with_raw(FILE* pfi, int complet
 		goto err;
 	}
 
-	if (complete) {
+	if (complete && !compare) {
 		rc = erase_unmapped_ubi_volumes(EXAMPLE_UBI_DEVICE, pfi_ubis,
 						err_buf, err_buf_size);
 		if (rc != 0) {
@@ -1029,25 +1206,40 @@ pfiflash_with_raw(FILE* pfi, int complet
 		goto err;
 	}
 
-	rc = process_ubi_volumes(pfi, curr_seqnum, pfi_ubis, bootenv, pdd_f,
-				 UBI_REMOVE, err_buf, err_buf_size);
-	if (rc != 0) {
-		EBUF_PREPEND("removing UBI volumes");
-		goto err;
-	}
+	if (!compare) {
+		rc = process_ubi_volumes(pfi, curr_seqnum, pfi_ubis, bootenv,
+				pdd_f, UBI_REMOVE, err_buf, err_buf_size);
+		if (rc != 0) {
+			EBUF_PREPEND("removing UBI volumes");
+			goto err;
+		}
 
-	rc = process_ubi_volumes(pfi, curr_seqnum, pfi_ubis, bootenv, pdd_f,
-				 UBI_WRITE, err_buf, err_buf_size);
-	if  (rc != 0) {
-		EBUF_PREPEND("writing UBI volumes");
-		goto err;
-	}
+		rc = process_ubi_volumes(pfi, curr_seqnum, pfi_ubis, bootenv,
+				pdd_f, UBI_WRITE, err_buf, err_buf_size);
+		if  (rc != 0) {
+			EBUF_PREPEND("writing UBI volumes");
+			goto err;
+		}
 
-	if (seqnum < 0) { /* mirror redundant pairs */
-		rc = mirror_ubi_volumes(EXAMPLE_UBI_DEVICE, pfi_ubis,
+		if (seqnum < 0) { /* mirror redundant pairs */
+			rc = mirror_ubi_volumes(EXAMPLE_UBI_DEVICE, pfi_ubis,
 					err_buf, err_buf_size);
-		if (rc != 0) {
-			EBUF_PREPEND("mirroring UBI volumes");
+			if (rc != 0) {
+				EBUF_PREPEND("mirroring UBI volumes");
+				goto err;
+			}
+		}
+	} else {
+		/* only compare volumes, don't alter the content */
+		rc = process_ubi_volumes(pfi, curr_seqnum, pfi_ubis, bootenv,
+				pdd_f, UBI_COMPARE, err_buf, err_buf_size);
+
+		if (rc == -PFIFLASH_CMP_DIFF)
+			/* update is necessary, return positive value */
+			rc = 1;
+
+		if (rc < 0) {
+			EBUF_PREPEND("comparing UBI volumes");
 			goto err;
 		}
 	}
@@ -1061,7 +1253,7 @@ pfiflash_with_raw(FILE* pfi, int complet
 
 
 /**
- * pfiflash - passes to pfiflash_with_raw
+ * pfiflash - passes to pfiflash_with_options
  * @pfi			PFI data file pointer
  * @complete		flag to erase unmapped volumes
  * @seqnum		sequence number
@@ -1069,8 +1261,8 @@ pfiflash_with_raw(FILE* pfi, int complet
  **/
 int
 pfiflash(FILE* pfi, int complete, int seqnum, pdd_handling_t pdd_handling,
-	 char *err_buf, size_t err_buf_size)
+		char *err_buf, size_t err_buf_size)
 {
-	return pfiflash_with_raw(pfi, complete, seqnum, pdd_handling,
+	return pfiflash_with_options(pfi, complete, seqnum, 0, pdd_handling,
 				 NULL, err_buf, err_buf_size);
 }
--- mtd-utils.orig/ubi-utils/src/pfiflash.h
+++ mtd-utils/ubi-utils/src/pfiflash.h
@@ -48,12 +48,13 @@ typedef enum pdd_handling_t
  * @brief Flashes a PFI file to UBI Device 0.
  * @param complete	[0|1] Do a complete system update.
  * @param seqnum	Index in a redundant group.
+ * @param compare	[0|1] Compare contents.
  * @param pdd_handling	The PDD handling algorithm.
  * @param rawdev	Device to use for raw flashing
  * @param err_buf	An error buffer.
  * @param err_buf_size	Size of the error buffer.
  */
-int pfiflash_with_raw(FILE* pfi, int complete, int seqnum,
+int pfiflash_with_options(FILE* pfi, int complete, int seqnum, int compare,
 		pdd_handling_t pdd_handling, const char* rawdev,
 		char *err_buf, size_t err_buf_size);
 
--- mtd-utils.orig/ubi-utils/src/pfiflash_error.h
+++ mtd-utils/ubi-utils/src/pfiflash_error.h
@@ -42,7 +42,9 @@ enum pfiflash_err {
 	PFIFLASH_ERR_MTD_OPEN,
 	PFIFLASH_ERR_MTD_CLOSE,
 	PFIFLASH_ERR_CRC_CHECK,
-	PFIFLASH_ERR_MTD_ERASE
+	PFIFLASH_ERR_MTD_ERASE,
+	PFIFLASH_ERR_COMPARE,
+	PFIFLASH_CMP_DIFF
 };
 
 const char *const PFIFLASH_ERRSTR[] = {
@@ -65,7 +67,9 @@ const char *const PFIFLASH_ERRSTR[] = {
 	"couldn't open MTD device %s",
 	"couldn't close MTD device %s",
 	"CRC check failed: given=0x%08x, calculated=0x%08x",
-	"couldn't erase raw mtd region"
+	"couldn't erase raw mtd region",
+	"couldn't compare volumes",
+	"on-flash data differ from pfi data, update is neccessary"
 };
 
 #endif /* __PFIFLASH_ERROR_H__ */
--- mtd-utils.orig/ubi-utils/src/bootenv.c
+++ mtd-utils/ubi-utils/src/bootenv.c
@@ -480,6 +480,54 @@ bootenv_write(FILE* fp, bootenv_t env)
 }
 
 int
+bootenv_compare(bootenv_t first, bootenv_t second)
+{
+	int rc;
+	size_t written_first, written_second;
+	char *buf_first, *buf_second;
+
+	if (first == NULL || second == NULL)
+		return -EINVAL;
+
+	buf_first = malloc(BOOTENV_MAXSIZE);
+	if (!buf_first)
+		return -ENOMEM;
+	buf_second = malloc(BOOTENV_MAXSIZE);
+	if (!buf_second) {
+		rc = -ENOMEM;
+		goto err;
+	}
+
+	rc = fill_output_buffer(first, buf_first, BOOTENV_MAXSIZE,
+			&written_first);
+	if (rc < 0)
+		goto err;
+	rc = fill_output_buffer(second, buf_second, BOOTENV_MAXSIZE,
+			&written_second);
+	if (rc < 0)
+		goto err;
+
+	if (written_first != written_second) {
+		rc = 1;
+		goto err;
+	}
+
+	rc = memcmp(buf_first, buf_second, written_first);
+	if (rc != 0) {
+		rc = 2;
+		goto err;
+	}
+
+err:
+	if (buf_first)
+		free(buf_first);
+	if (buf_second)
+		free(buf_second);
+
+	return rc;
+ * @return 0 if bootenvs are equal
+ * @return < 0 if error
+ * @return > 0 if unequal
+ */
+int bootenv_compare(bootenv_t first, bootenv_t second);
+
+/**
  * @brief Prototype for a PDD handling funtion
  */
 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC/PATCH] Ubi-Utils: add compare feature
  2007-07-09 19:32 [RFC/PATCH] Ubi-Utils: add compare feature Alexander Schmidt
@ 2007-07-10  6:52 ` Frank Haverkamp
  2007-07-10 10:14   ` Alexander Schmidt
  2007-07-10 12:52   ` Josh Boyer
  2007-07-10  7:09 ` Artem Bityutskiy
  1 sibling, 2 replies; 7+ messages in thread
From: Frank Haverkamp @ 2007-07-10  6:52 UTC (permalink / raw)
  To: Alexander Schmidt
  Cc: Andreas Arnez, linux-mtd@lists.infradead.org, haver,
	Frank Haverkamp

[-- Attachment #1: Type: text/plain, Size: 582 bytes --]

Hi Alex,

On Mon, 2007-07-09 at 21:32 +0200, Alexander Schmidt wrote:
> +#define COMPARE_BUFFER_SIZE 1024

I think using the something like 2KiB or even the 128KiB would be nicer.

> +
>  static const char copyright [] __unused =
>         "Copyright (c) International Business Machines Corp., 2006";
2007
> 

Frank

-- 
IBM Deutschland Entwicklung GmbH, Schoenaicher Str. 220, 71032
Boeblingen, Vorsitzender des Aufsichtsrats: Martin Jetter
Geschäftsführung: Herbert Kircher, Sitz der Gesellschaft: Böblingen,
Registergericht: Amtsgericht Stuttgart, HRB 243294

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5269 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC/PATCH] Ubi-Utils: add compare feature
  2007-07-09 19:32 [RFC/PATCH] Ubi-Utils: add compare feature Alexander Schmidt
  2007-07-10  6:52 ` Frank Haverkamp
@ 2007-07-10  7:09 ` Artem Bityutskiy
  2007-07-10 10:20   ` Alexander Schmidt
  1 sibling, 1 reply; 7+ messages in thread
From: Artem Bityutskiy @ 2007-07-10  7:09 UTC (permalink / raw)
  To: Alexander Schmidt
  Cc: linux-mtd@lists.infradead.org, Andreas Arnez, Frank Haverkamp

Hi Alex,

On Mon, 2007-07-09 at 21:32 +0200, Alexander Schmidt wrote:
> this is a new feature for pfiflash, called "--compare". It allows the user
> to simulate a pfiflash session without actually changing the flash
> content. If the flash content is equal to the data in the pfif file,
> pfiflash returns zero. A positive value is returned when the flash
> content differs from the pfi file, which indicates that an update is
> necessary. This feature is useful when a controller mounts an NFS share
> during boot and has to determine if a pfi file stored on this share
> contains a code update. Modified PDD values are also registered by the
> compare feature.
> 
> The patch is already usable but does not contain a check for modified
> volume type, size or alignment yet. These values need to be read from
> sysfs and compared to the values in the pfi file. I'm sending this out
> to get comments/reviews on what I've done yet.

since I do not use the pfi stuff and it is your team's child, and I
really do not have time now to give it comments or reviews, I'll just
blindly push your patch as soon as you send the final version.

Thank you.

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC/PATCH] Ubi-Utils: add compare feature
  2007-07-10  6:52 ` Frank Haverkamp
@ 2007-07-10 10:14   ` Alexander Schmidt
  2007-07-10 12:52   ` Josh Boyer
  1 sibling, 0 replies; 7+ messages in thread
From: Alexander Schmidt @ 2007-07-10 10:14 UTC (permalink / raw)
  To: linux-mtd, haver; +Cc: Andreas Arnez

Hi Frank,

> > +#define COMPARE_BUFFER_SIZE 1024
> 
> I think using the something like 2KiB or even the 128KiB would be nicer.

I've played around with several buffer sizes (1KiB, 2KiB, 4KiB and
128KiB), and to my surprise, there was no performance impact.
I've used a 11MiB pfi file and the comparison always takes 5.5 seconds.
But I will change the value to 2KiB, it at least looks nicer.
 
> > +
> >  static const char copyright [] __unused =
> >         "Copyright (c) International Business Machines Corp., 2006";
> 2007

Done.

Thanks for taking a look.
Alex 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC/PATCH] Ubi-Utils: add compare feature
  2007-07-10  7:09 ` Artem Bityutskiy
@ 2007-07-10 10:20   ` Alexander Schmidt
  0 siblings, 0 replies; 7+ messages in thread
From: Alexander Schmidt @ 2007-07-10 10:20 UTC (permalink / raw)
  To: linux-mtd, dedekind; +Cc: Frank Haverkamp, Andreas Arnez

Hi Artem,

> since I do not use the pfi stuff and it is your team's child, and I
> really do not have time now to give it comments or reviews, I'll just
> blindly push your patch as soon as you send the final version.
> 
> Thank you.

Okay, then I will send you the final version soon. Thanks!

Alex

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC/PATCH] Ubi-Utils: add compare feature
  2007-07-10  6:52 ` Frank Haverkamp
  2007-07-10 10:14   ` Alexander Schmidt
@ 2007-07-10 12:52   ` Josh Boyer
  2007-07-10 15:41     ` Alexander Schmidt
  1 sibling, 1 reply; 7+ messages in thread
From: Josh Boyer @ 2007-07-10 12:52 UTC (permalink / raw)
  To: haver
  Cc: Alexander Schmidt, linux-mtd@lists.infradead.org, Andreas Arnez,
	Frank Haverkamp

On Tue, 2007-07-10 at 08:52 +0200, Frank Haverkamp wrote:
> Hi Alex,
> 
> On Mon, 2007-07-09 at 21:32 +0200, Alexander Schmidt wrote:
> > +#define COMPARE_BUFFER_SIZE 1024
> 
> I think using the something like 2KiB or even the 128KiB would be nicer.
> 
> > +
> >  static const char copyright [] __unused =
> >         "Copyright (c) International Business Machines Corp., 2006";
> 2007

And drop the (c) part.  It's not legally valid.

josh

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC/PATCH] Ubi-Utils: add compare feature
  2007-07-10 12:52   ` Josh Boyer
@ 2007-07-10 15:41     ` Alexander Schmidt
  0 siblings, 0 replies; 7+ messages in thread
From: Alexander Schmidt @ 2007-07-10 15:41 UTC (permalink / raw)
  To: linux-mtd; +Cc: haver, Frank Haverkamp, Andreas Arnez

On Tuesday 10 July 2007, Josh Boyer wrote:
> 
> And drop the (c) part.  It's not legally valid.

Okay, done.

Alex

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2007-07-10 15:41 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-07-09 19:32 [RFC/PATCH] Ubi-Utils: add compare feature Alexander Schmidt
2007-07-10  6:52 ` Frank Haverkamp
2007-07-10 10:14   ` Alexander Schmidt
2007-07-10 12:52   ` Josh Boyer
2007-07-10 15:41     ` Alexander Schmidt
2007-07-10  7:09 ` Artem Bityutskiy
2007-07-10 10:20   ` Alexander Schmidt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox