Re: [PATCH v2 10/12] USB: serial: io_edgeport: deassert DTR/RTS on close() and B0

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] USB: serial: io_edgeport: DTR/RTS dropped on reopen with HUPCL cleared
--

commit 04f867c0006b106f7227398e82b66d8d1816c7f9
Author: Johan Hovold <johan@kernel.org>

USB: serial: io_edgeport: deassert DTR/RTS on close() and B0

This commit adds a dtr_rts() callback to handle asserting and deasserting DTR
and RTS on open(), close(), and hangup, and updates edge_set_termios() to
handle B0 transitions.

> diff --git a/drivers/usb/serial/io_edgeport.c b/drivers/usb/serial/io_edgeport.c
> index 566f0f1a16324..ebbe8c5370cdb 100644
> --- a/drivers/usb/serial/io_edgeport.c
> +++ b/drivers/usb/serial/io_edgeport.c

[ ... ]

> @@ -2412,9 +2429,12 @@ static void edge_set_termios(struct tty_struct *tty, struct usb_serial_port *por
>  
>  	/* set up the MCR register and send it to the EdgePort */
>  	mutex_lock(&edge_port->mutex);
> -	edge_port->shadowMCR = MCR_MASTER_IE;
> -	if (cflag & CBAUD)
> -		edge_port->shadowMCR |= (MCR_DTR | MCR_RTS);
> +	if (cflag & CBAUD) {
> +		if (old_termios && (old_termios->c_cflag & CBAUD) == B0)
> +			edge_port->shadowMCR |= MCR_DTR | MCR_RTS;
> +	} else {
> +		edge_port->shadowMCR &= ~(MCR_DTR | MCR_RTS);
> +	}
>  
>  	status = send_cmd_write_uart_register(edge_port, MCR,
>  						edge_port->shadowMCR);

[Severity: Medium]
Will this cause DTR and RTS to be momentarily dropped when reopening a port
that was closed with HUPCL cleared?

When a port is closed with HUPCL cleared, tty_port_shutdown_locked() leaves
DTR and RTS asserted. On a subsequent open, serial_open() -> tty_port_open()
-> serial_port_activate() calls edge_open(), which resets edge_port->shadowMCR
and calls edge_set_termios() before tty_port_block_til_ready() calls
edge_dtr_rts(port, 1):

drivers/usb/serial/io_edgeport.c:edge_open() {
	...
	/* Must always set this bit to enable ints! */
	mutex_lock(&edge_port->mutex);
	edge_port->shadowMCR = MCR_MASTER_IE;
	mutex_unlock(&edge_port->mutex);
	...
	if (tty)
		edge_set_termios(tty, port, NULL);
	...
}

Because edge_open() clears MCR_DTR and MCR_RTS in shadowMCR and passes
old_termios as NULL to edge_set_termios(), edge_set_termios() no longer sets
MCR_DTR | MCR_RTS and writes shadowMCR without DTR and RTS to the device
before tty_port_block_til_ready() reasserts them.

Should edge_port->shadowMCR = MCR_MASTER_IE be initialized once in
edge_port_probe() instead of being reset on every open in edge_open() so that
DTR and RTS are preserved across a close and reopen when HUPCL is cleared?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261023121530.186541-1-johan@kernel.org?part=10