Uwe Hermann wrote:
On Wed, Oct 07, 2009 at 08:22:52PM +0200, Peter Stuge wrote:
+++ trunk/coreboot-v2/src/Kconfig 2009-10-07 16:15:40 UTC (rev 4734)
..
+# TODO: Defined if no payload? Breaks build? +config COMPRESSED_PAYLOAD_LZMA
- bool "Use LZMA compression for payloads"
- default y
- depends on PAYLOAD_ELF
- help
In order to reduce the size payloads take up in the ROM chipcoreboot can compress them using the LZMA algorithm.Maybe:
default !PAYLOAD_NONE
Probably, but it's equivalent right now as we only support one type of payload in v2 so far (unless I'm mistaken).
Oh, wait. I'm confused. Do you mean "depends on !PAYLOAD_NONE" maybe?
Yes, that too. Maybe default y is OK with depends on !PAYLOAD_NONE?
Is the default value ignored if the dependency is not satisfied?
default "1106,3230" help
The comma-separated PCI vendor and device ID that would associateyour VGA BIOS to your video card.
..
This is a fun one. I think the default here should be empty, and should be overridden in suitable component Kconfigs.
If by "component" you mean mainboard I agree.
Not really, more like northbridge. Most boards which need a cbfs VGA BIOS have the PCI device in the northbridge, so that's where I think the default value belongs.
Some boards have discrete onboard graphics and need a VGA BIOS for that - then let the mainboard set another default.
As the VGA blobs are extracted from BIOS images and we can check the IDs for a given board we can easily set those as per-board defaults (but still leave them changeable by the user).
Even if the VGA BIOS is different from one board to another with the same northbridge (think VIA boards with and without TV driver e.g.) I think the PCI ID will stay the same. No?
+++ trunk/coreboot-v2/src/arch/i386/Kconfig 2009-10-07 16:15:40 UTC (rev 4734) @@ -1,23 +1,22 @@ +# This option is used to set the architecture of a mainboard to X86. +# It is usually set in mainboard/*/Kconfig. config ARCH_X86 bool
- help
This option is used to set the architecture of a mainboard.It is usually set in mainboard/*/Kconfig.I'm not so sure about this. There is a point in having these help texts in the Kconfig structure.
..
Ah, that's part of another patch I'm preparing, as discussed on IRC.
User-visible (viewable in menuconfig) help texts use the "help" keyword (otherwise they don't appear in menuconfig).
Invisible help texts (those for options which don't appear in menuconfig) will be changed to #-comments. These are _not_ meant for users (they won't see them in menuconfig anyway) but rather for developers viewing the Kconfig files.
If the help messages are within the Kconfig structure it will be possible to automatically produce documentation for developers who will eventually work inside Kconfig files. I think this is valuable. Like doxygen for Kconfig. If this information is in comments, then it is unneccessarily difficult to get an overview of all the options.
I only recently found that the master Options.lb has a description of each option. That is very nice, and I would like to have that also with Kconfig.
Not all help texts are distinguished in this way yet, that's what I intend to fix over time.
I understand that this was the intention, but I'm not sure it's the best place to store those comments.
+++ trunk/coreboot-v2/src/console/Kconfig 2009-10-07 16:15:40 UTC (rev 4734) @@ -1,53 +1,96 @@ config CONSOLE_SERIAL8250
..
- bool "Enable serial port console output" default y
- help
Send coreboot debug output to a serial port console.Put the help message in the flag description.
I don't understand what you mean here.
Suggest
- bool "Enable serial port console output" + bool "Send coreboot messages to a serial port console."
and remove the help text.
config SERIAL_SET_SPEED
- bool "Override the serial console BAUD rate"
- bool "Override the serial port BAUD rate" default y depends on CONSOLE_SERIAL8250
Override what? There is noone before us.
Hm, true. Are there corner cases (soft-reboots, resume, ...) where we may _not_ want to re-init serial or re-set the BAUD rate?
Interesting.
Or maybe "override" the hardware defaults is meant here? Not sure.
Me neither. Does anyone else know? In any case, maybe we just need to change the wording slightly to make it more clear?
"Set a serial port baud rate"
With a help text describing what happens if it is not selected.
config CONSOLE_VGA
- bool "Use VGA console, once initialized."
- bool "Use VGA console once initialized" default n
What is it that can use the VGA console? This setting is not really about using VGA as console - or is it? Can anything actually ever be seen there?
I think Stefan mentioned on IRC recently that it's pretty useless and we might remove support for this completely in future.
I'm fine with that.
VGA BIOS options control if the payload will be able to use VGA. That's all that matters.
+config PCI_OPTION_ROM_RUN_REALMODE
..
If you select this option, PCI option ROMs will be executednatively on the hardware (a 32bit x86 system is required).So this should depend on something.
The whole "choice" construct depends on stuff:
depends on PCI_ROM_RUN || VGA_ROM_RUN
I was thinking about "a 32bit x86 system" in particular. If it doesn't run on 64bit systems, I think we should use Kconfig to hide this choice.
Does this need CONFIG_ prepended? Hopefully not.
No. In Kconfig files CONFIG_ prefixes are not needed (and not allowed), in Makefiles and C code, you need CONFIG_.
Good. Thanks for clarification! :)
+# Map the config names to a hex value (bytes). config ROM_SIZE hex default 0x20000 if COREBOOT_ROMSIZE_KB_128
Are more default lines needed here?
Hm, why? There should be as many default lines as we allow ROM sizes to be selected in menuconfig.
Yeah, sorry, I thought they weren't there but they were just not in the context of the hunk.
//Peter