[PATCH v2 0/3] Add support for Quad Input Page Program to is25wp256

[PATCH v2 0/3] Add support for Quad Input Page Program to is25wp256

[Sudip Mukherjee]

This patch series enables SFDP support for is25wp256. And also adds
Quad Input Page Program capability as its not discoverable from SFDP.

Changes in v2:
Enable SFDP instead of using no-sfdp-flags.

Sudip Mukherjee (3):
  mtd: spi-nor: issi: is25wp256: Init flash based on SFDP
  mtd: spi-nor: add SFDP fixups for Quad Page Program
  mtd: spi-nor: issi: is25wp256: Enable Quad Input Page Program

 drivers/mtd/spi-nor/core.c | 9 +++++++++
 drivers/mtd/spi-nor/core.h | 2 ++
 drivers/mtd/spi-nor/issi.c | 4 ++--
 3 files changed, 13 insertions(+), 2 deletions(-)

-- 
2.30.2

[PATCH v2 0/3] Add support for Quad Input Page Program to is25wp256

[Sudip Mukherjee]

This patch series enables SFDP support for is25wp256. And also adds
Quad Input Page Program capability as its not discoverable from SFDP.

Changes in v2:
Enable SFDP instead of using no-sfdp-flags.

Sudip Mukherjee (3):
  mtd: spi-nor: issi: is25wp256: Init flash based on SFDP
  mtd: spi-nor: add SFDP fixups for Quad Page Program
  mtd: spi-nor: issi: is25wp256: Enable Quad Input Page Program

 drivers/mtd/spi-nor/core.c | 9 +++++++++
 drivers/mtd/spi-nor/core.h | 2 ++
 drivers/mtd/spi-nor/issi.c | 4 ++--
 3 files changed, 13 insertions(+), 2 deletions(-)

-- 
2.30.2


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

[PATCH v2 1/3] mtd: spi-nor: issi: is25wp256: Init flash based on SFDP

[Sudip Mukherjee]

The datasheet of is25wp256 says it supports SFDP. Get rid of the static
initialization of the flash parameters and init them when parsing SFDP.

Testing showed the flash using SPINOR_OP_READ_1_1_4_4B 0x6c,
SPINOR_OP_PP_4B 0x12 and SPINOR_OP_BE_4K_4B 0x21 before enabling SFDP.
After this patch, it parses the SFDP information and still uses the
same opcodes.

Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com>
---
 drivers/mtd/spi-nor/issi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/spi-nor/issi.c b/drivers/mtd/spi-nor/issi.c
index 89a66a19d754..8b48459b5054 100644
--- a/drivers/mtd/spi-nor/issi.c
+++ b/drivers/mtd/spi-nor/issi.c
@@ -71,7 +71,7 @@ static const struct flash_info issi_nor_parts[] = {
 	{ "is25wp128",  INFO(0x9d7018, 0, 64 * 1024, 256)
 		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
 	{ "is25wp256", INFO(0x9d7019, 0, 64 * 1024, 512)
-		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
+		PARSE_SFDP
 		FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
 		.fixups = &is25lp256_fixups },
 
-- 
2.30.2

[PATCH v2 1/3] mtd: spi-nor: issi: is25wp256: Init flash based on SFDP

[Sudip Mukherjee]

The datasheet of is25wp256 says it supports SFDP. Get rid of the static
initialization of the flash parameters and init them when parsing SFDP.

Testing showed the flash using SPINOR_OP_READ_1_1_4_4B 0x6c,
SPINOR_OP_PP_4B 0x12 and SPINOR_OP_BE_4K_4B 0x21 before enabling SFDP.
After this patch, it parses the SFDP information and still uses the
same opcodes.

Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com>
---
 drivers/mtd/spi-nor/issi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/spi-nor/issi.c b/drivers/mtd/spi-nor/issi.c
index 89a66a19d754..8b48459b5054 100644
--- a/drivers/mtd/spi-nor/issi.c
+++ b/drivers/mtd/spi-nor/issi.c
@@ -71,7 +71,7 @@ static const struct flash_info issi_nor_parts[] = {
 	{ "is25wp128",  INFO(0x9d7018, 0, 64 * 1024, 256)
 		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
 	{ "is25wp256", INFO(0x9d7019, 0, 64 * 1024, 512)
-		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
+		PARSE_SFDP
 		FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
 		.fixups = &is25lp256_fixups },
 
-- 
2.30.2


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

[PATCH v2 2/3] mtd: spi-nor: add SFDP fixups for Quad Page Program

[Sudip Mukherjee]

SFDP table of some flash chips do not advertise support of Quad Input
Page Program even though it has support. Use fixup flags and add hardware
cap for these chips.

Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com>
---
 drivers/mtd/spi-nor/core.c | 9 +++++++++
 drivers/mtd/spi-nor/core.h | 2 ++
 2 files changed, 11 insertions(+)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index f2c64006f8d7..7542404332a5 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -1962,6 +1962,12 @@ spi_nor_spimem_adjust_hwcaps(struct spi_nor *nor, u32 *hwcaps)
 	if (nor->flags & SNOR_F_BROKEN_RESET)
 		*hwcaps &= ~(SNOR_HWCAPS_X_X_X | SNOR_HWCAPS_X_X_X_DTR);
 
+	if (nor->flags & SNOR_F_HAS_QUAD_PP) {
+		*hwcaps |= SNOR_HWCAPS_PP_1_1_4;
+		spi_nor_set_pp_settings(&params->page_programs[SNOR_CMD_PP_1_1_4],
+					SPINOR_OP_PP_1_1_4, SNOR_PROTO_1_1_4);
+	}
+
 	for (cap = 0; cap < sizeof(*hwcaps) * BITS_PER_BYTE; cap++) {
 		int rdidx, ppidx;
 
@@ -2446,6 +2452,9 @@ static void spi_nor_init_fixup_flags(struct spi_nor *nor)
 
 	if (fixup_flags & SPI_NOR_IO_MODE_EN_VOLATILE)
 		nor->flags |= SNOR_F_IO_MODE_EN_VOLATILE;
+
+	if (fixup_flags & SPI_NOR_QUAD_PP)
+		nor->flags |= SNOR_F_HAS_QUAD_PP;
 }
 
 /**
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index 85b0cf254e97..7dbdf16a67b4 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -130,6 +130,7 @@ enum spi_nor_option_flags {
 	SNOR_F_IO_MODE_EN_VOLATILE = BIT(11),
 	SNOR_F_SOFT_RESET	= BIT(12),
 	SNOR_F_SWP_IS_VOLATILE	= BIT(13),
+	SNOR_F_HAS_QUAD_PP	= BIT(14),
 };
 
 struct spi_nor_read_command {
@@ -520,6 +521,7 @@ struct flash_info {
 	u8 fixup_flags;
 #define SPI_NOR_4B_OPCODES		BIT(0)
 #define SPI_NOR_IO_MODE_EN_VOLATILE	BIT(1)
+#define SPI_NOR_QUAD_PP			BIT(2)
 
 	u8 mfr_flags;
 
-- 
2.30.2

[PATCH v2 2/3] mtd: spi-nor: add SFDP fixups for Quad Page Program

[Sudip Mukherjee]

SFDP table of some flash chips do not advertise support of Quad Input
Page Program even though it has support. Use fixup flags and add hardware
cap for these chips.

Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com>
---
 drivers/mtd/spi-nor/core.c | 9 +++++++++
 drivers/mtd/spi-nor/core.h | 2 ++
 2 files changed, 11 insertions(+)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index f2c64006f8d7..7542404332a5 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -1962,6 +1962,12 @@ spi_nor_spimem_adjust_hwcaps(struct spi_nor *nor, u32 *hwcaps)
 	if (nor->flags & SNOR_F_BROKEN_RESET)
 		*hwcaps &= ~(SNOR_HWCAPS_X_X_X | SNOR_HWCAPS_X_X_X_DTR);
 
+	if (nor->flags & SNOR_F_HAS_QUAD_PP) {
+		*hwcaps |= SNOR_HWCAPS_PP_1_1_4;
+		spi_nor_set_pp_settings(&params->page_programs[SNOR_CMD_PP_1_1_4],
+					SPINOR_OP_PP_1_1_4, SNOR_PROTO_1_1_4);
+	}
+
 	for (cap = 0; cap < sizeof(*hwcaps) * BITS_PER_BYTE; cap++) {
 		int rdidx, ppidx;
 
@@ -2446,6 +2452,9 @@ static void spi_nor_init_fixup_flags(struct spi_nor *nor)
 
 	if (fixup_flags & SPI_NOR_IO_MODE_EN_VOLATILE)
 		nor->flags |= SNOR_F_IO_MODE_EN_VOLATILE;
+
+	if (fixup_flags & SPI_NOR_QUAD_PP)
+		nor->flags |= SNOR_F_HAS_QUAD_PP;
 }
 
 /**
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index 85b0cf254e97..7dbdf16a67b4 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -130,6 +130,7 @@ enum spi_nor_option_flags {
 	SNOR_F_IO_MODE_EN_VOLATILE = BIT(11),
 	SNOR_F_SOFT_RESET	= BIT(12),
 	SNOR_F_SWP_IS_VOLATILE	= BIT(13),
+	SNOR_F_HAS_QUAD_PP	= BIT(14),
 };
 
 struct spi_nor_read_command {
@@ -520,6 +521,7 @@ struct flash_info {
 	u8 fixup_flags;
 #define SPI_NOR_4B_OPCODES		BIT(0)
 #define SPI_NOR_IO_MODE_EN_VOLATILE	BIT(1)
+#define SPI_NOR_QUAD_PP			BIT(2)
 
 	u8 mfr_flags;
 
-- 
2.30.2


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

[PATCH v2 3/3] mtd: spi-nor: issi: is25wp256: Enable Quad Input Page Program

[Sudip Mukherjee]

The flash chip is25wp256 supports Quad Input Page Program but it is not
discoverable from the SFDP table. Enable the fixup flag for is25wp256.

Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com>
---
 drivers/mtd/spi-nor/issi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/spi-nor/issi.c b/drivers/mtd/spi-nor/issi.c
index 8b48459b5054..36e6c4a1d022 100644
--- a/drivers/mtd/spi-nor/issi.c
+++ b/drivers/mtd/spi-nor/issi.c
@@ -72,7 +72,7 @@ static const struct flash_info issi_nor_parts[] = {
 		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
 	{ "is25wp256", INFO(0x9d7019, 0, 64 * 1024, 512)
 		PARSE_SFDP
-		FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
+		FIXUP_FLAGS(SPI_NOR_4B_OPCODES | SPI_NOR_QUAD_PP)
 		.fixups = &is25lp256_fixups },
 
 	/* PMC */
-- 
2.30.2

[PATCH v2 3/3] mtd: spi-nor: issi: is25wp256: Enable Quad Input Page Program

[Sudip Mukherjee]

The flash chip is25wp256 supports Quad Input Page Program but it is not
discoverable from the SFDP table. Enable the fixup flag for is25wp256.

Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com>
---
 drivers/mtd/spi-nor/issi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/spi-nor/issi.c b/drivers/mtd/spi-nor/issi.c
index 8b48459b5054..36e6c4a1d022 100644
--- a/drivers/mtd/spi-nor/issi.c
+++ b/drivers/mtd/spi-nor/issi.c
@@ -72,7 +72,7 @@ static const struct flash_info issi_nor_parts[] = {
 		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
 	{ "is25wp256", INFO(0x9d7019, 0, 64 * 1024, 512)
 		PARSE_SFDP
-		FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
+		FIXUP_FLAGS(SPI_NOR_4B_OPCODES | SPI_NOR_QUAD_PP)
 		.fixups = &is25lp256_fixups },
 
 	/* PMC */
-- 
2.30.2


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

Re: [PATCH v2 1/3] mtd: spi-nor: issi: is25wp256: Init flash based on SFDP

[Tudor.Ambarus]

On 8/9/22 23:14, Sudip Mukherjee wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> The datasheet of is25wp256 says it supports SFDP. Get rid of the static
> initialization of the flash parameters and init them when parsing SFDP.
> 
> Testing showed the flash using SPINOR_OP_READ_1_1_4_4B 0x6c,
> SPINOR_OP_PP_4B 0x12 and SPINOR_OP_BE_4K_4B 0x21 before enabling SFDP.
> After this patch, it parses the SFDP information and still uses the
> same opcodes.
> 
> Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com>
> ---
>  drivers/mtd/spi-nor/issi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/spi-nor/issi.c b/drivers/mtd/spi-nor/issi.c
> index 89a66a19d754..8b48459b5054 100644
> --- a/drivers/mtd/spi-nor/issi.c
> +++ b/drivers/mtd/spi-nor/issi.c
> @@ -71,7 +71,7 @@ static const struct flash_info issi_nor_parts[] = {
>         { "is25wp128",  INFO(0x9d7018, 0, 64 * 1024, 256)
>                 NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
>         { "is25wp256", INFO(0x9d7019, 0, 64 * 1024, 512)
> -               NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
> +               PARSE_SFDP
>                 FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
>                 .fixups = &is25lp256_fixups },
> 
> --
> 2.30.2
> 

Looks good to me.
When submitting flash updates or new flash additions, we require contributors
to do a little test using mtd-utils and to dump the SPI NOR sysfs entries.
Would you please do that?

Here's the simple test:

Run the test_qspi.sh script: 
#!/bin/sh 

dd if=/dev/urandom of=./qspi_test bs=1M count=6 
mtd_debug write /dev/mtd5 0 6291456 qspi_test 
mtd_debug erase /dev/mtd5 0 6291456 
mtd_debug read /dev/mtd5 0 6291456 qspi_read 
hexdump qspi_read 
mtd_debug write /dev/mtd5 0 6291456 qspi_test 
mtd_debug read /dev/mtd5 0 6291456 qspi_read 
sha1sum qspi_test qspi_read 

The two SHA-1 sums must be the same to pass this test. 

Here's an example on how to dumps the sysfs entries:
zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/partname
s25hl02gt
zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/jedec_id
342a1c0f0090
zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/manufacturer
spansion
zynq> xxd -p /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
53464450080104ff00080114000100ff84000102500100ff81000118e001
00ff8700011c580100ff88000106c80100ffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffe720faffffffff7f48eb086b00ff
88bbfeffffffffff00ffffff48eb0c2000ff00ff12d823faff8b82e7ffec
ec031c608a857a75f766805c8cd6ddfff938c0a1000000000000bc000000
0000f7f5ffff7b920ffe21ffffdc0000800000000000c0ffc3fbc8ffe3fb
00650090066500b10065009600650095716503d0716503d000000000b02e
000088a489aa716503967165039600000000000000000000000000000000
000000000000000000000000000000000000000000000000716505d57165
05d50000a015000080080000000800008010000000100000801800000018
fc65ff0804008000fc65ff0402008000fc65ff0804008008fd65ff040200
8008fe0202fff1ff0100f8ff0100f8fffb0ffe0902fff8fffb0ff8ff0100
f1ff0100fe0104fff1ff0100f8ff0100f8fff70ff8ff0100f1ff0100ff0a
00fff8ffff0f
zynq> md5sum /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
86aef254bcfdf763bdb92e4c31667242  /sys/bus/spi/devices/spi0.0/spi-nor/sfdp

Thanks!


-- 
Cheers,
ta

Re: [PATCH v2 1/3] mtd: spi-nor: issi: is25wp256: Init flash based on SFDP

[Tudor.Ambarus]

On 8/9/22 23:14, Sudip Mukherjee wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> The datasheet of is25wp256 says it supports SFDP. Get rid of the static
> initialization of the flash parameters and init them when parsing SFDP.
> 
> Testing showed the flash using SPINOR_OP_READ_1_1_4_4B 0x6c,
> SPINOR_OP_PP_4B 0x12 and SPINOR_OP_BE_4K_4B 0x21 before enabling SFDP.
> After this patch, it parses the SFDP information and still uses the
> same opcodes.
> 
> Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com>
> ---
>  drivers/mtd/spi-nor/issi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/spi-nor/issi.c b/drivers/mtd/spi-nor/issi.c
> index 89a66a19d754..8b48459b5054 100644
> --- a/drivers/mtd/spi-nor/issi.c
> +++ b/drivers/mtd/spi-nor/issi.c
> @@ -71,7 +71,7 @@ static const struct flash_info issi_nor_parts[] = {
>         { "is25wp128",  INFO(0x9d7018, 0, 64 * 1024, 256)
>                 NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
>         { "is25wp256", INFO(0x9d7019, 0, 64 * 1024, 512)
> -               NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
> +               PARSE_SFDP
>                 FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
>                 .fixups = &is25lp256_fixups },
> 
> --
> 2.30.2
> 

Looks good to me.
When submitting flash updates or new flash additions, we require contributors
to do a little test using mtd-utils and to dump the SPI NOR sysfs entries.
Would you please do that?

Here's the simple test:

Run the test_qspi.sh script: 
#!/bin/sh 

dd if=/dev/urandom of=./qspi_test bs=1M count=6 
mtd_debug write /dev/mtd5 0 6291456 qspi_test 
mtd_debug erase /dev/mtd5 0 6291456 
mtd_debug read /dev/mtd5 0 6291456 qspi_read 
hexdump qspi_read 
mtd_debug write /dev/mtd5 0 6291456 qspi_test 
mtd_debug read /dev/mtd5 0 6291456 qspi_read 
sha1sum qspi_test qspi_read 

The two SHA-1 sums must be the same to pass this test. 

Here's an example on how to dumps the sysfs entries:
zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/partname
s25hl02gt
zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/jedec_id
342a1c0f0090
zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/manufacturer
spansion
zynq> xxd -p /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
53464450080104ff00080114000100ff84000102500100ff81000118e001
00ff8700011c580100ff88000106c80100ffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
ffffffffffffffffffffffffffffffffe720faffffffff7f48eb086b00ff
88bbfeffffffffff00ffffff48eb0c2000ff00ff12d823faff8b82e7ffec
ec031c608a857a75f766805c8cd6ddfff938c0a1000000000000bc000000
0000f7f5ffff7b920ffe21ffffdc0000800000000000c0ffc3fbc8ffe3fb
00650090066500b10065009600650095716503d0716503d000000000b02e
000088a489aa716503967165039600000000000000000000000000000000
000000000000000000000000000000000000000000000000716505d57165
05d50000a015000080080000000800008010000000100000801800000018
fc65ff0804008000fc65ff0402008000fc65ff0804008008fd65ff040200
8008fe0202fff1ff0100f8ff0100f8fffb0ffe0902fff8fffb0ff8ff0100
f1ff0100fe0104fff1ff0100f8ff0100f8fff70ff8ff0100f1ff0100ff0a
00fff8ffff0f
zynq> md5sum /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
86aef254bcfdf763bdb92e4c31667242  /sys/bus/spi/devices/spi0.0/spi-nor/sfdp

Thanks!


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

Re: [PATCH v2 2/3] mtd: spi-nor: add SFDP fixups for Quad Page Program

[Tudor.Ambarus]

On 8/9/22 23:14, Sudip Mukherjee wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> SFDP table of some flash chips do not advertise support of Quad Input
> Page Program even though it has support. Use fixup flags and add hardware
> cap for these chips.
> 
> Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com>
> ---
>  drivers/mtd/spi-nor/core.c | 9 +++++++++
>  drivers/mtd/spi-nor/core.h | 2 ++
>  2 files changed, 11 insertions(+)
> 
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index f2c64006f8d7..7542404332a5 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -1962,6 +1962,12 @@ spi_nor_spimem_adjust_hwcaps(struct spi_nor *nor, u32 *hwcaps)
>         if (nor->flags & SNOR_F_BROKEN_RESET)
>                 *hwcaps &= ~(SNOR_HWCAPS_X_X_X | SNOR_HWCAPS_X_X_X_DTR);
> 
> +       if (nor->flags & SNOR_F_HAS_QUAD_PP) {
> +               *hwcaps |= SNOR_HWCAPS_PP_1_1_4;
> +               spi_nor_set_pp_settings(&params->page_programs[SNOR_CMD_PP_1_1_4],
> +                                       SPINOR_OP_PP_1_1_4, SNOR_PROTO_1_1_4);
> +       }

setting SPINOR_OP_PP_1_1_4 should be done in spi_nor_late_init_params().
spi_nor_late_init_params() is used to adjust the ops supported by the flash
with the ones supported by the controller.

> +
>         for (cap = 0; cap < sizeof(*hwcaps) * BITS_PER_BYTE; cap++) {
>                 int rdidx, ppidx;
> 
> @@ -2446,6 +2452,9 @@ static void spi_nor_init_fixup_flags(struct spi_nor *nor)
> 
>         if (fixup_flags & SPI_NOR_IO_MODE_EN_VOLATILE)
>                 nor->flags |= SNOR_F_IO_MODE_EN_VOLATILE;
> +
> +       if (fixup_flags & SPI_NOR_QUAD_PP)
> +               nor->flags |= SNOR_F_HAS_QUAD_PP;
>  }
> 
>  /**
> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
> index 85b0cf254e97..7dbdf16a67b4 100644
> --- a/drivers/mtd/spi-nor/core.h
> +++ b/drivers/mtd/spi-nor/core.h
> @@ -130,6 +130,7 @@ enum spi_nor_option_flags {
>         SNOR_F_IO_MODE_EN_VOLATILE = BIT(11),
>         SNOR_F_SOFT_RESET       = BIT(12),
>         SNOR_F_SWP_IS_VOLATILE  = BIT(13),
> +       SNOR_F_HAS_QUAD_PP      = BIT(14),

you won't need this
>  };
> 
>  struct spi_nor_read_command {
> @@ -520,6 +521,7 @@ struct flash_info {
>         u8 fixup_flags;
>  #define SPI_NOR_4B_OPCODES             BIT(0)
>  #define SPI_NOR_IO_MODE_EN_VOLATILE    BIT(1)
> +#define SPI_NOR_QUAD_PP                        BIT(2)

No, as I previously said, SPI_NOR_QUAD_PP should be declared as a
info->flags, not as info->fixup_flags.

> 
>         u8 mfr_flags;
> 
> --
> 2.30.2
> 


-- 
Cheers,
ta

Re: [PATCH v2 2/3] mtd: spi-nor: add SFDP fixups for Quad Page Program

[Tudor.Ambarus]

On 8/9/22 23:14, Sudip Mukherjee wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> SFDP table of some flash chips do not advertise support of Quad Input
> Page Program even though it has support. Use fixup flags and add hardware
> cap for these chips.
> 
> Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com>
> ---
>  drivers/mtd/spi-nor/core.c | 9 +++++++++
>  drivers/mtd/spi-nor/core.h | 2 ++
>  2 files changed, 11 insertions(+)
> 
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index f2c64006f8d7..7542404332a5 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -1962,6 +1962,12 @@ spi_nor_spimem_adjust_hwcaps(struct spi_nor *nor, u32 *hwcaps)
>         if (nor->flags & SNOR_F_BROKEN_RESET)
>                 *hwcaps &= ~(SNOR_HWCAPS_X_X_X | SNOR_HWCAPS_X_X_X_DTR);
> 
> +       if (nor->flags & SNOR_F_HAS_QUAD_PP) {
> +               *hwcaps |= SNOR_HWCAPS_PP_1_1_4;
> +               spi_nor_set_pp_settings(&params->page_programs[SNOR_CMD_PP_1_1_4],
> +                                       SPINOR_OP_PP_1_1_4, SNOR_PROTO_1_1_4);
> +       }

setting SPINOR_OP_PP_1_1_4 should be done in spi_nor_late_init_params().
spi_nor_late_init_params() is used to adjust the ops supported by the flash
with the ones supported by the controller.

> +
>         for (cap = 0; cap < sizeof(*hwcaps) * BITS_PER_BYTE; cap++) {
>                 int rdidx, ppidx;
> 
> @@ -2446,6 +2452,9 @@ static void spi_nor_init_fixup_flags(struct spi_nor *nor)
> 
>         if (fixup_flags & SPI_NOR_IO_MODE_EN_VOLATILE)
>                 nor->flags |= SNOR_F_IO_MODE_EN_VOLATILE;
> +
> +       if (fixup_flags & SPI_NOR_QUAD_PP)
> +               nor->flags |= SNOR_F_HAS_QUAD_PP;
>  }
> 
>  /**
> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
> index 85b0cf254e97..7dbdf16a67b4 100644
> --- a/drivers/mtd/spi-nor/core.h
> +++ b/drivers/mtd/spi-nor/core.h
> @@ -130,6 +130,7 @@ enum spi_nor_option_flags {
>         SNOR_F_IO_MODE_EN_VOLATILE = BIT(11),
>         SNOR_F_SOFT_RESET       = BIT(12),
>         SNOR_F_SWP_IS_VOLATILE  = BIT(13),
> +       SNOR_F_HAS_QUAD_PP      = BIT(14),

you won't need this
>  };
> 
>  struct spi_nor_read_command {
> @@ -520,6 +521,7 @@ struct flash_info {
>         u8 fixup_flags;
>  #define SPI_NOR_4B_OPCODES             BIT(0)
>  #define SPI_NOR_IO_MODE_EN_VOLATILE    BIT(1)
> +#define SPI_NOR_QUAD_PP                        BIT(2)

No, as I previously said, SPI_NOR_QUAD_PP should be declared as a
info->flags, not as info->fixup_flags.

> 
>         u8 mfr_flags;
> 
> --
> 2.30.2
> 


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

Re: [PATCH v2 2/3] mtd: spi-nor: add SFDP fixups for Quad Page Program

[Tudor.Ambarus]

On 8/10/22 11:06, Tudor.Ambarus@microchip.com wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On 8/9/22 23:14, Sudip Mukherjee wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> SFDP table of some flash chips do not advertise support of Quad Input
>> Page Program even though it has support. Use fixup flags and add hardware
>> cap for these chips.
>>
>> Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com>
>> ---
>>  drivers/mtd/spi-nor/core.c | 9 +++++++++
>>  drivers/mtd/spi-nor/core.h | 2 ++
>>  2 files changed, 11 insertions(+)
>>
>> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
>> index f2c64006f8d7..7542404332a5 100644
>> --- a/drivers/mtd/spi-nor/core.c
>> +++ b/drivers/mtd/spi-nor/core.c
>> @@ -1962,6 +1962,12 @@ spi_nor_spimem_adjust_hwcaps(struct spi_nor *nor, u32 *hwcaps)
>>         if (nor->flags & SNOR_F_BROKEN_RESET)
>>                 *hwcaps &= ~(SNOR_HWCAPS_X_X_X | SNOR_HWCAPS_X_X_X_DTR);
>>
>> +       if (nor->flags & SNOR_F_HAS_QUAD_PP) {
>> +               *hwcaps |= SNOR_HWCAPS_PP_1_1_4;
>> +               spi_nor_set_pp_settings(&params->page_programs[SNOR_CMD_PP_1_1_4],
>> +                                       SPINOR_OP_PP_1_1_4, SNOR_PROTO_1_1_4);
>> +       }
> 
> setting SPINOR_OP_PP_1_1_4 should be done in spi_nor_late_init_params().
> spi_nor_late_init_params() is used to adjust the ops supported by the flash

^ s/spi_nor_late_init_params/spi_nor_spimem_adjust_hwcaps

> with the ones supported by the controller.
> 
>> +
>>         for (cap = 0; cap < sizeof(*hwcaps) * BITS_PER_BYTE; cap++) {
>>                 int rdidx, ppidx;
>>
>> @@ -2446,6 +2452,9 @@ static void spi_nor_init_fixup_flags(struct spi_nor *nor)
>>
>>         if (fixup_flags & SPI_NOR_IO_MODE_EN_VOLATILE)
>>                 nor->flags |= SNOR_F_IO_MODE_EN_VOLATILE;
>> +
>> +       if (fixup_flags & SPI_NOR_QUAD_PP)
>> +               nor->flags |= SNOR_F_HAS_QUAD_PP;
>>  }
>>
>>  /**
>> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
>> index 85b0cf254e97..7dbdf16a67b4 100644
>> --- a/drivers/mtd/spi-nor/core.h
>> +++ b/drivers/mtd/spi-nor/core.h
>> @@ -130,6 +130,7 @@ enum spi_nor_option_flags {
>>         SNOR_F_IO_MODE_EN_VOLATILE = BIT(11),
>>         SNOR_F_SOFT_RESET       = BIT(12),
>>         SNOR_F_SWP_IS_VOLATILE  = BIT(13),
>> +       SNOR_F_HAS_QUAD_PP      = BIT(14),
> 
> you won't need this
>>  };
>>
>>  struct spi_nor_read_command {
>> @@ -520,6 +521,7 @@ struct flash_info {
>>         u8 fixup_flags;
>>  #define SPI_NOR_4B_OPCODES             BIT(0)
>>  #define SPI_NOR_IO_MODE_EN_VOLATILE    BIT(1)
>> +#define SPI_NOR_QUAD_PP                        BIT(2)
> 
> No, as I previously said, SPI_NOR_QUAD_PP should be declared as a
> info->flags, not as info->fixup_flags.
> 
>>
>>         u8 mfr_flags;
>>
>> --
>> 2.30.2
>>
> 
> 
> --
> Cheers,
> ta
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/


-- 
Cheers,
ta

Re: [PATCH v2 2/3] mtd: spi-nor: add SFDP fixups for Quad Page Program

[Tudor.Ambarus]

On 8/10/22 11:06, Tudor.Ambarus@microchip.com wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On 8/9/22 23:14, Sudip Mukherjee wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> SFDP table of some flash chips do not advertise support of Quad Input
>> Page Program even though it has support. Use fixup flags and add hardware
>> cap for these chips.
>>
>> Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com>
>> ---
>>  drivers/mtd/spi-nor/core.c | 9 +++++++++
>>  drivers/mtd/spi-nor/core.h | 2 ++
>>  2 files changed, 11 insertions(+)
>>
>> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
>> index f2c64006f8d7..7542404332a5 100644
>> --- a/drivers/mtd/spi-nor/core.c
>> +++ b/drivers/mtd/spi-nor/core.c
>> @@ -1962,6 +1962,12 @@ spi_nor_spimem_adjust_hwcaps(struct spi_nor *nor, u32 *hwcaps)
>>         if (nor->flags & SNOR_F_BROKEN_RESET)
>>                 *hwcaps &= ~(SNOR_HWCAPS_X_X_X | SNOR_HWCAPS_X_X_X_DTR);
>>
>> +       if (nor->flags & SNOR_F_HAS_QUAD_PP) {
>> +               *hwcaps |= SNOR_HWCAPS_PP_1_1_4;
>> +               spi_nor_set_pp_settings(&params->page_programs[SNOR_CMD_PP_1_1_4],
>> +                                       SPINOR_OP_PP_1_1_4, SNOR_PROTO_1_1_4);
>> +       }
> 
> setting SPINOR_OP_PP_1_1_4 should be done in spi_nor_late_init_params().
> spi_nor_late_init_params() is used to adjust the ops supported by the flash

^ s/spi_nor_late_init_params/spi_nor_spimem_adjust_hwcaps

> with the ones supported by the controller.
> 
>> +
>>         for (cap = 0; cap < sizeof(*hwcaps) * BITS_PER_BYTE; cap++) {
>>                 int rdidx, ppidx;
>>
>> @@ -2446,6 +2452,9 @@ static void spi_nor_init_fixup_flags(struct spi_nor *nor)
>>
>>         if (fixup_flags & SPI_NOR_IO_MODE_EN_VOLATILE)
>>                 nor->flags |= SNOR_F_IO_MODE_EN_VOLATILE;
>> +
>> +       if (fixup_flags & SPI_NOR_QUAD_PP)
>> +               nor->flags |= SNOR_F_HAS_QUAD_PP;
>>  }
>>
>>  /**
>> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
>> index 85b0cf254e97..7dbdf16a67b4 100644
>> --- a/drivers/mtd/spi-nor/core.h
>> +++ b/drivers/mtd/spi-nor/core.h
>> @@ -130,6 +130,7 @@ enum spi_nor_option_flags {
>>         SNOR_F_IO_MODE_EN_VOLATILE = BIT(11),
>>         SNOR_F_SOFT_RESET       = BIT(12),
>>         SNOR_F_SWP_IS_VOLATILE  = BIT(13),
>> +       SNOR_F_HAS_QUAD_PP      = BIT(14),
> 
> you won't need this
>>  };
>>
>>  struct spi_nor_read_command {
>> @@ -520,6 +521,7 @@ struct flash_info {
>>         u8 fixup_flags;
>>  #define SPI_NOR_4B_OPCODES             BIT(0)
>>  #define SPI_NOR_IO_MODE_EN_VOLATILE    BIT(1)
>> +#define SPI_NOR_QUAD_PP                        BIT(2)
> 
> No, as I previously said, SPI_NOR_QUAD_PP should be declared as a
> info->flags, not as info->fixup_flags.
> 
>>
>>         u8 mfr_flags;
>>
>> --
>> 2.30.2
>>
> 
> 
> --
> Cheers,
> ta
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/


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

Re: [PATCH v2 2/3] mtd: spi-nor: add SFDP fixups for Quad Page Program

[Sudip Mukherjee]

On Wed, Aug 10, 2022 at 9:06 AM <Tudor.Ambarus@microchip.com> wrote:
>
> On 8/9/22 23:14, Sudip Mukherjee wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >
> > SFDP table of some flash chips do not advertise support of Quad Input
> > Page Program even though it has support. Use fixup flags and add hardware
> > cap for these chips.
> >

<snip>

> > @@ -520,6 +521,7 @@ struct flash_info {
> >         u8 fixup_flags;
> >  #define SPI_NOR_4B_OPCODES             BIT(0)
> >  #define SPI_NOR_IO_MODE_EN_VOLATILE    BIT(1)
> > +#define SPI_NOR_QUAD_PP                        BIT(2)
>
> No, as I previously said, SPI_NOR_QUAD_PP should be declared as a
> info->flags, not as info->fixup_flags.

Sorry, I was confused as info->fixup_flags says "it indicates support
that can be discovered via SFDP ideally, but can not be discovered
for this particular flash because the SFDP table that indicates this
support is not defined by the flash."

--
Regards
Sudip

Re: [PATCH v2 2/3] mtd: spi-nor: add SFDP fixups for Quad Page Program

[Sudip Mukherjee]

On Wed, Aug 10, 2022 at 9:06 AM <Tudor.Ambarus@microchip.com> wrote:
>
> On 8/9/22 23:14, Sudip Mukherjee wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >
> > SFDP table of some flash chips do not advertise support of Quad Input
> > Page Program even though it has support. Use fixup flags and add hardware
> > cap for these chips.
> >

<snip>

> > @@ -520,6 +521,7 @@ struct flash_info {
> >         u8 fixup_flags;
> >  #define SPI_NOR_4B_OPCODES             BIT(0)
> >  #define SPI_NOR_IO_MODE_EN_VOLATILE    BIT(1)
> > +#define SPI_NOR_QUAD_PP                        BIT(2)
>
> No, as I previously said, SPI_NOR_QUAD_PP should be declared as a
> info->flags, not as info->fixup_flags.

Sorry, I was confused as info->fixup_flags says "it indicates support
that can be discovered via SFDP ideally, but can not be discovered
for this particular flash because the SFDP table that indicates this
support is not defined by the flash."

--
Regards
Sudip

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

Re: [PATCH v2 2/3] mtd: spi-nor: add SFDP fixups for Quad Page Program

[Sudip Mukherjee]

On Wed, Aug 10, 2022 at 9:25 AM <Tudor.Ambarus@microchip.com> wrote:
>
> On 8/10/22 11:06, Tudor.Ambarus@microchip.com wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >
> > On 8/9/22 23:14, Sudip Mukherjee wrote:
> >> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >>
> >> SFDP table of some flash chips do not advertise support of Quad Input
> >> Page Program even though it has support. Use fixup flags and add hardware
> >> cap for these chips.
> >>
> >> Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com>
> >> ---
> >>  drivers/mtd/spi-nor/core.c | 9 +++++++++
> >>  drivers/mtd/spi-nor/core.h | 2 ++
> >>  2 files changed, 11 insertions(+)
> >>
> >> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> >> index f2c64006f8d7..7542404332a5 100644
> >> --- a/drivers/mtd/spi-nor/core.c
> >> +++ b/drivers/mtd/spi-nor/core.c
> >> @@ -1962,6 +1962,12 @@ spi_nor_spimem_adjust_hwcaps(struct spi_nor *nor, u32 *hwcaps)
> >>         if (nor->flags & SNOR_F_BROKEN_RESET)
> >>                 *hwcaps &= ~(SNOR_HWCAPS_X_X_X | SNOR_HWCAPS_X_X_X_DTR);
> >>
> >> +       if (nor->flags & SNOR_F_HAS_QUAD_PP) {
> >> +               *hwcaps |= SNOR_HWCAPS_PP_1_1_4;
> >> +               spi_nor_set_pp_settings(&params->page_programs[SNOR_CMD_PP_1_1_4],
> >> +                                       SPINOR_OP_PP_1_1_4, SNOR_PROTO_1_1_4);
> >> +       }
> >
> > setting SPINOR_OP_PP_1_1_4 should be done in spi_nor_late_init_params().
> > spi_nor_late_init_params() is used to adjust the ops supported by the flash
>
> ^ s/spi_nor_late_init_params/spi_nor_spimem_adjust_hwcaps

So, do you mean something like this:

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index f2c64006f8d7..2f41937b826d 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -1962,6 +1962,12 @@ spi_nor_spimem_adjust_hwcaps(struct spi_nor
*nor, u32 *hwcaps)
  if (nor->flags & SNOR_F_BROKEN_RESET)
  *hwcaps &= ~(SNOR_HWCAPS_X_X_X | SNOR_HWCAPS_X_X_X_DTR);

+ if (nor->info->flags & SPI_NOR_QUAD_PP) {
+ *hwcaps |= SNOR_HWCAPS_PP_1_1_4;
+ spi_nor_set_pp_settings(&params->page_programs[SNOR_CMD_PP_1_1_4],
+ SPINOR_OP_PP_1_1_4, SNOR_PROTO_1_1_4);
+ }
+
  for (cap = 0; cap < sizeof(*hwcaps) * BITS_PER_BYTE; cap++) {
  int rdidx, ppidx;

diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index 85b0cf254e97..10aa1c72000f 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -507,6 +507,7 @@ struct flash_info {
 #define SPI_NOR_NO_ERASE BIT(6)
 #define NO_CHIP_ERASE BIT(7)
 #define SPI_NOR_NO_FR BIT(8)
+#define SPI_NOR_QUAD_PP BIT(9)

  u8 no_sfdp_flags;
 #define SPI_NOR_SKIP_SFDP BIT(0)
diff --git a/drivers/mtd/spi-nor/issi.c b/drivers/mtd/spi-nor/issi.c
index 89a66a19d754..014cd9038bed 100644
--- a/drivers/mtd/spi-nor/issi.c
+++ b/drivers/mtd/spi-nor/issi.c
@@ -71,8 +71,9 @@ static const struct flash_info issi_nor_parts[] = {
  { "is25wp128",  INFO(0x9d7018, 0, 64 * 1024, 256)
  NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
  { "is25wp256", INFO(0x9d7019, 0, 64 * 1024, 512)
- NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
+ PARSE_SFDP
  FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
+ FLAGS(SPI_NOR_QUAD_PP)
  .fixups = &is25lp256_fixups },

  /* PMC */


--
Regards
Sudip

Re: [PATCH v2 2/3] mtd: spi-nor: add SFDP fixups for Quad Page Program

[Sudip Mukherjee]

On Wed, Aug 10, 2022 at 9:25 AM <Tudor.Ambarus@microchip.com> wrote:
>
> On 8/10/22 11:06, Tudor.Ambarus@microchip.com wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >
> > On 8/9/22 23:14, Sudip Mukherjee wrote:
> >> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >>
> >> SFDP table of some flash chips do not advertise support of Quad Input
> >> Page Program even though it has support. Use fixup flags and add hardware
> >> cap for these chips.
> >>
> >> Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com>
> >> ---
> >>  drivers/mtd/spi-nor/core.c | 9 +++++++++
> >>  drivers/mtd/spi-nor/core.h | 2 ++
> >>  2 files changed, 11 insertions(+)
> >>
> >> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> >> index f2c64006f8d7..7542404332a5 100644
> >> --- a/drivers/mtd/spi-nor/core.c
> >> +++ b/drivers/mtd/spi-nor/core.c
> >> @@ -1962,6 +1962,12 @@ spi_nor_spimem_adjust_hwcaps(struct spi_nor *nor, u32 *hwcaps)
> >>         if (nor->flags & SNOR_F_BROKEN_RESET)
> >>                 *hwcaps &= ~(SNOR_HWCAPS_X_X_X | SNOR_HWCAPS_X_X_X_DTR);
> >>
> >> +       if (nor->flags & SNOR_F_HAS_QUAD_PP) {
> >> +               *hwcaps |= SNOR_HWCAPS_PP_1_1_4;
> >> +               spi_nor_set_pp_settings(&params->page_programs[SNOR_CMD_PP_1_1_4],
> >> +                                       SPINOR_OP_PP_1_1_4, SNOR_PROTO_1_1_4);
> >> +       }
> >
> > setting SPINOR_OP_PP_1_1_4 should be done in spi_nor_late_init_params().
> > spi_nor_late_init_params() is used to adjust the ops supported by the flash
>
> ^ s/spi_nor_late_init_params/spi_nor_spimem_adjust_hwcaps

So, do you mean something like this:

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index f2c64006f8d7..2f41937b826d 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -1962,6 +1962,12 @@ spi_nor_spimem_adjust_hwcaps(struct spi_nor
*nor, u32 *hwcaps)
  if (nor->flags & SNOR_F_BROKEN_RESET)
  *hwcaps &= ~(SNOR_HWCAPS_X_X_X | SNOR_HWCAPS_X_X_X_DTR);

+ if (nor->info->flags & SPI_NOR_QUAD_PP) {
+ *hwcaps |= SNOR_HWCAPS_PP_1_1_4;
+ spi_nor_set_pp_settings(&params->page_programs[SNOR_CMD_PP_1_1_4],
+ SPINOR_OP_PP_1_1_4, SNOR_PROTO_1_1_4);
+ }
+
  for (cap = 0; cap < sizeof(*hwcaps) * BITS_PER_BYTE; cap++) {
  int rdidx, ppidx;

diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index 85b0cf254e97..10aa1c72000f 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -507,6 +507,7 @@ struct flash_info {
 #define SPI_NOR_NO_ERASE BIT(6)
 #define NO_CHIP_ERASE BIT(7)
 #define SPI_NOR_NO_FR BIT(8)
+#define SPI_NOR_QUAD_PP BIT(9)

  u8 no_sfdp_flags;
 #define SPI_NOR_SKIP_SFDP BIT(0)
diff --git a/drivers/mtd/spi-nor/issi.c b/drivers/mtd/spi-nor/issi.c
index 89a66a19d754..014cd9038bed 100644
--- a/drivers/mtd/spi-nor/issi.c
+++ b/drivers/mtd/spi-nor/issi.c
@@ -71,8 +71,9 @@ static const struct flash_info issi_nor_parts[] = {
  { "is25wp128",  INFO(0x9d7018, 0, 64 * 1024, 256)
  NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
  { "is25wp256", INFO(0x9d7019, 0, 64 * 1024, 512)
- NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
+ PARSE_SFDP
  FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
+ FLAGS(SPI_NOR_QUAD_PP)
  .fixups = &is25lp256_fixups },

  /* PMC */


--
Regards
Sudip

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

Re: [PATCH v2 2/3] mtd: spi-nor: add SFDP fixups for Quad Page Program

[Tudor.Ambarus]

On 8/10/22 18:14, Sudip Mukherjee wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On Wed, Aug 10, 2022 at 9:06 AM <Tudor.Ambarus@microchip.com> wrote:
>>
>> On 8/9/22 23:14, Sudip Mukherjee wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>
>>> SFDP table of some flash chips do not advertise support of Quad Input
>>> Page Program even though it has support. Use fixup flags and add hardware
>>> cap for these chips.
>>>
> 
> <snip>
> 
>>> @@ -520,6 +521,7 @@ struct flash_info {
>>>         u8 fixup_flags;
>>>  #define SPI_NOR_4B_OPCODES             BIT(0)
>>>  #define SPI_NOR_IO_MODE_EN_VOLATILE    BIT(1)
>>> +#define SPI_NOR_QUAD_PP                        BIT(2)
>>
>> No, as I previously said, SPI_NOR_QUAD_PP should be declared as a
>> info->flags, not as info->fixup_flags.
> 
> Sorry, I was confused as info->fixup_flags says "it indicates support
> that can be discovered via SFDP ideally, but can not be discovered
> for this particular flash because the SFDP table that indicates this
> support is not defined by the flash."
> 
Right, I've just sent a patch addressing this, hopefully is clearer now.
Check it here:
https://lore.kernel.org/linux-mtd/20220810155924.1366072-1-tudor.ambarus@microchip.com/T/#u
> --
> Regards
> Sudip


-- 
Cheers,
ta

Re: [PATCH v2 2/3] mtd: spi-nor: add SFDP fixups for Quad Page Program

[Tudor.Ambarus]

On 8/10/22 18:14, Sudip Mukherjee wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On Wed, Aug 10, 2022 at 9:06 AM <Tudor.Ambarus@microchip.com> wrote:
>>
>> On 8/9/22 23:14, Sudip Mukherjee wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>
>>> SFDP table of some flash chips do not advertise support of Quad Input
>>> Page Program even though it has support. Use fixup flags and add hardware
>>> cap for these chips.
>>>
> 
> <snip>
> 
>>> @@ -520,6 +521,7 @@ struct flash_info {
>>>         u8 fixup_flags;
>>>  #define SPI_NOR_4B_OPCODES             BIT(0)
>>>  #define SPI_NOR_IO_MODE_EN_VOLATILE    BIT(1)
>>> +#define SPI_NOR_QUAD_PP                        BIT(2)
>>
>> No, as I previously said, SPI_NOR_QUAD_PP should be declared as a
>> info->flags, not as info->fixup_flags.
> 
> Sorry, I was confused as info->fixup_flags says "it indicates support
> that can be discovered via SFDP ideally, but can not be discovered
> for this particular flash because the SFDP table that indicates this
> support is not defined by the flash."
> 
Right, I've just sent a patch addressing this, hopefully is clearer now.
Check it here:
https://lore.kernel.org/linux-mtd/20220810155924.1366072-1-tudor.ambarus@microchip.com/T/#u
> --
> Regards
> Sudip


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

Re: [PATCH v2 1/3] mtd: spi-nor: issi: is25wp256: Init flash based on SFDP

[Sudip Mukherjee]

Hi Tudor,

On Wed, Aug 10, 2022 at 8:55 AM <Tudor.Ambarus@microchip.com> wrote:
>
> On 8/9/22 23:14, Sudip Mukherjee wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >
> > The datasheet of is25wp256 says it supports SFDP. Get rid of the static
> > initialization of the flash parameters and init them when parsing SFDP.
> >
> > Testing showed the flash using SPINOR_OP_READ_1_1_4_4B 0x6c,
> > SPINOR_OP_PP_4B 0x12 and SPINOR_OP_BE_4K_4B 0x21 before enabling SFDP.
> > After this patch, it parses the SFDP information and still uses the
> > same opcodes.
> >
> > Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com>
> > ---
> >  drivers/mtd/spi-nor/issi.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/mtd/spi-nor/issi.c b/drivers/mtd/spi-nor/issi.c
> > index 89a66a19d754..8b48459b5054 100644
> > --- a/drivers/mtd/spi-nor/issi.c
> > +++ b/drivers/mtd/spi-nor/issi.c
> > @@ -71,7 +71,7 @@ static const struct flash_info issi_nor_parts[] = {
> >         { "is25wp128",  INFO(0x9d7018, 0, 64 * 1024, 256)
> >                 NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> >         { "is25wp256", INFO(0x9d7019, 0, 64 * 1024, 512)
> > -               NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
> > +               PARSE_SFDP
> >                 FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
> >                 .fixups = &is25lp256_fixups },
> >
> > --
> > 2.30.2
> >
>
> Looks good to me.
> When submitting flash updates or new flash additions, we require contributors
> to do a little test using mtd-utils and to dump the SPI NOR sysfs entries.
> Would you please do that?
>
> Here's the simple test:

Here is the test result with only the PARSE_SFDP change. I can resend
the patch if you want.
I will work on the other patch enabling Quad PP now and send that
along with its own test result.

# dd if=/dev/urandom of=./qspi_test bs=1M count=6
6+0 records in
6+0 records out

# mtd_debug write /dev/mtd4 0 6291456 qspi_test
Copied 6291456 bytes from qspi_test to address 0x00000000 in flash

# mtd_debug erase /dev/mtd4 0 6291456
Erased 6291456 bytes from address 0x00000000 in flash

# mtd_debug read /dev/mtd4 0 6291456 qspi_read
Copied 6291456 bytes from address 0x00000000 in flash to qspi_read

# hexdump qspi_read
0000000 ffff ffff ffff ffff ffff ffff ffff ffff
*
0600000

# mtd_debug write /dev/mtd4 0 6291456 qspi_test
Copied 6291456 bytes from qspi_test to address 0x00000000 in flash

# mtd_debug read /dev/mtd4 0 6291456 qspi_read
Copied 6291456 bytes from address 0x00000000 in flash to qspi_read

# sha1sum qspi_test qspi_read
57f8d4fee65622104e24276e865f662844f12242  qspi_test
57f8d4fee65622104e24276e865f662844f12242  qspi_read

# cat /sys/bus/spi/devices/spi0.0/spi-nor/partname
is25wp256

# cat /sys/bus/spi/devices/spi0.0/spi-nor/jedec_id
9d7019

# cat /sys/bus/spi/devices/spi0.0/spi-nor/manufacturer
issi

# xxd -p /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
53464450060101ff00060110300000ff9d05010380000002ffffffffffff
ffffffffffffffffffffffffffffffffffffe520f9ffffffff0f44eb086b
083b80bbfeffffffffff00ffffff44eb0c200f5210d800ff234ac90082d8
11cecccd68467a757a75f7aed55c4a422cfff030faa9ffffffffffffffff
ffffffffffffffff501950169ff9c0648fefffff

# md5sum /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
ba14818b9ec42713f24d94d66bb90ba0  /sys/bus/spi/devices/spi0.0/spi-nor/sfdp


--
Regards
Sudip

Re: [PATCH v2 1/3] mtd: spi-nor: issi: is25wp256: Init flash based on SFDP

[Sudip Mukherjee]

Hi Tudor,

On Wed, Aug 10, 2022 at 8:55 AM <Tudor.Ambarus@microchip.com> wrote:
>
> On 8/9/22 23:14, Sudip Mukherjee wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >
> > The datasheet of is25wp256 says it supports SFDP. Get rid of the static
> > initialization of the flash parameters and init them when parsing SFDP.
> >
> > Testing showed the flash using SPINOR_OP_READ_1_1_4_4B 0x6c,
> > SPINOR_OP_PP_4B 0x12 and SPINOR_OP_BE_4K_4B 0x21 before enabling SFDP.
> > After this patch, it parses the SFDP information and still uses the
> > same opcodes.
> >
> > Signed-off-by: Sudip Mukherjee <sudip.mukherjee@sifive.com>
> > ---
> >  drivers/mtd/spi-nor/issi.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/mtd/spi-nor/issi.c b/drivers/mtd/spi-nor/issi.c
> > index 89a66a19d754..8b48459b5054 100644
> > --- a/drivers/mtd/spi-nor/issi.c
> > +++ b/drivers/mtd/spi-nor/issi.c
> > @@ -71,7 +71,7 @@ static const struct flash_info issi_nor_parts[] = {
> >         { "is25wp128",  INFO(0x9d7018, 0, 64 * 1024, 256)
> >                 NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> >         { "is25wp256", INFO(0x9d7019, 0, 64 * 1024, 512)
> > -               NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
> > +               PARSE_SFDP
> >                 FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
> >                 .fixups = &is25lp256_fixups },
> >
> > --
> > 2.30.2
> >
>
> Looks good to me.
> When submitting flash updates or new flash additions, we require contributors
> to do a little test using mtd-utils and to dump the SPI NOR sysfs entries.
> Would you please do that?
>
> Here's the simple test:

Here is the test result with only the PARSE_SFDP change. I can resend
the patch if you want.
I will work on the other patch enabling Quad PP now and send that
along with its own test result.

# dd if=/dev/urandom of=./qspi_test bs=1M count=6
6+0 records in
6+0 records out

# mtd_debug write /dev/mtd4 0 6291456 qspi_test
Copied 6291456 bytes from qspi_test to address 0x00000000 in flash

# mtd_debug erase /dev/mtd4 0 6291456
Erased 6291456 bytes from address 0x00000000 in flash

# mtd_debug read /dev/mtd4 0 6291456 qspi_read
Copied 6291456 bytes from address 0x00000000 in flash to qspi_read

# hexdump qspi_read
0000000 ffff ffff ffff ffff ffff ffff ffff ffff
*
0600000

# mtd_debug write /dev/mtd4 0 6291456 qspi_test
Copied 6291456 bytes from qspi_test to address 0x00000000 in flash

# mtd_debug read /dev/mtd4 0 6291456 qspi_read
Copied 6291456 bytes from address 0x00000000 in flash to qspi_read

# sha1sum qspi_test qspi_read
57f8d4fee65622104e24276e865f662844f12242  qspi_test
57f8d4fee65622104e24276e865f662844f12242  qspi_read

# cat /sys/bus/spi/devices/spi0.0/spi-nor/partname
is25wp256

# cat /sys/bus/spi/devices/spi0.0/spi-nor/jedec_id
9d7019

# cat /sys/bus/spi/devices/spi0.0/spi-nor/manufacturer
issi

# xxd -p /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
53464450060101ff00060110300000ff9d05010380000002ffffffffffff
ffffffffffffffffffffffffffffffffffffe520f9ffffffff0f44eb086b
083b80bbfeffffffffff00ffffff44eb0c200f5210d800ff234ac90082d8
11cecccd68467a757a75f7aed55c4a422cfff030faa9ffffffffffffffff
ffffffffffffffff501950169ff9c0648fefffff

# md5sum /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
ba14818b9ec42713f24d94d66bb90ba0  /sys/bus/spi/devices/spi0.0/spi-nor/sfdp


--
Regards
Sudip

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