On Fri, 17 Dec 2010 16:34:23 +0100
Libor Pechacek <lpechacek@xxxxxxx> wrote:
> Alan's commit 335f8514f200e63d689113d29cb7253a5c282967 introduced
> .carrier_raised function in several drivers. That also means
> tty_port_block_til_ready can now suspend the process trying to open the serial
> port when Carrier Detect is low and put it into tty_port.open_wait queue. We
> need to wake up the process when the Carrier Detect goes high.
(catching up after Christmas break)
> @Greg: There are two drivers left to be fixed - drivers/usb/serial/cp210x.c and
> drivers/usb/serial/keyspan_pda.c. cp210x device does not seem to have an
> interrupt endpoint to report the changes so a polling thread seems to be needed
> to detect DCD changes. I can implement the polling for cp210x if you don't
> have a better idea.
It might be worth making tty_port_open know how to handle such devices as
I'd bet there will be others out there somewhere. Perhaps add a dcd_poll
field that holds the poll time in ms then in the tty_port_block_til_ready
code change from
tty_unlock();
schedule();
tty_lock();
to
tty_unlock();
if (port->dcd_poll)
schedule_timeout(port->dcd_poll);
else
schedule();
tty_lock
??
> The keyspan_pda can report the changes asynchronously, but I haven't found the
> message format description anywhere. Any idea how that driver can be fixed
> besides dropping .carrier_raised?
I don't - Greg may have info or of course you could cheat and wake the
queue whenever you see an unknown message format, then ask the device 8)
> +/**
> + * usb_serial_handle_dcd_change - handle a change of carrier detect state
> + * @port: usb_serial_port structure for the open port
> + * @status: new carrier detect status, nonzero if active
> + */
> +void usb_serial_handle_dcd_change(struct usb_serial_port *usb_port,
> + unsigned int status)
> +{
> + struct tty_port *port = &usb_port->port;
> + struct tty_struct *tty = port->tty;
> +
> + dbg("%s - port %d, status %d", __func__, usb_port->number, status);
> +
> + if (status)
> + wake_up_interruptible(&port->open_wait);
> + else if (tty && !C_CLOCAL(tty))
> + tty_hangup(tty);
> +}
> +EXPORT_SYMBOL_GPL(usb_serial_handle_dcd_change);
This looks good except that port->tty isn't necessarily a safe
de-reference so it would be better to pass the tty into the function so
the caller must think about that problem (and all the calling points
appear to have a tty reference held ready)
Looks good
Alan
--
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
