From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lj1-f196.google.com ([209.85.208.196]:35240 "EHLO mail-lj1-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726370AbeLRKyU (ORCPT ); Tue, 18 Dec 2018 05:54:20 -0500 Date: Tue, 18 Dec 2018 11:54:19 +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 v3] cdc-acm: fix abnormal DATA RX issue for Mediatek Preloader. Message-ID: <20181218105419.GL20658@localhost> References: <1544671676-23912-1-git-send-email-macpaul.lin@mediatek.com> <1545129680-3224-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: <1545129680-3224-1-git-send-email-macpaul.lin@mediatek.com> Sender: stable-owner@vger.kernel.org List-ID: 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 > --- > 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 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: [v3] cdc-acm: fix abnormal DATA RX issue for Mediatek Preloader. From: Johan Hovold Message-Id: <20181218105419.GL20658@localhost> Date: Tue, 18 Dec 2018 11:54:19 +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: T24gVHVlLCBEZWMgMTgsIDIwMTggYXQgMDY6NDE6MjBQTSArMDgwMCwgTWFjcGF1bCBMaW4gd3Jv 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+Cj4gLS0tCj4gQ2hhbmdlcyBmb3IgdjI6Cj4gIC0gTW92ZSBxdWly a3MgdGVzdGluZyBvZiBESVNBQkxFX0VDSE8gZmxhZyBpbnRvIGFjbV90dHlfaW5zdGFsbCgpLgo+ ICAtIENoYW5nZSBxdWlya3MgdGVzdGluZyBpbnRvIGJpdHdpc2UgY29tcGFyaXNvbi4KPiBDaGFu Z2VzIGZvciB2MzoKPiAgLSBSZXBsYWNlIGNsZWFyIGZsYWcgb3BlcmF0aW9uIGZyb20gZHJpdmVy LT5pbml0X3Rlcm1pb3MgdG8gdHR5LT50ZXJtaW9zLgo+ICAtIFJlbW92ZSBwYXJlbnRoZXNpcyBv ZiBFQ0hPIGZsYWcuCgpUaGFua3MgZm9yIHRoZSB1cGRhdGUuIExvb2tzIGdvb2QsIGp1c3Qgc29t ZSBtaW5vciBpc3N1ZXMgYmVsb3cuCgo+ICBkcml2ZXJzL3VzYi9jbGFzcy9jZGMtYWNtLmMgfCAx MyArKysrKysrKysrKystCj4gIGRyaXZlcnMvdXNiL2NsYXNzL2NkYy1hY20uaCB8ICAxICsKPiAg MiBmaWxlcyBjaGFuZ2VkLCAxMyBpbnNlcnRpb25zKCspLCAxIGRlbGV0aW9uKC0pCj4gCj4gZGlm ZiAtLWdpdCBhL2RyaXZlcnMvdXNiL2NsYXNzL2NkYy1hY20uYyBiL2RyaXZlcnMvdXNiL2NsYXNz L2NkYy1hY20uYwo+IGluZGV4IDFiNjhmZWQuLmMxYjg4YzMgMTAwNjQ0Cj4gLS0tIGEvZHJpdmVy cy91c2IvY2xhc3MvY2RjLWFjbS5jCj4gKysrIGIvZHJpdmVycy91c2IvY2xhc3MvY2RjLWFjbS5j Cj4gQEAgLTU3MSw2ICs1NzEsNyBAQCBzdGF0aWMgdm9pZCBhY21fc29mdGludChzdHJ1Y3Qgd29y a19zdHJ1Y3QgKndvcmspCj4gIHN0YXRpYyBpbnQgYWNtX3R0eV9pbnN0YWxsKHN0cnVjdCB0dHlf ZHJpdmVyICpkcml2ZXIsIHN0cnVjdCB0dHlfc3RydWN0ICp0dHkpCj4gIHsKPiAgCXN0cnVjdCBh Y20gKmFjbTsKPiArCXVuc2lnbmVkIGxvbmcgcXVpcmtzOwo+ICAJaW50IHJldHZhbDsKPiAgCj4g IAlhY20gPSBhY21fZ2V0X2J5X21pbm9yKHR0eS0+aW5kZXgpOwo+IEBAIC01ODMsNiArNTg0LDEz IEBAIHN0YXRpYyBpbnQgYWNtX3R0eV9pbnN0YWxsKHN0cnVjdCB0dHlfZHJpdmVyICpkcml2ZXIs IHN0cnVjdCB0dHlfc3RydWN0ICp0dHkpCj4gIAo+ICAJdHR5LT5kcml2ZXJfZGF0YSA9IGFjbTsK PiAgCj4gKwkvKiBnZXQgbm9ybWFsIHF1aXJrcyAqLwo+ICsJcXVpcmtzID0gYWNtLT5xdWlya3M7 CgpJJ2QgZHJvcCB0aGUgcXVpcmtzIHZhcmlhYmxlIGFzIHlvdSBvbmx5IG5lZWQgaXQgb25jZSBh bmQgYWNtLT5xdWlya3MgaXMKcHJldHR5IHNob3J0IHRvIGJlZ2luIHdpdGguCgo+ICsKPiArCS8q IGhhbmRsZSBhY3RpdmUgaGFuZHNoYWtlIHRyaWdnZXJlZCBieSBkZXZpY2UgKi8KClBsZWFzZSBt YWtlIHRoaXMgZGVzY3JpcHRpb24gbW9yZSBnZW5lcmFsOyBvdGhlciBkZXZpY2VzIG1heSBhbHNv IGVuZCB1cApuZWVkaW5nIHRvIHN1cHByZXNzIGluaXRpYWwgZWNob2luZy4KCj4gKwlpZiAocXVp cmtzICYgRElTQUJMRV9FQ0hPKQo+ICsJCXR0eS0+dGVybWlvcy5jX2xmbGFnICY9IH5FQ0hPOwoK QW5kIGZpbmFsbHksIHBsZWFzZSBtb3ZlIHRoaXMganVzdCBhZnRlciBpbnN0YWxsaW5nIHRoZSB0 dHkgKGkuZS4gYmVmb3JlCnNldHRpbmcgdHR5LT5kcml2ZXIgZGF0YSkuCgo+ICsKPiAgCXJldHVy biAwOwoKVGhhbmtzLApKb2hhbgo=