[PATCH] mtd: spi-nor: Describe locking support for MX25L51245G

[PATCH] mtd: spi-nor: Describe locking support for MX25L51245G

[Marek Vasut]

The MX25L51245G part has the same 4-BP locking mechanism as MX25L12805D.
Describe the locking support.

Signed-off-by: Marek Vasut <marex@nabladev.com>
---
Cc: Cheng Ming Lin <chengminglin@mxic.com.tw>
Cc: Michael Walle <mwalle@kernel.org>
Cc: Miquel Raynal <miquel.raynal@bootlin.com>
Cc: Pratyush Yadav <pratyush@kernel.org>
Cc: Richard Weinberger <richard@nod.at>
Cc: Takahiro Kuwano <takahiro.kuwano@infineon.com>
Cc: Vignesh Raghavendra <vigneshr@ti.com>
Cc: linux-kernel@vger.kernel.org
Cc: linux-mtd@lists.infradead.org
---
 drivers/mtd/spi-nor/macronix.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
index b3fdb4b2fa1ca..4de1b490b16b6 100644
--- a/drivers/mtd/spi-nor/macronix.c
+++ b/drivers/mtd/spi-nor/macronix.c
@@ -165,6 +165,7 @@ static const struct flash_info macronix_nor_parts[] = {
 	}, {
 		/* MX25L51245G, MX25L51273G, MX66L51235F */
 		.id = SNOR_ID(0xc2, 0x20, 0x1a),
+		.flags = SPI_NOR_HAS_LOCK | SPI_NOR_4BIT_BP,
 	}, {
 		/* MX66L1G45G */
 		.id = SNOR_ID(0xc2, 0x20, 0x1b),
-- 
2.53.0

RE: [PATCH] mtd: spi-nor: Describe locking support for MX25L51245G

[Takahiro.Kuwano]

Hi Marek,

> The MX25L51245G part has the same 4-BP locking mechanism as MX25L12805D.
> Describe the locking support.
> 
> Signed-off-by: Marek Vasut <marex@nabladev.com>
> ---
> Cc: Cheng Ming Lin <chengminglin@mxic.com.tw>
> Cc: Michael Walle <mwalle@kernel.org>
> Cc: Miquel Raynal <miquel.raynal@bootlin.com>
> Cc: Pratyush Yadav <pratyush@kernel.org>
> Cc: Richard Weinberger <richard@nod.at>
> Cc: Takahiro Kuwano <takahiro.kuwano@infineon.com>
> Cc: Vignesh Raghavendra <vigneshr@ti.com>
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-mtd@lists.infradead.org
> ---

Can you test locking as described in [1] and provide test log?

>  drivers/mtd/spi-nor/macronix.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
> index b3fdb4b2fa1ca..4de1b490b16b6 100644
> --- a/drivers/mtd/spi-nor/macronix.c
> +++ b/drivers/mtd/spi-nor/macronix.c
> @@ -165,6 +165,7 @@ static const struct flash_info macronix_nor_parts[] = {
>         }, {
>                 /* MX25L51245G, MX25L51273G, MX66L51235F */
>                 .id = SNOR_ID(0xc2, 0x20, 0x1a),
> +               .flags = SPI_NOR_HAS_LOCK | SPI_NOR_4BIT_BP,

According to datasheets [2][3][4], those chips have TB bit in Configuration
Register bit-3 and the bit is OTP. The existing HAS_TB and HAS_TB_SR_BIT6
flags don't work as they expect non-OTP bit in Status Register.
Just supporting top area protection (TB==0) would be fine with me, but in
that case, fixup should check TB and activate locking only when TB==0.

Thanks,
Takahiro

>         }, {
>                 /* MX66L1G45G */
>                 .id = SNOR_ID(0xc2, 0x20, 0x1b),
> --
> 2.53.0

[1] https://docs.kernel.org/driver-api/mtd/spi-nor.html
[2] https://www.macronix.com/Lists/Datasheet/Attachments/8381/MX25L51245G,%203V,%20512Mb,%20v1.3.pdf
[3] https://www.mxic.com.tw/Lists/Datasheet/Attachments/9176/MX25L51273G,%203V,%20512Mb,%20v1.7.pdf
[4] https://www.macronix.com/Lists/Datasheet/Attachments/8657/MX66L51235F,%203V,%20512Mb,%20v1.1.pdf

Re: [PATCH] mtd: spi-nor: Describe locking support for MX25L51245G

[Marek Vasut]

On 10/1/26 9:17 AM, Takahiro.Kuwano@infineon.com wrote:

Hello Kuwano-san,

I apologize for my late reply.

>> The MX25L51245G part has the same 4-BP locking mechanism as MX25L12805D.
>> Describe the locking support.
>>
>> Signed-off-by: Marek Vasut <marex@nabladev.com>
>> ---
>> Cc: Cheng Ming Lin <chengminglin@mxic.com.tw>
>> Cc: Michael Walle <mwalle@kernel.org>
>> Cc: Miquel Raynal <miquel.raynal@bootlin.com>
>> Cc: Pratyush Yadav <pratyush@kernel.org>
>> Cc: Richard Weinberger <richard@nod.at>
>> Cc: Takahiro Kuwano <takahiro.kuwano@infineon.com>
>> Cc: Vignesh Raghavendra <vigneshr@ti.com>
>> Cc: linux-kernel@vger.kernel.org
>> Cc: linux-mtd@lists.infradead.org
>> ---
> 
> Can you test locking as described in [1] and provide test log?
> 
>>   drivers/mtd/spi-nor/macronix.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
>> index b3fdb4b2fa1ca..4de1b490b16b6 100644
>> --- a/drivers/mtd/spi-nor/macronix.c
>> +++ b/drivers/mtd/spi-nor/macronix.c
>> @@ -165,6 +165,7 @@ static const struct flash_info macronix_nor_parts[] = {
>>          }, {
>>                  /* MX25L51245G, MX25L51273G, MX66L51235F */
>>                  .id = SNOR_ID(0xc2, 0x20, 0x1a),
>> +               .flags = SPI_NOR_HAS_LOCK | SPI_NOR_4BIT_BP,
> 
> According to datasheets [2][3][4], those chips have TB bit in Configuration
> Register bit-3 and the bit is OTP.

Yikes, that is correct and also scary.

> The existing HAS_TB and HAS_TB_SR_BIT6
> flags don't work as they expect non-OTP bit in Status Register.
> Just supporting top area protection (TB==0) would be fine with me, but in
> that case, fixup should check TB and activate locking only when TB==0.

I came up with this addition, is this what you have in mind ?

diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
index e97f5cbd9aad2..1539ff3ed404e 100644
--- a/drivers/mtd/spi-nor/macronix.c
+++ b/drivers/mtd/spi-nor/macronix.c
@@ -15,6 +15,9 @@
  #define MXIC_NOR_REG_DOPI_EN	0x2		/* Enable Octal DTR */
  #define MXIC_NOR_REG_SPI_EN	0x0		/* Enable SPI */

+#define MX25L51245G_OP_RDCR	0x15
+#define MX25L51245G_CR_TB	BIT(3)
+
  /* Convert dummy cycles to bit pattern */
  #define MXIC_NOR_REG_DC(p) \
  	((20 - (p)) >> 1)
@@ -45,6 +48,52 @@ mx25l25635_post_bfpt_fixups(struct spi_nor *nor,
  	return 0;
  }

+static int mx25l51245_read_cr(struct spi_nor *nor, u8 *cr)
+{
+	int ret;
+
+	if (nor->spimem) {
+		struct spi_mem_op op = SPI_MEM_OP(SPI_MEM_OP_CMD(MX25L51245G_OP_RDCR, 0),
+						  SPI_MEM_OP_NO_ADDR, SPI_MEM_OP_NO_DUMMY,
+						  SPI_MEM_OP_DATA_IN(1, cr, 0));
+
+		spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
+
+		ret = spi_mem_exec_op(nor->spimem, &op);
+	} else {
+		ret = spi_nor_controller_ops_read_reg(nor, MX25L51245G_OP_RDCR, cr, 1);
+	}
+
+	if (ret)
+		dev_dbg(nor->dev, "error %d reading CR\n", ret);
+
+	return ret;
+}
+
+static int
+mx25l51245_post_bfpt_fixups(struct spi_nor *nor,
+			    const struct sfdp_parameter_header *bfpt_header,
+			    const struct sfdp_bfpt *bfpt)
+{
+	int ret;
+
+	ret = mx25l51245_read_cr(nor, nor->bouncebuf);
+	if (ret)
+		return ret;
+
+	/*
+	 * MX25L51245G supports locking, but the CR register TB bit is OTP.
+	 * Enable locking only in case the TB bit is not programmed, or set
+	 * to 0.
+	 */
+	if (nor->bouncebuf[0] & MX25L51245G_CR_TB)
+		dev_dbg(nor->dev, "CR TB is set, bottom protection not supported\n");
+	else
+		nor->flags |= SNOR_F_HAS_LOCK | SNOR_F_HAS_4BIT_BP;
+
+	return 0;
+}
+
  static int
  macronix_qpp4b_post_sfdp_fixups(struct spi_nor *nor)
  {
@@ -89,6 +139,7 @@ static const struct spi_nor_fixups mx25l25635_fixups = {
  };

  static const struct spi_nor_fixups macronix_qpp4b_fixups = {
+	.post_bfpt = mx25l51245_post_bfpt_fixups,
  	.post_sfdp = macronix_qpp4b_post_sfdp_fixups,
  };

Which led to this locking log:

"
+ mktemp -d
+ cd /tmp/tmp.45Mtr7ceEE
+ cat /sys/bus/spi/devices/spi2.0/spi-nor/jedec_id
c2201a
+ cat /sys/bus/spi/devices/spi2.0/spi-nor/manufacturer
macronix
+ xxd -p /sys/bus/spi/devices/spi2.0/spi-nor/sfdp
53464450060102ff00060110300000ffc2000104100100ff84000102c000
00ffffffffffffffffffffffffffffffffffe520fbffffffff1f44eb086b
083b04bbfeffffffffff00ffffff44eb0c200f5210d800ffd649c50081df
04e34403673830b030b0f7bdd55c4a9e29fff050f985ffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffff7fefffff215cdcffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffff003600279df9c06485cbffffffffffff
+ cat /sys/kernel/debug/spi-nor/spi2.0/capabilities
Supported read modes by the flash
  1S-1S-1S
   opcode        0x13
   mode cycles   0
   dummy cycles  0
  1S-1S-2S
   opcode        0x3c
   mode cycles   0
   dummy cycles  8
  1S-2S-2S
   opcode        0xbc
   mode cycles   0
   dummy cycles  4
  1S-1S-4S
   opcode        0x6c
   mode cycles   0
   dummy cycles  8
  1S-4S-4S
   opcode        0xec
   mode cycles   2
   dummy cycles  4
  4S-4S-4S
   opcode        0xec
   mode cycles   2
   dummy cycles  4

Supported page program modes by the flash
  1S-1S-1S
   opcode        0x12
  1S-1S-4S
   opcode        0x34
  1S-4S-4S
   opcode        0x3e
+ cat /sys/kernel/debug/spi-nor/spi2.0/params
name            (null)
id              c2 20 1a c2 20 1a
size            64.0 MiB
write size      1
page size       256
address nbytes  4
flags           4B_OPCODES | HAS_4BAIT | HAS_LOCK | HAS_4BIT_BP | SOFT_RESET

opcodes
  read           0xec
   dummy cycles  6
  erase          0x21
  program        0x3e
  8D extension   none

protocols
  read           1S-4S-4S
  write          1S-4S-4S
  register       1S-1S-1S

erase commands
  21 (4.00 KiB) [1]
  5c (32.0 KiB) [2]
  dc (64.0 KiB) [3]
  c7 (64.0 MiB)

sector map
  region (in hex)   | erase mask | overlaid
  ------------------+------------+----------
  00000000-03ffffff |     [ 1  ] | no
+ dd if=/dev/urandom of=./spi_test bs=1M count=2
2+0 records in
2+0 records out
2097152 bytes (2.1 MB, 2.0 MiB) copied, 0.0514406 s, 40.8 MB/s
+ mtd_debug erase /dev/mtd0 0 2097152
Erased 2097152 bytes from address 0x00000000 in flash
+ mtd_debug read /dev/mtd0 0 2097152 spi_read
Copied 2097152 bytes from address 0x00000000 in flash to spi_read
+ hexdump spi_read
0000000 ffff ffff ffff ffff ffff ffff ffff ffff
*
0200000
+ sha256sum spi_read
4bda3a28f4ffe603c0ec1258c0034d65a1a0d35ab7bd523a834608adabf03cc5  spi_read
+ mtd_debug write /dev/mtd0 0 2097152 spi_test
Copied 2097152 bytes from spi_test to address 0x00000000 in flash
+ mtd_debug read /dev/mtd0 0 2097152 spi_read
Copied 2097152 bytes from address 0x00000000 in flash to spi_read
+ sha256sum spi_read spi_test
fa5dea0c7a85ed0bc865bd0b6ed09e2aef41bb336d9805a93bcfbbd26004ea7a  spi_read
fa5dea0c7a85ed0bc865bd0b6ed09e2aef41bb336d9805a93bcfbbd26004ea7a  spi_test
+ mtd_debug erase /dev/mtd0 0 2097152
Erased 2097152 bytes from address 0x00000000 in flash
+ mtd_debug read /dev/mtd0 0 2097152 spi_read
Copied 2097152 bytes from address 0x00000000 in flash to spi_read
+ sha256sum spi_read spi_test
4bda3a28f4ffe603c0ec1258c0034d65a1a0d35ab7bd523a834608adabf03cc5  spi_read
fa5dea0c7a85ed0bc865bd0b6ed09e2aef41bb336d9805a93bcfbbd26004ea7a  spi_test
+ mtd_debug info /dev/mtd0
mtd.type = MTD_NORFLASH
mtd.flags = MTD_CAP_NORFLASH
mtd.size = 67108864 (64M)
mtd.erasesize = 4096 (4K)
mtd.writesize = 1
mtd.oobsize = 0
regions = 0

+ flash_lock -u /dev/mtd0
+ flash_lock -i /dev/mtd0
Device: /dev/mtd0
Start: 0
Len: 0x4000000
Lock status: unlocked
Return code: 0
+ mtd_debug erase /dev/mtd0 0 2097152
Erased 2097152 bytes from address 0x00000000 in flash
+ mtd_debug write /dev/mtd0 0 2097152 spi_test
Copied 2097152 bytes from spi_test to address 0x00000000 in flash
+ mtd_debug read /dev/mtd0 0 2097152 spi_read
Copied 2097152 bytes from address 0x00000000 in flash to spi_read
+ sha256sum spi_read spi_test
fa5dea0c7a85ed0bc865bd0b6ed09e2aef41bb336d9805a93bcfbbd26004ea7a  spi_read
fa5dea0c7a85ed0bc865bd0b6ed09e2aef41bb336d9805a93bcfbbd26004ea7a  spi_test
+ grep -A4 locked sectors /sys/kernel/debug/spi-nor/spi2.0/params
+ flash_lock -l /dev/mtd0
+ flash_lock -i /dev/mtd0
Device: /dev/mtd0
Start: 0
Len: 0x4000000
Lock status: locked
Return code: 1
+ mtd_debug erase /dev/mtd0 0 2097152
Erased 2097152 bytes from address 0x00000000 in flash
+ mtd_debug read /dev/mtd0 0 2097152 spi_read
Copied 2097152 bytes from address 0x00000000 in flash to spi_read
+ sha256sum spi_read spi_test
fa5dea0c7a85ed0bc865bd0b6ed09e2aef41bb336d9805a93bcfbbd26004ea7a  spi_read
fa5dea0c7a85ed0bc865bd0b6ed09e2aef41bb336d9805a93bcfbbd26004ea7a  spi_test
+ dd if=/dev/urandom of=./spi_test2 bs=1M count=2
2+0 records in
2+0 records out
2097152 bytes (2.1 MB, 2.0 MiB) copied, 0.051125 s, 41.0 MB/s
+ mtd_debug write /dev/mtd0 0 2097152 spi_test2
Copied 2097152 bytes from spi_test2 to address 0x00000000 in flash
+ mtd_debug read /dev/mtd0 0 2097152 spi_read2
Copied 2097152 bytes from address 0x00000000 in flash to spi_read2
+ sha256sum spi_read spi_read2 spi_test spi_test2
fa5dea0c7a85ed0bc865bd0b6ed09e2aef41bb336d9805a93bcfbbd26004ea7a  spi_read
fa5dea0c7a85ed0bc865bd0b6ed09e2aef41bb336d9805a93bcfbbd26004ea7a  spi_read2
fa5dea0c7a85ed0bc865bd0b6ed09e2aef41bb336d9805a93bcfbbd26004ea7a  spi_test
eed3b73016802a59982c74a45ce96deafb7cd486cc5cd831e030239c0ddeb8cc  spi_test2
+ grep -A4 locked sectors /sys/kernel/debug/spi-nor/spi2.0/params
+ flash_lock -u /dev/mtd0
"