cdc-acm: fix abnormal DATA RX issue for Mediatek Preloader.
Quoted 12 lines · Macpaul Lin
Quoted 10 lines · Johan Hovold
Quoted 5 lines · Macpaul Lin
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")?
Quoted 8 lines
No, "XX" just meant any random data of any possible length, not the real characters "XX". From the bottom of hardware FIIO, usb driver buffers, to the higher level CDC driver and download handshake flow, the firmware keeps separated RX and TX data buffers. The initial command "READY" is static declared and only been used once when the handshake flow starts in stead of dynamic allocated. We've also tested memory set at the time when RX/TX buffer is allocated when system boot, but the problem did not recovered.
You can enable (verbose) debugging in the cdc-acm driver to see how many bytes it receives from the device (and how many bytes it echoes back) in order to determine where these garbage characters come from. Just define DEBUG and VERBOSE_DEBUG in the cdc-acm driver.
Quoted 11 lines
Quoted 3 lines
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.
We've also reviewed the firmware code, too. This mechanism of download flow (firmware code) has been used more than 10 years and has never been changed. Our phone product ships 100 million per year. What I mean here is this download flow has been verified in the factory producing line both on Windows and Linux PC so many time. Hence the possibility of bug in the data buffer management is very rare.
Quoted 12 lines
This problem has been reported by one of our customer about 1 month ago because they upgraded their factory PC from Ubuntu 16.04 (Linux 4.4) to Ubuntu 18.04. (Linux 4.15). Hence Mediatek has done lots of bisect test from 4.4 release to 4.15 on both tty and cdc-acm changes to find out which commit caused behavior change on PC side. Because the new reset termios state method comes with another 2 patches. We also tested these patch separately to confirm the bisect result.
tty: reset termios state on device registration
tty: drop obsoletetermios_lockedcomments
tty: close race between device register and open
We've also remove reset termios state commit on latest 4.19 kernel to confirm the behavior of PC is back to work for the download, too.
That's great, but the conclusion remains; the problem would have been there on *first* open also before the termios reset change.
Quoted 11 lines
Quoted 7 lines
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.Thanks for you comment!
I'll make a change to move the clearing ECHO intotty_acm_install()and to check if it also work for the download process. Thanks a lot!
Good. You're still modifying the shared driver init_termios in your v2, but I'll comment on that in a reply to the patch.
Thanks,
Johan