Re: [flashrom] [RFC] JEDEC refactor w/ conversion notes and file eliminations

25 Dec 2009

On 25.12.2009 22:48, Sean Nelson wrote:
...
On 12/25/09 11:23 AM, Carl-Daniel Hailfinger wrote:
...
On 25.12.2009 07:03, Sean Nelson wrote:
...

en29f002a ->  file_not_used

What does the file_not_used mean? Was it never hooked up at all? In that
case, it would be cool if we could go through each of the functions and
check if we have to add a corresponding flashchip entry for it.
file not used is a file that is not hooked up anywhere in the source
code.  What I could figure out is that the en29f002a functions are
nearly exactly the same as the am29f040b. And the flashchip entries
are already using the _29f040b functions.
Ah OK. I think sometime in the past we forgot to create flashchips.c
entries for the chips en29f002a.c was created for.
...
As it stands, we still need/use w29ee011 for that very check.
The good news is that we can narrow down the check. The chip that dies
is LPC, but w29ee011 is Parallel, so if buses_supported doesn't include
LPC, we can probe without worries.
...
...
Mapping flash registers or not is something that should end up outside
probe, probably being controlled by a flag in a to-be-created bitfield
in struct flashchip. For now, I think a pretty good heuristic is that
LPC/FWH flahs has registers, and Parallel/SPI doesn't because Parallel
doesn't have enough address lines for this and SPI has opcodes which
take care of this register stuff.
In struct flashchips, I've added "int remap" for this very reason.
I'd rather have
int feature_bits;
in struct flashchip and set a registermap bit like this:
#define FEATURE_REGISTERMAP (1 << 0)
.feature_bits = FEATURE_REGISTERMAP,
...
Everything is now using mask-only.
Very nice.
...
I've removed the common stuff from the header except for
erase_sector_jedec_common since at the moment I'd like to keep the
other files intact and its used in the other files.
Thanks.
...
...
I don't see a replacement for write_jedec_1. Is there something I
missed? Some chips need the byte program sequence before each
single-byte write.
One way of solving and replacing write_jedec_1 is to add a
byte_based_write field to struct flashchips
Hm yes. For now, I'd like to keep it as write_jedec_1 if possible.
...
...
Why is verify_range commented out here?
I couldn't figure out how to get "int start" back for it, I think I
fixed it. See the patch.
Will take a look.
Blanket answer to the rest of your replies: OK.
...

diff --git a/chipdrivers.h b/chipdrivers.h
index adcb46d..3ccc478 100644
--- a/chipdrivers.h
+++ b/chipdrivers.h
@@ -78,20 +78,21 @@ void toggle_ready_jedec(chipaddr dst);
 void data_polling_jedec(chipaddr dst, uint8_t data);
 void start_program_jedec(chipaddr bios);
 int write_byte_program_jedec(chipaddr bios, uint8_t *src,
   		     chipaddr dst);
 int probe_jedec(struct flashchip *flash);
 int erase_chip_jedec(struct flashchip *flash);
 int write_jedec(struct flashchip *flash, uint8_t *buf);
-int write_jedec_1(struct flashchip *flash, uint8_t *buf);
 int erase_sector_jedec(struct flashchip *flash, unsigned int page, unsigned int pagesize);
 int erase_block_jedec(struct flashchip *flash, unsigned int page, unsigned int blocksize);
 int erase_chip_block_jedec(struct flashchip *flash, unsigned int page, unsigned int blocksize);
-int write_sector_jedec(chipaddr bios, uint8_t *src,

       chipaddr dst, unsigned int page_size);



+int write_sector_jedec_common(struct flashchip *flash, uint8_t *src,

		chipaddr dst, unsigned int page_size,


		unsigned int mask);




/* m29f002.c */
 int erase_m29f002(struct flashchip *flash);
 int write_m29f002t(struct flashchip *flash, uint8_t *buf);
 int write_m29f002b(struct flashchip *flash, uint8_t *buf);
/* m29f400bt.c */
diff --git a/flash.h b/flash.h
index feac98e..cb3bc77 100644
--- a/flash.h
+++ b/flash.h
@@ -156,14 +156,16 @@ struct flashchip {
    * Identification code.
    */
   uint32_t manufacture_id;
   uint32_t model_id;
int total_size;
   int page_size;

int remap;

See my bitfield comments above.
...

int byte_based_write;

Can we please either postpone this and keep jedec_write_1 or at least
rename the field to write_granularity (in bytes) or somesuch. The latter
will require setting a boatload of chips to write_granularity=256, and
the former will require more stuff inside jedec.c. Bah.
...
/*
    * Indicate if flashrom has been tested with this flash chip and if
    * everything worked correctly.
    */
   uint32_t tested;
diff --git a/flashchips.c b/flashchips.c
index 59f9139..af9039d 100644
--- a/flashchips.c
+++ b/flashchips.c
@@ -102,15 +102,15 @@ struct flashchip flashchips[] = {
   			},
   			.block_erase = erase_sector_jedec,
   		}, {
   			.eraseblocks = { {256 * 1024, 1} },
   			.block_erase = erase_chip_block_jedec,
   		},
   	},

.write		= write_jedec_1,




.write		= write_jedec,

.read		= read_memmapped,
},
{
.vendor		= "AMD",
.name		= "Am29F002(N)BT",
.bustype	= CHIP_BUSTYPE_PARALLEL,


@@ -133,15 +133,15 @@ struct flashchip flashchips[] = {
   			},
   			.block_erase = erase_sector_jedec,
   		}, {
   			.eraseblocks = { {256 * 1024, 1} },
   			.block_erase = erase_chip_block_jedec,
   		},
   	},

.write		= write_jedec_1,




.write		= write_jedec,

.read		= read_memmapped,
},
{
.vendor		= "AMD",
.name		= "Am29F016D",
.bustype	= CHIP_BUSTYPE_PARALLEL,


@@ -1178,15 +1178,15 @@ struct flashchip flashchips[] = {
   			},
   			.block_erase = erase_sector_29f002,
   		}, {
   			.eraseblocks = { {256 * 1024, 1} },
   			.block_erase = erase_chip_29f002,
   		},
   	},

.write		= write_jedec_1,




.write		= write_jedec,

.read		= read_memmapped,
},
{
.vendor		= "AMIC",
.name		= "A29002T",
.bustype	= CHIP_BUSTYPE_PARALLEL,


@@ -1209,15 +1209,15 @@ struct flashchip flashchips[] = {
   			},
   			.block_erase = erase_sector_29f002,
   		}, {
   			.eraseblocks = { {256 * 1024, 1} },
   			.block_erase = erase_chip_29f002,
   		},
   	},

.write		= write_jedec_1,




.write		= write_jedec,

.read		= read_memmapped,
},
{
.vendor		= "AMIC",
.name		= "A29040B",
.bustype	= CHIP_BUSTYPE_PARALLEL,


@@ -1570,15 +1570,15 @@ struct flashchip flashchips[] = {
   			},
   			.block_erase = erase_sector_jedec,
   		}, {
   			.eraseblocks = { {256 * 1024, 1} },
   			.block_erase = erase_chip_block_jedec,
   		},
   	},

.write		= write_jedec_1,




.write		= write_jedec,

.read		= read_memmapped,
},
{
.vendor		= "EON",
.name		= "EN29F002(A)(N)T",
.bustype	= CHIP_BUSTYPE_PARALLEL,


@@ -1601,15 +1601,15 @@ struct flashchip flashchips[] = {
   			},
   			.block_erase = erase_sector_jedec,
   		}, {
   			.eraseblocks = { {256 * 1024, 1} },
   			.block_erase = erase_chip_block_jedec,
   		},
   	},

.write		= write_jedec_1,




.write		= write_jedec,

.read		= read_memmapped,
},
{
.vendor		= "Fujitsu",
.name		= "MBM29F004BC",
.bustype	= CHIP_BUSTYPE_PARALLEL,


@@ -2108,15 +2108,15 @@ struct flashchip flashchips[] = {
   	.model_id	= MX_29F001B,
   	.total_size	= 128,
   	.page_size	= 32 * 1024,
   	.tested		= TEST_OK_PRE,
   	.probe		= probe_29f002,
   	.probe_timing	= TIMING_IGNORED, /* routine don't use probe_timing (mx29f002.c) */
   	.erase		= erase_29f002,

.write		= write_jedec_1,




.write		= write_jedec,

.read		= read_memmapped,
},
{
.vendor		= "Macronix",
.name		= "MX29F001T",
.bustype	= CHIP_BUSTYPE_PARALLEL,


@@ -2124,15 +2124,15 @@ struct flashchip flashchips[] = {
   	.model_id	= MX_29F001T,
   	.total_size	= 128,
   	.page_size	= 32 * 1024,
   	.tested		= TEST_OK_PRE,
   	.probe		= probe_29f002,
   	.probe_timing	= TIMING_IGNORED, /* routine don't use probe_timing (mx29f002.c) */
   	.erase		= erase_29f002,

.write		= write_jedec_1,




.write		= write_jedec,

.read		= read_memmapped,
},
{
.vendor		= "Macronix",
.name		= "MX29F002B",
.bustype	= CHIP_BUSTYPE_PARALLEL,


@@ -2155,15 +2155,15 @@ struct flashchip flashchips[] = {
   			},
   			.block_erase = erase_sector_29f002,	/* This erase function has 64k blocksize for eLiteFlash */
   		}, {
   			.eraseblocks = { {256 * 1024, 1} },
   			.block_erase = erase_chip_29f002,
   		},
   	},

.write		= write_jedec_1,




.write		= write_jedec,

.read		= read_memmapped,
},
{
.vendor		= "Macronix",
.name		= "MX29F002T",
.bustype	= CHIP_BUSTYPE_PARALLEL,


@@ -2186,15 +2186,15 @@ struct flashchip flashchips[] = {
   			},
   			.block_erase = erase_sector_29f002,	/* This erase function has 64k blocksize for eLiteFlash */
   		}, {
   			.eraseblocks = { {256 * 1024, 1} },
   			.block_erase = erase_chip_29f002,
   		},
   	},

.write		= write_jedec_1,




.write		= write_jedec,

.read		= read_memmapped,
},
{
.vendor		= "Macronix",
.name		= "MX29LV040",
.bustype	= CHIP_BUSTYPE_PARALLEL,


@@ -2202,15 +2202,15 @@ struct flashchip flashchips[] = {
   	.model_id	= MX_29LV040,
   	.total_size	= 512,
   	.page_size	= 64 * 1024,
   	.tested		= TEST_OK_PR,
   	.probe		= probe_29f002,
   	.probe_timing	= TIMING_IGNORED, /* routine don't use probe_timing (mx29f002.c) */
   	.erase		= erase_29f002,

.write		= write_jedec_1,




.write		= write_jedec,

.read		= read_memmapped,
},
{
.vendor		= "Numonyx",
.name		= "M25PE10",
.bustype	= CHIP_BUSTYPE_SPI,


diff --git a/jedec.c b/jedec.c
index d6cad41..302bee0 100644
--- a/jedec.c
+++ b/jedec.c
@@ -83,22 +83,25 @@ void data_polling_jedec(chipaddr dst, uint8_t data)
   		break;
   	}
   }
   if (i > 0x100000)
   	printf_debug("%s: excessive loops, i=0x%x\n", __func__, i);
 }
-void start_program_jedec(chipaddr bios)
+void start_program_jedec_common(struct flashchip *flash, unsigned int mask)
 {

chip_writeb(0xAA, bios + 0x5555);
chip_writeb(0x55, bios + 0x2AAA);
chip_writeb(0xA0, bios + 0x5555);


chipaddr bios = flash->virtual_memory;
chip_writeb(0xAA, bios + (0x5555 & mask));
chip_writeb(0x55, bios + (0x2AAA & mask));
chip_writeb(0xA0, bios + (0x5555 & mask));

Exactly as I wanted it.
...
}
-int probe_jedec(struct flashchip *flash)
+int probe_jedec_common(struct flashchip *flash,

	unsigned int mid_addr, unsigned int did_addr,


	unsigned int mask, int long_reset)



{
   chipaddr bios = flash->virtual_memory;
   uint8_t id1, id2;
   uint32_t largeid1, largeid2;
   uint32_t flashcontent1, flashcontent2;
   int probe_timing_enter, probe_timing_exit;
@@ -114,27 +117,27 @@ int probe_jedec(struct flashchip *flash)
   } else {
   	printf("Chip has negative value in probe_timing, failing "
   	       "without chip access\n");
   	return 0;
   }
/* Issue JEDEC Product ID Entry command */

chip_writeb(0xAA, bios + 0x5555);


chip_writeb(0xAA, bios + (0x5555 & mask));
if (probe_timing_enter)
programmer_delay(10);


chip_writeb(0x55, bios + 0x2AAA);


chip_writeb(0x55, bios + (0x2AAA & mask));
if (probe_timing_enter)
programmer_delay(10);


chip_writeb(0x90, bios + 0x5555);


chip_writeb(0x90, bios + (0x5555 & mask));
if (probe_timing_enter)
programmer_delay(probe_timing_enter);
/* Read product ID */



id1 = chip_readb(bios);
id2 = chip_readb(bios + 0x01);


id1 = chip_readb(bios + mid_addr);
id2 = chip_readb(bios + did_addr);

Well, it seems I missed this in the last review. We should always read
the first part of the ID at 0x00/0x01 and I thought did_addr/mid_addr
specified the followup locations used for largeid1/largeid2.
...
largeid1 = id1;
   largeid2 = id2;
/* Check if it is a continuation ID, this should be a while loop. */
   if (id1 == 0x7F) {
   	largeid1 <<= 8;
   	id1 = chip_readb(bios + 0x100);
@@ -143,21 +146,24 @@ int probe_jedec(struct flashchip *flash)
   if (id2 == 0x7F) {
   	largeid2 <<= 8;
   	id2 = chip_readb(bios + 0x101);
   	largeid2 |= id2;
   }
/* Issue JEDEC Product ID Exit command */

chip_writeb(0xAA, bios + 0x5555);
if (probe_timing_exit)
programmer_delay(10);


chip_writeb(0x55, bios + 0x2AAA);
if (probe_timing_exit)
programmer_delay(10);


chip_writeb(0xF0, bios + 0x5555);


if (long_reset)

{

chip_writeb(0xAA, bios + (0x5555 & mask));


if (probe_timing_exit)


	programmer_delay(10);


chip_writeb(0x55, bios + (0x2AAA & mask));


if (probe_timing_exit)


	programmer_delay(10);


}

chip_writeb(0xF0, bios + (0x5555 & mask));
if (probe_timing_exit)
programmer_delay(probe_timing_exit);
printf_debug("%s: id1 0x%02x, id2 0x%02x", __func__, largeid1, largeid2);
if (!oddparity(id1))
printf_debug(", id1 parity violation");


@@ -180,243 +186,213 @@ int probe_jedec(struct flashchip *flash)
   if (largeid2 == flashcontent2)
   	printf_debug(", id2 is normal flash content");
printf_debug("\n");
   if (largeid1 == flash->manufacture_id && largeid2 == flash->model_id)
   	return 1;

if (flash->remap)
map_flash_registers(flash);




I'll be honest. If we really have remap (or registermap) as a value in
struct flashchip, we could move the register mapping to generic probe
code in flashrom.c.
...

return 0;

}
-int erase_sector_jedec(struct flashchip *flash, unsigned int page, unsigned int pagesize)
+int erase_sector_jedec_common(struct flashchip *flash, unsigned int page,

	      unsigned int pagesize, unsigned int mask)



{
   chipaddr bios = flash->virtual_memory;
/*  Issue the Sector Erase command   */

chip_writeb(0xAA, bios + 0x5555);


chip_writeb(0xAA, bios + (0x5555 & mask));
programmer_delay(10);


chip_writeb(0x55, bios + 0x2AAA);


chip_writeb(0x55, bios + (0x2AAA & mask));
programmer_delay(10);


chip_writeb(0x80, bios + 0x5555);


chip_writeb(0x80, bios + (0x5555 & mask));
programmer_delay(10);


chip_writeb(0xAA, bios + 0x5555);


chip_writeb(0xAA, bios + (0x5555 & mask));
programmer_delay(10);


chip_writeb(0x55, bios + 0x2AAA);


chip_writeb(0x55, bios + (0x2AAA & mask));
programmer_delay(10);
chip_writeb(0x30, bios + page);
programmer_delay(10);
/* wait for Toggle bit ready         */
toggle_ready_jedec_slow(bios);
if (check_erased_range(flash, page, pagesize)) {
fprintf(stderr,"ERASE FAILED!\n");
return -1;
}
return 0;


}
-int erase_block_jedec(struct flashchip *flash, unsigned int block, unsigned int blocksize)
+int erase_block_jedec_common(struct flashchip *flash, unsigned int block,

	     unsigned int blocksize, unsigned int mask)



{
   chipaddr bios = flash->virtual_memory;
/*  Issue the Sector Erase command   */

chip_writeb(0xAA, bios + 0x5555);


chip_writeb(0xAA, bios + (0x5555 & mask));
programmer_delay(10);


chip_writeb(0x55, bios + 0x2AAA);


chip_writeb(0x55, bios + (0x2AAA & mask));
programmer_delay(10);


chip_writeb(0x80, bios + 0x5555);


chip_writeb(0x80, bios + (0x5555 & mask));
programmer_delay(10);


chip_writeb(0xAA, bios + 0x5555);


chip_writeb(0xAA, bios + (0x5555 & mask));
programmer_delay(10);


chip_writeb(0x55, bios + 0x2AAA);


chip_writeb(0x55, bios + (0x2AAA & mask));
programmer_delay(10);
chip_writeb(0x50, bios + block);
programmer_delay(10);
/* wait for Toggle bit ready         */
toggle_ready_jedec_slow(bios);
if (check_erased_range(flash, block, blocksize)) {
fprintf(stderr,"ERASE FAILED!\n");
return -1;
}
return 0;


}
-/* erase chip with block_erase() prototype */
-int erase_chip_block_jedec(struct flashchip *flash, unsigned int addr, unsigned int blocksize)
-{

if ((addr != 0) || (blocksize != flash->total_size * 1024)) {
fprintf(stderr, "%s called with incorrect arguments\n",


	__func__);


return -1;


}
return erase_chip_jedec(flash);

-}



-int erase_chip_jedec(struct flashchip *flash)
+int erase_chip_jedec_common(struct flashchip *flash, unsigned int mask)
 {
   int total_size = flash->total_size * 1024;
   chipaddr bios = flash->virtual_memory;
/*  Issue the JEDEC Chip Erase command   */

chip_writeb(0xAA, bios + 0x5555);


chip_writeb(0xAA, bios + (0x5555 & mask));
programmer_delay(10);


chip_writeb(0x55, bios + 0x2AAA);


chip_writeb(0x55, bios + (0x2AAA & mask));
programmer_delay(10);


chip_writeb(0x80, bios + 0x5555);


chip_writeb(0x80, bios + (0x5555 & mask));
programmer_delay(10);


chip_writeb(0xAA, bios + 0x5555);


chip_writeb(0xAA, bios + (0x5555 & mask));
programmer_delay(10);


chip_writeb(0x55, bios + 0x2AAA);


chip_writeb(0x55, bios + (0x2AAA & mask));
programmer_delay(10);


chip_writeb(0x10, bios + 0x5555);


chip_writeb(0x10, bios + (0x5555 & mask));
programmer_delay(10);
toggle_ready_jedec_slow(bios);
if (check_erased_range(flash, 0, total_size)) {
fprintf(stderr,"ERASE FAILED!\n");
return -1;
}
return 0;


}
-int write_page_write_jedec(struct flashchip *flash, uint8_t *src,

	   int start, int page_size)



+int write_sector_jedec_common(struct flashchip *flash, uint8_t *src,

		  chipaddr dst, unsigned int page_size,


		  unsigned int mask)




Not sure about the changed prototype from "int start" to "chipaddr dst".
Usage of chipaddr can go horribly wrong if confused with start. I am
guilty of a few blunders in that area myself (did find most of them
before submitting, though).
...
{
   int i, tried = 0, failed;

uint8_t *s = src;
chipaddr bios = flash->virtual_memory;
chipaddr dst = bios + start;
chipaddr d = dst;


chipaddr v = bios;
unsigned int start = dst - bios;

retry:
   /* Issue JEDEC Data Unprotect comand */
While you're at it, call it JEDEC Start Program command. That makes it
clear what we're doing here.
...

start_program_jedec(bios);


start_program_jedec_common(flash, mask);
/* transfer data from source to destination */



for (i = 0; i < page_size; i++) {
/* If the data is 0xFF, don't program it */




if (flash->byte_based_write)

I don't like the if/else branch. Either use my write_granularity
suggestion (and that variable can then be used as direct replacement of
page_size) or at least set a local variable (e.g. named
write_granularity or if you want write_page_size or whatever) to the
value used as loop terminator (1 is a valid number of loops).
...

{
for (i = 0; i < page_size; i++) {


	/* If the data is 0xFF, don't program it */


	if (*src != 0xFF)


		chip_writeb(*src, dst);


	dst++;


	src++;


}


v = dst - 1;


}
else
{
if (*src != 0xFF)
	chip_writeb(*src, dst);


dst++;


src++;

}

toggle_ready_jedec(dst - 1);


dst = d;

src = s;



toggle_ready_jedec(v);
failed = verify_range(flash, src, start, page_size, NULL);
if (failed && tried++ < MAX_REFLASH_TRIES) {
fprintf(stderr, "retrying.\n");
goto retry;
}
if (failed) {
fprintf(stderr, " page 0x%lx failed!\n",



	(d - bios) / page_size);


}
return failed;

-}



-int write_byte_program_jedec(chipaddr bios, uint8_t *src,

	     chipaddr dst)



-{

int tried = 0, failed = 0;

/* If the data is 0xFF, don't program it and don't complain. */
if (*src == 0xFF) {
return 0;


}


-retry:

/* Issue JEDEC Byte Program command */
start_program_jedec(bios);

/* transfer data from source to destination */
chip_writeb(*src, dst);
toggle_ready_jedec(bios);

if (chip_readb(dst) != *src && tried++ < MAX_REFLASH_TRIES) {
goto retry;


}

if (tried >= MAX_REFLASH_TRIES)
failed = 1;



return failed;

-}



-int write_sector_jedec(chipaddr bios, uint8_t *src,

       chipaddr dst, unsigned int page_size)



-{

int i, failed = 0;
chipaddr olddst;

olddst = dst;
for (i = 0; i < page_size; i++) {
if (write_byte_program_jedec(bios, src, dst))


	failed = 1;


dst++, src++;




	(dst - bios) / page_size);

}


if (failed)
fprintf(stderr, " writing sector at 0x%lx failed!\n", olddst);


return failed;

}
-int write_jedec(struct flashchip *flash, uint8_t *buf)
+int write_jedec_common(struct flashchip *flash, uint8_t *buf, unsigned int mask)
 {
   int i, failed = 0;
   int total_size = flash->total_size * 1024;
   int page_size = flash->page_size;

if (erase_chip_jedec(flash)) {


if (erase_chip_jedec_common(flash, mask)) {
fprintf(stderr,"ERASE FAILED!\n");
return -1;
}
printf("Programming page: ");
for (i = 0; i < total_size / page_size; i++) {
printf("%04d at address: 0x%08x", i, i * page_size);



if (write_page_write_jedec(flash, buf + i * page_size,


			   i * page_size, page_size))




if (write_sector_jedec_common(flash, buf + i * page_size,


			   i * page_size, page_size, mask))




Hm. Using page_size here seems very out of place. Not your fault,
though. It should be revisited once we kill page_size.
...
	failed = 1;
printf("\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b");

}
   printf("\n");
return failed;
 }
-int write_jedec_1(struct flashchip *flash, uint8_t * buf)
-{

int i;

chipaddr bios = flash->virtual_memory;

chipaddr dst = bios;

programmer_delay(10);

if (erase_flash(flash)) {

fprintf(stderr, "ERASE FAILED!\n");



+/* erase chip with block_erase() prototype */
+int erase_chip_block_jedec(struct flashchip *flash, unsigned int addr,

	   unsigned int blocksize)



+{

if ((addr != 0) || (blocksize != flash->total_size * 1024)) {
fprintf(stderr, "%s called with incorrect arguments\n",


	__func__);

return -1;
}
return erase_chip_jedec_common(flash, 0xffff);

+}

printf("Programming page: ");
for (i = 0; i < flash->total_size; i++) {
if ((i & 0x3) == 0)


	printf("address: 0x%08lx", (unsigned long)i * 1024);



+void start_program_jedec(struct flashchip *flash)
+{

start_program_jedec_common(flash, 0xffff);

+}
Is this still used?
...

           write_sector_jedec(bios, buf + i * 1024, dst + i * 1024, 1024);



+int probe_jedec(struct flashchip *flash)
+{

return probe_jedec_common(flash, 0x00, 0x01, 0xffff, 1);

#define SHORT_RESET 0
#define LONG_RESET 1
and use LONG_RESET here. It would make some stuff more readable.
Or we go the full way to abstraction and add a second bit to the
.feature_bits in struct flashchip.
#define FEATURE_RESET_LONG (1 << 1)
#define FEATURE_RESET_SHORT (0 << 1)
#define FEATURE_RESET_EITHER FEATURE_RESET_SHORT
.feature_bits = FEATURE_LONGRESET,
...
+}

if ((i & 0x3) == 0)


	printf("\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b");


}

+int erase_sector_jedec(struct flashchip *flash, unsigned int page, unsigned int size)
+{

return erase_sector_jedec_common(flash, page, size, 0xffff);

+}
+int erase_block_jedec(struct flashchip *flash, unsigned int page, unsigned int size)
+{

return erase_block_jedec_common(flash, page, size, 0xffff);

+}

printf("\n");
return 0;

+int erase_chip_jedec(struct flashchip *flash)
+{

return erase_chip_jedec_common(flash, 0xffff);

}



+int write_jedec(struct flashchip *flash, uint8_t *buf)
+{

return write_jedec_common(flash, buf, 0xffff);

+}



diff --git a/pm29f002.c b/pm29f002.c
index bf78d13..8f1d010 100644
--- a/pm29f002.c
+++ b/pm29f002.c
@@ -17,15 +17,15 @@

along with this program; if not, write to the Free Software
Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 USA

*/
#include "flash.h"
/* if write_sector_jedec is used,

this is write_jedec_1 */


this is write_jedec */

My usual write_jedec_1 complaint.
...
int write_pm29f002(struct flashchip *flash, uint8_t *buf)
 {
   int i, total_size = flash->total_size * 1024;
   chipaddr bios = flash->virtual_memory;
   chipaddr dst = bios;
if (erase_flash(flash)) {
diff --git a/pm49fl00x.c b/pm49fl00x.c
index 27a1163..424b0ed 100644
--- a/pm49fl00x.c
+++ b/pm49fl00x.c
@@ -97,16 +97,16 @@ int write_49fl00x(struct flashchip *flash, uint8_t *buf)
   	if (erase_block_jedec(flash, i * page_size, page_size)) {
   		fprintf(stderr, "ERASE FAILED!\n");
   		return -1;
   	}
/* write to the sector */
printf("%04d at address: 0x%08x", i, i * page_size);


write_sector_jedec(bios, buf + i * page_size,


		   bios + i * page_size, page_size);




write_sector_jedec_common(flash, buf + i * page_size,


		   bios + i * page_size, page_size, 0xffff);

printf("\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b");
fflush(stdout);
}
printf("\n");
/* protected */
write_lockbits_49fl00x(flash->virtual_registers, total_size, 1,


diff --git a/sst49lf040.c b/sst49lf040.c
index ab1c918..c91a139 100644
--- a/sst49lf040.c
+++ b/sst49lf040.c
@@ -55,16 +55,16 @@ int write_49lf040(struct flashchip *flash, uint8_t *buf)
   		return -1;
   	}
/* write to the sector */
if (i % 10 == 0)
	printf("%04d at address: 0x%08x ", i, i * page_size);



write_sector_jedec(bios, buf + i * page_size,


		   bios + i * page_size, page_size);




write_sector_jedec_common(flash, buf + i * page_size,


		   bios + i * page_size, page_size, 0xffff);


if (i % 10 == 0)
	printf("\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b");
fflush(stdout);
}
printf("\n");


diff --git a/sst_fwhub.c b/sst_fwhub.c
index f09aa54..4a976e6 100644
--- a/sst_fwhub.c
+++ b/sst_fwhub.c
@@ -153,16 +153,16 @@ int write_sst_fwhub(struct flashchip *flash, uint8_t *buf)
   	flash->read(flash, readbuf, i * page_size, page_size);
   	if (memcmp((void *)(buf + i * page_size),
   		   (void *)(readbuf), page_size)) {
   		rc = erase_sst_fwhub_block(flash, i * page_size,
   					   page_size);
   		if (rc)
   			return 1;

	write_sector_jedec(bios, buf + i * page_size,


			   bios + i * page_size, page_size);




	write_sector_jedec_common(flash, buf + i * page_size,


			   bios + i * page_size, page_size, 0xffff);

}
printf("\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b");
}
printf("\n");
return 0;


}
diff --git a/w39v040c.c b/w39v040c.c
index 722ae29..66ab115 100644
--- a/w39v040c.c
+++ b/w39v040c.c
@@ -76,15 +76,15 @@ int write_w39v040c(struct flashchip *flash, uint8_t *buf)
   	fprintf(stderr, "ERASE FAILED!\n");
   	return -1;
   }
printf("Programming page: ");
   for (i = 0; i < total_size / page_size; i++) {
   	printf("%04d at address: 0x%08x", i, i * page_size);

write_sector_jedec(bios, buf + i * page_size,


		   bios + i * page_size, page_size);




write_sector_jedec_common(flash, buf + i * page_size,


		   bios + i * page_size, page_size, 0xffff);

printf("\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b");
}
printf("\n");
return 0;


}
diff --git a/w39v080fa.c b/w39v080fa.c
index 580657f..311e55b 100644
--- a/w39v080fa.c
+++ b/w39v080fa.c
@@ -178,13 +178,13 @@ int write_winbond_fwhub(struct flashchip *flash, uint8_t *buf)
if (erase_winbond_fwhub(flash))
   	return -1;
printf("Programming: ");
   for (i = 0; i < total_size; i += flash->page_size) {
   	printf("0x%08x\b\b\b\b\b\b\b\b\b\b", i);

write_sector_jedec(bios, buf + i, bios + i, flash->page_size);




write_sector_jedec_common(flash, buf + i, bios + i, flash->page_size, 0xffff);

}
printf("\n");
return 0;


}
diff --git a/w49f002u.c b/w49f002u.c
index d12bc72..87ce000 100644
--- a/w49f002u.c
+++ b/w49f002u.c
@@ -32,16 +32,16 @@ int write_49f002(struct flashchip *flash, uint8_t *buf)
   	return -1;
   }
printf("Programming page: ");
   for (i = 0; i < total_size / page_size; i++) {
   	printf("%04d at address: 0x%08x ", i, i * page_size);
   	/* Byte-wise writing of 'page_size' bytes. */

write_sector_jedec(bios, buf + i * page_size,


		   bios + i * page_size, page_size);




write_sector_jedec_common(flash, buf + i * page_size,


		   bios + i * page_size, page_size, 0xffff);

printf("\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b");
fflush(stdout);
}
printf("\n");
return 0;


}
I have to admit I didn't check the conversion of the chip drivers. They
look sane on a quick glance, but I'll have to verify calling conventions
(int start vs. chipaddr dst) closely before giving a final Ack.
To be honest, I will kill all progress printing first thing after this
is committed. Better no progress printing than the current slow (and
sometimes buggy) stuff.
Regards,
Carl-Daniel
-- 
Developer quote of the year:
"We are juggling too many chainsaws and flaming arrows and tigers."



    

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

Re: [flashrom] [RFC] JEDEC refactor w/ conversion notes and file eliminations