Re: [PATCH v3 2/3] mtd: spi-nor: add 4bit block protection support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Jungseung Lee,

在 2020/1/17 23:06, Jungseung Lee 写道:
Hi, Tudor,

On Tue, Jan 14, 2020 at 7:49 PM <Tudor.Ambarus@xxxxxxxxxxxxx> wrote:
Hi, Jungseung,

Thanks for working on this.

On Monday, January 13, 2020 7:59:06 AM EET Jungseung Lee wrote:
EXTERNAL EMAIL: Do not click links or open attachments unless you know the
content is safe

Currently, we are supporting block protection only for
flash chips with 3 block protection bits in the SR register.
This patch enables block protection support for some flash with
4 block protection bits(bp0-3).
Some? Isn't this generic for all the flashes that support BP0-3?

This one would be a generic solution to support BP0-3 on Status Register.
>From my study, this covers all the flashes listed on spi-nor.c that
have BP0-3 bit on SR.
It looks like I have to change this description.

Note that it is NOT for some flashes that have BP0-3 in another register.
As you know, just like SPI_NOR_HAS_TB did.

Signed-off-by: Jungseung Lee <js07.lee@xxxxxxxxxxx>
---
v3 :
   Fix wrong ofs calculation on v2 patch
v2 :
   Add sample table portion about 4bit block protection on the comment
   Trivial coding style change

  drivers/mtd/spi-nor/spi-nor.c | 127 +++++++++++++++++++++++++++++-----
  include/linux/mtd/spi-nor.h   |   8 +++
  2 files changed, 119 insertions(+), 16 deletions(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index e3da6a8654a8..7e8af6c4fdfa 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -238,6 +238,14 @@ struct flash_info {
                                          * status register. Must be used
with * SPI_NOR_HAS_TB.
                                          */
+#define SPI_NOR_HAS_BP3                BIT(17) /*
+                                        * Flash SR has 4 bit fields (BP0-3)
+                                        * for block protection.
+                                        */
+#define SPI_NOR_BP3_SR_BIT6    BIT(18) /*
+                                        * BP3 is bit 6 of status register.
+                                        * Must be used with
Are we safe to replace SPI_NOR_TB_SR_BIT6 and SPI_NOR_BP3_SR_BIT6 with a
SPI_NOR_SR_TB_BIT6_BP3_BIT5? Or maybe with a SPI_NOR_SR_BP3_BIT6_TB_BIT5, how
is more convenient?

Let's think about some flash in which BP0-3 exists in the status
register but TB exists in another register.
for example, mx25u12835f.
I haven't tested yet, but according to the datasheet, I think this
patch can support 4bit block protection for the flash.

In order to embrace the case, how about letting them as It is.
Is there any suggestion?

In my board(hisilicon D06) there is a mx25u12835f Flash, and i enable SPI_NOR_HAS_LOCK and SPI_NOR_HAS_BP3 which is not enabled in spi-nor.c, find it seems not work:

Euler:~ # ls /dev/mtd0
/dev/mtd0
Euler:~ # ./flash_lock -i /dev/mtd0
Device: /dev/mtd0
Start: 0
Len: 0x1000000
Lock status: unlocked
Return code: 0
Euler:~ #
Euler:~ # ./flash_lock /dev/mtd0
flash_lock: error!: could not lock device: /dev/mtd0

            error 5 (Input/output error)
Euler:~ # ./flash_lock -i /dev/mtd0
Device: /dev/mtd0
Start: 0
Len: 0x1000000
Lock status: locked
Return code: 1
Euler:~ #

I find that in function spi_nor_write_sr_and_check(), if spi_nor has 16bit SR, spi_nor_write_16bit_sr_and_check() will be called, if not spi_nor_write_sr1_and_check() will be called. For mx25u12835f, it just has 8bit SR, but SNOR_F_HAS_16BIT_SR is set as default, so it causes above issue; Even hack the code to not set flag SNOR_F_HAS_16BIT_SR, it seems that lock is still not valided as Program/Erase still success even if it is locked:

Euler:~ # ./flash_lock -i /dev/mtd0
Device: /dev/mtd0
Start: 0
Len: 0x1000000
Lock status: unlocked
Return code: 0
Euler:~ #
Euler:~ # ./mtd_debug erase /dev/mtd0 0xe40000 4096
Erased 4096 bytes from address 0x00e40000 in flash
Euler:~ #
Euler:~ # ./flash_lock /dev/mtd0
Euler:~ #
Euler:~ # ./flash_lock -i /dev/mtd0
Device: /dev/mtd0
Start: 0
Len: 0x1000000
Lock status: locked
Return code: 1
Euler:~ #
Euler:~ # ./mtd_debug erase /dev/mtd0 0xe40000 4096
Erased 4096 bytes from address 0x00e40000 in flash
Euler:~ # ./flash_lock -i /dev/mtd0
Device: /dev/mtd0
Start: 0
Len: 0x1000000
Lock status: locked
Return code: 1
Euler:~ #
Euler:~ #


Thanks,
chenxiang



Cheers,
ta

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

.




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




[Index of Archives]     [LARTC]     [Bugtraq]     [Yosemite Forum]     [Photo]

  Powered by Linux