From: "Vitaly V. Ch" <vitaly.v.ch-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
To: USB list <linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Greg KH <greg-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>,
bryder-sJ/iWh9BUns@public.gmane.org,
kuba-6OO/Rnbgvk2D7HvB/+pQRQ@public.gmane.org
Subject: Review and minor fixes for ftdi_sio.h
Date: Fri, 5 Feb 2010 11:54:56 +0200 [thread overview]
Message-ID: <6efe08af1002050154k8ae2a9evff2971afce407b52@mail.gmail.com> (raw)
[-- Attachment #1: Type: text/plain, Size: 2003 bytes --]
Hi all!
I'm find then ftdi_sio.c in 2.4.37.8 kernel have next bug: if I have
three ftdi-232 simultaneously connected to middle-loaded host,
application which use one of them will hung in driver, usually during
few hour.
At now i try to revise that driver and also I review driver in 2.6.32
and do few minor fixes + found out possible buggy place.
****************************************
1) function ftdi_232am_baud_base_to_divisor() in 2.6.32 have next view:
static unsigned short int ftdi_232am_baud_base_to_divisor(int baud, int base)
{
unsigned short int divisor;
/* divisor shifted 3 bits to the left */
int divisor3 = base / 2 / baud;
if ((divisor3 & 0x7) == 7)
divisor3++; /* round x.7/8 up to x+1 */
divisor = divisor3 >> 3;
divisor3 &= 0x7;
if (divisor3 == 1)
divisor |= 0xc000; /* 0.125 */
else if (divisor3 >= 4)
divisor |= 0x4000; /* 0.5 */
else if (divisor3 != 0)
divisor |= 0x8000; /* 0.25 */
/// @warning possible at this place we have bug
else if (divisor == 1)
divisor = 0; /* special case for maximum baud rate */
return divisor;
}
but POSSIBLE may be need:
static unsigned short int ftdi_232am_baud_base_to_divisor(int baud, int base)
{
unsigned short int divisor;
/* divisor shifted 3 bits to the left */
int divisor3 = base / 2 / baud;
if ((divisor3 & 0x7) == 7)
divisor3++; /* round x.7/8 up to x+1 */
divisor = divisor3 >> 3;
divisor3 &= 0x7;
if (divisor3 == 1)
divisor |= 0xc000; /* 0.125 */
else if (divisor3 >= 4)
divisor |= 0x4000; /* 0.5 */
else if (divisor3 != 0)
divisor |= 0x8000; /* 0.25 */
if (divisor == 1)
divisor = 0; /* special case for maximum baud rate */
return divisor;
}
Can anyone guide me?
*********************************************************
2) In the attached patch I do few small fixes. About half of them must
be merged into kernel trunk
\\wbr Vitaly Chernookiy
[-- Attachment #2: ftdi_sio.c.diff --]
[-- Type: application/octet-stream, Size: 8663 bytes --]
=== modified file 'ftdi_sio.c'
--- ftdi_sio.c 2010-02-03 16:20:35 +0000
+++ ftdi_sio.c 2010-02-05 09:08:57 +0000
@@ -100,14 +100,14 @@
void (*port_probe)(struct ftdi_private *);
};
-static int ftdi_jtag_probe(struct usb_serial *serial);
+static int ftdi_olimex_jtag_probe(struct usb_serial *serial);
static int ftdi_mtxorb_hack_setup(struct usb_serial *serial);
static int ftdi_NDI_device_setup(struct usb_serial *serial);
static void ftdi_USB_UIRT_setup(struct ftdi_private *priv);
static void ftdi_HE_TIRA1_setup(struct ftdi_private *priv);
-static struct ftdi_sio_quirk ftdi_jtag_quirk = {
- .probe = ftdi_jtag_probe,
+static struct ftdi_sio_quirk ftdi_olimex_jtag_quirk = {
+ .probe = ftdi_olimex_jtag_probe,
};
static struct ftdi_sio_quirk ftdi_mtxorb_hack_quirk = {
@@ -144,7 +144,8 @@
-static struct usb_device_id id_table_combined [] = {
+static
+struct usb_device_id id_table_combined [] = {
{ USB_DEVICE(FTDI_VID, FTDI_AMC232_PID) },
{ USB_DEVICE(FTDI_VID, FTDI_CANUSB_PID) },
{ USB_DEVICE(FTDI_VID, FTDI_CANDAPTER_PID) },
@@ -682,17 +683,17 @@
{ USB_DEVICE(FTDI_VID, FTDI_ELSTER_UNICOM_PID) },
{ USB_DEVICE(FTDI_VID, FTDI_PROPOX_JTAGCABLEII_PID) },
{ USB_DEVICE(OLIMEX_VID, OLIMEX_ARM_USB_OCD_PID),
- .driver_info = (kernel_ulong_t)&ftdi_jtag_quirk },
+ .driver_info = (kernel_ulong_t)&ftdi_olimex_jtag_quirk },
{ USB_DEVICE(FIC_VID, FIC_NEO1973_DEBUG_PID),
- .driver_info = (kernel_ulong_t)&ftdi_jtag_quirk },
+ .driver_info = (kernel_ulong_t)&ftdi_olimex_jtag_quirk },
{ USB_DEVICE(FTDI_VID, FTDI_OOCDLINK_PID),
- .driver_info = (kernel_ulong_t)&ftdi_jtag_quirk },
+ .driver_info = (kernel_ulong_t)&ftdi_olimex_jtag_quirk },
{ USB_DEVICE(FTDI_VID, LMI_LM3S_DEVEL_BOARD_PID),
- .driver_info = (kernel_ulong_t)&ftdi_jtag_quirk },
+ .driver_info = (kernel_ulong_t)&ftdi_olimex_jtag_quirk },
{ USB_DEVICE(FTDI_VID, LMI_LM3S_EVAL_BOARD_PID),
- .driver_info = (kernel_ulong_t)&ftdi_jtag_quirk },
+ .driver_info = (kernel_ulong_t)&ftdi_olimex_jtag_quirk },
{ USB_DEVICE(FTDI_VID, FTDI_TURTELIZER_PID),
- .driver_info = (kernel_ulong_t)&ftdi_jtag_quirk },
+ .driver_info = (kernel_ulong_t)&ftdi_olimex_jtag_quirk },
{ USB_DEVICE(RATOC_VENDOR_ID, RATOC_PRODUCT_ID_USB60F) },
{ USB_DEVICE(FTDI_VID, FTDI_REU_TINY_PID) },
{ USB_DEVICE(PAPOUCH_VID, PAPOUCH_QUIDO4x4_PID) },
@@ -704,17 +705,17 @@
{ USB_DEVICE(DE_VID, STB_PID) },
{ USB_DEVICE(DE_VID, WHT_PID) },
{ USB_DEVICE(ADI_VID, ADI_GNICE_PID),
- .driver_info = (kernel_ulong_t)&ftdi_jtag_quirk },
+ .driver_info = (kernel_ulong_t)&ftdi_olimex_jtag_quirk },
{ USB_DEVICE(ADI_VID, ADI_GNICEPLUS_PID),
- .driver_info = (kernel_ulong_t)&ftdi_jtag_quirk },
+ .driver_info = (kernel_ulong_t)&ftdi_olimex_jtag_quirk },
{ USB_DEVICE(JETI_VID, JETI_SPC1201_PID) },
{ USB_DEVICE(MARVELL_VID, MARVELL_SHEEVAPLUG_PID),
- .driver_info = (kernel_ulong_t)&ftdi_jtag_quirk },
+ .driver_info = (kernel_ulong_t)&ftdi_olimex_jtag_quirk },
{ USB_DEVICE(LARSENBRUSGAARD_VID, LB_ALTITRACK_PID) },
{ USB_DEVICE(GN_OTOMETRICS_VID, AURICAL_USB_PID) },
{ USB_DEVICE(BAYER_VID, BAYER_CONTOUR_CABLE_PID) },
{ USB_DEVICE(FTDI_VID, MARVELL_OPENRD_PID),
- .driver_info = (kernel_ulong_t)&ftdi_jtag_quirk },
+ .driver_info = (kernel_ulong_t)&ftdi_olimex_jtag_quirk },
{ USB_DEVICE(FTDI_VID, HAMEG_HO820_PID) },
{ USB_DEVICE(FTDI_VID, HAMEG_HO870_PID) },
{ }, /* Optional parameter entry */
@@ -842,11 +843,12 @@
divisor = divisor3 >> 3;
divisor3 &= 0x7;
if (divisor3 == 1)
- divisor |= 0xc000;
+ divisor |= 0xc000; /* 0.125 */
else if (divisor3 >= 4)
- divisor |= 0x4000;
+ divisor |= 0x4000; /* 0.5 */
else if (divisor3 != 0)
- divisor |= 0x8000;
+ divisor |= 0x8000; /* 0.25 */
+ /// @warning possible at this palce we have bug
else if (divisor == 1)
divisor = 0; /* special case for maximum baud rate */
return divisor;
@@ -867,9 +869,9 @@
divisor |= (__u32)divfrac[divisor3 & 0x7] << 14;
/* Deal with special cases for highest baud rates. */
if (divisor == 1)
- divisor = 0;
+ divisor = 0; /* 1.0 */
else if (divisor == 0x4001)
- divisor = 1;
+ divisor = 1; /* 1.5 */
return divisor;
}
@@ -891,9 +893,9 @@
divisor |= (__u32)divfrac[divisor3 & 0x7] << 14;
/* Deal with special cases for highest baud rates. */
if (divisor == 1)
- divisor = 0;
+ divisor = 0; /* 1.0 */
else if (divisor == 0x4001)
- divisor = 1;
+ divisor = 1; /* 1.5 */
/*
* Set this bit to turn off a divide by 2.5 on baud rate generator
* This enables baud rates up to 12Mbaud but cannot reach below 1200
@@ -947,7 +949,7 @@
kfree(buf);
if (rv < 0) {
- dbg("%s Error from MODEM_CTRL urb: DTR %s, RTS %s",
+ err("%s Error from MODEM_CTRL urb: DTR %s, RTS %s",
__FUNCTION__,
(set & TIOCM_DTR) ? "HIGH" :
(clear & TIOCM_DTR) ? "LOW" : "unchanged",
@@ -975,6 +977,8 @@
int baud;
/*
+ * Obtaining the actual baud rate is a little tricky since unix traditionally
+ * somehow ignored the possibility to set non-standard baud rates.
* The logic involved in setting the baudrate can be cleanly split into
* 3 steps.
* 1. Standard baud rates are set in tty->termios->c_cflag
@@ -1270,7 +1274,7 @@
/* Determine type of FTDI chip based on USB config and descriptor. */
static void ftdi_determine_type(struct usb_serial_port *port)
-{
+{ /* ftdi_determine_type */
struct ftdi_private *priv = usb_get_serial_port_data(port);
struct usb_serial *serial = port->serial;
struct usb_device *udev = serial->dev;
@@ -1335,7 +1339,7 @@
priv->chip_type = FT232RL;
}
dev_info(&udev->dev, "Detected %s\n", ftdi_chip_name[priv->chip_type]);
-}
+} /* ftdi_determine_type */
/* Determine the maximum packet size for the device. This depends on the chip
@@ -1633,7 +1637,7 @@
* Neo1973 Debug Board is reserved for JTAG interface and can be accessed from
* userspace using openocd.
*/
-static int ftdi_jtag_probe(struct usb_serial *serial)
+static int ftdi_olimex_jtag_probe(struct usb_serial *serial)
{
struct usb_device *udev = serial->dev;
struct usb_interface *interface = serial->interface;
@@ -1682,6 +1686,10 @@
remove_sysfs_attrs(port);
+ /* all open ports are closed at this point
+ * (by usbserial.c:__serial_close, which calls ftdi_close)
+ */
+
kref_put(&priv->kref, ftdi_sio_priv_release);
return 0;
@@ -1992,10 +2000,10 @@
}
spin_unlock_irqrestore(&priv->tx_lock, flags);
return room;
-}
+} /* ftdi_write_room */
static int ftdi_chars_in_buffer(struct tty_struct *tty)
-{
+{ /* ftdi_chars_in_buffer */
struct usb_serial_port *port = tty->driver_data;
struct ftdi_private *priv = usb_get_serial_port_data(port);
int buffered;
@@ -2012,7 +2020,7 @@
buffered = 0;
}
return buffered;
-}
+} /* ftdi_chars_in_buffer */
static int ftdi_process_packet(struct tty_struct *tty,
struct usb_serial_port *port, struct ftdi_private *priv,
@@ -2081,7 +2089,7 @@
}
static void ftdi_process_read(struct usb_serial_port *port)
-{
+{ /* ftdi_process_read */
struct urb *urb = port->read_urb;
struct tty_struct *tty;
struct ftdi_private *priv = usb_get_serial_port_data(port);
@@ -2102,10 +2110,10 @@
if (count)
tty_flip_buffer_push(tty);
tty_kref_put(tty);
-}
+} /* ftdi_process_read */
static void ftdi_read_bulk_callback(struct urb *urb)
-{
+{ /* ftdi_read_bulk_callback */
struct usb_serial_port *port = urb->context;
unsigned long flags;
@@ -2128,7 +2136,7 @@
ftdi_submit_read_urb(port, GFP_ATOMIC);
} else
spin_unlock_irqrestore(&port->lock, flags);
-}
+} /* ftdi_read_bulk_callback */
static void ftdi_break_ctl(struct tty_struct *tty, int break_state)
{
@@ -2329,7 +2337,7 @@
}
return;
-}
+} /* ftdi_termios */
static int ftdi_tiocmget(struct tty_struct *tty, struct file *file)
{
@@ -2383,7 +2391,6 @@
static int ftdi_tiocmset(struct tty_struct *tty, struct file *file,
unsigned int set, unsigned int clear)
{
- struct usb_serial_port *port = tty->driver_data;
dbg("%s TIOCMSET", __FUNCTION__);
return update_mctrl(port, set, clear);
}
@@ -2441,7 +2448,7 @@
}
/*
* Otherwise caller can't care less about what
- * happened,and so we continue to wait for more
+ * happened, and so we continue to wait for more
* events.
*/
}
@@ -2450,12 +2457,13 @@
default:
break;
}
+
/* This is not necessarily an error - turns out the higher layers
* will do some ioctls themselves (see comment above)
*/
dbg("%s arg not supported - it was 0x%04x - check /usr/include/asm/ioctls.h", __FUNCTION__, cmd);
return -ENOIOCTLCMD;
-}
+} /* ftdi_ioctl */
static void ftdi_throttle(struct tty_struct *tty)
{
next reply other threads:[~2010-02-05 9:54 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-02-05 9:54 Vitaly V. Ch [this message]
2010-02-05 16:38 ` Review and minor fixes for ftdi_sio.h Greg KH
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=6efe08af1002050154k8ae2a9evff2971afce407b52@mail.gmail.com \
--to=vitaly.v.ch-re5jqeeqqe8avxtiumwx3w@public.gmane.org \
--cc=bryder-sJ/iWh9BUns@public.gmane.org \
--cc=greg-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org \
--cc=kuba-6OO/Rnbgvk2D7HvB/+pQRQ@public.gmane.org \
--cc=linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox