USB: f81534: v5 patch
Subsystems (2)
v5 cover USB: f81534: v5 patch
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(-)
Signature · 1 line
1.9.1
PATCH v5 1/8 USB: f81232: Rename private struct member name
Change private struct member name from line_status to modem_status. It will store MSR for some functions used
--- drivers/usb/serial/f81232.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
drivers/usb/serial/f81232.c +4 -4
@@ -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 ; }
Signature · 1 line
1.9.1
PATCH v5 2/8 USB: f81232: implement read IIR/MSR with endpoint
In reply to Peter HungThe 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.
--- drivers/usb/serial/f81232.c | 109 ++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 100 insertions(+), 9 deletions(-)
drivers/usb/serial/f81232.c +100 -9
@@ -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 ; }
Signature · 1 line
1.9.1
PATCH v5 3/8 USB: f81232: implement RX bulk-in ep
In reply to Peter HungThe F81232 bulk-in is RX data + LSR channel, data format is [LSR+Data][LSR+Data]..... , We had reimplemented in this patch.
--- drivers/usb/serial/f81232.c | 68 +++++++++++++++++++++++---------------------- 1 file changed, 35 insertions(+), 33 deletions(-)
drivers/usb/serial/f81232.c +35 -33
@@ -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);
Signature · 1 line
1.9.1
Johan Hovold
maintainerRe: [PATCH V5 3/8] USB: f81232: implement RX bulk-in ep
Quoted 49 lines · Peter Hung
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(-)drivers/usb/serial/f81232.c +2
-26@@ -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) {
drivers/usb/serial/f81232.c exit: · line 197
Quoted 10 lines
drivers/usb/serial/f81232.c +3
-7-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 ;.
drivers/usb/serial/f81232.c exit: · line 200
Quoted 3 lines
drivers/usb/serial/f81232.c +3
-0+ 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.
drivers/usb/serial/f81232.c exit: · line 213
Quoted 13 lines
drivers/usb/serial/f81232.c +13
-0+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.
drivers/usb/serial/f81232.c exit: · line 221
Quoted 8 lines
drivers/usb/serial/f81232.c +8
-0+ 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.
drivers/usb/serial/f81232.c exit: · line 227
Quoted 6 lines
drivers/usb/serial/f81232.c +6
-0+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);drivers/usb/serial/f81232.c exit: · line 230
Quoted 3 lines
drivers/usb/serial/f81232.c +0
-0}tty_flip_buffer_push (&port->port);
I'll try to look at the rest of the series soon.
Johan
Johan Hovold
maintainerRe: [PATCH V5 3/8] USB: f81232: implement RX bulk-in ep
Quoted 3 lines · Johan Hovold
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
PATCH v5 4/8 USB: f81232: implement set_termios
In reply to Peter HungThe 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()
--- drivers/usb/serial/f81232.c | 145 +++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 138 insertions(+), 7 deletions(-)
drivers/usb/serial/f81232.c +138 -7
@@ -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 );
Signature · 1 line
1.9.1
PATCH v5 5/8 USB: f81232: implement MCR/MSR function
In reply to Peter HungThis 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.
--- drivers/usb/serial/f81232.c | 98 +++++++++++++++++++++++++++++++++++---------- 1 file changed, 77 insertions(+), 21 deletions(-)
drivers/usb/serial/f81232.c +77 -21
@@ -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)
Signature · 1 line
1.9.1
PATCH v5 6/8 USB: f81232: clarify f81232_ioctl()
In reply to Peter HungWe 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.
--- drivers/usb/serial/f81232.c | 34 +++++++++++++++++++++++----------- 1 file changed, 23 insertions(+), 11 deletions(-)
drivers/usb/serial/f81232.c +23 -11
@@ -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 ; }
Signature · 1 line
1.9.1
Re: [PATCH V5 6/8] USB: f81232: clarify f81232_ioctl()
Hello.
Quoted 2 lines · Peter Hung
We extract TIOCGSERIAL section in
f81232_ioctl()tof81232_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.
Quoted 2 lines
The
f81232_set_mctrl()replaceset_control_lines()to do MCR control so we clean-up theset_control_lines()function.
I don't see where are you doing this...
Quoted 47 lines
Signed-off-by Peter Hung <[email protected]>--- drivers/usb/serial/f81232.c | 34 +++++++++++++++++++++++----------- 1 file changed, 23 insertions(+), 11 deletions(-)drivers/usb/serial/f81232.c +22
-5@@ -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;
drivers/usb/serial/f81232.c static int f81232_carrier_raised(struct usb_serial_port *port) · line 551
Quoted 10 lines
drivers/usb/serial/f81232.c +1
-6- 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
Re: [PATCH V5 6/8] USB: f81232: clarify f81232_ioctl()
Hello,
Sergei Shtylyov 於 2015/2/6 下午 08:21 寫道:
Quoted 6 lines
Quoted 3 lines
We extract TIOCGSERIAL section in
f81232_ioctl()tof81232_get_serial_info()
to make it clarifyYou're also changing 'ser.baud_rate' from 460800 to 115200. Andexplicitly 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 ;
Quoted 6 lines
Quoted 3 lines
The
f81232_set_mctrl()replaceset_control_lines()to do MCR control so we clean-up theset_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.
Signature · 2 lines
With Best Regards, Peter Hung
Johan Hovold
maintainerRe: [PATCH V5 6/8] USB: f81232: clarify f81232_ioctl()
Quoted 16 lines · Peter Hung
Hello,
Sergei Shtylyov 於 2015/2/6 下午 08:21 寫道:
Quoted 6 lines
Quoted 3 lines
We extract TIOCGSERIAL section in
f81232_ioctl()tof81232_get_serial_info()
to make it clarifyYou're also changing 'ser.baud_rate' from 460800 to 115200. Andexplicitly 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:
Quoted 6 lines
port->close_delay= HZ / 2; /* .5 seconds */port->closing_wait= 30 * HZ;/* 30 seconds */We had increasing
close_delayabout 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.
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
The
f81232_set_mctrl()replaceset_control_lines()to do MCR control so we clean-up theset_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
Re: [PATCH V5 6/8] USB: f81232: clarify f81232_ioctl()
Hello,
Johan Hovold 於 2015/2/9 下午 04:42 寫道:
Quoted 4 lines
The value you should be returning is
jiffies_to_msecs(port->port.closing_wait)/ 10, unless the value isASYNC_CLOSING_WAIT_NONEin which case you simply return that, and similarly forclose_delay.
I'll try to fix it, or reuse default value next version.
Quoted 5 lines
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.
Signature · 2 lines
With Best Regards, Peter Hung
PATCH v5 7/8 USB: f81232: fix error in f81232_carrier_raised()
In reply to Peter HungIt's should compared with UART_MSR_DCD, not UART_DCD. also we clean-up some non-used define to avoid impropriety use.
--- drivers/usb/serial/f81232.c | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-)
drivers/usb/serial/f81232.c +1 -15
@@ -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 ; }
Signature · 1 line
1.9.1
PATCH v5 8/8 USB: f81232: modify/add author
In reply to Peter HungAdd me to co-author and fix no '>' in greg kh's email
--- drivers/usb/serial/f81232.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/usb/serial/f81232.c +2 -1
@@ -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" );
Signature · 1 line
1.9.1