linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/10] USB: serial: xr: fix up remaining issues in new driver
@ 2021-01-21 10:29 Johan Hovold
  2021-01-21 10:29 ` [PATCH] USB: serial: cp210x: suppress modem-control error on open and close Johan Hovold
                   ` (11 more replies)
  0 siblings, 12 replies; 14+ messages in thread
From: Johan Hovold @ 2021-01-21 10:29 UTC (permalink / raw)
  To: linux-usb; +Cc: Manivannan Sadhasivam, linux-kernel, Johan Hovold

This series fixes the remaining issues in the new MaxLinear driver that
were pointed out here:

	https://lore.kernel.org/r/YAlVLOqzx8otPgOg@hovoldconsulting.com

Johan


Johan Hovold (10):
  USB: serial: xr: fix NULL-deref at probe
  USB: serial: xr: fix interface leak at disconnect
  USB: serial: xr: use subsystem usb_device at probe
  USB: serial: xr: use termios flag helpers
  USB: serial: xr: document vendor-request recipient
  USB: serial: xr: clean up line-settings handling
  USB: serial: xr: simplify line-speed logic
  USB: serial: xr: fix gpio-mode handling
  USB: serial: xr: fix pin configuration
  USB: serial: xr: fix B0 handling

 drivers/usb/serial/xr_serial.c | 102 +++++++++++++++++++--------------
 1 file changed, 59 insertions(+), 43 deletions(-)

-- 
2.26.2


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

* [PATCH] USB: serial: cp210x: suppress modem-control error on open and close
  2021-01-21 10:29 [PATCH 00/10] USB: serial: xr: fix up remaining issues in new driver Johan Hovold
@ 2021-01-21 10:29 ` Johan Hovold
  2021-01-21 10:32   ` Johan Hovold
  2021-01-21 10:29 ` [PATCH 01/10] USB: serial: xr: fix NULL-deref at probe Johan Hovold
                   ` (10 subsequent siblings)
  11 siblings, 1 reply; 14+ messages in thread
From: Johan Hovold @ 2021-01-21 10:29 UTC (permalink / raw)
  To: linux-usb; +Cc: Manivannan Sadhasivam, linux-kernel, Johan Hovold, Pho Tran

The CP210X_SET_MHS request cannot be used to control DTR/RTS when
hardware flow control is enabled and instead returns an error which is
currently logged as:

	cp210x ttyUSB0: failed set request 0x7 status: -32

Add a crtscts flag to keep track of the hardware flow-control setting
and use it to suppress the request in dtr_rts().

Note that both lines are still deasserted when disabling the UART as
part of close().

Also note that TIOCMSET is left unchanged and will continue to return an
error to user-space when flow control is enabled (i.e. instead of
disabling and re-enabling auto-RTS when RTS is deasserted and
re-asserted).

Reported-by: Pho Tran <pho.tran@silabs.com>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/cp210x.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/serial/cp210x.c b/drivers/usb/serial/cp210x.c
index d813a052738f..ac1e5cbe61dd 100644
--- a/drivers/usb/serial/cp210x.c
+++ b/drivers/usb/serial/cp210x.c
@@ -264,7 +264,8 @@ struct cp210x_port_private {
 	u8			bInterfaceNumber;
 	bool			event_mode;
 	enum cp210x_event_state event_state;
-	u8 lsr;
+	u8			lsr;
+	bool			crtscts;
 };
 
 static struct usb_serial_driver cp210x_device = {
@@ -1117,6 +1118,7 @@ static bool cp210x_termios_change(const struct ktermios *a, const struct ktermio
 static void cp210x_set_flow_control(struct tty_struct *tty,
 		struct usb_serial_port *port, struct ktermios *old_termios)
 {
+	struct cp210x_port_private *port_priv = usb_get_serial_port_data(port);
 	struct cp210x_special_chars chars;
 	struct cp210x_flow_ctl flow_ctl;
 	u32 flow_repl;
@@ -1161,10 +1163,12 @@ static void cp210x_set_flow_control(struct tty_struct *tty,
 		ctl_hs |= CP210X_SERIAL_CTS_HANDSHAKE;
 		flow_repl &= ~CP210X_SERIAL_RTS_MASK;
 		flow_repl |= CP210X_SERIAL_RTS_SHIFT(CP210X_SERIAL_RTS_FLOW_CTL);
+		port_priv->crtscts = true;
 	} else {
 		ctl_hs &= ~CP210X_SERIAL_CTS_HANDSHAKE;
 		flow_repl &= ~CP210X_SERIAL_RTS_MASK;
 		flow_repl |= CP210X_SERIAL_RTS_SHIFT(CP210X_SERIAL_RTS_ACTIVE);
+		port_priv->crtscts = false;
 	}
 
 	if (I_IXOFF(tty))
@@ -1298,6 +1302,16 @@ static int cp210x_tiocmset_port(struct usb_serial_port *port,
 
 static void cp210x_dtr_rts(struct usb_serial_port *port, int on)
 {
+	struct cp210x_port_private *port_priv = usb_get_serial_port_data(port);
+
+	/*
+	 * CP210X_SET_MHS cannot be used to control DTR/RTS when hardware flow
+	 * control is enabled. Note that both lines are still deasserted when
+	 * disabling the UART.
+	 */
+	if (port_priv->crtscts)
+		return;
+
 	if (on)
 		cp210x_tiocmset_port(port, TIOCM_DTR | TIOCM_RTS, 0);
 	else
-- 
2.26.2


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

* [PATCH 01/10] USB: serial: xr: fix NULL-deref at probe
  2021-01-21 10:29 [PATCH 00/10] USB: serial: xr: fix up remaining issues in new driver Johan Hovold
  2021-01-21 10:29 ` [PATCH] USB: serial: cp210x: suppress modem-control error on open and close Johan Hovold
@ 2021-01-21 10:29 ` Johan Hovold
  2021-01-21 10:29 ` [PATCH 02/10] USB: serial: xr: fix interface leak at disconnect Johan Hovold
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Johan Hovold @ 2021-01-21 10:29 UTC (permalink / raw)
  To: linux-usb; +Cc: Manivannan Sadhasivam, linux-kernel, Johan Hovold

Make sure that the probed device has an interface 0 to avoid
dereferencing a NULL pointer in case of a malicious device or during
USB-descriptor fuzzing.

Fixes: a8f54b7bd132 ("USB: serial: add MaxLinear/Exar USB to Serial driver")
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/xr_serial.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/serial/xr_serial.c b/drivers/usb/serial/xr_serial.c
index bdb2df27b50b..7be6da6a5cf3 100644
--- a/drivers/usb/serial/xr_serial.c
+++ b/drivers/usb/serial/xr_serial.c
@@ -552,6 +552,9 @@ static int xr_probe(struct usb_serial *serial, const struct usb_device_id *id)
 
 	/* But claim the control interface during data interface probe */
 	control_interface = usb_ifnum_to_if(usb_dev, 0);
+	if (!control_interface)
+		return -ENODEV;
+
 	ret = usb_driver_claim_interface(driver, control_interface, NULL);
 	if (ret) {
 		dev_err(&serial->interface->dev, "Failed to claim control interface\n");
-- 
2.26.2


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

* [PATCH 02/10] USB: serial: xr: fix interface leak at disconnect
  2021-01-21 10:29 [PATCH 00/10] USB: serial: xr: fix up remaining issues in new driver Johan Hovold
  2021-01-21 10:29 ` [PATCH] USB: serial: cp210x: suppress modem-control error on open and close Johan Hovold
  2021-01-21 10:29 ` [PATCH 01/10] USB: serial: xr: fix NULL-deref at probe Johan Hovold
@ 2021-01-21 10:29 ` Johan Hovold
  2021-01-21 10:29 ` [PATCH 03/10] USB: serial: xr: use subsystem usb_device at probe Johan Hovold
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Johan Hovold @ 2021-01-21 10:29 UTC (permalink / raw)
  To: linux-usb; +Cc: Manivannan Sadhasivam, linux-kernel, Johan Hovold

Make sure to release the control interface at disconnect so that the
driver can be unbound without leaking resources (and later rebound).

Fixes: a8f54b7bd132 ("USB: serial: add MaxLinear/Exar USB to Serial driver")
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/xr_serial.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/usb/serial/xr_serial.c b/drivers/usb/serial/xr_serial.c
index 7be6da6a5cf3..5e110b0c8e71 100644
--- a/drivers/usb/serial/xr_serial.c
+++ b/drivers/usb/serial/xr_serial.c
@@ -564,6 +564,15 @@ static int xr_probe(struct usb_serial *serial, const struct usb_device_id *id)
 	return 0;
 }
 
+static void xr_disconnect(struct usb_serial *serial)
+{
+	struct usb_driver *driver = serial->type->usb_driver;
+	struct usb_interface *control_interface;
+
+	control_interface = usb_ifnum_to_if(serial->dev, 0);
+	usb_driver_release_interface(driver, control_interface);
+}
+
 static const struct usb_device_id id_table[] = {
 	{ USB_DEVICE(0x04e2, 0x1410) }, /* XR21V141X */
 	{ }
@@ -578,6 +587,7 @@ static struct usb_serial_driver xr_device = {
 	.id_table		= id_table,
 	.num_ports		= 1,
 	.probe			= xr_probe,
+	.disconnect		= xr_disconnect,
 	.open			= xr_open,
 	.close			= xr_close,
 	.break_ctl		= xr_break_ctl,
-- 
2.26.2


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

* [PATCH 03/10] USB: serial: xr: use subsystem usb_device at probe
  2021-01-21 10:29 [PATCH 00/10] USB: serial: xr: fix up remaining issues in new driver Johan Hovold
                   ` (2 preceding siblings ...)
  2021-01-21 10:29 ` [PATCH 02/10] USB: serial: xr: fix interface leak at disconnect Johan Hovold
@ 2021-01-21 10:29 ` Johan Hovold
  2021-01-21 10:29 ` [PATCH 04/10] USB: serial: xr: use termios flag helpers Johan Hovold
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Johan Hovold @ 2021-01-21 10:29 UTC (permalink / raw)
  To: linux-usb; +Cc: Manivannan Sadhasivam, linux-kernel, Johan Hovold

Use the subsystem struct usb_device pointer at probe instead of
deriving it from the interface pointer.

Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/xr_serial.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/usb/serial/xr_serial.c b/drivers/usb/serial/xr_serial.c
index 5e110b0c8e71..8f81f866d681 100644
--- a/drivers/usb/serial/xr_serial.c
+++ b/drivers/usb/serial/xr_serial.c
@@ -541,7 +541,6 @@ static void xr_close(struct usb_serial_port *port)
 
 static int xr_probe(struct usb_serial *serial, const struct usb_device_id *id)
 {
-	struct usb_device *usb_dev = interface_to_usbdev(serial->interface);
 	struct usb_driver *driver = serial->type->usb_driver;
 	struct usb_interface *control_interface;
 	int ret;
@@ -551,7 +550,7 @@ static int xr_probe(struct usb_serial *serial, const struct usb_device_id *id)
 		return -ENODEV;
 
 	/* But claim the control interface during data interface probe */
-	control_interface = usb_ifnum_to_if(usb_dev, 0);
+	control_interface = usb_ifnum_to_if(serial->dev, 0);
 	if (!control_interface)
 		return -ENODEV;
 
-- 
2.26.2


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

* [PATCH 04/10] USB: serial: xr: use termios flag helpers
  2021-01-21 10:29 [PATCH 00/10] USB: serial: xr: fix up remaining issues in new driver Johan Hovold
                   ` (3 preceding siblings ...)
  2021-01-21 10:29 ` [PATCH 03/10] USB: serial: xr: use subsystem usb_device at probe Johan Hovold
@ 2021-01-21 10:29 ` Johan Hovold
  2021-01-21 10:29 ` [PATCH 05/10] USB: serial: xr: document vendor-request recipient Johan Hovold
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Johan Hovold @ 2021-01-21 10:29 UTC (permalink / raw)
  To: linux-usb; +Cc: Manivannan Sadhasivam, linux-kernel, Johan Hovold

Use the termios flag helpers consistently, including for CRTSCTS.

Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/xr_serial.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/usb/serial/xr_serial.c b/drivers/usb/serial/xr_serial.c
index 8f81f866d681..52909dccb4dc 100644
--- a/drivers/usb/serial/xr_serial.c
+++ b/drivers/usb/serial/xr_serial.c
@@ -401,7 +401,6 @@ static int xr_set_baudrate(struct tty_struct *tty,
 static void xr_set_flow_mode(struct tty_struct *tty,
 			     struct usb_serial_port *port)
 {
-	unsigned int cflag = tty->termios.c_cflag;
 	u8 flow, gpio_mode;
 	int ret;
 
@@ -409,7 +408,7 @@ static void xr_set_flow_mode(struct tty_struct *tty,
 	if (ret)
 		return;
 
-	if (cflag & CRTSCTS) {
+	if (C_CRTSCTS(tty)) {
 		dev_dbg(&port->dev, "Enabling hardware flow ctrl\n");
 
 		/*
-- 
2.26.2


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

* [PATCH 05/10] USB: serial: xr: document vendor-request recipient
  2021-01-21 10:29 [PATCH 00/10] USB: serial: xr: fix up remaining issues in new driver Johan Hovold
                   ` (4 preceding siblings ...)
  2021-01-21 10:29 ` [PATCH 04/10] USB: serial: xr: use termios flag helpers Johan Hovold
@ 2021-01-21 10:29 ` Johan Hovold
  2021-01-21 10:29 ` [PATCH 06/10] USB: serial: xr: clean up line-settings handling Johan Hovold
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Johan Hovold @ 2021-01-21 10:29 UTC (permalink / raw)
  To: linux-usb; +Cc: Manivannan Sadhasivam, linux-kernel, Johan Hovold

Add the missing device-recipient define to the vendor control requests
for completeness.

Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/xr_serial.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/serial/xr_serial.c b/drivers/usb/serial/xr_serial.c
index 52909dccb4dc..202263211ba9 100644
--- a/drivers/usb/serial/xr_serial.c
+++ b/drivers/usb/serial/xr_serial.c
@@ -116,8 +116,8 @@ static int xr_set_reg(struct usb_serial_port *port, u8 block, u8 reg, u8 val)
 	ret = usb_control_msg(serial->dev,
 			      usb_sndctrlpipe(serial->dev, 0),
 			      XR21V141X_SET_REQ,
-			      USB_DIR_OUT | USB_TYPE_VENDOR, val,
-			      reg | (block << 8), NULL, 0,
+			      USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
+			      val, reg | (block << 8), NULL, 0,
 			      USB_CTRL_SET_TIMEOUT);
 	if (ret < 0) {
 		dev_err(&port->dev, "Failed to set reg 0x%02x: %d\n", reg, ret);
@@ -140,8 +140,8 @@ static int xr_get_reg(struct usb_serial_port *port, u8 block, u8 reg, u8 *val)
 	ret = usb_control_msg(serial->dev,
 			      usb_rcvctrlpipe(serial->dev, 0),
 			      XR21V141X_GET_REQ,
-			      USB_DIR_IN | USB_TYPE_VENDOR, 0,
-			      reg | (block << 8), dmabuf, 1,
+			      USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
+			      0, reg | (block << 8), dmabuf, 1,
 			      USB_CTRL_GET_TIMEOUT);
 	if (ret == 1) {
 		*val = *dmabuf;
-- 
2.26.2


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

* [PATCH 06/10] USB: serial: xr: clean up line-settings handling
  2021-01-21 10:29 [PATCH 00/10] USB: serial: xr: fix up remaining issues in new driver Johan Hovold
                   ` (5 preceding siblings ...)
  2021-01-21 10:29 ` [PATCH 05/10] USB: serial: xr: document vendor-request recipient Johan Hovold
@ 2021-01-21 10:29 ` Johan Hovold
  2021-01-21 10:29 ` [PATCH 07/10] USB: serial: xr: simplify line-speed logic Johan Hovold
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Johan Hovold @ 2021-01-21 10:29 UTC (permalink / raw)
  To: linux-usb; +Cc: Manivannan Sadhasivam, linux-kernel, Johan Hovold

Shift the line-setting values when defining them rather than in
set_termios() for consistency and improved readability.

Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/xr_serial.c | 34 +++++++++++++++-------------------
 1 file changed, 15 insertions(+), 19 deletions(-)

diff --git a/drivers/usb/serial/xr_serial.c b/drivers/usb/serial/xr_serial.c
index 202263211ba9..2000277bacc1 100644
--- a/drivers/usb/serial/xr_serial.c
+++ b/drivers/usb/serial/xr_serial.c
@@ -71,17 +71,17 @@ struct xr_txrx_clk_mask {
 #define XR21V141X_UART_DATA_8		0x8
 
 #define XR21V141X_UART_PARITY_MASK	GENMASK(6, 4)
-#define XR21V141X_UART_PARITY_SHIFT	0x4
-#define XR21V141X_UART_PARITY_NONE	0x0
-#define XR21V141X_UART_PARITY_ODD	0x1
-#define XR21V141X_UART_PARITY_EVEN	0x2
-#define XR21V141X_UART_PARITY_MARK	0x3
-#define XR21V141X_UART_PARITY_SPACE	0x4
+#define XR21V141X_UART_PARITY_SHIFT	4
+#define XR21V141X_UART_PARITY_NONE	(0x0 << XR21V141X_UART_PARITY_SHIFT)
+#define XR21V141X_UART_PARITY_ODD	(0x1 << XR21V141X_UART_PARITY_SHIFT)
+#define XR21V141X_UART_PARITY_EVEN	(0x2 << XR21V141X_UART_PARITY_SHIFT)
+#define XR21V141X_UART_PARITY_MARK	(0x3 << XR21V141X_UART_PARITY_SHIFT)
+#define XR21V141X_UART_PARITY_SPACE	(0x4 << XR21V141X_UART_PARITY_SHIFT)
 
 #define XR21V141X_UART_STOP_MASK	BIT(7)
-#define XR21V141X_UART_STOP_SHIFT	0x7
-#define XR21V141X_UART_STOP_1		0x0
-#define XR21V141X_UART_STOP_2		0x1
+#define XR21V141X_UART_STOP_SHIFT	7
+#define XR21V141X_UART_STOP_1		(0x0 << XR21V141X_UART_STOP_SHIFT)
+#define XR21V141X_UART_STOP_2		(0x1 << XR21V141X_UART_STOP_SHIFT)
 
 #define XR21V141X_UART_FLOW_MODE_NONE	0x0
 #define XR21V141X_UART_FLOW_MODE_HW	0x1
@@ -477,25 +477,21 @@ static void xr_set_termios(struct tty_struct *tty,
 	if (C_PARENB(tty)) {
 		if (C_CMSPAR(tty)) {
 			if (C_PARODD(tty))
-				bits |= XR21V141X_UART_PARITY_MARK <<
-					XR21V141X_UART_PARITY_SHIFT;
+				bits |= XR21V141X_UART_PARITY_MARK;
 			else
-				bits |= XR21V141X_UART_PARITY_SPACE <<
-					XR21V141X_UART_PARITY_SHIFT;
+				bits |= XR21V141X_UART_PARITY_SPACE;
 		} else {
 			if (C_PARODD(tty))
-				bits |= XR21V141X_UART_PARITY_ODD <<
-					XR21V141X_UART_PARITY_SHIFT;
+				bits |= XR21V141X_UART_PARITY_ODD;
 			else
-				bits |= XR21V141X_UART_PARITY_EVEN <<
-					XR21V141X_UART_PARITY_SHIFT;
+				bits |= XR21V141X_UART_PARITY_EVEN;
 		}
 	}
 
 	if (C_CSTOPB(tty))
-		bits |= XR21V141X_UART_STOP_2 << XR21V141X_UART_STOP_SHIFT;
+		bits |= XR21V141X_UART_STOP_2;
 	else
-		bits |= XR21V141X_UART_STOP_1 << XR21V141X_UART_STOP_SHIFT;
+		bits |= XR21V141X_UART_STOP_1;
 
 	ret = xr_set_reg_uart(port, XR21V141X_REG_FORMAT, bits);
 	if (ret)
-- 
2.26.2


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

* [PATCH 07/10] USB: serial: xr: simplify line-speed logic
  2021-01-21 10:29 [PATCH 00/10] USB: serial: xr: fix up remaining issues in new driver Johan Hovold
                   ` (6 preceding siblings ...)
  2021-01-21 10:29 ` [PATCH 06/10] USB: serial: xr: clean up line-settings handling Johan Hovold
@ 2021-01-21 10:29 ` Johan Hovold
  2021-01-21 10:29 ` [PATCH 08/10] USB: serial: xr: fix gpio-mode handling Johan Hovold
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Johan Hovold @ 2021-01-21 10:29 UTC (permalink / raw)
  To: linux-usb; +Cc: Manivannan Sadhasivam, linux-kernel, Johan Hovold

Simplify the changed-line-speed conditional expression.

Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/xr_serial.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/usb/serial/xr_serial.c b/drivers/usb/serial/xr_serial.c
index 2000277bacc1..fc727f4283f2 100644
--- a/drivers/usb/serial/xr_serial.c
+++ b/drivers/usb/serial/xr_serial.c
@@ -451,8 +451,7 @@ static void xr_set_termios(struct tty_struct *tty,
 	u8 bits = 0;
 	int ret;
 
-	if ((old_termios && tty->termios.c_ospeed != old_termios->c_ospeed) ||
-	    !old_termios)
+	if (!old_termios || (tty->termios.c_ospeed != old_termios->c_ospeed))
 		xr_set_baudrate(tty, port);
 
 	switch (C_CSIZE(tty)) {
-- 
2.26.2


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

* [PATCH 08/10] USB: serial: xr: fix gpio-mode handling
  2021-01-21 10:29 [PATCH 00/10] USB: serial: xr: fix up remaining issues in new driver Johan Hovold
                   ` (7 preceding siblings ...)
  2021-01-21 10:29 ` [PATCH 07/10] USB: serial: xr: simplify line-speed logic Johan Hovold
@ 2021-01-21 10:29 ` Johan Hovold
  2021-01-21 10:29 ` [PATCH 09/10] USB: serial: xr: fix pin configuration Johan Hovold
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 14+ messages in thread
From: Johan Hovold @ 2021-01-21 10:29 UTC (permalink / raw)
  To: linux-usb; +Cc: Manivannan Sadhasivam, linux-kernel, Johan Hovold

Fix the gpio-mode handling so that all the pins are under driver control
(i.e. in gpio mode) when hardware flow control is disabled.

This is specifically needed to be able to control RTS.

Fixes: a8f54b7bd132 ("USB: serial: add MaxLinear/Exar USB to Serial driver")
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/xr_serial.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/serial/xr_serial.c b/drivers/usb/serial/xr_serial.c
index fc727f4283f2..183731cd2ef7 100644
--- a/drivers/usb/serial/xr_serial.c
+++ b/drivers/usb/serial/xr_serial.c
@@ -408,14 +408,11 @@ static void xr_set_flow_mode(struct tty_struct *tty,
 	if (ret)
 		return;
 
+	/* Set GPIO mode for controlling the pins manually by default. */
+	gpio_mode &= ~XR21V141X_UART_MODE_GPIO_MASK;
+
 	if (C_CRTSCTS(tty)) {
 		dev_dbg(&port->dev, "Enabling hardware flow ctrl\n");
-
-		/*
-		 * RTS/CTS is the default flow control mode, so set GPIO mode
-		 * for controlling the pins manually by default.
-		 */
-		gpio_mode &= ~XR21V141X_UART_MODE_GPIO_MASK;
 		gpio_mode |= XR21V141X_UART_MODE_RTS_CTS;
 		flow = XR21V141X_UART_FLOW_MODE_HW;
 	} else if (I_IXON(tty)) {
-- 
2.26.2


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

* [PATCH 09/10] USB: serial: xr: fix pin configuration
  2021-01-21 10:29 [PATCH 00/10] USB: serial: xr: fix up remaining issues in new driver Johan Hovold
                   ` (8 preceding siblings ...)
  2021-01-21 10:29 ` [PATCH 08/10] USB: serial: xr: fix gpio-mode handling Johan Hovold
@ 2021-01-21 10:29 ` Johan Hovold
  2021-01-21 10:29 ` [PATCH 10/10] USB: serial: xr: fix B0 handling Johan Hovold
  2021-01-26 15:26 ` [PATCH 00/10] USB: serial: xr: fix up remaining issues in new driver Johan Hovold
  11 siblings, 0 replies; 14+ messages in thread
From: Johan Hovold @ 2021-01-21 10:29 UTC (permalink / raw)
  To: linux-usb; +Cc: Manivannan Sadhasivam, linux-kernel, Johan Hovold

Make sure that the modem pins are set up correctly when opening the
port to avoid leaving, for example, DTR and RTS configured as inputs,
which is the device default.

This is specifically needed to be able to control DTR and RTS when
hardware flow control is disabled.

Fixes: a8f54b7bd132 ("USB: serial: add MaxLinear/Exar USB to Serial driver")
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/xr_serial.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/usb/serial/xr_serial.c b/drivers/usb/serial/xr_serial.c
index 183731cd2ef7..f67e7dba9509 100644
--- a/drivers/usb/serial/xr_serial.c
+++ b/drivers/usb/serial/xr_serial.c
@@ -502,6 +502,7 @@ static void xr_set_termios(struct tty_struct *tty,
 
 static int xr_open(struct tty_struct *tty, struct usb_serial_port *port)
 {
+	u8 gpio_dir;
 	int ret;
 
 	ret = xr_uart_enable(port);
@@ -510,6 +511,13 @@ static int xr_open(struct tty_struct *tty, struct usb_serial_port *port)
 		return ret;
 	}
 
+	/*
+	 * Configure DTR and RTS as outputs and RI, CD, DSR and CTS as
+	 * inputs.
+	 */
+	gpio_dir = XR21V141X_UART_MODE_DTR | XR21V141X_UART_MODE_RTS;
+	xr_set_reg_uart(port, XR21V141X_REG_GPIO_DIR, gpio_dir);
+
 	/* Setup termios */
 	if (tty)
 		xr_set_termios(tty, port, NULL);
-- 
2.26.2


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

* [PATCH 10/10] USB: serial: xr: fix B0 handling
  2021-01-21 10:29 [PATCH 00/10] USB: serial: xr: fix up remaining issues in new driver Johan Hovold
                   ` (9 preceding siblings ...)
  2021-01-21 10:29 ` [PATCH 09/10] USB: serial: xr: fix pin configuration Johan Hovold
@ 2021-01-21 10:29 ` Johan Hovold
  2021-01-26 15:26 ` [PATCH 00/10] USB: serial: xr: fix up remaining issues in new driver Johan Hovold
  11 siblings, 0 replies; 14+ messages in thread
From: Johan Hovold @ 2021-01-21 10:29 UTC (permalink / raw)
  To: linux-usb; +Cc: Manivannan Sadhasivam, linux-kernel, Johan Hovold

Fix up B0 handling which should leave the baud rate unchanged and
specifically not report back a non-B0 rate when B0 is requested; must
temporarily disable hardware flow control so that RTS can be deasserted;
and should reassert DTR/RTS when moving from B0.

Fixes: a8f54b7bd132 ("USB: serial: add MaxLinear/Exar USB to Serial driver")
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/xr_serial.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/serial/xr_serial.c b/drivers/usb/serial/xr_serial.c
index f67e7dba9509..483d07dee19d 100644
--- a/drivers/usb/serial/xr_serial.c
+++ b/drivers/usb/serial/xr_serial.c
@@ -341,8 +341,11 @@ static int xr_set_baudrate(struct tty_struct *tty,
 	u16 tx_mask, rx_mask;
 	int ret;
 
-	baud = clamp(tty->termios.c_ospeed, XR21V141X_MIN_SPEED,
-		     XR21V141X_MAX_SPEED);
+	baud = tty->termios.c_ospeed;
+	if (!baud)
+		return 0;
+
+	baud = clamp(baud, XR21V141X_MIN_SPEED, XR21V141X_MAX_SPEED);
 	divisor = XR_INT_OSC_HZ / baud;
 	idx = ((32 * XR_INT_OSC_HZ) / baud) & 0x1f;
 	tx_mask = xr21v141x_txrx_clk_masks[idx].tx;
@@ -399,7 +402,8 @@ static int xr_set_baudrate(struct tty_struct *tty,
 }
 
 static void xr_set_flow_mode(struct tty_struct *tty,
-			     struct usb_serial_port *port)
+			     struct usb_serial_port *port,
+			     struct ktermios *old_termios)
 {
 	u8 flow, gpio_mode;
 	int ret;
@@ -411,7 +415,7 @@ static void xr_set_flow_mode(struct tty_struct *tty,
 	/* Set GPIO mode for controlling the pins manually by default. */
 	gpio_mode &= ~XR21V141X_UART_MODE_GPIO_MASK;
 
-	if (C_CRTSCTS(tty)) {
+	if (C_CRTSCTS(tty) && C_BAUD(tty) != B0) {
 		dev_dbg(&port->dev, "Enabling hardware flow ctrl\n");
 		gpio_mode |= XR21V141X_UART_MODE_RTS_CTS;
 		flow = XR21V141X_UART_FLOW_MODE_HW;
@@ -438,6 +442,11 @@ static void xr_set_flow_mode(struct tty_struct *tty,
 	xr_uart_enable(port);
 
 	xr_set_reg_uart(port, XR21V141X_REG_GPIO_MODE, gpio_mode);
+
+	if (C_BAUD(tty) == B0)
+		xr_dtr_rts(port, 0);
+	else if (old_termios && (old_termios->c_cflag & CBAUD) == B0)
+		xr_dtr_rts(port, 1);
 }
 
 static void xr_set_termios(struct tty_struct *tty,
@@ -493,11 +502,7 @@ static void xr_set_termios(struct tty_struct *tty,
 	if (ret)
 		return;
 
-	/* If baud rate is B0, clear DTR and RTS */
-	if (C_BAUD(tty) == B0)
-		xr_dtr_rts(port, 0);
-
-	xr_set_flow_mode(tty, port);
+	xr_set_flow_mode(tty, port, old_termios);
 }
 
 static int xr_open(struct tty_struct *tty, struct usb_serial_port *port)
-- 
2.26.2


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

* Re: [PATCH] USB: serial: cp210x: suppress modem-control error on open and close
  2021-01-21 10:29 ` [PATCH] USB: serial: cp210x: suppress modem-control error on open and close Johan Hovold
@ 2021-01-21 10:32   ` Johan Hovold
  0 siblings, 0 replies; 14+ messages in thread
From: Johan Hovold @ 2021-01-21 10:32 UTC (permalink / raw)
  To: linux-usb; +Cc: Manivannan Sadhasivam, linux-kernel, Johan Hovold, Pho Tran

On Thu, Jan 21, 2021 at 11:29:12AM +0100, Johan Hovold wrote:
> The CP210X_SET_MHS request cannot be used to control DTR/RTS when
> hardware flow control is enabled and instead returns an error which is
> currently logged as:
> 
> 	cp210x ttyUSB0: failed set request 0x7 status: -32
> 
> Add a crtscts flag to keep track of the hardware flow-control setting
> and use it to suppress the request in dtr_rts().
> 
> Note that both lines are still deasserted when disabling the UART as
> part of close().
> 
> Also note that TIOCMSET is left unchanged and will continue to return an
> error to user-space when flow control is enabled (i.e. instead of
> disabling and re-enabling auto-RTS when RTS is deasserted and
> re-asserted).
> 
> Reported-by: Pho Tran <pho.tran@silabs.com>
> Signed-off-by: Johan Hovold <johan@kernel.org>
> ---

Oops, this one wasn't supposed to be submitted again. Please ignore.

Johan

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

* Re: [PATCH 00/10] USB: serial: xr: fix up remaining issues in new driver
  2021-01-21 10:29 [PATCH 00/10] USB: serial: xr: fix up remaining issues in new driver Johan Hovold
                   ` (10 preceding siblings ...)
  2021-01-21 10:29 ` [PATCH 10/10] USB: serial: xr: fix B0 handling Johan Hovold
@ 2021-01-26 15:26 ` Johan Hovold
  11 siblings, 0 replies; 14+ messages in thread
From: Johan Hovold @ 2021-01-26 15:26 UTC (permalink / raw)
  To: linux-usb; +Cc: Manivannan Sadhasivam, linux-kernel, Johan Hovold

On Thu, Jan 21, 2021 at 11:29:11AM +0100, Johan Hovold wrote:
> This series fixes the remaining issues in the new MaxLinear driver that
> were pointed out here:
> 
> 	https://lore.kernel.org/r/YAlVLOqzx8otPgOg@hovoldconsulting.com

> Johan Hovold (10):
>   USB: serial: xr: fix NULL-deref at probe
>   USB: serial: xr: fix interface leak at disconnect
>   USB: serial: xr: use subsystem usb_device at probe
>   USB: serial: xr: use termios flag helpers
>   USB: serial: xr: document vendor-request recipient
>   USB: serial: xr: clean up line-settings handling
>   USB: serial: xr: simplify line-speed logic
>   USB: serial: xr: fix gpio-mode handling
>   USB: serial: xr: fix pin configuration
>   USB: serial: xr: fix B0 handling
> 
>  drivers/usb/serial/xr_serial.c | 102 +++++++++++++++++++--------------
>  1 file changed, 59 insertions(+), 43 deletions(-)

I have applied these now.

Johan

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

end of thread, other threads:[~2021-01-26 15:28 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-21 10:29 [PATCH 00/10] USB: serial: xr: fix up remaining issues in new driver Johan Hovold
2021-01-21 10:29 ` [PATCH] USB: serial: cp210x: suppress modem-control error on open and close Johan Hovold
2021-01-21 10:32   ` Johan Hovold
2021-01-21 10:29 ` [PATCH 01/10] USB: serial: xr: fix NULL-deref at probe Johan Hovold
2021-01-21 10:29 ` [PATCH 02/10] USB: serial: xr: fix interface leak at disconnect Johan Hovold
2021-01-21 10:29 ` [PATCH 03/10] USB: serial: xr: use subsystem usb_device at probe Johan Hovold
2021-01-21 10:29 ` [PATCH 04/10] USB: serial: xr: use termios flag helpers Johan Hovold
2021-01-21 10:29 ` [PATCH 05/10] USB: serial: xr: document vendor-request recipient Johan Hovold
2021-01-21 10:29 ` [PATCH 06/10] USB: serial: xr: clean up line-settings handling Johan Hovold
2021-01-21 10:29 ` [PATCH 07/10] USB: serial: xr: simplify line-speed logic Johan Hovold
2021-01-21 10:29 ` [PATCH 08/10] USB: serial: xr: fix gpio-mode handling Johan Hovold
2021-01-21 10:29 ` [PATCH 09/10] USB: serial: xr: fix pin configuration Johan Hovold
2021-01-21 10:29 ` [PATCH 10/10] USB: serial: xr: fix B0 handling Johan Hovold
2021-01-26 15:26 ` [PATCH 00/10] USB: serial: xr: fix up remaining issues in new driver 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).