* [PATCH v3 0/5] Add octal DTR support for Macronix flash
@ 2023-08-04 9:54 Jaime Liao
2023-08-04 9:54 ` [PATCH v3 1/5] mtd: spi-nor: add Octal " Jaime Liao
` (5 more replies)
0 siblings, 6 replies; 21+ messages in thread
From: Jaime Liao @ 2023-08-04 9:54 UTC (permalink / raw)
To: linux-mtd, tudor.ambarus, pratyush, michael, miquel.raynal
Cc: leoyu, JaimeLiao
From: JaimeLiao <jaimeliao@mxic.com.tw>
Add method for Macronix Octal DTR Eable/Disable.
Hook manufacture by comparing ID 1st byte.
Merge Tudor's patch "Allow specifying the byte order in DTR mode"
https://patchwork.ozlabs.org/project/linux-mtd/list/?series=290014&state=*
v3:
Add patch for hook manufacturer by comparing ID 1st byte.
Add patches for specifying the byte order in DTR mode by merging
Tudor's patch.
v2:
Following exsting rules to re-create Macronix specify Octal DTR method.
change signature to jaimeliao@mxic.com.tw
Clear sector size information in flash INFO.
JaimeLiao (5):
mtd: spi-nor: add Octal DTR support for Macronix flash
mtd: spi-nor: core: Hook manufacture by checking first byte ID
spi: spi-mem: Allow specifying the byte order in DTR mode
mtd: spi-nor: core: Allow specifying the byte order in DTR mode
mtd: spi-nor: sfdp: Get the 8D-8D-8D byte order from BFPT
drivers/mtd/spi-nor/core.c | 19 +++++--
drivers/mtd/spi-nor/core.h | 1 +
drivers/mtd/spi-nor/macronix.c | 99 ++++++++++++++++++++++++++++++++++
drivers/mtd/spi-nor/sfdp.c | 4 ++
drivers/mtd/spi-nor/sfdp.h | 1 +
drivers/spi/spi-mem.c | 3 ++
include/linux/spi/spi-mem.h | 6 +++
7 files changed, 129 insertions(+), 4 deletions(-)
--
2.25.1
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v3 1/5] mtd: spi-nor: add Octal DTR support for Macronix flash
2023-08-04 9:54 [PATCH v3 0/5] Add octal DTR support for Macronix flash Jaime Liao
@ 2023-08-04 9:54 ` Jaime Liao
2023-08-07 6:44 ` Michael Walle
2023-08-04 9:54 ` [PATCH v3 2/5] mtd: spi-nor: core: Hook manufacture by checking first byte ID Jaime Liao
` (4 subsequent siblings)
5 siblings, 1 reply; 21+ messages in thread
From: Jaime Liao @ 2023-08-04 9:54 UTC (permalink / raw)
To: linux-mtd, tudor.ambarus, pratyush, michael, miquel.raynal
Cc: leoyu, JaimeLiao
From: JaimeLiao <jaimeliao@mxic.com.tw>
Create Macronix specify method for enable Octal DTR mode and
set 20 dummy cycles to allow running at the maximum supported
frequency for Macronix Octal flash.
Use number of dummy cycles which is parse by SFDP then convert
it to bit pattern and set in CR2 register.
Set CR2 register for enable octal dtr mode.
Signed-off-by: JaimeLiao <jaimeliao@mxic.com.tw>
Co-developed-by: Tudor Ambarus <tudor.ambarus@linaro.org>
---
drivers/mtd/spi-nor/macronix.c | 99 ++++++++++++++++++++++++++++++++++
1 file changed, 99 insertions(+)
diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
index 04888258e891..679266cfbe8c 100644
--- a/drivers/mtd/spi-nor/macronix.c
+++ b/drivers/mtd/spi-nor/macronix.c
@@ -8,6 +8,24 @@
#include "core.h"
+#define SPINOR_OP_MXIC_RD_ANY_REG 0x71 /* Read volatile configuration register 2 */
+#define SPINOR_OP_MXIC_WR_ANY_REG 0x72 /* Write volatile configuration register 2 */
+#define SPINOR_REG_MXIC_CR2_MODE 0x00000000 /* CR2 address for setting octal DTR mode */
+#define SPINOR_REG_MXIC_CR2_DC 0x00000300 /* CR2 address for setting dummy cycles */
+#define SPINOR_REG_MXIC_OPI_DTR_EN 0x2 /* Enable Octal DTR */
+#define SPINOR_REG_MXIC_SPI_EN 0x0 /* Enable SPI */
+#define SPINOR_REG_MXIC_ADDR_BYTES 4 /* Fixed R/W volatile address bytes to 4 */
+/* Convert dummy cycles to bit pattern */
+#define SPINOR_REG_MXIC_DC(p) \
+ ((20 - p)/2)
+
+/* Macronix SPI NOR flash operations. */
+#define MXIC_NOR_WR_ANY_REG_OP(naddr, addr, ndata, buf) \
+ SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_MXIC_WR_ANY_REG, 0), \
+ SPI_MEM_OP_ADDR(naddr, addr, 0), \
+ SPI_MEM_OP_NO_DUMMY, \
+ SPI_MEM_OP_DATA_OUT(ndata, buf, 0))
+
static int
mx25l25635_post_bfpt_fixups(struct spi_nor *nor,
const struct sfdp_parameter_header *bfpt_header,
@@ -105,6 +123,86 @@ static const struct flash_info macronix_nor_parts[] = {
FIXUP_FLAGS(SPI_NOR_4B_OPCODES) },
};
+static int macronix_nor_octal_dtr_en(struct spi_nor *nor)
+{
+ struct spi_mem_op op;
+ u8 *buf = nor->bouncebuf, i;
+ int ret;
+
+ /* Use dummy cycles which is parse by SFDP and convert to bit pattern. */
+ buf[0] = SPINOR_REG_MXIC_DC(nor->params->reads[SNOR_CMD_READ_8_8_8_DTR].num_wait_states);
+ op = (struct spi_mem_op)
+ MXIC_NOR_WR_ANY_REG_OP(SPINOR_REG_MXIC_ADDR_BYTES,
+ SPINOR_REG_MXIC_CR2_DC, 1, buf);
+
+ ret = spi_nor_write_any_volatile_reg(nor, &op, nor->reg_proto);
+ if (ret)
+ return ret;
+
+ /* Set the octal and DTR enable bits. */
+ buf[0] = SPINOR_REG_MXIC_OPI_DTR_EN;
+ op = (struct spi_mem_op)
+ MXIC_NOR_WR_ANY_REG_OP(SPINOR_REG_MXIC_ADDR_BYTES,
+ SPINOR_REG_MXIC_CR2_MODE, 1, buf);
+ ret = spi_nor_write_any_volatile_reg(nor, &op, nor->reg_proto);
+ if (ret)
+ return ret;
+
+ /* Read flash ID to make sure the switch was successful. */
+ ret = spi_nor_read_id(nor, 4, 4, buf, SNOR_PROTO_8_8_8_DTR);
+ if (ret) {
+ dev_dbg(nor->dev, "error %d reading JEDEC ID after enabling 8D-8D-8D mode\n", ret);
+ return ret;
+ }
+
+ /* Macronix SPI-NOR flash 8D-8D-8D read ID would get 6 bytes data A-A-B-B-C-C */
+ for (i = 0; i < nor->info->id_len; i++)
+ if (buf[i * 2] != nor->info->id[i])
+ return -EINVAL;
+
+ return 0;
+}
+
+static int macronix_nor_octal_dtr_dis(struct spi_nor *nor)
+{
+ struct spi_mem_op op;
+ u8 *buf = nor->bouncebuf;
+ int ret;
+
+ /*
+ * The register is 1-byte wide, but 1-byte transactions are not
+ * allowed in 8D-8D-8D mode. Since there is no register at the
+ * next location, just initialize the value to 0 and let the
+ * transaction go on.
+ */
+ buf[0] = SPINOR_REG_MXIC_SPI_EN;
+ buf[1] = 0x0;
+ op = (struct spi_mem_op)
+ MXIC_NOR_WR_ANY_REG_OP(SPINOR_REG_MXIC_ADDR_BYTES,
+ SPINOR_REG_MXIC_CR2_MODE, 2, buf);
+ ret = spi_nor_write_any_volatile_reg(nor, &op, SNOR_PROTO_8_8_8_DTR);
+ if (ret)
+ return ret;
+
+ /* Read flash ID to make sure the switch was successful. */
+ ret = spi_nor_read_id(nor, 0, 0, buf, SNOR_PROTO_1_1_1);
+ if (ret) {
+ dev_dbg(nor->dev, "error %d reading JEDEC ID after disabling 8D-8D-8D mode\n", ret);
+ return ret;
+ }
+
+ if (memcmp(buf, nor->info->id, nor->info->id_len))
+ return -EINVAL;
+
+ return 0;
+}
+
+static int macronix_nor_octal_dtr_enable(struct spi_nor *nor, bool enable)
+{
+ return enable ? macronix_nor_octal_dtr_en(nor) :
+ macronix_nor_octal_dtr_dis(nor);
+}
+
static void macronix_nor_default_init(struct spi_nor *nor)
{
nor->params->quad_enable = spi_nor_sr1_bit6_quad_enable;
@@ -114,6 +212,7 @@ static void macronix_nor_late_init(struct spi_nor *nor)
{
if (!nor->params->set_4byte_addr_mode)
nor->params->set_4byte_addr_mode = spi_nor_set_4byte_addr_mode_en4b_ex4b;
+ nor->params->octal_dtr_enable = macronix_nor_octal_dtr_enable;
}
static const struct spi_nor_fixups macronix_nor_fixups = {
--
2.25.1
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v3 2/5] mtd: spi-nor: core: Hook manufacture by checking first byte ID
2023-08-04 9:54 [PATCH v3 0/5] Add octal DTR support for Macronix flash Jaime Liao
2023-08-04 9:54 ` [PATCH v3 1/5] mtd: spi-nor: add Octal " Jaime Liao
@ 2023-08-04 9:54 ` Jaime Liao
2023-08-07 6:37 ` Michael Walle
2023-08-04 9:54 ` [PATCH v3 3/5] spi: spi-mem: Allow specifying the byte order in DTR mode Jaime Liao
` (3 subsequent siblings)
5 siblings, 1 reply; 21+ messages in thread
From: Jaime Liao @ 2023-08-04 9:54 UTC (permalink / raw)
To: linux-mtd, tudor.ambarus, pratyush, michael, miquel.raynal
Cc: leoyu, JaimeLiao
From: JaimeLiao <jaimeliao@mxic.com.tw>
Patch ID for flash support is a thing that we keep striving to do.
Follow the optimization of software architecture, parse SFDP is
the mainstream in SPI-NOR subsystem.
Label "spi-nor-generic" to the flash which didn't include in ID table
but support SFDP, is a good way for flash support.
Building upon this fundation, achieve hooking the manufacturer using the
1st ID byte.
Consequently, extend support even for parts not descibed in SFDP.
Signed-off-by: JaimeLiao <jaimeliao@mxic.com.tw>
---
drivers/mtd/spi-nor/core.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 5f29fac8669a..eb0a09c06bf4 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -2036,10 +2036,13 @@ static const struct flash_info *spi_nor_match_id(struct spi_nor *nor,
for (i = 0; i < ARRAY_SIZE(manufacturers); i++) {
for (j = 0; j < manufacturers[i]->nparts; j++) {
part = &manufacturers[i]->parts[j];
- if (part->id_len &&
- !memcmp(part->id, id, part->id_len)) {
- nor->manufacturer = manufacturers[i];
- return part;
+ if (part->id_len) {
+ /* Hook manufacturers when first byte (maf ID) is same as other IDs on ID table */
+ if (!nor->manufacturer && !memcmp(part->id, id, 1))
+ nor->manufacturer = manufacturers[i];
+
+ if (!memcmp(part->id, id, part->id_len))
+ return part;
}
}
}
--
2.25.1
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v3 3/5] spi: spi-mem: Allow specifying the byte order in DTR mode
2023-08-04 9:54 [PATCH v3 0/5] Add octal DTR support for Macronix flash Jaime Liao
2023-08-04 9:54 ` [PATCH v3 1/5] mtd: spi-nor: add Octal " Jaime Liao
2023-08-04 9:54 ` [PATCH v3 2/5] mtd: spi-nor: core: Hook manufacture by checking first byte ID Jaime Liao
@ 2023-08-04 9:54 ` Jaime Liao
2023-08-07 6:40 ` Michael Walle
2023-08-04 9:54 ` [PATCH v3 4/5] mtd: spi-nor: core: " Jaime Liao
` (2 subsequent siblings)
5 siblings, 1 reply; 21+ messages in thread
From: Jaime Liao @ 2023-08-04 9:54 UTC (permalink / raw)
To: linux-mtd, tudor.ambarus, pratyush, michael, miquel.raynal
Cc: leoyu, JaimeLiao
From: JaimeLiao <jaimeliao@mxic.com.tw>
There are NOR flashes (Macronix) that swap the bytes on a 16-bit
boundary when configured in Octal DTR mode. The byte order of
16-bit words is swapped when read or written in Octal Double
Transfer Rate (DTR) mode compared to Single Transfer Rate (STR)
modes. If one writes D0 D1 D2 D3 bytes using 1-1-1 mode, and uses
8D-8D-8D SPI mode for reading, it will read back D1 D0 D3 D2.
Swapping the bytes may introduce some endianness problems. It can
affect the boot sequence if the entire boot sequence is not handled
in either 8D-8D-8D mode or 1-1-1 mode. So we must swap the bytes
back to have the same byte order as in STR modes. Fortunately there
are controllers that could swap the bytes back at runtime,
addressing the flash's endiannesses requirements. Provide a way for
the upper layers to specify the byte order in Octal DTR mode.
Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
---
drivers/spi/spi-mem.c | 3 +++
include/linux/spi/spi-mem.h | 6 ++++++
2 files changed, 9 insertions(+)
diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c
index edd7430d4c05..fd6b1c69ab9b 100644
--- a/drivers/spi/spi-mem.c
+++ b/drivers/spi/spi-mem.c
@@ -171,6 +171,9 @@ bool spi_mem_default_supports_op(struct spi_mem *mem,
if (op_is_dtr) {
if (!spi_mem_controller_is_capable(ctlr, dtr))
return false;
+ if (op->data.dtr_swab16 &&
+ !(spi_mem_controller_is_capable(ctlr, dtr_swab16)))
+ return false;
if (op->cmd.nbytes != 2)
return false;
diff --git a/include/linux/spi/spi-mem.h b/include/linux/spi/spi-mem.h
index 8e984d75f5b6..9da6d53a29a3 100644
--- a/include/linux/spi/spi-mem.h
+++ b/include/linux/spi/spi-mem.h
@@ -89,6 +89,8 @@ enum spi_mem_data_dir {
* @dummy.dtr: whether the dummy bytes should be sent in DTR mode or not
* @data.buswidth: number of IO lanes used to send/receive the data
* @data.dtr: whether the data should be sent in DTR mode or not
+ * @data.dtr_swab16: whether the byte order of 16-bit words is swapped when read
+ * or written in Octal DTR mode compare to STR mode
* @data.ecc: whether error correction is required or not
* @data.dir: direction of the transfer
* @data.nbytes: number of data bytes to send/receive. Can be zero if the
@@ -120,6 +122,7 @@ struct spi_mem_op {
struct {
u8 buswidth;
u8 dtr : 1;
+ u8 dtr_swab16 : 1;
u8 ecc : 1;
enum spi_mem_data_dir dir;
unsigned int nbytes;
@@ -290,10 +293,13 @@ struct spi_controller_mem_ops {
/**
* struct spi_controller_mem_caps - SPI memory controller capabilities
* @dtr: Supports DTR operations
+ * @dtr_swab16: Supports swapping bytes on a 16 bit boundary when configured in
+ Octal DTR
* @ecc: Supports operations with error correction
*/
struct spi_controller_mem_caps {
bool dtr;
+ bool dtr_swab16;
bool ecc;
};
--
2.25.1
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v3 4/5] mtd: spi-nor: core: Allow specifying the byte order in DTR mode
2023-08-04 9:54 [PATCH v3 0/5] Add octal DTR support for Macronix flash Jaime Liao
` (2 preceding siblings ...)
2023-08-04 9:54 ` [PATCH v3 3/5] spi: spi-mem: Allow specifying the byte order in DTR mode Jaime Liao
@ 2023-08-04 9:54 ` Jaime Liao
2023-08-04 9:54 ` [PATCH v3 5/5] mtd: spi-nor: sfdp: Get the 8D-8D-8D byte order from BFPT Jaime Liao
2023-08-07 6:42 ` [PATCH v3 0/5] Add octal DTR support for Macronix flash Michael Walle
5 siblings, 0 replies; 21+ messages in thread
From: Jaime Liao @ 2023-08-04 9:54 UTC (permalink / raw)
To: linux-mtd, tudor.ambarus, pratyush, michael, miquel.raynal
Cc: leoyu, JaimeLiao
From: JaimeLiao <jaimeliao@mxic.com.tw>
Macronix swaps bytes on a 16-bit boundary when configured in Octal DTR.
The byte order of 16-bit words is swapped when read or written in 8D-8D-8D
mode compared to STR modes. Allow operations to specify the byte order in
DTR mode, so that controllers can swap the bytes back at run-time to
address the flash's endianness requirements, if they are capable. If the
controllers are not capable of swapping the bytes, the protocol is
downgrade via spi_nor_spimem_adjust_hwcaps(). When available, the swapping
of the bytes is always done regardless if it's a data or register access,
so that we comply with the JESD216 requirements: "Byte order of 16-bit
words is swapped when read in 8D-8D-8D mode compared to 1-1-1".
Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
---
drivers/mtd/spi-nor/core.c | 8 ++++++++
drivers/mtd/spi-nor/core.h | 1 +
2 files changed, 9 insertions(+)
diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index eb0a09c06bf4..447cf1bc7fa4 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -70,6 +70,13 @@ static u8 spi_nor_get_cmd_ext(const struct spi_nor *nor,
}
}
+static inline bool spi_nor_is_octal_dtr_swab16(const struct spi_nor *nor,
+ enum spi_nor_protocol proto)
+{
+ return (proto == SNOR_PROTO_8_8_8_DTR) &&
+ (nor->flags & SNOR_F_DTR_SWAB16);
+}
+
/**
* spi_nor_spimem_setup_op() - Set up common properties of a spi-mem op.
* @nor: pointer to a 'struct spi_nor'
@@ -105,6 +112,7 @@ void spi_nor_spimem_setup_op(const struct spi_nor *nor,
op->addr.dtr = true;
op->dummy.dtr = true;
op->data.dtr = true;
+ op->data.dtr_swab16 = spi_nor_is_octal_dtr_swab16(nor, proto);
/* 2 bytes per clock cycle in DTR mode. */
op->dummy.nbytes *= 2;
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index 4fb5ff09c63a..08925e222416 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -132,6 +132,7 @@ enum spi_nor_option_flags {
SNOR_F_SWP_IS_VOLATILE = BIT(13),
SNOR_F_RWW = BIT(14),
SNOR_F_ECC = BIT(15),
+ SNOR_F_DTR_SWAB16 = BIT(16),
};
struct spi_nor_read_command {
--
2.25.1
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v3 5/5] mtd: spi-nor: sfdp: Get the 8D-8D-8D byte order from BFPT
2023-08-04 9:54 [PATCH v3 0/5] Add octal DTR support for Macronix flash Jaime Liao
` (3 preceding siblings ...)
2023-08-04 9:54 ` [PATCH v3 4/5] mtd: spi-nor: core: " Jaime Liao
@ 2023-08-04 9:54 ` Jaime Liao
2023-08-07 6:42 ` [PATCH v3 0/5] Add octal DTR support for Macronix flash Michael Walle
5 siblings, 0 replies; 21+ messages in thread
From: Jaime Liao @ 2023-08-04 9:54 UTC (permalink / raw)
To: linux-mtd, tudor.ambarus, pratyush, michael, miquel.raynal
Cc: leoyu, JaimeLiao
From: JaimeLiao <jaimeliao@mxic.com.tw>
Parse BFPT in order to retrieve the byte order in 8D-8D-8D mode.
Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
---
drivers/mtd/spi-nor/sfdp.c | 4 ++++
drivers/mtd/spi-nor/sfdp.h | 1 +
2 files changed, 5 insertions(+)
diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
index b3b11dfed789..2241207556bf 100644
--- a/drivers/mtd/spi-nor/sfdp.c
+++ b/drivers/mtd/spi-nor/sfdp.c
@@ -650,6 +650,10 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
return -EOPNOTSUPP;
}
+ /* Byte order in 8D-8D-8D mode */
+ if (bfpt.dwords[SFDP_DWORD(18)] & BFPT_DWORD18_BYTE_ORDER_SWAPPED)
+ nor->flags |= SNOR_F_DTR_SWAB16;
+
return spi_nor_post_bfpt_fixups(nor, bfpt_header, &bfpt);
}
diff --git a/drivers/mtd/spi-nor/sfdp.h b/drivers/mtd/spi-nor/sfdp.h
index 6eb99e1cdd61..eba760941d43 100644
--- a/drivers/mtd/spi-nor/sfdp.h
+++ b/drivers/mtd/spi-nor/sfdp.h
@@ -123,6 +123,7 @@ struct sfdp_bfpt {
#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 */
+#define BFPT_DWORD18_BYTE_ORDER_SWAPPED BIT(31) /* Byte order of 16-bit words in 8D-8D-8D mode */
struct sfdp_parameter_header {
u8 id_lsb;
--
2.25.1
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v3 2/5] mtd: spi-nor: core: Hook manufacture by checking first byte ID
2023-08-04 9:54 ` [PATCH v3 2/5] mtd: spi-nor: core: Hook manufacture by checking first byte ID Jaime Liao
@ 2023-08-07 6:37 ` Michael Walle
2023-08-09 1:04 ` liao jaime
0 siblings, 1 reply; 21+ messages in thread
From: Michael Walle @ 2023-08-07 6:37 UTC (permalink / raw)
To: Jaime Liao
Cc: linux-mtd, tudor.ambarus, pratyush, miquel.raynal, leoyu,
JaimeLiao
Am 2023-08-04 11:54, schrieb Jaime Liao:
> From: JaimeLiao <jaimeliao@mxic.com.tw>
>
> Patch ID for flash support is a thing that we keep striving to do.
> Follow the optimization of software architecture, parse SFDP is
> the mainstream in SPI-NOR subsystem.
> Label "spi-nor-generic" to the flash which didn't include in ID table
> but support SFDP, is a good way for flash support.
> Building upon this fundation, achieve hooking the manufacturer using
> the
> 1st ID byte.
This won't work beacuse the manufacturer id is not always
one byte long, think of continuation codes. In fact, as the
flash_info table is of now, we cannot even rely on the
continuation codes, but we have to always check for the
complete id_len, i.e. there is at least one hack where
the id is reversed and the manufacturer is the last byte,
iirc. some oddball cypress mram chip.
If you want to get the correct manufacturer for spi-nor-generic,
you should extract it from the SFDP tables. It seems that the
BFPT don't include a manufacturer id, but if there are proprietary
tables, you *might* use that id. I say might, because it only works
with one byte manufacturer ids, no continuation codes... *sigh*
-michael
> Consequently, extend support even for parts not descibed in SFDP.
>
> Signed-off-by: JaimeLiao <jaimeliao@mxic.com.tw>
> ---
> drivers/mtd/spi-nor/core.c | 11 +++++++----
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index 5f29fac8669a..eb0a09c06bf4 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -2036,10 +2036,13 @@ static const struct flash_info
> *spi_nor_match_id(struct spi_nor *nor,
> for (i = 0; i < ARRAY_SIZE(manufacturers); i++) {
> for (j = 0; j < manufacturers[i]->nparts; j++) {
> part = &manufacturers[i]->parts[j];
> - if (part->id_len &&
> - !memcmp(part->id, id, part->id_len)) {
> - nor->manufacturer = manufacturers[i];
> - return part;
> + if (part->id_len) {
> + /* Hook manufacturers when first byte (maf ID) is same as other
> IDs on ID table */
> + if (!nor->manufacturer && !memcmp(part->id, id, 1))
> + nor->manufacturer = manufacturers[i];
> +
> + if (!memcmp(part->id, id, part->id_len))
> + return part;
> }
> }
> }
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 3/5] spi: spi-mem: Allow specifying the byte order in DTR mode
2023-08-04 9:54 ` [PATCH v3 3/5] spi: spi-mem: Allow specifying the byte order in DTR mode Jaime Liao
@ 2023-08-07 6:40 ` Michael Walle
2023-08-09 1:36 ` liao jaime
0 siblings, 1 reply; 21+ messages in thread
From: Michael Walle @ 2023-08-07 6:40 UTC (permalink / raw)
To: Jaime Liao
Cc: linux-mtd, tudor.ambarus, pratyush, miquel.raynal, leoyu,
JaimeLiao
Am 2023-08-04 11:54, schrieb Jaime Liao:
> From: JaimeLiao <jaimeliao@mxic.com.tw>
This doesn't match..
>
> There are NOR flashes (Macronix) that swap the bytes on a 16-bit
> boundary when configured in Octal DTR mode. The byte order of
> 16-bit words is swapped when read or written in Octal Double
> Transfer Rate (DTR) mode compared to Single Transfer Rate (STR)
> modes. If one writes D0 D1 D2 D3 bytes using 1-1-1 mode, and uses
> 8D-8D-8D SPI mode for reading, it will read back D1 D0 D3 D2.
> Swapping the bytes may introduce some endianness problems. It can
> affect the boot sequence if the entire boot sequence is not handled
> in either 8D-8D-8D mode or 1-1-1 mode. So we must swap the bytes
> back to have the same byte order as in STR modes. Fortunately there
> are controllers that could swap the bytes back at runtime,
> addressing the flash's endiannesses requirements. Provide a way for
> the upper layers to specify the byte order in Octal DTR mode.
>
> Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
.. this. If you pick up a patch from another author, you should
keep the author of the patch.
-michael
> ---
> drivers/spi/spi-mem.c | 3 +++
> include/linux/spi/spi-mem.h | 6 ++++++
> 2 files changed, 9 insertions(+)
>
> diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c
> index edd7430d4c05..fd6b1c69ab9b 100644
> --- a/drivers/spi/spi-mem.c
> +++ b/drivers/spi/spi-mem.c
> @@ -171,6 +171,9 @@ bool spi_mem_default_supports_op(struct spi_mem
> *mem,
> if (op_is_dtr) {
> if (!spi_mem_controller_is_capable(ctlr, dtr))
> return false;
> + if (op->data.dtr_swab16 &&
> + !(spi_mem_controller_is_capable(ctlr, dtr_swab16)))
> + return false;
>
> if (op->cmd.nbytes != 2)
> return false;
> diff --git a/include/linux/spi/spi-mem.h b/include/linux/spi/spi-mem.h
> index 8e984d75f5b6..9da6d53a29a3 100644
> --- a/include/linux/spi/spi-mem.h
> +++ b/include/linux/spi/spi-mem.h
> @@ -89,6 +89,8 @@ enum spi_mem_data_dir {
> * @dummy.dtr: whether the dummy bytes should be sent in DTR mode or
> not
> * @data.buswidth: number of IO lanes used to send/receive the data
> * @data.dtr: whether the data should be sent in DTR mode or not
> + * @data.dtr_swab16: whether the byte order of 16-bit words is swapped
> when read
> + * or written in Octal DTR mode compare to STR mode
> * @data.ecc: whether error correction is required or not
> * @data.dir: direction of the transfer
> * @data.nbytes: number of data bytes to send/receive. Can be zero if
> the
> @@ -120,6 +122,7 @@ struct spi_mem_op {
> struct {
> u8 buswidth;
> u8 dtr : 1;
> + u8 dtr_swab16 : 1;
> u8 ecc : 1;
> enum spi_mem_data_dir dir;
> unsigned int nbytes;
> @@ -290,10 +293,13 @@ struct spi_controller_mem_ops {
> /**
> * struct spi_controller_mem_caps - SPI memory controller capabilities
> * @dtr: Supports DTR operations
> + * @dtr_swab16: Supports swapping bytes on a 16 bit boundary when
> configured in
> + Octal DTR
> * @ecc: Supports operations with error correction
> */
> struct spi_controller_mem_caps {
> bool dtr;
> + bool dtr_swab16;
> bool ecc;
> };
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 0/5] Add octal DTR support for Macronix flash
2023-08-04 9:54 [PATCH v3 0/5] Add octal DTR support for Macronix flash Jaime Liao
` (4 preceding siblings ...)
2023-08-04 9:54 ` [PATCH v3 5/5] mtd: spi-nor: sfdp: Get the 8D-8D-8D byte order from BFPT Jaime Liao
@ 2023-08-07 6:42 ` Michael Walle
5 siblings, 0 replies; 21+ messages in thread
From: Michael Walle @ 2023-08-07 6:42 UTC (permalink / raw)
To: Jaime Liao
Cc: linux-mtd, tudor.ambarus, pratyush, miquel.raynal, leoyu,
JaimeLiao
Hi,
> Add patches for specifying the byte order in DTR mode by merging
> Tudor's patch.
I'm not sure what this has to do with this patchset. It doesn't get
used at all, nor has this something to do with the flash responding
as STR for the read_id op.
-michael
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 1/5] mtd: spi-nor: add Octal DTR support for Macronix flash
2023-08-04 9:54 ` [PATCH v3 1/5] mtd: spi-nor: add Octal " Jaime Liao
@ 2023-08-07 6:44 ` Michael Walle
0 siblings, 0 replies; 21+ messages in thread
From: Michael Walle @ 2023-08-07 6:44 UTC (permalink / raw)
To: Jaime Liao
Cc: linux-mtd, tudor.ambarus, pratyush, miquel.raynal, leoyu,
JaimeLiao
Hi,
> + /* Macronix SPI-NOR flash 8D-8D-8D read ID would get 6 bytes data
> A-A-B-B-C-C */
> + for (i = 0; i < nor->info->id_len; i++)
> + if (buf[i * 2] != nor->info->id[i])
> + return -EINVAL;
Still wrong. See comments on previous version.
-michael
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 2/5] mtd: spi-nor: core: Hook manufacture by checking first byte ID
2023-08-07 6:37 ` Michael Walle
@ 2023-08-09 1:04 ` liao jaime
2023-08-10 7:27 ` Michael Walle
0 siblings, 1 reply; 21+ messages in thread
From: liao jaime @ 2023-08-09 1:04 UTC (permalink / raw)
To: Michael Walle
Cc: linux-mtd, tudor.ambarus, pratyush, miquel.raynal, leoyu,
JaimeLiao
Hi Michael
>
> Am 2023-08-04 11:54, schrieb Jaime Liao:
> > From: JaimeLiao <jaimeliao@mxic.com.tw>
> >
> > Patch ID for flash support is a thing that we keep striving to do.
> > Follow the optimization of software architecture, parse SFDP is
> > the mainstream in SPI-NOR subsystem.
> > Label "spi-nor-generic" to the flash which didn't include in ID table
> > but support SFDP, is a good way for flash support.
> > Building upon this fundation, achieve hooking the manufacturer using
> > the
> > 1st ID byte.
>
> This won't work beacuse the manufacturer id is not always
> one byte long, think of continuation codes. In fact, as the
> flash_info table is of now, we cannot even rely on the
> continuation codes, but we have to always check for the
> complete id_len, i.e. there is at least one hack where
> the id is reversed and the manufacturer is the last byte,
> iirc. some oddball cypress mram chip.
According JEDEC standard, 1st byte is manufacture ID.
I check id table, "cy15x104q" with multi manufacture ID in
later bytes by RDID command(9F).
Maybe some old flash didn't follow this but I think it isn't
a high percentage.
Most of all and new product are follow JEDEC.
>
> If you want to get the correct manufacturer for spi-nor-generic,
> you should extract it from the SFDP tables. It seems that the
> BFPT don't include a manufacturer id, but if there are proprietary
> tables, you *might* use that id. I say might, because it only works
> with one byte manufacturer ids, no continuation codes... *sigh*
Unfortunately, Macronix didn't include proprietary tables in SFDP for
octal flashes and as I understanding proprietary table is not a stardard.
So that content and address are not identical each vendor, it will need
a manufacturer fixups for reading.
>
> -michael
>
>
> > Consequently, extend support even for parts not descibed in SFDP.
> >
> > Signed-off-by: JaimeLiao <jaimeliao@mxic.com.tw>
> > ---
> > drivers/mtd/spi-nor/core.c | 11 +++++++----
> > 1 file changed, 7 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> > index 5f29fac8669a..eb0a09c06bf4 100644
> > --- a/drivers/mtd/spi-nor/core.c
> > +++ b/drivers/mtd/spi-nor/core.c
> > @@ -2036,10 +2036,13 @@ static const struct flash_info
> > *spi_nor_match_id(struct spi_nor *nor,
> > for (i = 0; i < ARRAY_SIZE(manufacturers); i++) {
> > for (j = 0; j < manufacturers[i]->nparts; j++) {
> > part = &manufacturers[i]->parts[j];
> > - if (part->id_len &&
> > - !memcmp(part->id, id, part->id_len)) {
> > - nor->manufacturer = manufacturers[i];
> > - return part;
> > + if (part->id_len) {
> > + /* Hook manufacturers when first byte (maf ID) is same as other
> > IDs on ID table */
> > + if (!nor->manufacturer && !memcmp(part->id, id, 1))
> > + nor->manufacturer = manufacturers[i];
> > +
> > + if (!memcmp(part->id, id, part->id_len))
> > + return part;
> > }
> > }
> > }
Thanks
Jaime
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 3/5] spi: spi-mem: Allow specifying the byte order in DTR mode
2023-08-07 6:40 ` Michael Walle
@ 2023-08-09 1:36 ` liao jaime
2023-08-10 7:31 ` Michael Walle
0 siblings, 1 reply; 21+ messages in thread
From: liao jaime @ 2023-08-09 1:36 UTC (permalink / raw)
To: Michael Walle
Cc: linux-mtd, tudor.ambarus, pratyush, miquel.raynal, leoyu,
JaimeLiao
Hi Michael
>
> Am 2023-08-04 11:54, schrieb Jaime Liao:
> > From: JaimeLiao <jaimeliao@mxic.com.tw>
>
> This doesn't match..
>
> >
> > There are NOR flashes (Macronix) that swap the bytes on a 16-bit
> > boundary when configured in Octal DTR mode. The byte order of
> > 16-bit words is swapped when read or written in Octal Double
> > Transfer Rate (DTR) mode compared to Single Transfer Rate (STR)
> > modes. If one writes D0 D1 D2 D3 bytes using 1-1-1 mode, and uses
> > 8D-8D-8D SPI mode for reading, it will read back D1 D0 D3 D2.
> > Swapping the bytes may introduce some endianness problems. It can
> > affect the boot sequence if the entire boot sequence is not handled
> > in either 8D-8D-8D mode or 1-1-1 mode. So we must swap the bytes
> > back to have the same byte order as in STR modes. Fortunately there
> > are controllers that could swap the bytes back at runtime,
> > addressing the flash's endiannesses requirements. Provide a way for
> > the upper layers to specify the byte order in Octal DTR mode.
> >
> > Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
>
> .. this. If you pick up a patch from another author, you should
> keep the author of the patch.
How could I do for this?
Because of Tudor's patch is on old Linux kernel version, it need
a small changes for suiting on v6.5-rc3.
>
> -michael
>
> > ---
> > drivers/spi/spi-mem.c | 3 +++
> > include/linux/spi/spi-mem.h | 6 ++++++
> > 2 files changed, 9 insertions(+)
> >
> > diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c
> > index edd7430d4c05..fd6b1c69ab9b 100644
> > --- a/drivers/spi/spi-mem.c
> > +++ b/drivers/spi/spi-mem.c
> > @@ -171,6 +171,9 @@ bool spi_mem_default_supports_op(struct spi_mem
> > *mem,
> > if (op_is_dtr) {
> > if (!spi_mem_controller_is_capable(ctlr, dtr))
> > return false;
> > + if (op->data.dtr_swab16 &&
> > + !(spi_mem_controller_is_capable(ctlr, dtr_swab16)))
> > + return false;
> >
> > if (op->cmd.nbytes != 2)
> > return false;
> > diff --git a/include/linux/spi/spi-mem.h b/include/linux/spi/spi-mem.h
> > index 8e984d75f5b6..9da6d53a29a3 100644
> > --- a/include/linux/spi/spi-mem.h
> > +++ b/include/linux/spi/spi-mem.h
> > @@ -89,6 +89,8 @@ enum spi_mem_data_dir {
> > * @dummy.dtr: whether the dummy bytes should be sent in DTR mode or
> > not
> > * @data.buswidth: number of IO lanes used to send/receive the data
> > * @data.dtr: whether the data should be sent in DTR mode or not
> > + * @data.dtr_swab16: whether the byte order of 16-bit words is swapped
> > when read
> > + * or written in Octal DTR mode compare to STR mode
> > * @data.ecc: whether error correction is required or not
> > * @data.dir: direction of the transfer
> > * @data.nbytes: number of data bytes to send/receive. Can be zero if
> > the
> > @@ -120,6 +122,7 @@ struct spi_mem_op {
> > struct {
> > u8 buswidth;
> > u8 dtr : 1;
> > + u8 dtr_swab16 : 1;
> > u8 ecc : 1;
> > enum spi_mem_data_dir dir;
> > unsigned int nbytes;
> > @@ -290,10 +293,13 @@ struct spi_controller_mem_ops {
> > /**
> > * struct spi_controller_mem_caps - SPI memory controller capabilities
> > * @dtr: Supports DTR operations
> > + * @dtr_swab16: Supports swapping bytes on a 16 bit boundary when
> > configured in
> > + Octal DTR
> > * @ecc: Supports operations with error correction
> > */
> > struct spi_controller_mem_caps {
> > bool dtr;
> > + bool dtr_swab16;
> > bool ecc;
> > };
Thanks your review
Jaime
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 2/5] mtd: spi-nor: core: Hook manufacture by checking first byte ID
2023-08-09 1:04 ` liao jaime
@ 2023-08-10 7:27 ` Michael Walle
2023-08-11 9:03 ` liao jaime
0 siblings, 1 reply; 21+ messages in thread
From: Michael Walle @ 2023-08-10 7:27 UTC (permalink / raw)
To: liao jaime
Cc: linux-mtd, tudor.ambarus, pratyush, miquel.raynal, leoyu,
JaimeLiao
Hi,
>> This won't work beacuse the manufacturer id is not always
>> one byte long, think of continuation codes. In fact, as the
>> flash_info table is of now, we cannot even rely on the
>> continuation codes, but we have to always check for the
>> complete id_len, i.e. there is at least one hack where
>> the id is reversed and the manufacturer is the last byte,
>> iirc. some oddball cypress mram chip.
> According JEDEC standard, 1st byte is manufacture ID.
> I check id table, "cy15x104q" with multi manufacture ID in
> later bytes by RDID command(9F).
Yes the currently supported cy15x104q is broken.. but nevertheless
it's there. Also some spansion flashes uses winbond manufacturer
ids.
> Maybe some old flash didn't follow this but I think it isn't
> a high percentage.
> Most of all and new product are follow JEDEC.
Have a look at JEP106 (mine is JEP106BC) and you'll see all the
manufacturer IDs defined by JEDEC and you'll see that except
for the first 127, all others are multibyte vendor IDs starting
with continuation codes.
And it seems that the command RDID 9F is not covered in any
JEDEC standard. At least I haven't found anything. If you know
where that command is defined, please let me know.
>> If you want to get the correct manufacturer for spi-nor-generic,
>> you should extract it from the SFDP tables. It seems that the
>> BFPT don't include a manufacturer id, but if there are proprietary
>> tables, you *might* use that id. I say might, because it only works
>> with one byte manufacturer ids, no continuation codes... *sigh*
>
> Unfortunately, Macronix didn't include proprietary tables in SFDP for
> octal flashes and as I understanding proprietary table is not a
> stardard.
> So that content and address are not identical each vendor, it will need
> a manufacturer fixups for reading.
Yes thats by definition not a standard, *but* the header with the
manufacturer id *is* standard.
So better include some proprietary tables in your next NOR flashes ;)
-michael
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 3/5] spi: spi-mem: Allow specifying the byte order in DTR mode
2023-08-09 1:36 ` liao jaime
@ 2023-08-10 7:31 ` Michael Walle
0 siblings, 0 replies; 21+ messages in thread
From: Michael Walle @ 2023-08-10 7:31 UTC (permalink / raw)
To: liao jaime
Cc: linux-mtd, tudor.ambarus, pratyush, miquel.raynal, leoyu,
JaimeLiao
Hi,
>> > There are NOR flashes (Macronix) that swap the bytes on a 16-bit
>> > boundary when configured in Octal DTR mode. The byte order of
>> > 16-bit words is swapped when read or written in Octal Double
>> > Transfer Rate (DTR) mode compared to Single Transfer Rate (STR)
>> > modes. If one writes D0 D1 D2 D3 bytes using 1-1-1 mode, and uses
>> > 8D-8D-8D SPI mode for reading, it will read back D1 D0 D3 D2.
>> > Swapping the bytes may introduce some endianness problems. It can
>> > affect the boot sequence if the entire boot sequence is not handled
>> > in either 8D-8D-8D mode or 1-1-1 mode. So we must swap the bytes
>> > back to have the same byte order as in STR modes. Fortunately there
>> > are controllers that could swap the bytes back at runtime,
>> > addressing the flash's endiannesses requirements. Provide a way for
>> > the upper layers to specify the byte order in Octal DTR mode.
>> >
>> > Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
>>
>> .. this. If you pick up a patch from another author, you should
>> keep the author of the patch.
> How could I do for this?
> Because of Tudor's patch is on old Linux kernel version, it need
> a small changes for suiting on v6.5-rc3.
Ahh I see, you changed that patch. I'd pick up the patch, leave the
original author and SoB, describe your changes and add your SoB
after the one from Tudor.
But I still don't see how these patches will help with your current
problem.
-michael
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 2/5] mtd: spi-nor: core: Hook manufacture by checking first byte ID
2023-08-10 7:27 ` Michael Walle
@ 2023-08-11 9:03 ` liao jaime
2023-08-11 10:11 ` Tudor Ambarus
2023-08-11 10:20 ` Michael Walle
0 siblings, 2 replies; 21+ messages in thread
From: liao jaime @ 2023-08-11 9:03 UTC (permalink / raw)
To: Michael Walle
Cc: linux-mtd, tudor.ambarus, pratyush, miquel.raynal, leoyu,
JaimeLiao
Hi Michael
>
> Hi,
>
> >> This won't work beacuse the manufacturer id is not always
> >> one byte long, think of continuation codes. In fact, as the
> >> flash_info table is of now, we cannot even rely on the
> >> continuation codes, but we have to always check for the
> >> complete id_len, i.e. there is at least one hack where
> >> the id is reversed and the manufacturer is the last byte,
> >> iirc. some oddball cypress mram chip.
> > According JEDEC standard, 1st byte is manufacture ID.
> > I check id table, "cy15x104q" with multi manufacture ID in
> > later bytes by RDID command(9F).
>
> Yes the currently supported cy15x104q is broken.. but nevertheless
> it's there. Also some spansion flashes uses winbond manufacturer
> ids.
Ok, got it.
I have a idea, create a member maybe name .check_vendor in spi_nor_manufacturer.
A example, macronix_check_vendor function for checking this IC whether
belong macronix or not.
Each vendor could create their checking function for ID table didn't
include that ID.
Is it ok?
For general data information from RDID(0x9f) command, I prefer
checking 1st byte.
After checking, gigadevice , eon, esmt and macronix could follow this method.
Some vendor with multiple maf ID still could use this function for
checking vendor.
For some special cases like Spansion and Windbond, I think their
function should based on others like vendor space in SFDP.
>
> > Maybe some old flash didn't follow this but I think it isn't
> > a high percentage.
> > Most of all and new product are follow JEDEC.
>
> Have a look at JEP106 (mine is JEP106BC) and you'll see all the
> manufacturer IDs defined by JEDEC and you'll see that except
> for the first 127, all others are multibyte vendor IDs starting
> with continuation codes.
>
> And it seems that the command RDID 9F is not covered in any
> JEDEC standard. At least I haven't found anything. If you know
> where that command is defined, please let me know.
Sorry, after checking, content of RDID 9F seems not a standard.
>
> >> If you want to get the correct manufacturer for spi-nor-generic,
> >> you should extract it from the SFDP tables. It seems that the
> >> BFPT don't include a manufacturer id, but if there are proprietary
> >> tables, you *might* use that id. I say might, because it only works
> >> with one byte manufacturer ids, no continuation codes... *sigh*
> >
> > Unfortunately, Macronix didn't include proprietary tables in SFDP for
> > octal flashes and as I understanding proprietary table is not a
> > stardard.
> > So that content and address are not identical each vendor, it will need
> > a manufacturer fixups for reading.
>
> Yes thats by definition not a standard, *but* the header with the
> manufacturer id *is* standard.
Got it.
>
> So better include some proprietary tables in your next NOR flashes ;)
In my opinion, this may not a good way because of SFDP is growing up
then proprietary tables may have conflict address in the future.
So that Macronix octal flashes didn't include it now.
>
> -michael
Thanks
Jaime
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 2/5] mtd: spi-nor: core: Hook manufacture by checking first byte ID
2023-08-11 9:03 ` liao jaime
@ 2023-08-11 10:11 ` Tudor Ambarus
2023-08-14 8:04 ` liao jaime
2023-08-11 10:20 ` Michael Walle
1 sibling, 1 reply; 21+ messages in thread
From: Tudor Ambarus @ 2023-08-11 10:11 UTC (permalink / raw)
To: liao jaime, Michael Walle
Cc: linux-mtd, pratyush, miquel.raynal, leoyu, JaimeLiao
On 8/11/23 10:03, liao jaime wrote:
>> So better include some proprietary tables in your next NOR flashes 😉
> In my opinion, this may not a good way because of SFDP is growing up
> then proprietary tables may have conflict address in the future.
> So that Macronix octal flashes didn't include it now.
>
SFDP and vendor tables can't collide as they use different Parameter
ID MSB in the Parameter Header. Check section 6.3.3 in jesd216f-02.
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 2/5] mtd: spi-nor: core: Hook manufacture by checking first byte ID
2023-08-11 9:03 ` liao jaime
2023-08-11 10:11 ` Tudor Ambarus
@ 2023-08-11 10:20 ` Michael Walle
2023-08-14 8:24 ` liao jaime
1 sibling, 1 reply; 21+ messages in thread
From: Michael Walle @ 2023-08-11 10:20 UTC (permalink / raw)
To: liao jaime
Cc: linux-mtd, tudor.ambarus, pratyush, miquel.raynal, leoyu,
JaimeLiao
Hi,
>> >> This won't work beacuse the manufacturer id is not always
>> >> one byte long, think of continuation codes. In fact, as the
>> >> flash_info table is of now, we cannot even rely on the
>> >> continuation codes, but we have to always check for the
>> >> complete id_len, i.e. there is at least one hack where
>> >> the id is reversed and the manufacturer is the last byte,
>> >> iirc. some oddball cypress mram chip.
>> > According JEDEC standard, 1st byte is manufacture ID.
>> > I check id table, "cy15x104q" with multi manufacture ID in
>> > later bytes by RDID command(9F).
>>
>> Yes the currently supported cy15x104q is broken.. but nevertheless
>> it's there. Also some spansion flashes uses winbond manufacturer
>> ids.
> Ok, got it.
> I have a idea, create a member maybe name .check_vendor in
> spi_nor_manufacturer.
> A example, macronix_check_vendor function for checking this IC whether
> belong macronix or not.
> Each vendor could create their checking function for ID table didn't
> include that ID.
> Is it ok?
Honestly, I'm against adding that vendor discovery stuff to the
generic nor driver. What's the use case of it? You can just
read the ID from userspace and a tool there can decide which
flash it is.
-michael
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 2/5] mtd: spi-nor: core: Hook manufacture by checking first byte ID
2023-08-11 10:11 ` Tudor Ambarus
@ 2023-08-14 8:04 ` liao jaime
0 siblings, 0 replies; 21+ messages in thread
From: liao jaime @ 2023-08-14 8:04 UTC (permalink / raw)
To: Tudor Ambarus
Cc: Michael Walle, linux-mtd, pratyush, miquel.raynal, leoyu,
JaimeLiao
Hi Tudor
>
>
>
> On 8/11/23 10:03, liao jaime wrote:
> >> So better include some proprietary tables in your next NOR flashes 😉
> > In my opinion, this may not a good way because of SFDP is growing up
> > then proprietary tables may have conflict address in the future.
> > So that Macronix octal flashes didn't include it now.
> >
>
> SFDP and vendor tables can't collide as they use different Parameter
> ID MSB in the Parameter Header. Check section 6.3.3 in jesd216f-02.
Thanks.
For SFDP vendor table, I will discuss with PM internally.
Thanks
Jaime
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 2/5] mtd: spi-nor: core: Hook manufacture by checking first byte ID
2023-08-11 10:20 ` Michael Walle
@ 2023-08-14 8:24 ` liao jaime
2023-08-31 3:18 ` liao jaime
0 siblings, 1 reply; 21+ messages in thread
From: liao jaime @ 2023-08-14 8:24 UTC (permalink / raw)
To: Michael Walle
Cc: linux-mtd, tudor.ambarus, pratyush, miquel.raynal, leoyu,
JaimeLiao
Hi Michael
>
> Hi,
>
> >> >> This won't work beacuse the manufacturer id is not always
> >> >> one byte long, think of continuation codes. In fact, as the
> >> >> flash_info table is of now, we cannot even rely on the
> >> >> continuation codes, but we have to always check for the
> >> >> complete id_len, i.e. there is at least one hack where
> >> >> the id is reversed and the manufacturer is the last byte,
> >> >> iirc. some oddball cypress mram chip.
> >> > According JEDEC standard, 1st byte is manufacture ID.
> >> > I check id table, "cy15x104q" with multi manufacture ID in
> >> > later bytes by RDID command(9F).
> >>
> >> Yes the currently supported cy15x104q is broken.. but nevertheless
> >> it's there. Also some spansion flashes uses winbond manufacturer
> >> ids.
> > Ok, got it.
> > I have a idea, create a member maybe name .check_vendor in
> > spi_nor_manufacturer.
> > A example, macronix_check_vendor function for checking this IC whether
> > belong macronix or not.
> > Each vendor could create their checking function for ID table didn't
> > include that ID.
> > Is it ok?
>
> Honestly, I'm against adding that vendor discovery stuff to the
> generic nor driver. What's the use case of it? You can just
> read the ID from userspace and a tool there can decide which
> flash it is.
Most of information could get by parsing SFDP.
We hope that highest protocol(8D-8D-8D) could used without adding IDs.
Because of octal_dtr function is still reply on mafacturer hook function.
So that vendor discovery could make up deficiency.
>
> -michael
Thanks
Jaime
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 2/5] mtd: spi-nor: core: Hook manufacture by checking first byte ID
2023-08-14 8:24 ` liao jaime
@ 2023-08-31 3:18 ` liao jaime
2023-09-04 14:54 ` Michael Walle
0 siblings, 1 reply; 21+ messages in thread
From: liao jaime @ 2023-08-31 3:18 UTC (permalink / raw)
To: Michael Walle
Cc: linux-mtd, tudor.ambarus, pratyush, miquel.raynal, leoyu,
JaimeLiao
Hi Michael , Tudor
>
> Hi Michael
>
> >
> > Hi,
> >
> > >> >> This won't work beacuse the manufacturer id is not always
> > >> >> one byte long, think of continuation codes. In fact, as the
> > >> >> flash_info table is of now, we cannot even rely on the
> > >> >> continuation codes, but we have to always check for the
> > >> >> complete id_len, i.e. there is at least one hack where
> > >> >> the id is reversed and the manufacturer is the last byte,
> > >> >> iirc. some oddball cypress mram chip.
> > >> > According JEDEC standard, 1st byte is manufacture ID.
> > >> > I check id table, "cy15x104q" with multi manufacture ID in
> > >> > later bytes by RDID command(9F).
> > >>
> > >> Yes the currently supported cy15x104q is broken.. but nevertheless
> > >> it's there. Also some spansion flashes uses winbond manufacturer
> > >> ids.
> > > Ok, got it.
> > > I have a idea, create a member maybe name .check_vendor in
> > > spi_nor_manufacturer.
> > > A example, macronix_check_vendor function for checking this IC whether
> > > belong macronix or not.
> > > Each vendor could create their checking function for ID table didn't
> > > include that ID.
> > > Is it ok?
> >
> > Honestly, I'm against adding that vendor discovery stuff to the
> > generic nor driver. What's the use case of it? You can just
> > read the ID from userspace and a tool there can decide which
> > flash it is.
> Most of information could get by parsing SFDP.
> We hope that highest protocol(8D-8D-8D) could used without adding IDs.
> Because of octal_dtr function is still reply on mafacturer hook function.
> So that vendor discovery could make up deficiency.
For this part, have any ideas?
We hope that we could do something for reducing reliability of ID table.
It will be good if new flashes could get all features without comparing flash id
on ID tables.
>
> >
> > -michael
> Thanks
> Jaime
Thanks
Jaime
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 2/5] mtd: spi-nor: core: Hook manufacture by checking first byte ID
2023-08-31 3:18 ` liao jaime
@ 2023-09-04 14:54 ` Michael Walle
0 siblings, 0 replies; 21+ messages in thread
From: Michael Walle @ 2023-09-04 14:54 UTC (permalink / raw)
To: liao jaime
Cc: linux-mtd, tudor.ambarus, pratyush, miquel.raynal, leoyu,
JaimeLiao
Hi,
>> Most of information could get by parsing SFDP.
>> We hope that highest protocol(8D-8D-8D) could used without adding IDs.
>> Because of octal_dtr function is still reply on mafacturer hook
>> function.
>> So that vendor discovery could make up deficiency.
>
> For this part, have any ideas?
> We hope that we could do something for reducing reliability of ID
> table.
> It will be good if new flashes could get all features without comparing
> flash id
> on ID tables.
Ahh, that was the use case for that. I suggest you look into the SCCR
SFDP
tables. You should get everything from the 23rd DWORD. At least the
Application
note you've sent me, shows these values are populated and by a brief
look
it seems to match your hand written code in patch 1. Which means you
don't
need any code in macronix.c. Instead you should extend the SCCR parser.
-michael
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2023-09-04 14:54 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-08-04 9:54 [PATCH v3 0/5] Add octal DTR support for Macronix flash Jaime Liao
2023-08-04 9:54 ` [PATCH v3 1/5] mtd: spi-nor: add Octal " Jaime Liao
2023-08-07 6:44 ` Michael Walle
2023-08-04 9:54 ` [PATCH v3 2/5] mtd: spi-nor: core: Hook manufacture by checking first byte ID Jaime Liao
2023-08-07 6:37 ` Michael Walle
2023-08-09 1:04 ` liao jaime
2023-08-10 7:27 ` Michael Walle
2023-08-11 9:03 ` liao jaime
2023-08-11 10:11 ` Tudor Ambarus
2023-08-14 8:04 ` liao jaime
2023-08-11 10:20 ` Michael Walle
2023-08-14 8:24 ` liao jaime
2023-08-31 3:18 ` liao jaime
2023-09-04 14:54 ` Michael Walle
2023-08-04 9:54 ` [PATCH v3 3/5] spi: spi-mem: Allow specifying the byte order in DTR mode Jaime Liao
2023-08-07 6:40 ` Michael Walle
2023-08-09 1:36 ` liao jaime
2023-08-10 7:31 ` Michael Walle
2023-08-04 9:54 ` [PATCH v3 4/5] mtd: spi-nor: core: " Jaime Liao
2023-08-04 9:54 ` [PATCH v3 5/5] mtd: spi-nor: sfdp: Get the 8D-8D-8D byte order from BFPT Jaime Liao
2023-08-07 6:42 ` [PATCH v3 0/5] Add octal DTR support for Macronix flash Michael Walle
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox