[PATCH] mtd: cfi: Wait for Block Erase operation to finish

[Joakim Tjernlund]

Paul Parsons <lost.distance at yahoo.com> wrote on 2012/02/29 01:23:21:
> From: Paul Parsons <lost.distance at yahoo.com>
>
> > > If an Erase Suspend Command (0xb0) is issued while a
> > Block Erase operation
> > > (0x20, 0xd0) is in progress, Status Register bit 6
> > (SR.6) will be set to 1.
> > > After an Erase Resume Command (0xd0) is issued, SR.6
> > will be set back to 0.
> > >
> > > Unfortunately when inval_cache_and_wait_for_operation()
> > is called to wait for
> > > a Block Erase operation to finish, it never checks that
> > SR.6 = 0 before
> > > returning. Consequently it can (and does) return while
> > a Block Erase operation
> > > is still suspended, resulting in random breakage.
> > >
> > > This patch ensures that when
> > inval_cache_and_wait_for_operation() is called to
> > > wait for a Block Erase operation to finish
> > (chip->state == FL_ERASING), it does
> > > not return until both SR.7 = 1 (as before) and SR.6 =
> > 0.
> >
> > Interesting but I do wonder, should not the
> >   if (chip->state != chip_state)
> > test just before the status test prevent this in the first
> > place?
>
> I'm not sure I understand. The (chip->state != chip_state) test forces
> inval_cache_and_wait_for_operation() to stop polling and sleep, pending
> the operation which suspended it. After it wakes it resumes polling. The
> test doesn't prevent inval_cache_and_wait_for_operation() returning
> prematurely, i.e. before the Block Erase has finished.

Been to long since I been in this code I guess, but I still don't get it.

If erase is suspended, chip->state is changed which will keep the function in
the for(;;) loop. Once erase has been resumed again chip->state will change back.
It seems to me that chip->state and SR.6 are mutally exclusive? I can not see how
you can get to
	status = map_read(map, cmd_adr);
	if (map_word_andequal(map, status, status_OK, status_OK))
if the the erase has been suspended.

>
> An important point which I suspect has been overlooked is that both SR.7
> and SR.6 are set when a Block Erase is suspended. Thus testing SR.7 only
> is not sufficient to determine that a Block Erase has completed.
>
> > > +   map_word status_76 = (chip->state
> > == FL_ERASING) ? CMD(0xc0) : CMD(0x80);
> >
> > hmm, I wonder if you can get by without the chip->state
> > test? Just always set it to 0xc0?
>
> No, unfortunately not. For example a write buffer sequence will finish
> whether or not a Block Erase is currently suspended; thus it must test
> SR.7 only, not SR.6.

I see, thanks.

 Jocke