Re: [PATCH 2/3] mtd: mediatek: driver for MTK Smart Device Gen1 NAND

[Boris Brezillon <boris.brezillon@free-electrons.com>]

On Tue, 8 Mar 2016 15:57:44 -0500
Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org> wrote:

> On 03/08/2016 03:20 PM, Brian Norris wrote:
> >> > If you feel strongly about it I don't mind adding an additional check after any
> >> > form of sleep (not so sure about adding it after a cpu_relax) but I don't think
> >> > it is needed.
> > It is non-negotiable that your timeout loops must be logically correct.
> > That is, you must recheck the exit condition before you declare a
> > timeout.
> 
> Hi Brian,
> 
> My point was that the current timeout loops (except one which is just
> implementing its own version of readx_poll_timeout) are logically correct as
> they are since they are not involving the scheduler: so doing the additional
> check after cpu_relax() is unnecessary - cpu_relax is a dmb instruction.
> 
> >
> > If you just follow Boris's suggestion of using the helper macros, then
> > you'll be fine.
> 
> I am sorry (not trying to be difficult here) but relaxed_poll_timeout calls
> usleep_range and involving the scheduler brings in a level of undeterminism (so
> we could have slept for 100 useconds or 1000)
> am I wrong? is under that case that we need to check after exiting the loop.
> 
> a different discussion is if using cpu_relax (busy loop) at all is a good idea:
> the way I see it, that should depend on the case but I suppose a silver bullet
> solution via the helper macros is ok too - and certainly more readable and
> easier to maintain - so will do as you suggest (correct all loops).
> 

Note that if you want to avoid sleeping between each test, you can use
readx_poll_timeout_atomic(), which are replacing usleep_range() calls
by udelay().


-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com