See patch.
Note that we need code-fixes for some chipsets and board when enabling some of the DEBUG options (my guess is that code was never tested much), but this is not related to my patch, those problems have been there already:
- Some romcc boards throw "too few registers" errors if you enable extra debugging options. That can partly fixed by changing code or by changing -mcpu=i386 to -mcpu=p2 or similar (if possible for the board/CPU).
- Some chipset code needs fixing as boards won't even compile if some of the DEBUG options are enabled.
- And there are further DEBUG* defines which are not so easily converted (e.g. some use integers like LOGLEVEL does, instead of just boolean on/off). I will have a look at converting more of those to kconfig later.
Uwe.
I like the idea of having Options that we use more available.
DEBUG_SMI was the default. Now it isn't. DEBUG_RAM_SETUP was the default for i440bx & vx800 in the Kontron code,
#if CONFIG_DEFAULT_CONSOLE_LOGLEVEL > 8 This never does anything, since 8 is SPEW.
I think it's too dangerous to play with options like this if we haven't tested it in hardware. Right now I'd like to match newconfig and kill it as quickly as possible. Supporting two versions and trying to keep them in sync is too hard.
Thanks, Myles
On Tue, Oct 20, 2009 at 11:37:02AM -0600, Myles Watson wrote:
I like the idea of having Options that we use more available.
DEBUG_SMI was the default. Now it isn't. DEBUG_RAM_SETUP was the default for i440bx & vx800
True, but that's intentional. Just like the "use lzma" or "execute vga blob" options these are now global, user-configurable (as opposed to per-board in Options.lb in newconfig). The old per-board or per-chipset defaults don't matter much, the user chooses if he/she wants the option.
I think these options are good candidates for filtering in the compare script.
in the Kontron code, #if CONFIG_DEFAULT_CONSOLE_LOGLEVEL > 8 This never does anything, since 8 is SPEW.
Due to the recent LOGLEVEL changes, yes. We should do a repo-wide fixing of any such occurences (there may be more of them). This is unrelated to my patch though, the check is no longer correct with and without the patch.
I'll post a followup-patch for this issue if nobody beats me to it.
Right now I'd like to match newconfig and kill it as quickly as possible.
Sure, same here. I don't think this patch interferes with this effort though.
Uwe.
On Tue, Oct 20, 2009 at 11:37:02AM -0600, Myles Watson wrote:
I like the idea of having Options that we use more available.
DEBUG_SMI was the default. Now it isn't. DEBUG_RAM_SETUP was the default for i440bx & vx800
True, but that's intentional. Just like the "use lzma" or "execute vga blob" options these are now global, user-configurable (as opposed to per-board in Options.lb in newconfig). The old per-board or per-chipset defaults don't matter much, the user chooses if he/she wants the option.
I think these options are good candidates for filtering in the compare script.
As long as the functionality isn't changed, I agree. I don't have any of the affected boards to test.
in the Kontron code, #if CONFIG_DEFAULT_CONSOLE_LOGLEVEL > 8 This never does anything, since 8 is SPEW.
Due to the recent LOGLEVEL changes, yes.
Before the changes you pointed out to me that SPEW was 8.
We should do a repo-wide fixing of any such occurences (there may be more of them). This is unrelated to my patch though, the check is no longer correct with and without the patch.
I was pointing out that it isn't compiled in, so if it breaks abuild, it will break abuild when the CONSOLE_LOGLEVEL line changes.
I'll post a followup-patch for this issue if nobody beats me to it.
Right now I'd like to match newconfig and kill it as quickly as possible.
Sure, same here. I don't think this patch interferes with this effort though.
As long as all of the code that gets enabled/disabled is only print statements, that's probably true. I was worried that some of these boards may depend on the correct setting of those debug flags to be functional.
Thanks, Myles
On Wed, Oct 21, 2009 at 01:23:21PM -0600, Myles Watson wrote:
As long as all of the code that gets enabled/disabled is only print statements, that's probably true. I was worried that some of these boards may depend on the correct setting of those debug flags to be functional.
I don't think so, and if that were the case it'd be a bug anyway.
Uwe.
On Wed, Oct 21, 2009 at 3:04 PM, Uwe Hermann uwe@hermann-uwe.de wrote:
On Wed, Oct 21, 2009 at 01:23:21PM -0600, Myles Watson wrote:
As long as all of the code that gets enabled/disabled is only print statements, that's probably true. I was worried that some of these
boards
may depend on the correct setting of those debug flags to be functional.
I don't think so, and if that were the case it'd be a bug anyway.
Unfortunately Coreboot is not yet bug free.
Thanks, Myles
Myles Watson wrote:
Right now I'd like to match newconfig and kill it as quickly as possible.
Sure, same here. I don't think this patch interferes with this effort though.
As long as all of the code that gets enabled/disabled is only print statements, that's probably true. I was worried that some of these boards may depend on the correct setting of those debug flags to be functional.
Which boards are that?
Stefan
As long as all of the code that gets enabled/disabled is only print statements, that's probably true. I was worried that some of these
boards
may depend on the correct setting of those debug flags to be functional.
Which boards are that?
There are lots of boards affected by this patch, and especially some of the DEBUG_CAR stuff looked like copy-paste from other places. I don't think we should encourage people to change those settings until they've been tested, or at least until someone is willing to go through and make sure that it only enables/disables print statements.
Thanks, Myles
-
Myles Watson -
Stefan Reinauer -
Uwe Hermann