linux-serial.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Jiri Slaby <jirislaby@kernel.org>,
	Vignesh Raghavendra <vigneshr@ti.com>,
	Alexander Sverdlin <alexander.sverdlin@gmail.com>,
	Johan Hovold <johan@kernel.org>,
	linux-serial@vger.kernel.org, kernel-janitors@vger.kernel.org
Subject: Re: [PATCH] serial: 8250_omap: fix a timeout loop condition
Date: Thu, 29 Apr 2021 16:02:15 +0300	[thread overview]
Message-ID: <20210429130215.GE21598@kadam> (raw)
In-Reply-To: <YIqTvcZ6ZrAEL7WE@smile.fi.intel.com>

On Thu, Apr 29, 2021 at 02:08:45PM +0300, Andy Shevchenko wrote:
> On Thu, Apr 29, 2021 at 10:19:22AM +0300, Dan Carpenter wrote:
> > This loop ends on -1 so the error message will never be printed.
> > 
> > Fixes: 4bcf59a5dea0 ("serial: 8250: 8250_omap: Account for data in flight during DMA teardown")
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> ...
> 
> >  			       poll_count--)
> >  				cpu_relax();
> >  
> > -			if (!poll_count)
> > +			if (poll_count == -1)
> 
> Why not to change poll_count-- to --poll_count?
>

Either one is fine.  I considered several different ways and wrote the
patch twice.  The downside of --poll_count is that it's an off by one
in that the author clearly intended to loop 25 times.  It doesn't really
matter if we only loop 24 but off by ones are aesthetically unpleasant.

regards,
dan carpenter


  reply	other threads:[~2021-04-29 13:02 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-29  7:19 [PATCH] serial: 8250_omap: fix a timeout loop condition Dan Carpenter
2021-04-29  9:23 ` Alexander Sverdlin
2021-04-29 11:08 ` Andy Shevchenko
2021-04-29 13:02   ` Dan Carpenter [this message]
2021-04-30  8:46     ` Andy Shevchenko
2021-04-30  8:47       ` Andy Shevchenko
2021-04-30 11:41       ` Dan Carpenter
2021-04-30 12:53         ` Andy Shevchenko
2021-04-30 13:33           ` Dan Carpenter
2021-04-30 14:21             ` Andy Shevchenko
2021-05-03  6:54               ` Dan Carpenter
2021-05-03  9:51                 ` Andy Shevchenko
2021-05-14  8:00                   ` Dan Carpenter

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=20210429130215.GE21598@kadam \
    --to=dan.carpenter@oracle.com \
    --cc=alexander.sverdlin@gmail.com \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jirislaby@kernel.org \
    --cc=johan@kernel.org \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=vigneshr@ti.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).