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

Jorge Ramirez-Ortiz
2016-03-08T20:58:09Z

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).