Re: [PATCH] serial: 8250_omap: fix a timeout loop condition

Andy Shevchenko
2021-04-30T08:46:13Z

On Thu, Apr 29, 2021 at 04:02:15PM +0300, Dan Carpenter wrote:
> 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.

I didn't get. If you use --poll_count you get exactly 25 times and moreover,
you may convert variable to unsigned type.

--
With Best Regards,
Andy Shevchenko