[Patch 1/1] don't suspend erase for erase in cfi_cmdset_0002

[Peter Wippich]

And Hi again, 

On Tue, 4 Aug 2009, Joakim Tjernlund wrote:

> Peter Wippich <pewi at gw-instruments.de> wrote on 03/08/2009 23:24:01:
> >
> >
> > Hi Jocke,
> >
> > On Mon, 3 Aug 2009, Joakim Tjernlund wrote:
> >
> > > Hi again :)
> > >
> > > I think mine is better because:
> > > 1) It matches cmdset_0001 and the code in general better.
> > > 2) It can handle FL_WRITING too.
> > >
> > > But I am not the maintainer and I don't use 0002 myself, just trying to
> > > help out.
> > >
> > > I hope you will constuct a cfip too for your JEDEC chips. It is no
> > > fun to be without erase suspend :(
> > >
> >
> > It's fine to have some discussion about that. From that at least we agree
> > that something is wrong here. The main difference between the suggested
> > patches is the change in behaviour from the current implementation.
> > Currently the default behaviour is that erase suspend will work with jedec
> > and cfi probed chips. I can not tell if this is by intention or by chance,
> > but I think if we change something it should not break existing
> > implementations.
> > So at least a final fix shall:
> > - use erase suspend for jedec chips for "read only" suspends were
> >   cfip == NULL
> 
> Are you sure all JEDEC chips support RO suspends? If not,
> I think this is a 3 step/pathes procedure:
> 1) Just fix the if stmt as I outlined. Then JEDEC chips wont bug, but won't have erase
>    suspend either unless:
> 
> 2) Add a fixup for your JEDEC flash that fills in the erase suspend bits you need. If all
>    JEDEC chips support RO suspends you can do a JEDEC generic fixup that adds this capability
>    to cfip.
> 
> 3) remove the FIXME and test if it works for you. Send it to the
>    list too and ask for some testing.
> 
> > - use erase suspend for all cfi probed chips for "read only" suspends
> >   if supported (as indicated by cfip->EraseSuspend)
> >
> > And for the write case, I can not tell why the erase suspend is viewed to
> > be broken here. If we change this,  as in your patch, severe testing is
> > needed to make sure it is realy working. Currently I don't have the free
> > resources to do this testing. Maybe we'll find some volunteer ?!.......
> >
> > I'll try to find a fix which will not break current implementations and
> > allows updates to implement erase suspend for writes easily. Appriciate
> > your comments.
> >
> > BTW: who's the maintainer and what are her/his  thoughts.
> 
> Don't konw.
 
Partly disagree. For Step 1 we shall leave erase suspend active for Jedec 
Chips because this is the current behaviour. If this behaviour is wrong 
we would have seen a lot of complains on the list (which aren't there). 
If we change the default behaviour then we have to add fxups for each and 
every Jedec chip which is used out there and supports erase suspend. And 
from cherry picking in the long list most chips which use 
cfi_cmdset_0002 support erase suspend. 

And BTW: I realy hate this "cfip->EraseSuspend & 0x1" stuff. Nobody knows 
what this is unless going through the CFI specification. It would make 
the code much better readable if we introduce some symbolic constants 
here. 

Ciao, 

Peter