Messages in this thread |  | | Date | Fri, 30 Nov 2001 19:31:55 +0100 | From | Martin Dalecki <> | Subject | Re: Coding style - a non-issue |
| |
Russell King wrote: > > On Fri, Nov 30, 2001 at 06:49:01PM +0100, Martin Dalecki wrote: > > Well sombeody really cares apparently! Thank's. > > Currently its a restructuring of serial.c to allow different uart type > ports to be driven without duplicating serial.c many times over. (the > generic_serial didn't hack it either). > > > Any pointers where to ahve a look at it? > > I should probably put this on a web page somewhere 8) > > :pserver:cvs@pubcvs.arm.linux.org.uk:/mnt/src/cvsroot, module serial > (no password) > > > BTW> Did you consider ther misc device idea? (Hooking serial at to the > > misc device stuff). > > I just caught that comment - I'll await your reply.
I'm just right now looking at the code found above. First of all: GREAT WORK! And then you are right a bit, I just don't see too much code duplication between the particular drivers there. However I still don't see the need to carrige the whole tty stuff just to drive a mouse... but I don't see a solution right now. I wouldn't be good to introduce a scatter heap of "micro" driver modules like the ALSA people did as well too..
However in serial/linux/drivers/serial/serial_clps711x.c the following wonders me a bit:
#ifdef CONFIG_DEVFS_FS normal_name: SERIAL_CLPS711X_NAME, callout_name: CALLOUT_CLPS711X_NAME, #else normal_name: SERIAL_CLPS711X_NAME, callout_name: CALLOUT_CLPS711X_NAME, #endif
What is the ifdef supposed to be good for?
One other suggestion serial_code.c could be just serial.c or code.c or main.c, since _xxxx.c should distinguish between particulart devices. It was a bit clumy to find the entry-point for me... And then we have in uart_register_driver:
normal->open = uart_open; normal->close = uart_close; normal->write = uart_write; normal->put_char = uart_put_char; normal->flush_chars = uart_flush_chars; normal->write_room = uart_write_room; normal->chars_in_buffer = uart_chars_in_buffer; normal->flush_buffer = uart_flush_buffer; normal->ioctl = uart_ioctl; normal->throttle = uart_throttle; normal->unthrottle = uart_unthrottle; normal->send_xchar = uart_send_xchar; normal->set_termios = uart_set_termios; normal->stop = uart_stop; normal->start = uart_start; normal->hangup = uart_hangup; normal->break_ctl = uart_break_ctl; normal->wait_until_sent = uart_wait_until_sent;
And so on....
Why not do:
{ static strcut tty_driver _normal = { open: uart_open, close: uart_close, ... };
...
*normal = _normal; } ... for the static stuff first and only initialize the dynamically calculated addresses dynamically later, like the double refferences... This would save already *quite a bit* of .text ;-).
Yeah I know I'm a bit perverse about optimizations....
You already do it for the callout device nearly this way:
/* * The callout device is just like the normal device except for * the major number and the subtype code. */ *callout = *normal; callout->name = drv->callout_name; callout->major = drv->callout_major; callout->subtype = SERIAL_TYPE_CALLOUT; callout->read_proc = NULL; callout->proc_entry = NULL;
Reagrds. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
|  |