[coreboot] [updated PATCH] Re: [cbv2]Add serial port information to lbtable

[Patrick Georgi]

Am Donnerstag, den 17.01.2008, 12:55 +0100 schrieb Torsten Duwe:
> On Wednesday 16 January 2008, Stefan Reinauer wrote:
> > * Uwe Hermann <uwe at hermann-uwe.de> [080116 16:03]:
> > > This doesn't look generic enough(?) Where does TTYS0_BASE come from?
> 
> Agreed.
TTYS0_BASE seems to be the standard name used across the code base.
The attached patch provides a no-op in case the configuration doesn't
provide a serial port.

> > > I think not all of the boards define it, and even if they did,
> > > TTYS0_BASE implies COM1, which is not always what you want.
> > > We need a mechanism to allow other ports here (COM2, COM3, COM4).
> 
> This goes off into the wrong direction, IMO.
The codebase doesn't seem to be prepared for multiple serial ports in
the first place (I only find TTYS0_*). I'd say, we add a port number to
the record once we actually support >1 ports.

> Both not appropriate, strictly. There is only one serial port, for the console 
> at best that everyone needs to care about until an OS is loaded.
As my initial query
(http://www.coreboot.org/pipermail/coreboot/2008-January/029192.html)
got no response, I decided to just work on the problem at hand, which is
- in my case - a hand over of information about the serial port (which
we don't do at all currently).

As you can see, my patch doesn't pretend that the serial port is the console, so a "you can find the console $here" record can be added without conflicts. That new record could then refer to the serial port as specified here, in case the console is on serial.

> The questions are:
> 1. of what type is the active console?
> a) VGA-compatible
> b) 8250-compatible
> c) ...
Different issue (see above)

> 3. Is there additional information? A smarter payload might want to use e.g. 
> the interrupt that we assigned to the port ;-)
Once we do that, the serial record could be extended by that
information. Given that we have a size field in the record, that can
even be done in a compatible way, or not?

I'll do patches for lxbios and v3 once this patch (and more importantly,
the record format) is accepted.
This patch is updated against latest svn, and provides an alternative in
case TTYS0_BASE isn't defined (see the #ifdef).


Regards,
Patrick Georgi
-------------- next part --------------
This patch adds a new record type for lbtable to provide information
about a serial port. If a port is defined in the board configuration,
add it to lbtable.

Signed-off-by: Patrick Georgi <patrick at georgi-clan.de>

Index: src/include/boot/coreboot_tables.h
===================================================================
--- src/include/boot/coreboot_tables.h	(Revision 3066)
+++ src/include/boot/coreboot_tables.h	(Arbeitskopie)
@@ -138,6 +138,13 @@
 	uint8_t  string[0];
 };
 
+#define LB_TAG_SERIAL		0x000e
+struct lb_serial {
+	uint32_t tag;
+	uint32_t size;
+	uint16_t ioport;
+};
+
 /* The following structures are for the cmos definitions table */
 #define LB_TAG_CMOS_OPTION_TABLE 200
 /* cmos header record */
Index: src/arch/i386/boot/coreboot_table.c
===================================================================
--- src/arch/i386/boot/coreboot_table.c	(Revision 3066)
+++ src/arch/i386/boot/coreboot_table.c	(Arbeitskopie)
@@ -74,6 +74,22 @@
 	return mem;
 }
 
+struct lb_serial *lb_serial(struct lb_header *header)
+{
+#if defined(TTYS0_BASE)
+	struct lb_record *rec;
+	struct lb_serial *serial;
+	rec = lb_new_record(header);
+	serial = (struct lb_serial *)rec;
+	serial->tag = LB_TAG_SERIAL;
+	serial->size = sizeof(*serial);
+	serial->ioport = TTYS0_BASE;
+	return serial;
+#else
+	return header;
+#endif
+}
+
 struct lb_mainboard *lb_mainboard(struct lb_header *header)
 {
 	struct lb_record *rec;
@@ -406,8 +422,10 @@
 	 * size of the coreboot table.
 	 */
 
-	/* Record our motheboard */
+	/* Record our motherboard */
 	lb_mainboard(head);
+	/* Record our console */
+	lb_serial(head);
 	/* Record our various random string information */
 	lb_strings(head);