On 22.12.2009 08:16, Sean Nelson wrote:
After asking if page_size was equivalent to a chip's sector size, while I was verifying datasheets and our flashchip support table, Carl-Daniel suggested I convert all chips to use struct eraseblock. Starting from the top of the file and working down, this is the first of many patches.
Signed-off-by: Sean Nelson audiohacked@gmail.com
Thanks for the patch!
A few comments: Some chips had erase_chip_jedec as erase function (AM_29F002BB, AM_29F002BT) but it seems you dropped calls to that function completely. Does the datasheet forbid chip erase or is this a problem with mismatched prototypes? If you need an example of how a wrapper can be created, look at spi.c:spi_block_erase_60() which performs exactly that task with error checking.
Some chips have a status where erase is marked as tested (TEST_OK_PREW etc.) and this should be adapted to read TEST_OK_PRW or somesuch. Simply kill the E.
The code does not compile because of the following reasons: erase_29f040b() and erase_sector_29f040b() do not match the expected prototype: int (*block_erase) (struct flashchip *flash, unsigned int blockaddr, unsigned int blocklen); You'll have to edit am29f040b.c and change the signature of these functions. You'll have to add an eraseblock list to AMIC_A29040B, PMC_29F002T, Pm29F0002B if you change the function signature of erase_sector_29f040b(). erase_sector_29f040b() is not declared in chipdrivers.h. erase_sector_29f040b() is static (remove the static keyword). You forgot the comma at the end of some eraseblock definitions.
Now just one comment about cosmetics: Could you write {16 * 1024, 1}, instead of {16*1024, 1}, and if you have block sizes of e.g. 8192M, write them as 8 * 1024 * 1024 (basically having a max factor of 1024). The spaces between number and operator are standard flashrom coding style and I do care about them. However, the limit on 1024 for numbers is pretty much arbitrary and if you want to ignore it, feel free to do so.