All of lore.kernel.org
 help / color / mirror / Atom feed
From: Macpaul Lin <macpaul.lin@mediatek.com>
To: Johan Hovold <johan@kernel.org>, Oliver Neukum <oneukum@suse.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	<linux-usb@vger.kernel.org>, <stable@vger.kernel.org>
Cc: Mediatek WSD Upstream <wsd_upstream@mediatek.com>,
	Macpaul Lin <macpaul.lin@mediatek.com>
Subject: [PATCH v2] cdc-acm: fix abnormal DATA RX issue for Mediatek Preloader.
Date: Tue, 18 Dec 2018 13:00:22 +0800	[thread overview]
Message-ID: <1545109222-25073-1-git-send-email-macpaul.lin@mediatek.com> (raw)
In-Reply-To: <1544671676-23912-1-git-send-email-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 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

WARNING: multiple messages have this Message-ID (diff)
From: macpaul.lin@mediatek.com
To: Johan Hovold <johan@kernel.org>, Oliver Neukum <oneukum@suse.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-usb@vger.kernel.org, stable@vger.kernel.org
Cc: Mediatek WSD Upstream <wsd_upstream@mediatek.com>,
	Macpaul Lin <macpaul.lin@mediatek.com>
Subject: [v2] cdc-acm: fix abnormal DATA RX issue for Mediatek Preloader.
Date: Tue, 18 Dec 2018 13:00:22 +0800	[thread overview]
Message-ID: <1545109222-25073-1-git-send-email-macpaul.lin@mediatek.com> (raw)

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)

  reply	other threads:[~2018-12-18  5:00 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-13  3:27 cdc-acm: fix abnormal DATA RX issue for Mediatek Preloader macpaul.lin
2018-12-18  5:00 ` Macpaul Lin [this message]
2018-12-18  5:00   ` [v2] " macpaul.lin
2018-12-18  9:02   ` [PATCH v2] " Johan Hovold
2018-12-18  9:02     ` [v2] " Johan Hovold
2018-12-18 10:41 ` [PATCH v3] " Macpaul Lin
2018-12-18 10:41   ` [v3] " macpaul.lin
2018-12-18 10:54   ` [PATCH v3] " Johan Hovold
2018-12-18 10:54     ` [v3] " Johan Hovold
2018-12-18 11:38 ` [PATCH v4] " Macpaul Lin
2018-12-18 11:38   ` [v4] " macpaul.lin
2018-12-18 11:44   ` [PATCH v4] " Johan Hovold
2018-12-18 11:44     ` [v4] " Johan Hovold
2018-12-18 11:59 ` [PATCH v5] " Macpaul Lin
2018-12-18 11:59   ` [v5] " macpaul.lin
2018-12-18 12:38   ` [PATCH v5] " Johan Hovold
2018-12-18 12:38     ` [v5] " Johan Hovold
2018-12-18 13:37     ` [PATCH v5] " Oliver Neukum
2018-12-18 13:37       ` [v5] " Oliver Neukum
2018-12-18 14:26       ` [PATCH v5] " Macpaul Lin
2018-12-18 14:26         ` [v5] " macpaul.lin
2018-12-18 15:19         ` [PATCH v5] " Macpaul Lin
2018-12-18 15:19           ` [v5] " macpaul.lin
2018-12-18 16:42           ` [PATCH v5] " Lars Melin
2018-12-18 16:42             ` [v5] " Lars Melin
2018-12-18 17:48             ` [PATCH v5] " Macpaul Lin
2018-12-18 17:48               ` [v5] " macpaul.lin
2018-12-19  0:45               ` [PATCH v5] " Lars Melin
2018-12-19  0:45                 ` [v5] " Lars Melin
2018-12-19  2:22               ` [PATCH v5] " Macpaul Lin
2018-12-19  2:22                 ` [v5] " macpaul.lin
2018-12-19  3:16                 ` [PATCH v5] " Macpaul Lin
2018-12-19  3:16                   ` [v5] " macpaul.lin
2018-12-19  3:31                   ` [PATCH v5] " Lars Melin
2018-12-19  3:31                     ` [v5] " Lars Melin
2018-12-19  4:03                     ` [PATCH v5] " Macpaul Lin
2018-12-19  4:03                       ` [v5] " macpaul.lin
2018-12-19  8:56                       ` [PATCH v5] " Oliver Neukum
2018-12-19  8:56                         ` [v5] " Oliver Neukum
2018-12-20 15:19                         ` [PATCH v5] " Greg Kroah-Hartman
2018-12-20 15:19                           ` [v5] " Greg Kroah-Hartman
2018-12-18 15:37 ` [PATCH v6] " Macpaul Lin
2018-12-18 15:37   ` [v6] " macpaul.lin
2018-12-19  4:11 ` [PATCH v7] " Macpaul Lin
2018-12-19  4:11   ` [v7] " macpaul.lin
2018-12-19  9:00   ` [PATCH v7] " Johan Hovold
2018-12-19  9:00     ` [v7] " Johan Hovold
2018-12-20 15:28     ` [PATCH v7] " Oliver Neukum
2018-12-20 15:28       ` [v7] " Oliver Neukum

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1545109222-25073-1-git-send-email-macpaul.lin@mediatek.com \
    --to=macpaul.lin@mediatek.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=johan@kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=oneukum@suse.com \
    --cc=stable@vger.kernel.org \
    --cc=wsd_upstream@mediatek.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.