Hi Carl-Daniel,
thanks a lot for bringing this up..
* Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net [120504 00:16]:
coreboot has a requirement to store some data in flash to get some chipsets to work well. There are multiple ways to achieve this:
- Write flashing code from scratch. Pointless.
That's what has been done for the AMD SB800... See src/southbridge/amd/cimx/sb800/spi.c for more information. That driver is very small, but will also only work for one known good combination of chipset + flashchip. http://review.coreboot.org/997 was done with more flexibility in mind; also regarding certain flash usage constraints. For example on the Samsung ChromeBook coreboot's CBFS lives completely in the read-only section of the flash chip for security reasons.
- Use parts of the flashrom codebase or libflashrom. Advantages:
coreboot and flashrom developers often work on both projects, enabling better cooperation. flashrom has the best chipset/flash support out there.
There is no doubt that flashrom is the most flexible and universal implementation out there. It's also almost 40k lines of code, whereas the current implementation covering many flash chips and ICH support fits in only 3000 lines. It may be hard (or not?) to keep the goals of flashrom (flexibility) and coreboot (small, simple) under one hood.
- Use parts of the U-Boot codebase (which has drivers derived from
flashrom). Advantages: The codebase is designed with firmware constraints in mind. http://review.coreboot.org/997 seems to implement this.
As a flashrom developer, I would like to see flashrom code used in coreboot, and I'm very interested in finding and fixing any obstacles to that goal.
You might be happy to know that portions of the u-boot driver actually came from flashrom.
Patrick, I think you worked on the coreboot patch mentioned above. Could you tell us about your reasons for choosing option 3, and how we can make option 2 viable?
Here's my take on this (which should not discourage Patrick from telling the full story and additional details that I might have missed)
The background of this is that with coreboot in it's current state you will only be able to resume from suspend when using the ChromiumOS version of u-boot as a payload on Sandybridge systems. When we started looking into these issues initially there was no SPI driver in coreboot while u-boot came readily equipped. So we passed the memory training data from coreboot to u-boot in CBMEM and let u-boot write it.
Reworking this to be independent from a certain payload (and getting features like resume from suspend to work with e.g. SeaBIOS) it was the simplest approach to move that portion of the code that already existed from u-boot to coreboot, which is what is working today.
This is, as every software project, work in progress and open to iterations. Each of those two before mentioned steps were what could be done with reasonable effort at the time, leading to the current situation of a working implementation.
Personally, I would appreciate if we can get this code in as it is for now, and replace it with a better solution as soon as somebody is able to come up with one.
Thanks, Stefan