On 20.08.2009 16:44, Marko Kraljevic wrote:
Carl-Daniel Hailfinger wrote:
Instead of a full review, I just wrote all this stuff in a special wiki page so others can benefit from this as well: http://www.coreboot.org/Flashrom/Random_notes . Please read it and comment if something is unclear.
thanks :)
You're welcome. If there are further questions, I'd like to add the answers to the wiki as well. Saves me explaining that each time someone asks ;-)
Ah, I hadn't realized that the high byte was ignored, that makes a big difference! If that's the case, then 29f002.c functions should work fine. I assume there is some difference between the 29f002 functions and the JEDEC ones, or else there would only be the JEDEC file. I'll look them both over this evening, and try flashing, and report back.
To be honest, in the early days of flashrom development, people just copied stuff around and renamed it. That's why we still have loads of duplicated code. From time to time, I go through the tree and clean up the worst offenders... But the 29f002 stuff might as well be a copy of jedec or some other functions and nobody had the time to convert usage to jedec.
The reset command you've added at the beginning of erase_29f001() makes sense, but AFAICS it would make sense in the original erase_29f002() function as well, so please change that one and use it afterwards. The resent command I'm talking about is chip_writeb(0xF0, bios + 0x555);
I believe that was in the original file ['f002.c], and it's just commented out in my modified version ['f001.c]. If it is there [as in mx29f002.c], should there not be a small delay between the reset, and writing the erase commands?
Ah ok, I misread that. Basically, it makes sense to run reset between probing and any other command because probing for other chips may confuse your chip enough to warrant a reset.
Either way, erase seems to function fine without the reset.
I guess I got a little too excited. I think I have a few more unlisted chips around here, I'll try and play with them in the future, and I'll make sure to check if existing functions will support them!
Hey, excitement is good. You went the obvious route and you even tested everything. That's absolutely fine. And you caused me to write up some useful stuff about flashrom development. That's even better.
In the case where you have a file written for one chip - in this case mx29f002.c - and it is found to support multiple chips in the future, is it a good idea to rename it to something more descriptive? Perhaps in this case, it would be called mx29f00x.c ?
Renaming can be done, but we didn't do that often. We use grep to look for matching sequences... maybe not the best method, but it works mostly.
Or is it better to keep the original name, do you think?
We want to release flashrom 0.9.1 today or tomorrow. I'd rather keep the name until 0.9.1 is released. 0.9.1 is done and we're only waiting for testers. I guess including a few additional chips won't be a problem if we can finish that patch today ;-)
Regards, Carl-Daniel