Thank you ron for your comments!
actually I was not aware the code I look at is more than 5 years old. You are absolutely right in that the patch was written after reading public ICH7 documentation and I do not know of any problem the current implementation causes. Moreover, I was not aware that the code might do something else than what one can know by reading the code and the public documentation. To be honest, if that was true, I would be disappointed, because I expected coreboot to be a possibility of getting away from private software and "you don't know what your machine does". However, I did debug the code by reading the write-once registers in question before and after the several write operations. The registers do not change their contents but due to the first write operation. By that I concluded the public documentation describes the chip's behavior correctly. After reading your comments about private documentation of course this conclusion might be wrong though.
At the moment I believe that applying the patch does no harm. At least this is my current knowledge by running it at the MacBook2,1 for a couple of hours. Switching forth and back between coreboot versions (back to safe ones) works. I do not know if this was true for other boards.
Who knows, it might(!) fix some 5 year old bug?
In case you were interessted, I uploaded the output of inteltool when running the macbook without the patch, that is by compiling [1] and by compiling the patch on top of [1]. Please see 5324 without patch 5387: http://paste.flashrom.org/view.php?id=2051 5324 plus patch 5387: http://paste.flashrom.org/view.php?id=2052 diff -y without vs with patch 5387: http://paste.flashrom.org/view.php?id=2053
any light shedding is much appreciated! best regards, Mono
[1] http://review.coreboot.org/#/c/5324/
On Sun, Mar 16, 2014 at 09:26:09AM -0700, ron minnich wrote:
So do I understand it correctly, that this patch is for hardware that has been working, in some cases for 5 years; that it has been written after reading a public doc; and that it was not developed in response to a known bug or issue?
Code that does not conform to a public doc is NOT a bug.
A word to the wise: in all too many cases, the public docs and the private docs disagree. In some cases, the private docs directly contradict the public docs. And, in still other cases, the hardware contradicts the private docs. That's what makes this so much fun.
It's the implementation of the hardware, not the documentation, that matters.
Changes made in response to a reading of public docs, that are not known to resolve an actual observed problem, are a very risky thing to do, especially with old chipsets. They should be rejected outright unless they are confirmed to fix a problem.
We've had people push this kind of change in the past and break boards.
So tread carefully.
ron
-- coreboot mailing list: coreboot@coreboot.org http://www.coreboot.org/mailman/listinfo/coreboot