[PATCH] fix serial console on LinuxBIOS

[PATCH] fix serial console on LinuxBIOS

[Robert Millan]

[-- Attachment #1: Type: text/plain, Size: 270 bytes --]


This patch fixes serial console on LinuxBIOS.

I'd appreciate comments (specially on the <grub/machine/machine.h> addition).

-- 
Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call, if you are unable to speak?
(as seen on /.)

[-- Attachment #2: serial_lb.diff --]
[-- Type: text/x-diff, Size: 7563 bytes --]


	* include/grub/i386/efi/machine.h: New file.
	* include/grub/i386/linuxbios/machine.h: Likewise.
	* include/grub/i386/pc/machine.h: Likewise.
	* include/grub/powerpc/ieee1275/machine.h: Likewise.
	* include/grub/sparc64/ieee1275/machine.h: Likewise.

	* term/i386/pc/serial.c: Include <grub/machine/machine.h>.
	(serial_hw_io_addr): New variable.
	(serial_hw_get_port): Obtain port address from `serial_hw_io_addr'
	instead of `(unsigned short *) 0x400'.

diff -Nurp grub2/include/grub/i386/efi/machine.h grub2.serial/include/grub/i386/efi/machine.h
--- grub2/include/grub/i386/efi/machine.h	1970-01-01 01:00:00.000000000 +0100
+++ grub2.serial/include/grub/i386/efi/machine.h	2007-11-07 21:46:10.000000000 +0100
@@ -0,0 +1,24 @@
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2007  Free Software Foundation, Inc.
+ *
+ *  GRUB is free software: you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation, either version 3 of the License, or
+ *  (at your option) any later version.
+ *
+ *  GRUB is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef GRUB_MACHINE_MACHINE_HEADER
+#define GRUB_MACHINE_MACHINE_HEADER	1
+
+#define GRUB_MACHINE_EFI	1
+
+#endif /* ! GRUB_MACHINE_MACHINE_HEADER */
diff -Nurp grub2/include/grub/i386/linuxbios/machine.h grub2.serial/include/grub/i386/linuxbios/machine.h
--- grub2/include/grub/i386/linuxbios/machine.h	1970-01-01 01:00:00.000000000 +0100
+++ grub2.serial/include/grub/i386/linuxbios/machine.h	2007-11-07 21:46:20.000000000 +0100
@@ -0,0 +1,24 @@
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2007  Free Software Foundation, Inc.
+ *
+ *  GRUB is free software: you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation, either version 3 of the License, or
+ *  (at your option) any later version.
+ *
+ *  GRUB is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef GRUB_MACHINE_MACHINE_HEADER
+#define GRUB_MACHINE_MACHINE_HEADER	1
+
+#define GRUB_MACHINE_LINUXBIOS	1
+
+#endif /* ! GRUB_MACHINE_MACHINE_HEADER */
diff -Nurp grub2/include/grub/i386/pc/machine.h grub2.serial/include/grub/i386/pc/machine.h
--- grub2/include/grub/i386/pc/machine.h	1970-01-01 01:00:00.000000000 +0100
+++ grub2.serial/include/grub/i386/pc/machine.h	2007-11-07 21:45:53.000000000 +0100
@@ -0,0 +1,24 @@
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2007  Free Software Foundation, Inc.
+ *
+ *  GRUB is free software: you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation, either version 3 of the License, or
+ *  (at your option) any later version.
+ *
+ *  GRUB is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef GRUB_MACHINE_MACHINE_HEADER
+#define GRUB_MACHINE_MACHINE_HEADER	1
+
+#define GRUB_MACHINE_PCBIOS	1
+
+#endif /* ! GRUB_MACHINE_MACHINE_HEADER */
diff -Nurp grub2/include/grub/powerpc/ieee1275/machine.h grub2.serial/include/grub/powerpc/ieee1275/machine.h
--- grub2/include/grub/powerpc/ieee1275/machine.h	1970-01-01 01:00:00.000000000 +0100
+++ grub2.serial/include/grub/powerpc/ieee1275/machine.h	2007-11-07 21:46:45.000000000 +0100
@@ -0,0 +1,24 @@
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2007  Free Software Foundation, Inc.
+ *
+ *  GRUB is free software: you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation, either version 3 of the License, or
+ *  (at your option) any later version.
+ *
+ *  GRUB is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef GRUB_MACHINE_MACHINE_HEADER
+#define GRUB_MACHINE_MACHINE_HEADER	1
+
+#define GRUB_MACHINE_IEEE1275	1
+
+#endif /* ! GRUB_MACHINE_MACHINE_HEADER */
diff -Nurp grub2/include/grub/sparc64/ieee1275/machine.h grub2.serial/include/grub/sparc64/ieee1275/machine.h
--- grub2/include/grub/sparc64/ieee1275/machine.h	1970-01-01 01:00:00.000000000 +0100
+++ grub2.serial/include/grub/sparc64/ieee1275/machine.h	2007-11-07 21:47:00.000000000 +0100
@@ -0,0 +1,24 @@
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2007  Free Software Foundation, Inc.
+ *
+ *  GRUB is free software: you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation, either version 3 of the License, or
+ *  (at your option) any later version.
+ *
+ *  GRUB is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef GRUB_MACHINE_MACHINE_HEADER
+#define GRUB_MACHINE_MACHINE_HEADER	1
+
+#define GRUB_MACHINE_IEEE1275	1
+
+#endif /* ! GRUB_MACHINE_MACHINE_HEADER */
diff -Nurp grub2/term/i386/pc/serial.c grub2.serial/term/i386/pc/serial.c
--- grub2/term/i386/pc/serial.c	2007-11-07 21:39:04.000000000 +0100
+++ grub2.serial/term/i386/pc/serial.c	2007-11-07 21:44:22.000000000 +0100
@@ -16,6 +16,7 @@
  *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include <grub/machine/machine.h>
 #include <grub/machine/serial.h>
 #include <grub/machine/console.h>
 #include <grub/term.h>
@@ -63,13 +64,18 @@ struct serial_port
 /* Serial port settings.  */
 static struct serial_port serial_settings;
 
+#ifdef GRUB_MACHINE_PCBIOS
+/* The BIOS data area.  */
+static const unsigned short *serial_hw_io_addr = (const unsigned short *) 0x0400;
+#else
+static const unsigned short serial_hw_io_addr[] = { 0x3f8, 0x2f8 };
+#endif
+
 /* Return the port number for the UNITth serial device.  */
 static inline unsigned short
 serial_hw_get_port (const unsigned short unit)
 {
-  /* The BIOS data area.  */
-  const unsigned short *addr = (const unsigned short *) 0x0400;
-  return addr[unit];
+  return serial_hw_io_addr[unit];
 }
 
 /* Fetch a key.  */

Re: [PATCH] fix serial console on LinuxBIOS

[Jerone Young]

Does Linux bios only allow for two serial ports? Are theres ports
addresses hard coded into
Linux BIOS?

I also creating all the machine.h for one ifdef is a little much. But
it may be needed. The
ifdef in serial.c should be changed to

#ifdef GRUB_MACHINE_LINUXBIOS
<then do this>
#else
<do what everyone else does>

Also what I don't understand about the current code is that it only
allows for serial ports
that have address 0x0400 and above (I'm guessing that is the c
trickery that is going on
here) when using addr[unit]. And you declare addr is 0x0400.


On Nov 7, 2007 2:53 PM, Robert Millan <rmh@aybabtu.com> wrote:
>
> This patch fixes serial console on LinuxBIOS.
>
> I'd appreciate comments (specially on the <grub/machine/machine.h> addition).
>
> --
> Robert Millan
>
> <GPLv2> I know my rights; I want my phone call!
> <DRM> What use is a phone call, if you are unable to speak?
> (as seen on /.)
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel
>
>

Re: [PATCH] fix serial console on LinuxBIOS

[Robert Millan]

On Wed, Nov 07, 2007 at 08:02:28PM -0600, Jerone Young wrote:
> Does Linux bios only allow for two serial ports?

I think the PC/AT architecture is limited to 2 serial ports.  Anyway,
LinuxBIOS does nothing to "allow" their use; it just initialises some
hardware and hands out control to GRUB in a non-intrusive manner.

> Are theres ports
> addresses hard coded into
> Linux BIOS?

Yes.  What is not hardcoded is the com1 vs com2 choice (chosen at build
time).  But that's a separate issue.

> I also creating all the machine.h for one ifdef is a little much. But
> it may be needed.

Well, if it was only serial.c I wouldn't propose it.  But not having these
macros makes us add new one-line files every once in a while, which I
think would be good to avoid.

> The
> ifdef in serial.c should be changed to
> 
> #ifdef GRUB_MACHINE_LINUXBIOS
> <then do this>
> #else
> <do what everyone else does>

It's not really LinuxBIOS-specific.  There's a (PC) BIOS-specific way,
which is reading ((short *) 0x400), and a generic one which is hardcoding
the information.

> Also what I don't understand about the current code is that it only
> allows for serial ports
> that have address 0x0400 and above (I'm guessing that is the c
> trickery that is going on
> here) when using addr[unit]. And you declare addr is 0x0400.

No, you missread it.  On BIOS, there's an array of 16-bit addresses at
0x400 (memory space).  On other platforms, we just declare the array in
our code instead (interestingly, the size of the whole array is the
same as the size of a pointer to 0x400 :-)).

-- 
Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call, if you are unable to speak?
(as seen on /.)

Re: [PATCH] fix serial console on LinuxBIOS

[Marco Gerards]

Robert Millan <rmh@aybabtu.com> writes:

> This patch fixes serial console on LinuxBIOS.
>
> I'd appreciate comments (specially on the <grub/machine/machine.h> addition).
>
> -- 
> Robert Millan
>
> <GPLv2> I know my rights; I want my phone call!
> <DRM> What use is a phone call, if you are unable to speak?
> (as seen on /.)
>

No header ;)

> 	* include/grub/i386/efi/machine.h: New file.
> 	* include/grub/i386/linuxbios/machine.h: Likewise.
> 	* include/grub/i386/pc/machine.h: Likewise.
> 	* include/grub/powerpc/ieee1275/machine.h: Likewise.
> 	* include/grub/sparc64/ieee1275/machine.h: Likewise.
>
> 	* term/i386/pc/serial.c: Include <grub/machine/machine.h>.
> 	(serial_hw_io_addr): New variable.
> 	(serial_hw_get_port): Obtain port address from `serial_hw_io_addr'
> 	instead of `(unsigned short *) 0x400'.

This seems fine to me.  What is your intended use for machine.h?  More
than just this?

Perhaps we can even use autoconf to define this in config.h?  That
would be better I think.

--
Marco

Re: [PATCH] fix serial console on LinuxBIOS

[Robert Millan]

On Fri, Nov 09, 2007 at 04:30:44PM +0100, Marco Gerards wrote:
> Robert Millan <rmh@aybabtu.com> writes:
> 
> > This patch fixes serial console on LinuxBIOS.
> >
> > I'd appreciate comments (specially on the <grub/machine/machine.h> addition).
> >
> > -- 
> > Robert Millan
> >
> > <GPLv2> I know my rights; I want my phone call!
> > <DRM> What use is a phone call, if you are unable to speak?
> > (as seen on /.)
> >
> 
> No header ;)

Uhm what header?

> > 	* include/grub/i386/efi/machine.h: New file.
> > 	* include/grub/i386/linuxbios/machine.h: Likewise.
> > 	* include/grub/i386/pc/machine.h: Likewise.
> > 	* include/grub/powerpc/ieee1275/machine.h: Likewise.
> > 	* include/grub/sparc64/ieee1275/machine.h: Likewise.
> >
> > 	* term/i386/pc/serial.c: Include <grub/machine/machine.h>.
> > 	(serial_hw_io_addr): New variable.
> > 	(serial_hw_get_port): Obtain port address from `serial_hw_io_addr'
> > 	instead of `(unsigned short *) 0x400'.
> 
> This seems fine to me.  What is your intended use for machine.h?  More
> than just this?

Situations very similar to this one, in that you just need to change a few
unportable lines while the overall structure of the file remains portable.

> Perhaps we can even use autoconf to define this in config.h?  That
> would be better I think.

autoconf already setups the cpu / machine symlinks.  Why ask it to tell the
same info twice?

-- 
Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call, if you are unable to speak?
(as seen on /.)

Re: [PATCH] fix serial console on LinuxBIOS

[Marco Gerards]

Robert Millan <rmh@aybabtu.com> writes:

> On Fri, Nov 09, 2007 at 04:30:44PM +0100, Marco Gerards wrote:
>> Robert Millan <rmh@aybabtu.com> writes:
>> 
>> > This patch fixes serial console on LinuxBIOS.
>> >
>> > I'd appreciate comments (specially on the <grub/machine/machine.h> addition).
>> >
>> > -- 
>> > Robert Millan
>> >
>> > <GPLv2> I know my rights; I want my phone call!
>> > <DRM> What use is a phone call, if you are unable to speak?
>> > (as seen on /.)
>> >
>> 
>> No header ;)
>
> Uhm what header?

That's what I said ;-)

What I meant was something like:

2007-10-31  Robert Millan  <rmh@aybabtu.com>

>> > 	* include/grub/i386/efi/machine.h: New file.
>> > 	* include/grub/i386/linuxbios/machine.h: Likewise.
>> > 	* include/grub/i386/pc/machine.h: Likewise.
>> > 	* include/grub/powerpc/ieee1275/machine.h: Likewise.
>> > 	* include/grub/sparc64/ieee1275/machine.h: Likewise.
>> >
>> > 	* term/i386/pc/serial.c: Include <grub/machine/machine.h>.
>> > 	(serial_hw_io_addr): New variable.
>> > 	(serial_hw_get_port): Obtain port address from `serial_hw_io_addr'
>> > 	instead of `(unsigned short *) 0x400'.
>> 
>> This seems fine to me.  What is your intended use for machine.h?  More
>> than just this?
>
> Situations very similar to this one, in that you just need to change a few
> unportable lines while the overall structure of the file remains portable.

What I meant was: what should and shouldn't be added to this .h?

>> Perhaps we can even use autoconf to define this in config.h?  That
>> would be better I think.
>
> autoconf already setups the cpu / machine symlinks.  Why ask it to tell the
> same info twice?

Isn't that what you are doing now?  But I have no objections to this
fix, please commit it :-)

--
Marco

Re: [PATCH] fix serial console on LinuxBIOS

[Robert Millan]

On Sat, Nov 10, 2007 at 04:29:07PM +0100, Marco Gerards wrote:
> >> 
> >> No header ;)
> >
> > Uhm what header?
> 
> That's what I said ;-)
> 
> What I meant was something like:
> 
> 2007-10-31  Robert Millan  <rmh@aybabtu.com>

Ah, right.  I tend to skip this because the first part becomes
obsolete easily, the second part is the same for all commits (since
I don't change my name too often ;-)), and the last part too.

> What I meant was: what should and shouldn't be added to this .h?

I wouldn't add anything else.

> > autoconf already setups the cpu / machine symlinks.  Why ask it to tell the
> > same info twice?
> 
> Isn't that what you are doing now?  But I have no objections to this
> fix, please commit it :-)

Ok, done.

-- 
Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call, if you are unable to speak?
(as seen on /.)