> + qt2_box_flush(serial, port->number, QT2_FLUSH_TX); /* flush tx buffer */
> + qt2_box_flush(serial, port->number, QT2_FLUSH_RX); /* flush rx buffer */
> + /* now wait for the flags to magically go back to being true */
> + jift = jiffies + (10 * HZ);
> + do {
> + if ((port_extra->rcv_flush == true) &&
Horkkkk...
Use wait_event and wakeup not busy loops.
Also btw if you implement the chars_in_buffer() method the kernel will
wait for that to report 0 before closing. That is the correct way to
handle it as the functionality is also needed for things like break
handling.
> + jift = jiffies + (10 * HZ); /* 10 sec timeout */
> + do {
> + status = qt2_box_get_register(serial, port->number,
> + QT2_LINE_STATUS_REGISTER, &lsr_value);
> + if (status < 0) {
> + dbg("%s(): qt2_box_get_register failed", __func__);
> + break;
> + }
> + if ((lsr_value & QT2_LSR_TEMT)) {
> + dbg("UART done sending");
> + break;
> + }
> + schedule();
If you have to poll then it might be a good idea to sleep between polls
so you don't jam up the USB bus.
> +/* called to deliver writes from the next layer up to the device */
> +static int qt2_write(struct tty_struct *tty, struct usb_serial_port *port,
> + const unsigned char *buf, int count)
> +{
> + struct usb_serial *serial; /* parent device struct */
> + __u8 header_array[5]; /* header used to direct writes to the correct
You will save yourself an enormous amount of pain if you ignore the
existing USB drivers on this one (as they mostly suck rocks) and
implement a proper 4K or so FIFO. That cleans up so much its unbelievable
(especially as we have the kfifo API ...). At that point you get proper
buffering and you just empty the fifo into an URB every time you get a
completion for the last one.
> +static int qt2_write_room(struct tty_struct *tty)
> +{
> + struct usb_serial_port *port = tty->driver_data;
> + /* parent usb_serial_port pointer */
> + struct quatech2_port *port_extra; /* extra data for this port */
> + int room = 0;
> + port_extra = qt2_get_port_private(port);
> +
> + if (port_extra->close_pending == true) {
> + dbg("%s(): port_extra->close_pending == true", __func__);
> + return -ENODEV;
> + }
> +
> + dbg("%s(): port %d", __func__, port->number);
> + if ((port->write_urb->status != -EINPROGRESS) &&
> + (port_extra->tx_pending_bytes == 0))
> + room = port->bulk_out_size - QT2_TX_HEADER_LENGTH;
> + return room;
This value must never go down by more than bytes written - its a promise
which a lot of USB drivers don't properly keep causing lost chars etc
> +static int qt2_ioctl(struct tty_struct *tty, struct file *file,
> + unsigned int cmd, unsigned long arg)
This looks pretty bogus.
TIOCMGET/TIOCMSET are not passed to the driver ioctl method but to your
mget and mset operators so you can dump that lot in favour of the code
you have later.
>
> + } else if (cmd == TIOCMIWAIT) {
> + dbg("%s() port %d, cmd == TIOCMIWAIT enter",
> + __func__, port->number);
This one if provided does need to be in the ioctl handler.
> + prev_msr_value = port_extra->shadowMSR & SERIAL_MSR_MASK;
> + while (1) {
> + add_wait_queue(&port_extra->wait, &wait);
> + set_current_state(TASK_INTERRUPTIBLE);
> + schedule();
> + dbg("%s(): port %d, cmd == TIOCMIWAIT here\n",
> + __func__, port->number);
> + remove_wait_queue(&port_extra->wait, &wait);
> + /* see if a signal woke us up */
> + if (signal_pending(current))
> + return -ERESTARTSYS;
> + msr_value = port_extra->shadowMSR & SERIAL_MSR_MASK;
> + if (msr_value == prev_msr_value)
> + return -EIO; /* no change - error */
Why is this an error and not just a spurious wake up ?
What about disconnecting the adapter btw - shouldn't that break out of
the loop with an error ?
>
--
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
