* [PATCH V1 0/2] Implement break control for F81232/F81534 @ 2016-12-09 5:39 Ji-Ze Hong (Peter Hong) 2016-12-09 5:39 ` [PATCH V1 1/2] usb:serial: Implement Fintek F81232 break on/off Ji-Ze Hong (Peter Hong) 2016-12-09 5:39 ` [PATCH V1 2/2] usb:serial: Implement Fintek f81534 " Ji-Ze Hong (Peter Hong) 0 siblings, 2 replies; 5+ messages in thread From: Ji-Ze Hong (Peter Hong) @ 2016-12-09 5:39 UTC (permalink / raw) To: johan Cc: gregkh, linux-usb, linux-kernel, tom_tsai, peter_hong, hpeter+linux_kernel The following 2 patches makes break control available for Fintek F81232/F81534. Ji-Ze Hong (Peter Hong) (2): usb:serial: Implement Fintek F81232 break on/off usb:serial: Implement Fintek f81534 break on/off drivers/usb/serial/f81232.c | 40 ++++++++++++++++++++++++++++++++++------ drivers/usb/serial/f81534.c | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 6 deletions(-) -- 1.9.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH V1 1/2] usb:serial: Implement Fintek F81232 break on/off 2016-12-09 5:39 [PATCH V1 0/2] Implement break control for F81232/F81534 Ji-Ze Hong (Peter Hong) @ 2016-12-09 5:39 ` Ji-Ze Hong (Peter Hong) 2017-01-09 15:14 ` Johan Hovold 2016-12-09 5:39 ` [PATCH V1 2/2] usb:serial: Implement Fintek f81534 " Ji-Ze Hong (Peter Hong) 1 sibling, 1 reply; 5+ messages in thread From: Ji-Ze Hong (Peter Hong) @ 2016-12-09 5:39 UTC (permalink / raw) To: johan Cc: gregkh, linux-usb, linux-kernel, tom_tsai, peter_hong, hpeter+linux_kernel Implement Fintek F81232 break on/off with LCR register, it's the same with 16550A LCR register layout. Signed-off-by: Ji-Ze Hong (Peter Hong) <hpeter+linux_kernel@gmail.com> --- drivers/usb/serial/f81232.c | 40 ++++++++++++++++++++++++++++++++++------ 1 file changed, 34 insertions(+), 6 deletions(-) diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c index 972f5a5..d45a70e 100644 --- a/drivers/usb/serial/f81232.c +++ b/drivers/usb/serial/f81232.c @@ -131,6 +131,21 @@ static int f81232_set_register(struct usb_serial_port *port, u16 reg, u8 val) return status; } +static int f81232_set_mask_register(struct usb_serial_port *port, u16 reg, + u8 mask, u8 val) +{ + int status; + u8 tmp; + + status = f81232_get_register(port, reg, &tmp); + if (status) + return status; + + tmp = (tmp & ~mask) | (val & mask); + + return f81232_set_register(port, reg, tmp); +} + static void f81232_read_msr(struct usb_serial_port *port) { int status; @@ -335,15 +350,27 @@ static void f81232_process_read_urb(struct urb *urb) tty_flip_buffer_push(&port->port); } +static void f81232_set_break(struct usb_serial_port *port, bool enable) +{ + int status; + u8 tmp = 0; + + if (enable) + tmp = UART_LCR_SBC; + + status = f81232_set_mask_register(port, LINE_CONTROL_REGISTER, + UART_LCR_SBC, tmp); + if (status) { + dev_err(&port->dev, "%s: set break failed: %x\n", __func__, + status); + } +} + static void f81232_break_ctl(struct tty_struct *tty, int break_state) { - /* FIXME - Stubbed out for now */ + struct usb_serial_port *port = tty->driver_data; - /* - * break_state = -1 to turn on break, and 0 to turn off break - * see drivers/char/tty_io.c to see it used. - * last_set_data_urb_value NEVER has the break bit set in it. - */ + f81232_set_break(port, break_state); } static void f81232_set_baudrate(struct usb_serial_port *port, speed_t baudrate) @@ -563,6 +590,7 @@ static void f81232_close(struct usb_serial_port *port) f81232_port_disable(port); usb_serial_generic_close(port); usb_kill_urb(port->interrupt_in_urb); + f81232_set_break(port, false); } static void f81232_dtr_rts(struct usb_serial_port *port, int on) -- 1.9.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH V1 1/2] usb:serial: Implement Fintek F81232 break on/off 2016-12-09 5:39 ` [PATCH V1 1/2] usb:serial: Implement Fintek F81232 break on/off Ji-Ze Hong (Peter Hong) @ 2017-01-09 15:14 ` Johan Hovold 0 siblings, 0 replies; 5+ messages in thread From: Johan Hovold @ 2017-01-09 15:14 UTC (permalink / raw) To: Ji-Ze Hong (Peter Hong) Cc: johan, gregkh, linux-usb, linux-kernel, tom_tsai, peter_hong, hpeter+linux_kernel On Fri, Dec 09, 2016 at 01:39:33PM +0800, Ji-Ze Hong (Peter Hong) wrote: > Implement Fintek F81232 break on/off with LCR register, > it's the same with 16550A LCR register layout. > > Signed-off-by: Ji-Ze Hong (Peter Hong) <hpeter+linux_kernel@gmail.com> > --- > drivers/usb/serial/f81232.c | 40 ++++++++++++++++++++++++++++++++++------ > 1 file changed, 34 insertions(+), 6 deletions(-) > > diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c > index 972f5a5..d45a70e 100644 > --- a/drivers/usb/serial/f81232.c > +++ b/drivers/usb/serial/f81232.c > @@ -131,6 +131,21 @@ static int f81232_set_register(struct usb_serial_port *port, u16 reg, u8 val) > return status; > } > > +static int f81232_set_mask_register(struct usb_serial_port *port, u16 reg, > + u8 mask, u8 val) > +{ > + int status; > + u8 tmp; > + > + status = f81232_get_register(port, reg, &tmp); > + if (status) > + return status; > + > + tmp = (tmp & ~mask) | (val & mask); > + > + return f81232_set_register(port, reg, tmp); > +} > + > static void f81232_read_msr(struct usb_serial_port *port) > { > int status; > @@ -335,15 +350,27 @@ static void f81232_process_read_urb(struct urb *urb) > tty_flip_buffer_push(&port->port); > } > > +static void f81232_set_break(struct usb_serial_port *port, bool enable) > +{ > + int status; > + u8 tmp = 0; > + > + if (enable) > + tmp = UART_LCR_SBC; > + > + status = f81232_set_mask_register(port, LINE_CONTROL_REGISTER, > + UART_LCR_SBC, tmp); > + if (status) { > + dev_err(&port->dev, "%s: set break failed: %x\n", __func__, You want %d here. And you can drop the __func__ prefix. > + status); > + } You should also coordinate this with the LCR update in set_termios, otherwise changing line settings would clear break etc. > +} > + > static void f81232_break_ctl(struct tty_struct *tty, int break_state) > { > - /* FIXME - Stubbed out for now */ > + struct usb_serial_port *port = tty->driver_data; > > - /* > - * break_state = -1 to turn on break, and 0 to turn off break > - * see drivers/char/tty_io.c to see it used. > - * last_set_data_urb_value NEVER has the break bit set in it. > - */ > + f81232_set_break(port, break_state); > } > > static void f81232_set_baudrate(struct usb_serial_port *port, speed_t baudrate) > @@ -563,6 +590,7 @@ static void f81232_close(struct usb_serial_port *port) > f81232_port_disable(port); > usb_serial_generic_close(port); > usb_kill_urb(port->interrupt_in_urb); > + f81232_set_break(port, false); I don't think this is needed, and then you can fold set_break into break_ctl above as well. > } > > static void f81232_dtr_rts(struct usb_serial_port *port, int on) Thanks, Johan ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH V1 2/2] usb:serial: Implement Fintek f81534 break on/off 2016-12-09 5:39 [PATCH V1 0/2] Implement break control for F81232/F81534 Ji-Ze Hong (Peter Hong) 2016-12-09 5:39 ` [PATCH V1 1/2] usb:serial: Implement Fintek F81232 break on/off Ji-Ze Hong (Peter Hong) @ 2016-12-09 5:39 ` Ji-Ze Hong (Peter Hong) 2017-01-09 15:21 ` Johan Hovold 1 sibling, 1 reply; 5+ messages in thread From: Ji-Ze Hong (Peter Hong) @ 2016-12-09 5:39 UTC (permalink / raw) To: johan Cc: gregkh, linux-usb, linux-kernel, tom_tsai, peter_hong, hpeter+linux_kernel Implement Fintek f81534 break on/off with LCR register It's the same with 16550A LCR register layout We'll add a shadow LCR variable to save the final LCR we had set due to the "read ep0" operations maybe slow down all the serial ports performance. Signed-off-by: Ji-Ze Hong (Peter Hong) <hpeter+linux_kernel@gmail.com> --- drivers/usb/serial/f81534.c | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/drivers/usb/serial/f81534.c b/drivers/usb/serial/f81534.c index 8282a6a..1b6ba81 100644 --- a/drivers/usb/serial/f81534.c +++ b/drivers/usb/serial/f81534.c @@ -126,8 +126,10 @@ struct f81534_serial_private { struct f81534_port_private { struct mutex mcr_mutex; + struct mutex lcr_mutex; unsigned long tx_empty; spinlock_t msr_lock; + u8 shadow_lcr; u8 shadow_mcr; u8 shadow_msr; u8 phy_num; @@ -683,6 +685,7 @@ static void f81534_set_termios(struct tty_struct *tty, struct usb_serial_port *port, struct ktermios *old_termios) { + struct f81534_port_private *port_priv = usb_get_serial_port_data(port); u8 new_lcr = 0; int status; u32 baud; @@ -721,6 +724,10 @@ static void f81534_set_termios(struct tty_struct *tty, break; } + mutex_lock(&port_priv->lcr_mutex); + port_priv->shadow_lcr = new_lcr; + mutex_unlock(&port_priv->lcr_mutex); + baud = tty_get_baud_rate(tty); if (!baud) return; @@ -812,6 +819,35 @@ static int f81534_read_msr(struct usb_serial_port *port) return 0; } +static void f81534_set_break(struct usb_serial_port *port, bool enable) +{ + struct f81534_port_private *port_priv = usb_get_serial_port_data(port); + int status; + + mutex_lock(&port_priv->lcr_mutex); + + if (enable) + port_priv->shadow_lcr |= UART_LCR_SBC; + else + port_priv->shadow_lcr &= ~UART_LCR_SBC; + + status = f81534_set_port_register(port, F81534_LINE_CONTROL_REG, + port_priv->shadow_lcr); + if (status) { + dev_err(&port->dev, "%s: set break failed: %x\n", __func__, + status); + } + + mutex_unlock(&port_priv->lcr_mutex); +} + +static void f81534_break_ctl(struct tty_struct *tty, int break_state) +{ + struct usb_serial_port *port = tty->driver_data; + + f81534_set_break(port, break_state); +} + static int f81534_open(struct tty_struct *tty, struct usb_serial_port *port) { struct f81534_serial_private *serial_priv = @@ -877,6 +913,8 @@ static void f81534_close(struct usb_serial_port *port) } mutex_unlock(&serial_priv->urb_mutex); + + f81534_set_break(port, false); } static int f81534_get_serial_info(struct usb_serial_port *port, @@ -1244,6 +1282,7 @@ static int f81534_port_probe(struct usb_serial_port *port) spin_lock_init(&port_priv->msr_lock); mutex_init(&port_priv->mcr_mutex); + mutex_init(&port_priv->lcr_mutex); /* Assign logic-to-phy mapping */ port_priv->phy_num = f81534_logic_to_phy_port(port->serial, port); @@ -1389,6 +1428,7 @@ static int f81534_resume(struct usb_serial *serial) .dtr_rts = f81534_dtr_rts, .process_read_urb = f81534_process_read_urb, .ioctl = f81534_ioctl, + .break_ctl = f81534_break_ctl, .tiocmget = f81534_tiocmget, .tiocmset = f81534_tiocmset, .write_bulk_callback = f81534_write_usb_callback, -- 1.9.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH V1 2/2] usb:serial: Implement Fintek f81534 break on/off 2016-12-09 5:39 ` [PATCH V1 2/2] usb:serial: Implement Fintek f81534 " Ji-Ze Hong (Peter Hong) @ 2017-01-09 15:21 ` Johan Hovold 0 siblings, 0 replies; 5+ messages in thread From: Johan Hovold @ 2017-01-09 15:21 UTC (permalink / raw) To: Ji-Ze Hong (Peter Hong) Cc: johan, gregkh, linux-usb, linux-kernel, tom_tsai, peter_hong, hpeter+linux_kernel On Fri, Dec 09, 2016 at 01:39:34PM +0800, Ji-Ze Hong (Peter Hong) wrote: > Implement Fintek f81534 break on/off with LCR register > It's the same with 16550A LCR register layout > > We'll add a shadow LCR variable to save the final LCR we > had set due to the "read ep0" operations maybe slow down all > the serial ports performance. > > Signed-off-by: Ji-Ze Hong (Peter Hong) <hpeter+linux_kernel@gmail.com> > --- > drivers/usb/serial/f81534.c | 40 ++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 40 insertions(+) > > diff --git a/drivers/usb/serial/f81534.c b/drivers/usb/serial/f81534.c > index 8282a6a..1b6ba81 100644 > --- a/drivers/usb/serial/f81534.c > +++ b/drivers/usb/serial/f81534.c > @@ -126,8 +126,10 @@ struct f81534_serial_private { > > struct f81534_port_private { > struct mutex mcr_mutex; > + struct mutex lcr_mutex; Using a single mutex is preferred. > unsigned long tx_empty; > spinlock_t msr_lock; > + u8 shadow_lcr; > u8 shadow_mcr; > u8 shadow_msr; > u8 phy_num; > @@ -683,6 +685,7 @@ static void f81534_set_termios(struct tty_struct *tty, > struct usb_serial_port *port, > struct ktermios *old_termios) > { > + struct f81534_port_private *port_priv = usb_get_serial_port_data(port); > u8 new_lcr = 0; > int status; > u32 baud; > @@ -721,6 +724,10 @@ static void f81534_set_termios(struct tty_struct *tty, > break; > } > > + mutex_lock(&port_priv->lcr_mutex); > + port_priv->shadow_lcr = new_lcr; > + mutex_unlock(&port_priv->lcr_mutex); And this is not sufficient to prevent races with break_ctl. Hold the mutex while updating LCR in set_port_config at least (and honour the break state). > + > baud = tty_get_baud_rate(tty); > if (!baud) > return; > @@ -812,6 +819,35 @@ static int f81534_read_msr(struct usb_serial_port *port) > return 0; > } > > +static void f81534_set_break(struct usb_serial_port *port, bool enable) > +{ > + struct f81534_port_private *port_priv = usb_get_serial_port_data(port); > + int status; > + > + mutex_lock(&port_priv->lcr_mutex); > + > + if (enable) > + port_priv->shadow_lcr |= UART_LCR_SBC; > + else > + port_priv->shadow_lcr &= ~UART_LCR_SBC; > + > + status = f81534_set_port_register(port, F81534_LINE_CONTROL_REG, > + port_priv->shadow_lcr); > + if (status) { > + dev_err(&port->dev, "%s: set break failed: %x\n", __func__, > + status); %d, __func__ not needed. > + } > + > + mutex_unlock(&port_priv->lcr_mutex); > +} > + > +static void f81534_break_ctl(struct tty_struct *tty, int break_state) > +{ > + struct usb_serial_port *port = tty->driver_data; > + > + f81534_set_break(port, break_state); > +} > + > static int f81534_open(struct tty_struct *tty, struct usb_serial_port *port) > { > struct f81534_serial_private *serial_priv = > @@ -877,6 +913,8 @@ static void f81534_close(struct usb_serial_port *port) > } > > mutex_unlock(&serial_priv->urb_mutex); > + > + f81534_set_break(port, false); Drop this too, and fold set_break into break_ctl above. > } > > static int f81534_get_serial_info(struct usb_serial_port *port, > @@ -1244,6 +1282,7 @@ static int f81534_port_probe(struct usb_serial_port *port) > > spin_lock_init(&port_priv->msr_lock); > mutex_init(&port_priv->mcr_mutex); > + mutex_init(&port_priv->lcr_mutex); > > /* Assign logic-to-phy mapping */ > port_priv->phy_num = f81534_logic_to_phy_port(port->serial, port); > @@ -1389,6 +1428,7 @@ static int f81534_resume(struct usb_serial *serial) > .dtr_rts = f81534_dtr_rts, > .process_read_urb = f81534_process_read_urb, > .ioctl = f81534_ioctl, > + .break_ctl = f81534_break_ctl, > .tiocmget = f81534_tiocmget, > .tiocmset = f81534_tiocmset, > .write_bulk_callback = f81534_write_usb_callback, Thanks, Johan ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-01-09 15:23 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-12-09 5:39 [PATCH V1 0/2] Implement break control for F81232/F81534 Ji-Ze Hong (Peter Hong) 2016-12-09 5:39 ` [PATCH V1 1/2] usb:serial: Implement Fintek F81232 break on/off Ji-Ze Hong (Peter Hong) 2017-01-09 15:14 ` Johan Hovold 2016-12-09 5:39 ` [PATCH V1 2/2] usb:serial: Implement Fintek f81534 " Ji-Ze Hong (Peter Hong) 2017-01-09 15:21 ` Johan Hovold
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.