2020-04-24 18:46:35

by Pratyush Yadav

[permalink] [raw]
Subject: [PATCH v4 00/16] mtd: spi-nor: add xSPI Octal DTR support

Hi,

This series adds support for octal DTR flashes in the spi-nor framework,
and then adds hooks for the Cypress Semper and Mircom Xcella flashes to
allow running them in octal DTR mode. This series assumes that the flash is handed to the kernel in Legacy SPI
mode.

Tested on TI J721e EVM with 1-bit ECC on the Cypress flash.

Changes in v4:
- Refactor the series to use the new spi-nor framework with the
manufacturer-specific bits separated from the core.

- Add support for Micron MT35XU512ABA.

- Use cmd.nbytes as the criteria of whether the data phase exists or not
instead of cmd.buf.in || cmd.buf.out in spi_nor_spimem_setup_op().

- Update Read FSR to use the same dummy cycles and address width as Read
SR.

- Fix BFPT parsing stopping too early for JESD216 rev B flashes.

- Use 2 byte reads for Read SR and FSR commands in DTR mode.

Changes in v3:
- Drop the DT properties "spi-rx-dtr" and "spi-tx-dtr". Instead, if
later a need is felt to disable DTR in case someone has a board with
Octal DTR capable flash but does not support DTR transactions for some
reason, a property like "spi-no-dtr" can be added.

- Remove mode bits SPI_RX_DTR and SPI_TX_DTR.

- Remove the Cadence Quadspi controller patch to un-block this series. I
will submit it as a separate patch.

- Rebase on latest 'master' and fix merge conflicts.

- Update read and write dirmap templates to use DTR.

- Rename 'is_dtr' to 'dtr'.

- Make 'dtr' a bitfield.

- Reject DTR ops in spi_mem_default_supports_op().

- Update atmel-quadspi to reject DTR ops. All other controller drivers
call spi_mem_default_supports_op() so they will automatically reject
DTR ops.

- Add support for both enabling and disabling DTR modes.

- Perform a Software Reset on flashes that support it when shutting
down.

- Disable Octal DTR mode on suspend, and re-enable it on resume.

- Drop enum 'spi_mem_cmd_ext' and make command opcode u16 instead.
Update spi-nor to use the 2-byte command instead of the command
extension. Since we still need a "extension type", mode that enum to
spi-nor and name it 'spi_nor_cmd_ext'.

- Default variable address width to 3 to fix SMPT parsing.

- Drop non-volatile change to uniform sector mode and rely on parsing
SMPT.

Changes in v2:
- Add DT properties "spi-rx-dtr" and "spi-tx-dtr" to allow expressing
DTR capabilities.

- Set the mode bits SPI_RX_DTR and SPI_TX_DTR when we discover the DT
properties "spi-rx-dtr" and spi-tx-dtr".

- spi_nor_cypress_octal_enable() was updating nor->params.read[] with
the intention of setting the correct number of dummy cycles. But this
function is called _after_ selecting the read so setting
nor->params.read[] will have no effect. So, update nor->read_dummy
directly.

- Fix spi_nor_spimem_check_readop() and spi_nor_spimem_check_pp()
passing nor->read_proto and nor->write_proto to
spi_nor_spimem_setup_op() instead of read->proto and pp->proto
respectively.

- Move the call to cqspi_setup_opcode_ext() inside cqspi_enable_dtr().
This avoids repeating the 'if (f_pdata->is_dtr)
cqspi_setup_opcode_ext()...` snippet multiple times.

- Call the default 'supports_op()' from cqspi_supports_mem_op(). This
makes sure the buswidth requirements are also enforced along with the
DTR requirements.

- Drop the 'is_dtr' argument from spi_check_dtr_req(). We only call it
when a phase is DTR so it is redundant.

Pratyush Yadav (16):
spi: spi-mem: allow specifying whether an op is DTR or not
spi: atmel-quadspi: reject DTR ops
spi: spi-mem: allow specifying a command's extension
mtd: spi-nor: add support for DTR protocol
mtd: spi-nor: default to address width of 3 for configurable widths
mtd: spi-nor: prepare BFPT parsing for JESD216 rev D
mtd: spi-nor: get command opcode extension type from BFPT
mtd: spi-nor: parse xSPI Profile 1.0 table
mtd: spi-nor: use dummy cycle and address width info from SFDP
mtd: spi-nor: do 2 byte reads for SR and FSR in DTR mode
mtd: spi-nor: enable octal DTR mode when possible
mtd: spi-nor: perform a Soft Reset on shutdown
mtd: spi-nor: Disable Octal DTR mode on suspend.
mtd: spi-nor: expose spi_nor_default_setup() in core.h
mtd: spi-nor: add support for Cypress Semper flash
mtd: spi-nor: allow using MT35XU512ABA in Octal DTR mode

drivers/mtd/spi-nor/core.c | 439 +++++++++++++++++++++++++++-----
drivers/mtd/spi-nor/core.h | 23 ++
drivers/mtd/spi-nor/micron-st.c | 113 +++++++-
drivers/mtd/spi-nor/sfdp.c | 114 ++++++++-
drivers/mtd/spi-nor/sfdp.h | 11 +-
drivers/mtd/spi-nor/spansion.c | 167 ++++++++++++
drivers/spi/atmel-quadspi.c | 4 +
drivers/spi/spi-mem.c | 3 +
include/linux/mtd/spi-nor.h | 50 +++-
include/linux/spi/spi-mem.h | 13 +-
10 files changed, 848 insertions(+), 89 deletions(-)

--
2.25.0


2020-04-24 18:46:36

by Pratyush Yadav

[permalink] [raw]
Subject: [PATCH v4 03/16] spi: spi-mem: allow specifying a command's extension

In xSPI mode, flashes expect 2-byte opcodes. The second byte is called
the "command extension". There can be 3 types of extensions in xSPI:
repeat, invert, and hex. When the extension type is "repeat", the same
opcode is sent twice. When it is "invert", the second byte is the
inverse of the opcode. When it is "hex" an additional opcode byte based
is sent with the command whose value can be anything.

So, make opcode a 16-bit value and add a 'nbytes', similar to how
multiple address widths are handled.

Signed-off-by: Pratyush Yadav <[email protected]>
---
include/linux/spi/spi-mem.h | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/include/linux/spi/spi-mem.h b/include/linux/spi/spi-mem.h
index e3dcb956bf61..731bb64c6ba6 100644
--- a/include/linux/spi/spi-mem.h
+++ b/include/linux/spi/spi-mem.h
@@ -69,6 +69,8 @@ enum spi_mem_data_dir {

/**
* struct spi_mem_op - describes a SPI memory operation
+ * @cmd.nbytes: number of opcode bytes (only 1 or 2 are valid). The opcode is
+ * sent MSB-first.
* @cmd.buswidth: number of IO lines used to transmit the command
* @cmd.opcode: operation opcode
* @cmd.dtr: whether the command opcode should be sent in DTR mode or not
@@ -94,9 +96,10 @@ enum spi_mem_data_dir {
*/
struct spi_mem_op {
struct {
+ u8 nbytes;
u8 buswidth;
u8 dtr : 1;
- u8 opcode;
+ u16 opcode;
} cmd;

struct {
--
2.25.0

2020-04-24 18:46:42

by Pratyush Yadav

[permalink] [raw]
Subject: [PATCH v4 07/16] mtd: spi-nor: get command opcode extension type from BFPT

Some devices in DTR mode expect an extra command byte called the
extension. The extension can either be same as the opcode, bitwise
inverse of the opcode, or another additional byte forming a 16-byte
opcode. Get the extension type from the BFPT. For now, only flashes with
"repeat" and "inverse" extensions are supported.

Signed-off-by: Pratyush Yadav <[email protected]>
---
drivers/mtd/spi-nor/sfdp.c | 17 +++++++++++++++++
drivers/mtd/spi-nor/sfdp.h | 6 ++++++
2 files changed, 23 insertions(+)

diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
index 96960f2f3d7a..ab086aa4746f 100644
--- a/drivers/mtd/spi-nor/sfdp.c
+++ b/drivers/mtd/spi-nor/sfdp.c
@@ -609,6 +609,23 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
if (bfpt_header->length == BFPT_DWORD_MAX_JESD216B)
return spi_nor_post_bfpt_fixups(nor, bfpt_header, &bfpt,
params);
+ /* 8D-8D-8D command extension. */
+ switch (bfpt.dwords[BFPT_DWORD(18)] & BFPT_DWORD18_CMD_EXT_MASK) {
+ case BFPT_DWORD18_CMD_EXT_REP:
+ nor->cmd_ext_type = SPI_NOR_EXT_REPEAT;
+ break;
+
+ case BFPT_DWORD18_CMD_EXT_INV:
+ nor->cmd_ext_type = SPI_NOR_EXT_INVERT;
+ break;
+
+ case BFPT_DWORD18_CMD_EXT_RES:
+ return -EINVAL;
+
+ case BFPT_DWORD18_CMD_EXT_16B:
+ dev_err(nor->dev, "16-bit opcodes not supported\n");
+ return -ENOTSUPP;
+ }

return spi_nor_post_bfpt_fixups(nor, bfpt_header, &bfpt, params);
}
diff --git a/drivers/mtd/spi-nor/sfdp.h b/drivers/mtd/spi-nor/sfdp.h
index f8198af43a63..e15e30796d62 100644
--- a/drivers/mtd/spi-nor/sfdp.h
+++ b/drivers/mtd/spi-nor/sfdp.h
@@ -84,6 +84,12 @@ struct sfdp_bfpt {
#define BFPT_DWORD15_QER_SR2_BIT1_NO_RD (0x4UL << 20)
#define BFPT_DWORD15_QER_SR2_BIT1 (0x5UL << 20) /* Spansion */

+#define BFPT_DWORD18_CMD_EXT_MASK GENMASK(30, 29)
+#define BFPT_DWORD18_CMD_EXT_REP (0x0UL << 29) /* Repeat */
+#define BFPT_DWORD18_CMD_EXT_INV (0x1UL << 29) /* Invert */
+#define BFPT_DWORD18_CMD_EXT_RES (0x2UL << 29) /* Reserved */
+#define BFPT_DWORD18_CMD_EXT_16B (0x3UL << 29) /* 16-bit opcode */
+
struct sfdp_parameter_header {
u8 id_lsb;
u8 minor;
--
2.25.0

2020-04-24 18:46:46

by Pratyush Yadav

[permalink] [raw]
Subject: [PATCH v4 05/16] mtd: spi-nor: default to address width of 3 for configurable widths

JESD216D.01 says that when the address width can be 3 or 4, it defaults
to 3 and enters 4-byte mode when given the appropriate command. So, when
we see a configurable width, default to 3 and let flash that default to
4 change it in a post-bfpt fixup.

This fixes SMPT parsing for flashes with configurable address width. If
the SMPT descriptor advertises variable address width, we use
nor->addr_width as the address width. But since it was not set to any
value from the SFDP table, the read command uses an address width of 0,
resulting in an incorrect read being issued.

Signed-off-by: Pratyush Yadav <[email protected]>
---
drivers/mtd/spi-nor/sfdp.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
index f917631c8110..5cecc4ba2141 100644
--- a/drivers/mtd/spi-nor/sfdp.c
+++ b/drivers/mtd/spi-nor/sfdp.c
@@ -460,6 +460,7 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
/* Number of address bytes. */
switch (bfpt.dwords[BFPT_DWORD(1)] & BFPT_DWORD1_ADDRESS_BYTES_MASK) {
case BFPT_DWORD1_ADDRESS_BYTES_3_ONLY:
+ case BFPT_DWORD1_ADDRESS_BYTES_3_OR_4:
nor->addr_width = 3;
break;

--
2.25.0

2020-04-24 18:46:49

by Pratyush Yadav

[permalink] [raw]
Subject: [PATCH v4 08/16] mtd: spi-nor: parse xSPI Profile 1.0 table

This table is indication that the flash is xSPI compliant and hence
supports octal DTR mode. Extract information like the fast read opcode,
the number of dummy cycles needed for a Read Status Register command,
and the number of address bytes needed for a Read Status Register
command.

The default dummy cycles for a fast octal DTR read are set to 20. Since
there is no simple way of determining the dummy cycles needed for the
fast read command, flashes that use a different value should update it
in their flash-specific hooks.

Since we want to set read settings, expose spi_nor_set_read_settings()
in core.h.

Signed-off-by: Pratyush Yadav <[email protected]>
---
drivers/mtd/spi-nor/core.c | 2 +-
drivers/mtd/spi-nor/core.h | 10 +++++
drivers/mtd/spi-nor/sfdp.c | 80 ++++++++++++++++++++++++++++++++++++++
3 files changed, 91 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index c8c1a3260e91..9c92fc897fb4 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -2355,7 +2355,7 @@ static int spi_nor_check(struct spi_nor *nor)
return 0;
}

-static void
+void
spi_nor_set_read_settings(struct spi_nor_read_command *read,
u8 num_mode_clocks,
u8 num_wait_states,
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index de1e3917889f..7e6df8322da0 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -192,6 +192,9 @@ struct spi_nor_locking_ops {
*
* @size: the flash memory density in bytes.
* @page_size: the page size of the SPI NOR flash memory.
+ * @rdsr_dummy: dummy cycles needed for Read Status Register command.
+ * @rdsr_addr_nbytes: dummy address bytes needed for Read Status Register
+ * command.
* @hwcaps: describes the read and page program hardware
* capabilities.
* @reads: read capabilities ordered by priority: the higher index
@@ -214,6 +217,8 @@ struct spi_nor_locking_ops {
struct spi_nor_flash_parameter {
u64 size;
u32 page_size;
+ u8 rdsr_dummy;
+ u8 rdsr_addr_nbytes;

struct spi_nor_hwcaps hwcaps;
struct spi_nor_read_command reads[SNOR_CMD_READ_MAX];
@@ -424,6 +429,11 @@ ssize_t spi_nor_write_data(struct spi_nor *nor, loff_t to, size_t len,

int spi_nor_hwcaps_read2cmd(u32 hwcaps);
u8 spi_nor_convert_3to4_read(u8 opcode);
+void spi_nor_set_read_settings(struct spi_nor_read_command *read,
+ u8 num_mode_clocks,
+ u8 num_wait_states,
+ u8 opcode,
+ enum spi_nor_protocol proto);
void spi_nor_set_pp_settings(struct spi_nor_pp_command *pp, u8 opcode,
enum spi_nor_protocol proto);

diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
index ab086aa4746f..cef8554ada19 100644
--- a/drivers/mtd/spi-nor/sfdp.c
+++ b/drivers/mtd/spi-nor/sfdp.c
@@ -4,6 +4,7 @@
* Copyright (C) 2014, Freescale Semiconductor, Inc.
*/

+#include <linux/bitfield.h>
#include <linux/slab.h>
#include <linux/sort.h>
#include <linux/mtd/spi-nor.h>
@@ -19,12 +20,14 @@
#define SFDP_BFPT_ID 0xff00 /* Basic Flash Parameter Table */
#define SFDP_SECTOR_MAP_ID 0xff81 /* Sector Map Table */
#define SFDP_4BAIT_ID 0xff84 /* 4-byte Address Instruction Table */
+#define SFDP_PROFILE1_ID 0xff05 /* xSPI Profile 1.0 table. */

#define SFDP_SIGNATURE 0x50444653U
#define SFDP_JESD216_MAJOR 1
#define SFDP_JESD216_MINOR 0
#define SFDP_JESD216A_MINOR 5
#define SFDP_JESD216B_MINOR 6
+#define SFDP_JESD216D_MINOR 8

struct sfdp_header {
u32 signature; /* Ox50444653U <=> "SFDP" */
@@ -70,6 +73,11 @@ struct sfdp_bfpt_erase {
u32 shift;
};

+/* xSPI Profile 1.0 table (from JESD216D.01). */
+#define PROFILE1_DWORD1_RD_FAST_CMD GENMASK(15, 8)
+#define PROFILE1_DWORD1_RDSR_DUMMY BIT(28)
+#define PROFILE1_DWORD1_RDSR_ADDR_BYTES BIT(29)
+
#define SMPT_CMD_ADDRESS_LEN_MASK GENMASK(23, 22)
#define SMPT_CMD_ADDRESS_LEN_0 (0x0UL << 22)
#define SMPT_CMD_ADDRESS_LEN_3 (0x1UL << 22)
@@ -1110,6 +1118,74 @@ static int spi_nor_parse_4bait(struct spi_nor *nor,
return ret;
}

+/**
+ * spi_nor_parse_profile1() - parse the xSPI Profile 1.0 table
+ * @nor: pointer to a 'struct spi_nor'
+ * @param_header: pointer to the 'struct sfdp_parameter_header' describing
+ * the 4-Byte Address Instruction Table length and version.
+ * @params: pointer to the 'struct spi_nor_flash_parameter' to be.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int spi_nor_parse_profile1(struct spi_nor *nor,
+ const struct sfdp_parameter_header *profile1_header,
+ struct spi_nor_flash_parameter *params)
+{
+ u32 *table, opcode, addr;
+ size_t len;
+ int ret, i;
+
+ len = profile1_header->length * sizeof(*table);
+ table = kmalloc(len, GFP_KERNEL);
+ if (!table)
+ return -ENOMEM;
+
+ addr = SFDP_PARAM_HEADER_PTP(profile1_header);
+ ret = spi_nor_read_sfdp(nor, addr, len, table);
+ if (ret)
+ goto out;
+
+ /* Fix endianness of the table DWORDs. */
+ for (i = 0; i < profile1_header->length; i++)
+ table[i] = le32_to_cpu(table[i]);
+
+ /* Get 8D-8D-8D fast read opcode and dummy cycles. */
+ opcode = FIELD_GET(PROFILE1_DWORD1_RD_FAST_CMD, table[0]);
+
+ /*
+ * Update the fast read settings. We set the default dummy cycles to 20
+ * here. Flashes can change this value if they need to when enabling
+ * octal mode.
+ */
+ params->hwcaps.mask |= SNOR_HWCAPS_READ_8_8_8_DTR;
+ spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_8_8_8_DTR],
+ 0, 20, opcode,
+ SNOR_PROTO_8_8_8_DTR);
+
+ /*
+ * Since the flash supports xSPI DTR reads, it should also support DTR
+ * Page Program opcodes.
+ */
+ params->hwcaps.mask |= SNOR_HWCAPS_PP_8_8_8_DTR;
+
+ /*
+ * Set the Read Status Register dummy cycles and dummy address bytes.
+ */
+ if (table[0] & PROFILE1_DWORD1_RDSR_DUMMY)
+ params->rdsr_dummy = 8;
+ else
+ params->rdsr_dummy = 4;
+
+ if (table[0] & PROFILE1_DWORD1_RDSR_ADDR_BYTES)
+ params->rdsr_addr_nbytes = 4;
+ else
+ params->rdsr_addr_nbytes = 0;
+
+out:
+ kfree(table);
+ return ret;
+}
+
/**
* spi_nor_parse_sfdp() - parse the Serial Flash Discoverable Parameters.
* @nor: pointer to a 'struct spi_nor'
@@ -1211,6 +1287,10 @@ int spi_nor_parse_sfdp(struct spi_nor *nor,
err = spi_nor_parse_4bait(nor, param_header, params);
break;

+ case SFDP_PROFILE1_ID:
+ err = spi_nor_parse_profile1(nor, param_header, params);
+ break;
+
default:
break;
}
--
2.25.0

2020-04-24 18:46:59

by Pratyush Yadav

[permalink] [raw]
Subject: [PATCH v4 06/16] mtd: spi-nor: prepare BFPT parsing for JESD216 rev D

JESD216 rev D makes BFPT 20 DWORDs. Update the BFPT size define to
reflect that.

The check for rev A or later compared the BFPT header length with the
maximum BFPT length, BFPT_DWORD_MAX. Since BFPT_DWORD_MAX was 16, and so
was the BFPT length for both rev A and B, this check worked fine. But
now, since BFPT_DWORD_MAX is 20, it means this check will also stop BFPT
parsing for rev A or B, since their length is 16.

So, instead check for BFPT_DWORD_MAX_JESD216 to stop BFPT parsing for
the first JESD216 version, and check for BFPT_DWORD_MAX_JESD216B for the
next two versions.

Signed-off-by: Pratyush Yadav <[email protected]>
---
drivers/mtd/spi-nor/sfdp.c | 7 ++++++-
drivers/mtd/spi-nor/sfdp.h | 5 +++--
2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
index 5cecc4ba2141..96960f2f3d7a 100644
--- a/drivers/mtd/spi-nor/sfdp.c
+++ b/drivers/mtd/spi-nor/sfdp.c
@@ -549,7 +549,7 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
SNOR_ERASE_TYPE_MASK;

/* Stop here if not JESD216 rev A or later. */
- if (bfpt_header->length < BFPT_DWORD_MAX)
+ if (bfpt_header->length == BFPT_DWORD_MAX_JESD216)
return spi_nor_post_bfpt_fixups(nor, bfpt_header, &bfpt,
params);

@@ -605,6 +605,11 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
return -EINVAL;
}

+ /* Stop here if JESD216 rev B. */
+ if (bfpt_header->length == BFPT_DWORD_MAX_JESD216B)
+ return spi_nor_post_bfpt_fixups(nor, bfpt_header, &bfpt,
+ params);
+
return spi_nor_post_bfpt_fixups(nor, bfpt_header, &bfpt, params);
}

diff --git a/drivers/mtd/spi-nor/sfdp.h b/drivers/mtd/spi-nor/sfdp.h
index e0a8ded04890..f8198af43a63 100644
--- a/drivers/mtd/spi-nor/sfdp.h
+++ b/drivers/mtd/spi-nor/sfdp.h
@@ -10,11 +10,11 @@
/* Basic Flash Parameter Table */

/*
- * JESD216 rev B defines a Basic Flash Parameter Table of 16 DWORDs.
+ * JESD216 rev D defines a Basic Flash Parameter Table of 20 DWORDs.
* They are indexed from 1 but C arrays are indexed from 0.
*/
#define BFPT_DWORD(i) ((i) - 1)
-#define BFPT_DWORD_MAX 16
+#define BFPT_DWORD_MAX 20

struct sfdp_bfpt {
u32 dwords[BFPT_DWORD_MAX];
@@ -22,6 +22,7 @@ struct sfdp_bfpt {

/* The first version of JESD216 defined only 9 DWORDs. */
#define BFPT_DWORD_MAX_JESD216 9
+#define BFPT_DWORD_MAX_JESD216B 16

/* 1st DWORD. */
#define BFPT_DWORD1_FAST_READ_1_1_2 BIT(16)
--
2.25.0

2020-04-24 18:47:21

by Pratyush Yadav

[permalink] [raw]
Subject: [PATCH v4 04/16] mtd: spi-nor: add support for DTR protocol

Double Transfer Rate (DTR) is SPI protocol in which data is transferred
on each clock edge as opposed to on each clock cycle. Make
framework-level changes to allow supporting flashes in DTR mode.

Right now, mixed DTR modes are not supported. So, for example a mode
like 4S-4D-4D will not work. All phases need to be either DTR or STR.

Signed-off-by: Pratyush Yadav <[email protected]>
---
drivers/mtd/spi-nor/core.c | 295 ++++++++++++++++++++++++++++--------
drivers/mtd/spi-nor/core.h | 6 +
drivers/mtd/spi-nor/sfdp.c | 9 +-
include/linux/mtd/spi-nor.h | 48 ++++--
4 files changed, 281 insertions(+), 77 deletions(-)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index cc68ea84318e..c8c1a3260e91 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -40,6 +40,76 @@

#define SPI_NOR_MAX_ADDR_WIDTH 4

+/**
+ * spi_nor_get_cmd_ext() - Get the command opcode extension based on the
+ * extension type.
+ * @nor: pointer to a 'struct spi_nor'
+ * @op: pointer to the 'struct spi_mem_op' whose properties
+ * need to be initialized.
+ *
+ * Right now, only "repeat" and "invert" are supported.
+ *
+ * Return: The opcode extension.
+ */
+static u8 spi_nor_get_cmd_ext(const struct spi_nor *nor,
+ const struct spi_mem_op *op)
+{
+ switch (nor->cmd_ext_type) {
+ case SPI_NOR_EXT_INVERT:
+ return ~op->cmd.opcode;
+
+ case SPI_NOR_EXT_REPEAT:
+ return op->cmd.opcode;
+
+ default:
+ dev_err(nor->dev, "Unknown command extension type\n");
+ return 0;
+ }
+}
+
+/**
+ * spi_nor_spimem_setup_op() - Set up common properties of a spi-mem op.
+ * @nor: pointer to a 'struct spi_nor'
+ * @op: pointer to the 'struct spi_mem_op' whose properties
+ * need to be initialized.
+ * @proto: the protocol from which the properties need to be set.
+ */
+void spi_nor_spimem_setup_op(const struct spi_nor *nor,
+ struct spi_mem_op *op,
+ const enum spi_nor_protocol proto)
+{
+ u8 ext;
+
+ op->cmd.buswidth = spi_nor_get_protocol_inst_nbits(proto);
+
+ if (op->addr.nbytes)
+ op->addr.buswidth = spi_nor_get_protocol_addr_nbits(proto);
+
+ if (op->dummy.nbytes)
+ op->dummy.buswidth = spi_nor_get_protocol_addr_nbits(proto);
+
+ if (op->data.nbytes)
+ op->data.buswidth = spi_nor_get_protocol_data_nbits(proto);
+
+ if (spi_nor_protocol_is_dtr(proto)) {
+ /*
+ * spi-mem supports mixed DTR modes, but right now we can only
+ * have all phases either DTR or STR. IOW, spi-mem can have
+ * something like 4S-4D-4D, but spi-nor can't. So, set all 4
+ * phases to either DTR or STR.
+ */
+ op->cmd.dtr = op->addr.dtr = op->dummy.dtr
+ = op->data.dtr = true;
+
+ /* 2 bytes per clock cycle in DTR mode. */
+ op->dummy.nbytes *= 2;
+
+ ext = spi_nor_get_cmd_ext(nor, op);
+ op->cmd.opcode = (op->cmd.opcode << 8) | ext;
+ op->cmd.nbytes = 2;
+ }
+}
+
/**
* spi_nor_spimem_bounce() - check if a bounce buffer is needed for the data
* transfer
@@ -104,14 +174,12 @@ static ssize_t spi_nor_spimem_read_data(struct spi_nor *nor, loff_t from,
ssize_t nbytes;
int error;

- /* get transfer protocols. */
- op.cmd.buswidth = spi_nor_get_protocol_inst_nbits(nor->read_proto);
- op.addr.buswidth = spi_nor_get_protocol_addr_nbits(nor->read_proto);
- op.dummy.buswidth = op.addr.buswidth;
- op.data.buswidth = spi_nor_get_protocol_data_nbits(nor->read_proto);
+ spi_nor_spimem_setup_op(nor, &op, nor->read_proto);

/* convert the dummy cycles to the number of bytes */
op.dummy.nbytes = (nor->read_dummy * op.dummy.buswidth) / 8;
+ if (spi_nor_protocol_is_dtr(nor->read_proto))
+ op.dummy.nbytes *= 2;

usebouncebuf = spi_nor_spimem_bounce(nor, &op);

@@ -169,13 +237,11 @@ static ssize_t spi_nor_spimem_write_data(struct spi_nor *nor, loff_t to,
ssize_t nbytes;
int error;

- op.cmd.buswidth = spi_nor_get_protocol_inst_nbits(nor->write_proto);
- op.addr.buswidth = spi_nor_get_protocol_addr_nbits(nor->write_proto);
- op.data.buswidth = spi_nor_get_protocol_data_nbits(nor->write_proto);
-
if (nor->program_opcode == SPINOR_OP_AAI_WP && nor->sst_write_second)
op.addr.nbytes = 0;

+ spi_nor_spimem_setup_op(nor, &op, nor->write_proto);
+
if (spi_nor_spimem_bounce(nor, &op))
memcpy(nor->bouncebuf, buf, op.data.nbytes);

@@ -227,10 +293,16 @@ int spi_nor_write_enable(struct spi_nor *nor)
SPI_MEM_OP_NO_DUMMY,
SPI_MEM_OP_NO_DATA);

+ spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
+
ret = spi_mem_exec_op(nor->spimem, &op);
} else {
- ret = nor->controller_ops->write_reg(nor, SPINOR_OP_WREN,
- NULL, 0);
+ if (spi_nor_protocol_is_dtr(nor->reg_proto))
+ ret = -ENOTSUPP;
+ else
+ ret = nor->controller_ops->write_reg(nor,
+ SPINOR_OP_WREN,
+ NULL, 0);
}

if (ret)
@@ -256,10 +328,16 @@ int spi_nor_write_disable(struct spi_nor *nor)
SPI_MEM_OP_NO_DUMMY,
SPI_MEM_OP_NO_DATA);

+ spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
+
ret = spi_mem_exec_op(nor->spimem, &op);
} else {
- ret = nor->controller_ops->write_reg(nor, SPINOR_OP_WRDI,
- NULL, 0);
+ if (spi_nor_protocol_is_dtr(nor->reg_proto))
+ ret = -ENOTSUPP;
+ else
+ ret = nor->controller_ops->write_reg(nor,
+ SPINOR_OP_WRDI,
+ NULL, 0);
}

if (ret)
@@ -318,10 +396,15 @@ static int spi_nor_read_fsr(struct spi_nor *nor, u8 *fsr)
SPI_MEM_OP_NO_DUMMY,
SPI_MEM_OP_DATA_IN(1, fsr, 1));

+ spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
+
ret = spi_mem_exec_op(nor->spimem, &op);
} else {
- ret = nor->controller_ops->read_reg(nor, SPINOR_OP_RDFSR,
- fsr, 1);
+ if (spi_nor_protocol_is_dtr(nor->reg_proto))
+ ret = -ENOTSUPP;
+ else
+ ret = nor->controller_ops->read_reg(nor, SPINOR_OP_RDFSR,
+ fsr, 1);
}

if (ret)
@@ -350,9 +433,15 @@ static int spi_nor_read_cr(struct spi_nor *nor, u8 *cr)
SPI_MEM_OP_NO_DUMMY,
SPI_MEM_OP_DATA_IN(1, cr, 1));

+ spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
+
ret = spi_mem_exec_op(nor->spimem, &op);
} else {
- ret = nor->controller_ops->read_reg(nor, SPINOR_OP_RDCR, cr, 1);
+ if (spi_nor_protocol_is_dtr(nor->reg_proto))
+ ret = -ENOTSUPP;
+ else
+ ret = nor->controller_ops->read_reg(nor, SPINOR_OP_RDCR,
+ cr, 1);
}

if (ret)
@@ -383,12 +472,17 @@ int spi_nor_set_4byte_addr_mode(struct spi_nor *nor, bool enable)
SPI_MEM_OP_NO_DUMMY,
SPI_MEM_OP_NO_DATA);

+ spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
+
ret = spi_mem_exec_op(nor->spimem, &op);
} else {
- ret = nor->controller_ops->write_reg(nor,
- enable ? SPINOR_OP_EN4B :
- SPINOR_OP_EX4B,
- NULL, 0);
+ if (spi_nor_protocol_is_dtr(nor->reg_proto))
+ ret = -ENOTSUPP;
+ else
+ ret = nor->controller_ops->write_reg(nor,
+ enable ? SPINOR_OP_EN4B :
+ SPINOR_OP_EX4B,
+ NULL, 0);
}

if (ret)
@@ -419,10 +513,15 @@ static int spansion_set_4byte_addr_mode(struct spi_nor *nor, bool enable)
SPI_MEM_OP_NO_DUMMY,
SPI_MEM_OP_DATA_OUT(1, nor->bouncebuf, 1));

+ spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
+
ret = spi_mem_exec_op(nor->spimem, &op);
} else {
- ret = nor->controller_ops->write_reg(nor, SPINOR_OP_BRWR,
- nor->bouncebuf, 1);
+ if (spi_nor_protocol_is_dtr(nor->reg_proto))
+ ret = -ENOTSUPP;
+ else
+ ret = nor->controller_ops->write_reg(nor, SPINOR_OP_BRWR,
+ nor->bouncebuf, 1);
}

if (ret)
@@ -451,10 +550,16 @@ int spi_nor_write_ear(struct spi_nor *nor, u8 ear)
SPI_MEM_OP_NO_DUMMY,
SPI_MEM_OP_DATA_OUT(1, nor->bouncebuf, 1));

+ spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
+
ret = spi_mem_exec_op(nor->spimem, &op);
} else {
- ret = nor->controller_ops->write_reg(nor, SPINOR_OP_WREAR,
- nor->bouncebuf, 1);
+ if (spi_nor_protocol_is_dtr(nor->reg_proto))
+ ret = -ENOTSUPP;
+ else
+ ret = nor->controller_ops->write_reg(nor,
+ SPINOR_OP_WREAR,
+ nor->bouncebuf, 1);
}

if (ret)
@@ -482,10 +587,16 @@ int spi_nor_xread_sr(struct spi_nor *nor, u8 *sr)
SPI_MEM_OP_NO_DUMMY,
SPI_MEM_OP_DATA_IN(1, sr, 1));

+ spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
+
ret = spi_mem_exec_op(nor->spimem, &op);
} else {
- ret = nor->controller_ops->read_reg(nor, SPINOR_OP_XRDSR,
- sr, 1);
+ if (spi_nor_protocol_is_dtr(nor->reg_proto))
+ ret = -ENOTSUPP;
+ else
+ ret = nor->controller_ops->read_reg(nor,
+ SPINOR_OP_XRDSR,
+ sr, 1);
}

if (ret)
@@ -527,10 +638,16 @@ static void spi_nor_clear_sr(struct spi_nor *nor)
SPI_MEM_OP_NO_DUMMY,
SPI_MEM_OP_NO_DATA);

+ spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
+
ret = spi_mem_exec_op(nor->spimem, &op);
} else {
- ret = nor->controller_ops->write_reg(nor, SPINOR_OP_CLSR,
- NULL, 0);
+ if (spi_nor_protocol_is_dtr(nor->reg_proto))
+ ret = -ENOTSUPP;
+ else
+ ret = nor->controller_ops->write_reg(nor,
+ SPINOR_OP_CLSR,
+ NULL, 0);
}

if (ret)
@@ -591,10 +708,16 @@ static void spi_nor_clear_fsr(struct spi_nor *nor)
SPI_MEM_OP_NO_DUMMY,
SPI_MEM_OP_NO_DATA);

+ spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
+
ret = spi_mem_exec_op(nor->spimem, &op);
} else {
- ret = nor->controller_ops->write_reg(nor, SPINOR_OP_CLFSR,
- NULL, 0);
+ if (spi_nor_protocol_is_dtr(nor->reg_proto))
+ ret = -ENOTSUPP;
+ else
+ ret = nor->controller_ops->write_reg(nor,
+ SPINOR_OP_CLFSR,
+ NULL, 0);
}

if (ret)
@@ -735,10 +858,16 @@ static int spi_nor_write_sr(struct spi_nor *nor, const u8 *sr, size_t len)
SPI_MEM_OP_NO_DUMMY,
SPI_MEM_OP_DATA_OUT(len, sr, 1));

+ spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
+
ret = spi_mem_exec_op(nor->spimem, &op);
} else {
- ret = nor->controller_ops->write_reg(nor, SPINOR_OP_WRSR,
- sr, len);
+ if (spi_nor_protocol_is_dtr(nor->reg_proto))
+ ret = -ENOTSUPP;
+ else
+ ret = nor->controller_ops->write_reg(nor,
+ SPINOR_OP_WRSR,
+ sr, len);
}

if (ret) {
@@ -937,10 +1066,16 @@ static int spi_nor_write_sr2(struct spi_nor *nor, const u8 *sr2)
SPI_MEM_OP_NO_DUMMY,
SPI_MEM_OP_DATA_OUT(1, sr2, 1));

+ spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
+
ret = spi_mem_exec_op(nor->spimem, &op);
} else {
- ret = nor->controller_ops->write_reg(nor, SPINOR_OP_WRSR2,
- sr2, 1);
+ if (spi_nor_protocol_is_dtr(nor->reg_proto))
+ ret = -ENOTSUPP;
+ else
+ ret = nor->controller_ops->write_reg(nor,
+ SPINOR_OP_WRSR2,
+ sr2, 1);
}

if (ret) {
@@ -971,10 +1106,16 @@ static int spi_nor_read_sr2(struct spi_nor *nor, u8 *sr2)
SPI_MEM_OP_NO_DUMMY,
SPI_MEM_OP_DATA_IN(1, sr2, 1));

+ spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
+
ret = spi_mem_exec_op(nor->spimem, &op);
} else {
- ret = nor->controller_ops->read_reg(nor, SPINOR_OP_RDSR2,
- sr2, 1);
+ if (spi_nor_protocol_is_dtr(nor->reg_proto))
+ ret = -ENOTSUPP;
+ else
+ ret = nor->controller_ops->read_reg(nor,
+ SPINOR_OP_RDSR2,
+ sr2, 1);
}

if (ret)
@@ -1002,10 +1143,16 @@ static int spi_nor_erase_chip(struct spi_nor *nor)
SPI_MEM_OP_NO_DUMMY,
SPI_MEM_OP_NO_DATA);

+ spi_nor_spimem_setup_op(nor, &op, nor->write_proto);
+
ret = spi_mem_exec_op(nor->spimem, &op);
} else {
- ret = nor->controller_ops->write_reg(nor, SPINOR_OP_CHIP_ERASE,
- NULL, 0);
+ if (spi_nor_protocol_is_dtr(nor->write_proto))
+ ret = -ENOTSUPP;
+ else
+ ret = nor->controller_ops->write_reg(nor,
+ SPINOR_OP_CHIP_ERASE,
+ NULL, 0);
}

if (ret)
@@ -1144,7 +1291,11 @@ static int spi_nor_erase_sector(struct spi_nor *nor, u32 addr)
SPI_MEM_OP_NO_DUMMY,
SPI_MEM_OP_NO_DATA);

+ spi_nor_spimem_setup_op(nor, &op, nor->write_proto);
+
return spi_mem_exec_op(nor->spimem, &op);
+ } else if (spi_nor_protocol_is_dtr(nor->write_proto)) {
+ return -ENOTSUPP;
} else if (nor->controller_ops->erase) {
return nor->controller_ops->erase(nor, addr);
}
@@ -2253,6 +2404,7 @@ int spi_nor_hwcaps_read2cmd(u32 hwcaps)
{ SNOR_HWCAPS_READ_1_8_8, SNOR_CMD_READ_1_8_8 },
{ SNOR_HWCAPS_READ_8_8_8, SNOR_CMD_READ_8_8_8 },
{ SNOR_HWCAPS_READ_1_8_8_DTR, SNOR_CMD_READ_1_8_8_DTR },
+ { SNOR_HWCAPS_READ_8_8_8_DTR, SNOR_CMD_READ_8_8_8_DTR },
};

return spi_nor_hwcaps2cmd(hwcaps, hwcaps_read2cmd,
@@ -2269,6 +2421,7 @@ static int spi_nor_hwcaps_pp2cmd(u32 hwcaps)
{ SNOR_HWCAPS_PP_1_1_8, SNOR_CMD_PP_1_1_8 },
{ SNOR_HWCAPS_PP_1_8_8, SNOR_CMD_PP_1_8_8 },
{ SNOR_HWCAPS_PP_8_8_8, SNOR_CMD_PP_8_8_8 },
+ { SNOR_HWCAPS_PP_8_8_8_DTR, SNOR_CMD_PP_8_8_8_DTR },
};

return spi_nor_hwcaps2cmd(hwcaps, hwcaps_pp2cmd,
@@ -2322,13 +2475,11 @@ static int spi_nor_spimem_check_readop(struct spi_nor *nor,
SPI_MEM_OP_DUMMY(0, 1),
SPI_MEM_OP_DATA_IN(0, NULL, 1));

- op.cmd.buswidth = spi_nor_get_protocol_inst_nbits(read->proto);
- op.addr.buswidth = spi_nor_get_protocol_addr_nbits(read->proto);
- op.data.buswidth = spi_nor_get_protocol_data_nbits(read->proto);
- op.dummy.buswidth = op.addr.buswidth;
op.dummy.nbytes = (read->num_mode_clocks + read->num_wait_states) *
op.dummy.buswidth / 8;

+ spi_nor_spimem_setup_op(nor, &op, read->proto);
+
return spi_nor_spimem_check_op(nor, &op);
}

@@ -2348,9 +2499,7 @@ static int spi_nor_spimem_check_pp(struct spi_nor *nor,
SPI_MEM_OP_NO_DUMMY,
SPI_MEM_OP_DATA_OUT(0, NULL, 1));

- op.cmd.buswidth = spi_nor_get_protocol_inst_nbits(pp->proto);
- op.addr.buswidth = spi_nor_get_protocol_addr_nbits(pp->proto);
- op.data.buswidth = spi_nor_get_protocol_data_nbits(pp->proto);
+ spi_nor_spimem_setup_op(nor, &op, pp->proto);

return spi_nor_spimem_check_op(nor, &op);
}
@@ -2368,11 +2517,10 @@ spi_nor_spimem_adjust_hwcaps(struct spi_nor *nor, u32 *hwcaps)
struct spi_nor_flash_parameter *params = nor->params;
unsigned int cap;

- /* DTR modes are not supported yet, mask them all. */
- *hwcaps &= ~SNOR_HWCAPS_DTR;
-
- /* X-X-X modes are not supported yet, mask them all. */
- *hwcaps &= ~SNOR_HWCAPS_X_X_X;
+ /* 2-2-2 and 4-4-4 modes are not supported yet, mask them all. */
+ *hwcaps &= ~(SNOR_HWCAPS_READ_2_2_2 |
+ SNOR_HWCAPS_READ_4_4_4 |
+ SNOR_HWCAPS_PP_4_4_4);

for (cap = 0; cap < sizeof(*hwcaps) * BITS_PER_BYTE; cap++) {
int rdidx, ppidx;
@@ -2779,6 +2927,13 @@ static void spi_nor_info_init_params(struct spi_nor *nor)
spi_nor_set_pp_settings(&params->page_programs[SNOR_CMD_PP],
SPINOR_OP_PP, SNOR_PROTO_1_1_1);

+ /*
+ * Since xSPI Page Program opcode is backward compatible with
+ * Legacy SPI, use Legacy SPI opcode there as well.
+ */
+ spi_nor_set_pp_settings(&params->page_programs[SNOR_CMD_PP_8_8_8_DTR],
+ SPINOR_OP_PP, SNOR_PROTO_8_8_8_DTR);
+
/*
* Sector Erase settings. Sort Erase Types in ascending order, with the
* smallest erase size starting at BIT(0).
@@ -2886,7 +3041,8 @@ static int spi_nor_init_params(struct spi_nor *nor)

spi_nor_manufacturer_init_params(nor);

- if ((nor->info->flags & (SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)) &&
+ if ((nor->info->flags & (SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
+ SPI_NOR_OCTAL_READ | SPI_NOR_OCTAL_DTR_READ)) &&
!(nor->info->flags & SPI_NOR_SKIP_SFDP))
spi_nor_sfdp_init_params(nor);

@@ -2948,7 +3104,9 @@ static int spi_nor_init(struct spi_nor *nor)
return err;
}

- if (nor->addr_width == 4 && !(nor->flags & SNOR_F_4B_OPCODES)) {
+ if (nor->addr_width == 4 &&
+ !(nor->info->flags & SPI_NOR_OCTAL_DTR_READ) &&
+ !(nor->flags & SNOR_F_4B_OPCODES)) {
/*
* If the RESET# pin isn't hooked up properly, or the system
* otherwise doesn't perform a reset command in the boot
@@ -3007,6 +3165,9 @@ static int spi_nor_set_addr_width(struct spi_nor *nor)
{
if (nor->addr_width) {
/* already configured from SFDP */
+ } else if (spi_nor_protocol_is_dtr(nor->read_proto)) {
+ /* Always use 4-byte addresses in DTR mode. */
+ nor->addr_width = 4;
} else if (nor->info->addr_width) {
nor->addr_width = nor->info->addr_width;
} else if (nor->mtd.size > 0x1000000) {
@@ -3244,14 +3405,19 @@ static int spi_nor_create_read_dirmap(struct spi_nor *nor)
};
struct spi_mem_op *op = &info.op_tmpl;

- /* get transfer protocols. */
- op->cmd.buswidth = spi_nor_get_protocol_inst_nbits(nor->read_proto);
- op->addr.buswidth = spi_nor_get_protocol_addr_nbits(nor->read_proto);
- op->dummy.buswidth = op->addr.buswidth;
- op->data.buswidth = spi_nor_get_protocol_data_nbits(nor->read_proto);
+ spi_nor_spimem_setup_op(nor, op, nor->read_proto);

/* convert the dummy cycles to the number of bytes */
op->dummy.nbytes = (nor->read_dummy * op->dummy.buswidth) / 8;
+ if (spi_nor_protocol_is_dtr(nor->read_proto))
+ op->dummy.nbytes *= 2;
+
+ /*
+ * Since spi_nor_spimem_setup_op() only sets buswidth when the number
+ * of data bytes is non-zero, the data buswidth won't be set here. So,
+ * do it explicitly.
+ */
+ op->data.buswidth = spi_nor_get_protocol_data_nbits(nor->read_proto);

nor->dirmap.rdesc = devm_spi_mem_dirmap_create(nor->dev, nor->spimem,
&info);
@@ -3270,15 +3436,18 @@ static int spi_nor_create_write_dirmap(struct spi_nor *nor)
};
struct spi_mem_op *op = &info.op_tmpl;

- /* get transfer protocols. */
- op->cmd.buswidth = spi_nor_get_protocol_inst_nbits(nor->write_proto);
- op->addr.buswidth = spi_nor_get_protocol_addr_nbits(nor->write_proto);
- op->dummy.buswidth = op->addr.buswidth;
- op->data.buswidth = spi_nor_get_protocol_data_nbits(nor->write_proto);
-
if (nor->program_opcode == SPINOR_OP_AAI_WP && nor->sst_write_second)
op->addr.nbytes = 0;

+ spi_nor_spimem_setup_op(nor, op, nor->write_proto);
+
+ /*
+ * Since spi_nor_spimem_setup_op() only sets buswidth when the number
+ * of data bytes is non-zero, the data buswidth won't be set here. So,
+ * do it explicitly.
+ */
+ op->data.buswidth = spi_nor_get_protocol_data_nbits(nor->write_proto);
+
nor->dirmap.wdesc = devm_spi_mem_dirmap_create(nor->dev, nor->spimem,
&info);
return PTR_ERR_OR_ZERO(nor->dirmap.wdesc);
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index 6f2f6b27173f..de1e3917889f 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -62,6 +62,7 @@ enum spi_nor_read_command_index {
SNOR_CMD_READ_1_8_8,
SNOR_CMD_READ_8_8_8,
SNOR_CMD_READ_1_8_8_DTR,
+ SNOR_CMD_READ_8_8_8_DTR,

SNOR_CMD_READ_MAX
};
@@ -78,6 +79,7 @@ enum spi_nor_pp_command_index {
SNOR_CMD_PP_1_1_8,
SNOR_CMD_PP_1_8_8,
SNOR_CMD_PP_8_8_8,
+ SNOR_CMD_PP_8_8_8_DTR,

SNOR_CMD_PP_MAX
};
@@ -311,6 +313,7 @@ struct flash_info {
* BP3 is bit 6 of status register.
* Must be used with SPI_NOR_4BIT_BP.
*/
+#define SPI_NOR_OCTAL_DTR_READ BIT(19) /* Flash supports octal DTR Read. */

/* Part specific fixup hooks. */
const struct spi_nor_fixups *fixups;
@@ -399,6 +402,9 @@ extern const struct spi_nor_manufacturer spi_nor_winbond;
extern const struct spi_nor_manufacturer spi_nor_xilinx;
extern const struct spi_nor_manufacturer spi_nor_xmc;

+void spi_nor_spimem_setup_op(const struct spi_nor *nor,
+ struct spi_mem_op *op,
+ const enum spi_nor_protocol proto);
int spi_nor_write_enable(struct spi_nor *nor);
int spi_nor_write_disable(struct spi_nor *nor);
int spi_nor_set_4byte_addr_mode(struct spi_nor *nor, bool enable);
diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
index f6038d3a3684..f917631c8110 100644
--- a/drivers/mtd/spi-nor/sfdp.c
+++ b/drivers/mtd/spi-nor/sfdp.c
@@ -1044,9 +1044,16 @@ static int spi_nor_parse_4bait(struct spi_nor *nor,
}

/* 4BAIT is the only SFDP table that indicates page program support. */
- if (pp_hwcaps & SNOR_HWCAPS_PP)
+ if (pp_hwcaps & SNOR_HWCAPS_PP) {
spi_nor_set_pp_settings(&params_pp[SNOR_CMD_PP],
SPINOR_OP_PP_4B, SNOR_PROTO_1_1_1);
+ /*
+ * Since xSPI Page Program opcode is backward compatible with
+ * Legacy SPI, use Legacy SPI opcode there as well.
+ */
+ spi_nor_set_pp_settings(&params_pp[SNOR_CMD_PP_8_8_8_DTR],
+ SPINOR_OP_PP_4B, SNOR_PROTO_8_8_8_DTR);
+ }
if (pp_hwcaps & SNOR_HWCAPS_PP_1_1_4)
spi_nor_set_pp_settings(&params_pp[SNOR_CMD_PP_1_1_4],
SPINOR_OP_PP_1_1_4_4B,
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index 1cc8ed5d59ed..9bb41d9401c6 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -182,6 +182,7 @@ enum spi_nor_protocol {
SNOR_PROTO_1_2_2_DTR = SNOR_PROTO_DTR(1, 2, 2),
SNOR_PROTO_1_4_4_DTR = SNOR_PROTO_DTR(1, 4, 4),
SNOR_PROTO_1_8_8_DTR = SNOR_PROTO_DTR(1, 8, 8),
+ SNOR_PROTO_8_8_8_DTR = SNOR_PROTO_DTR(8, 8, 8),
};

static inline bool spi_nor_protocol_is_dtr(enum spi_nor_protocol proto)
@@ -228,7 +229,7 @@ struct spi_nor_hwcaps {
* then Quad SPI protocols before Dual SPI protocols, Fast Read and lastly
* (Slow) Read.
*/
-#define SNOR_HWCAPS_READ_MASK GENMASK(14, 0)
+#define SNOR_HWCAPS_READ_MASK GENMASK(15, 0)
#define SNOR_HWCAPS_READ BIT(0)
#define SNOR_HWCAPS_READ_FAST BIT(1)
#define SNOR_HWCAPS_READ_1_1_1_DTR BIT(2)
@@ -245,11 +246,12 @@ struct spi_nor_hwcaps {
#define SNOR_HWCAPS_READ_4_4_4 BIT(9)
#define SNOR_HWCAPS_READ_1_4_4_DTR BIT(10)

-#define SNOR_HWCAPS_READ_OCTAL GENMASK(14, 11)
+#define SNOR_HWCAPS_READ_OCTAL GENMASK(15, 11)
#define SNOR_HWCAPS_READ_1_1_8 BIT(11)
#define SNOR_HWCAPS_READ_1_8_8 BIT(12)
#define SNOR_HWCAPS_READ_8_8_8 BIT(13)
#define SNOR_HWCAPS_READ_1_8_8_DTR BIT(14)
+#define SNOR_HWCAPS_READ_8_8_8_DTR BIT(15)

/*
* Page Program capabilities.
@@ -260,18 +262,19 @@ struct spi_nor_hwcaps {
* JEDEC/SFDP standard to define them. Also at this moment no SPI flash memory
* implements such commands.
*/
-#define SNOR_HWCAPS_PP_MASK GENMASK(22, 16)
-#define SNOR_HWCAPS_PP BIT(16)
+#define SNOR_HWCAPS_PP_MASK GENMASK(23, 16)
+#define SNOR_HWCAPS_PP BIT(16)

-#define SNOR_HWCAPS_PP_QUAD GENMASK(19, 17)
-#define SNOR_HWCAPS_PP_1_1_4 BIT(17)
-#define SNOR_HWCAPS_PP_1_4_4 BIT(18)
-#define SNOR_HWCAPS_PP_4_4_4 BIT(19)
+#define SNOR_HWCAPS_PP_QUAD GENMASK(19, 17)
+#define SNOR_HWCAPS_PP_1_1_4 BIT(17)
+#define SNOR_HWCAPS_PP_1_4_4 BIT(18)
+#define SNOR_HWCAPS_PP_4_4_4 BIT(19)

-#define SNOR_HWCAPS_PP_OCTAL GENMASK(22, 20)
-#define SNOR_HWCAPS_PP_1_1_8 BIT(20)
-#define SNOR_HWCAPS_PP_1_8_8 BIT(21)
-#define SNOR_HWCAPS_PP_8_8_8 BIT(22)
+#define SNOR_HWCAPS_PP_OCTAL GENMASK(23, 20)
+#define SNOR_HWCAPS_PP_1_1_8 BIT(20)
+#define SNOR_HWCAPS_PP_1_8_8 BIT(21)
+#define SNOR_HWCAPS_PP_8_8_8 BIT(22)
+#define SNOR_HWCAPS_PP_8_8_8_DTR BIT(23)

#define SNOR_HWCAPS_X_X_X (SNOR_HWCAPS_READ_2_2_2 | \
SNOR_HWCAPS_READ_4_4_4 | \
@@ -282,7 +285,8 @@ struct spi_nor_hwcaps {
#define SNOR_HWCAPS_DTR (SNOR_HWCAPS_READ_1_1_1_DTR | \
SNOR_HWCAPS_READ_1_2_2_DTR | \
SNOR_HWCAPS_READ_1_4_4_DTR | \
- SNOR_HWCAPS_READ_1_8_8_DTR)
+ SNOR_HWCAPS_READ_1_8_8_DTR | \
+ SNOR_HWCAPS_READ_8_8_8_DTR)

#define SNOR_HWCAPS_ALL (SNOR_HWCAPS_READ_MASK | \
SNOR_HWCAPS_PP_MASK)
@@ -318,6 +322,22 @@ struct spi_nor_controller_ops {
int (*erase)(struct spi_nor *nor, loff_t offs);
};

+/**
+ * enum spi_nor_cmd_ext - describes the command opcode extension in DTR mode
+ * @SPI_NOR_EXT_NONE: no extension. This is the default, and is used in Legacy
+ * SPI mode
+ * @SPI_NOR_EXT_REPEAT: the extension is same as the opcode
+ * @SPI_NOR_EXT_INVERT: the extension is the bitwise inverse of the opcode
+ * @SPI_NOR_EXT_HEX: the extension is any hex value. The command and opcode
+ * combine to form a 16-bit opcode.
+ */
+enum spi_nor_cmd_ext {
+ SPI_NOR_EXT_NONE = 0,
+ SPI_NOR_EXT_REPEAT,
+ SPI_NOR_EXT_INVERT,
+ SPI_NOR_EXT_HEX,
+};
+
/*
* Forward declarations that are used internally by the core and manufacturer
* drivers.
@@ -345,6 +365,7 @@ struct spi_nor_flash_parameter;
* @program_opcode: the program opcode
* @sst_write_second: used by the SST write operation
* @flags: flag options for the current SPI-NOR (SNOR_F_*)
+ * @cmd_ext_type: the command opcode extension type for DTR mode.
* @read_proto: the SPI protocol for read operations
* @write_proto: the SPI protocol for write operations
* @reg_proto the SPI protocol for read_reg/write_reg/erase operations
@@ -376,6 +397,7 @@ struct spi_nor {
enum spi_nor_protocol reg_proto;
bool sst_write_second;
u32 flags;
+ enum spi_nor_cmd_ext cmd_ext_type;

const struct spi_nor_controller_ops *controller_ops;

--
2.25.0

2020-04-24 18:48:07

by Pratyush Yadav

[permalink] [raw]
Subject: [PATCH v4 09/16] mtd: spi-nor: use dummy cycle and address width info from SFDP

The xSPI Profile 1.0 table specifies how many dummy cycles and address
bytes are needed for the Read Status Register command in octal DTR mode.
Use that information to send the correct Read SR command.

Signed-off-by: Pratyush Yadav <[email protected]>
---
drivers/mtd/spi-nor/core.c | 25 +++++++++++++++++++++++--
1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 9c92fc897fb4..3d181c7deda7 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -357,6 +357,8 @@ int spi_nor_write_disable(struct spi_nor *nor)
static int spi_nor_read_sr(struct spi_nor *nor, u8 *sr)
{
int ret;
+ u8 addr_bytes = nor->params->rdsr_addr_nbytes;
+ u8 dummy = nor->params->rdsr_dummy;

if (nor->spimem) {
struct spi_mem_op op =
@@ -365,10 +367,21 @@ static int spi_nor_read_sr(struct spi_nor *nor, u8 *sr)
SPI_MEM_OP_NO_DUMMY,
SPI_MEM_OP_DATA_IN(1, sr, 1));

+ if (spi_nor_protocol_is_dtr(nor->reg_proto)) {
+ op.addr.nbytes = addr_bytes;
+ op.addr.val = 0;
+ op.dummy.nbytes = dummy;
+ }
+
+ spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
+
ret = spi_mem_exec_op(nor->spimem, &op);
} else {
- ret = nor->controller_ops->read_reg(nor, SPINOR_OP_RDSR,
- sr, 1);
+ if (spi_nor_protocol_is_dtr(nor->reg_proto))
+ ret = -ENOTSUPP;
+ else
+ ret = nor->controller_ops->read_reg(nor, SPINOR_OP_RDSR,
+ sr, 1);
}

if (ret)
@@ -388,6 +401,8 @@ static int spi_nor_read_sr(struct spi_nor *nor, u8 *sr)
static int spi_nor_read_fsr(struct spi_nor *nor, u8 *fsr)
{
int ret;
+ u8 addr_bytes = nor->params->rdsr_addr_nbytes;
+ u8 dummy = nor->params->rdsr_dummy;

if (nor->spimem) {
struct spi_mem_op op =
@@ -396,6 +411,12 @@ static int spi_nor_read_fsr(struct spi_nor *nor, u8 *fsr)
SPI_MEM_OP_NO_DUMMY,
SPI_MEM_OP_DATA_IN(1, fsr, 1));

+ if (spi_nor_protocol_is_dtr(nor->reg_proto)) {
+ op.addr.nbytes = addr_bytes;
+ op.addr.val = 0;
+ op.dummy.nbytes = dummy;
+ }
+
spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);

ret = spi_mem_exec_op(nor->spimem, &op);
--
2.25.0

2020-04-24 18:48:20

by Pratyush Yadav

[permalink] [raw]
Subject: [PATCH v4 13/16] mtd: spi-nor: Disable Octal DTR mode on suspend.

On resume, the init procedure will be run that will re-enable it.

Signed-off-by: Pratyush Yadav <[email protected]>
---
drivers/mtd/spi-nor/core.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index b0f94d6d28bf..b9b9c2b9a3da 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -3227,6 +3227,23 @@ static void spi_nor_soft_reset(struct spi_nor *nor)
usleep_range(SPI_NOR_SRST_SLEEP_MIN, SPI_NOR_SRST_SLEEP_MAX);
}

+/* mtd suspend handler */
+static int spi_nor_suspend(struct mtd_info *mtd)
+{
+ struct spi_nor *nor = mtd_to_spi_nor(mtd);
+ struct device *dev = nor->dev;
+ int ret;
+
+ /* Disable octal DTR mode if we enabled it. */
+ ret = spi_nor_octal_dtr_enable(nor, false);
+ if (ret) {
+ dev_err(dev, "suspend() failed\n");
+ return ret;
+ }
+
+ return 0;
+}
+
/* mtd resume handler */
static void spi_nor_resume(struct mtd_info *mtd)
{
@@ -3420,6 +3437,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
mtd->size = nor->params->size;
mtd->_erase = spi_nor_erase;
mtd->_read = spi_nor_read;
+ mtd->_suspend = spi_nor_suspend;
mtd->_resume = spi_nor_resume;

if (nor->params->locking_ops) {
--
2.25.0

2020-04-24 18:48:35

by Pratyush Yadav

[permalink] [raw]
Subject: [PATCH v4 14/16] mtd: spi-nor: expose spi_nor_default_setup() in core.h

Flashes might want to add a custom setup hook to configure the flash in
the proper mode for operation. But after that, they would still want to
run the default setup hook because it selects the read, program, and
erase operations. Since there is little point in repeating all that
code, expose the spi_nor_default_setup() in core.h to
manufacturer-specific files.

Signed-off-by: Pratyush Yadav <[email protected]>
---
drivers/mtd/spi-nor/core.c | 4 ++--
drivers/mtd/spi-nor/core.h | 3 +++
2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index b9b9c2b9a3da..cfd78aeacc88 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -2785,8 +2785,8 @@ static int spi_nor_select_erase(struct spi_nor *nor)
return 0;
}

-static int spi_nor_default_setup(struct spi_nor *nor,
- const struct spi_nor_hwcaps *hwcaps)
+int spi_nor_default_setup(struct spi_nor *nor,
+ const struct spi_nor_hwcaps *hwcaps)
{
struct spi_nor_flash_parameter *params = nor->params;
u32 ignored_mask, shared_mask;
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index 86c31357b321..00bf6b35ec3e 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -453,6 +453,9 @@ int spi_nor_post_bfpt_fixups(struct spi_nor *nor,
const struct sfdp_bfpt *bfpt,
struct spi_nor_flash_parameter *params);

+int spi_nor_default_setup(struct spi_nor *nor,
+ const struct spi_nor_hwcaps *hwcaps);
+
static struct spi_nor __maybe_unused *mtd_to_spi_nor(struct mtd_info *mtd)
{
return mtd->priv;
--
2.25.0

2020-04-24 18:48:36

by Pratyush Yadav

[permalink] [raw]
Subject: [PATCH v4 16/16] mtd: spi-nor: allow using MT35XU512ABA in Octal DTR mode

Since this flash doesn't have a Profile 1.0 table, the Octal DTR
capabilities are enabled in the post SFDP fixup, along with the 8D-8D-8D
fast read settings.

Enable Octal DTR mode with 20 dummy cycles to allow running at the
maximum supported frequency of 200Mhz.

The flash supports the soft reset sequence. So, add the flag in the
flash's info.

Signed-off-by: Pratyush Yadav <[email protected]>
---
drivers/mtd/spi-nor/micron-st.c | 113 +++++++++++++++++++++++++++++++-
1 file changed, 112 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/spi-nor/micron-st.c b/drivers/mtd/spi-nor/micron-st.c
index 02c0b53f6097..5a1f0428032f 100644
--- a/drivers/mtd/spi-nor/micron-st.c
+++ b/drivers/mtd/spi-nor/micron-st.c
@@ -8,10 +8,121 @@

#include "core.h"

+#define SPINOR_OP_MT_DTR_RD 0xfd /* Fast Read opcode in DTR mode */
+#define SPINOR_OP_MT_RD_ANY_REG 0x85 /* Read volatile register */
+#define SPINOR_OP_MT_WR_ANY_REG 0x81 /* Write volatile register */
+#define SPINOR_REG_MT_CFR0V 0x00 /* For setting octal DTR mode */
+#define SPINOR_REG_MT_CFR1V 0x01 /* For setting dummy cycles */
+#define SPINOR_MT_DTR_NO_DQS 0xc7 /* Enable Octal DTR without DQS. */
+#define SPINOR_MT_EXSPI 0xff /* Enable Extended SPI (default) */
+
+static int spi_nor_micron_octal_dtr_enable(struct spi_nor *nor, bool enable)
+{
+ struct spi_mem_op op;
+ u8 *buf = nor->bouncebuf;
+ u8 addr_width;
+ int ret;
+
+ if (enable)
+ addr_width = 3;
+ else
+ addr_width = 4;
+
+ ret = spi_nor_write_enable(nor);
+ if (ret)
+ return ret;
+
+ if (enable)
+ *buf = SPINOR_MT_DTR_NO_DQS;
+ else
+ *buf = SPINOR_MT_EXSPI;
+ op = (struct spi_mem_op)
+ SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_MT_WR_ANY_REG, 1),
+ SPI_MEM_OP_ADDR(addr_width, SPINOR_REG_MT_CFR0V, 1),
+ SPI_MEM_OP_NO_DUMMY,
+ SPI_MEM_OP_DATA_OUT(1, 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);
+ if (ret) {
+ dev_err(nor->dev, "Failed to enable octal DTR mode\n");
+ return ret;
+ }
+
+ return 0;
+}
+
+static int mt35xu512aba_setup(struct spi_nor *nor,
+ const struct spi_nor_hwcaps *hwcaps)
+{
+ struct spi_mem_op op;
+ u8 *buf = nor->bouncebuf;
+ u8 addr_width = 3;
+ int ret;
+
+ if (!nor->spimem) {
+ dev_err(nor->dev,
+ "operation not supported for non-spimem drivers\n");
+ return -ENOTSUPP;
+ }
+
+ /* Set dummy cycles for Fast Read to the default of 20. */
+ 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(addr_width, 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;
+
+
+ return spi_nor_default_setup(nor, hwcaps);
+}
+
+static void mt35xu512aba_default_init(struct spi_nor *nor)
+{
+ nor->params->octal_dtr_enable = spi_nor_micron_octal_dtr_enable;
+ nor->params->setup = mt35xu512aba_setup;
+}
+
+static void mt35xu512aba_post_sfdp_fixup(struct spi_nor *nor)
+{
+ /* Set the Fast Read settings. */
+ nor->params->hwcaps.mask |= SNOR_HWCAPS_READ_8_8_8_DTR;
+ spi_nor_set_read_settings(&nor->params->reads[SNOR_CMD_READ_8_8_8_DTR],
+ 0, 20, SPINOR_OP_MT_DTR_RD,
+ SNOR_PROTO_8_8_8_DTR);
+
+ nor->params->hwcaps.mask |= SNOR_HWCAPS_PP_8_8_8_DTR;
+
+ nor->cmd_ext_type = SPI_NOR_EXT_REPEAT;
+ nor->params->rdsr_dummy = 8;
+ nor->params->rdsr_addr_nbytes = 0;
+}
+
+static struct spi_nor_fixups mt35xu512aba_fixups = {
+ .default_init = mt35xu512aba_default_init,
+ .post_sfdp = mt35xu512aba_post_sfdp_fixup,
+};
+
static const struct flash_info micron_parts[] = {
{ "mt35xu512aba", INFO(0x2c5b1a, 0, 128 * 1024, 512,
SECT_4K | USE_FSR | SPI_NOR_OCTAL_READ |
- SPI_NOR_4B_OPCODES) },
+ SPI_NOR_4B_OPCODES | SPI_NOR_OCTAL_DTR_READ |
+ SPI_NOR_SOFT_RESET)
+ .fixups = &mt35xu512aba_fixups},
{ "mt35xu02g", INFO(0x2c5b1c, 0, 128 * 1024, 2048,
SECT_4K | USE_FSR | SPI_NOR_OCTAL_READ |
SPI_NOR_4B_OPCODES) },
--
2.25.0

2020-04-24 18:49:22

by Pratyush Yadav

[permalink] [raw]
Subject: [PATCH v4 11/16] mtd: spi-nor: enable octal DTR mode when possible

Allow flashes to specify a hook to enable octal DTR mode. Use this hook
whenever possible to get optimal transfer speeds.

Signed-off-by: Pratyush Yadav <[email protected]>
---
drivers/mtd/spi-nor/core.c | 35 +++++++++++++++++++++++++++++++++++
drivers/mtd/spi-nor/core.h | 2 ++
2 files changed, 37 insertions(+)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 6eb2b1781972..c75765f09d62 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -3085,6 +3085,35 @@ static int spi_nor_init_params(struct spi_nor *nor)
return 0;
}

+/** spi_nor_octal_dtr_enable() - enable Octal DTR I/O if needed
+ * @nor: pointer to a 'struct spi_nor'
+ * @enable: whether to enable or disable Octal DTR
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int spi_nor_octal_dtr_enable(struct spi_nor *nor, bool enable)
+{
+ int ret;
+
+ if (!nor->params->octal_dtr_enable)
+ return 0;
+
+ if (!(spi_nor_get_protocol_width(nor->read_proto) == 8 ||
+ spi_nor_get_protocol_width(nor->write_proto) == 8))
+ return 0;
+
+ ret = nor->params->octal_dtr_enable(nor, enable);
+ if (ret)
+ return ret;
+
+ if (enable)
+ nor->reg_proto = SNOR_PROTO_8_8_8_DTR;
+ else
+ nor->reg_proto = SNOR_PROTO_1_1_1;
+
+ return 0;
+}
+
/**
* spi_nor_quad_enable() - enable Quad I/O if needed.
* @nor: pointer to a 'struct spi_nor'
@@ -3124,6 +3153,12 @@ static int spi_nor_init(struct spi_nor *nor)
{
int err;

+ err = spi_nor_octal_dtr_enable(nor, true);
+ if (err) {
+ dev_dbg(nor->dev, "octal mode not supported\n");
+ return err;
+ }
+
err = spi_nor_quad_enable(nor);
if (err) {
dev_dbg(nor->dev, "quad mode not supported\n");
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index 7e6df8322da0..6338d32a0d77 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -203,6 +203,7 @@ struct spi_nor_locking_ops {
* higher index in the array, the higher priority.
* @erase_map: the erase map parsed from the SFDP Sector Map Parameter
* Table.
+ * @octal_dtr_enable: enables SPI NOR octal DTR mode.
* @quad_enable: enables SPI NOR quad mode.
* @set_4byte_addr_mode: puts the SPI NOR in 4 byte addressing mode.
* @convert_addr: converts an absolute address into something the flash
@@ -226,6 +227,7 @@ struct spi_nor_flash_parameter {

struct spi_nor_erase_map erase_map;

+ int (*octal_dtr_enable)(struct spi_nor *nor, bool enable);
int (*quad_enable)(struct spi_nor *nor);
int (*set_4byte_addr_mode)(struct spi_nor *nor, bool enable);
u32 (*convert_addr)(struct spi_nor *nor, u32 addr);
--
2.25.0

2020-04-24 18:49:46

by Pratyush Yadav

[permalink] [raw]
Subject: [PATCH v4 12/16] mtd: spi-nor: perform a Soft Reset on shutdown

A Software Reset sequence will return the flash to Power-on-Reset (POR)
state. It consists of two commands: Soft Reset Enable and Soft Reset.

Perform a Soft Reset on shutdown on flashes that support it so that the
flash can be reset to its initial state and any configurations made by
spi-nor (given that they're only done in volatile registers) will be
reset. This will hand back the flash in pristine state for any further
operations on it.

Signed-off-by: Pratyush Yadav <[email protected]>
---
drivers/mtd/spi-nor/core.c | 45 +++++++++++++++++++++++++++++++++++++
drivers/mtd/spi-nor/core.h | 2 ++
include/linux/mtd/spi-nor.h | 2 ++
3 files changed, 49 insertions(+)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index c75765f09d62..b0f94d6d28bf 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -40,6 +40,9 @@

#define SPI_NOR_MAX_ADDR_WIDTH 4

+#define SPI_NOR_SRST_SLEEP_MIN 200
+#define SPI_NOR_SRST_SLEEP_MAX 400
+
/**
* spi_nor_get_cmd_ext() - Get the command opcode extension based on the
* extension type.
@@ -3189,6 +3192,41 @@ static int spi_nor_init(struct spi_nor *nor)
return 0;
}

+static void spi_nor_soft_reset(struct spi_nor *nor)
+{
+ struct spi_mem_op op;
+ int ret;
+
+ op = (struct spi_mem_op)SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_SRSTEN, 8),
+ SPI_MEM_OP_NO_DUMMY,
+ SPI_MEM_OP_NO_ADDR,
+ SPI_MEM_OP_NO_DATA);
+ spi_nor_spimem_setup_op(nor, &op, SNOR_PROTO_8_8_8_DTR);
+ ret = spi_mem_exec_op(nor->spimem, &op);
+ if (ret) {
+ dev_warn(nor->dev, "Software reset failed: %d\n", ret);
+ return;
+ }
+
+ op = (struct spi_mem_op)SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_SRST, 8),
+ SPI_MEM_OP_NO_DUMMY,
+ SPI_MEM_OP_NO_ADDR,
+ SPI_MEM_OP_NO_DATA);
+ spi_nor_spimem_setup_op(nor, &op, SNOR_PROTO_8_8_8_DTR);
+ ret = spi_mem_exec_op(nor->spimem, &op);
+ if (ret) {
+ dev_warn(nor->dev, "Software reset failed: %d\n", ret);
+ return;
+ }
+
+ /*
+ * Software Reset is not instant, and the delay varies from flash to
+ * flash. Looking at a few flashes, most range somewhere below 100
+ * microseconds. So, sleep for a range of 200-400 us.
+ */
+ usleep_range(SPI_NOR_SRST_SLEEP_MIN, SPI_NOR_SRST_SLEEP_MAX);
+}
+
/* mtd resume handler */
static void spi_nor_resume(struct mtd_info *mtd)
{
@@ -3208,6 +3246,10 @@ void spi_nor_restore(struct spi_nor *nor)
if (nor->addr_width == 4 && !(nor->flags & SNOR_F_4B_OPCODES) &&
nor->flags & SNOR_F_BROKEN_RESET)
nor->params->set_4byte_addr_mode(nor, false);
+
+ if (nor->info->flags & SPI_NOR_OCTAL_DTR_READ &&
+ nor->flags & SNOR_F_SOFT_RESET)
+ spi_nor_soft_reset(nor);
}
EXPORT_SYMBOL_GPL(spi_nor_restore);

@@ -3394,6 +3436,9 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
nor->flags |= SNOR_F_HAS_SR_TB_BIT6;
}

+ if (info->flags & SPI_NOR_SOFT_RESET)
+ nor->flags |= SNOR_F_SOFT_RESET;
+
if (info->flags & NO_CHIP_ERASE)
nor->flags |= SNOR_F_NO_OP_CHIP_ERASE;
if (info->flags & USE_CLSR)
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index 6338d32a0d77..86c31357b321 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -26,6 +26,7 @@ enum spi_nor_option_flags {
SNOR_F_HAS_SR_TB_BIT6 = BIT(11),
SNOR_F_HAS_4BIT_BP = BIT(12),
SNOR_F_HAS_SR_BP3_BIT6 = BIT(13),
+ SNOR_F_SOFT_RESET = BIT(14),
};

struct spi_nor_read_command {
@@ -321,6 +322,7 @@ struct flash_info {
* Must be used with SPI_NOR_4BIT_BP.
*/
#define SPI_NOR_OCTAL_DTR_READ BIT(19) /* Flash supports octal DTR Read. */
+#define SPI_NOR_SOFT_RESET BIT(20) /* Flash supports soft reset command */

/* Part specific fixup hooks. */
const struct spi_nor_fixups *fixups;
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index 9bb41d9401c6..7b828ee1c036 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -51,6 +51,8 @@
#define SPINOR_OP_CLFSR 0x50 /* Clear flag status register */
#define SPINOR_OP_RDEAR 0xc8 /* Read Extended Address Register */
#define SPINOR_OP_WREAR 0xc5 /* Write Extended Address Register */
+#define SPINOR_OP_SRSTEN 0x66 /* Software Reset Enable */
+#define SPINOR_OP_SRST 0x99 /* Software Reset */

/* 4-byte address opcodes - used on Spansion and some Macronix flashes. */
#define SPINOR_OP_READ_4B 0x13 /* Read data bytes (low frequency) */
--
2.25.0

2020-04-24 18:50:27

by Pratyush Yadav

[permalink] [raw]
Subject: [PATCH v4 15/16] mtd: spi-nor: add support for Cypress Semper flash

The Cypress Semper flash is an xSPI compliant octal DTR flash. Add
support for using it in octal DTR mode.

The flash by default boots in a hybrid sector mode. But the sector map
table on the part I had was programmed incorrectly and the SMPT values
on the flash don't match the public datasheet. Specifically, in some
places erase type 3 was used instead of 4. In addition, the region sizes
were incorrect in some places. So, for testing I set CFR3N[3] to enable
uniform sector sizes. Since the uniform sector mode bit is a
non-volatile bit, this series does not change it to avoid making any
permanent changes to the flash configuration. The correct data to
implement a fixup is not available right now and will be done in a
follow-up patch if needed.

Signed-off-by: Pratyush Yadav <[email protected]>
---
drivers/mtd/spi-nor/spansion.c | 167 +++++++++++++++++++++++++++++++++
1 file changed, 167 insertions(+)

diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
index 88183eba8ac1..cc75134f579c 100644
--- a/drivers/mtd/spi-nor/spansion.c
+++ b/drivers/mtd/spi-nor/spansion.c
@@ -8,6 +8,168 @@

#include "core.h"

+/* For Cypress flash. */
+#define SPINOR_OP_RD_ANY_REG 0x65 /* Read any register */
+#define SPINOR_OP_WR_ANY_REG 0x71 /* Write any register */
+#define SPINOR_REG_CYPRESS_CFR2V 0x00800003
+#define SPINOR_REG_CYPRESS_CFR2V_MEMLAT_8_20 0x8
+#define SPINOR_REG_CYPRESS_CFR3V 0x00800004
+#define SPINOR_REG_CYPRESS_CFR3V_PGSZ BIT(4) /* Page size. */
+#define SPINOR_REG_CYPRESS_CFR4V 0x00800005
+#define SPINOR_REG_CYPRESS_CFR5V 0x00800006
+#define SPINOR_REG_CYPRESS_CFR5V_OCT_DTR_EN 0x3
+#define SPINOR_REG_CYPRESS_CFR5V_OCT_DTR_DS 0
+#define SPINOR_OP_CYPRESS_RD_FAST 0xee
+
+/**
+ * spi_nor_cypress_octal_enable() - Enable octal DTR mode on Cypress flashes.
+ * @nor: pointer to a 'struct spi_nor'
+ *
+ * This also sets the memory access latency cycles to 20, which is the default
+ * in the spi-nor framework.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int spi_nor_cypress_octal_enable(struct spi_nor *nor, bool enable)
+{
+ struct spi_mem_op op;
+ u8 *buf = nor->bouncebuf;
+ u8 addr_width;
+ int ret;
+
+ if (enable)
+ addr_width = 3;
+ else
+ addr_width = 4;
+
+ if (enable) {
+ /* Use 20 dummy cycles for memory array reads. */
+ ret = spi_nor_write_enable(nor);
+ if (ret)
+ return ret;
+
+ *buf = SPINOR_REG_CYPRESS_CFR2V_MEMLAT_8_20;
+ op = (struct spi_mem_op)
+ SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WR_ANY_REG, 1),
+ SPI_MEM_OP_ADDR(addr_width,
+ SPINOR_REG_CYPRESS_CFR2V,
+ 1),
+ SPI_MEM_OP_NO_DUMMY,
+ SPI_MEM_OP_DATA_OUT(1, buf, 1));
+ ret = spi_mem_exec_op(nor->spimem, &op);
+ if (ret) {
+ dev_warn(nor->dev,
+ "failed to set default memory latency value: %d\n",
+ ret);
+ return ret;
+ }
+ ret = spi_nor_wait_till_ready(nor);
+ if (ret)
+ return ret;
+
+ nor->read_dummy = 20;
+ }
+
+ /* Set/unset the octal and DTR enable bits. */
+ ret = spi_nor_write_enable(nor);
+ if (ret)
+ return ret;
+
+ if (enable)
+ *buf = SPINOR_REG_CYPRESS_CFR5V_OCT_DTR_EN;
+ else
+ *buf = SPINOR_REG_CYPRESS_CFR5V_OCT_DTR_DS;
+ op = (struct spi_mem_op)
+ SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WR_ANY_REG, 1),
+ SPI_MEM_OP_ADDR(addr_width,
+ SPINOR_REG_CYPRESS_CFR5V,
+ 1),
+ SPI_MEM_OP_NO_DUMMY,
+ SPI_MEM_OP_DATA_OUT(1, 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);
+ if (ret) {
+ dev_warn(nor->dev, "Failed to enable octal DTR mode\n");
+ return ret;
+ }
+
+ return 0;
+}
+
+static void s28hs512t_default_init(struct spi_nor *nor)
+{
+ nor->params->octal_dtr_enable = spi_nor_cypress_octal_enable;
+}
+
+static void s28hs512t_post_sfdp_fixup(struct spi_nor *nor)
+{
+ /*
+ * On older versions of the flash the xSPI Profile 1.0 table has the
+ * 8D-8D-8D Fast Read opcode as 0x00. But it actually should be 0xEE.
+ */
+ if (nor->params->reads[SNOR_CMD_READ_8_8_8_DTR].opcode == 0)
+ nor->params->reads[SNOR_CMD_READ_8_8_8_DTR].opcode =
+ SPINOR_OP_CYPRESS_RD_FAST;
+
+ /* This flash is also missing the 4-byte Page Program opcode bit. */
+ spi_nor_set_pp_settings(&nor->params->page_programs[SNOR_CMD_PP],
+ SPINOR_OP_PP_4B, SNOR_PROTO_1_1_1);
+ /*
+ * Since xSPI Page Program opcode is backward compatible with
+ * Legacy SPI, use Legacy SPI opcode there as well.
+ */
+ spi_nor_set_pp_settings(&nor->params->page_programs[SNOR_CMD_PP_8_8_8_DTR],
+ SPINOR_OP_PP_4B, SNOR_PROTO_8_8_8_DTR);
+
+ /*
+ * The xSPI Profile 1.0 table advertises the number of additional
+ * address bytes needed for Read Status Register command as 0 but the
+ * 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,
+ struct spi_nor_flash_parameter *params)
+{
+ struct spi_mem_op op;
+ u8 *buf = nor->bouncebuf;
+ u8 addr_width = 3;
+ int ret;
+
+ /*
+ * The BFPT table advertises a 512B page size but the page size is
+ * actually configurable (with the default being 256B). Read from
+ * CFR3V[4] and set the correct size.
+ */
+ op = (struct spi_mem_op)
+ SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_RD_ANY_REG, 1),
+ SPI_MEM_OP_ADDR(addr_width, SPINOR_REG_CYPRESS_CFR3V, 1),
+ SPI_MEM_OP_NO_DUMMY,
+ SPI_MEM_OP_DATA_IN(1, buf, 1));
+ ret = spi_mem_exec_op(nor->spimem, &op);
+ if (ret)
+ return ret;
+
+ if (*buf & SPINOR_REG_CYPRESS_CFR3V_PGSZ)
+ params->page_size = 512;
+ else
+ params->page_size = 256;
+
+ return 0;
+}
+
+static struct spi_nor_fixups s28hs512t_fixups = {
+ .default_init = s28hs512t_default_init,
+ .post_sfdp = s28hs512t_post_sfdp_fixup,
+ .post_bfpt = s28hs512t_post_bfpt_fixup,
+};
+
static const struct flash_info spansion_parts[] = {
/* Spansion/Cypress -- single (large) sector size only, at least
* for the chips listed here (without boot sectors).
@@ -72,6 +234,11 @@ static const struct flash_info spansion_parts[] = {
{ "s25fl256l", INFO(0x016019, 0, 64 * 1024, 512,
SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
SPI_NOR_4B_OPCODES) },
+ { "s28hs512t", INFO(0x345b1a, 0, 256 * 1024, 256,
+ SECT_4K | SPI_NOR_OCTAL_DTR_READ |
+ SPI_NOR_SOFT_RESET)
+ .fixups = &s28hs512t_fixups,
+ },
};

static void spansion_post_sfdp_fixups(struct spi_nor *nor)
--
2.25.0

2020-04-24 20:35:34

by Pratyush Yadav

[permalink] [raw]
Subject: [PATCH v4 10/16] mtd: spi-nor: do 2 byte reads for SR and FSR in DTR mode

Some controllers, like the cadence qspi controller, have trouble reading
only 1 byte in DTR mode. So, do 2 byte reads for SR and FSR commands in
DTR mode, and then discard the second byte.

Signed-off-by: Pratyush Yadav <[email protected]>
---
drivers/mtd/spi-nor/core.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 3d181c7deda7..6eb2b1781972 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -350,7 +350,7 @@ int spi_nor_write_disable(struct spi_nor *nor)
* spi_nor_read_sr() - Read the Status Register.
* @nor: pointer to 'struct spi_nor'.
* @sr: pointer to a DMA-able buffer where the value of the
- * Status Register will be written.
+ * Status Register will be written. Should be at least 2 bytes.
*
* Return: 0 on success, -errno otherwise.
*/
@@ -371,6 +371,11 @@ static int spi_nor_read_sr(struct spi_nor *nor, u8 *sr)
op.addr.nbytes = addr_bytes;
op.addr.val = 0;
op.dummy.nbytes = dummy;
+ /*
+ * We don't want to read only one byte in DTR mode. So,
+ * read 2 and then discard the second byte.
+ */
+ op.data.nbytes = 2;
}

spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
@@ -394,7 +399,8 @@ static int spi_nor_read_sr(struct spi_nor *nor, u8 *sr)
* spi_nor_read_fsr() - Read the Flag Status Register.
* @nor: pointer to 'struct spi_nor'
* @fsr: pointer to a DMA-able buffer where the value of the
- * Flag Status Register will be written.
+ * Flag Status Register will be written. Should be at least 2
+ * bytes.
*
* Return: 0 on success, -errno otherwise.
*/
@@ -415,6 +421,11 @@ static int spi_nor_read_fsr(struct spi_nor *nor, u8 *fsr)
op.addr.nbytes = addr_bytes;
op.addr.val = 0;
op.dummy.nbytes = dummy;
+ /*
+ * We don't want to read only one byte in DTR mode. So,
+ * read 2 and then discard the second byte.
+ */
+ op.data.nbytes = 2;
}

spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
--
2.25.0

2020-04-26 03:56:21

by Yicong Yang

[permalink] [raw]
Subject: Re: [PATCH v4 05/16] mtd: spi-nor: default to address width of 3 for configurable widths

On 2020/4/25 2:43, Pratyush Yadav wrote:
> JESD216D.01 says that when the address width can be 3 or 4, it defaults
> to 3 and enters 4-byte mode when given the appropriate command. So, when
> we see a configurable width, default to 3 and let flash that default to
> 4 change it in a post-bfpt fixup.
>
> This fixes SMPT parsing for flashes with configurable address width. If
> the SMPT descriptor advertises variable address width, we use
> nor->addr_width as the address width. But since it was not set to any
> value from the SFDP table, the read command uses an address width of 0,
> resulting in an incorrect read being issued.
>
> Signed-off-by: Pratyush Yadav <[email protected]>
> ---
> drivers/mtd/spi-nor/sfdp.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
> index f917631c8110..5cecc4ba2141 100644
> --- a/drivers/mtd/spi-nor/sfdp.c
> +++ b/drivers/mtd/spi-nor/sfdp.c
> @@ -460,6 +460,7 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
> /* Number of address bytes. */
> switch (bfpt.dwords[BFPT_DWORD(1)] & BFPT_DWORD1_ADDRESS_BYTES_MASK) {
> case BFPT_DWORD1_ADDRESS_BYTES_3_ONLY:
> + case BFPT_DWORD1_ADDRESS_BYTES_3_OR_4:
> nor->addr_width = 3;
> break;

Should we also assign address width to 3 in default condition. At least we should not
leave it uninitialized here.

Regards,
Yicong


>

2020-04-27 17:28:27

by Pratyush Yadav

[permalink] [raw]
Subject: Re: [PATCH v4 05/16] mtd: spi-nor: default to address width of 3 for configurable widths

Hi Yicong,

On 26/04/20 11:53AM, Yicong Yang wrote:
> On 2020/4/25 2:43, Pratyush Yadav wrote:
> > JESD216D.01 says that when the address width can be 3 or 4, it defaults
> > to 3 and enters 4-byte mode when given the appropriate command. So, when
> > we see a configurable width, default to 3 and let flash that default to
> > 4 change it in a post-bfpt fixup.
> >
> > This fixes SMPT parsing for flashes with configurable address width. If
> > the SMPT descriptor advertises variable address width, we use
> > nor->addr_width as the address width. But since it was not set to any
> > value from the SFDP table, the read command uses an address width of 0,
> > resulting in an incorrect read being issued.
> >
> > Signed-off-by: Pratyush Yadav <[email protected]>
> > ---
> > drivers/mtd/spi-nor/sfdp.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
> > index f917631c8110..5cecc4ba2141 100644
> > --- a/drivers/mtd/spi-nor/sfdp.c
> > +++ b/drivers/mtd/spi-nor/sfdp.c
> > @@ -460,6 +460,7 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
> > /* Number of address bytes. */
> > switch (bfpt.dwords[BFPT_DWORD(1)] & BFPT_DWORD1_ADDRESS_BYTES_MASK) {
> > case BFPT_DWORD1_ADDRESS_BYTES_3_ONLY:
> > + case BFPT_DWORD1_ADDRESS_BYTES_3_OR_4:
> > nor->addr_width = 3;
> > break;
>
> Should we also assign address width to 3 in default condition. At least we should not
> leave it uninitialized here.

The default condition would be taken when this field is 3. The value 3
is reserved, and so no current device should use this value. That said,
I don't see any downsides of doing so. If the value 3 means something
else in later revisions of the standard, this code would need to change
anyway. If not, we would use a relatively sane default for devices with
a faulty BFPT.

I haven't received any comments on my series so far. If end up having to
re-roll it, I will add this change. Otherwise, I'm not sure if it is a
good idea to re-roll a 16-patch series for a one liner that isn't fixing
some major bug. In that case, maybe you can send an independent patch
that does this after mine is merged?

--
Regards,
Pratyush Yadav

2020-04-28 01:38:00

by Yicong Yang

[permalink] [raw]
Subject: Re: [PATCH v4 05/16] mtd: spi-nor: default to address width of 3 for configurable widths

Hi Pratyush,


On 2020/4/28 1:23, Pratyush Yadav wrote:
> Hi Yicong,
>
> On 26/04/20 11:53AM, Yicong Yang wrote:
>> On 2020/4/25 2:43, Pratyush Yadav wrote:
>>> JESD216D.01 says that when the address width can be 3 or 4, it defaults
>>> to 3 and enters 4-byte mode when given the appropriate command. So, when
>>> we see a configurable width, default to 3 and let flash that default to
>>> 4 change it in a post-bfpt fixup.
>>>
>>> This fixes SMPT parsing for flashes with configurable address width. If
>>> the SMPT descriptor advertises variable address width, we use
>>> nor->addr_width as the address width. But since it was not set to any
>>> value from the SFDP table, the read command uses an address width of 0,
>>> resulting in an incorrect read being issued.
>>>
>>> Signed-off-by: Pratyush Yadav <[email protected]>
>>> ---
>>> drivers/mtd/spi-nor/sfdp.c | 1 +
>>> 1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
>>> index f917631c8110..5cecc4ba2141 100644
>>> --- a/drivers/mtd/spi-nor/sfdp.c
>>> +++ b/drivers/mtd/spi-nor/sfdp.c
>>> @@ -460,6 +460,7 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
>>> /* Number of address bytes. */
>>> switch (bfpt.dwords[BFPT_DWORD(1)] & BFPT_DWORD1_ADDRESS_BYTES_MASK) {
>>> case BFPT_DWORD1_ADDRESS_BYTES_3_ONLY:
>>> + case BFPT_DWORD1_ADDRESS_BYTES_3_OR_4:
>>> nor->addr_width = 3;
>>> break;
>> Should we also assign address width to 3 in default condition. At least we should not
>> leave it uninitialized here.
> The default condition would be taken when this field is 3. The value 3
> is reserved, and so no current device should use this value. That said,
> I don't see any downsides of doing so. If the value 3 means something
> else in later revisions of the standard, this code would need to change
> anyway. If not, we would use a relatively sane default for devices with
> a faulty BFPT.

The purpose is to set one possible value which may be used later in parsing smpt.
In current driver, if we do nothing with the post-bfpt fixup, then the width will
be unset. Otherwise, maybe the width can also be set in spi_nor_smpt_addr_width()

default:
+ if (!nor->addr_width)
+ nor->addr_width = 3;
return nor->addr_width;

But set when parsing bfpt seems more reasonable.

> I haven't received any comments on my series so far. If end up having to
> re-roll it, I will add this change. Otherwise, I'm not sure if it is a
> good idea to re-roll a 16-patch series for a one liner that isn't fixing
> some major bug. In that case, maybe you can send an independent patch
> that does this after mine is merged?

Fine. :)

Regards,
Yicong



2020-04-28 05:28:27

by Tudor Ambarus

[permalink] [raw]
Subject: Re: [PATCH v4 05/16] mtd: spi-nor: default to address width of 3 for configurable widths

On Tuesday, April 28, 2020 4:34:46 AM EEST Yicong Yang wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
>
> Hi Pratyush,
>
> On 2020/4/28 1:23, Pratyush Yadav wrote:
> > Hi Yicong,
> >
> > On 26/04/20 11:53AM, Yicong Yang wrote:
> >> On 2020/4/25 2:43, Pratyush Yadav wrote:
> >>> JESD216D.01 says that when the address width can be 3 or 4, it defaults
> >>> to 3 and enters 4-byte mode when given the appropriate command. So, when
> >>> we see a configurable width, default to 3 and let flash that default to
> >>> 4 change it in a post-bfpt fixup.
> >>>
> >>> This fixes SMPT parsing for flashes with configurable address width. If
> >>> the SMPT descriptor advertises variable address width, we use
> >>> nor->addr_width as the address width. But since it was not set to any
> >>> value from the SFDP table, the read command uses an address width of 0,
> >>> resulting in an incorrect read being issued.
> >>>
> >>> Signed-off-by: Pratyush Yadav <[email protected]>
> >>> ---
> >>>
> >>> drivers/mtd/spi-nor/sfdp.c | 1 +
> >>> 1 file changed, 1 insertion(+)
> >>>
> >>> diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
> >>> index f917631c8110..5cecc4ba2141 100644
> >>> --- a/drivers/mtd/spi-nor/sfdp.c
> >>> +++ b/drivers/mtd/spi-nor/sfdp.c
> >>> @@ -460,6 +460,7 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
> >>>
> >>> /* Number of address bytes. */
> >>> switch (bfpt.dwords[BFPT_DWORD(1)] & BFPT_DWORD1_ADDRESS_BYTES_MASK)
> >>> {
> >>>
> >>> case BFPT_DWORD1_ADDRESS_BYTES_3_ONLY:
> >>> + case BFPT_DWORD1_ADDRESS_BYTES_3_OR_4:
> >>> nor->addr_width = 3;
> >>> break;
> >>
> >> Should we also assign address width to 3 in default condition. At least
> >> we should not leave it uninitialized here.
> >
> > The default condition would be taken when this field is 3. The value 3
> > is reserved, and so no current device should use this value. That said,
> > I don't see any downsides of doing so. If the value 3 means something
> > else in later revisions of the standard, this code would need to change
> > anyway. If not, we would use a relatively sane default for devices with
> > a faulty BFPT.
>
> The purpose is to set one possible value which may be used later in parsing
> smpt. In current driver, if we do nothing with the post-bfpt fixup, then
> the width will be unset. Otherwise, maybe the width can also be set in
> spi_nor_smpt_addr_width()
>
> default:
> + if (!nor->addr_width)
> + nor->addr_width = 3;
> return nor->addr_width;
>
> But set when parsing bfpt seems more reasonable.

Please don't. Any deviations from the standard should be addressed with fixup
hooks.

>
> > I haven't received any comments on my series so far. If end up having to

Thanks for the patience, they are in my todo list, I will get soon to them.

Cheers,
ta

2020-05-10 11:24:34

by Tudor Ambarus

[permalink] [raw]
Subject: Re: [PATCH v4 12/16] mtd: spi-nor: perform a Soft Reset on shutdown

Hi, Pratyush,

On Friday, April 24, 2020 9:44:06 PM EEST Pratyush Yadav wrote:
> A Software Reset sequence will return the flash to Power-on-Reset (POR)
> state. It consists of two commands: Soft Reset Enable and Soft Reset.
>
> Perform a Soft Reset on shutdown on flashes that support it so that the
> flash can be reset to its initial state and any configurations made by
> spi-nor (given that they're only done in volatile registers) will be
> reset. This will hand back the flash in pristine state for any further
> operations on it.

Please don't introduce SPI_NOR_SOFT_RESET yet. We should instead determine the
software reset sequence by inspecting BFPT[16], bits 13:8.

Cheers,
ta

2020-05-11 09:04:43

by Tudor Ambarus

[permalink] [raw]
Subject: Re: [PATCH v4 00/16] mtd: spi-nor: add xSPI Octal DTR support

Hi, Pratyush, Boris,

On Friday, April 24, 2020 9:43:54 PM EEST Pratyush Yadav wrote:
> This series adds support for octal DTR flashes in the spi-nor framework,

I'm still learning about this, but I can give you my 2 cents as of now, to
open the discussion. Enabling 2-2-2, 4-4-4, and 8-8-8 modes is dangerous
because the flash may not recover from unexpected resets. Entering one of
these modes can be:
1/ volatile selectable, the device return to the 1-1-1 protocol after the next
power-on. I guess this is conditioned by the optional RESET pin, but I'll have
to check. Also the flash can return to the 1-1-1 mode using the software reset
or through writing to its Configuration Register, without power-on or power-
off.
2/ non-volatile selectable in which RESET# and software reset are useless, the
flash defaults to the mode selected in the non volatile Configuration Register
bits. The only way to get back to 1-1-1 is to write to the Configuration
Register.

Not recovering from unexpected resets is unacceptable. One should always
prefer option 1/ and condition the entering in 2-2-2, 4-4-4 and 8-8-8 with the
presence of the optional RESET pin.

For the unfortunate flashes that support just option 2/, we should not enter
these modes on our own, just by discovering the capabilities from the SFDP
tables or by the flags in the flash_info struct. The best we can do for them
is to move the responsibility to the user. Maybe to add a Kconfig option that
is disabled by default with which we condition the entering in 2-2-2, 4-4-4 or
8-8-8 modes. Once entered in one of these modes, if an unexpected reset comes,
you most likely are doomed, because early stage bootloaders may not work in
these modes and you'll not be able to boot the board. Assuming that one uses
other environment to boot the board, we should at least make sure that the
flash works in linux after an unexpected reset. We should try to determine in
which mode we are at init, so maybe an extension of the default_init hook is
needed. But all this looks like a BIG compromise, I'm not yet sure if we
should adress 2/. Thoughts?

I'm still looking into this.

Cheers,
ta

2020-05-11 09:30:30

by Boris Brezillon

[permalink] [raw]
Subject: Re: [PATCH v4 00/16] mtd: spi-nor: add xSPI Octal DTR support

On Mon, 11 May 2020 09:00:35 +0000
<[email protected]> wrote:

> Hi, Pratyush, Boris,
>
> On Friday, April 24, 2020 9:43:54 PM EEST Pratyush Yadav wrote:
> > This series adds support for octal DTR flashes in the spi-nor framework,
>
> I'm still learning about this, but I can give you my 2 cents as of now, to
> open the discussion. Enabling 2-2-2, 4-4-4, and 8-8-8 modes is dangerous
> because the flash may not recover from unexpected resets. Entering one of
> these modes can be:
> 1/ volatile selectable, the device return to the 1-1-1 protocol after the next
> power-on. I guess this is conditioned by the optional RESET pin, but I'll have
> to check. Also the flash can return to the 1-1-1 mode using the software reset
> or through writing to its Configuration Register, without power-on or power-
> off.

My understanding is that there's no standard software reset procedure
that guarantees no conflict with existing 1S commands, so even the
software reset approach doesn't work here.

> 2/ non-volatile selectable in which RESET# and software reset are useless, the
> flash defaults to the mode selected in the non volatile Configuration Register
> bits. The only way to get back to 1-1-1 is to write to the Configuration
> Register.

I'm less worried about this case though, since I'd expect the ROM
code and bootloaders to be able to deal with xD-xD-xD modes when the
flash is set in this mode by default. That implies letting Linux know
about this default mode of course, maybe through an extra DT
property/cmdline param.

>
> Not recovering from unexpected resets is unacceptable. One should always
> prefer option 1/ and condition the entering in 2-2-2, 4-4-4 and 8-8-8 with the
> presence of the optional RESET pin.

Totally agree with you on that one, but we know what happens in
practice...

>
> For the unfortunate flashes that support just option 2/, we should not enter
> these modes on our own, just by discovering the capabilities from the SFDP
> tables or by the flags in the flash_info struct. The best we can do for them
> is to move the responsibility to the user. Maybe to add a Kconfig option that
> is disabled by default with which we condition the entering in 2-2-2, 4-4-4 or
> 8-8-8 modes.

Hm, a Kconfig option doesn't sound like the right solution to the
problem, since it should be a per-flash decision, not something you set
system-wise.

> Once entered in one of these modes, if an unexpected reset comes,
> you most likely are doomed, because early stage bootloaders may not work in
> these modes and you'll not be able to boot the board. Assuming that one uses
> other environment to boot the board, we should at least make sure that the
> flash works in linux after an unexpected reset. We should try to determine in
> which mode we are at init, so maybe an extension of the default_init hook is
> needed. But all this looks like a BIG compromise, I'm not yet sure if we
> should adress 2/. Thoughts?

We should definitely not write non-volatile regs on our own, but
instead use the mode that's been chosen there. I doubt anyone
setting the non-volative conf to 8D-8D-8D will ever want to go back to
1S-1S-1S anyway, so 8D -> 1S transitions are not really an issue, right?

Of course, that still leaves us with the 'mode detection' issue, and I
have no solution other than flagging it through the DT/cmdline for that
one...

2020-05-11 09:48:39

by Vignesh Raghavendra

[permalink] [raw]
Subject: Re: [PATCH v4 00/16] mtd: spi-nor: add xSPI Octal DTR support



On 11/05/20 2:30 pm, [email protected] wrote:
> Hi, Pratyush, Boris,
>
> On Friday, April 24, 2020 9:43:54 PM EEST Pratyush Yadav wrote:
>> This series adds support for octal DTR flashes in the spi-nor framework,
>
> I'm still learning about this, but I can give you my 2 cents as of now, to
> open the discussion. Enabling 2-2-2, 4-4-4, and 8-8-8 modes is dangerous
> because the flash may not recover from unexpected resets.

Unfortunately, xSPI compliant flashes need to support 1S-1S-1S and
8D-8D-8D (or 4S-4D-4D) mode only. So we have to start supporting
state-full modes

> Entering one of these modes can be:
> 1/ volatile selectable, the device return to the 1-1-1 protocol after the next
> power-on. I guess this is conditioned by the optional RESET pin, but I'll have
> to check. Also the flash can return to the 1-1-1 mode using the software reset
> or through writing to its Configuration Register, without power-on or power-
> off.

Right, I guess switching to octal mode be made conditional based upon
SNOR_F_BROKEN_RESET?

> 2/ non-volatile selectable in which RESET# and software reset are useless, the
> flash defaults to the mode selected in the non volatile Configuration Register
> bits. The only way to get back to 1-1-1 is to write to the Configuration
> Register.
>

In addition to reset issue, supporting flash that boot up in Octal DDR
mode (due to non-volatile setting) is still pretty difficult. Commands
like Read ID and READ SFDP (that are used for flash discovery at
runtime) follow different protocols across different vendors in Octal
DDR mode. So its almost impossible to support such flashes w/o a hint
about device type from DT (or somewhere else).
I would really stick to option 1 for now until someone makes a
compelling case to support option 2.


> Not recovering from unexpected resets is unacceptable. One should always
> prefer option 1/ and condition the entering in 2-2-2, 4-4-4 and 8-8-8 with the
> presence of the optional RESET pin.
>
> For the unfortunate flashes that support just option 2/, we should not enter
> these modes on our own, just by discovering the capabilities from the SFDP
> tables or by the flags in the flash_info struct. The best we can do for them
> is to move the responsibility to the user. Maybe to add a Kconfig option that
> is disabled by default with which we condition the entering in 2-2-2, 4-4-4 or
> 8-8-8 modes. Once entered in one of these modes, if an unexpected reset comes,
> you most likely are doomed, because early stage bootloaders may not work in
> these modes and you'll not be able to boot the board. Assuming that one uses
> other environment to boot the board, we should at least make sure that the
> flash works in linux after an unexpected reset. We should try to determine in
> which mode we are at init, so maybe an extension of the default_init hook is
> needed. But all this looks like a BIG compromise, I'm not yet sure if we
> should adress 2/. Thoughts?
>


Agree, lets not worry about option 2 for now...

Regards
Vignesh

2020-05-11 18:05:53

by Pratyush Yadav

[permalink] [raw]
Subject: Re: [PATCH v4 12/16] mtd: spi-nor: perform a Soft Reset on shutdown

On 10/05/20 11:22AM, [email protected] wrote:
> Hi, Pratyush,
>
> On Friday, April 24, 2020 9:44:06 PM EEST Pratyush Yadav wrote:
> > A Software Reset sequence will return the flash to Power-on-Reset (POR)
> > state. It consists of two commands: Soft Reset Enable and Soft Reset.
> >
> > Perform a Soft Reset on shutdown on flashes that support it so that the
> > flash can be reset to its initial state and any configurations made by
> > spi-nor (given that they're only done in volatile registers) will be
> > reset. This will hand back the flash in pristine state for any further
> > operations on it.
>
> Please don't introduce SPI_NOR_SOFT_RESET yet. We should instead determine the
> software reset sequence by inspecting BFPT[16], bits 13:8.

Ok. Will do.

--
Regards,
Pratyush Yadav
Texas Instruments India

2020-05-11 18:28:22

by Pratyush Yadav

[permalink] [raw]
Subject: Re: [PATCH v4 00/16] mtd: spi-nor: add xSPI Octal DTR support

On 11/05/20 11:27AM, Boris Brezillon wrote:
> On Mon, 11 May 2020 09:00:35 +0000
> <[email protected]> wrote:
>
> > Hi, Pratyush, Boris,
> >
> > On Friday, April 24, 2020 9:43:54 PM EEST Pratyush Yadav wrote:
> > > This series adds support for octal DTR flashes in the spi-nor framework,
> >
> > I'm still learning about this, but I can give you my 2 cents as of now, to
> > open the discussion. Enabling 2-2-2, 4-4-4, and 8-8-8 modes is dangerous
> > because the flash may not recover from unexpected resets. Entering one of
> > these modes can be:
> > 1/ volatile selectable, the device return to the 1-1-1 protocol after the next
> > power-on. I guess this is conditioned by the optional RESET pin, but I'll have
> > to check. Also the flash can return to the 1-1-1 mode using the software reset
> > or through writing to its Configuration Register, without power-on or power-
> > off.
>
> My understanding is that there's no standard software reset procedure
> that guarantees no conflict with existing 1S commands, so even the
> software reset approach doesn't work here.
>
> > 2/ non-volatile selectable in which RESET# and software reset are useless, the
> > flash defaults to the mode selected in the non volatile Configuration Register
> > bits. The only way to get back to 1-1-1 is to write to the Configuration
> > Register.
>
> I'm less worried about this case though, since I'd expect the ROM
> code and bootloaders to be able to deal with xD-xD-xD modes when the
> flash is set in this mode by default. That implies letting Linux know
> about this default mode of course, maybe through an extra DT
> property/cmdline param.
>
> >
> > Not recovering from unexpected resets is unacceptable. One should always
> > prefer option 1/ and condition the entering in 2-2-2, 4-4-4 and 8-8-8 with the
> > presence of the optional RESET pin.
>
> Totally agree with you on that one, but we know what happens in
> practice...
>
> >
> > For the unfortunate flashes that support just option 2/, we should not enter
> > these modes on our own, just by discovering the capabilities from the SFDP
> > tables or by the flags in the flash_info struct. The best we can do for them
> > is to move the responsibility to the user. Maybe to add a Kconfig option that
> > is disabled by default with which we condition the entering in 2-2-2, 4-4-4 or
> > 8-8-8 modes.
>
> Hm, a Kconfig option doesn't sound like the right solution to the
> problem, since it should be a per-flash decision, not something you set
> system-wise.

Agreed.

Is there any such flash in use today? The two flashes the series adds
support for both have volatile configuration for 8D mode. Unless we have
to support a flash like this in practice, I think such a change is out
of the scope of this series.

> > Once entered in one of these modes, if an unexpected reset comes,
> > you most likely are doomed, because early stage bootloaders may not work in
> > these modes and you'll not be able to boot the board. Assuming that one uses
> > other environment to boot the board, we should at least make sure that the
> > flash works in linux after an unexpected reset. We should try to determine in
> > which mode we are at init, so maybe an extension of the default_init hook is
> > needed. But all this looks like a BIG compromise, I'm not yet sure if we
> > should adress 2/. Thoughts?
>
> We should definitely not write non-volatile regs on our own, but
> instead use the mode that's been chosen there. I doubt anyone
> setting the non-volative conf to 8D-8D-8D will ever want to go back to
> 1S-1S-1S anyway, so 8D -> 1S transitions are not really an issue, right?
>
> Of course, that still leaves us with the 'mode detection' issue, and I
> have no solution other than flagging it through the DT/cmdline for that
> one...

Correct. I tried doing it, and the best way I could figure out was to
try reading the SFDP signature in 1S and 8D mode, and see where we get
the correct value. But unfortunately, because the Read ID command is
different in 8D mode for different flashes, we can't then figure out
which flash it actually is.

--
Regards,
Pratyush Yadav
Texas Instruments India

2020-05-12 06:20:46

by Tudor Ambarus

[permalink] [raw]
Subject: Re: [PATCH v4 00/16] mtd: spi-nor: add xSPI Octal DTR support

Hi, Boris, Pratyush,

I stripped case 2/, we'll not treat it for now.

On Monday, May 11, 2020 12:27:12 PM EEST Boris Brezillon wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
>
> On Mon, 11 May 2020 09:00:35 +0000
>
> <[email protected]> wrote:
> > Hi, Pratyush, Boris,
> >
> > On Friday, April 24, 2020 9:43:54 PM EEST Pratyush Yadav wrote:
> > > This series adds support for octal DTR flashes in the spi-nor framework,
> >
> > I'm still learning about this, but I can give you my 2 cents as of now, to
> > open the discussion. Enabling 2-2-2, 4-4-4, and 8-8-8 modes is dangerous
> > because the flash may not recover from unexpected resets. Entering one of
> > these modes can be:
> > 1/ volatile selectable, the device return to the 1-1-1 protocol after the
> > next power-on. I guess this is conditioned by the optional RESET pin, but
> > I'll have to check. Also the flash can return to the 1-1-1 mode using the
> > software reset or through writing to its Configuration Register, without
> > power-on or power- off.
>
> My understanding is that there's no standard software reset procedure
> that guarantees no conflict with existing 1S commands, so even the
> software reset approach doesn't work here.
>

The software reset procedure can't protect you from unexpected resets, but the
hardware with its optional reset pin can. Pratyush to confirm.

cut

>
> > Not recovering from unexpected resets is unacceptable. One should always
> > prefer option 1/ and condition the entering in 2-2-2, 4-4-4 and 8-8-8 with
> > the presence of the optional RESET pin.
>
> Totally agree with you on that one, but we know what happens in
> practice...

What I proposed is to condition the entering in the state-full modes with the
presence of the optional RESET pin. We would introduce an optional device tree
property for the RESET pin. If hardware doesn't implement the optional RESET#
signal, then we will not enter in the state-full modes.

Cheers,
ta


2020-05-12 09:52:08

by Vignesh Raghavendra

[permalink] [raw]
Subject: Re: [PATCH v4 00/16] mtd: spi-nor: add xSPI Octal DTR support



On 12/05/20 11:46 am, [email protected] wrote:
> Hi, Boris, Pratyush,
>
> I stripped case 2/, we'll not treat it for now.
>
> On Monday, May 11, 2020 12:27:12 PM EEST Boris Brezillon wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
>> content is safe
>>
>> On Mon, 11 May 2020 09:00:35 +0000
>>
>> <[email protected]> wrote:
>>> Hi, Pratyush, Boris,
>>>
>>> On Friday, April 24, 2020 9:43:54 PM EEST Pratyush Yadav wrote:
>>>> This series adds support for octal DTR flashes in the spi-nor framework,
>>>
>>> I'm still learning about this, but I can give you my 2 cents as of now, to
>>> open the discussion. Enabling 2-2-2, 4-4-4, and 8-8-8 modes is dangerous
>>> because the flash may not recover from unexpected resets. Entering one of
>>> these modes can be:
>>> 1/ volatile selectable, the device return to the 1-1-1 protocol after the
>>> next power-on. I guess this is conditioned by the optional RESET pin, but
>>> I'll have to check. Also the flash can return to the 1-1-1 mode using the
>>> software reset or through writing to its Configuration Register, without
>>> power-on or power- off.
>>
>> My understanding is that there's no standard software reset procedure
>> that guarantees no conflict with existing 1S commands, so even the
>> software reset approach doesn't work here.
>>
>
> The software reset procedure can't protect you from unexpected resets, but the
> hardware with its optional reset pin can. Pratyush to confirm.
>
> cut
>
>>
>>> Not recovering from unexpected resets is unacceptable. One should always
>>> prefer option 1/ and condition the entering in 2-2-2, 4-4-4 and 8-8-8 with
>>> the presence of the optional RESET pin.
>>
>> Totally agree with you on that one, but we know what happens in
>> practice...
>
> What I proposed is to condition the entering in the state-full modes with the
> presence of the optional RESET pin. We would introduce an optional device tree
> property for the RESET pin. If hardware doesn't implement the optional RESET#
> signal, then we will not enter in the state-full modes.
>

Are you asking for dedicated SW controllable reset line or just an
indication from DT that OSPI reset line is connected to board level
soft/hard reset lines?

Mandating SW controllable RESET line is bit of a stretch IMO... Board
design may not allow wasting dedicated pin due to lack of GPIOs perhaps..

For eg.: TI EVM has OSPI reset line connected to board level reset out.
This ensures any soft/warm/hard CPU reset will trigger OSPI Flash reset,
but there is no SW control that allows OSPI flash alone to be reset.
Isn't such a reset mechanism sufficient?

Regards
Vignesh

2020-05-12 11:32:23

by Tudor Ambarus

[permalink] [raw]
Subject: Re: [PATCH v4 00/16] mtd: spi-nor: add xSPI Octal DTR support

Hi, Vignesh,

On Tuesday, May 12, 2020 12:49:07 PM EEST Vignesh Raghavendra wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
> On 12/05/20 11:46 am, [email protected] wrote:
> > Hi, Boris, Pratyush,
> >
> > I stripped case 2/, we'll not treat it for now.
> >
> > On Monday, May 11, 2020 12:27:12 PM EEST Boris Brezillon wrote:
> >> EXTERNAL EMAIL: Do not click links or open attachments unless you know
> >> the
> >> content is safe
> >>
> >> On Mon, 11 May 2020 09:00:35 +0000
> >>
> >> <[email protected]> wrote:
> >>> Hi, Pratyush, Boris,
> >>>
> >>> On Friday, April 24, 2020 9:43:54 PM EEST Pratyush Yadav wrote:
> >>>> This series adds support for octal DTR flashes in the spi-nor
> >>>> framework,
> >>>
> >>> I'm still learning about this, but I can give you my 2 cents as of now,
> >>> to
> >>> open the discussion. Enabling 2-2-2, 4-4-4, and 8-8-8 modes is dangerous
> >>> because the flash may not recover from unexpected resets. Entering one
> >>> of
> >>> these modes can be:
> >>> 1/ volatile selectable, the device return to the 1-1-1 protocol after
> >>> the
> >>> next power-on. I guess this is conditioned by the optional RESET pin,
> >>> but
> >>> I'll have to check. Also the flash can return to the 1-1-1 mode using
> >>> the
> >>> software reset or through writing to its Configuration Register, without
> >>> power-on or power- off.
> >>
> >> My understanding is that there's no standard software reset procedure
> >> that guarantees no conflict with existing 1S commands, so even the
> >> software reset approach doesn't work here.
> >
> > The software reset procedure can't protect you from unexpected resets, but
> > the hardware with its optional reset pin can. Pratyush to confirm.
> >
> > cut
> >
> >>> Not recovering from unexpected resets is unacceptable. One should always
> >>> prefer option 1/ and condition the entering in 2-2-2, 4-4-4 and 8-8-8
> >>> with
> >>> the presence of the optional RESET pin.
> >>
> >> Totally agree with you on that one, but we know what happens in
> >> practice...
> >
> > What I proposed is to condition the entering in the state-full modes with
> > the presence of the optional RESET pin. We would introduce an optional
> > device tree property for the RESET pin. If hardware doesn't implement the
> > optional RESET# signal, then we will not enter in the state-full modes.
>
> Are you asking for dedicated SW controllable reset line or just an
> indication from DT that OSPI reset line is connected to board level
> soft/hard reset lines?

I don't see a need for the reset line to be SW controllable, a simple
indication from the device tree should be enough.

>
> Mandating SW controllable RESET line is bit of a stretch IMO... Board
> design may not allow wasting dedicated pin due to lack of GPIOs perhaps..
>
> For eg.: TI EVM has OSPI reset line connected to board level reset out.
> This ensures any soft/warm/hard CPU reset will trigger OSPI Flash reset,
> but there is no SW control that allows OSPI flash alone to be reset.
> Isn't such a reset mechanism sufficient?
>

I think it is, yes.

Cheers,
ta

2020-05-12 18:49:50

by Pratyush Yadav

[permalink] [raw]
Subject: Re: [PATCH v4 00/16] mtd: spi-nor: add xSPI Octal DTR support

On 12/05/20 11:29AM, [email protected] wrote:
> Hi, Vignesh,
>
> > > The software reset procedure can't protect you from unexpected
> > > resets, but
> > > the hardware with its optional reset pin can. Pratyush to confirm.
> > >
> > > cut
> > >
> > >>> Not recovering from unexpected resets is unacceptable. One should always
> > >>> prefer option 1/ and condition the entering in 2-2-2, 4-4-4 and 8-8-8
> > >>> with
> > >>> the presence of the optional RESET pin.
> > >>
> > >> Totally agree with you on that one, but we know what happens in
> > >> practice...
> > >
> > > What I proposed is to condition the entering in the state-full modes with
> > > the presence of the optional RESET pin. We would introduce an optional
> > > device tree property for the RESET pin. If hardware doesn't implement the
> > > optional RESET# signal, then we will not enter in the state-full modes.
> >
> > Are you asking for dedicated SW controllable reset line or just an
> > indication from DT that OSPI reset line is connected to board level
> > soft/hard reset lines?
>
> I don't see a need for the reset line to be SW controllable, a simple
> indication from the device tree should be enough.

We already have the property "broken-flash-reset". Should we re-use it
or should we have a opt-in property instead of an opt-out one?

> >
> > Mandating SW controllable RESET line is bit of a stretch IMO... Board
> > design may not allow wasting dedicated pin due to lack of GPIOs perhaps..
> >
> > For eg.: TI EVM has OSPI reset line connected to board level reset out.
> > This ensures any soft/warm/hard CPU reset will trigger OSPI Flash reset,
> > but there is no SW control that allows OSPI flash alone to be reset.
> > Isn't such a reset mechanism sufficient?
> >
>
> I think it is, yes.

--
Regards,
Pratyush Yadav
Texas Instruments India