Messages in this thread | | | Date | Mon, 9 Aug 2004 10:51:13 -0700 | From | "Randy.Dunlap" <> | Subject | Re: [RFC] Host Virtual Serial Interface driver |
| |
On Fri, 06 Aug 2004 16:23:05 -0500 Hollis Blanchard wrote:
| Hi, I have a new char driver I'd like to get comments on. It is specific | to IBM's p5 server line; I've included a description from the comments | here: ... | I've included the whole file below; it's pretty much self-contained. All | comments welcome. | | -- | | #include <linux/init.h> | #include <linux/module.h> | #include <linux/console.h> | #include <linux/major.h> | #include <linux/kernel.h> | #include <linux/sysrq.h> | #include <linux/tty.h> | #include <linux/tty_flip.h> | #include <linux/sched.h> | #include <linux/kbd_kern.h> | #include <linux/spinlock.h> | #include <linux/ctype.h> | #include <linux/interrupt.h> | #include <linux/delay.h>
To the extent possible, we like to put the linux/* files in alpha order, and same with the asm/* files. Separately, as you have them.
| #include <asm/uaccess.h> | #include <asm/hvconsole.h> | #include <asm/prom.h> | #include <asm/hvcall.h> | #include <asm/vio.h> | | #define __ALIGNED__ __attribute__((__aligned__(sizeof(long))))
You should explain this bit (__ALIGNED__).
| static inline int hdrlen(const uint8_t *packet) | { | const int lengths[] = { 4, 6, 6, 8, }; | struct hvsi_header *header = (struct hvsi_header *)packet; | | return lengths[VS_DATA_PACKET_HEADER - header->type]; | }
Any chance of bad data (value) in header->type ?
| if (hangup) { | tty_hangup(hangup); | }
extra braces (style); maybe in a few other places also.
-- ~Randy - 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/
| |