Linux-mtd Archive on lore.kernel.org
 help / color / Atom feed
From: John Garry <john.garry@huawei.com>
To: <Tudor.Ambarus@microchip.com>, <linux-mtd@lists.infradead.org>
Cc: broonie@kernel.org, fengsheng5@huawei.com
Subject: Re: flash_lock issue for n25q 128mb spi nor part
Date: Tue, 3 Dec 2019 13:57:35 +0000
Message-ID: <b73c8a25-a58d-a1f1-f68c-0ba35a5c7c51@huawei.com> (raw)
In-Reply-To: <00cf6eab-9798-b0e9-e4a2-5b2f8374b698@huawei.com>

On 03/12/2019 12:35, John Garry wrote:
> On 03/12/2019 12:27, Tudor.Ambarus@microchip.com wrote:
>>
>>
>> On 12/3/19 2:05 PM, Tudor.Ambarus@microchip.com wrote:
>>> diff --git a/drivers/mtd/spi-nor/spi-nor.c 
>>> b/drivers/mtd/spi-nor/spi-nor.c
>>> index f4afe123e9dc..f1490c7b5cb9 100644
>>> --- a/drivers/mtd/spi-nor/spi-nor.c
>>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>>> @@ -1033,10 +1033,19 @@ static int 
>>> spi_nor_write_16bit_sr_and_check(struct spi_nor *nor, u8 sr1)
>>>
>>>          sr_cr[0] = sr1;
>>>
>>> +       dev_err(nor->dev, "before write: sr_cr[0] = %02x, sr_cr[1] = 
>>> %02x\n",
>>> +               sr_cr[0], sr_cr[1]);
>>> +
>>>          ret = spi_nor_write_sr(nor, sr_cr, 2);
>>>          if (ret)
>>>                  return ret;
>>>
>>> +       ret = spi_nor_read_sr(nor, &sr_cr[0]);
>>> +       if (ret)
>>> +               return ret;
>>> +
>>> +       dev_err(nor->dev, "read back sr1: sr_cr[0] = %02x\n", sr_cr[0]);
>>> +
>>>          if (nor->flags & SNOR_F_NO_READ_CR)
>>>                  return 0;
>>>
>>> @@ -1046,6 +1055,8 @@ static int 
>>> spi_nor_write_16bit_sr_and_check(struct spi_nor *nor, u8 sr1)
>>>          if (ret)
>>>                  return ret;
>>>
>>> +       dev_err(nor->dev, "read back sr2: sr_cr[1] = %02x\n", sr_cr[1]);
>>> +
>>>          if (cr_written != sr_cr[1]) {
>>>                  dev_dbg(nor->dev, "CR: read back test failed\
>>
>> On n25q256a I obtain:
>>
>> root@sama5d2-xplained-sd:~# flash_lock -l /dev/mtd1
>> spi-nor spi1.0: before write: sr_cr[0] = 9e, sr_cr[1] = ff
>> spi-nor spi1.0: read back sr1: sr_cr[0] = 02
>> spi-nor spi1.0: read back sr2: sr_cr[1] = ff
>>
> 
> So here is what I get:
> 
> root@ubuntu:/home/john# flash_lock -l /dev/mtd0
> [  109.037492] spi-nor spi-PRP0001:00: before write: sr_cr[0] = 9e, 
> sr_cr[1] = 00
> [  109.044769] spi-nor spi-PRP0001:00: read back sr1: sr_cr[0] = 02
> [  109.050790] spi-nor spi-PRP0001:00: read back sr2: sr_cr[1] = 00
> 
> 
>> the 16 bit write SR does not execute correctly and the WE remains set. If
>> neither of the micron flashes do not support the 16 bit write SR, we 
>> can add a
>> condition based on MFR. Let me check few datasheets.
>>
> 
> OK, thanks.

Hi Tudor,

Something else which I wanted to ask is why we don't check the FSR after 
a write command for that part?

Without it, if the flash is locked, a flash write reports no error, 
which could be maybe misleading.

A more real problem is that I find that SR.WEL is not cleared when we 
attempt to write when the flash is locked, and this causes issues when 
we attempt to reset the SR.

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.

I'm just saying this while it's fresh in my head - I don't want to seem 
pushy :)

Thanks,
John

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

  reply index

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-02 17:28 John Garry
2019-12-03  9:45 ` Tudor.Ambarus
2019-12-03 10:31   ` John Garry
2019-12-03 11:07     ` Tudor.Ambarus
2019-12-03 11:44       ` John Garry
2019-12-03 12:05         ` Tudor.Ambarus
2019-12-03 12:27           ` Tudor.Ambarus
2019-12-03 12:35             ` John Garry
2019-12-03 13:57               ` John Garry [this message]
2019-12-03 14:44                 ` Tudor.Ambarus
2019-12-03 15:29                   ` John Garry
2019-12-04 11:10                     ` John Garry
2019-12-03 14:16               ` [PATCH] mtd: spi-nor: Fix the write Status Register on micron flashes Tudor.Ambarus
2019-12-03 14:50                 ` [PATCH v2] mtd: spi-nor: Fix the writing of the " Tudor.Ambarus
2019-12-04 10:18                   ` John Garry

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b73c8a25-a58d-a1f1-f68c-0ba35a5c7c51@huawei.com \
    --to=john.garry@huawei.com \
    --cc=Tudor.Ambarus@microchip.com \
    --cc=broonie@kernel.org \
    --cc=fengsheng5@huawei.com \
    --cc=linux-mtd@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-mtd Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-mtd/0 linux-mtd/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-mtd linux-mtd/ https://lore.kernel.org/linux-mtd \
		linux-mtd@lists.infradead.org
	public-inbox-index linux-mtd

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-mtd


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git