On Sat, 27 Aug 2011 04:28:28 +0100
Andrew Morgan <[email protected]> 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 <[email protected]>
> 

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 <[email protected]>

do you have commit rights?
-- 
Kind regards/Mit freundlichen Grüßen, Stefan Tauner

_______________________________________________
flashrom mailing list
[email protected]
http://www.flashrom.org/mailman/listinfo/flashrom

Reply via email to