Carl-Daniel Hailfinger wrote:
On 17.11.2008 14:28, Stefan Reinauer wrote:
I tried factoring it out, but since we don't want to die() completely, we have to return with an error code from this function. That pretty much rules out factoring out the checks into a separate function. I could move all checks one level down in the call graph and clutter up only the end of each function. What do you think?
I guess your current version is fine then
+int spi_chip_erase_60_c7(struct flashchip *flash) +{
- int result;
- result = spi_chip_erase_60(flash);
- if (result) {
printf_debug("spi_chip_erase_60 failed, trying c7\n");
result = spi_chip_erase_c7(flash);
- }
- return result;
+}
I don't particularly like this. Maybe we should have spi_chip_erase() try all different erase functions in a row, and check whether the erase was actually successfull (all bytes 0xff)?
That's a bit difficult. There are some chips which have either the 0x60 or the 0xc7 opcode used for something else, so we need to keep this chip specific.
Hm.. So do we need to keep an array of supported read, write, erase, id opcodes for each spi chip to handle this? That way we could check the chip's capabilities and compare to the OPMENUs capabilities.
That one function does one job, but I'm a little concerned that we end up writing functions for all possible variations of possible commands and have a hard time tracking it afterwards.
Stefan