[PATCH] ptmx: adding handshake support

[PATCH] ptmx: adding handshake support

[sander van ginkel]

For serial over ethernet communication, /dev/ptmx will be used to  
create a virtual device.
Due to the lack of a modem status register and a modem control  
register in the ptmx driver,
it won't be possible to transfer handshake signaling.

I wrote this patch to add these registers to the ptmx driver.
The modem status register, which controls CD,CTS,DSR,RI is normally  
read-only. To be able to
pass these signals from the remote port to the local ptmx device, I've  
added an IOCTL routine.
The result of a remote TIOCMGET IOCTL call , can be passed directly  
through this IOCTL routine.
The IOCTL value I used for this is, 0x5426 which was formerly known as  
TIOCTTYGSTRUCT.

I've tested this patch at an 2.6.16.27 kernel.

Suggestions and/or other test results are welcome.

Sander


Signed-off-by: Sander van Ginkel <sander@van-ginkel.eu>

===================================================================
--- drivers/char/pty.c	2008-03-03 20:02:09.000000000 +0100
+++ drivers/char/pty-new.c	2008-03-02 21:55:53.000000000 +0100
@@ -7,6 +7,8 @@
   *    -- C. Scott Ananian <cananian@alumni.princeton.edu>, 14-Jan-1998
   *  Added TTY_DO_WRITE_WAKEUP to enable n_tty to send POLL_OUT to
   *      waiting writers -- Sapan Bhatia <sapan@corewars.org>
+ *  Added support for MCR/MSR, used for serial over ethernet
+ *    -- Sander van Ginkel <sander@van-ginkel.eu>
   *
   *
   */
@@ -32,6 +34,16 @@
  #include <linux/bitops.h>
  #include <linux/devpts_fs.h>

+struct pty_mcrmsr {
+
+	struct semaphore        sem;            /* locks this structure */
+
+	/* for tiocmget and tiocmset functions */
+
+	int                     msr;            /* MSR shadow */
+	int                     mcr;            /* MCR shadow */
+};
+
  /* These are global because they are accessed in tty_io.c */
  #ifdef CONFIG_UNIX98_PTYS
  struct tty_driver *ptm_driver;
@@ -199,6 +211,7 @@

  static int pty_open(struct tty_struct *tty, struct file * filp)
  {
+        struct pty_mcrmsr *mcrmsr;
  	int	retval = -ENODEV;

  	if (!tty || !tty->link)
@@ -216,10 +229,90 @@
  	set_bit(TTY_THROTTLED, &tty->flags);
  	set_bit(TTY_DO_WRITE_WAKEUP, &tty->flags);
  	retval = 0;
+
+	/* initialize the pointer in case something fails */
+
+	tty->driver_data = NULL;
+	tty->link->driver_data = NULL;
+
+	/* first time accessing this device, let's create it */
+
+	mcrmsr = kmalloc(sizeof(*mcrmsr), GFP_KERNEL);
+
+	init_MUTEX(&mcrmsr->sem);
+
+	if (mcrmsr != NULL) {
+		down(&mcrmsr->sem);
+
+		/* save our structure within the tty structure */
+
+		mcrmsr->mcr=0;
+		mcrmsr->msr=0;
+
+		tty->driver_data = mcrmsr;
+		tty->link->driver_data = mcrmsr;
+
+		up(&mcrmsr->sem);
+
+	}
+
  out:
  	return retval;
  }

+static int pty_tiocmget(struct tty_struct *tty, struct file *file)
+{
+	struct pty_mcrmsr *mcrmsr = tty->driver_data;
+	unsigned int result = 0;
+	unsigned int msr=0;
+	unsigned int mcr=0;
+
+	if (tty->driver_data != NULL) {
+		msr = mcrmsr->msr;
+		mcr = mcrmsr->mcr;
+	}
+
+	result = ((mcr & 0x01)  ? TIOCM_DTR  : 0) |  /* DTR is set */
+		((mcr & 0x02)  ? TIOCM_RTS  : 0) |  /* RTS is set */
+		((mcr & 0x04)  ? TIOCM_LOOP : 0) |  /* LOOP is set */
+		((msr & 0x08)  ? TIOCM_CTS  : 0) |  /* CTS is set */
+		((msr & 0x10)  ? TIOCM_CAR  : 0) |  /* Carrier detect is set*/
+		((msr & 0x20)  ? TIOCM_RI   : 0) |  /* Ring Indicator is set */
+		((msr & 0x40)  ? TIOCM_DSR  : 0);   /* DSR is set */
+
+	return result;
+}
+
+static int pty_tiocmset(struct tty_struct *tty, struct file  
*file,unsigned int set, unsigned int clear)
+{
+	struct pty_mcrmsr *mcrmsr = tty->driver_data;
+	unsigned int mcr=0;
+
+	if (tty->driver_data != NULL)
+		mcr = mcrmsr->mcr;
+
+	if (set & TIOCM_DTR)
+		mcr |= 0x01;
+	if (set & TIOCM_RTS)
+		mcr |= 0x02;
+	if (set & TIOCM_LOOP)
+		mcr |= 0x04;
+
+	if (clear & TIOCM_DTR)
+		mcr &= ~0x01;
+	if (clear & TIOCM_RTS)
+		mcr &= ~0x02;
+	if (clear & TIOCM_LOOP)
+		mcr &= ~0x04;
+
+	/* set the new MCR value in the device */
+
+	if (tty->driver_data != NULL)
+		mcrmsr->mcr=mcr;
+
+	return 0;
+}
+
  static void pty_set_termios(struct tty_struct *tty, struct termios  
*old_termios)
  {
          tty->termios->c_cflag &= ~(CSIZE | PARENB);
@@ -235,6 +328,8 @@
  	.chars_in_buffer = pty_chars_in_buffer,
  	.unthrottle = pty_unthrottle,
  	.set_termios = pty_set_termios,
+	.tiocmget = pty_tiocmget,
+	.tiocmset = pty_tiocmset,
  };

  /* Traditional BSD devices */
@@ -339,11 +434,44 @@
  static int pty_unix98_ioctl(struct tty_struct *tty, struct file *file,
  			    unsigned int cmd, unsigned long arg)
  {
+	unsigned int value;
+	struct pty_mcrmsr *mcrmsr = tty->driver_data;
+	unsigned int msr=0;
+	unsigned int mcr=0;
+
  	switch (cmd) {
  	case TIOCSPTLCK: /* Set PT Lock (disallow slave open) */
  		return pty_set_lock(tty, (int __user *)arg);
  	case TIOCGPTN: /* Get PT Number */
  		return put_user(tty->index, (unsigned int __user *)arg);
+
+	case 0x5426: /* Set all of the handshake line, even the normally  
read only */
+		{
+		if (copy_from_user(&value,(unsigned int *)arg,sizeof(unsigned int)))
+			return -EFAULT;
+
+		if (value & 0x0002) /* DTR set */
+			mcr|=0x01;
+		if (value & 0x0004) /* RTS set */
+			mcr|=0x02;
+		if (value & 0x8000) /* LOOP set */
+			mcr|=0x04;
+		if (value & 0x0020) /* CTS set */
+			msr|=0x08;
+		if (value & 0x0040) /* DCD set */
+			msr|=0x10;
+		if (value & 0x0080) /* RI set */
+			msr|=0x20;
+		if (value & 0x0100) /* DSR set */
+			msr|=0x40;
+
+		if (tty->driver_data != NULL)
+			{
+			mcrmsr->msr=msr;
+			mcrmsr->mcr=mcr;
+			}
+		return 0;
+		}
  	}

  	return -ENOIOCTLCMD;

Re: [PATCH] ptmx: adding handshake support

[postmaster]

This weekend I upgraded my kernel to 2.6.22.5 and tried my patch.
Unfortunately pty.c seems to be a slightly different, so generated a new
patch:

  Signed-off-by: Sander van Ginkel <sander@van-ginkel.eu>

===================================================================
--- linux-2.6.22.5.orig/drivers/char/pty.c	2007-08-23 01:23:54.000000000 +0200
+++ linux-2.6.22.5.new/drivers/char/pty.c	2008-03-23 15:25:24.000000000 +0100
@@ -7,6 +7,8 @@
   *    -- C. Scott Ananian <cananian@alumni.princeton.edu>, 14-Jan-1998
   *  Added TTY_DO_WRITE_WAKEUP to enable n_tty to send POLL_OUT to
   *      waiting writers -- Sapan Bhatia <sapan@corewars.org>
+ *  Added support for MCR/MSR, used for serial over ethernet
+ *    -- Sander van Ginkel <sander@van-ginkel.eu>
   *
   *
   */
@@ -29,6 +31,16 @@
  #include <linux/bitops.h>
  #include <linux/devpts_fs.h>

+struct pty_mcrmsr {
+
+	struct semaphore        sem;            /* locks this structure */
+
+	/* for tiocmget and tiocmset functions */
+
+	int                     msr;            /* MSR shadow */
+	int                     mcr;            /* MCR shadow */
+};
+
  /* These are global because they are accessed in tty_io.c */
  #ifdef CONFIG_UNIX98_PTYS
  struct tty_driver *ptm_driver;
@@ -196,6 +208,7 @@

  static int pty_open(struct tty_struct *tty, struct file * filp)
  {
+        struct pty_mcrmsr *mcrmsr;
  	int	retval = -ENODEV;

  	if (!tty || !tty->link)
@@ -213,10 +226,90 @@
  	set_bit(TTY_THROTTLED, &tty->flags);
  	set_bit(TTY_DO_WRITE_WAKEUP, &tty->flags);
  	retval = 0;
+
+	/* initialize the pointer in case something fails */
+
+	tty->driver_data = NULL;
+	tty->link->driver_data = NULL;
+
+	/* first time accessing this device, let's create it */
+
+	mcrmsr = kmalloc(sizeof(*mcrmsr), GFP_KERNEL);
+
+	init_MUTEX(&mcrmsr->sem);
+
+	if (mcrmsr != NULL) {
+		down(&mcrmsr->sem);
+
+		/* save our structure within the tty structure */
+
+		mcrmsr->mcr=0;
+		mcrmsr->msr=0;
+
+		tty->driver_data = mcrmsr;
+		tty->link->driver_data = mcrmsr;
+
+		up(&mcrmsr->sem);
+
+	}
+
  out:
  	return retval;
  }

+static int pty_tiocmget(struct tty_struct *tty, struct file *file)
+{
+	struct pty_mcrmsr *mcrmsr = tty->driver_data;
+	unsigned int result = 0;
+	unsigned int msr=0;
+	unsigned int mcr=0;
+
+	if (tty->driver_data != NULL) {
+		msr = mcrmsr->msr;
+		mcr = mcrmsr->mcr;
+	}
+
+	result = ((mcr & 0x01)  ? TIOCM_DTR  : 0) |  /* DTR is set */
+		((mcr & 0x02)  ? TIOCM_RTS  : 0) |  /* RTS is set */
+		((mcr & 0x04)  ? TIOCM_LOOP : 0) |  /* LOOP is set */
+		((msr & 0x08)  ? TIOCM_CTS  : 0) |  /* CTS is set */
+		((msr & 0x10)  ? TIOCM_CAR  : 0) |  /* Carrier detect is set*/
+		((msr & 0x20)  ? TIOCM_RI   : 0) |  /* Ring Indicator is set */
+		((msr & 0x40)  ? TIOCM_DSR  : 0);   /* DSR is set */
+
+	return result;
+}
+
+static int pty_tiocmset(struct tty_struct *tty, struct file  
*file,unsigned int set, unsigned int clear)
+{
+	struct pty_mcrmsr *mcrmsr = tty->driver_data;
+	unsigned int mcr=0;
+
+	if (tty->driver_data != NULL)
+		mcr = mcrmsr->mcr;
+
+	if (set & TIOCM_DTR)
+		mcr |= 0x01;
+	if (set & TIOCM_RTS)
+		mcr |= 0x02;
+	if (set & TIOCM_LOOP)
+		mcr |= 0x04;
+
+	if (clear & TIOCM_DTR)
+		mcr &= ~0x01;
+	if (clear & TIOCM_RTS)
+		mcr &= ~0x02;
+	if (clear & TIOCM_LOOP)
+		mcr &= ~0x04;
+
+	/* set the new MCR value in the device */
+
+	if (tty->driver_data != NULL)
+		mcrmsr->mcr=mcr;
+
+	return 0;
+}
+
  static void pty_set_termios(struct tty_struct *tty, struct ktermios  
*old_termios)
  {
          tty->termios->c_cflag &= ~(CSIZE | PARENB);
@@ -232,6 +325,8 @@
  	.chars_in_buffer = pty_chars_in_buffer,
  	.unthrottle = pty_unthrottle,
  	.set_termios = pty_set_termios,
+	.tiocmget = pty_tiocmget,
+	.tiocmset = pty_tiocmset,
  };

  /* Traditional BSD devices */
@@ -338,11 +433,44 @@
  static int pty_unix98_ioctl(struct tty_struct *tty, struct file *file,
  			    unsigned int cmd, unsigned long arg)
  {
+	unsigned int value;
+	struct pty_mcrmsr *mcrmsr = tty->driver_data;
+	unsigned int msr=0;
+	unsigned int mcr=0;
+
  	switch (cmd) {
  	case TIOCSPTLCK: /* Set PT Lock (disallow slave open) */
  		return pty_set_lock(tty, (int __user *)arg);
  	case TIOCGPTN: /* Get PT Number */
  		return put_user(tty->index, (unsigned int __user *)arg);
+
+	case 0x5426: /* Set all of the handshake line, even the normally  
read only */
+		{
+		if (copy_from_user(&value,(unsigned int *)arg,sizeof(unsigned int)))
+			return -EFAULT;
+
+		if (value & 0x0002) /* DTR set */
+			mcr|=0x01;
+		if (value & 0x0004) /* RTS set */
+			mcr|=0x02;
+		if (value & 0x8000) /* LOOP set */
+			mcr|=0x04;
+		if (value & 0x0020) /* CTS set */
+			msr|=0x08;
+		if (value & 0x0040) /* DCD set */
+			msr|=0x10;
+		if (value & 0x0080) /* RI set */
+			msr|=0x20;
+		if (value & 0x0100) /* DSR set */
+			msr|=0x40;
+
+		if (tty->driver_data != NULL)
+			{
+			mcrmsr->msr=msr;
+			mcrmsr->mcr=mcr;
+			}
+		return 0;
+		}
  	}

  	return -ENOIOCTLCMD;


Quoting sander van ginkel <sander@van-ginkel.eu>:

> For serial over ethernet communication, /dev/ptmx will be used to
> create a virtual device.
> Due to the lack of a modem status register and a modem control register
> in the ptmx driver,
> it won't be possible to transfer handshake signaling.
>
> I wrote this patch to add these registers to the ptmx driver.
> The modem status register, which controls CD,CTS,DSR,RI is normally
> read-only. To be able to
> pass these signals from the remote port to the local ptmx device, I've
> added an IOCTL routine.
> The result of a remote TIOCMGET IOCTL call , can be passed directly
> through this IOCTL routine.
> The IOCTL value I used for this is, 0x5426 which was formerly known as
> TIOCTTYGSTRUCT.
>
> I've tested this patch at an 2.6.16.27 kernel.
>
> Suggestions and/or other test results are welcome.
>
> Sander
>
>
> Signed-off-by: Sander van Ginkel <sander@van-ginkel.eu>
>
> ===================================================================
> --- drivers/char/pty.c	2008-03-03 20:02:09.000000000 +0100
> +++ drivers/char/pty-new.c	2008-03-02 21:55:53.000000000 +0100
> @@ -7,6 +7,8 @@
>   *    -- C. Scott Ananian <cananian@alumni.princeton.edu>, 14-Jan-1998
>   *  Added TTY_DO_WRITE_WAKEUP to enable n_tty to send POLL_OUT to
>   *      waiting writers -- Sapan Bhatia <sapan@corewars.org>
> + *  Added support for MCR/MSR, used for serial over ethernet
> + *    -- Sander van Ginkel <sander@van-ginkel.eu>
>   *
>   *
>   */
> @@ -32,6 +34,16 @@
>  #include <linux/bitops.h>
>  #include <linux/devpts_fs.h>
>
> +struct pty_mcrmsr {
> +
> +	struct semaphore        sem;            /* locks this structure */
> +
> +	/* for tiocmget and tiocmset functions */
> +
> +	int                     msr;            /* MSR shadow */
> +	int                     mcr;            /* MCR shadow */
> +};
> +
>  /* These are global because they are accessed in tty_io.c */
>  #ifdef CONFIG_UNIX98_PTYS
>  struct tty_driver *ptm_driver;
> @@ -199,6 +211,7 @@
>
>  static int pty_open(struct tty_struct *tty, struct file * filp)
>  {
> +        struct pty_mcrmsr *mcrmsr;
>  	int	retval = -ENODEV;
>
>  	if (!tty || !tty->link)
> @@ -216,10 +229,90 @@
>  	set_bit(TTY_THROTTLED, &tty->flags);
>  	set_bit(TTY_DO_WRITE_WAKEUP, &tty->flags);
>  	retval = 0;
> +
> +	/* initialize the pointer in case something fails */
> +
> +	tty->driver_data = NULL;
> +	tty->link->driver_data = NULL;
> +
> +	/* first time accessing this device, let's create it */
> +
> +	mcrmsr = kmalloc(sizeof(*mcrmsr), GFP_KERNEL);
> +
> +	init_MUTEX(&mcrmsr->sem);
> +
> +	if (mcrmsr != NULL) {
> +		down(&mcrmsr->sem);
> +
> +		/* save our structure within the tty structure */
> +
> +		mcrmsr->mcr=0;
> +		mcrmsr->msr=0;
> +
> +		tty->driver_data = mcrmsr;
> +		tty->link->driver_data = mcrmsr;
> +
> +		up(&mcrmsr->sem);
> +
> +	}
> +
>  out:
>  	return retval;
>  }
>
> +static int pty_tiocmget(struct tty_struct *tty, struct file *file)
> +{
> +	struct pty_mcrmsr *mcrmsr = tty->driver_data;
> +	unsigned int result = 0;
> +	unsigned int msr=0;
> +	unsigned int mcr=0;
> +
> +	if (tty->driver_data != NULL) {
> +		msr = mcrmsr->msr;
> +		mcr = mcrmsr->mcr;
> +	}
> +
> +	result = ((mcr & 0x01)  ? TIOCM_DTR  : 0) |  /* DTR is set */
> +		((mcr & 0x02)  ? TIOCM_RTS  : 0) |  /* RTS is set */
> +		((mcr & 0x04)  ? TIOCM_LOOP : 0) |  /* LOOP is set */
> +		((msr & 0x08)  ? TIOCM_CTS  : 0) |  /* CTS is set */
> +		((msr & 0x10)  ? TIOCM_CAR  : 0) |  /* Carrier detect is set*/
> +		((msr & 0x20)  ? TIOCM_RI   : 0) |  /* Ring Indicator is set */
> +		((msr & 0x40)  ? TIOCM_DSR  : 0);   /* DSR is set */
> +
> +	return result;
> +}
> +
> +static int pty_tiocmset(struct tty_struct *tty, struct file
> *file,unsigned int set, unsigned int clear)
> +{
> +	struct pty_mcrmsr *mcrmsr = tty->driver_data;
> +	unsigned int mcr=0;
> +
> +	if (tty->driver_data != NULL)
> +		mcr = mcrmsr->mcr;
> +
> +	if (set & TIOCM_DTR)
> +		mcr |= 0x01;
> +	if (set & TIOCM_RTS)
> +		mcr |= 0x02;
> +	if (set & TIOCM_LOOP)
> +		mcr |= 0x04;
> +
> +	if (clear & TIOCM_DTR)
> +		mcr &= ~0x01;
> +	if (clear & TIOCM_RTS)
> +		mcr &= ~0x02;
> +	if (clear & TIOCM_LOOP)
> +		mcr &= ~0x04;
> +
> +	/* set the new MCR value in the device */
> +
> +	if (tty->driver_data != NULL)
> +		mcrmsr->mcr=mcr;
> +
> +	return 0;
> +}
> +
>  static void pty_set_termios(struct tty_struct *tty, struct termios
> *old_termios)
>  {
>          tty->termios->c_cflag &= ~(CSIZE | PARENB);
> @@ -235,6 +328,8 @@
>  	.chars_in_buffer = pty_chars_in_buffer,
>  	.unthrottle = pty_unthrottle,
>  	.set_termios = pty_set_termios,
> +	.tiocmget = pty_tiocmget,
> +	.tiocmset = pty_tiocmset,
>  };
>
>  /* Traditional BSD devices */
> @@ -339,11 +434,44 @@
>  static int pty_unix98_ioctl(struct tty_struct *tty, struct file *file,
>  			    unsigned int cmd, unsigned long arg)
>  {
> +	unsigned int value;
> +	struct pty_mcrmsr *mcrmsr = tty->driver_data;
> +	unsigned int msr=0;
> +	unsigned int mcr=0;
> +
>  	switch (cmd) {
>  	case TIOCSPTLCK: /* Set PT Lock (disallow slave open) */
>  		return pty_set_lock(tty, (int __user *)arg);
>  	case TIOCGPTN: /* Get PT Number */
>  		return put_user(tty->index, (unsigned int __user *)arg);
> +
> +	case 0x5426: /* Set all of the handshake line, even the normally read
> only */
> +		{
> +		if (copy_from_user(&value,(unsigned int *)arg,sizeof(unsigned int)))
> +			return -EFAULT;
> +
> +		if (value & 0x0002) /* DTR set */
> +			mcr|=0x01;
> +		if (value & 0x0004) /* RTS set */
> +			mcr|=0x02;
> +		if (value & 0x8000) /* LOOP set */
> +			mcr|=0x04;
> +		if (value & 0x0020) /* CTS set */
> +			msr|=0x08;
> +		if (value & 0x0040) /* DCD set */
> +			msr|=0x10;
> +		if (value & 0x0080) /* RI set */
> +			msr|=0x20;
> +		if (value & 0x0100) /* DSR set */
> +			msr|=0x40;
> +
> +		if (tty->driver_data != NULL)
> +			{
> +			mcrmsr->msr=msr;
> +			mcrmsr->mcr=mcr;
> +			}
> +		return 0;
> +		}
>  	}
>
>  	return -ENOIOCTLCMD;

Re: [PATCH] ptmx: adding handshake support

[Alan Cox]

On Sun, 30 Mar 2008 14:09:48 +0200
postmaster@van-ginkel.eu wrote:

> This weekend I upgraded my kernel to 2.6.22.5 and tried my patch.
> Unfortunately pty.c seems to be a slightly different, so generated a new
> patch:

Really you want to be working against 2.6.25-rc5-mm1 or later as there
are big changes in the tty layer pending, some of which affect the
locking on stuff like this.

> +struct pty_mcrmsr {
> +
> +	struct semaphore        sem;            /* locks this structure */

You create a lock but don't use it. Also btw the -mm tty layer has a
ctrl_lock you could use instead.

> +	/* for tiocmget and tiocmset functions */
> +
> +	int                     msr;            /* MSR shadow */
> +	int                     mcr;            /* MCR shadow */

Could be one value using the existing bit masks.

> +	mcrmsr = kmalloc(sizeof(*mcrmsr), GFP_KERNEL);
> +
> +	init_MUTEX(&mcrmsr->sem);

Crashes on allocation failure

> +	if (tty->driver_data != NULL) {
> +		msr = mcrmsr->msr;
> +		mcr = mcrmsr->mcr;
> +	}

Always true

> +
> +	result = ((mcr & 0x01)  ? TIOCM_DTR  : 0) |  /* DTR is set */
> +		((mcr & 0x02)  ? TIOCM_RTS  : 0) |  /* RTS is set */
> +		((mcr & 0x04)  ? TIOCM_LOOP : 0) |  /* LOOP is set */
> +		((msr & 0x08)  ? TIOCM_CTS  : 0) |  /* CTS is set */
> +		((msr & 0x10)  ? TIOCM_CAR  : 0) |  /* Carrier detect is set*/
> +		((msr & 0x20)  ? TIOCM_RI   : 0) |  /* Ring Indicator is set */
> +		((msr & 0x40)  ? TIOCM_DSR  : 0);   /* DSR is set */

Use the mcr/msr bits as is and you don't need this mess

> +	if (set & TIOCM_DTR)
> +		mcr |= 0x01;
> +	if (set & TIOCM_RTS)
> +		mcr |= 0x02;
> +	if (set & TIOCM_LOOP)

Ditto

> +	/* set the new MCR value in the device */
> +
> +	if (tty->driver_data != NULL)
> +		mcrmsr->mcr=mcr;

That may cause wakeups, open, hangup etc to occur - does that need to be
considered here.

> +	case 0x5426: /* Set all of the handshake line, even the normally  
> read only */

Use a proper value as I said before.

Re: [PATCH] ptmx: adding handshake support

[sander van ginkel]

Ok, I will test this kernel if its stable enough for my needs, and  
later this week I will move on to the latest kernel.
Further I shall look at your comments and make some corrections to my code.

One thing thats comming up to me:

>> +    case 0x5426: /* Set all of the handshake line, even the  
>> normally  read only */
>>

> Use a proper value as I said before.


I may pick the last value + 1 defined in the lib?

thanks for your input


Quoting Alan Cox <alan@lxorguk.ukuu.org.uk>:

> On Sun, 30 Mar 2008 14:09:48 +0200
> postmaster@van-ginkel.eu wrote:
>
>> This weekend I upgraded my kernel to 2.6.22.5 and tried my patch.
>> Unfortunately pty.c seems to be a slightly different, so generated a new
>> patch:
>
> Really you want to be working against 2.6.25-rc5-mm1 or later as there
> are big changes in the tty layer pending, some of which affect the
> locking on stuff like this.
>
>> +struct pty_mcrmsr {
>> +
>> +	struct semaphore        sem;            /* locks this structure */
>
> You create a lock but don't use it. Also btw the -mm tty layer has a
> ctrl_lock you could use instead.
>
>> +	/* for tiocmget and tiocmset functions */
>> +
>> +	int                     msr;            /* MSR shadow */
>> +	int                     mcr;            /* MCR shadow */
>
> Could be one value using the existing bit masks.
>
>> +	mcrmsr = kmalloc(sizeof(*mcrmsr), GFP_KERNEL);
>> +
>> +	init_MUTEX(&mcrmsr->sem);
>
> Crashes on allocation failure
>
>> +	if (tty->driver_data != NULL) {
>> +		msr = mcrmsr->msr;
>> +		mcr = mcrmsr->mcr;
>> +	}
>
> Always true
>
>> +
>> +	result = ((mcr & 0x01)  ? TIOCM_DTR  : 0) |  /* DTR is set */
>> +		((mcr & 0x02)  ? TIOCM_RTS  : 0) |  /* RTS is set */
>> +		((mcr & 0x04)  ? TIOCM_LOOP : 0) |  /* LOOP is set */
>> +		((msr & 0x08)  ? TIOCM_CTS  : 0) |  /* CTS is set */
>> +		((msr & 0x10)  ? TIOCM_CAR  : 0) |  /* Carrier detect is set*/
>> +		((msr & 0x20)  ? TIOCM_RI   : 0) |  /* Ring Indicator is set */
>> +		((msr & 0x40)  ? TIOCM_DSR  : 0);   /* DSR is set */
>
> Use the mcr/msr bits as is and you don't need this mess
>
>> +	if (set & TIOCM_DTR)
>> +		mcr |= 0x01;
>> +	if (set & TIOCM_RTS)
>> +		mcr |= 0x02;
>> +	if (set & TIOCM_LOOP)
>
> Ditto
>
>> +	/* set the new MCR value in the device */
>> +
>> +	if (tty->driver_data != NULL)
>> +		mcrmsr->mcr=mcr;
>
> That may cause wakeups, open, hangup etc to occur - does that need to be
> considered here.
>
>> +	case 0x5426: /* Set all of the handshake line, even the normally
>> read only */
>
> Use a proper value as I said before.
>
>
>
>

Re: [PATCH] ptmx: adding handshake support

[sander van ginkel]

I've created a new patch, build against linux-2.6.25-rc8.

Signed-off-by: Sander van Ginkel <sander@van-ginkel.eu>

===================================================================

diff -pru linux-2.6.25-rc8.orig/drivers/char/pty.c 
linux-2.6.25-rc8/drivers/char/pty.c
--- linux-2.6.25-rc8.orig/drivers/char/pty.c    2008-04-01 
21:44:26.000000000 +0200
+++ linux-2.6.25-rc8/drivers/char/pty.c    2008-04-13 16:40:03.000000000 
+0200
@@ -7,6 +7,8 @@
  *    -- C. Scott Ananian <cananian@alumni.princeton.edu>, 14-Jan-1998
  *  Added TTY_DO_WRITE_WAKEUP to enable n_tty to send POLL_OUT to
  *      waiting writers -- Sapan Bhatia <sapan@corewars.org>
+ *  Added support for MCR/MSR, used for serial over ethernet
+ *    -- Sander van Ginkel <sander@van-ginkel.eu>
  *
  *
  */
@@ -196,6 +198,7 @@ static void pty_flush_buffer(struct tty_
 
 static int pty_open(struct tty_struct *tty, struct file * filp)
 {
+    unsigned int    *mcrmsr;
     int    retval = -ENODEV;
 
     if (!tty || !tty->link)
@@ -212,11 +215,56 @@ static int pty_open(struct tty_struct *t
     clear_bit(TTY_OTHER_CLOSED, &tty->link->flags);
     set_bit(TTY_THROTTLED, &tty->flags);
     set_bit(TTY_DO_WRITE_WAKEUP, &tty->flags);
+
+    /* initialize the pointer in case something fails */
+   
+    tty->driver_data = NULL;                         
+    tty->link->driver_data = NULL;
+
+    /* first time accessing this device, let's create it */
+
+    mcrmsr = kmalloc(sizeof(*mcrmsr), GFP_KERNEL);
+
+    if (mcrmsr != NULL) {
+
+        /* save our data within the tty structure */
+
+        *mcrmsr=0;
+
+        tty->driver_data = mcrmsr;
+        tty->link->driver_data = mcrmsr;
+    }
+    else
+    {
+        goto out;
+    }       
+
     retval = 0;
+
 out:
     return retval;
 }
 
+static int pty_tiocmget(struct tty_struct *tty, struct file *file)
+{
+    unsigned int *mcrmsr;
+
+    mcrmsr = tty->driver_data;
+
+    return *mcrmsr;
+}
+
+static int pty_tiocmset(struct tty_struct *tty, struct file 
*file,unsigned int set, unsigned int clear)
+{
+    unsigned int *mcrmsr;
+
+    mcrmsr = tty->driver_data;
+    *mcrmsr=set;
+    tty->driver_data=mcrmsr;
+
+    return 0;
+}
+
 static void pty_set_termios(struct tty_struct *tty, struct ktermios 
*old_termios)
 {
         tty->termios->c_cflag &= ~(CSIZE | PARENB);
@@ -232,6 +280,8 @@ static const struct tty_operations pty_o
     .chars_in_buffer = pty_chars_in_buffer,
     .unthrottle = pty_unthrottle,
     .set_termios = pty_set_termios,
+    .tiocmget = pty_tiocmget,
+    .tiocmset = pty_tiocmset,
 };
 
 /* Traditional BSD devices */
@@ -364,11 +414,27 @@ static struct ctl_table pty_root_table[]
 static int pty_unix98_ioctl(struct tty_struct *tty, struct file *file,
                 unsigned int cmd, unsigned long arg)
 {
+    unsigned int value;
+    unsigned int *mcrmsr;
+
+        mcrmsr=tty->driver_data;
+
     switch (cmd) {
     case TIOCSPTLCK: /* Set PT Lock (disallow slave open) */
         return pty_set_lock(tty, (int __user *)arg);
     case TIOCGPTN: /* Get PT Number */
         return put_user(tty->index, (unsigned int __user *)arg);
+
+    case VMCRMSR: /* Set all of the handshake line, even the normally 
read only */
+        {
+        if (copy_from_user(&value,(unsigned int *)arg,sizeof(unsigned 
int)))
+            return -EFAULT;
+
+        *mcrmsr=value;
+        tty->driver_data=mcrmsr;
+
+        return 0;
+        }         
     }
 
     return -ENOIOCTLCMD;

Re: [PATCH] ptmx: adding handshake support

[Alan Cox]

> + *  Added support for MCR/MSR, used for serial over ethernet
> + *    -- Sander van Ginkel <sander@van-ginkel.eu>

We've been trying to get rid of these long lists in the code and put them
in the git tree (git whatchanged/git blame show the info rather better)

> +
> +    /* initialize the pointer in case something fails */
> +   
> +    tty->driver_data = NULL;                         
> +    tty->link->driver_data = NULL;

Not needed

  +    /* first time accessing this device, let's create it */
> +
> +    mcrmsr = kmalloc(sizeof(*mcrmsr), GFP_KERNEL);

kzalloc will clear it for you...

> +static int pty_tiocmset(struct tty_struct *tty, struct file 
> *file,unsigned int set, unsigned int clear)
> +{
> +    unsigned int *mcrmsr;
> +
> +    mcrmsr = tty->driver_data;
> +    *mcrmsr=set;
> +    tty->driver_data=mcrmsr;

Why this last assignment ?

> +
> +    case VMCRMSR: /* Set all of the handshake line, even the normally 
> read only */
> +        {
> +        if (copy_from_user(&value,(unsigned int *)arg,sizeof(unsigned 
> int)))
> +            return -EFAULT;
> +
> +        *mcrmsr=value;

Ok - possibly we shouldn't allow people to set undefined bits but I'm not
sure it matters

> +        tty->driver_data=mcrmsr;

Why ??


I am curious how this is handled by other Unix systems and if there is an
ioctl we can follow from other systems ?


Looks basically ok, coding style is wrong, some odd extra assignments but
I agree entirely with the idea of adding this functionality to keep
remote serial drivers in user space.

I'll try and find out if other Unixes have similar features we can use to
keep API consistency tidy it up and fold it at some point in the next
couple of weeks.

Alan

Re: [PATCH] ptmx: adding handshake support

[Randy Dunlap]

On Mon, 14 Apr 2008 22:39:21 +0200 sander van ginkel wrote:

> Corrected the last issues

Not AFAICT.  There's no full changelog/description here...
and see below.


> Signed-off-by: Sander van Ginkel <sander@van-ginkel.eu>
> 
> ===================================================================
> 
> diff -pru linux-2.6.25-rc8.orig/drivers/char/pty.c 
> linux-2.6.25-rc8/drivers/char/pty.c
> --- linux-2.6.25-rc8.orig/drivers/char/pty.c    2008-04-01 
> 21:44:26.000000000 +0200
> +++ linux-2.6.25-rc8/drivers/char/pty.c    2008-04-14 23:01:56.000000000 
> +0200
> @@ -196,6 +196,7 @@ static void pty_flush_buffer(struct tty_
>  
>  static int pty_open(struct tty_struct *tty, struct file * filp)
>  {
> +    unsigned int    *mcrmsr;

Something converted tabs to spaces throughout the entire patch.


>      int    retval = -ENODEV;
>  
>      if (!tty || !tty->link)

---
~Randy

Re: [PATCH] ptmx: adding handshake support

[sander van ginkel]

Corrected the last issues

Signed-off-by: Sander van Ginkel <sander@van-ginkel.eu>

===================================================================

diff -pru linux-2.6.25-rc8.orig/drivers/char/pty.c 
linux-2.6.25-rc8/drivers/char/pty.c
--- linux-2.6.25-rc8.orig/drivers/char/pty.c    2008-04-01 
21:44:26.000000000 +0200
+++ linux-2.6.25-rc8/drivers/char/pty.c    2008-04-14 23:01:56.000000000 
+0200
@@ -196,6 +196,7 @@ static void pty_flush_buffer(struct tty_
 
 static int pty_open(struct tty_struct *tty, struct file * filp)
 {
+    unsigned int    *mcrmsr;
     int    retval = -ENODEV;
 
     if (!tty || !tty->link)
@@ -212,11 +213,50 @@ static int pty_open(struct tty_struct *t
     clear_bit(TTY_OTHER_CLOSED, &tty->link->flags);
     set_bit(TTY_THROTTLED, &tty->flags);
     set_bit(TTY_DO_WRITE_WAKEUP, &tty->flags);
+
+    /* first time accessing this device, let's create it */
+
+    mcrmsr = kzalloc(sizeof(*mcrmsr), GFP_KERNEL);
+
+    if (mcrmsr != NULL) {
+
+        /* save our data within the tty structure */
+
+        *mcrmsr=0;
+
+        tty->driver_data = mcrmsr;
+        tty->link->driver_data = mcrmsr;
+    }
+    else
+    {
+        goto out;
+    }      
+
     retval = 0;
+
 out:
     return retval;
 }
 
+static int pty_tiocmget(struct tty_struct *tty, struct file *file)
+{
+    unsigned int *mcrmsr;
+
+    mcrmsr = tty->driver_data;
+
+    return *mcrmsr;
+}
+
+static int pty_tiocmset(struct tty_struct *tty, struct file 
*file,unsigned int set, unsigned int clear)
+{
+    unsigned int *mcrmsr;
+
+    mcrmsr = tty->driver_data;
+    *mcrmsr=set;
+
+    return 0;
+}
+
 static void pty_set_termios(struct tty_struct *tty, struct ktermios 
*old_termios)
 {
         tty->termios->c_cflag &= ~(CSIZE | PARENB);
@@ -232,6 +272,8 @@ static const struct tty_operations pty_o
     .chars_in_buffer = pty_chars_in_buffer,
     .unthrottle = pty_unthrottle,
     .set_termios = pty_set_termios,
+    .tiocmget = pty_tiocmget,
+    .tiocmset = pty_tiocmset,
 };
 
 /* Traditional BSD devices */
@@ -364,11 +406,26 @@ static struct ctl_table pty_root_table[]
 static int pty_unix98_ioctl(struct tty_struct *tty, struct file *file,
                 unsigned int cmd, unsigned long arg)
 {
+    unsigned int value;
+    unsigned int *mcrmsr;
+
+        mcrmsr=tty->driver_data;
+
     switch (cmd) {
     case TIOCSPTLCK: /* Set PT Lock (disallow slave open) */
         return pty_set_lock(tty, (int __user *)arg);
     case TIOCGPTN: /* Get PT Number */
         return put_user(tty->index, (unsigned int __user *)arg);
+
+    case VMCRMSR: /* Set all of the handshake line, even the normally 
read only */
+        {
+        if (copy_from_user(&value,(unsigned int *)arg,sizeof(unsigned 
int)))
+            return -EFAULT;
+
+        *mcrmsr=value;
+
+        return 0;
+        }        
     }
 
     return -ENOIOCTLCMD;

Re: [PATCH] ptmx: adding handshake support

[Alan Cox]

On Mon, 14 Apr 2008 22:39:21 +0200
sander van ginkel <sander@van-ginkel.eu> wrote:

> Corrected the last issues
> 
> Signed-off-by: Sander van Ginkel <sander@van-ginkel.eu>

NAK

Having played with this (and fixed up further issues) it simply doesn't
work in the real world. pty/tty pairs are always 8bit, don't support
CREAD and can't pass break signals, emulate multiple flow control types
or parity.

A virtual serial device is still a good idea, but a quick hack on pty
devices doesn't actually work - the problem is far more complicated.

Alan

[PATCH] ptmx: adding handshake support

[sander van ginkel]

For serial over ethernet communication, /dev/ptmx will be used to create 
a virtual device.
Due to the lack of a modem status register and a modem control register 
in the ptmx driver,
it won't be possible to transfer handshake signaling.

I wrote this patch to add these registers to the ptmx driver.
The modem status register, which controls CD,CTS,DSR,RI is normally 
read-only. To be able to
pass these signals from the remote port to the local ptmx device, I've 
added an IOCTL routine.
The result of a remote TIOCMGET IOCTL call , can be passed directly 
through this IOCTL routine.
The IOCTL value I used for this is, 0x5426 which was formerly known as 
TIOCTTYGSTRUCT.

I've tested this patch at an 2.6.16.27 kernel.

Suggestions and/or other test results are welcome.

Sander


Signed-off-by: Sander van Ginkel <sander@van-ginkel.eu>

===================================================================
--- drivers/char/pty.c    2008-03-03 20:02:09.000000000 +0100
+++ drivers/char/pty-new.c    2008-03-02 21:55:53.000000000 +0100
@@ -7,6 +7,8 @@
  *    -- C. Scott Ananian <cananian@alumni.princeton.edu>, 14-Jan-1998
  *  Added TTY_DO_WRITE_WAKEUP to enable n_tty to send POLL_OUT to
  *      waiting writers -- Sapan Bhatia <sapan@corewars.org>
+ *  Added support for MCR/MSR, used for serial over ethernet
+ *    -- Sander van Ginkel <sander@van-ginkel.eu>
  *
  *
  */
@@ -32,6 +34,16 @@
 #include <linux/bitops.h>
 #include <linux/devpts_fs.h>
 
+struct pty_mcrmsr {
+
+    struct semaphore        sem;            /* locks this structure */
+
+    /* for tiocmget and tiocmset functions */
+
+    int                     msr;            /* MSR shadow */
+    int                     mcr;            /* MCR shadow */
+};
+
 /* These are global because they are accessed in tty_io.c */
 #ifdef CONFIG_UNIX98_PTYS
 struct tty_driver *ptm_driver;
@@ -199,6 +211,7 @@
 
 static int pty_open(struct tty_struct *tty, struct file * filp)
 {
+        struct pty_mcrmsr *mcrmsr;
     int    retval = -ENODEV;
 
     if (!tty || !tty->link)
@@ -216,10 +229,90 @@
     set_bit(TTY_THROTTLED, &tty->flags);
     set_bit(TTY_DO_WRITE_WAKEUP, &tty->flags);
     retval = 0;
+
+    /* initialize the pointer in case something fails */
+    
+    tty->driver_data = NULL;                          
+    tty->link->driver_data = NULL;
+
+    /* first time accessing this device, let's create it */
+
+    mcrmsr = kmalloc(sizeof(*mcrmsr), GFP_KERNEL);
+
+    init_MUTEX(&mcrmsr->sem);
+
+    if (mcrmsr != NULL) {
+        down(&mcrmsr->sem);
+
+        /* save our structure within the tty structure */
+
+        mcrmsr->mcr=0;
+        mcrmsr->msr=0;
+
+        tty->driver_data = mcrmsr;
+        tty->link->driver_data = mcrmsr;
+
+        up(&mcrmsr->sem);
+
+    }
+
 out:
     return retval;
 }
 
+static int pty_tiocmget(struct tty_struct *tty, struct file *file)
+{
+    struct pty_mcrmsr *mcrmsr = tty->driver_data;
+    unsigned int result = 0;
+    unsigned int msr=0;
+    unsigned int mcr=0;
+
+    if (tty->driver_data != NULL) {
+        msr = mcrmsr->msr;
+        mcr = mcrmsr->mcr;
+    }
+
+    result = ((mcr & 0x01)  ? TIOCM_DTR  : 0) |  /* DTR is set */
+        ((mcr & 0x02)  ? TIOCM_RTS  : 0) |  /* RTS is set */
+        ((mcr & 0x04)  ? TIOCM_LOOP : 0) |  /* LOOP is set */
+        ((msr & 0x08)  ? TIOCM_CTS  : 0) |  /* CTS is set */
+        ((msr & 0x10)  ? TIOCM_CAR  : 0) |  /* Carrier detect is set*/
+        ((msr & 0x20)  ? TIOCM_RI   : 0) |  /* Ring Indicator is set */
+        ((msr & 0x40)  ? TIOCM_DSR  : 0);   /* DSR is set */
+
+    return result;
+}
+
+static int pty_tiocmset(struct tty_struct *tty, struct file 
*file,unsigned int set, unsigned int clear)
+{
+    struct pty_mcrmsr *mcrmsr = tty->driver_data;
+    unsigned int mcr=0;
+
+    if (tty->driver_data != NULL)
+        mcr = mcrmsr->mcr;
+
+    if (set & TIOCM_DTR)
+        mcr |= 0x01;
+    if (set & TIOCM_RTS)
+        mcr |= 0x02;
+    if (set & TIOCM_LOOP)
+        mcr |= 0x04;
+
+    if (clear & TIOCM_DTR)
+        mcr &= ~0x01;
+    if (clear & TIOCM_RTS)
+        mcr &= ~0x02;
+    if (clear & TIOCM_LOOP)
+        mcr &= ~0x04;
+
+    /* set the new MCR value in the device */
+
+    if (tty->driver_data != NULL)
+        mcrmsr->mcr=mcr;
+
+    return 0;
+}
+
 static void pty_set_termios(struct tty_struct *tty, struct termios 
*old_termios)
 {
         tty->termios->c_cflag &= ~(CSIZE | PARENB);
@@ -235,6 +328,8 @@
     .chars_in_buffer = pty_chars_in_buffer,
     .unthrottle = pty_unthrottle,
     .set_termios = pty_set_termios,
+    .tiocmget = pty_tiocmget,
+    .tiocmset = pty_tiocmset,
 };
 
 /* Traditional BSD devices */
@@ -339,11 +434,44 @@
 static int pty_unix98_ioctl(struct tty_struct *tty, struct file *file,
                 unsigned int cmd, unsigned long arg)
 {
+    unsigned int value;
+    struct pty_mcrmsr *mcrmsr = tty->driver_data;
+    unsigned int msr=0;
+    unsigned int mcr=0;
+
     switch (cmd) {
     case TIOCSPTLCK: /* Set PT Lock (disallow slave open) */
         return pty_set_lock(tty, (int __user *)arg);
     case TIOCGPTN: /* Get PT Number */
         return put_user(tty->index, (unsigned int __user *)arg);
+
+    case 0x5426: /* Set all of the handshake line, even the normally 
read only */
+        {
+        if (copy_from_user(&value,(unsigned int *)arg,sizeof(unsigned 
int)))
+            return -EFAULT;
+
+        if (value & 0x0002) /* DTR set */
+            mcr|=0x01;
+        if (value & 0x0004) /* RTS set */
+            mcr|=0x02;
+        if (value & 0x8000) /* LOOP set */
+            mcr|=0x04;
+        if (value & 0x0020) /* CTS set */
+            msr|=0x08;
+        if (value & 0x0040) /* DCD set */
+            msr|=0x10;
+        if (value & 0x0080) /* RI set */
+            msr|=0x20;
+        if (value & 0x0100) /* DSR set */
+            msr|=0x40;
+
+        if (tty->driver_data != NULL)
+            {
+            mcrmsr->msr=msr;
+            mcrmsr->mcr=mcr;
+            }
+        return 0;
+        }          
     }
 
     return -ENOIOCTLCMD;