linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/12] Series short description
@ 2009-11-18 14:14 Alan Cox
  2009-11-18 14:14 ` [PATCH 01/12] isicom: switch to the new tty_port_open helper Alan Cox
                   ` (11 more replies)
  0 siblings, 12 replies; 14+ messages in thread
From: Alan Cox @ 2009-11-18 14:14 UTC (permalink / raw)
  To: greg, linux-kernel

Further serial whacking (Resend). Mostly attack the moxa/mxser/isicom drivers
and try to beat them into shape.

---

Alan Cox (12):
      moxa: split open lock
      moxa: Kill the use of lock_kernel
      moxa: Fix modem op locking
      moxa: Kill off the throttle method
      Locking clean up
      moxa: rework the locking a bit
      moxa: Use more tty_port ops
      isicom: fix deadlock on shutdown
      mxser: Use the new locking rules to fix setserial properly
      mxser: use the tty_port_open method
      isicom: sort out the board init logic
      isicom: switch to the new tty_port_open helper


 drivers/char/isicom.c  |  115 ++++---------------
 drivers/char/moxa.c    |  291 ++++++++++++++----------------------------------
 drivers/char/mxser.c   |  248 ++++++++++++++++++-----------------------
 include/linux/isicom.h |    1 
 4 files changed, 219 insertions(+), 436 deletions(-)


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 01/12] isicom: switch to the new tty_port_open helper
  2009-11-18 14:14 [PATCH 00/12] Series short description Alan Cox
@ 2009-11-18 14:14 ` Alan Cox
  2009-11-18 14:14 ` [PATCH 02/12] isicom: sort out the board init logic Alan Cox
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Alan Cox @ 2009-11-18 14:14 UTC (permalink / raw)
  To: greg, linux-kernel

Trivial conversion in this case so might as well do it while testing the
port_open design is right

Signed-off-by: Alan Cox <alan@linux.intel.com>
---

 drivers/char/isicom.c |   88 ++++++++++++-------------------------------------
 1 files changed, 21 insertions(+), 67 deletions(-)


diff --git a/drivers/char/isicom.c b/drivers/char/isicom.c
index 426bfdd..e7be3ec 100644
--- a/drivers/char/isicom.c
+++ b/drivers/char/isicom.c
@@ -804,24 +804,21 @@ static inline void isicom_setup_board(struct isi_board *bp)
 	bp->status |= BOARD_ACTIVE;
 	for (channel = 0; channel < bp->port_count; channel++, port++)
 		drop_dtr_rts(port);
+	bp->count++;
 	spin_unlock_irqrestore(&bp->card_lock, flags);
 }
 
-static int isicom_setup_port(struct tty_struct *tty)
+static int isicom_activate(struct tty_port *tport, struct tty_struct *tty)
 {
-	struct isi_port *port = tty->driver_data;
+	struct isi_port *port = container_of(tport, struct isi_port, port);
 	struct isi_board *card = port->card;
 	unsigned long flags;
 
-	if (port->port.flags & ASYNC_INITIALIZED)
-		return 0;
-	if (tty_port_alloc_xmit_buf(&port->port) < 0)
+	if (tty_port_alloc_xmit_buf(tport) < 0)
 		return -ENOMEM;
 
 	spin_lock_irqsave(&card->card_lock, flags);
-	clear_bit(TTY_IO_ERROR, &tty->flags);
-	if (port->port.count == 1)
-		card->count++;
+	isicom_setup_board(card);
 
 	port->xmit_cnt = port->xmit_head = port->xmit_tail = 0;
 
@@ -832,9 +829,7 @@ static int isicom_setup_port(struct tty_struct *tty)
 		outw(((ISICOM_KILLTX | ISICOM_KILLRX) << 8) | 0x06, card->base);
 		InterruptTheCard(card->base);
 	}
-
 	isicom_config_port(tty);
-	port->port.flags |= ASYNC_INITIALIZED;
 	spin_unlock_irqrestore(&card->card_lock, flags);
 
 	return 0;
@@ -871,31 +866,20 @@ static struct tty_port *isicom_find_port(struct tty_struct *tty)
 
 	return &port->port;
 }
-	
+
 static int isicom_open(struct tty_struct *tty, struct file *filp)
 {
 	struct isi_port *port;
 	struct isi_board *card;
 	struct tty_port *tport;
-	int error = 0;
 
 	tport = isicom_find_port(tty);
 	if (tport == NULL)
 		return -ENODEV;
 	port = container_of(tport, struct isi_port, port);
 	card = &isi_card[BOARD(tty->index)];
-	isicom_setup_board(card);
 
-	/* FIXME: locking on port.count etc */
-	port->port.count++;
-	tty->driver_data = port;
-	tty_port_tty_set(&port->port, tty);
-	/* FIXME: Locking on Initialized flag */
-	if (!test_bit(ASYNCB_INITIALIZED, &tport->flags))
-		error = isicom_setup_port(tty);
-	if (error == 0)
-		error = tty_port_block_til_ready(&port->port, tty, filp);
-	return error;
+	return tty_port_open(tport, tty, filp);
 }
 
 /* close et all */
@@ -914,40 +898,21 @@ static void isicom_shutdown_port(struct isi_port *port)
 
 	tty = tty_port_tty_get(&port->port);
 
-	if (!(port->port.flags & ASYNC_INITIALIZED)) {
-		tty_kref_put(tty);
-		return;
-	}
-
 	tty_port_free_xmit_buf(&port->port);
-	port->port.flags &= ~ASYNC_INITIALIZED;
-	/* 3rd October 2000 : Vinayak P Risbud */
-	tty_port_tty_set(&port->port, NULL);
-
-	/*Fix done by Anil .S on 30-04-2001
-	remote login through isi port has dtr toggle problem
-	due to which the carrier drops before the password prompt
-	appears on the remote end. Now we drop the dtr only if the
-	HUPCL(Hangup on close) flag is set for the tty*/
-
-	if (C_HUPCL(tty))
-		/* drop dtr on this port */
-		drop_dtr(port);
-
-	/* any other port uninits  */
-	if (tty)
-		set_bit(TTY_IO_ERROR, &tty->flags);
-
 	if (--card->count < 0) {
 		pr_dbg("isicom_shutdown_port: bad board(0x%lx) count %d.\n",
 			card->base, card->count);
 		card->count = 0;
 	}
 
-	/* last port was closed, shutdown that boad too */
-	if (C_HUPCL(tty)) {
-		if (!card->count)
-			isicom_shutdown_board(card);
+	/* last port was closed, shutdown that board too */
+	if (tty && C_HUPCL(tty)) {
+		/* FIXME: this logic is bogus - it's the old logic that was
+		   bogus before but it still wants fixing */
+		if (!card->count) {
+			if (card->status & BOARD_ACTIVE)
+				card->status &= ~BOARD_ACTIVE;
+		}
 	}
 	tty_kref_put(tty);
 }
@@ -968,7 +933,7 @@ static void isicom_flush_buffer(struct tty_struct *tty)
 	tty_wakeup(tty);
 }
 
-static void isicom_close_port(struct tty_port *port)
+static void isicom_shutdown(struct tty_port *port)
 {
 	struct isi_port *ip = container_of(port, struct isi_port, port);
 	struct isi_board *card = ip->card;
@@ -977,10 +942,8 @@ static void isicom_close_port(struct tty_port *port)
 	/* indicate to the card that no more data can be received
 	   on this port */
 	spin_lock_irqsave(&card->card_lock, flags);
-	if (port->flags & ASYNC_INITIALIZED) {
-		card->port_status &= ~(1 << ip->channel);
-		outw(card->port_status, card->base + 0x02);
-	}
+	card->port_status &= ~(1 << ip->channel);
+	outw(card->port_status, card->base + 0x02);
 	isicom_shutdown_port(ip);
 	spin_unlock_irqrestore(&card->card_lock, flags);
 }
@@ -991,12 +954,7 @@ static void isicom_close(struct tty_struct *tty, struct file *filp)
 	struct tty_port *port = &ip->port;
 	if (isicom_paranoia_check(ip, tty->name, "isicom_close"))
 		return;
-
-	if (tty_port_close_start(port, tty, filp) == 0)
-		return;
-	isicom_close_port(port);
-	isicom_flush_buffer(tty);
-	tty_port_close_end(port, tty);
+	tty_port_close(port, tty, filp);
 }
 
 /* write et all */
@@ -1326,15 +1284,9 @@ static void isicom_start(struct tty_struct *tty)
 static void isicom_hangup(struct tty_struct *tty)
 {
 	struct isi_port *port = tty->driver_data;
-	unsigned long flags;
 
 	if (isicom_paranoia_check(port, tty->name, "isicom_hangup"))
 		return;
-
-	spin_lock_irqsave(&port->card->card_lock, flags);
-	isicom_shutdown_port(port);
-	spin_unlock_irqrestore(&port->card->card_lock, flags);
-
 	tty_port_hangup(&port->port);
 }
 
@@ -1367,6 +1319,8 @@ static const struct tty_operations isicom_ops = {
 static const struct tty_port_operations isicom_port_ops = {
 	.carrier_raised		= isicom_carrier_raised,
 	.dtr_rts		= isicom_dtr_rts,
+	.activate		= isicom_activate,
+	.shutdown		= isicom_shutdown,
 };
 
 static int __devinit reset_card(struct pci_dev *pdev,


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 02/12] isicom: sort out the board init logic
  2009-11-18 14:14 [PATCH 00/12] Series short description Alan Cox
  2009-11-18 14:14 ` [PATCH 01/12] isicom: switch to the new tty_port_open helper Alan Cox
@ 2009-11-18 14:14 ` Alan Cox
  2009-11-18 14:15 ` [PATCH 03/12] mxser: use the tty_port_open method Alan Cox
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Alan Cox @ 2009-11-18 14:14 UTC (permalink / raw)
  To: greg, linux-kernel

Split this into two flags - INIT meaning the board is set up and ACTIVE
meaning the board has ports open. Remove the broken HUPCL casing and push
the counts somewhere sensible.

Signed-off-by: Alan Cox <alan@linux.intel.com>
---

 drivers/char/isicom.c  |   41 +++++++++++------------------------------
 include/linux/isicom.h |    1 +
 2 files changed, 12 insertions(+), 30 deletions(-)


diff --git a/drivers/char/isicom.c b/drivers/char/isicom.c
index e7be3ec..1e91c30 100644
--- a/drivers/char/isicom.c
+++ b/drivers/char/isicom.c
@@ -793,21 +793,19 @@ static inline void isicom_setup_board(struct isi_board *bp)
 {
 	int channel;
 	struct isi_port *port;
-	unsigned long flags;
 
-	spin_lock_irqsave(&bp->card_lock, flags);
-	if (bp->status & BOARD_ACTIVE) {
-		spin_unlock_irqrestore(&bp->card_lock, flags);
-		return;
-	}
-	port = bp->ports;
-	bp->status |= BOARD_ACTIVE;
-	for (channel = 0; channel < bp->port_count; channel++, port++)
-		drop_dtr_rts(port);
 	bp->count++;
-	spin_unlock_irqrestore(&bp->card_lock, flags);
+	if (!(bp->status & BOARD_INIT)) {
+		port = bp->ports;
+		for (channel = 0; channel < bp->port_count; channel++, port++)
+			drop_dtr_rts(port);
+	}
+	bp->status |= BOARD_ACTIVE | BOARD_INIT;
 }
 
+/* Activate and thus setup board are protected from races against shutdown
+   by the tty_port mutex */
+
 static int isicom_activate(struct tty_port *tport, struct tty_struct *tty)
 {
 	struct isi_port *port = container_of(tport, struct isi_port, port);
@@ -884,19 +882,10 @@ static int isicom_open(struct tty_struct *tty, struct file *filp)
 
 /* close et all */
 
-static inline void isicom_shutdown_board(struct isi_board *bp)
-{
-	if (bp->status & BOARD_ACTIVE)
-		bp->status &= ~BOARD_ACTIVE;
-}
-
 /* card->lock HAS to be held */
 static void isicom_shutdown_port(struct isi_port *port)
 {
 	struct isi_board *card = port->card;
-	struct tty_struct *tty;
-
-	tty = tty_port_tty_get(&port->port);
 
 	tty_port_free_xmit_buf(&port->port);
 	if (--card->count < 0) {
@@ -904,17 +893,9 @@ static void isicom_shutdown_port(struct isi_port *port)
 			card->base, card->count);
 		card->count = 0;
 	}
-
 	/* last port was closed, shutdown that board too */
-	if (tty && C_HUPCL(tty)) {
-		/* FIXME: this logic is bogus - it's the old logic that was
-		   bogus before but it still wants fixing */
-		if (!card->count) {
-			if (card->status & BOARD_ACTIVE)
-				card->status &= ~BOARD_ACTIVE;
-		}
-	}
-	tty_kref_put(tty);
+	if (!card->count)
+		card->status &= BOARD_ACTIVE;
 }
 
 static void isicom_flush_buffer(struct tty_struct *tty)
diff --git a/include/linux/isicom.h b/include/linux/isicom.h
index bbd4219..b92e056 100644
--- a/include/linux/isicom.h
+++ b/include/linux/isicom.h
@@ -67,6 +67,7 @@
 
 #define		FIRMWARE_LOADED		0x0001
 #define		BOARD_ACTIVE		0x0002
+#define		BOARD_INIT		0x0004
 
  	/* isi_port status bitmap  */
 


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 03/12] mxser: use the tty_port_open method
  2009-11-18 14:14 [PATCH 00/12] Series short description Alan Cox
  2009-11-18 14:14 ` [PATCH 01/12] isicom: switch to the new tty_port_open helper Alan Cox
  2009-11-18 14:14 ` [PATCH 02/12] isicom: sort out the board init logic Alan Cox
@ 2009-11-18 14:15 ` Alan Cox
  2009-11-18 14:15 ` [PATCH 04/12] mxser: Use the new locking rules to fix setserial properly Alan Cox
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Alan Cox @ 2009-11-18 14:15 UTC (permalink / raw)
  To: greg, linux-kernel

At first this looks a fairly trivial conversion but we can't quite push
everything into the right format yet. The open side is easy but care is needed
over the setserial methods. Fix up the locking now that we've adopted the
port->mutex locking rule for the initialization.

Signed-off-by: Alan Cox <alan@linux.intel.com>
---

 drivers/char/mxser.c |  111 ++++++++++++++++----------------------------------
 1 files changed, 35 insertions(+), 76 deletions(-)


diff --git a/drivers/char/mxser.c b/drivers/char/mxser.c
index 5e28d39..9dac516 100644
--- a/drivers/char/mxser.c
+++ b/drivers/char/mxser.c
@@ -856,9 +856,9 @@ static void mxser_check_modem_status(struct tty_struct *tty,
 	}
 }
 
-static int mxser_startup(struct tty_struct *tty)
+static int mxser_activate(struct tty_port *port, struct tty_struct *tty)
 {
-	struct mxser_port *info = tty->driver_data;
+	struct mxser_port *info = container_of(port, struct mxser_port, port);
 	unsigned long page;
 	unsigned long flags;
 
@@ -868,22 +868,13 @@ static int mxser_startup(struct tty_struct *tty)
 
 	spin_lock_irqsave(&info->slock, flags);
 
-	if (info->port.flags & ASYNC_INITIALIZED) {
-		free_page(page);
-		spin_unlock_irqrestore(&info->slock, flags);
-		return 0;
-	}
-
 	if (!info->ioaddr || !info->type) {
 		set_bit(TTY_IO_ERROR, &tty->flags);
 		free_page(page);
 		spin_unlock_irqrestore(&info->slock, flags);
 		return 0;
 	}
-	if (info->port.xmit_buf)
-		free_page(page);
-	else
-		info->port.xmit_buf = (unsigned char *) page;
+	info->port.xmit_buf = (unsigned char *) page;
 
 	/*
 	 * Clear the FIFO buffers and disable them
@@ -951,24 +942,19 @@ static int mxser_startup(struct tty_struct *tty)
 	 * and set the speed of the serial port
 	 */
 	mxser_change_speed(tty, NULL);
-	info->port.flags |= ASYNC_INITIALIZED;
 	spin_unlock_irqrestore(&info->slock, flags);
 
 	return 0;
 }
 
 /*
- * This routine will shutdown a serial port; interrupts maybe disabled, and
- * DTR is dropped if the hangup on close termio flag is on.
+ * This routine will shutdown a serial port
  */
-static void mxser_shutdown(struct tty_struct *tty)
+static void mxser_shutdown_port(struct tty_port *port)
 {
-	struct mxser_port *info = tty->driver_data;
+	struct mxser_port *info = container_of(port, struct mxser_port, port);
 	unsigned long flags;
 
-	if (!(info->port.flags & ASYNC_INITIALIZED))
-		return;
-
 	spin_lock_irqsave(&info->slock, flags);
 
 	/*
@@ -978,7 +964,7 @@ static void mxser_shutdown(struct tty_struct *tty)
 	wake_up_interruptible(&info->port.delta_msr_wait);
 
 	/*
-	 * Free the IRQ, if necessary
+	 * Free the xmit buffer, if necessary
 	 */
 	if (info->port.xmit_buf) {
 		free_page((unsigned long) info->port.xmit_buf);
@@ -988,10 +974,6 @@ static void mxser_shutdown(struct tty_struct *tty)
 	info->IER = 0;
 	outb(0x00, info->ioaddr + UART_IER);
 
-	if (tty->termios->c_cflag & HUPCL)
-		info->MCR &= ~(UART_MCR_DTR | UART_MCR_RTS);
-	outb(info->MCR, info->ioaddr + UART_MCR);
-
 	/* clear Rx/Tx FIFO's */
 	if (info->board->chip_flag)
 		outb(UART_FCR_CLEAR_RCVR | UART_FCR_CLEAR_XMIT |
@@ -1004,9 +986,6 @@ static void mxser_shutdown(struct tty_struct *tty)
 	/* read data port to reset things */
 	(void) inb(info->ioaddr + UART_RX);
 
-	set_bit(TTY_IO_ERROR, &tty->flags);
-
-	info->port.flags &= ~ASYNC_INITIALIZED;
 
 	if (info->board->chip_flag)
 		SET_MOXA_MUST_NO_SOFTWARE_FLOW_CONTROL(info->ioaddr);
@@ -1023,8 +1002,7 @@ static void mxser_shutdown(struct tty_struct *tty)
 static int mxser_open(struct tty_struct *tty, struct file *filp)
 {
 	struct mxser_port *info;
-	unsigned long flags;
-	int retval, line;
+	int line;
 
 	line = tty->index;
 	if (line == MXSER_PORTS)
@@ -1035,23 +1013,7 @@ static int mxser_open(struct tty_struct *tty, struct file *filp)
 	if (!info->ioaddr)
 		return -ENODEV;
 
-	tty->driver_data = info;
-	tty_port_tty_set(&info->port, tty);
-	/*
-	 * Start up serial port
-	 */
-	spin_lock_irqsave(&info->port.lock, flags);
-	info->port.count++;
-	spin_unlock_irqrestore(&info->port.lock, flags);
-	retval = mxser_startup(tty);
-	if (retval)
-		return retval;
-
-	retval = tty_port_block_til_ready(&info->port, tty, filp);
-	if (retval)
-		return retval;
-
-	return 0;
+	return tty_port_open(&info->port, tty, filp);
 }
 
 static void mxser_flush_buffer(struct tty_struct *tty)
@@ -1075,19 +1037,11 @@ static void mxser_flush_buffer(struct tty_struct *tty)
 }
 
 
-static void mxser_close_port(struct tty_struct *tty, struct tty_port *port)
+static void mxser_close_port(struct tty_port *port)
 {
 	struct mxser_port *info = container_of(port, struct mxser_port, port);
 	unsigned long timeout;
 	/*
-	 * Save the termios structure, since this port may have
-	 * separate termios for callout and dialin.
-	 *
-	 * FIXME: Can this go ?
-	 */
-	if (port->flags & ASYNC_NORMAL_ACTIVE)
-		info->normal_termios = *tty->termios;
-	/*
 	 * At this point we stop accepting input.  To do this, we
 	 * disable the receive line status interrupts, and tell the
 	 * interrupt driver to stop checking the data ready bit in the
@@ -1097,22 +1051,18 @@ static void mxser_close_port(struct tty_struct *tty, struct tty_port *port)
 	if (info->board->chip_flag)
 		info->IER &= ~MOXA_MUST_RECV_ISR;
 
-	if (port->flags & ASYNC_INITIALIZED) {
-		outb(info->IER, info->ioaddr + UART_IER);
-		/*
-		 * Before we drop DTR, make sure the UART transmitter
-		 * has completely drained; this is especially
-		 * important if there is a transmit FIFO!
-		 */
-		timeout = jiffies + HZ;
-		while (!(inb(info->ioaddr + UART_LSR) & UART_LSR_TEMT)) {
-			schedule_timeout_interruptible(5);
-			if (time_after(jiffies, timeout))
-				break;
-		}
+	outb(info->IER, info->ioaddr + UART_IER);
+	/*
+	 * Before we drop DTR, make sure the UART transmitter
+	 * has completely drained; this is especially
+	 * important if there is a transmit FIFO!
+	 */
+	timeout = jiffies + HZ;
+	while (!(inb(info->ioaddr + UART_LSR) & UART_LSR_TEMT)) {
+		schedule_timeout_interruptible(5);
+		if (time_after(jiffies, timeout))
+			break;
 	}
-	mxser_shutdown(tty);
-
 }
 
 /*
@@ -1130,8 +1080,12 @@ static void mxser_close(struct tty_struct *tty, struct file *filp)
 		return;
 	if (tty_port_close_start(port, tty, filp) == 0)
 		return;
-	mxser_close_port(tty, port);
+	mutex_lock(&port->mutex);
+	mxser_close_port(port);
 	mxser_flush_buffer(tty);
+	mxser_shutdown_port(port);
+	clear_bit(ASYNCB_INITIALIZED, &port->flags);
+	mutex_unlock(&port->mutex);
 	/* Right now the tty_port set is done outside of the close_end helper
 	   as we don't yet have everyone using refcounts */	
 	tty_port_close_end(port, tty);
@@ -1329,9 +1283,13 @@ static int mxser_set_serial_info(struct tty_struct *tty,
 			mxser_change_speed(tty, NULL);
 			spin_unlock_irqrestore(&info->slock, sl_flags);
 		}
-	} else
-		retval = mxser_startup(tty);
-
+	} else {
+		mutex_lock(&info->port.mutex);
+		retval = mxser_activate(&info->port, tty);
+		if (retval == 0)
+			set_bit(ASYNCB_INITIALIZED, &info->port.flags);
+		mutex_unlock(&info->port.mutex);
+	}
 	return retval;
 }
 
@@ -2059,7 +2017,6 @@ static void mxser_hangup(struct tty_struct *tty)
 	struct mxser_port *info = tty->driver_data;
 
 	mxser_flush_buffer(tty);
-	mxser_shutdown(tty);
 	tty_port_hangup(&info->port);
 }
 
@@ -2363,6 +2320,8 @@ static const struct tty_operations mxser_ops = {
 struct tty_port_operations mxser_port_ops = {
 	.carrier_raised = mxser_carrier_raised,
 	.dtr_rts = mxser_dtr_rts,
+	.activate = mxser_activate,
+	.shutdown = mxser_shutdown_port,
 };
 
 /*


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 04/12] mxser: Use the new locking rules to fix setserial properly
  2009-11-18 14:14 [PATCH 00/12] Series short description Alan Cox
                   ` (2 preceding siblings ...)
  2009-11-18 14:15 ` [PATCH 03/12] mxser: use the tty_port_open method Alan Cox
@ 2009-11-18 14:15 ` Alan Cox
  2009-11-20 12:08   ` Dan Carpenter
  2009-11-18 14:15 ` [PATCH 05/12] isicom: fix deadlock on shutdown Alan Cox
                   ` (7 subsequent siblings)
  11 siblings, 1 reply; 14+ messages in thread
From: Alan Cox @ 2009-11-18 14:15 UTC (permalink / raw)
  To: greg, linux-kernel

Propogate the init/shutdown mutex through the setserial logic. Use the proper
locks for the various bits still using the BKL. Kill the BKL in this driver.

Signed-off-by: Alan Cox <alan@linux.intel.com>
---

 drivers/char/mxser.c |  145 +++++++++++++++++++++++++++-----------------------
 1 files changed, 78 insertions(+), 67 deletions(-)


diff --git a/drivers/char/mxser.c b/drivers/char/mxser.c
index 9dac516..4c803c7 100644
--- a/drivers/char/mxser.c
+++ b/drivers/char/mxser.c
@@ -23,7 +23,6 @@
 #include <linux/errno.h>
 #include <linux/signal.h>
 #include <linux/sched.h>
-#include <linux/smp_lock.h>
 #include <linux/timer.h>
 #include <linux/interrupt.h>
 #include <linux/tty.h>
@@ -1229,6 +1228,7 @@ static int mxser_set_serial_info(struct tty_struct *tty,
 		struct serial_struct __user *new_info)
 {
 	struct mxser_port *info = tty->driver_data;
+	struct tty_port *port = &info->port;
 	struct serial_struct new_serial;
 	speed_t baud;
 	unsigned long sl_flags;
@@ -1244,7 +1244,7 @@ static int mxser_set_serial_info(struct tty_struct *tty,
 			new_serial.port != info->ioaddr)
 		return -EINVAL;
 
-	flags = info->port.flags & ASYNC_SPD_MASK;
+	flags = port->flags & ASYNC_SPD_MASK;
 
 	if (!capable(CAP_SYS_ADMIN)) {
 		if ((new_serial.baud_base != info->baud_base) ||
@@ -1258,16 +1258,17 @@ static int mxser_set_serial_info(struct tty_struct *tty,
 		 * OK, past this point, all the error checking has been done.
 		 * At this point, we start making changes.....
 		 */
-		info->port.flags = ((info->port.flags & ~ASYNC_FLAGS) |
+		port->flags = ((port->flags & ~ASYNC_FLAGS) |
 				(new_serial.flags & ASYNC_FLAGS));
-		info->port.close_delay = new_serial.close_delay * HZ / 100;
-		info->port.closing_wait = new_serial.closing_wait * HZ / 100;
-		tty->low_latency = (info->port.flags & ASYNC_LOW_LATENCY)
-								? 1 : 0;
-		if ((info->port.flags & ASYNC_SPD_MASK) == ASYNC_SPD_CUST &&
+		port->close_delay = new_serial.close_delay * HZ / 100;
+		port->closing_wait = new_serial.closing_wait * HZ / 100;
+		tty->low_latency = (port->flags & ASYNC_LOW_LATENCY) ? 1 : 0;
+		if ((port->flags & ASYNC_SPD_MASK) == ASYNC_SPD_CUST &&
 				(new_serial.baud_base != info->baud_base ||
 				new_serial.custom_divisor !=
 				info->custom_divisor)) {
+			if (new_serial.custom_divisor == 0)
+				return -EINVAL;
 			baud = new_serial.baud_base / new_serial.custom_divisor;
 			tty_encode_baud_rate(tty, baud, baud);
 		}
@@ -1277,18 +1278,16 @@ static int mxser_set_serial_info(struct tty_struct *tty,
 
 	process_txrx_fifo(info);
 
-	if (info->port.flags & ASYNC_INITIALIZED) {
-		if (flags != (info->port.flags & ASYNC_SPD_MASK)) {
+	if (test_bit(ASYNCB_INITIALIZED, &port->flags)) {
+		if (flags != (port->flags & ASYNC_SPD_MASK)) {
 			spin_lock_irqsave(&info->slock, sl_flags);
 			mxser_change_speed(tty, NULL);
 			spin_unlock_irqrestore(&info->slock, sl_flags);
 		}
 	} else {
-		mutex_lock(&info->port.mutex);
-		retval = mxser_activate(&info->port, tty);
+		retval = mxser_activate(port, tty);
 		if (retval == 0)
-			set_bit(ASYNCB_INITIALIZED, &info->port.flags);
-		mutex_unlock(&info->port.mutex);
+			set_bit(ASYNCB_INITIALIZED, &port->flags);
 	}
 	return retval;
 }
@@ -1478,7 +1477,8 @@ static int __init mxser_read_register(int port, unsigned short *regs)
 
 static int mxser_ioctl_special(unsigned int cmd, void __user *argp)
 {
-	struct mxser_port *port;
+	struct mxser_port *ip;
+	struct tty_port *port;
 	struct tty_struct *tty;
 	int result, status;
 	unsigned int i, j;
@@ -1494,38 +1494,39 @@ static int mxser_ioctl_special(unsigned int cmd, void __user *argp)
 
 	case MOXA_CHKPORTENABLE:
 		result = 0;
-		lock_kernel();
 		for (i = 0; i < MXSER_BOARDS; i++)
 			for (j = 0; j < MXSER_PORTS_PER_BOARD; j++)
 				if (mxser_boards[i].ports[j].ioaddr)
 					result |= (1 << i);
-		unlock_kernel();
 		return put_user(result, (unsigned long __user *)argp);
 	case MOXA_GETDATACOUNT:
-		lock_kernel();
+		/* The receive side is locked by port->slock but it isn't
+		   clear that an exact snapshot is worth copying here */
 		if (copy_to_user(argp, &mxvar_log, sizeof(mxvar_log)))
 			ret = -EFAULT;
-		unlock_kernel();
 		return ret;
 	case MOXA_GETMSTATUS: {
 		struct mxser_mstatus ms, __user *msu = argp;
-		lock_kernel();
 		for (i = 0; i < MXSER_BOARDS; i++)
 			for (j = 0; j < MXSER_PORTS_PER_BOARD; j++) {
-				port = &mxser_boards[i].ports[j];
+				ip = &mxser_boards[i].ports[j];
+				port = &ip->port;
 				memset(&ms, 0, sizeof(ms));
 
-				if (!port->ioaddr)
+				mutex_lock(&port->mutex);
+				if (!ip->ioaddr)
 					goto copy;
 				
-				tty = tty_port_tty_get(&port->port);
+				tty = tty_port_tty_get(port);
 
 				if (!tty || !tty->termios)
-					ms.cflag = port->normal_termios.c_cflag;
+					ms.cflag = ip->normal_termios.c_cflag;
 				else
 					ms.cflag = tty->termios->c_cflag;
 				tty_kref_put(tty);
-				status = inb(port->ioaddr + UART_MSR);
+				spin_lock_irq(&ip->slock);
+				status = inb(ip->ioaddr + UART_MSR);
+				spin_unlock_irq(&ip->slock);
 				if (status & UART_MSR_DCD)
 					ms.dcd = 1;
 				if (status & UART_MSR_DSR)
@@ -1533,13 +1534,11 @@ static int mxser_ioctl_special(unsigned int cmd, void __user *argp)
 				if (status & UART_MSR_CTS)
 					ms.cts = 1;
 			copy:
-				if (copy_to_user(msu, &ms, sizeof(ms))) {
-					unlock_kernel();
+				mutex_unlock(&port->mutex);
+				if (copy_to_user(msu, &ms, sizeof(ms)))
 					return -EFAULT;
-				}
 				msu++;
 			}
-		unlock_kernel();
 		return 0;
 	}
 	case MOXA_ASPP_MON_EXT: {
@@ -1551,41 +1550,48 @@ static int mxser_ioctl_special(unsigned int cmd, void __user *argp)
 		if (!me)
 			return -ENOMEM;
 
-		lock_kernel();
 		for (i = 0, p = 0; i < MXSER_BOARDS; i++) {
 			for (j = 0; j < MXSER_PORTS_PER_BOARD; j++, p++) {
 				if (p >= ARRAY_SIZE(me->rx_cnt)) {
 					i = MXSER_BOARDS;
 					break;
 				}
-				port = &mxser_boards[i].ports[j];
-				if (!port->ioaddr)
+				ip = &mxser_boards[i].ports[j];
+				port = &ip->port;
+
+				mutex_lock(&port->mutex);
+				if (!ip->ioaddr) {
+					mutex_unlock(&port->mutex);
 					continue;
+				}
 
-				status = mxser_get_msr(port->ioaddr, 0, p);
+				spin_lock_irq(&ip->slock);
+				status = mxser_get_msr(ip->ioaddr, 0, p);
 
 				if (status & UART_MSR_TERI)
-					port->icount.rng++;
+					ip->icount.rng++;
 				if (status & UART_MSR_DDSR)
-					port->icount.dsr++;
+					ip->icount.dsr++;
 				if (status & UART_MSR_DDCD)
-					port->icount.dcd++;
+					ip->icount.dcd++;
 				if (status & UART_MSR_DCTS)
-					port->icount.cts++;
+					ip->icount.cts++;
 
-				port->mon_data.modem_status = status;
-				me->rx_cnt[p] = port->mon_data.rxcnt;
-				me->tx_cnt[p] = port->mon_data.txcnt;
-				me->up_rxcnt[p] = port->mon_data.up_rxcnt;
-				me->up_txcnt[p] = port->mon_data.up_txcnt;
+				ip->mon_data.modem_status = status;
+				me->rx_cnt[p] = ip->mon_data.rxcnt;
+				me->tx_cnt[p] = ip->mon_data.txcnt;
+				me->up_rxcnt[p] = ip->mon_data.up_rxcnt;
+				me->up_txcnt[p] = ip->mon_data.up_txcnt;
 				me->modem_status[p] =
-					port->mon_data.modem_status;
-				tty = tty_port_tty_get(&port->port);
+					ip->mon_data.modem_status;
+				spin_unlock_irq(&ip->slock);
+
+				tty = tty_port_tty_get(&ip->port);
 
 				if (!tty || !tty->termios) {
-					cflag = port->normal_termios.c_cflag;
-					iflag = port->normal_termios.c_iflag;
-					me->baudrate[p] = tty_termios_baud_rate(&port->normal_termios);
+					cflag = ip->normal_termios.c_cflag;
+					iflag = ip->normal_termios.c_iflag;
+					me->baudrate[p] = tty_termios_baud_rate(&ip->normal_termios);
 				} else {
 					cflag = tty->termios->c_cflag;
 					iflag = tty->termios->c_iflag;
@@ -1604,16 +1610,15 @@ static int mxser_ioctl_special(unsigned int cmd, void __user *argp)
 				if (iflag & (IXON | IXOFF))
 					me->flowctrl[p] |= 0x0C;
 
-				if (port->type == PORT_16550A)
+				if (ip->type == PORT_16550A)
 					me->fifo[p] = 1;
 
-				opmode = inb(port->opmode_ioaddr) >>
-						((p % 4) * 2);
+				opmode = inb(ip->opmode_ioaddr)>>((p % 4) * 2);
 				opmode &= OP_MODE_MASK;
 				me->iftype[p] = opmode;
+				mutex_unlock(&port->mutex);
 			}
 		}
-		unlock_kernel();
 		if (copy_to_user(argp, me, sizeof(*me)))
 			ret = -EFAULT;
 		kfree(me);
@@ -1650,6 +1655,7 @@ static int mxser_ioctl(struct tty_struct *tty, struct file *file,
 		unsigned int cmd, unsigned long arg)
 {
 	struct mxser_port *info = tty->driver_data;
+	struct tty_port *port = &info->port;
 	struct async_icount cnow;
 	unsigned long flags;
 	void __user *argp = (void __user *)arg;
@@ -1674,20 +1680,20 @@ static int mxser_ioctl(struct tty_struct *tty, struct file *file,
 					opmode != RS422_MODE &&
 					opmode != RS485_4WIRE_MODE)
 				return -EFAULT;
-			lock_kernel();
 			mask = ModeMask[p];
 			shiftbit = p * 2;
+			spin_lock_irq(&info->slock);
 			val = inb(info->opmode_ioaddr);
 			val &= mask;
 			val |= (opmode << shiftbit);
 			outb(val, info->opmode_ioaddr);
-			unlock_kernel();
+			spin_unlock_irq(&info->slock);
 		} else {
-			lock_kernel();
 			shiftbit = p * 2;
+			spin_lock_irq(&info->slock);
 			opmode = inb(info->opmode_ioaddr) >> shiftbit;
+			spin_unlock_irq(&info->slock);
 			opmode &= OP_MODE_MASK;
-			unlock_kernel();
 			if (put_user(opmode, (int __user *)argp))
 				return -EFAULT;
 		}
@@ -1700,14 +1706,14 @@ static int mxser_ioctl(struct tty_struct *tty, struct file *file,
 
 	switch (cmd) {
 	case TIOCGSERIAL:
-		lock_kernel();
+		mutex_lock(&port->mutex);
 		retval = mxser_get_serial_info(tty, argp);
-		unlock_kernel();
+		mutex_unlock(&port->mutex);
 		return retval;
 	case TIOCSSERIAL:
-		lock_kernel();
+		mutex_lock(&port->mutex);
 		retval = mxser_set_serial_info(tty, argp);
-		unlock_kernel();
+		mutex_unlock(&port->mutex);
 		return retval;
 	case TIOCSERGETLSR:	/* Get line status register */
 		return  mxser_get_lsr_info(info, argp);
@@ -1753,31 +1759,33 @@ static int mxser_ioctl(struct tty_struct *tty, struct file *file,
 	case MOXA_HighSpeedOn:
 		return put_user(info->baud_base != 115200 ? 1 : 0, (int __user *)argp);
 	case MOXA_SDS_RSTICOUNTER:
-		lock_kernel();
+		spin_lock_irq(&info->slock);
 		info->mon_data.rxcnt = 0;
 		info->mon_data.txcnt = 0;
-		unlock_kernel();
+		spin_unlock_irq(&info->slock);
 		return 0;
 
 	case MOXA_ASPP_OQUEUE:{
 		int len, lsr;
 
-		lock_kernel();
 		len = mxser_chars_in_buffer(tty);
+		spin_lock(&info->slock);
 		lsr = inb(info->ioaddr + UART_LSR) & UART_LSR_THRE;
+		spin_unlock_irq(&info->slock);
 		len += (lsr ? 0 : 1);
-		unlock_kernel();
 
 		return put_user(len, (int __user *)argp);
 	}
 	case MOXA_ASPP_MON: {
 		int mcr, status;
 
-		lock_kernel();
+		spin_lock(&info->slock);
 		status = mxser_get_msr(info->ioaddr, 1, tty->index);
 		mxser_check_modem_status(tty, info, status);
 
 		mcr = inb(info->ioaddr + UART_MCR);
+		spin_unlock(&info->slock);
+		
 		if (mcr & MOXA_MUST_MCR_XON_FLAG)
 			info->mon_data.hold_reason &= ~NPPI_NOTIFY_XOFFHOLD;
 		else
@@ -1792,7 +1800,7 @@ static int mxser_ioctl(struct tty_struct *tty, struct file *file,
 			info->mon_data.hold_reason |= NPPI_NOTIFY_CTSHOLD;
 		else
 			info->mon_data.hold_reason &= ~NPPI_NOTIFY_CTSHOLD;
-		unlock_kernel();
+
 		if (copy_to_user(argp, &info->mon_data,
 				sizeof(struct mxser_mon)))
 			return -EFAULT;
@@ -1951,6 +1959,7 @@ static void mxser_wait_until_sent(struct tty_struct *tty, int timeout)
 {
 	struct mxser_port *info = tty->driver_data;
 	unsigned long orig_jiffies, char_time;
+	unsigned long flags;
 	int lsr;
 
 	if (info->type == PORT_UNKNOWN)
@@ -1990,19 +1999,21 @@ static void mxser_wait_until_sent(struct tty_struct *tty, int timeout)
 		timeout, char_time);
 	printk("jiff=%lu...", jiffies);
 #endif
-	lock_kernel();
+	spin_lock_irqsave(&info->slock, flags);
 	while (!((lsr = inb(info->ioaddr + UART_LSR)) & UART_LSR_TEMT)) {
 #ifdef SERIAL_DEBUG_RS_WAIT_UNTIL_SENT
 		printk("lsr = %d (jiff=%lu)...", lsr, jiffies);
 #endif
+		spin_unlock_irqrestore(&info->slock, flags);
 		schedule_timeout_interruptible(char_time);
 		if (signal_pending(current))
 			break;
 		if (timeout && time_after(jiffies, orig_jiffies + timeout))
 			break;
+		spin_lock_irqsave(&info->slock, flags);
 	}
+	spin_unlock_irqrestore(&info->slock, flags);
 	set_current_state(TASK_RUNNING);
-	unlock_kernel();
 
 #ifdef SERIAL_DEBUG_RS_WAIT_UNTIL_SENT
 	printk("lsr = %d (jiff=%lu)...done\n", lsr, jiffies);


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 05/12] isicom: fix deadlock on shutdown
  2009-11-18 14:14 [PATCH 00/12] Series short description Alan Cox
                   ` (3 preceding siblings ...)
  2009-11-18 14:15 ` [PATCH 04/12] mxser: Use the new locking rules to fix setserial properly Alan Cox
@ 2009-11-18 14:15 ` Alan Cox
  2009-11-18 14:15 ` [PATCH 06/12] moxa: Use more tty_port ops Alan Cox
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Alan Cox @ 2009-11-18 14:15 UTC (permalink / raw)
  To: greg, linux-kernel

Alexander Strakh <strakh@ispras.ru> reported

	KERNEL_VERSION: 2.6.31
	DESCRIBE:
Driver drivers/char/isicom.c might sleep in atomic  context, because it
calls 
tty_port_xmit_buf under spin_lock.

./drivers/char/isicom.c:
1307 static void isicom_hangup(struct tty_struct *tty)
1308 {
...
1315         spin_lock_irqsave(&port->card->card_lock, flags);
1316         isicom_shutdown_port(port);
...

Path to might_sleep macro from isicom_hangup:
1. isicom_hangup calls spin_lock_irqsave (drivers/char/isicom.c:1315) and
then 
calls isicom_shutdown_port.
2. isiscom_shutdown_port calls tty_port_free_xmit_buf at 
drivers/char/isicom.c:906
3. tty_port_free_xmit_buf calls mutex_lock at drivers/char/tty_port:48

Found by Linux Driver Verification Project.

Signed-off-by: Alan Cox <alan@linux.intel.com>
---

 drivers/char/isicom.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)


diff --git a/drivers/char/isicom.c b/drivers/char/isicom.c
index 1e91c30..300d5bd 100644
--- a/drivers/char/isicom.c
+++ b/drivers/char/isicom.c
@@ -887,7 +887,6 @@ static void isicom_shutdown_port(struct isi_port *port)
 {
 	struct isi_board *card = port->card;
 
-	tty_port_free_xmit_buf(&port->port);
 	if (--card->count < 0) {
 		pr_dbg("isicom_shutdown_port: bad board(0x%lx) count %d.\n",
 			card->base, card->count);
@@ -927,6 +926,7 @@ static void isicom_shutdown(struct tty_port *port)
 	outw(card->port_status, card->base + 0x02);
 	isicom_shutdown_port(ip);
 	spin_unlock_irqrestore(&card->card_lock, flags);
+	tty_port_free_xmit_buf(port);
 }
 
 static void isicom_close(struct tty_struct *tty, struct file *filp)


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 06/12] moxa: Use more tty_port ops
  2009-11-18 14:14 [PATCH 00/12] Series short description Alan Cox
                   ` (4 preceding siblings ...)
  2009-11-18 14:15 ` [PATCH 05/12] isicom: fix deadlock on shutdown Alan Cox
@ 2009-11-18 14:15 ` Alan Cox
  2009-11-18 14:15 ` [PATCH 07/12] moxa: rework the locking a bit Alan Cox
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Alan Cox @ 2009-11-18 14:15 UTC (permalink / raw)
  To: greg, linux-kernel

Rework a few bits of this into tty_port format

Signed-off-by: Alan Cox <alan@linux.intel.com>
---

 drivers/char/moxa.c |  139 ++++++++-------------------------------------------
 1 files changed, 22 insertions(+), 117 deletions(-)


diff --git a/drivers/char/moxa.c b/drivers/char/moxa.c
index dd0083b..96b2985 100644
--- a/drivers/char/moxa.c
+++ b/drivers/char/moxa.c
@@ -206,8 +206,9 @@ static int moxa_tiocmset(struct tty_struct *tty, struct file *file,
 static void moxa_poll(unsigned long);
 static void moxa_set_tty_param(struct tty_struct *, struct ktermios *);
 static void moxa_setup_empty_event(struct tty_struct *);
-static void moxa_shut_down(struct tty_struct *);
+static void moxa_shutdown(struct tty_port *);
 static int moxa_carrier_raised(struct tty_port *);
+static void moxa_dtr_rts(struct tty_port *, int);
 /*
  * moxa board interface functions:
  */
@@ -409,6 +410,8 @@ static const struct tty_operations moxa_ops = {
 
 static const struct tty_port_operations moxa_port_ops = {
 	.carrier_raised = moxa_carrier_raised,
+	.dtr_rts = moxa_dtr_rts,
+	.shutdown = moxa_shutdown,
 };
 
 static struct tty_driver *moxaDriver;
@@ -1112,14 +1115,12 @@ static void __exit moxa_exit(void)
 module_init(moxa_init);
 module_exit(moxa_exit);
 
-static void moxa_close_port(struct tty_struct *tty)
+static void moxa_shutdown(struct tty_port *port)
 {
-	struct moxa_port *ch = tty->driver_data;
-	moxa_shut_down(tty);
+	struct moxa_port *ch = container_of(port, struct moxa_port, port);
+        MoxaPortDisable(ch);
 	MoxaPortFlushData(ch, 2);
-	ch->port.flags &= ~ASYNC_NORMAL_ACTIVE;
-	tty->driver_data = NULL;
-	tty_port_tty_set(&ch->port, NULL);
+	clear_bit(ASYNCB_NORMAL_ACTIVE, &port->flags);
 }
 
 static int moxa_carrier_raised(struct tty_port *port)
@@ -1133,39 +1134,13 @@ static int moxa_carrier_raised(struct tty_port *port)
 	return dcd;
 }
 
-static int moxa_block_till_ready(struct tty_struct *tty, struct file *filp,
-			    struct moxa_port *ch)
+static void moxa_dtr_rts(struct tty_port *port, int onoff)
 {
-	struct tty_port *port = &ch->port;
-	DEFINE_WAIT(wait);
-	int retval = 0;
-	u8 dcd;
-
-	while (1) {
-		prepare_to_wait(&port->open_wait, &wait, TASK_INTERRUPTIBLE);
-		if (tty_hung_up_p(filp)) {
-#ifdef SERIAL_DO_RESTART
-			retval = -ERESTARTSYS;
-#else
-			retval = -EAGAIN;
-#endif
-			break;
-		}
-		dcd = tty_port_carrier_raised(port);
-		if (dcd)
-			break;
-
-		if (signal_pending(current)) {
-			retval = -ERESTARTSYS;
-			break;
-		}
-		schedule();
-	}
-	finish_wait(&port->open_wait, &wait);
-
-	return retval;
+	struct moxa_port *ch = container_of(port, struct moxa_port, port);
+	MoxaPortLineCtrl(ch, onoff, onoff);
 }
 
+
 static int moxa_open(struct tty_struct *tty, struct file *filp)
 {
 	struct moxa_board_conf *brd;
@@ -1194,6 +1169,7 @@ static int moxa_open(struct tty_struct *tty, struct file *filp)
 	ch->port.count++;
 	tty->driver_data = ch;
 	tty_port_tty_set(&ch->port, tty);
+	mutex_lock(&ch->port.mutex);
 	if (!(ch->port.flags & ASYNC_INITIALIZED)) {
 		ch->statusflags = 0;
 		moxa_set_tty_param(tty, tty->termios);
@@ -1202,57 +1178,21 @@ static int moxa_open(struct tty_struct *tty, struct file *filp)
 		MoxaSetFifo(ch, ch->type == PORT_16550A);
 		ch->port.flags |= ASYNC_INITIALIZED;
 	}
+	mutex_unlock(&ch->port.mutex);
 	mutex_unlock(&moxa_openlock);
-
-	retval = 0;
-	if (!(filp->f_flags & O_NONBLOCK) && !C_CLOCAL(tty))
-		retval = moxa_block_till_ready(tty, filp, ch);
-	mutex_lock(&moxa_openlock);
-	if (retval) {
-		if (ch->port.count) /* 0 means already hung up... */
-			if (--ch->port.count == 0)
-				moxa_close_port(tty);
-	} else
-		ch->port.flags |= ASYNC_NORMAL_ACTIVE;
-	mutex_unlock(&moxa_openlock);
-
+	
+	retval = tty_port_block_til_ready(&ch->port, tty, filp);
+	if (retval == 0)
+	        set_bit(ASYNCB_NORMAL_ACTIVE, &ch->port.flags);
 	return retval;
 }
 
 static void moxa_close(struct tty_struct *tty, struct file *filp)
 {
-	struct moxa_port *ch;
-	int port;
-
-	port = tty->index;
-	if (port == MAX_PORTS || tty_hung_up_p(filp))
-		return;
-
-	mutex_lock(&moxa_openlock);
-	ch = tty->driver_data;
-	if (ch == NULL)
-		goto unlock;
-	if (tty->count == 1 && ch->port.count != 1) {
-		printk(KERN_WARNING "moxa_close: bad serial port count; "
-			"tty->count is 1, ch->port.count is %d\n", ch->port.count);
-		ch->port.count = 1;
-	}
-	if (--ch->port.count < 0) {
-		printk(KERN_WARNING "moxa_close: bad serial port count, "
-			"device=%s\n", tty->name);
-		ch->port.count = 0;
-	}
-	if (ch->port.count)
-		goto unlock;
-
+	struct moxa_port *ch = tty->driver_data;
 	ch->cflag = tty->termios->c_cflag;
-	if (ch->port.flags & ASYNC_INITIALIZED) {
-		moxa_setup_empty_event(tty);
-		tty_wait_until_sent(tty, 30 * HZ);	/* 30 seconds timeout */
-	}
-
-	moxa_close_port(tty);
-unlock:
+	mutex_lock(&moxa_openlock);
+	tty_port_close(&ch->port, tty, filp);
 	mutex_unlock(&moxa_openlock);
 }
 
@@ -1300,14 +1240,6 @@ static int moxa_chars_in_buffer(struct tty_struct *tty)
 	struct moxa_port *ch = tty->driver_data;
 	int chars;
 
-	/*
-	 * Sigh...I have to check if driver_data is NULL here, because
-	 * if an open() fails, the TTY subsystem eventually calls
-	 * tty_wait_until_sent(), which calls the driver's chars_in_buffer()
-	 * routine.  And since the open() failed, we return 0 here.  TDJ
-	 */
-	if (ch == NULL)
-		return 0;
 	lock_kernel();
 	chars = MoxaPortTxQueue(ch);
 	if (chars) {
@@ -1436,15 +1368,8 @@ static void moxa_hangup(struct tty_struct *tty)
 
 	mutex_lock(&moxa_openlock);
 	ch = tty->driver_data;
-	if (ch == NULL) {
-		mutex_unlock(&moxa_openlock);
-		return;
-	}
-	ch->port.count = 0;
-	moxa_close_port(tty);
+	tty_port_hangup(&ch->port);
 	mutex_unlock(&moxa_openlock);
-
-	wake_up_interruptible(&ch->port.open_wait);
 }
 
 static void moxa_new_dcdstate(struct moxa_port *p, u8 dcd)
@@ -1597,26 +1522,6 @@ static void moxa_setup_empty_event(struct tty_struct *tty)
 	spin_unlock_bh(&moxa_lock);
 }
 
-static void moxa_shut_down(struct tty_struct *tty)
-{
-	struct moxa_port *ch = tty->driver_data;
-
-	if (!(ch->port.flags & ASYNC_INITIALIZED))
-		return;
-
-	MoxaPortDisable(ch);
-
-	/*
-	 * If we're a modem control device and HUPCL is on, drop RTS & DTR.
-	 */
-	if (C_HUPCL(tty))
-		MoxaPortLineCtrl(ch, 0, 0);
-
-	spin_lock_bh(&moxa_lock);
-	ch->port.flags &= ~ASYNC_INITIALIZED;
-	spin_unlock_bh(&moxa_lock);
-}
-
 /*****************************************************************************
  *	Driver level functions: 					     *
  *****************************************************************************/


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 07/12] moxa: rework the locking a bit
  2009-11-18 14:14 [PATCH 00/12] Series short description Alan Cox
                   ` (5 preceding siblings ...)
  2009-11-18 14:15 ` [PATCH 06/12] moxa: Use more tty_port ops Alan Cox
@ 2009-11-18 14:15 ` Alan Cox
  2009-11-18 14:16 ` [PATCH 08/12] Locking clean up Alan Cox
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Alan Cox @ 2009-11-18 14:15 UTC (permalink / raw)
  To: greg, linux-kernel

Introduce a lock for moxafunc() to protect the cases where were get collisions
between two function requests at the same time.

Signed-off-by: Alan Cox <alan@linux.intel.com>
---

 drivers/char/moxa.c |   27 ++++++++++++++++++++++-----
 1 files changed, 22 insertions(+), 5 deletions(-)


diff --git a/drivers/char/moxa.c b/drivers/char/moxa.c
index 96b2985..ba84c4d 100644
--- a/drivers/char/moxa.c
+++ b/drivers/char/moxa.c
@@ -248,9 +248,25 @@ static void moxa_wait_finish(void __iomem *ofsAddr)
 
 static void moxafunc(void __iomem *ofsAddr, u16 cmd, u16 arg)
 {
+        unsigned long flags;
+        spin_lock_irqsave(&moxafunc_lock, flags);
 	writew(arg, ofsAddr + FuncArg);
 	writew(cmd, ofsAddr + FuncCode);
 	moxa_wait_finish(ofsAddr);
+	spin_unlock_irqrestore(&moxafunc_lock, flags);
+}
+
+static int moxafuncret(void __iomem *ofsAddr, u16 cmd, u16 arg)
+{
+        unsigned long flags;
+        u16 ret;
+        spin_lock_irqsave(&moxafunc_lock, flags);
+	writew(arg, ofsAddr + FuncArg);
+	writew(cmd, ofsAddr + FuncCode);
+	moxa_wait_finish(ofsAddr);
+	ret = readw(ofsAddr + FuncArg);
+	spin_unlock_irqrestore(&moxafunc_lock, flags);
+	return ret;
 }
 
 static void moxa_low_water_check(void __iomem *ofsAddr)
@@ -417,6 +433,7 @@ static const struct tty_port_operations moxa_port_ops = {
 static struct tty_driver *moxaDriver;
 static DEFINE_TIMER(moxaTimer, moxa_poll, 0, 0);
 static DEFINE_SPINLOCK(moxa_lock);
+static DEFINE_SPINLOCK(moxafunc_lock);
 
 /*
  * HW init
@@ -1823,10 +1840,12 @@ static int MoxaPortSetTermio(struct moxa_port *port, struct ktermios *termio,
 	baud = MoxaPortSetBaud(port, baud);
 
 	if (termio->c_iflag & (IXON | IXOFF | IXANY)) {
+	        spin_lock_irq(&moxafunc_lock);
 		writeb(termio->c_cc[VSTART], ofsAddr + FuncArg);
 		writeb(termio->c_cc[VSTOP], ofsAddr + FuncArg1);
 		writeb(FC_SetXonXoff, ofsAddr + FuncCode);
 		moxa_wait_finish(ofsAddr);
+		spin_unlock_irqrestore(&moxafunc_lock);
 
 	}
 	return baud;
@@ -1879,12 +1898,10 @@ static int MoxaPortLineStatus(struct moxa_port *port)
 	int val;
 
 	ofsAddr = port->tableAddr;
-	if (MOXA_IS_320(port->board)) {
-		moxafunc(ofsAddr, FC_LineStatus, 0);
-		val = readw(ofsAddr + FuncArg);
-	} else {
+	if (MOXA_IS_320(port->board))
+		val = moxafuncret(ofsAddr, FC_LineStatus, 0);
+	else
 		val = readw(ofsAddr + FlagStat) >> 4;
-	}
 	val &= 0x0B;
 	if (val & 8)
 		val |= 4;


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 08/12] Locking clean up
  2009-11-18 14:14 [PATCH 00/12] Series short description Alan Cox
                   ` (6 preceding siblings ...)
  2009-11-18 14:15 ` [PATCH 07/12] moxa: rework the locking a bit Alan Cox
@ 2009-11-18 14:16 ` Alan Cox
  2009-11-18 14:16 ` [PATCH 09/12] moxa: Kill off the throttle method Alan Cox
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Alan Cox @ 2009-11-18 14:16 UTC (permalink / raw)
  To: greg, linux-kernel

- The open lock is needed to fix up the case of a board reset occuring during
  tty open but too early for a sane hangup response.
- The lock however got used for other cases
- Use the port mutex for get/setserial
- Fix up the confused lack of locking on the THROTTLE and other bits in the
  private flags. Just use set/test/clear bit and it covers the cases we need

Signed-off-by: Alan Cox <alan@linux.intel.com>
---

 drivers/char/moxa.c |   58 ++++++++++++++++++++++-----------------------------
 1 files changed, 25 insertions(+), 33 deletions(-)


diff --git a/drivers/char/moxa.c b/drivers/char/moxa.c
index ba84c4d..e6a79b0 100644
--- a/drivers/char/moxa.c
+++ b/drivers/char/moxa.c
@@ -151,10 +151,10 @@ struct mon_str {
 };
 
 /* statusflags */
-#define TXSTOPPED	0x1
-#define LOWWAIT 	0x2
-#define EMPTYWAIT	0x4
-#define THROTTLE	0x8
+#define TXSTOPPED	1
+#define LOWWAIT 	2
+#define EMPTYWAIT	3
+#define THROTTLE	4
 
 #define SERIAL_DO_RESTART
 
@@ -235,6 +235,8 @@ static void MoxaSetFifo(struct moxa_port *port, int enable);
  * I/O functions
  */
 
+static DEFINE_SPINLOCK(moxafunc_lock);
+
 static void moxa_wait_finish(void __iomem *ofsAddr)
 {
 	unsigned long end = jiffies + moxaFuncTout;
@@ -381,14 +383,14 @@ copy:
 		break;
 	}
 	case TIOCGSERIAL:
-		mutex_lock(&moxa_openlock);
+	        mutex_lock(&ch->port.mutex);
 		ret = moxa_get_serial_info(ch, argp);
-		mutex_unlock(&moxa_openlock);
+		mutex_unlock(&ch->port.mutex);
 		break;
 	case TIOCSSERIAL:
-		mutex_lock(&moxa_openlock);
+	        mutex_lock(&ch->port.mutex);
 		ret = moxa_set_serial_info(ch, argp);
-		mutex_unlock(&moxa_openlock);
+		mutex_unlock(&ch->port.mutex);
 		break;
 	default:
 		ret = -ENOIOCTLCMD;
@@ -433,7 +435,6 @@ static const struct tty_port_operations moxa_port_ops = {
 static struct tty_driver *moxaDriver;
 static DEFINE_TIMER(moxaTimer, moxa_poll, 0, 0);
 static DEFINE_SPINLOCK(moxa_lock);
-static DEFINE_SPINLOCK(moxafunc_lock);
 
 /*
  * HW init
@@ -1208,9 +1209,7 @@ static void moxa_close(struct tty_struct *tty, struct file *filp)
 {
 	struct moxa_port *ch = tty->driver_data;
 	ch->cflag = tty->termios->c_cflag;
-	mutex_lock(&moxa_openlock);
 	tty_port_close(&ch->port, tty, filp);
-	mutex_unlock(&moxa_openlock);
 }
 
 static int moxa_write(struct tty_struct *tty,
@@ -1226,7 +1225,7 @@ static int moxa_write(struct tty_struct *tty,
 	len = MoxaPortWriteData(tty, buf, count);
 	spin_unlock_bh(&moxa_lock);
 
-	ch->statusflags |= LOWWAIT;
+	set_bit(LOWWAIT, &ch->statusflags);
 	return len;
 }
 
@@ -1264,7 +1263,7 @@ static int moxa_chars_in_buffer(struct tty_struct *tty)
 		 * Make it possible to wakeup anything waiting for output
 		 * in tty_ioctl.c, etc.
 		 */
-		if (!(ch->statusflags & EMPTYWAIT))
+		if (!test_bit(EMPTYWAIT, &ch->statusflags))
 			moxa_setup_empty_event(tty);
 	}
 	unlock_kernel();
@@ -1332,14 +1331,14 @@ static void moxa_throttle(struct tty_struct *tty)
 {
 	struct moxa_port *ch = tty->driver_data;
 
-	ch->statusflags |= THROTTLE;
+	set_bit(THROTTLE, &ch->statusflags);
 }
 
 static void moxa_unthrottle(struct tty_struct *tty)
 {
 	struct moxa_port *ch = tty->driver_data;
 
-	ch->statusflags &= ~THROTTLE;
+	clear_bit(THROTTLE, &ch->statusflags);
 }
 
 static void moxa_set_termios(struct tty_struct *tty,
@@ -1361,7 +1360,7 @@ static void moxa_stop(struct tty_struct *tty)
 	if (ch == NULL)
 		return;
 	MoxaPortTxDisable(ch);
-	ch->statusflags |= TXSTOPPED;
+	set_bit(TXSTOPPED, &ch->statusflags);
 }
 
 
@@ -1376,17 +1375,13 @@ static void moxa_start(struct tty_struct *tty)
 		return;
 
 	MoxaPortTxEnable(ch);
-	ch->statusflags &= ~TXSTOPPED;
+	clear_bit(TXSTOPPED, &ch->statusflags);
 }
 
 static void moxa_hangup(struct tty_struct *tty)
 {
-	struct moxa_port *ch;
-
-	mutex_lock(&moxa_openlock);
-	ch = tty->driver_data;
+	struct moxa_port *ch = tty->driver_data;
 	tty_port_hangup(&ch->port);
-	mutex_unlock(&moxa_openlock);
 }
 
 static void moxa_new_dcdstate(struct moxa_port *p, u8 dcd)
@@ -1412,24 +1407,24 @@ static int moxa_poll_port(struct moxa_port *p, unsigned int handle,
 	u16 intr;
 
 	if (tty) {
-		if ((p->statusflags & EMPTYWAIT) &&
+		if (test_bit(EMPTYWAIT, &p->statusflags) &&
 				MoxaPortTxQueue(p) == 0) {
-			p->statusflags &= ~EMPTYWAIT;
+			clear_bit(EMPTYWAIT, &p->statusflags);
 			tty_wakeup(tty);
 		}
-		if ((p->statusflags & LOWWAIT) && !tty->stopped &&
+		if (test_bit(LOWWAIT, &p->statusflags) && !tty->stopped &&
 				MoxaPortTxQueue(p) <= WAKEUP_CHARS) {
-			p->statusflags &= ~LOWWAIT;
+			clear_bit(LOWWAIT, &p->statusflags);
 			tty_wakeup(tty);
 		}
 
-		if (inited && !(p->statusflags & THROTTLE) &&
+		if (inited && !test_bit(THROTTLE, &p->statusflags) &&
 				MoxaPortRxQueue(p) > 0) { /* RX */
 			MoxaPortReadData(p);
 			tty_schedule_flip(tty);
 		}
 	} else {
-		p->statusflags &= ~EMPTYWAIT;
+		clear_bit(EMPTYWAIT, &p->statusflags);
 		MoxaPortFlushData(p, 0); /* flush RX */
 	}
 
@@ -1533,10 +1528,7 @@ static void moxa_set_tty_param(struct tty_struct *tty, struct ktermios *old_term
 static void moxa_setup_empty_event(struct tty_struct *tty)
 {
 	struct moxa_port *ch = tty->driver_data;
-
-	spin_lock_bh(&moxa_lock);
-	ch->statusflags |= EMPTYWAIT;
-	spin_unlock_bh(&moxa_lock);
+	set_bit(EMPTYWAIT, &ch->statusflags);
 }
 
 /*****************************************************************************
@@ -1845,7 +1837,7 @@ static int MoxaPortSetTermio(struct moxa_port *port, struct ktermios *termio,
 		writeb(termio->c_cc[VSTOP], ofsAddr + FuncArg1);
 		writeb(FC_SetXonXoff, ofsAddr + FuncCode);
 		moxa_wait_finish(ofsAddr);
-		spin_unlock_irqrestore(&moxafunc_lock);
+		spin_unlock_irq(&moxafunc_lock);
 
 	}
 	return baud;


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 09/12] moxa: Kill off the throttle method
  2009-11-18 14:14 [PATCH 00/12] Series short description Alan Cox
                   ` (7 preceding siblings ...)
  2009-11-18 14:16 ` [PATCH 08/12] Locking clean up Alan Cox
@ 2009-11-18 14:16 ` Alan Cox
  2009-11-18 14:16 ` [PATCH 10/12] moxa: Fix modem op locking Alan Cox
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Alan Cox @ 2009-11-18 14:16 UTC (permalink / raw)
  To: greg, linux-kernel

The tty flag can be tested so the shadow flag isn't needed

Signed-off-by: Alan Cox <alan@linux.intel.com>
---

 drivers/char/moxa.c |   21 +--------------------
 1 files changed, 1 insertions(+), 20 deletions(-)


diff --git a/drivers/char/moxa.c b/drivers/char/moxa.c
index e6a79b0..b640409 100644
--- a/drivers/char/moxa.c
+++ b/drivers/char/moxa.c
@@ -154,7 +154,6 @@ struct mon_str {
 #define TXSTOPPED	1
 #define LOWWAIT 	2
 #define EMPTYWAIT	3
-#define THROTTLE	4
 
 #define SERIAL_DO_RESTART
 
@@ -194,8 +193,6 @@ static int moxa_write(struct tty_struct *, const unsigned char *, int);
 static int moxa_write_room(struct tty_struct *);
 static void moxa_flush_buffer(struct tty_struct *);
 static int moxa_chars_in_buffer(struct tty_struct *);
-static void moxa_throttle(struct tty_struct *);
-static void moxa_unthrottle(struct tty_struct *);
 static void moxa_set_termios(struct tty_struct *, struct ktermios *);
 static void moxa_stop(struct tty_struct *);
 static void moxa_start(struct tty_struct *);
@@ -415,8 +412,6 @@ static const struct tty_operations moxa_ops = {
 	.flush_buffer = moxa_flush_buffer,
 	.chars_in_buffer = moxa_chars_in_buffer,
 	.ioctl = moxa_ioctl,
-	.throttle = moxa_throttle,
-	.unthrottle = moxa_unthrottle,
 	.set_termios = moxa_set_termios,
 	.stop = moxa_stop,
 	.start = moxa_start,
@@ -1327,20 +1322,6 @@ static int moxa_tiocmset(struct tty_struct *tty, struct file *file,
 	return 0;
 }
 
-static void moxa_throttle(struct tty_struct *tty)
-{
-	struct moxa_port *ch = tty->driver_data;
-
-	set_bit(THROTTLE, &ch->statusflags);
-}
-
-static void moxa_unthrottle(struct tty_struct *tty)
-{
-	struct moxa_port *ch = tty->driver_data;
-
-	clear_bit(THROTTLE, &ch->statusflags);
-}
-
 static void moxa_set_termios(struct tty_struct *tty,
 		struct ktermios *old_termios)
 {
@@ -1418,7 +1399,7 @@ static int moxa_poll_port(struct moxa_port *p, unsigned int handle,
 			tty_wakeup(tty);
 		}
 
-		if (inited && !test_bit(THROTTLE, &p->statusflags) &&
+		if (inited && !test_bit(TTY_THROTTLED, &tty->flags) &&
 				MoxaPortRxQueue(p) > 0) { /* RX */
 			MoxaPortReadData(p);
 			tty_schedule_flip(tty);


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 10/12] moxa: Fix modem op locking
  2009-11-18 14:14 [PATCH 00/12] Series short description Alan Cox
                   ` (8 preceding siblings ...)
  2009-11-18 14:16 ` [PATCH 09/12] moxa: Kill off the throttle method Alan Cox
@ 2009-11-18 14:16 ` Alan Cox
  2009-11-18 14:16 ` [PATCH 11/12] moxa: Kill the use of lock_kernel Alan Cox
  2009-11-18 14:17 ` [PATCH 12/12] moxa: split open lock Alan Cox
  11 siblings, 0 replies; 14+ messages in thread
From: Alan Cox @ 2009-11-18 14:16 UTC (permalink / raw)
  To: greg, linux-kernel

This is overkill and mostly not needed

Signed-off-by: Alan Cox <alan@linux.intel.com>
---

 drivers/char/moxa.c |   25 ++++++++++---------------
 1 files changed, 10 insertions(+), 15 deletions(-)


diff --git a/drivers/char/moxa.c b/drivers/char/moxa.c
index b640409..f9816e6 100644
--- a/drivers/char/moxa.c
+++ b/drivers/char/moxa.c
@@ -139,7 +139,7 @@ struct moxa_port {
 	int cflag;
 	unsigned long statusflags;
 
-	u8 DCDState;
+	u8 DCDState;		/* Protected by the port lock */
 	u8 lineCtrl;
 	u8 lowChkFlag;
 };
@@ -1141,9 +1141,9 @@ static int moxa_carrier_raised(struct tty_port *port)
 	struct moxa_port *ch = container_of(port, struct moxa_port, port);
 	int dcd;
 
-	spin_lock_bh(&moxa_lock);
+	spin_lock_irq(&port->lock);
 	dcd = ch->DCDState;
-	spin_unlock_bh(&moxa_lock);
+	spin_unlock_irq(&port->lock);
 	return dcd;
 }
 
@@ -1267,16 +1267,9 @@ static int moxa_chars_in_buffer(struct tty_struct *tty)
 
 static int moxa_tiocmget(struct tty_struct *tty, struct file *file)
 {
-	struct moxa_port *ch;
+	struct moxa_port *ch = tty->driver_data;
 	int flag = 0, dtr, rts;
 
-	mutex_lock(&moxa_openlock);
-	ch = tty->driver_data;
-	if (!ch) {
-		mutex_unlock(&moxa_openlock);
-		return -EINVAL;
-	}
-
 	MoxaPortGetLineOut(ch, &dtr, &rts);
 	if (dtr)
 		flag |= TIOCM_DTR;
@@ -1289,7 +1282,6 @@ static int moxa_tiocmget(struct tty_struct *tty, struct file *file)
 		flag |= TIOCM_DSR;
 	if (dtr & 4)
 		flag |= TIOCM_CD;
-	mutex_unlock(&moxa_openlock);
 	return flag;
 }
 
@@ -1368,15 +1360,20 @@ static void moxa_hangup(struct tty_struct *tty)
 static void moxa_new_dcdstate(struct moxa_port *p, u8 dcd)
 {
 	struct tty_struct *tty;
+	unsigned long flags;
 	dcd = !!dcd;
 
+	spin_lock_irqsave(&p->port.lock, flags);
 	if (dcd != p->DCDState) {
+        	p->DCDState = dcd;
+        	spin_unlock_irqrestore(&p->port.lock, flags);
 		tty = tty_port_tty_get(&p->port);
 		if (tty && C_CLOCAL(tty) && !dcd)
 			tty_hangup(tty);
 		tty_kref_put(tty);
 	}
-	p->DCDState = dcd;
+	else
+		spin_unlock_irqrestore(&p->port.lock, flags);
 }
 
 static int moxa_poll_port(struct moxa_port *p, unsigned int handle,
@@ -1878,9 +1875,7 @@ static int MoxaPortLineStatus(struct moxa_port *port)
 	val &= 0x0B;
 	if (val & 8)
 		val |= 4;
-	spin_lock_bh(&moxa_lock);
 	moxa_new_dcdstate(port, val & 8);
-	spin_unlock_bh(&moxa_lock);
 	val &= 7;
 	return val;
 }


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 11/12] moxa: Kill the use of lock_kernel
  2009-11-18 14:14 [PATCH 00/12] Series short description Alan Cox
                   ` (9 preceding siblings ...)
  2009-11-18 14:16 ` [PATCH 10/12] moxa: Fix modem op locking Alan Cox
@ 2009-11-18 14:16 ` Alan Cox
  2009-11-18 14:17 ` [PATCH 12/12] moxa: split open lock Alan Cox
  11 siblings, 0 replies; 14+ messages in thread
From: Alan Cox @ 2009-11-18 14:16 UTC (permalink / raw)
  To: greg, linux-kernel

It isn't needed here any more

Signed-off-by: Alan Cox <alan@linux.intel.com>
---

 drivers/char/moxa.c |   16 ++--------------
 1 files changed, 2 insertions(+), 14 deletions(-)


diff --git a/drivers/char/moxa.c b/drivers/char/moxa.c
index f9816e6..f8e673a 100644
--- a/drivers/char/moxa.c
+++ b/drivers/char/moxa.c
@@ -34,7 +34,6 @@
 #include <linux/tty.h>
 #include <linux/tty_flip.h>
 #include <linux/major.h>
-#include <linux/smp_lock.h>
 #include <linux/string.h>
 #include <linux/fcntl.h>
 #include <linux/ptrace.h>
@@ -202,7 +201,6 @@ static int moxa_tiocmset(struct tty_struct *tty, struct file *file,
 			 unsigned int set, unsigned int clear);
 static void moxa_poll(unsigned long);
 static void moxa_set_tty_param(struct tty_struct *, struct ktermios *);
-static void moxa_setup_empty_event(struct tty_struct *);
 static void moxa_shutdown(struct tty_port *);
 static int moxa_carrier_raised(struct tty_port *);
 static void moxa_dtr_rts(struct tty_port *, int);
@@ -1251,17 +1249,13 @@ static int moxa_chars_in_buffer(struct tty_struct *tty)
 	struct moxa_port *ch = tty->driver_data;
 	int chars;
 
-	lock_kernel();
 	chars = MoxaPortTxQueue(ch);
-	if (chars) {
+	if (chars)
 		/*
 		 * Make it possible to wakeup anything waiting for output
 		 * in tty_ioctl.c, etc.
 		 */
-		if (!test_bit(EMPTYWAIT, &ch->statusflags))
-			moxa_setup_empty_event(tty);
-	}
-	unlock_kernel();
+        	set_bit(EMPTYWAIT, &ch->statusflags);
 	return chars;
 }
 
@@ -1503,12 +1497,6 @@ static void moxa_set_tty_param(struct tty_struct *tty, struct ktermios *old_term
 	tty_encode_baud_rate(tty, baud, baud);
 }
 
-static void moxa_setup_empty_event(struct tty_struct *tty)
-{
-	struct moxa_port *ch = tty->driver_data;
-	set_bit(EMPTYWAIT, &ch->statusflags);
-}
-
 /*****************************************************************************
  *	Driver level functions: 					     *
  *****************************************************************************/


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 12/12] moxa: split open lock
  2009-11-18 14:14 [PATCH 00/12] Series short description Alan Cox
                   ` (10 preceding siblings ...)
  2009-11-18 14:16 ` [PATCH 11/12] moxa: Kill the use of lock_kernel Alan Cox
@ 2009-11-18 14:17 ` Alan Cox
  11 siblings, 0 replies; 14+ messages in thread
From: Alan Cox @ 2009-11-18 14:17 UTC (permalink / raw)
  To: greg, linux-kernel

moxa_openlock is used for several situations where we want to handle the
case of an ioctl that crosses many ports (not just the open tty), and also
cases where an open races a deinit (eg a pci unplug) and we hangup a port
before we can cope with that.

The non open race cases can use the moxa_lock spinlock. This simplifies sorting
out the remaining mess.

Signed-off-by: Alan Cox <alan@linux.intel.com>
---

 drivers/char/moxa.c |   23 +++++++++++------------
 1 files changed, 11 insertions(+), 12 deletions(-)


diff --git a/drivers/char/moxa.c b/drivers/char/moxa.c
index f8e673a..16b8aed 100644
--- a/drivers/char/moxa.c
+++ b/drivers/char/moxa.c
@@ -163,6 +163,7 @@ static struct mon_str moxaLog;
 static unsigned int moxaFuncTout = HZ / 2;
 static unsigned int moxaLowWaterChk;
 static DEFINE_MUTEX(moxa_openlock);
+static DEFINE_SPINLOCK(moxa_lock);
 /* Variables for insmod */
 #ifdef MODULE
 static unsigned long baseaddr[MAX_BOARDS];
@@ -313,22 +314,20 @@ static int moxa_ioctl(struct tty_struct *tty, struct file *file,
 		struct moxa_port *p;
 		unsigned int i, j;
 
-		mutex_lock(&moxa_openlock);
 		for (i = 0; i < MAX_BOARDS; i++) {
 			p = moxa_boards[i].ports;
 			for (j = 0; j < MAX_PORTS_PER_BOARD; j++, p++, argm++) {
 				memset(&tmp, 0, sizeof(tmp));
+				spin_lock_bh(&moxa_lock);
 				if (moxa_boards[i].ready) {
 					tmp.inq = MoxaPortRxQueue(p);
 					tmp.outq = MoxaPortTxQueue(p);
 				}
-				if (copy_to_user(argm, &tmp, sizeof(tmp))) {
-					mutex_unlock(&moxa_openlock);
+				spin_unlock_bh(&moxa_lock);
+				if (copy_to_user(argm, &tmp, sizeof(tmp)))
 					return -EFAULT;
-				}
 			}
 		}
-		mutex_unlock(&moxa_openlock);
 		break;
 	} case MOXA_GET_OQUEUE:
 		status = MoxaPortTxQueue(ch);
@@ -344,16 +343,20 @@ static int moxa_ioctl(struct tty_struct *tty, struct file *file,
 		struct moxa_port *p;
 		unsigned int i, j;
 
-		mutex_lock(&moxa_openlock);
 		for (i = 0; i < MAX_BOARDS; i++) {
 			p = moxa_boards[i].ports;
 			for (j = 0; j < MAX_PORTS_PER_BOARD; j++, p++, argm++) {
 				struct tty_struct *ttyp;
 				memset(&tmp, 0, sizeof(tmp));
-				if (!moxa_boards[i].ready)
+				spin_lock_bh(&moxa_lock);
+				if (!moxa_boards[i].ready) {
+				        spin_unlock_bh(&moxa_lock);
 					goto copy;
+                                }
 
 				status = MoxaPortLineStatus(p);
+				spin_unlock_bh(&moxa_lock);
+
 				if (status & 1)
 					tmp.cts = 1;
 				if (status & 2)
@@ -368,13 +371,10 @@ static int moxa_ioctl(struct tty_struct *tty, struct file *file,
 					tmp.cflag = ttyp->termios->c_cflag;
 				tty_kref_put(tty);
 copy:
-				if (copy_to_user(argm, &tmp, sizeof(tmp))) {
-					mutex_unlock(&moxa_openlock);
+				if (copy_to_user(argm, &tmp, sizeof(tmp)))
 					return -EFAULT;
-				}
 			}
 		}
-		mutex_unlock(&moxa_openlock);
 		break;
 	}
 	case TIOCGSERIAL:
@@ -427,7 +427,6 @@ static const struct tty_port_operations moxa_port_ops = {
 
 static struct tty_driver *moxaDriver;
 static DEFINE_TIMER(moxaTimer, moxa_poll, 0, 0);
-static DEFINE_SPINLOCK(moxa_lock);
 
 /*
  * HW init


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 04/12] mxser: Use the new locking rules to fix setserial properly
  2009-11-18 14:15 ` [PATCH 04/12] mxser: Use the new locking rules to fix setserial properly Alan Cox
@ 2009-11-20 12:08   ` Dan Carpenter
  0 siblings, 0 replies; 14+ messages in thread
From: Dan Carpenter @ 2009-11-20 12:08 UTC (permalink / raw)
  To: Alan Cox; +Cc: greg, linux-kernel

On Wed, Nov 18, 2009 at 02:15:23PM +0000, Alan Cox wrote:
> Propogate the init/shutdown mutex through the setserial logic. Use the proper
> locks for the various bits still using the BKL. Kill the BKL in this driver.
> 

>  drivers/char/mxser.c |  145 +++++++++++++++++++++++++++-----------------------

> @@ -1951,6 +1959,7 @@ static void mxser_wait_until_sent(struct tty_struct *tty, int timeout)

> +	spin_lock_irqsave(&info->slock, flags);
>  	while (!((lsr = inb(info->ioaddr + UART_LSR)) & UART_LSR_TEMT)) {
>  #ifdef SERIAL_DEBUG_RS_WAIT_UNTIL_SENT
>  		printk("lsr = %d (jiff=%lu)...", lsr, jiffies);
>  #endif
> +		spin_unlock_irqrestore(&info->slock, flags);
>  		schedule_timeout_interruptible(char_time);
>  		if (signal_pending(current))
>  			break;
>  		if (timeout && time_after(jiffies, orig_jiffies + timeout))
>  			break;

If we break out of this while loop we get a double unlock.

> +		spin_lock_irqsave(&info->slock, flags);
>  	}
> +	spin_unlock_irqrestore(&info->slock, flags);
>  	set_current_state(TASK_RUNNING);
> -	unlock_kernel();

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2009-11-20 12:17 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-11-18 14:14 [PATCH 00/12] Series short description Alan Cox
2009-11-18 14:14 ` [PATCH 01/12] isicom: switch to the new tty_port_open helper Alan Cox
2009-11-18 14:14 ` [PATCH 02/12] isicom: sort out the board init logic Alan Cox
2009-11-18 14:15 ` [PATCH 03/12] mxser: use the tty_port_open method Alan Cox
2009-11-18 14:15 ` [PATCH 04/12] mxser: Use the new locking rules to fix setserial properly Alan Cox
2009-11-20 12:08   ` Dan Carpenter
2009-11-18 14:15 ` [PATCH 05/12] isicom: fix deadlock on shutdown Alan Cox
2009-11-18 14:15 ` [PATCH 06/12] moxa: Use more tty_port ops Alan Cox
2009-11-18 14:15 ` [PATCH 07/12] moxa: rework the locking a bit Alan Cox
2009-11-18 14:16 ` [PATCH 08/12] Locking clean up Alan Cox
2009-11-18 14:16 ` [PATCH 09/12] moxa: Kill off the throttle method Alan Cox
2009-11-18 14:16 ` [PATCH 10/12] moxa: Fix modem op locking Alan Cox
2009-11-18 14:16 ` [PATCH 11/12] moxa: Kill the use of lock_kernel Alan Cox
2009-11-18 14:17 ` [PATCH 12/12] moxa: split open lock Alan Cox

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).