* [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* 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
* [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 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