Re: [flashrom] [PATCH] add support for AT49F040

[Carl-Daniel Hailfinger]

Hi David,

you somehow manage to find really evil chips.

On 31.08.2010 23:19, David Borg wrote:
> I was adding support for the AT49F040 parallel flash chip, but have
> some questions. This chip has a  boot block which once locked cannot
> be erased / rewritten.

Yes, permanent lockdown sucks.
> Should I add erase functions for both cases?

I'd say yes, but currently flashrom can't specify erase functions which
work only for a part of the chip.
Maybe add a comment to flashchips.c that there is an
erase-without-bootblock which only applies to chips with locked bootblock.
> Currently the erase function handles unlocked chips only.

There is the possibility of adding a printlock function which would be
extremely hopeful here. While it won't solve the lockdown problem, it
can at least make users aware of the lockdown.
> In addition,
> the datasheet specifies no command timings, is the timing something I
> can infer from the read / program cycle characteristics tables?

No. I'd say this is a chip with zero delay (that is, zero additional delay).
> Signed-off-by: David Borg
>
> datasheet: http://pdf1.alldatasheet.com/datasheet-pdf/view/56184/ATMEL/AT49F040.html
> Index: flashchips.c
>
> --- flashchips.c	(revision 1145)
> +++ flashchips.c	(working copy)
> @@ -1985,6 +1985,29 @@
>    },
>
> {
> .vendor		= "Atmel",
> .name		= "AT49F040",
> .bustype	= CHIP_BUSTYPE_PARALLEL,
> .manufacture_id	= ATMEL_ID,
> .model_id	= AT_49F040,
> .total_size	= 512,
> .page_size	= 512,
> .feature_bits	= FEATURE_EITHER_RESET | FEATURE_ADDR_FULL,
> .tested		= TEST_UNTESTED,
> .probe		= probe_jedec,
> .probe_timing	= TIMING_ZERO,  /* Datasheet has no timing info specified */
> .block_erasers	=
> {
> 	{
> 		.eraseblocks = { {512 * 1024, 1} },
> 		.block_erase = erase_chip_block_jedec,
> 	}

Comment about the erase-without-bootblock goes here.
> },
> .write		= write_jedec_1,
> .read		= read_memmapped,
> },
> {
> .vendor		= "EMST",
> .name		= "F49B002UA",
> .bustype	= CHIP_BUSTYPE_PARALLEL,
> Index: flashchips.h
>
> --- flashchips.h	(revision 1145)
> +++ flashchips.h	(working copy)
> @@ -174,6 +174,7 @@
>  #define AT_49F020		0x0B
>  #define AT_49F002N		0x07	/* for AT49F002(N)  */
>  #define AT_49F002NT		0x08	/* for AT49F002(N)T */
> +#define AT_49F040		0x13

I think this part no longer applies to the tree.
> #define CATALYST_ID		0x31	/* Catalyst */

Looks good, we should get this merged with the bootblock lock printing
and a comment in flashchips.c. Will ack after that change.

Regards,
Carl-Daniel