svn@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.
Wouldn't the right fix be to explicitly only check those areas that pass are not "expected to fail"?
If RAM does not check, it's an error, not a debug message, and it should be printed as BIOS_ERR.
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.
Stefan
On Thu, Nov 20, 2008 at 12:23 PM, Stefan Reinauer stepan@coresystems.dewrote:
svn@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.
Wouldn't the right fix be to explicitly only check those areas that pass are not "expected to fail"?
If RAM does not check, it's an error, not a debug message, and it should be printed as BIOS_ERR.
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.
They're using ram_check for ram sizing without spd info, and don't want to print an error when one doesn't occur, so now they won't print one when one does occur either. The only way to satisfy both cases is code duplication or passing ram_check a debug level.
-Corey
On N, 2008-11-20 at 18:23 +0100, Stefan Reinauer wrote:
svn@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
-
Corey Osgood -
Mart Raudsepp -
Stefan Reinauer