Re: [PATCH v3 1/1] USB: ch341: set tty baud speed according to tty struct
Hi Johan,
Sorry for the delay ... quite busy.
I posted the lastest patch v4 on the ML.
Regards,
Nicolas
On 02/27/2015 01:09 AM, Johan Hovold wrote:
On Thu, Feb 26, 2015 at 10:02:41AM -0500, Nicolas PLANEL wrote:
The ch341_set_baudrate() function initialize the device baud speed according
to the value on priv->baud_rate. By default the ch341_open() set it to a
hardcoded value (DEFAULT_BAUD_RATE 9600). Unfortunately, the tty_struct is
not initialized with the same default value. (usually 56700)
This means that the tty_struct and the device baud rate generator are not
synchronized after opening the port.
Fixup is done by calling ch341_set_termios() if tty exist.
Remove unnecessary variable priv->baud_rate setup as it's already done by
ch341_port_probe().
Please try to break your commit message lines at about 72 cols or so.
Signed-off-by: Nicolas PLANEL
---
drivers/usb/serial/ch341.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c
index 2d72aa3564a3..5d28ca165fdf 100644
--- a/drivers/usb/serial/ch341.c
+++ b/drivers/usb/serial/ch341.c
@@ -84,6 +84,10 @@ struct ch341_private {
u8 line_status; /* active status of modem control inputs */
};
+static void ch341_set_termios(struct tty_struct *tty,
+ struct usb_serial_port *port,
+ struct ktermios *old_termios);
+
static int ch341_control_out(struct usb_device *dev, u8 request,
u16 value, u16 index)
{
@@ -309,8 +313,6 @@ static int ch341_open(struct tty_struct *tty, struct
usb_serial_port *port)
struct ch341_private *priv = usb_get_serial_port_data(port);
int r;
- priv->baud_rate = DEFAULT_BAUD_RATE;
-
r = ch341_configure(serial->dev, priv);
if (r)
goto out;
@@ -323,6 +325,9 @@ static int ch341_open(struct tty_struct *tty, struct
usb_serial_port *port)
if (r)
goto out;
+ if (tty)
+ ch341_set_termios(tty, port, NULL);
+
Thanks for the v3. Looking at the code now, I see that the calls to
set baudrate and handshake in open are still there. I think you should
remove those now that you call set_termios.
Care to fix that up and resend?
Thanks,
Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 0/1] USB: ch341: set tty baud speed according to tty struct
Nicolas PLANEL (1): USB: ch341: set tty baud speed according to tty struct drivers/usb/serial/ch341.c | 15 ++- 1 file changed, 6 insertions(+), 9 deletions(-) -- 2.3.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 1/1] USB: ch341: set tty baud speed according to tty struct
The ch341_set_baudrate() function initialize the device baud speed
according to the value on priv->baud_rate. By default the ch341_open() set
it to a hardcoded value (DEFAULT_BAUD_RATE 9600). Unfortunately, the
tty_struct is not initialized with the same default value. (usually 56700)
This means that the tty_struct and the device baud rate generator are not
synchronized after opening the port.
Fixup is done by calling ch341_set_termios() if tty exist.
Remove unnecessary variable priv->baud_rate setup as it's already done by
ch341_port_probe().
Remove unnecessary call to ch341_set_{handshake,baudrate}() in
ch341_open() as there already called in ch341_configure() and
ch341_set_termios()
Signed-off-by: Nicolas PLANEL
---
drivers/usb/serial/ch341.c | 15 ++-
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c
index 2d72aa3564a3..ede4f5fcfadd 100644
--- a/drivers/usb/serial/ch341.c
+++ b/drivers/usb/serial/ch341.c
@@ -84,6 +84,10 @@ struct ch341_private {
u8 line_status; /* active status of modem control inputs */
};
+static void ch341_set_termios(struct tty_struct *tty,
+ struct usb_serial_port *port,
+ struct ktermios *old_termios);
+
static int ch341_control_out(struct usb_device *dev, u8 request,
u16 value, u16 index)
{
@@ -309,19 +313,12 @@ static int ch341_open(struct tty_struct *tty, struct
usb_serial_port *port)
struct ch341_private *priv = usb_get_serial_port_data(port);
int r;
- priv->baud_rate = DEFAULT_BAUD_RATE;
-
r = ch341_configure(serial->dev, priv);
if (r)
goto out;
- r = ch341_set_handshake(serial->dev, priv->line_control);
- if (r)
- goto out;
-
- r = ch341_set_baudrate(serial->dev, priv);
- if (r)
- goto out;
+ if (tty)
+ ch341_set_termios(tty, port, NULL);
dev_dbg(&port->dev, "%s - submitting interrupt urb\n", __func__);
r = usb_submit_urb(port->interrupt_in_urb, GFP_KERNEL);
--
2.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 1/1] USB: ch341: set tty baud speed according to tty struct
The ch341_set_baudrate() function initialize the device baud speed according
to the value on priv->baud_rate. By default the ch341_open() set it to a
hardcoded value (DEFAULT_BAUD_RATE 9600). Unfortunately, the tty_struct is
not initialized with the same default value. (usually 56700)
This means that the tty_struct and the device baud rate generator are not
synchronized after opening the port.
Fixup is done by calling ch341_set_termios() if tty exist.
Remove unnecessary variable priv->baud_rate setup as it's already done by
ch341_port_probe().
Signed-off-by: Nicolas PLANEL
---
drivers/usb/serial/ch341.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c
index 2d72aa3564a3..5d28ca165fdf 100644
--- a/drivers/usb/serial/ch341.c
+++ b/drivers/usb/serial/ch341.c
@@ -84,6 +84,10 @@ struct ch341_private {
u8 line_status; /* active status of modem control inputs */
};
+static void ch341_set_termios(struct tty_struct *tty,
+ struct usb_serial_port *port,
+ struct ktermios *old_termios);
+
static int ch341_control_out(struct usb_device *dev, u8 request,
u16 value, u16 index)
{
@@ -309,8 +313,6 @@ static int ch341_open(struct tty_struct *tty, struct
usb_serial_port *port)
struct ch341_private *priv = usb_get_serial_port_data(port);
int r;
- priv->baud_rate = DEFAULT_BAUD_RATE;
-
r = ch341_configure(serial->dev, priv);
if (r)
goto out;
@@ -323,6 +325,9 @@ static int ch341_open(struct tty_struct *tty, struct
usb_serial_port *port)
if (r)
goto out;
+ if (tty)
+ ch341_set_termios(tty, port, NULL);
+
dev_dbg(&port->dev, "%s - submitting interrupt urb\n", __func__);
r = usb_submit_urb(port->interrupt_in_urb, GFP_KERNEL);
if (r) {
--
2.3.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 0/1] USB: ch341: set tty baud speed according to tty struct
Nicolas PLANEL (1): USB: ch341: set tty baud speed according to tty struct drivers/usb/serial/ch341.c | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) -- 2.3.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: usb : serial : ch341 : set tty baud speed according to tty struct
On 02/18/2015 02:05 PM, Karl Palsson wrote: Johan Hovold wrote: On Wed, Feb 18, 2015 at 11:32:38AM +0700, Johan Hovold wrote: On Tue, Feb 17, 2015 at 10:45:11PM -0500, Nicolas PLANEL wrote: I believe the fix should be implemented slightly differently however. Most usb-serial driver call set_termios from open to handle this issue. It looks like you could simply replace the calls to set baudrate and "handshake" in open with ch341_set_termios(tty, port, NULL); You currently need to make sure tty is not NULL in case the device is being used as a console by the way. Just skip the set_termios call in that case. I've already got most of this working in my branch overhauling this driver. I agree, the open call shouldn't be doing all the hard resetting of settings that it currently does. I've implemented more of set termios / get termios. It's not ready for submission yet, I've got a lot of debug added that needs to be rebased and squished out, and I still want to do more testing with hardware flow control and the rest of the modem status lines. https://github.com/karlp/linux/commits/ch341-3.18.6 By implementing proper reading of settings from the device, a lot of the private copies can just be dropped out altogether. Sincerely, Karl Palsson I will try your branch and send you my comment. Regards, Nicolas -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 0/1] USB: ch341: set tty baud speed according to tty struct
Nicolas PLANEL (1): USB: ch341: set tty baud speed according to tty struct drivers/usb/serial/ch341.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/1] USB: ch341: set tty baud speed according to tty struct
The ch341_set_baudrate() function initialize the device baud speed according
to the value on priv->baud_rate. By default the ch341_open() set it to a
hardcoded value (DEFAULT_BAUD_RATE 9600). Unfortunately, the tty_struct is
not initialized with the same default value. (usually 56700)
This means that the tty_struct and the device baud rate generator are not
synchronized after opening the port.
Fixup is done by calling ch341_set_termios() if tty exist.
Remove unnecessary variable priv->baud_rate setup as it's already done by
ch341_port_probe().
Signed-off-by: Nicolas PLANEL
---
drivers/usb/serial/ch341.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c
index 2d72aa3564a3..8cc22a35f7f3 100644
--- a/drivers/usb/serial/ch341.c
+++ b/drivers/usb/serial/ch341.c
@@ -309,8 +309,6 @@ static int ch341_open(struct tty_struct *tty, struct
usb_serial_port *port)
struct ch341_private *priv = usb_get_serial_port_data(port);
int r;
- priv->baud_rate = DEFAULT_BAUD_RATE;
-
r = ch341_configure(serial->dev, priv);
if (r)
goto out;
@@ -323,6 +321,9 @@ static int ch341_open(struct tty_struct *tty, struct
usb_serial_port *port)
if (r)
goto out;
+ if (tty)
+ ch341_set_termios(tty, port, NULL);
+
dev_dbg(&port->dev, "%s - submitting interrupt urb\n", __func__);
r = usb_submit_urb(port->interrupt_in_urb, GFP_KERNEL);
if (r) {
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: usb : serial : ch341 : set tty baud speed according to tty struct
From 467794e88dc08f61e1068c510c97baa9a12b841d Mon Sep 17 00:00:00 2001
From: Nicolas PLANEL
Date: Tue, 17 Feb 2015 00:59:14 -0500
Subject: [PATCH] USB: ch341: set tty baud speed according to tty struct
The ch341_set_baudrate() function initialize the device baud speed according
to the value on priv->baud_rate. By default the ch341_open() set it to a
hardcoded value (DEFAULT_BAUD_RATE 9600). Unfortunately, the tty_struct is
not initialized with the same default value. (usually 56700)
This means that the tty_struct and the device baud rate generator are not
synchronized after opening the port.
Fixup is elementary simple by calling tty_get_baud_rate() and
tty_encode_baud_rate() helper to sync up the baud rate during the
ch341_open() call.
Signed-off-by: Nicolas PLANEL
---
drivers/usb/serial/ch341.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c
index 2d72aa3564a3..2dbbbf6d4187 100644
--- a/drivers/usb/serial/ch341.c
+++ b/drivers/usb/serial/ch341.c
@@ -307,9 +307,13 @@ static int ch341_open(struct tty_struct *tty,
struct usb_serial_port *port)
{
struct usb_serial *serial = port->serial;
struct ch341_private *priv = usb_get_serial_port_data(port);
+unsigned baud;
int r;
priv->baud_rate = DEFAULT_BAUD_RATE;
+baud = tty_get_baud_rate(tty);
+if (baud)
+priv->baud_rate = baud;
r = ch341_configure(serial->dev, priv);
if (r)
@@ -323,6 +327,9 @@ static int ch341_open(struct tty_struct *tty, struct
usb_serial_port *port)
if (r)
goto out;
+baud = priv->baud_rate;
+tty_encode_baud_rate(tty, baud, baud);
+
dev_dbg(&port->dev, "%s - submitting interrupt urb\n", __func__);
r = usb_submit_urb(port->interrupt_in_urb, GFP_KERNEL);
if (r) {
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
usb : serial : ch341 : set tty baud speed according to tty struct
Hi,
According to git log your are currently the two maintainer of the ch341
usb serial driver.
Thanks to pyserial, that did not initialize the serial port speed like
minicom.
Basically pyserial only set the port speed (calling TIOCSSERIAL ioctl)
if the user request a different value different currently used.
(TIOCGSERIAL)
A long sentence in short code :
if ser.baudrate != user_requested_baudrate:
ser.baudrate = user_requested_baudrate
In short, if the python code open a serial port at a speed of 57600
baud, the hw speed line will be in reality at 9600 baud. (driver default
speed)
Regards,
Nicolas PLANEL
===
From 16df0d507a653c48f757635f717f2ba886f8c5ff Mon Sep 17 00:00:00 2001
From: Nicolas PLANEL
Date: Tue, 17 Feb 2015 00:59:14 -0500
Subject: [PATCH] USB: ch341: set tty baud speed according to tty struct
The ch341_set_baudrate() function initialize the device baud speed according
to the value on priv->baud_rate. By default the ch341_open() set it to a
hardcoded value (DEFAULT_BAUD_RATE 9600). Unfortunately, the tty_struct is
not initialized with the same default value. (usually 56700)
This means that the tty_struct and the device baud rate generator are not
synchronized after opening the port.
Fixup is elementary simple by calling tty_get_baud_rate() and
tty_encode_baud_rate() helper to sync up the baud rate during the
ch341_open() call.
---
drivers/usb/serial/ch341.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c
index 2d72aa3..2dbbbf6 100644
--- a/drivers/usb/serial/ch341.c
+++ b/drivers/usb/serial/ch341.c
@@ -307,9 +307,13 @@ static int ch341_open(struct tty_struct *tty,
struct usb_serial_port *port)
{
struct usb_serial *serial = port->serial;
struct ch341_private *priv = usb_get_serial_port_data(port);
+unsigned baud;
int r;
priv->baud_rate = DEFAULT_BAUD_RATE;
+baud = tty_get_baud_rate(tty);
+if (baud)
+priv->baud_rate = baud;
r = ch341_configure(serial->dev, priv);
if (r)
@@ -323,6 +327,9 @@ static int ch341_open(struct tty_struct *tty, struct
usb_serial_port *port)
if (r)
goto out;
+baud = priv->baud_rate;
+tty_encode_baud_rate(tty, baud, baud);
+
dev_dbg(&port->dev, "%s - submitting interrupt urb\n", __func__);
r = usb_submit_urb(port->interrupt_in_urb, GFP_KERNEL);
if (r) {
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
