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

[Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>]

On 03/08/2016 01:17 PM, Brian Norris wrote:
>> > You may want to use readl_relaxed_poll_timeout() (even though there's
>> > no way to specify a range).
>> > This comment applies to all the places where you're implementing this
>> > kind of loop.
> What's more, this timeout loop (and probably many of the others) is
> wrong. You need to do one last status check before declaring a timeout,
> since the device may become ready while you're sleeping. It's the same
> problem as we've resolved here:
>
> http://git.infradead.org/l2-mtd.git/commitdiff/9ebfdf5b18493f338237ef9861a555c2f79b0c17
> Subject: "mtd: nand: check status before reporting timeout"

I don't think it is quite the same scenario: in the case that you are describing
the wait is actually rescheduling and yes, that could kick the process out of
the CPU for a while (in the millisecond range).

In this driver however, we are either sleeping for a bounded amount of time (+/-
a margin) in microseconds OR  calling cpu_relax() which is just a memory barrier
in arm.
In the former case, I agree that sleeping for a microsecond range (since there
is not a guaranteed maximum jitter in theory) could go wild but that is highly
unlikely.

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.