All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
Cc: linux-serial@vger.kernel.org,
	Greg KH <gregkh@linuxfoundation.org>,
	Jiri Slaby <jirislaby@kernel.org>,
	linux-kernel@vger.kernel.org,
	Gilles Buloz <gilles.buloz@kontron.com>,
	Johan Hovold <johan@kernel.org>
Subject: Re: [PATCH v3 5/5] tty: Use flow-control char function on closing path
Date: Mon, 11 Apr 2022 14:28:44 +0300	[thread overview]
Message-ID: <YlQQ7MYBiK98n5XK@smile.fi.intel.com> (raw)
In-Reply-To: <20220411094859.10894-6-ilpo.jarvinen@linux.intel.com>

On Mon, Apr 11, 2022 at 12:48:59PM +0300, Ilpo Järvinen wrote:
> Use n_tty_receive_char_flow_ctrl also on the closing path. This makes
> the code cleaner and consistent.
> 
> However, there a small change of regression!
> 
> The earlier closing path has a small difference compared with the
> normal receive path. If START_CHAR and STOP_CHAR are equal, their
> precedence is different depending on which path a character is
> processed. I don't know whether this difference was intentional or
> not, and if equal START_CHAR and STOP_CHAR is actually used anywhere.
> But it feels not so useful corner case.
> 
> While this change would logically belong to those earlier changes,
> having a separate patch for this is useful. If this regresses, bisect
> can pinpoint this change rather than the large patch. Also, this
> change is not necessary to minimal fix for the issue addressed in
> the previous patch.

This is nice, if it flies!
FWIW,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> ---
>  drivers/tty/n_tty.c | 13 ++++---------
>  1 file changed, 4 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
> index fede29ed8daf..e6f47858f98f 100644
> --- a/drivers/tty/n_tty.c
> +++ b/drivers/tty/n_tty.c
> @@ -1414,15 +1414,10 @@ static void n_tty_receive_char_closing(struct tty_struct *tty, unsigned char c,
>  		c = tolower(c);
>  
>  	if (I_IXON(tty)) {
> -		if (c == STOP_CHAR(tty)) {
> -			if (!lookahead_done)
> -				stop_tty(tty);
> -		} else if (c == START_CHAR(tty) && lookahead_done) {
> -			return;
> -		} else if (c == START_CHAR(tty) ||
> -			 (tty->flow.stopped && !tty->flow.tco_stopped && I_IXANY(tty) &&
> -			  c != INTR_CHAR(tty) && c != QUIT_CHAR(tty) &&
> -			  c != SUSP_CHAR(tty))) {
> +		if (!n_tty_receive_char_flow_ctrl(tty, c, lookahead_done) &&
> +		    tty->flow.stopped && !tty->flow.tco_stopped && I_IXANY(tty) &&
> +		    c != INTR_CHAR(tty) && c != QUIT_CHAR(tty) &&
> +		    c != SUSP_CHAR(tty)) {
>  			start_tty(tty);
>  			process_echoes(tty);
>  		}
> -- 
> 2.30.2
> 

-- 
With Best Regards,
Andy Shevchenko



      reply	other threads:[~2022-04-11 11:32 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-11  9:48 [PATCH v3 0/5] tty/serial: Process XON/XOFF robustly Ilpo Järvinen
2022-04-11  9:48 ` [PATCH v3 1/5] tty: Add function for handling flow control chars Ilpo Järvinen
2022-04-11  9:48 ` [PATCH v3 2/5] tty: Simplify receive flow control char logic Ilpo Järvinen
2022-04-22 14:30   ` Greg KH
2022-04-11  9:48 ` [PATCH v3 3/5] tty: Add lookahead param to receive_buf Ilpo Järvinen
2022-04-22 14:34   ` Greg KH
2022-04-22 20:09     ` Ilpo Järvinen
2022-04-26  7:48       ` Ilpo Järvinen
2022-04-26  7:59         ` Greg KH
2022-04-11  9:48 ` [PATCH v3 4/5] tty: Implement lookahead to process XON/XOFF timely Ilpo Järvinen
2022-04-11  9:48 ` [PATCH v3 5/5] tty: Use flow-control char function on closing path Ilpo Järvinen
2022-04-11 11:28   ` Andy Shevchenko [this message]

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=YlQQ7MYBiK98n5XK@smile.fi.intel.com \
    --to=andriy.shevchenko@linux.intel.com \
    --cc=gilles.buloz@kontron.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=ilpo.jarvinen@linux.intel.com \
    --cc=jirislaby@kernel.org \
    --cc=johan@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    /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.