All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/7] Add support for the other MaxLinear/Exar UARTs
@ 2021-03-24  7:41 Mauro Carvalho Chehab
  2021-03-24  7:41 ` [PATCH v2 1/7] USB: serial: xr: simplify its namespace Mauro Carvalho Chehab
                   ` (7 more replies)
  0 siblings, 8 replies; 15+ messages in thread
From: Mauro Carvalho Chehab @ 2021-03-24  7:41 UTC (permalink / raw)
  To: Johan Hovold, Greg Kroah-Hartman
  Cc: Mauro Carvalho Chehab, Oliver Neukum, linux-kernel, linux-usb

The current version of the xr_serial driver handles one one of the several
MaxLinear/Exar UARTs and UART bridges. There are currently 12 such
models. Only one is currently supported.

They were tested only on  XR21B1410 and  XR21B1424 models,
although I'm pretty much confident that the other models will also
work, as they're very close to either one of those.

Patch 1 is just a cleanup for the registers namespace. It removes
the model number from registers that are used on multiple versions
of the chipset.

Patch 2 adds a table to be used to map the register for each specific
variant of this chipset;

Patch 3 adds support for XR21B1412 and XR21B1414. The only
difference on those devices is that they support multiple UART ports.

Patch 4 adds support for  XR21B142X, which uses a mix of data and
control interfaces to setup the line discipline.

Patch 5 adds support for  XR21B1410 variant. This one is identical
to  XR21B1411, except that it uses a different register set;

Patch 6 adds support for XR2280X devices:
	https://www.maxlinear.com/product/interface/bridges/usb-ethernet-bridges/xr22800

Such devices are compound devices that are similar to the USB
UARTs from programming PoV. The only difference is the register
set.

Patch 7 updates the CDC ignore list.

After this series, all MaxLinear USB UARTs described at:
	https://www.maxlinear.com/products/interface/uarts/usb-uarts

Should be supported, except for XR21B1421. This one have just a control
interface, and uses standard HID protocol. No idea how to support it. Also,
I don't have such device to test ;-)

In order to write this patch series, I used the datasheets, plus the Linux
driver source code, available at MaxLinear, and a previous port made by
Patong Yang:

   https://lore.kernel.org/linux-usb/20180404070634.nhspvmxcjwfgjkcv@advantechmxl-desktop

---
v2:
  - rebased on the top of next-20210323;
  - free port_priv before calling usb_set_serial_data.

Mauro Carvalho Chehab (7):
  USB: serial: xr: simplify its namespace
  USB: serial: xr: use a table for device-specific settings
  USB: serial: xr: add support for multi-port XR21V141X variants
  USB: serial: xr: add support for XR21B142X devices
  USB: serial: xr: add support for XR21B1411
  USB: serial: xr: add support for XR2280X devices
  USB: cdc-acm: add other non-standard xr_serial models to ignore list

 drivers/usb/class/cdc-acm.c    |  17 +-
 drivers/usb/serial/xr_serial.c | 694 +++++++++++++++++++++++++--------
 2 files changed, 556 insertions(+), 155 deletions(-)

-- 
2.30.2



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

* [PATCH v2 1/7] USB: serial: xr: simplify its namespace
  2021-03-24  7:41 [PATCH v2 0/7] Add support for the other MaxLinear/Exar UARTs Mauro Carvalho Chehab
@ 2021-03-24  7:41 ` Mauro Carvalho Chehab
  2021-03-24  7:41 ` [PATCH v2 2/7] USB: serial: xr: use a table for device-specific settings Mauro Carvalho Chehab
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 15+ messages in thread
From: Mauro Carvalho Chehab @ 2021-03-24  7:41 UTC (permalink / raw)
  To: Johan Hovold, Greg Kroah-Hartman
  Cc: Mauro Carvalho Chehab, linux-kernel, linux-usb

There are several registers that work with different models
of this chipset. Simplify their namespaces.

No functional changes.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 drivers/usb/serial/xr_serial.c | 224 ++++++++++++++++-----------------
 1 file changed, 112 insertions(+), 112 deletions(-)

diff --git a/drivers/usb/serial/xr_serial.c b/drivers/usb/serial/xr_serial.c
index 0ca04906da4b..169c7ef11d73 100644
--- a/drivers/usb/serial/xr_serial.c
+++ b/drivers/usb/serial/xr_serial.c
@@ -25,73 +25,73 @@ struct xr_txrx_clk_mask {
 };
 
 #define XR_INT_OSC_HZ			48000000U
-#define XR21V141X_MIN_SPEED		46U
-#define XR21V141X_MAX_SPEED		XR_INT_OSC_HZ
+#define MIN_SPEED			46U
+#define MAX_SPEED			XR_INT_OSC_HZ
 
 /* USB Requests */
-#define XR21V141X_SET_REQ		0
-#define XR21V141X_GET_REQ		1
-
-#define XR21V141X_CLOCK_DIVISOR_0	0x04
-#define XR21V141X_CLOCK_DIVISOR_1	0x05
-#define XR21V141X_CLOCK_DIVISOR_2	0x06
-#define XR21V141X_TX_CLOCK_MASK_0	0x07
-#define XR21V141X_TX_CLOCK_MASK_1	0x08
-#define XR21V141X_RX_CLOCK_MASK_0	0x09
-#define XR21V141X_RX_CLOCK_MASK_1	0x0a
-
-/* XR21V141X register blocks */
-#define XR21V141X_UART_REG_BLOCK	0
-#define XR21V141X_UM_REG_BLOCK		4
-#define XR21V141X_UART_CUSTOM_BLOCK	0x66
-
-/* XR21V141X UART Manager Registers */
-#define XR21V141X_UM_FIFO_ENABLE_REG	0x10
-#define XR21V141X_UM_ENABLE_TX_FIFO	0x01
-#define XR21V141X_UM_ENABLE_RX_FIFO	0x02
-
-#define XR21V141X_UM_RX_FIFO_RESET	0x18
-#define XR21V141X_UM_TX_FIFO_RESET	0x1c
-
-#define XR21V141X_UART_ENABLE_TX	0x1
-#define XR21V141X_UART_ENABLE_RX	0x2
-
-#define XR21V141X_UART_MODE_RI		BIT(0)
-#define XR21V141X_UART_MODE_CD		BIT(1)
-#define XR21V141X_UART_MODE_DSR		BIT(2)
-#define XR21V141X_UART_MODE_DTR		BIT(3)
-#define XR21V141X_UART_MODE_CTS		BIT(4)
-#define XR21V141X_UART_MODE_RTS		BIT(5)
-
-#define XR21V141X_UART_BREAK_ON		0xff
-#define XR21V141X_UART_BREAK_OFF	0
-
-#define XR21V141X_UART_DATA_MASK	GENMASK(3, 0)
-#define XR21V141X_UART_DATA_7		0x7
-#define XR21V141X_UART_DATA_8		0x8
-
-#define XR21V141X_UART_PARITY_MASK	GENMASK(6, 4)
-#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	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
-#define XR21V141X_UART_FLOW_MODE_SW	0x2
-
-#define XR21V141X_UART_MODE_GPIO_MASK	GENMASK(2, 0)
-#define XR21V141X_UART_MODE_RTS_CTS	0x1
-#define XR21V141X_UART_MODE_DTR_DSR	0x2
-#define XR21V141X_UART_MODE_RS485	0x3
-#define XR21V141X_UART_MODE_RS485_ADDR	0x4
+#define SET_REQ				0
+#define GET_REQ				1
+
+#define CLOCK_DIVISOR_0			0x04
+#define CLOCK_DIVISOR_1			0x05
+#define CLOCK_DIVISOR_2			0x06
+#define TX_CLOCK_MASK_0			0x07
+#define TX_CLOCK_MASK_1			0x08
+#define RX_CLOCK_MASK_0			0x09
+#define RX_CLOCK_MASK_1			0x0a
+
+/* Register blocks */
+#define UART_REG_BLOCK			0
+#define UM_REG_BLOCK			4
+#define UART_CUSTOM_BLOCK		0x66
+
+/* UART Manager Registers */
+#define UM_FIFO_ENABLE_REG		0x10
+#define UM_ENABLE_TX_FIFO		0x01
+#define UM_ENABLE_RX_FIFO		0x02
+
+#define UM_RX_FIFO_RESET		0x18
+#define UM_TX_FIFO_RESET		0x1c
+
+#define UART_ENABLE_TX			0x1
+#define UART_ENABLE_RX			0x2
+
+#define UART_MODE_RI			BIT(0)
+#define UART_MODE_CD			BIT(1)
+#define UART_MODE_DSR			BIT(2)
+#define UART_MODE_DTR			BIT(3)
+#define UART_MODE_CTS			BIT(4)
+#define UART_MODE_RTS			BIT(5)
+
+#define UART_BREAK_ON			0xff
+#define UART_BREAK_OFF			0
+
+#define UART_DATA_MASK			GENMASK(3, 0)
+#define UART_DATA_7			0x7
+#define UART_DATA_8			0x8
+
+#define UART_PARITY_MASK		GENMASK(6, 4)
+#define UART_PARITY_SHIFT		4
+#define UART_PARITY_NONE		(0x0 << UART_PARITY_SHIFT)
+#define UART_PARITY_ODD			(0x1 << UART_PARITY_SHIFT)
+#define UART_PARITY_EVEN		(0x2 << UART_PARITY_SHIFT)
+#define UART_PARITY_MARK		(0x3 << UART_PARITY_SHIFT)
+#define UART_PARITY_SPACE		(0x4 << UART_PARITY_SHIFT)
+
+#define UART_STOP_MASK			BIT(7)
+#define UART_STOP_SHIFT			7
+#define UART_STOP_1			(0x0 << UART_STOP_SHIFT)
+#define UART_STOP_2			(0x1 << UART_STOP_SHIFT)
+
+#define UART_FLOW_MODE_NONE		0x0
+#define UART_FLOW_MODE_HW		0x1
+#define UART_FLOW_MODE_SW		0x2
+
+#define UART_MODE_GPIO_MASK		GENMASK(2, 0)
+#define UART_MODE_RTS_CTS		0x1
+#define UART_MODE_DTR_DSR		0x2
+#define UART_MODE_RS485			0x3
+#define UART_MODE_RS485_ADDR		0x4
 
 #define XR21V141X_REG_ENABLE		0x03
 #define XR21V141X_REG_FORMAT		0x0b
@@ -115,7 +115,7 @@ 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,
+			      SET_REQ,
 			      USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
 			      val, reg | (block << 8), NULL, 0,
 			      USB_CTRL_SET_TIMEOUT);
@@ -139,7 +139,7 @@ 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,
+			      GET_REQ,
 			      USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
 			      0, reg | (block << 8), dmabuf, 1,
 			      USB_CTRL_GET_TIMEOUT);
@@ -159,17 +159,17 @@ static int xr_get_reg(struct usb_serial_port *port, u8 block, u8 reg, u8 *val)
 
 static int xr_set_reg_uart(struct usb_serial_port *port, u8 reg, u8 val)
 {
-	return xr_set_reg(port, XR21V141X_UART_REG_BLOCK, reg, val);
+	return xr_set_reg(port, UART_REG_BLOCK, reg, val);
 }
 
 static int xr_get_reg_uart(struct usb_serial_port *port, u8 reg, u8 *val)
 {
-	return xr_get_reg(port, XR21V141X_UART_REG_BLOCK, reg, val);
+	return xr_get_reg(port, UART_REG_BLOCK, reg, val);
 }
 
 static int xr_set_reg_um(struct usb_serial_port *port, u8 reg, u8 val)
 {
-	return xr_set_reg(port, XR21V141X_UM_REG_BLOCK, reg, val);
+	return xr_set_reg(port, UM_REG_BLOCK, reg, val);
 }
 
 /*
@@ -184,18 +184,18 @@ static int xr_uart_enable(struct usb_serial_port *port)
 {
 	int ret;
 
-	ret = xr_set_reg_um(port, XR21V141X_UM_FIFO_ENABLE_REG,
-			    XR21V141X_UM_ENABLE_TX_FIFO);
+	ret = xr_set_reg_um(port, UM_FIFO_ENABLE_REG,
+			    UM_ENABLE_TX_FIFO);
 	if (ret)
 		return ret;
 
 	ret = xr_set_reg_uart(port, XR21V141X_REG_ENABLE,
-			      XR21V141X_UART_ENABLE_TX | XR21V141X_UART_ENABLE_RX);
+			      UART_ENABLE_TX | UART_ENABLE_RX);
 	if (ret)
 		return ret;
 
-	ret = xr_set_reg_um(port, XR21V141X_UM_FIFO_ENABLE_REG,
-			    XR21V141X_UM_ENABLE_TX_FIFO | XR21V141X_UM_ENABLE_RX_FIFO);
+	ret = xr_set_reg_um(port, UM_FIFO_ENABLE_REG,
+			    UM_ENABLE_TX_FIFO | UM_ENABLE_RX_FIFO);
 
 	if (ret)
 		xr_set_reg_uart(port, XR21V141X_REG_ENABLE, 0);
@@ -211,7 +211,7 @@ static int xr_uart_disable(struct usb_serial_port *port)
 	if (ret)
 		return ret;
 
-	ret = xr_set_reg_um(port, XR21V141X_UM_FIFO_ENABLE_REG, 0);
+	ret = xr_set_reg_um(port, UM_FIFO_ENABLE_REG, 0);
 
 	return ret;
 }
@@ -230,12 +230,12 @@ static int xr_tiocmget(struct tty_struct *tty)
 	 * Modem control pins are active low, so reading '0' means it is active
 	 * and '1' means not active.
 	 */
-	ret = ((status & XR21V141X_UART_MODE_DTR) ? 0 : TIOCM_DTR) |
-	      ((status & XR21V141X_UART_MODE_RTS) ? 0 : TIOCM_RTS) |
-	      ((status & XR21V141X_UART_MODE_CTS) ? 0 : TIOCM_CTS) |
-	      ((status & XR21V141X_UART_MODE_DSR) ? 0 : TIOCM_DSR) |
-	      ((status & XR21V141X_UART_MODE_RI) ? 0 : TIOCM_RI) |
-	      ((status & XR21V141X_UART_MODE_CD) ? 0 : TIOCM_CD);
+	ret = ((status & UART_MODE_DTR) ? 0 : TIOCM_DTR) |
+	      ((status & UART_MODE_RTS) ? 0 : TIOCM_RTS) |
+	      ((status & UART_MODE_CTS) ? 0 : TIOCM_CTS) |
+	      ((status & UART_MODE_DSR) ? 0 : TIOCM_DSR) |
+	      ((status & UART_MODE_RI) ? 0 : TIOCM_RI) |
+	      ((status & UART_MODE_CD) ? 0 : TIOCM_CD);
 
 	return ret;
 }
@@ -249,13 +249,13 @@ static int xr_tiocmset_port(struct usb_serial_port *port,
 
 	/* Modem control pins are active low, so set & clr are swapped */
 	if (set & TIOCM_RTS)
-		gpio_clr |= XR21V141X_UART_MODE_RTS;
+		gpio_clr |= UART_MODE_RTS;
 	if (set & TIOCM_DTR)
-		gpio_clr |= XR21V141X_UART_MODE_DTR;
+		gpio_clr |= UART_MODE_DTR;
 	if (clear & TIOCM_RTS)
-		gpio_set |= XR21V141X_UART_MODE_RTS;
+		gpio_set |= UART_MODE_RTS;
 	if (clear & TIOCM_DTR)
-		gpio_set |= XR21V141X_UART_MODE_DTR;
+		gpio_set |= UART_MODE_DTR;
 
 	/* Writing '0' to gpio_{set/clr} bits has no effect, so no need to do */
 	if (gpio_clr)
@@ -289,12 +289,12 @@ static void xr_break_ctl(struct tty_struct *tty, int break_state)
 	u8 state;
 
 	if (break_state == 0)
-		state = XR21V141X_UART_BREAK_OFF;
+		state = UART_BREAK_OFF;
 	else
-		state = XR21V141X_UART_BREAK_ON;
+		state = UART_BREAK_ON;
 
 	dev_dbg(&port->dev, "Turning break %s\n",
-		state == XR21V141X_UART_BREAK_OFF ? "off" : "on");
+		state == UART_BREAK_OFF ? "off" : "on");
 	xr_set_reg_uart(port, XR21V141X_REG_TX_BREAK, state);
 }
 
@@ -345,7 +345,7 @@ static int xr_set_baudrate(struct tty_struct *tty,
 	if (!baud)
 		return 0;
 
-	baud = clamp(baud, XR21V141X_MIN_SPEED, XR21V141X_MAX_SPEED);
+	baud = clamp(baud, MIN_SPEED, MAX_SPEED);
 	divisor = XR_INT_OSC_HZ / baud;
 	idx = ((32 * XR_INT_OSC_HZ) / baud) & 0x1f;
 	tx_mask = xr21v141x_txrx_clk_masks[idx].tx;
@@ -361,37 +361,37 @@ static int xr_set_baudrate(struct tty_struct *tty,
 	 * oscillator and 19-bit programmable divisor. So theoretically it can
 	 * generate most commonly used baud rates with high accuracy.
 	 */
-	ret = xr_set_reg_uart(port, XR21V141X_CLOCK_DIVISOR_0,
+	ret = xr_set_reg_uart(port, CLOCK_DIVISOR_0,
 			      divisor & 0xff);
 	if (ret)
 		return ret;
 
-	ret = xr_set_reg_uart(port, XR21V141X_CLOCK_DIVISOR_1,
+	ret = xr_set_reg_uart(port, CLOCK_DIVISOR_1,
 			      (divisor >>  8) & 0xff);
 	if (ret)
 		return ret;
 
-	ret = xr_set_reg_uart(port, XR21V141X_CLOCK_DIVISOR_2,
+	ret = xr_set_reg_uart(port, CLOCK_DIVISOR_2,
 			      (divisor >> 16) & 0xff);
 	if (ret)
 		return ret;
 
-	ret = xr_set_reg_uart(port, XR21V141X_TX_CLOCK_MASK_0,
+	ret = xr_set_reg_uart(port, TX_CLOCK_MASK_0,
 			      tx_mask & 0xff);
 	if (ret)
 		return ret;
 
-	ret = xr_set_reg_uart(port, XR21V141X_TX_CLOCK_MASK_1,
+	ret = xr_set_reg_uart(port, TX_CLOCK_MASK_1,
 			      (tx_mask >>  8) & 0xff);
 	if (ret)
 		return ret;
 
-	ret = xr_set_reg_uart(port, XR21V141X_RX_CLOCK_MASK_0,
+	ret = xr_set_reg_uart(port, RX_CLOCK_MASK_0,
 			      rx_mask & 0xff);
 	if (ret)
 		return ret;
 
-	ret = xr_set_reg_uart(port, XR21V141X_RX_CLOCK_MASK_1,
+	ret = xr_set_reg_uart(port, RX_CLOCK_MASK_1,
 			      (rx_mask >>  8) & 0xff);
 	if (ret)
 		return ret;
@@ -413,24 +413,24 @@ static void xr_set_flow_mode(struct tty_struct *tty,
 		return;
 
 	/* Set GPIO mode for controlling the pins manually by default. */
-	gpio_mode &= ~XR21V141X_UART_MODE_GPIO_MASK;
+	gpio_mode &= ~UART_MODE_GPIO_MASK;
 
 	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;
+		gpio_mode |= UART_MODE_RTS_CTS;
+		flow = UART_FLOW_MODE_HW;
 	} else if (I_IXON(tty)) {
 		u8 start_char = START_CHAR(tty);
 		u8 stop_char = STOP_CHAR(tty);
 
 		dev_dbg(&port->dev, "Enabling sw flow ctrl\n");
-		flow = XR21V141X_UART_FLOW_MODE_SW;
+		flow = UART_FLOW_MODE_SW;
 
 		xr_set_reg_uart(port, XR21V141X_REG_XON_CHAR, start_char);
 		xr_set_reg_uart(port, XR21V141X_REG_XOFF_CHAR, stop_char);
 	} else {
 		dev_dbg(&port->dev, "Disabling flow ctrl\n");
-		flow = XR21V141X_UART_FLOW_MODE_NONE;
+		flow = UART_FLOW_MODE_NONE;
 	}
 
 	/*
@@ -468,35 +468,35 @@ static void xr_set_termios(struct tty_struct *tty,
 		if (old_termios)
 			termios->c_cflag |= old_termios->c_cflag & CSIZE;
 		else
-			bits |= XR21V141X_UART_DATA_8;
+			bits |= UART_DATA_8;
 		break;
 	case CS7:
-		bits |= XR21V141X_UART_DATA_7;
+		bits |= UART_DATA_7;
 		break;
 	case CS8:
 	default:
-		bits |= XR21V141X_UART_DATA_8;
+		bits |= UART_DATA_8;
 		break;
 	}
 
 	if (C_PARENB(tty)) {
 		if (C_CMSPAR(tty)) {
 			if (C_PARODD(tty))
-				bits |= XR21V141X_UART_PARITY_MARK;
+				bits |= UART_PARITY_MARK;
 			else
-				bits |= XR21V141X_UART_PARITY_SPACE;
+				bits |= UART_PARITY_SPACE;
 		} else {
 			if (C_PARODD(tty))
-				bits |= XR21V141X_UART_PARITY_ODD;
+				bits |= UART_PARITY_ODD;
 			else
-				bits |= XR21V141X_UART_PARITY_EVEN;
+				bits |= UART_PARITY_EVEN;
 		}
 	}
 
 	if (C_CSTOPB(tty))
-		bits |= XR21V141X_UART_STOP_2;
+		bits |= UART_STOP_2;
 	else
-		bits |= XR21V141X_UART_STOP_1;
+		bits |= UART_STOP_1;
 
 	ret = xr_set_reg_uart(port, XR21V141X_REG_FORMAT, bits);
 	if (ret)
@@ -520,7 +520,7 @@ static int xr_open(struct tty_struct *tty, struct usb_serial_port *port)
 	 * Configure DTR and RTS as outputs and RI, CD, DSR and CTS as
 	 * inputs.
 	 */
-	gpio_dir = XR21V141X_UART_MODE_DTR | XR21V141X_UART_MODE_RTS;
+	gpio_dir = UART_MODE_DTR | UART_MODE_RTS;
 	xr_set_reg_uart(port, XR21V141X_REG_GPIO_DIR, gpio_dir);
 
 	/* Setup termios */
-- 
2.30.2


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

* [PATCH v2 2/7] USB: serial: xr: use a table for device-specific settings
  2021-03-24  7:41 [PATCH v2 0/7] Add support for the other MaxLinear/Exar UARTs Mauro Carvalho Chehab
  2021-03-24  7:41 ` [PATCH v2 1/7] USB: serial: xr: simplify its namespace Mauro Carvalho Chehab
@ 2021-03-24  7:41 ` Mauro Carvalho Chehab
  2021-03-30 14:44   ` Johan Hovold
  2021-03-24  7:41 ` [PATCH v2 3/7] USB: serial: xr: add support for multi-port XR21V141X variants Mauro Carvalho Chehab
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 15+ messages in thread
From: Mauro Carvalho Chehab @ 2021-03-24  7:41 UTC (permalink / raw)
  To: Johan Hovold, Greg Kroah-Hartman
  Cc: Mauro Carvalho Chehab, linux-kernel, linux-usb

The same driver is used by a wide range of MaxLinear devices.

Other models are close enough to use the same driver, but they
use a different register set.

So, instead of having the registers hardcoded at the driver,
use a table. This will allow further patches to add support for
other devices.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 drivers/usb/serial/xr_serial.c | 151 ++++++++++++++++++++++++---------
 1 file changed, 113 insertions(+), 38 deletions(-)

diff --git a/drivers/usb/serial/xr_serial.c b/drivers/usb/serial/xr_serial.c
index 169c7ef11d73..518c4725431a 100644
--- a/drivers/usb/serial/xr_serial.c
+++ b/drivers/usb/serial/xr_serial.c
@@ -28,10 +28,6 @@ struct xr_txrx_clk_mask {
 #define MIN_SPEED			46U
 #define MAX_SPEED			XR_INT_OSC_HZ
 
-/* USB Requests */
-#define SET_REQ				0
-#define GET_REQ				1
-
 #define CLOCK_DIVISOR_0			0x04
 #define CLOCK_DIVISOR_1			0x05
 #define CLOCK_DIVISOR_2			0x06
@@ -93,29 +89,73 @@ struct xr_txrx_clk_mask {
 #define UART_MODE_RS485			0x3
 #define UART_MODE_RS485_ADDR		0x4
 
-#define XR21V141X_REG_ENABLE		0x03
-#define XR21V141X_REG_FORMAT		0x0b
-#define XR21V141X_REG_FLOW_CTRL		0x0c
-#define XR21V141X_REG_XON_CHAR		0x10
-#define XR21V141X_REG_XOFF_CHAR		0x11
-#define XR21V141X_REG_LOOPBACK		0x12
-#define XR21V141X_REG_TX_BREAK		0x14
-#define XR21V141X_REG_RS845_DELAY	0x15
-#define XR21V141X_REG_GPIO_MODE		0x1a
-#define XR21V141X_REG_GPIO_DIR		0x1b
-#define XR21V141X_REG_GPIO_INT_MASK	0x1c
-#define XR21V141X_REG_GPIO_SET		0x1d
-#define XR21V141X_REG_GPIO_CLR		0x1e
-#define XR21V141X_REG_GPIO_STATUS	0x1f
+enum xr_model {
+	XR21V141X,
+	MAX_XR_MODELS
+};
+
+enum xr_hal_type {
+	REG_ENABLE,
+	REG_FORMAT,
+	REG_FLOW_CTRL,
+	REG_XON_CHAR,
+	REG_XOFF_CHAR,
+	REG_TX_BREAK,
+	REG_RS485_DELAY,
+	REG_GPIO_MODE,
+	REG_GPIO_DIR,
+	REG_GPIO_SET,
+	REG_GPIO_CLR,
+	REG_GPIO_STATUS,
+	REG_GPIO_INT_MASK,
+	REG_CUSTOMIZED_INT,
+	REG_GPIO_PULL_UP_ENABLE,
+	REG_GPIO_PULL_DOWN_ENABLE,
+	REG_LOOPBACK,
+	REG_LOW_LATENCY,
+	REG_CUSTOM_DRIVER,
+
+	REQ_SET,
+	REQ_GET,
+
+	MAX_XR_HAL_TYPE
+};
+
+static const int xr_hal_table[MAX_XR_MODELS][MAX_XR_HAL_TYPE] = {
+	[XR21V141X] = {
+		[REG_ENABLE] =				0x03,
+		[REG_FORMAT] =				0x0b,
+		[REG_FLOW_CTRL] =			0x0c,
+		[REG_XON_CHAR] =			0x10,
+		[REG_XOFF_CHAR] =			0x11,
+		[REG_LOOPBACK] =			0x12,
+		[REG_TX_BREAK] =			0x14,
+		[REG_RS485_DELAY] =			0x15,
+		[REG_GPIO_MODE] =			0x1a,
+		[REG_GPIO_DIR] =			0x1b,
+		[REG_GPIO_INT_MASK] =			0x1c,
+		[REG_GPIO_SET] =			0x1d,
+		[REG_GPIO_CLR] =			0x1e,
+		[REG_GPIO_STATUS] =			0x1f,
+
+		[REQ_SET] =				0,
+		[REQ_GET] =				1,
+	}
+};
+
+struct xr_port_private {
+	enum xr_model model;
+};
 
 static int xr_set_reg(struct usb_serial_port *port, u8 block, u8 reg, u8 val)
 {
+	struct xr_port_private *port_priv = usb_get_serial_data(port->serial);
 	struct usb_serial *serial = port->serial;
 	int ret;
 
 	ret = usb_control_msg(serial->dev,
 			      usb_sndctrlpipe(serial->dev, 0),
-			      SET_REQ,
+			      xr_hal_table[port_priv->model][REQ_SET],
 			      USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
 			      val, reg | (block << 8), NULL, 0,
 			      USB_CTRL_SET_TIMEOUT);
@@ -129,6 +169,7 @@ static int xr_set_reg(struct usb_serial_port *port, u8 block, u8 reg, u8 val)
 
 static int xr_get_reg(struct usb_serial_port *port, u8 block, u8 reg, u8 *val)
 {
+	struct xr_port_private *port_priv = usb_get_serial_data(port->serial);
 	struct usb_serial *serial = port->serial;
 	u8 *dmabuf;
 	int ret;
@@ -139,7 +180,7 @@ 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),
-			      GET_REQ,
+			      xr_hal_table[port_priv->model][REQ_GET],
 			      USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
 			      0, reg | (block << 8), dmabuf, 1,
 			      USB_CTRL_GET_TIMEOUT);
@@ -182,6 +223,7 @@ static int xr_set_reg_um(struct usb_serial_port *port, u8 reg, u8 val)
  */
 static int xr_uart_enable(struct usb_serial_port *port)
 {
+	struct xr_port_private *port_priv = usb_get_serial_data(port->serial);
 	int ret;
 
 	ret = xr_set_reg_um(port, UM_FIFO_ENABLE_REG,
@@ -189,7 +231,7 @@ static int xr_uart_enable(struct usb_serial_port *port)
 	if (ret)
 		return ret;
 
-	ret = xr_set_reg_uart(port, XR21V141X_REG_ENABLE,
+	ret = xr_set_reg_uart(port, xr_hal_table[port_priv->model][REG_ENABLE],
 			      UART_ENABLE_TX | UART_ENABLE_RX);
 	if (ret)
 		return ret;
@@ -198,16 +240,18 @@ static int xr_uart_enable(struct usb_serial_port *port)
 			    UM_ENABLE_TX_FIFO | UM_ENABLE_RX_FIFO);
 
 	if (ret)
-		xr_set_reg_uart(port, XR21V141X_REG_ENABLE, 0);
+		xr_set_reg_uart(port, xr_hal_table[port_priv->model][REG_ENABLE], 0);
 
 	return ret;
 }
 
 static int xr_uart_disable(struct usb_serial_port *port)
 {
+	struct xr_port_private *port_priv = usb_get_serial_data(port->serial);
 	int ret;
 
-	ret = xr_set_reg_uart(port, XR21V141X_REG_ENABLE, 0);
+	ret = xr_set_reg_uart(port,
+			      xr_hal_table[port_priv->model][REG_ENABLE], 0);
 	if (ret)
 		return ret;
 
@@ -219,10 +263,13 @@ static int xr_uart_disable(struct usb_serial_port *port)
 static int xr_tiocmget(struct tty_struct *tty)
 {
 	struct usb_serial_port *port = tty->driver_data;
+	struct xr_port_private *port_priv = usb_get_serial_data(port->serial);
 	u8 status;
 	int ret;
 
-	ret = xr_get_reg_uart(port, XR21V141X_REG_GPIO_STATUS, &status);
+	ret = xr_get_reg_uart(port,
+			      xr_hal_table[port_priv->model][REG_GPIO_STATUS],
+			      &status);
 	if (ret)
 		return ret;
 
@@ -243,6 +290,7 @@ static int xr_tiocmget(struct tty_struct *tty)
 static int xr_tiocmset_port(struct usb_serial_port *port,
 			    unsigned int set, unsigned int clear)
 {
+	struct xr_port_private *port_priv = usb_get_serial_data(port->serial);
 	u8 gpio_set = 0;
 	u8 gpio_clr = 0;
 	int ret = 0;
@@ -259,10 +307,14 @@ static int xr_tiocmset_port(struct usb_serial_port *port,
 
 	/* Writing '0' to gpio_{set/clr} bits has no effect, so no need to do */
 	if (gpio_clr)
-		ret = xr_set_reg_uart(port, XR21V141X_REG_GPIO_CLR, gpio_clr);
+		ret = xr_set_reg_uart(port,
+				      xr_hal_table[port_priv->model][REG_GPIO_CLR],
+				      gpio_clr);
 
 	if (gpio_set)
-		ret = xr_set_reg_uart(port, XR21V141X_REG_GPIO_SET, gpio_set);
+		ret = xr_set_reg_uart(port,
+				      xr_hal_table[port_priv->model][REG_GPIO_SET],
+				      gpio_set);
 
 	return ret;
 }
@@ -286,6 +338,7 @@ static void xr_dtr_rts(struct usb_serial_port *port, int on)
 static void xr_break_ctl(struct tty_struct *tty, int break_state)
 {
 	struct usb_serial_port *port = tty->driver_data;
+	struct xr_port_private *port_priv = usb_get_serial_data(port->serial);
 	u8 state;
 
 	if (break_state == 0)
@@ -295,7 +348,8 @@ static void xr_break_ctl(struct tty_struct *tty, int break_state)
 
 	dev_dbg(&port->dev, "Turning break %s\n",
 		state == UART_BREAK_OFF ? "off" : "on");
-	xr_set_reg_uart(port, XR21V141X_REG_TX_BREAK, state);
+	xr_set_reg_uart(port, xr_hal_table[port_priv->model][REG_TX_BREAK],
+			state);
 }
 
 /* Tx and Rx clock mask values obtained from section 3.3.4 of datasheet */
@@ -405,10 +459,11 @@ static void xr_set_flow_mode(struct tty_struct *tty,
 			     struct usb_serial_port *port,
 			     struct ktermios *old_termios)
 {
+	struct xr_port_private *port_priv = usb_get_serial_data(port->serial);
 	u8 flow, gpio_mode;
 	int ret;
 
-	ret = xr_get_reg_uart(port, XR21V141X_REG_GPIO_MODE, &gpio_mode);
+	ret = xr_get_reg_uart(port, xr_hal_table[port_priv->model][REG_GPIO_MODE], &gpio_mode);
 	if (ret)
 		return;
 
@@ -426,8 +481,8 @@ static void xr_set_flow_mode(struct tty_struct *tty,
 		dev_dbg(&port->dev, "Enabling sw flow ctrl\n");
 		flow = UART_FLOW_MODE_SW;
 
-		xr_set_reg_uart(port, XR21V141X_REG_XON_CHAR, start_char);
-		xr_set_reg_uart(port, XR21V141X_REG_XOFF_CHAR, stop_char);
+		xr_set_reg_uart(port, xr_hal_table[port_priv->model][REG_XON_CHAR], start_char);
+		xr_set_reg_uart(port, xr_hal_table[port_priv->model][REG_XOFF_CHAR], stop_char);
 	} else {
 		dev_dbg(&port->dev, "Disabling flow ctrl\n");
 		flow = UART_FLOW_MODE_NONE;
@@ -438,10 +493,10 @@ static void xr_set_flow_mode(struct tty_struct *tty,
 	 * FLOW_CONTROL register.
 	 */
 	xr_uart_disable(port);
-	xr_set_reg_uart(port, XR21V141X_REG_FLOW_CTRL, flow);
+	xr_set_reg_uart(port, xr_hal_table[port_priv->model][REG_FLOW_CTRL], flow);
 	xr_uart_enable(port);
 
-	xr_set_reg_uart(port, XR21V141X_REG_GPIO_MODE, gpio_mode);
+	xr_set_reg_uart(port, xr_hal_table[port_priv->model][REG_GPIO_MODE], gpio_mode);
 
 	if (C_BAUD(tty) == B0)
 		xr_dtr_rts(port, 0);
@@ -453,9 +508,9 @@ static void xr_set_termios(struct tty_struct *tty,
 			   struct usb_serial_port *port,
 			   struct ktermios *old_termios)
 {
+	struct xr_port_private *port_priv = usb_get_serial_data(port->serial);
 	struct ktermios *termios = &tty->termios;
 	u8 bits = 0;
-	int ret;
 
 	if (!old_termios || (tty->termios.c_ospeed != old_termios->c_ospeed))
 		xr_set_baudrate(tty, port);
@@ -498,15 +553,16 @@ static void xr_set_termios(struct tty_struct *tty,
 	else
 		bits |= UART_STOP_1;
 
-	ret = xr_set_reg_uart(port, XR21V141X_REG_FORMAT, bits);
-	if (ret)
-		return;
+	xr_set_reg_uart(port,
+			xr_hal_table[port_priv->model][REG_FORMAT],
+			bits);
 
 	xr_set_flow_mode(tty, port, old_termios);
 }
 
 static int xr_open(struct tty_struct *tty, struct usb_serial_port *port)
 {
+	struct xr_port_private *port_priv = usb_get_serial_data(port->serial);
 	u8 gpio_dir;
 	int ret;
 
@@ -521,7 +577,7 @@ static int xr_open(struct tty_struct *tty, struct usb_serial_port *port)
 	 * inputs.
 	 */
 	gpio_dir = UART_MODE_DTR | UART_MODE_RTS;
-	xr_set_reg_uart(port, XR21V141X_REG_GPIO_DIR, gpio_dir);
+	xr_set_reg_uart(port, xr_hal_table[port_priv->model][REG_GPIO_DIR], gpio_dir);
 
 	/* Setup termios */
 	if (tty)
@@ -545,15 +601,33 @@ static void xr_close(struct usb_serial_port *port)
 
 static int xr_probe(struct usb_serial *serial, const struct usb_device_id *id)
 {
+	struct xr_port_private *port_priv;
+
 	/* Don't bind to control interface */
 	if (serial->interface->cur_altsetting->desc.bInterfaceNumber == 0)
 		return -ENODEV;
 
+	port_priv = kzalloc(sizeof(*port_priv), GFP_KERNEL);
+	if (!port_priv)
+		return -ENOMEM;
+
+	port_priv->model = id->driver_info;
+
+	usb_set_serial_data(serial, port_priv);
+
 	return 0;
 }
 
+static void xr_disconnect(struct usb_serial *serial)
+{
+	struct xr_port_private *port_priv = usb_get_serial_data(serial);
+
+	usb_set_serial_data(serial, 0);
+	kfree(port_priv);
+}
+
 static const struct usb_device_id id_table[] = {
-	{ USB_DEVICE(0x04e2, 0x1410) }, /* XR21V141X */
+	{ USB_DEVICE(0x04e2, 0x1410), .driver_info = XR21V141X},
 	{ }
 };
 MODULE_DEVICE_TABLE(usb, id_table);
@@ -566,6 +640,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.30.2


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

* [PATCH v2 3/7] USB: serial: xr: add support for multi-port XR21V141X variants
  2021-03-24  7:41 [PATCH v2 0/7] Add support for the other MaxLinear/Exar UARTs Mauro Carvalho Chehab
  2021-03-24  7:41 ` [PATCH v2 1/7] USB: serial: xr: simplify its namespace Mauro Carvalho Chehab
  2021-03-24  7:41 ` [PATCH v2 2/7] USB: serial: xr: use a table for device-specific settings Mauro Carvalho Chehab
@ 2021-03-24  7:41 ` Mauro Carvalho Chehab
  2021-03-30 14:50   ` Johan Hovold
  2021-03-24  7:41 ` [PATCH v2 4/7] USB: serial: xr: add support for XR21B142X devices Mauro Carvalho Chehab
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 15+ messages in thread
From: Mauro Carvalho Chehab @ 2021-03-24  7:41 UTC (permalink / raw)
  To: Johan Hovold, Greg Kroah-Hartman
  Cc: Mauro Carvalho Chehab, linux-kernel, linux-usb

XR21V1412 and XR21V1414 have exactly the same interface, but
they support multiple 2 and 4 ports, respectively.

On such devices, the "CDC Union" field shows how they're
grouped, as can be seen on those lsusb -v outputs:

	https://linux-usb.vger.kernel.narkive.com/YaTYwHkM/usb-uart-device-from-exar-co-not-working-with-cdc-acm-but-usbserial
	https://jquery.developreference.com/article/22043997/udev+rule+with+bInterfaceNumber+doesn't+work+%5bclosed%5d

So, for instance, on XR21V1414, (0x04e2:0x1414), the 3rd
port is:

	CDC Union:
		bMasterInterface 4
		bSlaveInterface 5

	CDC Call Management:
		bmCapabilities 0x01
			call management
				bDataInterface 5

In other words, the control interface is an even number,
and the data interface is the next odd number.

The logic to get the proper register for an specific channel
came from this patch:

	https://lore.kernel.org/linux-usb/20180404070634.nhspvmxcjwfgjkcv@advantechmxl-desktop

Add support for them.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 drivers/usb/serial/xr_serial.c | 30 ++++++++++++++++++++++++++++--
 1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/serial/xr_serial.c b/drivers/usb/serial/xr_serial.c
index 518c4725431a..f161394d9b2d 100644
--- a/drivers/usb/serial/xr_serial.c
+++ b/drivers/usb/serial/xr_serial.c
@@ -145,6 +145,7 @@ static const int xr_hal_table[MAX_XR_MODELS][MAX_XR_HAL_TYPE] = {
 
 struct xr_port_private {
 	enum xr_model model;
+	unsigned int channel;
 };
 
 static int xr_set_reg(struct usb_serial_port *port, u8 block, u8 reg, u8 val)
@@ -153,6 +154,14 @@ static int xr_set_reg(struct usb_serial_port *port, u8 block, u8 reg, u8 val)
 	struct usb_serial *serial = port->serial;
 	int ret;
 
+	switch (port_priv->model) {
+	case XR21V141X:
+		if (port_priv->channel)
+			reg |= (port_priv->channel - 1) << 8;
+		break;
+	default:
+		return -EINVAL;
+	};
 	ret = usb_control_msg(serial->dev,
 			      usb_sndctrlpipe(serial->dev, 0),
 			      xr_hal_table[port_priv->model][REQ_SET],
@@ -178,6 +187,14 @@ static int xr_get_reg(struct usb_serial_port *port, u8 block, u8 reg, u8 *val)
 	if (!dmabuf)
 		return -ENOMEM;
 
+	switch (port_priv->model) {
+	case XR21V141X:
+		if (port_priv->channel)
+			reg |= (port_priv->channel - 1) << 8;
+		break;
+	default:
+		return -EINVAL;
+	};
 	ret = usb_control_msg(serial->dev,
 			      usb_rcvctrlpipe(serial->dev, 0),
 			      xr_hal_table[port_priv->model][REQ_GET],
@@ -601,17 +618,24 @@ 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_interface *intf = serial->interface;
+	struct usb_endpoint_descriptor *data_ep;
 	struct xr_port_private *port_priv;
+	int ifnum;
 
-	/* Don't bind to control interface */
-	if (serial->interface->cur_altsetting->desc.bInterfaceNumber == 0)
+	/* Attach only data interfaces */
+	ifnum = intf->cur_altsetting->desc.bInterfaceNumber;
+	if (!(ifnum % 2))
 		return -ENODEV;
 
 	port_priv = kzalloc(sizeof(*port_priv), GFP_KERNEL);
 	if (!port_priv)
 		return -ENOMEM;
 
+	data_ep = &intf->cur_altsetting->endpoint[0].desc;
+
 	port_priv->model = id->driver_info;
+	port_priv->channel = data_ep->bEndpointAddress;
 
 	usb_set_serial_data(serial, port_priv);
 
@@ -628,6 +652,8 @@ static void xr_disconnect(struct usb_serial *serial)
 
 static const struct usb_device_id id_table[] = {
 	{ USB_DEVICE(0x04e2, 0x1410), .driver_info = XR21V141X},
+	{ USB_DEVICE(0x04e2, 0x1412), .driver_info = XR21V141X},
+	{ USB_DEVICE(0x04e2, 0x1414), .driver_info = XR21V141X},
 	{ }
 };
 MODULE_DEVICE_TABLE(usb, id_table);
-- 
2.30.2


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

* [PATCH v2 4/7] USB: serial: xr: add support for XR21B142X devices
  2021-03-24  7:41 [PATCH v2 0/7] Add support for the other MaxLinear/Exar UARTs Mauro Carvalho Chehab
                   ` (2 preceding siblings ...)
  2021-03-24  7:41 ` [PATCH v2 3/7] USB: serial: xr: add support for multi-port XR21V141X variants Mauro Carvalho Chehab
@ 2021-03-24  7:41 ` Mauro Carvalho Chehab
  2021-03-30 15:04   ` Johan Hovold
  2021-03-24  7:41 ` [PATCH v2 5/7] USB: serial: xr: add support for XR21B1411 Mauro Carvalho Chehab
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 15+ messages in thread
From: Mauro Carvalho Chehab @ 2021-03-24  7:41 UTC (permalink / raw)
  To: Johan Hovold, Greg Kroah-Hartman
  Cc: Mauro Carvalho Chehab, linux-kernel, linux-usb

Those devices can have 1, 2 or 4 ports. They're similar to
XR21B141X models, except for:

- the register numbers and reqs are different;
- some settings are done via the control interface, by
  using CDC commands.
- a few other minor differences.

The same way as XR21B141X, the control and data interfaces
are grouped via CDC union, where the even interface is for
control, and the subsequent one for data. So, on XR21B142X,

      CDC Union:
        bMasterInterface        0
        bSlaveInterface         1

        bDataInterface          1

      CDC Union:
        bMasterInterface        0
        bSlaveInterface         1

        bDataInterface          1

      CDC Union:
        bMasterInterface        2
        bSlaveInterface         3

        bDataInterface          3

      CDC Union:
        bMasterInterface        4
        bSlaveInterface         5

        bDataInterface          5

      CDC Union:
        bMasterInterface        6
        bSlaveInterface         7

        bDataInterface          7

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 drivers/usb/serial/xr_serial.c | 235 ++++++++++++++++++++++++++++++++-
 1 file changed, 231 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/serial/xr_serial.c b/drivers/usb/serial/xr_serial.c
index f161394d9b2d..058624d15505 100644
--- a/drivers/usb/serial/xr_serial.c
+++ b/drivers/usb/serial/xr_serial.c
@@ -16,6 +16,7 @@
 #include <linux/slab.h>
 #include <linux/tty.h>
 #include <linux/usb.h>
+#include <linux/usb/cdc.h>
 #include <linux/usb/serial.h>
 
 struct xr_txrx_clk_mask {
@@ -89,8 +90,17 @@ struct xr_txrx_clk_mask {
 #define UART_MODE_RS485			0x3
 #define UART_MODE_RS485_ADDR		0x4
 
+/* Used on devices that need the CDC control interface */
+#define URM_RESET_RX_FIFO_BASE		0x18
+#define URM_RESET_TX_FIFO_BASE		0x1c
+
+#define CDC_DATA_INTERFACE_TYPE		0x0a
+
+#define VIA_CDC_REGISTER		-1
+
 enum xr_model {
 	XR21V141X,
+	XR21B142X,
 	MAX_XR_MODELS
 };
 
@@ -140,12 +150,37 @@ static const int xr_hal_table[MAX_XR_MODELS][MAX_XR_HAL_TYPE] = {
 
 		[REQ_SET] =				0,
 		[REQ_GET] =				1,
+	},
+	[XR21B142X] = {
+		[REG_ENABLE] =				0x00,
+		[REG_FORMAT] =				VIA_CDC_REGISTER,
+		[REG_FLOW_CTRL] =			0x06,
+		[REG_XON_CHAR] =			0x07,
+		[REG_XOFF_CHAR] =			0x08,
+		[REG_TX_BREAK] =			0x0a,
+		[REG_RS485_DELAY] =			0x0b,
+		[REG_GPIO_MODE] =			0x0c,
+		[REG_GPIO_DIR] =			0x0d,
+		[REG_GPIO_SET] =			0x0e,
+		[REG_GPIO_CLR] =			0x0f,
+		[REG_GPIO_STATUS] =			0x10,
+		[REG_GPIO_INT_MASK] =			0x11,
+		[REG_CUSTOMIZED_INT] =			0x12,
+		[REG_GPIO_PULL_UP_ENABLE] =		0x14,
+		[REG_GPIO_PULL_DOWN_ENABLE] =		0x15,
+		[REG_LOOPBACK] =			0x16,
+		[REG_LOW_LATENCY] =			0x46,
+		[REG_CUSTOM_DRIVER] =			0x60,
+
+		[REQ_SET] =				0,
+		[REQ_GET] =				0,
 	}
 };
 
 struct xr_port_private {
 	enum xr_model model;
 	unsigned int channel;
+	struct usb_interface *control_if;
 };
 
 static int xr_set_reg(struct usb_serial_port *port, u8 block, u8 reg, u8 val)
@@ -159,6 +194,9 @@ static int xr_set_reg(struct usb_serial_port *port, u8 block, u8 reg, u8 val)
 		if (port_priv->channel)
 			reg |= (port_priv->channel - 1) << 8;
 		break;
+	case XR21B142X:
+		reg |= (port_priv->channel - 4) << 1;
+		break;
 	default:
 		return -EINVAL;
 	};
@@ -192,6 +230,9 @@ static int xr_get_reg(struct usb_serial_port *port, u8 block, u8 reg, u8 *val)
 		if (port_priv->channel)
 			reg |= (port_priv->channel - 1) << 8;
 		break;
+	case XR21B142X:
+		reg |= (port_priv->channel - 4) << 1;
+		break;
 	default:
 		return -EINVAL;
 	};
@@ -215,6 +256,43 @@ static int xr_get_reg(struct usb_serial_port *port, u8 block, u8 reg, u8 *val)
 	return ret;
 }
 
+static int xr_usb_serial_ctrl_msg(struct usb_serial_port *port,
+				  int request, int val,
+				  void *buf, int len)
+{
+	struct xr_port_private *port_priv = usb_get_serial_data(port->serial);
+	int if_num = port_priv->control_if->altsetting[0].desc.bInterfaceNumber;
+	struct usb_serial *serial = port->serial;
+	u8 *dmabuf = NULL;
+	int ret;
+
+	if (len) {
+		dmabuf = kmemdup(buf, len, GFP_KERNEL);
+		if (!dmabuf)
+			return -ENOMEM;
+	}
+
+	ret = usb_control_msg(serial->dev,
+			      usb_rcvctrlpipe(serial->dev, 0),
+			      request,
+			      USB_TYPE_CLASS | USB_RECIP_INTERFACE,
+			      val,
+			      if_num, dmabuf, len,
+			      USB_CTRL_GET_TIMEOUT);
+
+	if (ret < 0) {
+		dev_err(&port->dev, "Failed to send a control msg: %d\n", ret);
+	} else {
+		if (dmabuf)
+			memcpy(buf, dmabuf, len);
+		ret = 0;
+	}
+
+	kfree(dmabuf);
+
+	return ret;
+}
+
 static int xr_set_reg_uart(struct usb_serial_port *port, u8 reg, u8 val)
 {
 	return xr_set_reg(port, UART_REG_BLOCK, reg, val);
@@ -243,6 +321,11 @@ static int xr_uart_enable(struct usb_serial_port *port)
 	struct xr_port_private *port_priv = usb_get_serial_data(port->serial);
 	int ret;
 
+	if (port_priv->model != XR21V141X)
+		return xr_set_reg_uart(port,
+				       xr_hal_table[port_priv->model][REG_ENABLE],
+				       UART_ENABLE_TX | UART_ENABLE_RX);
+
 	ret = xr_set_reg_um(port, UM_FIFO_ENABLE_REG,
 			    UM_ENABLE_TX_FIFO);
 	if (ret)
@@ -272,11 +355,37 @@ static int xr_uart_disable(struct usb_serial_port *port)
 	if (ret)
 		return ret;
 
+	if (port_priv->model != XR21V141X)
+		return 0;
+
 	ret = xr_set_reg_um(port, UM_FIFO_ENABLE_REG, 0);
 
 	return ret;
 }
 
+static int fifo_reset(struct usb_serial_port *port)
+{
+	struct xr_port_private *port_priv = usb_get_serial_data(port->serial);
+	int channel = port_priv->channel;
+	int ret = 0;
+
+	if (port_priv->model != XR21V141X)
+		return 0;
+
+	if (channel)
+		channel--;
+
+	ret = xr_set_reg_um(port,
+			    URM_RESET_RX_FIFO_BASE + channel, 0xff);
+	if (ret)
+		return ret;
+
+	ret = xr_set_reg_um(port,
+			    URM_RESET_TX_FIFO_BASE + channel, 0xff);
+
+	return ret;
+}
+
 static int xr_tiocmget(struct tty_struct *tty)
 {
 	struct usb_serial_port *port = tty->driver_data;
@@ -358,6 +467,12 @@ static void xr_break_ctl(struct tty_struct *tty, int break_state)
 	struct xr_port_private *port_priv = usb_get_serial_data(port->serial);
 	u8 state;
 
+	if (port_priv->model != XR21V141X) {
+		xr_usb_serial_ctrl_msg(port, USB_CDC_REQ_SEND_BREAK, state,
+				       NULL, 0);
+		return;
+	}
+
 	if (break_state == 0)
 		state = UART_BREAK_OFF;
 	else
@@ -505,6 +620,13 @@ static void xr_set_flow_mode(struct tty_struct *tty,
 		flow = UART_FLOW_MODE_NONE;
 	}
 
+	/*
+	 * Add support for the TXT and RXT function for 0x1420, 0x1422, 0x1424,
+	 * by setting GPIO_MODE [9:8] = '11'
+	 */
+	if (port_priv->model == XR21B142X)
+		gpio_mode |= 0x300;
+
 	/*
 	 * As per the datasheet, UART needs to be disabled while writing to
 	 * FLOW_CONTROL register.
@@ -521,9 +643,55 @@ static void xr_set_flow_mode(struct tty_struct *tty,
 		xr_dtr_rts(port, 1);
 }
 
-static void xr_set_termios(struct tty_struct *tty,
-			   struct usb_serial_port *port,
-			   struct ktermios *old_termios)
+static void xr_set_termios_cdc(struct tty_struct *tty,
+			       struct usb_serial_port *port,
+			       struct ktermios *old_termios)
+{
+	struct ktermios *termios = &tty->termios;
+	struct usb_cdc_line_coding line = { 0 };
+	int clear, set;
+
+	line.dwDTERate = cpu_to_le32(tty_get_baud_rate(tty));
+	line.bCharFormat = termios->c_cflag & CSTOPB ? 1 : 0;
+	line.bParityType = termios->c_cflag & PARENB ?
+			   (termios->c_cflag & PARODD ? 1 : 2) +
+			   (termios->c_cflag & CMSPAR ? 2 : 0) : 0;
+
+	switch (C_CSIZE(tty)) {
+	case CS5:
+		line.bDataBits = 5;
+		break;
+	case CS6:
+		line.bDataBits = 6;
+		break;
+	case CS7:
+		line.bDataBits = 7;
+		break;
+	case CS8:
+	default:
+		line.bDataBits = 8;
+		break;
+	}
+
+	if (!line.dwDTERate) {
+		line.dwDTERate = tty->termios.c_ospeed;
+		clear = UART_MODE_DTR;
+	} else {
+		set = UART_MODE_DTR;
+	}
+
+	if (clear || set)
+		xr_tiocmset_port(port, set, clear);
+
+	xr_set_flow_mode(tty, port, old_termios);
+
+	xr_usb_serial_ctrl_msg(port, USB_CDC_REQ_SET_LINE_CODING, 0,
+			       &line, sizeof(line));
+}
+
+static void xr_set_termios_format_reg(struct tty_struct *tty,
+				      struct usb_serial_port *port,
+				      struct ktermios *old_termios)
 {
 	struct xr_port_private *port_priv = usb_get_serial_data(port->serial);
 	struct ktermios *termios = &tty->termios;
@@ -532,6 +700,8 @@ static void xr_set_termios(struct tty_struct *tty,
 	if (!old_termios || (tty->termios.c_ospeed != old_termios->c_ospeed))
 		xr_set_baudrate(tty, port);
 
+	/* For models with a private CHARACTER_FORMAT register */
+
 	switch (C_CSIZE(tty)) {
 	case CS5:
 	case CS6:
@@ -577,6 +747,28 @@ static void xr_set_termios(struct tty_struct *tty,
 	xr_set_flow_mode(tty, port, old_termios);
 }
 
+static void xr_set_termios(struct tty_struct *tty,
+			   struct usb_serial_port *port,
+			   struct ktermios *old_termios)
+{
+	struct xr_port_private *port_priv = usb_get_serial_data(port->serial);
+
+	/*
+	 * Different models have different ways to setup character format:
+	 *
+	 * - XR2280X and XR21V141X have their on private register. On
+	 *   such models, 5-6 bits is not supported;
+	 * - The other models use a standard CDC register.
+	 *
+	 * As we need to do different things with regards to 5-6 bits,
+	 * the actual implementation is made on two different functions.
+	 */
+	if (xr_hal_table[port_priv->model][REG_FORMAT] == VIA_CDC_REGISTER)
+		xr_set_termios_cdc(tty, port, old_termios);
+	else
+		xr_set_termios_format_reg(tty, port, old_termios);
+}
+
 static int xr_open(struct tty_struct *tty, struct usb_serial_port *port)
 {
 	struct xr_port_private *port_priv = usb_get_serial_data(port->serial);
@@ -596,6 +788,13 @@ static int xr_open(struct tty_struct *tty, struct usb_serial_port *port)
 	gpio_dir = UART_MODE_DTR | UART_MODE_RTS;
 	xr_set_reg_uart(port, xr_hal_table[port_priv->model][REG_GPIO_DIR], gpio_dir);
 
+	ret = fifo_reset(port);
+	if (ret) {
+		dev_err(&port->dev, "Failed to reset FIFO\n");
+		return ret;
+	}
+
+
 	/* Setup termios */
 	if (tty)
 		xr_set_termios(tty, port, NULL);
@@ -618,25 +817,39 @@ 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_driver *driver = serial->type->usb_driver;
 	struct usb_interface *intf = serial->interface;
 	struct usb_endpoint_descriptor *data_ep;
+	struct usb_device *udev = serial->dev;
 	struct xr_port_private *port_priv;
-	int ifnum;
+	struct usb_interface *ctrl_intf;
+	int ifnum, ctrl_ifnum;
 
 	/* Attach only data interfaces */
 	ifnum = intf->cur_altsetting->desc.bInterfaceNumber;
 	if (!(ifnum % 2))
 		return -ENODEV;
 
+	/* Control interfaces are the even numbers */
+	ctrl_ifnum = ifnum - ifnum % 2;
+
 	port_priv = kzalloc(sizeof(*port_priv), GFP_KERNEL);
 	if (!port_priv)
 		return -ENOMEM;
 
 	data_ep = &intf->cur_altsetting->endpoint[0].desc;
+	ctrl_intf = usb_ifnum_to_if(udev, ctrl_ifnum);
 
+	port_priv->control_if = usb_get_intf(ctrl_intf);
 	port_priv->model = id->driver_info;
 	port_priv->channel = data_ep->bEndpointAddress;
 
+	/* Wake up control interface */
+	pm_suspend_ignore_children(&ctrl_intf->dev, false);
+	if (driver->supports_autosuspend)
+		pm_runtime_enable(&ctrl_intf->dev);
+	else
+	    pm_runtime_set_active(&ctrl_intf->dev);
 	usb_set_serial_data(serial, port_priv);
 
 	return 0;
@@ -645,6 +858,15 @@ static int xr_probe(struct usb_serial *serial, const struct usb_device_id *id)
 static void xr_disconnect(struct usb_serial *serial)
 {
 	struct xr_port_private *port_priv = usb_get_serial_data(serial);
+	struct usb_driver *driver = serial->type->usb_driver;
+	struct usb_interface *ctrl_intf = port_priv->control_if;
+
+	if (driver->supports_autosuspend)
+		pm_runtime_disable(&ctrl_intf->dev);
+
+	pm_runtime_set_suspended(&ctrl_intf->dev);
+
+	usb_put_intf(ctrl_intf);
 
 	usb_set_serial_data(serial, 0);
 	kfree(port_priv);
@@ -654,6 +876,11 @@ static const struct usb_device_id id_table[] = {
 	{ USB_DEVICE(0x04e2, 0x1410), .driver_info = XR21V141X},
 	{ USB_DEVICE(0x04e2, 0x1412), .driver_info = XR21V141X},
 	{ USB_DEVICE(0x04e2, 0x1414), .driver_info = XR21V141X},
+
+	{ USB_DEVICE(0x04e2, 0x1420), .driver_info = XR21B142X},
+	{ USB_DEVICE(0x04e2, 0x1422), .driver_info = XR21B142X},
+	{ USB_DEVICE(0x04e2, 0x1424), .driver_info = XR21B142X},
+
 	{ }
 };
 MODULE_DEVICE_TABLE(usb, id_table);
-- 
2.30.2


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

* [PATCH v2 5/7] USB: serial: xr: add support for XR21B1411
  2021-03-24  7:41 [PATCH v2 0/7] Add support for the other MaxLinear/Exar UARTs Mauro Carvalho Chehab
                   ` (3 preceding siblings ...)
  2021-03-24  7:41 ` [PATCH v2 4/7] USB: serial: xr: add support for XR21B142X devices Mauro Carvalho Chehab
@ 2021-03-24  7:41 ` Mauro Carvalho Chehab
  2021-03-30 15:07   ` Johan Hovold
  2021-03-24  7:41 ` [PATCH v2 6/7] USB: serial: xr: add support for XR2280X devices Mauro Carvalho Chehab
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 15+ messages in thread
From: Mauro Carvalho Chehab @ 2021-03-24  7:41 UTC (permalink / raw)
  To: Johan Hovold, Greg Kroah-Hartman
  Cc: Mauro Carvalho Chehab, linux-kernel, linux-usb

There's nothing special on this device: it has just one port.

The only difference is that it uses a different register set.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 drivers/usb/serial/xr_serial.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/drivers/usb/serial/xr_serial.c b/drivers/usb/serial/xr_serial.c
index 058624d15505..b1044dd3e994 100644
--- a/drivers/usb/serial/xr_serial.c
+++ b/drivers/usb/serial/xr_serial.c
@@ -99,6 +99,7 @@ struct xr_txrx_clk_mask {
 #define VIA_CDC_REGISTER		-1
 
 enum xr_model {
+	XR21B1411,
 	XR21V141X,
 	XR21B142X,
 	MAX_XR_MODELS
@@ -132,6 +133,30 @@ enum xr_hal_type {
 };
 
 static const int xr_hal_table[MAX_XR_MODELS][MAX_XR_HAL_TYPE] = {
+	[XR21B1411] = {
+		[REG_ENABLE] =				0xc00,
+		[REG_FORMAT] =				VIA_CDC_REGISTER,
+		[REG_FLOW_CTRL] =			0xc06,
+		[REG_XON_CHAR] =			0xc07,
+		[REG_XOFF_CHAR] =			0xc08,
+		[REG_TX_BREAK] =			0xc0a,
+		[REG_RS485_DELAY] =			0xc0b,
+		[REG_GPIO_MODE] =			0xc0c,
+		[REG_GPIO_DIR] =			0xc0d,
+		[REG_GPIO_SET] =			0xc0e,
+		[REG_GPIO_CLR] =			0xc0f,
+		[REG_GPIO_STATUS] =			0xc10,
+		[REG_GPIO_INT_MASK] =			0xc11,
+		[REG_CUSTOMIZED_INT] =			0xc12,
+		[REG_GPIO_PULL_UP_ENABLE] =		0xc14,
+		[REG_GPIO_PULL_DOWN_ENABLE] =		0xc15,
+		[REG_LOOPBACK] =			0xc16,
+		[REG_LOW_LATENCY] =			0xcc2,
+		[REG_CUSTOM_DRIVER] =			0x20d,
+
+		[REQ_SET] =				0,
+		[REQ_GET] =				1,
+	},
 	[XR21V141X] = {
 		[REG_ENABLE] =				0x03,
 		[REG_FORMAT] =				0x0b,
@@ -190,6 +215,8 @@ static int xr_set_reg(struct usb_serial_port *port, u8 block, u8 reg, u8 val)
 	int ret;
 
 	switch (port_priv->model) {
+	case XR21B1411:
+		break;
 	case XR21V141X:
 		if (port_priv->channel)
 			reg |= (port_priv->channel - 1) << 8;
@@ -226,6 +253,8 @@ static int xr_get_reg(struct usb_serial_port *port, u8 block, u8 reg, u8 *val)
 		return -ENOMEM;
 
 	switch (port_priv->model) {
+	case XR21B1411:
+		break;
 	case XR21V141X:
 		if (port_priv->channel)
 			reg |= (port_priv->channel - 1) << 8;
@@ -874,6 +903,7 @@ static void xr_disconnect(struct usb_serial *serial)
 
 static const struct usb_device_id id_table[] = {
 	{ USB_DEVICE(0x04e2, 0x1410), .driver_info = XR21V141X},
+	{ USB_DEVICE(0x04e2, 0x1411), .driver_info = XR21B1411},
 	{ USB_DEVICE(0x04e2, 0x1412), .driver_info = XR21V141X},
 	{ USB_DEVICE(0x04e2, 0x1414), .driver_info = XR21V141X},
 
-- 
2.30.2


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

* [PATCH v2 6/7] USB: serial: xr: add support for XR2280X devices
  2021-03-24  7:41 [PATCH v2 0/7] Add support for the other MaxLinear/Exar UARTs Mauro Carvalho Chehab
                   ` (4 preceding siblings ...)
  2021-03-24  7:41 ` [PATCH v2 5/7] USB: serial: xr: add support for XR21B1411 Mauro Carvalho Chehab
@ 2021-03-24  7:41 ` Mauro Carvalho Chehab
  2021-03-30 15:08   ` Johan Hovold
  2021-03-24  7:41 ` [PATCH v2 7/7] USB: cdc-acm: add other non-standard xr_serial models to ignore list Mauro Carvalho Chehab
  2021-03-30 14:35 ` [PATCH v2 0/7] Add support for the other MaxLinear/Exar UARTs Johan Hovold
  7 siblings, 1 reply; 15+ messages in thread
From: Mauro Carvalho Chehab @ 2021-03-24  7:41 UTC (permalink / raw)
  To: Johan Hovold, Greg Kroah-Hartman
  Cc: Mauro Carvalho Chehab, linux-kernel, linux-usb

There's nothing special on those devices either. They just
use a different set of registers.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 drivers/usb/serial/xr_serial.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/drivers/usb/serial/xr_serial.c b/drivers/usb/serial/xr_serial.c
index b1044dd3e994..72365ffbc1b8 100644
--- a/drivers/usb/serial/xr_serial.c
+++ b/drivers/usb/serial/xr_serial.c
@@ -99,6 +99,7 @@ struct xr_txrx_clk_mask {
 #define VIA_CDC_REGISTER		-1
 
 enum xr_model {
+	XR2280X,
 	XR21B1411,
 	XR21V141X,
 	XR21B142X,
@@ -133,6 +134,30 @@ enum xr_hal_type {
 };
 
 static const int xr_hal_table[MAX_XR_MODELS][MAX_XR_HAL_TYPE] = {
+	[XR2280X] = {
+		[REG_ENABLE] =				0x40,
+		[REG_FORMAT] =				0x45,
+		[REG_FLOW_CTRL] =			0x46,
+		[REG_XON_CHAR] =			0x47,
+		[REG_XOFF_CHAR] =			0x48,
+		[REG_TX_BREAK] =			0x4a,
+		[REG_RS485_DELAY] =			0x4b,
+		[REG_GPIO_MODE] =			0x4c,
+		[REG_GPIO_DIR] =			0x4d,
+		[REG_GPIO_SET] =			0x4e,
+		[REG_GPIO_CLR] =			0x4f,
+		[REG_GPIO_STATUS] =			0x50,
+		[REG_GPIO_INT_MASK] =			0x51,
+		[REG_CUSTOMIZED_INT] =			0x52,
+		[REG_GPIO_PULL_UP_ENABLE] =		0x54,
+		[REG_GPIO_PULL_DOWN_ENABLE] =		0x55,
+		[REG_LOOPBACK] =			0x56,
+		[REG_LOW_LATENCY] =			0x66,
+		[REG_CUSTOM_DRIVER] =			0x81,
+
+		[REQ_SET] =				5,
+		[REQ_GET] =				5,
+	},
 	[XR21B1411] = {
 		[REG_ENABLE] =				0xc00,
 		[REG_FORMAT] =				VIA_CDC_REGISTER,
@@ -215,6 +240,7 @@ static int xr_set_reg(struct usb_serial_port *port, u8 block, u8 reg, u8 val)
 	int ret;
 
 	switch (port_priv->model) {
+	case XR2280X:
 	case XR21B1411:
 		break;
 	case XR21V141X:
@@ -253,6 +279,7 @@ static int xr_get_reg(struct usb_serial_port *port, u8 block, u8 reg, u8 *val)
 		return -ENOMEM;
 
 	switch (port_priv->model) {
+	case XR2280X:
 	case XR21B1411:
 		break;
 	case XR21V141X:
@@ -902,6 +929,11 @@ static void xr_disconnect(struct usb_serial *serial)
 }
 
 static const struct usb_device_id id_table[] = {
+	{ USB_DEVICE(0x04e2, 0x1400), .driver_info = XR2280X},
+	{ USB_DEVICE(0x04e2, 0x1401), .driver_info = XR2280X},
+	{ USB_DEVICE(0x04e2, 0x1402), .driver_info = XR2280X},
+	{ USB_DEVICE(0x04e2, 0x1403), .driver_info = XR2280X},
+
 	{ USB_DEVICE(0x04e2, 0x1410), .driver_info = XR21V141X},
 	{ USB_DEVICE(0x04e2, 0x1411), .driver_info = XR21B1411},
 	{ USB_DEVICE(0x04e2, 0x1412), .driver_info = XR21V141X},
-- 
2.30.2


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

* [PATCH v2 7/7] USB: cdc-acm: add other non-standard xr_serial models to ignore list
  2021-03-24  7:41 [PATCH v2 0/7] Add support for the other MaxLinear/Exar UARTs Mauro Carvalho Chehab
                   ` (5 preceding siblings ...)
  2021-03-24  7:41 ` [PATCH v2 6/7] USB: serial: xr: add support for XR2280X devices Mauro Carvalho Chehab
@ 2021-03-24  7:41 ` Mauro Carvalho Chehab
  2021-03-30 15:14   ` Johan Hovold
  2021-03-30 14:35 ` [PATCH v2 0/7] Add support for the other MaxLinear/Exar UARTs Johan Hovold
  7 siblings, 1 reply; 15+ messages in thread
From: Mauro Carvalho Chehab @ 2021-03-24  7:41 UTC (permalink / raw)
  To: Johan Hovold, Greg Kroah-Hartman
  Cc: Mauro Carvalho Chehab, Oliver Neukum, linux-kernel, linux-usb

Now that the xr_serial got support for other models, add their
USB IDs as well, as those devices won't work with the standard
CDC driver.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 drivers/usb/class/cdc-acm.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index 39ddb5585ded..839b80093478 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -1902,9 +1902,20 @@ static const struct usb_device_id acm_ids[] = {
 #endif
 
 #if IS_ENABLED(CONFIG_USB_SERIAL_XR)
-	{ USB_DEVICE(0x04e2, 0x1410),   /* Ignore XR21V141X USB to Serial converter */
-	.driver_info = IGNORE_DEVICE,
-	},
+	/* Ignore MaxLinear/Exar USB UARTs and USB UART bridges */
+	{ USB_DEVICE(0x04e2, 0x1400), .driver_info = IGNORE_DEVICE,},
+	{ USB_DEVICE(0x04e2, 0x1401), .driver_info = IGNORE_DEVICE,},
+	{ USB_DEVICE(0x04e2, 0x1402), .driver_info = IGNORE_DEVICE,},
+	{ USB_DEVICE(0x04e2, 0x1403), .driver_info = IGNORE_DEVICE,},
+
+	{ USB_DEVICE(0x04e2, 0x1410), .driver_info = IGNORE_DEVICE,},
+	{ USB_DEVICE(0x04e2, 0x1411), .driver_info = IGNORE_DEVICE,},
+	{ USB_DEVICE(0x04e2, 0x1412), .driver_info = IGNORE_DEVICE,},
+	{ USB_DEVICE(0x04e2, 0x1414), .driver_info = IGNORE_DEVICE,},
+
+	{ USB_DEVICE(0x04e2, 0x1420), .driver_info = IGNORE_DEVICE,},
+	{ USB_DEVICE(0x04e2, 0x1422), .driver_info = IGNORE_DEVICE,},
+	{ USB_DEVICE(0x04e2, 0x1424), .driver_info = IGNORE_DEVICE,},
 #endif
 
 	/*Samsung phone in firmware update mode */
-- 
2.30.2


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

* Re: [PATCH v2 0/7] Add support for the other MaxLinear/Exar UARTs
  2021-03-24  7:41 [PATCH v2 0/7] Add support for the other MaxLinear/Exar UARTs Mauro Carvalho Chehab
                   ` (6 preceding siblings ...)
  2021-03-24  7:41 ` [PATCH v2 7/7] USB: cdc-acm: add other non-standard xr_serial models to ignore list Mauro Carvalho Chehab
@ 2021-03-30 14:35 ` Johan Hovold
  7 siblings, 0 replies; 15+ messages in thread
From: Johan Hovold @ 2021-03-30 14:35 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Greg Kroah-Hartman, Oliver Neukum, linux-kernel, linux-usb

On Wed, Mar 24, 2021 at 08:41:04AM +0100, Mauro Carvalho Chehab wrote:
> The current version of the xr_serial driver handles one one of the several
> MaxLinear/Exar UARTs and UART bridges. There are currently 12 such
> models. Only one is currently supported.

As I mentioned earlier, proper handling of the CDC devices requires
support in USB serial core, which I've now implemented.

With that and by parsing the Union descriptor to determine the interface
layout, probing can also be cleaned up quite a bit.

Looking at this series I found a few things that have been overlooked,
such as the device types having different register widths (and one even
differing register address widths), the custom driver flag not being set
and a memory leak. I'll comment on some of these separately.

It also seems the type abstraction can be handled better by using a
more structured approach, which also allows getting rid of some of the
type conditionals spread throughout the code.

Another key observation here is that it's the currently supported type
which is the one that stands out from the rest. And while all four types
supports CDC requests, it's only the SET_LINE_CODING one which is
actually required to be used (by the three new types). This also allows
for a cleaner implementation.

I ended up implementing support myself in order to make sense of all the
ways these device types differ while digging through the datasheets and
thinking about how best to implement this.

I'll be posting a fix and two series while keeping you on CC. Would you
mind giving it a spin with your XR21B142X?

Johan

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

* Re: [PATCH v2 2/7] USB: serial: xr: use a table for device-specific settings
  2021-03-24  7:41 ` [PATCH v2 2/7] USB: serial: xr: use a table for device-specific settings Mauro Carvalho Chehab
@ 2021-03-30 14:44   ` Johan Hovold
  0 siblings, 0 replies; 15+ messages in thread
From: Johan Hovold @ 2021-03-30 14:44 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: Greg Kroah-Hartman, linux-kernel, linux-usb

On Wed, Mar 24, 2021 at 08:41:06AM +0100, Mauro Carvalho Chehab wrote:
> The same driver is used by a wide range of MaxLinear devices.
> 
> Other models are close enough to use the same driver, but they
> use a different register set.
> 
> So, instead of having the registers hardcoded at the driver,
> use a table. This will allow further patches to add support for
> other devices.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> ---
>  drivers/usb/serial/xr_serial.c | 151 ++++++++++++++++++++++++---------
>  1 file changed, 113 insertions(+), 38 deletions(-)
 
>  static int xr_probe(struct usb_serial *serial, const struct usb_device_id *id)
>  {
> +	struct xr_port_private *port_priv;
> +
>  	/* Don't bind to control interface */
>  	if (serial->interface->cur_altsetting->desc.bInterfaceNumber == 0)
>  		return -ENODEV;
>  
> +	port_priv = kzalloc(sizeof(*port_priv), GFP_KERNEL);
> +	if (!port_priv)
> +		return -ENOMEM;

For historical reasons, you cannot allocate memory in probe() directly
(unless using devres) or this can leak on later probe errors.

Instead interface-wide allocations are done in attach() and released in
release(), while port-specific allocations are done in port_probe() and
released in port_remove().

Johan

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

* Re: [PATCH v2 3/7] USB: serial: xr: add support for multi-port XR21V141X variants
  2021-03-24  7:41 ` [PATCH v2 3/7] USB: serial: xr: add support for multi-port XR21V141X variants Mauro Carvalho Chehab
@ 2021-03-30 14:50   ` Johan Hovold
  0 siblings, 0 replies; 15+ messages in thread
From: Johan Hovold @ 2021-03-30 14:50 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: Greg Kroah-Hartman, linux-kernel, linux-usb

On Wed, Mar 24, 2021 at 08:41:07AM +0100, Mauro Carvalho Chehab wrote:
> XR21V1412 and XR21V1414 have exactly the same interface, but
> they support multiple 2 and 4 ports, respectively.
> 
> On such devices, the "CDC Union" field shows how they're
> grouped, as can be seen on those lsusb -v outputs:
> 
> 	https://linux-usb.vger.kernel.narkive.com/YaTYwHkM/usb-uart-device-from-exar-co-not-working-with-cdc-acm-but-usbserial
> 	https://jquery.developreference.com/article/22043997/udev+rule+with+bInterfaceNumber+doesn't+work+%5bclosed%5d
> 
> So, for instance, on XR21V1414, (0x04e2:0x1414), the 3rd
> port is:
> 
> 	CDC Union:
> 		bMasterInterface 4
> 		bSlaveInterface 5
> 
> 	CDC Call Management:
> 		bmCapabilities 0x01
> 			call management
> 				bDataInterface 5
> 
> In other words, the control interface is an even number,
> and the data interface is the next odd number.
> 
> The logic to get the proper register for an specific channel
> came from this patch:
> 
> 	https://lore.kernel.org/linux-usb/20180404070634.nhspvmxcjwfgjkcv@advantechmxl-desktop
> 
> Add support for them.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> ---
>  drivers/usb/serial/xr_serial.c | 30 ++++++++++++++++++++++++++++--
>  1 file changed, 28 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/serial/xr_serial.c b/drivers/usb/serial/xr_serial.c
> index 518c4725431a..f161394d9b2d 100644
> --- a/drivers/usb/serial/xr_serial.c
> +++ b/drivers/usb/serial/xr_serial.c
> @@ -145,6 +145,7 @@ static const int xr_hal_table[MAX_XR_MODELS][MAX_XR_HAL_TYPE] = {
>  
>  struct xr_port_private {
>  	enum xr_model model;
> +	unsigned int channel;
>  };
>  
>  static int xr_set_reg(struct usb_serial_port *port, u8 block, u8 reg, u8 val)
> @@ -153,6 +154,14 @@ static int xr_set_reg(struct usb_serial_port *port, u8 block, u8 reg, u8 val)
>  	struct usb_serial *serial = port->serial;
>  	int ret;
>  
> +	switch (port_priv->model) {
> +	case XR21V141X:
> +		if (port_priv->channel)
> +			reg |= (port_priv->channel - 1) << 8;

reg is u8 so you're simply discarding the channel index here and
effectively only the first port will work as intended after this patch.

> +		break;
> +	default:
> +		return -EINVAL;
> +	};
>  	ret = usb_control_msg(serial->dev,
>  			      usb_sndctrlpipe(serial->dev, 0),
>  			      xr_hal_table[port_priv->model][REQ_SET],
 
>  static int xr_probe(struct usb_serial *serial, const struct usb_device_id *id)
>  {
> +	struct usb_interface *intf = serial->interface;
> +	struct usb_endpoint_descriptor *data_ep;
>  	struct xr_port_private *port_priv;
> +	int ifnum;
>  
> -	/* Don't bind to control interface */
> -	if (serial->interface->cur_altsetting->desc.bInterfaceNumber == 0)
> +	/* Attach only data interfaces */
> +	ifnum = intf->cur_altsetting->desc.bInterfaceNumber;
> +	if (!(ifnum % 2))
>  		return -ENODEV;
>  
>  	port_priv = kzalloc(sizeof(*port_priv), GFP_KERNEL);
>  	if (!port_priv)
>  		return -ENOMEM;
>  
> +	data_ep = &intf->cur_altsetting->endpoint[0].desc;
> +
>  	port_priv->model = id->driver_info;
> +	port_priv->channel = data_ep->bEndpointAddress;

This creative but it seems cleaner to simply use the interface number
of the control interface divided by two. That gives you a zero-based
index which doesn't require the "channel--" added at various places by
the rest of the series.

Johan

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

* Re: [PATCH v2 4/7] USB: serial: xr: add support for XR21B142X devices
  2021-03-24  7:41 ` [PATCH v2 4/7] USB: serial: xr: add support for XR21B142X devices Mauro Carvalho Chehab
@ 2021-03-30 15:04   ` Johan Hovold
  0 siblings, 0 replies; 15+ messages in thread
From: Johan Hovold @ 2021-03-30 15:04 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: Greg Kroah-Hartman, linux-kernel, linux-usb

On Wed, Mar 24, 2021 at 08:41:08AM +0100, Mauro Carvalho Chehab wrote:
> Those devices can have 1, 2 or 4 ports. They're similar to
> XR21B141X models, except for:
> 
> - the register numbers and reqs are different;
> - some settings are done via the control interface, by
>   using CDC commands.
> - a few other minor differences.
> 
> The same way as XR21B141X, the control and data interfaces
> are grouped via CDC union, where the even interface is for
> control, and the subsequent one for data. So, on XR21B142X,
> 
>       CDC Union:
>         bMasterInterface        0
>         bSlaveInterface         1
> 
>         bDataInterface          1
> 
>       CDC Union:
>         bMasterInterface        0
>         bSlaveInterface         1
> 
>         bDataInterface          1
> 
>       CDC Union:
>         bMasterInterface        2
>         bSlaveInterface         3
> 
>         bDataInterface          3
> 
>       CDC Union:
>         bMasterInterface        4
>         bSlaveInterface         5
> 
>         bDataInterface          5
> 
>       CDC Union:
>         bMasterInterface        6
>         bSlaveInterface         7
> 
>         bDataInterface          7
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> ---
>  drivers/usb/serial/xr_serial.c | 235 ++++++++++++++++++++++++++++++++-
>  1 file changed, 231 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/usb/serial/xr_serial.c b/drivers/usb/serial/xr_serial.c
> index f161394d9b2d..058624d15505 100644
> --- a/drivers/usb/serial/xr_serial.c
> +++ b/drivers/usb/serial/xr_serial.c
> @@ -16,6 +16,7 @@
>  #include <linux/slab.h>
>  #include <linux/tty.h>
>  #include <linux/usb.h>
> +#include <linux/usb/cdc.h>
>  #include <linux/usb/serial.h>
>  
>  struct xr_txrx_clk_mask {
> @@ -89,8 +90,17 @@ struct xr_txrx_clk_mask {
>  #define UART_MODE_RS485			0x3
>  #define UART_MODE_RS485_ADDR		0x4
>  
> +/* Used on devices that need the CDC control interface */
> +#define URM_RESET_RX_FIFO_BASE		0x18
> +#define URM_RESET_TX_FIFO_BASE		0x1c

The driver already has defines for these.

> +#define CDC_DATA_INTERFACE_TYPE		0x0a
> +
> +#define VIA_CDC_REGISTER		-1
> +
>  enum xr_model {
>  	XR21V141X,
> +	XR21B142X,
>  	MAX_XR_MODELS
>  };
>  
> @@ -140,12 +150,37 @@ static const int xr_hal_table[MAX_XR_MODELS][MAX_XR_HAL_TYPE] = {
>  
>  		[REQ_SET] =				0,
>  		[REQ_GET] =				1,
> +	},
> +	[XR21B142X] = {
> +		[REG_ENABLE] =				0x00,
> +		[REG_FORMAT] =				VIA_CDC_REGISTER,

Here you're mixing register addresses in with boolean flags in an
int-array. A structure seems more appropriate.

> +		[REG_FLOW_CTRL] =			0x06,
> +		[REG_XON_CHAR] =			0x07,
> +		[REG_XOFF_CHAR] =			0x08,
> +		[REG_TX_BREAK] =			0x0a,
> +		[REG_RS485_DELAY] =			0x0b,
> +		[REG_GPIO_MODE] =			0x0c,
> +		[REG_GPIO_DIR] =			0x0d,
> +		[REG_GPIO_SET] =			0x0e,
> +		[REG_GPIO_CLR] =			0x0f,
> +		[REG_GPIO_STATUS] =			0x10,
> +		[REG_GPIO_INT_MASK] =			0x11,
> +		[REG_CUSTOMIZED_INT] =			0x12,
> +		[REG_GPIO_PULL_UP_ENABLE] =		0x14,
> +		[REG_GPIO_PULL_DOWN_ENABLE] =		0x15,
> +		[REG_LOOPBACK] =			0x16,
> +		[REG_LOW_LATENCY] =			0x46,
> +		[REG_CUSTOM_DRIVER] =			0x60,
> +
> +		[REQ_SET] =				0,
> +		[REQ_GET] =				0,
>  	}
>  };
>  
>  struct xr_port_private {
>  	enum xr_model model;
>  	unsigned int channel;
> +	struct usb_interface *control_if;
>  };
>  
>  static int xr_set_reg(struct usb_serial_port *port, u8 block, u8 reg, u8 val)
> @@ -159,6 +194,9 @@ static int xr_set_reg(struct usb_serial_port *port, u8 block, u8 reg, u8 val)
>  		if (port_priv->channel)
>  			reg |= (port_priv->channel - 1) << 8;
>  		break;
> +	case XR21B142X:
> +		reg |= (port_priv->channel - 4) << 1;

This just look really weird. This type uses the control interface number
as channel index.

But more importantly, you don't increase the register width which is
16-bit for XR21B142X and the request recipient is the interface, not the
device for this type.

>  static int xr_set_reg_uart(struct usb_serial_port *port, u8 reg, u8 val)
>  {
>  	return xr_set_reg(port, UART_REG_BLOCK, reg, val);
> @@ -243,6 +321,11 @@ static int xr_uart_enable(struct usb_serial_port *port)
>  	struct xr_port_private *port_priv = usb_get_serial_data(port->serial);
>  	int ret;
>  
> +	if (port_priv->model != XR21V141X)
> +		return xr_set_reg_uart(port,
> +				       xr_hal_table[port_priv->model][REG_ENABLE],
> +				       UART_ENABLE_TX | UART_ENABLE_RX);
> +

I handle this by overriding the default implementation for XR21V141X
instead.

>  	ret = xr_set_reg_um(port, UM_FIFO_ENABLE_REG,
>  			    UM_ENABLE_TX_FIFO);
>  	if (ret)
> @@ -272,11 +355,37 @@ static int xr_uart_disable(struct usb_serial_port *port)
>  	if (ret)
>  		return ret;
>  
> +	if (port_priv->model != XR21V141X)
> +		return 0;
> +

That allows not spreading conditionals like this throughout the driver.

>  	ret = xr_set_reg_um(port, UM_FIFO_ENABLE_REG, 0);
>  
>  	return ret;
>  }
>  
> +static int fifo_reset(struct usb_serial_port *port)
> +{
> +	struct xr_port_private *port_priv = usb_get_serial_data(port->serial);
> +	int channel = port_priv->channel;
> +	int ret = 0;
> +
> +	if (port_priv->model != XR21V141X)
> +		return 0;
> +
> +	if (channel)
> +		channel--;

This isn't very nice; a zero-based index is more natural.

> +
> +	ret = xr_set_reg_um(port,
> +			    URM_RESET_RX_FIFO_BASE + channel, 0xff);
> +	if (ret)
> +		return ret;
> +
> +	ret = xr_set_reg_um(port,
> +			    URM_RESET_TX_FIFO_BASE + channel, 0xff);
> +
> +	return ret;
> +}

This looks malplaced, since it implements FIFO reset for the currently
supported types, but not for the one you're adding support for here.

> +
>  static int xr_tiocmget(struct tty_struct *tty)
>  {
>  	struct usb_serial_port *port = tty->driver_data;
> @@ -358,6 +467,12 @@ static void xr_break_ctl(struct tty_struct *tty, int break_state)
>  	struct xr_port_private *port_priv = usb_get_serial_data(port->serial);
>  	u8 state;
>  
> +	if (port_priv->model != XR21V141X) {
> +		xr_usb_serial_ctrl_msg(port, USB_CDC_REQ_SEND_BREAK, state,
> +				       NULL, 0);
> +		return;
> +	}

This isn't needed at all since all types have TX_BREAK register.

> +
>  	if (break_state == 0)
>  		state = UART_BREAK_OFF;
>  	else
> @@ -505,6 +620,13 @@ static void xr_set_flow_mode(struct tty_struct *tty,
>  		flow = UART_FLOW_MODE_NONE;
>  	}
>  
> +	/*
> +	 * Add support for the TXT and RXT function for 0x1420, 0x1422, 0x1424,
> +	 * by setting GPIO_MODE [9:8] = '11'
> +	 */
> +	if (port_priv->model == XR21B142X)
> +		gpio_mode |= 0x300;

Not needed either, since we're reading back the (default) gpio mode just
above.

But again, you're not increasing the type width here, which is still u8
so this has no effect currently.

> +
>  	/*
>  	 * As per the datasheet, UART needs to be disabled while writing to
>  	 * FLOW_CONTROL register.
> @@ -521,9 +643,55 @@ static void xr_set_flow_mode(struct tty_struct *tty,
>  		xr_dtr_rts(port, 1);
>  }
>  
> -static void xr_set_termios(struct tty_struct *tty,
> -			   struct usb_serial_port *port,
> -			   struct ktermios *old_termios)
> +static void xr_set_termios_cdc(struct tty_struct *tty,
> +			       struct usb_serial_port *port,
> +			       struct ktermios *old_termios)
> +{
> +	struct ktermios *termios = &tty->termios;
> +	struct usb_cdc_line_coding line = { 0 };
> +	int clear, set;
> +
> +	line.dwDTERate = cpu_to_le32(tty_get_baud_rate(tty));
> +	line.bCharFormat = termios->c_cflag & CSTOPB ? 1 : 0;
> +	line.bParityType = termios->c_cflag & PARENB ?
> +			   (termios->c_cflag & PARODD ? 1 : 2) +
> +			   (termios->c_cflag & CMSPAR ? 2 : 0) : 0;

I know this comes from cdc-acm.c, but those ?: are still horrid. :)

> +
> +	switch (C_CSIZE(tty)) {
> +	case CS5:
> +		line.bDataBits = 5;
> +		break;
> +	case CS6:
> +		line.bDataBits = 6;
> +		break;

The types you add later does not support CS5 and CS6 but IIRC you don't
handle that.

> +	case CS7:
> +		line.bDataBits = 7;
> +		break;
> +	case CS8:
> +	default:
> +		line.bDataBits = 8;
> +		break;
> +	}
> +
> +	if (!line.dwDTERate) {
> +		line.dwDTERate = tty->termios.c_ospeed;

This is broken, and still sets the line rate to 0.

> +		clear = UART_MODE_DTR;
> +	} else {
> +		set = UART_MODE_DTR;
> +	}
> +
> +	if (clear || set)
> +		xr_tiocmset_port(port, set, clear);

This is already handled by the driver in xr_set_flow_mode() below.

> +	xr_set_flow_mode(tty, port, old_termios);
> +
> +	xr_usb_serial_ctrl_msg(port, USB_CDC_REQ_SET_LINE_CODING, 0,
> +			       &line, sizeof(line));
> +}
> +
> +static void xr_set_termios_format_reg(struct tty_struct *tty,
> +				      struct usb_serial_port *port,
> +				      struct ktermios *old_termios)
>  {
>  	struct xr_port_private *port_priv = usb_get_serial_data(port->serial);
>  	struct ktermios *termios = &tty->termios;
> @@ -532,6 +700,8 @@ static void xr_set_termios(struct tty_struct *tty,
>  	if (!old_termios || (tty->termios.c_ospeed != old_termios->c_ospeed))
>  		xr_set_baudrate(tty, port);
>  
> +	/* For models with a private CHARACTER_FORMAT register */
> +
>  	switch (C_CSIZE(tty)) {
>  	case CS5:
>  	case CS6:
> @@ -577,6 +747,28 @@ static void xr_set_termios(struct tty_struct *tty,
>  	xr_set_flow_mode(tty, port, old_termios);
>  }
>  
> +static void xr_set_termios(struct tty_struct *tty,
> +			   struct usb_serial_port *port,
> +			   struct ktermios *old_termios)
> +{
> +	struct xr_port_private *port_priv = usb_get_serial_data(port->serial);
> +
> +	/*
> +	 * Different models have different ways to setup character format:
> +	 *
> +	 * - XR2280X and XR21V141X have their on private register. On
> +	 *   such models, 5-6 bits is not supported;
> +	 * - The other models use a standard CDC register.
> +	 *
> +	 * As we need to do different things with regards to 5-6 bits,
> +	 * the actual implementation is made on two different functions.
> +	 */
> +	if (xr_hal_table[port_priv->model][REG_FORMAT] == VIA_CDC_REGISTER)
> +		xr_set_termios_cdc(tty, port, old_termios);
> +	else
> +		xr_set_termios_format_reg(tty, port, old_termios);
> +}
> +
>  static int xr_open(struct tty_struct *tty, struct usb_serial_port *port)
>  {
>  	struct xr_port_private *port_priv = usb_get_serial_data(port->serial);
> @@ -596,6 +788,13 @@ static int xr_open(struct tty_struct *tty, struct usb_serial_port *port)
>  	gpio_dir = UART_MODE_DTR | UART_MODE_RTS;
>  	xr_set_reg_uart(port, xr_hal_table[port_priv->model][REG_GPIO_DIR], gpio_dir);
>  
> +	ret = fifo_reset(port);
> +	if (ret) {
> +		dev_err(&port->dev, "Failed to reset FIFO\n");
> +		return ret;
> +	}
> +
> +
>  	/* Setup termios */
>  	if (tty)
>  		xr_set_termios(tty, port, NULL);
> @@ -618,25 +817,39 @@ 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_driver *driver = serial->type->usb_driver;
>  	struct usb_interface *intf = serial->interface;
>  	struct usb_endpoint_descriptor *data_ep;
> +	struct usb_device *udev = serial->dev;
>  	struct xr_port_private *port_priv;
> -	int ifnum;
> +	struct usb_interface *ctrl_intf;
> +	int ifnum, ctrl_ifnum;
>  
>  	/* Attach only data interfaces */
>  	ifnum = intf->cur_altsetting->desc.bInterfaceNumber;
>  	if (!(ifnum % 2))
>  		return -ENODEV;
>  
> +	/* Control interfaces are the even numbers */
> +	ctrl_ifnum = ifnum - ifnum % 2;
> +
>  	port_priv = kzalloc(sizeof(*port_priv), GFP_KERNEL);
>  	if (!port_priv)
>  		return -ENOMEM;
>  
>  	data_ep = &intf->cur_altsetting->endpoint[0].desc;'

Whoops, NULL-deref in case of a malicous device without endpoints.

> +	ctrl_intf = usb_ifnum_to_if(udev, ctrl_ifnum);
>  
> +	port_priv->control_if = usb_get_intf(ctrl_intf);
>  	port_priv->model = id->driver_info;
>  	port_priv->channel = data_ep->bEndpointAddress;
>  
> +	/* Wake up control interface */
> +	pm_suspend_ignore_children(&ctrl_intf->dev, false);
> +	if (driver->supports_autosuspend)
> +		pm_runtime_enable(&ctrl_intf->dev);
> +	else
> +	    pm_runtime_set_active(&ctrl_intf->dev);

Now this is really nasty, but all goes away with the new multi-interface
support.

>  	usb_set_serial_data(serial, port_priv);
>  
>  	return 0;
> @@ -645,6 +858,15 @@ static int xr_probe(struct usb_serial *serial, const struct usb_device_id *id)
>  static void xr_disconnect(struct usb_serial *serial)
>  {
>  	struct xr_port_private *port_priv = usb_get_serial_data(serial);
> +	struct usb_driver *driver = serial->type->usb_driver;
> +	struct usb_interface *ctrl_intf = port_priv->control_if;
> +
> +	if (driver->supports_autosuspend)
> +		pm_runtime_disable(&ctrl_intf->dev);
> +
> +	pm_runtime_set_suspended(&ctrl_intf->dev);
> +
> +	usb_put_intf(ctrl_intf);
>  
>  	usb_set_serial_data(serial, 0);
>  	kfree(port_priv);
> @@ -654,6 +876,11 @@ static const struct usb_device_id id_table[] = {
>  	{ USB_DEVICE(0x04e2, 0x1410), .driver_info = XR21V141X},
>  	{ USB_DEVICE(0x04e2, 0x1412), .driver_info = XR21V141X},
>  	{ USB_DEVICE(0x04e2, 0x1414), .driver_info = XR21V141X},
> +
> +	{ USB_DEVICE(0x04e2, 0x1420), .driver_info = XR21B142X},
> +	{ USB_DEVICE(0x04e2, 0x1422), .driver_info = XR21B142X},
> +	{ USB_DEVICE(0x04e2, 0x1424), .driver_info = XR21B142X},
> +
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(usb, id_table);

Johan

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

* Re: [PATCH v2 5/7] USB: serial: xr: add support for XR21B1411
  2021-03-24  7:41 ` [PATCH v2 5/7] USB: serial: xr: add support for XR21B1411 Mauro Carvalho Chehab
@ 2021-03-30 15:07   ` Johan Hovold
  0 siblings, 0 replies; 15+ messages in thread
From: Johan Hovold @ 2021-03-30 15:07 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: Greg Kroah-Hartman, linux-kernel, linux-usb

On Wed, Mar 24, 2021 at 08:41:09AM +0100, Mauro Carvalho Chehab wrote:
> There's nothing special on this device: it has just one port.
> 
> The only difference is that it uses a different register set.

Not quite true. First, it uses 12-bit registers, which the driver isn't
updated for. Similarly, it used 12-bit register *addresses* which
likewise the driver isn't updated for and the upper bits are simply
discarded.

This device also doesn't support CS5 and CS6 which would need to be
handled.

> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> ---
>  drivers/usb/serial/xr_serial.c | 30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
> 
> diff --git a/drivers/usb/serial/xr_serial.c b/drivers/usb/serial/xr_serial.c
> index 058624d15505..b1044dd3e994 100644
> --- a/drivers/usb/serial/xr_serial.c
> +++ b/drivers/usb/serial/xr_serial.c
> @@ -99,6 +99,7 @@ struct xr_txrx_clk_mask {
>  #define VIA_CDC_REGISTER		-1
>  
>  enum xr_model {
> +	XR21B1411,
>  	XR21V141X,
>  	XR21B142X,
>  	MAX_XR_MODELS
> @@ -132,6 +133,30 @@ enum xr_hal_type {
>  };
>  
>  static const int xr_hal_table[MAX_XR_MODELS][MAX_XR_HAL_TYPE] = {
> +	[XR21B1411] = {
> +		[REG_ENABLE] =				0xc00,
> +		[REG_FORMAT] =				VIA_CDC_REGISTER,
> +		[REG_FLOW_CTRL] =			0xc06,
> +		[REG_XON_CHAR] =			0xc07,
> +		[REG_XOFF_CHAR] =			0xc08,
> +		[REG_TX_BREAK] =			0xc0a,
> +		[REG_RS485_DELAY] =			0xc0b,
> +		[REG_GPIO_MODE] =			0xc0c,
> +		[REG_GPIO_DIR] =			0xc0d,
> +		[REG_GPIO_SET] =			0xc0e,
> +		[REG_GPIO_CLR] =			0xc0f,
> +		[REG_GPIO_STATUS] =			0xc10,
> +		[REG_GPIO_INT_MASK] =			0xc11,
> +		[REG_CUSTOMIZED_INT] =			0xc12,
> +		[REG_GPIO_PULL_UP_ENABLE] =		0xc14,
> +		[REG_GPIO_PULL_DOWN_ENABLE] =		0xc15,
> +		[REG_LOOPBACK] =			0xc16,
> +		[REG_LOW_LATENCY] =			0xcc2,
> +		[REG_CUSTOM_DRIVER] =			0x20d,
> +
> +		[REQ_SET] =				0,
> +		[REQ_GET] =				1,
> +	},
>  	[XR21V141X] = {
>  		[REG_ENABLE] =				0x03,
>  		[REG_FORMAT] =				0x0b,

Johan

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

* Re: [PATCH v2 6/7] USB: serial: xr: add support for XR2280X devices
  2021-03-24  7:41 ` [PATCH v2 6/7] USB: serial: xr: add support for XR2280X devices Mauro Carvalho Chehab
@ 2021-03-30 15:08   ` Johan Hovold
  0 siblings, 0 replies; 15+ messages in thread
From: Johan Hovold @ 2021-03-30 15:08 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: Greg Kroah-Hartman, linux-kernel, linux-usb

On Wed, Mar 24, 2021 at 08:41:10AM +0100, Mauro Carvalho Chehab wrote:
> There's nothing special on those devices either. They just
> use a different set of registers.

This device also use 16-bit addresses and doesn't support CS5 and CS6.

Johan

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

* Re: [PATCH v2 7/7] USB: cdc-acm: add other non-standard xr_serial models to ignore list
  2021-03-24  7:41 ` [PATCH v2 7/7] USB: cdc-acm: add other non-standard xr_serial models to ignore list Mauro Carvalho Chehab
@ 2021-03-30 15:14   ` Johan Hovold
  0 siblings, 0 replies; 15+ messages in thread
From: Johan Hovold @ 2021-03-30 15:14 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Greg Kroah-Hartman, Oliver Neukum, linux-kernel, linux-usb

On Wed, Mar 24, 2021 at 08:41:11AM +0100, Mauro Carvalho Chehab wrote:
> Now that the xr_serial got support for other models, add their
> USB IDs as well, as those devices won't work with the standard
> CDC driver.

As far as I understand these devices should work also with the standard
class driver, but the problem is that your development board is broken
in that CTS isn't wired up properly so that TX is stalled (I even
verified that in the schematics).

Sure, there are other features available in custom-driver mode, but
still funny (sad?) if all that would really have been needed was a tiny
bit of led. :)

I kept this commit but updated the commit message and cleaned up the
entries below somewhat.

> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> ---
>  drivers/usb/class/cdc-acm.c | 17 ++++++++++++++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
> index 39ddb5585ded..839b80093478 100644
> --- a/drivers/usb/class/cdc-acm.c
> +++ b/drivers/usb/class/cdc-acm.c
> @@ -1902,9 +1902,20 @@ static const struct usb_device_id acm_ids[] = {
>  #endif
>  
>  #if IS_ENABLED(CONFIG_USB_SERIAL_XR)
> -	{ USB_DEVICE(0x04e2, 0x1410),   /* Ignore XR21V141X USB to Serial converter */
> -	.driver_info = IGNORE_DEVICE,
> -	},
> +	/* Ignore MaxLinear/Exar USB UARTs and USB UART bridges */
> +	{ USB_DEVICE(0x04e2, 0x1400), .driver_info = IGNORE_DEVICE,},
> +	{ USB_DEVICE(0x04e2, 0x1401), .driver_info = IGNORE_DEVICE,},
> +	{ USB_DEVICE(0x04e2, 0x1402), .driver_info = IGNORE_DEVICE,},
> +	{ USB_DEVICE(0x04e2, 0x1403), .driver_info = IGNORE_DEVICE,},
> +
> +	{ USB_DEVICE(0x04e2, 0x1410), .driver_info = IGNORE_DEVICE,},
> +	{ USB_DEVICE(0x04e2, 0x1411), .driver_info = IGNORE_DEVICE,},
> +	{ USB_DEVICE(0x04e2, 0x1412), .driver_info = IGNORE_DEVICE,},
> +	{ USB_DEVICE(0x04e2, 0x1414), .driver_info = IGNORE_DEVICE,},
> +
> +	{ USB_DEVICE(0x04e2, 0x1420), .driver_info = IGNORE_DEVICE,},
> +	{ USB_DEVICE(0x04e2, 0x1422), .driver_info = IGNORE_DEVICE,},
> +	{ USB_DEVICE(0x04e2, 0x1424), .driver_info = IGNORE_DEVICE,},
>  #endif
>  
>  	/*Samsung phone in firmware update mode */

Johan

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

end of thread, other threads:[~2021-03-30 15:15 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-24  7:41 [PATCH v2 0/7] Add support for the other MaxLinear/Exar UARTs Mauro Carvalho Chehab
2021-03-24  7:41 ` [PATCH v2 1/7] USB: serial: xr: simplify its namespace Mauro Carvalho Chehab
2021-03-24  7:41 ` [PATCH v2 2/7] USB: serial: xr: use a table for device-specific settings Mauro Carvalho Chehab
2021-03-30 14:44   ` Johan Hovold
2021-03-24  7:41 ` [PATCH v2 3/7] USB: serial: xr: add support for multi-port XR21V141X variants Mauro Carvalho Chehab
2021-03-30 14:50   ` Johan Hovold
2021-03-24  7:41 ` [PATCH v2 4/7] USB: serial: xr: add support for XR21B142X devices Mauro Carvalho Chehab
2021-03-30 15:04   ` Johan Hovold
2021-03-24  7:41 ` [PATCH v2 5/7] USB: serial: xr: add support for XR21B1411 Mauro Carvalho Chehab
2021-03-30 15:07   ` Johan Hovold
2021-03-24  7:41 ` [PATCH v2 6/7] USB: serial: xr: add support for XR2280X devices Mauro Carvalho Chehab
2021-03-30 15:08   ` Johan Hovold
2021-03-24  7:41 ` [PATCH v2 7/7] USB: cdc-acm: add other non-standard xr_serial models to ignore list Mauro Carvalho Chehab
2021-03-30 15:14   ` Johan Hovold
2021-03-30 14:35 ` [PATCH v2 0/7] Add support for the other MaxLinear/Exar UARTs Johan Hovold

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.