2022-04-22 04:51:18

by Tudor Ambarus

[permalink] [raw]
Subject: [PATCH v4 00/11] mtd: spi-nor: Rework Octal DTR methods

v4:
- s/spi_nor_read_reg/spi_nor_read_any_reg and
s/spi_nor_write_reg/spi_nor_write_any_volatile_reg to comply with Micron
and Cypress's naming scheme. Update documentation description on
spi_nor_write_any_volatile_reg.
- 2 new patches, the last ones, where I remove status polling on write
volatile registers
- collect R-bs
v3:
- queue patch: "mtd: spi-nor: Introduce templates for SPI NOR operations"
from
https://lore.kernel.org/lkml/[email protected]/
The dependency chain between patches was too long and hard to follow.

- Rework patch "mtd: spi-nor: core: Use auto-detection only once"
according to Pratyush's and Michael's suggestions
- Remove dev_dbg messge from spi_nor_read_id() method and let the callers
use their own message (detection, octal dtr enable/disable)
- detailed version changes in each patch

v2:
- Fix bug on octal dtr disable, s/nor->reg_proto/SNOR_PROTO_1_1_1,
because after disable the nor->proto is not yet updated
- update function/macros names to comply with Michael's rename series.

Tudor Ambarus (11):
mtd: spi-nor: Rename method, s/spi_nor_match_id/spi_nor_match_name
mtd: spi-nor: Introduce spi_nor_match_id()
mtd: spi-nor: core: Use auto-detection only once
mtd: spi-nor: core: Introduce method for RDID op
mtd: spi-nor: manufacturers: Use spi_nor_read_id() core method
mtd: spi-nor: core: Add helpers to read/write any register
mtd: spi-nor: micron-st: Rework spi_nor_micron_octal_dtr_enable()
mtd: spi-nor: spansion: Rework spi_nor_cypress_octal_dtr_enable()
mtd: spi-nor: Introduce templates for SPI NOR operations
mtd: spi-nor: spansion: Remove status polling on volatile registers
write
mtd: spi-nor: micron-st: Remove status polling on volatile registers
write

drivers/mtd/spi-nor/core.c | 254 ++++++++++++++++----------------
drivers/mtd/spi-nor/core.h | 115 +++++++++++++++
drivers/mtd/spi-nor/micron-st.c | 139 ++++++++---------
drivers/mtd/spi-nor/spansion.c | 159 ++++++++++----------
drivers/mtd/spi-nor/xilinx.c | 12 +-
5 files changed, 404 insertions(+), 275 deletions(-)

--
2.25.1


2022-04-22 09:44:57

by Tudor Ambarus

[permalink] [raw]
Subject: [PATCH v4 05/11] mtd: spi-nor: manufacturers: Use spi_nor_read_id() core method

Use spi_nor_read_id() core method to avoid duplication of code. Now the ID
is read on the full SPI_NOR_MAX_ID_LEN instead of
round_up(nor->info->id_len, 2), but it doesn't harm to read more ID bytes,
so the change comes with no secondary effects. dev_dbg messages in case
spi_nor_read_id() fails, will be added in a further patch after we split
the octal DTR enable/disable methods.

Signed-off-by: Tudor Ambarus <[email protected]>
Reviewed-by: Pratyush Yadav <[email protected]>
Reviewed-by: Michael Walle <[email protected]>
---
drivers/mtd/spi-nor/micron-st.c | 13 +++----------
drivers/mtd/spi-nor/spansion.c | 13 +++----------
2 files changed, 6 insertions(+), 20 deletions(-)

diff --git a/drivers/mtd/spi-nor/micron-st.c b/drivers/mtd/spi-nor/micron-st.c
index 8a20475ce77a..41b87868ecf9 100644
--- a/drivers/mtd/spi-nor/micron-st.c
+++ b/drivers/mtd/spi-nor/micron-st.c
@@ -91,17 +91,10 @@ static int micron_st_nor_octal_dtr_enable(struct spi_nor *nor, bool enable)
return ret;

/* Read flash ID to make sure the switch was successful. */
- op = (struct spi_mem_op)
- SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_RDID, 1),
- SPI_MEM_OP_NO_ADDR,
- SPI_MEM_OP_DUMMY(enable ? 8 : 0, 1),
- SPI_MEM_OP_DATA_IN(round_up(nor->info->id_len, 2),
- buf, 1));
-
if (enable)
- spi_nor_spimem_setup_op(nor, &op, SNOR_PROTO_8_8_8_DTR);
-
- ret = spi_mem_exec_op(nor->spimem, &op);
+ ret = spi_nor_read_id(nor, 0, 8, buf, SNOR_PROTO_8_8_8_DTR);
+ else
+ ret = spi_nor_read_id(nor, 0, 0, buf, SNOR_PROTO_1_1_1);
if (ret)
return ret;

diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
index f24e546e04a5..c5988312cc91 100644
--- a/drivers/mtd/spi-nor/spansion.c
+++ b/drivers/mtd/spi-nor/spansion.c
@@ -98,17 +98,10 @@ static int cypress_nor_octal_dtr_enable(struct spi_nor *nor, bool enable)
return ret;

/* Read flash ID to make sure the switch was successful. */
- op = (struct spi_mem_op)
- SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_RDID, 1),
- SPI_MEM_OP_ADDR(enable ? 4 : 0, 0, 1),
- SPI_MEM_OP_DUMMY(enable ? 3 : 0, 1),
- SPI_MEM_OP_DATA_IN(round_up(nor->info->id_len, 2),
- buf, 1));
-
if (enable)
- spi_nor_spimem_setup_op(nor, &op, SNOR_PROTO_8_8_8_DTR);
-
- ret = spi_mem_exec_op(nor->spimem, &op);
+ ret = spi_nor_read_id(nor, 4, 3, buf, SNOR_PROTO_8_8_8_DTR);
+ else
+ ret = spi_nor_read_id(nor, 0, 0, buf, SNOR_PROTO_1_1_1);
if (ret)
return ret;

--
2.25.1

2022-04-22 18:52:30

by Tudor Ambarus

[permalink] [raw]
Subject: [PATCH v4 07/11] mtd: spi-nor: micron-st: Rework spi_nor_micron_octal_dtr_enable()

Introduce template operation to remove code duplication.
Split spi_nor_micron_octal_dtr_enable() in spi_nor_micron_octal_dtr_en()
and spi_nor_micron_octal_dtr_dis() as it no longer made sense to try to
keep everything alltogether: too many "if (enable)" throughout the code,
which made the code difficult to follow. Add dev_dbg messages in case
spi_nor_read_id() fails.

Signed-off-by: Tudor Ambarus <[email protected]>
Reviewed-by: Pratyush Yadav <[email protected]>
Reviewed-by: Michael Walle <[email protected]>
---
drivers/mtd/spi-nor/micron-st.c | 111 +++++++++++++++++---------------
1 file changed, 60 insertions(+), 51 deletions(-)

diff --git a/drivers/mtd/spi-nor/micron-st.c b/drivers/mtd/spi-nor/micron-st.c
index 41b87868ecf9..ce62e6be8fd2 100644
--- a/drivers/mtd/spi-nor/micron-st.c
+++ b/drivers/mtd/spi-nor/micron-st.c
@@ -28,75 +28,78 @@
#define FSR_P_ERR BIT(4) /* Program operation status */
#define FSR_PT_ERR BIT(1) /* Protection error bit */

-static int micron_st_nor_octal_dtr_enable(struct spi_nor *nor, bool enable)
+/* Micron ST SPI NOR flash operations. */
+#define MICRON_ST_NOR_WR_ANY_REG_OP(naddr, addr, ndata, buf) \
+ SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_MT_WR_ANY_REG, 0), \
+ SPI_MEM_OP_ADDR(naddr, addr, 0), \
+ SPI_MEM_OP_NO_DUMMY, \
+ SPI_MEM_OP_DATA_OUT(ndata, buf, 0))
+
+static int micron_st_nor_octal_dtr_en(struct spi_nor *nor)
{
struct spi_mem_op op;
u8 *buf = nor->bouncebuf;
int ret;

- if (enable) {
- /* Use 20 dummy cycles for memory array reads. */
- ret = spi_nor_write_enable(nor);
- if (ret)
- return ret;
-
- *buf = 20;
- op = (struct spi_mem_op)
- SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_MT_WR_ANY_REG, 1),
- SPI_MEM_OP_ADDR(3, SPINOR_REG_MT_CFR1V, 1),
- SPI_MEM_OP_NO_DUMMY,
- SPI_MEM_OP_DATA_OUT(1, buf, 1));
-
- ret = spi_mem_exec_op(nor->spimem, &op);
- if (ret)
- return ret;
-
- ret = spi_nor_wait_till_ready(nor);
- if (ret)
- return ret;
- }
+ /* Use 20 dummy cycles for memory array reads. */
+ *buf = 20;
+ op = (struct spi_mem_op)
+ MICRON_ST_NOR_WR_ANY_REG_OP(3, SPINOR_REG_MT_CFR1V, 1, buf);
+ ret = spi_nor_write_any_volatile_reg(nor, &op, nor->reg_proto);
+ if (ret)
+ return ret;
+ ret = spi_nor_wait_till_ready(nor);
+ if (ret)
+ return ret;

- ret = spi_nor_write_enable(nor);
+ buf[0] = SPINOR_MT_OCT_DTR;
+ op = (struct spi_mem_op)
+ MICRON_ST_NOR_WR_ANY_REG_OP(3, SPINOR_REG_MT_CFR0V, 1, buf);
+ ret = spi_nor_write_any_volatile_reg(nor, &op, nor->reg_proto);
if (ret)
return ret;

- if (enable) {
- buf[0] = SPINOR_MT_OCT_DTR;
- } else {
- /*
- * The register is 1-byte wide, but 1-byte transactions are not
- * allowed in 8D-8D-8D mode. The next register is the dummy
- * cycle configuration register. Since the transaction needs to
- * be at least 2 bytes wide, set the next register to its
- * default value. This also makes sense because the value was
- * changed when enabling 8D-8D-8D mode, it should be reset when
- * disabling.
- */
- buf[0] = SPINOR_MT_EXSPI;
- buf[1] = SPINOR_REG_MT_CFR1V_DEF;
+ /* Read flash ID to make sure the switch was successful. */
+ ret = spi_nor_read_id(nor, 0, 8, buf, SNOR_PROTO_8_8_8_DTR);
+ if (ret) {
+ dev_dbg(nor->dev, "error %d reading JEDEC ID after enabling 8D-8D-8D mode\n", ret);
+ return ret;
}

- op = (struct spi_mem_op)
- SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_MT_WR_ANY_REG, 1),
- SPI_MEM_OP_ADDR(enable ? 3 : 4,
- SPINOR_REG_MT_CFR0V, 1),
- SPI_MEM_OP_NO_DUMMY,
- SPI_MEM_OP_DATA_OUT(enable ? 1 : 2, buf, 1));
+ if (memcmp(buf, nor->info->id, nor->info->id_len))
+ return -EINVAL;

- if (!enable)
- spi_nor_spimem_setup_op(nor, &op, SNOR_PROTO_8_8_8_DTR);
+ return 0;
+}
+
+static int micron_st_nor_octal_dtr_dis(struct spi_nor *nor)
+{
+ struct spi_mem_op op;
+ u8 *buf = nor->bouncebuf;
+ int ret;

- ret = spi_mem_exec_op(nor->spimem, &op);
+ /*
+ * The register is 1-byte wide, but 1-byte transactions are not allowed
+ * in 8D-8D-8D mode. The next register is the dummy cycle configuration
+ * register. Since the transaction needs to be at least 2 bytes wide,
+ * set the next register to its default value. This also makes sense
+ * because the value was changed when enabling 8D-8D-8D mode, it should
+ * be reset when disabling.
+ */
+ buf[0] = SPINOR_MT_EXSPI;
+ buf[1] = SPINOR_REG_MT_CFR1V_DEF;
+ op = (struct spi_mem_op)
+ MICRON_ST_NOR_WR_ANY_REG_OP(4, SPINOR_REG_MT_CFR0V, 2, buf);
+ ret = spi_nor_write_any_volatile_reg(nor, &op, SNOR_PROTO_8_8_8_DTR);
if (ret)
return ret;

/* Read flash ID to make sure the switch was successful. */
- if (enable)
- ret = spi_nor_read_id(nor, 0, 8, buf, SNOR_PROTO_8_8_8_DTR);
- else
- ret = spi_nor_read_id(nor, 0, 0, buf, SNOR_PROTO_1_1_1);
- if (ret)
+ ret = spi_nor_read_id(nor, 0, 0, buf, SNOR_PROTO_1_1_1);
+ if (ret) {
+ dev_dbg(nor->dev, "error %d reading JEDEC ID after disabling 8D-8D-8D mode\n", ret);
return ret;
+ }

if (memcmp(buf, nor->info->id, nor->info->id_len))
return -EINVAL;
@@ -104,6 +107,12 @@ static int micron_st_nor_octal_dtr_enable(struct spi_nor *nor, bool enable)
return 0;
}

+static int micron_st_nor_octal_dtr_enable(struct spi_nor *nor, bool enable)
+{
+ return enable ? micron_st_nor_octal_dtr_en(nor) :
+ micron_st_nor_octal_dtr_dis(nor);
+}
+
static void mt35xu512aba_default_init(struct spi_nor *nor)
{
nor->params->octal_dtr_enable = micron_st_nor_octal_dtr_enable;
--
2.25.1

2022-04-22 18:57:12

by Tudor Ambarus

[permalink] [raw]
Subject: [PATCH v4 08/11] mtd: spi-nor: spansion: Rework spi_nor_cypress_octal_dtr_enable()

Introduce template operation to remove code duplication.
Split spi_nor_cypress_octal_dtr_enable() in
spi_nor_cypress_octal_dtr_ena() spi_nor_cypress_octal_dtr_dis() as it no
longer made sense to try to keep everything alltogether: too many
"if (enable)" throughout the code, which made the code difficult to read.
Add debug messages in case spi_nor_read_id() fails.

Signed-off-by: Tudor Ambarus <[email protected]>
Reviewed-by: Pratyush Yadav <[email protected]>
Reviewed-by: Michael Walle <[email protected]>
---
drivers/mtd/spi-nor/spansion.c | 128 ++++++++++++++++++---------------
1 file changed, 69 insertions(+), 59 deletions(-)

diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
index c5988312cc91..56b43074ef17 100644
--- a/drivers/mtd/spi-nor/spansion.c
+++ b/drivers/mtd/spi-nor/spansion.c
@@ -23,87 +23,81 @@
#define SPINOR_REG_CYPRESS_CFR5V_OCT_DTR_DS 0
#define SPINOR_OP_CYPRESS_RD_FAST 0xee

-/**
- * cypress_nor_octal_dtr_enable() - Enable octal DTR on Cypress flashes.
- * @nor: pointer to a 'struct spi_nor'
- * @enable: whether to enable or disable Octal DTR
- *
- * This also sets the memory access latency cycles to 24 to allow the flash to
- * run at up to 200MHz.
- *
- * Return: 0 on success, -errno otherwise.
- */
-static int cypress_nor_octal_dtr_enable(struct spi_nor *nor, bool enable)
+/* Cypress SPI NOR flash operations. */
+#define CYPRESS_NOR_WR_ANY_REG_OP(naddr, addr, ndata, buf) \
+ SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WR_ANY_REG, 0), \
+ SPI_MEM_OP_ADDR(naddr, addr, 0), \
+ SPI_MEM_OP_NO_DUMMY, \
+ SPI_MEM_OP_DATA_OUT(ndata, buf, 0))
+
+static int cypress_nor_octal_dtr_en(struct spi_nor *nor)
{
struct spi_mem_op op;
u8 *buf = nor->bouncebuf;
int ret;

- if (enable) {
- /* Use 24 dummy cycles for memory array reads. */
- ret = spi_nor_write_enable(nor);
- if (ret)
- return ret;
+ /* Use 24 dummy cycles for memory array reads. */
+ *buf = SPINOR_REG_CYPRESS_CFR2V_MEMLAT_11_24;
+ op = (struct spi_mem_op)
+ CYPRESS_NOR_WR_ANY_REG_OP(3, SPINOR_REG_CYPRESS_CFR2V, 1, buf);

- *buf = SPINOR_REG_CYPRESS_CFR2V_MEMLAT_11_24;
- op = (struct spi_mem_op)
- SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WR_ANY_REG, 1),
- SPI_MEM_OP_ADDR(3, SPINOR_REG_CYPRESS_CFR2V,
- 1),
- SPI_MEM_OP_NO_DUMMY,
- SPI_MEM_OP_DATA_OUT(1, buf, 1));
+ ret = spi_nor_write_any_volatile_reg(nor, &op, nor->reg_proto);
+ if (ret)
+ return ret;

- ret = spi_mem_exec_op(nor->spimem, &op);
- if (ret)
- return ret;
+ ret = spi_nor_wait_till_ready(nor);
+ if (ret)
+ return ret;

- ret = spi_nor_wait_till_ready(nor);
- if (ret)
- return ret;
+ nor->read_dummy = 24;

- nor->read_dummy = 24;
- }
+ /* Set the octal and DTR enable bits. */
+ buf[0] = SPINOR_REG_CYPRESS_CFR5V_OCT_DTR_EN;
+ op = (struct spi_mem_op)
+ CYPRESS_NOR_WR_ANY_REG_OP(3, SPINOR_REG_CYPRESS_CFR5V, 1, buf);

- /* Set/unset the octal and DTR enable bits. */
- ret = spi_nor_write_enable(nor);
+ ret = spi_nor_write_any_volatile_reg(nor, &op, nor->reg_proto);
if (ret)
return ret;

- if (enable) {
- buf[0] = SPINOR_REG_CYPRESS_CFR5V_OCT_DTR_EN;
- } else {
- /*
- * The register is 1-byte wide, but 1-byte transactions are not
- * allowed in 8D-8D-8D mode. Since there is no register at the
- * next location, just initialize the value to 0 and let the
- * transaction go on.
- */
- buf[0] = SPINOR_REG_CYPRESS_CFR5V_OCT_DTR_DS;
- buf[1] = 0;
+ /* Read flash ID to make sure the switch was successful. */
+ ret = spi_nor_read_id(nor, 4, 3, buf, SNOR_PROTO_8_8_8_DTR);
+ if (ret) {
+ dev_dbg(nor->dev, "error %d reading JEDEC ID after enabling 8D-8D-8D mode\n", ret);
+ return ret;
}

- op = (struct spi_mem_op)
- SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WR_ANY_REG, 1),
- SPI_MEM_OP_ADDR(enable ? 3 : 4,
- SPINOR_REG_CYPRESS_CFR5V,
- 1),
- SPI_MEM_OP_NO_DUMMY,
- SPI_MEM_OP_DATA_OUT(enable ? 1 : 2, buf, 1));
+ if (memcmp(buf, nor->info->id, nor->info->id_len))
+ return -EINVAL;

- if (!enable)
- spi_nor_spimem_setup_op(nor, &op, SNOR_PROTO_8_8_8_DTR);
+ return 0;
+}

- ret = spi_mem_exec_op(nor->spimem, &op);
+static int cypress_nor_octal_dtr_dis(struct spi_nor *nor)
+{
+ struct spi_mem_op op;
+ u8 *buf = nor->bouncebuf;
+ int ret;
+
+ /*
+ * The register is 1-byte wide, but 1-byte transactions are not allowed
+ * in 8D-8D-8D mode. Since there is no register at the next location,
+ * just initialize the value to 0 and let the transaction go on.
+ */
+ buf[0] = SPINOR_REG_CYPRESS_CFR5V_OCT_DTR_DS;
+ buf[1] = 0;
+ op = (struct spi_mem_op)
+ CYPRESS_NOR_WR_ANY_REG_OP(4, SPINOR_REG_CYPRESS_CFR5V, 2, buf);
+ ret = spi_nor_write_any_volatile_reg(nor, &op, SNOR_PROTO_8_8_8_DTR);
if (ret)
return ret;

/* Read flash ID to make sure the switch was successful. */
- if (enable)
- ret = spi_nor_read_id(nor, 4, 3, buf, SNOR_PROTO_8_8_8_DTR);
- else
- ret = spi_nor_read_id(nor, 0, 0, buf, SNOR_PROTO_1_1_1);
- if (ret)
+ ret = spi_nor_read_id(nor, 0, 0, buf, SNOR_PROTO_1_1_1);
+ if (ret) {
+ dev_dbg(nor->dev, "error %d reading JEDEC ID after disabling 8D-8D-8D mode\n", ret);
return ret;
+ }

if (memcmp(buf, nor->info->id, nor->info->id_len))
return -EINVAL;
@@ -111,6 +105,22 @@ static int cypress_nor_octal_dtr_enable(struct spi_nor *nor, bool enable)
return 0;
}

+/**
+ * cypress_nor_octal_dtr_enable() - Enable octal DTR on Cypress flashes.
+ * @nor: pointer to a 'struct spi_nor'
+ * @enable: whether to enable or disable Octal DTR
+ *
+ * This also sets the memory access latency cycles to 24 to allow the flash to
+ * run at up to 200MHz.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int cypress_nor_octal_dtr_enable(struct spi_nor *nor, bool enable)
+{
+ return enable ? cypress_nor_octal_dtr_en(nor) :
+ cypress_nor_octal_dtr_dis(nor);
+}
+
static void s28hs512t_default_init(struct spi_nor *nor)
{
nor->params->octal_dtr_enable = cypress_nor_octal_dtr_enable;
--
2.25.1

2022-04-22 19:35:26

by Tudor Ambarus

[permalink] [raw]
Subject: [PATCH v4 04/11] mtd: spi-nor: core: Introduce method for RDID op

RDID is used in the core to auto detect the flash, but also by some
manufacturer drivers that contain flashes that support Octal DTR mode,
so that they can read the flash ID after the switch to Octal DTR was made
to test if the switch was successful. Introduce a core method for RDID op
to avoid code duplication.

Signed-off-by: Tudor Ambarus <[email protected]>
Reviewed-by: Michael Walle <[email protected]>
---
drivers/mtd/spi-nor/core.c | 50 ++++++++++++++++++++++++++------------
drivers/mtd/spi-nor/core.h | 9 +++++++
2 files changed, 44 insertions(+), 15 deletions(-)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index b55d922d46dd..6165dc7bfd17 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -369,6 +369,37 @@ int spi_nor_write_disable(struct spi_nor *nor)
return ret;
}

+/**
+ * spi_nor_read_id() - Read the JEDEC ID.
+ * @nor: pointer to 'struct spi_nor'.
+ * @naddr: number of address bytes to send. Can be zero if the operation
+ * does not need to send an address.
+ * @ndummy: number of dummy bytes to send after an opcode or address. Can
+ * be zero if the operation does not require dummy bytes.
+ * @id: pointer to a DMA-able buffer where the value of the JEDEC ID
+ * will be written.
+ * @proto: the SPI protocol for register operation.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+int spi_nor_read_id(struct spi_nor *nor, u8 naddr, u8 ndummy, u8 *id,
+ enum spi_nor_protocol proto)
+{
+ int ret;
+
+ if (nor->spimem) {
+ struct spi_mem_op op =
+ SPI_NOR_READID_OP(naddr, ndummy, id, SPI_NOR_MAX_ID_LEN);
+
+ spi_nor_spimem_setup_op(nor, &op, proto);
+ ret = spi_mem_exec_op(nor->spimem, &op);
+ } else {
+ ret = nor->controller_ops->read_reg(nor, SPINOR_OP_RDID, id,
+ SPI_NOR_MAX_ID_LEN);
+ }
+ return ret;
+}
+
/**
* spi_nor_read_sr() - Read the Status Register.
* @nor: pointer to 'struct spi_nor'.
@@ -1649,24 +1680,13 @@ static const struct flash_info *spi_nor_match_id(struct spi_nor *nor,
return NULL;
}

-static const struct flash_info *spi_nor_read_id(struct spi_nor *nor)
+static const struct flash_info *spi_nor_detect(struct spi_nor *nor)
{
const struct flash_info *info;
u8 *id = nor->bouncebuf;
int ret;

- if (nor->spimem) {
- struct spi_mem_op op =
- SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_RDID, 1),
- SPI_MEM_OP_NO_ADDR,
- SPI_MEM_OP_NO_DUMMY,
- SPI_MEM_OP_DATA_IN(SPI_NOR_MAX_ID_LEN, id, 1));
-
- ret = spi_mem_exec_op(nor->spimem, &op);
- } else {
- ret = nor->controller_ops->read_reg(nor, SPINOR_OP_RDID, id,
- SPI_NOR_MAX_ID_LEN);
- }
+ ret = spi_nor_read_id(nor, 0, 0, id, nor->reg_proto);
if (ret) {
dev_dbg(nor->dev, "error %d reading JEDEC ID\n", ret);
return ERR_PTR(ret);
@@ -2903,7 +2923,7 @@ static const struct flash_info *spi_nor_get_flash_info(struct spi_nor *nor,
}
/* Try to auto-detect if chip name wasn't specified or not found */
if (!info)
- return spi_nor_read_id(nor);
+ return spi_nor_detect(nor);

/*
* If caller has specified name of flash model that can normally be
@@ -2912,7 +2932,7 @@ static const struct flash_info *spi_nor_get_flash_info(struct spi_nor *nor,
if (name && info->id_len) {
const struct flash_info *jinfo;

- jinfo = spi_nor_read_id(nor);
+ jinfo = spi_nor_detect(nor);
if (IS_ERR(jinfo)) {
return jinfo;
} else if (jinfo != info) {
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index b7fd760e3b47..f952061d5c24 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -11,6 +11,13 @@

#define SPI_NOR_MAX_ID_LEN 6

+/* Standard SPI NOR flash operations. */
+#define SPI_NOR_READID_OP(naddr, ndummy, buf, len) \
+ SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_RDID, 0), \
+ SPI_MEM_OP_ADDR(naddr, 0, 0), \
+ SPI_MEM_OP_DUMMY(ndummy, 0), \
+ SPI_MEM_OP_DATA_IN(len, buf, 0))
+
enum spi_nor_option_flags {
SNOR_F_HAS_SR_TB = BIT(0),
SNOR_F_NO_OP_CHIP_ERASE = BIT(1),
@@ -534,6 +541,8 @@ void spi_nor_unlock_and_unprep(struct spi_nor *nor);
int spi_nor_sr1_bit6_quad_enable(struct spi_nor *nor);
int spi_nor_sr2_bit1_quad_enable(struct spi_nor *nor);
int spi_nor_sr2_bit7_quad_enable(struct spi_nor *nor);
+int spi_nor_read_id(struct spi_nor *nor, u8 naddr, u8 ndummy, u8 *id,
+ enum spi_nor_protocol reg_proto);
int spi_nor_read_sr(struct spi_nor *nor, u8 *sr);
int spi_nor_sr_ready(struct spi_nor *nor);
int spi_nor_read_cr(struct spi_nor *nor, u8 *cr);
--
2.25.1

2022-04-22 21:34:16

by Tudor Ambarus

[permalink] [raw]
Subject: [PATCH v4 06/11] mtd: spi-nor: core: Add helpers to read/write any register

There are manufacturers that use registers indexed by address. Some of
them support "read/write any register" opcodes. Provide core methods that
can be used by all manufacturers. SPI NOR controller ops are intentionally
not supported as we intend to move all the SPI NOR controller drivers
under the SPI subsystem.

Signed-off-by: Tudor Ambarus <[email protected]>
Tested-by: Takahiro Kuwano <[email protected]>
Reviewed-by: Pratyush Yadav <[email protected]>
Reviewed-by: Michael Walle <[email protected]>
---
drivers/mtd/spi-nor/core.c | 46 ++++++++++++++++++++++++++++++++++++++
drivers/mtd/spi-nor/core.h | 4 ++++
2 files changed, 50 insertions(+)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 6165dc7bfd17..08bf58e5dbd1 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -307,6 +307,52 @@ ssize_t spi_nor_write_data(struct spi_nor *nor, loff_t to, size_t len,
return nor->controller_ops->write(nor, to, len, buf);
}

+/**
+ * spi_nor_read_any_reg() - read any register from flash memory, nonvolatile or
+ * volatile.
+ * @nor: pointer to 'struct spi_nor'.
+ * @op: SPI memory operation. op->data.buf must be DMA-able.
+ * @proto: SPI protocol to use for the register operation.
+ *
+ * Return: zero on success, -errno otherwise
+ */
+int spi_nor_read_any_reg(struct spi_nor *nor, struct spi_mem_op *op,
+ enum spi_nor_protocol proto)
+{
+ if (!nor->spimem)
+ return -EOPNOTSUPP;
+
+ spi_nor_spimem_setup_op(nor, op, proto);
+ return spi_nor_spimem_exec_op(nor, op);
+}
+
+/**
+ * spi_nor_write_any_volatile_reg() - write any volatile register to flash
+ * memory.
+ * @nor: pointer to 'struct spi_nor'
+ * @op: SPI memory operation. op->data.buf must be DMA-able.
+ * @proto: SPI protocol to use for the register operation.
+ *
+ * Writing volatile registers are instant according to some manufacturers
+ * (Cypress, Micron) and do not need any status polling.
+ *
+ * Return: zero on success, -errno otherwise
+ */
+int spi_nor_write_any_volatile_reg(struct spi_nor *nor, struct spi_mem_op *op,
+ enum spi_nor_protocol proto)
+{
+ int ret;
+
+ if (!nor->spimem)
+ return -EOPNOTSUPP;
+
+ ret = spi_nor_write_enable(nor);
+ if (ret)
+ return ret;
+ spi_nor_spimem_setup_op(nor, op, proto);
+ return spi_nor_spimem_exec_op(nor, op);
+}
+
/**
* spi_nor_write_enable() - Set write enable latch with Write Enable command.
* @nor: pointer to 'struct spi_nor'.
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index f952061d5c24..62ddadba0c33 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -554,6 +554,10 @@ ssize_t spi_nor_read_data(struct spi_nor *nor, loff_t from, size_t len,
u8 *buf);
ssize_t spi_nor_write_data(struct spi_nor *nor, loff_t to, size_t len,
const u8 *buf);
+int spi_nor_read_any_reg(struct spi_nor *nor, struct spi_mem_op *op,
+ enum spi_nor_protocol proto);
+int spi_nor_write_any_volatile_reg(struct spi_nor *nor, struct spi_mem_op *op,
+ enum spi_nor_protocol proto);
int spi_nor_erase_sector(struct spi_nor *nor, u32 addr);

int spi_nor_otp_read_secr(struct spi_nor *nor, loff_t addr, size_t len, u8 *buf);
--
2.25.1

2022-04-22 22:18:28

by Tudor Ambarus

[permalink] [raw]
Subject: [PATCH v4 10/11] mtd: spi-nor: spansion: Remove status polling on volatile registers write

Writing volatile registers are instant according to Cypress and do not
need any status polling. Remove status polling on volatile registers write.

Signed-off-by: Tudor Ambarus <[email protected]>
---
drivers/mtd/spi-nor/spansion.c | 4 ----
1 file changed, 4 deletions(-)

diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
index 7404ca067ca9..43cd6cd92537 100644
--- a/drivers/mtd/spi-nor/spansion.c
+++ b/drivers/mtd/spi-nor/spansion.c
@@ -57,10 +57,6 @@ static int cypress_nor_octal_dtr_en(struct spi_nor *nor)
if (ret)
return ret;

- ret = spi_nor_wait_till_ready(nor);
- if (ret)
- return ret;
-
nor->read_dummy = 24;

/* Set the octal and DTR enable bits. */
--
2.25.1

2022-04-22 22:30:56

by Tudor Ambarus

[permalink] [raw]
Subject: [PATCH v4 02/11] mtd: spi-nor: Introduce spi_nor_match_id()

Similar to spi_nor_match_name() extend the search of flash_info through
all the manufacturers, this time doing the match by ID. There's no reason
to limit the search per manufacturer yet, do it globally, search the flash
in all the parts of all manufacturers in a single method.

Signed-off-by: Tudor Ambarus <[email protected]>
Reviewed-by: Pratyush Yadav <[email protected]>
Reviewed-by: Michael Walle <[email protected]>
---
drivers/mtd/spi-nor/core.c | 40 ++++++++++++++++++--------------------
1 file changed, 19 insertions(+), 21 deletions(-)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 214d3a1ac6b0..b9cc8bbf1f62 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -1629,16 +1629,21 @@ static const struct spi_nor_manufacturer *manufacturers[] = {
&spi_nor_xmc,
};

-static const struct flash_info *
-spi_nor_search_part_by_id(const struct flash_info *parts, unsigned int nparts,
- const u8 *id)
+static const struct flash_info *spi_nor_match_id(struct spi_nor *nor,
+ const u8 *id)
{
- unsigned int i;
+ const struct flash_info *part;
+ unsigned int i, j;

- for (i = 0; i < nparts; i++) {
- if (parts[i].id_len &&
- !memcmp(parts[i].id, id, parts[i].id_len))
- return &parts[i];
+ for (i = 0; i < ARRAY_SIZE(manufacturers); i++) {
+ for (j = 0; j < manufacturers[i]->nparts; j++) {
+ part = &manufacturers[i]->parts[j];
+ if (part->id_len &&
+ !memcmp(part->id, id, part->id_len)) {
+ nor->manufacturer = manufacturers[i];
+ return part;
+ }
+ }
}

return NULL;
@@ -1648,7 +1653,6 @@ static const struct flash_info *spi_nor_read_id(struct spi_nor *nor)
{
const struct flash_info *info;
u8 *id = nor->bouncebuf;
- unsigned int i;
int ret;

if (nor->spimem) {
@@ -1668,19 +1672,13 @@ static const struct flash_info *spi_nor_read_id(struct spi_nor *nor)
return ERR_PTR(ret);
}

- for (i = 0; i < ARRAY_SIZE(manufacturers); i++) {
- info = spi_nor_search_part_by_id(manufacturers[i]->parts,
- manufacturers[i]->nparts,
- id);
- if (info) {
- nor->manufacturer = manufacturers[i];
- return info;
- }
+ info = spi_nor_match_id(nor, id);
+ if (!info) {
+ dev_err(nor->dev, "unrecognized JEDEC id bytes: %*ph\n",
+ SPI_NOR_MAX_ID_LEN, id);
+ return ERR_PTR(-ENODEV);
}
-
- dev_err(nor->dev, "unrecognized JEDEC id bytes: %*ph\n",
- SPI_NOR_MAX_ID_LEN, id);
- return ERR_PTR(-ENODEV);
+ return info;
}

static int spi_nor_read(struct mtd_info *mtd, loff_t from, size_t len,
--
2.25.1

2022-04-27 11:54:30

by Pratyush Yadav

[permalink] [raw]
Subject: Re: [PATCH v4 00/11] mtd: spi-nor: Rework Octal DTR methods

On Wed, 20 Apr 2022 13:34:16 +0300, Tudor Ambarus wrote:
> v4:
> - s/spi_nor_read_reg/spi_nor_read_any_reg and
> s/spi_nor_write_reg/spi_nor_write_any_volatile_reg to comply with Micron
> and Cypress's naming scheme. Update documentation description on
> spi_nor_write_any_volatile_reg.
> - 2 new patches, the last ones, where I remove status polling on write
> volatile registers
> - collect R-bs
> v3:
> - queue patch: "mtd: spi-nor: Introduce templates for SPI NOR operations"
> from
> https://lore.kernel.org/lkml/[email protected]/
> The dependency chain between patches was too long and hard to follow.
>
> [...]

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git spi-nor/next, thanks!
[01/11] mtd: spi-nor: Rename method, s/spi_nor_match_id/spi_nor_match_name
https://git.kernel.org/mtd/c/b1145d6f1e
[02/11] mtd: spi-nor: Introduce spi_nor_match_id()
https://git.kernel.org/mtd/c/d0ddd88438
[03/11] mtd: spi-nor: core: Use auto-detection only once
https://git.kernel.org/mtd/c/bffabd1c72
[04/11] mtd: spi-nor: core: Introduce method for RDID op
https://git.kernel.org/mtd/c/86b6b55ffb
[05/11] mtd: spi-nor: manufacturers: Use spi_nor_read_id() core method
https://git.kernel.org/mtd/c/a007d81aa5
[06/11] mtd: spi-nor: core: Add helpers to read/write any register
https://git.kernel.org/mtd/c/a604ab33cb
[07/11] mtd: spi-nor: micron-st: Rework spi_nor_micron_octal_dtr_enable()
https://git.kernel.org/mtd/c/4629adaff7
[08/11] mtd: spi-nor: spansion: Rework spi_nor_cypress_octal_dtr_enable()
https://git.kernel.org/mtd/c/27ff0d34fb
[09/11] mtd: spi-nor: Introduce templates for SPI NOR operations
https://git.kernel.org/mtd/c/c0abb861c5
[10/11] mtd: spi-nor: spansion: Remove status polling on volatile registers write
https://git.kernel.org/mtd/c/467f0e8381
[11/11] mtd: spi-nor: micron-st: Remove status polling on volatile registers write
https://git.kernel.org/mtd/c/37841975b3

--
Regards,
Pratyush Yadav
Texas Instruments Inc.