Re: [flashrom] [PATCH] Convert chips to block erasers: ST

[Michael Karcher]

Am Sonntag, den 17.01.2010, 23:01 -0800 schrieb Sean Nelson:
> @@ -4155,15 +4265,25 @@ struct flashchip flashchips[] = {
>                 .manufacture_id = ST_ID,
>                 .model_id       = ST_M25P128,
>                 .total_size     = 16384,
>                 .page_size      = 256,
>                 .tested         = TEST_UNTESTED,
>                 .probe          = probe_spi_rdid,
>                 .probe_timing   = TIMING_ZERO,
>           .erase          = spi_chip_erase_c7,
>           .erase          = NULL,
>           .block_erasers  =
>           {
>                   {
>                           .eraseblocks = { {256 * 1024, 128} },
>                           .block_erase = spi_block_erase_d8,
>                   }, {
>                           .eraseblocks = { {8 * 1024 * 1024, 1} },
>                           .block_erase = spi_block_erase_c7,
>                   }
>           },
>           .write          = spi_chip_write_256,
>           .read           = spi_chip_read,
>   },

Total chip size is wrong. Carl-Daniels self-check should catch that one, too
> @@ -4171,47 +4291,87 @@ struct flashchip flashchips[] = {
>                 .manufacture_id = ST_ID,
>                 .model_id       = ST_M29F002B,
>                 .total_size     = 256,
>                 .page_size      = 64 * 1024,
>                 .tested         = TEST_UNTESTED,
>                 .probe          = probe_jedec,
>                 .probe_timing   = TIMING_ZERO,  /* Datasheet has no timing info specified */
>           .erase          = erase_m29f002,
>           .erase          = NULL,
>           .block_erasers  =
>           {
>                   {
>                           .eraseblocks = {
>                                   {16 * 1024, 1},
>                                   {8 * 1024, 2},
>                                   {32 * 1024, 1},
>                                   {64 * 1024, 3},
>                           },
>                           .block_erase = erase_sector_jedec,
>                   }, {
>                           .eraseblocks = { {8 * 1024 * 1024, 1} },
>                           .block_erase = erase_chip_block_jedec,
>                   }
>           },

Total chip size is wrong. Should be 256 * 1024. The self-test should
catch it.
>     {
>             .vendor         = "ST",
>             .name           = "M29F002T/NT",

[...]
>           .block_erasers  =
>           {
>                   {
>                           .eraseblocks = {
>                                   {64 * 1024, 3},
>                                   {32 * 1024, 1},
>                                   {8 * 1024, 2},
>                                   {16 * 1024, 1},
>                           },
>                           .block_erase = erase_sector_jedec,
>                   }, {
>                           .eraseblocks = { {8 * 1024 * 1024, 1} },
>                           .block_erase = erase_chip_block_jedec,
>                   }
>           },

Same here.
>     {
>             .vendor         = "ST",
>             .name           = "M50FLW040A",

[...]
>           .erase          = NULL,
>           .block_erasers  =
>           {
>                   {
>                           .eraseblocks = { {64 * 1024, 8}, },
>                           .block_erase =
>
> erase_block_stm50flw0x0x,
>                   }, {
>                           .eraseblocks = { {512 * 1024, 1} },
>                           .block_erase =
>
> erase_chip_stm50flw0x0x,
>                   }
>           },
>           .write          = write_stm50flw0x0x,
>           .read           = read_memmapped,
>   },

This chip has subdivided blocks at the top/bottom (different for A and
B). The subdivided blocks can be erased by block_erase, but the single
subdivisions could be erased on its own with sector erase commands.
Alas, flashrom doesn't have the infrastructure to use different commands
in one block_erasers choice. One could work around with a block_erase
function that checks the size - but this function would have to be
special-cased for this chip again.

For this patch: Keep it as is.
> @@ -4285,32 +4490,52 @@ struct flashchip flashchips[] = {
>                 .model_id       = ST_M50FLW040B,

same here.
>             .model_id       = ST_M50FLW080A,

same here.
>             .model_id       = ST_M50FLW080B,

and here.
>                           .block_erase = erase_82802ab_block,

Ah, BTW, what is the reason to have erase_block_stm50flw0x0x and
erase_82802ab_block? They seem to be redundant. Cleanup is left for
another patch, though.
>     {
>             .vendor         = "ST",
>             .name           = "M50LPW116",
>
> @@ -4399,15 +4679,31 @@ struct flashchip flashchips[] = {

[...]
>                           .eraseblocks = {
>                                   {4 * 1024, 16},
>                                   {64 * 1024, 29},
>                                   {32 * 1024, 1},
>                                   {8 * 1024, 2},
>                                   {16 * 1024, 1},
>                           },

You should have 31, not 29 64k Blocks. The automated checker will find
it.

All errors found will be auto-detected by the checking infrastructure
now in flashrom, so I already send you an
Acked-by: Michael Karcher 
for a version that passes the self-test.

The other issues need not be addressed before 0.9.2.

Regards,
  Michael Karcher