On Tue, 12 Apr 2011 15:55:58 -0400 Zeus Castro thezeusjuice@gmail.com wrote:
Added support for SST25LF080A flash chip, based on public datasheet available here: http://www.sst.com/dotAsset/40316.pdf
hello <awesomely named guy> :)
disclamer: this is my first patch review for flashrom.
Index: flashchips.h
-#define SST_SST25VF080_REMS 0x80 /* REMS or RES opcode */ +#define SST_SST25VF080_REMS 0x80 /* REMS or RES opcode, same as SST25LF080A */
since the SST25VF080 is not implemented and that define is not used at all as far as i (and grep) can see, there is no reason to use the rems suffix imho.
Index: flashchips.c
the whole block should be moved up (with the following SST25LF040A chip too). it is not alphabetically sorted.
=================================================================== --- flashchips.c (revision 1285) +++ flashchips.c (working copy) @@ -5342,6 +5342,35 @@
{ .vendor = "SST",
.name = "SST25LF080A.RES",
i dont see a reason why you are using .RES here. according to the datasheet REMS work as well as RES. but maybe i dont know some detail.
.bustype = CHIP_BUSTYPE_SPI,.manufacture_id = SST_ID,.model_id = SST_SST25VF080_REMS,.total_size = 1024,
ok
.page_size = 256,
ok, because page_size on the whole is fucked up. :)
.tested = TEST_UNTESTED,
did you really not test it? why have you added it?
.probe = probe_spi_res2,
ok, or probe_spi_rems
.probe_timing = TIMING_ZERO,.block_erasers ={{.eraseblocks = { {4 * 1024, 256} },.block_erase = spi_block_erase_20,}, {.eraseblocks = { {32 * 1024, 32} },.block_erase = spi_block_erase_52,}, {.eraseblocks = { {1024 * 1024, 1} },.block_erase = spi_block_erase_60,},},
correct
.unlock = spi_disable_blockprotect,
this will not work if bit 7 (BPL) is set some of the at25* write protections are similar. one might use them.
.write = spi_chip_write_1,.read = spi_chip_read,- },
- {
.vendor = "SST",
ok