Be less noisy in ram_check in non-debug loglevels
As DBE61 support now runs ram_check for non-debug purposes and has expected failures, downgrade the per-address looped fail notification printk from BIOS_ERR to BIOS_DEBUG. To compensate promote the one-time result print from DEBUG to ERR if it was a failure.
Signed-off-by: Mart Raudsepp mart.raudsepp@artecdesign.ee --- lib/ramtest.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/ramtest.c b/lib/ramtest.c index 02a4bdb..24af6b4 100644 --- a/lib/ramtest.c +++ b/lib/ramtest.c @@ -97,7 +97,7 @@ static int ram_verify(unsigned long start, unsigned long stop) value = ram_read_phys(addr); if (value != addr) { /* Display address with error. */ - printk(BIOS_ERR, "Fail @%lx Read value=%lx\n", + printk(BIOS_DEBUG, "Fail @%lx Read value=%lx\n", addr, value); /* Abort after 256 verify errors. */ if (++i > 256) { @@ -111,7 +111,7 @@ static int ram_verify(unsigned long start, unsigned long stop) printk(BIOS_DEBUG, "%lx\r", addr);
/* Print whether or not the verify failed. */ - printk(BIOS_DEBUG, "\nDRAM range %sverified.", i ? "_NOT_ " : ""); + printk(i ? BIOS_ERR : BIOS_DEBUG, "\nDRAM range %sverified.\n", i ? "_NOT_ " : ""); return i; }
Mart Raudsepp wrote:
Be less noisy in ram_check in non-debug loglevels
As DBE61 support now runs ram_check for non-debug purposes and has expected failures, downgrade the per-address looped fail notification printk from BIOS_ERR to BIOS_DEBUG. To compensate promote the one-time result print from DEBUG to ERR if it was a failure.
Signed-off-by: Mart Raudsepp mart.raudsepp@artecdesign.ee
Acked-by: Peter Stuge peter@stuge.se
On 13.11.2008 02:07, Peter Stuge wrote:
Mart Raudsepp wrote:
Be less noisy in ram_check in non-debug loglevels
As DBE61 support now runs ram_check for non-debug purposes and has expected failures, downgrade the per-address looped fail notification printk from BIOS_ERR to BIOS_DEBUG. To compensate promote the one-time result print from DEBUG to ERR if it was a failure.
Signed-off-by: Mart Raudsepp mart.raudsepp@artecdesign.ee
Acked-by: Peter Stuge peter@stuge.se
I'm not so totally sure we really want this change. I won't veto it, but I think RAM test errors should always be shown with full verbosity. If it is really needed, you could add another parameter to ram_check which specifies the log level.
Regards, Carl-Daniel
Carl-Daniel Hailfinger wrote:
Be less noisy in ram_check in non-debug loglevels
I'm not so totally sure we really want this change. I won't veto it, but I think RAM test errors should always be shown with full verbosity.
Because the result is really meaningless in the general case I like the patch.
//Peter
On 13.11.2008 02:43, Peter Stuge wrote:
Carl-Daniel Hailfinger wrote:
Be less noisy in ram_check in non-debug loglevels
I'm not so totally sure we really want this change. I won't veto it, but I think RAM test errors should always be shown with full verbosity.
Because the result is really meaningless in the general case I like the patch.
You managed to snip my suggested solution when you quoted my mail. Here it is again for your convenience:
If it is really needed, you could add another parameter to ram_check which specifies the log level.
Regards, Carl-Daniel
On N, 2008-11-13 at 03:14 +0100, Carl-Daniel Hailfinger wrote:
On 13.11.2008 02:43, Peter Stuge wrote:
Carl-Daniel Hailfinger wrote:
Be less noisy in ram_check in non-debug loglevels
I'm not so totally sure we really want this change. I won't veto it, but I think RAM test errors should always be shown with full verbosity.
Because the result is really meaningless in the general case I like the patch.
You managed to snip my suggested solution when you quoted my mail. Here it is again for your convenience:
If it is really needed, you could add another parameter to ram_check which specifies the log level.
I'd actually go as far as rip out the last printk too, or rather always have it BIOS_DEBUG, not BIOS_ERR when it fails. Rationale is the following:
ram_check has three viable use cases that I can think of:
a) During development as a local addition of a call to it while doing memory timing fixing and work b) To do automatic memory sizing a la DBE61 c) To react somehow to memory failure, perhaps disabling one DIMM out of multiple or loading a payload that runs in CAR that instructs the user to fix his RAM or whatever.
b) and c) means its used in production code that shouldn't be spitting all this failure spam into serial, but instead the return value of ram_check should be checked by the caller and reacted upon (goes together with the warn_unused suggestion)
a) means it's done during development, in which case the developer better already have BIOS_DEBUG or BIOS_SPEW selected.
Besides, as the documentation of ram_check code itself says - it is "is my RAM properly configured" test, not a "is my RAM faulty" test. So what relevance do the exact verbose read failures have in a production builds console output (for b and c case, but especially b)?
So all that put together, I don't see a case where the exact failure location and their values should be spammed to console with less verbose loglevel than DEBUG.
Regards, Mart Raudsepp
On Wed, Nov 12, 2008 at 10:08 PM, Mart Raudsepp < mart.raudsepp@artecdesign.ee> wrote:
On N, 2008-11-13 at 03:14 +0100, Carl-Daniel Hailfinger wrote:
On 13.11.2008 02:43, Peter Stuge wrote:
Carl-Daniel Hailfinger wrote:
Be less noisy in ram_check in non-debug loglevels
I'm not so totally sure we really want this change. I won't veto it, but I think RAM test errors should always be shown with full verbosity.
Because the result is really meaningless in the general case I like the patch.
You managed to snip my suggested solution when you quoted my mail. Here it is again for your convenience:
If it is really needed, you could add another parameter to ram_check which specifies the log level.
I'd actually go as far as rip out the last printk too, or rather always have it BIOS_DEBUG, not BIOS_ERR when it fails. Rationale is the following:
ram_check has three viable use cases that I can think of:
a) During development as a local addition of a call to it while doing memory timing fixing and work b) To do automatic memory sizing a la DBE61 c) To react somehow to memory failure, perhaps disabling one DIMM out of multiple or loading a payload that runs in CAR that instructs the user to fix his RAM or whatever.
b) and c) means its used in production code that shouldn't be spitting all this failure spam into serial, but instead the return value of ram_check should be checked by the caller and reacted upon (goes together with the warn_unused suggestion)
If you're running it in production code to detect the ram size, do you really want to send an error message after you find the (expected) end of ram? In other words, do you want to have an error message show up on every boot displaying an error message about a ram failure, and the address of that failure, that an end user (familiar only with typical BIOSes) might believe to be a bad thing?
-Corey
On Wed, Nov 12, 2008 at 10:24 PM, Corey Osgood corey.osgood@gmail.com wrote:
If you're running it in production code to detect the ram size, do you really want to send an error message after you find the (expected) end of ram? In other words, do you want to have an error message show up on every boot displaying an error message about a ram failure, and the address of that failure, that an end user (familiar only with typical BIOSes) might believe to be a bad thing?
yes, a thing that is not an error should never look like an error ...
ron
Ühel kenal päeval, N, 2008-11-13 kell 05:08, kirjutas Mart Raudsepp:
On N, 2008-11-13 at 03:14 +0100, Carl-Daniel Hailfinger wrote:
On 13.11.2008 02:43, Peter Stuge wrote:
Carl-Daniel Hailfinger wrote:
Be less noisy in ram_check in non-debug loglevels
I'm not so totally sure we really want this change. I won't veto it, but I think RAM test errors should always be shown with full verbosity.
Because the result is really meaningless in the general case I like the patch.
You managed to snip my suggested solution when you quoted my mail. Here it is again for your convenience:
If it is really needed, you could add another parameter to ram_check which specifies the log level.
I'd actually go as far as rip out the last printk too, or rather always have it BIOS_DEBUG, not BIOS_ERR when it fails. Rationale is the following:
ram_check has three viable use cases that I can think of:
a) During development as a local addition of a call to it while doing memory timing fixing and work b) To do automatic memory sizing a la DBE61 c) To react somehow to memory failure, perhaps disabling one DIMM out of multiple or loading a payload that runs in CAR that instructs the user to fix his RAM or whatever.
b) and c) means its used in production code that shouldn't be spitting all this failure spam into serial, but instead the return value of ram_check should be checked by the caller and reacted upon (goes together with the warn_unused suggestion)
a) means it's done during development, in which case the developer better already have BIOS_DEBUG or BIOS_SPEW selected.
Besides, as the documentation of ram_check code itself says - it is "is my RAM properly configured" test, not a "is my RAM faulty" test. So what relevance do the exact verbose read failures have in a production builds console output (for b and c case, but especially b)?
So all that put together, I don't see a case where the exact failure location and their values should be spammed to console with less verbose loglevel than DEBUG.
Regards, Mart Raudsepp
Attached is a take on that.
-- Mart Raudsepp
Mart Raudsepp wrote:
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. Document that if something is wanted to be reported in non-debug loglevels, one should do so in the caller based on the return value. Tweak a debug string in ram_verify to be more descriptive.
Signed-off-by: Mart Raudsepp mart.raudsepp@artecdesign.ee
Acked-by: Peter Stuge peter@stuge.se
-
Carl-Daniel Hailfinger -
Corey Osgood -
Mart Raudsepp -
Peter Stuge -
ron minnich