This series patch V5 is improvement from V4 as following:
1. transform all function not to use private data as parameter, using
usb_serial_port instead.
2. Some init step we extract it from set_term() to f81232_port_init()
and run it when open port only.
3. We'll force re-read msr in tiocmget() because the IIR with MSR change
maybe delay received.
4. process_read_urb() add process of Break/FrameError/ParityError.
5. clarify a lot of code about Johan suggested.
Peter Hung (8):
USB: f81232: Rename private struct member name
USB: f81232: implement read IIR/MSR with endpoint
USB: f81232: implement RX bulk-in ep
USB: f81232: implement set_termios
USB: f81232: implement MCR/MSR function
USB: f81232: clarify f81232_ioctl()
USB: f81232: fix error in f81232_carrier_raised()
USB: f81232: modify/add author
drivers/usb/serial/f81232.c | 471 +++++++++++++++++++++++++++++++++++---------
1 file changed, 375 insertions(+), 96 deletions(-)
--
1.9.1
Change private struct member name from line_status to modem_status.
It will store MSR for some functions used
Signed-off-by: Peter Hung <[email protected]>
---
drivers/usb/serial/f81232.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
index c5dc233..669a2f2 100644
--- a/drivers/usb/serial/f81232.c
+++ b/drivers/usb/serial/f81232.c
@@ -47,7 +47,7 @@ MODULE_DEVICE_TABLE(usb, id_table);
struct f81232_private {
spinlock_t lock;
u8 line_control;
- u8 line_status;
+ u8 modem_status;
};
static void f81232_update_line_status(struct usb_serial_port *port,
@@ -113,8 +113,8 @@ static void f81232_process_read_urb(struct urb *urb)
/* update line status */
spin_lock_irqsave(&priv->lock, flags);
- line_status = priv->line_status;
- priv->line_status &= ~UART_STATE_TRANSIENT_MASK;
+ line_status = priv->modem_status;
+ priv->modem_status &= ~UART_STATE_TRANSIENT_MASK;
spin_unlock_irqrestore(&priv->lock, flags);
if (!urb->actual_length)
@@ -241,7 +241,7 @@ static void f81232_dtr_rts(struct usb_serial_port *port, int on)
static int f81232_carrier_raised(struct usb_serial_port *port)
{
struct f81232_private *priv = usb_get_serial_port_data(port);
- if (priv->line_status & UART_DCD)
+ if (priv->modem_status & UART_DCD)
return 1;
return 0;
}
--
1.9.1
The interrupt Endpoint will report current IIR. If we got IIR with MSR Changed
, We will do read MSR with interrupt_work worker to do f81232_read_msr() func.
Signed-off-by: Peter Hung <[email protected]>
---
drivers/usb/serial/f81232.c | 109 ++++++++++++++++++++++++++++++++++++++++----
1 file changed, 100 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
index 669a2f2..ec4609d 100644
--- a/drivers/usb/serial/f81232.c
+++ b/drivers/usb/serial/f81232.c
@@ -23,6 +23,7 @@
#include <linux/uaccess.h>
#include <linux/usb.h>
#include <linux/usb/serial.h>
+#include <linux/serial_reg.h>
static const struct usb_device_id id_table[] = {
{ USB_DEVICE(0x1934, 0x0706) },
@@ -30,6 +31,13 @@ static const struct usb_device_id id_table[] = {
};
MODULE_DEVICE_TABLE(usb, id_table);
+/* USB Control EP parameter */
+#define F81232_REGISTER_REQUEST 0xA0
+#define F81232_GET_REGISTER 0xc0
+
+#define SERIAL_BASE_ADDRESS 0x0120
+#define MODEM_STATUS_REGISTER (0x06 + SERIAL_BASE_ADDRESS)
+
#define CONTROL_DTR 0x01
#define CONTROL_RTS 0x02
@@ -48,19 +56,92 @@ struct f81232_private {
spinlock_t lock;
u8 line_control;
u8 modem_status;
+
+ struct work_struct interrupt_work;
+ struct usb_serial_port *port;
};
-static void f81232_update_line_status(struct usb_serial_port *port,
+static int f81232_get_register(struct usb_serial_port *port,
+ u16 reg, u8 *data)
+{
+ int status;
+ struct usb_device *dev = port->serial->dev;
+
+ status = usb_control_msg(dev,
+ usb_rcvctrlpipe(dev, 0),
+ F81232_REGISTER_REQUEST,
+ F81232_GET_REGISTER,
+ reg,
+ 0,
+ data,
+ sizeof(*data),
+ USB_CTRL_GET_TIMEOUT);
+ if (status < 0)
+ dev_err(&port->dev, "%s status: %d\n", __func__, status);
+
+ return status;
+}
+
+static void f81232_read_msr(struct usb_serial_port *port)
+{
+ int status;
+ unsigned long flags;
+ u8 current_msr;
+ struct tty_struct *tty;
+ struct f81232_private *priv = usb_get_serial_port_data(port);
+
+ status = f81232_get_register(port, MODEM_STATUS_REGISTER,
+ ¤t_msr);
+ if (status < 0) {
+ /* Retain the error even reported in f81232_get_register()
+ to make debug easily :D */
+ dev_err(&port->dev, "%s fail, status: %d\n", __func__, status);
+ return;
+ }
+
+ if (!(current_msr & UART_MSR_ANY_DELTA))
+ return;
+
+ tty = tty_port_tty_get(&port->port);
+ if (tty) {
+ if (current_msr & UART_MSR_DDCD) {
+ usb_serial_handle_dcd_change(port, tty,
+ current_msr & UART_MSR_DCD);
+ }
+
+ tty_kref_put(tty);
+ }
+
+ spin_lock_irqsave(&priv->lock, flags);
+ priv->modem_status = current_msr;
+ spin_unlock_irqrestore(&priv->lock, flags);
+
+ wake_up_interruptible(&port->port.delta_msr_wait);
+}
+
+static void f81232_update_modem_status(struct usb_serial_port *port,
unsigned char *data,
unsigned int actual_length)
{
- /*
- * FIXME: Update port->icount, and call
- *
- * wake_up_interruptible(&port->port.delta_msr_wait);
- *
- * on MSR changes.
- */
+ struct f81232_private *priv = usb_get_serial_port_data(port);
+
+ if (!actual_length)
+ return;
+
+ switch (data[0] & 0x07) {
+ case 0x00: /* msr change */
+ dev_dbg(&port->dev, "IIR: MSR Change: %x\n", data[0]);
+ schedule_work(&priv->interrupt_work);
+ break;
+ case 0x02: /* tx-empty */
+ break;
+ case 0x04: /* rx data available */
+ break;
+ case 0x06: /* lsr change */
+ /* we can forget it. the LSR will read from bulk-in */
+ dev_dbg(&port->dev, "IIR: LSR Change: %x\n", data[0]);
+ break;
+ }
}
static void f81232_read_int_callback(struct urb *urb)
@@ -91,7 +172,7 @@ static void f81232_read_int_callback(struct urb *urb)
usb_serial_debug_data(&port->dev, __func__,
urb->actual_length, urb->transfer_buffer);
- f81232_update_line_status(port, data, actual_length);
+ f81232_update_modem_status(port, data, actual_length);
exit:
retval = usb_submit_urb(urb, GFP_ATOMIC);
@@ -270,6 +351,14 @@ static int f81232_ioctl(struct tty_struct *tty,
return -ENOIOCTLCMD;
}
+static void f81232_interrupt_work(struct work_struct *work)
+{
+ struct f81232_private *priv =
+ container_of(work, struct f81232_private, interrupt_work);
+
+ f81232_read_msr(priv->port);
+}
+
static int f81232_port_probe(struct usb_serial_port *port)
{
struct f81232_private *priv;
@@ -279,10 +368,12 @@ static int f81232_port_probe(struct usb_serial_port *port)
return -ENOMEM;
spin_lock_init(&priv->lock);
+ INIT_WORK(&priv->interrupt_work, f81232_interrupt_work);
usb_set_serial_port_data(port, priv);
port->port.drain_delay = 256;
+ priv->port = port;
return 0;
}
--
1.9.1
The F81232 bulk-in is RX data + LSR channel, data format is
[LSR+Data][LSR+Data]..... , We had reimplemented in this patch.
Signed-off-by: Peter Hung <[email protected]>
---
drivers/usb/serial/f81232.c | 68 +++++++++++++++++++++++----------------------
1 file changed, 35 insertions(+), 33 deletions(-)
diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
index ec4609d..9ea498a 100644
--- a/drivers/usb/serial/f81232.c
+++ b/drivers/usb/serial/f81232.c
@@ -185,44 +185,46 @@ exit:
static void f81232_process_read_urb(struct urb *urb)
{
struct usb_serial_port *port = urb->context;
- struct f81232_private *priv = usb_get_serial_port_data(port);
unsigned char *data = urb->transfer_buffer;
- char tty_flag = TTY_NORMAL;
- unsigned long flags;
- u8 line_status;
+ char tty_flag;
int i;
- /* update line status */
- spin_lock_irqsave(&priv->lock, flags);
- line_status = priv->modem_status;
- priv->modem_status &= ~UART_STATE_TRANSIENT_MASK;
- spin_unlock_irqrestore(&priv->lock, flags);
-
- if (!urb->actual_length)
+ if (urb->actual_length < 2)
return;
- /* break takes precedence over parity, */
- /* which takes precedence over framing errors */
- if (line_status & UART_BREAK_ERROR)
- tty_flag = TTY_BREAK;
- else if (line_status & UART_PARITY_ERROR)
- tty_flag = TTY_PARITY;
- else if (line_status & UART_FRAME_ERROR)
- tty_flag = TTY_FRAME;
- dev_dbg(&port->dev, "%s - tty_flag = %d\n", __func__, tty_flag);
-
- /* overrun is special, not associated with a char */
- if (line_status & UART_OVERRUN_ERROR)
- tty_insert_flip_char(&port->port, 0, TTY_OVERRUN);
-
- if (port->port.console && port->sysrq) {
- for (i = 0; i < urb->actual_length; ++i)
- if (!usb_serial_handle_sysrq_char(port, data[i]))
- tty_insert_flip_char(&port->port, data[i],
- tty_flag);
- } else {
- tty_insert_flip_string_fixed_flag(&port->port, data, tty_flag,
- urb->actual_length);
+ /* bulk-in data: [LSR(1Byte)+DATA(1Byte)][LSR(1Byte)+DATA(1Byte)]... */
+
+ for (i = 0 ; i < urb->actual_length ; i += 2) {
+ tty_flag = TTY_NORMAL;
+
+ if (unlikely(data[i+0] & UART_LSR_BRK_ERROR_BITS)) {
+ if (data[i+0] & UART_LSR_BI) {
+ tty_flag = TTY_BREAK;
+ port->icount.brk++;
+ usb_serial_handle_break(port);
+ } else if (data[i+0] & UART_LSR_PE) {
+ tty_flag = TTY_PARITY;
+ port->icount.parity++;
+ } else if (data[i+0] & UART_LSR_FE) {
+ tty_flag = TTY_FRAME;
+ port->icount.frame++;
+ }
+
+ if (data[0] & UART_LSR_OE) {
+ port->icount.overrun++;
+ tty_insert_flip_char(&port->port, 0,
+ TTY_OVERRUN);
+ }
+ }
+
+ if (port->port.console && port->sysrq) {
+ if (!usb_serial_handle_sysrq_char(port, data[i+1]))
+ tty_insert_flip_char(&port->port, data[i+1],
+ tty_flag);
+ } else {
+ tty_insert_flip_string_fixed_flag(&port->port,
+ &data[i+1], tty_flag, 1);
+ }
}
tty_flip_buffer_push(&port->port);
--
1.9.1
The original driver had do not any h/w change in driver.
This patch implements with configure H/W for
baud/parity/word length/stop bits functional.
Some init step extract to f81232_port_init(), called once with open().
And refine baudrate setting to f81232_set_baudrate()
Signed-off-by: Peter Hung <[email protected]>
---
drivers/usb/serial/f81232.c | 145 +++++++++++++++++++++++++++++++++++++++++---
1 file changed, 138 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
index 9ea498a..06d1eb0 100644
--- a/drivers/usb/serial/f81232.c
+++ b/drivers/usb/serial/f81232.c
@@ -31,11 +31,19 @@ static const struct usb_device_id id_table[] = {
};
MODULE_DEVICE_TABLE(usb, id_table);
+/* Maximum baudrate for F81232 */
+#define F81232_MAX_BAUDRATE 115200L
+
/* USB Control EP parameter */
#define F81232_REGISTER_REQUEST 0xA0
#define F81232_GET_REGISTER 0xc0
+#define F81232_SET_REGISTER 0x40
#define SERIAL_BASE_ADDRESS 0x0120
+#define RECEIVE_BUFFER_REGISTER (0x00 + SERIAL_BASE_ADDRESS)
+#define INTERRUPT_ENABLE_REGISTER (0x01 + SERIAL_BASE_ADDRESS)
+#define FIFO_CONTROL_REGISTER (0x02 + SERIAL_BASE_ADDRESS)
+#define LINE_CONTROL_REGISTER (0x03 + SERIAL_BASE_ADDRESS)
#define MODEM_STATUS_REGISTER (0x06 + SERIAL_BASE_ADDRESS)
#define CONTROL_DTR 0x01
@@ -61,6 +69,14 @@ struct f81232_private {
struct usb_serial_port *port;
};
+static int calc_baud_divisor(u32 baudrate)
+{
+ if (!baudrate)
+ return 0;
+ else
+ return DIV_ROUND_CLOSEST(F81232_MAX_BAUDRATE, baudrate);
+}
+
static int f81232_get_register(struct usb_serial_port *port,
u16 reg, u8 *data)
{
@@ -82,6 +98,27 @@ static int f81232_get_register(struct usb_serial_port *port,
return status;
}
+static int f81232_set_register(struct usb_serial_port *port,
+ u16 reg, u8 data)
+{
+ int status;
+ struct usb_device *dev = port->serial->dev;
+
+ status = usb_control_msg(dev,
+ usb_sndctrlpipe(dev, 0),
+ F81232_REGISTER_REQUEST,
+ F81232_SET_REGISTER,
+ reg,
+ 0,
+ &data,
+ sizeof(data),
+ USB_CTRL_SET_TIMEOUT);
+ if (status < 0)
+ dev_err(&port->dev, "%s status: %d\n", __func__, status);
+
+ return status;
+}
+
static void f81232_read_msr(struct usb_serial_port *port)
{
int status;
@@ -247,18 +284,106 @@ static void f81232_break_ctl(struct tty_struct *tty, int break_state)
*/
}
+static void f81232_set_baudrate(struct usb_serial_port *port, int baudrate)
+{
+ u8 divisor;
+ int status = 0;
+
+ divisor = calc_baud_divisor(baudrate);
+
+ status = f81232_set_register(port, LINE_CONTROL_REGISTER,
+ UART_LCR_DLAB); /* DLAB */
+ if (status < 0) {
+ dev_err(&port->dev, "%s status: %d line:%d\n",
+ __func__, status, __LINE__);
+ }
+
+ status = f81232_set_register(port, RECEIVE_BUFFER_REGISTER,
+ divisor & 0x00ff); /* low */
+ if (status < 0) {
+ dev_err(&port->dev, "%s status: %d line:%d\n",
+ __func__, status, __LINE__);
+ }
+
+ status = f81232_set_register(port, INTERRUPT_ENABLE_REGISTER,
+ (divisor & 0xff00) >> 8); /* high */
+ if (status < 0) {
+ dev_err(&port->dev, "%s status: %d line:%d\n", __func__,
+ status, __LINE__);
+ }
+
+ status = f81232_set_register(port, LINE_CONTROL_REGISTER, 0x00);
+ if (status < 0) {
+ dev_err(&port->dev, "%s status: %d line:%d\n", __func__,
+ status, __LINE__);
+ }
+}
+
+static int f81232_port_init(struct usb_serial_port *port)
+{
+ u8 data;
+ int status = 0;
+
+ /* fifo on, trigger8, clear TX/RX*/
+ data = UART_FCR_TRIGGER_8 | UART_FCR_ENABLE_FIFO | UART_FCR_CLEAR_RCVR
+ | UART_FCR_CLEAR_XMIT;
+
+ status |= f81232_set_register(port, FIFO_CONTROL_REGISTER, data);
+
+ /* MSR Interrupt only, LSR will read from Bulk-in odd byte */
+ data = UART_IER_MSI;
+
+ /* IER */
+ status |= f81232_set_register(port, INTERRUPT_ENABLE_REGISTER, data);
+ if (status < 0) {
+ dev_err(&port->dev, "%s set error: %d\n", __func__, status);
+ return status;
+ }
+
+ return 0;
+}
+
static void f81232_set_termios(struct tty_struct *tty,
struct usb_serial_port *port, struct ktermios *old_termios)
{
- /* FIXME - Stubbed out for now */
+ u8 new_lcr = 0;
+ int status = 0;
- /* Don't change anything if nothing has changed */
- if (old_termios && !tty_termios_hw_change(&tty->termios, old_termios))
- return;
+ f81232_set_baudrate(port, tty_get_baud_rate(tty));
+
+ if (C_PARENB(tty)) {
+ new_lcr |= UART_LCR_PARITY;
+
+ if (!C_PARODD(tty))
+ new_lcr |= UART_LCR_EPAR;
+
+ if (C_CMSPAR(tty))
+ new_lcr |= UART_LCR_SPAR;
+ }
+
+ if (C_CSTOPB(tty))
+ new_lcr |= UART_LCR_STOP;
+
+ switch (C_CSIZE(tty)) {
+ case CS5:
+ new_lcr |= UART_LCR_WLEN5;
+ break;
+ case CS6:
+ new_lcr |= UART_LCR_WLEN6;
+ break;
+ case CS7:
+ new_lcr |= UART_LCR_WLEN7;
+ break;
+ default:
+ case CS8:
+ new_lcr |= UART_LCR_WLEN8;
+ break;
+ }
+
+ status = f81232_set_register(port, LINE_CONTROL_REGISTER, new_lcr);
+ if (status < 0)
+ dev_err(&port->dev, "%s set error: %d\n", __func__, status);
- /* Do the real work here... */
- if (old_termios)
- tty_termios_copy_hw(&tty->termios, old_termios);
}
static int f81232_tiocmget(struct tty_struct *tty)
@@ -278,6 +403,12 @@ static int f81232_open(struct tty_struct *tty, struct usb_serial_port *port)
{
int result;
+ result = f81232_port_init(port);
+ if (result) {
+ dev_err(&port->dev, "%s - init fail: %d\n", __func__, result);
+ return result;
+ }
+
/* Setup termios */
if (tty)
f81232_set_termios(tty, port, NULL);
--
1.9.1
This patch implement relative MCR/MSR function, such like
tiocmget()/tiocmset()/dtr_rts().
The f81232_set_mctrl() replace set_control_lines() to do MCR control
so we clean-up the set_control_lines() function.
Signed-off-by: Peter Hung <[email protected]>
---
drivers/usb/serial/f81232.c | 98 +++++++++++++++++++++++++++++++++++----------
1 file changed, 77 insertions(+), 21 deletions(-)
diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
index 06d1eb0..e1cdf42 100644
--- a/drivers/usb/serial/f81232.c
+++ b/drivers/usb/serial/f81232.c
@@ -44,6 +44,7 @@ MODULE_DEVICE_TABLE(usb, id_table);
#define INTERRUPT_ENABLE_REGISTER (0x01 + SERIAL_BASE_ADDRESS)
#define FIFO_CONTROL_REGISTER (0x02 + SERIAL_BASE_ADDRESS)
#define LINE_CONTROL_REGISTER (0x03 + SERIAL_BASE_ADDRESS)
+#define MODEM_CONTROL_REGISTER (0x04 + SERIAL_BASE_ADDRESS)
#define MODEM_STATUS_REGISTER (0x06 + SERIAL_BASE_ADDRESS)
#define CONTROL_DTR 0x01
@@ -156,6 +157,50 @@ static void f81232_read_msr(struct usb_serial_port *port)
wake_up_interruptible(&port->port.delta_msr_wait);
}
+static int f81232_set_mctrl(struct usb_serial_port *port,
+ unsigned int set, unsigned int clear)
+{
+ u8 urb_value;
+ int status;
+ unsigned long flags;
+ struct f81232_private *priv = usb_get_serial_port_data(port);
+
+ if (((set | clear) & (TIOCM_DTR | TIOCM_RTS)) == 0)
+ return 0; /* no change */
+
+ /* 'set' takes precedence over 'clear' */
+ clear &= ~set;
+
+ /* force enable interrupt with OUT2 */
+ urb_value = UART_MCR_OUT2 | priv->line_control;
+
+ if (clear & TIOCM_DTR)
+ urb_value &= ~UART_MCR_DTR;
+
+ if (clear & TIOCM_RTS)
+ urb_value &= ~UART_MCR_RTS;
+
+ if (set & TIOCM_DTR)
+ urb_value |= UART_MCR_DTR;
+
+ if (set & TIOCM_RTS)
+ urb_value |= UART_MCR_RTS;
+
+ dev_dbg(&port->dev, "%s new:%02x old:%02x\n", __func__,
+ urb_value, priv->line_control);
+
+ status = f81232_set_register(port, MODEM_CONTROL_REGISTER, urb_value);
+ if (status < 0) {
+ dev_err(&port->dev, "%s set MCR status < 0\n", __func__);
+ } else {
+ spin_lock_irqsave(&priv->lock, flags);
+ priv->line_control = urb_value;
+ spin_unlock_irqrestore(&priv->lock, flags);
+ }
+
+ return status;
+}
+
static void f81232_update_modem_status(struct usb_serial_port *port,
unsigned char *data,
unsigned int actual_length)
@@ -267,12 +312,6 @@ static void f81232_process_read_urb(struct urb *urb)
tty_flip_buffer_push(&port->port);
}
-static int set_control_lines(struct usb_device *dev, u8 value)
-{
- /* FIXME - Stubbed out for now */
- return 0;
-}
-
static void f81232_break_ctl(struct tty_struct *tty, int break_state)
{
/* FIXME - Stubbed out for now */
@@ -388,15 +427,41 @@ static void f81232_set_termios(struct tty_struct *tty,
static int f81232_tiocmget(struct tty_struct *tty)
{
- /* FIXME - Stubbed out for now */
- return 0;
+ int r;
+ struct usb_serial_port *port = tty->driver_data;
+ struct f81232_private *port_priv = usb_get_serial_port_data(port);
+ unsigned long flags;
+ u8 mcr, msr;
+
+ /* force get current MSR changed state */
+ f81232_read_msr(port);
+
+ spin_lock_irqsave(&port_priv->lock, flags);
+ mcr = port_priv->line_control;
+ msr = port_priv->modem_status;
+ spin_unlock_irqrestore(&port_priv->lock, flags);
+
+ r = (mcr & UART_MCR_DTR ? TIOCM_DTR : 0) |
+ (mcr & UART_MCR_RTS ? TIOCM_RTS : 0) |
+ (msr & UART_MSR_CTS ? TIOCM_CTS : 0) |
+ (msr & UART_MSR_DCD ? TIOCM_CAR : 0) |
+ (msr & UART_MSR_RI ? TIOCM_RI : 0) |
+ (msr & UART_MSR_DSR ? TIOCM_DSR : 0);
+
+ return r;
}
static int f81232_tiocmset(struct tty_struct *tty,
unsigned int set, unsigned int clear)
{
- /* FIXME - Stubbed out for now */
- return 0;
+ int status;
+ struct usb_serial_port *port = tty->driver_data;
+
+ status = f81232_set_mctrl(port, set, clear);
+ if (status < 0)
+ return usb_translate_errors(status);
+ else
+ return 0;
}
static int f81232_open(struct tty_struct *tty, struct usb_serial_port *port)
@@ -437,19 +502,10 @@ static void f81232_close(struct usb_serial_port *port)
static void f81232_dtr_rts(struct usb_serial_port *port, int on)
{
- struct f81232_private *priv = usb_get_serial_port_data(port);
- unsigned long flags;
- u8 control;
-
- spin_lock_irqsave(&priv->lock, flags);
- /* Change DTR and RTS */
if (on)
- priv->line_control |= (CONTROL_DTR | CONTROL_RTS);
+ f81232_set_mctrl(port, TIOCM_DTR | TIOCM_RTS, 0);
else
- priv->line_control &= ~(CONTROL_DTR | CONTROL_RTS);
- control = priv->line_control;
- spin_unlock_irqrestore(&priv->lock, flags);
- set_control_lines(port->serial->dev, control);
+ f81232_set_mctrl(port, 0, TIOCM_DTR | TIOCM_RTS);
}
static int f81232_carrier_raised(struct usb_serial_port *port)
--
1.9.1
We extract TIOCGSERIAL section in f81232_ioctl() to f81232_get_serial_info()
to make it clarify
The f81232_set_mctrl() replace set_control_lines() to do MCR control
so we clean-up the set_control_lines() function.
Signed-off-by: Peter Hung <[email protected]>
---
drivers/usb/serial/f81232.c | 34 +++++++++++++++++++++++-----------
1 file changed, 23 insertions(+), 11 deletions(-)
diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
index e1cdf42..4dddb44 100644
--- a/drivers/usb/serial/f81232.c
+++ b/drivers/usb/serial/f81232.c
@@ -516,24 +516,36 @@ static int f81232_carrier_raised(struct usb_serial_port *port)
return 0;
}
+static int f81232_get_serial_info(struct usb_serial_port *port,
+ unsigned long arg)
+{
+ struct serial_struct ser;
+
+ memset(&ser, 0, sizeof(ser));
+
+ ser.flags = ASYNC_SKIP_TEST | ASYNC_AUTO_IRQ;
+ ser.xmit_fifo_size = port->bulk_out_size;
+ ser.close_delay = 5 * HZ;
+ ser.closing_wait = 30 * HZ;
+ ser.type = PORT_16550A;
+ ser.line = port->minor;
+ ser.port = port->port_number;
+ ser.baud_base = 115200;
+
+ if (copy_to_user((void __user *)arg, &ser, sizeof(ser)))
+ return -EFAULT;
+
+ return 0;
+}
+
static int f81232_ioctl(struct tty_struct *tty,
unsigned int cmd, unsigned long arg)
{
- struct serial_struct ser;
struct usb_serial_port *port = tty->driver_data;
switch (cmd) {
case TIOCGSERIAL:
- memset(&ser, 0, sizeof ser);
- ser.type = PORT_16654;
- ser.line = port->minor;
- ser.port = port->port_number;
- ser.baud_base = 460800;
-
- if (copy_to_user((void __user *)arg, &ser, sizeof ser))
- return -EFAULT;
-
- return 0;
+ return f81232_get_serial_info(port, arg);
default:
break;
}
--
1.9.1
It's should compared with UART_MSR_DCD, not UART_DCD.
also we clean-up some non-used define to avoid impropriety use.
Signed-off-by: Peter Hung <[email protected]>
---
drivers/usb/serial/f81232.c | 16 +---------------
1 file changed, 1 insertion(+), 15 deletions(-)
diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
index 4dddb44..07abf0c 100644
--- a/drivers/usb/serial/f81232.c
+++ b/drivers/usb/serial/f81232.c
@@ -47,20 +47,6 @@ MODULE_DEVICE_TABLE(usb, id_table);
#define MODEM_CONTROL_REGISTER (0x04 + SERIAL_BASE_ADDRESS)
#define MODEM_STATUS_REGISTER (0x06 + SERIAL_BASE_ADDRESS)
-#define CONTROL_DTR 0x01
-#define CONTROL_RTS 0x02
-
-#define UART_STATE 0x08
-#define UART_STATE_TRANSIENT_MASK 0x74
-#define UART_DCD 0x01
-#define UART_DSR 0x02
-#define UART_BREAK_ERROR 0x04
-#define UART_RING 0x08
-#define UART_FRAME_ERROR 0x10
-#define UART_PARITY_ERROR 0x20
-#define UART_OVERRUN_ERROR 0x40
-#define UART_CTS 0x80
-
struct f81232_private {
spinlock_t lock;
u8 line_control;
@@ -511,7 +497,7 @@ static void f81232_dtr_rts(struct usb_serial_port *port, int on)
static int f81232_carrier_raised(struct usb_serial_port *port)
{
struct f81232_private *priv = usb_get_serial_port_data(port);
- if (priv->modem_status & UART_DCD)
+ if (priv->modem_status & UART_MSR_DCD)
return 1;
return 0;
}
--
1.9.1
Add me to co-author and fix no '>' in greg kh's email
Signed-off-by: Peter Hung <[email protected]>
---
drivers/usb/serial/f81232.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
index 07abf0c..8799b66 100644
--- a/drivers/usb/serial/f81232.c
+++ b/drivers/usb/serial/f81232.c
@@ -608,5 +608,6 @@ static struct usb_serial_driver * const serial_drivers[] = {
module_usb_serial_driver(serial_drivers, id_table);
MODULE_DESCRIPTION("Fintek F81232 USB to serial adaptor driver");
-MODULE_AUTHOR("Greg Kroah-Hartman <[email protected]");
+MODULE_AUTHOR("Greg Kroah-Hartman <[email protected]>");
+MODULE_AUTHOR("Peter Hong <[email protected]>");
MODULE_LICENSE("GPL v2");
--
1.9.1
Hello.
On 2/6/2015 12:46 PM, Peter Hung wrote:
> We extract TIOCGSERIAL section in f81232_ioctl() to f81232_get_serial_info()
> to make it clarify
You're also changing 'ser.baud_rate' from 460800 to 115200. And explicitly
overriding some previously initialized to 0 fields.
> The f81232_set_mctrl() replace set_control_lines() to do MCR control
> so we clean-up the set_control_lines() function.
I don't see where are you doing this...
> Signed-off-by: Peter Hung <[email protected]>
> ---
> drivers/usb/serial/f81232.c | 34 +++++++++++++++++++++++-----------
> 1 file changed, 23 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
> index e1cdf42..4dddb44 100644
> --- a/drivers/usb/serial/f81232.c
> +++ b/drivers/usb/serial/f81232.c
> @@ -516,24 +516,36 @@ static int f81232_carrier_raised(struct usb_serial_port *port)
> return 0;
> }
>
> +static int f81232_get_serial_info(struct usb_serial_port *port,
> + unsigned long arg)
> +{
> + struct serial_struct ser;
> +
> + memset(&ser, 0, sizeof(ser));
> +
> + ser.flags = ASYNC_SKIP_TEST | ASYNC_AUTO_IRQ;
> + ser.xmit_fifo_size = port->bulk_out_size;
> + ser.close_delay = 5 * HZ;
> + ser.closing_wait = 30 * HZ;
> + ser.type = PORT_16550A;
> + ser.line = port->minor;
> + ser.port = port->port_number;
> + ser.baud_base = 115200;
> +
> + if (copy_to_user((void __user *)arg, &ser, sizeof(ser)))
> + return -EFAULT;
> +
> + return 0;
> +}
> +
> static int f81232_ioctl(struct tty_struct *tty,
> unsigned int cmd, unsigned long arg)
> {
> - struct serial_struct ser;
> struct usb_serial_port *port = tty->driver_data;
>
> switch (cmd) {
> case TIOCGSERIAL:
> - memset(&ser, 0, sizeof ser);
> - ser.type = PORT_16654;
> - ser.line = port->minor;
> - ser.port = port->port_number;
> - ser.baud_base = 460800;
> -
> - if (copy_to_user((void __user *)arg, &ser, sizeof ser))
> - return -EFAULT;
> -
> - return 0;
> + return f81232_get_serial_info(port, arg);
> default:
> break;
> }
WBR, Sergei
Hello,
Sergei Shtylyov 於 2015/2/6 下午 08:21 寫道:
>> We extract TIOCGSERIAL section in f81232_ioctl() to
>> f81232_get_serial_info()
>> to make it clarify
>
> You're also changing 'ser.baud_rate' from 460800 to 115200. And
> explicitly overriding some previously initialized to 0 fields.
F81232 max baudrate is only 115200bps, so I set it for
1.8432MHz/16 = 115200.
We had add some closing time referenced from serial_core.c. The default
value is:
port->close_delay = HZ / 2; /* .5 seconds */
port->closing_wait = 30 * HZ;/* 30 seconds */
We had increasing close_delay about 10x to
port->close_delay = 5 * HZ ;
>
>> The f81232_set_mctrl() replace set_control_lines() to do MCR control
>> so we clean-up the set_control_lines() function.
>
> I don't see where are you doing this...
>
This text is my patch V5 5/8 second section. I had wrong operation of
copy & paste. It's doesn't need for this patch, sorry for it.
--
With Best Regards,
Peter Hung
On Mon, Feb 09, 2015 at 02:59:12PM +0800, Peter Hung wrote:
> Hello,
>
> Sergei Shtylyov 於 2015/2/6 下午 08:21 寫道:
> >> We extract TIOCGSERIAL section in f81232_ioctl() to
> >> f81232_get_serial_info()
> >> to make it clarify
> >
> > You're also changing 'ser.baud_rate' from 460800 to 115200. And
> > explicitly overriding some previously initialized to 0 fields.
>
> F81232 max baudrate is only 115200bps, so I set it for
> 1.8432MHz/16 = 115200.
>
> We had add some closing time referenced from serial_core.c. The default
> value is:
>
> port->close_delay = HZ / 2; /* .5 seconds */
> port->closing_wait = 30 * HZ;/* 30 seconds */
>
> We had increasing close_delay about 10x to
>
> port->close_delay = 5 * HZ ;
You're never changing anything, you're just reporting an incorrect value
to userspace here.
The value you should be returning is
jiffies_to_msecs(port->port.closing_wait) / 10, unless the value is
ASYNC_CLOSING_WAIT_NONE in which case you simply return that, and
similarly for close_delay.
> >> The f81232_set_mctrl() replace set_control_lines() to do MCR control
> >> so we clean-up the set_control_lines() function.
> >
> > I don't see where are you doing this...
> >
>
> This text is my patch V5 5/8 second section. I had wrong operation of
> copy & paste. It's doesn't need for this patch, sorry for it.
Make sure to update the commit log for the next revision so that it
describes what you actually do.
I will probably not have time to review this version this week I'm
afraid.
Johan
Hello,
Johan Hovold 於 2015/2/9 下午 04:42 寫道:
> The value you should be returning is
> jiffies_to_msecs(port->port.closing_wait) / 10, unless the value is
> ASYNC_CLOSING_WAIT_NONE in which case you simply return that, and
> similarly for close_delay.
I'll try to fix it, or reuse default value next version.
> Make sure to update the commit log for the next revision so that it
> describes what you actually do.
>
> I will probably not have time to review this version this week I'm
> afraid.
OK, I'll review it again, fix and resend it after Chinese New Year (2/23+).
Thanks all seniors.
--
With Best Regards,
Peter Hung
On Fri, Feb 06, 2015 at 05:46:49PM +0800, Peter Hung wrote:
> The F81232 bulk-in is RX data + LSR channel, data format is
> [LSR+Data][LSR+Data]..... , We had reimplemented in this patch.
>
> Signed-off-by: Peter Hung <[email protected]>
> ---
> drivers/usb/serial/f81232.c | 68 +++++++++++++++++++++++----------------------
> 1 file changed, 35 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
> index ec4609d..9ea498a 100644
> --- a/drivers/usb/serial/f81232.c
> +++ b/drivers/usb/serial/f81232.c
> @@ -185,44 +185,46 @@ exit:
> static void f81232_process_read_urb(struct urb *urb)
> {
> struct usb_serial_port *port = urb->context;
> - struct f81232_private *priv = usb_get_serial_port_data(port);
> unsigned char *data = urb->transfer_buffer;
> - char tty_flag = TTY_NORMAL;
> - unsigned long flags;
> - u8 line_status;
> + char tty_flag;
> int i;
>
> - /* update line status */
> - spin_lock_irqsave(&priv->lock, flags);
> - line_status = priv->modem_status;
> - priv->modem_status &= ~UART_STATE_TRANSIENT_MASK;
> - spin_unlock_irqrestore(&priv->lock, flags);
> -
> - if (!urb->actual_length)
> + if (urb->actual_length < 2)
> return;
>
> - /* break takes precedence over parity, */
> - /* which takes precedence over framing errors */
> - if (line_status & UART_BREAK_ERROR)
> - tty_flag = TTY_BREAK;
> - else if (line_status & UART_PARITY_ERROR)
> - tty_flag = TTY_PARITY;
> - else if (line_status & UART_FRAME_ERROR)
> - tty_flag = TTY_FRAME;
> - dev_dbg(&port->dev, "%s - tty_flag = %d\n", __func__, tty_flag);
> -
> - /* overrun is special, not associated with a char */
> - if (line_status & UART_OVERRUN_ERROR)
> - tty_insert_flip_char(&port->port, 0, TTY_OVERRUN);
> -
> - if (port->port.console && port->sysrq) {
> - for (i = 0; i < urb->actual_length; ++i)
> - if (!usb_serial_handle_sysrq_char(port, data[i]))
> - tty_insert_flip_char(&port->port, data[i],
> - tty_flag);
> - } else {
> - tty_insert_flip_string_fixed_flag(&port->port, data, tty_flag,
> - urb->actual_length);
> + /* bulk-in data: [LSR(1Byte)+DATA(1Byte)][LSR(1Byte)+DATA(1Byte)]... */
> +
> + for (i = 0 ; i < urb->actual_length ; i += 2) {
No space after ;.
> + tty_flag = TTY_NORMAL;
> +
> + if (unlikely(data[i+0] & UART_LSR_BRK_ERROR_BITS)) {
Always use spaces around binary operators such as +, but in this case
I think it would be preferable to introduce a temporary variable u8 lsr,
which you update at the beginning of the loop construct.
> + if (data[i+0] & UART_LSR_BI) {
> + tty_flag = TTY_BREAK;
> + port->icount.brk++;
> + usb_serial_handle_break(port);
> + } else if (data[i+0] & UART_LSR_PE) {
> + tty_flag = TTY_PARITY;
> + port->icount.parity++;
> + } else if (data[i+0] & UART_LSR_FE) {
> + tty_flag = TTY_FRAME;
> + port->icount.frame++;
> + }
> +
> + if (data[0] & UART_LSR_OE) {
You probably want data[i] (lsr) here as well.
> + port->icount.overrun++;
> + tty_insert_flip_char(&port->port, 0,
> + TTY_OVERRUN);
> + }
> + }
> +
> + if (port->port.console && port->sysrq) {
> + if (!usb_serial_handle_sysrq_char(port, data[i+1]))
Spaces around + again.
> + tty_insert_flip_char(&port->port, data[i+1],
> + tty_flag);
> + } else {
> + tty_insert_flip_string_fixed_flag(&port->port,
> + &data[i+1], tty_flag, 1);
> + }
Use tty_insert_flip_char in both branches. In fact, you could rewrite
this as:
if (port->port.console && port->sysrq) {
if (usb_serial_handle_sysrq_char(port, data[i + 1]))
continue;
}
tty_insert_flip_char(&port->port, data[i + 1], tty_flag);
> }
>
> tty_flip_buffer_push(&port->port);
I'll try to look at the rest of the series soon.
Johan
On Mon, Feb 16, 2015 at 07:59:45PM +0700, Johan Hovold wrote:
> On Fri, Feb 06, 2015 at 05:46:49PM +0800, Peter Hung wrote:
> I'll try to look at the rest of the series soon.
Managed to comment on v5 rather than v6, but looks like comments on this
patch still apply.
Johan