mtd: cfi_cmdset_0002: Read data 3 times to check write operation status

[IKEGAMI Tokunori]

> -----Original Message-----
> From: Joakim Tjernlund [mailto:Joakim.Tjernlund at infinera.com]
> Sent: Wednesday, April 25, 2018 2:22 AM
> To: IKEGAMI Tokunori; linux-mtd at lists.infradead.org
> Cc: PACKHAM Chris
> Subject: Re: mtd: cfi_cmdset_0002: Read data 3 times to check write
> operation status
> 
> On Tue, 2018-04-24 at 14:08 +0000, IKEGAMI Tokunori wrote:
> > CAUTION: This email originated from outside of the organization. Do not
> click links or open attachments unless you recognize the sender and know
> the content is safe.
> >
> >
> > Hi Jocke-san,
> >
> > Thanks for your comment.
> >
> > > I find this a bit odd. I am looking at some other erase suspend problems
> we
> > > see on Cypress AMD NOR flash and I have noted that there is more than
> just DQ6 toggling.
> > > Checking DQ5 in combination with DQ6 is one.
> >
> >   By the way I have also found only once do_erase_oneblock() was failed
> by the timeout on the write failure situation.
> >   So I thought that the error also can be resolved by the 3 times reading
> for checking chip_ready().
> 
> We have see several times that too many Erase suspend for one block will
> cause 5.5.2.6 DQ5: Exceeded Timing
> Limits
> This condition is not checked for nor handled in any way.

I have just checked the error in the past again.
This was caused by the error case of chip_good() in the do_erase_oneblock().
But it was not caused by the error case of chip_ready() actually.
So it seems a similar case but the flash device is different.
I have confirmed it on the MACRONIX flash device MX29GL512FHT2I-11G.
But the issue was not reproduced at that time.
I thought that the issue is also resolved by the change to read 3 times but not sure.

> >
> > > How did you you arrive at reading 3 times is correct? Is the any
> documentation
> > > supporting this?
> >
> >   About the 3 times reading is described by the page 32 on following
> Cypress S29GLxxxP flash data sheet.
> >     <http://www.cypress.com/file/219926/download>
> 
> I don't have such info for my chip at page 32, I am looking at
> S29GL01GS/S29GL512S
> S29GL256S/S29GL128S from http://www.cypress.com/file/177976/download
> 
> I am looking at chap 5.5.2.5 Reading Toggle Bits DQ6/DQ2

Yes it is described by the Cypress S29GLxxxP flash data sheet only as far as I know.
Also it is not described by the MACRONIX flash device data sheet.
But actually I have confirmed that the 3rd time read data was changed on the MACRONIX flash device also.
Also if the bus timing between CPU and flash was changed by enabling ExternalSync mode on our CPU then the both Cypress and MACRONIX devices are failed to write.
  Note: This is caused on other alternative flash devices on our products.
So it seems that it is better to be changed to read 3 times for all flash devices supported by the cfi_cmdset_0002 driver.

> >
> > > All in all, error handling in cmdset0002 is lacking.
> >
> >   I have understood as that it is for example FIXME parts for the error
> case in cfi_cmdset_0002.
> >   Is this correct?
> 
> Haven't looked at all the FIXME's but I am looking are how erase suspend
> failure are handled

  I see and noted it.

> >
> > Regards,
> > Ikegami
> >
> > -----Original Message-----
> > From: Joakim Tjernlund [mailto:Joakim.Tjernlund at infinera.com]
> > Sent: Tuesday, April 24, 2018 8:05 PM
> > To: IKEGAMI Tokunori; linux-mtd at lists.infradead.org
> > Cc: PACKHAM Chris
> > Subject: Re: mtd: cfi_cmdset_0002: Read data 3 times to check write
> operation status
> >
> > On Tue, 2018-04-24 at 05:54 +0000, IKEGAMI Tokunori wrote:
> > >
> > > Hi,
> > >
> > > Let us consult to change mtd cfi_cmdset_0002 to read data 3 times as
> below patch.
> > > Can we change the mtd cfi_cmdset_0002 driver like this?
> > > If any comment or concern please let us know.
> > >
> > > From d924822c996b9c0eccc815e5018a0c3ea6077137 Mon Sep 17 00:00:00 2001
> > > From: Tokunori Ikegami <ikegami at allied-telesis.co.jp>
> > > Date: Tue, 24 Apr 2018 13:26:37 +0900
> > > Subject: [PATCH 2/2] mtd: cfi_cmdset_0002: Read data 3 times to check
> write
> > >  operation status
> > >
> > > Cypress S29GLxxxP flash is needed to read data 3 times to check DQ6
> toggles.
> > > Actually the read data is sometimes changed by the 3rd reading.
> > > Also this is caused on other flash device also.
> > > The flash write failure is possible to be caused by the error.
> > > To resolve the issue change the read number of times to 3 from 2.
> >
> > Hi
> >
> > I find this a bit odd. I am looking at some other erase suspend problems
> we
> > see on Cypress AMD NOR flash and I have noted that there is more than
> just DQ6 toggling.
> > Checking DQ5 in combination with DQ6 is one.
> > How did you you arrive at reading 3 times is correct? Is the any
> documentation
> > supporting this?
> >
> > All in all, error handling in cmdset0002 is lacking.
> >
> >  Jocke
> >