[flashrom] Support for MX29F001T/B

[Marko Kraljevic]

Carl-Daniel Hailfinger wrote:

> 
> The following guidelines are for coreboot, but most of them apply to
> flashrom as well: http://www.coreboot.org/Development_Guidelines
> The really important part is about the Signed-off-by procedure.
> 

Sounds good, I'll read through that.


> We try to reuse as much code as possible and create new files only if
> absolutely needed, so if you find a function somewhere in the tree which
> already does what you want (even if it is for a totally different chip),
> please use it.
> 
> 

Excellent, I was thinking the same thing. No sense in having to maintain 
redundant code.


> Looks good. And for a first patch, you managed to not only write the
> code, but also test it. Some hints about proper submission procedure are
> mentioned above.
> 
> 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 :)


> As you can see, there are quite a lot of probe functions which seem to
> work fine (and that's mostly because of the ignored address bits).
> probe_jedec is the most-used function in our tree, so if the sequence
> looks ok, please use that one.
> 

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.


> 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?
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!

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 ?
Or is it better to keep the original name, do you think?

Mark