On Tue, Dec 18, 2018 at 07:59:46PM +0800, Macpaul Lin wrote: > 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. This change only affects > subsequent connects if the reconnected device happens to get the same minor > number. > > 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. > > Signed-off-by: Macpaul Lin <[email protected]> > Cc: [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 quirks testing from init_termios to tty->termios. > - Remove parenthesis for ECHO flag. > Changes for v4: > - Drop quirks varible to simplify the patch. > - Move termios operation right after the driver_data has been installed. > - Write general style comment for suppressing initial echoing. > Changes for v5: > - Fix: termios operation right abover the driver_data has been installed. > - Update commit comment about this patch affects the reconnected device > which get the same minor numbers.
Thanks for sticking with it. After addressing a comment below you have my: Reviewed-by: Johan Hovold <[email protected]> > > drivers/usb/class/cdc-acm.c | 12 +++++++++++- > drivers/usb/class/cdc-acm.h | 1 + > 2 files changed, 12 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c > index 1b68fed..336cf13 100644 > --- a/drivers/usb/class/cdc-acm.c > +++ b/drivers/usb/class/cdc-acm.c > @@ -581,6 +581,13 @@ static int acm_tty_install(struct tty_driver *driver, > struct tty_struct *tty) > if (retval) > goto error_init_termios; > > + /* > + * Suppress initial echoing for some devices which might send data > + * immediately after acm driver has been installed. > + */ > + if (acm->quirks & DISABLE_ECHO) > + tty->termios.c_lflag &= ~ECHO; > + > tty->driver_data = acm; > > return 0; > @@ -1655,7 +1662,10 @@ static int acm_pre_reset(struct usb_interface *intf) > .driver_info = NO_UNION_NORMAL, /* has no union descriptor */ > }, > { USB_DEVICE(0x0e8d, 0x0003), /* FIREFLY, MediaTek Inc; > [email protected] */ > - .driver_info = NO_UNION_NORMAL, /* has no union descriptor */ > + .driver_info = DISABLE_ECHO, /* DISABLE ECHO in termios flag */ I just noticed that you remove the NO_UNION_NORMAL here, which looks wrong and definitely requires a motivation. Thanks, Johan
