[PATCH v1 0/4] usb: serial: mxuport: extend MXU50U support and runtime controls

[PATCH v1 0/4] usb: serial: mxuport: extend MXU50U support and runtime controls

[Crescent Hsieh]

This series extends the mxuport driver for additional MXU50U-based devices
and adds runtime configuration support for interface mode and UART FIFO
control.

The first patch adds support for additional 3-, 5-, 6-, and 7-port MXU50U
devices and updates the firmware selection logic accordingly.

The second patch implements TX flow control based on the device SEND_NEXT
event so the driver stops queueing bulk-out data once the device buffer is
full and resumes when the device is ready again.

The third patch adds serial interface mode configuration through
TIOCSRS485/TIOCGRS485, allowing userspace to switch between RS232, RS422,
2-wire RS485, and 4-wire RS485 modes.

The final patch adds a per-port sysfs attribute for controlling the UART
FIFO state at runtime.

Together these changes improve hardware coverage and expose the runtime
controls supported by the device firmware.

Crescent Hsieh (4):
  usb: serial: mxuport: add support for more MXU50U UART devices
  usb: serial: mxuport: handle SEND_NEXT tx flow control
  usb: serial: mxuport: support serial interface mode configuration
  usb: serial: mxuport: add sysfs control for UART FIFO

 drivers/usb/serial/mxuport.c | 319 +++++++++++++++++++++++++++++++++--
 1 file changed, 305 insertions(+), 14 deletions(-)

-- 
2.43.0

[PATCH v1 1/4] usb: serial: mxuport: add support for more MXU50U UART devices

[Crescent Hsieh]

Add support for additional Moxa devices using the MXU50U UART family.

Extend the device ID table and port-count handling for 3-, 5-, 6- and
7-port devices, and update firmware selection to use the matching
MXU50U firmware images and version offsets.

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

diff --git a/drivers/usb/serial/mxuport.c b/drivers/usb/serial/mxuport.c
index ad5fdf55a02e..034b506322c2 100644
--- a/drivers/usb/serial/mxuport.c
+++ b/drivers/usb/serial/mxuport.c
@@ -39,6 +39,25 @@
 #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
@@ -48,6 +67,9 @@
 #define VER_ADDR_1		0x20
 #define VER_ADDR_2		0x24
 #define VER_ADDR_3		0x28
+#define NEW_ADDR_1		0x86
+#define NEW_ADDR_2		0x88
+#define NEW_ADDR_3		0x8A
 
 /* Definitions for USB vendor request */
 #define RQ_VENDOR_NONE			0x00
@@ -147,9 +169,13 @@
 #define MX_WAIT_FOR_SEND_NEXT		0x0080
 
 #define MX_UPORT_2_PORT			BIT(0)
-#define MX_UPORT_4_PORT			BIT(1)
-#define MX_UPORT_8_PORT			BIT(2)
-#define MX_UPORT_16_PORT		BIT(3)
+#define MX_UPORT_3_PORT			BIT(1)
+#define MX_UPORT_4_PORT			BIT(2)
+#define MX_UPORT_5_PORT			BIT(3)
+#define MX_UPORT_6_PORT			BIT(4)
+#define MX_UPORT_7_PORT			BIT(5)
+#define MX_UPORT_8_PORT			BIT(6)
+#define MX_UPORT_16_PORT		BIT(7)
 
 /* This structure holds all of the local port information */
 struct mxuport_port {
@@ -179,7 +205,39 @@ static const struct usb_device_id mxuport_idtable[] = {
 	  .driver_info = MX_UPORT_16_PORT },
 	{ USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1653_PID),
 	  .driver_info = MX_UPORT_16_PORT },
-	{}			/* Terminating entry */
+	{ USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1252_PID),
+	  .driver_info = MX_UPORT_2_PORT },
+	{ USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1253_PID),
+	  .driver_info = MX_UPORT_2_PORT },
+	{ USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1411_PID),
+	  .driver_info = MX_UPORT_4_PORT },
+	{ USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1452_PID),
+	  .driver_info = MX_UPORT_4_PORT },
+	{ USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1453_PID),
+	  .driver_info = MX_UPORT_4_PORT },
+	{ USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1619_PID),
+	  .driver_info = MX_UPORT_8_PORT },
+	{ USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1659_PID),
+	  .driver_info = MX_UPORT_8_PORT },
+	{ USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT165A_PID),
+	  .driver_info = MX_UPORT_8_PORT },
+	{ USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT165B_PID),
+	  .driver_info = MX_UPORT_8_PORT },
+	{ USB_DEVICE(MX_USBSERIAL_VID, MX_MU250U_PID),
+	  .driver_info = MX_UPORT_2_PORT },
+	{ USB_DEVICE(MX_USBSERIAL_VID, MX_MU450U_PID),
+	  .driver_info = MX_UPORT_4_PORT },
+	{ USB_DEVICE(MX_USBSERIAL_VID, MX_MU850U_PID),
+	  .driver_info = MX_UPORT_8_PORT },
+	{ USB_DEVICE(MX_USBSERIAL_VID, MX_MU850U_6PORT_PID),
+	  .driver_info = MX_UPORT_6_PORT },
+	{ USB_DEVICE(MX_USBSERIAL_VID, MX_MUX50U_3PORT_PID),
+	  .driver_info = MX_UPORT_3_PORT },
+	{ USB_DEVICE(MX_USBSERIAL_VID, MX_MU850U_5PORT_PID),
+	  .driver_info = MX_UPORT_5_PORT },
+	{ USB_DEVICE(MX_USBSERIAL_VID, MX_MU850U_7PORT_PID),
+	  .driver_info = MX_UPORT_7_PORT },
+	{} /* Terminating entry */
 };
 
 MODULE_DEVICE_TABLE(usb, mxuport_idtable);
@@ -944,8 +1002,16 @@ static int mxuport_calc_num_ports(struct usb_serial *serial,
 
 	if (features & MX_UPORT_2_PORT) {
 		num_ports = 2;
+	} else if (features & MX_UPORT_3_PORT) {
+		num_ports = 3;
 	} else if (features & MX_UPORT_4_PORT) {
 		num_ports = 4;
+	} else if (features & MX_UPORT_5_PORT) {
+		num_ports = 5;
+	} else if (features & MX_UPORT_6_PORT) {
+		num_ports = 6;
+	} else if (features & MX_UPORT_7_PORT) {
+		num_ports = 7;
 	} else if (features & MX_UPORT_8_PORT) {
 		num_ports = 8;
 	} else if (features & MX_UPORT_16_PORT) {
@@ -1053,6 +1119,7 @@ static int mxuport_probe(struct usb_serial *serial,
 	int local_ver;
 	char buf[32];
 	int err;
+	bool is_mux50u = false;
 
 	/* Load our firmware */
 	err = mxuport_send_ctrl_urb(serial, RQ_VENDOR_QUERY_FW_CONFIG, 0, 0);
@@ -1065,12 +1132,41 @@ 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",
+	dev_dbg(&serial->interface->dev, "Device firmware version v%d.%d.%d\n",
 		(version & 0xff0000) >> 16,
 		(version & 0xff00) >> 8,
 		(version & 0xff));
 
-	snprintf(buf, sizeof(buf) - 1, "moxa/moxa-%04x.fw", productid);
+	switch (productid) {
+	case MX_UPORT1252_PID:
+	case MX_UPORT1253_PID:
+	case MX_UPORT1411_PID:
+	case MX_UPORT1452_PID:
+	case MX_UPORT1453_PID:
+	case MX_UPORT1619_PID:
+	case MX_UPORT1659_PID:
+	case MX_UPORT165A_PID:
+	case MX_UPORT165B_PID:
+		is_mux50u = true;
+		snprintf(buf, sizeof(buf) - 1, "moxa/moxa-up-mux50u.fw");
+
+		break;
+	case MX_MU250U_PID:
+	case MX_MU450U_PID:
+	case MX_MU850U_PID:
+	case MX_MU850U_6PORT_PID:
+	case MX_MUX50U_3PORT_PID:
+	case MX_MU850U_5PORT_PID:
+	case MX_MU850U_7PORT_PID:
+		is_mux50u = true;
+		snprintf(buf, sizeof(buf) - 1, "moxa/moxa-pf-mux50u.fw");
+
+		break;
+	default:
+		snprintf(buf, sizeof(buf) - 1, "moxa/moxa-%04x.fw", productid);
+
+		break;
+	}
 
 	err = request_firmware(&fw_p, buf, &serial->interface->dev);
 	if (err) {
@@ -1080,14 +1176,22 @@ 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]);
+		if (is_mux50u) {
+			local_ver = ((fw_p->data[NEW_ADDR_1] << 16) |
+				     (fw_p->data[NEW_ADDR_2] << 8) |
+				     (fw_p->data[NEW_ADDR_3]));
+		} else {
+			local_ver = ((fw_p->data[VER_ADDR_1] << 16) |
+				     (fw_p->data[VER_ADDR_2] << 8) |
+				     (fw_p->data[VER_ADDR_3]));
+		}
 		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%d.%d.%d\n",
+			(local_ver & 0xff0000) >> 16,
+			(local_ver & 0xff00) >> 8,
+			(local_ver & 0xff));
+
+		if (local_ver != version) {
 			err = mxuport_download_fw(serial, fw_p);
 			if (err)
 				goto out;
@@ -1098,7 +1202,7 @@ static int mxuport_probe(struct usb_serial *serial,
 	}
 
 	dev_info(&serial->interface->dev,
-		 "Using device firmware version v%x.%x.%x\n",
+		 "Using device firmware version v%d.%d.%d\n",
 		 (version & 0xff0000) >> 16,
 		 (version & 0xff00) >> 8,
 		 (version & 0xff));
-- 
2.43.0

[PATCH v1 2/4] usb: serial: mxuport: handle SEND_NEXT tx flow control

[Crescent Hsieh]

Track the transmitted payload size per port and stop queueing more data
once a bulk-out transfer reaches the device buffer threshold.

Resume transmission when the device reports UPORT_EVENT_SEND_NEXT, and
reset the TX flow-control state when the port is opened.

This prevents the driver from queueing more TX data until the device
reports that it is ready to accept the next transfer.

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

diff --git a/drivers/usb/serial/mxuport.c b/drivers/usb/serial/mxuport.c
index 034b506322c2..4d29a431cefd 100644
--- a/drivers/usb/serial/mxuport.c
+++ b/drivers/usb/serial/mxuport.c
@@ -179,6 +179,8 @@
 
 /* This structure holds all of the local port information */
 struct mxuport_port {
+	u32 sent_payload;
+	u32 hold_reason;
 	u8 mcr_state;		/* Last MCR state */
 	u8 msr_state;		/* Last MSR state */
 	struct mutex mutex;	/* Protects mcr_state */
@@ -250,9 +252,13 @@ 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;
 	int count;
 
+	if (mxport->hold_reason & MX_WAIT_FOR_SEND_NEXT)
+		return 0;
+
 	count = kfifo_out_locked(&port->write_fifo, buf + HEADER_SIZE,
 				 size - HEADER_SIZE,
 				 &port->lock);
@@ -263,6 +269,13 @@ static int mxuport_prepare_write_buffer(struct usb_serial_port *port,
 	dev_dbg(&port->dev, "%s - size %zd count %d\n", __func__,
 		size, count);
 
+	mxport->sent_payload += count;
+
+	if (mxport->sent_payload >= port->bulk_out_size) {
+		mxport->hold_reason |= MX_WAIT_FOR_SEND_NEXT;
+		buf[0] |= 0x80;
+	}
+
 	return count + HEADER_SIZE;
 }
 
@@ -484,6 +497,9 @@ 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;
+
 	dev_dbg(&port->dev, "%s - receive event : %04x\n", __func__, event);
 
 	switch (event) {
@@ -492,6 +508,13 @@ static void mxuport_process_read_urb_event(struct usb_serial_port *port,
 		 * Sent as part of the flow control on device buffers.
 		 * Not currently used.
 		 */
+		if (mxport->hold_reason & MX_WAIT_FOR_SEND_NEXT) {
+			spin_lock_irqsave(&mxport->spinlock, flags);
+			mxport->hold_reason &= ~MX_WAIT_FOR_SEND_NEXT;
+			mxport->sent_payload = 0;
+			usb_serial_generic_write_start(port, GFP_ATOMIC);
+			spin_unlock_irqrestore(&mxport->spinlock, flags);
+		}
 		break;
 	case UPORT_EVENT_MSR:
 		mxuport_msr_event(port, buf);
@@ -1318,6 +1341,9 @@ static int mxuport_open(struct tty_struct *tty, struct usb_serial_port *port)
 	 * returns.
 	 */
 	mxport->msr_state = 0;
+	mxport->sent_payload = 0;
+	mxport->hold_reason = 0;
+	kfifo_reset(&port->write_fifo);
 
 	return err;
 }
-- 
2.43.0

[PATCH v1 3/4] usb: serial: mxuport: support serial interface mode configuration

[Crescent Hsieh]

Add support for configuring the serial interface mode through
TIOCSRS485 and TIOCGRS485 using struct serial_rs485.

Sanitize the requested RS-485 settings and map them to the device
interface modes before issuing the vendor command to the firmware.

This allows userspace to switch between RS232, RS422, 2-wire RS485,
and 4-wire RS485, and to query the current per-port configuration.

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

diff --git a/drivers/usb/serial/mxuport.c b/drivers/usb/serial/mxuport.c
index 4d29a431cefd..9a8bb4f02da3 100644
--- a/drivers/usb/serial/mxuport.c
+++ b/drivers/usb/serial/mxuport.c
@@ -183,6 +183,7 @@ struct mxuport_port {
 	u32 hold_reason;
 	u8 mcr_state;		/* Last MCR state */
 	u8 msr_state;		/* Last MSR state */
+	struct serial_rs485 rs485;
 	struct mutex mutex;	/* Protects mcr_state */
 	spinlock_t spinlock;	/* Protects msr_state */
 };
@@ -1348,6 +1349,90 @@ static int mxuport_open(struct tty_struct *tty, struct usb_serial_port *port)
 	return err;
 }
 
+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);
+		return;
+	}
+	rs485->flags &= (SER_RS485_ENABLED | SER_RS485_RX_DURING_TX);
+
+	memset(rs485->padding, 0, sizeof(rs485->padding));
+}
+
+static int mxuport_rs485_config(struct usb_serial_port *port,
+				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 *rs485)
+{
+	struct mxuport_port *mxport = usb_get_serial_port_data(port);
+
+	if (copy_to_user(rs485, &mxport->rs485, sizeof(mxport->rs485)))
+		return -EFAULT;
+
+	return 0;
+}
+
+static int mxuport_set_rs485_config(struct usb_serial_port *port,
+				    struct serial_rs485 __user *rs485_user)
+{
+	struct mxuport_port *mxport = usb_get_serial_port_data(port);
+	struct serial_rs485 rs485;
+	int ret;
+
+	if (copy_from_user(&rs485, rs485_user, sizeof(*rs485_user)))
+		return -EFAULT;
+
+	mxuport_sanitize_serial_rs485(&rs485);
+
+	ret = mxuport_rs485_config(port, &rs485);
+	if (!ret)
+		mxport->rs485 = rs485;
+
+	if (copy_to_user(rs485_user, &mxport->rs485, sizeof(mxport->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 *uarg = (void __user *)arg;
+
+	switch (cmd) {
+	case TIOCGRS485:
+		return mxuport_get_rs485_config(port, uarg);
+	case TIOCSRS485:
+		return mxuport_set_rs485_config(port, uarg);
+	}
+
+	return -ENOIOCTLCMD;
+}
+
 static void mxuport_close(struct usb_serial_port *port)
 {
 	struct usb_serial *serial = port->serial;
@@ -1421,6 +1506,7 @@ static struct usb_serial_driver mxuport_device = {
 	.calc_num_ports		= mxuport_calc_num_ports,
 	.open			= mxuport_open,
 	.close			= mxuport_close,
+	.ioctl			= mxuport_ioctl,
 	.set_termios		= mxuport_set_termios,
 	.break_ctl		= mxuport_break_ctl,
 	.tx_empty		= mxuport_tx_empty,
-- 
2.43.0

[PATCH v1 4/4] usb: serial: mxuport: add sysfs control for UART FIFO

[Crescent Hsieh]

Add a per-port sysfs attribute, uart_fifo, to allow userspace to enable
or disable the UART FIFO at runtime.

Map the requested state to the RQ_VENDOR_SET_FIFO_DISABLE vendor command
and track the current FIFO setting in the per-port private data.

Initialize the FIFO state during port probe and remove the sysfs
attribute when the port is released.

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

diff --git a/drivers/usb/serial/mxuport.c b/drivers/usb/serial/mxuport.c
index 9a8bb4f02da3..24b86d1a31d4 100644
--- a/drivers/usb/serial/mxuport.c
+++ b/drivers/usb/serial/mxuport.c
@@ -183,6 +183,7 @@ struct mxuport_port {
 	u32 hold_reason;
 	u8 mcr_state;		/* Last MCR state */
 	u8 msr_state;		/* Last MSR state */
+	bool fifo_enabled;
 	struct serial_rs485 rs485;
 	struct mutex mutex;	/* Protects mcr_state */
 	spinlock_t spinlock;	/* Protects msr_state */
@@ -1242,6 +1243,68 @@ static int mxuport_probe(struct usb_serial *serial,
 	return err;
 }
 
+static ssize_t uart_fifo_show(struct device *dev, struct device_attribute *attr,
+			      char *buf)
+{
+	struct usb_serial_port *port = to_usb_serial_port(dev);
+	struct mxuport_port *mxport = usb_get_serial_port_data(port);
+
+	if (mxport->fifo_enabled)
+		return sysfs_emit(buf, "enabled\n");
+
+	return sysfs_emit(buf, "disabled\n");
+}
+
+static ssize_t uart_fifo_store(struct device *dev,
+			       struct device_attribute *attr,
+			       const char *buf, size_t count)
+{
+	struct usb_serial_port *port = to_usb_serial_port(dev);
+	struct mxuport_port *mxport = usb_get_serial_port_data(port);
+	struct usb_serial *serial = port->serial;
+	bool state;
+	int ret;
+
+	if (!count)
+		return -EINVAL;
+
+	ret = kstrtobool(buf, &state);
+	if (ret < 0)
+		return ret;
+
+	if (state == 1) {
+		ret = mxuport_send_ctrl_urb(serial, RQ_VENDOR_SET_FIFO_DISABLE,
+					    0, port->port_number);
+		if (ret)
+			return ret;
+
+		mxport->fifo_enabled = true;
+	} else if (state == 0) {
+		ret = mxuport_send_ctrl_urb(serial, RQ_VENDOR_SET_FIFO_DISABLE,
+					    1, port->port_number);
+		if (ret)
+			return ret;
+
+		mxport->fifo_enabled = false;
+	} else {
+		return -EINVAL;
+	}
+
+	return count;
+}
+static DEVICE_ATTR_RW(uart_fifo);
+
+static int mxuport_create_sysfs_attrs(struct usb_serial_port *port)
+{
+	return device_create_file(&port->dev, &dev_attr_uart_fifo);
+}
+
+static int mxuport_remove_sysfs_attrs(struct usb_serial_port *port)
+{
+	device_remove_file(&port->dev, &dev_attr_uart_fifo);
+
+	return 0;
+}
 
 static int mxuport_port_probe(struct usb_serial_port *port)
 {
@@ -1266,18 +1329,29 @@ static int mxuport_port_probe(struct usb_serial_port *port)
 	if (err)
 		return err;
 
+	mxport->fifo_enabled = true;
+
 	/* Set transmission mode (Hi-Performance) */
 	err = mxuport_send_ctrl_urb(serial, RQ_VENDOR_SET_HIGH_PERFOR,
 				    0, port->port_number);
 	if (err)
 		return err;
 
+	err = mxuport_create_sysfs_attrs(port);
+	if (err)
+		return err;
+
 	/* Set interface (RS-232) */
 	return mxuport_send_ctrl_urb(serial, RQ_VENDOR_SET_INTERFACE,
 				     MX_INT_RS232,
 				     port->port_number);
 }
 
+static void mxuport_port_remove(struct usb_serial_port *port)
+{
+	mxuport_remove_sysfs_attrs(port);
+}
+
 static int mxuport_attach(struct usb_serial *serial)
 {
 	struct usb_serial_port *port0 = serial->port[0];
@@ -1501,6 +1575,7 @@ static struct usb_serial_driver mxuport_device = {
 	.num_bulk_out		= 1,
 	.probe			= mxuport_probe,
 	.port_probe		= mxuport_port_probe,
+	.port_remove		= mxuport_port_remove,
 	.attach			= mxuport_attach,
 	.release		= mxuport_release,
 	.calc_num_ports		= mxuport_calc_num_ports,
-- 
2.43.0

Re: [PATCH v1 0/4] usb: serial: mxuport: extend MXU50U support and runtime controls

[Crescent Hsieh]

Hi,

Gentle ping on this series.
Please let me know if I should revise or split anything.

Related firmware patch:
https://lore.kernel.org/all/20260324024635.349522-1-crescentcy.hsieh@moxa.com/

Thanks,
Crescent Hsieh

Re: [PATCH v1 0/4] usb: serial: mxuport: extend MXU50U support and runtime controls

[Johan Hovold]

On Mon, Mar 30, 2026 at 03:24:06PM +0800, Crescent Hsieh wrote:

> Gentle ping on this series.
> Please let me know if I should revise or split anything.

This one was posted less than a week ago and is still in my queue. I'll
get to reviewing it in time, but you should generally wait at least a
couple of weeks before sending reminders.

> Related firmware patch:
> https://lore.kernel.org/all/20260324024635.349522-1-crescentcy.hsieh@moxa.com/

That's great that you've already pushed the firmware.

Johan

Re: [PATCH v1 0/4] usb: serial: mxuport: extend MXU50U support and runtime controls

[Crescent Hsieh]

Hi Johan,

Gentle ping on this series.

It's been about a month since v1 was posted, so I just wanted to check
whether you've had a chance to review it, or if I should resend or
split anything.

Thanks for your time.

Thanks,
Crescent Hsieh

Re: [PATCH v1 0/4] usb: serial: mxuport: extend MXU50U support and runtime controls

[Johan Hovold]

On Mon, Apr 27, 2026 at 11:37:59AM +0800, Crescent Hsieh wrote:

> Gentle ping on this series.
> 
> It's been about a month since v1 was posted, so I just wanted to check
> whether you've had a chance to review it, or if I should resend or
> split anything.

The merge window just closed so I'll start processing my queue again
shortly.

Johan

Re: [PATCH v1 1/4] usb: serial: mxuport: add support for more MXU50U UART devices

[Johan Hovold]

On Tue, Mar 24, 2026 at 11:50:38AM +0800, Crescent Hsieh wrote:
> Add support for additional Moxa devices using the MXU50U UART family.

Please mention which models since its not obvious from the PID defines
which mostly reflects the PID itself.

You should amend the driver header as well which lists the currently
supported models.

> Extend the device ID table and port-count handling for 3-, 5-, 6- and
> 7-port devices, and update firmware selection to use the matching
> MXU50U firmware images and version offsets.
> 
> Signed-off-by: Crescent Hsieh <crescentcy.hsieh@moxa.com>
> ---
>  drivers/usb/serial/mxuport.c | 132 +++++++++++++++++++++++++++++++----
>  1 file changed, 118 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/usb/serial/mxuport.c b/drivers/usb/serial/mxuport.c
> index ad5fdf55a02e..034b506322c2 100644
> --- a/drivers/usb/serial/mxuport.c
> +++ b/drivers/usb/serial/mxuport.c
> @@ -39,6 +39,25 @@
>  #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
> @@ -48,6 +67,9 @@
>  #define VER_ADDR_1		0x20
>  #define VER_ADDR_2		0x24
>  #define VER_ADDR_3		0x28
> +#define NEW_ADDR_1		0x86
> +#define NEW_ADDR_2		0x88
> +#define NEW_ADDR_3		0x8A

These are still offsets for the version so please include that in the
name. And "new" tends to get old, better to include the family name, so
something like:

	MXU50U_VER_ADDR_n

>  /* Definitions for USB vendor request */
>  #define RQ_VENDOR_NONE			0x00
> @@ -147,9 +169,13 @@
>  #define MX_WAIT_FOR_SEND_NEXT		0x0080
>  
>  #define MX_UPORT_2_PORT			BIT(0)
> -#define MX_UPORT_4_PORT			BIT(1)
> -#define MX_UPORT_8_PORT			BIT(2)
> -#define MX_UPORT_16_PORT		BIT(3)
> +#define MX_UPORT_3_PORT			BIT(1)
> +#define MX_UPORT_4_PORT			BIT(2)
> +#define MX_UPORT_5_PORT			BIT(3)
> +#define MX_UPORT_6_PORT			BIT(4)
> +#define MX_UPORT_7_PORT			BIT(5)
> +#define MX_UPORT_8_PORT			BIT(6)
> +#define MX_UPORT_16_PORT		BIT(7)

We don't need 8 bits to encode up to 16 ports. Please rebase this on the
patch I just submitted here which cleans this up:

	https://lore.kernel.org/r/20260507141114.284470-1-johan@kernel.org

> @@ -1053,6 +1119,7 @@ static int mxuport_probe(struct usb_serial *serial,
>  	int local_ver;
>  	char buf[32];
>  	int err;
> +	bool is_mux50u = false;
>  
>  	/* Load our firmware */
>  	err = mxuport_send_ctrl_urb(serial, RQ_VENDOR_QUERY_FW_CONFIG, 0, 0);
> @@ -1065,12 +1132,41 @@ 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",
> +	dev_dbg(&serial->interface->dev, "Device firmware version v%d.%d.%d\n",

This looks like an unrelated change. Please do this in preparatory patch
explaining why you prefer decimal notation.

>  		(version & 0xff0000) >> 16,
>  		(version & 0xff00) >> 8,
>  		(version & 0xff));
>  
> -	snprintf(buf, sizeof(buf) - 1, "moxa/moxa-%04x.fw", productid);
> +	switch (productid) {
> +	case MX_UPORT1252_PID:
> +	case MX_UPORT1253_PID:
> +	case MX_UPORT1411_PID:
> +	case MX_UPORT1452_PID:
> +	case MX_UPORT1453_PID:
> +	case MX_UPORT1619_PID:
> +	case MX_UPORT1659_PID:
> +	case MX_UPORT165A_PID:
> +	case MX_UPORT165B_PID:
> +		is_mux50u = true;
> +		snprintf(buf, sizeof(buf) - 1, "moxa/moxa-up-mux50u.fw");
> +
> +		break;
> +	case MX_MU250U_PID:
> +	case MX_MU450U_PID:
> +	case MX_MU850U_PID:
> +	case MX_MU850U_6PORT_PID:
> +	case MX_MUX50U_3PORT_PID:
> +	case MX_MU850U_5PORT_PID:
> +	case MX_MU850U_7PORT_PID:
> +		is_mux50u = true;
> +		snprintf(buf, sizeof(buf) - 1, "moxa/moxa-pf-mux50u.fw");

We shouldn't have to maintain two tables with PIDs so please encode this
information in the id-table (using two new flags) if you really can't
query the device about this.

We already request the running firmware version. That's not enough to
determine if this is a mux50u device? Perhaps there is some other way?
Can the descriptors be used (e.g. bcdUSB)?

What does "up" vs "pf" stand for? Please mention this in commit message
as well (i.e. the two subfamilies).

> +
> +		break;
> +	default:
> +		snprintf(buf, sizeof(buf) - 1, "moxa/moxa-%04x.fw", productid);
> +
> +		break;
> +	}
>  
>  	err = request_firmware(&fw_p, buf, &serial->interface->dev);
>  	if (err) {
> @@ -1080,14 +1176,22 @@ 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]);
> +		if (is_mux50u) {
> +			local_ver = ((fw_p->data[NEW_ADDR_1] << 16) |
> +				     (fw_p->data[NEW_ADDR_2] << 8) |
> +				     (fw_p->data[NEW_ADDR_3]));
> +		} else {
> +			local_ver = ((fw_p->data[VER_ADDR_1] << 16) |
> +				     (fw_p->data[VER_ADDR_2] << 8) |
> +				     (fw_p->data[VER_ADDR_3]));
> +		}

This is probably better handled in a helper function which can make sure
that the right offsets are used depending on the family. For example:

	mxuport_parse_fw_version(const struct firmware *fw, bool mux50u)

>  		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%d.%d.%d\n",
> +			(local_ver & 0xff0000) >> 16,
> +			(local_ver & 0xff00) >> 8,
> +			(local_ver & 0xff));
> +
> +		if (local_ver != version) {

This looks like a behaviour change that should be motivated. Why is it
OK to downgrade the device fw?

>  			err = mxuport_download_fw(serial, fw_p);
>  			if (err)
>  				goto out;
> @@ -1098,7 +1202,7 @@ static int mxuport_probe(struct usb_serial *serial,
>  	}
>  
>  	dev_info(&serial->interface->dev,
> -		 "Using device firmware version v%x.%x.%x\n",
> +		 "Using device firmware version v%d.%d.%d\n",
>  		 (version & 0xff0000) >> 16,
>  		 (version & 0xff00) >> 8,
>  		 (version & 0xff));

Johan

Re: [PATCH v1 2/4] usb: serial: mxuport: handle SEND_NEXT tx flow control

[Johan Hovold]

On Tue, Mar 24, 2026 at 11:50:39AM +0800, Crescent Hsieh wrote:
> Track the transmitted payload size per port and stop queueing more data
> once a bulk-out transfer reaches the device buffer threshold.
> 
> Resume transmission when the device reports UPORT_EVENT_SEND_NEXT, and
> reset the TX flow-control state when the port is opened.
> 
> This prevents the driver from queueing more TX data until the device
> reports that it is ready to accept the next transfer.

This explains what the patch does but says nothing about why you think
this is needed (which is the more important part).

The currently supported devices seems to work fine without this and
introducing this scheme should impact throughput negatively.

> Signed-off-by: Crescent Hsieh <crescentcy.hsieh@moxa.com>
> ---
>  drivers/usb/serial/mxuport.c | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/drivers/usb/serial/mxuport.c b/drivers/usb/serial/mxuport.c
> index 034b506322c2..4d29a431cefd 100644
> --- a/drivers/usb/serial/mxuport.c
> +++ b/drivers/usb/serial/mxuport.c
> @@ -179,6 +179,8 @@
>  
>  /* This structure holds all of the local port information */
>  struct mxuport_port {
> +	u32 sent_payload;
> +	u32 hold_reason;
>  	u8 mcr_state;		/* Last MCR state */
>  	u8 msr_state;		/* Last MSR state */
>  	struct mutex mutex;	/* Protects mcr_state */
> @@ -250,9 +252,13 @@ 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;
>  	int count;
>  
> +	if (mxport->hold_reason & MX_WAIT_FOR_SEND_NEXT)
> +		return 0;

The generic write implementation does not support drivers returning zero
here (and has already made sure that there is data in the fifo) so if
this is at all needed that may need to be addressed.

> +
>  	count = kfifo_out_locked(&port->write_fifo, buf + HEADER_SIZE,
>  				 size - HEADER_SIZE,
>  				 &port->lock);
> @@ -263,6 +269,13 @@ static int mxuport_prepare_write_buffer(struct usb_serial_port *port,
>  	dev_dbg(&port->dev, "%s - size %zd count %d\n", __func__,
>  		size, count);
>  
> +	mxport->sent_payload += count;
> +
> +	if (mxport->sent_payload >= port->bulk_out_size) {
> +		mxport->hold_reason |= MX_WAIT_FOR_SEND_NEXT;
> +		buf[0] |= 0x80;
> +	}
> +
>  	return count + HEADER_SIZE;
>  }
>  
> @@ -484,6 +497,9 @@ 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;
> +
>  	dev_dbg(&port->dev, "%s - receive event : %04x\n", __func__, event);
>  
>  	switch (event) {
> @@ -492,6 +508,13 @@ static void mxuport_process_read_urb_event(struct usb_serial_port *port,
>  		 * Sent as part of the flow control on device buffers.
>  		 * Not currently used.
>  		 */
> +		if (mxport->hold_reason & MX_WAIT_FOR_SEND_NEXT) {
> +			spin_lock_irqsave(&mxport->spinlock, flags);
> +			mxport->hold_reason &= ~MX_WAIT_FOR_SEND_NEXT;
> +			mxport->sent_payload = 0;
> +			usb_serial_generic_write_start(port, GFP_ATOMIC);
> +			spin_unlock_irqrestore(&mxport->spinlock, flags);
> +		}

The locking here looks questionable.

>  		break;
>  	case UPORT_EVENT_MSR:
>  		mxuport_msr_event(port, buf);
> @@ -1318,6 +1341,9 @@ static int mxuport_open(struct tty_struct *tty, struct usb_serial_port *port)
>  	 * returns.
>  	 */
>  	mxport->msr_state = 0;
> +	mxport->sent_payload = 0;
> +	mxport->hold_reason = 0;
> +	kfifo_reset(&port->write_fifo);

The fifo is already cleared on close.

>  
>  	return err;
>  }

Johan

Re: [PATCH v1 3/4] usb: serial: mxuport: support serial interface mode configuration

[Johan Hovold]

On Tue, Mar 24, 2026 at 11:50:40AM +0800, Crescent Hsieh wrote:
> Add support for configuring the serial interface mode through
> TIOCSRS485 and TIOCGRS485 using struct serial_rs485.
> 
> Sanitize the requested RS-485 settings and map them to the device
> interface modes before issuing the vendor command to the firmware.
> 
> This allows userspace to switch between RS232, RS422, 2-wire RS485,
> and 4-wire RS485, and to query the current per-port configuration.
> 
> Signed-off-by: Crescent Hsieh <crescentcy.hsieh@moxa.com>
 
> +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);
> +		return;
> +	}
> +	rs485->flags &= (SER_RS485_ENABLED | SER_RS485_RX_DURING_TX);

I think you need to clear the other unsupported fields as well (i.e. the
delays).

> +
> +	memset(rs485->padding, 0, sizeof(rs485->padding));
> +}
> +
> +static int mxuport_rs485_config(struct usb_serial_port *port,
> +				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 *rs485)
> +{
> +	struct mxuport_port *mxport = usb_get_serial_port_data(port);
> +
> +	if (copy_to_user(rs485, &mxport->rs485, sizeof(mxport->rs485)))
> +		return -EFAULT;
> +
> +	return 0;
> +}
> +
> +static int mxuport_set_rs485_config(struct usb_serial_port *port,
> +				    struct serial_rs485 __user *rs485_user)
> +{
> +	struct mxuport_port *mxport = usb_get_serial_port_data(port);
> +	struct serial_rs485 rs485;
> +	int ret;
> +
> +	if (copy_from_user(&rs485, rs485_user, sizeof(*rs485_user)))
> +		return -EFAULT;
> +
> +	mxuport_sanitize_serial_rs485(&rs485);
> +
> +	ret = mxuport_rs485_config(port, &rs485);
> +	if (!ret)
> +		mxport->rs485 = rs485;

We need some locking here as these ioctls can execute in parallel. 

> +
> +	if (copy_to_user(rs485_user, &mxport->rs485, sizeof(mxport->rs485)))
> +		return -EFAULT;
> +
> +	return 0;
> +}

Johan

Re: [PATCH v1 4/4] usb: serial: mxuport: add sysfs control for UART FIFO

[Johan Hovold]

On Tue, Mar 24, 2026 at 11:50:41AM +0800, Crescent Hsieh wrote:
> Add a per-port sysfs attribute, uart_fifo, to allow userspace to enable
> or disable the UART FIFO at runtime.

Why would you want to do that?

We should try to avoid driver specific sysfs knobs.

> Map the requested state to the RQ_VENDOR_SET_FIFO_DISABLE vendor command
> and track the current FIFO setting in the per-port private data.
> 
> Initialize the FIFO state during port probe and remove the sysfs
> attribute when the port is released.

Johan