Fix code comments/documentation related to printk. Sprinkle TODOs all over obviously wrong comments. Document possible execution flow alternatives for later investigation.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
---
Index: LinuxBIOSv3/mainboard/adl/msm800sev/stage1.c =================================================================== --- LinuxBIOSv3/mainboard/adl/msm800sev/stage1.c (Revision 509) +++ LinuxBIOSv3/mainboard/adl/msm800sev/stage1.c (Arbeitskopie) @@ -51,5 +51,5 @@ */ cs5536_disable_internal_uart(); w83627hf_enable_serial(0x2e, SERIAL_DEV, SERIAL_IOBASE); - printk(BIOS_DEBUG, "Done %s\n", __FUNCTION__); + /* We can NOT yet use printk! */ } Index: LinuxBIOSv3/mainboard/emulation/qemu-x86/stage1.c =================================================================== --- LinuxBIOSv3/mainboard/emulation/qemu-x86/stage1.c (Revision 509) +++ LinuxBIOSv3/mainboard/emulation/qemu-x86/stage1.c (Arbeitskopie) @@ -17,7 +17,7 @@ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA */
-/* no printk allowed until hardware is ready; hardware is ready */ +/* no printk allowed until hardware is ready; hardware is not yet ready */
/** * start up hardware needed for stage1 Index: LinuxBIOSv3/lib/stage2.c =================================================================== --- LinuxBIOSv3/lib/stage2.c (Revision 509) +++ LinuxBIOSv3/lib/stage2.c (Arbeitskopie) @@ -37,7 +37,10 @@ * * Device Enumeration: in the dev_enumerate() phase. * - * TODO: Check whether this documentation is still correct. Improve it. + * TODO: + * - Check whether this documentation is still correct. Improve it. + * - Replace magic constants by #defines. + * - Fix POST codes. Right now phases 1+3 have POST 0x30, phases 2+4 have 0x40. */ int stage2(void) { @@ -54,13 +57,19 @@
post_code(0x20);
- /* TODO: Explain why we use printk here although it is impossible */ + /* TODO: Explain why we use printk here although it is claimed to be + * impossible according to the documentation. The "has to be done + * before printk can be used" comment below seems to suggest the same. + * However, we already enable serial in arch/x86/stage1.c:stage1_main() + * when we call hardware_stage1(); uart_init(); console_init(); + */ printk(BIOS_NOTICE, console_test);
dev_init();
/* Console init, also ANYTHING that has to be done - * before printk can be used. + * before printk can be used. + * FIXME: This comment contradicts the comment above. */ post_code(0x30); dev_phase1(); Index: LinuxBIOSv3/arch/x86/stage1.c =================================================================== --- LinuxBIOSv3/arch/x86/stage1.c (Revision 509) +++ LinuxBIOSv3/arch/x86/stage1.c (Arbeitskopie) @@ -108,8 +108,12 @@
// uart_init(); // initialize serial port - console_init(); // print banner
+ /* Exactly from now on we can use printk. Celebrate this by printing + * a LB banner. + */ + console_init(); + if (bist!=0) { printk(BIOS_INFO, "BIST FAILED: %08x", bist); die(""); @@ -133,6 +137,30 @@ // FIXME check integrity
+ /* Right now we have to fit + * initram stack + * lar stack + * into cache during CAR. IFF we can be sure that + * nrv2b stack + * initram code + * lar code + * fit into cache as well, we can compress initram code and lar code, + * leading to potential speedup (due to having to read less from ROM) + * and size savings. However, initram code and lar code would have + * to be compiled as PIC code, not as XIP code. + * + * Suggested code flow: + * unrv2b(lar_code); + * ... + * execute_in_car(&archive, "normal/initram"); + * + * A less complicated (from a linker perspective) variant would + * compress only initram code, but it is not clear whether all hardware + * can run code from cache while training memory in the area designated + * by the cache. Some processors might drop cache contents in that case, + * rendering the ideas above mostly moot. + * TODO: Check how this works in the real world. + */ // find first initram if (check_normal_boot_flag()) { printk(BIOS_DEBUG, "Choosing normal boot.\n");
I think printk is always available - any previous printk pain has been (finally) resolved.
On 15/11/07 01:25 +0100, Carl-Daniel Hailfinger wrote:
Fix code comments/documentation related to printk. Sprinkle TODOs all over obviously wrong comments. Document possible execution flow alternatives for later investigation.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Index: LinuxBIOSv3/mainboard/adl/msm800sev/stage1.c
--- LinuxBIOSv3/mainboard/adl/msm800sev/stage1.c (Revision 509) +++ LinuxBIOSv3/mainboard/adl/msm800sev/stage1.c (Arbeitskopie) @@ -51,5 +51,5 @@ */ cs5536_disable_internal_uart(); w83627hf_enable_serial(0x2e, SERIAL_DEV, SERIAL_IOBASE);
- printk(BIOS_DEBUG, "Done %s\n", __FUNCTION__);
- /* We can NOT yet use printk! */
} Index: LinuxBIOSv3/mainboard/emulation/qemu-x86/stage1.c =================================================================== --- LinuxBIOSv3/mainboard/emulation/qemu-x86/stage1.c (Revision 509) +++ LinuxBIOSv3/mainboard/emulation/qemu-x86/stage1.c (Arbeitskopie) @@ -17,7 +17,7 @@
- Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
*/
-/* no printk allowed until hardware is ready; hardware is ready */ +/* no printk allowed until hardware is ready; hardware is not yet ready */
/**
- start up hardware needed for stage1
Index: LinuxBIOSv3/lib/stage2.c
--- LinuxBIOSv3/lib/stage2.c (Revision 509) +++ LinuxBIOSv3/lib/stage2.c (Arbeitskopie) @@ -37,7 +37,10 @@
- Device Enumeration: in the dev_enumerate() phase.
- TODO: Check whether this documentation is still correct. Improve it.
- TODO:
- Check whether this documentation is still correct. Improve it.
- Replace magic constants by #defines.
*/
- Fix POST codes. Right now phases 1+3 have POST 0x30, phases 2+4 have 0x40.
int stage2(void) { @@ -54,13 +57,19 @@
post_code(0x20);
- /* TODO: Explain why we use printk here although it is impossible */
/* TODO: Explain why we use printk here although it is claimed to be
* impossible according to the documentation. The "has to be done
* before printk can be used" comment below seems to suggest the same.
* However, we already enable serial in arch/x86/stage1.c:stage1_main()
* when we call hardware_stage1(); uart_init(); console_init();
*/
printk(BIOS_NOTICE, console_test);
dev_init();
/* Console init, also ANYTHING that has to be done
* before printk can be used.
* before printk can be used.
*/ post_code(0x30); dev_phase1();* FIXME: This comment contradicts the comment above.
Index: LinuxBIOSv3/arch/x86/stage1.c
--- LinuxBIOSv3/arch/x86/stage1.c (Revision 509) +++ LinuxBIOSv3/arch/x86/stage1.c (Arbeitskopie) @@ -108,8 +108,12 @@
// uart_init(); // initialize serial port
- console_init(); // print banner
- /* Exactly from now on we can use printk. Celebrate this by printing
* a LB banner.
*/
- console_init();
- if (bist!=0) { printk(BIOS_INFO, "BIST FAILED: %08x", bist); die("");
@@ -133,6 +137,30 @@ // FIXME check integrity
- /* Right now we have to fit
* initram stack
* lar stack
* into cache during CAR. IFF we can be sure that
* nrv2b stack
* initram code
* lar code
* fit into cache as well, we can compress initram code and lar code,
* leading to potential speedup (due to having to read less from ROM)
* and size savings. However, initram code and lar code would have
* to be compiled as PIC code, not as XIP code.
*
* Suggested code flow:
* unrv2b(lar_code);
* ...
* execute_in_car(&archive, "normal/initram");
*
* A less complicated (from a linker perspective) variant would
* compress only initram code, but it is not clear whether all hardware
* can run code from cache while training memory in the area designated
* by the cache. Some processors might drop cache contents in that case,
* rendering the ideas above mostly moot.
* TODO: Check how this works in the real world.
// find first initram if (check_normal_boot_flag()) { printk(BIOS_DEBUG, "Choosing normal boot.\n");*/
-- linuxbios mailing list linuxbios@linuxbios.org http://www.linuxbios.org/mailman/listinfo/linuxbios
On 15.11.2007 01:32, Jordan Crouse wrote:
I think printk is always available - any previous printk pain has been (finally) resolved.
That warrants updating the design doc as well as another question: Is printk available before early serial port setup? AFAICS it is not. We have to differentiate between 4 stages (not in our stage sense) for printk: 1. No CAR, no RAM, no early serial - > printk is not feasible 2. CAR, but no RAM, no early serial -> printk to an internal buffer could work, but the user can't see that 3. CAR, early serial, but no RAM -> printk to serial works 4. (much later) RAM, serial, VGA -> printk to serial and VGA works
When do we want to declare printk as usable?
Regards, Carl-Daniel
On 15/11/07 11:45 +0100, Carl-Daniel Hailfinger wrote:
On 15.11.2007 01:32, Jordan Crouse wrote:
I think printk is always available - any previous printk pain has been (finally) resolved.
That warrants updating the design doc as well as another question: Is printk available before early serial port setup? AFAICS it is not. We have to differentiate between 4 stages (not in our stage sense) for printk:
- No CAR, no RAM, no early serial - > printk is not feasible
- CAR, but no RAM, no early serial -> printk to an internal buffer
could work, but the user can't see that 3. CAR, early serial, but no RAM -> printk to serial works 4. (much later) RAM, serial, VGA -> printk to serial and VGA works
When do we want to declare printk as usable?
The question is - do we care? Technically, the printk guts should be smart enough to know if it can use the outputs or not - either by purposely returning early, or maybe just by pushing bits to an I/O port that isn't listening yet.
If you, as a developer happen to printk() before that point, then you will miss your message, that is true. To me that is far better then trying to impose a self-policed moratorium on using printk() too early, which despite all the documentation in the world, somebody is bound to ignore or screw up. Imposing restrictions on developers without software or hardware enforcement will only end in disaster.
The fun part is that it will just work out in the end. Developers who have a reason to be adding printks to very early init (pre raminit) will probably also be the ones who are adding the early serial code too.
Jordan
On 15.11.2007 17:20, Jordan Crouse wrote:
On 15/11/07 11:45 +0100, Carl-Daniel Hailfinger wrote:
On 15.11.2007 01:32, Jordan Crouse wrote:
I think printk is always available - any previous printk pain has been (finally) resolved.
That warrants updating the design doc as well as another question: Is printk available before early serial port setup? AFAICS it is not. We have to differentiate between 4 stages (not in our stage sense) for printk:
- No CAR, no RAM, no early serial - > printk is not feasible
- CAR, but no RAM, no early serial -> printk to an internal buffer
could work, but the user can't see that 3. CAR, early serial, but no RAM -> printk to serial works 4. (much later) RAM, serial, VGA -> printk to serial and VGA works
When do we want to declare printk as usable?
The question is - do we care? Technically, the printk guts should be smart enough to know if it can use the outputs or not - either by purposely returning early, or maybe just by pushing bits to an I/O port that isn't listening yet.
If pushing bits to a non-listening I/O port is harmless, that's nice.
If you, as a developer happen to printk() before that point, then you will miss your message, that is true. To me that is far better then trying to impose a self-policed moratorium on using printk() too early, which despite all the documentation in the world, somebody is bound to ignore or screw up. Imposing restrictions on developers without software or hardware enforcement will only end in disaster.
OK, I'll make sure printk gets a bit smarter and update the docs.
The fun part is that it will just work out in the end. Developers who have a reason to be adding printks to very early init (pre raminit) will probably also be the ones who are adding the early serial code too.
Thanks for the explanation. Expect patches.
Regards, Carl-Daniel