> This patch apply the changes done from 0.5 to 0.9 to the last kernel version.
> The changes covered:
>
> - basic implementation of ioctl(),
These appear to mostly be regressions. A lot of crud got removed from
drivers going into the kernel and most of this is putting back stuff that
shouldn't be there. A few bits are correct however.
Can you split out the parity change and the locking change into two
separate patches, test them and dump the rest ?
(More detailed summary of their changes you turned into a patch..)
> - int writelen; /* num of byte to write to device */
> + u8 writelen; /* num of byte to write to device */
int is usually faster and as packed takes the same space.
> +static int iuu_ioctl(struct tty_struct *tty, struct file *file,
> + unsigned int cmd, unsigned long arg)
> +{
> + int mask;
> + struct usb_serial_port *port = tty->driver_data;
> +
> + dbg("%s (%d) cmd = 0x%04x", __func__, port->number, cmd);
> +
> + switch (cmd) {
> +
> + /* could be usefull later... */
> + case TCFLSH:
> + return 0;
This is wrong (the tty layer handles this itself)
> +
> + case TIOCGSERIAL:
> + case TCGETS:
> + dbg("%s TIOCGSERIAL", __func__);
> + mask = CLOCAL | CREAD | CS8 | B9600 | TIOCM_CTS | CSTOPB
> + | PARODD;
> + put_user(mask, &arg);
> + return 0;
> + case TIOCMBIS:
> + case TIOCMBIC:
> + case TIOCSSERIAL:
> + if (get_user(mask, (unsigned long *)arg))
> + return -EFAULT;
> + dbg("%s (%d) msg = %04x", __func__, port->number, mask);
> + return 0;
These are wrong (tty layer handles them itself)
> + case TIOCMIWAIT:
> + dbg("%s (%d) TIOCMIWAIT", __func__, port->number);
> + return 0;
> +
> + case TCSBRK:
> + dbg("%s (%d) TIOBRK", __func__, port->number);
> + return 0;
These are wrong (ditto)
> @@ -651,32 +697,34 @@ static int iuu_bulk_write(struct usb_serial_port *port)
> unsigned long flags;
> int result;
> int i;
> + u8 buf_len;
> char *buf_ptr = port->write_urb->transfer_buffer;
> dbg("%s - enter", __func__);
>
> + spin_lock_irqsave(&priv->lock, flags);
> *buf_ptr++ = IUU_UART_ESC;
The locking looks like it might be a real fix
> @@ -819,7 +863,7 @@ static int iuu_uart_on(struct usb_serial_port *port)
> buf[0] = IUU_UART_ENABLE;
> buf[1] = (u8) ((IUU_BAUD_9600 >> 8) & 0x00FF);
> buf[2] = (u8) (0x00FF & IUU_BAUD_9600);
> - buf[3] = (u8) (0x0F0 & IUU_TWO_STOP_BITS) | (0x07 & IUU_PARITY_EVEN);
> + buf[3] = (u8) (0x0F0 & IUU_ONE_STOP_BIT) | (0x07 & IUU_PARITY_EVEN);
>
> status = bulk_immediate(port, buf, 4);
Might be valid
> if (status != IUU_OPERATION_OK) {
> @@ -946,6 +990,30 @@ static int iuu_uart_baud(struct usb_serial_port *port, u32 baud,
> return status;
> }
>
> +static void iuu_set_termios(struct tty_struct *tty,
> + struct usb_serial_port *port, struct ktermios *old)
> +{
> + unsigned int cflag = tty->termios->c_cflag;
> + int status;
> + u32 actual;
> + dbg("%s (%d) ", __func__, port->number);
> + if (cflag & PARODD) {
> + status = iuu_uart_baud(port,
> + (clockmode == 2) ? 16457 : 9600 * boost / 100,
> + &actual, IUU_PARITY_ODD | IUU_TWO_STOP_BITS);
> + dbg("%s (%d) ODD", __func__, port->number);
> + return;
> + }
> +
> + if (cflag & PARENB) {
> + status = iuu_uart_baud(port,
> + (clockmode == 2) ? 16457 : 9600 * boost / 100,
> + &actual, IUU_PARITY_EVEN | IUU_ONE_STOP_BIT);
> + dbg("%s (%d) EVEN", __func__, port->number);
> + }
> +
> +}
Very incomplete and also somewhat wrong, but at least it shows how to set
the feature. A proper implementation needs to copy back the old termios
hardware settings and only change the returned parity flag so that the
user app knows most of its settings were rejected. It looks very odd that
it changes the number of stop bits according to the parity however.
> +
> static int set_control_lines(struct usb_device *dev, u8 value)
> {
> return 0;
> @@ -1044,13 +1112,16 @@ static int iuu_open(struct tty_struct *tty,
> if (tty && !priv->termios_initialized) {
> *(tty->termios) = tty_std_termios;
> tty->termios->c_cflag = CLOCAL | CREAD | CS8 | B9600
> - | TIOCM_CTS | CSTOPB | PARENB;
> + | TIOCM_CTS | PARENB;
> + tty->termios->c_cflag &= ~(CSIZE|PARODD|CSTOPB);
> + tty->termios->c_lflag &= ~(ECHO|ECHONL|ICANON|ISIG|IEXTEN);
> + tty->termios->c_oflag &= ~OPOST;;
> + tty->termios->c_iflag &= ~(IGNBRK|BRKINT|PARMRK|ISTRIP
> + |INLCR|IGNCR|ICRNL|IXON);
Nope - the existing code seems right
> priv->termios_initialized = 1;
> + tty->low_latency = 1;
No - this was a long standing bug that is already fixed in the kernel
driver.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
