Hi,
1)
Attached is a patch that replaces the use of usb_unlink_urb() of the read
URB when the device is being closed with usb_kill_urb(). The second call
is more appropriate and recommended by urb.c. In fact, the ftdi_close()
tries to shut down the URB's synchronously since the URB_ASYNC_UNLINK
flag is cleared. The current version with usb_unlink_urb(), causes the
kernel to generate a warning about this. Did I miss anything here, or is
it right to use the usb_kill_urb()?
-------------------------------------------------------------------------
--- linux-2.6.9-rc1-mm1/drivers/usb/serial/ftdi_sio.c 2004-09-08 04:51:23.000000000
+0200
+++ linux-2.6.9-rc1-mm1-ccs/drivers/usb/serial/ftdi_sio.c 2004-09-08
10:30:40.000000000 +0200
@@ -1487,17 +1487,10 @@
}
} /* Note change no line if hupcl is off */
- /* shutdown our bulk read */
- if (port->read_urb) {
- if (usb_unlink_urb (port->read_urb) < 0) {
- /* Generally, this isn't an error. If the previous
- read bulk callback occurred (or is about to occur)
- while the port was being closed or was throtted
- (and is still throttled), the read urb will not
- have been submitted. */
- dbg("%s - failed to unlink read urb (generally not an error)",
__FUNCTION__);
- }
- }
+ /* shutdown our bulk read - synchronously */
+ if (port->read_urb)
+ usb_kill_urb(port->read_urb);
+
} /* ftdi_close */
----------------------------------------------------------------------
2) I don't agree with the DTR handling on ftdi_sio, I believe the driver
should perform proper transitions from BXXX->B0 baudrates and vica versa.
I know, that the current driver finally sets DTR properly when
baudrate!=B0, but performs unnecessary termios settings.
A long time ago, I proposed a patch, but it hasn't been accepted. In
fact, the patch was doing exactly the same thing as many other tty
drivers do. I am attaching the old (2.4.25) patch. The section that has
been marked as 'acrobatics' uses the RELEVANT_IFLAG to determine whether
a change in termios setting occured. Other things that the patch does, is
just code cleanup, and moving some of the functionality of into separate
inlined functions. Anybody interested in this again? This second patch
won't most probably apply cleanly against the latest 2.4 kernel. I am
just attaching it to show the idea, what I was trying to do.. I will port
it to 2.6.x anyways.
--- linux-2.4.27-pre1/drivers/usb/serial/ftdi_sio.h 2004-04-22 23:16:51.000000000
+0200
+++ linux-2.4.25-jc-ftdi1/drivers/usb/serial/ftdi_sio.h 2004-03-10 09:55:13.000000000
+0100
@@ -165,6 +153,11 @@
#define PROTEGO_SPECIAL_3 0xFC72 /* special/unknown device */
#define PROTEGO_SPECIAL_4 0xFC73 /* special/unknown device */
+/* CCS Inc. ICDU/ICDU40 product ID - the FT232BM is used in an in-circuit-debugger */
+/* unit for PIC16's/PIC18's */
+#define FTDI_CCSICDU20_0_PID 0xF9D0
+#define FTDI_CCSICDU40_1_PID 0xF9D1
+
/* Commands */
#define FTDI_SIO_RESET 0 /* Reset the port */
#define FTDI_SIO_MODEM_CTRL 1 /* Set the modem control register */
--- linux-2.4.27-pre1/drivers/usb/serial/ftdi_sio.c 2004-04-22 23:16:51.000000000
+0200
+++ linux-2.4.25-jc-ftdi1/drivers/usb/serial/ftdi_sio.c 2004-04-22 23:25:33.000000000
+0200
@@ -16,6 +16,15 @@
*
* See http://ftdi-usb-sio.sourceforge.net for upto date testing info
* and extra documentation
+ *
+ * (07/Mar/2004) Jan Capek
+ * Added PID's for ICD-U20/ICD-U40 - incircuit PIC debuggers from CCS Inc.
+ * Extended ftdi_set_termios(), so that it handles changes of the
+ * termios setting with regards to old_termios. The code has been cleaned up
+ * and parts of the functionality have been moved to helper functions:
+ * - ftdi_set_cs_stopb_parb() - sets up character size, stop bits, parity bits
+ * - ftdi_handle_B0_transitions() - this handles transitions back and forth
+ * between to B0 and other baudrates (DTR is changed accordingly)
*
* (09/Feb/2004) Ian Abbott
* Changed full name of USB-UIRT device to avoid "/" character.
@@ -250,7 +254,7 @@
/*
* Version Information
*/
-#define DRIVER_VERSION "v1.3.5"
+#define DRIVER_VERSION "v1.3.6"
#define DRIVER_AUTHOR "Greg Kroah-Hartman <[EMAIL PROTECTED]>, Bill Ryder <[EMAIL
PROTECTED]>, Kuba Ober <[EMAIL PROTECTED]>"
#define DRIVER_DESC "USB FTDI Serial Converters Driver"
@@ -1011,6 +1006,120 @@
} /* set_serial_info */
+
+/*
+ * Sets character size(CS), stopbits(STOPB) and parity bits(PARB) for
+ * the usb_serial specified.
+ *
+ * @serial: FTDI device, where the settings are to be performed
+ * @cflag: new control flags from the termios settings
+ *
+ * Returns: wValue, that was written to the device via usb control message.
+ * It is used by the break command.
+ *
+ */
+static inline __u16 ftdi_set_cs_stopb_parb(struct usb_serial *serial, unsigned int
cflag)
+{
+ __u16 urb_value = 0;
+ char buf[1];
+
+ dbg("%s", __FUNCTION__);
+ urb_value |= (cflag & CSTOPB ? FTDI_SIO_SET_DATA_STOP_BITS_2 :
+ FTDI_SIO_SET_DATA_STOP_BITS_1);
+ urb_value |= (cflag & PARENB ?
+ (cflag & PARODD ? FTDI_SIO_SET_DATA_PARITY_ODD :
+ FTDI_SIO_SET_DATA_PARITY_EVEN) :
+ FTDI_SIO_SET_DATA_PARITY_NONE);
+ if (cflag & CSIZE) {
+ switch (cflag & CSIZE) {
+ case CS5: urb_value |= 5; dbg("Setting CS5"); break;
+ case CS6: urb_value |= 6; dbg("Setting CS6"); break;
+ case CS7: urb_value |= 7; dbg("Setting CS7"); break;
+ case CS8: urb_value |= 8; dbg("Setting CS8"); break;
+ default:
+ err("CSIZE was set but not CS5-CS8");
+ }
+ }
+ if (usb_control_msg(serial->dev, usb_sndctrlpipe(serial->dev, 0),
+ FTDI_SIO_SET_DATA_REQUEST,
+ FTDI_SIO_SET_DATA_REQUEST_TYPE,
+ urb_value , 0,
+ buf, 0, 100) < 0) {
+ err("%s FAILED to set databits/stopbits/parity", __FUNCTION__);
+ }
+
+ return urb_value;
+}
+
+/*
+ * This is to make the ftdi_set_termios() function more readable as it originally
+ * didn't handle transitions from/to B0 correctly. The problem was in the fact
+ * that once the baudrate was dropped to B0, the DTR/RTS was pulled low however
+ * on a transition from B0 to Bxxx, the DTR would stay low all the time.
+ * This was semantically incorrect as some terminal programs (e.g. minicom) uses
+ * B0 to drop DTR and reset the modem and expect it to go high when any other
+ * baudrate is set(see serial.c).
+ *
+ * Transitions in baurate are handled as follows:
+ * - Bxxxx -> B0 - DTR and RTS is dropped and disables the HW flowcontrol
+ * - B0 -> Bxxxx - DTR is pulled high. RTS is pulled high only if HW
+ * flow control is not enabled.
+ *
+ * @port: pointer to the usb_serial_port structure as the port is referenced
+ * when handling dtr/rts changes
+ * @cflag: new termios control flags
+ * @old_termios: pointer to the old termios settings (this might be NULL)
+ * @old_baudrate: original baudrate, that we perform the transition from
+ * this is valid only if old_termios != NULL
+ *
+ * Returns: nothing
+ */
+static inline void ftdi_handle_B0_transitions(struct usb_serial_port *port,
+ unsigned int cflag,
+ struct termios *old_termios,
+ unsigned int old_baudrate)
+{
+ struct usb_serial *serial = port->serial;
+
+ unsigned int new_baudrate = cflag & CBAUD;
+ char buf[1];
+
+ /* Handle transition to B0 */
+ if (((old_baudrate != B0) || !old_termios) && (new_baudrate == B0)) {
+ dbg("%s: Bxxx->B0 transition", __FUNCTION__);
+ /* Disable flow control */
+ if (usb_control_msg(serial->dev, usb_sndctrlpipe(serial->dev, 0),
+ FTDI_SIO_SET_FLOW_CTRL_REQUEST,
+ FTDI_SIO_SET_FLOW_CTRL_REQUEST_TYPE,
+ 0, FTDI_SIO_DISABLE_FLOW_CTRL,
+ buf, 0, WDR_TIMEOUT) < 0) {
+ err("%s error from disable flowcontrol urb", __FUNCTION__);
+ }
+
+ /* Drop RTS and DTR */
+ if (set_dtr(port, LOW) < 0) {
+ err("%s Error from DTR LOW urb", __FUNCTION__);
+ }
+ if (set_rts(port, LOW) < 0) {
+ err("%s Error from RTS LOW urb", __FUNCTION__);
+ }
+
+ }
+ /* Handle transition from B0 */
+ else if (((old_baudrate == B0) || !old_termios) && (new_baudrate != B0)) {
+ dbg("%s: B0->Bxxx transition", __FUNCTION__);
+ /* Set DTR high*/
+ if (set_dtr(port, HIGH) < 0){
+ err("%s Error from DTR LOW urb", __FUNCTION__);
+ }
+ if (!(cflag & CRTSCTS))
+ if (set_rts(port, HIGH) < 0) {
+ err("%s Error from RTS LOW urb", __FUNCTION__);
+ }
+ }
+
+}
+
/*
* ***************************************************************************
* FTDI driver specific functions
@@ -1752,16 +1860,32 @@
}
+/* used when checking if following input flags have changed */
+#define RELEVANT_IFLAG(iflag) (iflag & (IGNBRK|BRKINT|IGNPAR|PARMRK|INPCK))
-/* old_termios contains the original termios settings and tty->termios contains
- * the new setting to be used
+/*
+ * Actions taken:
+ * - checks if there any forced setting to be done (force_baud and force_rtscts)
+ * - in order to implement the same semantics as a regular serial driver, the
+ * old_termios settings are taken into account
+ * - when no change in settings is observed, function returns
+ * - function now handles correctly transitions back and forth between B0 and
+ * other baudrates (via ftdi_handle_B0_transitions())
+ * - other termios settings are performed (character size, stop bits -> using
+ * helper function ftdi_set_cs_stopb_parb(), parity bits, flow control)
+ *
+ * @port: pointer to the usb_serial_port structure as the port is referenced
+ * when handling various settings
+ * @old_termios: pointer to the old termios settings (this might be NULL too),
+ * tty->termios contains the new setting to be used
+ *
* WARNING: set_termios calls this with old_termios in kernel space
*/
-
static void ftdi_set_termios (struct usb_serial_port *port, struct termios
*old_termios)
{ /* ftdi_termios */
struct usb_serial *serial = port->serial;
unsigned int cflag;
+ unsigned int old_baudrate = B0; /* baudrate value from old_termios for B0
transition detection */
struct ftdi_private *priv = (struct ftdi_private *)port->private;
__u16 urb_value; /* will hold the new flags */
char buf[1]; /* Perhaps I should dynamically alloc this? */
@@ -1788,64 +1912,33 @@
cflag = port->tty->termios->c_cflag;
- /* FIXME -For this cut I don't care if the line is really changing or
- not - so just do the change regardless - should be able to
- compare old_termios and tty->termios */
+ /* Check if any change in termios settings occured*/
+ if (old_termios) {
+ if ((cflag == old_termios->c_cflag) &&
+ (RELEVANT_IFLAG(port->tty->termios->c_iflag) ==
+ RELEVANT_IFLAG(old_termios->c_iflag))) {
+ dbg("%s - nothing to change", __FUNCTION__);
+ return;
+ }
+ old_baudrate = old_termios->c_cflag & CBAUD;
+ }
+
+
/* NOTE These routines can get interrupted by
ftdi_sio_read_bulk_callback - need to examine what this
means - don't see any problems yet */
/* Set number of data bits, parity, stop bits */
-
- urb_value = 0;
- urb_value |= (cflag & CSTOPB ? FTDI_SIO_SET_DATA_STOP_BITS_2 :
- FTDI_SIO_SET_DATA_STOP_BITS_1);
- urb_value |= (cflag & PARENB ?
- (cflag & PARODD ? FTDI_SIO_SET_DATA_PARITY_ODD :
- FTDI_SIO_SET_DATA_PARITY_EVEN) :
- FTDI_SIO_SET_DATA_PARITY_NONE);
- if (cflag & CSIZE) {
- switch (cflag & CSIZE) {
- case CS5: urb_value |= 5; dbg("Setting CS5"); break;
- case CS6: urb_value |= 6; dbg("Setting CS6"); break;
- case CS7: urb_value |= 7; dbg("Setting CS7"); break;
- case CS8: urb_value |= 8; dbg("Setting CS8"); break;
- default:
- err("CSIZE was set but not CS5-CS8");
- }
- }
-
/* This is needed by the break command since it uses the same command - but is
* or'ed with this value */
- priv->last_set_data_urb_value = urb_value;
+ priv->last_set_data_urb_value = ftdi_set_cs_stopb_parb(serial, cflag);
- if (usb_control_msg(serial->dev, usb_sndctrlpipe(serial->dev, 0),
- FTDI_SIO_SET_DATA_REQUEST,
- FTDI_SIO_SET_DATA_REQUEST_TYPE,
- urb_value , 0,
- buf, 0, 100) < 0) {
- err("%s FAILED to set databits/stopbits/parity", __FUNCTION__);
- }
+ /* B0->Bxxxx and Bxxxx->B0 baudrate transitions need special handling so that
+ DTR and RTS reflect the actual state */
+ ftdi_handle_B0_transitions(port, cflag, old_termios, old_baudrate);
/* Now do the baudrate */
- if ((cflag & CBAUD) == B0 ) {
- /* Disable flow control */
- if (usb_control_msg(serial->dev, usb_sndctrlpipe(serial->dev, 0),
- FTDI_SIO_SET_FLOW_CTRL_REQUEST,
- FTDI_SIO_SET_FLOW_CTRL_REQUEST_TYPE,
- 0, 0,
- buf, 0, WDR_TIMEOUT) < 0) {
- err("%s error from disable flowcontrol urb", __FUNCTION__);
- }
- /* Drop RTS and DTR */
- if (set_dtr(port, LOW) < 0){
- err("%s Error from DTR LOW urb", __FUNCTION__);
- }
- if (set_rts(port, LOW) < 0){
- err("%s Error from RTS LOW urb", __FUNCTION__);
- }
-
- } else {
+ if ((cflag & CBAUD) != B0 ) {
/* set the baudrate determined before */
if (change_speed(port)) {
err("%s urb failed to set baurdrate", __FUNCTION__);
----------------------------------------------------------------------------
Thanks,
Jan
Jan Capek - CCS Inc.
Firmware developer
-------------------------------------------------------
This SF.Net email is sponsored by BEA Weblogic Workshop
FREE Java Enterprise J2EE developer tools!
Get your free copy of BEA WebLogic Workshop 8.1 today.
http://ads.osdn.com/?ad_id=5047&alloc_id=10808&op=click
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
