lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:	Thu, 06 Mar 2014 22:26:46 -0800
From:	Joe Perches <joe@...ches.com>
To:	Greg KH <gregkh@...uxfoundation.org>
Cc:	jon@...gle.org, jslaby@...e.cz, linux-kernel@...r.kernel.org,
	linux-serial@...r.kernel.org, Jon Ringle <jringle@...dpoint.com>
Subject: Re: [PATCH] RFC: WIP: sc16is7xx.c

On Thu, 2014-03-06 at 21:52 -0800, Greg KH wrote:
> On Thu, Mar 06, 2014 at 09:35:46PM -0500, jon@...gle.org wrote:
> > From: Jon Ringle <jringle@...dpoint.com>
> > 
> > I am requesting comments on this serial driver.
> > I am currently having some latency issues with it where I experience
> > packet loss at speed of 19200.
> > 
> > I welcome any and all comments.
> 
> The basic coding style problems make it hard to read to be able to help
> review it, sorry.
> 
> Yes, brains have patterns, you want to follow the same patterns as
> others, it matters.

Here's a patternizing patch on top of this...

It's an extended version of what I sent Jon privately.

Mostly whitespace but some other neatening too.
Brace removals, tabifying, 80 column comments,
spelling typos, pr_<level>, c90 comments, etc.

I don't care that's it does a lot of things in
a single patch because this hasn't ever been
submitted before and I hope Jon rolls it into
his next submission.

---
 drivers/tty/serial/sc16is7xx.c | 485 +++++++++++++++++++++--------------------
 1 file changed, 253 insertions(+), 232 deletions(-)

diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
index 5daed84..045241e 100644
--- a/drivers/tty/serial/sc16is7xx.c
+++ b/drivers/tty/serial/sc16is7xx.c
@@ -1,28 +1,34 @@
 /*
- * The SC16IS740/750/760 is a slave I2C-bus/SPI interface to a single-channel high
- * performance UART. The SC16IS740/750/760’s internal register set is backward-compatible
- * with the widely used and widely popular 16C450. The SC16IS740/750/760 also provides
- * additional advanced features such as auto hardware and software flow control, automatic
- * RS-485 support, and software reset.
- *
- * Copyright (C) 2014 GridPoint
+ * SC16IS740/750/760 tty serial driver - Copyright (C) 2014 GridPoint
  *
  * 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; either version 2 of the License, or
  * (at your option) any later version.
  *
+ * The SC16IS740/750/760 is a slave I2C-bus/SPI interface to a single-channel
+ * high performance UART. The SC16IS740/750/760’s internal register set is
+ * backward-compatible with the widely used and widely popular 16C450.
+ *
+ * The SC16IS740/750/760 also provides additional advanced features such as
+ * auto hardware and software flow control, automatic RS-485 support, and
+ * software reset.
+ *
  * Notes:
+ *
  * The sc16is740 driver is used for the GPEC RS485 Half duplex communication.
- * In the EC1K board the sc16is740 RTS line is connected to the SN65HVD1780DR chip and which
- * is used to signal the RS485 diretion. When the RTS is low the RS485 direction is set to
- * output from the CPU.
- * To set the RS485 diretion we use the sc16is740 internal RS485 feature where the chip drives
- * the RTS line when the data is written to the TX FIFO (see the spec note for the EFCR[4] bit
- * configuration).
  *
+ * In the EC1K board the sc16is740 RTS line is connected to a SN65HVD1780DR
+ * chip which is used to signal the RS485 direction.
+ * When RTS is low, the RS485 direction is set to output from the CPU.
+ *
+ * To set the RS485 direction we use the sc16is740 internal RS485 feature
+ * where the chip drives the RTS line when the data is written to the TX FIFO
+ * (see the spec note for the EFCR[4] bit configuration).
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/delay.h>
 #include <linux/device.h>
 #include <linux/gpio.h>
@@ -39,116 +45,139 @@
 #include <linux/tty_flip.h>
 #include <linux/wait.h>
 
-#define DRV_VERSION 		"0.3"
-#define DRV_NAME            "sc16is7xx"
-#define DEV_NAME            "ttySC"
-#define SC16IS7XX_MAJOR 	204  /* Take place of the /dev/ttySC0 SCI serial port 0 */
-#define SC16IS7XX_MINOR  	8
+#define DRV_VERSION	"0.3"
+#define DRV_NAME	"sc16is7xx"
+#define DEV_NAME	"ttySC"
+
+#define SC16IS7XX_MAJOR	204	/* Take place of the /dev/ttySC0
+				 * SCI serial port 0
+				 */
+#define SC16IS7XX_MINOR	8
 
 /*
  * Software Definitions
  */
 /* Commands sent from the uart callbacks to the work handler */
-#define SC16IS7XX_CMND_STOP_RX       (0) /* Disable the RX interrupt */
-#define SC16IS7XX_CMND_START_TX      (1) /* Enable  the TX holding register empty interrupt */
-#define SC16IS7XX_CMND_STOP_TX       (2) /* Disable the TX holding register empty interrupt */
-#define SC16IS7XX_CMND_NEW_TERMIOS   (3) /* Apply new termios configuration */
-#define SC16IS7XX_CMND_BREAK_CTRL    (4)
-#define SC16IS7XX_CMND_TX_RX         (5)
-
-#define SC16IS7XX_CLEAR_FIFO_ON_TX   /* If defined controller will clear tx fifo before it transmits chars */
+#define SC16IS7XX_CMND_STOP_RX		(0) /* Disable the RX interrupt */
+#define SC16IS7XX_CMND_START_TX		(1) /* Enable  the TX holding register
+					     * empty interrupt
+					     */
+#define SC16IS7XX_CMND_STOP_TX		(2) /* Disable the TX holding register
+					     * empty interrupt
+					     */
+#define SC16IS7XX_CMND_NEW_TERMIOS	(3) /* Apply termios configuration */
+#define SC16IS7XX_CMND_BREAK_CTRL	(4)
+#define SC16IS7XX_CMND_TX_RX		(5)
+
+#define SC16IS7XX_CLEAR_FIFO_ON_TX   /* If defined controller will clear
+				      * tx fifo before it transmits chars
+				      */
 
 /*
  * SC16IS7XX Hardware Definitions
  */
-#define DA850_RS485_INT_PIN  GPIO_TO_PIN(0,4)
+#define DA850_RS485_INT_PIN	GPIO_TO_PIN(0, 4)
 
-#define SC16IS7XX_CLK          		7372800 /* crystal clock connected to the SC16IS7XX chip */
-#define SC16IS7XX_DEFAULT_BAUDRATE     19200
+#define SC16IS7XX_CLK		7372800 /* crystal clock connected
+					 * to the SC16IS7XX chip
+					 */
+#define SC16IS7XX_DEFAULT_BAUDRATE	19200
 
 /* General registers set */
-#define SC16IS7XX_TCR 0x06
-#define SC16IS7XX_TLR 0x07
-#define SC16IS7XX_TXLVL  0x08
-#define SC16IS7XX_RXLVL  0x09
-#define SC16IS7XX_EFCR   0x0F
+#define SC16IS7XX_TCR		0x06
+#define SC16IS7XX_TLR		0x07
+#define SC16IS7XX_TXLVL		0x08
+#define SC16IS7XX_RXLVL		0x09
+#define SC16IS7XX_EFCR		0x0F
 
 /* LCR / MCR configurations */
-#define UART_LCR_8N1	UART_LCR_WLEN8
+#define UART_LCR_8N1		UART_LCR_WLEN8
 
-#define SC16IS7XX_LCRVAL UART_LCR_8N1           /* 8 data, 1 stop, no parity */
-#define SC16IS7XX_MCRVAL (UART_MCR_DTR|UART_MCR_RTS) /* clock divisor = 1,UART mode,lpback disabled,RTS/DTR are set,TCR/TLR disabled */
+#define SC16IS7XX_LCRVAL	UART_LCR_8N1	/* 8 data, 1 stop, no parity */
+#define SC16IS7XX_MCRVAL	(UART_MCR_DTR | UART_MCR_RTS)
+						/* clock divisor = 1,
+						 * UART mode,
+						 * loopback disabled,
+						 * RTS/DTR are set,
+						 * TCR/TLR disabled
+						 */
 
 /* SC16IS7XX Internal register address translation */
-#define REG_TO_I2C_ADDR(reg) (((reg) & 0x0f) << 3)
-
-#define SC16IS7XX_FIFO_SIZE 	  64  /* Rx fifo size */
+#define REG_TO_I2C_ADDR(reg)	(((reg) & 0x0f) << 3)
 
-#define SC16IS7XX_LOAD_SIZE       64  /* Tx fifo size */
+#define SC16IS7XX_FIFO_SIZE	64	/* Rx fifo size */
+#define SC16IS7XX_LOAD_SIZE	64	/* Tx fifo size */
 
 struct uart_sc16is7xx_port {
 	struct uart_port port;
 
 	/* private to the driver */
-	struct i2c_client* client; /* I2C client handle */
+	struct i2c_client *client;	/* I2C client handle */
 
-	int tx_empty;		/* last TX empty bit */
+	int tx_empty;			/* last TX empty bit */
 
-	unsigned long command;  /* Command to the work executed from the workqueue */
+	unsigned long command;		/* Command to the work executed
+					 * from the workqueue
+					 */
 
-	struct ktermios* termios_new;
-	struct ktermios* termios_old;
+	struct ktermios *termios_new;
+	struct ktermios *termios_old;
 
 	int break_state;
 
-	char ier; /* cache Interrupt Enable Register to manage the interrupt from the work */
+	char ier;	/* cache Interrupt Enable Register to
+			 * manage the interrupt from the work
+			 */
 	char lcr;
-	char fcr; /* cache the FIFO control register to hold write only values of that register */
+	char fcr;	/* cache the FIFO control register to
+			 * hold write only values of that register
+			 */
 
 	spinlock_t lock;
 
 	/* set to 1 to make the workhandler exit as soon as possible */
-	int  force_end_work;
+	int force_end_work;
 };
 
-static inline unsigned char sc16is7xx_read_reg(struct uart_sc16is7xx_port* up, unsigned char reg)
+static inline unsigned char sc16is7xx_read_reg(struct uart_sc16is7xx_port *up,
+					       unsigned char reg)
 {
 	int rc;
 	u8 val = 0;
-
 	u8 sc_reg = REG_TO_I2C_ADDR(reg);
 
 	rc = i2c_master_send(up->client, &sc_reg, 1);
-	if(rc < 0)
-	{
-		dev_err(&up->client->dev,"%s I2C error writing the i2c client rc = %d\n",__FUNCTION__, rc);
+	if (rc < 0) {
+		dev_err(&up->client->dev,
+			"%s I2C error writing the i2c client rc = %d\n",
+			__func__, rc);
 		goto out;
 	}
 
 	rc = i2c_master_recv(up->client, &val, 1);
-	if(rc < 0)
-	{
-		dev_err(&up->client->dev,"%s I2C error reading from the i2c client rc = %d\n",__FUNCTION__, rc);
-	}
+	if (rc < 0)
+		dev_err(&up->client->dev,
+			"%s I2C error reading from the i2c client rc = %d\n",
+			__func__, rc);
 
 out:
 	return val;
 }
 
-static inline void sc16is7xx_write_reg(struct uart_sc16is7xx_port *up, unsigned char reg, unsigned char value)
+static inline void sc16is7xx_write_reg(struct uart_sc16is7xx_port *up,
+				       unsigned char reg, unsigned char value)
 {
 	int rc;
-
 	u8 msg[2];
 
 	msg[0] = REG_TO_I2C_ADDR(reg);
 	msg[1] = value;
 
 	rc = i2c_master_send(up->client, msg, 2);
-	if(rc < 0)
-	{
-		dev_err(&up->client->dev,"%s I2C error writing the i2c client rc = %d\n",__FUNCTION__, rc);
-	}
+	if (rc < 0)
+		dev_err(&up->client->dev,
+			"%s I2C error writing the i2c client rc = %d\n",
+			__func__, rc);
 }
 
 static void sc16is7xx_enable_irq(struct uart_sc16is7xx_port *up, int mask)
@@ -163,45 +192,54 @@ static void sc16is7xx_disable_irq(struct uart_sc16is7xx_port *up, int mask)
 	sc16is7xx_write_reg(up, UART_IER, up->ier);
 }
 
-
-static void sc16is7xx_set_baudrate(struct uart_sc16is7xx_port* up, int baudrate)
+static void sc16is7xx_set_baudrate(struct uart_sc16is7xx_port *up, int baudrate)
 {
 	u8 lcr, ier;
 	u32 divisor;
 
 	ier = sc16is7xx_read_reg(up, UART_IER);
 	lcr = sc16is7xx_read_reg(up, UART_LCR);
-	sc16is7xx_write_reg(up, UART_IER, 0x00); // Disable interrupts
 
-	sc16is7xx_write_reg(up, SC16IS7XX_EFCR, 0x06 ); // Disable TX/RX
+	/* Disable interrupts */
+	sc16is7xx_write_reg(up, UART_IER, 0x00);
+
+	/* Disable TX/RX */
+	sc16is7xx_write_reg(up, SC16IS7XX_EFCR, 0x06);
 
-	sc16is7xx_write_reg(up, UART_LCR, UART_LCR_CONF_MODE_B);   // Open the LCR divisors for configuration
+	/* Open the LCR divisors for configuration */
+	sc16is7xx_write_reg(up, UART_LCR, UART_LCR_CONF_MODE_B);
 
-	sc16is7xx_write_reg(up, UART_EFR, 0x10);   // Enable enhanced features and internal clock divider
+	/* Enable enhanced features and internal clock divider */
+	sc16is7xx_write_reg(up, UART_EFR, 0x10);
 
-	sc16is7xx_write_reg(up, UART_MCR, UART_MCR_CLKSEL|4);  // Set the input clock divisor to 1
+	/* Set the input clock divisor to 1 */
+	sc16is7xx_write_reg(up, UART_MCR, UART_MCR_CLKSEL|4);
 
 	/* Get the baudrate divisor from the upper port layer */
 	divisor = uart_get_divisor(&up->port, baudrate);
 
-	sc16is7xx_write_reg(up, UART_DLL, divisor & 0xff );      // Write the new divisor
+	/* Write the new divisor */
+	sc16is7xx_write_reg(up, UART_DLL, divisor & 0xff);
 	sc16is7xx_write_reg(up, UART_DLM, (divisor >> 8) & 0xff);
 
-	sc16is7xx_write_reg(up, UART_LCR, lcr);   // Put LCR back to the normal mode
+	/* Put LCR back to the normal mode */
+	sc16is7xx_write_reg(up, UART_LCR, lcr);
 
 	sc16is7xx_write_reg(up, SC16IS7XX_TLR, 0x0f);
+
+	/* Enable the FIFOs */
 	up->fcr = UART_FCR_ENABLE_FIFO;
-	sc16is7xx_write_reg(up, UART_FCR, up->fcr);   // Enable the FIFOs
+	sc16is7xx_write_reg(up, UART_FCR, up->fcr);
 
-	sc16is7xx_write_reg(up, SC16IS7XX_EFCR, 0x10); // Enable the Rx and Tx and  control the RTS (RS485_DIR) line
+	/* Enable the Rx and Tx and  control the RTS (RS485_DIR) line */
+	sc16is7xx_write_reg(up, SC16IS7XX_EFCR, 0x10);
 
-	sc16is7xx_write_reg(up, UART_IER, ier); // Restore the interrupts configuration
+	/* Restore the interrupts configuration */
+	sc16is7xx_write_reg(up, UART_IER, ier);
 }
 
-/*
- * The function actually delivers the Tx characters to the HW
- */
-static void sc16is7xx_transmit_chars(struct uart_sc16is7xx_port* up)
+/* deliver the Tx characters to the HW */
+static void sc16is7xx_transmit_chars(struct uart_sc16is7xx_port *up)
 {
 	struct circ_buf *xmit = &up->port.state->xmit;
 	int count;
@@ -215,6 +253,7 @@ static void sc16is7xx_transmit_chars(struct uart_sc16is7xx_port* up)
 		up->port.x_char = 0;
 		return;
 	}
+
 	if (uart_circ_empty(xmit) || uart_tx_stopped(&up->port)) {
 		up->ier &= ~UART_IER_THRI;
 		return;
@@ -235,15 +274,13 @@ static void sc16is7xx_transmit_chars(struct uart_sc16is7xx_port* up)
 	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
 		uart_write_wakeup(&up->port);
 
-	if (uart_circ_empty(xmit)) {
+	if (uart_circ_empty(xmit))
 		up->ier &= ~UART_IER_THRI;
-	}
 }
 
-/*
- * The function actually receives the characters from the HW and transfers them up to the TTY layer.
- */
-static inline void sc16is7xx_receive_chars(struct uart_sc16is7xx_port *up, int *status)
+/* receives characters from the HW and transfer themto the TTY layer */
+static inline void sc16is7xx_receive_chars(struct uart_sc16is7xx_port *up,
+					   int *status)
 {
 	unsigned int ch, flag;
 	int chars = 0;
@@ -252,15 +289,13 @@ static inline void sc16is7xx_receive_chars(struct uart_sc16is7xx_port *up, int *
 	dev_dbg(&up->client->dev, "%s\n", __func__);
 
 	do {
-		if (likely(*status & UART_LSR_DR)) {
+		if (likely(*status & UART_LSR_DR))
 			ch = sc16is7xx_read_reg(up, UART_RX);
-		} else {
+		else
 			ch = 0xffff;
-		}
 
-		if (*status & up->port.ignore_status_mask) {
+		if (*status & up->port.ignore_status_mask)
 			goto ignore_char;
-		}
 
 		flag = TTY_NORMAL;
 		up->port.icount.rx++;
@@ -273,64 +308,56 @@ static inline void sc16is7xx_receive_chars(struct uart_sc16is7xx_port *up, int *
 				*status &= ~(UART_LSR_FE | UART_LSR_PE);
 				up->port.icount.brk++;
 				/*
-				 * We do the SysRQ and SAK checking
-				 * here because otherwise the break
-				 * may get masked by ignore_status_mask
-				 * or read_status_mask.
+				 * We do the SysRQ and SAK checking here
+				 * because otherwise the break may get masked
+				 * by ignore_status_mask or read_status_mask
 				 */
-				if (uart_handle_break(&up->port)) {
+				if (uart_handle_break(&up->port))
 					goto ignore_char;
-				}
 			} else if (*status & UART_LSR_PE) {
 				up->port.icount.parity++;
 			} else if (*status & UART_LSR_FE) {
 				up->port.icount.frame++;
 			}
-			if (*status & UART_LSR_OE) {
+			if (*status & UART_LSR_OE)
 				up->port.icount.overrun++;
-			}
 
-			/*
-			 * Mask off conditions which should be ignored.
-			 */
+			/* Mask off conditions which should be ignored */
 			*status &= up->port.read_status_mask;
 
-			if (*status & UART_LSR_BI) {
+			if (*status & UART_LSR_BI)
 				flag = TTY_BREAK;
-			} else if (*status & UART_LSR_PE) {
+			else if (*status & UART_LSR_PE)
 				flag = TTY_PARITY;
-			} else if (*status & UART_LSR_FE) {
+			else if (*status & UART_LSR_FE)
 				flag = TTY_FRAME;
-			}
 		}
 
-		if (unlikely(0xffff == ch)) {
+		if (unlikely(0xffff == ch))
 			goto ignore_char;
-		}
 
-		if (uart_handle_sysrq_char(&up->port, ch)) {
+		if (uart_handle_sysrq_char(&up->port, ch))
 			goto ignore_char;
-		}
 
 		uart_insert_char(&up->port, *status, UART_LSR_OE, ch, flag);
 		chars++;
 
 ignore_char:
 		*status = sc16is7xx_read_reg(up, UART_LSR);
-	} while ((*status & (UART_LSR_DR | UART_LSR_BI)) && (max_count-- > 0) && !up->force_end_work);
+	} while ((*status & (UART_LSR_DR | UART_LSR_BI)) &&
+		 (max_count-- > 0) && !up->force_end_work);
 
-	dev_dbg(&up->client->dev, "%s RX:%d chars oe:%d\n", __func__, chars, up->port.icount.overrun);
+	dev_dbg(&up->client->dev, "%s RX:%d chars oe:%d\n",
+		__func__, chars, up->port.icount.overrun);
 
-	if (chars > 0) {
+	if (chars > 0)
 		tty_flip_buffer_push(&(up->port.state->port));
-	}
 }
 
-static void
-sc16is7xx_set_termios_work(struct uart_sc16is7xx_port *up)
+static void sc16is7xx_set_termios_work(struct uart_sc16is7xx_port *up)
 {
-	struct ktermios* termios = up->termios_new;
-	struct ktermios* old = up->termios_old;
+	struct ktermios *termios = up->termios_new;
+	struct ktermios *old = up->termios_old;
 	unsigned long flags;
 	unsigned char cval;
 	unsigned int baud;
@@ -347,8 +374,8 @@ sc16is7xx_set_termios_work(struct uart_sc16is7xx_port *up)
 	case CS7:
 		cval = UART_LCR_WLEN7;
 		break;
-	default:
 	case CS8:
+	default:
 		cval = UART_LCR_WLEN8;
 		break;
 	}
@@ -362,29 +389,22 @@ sc16is7xx_set_termios_work(struct uart_sc16is7xx_port *up)
 
 	sc16is7xx_write_reg(up, UART_LCR, cval);
 
-	/*
-	 * Ask the core to calculate the divisor for us.
-	 */
+	/* Ask the core to calculate the divisor for us */
 	baud = uart_get_baud_rate(&up->port, termios, old, 9600, 115200);
 	sc16is7xx_set_baudrate(up, baud);
 
-	up->port.state->port.low_latency = 1; // Low latency since we Tx from the work queue
+	/* Low latency since we Tx from the work queue */
+	up->port.state->port.low_latency = 1;
 
-	/*
-	 * Update the per-port timeout.
-	 */
+	/* Update the per-port timeout */
 	uart_update_timeout(&up->port, termios->c_cflag, baud);
 
-	/*
-	 * ignore all characters if CREAD is not set
-	 */
+	/* ignore all characters if CREAD is not set */
 	if ((termios->c_cflag & CREAD) == 0)
 		up->port.ignore_status_mask |= UART_LSR_DR;
 
-	if(tty_termios_baud_rate(termios))
-	{
-		tty_termios_encode_baud_rate(termios,baud,baud);
-	}
+	if (tty_termios_baud_rate(termios))
+		tty_termios_encode_baud_rate(termios, baud, baud);
 
 	spin_unlock_irqrestore(&up->lock, flags);
 }
@@ -395,53 +415,59 @@ static void sc16is7xx_work(struct uart_sc16is7xx_port *up)
 
 	up->ier = sc16is7xx_read_reg(up, UART_IER);
 
-	/* We cannot handle the interrupts while in the work queue so we disable the RX interrupt.
-	 * It is ok because of during the reception of the characters we check the status of the
+	/*
+	 * We cannot handle the interrupts while in the work queue so we
+	 * disable the RX interrupt.  It is ok because of during the
+	 * reception of the characters we check the status of the
 	 * interrupt register and process all incoming packets
 	 */
 	sc16is7xx_write_reg(up, UART_IER, 0x00);
 
-	dev_dbg(&up->client->dev, "%s: start work  command:%04lx ier:0x%02x\n", __func__, up->command, (int)up->ier);
+	dev_dbg(&up->client->dev, "%s: start work - command:%04lx ier:0x%02x\n",
+		__func__, up->command, (int)up->ier);
 
-	if(test_and_clear_bit(SC16IS7XX_CMND_NEW_TERMIOS, &up->command)) {
+	if (test_and_clear_bit(SC16IS7XX_CMND_NEW_TERMIOS, &up->command)) {
 		dev_dbg(&up->client->dev, "CMND_NEW_TERMIOS\n");
 		/* User requested the changes in the Terminal Configurations */
 		sc16is7xx_set_termios_work(up);
 	}
 
-	if(test_and_clear_bit(SC16IS7XX_CMND_BREAK_CTRL, &up->command)) {
-		if (up->break_state == -1) {
+	if (test_and_clear_bit(SC16IS7XX_CMND_BREAK_CTRL, &up->command)) {
+		if (up->break_state == -1)
 			up->lcr |= UART_LCR_SBC;
-		} else {
+		else
 			up->lcr &= ~UART_LCR_SBC;
-		}
 		sc16is7xx_write_reg(up, UART_LCR, up->lcr);
 	}
 
-	if(test_and_clear_bit(SC16IS7XX_CMND_START_TX, &up->command)) {
+	if (test_and_clear_bit(SC16IS7XX_CMND_START_TX, &up->command)) {
 		dev_dbg(&up->client->dev, "CMND_START_TX\n");
 		/* Enable the Tx holding register empty interrupt */
 		up->ier |= UART_IER_THRI;
 
 #ifdef SC16IS7XX_CLEAR_FIFO_ON_TX
 		/* Clear Tx Fifo to remove the junk characters if any */
-		if(up->fcr & UART_FCR_ENABLE_FIFO) {
+		if (up->fcr & UART_FCR_ENABLE_FIFO) {
 			/* Fifo is enabled */
-			sc16is7xx_write_reg(up, UART_FCR, up->fcr & UART_FCR_CLEAR_XMIT);
+			sc16is7xx_write_reg(up, UART_FCR,
+					    up->fcr & UART_FCR_CLEAR_XMIT);
 			dev_dbg(&up->client->dev, "Clear FIFO\n");
 		}
 #endif
 	}
 
-	if(test_and_clear_bit(SC16IS7XX_CMND_STOP_TX, &up->command)) {
+	if (test_and_clear_bit(SC16IS7XX_CMND_STOP_TX, &up->command)) {
 		dev_dbg(&up->client->dev, "CMND_STOP_TX\n");
 		/* Disable the Tx holding register interrupt  */
 		up->ier &= ~UART_IER_THRI;
 	}
 
-	if(test_and_clear_bit(SC16IS7XX_CMND_STOP_RX, &up->command)) {
+	if (test_and_clear_bit(SC16IS7XX_CMND_STOP_RX, &up->command)) {
 		dev_dbg(&up->client->dev, "CMND_STOP_RX\n");
-		/* User requested to stop the RX interrupt so we clear the interrupt enable register */
+		/*
+		 * User requested to stop the RX interrupt so we clear the
+		 * interrupt enable register
+		 */
 		up->ier &= ~UART_IER_RDI;
 	}
 
@@ -449,35 +475,33 @@ static void sc16is7xx_work(struct uart_sc16is7xx_port *up)
 	lsr = sc16is7xx_read_reg(up, UART_LSR);
 	if ((lsr & (UART_LSR_DR | UART_LSR_BI)) && (up->ier & UART_IER_RDI)) {
 		sc16is7xx_receive_chars(up, &lsr);
-		if (lsr & (UART_LSR_DR | UART_LSR_BI)) {
-			/* There is more to receive */
+		/* check if there is more to receive */
+		if (lsr & (UART_LSR_DR | UART_LSR_BI))
 			set_bit(SC16IS7XX_CMND_TX_RX, &up->command);
-		}
 	}
 
-	if ((lsr & UART_LSR_THRE) && (up->ier & UART_IER_THRI)) {
+	if ((lsr & UART_LSR_THRE) && (up->ier & UART_IER_THRI))
 		sc16is7xx_transmit_chars(up);
-	}
 
-	if(sc16is7xx_read_reg(up, UART_LSR) & UART_LSR_THRE)
-	{
-		dev_dbg(&up->client->dev, "%s: TX_EMPTY\n", __FUNCTION__);
+	if (sc16is7xx_read_reg(up, UART_LSR) & UART_LSR_THRE) {
+		dev_dbg(&up->client->dev, "%s: TX_EMPTY\n", __func__);
 		up->tx_empty = TIOCSER_TEMT;
-	}
-	else
-	{
-		dev_dbg(&up->client->dev, "%s: TX_NOT_EMPTY\n", __FUNCTION__);
-		up->tx_empty = 0; /* port is not ready to accept  the new TX characters */
+	} else {
+		/* port is not ready to accept the new TX characters */
+		dev_dbg(&up->client->dev, "%s: TX_NOT_EMPTY\n", __func__);
+		up->tx_empty = 0;
 	}
 
-	dev_dbg(&up->client->dev, "%s: end work ier 0x%02X\n", __func__, (int)up->ier);
+	dev_dbg(&up->client->dev, "%s: end work - ier 0x%02X\n",
+		__func__, (int)up->ier);
 
-	sc16is7xx_write_reg(up, UART_IER, up->ier); /* Restore the interrupts */
+	/* Restore the interrupts */
+	sc16is7xx_write_reg(up, UART_IER, up->ier);
 }
 
-static irqreturn_t sc16is7xx_ist(int irq, void* dev_id)
+static irqreturn_t sc16is7xx_ist(int irq, void *dev_id)
 {
-	struct uart_sc16is7xx_port* up = (struct uart_sc16is7xx_port *)dev_id;
+	struct uart_sc16is7xx_port *up = (struct uart_sc16is7xx_port *)dev_id;
 	unsigned long flags;
 
 	spin_lock_irqsave(&up->lock, flags);
@@ -491,18 +515,25 @@ static unsigned int sc16is7xx_tx_empty(struct uart_port *port)
 {
 	struct uart_sc16is7xx_port *up = (struct uart_sc16is7xx_port *)port;
 
-	dev_dbg(&up->client->dev, "%s:(%d)\n", __FUNCTION__, up->tx_empty);
+	dev_dbg(&up->client->dev, "%s:(%d)\n", __func__, up->tx_empty);
+
 	return up->tx_empty;
 }
 
 static void sc16is7xx_set_mctrl(struct uart_port *port, unsigned int mctrl)
 {
-	/* Just a placeholder. We do not have modem control lines in our RS485 port */
+	/*
+	 * Just a placeholder
+	 * We do not have modem control lines in our RS485 port
+	 */
 }
 
 static unsigned int sc16is7xx_get_mctrl(struct uart_port *port)
 {
-	/* Just a placeholder. We do not have modem control lines in our RS485 port */
+	/*
+	 * Just a placeholder
+	 * We do not have modem control lines in our RS485 port
+	 */
 	return TIOCM_CAR | TIOCM_RNG | TIOCM_DSR | TIOCM_CTS;
 }
 
@@ -548,7 +579,7 @@ static void sc16is7xx_stop_tx(struct uart_port *port)
 	spin_unlock_irqrestore(&up->lock, flags);
 }
 
-static void sc16is7xxs_enable_ms(struct uart_port* port)
+static void sc16is7xxs_enable_ms(struct uart_port *port)
 {
 	/* Do nothing */
 }
@@ -560,7 +591,7 @@ static void sc16is7xx_break_ctl(struct uart_port *port, int break_state)
 
 	spin_lock_irqsave(&up->lock, flags);
 
-	dev_dbg(&up->client->dev, "%s:(%d)\n", __FUNCTION__, break_state);
+	dev_dbg(&up->client->dev, "%s:(%d)\n", __func__, break_state);
 	up->break_state = break_state;
 	set_bit(SC16IS7XX_CMND_BREAK_CTRL, &up->command);
 	sc16is7xx_work(up);
@@ -577,18 +608,15 @@ static void sc16is7xx_shutdown(struct uart_port *port)
 
 	dev_dbg(&up->client->dev, "%s\n", __func__);
 
-	/*
-	 * Disable interrupts from this port
-	 */
+	/* Disable interrupts from this port */
 	sc16is7xx_disable_irq(up, UART_IER_THRI | UART_IER_RDI);
 
-	/*
-	 * Disable break condition and FIFOs
-	 */
-	sc16is7xx_write_reg(up, UART_LCR, sc16is7xx_read_reg(up, UART_LCR) & ~UART_LCR_SBC);
-	sc16is7xx_write_reg(up, UART_FCR, UART_FCR_ENABLE_FIFO |
-			    UART_FCR_CLEAR_RCVR |
-			    UART_FCR_CLEAR_XMIT);
+	/* Disable break condition and FIFOs */
+	sc16is7xx_write_reg(up, UART_LCR,
+			    sc16is7xx_read_reg(up, UART_LCR) & ~UART_LCR_SBC);
+	sc16is7xx_write_reg(up, UART_FCR,
+			    (UART_FCR_ENABLE_FIFO | UART_FCR_CLEAR_RCVR |
+			     UART_FCR_CLEAR_XMIT));
 	sc16is7xx_write_reg(up, UART_FCR, 0);
 
 	spin_unlock_irqrestore(&up->lock, flags);
@@ -610,37 +638,37 @@ static int sc16is7xx_startup(struct uart_port *port)
 	up->break_state = -1;
 	up->tx_empty = TIOCSER_TEMT;
 
-	// Disable IRQs to configure
+	/* Disable IRQs to configure */
 	sc16is7xx_write_reg(up, UART_IER, 0);
 
-	/*
-	 * Now, initialize the UART
-	 */
+	/* Now, initialize the UART */
 	sc16is7xx_write_reg(up, UART_LCR, UART_LCR_8N1);
 
 	/*
-	 * Clear the FIFO buffers and disable them.
+	 * Clear the FIFO buffers and disable them
 	 * (they will be reenabled in set_termios())
 	 */
-	while(sc16is7xx_read_reg(up, UART_LSR) & (UART_LSR_DR | UART_LSR_BI))
-	{
-		/* Empty the RX holding register to prevent printing stale characters on the screen */
+	while (sc16is7xx_read_reg(up, UART_LSR) & (UART_LSR_DR | UART_LSR_BI)) {
+		/*
+		 * Empty the RX holding register to prevent printing
+		 * stale characters on the screen
+		 */
 		sc16is7xx_read_reg(up, UART_RX);
 	}
 
-	/*
-	 * Finally, enable interrupts.
-	 */
+	/* Finally, enable interrupts */
 	sc16is7xx_enable_irq(up, UART_IER_RDI);
 
 	spin_unlock_irqrestore(&up->lock, flags);
+
 	return 0;
 }
 
-static void
-sc16is7xx_set_termios(struct uart_port* port, struct ktermios* termios, struct ktermios* old)
+static void sc16is7xx_set_termios(struct uart_port *port,
+				  struct ktermios *termios,
+				  struct ktermios *old)
 {
-	struct uart_sc16is7xx_port * up = (struct uart_sc16is7xx_port *)port;
+	struct uart_sc16is7xx_port *up = (struct uart_sc16is7xx_port *)port;
 	unsigned long flags;
 
 	spin_lock_irqsave(&up->lock, flags);
@@ -678,15 +706,14 @@ static void sc16is7xx_config_port(struct uart_port *port, int flags)
 		up->port.type = PORT_SC16IS7XX;
 }
 
-static int sc16is7xx_verify_port(struct uart_port *port, struct serial_struct *ser)
+static int sc16is7xx_verify_port(struct uart_port *port,
+				 struct serial_struct *ser)
 {
-	int ret = -EINVAL;
-
 	if (ser->type == PORT_UNKNOWN || ser->type == PORT_SC16IS7XX)
-		ret = 0;
-	return ret;
-}
+		return 0;
 
+	return -EINVAL;
+}
 
 static struct uart_ops sc16is7xx_ops = {
 	.tx_empty	= sc16is7xx_tx_empty,
@@ -701,65 +728,61 @@ static struct uart_ops sc16is7xx_ops = {
 	.shutdown	= sc16is7xx_shutdown,
 	.set_termios	= sc16is7xx_set_termios,
 	.type		= sc16is7xx_type,
-	.release_port   = sc16is7xx_release_port,
-	.request_port   = sc16is7xx_request_port,
+	.release_port	= sc16is7xx_release_port,
+	.request_port	= sc16is7xx_request_port,
 	.config_port	= sc16is7xx_config_port,
 	.verify_port	= sc16is7xx_verify_port,
 };
 
 static struct uart_driver sc16is7xx_uart_driver = {
 	.owner		= THIS_MODULE,
-	.driver_name = DRV_NAME,
-	.dev_name    = DEV_NAME,
-	.major       = SC16IS7XX_MAJOR,
-	.minor       = SC16IS7XX_MINOR,
-	.nr          = 1,
+	.driver_name	= DRV_NAME,
+	.dev_name	= DEV_NAME,
+	.major		= SC16IS7XX_MAJOR,
+	.minor		= SC16IS7XX_MINOR,
+	.nr		= 1,
 };
-static int uart_driver_registered = 0;
+
+static int uart_driver_registered;
 
 static int sc16is7xx_probe(struct i2c_client *client,
 			   const struct i2c_device_id *id)
 {
 	int retval;
-	struct uart_sc16is7xx_port* up = NULL; /* user port */
+	struct uart_sc16is7xx_port *up = NULL; /* user port */
 
 	if (!uart_driver_registered) {
 		retval = uart_register_driver(&sc16is7xx_uart_driver);
 		if (retval) {
-			printk(KERN_ERR "Couldn't register sc16is7xx uart driver\n");
+			pr_err("Couldn't register sc16is7xx uart driver\n");
 			return retval;
 		}
 		uart_driver_registered = 1;
 	}
 
-	up = kzalloc(sizeof(struct uart_sc16is7xx_port), GFP_KERNEL);
-	if (!up) {
-		dev_warn(&client->dev,
-			 "kmalloc for sc16is7xx structure failed!\n");
+	up = kzalloc(sizeof(*up), GFP_KERNEL);
+	if (!up)
 		return -ENOMEM;
-	}
 
 	/* First check if adaptor is OK and it supports our I2C functionality */
-	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
-	{
-		dev_err(&client->dev,"Can't find the sc16is7xx chip\n");
+	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
+		dev_err(&client->dev, "Can't find the sc16is7xx chip\n");
 		retval = -ENODEV;
 		goto exit;
 	}
 
-	dev_info(&client->dev,"chip found, driver version " DRV_VERSION "\n");
+	dev_info(&client->dev, "chip found, driver version " DRV_VERSION "\n");
 
 	/* Configure the GPIO IRQ line */
 	retval = gpio_request(DA850_RS485_INT_PIN, "SC16IS7xx INT");
-
-	if(retval)
-	{
-		dev_err(&client->dev,"Can't request gpio interrupt pin \n");
+	if (retval) {
+		dev_err(&client->dev, "Can't request gpio interrupt pin\n");
 		retval = -EIO;
 		goto exit;
 	}
 
-	gpio_direction_input(DA850_RS485_INT_PIN); // Set GPIO IRQ pin to be input
+	/* Set GPIO IRQ pin to be input */
+	gpio_direction_input(DA850_RS485_INT_PIN);
 
 	up->client = client;
 
@@ -768,8 +791,8 @@ static int sc16is7xx_probe(struct i2c_client *client,
 	up->port.uartclk = SC16IS7XX_CLK;
 	up->port.fifosize = SC16IS7XX_FIFO_SIZE;
 	up->port.ops = &sc16is7xx_ops;
-	up->port.iotype   = UPIO_PORT;
-	up->port.flags    = UPF_FIXED_TYPE | UPF_FIXED_PORT;
+	up->port.iotype = UPIO_PORT;
+	up->port.flags = UPF_FIXED_TYPE | UPF_FIXED_PORT;
 	up->port.line = 0;
 	up->port.type = PORT_SC16IS7XX;
 	up->port.dev = &client->dev;
@@ -780,7 +803,7 @@ static int sc16is7xx_probe(struct i2c_client *client,
 			 "uart_add_one_port failed with error %d\n",
 			 retval);
 
-	i2c_set_clientdata(client,up);
+	i2c_set_clientdata(client, up);
 
 	/* Disable interrupts */
 	up->ier = 0;
@@ -790,7 +813,8 @@ static int sc16is7xx_probe(struct i2c_client *client,
 				      NULL, sc16is7xx_ist,
 				      IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
 				      DRV_NAME, up) < 0) {
-		dev_err(&up->client->dev, "cannot allocate irq %d\n", up->port.irq);
+		dev_err(&up->client->dev, "cannot allocate irq %d\n",
+			up->port.irq);
 		return -EBUSY;
 	}
 
@@ -805,10 +829,8 @@ static int sc16is7xx_remove(struct i2c_client *client)
 {
 	struct uart_sc16is7xx_port *up = i2c_get_clientdata(client);
 
-	if(!uart_driver_registered)
-	{
+	if (!uart_driver_registered)
 		return 0;
-	}
 
 	devm_free_irq(&client->dev, up->port.irq, up);
 
@@ -816,7 +838,6 @@ static int sc16is7xx_remove(struct i2c_client *client)
 	dev_dbg(&client->dev, "%s: removing port\n", __func__);
 	uart_remove_one_port(&sc16is7xx_uart_driver, &up->port);
 	kfree(up);
-	up = NULL;
 
 	pr_debug("removing sc16is7xx driver\n");
 	uart_unregister_driver(&sc16is7xx_uart_driver);
@@ -827,7 +848,7 @@ static int sc16is7xx_remove(struct i2c_client *client)
 }
 
 static const struct i2c_device_id sc16is7xx_i2c_id[] = {
-	{ "sc16is7xx",0},
+	{ "sc16is7xx", 0},
 	{ }
 };
 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ