On Mon, 5 Nov 2018 12:03:04 +0000 Joakim Tjernlund <Joakim.Tjernlund@xxxxxxxxxxxx> wrote: > On Mon, 2018-11-05 at 11:15 +0100, Boris Brezillon 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. > > > > > > On Fri, 26 Oct 2018 01:32:09 +0900 > > Tokunori Ikegami <ikegami@xxxxxxxxxxxxxxxxxxxx> wrote: > > > > > In OpenWrt Project the flash write error caused on some products. > > > > It's okay to mention that the issue was discovered by the OpenWRT team, > > but I'd rephrase it differently. > > > > "As reported by the OpenWRT team, write requests sometimes fail on some > > platforms". > > > > > Also the issue can be fixed by using chip_good() instead of chip_ready(). > > > The chip_ready() just checks the value from flash memory twice. > > > And the chip_good() checks the value with the expected value. > > > Probably the issue can be fixed as checked correctly by the chip_good(). > > > So change to use chip_good() instead of chip_ready(). > > > > Well, that's not really explaining why you think chip_good() should be > > used instead of chip_ready(). So I went on and looked at the > > chip_good(), chip_ready() and do_write_oneword() implementation, and > > also looked at users of do_write_oneword(). It seems this function is > > used to write data to the flash, and apparently the "one bit should > > toggle to reflect a busy state" does not apply when writing things to > > the memory array (it's probably working for other CFI commands, but I > > guess it takes more time to actually change the level of a NOR cell, > > hence the result of 2 identical reads does not mean that the write is > > done). > > > > Also, it seems that cmdset_0001 is not implementing chip_ready() the > > same way, and I wonder if cmdset_0002 implementation is correct to > > start with. Or maybe I don't get what chip_ready() is for. > > > The 0001 cmd set is quite different to 0002 and 0001 is the superior one. > If you look at recent 0002 cmd sets they offer an alternative cmd > set to replace the all the "toggle" ones with something that is > same/similar to what 0001 offers. Okay. Do you know when chip_ready() (the one that checks if something changes between 2 reads) should be used and when it shouldn't?