2008-03-11 20:42:20

by sander van ginkel

[permalink] [raw]
Subject: [PATCH] ptmx: adding handshake support

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 <[email protected]>

===================================================================
--- 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 <[email protected]>, 14-Jan-1998
* Added TTY_DO_WRITE_WAKEUP to enable n_tty to send POLL_OUT to
* waiting writers -- Sapan Bhatia <[email protected]>
+ * Added support for MCR/MSR, used for serial over ethernet
+ * -- Sander van Ginkel <[email protected]>
*
*
*/
@@ -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;


2008-03-30 12:24:07

by postmaster

[permalink] [raw]
Subject: Re: [PATCH] ptmx: adding handshake support

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 <[email protected]>

===================================================================
--- 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 <[email protected]>, 14-Jan-1998
* Added TTY_DO_WRITE_WAKEUP to enable n_tty to send POLL_OUT to
* waiting writers -- Sapan Bhatia <[email protected]>
+ * Added support for MCR/MSR, used for serial over ethernet
+ * -- Sander van Ginkel <[email protected]>
*
*
*/
@@ -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 <[email protected]>:

> 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 <[email protected]>
>
> ===================================================================
> --- 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 <[email protected]>, 14-Jan-1998
> * Added TTY_DO_WRITE_WAKEUP to enable n_tty to send POLL_OUT to
> * waiting writers -- Sapan Bhatia <[email protected]>
> + * Added support for MCR/MSR, used for serial over ethernet
> + * -- Sander van Ginkel <[email protected]>
> *
> *
> */
> @@ -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;


2008-03-30 12:34:32

by Alan

[permalink] [raw]
Subject: Re: [PATCH] ptmx: adding handshake support

On Sun, 30 Mar 2008 14:09:48 +0200
[email protected] 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.


2008-03-30 12:55:39

by sander van ginkel

[permalink] [raw]
Subject: Re: [PATCH] ptmx: adding handshake support

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 <[email protected]>:

> On Sun, 30 Mar 2008 14:09:48 +0200
> [email protected] 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.
>
>
>
>