Re: Hang on reboot in nand_get_device()

[Brian Norris <computersforpeace@gmail.com>]

On Mon, Nov 09, 2015 at 10:36:13PM +0100, Boris Brezillon wrote:
> Just want to add that this discussion shouldn't prevent your fix from
> being applied. The main reason I'm arguing here is because I want to
> understand the rationale behind the current handling of FL_PM_SUSPENDED
> and FL_SHUTDOWN.

Sure, that's reasonable. I'd also like to touch this code only once (or
very close to that) in the near future, so it's best if we get to a good
understanding.

I'll send this as a proper patch, if that sounds OK:

http://patchwork.ozlabs.org/patch/541065/

> On Mon, 9 Nov 2015 21:55:08 +0100
> Boris Brezillon <boris.brezillon@free-electrons.com> wrote:
> 
> > > > 
> > > > diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> > > > index ceb68ca..812b8b1 100644
> > > > --- a/drivers/mtd/nand/nand_base.c
> > > > +++ b/drivers/mtd/nand/nand_base.c
> > > > @@ -830,6 +830,20 @@ nand_get_device(struct mtd_info *mtd, int new_state)
> > > >  retry:
> > > >         spin_lock(lock);
> > > >  
> > > > +       /* putting the NAND chip in shutdown state should always succeed. */
> > > > +       if (new_state == FL_SHUTDOWN) {
> > > > +               /*
> > > > +                * release the controller if the chip put in shutdown state
> > > > +                * is the current active device.
> > > > +                */
> > > > +               if (chip->controller->active == chip)
> > > > +                       chip->controller->active = NULL;
> > > > +
> > > > +               chip->state = new_state;
> > > > +               spin_unlock(lock);
> > > > +               return 0;
> > > > +       }
> > > > +
> > > >         /* Hardware controller shared among independent devices */
> > > >         if (!chip->controller->active)
> > > >                 chip->controller->active = chip;
> > > > 
> > > 
> > > This looks a lot more subtle and potentially wrong. What exactly is the
> > > rationale here? It appears you're kind of unlocking the controller (any
> > > other flash on the same controller can still go ahead) but at the same
> > > time forcing no further users of this particular flash.
> 
> It's even worst: I'm not waiting for the chip to become ready, so I'm
> potentially re-introducing the bug Scott was trying to solve with his
> reboot notifier.

Ah, I see! Good catch. My distaste for duplication pays off, then :)

Brian