Re: [PATCH v3 1/1] USB: ch341: set tty baud speed according to tty struct

2015-03-01 Thread Nicolas PLANEL

   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


Re: [PATCH v3 1/1] USB: ch341: set tty baud speed according to tty struct

2015-02-26 Thread Johan Hovold
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 v3 1/1] USB: ch341: set tty baud speed according to tty struct

2015-02-26 Thread Nicolas PLANEL
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