Re: [flashrom] [PATCH] Support Intel 82F004BE/BV-T chip

[Sean Nelson]

On 1/16/2010 4:21 PM, Michael Karcher wrote:
> This chip is found in the ThinkPad T20. Implementation is from datasheet only,
> test welcome. Be prepared for eventual failures, and do not reboot if
> verify after write fails.
> Signed-off-by: Michael Karcher
>
> Makefile      |    2 +-
>   chipdrivers.h |    4 ++
>   flashchips.c  |   31 +++++++++++++++++++
>   flashchips.h  |    2 +
>   i28f00x.c     |   91 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   5 files changed, 129 insertions(+), 1 deletions(-)
>   create mode 100644 i28f00x.c
>
> diff --git a/Makefile b/Makefile
> index cdd45d1..2b882d4 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -44,7 +44,7 @@ endif
>   CHIP_OBJS = jedec.o stm50flw0x0x.o w39v080fa.o sharplhf00l04.o w29ee011.o \
>    sst28sf040.o am29f040b.o mx29f002.o m29f400bt.o pm29f002.o w39v040c.o \
>    w49f002u.o 82802ab.o pm49fl00x.o sst49lf040.o en29f002a.o m29f002.o \
> sst49lfxxxc.o sst_fwhub.o flashchips.o spi.o
>
> sst49lfxxxc.o sst_fwhub.o flashchips.o spi.o i28f00x.o
>
> LIB_OBJS = layout.o
>
> diff --git a/chipdrivers.h b/chipdrivers.h
> index e7261e0..be1d00b 100644
> --- a/chipdrivers.h
> +++ b/chipdrivers.h
> @@ -52,11 +52,15 @@ int spi_nbyte_read(int addr, uint8_t *bytes, int len);
>   int spi_read_chunked(struct flashchip *flash, uint8_t *buf, int start, int len, int chunksize);
>   int spi_aai_write(struct flashchip *flash, uint8_t *buf);
>
> +/* i28f00x.c */
> +int write_28f00x(struct flashchip *flash, uint8_t *buf);
> /* 82802ab.c */
> uint8_t wait_82802ab(chipaddr bios);
> int probe_82802ab(struct flashchip *flash);
> int erase_82802ab(struct flashchip *flash);
> int erase_82802ab_block(struct flashchip *flash, unsigned int page, unsigned int pagesize);
>
> +void write_page_82802ab(chipaddr bios, uint8_t *src, chipaddr dst, int page_size);
>   int write_82802ab(struct flashchip *flash, uint8_t *buf);
>
> /* am29f040b.c */
> diff --git a/flashchips.c b/flashchips.c
> index 8e90d2a..3698c8c 100644
> --- a/flashchips.c
> +++ b/flashchips.c
> @@ -2385,6 +2385,37 @@ struct flashchip flashchips[] = {
>
> {
>    	.vendor		= "Intel",
> .name		= "28F004BV/BE-T",
> .bustype	= CHIP_BUSTYPE_PARALLEL,
> .manufacture_id	= INTEL_ID,
> .model_id	= P28F004BT,
> .total_size	= 512,
> .page_size	= 128 * 1024, /* for write_i28f00x: maximal block size */
> .tested		= TEST_UNTESTED,
> .probe		= probe_jedec,
> .probe_timing	= TIMING_ZERO,	/* Datasheet has no timing info specified */
> .erase		= NULL,
> .block_erasers	=
> {
> 	{
> 		/* Keep this the first block_erasers
> 		   entry. The write functions spies here to
> 		   get the chip layout */
> 		.eraseblocks = {
> 			{128 * 1024, 3},
> 			{96 * 1024, 1},
> 			{8 * 1024, 2},
> 			{16 * 1024, 1},
> 		},
> 		.block_erase = erase_82802ab_block,
> 	},
> },
> .write		= write_28f00x,
> .read		= read_memmapped,
> },
> {
> .vendor		= "Intel",
> .name		= "82802AB",
> .bustype	= CHIP_BUSTYPE_FWH,
> .manufacture_id	= INTEL_ID,
>
> diff --git a/flashchips.h b/flashchips.h
> index 2a4d93b..21d7ad5 100644
> --- a/flashchips.h
> +++ b/flashchips.h
> @@ -256,6 +256,8 @@
>   #define I_82802AC		0xAC
>   #define P28F001BXT		0x94	/* 28F001BX-T */
>   #define P28F001BXB		0x95	/* 28F001BX-B */
> +#define P28F004BT		0x78	/* 28F004BV/BE-T */
> +#define P28F004BB		0x79	/* 28F004BV/BE-B */
>   #define SHARP_LH28F008SA	0xA2	/* Sharp chip, Intel Vendor ID */
>   #define SHARP_LH28F008SC	0xA6	/* Sharp chip, Intel Vendor ID */
>
> diff --git a/i28f00x.c b/i28f00x.c
> new file mode 100644
> index 0000000..f312fb1
> --- /dev/null
> +++ b/i28f00x.c
> @@ -0,0 +1,91 @@
> +/*
> This file is part of the flashrom project.
> Copyright (C) 2010 Michael Karcher
> This program is free software; you can redistribute it and/or modify
> it under the terms of the GNU General Public License as published by
> the Free Software Foundation; either version 2 of the License, or
> (at your option) any later version.
> This program is distributed in the hope that it will be useful,
> but WITHOUT ANY WARRANTY; without even the implied warranty of
> MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> GNU General Public License for more details.
> You should have received a copy of the GNU General Public License
> along with this program; if not, write to the Free Software
> Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 USA
> */
>
> +/* based on the Intel 82802A/B code which is
> Copyright (C) 2000 Silicon Integrated System Corporation
> */
>
> +/*
> Datasheet:
> Name: 4-Mbit (256K x 16, 512K x 8) SmartVoltage DataSheet
> URL: http://www.intel.com/design/archives/flash/docs/290530.HTM
> PDF: http://download.intel.com/design/archives/flash/docs/29053006.pdf
> Order number: 290430-006
> */
>
> +#include<stdio.h>
> +#include<stdlib.h>
> +#include<string.h>
> +#include "flash.h"
> +#include "chipdrivers.h"
>
> +int write_28f00x(struct flashchip *flash, uint8_t *buf)
> +{
> int i, j, blocknum;
> chipaddr bios = flash->virtual_memory;
> int blockoffset = 0;
> uint8_t *tmpbuf = malloc(flash->page_size);
> struct eraseblock * blocks = flash->block_erasers[0].eraseblocks;
> if (!tmpbuf) {
> printf("Could not allocate memory!\n");
> exit(1);
> }
> printf("Programming block: \n");
> blocknum = 0;
> for (i = 0; blocks[i].size != 0; i++) {
> for(j = 0; j<  blocks[i].count; j++) {
> 	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("%04d at address: 0x%08x", blocknum,
> 	       blockoffset);
> 	/* 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 + blockoffset, blocks[i].size);
> 	if (!memcmp((void *)(buf + blockoffset), tmpbuf,
> 	            blocks[i].size)) {
> 		printf("SKIPPED\n");
> 		continue;
> 	}
> 	/* erase block by block and write block by block;
> 	   this is the most secure way */
> 	if (erase_82802ab_block(flash, blockoffset,
> 	                        blocks[i].size)) {
> 		fprintf(stderr, "ERASE FAILED!\n");
> 		return -1;
> 	}
> 	write_page_82802ab(bios, buf + blockoffset,
> 	                   bios + blockoffset, blocks[i].size);
> 	blockoffset += blocks[i].size;
> 	blocknum++;
> }
> }
> printf("\n");
> free(tmpbuf);
> return 0;
>
> +}

It's great that you used block_erasers for the chip definition, but I 
don't understand the reason for write_28f00x. It looks exactly the same 
as write_82802 except for using erase blocks segments. i28f00x.c is 
completely unnecessary and extraneous in my opinion.

[PATCH] Support Intel 82F004BE/BV-T chip

[Michael Karcher]

Am Samstag, den 16.01.2010, 21:16 -0800 schrieb Sean Nelson:
> > +int write_28f00x(struct flashchip *flash, uint8_t *buf)
> > +{
> > int i, j, blocknum;
> > chipaddr bios = flash->virtual_memory;
> > int blockoffset = 0;
> > uint8_t *tmpbuf = malloc(flash->page_size);
> > struct eraseblock * blocks = flash->block_erasers[0].eraseblocks;
> > if (!tmpbuf) {
> > printf("Could not allocate memory!\n");
> > exit(1);
> > }
> > printf("Programming block: \n");
> > blocknum = 0;
> > for (i = 0; blocks[i].size != 0; i++) {
> > for(j = 0; j<  blocks[i].count; j++) {
> > 	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("%04d at address: 0x%08x", blocknum,
> > 	       blockoffset);
> > 	/* 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 + blockoffset, blocks[i].size);
> > 	if (!memcmp((void *)(buf + blockoffset), tmpbuf,
> > 	            blocks[i].size)) {
> > 		printf("SKIPPED\n");
> > 		continue;
> > 	}
> > 	/* erase block by block and write block by block;
> > 	   this is the most secure way */
> > 	if (erase_82802ab_block(flash, blockoffset,
> > 	                        blocks[i].size)) {
> > 		fprintf(stderr, "ERASE FAILED!\n");
> > 		return -1;
> > 	}
> > 	write_page_82802ab(bios, buf + blockoffset,
> > 	                   bios + blockoffset, blocks[i].size);
> > 	blockoffset += blocks[i].size;
> > 	blocknum++;
> > }
> > }
> > printf("\n");
> > free(tmpbuf);
> > return 0;
> >
> > +}
>
> It's great that you used block_erasers for the chip definition, but I 
> don't understand the reason for write_28f00x. It looks exactly the same 
> as write_82802 except for using erase blocks segments. i28f00x.c is 
> completely unnecessary and extraneous in my opinion.

write_82802ab only works if all erase blocks are of the same size,
namely the page_size specified in the chip. The 28f00x chips have erase
blocks of different sizes, the 82802A/B don't.

Regards,
  Michael Karcher

[PATCH] Support Intel 82F004BE/BV-T chip

[Sean Nelson]

I'm hesitant to ack this, considering we're working on eliminating chip 
drivers and will soon move to a block-based write system. Maybe you 
could add it to 82802ab and call it write_block_82802ab? or rewrite the 
write functions in 82802ab.c to be block based?

[PATCH] Support Intel 82F004BE/BV-T chip

[Carl-Daniel Hailfinger]

On 17.01.2010 10:25, Sean Nelson wrote:
> I'm hesitant to ack this, considering we're working on eliminating
> chip drivers and will soon move to a block-based write system. Maybe
> you could add it to 82802ab and call it write_block_82802ab? or
> rewrite the write functions in 82802ab.c to be block based?

I have a really small patch which does eraseblock walking and writes
each eraseblock after erasing it. It will be sent once Sean's patches
are reviewed and committed.

Basically, it is the last missing infrastructural piece to get partial
flashing (and rolling updates or however you call
erase-a-block-then-write-that-block).

Regards,
Carl-Daniel