Hello,
This is repost of patchset from Boris Brezillon's
[RFC,00/18] mtd: spi-nor: Proposal for 8-8-8 mode support [1].
Background from cover letter for RFC[1].
The trend has been around Octal NOR Flash lately and the latest mainline
already supports 1-1-8 and 1-8-8 modes.
Boris opened a discussion on how we should support stateful modes (X-X-X
and XD-XD-XD, where X is the bus width and D means Double Transfer Rate).
JESD216C has defined specification for Octal 8-8-8 and 8D-8D-8D.
It defined command and command extension in
JEDEC Basic Flash Parameter Table(18th DWORD) as well as how to
enable 8-8-8/8D-8D-8D mode sequences (Write CFG Reg 2).
The first set of patches is according to JESD216C adding Double Transfer
Rate(DTR) fields, extension command and command bytes number to the
spi_mem_op struct.
The second set of patches define the relevant macrons and enum in spi-nor
layer for Octal 8-8-8 and 8D-8D-8D mode operation.
The last set of patches in the series are modifying spi_nor_fixups hook to
tweak flash parameters for spi_nor_read/pp_setting() and then in a
chip-specific way to enter 8-8-8 or 8D-8D-8D modes on a Macronix chip.
Also patched spi-mxic driver for testing on Macronix's Zynq PicoZed board
with Macronix's SPI controller (spi-mxic.c) and mx25uw51245g Octal flash.
[1] https://patchwork.ozlabs.org/cover/982926/
thnaks for your time and review.
best regards,
Mason
Mason Yang (4):
spi: spi-mem: Add support for Octal 8D-8D-8D mode
mtd: spi-nor: Add support for Octal 8D-8D-8D mode
mtd: spi-nor: Add Octal 8D-8D-8D mode support for Macronix
mx25uw51245g
spi: mxic: Add support for Octal 8D-8D-8D mode
drivers/mtd/spi-nor/spi-nor.c | 273 +++++++++++++++++++++++++++++++++++++++++-
drivers/spi/spi-mem.c | 8 +-
drivers/spi/spi-mxic.c | 98 ++++++++++-----
include/linux/mtd/spi-nor.h | 61 +++++++++-
include/linux/spi/spi-mem.h | 13 ++
5 files changed, 410 insertions(+), 43 deletions(-)
--
1.9.1
According to JESD216C SPI NORs are using 2 bytes opcodes
when operated in OPI (Octal Peripheral Interface).
To add extension command, command bytes number and
Double Transfer Rate(DTR) fields to the spi_mem_op struct.
Signed-off-by: Boris Brezillon <[email protected]>
Signed-off-by: Mason Yang <[email protected]>
---
drivers/spi/spi-mem.c | 8 +++++++-
include/linux/spi/spi-mem.h | 13 +++++++++++++
2 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c
index 9f0fa9f..eb33dd85 100644
--- a/drivers/spi/spi-mem.c
+++ b/drivers/spi/spi-mem.c
@@ -154,6 +154,12 @@ bool spi_mem_default_supports_op(struct spi_mem *mem,
op->data.dir == SPI_MEM_DATA_OUT))
return false;
+ if (op->cmd.dtr || op->addr.dtr || op->dummy.dtr || op->data.dtr)
+ return false;
+
+ if (op->cmd.nbytes != 1)
+ return false;
+
return true;
}
EXPORT_SYMBOL_GPL(spi_mem_default_supports_op);
@@ -168,7 +174,7 @@ static bool spi_mem_buswidth_is_valid(u8 buswidth)
static int spi_mem_check_op(const struct spi_mem_op *op)
{
- if (!op->cmd.buswidth)
+ if (!op->cmd.buswidth || op->cmd.nbytes < 1 || op->cmd.nbytes > 2)
return -EINVAL;
if ((op->addr.nbytes && !op->addr.buswidth) ||
diff --git a/include/linux/spi/spi-mem.h b/include/linux/spi/spi-mem.h
index af9ff2f..bf54079 100644
--- a/include/linux/spi/spi-mem.h
+++ b/include/linux/spi/spi-mem.h
@@ -17,6 +17,7 @@
{ \
.buswidth = __buswidth, \
.opcode = __opcode, \
+ .nbytes = 1, \
}
#define SPI_MEM_OP_ADDR(__nbytes, __val, __buswidth) \
@@ -69,11 +70,15 @@ 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)
* @cmd.buswidth: number of IO lines used to transmit the command
+ * @cmd.dtr: set true to transfer opcode in double transfer rate mode
* @cmd.opcode: operation opcode
+ * @cmd.ext_opcode: extension operation opcode
* @addr.nbytes: number of address bytes to send. Can be zero if the operation
* does not need to send an address
* @addr.buswidth: number of IO lines used to transmit the address cycles
+ * @addr.dtr: set true to transfer address bytes in double transfer rate mode
* @addr.val: address value. This value is always sent MSB first on the bus.
* Note that only @addr.nbytes are taken into account in this
* address value, so users should make sure the value fits in the
@@ -81,34 +86,42 @@ enum spi_mem_data_dir {
* @dummy.nbytes: number of dummy bytes to send after an opcode or address. Can
* be zero if the operation does not require dummy bytes
* @dummy.buswidth: number of IO lanes used to transmit the dummy bytes
+ * @dummy.dtr: set true to transfer dummy bytes in double transfer rate mode
* @data.buswidth: number of IO lanes used to send/receive the data
* @data.dir: direction of the transfer
* @data.nbytes: number of data bytes to send/receive. Can be zero if the
* operation does not involve transferring data
+ * @data.dtr: set true to transfer data bytes in double transfer rate mode
* @data.buf.in: input buffer (must be DMA-able)
* @data.buf.out: output buffer (must be DMA-able)
*/
struct spi_mem_op {
struct {
+ u8 nbytes;
u8 buswidth;
+ bool dtr;
u8 opcode;
+ u8 ext_opcode;
} cmd;
struct {
u8 nbytes;
u8 buswidth;
+ bool dtr;
u64 val;
} addr;
struct {
u8 nbytes;
u8 buswidth;
+ bool dtr;
} dummy;
struct {
u8 buswidth;
enum spi_mem_data_dir dir;
unsigned int nbytes;
+ bool dtr;
union {
void *in;
const void *out;
--
1.9.1
According to JESD216C (JEDEC Basic Flash Parameter Table 18th DWORD)
Octal DTR(8D-8D-8D) command and command extension (00b: same, 01b: inverse)
to add extension command mode in spi_nor struct and to add write_cr2
(Write CFG Reg 2) for 8-8-8/8D-8D-8D mode sequences enable.
Define the relevant macrons and enum to add such modes and make sure
op->xxx.dtr fields, command nbytes and extension command are properly
filled and unmask DTR and X-X-X modes in spi_nor_spimem_adjust_hwcaps()
so that DTR and X-X-X support detection is done through
spi_mem_supports_op().
Signed-off-by: Boris Brezillon <[email protected]>
Signed-off-by: Mason Yang <[email protected]>
---
drivers/mtd/spi-nor/spi-nor.c | 159 ++++++++++++++++++++++++++++++++++++++++--
include/linux/mtd/spi-nor.h | 58 +++++++++++++--
2 files changed, 206 insertions(+), 11 deletions(-)
diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 7acf4a9..4cdf52d 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -320,6 +320,23 @@ static ssize_t spi_nor_spimem_read_data(struct spi_nor *nor, loff_t from,
/* convert the dummy cycles to the number of bytes */
op.dummy.nbytes = (nor->read_dummy * op.dummy.buswidth) / 8;
+ if (spi_nor_protocol_is_8_8_8(nor->read_proto)) {
+ op.cmd.nbytes = 2;
+
+ if (nor->ext_cmd_mode == EXT_CMD_IS_INVERSE)
+ op.cmd.ext_opcode = ~nor->read_opcode;
+ else
+ op.cmd.ext_opcode = nor->read_opcode;
+
+ if (spi_nor_protocol_is_8D_8D_8D(nor->read_proto)) {
+ op.dummy.nbytes *= 2;
+ op.cmd.dtr = true;
+ op.addr.dtr = true;
+ op.dummy.dtr = true;
+ op.data.dtr = true;
+ }
+ }
+
return spi_nor_spimem_xfer_data(nor, &op);
}
@@ -367,6 +384,21 @@ static ssize_t spi_nor_spimem_write_data(struct spi_nor *nor, loff_t to,
if (nor->program_opcode == SPINOR_OP_AAI_WP && nor->sst_write_second)
op.addr.nbytes = 0;
+ if (spi_nor_protocol_is_8_8_8(nor->write_proto)) {
+ op.cmd.nbytes = 2;
+
+ if (nor->ext_cmd_mode == EXT_CMD_IS_INVERSE)
+ op.cmd.ext_opcode = ~nor->program_opcode;
+ else
+ op.cmd.ext_opcode = nor->program_opcode;
+
+ if (spi_nor_protocol_is_8D_8D_8D(nor->write_proto)) {
+ op.cmd.dtr = true;
+ op.addr.dtr = true;
+ op.data.dtr = true;
+ }
+ }
+
return spi_nor_spimem_xfer_data(nor, &op);
}
@@ -404,6 +436,30 @@ static int read_sr(struct spi_nor *nor)
SPI_MEM_OP_NO_DUMMY,
SPI_MEM_OP_DATA_IN(1, nor->bouncebuf, 1));
+ if (spi_nor_protocol_is_8_8_8(nor->read_proto)) {
+ op.cmd.buswidth = 8;
+ op.addr.buswidth = 8;
+ op.dummy.buswidth = 8;
+ op.data.buswidth = 8;
+ op.cmd.nbytes = 2;
+ op.addr.nbytes = 4;
+ op.dummy.nbytes = 4;
+ op.addr.val = 0;
+
+ if (nor->ext_cmd_mode == EXT_CMD_IS_INVERSE)
+ op.cmd.ext_opcode = ~SPINOR_OP_RDSR;
+ else
+ op.cmd.ext_opcode = SPINOR_OP_RDSR;
+
+ if (spi_nor_protocol_is_8D_8D_8D(nor->read_proto)) {
+ op.dummy.nbytes *= 2;
+ op.cmd.dtr = true;
+ op.addr.dtr = true;
+ op.dummy.dtr = true;
+ op.data.dtr = true;
+ }
+ }
+
ret = spi_mem_exec_op(nor->spimem, &op);
} else {
ret = nor->read_reg(nor, SPINOR_OP_RDSR, nor->bouncebuf, 1);
@@ -508,6 +564,19 @@ static int write_enable(struct spi_nor *nor)
SPI_MEM_OP_NO_DUMMY,
SPI_MEM_OP_NO_DATA);
+ if (spi_nor_protocol_is_8_8_8(nor->write_proto)) {
+ op.cmd.buswidth = 8;
+ op.cmd.nbytes = 2;
+
+ if (spi_nor_protocol_is_8D_8D_8D(nor->write_proto))
+ op.cmd.dtr = true;
+
+ if (nor->ext_cmd_mode == EXT_CMD_IS_INVERSE)
+ op.cmd.ext_opcode = ~SPINOR_OP_WREN;
+ else
+ op.cmd.ext_opcode = SPINOR_OP_WREN;
+ }
+
return spi_mem_exec_op(nor->spimem, &op);
}
@@ -526,12 +595,65 @@ static int write_disable(struct spi_nor *nor)
SPI_MEM_OP_NO_DUMMY,
SPI_MEM_OP_NO_DATA);
+ if (spi_nor_protocol_is_8_8_8(nor->write_proto)) {
+ op.cmd.buswidth = 8;
+ op.cmd.nbytes = 2;
+
+ if (spi_nor_protocol_is_8D_8D_8D(nor->write_proto))
+ op.cmd.dtr = true;
+
+ if (nor->ext_cmd_mode == EXT_CMD_IS_INVERSE)
+ op.cmd.ext_opcode = ~SPINOR_OP_WRDI;
+ else
+ op.cmd.ext_opcode = SPINOR_OP_WRDI;
+ }
+
return spi_mem_exec_op(nor->spimem, &op);
}
return nor->write_reg(nor, SPINOR_OP_WRDI, NULL, 0);
}
+/*
+ * Write configuration register 2 one byte
+ * Returns negative if error occurred.
+ */
+static int write_cr2(struct spi_nor *nor, u32 addr, u8 val)
+{
+ write_enable(nor);
+
+ nor->bouncebuf[0] = val;
+ if (nor->spimem) {
+ struct spi_mem_op op =
+ SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WRCR2, 1),
+ SPI_MEM_OP_ADDR(4, addr, 1),
+ SPI_MEM_OP_NO_DUMMY,
+ SPI_MEM_OP_DATA_OUT(1, nor->bouncebuf, 1));
+
+ if (spi_nor_protocol_is_8_8_8(nor->write_proto)) {
+ op.cmd.buswidth = 8;
+ op.addr.buswidth = 8;
+ op.data.buswidth = 8;
+ op.cmd.nbytes = 2;
+
+ if (spi_nor_protocol_is_8D_8D_8D(nor->write_proto)) {
+ op.cmd.dtr = true;
+ op.addr.dtr = true;
+ op.data.dtr = true;
+ }
+
+ if (nor->ext_cmd_mode == EXT_CMD_IS_INVERSE)
+ op.cmd.ext_opcode = ~SPINOR_OP_WRCR2;
+ else
+ op.cmd.ext_opcode = SPINOR_OP_WRCR2;
+ }
+
+ return spi_mem_exec_op(nor->spimem, &op);
+ }
+
+ return -ENOTSUPP;
+}
+
static struct spi_nor *mtd_to_spi_nor(struct mtd_info *mtd)
{
return mtd->priv;
@@ -868,6 +990,19 @@ static int erase_chip(struct spi_nor *nor)
SPI_MEM_OP_NO_DUMMY,
SPI_MEM_OP_NO_DATA);
+ if (spi_nor_protocol_is_8_8_8(nor->write_proto)) {
+ op.cmd.buswidth = 8;
+ op.cmd.nbytes = 2;
+
+ if (spi_nor_protocol_is_8D_8D_8D(nor->write_proto))
+ op.cmd.dtr = true;
+
+ if (nor->ext_cmd_mode == EXT_CMD_IS_INVERSE)
+ op.cmd.ext_opcode = ~SPINOR_OP_CHIP_ERASE;
+ else
+ op.cmd.ext_opcode = SPINOR_OP_CHIP_ERASE;
+ }
+
return spi_mem_exec_op(nor->spimem, &op);
}
@@ -945,6 +1080,22 @@ static int spi_nor_erase_sector(struct spi_nor *nor, u32 addr)
SPI_MEM_OP_NO_DUMMY,
SPI_MEM_OP_NO_DATA);
+ if (spi_nor_protocol_is_8_8_8(nor->write_proto)) {
+ op.cmd.buswidth = 8;
+ op.addr.buswidth = 8;
+ op.cmd.nbytes = 2;
+
+ if (spi_nor_protocol_is_8D_8D_8D(nor->write_proto)) {
+ op.cmd.dtr = true;
+ op.addr.dtr = true;
+ }
+
+ if (nor->ext_cmd_mode == EXT_CMD_IS_INVERSE)
+ op.cmd.ext_opcode = ~nor->erase_opcode;
+ else
+ op.cmd.ext_opcode = nor->erase_opcode;
+ }
+
return spi_mem_exec_op(nor->spimem, &op);
}
@@ -2825,6 +2976,7 @@ static 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_8D_8D_8D },
};
return spi_nor_hwcaps2cmd(hwcaps, hwcaps_read2cmd,
@@ -2841,6 +2993,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_8D_8D_8D },
};
return spi_nor_hwcaps2cmd(hwcaps, hwcaps_pp2cmd,
@@ -3010,12 +3163,6 @@ static int spi_nor_spimem_check_pp(struct spi_nor *nor,
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;
-
for (cap = 0; cap < sizeof(*hwcaps) * BITS_PER_BYTE; cap++) {
int rdidx, ppidx;
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index fc0b4b1..2e720ca 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -84,6 +84,9 @@
#define SPINOR_OP_BE_4K_4B 0x21 /* Erase 4KiB block */
#define SPINOR_OP_BE_32K_4B 0x5c /* Erase 32KiB block */
#define SPINOR_OP_SE_4B 0xdc /* Sector erase (usually 64KiB) */
+#define SPINOR_OP_READ_8_8_8 SPINOR_OP_READ_1_4_4_4B
+#define SPINOR_OP_PP_8_8_8 SPINOR_OP_PP_4B
+#define SPINOR_OP_PP_8D_8D_8D SPINOR_OP_PP_4B
/* Double Transfer Rate opcodes - defined in JEDEC JESD216B. */
#define SPINOR_OP_READ_1_1_1_DTR 0x0d
@@ -93,6 +96,7 @@
#define SPINOR_OP_READ_1_1_1_DTR_4B 0x0e
#define SPINOR_OP_READ_1_2_2_DTR_4B 0xbe
#define SPINOR_OP_READ_1_4_4_DTR_4B 0xee
+#define SPINOR_OP_READ_8D_8D_8D SPINOR_OP_READ_1_4_4_DTR_4B
/* Used for SST flashes only. */
#define SPINOR_OP_BP 0x02 /* Byte program */
@@ -107,6 +111,8 @@
#define XSR_PAGESIZE BIT(0) /* Page size in Po2 or Linear */
#define XSR_RDY BIT(7) /* Ready */
+/* Write CFG Reg 2 - defined in JEDEC JESD216C. */
+#define SPINOR_OP_WRCR2 0x72 /* Write configuration register 2 */
/* Used for Macronix and Winbond flashes. */
#define SPINOR_OP_EN4B 0xb7 /* Enter 4-byte mode */
@@ -150,6 +156,13 @@
/* Status Register 2 bits. */
#define SR2_QUAD_EN_BIT7 BIT(7)
+/* Configuration register 2, offset 0 */
+#define CR2_REG0 0x0
+#define CR2_REG0_MODE_MASK GENMASK(1, 0)
+#define CR2_REG0_MODE_SPI 0
+#define CR2_REG0_MODE_OPI_STR 1
+#define CR2_REG0_MODE_OPI_DTR 2
+
/* Supported SPI protocols */
#define SNOR_PROTO_INST_MASK GENMASK(23, 16)
#define SNOR_PROTO_INST_SHIFT 16
@@ -170,6 +183,7 @@
SNOR_PROTO_DATA_MASK)
#define SNOR_PROTO_IS_DTR BIT(24) /* Double Transfer Rate */
+#define SNOR_PROTO_IS_8D_8D_8D BIT(25) /* Full Octal DTR */
#define SNOR_PROTO_STR(_inst_nbits, _addr_nbits, _data_nbits) \
(SNOR_PROTO_INST(_inst_nbits) | \
@@ -179,6 +193,10 @@
(SNOR_PROTO_IS_DTR | \
SNOR_PROTO_STR(_inst_nbits, _addr_nbits, _data_nbits))
+#define SNOR_PROTO_8D_8D_8D(_nbits) \
+ (SNOR_PROTO_IS_8D_8D_8D | \
+ SNOR_PROTO_STR(_nbits, _nbits, _nbits))
+
enum spi_nor_protocol {
SNOR_PROTO_1_1_1 = SNOR_PROTO_STR(1, 1, 1),
SNOR_PROTO_1_1_2 = SNOR_PROTO_STR(1, 1, 2),
@@ -195,6 +213,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_8D_8D_8D(8),
};
static inline bool spi_nor_protocol_is_dtr(enum spi_nor_protocol proto)
@@ -202,6 +221,16 @@ static inline bool spi_nor_protocol_is_dtr(enum spi_nor_protocol proto)
return !!(proto & SNOR_PROTO_IS_DTR);
}
+static inline bool spi_nor_protocol_is_8_8_8(enum spi_nor_protocol proto)
+{
+ return !!(proto & SNOR_PROTO_8_8_8);
+}
+
+static inline bool spi_nor_protocol_is_8D_8D_8D(enum spi_nor_protocol proto)
+{
+ return !!(proto & SNOR_PROTO_IS_8D_8D_8D);
+}
+
static inline u8 spi_nor_get_protocol_inst_nbits(enum spi_nor_protocol proto)
{
return ((unsigned long)(proto & SNOR_PROTO_INST_MASK)) >>
@@ -349,7 +378,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)
@@ -366,11 +395,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.
@@ -381,7 +411,7 @@ 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_MASK GENMASK(23, 16)
#define SNOR_HWCAPS_PP BIT(16)
#define SNOR_HWCAPS_PP_QUAD GENMASK(19, 17)
@@ -389,10 +419,17 @@ struct spi_nor_hwcaps {
#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_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_OPI_FULL_STR (SNOR_HWCAPS_READ_8_8_8 | \
+ SNOR_HWCAPS_PP_8_8_8)
+
+#define SNOR_HWCAPS_OPI_FULL_DTR (SNOR_HWCAPS_READ_8_8_8_DTR | \
+ SNOR_HWCAPS_PP_8_8_8_DTR)
#define SNOR_HWCAPS_X_X_X (SNOR_HWCAPS_READ_2_2_2 | \
SNOR_HWCAPS_READ_4_4_4 | \
@@ -403,7 +440,9 @@ 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 | \
+ SNOR_HWCAPS_PP_8_8_8_DTR)
#define SNOR_HWCAPS_ALL (SNOR_HWCAPS_READ_MASK | \
SNOR_HWCAPS_PP_MASK)
@@ -442,6 +481,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_8D_8D_8D,
SNOR_CMD_READ_MAX
};
@@ -458,6 +498,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_8D_8D_8D,
SNOR_CMD_PP_MAX
};
@@ -528,6 +569,11 @@ struct spi_nor_flash_parameter {
*/
struct flash_info;
+enum extension_cmd_mode {
+ EXT_CMD_IS_CMD,
+ EXT_CMD_IS_INVERSE,
+};
+
/**
* struct spi_nor - Structure for defining a the SPI NOR layer
* @mtd: point to a mtd_info structure
@@ -537,6 +583,7 @@ struct spi_nor_flash_parameter {
* @bouncebuf: bounce buffer used when the buffer passed by the MTD
* layer is not DMA-able
* @bouncebuf_size: size of the bounce buffer
+ * @ext_cmd_mode: extension command mode, 0: same, 1: inverse
* @info: spi-nor part JDEC MFR id and other info
* @page_size: the page size of the SPI NOR
* @addr_width: number of address bytes
@@ -575,6 +622,7 @@ struct spi_nor {
struct spi_mem *spimem;
u8 *bouncebuf;
size_t bouncebuf_size;
+ enum extension_cmd_mode ext_cmd_mode;
const struct flash_info *info;
u32 page_size;
u8 addr_width;
--
1.9.1
mx25uw51245g is a SPI NOR that supports 1-1-1/8-8-8 mode and the SFDPRD
command but returns an empty SFDP page. This forces us to add a new entry
in the flash_info table and patch spi_nor_fixups hooks to tweak flash
parameters for spi_nor_read/pp_setting() and then to enter
Octal 8D-8D-8D modes.
Signed-off-by: Boris Brezillon <[email protected]>
Signed-off-by: Mason Yang <[email protected]>
---
drivers/mtd/spi-nor/spi-nor.c | 114 ++++++++++++++++++++++++++++++++++++++++++
include/linux/mtd/spi-nor.h | 3 ++
2 files changed, 117 insertions(+)
diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 4cdf52d..9013590 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -615,6 +615,56 @@ static int write_disable(struct spi_nor *nor)
}
/*
+ * Read configuration register 2, returning its value in the
+ * location. Return the configuration register 2 value.
+ * Returns negative if error occurred.
+ */
+static int read_cr2(struct spi_nor *nor, u32 addr)
+{
+ int ret;
+
+ if (nor->spimem) {
+ struct spi_mem_op op =
+ SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_RDCR2, 1),
+ SPI_MEM_OP_ADDR(4, addr, 1),
+ SPI_MEM_OP_DUMMY(4, 1),
+ SPI_MEM_OP_DATA_IN(1, nor->bouncebuf, 1));
+
+ if (spi_nor_protocol_is_8_8_8(nor->read_proto)) {
+ op.cmd.buswidth = 8;
+ op.addr.buswidth = 8;
+ op.dummy.buswidth = 8;
+ op.data.buswidth = 8;
+ op.cmd.nbytes = 2;
+
+ if (spi_nor_protocol_is_8D_8D_8D(nor->read_proto)) {
+ op.dummy.nbytes *= 2;
+ op.cmd.dtr = true;
+ op.addr.dtr = true;
+ op.dummy.dtr = true;
+ op.data.dtr = true;
+ }
+
+ if (nor->ext_cmd_mode == EXT_CMD_IS_INVERSE)
+ op.cmd.ext_opcode = ~SPINOR_OP_RDCR2;
+ else
+ op.cmd.ext_opcode = SPINOR_OP_RDCR2;
+ }
+
+ ret = spi_mem_exec_op(nor->spimem, &op);
+ } else {
+ ret = -ENOTSUPP;
+ }
+
+ if (ret < 0) {
+ dev_err(nor->dev, "error %d reading CR\n", ret);
+ return ret;
+ }
+
+ return nor->bouncebuf[0];
+}
+
+/*
* Write configuration register 2 one byte
* Returns negative if error occurred.
*/
@@ -2275,10 +2325,72 @@ static int spi_nor_spansion_clear_sr_bp(struct spi_nor *nor)
return 0;
}
+static 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);
+
+static void
+spi_nor_set_pp_settings(struct spi_nor_pp_command *pp,
+ u8 opcode,
+ enum spi_nor_protocol proto);
+
+static void
+mx25uw51245g_default_init(struct spi_nor *nor)
+{
+ struct spi_nor_flash_parameter *params = &nor->params;
+
+ if (!(nor->spimem->spi->mode & (SPI_RX_OCTAL | SPI_TX_OCTAL)))
+ return;
+
+ /* Octal 8S-8S-8S mode */
+ params->hwcaps.mask |= SNOR_HWCAPS_OPI_FULL_STR;
+ spi_nor_set_read_settings(¶ms->reads[SNOR_CMD_READ_8_8_8],
+ 0, 20, SPINOR_OP_READ_8_8_8,
+ SNOR_PROTO_8_8_8);
+
+ spi_nor_set_pp_settings(¶ms->page_programs[SNOR_CMD_PP_8_8_8],
+ SPINOR_OP_PP_8_8_8, SNOR_PROTO_8_8_8);
+
+ /* Octal 8D-8D-8D mode */
+ params->hwcaps.mask |= SNOR_HWCAPS_OPI_FULL_DTR;
+ spi_nor_set_read_settings(¶ms->reads[SNOR_CMD_READ_8D_8D_8D],
+ 0, 20, SPINOR_OP_READ_8D_8D_8D,
+ SNOR_PROTO_8_8_8_DTR);
+
+ spi_nor_set_pp_settings(¶ms->page_programs[SNOR_CMD_PP_8D_8D_8D],
+ SPINOR_OP_PP_8D_8D_8D, SNOR_PROTO_8_8_8_DTR);
+
+ nor->ext_cmd_mode = EXT_CMD_IS_INVERSE;
+}
+
+static void
+mx25uw51245g_post_sfdp_fixups(struct spi_nor *nor)
+{
+ struct spi_nor_flash_parameter *params = &nor->params;
+ u8 cr2;
+
+ cr2 = read_cr2(nor, CR2_REG0) & CR2_REG0_MODE_MASK;
+
+ if (params->hwcaps.mask & SNOR_HWCAPS_OPI_FULL_DTR)
+ cr2 |= CR2_REG0_MODE_OPI_DTR;
+ else if (params->hwcaps.mask & SNOR_HWCAPS_OPI_FULL_STR)
+ cr2 |= CR2_REG0_MODE_OPI_STR;
+
+ write_cr2(nor, CR2_REG0, cr2);
+}
+
static struct spi_nor_fixups mx25l25635_fixups = {
.post_bfpt = mx25l25635_post_bfpt_fixups,
};
+static struct spi_nor_fixups mx25uw51245g_fixups = {
+ .default_init = mx25uw51245g_default_init,
+ .post_sfdp = mx25uw51245g_post_sfdp_fixups,
+};
+
static void gd25q256_default_init(struct spi_nor *nor)
{
/*
@@ -2442,6 +2554,8 @@ static void gd25q256_default_init(struct spi_nor *nor)
SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
.fixups = &mx25l25635_fixups },
{ "mx25u25635f", INFO(0xc22539, 0, 64 * 1024, 512, SECT_4K | SPI_NOR_4B_OPCODES) },
+ { "mx25uw51245g", INFO(0xc2813a, 0, 64 * 1024, 1024, SECT_4K | SPI_NOR_4B_OPCODES)
+ .fixups = &mx25uw51245g_fixups},
{ "mx25v8035f", INFO(0xc22314, 0, 64 * 1024, 16,
SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
{ "mx25l25655e", INFO(0xc22619, 0, 64 * 1024, 512, 0) },
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index 2e720ca..3aa54dd 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -114,6 +114,9 @@
/* Write CFG Reg 2 - defined in JEDEC JESD216C. */
#define SPINOR_OP_WRCR2 0x72 /* Write configuration register 2 */
+/* Used for Macronix flashes only */
+#define SPINOR_OP_RDCR2 0x71 /* Read configuration register 2 */
+
/* Used for Macronix and Winbond flashes. */
#define SPINOR_OP_EN4B 0xb7 /* Enter 4-byte mode */
#define SPINOR_OP_EX4B 0xe9 /* Exit 4-byte mode */
--
1.9.1
patch driver for 8-8-8 and 8D-8D-8D mode support.
Signed-off-by: Mason Yang <[email protected]>
---
drivers/spi/spi-mxic.c | 98 ++++++++++++++++++++++++++++++++++----------------
1 file changed, 67 insertions(+), 31 deletions(-)
diff --git a/drivers/spi/spi-mxic.c b/drivers/spi/spi-mxic.c
index f48563c..50e2055 100644
--- a/drivers/spi/spi-mxic.c
+++ b/drivers/spi/spi-mxic.c
@@ -280,10 +280,58 @@ static void mxic_spi_hw_init(struct mxic_spi *mxic)
mxic->regs + HC_CFG);
}
+static u32 mxic_spi_mem_prep_op_cfg(const struct spi_mem_op *op)
+{
+ u32 cfg = OP_CMD_BYTES(op->cmd.nbytes) |
+ OP_CMD_BUSW(fls(op->cmd.buswidth) - 1) |
+ (op->cmd.dtr ? OP_CMD_DDR : 0);
+
+ if (op->addr.nbytes)
+ cfg |= OP_ADDR_BYTES(op->addr.nbytes) |
+ OP_ADDR_BUSW(fls(op->addr.buswidth) - 1) |
+ (op->addr.dtr ? OP_ADDR_DDR : 0);
+
+ if (op->dummy.nbytes)
+ cfg |= OP_DUMMY_CYC(op->dummy.nbytes);
+
+ if (op->data.nbytes) {
+ cfg |= OP_DATA_BUSW(fls(op->data.buswidth) - 1) |
+ (op->data.dtr ? OP_DATA_DDR : 0);
+ if (op->data.dir == SPI_MEM_DATA_IN) {
+ cfg |= OP_READ;
+ if (op->data.dtr == OP_DATA_DDR)
+ cfg |= OP_DQS_EN;
+ }
+ }
+
+ return cfg;
+}
+
+static void mxic_spi_set_hc_cfg(struct spi_device *spi, u32 flags)
+{
+ struct mxic_spi *mxic = spi_master_get_devdata(spi->master);
+ int nio = 1;
+
+ if (spi->mode & (SPI_RX_OCTAL | SPI_TX_OCTAL))
+ nio = 8;
+ else if (spi->mode & (SPI_TX_QUAD | SPI_RX_QUAD))
+ nio = 4;
+ else if (spi->mode & (SPI_TX_DUAL | SPI_RX_DUAL))
+ nio = 2;
+
+ writel(flags | HC_CFG_NIO(nio) |
+ HC_CFG_TYPE(spi->chip_select, HC_CFG_TYPE_SPI_NOR) |
+ HC_CFG_SLV_ACT(spi->chip_select) | HC_CFG_IDLE_SIO_LVL(1),
+ mxic->regs + HC_CFG);
+}
+
static int mxic_spi_data_xfer(struct mxic_spi *mxic, const void *txbuf,
void *rxbuf, unsigned int len)
{
unsigned int pos = 0;
+ bool dtr_enabled;
+
+ dtr_enabled = (readl(mxic->regs + SS_CTRL(0)) & OP_DATA_DDR);
while (pos < len) {
unsigned int nbytes = len - pos;
@@ -302,6 +350,9 @@ static int mxic_spi_data_xfer(struct mxic_spi *mxic, const void *txbuf,
if (ret)
return ret;
+ if (dtr_enabled && len & 0x1)
+ nbytes++;
+
writel(data, mxic->regs + TXD(nbytes % 4));
if (rxbuf) {
@@ -319,6 +370,8 @@ static int mxic_spi_data_xfer(struct mxic_spi *mxic, const void *txbuf,
data = readl(mxic->regs + RXD);
data >>= (8 * (4 - nbytes));
+ if (dtr_enabled && len & 0x1)
+ nbytes++;
memcpy(rxbuf + pos, &data, nbytes);
WARN_ON(readl(mxic->regs + INT_STS) & INT_RX_NOT_EMPTY);
} else {
@@ -335,8 +388,8 @@ static int mxic_spi_data_xfer(struct mxic_spi *mxic, const void *txbuf,
static bool mxic_spi_mem_supports_op(struct spi_mem *mem,
const struct spi_mem_op *op)
{
- if (op->data.buswidth > 4 || op->addr.buswidth > 4 ||
- op->dummy.buswidth > 4 || op->cmd.buswidth > 4)
+ if (op->data.buswidth > 8 || op->addr.buswidth > 8 ||
+ op->dummy.buswidth > 8 || op->cmd.buswidth > 8)
return false;
if (op->data.nbytes && op->dummy.nbytes &&
@@ -353,47 +406,29 @@ static int mxic_spi_mem_exec_op(struct spi_mem *mem,
const struct spi_mem_op *op)
{
struct mxic_spi *mxic = spi_master_get_devdata(mem->spi->master);
- int nio = 1, i, ret;
- u32 ss_ctrl;
+ int i, ret;
u8 addr[8];
+ u8 cmd[2];
ret = mxic_spi_set_freq(mxic, mem->spi->max_speed_hz);
if (ret)
return ret;
- if (mem->spi->mode & (SPI_TX_QUAD | SPI_RX_QUAD))
- nio = 4;
- else if (mem->spi->mode & (SPI_TX_DUAL | SPI_RX_DUAL))
- nio = 2;
+ mxic_spi_set_hc_cfg(mem->spi, HC_CFG_MAN_CS_EN);
- writel(HC_CFG_NIO(nio) |
- HC_CFG_TYPE(mem->spi->chip_select, HC_CFG_TYPE_SPI_NOR) |
- HC_CFG_SLV_ACT(mem->spi->chip_select) | HC_CFG_IDLE_SIO_LVL(1) |
- HC_CFG_MAN_CS_EN,
- mxic->regs + HC_CFG);
writel(HC_EN_BIT, mxic->regs + HC_EN);
- ss_ctrl = OP_CMD_BYTES(1) | OP_CMD_BUSW(fls(op->cmd.buswidth) - 1);
-
- if (op->addr.nbytes)
- ss_ctrl |= OP_ADDR_BYTES(op->addr.nbytes) |
- OP_ADDR_BUSW(fls(op->addr.buswidth) - 1);
-
- if (op->dummy.nbytes)
- ss_ctrl |= OP_DUMMY_CYC(op->dummy.nbytes);
-
- if (op->data.nbytes) {
- ss_ctrl |= OP_DATA_BUSW(fls(op->data.buswidth) - 1);
- if (op->data.dir == SPI_MEM_DATA_IN)
- ss_ctrl |= OP_READ;
- }
-
- writel(ss_ctrl, mxic->regs + SS_CTRL(mem->spi->chip_select));
+ writel(mxic_spi_mem_prep_op_cfg(op),
+ mxic->regs + SS_CTRL(mem->spi->chip_select));
writel(readl(mxic->regs + HC_CFG) | HC_CFG_MAN_CS_ASSERT,
mxic->regs + HC_CFG);
- ret = mxic_spi_data_xfer(mxic, &op->cmd.opcode, NULL, 1);
+ cmd[0] = op->cmd.opcode;
+ if (op->cmd.nbytes == 2)
+ cmd[1] = op->cmd.ext_opcode;
+
+ ret = mxic_spi_data_xfer(mxic, cmd, NULL, op->cmd.nbytes);
if (ret)
goto out;
@@ -566,7 +601,8 @@ static int mxic_spi_probe(struct platform_device *pdev)
master->bits_per_word_mask = SPI_BPW_MASK(8);
master->mode_bits = SPI_CPOL | SPI_CPHA |
SPI_RX_DUAL | SPI_TX_DUAL |
- SPI_RX_QUAD | SPI_TX_QUAD;
+ SPI_RX_QUAD | SPI_TX_QUAD |
+ SPI_RX_OCTAL | SPI_TX_OCTAL;
mxic_spi_hw_init(mxic);
--
1.9.1
Hi Mason,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on spi/for-next]
[also build test WARNING on v5.4-rc7]
[cannot apply to next-20191114]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Mason-Yang/mtd-spi-nor-Add-support-for-Octal-8D-8D-8D-mode/20191115-170233
base: https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
config: c6x-allyesconfig (attached as .config)
compiler: c6x-elf-gcc (GCC) 7.4.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.4.0 make.cross ARCH=c6x
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <[email protected]>
All warnings (new ones prefixed by >>):
drivers//mtd/spi-nor/spi-nor.c: In function 'erase_chip':
>> drivers//mtd/spi-nor/spi-nor.c:1001:25: warning: large integer implicitly truncated to unsigned type [-Woverflow]
op.cmd.ext_opcode = ~SPINOR_OP_CHIP_ERASE;
^
At top level:
drivers//mtd/spi-nor/spi-nor.c:621:12: warning: 'write_cr2' defined but not used [-Wunused-function]
static int write_cr2(struct spi_nor *nor, u32 addr, u8 val)
^~~~~~~~~
vim +1001 drivers//mtd/spi-nor/spi-nor.c
976
977 /*
978 * Erase the whole flash memory
979 *
980 * Returns 0 if successful, non-zero otherwise.
981 */
982 static int erase_chip(struct spi_nor *nor)
983 {
984 dev_dbg(nor->dev, " %lldKiB\n", (long long)(nor->mtd.size >> 10));
985
986 if (nor->spimem) {
987 struct spi_mem_op op =
988 SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_CHIP_ERASE, 1),
989 SPI_MEM_OP_NO_ADDR,
990 SPI_MEM_OP_NO_DUMMY,
991 SPI_MEM_OP_NO_DATA);
992
993 if (spi_nor_protocol_is_8_8_8(nor->write_proto)) {
994 op.cmd.buswidth = 8;
995 op.cmd.nbytes = 2;
996
997 if (spi_nor_protocol_is_8D_8D_8D(nor->write_proto))
998 op.cmd.dtr = true;
999
1000 if (nor->ext_cmd_mode == EXT_CMD_IS_INVERSE)
> 1001 op.cmd.ext_opcode = ~SPINOR_OP_CHIP_ERASE;
1002 else
1003 op.cmd.ext_opcode = SPINOR_OP_CHIP_ERASE;
1004 }
1005
1006 return spi_mem_exec_op(nor->spimem, &op);
1007 }
1008
1009 return nor->write_reg(nor, SPINOR_OP_CHIP_ERASE, NULL, 0);
1010 }
1011
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/hyperkitty/list/[email protected] Intel Corporation
Hi Mason,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on spi/for-next]
[also build test WARNING on v5.4-rc7]
[cannot apply to next-20191115]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Mason-Yang/mtd-spi-nor-Add-support-for-Octal-8D-8D-8D-mode/20191115-170233
base: https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
config: nds32-allyesconfig (attached as .config)
compiler: nds32le-linux-gcc (GCC) 8.1.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=8.1.0 make.cross ARCH=nds32
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <[email protected]>
All warnings (new ones prefixed by >>):
drivers/mtd/spi-nor/spi-nor.c: In function 'erase_chip':
>> drivers/mtd/spi-nor/spi-nor.c:1001:25: warning: unsigned conversion from 'int' to 'u8' {aka 'unsigned char'} changes value from '-200' to '56' [-Woverflow]
op.cmd.ext_opcode = ~SPINOR_OP_CHIP_ERASE;
^
At top level:
drivers/mtd/spi-nor/spi-nor.c:621:12: warning: 'write_cr2' defined but not used [-Wunused-function]
static int write_cr2(struct spi_nor *nor, u32 addr, u8 val)
^~~~~~~~~
vim +1001 drivers/mtd/spi-nor/spi-nor.c
976
977 /*
978 * Erase the whole flash memory
979 *
980 * Returns 0 if successful, non-zero otherwise.
981 */
982 static int erase_chip(struct spi_nor *nor)
983 {
984 dev_dbg(nor->dev, " %lldKiB\n", (long long)(nor->mtd.size >> 10));
985
986 if (nor->spimem) {
987 struct spi_mem_op op =
988 SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_CHIP_ERASE, 1),
989 SPI_MEM_OP_NO_ADDR,
990 SPI_MEM_OP_NO_DUMMY,
991 SPI_MEM_OP_NO_DATA);
992
993 if (spi_nor_protocol_is_8_8_8(nor->write_proto)) {
994 op.cmd.buswidth = 8;
995 op.cmd.nbytes = 2;
996
997 if (spi_nor_protocol_is_8D_8D_8D(nor->write_proto))
998 op.cmd.dtr = true;
999
1000 if (nor->ext_cmd_mode == EXT_CMD_IS_INVERSE)
> 1001 op.cmd.ext_opcode = ~SPINOR_OP_CHIP_ERASE;
1002 else
1003 op.cmd.ext_opcode = SPINOR_OP_CHIP_ERASE;
1004 }
1005
1006 return spi_mem_exec_op(nor->spimem, &op);
1007 }
1008
1009 return nor->write_reg(nor, SPINOR_OP_CHIP_ERASE, NULL, 0);
1010 }
1011
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/hyperkitty/list/[email protected] Intel Corporation
Hi Mason,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on spi/for-next]
[also build test WARNING on v5.4-rc7]
[cannot apply to next-20191115]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Mason-Yang/mtd-spi-nor-Add-support-for-Octal-8D-8D-8D-mode/20191115-170233
base: https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
config: c6x-allyesconfig (attached as .config)
compiler: c6x-elf-gcc (GCC) 7.4.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.4.0 make.cross ARCH=c6x
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <[email protected]>
All warnings (new ones prefixed by >>):
drivers//spi/spi-mxic.c: In function 'mxic_spi_mem_prep_op_cfg':
>> drivers//spi/spi-mxic.c:302:21: warning: comparison of constant '256' with boolean expression is always false [-Wbool-compare]
if (op->data.dtr == OP_DATA_DDR)
^~
vim +/256 +302 drivers//spi/spi-mxic.c
282
283 static u32 mxic_spi_mem_prep_op_cfg(const struct spi_mem_op *op)
284 {
285 u32 cfg = OP_CMD_BYTES(op->cmd.nbytes) |
286 OP_CMD_BUSW(fls(op->cmd.buswidth) - 1) |
287 (op->cmd.dtr ? OP_CMD_DDR : 0);
288
289 if (op->addr.nbytes)
290 cfg |= OP_ADDR_BYTES(op->addr.nbytes) |
291 OP_ADDR_BUSW(fls(op->addr.buswidth) - 1) |
292 (op->addr.dtr ? OP_ADDR_DDR : 0);
293
294 if (op->dummy.nbytes)
295 cfg |= OP_DUMMY_CYC(op->dummy.nbytes);
296
297 if (op->data.nbytes) {
298 cfg |= OP_DATA_BUSW(fls(op->data.buswidth) - 1) |
299 (op->data.dtr ? OP_DATA_DDR : 0);
300 if (op->data.dir == SPI_MEM_DATA_IN) {
301 cfg |= OP_READ;
> 302 if (op->data.dtr == OP_DATA_DDR)
303 cfg |= OP_DQS_EN;
304 }
305 }
306
307 return cfg;
308 }
309
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/hyperkitty/list/[email protected] Intel Corporation
Hi Mason,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on spi/for-next]
[also build test WARNING on v5.4-rc7]
[cannot apply to next-20191115]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Mason-Yang/mtd-spi-nor-Add-support-for-Octal-8D-8D-8D-mode/20191115-170233
base: https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
config: x86_64-randconfig-a003-20191115 (attached as .config)
compiler: gcc-7 (Debian 7.4.0-14) 7.4.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <[email protected]>
All warnings (new ones prefixed by >>):
In file included from include/linux/err.h:5:0,
from include/linux/clk.h:12,
from drivers//spi/spi-mxic.c:11:
drivers//spi/spi-mxic.c: In function 'mxic_spi_mem_prep_op_cfg':
drivers//spi/spi-mxic.c:302:21: warning: comparison of constant '256' with boolean expression is always false [-Wbool-compare]
if (op->data.dtr == OP_DATA_DDR)
^
include/linux/compiler.h:58:52: note: in definition of macro '__trace_if_var'
#define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
^~~~
>> drivers//spi/spi-mxic.c:302:4: note: in expansion of macro 'if'
if (op->data.dtr == OP_DATA_DDR)
^~
drivers//spi/spi-mxic.c:302:21: warning: comparison of constant '256' with boolean expression is always false [-Wbool-compare]
if (op->data.dtr == OP_DATA_DDR)
^
include/linux/compiler.h:58:61: note: in definition of macro '__trace_if_var'
#define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
^~~~
>> drivers//spi/spi-mxic.c:302:4: note: in expansion of macro 'if'
if (op->data.dtr == OP_DATA_DDR)
^~
drivers//spi/spi-mxic.c:302:21: warning: comparison of constant '256' with boolean expression is always false [-Wbool-compare]
if (op->data.dtr == OP_DATA_DDR)
^
include/linux/compiler.h:69:3: note: in definition of macro '__trace_if_value'
(cond) ? \
^~~~
include/linux/compiler.h:56:28: note: in expansion of macro '__trace_if_var'
#define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
^~~~~~~~~~~~~~
>> drivers//spi/spi-mxic.c:302:4: note: in expansion of macro 'if'
if (op->data.dtr == OP_DATA_DDR)
^~
Cyclomatic Complexity 1 include/linux/err.h:PTR_ERR
Cyclomatic Complexity 1 arch/x86/include/asm/bitops.h:fls
Cyclomatic Complexity 1 include/linux/ktime.h:ktime_add_us
Cyclomatic Complexity 1 arch/x86/include/asm/io.h:readl
Cyclomatic Complexity 1 arch/x86/include/asm/io.h:writel
Cyclomatic Complexity 1 include/linux/device.h:dev_get_drvdata
Cyclomatic Complexity 1 include/linux/device.h:dev_set_drvdata
Cyclomatic Complexity 1 include/linux/platform_device.h:platform_get_drvdata
Cyclomatic Complexity 1 include/linux/platform_device.h:platform_set_drvdata
Cyclomatic Complexity 1 include/linux/spi/spi.h:spi_controller_get_devdata
Cyclomatic Complexity 1 drivers//spi/spi-mxic.c:mxic_spi_set_input_delay_dqs
Cyclomatic Complexity 1 drivers//spi/spi-mxic.c:mxic_spi_hw_init
Cyclomatic Complexity 1 drivers//spi/spi-mxic.c:mxic_spi_driver_init
Cyclomatic Complexity 7 include/linux/ktime.h:ktime_compare
Cyclomatic Complexity 4 drivers//spi/spi-mxic.c:mxic_spi_set_cs
Cyclomatic Complexity 10 drivers//spi/spi-mxic.c:mxic_spi_set_hc_cfg
Cyclomatic Complexity 16 drivers//spi/spi-mxic.c:mxic_spi_mem_prep_op_cfg
Cyclomatic Complexity 7 include/linux/clk.h:clk_prepare_enable
Cyclomatic Complexity 1 include/linux/clk.h:clk_disable_unprepare
Cyclomatic Complexity 7 drivers//spi/spi-mxic.c:mxic_spi_clk_enable
Cyclomatic Complexity 4 drivers//spi/spi-mxic.c:mxic_spi_runtime_resume
Cyclomatic Complexity 1 drivers//spi/spi-mxic.c:mxic_spi_clk_disable
Cyclomatic Complexity 1 drivers//spi/spi-mxic.c:mxic_spi_runtime_suspend
Cyclomatic Complexity 1 include/linux/pm_runtime.h:pm_runtime_disable
Cyclomatic Complexity 1 drivers//spi/spi-mxic.c:mxic_spi_remove
Cyclomatic Complexity 10 drivers//spi/spi-mxic.c:mxic_spi_clk_setup
Cyclomatic Complexity 10 drivers//spi/spi-mxic.c:mxic_spi_set_freq
Cyclomatic Complexity 79 drivers//spi/spi-mxic.c:mxic_spi_data_xfer
Cyclomatic Complexity 42 drivers//spi/spi-mxic.c:mxic_spi_transfer_one
Cyclomatic Complexity 19 drivers//spi/spi-mxic.c:mxic_spi_mem_exec_op
Cyclomatic Complexity 1 include/linux/err.h:IS_ERR
Cyclomatic Complexity 29 drivers//spi/spi-mxic.c:mxic_spi_mem_supports_op
Cyclomatic Complexity 1 include/linux/spi/spi.h:spi_alloc_master
Cyclomatic Complexity 4 include/linux/spi/spi.h:spi_controller_put
Cyclomatic Complexity 15 drivers//spi/spi-mxic.c:mxic_spi_probe
Cyclomatic Complexity 1 drivers//spi/spi-mxic.c:mxic_spi_driver_exit
vim +/if +302 drivers//spi/spi-mxic.c
282
283 static u32 mxic_spi_mem_prep_op_cfg(const struct spi_mem_op *op)
284 {
285 u32 cfg = OP_CMD_BYTES(op->cmd.nbytes) |
286 OP_CMD_BUSW(fls(op->cmd.buswidth) - 1) |
287 (op->cmd.dtr ? OP_CMD_DDR : 0);
288
289 if (op->addr.nbytes)
290 cfg |= OP_ADDR_BYTES(op->addr.nbytes) |
291 OP_ADDR_BUSW(fls(op->addr.buswidth) - 1) |
292 (op->addr.dtr ? OP_ADDR_DDR : 0);
293
294 if (op->dummy.nbytes)
295 cfg |= OP_DUMMY_CYC(op->dummy.nbytes);
296
297 if (op->data.nbytes) {
298 cfg |= OP_DATA_BUSW(fls(op->data.buswidth) - 1) |
299 (op->data.dtr ? OP_DATA_DDR : 0);
300 if (op->data.dir == SPI_MEM_DATA_IN) {
301 cfg |= OP_READ;
> 302 if (op->data.dtr == OP_DATA_DDR)
303 cfg |= OP_DQS_EN;
304 }
305 }
306
307 return cfg;
308 }
309
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/hyperkitty/list/[email protected] Intel Corporation
Hi Mason,
On 15/11/19 2:28 pm, Mason Yang wrote:
> According to JESD216C (JEDEC Basic Flash Parameter Table 18th DWORD)
> Octal DTR(8D-8D-8D) command and command extension (00b: same, 01b: inverse)
> to add extension command mode in spi_nor struct and to add write_cr2
> (Write CFG Reg 2) for 8-8-8/8D-8D-8D mode sequences enable.
>
But I don't see any code setting "nor->ext_cmd_mode" based on BFPT?
Any new feature that we add to spi-nor should make use of autodiscovery
feature made possible by SFDP tables. Could you modify the patch to
discover capabilities supported by flash and opcodes to be used from
SFDP table?
> Define the relevant macrons and enum to add such modes and make sure
> op->xxx.dtr fields, command nbytes and extension command are properly
> filled and unmask DTR and X-X-X modes in spi_nor_spimem_adjust_hwcaps()
> so that DTR and X-X-X support detection is done through
> spi_mem_supports_op().
>
> Signed-off-by: Boris Brezillon <[email protected]>
> Signed-off-by: Mason Yang <[email protected]>
> ---
> drivers/mtd/spi-nor/spi-nor.c | 159 ++++++++++++++++++++++++++++++++++++++++--
> include/linux/mtd/spi-nor.h | 58 +++++++++++++--
> 2 files changed, 206 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 7acf4a9..4cdf52d 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -320,6 +320,23 @@ static ssize_t spi_nor_spimem_read_data(struct spi_nor *nor, loff_t from,
> /* convert the dummy cycles to the number of bytes */
> op.dummy.nbytes = (nor->read_dummy * op.dummy.buswidth) / 8;
>
> + if (spi_nor_protocol_is_8_8_8(nor->read_proto)) {
> + op.cmd.nbytes = 2;
Can we get this info from SFDP too?
> +
> + if (nor->ext_cmd_mode == EXT_CMD_IS_INVERSE)
> + op.cmd.ext_opcode = ~nor->read_opcode;
> + else
> + op.cmd.ext_opcode = nor->read_opcode;
> +
> + if (spi_nor_protocol_is_8D_8D_8D(nor->read_proto)) {
> + op.dummy.nbytes *= 2;
> + op.cmd.dtr = true;
> + op.addr.dtr = true;
> + op.dummy.dtr = true;
> + op.data.dtr = true;
Can we have a macro for this initialization?
> + }
> + }
> +
> return spi_nor_spimem_xfer_data(nor, &op);
> }
>
> @@ -367,6 +384,21 @@ static ssize_t spi_nor_spimem_write_data(struct spi_nor *nor, loff_t to,
> if (nor->program_opcode == SPINOR_OP_AAI_WP && nor->sst_write_second)
> op.addr.nbytes = 0;
>
> + if (spi_nor_protocol_is_8_8_8(nor->write_proto)) {
> + op.cmd.nbytes = 2;
> +
> + if (nor->ext_cmd_mode == EXT_CMD_IS_INVERSE)
> + op.cmd.ext_opcode = ~nor->program_opcode;
> + else
> + op.cmd.ext_opcode = nor->program_opcode;
> +
> + if (spi_nor_protocol_is_8D_8D_8D(nor->write_proto)) {
> + op.cmd.dtr = true;
> + op.addr.dtr = true;
> + op.data.dtr = true;
> + }
> + }
> +
> return spi_nor_spimem_xfer_data(nor, &op);
> }
>
> @@ -404,6 +436,30 @@ static int read_sr(struct spi_nor *nor)
> SPI_MEM_OP_NO_DUMMY,
> SPI_MEM_OP_DATA_IN(1, nor->bouncebuf, 1));
>
This is not based on the latest tree.
> + if (spi_nor_protocol_is_8_8_8(nor->read_proto)) {
> + op.cmd.buswidth = 8;
> + op.addr.buswidth = 8;
> + op.dummy.buswidth = 8;
> + op.data.buswidth = 8;
> + op.cmd.nbytes = 2;
> + op.addr.nbytes = 4;
> + op.dummy.nbytes = 4;
> + op.addr.val = 0;
This is not scalable... There will be bunch of if...else ladders when we
want to support other X-X-X modes... Can't these be derived from
nor->reg_proto? And then borrow the logic from spi_nor_spimem_read_data()?
Could you have a look at Boris's initial submission to add 8-8-8 mode at
https://patchwork.kernel.org/cover/10638055/ ?
You could use that series as the base for your changes/additions.
I am fine if you want to call 2nd byte of opcode as ext_opcode
Regards
Vignesh
> +
> + if (nor->ext_cmd_mode == EXT_CMD_IS_INVERSE)
> + op.cmd.ext_opcode = ~SPINOR_OP_RDSR;
> + else
> + op.cmd.ext_opcode = SPINOR_OP_RDSR;
> +
> + if (spi_nor_protocol_is_8D_8D_8D(nor->read_proto)) {
> + op.dummy.nbytes *= 2;
> + op.cmd.dtr = true;
> + op.addr.dtr = true;
> + op.dummy.dtr = true;
> + op.data.dtr = true;
> + }
> + }
> +
> ret = spi_mem_exec_op(nor->spimem, &op);
> } else {
> ret = nor->read_reg(nor, SPINOR_OP_RDSR, nor->bouncebuf, 1);
> @@ -508,6 +564,19 @@ static int write_enable(struct spi_nor *nor)
> SPI_MEM_OP_NO_DUMMY,
> SPI_MEM_OP_NO_DATA);
>
> + if (spi_nor_protocol_is_8_8_8(nor->write_proto)) {
> + op.cmd.buswidth = 8;
> + op.cmd.nbytes = 2;
> +
> + if (spi_nor_protocol_is_8D_8D_8D(nor->write_proto))
> + op.cmd.dtr = true;
> +
> + if (nor->ext_cmd_mode == EXT_CMD_IS_INVERSE)
> + op.cmd.ext_opcode = ~SPINOR_OP_WREN;
> + else
> + op.cmd.ext_opcode = SPINOR_OP_WREN;
> + }
> +
> return spi_mem_exec_op(nor->spimem, &op);
> }
>
> @@ -526,12 +595,65 @@ static int write_disable(struct spi_nor *nor)
> SPI_MEM_OP_NO_DUMMY,
> SPI_MEM_OP_NO_DATA);
>
> + if (spi_nor_protocol_is_8_8_8(nor->write_proto)) {
> + op.cmd.buswidth = 8;
> + op.cmd.nbytes = 2;
> +
> + if (spi_nor_protocol_is_8D_8D_8D(nor->write_proto))
> + op.cmd.dtr = true;
> +
> + if (nor->ext_cmd_mode == EXT_CMD_IS_INVERSE)
> + op.cmd.ext_opcode = ~SPINOR_OP_WRDI;
> + else
> + op.cmd.ext_opcode = SPINOR_OP_WRDI;
> + }
> +
> return spi_mem_exec_op(nor->spimem, &op);
> }
>
> return nor->write_reg(nor, SPINOR_OP_WRDI, NULL, 0);
> }
>
> +/*
> + * Write configuration register 2 one byte
> + * Returns negative if error occurred.
> + */
> +static int write_cr2(struct spi_nor *nor, u32 addr, u8 val)
> +{
> + write_enable(nor);
> +
> + nor->bouncebuf[0] = val;
> + if (nor->spimem) {
> + struct spi_mem_op op =
> + SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WRCR2, 1),
> + SPI_MEM_OP_ADDR(4, addr, 1),
> + SPI_MEM_OP_NO_DUMMY,
> + SPI_MEM_OP_DATA_OUT(1, nor->bouncebuf, 1));
> +
> + if (spi_nor_protocol_is_8_8_8(nor->write_proto)) {
> + op.cmd.buswidth = 8;
> + op.addr.buswidth = 8;
> + op.data.buswidth = 8;
> + op.cmd.nbytes = 2;
> +
> + if (spi_nor_protocol_is_8D_8D_8D(nor->write_proto)) {
> + op.cmd.dtr = true;
> + op.addr.dtr = true;
> + op.data.dtr = true;
> + }
> +
> + if (nor->ext_cmd_mode == EXT_CMD_IS_INVERSE)
> + op.cmd.ext_opcode = ~SPINOR_OP_WRCR2;
> + else
> + op.cmd.ext_opcode = SPINOR_OP_WRCR2;
> + }
> +
> + return spi_mem_exec_op(nor->spimem, &op);
> + }
> +
> + return -ENOTSUPP;
> +}
> +
> static struct spi_nor *mtd_to_spi_nor(struct mtd_info *mtd)
> {
> return mtd->priv;
> @@ -868,6 +990,19 @@ static int erase_chip(struct spi_nor *nor)
> SPI_MEM_OP_NO_DUMMY,
> SPI_MEM_OP_NO_DATA);
>
> + if (spi_nor_protocol_is_8_8_8(nor->write_proto)) {
> + op.cmd.buswidth = 8;
> + op.cmd.nbytes = 2;
> +
> + if (spi_nor_protocol_is_8D_8D_8D(nor->write_proto))
> + op.cmd.dtr = true;
> +
> + if (nor->ext_cmd_mode == EXT_CMD_IS_INVERSE)
> + op.cmd.ext_opcode = ~SPINOR_OP_CHIP_ERASE;
> + else
> + op.cmd.ext_opcode = SPINOR_OP_CHIP_ERASE;
> + }
> +
> return spi_mem_exec_op(nor->spimem, &op);
> }
>
> @@ -945,6 +1080,22 @@ static int spi_nor_erase_sector(struct spi_nor *nor, u32 addr)
> SPI_MEM_OP_NO_DUMMY,
> SPI_MEM_OP_NO_DATA);
>
> + if (spi_nor_protocol_is_8_8_8(nor->write_proto)) {
> + op.cmd.buswidth = 8;
> + op.addr.buswidth = 8;
> + op.cmd.nbytes = 2;
> +
> + if (spi_nor_protocol_is_8D_8D_8D(nor->write_proto)) {
> + op.cmd.dtr = true;
> + op.addr.dtr = true;
> + }
> +
> + if (nor->ext_cmd_mode == EXT_CMD_IS_INVERSE)
> + op.cmd.ext_opcode = ~nor->erase_opcode;
> + else
> + op.cmd.ext_opcode = nor->erase_opcode;
> + }
> +
> return spi_mem_exec_op(nor->spimem, &op);
> }
>
> @@ -2825,6 +2976,7 @@ static 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_8D_8D_8D },
> };
>
> return spi_nor_hwcaps2cmd(hwcaps, hwcaps_read2cmd,
> @@ -2841,6 +2993,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_8D_8D_8D },
> };
>
> return spi_nor_hwcaps2cmd(hwcaps, hwcaps_pp2cmd,
> @@ -3010,12 +3163,6 @@ static int spi_nor_spimem_check_pp(struct spi_nor *nor,
> 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;
> -
> for (cap = 0; cap < sizeof(*hwcaps) * BITS_PER_BYTE; cap++) {
> int rdidx, ppidx;
>
> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> index fc0b4b1..2e720ca 100644
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -84,6 +84,9 @@
> #define SPINOR_OP_BE_4K_4B 0x21 /* Erase 4KiB block */
> #define SPINOR_OP_BE_32K_4B 0x5c /* Erase 32KiB block */
> #define SPINOR_OP_SE_4B 0xdc /* Sector erase (usually 64KiB) */
> +#define SPINOR_OP_READ_8_8_8 SPINOR_OP_READ_1_4_4_4B
> +#define SPINOR_OP_PP_8_8_8 SPINOR_OP_PP_4B
> +#define SPINOR_OP_PP_8D_8D_8D SPINOR_OP_PP_4B
>
> /* Double Transfer Rate opcodes - defined in JEDEC JESD216B. */
> #define SPINOR_OP_READ_1_1_1_DTR 0x0d
> @@ -93,6 +96,7 @@
> #define SPINOR_OP_READ_1_1_1_DTR_4B 0x0e
> #define SPINOR_OP_READ_1_2_2_DTR_4B 0xbe
> #define SPINOR_OP_READ_1_4_4_DTR_4B 0xee
> +#define SPINOR_OP_READ_8D_8D_8D SPINOR_OP_READ_1_4_4_DTR_4B
>
> /* Used for SST flashes only. */
> #define SPINOR_OP_BP 0x02 /* Byte program */
> @@ -107,6 +111,8 @@
> #define XSR_PAGESIZE BIT(0) /* Page size in Po2 or Linear */
> #define XSR_RDY BIT(7) /* Ready */
>
> +/* Write CFG Reg 2 - defined in JEDEC JESD216C. */
> +#define SPINOR_OP_WRCR2 0x72 /* Write configuration register 2 */
>
> /* Used for Macronix and Winbond flashes. */
> #define SPINOR_OP_EN4B 0xb7 /* Enter 4-byte mode */
> @@ -150,6 +156,13 @@
> /* Status Register 2 bits. */
> #define SR2_QUAD_EN_BIT7 BIT(7)
>
> +/* Configuration register 2, offset 0 */
> +#define CR2_REG0 0x0
> +#define CR2_REG0_MODE_MASK GENMASK(1, 0)
> +#define CR2_REG0_MODE_SPI 0
> +#define CR2_REG0_MODE_OPI_STR 1
> +#define CR2_REG0_MODE_OPI_DTR 2
> +
> /* Supported SPI protocols */
> #define SNOR_PROTO_INST_MASK GENMASK(23, 16)
> #define SNOR_PROTO_INST_SHIFT 16
> @@ -170,6 +183,7 @@
> SNOR_PROTO_DATA_MASK)
>
> #define SNOR_PROTO_IS_DTR BIT(24) /* Double Transfer Rate */
> +#define SNOR_PROTO_IS_8D_8D_8D BIT(25) /* Full Octal DTR */
>
> #define SNOR_PROTO_STR(_inst_nbits, _addr_nbits, _data_nbits) \
> (SNOR_PROTO_INST(_inst_nbits) | \
> @@ -179,6 +193,10 @@
> (SNOR_PROTO_IS_DTR | \
> SNOR_PROTO_STR(_inst_nbits, _addr_nbits, _data_nbits))
>
> +#define SNOR_PROTO_8D_8D_8D(_nbits) \
> + (SNOR_PROTO_IS_8D_8D_8D | \
> + SNOR_PROTO_STR(_nbits, _nbits, _nbits))
> +
> enum spi_nor_protocol {
> SNOR_PROTO_1_1_1 = SNOR_PROTO_STR(1, 1, 1),
> SNOR_PROTO_1_1_2 = SNOR_PROTO_STR(1, 1, 2),
> @@ -195,6 +213,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_8D_8D_8D(8),
> };
>
> static inline bool spi_nor_protocol_is_dtr(enum spi_nor_protocol proto)
> @@ -202,6 +221,16 @@ static inline bool spi_nor_protocol_is_dtr(enum spi_nor_protocol proto)
> return !!(proto & SNOR_PROTO_IS_DTR);
> }
>
> +static inline bool spi_nor_protocol_is_8_8_8(enum spi_nor_protocol proto)
> +{
> + return !!(proto & SNOR_PROTO_8_8_8);
> +}
> +
> +static inline bool spi_nor_protocol_is_8D_8D_8D(enum spi_nor_protocol proto)
> +{
> + return !!(proto & SNOR_PROTO_IS_8D_8D_8D);
> +}
> +
> static inline u8 spi_nor_get_protocol_inst_nbits(enum spi_nor_protocol proto)
> {
> return ((unsigned long)(proto & SNOR_PROTO_INST_MASK)) >>
> @@ -349,7 +378,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)
> @@ -366,11 +395,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.
> @@ -381,7 +411,7 @@ 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_MASK GENMASK(23, 16)
> #define SNOR_HWCAPS_PP BIT(16)
>
> #define SNOR_HWCAPS_PP_QUAD GENMASK(19, 17)
> @@ -389,10 +419,17 @@ struct spi_nor_hwcaps {
> #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_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_OPI_FULL_STR (SNOR_HWCAPS_READ_8_8_8 | \
> + SNOR_HWCAPS_PP_8_8_8)
> +
> +#define SNOR_HWCAPS_OPI_FULL_DTR (SNOR_HWCAPS_READ_8_8_8_DTR | \
> + SNOR_HWCAPS_PP_8_8_8_DTR)
>
> #define SNOR_HWCAPS_X_X_X (SNOR_HWCAPS_READ_2_2_2 | \
> SNOR_HWCAPS_READ_4_4_4 | \
> @@ -403,7 +440,9 @@ 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 | \
> + SNOR_HWCAPS_PP_8_8_8_DTR)
>
> #define SNOR_HWCAPS_ALL (SNOR_HWCAPS_READ_MASK | \
> SNOR_HWCAPS_PP_MASK)
> @@ -442,6 +481,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_8D_8D_8D,
>
> SNOR_CMD_READ_MAX
> };
> @@ -458,6 +498,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_8D_8D_8D,
>
> SNOR_CMD_PP_MAX
> };
> @@ -528,6 +569,11 @@ struct spi_nor_flash_parameter {
> */
> struct flash_info;
>
> +enum extension_cmd_mode {
> + EXT_CMD_IS_CMD,
> + EXT_CMD_IS_INVERSE,
> +};
> +
> /**
> * struct spi_nor - Structure for defining a the SPI NOR layer
> * @mtd: point to a mtd_info structure
> @@ -537,6 +583,7 @@ struct spi_nor_flash_parameter {
> * @bouncebuf: bounce buffer used when the buffer passed by the MTD
> * layer is not DMA-able
> * @bouncebuf_size: size of the bounce buffer
> + * @ext_cmd_mode: extension command mode, 0: same, 1: inverse
> * @info: spi-nor part JDEC MFR id and other info
> * @page_size: the page size of the SPI NOR
> * @addr_width: number of address bytes
> @@ -575,6 +622,7 @@ struct spi_nor {
> struct spi_mem *spimem;
> u8 *bouncebuf;
> size_t bouncebuf_size;
> + enum extension_cmd_mode ext_cmd_mode;
> const struct flash_info *info;
> u32 page_size;
> u8 addr_width;
>
--
Regards
Vignesh
On 15/11/19 2:28 pm, Mason Yang wrote:
> mx25uw51245g is a SPI NOR that supports 1-1-1/8-8-8 mode and the SFDPRD
> command but returns an empty SFDP page. This forces us to add a new entry
> in the flash_info table and patch spi_nor_fixups hooks to tweak flash
> parameters for spi_nor_read/pp_setting() and then to enter
> Octal 8D-8D-8D modes.
>
> Signed-off-by: Boris Brezillon <[email protected]>
> Signed-off-by: Mason Yang <[email protected]>
> ---
> drivers/mtd/spi-nor/spi-nor.c | 114 ++++++++++++++++++++++++++++++++++++++++++
> include/linux/mtd/spi-nor.h | 3 ++
> 2 files changed, 117 insertions(+)
>
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 4cdf52d..9013590 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -615,6 +615,56 @@ static int write_disable(struct spi_nor *nor)
> }
>
> /*
> + * Read configuration register 2, returning its value in the
> + * location. Return the configuration register 2 value.
> + * Returns negative if error occurred.
> + */
> +static int read_cr2(struct spi_nor *nor, u32 addr)
Please prefix spi_nor_* for all new functions.
Also, include manf name if its vendor specific.
> +{
> + int ret;
> +
> + if (nor->spimem) {
> + struct spi_mem_op op =
> + SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_RDCR2, 1),
> + SPI_MEM_OP_ADDR(4, addr, 1),
> + SPI_MEM_OP_DUMMY(4, 1),
> + SPI_MEM_OP_DATA_IN(1, nor->bouncebuf, 1));
> +
> + if (spi_nor_protocol_is_8_8_8(nor->read_proto)) {
> + op.cmd.buswidth = 8;
> + op.addr.buswidth = 8;
> + op.dummy.buswidth = 8;
> + op.data.buswidth = 8;
> + op.cmd.nbytes = 2;
> +
> + if (spi_nor_protocol_is_8D_8D_8D(nor->read_proto)) {
> + op.dummy.nbytes *= 2;
> + op.cmd.dtr = true;
> + op.addr.dtr = true;
> + op.dummy.dtr = true;
> + op.data.dtr = true;
> + }
> +
> + if (nor->ext_cmd_mode == EXT_CMD_IS_INVERSE)
> + op.cmd.ext_opcode = ~SPINOR_OP_RDCR2;
> + else
> + op.cmd.ext_opcode = SPINOR_OP_RDCR2;
> + }
> +
> + ret = spi_mem_exec_op(nor->spimem, &op);
> + } else {
> + ret = -ENOTSUPP;
> + }
> +
> + if (ret < 0) {
> + dev_err(nor->dev, "error %d reading CR\n", ret);
> + return ret;
> + }
> +
> + return nor->bouncebuf[0];
> +}
> +
> +/*
> * Write configuration register 2 one byte
> * Returns negative if error occurred.
> */
> @@ -2275,10 +2325,72 @@ static int spi_nor_spansion_clear_sr_bp(struct spi_nor *nor)
> return 0;
> }
>
> +static 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);
> +
> +static void
> +spi_nor_set_pp_settings(struct spi_nor_pp_command *pp,
> + u8 opcode,
> + enum spi_nor_protocol proto);
> +
> +static void
> +mx25uw51245g_default_init(struct spi_nor *nor)
> +{
> + struct spi_nor_flash_parameter *params = &nor->params;
> +
> + if (!(nor->spimem->spi->mode & (SPI_RX_OCTAL | SPI_TX_OCTAL)))
> + return;
> +
> + /* Octal 8S-8S-8S mode */
> + params->hwcaps.mask |= SNOR_HWCAPS_OPI_FULL_STR;
> + spi_nor_set_read_settings(¶ms->reads[SNOR_CMD_READ_8_8_8],
> + 0, 20, SPINOR_OP_READ_8_8_8,
> + SNOR_PROTO_8_8_8);
> +
> + spi_nor_set_pp_settings(¶ms->page_programs[SNOR_CMD_PP_8_8_8],
> + SPINOR_OP_PP_8_8_8, SNOR_PROTO_8_8_8);
> +
> + /* Octal 8D-8D-8D mode */
> + params->hwcaps.mask |= SNOR_HWCAPS_OPI_FULL_DTR;
> + spi_nor_set_read_settings(¶ms->reads[SNOR_CMD_READ_8D_8D_8D],
> + 0, 20, SPINOR_OP_READ_8D_8D_8D,
> + SNOR_PROTO_8_8_8_DTR);
> +
> + spi_nor_set_pp_settings(¶ms->page_programs[SNOR_CMD_PP_8D_8D_8D],
> + SPINOR_OP_PP_8D_8D_8D, SNOR_PROTO_8_8_8_DTR);
> +
> + nor->ext_cmd_mode = EXT_CMD_IS_INVERSE;
> +}
I don't see anything that is macronix specific here.. Can this be moved to
generic code with information parsed from SFDP table?
> +
> +static void
> +mx25uw51245g_post_sfdp_fixups(struct spi_nor *nor)
> +{
> + struct spi_nor_flash_parameter *params = &nor->params;
> + u8 cr2;
> +
> + cr2 = read_cr2(nor, CR2_REG0) & CR2_REG0_MODE_MASK;
> +
> + if (params->hwcaps.mask & SNOR_HWCAPS_OPI_FULL_DTR)
> + cr2 |= CR2_REG0_MODE_OPI_DTR;
> + else if (params->hwcaps.mask & SNOR_HWCAPS_OPI_FULL_STR)
> + cr2 |= CR2_REG0_MODE_OPI_STR;
> +
> + write_cr2(nor, CR2_REG0, cr2);
> +}
> +
I see this as a misuse of sfdp_fixups hook:
* @post_sfdp: called after SFDP has been parsed (is also called for SPI NORs
* that do not support RDSFDP). Typically used to tweak various
* parameters that could not be extracted by other means (i.e.
* when information provided by the SFDP/flash_info tables are
* incomplete or wrong).
*
This should only tweak options parsed by SFDP and not be used to
configure flash to a different mode. Please add a separate function
to do so. See https://patchwork.kernel.org/patch/10638085/
Also, I guess JESD216C provides a way to discover command sequence
to enter OPI mode?
> static struct spi_nor_fixups mx25l25635_fixups = {
> .post_bfpt = mx25l25635_post_bfpt_fixups,
> };
>
> +static struct spi_nor_fixups mx25uw51245g_fixups = {
> + .default_init = mx25uw51245g_default_init,
> + .post_sfdp = mx25uw51245g_post_sfdp_fixups,
> +};
> +
> static void gd25q256_default_init(struct spi_nor *nor)
> {
> /*
> @@ -2442,6 +2554,8 @@ static void gd25q256_default_init(struct spi_nor *nor)
> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
> .fixups = &mx25l25635_fixups },
> { "mx25u25635f", INFO(0xc22539, 0, 64 * 1024, 512, SECT_4K | SPI_NOR_4B_OPCODES) },
> + { "mx25uw51245g", INFO(0xc2813a, 0, 64 * 1024, 1024, SECT_4K | SPI_NOR_4B_OPCODES)
> + .fixups = &mx25uw51245g_fixups},
> { "mx25v8035f", INFO(0xc22314, 0, 64 * 1024, 16,
> SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> { "mx25l25655e", INFO(0xc22619, 0, 64 * 1024, 512, 0) },
> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> index 2e720ca..3aa54dd 100644
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -114,6 +114,9 @@
> /* Write CFG Reg 2 - defined in JEDEC JESD216C. */
> #define SPINOR_OP_WRCR2 0x72 /* Write configuration register 2 */
>
> +/* Used for Macronix flashes only */
> +#define SPINOR_OP_RDCR2 0x71 /* Read configuration register 2 */
> +
> /* Used for Macronix and Winbond flashes. */
> #define SPINOR_OP_EN4B 0xb7 /* Enter 4-byte mode */
> #define SPINOR_OP_EX4B 0xe9 /* Exit 4-byte mode */
>
--
Regards
Vignesh
Hi Vignesh,
> >
> > /*
> > + * Read configuration register 2, returning its value in the
> > + * location. Return the configuration register 2 value.
> > + * Returns negative if error occurred.
> > + */
> > +static int read_cr2(struct spi_nor *nor, u32 addr)
>
> Please prefix spi_nor_* for all new functions.
> Also, include manf name if its vendor specific.
okay, will fix it.
>
> > +{
> > + int ret;
> > +
> > + if (nor->spimem) {
> > + struct spi_mem_op op =
> > + SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_RDCR2, 1),
> > + SPI_MEM_OP_ADDR(4, addr, 1),
> > + SPI_MEM_OP_DUMMY(4, 1),
> > + SPI_MEM_OP_DATA_IN(1, nor->bouncebuf, 1));
> > +
> > + if (spi_nor_protocol_is_8_8_8(nor->read_proto)) {
> > + op.cmd.buswidth = 8;
> > + op.addr.buswidth = 8;
> > + op.dummy.buswidth = 8;
> > + op.data.buswidth = 8;
> > + op.cmd.nbytes = 2;
> > +
> > + if (spi_nor_protocol_is_8D_8D_8D(nor->read_proto)) {
> > + op.dummy.nbytes *= 2;
> > + op.cmd.dtr = true;
> > + op.addr.dtr = true;
> > + op.dummy.dtr = true;
> > + op.data.dtr = true;
> > + }
> > +
> > + if (nor->ext_cmd_mode == EXT_CMD_IS_INVERSE)
> > + op.cmd.ext_opcode = ~SPINOR_OP_RDCR2;
> > + else
> > + op.cmd.ext_opcode = SPINOR_OP_RDCR2;
> > + }
> > +
> > + ret = spi_mem_exec_op(nor->spimem, &op);
> > + } else {
> > + ret = -ENOTSUPP;
> > + }
> > +
> > + if (ret < 0) {
> > + dev_err(nor->dev, "error %d reading CR\n", ret);
> > + return ret;
> > + }
> > +
> > + return nor->bouncebuf[0];
> > +}
> > +
> > +/*
> > * Write configuration register 2 one byte
> > * Returns negative if error occurred.
> > */
> > @@ -2275,10 +2325,72 @@ static int spi_nor_spansion_clear_sr_bp(struct
spi_nor *nor)
> > return 0;
> > }
> >
> > +static 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);
> > +
> > +static void
> > +spi_nor_set_pp_settings(struct spi_nor_pp_command *pp,
> > + u8 opcode,
> > + enum spi_nor_protocol proto);
> > +
> > +static void
> > +mx25uw51245g_default_init(struct spi_nor *nor)
> > +{
> > + struct spi_nor_flash_parameter *params = &nor->params;
> > +
> > + if (!(nor->spimem->spi->mode & (SPI_RX_OCTAL | SPI_TX_OCTAL)))
> > + return;
> > +
> > + /* Octal 8S-8S-8S mode */
> > + params->hwcaps.mask |= SNOR_HWCAPS_OPI_FULL_STR;
> > + spi_nor_set_read_settings(¶ms->reads[SNOR_CMD_READ_8_8_8],
> > + 0, 20, SPINOR_OP_READ_8_8_8,
> > + SNOR_PROTO_8_8_8);
> > +
> > + spi_nor_set_pp_settings(¶ms->page_programs[SNOR_CMD_PP_8_8_8],
> > + SPINOR_OP_PP_8_8_8, SNOR_PROTO_8_8_8);
> > +
> > + /* Octal 8D-8D-8D mode */
> > + params->hwcaps.mask |= SNOR_HWCAPS_OPI_FULL_DTR;
> > + spi_nor_set_read_settings(¶ms->reads[SNOR_CMD_READ_8D_8D_8D],
> > + 0, 20, SPINOR_OP_READ_8D_8D_8D,
> > + SNOR_PROTO_8_8_8_DTR);
> > +
> > +
spi_nor_set_pp_settings(¶ms->page_programs[SNOR_CMD_PP_8D_8D_8D],
> > + SPINOR_OP_PP_8D_8D_8D, SNOR_PROTO_8_8_8_DTR);
> > +
> > + nor->ext_cmd_mode = EXT_CMD_IS_INVERSE;
> > +}
>
> I don't see anything that is macronix specific here.. Can this be moved
to
> generic code with information parsed from SFDP table?
This mx25uw51245g device support SFDP command but returns an empty SFDP
page.
>
> > +
> > +static void
> > +mx25uw51245g_post_sfdp_fixups(struct spi_nor *nor)
> > +{
> > + struct spi_nor_flash_parameter *params = &nor->params;
> > + u8 cr2;
> > +
> > + cr2 = read_cr2(nor, CR2_REG0) & CR2_REG0_MODE_MASK;
> > +
> > + if (params->hwcaps.mask & SNOR_HWCAPS_OPI_FULL_DTR)
> > + cr2 |= CR2_REG0_MODE_OPI_DTR;
> > + else if (params->hwcaps.mask & SNOR_HWCAPS_OPI_FULL_STR)
> > + cr2 |= CR2_REG0_MODE_OPI_STR;
> > +
> > + write_cr2(nor, CR2_REG0, cr2);
> > +}
> > +
>
> I see this as a misuse of sfdp_fixups hook:
>
> * @post_sfdp: called after SFDP has been parsed (is also called for SPI
NORs
> * that do not support RDSFDP). Typically used to tweak
various
> * parameters that could not be extracted by other means
(i.e.
> * when information provided by the SFDP/flash_info tables
are
> * incomplete or wrong).
> *
>
>
> This should only tweak options parsed by SFDP and not be used to
> configure flash to a different mode. Please add a separate function
> to do so. See https://patchwork.kernel.org/patch/10638085/
>
okay.
My idea is that device changed to 8D-8D-8D stateful mode after SFDP
parsed.
But if SFDP page table is broken in device and driver will just configure
the device into 8D-8D-8D mode directly.
thanks for your time & comments.
Mason
CONFIDENTIALITY NOTE:
This e-mail and any attachments may contain confidential information
and/or personal data, which is protected by applicable laws. Please be
reminded that duplication, disclosure, distribution, or use of this e-mail
(and/or its attachments) or any part thereof is prohibited. If you receive
this e-mail in error, please notify us immediately and delete this mail as
well as its attachment(s) from your system. In addition, please be
informed that collection, processing, and/or use of personal data is
prohibited unless expressly permitted by personal data protection laws.
Thank you for your attention and cooperation.
Macronix International Co., Ltd.
=====================================================================
============================================================================
CONFIDENTIALITY NOTE:
This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation.
Macronix International Co., Ltd.
=====================================================================
Hi Vignesh,
>
> On 15/11/19 2:28 pm, Mason Yang wrote:
> > According to JESD216C (JEDEC Basic Flash Parameter Table 18th DWORD)
> > Octal DTR(8D-8D-8D) command and command extension (00b: same, 01b:
inverse)
> > to add extension command mode in spi_nor struct and to add write_cr2
> > (Write CFG Reg 2) for 8-8-8/8D-8D-8D mode sequences enable.
> >
>
> But I don't see any code setting "nor->ext_cmd_mode" based on BFPT?
>
> Any new feature that we add to spi-nor should make use of autodiscovery
> feature made possible by SFDP tables. Could you modify the patch to
> discover capabilities supported by flash and opcodes to be used from
> SFDP table?
Got it but our device will return a empty SFDP table.
>
>
> > Define the relevant macrons and enum to add such modes and make sure
> > op->xxx.dtr fields, command nbytes and extension command are properly
> > filled and unmask DTR and X-X-X modes in
spi_nor_spimem_adjust_hwcaps()
> > so that DTR and X-X-X support detection is done through
> > spi_mem_supports_op().
> >
> > Signed-off-by: Boris Brezillon <[email protected]>
> > Signed-off-by: Mason Yang <[email protected]>
> > ---
> > drivers/mtd/spi-nor/spi-nor.c | 159
++++++++++++++++++++++++++++++++++++++++--
> > include/linux/mtd/spi-nor.h | 58 +++++++++++++--
> > 2 files changed, 206 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/mtd/spi-nor/spi-nor.c
b/drivers/mtd/spi-nor/spi-nor.c
> > index 7acf4a9..4cdf52d 100644
> > --- a/drivers/mtd/spi-nor/spi-nor.c
> > +++ b/drivers/mtd/spi-nor/spi-nor.c
> > @@ -320,6 +320,23 @@ static ssize_t spi_nor_spimem_read_data(struct
spi_nor
> *nor, loff_t from,
> > /* convert the dummy cycles to the number of bytes */
> > op.dummy.nbytes = (nor->read_dummy * op.dummy.buswidth) / 8;
> >
> > + if (spi_nor_protocol_is_8_8_8(nor->read_proto)) {
> > + op.cmd.nbytes = 2;
>
> Can we get this info from SFDP too?
>
> > +
> > + if (nor->ext_cmd_mode == EXT_CMD_IS_INVERSE)
> > + op.cmd.ext_opcode = ~nor->read_opcode;
> > + else
> > + op.cmd.ext_opcode = nor->read_opcode;
> > +
> > + if (spi_nor_protocol_is_8D_8D_8D(nor->read_proto)) {
> > + op.dummy.nbytes *= 2;
>
> > + op.cmd.dtr = true;
> > + op.addr.dtr = true;
> > + op.dummy.dtr = true;
> > + op.data.dtr = true;
>
> Can we have a macro for this initialization?
okay, will fix it.
>
>
> > + }
> > + }
> > +
> > return spi_nor_spimem_xfer_data(nor, &op);
> > }
> >
> > @@ -367,6 +384,21 @@ static ssize_t spi_nor_spimem_write_data(struct
spi_nor
> *nor, loff_t to,
> > if (nor->program_opcode == SPINOR_OP_AAI_WP &&
nor->sst_write_second)
> > op.addr.nbytes = 0;
> >
> > + if (spi_nor_protocol_is_8_8_8(nor->write_proto)) {
> > + op.cmd.nbytes = 2;
> > +
> > + if (nor->ext_cmd_mode == EXT_CMD_IS_INVERSE)
> > + op.cmd.ext_opcode = ~nor->program_opcode;
> > + else
> > + op.cmd.ext_opcode = nor->program_opcode;
> > +
> > + if (spi_nor_protocol_is_8D_8D_8D(nor->write_proto)) {
> > + op.cmd.dtr = true;
> > + op.addr.dtr = true;
> > + op.data.dtr = true;
> > + }
> > + }
> > +
> > return spi_nor_spimem_xfer_data(nor, &op);
> > }
> >
> > @@ -404,6 +436,30 @@ static int read_sr(struct spi_nor *nor)
> > SPI_MEM_OP_NO_DUMMY,
> > SPI_MEM_OP_DATA_IN(1, nor->bouncebuf, 1));
> >
>
> This is not based on the latest tree.
>
> > + if (spi_nor_protocol_is_8_8_8(nor->read_proto)) {
> > + op.cmd.buswidth = 8;
> > + op.addr.buswidth = 8;
> > + op.dummy.buswidth = 8;
> > + op.data.buswidth = 8;
> > + op.cmd.nbytes = 2;
> > + op.addr.nbytes = 4;
> > + op.dummy.nbytes = 4;
> > + op.addr.val = 0;
>
> This is not scalable... There will be bunch of if...else ladders when we
> want to support other X-X-X modes... Can't these be derived from
> nor->reg_proto? And then borrow the logic from
spi_nor_spimem_read_data()?
>
Got it !
>
> Could you have a look at Boris's initial submission to add 8-8-8 mode at
> https://patchwork.kernel.org/cover/10638055/ ?
> You could use that series as the base for your changes/additions.
Got it.
My idea is to support 8D-8D-8D mode with a minimum patches because
there is no define for 1D-1D-1D, 2D-2D-2D and 4D-4D-4D mode in JEDEC
if I am right.
>
> I am fine if you want to call 2nd byte of opcode as ext_opcode
>
> Regards
> Vignesh
>
thanks for your time & comments.
Mason
CONFIDENTIALITY NOTE:
This e-mail and any attachments may contain confidential information
and/or personal data, which is protected by applicable laws. Please be
reminded that duplication, disclosure, distribution, or use of this e-mail
(and/or its attachments) or any part thereof is prohibited. If you receive
this e-mail in error, please notify us immediately and delete this mail as
well as its attachment(s) from your system. In addition, please be
informed that collection, processing, and/or use of personal data is
prohibited unless expressly permitted by personal data protection laws.
Thank you for your attention and cooperation.
Macronix International Co., Ltd.
=====================================================================
============================================================================
CONFIDENTIALITY NOTE:
This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation.
Macronix International Co., Ltd.
=====================================================================
Hi,
On 09/12/19 1:26 pm, [email protected] wrote:
>
> Hi Vignesh,
>
>>
>> On 15/11/19 2:28 pm, Mason Yang wrote:
>>> According to JESD216C (JEDEC Basic Flash Parameter Table 18th DWORD)
>>> Octal DTR(8D-8D-8D) command and command extension (00b: same, 01b:
> inverse)
>>> to add extension command mode in spi_nor struct and to add write_cr2
>>> (Write CFG Reg 2) for 8-8-8/8D-8D-8D mode sequences enable.
>>>
>>
>> But I don't see any code setting "nor->ext_cmd_mode" based on BFPT?
>>
>> Any new feature that we add to spi-nor should make use of autodiscovery
>> feature made possible by SFDP tables. Could you modify the patch to
>> discover capabilities supported by flash and opcodes to be used from
>> SFDP table?
>
> Got it but our device will return a empty SFDP table.
>
If flash you tested on does not support JEDEC 216C then don't mention
about it. Above commit message gives an impression that flash in JEDEC
216C compliant.
>>
>>
>>> Define the relevant macrons and enum to add such modes and make sure
>>> op->xxx.dtr fields, command nbytes and extension command are properly
>>> filled and unmask DTR and X-X-X modes in
> spi_nor_spimem_adjust_hwcaps()
>>> so that DTR and X-X-X support detection is done through
>>> spi_mem_supports_op().
>>>
[...]
>>> @@ -404,6 +436,30 @@ static int read_sr(struct spi_nor *nor)
>>> SPI_MEM_OP_NO_DUMMY,
>>> SPI_MEM_OP_DATA_IN(1, nor->bouncebuf, 1));
>>>
>>
>> This is not based on the latest tree.
>>
>>> + if (spi_nor_protocol_is_8_8_8(nor->read_proto)) {
>>> + op.cmd.buswidth = 8;
>>> + op.addr.buswidth = 8;
>>> + op.dummy.buswidth = 8;
>>> + op.data.buswidth = 8;
>>> + op.cmd.nbytes = 2;
>>> + op.addr.nbytes = 4;
>>> + op.dummy.nbytes = 4;
>>> + op.addr.val = 0;
>>
>> This is not scalable... There will be bunch of if...else ladders when we
>> want to support other X-X-X modes... Can't these be derived from
>> nor->reg_proto? And then borrow the logic from
> spi_nor_spimem_read_data()?
>>
>
> Got it !
>
>>
>> Could you have a look at Boris's initial submission to add 8-8-8 mode at
>> https://patchwork.kernel.org/cover/10638055/ ?
>> You could use that series as the base for your changes/additions.
>
> Got it.
> My idea is to support 8D-8D-8D mode with a minimum patches because
> there is no define for 1D-1D-1D, 2D-2D-2D and 4D-4D-4D mode in JEDEC
> if I am right.
>
JESD251-A1 does talk about 4S-4D-4D right? Also none of the JEDEC
standards prohibit flash vendors from supporting other X-X-X modes.
I think you haven't thought about bigger picture here. Flash devices
that support other mode exist today and we would need the framework to
be built such that these modes can be added in future.
I suggest you start with Boris's series [1] as base and port it to
latest kernel. Isn't that series alone enough to support Macronix Octal
flashes at least?
If required, you could also always include additional patches adding new
features.
[1] https://patchwork.kernel.org/cover/10638055/
--
Regards
Vignesh
Hi Vignesh,
> >> On 15/11/19 2:28 pm, Mason Yang wrote:
> >>> According to JESD216C (JEDEC Basic Flash Parameter Table 18th DWORD)
> >>> Octal DTR(8D-8D-8D) command and command extension (00b: same, 01b:
> > inverse)
> >>> to add extension command mode in spi_nor struct and to add write_cr2
> >>> (Write CFG Reg 2) for 8-8-8/8D-8D-8D mode sequences enable.
> >>>
> >>
> >> But I don't see any code setting "nor->ext_cmd_mode" based on BFPT?
> >>
> >> Any new feature that we add to spi-nor should make use of
autodiscovery
> >> feature made possible by SFDP tables. Could you modify the patch to
> >> discover capabilities supported by flash and opcodes to be used from
> >> SFDP table?
> >
> > Got it but our device will return a empty SFDP table.
> >
>
> If flash you tested on does not support JEDEC 216C then don't mention
> about it. Above commit message gives an impression that flash in JEDEC
> 216C compliant.
>
okay, got it.
> >>
> >>
> >>> Define the relevant macrons and enum to add such modes and make sure
> >>> op->xxx.dtr fields, command nbytes and extension command are
properly
> >>> filled and unmask DTR and X-X-X modes in
> > spi_nor_spimem_adjust_hwcaps()
> >>> so that DTR and X-X-X support detection is done through
> >>> spi_mem_supports_op().
> >>>
> [...]
> >>> @@ -404,6 +436,30 @@ static int read_sr(struct spi_nor *nor)
> >>> SPI_MEM_OP_NO_DUMMY,
> >>> SPI_MEM_OP_DATA_IN(1, nor->bouncebuf, 1));
> >>>
> >>
> >> This is not based on the latest tree.
> >>
> >>> + if (spi_nor_protocol_is_8_8_8(nor->read_proto)) {
> >>> + op.cmd.buswidth = 8;
> >>> + op.addr.buswidth = 8;
> >>> + op.dummy.buswidth = 8;
> >>> + op.data.buswidth = 8;
> >>> + op.cmd.nbytes = 2;
> >>> + op.addr.nbytes = 4;
> >>> + op.dummy.nbytes = 4;
> >>> + op.addr.val = 0;
> >>
> >> This is not scalable... There will be bunch of if...else ladders when
we
> >> want to support other X-X-X modes... Can't these be derived from
> >> nor->reg_proto? And then borrow the logic from
> > spi_nor_spimem_read_data()?
> >>
> >
> > Got it !
> >
> >>
> >> Could you have a look at Boris's initial submission to add 8-8-8 mode
at
> >> https://patchwork.kernel.org/cover/10638055/ ?
> >> You could use that series as the base for your changes/additions.
> >
> > Got it.
> > My idea is to support 8D-8D-8D mode with a minimum patches because
> > there is no define for 1D-1D-1D, 2D-2D-2D and 4D-4D-4D mode in JEDEC
> > if I am right.
> >
>
> JESD251-A1 does talk about 4S-4D-4D right? Also none of the JEDEC
> standards prohibit flash vendors from supporting other X-X-X modes.
>
> I think you haven't thought about bigger picture here. Flash devices
> that support other mode exist today and we would need the framework to
> be built such that these modes can be added in future.
>
> I suggest you start with Boris's series [1] as base and port it to
> latest kernel. Isn't that series alone enough to support Macronix Octal
> flashes at least?
> If required, you could also always include additional patches adding new
> features.
okay.
>
> [1] https://patchwork.kernel.org/cover/10638055/
>
> --
> Regards
> Vignesh
thanks for your time & comments.
Mason
CONFIDENTIALITY NOTE:
This e-mail and any attachments may contain confidential information
and/or personal data, which is protected by applicable laws. Please be
reminded that duplication, disclosure, distribution, or use of this e-mail
(and/or its attachments) or any part thereof is prohibited. If you receive
this e-mail in error, please notify us immediately and delete this mail as
well as its attachment(s) from your system. In addition, please be
informed that collection, processing, and/or use of personal data is
prohibited unless expressly permitted by personal data protection laws.
Thank you for your attention and cooperation.
Macronix International Co., Ltd.
=====================================================================
============================================================================
CONFIDENTIALITY NOTE:
This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation.
Macronix International Co., Ltd.
=====================================================================
Hi, Mason,
From the discussion you had with Vignesh, I understand that a v2 will follow. A
nit below.
On 11/15/19 10:58 AM, Mason Yang wrote:
> Hello,
>
> This is repost of patchset from Boris Brezillon's
> [RFC,00/18] mtd: spi-nor: Proposal for 8-8-8 mode support [1].
>
[cut]
> Mason Yang (4):
Did you intentionally overwrite Boris's authorship? If yes, would you please
describe what changed from Boris's patch set?
Cheers,
ta
Hi Tudor,
>
> Re: [PATCH 0/4] mtd: spi-nor: Add support for Octal 8D-8D-8D mode
>
> Hi, Mason,
>
> From the discussion you had with Vignesh, I understand that a v2 will
follow. A
> nit below.
>
> On 11/15/19 10:58 AM, Mason Yang wrote:
> > Hello,
> >
> > This is repost of patchset from Boris Brezillon's
> > [RFC,00/18] mtd: spi-nor: Proposal for 8-8-8 mode support [1].
> >
>
> [cut]
>
> > Mason Yang (4):
>
> Did you intentionally overwrite Boris's authorship? If yes, would you
please
> describe what changed from Boris's patch set?
okay, sure.
I will describe it in v2 patch set.
>
> Cheers,
> ta
thanks for your time & comments.
Mason
CONFIDENTIALITY NOTE:
This e-mail and any attachments may contain confidential information
and/or personal data, which is protected by applicable laws. Please be
reminded that duplication, disclosure, distribution, or use of this e-mail
(and/or its attachments) or any part thereof is prohibited. If you receive
this e-mail in error, please notify us immediately and delete this mail as
well as its attachment(s) from your system. In addition, please be
informed that collection, processing, and/or use of personal data is
prohibited unless expressly permitted by personal data protection laws.
Thank you for your attention and cooperation.
Macronix International Co., Ltd.
=====================================================================
============================================================================
CONFIDENTIALITY NOTE:
This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation.
Macronix International Co., Ltd.
=====================================================================