Re: [flashrom] New flash chips, AT49F010 and CAT28F512

[Stefan Tauner]

On Sat, 27 Aug 2011 04:28:28 +0100
Andrew Morgan  wrote:
> AT49F010 patch is basically a copy of the existing AT49F020 code, but 
> with half the size and the correct ID.
> The log was from a write of random data, after the chip already 
> contained different random data. I did separate read, erase, write tests 
> before but this log seems to show that PREW all work.
>
> CAT28F512 I have successfully probed and read data from, but erase 
> fails. Could be because it needs 12v VPP. The CAT28F512 I have is 
> soldered on to a NIC. I haven't investigated if VPP is connected at all 
> yet. Thought I'd include the patch as-is anyway.
>
> Both patches:
> Signed-off-by: Andrew Morgan

hello andrew and thanks for the patches!

btw patch_es_: please post one per mail in the future, because
patchwork seems to be too dumb to recognize multiple patches in one
mail (http://patchwork.coreboot.org/patch/3395/).

review of the AT49F010 patch follows:
> Index: flashchips.c
>
> --- flashchips.c	(revision 1422)
> +++ flashchips.c	(working copy)
> @@ -2218,6 +2218,30 @@
>
> {
>    	.vendor		= "Atmel",
> .name		= "AT49F010",

.name		= "AT49(H)F010",
The H version is just a faster version (but with same VCC).
> .bustype	= BUS_PARALLEL,
> .manufacture_id	= ATMEL_ID,
> .model_id	= ATMEL_AT49F010,
> .total_size	= 128,
> .page_size	= 128,

should probably be 256 for now (semantics are different for each chip
driver for parallel chips afaik and it does not matter for jedec
routines iirc. NB: page_size is in bytes, so syncing it with total_size
does not make sense.)...
> .feature_bits	= FEATURE_EITHER_RESET,
> .tested		= TEST_OK_PREW,
> .probe		= probe_jedec,
> .probe_timing	= TIMING_ZERO,	/* Datasheet has no timing info specified */
> .block_erasers	=
> {
> 	{
> 		.eraseblocks = { {128 * 1024, 1} },
> 		.block_erase = erase_chip_block_jedec,
> 	}
> },
> .write		= write_jedec_1,
> .read		= read_memmapped,
> .voltage	= {4500, 5500},
> },
> {
> .vendor		= "Atmel",
> .name		= "AT49F020",
> .bustype	= BUS_PARALLEL,
> .manufacture_id	= ATMEL_ID,
> Index: flashchips.h
>
> --- flashchips.h	(revision 1422)
> +++ flashchips.h	(working copy)
> @@ -181,6 +181,7 @@
>  #define ATMEL_AT45DB642		/* No ID available */
>  #define ATMEL_AT45DB642D	0x2800
>  #define ATMEL_AT49BV512		0x03
> +#define ATMEL_AT49F010		0x17	/* Also AT49HF010 */

correct id, but the comment should be /* Same as AT49HF010 */ to be consistent.
> #define ATMEL_AT49F020		0x0B
>  #define ATMEL_AT49F002N		0x07	/* for AT49F002(N)  */
>  #define ATMEL_AT49F002NT		0x08	/* for AT49F002(N)T */

this chip and also the 2 Mb and 4 Mb versions support a boot block
protection that can be detected by software. it would be nice to add
a .printlock function to do this and inform the user. i have seen the
scheme before. Maybe there is already code in flashrom... but it is not
that important.
Adding the 4 Mb version OTOH is trivial (ID 0x13) and would be
appreciated. Nevertheless after addressing the in-line comments this
patch is:
Acked-by: Stefan Tauner

do you have commit rights?