Hi,
attached patch removes most of the #ifdefs in libc/console.c, and replaces it with two queues (input, output) where drivers (serial, keyboard, video, usb) can attach. The only things left with #ifdefs are initialization (at some point the drivers must get a chance to register) and usb's polling, which should end up in most tight loops. Maybe there's a better way for usb poll.
I didn't use linker tricks (separate section for the device structs, iterating over that, etc) as I think that would have to be included in the build system of libpayload users. They should have it simple, stupid.
Comments, critique and ideas for improvements welcome.
Signed-off-by: Patrick Georgi patrick.georgi@coresystems.de
=== libc/console.c ================================================================== --- libc/console.c (revision 2218) +++ libc/console.c (local) @@ -31,6 +31,21 @@ #include <libpayload.h> #include <usb/usb.h>
+console_output_driver *console_out = 0; +console_input_driver *console_in = 0; + +void console_add_output_driver(console_output_driver *out) +{ + out->next = console_out; + console_out = out; +} + +void console_add_input_driver(console_input_driver *in) +{ + in->next = console_in; + console_in = in; +} + void console_init(void) { #ifdef CONFIG_VIDEO_CONSOLE @@ -46,15 +61,14 @@
static void device_putchar(unsigned char c) { -#ifdef CONFIG_VIDEO_CONSOLE - video_console_putchar(0x700| c); -#endif -#ifdef CONFIG_SERIAL_CONSOLE - serial_putchar(c); -#endif + console_output_driver *out = console_out; + while (out != 0) { + out->putchar(c); + out = out->next; + } }
-int putchar(int c) +int putchar(unsigned int c) { c &= 0xff; if (c == '\n') @@ -78,19 +92,15 @@
int havekey(void) { -#ifdef CONFIG_USB_HID +#ifdef CONFIG_USB usb_poll(); - if (usbhid_havechar()) - return 1; #endif -#ifdef CONFIG_SERIAL_CONSOLE - if (serial_havechar()) - return 1; -#endif -#ifdef CONFIG_PC_KEYBOARD - if (keyboard_havechar()) - return 1; -#endif + console_input_driver *in = console_in; + while (in != 0) { + if (in->havekey()) + return 1; + in = in->next; + } return 0; }
@@ -101,19 +111,15 @@ int getchar(void) { while (1) { -#ifdef CONFIG_USB_HID +#ifdef CONFIG_USB usb_poll(); - if (usbhid_havechar()) - return usbhid_getchar(); #endif -#ifdef CONFIG_SERIAL_CONSOLE - if (serial_havechar()) - return serial_getchar(); -#endif -#ifdef CONFIG_PC_KEYBOARD - if (keyboard_havechar()) - return keyboard_getchar(); -#endif + console_input_driver *in = console_in; + while (in != 0) { + if (in->havechar()) + return in->getchar(); + in = in->next; + } } }
=== include/libpayload.h ================================================================== --- include/libpayload.h (revision 2218) +++ include/libpayload.h (local) @@ -143,7 +143,7 @@ */ void serial_init(void); void serial_hardware_init(int port, int speed, int word_bits, int parity, int stop_bits); -void serial_putchar(unsigned char c); +void serial_putchar(unsigned int c); int serial_havechar(void); int serial_getchar(void); void serial_clear(void); @@ -190,7 +190,7 @@ * @{ */ void console_init(void); -int putchar(int c); +int putchar(unsigned int c); int puts(const char *s); int havekey(void); int getchar(void); @@ -198,6 +198,22 @@
extern int last_putchar;
+struct console_input_driver_struct; +typedef struct console_input_driver_struct { + struct console_input_driver_struct *next; + int (*havekey) (void); + int (*getchar) (void); +} console_input_driver; + +struct console_output_driver_struct; +typedef struct console_output_driver_struct { + struct console_output_driver_struct *next; + void (*putchar) (unsigned int); +} console_output_driver; + +void console_add_output_driver(console_output_driver *out); +void console_add_input_driver(console_input_driver *in); + #define havechar havekey /** @} */
=== drivers/keyboard.c ================================================================== --- drivers/keyboard.c (revision 2218) +++ drivers/keyboard.c (local) @@ -327,6 +327,11 @@ return 0; }
+static console_input_driver cons = { + .havekey = keyboard_havechar, + .getchar = keyboard_getchar +}; + void keyboard_init(void) { u8 mode; @@ -345,5 +350,7 @@
/* Write the new mode */ keyboard_set_mode(mode); + + console_add_input_driver(&cons); }
=== drivers/serial.c ================================================================== --- drivers/serial.c (revision 2218) +++ drivers/serial.c (local) @@ -58,15 +58,27 @@ outb(reg &= ~0x80, port + 0x03); }
+static console_input_driver consin = { + .havekey = serial_havechar, + .getchar = serial_getchar +}; + +static console_output_driver consout = { + .putchar = serial_putchar +}; + void serial_init(void) { #ifdef CONFIG_SERIAL_SET_SPEED serial_hardware_init(IOBASE, CONFIG_SERIAL_BAUD_RATE, 8, 0, 1); #endif + console_add_input_driver(&consin); + console_add_output_driver(&consout); }
-void serial_putchar(unsigned char c) +void serial_putchar(unsigned int c) { + c &= 0xff; while ((inb(IOBASE + 0x05) & 0x20) == 0) ; outb(c, IOBASE); } === drivers/usb/usbhid.c ================================================================== --- drivers/usb/usbhid.c (revision 2218) +++ drivers/usb/usbhid.c (local) @@ -145,10 +145,21 @@ dev->controller->control (dev, OUT, sizeof (dev_req_t), &dr, 0, 0); }
+static console_input_driver cons = { + .havekey = usbhid_havechar, + .getchar = usbhid_getchar +}; + void usb_hid_init (usbdev_t *dev) {
+ static int installed = 0; + if (!installed) { + installed = 1; + console_add_input_driver (&cons); + } + configuration_descriptor_t *cd = (configuration_descriptor_t*)dev->configuration; interface_descriptor_t *interface = (interface_descriptor_t*)(((char *) cd) + cd->bLength);
=== drivers/video/video.c ================================================================== --- drivers/video/video.c (revision 2218) +++ drivers/video/video.c (local) @@ -111,6 +111,11 @@
void video_console_putchar(unsigned int ch) { + /* replace black-on-black with light-gray-on-black. + do it here, instead of in libc/console.c */ + if ((ch & 0xFF00) == 0) { + ch |= 0x0700; + } switch(ch & 0xFF) { case '\r': cursorx = 0; @@ -165,6 +170,10 @@ video_console_fixup_cursor(); }
+static console_output_driver cons = { + .putchar = video_console_putchar +}; + int video_console_init(void) { int i; @@ -187,6 +196,8 @@ return 0; }
+ console_add_output_driver(&cons); + return 0; }
On 17/10/08 13:25 +0200, Patrick Georgi wrote:
Hi,
attached patch removes most of the #ifdefs in libc/console.c, and replaces it with two queues (input, output) where drivers (serial, keyboard, video, usb) can attach. The only things left with #ifdefs are initialization (at some point the drivers must get a chance to register) and usb's polling, which should end up in most tight loops. Maybe there's a better way for usb poll.
How does poll differ from havechar? Should we add a poll method to the console drivers that is just null on most systems?
I didn't use linker tricks (separate section for the device structs, iterating over that, etc) as I think that would have to be included in the build system of libpayload users. They should have it simple, stupid.
Yeah, I don't think linker tricks will buy us anything. One thing that concerns me is ordering. The input should go from least reliable to most reliable (ie - from USB to port 60/64). The way you have organized it enforces that ordering. Linker tricks wouldn't.
Comments, critique and ideas for improvements welcome.
I like this design. My thought was to use a static array of structs in console.c. I like your design better because it allows for us to completely skip over an input method if it fails init. I struggled with this very problem yesterday on a system without a superio - port 60/64 hangs. My solution was to do a "does the controller exist" check in every function - your changes makes that only nessesary in the the init function.
Signed-off-by: Patrick Georgi patrick.georgi@coresystems.de
Comments below.
=== libc/console.c
--- libc/console.c (revision 2218) +++ libc/console.c (local) @@ -31,6 +31,21 @@ #include <libpayload.h> #include <usb/usb.h>
+console_output_driver *console_out = 0; +console_input_driver *console_in = 0;
If not asssigned, gcc will automatically put these variables in the .bss section. If they are assigned (even to zero), some compilers will put them in the .data section increasing the size of the binary. Just let them float free.. :)
+void console_add_output_driver(console_output_driver *out) +{
- out->next = console_out;
- console_out = out;
+}
+void console_add_input_driver(console_input_driver *in) +{
- in->next = console_in;
- console_in = in;
+}
void console_init(void) { #ifdef CONFIG_VIDEO_CONSOLE @@ -46,15 +61,14 @@
static void device_putchar(unsigned char c) { -#ifdef CONFIG_VIDEO_CONSOLE
- video_console_putchar(0x700| c);
-#endif -#ifdef CONFIG_SERIAL_CONSOLE
- serial_putchar(c);
-#endif
- console_output_driver *out = console_out;
- while (out != 0) {
out->putchar(c);out = out->next;
for (out = console_out; out != out; out = out->next) out->putchar(c);
I'm just picking nits - either way is acceptable for sure.
- }
}
-int putchar(int c) +int putchar(unsigned int c) { c &= 0xff; if (c == '\n') @@ -78,19 +92,15 @@
int havekey(void) { -#ifdef CONFIG_USB_HID +#ifdef CONFIG_USB usb_poll();
- if (usbhid_havechar())
return 1;#endif -#ifdef CONFIG_SERIAL_CONSOLE
- if (serial_havechar())
return 1;-#endif -#ifdef CONFIG_PC_KEYBOARD
- if (keyboard_havechar())
return 1;-#endif
- console_input_driver *in = console_in;
- while (in != 0) {
if (in->havekey())return 1;in = in->next;- } return 0;
}
@@ -101,19 +111,15 @@ int getchar(void) { while (1) { -#ifdef CONFIG_USB_HID +#ifdef CONFIG_USB usb_poll();
if (usbhid_havechar())return usbhid_getchar();#endif -#ifdef CONFIG_SERIAL_CONSOLE
if (serial_havechar())return serial_getchar();-#endif -#ifdef CONFIG_PC_KEYBOARD
if (keyboard_havechar())return keyboard_getchar();-#endif
console_input_driver *in = console_in;while (in != 0) {if (in->havechar())return in->getchar();in = in->next;}}
=== include/libpayload.h
--- include/libpayload.h (revision 2218) +++ include/libpayload.h (local) @@ -143,7 +143,7 @@ */ void serial_init(void); void serial_hardware_init(int port, int speed, int word_bits, int parity, int stop_bits); -void serial_putchar(unsigned char c); +void serial_putchar(unsigned int c); int serial_havechar(void); int serial_getchar(void); void serial_clear(void); @@ -190,7 +190,7 @@
- @{
*/ void console_init(void); -int putchar(int c); +int putchar(unsigned int c); int puts(const char *s); int havekey(void); int getchar(void); @@ -198,6 +198,22 @@
extern int last_putchar;
+struct console_input_driver_struct; +typedef struct console_input_driver_struct {
- struct console_input_driver_struct *next;
- int (*havekey) (void);
- int (*getchar) (void);
+} console_input_driver;
Do we have to typedef these? I hate typedefs. 'struct console_input_driver' is perfect to me - it tells me everything I need to know about the variable.
+struct console_output_driver_struct; +typedef struct console_output_driver_struct {
- struct console_output_driver_struct *next;
- void (*putchar) (unsigned int);
+} console_output_driver;
+void console_add_output_driver(console_output_driver *out); +void console_add_input_driver(console_input_driver *in);
#define havechar havekey /** @} */
=== drivers/keyboard.c
--- drivers/keyboard.c (revision 2218) +++ drivers/keyboard.c (local) @@ -327,6 +327,11 @@ return 0; }
+static console_input_driver cons = {
- .havekey = keyboard_havechar,
- .getchar = keyboard_getchar
+};
void keyboard_init(void) { u8 mode; @@ -345,5 +350,7 @@
/* Write the new mode */ keyboard_set_mode(mode);
- console_add_input_driver(&cons);
}
=== drivers/serial.c
--- drivers/serial.c (revision 2218) +++ drivers/serial.c (local) @@ -58,15 +58,27 @@ outb(reg &= ~0x80, port + 0x03); }
+static console_input_driver consin = {
- .havekey = serial_havechar,
- .getchar = serial_getchar
+};
+static console_output_driver consout = {
- .putchar = serial_putchar
+};
void serial_init(void) { #ifdef CONFIG_SERIAL_SET_SPEED serial_hardware_init(IOBASE, CONFIG_SERIAL_BAUD_RATE, 8, 0, 1); #endif
- console_add_input_driver(&consin);
- console_add_output_driver(&consout);
}
-void serial_putchar(unsigned char c) +void serial_putchar(unsigned int c) {
- c &= 0xff; while ((inb(IOBASE + 0x05) & 0x20) == 0) ; outb(c, IOBASE);
} === drivers/usb/usbhid.c ================================================================== --- drivers/usb/usbhid.c (revision 2218) +++ drivers/usb/usbhid.c (local) @@ -145,10 +145,21 @@ dev->controller->control (dev, OUT, sizeof (dev_req_t), &dr, 0, 0); }
+static console_input_driver cons = {
- .havekey = usbhid_havechar,
- .getchar = usbhid_getchar
+};
void usb_hid_init (usbdev_t *dev) {
- static int installed = 0;
- if (!installed) {
installed = 1;console_add_input_driver (&cons);- }
- configuration_descriptor_t *cd = (configuration_descriptor_t*)dev->configuration; interface_descriptor_t *interface = (interface_descriptor_t*)(((char *) cd) + cd->bLength);
=== drivers/video/video.c
--- drivers/video/video.c (revision 2218) +++ drivers/video/video.c (local) @@ -111,6 +111,11 @@
void video_console_putchar(unsigned int ch) {
- /* replace black-on-black with light-gray-on-black.
do it here, instead of in libc/console.c */- if ((ch & 0xFF00) == 0) {
ch |= 0x0700;- } switch(ch & 0xFF) { case '\r': cursorx = 0;
@@ -165,6 +170,10 @@ video_console_fixup_cursor(); }
+static console_output_driver cons = {
- .putchar = video_console_putchar
+};
int video_console_init(void) { int i; @@ -187,6 +196,8 @@ return 0; }
console_add_output_driver(&cons);- return 0;
}
-- coreboot mailing list: coreboot@coreboot.org http://www.coreboot.org/mailman/listinfo/coreboot
Jordan Crouse schrieb:
How does poll differ from havechar? Should we add a poll method to the console drivers that is just null on most systems?
The usb poll code handles all usb related issues (eg. attach/detach of devices, handling interrupt queues like with usb hid). It should end up in all tight loops that take a while. If anything, an "idle handler" would be the right place, I think.
I'll handle all your other suggestions and post another patch.
Regards, Patrick
Updated patch attached. Same changes as before, but with some more style (Thanks Jordan!)
Signed-off-by: Patrick Georgi patrick.georgi@coresystems.de
=== drivers/keyboard.c ================================================================== --- drivers/keyboard.c (revision 2252) +++ drivers/keyboard.c (local) @@ -326,6 +326,11 @@ return 0; }
+static struct console_input_driver cons = { + .havekey = keyboard_havechar, + .getchar = keyboard_getchar +}; + void keyboard_init(void) { u8 mode; @@ -350,5 +355,7 @@
/* Write the new mode */ keyboard_set_mode(mode); + + console_add_input_driver(&cons); }
=== drivers/serial.c ================================================================== --- drivers/serial.c (revision 2252) +++ drivers/serial.c (local) @@ -58,15 +58,27 @@ outb(reg &= ~0x80, port + 0x03); }
+static struct console_input_driver consin = { + .havekey = serial_havechar, + .getchar = serial_getchar +}; + +static struct console_output_driver consout = { + .putchar = serial_putchar +}; + void serial_init(void) { #ifdef CONFIG_SERIAL_SET_SPEED serial_hardware_init(IOBASE, CONFIG_SERIAL_BAUD_RATE, 8, 0, 1); #endif + console_add_input_driver(&consin); + console_add_output_driver(&consout); }
-void serial_putchar(unsigned char c) +void serial_putchar(unsigned int c) { + c &= 0xff; while ((inb(IOBASE + 0x05) & 0x20) == 0) ; outb(c, IOBASE); } === drivers/usb/usbhid.c ================================================================== --- drivers/usb/usbhid.c (revision 2252) +++ drivers/usb/usbhid.c (local) @@ -145,10 +145,21 @@ dev->controller->control (dev, OUT, sizeof (dev_req_t), &dr, 0, 0); }
+static struct console_input_driver cons = { + .havekey = usbhid_havechar, + .getchar = usbhid_getchar +}; + void usb_hid_init (usbdev_t *dev) {
+ static int installed = 0; + if (!installed) { + installed = 1; + console_add_input_driver (&cons); + } + configuration_descriptor_t *cd = (configuration_descriptor_t*)dev->configuration; interface_descriptor_t *interface = (interface_descriptor_t*)(((char *) cd) + cd->bLength);
=== drivers/video/video.c ================================================================== --- drivers/video/video.c (revision 2252) +++ drivers/video/video.c (local) @@ -111,6 +111,11 @@
void video_console_putchar(unsigned int ch) { + /* replace black-on-black with light-gray-on-black. + do it here, instead of in libc/console.c */ + if ((ch & 0xFF00) == 0) { + ch |= 0x0700; + } switch(ch & 0xFF) { case '\r': cursorx = 0; @@ -165,6 +170,10 @@ video_console_fixup_cursor(); }
+static struct console_output_driver cons = { + .putchar = video_console_putchar +}; + int video_console_init(void) { int i; @@ -187,6 +196,8 @@ return 0; }
+ console_add_output_driver(&cons); + return 0; }
=== include/libpayload.h ================================================================== --- include/libpayload.h (revision 2252) +++ include/libpayload.h (local) @@ -145,7 +145,7 @@ */ void serial_init(void); void serial_hardware_init(int port, int speed, int word_bits, int parity, int stop_bits); -void serial_putchar(unsigned char c); +void serial_putchar(unsigned int c); int serial_havechar(void); int serial_getchar(void); void serial_clear(void); @@ -192,7 +192,7 @@ * @{ */ void console_init(void); -int putchar(int c); +int putchar(unsigned int c); int puts(const char *s); int havekey(void); int getchar(void); @@ -200,6 +200,22 @@
extern int last_putchar;
+struct console_input_driver; +struct console_input_driver { + struct console_input_driver *next; + int (*havekey) (void); + int (*getchar) (void); +}; + +struct console_output_driver; +struct console_output_driver { + struct console_output_driver *next; + void (*putchar) (unsigned int); +}; + +void console_add_output_driver(struct console_output_driver *out); +void console_add_input_driver(struct console_input_driver *in); + #define havechar havekey /** @} */
=== libc/console.c ================================================================== --- libc/console.c (revision 2252) +++ libc/console.c (local) @@ -31,6 +31,21 @@ #include <libpayload.h> #include <usb/usb.h>
+struct console_output_driver *console_out; +struct console_input_driver *console_in; + +void console_add_output_driver(struct console_output_driver *out) +{ + out->next = console_out; + console_out = out; +} + +void console_add_input_driver(struct console_input_driver *in) +{ + in->next = console_in; + console_in = in; +} + void console_init(void) { #ifdef CONFIG_VIDEO_CONSOLE @@ -46,15 +61,12 @@
static void device_putchar(unsigned char c) { -#ifdef CONFIG_VIDEO_CONSOLE - video_console_putchar(0x700| c); -#endif -#ifdef CONFIG_SERIAL_CONSOLE - serial_putchar(c); -#endif + struct console_output_driver *out; + for (out = console_out; out != 0; out = out->next) + out->putchar(c); }
-int putchar(int c) +int putchar(unsigned int c) { c &= 0xff; if (c == '\n') @@ -78,19 +90,13 @@
int havekey(void) { -#ifdef CONFIG_USB_HID +#ifdef CONFIG_USB usb_poll(); - if (usbhid_havechar()) - return 1; #endif -#ifdef CONFIG_SERIAL_CONSOLE - if (serial_havechar()) - return 1; -#endif -#ifdef CONFIG_PC_KEYBOARD - if (keyboard_havechar()) - return 1; -#endif + struct console_input_driver *in; + for (in = console_in; in != 0; in = in->next) + if (in->havekey()) + return 1; return 0; }
@@ -101,19 +107,13 @@ int getchar(void) { while (1) { -#ifdef CONFIG_USB_HID +#ifdef CONFIG_USB usb_poll(); - if (usbhid_havechar()) - return usbhid_getchar(); #endif -#ifdef CONFIG_SERIAL_CONSOLE - if (serial_havechar()) - return serial_getchar(); -#endif -#ifdef CONFIG_PC_KEYBOARD - if (keyboard_havechar()) - return keyboard_getchar(); -#endif + struct console_input_driver *in = console_in; + for (in = console_in; in != 0; in = in->next) + if (in->havechar()) + return in->getchar(); } }
On 21/10/08 15:08 +0200, Patrick Georgi wrote:
Updated patch attached. Same changes as before, but with some more style (Thanks Jordan!)
Signed-off-by: Patrick Georgi patrick.georgi@coresystems.de
Excellent - I like this.
Signed-off-by: Jordan Crouse jordan.crouse@amd.com
=== drivers/keyboard.c
--- drivers/keyboard.c (revision 2252) +++ drivers/keyboard.c (local) @@ -326,6 +326,11 @@ return 0; }
+static struct console_input_driver cons = {
- .havekey = keyboard_havechar,
- .getchar = keyboard_getchar
+};
void keyboard_init(void) { u8 mode; @@ -350,5 +355,7 @@
/* Write the new mode */ keyboard_set_mode(mode);
- console_add_input_driver(&cons);
}
=== drivers/serial.c
--- drivers/serial.c (revision 2252) +++ drivers/serial.c (local) @@ -58,15 +58,27 @@ outb(reg &= ~0x80, port + 0x03); }
+static struct console_input_driver consin = {
- .havekey = serial_havechar,
- .getchar = serial_getchar
+};
+static struct console_output_driver consout = {
- .putchar = serial_putchar
+};
void serial_init(void) { #ifdef CONFIG_SERIAL_SET_SPEED serial_hardware_init(IOBASE, CONFIG_SERIAL_BAUD_RATE, 8, 0, 1); #endif
- console_add_input_driver(&consin);
- console_add_output_driver(&consout);
}
-void serial_putchar(unsigned char c) +void serial_putchar(unsigned int c) {
- c &= 0xff; while ((inb(IOBASE + 0x05) & 0x20) == 0) ; outb(c, IOBASE);
} === drivers/usb/usbhid.c ================================================================== --- drivers/usb/usbhid.c (revision 2252) +++ drivers/usb/usbhid.c (local) @@ -145,10 +145,21 @@ dev->controller->control (dev, OUT, sizeof (dev_req_t), &dr, 0, 0); }
+static struct console_input_driver cons = {
- .havekey = usbhid_havechar,
- .getchar = usbhid_getchar
+};
void usb_hid_init (usbdev_t *dev) {
- static int installed = 0;
- if (!installed) {
installed = 1;console_add_input_driver (&cons);- }
- configuration_descriptor_t *cd = (configuration_descriptor_t*)dev->configuration; interface_descriptor_t *interface = (interface_descriptor_t*)(((char *) cd) + cd->bLength);
=== drivers/video/video.c
--- drivers/video/video.c (revision 2252) +++ drivers/video/video.c (local) @@ -111,6 +111,11 @@
void video_console_putchar(unsigned int ch) {
- /* replace black-on-black with light-gray-on-black.
do it here, instead of in libc/console.c */- if ((ch & 0xFF00) == 0) {
ch |= 0x0700;- } switch(ch & 0xFF) { case '\r': cursorx = 0;
@@ -165,6 +170,10 @@ video_console_fixup_cursor(); }
+static struct console_output_driver cons = {
- .putchar = video_console_putchar
+};
int video_console_init(void) { int i; @@ -187,6 +196,8 @@ return 0; }
console_add_output_driver(&cons);- return 0;
}
=== include/libpayload.h
--- include/libpayload.h (revision 2252) +++ include/libpayload.h (local) @@ -145,7 +145,7 @@ */ void serial_init(void); void serial_hardware_init(int port, int speed, int word_bits, int parity, int stop_bits); -void serial_putchar(unsigned char c); +void serial_putchar(unsigned int c); int serial_havechar(void); int serial_getchar(void); void serial_clear(void); @@ -192,7 +192,7 @@
- @{
*/ void console_init(void); -int putchar(int c); +int putchar(unsigned int c); int puts(const char *s); int havekey(void); int getchar(void); @@ -200,6 +200,22 @@
extern int last_putchar;
+struct console_input_driver; +struct console_input_driver {
- struct console_input_driver *next;
- int (*havekey) (void);
- int (*getchar) (void);
+};
+struct console_output_driver; +struct console_output_driver {
- struct console_output_driver *next;
- void (*putchar) (unsigned int);
+};
+void console_add_output_driver(struct console_output_driver *out); +void console_add_input_driver(struct console_input_driver *in);
#define havechar havekey /** @} */
=== libc/console.c
--- libc/console.c (revision 2252) +++ libc/console.c (local) @@ -31,6 +31,21 @@ #include <libpayload.h> #include <usb/usb.h>
+struct console_output_driver *console_out; +struct console_input_driver *console_in;
+void console_add_output_driver(struct console_output_driver *out) +{
- out->next = console_out;
- console_out = out;
+}
+void console_add_input_driver(struct console_input_driver *in) +{
- in->next = console_in;
- console_in = in;
+}
void console_init(void) { #ifdef CONFIG_VIDEO_CONSOLE @@ -46,15 +61,12 @@
static void device_putchar(unsigned char c) { -#ifdef CONFIG_VIDEO_CONSOLE
- video_console_putchar(0x700| c);
-#endif -#ifdef CONFIG_SERIAL_CONSOLE
- serial_putchar(c);
-#endif
- struct console_output_driver *out;
- for (out = console_out; out != 0; out = out->next)
out->putchar(c);}
-int putchar(int c) +int putchar(unsigned int c) { c &= 0xff; if (c == '\n') @@ -78,19 +90,13 @@
int havekey(void) { -#ifdef CONFIG_USB_HID +#ifdef CONFIG_USB usb_poll();
- if (usbhid_havechar())
return 1;#endif -#ifdef CONFIG_SERIAL_CONSOLE
- if (serial_havechar())
return 1;-#endif -#ifdef CONFIG_PC_KEYBOARD
- if (keyboard_havechar())
return 1;-#endif
- struct console_input_driver *in;
- for (in = console_in; in != 0; in = in->next)
if (in->havekey())return 1;}
@@ -101,19 +107,13 @@ int getchar(void) { while (1) { -#ifdef CONFIG_USB_HID +#ifdef CONFIG_USB usb_poll();
if (usbhid_havechar())return usbhid_getchar();#endif -#ifdef CONFIG_SERIAL_CONSOLE
if (serial_havechar())return serial_getchar();-#endif -#ifdef CONFIG_PC_KEYBOARD
if (keyboard_havechar())return keyboard_getchar();-#endif
struct console_input_driver *in = console_in;for (in = console_in; in != 0; in = in->next)if (in->havechar())return in->getchar();}
On Fri, Oct 17, 2008 at 4:25 AM, Patrick Georgi patrick@georgi-clan.de wrote:
I didn't use linker tricks (separate section for the device structs, iterating over that, etc) as I think that would have to be included in the build system of libpayload users. They should have it simple, stupid.
woo hoo! no linker tricks! This is a really, really nice design IMHO.
ron
-
Jordan Crouse -
Patrick Georgi -
ron minnich