Re: mtd: cfi: Fixed endless loop problem in CFI when value was written but corrupted.

From: Boris Brezillon <boris.brezillon@collabora.com>
To: Mark Tomlinson <Mark.Tomlinson@alliedtelesis.co.nz>
Cc: "Joakim.Tjernlund@infinera.com" <Joakim.Tjernlund@infinera.com>,
	"ikegami@allied-telesis.co.jp" <ikegami@allied-telesis.co.jp>,
	Przemyslaw Sobon <psobon@amazon.com>,
	Chris Packham <Chris.Packham@alliedtelesis.co.nz>,
	"linux-mtd@lists.infradead.org" <linux-mtd@lists.infradead.org>,
	"liujian56@huawei.com" <liujian56@huawei.com>,
	"fbettoni@gmail.com" <fbettoni@gmail.com>
Subject: Re: mtd: cfi: Fixed endless loop problem in CFI when value was written but corrupted.
Date: Wed, 20 Feb 2019 09:03:20 +0100	[thread overview]
Message-ID: <20190220090320.01aa2357@collabora.com> (raw)
In-Reply-To: <f5d597dd-d09d-b8f8-a800-756a23954164@alliedtelesis.co.nz>

On Tue, 19 Feb 2019 20:02:37 +0000
Mark Tomlinson <Mark.Tomlinson@alliedtelesis.co.nz> wrote:

> On 19/02/19 9:00 PM, Boris Brezillon wrote:
> > On Thu, 14 Feb 2019 00:39:09 +0000
> > Chris Packham <Chris.Packham@alliedtelesis.co.nz> wrote:
> >  
> >> Hi All,
> >>
> >> On 8/02/19 12:58 PM, Przemyslaw Sobon wrote:  
> >>> Fixes: dfeae1073583(mtd: cfi_cmdset_0002: Change write buffer to
> >>>          check correct value)
> >>>
> >>> There was an endless loop in CFI Flash driver when a value was written
> >>> incorrectly. In such case chip_ready returns true but chip_good returns
> >>> false and we never get out of the loop.
> >>>
> >>> The solution was to break the loop in 2 cases, either device is ready or
> >>> device is not ready and timeout elapsed. The correctness of the write is
> >>> checked after the loop ended. That way we ensure the loop always ends.
> >>>
> >>> Signed-off-by: Przemyslaw Sobon <psobon@amazon.com>  
> >> Mark (cc'd) has done some testing here, and assuming he's happy with the
> >> forgery.
> >>
> >> Tested-by: Mark Tomlinson <Mark.Tomlinson@alliedtelesis.co.nz>  
> > I'm a bit lost. Ikegami told us that checking for chip_ready() was not
> > enough and chip_good() could return true after a few tests even though
> > it initially returned false.
> >
> > I'd really like to get that fixed, but it looks like you haven't reached
> > a consensus on what the appropriate fix is :-/.  
> I have done some further testing and this patch doesn't work 100%. It
> appears at least some flash chips do not start toggling immediately, and
> therefore chip_ready() can return true early. A timeout is reported,
> even though that isn't what happened.
> 
> chip_good() makes an additional check over chip_ready() and is the call
> I believe we should be using. I will submit a new patch which should fix
> the infinite loop as well as not mis-reporting errors.

No, please, don't do that. We already have 3 versions of the same fix
floating around (one from Ikegami, one from Liu Jian and another one
from Przemyslaw). Can you please sync and submit a single patch that
all of you agree on?

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/