All of lore.kernel.org
 help / color / mirror / Atom feed
From: Johan Hovold <johan@kernel.org>
To: Macpaul Lin <macpaul.lin@mediatek.com>
Cc: Johan Hovold <johan@kernel.org>, Oliver Neukum <oneukum@suse.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	wsd_upstream@mediatek.com, linux-usb@vger.kernel.org
Subject: cdc-acm: fix abnormal DATA RX issue for Mediatek Preloader.
Date: Fri, 14 Dec 2018 12:07:31 +0100	[thread overview]
Message-ID: <20181214110731.GC20658@localhost> (raw)

On Fri, Dec 14, 2018 at 10:00:16AM +0800, Macpaul Lin wrote:
> On Thu, 2018-12-13 at 11:18 +0100, Johan Hovold wrote:
> > On Thu, Dec 13, 2018 at 11:13:54AM +0100, Oliver Neukum wrote:
> > > On Do, 2018-12-13 at 10:43 +0100, Johan Hovold wrote:
> > > > On Thu, Dec 13, 2018 at 11:27:56AM +0800, macpaul.lin@mediatek.com wrote:
> > > > > From: Macpaul Lin <macpaul.lin@mediatek.com>
> > > > > 
> > > > > +	/* handle active handshake triggered by device */
> > > > > +	if (quirks == DISABLE_ECHO)
> > > > > +		acm_tty_driver->init_termios.c_lflag &= ~(ECHO);
> > > > 
> > > > You cannot change the driver init_termios like this as that will affect
> > > > all cdc-acm devices that are probed later. If this is at all needed,
> > > > this will have to be done at tty install time.
> > > 
> > > Right. How do with decide on a sensible default anyway?
> > 
> > I think the current defaults are sensible. They are based on
> > tty_std_termios, which has ECHO set, as for most (all?) tty drivers.

> Well, the problem is that the phone device (preloader) will get
> "READYXX" even "no any download tool has ever been launched on PC".

Something much hold the port open on the host (PC) side for it to be
echoed back, but perhaps the outgoing data is buffered in the device
until it is eventually opened.

> After the reset termios state change simply set EHCO enable, each time
> the phone device simple send out "READY" to PC after USB has been
> connected. The "READY" indication has no any "\0" character at the end
> of the command string, hence it will receive some dirty data (ex. 
> "READYXX") back on RX path.

So you have a firmware bug which sends out some garbage characters
("XX")?

Either way, you should have hit this also before the commit which
started resetting the terminal settings whenever a device was plugged
in instead of reusing a potentially random other device's settings.

However, after clearing echo and replugging the device, nothing would
have been echoed back on next open, but only *if* the device happened to
be assigned the same minor number.

But if this confuses your firmware and there's no way to restart the
handshake in your host tool, perhaps clearing ECHO at tty install time
(i.e. in tty_acm_install()) is the only way to deal with this.

Johan

             reply	other threads:[~2018-12-14 11:07 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-14 11:07 Johan Hovold [this message]
  -- strict thread matches above, loose matches on Subject: below --
2018-12-18  8:55 cdc-acm: fix abnormal DATA RX issue for Mediatek Preloader Johan Hovold
2018-12-17  5:50 macpaul.lin
2018-12-14  2:00 macpaul.lin
2018-12-13 10:18 Johan Hovold
2018-12-13 10:13 Oliver Neukum
2018-12-13  9:43 Johan Hovold
2018-12-13  9:23 Oliver Neukum
2018-12-13  3:27 macpaul.lin

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=20181214110731.GC20658@localhost \
    --to=johan@kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=macpaul.lin@mediatek.com \
    --cc=oneukum@suse.com \
    --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.