Skip to contentSkip to conversation

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

PATCH Superseded Johan Hovold 1 participant 1 message Dec 18, 2018

Subsystems (2)
Stable branch USB

v3 of 7 Superseded Latest is v7 →

lore
Share & tools
lore

Johan Hovold

author
#0 lore raw

PATCH v3 1/1 cdc-acm: fix abnormal DATA RX issue for Mediatek Preloader.

Quoted 24 lines · Macpaul Lin

Mediatek Preloader is a proprietary embedded boot loader for loading Little Kernel and Linux into device DRAM.

This boot loader also handle firmware update. Mediatek Preloader will be enumerated as a virtual COM port when the device is connected to Windows or Linux OS via CDC-ACM class driver. When the USB enumeration has been done, Mediatek Preloader will send out handshake command "READY" to PC actively instead of waiting command from the download tool. Since Linux 4.12, the commit "tty: reset termios state on device registration" (93857edd9829e144acb6c7e72d593f6e01aead66) causes Mediatek Preloader receiving some abnoraml command like "READYXX" as it sent. This will be recognized as an incorrect response. The behavior change also causes the download handshake fail.

By disabling the ECHO termios flag could avoid this problem. However, it cannot be done by user space configuration when download tool open /dev/ttyACM0. This is because the device running Mediatek Preloader will send handshake command "READY" immediately once the CDC-ACM driver is ready.

This patch wants to fix above problem by introducing "DISABLE_ECHO" property in driver_info. When Mediatek Preloader is connected, the CDC-ACM driver could disable ECHO flag in termios to avoid the problem.

Quoted 8 lines
Signed-off-by Macpaul Lin <[email protected]>

---
Changes for v2:
- Move quirks testing of DISABLE_ECHO flag into acm_tty_install().
- Change quirks testing into bitwise comparison.
Changes for v3:
- Replace clear flag operation from driver->init_termios to tty->termios.
- Remove parenthesis of ECHO flag.

Thanks for the update. Looks good, just some minor issues below.

Quoted 17 lines
 drivers/usb/class/cdc-acm.c | 13 ++++++++++++-
 drivers/usb/class/cdc-acm.h |  1 +
 2 files changed, 13 insertions(+), 1 deletion(-)
drivers/usb/class/cdc-acm.c +1 -0
index 1b68fed..c1b88c3 100644
@@ -571,6 +571,7 @@ static void acm_softint(struct work_struct *work)
 static int acm_tty_install(struct tty_driver *driver, struct tty_struct *tty)
 {
 	struct acm *acm;
+	unsigned long quirks;
 	int retval;
 
 	acm = acm_get_by_minor(tty->index);

drivers/usb/class/cdc-acm.c static int acm_tty_install(struct tty_driver *driver, struct tty_struct *tty) · line 588

Quoted 5 lines
drivers/usb/class/cdc-acm.c +2 -0
@@ -583,6 +584,13 @@ static int acm_tty_install(struct tty_driver *driver, struct tty_struct *tty)
 
 	tty->driver_data = acm;
 
+	/* get normal quirks */
+	quirks = acm->quirks;

I'd drop the quirks variable as you only need it once and acm->quirks is pretty short to begin with.

drivers/usb/class/cdc-acm.c static int acm_tty_install(struct tty_driver *driver, struct tty_struct *tty) · line 590

Quoted 2 lines
drivers/usb/class/cdc-acm.c +2 -0
+
+	/* handle active handshake triggered by device */

Please make this description more general; other devices may also end up needing to suppress initial echoing.

drivers/usb/class/cdc-acm.c static int acm_tty_install(struct tty_driver *driver, struct tty_struct *tty) · line 592

Quoted 2 lines
drivers/usb/class/cdc-acm.c +2 -0
+	if (quirks & DISABLE_ECHO)
+		tty->termios.c_lflag &= ~ECHO;

And finally, please move this just after installing the tty (i.e. before setting tty->driver data).

drivers/usb/class/cdc-acm.c static int acm_tty_install(struct tty_driver *driver, struct tty_struct *tty) · line 594

Quoted 2 lines
drivers/usb/class/cdc-acm.c +1 -0
+
 	return 0;

Thanks,
Johan