Re: [PATCH] USB: serial: ch341: Clear interrupt register after each interrupt

[John Watts]

On Wed, Mar 19, 2025 at 01:52:20AM +1100, John Watts wrote:
> The CH340 adapter I have won't send interrupts unless you clear the
> 0x2727 register after each interrupt. The Windows driver does this but
> neither the mainline Linux or OEM Linux driver do this.
> 
> Without this fix the device status flags for hardware flow control are
> never updated.
> 
> As far as I can tell there is not a register to configure this
> behaviour, this seems to be a hardware quirk. The chip reports an
> identical version, vendor and product to a working chip. It's possible
> this is a clone chip only tested against Windows.
> 
> Signed-off-by: John Watts <contact@xxxxxxxxxx>
> ---
> This fixes hardware flow control flags like RTS and CTS not updating on
> a specific (fake?) CH340 chip I have.
> 
> John Paul Morrison reported an issue that sounds similar to this one in
> 2022:
> https://lore.kernel.org/all/YlP1poVgy0bAP3MN@xxxxxxxxxxxxxxxxxxxx/T/

After a lot more testing it looks like this isn't the correct way to fix
this issue: Status changes will still be dropped if they trigger two
interrupts in quick succession as I don't poll the status. This is
something that can happen fairly easily with a null modem cable that
connects DTR to DSR+DCD.

The only proper fix here is to put modem status updates in to its own
worker that is triggered when we suspect the modem status is out of
date. It would clear interrupts then read the status properly.

John.

> ---
>  drivers/usb/serial/ch341.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 51 insertions(+)
> 
> diff --git a/drivers/usb/serial/ch341.c b/drivers/usb/serial/ch341.c
> index d10e4c4848a0ab9073c4c93638a8796ab61ce3a6..8edbac18146cebd0ff7b9cfaca6853a2c5f047df 100644
> --- a/drivers/usb/serial/ch341.c
> +++ b/drivers/usb/serial/ch341.c
> @@ -63,6 +63,7 @@
>  #define CH341_REG_DIVISOR      0x13
>  #define CH341_REG_LCR          0x18
>  #define CH341_REG_LCR2         0x25
> +#define CH341_REG_INTERRUPT    0x2727
>  
>  #define CH341_NBREAK_BITS      0x01
>  
> @@ -102,6 +103,9 @@ struct ch341_private {
>  	u8 version;
>  
>  	unsigned long break_end;
> +
> +	struct work_struct interrupt_work;
> +	struct usb_serial_port *port;
>  };
>  
>  static void ch341_set_termios(struct tty_struct *tty,
> @@ -306,6 +310,32 @@ static int ch341_get_status(struct usb_device *dev, struct ch341_private *priv)
>  	return 0;
>  }
>  
> +static int ch341_clear_interrupt(struct usb_device *dev)
> +{
> +	int r;
> +
> +	r = ch341_control_out(dev, CH341_REQ_WRITE_REG,
> +			CH341_REG_INTERRUPT, 0);
> +	if (r)
> +		return r;
> +
> +	return 0;
> +}
> +
> +static void ch341_interrupt_work(struct work_struct *work)
> +{
> +	struct ch341_private *priv =
> +		container_of(work, struct ch341_private, interrupt_work);
> +	struct usb_serial_port *port = priv->port;
> +	int ret;
> +
> +	ret = ch341_clear_interrupt(port->serial->dev);
> +	if (ret < 0) {
> +		dev_err_once(&port->dev, "failed to clear interrupt: %d\n",
> +			ret);
> +	}
> +}
> +
>  /* -------------------------------------------------------------------------- */
>  
>  static int ch341_configure(struct usb_device *dev, struct ch341_private *priv)
> @@ -399,6 +429,9 @@ static int ch341_port_probe(struct usb_serial_port *port)
>  	if (r < 0)
>  		goto error;
>  
> +	INIT_WORK(&priv->interrupt_work, ch341_interrupt_work);
> +	priv->port = port;
> +
>  	return 0;
>  
>  error:	kfree(priv);
> @@ -438,8 +471,10 @@ static void ch341_dtr_rts(struct usb_serial_port *port, int on)
>  
>  static void ch341_close(struct usb_serial_port *port)
>  {
> +	struct ch341_private *priv = usb_get_serial_port_data(port);
>  	usb_serial_generic_close(port);
>  	usb_kill_urb(port->interrupt_in_urb);
> +	flush_work(&priv->interrupt_work);
>  }
>  
>  
> @@ -466,6 +501,12 @@ static int ch341_open(struct tty_struct *tty, struct usb_serial_port *port)
>  		goto err_kill_interrupt_urb;
>  	}
>  
> +	r = ch341_clear_interrupt(port->serial->dev);
> +	if (r < 0) {
> +		dev_err(&port->dev, "failed to clear interrupt: %d\n", r);
> +		goto err_kill_interrupt_urb;
> +	}
> +
>  	r = usb_serial_generic_open(tty, port);
>  	if (r)
>  		goto err_kill_interrupt_urb;
> @@ -474,6 +515,7 @@ static int ch341_open(struct tty_struct *tty, struct usb_serial_port *port)
>  
>  err_kill_interrupt_urb:
>  	usb_kill_urb(port->interrupt_in_urb);
> +	flush_work(&priv->interrupt_work);
>  
>  	return r;
>  }
> @@ -747,6 +789,7 @@ static void ch341_update_status(struct usb_serial_port *port,
>  static void ch341_read_int_callback(struct urb *urb)
>  {
>  	struct usb_serial_port *port = urb->context;
> +	struct ch341_private *priv = usb_get_serial_port_data(port);
>  	unsigned char *data = urb->transfer_buffer;
>  	unsigned int len = urb->actual_length;
>  	int status;
> @@ -770,6 +813,8 @@ static void ch341_read_int_callback(struct urb *urb)
>  
>  	usb_serial_debug_data(&port->dev, __func__, len, data);
>  	ch341_update_status(port, data, len);
> +	schedule_work(&priv->interrupt_work);
> +
>  exit:
>  	status = usb_submit_urb(urb, GFP_ATOMIC);
>  	if (status) {
> @@ -830,6 +875,12 @@ static int ch341_reset_resume(struct usb_serial *serial)
>  			dev_err(&port->dev, "failed to read modem status: %d\n",
>  				ret);
>  		}
> +
> +		ret = ch341_clear_interrupt(port->serial->dev);
> +		if (ret < 0) {
> +			dev_err(&port->dev, "failed to clear interrupt: %d\n",
> +				ret);
> +		}
>  	}
>  
>  	return usb_serial_generic_resume(serial);
> 
> ---
> base-commit: ffd294d346d185b70e28b1a28abe367bbfe53c04
> change-id: 20250319-ch341-aab9934f1c72
> 
> Best regards,
> -- 
> John Watts <contact@xxxxxxxxxx>
>