Patchwork console code cleanup

login
register
about
Submitter Stefan Reinauer
Date 2009-10-24 01:12:51
Message ID <4AE25493.3010202@coresystems.de>
Download mbox | patch
Permalink /patch/471/
State Accepted
Headers show

Comments

Stefan Reinauer - 2009-10-24 01:12:51
See patch
Carl-Daniel Hailfinger - 2009-10-24 09:07:33
On 24.10.2009 03:12, Stefan Reinauer wrote:
> re-order console output functions, add proper prototypes, 
> drop claim that our files were blatantly copied, because they have been
> rewritten a very long time ago.
>
> Signed-off-by: Stefan Reinauer <stepan@coresystems.de>
>   

I don't have a v2 tree handy right now, so I can't diff the result of
your patch with my v3 tree. How are your new code and the v3 code
related? Is this a rewrite of the existing v2 code or really just a
reodering? (diff generated a patch which is not exactly readable, but
that's not your fault.)

Looks good, though.

If you drop the unrelated ACPI hunk in src/config/Options.lb, this is
Acked-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net>

Regards,
Carl-Daniel
Stefan Reinauer - 2009-10-24 13:08:32
Carl-Daniel Hailfinger wrote:
> On 24.10.2009 03:12, Stefan Reinauer wrote:
>   
>> re-order console output functions, add proper prototypes, 
>> drop claim that our files were blatantly copied, because they have been
>> rewritten a very long time ago.
>>
>> Signed-off-by: Stefan Reinauer <stepan@coresystems.de>
>>   
>>     
>
> I don't have a v2 tree handy right now, so I can't diff the result of
> your patch with my v3 tree. How are your new code and the v3 code
> related? Is this a rewrite of the existing v2 code or really just a
> reodering? (diff generated a patch which is not exactly readable, but
> that's not your fault.)
>   
It's just moving functions around. Oops, and the changelog missed this:
it fixes a locking issue in sprintf.

> Looks good, though.
>
> If you drop the unrelated ACPI hunk in src/config/Options.lb, this is
> Acked-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net>
>   
This sneaked in accidentally...ACPI hunk dropped. Thanks, r4837

Patch

Index: src/include/console/vtxprintf.h
===================================================================
--- src/include/console/vtxprintf.h	(revision 0)
+++ src/include/console/vtxprintf.h	(revision 0)
@@ -0,0 +1,29 @@ 
+/*
+ * This file is part of the coreboot project.
+ * 
+ * Copyright (C) 2009 coresystems GmbH
+ *
+ * This program 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; version 2 of
+ * the License.
+ *
+ * This program 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 this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston,
+ * MA 02110-1301 USA
+ */
+
+#ifndef __CONSOLE_VTXPRINTF_H
+#define __CONSOLE_VTXPRINTF_H
+
+#include <stdarg.h>
+
+int vtxprintf(void (*tx_byte)(unsigned char byte), const char *fmt, va_list args);
+
+#endif
Index: src/console/vtxprintf.c
===================================================================
--- src/console/vtxprintf.c	(revision 4831)
+++ src/console/vtxprintf.c	(working copy)
@@ -7,50 +7,13 @@ 
 #include <stdarg.h>
 #include <string.h>
 #include <div64.h>
+#include <console/vtxprintf.h>
 
 /* haha, don't need ctype.c */
 #define isdigit(c)	((c) >= '0' && (c) <= '9')
 #define is_digit isdigit
 #define isxdigit(c)	(((c) >= '0' && (c) <= '9') || ((c) >= 'a' && (c) <= 'f') || ((c) >= 'A' && (c) <= 'F'))
 
-#if 0
-/* We are using number() instead. So this code is obsoleted and should
- * probably go away.
- */
-
-static unsigned long simple_strtoul(const char *cp,char **endp,unsigned int base)
-{
-	unsigned long result = 0,value;
-
-	if (!base) {
-		base = 10;
-		if (*cp == '0') {
-			base = 8;
-			cp++;
-			if ((*cp == 'x') && isxdigit(cp[1])) {
-				cp++;
-				base = 16;
-			}
-		}
-	}
-	while (isxdigit(*cp) && (value = isdigit(*cp) ? *cp-'0' : (islower(*cp)
-	    ? toupper(*cp) : *cp)-'A'+10) < base) {
-		result = result*base + value;
-		cp++;
-	}
-	if (endp)
-		*endp = (char *)cp;
-	return result;
-}
-
-static long simple_strtol(const char *cp,char **endp,unsigned int base)
-{
-	if(*cp=='-')
-		return -simple_strtoul(cp+1,endp,base);
-	return simple_strtoul(cp,endp,base);
-}
-#endif
-
 static int skip_atoi(const char **s)
 {
 	int i=0;
Index: src/console/Makefile.inc
===================================================================
--- src/console/Makefile.inc	(revision 4831)
+++ src/console/Makefile.inc	(working copy)
@@ -4,7 +4,6 @@ 
 obj-y += vsprintf.o
 
 initobj-y += vtxprintf.o
-initobj-y += vsprintf.o
 
 driver-$(CONFIG_CONSOLE_SERIAL8250) += uart8250_console.o
 driver-$(CONFIG_USBDEBUG_DIRECT) += usbdebug_direct_console.o
Index: src/console/printk.c
===================================================================
--- src/console/printk.c	(revision 4831)
+++ src/console/printk.c	(working copy)
@@ -7,6 +7,7 @@ 
 
 #include <stdarg.h>
 #include <smp/spinlock.h>
+#include <console/vtxprintf.h>
 #include <console/console.h>
 
 /* printk's without a loglevel use this.. */
@@ -22,9 +23,6 @@ 
 int minimum_console_loglevel = MINIMUM_CONSOLE_LOGLEVEL;
 int default_console_loglevel = CONFIG_DEFAULT_CONSOLE_LOGLEVEL;
 
-void display(char*);
-extern int vtxprintf(void (*)(unsigned char), const char *, va_list);
-
 static spinlock_t console_lock = SPIN_LOCK_UNLOCKED;
 
 int do_printk(int msg_level, const char *fmt, ...)
Index: src/console/vsprintf.c
===================================================================
--- src/console/vsprintf.c	(revision 4831)
+++ src/console/vsprintf.c	(working copy)
@@ -1,45 +1,61 @@ 
 /*
- *  linux/lib/vsprintf.c
+ * This file is part of the coreboot project.
+ * 
+ * Copyright (C) 2009 coresystems GmbH
  *
- *  Copyright (C) 1991, 1992  Linus Torvalds
+ * This program 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; version 2 of
+ * the License.
+ *
+ * This program 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 this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston,
+ * MA 02110-1301 USA
  */
 
-/* vsprintf.c -- Lars Wirzenius & Linus Torvalds. */
-/*
- * Wirzenius wrote this portably, Torvalds fucked it up :-)
- */
 #include <stdarg.h>
-#include <string.h>
+#include <smp/spinlock.h>
+#include <console/vtxprintf.h>
 
-int vtxprintf(void (*tx_byte)(unsigned char byte), const char *fmt, va_list args);
+static spinlock_t vsprintf_lock = SPIN_LOCK_UNLOCKED;
 
-int vsprintf(char * buf, const char *fmt, va_list args)
+static char *str_buf;
+
+static void str_tx_byte(unsigned char byte)
 {
-	char *str_buf;
+	*str_buf = byte;
+	str_buf++;
+}
 
-	/* this function is only used by vsprintf.
-	   To keep str_buf local (for reentrancy
-	   and to avoid .bss use, nest it */
-	void str_tx_byte(unsigned char byte)
-	{
-		*str_buf = byte;
-		str_buf++;
-	}
+static int vsprintf(char *buf, const char *fmt, va_list args)
+{
+	int i;
 
-	int i;
+	spin_lock(&vsprintf_lock);
+
 	str_buf = buf;
 	i = vtxprintf(str_tx_byte, fmt, args);
 	*str_buf = '\0';
+
+	spin_unlock(&vsprintf_lock);
+
 	return i;
 }
 
-int sprintf(char * buf, const char *fmt, ...)
+int sprintf(char *buf, const char *fmt, ...)
 {
 	va_list args;
 	int i;
 
 	va_start(args, fmt);
-	i=vsprintf(buf,fmt,args);
+	i = vsprintf(buf, fmt, args);
 	va_end(args);
+
 	return i;
 }
Index: src/config/Options.lb
===================================================================
--- src/config/Options.lb	(revision 4831)
+++ src/config/Options.lb	(working copy)
@@ -862,10 +862,16 @@ 
 	comment "Define to build ACPI tables"
 end
 
+define CONFIG_HAVE_ACPI_SLIC
+	default 0
+	export always
+	comment "Define to build ACPI SLIC table"
+end
+
 define CONFIG_HAVE_ACPI_RESUME
 	default 0
 	export always
-	comment "Define to build ACPI with resume support"
+	comment "Enable for ACPI S3 Support"
 end
 
 define CONFIG_ACPI_SSDTX_NUM
Index: src/arch/ppc/lib/printk_init.c
===================================================================
--- src/arch/ppc/lib/printk_init.c	(revision 4831)
+++ src/arch/ppc/lib/printk_init.c	(working copy)
@@ -1,26 +1,34 @@ 
 /*
- *  blantantly copied from linux/kernel/printk.c
+ * This file is part of the coreboot project.
+ * 
+ * This program 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; version 2 of
+ * the License.
  *
- *  Copyright (C) 1991, 1992  Linus Torvalds
+ * This program 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 this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston,
+ * MA 02110-1301 USA
  */
+
 #include <stdarg.h>
+#include <console/vtxprintf.h>
 #include <console/loglevel.h>
+#include <uart8250.h>
 
-/* printk's without a loglevel use this.. */
-#define DEFAULT_MESSAGE_LOGLEVEL 4 /* BIOS_WARNING */
-
-/* Keep together for sysctl support */
-
 int console_loglevel = CONFIG_DEFAULT_CONSOLE_LOGLEVEL;
 
-extern int vtxprintf(void (*)(unsigned char), const char *, va_list);
-extern void uart8250_tx_byte(unsigned, unsigned char);
-
 void console_tx_byte(unsigned char byte)
 {
 	if (byte == '\n')
 		uart8250_tx_byte(CONFIG_TTYS0_BASE, '\r');
+
 	uart8250_tx_byte(CONFIG_TTYS0_BASE, byte);
 }
 
Index: src/arch/i386/include/stdint.h
===================================================================
--- src/arch/i386/include/stdint.h	(revision 4831)
+++ src/arch/i386/include/stdint.h	(working copy)
@@ -1,7 +1,7 @@ 
 #ifndef I386_STDINT_H
 #define I386_STDINT_H
 
-#if defined(__GNUC__) && !defined(__ROMCC__)
+#if defined(__GNUC__)
 #define __HAVE_LONG_LONG__ 1
 #else
 #define __HAVE_LONG_LONG__ 0
Index: src/arch/i386/lib/console.c
===================================================================
--- src/arch/i386/lib/console.c	(revision 4831)
+++ src/arch/i386/lib/console.c	(working copy)
@@ -1,18 +1,9 @@ 
 #include <console/loglevel.h>
 
 #if CONFIG_USE_PRINTK_IN_CAR == 0
-static void __console_tx_byte(unsigned char byte)
-{
-	uart_tx_byte(byte);
-}
-
 #include "console_print.c"
-
-#else  
-/* CONFIG_USE_PRINTK_IN_CAR == 1 */
-
+#else  /* CONFIG_USE_PRINTK_IN_CAR == 1 */
 #include "console_printk.c"
-
 #endif /* CONFIG_USE_PRINTK_IN_CAR */
 
 #ifndef COREBOOT_EXTRA_VERSION
@@ -32,7 +23,7 @@ 
 }
 
 
-static void die(const char *str)
+void die(const char *str)
 {
 	print_emerg(str);
 	do {
Index: src/arch/i386/lib/console_print.c
===================================================================
--- src/arch/i386/lib/console_print.c	(revision 4831)
+++ src/arch/i386/lib/console_print.c	(working copy)
@@ -1,3 +1,7 @@ 
+static void __console_tx_byte(unsigned char byte)
+{
+	uart_tx_byte(byte);
+}
 
 static void __console_tx_nibble(unsigned nibble)
 {
Index: src/arch/i386/lib/printk_init.c
===================================================================
--- src/arch/i386/lib/printk_init.c	(revision 4831)
+++ src/arch/i386/lib/printk_init.c	(working copy)
@@ -1,31 +1,42 @@ 
 /*
- *  blantantly copied from linux/kernel/printk.c
+ * This file is part of the coreboot project.
+ * 
+ * This program 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; version 2 of
+ * the License.
  *
- *  Copyright (C) 1991, 1992  Linus Torvalds
+ * This program 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.
  *
- *  by yhlu moved from arch/ppc/lib/printk_init.c, removed the global variable console_loglevel
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston,
+ * MA 02110-1301 USA
  */
+
 #include <stdarg.h>
+#include <console/vtxprintf.h>
 #include <console/loglevel.h>
+#include <uart8250.h>
 
-/* printk's without a loglevel use this.. */
-#define DEFAULT_MESSAGE_LOGLEVEL 4 /* BIOS_WARNING */
-
-/* Keep together for sysctl support */
-/* Using an global varible can cause problem when we reset the stack from cache as ram to ram*/
+/* Using a global varible can cause problems when we reset the stack 
+ * from cache as ram to ram. If we make this a define USE_SHARED_STACK
+ * we could use the same code on all architectures.
+ */
 #if 0
 int console_loglevel = CONFIG_DEFAULT_CONSOLE_LOGLEVEL;
 #else
-#define console_loglevel ASM_CONSOLE_LOGLEVEL
+#define console_loglevel CONFIG_DEFAULT_CONSOLE_LOGLEVEL
 #endif
 
-extern int vtxprintf(void (*)(unsigned char), const char *, va_list);
-extern void uart8250_tx_byte(unsigned, unsigned char);
-
 void console_tx_byte(unsigned char byte)
 {
 	if (byte == '\n')
 		uart8250_tx_byte(CONFIG_TTYS0_BASE, '\r');
+
 	uart8250_tx_byte(CONFIG_TTYS0_BASE, byte);
 }