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 <macpaul.lin@xxxxxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
> ---
> 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 <johan@xxxxxxxxxx>
>
> 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; andrey.arapov@xxxxxxxxx */
> - .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
