linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3] usb-serial:cp210x: add support to software flow control
@ 2020-08-20  7:52 Sheng Long Wang
  2020-08-21  5:32 ` Jan Kiszka
  2020-08-24  9:38 ` Johan Hovold
  0 siblings, 2 replies; 9+ messages in thread
From: Sheng Long Wang @ 2020-08-20  7:52 UTC (permalink / raw)
  To: johan, gregkh; +Cc: linux-usb, linux-kernel, jan.kiszka, Wang Sheng Long

From: Wang Sheng Long <shenglong.wang.ext@siemens.com>

When data is transmitted between two serial ports,
the phenomenon of data loss often occurs. The two kinds
of flow control commonly used in serial communication
are hardware flow control and software flow control.

In serial communication, If you only use RX/TX/GND Pins, you
can't do hardware flow. So we often used software flow control
and prevent data loss. The user sets the software flow control
through the application program, and the application program
sets the software flow control mode for the serial port
chip through the driver.

For the cp210 serial port chip, its driver lacks the
software flow control setting code, so the user cannot set
the software flow control function through the application
program. This adds the missing software flow control.

Signed-off-by: Wang Sheng Long <shenglong.wang.ext@siemens.com>

Changes in v3:
-fixed code style, It mainly adjusts the code style acccording
 to kernel specification.
---
 drivers/usb/serial/cp210x.c | 118 ++++++++++++++++++++++++++++++++++--
 1 file changed, 113 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c
index e732949f65..c66a0e0fb9 100644
--- a/drivers/usb/serial/cp210x.c
+++ b/drivers/usb/serial/cp210x.c
@@ -380,6 +380,9 @@ static struct usb_serial_driver * const serial_drivers[] = {
 #define CP210X_PARTNUM_CP2102N_QFN20	0x22
 #define CP210X_PARTNUM_UNKNOWN	0xFF
 
+#define CP210X_VSTART	0x11
+#define CP210X_VSTOP	0x13
+
 /* CP210X_GET_COMM_STATUS returns these 0x13 bytes */
 struct cp210x_comm_status {
 	__le32   ulErrors;
@@ -391,6 +394,15 @@ struct cp210x_comm_status {
 	u8       bReserved;
 } __packed;
 
+struct cp210x_chars_response {
+	u8	eofchar;
+	u8	errochar;
+	u8	breakchar;
+	u8	eventchar;
+	u8	xonchar;
+	u8	xoffchar;
+} __packed;
+
 /*
  * CP210X_PURGE - 16 bits passed in wValue of USB request.
  * SiLabs app note AN571 gives a strange description of the 4 bits:
@@ -624,6 +636,45 @@ static int cp210x_read_vendor_block(struct usb_serial *serial, u8 type, u16 val,
 	return result;
 }
 
+/*
+ * Read and Write Character Responses operate
+ * Register SET_CHARS/GET_CHATS
+ */
+static int cp210x_operate_chars_block(struct usb_serial_port *port,
+				u8 req, u8 type, void *buf, int bufsize)
+{
+	struct usb_serial *serial = port->serial;
+	struct cp210x_port_private *port_priv = usb_get_serial_port_data(port);
+	void *dmabuf;
+	int result;
+
+	dmabuf = kmemdup(buf, bufsize, GFP_KERNEL);
+	if (!dmabuf)
+		return -ENOMEM;
+
+	result = usb_control_msg(serial->dev,
+				usb_rcvctrlpipe(serial->dev, 0),
+				req, type, 0, port_priv->bInterfaceNumber,
+				dmabuf, bufsize, USB_CTRL_SET_TIMEOUT);
+
+	if (result == bufsize) {
+		if (type == REQTYPE_DEVICE_TO_HOST)
+			memcpy(buf, dmabuf, bufsize);
+
+		result = 0;
+	} else {
+		dev_err(&port->dev, "failed get req 0x%x size %d status: %d\n",
+			req, bufsize, result);
+		if (result >= 0)
+			result = -EIO;
+
+	}
+
+	kfree(dmabuf);
+
+	return result;
+}
+
 /*
  * Writes any 16-bit CP210X_ register (req) whose value is passed
  * entirely in the wValue field of the USB request.
@@ -1134,11 +1185,17 @@ static void cp210x_set_termios(struct tty_struct *tty,
 		struct usb_serial_port *port, struct ktermios *old_termios)
 {
 	struct device *dev = &port->dev;
-	unsigned int cflag, old_cflag;
+	struct cp210x_chars_response charsres;
+	struct cp210x_flow_ctl flow_ctl;
+	unsigned int cflag, old_cflag, iflag;
 	u16 bits;
+	int result;
+	u32 ctl_hs;
+	u32 flow_repl;
 
 	cflag = tty->termios.c_cflag;
 	old_cflag = old_termios->c_cflag;
+	iflag = tty->termios.c_iflag;
 
 	if (tty->termios.c_ospeed != old_termios->c_ospeed)
 		cp210x_change_speed(tty, port, old_termios);
@@ -1212,10 +1269,6 @@ static void cp210x_set_termios(struct tty_struct *tty,
 	}
 
 	if ((cflag & CRTSCTS) != (old_cflag & CRTSCTS)) {
-		struct cp210x_flow_ctl flow_ctl;
-		u32 ctl_hs;
-		u32 flow_repl;
-
 		cp210x_read_reg_block(port, CP210X_GET_FLOW, &flow_ctl,
 				sizeof(flow_ctl));
 		ctl_hs = le32_to_cpu(flow_ctl.ulControlHandshake);
@@ -1252,6 +1305,61 @@ static void cp210x_set_termios(struct tty_struct *tty,
 				sizeof(flow_ctl));
 	}
 
+	/*
+	 * Set Software  Flow  Control
+	 * Check the IXOFF/IXON status in the iflag component of the
+	 * termios structure.
+	 *
+	 */
+	if ((iflag & IXOFF) || (iflag & IXON)) {
+
+		result = cp210x_operate_chars_block(port,
+						CP210X_GET_CHARS,
+						REQTYPE_DEVICE_TO_HOST,
+						&charsres,
+						sizeof(charsres));
+
+		if (result < 0) {
+			dev_err(dev, "Read Characrter Responses failed\n");
+			return;
+		}
+		charsres.xonchar  = CP210X_VSTART;
+		charsres.xoffchar = CP210X_VSTOP;
+		result = cp210x_operate_chars_block(port,
+						CP210X_SET_CHARS,
+						REQTYPE_HOST_TO_INTERFACE,
+						&charsres,
+						sizeof(charsres));
+		if (result < 0) {
+			memset(&charsres, 0, sizeof(charsres));
+			dev_err(dev, "Write Characrter Responses failed\n");
+			return;
+		}
+
+		/*Set  Rx/Tx Flow Contrl  Flag in ulFlowReplace*/
+		cp210x_read_reg_block(port,
+					CP210X_GET_FLOW,
+					&flow_ctl,
+					sizeof(flow_ctl));
+
+		flow_repl = le32_to_cpu(flow_ctl.ulFlowReplace);
+
+		if (iflag & IXOFF)
+			flow_repl |= CP210X_SERIAL_AUTO_RECEIVE;
+		else
+			flow_repl &= ~CP210X_SERIAL_AUTO_RECEIVE;
+
+		if (iflag & IXON)
+			flow_repl |= CP210X_SERIAL_AUTO_TRANSMIT;
+		else
+			flow_repl &= ~CP210X_SERIAL_AUTO_TRANSMIT;
+
+		flow_ctl.ulFlowReplace = cpu_to_le32(flow_repl);
+		cp210x_write_reg_block(port,
+					CP210X_SET_FLOW,
+					&flow_ctl,
+					sizeof(flow_ctl));
+	}
 }
 
 static int cp210x_tiocmset(struct tty_struct *tty,
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v3] usb-serial:cp210x: add support to software flow control
  2020-08-20  7:52 [PATCH v3] usb-serial:cp210x: add support to software flow control Sheng Long Wang
@ 2020-08-21  5:32 ` Jan Kiszka
  2020-08-24  9:09   ` Johan Hovold
  2020-08-24  9:38 ` Johan Hovold
  1 sibling, 1 reply; 9+ messages in thread
From: Jan Kiszka @ 2020-08-21  5:32 UTC (permalink / raw)
  To: Sheng Long Wang, Wang Sheng Long; +Cc: johan, gregkh, linux-usb, linux-kernel

On 20.08.20 09:52, Sheng Long Wang wrote:
> From: Wang Sheng Long <shenglong.wang.ext@siemens.com>
> 
> When data is transmitted between two serial ports,
> the phenomenon of data loss often occurs. The two kinds
> of flow control commonly used in serial communication
> are hardware flow control and software flow control.
> 
> In serial communication, If you only use RX/TX/GND Pins, you
> can't do hardware flow. So we often used software flow control
> and prevent data loss. The user sets the software flow control
> through the application program, and the application program
> sets the software flow control mode for the serial port
> chip through the driver.
> 
> For the cp210 serial port chip, its driver lacks the
> software flow control setting code, so the user cannot set
> the software flow control function through the application
> program. This adds the missing software flow control.
> 
> Signed-off-by: Wang Sheng Long <shenglong.wang.ext@siemens.com>
> 
> Changes in v3:
> -fixed code style, It mainly adjusts the code style acccording
>  to kernel specification.

Patch does not apply. You forgot to rebase over latest tty/tty-next or
linux master.

Jan

> ---
>  drivers/usb/serial/cp210x.c | 118 ++++++++++++++++++++++++++++++++++--
>  1 file changed, 113 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c
> index e732949f65..c66a0e0fb9 100644
> --- a/drivers/usb/serial/cp210x.c
> +++ b/drivers/usb/serial/cp210x.c
> @@ -380,6 +380,9 @@ static struct usb_serial_driver * const serial_drivers[] = {
>  #define CP210X_PARTNUM_CP2102N_QFN20	0x22
>  #define CP210X_PARTNUM_UNKNOWN	0xFF
>  
> +#define CP210X_VSTART	0x11
> +#define CP210X_VSTOP	0x13
> +
>  /* CP210X_GET_COMM_STATUS returns these 0x13 bytes */
>  struct cp210x_comm_status {
>  	__le32   ulErrors;
> @@ -391,6 +394,15 @@ struct cp210x_comm_status {
>  	u8       bReserved;
>  } __packed;
>  
> +struct cp210x_chars_response {
> +	u8	eofchar;
> +	u8	errochar;
> +	u8	breakchar;
> +	u8	eventchar;
> +	u8	xonchar;
> +	u8	xoffchar;
> +} __packed;
> +
>  /*
>   * CP210X_PURGE - 16 bits passed in wValue of USB request.
>   * SiLabs app note AN571 gives a strange description of the 4 bits:
> @@ -624,6 +636,45 @@ static int cp210x_read_vendor_block(struct usb_serial *serial, u8 type, u16 val,
>  	return result;
>  }
>  
> +/*
> + * Read and Write Character Responses operate
> + * Register SET_CHARS/GET_CHATS
> + */
> +static int cp210x_operate_chars_block(struct usb_serial_port *port,
> +				u8 req, u8 type, void *buf, int bufsize)
> +{
> +	struct usb_serial *serial = port->serial;
> +	struct cp210x_port_private *port_priv = usb_get_serial_port_data(port);
> +	void *dmabuf;
> +	int result;
> +
> +	dmabuf = kmemdup(buf, bufsize, GFP_KERNEL);
> +	if (!dmabuf)
> +		return -ENOMEM;
> +
> +	result = usb_control_msg(serial->dev,
> +				usb_rcvctrlpipe(serial->dev, 0),
> +				req, type, 0, port_priv->bInterfaceNumber,
> +				dmabuf, bufsize, USB_CTRL_SET_TIMEOUT);
> +
> +	if (result == bufsize) {
> +		if (type == REQTYPE_DEVICE_TO_HOST)
> +			memcpy(buf, dmabuf, bufsize);
> +
> +		result = 0;
> +	} else {
> +		dev_err(&port->dev, "failed get req 0x%x size %d status: %d\n",
> +			req, bufsize, result);
> +		if (result >= 0)
> +			result = -EIO;
> +
> +	}
> +
> +	kfree(dmabuf);
> +
> +	return result;
> +}
> +
>  /*
>   * Writes any 16-bit CP210X_ register (req) whose value is passed
>   * entirely in the wValue field of the USB request.
> @@ -1134,11 +1185,17 @@ static void cp210x_set_termios(struct tty_struct *tty,
>  		struct usb_serial_port *port, struct ktermios *old_termios)
>  {
>  	struct device *dev = &port->dev;
> -	unsigned int cflag, old_cflag;
> +	struct cp210x_chars_response charsres;
> +	struct cp210x_flow_ctl flow_ctl;
> +	unsigned int cflag, old_cflag, iflag;
>  	u16 bits;
> +	int result;
> +	u32 ctl_hs;
> +	u32 flow_repl;
>  
>  	cflag = tty->termios.c_cflag;
>  	old_cflag = old_termios->c_cflag;
> +	iflag = tty->termios.c_iflag;
>  
>  	if (tty->termios.c_ospeed != old_termios->c_ospeed)
>  		cp210x_change_speed(tty, port, old_termios);
> @@ -1212,10 +1269,6 @@ static void cp210x_set_termios(struct tty_struct *tty,
>  	}
>  
>  	if ((cflag & CRTSCTS) != (old_cflag & CRTSCTS)) {
> -		struct cp210x_flow_ctl flow_ctl;
> -		u32 ctl_hs;
> -		u32 flow_repl;
> -
>  		cp210x_read_reg_block(port, CP210X_GET_FLOW, &flow_ctl,
>  				sizeof(flow_ctl));
>  		ctl_hs = le32_to_cpu(flow_ctl.ulControlHandshake);
> @@ -1252,6 +1305,61 @@ static void cp210x_set_termios(struct tty_struct *tty,
>  				sizeof(flow_ctl));
>  	}
>  
> +	/*
> +	 * Set Software  Flow  Control
> +	 * Check the IXOFF/IXON status in the iflag component of the
> +	 * termios structure.
> +	 *
> +	 */
> +	if ((iflag & IXOFF) || (iflag & IXON)) {
> +
> +		result = cp210x_operate_chars_block(port,
> +						CP210X_GET_CHARS,
> +						REQTYPE_DEVICE_TO_HOST,
> +						&charsres,
> +						sizeof(charsres));
> +
> +		if (result < 0) {
> +			dev_err(dev, "Read Characrter Responses failed\n");
> +			return;
> +		}
> +		charsres.xonchar  = CP210X_VSTART;
> +		charsres.xoffchar = CP210X_VSTOP;
> +		result = cp210x_operate_chars_block(port,
> +						CP210X_SET_CHARS,
> +						REQTYPE_HOST_TO_INTERFACE,
> +						&charsres,
> +						sizeof(charsres));
> +		if (result < 0) {
> +			memset(&charsres, 0, sizeof(charsres));
> +			dev_err(dev, "Write Characrter Responses failed\n");
> +			return;
> +		}
> +
> +		/*Set  Rx/Tx Flow Contrl  Flag in ulFlowReplace*/
> +		cp210x_read_reg_block(port,
> +					CP210X_GET_FLOW,
> +					&flow_ctl,
> +					sizeof(flow_ctl));
> +
> +		flow_repl = le32_to_cpu(flow_ctl.ulFlowReplace);
> +
> +		if (iflag & IXOFF)
> +			flow_repl |= CP210X_SERIAL_AUTO_RECEIVE;
> +		else
> +			flow_repl &= ~CP210X_SERIAL_AUTO_RECEIVE;
> +
> +		if (iflag & IXON)
> +			flow_repl |= CP210X_SERIAL_AUTO_TRANSMIT;
> +		else
> +			flow_repl &= ~CP210X_SERIAL_AUTO_TRANSMIT;
> +
> +		flow_ctl.ulFlowReplace = cpu_to_le32(flow_repl);
> +		cp210x_write_reg_block(port,
> +					CP210X_SET_FLOW,
> +					&flow_ctl,
> +					sizeof(flow_ctl));
> +	}
>  }
>  
>  static int cp210x_tiocmset(struct tty_struct *tty,
> 

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3] usb-serial:cp210x: add support to software flow control
  2020-08-21  5:32 ` Jan Kiszka
@ 2020-08-24  9:09   ` Johan Hovold
  2020-08-24  9:28     ` Wang, Sheng Long
  0 siblings, 1 reply; 9+ messages in thread
From: Johan Hovold @ 2020-08-24  9:09 UTC (permalink / raw)
  To: Jan Kiszka
  Cc: Sheng Long Wang, Wang Sheng Long, johan, gregkh, linux-usb, linux-kernel

On Fri, Aug 21, 2020 at 07:32:58AM +0200, Jan Kiszka wrote:
> On 20.08.20 09:52, Sheng Long Wang wrote:
> > From: Wang Sheng Long <shenglong.wang.ext@siemens.com>
> > 
> > When data is transmitted between two serial ports,
> > the phenomenon of data loss often occurs. The two kinds
> > of flow control commonly used in serial communication
> > are hardware flow control and software flow control.
> > 
> > In serial communication, If you only use RX/TX/GND Pins, you
> > can't do hardware flow. So we often used software flow control
> > and prevent data loss. The user sets the software flow control
> > through the application program, and the application program
> > sets the software flow control mode for the serial port
> > chip through the driver.
> > 
> > For the cp210 serial port chip, its driver lacks the
> > software flow control setting code, so the user cannot set
> > the software flow control function through the application
> > program. This adds the missing software flow control.
> > 
> > Signed-off-by: Wang Sheng Long <shenglong.wang.ext@siemens.com>
> > 
> > Changes in v3:
> > -fixed code style, It mainly adjusts the code style acccording
> >  to kernel specification.
> 
> Patch does not apply. You forgot to rebase over latest tty/tty-next or
> linux master.

That should be the usb-next branch of the usb-serial tree:

	https://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git/

or linux-next (or, currently, Linus's master branch).

You can use "scripts/get_maintainer.sh --scm" to determine which tree to
base your work against.

Johan

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH v3] usb-serial:cp210x: add support to software flow control
  2020-08-24  9:09   ` Johan Hovold
@ 2020-08-24  9:28     ` Wang, Sheng Long
  2020-08-24  9:36       ` Jan Kiszka
  0 siblings, 1 reply; 9+ messages in thread
From: Wang, Sheng Long @ 2020-08-24  9:28 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Sheng Long Wang, gregkh, linux-usb, linux-kernel, Kiszka, Jan

Hi Johan,

    Thanks for your reminding. I am adjusting patch according to the latest Linux Master branch.

With best regards,
Wang Sheng Long
Siemens Ltd., China
RC-CN DI FA R&D SW
Tianyuan road No.99
611731 CHENGDU, China
Mobil: +86 15281074996
mailto:shenglong.wang.ext@siemens.com



-----Original Message-----
From: Johan Hovold <johan@kernel.org> 
Sent: Monday, August 24, 2020 5:10 PM
To: Kiszka, Jan (CT RDA IOT SES-DE) <jan.kiszka@siemens.com>
Cc: Sheng Long Wang <china_shenglong@163.com>; Wang, Sheng Long (EXT) (RC-CN DI FA R&D SW) <shenglong.wang.ext@siemens.com>; johan@kernel.org; gregkh@linuxfoundation.org; linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3] usb-serial:cp210x: add support to software flow control

On Fri, Aug 21, 2020 at 07:32:58AM +0200, Jan Kiszka wrote:
> On 20.08.20 09:52, Sheng Long Wang wrote:
> > From: Wang Sheng Long <shenglong.wang.ext@siemens.com>
> > 
> > When data is transmitted between two serial ports, the phenomenon of 
> > data loss often occurs. The two kinds of flow control commonly used 
> > in serial communication are hardware flow control and software flow 
> > control.
> > 
> > In serial communication, If you only use RX/TX/GND Pins, you can't 
> > do hardware flow. So we often used software flow control and prevent 
> > data loss. The user sets the software flow control through the 
> > application program, and the application program sets the software 
> > flow control mode for the serial port chip through the driver.
> > 
> > For the cp210 serial port chip, its driver lacks the software flow 
> > control setting code, so the user cannot set the software flow 
> > control function through the application program. This adds the 
> > missing software flow control.
> > 
> > Signed-off-by: Wang Sheng Long <shenglong.wang.ext@siemens.com>
> > 
> > Changes in v3:
> > -fixed code style, It mainly adjusts the code style acccording  to 
> > kernel specification.
> 
> Patch does not apply. You forgot to rebase over latest tty/tty-next or 
> linux master.

That should be the usb-next branch of the usb-serial tree:

	https://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git/

or linux-next (or, currently, Linus's master branch).

You can use "scripts/get_maintainer.sh --scm" to determine which tree to base your work against.

Johan

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3] usb-serial:cp210x: add support to software flow control
  2020-08-24  9:28     ` Wang, Sheng Long
@ 2020-08-24  9:36       ` Jan Kiszka
  2020-08-24  9:43         ` Johan Hovold
  0 siblings, 1 reply; 9+ messages in thread
From: Jan Kiszka @ 2020-08-24  9:36 UTC (permalink / raw)
  To: Wang, Sheng Long (EXT) (RC-CN DI FA R&D SW), Johan Hovold
  Cc: Sheng Long Wang, gregkh, linux-usb, linux-kernel

On 24.08.20 11:28, Wang, Sheng Long (EXT) (RC-CN DI FA R&D SW) wrote:
> Hi Johan,
> 
>     Thanks for your reminding. I am adjusting patch according to the latest Linux Master branch.

Use git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git, branch
is likely usb-linus, as Johan requested - just in case there is another
conflict due to a patch not yet in Linux master but in that tree.

> 
> With best regards,
> Wang Sheng Long
> Siemens Ltd., China
> RC-CN DI FA R&D SW
> Tianyuan road No.99
> 611731 CHENGDU, China
> Mobil: +86 15281074996
> mailto:shenglong.wang.ext@siemens.com
> 
> 
> 
> -----Original Message-----
> From: Johan Hovold <johan@kernel.org> 
> Sent: Monday, August 24, 2020 5:10 PM
> To: Kiszka, Jan (CT RDA IOT SES-DE) <jan.kiszka@siemens.com>
> Cc: Sheng Long Wang <china_shenglong@163.com>; Wang, Sheng Long (EXT) (RC-CN DI FA R&D SW) <shenglong.wang.ext@siemens.com>; johan@kernel.org; gregkh@linuxfoundation.org; linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v3] usb-serial:cp210x: add support to software flow control
> 
> On Fri, Aug 21, 2020 at 07:32:58AM +0200, Jan Kiszka wrote:
>> On 20.08.20 09:52, Sheng Long Wang wrote:
>>> From: Wang Sheng Long <shenglong.wang.ext@siemens.com>
>>>
>>> When data is transmitted between two serial ports, the phenomenon of 
>>> data loss often occurs. The two kinds of flow control commonly used 
>>> in serial communication are hardware flow control and software flow 
>>> control.
>>>
>>> In serial communication, If you only use RX/TX/GND Pins, you can't 
>>> do hardware flow. So we often used software flow control and prevent 
>>> data loss. The user sets the software flow control through the 
>>> application program, and the application program sets the software 
>>> flow control mode for the serial port chip through the driver.
>>>
>>> For the cp210 serial port chip, its driver lacks the software flow 
>>> control setting code, so the user cannot set the software flow 
>>> control function through the application program. This adds the 
>>> missing software flow control.
>>>
>>> Signed-off-by: Wang Sheng Long <shenglong.wang.ext@siemens.com>
>>>
>>> Changes in v3:
>>> -fixed code style, It mainly adjusts the code style acccording  to 
>>> kernel specification.
>>
>> Patch does not apply. You forgot to rebase over latest tty/tty-next or 
>> linux master.
> 
> That should be the usb-next branch of the usb-serial tree:
> 
> 	https://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git/
> 
> or linux-next (or, currently, Linus's master branch).
> 
> You can use "scripts/get_maintainer.sh --scm" to determine which tree to base your work against.

Thanks for correcting! But it's scripts/get_maintainer.pl. ;)

Jan

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3] usb-serial:cp210x: add support to software flow control
  2020-08-20  7:52 [PATCH v3] usb-serial:cp210x: add support to software flow control Sheng Long Wang
  2020-08-21  5:32 ` Jan Kiszka
@ 2020-08-24  9:38 ` Johan Hovold
  1 sibling, 0 replies; 9+ messages in thread
From: Johan Hovold @ 2020-08-24  9:38 UTC (permalink / raw)
  To: Sheng Long Wang
  Cc: johan, gregkh, linux-usb, linux-kernel, jan.kiszka, Wang Sheng Long

On Thu, Aug 20, 2020 at 03:52:40PM +0800, Sheng Long Wang wrote:
> From: Wang Sheng Long <shenglong.wang.ext@siemens.com>
> 
> When data is transmitted between two serial ports,
> the phenomenon of data loss often occurs. The two kinds
> of flow control commonly used in serial communication
> are hardware flow control and software flow control.
> 
> In serial communication, If you only use RX/TX/GND Pins, you
> can't do hardware flow. So we often used software flow control
> and prevent data loss. The user sets the software flow control
> through the application program, and the application program
> sets the software flow control mode for the serial port
> chip through the driver.
> 
> For the cp210 serial port chip, its driver lacks the
> software flow control setting code, so the user cannot set
> the software flow control function through the application
> program. This adds the missing software flow control.
> 
> Signed-off-by: Wang Sheng Long <shenglong.wang.ext@siemens.com>
> 
> Changes in v3:
> -fixed code style, It mainly adjusts the code style acccording
>  to kernel specification.
> ---
>  drivers/usb/serial/cp210x.c | 118 ++++++++++++++++++++++++++++++++++--
>  1 file changed, 113 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c
> index e732949f65..c66a0e0fb9 100644
> --- a/drivers/usb/serial/cp210x.c
> +++ b/drivers/usb/serial/cp210x.c
> @@ -380,6 +380,9 @@ static struct usb_serial_driver * const serial_drivers[] = {
>  #define CP210X_PARTNUM_CP2102N_QFN20	0x22
>  #define CP210X_PARTNUM_UNKNOWN	0xFF
>  
> +#define CP210X_VSTART	0x11
> +#define CP210X_VSTOP	0x13

These should come from the termios settings.

> +
>  /* CP210X_GET_COMM_STATUS returns these 0x13 bytes */
>  struct cp210x_comm_status {
>  	__le32   ulErrors;
> @@ -391,6 +394,15 @@ struct cp210x_comm_status {
>  	u8       bReserved;
>  } __packed;
>  
> +struct cp210x_chars_response {

Please rename cp210x_special_chars.

> +	u8	eofchar;
> +	u8	errochar;
> +	u8	breakchar;
> +	u8	eventchar;
> +	u8	xonchar;
> +	u8	xoffchar;

Please revert to using the field names from the spec for consistency.

> +} __packed;

No need for __packed.

> +
>  /*
>   * CP210X_PURGE - 16 bits passed in wValue of USB request.
>   * SiLabs app note AN571 gives a strange description of the 4 bits:
> @@ -624,6 +636,45 @@ static int cp210x_read_vendor_block(struct usb_serial *serial, u8 type, u16 val,
>  	return result;
>  }
>  
> +/*
> + * Read and Write Character Responses operate
> + * Register SET_CHARS/GET_CHATS

This comment makes very little sense to me. Please fix up (including
typos) or drop.

> + */
> +static int cp210x_operate_chars_block(struct usb_serial_port *port,
> +				u8 req, u8 type, void *buf, int bufsize)
> +{
> +	struct usb_serial *serial = port->serial;
> +	struct cp210x_port_private *port_priv = usb_get_serial_port_data(port);
> +	void *dmabuf;
> +	int result;
> +
> +	dmabuf = kmemdup(buf, bufsize, GFP_KERNEL);
> +	if (!dmabuf)
> +		return -ENOMEM;
> +
> +	result = usb_control_msg(serial->dev,
> +				usb_rcvctrlpipe(serial->dev, 0),

This should be usb_sndctrlpipe() when updating the settings.

> +				req, type, 0, port_priv->bInterfaceNumber,
> +				dmabuf, bufsize, USB_CTRL_SET_TIMEOUT);
> +
> +	if (result == bufsize) {
> +		if (type == REQTYPE_DEVICE_TO_HOST)
> +			memcpy(buf, dmabuf, bufsize);
> +
> +		result = 0;
> +	} else {
> +		dev_err(&port->dev, "failed get req 0x%x size %d status: %d\n",
> +			req, bufsize, result);
> +		if (result >= 0)
> +			result = -EIO;
> +

Spurious newline.

> +	}
> +
> +	kfree(dmabuf);
> +
> +	return result;
> +}

Replace this with two dedicated helpers cp210x_get_chars() and
cp210x_set_chars().

> +
>  /*
>   * Writes any 16-bit CP210X_ register (req) whose value is passed
>   * entirely in the wValue field of the USB request.
> @@ -1134,11 +1185,17 @@ static void cp210x_set_termios(struct tty_struct *tty,
>  		struct usb_serial_port *port, struct ktermios *old_termios)
>  {
>  	struct device *dev = &port->dev;
> -	unsigned int cflag, old_cflag;
> +	struct cp210x_chars_response charsres;
> +	struct cp210x_flow_ctl flow_ctl;
> +	unsigned int cflag, old_cflag, iflag;
>  	u16 bits;
> +	int result;
> +	u32 ctl_hs;
> +	u32 flow_repl;
>  
>  	cflag = tty->termios.c_cflag;
>  	old_cflag = old_termios->c_cflag;
> +	iflag = tty->termios.c_iflag;
>  
>  	if (tty->termios.c_ospeed != old_termios->c_ospeed)
>  		cp210x_change_speed(tty, port, old_termios);
> @@ -1212,10 +1269,6 @@ static void cp210x_set_termios(struct tty_struct *tty,
>  	}
>  
>  	if ((cflag & CRTSCTS) != (old_cflag & CRTSCTS)) {
> -		struct cp210x_flow_ctl flow_ctl;
> -		u32 ctl_hs;
> -		u32 flow_repl;
> -
>  		cp210x_read_reg_block(port, CP210X_GET_FLOW, &flow_ctl,
>  				sizeof(flow_ctl));
>  		ctl_hs = le32_to_cpu(flow_ctl.ulControlHandshake);
> @@ -1252,6 +1305,61 @@ static void cp210x_set_termios(struct tty_struct *tty,
>  				sizeof(flow_ctl));
>  	}
>  
> +	/*
> +	 * Set Software  Flow  Control
> +	 * Check the IXOFF/IXON status in the iflag component of the
> +	 * termios structure.
> +	 *
> +	 */

Drop this comment.

> +	if ((iflag & IXOFF) || (iflag & IXON)) {
> +
> +		result = cp210x_operate_chars_block(port,
> +						CP210X_GET_CHARS,
> +						REQTYPE_DEVICE_TO_HOST,
> +						&charsres,
> +						sizeof(charsres));
> +
> +		if (result < 0) {
> +			dev_err(dev, "Read Characrter Responses failed\n");

Please run your patch through a spell checker. And no need for Capital
letters everywhere.

> +			return;

You shouldn't just return here (more important with latest driver).

> +		}
> +		charsres.xonchar  = CP210X_VSTART;
> +		charsres.xoffchar = CP210X_VSTOP;

These should come from termios (see START_CHAR() and STOP_CHAR()).

> +		result = cp210x_operate_chars_block(port,
> +						CP210X_SET_CHARS,
> +						REQTYPE_HOST_TO_INTERFACE,
> +						&charsres,
> +						sizeof(charsres));
> +		if (result < 0) {
> +			memset(&charsres, 0, sizeof(charsres));

Not needed.

> +			dev_err(dev, "Write Characrter Responses failed\n");
> +			return;
> +		}
> +
> +		/*Set  Rx/Tx Flow Contrl  Flag in ulFlowReplace*/

Spaces after and before *. Random spaces mid sentence. Capitalisation...

And generally not useful, please drop it.

> +		cp210x_read_reg_block(port,
> +					CP210X_GET_FLOW,
> +					&flow_ctl,
> +					sizeof(flow_ctl));

Error handling.

> +
> +		flow_repl = le32_to_cpu(flow_ctl.ulFlowReplace);
> +
> +		if (iflag & IXOFF)
> +			flow_repl |= CP210X_SERIAL_AUTO_RECEIVE;
> +		else
> +			flow_repl &= ~CP210X_SERIAL_AUTO_RECEIVE;
> +
> +		if (iflag & IXON)
> +			flow_repl |= CP210X_SERIAL_AUTO_TRANSMIT;
> +		else
> +			flow_repl &= ~CP210X_SERIAL_AUTO_TRANSMIT;
> +
> +		flow_ctl.ulFlowReplace = cpu_to_le32(flow_repl);
> +		cp210x_write_reg_block(port,
> +					CP210X_SET_FLOW,
> +					&flow_ctl,
> +					sizeof(flow_ctl));

Error handling?

> +	}
>  }
>  
>  static int cp210x_tiocmset(struct tty_struct *tty,

Johan

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3] usb-serial:cp210x: add support to software flow control
  2020-08-24  9:36       ` Jan Kiszka
@ 2020-08-24  9:43         ` Johan Hovold
  2020-08-24 10:16           ` Wang, Sheng Long
  0 siblings, 1 reply; 9+ messages in thread
From: Johan Hovold @ 2020-08-24  9:43 UTC (permalink / raw)
  To: Jan Kiszka
  Cc: Wang, Sheng Long (EXT) (RC-CN DI FA R&D SW),
	Johan Hovold, Sheng Long Wang, gregkh, linux-usb, linux-kernel

On Mon, Aug 24, 2020 at 11:36:50AM +0200, Jan Kiszka wrote:
> On 24.08.20 11:28, Wang, Sheng Long (EXT) (RC-CN DI FA R&D SW) wrote:
> > Hi Johan,
> > 
> >     Thanks for your reminding. I am adjusting patch according to the latest Linux Master branch.
> 
> Use git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git, branch
> is likely usb-linus, as Johan requested - just in case there is another
> conflict due to a patch not yet in Linux master but in that tree.

The usb-next is used for new features like this one, but again,
currently Linus's master branch works as well.

> > -----Original Message-----
> > From: Johan Hovold <johan@kernel.org> 
> > Sent: Monday, August 24, 2020 5:10 PM
> > To: Kiszka, Jan (CT RDA IOT SES-DE) <jan.kiszka@siemens.com>
> > Cc: Sheng Long Wang <china_shenglong@163.com>; Wang, Sheng Long (EXT) (RC-CN DI FA R&D SW) <shenglong.wang.ext@siemens.com>; johan@kernel.org; gregkh@linuxfoundation.org; linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org
> > Subject: Re: [PATCH v3] usb-serial:cp210x: add support to software flow control
> > 
> > On Fri, Aug 21, 2020 at 07:32:58AM +0200, Jan Kiszka wrote:
> >> On 20.08.20 09:52, Sheng Long Wang wrote:
> >>> From: Wang Sheng Long <shenglong.wang.ext@siemens.com>
> >>>
> >>> When data is transmitted between two serial ports, the phenomenon of 
> >>> data loss often occurs. The two kinds of flow control commonly used 
> >>> in serial communication are hardware flow control and software flow 
> >>> control.
> >>>
> >>> In serial communication, If you only use RX/TX/GND Pins, you can't 
> >>> do hardware flow. So we often used software flow control and prevent 
> >>> data loss. The user sets the software flow control through the 
> >>> application program, and the application program sets the software 
> >>> flow control mode for the serial port chip through the driver.
> >>>
> >>> For the cp210 serial port chip, its driver lacks the software flow 
> >>> control setting code, so the user cannot set the software flow 
> >>> control function through the application program. This adds the 
> >>> missing software flow control.
> >>>
> >>> Signed-off-by: Wang Sheng Long <shenglong.wang.ext@siemens.com>
> >>>
> >>> Changes in v3:
> >>> -fixed code style, It mainly adjusts the code style acccording  to 
> >>> kernel specification.
> >>
> >> Patch does not apply. You forgot to rebase over latest tty/tty-next or 
> >> linux master.
> > 
> > That should be the usb-next branch of the usb-serial tree:
> > 
> > 	https://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git/
> > 
> > or linux-next (or, currently, Linus's master branch).
> > 
> > You can use "scripts/get_maintainer.sh --scm" to determine which tree to base your work against.
> 
> Thanks for correcting! But it's scripts/get_maintainer.pl. ;)

Heh. Thanks for catching that.

Johan

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH v3] usb-serial:cp210x: add support to software flow control
  2020-08-24  9:43         ` Johan Hovold
@ 2020-08-24 10:16           ` Wang, Sheng Long
  2020-08-24 11:16             ` Johan Hovold
  0 siblings, 1 reply; 9+ messages in thread
From: Wang, Sheng Long @ 2020-08-24 10:16 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Sheng Long Wang, gregkh, linux-usb, linux-kernel, Kiszka, Jan

Hi, Johan

  So, Is it currently possible for me to use which Branch?  :)
 
  https://github.com/torvalds/linux  master branch

 Or use  https://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git

Thanks!
BR/Sheng Long

-----Original Message-----
From: Johan Hovold <johan@kernel.org> 
Sent: Monday, August 24, 2020 5:43 PM
To: Kiszka, Jan (CT RDA IOT SES-DE) <jan.kiszka@siemens.com>
Cc: Wang, Sheng Long (EXT) (RC-CN DI FA R&D SW) <shenglong.wang.ext@siemens.com>; Johan Hovold <johan@kernel.org>; Sheng Long Wang <china_shenglong@163.com>; gregkh@linuxfoundation.org; linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3] usb-serial:cp210x: add support to software flow control

On Mon, Aug 24, 2020 at 11:36:50AM +0200, Jan Kiszka wrote:
> On 24.08.20 11:28, Wang, Sheng Long (EXT) (RC-CN DI FA R&D SW) wrote:
> > Hi Johan,
> > 
> >     Thanks for your reminding. I am adjusting patch according to the latest Linux Master branch.
> 
> Use git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git, 
> branch is likely usb-linus, as Johan requested - just in case there is 
> another conflict due to a patch not yet in Linux master but in that tree.

The usb-next is used for new features like this one, but again, currently Linus's master branch works as well.

> > -----Original Message-----
> > From: Johan Hovold <johan@kernel.org>
> > Sent: Monday, August 24, 2020 5:10 PM
> > To: Kiszka, Jan (CT RDA IOT SES-DE) <jan.kiszka@siemens.com>
> > Cc: Sheng Long Wang <china_shenglong@163.com>; Wang, Sheng Long 
> > (EXT) (RC-CN DI FA R&D SW) <shenglong.wang.ext@siemens.com>; 
> > johan@kernel.org; gregkh@linuxfoundation.org; 
> > linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org
> > Subject: Re: [PATCH v3] usb-serial:cp210x: add support to software 
> > flow control
> > 
> > On Fri, Aug 21, 2020 at 07:32:58AM +0200, Jan Kiszka wrote:
> >> On 20.08.20 09:52, Sheng Long Wang wrote:
> >>> From: Wang Sheng Long <shenglong.wang.ext@siemens.com>
> >>>
> >>> When data is transmitted between two serial ports, the phenomenon 
> >>> of data loss often occurs. The two kinds of flow control commonly 
> >>> used in serial communication are hardware flow control and 
> >>> software flow control.
> >>>
> >>> In serial communication, If you only use RX/TX/GND Pins, you can't 
> >>> do hardware flow. So we often used software flow control and 
> >>> prevent data loss. The user sets the software flow control through 
> >>> the application program, and the application program sets the 
> >>> software flow control mode for the serial port chip through the driver.
> >>>
> >>> For the cp210 serial port chip, its driver lacks the software flow 
> >>> control setting code, so the user cannot set the software flow 
> >>> control function through the application program. This adds the 
> >>> missing software flow control.
> >>>
> >>> Signed-off-by: Wang Sheng Long <shenglong.wang.ext@siemens.com>
> >>>
> >>> Changes in v3:
> >>> -fixed code style, It mainly adjusts the code style acccording  to 
> >>> kernel specification.
> >>
> >> Patch does not apply. You forgot to rebase over latest tty/tty-next 
> >> or linux master.
> > 
> > That should be the usb-next branch of the usb-serial tree:
> > 
> > 	
> > https://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git
> > /
> > 
> > or linux-next (or, currently, Linus's master branch).
> > 
> > You can use "scripts/get_maintainer.sh --scm" to determine which tree to base your work against.
> 
> Thanks for correcting! But it's scripts/get_maintainer.pl. ;)

Heh. Thanks for catching that.

Johan

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3] usb-serial:cp210x: add support to software flow control
  2020-08-24 10:16           ` Wang, Sheng Long
@ 2020-08-24 11:16             ` Johan Hovold
  0 siblings, 0 replies; 9+ messages in thread
From: Johan Hovold @ 2020-08-24 11:16 UTC (permalink / raw)
  To: Wang, Sheng Long
  Cc: Johan Hovold, Sheng Long Wang, gregkh, linux-usb, linux-kernel,
	Kiszka, Jan

On Mon, Aug 24, 2020 at 10:16:13AM +0000, Wang, Sheng Long wrote:
> Hi, Johan
> 
>   So, Is it currently possible for me to use which Branch?  :)
>  
>   https://github.com/torvalds/linux  master branch
> 
>  Or use  https://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git

Please use the usb-next branch from 

	https://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git

Johan

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2020-08-24 11:17 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-20  7:52 [PATCH v3] usb-serial:cp210x: add support to software flow control Sheng Long Wang
2020-08-21  5:32 ` Jan Kiszka
2020-08-24  9:09   ` Johan Hovold
2020-08-24  9:28     ` Wang, Sheng Long
2020-08-24  9:36       ` Jan Kiszka
2020-08-24  9:43         ` Johan Hovold
2020-08-24 10:16           ` Wang, Sheng Long
2020-08-24 11:16             ` Johan Hovold
2020-08-24  9:38 ` Johan Hovold

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).