Linux-USB Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v5] usb-serial:cp210x: add support to software flow control
@ 2020-10-15  6:47 Sheng Long Wang
  2020-10-15 10:09 ` kernel test robot
  0 siblings, 1 reply; 2+ messages in thread
From: Sheng Long Wang @ 2020-10-15  6:47 UTC (permalink / raw)
  To: johan, gregkh; +Cc: linux-usb, linux-kernel, 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.

Changes in v4:
- It mainly adjusts the patch based on the last usb-next branch
  of the usb-serial.

Changes in v5:
- Fixes:
  * According to the cp210x specification, use usb_control_msg()
    requesttype 'REQTYPE_DEVICE_TO_HOST' is modified to
    'REQTYPE_INTERFACE_TO_HOST' in cp210x_get_chars().

  * If modify IXOFF/IXON has been changed, we can call set software
    flow control code.

  * If the setting software flow control wrong, do not continue
    processing proceed with updating software flow control.
---
 drivers/usb/serial/cp210x.c | 128 ++++++++++++++++++++++++++++++++++--
 1 file changed, 123 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c
index d0c05aa8a0d6..d2edf9e4d484 100644
--- a/drivers/usb/serial/cp210x.c
+++ b/drivers/usb/serial/cp210x.c
@@ -412,6 +412,15 @@ struct cp210x_comm_status {
 	u8       bReserved;
 } __packed;
 
+struct cp210x_special_chars {
+	u8	bEofChar;
+	u8	bErrorChar;
+	u8	bBreakChar;
+	u8	bEventChar;
+	u8	bXonChar;
+	u8	bXoffChar;
+};
+
 /*
  * CP210X_PURGE - 16 bits passed in wValue of USB request.
  * SiLabs app note AN571 gives a strange description of the 4 bits:
@@ -675,6 +684,70 @@ static int cp210x_read_vendor_block(struct usb_serial *serial, u8 type, u16 val,
 	return result;
 }
 
+static int cp210x_get_chars(struct usb_serial_port *port, void *buf)
+{
+	struct usb_serial *serial = port->serial;
+	struct cp210x_port_private *port_priv = usb_get_serial_port_data(port);
+	struct cp210x_special_chars *special_chars;
+	void *dmabuf;
+	int result;
+
+	dmabuf = kmemdup(buf, sizeof(*special_chars), GFP_KERNEL);
+	if (!dmabuf)
+		return -ENOMEM;
+
+	result = usb_control_msg(serial->dev,
+				usb_rcvctrlpipe(serial->dev, 0),
+				CP210X_GET_CHARS, REQTYPE_INTERFACE_TO_HOST, 0,
+				port_priv->bInterfaceNumber,
+				dmabuf, sizeof(*special_chars),
+				USB_CTRL_GET_TIMEOUT);
+
+	if (result == sizeof(*special_chars)) {
+		memcpy(buf, dmabuf, sizeof(*special_chars));
+		result = 0;
+	} else {
+		dev_err(&port->dev, "failed to get special chars: %d\n", result);
+		if (result >= 0)
+			result = -EIO;
+	}
+
+	kfree(dmabuf);
+
+	return result;
+}
+
+static int cp210x_set_chars(struct usb_serial_port *port, void *buf)
+{
+	struct usb_serial *serial = port->serial;
+	struct cp210x_port_private *port_priv = usb_get_serial_port_data(port);
+	struct cp210x_special_chars *special_chars;
+	void *dmabuf;
+	int result;
+
+	dmabuf = kmemdup(buf, sizeof(*special_chars), GFP_KERNEL);
+	if (!dmabuf)
+		return -ENOMEM;
+
+	result = usb_control_msg(serial->dev,
+				usb_sndctrlpipe(serial->dev, 0),
+				CP210X_SET_CHARS, REQTYPE_HOST_TO_INTERFACE, 0,
+				port_priv->bInterfaceNumber,
+				dmabuf, sizeof(*special_chars), USB_CTRL_SET_TIMEOUT);
+
+	if (result == sizeof(*special_chars)) {
+		result = 0;
+	} else {
+		dev_err(&port->dev, "failed to set special chars: %d\n", 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.
@@ -1356,11 +1429,18 @@ 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;
+	unsigned int cflag, old_cflag, iflag, old_iflag;
+	struct cp210x_special_chars special_chars;
+	struct cp210x_flow_ctl flow_ctl;
 	u16 bits;
+	int result;
+	u32 ctl_hs;
+	u32 flow_repl;
 
 	cflag = tty->termios.c_cflag;
+	iflag = tty->termios.c_iflag;
 	old_cflag = old_termios->c_cflag;
+	old_iflag = old_termios->c_iflag;
 
 	if (tty->termios.c_ospeed != old_termios->c_ospeed)
 		cp210x_change_speed(tty, port, old_termios);
@@ -1434,10 +1514,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);
@@ -1474,6 +1550,48 @@ static void cp210x_set_termios(struct tty_struct *tty,
 				sizeof(flow_ctl));
 	}
 
+	if (((iflag & IXOFF) != (old_iflag & IXOFF)) ||
+		((iflag & IXON) != (old_iflag & IXON))) {
+		result = cp210x_get_chars(port, &special_chars);
+		if (result < 0)
+			goto out;
+
+		special_chars.bXonChar  = START_CHAR(tty);
+		special_chars.bXoffChar = STOP_CHAR(tty);
+
+		result = cp210x_set_chars(port, &special_chars);
+		if (result < 0)
+			goto out;
+
+		result = cp210x_read_reg_block(port,
+					CP210X_GET_FLOW,
+					&flow_ctl,
+					sizeof(flow_ctl));
+		if (result < 0)
+			goto out;
+
+		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);
+		result = cp210x_write_reg_block(port,
+					CP210X_SET_FLOW,
+					&flow_ctl,
+					sizeof(flow_ctl));
+	}
+out:
+	if (result < 0)
+		dev_err(dev, "failed to set software flow control: %d\n", result);
+
 	/*
 	 * Enable event-insertion mode only if input parity checking is
 	 * enabled for now.
-- 
2.17.1



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

* Re: [PATCH v5] usb-serial:cp210x: add support to software flow control
  2020-10-15  6:47 [PATCH v5] usb-serial:cp210x: add support to software flow control Sheng Long Wang
@ 2020-10-15 10:09 ` kernel test robot
  0 siblings, 0 replies; 2+ messages in thread
From: kernel test robot @ 2020-10-15 10:09 UTC (permalink / raw)
  To: Sheng Long Wang, johan, gregkh
  Cc: kbuild-all, clang-built-linux, linux-usb, linux-kernel, Wang Sheng Long


[-- Attachment #1: Type: text/plain, Size: 8956 bytes --]

Hi Sheng,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on usb-serial/usb-next]
[also build test WARNING on v5.9 next-20201015]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Sheng-Long-Wang/usb-serial-cp210x-add-support-to-software-flow-control/20201015-150459
base:   https://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git usb-next
config: x86_64-randconfig-a004-20201014 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project e7b4feea8e1bf520b34ad8c116abab6677344b74)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # https://github.com/0day-ci/linux/commit/87886eacf097dd70bd9f9391eb329fa40706038a
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Sheng-Long-Wang/usb-serial-cp210x-add-support-to-software-flow-control/20201015-150459
        git checkout 87886eacf097dd70bd9f9391eb329fa40706038a
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/usb/serial/cp210x.c:1553:6: warning: variable 'result' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
           if (((iflag & IXOFF) != (old_iflag & IXOFF)) ||
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/usb/serial/cp210x.c:1592:6: note: uninitialized use occurs here
           if (result < 0)
               ^~~~~~
   drivers/usb/serial/cp210x.c:1553:2: note: remove the 'if' if its condition is always true
           if (((iflag & IXOFF) != (old_iflag & IXOFF)) ||
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/usb/serial/cp210x.c:1436:12: note: initialize the variable 'result' to silence this warning
           int result;
                     ^
                      = 0
   1 warning generated.

vim +1553 drivers/usb/serial/cp210x.c

  1427	
  1428	static void cp210x_set_termios(struct tty_struct *tty,
  1429			struct usb_serial_port *port, struct ktermios *old_termios)
  1430	{
  1431		struct device *dev = &port->dev;
  1432		unsigned int cflag, old_cflag, iflag, old_iflag;
  1433		struct cp210x_special_chars special_chars;
  1434		struct cp210x_flow_ctl flow_ctl;
  1435		u16 bits;
  1436		int result;
  1437		u32 ctl_hs;
  1438		u32 flow_repl;
  1439	
  1440		cflag = tty->termios.c_cflag;
  1441		iflag = tty->termios.c_iflag;
  1442		old_cflag = old_termios->c_cflag;
  1443		old_iflag = old_termios->c_iflag;
  1444	
  1445		if (tty->termios.c_ospeed != old_termios->c_ospeed)
  1446			cp210x_change_speed(tty, port, old_termios);
  1447	
  1448		/* If the number of data bits is to be updated */
  1449		if ((cflag & CSIZE) != (old_cflag & CSIZE)) {
  1450			cp210x_get_line_ctl(port, &bits);
  1451			bits &= ~BITS_DATA_MASK;
  1452			switch (cflag & CSIZE) {
  1453			case CS5:
  1454				bits |= BITS_DATA_5;
  1455				dev_dbg(dev, "%s - data bits = 5\n", __func__);
  1456				break;
  1457			case CS6:
  1458				bits |= BITS_DATA_6;
  1459				dev_dbg(dev, "%s - data bits = 6\n", __func__);
  1460				break;
  1461			case CS7:
  1462				bits |= BITS_DATA_7;
  1463				dev_dbg(dev, "%s - data bits = 7\n", __func__);
  1464				break;
  1465			case CS8:
  1466			default:
  1467				bits |= BITS_DATA_8;
  1468				dev_dbg(dev, "%s - data bits = 8\n", __func__);
  1469				break;
  1470			}
  1471			if (cp210x_write_u16_reg(port, CP210X_SET_LINE_CTL, bits))
  1472				dev_dbg(dev, "Number of data bits requested not supported by device\n");
  1473		}
  1474	
  1475		if ((cflag     & (PARENB|PARODD|CMSPAR)) !=
  1476		    (old_cflag & (PARENB|PARODD|CMSPAR))) {
  1477			cp210x_get_line_ctl(port, &bits);
  1478			bits &= ~BITS_PARITY_MASK;
  1479			if (cflag & PARENB) {
  1480				if (cflag & CMSPAR) {
  1481					if (cflag & PARODD) {
  1482						bits |= BITS_PARITY_MARK;
  1483						dev_dbg(dev, "%s - parity = MARK\n", __func__);
  1484					} else {
  1485						bits |= BITS_PARITY_SPACE;
  1486						dev_dbg(dev, "%s - parity = SPACE\n", __func__);
  1487					}
  1488				} else {
  1489					if (cflag & PARODD) {
  1490						bits |= BITS_PARITY_ODD;
  1491						dev_dbg(dev, "%s - parity = ODD\n", __func__);
  1492					} else {
  1493						bits |= BITS_PARITY_EVEN;
  1494						dev_dbg(dev, "%s - parity = EVEN\n", __func__);
  1495					}
  1496				}
  1497			}
  1498			if (cp210x_write_u16_reg(port, CP210X_SET_LINE_CTL, bits))
  1499				dev_dbg(dev, "Parity mode not supported by device\n");
  1500		}
  1501	
  1502		if ((cflag & CSTOPB) != (old_cflag & CSTOPB)) {
  1503			cp210x_get_line_ctl(port, &bits);
  1504			bits &= ~BITS_STOP_MASK;
  1505			if (cflag & CSTOPB) {
  1506				bits |= BITS_STOP_2;
  1507				dev_dbg(dev, "%s - stop bits = 2\n", __func__);
  1508			} else {
  1509				bits |= BITS_STOP_1;
  1510				dev_dbg(dev, "%s - stop bits = 1\n", __func__);
  1511			}
  1512			if (cp210x_write_u16_reg(port, CP210X_SET_LINE_CTL, bits))
  1513				dev_dbg(dev, "Number of stop bits requested not supported by device\n");
  1514		}
  1515	
  1516		if ((cflag & CRTSCTS) != (old_cflag & CRTSCTS)) {
  1517			cp210x_read_reg_block(port, CP210X_GET_FLOW, &flow_ctl,
  1518					sizeof(flow_ctl));
  1519			ctl_hs = le32_to_cpu(flow_ctl.ulControlHandshake);
  1520			flow_repl = le32_to_cpu(flow_ctl.ulFlowReplace);
  1521			dev_dbg(dev, "%s - read ulControlHandshake=0x%08x, ulFlowReplace=0x%08x\n",
  1522					__func__, ctl_hs, flow_repl);
  1523	
  1524			ctl_hs &= ~CP210X_SERIAL_DSR_HANDSHAKE;
  1525			ctl_hs &= ~CP210X_SERIAL_DCD_HANDSHAKE;
  1526			ctl_hs &= ~CP210X_SERIAL_DSR_SENSITIVITY;
  1527			ctl_hs &= ~CP210X_SERIAL_DTR_MASK;
  1528			ctl_hs |= CP210X_SERIAL_DTR_SHIFT(CP210X_SERIAL_DTR_ACTIVE);
  1529			if (cflag & CRTSCTS) {
  1530				ctl_hs |= CP210X_SERIAL_CTS_HANDSHAKE;
  1531	
  1532				flow_repl &= ~CP210X_SERIAL_RTS_MASK;
  1533				flow_repl |= CP210X_SERIAL_RTS_SHIFT(
  1534						CP210X_SERIAL_RTS_FLOW_CTL);
  1535				dev_dbg(dev, "%s - flow control = CRTSCTS\n", __func__);
  1536			} else {
  1537				ctl_hs &= ~CP210X_SERIAL_CTS_HANDSHAKE;
  1538	
  1539				flow_repl &= ~CP210X_SERIAL_RTS_MASK;
  1540				flow_repl |= CP210X_SERIAL_RTS_SHIFT(
  1541						CP210X_SERIAL_RTS_ACTIVE);
  1542				dev_dbg(dev, "%s - flow control = NONE\n", __func__);
  1543			}
  1544	
  1545			dev_dbg(dev, "%s - write ulControlHandshake=0x%08x, ulFlowReplace=0x%08x\n",
  1546					__func__, ctl_hs, flow_repl);
  1547			flow_ctl.ulControlHandshake = cpu_to_le32(ctl_hs);
  1548			flow_ctl.ulFlowReplace = cpu_to_le32(flow_repl);
  1549			cp210x_write_reg_block(port, CP210X_SET_FLOW, &flow_ctl,
  1550					sizeof(flow_ctl));
  1551		}
  1552	
> 1553		if (((iflag & IXOFF) != (old_iflag & IXOFF)) ||
  1554			((iflag & IXON) != (old_iflag & IXON))) {
  1555			result = cp210x_get_chars(port, &special_chars);
  1556			if (result < 0)
  1557				goto out;
  1558	
  1559			special_chars.bXonChar  = START_CHAR(tty);
  1560			special_chars.bXoffChar = STOP_CHAR(tty);
  1561	
  1562			result = cp210x_set_chars(port, &special_chars);
  1563			if (result < 0)
  1564				goto out;
  1565	
  1566			result = cp210x_read_reg_block(port,
  1567						CP210X_GET_FLOW,
  1568						&flow_ctl,
  1569						sizeof(flow_ctl));
  1570			if (result < 0)
  1571				goto out;
  1572	
  1573			flow_repl = le32_to_cpu(flow_ctl.ulFlowReplace);
  1574	
  1575			if (iflag & IXOFF)
  1576				flow_repl |= CP210X_SERIAL_AUTO_RECEIVE;
  1577			else
  1578				flow_repl &= ~CP210X_SERIAL_AUTO_RECEIVE;
  1579	
  1580			if (iflag & IXON)
  1581				flow_repl |= CP210X_SERIAL_AUTO_TRANSMIT;
  1582			else
  1583				flow_repl &= ~CP210X_SERIAL_AUTO_TRANSMIT;
  1584	
  1585			flow_ctl.ulFlowReplace = cpu_to_le32(flow_repl);
  1586			result = cp210x_write_reg_block(port,
  1587						CP210X_SET_FLOW,
  1588						&flow_ctl,
  1589						sizeof(flow_ctl));
  1590		}
  1591	out:
  1592		if (result < 0)
  1593			dev_err(dev, "failed to set software flow control: %d\n", result);
  1594	
  1595		/*
  1596		 * Enable event-insertion mode only if input parity checking is
  1597		 * enabled for now.
  1598		 */
  1599		if (I_INPCK(tty))
  1600			cp210x_enable_event_mode(port);
  1601		else
  1602			cp210x_disable_event_mode(port);
  1603	}
  1604	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 39904 bytes --]

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

end of thread, back to index

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-15  6:47 [PATCH v5] usb-serial:cp210x: add support to software flow control Sheng Long Wang
2020-10-15 10:09 ` kernel test robot

Linux-USB Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-usb/0 linux-usb/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-usb linux-usb/ https://lore.kernel.org/linux-usb \
		linux-usb@vger.kernel.org
	public-inbox-index linux-usb

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-usb


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git