linux-mtd.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/8] mtd: spi-nor: Add support for Infineon SEMPER s25hl02gt and s25hs02gt
@ 2022-08-06  6:34 tkuw584924
  2022-08-06  6:34 ` [PATCH 1/8] mtd: spi-nor: core: Introduce number of dice and volatile register offset params tkuw584924
                   ` (7 more replies)
  0 siblings, 8 replies; 19+ messages in thread
From: tkuw584924 @ 2022-08-06  6:34 UTC (permalink / raw)
  To: linux-mtd
  Cc: tudor.ambarus, pratyush, michael, miquel.raynal, richard,
	vigneshr, tkuw584924, Bacem.Daassi, Takahiro Kuwano

From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>

Infineon SEMPER s25hl02gt and s25hs02gt are multi-chip (dual-die) package
parts and the datasheet can be found at:
https://www.infineon.com/dgdl/Infineon-S25HS02GT_S25HS04GT_S25HL02GT_S25HL04GT_2-Gb_DDP_4-Gb_QDP_HS-T_1.8-V_HL-T_3_0-V_Semper_Flash_with_Quad_SPI-DataSheet-v01_00-EN.pdf?fileId=8ac78c8c7e7124d1017f01b9a5055b7b

The key characteristics of multi-chip devices are:
- Each die has dedicataed registers so we need to configure and check
  registers in all the dice
- Read ops can cross the die boundary
- 4-byte address mode by default
- Legacy chip-erase command is not supported

To support multi-chip devices, core and sfdp need to determine the number
of dice and volatile register offset for each die. The SCCR map and newly
added SCCR map for multi-chip params in SFDP helps to do it.

In manufacturer specific code (spansion.c), configuration and status check
are updated to support multi-chip.

The s25hl02gt and s25hs02gt support chip-erase-addressed command followed
by die address instead of legacy chip-erase command. This series does not
add support for chip-erase-addressed. Another patch set may be created to
introduced how we can utilise that command.

Tested with Xilinx Zynq-7000 platform. Existing devices that share the
same manufacturer code (S25Hx-T and S28HS512T) are also tested.

ID, SFDP, and test log:
--------------------------------------------------------------------------
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
zynq> test_qspi.sh
6+0 records in
6+0 records out
6291456 bytes (6.0MB) copied, 0.231488 seconds, 25.9MB/s
Copied 6291456 bytes from qspi_test to address 0x00000000 in flash
Erased 6291456 bytes from address 0x00000000 in flash
Copied 6291456 bytes from address 0x00000000 in flash to qspi_read
0000000 ffff ffff ffff ffff ffff ffff ffff ffff
*
0600000
Copied 6291456 bytes from qspi_test to address 0x00000000 in flash
Copied 6291456 bytes from address 0x00000000 in flash to qspi_read
9f433bd1566013c98b94f5ff441859314c1fc7d6  qspi_test
9f433bd1566013c98b94f5ff441859314c1fc7d6  qspi_read
--------------------------------------------------------------------------
zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/partname
s25hs02gt
zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/jedec_id
342b1c0f0090
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
zynq> test_qspi.sh
6+0 records in
6+0 records out
6291456 bytes (6.0MB) copied, 0.231691 seconds, 25.9MB/s
Copied 6291456 bytes from qspi_test to address 0x00000000 in flash
Erased 6291456 bytes from address 0x00000000 in flash
Copied 6291456 bytes from address 0x00000000 in flash to qspi_read
0000000 ffff ffff ffff ffff ffff ffff ffff ffff
*
0600000
Copied 6291456 bytes from qspi_test to address 0x00000000 in flash
Copied 6291456 bytes from address 0x00000000 in flash to qspi_read
58b52b293f93574507b6ffb06f0e79b57b0f1f28  qspi_test
58b52b293f93574507b6ffb06f0e79b57b0f1f28  qspi_read
--------------------------------------------------------------------------

Takahiro Kuwano (8):
  mtd: spi-nor: core: Introduce number of dice and volatile register
    offset params
  mtd: spi-nor: sfdp: Extract volatile register offset from SCCR map
  mtd: spi-nor: sfdp: Add support for SCCR map for multi-chip device
  mtd: spi-nor: spansion: Rework cypress_nor_set_page_size() for
    multi-chip device support
  mtd: spi-nor: spansion: Rework cypress_nor_quad_enable_volatile() for
    multi-chip device support
  mtd: spi-nor: spansion: Add a new ->ready() hook for multi-chip device
  mtd: spi-nor: spansion: Introduce DEF_4BAM mfr flag
  mtd: spi-nor: spansion: Add support for Infineon

 drivers/mtd/spi-nor/core.c     |   1 +
 drivers/mtd/spi-nor/core.h     |   4 +
 drivers/mtd/spi-nor/sfdp.c     |  76 +++++++++++
 drivers/mtd/spi-nor/spansion.c | 232 ++++++++++++++++++++++-----------
 4 files changed, 237 insertions(+), 76 deletions(-)

-- 
2.25.1


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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 1/8] mtd: spi-nor: core: Introduce number of dice and volatile register offset params
  2022-08-06  6:34 [PATCH 0/8] mtd: spi-nor: Add support for Infineon SEMPER s25hl02gt and s25hs02gt tkuw584924
@ 2022-08-06  6:34 ` tkuw584924
  2022-08-06  6:34 ` [PATCH 2/8] mtd: spi-nor: sfdp: Extract volatile register offset from SCCR map tkuw584924
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 19+ messages in thread
From: tkuw584924 @ 2022-08-06  6:34 UTC (permalink / raw)
  To: linux-mtd
  Cc: tudor.ambarus, pratyush, michael, miquel.raynal, richard,
	vigneshr, tkuw584924, Bacem.Daassi, Takahiro Kuwano

From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>

In use of mult-chip devices, we need to access registers in each die for
configuration and status check. The number of dice in the device and
volatile register offset for each die are essential to iterate register
access ops.

The number of dice must be initialized as 1 and may be updated through
SFDP parse.

The volatile register offset table is dinamically allocated based on
the number of dice then filled with offset values collected from SFDP.

Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
---
 drivers/mtd/spi-nor/core.c | 1 +
 drivers/mtd/spi-nor/core.h | 4 ++++
 2 files changed, 5 insertions(+)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index f2c64006f8d7..1fe7a33f3189 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -2541,6 +2541,7 @@ static void spi_nor_init_default_params(struct spi_nor *nor)
 	params->writesize = 1;
 	params->size = (u64)info->sector_size * info->n_sectors;
 	params->page_size = info->page_size;
+	params->num_of_dice = 1;
 
 	if (!(info->flags & SPI_NOR_NO_FR)) {
 		/* Default to Fast Read for DT and non-DT platform devices. */
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index 85b0cf254e97..9a4fbeaf90ae 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -349,6 +349,8 @@ struct spi_nor_otp {
  *			in octal DTR mode.
  * @rdsr_addr_nbytes:	dummy address bytes needed for Read Status Register
  *			command in octal DTR mode.
+ * @num_of_dice:	number of dice in the flash memory.
+ * @vreg_offset:	volatile register offset for each die.
  * @hwcaps:		describes the read and page program hardware
  *			capabilities.
  * @reads:		read capabilities ordered by priority: the higher index
@@ -381,6 +383,8 @@ struct spi_nor_flash_parameter {
 	u8				addr_mode_nbytes;
 	u8				rdsr_dummy;
 	u8				rdsr_addr_nbytes;
+	u8				num_of_dice;
+	u32				*vreg_offset;
 
 	struct spi_nor_hwcaps		hwcaps;
 	struct spi_nor_read_command	reads[SNOR_CMD_READ_MAX];
-- 
2.25.1


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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 2/8] mtd: spi-nor: sfdp: Extract volatile register offset from SCCR map
  2022-08-06  6:34 [PATCH 0/8] mtd: spi-nor: Add support for Infineon SEMPER s25hl02gt and s25hs02gt tkuw584924
  2022-08-06  6:34 ` [PATCH 1/8] mtd: spi-nor: core: Introduce number of dice and volatile register offset params tkuw584924
@ 2022-08-06  6:34 ` tkuw584924
  2022-08-06  6:34 ` [PATCH 3/8] mtd: spi-nor: sfdp: Add support for SCCR map for multi-chip device tkuw584924
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 19+ messages in thread
From: tkuw584924 @ 2022-08-06  6:34 UTC (permalink / raw)
  To: linux-mtd
  Cc: tudor.ambarus, pratyush, michael, miquel.raynal, richard,
	vigneshr, tkuw584924, Bacem.Daassi, Takahiro Kuwano

From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>

The volatile register offset resides in the 1st DWORD of SCCR map. Allocate
the table and copy the offset value.

The table may be allocated when the SCCR map for multi-chip is parsed.
Since we cannot assume SCCR parse is always in ahead of SCCR multi-chip,
we need to check if the table is already allocated or not.

Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
---
 drivers/mtd/spi-nor/sfdp.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
index 2257f1b4c2e2..3d612dc4c63c 100644
--- a/drivers/mtd/spi-nor/sfdp.c
+++ b/drivers/mtd/spi-nor/sfdp.c
@@ -1206,6 +1206,7 @@ static int spi_nor_parse_profile1(struct spi_nor *nor,
 static int spi_nor_parse_sccr(struct spi_nor *nor,
 			      const struct sfdp_parameter_header *sccr_header)
 {
+	struct spi_nor_flash_parameter *params = nor->params;
 	u32 *dwords, addr;
 	size_t len;
 	int ret;
@@ -1222,6 +1223,17 @@ static int spi_nor_parse_sccr(struct spi_nor *nor,
 
 	le32_to_cpu_array(dwords, sccr_header->length);
 
+	/* Address offset for volatile registers (die 0) */
+	if (!params->vreg_offset) {
+		params->vreg_offset = devm_kmalloc(nor->dev, sizeof(*dwords),
+						   GFP_KERNEL);
+		if (!params->vreg_offset) {
+			ret = -ENOMEM;
+			goto out;
+		}
+	}
+	params->vreg_offset[0] = dwords[0];
+
 	if (FIELD_GET(SCCR_DWORD22_OCTAL_DTR_EN_VOLATILE, dwords[22]))
 		nor->flags |= SNOR_F_IO_MODE_EN_VOLATILE;
 
-- 
2.25.1


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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 3/8] mtd: spi-nor: sfdp: Add support for SCCR map for multi-chip device
  2022-08-06  6:34 [PATCH 0/8] mtd: spi-nor: Add support for Infineon SEMPER s25hl02gt and s25hs02gt tkuw584924
  2022-08-06  6:34 ` [PATCH 1/8] mtd: spi-nor: core: Introduce number of dice and volatile register offset params tkuw584924
  2022-08-06  6:34 ` [PATCH 2/8] mtd: spi-nor: sfdp: Extract volatile register offset from SCCR map tkuw584924
@ 2022-08-06  6:34 ` tkuw584924
  2022-08-06  6:34 ` [PATCH 4/8] mtd: spi-nor: spansion: Rework cypress_nor_set_page_size() for multi-chip device support tkuw584924
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 19+ messages in thread
From: tkuw584924 @ 2022-08-06  6:34 UTC (permalink / raw)
  To: linux-mtd
  Cc: tudor.ambarus, pratyush, michael, miquel.raynal, richard,
	vigneshr, tkuw584924, Bacem.Daassi, Takahiro Kuwano

From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>

SCCR map for multi-chip devices contains the number of additional dice in
the device and register offset values for each additional dice.

spi_nor_parse_sccr_mc() is added to determine the number of dice and
volatile register offset for each die. The volatile register offset table
may already be allocated and contains offset value for die-0 via SCCR map
parse. So, we should use devm_krealloc() to expand the table with
preserving die-0 offset.

Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
---
 drivers/mtd/spi-nor/sfdp.c | 64 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 64 insertions(+)

diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
index 3d612dc4c63c..8ef3b11fae49 100644
--- a/drivers/mtd/spi-nor/sfdp.c
+++ b/drivers/mtd/spi-nor/sfdp.c
@@ -26,6 +26,11 @@
 					 * Status, Control and Configuration
 					 * Register Map.
 					 */
+#define SFDP_SCCR_MAP_MC_ID	0xff88	/*
+					 * Status, Control and Configuration
+					 * Register Map Offsets for Multi-Chip
+					 * SPI Memory Devices.
+					 */
 
 #define SFDP_SIGNATURE		0x50444653U
 
@@ -1242,6 +1247,61 @@ static int spi_nor_parse_sccr(struct spi_nor *nor,
 	return ret;
 }
 
+/**
+ * spi_nor_parse_sccr_mc() - Parse the Status, Control and Configuration
+ *                           Register Map Offsets for Multi-Chip SPI Memory
+ *                           Devices.
+ * @nor:		pointer to a 'struct spi_nor'
+ * @sccr_mc_header:	pointer to the 'struct sfdp_parameter_header' describing
+ *			the SCCR Map offsets table length and version.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int spi_nor_parse_sccr_mc(struct spi_nor *nor,
+				 const struct sfdp_parameter_header *sccr_mc_header)
+{
+	struct spi_nor_flash_parameter *params = nor->params;
+	u32 *dwords, addr;
+	size_t len;
+	int ret;
+	u8 i;
+
+	len = sccr_mc_header->length * sizeof(*dwords);
+	dwords = kmalloc(len, GFP_KERNEL);
+	if (!dwords)
+		return -ENOMEM;
+
+	addr = SFDP_PARAM_HEADER_PTP(sccr_mc_header);
+	ret = spi_nor_read_sfdp(nor, addr, len, dwords);
+	if (ret)
+		goto out;
+
+	le32_to_cpu_array(dwords, sccr_mc_header->length);
+
+	/*
+	 * Pair of DOWRDs (volatile and non-volatile register offsets) per
+	 * additional die. Hence, length = 2 * (number of additional dice).
+	 */
+	params->num_of_dice += sccr_mc_header->length / 2;
+
+	/* Address offset for volatile registers of additional dice */
+	params->vreg_offset =
+			devm_krealloc(nor->dev, params->vreg_offset,
+				      params->num_of_dice * sizeof(*dwords),
+				      GFP_KERNEL);
+	if (!params->vreg_offset) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	for (i = 1; i < params->num_of_dice; i++)
+		params->vreg_offset[i] = dwords[(i - 1) * 2];
+
+out:
+	kfree(dwords);
+	return ret;
+}
+
 /**
  * spi_nor_post_sfdp_fixups() - Updates the flash's parameters and settings
  * after SFDP has been parsed. Called only for flashes that define JESD216 SFDP
@@ -1424,6 +1484,10 @@ int spi_nor_parse_sfdp(struct spi_nor *nor)
 			err = spi_nor_parse_sccr(nor, param_header);
 			break;
 
+		case SFDP_SCCR_MAP_MC_ID:
+			err = spi_nor_parse_sccr_mc(nor, param_header);
+			break;
+
 		default:
 			break;
 		}
-- 
2.25.1


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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 4/8] mtd: spi-nor: spansion: Rework cypress_nor_set_page_size() for multi-chip device support
  2022-08-06  6:34 [PATCH 0/8] mtd: spi-nor: Add support for Infineon SEMPER s25hl02gt and s25hs02gt tkuw584924
                   ` (2 preceding siblings ...)
  2022-08-06  6:34 ` [PATCH 3/8] mtd: spi-nor: sfdp: Add support for SCCR map for multi-chip device tkuw584924
@ 2022-08-06  6:34 ` tkuw584924
  2022-08-06  6:34 ` [PATCH 5/8] mtd: spi-nor: spansion: Rework cypress_nor_quad_enable_volatile() " tkuw584924
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 19+ messages in thread
From: tkuw584924 @ 2022-08-06  6:34 UTC (permalink / raw)
  To: linux-mtd
  Cc: tudor.ambarus, pratyush, michael, miquel.raynal, richard,
	vigneshr, tkuw584924, Bacem.Daassi, Takahiro Kuwano

From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>

For multi-chip devices, we can use 512B page only when the all dice are
configured as 512B page size. Simple for-loop with params->num_of_dice can
check the CFR3V in each die. The volatile register address is calculated
inside the loop by using die number and volatile register offset.

The location of cypress_nor_set_page_size() call is moved from
post_bfpt_fixup() to post_sfdp_fixup(), because the number of dice and
volatile register offset are parsed after BFPT. The post_sfdp_fixup()
does not return error code so cypress_nor_set_page_size() prints error
message instead of returning error code.

Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
---
 drivers/mtd/spi-nor/spansion.c | 47 ++++++++++++++++++----------------
 1 file changed, 25 insertions(+), 22 deletions(-)

diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
index 676ffd6d12ec..d82dd750da9a 100644
--- a/drivers/mtd/spi-nor/spansion.c
+++ b/drivers/mtd/spi-nor/spansion.c
@@ -18,7 +18,7 @@
 #define SPINOR_REG_CYPRESS_CFR1V_QUAD_EN	BIT(1)	/* Quad Enable */
 #define SPINOR_REG_CYPRESS_CFR2V		0x00800003
 #define SPINOR_REG_CYPRESS_CFR2V_MEMLAT_11_24	0xb
-#define SPINOR_REG_CYPRESS_CFR3V		0x00800004
+#define SPINOR_REG_CYPRESS_CFR3			0x4
 #define SPINOR_REG_CYPRESS_CFR3V_PGSZ		BIT(4) /* Page size. */
 #define SPINOR_REG_CYPRESS_CFR5V		0x00800006
 #define SPINOR_REG_CYPRESS_CFR5V_OCT_DTR_EN	0x3
@@ -190,27 +190,34 @@ static int cypress_nor_quad_enable_volatile(struct spi_nor *nor)
  * The BFPT table advertises a 512B or 256B page size depending on part but the
  * page size is actually configurable (with the default being 256B). Read from
  * CFR3V[4] and set the correct size.
- *
- * Return: 0 on success, -errno otherwise.
  */
-static int cypress_nor_set_page_size(struct spi_nor *nor)
+static void cypress_nor_set_page_size(struct spi_nor *nor)
 {
+	struct spi_nor_flash_parameter *params = nor->params;
 	struct spi_mem_op op =
-		CYPRESS_NOR_RD_ANY_REG_OP(nor->params->addr_mode_nbytes,
-					  SPINOR_REG_CYPRESS_CFR3V,
+		CYPRESS_NOR_RD_ANY_REG_OP(params->addr_mode_nbytes, 0,
 					  nor->bouncebuf);
 	int ret;
+	u8 i;
 
-	ret = spi_nor_read_any_reg(nor, &op, nor->reg_proto);
-	if (ret)
-		return ret;
+	/* Set to default page size */
+	params->page_size = 256;
 
-	if (nor->bouncebuf[0] & SPINOR_REG_CYPRESS_CFR3V_PGSZ)
-		nor->params->page_size = 512;
-	else
-		nor->params->page_size = 256;
+	for (i = 0; i < params->num_of_dice; i++) {
+		op.addr.val = params->vreg_offset[i] + SPINOR_REG_CYPRESS_CFR3;
 
-	return 0;
+		ret = spi_nor_read_any_reg(nor, &op, nor->reg_proto);
+		if (ret) {
+			dev_err(nor->dev, "Failed to read page size configuration. Use default 256B.\n");
+			return;
+		}
+
+		if (!(nor->bouncebuf[0] & SPINOR_REG_CYPRESS_CFR3V_PGSZ))
+			return;
+	}
+
+	/* We reach here only when all dice are configured to 512B */
+	params->page_size = 512;
 }
 
 static int
@@ -221,7 +228,7 @@ s25hx_t_post_bfpt_fixup(struct spi_nor *nor,
 	/* Replace Quad Enable with volatile version */
 	nor->params->quad_enable = cypress_nor_quad_enable_volatile;
 
-	return cypress_nor_set_page_size(nor);
+	return 0;
 }
 
 static void s25hx_t_post_sfdp_fixup(struct spi_nor *nor)
@@ -246,6 +253,8 @@ static void s25hx_t_post_sfdp_fixup(struct spi_nor *nor)
 			break;
 		}
 	}
+
+	cypress_nor_set_page_size(nor);
 }
 
 static void s25hx_t_late_init(struct spi_nor *nor)
@@ -313,19 +322,13 @@ static void s28hs512t_post_sfdp_fixup(struct spi_nor *nor)
 	 * actual value for that is 4.
 	 */
 	nor->params->rdsr_addr_nbytes = 4;
-}
 
-static int s28hs512t_post_bfpt_fixup(struct spi_nor *nor,
-				     const struct sfdp_parameter_header *bfpt_header,
-				     const struct sfdp_bfpt *bfpt)
-{
-	return cypress_nor_set_page_size(nor);
+	cypress_nor_set_page_size(nor);
 }
 
 static const struct spi_nor_fixups s28hs512t_fixups = {
 	.default_init = s28hs512t_default_init,
 	.post_sfdp = s28hs512t_post_sfdp_fixup,
-	.post_bfpt = s28hs512t_post_bfpt_fixup,
 };
 
 static int
-- 
2.25.1


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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 5/8] mtd: spi-nor: spansion: Rework cypress_nor_quad_enable_volatile() for multi-chip device support
  2022-08-06  6:34 [PATCH 0/8] mtd: spi-nor: Add support for Infineon SEMPER s25hl02gt and s25hs02gt tkuw584924
                   ` (3 preceding siblings ...)
  2022-08-06  6:34 ` [PATCH 4/8] mtd: spi-nor: spansion: Rework cypress_nor_set_page_size() for multi-chip device support tkuw584924
@ 2022-08-06  6:34 ` tkuw584924
  2022-08-10 14:40   ` Takahiro Kuwano
  2022-08-06  6:34 ` [PATCH 6/8] mtd: spi-nor: spansion: Add a new ->ready() hook for multi-chip device tkuw584924
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 19+ messages in thread
From: tkuw584924 @ 2022-08-06  6:34 UTC (permalink / raw)
  To: linux-mtd
  Cc: tudor.ambarus, pratyush, michael, miquel.raynal, richard,
	vigneshr, tkuw584924, Bacem.Daassi, Takahiro Kuwano

From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>

For multi-chip devices, we need to enable QUAD by updating CFR1V in all
dice in the device. That is done by for-loop with params->num_of_dice.
The volatile register address is calculated inside the loop by using die
number and volatile register offset.

Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
---
 drivers/mtd/spi-nor/spansion.c | 65 +++++++++++++++++-----------------
 1 file changed, 33 insertions(+), 32 deletions(-)

diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
index d82dd750da9a..d7a61ea63139 100644
--- a/drivers/mtd/spi-nor/spansion.c
+++ b/drivers/mtd/spi-nor/spansion.c
@@ -14,7 +14,7 @@
 #define SPINOR_OP_CLSR		0x30	/* Clear status register 1 */
 #define SPINOR_OP_RD_ANY_REG			0x65	/* Read any register */
 #define SPINOR_OP_WR_ANY_REG			0x71	/* Write any register */
-#define SPINOR_REG_CYPRESS_CFR1V		0x00800002
+#define SPINOR_REG_CYPRESS_CFR1			0x2
 #define SPINOR_REG_CYPRESS_CFR1V_QUAD_EN	BIT(1)	/* Quad Enable */
 #define SPINOR_REG_CYPRESS_CFR2V		0x00800003
 #define SPINOR_REG_CYPRESS_CFR2V_MEMLAT_11_24	0xb
@@ -137,46 +137,47 @@ static int cypress_nor_octal_dtr_dis(struct spi_nor *nor)
 static int cypress_nor_quad_enable_volatile(struct spi_nor *nor)
 {
 	struct spi_mem_op op;
+	u32 addr;
+	u8 i;
 	u8 addr_mode_nbytes = nor->params->addr_mode_nbytes;
 	u8 cfr1v_written;
 	int ret;
 
-	op = (struct spi_mem_op)
-		CYPRESS_NOR_RD_ANY_REG_OP(addr_mode_nbytes,
-					  SPINOR_REG_CYPRESS_CFR1V,
-					  nor->bouncebuf);
-
-	ret = spi_nor_read_any_reg(nor, &op, nor->reg_proto);
-	if (ret)
-		return ret;
+	for (i = 0; i < nor->params->num_of_dice; i++) {
+		addr = nor->params->vreg_offset[i] + SPINOR_REG_CYPRESS_CFR1;
+		op = (struct spi_mem_op)
+			CYPRESS_NOR_RD_ANY_REG_OP(addr_mode_nbytes, addr,
+						  nor->bouncebuf);
+		ret = spi_nor_read_any_reg(nor, &op, nor->reg_proto);
+		if (ret)
+			return ret;
 
-	if (nor->bouncebuf[0] & SPINOR_REG_CYPRESS_CFR1V_QUAD_EN)
-		return 0;
+		if (nor->bouncebuf[0] & SPINOR_REG_CYPRESS_CFR1V_QUAD_EN)
+			return 0;
 
-	/* Update the Quad Enable bit. */
-	nor->bouncebuf[0] |= SPINOR_REG_CYPRESS_CFR1V_QUAD_EN;
-	op = (struct spi_mem_op)
-		CYPRESS_NOR_WR_ANY_REG_OP(addr_mode_nbytes,
-					  SPINOR_REG_CYPRESS_CFR1V, 1,
-					  nor->bouncebuf);
-	ret = spi_nor_write_any_volatile_reg(nor, &op, nor->reg_proto);
-	if (ret)
-		return ret;
+		/* Update the Quad Enable bit. */
+		nor->bouncebuf[0] |= SPINOR_REG_CYPRESS_CFR1V_QUAD_EN;
+		op = (struct spi_mem_op)
+			CYPRESS_NOR_WR_ANY_REG_OP(addr_mode_nbytes, addr, 1,
+						  nor->bouncebuf);
+		ret = spi_nor_write_any_volatile_reg(nor, &op, nor->reg_proto);
+		if (ret)
+			return ret;
 
-	cfr1v_written = nor->bouncebuf[0];
+		cfr1v_written = nor->bouncebuf[0];
 
-	/* Read back and check it. */
-	op = (struct spi_mem_op)
-		CYPRESS_NOR_RD_ANY_REG_OP(addr_mode_nbytes,
-					  SPINOR_REG_CYPRESS_CFR1V,
-					  nor->bouncebuf);
-	ret = spi_nor_read_any_reg(nor, &op, nor->reg_proto);
-	if (ret)
-		return ret;
+		/* Read back and check it. */
+		op = (struct spi_mem_op)
+			CYPRESS_NOR_RD_ANY_REG_OP(addr_mode_nbytes, addr,
+						  nor->bouncebuf);
+		ret = spi_nor_read_any_reg(nor, &op, nor->reg_proto);
+		if (ret)
+			return ret;
 
-	if (nor->bouncebuf[0] != cfr1v_written) {
-		dev_err(nor->dev, "CFR1: Read back test failed\n");
-		return -EIO;
+		if (nor->bouncebuf[0] != cfr1v_written) {
+			dev_err(nor->dev, "CFR1: Read back test failed\n");
+			return -EIO;
+		}
 	}
 
 	return 0;
-- 
2.25.1


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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 6/8] mtd: spi-nor: spansion: Add a new ->ready() hook for multi-chip device
  2022-08-06  6:34 [PATCH 0/8] mtd: spi-nor: Add support for Infineon SEMPER s25hl02gt and s25hs02gt tkuw584924
                   ` (4 preceding siblings ...)
  2022-08-06  6:34 ` [PATCH 5/8] mtd: spi-nor: spansion: Rework cypress_nor_quad_enable_volatile() " tkuw584924
@ 2022-08-06  6:34 ` tkuw584924
  2022-08-06  6:34 ` [PATCH 7/8] mtd: spi-nor: spansion: Introduce DEF_4BAM mfr flag tkuw584924
  2022-08-06  6:34 ` [PATCH 8/8] mtd: spi-nor: spansion: Add support for Infineon tkuw584924
  7 siblings, 0 replies; 19+ messages in thread
From: tkuw584924 @ 2022-08-06  6:34 UTC (permalink / raw)
  To: linux-mtd
  Cc: tudor.ambarus, pratyush, michael, miquel.raynal, richard,
	vigneshr, tkuw584924, Bacem.Daassi, Takahiro Kuwano

From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>

For multi-chip devices, we need to make sure the all dice in the device
are ready. The cypress_nor_sr_ready_and_clear() reads SR in each die and
returns true only when all dice are ready. This function also takes care
for program or erase error handling by reusing spansion_nor_clear_sr().
To do that, spansion_nor_clear_sr() is moved to top.

Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
---
 drivers/mtd/spi-nor/spansion.c | 95 ++++++++++++++++++++++++++--------
 1 file changed, 72 insertions(+), 23 deletions(-)

diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
index d7a61ea63139..fc9cc3484aa3 100644
--- a/drivers/mtd/spi-nor/spansion.c
+++ b/drivers/mtd/spi-nor/spansion.c
@@ -14,6 +14,7 @@
 #define SPINOR_OP_CLSR		0x30	/* Clear status register 1 */
 #define SPINOR_OP_RD_ANY_REG			0x65	/* Read any register */
 #define SPINOR_OP_WR_ANY_REG			0x71	/* Write any register */
+#define SPINOR_REG_CYPRESS_STR1			0x0
 #define SPINOR_REG_CYPRESS_CFR1			0x2
 #define SPINOR_REG_CYPRESS_CFR1V_QUAD_EN	BIT(1)	/* Quad Enable */
 #define SPINOR_REG_CYPRESS_CFR2V		0x00800003
@@ -44,6 +45,29 @@
 		   SPI_MEM_OP_NO_DUMMY,					\
 		   SPI_MEM_OP_NO_DATA)
 
+/**
+ * spansion_nor_clear_sr() - Clear the Status Register.
+ * @nor:	pointer to 'struct spi_nor'.
+ */
+static void spansion_nor_clear_sr(struct spi_nor *nor)
+{
+	int ret;
+
+	if (nor->spimem) {
+		struct spi_mem_op op = SPANSION_CLSR_OP;
+
+		spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
+
+		ret = spi_mem_exec_op(nor->spimem, &op);
+	} else {
+		ret = spi_nor_controller_ops_write_reg(nor, SPINOR_OP_CLSR,
+						       NULL, 0);
+	}
+
+	if (ret)
+		dev_dbg(nor->dev, "error %d clearing SR\n", ret);
+}
+
 static int cypress_nor_octal_dtr_en(struct spi_nor *nor)
 {
 	struct spi_mem_op op;
@@ -221,6 +245,51 @@ static void cypress_nor_set_page_size(struct spi_nor *nor)
 	params->page_size = 512;
 }
 
+/**
+ * cypress_nor_sr_ready_and_clear() - Query the Status Register of each die by
+ * using Read Any Regiser command to see if the whole flash is ready for new
+ * commands and clear it if there are any errors.
+ * @nor:	pointer to 'struct spi_nor'.
+ *
+ * Return: 1 if ready, 0 if not ready, -errno on errors.
+ */
+static int cypress_nor_sr_ready_and_clear(struct spi_nor *nor)
+{
+	struct spi_mem_op op =
+		CYPRESS_NOR_RD_ANY_REG_OP(nor->params->addr_mode_nbytes, 0,
+					  nor->bouncebuf);
+	int ret;
+	u8 i;
+
+	for (i = 0; i < nor->params->num_of_dice; i++) {
+		op.addr.val =
+			nor->params->vreg_offset[i] + SPINOR_REG_CYPRESS_STR1;
+		ret = spi_nor_read_any_reg(nor, &op, nor->reg_proto);
+		if (ret)
+			return ret;
+
+		if (nor->bouncebuf[0] & (SR_E_ERR | SR_P_ERR)) {
+			if (nor->bouncebuf[0] & SR_E_ERR)
+				dev_err(nor->dev, "Erase Error occurred\n");
+			else
+				dev_err(nor->dev, "Programming Error occurred\n");
+
+			spansion_nor_clear_sr(nor);
+
+			ret = spi_nor_write_disable(nor);
+			if (ret)
+				return ret;
+
+			return -EIO;
+		}
+
+		if (nor->bouncebuf[0] & SR_WIP)
+			return 0;
+	}
+
+	return 1;
+}
+
 static int
 s25hx_t_post_bfpt_fixup(struct spi_nor *nor,
 			const struct sfdp_parameter_header *bfpt_header,
@@ -267,6 +336,9 @@ static void s25hx_t_late_init(struct spi_nor *nor)
 
 	/* The writesize should be ECC data unit size */
 	params->writesize = 16;
+
+	if (nor->params->num_of_dice > 1)
+		nor->params->ready = cypress_nor_sr_ready_and_clear;
 }
 
 static struct spi_nor_fixups s25hx_t_fixups = {
@@ -470,29 +542,6 @@ static const struct flash_info spansion_nor_parts[] = {
 	},
 };
 
-/**
- * spansion_nor_clear_sr() - Clear the Status Register.
- * @nor:	pointer to 'struct spi_nor'.
- */
-static void spansion_nor_clear_sr(struct spi_nor *nor)
-{
-	int ret;
-
-	if (nor->spimem) {
-		struct spi_mem_op op = SPANSION_CLSR_OP;
-
-		spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
-
-		ret = spi_mem_exec_op(nor->spimem, &op);
-	} else {
-		ret = spi_nor_controller_ops_write_reg(nor, SPINOR_OP_CLSR,
-						       NULL, 0);
-	}
-
-	if (ret)
-		dev_dbg(nor->dev, "error %d clearing SR\n", ret);
-}
-
 /**
  * spansion_nor_sr_ready_and_clear() - Query the Status Register to see if the
  * flash is ready for new commands and clear it if there are any errors.
-- 
2.25.1


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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 7/8] mtd: spi-nor: spansion: Introduce DEF_4BAM mfr flag
  2022-08-06  6:34 [PATCH 0/8] mtd: spi-nor: Add support for Infineon SEMPER s25hl02gt and s25hs02gt tkuw584924
                   ` (5 preceding siblings ...)
  2022-08-06  6:34 ` [PATCH 6/8] mtd: spi-nor: spansion: Add a new ->ready() hook for multi-chip device tkuw584924
@ 2022-08-06  6:34 ` tkuw584924
  2022-08-06  6:34 ` [PATCH 8/8] mtd: spi-nor: spansion: Add support for Infineon tkuw584924
  7 siblings, 0 replies; 19+ messages in thread
From: tkuw584924 @ 2022-08-06  6:34 UTC (permalink / raw)
  To: linux-mtd
  Cc: tudor.ambarus, pratyush, michael, miquel.raynal, richard,
	vigneshr, tkuw584924, Bacem.Daassi, Takahiro Kuwano

From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>

The params->addr_mode_nbytes is set to 3 during BFPT parse in case of
BFPT_DWORD1_ADDRESS_BYTES_3_OR_4. Infineon SEMPER multi-chip devices are
in 4-byte address mode by factory default. A new manufacturer flag,
DEF_4BAM is introduced for such devices to fix the
params->addr_mode_nbytes to 4 in post_bfpt_fixup().

Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
---
 drivers/mtd/spi-nor/spansion.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
index fc9cc3484aa3..22b0be5d312b 100644
--- a/drivers/mtd/spi-nor/spansion.c
+++ b/drivers/mtd/spi-nor/spansion.c
@@ -10,6 +10,7 @@
 
 /* flash_info mfr_flag. Used to clear sticky prorietary SR bits. */
 #define USE_CLSR	BIT(0)
+#define DEF_4BAM	BIT(1)	/* 4-byte address mode by default */
 
 #define SPINOR_OP_CLSR		0x30	/* Clear status register 1 */
 #define SPINOR_OP_RD_ANY_REG			0x65	/* Read any register */
@@ -581,6 +582,17 @@ static int spansion_nor_sr_ready_and_clear(struct spi_nor *nor)
 	return !(nor->bouncebuf[0] & SR_WIP);
 }
 
+static int
+spansion_post_bfpt_fixup(struct spi_nor *nor,
+			 const struct sfdp_parameter_header *bfpt_header,
+			 const struct sfdp_bfpt *bfpt)
+{
+	if (nor->info->mfr_flags & DEF_4BAM)
+		nor->params->addr_mode_nbytes = 4;
+
+	return 0;
+}
+
 static void spansion_nor_late_init(struct spi_nor *nor)
 {
 	if (nor->params->size > SZ_16M) {
@@ -595,6 +607,7 @@ static void spansion_nor_late_init(struct spi_nor *nor)
 }
 
 static const struct spi_nor_fixups spansion_nor_fixups = {
+	.post_bfpt = spansion_post_bfpt_fixup,
 	.late_init = spansion_nor_late_init,
 };
 
-- 
2.25.1


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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 8/8] mtd: spi-nor: spansion: Add support for Infineon
  2022-08-06  6:34 [PATCH 0/8] mtd: spi-nor: Add support for Infineon SEMPER s25hl02gt and s25hs02gt tkuw584924
                   ` (6 preceding siblings ...)
  2022-08-06  6:34 ` [PATCH 7/8] mtd: spi-nor: spansion: Introduce DEF_4BAM mfr flag tkuw584924
@ 2022-08-06  6:34 ` tkuw584924
  2022-08-08  4:47   ` Tudor.Ambarus
  7 siblings, 1 reply; 19+ messages in thread
From: tkuw584924 @ 2022-08-06  6:34 UTC (permalink / raw)
  To: linux-mtd
  Cc: tudor.ambarus, pratyush, michael, miquel.raynal, richard,
	vigneshr, tkuw584924, Bacem.Daassi, Takahiro Kuwano

From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>

s25hl02gt and s25hs02gt

Add ID, flags, and fixup for s25hl02gt and s25hs02gt.
These parts are
  - Dual-die package parts
  - Not support chip erase
  - 4-byte addressing mode by default
  - Wrong param in SCCR map that needs to be fixed

Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
---
 drivers/mtd/spi-nor/spansion.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
index 22b0be5d312b..11f0bb509900 100644
--- a/drivers/mtd/spi-nor/spansion.c
+++ b/drivers/mtd/spi-nor/spansion.c
@@ -325,6 +325,10 @@ static void s25hx_t_post_sfdp_fixup(struct spi_nor *nor)
 		}
 	}
 
+	/* Fix the number of dice that is wrongly advertised in 2Gb parts. */
+	if (nor->params->size == SZ_256M)
+		nor->params->num_of_dice = 2;
+
 	cypress_nor_set_page_size(nor);
 }
 
@@ -526,6 +530,11 @@ static const struct flash_info spansion_nor_parts[] = {
 		PARSE_SFDP
 		MFR_FLAGS(USE_CLSR)
 		.fixups = &s25hx_t_fixups },
+	{ "s25hl02gt",  INFO6(0x342a1c, 0x0f0090, 256 * 1024, 1024)
+		PARSE_SFDP
+		FLAGS(NO_CHIP_ERASE)
+		MFR_FLAGS(DEF_4BAM)
+		.fixups = &s25hx_t_fixups },
 	{ "s25hs512t",  INFO6(0x342b1a, 0x0f0390, 256 * 1024, 256)
 		PARSE_SFDP
 		MFR_FLAGS(USE_CLSR)
@@ -534,6 +543,11 @@ static const struct flash_info spansion_nor_parts[] = {
 		PARSE_SFDP
 		MFR_FLAGS(USE_CLSR)
 		.fixups = &s25hx_t_fixups },
+	{ "s25hs02gt",  INFO6(0x342b1c, 0x0f0090, 256 * 1024, 1024)
+		PARSE_SFDP
+		FLAGS(NO_CHIP_ERASE)
+		MFR_FLAGS(DEF_4BAM)
+		.fixups = &s25hx_t_fixups },
 	{ "cy15x104q",  INFO6(0x042cc2, 0x7f7f7f, 512 * 1024, 1)
 		FLAGS(SPI_NOR_NO_ERASE) },
 	{ "s28hs512t",   INFO(0x345b1a,      0, 256 * 1024, 256)
-- 
2.25.1


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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH 8/8] mtd: spi-nor: spansion: Add support for Infineon
  2022-08-06  6:34 ` [PATCH 8/8] mtd: spi-nor: spansion: Add support for Infineon tkuw584924
@ 2022-08-08  4:47   ` Tudor.Ambarus
  2022-08-08  5:42     ` Takahiro Kuwano
  0 siblings, 1 reply; 19+ messages in thread
From: Tudor.Ambarus @ 2022-08-08  4:47 UTC (permalink / raw)
  To: tkuw584924, linux-mtd
  Cc: pratyush, michael, miquel.raynal, richard, vigneshr,
	Bacem.Daassi, Takahiro.Kuwano

On 8/6/22 09:34, tkuw584924@gmail.com wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>

Hi!

> 
> s25hl02gt and s25hs02gt
> 
> Add ID, flags, and fixup for s25hl02gt and s25hs02gt.
> These parts are
>   - Dual-die package parts
>   - Not support chip erase
>   - 4-byte addressing mode by default

CFR2N[7] CFR2V[7] says that: "For the DDP or QDP devices, if ADRBYT = 0
only the first 128 Mb of die 1 can be accessed."
So there are flashes of the same family that are by default in 3 byte address
mode. You added support just for a subset of them and used a generic name,
which is not accurate, right?

Can we instead make an algorithm to determine the current address mode?

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 8/8] mtd: spi-nor: spansion: Add support for Infineon
  2022-08-08  4:47   ` Tudor.Ambarus
@ 2022-08-08  5:42     ` Takahiro Kuwano
  2022-08-08  6:08       ` Tudor.Ambarus
  0 siblings, 1 reply; 19+ messages in thread
From: Takahiro Kuwano @ 2022-08-08  5:42 UTC (permalink / raw)
  To: Tudor.Ambarus, linux-mtd
  Cc: pratyush, michael, miquel.raynal, richard, vigneshr,
	Bacem.Daassi, Takahiro.Kuwano

On 8/8/2022 1:47 PM, Tudor.Ambarus@microchip.com wrote:
> On 8/6/22 09:34, tkuw584924@gmail.com wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
> 
> Hi!
> 
>>
>> s25hl02gt and s25hs02gt
>>
>> Add ID, flags, and fixup for s25hl02gt and s25hs02gt.
>> These parts are
>>   - Dual-die package parts
>>   - Not support chip erase
>>   - 4-byte addressing mode by default
> 
> CFR2N[7] CFR2V[7] says that: "For the DDP or QDP devices, if ADRBYT = 0
> only the first 128 Mb of die 1 can be accessed."
> So there are flashes of the same family that are by default in 3 byte address
> mode. You added support just for a subset of them and used a generic name,
> which is not accurate, right?
> 
We added model #15 (3-byte address mode by default) to address special
requirement from a customer who needs to use bootrom with 3-byte addressing.
Anyway, I overlooked model # difference. Thanks for pointing out this.

> Can we instead make an algorithm to determine the current address mode?
> 
I have just found that we can distinguish model # via BFPT DWORD16.
If Hardware reset, Software reset, or Power cycle can exit 4-byte address
mode, that means the device is 3-byte address mode by default.
The questions is can we implement in spi_nor_parse_bfpt() in sfdp.c or
post_bfpt_fixup() in manufacturer code?

Thanks,
Takahiro

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 8/8] mtd: spi-nor: spansion: Add support for Infineon
  2022-08-08  5:42     ` Takahiro Kuwano
@ 2022-08-08  6:08       ` Tudor.Ambarus
  2022-08-08  6:41         ` Takahiro Kuwano
  0 siblings, 1 reply; 19+ messages in thread
From: Tudor.Ambarus @ 2022-08-08  6:08 UTC (permalink / raw)
  To: tkuw584924, linux-mtd
  Cc: pratyush, michael, miquel.raynal, richard, vigneshr,
	Bacem.Daassi, Takahiro.Kuwano

On 8/8/22 08:42, Takahiro Kuwano wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On 8/8/2022 1:47 PM, Tudor.Ambarus@microchip.com wrote:
>> On 8/6/22 09:34, tkuw584924@gmail.com wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>
>>> From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
>>
>> Hi!
>>
>>>
>>> s25hl02gt and s25hs02gt
>>>
>>> Add ID, flags, and fixup for s25hl02gt and s25hs02gt.
>>> These parts are
>>>   - Dual-die package parts
>>>   - Not support chip erase
>>>   - 4-byte addressing mode by default
>>
>> CFR2N[7] CFR2V[7] says that: "For the DDP or QDP devices, if ADRBYT = 0
>> only the first 128 Mb of die 1 can be accessed."
>> So there are flashes of the same family that are by default in 3 byte address
>> mode. You added support just for a subset of them and used a generic name,
>> which is not accurate, right?
>>
> We added model #15 (3-byte address mode by default) to address special
> requirement from a customer who needs to use bootrom with 3-byte addressing.
> Anyway, I overlooked model # difference. Thanks for pointing out this.
> 
>> Can we instead make an algorithm to determine the current address mode?
>>
> I have just found that we can distinguish model # via BFPT DWORD16.
> If Hardware reset, Software reset, or Power cycle can exit 4-byte address
> mode, that means the device is 3-byte address mode by default.

I don't think this will help us. It doesn't matter the default mode if you
have a non volatile register that can be updated and changes the default
mode.

Are there any registers/data that can be read successively in 3 byte addr mode
and then in 4 byte addr mode? We'll then compare what we receive from the flash
with a known value and determine the mode.

> The questions is can we implement in spi_nor_parse_bfpt() in sfdp.c or
> post_bfpt_fixup() in manufacturer code?
> 


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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 8/8] mtd: spi-nor: spansion: Add support for Infineon
  2022-08-08  6:08       ` Tudor.Ambarus
@ 2022-08-08  6:41         ` Takahiro Kuwano
  2022-08-08  7:34           ` Tudor.Ambarus
  0 siblings, 1 reply; 19+ messages in thread
From: Takahiro Kuwano @ 2022-08-08  6:41 UTC (permalink / raw)
  To: Tudor.Ambarus, linux-mtd
  Cc: pratyush, michael, miquel.raynal, richard, vigneshr,
	Bacem.Daassi, Takahiro.Kuwano

On 8/8/2022 3:08 PM, Tudor.Ambarus@microchip.com wrote:
> On 8/8/22 08:42, Takahiro Kuwano wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> On 8/8/2022 1:47 PM, Tudor.Ambarus@microchip.com wrote:
>>> On 8/6/22 09:34, tkuw584924@gmail.com wrote:
>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>>
>>>> From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
>>>
>>> Hi!
>>>
>>>>
>>>> s25hl02gt and s25hs02gt
>>>>
>>>> Add ID, flags, and fixup for s25hl02gt and s25hs02gt.
>>>> These parts are
>>>>   - Dual-die package parts
>>>>   - Not support chip erase
>>>>   - 4-byte addressing mode by default
>>>
>>> CFR2N[7] CFR2V[7] says that: "For the DDP or QDP devices, if ADRBYT = 0
>>> only the first 128 Mb of die 1 can be accessed."
>>> So there are flashes of the same family that are by default in 3 byte address
>>> mode. You added support just for a subset of them and used a generic name,
>>> which is not accurate, right?
>>>
>> We added model #15 (3-byte address mode by default) to address special
>> requirement from a customer who needs to use bootrom with 3-byte addressing.
>> Anyway, I overlooked model # difference. Thanks for pointing out this.
>>
>>> Can we instead make an algorithm to determine the current address mode?
>>>
>> I have just found that we can distinguish model # via BFPT DWORD16.
>> If Hardware reset, Software reset, or Power cycle can exit 4-byte address
>> mode, that means the device is 3-byte address mode by default.
> 
> I don't think this will help us. It doesn't matter the default mode if you
> have a non volatile register that can be updated and changes the default
> mode.
> 
> Are there any registers/data that can be read successively in 3 byte addr mode
> and then in 4 byte addr mode? We'll then compare what we receive from the flash
> with a known value and determine the mode.
> 
As we discussed before [0], if address mode in the controller and device are
different, the read data will be undetermined.

But if we really want...
Compare SR1 data read by RDSR1(05h - No Addr) and RDAR(65h - Addr 0).
In most cases (without block protection), SR1=00h. The value of 00h would be
awkward to determine if this is 'real' output from Flash or not. So, use
WREN(06h) and WRDI(04h) that flips BIT(1) in SR1.

Therefore, something like:
1) RDSR1
2) RDAR with 3-byte addr (000000h)
3) If #1 == #2
	4) WREN
	5) RDAR with 3-byte addr (000000h)
	6) BIT(1) is SR1==1?
	...

Or simply WREN -> RDAR -> WRDI -> RDAR then check if only BIT(1) is toggled.

[0] https://lore.kernel.org/all/070bfe6a-00e8-1c59-c9db-52d249dfbcfe@microchip.com/

Thanks,
Takahiro

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 8/8] mtd: spi-nor: spansion: Add support for Infineon
  2022-08-08  6:41         ` Takahiro Kuwano
@ 2022-08-08  7:34           ` Tudor.Ambarus
  2022-08-08  8:09             ` Takahiro Kuwano
  0 siblings, 1 reply; 19+ messages in thread
From: Tudor.Ambarus @ 2022-08-08  7:34 UTC (permalink / raw)
  To: tkuw584924, linux-mtd
  Cc: pratyush, michael, miquel.raynal, richard, vigneshr,
	Bacem.Daassi, Takahiro.Kuwano

On 8/8/22 09:41, Takahiro Kuwano wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On 8/8/2022 3:08 PM, Tudor.Ambarus@microchip.com wrote:
>> On 8/8/22 08:42, Takahiro Kuwano wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>
>>> On 8/8/2022 1:47 PM, Tudor.Ambarus@microchip.com wrote:
>>>> On 8/6/22 09:34, tkuw584924@gmail.com wrote:
>>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>>>
>>>>> From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
>>>>
>>>> Hi!
>>>>
>>>>>
>>>>> s25hl02gt and s25hs02gt
>>>>>
>>>>> Add ID, flags, and fixup for s25hl02gt and s25hs02gt.
>>>>> These parts are
>>>>>   - Dual-die package parts
>>>>>   - Not support chip erase
>>>>>   - 4-byte addressing mode by default
>>>>
>>>> CFR2N[7] CFR2V[7] says that: "For the DDP or QDP devices, if ADRBYT = 0
>>>> only the first 128 Mb of die 1 can be accessed."
>>>> So there are flashes of the same family that are by default in 3 byte address
>>>> mode. You added support just for a subset of them and used a generic name,
>>>> which is not accurate, right?
>>>>
>>> We added model #15 (3-byte address mode by default) to address special
>>> requirement from a customer who needs to use bootrom with 3-byte addressing.
>>> Anyway, I overlooked model # difference. Thanks for pointing out this.
>>>
>>>> Can we instead make an algorithm to determine the current address mode?
>>>>
>>> I have just found that we can distinguish model # via BFPT DWORD16.
>>> If Hardware reset, Software reset, or Power cycle can exit 4-byte address
>>> mode, that means the device is 3-byte address mode by default.
>>
>> I don't think this will help us. It doesn't matter the default mode if you
>> have a non volatile register that can be updated and changes the default
>> mode.
>>
>> Are there any registers/data that can be read successively in 3 byte addr mode
>> and then in 4 byte addr mode? We'll then compare what we receive from the flash
>> with a known value and determine the mode.
>>
> As we discussed before [0], if address mode in the controller and device are

I remember, yes, but without determining the mode, the driver will work only
with flashes that come with the factory settings. The driver will be unusable
if someone changes the address mode in a non volatile way, right?

> different, the read data will be undetermined.
> 
> But if we really want...
> Compare SR1 data read by RDSR1(05h - No Addr) and RDAR(65h - Addr 0).
> In most cases (without block protection), SR1=00h. The value of 00h would be
> awkward to determine if this is 'real' output from Flash or not. So, use> WREN(06h) and WRDI(04h) that flips BIT(1) in SR1.

Would be good to have more fixed/OPT-like bits, or if we could change more bits
to increase the chances to not hit just some undetermined data.
> 
> Therefore, something like:
> 1) RDSR1
> 2) RDAR with 3-byte addr (000000h)
> 3) If #1 == #2
>         4) WREN
>         5) RDAR with 3-byte addr (000000h)
>         6) BIT(1) is SR1==1?
>         ...
> 
> Or simply WREN -> RDAR -> WRDI -> RDAR then check if only BIT(1) is toggled.

Both may work, yes, but making the assumption on only one bit is fragile.
Can we use the Read Any Register Command with 3 and 4 byte address modes and
compare the values? Are there any registers with fixed values?

> 
> [0] https://lore.kernel.org/all/070bfe6a-00e8-1c59-c9db-52d249dfbcfe@microchip.com/
> 
> Thanks,
> Takahiro


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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 8/8] mtd: spi-nor: spansion: Add support for Infineon
  2022-08-08  7:34           ` Tudor.Ambarus
@ 2022-08-08  8:09             ` Takahiro Kuwano
  2022-08-08  8:26               ` Tudor.Ambarus
  0 siblings, 1 reply; 19+ messages in thread
From: Takahiro Kuwano @ 2022-08-08  8:09 UTC (permalink / raw)
  To: Tudor.Ambarus, linux-mtd
  Cc: pratyush, michael, miquel.raynal, richard, vigneshr,
	Bacem.Daassi, Takahiro.Kuwano

On 8/8/2022 4:34 PM, Tudor.Ambarus@microchip.com wrote:
> On 8/8/22 09:41, Takahiro Kuwano wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> On 8/8/2022 3:08 PM, Tudor.Ambarus@microchip.com wrote:
>>> On 8/8/22 08:42, Takahiro Kuwano wrote:
>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>>
>>>> On 8/8/2022 1:47 PM, Tudor.Ambarus@microchip.com wrote:
>>>>> On 8/6/22 09:34, tkuw584924@gmail.com wrote:
>>>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>>>>
>>>>>> From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
>>>>>
>>>>> Hi!
>>>>>
>>>>>>
>>>>>> s25hl02gt and s25hs02gt
>>>>>>
>>>>>> Add ID, flags, and fixup for s25hl02gt and s25hs02gt.
>>>>>> These parts are
>>>>>>   - Dual-die package parts
>>>>>>   - Not support chip erase
>>>>>>   - 4-byte addressing mode by default
>>>>>
>>>>> CFR2N[7] CFR2V[7] says that: "For the DDP or QDP devices, if ADRBYT = 0
>>>>> only the first 128 Mb of die 1 can be accessed."
>>>>> So there are flashes of the same family that are by default in 3 byte address
>>>>> mode. You added support just for a subset of them and used a generic name,
>>>>> which is not accurate, right?
>>>>>
>>>> We added model #15 (3-byte address mode by default) to address special
>>>> requirement from a customer who needs to use bootrom with 3-byte addressing.
>>>> Anyway, I overlooked model # difference. Thanks for pointing out this.
>>>>
>>>>> Can we instead make an algorithm to determine the current address mode?
>>>>>
>>>> I have just found that we can distinguish model # via BFPT DWORD16.
>>>> If Hardware reset, Software reset, or Power cycle can exit 4-byte address
>>>> mode, that means the device is 3-byte address mode by default.
>>>
>>> I don't think this will help us. It doesn't matter the default mode if you
>>> have a non volatile register that can be updated and changes the default
>>> mode.
>>>
>>> Are there any registers/data that can be read successively in 3 byte addr mode
>>> and then in 4 byte addr mode? We'll then compare what we receive from the flash
>>> with a known value and determine the mode.
>>>
>> As we discussed before [0], if address mode in the controller and device are
> 
> I remember, yes, but without determining the mode, the driver will work only
> with flashes that come with the factory settings. The driver will be unusable
> if someone changes the address mode in a non volatile way, right?
> 
Yes, right.

>> different, the read data will be undetermined.
>>
>> But if we really want...
>> Compare SR1 data read by RDSR1(05h - No Addr) and RDAR(65h - Addr 0).
>> In most cases (without block protection), SR1=00h. The value of 00h would be
>> awkward to determine if this is 'real' output from Flash or not. So, use> WREN(06h) and WRDI(04h) that flips BIT(1) in SR1.
> 
> Would be good to have more fixed/OPT-like bits, or if we could change more bits
> to increase the chances to not hit just some undetermined data.
>>
>> Therefore, something like:
>> 1) RDSR1
>> 2) RDAR with 3-byte addr (000000h)
>> 3) If #1 == #2
>>         4) WREN
>>         5) RDAR with 3-byte addr (000000h)
>>         6) BIT(1) is SR1==1?
>>         ...
>>
>> Or simply WREN -> RDAR -> WRDI -> RDAR then check if only BIT(1) is toggled.
> 
> Both may work, yes, but making the assumption on only one bit is fragile.
> Can we use the Read Any Register Command with 3 and 4 byte address modes and
> compare the values? Are there any registers with fixed values?
>
Register values can vary because it's register:)

So... let's use Data Integrity Check CRC registers. These registers do not
have fixed values but we can calculate expected values by offline. Read
several bytes (>=4) from Flash array with Read(03h) then calculate CRC by
crc32(). Issue Data integrity Check command (5Bh) followed by start and
end address (4-byte for each), wait till ready. Read calculated CRC by
Read Any Register in 3 and 4 byte address (00800095h~0080098h) then compare
the crc32() result and register read result.

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 8/8] mtd: spi-nor: spansion: Add support for Infineon
  2022-08-08  8:09             ` Takahiro Kuwano
@ 2022-08-08  8:26               ` Tudor.Ambarus
  2022-08-08  8:31                 ` Takahiro Kuwano
  0 siblings, 1 reply; 19+ messages in thread
From: Tudor.Ambarus @ 2022-08-08  8:26 UTC (permalink / raw)
  To: tkuw584924, linux-mtd
  Cc: pratyush, michael, miquel.raynal, richard, vigneshr,
	Bacem.Daassi, Takahiro.Kuwano

On 8/8/22 11:09, Takahiro Kuwano wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On 8/8/2022 4:34 PM, Tudor.Ambarus@microchip.com wrote:
>> On 8/8/22 09:41, Takahiro Kuwano wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>
>>> On 8/8/2022 3:08 PM, Tudor.Ambarus@microchip.com wrote:
>>>> On 8/8/22 08:42, Takahiro Kuwano wrote:
>>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>>>
>>>>> On 8/8/2022 1:47 PM, Tudor.Ambarus@microchip.com wrote:
>>>>>> On 8/6/22 09:34, tkuw584924@gmail.com wrote:
>>>>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>>>>>
>>>>>>> From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
>>>>>>
>>>>>> Hi!
>>>>>>
>>>>>>>
>>>>>>> s25hl02gt and s25hs02gt
>>>>>>>
>>>>>>> Add ID, flags, and fixup for s25hl02gt and s25hs02gt.
>>>>>>> These parts are
>>>>>>>   - Dual-die package parts
>>>>>>>   - Not support chip erase
>>>>>>>   - 4-byte addressing mode by default
>>>>>>
>>>>>> CFR2N[7] CFR2V[7] says that: "For the DDP or QDP devices, if ADRBYT = 0
>>>>>> only the first 128 Mb of die 1 can be accessed."
>>>>>> So there are flashes of the same family that are by default in 3 byte address
>>>>>> mode. You added support just for a subset of them and used a generic name,
>>>>>> which is not accurate, right?
>>>>>>
>>>>> We added model #15 (3-byte address mode by default) to address special
>>>>> requirement from a customer who needs to use bootrom with 3-byte addressing.
>>>>> Anyway, I overlooked model # difference. Thanks for pointing out this.
>>>>>
>>>>>> Can we instead make an algorithm to determine the current address mode?
>>>>>>
>>>>> I have just found that we can distinguish model # via BFPT DWORD16.
>>>>> If Hardware reset, Software reset, or Power cycle can exit 4-byte address
>>>>> mode, that means the device is 3-byte address mode by default.
>>>>
>>>> I don't think this will help us. It doesn't matter the default mode if you
>>>> have a non volatile register that can be updated and changes the default
>>>> mode.
>>>>
>>>> Are there any registers/data that can be read successively in 3 byte addr mode
>>>> and then in 4 byte addr mode? We'll then compare what we receive from the flash
>>>> with a known value and determine the mode.
>>>>
>>> As we discussed before [0], if address mode in the controller and device are
>>
>> I remember, yes, but without determining the mode, the driver will work only
>> with flashes that come with the factory settings. The driver will be unusable
>> if someone changes the address mode in a non volatile way, right?
>>
> Yes, right.
> 
>>> different, the read data will be undetermined.
>>>
>>> But if we really want...
>>> Compare SR1 data read by RDSR1(05h - No Addr) and RDAR(65h - Addr 0).
>>> In most cases (without block protection), SR1=00h. The value of 00h would be
>>> awkward to determine if this is 'real' output from Flash or not. So, use> WREN(06h) and WRDI(04h) that flips BIT(1) in SR1.
>>
>> Would be good to have more fixed/OPT-like bits, or if we could change more bits
>> to increase the chances to not hit just some undetermined data.
>>>
>>> Therefore, something like:
>>> 1) RDSR1
>>> 2) RDAR with 3-byte addr (000000h)
>>> 3) If #1 == #2
>>>         4) WREN
>>>         5) RDAR with 3-byte addr (000000h)
>>>         6) BIT(1) is SR1==1?
>>>         ...
>>>
>>> Or simply WREN -> RDAR -> WRDI -> RDAR then check if only BIT(1) is toggled.
>>
>> Both may work, yes, but making the assumption on only one bit is fragile.
>> Can we use the Read Any Register Command with 3 and 4 byte address modes and
>> compare the values? Are there any registers with fixed values?
>>
> Register values can vary because it's register:)
> 
> So... let's use Data Integrity Check CRC registers. These registers do not
> have fixed values but we can calculate expected values by offline. Read
> several bytes (>=4) from Flash array with Read(03h) then calculate CRC by
> crc32(). Issue Data integrity Check command (5Bh) followed by start and
> end address (4-byte for each), wait till ready. Read calculated CRC by
> Read Any Register in 3 and 4 byte address (00800095h~0080098h) then compare
> the crc32() result and register read result.

Much better, yes. I think it is worth it. What's your opinion, Takahiro?
Others, Michael, Pratyush?

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 8/8] mtd: spi-nor: spansion: Add support for Infineon
  2022-08-08  8:26               ` Tudor.Ambarus
@ 2022-08-08  8:31                 ` Takahiro Kuwano
  2022-08-12  8:15                   ` Takahiro Kuwano
  0 siblings, 1 reply; 19+ messages in thread
From: Takahiro Kuwano @ 2022-08-08  8:31 UTC (permalink / raw)
  To: Tudor.Ambarus, linux-mtd
  Cc: pratyush, michael, miquel.raynal, richard, vigneshr,
	Bacem.Daassi, Takahiro.Kuwano

On 8/8/2022 5:26 PM, Tudor.Ambarus@microchip.com wrote:
> On 8/8/22 11:09, Takahiro Kuwano wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> On 8/8/2022 4:34 PM, Tudor.Ambarus@microchip.com wrote:
>>> On 8/8/22 09:41, Takahiro Kuwano wrote:
>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>>
>>>> On 8/8/2022 3:08 PM, Tudor.Ambarus@microchip.com wrote:
>>>>> On 8/8/22 08:42, Takahiro Kuwano wrote:
>>>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>>>>
>>>>>> On 8/8/2022 1:47 PM, Tudor.Ambarus@microchip.com wrote:
>>>>>>> On 8/6/22 09:34, tkuw584924@gmail.com wrote:
>>>>>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>>>>>>
>>>>>>>> From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
>>>>>>>
>>>>>>> Hi!
>>>>>>>
>>>>>>>>
>>>>>>>> s25hl02gt and s25hs02gt
>>>>>>>>
>>>>>>>> Add ID, flags, and fixup for s25hl02gt and s25hs02gt.
>>>>>>>> These parts are
>>>>>>>>   - Dual-die package parts
>>>>>>>>   - Not support chip erase
>>>>>>>>   - 4-byte addressing mode by default
>>>>>>>
>>>>>>> CFR2N[7] CFR2V[7] says that: "For the DDP or QDP devices, if ADRBYT = 0
>>>>>>> only the first 128 Mb of die 1 can be accessed."
>>>>>>> So there are flashes of the same family that are by default in 3 byte address
>>>>>>> mode. You added support just for a subset of them and used a generic name,
>>>>>>> which is not accurate, right?
>>>>>>>
>>>>>> We added model #15 (3-byte address mode by default) to address special
>>>>>> requirement from a customer who needs to use bootrom with 3-byte addressing.
>>>>>> Anyway, I overlooked model # difference. Thanks for pointing out this.
>>>>>>
>>>>>>> Can we instead make an algorithm to determine the current address mode?
>>>>>>>
>>>>>> I have just found that we can distinguish model # via BFPT DWORD16.
>>>>>> If Hardware reset, Software reset, or Power cycle can exit 4-byte address
>>>>>> mode, that means the device is 3-byte address mode by default.
>>>>>
>>>>> I don't think this will help us. It doesn't matter the default mode if you
>>>>> have a non volatile register that can be updated and changes the default
>>>>> mode.
>>>>>
>>>>> Are there any registers/data that can be read successively in 3 byte addr mode
>>>>> and then in 4 byte addr mode? We'll then compare what we receive from the flash
>>>>> with a known value and determine the mode.
>>>>>
>>>> As we discussed before [0], if address mode in the controller and device are
>>>
>>> I remember, yes, but without determining the mode, the driver will work only
>>> with flashes that come with the factory settings. The driver will be unusable
>>> if someone changes the address mode in a non volatile way, right?
>>>
>> Yes, right.
>>
>>>> different, the read data will be undetermined.
>>>>
>>>> But if we really want...
>>>> Compare SR1 data read by RDSR1(05h - No Addr) and RDAR(65h - Addr 0).
>>>> In most cases (without block protection), SR1=00h. The value of 00h would be
>>>> awkward to determine if this is 'real' output from Flash or not. So, use> WREN(06h) and WRDI(04h) that flips BIT(1) in SR1.
>>>
>>> Would be good to have more fixed/OPT-like bits, or if we could change more bits
>>> to increase the chances to not hit just some undetermined data.
>>>>
>>>> Therefore, something like:
>>>> 1) RDSR1
>>>> 2) RDAR with 3-byte addr (000000h)
>>>> 3) If #1 == #2
>>>>         4) WREN
>>>>         5) RDAR with 3-byte addr (000000h)
>>>>         6) BIT(1) is SR1==1?
>>>>         ...
>>>>
>>>> Or simply WREN -> RDAR -> WRDI -> RDAR then check if only BIT(1) is toggled.
>>>
>>> Both may work, yes, but making the assumption on only one bit is fragile.
>>> Can we use the Read Any Register Command with 3 and 4 byte address modes and
>>> compare the values? Are there any registers with fixed values?
>>>
>> Register values can vary because it's register:)
>>
>> So... let's use Data Integrity Check CRC registers. These registers do not
>> have fixed values but we can calculate expected values by offline. Read
>> several bytes (>=4) from Flash array with Read(03h) then calculate CRC by
>> crc32(). Issue Data integrity Check command (5Bh) followed by start and
>> end address (4-byte for each), wait till ready. Read calculated CRC by
>> Read Any Register in 3 and 4 byte address (00800095h~0080098h) then compare
>> the crc32() result and register read result.
> 
> Much better, yes. I think it is worth it. What's your opinion, Takahiro?
> Others, Michael, Pratyush?
> 
OK, I will prototype and test it.

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 5/8] mtd: spi-nor: spansion: Rework cypress_nor_quad_enable_volatile() for multi-chip device support
  2022-08-06  6:34 ` [PATCH 5/8] mtd: spi-nor: spansion: Rework cypress_nor_quad_enable_volatile() " tkuw584924
@ 2022-08-10 14:40   ` Takahiro Kuwano
  0 siblings, 0 replies; 19+ messages in thread
From: Takahiro Kuwano @ 2022-08-10 14:40 UTC (permalink / raw)
  To: linux-mtd
  Cc: tudor.ambarus, pratyush, michael, miquel.raynal, richard,
	vigneshr, Bacem.Daassi, Takahiro Kuwano

On 8/6/2022 3:34 PM, tkuw584924@gmail.com wrote:
> From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
> 
> For multi-chip devices, we need to enable QUAD by updating CFR1V in all
> dice in the device. That is done by for-loop with params->num_of_dice.
> The volatile register address is calculated inside the loop by using die
> number and volatile register offset.
> 
> Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
> ---
>  drivers/mtd/spi-nor/spansion.c | 65 +++++++++++++++++-----------------
>  1 file changed, 33 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
> index d82dd750da9a..d7a61ea63139 100644
> --- a/drivers/mtd/spi-nor/spansion.c
> +++ b/drivers/mtd/spi-nor/spansion.c
> @@ -14,7 +14,7 @@
>  #define SPINOR_OP_CLSR		0x30	/* Clear status register 1 */
>  #define SPINOR_OP_RD_ANY_REG			0x65	/* Read any register */
>  #define SPINOR_OP_WR_ANY_REG			0x71	/* Write any register */
> -#define SPINOR_REG_CYPRESS_CFR1V		0x00800002
> +#define SPINOR_REG_CYPRESS_CFR1			0x2
>  #define SPINOR_REG_CYPRESS_CFR1V_QUAD_EN	BIT(1)	/* Quad Enable */
>  #define SPINOR_REG_CYPRESS_CFR2V		0x00800003
>  #define SPINOR_REG_CYPRESS_CFR2V_MEMLAT_11_24	0xb
> @@ -137,46 +137,47 @@ static int cypress_nor_octal_dtr_dis(struct spi_nor *nor)
>  static int cypress_nor_quad_enable_volatile(struct spi_nor *nor)
>  {
>  	struct spi_mem_op op;
> +	u32 addr;
> +	u8 i;
>  	u8 addr_mode_nbytes = nor->params->addr_mode_nbytes;
>  	u8 cfr1v_written;
>  	int ret;
>  
> -	op = (struct spi_mem_op)
> -		CYPRESS_NOR_RD_ANY_REG_OP(addr_mode_nbytes,
> -					  SPINOR_REG_CYPRESS_CFR1V,
> -					  nor->bouncebuf);
> -
> -	ret = spi_nor_read_any_reg(nor, &op, nor->reg_proto);
> -	if (ret)
> -		return ret;
> +	for (i = 0; i < nor->params->num_of_dice; i++) {
> +		addr = nor->params->vreg_offset[i] + SPINOR_REG_CYPRESS_CFR1;
> +		op = (struct spi_mem_op)
> +			CYPRESS_NOR_RD_ANY_REG_OP(addr_mode_nbytes, addr,
> +						  nor->bouncebuf);
> +		ret = spi_nor_read_any_reg(nor, &op, nor->reg_proto);
> +		if (ret)
> +			return ret;
>  
> -	if (nor->bouncebuf[0] & SPINOR_REG_CYPRESS_CFR1V_QUAD_EN)
> -		return 0;
> +		if (nor->bouncebuf[0] & SPINOR_REG_CYPRESS_CFR1V_QUAD_EN)
> +			return 0;
Should be 'continue;'.

>  
> -	/* Update the Quad Enable bit. */
> -	nor->bouncebuf[0] |= SPINOR_REG_CYPRESS_CFR1V_QUAD_EN;
> -	op = (struct spi_mem_op)
> -		CYPRESS_NOR_WR_ANY_REG_OP(addr_mode_nbytes,
> -					  SPINOR_REG_CYPRESS_CFR1V, 1,
> -					  nor->bouncebuf);
> -	ret = spi_nor_write_any_volatile_reg(nor, &op, nor->reg_proto);
> -	if (ret)
> -		return ret;
> +		/* Update the Quad Enable bit. */
> +		nor->bouncebuf[0] |= SPINOR_REG_CYPRESS_CFR1V_QUAD_EN;
> +		op = (struct spi_mem_op)
> +			CYPRESS_NOR_WR_ANY_REG_OP(addr_mode_nbytes, addr, 1,
> +						  nor->bouncebuf);
> +		ret = spi_nor_write_any_volatile_reg(nor, &op, nor->reg_proto);
> +		if (ret)
> +			return ret;
>  
> -	cfr1v_written = nor->bouncebuf[0];
> +		cfr1v_written = nor->bouncebuf[0];
>  
> -	/* Read back and check it. */
> -	op = (struct spi_mem_op)
> -		CYPRESS_NOR_RD_ANY_REG_OP(addr_mode_nbytes,
> -					  SPINOR_REG_CYPRESS_CFR1V,
> -					  nor->bouncebuf);
> -	ret = spi_nor_read_any_reg(nor, &op, nor->reg_proto);
> -	if (ret)
> -		return ret;
> +		/* Read back and check it. */
> +		op = (struct spi_mem_op)
> +			CYPRESS_NOR_RD_ANY_REG_OP(addr_mode_nbytes, addr,
> +						  nor->bouncebuf);
> +		ret = spi_nor_read_any_reg(nor, &op, nor->reg_proto);
> +		if (ret)
> +			return ret;
>  
> -	if (nor->bouncebuf[0] != cfr1v_written) {
> -		dev_err(nor->dev, "CFR1: Read back test failed\n");
> -		return -EIO;
> +		if (nor->bouncebuf[0] != cfr1v_written) {
> +			dev_err(nor->dev, "CFR1: Read back test failed\n");
> +			return -EIO;
> +		}
>  	}
>  
>  	return 0;

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 8/8] mtd: spi-nor: spansion: Add support for Infineon
  2022-08-08  8:31                 ` Takahiro Kuwano
@ 2022-08-12  8:15                   ` Takahiro Kuwano
  0 siblings, 0 replies; 19+ messages in thread
From: Takahiro Kuwano @ 2022-08-12  8:15 UTC (permalink / raw)
  To: Tudor.Ambarus, linux-mtd
  Cc: pratyush, michael, miquel.raynal, richard, vigneshr,
	Bacem.Daassi, Takahiro.Kuwano

Hi Tudor,

I submitted another series about address mode discovery.
Please review that. I will revise this series after another one is settled.

Thanks,
Takahiro

On 8/8/2022 5:31 PM, Takahiro Kuwano wrote:
> On 8/8/2022 5:26 PM, Tudor.Ambarus@microchip.com wrote:
>> On 8/8/22 11:09, Takahiro Kuwano wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>
>>> On 8/8/2022 4:34 PM, Tudor.Ambarus@microchip.com wrote:
>>>> On 8/8/22 09:41, Takahiro Kuwano wrote:
>>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>>>
>>>>> On 8/8/2022 3:08 PM, Tudor.Ambarus@microchip.com wrote:
>>>>>> On 8/8/22 08:42, Takahiro Kuwano wrote:
>>>>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>>>>>
>>>>>>> On 8/8/2022 1:47 PM, Tudor.Ambarus@microchip.com wrote:
>>>>>>>> On 8/6/22 09:34, tkuw584924@gmail.com wrote:
>>>>>>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>>>>>>>
>>>>>>>>> From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
>>>>>>>>
>>>>>>>> Hi!
>>>>>>>>
>>>>>>>>>
>>>>>>>>> s25hl02gt and s25hs02gt
>>>>>>>>>
>>>>>>>>> Add ID, flags, and fixup for s25hl02gt and s25hs02gt.
>>>>>>>>> These parts are
>>>>>>>>>   - Dual-die package parts
>>>>>>>>>   - Not support chip erase
>>>>>>>>>   - 4-byte addressing mode by default
>>>>>>>>
>>>>>>>> CFR2N[7] CFR2V[7] says that: "For the DDP or QDP devices, if ADRBYT = 0
>>>>>>>> only the first 128 Mb of die 1 can be accessed."
>>>>>>>> So there are flashes of the same family that are by default in 3 byte address
>>>>>>>> mode. You added support just for a subset of them and used a generic name,
>>>>>>>> which is not accurate, right?
>>>>>>>>
>>>>>>> We added model #15 (3-byte address mode by default) to address special
>>>>>>> requirement from a customer who needs to use bootrom with 3-byte addressing.
>>>>>>> Anyway, I overlooked model # difference. Thanks for pointing out this.
>>>>>>>
>>>>>>>> Can we instead make an algorithm to determine the current address mode?
>>>>>>>>
>>>>>>> I have just found that we can distinguish model # via BFPT DWORD16.
>>>>>>> If Hardware reset, Software reset, or Power cycle can exit 4-byte address
>>>>>>> mode, that means the device is 3-byte address mode by default.
>>>>>>
>>>>>> I don't think this will help us. It doesn't matter the default mode if you
>>>>>> have a non volatile register that can be updated and changes the default
>>>>>> mode.
>>>>>>
>>>>>> Are there any registers/data that can be read successively in 3 byte addr mode
>>>>>> and then in 4 byte addr mode? We'll then compare what we receive from the flash
>>>>>> with a known value and determine the mode.
>>>>>>
>>>>> As we discussed before [0], if address mode in the controller and device are
>>>>
>>>> I remember, yes, but without determining the mode, the driver will work only
>>>> with flashes that come with the factory settings. The driver will be unusable
>>>> if someone changes the address mode in a non volatile way, right?
>>>>
>>> Yes, right.
>>>
>>>>> different, the read data will be undetermined.
>>>>>
>>>>> But if we really want...
>>>>> Compare SR1 data read by RDSR1(05h - No Addr) and RDAR(65h - Addr 0).
>>>>> In most cases (without block protection), SR1=00h. The value of 00h would be
>>>>> awkward to determine if this is 'real' output from Flash or not. So, use> WREN(06h) and WRDI(04h) that flips BIT(1) in SR1.
>>>>
>>>> Would be good to have more fixed/OPT-like bits, or if we could change more bits
>>>> to increase the chances to not hit just some undetermined data.
>>>>>
>>>>> Therefore, something like:
>>>>> 1) RDSR1
>>>>> 2) RDAR with 3-byte addr (000000h)
>>>>> 3) If #1 == #2
>>>>>         4) WREN
>>>>>         5) RDAR with 3-byte addr (000000h)
>>>>>         6) BIT(1) is SR1==1?
>>>>>         ...
>>>>>
>>>>> Or simply WREN -> RDAR -> WRDI -> RDAR then check if only BIT(1) is toggled.
>>>>
>>>> Both may work, yes, but making the assumption on only one bit is fragile.
>>>> Can we use the Read Any Register Command with 3 and 4 byte address modes and
>>>> compare the values? Are there any registers with fixed values?
>>>>
>>> Register values can vary because it's register:)
>>>
>>> So... let's use Data Integrity Check CRC registers. These registers do not
>>> have fixed values but we can calculate expected values by offline. Read
>>> several bytes (>=4) from Flash array with Read(03h) then calculate CRC by
>>> crc32(). Issue Data integrity Check command (5Bh) followed by start and
>>> end address (4-byte for each), wait till ready. Read calculated CRC by
>>> Read Any Register in 3 and 4 byte address (00800095h~0080098h) then compare
>>> the crc32() result and register read result.
>>
>> Much better, yes. I think it is worth it. What's your opinion, Takahiro?
>> Others, Michael, Pratyush?
>>
> OK, I will prototype and test it.

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2022-08-12  8:15 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-06  6:34 [PATCH 0/8] mtd: spi-nor: Add support for Infineon SEMPER s25hl02gt and s25hs02gt tkuw584924
2022-08-06  6:34 ` [PATCH 1/8] mtd: spi-nor: core: Introduce number of dice and volatile register offset params tkuw584924
2022-08-06  6:34 ` [PATCH 2/8] mtd: spi-nor: sfdp: Extract volatile register offset from SCCR map tkuw584924
2022-08-06  6:34 ` [PATCH 3/8] mtd: spi-nor: sfdp: Add support for SCCR map for multi-chip device tkuw584924
2022-08-06  6:34 ` [PATCH 4/8] mtd: spi-nor: spansion: Rework cypress_nor_set_page_size() for multi-chip device support tkuw584924
2022-08-06  6:34 ` [PATCH 5/8] mtd: spi-nor: spansion: Rework cypress_nor_quad_enable_volatile() " tkuw584924
2022-08-10 14:40   ` Takahiro Kuwano
2022-08-06  6:34 ` [PATCH 6/8] mtd: spi-nor: spansion: Add a new ->ready() hook for multi-chip device tkuw584924
2022-08-06  6:34 ` [PATCH 7/8] mtd: spi-nor: spansion: Introduce DEF_4BAM mfr flag tkuw584924
2022-08-06  6:34 ` [PATCH 8/8] mtd: spi-nor: spansion: Add support for Infineon tkuw584924
2022-08-08  4:47   ` Tudor.Ambarus
2022-08-08  5:42     ` Takahiro Kuwano
2022-08-08  6:08       ` Tudor.Ambarus
2022-08-08  6:41         ` Takahiro Kuwano
2022-08-08  7:34           ` Tudor.Ambarus
2022-08-08  8:09             ` Takahiro Kuwano
2022-08-08  8:26               ` Tudor.Ambarus
2022-08-08  8:31                 ` Takahiro Kuwano
2022-08-12  8:15                   ` Takahiro Kuwano

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).