> 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.
>
>
>
>
