From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lj1-f194.google.com ([209.85.208.194]:36273 "EHLO mail-lj1-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726422AbeLRLor (ORCPT ); Tue, 18 Dec 2018 06:44:47 -0500 Date: Tue, 18 Dec 2018 12:44:47 +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 v4] cdc-acm: fix abnormal DATA RX issue for Mediatek Preloader. Message-ID: <20181218114447.GB10562@localhost> References: <1544671676-23912-1-git-send-email-macpaul.lin@mediatek.com> <1545133087-5145-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: <1545133087-5145-1-git-send-email-macpaul.lin@mediatek.com> Sender: stable-owner@vger.kernel.org List-ID: 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 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 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: [v4] cdc-acm: fix abnormal DATA RX issue for Mediatek Preloader. From: Johan Hovold Message-Id: <20181218114447.GB10562@localhost> Date: Tue, 18 Dec 2018 12:44:47 +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: T24gVHVlLCBEZWMgMTgsIDIwMTggYXQgMDc6Mzg6MDdQTSArMDgwMCwgTWFjcGF1bCBMaW4gd3Jv dGU6Cj4gTWVkaWF0ZWsgUHJlbG9hZGVyIGlzIGEgcHJvcHJpZXRhcnkgZW1iZWRkZWQgYm9vdCBs b2FkZXIgZm9yIGxvYWRpbmcKPiBMaXR0bGUgS2VybmVsIGFuZCBMaW51eCBpbnRvIGRldmljZSBE UkFNLgo+IAo+IFRoaXMgYm9vdCBsb2FkZXIgYWxzbyBoYW5kbGUgZmlybXdhcmUgdXBkYXRlLiBN ZWRpYXRlayBQcmVsb2FkZXIgd2lsbCBiZQo+IGVudW1lcmF0ZWQgYXMgYSB2aXJ0dWFsIENPTSBw b3J0IHdoZW4gdGhlIGRldmljZSBpcyBjb25uZWN0ZWQgdG8gV2luZG93cwo+IG9yIExpbnV4IE9T IHZpYSBDREMtQUNNIGNsYXNzIGRyaXZlci4gV2hlbiB0aGUgVVNCIGVudW1lcmF0aW9uIGhhcyBi ZWVuCj4gZG9uZSwgTWVkaWF0ZWsgUHJlbG9hZGVyIHdpbGwgc2VuZCBvdXQgaGFuZHNoYWtlIGNv bW1hbmQgIlJFQURZIiB0byBQQwo+IGFjdGl2ZWx5IGluc3RlYWQgb2Ygd2FpdGluZyBjb21tYW5k IGZyb20gdGhlIGRvd25sb2FkIHRvb2wuCj4gU2luY2UgTGludXggNC4xMiwgdGhlIGNvbW1pdCAi dHR5OiByZXNldCB0ZXJtaW9zIHN0YXRlIG9uIGRldmljZQo+IHJlZ2lzdHJhdGlvbiIgKDkzODU3 ZWRkOTgyOWUxNDRhY2I2YzdlNzJkNTkzZjZlMDFhZWFkNjYpIGNhdXNlcyBNZWRpYXRlawo+IFBy ZWxvYWRlciByZWNlaXZpbmcgc29tZSBhYm5vcmFtbCBjb21tYW5kIGxpa2UgIlJFQURZWFgiIGFz IGl0IHNlbnQuCj4gVGhpcyB3aWxsIGJlIHJlY29nbml6ZWQgYXMgYW4gaW5jb3JyZWN0IHJlc3Bv bnNlLiBUaGUgYmVoYXZpb3IgY2hhbmdlCj4gYWxzbyBjYXVzZXMgdGhlIGRvd25sb2FkIGhhbmRz aGFrZSBmYWlsLgoKUGxlYXNlIG1lbnRpb24gdGhhdCB0aGlzIGNoYW5nZSBvbmx5IGFmZmVjdHMg c3Vic2VxdWVudCBjb25uZWN0cyBpZiB0aGUKcmVjb25uZWN0ZWQgZGV2aWNlIGhhcHBlbnMgdG8g Z2V0IHRoZSBzYW1lIG1pbm9yIG51bWJlci4KCj4gQnkgZGlzYWJsaW5nIHRoZSBFQ0hPIHRlcm1p b3MgZmxhZyBjb3VsZCBhdm9pZCB0aGlzIHByb2JsZW0uIEhvd2V2ZXIsIGl0Cj4gY2Fubm90IGJl IGRvbmUgYnkgdXNlciBzcGFjZSBjb25maWd1cmF0aW9uIHdoZW4gZG93bmxvYWQgdG9vbCBvcGVu Cj4gL2Rldi90dHlBQ00wLiBUaGlzIGlzIGJlY2F1c2UgdGhlIGRldmljZSBydW5uaW5nIE1lZGlh dGVrIFByZWxvYWRlciB3aWxsCj4gc2VuZCBoYW5kc2hha2UgY29tbWFuZCAiUkVBRFkiIGltbWVk aWF0ZWx5IG9uY2UgdGhlIENEQy1BQ00gZHJpdmVyIGlzCj4gcmVhZHkuCj4gCj4gVGhpcyBwYXRj aCB3YW50cyB0byBmaXggYWJvdmUgcHJvYmxlbSBieSBpbnRyb2R1Y2luZyAiRElTQUJMRV9FQ0hP Igo+IHByb3BlcnR5IGluIGRyaXZlcl9pbmZvLiBXaGVuIE1lZGlhdGVrIFByZWxvYWRlciBpcyBj b25uZWN0ZWQsIHRoZQo+IENEQy1BQ00gZHJpdmVyIGNvdWxkIGRpc2FibGUgRUNITyBmbGFnIGlu IHRlcm1pb3MgdG8gYXZvaWQgdGhlIHByb2JsZW0uCj4gCj4gU2lnbmVkLW9mZi1ieTogTWFjcGF1 bCBMaW4gPG1hY3BhdWwubGluQG1lZGlhdGVrLmNvbT4KCkFuZCB5b3UgY2FuIGFkZCBhIENDIHN0 YWJsZSB0YWcgaGVyZS4KCj4gLS0tCj4gQ2hhbmdlcyBmb3IgdjI6Cj4gIC0gTW92ZSBxdWlya3Mg dGVzdGluZyBvZiBESVNBQkxFX0VDSE8gZmxhZyBpbnRvIGFjbV90dHlfaW5zdGFsbCgpLgo+ICAt IENoYW5nZSBxdWlya3MgdGVzdGluZyBpbnRvIGJpdHdpc2UgY29tcGFyaXNvbi4KPiBDaGFuZ2Vz IGZvciB2MzoKPiAgLSBSZXBsYWNlIHF1aXJrcyB0ZXN0aW5nIGZyb20gaW5pdF90ZXJtaW9zIHRv IHR0eS0+dGVybWlvcy4KPiAgLSBSZW1vdmUgcGFyZW50aGVzaXMgZm9yIEVDSE8gZmxhZy4KPiBD aGFuZ2VzIGZvciB2NDoKPiAgLSBEcm9wIHF1aXJrcyB2YXJpYmxlIHRvIHNpbXBsaWZ5IHRoZSBw YXRjaC4KPiAgLSBNb3ZlIHRlcm1pb3Mgb3BlcmF0aW9uIHJpZ2h0IGFmdGVyIHRoZSBkcml2ZXJf ZGF0YSBoYXMgYmVlbiBpbnN0YWxsZWQuCgpObywgaXQgc2hvdWxkIGdvIGFmdGVyIHRoZSAqdHR5 KiBoYXMgYmVlbiBpbnN0YWxsZWQgKGFuZCAqYmVmb3JlKgpzZXR0aW5nIGRyaXZlcl9kYXRhKQoK PiAgLSBXcml0ZSBnZW5lcmFsIHN0eWxlIGNvbW1lbnQgZm9yIHN1cHByZXNzaW5nIGluaXRpYWwg ZWNob2luZy4gCgo+ICBkcml2ZXJzL3VzYi9jbGFzcy9jZGMtYWNtLmMgfCAxMiArKysrKysrKysr Ky0KPiAgZHJpdmVycy91c2IvY2xhc3MvY2RjLWFjbS5oIHwgIDEgKwo+ICAyIGZpbGVzIGNoYW5n ZWQsIDEyIGluc2VydGlvbnMoKyksIDEgZGVsZXRpb24oLSkKPiAKPiBkaWZmIC0tZ2l0IGEvZHJp dmVycy91c2IvY2xhc3MvY2RjLWFjbS5jIGIvZHJpdmVycy91c2IvY2xhc3MvY2RjLWFjbS5jCj4g aW5kZXggMWI2OGZlZC4uYWI4NjA5ZSAxMDA2NDQKPiAtLS0gYS9kcml2ZXJzL3VzYi9jbGFzcy9j ZGMtYWNtLmMKPiArKysgYi9kcml2ZXJzL3VzYi9jbGFzcy9jZGMtYWNtLmMKPiBAQCAtNTgzLDYg KzU4MywxMyBAQCBzdGF0aWMgaW50IGFjbV90dHlfaW5zdGFsbChzdHJ1Y3QgdHR5X2RyaXZlciAq ZHJpdmVyLCBzdHJ1Y3QgdHR5X3N0cnVjdCAqdHR5KQo+ICAKPiAgCXR0eS0+ZHJpdmVyX2RhdGEg PSBhY207Cj4gIAo+ICsJLyoKPiArCSAqIFN1cHByZXNzIGluaXRpYWwgZWNob2luZyBmb3Igc29t ZSBkZXZpY2VzIHdoaWNoIG1pZ2h0IHNlbmQgZGF0YQo+ICsJICogaW1tZWRpYXRlbHkgYWZ0ZXIg YWNtIGRyaXZlciBoYXMgYmVlbiBpbnN0YWxsZWQuCj4gKwkgKi8KPiArCWlmIChhY20tPnF1aXJr cyAmIERJU0FCTEVfRUNITykKPiArCQl0dHktPnRlcm1pb3MuY19sZmxhZyAmPSB+RUNITzsKCllv dSBmb3Jnb3QgdG8gbW92ZSB0aGlzIGFib3ZlIHNldHRpbmcgdHR5LT5kcml2ZXIgZGF0YS4KCj4g Kwo+ICAJcmV0dXJuIDA7CgpUaGFua3MsCkpvaGFuCg==