ke...@et... (Ken Yap) writes:
> |Hi,
> | In 5.3.1, if -DCONSOLE_FIRMWARE, will compile error:
> |bin/bootlib.a(misc.o)(.text+0x267): In function `putchar':
> |: undefined reference to `console_putc'
> |bin/bootlib.a(misc.o)(.text+0x27d): In function `getchar':
> |: undefined reference to `console_ischar'
> |bin/bootlib.a(misc.o)(.text+0x286): In function `getchar':
> |: undefined reference to `console_getc'
> |bin/bootlib.a(misc.o)(.text+0x2b3): In function `iskey':
> |: undefined reference to `console_ischar'
> |make: *** [bin/via-rhine.tmp] Error 1
> |
> | I copyed video_subr.c and vga.h from linuxbios and modify
> |video_subr.c, main.c to support video output under CONSOLE_FIRMWARE.
>
> I think this patch is wrong for several reasons.
>
> CONSOLE_FIRMWARE is for display using the PCBIOS. It's not an error in
> the PCBIOS build because those routines are supplied by an assembly code
> interface to the PCBIOS.
CONSOLE_FIRMWARE also works under EFI with a different implementation.
It is totally inappropriate under LinuxBIOS as LinuxBIOS will _never_
provide callbacks.
> I'm not sure what you are booting. It sounds like you are booting
> LinuxBIOS and loading WinCE but not using a serial console. Then I think
> you should:
>
> 1. First try to hook into the LinuxBIOS routines for console I/O if
> possible. (Is it? I don't know whether LinuxBIOS provides entry points
> for console I/O.)
It does not, nor will it. At best it will provide a temple entry saying
use this piece of hardware for your console.
> 2. Otherwise, the files video_subr.c and vga.h should definitely go
> under arch/i386/ (you put them in core/) because the code are specific
> to i386 and won't work for other architectures. Then you need to make
> the code conditional on CONSOLE_FIRMWARE if you want to reuse that
> symbol. Question to the list: should these files be classified as
> CONSOLE_FIRMWARE or should another symbol be invented?
Another symbol say CONSOLE_DIRECT_VGA or something like that. These
routines are analogous to the serial port support. Possibly we
should create a drivers/console?
So moving serial.c is probably appropriate as well.
> 3. For a similar reason, your previous patch to the top level
> Makefile.main to make osloader.c depend on wince_loader is also wrong
> because the top level Makefile.main should be architecture independent.
That dependency is in some sense correct. Since all of the loaders
are included into one file for size reasons. But except in the most
peculiar circumstances I don't think it comes up as a problem.
> So this patch is not accepted as it is because it breaks the separation
> between the arch-independent and arch-specific parts of the source tree.
> It needs rework.
Sounds correct.
> Please monitor the etherboot-devel list for answers from Eric on the
> best way to proceed.
Ken you did a pretty decent job. I have not seen the patch so I can't
really comment on it beyond what I have done.
Eric
|