[coreboot] r1047 - coreboot-v3/lib

[Mart Raudsepp]

On N, 2008-11-20 at 18:23 +0100, Stefan Reinauer wrote:
> svn at coreboot.org wrote:
> > Author: mraudsepp
> > Date: 2008-11-20 13:20:35 +0100 (Thu, 20 Nov 2008)
> > New Revision: 1047
> >
> > Modified:
> >    coreboot-v3/lib/ramtest.c
> > Log:
> > Be silent in ram_check in non-debug loglevels
> >
> > As DBE61 support now runs ram_check for non-debug purposes and has expected failures
> > on DBE61A, downgrade the per-address looped fail notification printk and other messages
> > from BIOS_ERR to BIOS_DEBUG.
> >   
> 
> This patch looks odd.
> 
> Why do you run the ram test at all if the debug level is below DEBUG, if
> none of the results are printed? Arguing with the parsable return code
> sounds a bit outrageous and would lead to see the same error printed
> twice with BIOS_DEBUG level.

The commit message says why - expected failure -- it is not an error.

> Wouldn't the right fix be to explicitly only check those areas that pass
> are not "expected to fail"?

It is checking a correct area that is expected to fail if the SPD setup
is wrong. Expected as in, if it is wrong, we try the memory setup for a
smaller amount for a different DBE61 variant.

> If RAM does not check, it's an error, not a debug message, and it should
> be printed as BIOS_ERR.

It is not an error in the DBE61 case (co-incidentally the only use of
ram_check other than for debug purposes right now).

> If auto.c tries to check areas that are not checkable (ie vga memory),
> that's a bug in auto.c and should be fixed there.

v3 doesn't have auto.c's, I assume you mean initram.c - it is checking
an area that is checkable, please refer to the thread "[PATCH 5/5] Be
less noisy in ram_check in non-debug loglevels" for a reasoning why it
is never necessary to show a message at BIOS_ERR console loglevel.


Regards,
Mart Raudsepp