Skip to contentSkip to conversation

cdc-acm: fix abnormal DATA RX issue for Mediatek Preloader.

DISCUSSION Johan Hovold 1 participant 1 message Dec 14, 2018 USB

lore
Share & tools
lore
#0 lore raw
Quoted 14 lines · Macpaul Lin
Quoted 13 lines · Johan Hovold
Quoted 12 lines · Oliver Neukum
Quoted 10 lines · Johan Hovold

Review

Quoted 5 lines · On Thu, Dec 13, 2018 at 11:27:56AM +0800, [email protected] wrote:

From: Macpaul Lin <[email protected]>

(quoted diff) +3 -0
+	/* handle active handshake triggered by device */
+	if (quirks == DISABLE_ECHO)
+		acm_tty_driver->init_termios.c_lflag &= ~(ECHO);

You cannot change the driver init_termios like this as that will affect all cdc-acm devices that are probed later. If this is at all needed, this will have to be done at tty install time.

Quoted 7 lines
Quoted 4 lines
Quoted 1 line

Right. How do with decide on a sensible default anyway?

I think the current defaults are sensible. They are based on tty_std_termios, which has ECHO set, as for most (all?) tty drivers.

Well, the problem is that the phone device (preloader) will get "READYXX" even "no any download tool has ever been launched on PC".

Something much hold the port open on the host (PC) side for it to be echoed back, but perhaps the outgoing data is buffered in the device until it is eventually opened.

Quoted 5 lines

After the reset termios state change simply set EHCO enable, each time the phone device simple send out "READY" to PC after USB has been connected. The "READY" indication has no any "\0" character at the end of the command string, hence it will receive some dirty data (ex. "READYXX") back on RX path.

So you have a firmware bug which sends out some garbage characters ("XX")?

Either way, you should have hit this also before the commit which started resetting the terminal settings whenever a device was plugged in instead of reusing a potentially random other device's settings.

However, after clearing echo and replugging the device, nothing would have been echoed back on next open, but only *if* the device happened to be assigned the same minor number.

But if this confuses your firmware and there's no way to restart the handshake in your host tool, perhaps clearing ECHO at tty install time (i.e. in tty_acm_install()) is the only way to deal with this.

Johan