Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1375380 > unrolled thread

[PATCH v3 06/13] USB: ch341: add support for parity, frame length, stop bits

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v3 06/13] USB: ch341: add support for parity, frame length, stop bits Grigori Goronzy <greg@chown.ath.cx> - 2016-04-11 01:50 +0200
    Re: [PATCH v3 06/13] USB: ch341: add support for parity, frame length,  stop bits Karl Palsson <karlp@tweak.net.au> - 2016-04-11 19:30 +0200
      Re: [PATCH v3 06/13] USB: ch341: add support for parity, frame  length, stop bits Grigori Goronzy <greg@chown.ath.cx> - 2016-04-11 20:30 +0200

#1375380 — [PATCH v3 06/13] USB: ch341: add support for parity, frame length, stop bits

With the new reinitialization method, configuring parity, different
frame lengths and different stop bit settings work as expected on
both CH340G and CH341A.  This has been extensively tested with a
logic analyzer.

v2: only set mark/space when parity is enabled, simplifications,
patch termios HW flags.

Tested-by: Ryan Barber <rfb@skyscraper.nu>
Signed-off-by: Grigori Goronzy <greg@chown.ath.cx>
---
 drivers/usb/serial/ch341.c | 40 ++++++++++++++++++++++++++++++----------
 1 file changed, 30 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c
index 6181616..99b4621 100644
--- a/drivers/usb/serial/ch341.c
+++ b/drivers/usb/serial/ch341.c
@@ -341,7 +341,6 @@ static void ch341_set_termios(struct tty_struct *tty,
 		struct usb_serial_port *port, struct ktermios *old_termios)
 {
 	struct ch341_private *priv = usb_get_serial_port_data(port);
-	unsigned baud_rate;
 	unsigned long flags;
 	unsigned char ctrl;
 	int r;
@@ -350,13 +349,39 @@ static void ch341_set_termios(struct tty_struct *tty,
 	if (old_termios && !tty_termios_hw_change(&tty->termios, old_termios))
 		return;
 
-	baud_rate = tty_get_baud_rate(tty);
+	priv->baud_rate = tty_get_baud_rate(tty);
 
-	priv->baud_rate = baud_rate;
+	ctrl = CH341_LCR_ENABLE_RX | CH341_LCR_ENABLE_TX;
 
-	ctrl = CH341_LCR_ENABLE_RX | CH341_LCR_ENABLE_TX | CH341_LCR_CS8;
+	switch (C_CSIZE(tty)) {
+	case CS5:
+		ctrl |= CH341_LCR_CS5;
+		break;
+	case CS6:
+		ctrl |= CH341_LCR_CS6;
+		break;
+	case CS7:
+		ctrl |= CH341_LCR_CS7;
+		break;
+	default:
+		tty->termios.c_cflag |= CS8;
+	case CS8:
+		ctrl |= CH341_LCR_CS8;
+		break;
+	}
+
+	if (C_PARENB(tty)) {
+		ctrl |= CH341_LCR_ENABLE_PAR;
+		if (C_PARODD(tty))
+			ctrl |= CH341_LCR_PAR_EVEN;
+		if (C_CMSPAR(tty))
+			ctrl |= CH341_LCR_MARK_SPACE;
+	}
+
+	if (C_CSTOPB(tty))
+		ctrl |= CH341_LCR_STOP_BITS_2;
 
-	if (baud_rate) {
+	if (priv->baud_rate) {
 		spin_lock_irqsave(&priv->lock, flags);
 		priv->line_control |= (CH341_BIT_DTR | CH341_BIT_RTS);
 		spin_unlock_irqrestore(&priv->lock, flags);
@@ -373,11 +398,6 @@ static void ch341_set_termios(struct tty_struct *tty,
 
 	ch341_set_handshake(port->serial->dev, priv->line_control);
 
-	/* Unimplemented:
-	 * (cflag & CSIZE) : data bits [5, 8]
-	 * (cflag & PARENB) : parity {NONE, EVEN, ODD}
-	 * (cflag & CSTOPB) : stop bits [1, 2]
-	 */
 }
 
 static void ch341_break_ctl(struct tty_struct *tty, int break_state)
-- 
1.9.1

[toc] | [next] | [standalone]

#1376172 — Re: [PATCH v3 06/13] USB: ch341: add support for parity, frame length, stop bits

[Multipart message — attachments visible in raw view] — view raw

Sorry if you get this twice, I was having some client problems,
but wanted to make sure you got this one...


Grigori Goronzy <greg@chown.ath.cx> wrote:
> With the new reinitialization method, configuring parity,
> different frame lengths and different stop bit settings work as
> expected on both CH340G and CH341A. This has been extensively
> tested with a logic analyzer.
> 
> v2: only set mark/space when parity is enabled,
> simplifications, patch termios HW flags.
> 
> Tested-by: Ryan Barber <rfb@skyscraper.nu>
> Signed-off-by: Grigori Goronzy <greg@chown.ath.cx>
> ---
>  drivers/usb/serial/ch341.c | 40 ++++++++++++++++++++++++++++++----------
>  1 file changed, 30 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/usb/serial/ch341.c
> b/drivers/usb/serial/ch341.c index 6181616..99b4621 100644
> --- a/drivers/usb/serial/ch341.c
> +++ b/drivers/usb/serial/ch341.c
> @@ -341,7 +341,6 @@ static void ch341_set_termios(struct tty_struct *tty,
>  		struct usb_serial_port *port, struct ktermios *old_termios)
>  {
>  	struct ch341_private *priv = usb_get_serial_port_data(port);
> -	unsigned baud_rate;
>  	unsigned long flags;
>  	unsigned char ctrl;
>  	int r;
> @@ -350,13 +349,39 @@ static void ch341_set_termios(struct tty_struct *tty,
>  	if (old_termios && !tty_termios_hw_change(&tty->termios, old_termios))
>  		return;
>  
> -	baud_rate = tty_get_baud_rate(tty);
> +	priv->baud_rate = tty_get_baud_rate(tty);
>  
> -	priv->baud_rate = baud_rate;
> +	ctrl = CH341_LCR_ENABLE_RX | CH341_LCR_ENABLE_TX;
>  
> -	ctrl = CH341_LCR_ENABLE_RX | CH341_LCR_ENABLE_TX | CH341_LCR_CS8;
> +	switch (C_CSIZE(tty)) {
> +	case CS5:
> +		ctrl |= CH341_LCR_CS5;
> +		break;
> +	case CS6:
> +		ctrl |= CH341_LCR_CS6;
> +		break;
> +	case CS7:
> +		ctrl |= CH341_LCR_CS7;
> +		break;
> +	default:
> +		tty->termios.c_cflag |= CS8;
> +	case CS8:
> +		ctrl |= CH341_LCR_CS8;
> +		break;
> +	}
> +
> +	if (C_PARENB(tty)) {
> +		ctrl |= CH341_LCR_ENABLE_PAR;
> +		if (C_PARODD(tty))
> +			ctrl |= CH341_LCR_PAR_EVEN;

Are you sure this does the right thing now? this is, as best as I
can tell, the inverse of what you had earlier, and doesn't read
right, if this is working, then I suggest renaming _LCR_PAR_EVEN
to LCR_PAR_ODD?

Cheers,
Karl P


> +		if (C_CMSPAR(tty))
> +			ctrl |= CH341_LCR_MARK_SPACE;
> +	}
> +
> +	if (C_CSTOPB(tty))
> +		ctrl |= CH341_LCR_STOP_BITS_2;
>  
> -	if (baud_rate) {
> +	if (priv->baud_rate) {
>  		spin_lock_irqsave(&priv->lock, flags);
>  		priv->line_control |= (CH341_BIT_DTR | CH341_BIT_RTS);
>  		spin_unlock_irqrestore(&priv->lock, flags);
> @@ -373,11 +398,6 @@ static void ch341_set_termios(struct tty_struct *tty,
>  
>  	ch341_set_handshake(port->serial->dev, priv->line_control);
>  
> -	/* Unimplemented:
> -	 * (cflag & CSIZE) : data bits [5, 8]
> -	 * (cflag & PARENB) : parity {NONE, EVEN, ODD}
> -	 * (cflag & CSTOPB) : stop bits [1, 2]
> -	 */
>  }
>  
>  static void ch341_break_ctl(struct tty_struct *tty, int break_state)
> -- 
> 1.9.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe
> linux-usb" in the body of a message to
> majordomo@vger.kernel.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html

[toc] | [prev] | [next] | [standalone]

#1376203 — Re: [PATCH v3 06/13] USB: ch341: add support for parity, frame length, stop bits

On 2016-04-11 19:25, Karl Palsson wrote:
> Sorry if you get this twice, I was having some client problems,
> but wanted to make sure you got this one...
> 
> 
> Grigori Goronzy <greg@chown.ath.cx> wrote:
>> With the new reinitialization method, configuring parity,
>> different frame lengths and different stop bit settings work as
>> expected on both CH340G and CH341A. This has been extensively
>> tested with a logic analyzer.
>> 
>> v2: only set mark/space when parity is enabled,
>> simplifications, patch termios HW flags.
>> 
>> Tested-by: Ryan Barber <rfb@skyscraper.nu>
>> Signed-off-by: Grigori Goronzy <greg@chown.ath.cx>
>> ---
>>  drivers/usb/serial/ch341.c | 40 
>> ++++++++++++++++++++++++++++++----------
>>  1 file changed, 30 insertions(+), 10 deletions(-)
>> 
>> diff --git a/drivers/usb/serial/ch341.c
>> b/drivers/usb/serial/ch341.c index 6181616..99b4621 100644
>> --- a/drivers/usb/serial/ch341.c
>> +++ b/drivers/usb/serial/ch341.c
>> @@ -341,7 +341,6 @@ static void ch341_set_termios(struct tty_struct 
>> *tty,
>>  		struct usb_serial_port *port, struct ktermios *old_termios)
>>  {
>>  	struct ch341_private *priv = usb_get_serial_port_data(port);
>> -	unsigned baud_rate;
>>  	unsigned long flags;
>>  	unsigned char ctrl;
>>  	int r;
>> @@ -350,13 +349,39 @@ static void ch341_set_termios(struct tty_struct 
>> *tty,
>>  	if (old_termios && !tty_termios_hw_change(&tty->termios, 
>> old_termios))
>>  		return;
>> 
>> -	baud_rate = tty_get_baud_rate(tty);
>> +	priv->baud_rate = tty_get_baud_rate(tty);
>> 
>> -	priv->baud_rate = baud_rate;
>> +	ctrl = CH341_LCR_ENABLE_RX | CH341_LCR_ENABLE_TX;
>> 
>> -	ctrl = CH341_LCR_ENABLE_RX | CH341_LCR_ENABLE_TX | CH341_LCR_CS8;
>> +	switch (C_CSIZE(tty)) {
>> +	case CS5:
>> +		ctrl |= CH341_LCR_CS5;
>> +		break;
>> +	case CS6:
>> +		ctrl |= CH341_LCR_CS6;
>> +		break;
>> +	case CS7:
>> +		ctrl |= CH341_LCR_CS7;
>> +		break;
>> +	default:
>> +		tty->termios.c_cflag |= CS8;
>> +	case CS8:
>> +		ctrl |= CH341_LCR_CS8;
>> +		break;
>> +	}
>> +
>> +	if (C_PARENB(tty)) {
>> +		ctrl |= CH341_LCR_ENABLE_PAR;
>> +		if (C_PARODD(tty))
>> +			ctrl |= CH341_LCR_PAR_EVEN;
> 
> Are you sure this does the right thing now? this is, as best as I
> can tell, the inverse of what you had earlier, and doesn't read
> right, if this is working, then I suggest renaming _LCR_PAR_EVEN
> to LCR_PAR_ODD?
> 

No, this is absolutely wrong, of course.  I only did some sporadic 
testing because my refactoring wasn't supposed to change functionality.  
Thanks for pointing it out!

Grigori

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web