[flashrom] [PATCH, v2] Intel 28F004/28F400 support

[Carl-Daniel Hailfinger]

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 at mkarcher.dialup.fu-berlin.de>
>   

With the comments below addressed, this is
Acked-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at 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

-- 
http://www.hailfinger.org/