
From: Jean Tourrilhes <jt@bougret.hpl.hp.com>

The irtty-sir driver, which is a TTY line discipline and a network driver,
need to be able to change the RTS and DTR line from a kernel thread.

It used to do that by calling the tty driver's user ioctl handler directly,
with a NULL file*.  But the uart ioctl handler is now checking the file* and
it dereferences null.

Rather than hacking the ioctl to know about NULL file pointers, the preferred
fix is to add a tty API to allow line disciplines to read/set the modem
control lines to the tty later, rather than having line disciplines play
games with IOCTLs.

That is what Jean's patch does.  It uses the internal set/clear API, because
this allows us to ignore the default state of control line, bypassing the
troubles with TIOCM_OUT2/TIOCM_OUT1 on various platforms.




 25-akpm/drivers/net/irda/irtty-sir.c   |   31 ++++++++++++++-----------------
 25-akpm/drivers/net/irda/sir_kthread.c |    7 +++++++
 25-akpm/drivers/serial/core.c          |   21 +++++++++++++++++++++
 25-akpm/include/linux/tty_driver.h     |    2 ++
 4 files changed, 44 insertions(+), 17 deletions(-)

diff -puN drivers/net/irda/irtty-sir.c~tty-modem-control-api drivers/net/irda/irtty-sir.c
--- 25/drivers/net/irda/irtty-sir.c~tty-modem-control-api	Thu Apr 10 15:09:16 2003
+++ 25-akpm/drivers/net/irda/irtty-sir.c	Thu Apr 10 15:09:16 2003
@@ -180,32 +180,29 @@ static int irtty_change_speed(struct sir
 static int irtty_set_dtr_rts(struct sir_dev *dev, int dtr, int rts)
 {
 	struct sirtty_cb *priv = dev->priv;
-	int arg = 0;
+	int set = 0;
+	int clear = 0;
 
 	ASSERT(priv != NULL, return -1;);
 	ASSERT(priv->magic == IRTTY_MAGIC, return -1;);
 
-#ifdef TIOCM_OUT2 /* Not defined for ARM */
-	arg = TIOCM_OUT2;
-#endif
 	if (rts)
-		arg |= TIOCM_RTS;
+		set |= TIOCM_RTS;
+	else
+		clear |= TIOCM_RTS;
 	if (dtr)
-		arg |= TIOCM_DTR;
+		set |= TIOCM_DTR;
+	else
+		clear |= TIOCM_DTR;
 
 	/*
-	 * The ioctl() function, or actually set_modem_info() in serial.c
-	 * expects a pointer to the argument in user space. This is working
-	 * here because we are always called from the kIrDAd thread which
-	 * has set_fs(KERNEL_DS) permanently set. Therefore copy_from_user()
-	 * is happy with our arg-parameter being local here in kernel space.
+	 * We can't use ioctl() because it expects a non-null file structure,
+	 * and we don't have that here.
+	 * This function is not yet defined for all tty driver, so
+	 * let's be careful... Jean II
 	 */
-
-	lock_kernel();
-	if (priv->tty->driver.ioctl(priv->tty, NULL, TIOCMSET, (unsigned long) &arg)) { 
-		IRDA_DEBUG(2, "%s(), error doing ioctl!\n", __FUNCTION__);
-	}
-	unlock_kernel();
+	ASSERT(priv->tty->driver.modem_ctrl != NULL, return -1;);
+	priv->tty->driver.modem_ctrl(priv->tty, set, clear);
 
 	return 0;
 }
diff -puN drivers/net/irda/sir_kthread.c~tty-modem-control-api drivers/net/irda/sir_kthread.c
--- 25/drivers/net/irda/sir_kthread.c~tty-modem-control-api	Thu Apr 10 15:09:16 2003
+++ 25-akpm/drivers/net/irda/sir_kthread.c	Thu Apr 10 15:09:16 2003
@@ -151,6 +151,13 @@ static int irda_thread(void *startup)
 
 	while (irda_rq_queue.thread != NULL) {
 
+		/* We use TASK_INTERRUPTIBLE, rather than
+		 * TASK_UNINTERRUPTIBLE.  Andrew Morton made this
+		 * change ; he told me that it is safe, because "signal
+		 * blocking is now handled in daemonize()", he added
+		 * that the problem is that "uninterruptible sleep
+		 * contributes to load average", making user worry.
+		 * Jean II */
 		set_task_state(current, TASK_INTERRUPTIBLE);
 		add_wait_queue(&irda_rq_queue.kick, &wait);
 		if (list_empty(&irda_rq_queue.request_list))
diff -puN drivers/serial/core.c~tty-modem-control-api drivers/serial/core.c
--- 25/drivers/serial/core.c~tty-modem-control-api	Thu Apr 10 15:09:16 2003
+++ 25-akpm/drivers/serial/core.c	Thu Apr 10 15:09:16 2003
@@ -1158,6 +1158,26 @@ uart_ioctl(struct tty_struct *tty, struc
 	return ret;
 }
 
+/*
+ * Called by line disciplines to change the various modem control bits...
+ * Line disciplines are implemented within the kernel, and therefore
+ * we don't want them to use the ioctl function above.
+ * Jean II
+ */
+static int
+uart_modem_ctrl(struct tty_struct *tty, unsigned int set, unsigned int clear)
+{
+	struct uart_state *state = tty->driver_data;
+
+	/* Set new values, if any */
+	/* Locking will be done in there */
+	uart_update_mctrl(state->port, set, clear);
+
+	/* Return new value */
+	return state->port->ops->get_mctrl(state->port);
+}
+
+
 static void uart_set_termios(struct tty_struct *tty, struct termios *old_termios)
 {
 	struct uart_state *state = tty->driver_data;
@@ -2150,6 +2170,7 @@ int uart_register_driver(struct uart_dri
 	normal->chars_in_buffer	= uart_chars_in_buffer;
 	normal->flush_buffer	= uart_flush_buffer;
 	normal->ioctl		= uart_ioctl;
+	normal->modem_ctrl	= uart_modem_ctrl;
 	normal->throttle	= uart_throttle;
 	normal->unthrottle	= uart_unthrottle;
 	normal->send_xchar	= uart_send_xchar;
diff -puN include/linux/tty_driver.h~tty-modem-control-api include/linux/tty_driver.h
--- 25/include/linux/tty_driver.h~tty-modem-control-api	Thu Apr 10 15:09:16 2003
+++ 25-akpm/include/linux/tty_driver.h	Thu Apr 10 15:09:16 2003
@@ -157,6 +157,8 @@ struct tty_driver {
 	int  (*chars_in_buffer)(struct tty_struct *tty);
 	int  (*ioctl)(struct tty_struct *tty, struct file * file,
 		    unsigned int cmd, unsigned long arg);
+	int  (*modem_ctrl)(struct tty_struct *tty,
+			   unsigned int set, unsigned int clear);
 	void (*set_termios)(struct tty_struct *tty, struct termios * old);
 	void (*throttle)(struct tty_struct * tty);
 	void (*unthrottle)(struct tty_struct * tty);

_
