On Mon, Mar 17, 2008 at 12:43 AM, ron minnich <<a href="mailto:rminnich@gmail.com">rminnich@gmail.com</a>> wrote:<br><div class="gmail_quote"><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<div class="Ih2E3d">On Sun, Mar 16, 2008 at 6:44 PM, Carl-Daniel Hailfinger<br>
<<a href="mailto:c-d.hailfinger.devel.2006@gmx.net">c-d.hailfinger.devel.2006@gmx.net</a>> wrote:<br>
<br>
>  Agreed, but at the same time, I'd like to keep this in the code as<br>
>  reference for future porters. Maybe have it depend on<br>
>  CONFIG_PARANOID_CHECKING or something like that?<br>
>  Or we simply comment it out once the board is running perfectly.<br>
<br>
</div>We're spending a lot of time on this one :-)<br>
<br>
i can't debug without the memtest, but it is true that we don't need<br>
it once a board works.<br>
<br>
One conclusion I come away with, which you may all disagree with.<br>
<br>
#ifdef option is the wrong way to go. Options should always have a<br>
defined value. Why? So we can let C do its job, e.g.:<br>
if (CONFIG_THIS_THAT == 'y') {<br>
 whatever();<br>
}<br>
<br>
This code will be enabled or completely optimized out, and it's pretty<br>
clear what's up. And it looks more like C than:<br>
#ifdef CONFIG_THIS_THAT<br>
  whatever();<br>
#endif<br>
<br>
That's my main conclusion from this thread, but I'm not worried enough<br>
about it either way to argue the point too much.<br>
<br>
thanks<br>
<font color="#888888"><br>
ron</font></blockquote><div><br>IMO, ram_check should be a lot more advanced and configurable then it already is. I'd like to see it work something like this:<br><br>* configured via Kconfig. Builder has the option of running it all the time, only if coreboot enters fallback, or if the memory size changes<br>
* size of the ram is stored in cmos, for above. obviously there would need to be some northbridge-specific function created to return the size of the ram<br>* ram_check has two options, basic or advanced. basic would be the current code, advanced would incorporate more tests, similar to memtest86's tests. user gets the option of which test is run. possibly basic runs when the memory size changes, advanced only runs in fallback.<br>
<br>The reasoning is that many problems, both with coreboot/BIOS and OS, can be attributed to the memory. If we have a system for diagnosing memory errors from coreboot, before we even attempt to use the potentially bad ram, we have that small advantage over the factory BIOS. On embedded, this seems like less of a concern, since the memory is usually soldered on and the user never touches it, but on K8 desktop boards, combined with users ignorant to ESD damage, this seems like it could be useful. If others want to see this, I'll try to make it happen.<br>
</div></div><br>-Corey<br>