Re: [PATCH v3 12/28] mtd: nand: pxa3xx: Use a completion to signal device ready

[Ezequiel Garcia <ezequiel.garcia@free-electrons.com>]

On Tue, Nov 05, 2013 at 09:27:59PM -0300, Ezequiel Garcia wrote:
> > 
> > I believe your handling of this 'is_ready' bit is a little unwise, as
> > you are actually creating extra concurrency that is unneeded. I'll
> > summarize what you're doing with this 'is_ready' field:
> > 
> >   cmdfunc() -> sets info->is_ready=0 for appropriate commands
> >             -> kicks off the hardware
> > 
> > The following two sequences may then occur concurrently:
> > 
> >   (1) pxa3xx_nand_irq -> ready interrupt occurs
> >                       -> set info->is_ready=1
> > 		      -> signal 'dev_ready' completion
> > 
> >   (2) waitfunc -> check info->is_ready, if it is 0...
> >                   |_ ... wait for the dev_ready completion
> > 
> > Instead of setting info->is_ready=1 under (1), you could set it in (2),

So, after some more careful thought and some testings, it seems you're
correct. Setting is_read in waitfunc() results in a non-racy solution and
the atomic_t can go away.

I'll take a look at the other remaining issues and see about preparing
a v4, so we can try to converge quick enough for v3.14.

Thanks!
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com