[coreboot] [RFT] Please test patch set intel/i945: Only write CID, PN and TCID once
mono at posteo.de
Sun Mar 16 22:00:32 CET 2014
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  and by compiling the patch on top of . Please see
5324 without patch 5387:
5324 plus patch 5387:
diff -y without vs with patch 5387:
any light shedding is much appreciated!
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
> 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.
> coreboot mailing list: coreboot at coreboot.org
More information about the coreboot