2015-02-05 10:56:23

by Jari Ruusu

[permalink] [raw]
Subject: usb serial: pl2303 driver TxD "break" stays after close() bug

Tested on 3.10.67 and 3.18.5 kernels with ATEN UC-232A usb-serial adapter.
No patch, sorry. To actually see the stuck "break" signal on TxD line, you
need either some sort of LED or voltmeter connected to the data transmit
line. Other RS-232 serial ports that I have access to (normal PC hardware
serial ports and FTDI usb-serial adapters) do not have this bug.


/* pl2303 TxD "break" stays after close() demo */

#include <stdio.h>
#include <unistd.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <sys/ioctl.h>
#include <termios.h>
#include <fcntl.h>
#include <stdlib.h>

int main(int argc, char **argv)
{
struct termios tt;
int fd;

if(argc < 2) {
fprintf(stderr, "usage: %s /dev/ttyUSB0\n", argv[0]);
exit(1);
}
if((fd = open(argv[1], O_RDWR | O_NOCTTY, 0)) == -1) {
perror("serial port open failed");
exit(1);
}
tcgetattr(fd, &tt);
cfmakeraw(&tt);
tcsetattr(fd, TCSANOW, &tt);
if(ioctl(fd, TIOCSBRK, 0) == -1) {
perror("set BRK failed");
exit(1);
}
close(fd);
exit(0);
}

--
Jari Ruusu 4096R/8132F189 12D6 4C3A DCDA 0AA4 27BD ACDF F073 3C80 8132 F189


2015-02-19 05:49:00

by Johan Hovold

[permalink] [raw]
Subject: Re: usb serial: pl2303 driver TxD "break" stays after close() bug

On Thu, Feb 05, 2015 at 12:56:20PM +0200, Jari Ruusu wrote:
> Tested on 3.10.67 and 3.18.5 kernels with ATEN UC-232A usb-serial adapter.
> No patch, sorry. To actually see the stuck "break" signal on TxD line, you
> need either some sort of LED or voltmeter connected to the data transmit
> line. Other RS-232 serial ports that I have access to (normal PC hardware
> serial ports and FTDI usb-serial adapters) do not have this bug.

Thanks for the report. I believe you may be right here, but I'm not able
to verify this until earliest next week.

What happens when you reopen the port? Is the break state cleared then?

Thanks,
Johan

2015-02-19 13:38:42

by Jari Ruusu

[permalink] [raw]
Subject: Re: usb serial: pl2303 driver TxD "break" stays after close() bug

On 2/19/15, Johan Hovold <[email protected]> wrote:
> What happens when you reopen the port? Is the break state cleared then?

Stuck "break" signal is not cleared on re-open.

To clear it, you need to poke it with ioctl(fd, TIOCCBRK, 0)
or disconnect the device.

--
Jari Ruusu 4096R/8132F189 12D6 4C3A DCDA 0AA4 27BD ACDF F073 3C80 8132 F189

2015-02-20 03:23:47

by Johan Hovold

[permalink] [raw]
Subject: Re: usb serial: pl2303 driver TxD "break" stays after close() bug

On Thu, Feb 19, 2015 at 03:38:39PM +0200, Jari Ruusu wrote:
> On 2/19/15, Johan Hovold <[email protected]> wrote:
> > What happens when you reopen the port? Is the break state cleared then?
>
> Stuck "break" signal is not cleared on re-open.
>
> To clear it, you need to poke it with ioctl(fd, TIOCCBRK, 0)
> or disconnect the device.

That's definitely a bug.

Care to test the patch below?

Thanks,
Johan


>From ee3f2d35adc59822d72f3908078da7f361c26577 Mon Sep 17 00:00:00 2001
From: Johan Hovold <[email protected]>
Date: Fri, 20 Feb 2015 10:05:51 +0700
Subject: [PATCH] USB: pl2303: disable break on shutdown

Currently an enabled break state is not disabled on final close nor on
re-open and has to be disabled manually.

Fix this by disabling break on port shutdown.

Reported-by: Jari Ruusu <[email protected]>
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/usb/serial/pl2303.c | 18 +++++++++++++-----
1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/serial/pl2303.c b/drivers/usb/serial/pl2303.c
index 0f872e6b2c87..f6fe4737185d 100644
--- a/drivers/usb/serial/pl2303.c
+++ b/drivers/usb/serial/pl2303.c
@@ -132,6 +132,7 @@ MODULE_DEVICE_TABLE(usb, id_table);
#define UART_OVERRUN_ERROR 0x40
#define UART_CTS 0x80

+static void pl2303_set_break(struct usb_serial_port *port, bool enable);

enum pl2303_type {
TYPE_01, /* Type 0 and 1 (difference unknown) */
@@ -613,6 +614,7 @@ static void pl2303_dtr_rts(struct usb_serial_port *port, int on)

static void pl2303_close(struct usb_serial_port *port)
{
+ pl2303_set_break(port, false);
usb_serial_generic_close(port);
usb_kill_urb(port->interrupt_in_urb);
}
@@ -741,17 +743,16 @@ static int pl2303_ioctl(struct tty_struct *tty,
return -ENOIOCTLCMD;
}

-static void pl2303_break_ctl(struct tty_struct *tty, int break_state)
+static void pl2303_set_break(struct usb_serial_port *port, bool enable)
{
- struct usb_serial_port *port = tty->driver_data;
struct usb_serial *serial = port->serial;
u16 state;
int result;

- if (break_state == 0)
- state = BREAK_OFF;
- else
+ if (enable)
state = BREAK_ON;
+ else
+ state = BREAK_OFF;

dev_dbg(&port->dev, "%s - turning break %s\n", __func__,
state == BREAK_OFF ? "off" : "on");
@@ -763,6 +764,13 @@ static void pl2303_break_ctl(struct tty_struct *tty, int break_state)
dev_err(&port->dev, "error sending break = %d\n", result);
}

+static void pl2303_break_ctl(struct tty_struct *tty, int state)
+{
+ struct usb_serial_port *port = tty->driver_data;
+
+ pl2303_set_break(port, state);
+}
+
static void pl2303_update_line_status(struct usb_serial_port *port,
unsigned char *data,
unsigned int actual_length)
--
2.0.5

2015-02-20 13:11:16

by Jari Ruusu

[permalink] [raw]
Subject: Re: usb serial: pl2303 driver TxD "break" stays after close() bug

On 2/20/15, Johan Hovold <[email protected]> wrote:
> On Thu, Feb 19, 2015 at 03:38:39PM +0200, Jari Ruusu wrote:
>> To clear it, you need to poke it with ioctl(fd, TIOCCBRK, 0)
>> or disconnect the device.
>
> That's definitely a bug.
>
> Care to test the patch below?

Your patch fixes the bug. Thanks.

--
Jari Ruusu 4096R/8132F189 12D6 4C3A DCDA 0AA4 27BD ACDF F073 3C80 8132 F189

2015-02-24 06:29:09

by Johan Hovold

[permalink] [raw]
Subject: Re: usb serial: pl2303 driver TxD "break" stays after close() bug

On Fri, Feb 20, 2015 at 03:11:12PM +0200, Jari Ruusu wrote:
> On 2/20/15, Johan Hovold <[email protected]> wrote:
> > On Thu, Feb 19, 2015 at 03:38:39PM +0200, Jari Ruusu wrote:
> >> To clear it, you need to poke it with ioctl(fd, TIOCCBRK, 0)
> >> or disconnect the device.
> >
> > That's definitely a bug.
> >
> > Care to test the patch below?
>
> Your patch fixes the bug. Thanks.

Thanks for testing. I'll queue this up for 3.20 next week.

Johan

2015-02-26 15:49:03

by Johan Hovold

[permalink] [raw]
Subject: [PATCH] USB: pl2303: disable break on shutdown

Currently an enabled break state is not disabled on final close nor on
re-open and has to be disabled manually.

Fix this by disabling break on port shutdown.

Reported-by: Jari Ruusu <[email protected]>
Tested-by: Jari Ruusu <[email protected]>
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/usb/serial/pl2303.c | 18 +++++++++++++-----
1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/serial/pl2303.c b/drivers/usb/serial/pl2303.c
index 0f872e6b2c87..829604d11f3f 100644
--- a/drivers/usb/serial/pl2303.c
+++ b/drivers/usb/serial/pl2303.c
@@ -132,6 +132,7 @@ MODULE_DEVICE_TABLE(usb, id_table);
#define UART_OVERRUN_ERROR 0x40
#define UART_CTS 0x80

+static void pl2303_set_break(struct usb_serial_port *port, bool enable);

enum pl2303_type {
TYPE_01, /* Type 0 and 1 (difference unknown) */
@@ -615,6 +616,7 @@ static void pl2303_close(struct usb_serial_port *port)
{
usb_serial_generic_close(port);
usb_kill_urb(port->interrupt_in_urb);
+ pl2303_set_break(port, false);
}

static int pl2303_open(struct tty_struct *tty, struct usb_serial_port *port)
@@ -741,17 +743,16 @@ static int pl2303_ioctl(struct tty_struct *tty,
return -ENOIOCTLCMD;
}

-static void pl2303_break_ctl(struct tty_struct *tty, int break_state)
+static void pl2303_set_break(struct usb_serial_port *port, bool enable)
{
- struct usb_serial_port *port = tty->driver_data;
struct usb_serial *serial = port->serial;
u16 state;
int result;

- if (break_state == 0)
- state = BREAK_OFF;
- else
+ if (enable)
state = BREAK_ON;
+ else
+ state = BREAK_OFF;

dev_dbg(&port->dev, "%s - turning break %s\n", __func__,
state == BREAK_OFF ? "off" : "on");
@@ -763,6 +764,13 @@ static void pl2303_break_ctl(struct tty_struct *tty, int break_state)
dev_err(&port->dev, "error sending break = %d\n", result);
}

+static void pl2303_break_ctl(struct tty_struct *tty, int state)
+{
+ struct usb_serial_port *port = tty->driver_data;
+
+ pl2303_set_break(port, state);
+}
+
static void pl2303_update_line_status(struct usb_serial_port *port,
unsigned char *data,
unsigned int actual_length)
--
2.0.5