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

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

[macpaul.lin]

From: Macpaul Lin <macpaul.lin@mediatek.com>

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

This boot loader also handle firmware updating. 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 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 sended.
Which will be recognized as an incorrect response. This behavior change
also causes the 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.

Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com>
---
 drivers/usb/class/cdc-acm.c | 9 ++++++++-
 drivers/usb/class/cdc-acm.h | 1 +
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index 1b68fed..2f744bb 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -1156,6 +1156,10 @@ static int acm_probe(struct usb_interface *intf,
 		goto skip_normal_probe;
 	}
 
+	/* handle active handshake triggered by device */
+	if (quirks == DISABLE_ECHO)
+		acm_tty_driver->init_termios.c_lflag &= ~(ECHO);
+
 	/* normal probing*/
 	if (!buffer) {
 		dev_err(&intf->dev, "Weird descriptor references\n");
@@ -1655,7 +1659,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@gmail.com */
-	.driver_info = NO_UNION_NORMAL, /* has no union descriptor */
+	.driver_info = DISABLE_ECHO, /* DISABLE ECHO in termios flag */
+	},
+	{ USB_DEVICE(0x0e8d, 0x2000), /* FIREFLY, MediaTek Inc; Preloader */
+	.driver_info = DISABLE_ECHO, /* DISABLE ECHO in termios flag */
 	},
 	{ USB_DEVICE(0x0e8d, 0x3329), /* MediaTek Inc GPS */
 	.driver_info = NO_UNION_NORMAL, /* has no union descriptor */
diff --git a/drivers/usb/class/cdc-acm.h b/drivers/usb/class/cdc-acm.h
index ca06b20..515aad0 100644
--- a/drivers/usb/class/cdc-acm.h
+++ b/drivers/usb/class/cdc-acm.h
@@ -140,3 +140,4 @@ struct acm {
 #define QUIRK_CONTROL_LINE_STATE	BIT(6)
 #define CLEAR_HALT_CONDITIONS		BIT(7)
 #define SEND_ZERO_PACKET		BIT(8)
+#define DISABLE_ECHO			BIT(9)

[PATCH v2] cdc-acm: fix abnormal DATA RX issue for Mediatek Preloader.

[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.

Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com>
---
Change for v2:
 - Move quirks testing of DISABLE_ECHO flag into acm_tty_install().
 - Change quirks testing into bitwise comparison.

 drivers/usb/class/cdc-acm.c | 13 ++++++++++++-
 drivers/usb/class/cdc-acm.h |  1 +
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index 1b68fed..f1a914d 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -571,12 +571,20 @@ 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);
 	if (!acm)
 		return -ENODEV;
 
+	/* get normal quirks */
+	quirks = acm->quirks;
+
+	/* handle active handshake triggered by device */
+	if (quirks & DISABLE_ECHO)
+		driver->init_termios.c_lflag &= ~(ECHO);
+
 	retval = tty_standard_install(driver, tty);
 	if (retval)
 		goto error_init_termios;
@@ -1655,7 +1663,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@gmail.com */
-	.driver_info = NO_UNION_NORMAL, /* has no union descriptor */
+	.driver_info = DISABLE_ECHO, /* DISABLE ECHO in termios flag */
+	},
+	{ USB_DEVICE(0x0e8d, 0x2000), /* FIREFLY, MediaTek Inc; Preloader */
+	.driver_info = DISABLE_ECHO, /* DISABLE ECHO in termios flag */
 	},
 	{ USB_DEVICE(0x0e8d, 0x3329), /* MediaTek Inc GPS */
 	.driver_info = NO_UNION_NORMAL, /* has no union descriptor */
diff --git a/drivers/usb/class/cdc-acm.h b/drivers/usb/class/cdc-acm.h
index ca06b20..515aad0 100644
--- a/drivers/usb/class/cdc-acm.h
+++ b/drivers/usb/class/cdc-acm.h
@@ -140,3 +140,4 @@ struct acm {
 #define QUIRK_CONTROL_LINE_STATE	BIT(6)
 #define CLEAR_HALT_CONDITIONS		BIT(7)
 #define SEND_ZERO_PACKET		BIT(8)
+#define DISABLE_ECHO			BIT(9)
-- 
1.9.1

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

[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.

Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com>
---
Change for v2:
 - Move quirks testing of DISABLE_ECHO flag into acm_tty_install().
 - Change quirks testing into bitwise comparison.

 drivers/usb/class/cdc-acm.c | 13 ++++++++++++-
 drivers/usb/class/cdc-acm.h |  1 +
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index 1b68fed..f1a914d 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -571,12 +571,20 @@ 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);
 	if (!acm)
 		return -ENODEV;
 
+	/* get normal quirks */
+	quirks = acm->quirks;
+
+	/* handle active handshake triggered by device */
+	if (quirks & DISABLE_ECHO)
+		driver->init_termios.c_lflag &= ~(ECHO);
+
 	retval = tty_standard_install(driver, tty);
 	if (retval)
 		goto error_init_termios;
@@ -1655,7 +1663,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@gmail.com */
-	.driver_info = NO_UNION_NORMAL, /* has no union descriptor */
+	.driver_info = DISABLE_ECHO, /* DISABLE ECHO in termios flag */
+	},
+	{ USB_DEVICE(0x0e8d, 0x2000), /* FIREFLY, MediaTek Inc; Preloader */
+	.driver_info = DISABLE_ECHO, /* DISABLE ECHO in termios flag */
 	},
 	{ USB_DEVICE(0x0e8d, 0x3329), /* MediaTek Inc GPS */
 	.driver_info = NO_UNION_NORMAL, /* has no union descriptor */
diff --git a/drivers/usb/class/cdc-acm.h b/drivers/usb/class/cdc-acm.h
index ca06b20..515aad0 100644
--- a/drivers/usb/class/cdc-acm.h
+++ b/drivers/usb/class/cdc-acm.h
@@ -140,3 +140,4 @@ struct acm {
 #define QUIRK_CONTROL_LINE_STATE	BIT(6)
 #define CLEAR_HALT_CONDITIONS		BIT(7)
 #define SEND_ZERO_PACKET		BIT(8)
+#define DISABLE_ECHO			BIT(9)

Re: [PATCH v2] cdc-acm: fix abnormal DATA RX issue for Mediatek Preloader.

[Johan Hovold]

On Tue, Dec 18, 2018 at 01:00:22PM +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.
> 
> 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@mediatek.com>
> ---
> Change for v2:
>  - Move quirks testing of DISABLE_ECHO flag into acm_tty_install().
>  - Change quirks testing into bitwise comparison.
> 
>  drivers/usb/class/cdc-acm.c | 13 ++++++++++++-
>  drivers/usb/class/cdc-acm.h |  1 +
>  2 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
> index 1b68fed..f1a914d 100644
> --- a/drivers/usb/class/cdc-acm.c
> +++ b/drivers/usb/class/cdc-acm.c
> @@ -571,12 +571,20 @@ 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);
>  	if (!acm)
>  		return -ENODEV;
>  
> +	/* get normal quirks */
> +	quirks = acm->quirks;
> +
> +	/* handle active handshake triggered by device */
> +	if (quirks & DISABLE_ECHO)
> +		driver->init_termios.c_lflag &= ~(ECHO);

You're still changing the shared driver init_termios here, which is no
better than doing so at probe time.

What I meant by moving clearing ECHO to tty install time was that that
will allow you do clear ECHO only for the tty being installed (and
specifically not affecting the termios of other cdc-acm devices).

> +
>  	retval = tty_standard_install(driver, tty);
>  	if (retval)
>  		goto error_init_termios;

So here, after tty_standard_install() returns you can modify
tty->termios after it has been initialised to the default (or saved)
settings.

Also note that you can drop the parenthesis around ECHO above.

Thanks,
Johan

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

[Johan Hovold]

On Tue, Dec 18, 2018 at 01:00:22PM +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.
> 
> 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@mediatek.com>
> ---
> Change for v2:
>  - Move quirks testing of DISABLE_ECHO flag into acm_tty_install().
>  - Change quirks testing into bitwise comparison.
> 
>  drivers/usb/class/cdc-acm.c | 13 ++++++++++++-
>  drivers/usb/class/cdc-acm.h |  1 +
>  2 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
> index 1b68fed..f1a914d 100644
> --- a/drivers/usb/class/cdc-acm.c
> +++ b/drivers/usb/class/cdc-acm.c
> @@ -571,12 +571,20 @@ 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);
>  	if (!acm)
>  		return -ENODEV;
>  
> +	/* get normal quirks */
> +	quirks = acm->quirks;
> +
> +	/* handle active handshake triggered by device */
> +	if (quirks & DISABLE_ECHO)
> +		driver->init_termios.c_lflag &= ~(ECHO);

You're still changing the shared driver init_termios here, which is no
better than doing so at probe time.

What I meant by moving clearing ECHO to tty install time was that that
will allow you do clear ECHO only for the tty being installed (and
specifically not affecting the termios of other cdc-acm devices).

> +
>  	retval = tty_standard_install(driver, tty);
>  	if (retval)
>  		goto error_init_termios;

So here, after tty_standard_install() returns you can modify
tty->termios after it has been initialised to the default (or saved)
settings.

Also note that you can drop the parenthesis around ECHO above.

Thanks,
Johan

[PATCH v3] cdc-acm: fix abnormal DATA RX issue for Mediatek Preloader.

[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.

Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com>
---
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.

 drivers/usb/class/cdc-acm.c | 13 ++++++++++++-
 drivers/usb/class/cdc-acm.h |  1 +
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index 1b68fed..c1b88c3 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -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);
@@ -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;
+
+	/* handle active handshake triggered by device */
+	if (quirks & DISABLE_ECHO)
+		tty->termios.c_lflag &= ~ECHO;
+
 	return 0;
 
 error_init_termios:
@@ -1655,7 +1663,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@gmail.com */
-	.driver_info = NO_UNION_NORMAL, /* has no union descriptor */
+	.driver_info = DISABLE_ECHO, /* DISABLE ECHO in termios flag */
+	},
+	{ USB_DEVICE(0x0e8d, 0x2000), /* FIREFLY, MediaTek Inc; Preloader */
+	.driver_info = DISABLE_ECHO, /* DISABLE ECHO in termios flag */
 	},
 	{ USB_DEVICE(0x0e8d, 0x3329), /* MediaTek Inc GPS */
 	.driver_info = NO_UNION_NORMAL, /* has no union descriptor */
diff --git a/drivers/usb/class/cdc-acm.h b/drivers/usb/class/cdc-acm.h
index ca06b20..515aad0 100644
--- a/drivers/usb/class/cdc-acm.h
+++ b/drivers/usb/class/cdc-acm.h
@@ -140,3 +140,4 @@ struct acm {
 #define QUIRK_CONTROL_LINE_STATE	BIT(6)
 #define CLEAR_HALT_CONDITIONS		BIT(7)
 #define SEND_ZERO_PACKET		BIT(8)
+#define DISABLE_ECHO			BIT(9)
-- 
1.9.1

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

[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.

Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com>
---
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.

 drivers/usb/class/cdc-acm.c | 13 ++++++++++++-
 drivers/usb/class/cdc-acm.h |  1 +
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index 1b68fed..c1b88c3 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -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);
@@ -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;
+
+	/* handle active handshake triggered by device */
+	if (quirks & DISABLE_ECHO)
+		tty->termios.c_lflag &= ~ECHO;
+
 	return 0;
 
 error_init_termios:
@@ -1655,7 +1663,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@gmail.com */
-	.driver_info = NO_UNION_NORMAL, /* has no union descriptor */
+	.driver_info = DISABLE_ECHO, /* DISABLE ECHO in termios flag */
+	},
+	{ USB_DEVICE(0x0e8d, 0x2000), /* FIREFLY, MediaTek Inc; Preloader */
+	.driver_info = DISABLE_ECHO, /* DISABLE ECHO in termios flag */
 	},
 	{ USB_DEVICE(0x0e8d, 0x3329), /* MediaTek Inc GPS */
 	.driver_info = NO_UNION_NORMAL, /* has no union descriptor */
diff --git a/drivers/usb/class/cdc-acm.h b/drivers/usb/class/cdc-acm.h
index ca06b20..515aad0 100644
--- a/drivers/usb/class/cdc-acm.h
+++ b/drivers/usb/class/cdc-acm.h
@@ -140,3 +140,4 @@ struct acm {
 #define QUIRK_CONTROL_LINE_STATE	BIT(6)
 #define CLEAR_HALT_CONDITIONS		BIT(7)
 #define SEND_ZERO_PACKET		BIT(8)
+#define DISABLE_ECHO			BIT(9)

Re: [PATCH v3] cdc-acm: fix abnormal DATA RX issue for Mediatek Preloader.

[Johan Hovold]

On Tue, Dec 18, 2018 at 06:41:20PM +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.
> 
> 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@mediatek.com>
> ---
> 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.

>  drivers/usb/class/cdc-acm.c | 13 ++++++++++++-
>  drivers/usb/class/cdc-acm.h |  1 +
>  2 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
> index 1b68fed..c1b88c3 100644
> --- a/drivers/usb/class/cdc-acm.c
> +++ b/drivers/usb/class/cdc-acm.c
> @@ -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);
> @@ -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.

> +
> +	/* handle active handshake triggered by device */

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

> +	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).

> +
>  	return 0;

Thanks,
Johan

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

[Johan Hovold]

On Tue, Dec 18, 2018 at 06:41:20PM +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.
> 
> 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@mediatek.com>
> ---
> 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.

>  drivers/usb/class/cdc-acm.c | 13 ++++++++++++-
>  drivers/usb/class/cdc-acm.h |  1 +
>  2 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
> index 1b68fed..c1b88c3 100644
> --- a/drivers/usb/class/cdc-acm.c
> +++ b/drivers/usb/class/cdc-acm.c
> @@ -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);
> @@ -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.

> +
> +	/* handle active handshake triggered by device */

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

> +	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).

> +
>  	return 0;

Thanks,
Johan

[PATCH v4] cdc-acm: fix abnormal DATA RX issue for Mediatek Preloader.

[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.

Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com>
---
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. 

 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..ab8609e 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -583,6 +583,13 @@ static int acm_tty_install(struct tty_driver *driver, struct tty_struct *tty)
 
 	tty->driver_data = acm;
 
+	/*
+	 * 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;
+
 	return 0;
 
 error_init_termios:
@@ -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@gmail.com */
-	.driver_info = NO_UNION_NORMAL, /* has no union descriptor */
+	.driver_info = DISABLE_ECHO, /* DISABLE ECHO in termios flag */
+	},
+	{ USB_DEVICE(0x0e8d, 0x2000), /* FIREFLY, MediaTek Inc; Preloader */
+	.driver_info = DISABLE_ECHO, /* DISABLE ECHO in termios flag */
 	},
 	{ USB_DEVICE(0x0e8d, 0x3329), /* MediaTek Inc GPS */
 	.driver_info = NO_UNION_NORMAL, /* has no union descriptor */
diff --git a/drivers/usb/class/cdc-acm.h b/drivers/usb/class/cdc-acm.h
index ca06b20..515aad0 100644
--- a/drivers/usb/class/cdc-acm.h
+++ b/drivers/usb/class/cdc-acm.h
@@ -140,3 +140,4 @@ struct acm {
 #define QUIRK_CONTROL_LINE_STATE	BIT(6)
 #define CLEAR_HALT_CONDITIONS		BIT(7)
 #define SEND_ZERO_PACKET		BIT(8)
+#define DISABLE_ECHO			BIT(9)
-- 
1.9.1

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

[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.

Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com>
---
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. 

 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..ab8609e 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -583,6 +583,13 @@ static int acm_tty_install(struct tty_driver *driver, struct tty_struct *tty)
 
 	tty->driver_data = acm;
 
+	/*
+	 * 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;
+
 	return 0;
 
 error_init_termios:
@@ -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@gmail.com */
-	.driver_info = NO_UNION_NORMAL, /* has no union descriptor */
+	.driver_info = DISABLE_ECHO, /* DISABLE ECHO in termios flag */
+	},
+	{ USB_DEVICE(0x0e8d, 0x2000), /* FIREFLY, MediaTek Inc; Preloader */
+	.driver_info = DISABLE_ECHO, /* DISABLE ECHO in termios flag */
 	},
 	{ USB_DEVICE(0x0e8d, 0x3329), /* MediaTek Inc GPS */
 	.driver_info = NO_UNION_NORMAL, /* has no union descriptor */
diff --git a/drivers/usb/class/cdc-acm.h b/drivers/usb/class/cdc-acm.h
index ca06b20..515aad0 100644
--- a/drivers/usb/class/cdc-acm.h
+++ b/drivers/usb/class/cdc-acm.h
@@ -140,3 +140,4 @@ struct acm {
 #define QUIRK_CONTROL_LINE_STATE	BIT(6)
 #define CLEAR_HALT_CONDITIONS		BIT(7)
 #define SEND_ZERO_PACKET		BIT(8)
+#define DISABLE_ECHO			BIT(9)

Re: [PATCH v4] cdc-acm: fix abnormal DATA RX issue for Mediatek Preloader.

[Johan Hovold]

On Tue, Dec 18, 2018 at 07:38:07PM +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.

Please mention that 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@mediatek.com>

And you can add a CC stable tag here.

> ---
> 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.

No, it should go after the *tty* has been installed (and *before*
setting driver_data)

>  - Write general style comment for suppressing initial echoing. 

>  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..ab8609e 100644
> --- a/drivers/usb/class/cdc-acm.c
> +++ b/drivers/usb/class/cdc-acm.c
> @@ -583,6 +583,13 @@ static int acm_tty_install(struct tty_driver *driver, struct tty_struct *tty)
>  
>  	tty->driver_data = acm;
>  
> +	/*
> +	 * 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;

You forgot to move this above setting tty->driver data.

> +
>  	return 0;

Thanks,
Johan

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

[Johan Hovold]

On Tue, Dec 18, 2018 at 07:38:07PM +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.

Please mention that 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@mediatek.com>

And you can add a CC stable tag here.

> ---
> 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.

No, it should go after the *tty* has been installed (and *before*
setting driver_data)

>  - Write general style comment for suppressing initial echoing. 

>  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..ab8609e 100644
> --- a/drivers/usb/class/cdc-acm.c
> +++ b/drivers/usb/class/cdc-acm.c
> @@ -583,6 +583,13 @@ static int acm_tty_install(struct tty_driver *driver, struct tty_struct *tty)
>  
>  	tty->driver_data = acm;
>  
> +	/*
> +	 * 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;

You forgot to move this above setting tty->driver data.

> +
>  	return 0;

Thanks,
Johan

[PATCH v5] cdc-acm: fix abnormal DATA RX issue for Mediatek Preloader.

[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. 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@mediatek.com>
Cc: stable@vger.kernel.org
---
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.

 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@gmail.com */
-	.driver_info = NO_UNION_NORMAL, /* has no union descriptor */
+	.driver_info = DISABLE_ECHO, /* DISABLE ECHO in termios flag */
+	},
+	{ USB_DEVICE(0x0e8d, 0x2000), /* FIREFLY, MediaTek Inc; Preloader */
+	.driver_info = DISABLE_ECHO, /* DISABLE ECHO in termios flag */
 	},
 	{ USB_DEVICE(0x0e8d, 0x3329), /* MediaTek Inc GPS */
 	.driver_info = NO_UNION_NORMAL, /* has no union descriptor */
diff --git a/drivers/usb/class/cdc-acm.h b/drivers/usb/class/cdc-acm.h
index ca06b20..515aad0 100644
--- a/drivers/usb/class/cdc-acm.h
+++ b/drivers/usb/class/cdc-acm.h
@@ -140,3 +140,4 @@ struct acm {
 #define QUIRK_CONTROL_LINE_STATE	BIT(6)
 #define CLEAR_HALT_CONDITIONS		BIT(7)
 #define SEND_ZERO_PACKET		BIT(8)
+#define DISABLE_ECHO			BIT(9)
-- 
1.9.1

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

[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. 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@mediatek.com>
Cc: stable@vger.kernel.org
---
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.

 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@gmail.com */
-	.driver_info = NO_UNION_NORMAL, /* has no union descriptor */
+	.driver_info = DISABLE_ECHO, /* DISABLE ECHO in termios flag */
+	},
+	{ USB_DEVICE(0x0e8d, 0x2000), /* FIREFLY, MediaTek Inc; Preloader */
+	.driver_info = DISABLE_ECHO, /* DISABLE ECHO in termios flag */
 	},
 	{ USB_DEVICE(0x0e8d, 0x3329), /* MediaTek Inc GPS */
 	.driver_info = NO_UNION_NORMAL, /* has no union descriptor */
diff --git a/drivers/usb/class/cdc-acm.h b/drivers/usb/class/cdc-acm.h
index ca06b20..515aad0 100644
--- a/drivers/usb/class/cdc-acm.h
+++ b/drivers/usb/class/cdc-acm.h
@@ -140,3 +140,4 @@ struct acm {
 #define QUIRK_CONTROL_LINE_STATE	BIT(6)
 #define CLEAR_HALT_CONDITIONS		BIT(7)
 #define SEND_ZERO_PACKET		BIT(8)
+#define DISABLE_ECHO			BIT(9)

Re: [PATCH v5] cdc-acm: fix abnormal DATA RX issue for Mediatek Preloader.

[Johan Hovold]

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@mediatek.com>
> Cc: stable@vger.kernel.org
> ---
> 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@kernel.org>

> 
>  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@gmail.com */
> -	.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

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

[Johan Hovold]

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@mediatek.com>
> Cc: stable@vger.kernel.org
> ---
> 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@kernel.org>

> 
>  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@gmail.com */
> -	.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

Re: [PATCH v5] cdc-acm: fix abnormal DATA RX issue for Mediatek Preloader.

[Oliver Neukum]

On Di, 2018-12-18 at 13:38 +0100, Johan Hovold wrote:
> On 
> > @@ -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@gmail.com */
> > -	.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
> 

Hi,

thank you and thank you Johan.
Unfortunately I cannot take this until the issue with the removed
quirk is clarified.

	Regards
		Oliver

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

[Oliver Neukum]

On Di, 2018-12-18 at 13:38 +0100, Johan Hovold wrote:
> On 
> > @@ -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@gmail.com */
> > -	.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
> 

Hi,

thank you and thank you Johan.
Unfortunately I cannot take this until the issue with the removed
quirk is clarified.

	Regards
		Oliver

Re: [PATCH v5] cdc-acm: fix abnormal DATA RX issue for Mediatek Preloader.

[Macpaul Lin]

On Tue, 2018-12-18 at 14:37 +0100, Oliver Neukum wrote:
> On Di, 2018-12-18 at 13:38 +0100, Johan Hovold wrote:
> > On 
> > > @@ -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@gmail.com */
> > > -	.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
> > 
> 
> Hi,
> 
> thank you and thank you Johan.
> Unfortunately I cannot take this until the issue with the removed
> quirk is clarified.
> 
> 	Regards
> 		Oliver
> 

Hi,
Thanks Johan's help on reviewing the updated patches.
According to Andrey's patch, the commit said this change is for
"Samsung X180 China cellphone" which might be also a Mediatek SoC based
phone using Mediatek VID.
https://goo.gl/a9ddNq (I've add a url for reference here).

But I'm not sure the PID is for Mediatek's BROM or customized Preloader.
(Both should be able to apply DISABLE_ECHO flag).
Maybe I can simply update PATCH v6 by removing FIREFLY if any one has
problem here.
Let's also loop Andrey for clarification.

Thanks!

Regards,
Macpaul Lin

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

[macpaul.lin]

On Tue, 2018-12-18 at 14:37 +0100, Oliver Neukum wrote:
> On Di, 2018-12-18 at 13:38 +0100, Johan Hovold wrote:
> > On 
> > > @@ -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@gmail.com */
> > > -	.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
> > 
> 
> Hi,
> 
> thank you and thank you Johan.
> Unfortunately I cannot take this until the issue with the removed
> quirk is clarified.
> 
> 	Regards
> 		Oliver
> 

Hi,
Thanks Johan's help on reviewing the updated patches.
According to Andrey's patch, the commit said this change is for
"Samsung X180 China cellphone" which might be also a Mediatek SoC based
phone using Mediatek VID.
https://goo.gl/a9ddNq (I've add a url for reference here).

But I'm not sure the PID is for Mediatek's BROM or customized Preloader.
(Both should be able to apply DISABLE_ECHO flag).
Maybe I can simply update PATCH v6 by removing FIREFLY if any one has
problem here.
Let's also loop Andrey for clarification.

Thanks!

Regards,
Macpaul Lin

Re: [PATCH v5] cdc-acm: fix abnormal DATA RX issue for Mediatek Preloader.

[Macpaul Lin]

On Tue, 2018-12-18 at 22:26 +0800, Macpaul Lin wrote:
> On Tue, 2018-12-18 at 14:37 +0100, Oliver Neukum wrote:
> > On Di, 2018-12-18 at 13:38 +0100, Johan Hovold wrote:
> > > On 
> > > > @@ -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@gmail.com */
> > > > -	.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
> > > 
> > 
> > Hi,
> > 
> > thank you and thank you Johan.
> > Unfortunately I cannot take this until the issue with the removed
> > quirk is clarified.
> > 
> > 	Regards
> > 		Oliver
> > 
> 
> Hi,
> Thanks Johan's help on reviewing the updated patches.
> According to Andrey's patch, the commit said this change is for
> "Samsung X180 China cellphone" which might be also a Mediatek SoC based
> phone using Mediatek VID.
> https://goo.gl/a9ddNq (I've add a url for reference here).
> 
> But I'm not sure the PID is for Mediatek's BROM or customized Preloader.
> (Both should be able to apply DISABLE_ECHO flag).
> Maybe I can simply update PATCH v6 by removing FIREFLY if any one has
> problem here.
> Let's also loop Andrey for clarification.

Hi all,

After double check the PID/VID officially used (registered) by Mediatek
Inc. The following VID/PID maps to the corresponding USB interface of
the firmware of cell phone.
VID:0x0e8d, PID:0x0003: Mediatek BROM. 
VID:0x0e8d, PID:0x2000: Mediatek Preloader.

I will update Patch v6 for changing Andrey's previous comments.

Thanks.

Regards,
Macpaul Lin.

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

[macpaul.lin]

On Tue, 2018-12-18 at 22:26 +0800, Macpaul Lin wrote:
> On Tue, 2018-12-18 at 14:37 +0100, Oliver Neukum wrote:
> > On Di, 2018-12-18 at 13:38 +0100, Johan Hovold wrote:
> > > On 
> > > > @@ -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@gmail.com */
> > > > -	.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
> > > 
> > 
> > Hi,
> > 
> > thank you and thank you Johan.
> > Unfortunately I cannot take this until the issue with the removed
> > quirk is clarified.
> > 
> > 	Regards
> > 		Oliver
> > 
> 
> Hi,
> Thanks Johan's help on reviewing the updated patches.
> According to Andrey's patch, the commit said this change is for
> "Samsung X180 China cellphone" which might be also a Mediatek SoC based
> phone using Mediatek VID.
> https://goo.gl/a9ddNq (I've add a url for reference here).
> 
> But I'm not sure the PID is for Mediatek's BROM or customized Preloader.
> (Both should be able to apply DISABLE_ECHO flag).
> Maybe I can simply update PATCH v6 by removing FIREFLY if any one has
> problem here.
> Let's also loop Andrey for clarification.

Hi all,

After double check the PID/VID officially used (registered) by Mediatek
Inc. The following VID/PID maps to the corresponding USB interface of
the firmware of cell phone.
VID:0x0e8d, PID:0x0003: Mediatek BROM. 
VID:0x0e8d, PID:0x2000: Mediatek Preloader.

I will update Patch v6 for changing Andrey's previous comments.

Thanks.

Regards,
Macpaul Lin.

[PATCH v6] cdc-acm: fix abnormal DATA RX issue for Mediatek Preloader.

[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. 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@mediatek.com>
Cc: stable@vger.kernel.org
---
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.
Changes for v6:
 - Update VID/PID:0x0e8d/0x0003 as Mediatek Inc BROM.
 - Update VID/PID:0x0e8d/0x2000 as Mediatek Inc Preloader.

 drivers/usb/class/cdc-acm.c | 14 ++++++++++++--
 drivers/usb/class/cdc-acm.h |  1 +
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index 1b68fed..161e2d4 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;
@@ -1654,8 +1661,11 @@ static int acm_pre_reset(struct usb_interface *intf)
 	{ USB_DEVICE(0x0870, 0x0001), /* Metricom GS Modem */
 	.driver_info = NO_UNION_NORMAL, /* has no union descriptor */
 	},
-	{ USB_DEVICE(0x0e8d, 0x0003), /* FIREFLY, MediaTek Inc; andrey.arapov@gmail.com */
-	.driver_info = NO_UNION_NORMAL, /* has no union descriptor */
+	{ USB_DEVICE(0x0e8d, 0x0003), /* MediaTek Inc BROM */
+	.driver_info = DISABLE_ECHO, /* DISABLE ECHO in termios flag */
+	},
+	{ USB_DEVICE(0x0e8d, 0x2000), /* MediaTek Inc Preloader */
+	.driver_info = DISABLE_ECHO, /* DISABLE ECHO in termios flag */
 	},
 	{ USB_DEVICE(0x0e8d, 0x3329), /* MediaTek Inc GPS */
 	.driver_info = NO_UNION_NORMAL, /* has no union descriptor */
diff --git a/drivers/usb/class/cdc-acm.h b/drivers/usb/class/cdc-acm.h
index ca06b20..515aad0 100644
--- a/drivers/usb/class/cdc-acm.h
+++ b/drivers/usb/class/cdc-acm.h
@@ -140,3 +140,4 @@ struct acm {
 #define QUIRK_CONTROL_LINE_STATE	BIT(6)
 #define CLEAR_HALT_CONDITIONS		BIT(7)
 #define SEND_ZERO_PACKET		BIT(8)
+#define DISABLE_ECHO			BIT(9)
-- 
1.9.1

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

[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. 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@mediatek.com>
Cc: stable@vger.kernel.org
---
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.
Changes for v6:
 - Update VID/PID:0x0e8d/0x0003 as Mediatek Inc BROM.
 - Update VID/PID:0x0e8d/0x2000 as Mediatek Inc Preloader.

 drivers/usb/class/cdc-acm.c | 14 ++++++++++++--
 drivers/usb/class/cdc-acm.h |  1 +
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index 1b68fed..161e2d4 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;
@@ -1654,8 +1661,11 @@ static int acm_pre_reset(struct usb_interface *intf)
 	{ USB_DEVICE(0x0870, 0x0001), /* Metricom GS Modem */
 	.driver_info = NO_UNION_NORMAL, /* has no union descriptor */
 	},
-	{ USB_DEVICE(0x0e8d, 0x0003), /* FIREFLY, MediaTek Inc; andrey.arapov@gmail.com */
-	.driver_info = NO_UNION_NORMAL, /* has no union descriptor */
+	{ USB_DEVICE(0x0e8d, 0x0003), /* MediaTek Inc BROM */
+	.driver_info = DISABLE_ECHO, /* DISABLE ECHO in termios flag */
+	},
+	{ USB_DEVICE(0x0e8d, 0x2000), /* MediaTek Inc Preloader */
+	.driver_info = DISABLE_ECHO, /* DISABLE ECHO in termios flag */
 	},
 	{ USB_DEVICE(0x0e8d, 0x3329), /* MediaTek Inc GPS */
 	.driver_info = NO_UNION_NORMAL, /* has no union descriptor */
diff --git a/drivers/usb/class/cdc-acm.h b/drivers/usb/class/cdc-acm.h
index ca06b20..515aad0 100644
--- a/drivers/usb/class/cdc-acm.h
+++ b/drivers/usb/class/cdc-acm.h
@@ -140,3 +140,4 @@ struct acm {
 #define QUIRK_CONTROL_LINE_STATE	BIT(6)
 #define CLEAR_HALT_CONDITIONS		BIT(7)
 #define SEND_ZERO_PACKET		BIT(8)
+#define DISABLE_ECHO			BIT(9)

Re: [PATCH v5] cdc-acm: fix abnormal DATA RX issue for Mediatek Preloader.

[Lars Melin]

On 12/18/2018 22:19, Macpaul Lin wrote:


> Hi all,
> 
> After double check the PID/VID officially used (registered) by Mediatek
> Inc. The following VID/PID maps to the corresponding USB interface of
> the firmware of cell phone.
> VID:0x0e8d, PID:0x0003: Mediatek BROM.
> VID:0x0e8d, PID:0x2000: Mediatek Preloader.
> 
> I will update Patch v6 for changing Andrey's previous comments.
> 

You should show us a verbose lsusb of your 0e8d:0003 before changing
Andrey's submission.


Season's Greeting
Lars

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

[Lars Melin]

On 12/18/2018 22:19, Macpaul Lin wrote:


> Hi all,
> 
> After double check the PID/VID officially used (registered) by Mediatek
> Inc. The following VID/PID maps to the corresponding USB interface of
> the firmware of cell phone.
> VID:0x0e8d, PID:0x0003: Mediatek BROM.
> VID:0x0e8d, PID:0x2000: Mediatek Preloader.
> 
> I will update Patch v6 for changing Andrey's previous comments.
> 

You should show us a verbose lsusb of your 0e8d:0003 before changing
Andrey's submission.


Season's Greeting
Lars

Re: [PATCH v5] cdc-acm: fix abnormal DATA RX issue for Mediatek Preloader.

[Macpaul Lin]

On Tue, 2018-12-18 at 23:42 +0700, Lars Melin wrote:
> On 12/18/2018 22:19, Macpaul Lin wrote:
> 
> 
> > Hi all,
> > 
> > After double check the PID/VID officially used (registered) by Mediatek
> > Inc. The following VID/PID maps to the corresponding USB interface of
> > the firmware of cell phone.
> > VID:0x0e8d, PID:0x0003: Mediatek BROM.
> > VID:0x0e8d, PID:0x2000: Mediatek Preloader.
> > 
> > I will update Patch v6 for changing Andrey's previous comments.
> > 
> 
> You should show us a verbose lsusb of your 0e8d:0003 before changing
> Andrey's submission.
> 
> 
> Season's Greeting
> Lars

Hi Lars, Oliver, and Johan,

Well, it's about 1:30 a.m. here in Taiwan. I can do lsusb dump tomorrow
in Mediatek's office.
But there are a lot of tutorials about hacking and flash method of
customized firmware over the WWW. I can find one of the tutorial shows
these VID/PID combinations has been used since 2014.
(Actually they've been registered officially earlier than 2014.)

For example, please check the reference tutorial:
https://mattboyer.github.io/PYaffs/2014/07/31/Hacklog%233.html
You can check lsusb dump in this picture.
https://mattboyer.github.io/PYaffs/images/03_device_usbview.png
Both VID/PID of Mediatek BROM and Preloader can be found in INF file
pasted in this tutorial, too.
Sorry for the proprietary information policy I cannot paste them all,
but the VID/PID set of BROM and Preloader are just exactly the same
as described in this tutorial.

Thanks!

Regards,
Macpaul Lin

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

[macpaul.lin]

On Tue, 2018-12-18 at 23:42 +0700, Lars Melin wrote:
> On 12/18/2018 22:19, Macpaul Lin wrote:
> 
> 
> > Hi all,
> > 
> > After double check the PID/VID officially used (registered) by Mediatek
> > Inc. The following VID/PID maps to the corresponding USB interface of
> > the firmware of cell phone.
> > VID:0x0e8d, PID:0x0003: Mediatek BROM.
> > VID:0x0e8d, PID:0x2000: Mediatek Preloader.
> > 
> > I will update Patch v6 for changing Andrey's previous comments.
> > 
> 
> You should show us a verbose lsusb of your 0e8d:0003 before changing
> Andrey's submission.
> 
> 
> Season's Greeting
> Lars

Hi Lars, Oliver, and Johan,

Well, it's about 1:30 a.m. here in Taiwan. I can do lsusb dump tomorrow
in Mediatek's office.
But there are a lot of tutorials about hacking and flash method of
customized firmware over the WWW. I can find one of the tutorial shows
these VID/PID combinations has been used since 2014.
(Actually they've been registered officially earlier than 2014.)

For example, please check the reference tutorial:
https://mattboyer.github.io/PYaffs/2014/07/31/Hacklog%233.html
You can check lsusb dump in this picture.
https://mattboyer.github.io/PYaffs/images/03_device_usbview.png
Both VID/PID of Mediatek BROM and Preloader can be found in INF file
pasted in this tutorial, too.
Sorry for the proprietary information policy I cannot paste them all,
but the VID/PID set of BROM and Preloader are just exactly the same
as described in this tutorial.

Thanks!

Regards,
Macpaul Lin

Re: [PATCH v5] cdc-acm: fix abnormal DATA RX issue for Mediatek Preloader.

[Lars Melin]

On 12/19/2018 00:48, Macpaul Lin wrote:

> Hi Lars, Oliver, and Johan,
> 
> Well, it's about 1:30 a.m. here in Taiwan. I can do lsusb dump tomorrow
> in Mediatek's office.
> But there are a lot of tutorials about hacking and flash method of
> customized firmware over the WWW. I can find one of the tutorial shows
> these VID/PID combinations has been used since 2014.
> (Actually they've been registered officially earlier than 2014.)
> 
> For example, please check the reference tutorial:
> https://mattboyer.github.io/PYaffs/2014/07/31/Hacklog%233.html
> You can check lsusb dump in this picture.
> https://mattboyer.github.io/PYaffs/images/03_device_usbview.png
> Both VID/PID of Mediatek BROM and Preloader can be found in INF file


Hi Macpaul,

the picture of an lsusb dump is not complete and does not show if there 
are union descriptors or if they are missing.
Since you remove Andrey's NO UNION DESCRIPTOR quirk then you have to 
show that there are union descriptors in your device.
If there are union descriptors in it then Mediatek has used the same 
vid:pid for two devices that differs from each other.

/Lars

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

[Lars Melin]

On 12/19/2018 00:48, Macpaul Lin wrote:

> Hi Lars, Oliver, and Johan,
> 
> Well, it's about 1:30 a.m. here in Taiwan. I can do lsusb dump tomorrow
> in Mediatek's office.
> But there are a lot of tutorials about hacking and flash method of
> customized firmware over the WWW. I can find one of the tutorial shows
> these VID/PID combinations has been used since 2014.
> (Actually they've been registered officially earlier than 2014.)
> 
> For example, please check the reference tutorial:
> https://mattboyer.github.io/PYaffs/2014/07/31/Hacklog%233.html
> You can check lsusb dump in this picture.
> https://mattboyer.github.io/PYaffs/images/03_device_usbview.png
> Both VID/PID of Mediatek BROM and Preloader can be found in INF file


Hi Macpaul,

the picture of an lsusb dump is not complete and does not show if there 
are union descriptors or if they are missing.
Since you remove Andrey's NO UNION DESCRIPTOR quirk then you have to 
show that there are union descriptors in your device.
If there are union descriptors in it then Mediatek has used the same 
vid:pid for two devices that differs from each other.

/Lars

Re: [PATCH v5] cdc-acm: fix abnormal DATA RX issue for Mediatek Preloader.

[Macpaul Lin]

On Wed, 2018-12-19 at 01:48 +0800, Macpaul Lin wrote:
> On Tue, 2018-12-18 at 23:42 +0700, Lars Melin wrote:
> > On 12/18/2018 22:19, Macpaul Lin wrote:
> > 
> > 
> > > Hi all,
> > > 
> > > After double check the PID/VID officially used (registered) by Mediatek
> > > Inc. The following VID/PID maps to the corresponding USB interface of
> > > the firmware of cell phone.
> > > VID:0x0e8d, PID:0x0003: Mediatek BROM.
> > > VID:0x0e8d, PID:0x2000: Mediatek Preloader.
> > > 
> > > I will update Patch v6 for changing Andrey's previous comments.
> > > 
> > 
> > You should show us a verbose lsusb of your 0e8d:0003 before changing
> > Andrey's submission.
> > 
> > 
> > Season's Greeting
> > Lars
> 
> Hi Lars, Oliver, and Johan,
> 
> Well, it's about 1:30 a.m. here in Taiwan. I can do lsusb dump tomorrow
> in Mediatek's office.
> But there are a lot of tutorials about hacking and flash method of
> customized firmware over the WWW. I can find one of the tutorial shows
> these VID/PID combinations has been used since 2014.
> (Actually they've been registered officially earlier than 2014.)
> 
> For example, please check the reference tutorial:
> https://mattboyer.github.io/PYaffs/2014/07/31/Hacklog%233.html
> You can check lsusb dump in this picture.
> https://mattboyer.github.io/PYaffs/images/03_device_usbview.png
> Both VID/PID of Mediatek BROM and Preloader can be found in INF file
> pasted in this tutorial, too.
> Sorry for the proprietary information policy I cannot paste them all,
> but the VID/PID set of BROM and Preloader are just exactly the same
> as described in this tutorial.
> 

Hi all,
Here is the lsusb dump of the BROM and Preloader state of the phone.

lsusb of BROM:
Bus 001 Device 012: ID 0e8d:0003 MediaTek Inc. MT6227 phone

lsusb of Preloader:
Bus 001 Device 011: ID 0e8d:2000 MediaTek Inc. MT65xx Preloader

dmesg of BROM:
[52138.849233] usb 1-1: new high-speed USB device number 12 using
ehci-pci
[52139.252557] usb 1-1: New USB device found, idVendor=0e8d,
idProduct=0003
[52139.252559] usb 1-1: New USB device strings: Mfr=0, Product=0,
SerialNumber=0
[52139.260179] cdc_acm 1-1:1.0: ttyACM0: USB ACM device

dmesg of Preloader:
[52145.052952] usb 1-1: new high-speed USB device number 13 using
ehci-pci
[52145.410432] usb 1-1: New USB device found, idVendor=0e8d,
idProduct=2000
[52145.410434] usb 1-1: New USB device strings: Mfr=1, Product=2,
SerialNumber=0
[52145.410435] usb 1-1: Product: MT65xx Preloader
[52145.410437] usb 1-1: Manufacturer: MediaTek

Regards,
Macpaul Lin

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

[macpaul.lin]

On Wed, 2018-12-19 at 01:48 +0800, Macpaul Lin wrote:
> On Tue, 2018-12-18 at 23:42 +0700, Lars Melin wrote:
> > On 12/18/2018 22:19, Macpaul Lin wrote:
> > 
> > 
> > > Hi all,
> > > 
> > > After double check the PID/VID officially used (registered) by Mediatek
> > > Inc. The following VID/PID maps to the corresponding USB interface of
> > > the firmware of cell phone.
> > > VID:0x0e8d, PID:0x0003: Mediatek BROM.
> > > VID:0x0e8d, PID:0x2000: Mediatek Preloader.
> > > 
> > > I will update Patch v6 for changing Andrey's previous comments.
> > > 
> > 
> > You should show us a verbose lsusb of your 0e8d:0003 before changing
> > Andrey's submission.
> > 
> > 
> > Season's Greeting
> > Lars
> 
> Hi Lars, Oliver, and Johan,
> 
> Well, it's about 1:30 a.m. here in Taiwan. I can do lsusb dump tomorrow
> in Mediatek's office.
> But there are a lot of tutorials about hacking and flash method of
> customized firmware over the WWW. I can find one of the tutorial shows
> these VID/PID combinations has been used since 2014.
> (Actually they've been registered officially earlier than 2014.)
> 
> For example, please check the reference tutorial:
> https://mattboyer.github.io/PYaffs/2014/07/31/Hacklog%233.html
> You can check lsusb dump in this picture.
> https://mattboyer.github.io/PYaffs/images/03_device_usbview.png
> Both VID/PID of Mediatek BROM and Preloader can be found in INF file
> pasted in this tutorial, too.
> Sorry for the proprietary information policy I cannot paste them all,
> but the VID/PID set of BROM and Preloader are just exactly the same
> as described in this tutorial.
> 

Hi all,
Here is the lsusb dump of the BROM and Preloader state of the phone.

lsusb of BROM:
Bus 001 Device 012: ID 0e8d:0003 MediaTek Inc. MT6227 phone

lsusb of Preloader:
Bus 001 Device 011: ID 0e8d:2000 MediaTek Inc. MT65xx Preloader

dmesg of BROM:
[52138.849233] usb 1-1: new high-speed USB device number 12 using
ehci-pci
[52139.252557] usb 1-1: New USB device found, idVendor=0e8d,
idProduct=0003
[52139.252559] usb 1-1: New USB device strings: Mfr=0, Product=0,
SerialNumber=0
[52139.260179] cdc_acm 1-1:1.0: ttyACM0: USB ACM device

dmesg of Preloader:
[52145.052952] usb 1-1: new high-speed USB device number 13 using
ehci-pci
[52145.410432] usb 1-1: New USB device found, idVendor=0e8d,
idProduct=2000
[52145.410434] usb 1-1: New USB device strings: Mfr=1, Product=2,
SerialNumber=0
[52145.410435] usb 1-1: Product: MT65xx Preloader
[52145.410437] usb 1-1: Manufacturer: MediaTek

Regards,
Macpaul Lin

Re: [PATCH v5] cdc-acm: fix abnormal DATA RX issue for Mediatek Preloader.

[Macpaul Lin]

On Wed, 2018-12-19 at 10:22 +0800, Macpaul Lin wrote:
> On Wed, 2018-12-19 at 01:48 +0800, Macpaul Lin wrote:
> > On Tue, 2018-12-18 at 23:42 +0700, Lars Melin wrote:
> > > On 12/18/2018 22:19, Macpaul Lin wrote:
> > > 
> > > 
> > > > Hi all,
> > > > 
> > > > After double check the PID/VID officially used (registered) by Mediatek
> > > > Inc. The following VID/PID maps to the corresponding USB interface of
> > > > the firmware of cell phone.
> > > > VID:0x0e8d, PID:0x0003: Mediatek BROM.
> > > > VID:0x0e8d, PID:0x2000: Mediatek Preloader.
> > > > 
> > > > I will update Patch v6 for changing Andrey's previous comments.
> > > > 
> > > 
> > > You should show us a verbose lsusb of your 0e8d:0003 before changing
> > > Andrey's submission.
> > > 
> > > 
> > > Season's Greeting
> > > Lars
> > 
Hi Lars,

Sorry I just missed your explain about the UNION descriptor and did not 
found in my mail box.
Here comes the verbose lsusb -v dump.

Bus 001 Device 003: ID 0e8d:0003 MediaTek Inc. MT6227 phone
[432/3160]
Device Descriptor:
  bLength                18
  bDescriptorType         1
  bcdUSB               1.10
  bDeviceClass            2 Communications
  bDeviceSubClass         0
  bDeviceProtocol         0
  bMaxPacketSize0        64
  idVendor           0x0e8d MediaTek Inc.
  idProduct          0x0003 MT6227 phone
  bcdDevice            1.00
  iManufacturer           0
  iProduct                0
  iSerial                 0
  bNumConfigurations      1
  Configuration Descriptor:
    bLength                 9
    bDescriptorType         2
    wTotalLength           67
    bNumInterfaces          2
    bConfigurationValue     1
    iConfiguration          0
    bmAttributes         0x80
      (Bus Powered)
    MaxPower                0mA
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        0
      bAlternateSetting       0
      bNumEndpoints           1
      bInterfaceClass         2 Communications
      bInterfaceSubClass      2 Abstract (modem)
      bInterfaceProtocol      1 AT-commands (v.25ter)
      iInterface              1 (error)
      CDC Header:
        bcdCDC               1.10
      CDC ACM:
        bmCapabilities       0x0f
          connection notifications
          sends break
          line coding and serial state
          get/set/clear comm features
      CDC Union:
        bMasterInterface        0
        bSlaveInterface         1
      CDC Call Management:
        bmCapabilities       0x03
          call management
          use DataInterface
        bDataInterface          1
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x84  EP 4 IN
        bmAttributes            3
          Transfer Type            Interrupt
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0040  1x 64 bytes
        bInterval               1
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        1
      bAlternateSetting       0
      bNumEndpoints           2
      bInterfaceClass        10 CDC Data
      bInterfaceSubClass      0 Unused
      bInterfaceProtocol      0
      iInterface              2 (error)
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x81  EP 1 IN
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0200  1x 512 bytes
        bInterval               0
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x01  EP 1 OUT
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0200  1x 512 bytes
        bInterval               0
Device Status:     0x7410
  (Bus Powered)

Bus 001 Device 002: ID 0e8d:2000 MediaTek Inc. MT65xx Preloader
Device Descriptor:
  bLength                18
  bDescriptorType         1
  bcdUSB               2.00
  bDeviceClass            2 Communications
  bDeviceSubClass         0
  bDeviceProtocol         0
  bMaxPacketSize0        64
  idVendor           0x0e8d MediaTek Inc.
  idProduct          0x2000 MT65xx Preloader
  bcdDevice            1.00
  iManufacturer           1 (error)
  iProduct                2 (error)
  iSerial                 0
  bNumConfigurations      1
  Configuration Descriptor:
    bLength                 9
    bDescriptorType         2
    wTotalLength           70
    bNumInterfaces          2
    bConfigurationValue     1
    iConfiguration          3 (error)
    bmAttributes         0xc0
      Self Powered
    MaxPower              500mA
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        1
      bAlternateSetting       0
      bNumEndpoints           2
      bInterfaceClass        10 CDC Data
      bInterfaceSubClass      0 Unused
      bInterfaceProtocol      0
      iInterface              4 (error)
      Endpoint Descriptor:
        bLength                 8
        bDescriptorType         5
        bEndpointAddress     0x01  EP 1 OUT
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0200  1x 512 bytes
        bInterval               0
      Endpoint Descriptor:
        bLength                 8
        bDescriptorType         5
        bEndpointAddress     0x81  EP 1 IN
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0200  1x 512 bytes
        bInterval               0
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        0
      bAlternateSetting       0
      bNumEndpoints           1
      bInterfaceClass         2 Communications
      bInterfaceSubClass      2 Abstract (modem)
      bInterfaceProtocol      1 AT-commands (v.25ter)
      iInterface              5 (error)
      CDC Header:
        bcdCDC               1.10
      CDC ACM:
        bmCapabilities       0x0f
          connection notifications
          sends break
          line coding and serial state
          get/set/clear comm features
      CDC Union:
        bMasterInterface        1
        bSlaveInterface         0
      CDC Call Management:
        bmCapabilities       0x03
          call management
          use DataInterface
        bDataInterface          0
      Endpoint Descriptor:
        bLength                 8
        bDescriptorType         5
        bEndpointAddress     0x83  EP 3 IN
        bmAttributes            3
          Transfer Type            Interrupt
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0040  1x 64 bytes
        bInterval              16
Device Status:     0x13f0
  (Bus Powered)
  Debug Mode

Regards,
Macpaul Lin

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

[macpaul.lin]

On Wed, 2018-12-19 at 10:22 +0800, Macpaul Lin wrote:
> On Wed, 2018-12-19 at 01:48 +0800, Macpaul Lin wrote:
> > On Tue, 2018-12-18 at 23:42 +0700, Lars Melin wrote:
> > > On 12/18/2018 22:19, Macpaul Lin wrote:
> > > 
> > > 
> > > > Hi all,
> > > > 
> > > > After double check the PID/VID officially used (registered) by Mediatek
> > > > Inc. The following VID/PID maps to the corresponding USB interface of
> > > > the firmware of cell phone.
> > > > VID:0x0e8d, PID:0x0003: Mediatek BROM.
> > > > VID:0x0e8d, PID:0x2000: Mediatek Preloader.
> > > > 
> > > > I will update Patch v6 for changing Andrey's previous comments.
> > > > 
> > > 
> > > You should show us a verbose lsusb of your 0e8d:0003 before changing
> > > Andrey's submission.
> > > 
> > > 
> > > Season's Greeting
> > > Lars
> > 
Hi Lars,

Sorry I just missed your explain about the UNION descriptor and did not 
found in my mail box.
Here comes the verbose lsusb -v dump.

Bus 001 Device 003: ID 0e8d:0003 MediaTek Inc. MT6227 phone
[432/3160]
Device Descriptor:
  bLength                18
  bDescriptorType         1
  bcdUSB               1.10
  bDeviceClass            2 Communications
  bDeviceSubClass         0
  bDeviceProtocol         0
  bMaxPacketSize0        64
  idVendor           0x0e8d MediaTek Inc.
  idProduct          0x0003 MT6227 phone
  bcdDevice            1.00
  iManufacturer           0
  iProduct                0
  iSerial                 0
  bNumConfigurations      1
  Configuration Descriptor:
    bLength                 9
    bDescriptorType         2
    wTotalLength           67
    bNumInterfaces          2
    bConfigurationValue     1
    iConfiguration          0
    bmAttributes         0x80
      (Bus Powered)
    MaxPower                0mA
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        0
      bAlternateSetting       0
      bNumEndpoints           1
      bInterfaceClass         2 Communications
      bInterfaceSubClass      2 Abstract (modem)
      bInterfaceProtocol      1 AT-commands (v.25ter)
      iInterface              1 (error)
      CDC Header:
        bcdCDC               1.10
      CDC ACM:
        bmCapabilities       0x0f
          connection notifications
          sends break
          line coding and serial state
          get/set/clear comm features
      CDC Union:
        bMasterInterface        0
        bSlaveInterface         1
      CDC Call Management:
        bmCapabilities       0x03
          call management
          use DataInterface
        bDataInterface          1
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x84  EP 4 IN
        bmAttributes            3
          Transfer Type            Interrupt
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0040  1x 64 bytes
        bInterval               1
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        1
      bAlternateSetting       0
      bNumEndpoints           2
      bInterfaceClass        10 CDC Data
      bInterfaceSubClass      0 Unused
      bInterfaceProtocol      0
      iInterface              2 (error)
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x81  EP 1 IN
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0200  1x 512 bytes
        bInterval               0
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x01  EP 1 OUT
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0200  1x 512 bytes
        bInterval               0
Device Status:     0x7410
  (Bus Powered)

Bus 001 Device 002: ID 0e8d:2000 MediaTek Inc. MT65xx Preloader
Device Descriptor:
  bLength                18
  bDescriptorType         1
  bcdUSB               2.00
  bDeviceClass            2 Communications
  bDeviceSubClass         0
  bDeviceProtocol         0
  bMaxPacketSize0        64
  idVendor           0x0e8d MediaTek Inc.
  idProduct          0x2000 MT65xx Preloader
  bcdDevice            1.00
  iManufacturer           1 (error)
  iProduct                2 (error)
  iSerial                 0
  bNumConfigurations      1
  Configuration Descriptor:
    bLength                 9
    bDescriptorType         2
    wTotalLength           70
    bNumInterfaces          2
    bConfigurationValue     1
    iConfiguration          3 (error)
    bmAttributes         0xc0
      Self Powered
    MaxPower              500mA
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        1
      bAlternateSetting       0
      bNumEndpoints           2
      bInterfaceClass        10 CDC Data
      bInterfaceSubClass      0 Unused
      bInterfaceProtocol      0
      iInterface              4 (error)
      Endpoint Descriptor:
        bLength                 8
        bDescriptorType         5
        bEndpointAddress     0x01  EP 1 OUT
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0200  1x 512 bytes
        bInterval               0
      Endpoint Descriptor:
        bLength                 8
        bDescriptorType         5
        bEndpointAddress     0x81  EP 1 IN
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0200  1x 512 bytes
        bInterval               0
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        0
      bAlternateSetting       0
      bNumEndpoints           1
      bInterfaceClass         2 Communications
      bInterfaceSubClass      2 Abstract (modem)
      bInterfaceProtocol      1 AT-commands (v.25ter)
      iInterface              5 (error)
      CDC Header:
        bcdCDC               1.10
      CDC ACM:
        bmCapabilities       0x0f
          connection notifications
          sends break
          line coding and serial state
          get/set/clear comm features
      CDC Union:
        bMasterInterface        1
        bSlaveInterface         0
      CDC Call Management:
        bmCapabilities       0x03
          call management
          use DataInterface
        bDataInterface          0
      Endpoint Descriptor:
        bLength                 8
        bDescriptorType         5
        bEndpointAddress     0x83  EP 3 IN
        bmAttributes            3
          Transfer Type            Interrupt
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0040  1x 64 bytes
        bInterval              16
Device Status:     0x13f0
  (Bus Powered)
  Debug Mode

Regards,
Macpaul Lin

Re: [PATCH v5] cdc-acm: fix abnormal DATA RX issue for Mediatek Preloader.

[Lars Melin]

On 12/19/2018 10:16, Macpaul Lin wrote:

.
> Here comes the verbose lsusb -v dump.
> 
> Bus 001 Device 003: ID 0e8d:0003 MediaTek Inc. MT6227 phone
> [432/3160]
> Device Descriptor:
>    bLength                18
>    bDescriptorType         1
>    bcdUSB               1.10
>    bDeviceClass            2 Communications
>    bDeviceSubClass         0
>    bDeviceProtocol         0
>    bMaxPacketSize0        64
>    idVendor           0x0e8d MediaTek Inc.
>    idProduct          0x0003 MT6227 phone
>    bcdDevice            1.00
>    iManufacturer           0
>    iProduct                0
>    iSerial                 0
>    bNumConfigurations      1
>    Configuration Descriptor:
>      bLength                 9
>      bDescriptorType         2
>      wTotalLength           67
>      bNumInterfaces          2
>      bConfigurationValue     1
>      iConfiguration          0
>      bmAttributes         0x80
>        (Bus Powered)
>      MaxPower                0mA
>      Interface Descriptor:
>        bLength                 9
>        bDescriptorType         4
>        bInterfaceNumber        0
>        bAlternateSetting       0
>        bNumEndpoints           1
>        bInterfaceClass         2 Communications
>        bInterfaceSubClass      2 Abstract (modem)
>        bInterfaceProtocol      1 AT-commands (v.25ter)
>        iInterface              1 (error)
>        CDC Header:
>          bcdCDC               1.10
>        CDC ACM:
>          bmCapabilities       0x0f
>            connection notifications
>            sends break
>            line coding and serial state
>            get/set/clear comm features
>        CDC Union:
>          bMasterInterface        0
>          bSlaveInterface         1
>        CDC Call Management:
>          bmCapabilities       0x03
>            call management
>            use DataInterface
>          bDataInterface          1
>        Endpoint Descriptor:
>          bLength                 7
>          bDescriptorType         5
>          bEndpointAddress     0x84  EP 4 IN
>          bmAttributes            3
>            Transfer Type            Interrupt
>            Synch Type               None
>            Usage Type               Data
>          wMaxPacketSize     0x0040  1x 64 bytes
>          bInterval               1
>      Interface Descriptor:
>        bLength                 9
>        bDescriptorType         4
>        bInterfaceNumber        1
>        bAlternateSetting       0
>        bNumEndpoints           2
>        bInterfaceClass        10 CDC Data
>        bInterfaceSubClass      0 Unused
>        bInterfaceProtocol      0
>        iInterface              2 (error)
>        Endpoint Descriptor:
>          bLength                 7
>          bDescriptorType         5
>          bEndpointAddress     0x81  EP 1 IN
>          bmAttributes            2
>            Transfer Type            Bulk
>            Synch Type               None
>            Usage Type               Data
>          wMaxPacketSize     0x0200  1x 512 bytes
>          bInterval               0
>        Endpoint Descriptor:
>          bLength                 7
>          bDescriptorType         5
>          bEndpointAddress     0x01  EP 1 OUT
>          bmAttributes            2
>            Transfer Type            Bulk
>            Synch Type               None
>            Usage Type               Data
>          wMaxPacketSize     0x0200  1x 512 bytes
>          bInterval               0
> Device Status:     0x7410
>    (Bus Powered)

> Regards,
> Macpaul Lin
> 

Hi Macpaul,
your verbose usb listing show me that Mediatek has made two different 
0e8d:003 devices, see my verbose lsusb listing below.
(Notice also the reverse order for cmd and data interfaces in it 
compared to yours).
USB id's are intended to identify a device and its needs so there should
never be more than one unique device per id.


Fairphone FP-1, MT6227  (no CDC union !!!)


MI_00 USB Single Port


Bus 001 Device 004: ID 0e8d:0003
Device Descriptor:
   bLength                18
   bDescriptorType         1
   bcdUSB               1.10
   bDeviceClass            2 Communications
   bDeviceSubClass         0
   bDeviceProtocol         0
   bMaxPacketSize0         8
   idVendor           0x0e8d
   idProduct          0x0003
   bcdDevice            0.01
   iManufacturer           3 MediaTek Inc
   iProduct                4 SEATTLE
   iSerial                 5 534574001004
   bNumConfigurations      1
   Configuration Descriptor:
     bLength                 9
     bDescriptorType         2
     wTotalLength           67
     bNumInterfaces          2
     bConfigurationValue     1
     iConfiguration          0
     bmAttributes         0x80
       (Bus Powered)
     MaxPower              500mA

     Interface Descriptor:
       bLength                 9
       bDescriptorType         4
       bInterfaceNumber        0
       bAlternateSetting       0
       bNumEndpoints           2
       bInterfaceClass        10 Data
       bInterfaceSubClass      0 Unused
       bInterfaceProtocol      0
       iInterface              1 6218B COM
       Endpoint Descriptor:
         bLength                 7
         bDescriptorType         5
         bEndpointAddress     0x81  EP 1 IN
         bmAttributes            2
           Transfer Type            Bulk
           Synch Type               None
           Usage Type               Data
         wMaxPacketSize     0x0040  1x 64 bytes
         bInterval               0
       Endpoint Descriptor:
         bLength                 7
         bDescriptorType         5
         bEndpointAddress     0x01  EP 1 OUT
         bmAttributes            2
           Transfer Type            Bulk
           Synch Type               None
           Usage Type               Data
         wMaxPacketSize     0x0040  1x 64 bytes
         bInterval               0

     Interface Descriptor:
       bLength                28
       bDescriptorType         4
       bInterfaceNumber        1
       bAlternateSetting       0
       bNumEndpoints           1
       bInterfaceClass         2 Communications
       bInterfaceSubClass      2 Abstract (modem)
       bInterfaceProtocol      1 AT-commands (v.25ter)
       iInterface              2 6218B COM
       Endpoint Descriptor:
         bLength                 7
         bDescriptorType         5
         bEndpointAddress     0x83  EP 3 IN
         bmAttributes            3
           Transfer Type            Interrupt
           Synch Type               None
           Usage Type               Data
         wMaxPacketSize     0x0008  1x 8 bytes
         bInterval               1
Device Status:     0x0000
   (Bus Powered)


/Lars

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

[Lars Melin]

On 12/19/2018 10:16, Macpaul Lin wrote:

.
> Here comes the verbose lsusb -v dump.
> 
> Bus 001 Device 003: ID 0e8d:0003 MediaTek Inc. MT6227 phone
> [432/3160]
> Device Descriptor:
>    bLength                18
>    bDescriptorType         1
>    bcdUSB               1.10
>    bDeviceClass            2 Communications
>    bDeviceSubClass         0
>    bDeviceProtocol         0
>    bMaxPacketSize0        64
>    idVendor           0x0e8d MediaTek Inc.
>    idProduct          0x0003 MT6227 phone
>    bcdDevice            1.00
>    iManufacturer           0
>    iProduct                0
>    iSerial                 0
>    bNumConfigurations      1
>    Configuration Descriptor:
>      bLength                 9
>      bDescriptorType         2
>      wTotalLength           67
>      bNumInterfaces          2
>      bConfigurationValue     1
>      iConfiguration          0
>      bmAttributes         0x80
>        (Bus Powered)
>      MaxPower                0mA
>      Interface Descriptor:
>        bLength                 9
>        bDescriptorType         4
>        bInterfaceNumber        0
>        bAlternateSetting       0
>        bNumEndpoints           1
>        bInterfaceClass         2 Communications
>        bInterfaceSubClass      2 Abstract (modem)
>        bInterfaceProtocol      1 AT-commands (v.25ter)
>        iInterface              1 (error)
>        CDC Header:
>          bcdCDC               1.10
>        CDC ACM:
>          bmCapabilities       0x0f
>            connection notifications
>            sends break
>            line coding and serial state
>            get/set/clear comm features
>        CDC Union:
>          bMasterInterface        0
>          bSlaveInterface         1
>        CDC Call Management:
>          bmCapabilities       0x03
>            call management
>            use DataInterface
>          bDataInterface          1
>        Endpoint Descriptor:
>          bLength                 7
>          bDescriptorType         5
>          bEndpointAddress     0x84  EP 4 IN
>          bmAttributes            3
>            Transfer Type            Interrupt
>            Synch Type               None
>            Usage Type               Data
>          wMaxPacketSize     0x0040  1x 64 bytes
>          bInterval               1
>      Interface Descriptor:
>        bLength                 9
>        bDescriptorType         4
>        bInterfaceNumber        1
>        bAlternateSetting       0
>        bNumEndpoints           2
>        bInterfaceClass        10 CDC Data
>        bInterfaceSubClass      0 Unused
>        bInterfaceProtocol      0
>        iInterface              2 (error)
>        Endpoint Descriptor:
>          bLength                 7
>          bDescriptorType         5
>          bEndpointAddress     0x81  EP 1 IN
>          bmAttributes            2
>            Transfer Type            Bulk
>            Synch Type               None
>            Usage Type               Data
>          wMaxPacketSize     0x0200  1x 512 bytes
>          bInterval               0
>        Endpoint Descriptor:
>          bLength                 7
>          bDescriptorType         5
>          bEndpointAddress     0x01  EP 1 OUT
>          bmAttributes            2
>            Transfer Type            Bulk
>            Synch Type               None
>            Usage Type               Data
>          wMaxPacketSize     0x0200  1x 512 bytes
>          bInterval               0
> Device Status:     0x7410
>    (Bus Powered)

> Regards,
> Macpaul Lin
> 

Hi Macpaul,
your verbose usb listing show me that Mediatek has made two different 
0e8d:003 devices, see my verbose lsusb listing below.
(Notice also the reverse order for cmd and data interfaces in it 
compared to yours).
USB id's are intended to identify a device and its needs so there should
never be more than one unique device per id.


Fairphone FP-1, MT6227  (no CDC union !!!)


MI_00 USB Single Port


Bus 001 Device 004: ID 0e8d:0003
Device Descriptor:
   bLength                18
   bDescriptorType         1
   bcdUSB               1.10
   bDeviceClass            2 Communications
   bDeviceSubClass         0
   bDeviceProtocol         0
   bMaxPacketSize0         8
   idVendor           0x0e8d
   idProduct          0x0003
   bcdDevice            0.01
   iManufacturer           3 MediaTek Inc
   iProduct                4 SEATTLE
   iSerial                 5 534574001004
   bNumConfigurations      1
   Configuration Descriptor:
     bLength                 9
     bDescriptorType         2
     wTotalLength           67
     bNumInterfaces          2
     bConfigurationValue     1
     iConfiguration          0
     bmAttributes         0x80
       (Bus Powered)
     MaxPower              500mA

     Interface Descriptor:
       bLength                 9
       bDescriptorType         4
       bInterfaceNumber        0
       bAlternateSetting       0
       bNumEndpoints           2
       bInterfaceClass        10 Data
       bInterfaceSubClass      0 Unused
       bInterfaceProtocol      0
       iInterface              1 6218B COM
       Endpoint Descriptor:
         bLength                 7
         bDescriptorType         5
         bEndpointAddress     0x81  EP 1 IN
         bmAttributes            2
           Transfer Type            Bulk
           Synch Type               None
           Usage Type               Data
         wMaxPacketSize     0x0040  1x 64 bytes
         bInterval               0
       Endpoint Descriptor:
         bLength                 7
         bDescriptorType         5
         bEndpointAddress     0x01  EP 1 OUT
         bmAttributes            2
           Transfer Type            Bulk
           Synch Type               None
           Usage Type               Data
         wMaxPacketSize     0x0040  1x 64 bytes
         bInterval               0

     Interface Descriptor:
       bLength                28
       bDescriptorType         4
       bInterfaceNumber        1
       bAlternateSetting       0
       bNumEndpoints           1
       bInterfaceClass         2 Communications
       bInterfaceSubClass      2 Abstract (modem)
       bInterfaceProtocol      1 AT-commands (v.25ter)
       iInterface              2 6218B COM
       Endpoint Descriptor:
         bLength                 7
         bDescriptorType         5
         bEndpointAddress     0x83  EP 3 IN
         bmAttributes            3
           Transfer Type            Interrupt
           Synch Type               None
           Usage Type               Data
         wMaxPacketSize     0x0008  1x 8 bytes
         bInterval               1
Device Status:     0x0000
   (Bus Powered)


/Lars

Re: [PATCH v5] cdc-acm: fix abnormal DATA RX issue for Mediatek Preloader.

[Macpaul Lin]

On Wed, 2018-12-19 at 10:31 +0700, Lars Melin wrote:
> On 12/19/2018 10:16, Macpaul Lin wrote: 
> 
> Hi Macpaul,
> your verbose usb listing show me that Mediatek has made two different 
> 0e8d:003 devices, see my verbose lsusb listing below.
> (Notice also the reverse order for cmd and data interfaces in it 
> compared to yours).
> USB id's are intended to identify a device and its needs so there should
> never be more than one unique device per id.
> 
> 
> Fairphone FP-1, MT6227  (no CDC union !!!)
> 
Hi Lars,

Ha ha ha, it is a little bit embarrassing.
What I've used to capture verbose log is MT6765 platform.
Then I've checked Fairphone FP-1, which is MT6589 a pretty old platform.
The BROM (boot ROM) has been maintained by other teams and will vary by
different SoC project in Mediatek. I'm not sure why they changed the
descriptors.

For the consistency of BROM's behavior, I'll update a new patch keeps
PID:0003 remain untouched. I'll trying to report it to BROM team and see
if they have any action on this issue.

Regards,
Macpaul Lin

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

[macpaul.lin]

On Wed, 2018-12-19 at 10:31 +0700, Lars Melin wrote:
> On 12/19/2018 10:16, Macpaul Lin wrote: 
> 
> Hi Macpaul,
> your verbose usb listing show me that Mediatek has made two different 
> 0e8d:003 devices, see my verbose lsusb listing below.
> (Notice also the reverse order for cmd and data interfaces in it 
> compared to yours).
> USB id's are intended to identify a device and its needs so there should
> never be more than one unique device per id.
> 
> 
> Fairphone FP-1, MT6227  (no CDC union !!!)
> 
Hi Lars,

Ha ha ha, it is a little bit embarrassing.
What I've used to capture verbose log is MT6765 platform.
Then I've checked Fairphone FP-1, which is MT6589 a pretty old platform.
The BROM (boot ROM) has been maintained by other teams and will vary by
different SoC project in Mediatek. I'm not sure why they changed the
descriptors.

For the consistency of BROM's behavior, I'll update a new patch keeps
PID:0003 remain untouched. I'll trying to report it to BROM team and see
if they have any action on this issue.

Regards,
Macpaul Lin

[PATCH v7] cdc-acm: fix abnormal DATA RX issue for Mediatek Preloader.

[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. 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@mediatek.com>
Cc: stable@vger.kernel.org
---
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.
Changes for v6:
 - Update VID/PID:0x0e8d/0x0003 as Mediatek Inc BROM.
 - Update VID/PID:0x0e8d/0x2000 as Mediatek Inc Preloader.
Changes for v7:
 - Keep VID/PID:0x0e8d/0x0003 unchanged because of 2 different UNION
   descriptor implementated in Mediatek Inc BROM (MT6589/MT6765).

 drivers/usb/class/cdc-acm.c | 10 ++++++++++
 drivers/usb/class/cdc-acm.h |  1 +
 2 files changed, 11 insertions(+)

diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index 1b68fed..ed8c62b 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;
@@ -1657,6 +1664,9 @@ static int acm_pre_reset(struct usb_interface *intf)
 	{ USB_DEVICE(0x0e8d, 0x0003), /* FIREFLY, MediaTek Inc; andrey.arapov@gmail.com */
 	.driver_info = NO_UNION_NORMAL, /* has no union descriptor */
 	},
+	{ USB_DEVICE(0x0e8d, 0x2000), /* MediaTek Inc Preloader */
+	.driver_info = DISABLE_ECHO, /* DISABLE ECHO in termios flag */
+	},
 	{ USB_DEVICE(0x0e8d, 0x3329), /* MediaTek Inc GPS */
 	.driver_info = NO_UNION_NORMAL, /* has no union descriptor */
 	},
diff --git a/drivers/usb/class/cdc-acm.h b/drivers/usb/class/cdc-acm.h
index ca06b20..515aad0 100644
--- a/drivers/usb/class/cdc-acm.h
+++ b/drivers/usb/class/cdc-acm.h
@@ -140,3 +140,4 @@ struct acm {
 #define QUIRK_CONTROL_LINE_STATE	BIT(6)
 #define CLEAR_HALT_CONDITIONS		BIT(7)
 #define SEND_ZERO_PACKET		BIT(8)
+#define DISABLE_ECHO			BIT(9)
-- 
1.9.1

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

[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. 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@mediatek.com>
Cc: stable@vger.kernel.org
---
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.
Changes for v6:
 - Update VID/PID:0x0e8d/0x0003 as Mediatek Inc BROM.
 - Update VID/PID:0x0e8d/0x2000 as Mediatek Inc Preloader.
Changes for v7:
 - Keep VID/PID:0x0e8d/0x0003 unchanged because of 2 different UNION
   descriptor implementated in Mediatek Inc BROM (MT6589/MT6765).

 drivers/usb/class/cdc-acm.c | 10 ++++++++++
 drivers/usb/class/cdc-acm.h |  1 +
 2 files changed, 11 insertions(+)

diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index 1b68fed..ed8c62b 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;
@@ -1657,6 +1664,9 @@ static int acm_pre_reset(struct usb_interface *intf)
 	{ USB_DEVICE(0x0e8d, 0x0003), /* FIREFLY, MediaTek Inc; andrey.arapov@gmail.com */
 	.driver_info = NO_UNION_NORMAL, /* has no union descriptor */
 	},
+	{ USB_DEVICE(0x0e8d, 0x2000), /* MediaTek Inc Preloader */
+	.driver_info = DISABLE_ECHO, /* DISABLE ECHO in termios flag */
+	},
 	{ USB_DEVICE(0x0e8d, 0x3329), /* MediaTek Inc GPS */
 	.driver_info = NO_UNION_NORMAL, /* has no union descriptor */
 	},
diff --git a/drivers/usb/class/cdc-acm.h b/drivers/usb/class/cdc-acm.h
index ca06b20..515aad0 100644
--- a/drivers/usb/class/cdc-acm.h
+++ b/drivers/usb/class/cdc-acm.h
@@ -140,3 +140,4 @@ struct acm {
 #define QUIRK_CONTROL_LINE_STATE	BIT(6)
 #define CLEAR_HALT_CONDITIONS		BIT(7)
 #define SEND_ZERO_PACKET		BIT(8)
+#define DISABLE_ECHO			BIT(9)

Re: [PATCH v5] cdc-acm: fix abnormal DATA RX issue for Mediatek Preloader.

[Oliver Neukum]

On Mi, 2018-12-19 at 12:03 +0800, Macpaul Lin wrote:
> On Wed, 2018-12-19 at 10:31 +0700, Lars Melin wrote:
> > On 12/19/2018 10:16, Macpaul Lin wrote: 
> > 
> > Hi Macpaul,
> > your verbose usb listing show me that Mediatek has made two different 
> > 0e8d:003 devices, see my verbose lsusb listing below.
> > (Notice also the reverse order for cmd and data interfaces in it 
> > compared to yours).
> > USB id's are intended to identify a device and its needs so there should
> > never be more than one unique device per id.
> > 
> > 
> > Fairphone FP-1, MT6227  (no CDC union !!!)
> > 
> 
> Hi Lars,
> 
> Ha ha ha, it is a little bit embarrassing.
> What I've used to capture verbose log is MT6765 platform.
> Then I've checked Fairphone FP-1, which is MT6589 a pretty old platform.
> The BROM (boot ROM) has been maintained by other teams and will vary by
> different SoC project in Mediatek. I'm not sure why they changed the
> descriptors.
> 
> For the consistency of BROM's behavior, I'll update a new patch keeps
> PID:0003 remain untouched. I'll trying to report it to BROM team and see
> if they have any action on this issue.

Thank you all for taking care of these important issues.
Please submit that new patch.

	Regards
		Oliver

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

[Oliver Neukum]

On Mi, 2018-12-19 at 12:03 +0800, Macpaul Lin wrote:
> On Wed, 2018-12-19 at 10:31 +0700, Lars Melin wrote:
> > On 12/19/2018 10:16, Macpaul Lin wrote: 
> > 
> > Hi Macpaul,
> > your verbose usb listing show me that Mediatek has made two different 
> > 0e8d:003 devices, see my verbose lsusb listing below.
> > (Notice also the reverse order for cmd and data interfaces in it 
> > compared to yours).
> > USB id's are intended to identify a device and its needs so there should
> > never be more than one unique device per id.
> > 
> > 
> > Fairphone FP-1, MT6227  (no CDC union !!!)
> > 
> 
> Hi Lars,
> 
> Ha ha ha, it is a little bit embarrassing.
> What I've used to capture verbose log is MT6765 platform.
> Then I've checked Fairphone FP-1, which is MT6589 a pretty old platform.
> The BROM (boot ROM) has been maintained by other teams and will vary by
> different SoC project in Mediatek. I'm not sure why they changed the
> descriptors.
> 
> For the consistency of BROM's behavior, I'll update a new patch keeps
> PID:0003 remain untouched. I'll trying to report it to BROM team and see
> if they have any action on this issue.

Thank you all for taking care of these important issues.
Please submit that new patch.

	Regards
		Oliver

Re: [PATCH v7] cdc-acm: fix abnormal DATA RX issue for Mediatek Preloader.

[Johan Hovold]

On Wed, Dec 19, 2018 at 12:11:03PM +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@mediatek.com>
> Cc: stable@vger.kernel.org
> ---
> 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.
> Changes for v6:
>  - Update VID/PID:0x0e8d/0x0003 as Mediatek Inc BROM.
>  - Update VID/PID:0x0e8d/0x2000 as Mediatek Inc Preloader.
> Changes for v7:
>  - Keep VID/PID:0x0e8d/0x0003 unchanged because of 2 different UNION
>    descriptor implementated in Mediatek Inc BROM (MT6589/MT6765).

Reviewed-by: Johan Hovold <johan@kernel.org>

Johan

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

[Johan Hovold]

On Wed, Dec 19, 2018 at 12:11:03PM +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@mediatek.com>
> Cc: stable@vger.kernel.org
> ---
> 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.
> Changes for v6:
>  - Update VID/PID:0x0e8d/0x0003 as Mediatek Inc BROM.
>  - Update VID/PID:0x0e8d/0x2000 as Mediatek Inc Preloader.
> Changes for v7:
>  - Keep VID/PID:0x0e8d/0x0003 unchanged because of 2 different UNION
>    descriptor implementated in Mediatek Inc BROM (MT6589/MT6765).

Reviewed-by: Johan Hovold <johan@kernel.org>

Johan

Re: [PATCH v5] cdc-acm: fix abnormal DATA RX issue for Mediatek Preloader.

[Greg Kroah-Hartman]

On Wed, Dec 19, 2018 at 09:56:22AM +0100, Oliver Neukum wrote:
> On Mi, 2018-12-19 at 12:03 +0800, Macpaul Lin wrote:
> > On Wed, 2018-12-19 at 10:31 +0700, Lars Melin wrote:
> > > On 12/19/2018 10:16, Macpaul Lin wrote: 
> > > 
> > > Hi Macpaul,
> > > your verbose usb listing show me that Mediatek has made two different 
> > > 0e8d:003 devices, see my verbose lsusb listing below.
> > > (Notice also the reverse order for cmd and data interfaces in it 
> > > compared to yours).
> > > USB id's are intended to identify a device and its needs so there should
> > > never be more than one unique device per id.
> > > 
> > > 
> > > Fairphone FP-1, MT6227  (no CDC union !!!)
> > > 
> > 
> > Hi Lars,
> > 
> > Ha ha ha, it is a little bit embarrassing.
> > What I've used to capture verbose log is MT6765 platform.
> > Then I've checked Fairphone FP-1, which is MT6589 a pretty old platform.
> > The BROM (boot ROM) has been maintained by other teams and will vary by
> > different SoC project in Mediatek. I'm not sure why they changed the
> > descriptors.
> > 
> > For the consistency of BROM's behavior, I'll update a new patch keeps
> > PID:0003 remain untouched. I'll trying to report it to BROM team and see
> > if they have any action on this issue.
> 
> Thank you all for taking care of these important issues.
> Please submit that new patch.

Can you review v7 please?

thanks,

greg k-h

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

[Greg Kroah-Hartman]

On Wed, Dec 19, 2018 at 09:56:22AM +0100, Oliver Neukum wrote:
> On Mi, 2018-12-19 at 12:03 +0800, Macpaul Lin wrote:
> > On Wed, 2018-12-19 at 10:31 +0700, Lars Melin wrote:
> > > On 12/19/2018 10:16, Macpaul Lin wrote: 
> > > 
> > > Hi Macpaul,
> > > your verbose usb listing show me that Mediatek has made two different 
> > > 0e8d:003 devices, see my verbose lsusb listing below.
> > > (Notice also the reverse order for cmd and data interfaces in it 
> > > compared to yours).
> > > USB id's are intended to identify a device and its needs so there should
> > > never be more than one unique device per id.
> > > 
> > > 
> > > Fairphone FP-1, MT6227  (no CDC union !!!)
> > > 
> > 
> > Hi Lars,
> > 
> > Ha ha ha, it is a little bit embarrassing.
> > What I've used to capture verbose log is MT6765 platform.
> > Then I've checked Fairphone FP-1, which is MT6589 a pretty old platform.
> > The BROM (boot ROM) has been maintained by other teams and will vary by
> > different SoC project in Mediatek. I'm not sure why they changed the
> > descriptors.
> > 
> > For the consistency of BROM's behavior, I'll update a new patch keeps
> > PID:0003 remain untouched. I'll trying to report it to BROM team and see
> > if they have any action on this issue.
> 
> Thank you all for taking care of these important issues.
> Please submit that new patch.

Can you review v7 please?

thanks,

greg k-h

Re: [PATCH v7] cdc-acm: fix abnormal DATA RX issue for Mediatek Preloader.

[Oliver Neukum]

On Mi, 2018-12-19 at 10:00 +0100, Johan Hovold wrote:
> On Wed, Dec 19, 2018 at 12:11:03PM +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@mediatek.com>
> > Cc: stable@vger.kernel.org
> > ---
> > 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.
> > Changes for v6:
> >  - Update VID/PID:0x0e8d/0x0003 as Mediatek Inc BROM.
> >  - Update VID/PID:0x0e8d/0x2000 as Mediatek Inc Preloader.
> > Changes for v7:
> >  - Keep VID/PID:0x0e8d/0x0003 unchanged because of 2 different UNION
> >    descriptor implementated in Mediatek Inc BROM (MT6589/MT6765).
> 
> Reviewed-by: Johan Hovold <johan@kernel.org>
Acked-by: Oliver Neukum <oneukum@suse.com>

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

[Oliver Neukum]

On Mi, 2018-12-19 at 10:00 +0100, Johan Hovold wrote:
> On Wed, Dec 19, 2018 at 12:11:03PM +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@mediatek.com>
> > Cc: stable@vger.kernel.org
> > ---
> > 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.
> > Changes for v6:
> >  - Update VID/PID:0x0e8d/0x0003 as Mediatek Inc BROM.
> >  - Update VID/PID:0x0e8d/0x2000 as Mediatek Inc Preloader.
> > Changes for v7:
> >  - Keep VID/PID:0x0e8d/0x0003 unchanged because of 2 different UNION
> >    descriptor implementated in Mediatek Inc BROM (MT6589/MT6765).
> 
> Reviewed-by: Johan Hovold <johan@kernel.org>
Acked-by: Oliver Neukum <oneukum@suse.com>