* [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 related [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, other threads:[~2020-10-15 10:10 UTC | newest]
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
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).