Convert ST to block erasers: ST M25P05-A ST M25P05.RES ST M25P10-A ST M25P10.RES ST M25P20 ST M25P40 ST M25P40-old ST M25P80 ST M25P16 ST M25P32 ST M25P64 ST M25P128 ST M29F002B ST M29F002T/NT ST M29F040B ST M29F400BT ST M29W010B ST M29W040B ST M50FLW040A ST M50FLW040B ST M50FLW080A ST M50FLW080B ST M50FW002 ST M50FW016 ST M50FW040 ST M50FW080 ST M50LPW116
Add erase_chip_stm50flw0x0x to stm50flw0x0x.c Add copyright to stm50flw0x0x
Signed-off-by: Sean Nelson audiohacked@gmail.com
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 flashrom@mkarcher.dialup.fu-berlin.de for a version that passes the self-test.
The other issues need not be addressed before 0.9.2.
Regards, Michael Karcher
On 1/18/2010 6:14 AM, Michael Karcher wrote:
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.
I thought it better to keep continuity in the chip names for now.
--- Convert ST to block erasers: ST M25P05-A ST M25P05.RES ST M25P10-A ST M25P10.RES ST M25P20 ST M25P40 ST M25P40-old ST M25P80 ST M25P16 ST M25P32 ST M25P64 ST M25P128 ST M29F002B ST M29F002T/NT ST M29F040B ST M29F400BT ST M29W010B ST M29W040B ST M50FLW040A ST M50FLW040B ST M50FLW080A ST M50FLW080B ST M50FW002 ST M50FW016 ST M50FW040 ST M50FW080 ST M50LPW116
Add erase_chip_stm50flw0x0x to stm50flw0x0x.c Add copyright to stm50flw0x0x
Updated patch with fixes for block sizes and counts.
Signed-off-by: Sean Nelson audiohacked@gmail.com
Am Montag, den 18.01.2010, 10:48 -0800 schrieb Sean Nelson:
.erase = erase_stm50flw0x0x,
.erase = NULL,.block_erasers ={{.eraseblocks = {{4 * 1024, 16},{4 * 1024, 16},{64 * 1024, 5},{4 * 1024, 16},},.block_erase = erase_sector_stm50flw0x0x,}, {.eraseblocks = { {64 * 1024, 8}, },.block_erase = erase_block_stm50flw0x0x,}, {.eraseblocks = { {512 * 1024, 1} },.block_erase = erase_chip_stm50flw0x0x,}},
I don't think this is going to work. erase_sector_stm50flw0x0x only works on the 4K sectors and not on the 64k blocks inbetween, if I read the datasheet correctly. As I already wrote in the first review: flashrom is currently unable to represent this architecture (different block sizes that have to be delete by different commands), so you better leave it off. Another possible, but a bit hacky, version would be to check sectorsize in erwase_sector_stm50flw0x0x and call erase_block_stm50flw0x0x if sectorsize is 64*1024.
Regards, Michael Karcher
On 1/18/2010 2:48 PM, Michael Karcher wrote:
I don't think this is going to work. erase_sector_stm50flw0x0x only works on the 4K sectors and not on the 64k blocks inbetween, if I read the datasheet correctly. As I already wrote in the first review: flashrom is currently unable to represent this architecture (different block sizes that have to be delete by different commands), so you better leave it off. Another possible, but a bit hacky, version would be to check sectorsize in erwase_sector_stm50flw0x0x and call erase_block_stm50flw0x0x if sectorsize is 64*1024.
Regards, Michael Karcher
The datasheet doesn't state either way that it wouldn't work on the 5x 64k sectors between the 4k blocks, but I'm just going to remove the sector erasers and leave the function.
On 19.01.2010 00:21, Sean Nelson wrote:
On 1/18/2010 2:48 PM, Michael Karcher wrote:
I don't think this is going to work. erase_sector_stm50flw0x0x only works on the 4K sectors and not on the 64k blocks inbetween, if I read the datasheet correctly. As I already wrote in the first review: flashrom is currently unable to represent this architecture (different block sizes that have to be delete by different commands), so you better leave it off. Another possible, but a bit hacky, version would be to check sectorsize in erwase_sector_stm50flw0x0x and call erase_block_stm50flw0x0x if sectorsize is 64*1024.
Regards, Michael Karcher
The datasheet doesn't state either way that it wouldn't work on the 5x 64k sectors between the 4k blocks, but I'm just going to remove the sector erasers and leave the function.
Maybe add a comment in flashchips.c for that chip which says the structure can't be expressed right now.
Regards, Carl-Daniel
Convert ST to block erasers: ST M25P05-A ST M25P05.RES ST M25P10-A ST M25P10.RES ST M25P20 ST M25P40 ST M25P40-old ST M25P80 ST M25P16 ST M25P32 ST M25P64 ST M25P128 ST M29F002B ST M29F002T/NT ST M29F040B ST M29F400BT ST M29W010B ST M29W040B ST M50FLW040A ST M50FLW040B ST M50FLW080A ST M50FLW080B ST M50FW002 ST M50FW016 ST M50FW040 ST M50FW080 ST M50LPW116
Add erase_chip_stm50flw0x0x to stm50flw0x0x.c Add copyright to stm50flw0x0x.c Fix block sizes and counts Omit M50FLW0x0x mixed sector/block eraser
Signed-off-by: Sean Nelson audiohacked@gmail.com
I really need the reviews on the other patches, this patch depends on another patch for erase_82802ab_chip.
On 19.01.2010 00:42, Sean Nelson wrote:
I really need the reviews on the other patches, this patch depends on another patch for erase_82802ab_chip.
I'm on it. Can you name the other patch this depends on?
Regards, Carl-Daniel
Am Montag, den 18.01.2010, 15:31 -0800 schrieb Sean Nelson:
Signed-off-by: Sean Nelson audiohacked@gmail.com
Acked-by: Michael Karcher flashrom@mkarcher.dialup.fu-berlin.de
On 1/18/10 3:47 PM, Michael Karcher wrote:
Am Montag, den 18.01.2010, 15:31 -0800 schrieb Sean Nelson:
Signed-off-by: Sean Nelsonaudiohacked@gmail.com
Acked-by: Michael Karcherflashrom@mkarcher.dialup.fu-berlin.de
I'm going to do one last patch, some of the ST chips use 82802ab functions and I'm going to change them to stm50flw0x0x but keep probe_82802ab; They are functionally the same.
Convert ST to block erasers: ST M25P05-A ST M25P05.RES ST M25P10-A ST M25P10.RES ST M25P20 ST M25P40 ST M25P40-old ST M25P80 ST M25P16 ST M25P32 ST M25P64 ST M25P128 ST M29F002B ST M29F002T/NT ST M29F040B ST M29F400BT ST M29W010B ST M29W040B ST M50FLW040A ST M50FLW040B ST M50FLW080A ST M50FLW080B ST M50FW002 ST M50FW016 ST M50FW040 ST M50FW080 ST M50LPW116
Add erase_chip_stm50flw0x0x to stm50flw0x0x.c Add copyright to stm50flw0x0x.c Fix block sizes and counts Omit M50FLW0x0x mixed sector/block eraser Convert the used 82802ab functions to their stm50flw0x0x equivalents
Signed-off-by: Sean Nelson audiohacked@gmail.com
On 19.01.2010 01:10, Sean Nelson wrote:
On 1/18/10 3:47 PM, Michael Karcher wrote:
Am Montag, den 18.01.2010, 15:31 -0800 schrieb Sean Nelson:
Signed-off-by: Sean Nelson audiohacked@gmail.com
Acked-by: Michael Karcher flashrom@mkarcher.dialup.fu-berlin.de
I'm going to do one last patch, some of the ST chips use 82802ab functions and I'm going to change them to stm50flw0x0x but keep probe_82802ab; They are functionally the same.
Convert ST to block erasers: ST M25P05-A ST M25P05.RES ST M25P10-A ST M25P10.RES ST M25P20 ST M25P40 ST M25P40-old ST M25P80 ST M25P16 ST M25P32 ST M25P64 ST M25P128 ST M29F002B ST M29F002T/NT ST M29F040B ST M29F400BT ST M29W010B ST M29W040B ST M50FLW040A ST M50FLW040B ST M50FLW080A ST M50FLW080B ST M50FW002 ST M50FW016 ST M50FW040 ST M50FW080 ST M50LPW116
Add erase_chip_stm50flw0x0x to stm50flw0x0x.c Add copyright to stm50flw0x0x.c Fix block sizes and counts Omit M50FLW0x0x mixed sector/block eraser Convert the used 82802ab functions to their stm50flw0x0x equivalents
Signed-off-by: Sean Nelson audiohacked@gmail.com
ERROR: Flash chip M25P128 erase function 0 region walking resulted in 0x2000000 bytes total, expected 0x1000000 bytes. Please report a bug at flashrom@flashrom.org ERROR: Flash chip M50FW002 erase function 1 region walking resulted in 0x100000 bytes total, expected 0x040000 bytes. Please report a bug at flashrom@flashrom.org
Regards, Carl-Daniel
On 19.01.2010 03:29, Carl-Daniel Hailfinger wrote:
On 19.01.2010 01:10, Sean Nelson wrote:
I'm going to do one last patch, some of the ST chips use 82802ab functions and I'm going to change them to stm50flw0x0x but keep probe_82802ab; They are functionally the same.
Convert ST to block erasers: ST M25Px ST M29Fx ST M29Wx ST M50FLWx ST M50FWx ST M50LPW116
Signed-off-by: Sean Nelson audiohacked@gmail.com
ERROR: Flash chip M25P128 erase function 0 region walking resulted in 0x2000000 bytes total, expected 0x1000000 bytes. Please report a bug at flashrom@flashrom.org ERROR: Flash chip M50FW002 erase function 1 region walking resulted in 0x100000 bytes total, expected 0x040000 bytes. Please report a bug at flashrom@flashrom.org
I might add that reviewing this one was more difficult because it used standard 3 lines of context and it was not obvious to which chips the patch belonged.
Regards, Carl-Daniel
On 1/18/10 6:34 PM, Carl-Daniel Hailfinger wrote:
I might add that reviewing this one was more difficult because it used standard 3 lines of context and it was not obvious to which chips the patch belonged.
Sorry, about that! I really need to fix my git config to use 7 or 9 lines of context.
Convert ST to block erasers: ST M25P05-A ST M25P05.RES ST M25P10-A ST M25P10.RES ST M25P20 ST M25P40 ST M25P40-old ST M25P80 ST M25P16 ST M25P32 ST M25P64 ST M25P128 ST M29F002B ST M29F002T/NT ST M29F040B ST M29F400BT ST M29W010B ST M29W040B ST M50FLW040A ST M50FLW040B ST M50FLW080A ST M50FLW080B ST M50FW002 ST M50FW016 ST M50FW040 ST M50FW080 ST M50LPW116
Add erase_chip_stm50flw0x0x to stm50flw0x0x.c Add copyright to stm50flw0x0x.c Fix block sizes and counts Omit M50FLW0x0x mixed sector/block eraser Convert the used 82802ab functions to their stm50flw0x0x equivalents Fix incorrect sizes as found by Carl-Daniel.
Signed-off-by: Sean Nelson audiohacked@gmail.com
Am Montag, den 18.01.2010, 15:21 -0800 schrieb Sean Nelson:
On 1/18/2010 2:48 PM, Michael Karcher wrote:
I don't think this is going to work. erase_sector_stm50flw0x0x only works on the 4K sectors and not on the 64k blocks inbetween, if I read the datasheet correctly.
The datasheet doesn't state either way that it wouldn't work on the 5x 64k sectors between the 4k blocks, but I'm just going to remove the sector erasers and leave the function.
If you take a look at appendix A, you will find that there are block numbers for all of the parts of the flash chip, but sector numbers only for the sectored parts. I interpreted it in the way that the other parts do not have sectors, thus making "sector erase" ill defined. Your approach seems sound.
Regards, Michael Karcher
This should be the last-last revision of this patch.
Convert ST to block erasers: ST M25P05-A ST M25P05.RES ST M25P10-A ST M25P10.RES ST M25P20 ST M25P40 ST M25P40-old ST M25P80 ST M25P16 ST M25P32 ST M25P64 ST M25P128 ST M29F002B ST M29F002T/NT ST M29F040B ST M29F400BT ST M29W010B ST M29W040B ST M50FLW040A ST M50FLW040B ST M50FLW080A ST M50FLW080B ST M50FW002 ST M50FW016 ST M50FW040 ST M50FW080 ST M50LPW116
Add erase_chip_stm50flw0x0x to stm50flw0x0x.c Add copyright to stm50flw0x0x.c Fix block sizes and counts Omit M50FLW0x0x mixed sector/block eraser Convert the used 82802ab functions to their stm50flw0x0x equivalents Fix incorrect sizes as found by Carl-Daniel. Add back M50FLW0x0x mixed sector/block eraser sans function pointer.
Signed-off-by: Sean Nelson audiohacked@gmail.com
On 19.01.2010 09:01, Sean Nelson wrote:
This should be the last-last revision of this patch.
Convert ST to block erasers: ST M25P05-A ST M25P05.RES ST M25P10-A ST M25P10.RES ST M25P20 ST M25P40 ST M25P40-old ST M25P80 ST M25P16 ST M25P32 ST M25P64 ST M25P128 ST M29F002B ST M29F002T/NT ST M29F040B ST M29F400BT ST M29W010B ST M29W040B ST M50FLW040A ST M50FLW040B ST M50FLW080A ST M50FLW080B ST M50FW002 ST M50FW016 ST M50FW040 ST M50FW080 ST M50LPW116
Add erase_chip_stm50flw0x0x to stm50flw0x0x.c Add copyright to stm50flw0x0x.c Fix block sizes and counts Omit M50FLW0x0x mixed sector/block eraser Convert the used 82802ab functions to their stm50flw0x0x equivalents Fix incorrect sizes as found by Carl-Daniel. Add back M50FLW0x0x mixed sector/block eraser sans function pointer.
Signed-off-by: Sean Nelson audiohacked@gmail.com
Acked-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
{ .vendor = "ST", .name = "M50FLW040A", .bustype = CHIP_BUSTYPE_FWH|CHIP_BUSTYPE_LPC, /* A/A Mux */ .manufacture_id = ST_ID, .model_id = ST_M50FLW040A, .total_size = 512, .page_size = 64 * 1024, .feature_bits = FEATURE_REGISTERMAP, .tested = TEST_UNTESTED, .probe = probe_jedec, .probe_timing = TIMING_FIXME,
.erase = erase_stm50flw0x0x,
.erase = NULL,.block_erasers ={{.eraseblocks = {{4 * 1024, 16}, /* sector */{64 * 1024, 5}, /* block */{4 * 1024, 16}, /* sector */{4 * 1024, 16}, /* sector */},.block_erase = NULL,
Not exactly what I had in mind (I thought about specifying the function and not the blocks), but flashrom can deal with this as well.
It might be a good idea to change the line
{64 * 1024, 5},
to
{320 * 1024, -1},
once flashrom can deal with negative sizes and interpret them as "function does not apply to this area". Or we create a special erase function which does different things based on eraseblock size/location.
Your code is OK right now (please don't follow my suggestion right now), and I just wanted to discuss further development options.
}, {.eraseblocks = { {64 * 1024, 8}, },.block_erase = erase_block_stm50flw0x0x,}, {.eraseblocks = { {512 * 1024, 1} },.block_erase = erase_chip_stm50flw0x0x,}},
Regards, Carl-Daniel
Am Dienstag, den 19.01.2010, 11:36 +0100 schrieb Carl-Daniel Hailfinger:
.eraseblocks = {{4 * 1024, 16}, /* sector */{64 * 1024, 5}, /* block */{4 * 1024, 16}, /* sector */{4 * 1024, 16}, /* sector */},.block_erase = NULL,
It might be a good idea to change the line
{64 * 1024, 5},
to
{320 * 1024, -1},
once flashrom can deal with negative sizes and interpret them as "function does not apply to this area".
I really like that idea, although it may make block walking harder. The idea is to have the walking code centralized, so it shouldn't matter that much.
On the other hand, things get messy if you have partly overlapping erase blocks in different erase functions, but I think this is just a "don't do it then" issue. I can't imagine any sane flash chip that would have partly overlapping erase blocks.
Or we create a special erase function which does different things based on eraseblock size/location.
And don't like this one, as these functions probably get chip specific, and currently we are heading to kill chip specific functions, don't we?
Regards, Michael Karcher
On 19.01.2010 11:45, Michael Karcher wrote:
Am Dienstag, den 19.01.2010, 11:36 +0100 schrieb Carl-Daniel Hailfinger:
.eraseblocks = {{4 * 1024, 16}, /* sector */{64 * 1024, 5}, /* block */{4 * 1024, 16}, /* sector */{4 * 1024, 16}, /* sector */},.block_erase = NULL,It might be a good idea to change the line
{64 * 1024, 5},
to
{320 * 1024, -1},
once flashrom can deal with negative sizes and interpret them as "function does not apply to this area".
I really like that idea, although it may make block walking harder. The idea is to have the walking code centralized, so it shouldn't matter that much.
Yes, my goal is to have an erase_range function which gets start and length as parameters and picks an erase function which can do that, or tells the caller that it the only possible erase is bigger than the requested erase. That erase_range function would do the block walking in a manner similar to what we have now.
On the other hand, things get messy if you have partly overlapping erase blocks in different erase functions, but I think this is just a "don't do it then" issue. I can't imagine any sane flash chip that would have partly overlapping erase blocks.
Heh. Sanity in flash chips? I want to go to that magic place. ;-) But yes, we can simply refuse to handle such stuff if it ever arises.
Or we create a special erase function which does different things based on eraseblock size/location.
And don't like this one, as these functions probably get chip specific, and currently we are heading to kill chip specific functions, don't we?
Indeed.
Regards, Carl-Daniel
On 1/19/2010 2:36 AM, Carl-Daniel Hailfinger wrote:
Acked-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Thanks, Committed in r872.
-
Carl-Daniel Hailfinger -
Michael Karcher -
Sean Nelson