diff mbox series

Fixed endless loop problem in CFI when value was written but corrupted.

Message ID 20190108165125.GA95492@dev-dsk-psobon-2c-1dd9f399.us-west-2.amazon.com
State Superseded
Delegated to: Boris Brezillon
Headers show
Series Fixed endless loop problem in CFI when value was written but corrupted. | expand

Commit Message

Przemyslaw Sobon Jan. 8, 2019, 4:51 p.m. UTC
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>
---
 drivers/mtd/chips/cfi_cmdset_0002.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

Comments

Boris Brezillon Jan. 14, 2019, 8:45 a.m. UTC | #1
Hi Przemyslaw,

Subject prefix should be "mtd: cfi: ".

On Tue, 8 Jan 2019 16:51:25 +0000
Przemyslaw Sobon <psobon@amazon.com> wrote:

> 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>
> ---
>  drivers/mtd/chips/cfi_cmdset_0002.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
> index 72428b6bfc47..6cc31d2057e9 100644
> --- a/drivers/mtd/chips/cfi_cmdset_0002.c
> +++ b/drivers/mtd/chips/cfi_cmdset_0002.c
> @@ -1879,15 +1879,18 @@ static int __xipram do_write_buffer(struct map_info *map, struct flchip *chip,
>  		if (time_after(jiffies, timeo) && !chip_ready(map, adr))
>  			break;
>  
> -		if (chip_good(map, adr, datum)) {
> -			xip_enable(map, chip, adr);
> -			goto op_done;
> -		}
> +		if (chip_ready(map, adr))
> +			break;
>  
>  		/* Latency issues. Drop the lock, wait a while and retry */
>  		UDELAY(map, chip, adr, 1);
>  	}
>  
> +	if (chip_good(map, adr, datum)) {
> +		xip_enable(map, chip, adr);
> +		goto op_done;
> +	}
> +
>  	/*
>  	 * Recovery from write-buffer programming failures requires
>  	 * the write-to-buffer-reset sequence.  Since the last part

Can you look at this series [1], it might fix your problem. I didn't
apply these patches because I was not happy with the explanation given
in the commit message and Ikegami never sent a v4 to address that :-/.

Regards,

Boris

[1]http://patchwork.ozlabs.org/project/linux-mtd/list/?series=72607
diff mbox series

Patch

diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
index 72428b6bfc47..6cc31d2057e9 100644
--- a/drivers/mtd/chips/cfi_cmdset_0002.c
+++ b/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -1879,15 +1879,18 @@  static int __xipram do_write_buffer(struct map_info *map, struct flchip *chip,
 		if (time_after(jiffies, timeo) && !chip_ready(map, adr))
 			break;
 
-		if (chip_good(map, adr, datum)) {
-			xip_enable(map, chip, adr);
-			goto op_done;
-		}
+		if (chip_ready(map, adr))
+			break;
 
 		/* Latency issues. Drop the lock, wait a while and retry */
 		UDELAY(map, chip, adr, 1);
 	}
 
+	if (chip_good(map, adr, datum)) {
+		xip_enable(map, chip, adr);
+		goto op_done;
+	}
+
 	/*
 	 * Recovery from write-buffer programming failures requires
 	 * the write-to-buffer-reset sequence.  Since the last part