From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Subject: [v2] cdc-acm: fix abnormal DATA RX issue for Mediatek Preloader. From: Johan Hovold Message-Id: <20181218090257.GI20658@localhost> Date: Tue, 18 Dec 2018 10:02:57 +0100 To: Macpaul Lin Cc: Johan Hovold , Oliver Neukum , Greg Kroah-Hartman , linux-usb@vger.kernel.org, stable@vger.kernel.org, Mediatek WSD Upstream List-ID: T24gVHVlLCBEZWMgMTgsIDIwMTggYXQgMDE6MDA6MjJQTSArMDgwMCwgTWFjcGF1bCBMaW4gd3Jv dGU6Cj4gTWVkaWF0ZWsgUHJlbG9hZGVyIGlzIGEgcHJvcHJpZXRhcnkgZW1iZWRkZWQgYm9vdCBs b2FkZXIgZm9yIGxvYWRpbmcKPiBMaXR0bGUgS2VybmVsIGFuZCBMaW51eCBpbnRvIGRldmljZSBE UkFNLgo+IAo+IFRoaXMgYm9vdCBsb2FkZXIgYWxzbyBoYW5kbGUgZmlybXdhcmUgdXBkYXRlLiBN ZWRpYXRlayBQcmVsb2FkZXIgd2lsbCBiZQo+IGVudW1lcmF0ZWQgYXMgYSB2aXJ0dWFsIENPTSBw b3J0IHdoZW4gdGhlIGRldmljZSBpcyBjb25uZWN0ZWQgdG8gV2luZG93cwo+IG9yIExpbnV4IE9T IHZpYSBDREMtQUNNIGNsYXNzIGRyaXZlci4gV2hlbiB0aGUgVVNCIGVudW1lcmF0aW9uIGhhcyBi ZWVuCj4gZG9uZSwgTWVkaWF0ZWsgUHJlbG9hZGVyIHdpbGwgc2VuZCBvdXQgaGFuZHNoYWtlIGNv bW1hbmQgIlJFQURZIiB0byBQQwo+IGFjdGl2ZWx5IGluc3RlYWQgb2Ygd2FpdGluZyBjb21tYW5k IGZyb20gdGhlIGRvd25sb2FkIHRvb2wuCj4gU2luY2UgTGludXggNC4xMiwgdGhlIGNvbW1pdCAi dHR5OiByZXNldCB0ZXJtaW9zIHN0YXRlIG9uIGRldmljZQo+IHJlZ2lzdHJhdGlvbiIgKDkzODU3 ZWRkOTgyOWUxNDRhY2I2YzdlNzJkNTkzZjZlMDFhZWFkNjYpIGNhdXNlcyBNZWRpYXRlawo+IFBy ZWxvYWRlciByZWNlaXZpbmcgc29tZSBhYm5vcmFtbCBjb21tYW5kIGxpa2UgIlJFQURZWFgiIGFz IGl0IHNlbnQuCj4gVGhpcyB3aWxsIGJlIHJlY29nbml6ZWQgYXMgYW4gaW5jb3JyZWN0IHJlc3Bv bnNlLiBUaGUgYmVoYXZpb3IgY2hhbmdlCj4gYWxzbyBjYXVzZXMgdGhlIGRvd25sb2FkIGhhbmRz aGFrZSBmYWlsLgo+IAo+IEJ5IGRpc2FibGluZyB0aGUgRUNITyB0ZXJtaW9zIGZsYWcgY291bGQg YXZvaWQgdGhpcyBwcm9ibGVtLiBIb3dldmVyLCBpdAo+IGNhbm5vdCBiZSBkb25lIGJ5IHVzZXIg c3BhY2UgY29uZmlndXJhdGlvbiB3aGVuIGRvd25sb2FkIHRvb2wgb3Blbgo+IC9kZXYvdHR5QUNN MC4gVGhpcyBpcyBiZWNhdXNlIHRoZSBkZXZpY2UgcnVubmluZyBNZWRpYXRlayBQcmVsb2FkZXIg d2lsbAo+IHNlbmQgaGFuZHNoYWtlIGNvbW1hbmQgIlJFQURZIiBpbW1lZGlhdGVseSBvbmNlIHRo ZSBDREMtQUNNIGRyaXZlciBpcwo+IHJlYWR5Lgo+IAo+IFRoaXMgcGF0Y2ggd2FudHMgdG8gZml4 IGFib3ZlIHByb2JsZW0gYnkgaW50cm9kdWNpbmcgIkRJU0FCTEVfRUNITyIKPiBwcm9wZXJ0eSBp biBkcml2ZXJfaW5mby4gV2hlbiBNZWRpYXRlayBQcmVsb2FkZXIgaXMgY29ubmVjdGVkLCB0aGUK PiBDREMtQUNNIGRyaXZlciBjb3VsZCBkaXNhYmxlIEVDSE8gZmxhZyBpbiB0ZXJtaW9zIHRvIGF2 b2lkIHRoZSBwcm9ibGVtLgo+IAo+IFNpZ25lZC1vZmYtYnk6IE1hY3BhdWwgTGluIDxtYWNwYXVs LmxpbkBtZWRpYXRlay5jb20+Cj4gLS0tCj4gQ2hhbmdlIGZvciB2MjoKPiAgLSBNb3ZlIHF1aXJr cyB0ZXN0aW5nIG9mIERJU0FCTEVfRUNITyBmbGFnIGludG8gYWNtX3R0eV9pbnN0YWxsKCkuCj4g IC0gQ2hhbmdlIHF1aXJrcyB0ZXN0aW5nIGludG8gYml0d2lzZSBjb21wYXJpc29uLgo+IAo+ICBk cml2ZXJzL3VzYi9jbGFzcy9jZGMtYWNtLmMgfCAxMyArKysrKysrKysrKystCj4gIGRyaXZlcnMv dXNiL2NsYXNzL2NkYy1hY20uaCB8ICAxICsKPiAgMiBmaWxlcyBjaGFuZ2VkLCAxMyBpbnNlcnRp b25zKCspLCAxIGRlbGV0aW9uKC0pCj4gCj4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvdXNiL2NsYXNz L2NkYy1hY20uYyBiL2RyaXZlcnMvdXNiL2NsYXNzL2NkYy1hY20uYwo+IGluZGV4IDFiNjhmZWQu LmYxYTkxNGQgMTAwNjQ0Cj4gLS0tIGEvZHJpdmVycy91c2IvY2xhc3MvY2RjLWFjbS5jCj4gKysr IGIvZHJpdmVycy91c2IvY2xhc3MvY2RjLWFjbS5jCj4gQEAgLTU3MSwxMiArNTcxLDIwIEBAIHN0 YXRpYyB2b2lkIGFjbV9zb2Z0aW50KHN0cnVjdCB3b3JrX3N0cnVjdCAqd29yaykKPiAgc3RhdGlj IGludCBhY21fdHR5X2luc3RhbGwoc3RydWN0IHR0eV9kcml2ZXIgKmRyaXZlciwgc3RydWN0IHR0 eV9zdHJ1Y3QgKnR0eSkKPiAgewo+ICAJc3RydWN0IGFjbSAqYWNtOwo+ICsJdW5zaWduZWQgbG9u ZyBxdWlya3M7Cj4gIAlpbnQgcmV0dmFsOwo+ICAKPiAgCWFjbSA9IGFjbV9nZXRfYnlfbWlub3Io dHR5LT5pbmRleCk7Cj4gIAlpZiAoIWFjbSkKPiAgCQlyZXR1cm4gLUVOT0RFVjsKPiAgCj4gKwkv KiBnZXQgbm9ybWFsIHF1aXJrcyAqLwo+ICsJcXVpcmtzID0gYWNtLT5xdWlya3M7Cj4gKwo+ICsJ LyogaGFuZGxlIGFjdGl2ZSBoYW5kc2hha2UgdHJpZ2dlcmVkIGJ5IGRldmljZSAqLwo+ICsJaWYg KHF1aXJrcyAmIERJU0FCTEVfRUNITykKPiArCQlkcml2ZXItPmluaXRfdGVybWlvcy5jX2xmbGFn ICY9IH4oRUNITyk7CgpZb3UncmUgc3RpbGwgY2hhbmdpbmcgdGhlIHNoYXJlZCBkcml2ZXIgaW5p dF90ZXJtaW9zIGhlcmUsIHdoaWNoIGlzIG5vCmJldHRlciB0aGFuIGRvaW5nIHNvIGF0IHByb2Jl IHRpbWUuCgpXaGF0IEkgbWVhbnQgYnkgbW92aW5nIGNsZWFyaW5nIEVDSE8gdG8gdHR5IGluc3Rh bGwgdGltZSB3YXMgdGhhdCB0aGF0CndpbGwgYWxsb3cgeW91IGRvIGNsZWFyIEVDSE8gb25seSBm b3IgdGhlIHR0eSBiZWluZyBpbnN0YWxsZWQgKGFuZApzcGVjaWZpY2FsbHkgbm90IGFmZmVjdGlu ZyB0aGUgdGVybWlvcyBvZiBvdGhlciBjZGMtYWNtIGRldmljZXMpLgoKPiArCj4gIAlyZXR2YWwg PSB0dHlfc3RhbmRhcmRfaW5zdGFsbChkcml2ZXIsIHR0eSk7Cj4gIAlpZiAocmV0dmFsKQo+ICAJ CWdvdG8gZXJyb3JfaW5pdF90ZXJtaW9zOwoKU28gaGVyZSwgYWZ0ZXIgdHR5X3N0YW5kYXJkX2lu c3RhbGwoKSByZXR1cm5zIHlvdSBjYW4gbW9kaWZ5CnR0eS0+dGVybWlvcyBhZnRlciBpdCBoYXMg YmVlbiBpbml0aWFsaXNlZCB0byB0aGUgZGVmYXVsdCAob3Igc2F2ZWQpCnNldHRpbmdzLgoKQWxz byBub3RlIHRoYXQgeW91IGNhbiBkcm9wIHRoZSBwYXJlbnRoZXNpcyBhcm91bmQgRUNITyBhYm92 ZS4KClRoYW5rcywKSm9oYW4K From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf1-f66.google.com ([209.85.167.66]:40783 "EHLO mail-lf1-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726367AbeLRJC6 (ORCPT ); Tue, 18 Dec 2018 04:02:58 -0500 Date: Tue, 18 Dec 2018 10:02:57 +0100 From: Johan Hovold To: Macpaul Lin Cc: Johan Hovold , Oliver Neukum , Greg Kroah-Hartman , linux-usb@vger.kernel.org, stable@vger.kernel.org, Mediatek WSD Upstream Subject: Re: [PATCH v2] cdc-acm: fix abnormal DATA RX issue for Mediatek Preloader. Message-ID: <20181218090257.GI20658@localhost> References: <1544671676-23912-1-git-send-email-macpaul.lin@mediatek.com> <1545109222-25073-1-git-send-email-macpaul.lin@mediatek.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1545109222-25073-1-git-send-email-macpaul.lin@mediatek.com> Sender: stable-owner@vger.kernel.org List-ID: 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 > --- > 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