Re: flash_lock issue for n25q 128mb spi nor part

[<Tudor.Ambarus@xxxxxxxxxxxxx>]

Hi, John,

On 12/4/19 1:10 PM, John Garry wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On 03/12/2019 15:29, John Garry wrote:
>>>>
>>>> Here's an example flow (with my hack to stop using 16b SR method):
>>>>
>>>> root@ubuntu:/home/john# flash_lock -l /dev/mtd0
>>>> root@ubuntu:/home/john# mtd_debug erase /dev/mtd0 0xe00000 4096
>>>> [   69.650642] spi-nor spi-PRP0001:00: at 0xe00000, len 4096
>>>> Erased 4096 bytes from address 0x00e00000 in flash
>>>> root@ubuntu:/home/john# mtd_debug write /dev/mtd0 0xe00000 4096 dump4096
>>>> [   77.093755] spi-nor spi-PRP0001:00: to 0x00e00000, len 4096
>>>> Copied 4096 bytes from dump4096 to address 0x00e00000 in flash
>>>> root@ubuntu:/home/john# mtd_debug read /dev/mtd0 0xe00000 4096 temp
>>>> [   82.162445] spi-nor spi-PRP0001:00: from 0x00e00000, len 4096
>>>> Copied 4096 bytes from address 0x00e00000 in flash to temp
>>>> root@ubuntu:/home/john# flash_lock -u /dev/mtd0
>>>> [   87.558435] spi-nor spi-PRP0001:00: SR1: read back test failed
>>>> flash_lock: error!: could not unlock device: /dev/mtd0
>>>>
>>>>              error 5 (Input/output error)
>>>> root@ubuntu:/home/john#
>>>>
>>>> Unlock reports an error as the the read back test in
>>>> spi_nor_write_sr1_and_check() fails as the SR.WEL has never been
>>>> cleared.
>>>>
>>>
>>> Interesting.
>>
>> I note that spi_nor_erase() exits with a call to
>> spi_nor_write_disable(), yet spi_nor_write() doesn't - maybe we should
>> add a similar call there.
> 
> So this remedies that issue for me:
> 
> commit c90de583d81f7c5c6cb1c8c021108a0e7e244b91 (HEAD)
> Author: John Garry <john.garry@xxxxxxxxxx>
> Date:   Wed Dec 4 10:26:49 2019 +0000
> 
>     Ensure we clear SR.WEL for any write failure due to locking
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index aeb3ad2dbfb8..3f12335eb20c 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -2875,6 +2875,8 @@ static int spi_nor_write(struct mtd_info *mtd,
> loff_t to, size_t len,
>                 i += written;
>         }
> 
> +       ret = spi_nor_write_disable(nor);
> +
>  write_err:
>         spi_nor_unlock_and_unprep(nor, SPI_NOR_OPS_WRITE);
>         return ret;
> 
> 
>>
>>>
>>>
>>> Does the following do the trick?
>>>
>>> -       { "n25q128a11",  INFO(0x20bb18, 0, 64 * 1024,  256, SECT_4K |
>>> SPI_NOR_QUAD_READ) },
>>> +       { "n25q128a11",  INFO(0x20bb18, 0, 64 * 1024,  256, SECT_4K |
>>> USE_FSR |
>>> +                             SPI_NOR_QUAD_READ) },
>>>
> 
> Yes, that works and it's nice to not have the silent failures:
> 
> root@ubuntu:/home/john# flash_lock -i /dev/mtd0
> Device: /dev/mtd0
> Start: 0
> Len: 0x1000000
> Lock status: locked
> Return code: 1
> root@ubuntu:/home/john#
> root@ubuntu:/home/john# mtd_debug erase /dev/mtd0 0xe00000 4096
> [   45.023353] spi-nor spi0.0: Erase operation failed.
> [   45.028257] spi-nor spi0.0: Attempted to modify a protected sector.
> MEMERASE: Input/output error
> mtd_debug write /dev/mtd0 0xe00000 4096 dump4096
> [   50.212013] spi-nor spi0.0: Program operation failed.
> [   50.217084] spi-nor spi0.0: Attempted to modify a protected sector.
> file_to_flash: write, size 0x1000, n 0x1000
> write(): Input/output error
> root@ubuntu:/home/john# mtd_debug read /dev/mtd0 0xe00000 4096 temp
> Copied 4096 bytes from address 0x00e00000 in flash to temp
> root@ubuntu:/home/john# flash_lock -u /dev/mtd0
> flash_lock: error!: could not unlock device: /dev/mtd0
> 
>             error 5 (Input/output error)
> root@ubuntu:/home/john# flash_lock -i /dev/mtd0
> Device: /dev/mtd0
> Start: 0
> Len: 0x1000000
> Lock status: unlocked
> Return code: 0
> root@ubuntu:/home/john# flash_lock -u /dev/mtd0
> root@ubuntu:/home/john#
> 
> But, as you may see, it seems that my change to spi_nor_write() is still
> required to stop the first unlock failure message, but it needs to be
> relocated to after write_err label, as we now jump there for
> spi_nor_wait_till_ready() failure. I guess the equivalent relocation is
> also required for spi_nor_erase().
> 
> Or maybe spi_nor_wait_till_ready() should clear this flag always.

I reproduced this on a n25q256a, with both erase and write. Did a lock,
an erase or write, and then the unlock raises an error on the read back test:
it receives 0x02 to write (the prev operation let the SR.WE set to 1),
and after write, it reads back 0x00 (which is correct, WE is de-asserted).

What is pretty strange is that Micron says about erase or program operations
that: "When the operation is in progress, the write in progress bit is set
to 1. The write enable latch bit is cleared to 0, whether the operation is
successful or not".

So what I guess it happens, is that when an erase/write command tries to
modify a software protected area, the flash completely ignores the command,
so no Write In Progress, and no clearing of the WE.

An idea would be to clear the WE in spi_nor_sr/fsr_ready() when errors.
I'll continue to debug/study this.

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