* [PATCH v2 1/4] USB: serial: mxuport: clean up firmware version handling
2026-06-23 8:01 [PATCH v2 0/4] USB: serial: mxuport: add MUX50U support and updates Crescent Hsieh
@ 2026-06-23 8:01 ` Crescent Hsieh
2026-07-21 14:34 ` Johan Hovold
2026-06-23 8:01 ` [PATCH v2 2/4] USB: serial: mxuport: add MUX50U-based device support Crescent Hsieh
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: Crescent Hsieh @ 2026-06-23 8:01 UTC (permalink / raw)
To: Johan Hovold, Greg Kroah-Hartman
Cc: linux-usb, linux-kernel, FangpingFP.Cheng, Epson.Chiang,
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
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v2 1/4] USB: serial: mxuport: clean up firmware version handling
2026-06-23 8:01 ` [PATCH v2 1/4] USB: serial: mxuport: clean up firmware version handling Crescent Hsieh
@ 2026-07-21 14:34 ` Johan Hovold
0 siblings, 0 replies; 8+ messages in thread
From: Johan Hovold @ 2026-07-21 14:34 UTC (permalink / raw)
To: Crescent Hsieh
Cc: Greg Kroah-Hartman, linux-usb, linux-kernel, FangpingFP.Cheng,
Epson.Chiang
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 2/4] USB: serial: mxuport: add MUX50U-based device support
2026-06-23 8:01 [PATCH v2 0/4] USB: serial: mxuport: add MUX50U support and updates Crescent Hsieh
2026-06-23 8:01 ` [PATCH v2 1/4] USB: serial: mxuport: clean up firmware version handling Crescent Hsieh
@ 2026-06-23 8:01 ` Crescent Hsieh
2026-07-21 14:59 ` Johan Hovold
2026-06-23 8:01 ` [PATCH v2 3/4] USB: serial: mxuport: handle SEND_NEXT transmit flow control Crescent Hsieh
2026-06-23 8:01 ` [PATCH v2 4/4] USB: serial: mxuport: support RS485 mode configuration Crescent Hsieh
3 siblings, 1 reply; 8+ messages in thread
From: Crescent Hsieh @ 2026-06-23 8:01 UTC (permalink / raw)
To: Johan Hovold, Greg Kroah-Hartman
Cc: linux-usb, linux-kernel, FangpingFP.Cheng, Epson.Chiang,
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
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v2 2/4] USB: serial: mxuport: add MUX50U-based device support
2026-06-23 8:01 ` [PATCH v2 2/4] USB: serial: mxuport: add MUX50U-based device support Crescent Hsieh
@ 2026-07-21 14:59 ` Johan Hovold
0 siblings, 0 replies; 8+ messages in thread
From: Johan Hovold @ 2026-07-21 14:59 UTC (permalink / raw)
To: Crescent Hsieh
Cc: Greg Kroah-Hartman, linux-usb, linux-kernel, FangpingFP.Cheng,
Epson.Chiang
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 3/4] USB: serial: mxuport: handle SEND_NEXT transmit flow control
2026-06-23 8:01 [PATCH v2 0/4] USB: serial: mxuport: add MUX50U support and updates Crescent Hsieh
2026-06-23 8:01 ` [PATCH v2 1/4] USB: serial: mxuport: clean up firmware version handling Crescent Hsieh
2026-06-23 8:01 ` [PATCH v2 2/4] USB: serial: mxuport: add MUX50U-based device support Crescent Hsieh
@ 2026-06-23 8:01 ` Crescent Hsieh
2026-07-21 15:51 ` Johan Hovold
2026-06-23 8:01 ` [PATCH v2 4/4] USB: serial: mxuport: support RS485 mode configuration Crescent Hsieh
3 siblings, 1 reply; 8+ messages in thread
From: Crescent Hsieh @ 2026-06-23 8:01 UTC (permalink / raw)
To: Johan Hovold, Greg Kroah-Hartman
Cc: linux-usb, linux-kernel, FangpingFP.Cheng, Epson.Chiang,
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
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v2 3/4] USB: serial: mxuport: handle SEND_NEXT transmit flow control
2026-06-23 8:01 ` [PATCH v2 3/4] USB: serial: mxuport: handle SEND_NEXT transmit flow control Crescent Hsieh
@ 2026-07-21 15:51 ` Johan Hovold
0 siblings, 0 replies; 8+ messages in thread
From: Johan Hovold @ 2026-07-21 15:51 UTC (permalink / raw)
To: Crescent Hsieh
Cc: Greg Kroah-Hartman, linux-usb, linux-kernel, FangpingFP.Cheng,
Epson.Chiang
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 4/4] USB: serial: mxuport: support RS485 mode configuration
2026-06-23 8:01 [PATCH v2 0/4] USB: serial: mxuport: add MUX50U support and updates Crescent Hsieh
` (2 preceding siblings ...)
2026-06-23 8:01 ` [PATCH v2 3/4] USB: serial: mxuport: handle SEND_NEXT transmit flow control Crescent Hsieh
@ 2026-06-23 8:01 ` Crescent Hsieh
3 siblings, 0 replies; 8+ messages in thread
From: Crescent Hsieh @ 2026-06-23 8:01 UTC (permalink / raw)
To: Johan Hovold, Greg Kroah-Hartman
Cc: linux-usb, linux-kernel, FangpingFP.Cheng, Epson.Chiang,
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
^ permalink raw reply related [flat|nested] 8+ messages in thread