Patchworkβ [v2] Intel 28F004/28F400 support

login
register
about
Submitter Michael Karcher
Date 2010-04-01 18:12:48
Message ID <1270145568-21892-1-git-send-email-flashrom@mkarcher.dialup.fu-berlin.de>
Download mbox | patch
Permalink /patch/1185/
State Accepted
Commit r991
Headers show

Comments

Michael Karcher - 2010-04-01 18:12:48
This patch removes blockwise write for i82802ab chips. It will be
reintroduced in post-0.9.2 in a generic way. It is needed to fix
FWH-like chips with non-uniform sectors. It adds IDs for 28F004/28F400

These are:
  Intel 28F001
  Sharp LHF00L04
  ST M50FW002
  ST M50LPW116

Signed-off-by: Michael Karcher <flashrom@mkarcher.dialup.fu-berlin.de>
---
 82802ab.c    |   54 +++++++++-------------------
 flash.h      |    2 +-
 flashchips.c |  110 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 flashchips.h |    4 ++
 4 files changed, 133 insertions(+), 37 deletions(-)
 create mode 100755 dmidecode
Carl-Daniel Hailfinger - 2010-04-03 10:08:26
On 01.04.2010 20:12, Michael Karcher wrote:
> Remove blockwise write for i82802ab chips. It will be reintroduced
> in post-0.9.2 in a generic way. This is needed to fix
> FWH-like chips with non-uniform sectors.
>
> These are:
>   Intel 28F001
>   Sharp LHF00L04
>   ST M50FW002
>   ST M50LPW116
>
> Add IDs for Intel 28F004/28F400
>   

I have modified the changelog a bit.

> Signed-off-by: Michael Karcher <flashrom@mkarcher.dialup.fu-berlin.de>
>   

With the comments below addressed, this is
Acked-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net>


>  82802ab.c    |   54 +++++++++-------------------
>  flash.h      |    2 +-
>  flashchips.c |  110 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  flashchips.h |    4 ++
>  4 files changed, 133 insertions(+), 37 deletions(-)
>  create mode 100755 dmidecode
>
> diff --git a/82802ab.c b/82802ab.c
> index aa7e45e..0d1bd1a 100644
> --- a/82802ab.c
> +++ b/82802ab.c
> @@ -48,6 +48,7 @@ int probe_82802ab(struct flashchip *flash)
>  	chipaddr bios = flash->virtual_memory;
>  	uint8_t id1, id2;
>  	uint8_t flashcontent1, flashcontent2;
> +	int shifted = (flash->feature_bits & FEATURE_ADDR_SHIFTED) != 0;
>   

Can we rely on the compiler to set shifted=1 if the feature bit is set?
We assign a boolean value to int shifted, and AFAIK the compiler is free
to use pretty much any value here as long as that value gives reliable
results (could be 0x2, could be 0xffffffff). What about the following
code instead?

+	int shifted = (flash->feature_bits & FEATURE_ADDR_SHIFTED) ? 1 : 0;



>  
>  	/* Reset to get a clean state */
>  	chip_writeb(0xFF, bios);
> @@ -178,44 +179,25 @@ void write_page_82802ab(chipaddr bios, uint8_t *src,
>  int write_82802ab(struct flashchip *flash, uint8_t *buf)
>  {
>  	int i;
> -	int total_size = flash->total_size * 1024;
> -	int page_size = flash->page_size;
>  	chipaddr bios = flash->virtual_memory;
> -	uint8_t *tmpbuf = malloc(page_size);
>  
> -	if (!tmpbuf) {
> -		msg_cerr("Could not allocate memory!\n");
> -		exit(1);
> +	if (erase_flash(flash)) {
> +		msg_cerr("ERASE FAILED!\n");
> +		return -1;
>  	}
> -	msg_cinfo("Programming page: \n");
> -	for (i = 0; i < total_size / page_size; i++) {
> [...]
> +	msg_cinfo("Programming page: ");
> +	for (i = 0; i < flash->total_size; i++) {
>   

You change the write chunk size from page_size to 1024. Does
"programming page" still make sense, or should it be "programming chunk at"?


> diff --git a/dmidecode b/dmidecode
> new file mode 100755
> index 0000000..e69de29
>   

Uhhh. Please make sure you don't commit dmidecode by accident.


> diff --git a/flashchips.c b/flashchips.c
> index 2dbc1e0..a9b6551 100644
> --- a/flashchips.c
> +++ b/flashchips.c
> @@ -2385,6 +2385,116 @@ struct flashchip flashchips[] = {
> +	{
> +		.vendor		= "Intel",
> +		.name		= "28F400BV/CV/CE-B",
> +		.bustype	= CHIP_BUSTYPE_PARALLEL,
> +		.manufacture_id	= INTEL_ID,
> +		.model_id	= P28F400BB,
> +	},
> +
> +	{
> +		.vendor		= "Intel",
> +		.name		= "28F400BV/CV/CE-B",
>   

Should be ...CE-T unless I'm mistaken.


> +		.bustype	= CHIP_BUSTYPE_PARALLEL,
> +		.manufacture_id	= INTEL_ID,
> +		.model_id	= P28F400BT,
>   

A compile test and run test with -p dummy (which triggers the automatic
chipdefinition selftest) would be appreciated before committing just to
make sure I didn't overlook anything problematic.

Regards,
Carl-Daniel
Michael Karcher - 2010-04-03 11:31:55
Am Samstag, den 03.04.2010, 12:08 +0200 schrieb Carl-Daniel Hailfinger:
> > Remove blockwise write for i82802ab chips. It will be reintroduced
> > in post-0.9.2 in a generic way. This is needed to fix
> > FWH-like chips with non-uniform sectors.
> I have modified the changelog a bit.
Fine, adjusted.

> > Signed-off-by: Michael Karcher <flashrom@mkarcher.dialup.fu-berlin.de>
> Acked-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net>
Thanks. Committed in r991


> > +	int shifted = (flash->feature_bits & FEATURE_ADDR_SHIFTED) != 0;
> Can we rely on the compiler to set shifted=1 if the feature bit is set?
As discussed on IRC, we can.

> > +	msg_cinfo("Programming page: ");
> > +	for (i = 0; i < flash->total_size; i++) {
> You change the write chunk size from page_size to 1024. Does
> "programming page" still make sense, or should it be "programming chunk at"?
I made it just "programming at".

> > diff --git a/dmidecode b/dmidecode
> > new file mode 100755
> > index 0000000..e69de29
> Uhhh. Please make sure you don't commit dmidecode by accident.
Thanks for catching that. I swear to never use "add all" again.

> > +		.vendor		= "Intel",
> > +		.name		= "28F400BV/CV/CE-B",
> Should be ...CE-T unless I'm mistaken.
It should. I fixed it.

> A compile test and run test with -p dummy (which triggers the automatic
> chipdefinition selftest) would be appreciated before committing just to
> make sure I didn't overlook anything problematic.
Done. Selftest passed.

Regards,
  Michael Karcher

Patch

diff --git a/82802ab.c b/82802ab.c
index aa7e45e..0d1bd1a 100644
--- a/82802ab.c
+++ b/82802ab.c
@@ -48,6 +48,7 @@  int probe_82802ab(struct flashchip *flash)
 	chipaddr bios = flash->virtual_memory;
 	uint8_t id1, id2;
 	uint8_t flashcontent1, flashcontent2;
+	int shifted = (flash->feature_bits & FEATURE_ADDR_SHIFTED) != 0;
 
 	/* Reset to get a clean state */
 	chip_writeb(0xFF, bios);
@@ -57,8 +58,8 @@  int probe_82802ab(struct flashchip *flash)
 	chip_writeb(0x90, bios);
 	programmer_delay(10);
 
-	id1 = chip_readb(bios);
-	id2 = chip_readb(bios + 0x01);
+	id1 = chip_readb(bios + (0x00 << shifted));
+	id2 = chip_readb(bios + (0x01 << shifted));
 
 	/* Leave ID mode */
 	chip_writeb(0xFF, bios);
@@ -71,8 +72,8 @@  int probe_82802ab(struct flashchip *flash)
 		msg_cdbg(", id1 parity violation");
 
 	/* Read the product ID location again. We should now see normal flash contents. */
-	flashcontent1 = chip_readb(bios);
-	flashcontent2 = chip_readb(bios + 0x01);
+	flashcontent1 = chip_readb(bios + (0x00 << shifted));
+	flashcontent2 = chip_readb(bios + (0x01 << shifted));
 
 	if (id1 == flashcontent1)
 		msg_cdbg(", id1 is normal flash content");
@@ -178,44 +179,25 @@  void write_page_82802ab(chipaddr bios, uint8_t *src,
 int write_82802ab(struct flashchip *flash, uint8_t *buf)
 {
 	int i;
-	int total_size = flash->total_size * 1024;
-	int page_size = flash->page_size;
 	chipaddr bios = flash->virtual_memory;
-	uint8_t *tmpbuf = malloc(page_size);
 
-	if (!tmpbuf) {
-		msg_cerr("Could not allocate memory!\n");
-		exit(1);
+	if (erase_flash(flash)) {
+		msg_cerr("ERASE FAILED!\n");
+		return -1;
 	}
-	msg_cinfo("Programming page: \n");
-	for (i = 0; i < total_size / page_size; i++) {
-		msg_cinfo("\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");
-		msg_cinfo("%04d at address: 0x%08x", i, i * page_size);
-
-		/* Auto Skip Blocks, which already contain the desired data
-		 * Faster, because we only write, what has changed
-		 * More secure, because blocks, which are excluded
-		 * (with the exclude or layout feature)
-		 * or not erased and rewritten; their data is retained also in
-		 * sudden power off situations
-		 */
-		chip_readn(tmpbuf, bios + i * page_size, page_size);
-		if (!memcmp((void *)(buf + i * page_size), tmpbuf, page_size)) {
-			msg_cdbg("SKIPPED\n");
-			continue;
-		}
 
-		/* erase block by block and write block by block; this is the most secure way */
-		if (erase_block_82802ab(flash, i * page_size, page_size)) {
-			msg_cerr("ERASE FAILED!\n");
-			return -1;
-		}
-		write_page_82802ab(bios, buf + i * page_size,
-				   bios + i * page_size, page_size);
+	msg_cinfo("Programming page: ");
+	for (i = 0; i < flash->total_size; i++) {
+		if ((i & 0x3) == 0)
+			msg_cinfo("address: 0x%08lx", (unsigned long)i * 1024);
+
+                write_page_82802ab(bios, buf + i * 1024, bios + i * 1024, 1024);
+
+		if ((i & 0x3) == 0)
+			msg_cinfo("\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b");
 	}
-	msg_cinfo("DONE!\n");
-	free(tmpbuf);
 
+	msg_cinfo("DONE!\n");
 	return 0;
 }
 
diff --git a/dmidecode b/dmidecode
new file mode 100755
index 0000000..e69de29
diff --git a/flash.h b/flash.h
index aee95a3..0a74363 100644
--- a/flash.h
+++ b/flash.h
@@ -166,7 +166,7 @@  enum chipbustype {
 #define FEATURE_ADDR_MASK	(3 << 2)
 #define FEATURE_ADDR_2AA	(1 << 2)
 #define FEATURE_ADDR_AAA	(2 << 2)
-#define FEATURE_ADDR_SHIFTED	0
+#define FEATURE_ADDR_SHIFTED	(1 << 5)
 
 struct flashchip {
 	const char *vendor;
diff --git a/flashchips.c b/flashchips.c
index 2dbc1e0..a9b6551 100644
--- a/flashchips.c
+++ b/flashchips.c
@@ -2385,6 +2385,116 @@  struct flashchip flashchips[] = {
 
 	{
 		.vendor		= "Intel",
+		.name		= "28F004BV/BE-B",
+		.bustype	= CHIP_BUSTYPE_PARALLEL,
+		.manufacture_id	= INTEL_ID,
+		.model_id	= P28F004BB,
+		.total_size	= 512,
+		.page_size	= 128 * 1024, /* maximal block size */
+		.tested		= TEST_UNTESTED,
+		.probe		= probe_82802ab,
+		.probe_timing	= TIMING_ZERO,	/* Datasheet has no timing info specified */
+		.block_erasers	=
+		{
+			{
+				.eraseblocks = { 
+					{16 * 1024, 1},
+					{8 * 1024, 2},
+					{96 * 1024, 1},
+					{128 * 1024, 3},
+				},
+				.block_erase = erase_block_82802ab,
+			},
+		},
+		.write		= write_82802ab,
+		.read		= read_memmapped,
+	},
+
+	{
+		.vendor		= "Intel",
+		.name		= "28F004BV/BE-T",
+		.bustype	= CHIP_BUSTYPE_PARALLEL,
+		.manufacture_id	= INTEL_ID,
+		.model_id	= P28F004BT,
+		.total_size	= 512,
+		.page_size	= 128 * 1024, /* maximal block size */
+		.tested		= TEST_UNTESTED,
+		.probe		= probe_82802ab,
+		.probe_timing	= TIMING_ZERO,	/* Datasheet has no timing info specified */
+		.block_erasers	=
+		{
+			{
+				.eraseblocks = { 
+					{128 * 1024, 3},
+					{96 * 1024, 1},
+					{8 * 1024, 2},
+					{16 * 1024, 1},
+				},
+				.block_erase = erase_block_82802ab,
+			},
+		},
+		.write		= write_82802ab,
+		.read		= read_memmapped,
+	},
+
+	{
+		.vendor		= "Intel",
+		.name		= "28F400BV/CV/CE-B",
+		.bustype	= CHIP_BUSTYPE_PARALLEL,
+		.manufacture_id	= INTEL_ID,
+		.model_id	= P28F400BB,
+		.total_size	= 512,
+		.page_size	= 128 * 1024, /* maximal block size */
+		.feature_bits	= FEATURE_ADDR_SHIFTED,
+		.tested		= TEST_UNTESTED,
+		.probe		= probe_82802ab,
+		.probe_timing	= TIMING_ZERO,	/* Datasheet has no timing info specified */
+		.block_erasers	=
+		{
+			{
+				.eraseblocks = { 
+					{16 * 1024, 1},
+					{8 * 1024, 2},
+					{96 * 1024, 1},
+					{128 * 1024, 3},
+				},
+				.block_erase = erase_block_82802ab,
+			},
+		},
+		.write		= write_82802ab,
+		.read		= read_memmapped,
+	},
+
+	{
+		.vendor		= "Intel",
+		.name		= "28F400BV/CV/CE-B",
+		.bustype	= CHIP_BUSTYPE_PARALLEL,
+		.manufacture_id	= INTEL_ID,
+		.model_id	= P28F400BT,
+		.total_size	= 512,
+		.page_size	= 128 * 1024, /* maximal block size */
+		.feature_bits	= FEATURE_ADDR_SHIFTED,
+		.tested		= TEST_UNTESTED,
+		.probe		= probe_82802ab,
+		.probe_timing	= TIMING_ZERO,	/* Datasheet has no timing info specified */
+		.block_erasers	=
+		{
+			{
+				.eraseblocks = { 
+					{128 * 1024, 3},
+					{96 * 1024, 1},
+					{8 * 1024, 2},
+					{16 * 1024, 1},
+				},
+				.block_erase = erase_block_82802ab,
+			},
+		},
+		.write		= write_82802ab,
+		.read		= read_memmapped,
+	},
+
+	{
+		.vendor		= "Intel",
 		.name		= "82802AB",
 		.bustype	= CHIP_BUSTYPE_FWH,
 		.manufacture_id	= INTEL_ID,
diff --git a/flashchips.h b/flashchips.h
index 4337e52..b6c2e8b 100644
--- a/flashchips.h
+++ b/flashchips.h
@@ -259,6 +259,10 @@ 
 #define E_28F016S5		0xAA
 #define P28F001BXT		0x94	/* 28F001BX-T */
 #define P28F001BXB		0x95	/* 28F001BX-B */
+#define P28F004BT		0x78	/* 28F004BV/BE-T */
+#define P28F004BB		0x79	/* 28F004BV/BE-B */
+#define P28F400BT		0x70	/* 28F400BV/CV/CE-T */
+#define P28F400BB		0x71	/* 28F400BV/CV/CE-B */
 #define SHARP_LH28F008SA	0xA2	/* Sharp chip, Intel Vendor ID */
 #define SHARP_LH28F008SC	0xA6	/* Sharp chip, Intel Vendor ID */