Re: [PATCH] mtd: cfi_cmdset_0002: Change erase functions to retry for error

[Boris Brezillon <boris.brezillon@bootlin.com>]

Hi,

On Tue, 8 May 2018 17:35:48 +0000
IKEGAMI Tokunori <ikegami@allied-telesis.co.jp> wrote:

> Hi Boris-san,
> 
> Thanks for your reviewing.
> 
> > > This is needed to prevent the flash erase error caused only once.
> > > It was caused by the error case of chip_good() in the do_erase_oneblock().
> > > Also it was confirmed on the MACRONIX flash device MX29GL512FHT2I-11G.
> > > But the error issue behavior is not able to reproduce at this moment.  
> > 
> > Hm, that's a problem. How can you be sure the retry approach fixes the
> > issue if you can't reproduce the it?  
> 
>   I can reproduce the flash write error by the same reproduction method.
>   And the error can be resolved by the same approach fixes as below.
>     <http://lists.infradead.org/pipermail/linux-mtd/2018-May/080794.html>

Hm, even the bug fix for the write path looks suspicious. BTW, you're
only checking the last written word, so how can you be sure all other
words have been written correctly? I don't know a lot about CFI chips,
but are you sure this is not a timing issue (timings mis-configured on
the controller side)?

>   So it seems that the erase error also can be resolved by the changes.

Well, I'd be more comfortable if someone else could confirm the bug
(ideally tested with a different controller) and validate the fix.

> 
> > > The flash controller is parallel Flash interface integrated on BCM53003.
> > >
> > > Also the change is possible to resolve other erase error.
> > > For example the Erase suspend issue was caused on Cypress AMD NOR flash.
> > >   S29GL01GS / S29GL512S / S29GL256S / S29GL128S  
> > 
> > Is the Erase suspend issue related to this problem?  
> 
>   I am not sure about the issue behavior detail since the issue is not caused on my environment.
>   The issue was mentioned by Jocke-san on the following mail.
>     <http://lists.infradead.org/pipermail/linux-mtd/2018-April/080518.html>
>   Also I am asking to him to try the original patch on this thread.

Ok, let's wait for Joakim's feedback then.

Thanks,

Boris