Thread (13 messages) 13 messages, 5 authors, 2008-10-10

Re: [PATCH] max3100 driver

From: Alan Cox <hidden>
Date: 2008-09-20 14:12:30
Also in: lkml

+#define MAX3100_MAJOR 204
+#define MAX3100_MINOR 128
+/* 4 MAX3100s should be enough for everyone */
+#define MAX_MAX3100 4
These need to be officially allocated if you need constant numbers
+static int max3100_sr(struct max3100_port_s *s, u16 tx, u16 *rx)
+{
+	struct spi_message message;
+	struct spi_transfer tran;
+	u16 etx, erx;
+	int status;
+
+	etx = htons(tx);
Use cpu_to_le/be or le/be_to_cpu functions, these make the intended
endianness clear.
+	*rx = ntohs(erx);
Ditto
+		if (rxchars > 0)
+			tty_flip_buffer_push(s->port.info->port.tty);
+		if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
If there has been a hangup the port.tty will be NULL...
+static void
+max3100_set_termios(struct uart_port *port, struct ktermios *termios,
+		   struct ktermios *old)
+{
+	struct max3100_port_s *s = container_of(port,
+						struct max3100_port_s,
+	if (!old || (termios->c_cflag != old->c_cflag)) {
This optimisation is wrong and not worth doing anyway
+		i = cflag & CBAUD;
+		switch (i) {
Use tty_get_baud_rate() to get the actual baud rate requested which is an
arbitary value.
+		default:
+			param_new = 1;
+			dev_warn(&s->spi->dev, "invalid baudrate\n");
Shouldn't warn on these, just be sure to use tty_encode_baud_rate to pass
back the actual rate the user ends up with.
+		if (termios->c_iflag & IGNPAR)
+			s->port.ignore_status_mask |=
+				MAX3100_STATUS_PE | MAX3100_STATUS_FE |
+				MAX3100_STATUS_OE;
Bits you don't support should also be cleared in the tty->termios struct
(eg markspace you don't seem to do)

+	max3100s[i] = kzalloc(sizeof(struct max3100_port_s), GFP_KERNEL);
+	if (!max3100s[i]) {
+		dev_warn(&spi->dev,
+			 "kmalloc for max3100 structure %d failed!\n", i);
Does this not then need to unregister the driver ?



Looks basically sound to me - just some minor cleanups needed.

Alan
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help