On Thu, Sep 14, 2017 at 09:08:20AM +0200, Gerd Hoffmann wrote:
> Redirect int10 calls to serial console output.
> Parse serial input and queue key events.
> The serial console can work both as primary display
> and in parallel to another vga display (splitmode).
Thanks. Looks good. I have a few minor comments, but they can all be
fixed post commit. (Let me know if you prefer otherwise.)
> --- a/Makefile
> +++ b/Makefile
> @@ -29,7 +29,7 @@ LD32BIT_FLAG:=-melf_i386
>
> # Source files
> SRCBOTH=misc.c stacks.c output.c string.c block.c cdrom.c disk.c mouse.c
> kbd.c \
> - system.c serial.c clock.c resume.c pnpbios.c vgahooks.c pcibios.c apm.c \
> + system.c serial.c sercon.c clock.c resume.c pnpbios.c vgahooks.c
> pcibios.c apm.c \
> cp437.c \
Line wrap.
> --- a/src/util.h
> +++ b/src/util.h
> @@ -234,6 +234,9 @@ void code_mutable_preinit(void);
> // serial.c
> void serial_setup(void);
> void lpt_setup(void);
> +void sercon_10(struct bregs *regs);
> +void sercon_setup(u16 iobase);
> +void sercon_check_event(void);
These should be in a "// sercon.c" section (which should be above the
"serial.c" block to keep the alphabetical ordering). Also,
sercon_10() doesn't need to be exported here.
[...]
> --- a/src/optionroms.c
> +++ b/src/optionroms.c
> @@ -12,6 +12,7 @@
> #include "hw/pcidevice.h" // foreachpci
> #include "hw/pci_ids.h" // PCI_CLASS_DISPLAY_VGA
> #include "hw/pci_regs.h" // PCI_ROM_ADDRESS
> +#include "hw/serialio.h" // PORT_SERIAL1
> #include "malloc.h" // rom_confirm
> #include "output.h" // dprintf
> #include "romfile.h" // romfile_loadint
> @@ -404,6 +405,8 @@ struct rom_header *VgaROM;
> void
> vgarom_setup(void)
> {
> + u16 ret, iobase = 0;
> +
> if (! CONFIG_OPTIONROMS)
> return;
>
> @@ -432,11 +435,22 @@ vgarom_setup(void)
> run_file_roms("vgaroms/", 1, NULL);
> rom_reserve(0);
>
> - if (rom_get_last() == BUILD_ROM_START)
> + ret = romfile_loadint("etc/sercon-enable", 0);
> + if (ret)
> + iobase = PORT_SERIAL1;
> +
> + if (rom_get_last() == BUILD_ROM_START) {
> // No VGA rom found
> + if (iobase) {
> + sercon_setup(iobase);
> + enable_vga_console();
> + }
> return;
> + }
>
> VgaROM = (void*)BUILD_ROM_START;
> + if (iobase)
> + sercon_setup(iobase);
> enable_vga_console();
> }
It would be preferable to not implement sercon logic in optionroms.c.
Instead, the call to enable_vga_console() can be moved into
post.c:maininit() and both it and sercon_setup() can be called
unconditionally there.
[...]
> +void VISIBLE16
> +sercon_10(struct bregs *regs)
> +{
[...]
> --- a/src/romlayout.S
> +++ b/src/romlayout.S
> @@ -414,6 +414,53 @@ __csm_return:
[...]
> + // call sercon_10
> + popl %eax
> + popw %ds
> +2: pushl $sercon_10
In keeping with the other assembler entry points, it would be
preferable to use the naming entry_sercon / handle_sercon().
-Kevin
_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
https://mail.coreboot.org/mailman/listinfo/seabios
