Messages in this thread | | | Date | Sun, 30 Mar 2008 13:16:52 +0100 | From | Alan Cox <> | Subject | Re: [PATCH] ptmx: adding handshake support |
| |
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.
| |