[PATCH v2 0/4] USB: serial: mxuport: add MUX50U support and updates

[PATCH v2 0/4] USB: serial: mxuport: add MUX50U support and updates

[Crescent Hsieh]

This series updates the mxuport driver to support MUX50U-based devices
and fixes a few device-specific runtime behaviours.

The first two patches clean up firmware version handling and add support
for UPort G2 and Platform UART devices. The firmware family is encoded in
the USB device-id table so that the table remains the single source for
both port count and firmware handling policy.

The third patch adds SEND_NEXT handling to pace host-to-device
transmission. The fourth patch adds RS485 mode configuration through the
standard TIOCSRS485/TIOCGRS485 interface.

Changes in v2:
  - [PATCH v1 1/4]:
    Split into [PATCH v2 1/4] and [PATCH v2 2/4]. Clean up firmware
    version parsing and printing in the first patch, and move MUX50U
    device support to the second patch. Encode the firmware family in
    the USB device-id table and use the system-register remap state for
    the MUX50U firmware download decision.
  - [PATCH v1 2/4]:
    Moved to [PATCH v2 3/4]. Add mxuport-specific write(), write_start(),
    and write_bulk_callback() paths so SEND_NEXT can stop and resume
    transmission without returning zero from prepare_write_buffer().
    Drop the open-time FIFO flush.
  - [PATCH v1 3/4]:
    Moved to [PATCH v2 4/4]. Clear unsupported serial_rs485 flags and
    fields, cache the sanitized configuration, and protect it with the
    per-port mutex.
  - [PATCH v1 4/4]:
    Drop the UART FIFO sysfs interface from this revision.

v1: https://lore.kernel.org/all/20260324035041.352190-1-crescentcy.hsieh@moxa.com/

Crescent Hsieh (4):
  USB: serial: mxuport: clean up firmware version handling
  USB: serial: mxuport: add MUX50U-based device support
  USB: serial: mxuport: handle SEND_NEXT transmit flow control
  USB: serial: mxuport: support RS485 mode configuration

 drivers/usb/serial/mxuport.c | 514 ++++++++++++++++++++++++++++++++---
 1 file changed, 470 insertions(+), 44 deletions(-)

-- 
2.43.0

[PATCH v2 1/4] USB: serial: mxuport: clean up firmware version handling

[Crescent Hsieh]

Add a small helper for parsing firmware versions and use it for the
bundled firmware image. This avoids open-coded firmware image offsets
in probe() and validates the image size before reading the version
bytes.

Keep the existing version comparison policy unchanged, but store the
version components explicitly so that the version can be printed
without unpacking a raw integer.

Print the firmware version fields in decimal, as these fields represent
readable version components rather than hexadecimal values.

Signed-off-by: Crescent Hsieh <crescentcy.hsieh@moxa.com>
---
 drivers/usb/serial/mxuport.c | 81 +++++++++++++++++++++++++-----------
 1 file changed, 57 insertions(+), 24 deletions(-)

diff --git a/drivers/usb/serial/mxuport.c b/drivers/usb/serial/mxuport.c
index e3c5a1b97542..5a9dd251ceff 100644
--- a/drivers/usb/serial/mxuport.c
+++ b/drivers/usb/serial/mxuport.c
@@ -43,9 +43,22 @@
 #define DOWN_BLOCK_SIZE		64
 
 /* Definitions for firmware info */
-#define VER_ADDR_1		0x20
-#define VER_ADDR_2		0x24
-#define VER_ADDR_3		0x28
+#define MX_FW_VER_BYTE1_OFF		0x20
+#define MX_FW_VER_BYTE2_OFF		0x24
+#define MX_FW_VER_BYTE3_OFF		0x28
+
+struct mxuport_fw_version {
+	u8 major;
+	u8 minor;
+	u8 build;
+	u32 value;
+};
+
+static const u16 mxuport_fw_ver_offsets[] = {
+	MX_FW_VER_BYTE1_OFF,
+	MX_FW_VER_BYTE2_OFF,
+	MX_FW_VER_BYTE3_OFF,
+};
 
 /* Definitions for USB vendor request */
 #define RQ_VENDOR_NONE			0x00
@@ -82,7 +95,6 @@
 
 #define RQ_VENDOR_RESET_DEVICE		0x23 /* Try to reset the device */
 #define RQ_VENDOR_QUERY_FW_CONFIG	0x24
-
 #define RQ_VENDOR_GET_VERSION		0x81 /* Get firmware version */
 #define RQ_VENDOR_GET_PAGE		0x82 /* Read flash page */
 #define RQ_VENDOR_GET_ROM_PROC		0x83 /* Get ROM process state */
@@ -970,8 +982,32 @@ static int mxuport_calc_num_ports(struct usb_serial *serial,
 	return num_ports;
 }
 
+static void mxuport_set_fw_version(struct mxuport_fw_version *version,
+				   u8 major, u8 minor, u8 build)
+{
+	version->major = major;
+	version->minor = minor;
+	version->build = build;
+	version->value = (major << 16) | (minor << 8) | build;
+}
+
+static int mxuport_parse_fw_version(const struct firmware *fw,
+				    const u16 *offsets,
+				    struct mxuport_fw_version *version)
+{
+	if (fw->size <= offsets[0] || fw->size <= offsets[1] ||
+	    fw->size <= offsets[2])
+		return -EINVAL;
+
+	mxuport_set_fw_version(version, fw->data[offsets[0]],
+			       fw->data[offsets[1]], fw->data[offsets[2]]);
+
+	return 0;
+}
+
 /* Get the version of the firmware currently running. */
-static int mxuport_get_fw_version(struct usb_serial *serial, u32 *version)
+static int mxuport_get_fw_version(struct usb_serial *serial,
+				  struct mxuport_fw_version *version)
 {
 	u8 *ver_buf;
 	int err;
@@ -988,7 +1024,7 @@ static int mxuport_get_fw_version(struct usb_serial *serial, u32 *version)
 		goto out;
 	}
 
-	*version = (ver_buf[0] << 16) | (ver_buf[1] << 8) | ver_buf[2];
+	mxuport_set_fw_version(version, ver_buf[0], ver_buf[1], ver_buf[2]);
 	err = 0;
 out:
 	kfree(ver_buf);
@@ -1049,8 +1085,7 @@ static int mxuport_probe(struct usb_serial *serial,
 {
 	u16 productid = le16_to_cpu(serial->dev->descriptor.idProduct);
 	const struct firmware *fw_p = NULL;
-	u32 version;
-	int local_ver;
+	struct mxuport_fw_version version, local_ver;
 	char buf[32];
 	int err;
 
@@ -1065,10 +1100,8 @@ static int mxuport_probe(struct usb_serial *serial,
 	if (err < 0)
 		return err;
 
-	dev_dbg(&serial->interface->dev, "Device firmware version v%x.%x.%x\n",
-		(version & 0xff0000) >> 16,
-		(version & 0xff00) >> 8,
-		(version & 0xff));
+	dev_dbg(&serial->interface->dev, "Device firmware version v%u.%u.%u\n",
+		version.major, version.minor, version.build);
 
 	snprintf(buf, sizeof(buf) - 1, "moxa/moxa-%04x.fw", productid);
 
@@ -1080,28 +1113,28 @@ static int mxuport_probe(struct usb_serial *serial,
 		/* Use the firmware already in the device */
 		err = 0;
 	} else {
-		local_ver = ((fw_p->data[VER_ADDR_1] << 16) |
-			     (fw_p->data[VER_ADDR_2] << 8) |
-			     fw_p->data[VER_ADDR_3]);
+		err = mxuport_parse_fw_version(fw_p, mxuport_fw_ver_offsets,
+					       &local_ver);
+		if (err)
+			goto out;
+
 		dev_dbg(&serial->interface->dev,
-			"Available firmware version v%x.%x.%x\n",
-			fw_p->data[VER_ADDR_1], fw_p->data[VER_ADDR_2],
-			fw_p->data[VER_ADDR_3]);
-		if (local_ver > version) {
+			"Available firmware version v%u.%u.%u\n",
+			local_ver.major, local_ver.minor, local_ver.build);
+
+		if (local_ver.value > version.value) {
 			err = mxuport_download_fw(serial, fw_p);
 			if (err)
 				goto out;
-			err  = mxuport_get_fw_version(serial, &version);
+			err = mxuport_get_fw_version(serial, &version);
 			if (err < 0)
 				goto out;
 		}
 	}
 
 	dev_info(&serial->interface->dev,
-		 "Using device firmware version v%x.%x.%x\n",
-		 (version & 0xff0000) >> 16,
-		 (version & 0xff00) >> 8,
-		 (version & 0xff));
+		 "Using device firmware version v%u.%u.%u\n",
+		 version.major, version.minor, version.build);
 
 	/*
 	 * Contains the features of this hardware. Store away for
-- 
2.43.0

[PATCH v2 2/4] USB: serial: mxuport: add MUX50U-based device support

[Crescent Hsieh]

Add support for MUX50U-based UPort G2 devices and Platform UARTs.

Encode the firmware family in the USB device id table so that the table
remains the single source for both the number of ports and the firmware
handling policy.

The MUX50U-based devices use two shared firmware images: UPort G2
devices use the UP firmware image (moxa-up-mux50u.fw), while Platform
UARTs use the PF firmware image (moxa-pf-mux50u.fw). Platform UARTs are
board-level UARTs accessed through the USB bus.

For MUX50U-based devices, the factory firmware reports versions in a
different format from the bundled runtime firmware image, so version
comparison cannot reliably determine whether a firmware download is
needed. The system-register remap state is used instead to check whether
the device has already booted the runtime firmware from SRAM.

Signed-off-by: Crescent Hsieh <crescentcy.hsieh@moxa.com>
---
 drivers/usb/serial/mxuport.c | 174 ++++++++++++++++++++++++++++++++---
 1 file changed, 162 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/serial/mxuport.c b/drivers/usb/serial/mxuport.c
index 5a9dd251ceff..067c8d9752e0 100644
--- a/drivers/usb/serial/mxuport.c
+++ b/drivers/usb/serial/mxuport.c
@@ -7,8 +7,11 @@
  *
  *	Supports the following Moxa USB to serial converters:
  *	 2 ports : UPort 1250, UPort 1250I
+ *		   UPort 1250 G2, UPort 1250I G2
  *	 4 ports : UPort 1410, UPort 1450, UPort 1450I
+ *		   UPort 1410 G2, UPort 1450 G2, UPort 1450I G2
  *	 8 ports : UPort 1610-8, UPort 1650-8
+ *		   UPort 1610-8 G2, UPort 1650-8 G2
  *	16 ports : UPort 1610-16, UPort 1650-16
  */
 
@@ -37,6 +40,24 @@
 #define MX_UPORT1613_PID	0x1613
 #define MX_UPORT1653_PID	0x1653
 
+#define MX_UPORT1252_PID	0x1252
+#define MX_UPORT1253_PID	0x1253
+#define MX_UPORT1411_PID	0x1411
+#define MX_UPORT1452_PID	0x1452
+#define MX_UPORT1453_PID	0x1453
+#define MX_UPORT1619_PID	0x1619
+#define MX_UPORT1659_PID	0x1659
+#define MX_UPORT165A_PID	0x165a
+#define MX_UPORT165B_PID	0x165b
+
+#define MX_MU250U_PID		0x0250
+#define MX_MU450U_PID		0x0450
+#define MX_MU850U_PID		0x0850
+#define MX_MU850U_6PORT_PID	0x7002
+#define MX_MUX50U_3PORT_PID	0x7003
+#define MX_MU850U_5PORT_PID	0x7004
+#define MX_MU850U_7PORT_PID	0x7005
+
 /* Definitions for USB info */
 #define HEADER_SIZE		4
 #define EVENT_LENGTH		8
@@ -46,6 +67,13 @@
 #define MX_FW_VER_BYTE1_OFF		0x20
 #define MX_FW_VER_BYTE2_OFF		0x24
 #define MX_FW_VER_BYTE3_OFF		0x28
+#define MX_MUX50U_FW_VER_BYTE1_OFF	0x86
+#define MX_MUX50U_FW_VER_BYTE2_OFF	0x88
+#define MX_MUX50U_FW_VER_BYTE3_OFF	0x8a
+
+#define MX_SYS_REG_REMAP_OFF		0x20
+#define MX_PWR_REMAP_MASK		0x03
+#define MX_REMAP_TO_SRAM		0x02
 
 struct mxuport_fw_version {
 	u8 major;
@@ -60,6 +88,12 @@ static const u16 mxuport_fw_ver_offsets[] = {
 	MX_FW_VER_BYTE3_OFF,
 };
 
+static const u16 mxuport_mux50u_fw_ver_offsets[] = {
+	MX_MUX50U_FW_VER_BYTE1_OFF,
+	MX_MUX50U_FW_VER_BYTE2_OFF,
+	MX_MUX50U_FW_VER_BYTE3_OFF,
+};
+
 /* Definitions for USB vendor request */
 #define RQ_VENDOR_NONE			0x00
 #define RQ_VENDOR_SET_BAUD		0x01 /* Set baud rate */
@@ -95,6 +129,7 @@ static const u16 mxuport_fw_ver_offsets[] = {
 
 #define RQ_VENDOR_RESET_DEVICE		0x23 /* Try to reset the device */
 #define RQ_VENDOR_QUERY_FW_CONFIG	0x24
+#define RQ_VENDOR_GET_SYS_REG		0x34
 #define RQ_VENDOR_GET_VERSION		0x81 /* Get firmware version */
 #define RQ_VENDOR_GET_PAGE		0x82 /* Read flash page */
 #define RQ_VENDOR_GET_ROM_PROC		0x83 /* Get ROM process state */
@@ -169,26 +204,65 @@ struct mxuport_port {
 #define MX_PORTS_OFFSET			1
 #define MX_PORTS(n)			(((n) - MX_PORTS_OFFSET) & MX_PORTS_MASK)
 
+#define MX_FW_FAMILY_MASK		GENMASK(5, 4)
+#define MX_FW_UPORT_G1			(0 << 4)
+#define MX_FW_UPORT_G2			(1 << 4)
+#define MX_FW_PLATFORM_UART		(2 << 4)
+#define MX_FW_FAMILY(info)		((info) & MX_FW_FAMILY_MASK)
+#define MX_DEVICE_INFO(ports, family)	(MX_PORTS(ports) | (family))
+
 /* Table of devices that work with this driver */
 static const struct usb_device_id mxuport_idtable[] = {
 	{ USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1250_PID),
-	  .driver_info = MX_PORTS(2) },
+	  .driver_info = MX_DEVICE_INFO(2, MX_FW_UPORT_G1) },
 	{ USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1251_PID),
-	  .driver_info = MX_PORTS(2) },
+	  .driver_info = MX_DEVICE_INFO(2, MX_FW_UPORT_G1) },
 	{ USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1410_PID),
-	  .driver_info = MX_PORTS(4) },
+	  .driver_info = MX_DEVICE_INFO(4, MX_FW_UPORT_G1) },
 	{ USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1450_PID),
-	  .driver_info = MX_PORTS(4) },
+	  .driver_info = MX_DEVICE_INFO(4, MX_FW_UPORT_G1) },
 	{ USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1451_PID),
-	  .driver_info = MX_PORTS(4) },
+	  .driver_info = MX_DEVICE_INFO(4, MX_FW_UPORT_G1) },
 	{ USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1618_PID),
-	  .driver_info = MX_PORTS(8) },
+	  .driver_info = MX_DEVICE_INFO(8, MX_FW_UPORT_G1) },
 	{ USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1658_PID),
-	  .driver_info = MX_PORTS(8) },
+	  .driver_info = MX_DEVICE_INFO(8, MX_FW_UPORT_G1) },
 	{ USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1613_PID),
-	  .driver_info = MX_PORTS(16) },
+	  .driver_info = MX_DEVICE_INFO(16, MX_FW_UPORT_G1) },
 	{ USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1653_PID),
-	  .driver_info = MX_PORTS(16) },
+	  .driver_info = MX_DEVICE_INFO(16, MX_FW_UPORT_G1) },
+	{ USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1252_PID),
+	  .driver_info = MX_DEVICE_INFO(2, MX_FW_UPORT_G2) },
+	{ USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1253_PID),
+	  .driver_info = MX_DEVICE_INFO(2, MX_FW_UPORT_G2) },
+	{ USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1411_PID),
+	  .driver_info = MX_DEVICE_INFO(4, MX_FW_UPORT_G2) },
+	{ USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1452_PID),
+	  .driver_info = MX_DEVICE_INFO(4, MX_FW_UPORT_G2) },
+	{ USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1453_PID),
+	  .driver_info = MX_DEVICE_INFO(4, MX_FW_UPORT_G2) },
+	{ USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1619_PID),
+	  .driver_info = MX_DEVICE_INFO(8, MX_FW_UPORT_G2) },
+	{ USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1659_PID),
+	  .driver_info = MX_DEVICE_INFO(8, MX_FW_UPORT_G2) },
+	{ USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT165A_PID),
+	  .driver_info = MX_DEVICE_INFO(8, MX_FW_UPORT_G2) },
+	{ USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT165B_PID),
+	  .driver_info = MX_DEVICE_INFO(8, MX_FW_UPORT_G2) },
+	{ USB_DEVICE(MX_USBSERIAL_VID, MX_MU250U_PID),
+	  .driver_info = MX_DEVICE_INFO(2, MX_FW_PLATFORM_UART) },
+	{ USB_DEVICE(MX_USBSERIAL_VID, MX_MU450U_PID),
+	  .driver_info = MX_DEVICE_INFO(4, MX_FW_PLATFORM_UART) },
+	{ USB_DEVICE(MX_USBSERIAL_VID, MX_MU850U_PID),
+	  .driver_info = MX_DEVICE_INFO(8, MX_FW_PLATFORM_UART) },
+	{ USB_DEVICE(MX_USBSERIAL_VID, MX_MU850U_6PORT_PID),
+	  .driver_info = MX_DEVICE_INFO(6, MX_FW_PLATFORM_UART) },
+	{ USB_DEVICE(MX_USBSERIAL_VID, MX_MUX50U_3PORT_PID),
+	  .driver_info = MX_DEVICE_INFO(3, MX_FW_PLATFORM_UART) },
+	{ USB_DEVICE(MX_USBSERIAL_VID, MX_MU850U_5PORT_PID),
+	  .driver_info = MX_DEVICE_INFO(5, MX_FW_PLATFORM_UART) },
+	{ USB_DEVICE(MX_USBSERIAL_VID, MX_MU850U_7PORT_PID),
+	  .driver_info = MX_DEVICE_INFO(7, MX_FW_PLATFORM_UART) },
 	{}			/* Terminating entry */
 };
 
@@ -1005,6 +1079,68 @@ static int mxuport_parse_fw_version(const struct firmware *fw,
 	return 0;
 }
 
+static const u16 *mxuport_fw_version_offsets(unsigned long features)
+{
+	switch (MX_FW_FAMILY(features)) {
+	case MX_FW_UPORT_G2:
+	case MX_FW_PLATFORM_UART:
+		return mxuport_mux50u_fw_ver_offsets;
+	default:
+		return mxuport_fw_ver_offsets;
+	}
+}
+
+static void mxuport_get_fw_name(unsigned long features, u16 productid,
+				char *buf, size_t size)
+{
+	switch (MX_FW_FAMILY(features)) {
+	case MX_FW_UPORT_G2:
+		strscpy(buf, "moxa/moxa-up-mux50u.fw", size);
+		break;
+	case MX_FW_PLATFORM_UART:
+		strscpy(buf, "moxa/moxa-pf-mux50u.fw", size);
+		break;
+	default:
+		snprintf(buf, size, "moxa/moxa-%04x.fw", productid);
+		break;
+	}
+}
+
+static bool mxuport_is_mux50u_device(unsigned long features)
+{
+	switch (MX_FW_FAMILY(features)) {
+	case MX_FW_UPORT_G2:
+	case MX_FW_PLATFORM_UART:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static int mxuport_fw_running_from_sram(struct usb_serial *serial,
+					bool *from_sram)
+{
+	u8 *buf;
+	u32 reg;
+	int err;
+
+	buf = kzalloc(4, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	err = mxuport_recv_ctrl_urb(serial, RQ_VENDOR_GET_SYS_REG,
+				    MX_SYS_REG_REMAP_OFF, 0, buf, 4);
+	if (err != 4)
+		goto out;
+
+	reg = get_unaligned_be32(buf);
+	*from_sram = (((reg >> 24) & MX_PWR_REMAP_MASK) == MX_REMAP_TO_SRAM);
+	err = 0;
+out:
+	kfree(buf);
+	return err;
+}
+
 /* Get the version of the firmware currently running. */
 static int mxuport_get_fw_version(struct usb_serial *serial,
 				  struct mxuport_fw_version *version)
@@ -1083,9 +1219,12 @@ static int mxuport_download_fw(struct usb_serial *serial,
 static int mxuport_probe(struct usb_serial *serial,
 			 const struct usb_device_id *id)
 {
+	unsigned long features = id->driver_info;
+	const u16 *fw_version_offsets;
 	u16 productid = le16_to_cpu(serial->dev->descriptor.idProduct);
 	const struct firmware *fw_p = NULL;
 	struct mxuport_fw_version version, local_ver;
+	bool download_fw, from_sram;
 	char buf[32];
 	int err;
 
@@ -1103,7 +1242,8 @@ static int mxuport_probe(struct usb_serial *serial,
 	dev_dbg(&serial->interface->dev, "Device firmware version v%u.%u.%u\n",
 		version.major, version.minor, version.build);
 
-	snprintf(buf, sizeof(buf) - 1, "moxa/moxa-%04x.fw", productid);
+	fw_version_offsets = mxuport_fw_version_offsets(features);
+	mxuport_get_fw_name(features, productid, buf, sizeof(buf));
 
 	err = request_firmware(&fw_p, buf, &serial->interface->dev);
 	if (err) {
@@ -1113,7 +1253,7 @@ static int mxuport_probe(struct usb_serial *serial,
 		/* Use the firmware already in the device */
 		err = 0;
 	} else {
-		err = mxuport_parse_fw_version(fw_p, mxuport_fw_ver_offsets,
+		err = mxuport_parse_fw_version(fw_p, fw_version_offsets,
 					       &local_ver);
 		if (err)
 			goto out;
@@ -1122,7 +1262,17 @@ static int mxuport_probe(struct usb_serial *serial,
 			"Available firmware version v%u.%u.%u\n",
 			local_ver.major, local_ver.minor, local_ver.build);
 
-		if (local_ver.value > version.value) {
+		if (mxuport_is_mux50u_device(features)) {
+			err = mxuport_fw_running_from_sram(serial, &from_sram);
+			if (err)
+				goto out;
+
+			download_fw = !from_sram;
+		} else {
+			download_fw = local_ver.value > version.value;
+		}
+
+		if (download_fw) {
 			err = mxuport_download_fw(serial, fw_p);
 			if (err)
 				goto out;
-- 
2.43.0

[PATCH v2 3/4] USB: serial: mxuport: handle SEND_NEXT transmit flow control

[Crescent Hsieh]

The device uses the SEND_NEXT event to pace host-to-device transmission.
Without waiting for this event, continuous transmission on multiple
ports can make the driver submit bulk-out URBs faster than the device
can process them, which can result in data errors during burn-in
testing.

Stop submitting further write URBs after requesting SEND_NEXT and resume
transmission when the matching event is received.

This cannot be implemented by returning zero from
prepare_write_buffer(), as the generic write implementation expects the
callback to return a transfer length once data is available in the write
FIFO. Add a mxuport-specific write path so that URB submission can be
stopped and resumed explicitly.

This may reduce throughput, but avoids data errors under sustained
transmit load.

Signed-off-by: Crescent Hsieh <crescentcy.hsieh@moxa.com>
---
 drivers/usb/serial/mxuport.c | 165 +++++++++++++++++++++++++++++++++--
 1 file changed, 156 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/serial/mxuport.c b/drivers/usb/serial/mxuport.c
index 067c8d9752e0..f3be2b3bd95b 100644
--- a/drivers/usb/serial/mxuport.c
+++ b/drivers/usb/serial/mxuport.c
@@ -147,6 +147,8 @@ static const u16 mxuport_mux50u_fw_ver_offsets[] = {
 #define UPORT_EVENT_LSR			4 /* Line status */
 #define UPORT_EVENT_MCR			5 /* Modem control */
 
+#define UPORT_REQUEST_SEND_NEXT		0x80
+
 /* Definitions for serial event type */
 #define SERIAL_EV_CTS			0x0008	/* CTS changed state */
 #define SERIAL_EV_DSR			0x0010	/* DSR changed state */
@@ -193,6 +195,8 @@ static const u16 mxuport_mux50u_fw_ver_offsets[] = {
 
 /* This structure holds all of the local port information */
 struct mxuport_port {
+	u32 sent_payload;
+	u8 hold_reason;
 	u8 mcr_state;		/* Last MCR state */
 	u8 msr_state;		/* Last MSR state */
 	struct mutex mutex;	/* Protects mcr_state */
@@ -276,22 +280,148 @@ MODULE_DEVICE_TABLE(usb, mxuport_idtable);
 static int mxuport_prepare_write_buffer(struct usb_serial_port *port,
 					void *dest, size_t size)
 {
+	struct mxuport_port *mxport = usb_get_serial_port_data(port);
 	u8 *buf = dest;
+	unsigned long flags;
+	bool request_send_next;
 	int count;
 
-	count = kfifo_out_locked(&port->write_fifo, buf + HEADER_SIZE,
-				 size - HEADER_SIZE,
-				 &port->lock);
+	spin_lock_irqsave(&port->lock, flags);
+	count = kfifo_out(&port->write_fifo, buf + HEADER_SIZE,
+			  size - HEADER_SIZE);
+	mxport->sent_payload += count;
+	request_send_next = mxport->sent_payload >= port->bulk_out_size;
+	if (request_send_next)
+		mxport->hold_reason |= MX_WAIT_FOR_SEND_NEXT;
+	spin_unlock_irqrestore(&port->lock, flags);
 
 	put_unaligned_be16(port->port_number, buf);
 	put_unaligned_be16(count, buf + 2);
 
+	if (request_send_next)
+		buf[0] |= UPORT_REQUEST_SEND_NEXT;
+
 	dev_dbg(&port->dev, "%s - size %zd count %d\n", __func__,
 		size, count);
 
 	return count + HEADER_SIZE;
 }
 
+static int mxuport_write_start(struct usb_serial_port *port, gfp_t mem_flags)
+{
+	struct mxuport_port *mxport = usb_get_serial_port_data(port);
+	struct urb *urb;
+	unsigned long flags;
+	int i;
+	int count;
+	int result;
+
+	if (test_and_set_bit_lock(USB_SERIAL_WRITE_BUSY, &port->flags))
+		return 0;
+retry:
+	spin_lock_irqsave(&port->lock, flags);
+	if ((mxport->hold_reason & MX_WAIT_FOR_SEND_NEXT) ||
+	    !port->write_urbs_free || !kfifo_len(&port->write_fifo)) {
+		clear_bit_unlock(USB_SERIAL_WRITE_BUSY, &port->flags);
+		spin_unlock_irqrestore(&port->lock, flags);
+		return 0;
+	}
+
+	i = (int)find_first_bit(&port->write_urbs_free,
+				ARRAY_SIZE(port->write_urbs));
+	spin_unlock_irqrestore(&port->lock, flags);
+
+	urb = port->write_urbs[i];
+	count = mxuport_prepare_write_buffer(port, urb->transfer_buffer,
+					     port->bulk_out_size);
+	urb->transfer_buffer_length = count;
+	usb_serial_debug_data(&port->dev, __func__, count,
+			      urb->transfer_buffer);
+
+	spin_lock_irqsave(&port->lock, flags);
+	port->tx_bytes += count;
+	spin_unlock_irqrestore(&port->lock, flags);
+
+	clear_bit(i, &port->write_urbs_free);
+	result = usb_submit_urb(urb, mem_flags);
+	if (result) {
+		dev_err_console(port, "%s - error submitting urb: %d\n",
+				__func__, result);
+		set_bit(i, &port->write_urbs_free);
+		spin_lock_irqsave(&port->lock, flags);
+		port->tx_bytes -= count;
+		if (mxport->hold_reason & MX_WAIT_FOR_SEND_NEXT) {
+			mxport->hold_reason &= ~MX_WAIT_FOR_SEND_NEXT;
+			mxport->sent_payload = 0;
+		}
+		spin_unlock_irqrestore(&port->lock, flags);
+
+		clear_bit_unlock(USB_SERIAL_WRITE_BUSY, &port->flags);
+		return result;
+	}
+
+	goto retry;
+}
+
+static int mxuport_write(struct tty_struct *tty, struct usb_serial_port *port,
+			 const unsigned char *buf, int count)
+{
+	int result;
+
+	if (!port->bulk_out_size)
+		return -ENODEV;
+
+	if (!count)
+		return 0;
+
+	count = kfifo_in_locked(&port->write_fifo, buf, count, &port->lock);
+	result = mxuport_write_start(port, GFP_ATOMIC);
+	if (result)
+		return result;
+
+	return count;
+}
+
+static void mxuport_write_bulk_callback(struct urb *urb)
+{
+	struct usb_serial_port *port = urb->context;
+	unsigned long flags;
+	int status = urb->status;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(port->write_urbs); ++i) {
+		if (port->write_urbs[i] == urb)
+			break;
+	}
+
+	spin_lock_irqsave(&port->lock, flags);
+	port->tx_bytes -= urb->transfer_buffer_length;
+	set_bit(i, &port->write_urbs_free);
+	spin_unlock_irqrestore(&port->lock, flags);
+
+	switch (status) {
+	case 0:
+		break;
+	case -ENOENT:
+	case -ECONNRESET:
+	case -ESHUTDOWN:
+		dev_dbg(&port->dev, "%s - urb stopped: %d\n", __func__,
+			status);
+		return;
+	case -EPIPE:
+		dev_err_console(port, "%s - urb stopped: %d\n", __func__,
+				status);
+		return;
+	default:
+		dev_err_console(port, "%s - nonzero urb status: %d\n",
+				__func__, status);
+		break;
+	}
+
+	mxuport_write_start(port, GFP_ATOMIC);
+	usb_serial_port_softint(port);
+}
+
 /* Read the given buffer in from the control pipe. */
 static int mxuport_recv_ctrl_urb(struct usb_serial *serial,
 				 u8 request, u16 value, u16 index,
@@ -510,14 +640,24 @@ static void mxuport_lsr_event(struct usb_serial_port *port, u8 buf[4])
 static void mxuport_process_read_urb_event(struct usb_serial_port *port,
 					   u8 buf[4], u32 event)
 {
+	struct mxuport_port *mxport = usb_get_serial_port_data(port);
+	unsigned long flags;
+	bool resume_tx = false;
+
 	dev_dbg(&port->dev, "%s - receive event : %04x\n", __func__, event);
 
 	switch (event) {
 	case UPORT_EVENT_SEND_NEXT:
-		/*
-		 * Sent as part of the flow control on device buffers.
-		 * Not currently used.
-		 */
+		spin_lock_irqsave(&port->lock, flags);
+		if (mxport->hold_reason & MX_WAIT_FOR_SEND_NEXT) {
+			mxport->hold_reason &= ~MX_WAIT_FOR_SEND_NEXT;
+			mxport->sent_payload = 0;
+			resume_tx = true;
+		}
+		spin_unlock_irqrestore(&port->lock, flags);
+
+		if (resume_tx)
+			mxuport_write_start(port, GFP_ATOMIC);
 		break;
 	case UPORT_EVENT_MSR:
 		mxuport_msr_event(port, buf);
@@ -1372,6 +1512,7 @@ static int mxuport_open(struct tty_struct *tty, struct usb_serial_port *port)
 {
 	struct mxuport_port *mxport = usb_get_serial_port_data(port);
 	struct usb_serial *serial = port->serial;
+	unsigned long flags;
 	int err;
 
 	/* Set receive host (enable) */
@@ -1396,6 +1537,11 @@ static int mxuport_open(struct tty_struct *tty, struct usb_serial_port *port)
 	 * TODO: use RQ_VENDOR_GET_MSR, once we know what it
 	 * returns.
 	 */
+	spin_lock_irqsave(&port->lock, flags);
+	mxport->sent_payload = 0;
+	mxport->hold_reason &= ~MX_WAIT_FOR_SEND_NEXT;
+	spin_unlock_irqrestore(&port->lock, flags);
+
 	mxport->msr_state = 0;
 
 	return err;
@@ -1451,7 +1597,7 @@ static int mxuport_resume(struct usb_serial *serial)
 		if (!tty_port_initialized(&port->port))
 			continue;
 
-		r = usb_serial_generic_write_start(port, GFP_NOIO);
+		r = mxuport_write_start(port, GFP_NOIO);
 		if (r < 0)
 			c++;
 	}
@@ -1477,6 +1623,8 @@ static struct usb_serial_driver mxuport_device = {
 	.set_termios		= mxuport_set_termios,
 	.break_ctl		= mxuport_break_ctl,
 	.tx_empty		= mxuport_tx_empty,
+	.write			= mxuport_write,
+	.write_bulk_callback	= mxuport_write_bulk_callback,
 	.tiocmiwait		= usb_serial_generic_tiocmiwait,
 	.get_icount		= usb_serial_generic_get_icount,
 	.throttle		= mxuport_throttle,
@@ -1485,7 +1633,6 @@ static struct usb_serial_driver mxuport_device = {
 	.tiocmset		= mxuport_tiocmset,
 	.dtr_rts		= mxuport_dtr_rts,
 	.process_read_urb	= mxuport_process_read_urb,
-	.prepare_write_buffer	= mxuport_prepare_write_buffer,
 	.resume			= mxuport_resume,
 };
 
-- 
2.43.0

[PATCH v2 4/4] USB: serial: mxuport: support RS485 mode configuration

[Crescent Hsieh]

Add support for TIOCSRS485 and TIOCGRS485 so that userspace can
configure the serial interface mode using struct serial_rs485.

Map the serial_rs485 flags to the device interface modes as follows:

  - RS232                  = no flags set
  - RS422                  = SER_RS485_ENABLED | SER_RS485_MODE_RS422
  - RS485_2W (half-duplex) = SER_RS485_ENABLED
  - RS485_4W (full-duplex) = SER_RS485_ENABLED | SER_RS485_RX_DURING_TX

Unsupported flags and fields are cleared before applying the
configuration. Cache the sanitized per-port configuration and serialize
access with the per-port mutex.

Signed-off-by: Crescent Hsieh <crescentcy.hsieh@moxa.com>
---
 drivers/usb/serial/mxuport.c | 98 +++++++++++++++++++++++++++++++++++-
 1 file changed, 97 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/serial/mxuport.c b/drivers/usb/serial/mxuport.c
index f3be2b3bd95b..79085595345c 100644
--- a/drivers/usb/serial/mxuport.c
+++ b/drivers/usb/serial/mxuport.c
@@ -199,7 +199,8 @@ struct mxuport_port {
 	u8 hold_reason;
 	u8 mcr_state;		/* Last MCR state */
 	u8 msr_state;		/* Last MSR state */
-	struct mutex mutex;	/* Protects mcr_state */
+	struct serial_rs485 rs485;
+	struct mutex mutex;	/* Protects per-port control state */
 	spinlock_t spinlock;	/* Protects msr_state */
 };
 
@@ -983,6 +984,100 @@ static int mxuport_tiocmget(struct tty_struct *tty)
 	return result;
 }
 
+static void mxuport_sanitize_serial_rs485(struct serial_rs485 *rs485)
+{
+	if (!(rs485->flags & SER_RS485_ENABLED)) {
+		memset(rs485, 0, sizeof(*rs485));
+		return;
+	}
+
+	if (rs485->flags & SER_RS485_MODE_RS422)
+		rs485->flags &= SER_RS485_ENABLED | SER_RS485_MODE_RS422;
+	else
+		rs485->flags &= SER_RS485_ENABLED | SER_RS485_RX_DURING_TX;
+
+	rs485->delay_rts_before_send = 0;
+	rs485->delay_rts_after_send = 0;
+	memset(rs485->padding, 0, sizeof(rs485->padding));
+}
+
+static int mxuport_rs485_config(struct usb_serial_port *port,
+				const struct serial_rs485 *rs485)
+{
+	struct usb_serial *serial = port->serial;
+	u16 mode = MX_INT_RS232;
+
+	if (rs485->flags & SER_RS485_ENABLED) {
+		if (rs485->flags & SER_RS485_MODE_RS422)
+			mode = MX_INT_RS422;
+		else if (rs485->flags & SER_RS485_RX_DURING_TX)
+			mode = MX_INT_4W_RS485;
+		else
+			mode = MX_INT_2W_RS485;
+	}
+
+	return mxuport_send_ctrl_urb(serial, RQ_VENDOR_SET_INTERFACE, mode,
+				     port->port_number);
+}
+
+static int mxuport_get_rs485_config(struct usb_serial_port *port,
+				    struct serial_rs485 __user *argp)
+{
+	struct mxuport_port *mxport = usb_get_serial_port_data(port);
+	struct serial_rs485 rs485;
+
+	mutex_lock(&mxport->mutex);
+	rs485 = mxport->rs485;
+	mutex_unlock(&mxport->mutex);
+
+	if (copy_to_user(argp, &rs485, sizeof(rs485)))
+		return -EFAULT;
+
+	return 0;
+}
+
+static int mxuport_set_rs485_config(struct usb_serial_port *port,
+				    struct serial_rs485 __user *argp)
+{
+	struct mxuport_port *mxport = usb_get_serial_port_data(port);
+	struct serial_rs485 rs485;
+	int err;
+
+	if (copy_from_user(&rs485, argp, sizeof(rs485)))
+		return -EFAULT;
+
+	mxuport_sanitize_serial_rs485(&rs485);
+
+	mutex_lock(&mxport->mutex);
+	err = mxuport_rs485_config(port, &rs485);
+	if (!err)
+		mxport->rs485 = rs485;
+	mutex_unlock(&mxport->mutex);
+	if (err)
+		return err;
+
+	if (copy_to_user(argp, &rs485, sizeof(rs485)))
+		return -EFAULT;
+
+	return 0;
+}
+
+static int mxuport_ioctl(struct tty_struct *tty, unsigned int cmd,
+			 unsigned long arg)
+{
+	struct usb_serial_port *port = tty->driver_data;
+	void __user *argp = (void __user *)arg;
+
+	switch (cmd) {
+	case TIOCGRS485:
+		return mxuport_get_rs485_config(port, argp);
+	case TIOCSRS485:
+		return mxuport_set_rs485_config(port, argp);
+	}
+
+	return -ENOIOCTLCMD;
+}
+
 static int mxuport_set_termios_flow(struct tty_struct *tty,
 				    const struct ktermios *old_termios,
 				    struct usb_serial_port *port,
@@ -1625,6 +1720,7 @@ static struct usb_serial_driver mxuport_device = {
 	.tx_empty		= mxuport_tx_empty,
 	.write			= mxuport_write,
 	.write_bulk_callback	= mxuport_write_bulk_callback,
+	.ioctl			= mxuport_ioctl,
 	.tiocmiwait		= usb_serial_generic_tiocmiwait,
 	.get_icount		= usb_serial_generic_get_icount,
 	.throttle		= mxuport_throttle,
-- 
2.43.0

Re: [PATCH v2 1/4] USB: serial: mxuport: clean up firmware version handling

[Johan Hovold]

On Tue, Jun 23, 2026 at 04:01:36PM +0800, Crescent Hsieh wrote:
> Add a small helper for parsing firmware versions and use it for the
> bundled firmware image. This avoids open-coded firmware image offsets
> in probe() and validates the image size before reading the version
> bytes.
> 
> Keep the existing version comparison policy unchanged, but store the
> version components explicitly so that the version can be printed
> without unpacking a raw integer.
> 
> Print the firmware version fields in decimal, as these fields represent
> readable version components rather than hexadecimal values.
> 
> Signed-off-by: Crescent Hsieh <crescentcy.hsieh@moxa.com>
> ---

Thanks for the v2. You seem to have addressed all my comments on v1 of
the series, which is unfortunately not as common as I wish it were. :)

> @@ -82,7 +95,6 @@
>  
>  #define RQ_VENDOR_RESET_DEVICE		0x23 /* Try to reset the device */
>  #define RQ_VENDOR_QUERY_FW_CONFIG	0x24
> -

Nit: this is an unrelated change

I see you add another define here in the next patch, but perhaps you
should keep the newline separators before and after that one.

>  #define RQ_VENDOR_GET_VERSION		0x81 /* Get firmware version */
>  #define RQ_VENDOR_GET_PAGE		0x82 /* Read flash page */
>  #define RQ_VENDOR_GET_ROM_PROC		0x83 /* Get ROM process state */
> @@ -970,8 +982,32 @@ static int mxuport_calc_num_ports(struct usb_serial *serial,
>  	return num_ports;
>  }
>  
> +static void mxuport_set_fw_version(struct mxuport_fw_version *version,
> +				   u8 major, u8 minor, u8 build)

The naming is a bit unfortunate as this is not an inverse of
mxuport_get_fw_version() but that should be ok.

> +{
> +	version->major = major;
> +	version->minor = minor;
> +	version->build = build;
> +	version->value = (major << 16) | (minor << 8) | build;
> +}
> +
> +static int mxuport_parse_fw_version(const struct firmware *fw,
> +				    const u16 *offsets,
> +				    struct mxuport_fw_version *version)
> +{
> +	if (fw->size <= offsets[0] || fw->size <= offsets[1] ||
> +	    fw->size <= offsets[2])
> +		return -EINVAL;

I just merged this sanity check:

	https://lore.kernel.org/all/20260715084611.45995-1-pengpeng@iscas.ac.cn/

so you may want to rebase on my usb-linus branch if there is a v3,
otherwise I can fix it up.

	https://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git/log/?h=usb-linus

> +
> +	mxuport_set_fw_version(version, fw->data[offsets[0]],
> +			       fw->data[offsets[1]], fw->data[offsets[2]]);
> +
> +	return 0;
> +}
> +
>  /* Get the version of the firmware currently running. */
> -static int mxuport_get_fw_version(struct usb_serial *serial, u32 *version)
> +static int mxuport_get_fw_version(struct usb_serial *serial,
> +				  struct mxuport_fw_version *version)
>  {
>  	u8 *ver_buf;
>  	int err;
> @@ -988,7 +1024,7 @@ static int mxuport_get_fw_version(struct usb_serial *serial, u32 *version)
>  		goto out;
>  	}
>  
> -	*version = (ver_buf[0] << 16) | (ver_buf[1] << 8) | ver_buf[2];
> +	mxuport_set_fw_version(version, ver_buf[0], ver_buf[1], ver_buf[2]);
>  	err = 0;
>  out:
>  	kfree(ver_buf);
> @@ -1049,8 +1085,7 @@ static int mxuport_probe(struct usb_serial *serial,
>  {
>  	u16 productid = le16_to_cpu(serial->dev->descriptor.idProduct);
>  	const struct firmware *fw_p = NULL;
> -	u32 version;
> -	int local_ver;
> +	struct mxuport_fw_version version, local_ver;

Nit: I'd move this line above fw_p to maintain reverse xmas style of the
declarations.

>  	char buf[32];
>  	int err;

Johan

Re: [PATCH v2 2/4] USB: serial: mxuport: add MUX50U-based device support

[Johan Hovold]

On Tue, Jun 23, 2026 at 04:01:37PM +0800, Crescent Hsieh wrote:
> Add support for MUX50U-based UPort G2 devices and Platform UARTs.
> 
> Encode the firmware family in the USB device id table so that the table
> remains the single source for both the number of ports and the firmware
> handling policy.
> 
> The MUX50U-based devices use two shared firmware images: UPort G2
> devices use the UP firmware image (moxa-up-mux50u.fw), while Platform
> UARTs use the PF firmware image (moxa-pf-mux50u.fw). Platform UARTs are
> board-level UARTs accessed through the USB bus.
> 
> For MUX50U-based devices, the factory firmware reports versions in a
> different format from the bundled runtime firmware image, so version
> comparison cannot reliably determine whether a firmware download is
> needed. The system-register remap state is used instead to check whether
> the device has already booted the runtime firmware from SRAM.
> 
> Signed-off-by: Crescent Hsieh <crescentcy.hsieh@moxa.com>
> ---
>  drivers/usb/serial/mxuport.c | 174 ++++++++++++++++++++++++++++++++---
>  1 file changed, 162 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/usb/serial/mxuport.c b/drivers/usb/serial/mxuport.c
> index 5a9dd251ceff..067c8d9752e0 100644
> --- a/drivers/usb/serial/mxuport.c
> +++ b/drivers/usb/serial/mxuport.c
> @@ -7,8 +7,11 @@
>   *
>   *	Supports the following Moxa USB to serial converters:
>   *	 2 ports : UPort 1250, UPort 1250I
> + *		   UPort 1250 G2, UPort 1250I G2
>   *	 4 ports : UPort 1410, UPort 1450, UPort 1450I
> + *		   UPort 1410 G2, UPort 1450 G2, UPort 1450I G2
>   *	 8 ports : UPort 1610-8, UPort 1650-8
> + *		   UPort 1610-8 G2, UPort 1650-8 G2
>   *	16 ports : UPort 1610-16, UPort 1650-16

Should you add the MUx50U ones here as well for completeness? At least
some of them seem to be missing.

But perhaps it's just me not getting how these PIDs map to products.

>   */
>  
> @@ -37,6 +40,24 @@
>  #define MX_UPORT1613_PID	0x1613
>  #define MX_UPORT1653_PID	0x1653
>  
> +#define MX_UPORT1252_PID	0x1252
> +#define MX_UPORT1253_PID	0x1253
> +#define MX_UPORT1411_PID	0x1411
> +#define MX_UPORT1452_PID	0x1452
> +#define MX_UPORT1453_PID	0x1453
> +#define MX_UPORT1619_PID	0x1619
> +#define MX_UPORT1659_PID	0x1659
> +#define MX_UPORT165A_PID	0x165a
> +#define MX_UPORT165B_PID	0x165b

I realise this is how the existing PID defines are named, but would it
not be possible to name the defines after the products they are used
for?

> +
> +#define MX_MU250U_PID		0x0250
> +#define MX_MU450U_PID		0x0450
> +#define MX_MU850U_PID		0x0850
> +#define MX_MU850U_6PORT_PID	0x7002
> +#define MX_MUX50U_3PORT_PID	0x7003
> +#define MX_MU850U_5PORT_PID	0x7004
> +#define MX_MU850U_7PORT_PID	0x7005
> +
>  /* Definitions for USB info */
>  #define HEADER_SIZE		4
>  #define EVENT_LENGTH		8
> @@ -46,6 +67,13 @@
>  #define MX_FW_VER_BYTE1_OFF		0x20
>  #define MX_FW_VER_BYTE2_OFF		0x24
>  #define MX_FW_VER_BYTE3_OFF		0x28
> +#define MX_MUX50U_FW_VER_BYTE1_OFF	0x86
> +#define MX_MUX50U_FW_VER_BYTE2_OFF	0x88
> +#define MX_MUX50U_FW_VER_BYTE3_OFF	0x8a
> +
> +#define MX_SYS_REG_REMAP_OFF		0x20
> +#define MX_PWR_REMAP_MASK		0x03
> +#define MX_REMAP_TO_SRAM		0x02
>  
>  struct mxuport_fw_version {
>  	u8 major;
> @@ -60,6 +88,12 @@ static const u16 mxuport_fw_ver_offsets[] = {
>  	MX_FW_VER_BYTE3_OFF,
>  };
>  
> +static const u16 mxuport_mux50u_fw_ver_offsets[] = {
> +	MX_MUX50U_FW_VER_BYTE1_OFF,
> +	MX_MUX50U_FW_VER_BYTE2_OFF,
> +	MX_MUX50U_FW_VER_BYTE3_OFF,
> +};
> +
>  /* Definitions for USB vendor request */
>  #define RQ_VENDOR_NONE			0x00
>  #define RQ_VENDOR_SET_BAUD		0x01 /* Set baud rate */
> @@ -95,6 +129,7 @@ static const u16 mxuport_fw_ver_offsets[] = {
>  
>  #define RQ_VENDOR_RESET_DEVICE		0x23 /* Try to reset the device */
>  #define RQ_VENDOR_QUERY_FW_CONFIG	0x24
> +#define RQ_VENDOR_GET_SYS_REG		0x34

So here I guess you should have a newline separator before and after
0x34 to follow the existing style (of grouping related registers).

>  #define RQ_VENDOR_GET_VERSION		0x81 /* Get firmware version */
>  #define RQ_VENDOR_GET_PAGE		0x82 /* Read flash page */
>  #define RQ_VENDOR_GET_ROM_PROC		0x83 /* Get ROM process state */
> @@ -169,26 +204,65 @@ struct mxuport_port {
>  #define MX_PORTS_OFFSET			1
>  #define MX_PORTS(n)			(((n) - MX_PORTS_OFFSET) & MX_PORTS_MASK)
>  
> +#define MX_FW_FAMILY_MASK		GENMASK(5, 4)
> +#define MX_FW_UPORT_G1			(0 << 4)
> +#define MX_FW_UPORT_G2			(1 << 4)
> +#define MX_FW_PLATFORM_UART		(2 << 4)
> +#define MX_FW_FAMILY(info)		((info) & MX_FW_FAMILY_MASK)
> +#define MX_DEVICE_INFO(ports, family)	(MX_PORTS(ports) | (family))
> +
>  /* Table of devices that work with this driver */
>  static const struct usb_device_id mxuport_idtable[] = {
>  	{ USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1250_PID),
> -	  .driver_info = MX_PORTS(2) },
> +	  .driver_info = MX_DEVICE_INFO(2, MX_FW_UPORT_G1) },
>  	{ USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1251_PID),
> -	  .driver_info = MX_PORTS(2) },
> +	  .driver_info = MX_DEVICE_INFO(2, MX_FW_UPORT_G1) },
>  	{ USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1410_PID),
> -	  .driver_info = MX_PORTS(4) },
> +	  .driver_info = MX_DEVICE_INFO(4, MX_FW_UPORT_G1) },
>  	{ USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1450_PID),
> -	  .driver_info = MX_PORTS(4) },
> +	  .driver_info = MX_DEVICE_INFO(4, MX_FW_UPORT_G1) },
>  	{ USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1451_PID),
> -	  .driver_info = MX_PORTS(4) },
> +	  .driver_info = MX_DEVICE_INFO(4, MX_FW_UPORT_G1) },
>  	{ USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1618_PID),
> -	  .driver_info = MX_PORTS(8) },
> +	  .driver_info = MX_DEVICE_INFO(8, MX_FW_UPORT_G1) },
>  	{ USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1658_PID),
> -	  .driver_info = MX_PORTS(8) },
> +	  .driver_info = MX_DEVICE_INFO(8, MX_FW_UPORT_G1) },
>  	{ USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1613_PID),
> -	  .driver_info = MX_PORTS(16) },
> +	  .driver_info = MX_DEVICE_INFO(16, MX_FW_UPORT_G1) },
>  	{ USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1653_PID),
> -	  .driver_info = MX_PORTS(16) },

My initial reaction was that it would be better to just leave the gen1
entries alone (with family implicitly set to MX_FW_UPORT_G1), but I
guess this is fine too.

> +	  .driver_info = MX_DEVICE_INFO(16, MX_FW_UPORT_G1) },
> +	{ USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1252_PID),
> +	  .driver_info = MX_DEVICE_INFO(2, MX_FW_UPORT_G2) },
> +	{ USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1253_PID),
> +	  .driver_info = MX_DEVICE_INFO(2, MX_FW_UPORT_G2) },
> +	{ USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1411_PID),
> +	  .driver_info = MX_DEVICE_INFO(4, MX_FW_UPORT_G2) },
> +	{ USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1452_PID),
> +	  .driver_info = MX_DEVICE_INFO(4, MX_FW_UPORT_G2) },
> +	{ USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1453_PID),
> +	  .driver_info = MX_DEVICE_INFO(4, MX_FW_UPORT_G2) },
> +	{ USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1619_PID),
> +	  .driver_info = MX_DEVICE_INFO(8, MX_FW_UPORT_G2) },
> +	{ USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1659_PID),
> +	  .driver_info = MX_DEVICE_INFO(8, MX_FW_UPORT_G2) },
> +	{ USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT165A_PID),
> +	  .driver_info = MX_DEVICE_INFO(8, MX_FW_UPORT_G2) },
> +	{ USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT165B_PID),
> +	  .driver_info = MX_DEVICE_INFO(8, MX_FW_UPORT_G2) },
> +	{ USB_DEVICE(MX_USBSERIAL_VID, MX_MU250U_PID),
> +	  .driver_info = MX_DEVICE_INFO(2, MX_FW_PLATFORM_UART) },
> +	{ USB_DEVICE(MX_USBSERIAL_VID, MX_MU450U_PID),
> +	  .driver_info = MX_DEVICE_INFO(4, MX_FW_PLATFORM_UART) },
> +	{ USB_DEVICE(MX_USBSERIAL_VID, MX_MU850U_PID),
> +	  .driver_info = MX_DEVICE_INFO(8, MX_FW_PLATFORM_UART) },
> +	{ USB_DEVICE(MX_USBSERIAL_VID, MX_MU850U_6PORT_PID),
> +	  .driver_info = MX_DEVICE_INFO(6, MX_FW_PLATFORM_UART) },
> +	{ USB_DEVICE(MX_USBSERIAL_VID, MX_MUX50U_3PORT_PID),
> +	  .driver_info = MX_DEVICE_INFO(3, MX_FW_PLATFORM_UART) },
> +	{ USB_DEVICE(MX_USBSERIAL_VID, MX_MU850U_5PORT_PID),
> +	  .driver_info = MX_DEVICE_INFO(5, MX_FW_PLATFORM_UART) },
> +	{ USB_DEVICE(MX_USBSERIAL_VID, MX_MU850U_7PORT_PID),
> +	  .driver_info = MX_DEVICE_INFO(7, MX_FW_PLATFORM_UART) },
>  	{}			/* Terminating entry */
>  };

Perhaps the family defines can be shortened to make this a bit more
readable, for example:

	MX_FW_UP_G1
	MX_FW_UP_G2
	MX_FW_PF

or even

	MX_UP_G1
	MX_UP_G2
	MX_PF

I didn't try and see what the result looks like. Perhaps the more
verbose names are preferred.

> @@ -1083,9 +1219,12 @@ static int mxuport_download_fw(struct usb_serial *serial,
>  static int mxuport_probe(struct usb_serial *serial,
>  			 const struct usb_device_id *id)
>  {
> +	unsigned long features = id->driver_info;
> +	const u16 *fw_version_offsets;

Nit: reverse xmas tends to be more readable

>  	u16 productid = le16_to_cpu(serial->dev->descriptor.idProduct);
>  	const struct firmware *fw_p = NULL;
>  	struct mxuport_fw_version version, local_ver;
> +	bool download_fw, from_sram;
>  	char buf[32];
>  	int err;

Johan

Re: [PATCH v2 3/4] USB: serial: mxuport: handle SEND_NEXT transmit flow control

[Johan Hovold]

On Tue, Jun 23, 2026 at 04:01:38PM +0800, Crescent Hsieh wrote:
> The device uses the SEND_NEXT event to pace host-to-device transmission.
> Without waiting for this event, continuous transmission on multiple
> ports can make the driver submit bulk-out URBs faster than the device
> can process them, which can result in data errors during burn-in
> testing.

Why does it matter if we're sending data for other ports? Isn't the
problem that we're sending more data than the per-port buffer can hold?

And is this an issue with all firmware versions, including the ones used
for the existing gen1 devices?
 
> Stop submitting further write URBs after requesting SEND_NEXT and resume
> transmission when the matching event is received.

How exactly does SEND_NEXT work? Is it signalled when the per-port
buffer drops below some threshold?

> This cannot be implemented by returning zero from
> prepare_write_buffer(), as the generic write implementation expects the
> callback to return a transfer length once data is available in the write
> FIFO. Add a mxuport-specific write path so that URB submission can be
> stopped and resumed explicitly.
> 
> This may reduce throughput, but avoids data errors under sustained
> transmit load.
> 
> Signed-off-by: Crescent Hsieh <crescentcy.hsieh@moxa.com>
> ---
>  drivers/usb/serial/mxuport.c | 165 +++++++++++++++++++++++++++++++++--
>  1 file changed, 156 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/usb/serial/mxuport.c b/drivers/usb/serial/mxuport.c
> index 067c8d9752e0..f3be2b3bd95b 100644
> --- a/drivers/usb/serial/mxuport.c
> +++ b/drivers/usb/serial/mxuport.c
> @@ -147,6 +147,8 @@ static const u16 mxuport_mux50u_fw_ver_offsets[] = {
>  #define UPORT_EVENT_LSR			4 /* Line status */
>  #define UPORT_EVENT_MCR			5 /* Modem control */
>  
> +#define UPORT_REQUEST_SEND_NEXT		0x80
> +
>  /* Definitions for serial event type */
>  #define SERIAL_EV_CTS			0x0008	/* CTS changed state */
>  #define SERIAL_EV_DSR			0x0010	/* DSR changed state */
> @@ -193,6 +195,8 @@ static const u16 mxuport_mux50u_fw_ver_offsets[] = {
>  
>  /* This structure holds all of the local port information */
>  struct mxuport_port {
> +	u32 sent_payload;
> +	u8 hold_reason;
>  	u8 mcr_state;		/* Last MCR state */
>  	u8 msr_state;		/* Last MSR state */
>  	struct mutex mutex;	/* Protects mcr_state */
> @@ -276,22 +280,148 @@ MODULE_DEVICE_TABLE(usb, mxuport_idtable);
>  static int mxuport_prepare_write_buffer(struct usb_serial_port *port,
>  					void *dest, size_t size)
>  {
> +	struct mxuport_port *mxport = usb_get_serial_port_data(port);
>  	u8 *buf = dest;
> +	unsigned long flags;
> +	bool request_send_next;
>  	int count;
>  
> -	count = kfifo_out_locked(&port->write_fifo, buf + HEADER_SIZE,
> -				 size - HEADER_SIZE,
> -				 &port->lock);
> +	spin_lock_irqsave(&port->lock, flags);
> +	count = kfifo_out(&port->write_fifo, buf + HEADER_SIZE,
> +			  size - HEADER_SIZE);
> +	mxport->sent_payload += count;
> +	request_send_next = mxport->sent_payload >= port->bulk_out_size;

How big are the per-port buffers?

> +	if (request_send_next)
> +		mxport->hold_reason |= MX_WAIT_FOR_SEND_NEXT;
> +	spin_unlock_irqrestore(&port->lock, flags);
>  
>  	put_unaligned_be16(port->port_number, buf);
>  	put_unaligned_be16(count, buf + 2);
>  
> +	if (request_send_next)
> +		buf[0] |= UPORT_REQUEST_SEND_NEXT;
> +
>  	dev_dbg(&port->dev, "%s - size %zd count %d\n", __func__,
>  		size, count);
>  
>  	return count + HEADER_SIZE;
>  }
>  
> +static int mxuport_write_start(struct usb_serial_port *port, gfp_t mem_flags)
> +{
> +	struct mxuport_port *mxport = usb_get_serial_port_data(port);
> +	struct urb *urb;
> +	unsigned long flags;
> +	int i;
> +	int count;
> +	int result;
> +
> +	if (test_and_set_bit_lock(USB_SERIAL_WRITE_BUSY, &port->flags))
> +		return 0;
> +retry:
> +	spin_lock_irqsave(&port->lock, flags);
> +	if ((mxport->hold_reason & MX_WAIT_FOR_SEND_NEXT) ||
> +	    !port->write_urbs_free || !kfifo_len(&port->write_fifo)) {
> +		clear_bit_unlock(USB_SERIAL_WRITE_BUSY, &port->flags);
> +		spin_unlock_irqrestore(&port->lock, flags);
> +		return 0;
> +	}
> +
> +	i = (int)find_first_bit(&port->write_urbs_free,
> +				ARRAY_SIZE(port->write_urbs));
> +	spin_unlock_irqrestore(&port->lock, flags);
> +
> +	urb = port->write_urbs[i];
> +	count = mxuport_prepare_write_buffer(port, urb->transfer_buffer,
> +					     port->bulk_out_size);
> +	urb->transfer_buffer_length = count;
> +	usb_serial_debug_data(&port->dev, __func__, count,
> +			      urb->transfer_buffer);
> +
> +	spin_lock_irqsave(&port->lock, flags);
> +	port->tx_bytes += count;
> +	spin_unlock_irqrestore(&port->lock, flags);
> +
> +	clear_bit(i, &port->write_urbs_free);
> +	result = usb_submit_urb(urb, mem_flags);
> +	if (result) {
> +		dev_err_console(port, "%s - error submitting urb: %d\n",
> +				__func__, result);
> +		set_bit(i, &port->write_urbs_free);
> +		spin_lock_irqsave(&port->lock, flags);
> +		port->tx_bytes -= count;

> +		if (mxport->hold_reason & MX_WAIT_FOR_SEND_NEXT) {
> +			mxport->hold_reason &= ~MX_WAIT_FOR_SEND_NEXT;
> +			mxport->sent_payload = 0;
> +		}

Shouldn't you undo the effects of prepare_write_buffer() and subtract
count from sent_payload (and clear the flag) unconditionally?

> +		spin_unlock_irqrestore(&port->lock, flags);
> +
> +		clear_bit_unlock(USB_SERIAL_WRITE_BUSY, &port->flags);
> +		return result;
> +	}
> +
> +	goto retry;
> +}

This is a more or less verbatim copy of the generic write
implementation. If we go this way you should at least mention that you
copied it in the commit message, but perhaps we should try to find a way
to generalise it instead.

Also, if you really need a custom implementation to throttle writes,
then shouldn't using one URB be enough? The other one is essentially
there to allow for higher throughput which we need to give up for
correctness here anyway.

> +
> +static int mxuport_write(struct tty_struct *tty, struct usb_serial_port *port,
> +			 const unsigned char *buf, int count)
> +{
> +	int result;
> +
> +	if (!port->bulk_out_size)
> +		return -ENODEV;

This is redundant in a custom implementation.

> +
> +	if (!count)
> +		return 0;
> +
> +	count = kfifo_in_locked(&port->write_fifo, buf, count, &port->lock);
> +	result = mxuport_write_start(port, GFP_ATOMIC);
> +	if (result)
> +		return result;
> +
> +	return count;
> +}

Johan