[PATCH v2 0/3] mtd: spi-nor: macronix: Cleanup macronix flash info

[PATCH v2 0/3] mtd: spi-nor: macronix: Cleanup macronix flash info

[Cheng Ming Lin]

From: Cheng Ming Lin <chengminglin@mxic.com.tw>

Organize Macronix flash information to make it more readable.

v2:
* Support 1-4-4 Read and 1-4-4 Page Program by adding fixups for MX25L3255E

Cheng Ming Lin (3):
  mtd: spi-nor: macronix: Drop the redundant flash info fields
  mtd: spi-nor: macronix: Remove duplicate flash info entries
  mtd: spi-nor: macronix: Add fixups for MX25L3255E

 drivers/mtd/spi-nor/macronix.c | 73 ++++++++++++++++++----------------
 1 file changed, 39 insertions(+), 34 deletions(-)

-- 
2.25.1

[PATCH v2 1/3] mtd: spi-nor: macronix: Drop the redundant flash info fields

[Cheng Ming Lin]

From: Cheng Ming Lin <chengminglin@mxic.com.tw>

Many flash devices share the same ID but have different part numbers.
To avoid confusion, the part number field is removed.

Additionally, since SFDP already provides size information and
functionality covered by no_sfdp_flags, these fields are also removed.

Furthermore, when 4-byte address instruction table is available,
the SPI_NOR_4B_OPCODES flag is no longer needed and is removed.

Signed-off-by: Cheng Ming Lin <chengminglin@mxic.com.tw>
Acked-by: Tudor Ambarus <tudor.ambarus@linaro.org>
---
 drivers/mtd/spi-nor/macronix.c | 36 +++++++++-------------------------
 1 file changed, 9 insertions(+), 27 deletions(-)

diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
index 4b09e5add630..2acd8e28d1c4 100644
--- a/drivers/mtd/spi-nor/macronix.c
+++ b/drivers/mtd/spi-nor/macronix.c
@@ -88,10 +88,8 @@ static const struct flash_info macronix_nor_parts[] = {
 		.name = "mx25l8005",
 		.size = SZ_1M,
 	}, {
+		/* MX25L1606E */
 		.id = SNOR_ID(0xc2, 0x20, 0x15),
-		.name = "mx25l1606e",
-		.size = SZ_2M,
-		.no_sfdp_flags = SECT_4K,
 	}, {
 		.id = SNOR_ID(0xc2, 0x20, 0x16),
 		.name = "mx25l3205d",
@@ -103,29 +101,21 @@ static const struct flash_info macronix_nor_parts[] = {
 		.size = SZ_8M,
 		.no_sfdp_flags = SECT_4K,
 	}, {
+		/* MX25L12805D */
 		.id = SNOR_ID(0xc2, 0x20, 0x18),
-		.name = "mx25l12805d",
-		.size = SZ_16M,
 		.flags = SPI_NOR_HAS_LOCK | SPI_NOR_4BIT_BP,
-		.no_sfdp_flags = SECT_4K,
 	}, {
+		/* MX25L25635E, MX25L25645G */
 		.id = SNOR_ID(0xc2, 0x20, 0x19),
-		.name = "mx25l25635e",
-		.size = SZ_32M,
-		.no_sfdp_flags = SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ,
 		.fixups = &mx25l25635_fixups
 	}, {
+		/* MX66L51235F */
 		.id = SNOR_ID(0xc2, 0x20, 0x1a),
-		.name = "mx66l51235f",
-		.size = SZ_64M,
-		.no_sfdp_flags = SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ,
 		.fixup_flags = SPI_NOR_4B_OPCODES,
 		.fixups = &macronix_qpp4b_fixups,
 	}, {
+		/* MX66L1G45G */
 		.id = SNOR_ID(0xc2, 0x20, 0x1b),
-		.name = "mx66l1g45g",
-		.size = SZ_128M,
-		.no_sfdp_flags = SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ,
 		.fixups = &macronix_qpp4b_fixups,
 	}, {
 		/* MX66L2G45G */
@@ -167,11 +157,8 @@ static const struct flash_info macronix_nor_parts[] = {
 		.size = SZ_16M,
 		.no_sfdp_flags = SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ,
 	}, {
+		/* MX25U51245G */
 		.id = SNOR_ID(0xc2, 0x25, 0x3a),
-		.name = "mx25u51245g",
-		.size = SZ_64M,
-		.no_sfdp_flags = SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ,
-		.fixup_flags = SPI_NOR_4B_OPCODES,
 		.fixups = &macronix_qpp4b_fixups,
 	}, {
 		.id = SNOR_ID(0xc2, 0x25, 0x3a),
@@ -185,11 +172,8 @@ static const struct flash_info macronix_nor_parts[] = {
 		.id = SNOR_ID(0xc2, 0x25, 0x3b),
 		.fixups = &macronix_qpp4b_fixups,
 	}, {
+		/* MX66U2G45G */
 		.id = SNOR_ID(0xc2, 0x25, 0x3c),
-		.name = "mx66u2g45g",
-		.size = SZ_256M,
-		.no_sfdp_flags = SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ,
-		.fixup_flags = SPI_NOR_4B_OPCODES,
 		.fixups = &macronix_qpp4b_fixups,
 	}, {
 		.id = SNOR_ID(0xc2, 0x26, 0x18),
@@ -215,15 +199,13 @@ static const struct flash_info macronix_nor_parts[] = {
 		.size = SZ_4M,
 		.no_sfdp_flags = SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ,
 	}, {
+		/* MX25UW51245G */
 		.id = SNOR_ID(0xc2, 0x81, 0x3a),
-		.name = "mx25uw51245g",
 		.n_banks = 4,
 		.flags = SPI_NOR_RWW,
 	}, {
+		/* MX25L3255E */
 		.id = SNOR_ID(0xc2, 0x9e, 0x16),
-		.name = "mx25l3255e",
-		.size = SZ_4M,
-		.no_sfdp_flags = SECT_4K,
 	},
 	/*
 	 * This spares us of adding new flash entries for flashes that can be
-- 
2.25.1

[PATCH v2 2/3] mtd: spi-nor: macronix: Remove duplicate flash info entries

[Cheng Ming Lin]

From: Cheng Ming Lin <chengminglin@mxic.com.tw>

To clean up the code and reduce duplication, this entry is removed.

Signed-off-by: Cheng Ming Lin <chengminglin@mxic.com.tw>
Reviewed-by: Tudor Ambarus <tudor.ambarus@linaro.org>
---
 drivers/mtd/spi-nor/macronix.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
index 2acd8e28d1c4..07e0bd0b70a0 100644
--- a/drivers/mtd/spi-nor/macronix.c
+++ b/drivers/mtd/spi-nor/macronix.c
@@ -160,13 +160,6 @@ static const struct flash_info macronix_nor_parts[] = {
 		/* MX25U51245G */
 		.id = SNOR_ID(0xc2, 0x25, 0x3a),
 		.fixups = &macronix_qpp4b_fixups,
-	}, {
-		.id = SNOR_ID(0xc2, 0x25, 0x3a),
-		.name = "mx66u51235f",
-		.size = SZ_64M,
-		.no_sfdp_flags = SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ,
-		.fixup_flags = SPI_NOR_4B_OPCODES,
-		.fixups = &macronix_qpp4b_fixups,
 	}, {
 		/* MX66U1G45G */
 		.id = SNOR_ID(0xc2, 0x25, 0x3b),
-- 
2.25.1

[PATCH v2 3/3] mtd: spi-nor: macronix: Add fixups for MX25L3255E

[Cheng Ming Lin]

From: Cheng Ming Lin <chengminglin@mxic.com.tw>

SFDP of MX25L3255E is JESD216, which does not include the Quad
Enable bit Requirement in BFPT. As a result, during BFPT parsing,
the quad_enable method is not set to spi_nor_sr1_bit6_quad_enable.
Therefore, it is necessary to correct this setting by late_init.

In addition, MX25L3255E also supports 1-4-4 page program in 3-byte
address mode. However, since the 3-byte address 1-4-4 page program
is not defined in SFDP, it needs to be configured in late_init.

Signed-off-by: Cheng Ming Lin <chengminglin@mxic.com.tw>
---
 drivers/mtd/spi-nor/macronix.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
index 07e0bd0b70a0..4bbd3b651cec 100644
--- a/drivers/mtd/spi-nor/macronix.c
+++ b/drivers/mtd/spi-nor/macronix.c
@@ -58,6 +58,31 @@ macronix_qpp4b_post_sfdp_fixups(struct spi_nor *nor)
 	return 0;
 }
 
+static int
+mx25l3255e_late_init_fixups(struct spi_nor *nor)
+{
+	/*
+	 * SFDP of MX25L3255E is JESD216, which does not include the Quad
+	 * Enable bit Requirement in BFPT. As a result, during BFPT parsing,
+	 * the quad_enable method is not set to spi_nor_sr1_bit6_quad_enable.
+	 * Therefore, it is necessary to correct this setting by late_init.
+	 */
+	nor->params->quad_enable = spi_nor_sr1_bit6_quad_enable;
+
+	/*
+	 * In addition, MX25L3255E also supports 1-4-4 page program in 3-byte
+	 * address mode. However, since the 3-byte address 1-4-4 page program
+	 * is not defined in SFDP, it needs to be configured in late_init.
+	 */
+	struct spi_nor_flash_parameter *params = nor->params;
+
+	params->hwcaps.mask |= SNOR_HWCAPS_PP_1_4_4;
+	spi_nor_set_pp_settings(&params->page_programs[SNOR_CMD_PP_1_4_4],
+				SPINOR_OP_PP_1_4_4, SNOR_PROTO_1_4_4);
+
+	return 0;
+}
+
 static const struct spi_nor_fixups mx25l25635_fixups = {
 	.post_bfpt = mx25l25635_post_bfpt_fixups,
 	.post_sfdp = macronix_qpp4b_post_sfdp_fixups,
@@ -67,6 +92,10 @@ static const struct spi_nor_fixups macronix_qpp4b_fixups = {
 	.post_sfdp = macronix_qpp4b_post_sfdp_fixups,
 };
 
+static const struct spi_nor_fixups mx25l3255e_fixups = {
+	.late_init = mx25l3255e_late_init_fixups,
+};
+
 static const struct flash_info macronix_nor_parts[] = {
 	{
 		.id = SNOR_ID(0xc2, 0x20, 0x10),
@@ -199,6 +228,7 @@ static const struct flash_info macronix_nor_parts[] = {
 	}, {
 		/* MX25L3255E */
 		.id = SNOR_ID(0xc2, 0x9e, 0x16),
+		.fixups = &mx25l3255e_fixups,
 	},
 	/*
 	 * This spares us of adding new flash entries for flashes that can be
-- 
2.25.1

Re: [PATCH v2 3/3] mtd: spi-nor: macronix: Add fixups for MX25L3255E

[Cheng Ming Lin]

Sorry for missing the minimum testing requirements.
Here are the details:

zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/jedec_id
c29e16
zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/manufacturer
macronix
zynq> hexdump -Cv /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
00000000  53 46 44 50 00 01 01 ff  00 00 01 09 30 00 00 ff  |SFDP........0...|
00000010  c2 00 01 04 60 00 00 ff  ff ff ff ff ff ff ff ff  |....`...........|
00000020  ff ff ff ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
00000030  e5 20 f1 ff ff ff ff 01  44 eb 08 6b 08 3b 04 bb  |. ......D..k.;..|
00000040  ee ff ff ff ff ff 00 ff  ff ff 00 ff 0c 20 0f 52  |............. .R|
00000050  10 d8 00 ff ff ff ff ff  ff ff ff ff ff ff ff ff  |................|
00000060  00 36 00 27 9e 49 ff ff  d9 f8 ff ff ff ff ff ff  |.6.'.I..........|
00000070
zynq> sha256sum /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
cf7b2e5b00388040786877de15f7594fcc064a1b7dbe34393a06d462cc1da9b0
/sys/bus/spi/devices/spi0.0/spi-nor/sfdp
zynq> cat /sys/kernel/debug/spi-nor/spi0.0/capabilities
Supported read modes by the flash
 1S-1S-1S
  opcode        0x03
  mode cycles   0
  dummy cycles  0
 1S-1S-2S
  opcode        0x3b
  mode cycles   0
  dummy cycles  8
 1S-2S-2S
  opcode        0xbb
  mode cycles   0
  dummy cycles  4
 1S-1S-4S
  opcode        0x6b
  mode cycles   0
  dummy cycles  8
 1S-4S-4S
  opcode        0xeb
  mode cycles   2
  dummy cycles  4

Supported page program modes by the flash
 1S-1S-1S
  opcode        0x02
 1S-4S-4S
  opcode        0x38
zynq> cat /sys/kernel/debug/spi-nor/spi0.0/params
name            (null)
id              c2 9e 16 c2 9e 16
size            4.00 MiB
write size      1
page size       256
address nbytes  3
flags           HAS_16BIT_SR

opcodes
 read           0xeb
  dummy cycles  6
 erase          0xd8
 program        0x38
 8D extension   none

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

erase commands
 20 (4.00 KiB) [1]
 52 (32.0 KiB) [2]
 d8 (64.0 KiB) [3]
 c7 (4.00 MiB)

sector map
 region (in hex)   | erase mask | overlaid
 ------------------+------------+----------
 00000000-003fffff |     [   3] | no
zynq> dd if=/dev/urandom of=/tmp/spi_test bs=1M count=2
2+0 records in
2+0 records out
2097152 bytes (2.0MB) copied, 0.084038 seconds, 23.8MB/s
zynq> mtd_debug erase /dev/mtd0 0 2097152
Erased 2097152 bytes from address 0x00000000 in flash
zynq> mtd_debug read /dev/mtd0 0 2097152 /tmp/spi_read
Copied 2097152 bytes from address 0x00000000 in flash to /tmp/spi_read
zynq> hexdump /tmp/spi_read
0000000 ffff ffff ffff ffff ffff ffff ffff ffff
*
0200000
zynq> sha256sum /tmp/spi_read
4bda3a28f4ffe603c0ec1258c0034d65a1a0d35ab7bd523a834608adabf03cc5  /tmp/spi_read
zynq> mtd_debug write /dev/mtd0 0 2097152 /tmp/spi_test
Copied 2097152 bytes from /tmp/spi_test to address 0x00000000 in flash
zynq> mtd_debug read /dev/mtd0 0 2097152 /tmp/spi_read
Copied 2097152 bytes from address 0x00000000 in flash to /tmp/spi_read
zynq> sha256sum /tmp/spi*
0be542d171336e052532d4b4f50b148293f7436f640211644df075da8682f54a  /tmp/spi_read
0be542d171336e052532d4b4f50b148293f7436f640211644df075da8682f54a  /tmp/spi_test
zynq> mtd_debug erase /dev/mtd0 0 2097152
Erased 2097152 bytes from address 0x00000000 in flash
zynq> mtd_debug read /dev/mtd0 0 2097152 /tmp/spi_read
Copied 2097152 bytes from address 0x00000000 in flash to /tmp/spi_read
zynq> sha256sum /tmp/spi*
4bda3a28f4ffe603c0ec1258c0034d65a1a0d35ab7bd523a834608adabf03cc5  /tmp/spi_read
0be542d171336e052532d4b4f50b148293f7436f640211644df075da8682f54a  /tmp/spi_test
zynq> mtd_debug info /dev/mtd0
mtd.type = MTD_NORFLASH
mtd.flags = MTD_CAP_NORFLASH
mtd.size = 4194304 (4M)
mtd.erasesize = 65536 (64K)
mtd.writesize = 1
mtd.oobsize = 0
regions = 0

Cheng Ming Lin <linchengming884@gmail.com> 於 2025年4月7日 週一 下午3:59寫道：
>
> From: Cheng Ming Lin <chengminglin@mxic.com.tw>
>
> SFDP of MX25L3255E is JESD216, which does not include the Quad
> Enable bit Requirement in BFPT. As a result, during BFPT parsing,
> the quad_enable method is not set to spi_nor_sr1_bit6_quad_enable.
> Therefore, it is necessary to correct this setting by late_init.
>
> In addition, MX25L3255E also supports 1-4-4 page program in 3-byte
> address mode. However, since the 3-byte address 1-4-4 page program
> is not defined in SFDP, it needs to be configured in late_init.
>
> Signed-off-by: Cheng Ming Lin <chengminglin@mxic.com.tw>
> ---
>  drivers/mtd/spi-nor/macronix.c | 30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
>
> diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
> index 07e0bd0b70a0..4bbd3b651cec 100644
> --- a/drivers/mtd/spi-nor/macronix.c
> +++ b/drivers/mtd/spi-nor/macronix.c
> @@ -58,6 +58,31 @@ macronix_qpp4b_post_sfdp_fixups(struct spi_nor *nor)
>         return 0;
>  }
>
> +static int
> +mx25l3255e_late_init_fixups(struct spi_nor *nor)
> +{
> +       /*
> +        * SFDP of MX25L3255E is JESD216, which does not include the Quad
> +        * Enable bit Requirement in BFPT. As a result, during BFPT parsing,
> +        * the quad_enable method is not set to spi_nor_sr1_bit6_quad_enable.
> +        * Therefore, it is necessary to correct this setting by late_init.
> +        */
> +       nor->params->quad_enable = spi_nor_sr1_bit6_quad_enable;
> +
> +       /*
> +        * In addition, MX25L3255E also supports 1-4-4 page program in 3-byte
> +        * address mode. However, since the 3-byte address 1-4-4 page program
> +        * is not defined in SFDP, it needs to be configured in late_init.
> +        */
> +       struct spi_nor_flash_parameter *params = nor->params;
> +
> +       params->hwcaps.mask |= SNOR_HWCAPS_PP_1_4_4;
> +       spi_nor_set_pp_settings(&params->page_programs[SNOR_CMD_PP_1_4_4],
> +                               SPINOR_OP_PP_1_4_4, SNOR_PROTO_1_4_4);
> +
> +       return 0;
> +}
> +
>  static const struct spi_nor_fixups mx25l25635_fixups = {
>         .post_bfpt = mx25l25635_post_bfpt_fixups,
>         .post_sfdp = macronix_qpp4b_post_sfdp_fixups,
> @@ -67,6 +92,10 @@ static const struct spi_nor_fixups macronix_qpp4b_fixups = {
>         .post_sfdp = macronix_qpp4b_post_sfdp_fixups,
>  };
>
> +static const struct spi_nor_fixups mx25l3255e_fixups = {
> +       .late_init = mx25l3255e_late_init_fixups,
> +};
> +
>  static const struct flash_info macronix_nor_parts[] = {
>         {
>                 .id = SNOR_ID(0xc2, 0x20, 0x10),
> @@ -199,6 +228,7 @@ static const struct flash_info macronix_nor_parts[] = {
>         }, {
>                 /* MX25L3255E */
>                 .id = SNOR_ID(0xc2, 0x9e, 0x16),
> +               .fixups = &mx25l3255e_fixups,
>         },
>         /*
>          * This spares us of adding new flash entries for flashes that can be
> --
> 2.25.1
>

Re: [PATCH v2 3/3] mtd: spi-nor: macronix: Add fixups for MX25L3255E

[Pratyush Yadav]

On Mon, Apr 07 2025, Cheng Ming Lin wrote:

> From: Cheng Ming Lin <chengminglin@mxic.com.tw>
>
> SFDP of MX25L3255E is JESD216, which does not include the Quad
> Enable bit Requirement in BFPT. As a result, during BFPT parsing,
> the quad_enable method is not set to spi_nor_sr1_bit6_quad_enable.
> Therefore, it is necessary to correct this setting by late_init.
>
> In addition, MX25L3255E also supports 1-4-4 page program in 3-byte
> address mode. However, since the 3-byte address 1-4-4 page program
> is not defined in SFDP, it needs to be configured in late_init.
>
> Signed-off-by: Cheng Ming Lin <chengminglin@mxic.com.tw>
> ---
>  drivers/mtd/spi-nor/macronix.c | 30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
>
> diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
> index 07e0bd0b70a0..4bbd3b651cec 100644
> --- a/drivers/mtd/spi-nor/macronix.c
> +++ b/drivers/mtd/spi-nor/macronix.c
> @@ -58,6 +58,31 @@ macronix_qpp4b_post_sfdp_fixups(struct spi_nor *nor)
>  	return 0;
>  }
>  
> +static int
> +mx25l3255e_late_init_fixups(struct spi_nor *nor)
> +{
> +	/*
> +	 * SFDP of MX25L3255E is JESD216, which does not include the Quad
> +	 * Enable bit Requirement in BFPT. As a result, during BFPT parsing,
> +	 * the quad_enable method is not set to spi_nor_sr1_bit6_quad_enable.
> +	 * Therefore, it is necessary to correct this setting by late_init.
> +	 */
> +	nor->params->quad_enable = spi_nor_sr1_bit6_quad_enable;
> +
> +	/*
> +	 * In addition, MX25L3255E also supports 1-4-4 page program in 3-byte
> +	 * address mode. However, since the 3-byte address 1-4-4 page program
> +	 * is not defined in SFDP, it needs to be configured in late_init.
> +	 */
> +	struct spi_nor_flash_parameter *params = nor->params;

Please don't have variable declarations in the middle of the function.
It looks odd. Though neither checkpatch nor cc seem to complain about
this. I thought they did...

Anyway, no need for a new revision, this can be fixed when applying by
me or Tudor.

Acked-by: Pratyush Yadav <pratyush@kernel.org>

> +
> +	params->hwcaps.mask |= SNOR_HWCAPS_PP_1_4_4;
> +	spi_nor_set_pp_settings(&params->page_programs[SNOR_CMD_PP_1_4_4],
> +				SPINOR_OP_PP_1_4_4, SNOR_PROTO_1_4_4);
> +
> +	return 0;
> +}
> +
[...]

-- 
Regards,
Pratyush Yadav

Re: [PATCH v2 0/3] mtd: spi-nor: macronix: Cleanup macronix flash info

[Pratyush Yadav]

On Mon, Apr 07 2025, Cheng Ming Lin wrote:

> From: Cheng Ming Lin <chengminglin@mxic.com.tw>
>
> Organize Macronix flash information to make it more readable.

Series applied to spi-nor/next. Thanks!

-- 
Regards,
Pratyush Yadav

Re: [PATCH v2 1/3] mtd: spi-nor: macronix: Drop the redundant flash info fields

[Guenter Roeck]

Hi,

On Mon, Apr 07, 2025 at 03:53:58PM +0800, Cheng Ming Lin wrote:
> From: Cheng Ming Lin <chengminglin@mxic.com.tw>
> 
> Many flash devices share the same ID but have different part numbers.
> To avoid confusion, the part number field is removed.
> 
> Additionally, since SFDP already provides size information and
> functionality covered by no_sfdp_flags, these fields are also removed.
> 
> Furthermore, when 4-byte address instruction table is available,
> the SPI_NOR_4B_OPCODES flag is no longer needed and is removed.
> 
> Signed-off-by: Cheng Ming Lin <chengminglin@mxic.com.tw>
> Acked-by: Tudor Ambarus <tudor.ambarus@linaro.org>

With this patch in place, some of my qemu tests no longer recognize the
flash chips (MX25L25635E/F). Do you have a suggestion on how to handle
this other than avoiding Macronix flash chips when working with qemu ?

Thanks,
Guenter

Re: [PATCH v2 1/3] mtd: spi-nor: macronix: Drop the redundant flash info fields

[Pratyush Yadav]

On Sat, Jun 07 2025, Guenter Roeck wrote:

> Hi,
>
> On Mon, Apr 07, 2025 at 03:53:58PM +0800, Cheng Ming Lin wrote:
>> From: Cheng Ming Lin <chengminglin@mxic.com.tw>
>> 
>> Many flash devices share the same ID but have different part numbers.
>> To avoid confusion, the part number field is removed.
>> 
>> Additionally, since SFDP already provides size information and
>> functionality covered by no_sfdp_flags, these fields are also removed.
>> 
>> Furthermore, when 4-byte address instruction table is available,
>> the SPI_NOR_4B_OPCODES flag is no longer needed and is removed.
>> 
>> Signed-off-by: Cheng Ming Lin <chengminglin@mxic.com.tw>
>> Acked-by: Tudor Ambarus <tudor.ambarus@linaro.org>
>
> With this patch in place, some of my qemu tests no longer recognize the
> flash chips (MX25L25635E/F). Do you have a suggestion on how to handle
> this other than avoiding Macronix flash chips when working with qemu ?

Could you share some logs? Does the flash fail to detect, or does the
SFDP-based probing fail? Since this is qemu, it would be even better if
you can share a setup/reproduction guide. I have been meaning to set up
qemu for SPI NOR testing for some time now, but never got around to
figuring it out.

-- 
Regards,
Pratyush Yadav

Re: [PATCH v2 1/3] mtd: spi-nor: macronix: Drop the redundant flash info fields

[Guenter Roeck]

On 6/8/25 05:53, Pratyush Yadav wrote:
> On Sat, Jun 07 2025, Guenter Roeck wrote:
> 
>> Hi,
>>
>> On Mon, Apr 07, 2025 at 03:53:58PM +0800, Cheng Ming Lin wrote:
>>> From: Cheng Ming Lin <chengminglin@mxic.com.tw>
>>>
>>> Many flash devices share the same ID but have different part numbers.
>>> To avoid confusion, the part number field is removed.
>>>
>>> Additionally, since SFDP already provides size information and
>>> functionality covered by no_sfdp_flags, these fields are also removed.
>>>
>>> Furthermore, when 4-byte address instruction table is available,
>>> the SPI_NOR_4B_OPCODES flag is no longer needed and is removed.
>>>
>>> Signed-off-by: Cheng Ming Lin <chengminglin@mxic.com.tw>
>>> Acked-by: Tudor Ambarus <tudor.ambarus@linaro.org>
>>
>> With this patch in place, some of my qemu tests no longer recognize the
>> flash chips (MX25L25635E/F). Do you have a suggestion on how to handle
>> this other than avoiding Macronix flash chips when working with qemu ?
> 
> Could you share some logs? Does the flash fail to detect, or does the
> SFDP-based probing fail? Since this is qemu, it would be even better if
> you can share a setup/reproduction guide. I have been meaning to set up
> qemu for SPI NOR testing for some time now, but never got around to
> figuring it out.
> 

I suspect that the SFDP data for the affected flashes is incorrect in qemu.
Since this is very likely a qemu problem, I'll just configure different flash
chips when running affected tests.

For reference, here is a sample qemu command line.

qemu-system-arm -M supermicro-x11spi-bmc,fmc-model=n25q256a13,spi-model=n25q256a13 \
	-kernel arch/arm/boot/zImage -no-reboot -snapshot -audio none \
	-drive file=/tmp/flash,format=raw,if=mtd -nic user \
	--append "root=/dev/mtdblock0 rootwait console=ttyS4,115200 earlycon=uart8250,mmio32,0x1e784000,115200n8" \
	-dtb arch/arm/boot/dts/aspeed/aspeed-bmc-supermicro-x11spi.dtb \
	-nographic -monitor null -serial stdio

This is with aspeed_g5_defconfig. In this example, ",fmc-model=n25q256a13,spi-model=n25q256a13"
is used to override the default flash model (MX25L25635E) because it longer works after
this patch. Normally that should not be required, but it is useful to test support
for different flash chips.

Guenter

Re: [PATCH v2 1/3] mtd: spi-nor: macronix: Drop the redundant flash info fields

[Guenter Roeck]

On 6/8/25 18:13, Guenter Roeck wrote:
> On 6/8/25 05:53, Pratyush Yadav wrote:
>> On Sat, Jun 07 2025, Guenter Roeck wrote:
>>
>>> Hi,
>>>
>>> On Mon, Apr 07, 2025 at 03:53:58PM +0800, Cheng Ming Lin wrote:
>>>> From: Cheng Ming Lin <chengminglin@mxic.com.tw>
>>>>
>>>> Many flash devices share the same ID but have different part numbers.
>>>> To avoid confusion, the part number field is removed.
>>>>
>>>> Additionally, since SFDP already provides size information and
>>>> functionality covered by no_sfdp_flags, these fields are also removed.
>>>>
>>>> Furthermore, when 4-byte address instruction table is available,
>>>> the SPI_NOR_4B_OPCODES flag is no longer needed and is removed.
>>>>
>>>> Signed-off-by: Cheng Ming Lin <chengminglin@mxic.com.tw>
>>>> Acked-by: Tudor Ambarus <tudor.ambarus@linaro.org>
>>>
>>> With this patch in place, some of my qemu tests no longer recognize the
>>> flash chips (MX25L25635E/F). Do you have a suggestion on how to handle
>>> this other than avoiding Macronix flash chips when working with qemu ?
>>
>> Could you share some logs? Does the flash fail to detect, or does the
>> SFDP-based probing fail? Since this is qemu, it would be even better if
>> you can share a setup/reproduction guide. I have been meaning to set up
>> qemu for SPI NOR testing for some time now, but never got around to
>> figuring it out.
>>
> 
> I suspect that the SFDP data for the affected flashes is incorrect in qemu.
> Since this is very likely a qemu problem, I'll just configure different flash
> chips when running affected tests.
> 

I was able to confirm the above. This is from the kernel log:

[    4.500000] spi-nor spi0.0: BFPT parsing failed. Please consider using SPI_NOR_SKIP_SFDP when declaring the flash
[    4.500000] spi-nor spi0.0: probe with driver spi-nor failed with error -22

Guenter

Re: [PATCH v2 1/3] mtd: spi-nor: macronix: Drop the redundant flash info fields

[Tudor Ambarus]

On 6/10/25 1:14 AM, Guenter Roeck wrote:
> On 6/8/25 18:13, Guenter Roeck wrote:
>> On 6/8/25 05:53, Pratyush Yadav wrote:
>>> On Sat, Jun 07 2025, Guenter Roeck wrote:
>>>
>>>> Hi,
>>>>
>>>> On Mon, Apr 07, 2025 at 03:53:58PM +0800, Cheng Ming Lin wrote:
>>>>> From: Cheng Ming Lin <chengminglin@mxic.com.tw>
>>>>>
>>>>> Many flash devices share the same ID but have different part numbers.
>>>>> To avoid confusion, the part number field is removed.
>>>>>
>>>>> Additionally, since SFDP already provides size information and
>>>>> functionality covered by no_sfdp_flags, these fields are also removed.
>>>>>
>>>>> Furthermore, when 4-byte address instruction table is available,
>>>>> the SPI_NOR_4B_OPCODES flag is no longer needed and is removed.
>>>>>
>>>>> Signed-off-by: Cheng Ming Lin <chengminglin@mxic.com.tw>
>>>>> Acked-by: Tudor Ambarus <tudor.ambarus@linaro.org>
>>>>
>>>> With this patch in place, some of my qemu tests no longer recognize the
>>>> flash chips (MX25L25635E/F). Do you have a suggestion on how to handle
>>>> this other than avoiding Macronix flash chips when working with qemu ?
>>>
>>> Could you share some logs? Does the flash fail to detect, or does the
>>> SFDP-based probing fail? Since this is qemu, it would be even better if
>>> you can share a setup/reproduction guide. I have been meaning to set up
>>> qemu for SPI NOR testing for some time now, but never got around to
>>> figuring it out.
>>>
>>
>> I suspect that the SFDP data for the affected flashes is incorrect in
>> qemu.
>> Since this is very likely a qemu problem, I'll just configure
>> different flash
>> chips when running affected tests.
>>
> 
> I was able to confirm the above. This is from the kernel log:
> 
> [    4.500000] spi-nor spi0.0: BFPT parsing failed. Please consider
> using SPI_NOR_SKIP_SFDP when declaring the flash
> [    4.500000] spi-nor spi0.0: probe with driver spi-nor failed with
> error -22
> 

It's likely the problem where the same flash ID is used for different
flavors of flashes. Typically we differentiate the flavors by comparing
their SFDP data and use post SFDP hooks to amend where needed. If no one
cares about fixing it, we can undo the change for the affected flash or
revert the patch entirely. Cheng?

Re: [PATCH v2 1/3] mtd: spi-nor: macronix: Drop the redundant flash info fields

[Cheng Ming Lin]

Tudor Ambarus <tudor.ambarus@linaro.org> 於 2025年6月10日 週二 下午2:46寫道：
>
>
>
> On 6/10/25 1:14 AM, Guenter Roeck wrote:
> > On 6/8/25 18:13, Guenter Roeck wrote:
> >> On 6/8/25 05:53, Pratyush Yadav wrote:
> >>> On Sat, Jun 07 2025, Guenter Roeck wrote:
> >>>
> >>>> Hi,
> >>>>
> >>>> On Mon, Apr 07, 2025 at 03:53:58PM +0800, Cheng Ming Lin wrote:
> >>>>> From: Cheng Ming Lin <chengminglin@mxic.com.tw>
> >>>>>
> >>>>> Many flash devices share the same ID but have different part numbers.
> >>>>> To avoid confusion, the part number field is removed.
> >>>>>
> >>>>> Additionally, since SFDP already provides size information and
> >>>>> functionality covered by no_sfdp_flags, these fields are also removed.
> >>>>>
> >>>>> Furthermore, when 4-byte address instruction table is available,
> >>>>> the SPI_NOR_4B_OPCODES flag is no longer needed and is removed.
> >>>>>
> >>>>> Signed-off-by: Cheng Ming Lin <chengminglin@mxic.com.tw>
> >>>>> Acked-by: Tudor Ambarus <tudor.ambarus@linaro.org>
> >>>>
> >>>> With this patch in place, some of my qemu tests no longer recognize the
> >>>> flash chips (MX25L25635E/F). Do you have a suggestion on how to handle
> >>>> this other than avoiding Macronix flash chips when working with qemu ?
> >>>
> >>> Could you share some logs? Does the flash fail to detect, or does the
> >>> SFDP-based probing fail? Since this is qemu, it would be even better if
> >>> you can share a setup/reproduction guide. I have been meaning to set up
> >>> qemu for SPI NOR testing for some time now, but never got around to
> >>> figuring it out.
> >>>
> >>
> >> I suspect that the SFDP data for the affected flashes is incorrect in
> >> qemu.
> >> Since this is very likely a qemu problem, I'll just configure
> >> different flash
> >> chips when running affected tests.
> >>
> >
> > I was able to confirm the above. This is from the kernel log:
> >
> > [    4.500000] spi-nor spi0.0: BFPT parsing failed. Please consider
> > using SPI_NOR_SKIP_SFDP when declaring the flash
> > [    4.500000] spi-nor spi0.0: probe with driver spi-nor failed with
> > error -22
> >
>
> It's likely the problem where the same flash ID is used for different
> flavors of flashes. Typically we differentiate the flavors by comparing
> their SFDP data and use post SFDP hooks to amend where needed. If no one
> cares about fixing it, we can undo the change for the affected flash or
> revert the patch entirely. Cheng?
>

Since this patch removed the size field, the issue occurs because the
"spi_nor_init_params" function now processes the "spi_nor_parse_sfdp"
function. As a result, the error is likely caused by a problem during the
SFDP parsing, which leads to the failure of "spi_nor_init_params".

The issue can be resolved by adding .size = SZ_32M to the flash_info for
MX25L25635E to prevent failures caused by SFDP parsing.

The root cause of this problem lies in the failure of parsing the SFDP
data for the flash, rather than an issue with the patch itself. I believe
we should not revert this patch.

Thanks,
Cheng Ming Lin

Re: [PATCH v2 1/3] mtd: spi-nor: macronix: Drop the redundant flash info fields

[Guenter Roeck]

On 6/9/25 23:46, Tudor Ambarus wrote:
> 
> 
> On 6/10/25 1:14 AM, Guenter Roeck wrote:
>> On 6/8/25 18:13, Guenter Roeck wrote:
>>> On 6/8/25 05:53, Pratyush Yadav wrote:
>>>> On Sat, Jun 07 2025, Guenter Roeck wrote:
>>>>
>>>>> Hi,
>>>>>
>>>>> On Mon, Apr 07, 2025 at 03:53:58PM +0800, Cheng Ming Lin wrote:
>>>>>> From: Cheng Ming Lin <chengminglin@mxic.com.tw>
>>>>>>
>>>>>> Many flash devices share the same ID but have different part numbers.
>>>>>> To avoid confusion, the part number field is removed.
>>>>>>
>>>>>> Additionally, since SFDP already provides size information and
>>>>>> functionality covered by no_sfdp_flags, these fields are also removed.
>>>>>>
>>>>>> Furthermore, when 4-byte address instruction table is available,
>>>>>> the SPI_NOR_4B_OPCODES flag is no longer needed and is removed.
>>>>>>
>>>>>> Signed-off-by: Cheng Ming Lin <chengminglin@mxic.com.tw>
>>>>>> Acked-by: Tudor Ambarus <tudor.ambarus@linaro.org>
>>>>>
>>>>> With this patch in place, some of my qemu tests no longer recognize the
>>>>> flash chips (MX25L25635E/F). Do you have a suggestion on how to handle
>>>>> this other than avoiding Macronix flash chips when working with qemu ?
>>>>
>>>> Could you share some logs? Does the flash fail to detect, or does the
>>>> SFDP-based probing fail? Since this is qemu, it would be even better if
>>>> you can share a setup/reproduction guide. I have been meaning to set up
>>>> qemu for SPI NOR testing for some time now, but never got around to
>>>> figuring it out.
>>>>
>>>
>>> I suspect that the SFDP data for the affected flashes is incorrect in
>>> qemu.
>>> Since this is very likely a qemu problem, I'll just configure
>>> different flash
>>> chips when running affected tests.
>>>
>>
>> I was able to confirm the above. This is from the kernel log:
>>
>> [    4.500000] spi-nor spi0.0: BFPT parsing failed. Please consider
>> using SPI_NOR_SKIP_SFDP when declaring the flash
>> [    4.500000] spi-nor spi0.0: probe with driver spi-nor failed with
>> error -22
>>
> 
> It's likely the problem where the same flash ID is used for different
> flavors of flashes. Typically we differentiate the flavors by comparing
> their SFDP data and use post SFDP hooks to amend where needed. If no one
> cares about fixing it, we can undo the change for the affected flash or
> revert the patch entirely. Cheng?
> 

Some debugging shows two problems with qemu: The returned SFPD data for one
of the flashes is wrong and does not reflect the data qemu is supposed
to provide, and qemu does not provide SFPD data for all flashes.

I don't know if the bad data is due to a bad Linux driver (unlikely), a bug
in qemu's SPI emulation code, or a bug in qemu's flash emulation code.
Unfortunately I don't really have time to track this down further.

Guenter

Re: [PATCH v2 1/3] mtd: spi-nor: macronix: Drop the redundant flash info fields

[Guenter Roeck]

On 6/10/25 00:29, Cheng Ming Lin wrote:
> Tudor Ambarus <tudor.ambarus@linaro.org> 於 2025年6月10日 週二 下午2:46寫道：
>>
>>
>>
>> On 6/10/25 1:14 AM, Guenter Roeck wrote:
>>> On 6/8/25 18:13, Guenter Roeck wrote:
>>>> On 6/8/25 05:53, Pratyush Yadav wrote:
>>>>> On Sat, Jun 07 2025, Guenter Roeck wrote:
>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> On Mon, Apr 07, 2025 at 03:53:58PM +0800, Cheng Ming Lin wrote:
>>>>>>> From: Cheng Ming Lin <chengminglin@mxic.com.tw>
>>>>>>>
>>>>>>> Many flash devices share the same ID but have different part numbers.
>>>>>>> To avoid confusion, the part number field is removed.
>>>>>>>
>>>>>>> Additionally, since SFDP already provides size information and
>>>>>>> functionality covered by no_sfdp_flags, these fields are also removed.
>>>>>>>
>>>>>>> Furthermore, when 4-byte address instruction table is available,
>>>>>>> the SPI_NOR_4B_OPCODES flag is no longer needed and is removed.
>>>>>>>
>>>>>>> Signed-off-by: Cheng Ming Lin <chengminglin@mxic.com.tw>
>>>>>>> Acked-by: Tudor Ambarus <tudor.ambarus@linaro.org>
>>>>>>
>>>>>> With this patch in place, some of my qemu tests no longer recognize the
>>>>>> flash chips (MX25L25635E/F). Do you have a suggestion on how to handle
>>>>>> this other than avoiding Macronix flash chips when working with qemu ?
>>>>>
>>>>> Could you share some logs? Does the flash fail to detect, or does the
>>>>> SFDP-based probing fail? Since this is qemu, it would be even better if
>>>>> you can share a setup/reproduction guide. I have been meaning to set up
>>>>> qemu for SPI NOR testing for some time now, but never got around to
>>>>> figuring it out.
>>>>>
>>>>
>>>> I suspect that the SFDP data for the affected flashes is incorrect in
>>>> qemu.
>>>> Since this is very likely a qemu problem, I'll just configure
>>>> different flash
>>>> chips when running affected tests.
>>>>
>>>
>>> I was able to confirm the above. This is from the kernel log:
>>>
>>> [    4.500000] spi-nor spi0.0: BFPT parsing failed. Please consider
>>> using SPI_NOR_SKIP_SFDP when declaring the flash
>>> [    4.500000] spi-nor spi0.0: probe with driver spi-nor failed with
>>> error -22
>>>
>>
>> It's likely the problem where the same flash ID is used for different
>> flavors of flashes. Typically we differentiate the flavors by comparing
>> their SFDP data and use post SFDP hooks to amend where needed. If no one
>> cares about fixing it, we can undo the change for the affected flash or
>> revert the patch entirely. Cheng?
>>
> 
> Since this patch removed the size field, the issue occurs because the
> "spi_nor_init_params" function now processes the "spi_nor_parse_sfdp"
> function. As a result, the error is likely caused by a problem during the
> SFDP parsing, which leads to the failure of "spi_nor_init_params".
> 
> The issue can be resolved by adding .size = SZ_32M to the flash_info for
> MX25L25635E to prevent failures caused by SFDP parsing.
> 
> The root cause of this problem lies in the failure of parsing the SFDP
> data for the flash, rather than an issue with the patch itself. I believe
> we should not revert this patch.
> 

I tend to agree. The problem is very likely on the qemu side and should be
addressed there.

Guenter

Re: [PATCH v2 1/3] mtd: spi-nor: macronix: Drop the redundant flash info fields

[Pratyush Yadav]

Hi Guenter,

On Tue, Jun 10 2025, Guenter Roeck wrote:

> On 6/9/25 23:46, Tudor Ambarus wrote:
>> On 6/10/25 1:14 AM, Guenter Roeck wrote:
>>> On 6/8/25 18:13, Guenter Roeck wrote:
>>>> On 6/8/25 05:53, Pratyush Yadav wrote:
>>>>> On Sat, Jun 07 2025, Guenter Roeck wrote:
[...]
>>>>>> With this patch in place, some of my qemu tests no longer recognize the
>>>>>> flash chips (MX25L25635E/F). Do you have a suggestion on how to handle
>>>>>> this other than avoiding Macronix flash chips when working with qemu ?
>>>>>
>>>>> Could you share some logs? Does the flash fail to detect, or does the
>>>>> SFDP-based probing fail? Since this is qemu, it would be even better if
>>>>> you can share a setup/reproduction guide. I have been meaning to set up
>>>>> qemu for SPI NOR testing for some time now, but never got around to
>>>>> figuring it out.
>>>>>
>>>>
>>>> I suspect that the SFDP data for the affected flashes is incorrect in
>>>> qemu.
>>>> Since this is very likely a qemu problem, I'll just configure
>>>> different flash
>>>> chips when running affected tests.
>>>>
[...]
>
> Some debugging shows two problems with qemu: The returned SFPD data for one
> of the flashes is wrong and does not reflect the data qemu is supposed
> to provide, and qemu does not provide SFPD data for all flashes.
>
> I don't know if the bad data is due to a bad Linux driver (unlikely), a bug
> in qemu's SPI emulation code, or a bug in qemu's flash emulation code.
> Unfortunately I don't really have time to track this down further.

I used the command you posted in [0] and tried to reproduce the bug, but
for me the flash probes just fine:

    root@romulus:/sys/bus/spi/devices/spi0.0/spi-nor# cat manufacturer 
    macronix
    root@romulus:/sys/bus/spi/devices/spi0.0/spi-nor# cat jedec_id 
    c22019
    root@romulus:/sys/bus/spi/devices/spi0.0/spi-nor# uname -a
    Linux romulus 6.16.0-rc2-00308-gf7301f856d35-dirty #5 SMP Sat Jun 21 19:29:15 CEST 2025 armv6l GNU/Linux

The rootfs is also mounted on a mtdblock device backed by this flash,
and everything appears to work fine.

Which version of qemu are you using? I can see that SFDP data for
mx25l25635e was added to qemu in commit dc907a667c ("m25p80: Add the
mx25l25635e SFPD table"), which was released in v7.2.0. Older versions
of qemu might run into trouble with this flash if SFDP data is not
available.

FWIW, my qemu version:

    $ qemu-system-arm --version
    QEMU emulator version 10.0.0
    Copyright (c) 2003-2025 Fabrice Bellard and the QEMU Project developers

[0] https://lore.kernel.org/linux-mtd/da58fc81-3c99-4951-85bc-e3c139283b5a@roeck-us.net/

-- 
Regards,
Pratyush Yadav