[coreboot] [PATCH][cbv2]Add serial port information to lbtable

[Stefan Reinauer]

* Uwe Hermann <uwe at hermann-uwe.de> [080116 16:03]:
> Looks good, but a few comments:
> 
> On Wed, Jan 16, 2008 at 12:34:14PM +0100, Patrick Georgi wrote:
> > the attached patch makes cbv2 add information about the serial port to
> > lbtable. For this, a new record type is created. Payloads can then parse
> > lbtable to figure out where to find the serial port (which is usually
> > used by lb as console already).
> > 
> > A patch for grub2 exists, too, and I'll implement a compatible change
> > for cbv3 once the record format (and overall design etc) is stable.
> 
> Have you checked lxbios continues to work after the changes?
> 
> 
> > Index: src/arch/i386/boot/linuxbios_table.c
> > ===================================================================
> > --- src/arch/i386/boot/linuxbios_table.c	(Revision 3049)
> > +++ src/arch/i386/boot/linuxbios_table.c	(Arbeitskopie)
> > @@ -74,6 +74,18 @@
> >  	return mem;
> >  }
> >  
> > +struct lb_serial *lb_serial(struct lb_header *header)
> > +{
> > +	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;
> 
> This doesn't look generic enough(?) Where does TTYS0_BASE come from?
> Does the code expect it to be defined in auto.c or cache_as_ram_auto.c?
> 
> 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).
> 
> Yes, I think it's just a name, in theory you could assign any value
> to TTYS0_BASE, but you get the point.

yes, the variable should not be named TTYS0 but SERIAL_CONSOLE or
something. But thats something I'd leave for v3.

-- 
coresystems GmbH • Brahmsstr. 16 • D-79104 Freiburg i. Br.
      Tel.: +49 761 7668825 • Fax: +49 761 7664613
Email: info at coresystems.de  • http://www.coresystems.de/
Registergericht: Amtsgericht Freiburg • HRB 7656
Geschäftsführer: Stefan Reinauer • Ust-IdNr.: DE245674866