* [PATCH v13 01/15] mtd: spi-nor: core: use EOPNOTSUPP instead of ENOTSUPP
2020-09-16 12:44 [PATCH v13 00/15] mtd: spi-nor: add xSPI Octal DTR support Pratyush Yadav
@ 2020-09-16 12:44 ` Pratyush Yadav
2020-09-29 11:30 ` Tudor.Ambarus
2020-09-16 12:44 ` [PATCH v13 02/15] mtd: spi-nor: core: add spi_nor_{read, write}_reg() helpers Pratyush Yadav
` (15 subsequent siblings)
16 siblings, 1 reply; 49+ messages in thread
From: Pratyush Yadav @ 2020-09-16 12:44 UTC (permalink / raw)
To: Tudor Ambarus, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, linux-mtd, linux-kernel
Cc: Boris Brezillon, Sekhar Nori, Pratyush Yadav
ENOTSUPP is not a SUSV4 error code. Using EOPNOTSUPP is preferred
in its stead.
Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
---
drivers/mtd/spi-nor/core.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 65eff4ce6ab1..623384ef9a5d 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -2297,7 +2297,7 @@ static int spi_nor_hwcaps_pp2cmd(u32 hwcaps)
*@nor: pointer to a 'struct spi_nor'
*@op: pointer to op template to be checked
*
- * Returns 0 if operation is supported, -ENOTSUPP otherwise.
+ * Returns 0 if operation is supported, -EOPNOTSUPP otherwise.
*/
static int spi_nor_spimem_check_op(struct spi_nor *nor,
struct spi_mem_op *op)
@@ -2311,12 +2311,12 @@ static int spi_nor_spimem_check_op(struct spi_nor *nor,
op->addr.nbytes = 4;
if (!spi_mem_supports_op(nor->spimem, op)) {
if (nor->mtd.size > SZ_16M)
- return -ENOTSUPP;
+ return -EOPNOTSUPP;
/* If flash size <= 16MB, 3 address bytes are sufficient */
op->addr.nbytes = 3;
if (!spi_mem_supports_op(nor->spimem, op))
- return -ENOTSUPP;
+ return -EOPNOTSUPP;
}
return 0;
@@ -2328,7 +2328,7 @@ static int spi_nor_spimem_check_op(struct spi_nor *nor,
*@nor: pointer to a 'struct spi_nor'
*@read: pointer to op template to be checked
*
- * Returns 0 if operation is supported, -ENOTSUPP otherwise.
+ * Returns 0 if operation is supported, -EOPNOTSUPP otherwise.
*/
static int spi_nor_spimem_check_readop(struct spi_nor *nor,
const struct spi_nor_read_command *read)
@@ -2354,7 +2354,7 @@ static int spi_nor_spimem_check_readop(struct spi_nor *nor,
*@nor: pointer to a 'struct spi_nor'
*@pp: pointer to op template to be checked
*
- * Returns 0 if operation is supported, -ENOTSUPP otherwise.
+ * Returns 0 if operation is supported, -EOPNOTSUPP otherwise.
*/
static int spi_nor_spimem_check_pp(struct spi_nor *nor,
const struct spi_nor_pp_command *pp)
--
2.28.0
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 49+ messages in thread
* Re: [PATCH v13 01/15] mtd: spi-nor: core: use EOPNOTSUPP instead of ENOTSUPP
2020-09-16 12:44 ` [PATCH v13 01/15] mtd: spi-nor: core: use EOPNOTSUPP instead of ENOTSUPP Pratyush Yadav
@ 2020-09-29 11:30 ` Tudor.Ambarus
0 siblings, 0 replies; 49+ messages in thread
From: Tudor.Ambarus @ 2020-09-29 11:30 UTC (permalink / raw)
To: p.yadav, miquel.raynal, richard, vigneshr, linux-mtd, linux-kernel
Cc: boris.brezillon, nsekhar
On 9/16/20 3:44 PM, Pratyush Yadav wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> ENOTSUPP is not a SUSV4 error code. Using EOPNOTSUPP is preferred
> in its stead.
>
> Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> ---
> drivers/mtd/spi-nor/core.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index 65eff4ce6ab1..623384ef9a5d 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -2297,7 +2297,7 @@ static int spi_nor_hwcaps_pp2cmd(u32 hwcaps)
> *@nor: pointer to a 'struct spi_nor'
> *@op: pointer to op template to be checked
> *
> - * Returns 0 if operation is supported, -ENOTSUPP otherwise.
> + * Returns 0 if operation is supported, -EOPNOTSUPP otherwise.
> */
> static int spi_nor_spimem_check_op(struct spi_nor *nor,
> struct spi_mem_op *op)
> @@ -2311,12 +2311,12 @@ static int spi_nor_spimem_check_op(struct spi_nor *nor,
> op->addr.nbytes = 4;
> if (!spi_mem_supports_op(nor->spimem, op)) {
> if (nor->mtd.size > SZ_16M)
> - return -ENOTSUPP;
> + return -EOPNOTSUPP;
>
> /* If flash size <= 16MB, 3 address bytes are sufficient */
> op->addr.nbytes = 3;
> if (!spi_mem_supports_op(nor->spimem, op))
> - return -ENOTSUPP;
> + return -EOPNOTSUPP;
> }
>
> return 0;
> @@ -2328,7 +2328,7 @@ static int spi_nor_spimem_check_op(struct spi_nor *nor,
> *@nor: pointer to a 'struct spi_nor'
> *@read: pointer to op template to be checked
> *
> - * Returns 0 if operation is supported, -ENOTSUPP otherwise.
> + * Returns 0 if operation is supported, -EOPNOTSUPP otherwise.
> */
> static int spi_nor_spimem_check_readop(struct spi_nor *nor,
> const struct spi_nor_read_command *read)
> @@ -2354,7 +2354,7 @@ static int spi_nor_spimem_check_readop(struct spi_nor *nor,
> *@nor: pointer to a 'struct spi_nor'
> *@pp: pointer to op template to be checked
> *
> - * Returns 0 if operation is supported, -ENOTSUPP otherwise.
> + * Returns 0 if operation is supported, -EOPNOTSUPP otherwise.
> */
> static int spi_nor_spimem_check_pp(struct spi_nor *nor,
> const struct spi_nor_pp_command *pp)
> --
> 2.28.0
>
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH v13 02/15] mtd: spi-nor: core: add spi_nor_{read, write}_reg() helpers
2020-09-16 12:44 [PATCH v13 00/15] mtd: spi-nor: add xSPI Octal DTR support Pratyush Yadav
2020-09-16 12:44 ` [PATCH v13 01/15] mtd: spi-nor: core: use EOPNOTSUPP instead of ENOTSUPP Pratyush Yadav
@ 2020-09-16 12:44 ` Pratyush Yadav
2020-09-29 11:38 ` [PATCH v13 02/15] mtd: spi-nor: core: add spi_nor_{read,write}_reg() helpers Tudor.Ambarus
2020-09-16 12:44 ` [PATCH v13 03/15] mtd: spi-nor: core: add spi_nor_controller_ops_erase helper Pratyush Yadav
` (14 subsequent siblings)
16 siblings, 1 reply; 49+ messages in thread
From: Pratyush Yadav @ 2020-09-16 12:44 UTC (permalink / raw)
To: Tudor Ambarus, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, linux-mtd, linux-kernel
Cc: Boris Brezillon, Sekhar Nori, Pratyush Yadav
They are thin wrappers around nor->controller_ops->{read,write}_reg().
In a future commit DTR support will be added. These ops can not be
supported by the {read,write}_reg() hooks and these helpers will make it
easier to reject those calls.
Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
---
drivers/mtd/spi-nor/core.c | 65 +++++++++++++++++++-------------------
1 file changed, 32 insertions(+), 33 deletions(-)
diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 623384ef9a5d..cbfc4c2df79d 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -82,6 +82,18 @@ static int spi_nor_spimem_exec_op(struct spi_nor *nor, struct spi_mem_op *op)
return spi_mem_exec_op(nor->spimem, op);
}
+static int spi_nor_read_reg(struct spi_nor *nor, u8 opcode, u8 *buf,
+ size_t len)
+{
+ return nor->controller_ops->read_reg(nor, opcode, buf, len);
+}
+
+static int spi_nor_write_reg(struct spi_nor *nor, u8 opcode, const u8 *buf,
+ size_t len)
+{
+ return nor->controller_ops->write_reg(nor, opcode, buf, len);
+}
+
/**
* spi_nor_spimem_read_data() - read data from flash's memory region via
* spi-mem
@@ -229,8 +241,7 @@ int spi_nor_write_enable(struct spi_nor *nor)
ret = spi_mem_exec_op(nor->spimem, &op);
} else {
- ret = nor->controller_ops->write_reg(nor, SPINOR_OP_WREN,
- NULL, 0);
+ ret = spi_nor_write_reg(nor, SPINOR_OP_WREN, NULL, 0);
}
if (ret)
@@ -258,8 +269,7 @@ int spi_nor_write_disable(struct spi_nor *nor)
ret = spi_mem_exec_op(nor->spimem, &op);
} else {
- ret = nor->controller_ops->write_reg(nor, SPINOR_OP_WRDI,
- NULL, 0);
+ ret = spi_nor_write_reg(nor, SPINOR_OP_WRDI, NULL, 0);
}
if (ret)
@@ -289,8 +299,7 @@ static int spi_nor_read_sr(struct spi_nor *nor, u8 *sr)
ret = spi_mem_exec_op(nor->spimem, &op);
} else {
- ret = nor->controller_ops->read_reg(nor, SPINOR_OP_RDSR,
- sr, 1);
+ ret = spi_nor_read_reg(nor, SPINOR_OP_RDSR, sr, 1);
}
if (ret)
@@ -320,8 +329,7 @@ static int spi_nor_read_fsr(struct spi_nor *nor, u8 *fsr)
ret = spi_mem_exec_op(nor->spimem, &op);
} else {
- ret = nor->controller_ops->read_reg(nor, SPINOR_OP_RDFSR,
- fsr, 1);
+ ret = spi_nor_read_reg(nor, SPINOR_OP_RDFSR, fsr, 1);
}
if (ret)
@@ -352,7 +360,7 @@ static int spi_nor_read_cr(struct spi_nor *nor, u8 *cr)
ret = spi_mem_exec_op(nor->spimem, &op);
} else {
- ret = nor->controller_ops->read_reg(nor, SPINOR_OP_RDCR, cr, 1);
+ ret = spi_nor_read_reg(nor, SPINOR_OP_RDCR, cr, 1);
}
if (ret)
@@ -385,10 +393,10 @@ int spi_nor_set_4byte_addr_mode(struct spi_nor *nor, bool enable)
ret = spi_mem_exec_op(nor->spimem, &op);
} else {
- ret = nor->controller_ops->write_reg(nor,
- enable ? SPINOR_OP_EN4B :
- SPINOR_OP_EX4B,
- NULL, 0);
+ ret = spi_nor_write_reg(nor,
+ enable ? SPINOR_OP_EN4B :
+ SPINOR_OP_EX4B,
+ NULL, 0);
}
if (ret)
@@ -421,8 +429,7 @@ static int spansion_set_4byte_addr_mode(struct spi_nor *nor, bool enable)
ret = spi_mem_exec_op(nor->spimem, &op);
} else {
- ret = nor->controller_ops->write_reg(nor, SPINOR_OP_BRWR,
- nor->bouncebuf, 1);
+ ret = spi_nor_write_reg(nor, SPINOR_OP_BRWR, nor->bouncebuf, 1);
}
if (ret)
@@ -453,8 +460,7 @@ int spi_nor_write_ear(struct spi_nor *nor, u8 ear)
ret = spi_mem_exec_op(nor->spimem, &op);
} else {
- ret = nor->controller_ops->write_reg(nor, SPINOR_OP_WREAR,
- nor->bouncebuf, 1);
+ ret = spi_nor_write_reg(nor, SPINOR_OP_WREAR, nor->bouncebuf, 1);
}
if (ret)
@@ -484,8 +490,7 @@ int spi_nor_xread_sr(struct spi_nor *nor, u8 *sr)
ret = spi_mem_exec_op(nor->spimem, &op);
} else {
- ret = nor->controller_ops->read_reg(nor, SPINOR_OP_XRDSR,
- sr, 1);
+ ret = spi_nor_read_reg(nor, SPINOR_OP_XRDSR, sr, 1);
}
if (ret)
@@ -529,8 +534,7 @@ static void spi_nor_clear_sr(struct spi_nor *nor)
ret = spi_mem_exec_op(nor->spimem, &op);
} else {
- ret = nor->controller_ops->write_reg(nor, SPINOR_OP_CLSR,
- NULL, 0);
+ ret = spi_nor_write_reg(nor, SPINOR_OP_CLSR, NULL, 0);
}
if (ret)
@@ -593,8 +597,7 @@ static void spi_nor_clear_fsr(struct spi_nor *nor)
ret = spi_mem_exec_op(nor->spimem, &op);
} else {
- ret = nor->controller_ops->write_reg(nor, SPINOR_OP_CLFSR,
- NULL, 0);
+ ret = spi_nor_write_reg(nor, SPINOR_OP_CLFSR, NULL, 0);
}
if (ret)
@@ -737,8 +740,7 @@ static int spi_nor_write_sr(struct spi_nor *nor, const u8 *sr, size_t len)
ret = spi_mem_exec_op(nor->spimem, &op);
} else {
- ret = nor->controller_ops->write_reg(nor, SPINOR_OP_WRSR,
- sr, len);
+ ret = spi_nor_write_reg(nor, SPINOR_OP_WRSR, sr, len);
}
if (ret) {
@@ -939,8 +941,7 @@ static int spi_nor_write_sr2(struct spi_nor *nor, const u8 *sr2)
ret = spi_mem_exec_op(nor->spimem, &op);
} else {
- ret = nor->controller_ops->write_reg(nor, SPINOR_OP_WRSR2,
- sr2, 1);
+ ret = spi_nor_write_reg(nor, SPINOR_OP_WRSR2, sr2, 1);
}
if (ret) {
@@ -973,8 +974,7 @@ static int spi_nor_read_sr2(struct spi_nor *nor, u8 *sr2)
ret = spi_mem_exec_op(nor->spimem, &op);
} else {
- ret = nor->controller_ops->read_reg(nor, SPINOR_OP_RDSR2,
- sr2, 1);
+ ret = spi_nor_read_reg(nor, SPINOR_OP_RDSR2, sr2, 1);
}
if (ret)
@@ -1004,8 +1004,7 @@ static int spi_nor_erase_chip(struct spi_nor *nor)
ret = spi_mem_exec_op(nor->spimem, &op);
} else {
- ret = nor->controller_ops->write_reg(nor, SPINOR_OP_CHIP_ERASE,
- NULL, 0);
+ ret = spi_nor_write_reg(nor, SPINOR_OP_CHIP_ERASE, NULL, 0);
}
if (ret)
@@ -1158,8 +1157,8 @@ static int spi_nor_erase_sector(struct spi_nor *nor, u32 addr)
addr >>= 8;
}
- return nor->controller_ops->write_reg(nor, nor->erase_opcode,
- nor->bouncebuf, nor->addr_width);
+ return spi_nor_write_reg(nor, nor->erase_opcode, nor->bouncebuf,
+ nor->addr_width);
}
/**
--
2.28.0
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 49+ messages in thread
* Re: [PATCH v13 02/15] mtd: spi-nor: core: add spi_nor_{read,write}_reg() helpers
2020-09-16 12:44 ` [PATCH v13 02/15] mtd: spi-nor: core: add spi_nor_{read, write}_reg() helpers Pratyush Yadav
@ 2020-09-29 11:38 ` Tudor.Ambarus
2020-09-29 12:54 ` Pratyush Yadav
0 siblings, 1 reply; 49+ messages in thread
From: Tudor.Ambarus @ 2020-09-29 11:38 UTC (permalink / raw)
To: p.yadav, miquel.raynal, richard, vigneshr, linux-mtd, linux-kernel
Cc: boris.brezillon, nsekhar
On 9/16/20 3:44 PM, Pratyush Yadav wrote:
> They are thin wrappers around nor->controller_ops->{read,write}_reg().
> In a future commit DTR support will be added. These ops can not be
> supported by the {read,write}_reg() hooks and these helpers will make it
> easier to reject those calls.
2/15 and 3/15 are introducing wrappers for the controller ops. How about
squashing the commits and using the same naming scheme?
spi_nor_controller_ops_{read_reg,write_reg,erase}
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v13 02/15] mtd: spi-nor: core: add spi_nor_{read,write}_reg() helpers
2020-09-29 11:38 ` [PATCH v13 02/15] mtd: spi-nor: core: add spi_nor_{read,write}_reg() helpers Tudor.Ambarus
@ 2020-09-29 12:54 ` Pratyush Yadav
0 siblings, 0 replies; 49+ messages in thread
From: Pratyush Yadav @ 2020-09-29 12:54 UTC (permalink / raw)
To: Tudor.Ambarus
Cc: vigneshr, richard, nsekhar, linux-kernel, boris.brezillon,
linux-mtd, miquel.raynal
On 29/09/20 11:38AM, Tudor.Ambarus@microchip.com wrote:
> On 9/16/20 3:44 PM, Pratyush Yadav wrote:
> > They are thin wrappers around nor->controller_ops->{read,write}_reg().
> > In a future commit DTR support will be added. These ops can not be
> > supported by the {read,write}_reg() hooks and these helpers will make it
> > easier to reject those calls.
>
> 2/15 and 3/15 are introducing wrappers for the controller ops. How about
> squashing the commits and using the same naming scheme?
>
> spi_nor_controller_ops_{read_reg,write_reg,erase}
Ok. Will do. For my information, the patches will still go in this merge
window, right?
--
Regards,
Pratyush Yadav
Texas Instruments India
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH v13 03/15] mtd: spi-nor: core: add spi_nor_controller_ops_erase helper
2020-09-16 12:44 [PATCH v13 00/15] mtd: spi-nor: add xSPI Octal DTR support Pratyush Yadav
2020-09-16 12:44 ` [PATCH v13 01/15] mtd: spi-nor: core: use EOPNOTSUPP instead of ENOTSUPP Pratyush Yadav
2020-09-16 12:44 ` [PATCH v13 02/15] mtd: spi-nor: core: add spi_nor_{read, write}_reg() helpers Pratyush Yadav
@ 2020-09-16 12:44 ` Pratyush Yadav
2020-09-16 12:44 ` [PATCH v13 04/15] mtd: spi-nor: add support for DTR protocol Pratyush Yadav
` (13 subsequent siblings)
16 siblings, 0 replies; 49+ messages in thread
From: Pratyush Yadav @ 2020-09-16 12:44 UTC (permalink / raw)
To: Tudor Ambarus, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, linux-mtd, linux-kernel
Cc: Boris Brezillon, Sekhar Nori, Pratyush Yadav
It is a thin wrapper around nor->controller_ops->erase(). In a future
commit DTR support will be added. These ops can not be supported by the
erase() hook and this helper will make it easier to reject those calls.
Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
---
drivers/mtd/spi-nor/core.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index cbfc4c2df79d..7009cb702e22 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -94,6 +94,11 @@ static int spi_nor_write_reg(struct spi_nor *nor, u8 opcode, const u8 *buf,
return nor->controller_ops->write_reg(nor, opcode, buf, len);
}
+static int spi_nor_controller_ops_erase(struct spi_nor *nor, loff_t offs)
+{
+ return nor->controller_ops->erase(nor, offs);
+}
+
/**
* spi_nor_spimem_read_data() - read data from flash's memory region via
* spi-mem
@@ -1145,7 +1150,7 @@ static int spi_nor_erase_sector(struct spi_nor *nor, u32 addr)
return spi_mem_exec_op(nor->spimem, &op);
} else if (nor->controller_ops->erase) {
- return nor->controller_ops->erase(nor, addr);
+ return spi_nor_controller_ops_erase(nor, addr);
}
/*
--
2.28.0
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [PATCH v13 04/15] mtd: spi-nor: add support for DTR protocol
2020-09-16 12:44 [PATCH v13 00/15] mtd: spi-nor: add xSPI Octal DTR support Pratyush Yadav
` (2 preceding siblings ...)
2020-09-16 12:44 ` [PATCH v13 03/15] mtd: spi-nor: core: add spi_nor_controller_ops_erase helper Pratyush Yadav
@ 2020-09-16 12:44 ` Pratyush Yadav
2020-09-16 12:44 ` [PATCH v13 05/15] mtd: spi-nor: sfdp: get command opcode extension type from BFPT Pratyush Yadav
` (12 subsequent siblings)
16 siblings, 0 replies; 49+ messages in thread
From: Pratyush Yadav @ 2020-09-16 12:44 UTC (permalink / raw)
To: Tudor Ambarus, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, linux-mtd, linux-kernel
Cc: Boris Brezillon, Sekhar Nori, Pratyush Yadav
Double Transfer Rate (DTR) is SPI protocol in which data is transferred
on each clock edge as opposed to on each clock cycle. Make
framework-level changes to allow supporting flashes in DTR mode.
Right now, mixed DTR modes are not supported. So, for example a mode
like 4S-4D-4D will not work. All phases need to be either DTR or STR.
Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
---
drivers/mtd/spi-nor/core.c | 222 +++++++++++++++++++++++++++++-------
drivers/mtd/spi-nor/core.h | 7 ++
drivers/mtd/spi-nor/sfdp.c | 9 +-
include/linux/mtd/spi-nor.h | 51 ++++++---
4 files changed, 236 insertions(+), 53 deletions(-)
diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 7009cb702e22..7445d7122304 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -40,6 +40,78 @@
#define SPI_NOR_MAX_ADDR_WIDTH 4
+/**
+ * spi_nor_get_cmd_ext() - Get the command opcode extension based on the
+ * extension type.
+ * @nor: pointer to a 'struct spi_nor'
+ * @op: pointer to the 'struct spi_mem_op' whose properties
+ * need to be initialized.
+ *
+ * Right now, only "repeat" and "invert" are supported.
+ *
+ * Return: The opcode extension.
+ */
+static u8 spi_nor_get_cmd_ext(const struct spi_nor *nor,
+ const struct spi_mem_op *op)
+{
+ switch (nor->cmd_ext_type) {
+ case SPI_NOR_EXT_INVERT:
+ return ~op->cmd.opcode;
+
+ case SPI_NOR_EXT_REPEAT:
+ return op->cmd.opcode;
+
+ default:
+ dev_err(nor->dev, "Unknown command extension type\n");
+ return 0;
+ }
+}
+
+/**
+ * spi_nor_spimem_setup_op() - Set up common properties of a spi-mem op.
+ * @nor: pointer to a 'struct spi_nor'
+ * @op: pointer to the 'struct spi_mem_op' whose properties
+ * need to be initialized.
+ * @proto: the protocol from which the properties need to be set.
+ */
+void spi_nor_spimem_setup_op(const struct spi_nor *nor,
+ struct spi_mem_op *op,
+ const enum spi_nor_protocol proto)
+{
+ u8 ext;
+
+ op->cmd.buswidth = spi_nor_get_protocol_inst_nbits(proto);
+
+ if (op->addr.nbytes)
+ op->addr.buswidth = spi_nor_get_protocol_addr_nbits(proto);
+
+ if (op->dummy.nbytes)
+ op->dummy.buswidth = spi_nor_get_protocol_addr_nbits(proto);
+
+ if (op->data.nbytes)
+ op->data.buswidth = spi_nor_get_protocol_data_nbits(proto);
+
+ if (spi_nor_protocol_is_dtr(proto)) {
+ /*
+ * SPIMEM supports mixed DTR modes, but right now we can only
+ * have all phases either DTR or STR. IOW, SPIMEM can have
+ * something like 4S-4D-4D, but SPI NOR can't. So, set all 4
+ * phases to either DTR or STR.
+ */
+ op->cmd.dtr = true;
+ op->addr.dtr = true;
+ op->dummy.dtr = true;
+ op->data.dtr = true;
+
+ /* 2 bytes per clock cycle in DTR mode. */
+ op->dummy.nbytes *= 2;
+
+ ext = spi_nor_get_cmd_ext(nor, op);
+ op->cmd.opcode = (op->cmd.opcode << 8) | ext;
+ op->cmd.nbytes = 2;
+ }
+}
+
/**
* spi_nor_spimem_bounce() - check if a bounce buffer is needed for the data
* transfer
@@ -85,17 +157,26 @@ static int spi_nor_spimem_exec_op(struct spi_nor *nor, struct spi_mem_op *op)
static int spi_nor_read_reg(struct spi_nor *nor, u8 opcode, u8 *buf,
size_t len)
{
+ if (spi_nor_protocol_is_dtr(nor->reg_proto))
+ return -EOPNOTSUPP;
+
return nor->controller_ops->read_reg(nor, opcode, buf, len);
}
static int spi_nor_write_reg(struct spi_nor *nor, u8 opcode, const u8 *buf,
size_t len)
{
+ if (spi_nor_protocol_is_dtr(nor->reg_proto))
+ return -EOPNOTSUPP;
+
return nor->controller_ops->write_reg(nor, opcode, buf, len);
}
static int spi_nor_controller_ops_erase(struct spi_nor *nor, loff_t offs)
{
+ if (spi_nor_protocol_is_dtr(nor->write_proto))
+ return -EOPNOTSUPP;
+
return nor->controller_ops->erase(nor, offs);
}
@@ -121,14 +202,12 @@ static ssize_t spi_nor_spimem_read_data(struct spi_nor *nor, loff_t from,
ssize_t nbytes;
int error;
- /* get transfer protocols. */
- op.cmd.buswidth = spi_nor_get_protocol_inst_nbits(nor->read_proto);
- op.addr.buswidth = spi_nor_get_protocol_addr_nbits(nor->read_proto);
- op.dummy.buswidth = op.addr.buswidth;
- op.data.buswidth = spi_nor_get_protocol_data_nbits(nor->read_proto);
+ spi_nor_spimem_setup_op(nor, &op, nor->read_proto);
/* convert the dummy cycles to the number of bytes */
op.dummy.nbytes = (nor->read_dummy * op.dummy.buswidth) / 8;
+ if (spi_nor_protocol_is_dtr(nor->read_proto))
+ op.dummy.nbytes *= 2;
usebouncebuf = spi_nor_spimem_bounce(nor, &op);
@@ -186,13 +265,11 @@ static ssize_t spi_nor_spimem_write_data(struct spi_nor *nor, loff_t to,
ssize_t nbytes;
int error;
- op.cmd.buswidth = spi_nor_get_protocol_inst_nbits(nor->write_proto);
- op.addr.buswidth = spi_nor_get_protocol_addr_nbits(nor->write_proto);
- op.data.buswidth = spi_nor_get_protocol_data_nbits(nor->write_proto);
-
if (nor->program_opcode == SPINOR_OP_AAI_WP && nor->sst_write_second)
op.addr.nbytes = 0;
+ spi_nor_spimem_setup_op(nor, &op, nor->write_proto);
+
if (spi_nor_spimem_bounce(nor, &op))
memcpy(nor->bouncebuf, buf, op.data.nbytes);
@@ -244,6 +321,8 @@ int spi_nor_write_enable(struct spi_nor *nor)
SPI_MEM_OP_NO_DUMMY,
SPI_MEM_OP_NO_DATA);
+ spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
+
ret = spi_mem_exec_op(nor->spimem, &op);
} else {
ret = spi_nor_write_reg(nor, SPINOR_OP_WREN, NULL, 0);
@@ -272,6 +351,8 @@ int spi_nor_write_disable(struct spi_nor *nor)
SPI_MEM_OP_NO_DUMMY,
SPI_MEM_OP_NO_DATA);
+ spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
+
ret = spi_mem_exec_op(nor->spimem, &op);
} else {
ret = spi_nor_write_reg(nor, SPINOR_OP_WRDI, NULL, 0);
@@ -302,6 +383,8 @@ static int spi_nor_read_sr(struct spi_nor *nor, u8 *sr)
SPI_MEM_OP_NO_DUMMY,
SPI_MEM_OP_DATA_IN(1, sr, 1));
+ spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
+
ret = spi_mem_exec_op(nor->spimem, &op);
} else {
ret = spi_nor_read_reg(nor, SPINOR_OP_RDSR, sr, 1);
@@ -332,6 +415,8 @@ static int spi_nor_read_fsr(struct spi_nor *nor, u8 *fsr)
SPI_MEM_OP_NO_DUMMY,
SPI_MEM_OP_DATA_IN(1, fsr, 1));
+ spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
+
ret = spi_mem_exec_op(nor->spimem, &op);
} else {
ret = spi_nor_read_reg(nor, SPINOR_OP_RDFSR, fsr, 1);
@@ -363,6 +448,8 @@ static int spi_nor_read_cr(struct spi_nor *nor, u8 *cr)
SPI_MEM_OP_NO_DUMMY,
SPI_MEM_OP_DATA_IN(1, cr, 1));
+ spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
+
ret = spi_mem_exec_op(nor->spimem, &op);
} else {
ret = spi_nor_read_reg(nor, SPINOR_OP_RDCR, cr, 1);
@@ -396,6 +483,8 @@ int spi_nor_set_4byte_addr_mode(struct spi_nor *nor, bool enable)
SPI_MEM_OP_NO_DUMMY,
SPI_MEM_OP_NO_DATA);
+ spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
+
ret = spi_mem_exec_op(nor->spimem, &op);
} else {
ret = spi_nor_write_reg(nor,
@@ -432,6 +521,8 @@ static int spansion_set_4byte_addr_mode(struct spi_nor *nor, bool enable)
SPI_MEM_OP_NO_DUMMY,
SPI_MEM_OP_DATA_OUT(1, nor->bouncebuf, 1));
+ spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
+
ret = spi_mem_exec_op(nor->spimem, &op);
} else {
ret = spi_nor_write_reg(nor, SPINOR_OP_BRWR, nor->bouncebuf, 1);
@@ -463,6 +554,8 @@ int spi_nor_write_ear(struct spi_nor *nor, u8 ear)
SPI_MEM_OP_NO_DUMMY,
SPI_MEM_OP_DATA_OUT(1, nor->bouncebuf, 1));
+ spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
+
ret = spi_mem_exec_op(nor->spimem, &op);
} else {
ret = spi_nor_write_reg(nor, SPINOR_OP_WREAR, nor->bouncebuf, 1);
@@ -493,6 +586,8 @@ int spi_nor_xread_sr(struct spi_nor *nor, u8 *sr)
SPI_MEM_OP_NO_DUMMY,
SPI_MEM_OP_DATA_IN(1, sr, 1));
+ spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
+
ret = spi_mem_exec_op(nor->spimem, &op);
} else {
ret = spi_nor_read_reg(nor, SPINOR_OP_XRDSR, sr, 1);
@@ -537,6 +632,8 @@ static void spi_nor_clear_sr(struct spi_nor *nor)
SPI_MEM_OP_NO_DUMMY,
SPI_MEM_OP_NO_DATA);
+ spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
+
ret = spi_mem_exec_op(nor->spimem, &op);
} else {
ret = spi_nor_write_reg(nor, SPINOR_OP_CLSR, NULL, 0);
@@ -600,6 +697,8 @@ static void spi_nor_clear_fsr(struct spi_nor *nor)
SPI_MEM_OP_NO_DUMMY,
SPI_MEM_OP_NO_DATA);
+ spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
+
ret = spi_mem_exec_op(nor->spimem, &op);
} else {
ret = spi_nor_write_reg(nor, SPINOR_OP_CLFSR, NULL, 0);
@@ -743,6 +842,8 @@ static int spi_nor_write_sr(struct spi_nor *nor, const u8 *sr, size_t len)
SPI_MEM_OP_NO_DUMMY,
SPI_MEM_OP_DATA_OUT(len, sr, 1));
+ spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
+
ret = spi_mem_exec_op(nor->spimem, &op);
} else {
ret = spi_nor_write_reg(nor, SPINOR_OP_WRSR, sr, len);
@@ -944,6 +1045,8 @@ static int spi_nor_write_sr2(struct spi_nor *nor, const u8 *sr2)
SPI_MEM_OP_NO_DUMMY,
SPI_MEM_OP_DATA_OUT(1, sr2, 1));
+ spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
+
ret = spi_mem_exec_op(nor->spimem, &op);
} else {
ret = spi_nor_write_reg(nor, SPINOR_OP_WRSR2, sr2, 1);
@@ -977,6 +1080,8 @@ static int spi_nor_read_sr2(struct spi_nor *nor, u8 *sr2)
SPI_MEM_OP_NO_DUMMY,
SPI_MEM_OP_DATA_IN(1, sr2, 1));
+ spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
+
ret = spi_mem_exec_op(nor->spimem, &op);
} else {
ret = spi_nor_read_reg(nor, SPINOR_OP_RDSR2, sr2, 1);
@@ -1007,6 +1112,8 @@ static int spi_nor_erase_chip(struct spi_nor *nor)
SPI_MEM_OP_NO_DUMMY,
SPI_MEM_OP_NO_DATA);
+ spi_nor_spimem_setup_op(nor, &op, nor->write_proto);
+
ret = spi_mem_exec_op(nor->spimem, &op);
} else {
ret = spi_nor_write_reg(nor, SPINOR_OP_CHIP_ERASE, NULL, 0);
@@ -1148,6 +1255,8 @@ static int spi_nor_erase_sector(struct spi_nor *nor, u32 addr)
SPI_MEM_OP_NO_DUMMY,
SPI_MEM_OP_NO_DATA);
+ spi_nor_spimem_setup_op(nor, &op, nor->write_proto);
+
return spi_mem_exec_op(nor->spimem, &op);
} else if (nor->controller_ops->erase) {
return spi_nor_controller_ops_erase(nor, addr);
@@ -2273,6 +2382,7 @@ int spi_nor_hwcaps_read2cmd(u32 hwcaps)
{ SNOR_HWCAPS_READ_1_8_8, SNOR_CMD_READ_1_8_8 },
{ SNOR_HWCAPS_READ_8_8_8, SNOR_CMD_READ_8_8_8 },
{ SNOR_HWCAPS_READ_1_8_8_DTR, SNOR_CMD_READ_1_8_8_DTR },
+ { SNOR_HWCAPS_READ_8_8_8_DTR, SNOR_CMD_READ_8_8_8_DTR },
};
return spi_nor_hwcaps2cmd(hwcaps, hwcaps_read2cmd,
@@ -2289,6 +2399,7 @@ static int spi_nor_hwcaps_pp2cmd(u32 hwcaps)
{ SNOR_HWCAPS_PP_1_1_8, SNOR_CMD_PP_1_1_8 },
{ SNOR_HWCAPS_PP_1_8_8, SNOR_CMD_PP_1_8_8 },
{ SNOR_HWCAPS_PP_8_8_8, SNOR_CMD_PP_8_8_8 },
+ { SNOR_HWCAPS_PP_8_8_8_DTR, SNOR_CMD_PP_8_8_8_DTR },
};
return spi_nor_hwcaps2cmd(hwcaps, hwcaps_pp2cmd,
@@ -2339,15 +2450,15 @@ static int spi_nor_spimem_check_readop(struct spi_nor *nor,
{
struct spi_mem_op op = SPI_MEM_OP(SPI_MEM_OP_CMD(read->opcode, 1),
SPI_MEM_OP_ADDR(3, 0, 1),
- SPI_MEM_OP_DUMMY(0, 1),
- SPI_MEM_OP_DATA_IN(0, NULL, 1));
+ SPI_MEM_OP_DUMMY(1, 1),
+ SPI_MEM_OP_DATA_IN(1, NULL, 1));
- op.cmd.buswidth = spi_nor_get_protocol_inst_nbits(read->proto);
- op.addr.buswidth = spi_nor_get_protocol_addr_nbits(read->proto);
- op.data.buswidth = spi_nor_get_protocol_data_nbits(read->proto);
- op.dummy.buswidth = op.addr.buswidth;
- op.dummy.nbytes = (read->num_mode_clocks + read->num_wait_states) *
- op.dummy.buswidth / 8;
+ spi_nor_spimem_setup_op(nor, &op, read->proto);
+
+ /* convert the dummy cycles to the number of bytes */
+ op.dummy.nbytes = (nor->read_dummy * op.dummy.buswidth) / 8;
+ if (spi_nor_protocol_is_dtr(nor->read_proto))
+ op.dummy.nbytes *= 2;
return spi_nor_spimem_check_op(nor, &op);
}
@@ -2366,11 +2477,9 @@ static int spi_nor_spimem_check_pp(struct spi_nor *nor,
struct spi_mem_op op = SPI_MEM_OP(SPI_MEM_OP_CMD(pp->opcode, 1),
SPI_MEM_OP_ADDR(3, 0, 1),
SPI_MEM_OP_NO_DUMMY,
- SPI_MEM_OP_DATA_OUT(0, NULL, 1));
+ SPI_MEM_OP_DATA_OUT(1, NULL, 1));
- op.cmd.buswidth = spi_nor_get_protocol_inst_nbits(pp->proto);
- op.addr.buswidth = spi_nor_get_protocol_addr_nbits(pp->proto);
- op.data.buswidth = spi_nor_get_protocol_data_nbits(pp->proto);
+ spi_nor_spimem_setup_op(nor, &op, pp->proto);
return spi_nor_spimem_check_op(nor, &op);
}
@@ -2388,12 +2497,16 @@ spi_nor_spimem_adjust_hwcaps(struct spi_nor *nor, u32 *hwcaps)
struct spi_nor_flash_parameter *params = nor->params;
unsigned int cap;
- /* DTR modes are not supported yet, mask them all. */
- *hwcaps &= ~SNOR_HWCAPS_DTR;
-
/* X-X-X modes are not supported yet, mask them all. */
*hwcaps &= ~SNOR_HWCAPS_X_X_X;
+ /*
+ * If the reset line is broken, we do not want to enter a stateful
+ * mode.
+ */
+ if (nor->flags & SNOR_F_BROKEN_RESET)
+ *hwcaps &= ~(SNOR_HWCAPS_X_X_X | SNOR_HWCAPS_X_X_X_DTR);
+
for (cap = 0; cap < sizeof(*hwcaps) * BITS_PER_BYTE; cap++) {
int rdidx, ppidx;
@@ -2648,7 +2761,7 @@ static int spi_nor_default_setup(struct spi_nor *nor,
* controller directly implements the spi_nor interface.
* Yet another reason to switch to spi-mem.
*/
- ignored_mask = SNOR_HWCAPS_X_X_X;
+ ignored_mask = SNOR_HWCAPS_X_X_X | SNOR_HWCAPS_X_X_X_DTR;
if (shared_mask & ignored_mask) {
dev_dbg(nor->dev,
"SPI n-n-n protocols are not supported.\n");
@@ -2794,11 +2907,28 @@ static void spi_nor_info_init_params(struct spi_nor *nor)
SNOR_PROTO_1_1_8);
}
+ if (info->flags & SPI_NOR_OCTAL_DTR_READ) {
+ params->hwcaps.mask |= SNOR_HWCAPS_READ_8_8_8_DTR;
+ spi_nor_set_read_settings(¶ms->reads[SNOR_CMD_READ_8_8_8_DTR],
+ 0, 20, SPINOR_OP_READ_FAST,
+ SNOR_PROTO_8_8_8_DTR);
+ }
+
/* Page Program settings. */
params->hwcaps.mask |= SNOR_HWCAPS_PP;
spi_nor_set_pp_settings(¶ms->page_programs[SNOR_CMD_PP],
SPINOR_OP_PP, SNOR_PROTO_1_1_1);
+ if (info->flags & SPI_NOR_OCTAL_DTR_PP) {
+ params->hwcaps.mask |= SNOR_HWCAPS_PP_8_8_8_DTR;
+ /*
+ * Since xSPI Page Program opcode is backward compatible with
+ * Legacy SPI, use Legacy SPI opcode there as well.
+ */
+ spi_nor_set_pp_settings(¶ms->page_programs[SNOR_CMD_PP_8_8_8_DTR],
+ SPINOR_OP_PP, SNOR_PROTO_8_8_8_DTR);
+ }
+
/*
* Sector Erase settings. Sort Erase Types in ascending order, with the
* smallest erase size starting at BIT(0).
@@ -2906,7 +3036,8 @@ static int spi_nor_init_params(struct spi_nor *nor)
spi_nor_manufacturer_init_params(nor);
- if ((nor->info->flags & (SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)) &&
+ if ((nor->info->flags & (SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
+ SPI_NOR_OCTAL_READ | SPI_NOR_OCTAL_DTR_READ)) &&
!(nor->info->flags & SPI_NOR_SKIP_SFDP))
spi_nor_sfdp_init_params(nor);
@@ -2969,7 +3100,9 @@ static int spi_nor_init(struct spi_nor *nor)
return err;
}
- if (nor->addr_width == 4 && !(nor->flags & SNOR_F_4B_OPCODES)) {
+ if (nor->addr_width == 4 &&
+ !spi_nor_protocol_is_dtr(nor->read_proto) &&
+ !(nor->flags & SNOR_F_4B_OPCODES)) {
/*
* If the RESET# pin isn't hooked up properly, or the system
* otherwise doesn't perform a reset command in the boot
@@ -3028,7 +3161,10 @@ static const struct flash_info *spi_nor_match_id(struct spi_nor *nor,
static int spi_nor_set_addr_width(struct spi_nor *nor)
{
- if (nor->addr_width) {
+ if (spi_nor_protocol_is_dtr(nor->read_proto)) {
+ /* Always use 4-byte addresses in DTR mode. */
+ nor->addr_width = 4;
+ } else if (nor->addr_width) {
/* already configured from SFDP */
} else if (nor->info->addr_width) {
nor->addr_width = nor->info->addr_width;
@@ -3267,14 +3403,19 @@ static int spi_nor_create_read_dirmap(struct spi_nor *nor)
};
struct spi_mem_op *op = &info.op_tmpl;
- /* get transfer protocols. */
- op->cmd.buswidth = spi_nor_get_protocol_inst_nbits(nor->read_proto);
- op->addr.buswidth = spi_nor_get_protocol_addr_nbits(nor->read_proto);
- op->dummy.buswidth = op->addr.buswidth;
- op->data.buswidth = spi_nor_get_protocol_data_nbits(nor->read_proto);
+ spi_nor_spimem_setup_op(nor, op, nor->read_proto);
/* convert the dummy cycles to the number of bytes */
op->dummy.nbytes = (nor->read_dummy * op->dummy.buswidth) / 8;
+ if (spi_nor_protocol_is_dtr(nor->read_proto))
+ op->dummy.nbytes *= 2;
+
+ /*
+ * Since spi_nor_spimem_setup_op() only sets buswidth when the number
+ * of data bytes is non-zero, the data buswidth won't be set here. So,
+ * do it explicitly.
+ */
+ op->data.buswidth = spi_nor_get_protocol_data_nbits(nor->read_proto);
nor->dirmap.rdesc = devm_spi_mem_dirmap_create(nor->dev, nor->spimem,
&info);
@@ -3293,15 +3434,18 @@ static int spi_nor_create_write_dirmap(struct spi_nor *nor)
};
struct spi_mem_op *op = &info.op_tmpl;
- /* get transfer protocols. */
- op->cmd.buswidth = spi_nor_get_protocol_inst_nbits(nor->write_proto);
- op->addr.buswidth = spi_nor_get_protocol_addr_nbits(nor->write_proto);
- op->dummy.buswidth = op->addr.buswidth;
- op->data.buswidth = spi_nor_get_protocol_data_nbits(nor->write_proto);
-
if (nor->program_opcode == SPINOR_OP_AAI_WP && nor->sst_write_second)
op->addr.nbytes = 0;
+ spi_nor_spimem_setup_op(nor, op, nor->write_proto);
+
+ /*
+ * Since spi_nor_spimem_setup_op() only sets buswidth when the number
+ * of data bytes is non-zero, the data buswidth won't be set here. So,
+ * do it explicitly.
+ */
+ op->data.buswidth = spi_nor_get_protocol_data_nbits(nor->write_proto);
+
nor->dirmap.wdesc = devm_spi_mem_dirmap_create(nor->dev, nor->spimem,
&info);
return PTR_ERR_OR_ZERO(nor->dirmap.wdesc);
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index 95aa32f3ceb1..125d27b0a72f 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -62,6 +62,7 @@ enum spi_nor_read_command_index {
SNOR_CMD_READ_1_8_8,
SNOR_CMD_READ_8_8_8,
SNOR_CMD_READ_1_8_8_DTR,
+ SNOR_CMD_READ_8_8_8_DTR,
SNOR_CMD_READ_MAX
};
@@ -78,6 +79,7 @@ enum spi_nor_pp_command_index {
SNOR_CMD_PP_1_1_8,
SNOR_CMD_PP_1_8_8,
SNOR_CMD_PP_8_8_8,
+ SNOR_CMD_PP_8_8_8_DTR,
SNOR_CMD_PP_MAX
};
@@ -311,6 +313,8 @@ struct flash_info {
* BP3 is bit 6 of status register.
* Must be used with SPI_NOR_4BIT_BP.
*/
+#define SPI_NOR_OCTAL_DTR_READ BIT(19) /* Flash supports octal DTR Read. */
+#define SPI_NOR_OCTAL_DTR_PP BIT(20) /* Flash supports Octal DTR Page Program */
/* Part specific fixup hooks. */
const struct spi_nor_fixups *fixups;
@@ -399,6 +403,9 @@ extern const struct spi_nor_manufacturer spi_nor_winbond;
extern const struct spi_nor_manufacturer spi_nor_xilinx;
extern const struct spi_nor_manufacturer spi_nor_xmc;
+void spi_nor_spimem_setup_op(const struct spi_nor *nor,
+ struct spi_mem_op *op,
+ const enum spi_nor_protocol proto);
int spi_nor_write_enable(struct spi_nor *nor);
int spi_nor_write_disable(struct spi_nor *nor);
int spi_nor_set_4byte_addr_mode(struct spi_nor *nor, bool enable);
diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
index e2a43d39eb5f..21fa9ab78eae 100644
--- a/drivers/mtd/spi-nor/sfdp.c
+++ b/drivers/mtd/spi-nor/sfdp.c
@@ -1047,9 +1047,16 @@ static int spi_nor_parse_4bait(struct spi_nor *nor,
}
/* 4BAIT is the only SFDP table that indicates page program support. */
- if (pp_hwcaps & SNOR_HWCAPS_PP)
+ if (pp_hwcaps & SNOR_HWCAPS_PP) {
spi_nor_set_pp_settings(¶ms_pp[SNOR_CMD_PP],
SPINOR_OP_PP_4B, SNOR_PROTO_1_1_1);
+ /*
+ * Since xSPI Page Program opcode is backward compatible with
+ * Legacy SPI, use Legacy SPI opcode there as well.
+ */
+ spi_nor_set_pp_settings(¶ms_pp[SNOR_CMD_PP_8_8_8_DTR],
+ SPINOR_OP_PP_4B, SNOR_PROTO_8_8_8_DTR);
+ }
if (pp_hwcaps & SNOR_HWCAPS_PP_1_1_4)
spi_nor_set_pp_settings(¶ms_pp[SNOR_CMD_PP_1_1_4],
SPINOR_OP_PP_1_1_4_4B,
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index 60bac2c0ec45..cd549042c53d 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -182,6 +182,7 @@ enum spi_nor_protocol {
SNOR_PROTO_1_2_2_DTR = SNOR_PROTO_DTR(1, 2, 2),
SNOR_PROTO_1_4_4_DTR = SNOR_PROTO_DTR(1, 4, 4),
SNOR_PROTO_1_8_8_DTR = SNOR_PROTO_DTR(1, 8, 8),
+ SNOR_PROTO_8_8_8_DTR = SNOR_PROTO_DTR(8, 8, 8),
};
static inline bool spi_nor_protocol_is_dtr(enum spi_nor_protocol proto)
@@ -228,7 +229,7 @@ struct spi_nor_hwcaps {
* then Quad SPI protocols before Dual SPI protocols, Fast Read and lastly
* (Slow) Read.
*/
-#define SNOR_HWCAPS_READ_MASK GENMASK(14, 0)
+#define SNOR_HWCAPS_READ_MASK GENMASK(15, 0)
#define SNOR_HWCAPS_READ BIT(0)
#define SNOR_HWCAPS_READ_FAST BIT(1)
#define SNOR_HWCAPS_READ_1_1_1_DTR BIT(2)
@@ -245,11 +246,12 @@ struct spi_nor_hwcaps {
#define SNOR_HWCAPS_READ_4_4_4 BIT(9)
#define SNOR_HWCAPS_READ_1_4_4_DTR BIT(10)
-#define SNOR_HWCAPS_READ_OCTAL GENMASK(14, 11)
+#define SNOR_HWCAPS_READ_OCTAL GENMASK(15, 11)
#define SNOR_HWCAPS_READ_1_1_8 BIT(11)
#define SNOR_HWCAPS_READ_1_8_8 BIT(12)
#define SNOR_HWCAPS_READ_8_8_8 BIT(13)
#define SNOR_HWCAPS_READ_1_8_8_DTR BIT(14)
+#define SNOR_HWCAPS_READ_8_8_8_DTR BIT(15)
/*
* Page Program capabilities.
@@ -260,18 +262,19 @@ struct spi_nor_hwcaps {
* JEDEC/SFDP standard to define them. Also at this moment no SPI flash memory
* implements such commands.
*/
-#define SNOR_HWCAPS_PP_MASK GENMASK(22, 16)
-#define SNOR_HWCAPS_PP BIT(16)
+#define SNOR_HWCAPS_PP_MASK GENMASK(23, 16)
+#define SNOR_HWCAPS_PP BIT(16)
-#define SNOR_HWCAPS_PP_QUAD GENMASK(19, 17)
-#define SNOR_HWCAPS_PP_1_1_4 BIT(17)
-#define SNOR_HWCAPS_PP_1_4_4 BIT(18)
-#define SNOR_HWCAPS_PP_4_4_4 BIT(19)
+#define SNOR_HWCAPS_PP_QUAD GENMASK(19, 17)
+#define SNOR_HWCAPS_PP_1_1_4 BIT(17)
+#define SNOR_HWCAPS_PP_1_4_4 BIT(18)
+#define SNOR_HWCAPS_PP_4_4_4 BIT(19)
-#define SNOR_HWCAPS_PP_OCTAL GENMASK(22, 20)
-#define SNOR_HWCAPS_PP_1_1_8 BIT(20)
-#define SNOR_HWCAPS_PP_1_8_8 BIT(21)
-#define SNOR_HWCAPS_PP_8_8_8 BIT(22)
+#define SNOR_HWCAPS_PP_OCTAL GENMASK(23, 20)
+#define SNOR_HWCAPS_PP_1_1_8 BIT(20)
+#define SNOR_HWCAPS_PP_1_8_8 BIT(21)
+#define SNOR_HWCAPS_PP_8_8_8 BIT(22)
+#define SNOR_HWCAPS_PP_8_8_8_DTR BIT(23)
#define SNOR_HWCAPS_X_X_X (SNOR_HWCAPS_READ_2_2_2 | \
SNOR_HWCAPS_READ_4_4_4 | \
@@ -279,10 +282,14 @@ struct spi_nor_hwcaps {
SNOR_HWCAPS_PP_4_4_4 | \
SNOR_HWCAPS_PP_8_8_8)
+#define SNOR_HWCAPS_X_X_X_DTR (SNOR_HWCAPS_READ_8_8_8_DTR | \
+ SNOR_HWCAPS_PP_8_8_8_DTR)
+
#define SNOR_HWCAPS_DTR (SNOR_HWCAPS_READ_1_1_1_DTR | \
SNOR_HWCAPS_READ_1_2_2_DTR | \
SNOR_HWCAPS_READ_1_4_4_DTR | \
- SNOR_HWCAPS_READ_1_8_8_DTR)
+ SNOR_HWCAPS_READ_1_8_8_DTR | \
+ SNOR_HWCAPS_READ_8_8_8_DTR)
#define SNOR_HWCAPS_ALL (SNOR_HWCAPS_READ_MASK | \
SNOR_HWCAPS_PP_MASK)
@@ -318,6 +325,22 @@ struct spi_nor_controller_ops {
int (*erase)(struct spi_nor *nor, loff_t offs);
};
+/**
+ * enum spi_nor_cmd_ext - describes the command opcode extension in DTR mode
+ * @SPI_NOR_EXT_NONE: no extension. This is the default, and is used in Legacy
+ * SPI mode
+ * @SPI_NOR_EXT_REPEAT: the extension is same as the opcode
+ * @SPI_NOR_EXT_INVERT: the extension is the bitwise inverse of the opcode
+ * @SPI_NOR_EXT_HEX: the extension is any hex value. The command and opcode
+ * combine to form a 16-bit opcode.
+ */
+enum spi_nor_cmd_ext {
+ SPI_NOR_EXT_NONE = 0,
+ SPI_NOR_EXT_REPEAT,
+ SPI_NOR_EXT_INVERT,
+ SPI_NOR_EXT_HEX,
+};
+
/*
* Forward declarations that are used internally by the core and manufacturer
* drivers.
@@ -345,6 +368,7 @@ struct spi_nor_flash_parameter;
* @program_opcode: the program opcode
* @sst_write_second: used by the SST write operation
* @flags: flag options for the current SPI NOR (SNOR_F_*)
+ * @cmd_ext_type: the command opcode extension type for DTR mode.
* @read_proto: the SPI protocol for read operations
* @write_proto: the SPI protocol for write operations
* @reg_proto: the SPI protocol for read_reg/write_reg/erase operations
@@ -376,6 +400,7 @@ struct spi_nor {
enum spi_nor_protocol reg_proto;
bool sst_write_second;
u32 flags;
+ enum spi_nor_cmd_ext cmd_ext_type;
const struct spi_nor_controller_ops *controller_ops;
--
2.28.0
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [PATCH v13 05/15] mtd: spi-nor: sfdp: get command opcode extension type from BFPT
2020-09-16 12:44 [PATCH v13 00/15] mtd: spi-nor: add xSPI Octal DTR support Pratyush Yadav
` (3 preceding siblings ...)
2020-09-16 12:44 ` [PATCH v13 04/15] mtd: spi-nor: add support for DTR protocol Pratyush Yadav
@ 2020-09-16 12:44 ` Pratyush Yadav
2020-09-30 6:17 ` Tudor.Ambarus
2020-09-16 12:44 ` [PATCH v13 06/15] mtd: spi-nor: sfdp: parse xSPI Profile 1.0 table Pratyush Yadav
` (11 subsequent siblings)
16 siblings, 1 reply; 49+ messages in thread
From: Pratyush Yadav @ 2020-09-16 12:44 UTC (permalink / raw)
To: Tudor Ambarus, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, linux-mtd, linux-kernel
Cc: Boris Brezillon, Sekhar Nori, Pratyush Yadav
Some devices in DTR mode expect an extra command byte called the
extension. The extension can either be same as the opcode, bitwise
inverse of the opcode, or another additional byte forming a 16-byte
opcode. Get the extension type from the BFPT. For now, only flashes with
"repeat" and "inverse" extensions are supported.
Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
---
drivers/mtd/spi-nor/sfdp.c | 18 ++++++++++++++++++
drivers/mtd/spi-nor/sfdp.h | 6 ++++++
2 files changed, 24 insertions(+)
diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
index 21fa9ab78eae..c77655968f80 100644
--- a/drivers/mtd/spi-nor/sfdp.c
+++ b/drivers/mtd/spi-nor/sfdp.c
@@ -606,6 +606,24 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
if (bfpt_header->length == BFPT_DWORD_MAX_JESD216B)
return spi_nor_post_bfpt_fixups(nor, bfpt_header, &bfpt,
params);
+ /* 8D-8D-8D command extension. */
+ switch (bfpt.dwords[BFPT_DWORD(18)] & BFPT_DWORD18_CMD_EXT_MASK) {
+ case BFPT_DWORD18_CMD_EXT_REP:
+ nor->cmd_ext_type = SPI_NOR_EXT_REPEAT;
+ break;
+
+ case BFPT_DWORD18_CMD_EXT_INV:
+ nor->cmd_ext_type = SPI_NOR_EXT_INVERT;
+ break;
+
+ case BFPT_DWORD18_CMD_EXT_RES:
+ dev_dbg(nor->dev, "Reserved command extension used\n");
+ break;
+
+ case BFPT_DWORD18_CMD_EXT_16B:
+ dev_dbg(nor->dev, "16-bit opcodes not supported\n");
+ return -EOPNOTSUPP;
+ }
return spi_nor_post_bfpt_fixups(nor, bfpt_header, &bfpt, params);
}
diff --git a/drivers/mtd/spi-nor/sfdp.h b/drivers/mtd/spi-nor/sfdp.h
index 7f9846b3a1ad..6d7243067252 100644
--- a/drivers/mtd/spi-nor/sfdp.h
+++ b/drivers/mtd/spi-nor/sfdp.h
@@ -90,6 +90,12 @@ struct sfdp_bfpt {
#define BFPT_DWORD15_QER_SR2_BIT1_NO_RD (0x4UL << 20)
#define BFPT_DWORD15_QER_SR2_BIT1 (0x5UL << 20) /* Spansion */
+#define BFPT_DWORD18_CMD_EXT_MASK GENMASK(30, 29)
+#define BFPT_DWORD18_CMD_EXT_REP (0x0UL << 29) /* Repeat */
+#define BFPT_DWORD18_CMD_EXT_INV (0x1UL << 29) /* Invert */
+#define BFPT_DWORD18_CMD_EXT_RES (0x2UL << 29) /* Reserved */
+#define BFPT_DWORD18_CMD_EXT_16B (0x3UL << 29) /* 16-bit opcode */
+
struct sfdp_parameter_header {
u8 id_lsb;
u8 minor;
--
2.28.0
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 49+ messages in thread
* Re: [PATCH v13 05/15] mtd: spi-nor: sfdp: get command opcode extension type from BFPT
2020-09-16 12:44 ` [PATCH v13 05/15] mtd: spi-nor: sfdp: get command opcode extension type from BFPT Pratyush Yadav
@ 2020-09-30 6:17 ` Tudor.Ambarus
0 siblings, 0 replies; 49+ messages in thread
From: Tudor.Ambarus @ 2020-09-30 6:17 UTC (permalink / raw)
To: p.yadav, miquel.raynal, richard, vigneshr, linux-mtd, linux-kernel
Cc: boris.brezillon, nsekhar
On 9/16/20 3:44 PM, Pratyush Yadav wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> Some devices in DTR mode expect an extra command byte called the
> extension. The extension can either be same as the opcode, bitwise
> inverse of the opcode, or another additional byte forming a 16-byte
> opcode. Get the extension type from the BFPT. For now, only flashes with
> "repeat" and "inverse" extensions are supported.
>
> Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> ---
> drivers/mtd/spi-nor/sfdp.c | 18 ++++++++++++++++++
> drivers/mtd/spi-nor/sfdp.h | 6 ++++++
> 2 files changed, 24 insertions(+)
>
> diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
> index 21fa9ab78eae..c77655968f80 100644
> --- a/drivers/mtd/spi-nor/sfdp.c
> +++ b/drivers/mtd/spi-nor/sfdp.c
> @@ -606,6 +606,24 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
> if (bfpt_header->length == BFPT_DWORD_MAX_JESD216B)
> return spi_nor_post_bfpt_fixups(nor, bfpt_header, &bfpt,
> params);
> + /* 8D-8D-8D command extension. */
> + switch (bfpt.dwords[BFPT_DWORD(18)] & BFPT_DWORD18_CMD_EXT_MASK) {
> + case BFPT_DWORD18_CMD_EXT_REP:
> + nor->cmd_ext_type = SPI_NOR_EXT_REPEAT;
> + break;
> +
> + case BFPT_DWORD18_CMD_EXT_INV:
> + nor->cmd_ext_type = SPI_NOR_EXT_INVERT;
> + break;
> +
> + case BFPT_DWORD18_CMD_EXT_RES:
> + dev_dbg(nor->dev, "Reserved command extension used\n");
> + break;
> +
> + case BFPT_DWORD18_CMD_EXT_16B:
> + dev_dbg(nor->dev, "16-bit opcodes not supported\n");
> + return -EOPNOTSUPP;
> + }
>
> return spi_nor_post_bfpt_fixups(nor, bfpt_header, &bfpt, params);
> }
> diff --git a/drivers/mtd/spi-nor/sfdp.h b/drivers/mtd/spi-nor/sfdp.h
> index 7f9846b3a1ad..6d7243067252 100644
> --- a/drivers/mtd/spi-nor/sfdp.h
> +++ b/drivers/mtd/spi-nor/sfdp.h
> @@ -90,6 +90,12 @@ struct sfdp_bfpt {
> #define BFPT_DWORD15_QER_SR2_BIT1_NO_RD (0x4UL << 20)
> #define BFPT_DWORD15_QER_SR2_BIT1 (0x5UL << 20) /* Spansion */
>
> +#define BFPT_DWORD18_CMD_EXT_MASK GENMASK(30, 29)
> +#define BFPT_DWORD18_CMD_EXT_REP (0x0UL << 29) /* Repeat */
> +#define BFPT_DWORD18_CMD_EXT_INV (0x1UL << 29) /* Invert */
> +#define BFPT_DWORD18_CMD_EXT_RES (0x2UL << 29) /* Reserved */
> +#define BFPT_DWORD18_CMD_EXT_16B (0x3UL << 29) /* 16-bit opcode */
> +
> struct sfdp_parameter_header {
> u8 id_lsb;
> u8 minor;
> --
> 2.28.0
>
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH v13 06/15] mtd: spi-nor: sfdp: parse xSPI Profile 1.0 table
2020-09-16 12:44 [PATCH v13 00/15] mtd: spi-nor: add xSPI Octal DTR support Pratyush Yadav
` (4 preceding siblings ...)
2020-09-16 12:44 ` [PATCH v13 05/15] mtd: spi-nor: sfdp: get command opcode extension type from BFPT Pratyush Yadav
@ 2020-09-16 12:44 ` Pratyush Yadav
2020-09-30 6:44 ` Tudor.Ambarus
2020-09-16 12:44 ` [PATCH v13 07/15] mtd: spi-nor: core: use dummy cycle and address width info from SFDP Pratyush Yadav
` (10 subsequent siblings)
16 siblings, 1 reply; 49+ messages in thread
From: Pratyush Yadav @ 2020-09-16 12:44 UTC (permalink / raw)
To: Tudor Ambarus, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, linux-mtd, linux-kernel
Cc: Boris Brezillon, Sekhar Nori, Pratyush Yadav
This table is indication that the flash is xSPI compliant and hence
supports octal DTR mode. Extract information like the fast read opcode,
dummy cycles, the number of dummy cycles needed for a Read Status
Register command, and the number of address bytes needed for a Read
Status Register command.
We don't know what speed the controller is running at. Find the fast
read dummy cycles for the fastest frequency the flash can run at to be
sure we are never short of dummy cycles. If nothing is available,
default to 20. Flashes that use a different value should update it in
their fixup hooks.
Since we want to set read settings, expose spi_nor_set_read_settings()
in core.h.
Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
---
drivers/mtd/spi-nor/core.c | 2 +-
drivers/mtd/spi-nor/core.h | 10 +++++
drivers/mtd/spi-nor/sfdp.c | 91 ++++++++++++++++++++++++++++++++++++++
3 files changed, 102 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 7445d7122304..cbb1aab27d03 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -2333,7 +2333,7 @@ static int spi_nor_check(struct spi_nor *nor)
return 0;
}
-static void
+void
spi_nor_set_read_settings(struct spi_nor_read_command *read,
u8 num_mode_clocks,
u8 num_wait_states,
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index 125d27b0a72f..42ec7692d8e7 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -192,6 +192,9 @@ struct spi_nor_locking_ops {
*
* @size: the flash memory density in bytes.
* @page_size: the page size of the SPI NOR flash memory.
+ * @rdsr_dummy: dummy cycles needed for Read Status Register command.
+ * @rdsr_addr_nbytes: dummy address bytes needed for Read Status Register
+ * command.
* @hwcaps: describes the read and page program hardware
* capabilities.
* @reads: read capabilities ordered by priority: the higher index
@@ -214,6 +217,8 @@ struct spi_nor_locking_ops {
struct spi_nor_flash_parameter {
u64 size;
u32 page_size;
+ u8 rdsr_dummy;
+ u8 rdsr_addr_nbytes;
struct spi_nor_hwcaps hwcaps;
struct spi_nor_read_command reads[SNOR_CMD_READ_MAX];
@@ -425,6 +430,11 @@ ssize_t spi_nor_write_data(struct spi_nor *nor, loff_t to, size_t len,
int spi_nor_hwcaps_read2cmd(u32 hwcaps);
u8 spi_nor_convert_3to4_read(u8 opcode);
+void spi_nor_set_read_settings(struct spi_nor_read_command *read,
+ u8 num_mode_clocks,
+ u8 num_wait_states,
+ u8 opcode,
+ enum spi_nor_protocol proto);
void spi_nor_set_pp_settings(struct spi_nor_pp_command *pp, u8 opcode,
enum spi_nor_protocol proto);
diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
index c77655968f80..cadb1ed27ffe 100644
--- a/drivers/mtd/spi-nor/sfdp.c
+++ b/drivers/mtd/spi-nor/sfdp.c
@@ -4,6 +4,7 @@
* Copyright (C) 2014, Freescale Semiconductor, Inc.
*/
+#include <linux/bitfield.h>
#include <linux/slab.h>
#include <linux/sort.h>
#include <linux/mtd/spi-nor.h>
@@ -19,6 +20,7 @@
#define SFDP_BFPT_ID 0xff00 /* Basic Flash Parameter Table */
#define SFDP_SECTOR_MAP_ID 0xff81 /* Sector Map Table */
#define SFDP_4BAIT_ID 0xff84 /* 4-byte Address Instruction Table */
+#define SFDP_PROFILE1_ID 0xff05 /* xSPI Profile 1.0 table. */
#define SFDP_SIGNATURE 0x50444653U
@@ -1108,6 +1110,91 @@ static int spi_nor_parse_4bait(struct spi_nor *nor,
return ret;
}
+#define PROFILE1_DWORD1_RDSR_ADDR_BYTES BIT(29)
+#define PROFILE1_DWORD1_RDSR_DUMMY BIT(28)
+#define PROFILE1_DWORD1_RD_FAST_CMD GENMASK(15, 8)
+#define PROFILE1_DWORD4_DUMMY_200MHZ GENMASK(11, 7)
+#define PROFILE1_DWORD5_DUMMY_166MHZ GENMASK(31, 27)
+#define PROFILE1_DWORD5_DUMMY_133MHZ GENMASK(21, 17)
+#define PROFILE1_DWORD5_DUMMY_100MHZ GENMASK(11, 7)
+#define PROFILE1_DUMMY_DEFAULT 20
+
+/**
+ * spi_nor_parse_profile1() - parse the xSPI Profile 1.0 table
+ * @nor: pointer to a 'struct spi_nor'
+ * @profile1_header: pointer to the 'struct sfdp_parameter_header' describing
+ * the 4-Byte Address Instruction Table length and version.
+ * @params: pointer to the 'struct spi_nor_flash_parameter' to be.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int spi_nor_parse_profile1(struct spi_nor *nor,
+ const struct sfdp_parameter_header *profile1_header,
+ struct spi_nor_flash_parameter *params)
+{
+ u32 *dwords, addr;
+ size_t len;
+ int ret;
+ u8 dummy, opcode;
+
+ len = profile1_header->length * sizeof(*dwords);
+ dwords = kmalloc(len, GFP_KERNEL);
+ if (!dwords)
+ return -ENOMEM;
+
+ addr = SFDP_PARAM_HEADER_PTP(profile1_header);
+ ret = spi_nor_read_sfdp(nor, addr, len, dwords);
+ if (ret)
+ goto out;
+
+ le32_to_cpu_array(dwords, profile1_header->length);
+
+ /* Get 8D-8D-8D fast read opcode and dummy cycles. */
+ opcode = FIELD_GET(PROFILE1_DWORD1_RD_FAST_CMD, dwords[0]);
+
+ /* Set the Read Status Register dummy cycles and dummy address bytes. */
+ if (dwords[0] & PROFILE1_DWORD1_RDSR_DUMMY)
+ params->rdsr_dummy = 8;
+ else
+ params->rdsr_dummy = 4;
+
+ if (dwords[0] & PROFILE1_DWORD1_RDSR_ADDR_BYTES)
+ params->rdsr_addr_nbytes = 4;
+ else
+ params->rdsr_addr_nbytes = 0;
+
+ /*
+ * We don't know what speed the controller is running at. Find the
+ * dummy cycles for the fastest frequency the flash can run at to be
+ * sure we are never short of dummy cycles. A value of 0 means the
+ * frequency is not supported.
+ *
+ * Default to PROFILE1_DUMMY_DEFAULT if we don't find anything, and let
+ * flashes set the correct value if needed in their fixup hooks.
+ */
+ dummy = FIELD_GET(PROFILE1_DWORD4_DUMMY_200MHZ, dwords[3]);
+ if (!dummy)
+ dummy = FIELD_GET(PROFILE1_DWORD5_DUMMY_166MHZ, dwords[4]);
+ if (!dummy)
+ dummy = FIELD_GET(PROFILE1_DWORD5_DUMMY_133MHZ, dwords[4]);
+ if (!dummy)
+ dummy = FIELD_GET(PROFILE1_DWORD5_DUMMY_100MHZ, dwords[4]);
+ if (!dummy)
+ dummy = PROFILE1_DUMMY_DEFAULT;
+
+ /* Round up to an even value to avoid tripping controllers up. */
+ dummy = round_up(dummy, 2);
+
+ /* Update the fast read settings. */
+ spi_nor_set_read_settings(¶ms->reads[SNOR_CMD_READ_8_8_8_DTR],
+ 0, dummy, opcode,
+ SNOR_PROTO_8_8_8_DTR);
+
+out:
+ kfree(dwords);
+ return ret;
+}
+
/**
* spi_nor_parse_sfdp() - parse the Serial Flash Discoverable Parameters.
* @nor: pointer to a 'struct spi_nor'
@@ -1209,6 +1296,10 @@ int spi_nor_parse_sfdp(struct spi_nor *nor,
err = spi_nor_parse_4bait(nor, param_header, params);
break;
+ case SFDP_PROFILE1_ID:
+ err = spi_nor_parse_profile1(nor, param_header, params);
+ break;
+
default:
break;
}
--
2.28.0
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 49+ messages in thread
* Re: [PATCH v13 06/15] mtd: spi-nor: sfdp: parse xSPI Profile 1.0 table
2020-09-16 12:44 ` [PATCH v13 06/15] mtd: spi-nor: sfdp: parse xSPI Profile 1.0 table Pratyush Yadav
@ 2020-09-30 6:44 ` Tudor.Ambarus
2020-09-30 6:53 ` Pratyush Yadav
0 siblings, 1 reply; 49+ messages in thread
From: Tudor.Ambarus @ 2020-09-30 6:44 UTC (permalink / raw)
To: p.yadav, miquel.raynal, richard, vigneshr, linux-mtd, linux-kernel
Cc: boris.brezillon, nsekhar
On 9/16/20 3:44 PM, Pratyush Yadav wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> This table is indication that the flash is xSPI compliant and hence
> supports octal DTR mode. Extract information like the fast read opcode,
> dummy cycles, the number of dummy cycles needed for a Read Status
> Register command, and the number of address bytes needed for a Read
> Status Register command.
>
> We don't know what speed the controller is running at. Find the fast
> read dummy cycles for the fastest frequency the flash can run at to be
> sure we are never short of dummy cycles. If nothing is available,
> default to 20. Flashes that use a different value should update it in
> their fixup hooks.
>
> Since we want to set read settings, expose spi_nor_set_read_settings()
> in core.h.
>
> Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
> ---
> drivers/mtd/spi-nor/core.c | 2 +-
> drivers/mtd/spi-nor/core.h | 10 +++++
> drivers/mtd/spi-nor/sfdp.c | 91 ++++++++++++++++++++++++++++++++++++++
> 3 files changed, 102 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index 7445d7122304..cbb1aab27d03 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -2333,7 +2333,7 @@ static int spi_nor_check(struct spi_nor *nor)
> return 0;
> }
>
> -static void
> +void
> spi_nor_set_read_settings(struct spi_nor_read_command *read,
> u8 num_mode_clocks,
> u8 num_wait_states,
> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
> index 125d27b0a72f..42ec7692d8e7 100644
> --- a/drivers/mtd/spi-nor/core.h
> +++ b/drivers/mtd/spi-nor/core.h
> @@ -192,6 +192,9 @@ struct spi_nor_locking_ops {
> *
> * @size: the flash memory density in bytes.
> * @page_size: the page size of the SPI NOR flash memory.
> + * @rdsr_dummy: dummy cycles needed for Read Status Register command.
> + * @rdsr_addr_nbytes: dummy address bytes needed for Read Status Register
> + * command.
> * @hwcaps: describes the read and page program hardware
> * capabilities.
> * @reads: read capabilities ordered by priority: the higher index
> @@ -214,6 +217,8 @@ struct spi_nor_locking_ops {
> struct spi_nor_flash_parameter {
> u64 size;
> u32 page_size;
> + u8 rdsr_dummy;
> + u8 rdsr_addr_nbytes;
>
> struct spi_nor_hwcaps hwcaps;
> struct spi_nor_read_command reads[SNOR_CMD_READ_MAX];
> @@ -425,6 +430,11 @@ ssize_t spi_nor_write_data(struct spi_nor *nor, loff_t to, size_t len,
>
> int spi_nor_hwcaps_read2cmd(u32 hwcaps);
> u8 spi_nor_convert_3to4_read(u8 opcode);
> +void spi_nor_set_read_settings(struct spi_nor_read_command *read,
> + u8 num_mode_clocks,
> + u8 num_wait_states,
> + u8 opcode,
> + enum spi_nor_protocol proto);
> void spi_nor_set_pp_settings(struct spi_nor_pp_command *pp, u8 opcode,
> enum spi_nor_protocol proto);
>
> diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
> index c77655968f80..cadb1ed27ffe 100644
> --- a/drivers/mtd/spi-nor/sfdp.c
> +++ b/drivers/mtd/spi-nor/sfdp.c
> @@ -4,6 +4,7 @@
> * Copyright (C) 2014, Freescale Semiconductor, Inc.
> */
>
> +#include <linux/bitfield.h>
> #include <linux/slab.h>
> #include <linux/sort.h>
> #include <linux/mtd/spi-nor.h>
> @@ -19,6 +20,7 @@
> #define SFDP_BFPT_ID 0xff00 /* Basic Flash Parameter Table */
> #define SFDP_SECTOR_MAP_ID 0xff81 /* Sector Map Table */
> #define SFDP_4BAIT_ID 0xff84 /* 4-byte Address Instruction Table */
> +#define SFDP_PROFILE1_ID 0xff05 /* xSPI Profile 1.0 table. */
>
> #define SFDP_SIGNATURE 0x50444653U
>
> @@ -1108,6 +1110,91 @@ static int spi_nor_parse_4bait(struct spi_nor *nor,
> return ret;
> }
>
> +#define PROFILE1_DWORD1_RDSR_ADDR_BYTES BIT(29)
> +#define PROFILE1_DWORD1_RDSR_DUMMY BIT(28)
> +#define PROFILE1_DWORD1_RD_FAST_CMD GENMASK(15, 8)
> +#define PROFILE1_DWORD4_DUMMY_200MHZ GENMASK(11, 7)
> +#define PROFILE1_DWORD5_DUMMY_166MHZ GENMASK(31, 27)
> +#define PROFILE1_DWORD5_DUMMY_133MHZ GENMASK(21, 17)
> +#define PROFILE1_DWORD5_DUMMY_100MHZ GENMASK(11, 7)
> +#define PROFILE1_DUMMY_DEFAULT 20
> +
> +/**
> + * spi_nor_parse_profile1() - parse the xSPI Profile 1.0 table
> + * @nor: pointer to a 'struct spi_nor'
> + * @profile1_header: pointer to the 'struct sfdp_parameter_header' describing
> + * the 4-Byte Address Instruction Table length and version.
Profile 1.0 Table
> + * @params: pointer to the 'struct spi_nor_flash_parameter' to be.
> + *
> + * Return: 0 on success, -errno otherwise.
> + */
> +static int spi_nor_parse_profile1(struct spi_nor *nor,
> + const struct sfdp_parameter_header *profile1_header,
> + struct spi_nor_flash_parameter *params)
> +{
> + u32 *dwords, addr;
> + size_t len;
> + int ret;
> + u8 dummy, opcode;
> +
> + len = profile1_header->length * sizeof(*dwords);
> + dwords = kmalloc(len, GFP_KERNEL);
> + if (!dwords)
> + return -ENOMEM;
> +
> + addr = SFDP_PARAM_HEADER_PTP(profile1_header);
> + ret = spi_nor_read_sfdp(nor, addr, len, dwords);
> + if (ret)
> + goto out;
> +
> + le32_to_cpu_array(dwords, profile1_header->length);
> +
> + /* Get 8D-8D-8D fast read opcode and dummy cycles. */
> + opcode = FIELD_GET(PROFILE1_DWORD1_RD_FAST_CMD, dwords[0]);
> +
> + /* Set the Read Status Register dummy cycles and dummy address bytes. */
> + if (dwords[0] & PROFILE1_DWORD1_RDSR_DUMMY)
> + params->rdsr_dummy = 8;
> + else
> + params->rdsr_dummy = 4;
> +
> + if (dwords[0] & PROFILE1_DWORD1_RDSR_ADDR_BYTES)
> + params->rdsr_addr_nbytes = 4;
> + else
> + params->rdsr_addr_nbytes = 0;
> +
> + /*
> + * We don't know what speed the controller is running at. Find the
> + * dummy cycles for the fastest frequency the flash can run at to be
> + * sure we are never short of dummy cycles. A value of 0 means the
> + * frequency is not supported.
> + *
> + * Default to PROFILE1_DUMMY_DEFAULT if we don't find anything, and let
> + * flashes set the correct value if needed in their fixup hooks.
> + */
> + dummy = FIELD_GET(PROFILE1_DWORD4_DUMMY_200MHZ, dwords[3]);
> + if (!dummy)
> + dummy = FIELD_GET(PROFILE1_DWORD5_DUMMY_166MHZ, dwords[4]);
> + if (!dummy)
> + dummy = FIELD_GET(PROFILE1_DWORD5_DUMMY_133MHZ, dwords[4]);
> + if (!dummy)
> + dummy = FIELD_GET(PROFILE1_DWORD5_DUMMY_100MHZ, dwords[4]);
> + if (!dummy)
> + dummy = PROFILE1_DUMMY_DEFAULT;
just a dev_dbg here, without assuming what default value means
> +
> + /* Round up to an even value to avoid tripping controllers up. */
> + dummy = round_up(dummy, 2);
> +
> + /* Update the fast read settings. */
> + spi_nor_set_read_settings(¶ms->reads[SNOR_CMD_READ_8_8_8_DTR],
> + 0, dummy, opcode,
> + SNOR_PROTO_8_8_8_DTR);
> +
> +out:
> + kfree(dwords);
> + return ret;
> +}
> +
> /**
> * spi_nor_parse_sfdp() - parse the Serial Flash Discoverable Parameters.
> * @nor: pointer to a 'struct spi_nor'
> @@ -1209,6 +1296,10 @@ int spi_nor_parse_sfdp(struct spi_nor *nor,
> err = spi_nor_parse_4bait(nor, param_header, params);
> break;
>
> + case SFDP_PROFILE1_ID:
> + err = spi_nor_parse_profile1(nor, param_header, params);
> + break;
> +
> default:
> break;
> }
> --
> 2.28.0
>
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v13 06/15] mtd: spi-nor: sfdp: parse xSPI Profile 1.0 table
2020-09-30 6:44 ` Tudor.Ambarus
@ 2020-09-30 6:53 ` Pratyush Yadav
0 siblings, 0 replies; 49+ messages in thread
From: Pratyush Yadav @ 2020-09-30 6:53 UTC (permalink / raw)
To: Tudor.Ambarus
Cc: vigneshr, richard, nsekhar, linux-kernel, boris.brezillon,
linux-mtd, miquel.raynal
On 30/09/20 06:44AM, Tudor.Ambarus@microchip.com wrote:
> On 9/16/20 3:44 PM, Pratyush Yadav wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >
> > This table is indication that the flash is xSPI compliant and hence
> > supports octal DTR mode. Extract information like the fast read opcode,
> > dummy cycles, the number of dummy cycles needed for a Read Status
> > Register command, and the number of address bytes needed for a Read
> > Status Register command.
> >
> > We don't know what speed the controller is running at. Find the fast
> > read dummy cycles for the fastest frequency the flash can run at to be
> > sure we are never short of dummy cycles. If nothing is available,
> > default to 20. Flashes that use a different value should update it in
> > their fixup hooks.
> >
> > Since we want to set read settings, expose spi_nor_set_read_settings()
> > in core.h.
> >
> > Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
> > ---
> > drivers/mtd/spi-nor/core.c | 2 +-
> > drivers/mtd/spi-nor/core.h | 10 +++++
> > drivers/mtd/spi-nor/sfdp.c | 91 ++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 102 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> > index 7445d7122304..cbb1aab27d03 100644
> > --- a/drivers/mtd/spi-nor/core.c
> > +++ b/drivers/mtd/spi-nor/core.c
...
> > @@ -1108,6 +1110,91 @@ static int spi_nor_parse_4bait(struct spi_nor
> > *nor,
> > return ret;
> > }
> >
> > +#define PROFILE1_DWORD1_RDSR_ADDR_BYTES BIT(29)
> > +#define PROFILE1_DWORD1_RDSR_DUMMY BIT(28)
> > +#define PROFILE1_DWORD1_RD_FAST_CMD GENMASK(15, 8)
> > +#define PROFILE1_DWORD4_DUMMY_200MHZ GENMASK(11, 7)
> > +#define PROFILE1_DWORD5_DUMMY_166MHZ GENMASK(31, 27)
> > +#define PROFILE1_DWORD5_DUMMY_133MHZ GENMASK(21, 17)
> > +#define PROFILE1_DWORD5_DUMMY_100MHZ GENMASK(11, 7)
> > +#define PROFILE1_DUMMY_DEFAULT 20
> > +
> > +/**
> > + * spi_nor_parse_profile1() - parse the xSPI Profile 1.0 table
> > + * @nor: pointer to a 'struct spi_nor'
> > + * @profile1_header: pointer to the 'struct sfdp_parameter_header' describing
> > + * the 4-Byte Address Instruction Table length and version.
>
> Profile 1.0 Table
Oops! Will fix.
> > + * @params: pointer to the 'struct spi_nor_flash_parameter' to be.
> > + *
> > + * Return: 0 on success, -errno otherwise.
> > + */
...
> > + /*
> > + * We don't know what speed the controller is running at. Find the
> > + * dummy cycles for the fastest frequency the flash can run at to be
> > + * sure we are never short of dummy cycles. A value of 0 means the
> > + * frequency is not supported.
> > + *
> > + * Default to PROFILE1_DUMMY_DEFAULT if we don't find anything, and let
> > + * flashes set the correct value if needed in their fixup hooks.
> > + */
> > + dummy = FIELD_GET(PROFILE1_DWORD4_DUMMY_200MHZ, dwords[3]);
> > + if (!dummy)
> > + dummy = FIELD_GET(PROFILE1_DWORD5_DUMMY_166MHZ, dwords[4]);
> > + if (!dummy)
> > + dummy = FIELD_GET(PROFILE1_DWORD5_DUMMY_133MHZ, dwords[4]);
> > + if (!dummy)
> > + dummy = FIELD_GET(PROFILE1_DWORD5_DUMMY_100MHZ, dwords[4]);
> > + if (!dummy)
> > + dummy = PROFILE1_DUMMY_DEFAULT;
>
> just a dev_dbg here, without assuming what default value means
And we leave dummy to 0, correct?
--
Regards,
Pratyush Yadav
Texas Instruments India
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH v13 07/15] mtd: spi-nor: core: use dummy cycle and address width info from SFDP
2020-09-16 12:44 [PATCH v13 00/15] mtd: spi-nor: add xSPI Octal DTR support Pratyush Yadav
` (5 preceding siblings ...)
2020-09-16 12:44 ` [PATCH v13 06/15] mtd: spi-nor: sfdp: parse xSPI Profile 1.0 table Pratyush Yadav
@ 2020-09-16 12:44 ` Pratyush Yadav
2020-09-30 6:46 ` Tudor.Ambarus
2020-09-16 12:44 ` [PATCH v13 08/15] mtd: spi-nor: core: do 2 byte reads for SR and FSR in DTR mode Pratyush Yadav
` (9 subsequent siblings)
16 siblings, 1 reply; 49+ messages in thread
From: Pratyush Yadav @ 2020-09-16 12:44 UTC (permalink / raw)
To: Tudor Ambarus, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, linux-mtd, linux-kernel
Cc: Boris Brezillon, Sekhar Nori, Pratyush Yadav
The xSPI Profile 1.0 table specifies how many dummy cycles and address
bytes are needed for the Read Status Register command in octal DTR mode.
Use that information to send the correct Read SR command.
Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
---
drivers/mtd/spi-nor/core.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index cbb1aab27d03..88c9e18067f4 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -383,6 +383,11 @@ static int spi_nor_read_sr(struct spi_nor *nor, u8 *sr)
SPI_MEM_OP_NO_DUMMY,
SPI_MEM_OP_DATA_IN(1, sr, 1));
+ if (spi_nor_protocol_is_dtr(nor->reg_proto)) {
+ op.addr.nbytes = nor->params->rdsr_addr_nbytes;
+ op.dummy.nbytes = nor->params->rdsr_dummy;
+ }
+
spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
ret = spi_mem_exec_op(nor->spimem, &op);
@@ -415,6 +420,11 @@ static int spi_nor_read_fsr(struct spi_nor *nor, u8 *fsr)
SPI_MEM_OP_NO_DUMMY,
SPI_MEM_OP_DATA_IN(1, fsr, 1));
+ if (spi_nor_protocol_is_dtr(nor->reg_proto)) {
+ op.addr.nbytes = nor->params->rdsr_addr_nbytes;
+ op.dummy.nbytes = nor->params->rdsr_dummy;
+ }
+
spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
ret = spi_mem_exec_op(nor->spimem, &op);
--
2.28.0
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 49+ messages in thread
* Re: [PATCH v13 07/15] mtd: spi-nor: core: use dummy cycle and address width info from SFDP
2020-09-16 12:44 ` [PATCH v13 07/15] mtd: spi-nor: core: use dummy cycle and address width info from SFDP Pratyush Yadav
@ 2020-09-30 6:46 ` Tudor.Ambarus
0 siblings, 0 replies; 49+ messages in thread
From: Tudor.Ambarus @ 2020-09-30 6:46 UTC (permalink / raw)
To: p.yadav, miquel.raynal, richard, vigneshr, linux-mtd, linux-kernel
Cc: boris.brezillon, nsekhar
On 9/16/20 3:44 PM, Pratyush Yadav wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> The xSPI Profile 1.0 table specifies how many dummy cycles and address
> bytes are needed for the Read Status Register command in octal DTR mode.
> Use that information to send the correct Read SR command.
>
> Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
> ---
> drivers/mtd/spi-nor/core.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index cbb1aab27d03..88c9e18067f4 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -383,6 +383,11 @@ static int spi_nor_read_sr(struct spi_nor *nor, u8 *sr)
> SPI_MEM_OP_NO_DUMMY,
> SPI_MEM_OP_DATA_IN(1, sr, 1));
>
> + if (spi_nor_protocol_is_dtr(nor->reg_proto)) {
this should be done just for octal dtr
> + op.addr.nbytes = nor->params->rdsr_addr_nbytes;
> + op.dummy.nbytes = nor->params->rdsr_dummy;
> + }
> +
> spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
>
> ret = spi_mem_exec_op(nor->spimem, &op);
> @@ -415,6 +420,11 @@ static int spi_nor_read_fsr(struct spi_nor *nor, u8 *fsr)
> SPI_MEM_OP_NO_DUMMY,
> SPI_MEM_OP_DATA_IN(1, fsr, 1));
>
> + if (spi_nor_protocol_is_dtr(nor->reg_proto)) {
same here
> + op.addr.nbytes = nor->params->rdsr_addr_nbytes;
> + op.dummy.nbytes = nor->params->rdsr_dummy;
> + }
> +
> spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
>
> ret = spi_mem_exec_op(nor->spimem, &op);
> --
> 2.28.0
>
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH v13 08/15] mtd: spi-nor: core: do 2 byte reads for SR and FSR in DTR mode
2020-09-16 12:44 [PATCH v13 00/15] mtd: spi-nor: add xSPI Octal DTR support Pratyush Yadav
` (6 preceding siblings ...)
2020-09-16 12:44 ` [PATCH v13 07/15] mtd: spi-nor: core: use dummy cycle and address width info from SFDP Pratyush Yadav
@ 2020-09-16 12:44 ` Pratyush Yadav
2020-09-30 6:50 ` Tudor.Ambarus
2020-09-16 12:44 ` [PATCH v13 09/15] mtd: spi-nor: core: enable octal DTR mode when possible Pratyush Yadav
` (8 subsequent siblings)
16 siblings, 1 reply; 49+ messages in thread
From: Pratyush Yadav @ 2020-09-16 12:44 UTC (permalink / raw)
To: Tudor Ambarus, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, linux-mtd, linux-kernel
Cc: Boris Brezillon, Sekhar Nori, Pratyush Yadav
Some controllers, like the cadence qspi controller, have trouble reading
only 1 byte in DTR mode. So, do 2 byte reads for SR and FSR commands in
DTR mode, and then discard the second byte.
Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
---
drivers/mtd/spi-nor/core.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 88c9e18067f4..87c568debf14 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -368,7 +368,7 @@ int spi_nor_write_disable(struct spi_nor *nor)
* spi_nor_read_sr() - Read the Status Register.
* @nor: pointer to 'struct spi_nor'.
* @sr: pointer to a DMA-able buffer where the value of the
- * Status Register will be written.
+ * Status Register will be written. Should be at least 2 bytes.
*
* Return: 0 on success, -errno otherwise.
*/
@@ -386,6 +386,11 @@ static int spi_nor_read_sr(struct spi_nor *nor, u8 *sr)
if (spi_nor_protocol_is_dtr(nor->reg_proto)) {
op.addr.nbytes = nor->params->rdsr_addr_nbytes;
op.dummy.nbytes = nor->params->rdsr_dummy;
+ /*
+ * We don't want to read only one byte in DTR mode. So,
+ * read 2 and then discard the second byte.
+ */
+ op.data.nbytes = 2;
}
spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
@@ -405,7 +410,8 @@ static int spi_nor_read_sr(struct spi_nor *nor, u8 *sr)
* spi_nor_read_fsr() - Read the Flag Status Register.
* @nor: pointer to 'struct spi_nor'
* @fsr: pointer to a DMA-able buffer where the value of the
- * Flag Status Register will be written.
+ * Flag Status Register will be written. Should be at least 2
+ * bytes.
*
* Return: 0 on success, -errno otherwise.
*/
@@ -423,6 +429,11 @@ static int spi_nor_read_fsr(struct spi_nor *nor, u8 *fsr)
if (spi_nor_protocol_is_dtr(nor->reg_proto)) {
op.addr.nbytes = nor->params->rdsr_addr_nbytes;
op.dummy.nbytes = nor->params->rdsr_dummy;
+ /*
+ * We don't want to read only one byte in DTR mode. So,
+ * read 2 and then discard the second byte.
+ */
+ op.data.nbytes = 2;
}
spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
--
2.28.0
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 49+ messages in thread
* Re: [PATCH v13 08/15] mtd: spi-nor: core: do 2 byte reads for SR and FSR in DTR mode
2020-09-16 12:44 ` [PATCH v13 08/15] mtd: spi-nor: core: do 2 byte reads for SR and FSR in DTR mode Pratyush Yadav
@ 2020-09-30 6:50 ` Tudor.Ambarus
2020-09-30 6:55 ` Pratyush Yadav
0 siblings, 1 reply; 49+ messages in thread
From: Tudor.Ambarus @ 2020-09-30 6:50 UTC (permalink / raw)
To: p.yadav, miquel.raynal, richard, vigneshr, linux-mtd, linux-kernel
Cc: boris.brezillon, nsekhar
On 9/16/20 3:44 PM, Pratyush Yadav wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> Some controllers, like the cadence qspi controller, have trouble reading
> only 1 byte in DTR mode. So, do 2 byte reads for SR and FSR commands in
did you get garbage when reading only one byte?
> DTR mode, and then discard the second byte.
>
> Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
> ---
> drivers/mtd/spi-nor/core.c | 15 +++++++++++++--
> 1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index 88c9e18067f4..87c568debf14 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -368,7 +368,7 @@ int spi_nor_write_disable(struct spi_nor *nor)
> * spi_nor_read_sr() - Read the Status Register.
> * @nor: pointer to 'struct spi_nor'.
> * @sr: pointer to a DMA-able buffer where the value of the
> - * Status Register will be written.
> + * Status Register will be written. Should be at least 2 bytes.
> *
> * Return: 0 on success, -errno otherwise.
> */
> @@ -386,6 +386,11 @@ static int spi_nor_read_sr(struct spi_nor *nor, u8 *sr)
> if (spi_nor_protocol_is_dtr(nor->reg_proto)) {
> op.addr.nbytes = nor->params->rdsr_addr_nbytes;
> op.dummy.nbytes = nor->params->rdsr_dummy;
> + /*
> + * We don't want to read only one byte in DTR mode. So,
> + * read 2 and then discard the second byte.
> + */
> + op.data.nbytes = 2;
just for octal dtr, but should be fine if you update the previous patch
> }
>
> spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
> @@ -405,7 +410,8 @@ static int spi_nor_read_sr(struct spi_nor *nor, u8 *sr)
> * spi_nor_read_fsr() - Read the Flag Status Register.
> * @nor: pointer to 'struct spi_nor'
> * @fsr: pointer to a DMA-able buffer where the value of the
> - * Flag Status Register will be written.
> + * Flag Status Register will be written. Should be at least 2
> + * bytes.
> *
> * Return: 0 on success, -errno otherwise.
> */
> @@ -423,6 +429,11 @@ static int spi_nor_read_fsr(struct spi_nor *nor, u8 *fsr)
> if (spi_nor_protocol_is_dtr(nor->reg_proto)) {
> op.addr.nbytes = nor->params->rdsr_addr_nbytes;
> op.dummy.nbytes = nor->params->rdsr_dummy;
> + /*
> + * We don't want to read only one byte in DTR mode. So,
> + * read 2 and then discard the second byte.
> + */
> + op.data.nbytes = 2;
> }
>
> spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
> --
> 2.28.0
>
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v13 08/15] mtd: spi-nor: core: do 2 byte reads for SR and FSR in DTR mode
2020-09-30 6:50 ` Tudor.Ambarus
@ 2020-09-30 6:55 ` Pratyush Yadav
0 siblings, 0 replies; 49+ messages in thread
From: Pratyush Yadav @ 2020-09-30 6:55 UTC (permalink / raw)
To: Tudor.Ambarus
Cc: vigneshr, richard, nsekhar, linux-kernel, boris.brezillon,
linux-mtd, miquel.raynal
On 30/09/20 06:50AM, Tudor.Ambarus@microchip.com wrote:
> On 9/16/20 3:44 PM, Pratyush Yadav wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >
> > Some controllers, like the cadence qspi controller, have trouble reading
> > only 1 byte in DTR mode. So, do 2 byte reads for SR and FSR commands in
>
> did you get garbage when reading only one byte?
Yes.
> > DTR mode, and then discard the second byte.
> >
> > Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
> > ---
> > drivers/mtd/spi-nor/core.c | 15 +++++++++++++--
> > 1 file changed, 13 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> > index 88c9e18067f4..87c568debf14 100644
> > --- a/drivers/mtd/spi-nor/core.c
> > +++ b/drivers/mtd/spi-nor/core.c
> > @@ -368,7 +368,7 @@ int spi_nor_write_disable(struct spi_nor *nor)
> > * spi_nor_read_sr() - Read the Status Register.
> > * @nor: pointer to 'struct spi_nor'.
> > * @sr: pointer to a DMA-able buffer where the value of the
> > - * Status Register will be written.
> > + * Status Register will be written. Should be at least 2 bytes.
> > *
> > * Return: 0 on success, -errno otherwise.
> > */
> > @@ -386,6 +386,11 @@ static int spi_nor_read_sr(struct spi_nor *nor, u8 *sr)
> > if (spi_nor_protocol_is_dtr(nor->reg_proto)) {
> > op.addr.nbytes = nor->params->rdsr_addr_nbytes;
> > op.dummy.nbytes = nor->params->rdsr_dummy;
> > + /*
> > + * We don't want to read only one byte in DTR mode. So,
> > + * read 2 and then discard the second byte.
> > + */
> > + op.data.nbytes = 2;
>
> just for octal dtr, but should be fine if you update the previous patch
Ok.
--
Regards,
Pratyush Yadav
Texas Instruments India
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH v13 09/15] mtd: spi-nor: core: enable octal DTR mode when possible
2020-09-16 12:44 [PATCH v13 00/15] mtd: spi-nor: add xSPI Octal DTR support Pratyush Yadav
` (7 preceding siblings ...)
2020-09-16 12:44 ` [PATCH v13 08/15] mtd: spi-nor: core: do 2 byte reads for SR and FSR in DTR mode Pratyush Yadav
@ 2020-09-16 12:44 ` Pratyush Yadav
2020-09-29 11:26 ` Tudor.Ambarus
2020-09-30 7:11 ` Tudor.Ambarus
2020-09-16 12:44 ` [PATCH v13 10/15] mtd: spi-nor: sfdp: detect Soft Reset sequence support from BFPT Pratyush Yadav
` (7 subsequent siblings)
16 siblings, 2 replies; 49+ messages in thread
From: Pratyush Yadav @ 2020-09-16 12:44 UTC (permalink / raw)
To: Tudor Ambarus, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, linux-mtd, linux-kernel
Cc: Boris Brezillon, Sekhar Nori, Pratyush Yadav
Allow flashes to specify a hook to enable octal DTR mode. Use this hook
whenever possible to get optimal transfer speeds.
Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
---
drivers/mtd/spi-nor/core.c | 35 +++++++++++++++++++++++++++++++++++
drivers/mtd/spi-nor/core.h | 2 ++
2 files changed, 37 insertions(+)
diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 87c568debf14..6ee93544d72f 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -3069,6 +3069,35 @@ static int spi_nor_init_params(struct spi_nor *nor)
return 0;
}
+/** spi_nor_octal_dtr_enable() - enable Octal DTR I/O if needed
+ * @nor: pointer to a 'struct spi_nor'
+ * @enable: whether to enable or disable Octal DTR
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int spi_nor_octal_dtr_enable(struct spi_nor *nor, bool enable)
+{
+ int ret;
+
+ if (!nor->params->octal_dtr_enable)
+ return 0;
+
+ if (!(nor->read_proto == SNOR_PROTO_8_8_8_DTR &&
+ nor->write_proto == SNOR_PROTO_8_8_8_DTR))
+ return 0;
+
+ ret = nor->params->octal_dtr_enable(nor, enable);
+ if (ret)
+ return ret;
+
+ if (enable)
+ nor->reg_proto = SNOR_PROTO_8_8_8_DTR;
+ else
+ nor->reg_proto = SNOR_PROTO_1_1_1;
+
+ return 0;
+}
+
/**
* spi_nor_quad_enable() - enable/disable Quad I/O if needed.
* @nor: pointer to a 'struct spi_nor'
@@ -3109,6 +3138,12 @@ static int spi_nor_init(struct spi_nor *nor)
{
int err;
+ err = spi_nor_octal_dtr_enable(nor, true);
+ if (err) {
+ dev_dbg(nor->dev, "octal mode not supported\n");
+ return err;
+ }
+
err = spi_nor_quad_enable(nor, true);
if (err) {
dev_dbg(nor->dev, "quad mode not supported\n");
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index 42ec7692d8e7..fcb5f071ebed 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -203,6 +203,7 @@ struct spi_nor_locking_ops {
* higher index in the array, the higher priority.
* @erase_map: the erase map parsed from the SFDP Sector Map Parameter
* Table.
+ * @octal_dtr_enable: enables SPI NOR octal DTR mode.
* @quad_enable: enables/disables SPI NOR Quad mode.
* @set_4byte_addr_mode: puts the SPI NOR in 4 byte addressing mode.
* @convert_addr: converts an absolute address into something the flash
@@ -226,6 +227,7 @@ struct spi_nor_flash_parameter {
struct spi_nor_erase_map erase_map;
+ int (*octal_dtr_enable)(struct spi_nor *nor, bool enable);
int (*quad_enable)(struct spi_nor *nor, bool enable);
int (*set_4byte_addr_mode)(struct spi_nor *nor, bool enable);
u32 (*convert_addr)(struct spi_nor *nor, u32 addr);
--
2.28.0
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 49+ messages in thread
* Re: [PATCH v13 09/15] mtd: spi-nor: core: enable octal DTR mode when possible
2020-09-16 12:44 ` [PATCH v13 09/15] mtd: spi-nor: core: enable octal DTR mode when possible Pratyush Yadav
@ 2020-09-29 11:26 ` Tudor.Ambarus
2020-09-29 12:51 ` Pratyush Yadav
2020-09-30 7:11 ` Tudor.Ambarus
1 sibling, 1 reply; 49+ messages in thread
From: Tudor.Ambarus @ 2020-09-29 11:26 UTC (permalink / raw)
To: p.yadav, miquel.raynal, richard, vigneshr, linux-mtd, linux-kernel
Cc: boris.brezillon, nsekhar
Hi,
On 9/16/20 3:44 PM, Pratyush Yadav wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> Allow flashes to specify a hook to enable octal DTR mode. Use this hook
> whenever possible to get optimal transfer speeds.
>
> Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
> ---
> drivers/mtd/spi-nor/core.c | 35 +++++++++++++++++++++++++++++++++++
> drivers/mtd/spi-nor/core.h | 2 ++
> 2 files changed, 37 insertions(+)
>
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index 87c568debf14..6ee93544d72f 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -3069,6 +3069,35 @@ static int spi_nor_init_params(struct spi_nor *nor)
> return 0;
> }
>
> +/** spi_nor_octal_dtr_enable() - enable Octal DTR I/O if needed
> + * @nor: pointer to a 'struct spi_nor'
> + * @enable: whether to enable or disable Octal DTR
> + *
> + * Return: 0 on success, -errno otherwise.
> + */
> +static int spi_nor_octal_dtr_enable(struct spi_nor *nor, bool enable)
> +{
> + int ret;
> +
> + if (!nor->params->octal_dtr_enable)
> + return 0;
> +
> + if (!(nor->read_proto == SNOR_PROTO_8_8_8_DTR &&
> + nor->write_proto == SNOR_PROTO_8_8_8_DTR))
> + return 0;
> +
> + ret = nor->params->octal_dtr_enable(nor, enable);
> + if (ret)
> + return ret;
> +
> + if (enable)
> + nor->reg_proto = SNOR_PROTO_8_8_8_DTR;
> + else
> + nor->reg_proto = SNOR_PROTO_1_1_1;
> +
> + return 0;
> +}
> +
> /**
> * spi_nor_quad_enable() - enable/disable Quad I/O if needed.
> * @nor: pointer to a 'struct spi_nor'
> @@ -3109,6 +3138,12 @@ static int spi_nor_init(struct spi_nor *nor)
> {
> int err;
>
> + err = spi_nor_octal_dtr_enable(nor, true);
> + if (err) {
> + dev_dbg(nor->dev, "octal mode not supported\n");
> + return err;
> + }
> +
> err = spi_nor_quad_enable(nor, true);
Is it possible to enable octal dtr and quad at the same time?
Maybe an 'if/else if' here depending on the values of nor->read_proto and
nor->write_proto
Cheers,
ta
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v13 09/15] mtd: spi-nor: core: enable octal DTR mode when possible
2020-09-29 11:26 ` Tudor.Ambarus
@ 2020-09-29 12:51 ` Pratyush Yadav
2020-09-29 13:05 ` Tudor.Ambarus
0 siblings, 1 reply; 49+ messages in thread
From: Pratyush Yadav @ 2020-09-29 12:51 UTC (permalink / raw)
To: Tudor.Ambarus
Cc: vigneshr, richard, nsekhar, linux-kernel, boris.brezillon,
linux-mtd, miquel.raynal
On 29/09/20 11:26AM, Tudor.Ambarus@microchip.com wrote:
> Hi,
>
> On 9/16/20 3:44 PM, Pratyush Yadav wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >
> > Allow flashes to specify a hook to enable octal DTR mode. Use this hook
> > whenever possible to get optimal transfer speeds.
> >
> > Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
> > ---
> > drivers/mtd/spi-nor/core.c | 35 +++++++++++++++++++++++++++++++++++
> > drivers/mtd/spi-nor/core.h | 2 ++
> > 2 files changed, 37 insertions(+)
> >
> > diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> > index 87c568debf14..6ee93544d72f 100644
> > --- a/drivers/mtd/spi-nor/core.c
> > +++ b/drivers/mtd/spi-nor/core.c
> > @@ -3069,6 +3069,35 @@ static int spi_nor_init_params(struct spi_nor *nor)
> > return 0;
> > }
> >
> > +/** spi_nor_octal_dtr_enable() - enable Octal DTR I/O if needed
> > + * @nor: pointer to a 'struct spi_nor'
> > + * @enable: whether to enable or disable Octal DTR
> > + *
> > + * Return: 0 on success, -errno otherwise.
> > + */
> > +static int spi_nor_octal_dtr_enable(struct spi_nor *nor, bool enable)
> > +{
> > + int ret;
> > +
> > + if (!nor->params->octal_dtr_enable)
> > + return 0;
> > +
> > + if (!(nor->read_proto == SNOR_PROTO_8_8_8_DTR &&
> > + nor->write_proto == SNOR_PROTO_8_8_8_DTR))
> > + return 0;
> > +
> > + ret = nor->params->octal_dtr_enable(nor, enable);
> > + if (ret)
> > + return ret;
> > +
> > + if (enable)
> > + nor->reg_proto = SNOR_PROTO_8_8_8_DTR;
> > + else
> > + nor->reg_proto = SNOR_PROTO_1_1_1;
> > +
> > + return 0;
> > +}
> > +
> > /**
> > * spi_nor_quad_enable() - enable/disable Quad I/O if needed.
> > * @nor: pointer to a 'struct spi_nor'
> > @@ -3109,6 +3138,12 @@ static int spi_nor_init(struct spi_nor *nor)
> > {
> > int err;
> >
> > + err = spi_nor_octal_dtr_enable(nor, true);
> > + if (err) {
> > + dev_dbg(nor->dev, "octal mode not supported\n");
> > + return err;
> > + }
> > +
> > err = spi_nor_quad_enable(nor, true);
>
> Is it possible to enable octal dtr and quad at the same time?
> Maybe an 'if/else if' here depending on the values of nor->read_proto and
> nor->write_proto
No it is not. If you look inside spi_nor_octal_dtr_enable() and
spi_nor_quad_enable(), they both are a no-op if the protocol does not
match. spi_nor_quad_enable() was already doing it this way so I made
spi_nor_octal_dtr_enable() follow suit. So this is effectively an
if-else on the value of nor->read_proto. I don't think an explicit one
is needed.
--
Regards,
Pratyush Yadav
Texas Instruments India
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v13 09/15] mtd: spi-nor: core: enable octal DTR mode when possible
2020-09-29 12:51 ` Pratyush Yadav
@ 2020-09-29 13:05 ` Tudor.Ambarus
0 siblings, 0 replies; 49+ messages in thread
From: Tudor.Ambarus @ 2020-09-29 13:05 UTC (permalink / raw)
To: p.yadav
Cc: vigneshr, richard, nsekhar, linux-kernel, boris.brezillon,
linux-mtd, miquel.raynal
On 9/29/20 3:51 PM, Pratyush Yadav wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> On 29/09/20 11:26AM, Tudor.Ambarus@microchip.com wrote:
>> Hi,
>>
>> On 9/16/20 3:44 PM, Pratyush Yadav wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>
>>> Allow flashes to specify a hook to enable octal DTR mode. Use this hook
>>> whenever possible to get optimal transfer speeds.
>>>
>>> Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
>>> ---
>>> drivers/mtd/spi-nor/core.c | 35 +++++++++++++++++++++++++++++++++++
>>> drivers/mtd/spi-nor/core.h | 2 ++
>>> 2 files changed, 37 insertions(+)
>>>
>>> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
>>> index 87c568debf14..6ee93544d72f 100644
>>> --- a/drivers/mtd/spi-nor/core.c
>>> +++ b/drivers/mtd/spi-nor/core.c
>>> @@ -3069,6 +3069,35 @@ static int spi_nor_init_params(struct spi_nor *nor)
>>> return 0;
>>> }
>>>
>>> +/** spi_nor_octal_dtr_enable() - enable Octal DTR I/O if needed
>>> + * @nor: pointer to a 'struct spi_nor'
>>> + * @enable: whether to enable or disable Octal DTR
>>> + *
>>> + * Return: 0 on success, -errno otherwise.
>>> + */
>>> +static int spi_nor_octal_dtr_enable(struct spi_nor *nor, bool enable)
>>> +{
>>> + int ret;
>>> +
>>> + if (!nor->params->octal_dtr_enable)
>>> + return 0;
>>> +
>>> + if (!(nor->read_proto == SNOR_PROTO_8_8_8_DTR &&
>>> + nor->write_proto == SNOR_PROTO_8_8_8_DTR))
>>> + return 0;
>>> +
>>> + ret = nor->params->octal_dtr_enable(nor, enable);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + if (enable)
>>> + nor->reg_proto = SNOR_PROTO_8_8_8_DTR;
>>> + else
>>> + nor->reg_proto = SNOR_PROTO_1_1_1;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> /**
>>> * spi_nor_quad_enable() - enable/disable Quad I/O if needed.
>>> * @nor: pointer to a 'struct spi_nor'
>>> @@ -3109,6 +3138,12 @@ static int spi_nor_init(struct spi_nor *nor)
>>> {
>>> int err;
>>>
>>> + err = spi_nor_octal_dtr_enable(nor, true);
>>> + if (err) {
>>> + dev_dbg(nor->dev, "octal mode not supported\n");
>>> + return err;
>>> + }
>>> +
>>> err = spi_nor_quad_enable(nor, true);
>>
>> Is it possible to enable octal dtr and quad at the same time?
>> Maybe an 'if/else if' here depending on the values of nor->read_proto and
>> nor->write_proto
>
> No it is not. If you look inside spi_nor_octal_dtr_enable() and
> spi_nor_quad_enable(), they both are a no-op if the protocol does not
> match. spi_nor_quad_enable() was already doing it this way so I made
> spi_nor_octal_dtr_enable() follow suit. So this is effectively an
> if-else on the value of nor->read_proto. I don't think an explicit one
> is needed.
you're right! thanks
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v13 09/15] mtd: spi-nor: core: enable octal DTR mode when possible
2020-09-16 12:44 ` [PATCH v13 09/15] mtd: spi-nor: core: enable octal DTR mode when possible Pratyush Yadav
2020-09-29 11:26 ` Tudor.Ambarus
@ 2020-09-30 7:11 ` Tudor.Ambarus
1 sibling, 0 replies; 49+ messages in thread
From: Tudor.Ambarus @ 2020-09-30 7:11 UTC (permalink / raw)
To: p.yadav, miquel.raynal, richard, vigneshr, linux-mtd, linux-kernel
Cc: boris.brezillon, nsekhar
On 9/16/20 3:44 PM, Pratyush Yadav wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> Allow flashes to specify a hook to enable octal DTR mode. Use this hook
> whenever possible to get optimal transfer speeds.
We need to restrict the access to octal dtr enable for flashes that enter in
8-8-8 dtr mode via non-volatile bits. It's what I tried in RFC 1/3.
Looks good ;)
>
> Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
> ---
> drivers/mtd/spi-nor/core.c | 35 +++++++++++++++++++++++++++++++++++
> drivers/mtd/spi-nor/core.h | 2 ++
> 2 files changed, 37 insertions(+)
>
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index 87c568debf14..6ee93544d72f 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -3069,6 +3069,35 @@ static int spi_nor_init_params(struct spi_nor *nor)
> return 0;
> }
>
> +/** spi_nor_octal_dtr_enable() - enable Octal DTR I/O if needed
> + * @nor: pointer to a 'struct spi_nor'
> + * @enable: whether to enable or disable Octal DTR
> + *
> + * Return: 0 on success, -errno otherwise.
> + */
> +static int spi_nor_octal_dtr_enable(struct spi_nor *nor, bool enable)
> +{
> + int ret;
> +
> + if (!nor->params->octal_dtr_enable)
> + return 0;
> +
> + if (!(nor->read_proto == SNOR_PROTO_8_8_8_DTR &&
> + nor->write_proto == SNOR_PROTO_8_8_8_DTR))
> + return 0;
> +
> + ret = nor->params->octal_dtr_enable(nor, enable);
> + if (ret)
> + return ret;
> +
> + if (enable)
> + nor->reg_proto = SNOR_PROTO_8_8_8_DTR;
> + else
> + nor->reg_proto = SNOR_PROTO_1_1_1;
> +
> + return 0;
> +}
> +
> /**
> * spi_nor_quad_enable() - enable/disable Quad I/O if needed.
> * @nor: pointer to a 'struct spi_nor'
> @@ -3109,6 +3138,12 @@ static int spi_nor_init(struct spi_nor *nor)
> {
> int err;
>
> + err = spi_nor_octal_dtr_enable(nor, true);
> + if (err) {
> + dev_dbg(nor->dev, "octal mode not supported\n");
> + return err;
> + }
> +
> err = spi_nor_quad_enable(nor, true);
> if (err) {
> dev_dbg(nor->dev, "quad mode not supported\n");
> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
> index 42ec7692d8e7..fcb5f071ebed 100644
> --- a/drivers/mtd/spi-nor/core.h
> +++ b/drivers/mtd/spi-nor/core.h
> @@ -203,6 +203,7 @@ struct spi_nor_locking_ops {
> * higher index in the array, the higher priority.
> * @erase_map: the erase map parsed from the SFDP Sector Map Parameter
> * Table.
> + * @octal_dtr_enable: enables SPI NOR octal DTR mode.
> * @quad_enable: enables/disables SPI NOR Quad mode.
> * @set_4byte_addr_mode: puts the SPI NOR in 4 byte addressing mode.
> * @convert_addr: converts an absolute address into something the flash
> @@ -226,6 +227,7 @@ struct spi_nor_flash_parameter {
>
> struct spi_nor_erase_map erase_map;
>
> + int (*octal_dtr_enable)(struct spi_nor *nor, bool enable);
> int (*quad_enable)(struct spi_nor *nor, bool enable);
> int (*set_4byte_addr_mode)(struct spi_nor *nor, bool enable);
> u32 (*convert_addr)(struct spi_nor *nor, u32 addr);
> --
> 2.28.0
>
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH v13 10/15] mtd: spi-nor: sfdp: detect Soft Reset sequence support from BFPT
2020-09-16 12:44 [PATCH v13 00/15] mtd: spi-nor: add xSPI Octal DTR support Pratyush Yadav
` (8 preceding siblings ...)
2020-09-16 12:44 ` [PATCH v13 09/15] mtd: spi-nor: core: enable octal DTR mode when possible Pratyush Yadav
@ 2020-09-16 12:44 ` Pratyush Yadav
2020-09-30 7:23 ` Tudor.Ambarus
2020-09-16 12:44 ` [PATCH v13 11/15] mtd: spi-nor: core: perform a Soft Reset on shutdown Pratyush Yadav
` (6 subsequent siblings)
16 siblings, 1 reply; 49+ messages in thread
From: Pratyush Yadav @ 2020-09-16 12:44 UTC (permalink / raw)
To: Tudor Ambarus, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, linux-mtd, linux-kernel
Cc: Boris Brezillon, Sekhar Nori, Pratyush Yadav
A Soft Reset sequence will return the flash to Power-on-Reset (POR)
state. It consists of two commands: Soft Reset Enable and Soft Reset.
Find out if the sequence is supported from BFPT DWORD 16.
Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
---
drivers/mtd/spi-nor/core.h | 1 +
drivers/mtd/spi-nor/sfdp.c | 4 ++++
drivers/mtd/spi-nor/sfdp.h | 2 ++
3 files changed, 7 insertions(+)
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index fcb5f071ebed..e2c7324d997e 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -26,6 +26,7 @@ enum spi_nor_option_flags {
SNOR_F_HAS_SR_TB_BIT6 = BIT(11),
SNOR_F_HAS_4BIT_BP = BIT(12),
SNOR_F_HAS_SR_BP3_BIT6 = BIT(13),
+ SNOR_F_SOFT_RESET = BIT(14),
};
struct spi_nor_read_command {
diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
index cadb1ed27ffe..f192710aca31 100644
--- a/drivers/mtd/spi-nor/sfdp.c
+++ b/drivers/mtd/spi-nor/sfdp.c
@@ -604,6 +604,10 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
break;
}
+ /* Soft Reset support. */
+ if (bfpt.dwords[BFPT_DWORD(16)] & BFPT_DWORD16_SOFT_RST)
+ nor->flags |= SNOR_F_SOFT_RESET;
+
/* Stop here if not JESD216 rev C or later. */
if (bfpt_header->length == BFPT_DWORD_MAX_JESD216B)
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 6d7243067252..8ae55e98084e 100644
--- a/drivers/mtd/spi-nor/sfdp.h
+++ b/drivers/mtd/spi-nor/sfdp.h
@@ -90,6 +90,8 @@ struct sfdp_bfpt {
#define BFPT_DWORD15_QER_SR2_BIT1_NO_RD (0x4UL << 20)
#define BFPT_DWORD15_QER_SR2_BIT1 (0x5UL << 20) /* Spansion */
+#define BFPT_DWORD16_SOFT_RST BIT(12)
+
#define BFPT_DWORD18_CMD_EXT_MASK GENMASK(30, 29)
#define BFPT_DWORD18_CMD_EXT_REP (0x0UL << 29) /* Repeat */
#define BFPT_DWORD18_CMD_EXT_INV (0x1UL << 29) /* Invert */
--
2.28.0
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 49+ messages in thread
* Re: [PATCH v13 10/15] mtd: spi-nor: sfdp: detect Soft Reset sequence support from BFPT
2020-09-16 12:44 ` [PATCH v13 10/15] mtd: spi-nor: sfdp: detect Soft Reset sequence support from BFPT Pratyush Yadav
@ 2020-09-30 7:23 ` Tudor.Ambarus
2020-09-30 7:31 ` Pratyush Yadav
0 siblings, 1 reply; 49+ messages in thread
From: Tudor.Ambarus @ 2020-09-30 7:23 UTC (permalink / raw)
To: p.yadav, miquel.raynal, richard, vigneshr, linux-mtd, linux-kernel
Cc: boris.brezillon, nsekhar
On 9/16/20 3:44 PM, Pratyush Yadav wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> A Soft Reset sequence will return the flash to Power-on-Reset (POR)
> state. It consists of two commands: Soft Reset Enable and Soft Reset.
> Find out if the sequence is supported from BFPT DWORD 16.
>
> Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
> ---
> drivers/mtd/spi-nor/core.h | 1 +
> drivers/mtd/spi-nor/sfdp.c | 4 ++++
> drivers/mtd/spi-nor/sfdp.h | 2 ++
> 3 files changed, 7 insertions(+)
>
> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
> index fcb5f071ebed..e2c7324d997e 100644
> --- a/drivers/mtd/spi-nor/core.h
> +++ b/drivers/mtd/spi-nor/core.h
> @@ -26,6 +26,7 @@ enum spi_nor_option_flags {
> SNOR_F_HAS_SR_TB_BIT6 = BIT(11),
> SNOR_F_HAS_4BIT_BP = BIT(12),
> SNOR_F_HAS_SR_BP3_BIT6 = BIT(13),
> + SNOR_F_SOFT_RESET = BIT(14),
> };
>
> struct spi_nor_read_command {
> diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
> index cadb1ed27ffe..f192710aca31 100644
> --- a/drivers/mtd/spi-nor/sfdp.c
> +++ b/drivers/mtd/spi-nor/sfdp.c
> @@ -604,6 +604,10 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
> break;
> }
>
> + /* Soft Reset support. */
> + if (bfpt.dwords[BFPT_DWORD(16)] & BFPT_DWORD16_SOFT_RST)
> + nor->flags |= SNOR_F_SOFT_RESET;
minimal support for SWRST but we can extend it later. Looks good.
> +
> /* Stop here if not JESD216 rev C or later. */
> if (bfpt_header->length == BFPT_DWORD_MAX_JESD216B)
> 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 6d7243067252..8ae55e98084e 100644
> --- a/drivers/mtd/spi-nor/sfdp.h
> +++ b/drivers/mtd/spi-nor/sfdp.h
> @@ -90,6 +90,8 @@ struct sfdp_bfpt {
> #define BFPT_DWORD15_QER_SR2_BIT1_NO_RD (0x4UL << 20)
> #define BFPT_DWORD15_QER_SR2_BIT1 (0x5UL << 20) /* Spansion */
>
> +#define BFPT_DWORD16_SOFT_RST BIT(12)
the name is too generic. How about
#define BFPT_DWORD16_SWRST_EN_RST BIT(12)
> +
> #define BFPT_DWORD18_CMD_EXT_MASK GENMASK(30, 29)
> #define BFPT_DWORD18_CMD_EXT_REP (0x0UL << 29) /* Repeat */
> #define BFPT_DWORD18_CMD_EXT_INV (0x1UL << 29) /* Invert */
> --
> 2.28.0
>
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v13 10/15] mtd: spi-nor: sfdp: detect Soft Reset sequence support from BFPT
2020-09-30 7:23 ` Tudor.Ambarus
@ 2020-09-30 7:31 ` Pratyush Yadav
0 siblings, 0 replies; 49+ messages in thread
From: Pratyush Yadav @ 2020-09-30 7:31 UTC (permalink / raw)
To: Tudor.Ambarus
Cc: vigneshr, richard, nsekhar, linux-kernel, boris.brezillon,
linux-mtd, miquel.raynal
On 30/09/20 07:23AM, Tudor.Ambarus@microchip.com wrote:
> On 9/16/20 3:44 PM, Pratyush Yadav wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >
> > A Soft Reset sequence will return the flash to Power-on-Reset (POR)
> > state. It consists of two commands: Soft Reset Enable and Soft Reset.
> > Find out if the sequence is supported from BFPT DWORD 16.
> >
> > Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
> > ---
> > drivers/mtd/spi-nor/core.h | 1 +
> > drivers/mtd/spi-nor/sfdp.c | 4 ++++
> > drivers/mtd/spi-nor/sfdp.h | 2 ++
> > 3 files changed, 7 insertions(+)
> >
> > diff --git a/drivers/mtd/spi-nor/sfdp.h b/drivers/mtd/spi-nor/sfdp.h
> > index 6d7243067252..8ae55e98084e 100644
> > --- a/drivers/mtd/spi-nor/sfdp.h
> > +++ b/drivers/mtd/spi-nor/sfdp.h
> > @@ -90,6 +90,8 @@ struct sfdp_bfpt {
> > #define BFPT_DWORD15_QER_SR2_BIT1_NO_RD (0x4UL << 20)
> > #define BFPT_DWORD15_QER_SR2_BIT1 (0x5UL << 20) /* Spansion */
> >
> > +#define BFPT_DWORD16_SOFT_RST BIT(12)
>
> the name is too generic. How about
>
> #define BFPT_DWORD16_SWRST_EN_RST BIT(12)
Ok.
>
> > +
> > #define BFPT_DWORD18_CMD_EXT_MASK GENMASK(30, 29)
> > #define BFPT_DWORD18_CMD_EXT_REP (0x0UL << 29) /* Repeat */
> > #define BFPT_DWORD18_CMD_EXT_INV (0x1UL << 29) /* Invert */
--
Regards,
Pratyush Yadav
Texas Instruments India
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH v13 11/15] mtd: spi-nor: core: perform a Soft Reset on shutdown
2020-09-16 12:44 [PATCH v13 00/15] mtd: spi-nor: add xSPI Octal DTR support Pratyush Yadav
` (9 preceding siblings ...)
2020-09-16 12:44 ` [PATCH v13 10/15] mtd: spi-nor: sfdp: detect Soft Reset sequence support from BFPT Pratyush Yadav
@ 2020-09-16 12:44 ` Pratyush Yadav
2020-09-29 13:08 ` Pratyush Yadav
2020-09-16 12:44 ` [PATCH v13 12/15] mtd: spi-nor: core: disable Octal DTR mode on suspend Pratyush Yadav
` (5 subsequent siblings)
16 siblings, 1 reply; 49+ messages in thread
From: Pratyush Yadav @ 2020-09-16 12:44 UTC (permalink / raw)
To: Tudor Ambarus, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, linux-mtd, linux-kernel
Cc: Boris Brezillon, Sekhar Nori, Pratyush Yadav
Perform a Soft Reset on shutdown on flashes that support it so that the
flash can be reset to its initial state and any configurations made by
spi-nor (given that they're only done in volatile registers) will be
reset. This will hand back the flash in pristine state for any further
operations on it.
Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
---
drivers/mtd/spi-nor/core.c | 41 +++++++++++++++++++++++++++++++++++++
include/linux/mtd/spi-nor.h | 2 ++
2 files changed, 43 insertions(+)
diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 6ee93544d72f..853dfa02f0de 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -40,6 +40,9 @@
#define SPI_NOR_MAX_ADDR_WIDTH 4
+#define SPI_NOR_SRST_SLEEP_MIN 200
+#define SPI_NOR_SRST_SLEEP_MAX 400
+
/**
* spi_nor_get_cmd_ext() - Get the command opcode extension based on the
* extension type.
@@ -3174,6 +3177,41 @@ static int spi_nor_init(struct spi_nor *nor)
return 0;
}
+static void spi_nor_soft_reset(struct spi_nor *nor)
+{
+ struct spi_mem_op op;
+ int ret;
+
+ op = (struct spi_mem_op)SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_SRSTEN, 8),
+ SPI_MEM_OP_NO_DUMMY,
+ SPI_MEM_OP_NO_ADDR,
+ SPI_MEM_OP_NO_DATA);
+ spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
+ ret = spi_mem_exec_op(nor->spimem, &op);
+ if (ret) {
+ dev_warn(nor->dev, "Software reset failed: %d\n", ret);
+ return;
+ }
+
+ op = (struct spi_mem_op)SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_SRST, 8),
+ SPI_MEM_OP_NO_DUMMY,
+ SPI_MEM_OP_NO_ADDR,
+ SPI_MEM_OP_NO_DATA);
+ spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
+ ret = spi_mem_exec_op(nor->spimem, &op);
+ if (ret) {
+ dev_warn(nor->dev, "Software reset failed: %d\n", ret);
+ return;
+ }
+
+ /*
+ * Software Reset is not instant, and the delay varies from flash to
+ * flash. Looking at a few flashes, most range somewhere below 100
+ * microseconds. So, sleep for a range of 200-400 us.
+ */
+ usleep_range(SPI_NOR_SRST_SLEEP_MIN, SPI_NOR_SRST_SLEEP_MAX);
+}
+
/* mtd resume handler */
static void spi_nor_resume(struct mtd_info *mtd)
{
@@ -3194,6 +3232,9 @@ void spi_nor_restore(struct spi_nor *nor)
nor->flags & SNOR_F_BROKEN_RESET)
nor->params->set_4byte_addr_mode(nor, false);
+ if (nor->flags & SNOR_F_SOFT_RESET)
+ spi_nor_soft_reset(nor);
+
spi_nor_quad_enable(nor, false);
}
EXPORT_SYMBOL_GPL(spi_nor_restore);
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index cd549042c53d..299685d15dc2 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -51,6 +51,8 @@
#define SPINOR_OP_CLFSR 0x50 /* Clear flag status register */
#define SPINOR_OP_RDEAR 0xc8 /* Read Extended Address Register */
#define SPINOR_OP_WREAR 0xc5 /* Write Extended Address Register */
+#define SPINOR_OP_SRSTEN 0x66 /* Software Reset Enable */
+#define SPINOR_OP_SRST 0x99 /* Software Reset */
/* 4-byte address opcodes - used on Spansion and some Macronix flashes. */
#define SPINOR_OP_READ_4B 0x13 /* Read data bytes (low frequency) */
--
2.28.0
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 49+ messages in thread
* Re: [PATCH v13 11/15] mtd: spi-nor: core: perform a Soft Reset on shutdown
2020-09-16 12:44 ` [PATCH v13 11/15] mtd: spi-nor: core: perform a Soft Reset on shutdown Pratyush Yadav
@ 2020-09-29 13:08 ` Pratyush Yadav
2020-09-30 7:32 ` Tudor.Ambarus
0 siblings, 1 reply; 49+ messages in thread
From: Pratyush Yadav @ 2020-09-29 13:08 UTC (permalink / raw)
To: Tudor Ambarus, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, linux-mtd, linux-kernel
Cc: Boris Brezillon, Sekhar Nori
On 16/09/20 06:14PM, Pratyush Yadav wrote:
> Perform a Soft Reset on shutdown on flashes that support it so that the
> flash can be reset to its initial state and any configurations made by
> spi-nor (given that they're only done in volatile registers) will be
> reset. This will hand back the flash in pristine state for any further
> operations on it.
>
> Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
> ---
> drivers/mtd/spi-nor/core.c | 41 +++++++++++++++++++++++++++++++++++++
> include/linux/mtd/spi-nor.h | 2 ++
> 2 files changed, 43 insertions(+)
>
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index 6ee93544d72f..853dfa02f0de 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -40,6 +40,9 @@
>
> #define SPI_NOR_MAX_ADDR_WIDTH 4
>
> +#define SPI_NOR_SRST_SLEEP_MIN 200
> +#define SPI_NOR_SRST_SLEEP_MAX 400
> +
> /**
> * spi_nor_get_cmd_ext() - Get the command opcode extension based on the
> * extension type.
> @@ -3174,6 +3177,41 @@ static int spi_nor_init(struct spi_nor *nor)
> return 0;
> }
>
> +static void spi_nor_soft_reset(struct spi_nor *nor)
> +{
> + struct spi_mem_op op;
> + int ret;
> +
> + op = (struct spi_mem_op)SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_SRSTEN, 8),
The buswidth used here should be 1 instead of 8. It makes no difference
in practice because the call to spi_nor_spimem_setup_op() immediately
after will over-write it to the correct value anyway, but let's follow
the style followed throughout the rest of the codebase. Will fix in the
next version.
> + SPI_MEM_OP_NO_DUMMY,
> + SPI_MEM_OP_NO_ADDR,
> + SPI_MEM_OP_NO_DATA);
> + spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
> + ret = spi_mem_exec_op(nor->spimem, &op);
> + if (ret) {
> + dev_warn(nor->dev, "Software reset failed: %d\n", ret);
> + return;
> + }
> +
> + op = (struct spi_mem_op)SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_SRST, 8),
Same here.
> + SPI_MEM_OP_NO_DUMMY,
> + SPI_MEM_OP_NO_ADDR,
> + SPI_MEM_OP_NO_DATA);
> + spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
> + ret = spi_mem_exec_op(nor->spimem, &op);
> + if (ret) {
> + dev_warn(nor->dev, "Software reset failed: %d\n", ret);
> + return;
> + }
> +
> + /*
> + * Software Reset is not instant, and the delay varies from flash to
> + * flash. Looking at a few flashes, most range somewhere below 100
> + * microseconds. So, sleep for a range of 200-400 us.
> + */
> + usleep_range(SPI_NOR_SRST_SLEEP_MIN, SPI_NOR_SRST_SLEEP_MAX);
> +}
> +
> /* mtd resume handler */
> static void spi_nor_resume(struct mtd_info *mtd)
> {
--
Regards,
Pratyush Yadav
Texas Instruments India
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v13 11/15] mtd: spi-nor: core: perform a Soft Reset on shutdown
2020-09-29 13:08 ` Pratyush Yadav
@ 2020-09-30 7:32 ` Tudor.Ambarus
2020-09-30 7:43 ` Pratyush Yadav
0 siblings, 1 reply; 49+ messages in thread
From: Tudor.Ambarus @ 2020-09-30 7:32 UTC (permalink / raw)
To: p.yadav, miquel.raynal, richard, vigneshr, linux-mtd, linux-kernel
Cc: boris.brezillon, nsekhar
On 9/29/20 4:08 PM, Pratyush Yadav wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> On 16/09/20 06:14PM, Pratyush Yadav wrote:
>> Perform a Soft Reset on shutdown on flashes that support it so that the
>> flash can be reset to its initial state and any configurations made by
>> spi-nor (given that they're only done in volatile registers) will be
>> reset. This will hand back the flash in pristine state for any further
>> operations on it.
>>
>> Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
>> ---
>> drivers/mtd/spi-nor/core.c | 41 +++++++++++++++++++++++++++++++++++++
>> include/linux/mtd/spi-nor.h | 2 ++
>> 2 files changed, 43 insertions(+)
>>
>> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
>> index 6ee93544d72f..853dfa02f0de 100644
>> --- a/drivers/mtd/spi-nor/core.c
>> +++ b/drivers/mtd/spi-nor/core.c
>> @@ -40,6 +40,9 @@
>>
>> #define SPI_NOR_MAX_ADDR_WIDTH 4
>>
>> +#define SPI_NOR_SRST_SLEEP_MIN 200
>> +#define SPI_NOR_SRST_SLEEP_MAX 400
>> +
>> /**
>> * spi_nor_get_cmd_ext() - Get the command opcode extension based on the
>> * extension type.
>> @@ -3174,6 +3177,41 @@ static int spi_nor_init(struct spi_nor *nor)
>> return 0;
>> }
>>
>> +static void spi_nor_soft_reset(struct spi_nor *nor)
>> +{
>> + struct spi_mem_op op;
>> + int ret;
>> +
>> + op = (struct spi_mem_op)SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_SRSTEN, 8),
can we avoid the cast?
>
> The buswidth used here should be 1 instead of 8. It makes no difference
> in practice because the call to spi_nor_spimem_setup_op() immediately
> after will over-write it to the correct value anyway, but let's follow
> the style followed throughout the rest of the codebase. Will fix in the
> next version.
or you can just set the buswidth to 0 so that the reader rises his eyebrow
and search for where it is updated. If you like it better you'll have to change
throughout the entire code base, maybe in 4/15 where setup_op is introduced.
>
>> + SPI_MEM_OP_NO_DUMMY,
>> + SPI_MEM_OP_NO_ADDR,
>> + SPI_MEM_OP_NO_DATA);
>> + spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
Still not a big fan of this, but we can update the op init later on. How
about some new lines around spi_nor_spimem_setup_op()? First time I read
the code I haven't seen it.
>> + ret = spi_mem_exec_op(nor->spimem, &op);
>> + if (ret) {
>> + dev_warn(nor->dev, "Software reset failed: %d\n", ret);
>> + return;
>> + }
>> +
>> + op = (struct spi_mem_op)SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_SRST, 8),
>
> Same here.
>
>> + SPI_MEM_OP_NO_DUMMY,
>> + SPI_MEM_OP_NO_ADDR,
>> + SPI_MEM_OP_NO_DATA);
>> + spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
>> + ret = spi_mem_exec_op(nor->spimem, &op);
>> + if (ret) {
>> + dev_warn(nor->dev, "Software reset failed: %d\n", ret);
>> + return;
>> + }
>> +
>> + /*
>> + * Software Reset is not instant, and the delay varies from flash to
>> + * flash. Looking at a few flashes, most range somewhere below 100
>> + * microseconds. So, sleep for a range of 200-400 us.
>> + */
>> + usleep_range(SPI_NOR_SRST_SLEEP_MIN, SPI_NOR_SRST_SLEEP_MAX);
>> +}
>> +
>> /* mtd resume handler */
>> static void spi_nor_resume(struct mtd_info *mtd)
>> {
>
> --
> Regards,
> Pratyush Yadav
> Texas Instruments India
>
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v13 11/15] mtd: spi-nor: core: perform a Soft Reset on shutdown
2020-09-30 7:32 ` Tudor.Ambarus
@ 2020-09-30 7:43 ` Pratyush Yadav
0 siblings, 0 replies; 49+ messages in thread
From: Pratyush Yadav @ 2020-09-30 7:43 UTC (permalink / raw)
To: Tudor.Ambarus
Cc: vigneshr, richard, nsekhar, linux-kernel, boris.brezillon,
linux-mtd, miquel.raynal
On 30/09/20 07:32AM, Tudor.Ambarus@microchip.com wrote:
> On 9/29/20 4:08 PM, Pratyush Yadav wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >
> > On 16/09/20 06:14PM, Pratyush Yadav wrote:
> >> Perform a Soft Reset on shutdown on flashes that support it so that the
> >> flash can be reset to its initial state and any configurations made by
> >> spi-nor (given that they're only done in volatile registers) will be
> >> reset. This will hand back the flash in pristine state for any further
> >> operations on it.
> >>
> >> Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
> >> ---
> >> drivers/mtd/spi-nor/core.c | 41 +++++++++++++++++++++++++++++++++++++
> >> include/linux/mtd/spi-nor.h | 2 ++
> >> 2 files changed, 43 insertions(+)
> >>
> >> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> >> index 6ee93544d72f..853dfa02f0de 100644
> >> --- a/drivers/mtd/spi-nor/core.c
> >> +++ b/drivers/mtd/spi-nor/core.c
> >> @@ -40,6 +40,9 @@
> >>
> >> #define SPI_NOR_MAX_ADDR_WIDTH 4
> >>
> >> +#define SPI_NOR_SRST_SLEEP_MIN 200
> >> +#define SPI_NOR_SRST_SLEEP_MAX 400
> >> +
> >> /**
> >> * spi_nor_get_cmd_ext() - Get the command opcode extension based on the
> >> * extension type.
> >> @@ -3174,6 +3177,41 @@ static int spi_nor_init(struct spi_nor *nor)
> >> return 0;
> >> }
> >>
> >> +static void spi_nor_soft_reset(struct spi_nor *nor)
> >> +{
> >> + struct spi_mem_op op;
> >> + int ret;
> >> +
> >> + op = (struct spi_mem_op)SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_SRSTEN, 8),
>
> can we avoid the cast?
No. The compiler complains about "expected expression before '{' token".
We can avoid it if the assignment is with the declaration but then we
can't re-use the variable op for the second command.
I like the cast better than using separate variables for the two (or
more in other cases) commands we execute.
> >
> > The buswidth used here should be 1 instead of 8. It makes no difference
> > in practice because the call to spi_nor_spimem_setup_op() immediately
> > after will over-write it to the correct value anyway, but let's follow
> > the style followed throughout the rest of the codebase. Will fix in the
> > next version.
>
> or you can just set the buswidth to 0 so that the reader rises his eyebrow
> and search for where it is updated. If you like it better you'll have to change
> throughout the entire code base, maybe in 4/15 where setup_op is introduced.
Ok. Will change it.
> >
> >> + SPI_MEM_OP_NO_DUMMY,
> >> + SPI_MEM_OP_NO_ADDR,
> >> + SPI_MEM_OP_NO_DATA);
> >> + spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
>
> Still not a big fan of this, but we can update the op init later on. How
> about some new lines around spi_nor_spimem_setup_op()? First time I read
> the code I haven't seen it.
Ok.
> >> + ret = spi_mem_exec_op(nor->spimem, &op);
> >> + if (ret) {
> >> + dev_warn(nor->dev, "Software reset failed: %d\n", ret);
> >> + return;
> >> + }
> >> +
> >> + op = (struct spi_mem_op)SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_SRST, 8),
> >
> > Same here.
> >
> >> + SPI_MEM_OP_NO_DUMMY,
> >> + SPI_MEM_OP_NO_ADDR,
> >> + SPI_MEM_OP_NO_DATA);
> >> + spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
> >> + ret = spi_mem_exec_op(nor->spimem, &op);
> >> + if (ret) {
> >> + dev_warn(nor->dev, "Software reset failed: %d\n", ret);
> >> + return;
> >> + }
> >> +
> >> + /*
> >> + * Software Reset is not instant, and the delay varies from flash to
> >> + * flash. Looking at a few flashes, most range somewhere below 100
> >> + * microseconds. So, sleep for a range of 200-400 us.
> >> + */
> >> + usleep_range(SPI_NOR_SRST_SLEEP_MIN, SPI_NOR_SRST_SLEEP_MAX);
> >> +}
> >> +
> >> /* mtd resume handler */
> >> static void spi_nor_resume(struct mtd_info *mtd)
> >> {
--
Regards,
Pratyush Yadav
Texas Instruments India
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH v13 12/15] mtd: spi-nor: core: disable Octal DTR mode on suspend.
2020-09-16 12:44 [PATCH v13 00/15] mtd: spi-nor: add xSPI Octal DTR support Pratyush Yadav
` (10 preceding siblings ...)
2020-09-16 12:44 ` [PATCH v13 11/15] mtd: spi-nor: core: perform a Soft Reset on shutdown Pratyush Yadav
@ 2020-09-16 12:44 ` Pratyush Yadav
2020-09-30 7:40 ` Tudor.Ambarus
2020-09-16 12:44 ` [PATCH v13 13/15] mtd: spi-nor: core: expose spi_nor_default_setup() in core.h Pratyush Yadav
` (4 subsequent siblings)
16 siblings, 1 reply; 49+ messages in thread
From: Pratyush Yadav @ 2020-09-16 12:44 UTC (permalink / raw)
To: Tudor Ambarus, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, linux-mtd, linux-kernel
Cc: Boris Brezillon, Sekhar Nori, Pratyush Yadav
On resume, the init procedure will be run that will re-enable it.
Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
---
drivers/mtd/spi-nor/core.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 853dfa02f0de..d5c92c9c7307 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -3212,6 +3212,23 @@ static void spi_nor_soft_reset(struct spi_nor *nor)
usleep_range(SPI_NOR_SRST_SLEEP_MIN, SPI_NOR_SRST_SLEEP_MAX);
}
+/* mtd suspend handler */
+static int spi_nor_suspend(struct mtd_info *mtd)
+{
+ struct spi_nor *nor = mtd_to_spi_nor(mtd);
+ struct device *dev = nor->dev;
+ int ret;
+
+ /* Disable octal DTR mode if we enabled it. */
+ ret = spi_nor_octal_dtr_enable(nor, false);
+ if (ret) {
+ dev_err(dev, "suspend() failed\n");
+ return ret;
+ }
+
+ return 0;
+}
+
/* mtd resume handler */
static void spi_nor_resume(struct mtd_info *mtd)
{
@@ -3406,6 +3423,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
mtd->size = nor->params->size;
mtd->_erase = spi_nor_erase;
mtd->_read = spi_nor_read;
+ mtd->_suspend = spi_nor_suspend;
mtd->_resume = spi_nor_resume;
if (nor->params->locking_ops) {
--
2.28.0
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 49+ messages in thread
* Re: [PATCH v13 12/15] mtd: spi-nor: core: disable Octal DTR mode on suspend.
2020-09-16 12:44 ` [PATCH v13 12/15] mtd: spi-nor: core: disable Octal DTR mode on suspend Pratyush Yadav
@ 2020-09-30 7:40 ` Tudor.Ambarus
2020-09-30 7:44 ` Pratyush Yadav
0 siblings, 1 reply; 49+ messages in thread
From: Tudor.Ambarus @ 2020-09-30 7:40 UTC (permalink / raw)
To: p.yadav, miquel.raynal, richard, vigneshr, linux-mtd, linux-kernel
Cc: boris.brezillon, nsekhar
On 9/16/20 3:44 PM, Pratyush Yadav wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> On resume, the init procedure will be run that will re-enable it.
>
> Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
> ---
> drivers/mtd/spi-nor/core.c | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index 853dfa02f0de..d5c92c9c7307 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -3212,6 +3212,23 @@ static void spi_nor_soft_reset(struct spi_nor *nor)
> usleep_range(SPI_NOR_SRST_SLEEP_MIN, SPI_NOR_SRST_SLEEP_MAX);
> }
>
> +/* mtd suspend handler */
> +static int spi_nor_suspend(struct mtd_info *mtd)
> +{
> + struct spi_nor *nor = mtd_to_spi_nor(mtd);
> + struct device *dev = nor->dev;
> + int ret;
> +
> + /* Disable octal DTR mode if we enabled it. */
> + ret = spi_nor_octal_dtr_enable(nor, false);
> + if (ret) {
> + dev_err(dev, "suspend() failed\n");
we can get rid of dev local variable as it is used only once. you can use
nor->dev directly
> + return ret;
> + }
and maybe just return ret; directly. spi_nor_octal_dtr_enable() returns 0 on
success.
Looks good.
> +
> + return 0;
> +}
> +
> /* mtd resume handler */
> static void spi_nor_resume(struct mtd_info *mtd)
> {
> @@ -3406,6 +3423,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
> mtd->size = nor->params->size;
> mtd->_erase = spi_nor_erase;
> mtd->_read = spi_nor_read;
> + mtd->_suspend = spi_nor_suspend;
> mtd->_resume = spi_nor_resume;
>
> if (nor->params->locking_ops) {
> --
> 2.28.0
>
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v13 12/15] mtd: spi-nor: core: disable Octal DTR mode on suspend.
2020-09-30 7:40 ` Tudor.Ambarus
@ 2020-09-30 7:44 ` Pratyush Yadav
0 siblings, 0 replies; 49+ messages in thread
From: Pratyush Yadav @ 2020-09-30 7:44 UTC (permalink / raw)
To: Tudor.Ambarus
Cc: vigneshr, richard, nsekhar, linux-kernel, boris.brezillon,
linux-mtd, miquel.raynal
On 30/09/20 07:40AM, Tudor.Ambarus@microchip.com wrote:
> On 9/16/20 3:44 PM, Pratyush Yadav wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >
> > On resume, the init procedure will be run that will re-enable it.
> >
> > Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
>
> > ---
> > drivers/mtd/spi-nor/core.c | 18 ++++++++++++++++++
> > 1 file changed, 18 insertions(+)
> >
> > diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> > index 853dfa02f0de..d5c92c9c7307 100644
> > --- a/drivers/mtd/spi-nor/core.c
> > +++ b/drivers/mtd/spi-nor/core.c
> > @@ -3212,6 +3212,23 @@ static void spi_nor_soft_reset(struct spi_nor *nor)
> > usleep_range(SPI_NOR_SRST_SLEEP_MIN, SPI_NOR_SRST_SLEEP_MAX);
> > }
> >
> > +/* mtd suspend handler */
> > +static int spi_nor_suspend(struct mtd_info *mtd)
> > +{
> > + struct spi_nor *nor = mtd_to_spi_nor(mtd);
> > + struct device *dev = nor->dev;
> > + int ret;
> > +
> > + /* Disable octal DTR mode if we enabled it. */
> > + ret = spi_nor_octal_dtr_enable(nor, false);
> > + if (ret) {
> > + dev_err(dev, "suspend() failed\n");
>
> we can get rid of dev local variable as it is used only once. you can use
> nor->dev directly
Ok.
> > + return ret;
> > + }
>
> and maybe just return ret; directly. spi_nor_octal_dtr_enable() returns 0 on
> success.
Ok.
> Looks good.
>
--
Regards,
Pratyush Yadav
Texas Instruments India
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH v13 13/15] mtd: spi-nor: core: expose spi_nor_default_setup() in core.h
2020-09-16 12:44 [PATCH v13 00/15] mtd: spi-nor: add xSPI Octal DTR support Pratyush Yadav
` (11 preceding siblings ...)
2020-09-16 12:44 ` [PATCH v13 12/15] mtd: spi-nor: core: disable Octal DTR mode on suspend Pratyush Yadav
@ 2020-09-16 12:44 ` Pratyush Yadav
2020-09-30 7:51 ` Tudor.Ambarus
2020-09-16 12:44 ` [PATCH v13 14/15] mtd: spi-nor: spansion: add support for Cypress Semper flash Pratyush Yadav
` (3 subsequent siblings)
16 siblings, 1 reply; 49+ messages in thread
From: Pratyush Yadav @ 2020-09-16 12:44 UTC (permalink / raw)
To: Tudor Ambarus, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, linux-mtd, linux-kernel
Cc: Boris Brezillon, Sekhar Nori, Pratyush Yadav
Flashes might want to add a custom setup hook to configure the flash in
the proper mode for operation. But after that, they would still want to
run the default setup hook because it selects the read, program, and
erase operations. Since there is little point in repeating all that
code, expose the spi_nor_default_setup() in core.h to
manufacturer-specific files.
Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
---
drivers/mtd/spi-nor/core.c | 4 ++--
drivers/mtd/spi-nor/core.h | 3 +++
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index d5c92c9c7307..34edfcf33172 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -2759,8 +2759,8 @@ static int spi_nor_select_erase(struct spi_nor *nor)
return 0;
}
-static int spi_nor_default_setup(struct spi_nor *nor,
- const struct spi_nor_hwcaps *hwcaps)
+int spi_nor_default_setup(struct spi_nor *nor,
+ const struct spi_nor_hwcaps *hwcaps)
{
struct spi_nor_flash_parameter *params = nor->params;
u32 ignored_mask, shared_mask;
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index e2c7324d997e..10dc03506f93 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -453,6 +453,9 @@ int spi_nor_post_bfpt_fixups(struct spi_nor *nor,
const struct sfdp_bfpt *bfpt,
struct spi_nor_flash_parameter *params);
+int spi_nor_default_setup(struct spi_nor *nor,
+ const struct spi_nor_hwcaps *hwcaps);
+
static struct spi_nor __maybe_unused *mtd_to_spi_nor(struct mtd_info *mtd)
{
return mtd->priv;
--
2.28.0
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 49+ messages in thread
* Re: [PATCH v13 13/15] mtd: spi-nor: core: expose spi_nor_default_setup() in core.h
2020-09-16 12:44 ` [PATCH v13 13/15] mtd: spi-nor: core: expose spi_nor_default_setup() in core.h Pratyush Yadav
@ 2020-09-30 7:51 ` Tudor.Ambarus
2020-09-30 8:03 ` Pratyush Yadav
0 siblings, 1 reply; 49+ messages in thread
From: Tudor.Ambarus @ 2020-09-30 7:51 UTC (permalink / raw)
To: p.yadav, miquel.raynal, richard, vigneshr, linux-mtd, linux-kernel
Cc: boris.brezillon, nsekhar
On 9/16/20 3:44 PM, Pratyush Yadav wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> Flashes might want to add a custom setup hook to configure the flash in
> the proper mode for operation. But after that, they would still want to
> run the default setup hook because it selects the read, program, and
> erase operations. Since there is little point in repeating all that
> code, expose the spi_nor_default_setup() in core.h to
> manufacturer-specific files.
But you don't use it in the following patches. Can we drop this one for now?
>
> Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
> ---
> drivers/mtd/spi-nor/core.c | 4 ++--
> drivers/mtd/spi-nor/core.h | 3 +++
> 2 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index d5c92c9c7307..34edfcf33172 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -2759,8 +2759,8 @@ static int spi_nor_select_erase(struct spi_nor *nor)
> return 0;
> }
>
> -static int spi_nor_default_setup(struct spi_nor *nor,
> - const struct spi_nor_hwcaps *hwcaps)
> +int spi_nor_default_setup(struct spi_nor *nor,
> + const struct spi_nor_hwcaps *hwcaps)
> {
> struct spi_nor_flash_parameter *params = nor->params;
> u32 ignored_mask, shared_mask;
> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
> index e2c7324d997e..10dc03506f93 100644
> --- a/drivers/mtd/spi-nor/core.h
> +++ b/drivers/mtd/spi-nor/core.h
> @@ -453,6 +453,9 @@ int spi_nor_post_bfpt_fixups(struct spi_nor *nor,
> const struct sfdp_bfpt *bfpt,
> struct spi_nor_flash_parameter *params);
>
> +int spi_nor_default_setup(struct spi_nor *nor,
> + const struct spi_nor_hwcaps *hwcaps);
> +
> static struct spi_nor __maybe_unused *mtd_to_spi_nor(struct mtd_info *mtd)
> {
> return mtd->priv;
> --
> 2.28.0
>
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v13 13/15] mtd: spi-nor: core: expose spi_nor_default_setup() in core.h
2020-09-30 7:51 ` Tudor.Ambarus
@ 2020-09-30 8:03 ` Pratyush Yadav
0 siblings, 0 replies; 49+ messages in thread
From: Pratyush Yadav @ 2020-09-30 8:03 UTC (permalink / raw)
To: Tudor.Ambarus
Cc: vigneshr, richard, nsekhar, linux-kernel, boris.brezillon,
linux-mtd, miquel.raynal
On 30/09/20 07:51AM, Tudor.Ambarus@microchip.com wrote:
> On 9/16/20 3:44 PM, Pratyush Yadav wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >
> > Flashes might want to add a custom setup hook to configure the flash in
> > the proper mode for operation. But after that, they would still want to
> > run the default setup hook because it selects the read, program, and
> > erase operations. Since there is little point in repeating all that
> > code, expose the spi_nor_default_setup() in core.h to
> > manufacturer-specific files.
>
> But you don't use it in the following patches. Can we drop this one for now?
I used it for earlier versions but not for the latest ones. This can be
dropped for now.
> >
> > Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
> > ---
> > drivers/mtd/spi-nor/core.c | 4 ++--
> > drivers/mtd/spi-nor/core.h | 3 +++
> > 2 files changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> > index d5c92c9c7307..34edfcf33172 100644
> > --- a/drivers/mtd/spi-nor/core.c
> > +++ b/drivers/mtd/spi-nor/core.c
> > @@ -2759,8 +2759,8 @@ static int spi_nor_select_erase(struct spi_nor *nor)
> > return 0;
> > }
> >
> > -static int spi_nor_default_setup(struct spi_nor *nor,
> > - const struct spi_nor_hwcaps *hwcaps)
> > +int spi_nor_default_setup(struct spi_nor *nor,
> > + const struct spi_nor_hwcaps *hwcaps)
> > {
> > struct spi_nor_flash_parameter *params = nor->params;
> > u32 ignored_mask, shared_mask;
> > diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
> > index e2c7324d997e..10dc03506f93 100644
> > --- a/drivers/mtd/spi-nor/core.h
> > +++ b/drivers/mtd/spi-nor/core.h
> > @@ -453,6 +453,9 @@ int spi_nor_post_bfpt_fixups(struct spi_nor *nor,
> > const struct sfdp_bfpt *bfpt,
> > struct spi_nor_flash_parameter *params);
> >
> > +int spi_nor_default_setup(struct spi_nor *nor,
> > + const struct spi_nor_hwcaps *hwcaps);
> > +
> > static struct spi_nor __maybe_unused *mtd_to_spi_nor(struct mtd_info *mtd)
> > {
> > return mtd->priv;
> > --
> > 2.28.0
> >
>
--
Regards,
Pratyush Yadav
Texas Instruments India
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH v13 14/15] mtd: spi-nor: spansion: add support for Cypress Semper flash
2020-09-16 12:44 [PATCH v13 00/15] mtd: spi-nor: add xSPI Octal DTR support Pratyush Yadav
` (12 preceding siblings ...)
2020-09-16 12:44 ` [PATCH v13 13/15] mtd: spi-nor: core: expose spi_nor_default_setup() in core.h Pratyush Yadav
@ 2020-09-16 12:44 ` Pratyush Yadav
2020-09-30 8:36 ` Tudor.Ambarus
2020-09-16 12:44 ` [PATCH v13 15/15] mtd: spi-nor: micron-st: allow using MT35XU512ABA in Octal DTR mode Pratyush Yadav
` (2 subsequent siblings)
16 siblings, 1 reply; 49+ messages in thread
From: Pratyush Yadav @ 2020-09-16 12:44 UTC (permalink / raw)
To: Tudor Ambarus, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, linux-mtd, linux-kernel
Cc: Boris Brezillon, Sekhar Nori, Pratyush Yadav
The Cypress Semper flash is an xSPI compliant octal DTR flash. Add
support for using it in octal DTR mode.
The flash by default boots in a hybrid sector mode. But the sector map
table on the part I had was programmed incorrectly and the SMPT values
on the flash don't match the public datasheet. Specifically, in some
places erase type 3 was used instead of 4. In addition, the region sizes
were incorrect in some places. So, for testing I set CFR3N[3] to enable
uniform sector sizes. Since the uniform sector mode bit is a
non-volatile bit, this series does not change it to avoid making any
permanent changes to the flash configuration. The correct data to
implement a fixup is not available right now and will be done in a
follow-up patch if needed.
Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
---
drivers/mtd/spi-nor/spansion.c | 166 +++++++++++++++++++++++++++++++++
1 file changed, 166 insertions(+)
diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
index 8429b4af999a..a34e250ea5a2 100644
--- a/drivers/mtd/spi-nor/spansion.c
+++ b/drivers/mtd/spi-nor/spansion.c
@@ -8,6 +8,167 @@
#include "core.h"
+#define SPINOR_OP_RD_ANY_REG 0x65 /* Read any register */
+#define SPINOR_OP_WR_ANY_REG 0x71 /* Write any register */
+#define SPINOR_REG_CYPRESS_CFR2V 0x00800003
+#define SPINOR_REG_CYPRESS_CFR2V_MEMLAT_11_24 0xb
+#define SPINOR_REG_CYPRESS_CFR3V 0x00800004
+#define SPINOR_REG_CYPRESS_CFR3V_PGSZ BIT(4) /* Page size. */
+#define SPINOR_REG_CYPRESS_CFR5V 0x00800006
+#define SPINOR_REG_CYPRESS_CFR5V_OCT_DTR_EN 0x3
+#define SPINOR_REG_CYPRESS_CFR5V_OCT_DTR_DS 0
+#define SPINOR_OP_CYPRESS_RD_FAST 0xee
+
+/**
+ * spi_nor_cypress_octal_dtr_enable() - Enable octal DTR on Cypress flashes.
+ * @nor: pointer to a 'struct spi_nor'
+ * @enable: whether to enable or disable Octal DTR
+ *
+ * This also sets the memory access latency cycles to 24 to allow the flash to
+ * run at up to 200MHz.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int spi_nor_cypress_octal_dtr_enable(struct spi_nor *nor, bool enable)
+{
+ struct spi_mem_op op;
+ u8 *buf = nor->bouncebuf;
+ u8 addr_width;
+ int ret;
+
+ if (enable)
+ addr_width = 3;
+ else
+ addr_width = 4;
+
+ if (enable) {
+ /* Use 24 dummy cycles for memory array reads. */
+ ret = spi_nor_write_enable(nor);
+ if (ret)
+ return ret;
+
+ *buf = SPINOR_REG_CYPRESS_CFR2V_MEMLAT_11_24;
+ op = (struct spi_mem_op)
+ SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WR_ANY_REG, 1),
+ SPI_MEM_OP_ADDR(addr_width,
+ SPINOR_REG_CYPRESS_CFR2V,
+ 1),
+ SPI_MEM_OP_NO_DUMMY,
+ SPI_MEM_OP_DATA_OUT(1, buf, 1));
+ ret = spi_mem_exec_op(nor->spimem, &op);
+ if (ret) {
+ dev_warn(nor->dev,
+ "failed to set default memory latency value: %d\n",
+ ret);
+ return ret;
+ }
+ ret = spi_nor_wait_till_ready(nor);
+ if (ret)
+ return ret;
+
+ nor->read_dummy = 24;
+ }
+
+ /* Set/unset the octal and DTR enable bits. */
+ ret = spi_nor_write_enable(nor);
+ if (ret)
+ return ret;
+
+ if (enable)
+ *buf = SPINOR_REG_CYPRESS_CFR5V_OCT_DTR_EN;
+ else
+ *buf = SPINOR_REG_CYPRESS_CFR5V_OCT_DTR_DS;
+ op = (struct spi_mem_op)
+ SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WR_ANY_REG, 1),
+ SPI_MEM_OP_ADDR(addr_width,
+ SPINOR_REG_CYPRESS_CFR5V,
+ 1),
+ SPI_MEM_OP_NO_DUMMY,
+ SPI_MEM_OP_DATA_OUT(1, buf, 1));
+
+ if (!enable)
+ spi_nor_spimem_setup_op(nor, &op, SNOR_PROTO_8_8_8_DTR);
+
+ ret = spi_mem_exec_op(nor->spimem, &op);
+ if (ret) {
+ dev_warn(nor->dev, "Failed to enable octal DTR mode\n");
+ return ret;
+ }
+
+ return 0;
+}
+
+static void s28hs512t_default_init(struct spi_nor *nor)
+{
+ nor->params->octal_dtr_enable = spi_nor_cypress_octal_dtr_enable;
+}
+
+static void s28hs512t_post_sfdp_fixup(struct spi_nor *nor)
+{
+ /*
+ * On older versions of the flash the xSPI Profile 1.0 table has the
+ * 8D-8D-8D Fast Read opcode as 0x00. But it actually should be 0xEE.
+ */
+ if (nor->params->reads[SNOR_CMD_READ_8_8_8_DTR].opcode == 0)
+ nor->params->reads[SNOR_CMD_READ_8_8_8_DTR].opcode =
+ SPINOR_OP_CYPRESS_RD_FAST;
+
+ /* This flash is also missing the 4-byte Page Program opcode bit. */
+ spi_nor_set_pp_settings(&nor->params->page_programs[SNOR_CMD_PP],
+ SPINOR_OP_PP_4B, SNOR_PROTO_1_1_1);
+ /*
+ * Since xSPI Page Program opcode is backward compatible with
+ * Legacy SPI, use Legacy SPI opcode there as well.
+ */
+ spi_nor_set_pp_settings(&nor->params->page_programs[SNOR_CMD_PP_8_8_8_DTR],
+ SPINOR_OP_PP_4B, SNOR_PROTO_8_8_8_DTR);
+
+ /*
+ * The xSPI Profile 1.0 table advertises the number of additional
+ * address bytes needed for Read Status Register command as 0 but the
+ * actual value for that is 4.
+ */
+ nor->params->rdsr_addr_nbytes = 4;
+}
+
+static int s28hs512t_post_bfpt_fixup(struct spi_nor *nor,
+ const struct sfdp_parameter_header *bfpt_header,
+ const struct sfdp_bfpt *bfpt,
+ struct spi_nor_flash_parameter *params)
+{
+ struct spi_mem_op op;
+ u8 *buf = nor->bouncebuf;
+ u8 addr_width = 3;
+ int ret;
+
+ /*
+ * The BFPT table advertises a 512B page size but the page size is
+ * actually configurable (with the default being 256B). Read from
+ * CFR3V[4] and set the correct size.
+ */
+ op = (struct spi_mem_op)
+ SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_RD_ANY_REG, 1),
+ SPI_MEM_OP_ADDR(addr_width, SPINOR_REG_CYPRESS_CFR3V, 1),
+ SPI_MEM_OP_NO_DUMMY,
+ SPI_MEM_OP_DATA_IN(1, buf, 1));
+ ret = spi_mem_exec_op(nor->spimem, &op);
+ if (ret)
+ return ret;
+
+ if (*buf & SPINOR_REG_CYPRESS_CFR3V_PGSZ)
+ params->page_size = 512;
+ else
+ params->page_size = 256;
+
+ return 0;
+}
+
+static struct spi_nor_fixups s28hs512t_fixups = {
+ .default_init = s28hs512t_default_init,
+ .post_sfdp = s28hs512t_post_sfdp_fixup,
+ .post_bfpt = s28hs512t_post_bfpt_fixup,
+};
+
static int
s25fs_s_post_bfpt_fixups(struct spi_nor *nor,
const struct sfdp_parameter_header *bfpt_header,
@@ -104,6 +265,11 @@ static const struct flash_info spansion_parts[] = {
SPI_NOR_4B_OPCODES) },
{ "cy15x104q", INFO6(0x042cc2, 0x7f7f7f, 512 * 1024, 1,
SPI_NOR_NO_ERASE) },
+ { "s28hs512t", INFO(0x345b1a, 0, 256 * 1024, 256,
+ SECT_4K | SPI_NOR_OCTAL_DTR_READ |
+ SPI_NOR_OCTAL_DTR_PP)
+ .fixups = &s28hs512t_fixups,
+ },
};
static void spansion_post_sfdp_fixups(struct spi_nor *nor)
--
2.28.0
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 49+ messages in thread
* Re: [PATCH v13 14/15] mtd: spi-nor: spansion: add support for Cypress Semper flash
2020-09-16 12:44 ` [PATCH v13 14/15] mtd: spi-nor: spansion: add support for Cypress Semper flash Pratyush Yadav
@ 2020-09-30 8:36 ` Tudor.Ambarus
2020-09-30 12:32 ` Pratyush Yadav
0 siblings, 1 reply; 49+ messages in thread
From: Tudor.Ambarus @ 2020-09-30 8:36 UTC (permalink / raw)
To: p.yadav, miquel.raynal, richard, vigneshr, linux-mtd, linux-kernel
Cc: boris.brezillon, nsekhar
On 9/16/20 3:44 PM, Pratyush Yadav wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> The Cypress Semper flash is an xSPI compliant octal DTR flash. Add
> support for using it in octal DTR mode.
>
> The flash by default boots in a hybrid sector mode. But the sector map
> table on the part I had was programmed incorrectly and the SMPT values
> on the flash don't match the public datasheet. Specifically, in some
> places erase type 3 was used instead of 4. In addition, the region sizes
> were incorrect in some places. So, for testing I set CFR3N[3] to enable
> uniform sector sizes. Since the uniform sector mode bit is a
> non-volatile bit, this series does not change it to avoid making any
> permanent changes to the flash configuration. The correct data to
> implement a fixup is not available right now and will be done in a
> follow-up patch if needed.
>
> Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
> ---
> drivers/mtd/spi-nor/spansion.c | 166 +++++++++++++++++++++++++++++++++
> 1 file changed, 166 insertions(+)
>
> diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
> index 8429b4af999a..a34e250ea5a2 100644
> --- a/drivers/mtd/spi-nor/spansion.c
> +++ b/drivers/mtd/spi-nor/spansion.c
> @@ -8,6 +8,167 @@
>
> #include "core.h"
>
> +#define SPINOR_OP_RD_ANY_REG 0x65 /* Read any register */
> +#define SPINOR_OP_WR_ANY_REG 0x71 /* Write any register */
> +#define SPINOR_REG_CYPRESS_CFR2V 0x00800003
> +#define SPINOR_REG_CYPRESS_CFR2V_MEMLAT_11_24 0xb
> +#define SPINOR_REG_CYPRESS_CFR3V 0x00800004
> +#define SPINOR_REG_CYPRESS_CFR3V_PGSZ BIT(4) /* Page size. */
> +#define SPINOR_REG_CYPRESS_CFR5V 0x00800006
> +#define SPINOR_REG_CYPRESS_CFR5V_OCT_DTR_EN 0x3
> +#define SPINOR_REG_CYPRESS_CFR5V_OCT_DTR_DS 0
> +#define SPINOR_OP_CYPRESS_RD_FAST 0xee
> +
> +/**
> + * spi_nor_cypress_octal_dtr_enable() - Enable octal DTR on Cypress flashes.
> + * @nor: pointer to a 'struct spi_nor'
> + * @enable: whether to enable or disable Octal DTR
> + *
> + * This also sets the memory access latency cycles to 24 to allow the flash to
> + * run at up to 200MHz.
> + *
> + * Return: 0 on success, -errno otherwise.
> + */
> +static int spi_nor_cypress_octal_dtr_enable(struct spi_nor *nor, bool enable)
> +{
> + struct spi_mem_op op;
> + u8 *buf = nor->bouncebuf;
> + u8 addr_width;
> + int ret;
> +
> + if (enable)
> + addr_width = 3;
> + else
> + addr_width = 4;
you can get rid of the addr_width variable and set the addr nbytes at op init
directly.
> +
> + if (enable) {
> + /* Use 24 dummy cycles for memory array reads. */
> + ret = spi_nor_write_enable(nor);
> + if (ret)
> + return ret;
> +
> + *buf = SPINOR_REG_CYPRESS_CFR2V_MEMLAT_11_24;
using nor->bouncebuf[0] makes the code easier to read (for me). No big deal anyway.
> + op = (struct spi_mem_op)
> + SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WR_ANY_REG, 1),
> + SPI_MEM_OP_ADDR(addr_width,
> + SPINOR_REG_CYPRESS_CFR2V,
> + 1),
> + SPI_MEM_OP_NO_DUMMY,
> + SPI_MEM_OP_DATA_OUT(1, buf, 1));
new line please
> + ret = spi_mem_exec_op(nor->spimem, &op);
> + if (ret) {
> + dev_warn(nor->dev,
> + "failed to set default memory latency value: %d\n",
> + ret);
do we really care for a message here? you'll get a dbg message from
spi_nor_octal_dtr_enable() too.
if you still want it, use dev_dbg please.
> + return ret;
> + }
new line please
> + ret = spi_nor_wait_till_ready(nor);
> + if (ret)
> + return ret;
> +
> + nor->read_dummy = 24;
> + }
> +
> + /* Set/unset the octal and DTR enable bits. */
> + ret = spi_nor_write_enable(nor);
> + if (ret)
> + return ret;
> +
> + if (enable)
> + *buf = SPINOR_REG_CYPRESS_CFR5V_OCT_DTR_EN;
> + else
> + *buf = SPINOR_REG_CYPRESS_CFR5V_OCT_DTR_DS;
> + op = (struct spi_mem_op)
> + SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WR_ANY_REG, 1),
> + SPI_MEM_OP_ADDR(addr_width,
> + SPINOR_REG_CYPRESS_CFR5V,
> + 1),
> + SPI_MEM_OP_NO_DUMMY,
> + SPI_MEM_OP_DATA_OUT(1, buf, 1));
> +
> + if (!enable)
> + spi_nor_spimem_setup_op(nor, &op, SNOR_PROTO_8_8_8_DTR);
> +
> + ret = spi_mem_exec_op(nor->spimem, &op);
> + if (ret) {
> + dev_warn(nor->dev, "Failed to enable octal DTR mode\n");
wrong error message for disable. Maybe get rid of it entirely and return exec_op
directly.
> + return ret;
> + }
> +
No need to wait here? you waited after SPINOR_REG_CYPRESS_CFR2V
> + return 0;
> +}
> +
> +static void s28hs512t_default_init(struct spi_nor *nor)
> +{
> + nor->params->octal_dtr_enable = spi_nor_cypress_octal_dtr_enable;
> +}
> +
> +static void s28hs512t_post_sfdp_fixup(struct spi_nor *nor)
> +{
> + /*
> + * On older versions of the flash the xSPI Profile 1.0 table has the
> + * 8D-8D-8D Fast Read opcode as 0x00. But it actually should be 0xEE.
> + */
> + if (nor->params->reads[SNOR_CMD_READ_8_8_8_DTR].opcode == 0)
> + nor->params->reads[SNOR_CMD_READ_8_8_8_DTR].opcode =
> + SPINOR_OP_CYPRESS_RD_FAST;
> +
> + /* This flash is also missing the 4-byte Page Program opcode bit. */
> + spi_nor_set_pp_settings(&nor->params->page_programs[SNOR_CMD_PP],
> + SPINOR_OP_PP_4B, SNOR_PROTO_1_1_1);
> + /*
> + * Since xSPI Page Program opcode is backward compatible with
> + * Legacy SPI, use Legacy SPI opcode there as well.
> + */
> + spi_nor_set_pp_settings(&nor->params->page_programs[SNOR_CMD_PP_8_8_8_DTR],
> + SPINOR_OP_PP_4B, SNOR_PROTO_8_8_8_DTR);
> +
> + /*
> + * The xSPI Profile 1.0 table advertises the number of additional
> + * address bytes needed for Read Status Register command as 0 but the
> + * actual value for that is 4.
> + */
> + nor->params->rdsr_addr_nbytes = 4;
> +}
> +
> +static int s28hs512t_post_bfpt_fixup(struct spi_nor *nor,
> + const struct sfdp_parameter_header *bfpt_header,
> + const struct sfdp_bfpt *bfpt,
> + struct spi_nor_flash_parameter *params)
> +{
> + struct spi_mem_op op;
> + u8 *buf = nor->bouncebuf;
> + u8 addr_width = 3;
> + int ret;
> +
> + /*
> + * The BFPT table advertises a 512B page size but the page size is
> + * actually configurable (with the default being 256B). Read from
> + * CFR3V[4] and set the correct size.
> + */
> + op = (struct spi_mem_op)
here you can get rid of the cast, as you have just one op.
> + SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_RD_ANY_REG, 1),
> + SPI_MEM_OP_ADDR(addr_width, SPINOR_REG_CYPRESS_CFR3V, 1),
set addr nbits to 3 directly
> + SPI_MEM_OP_NO_DUMMY,
> + SPI_MEM_OP_DATA_IN(1, buf, 1));
I would use nor->bouncebuf directly
new line please
> + ret = spi_mem_exec_op(nor->spimem, &op);
> + if (ret)
> + return ret;
> +
> + if (*buf & SPINOR_REG_CYPRESS_CFR3V_PGSZ)
> + params->page_size = 512;
> + else
> + params->page_size = 256;
> +
> + return 0;
> +}
> +
> +static struct spi_nor_fixups s28hs512t_fixups = {
> + .default_init = s28hs512t_default_init,
> + .post_sfdp = s28hs512t_post_sfdp_fixup,
> + .post_bfpt = s28hs512t_post_bfpt_fixup,
> +};
> +
> static int
> s25fs_s_post_bfpt_fixups(struct spi_nor *nor,
> const struct sfdp_parameter_header *bfpt_header,
> @@ -104,6 +265,11 @@ static const struct flash_info spansion_parts[] = {
> SPI_NOR_4B_OPCODES) },
> { "cy15x104q", INFO6(0x042cc2, 0x7f7f7f, 512 * 1024, 1,
> SPI_NOR_NO_ERASE) },
> + { "s28hs512t", INFO(0x345b1a, 0, 256 * 1024, 256,
> + SECT_4K | SPI_NOR_OCTAL_DTR_READ |
> + SPI_NOR_OCTAL_DTR_PP)
> + .fixups = &s28hs512t_fixups,
the .fixups init is not aligned with the other .fixups in the file. Is the alignment wrong?
looks good.
> + },
> };
>
> static void spansion_post_sfdp_fixups(struct spi_nor *nor)
> --
> 2.28.0
>
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v13 14/15] mtd: spi-nor: spansion: add support for Cypress Semper flash
2020-09-30 8:36 ` Tudor.Ambarus
@ 2020-09-30 12:32 ` Pratyush Yadav
0 siblings, 0 replies; 49+ messages in thread
From: Pratyush Yadav @ 2020-09-30 12:32 UTC (permalink / raw)
To: Tudor.Ambarus
Cc: vigneshr, richard, nsekhar, linux-kernel, boris.brezillon,
linux-mtd, miquel.raynal
On 30/09/20 08:36AM, Tudor.Ambarus@microchip.com wrote:
> On 9/16/20 3:44 PM, Pratyush Yadav wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >
> > The Cypress Semper flash is an xSPI compliant octal DTR flash. Add
> > support for using it in octal DTR mode.
> >
> > The flash by default boots in a hybrid sector mode. But the sector map
> > table on the part I had was programmed incorrectly and the SMPT values
> > on the flash don't match the public datasheet. Specifically, in some
> > places erase type 3 was used instead of 4. In addition, the region sizes
> > were incorrect in some places. So, for testing I set CFR3N[3] to enable
> > uniform sector sizes. Since the uniform sector mode bit is a
> > non-volatile bit, this series does not change it to avoid making any
> > permanent changes to the flash configuration. The correct data to
> > implement a fixup is not available right now and will be done in a
> > follow-up patch if needed.
> >
> > Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
> > ---
> > drivers/mtd/spi-nor/spansion.c | 166 +++++++++++++++++++++++++++++++++
> > 1 file changed, 166 insertions(+)
> >
> > diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
> > index 8429b4af999a..a34e250ea5a2 100644
> > --- a/drivers/mtd/spi-nor/spansion.c
> > +++ b/drivers/mtd/spi-nor/spansion.c
> > @@ -8,6 +8,167 @@
> >
> > #include "core.h"
> >
> > +#define SPINOR_OP_RD_ANY_REG 0x65 /* Read any register */
> > +#define SPINOR_OP_WR_ANY_REG 0x71 /* Write any register */
> > +#define SPINOR_REG_CYPRESS_CFR2V 0x00800003
> > +#define SPINOR_REG_CYPRESS_CFR2V_MEMLAT_11_24 0xb
> > +#define SPINOR_REG_CYPRESS_CFR3V 0x00800004
> > +#define SPINOR_REG_CYPRESS_CFR3V_PGSZ BIT(4) /* Page size. */
> > +#define SPINOR_REG_CYPRESS_CFR5V 0x00800006
> > +#define SPINOR_REG_CYPRESS_CFR5V_OCT_DTR_EN 0x3
> > +#define SPINOR_REG_CYPRESS_CFR5V_OCT_DTR_DS 0
> > +#define SPINOR_OP_CYPRESS_RD_FAST 0xee
> > +
> > +/**
> > + * spi_nor_cypress_octal_dtr_enable() - Enable octal DTR on Cypress flashes.
> > + * @nor: pointer to a 'struct spi_nor'
> > + * @enable: whether to enable or disable Octal DTR
> > + *
> > + * This also sets the memory access latency cycles to 24 to allow the flash to
> > + * run at up to 200MHz.
> > + *
> > + * Return: 0 on success, -errno otherwise.
> > + */
> > +static int spi_nor_cypress_octal_dtr_enable(struct spi_nor *nor, bool enable)
> > +{
> > + struct spi_mem_op op;
> > + u8 *buf = nor->bouncebuf;
> > + u8 addr_width;
> > + int ret;
> > +
> > + if (enable)
> > + addr_width = 3;
> > + else
> > + addr_width = 4;
>
> you can get rid of the addr_width variable and set the addr nbytes at op init
> directly.
Ok.
> > +
> > + if (enable) {
> > + /* Use 24 dummy cycles for memory array reads. */
> > + ret = spi_nor_write_enable(nor);
> > + if (ret)
> > + return ret;
> > +
> > + *buf = SPINOR_REG_CYPRESS_CFR2V_MEMLAT_11_24;
>
> using nor->bouncebuf[0] makes the code easier to read (for me). No big deal anyway.
>
> > + op = (struct spi_mem_op)
> > + SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WR_ANY_REG, 1),
> > + SPI_MEM_OP_ADDR(addr_width,
> > + SPINOR_REG_CYPRESS_CFR2V,
> > + 1),
> > + SPI_MEM_OP_NO_DUMMY,
> > + SPI_MEM_OP_DATA_OUT(1, buf, 1));
>
> new line please
Ok.
> > + ret = spi_mem_exec_op(nor->spimem, &op);
> > + if (ret) {
> > + dev_warn(nor->dev,
> > + "failed to set default memory latency value: %d\n",
> > + ret);
>
> do we really care for a message here? you'll get a dbg message from
> spi_nor_octal_dtr_enable() too.
>
> if you still want it, use dev_dbg please.
Ok. I'll use dev_dbg().
> > + return ret;
> > + }
>
> new line please
Ok.
> > + ret = spi_nor_wait_till_ready(nor);
> > + if (ret)
> > + return ret;
> > +
> > + nor->read_dummy = 24;
> > + }
> > +
> > + /* Set/unset the octal and DTR enable bits. */
> > + ret = spi_nor_write_enable(nor);
> > + if (ret)
> > + return ret;
> > +
> > + if (enable)
> > + *buf = SPINOR_REG_CYPRESS_CFR5V_OCT_DTR_EN;
> > + else
> > + *buf = SPINOR_REG_CYPRESS_CFR5V_OCT_DTR_DS;
> > + op = (struct spi_mem_op)
> > + SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WR_ANY_REG, 1),
> > + SPI_MEM_OP_ADDR(addr_width,
> > + SPINOR_REG_CYPRESS_CFR5V,
> > + 1),
> > + SPI_MEM_OP_NO_DUMMY,
> > + SPI_MEM_OP_DATA_OUT(1, buf, 1));
> > +
> > + if (!enable)
> > + spi_nor_spimem_setup_op(nor, &op, SNOR_PROTO_8_8_8_DTR);
> > +
> > + ret = spi_mem_exec_op(nor->spimem, &op);
> > + if (ret) {
> > + dev_warn(nor->dev, "Failed to enable octal DTR mode\n");
>
> wrong error message for disable. Maybe get rid of it entirely and return exec_op
> directly.
Ok.
> > + return ret;
> > + }
> > +
>
> No need to wait here? you waited after SPINOR_REG_CYPRESS_CFR2V
Yes, we need to wait but which mode do we poll the status register in?
If we poll it in 8D mode and the flash isn't done switching to it yet,
we will get garbage data which might falsely be interpreted as "done".
Maybe a simple udelay() will do the trick but the delay will have to be
a guess. I have no data on how long it takes to switch to 8D mode and I
have no way of measuring it either. I feel 500us should be enough, but
again I have no data to say for sure. Dunno...
> > + return 0;
> > +}
> > +
> > +static void s28hs512t_default_init(struct spi_nor *nor)
> > +{
> > + nor->params->octal_dtr_enable = spi_nor_cypress_octal_dtr_enable;
> > +}
> > +
> > +static void s28hs512t_post_sfdp_fixup(struct spi_nor *nor)
> > +{
> > + /*
> > + * On older versions of the flash the xSPI Profile 1.0 table has the
> > + * 8D-8D-8D Fast Read opcode as 0x00. But it actually should be 0xEE.
> > + */
> > + if (nor->params->reads[SNOR_CMD_READ_8_8_8_DTR].opcode == 0)
> > + nor->params->reads[SNOR_CMD_READ_8_8_8_DTR].opcode =
> > + SPINOR_OP_CYPRESS_RD_FAST;
> > +
> > + /* This flash is also missing the 4-byte Page Program opcode bit. */
> > + spi_nor_set_pp_settings(&nor->params->page_programs[SNOR_CMD_PP],
> > + SPINOR_OP_PP_4B, SNOR_PROTO_1_1_1);
> > + /*
> > + * Since xSPI Page Program opcode is backward compatible with
> > + * Legacy SPI, use Legacy SPI opcode there as well.
> > + */
> > + spi_nor_set_pp_settings(&nor->params->page_programs[SNOR_CMD_PP_8_8_8_DTR],
> > + SPINOR_OP_PP_4B, SNOR_PROTO_8_8_8_DTR);
> > +
> > + /*
> > + * The xSPI Profile 1.0 table advertises the number of additional
> > + * address bytes needed for Read Status Register command as 0 but the
> > + * actual value for that is 4.
> > + */
> > + nor->params->rdsr_addr_nbytes = 4;
> > +}
> > +
> > +static int s28hs512t_post_bfpt_fixup(struct spi_nor *nor,
> > + const struct sfdp_parameter_header *bfpt_header,
> > + const struct sfdp_bfpt *bfpt,
> > + struct spi_nor_flash_parameter *params)
> > +{
> > + struct spi_mem_op op;
> > + u8 *buf = nor->bouncebuf;
> > + u8 addr_width = 3;
> > + int ret;
> > +
> > + /*
> > + * The BFPT table advertises a 512B page size but the page size is
> > + * actually configurable (with the default being 256B). Read from
> > + * CFR3V[4] and set the correct size.
> > + */
> > + op = (struct spi_mem_op)
>
> here you can get rid of the cast, as you have just one op.
>
>
> > + SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_RD_ANY_REG, 1),
> > + SPI_MEM_OP_ADDR(addr_width, SPINOR_REG_CYPRESS_CFR3V, 1),
>
> set addr nbits to 3 directly
>
> > + SPI_MEM_OP_NO_DUMMY,
> > + SPI_MEM_OP_DATA_IN(1, buf, 1));
>
> I would use nor->bouncebuf directly
>
> new line please
Ok.
> > + ret = spi_mem_exec_op(nor->spimem, &op);
> > + if (ret)
> > + return ret;
> > +
> > + if (*buf & SPINOR_REG_CYPRESS_CFR3V_PGSZ)
> > + params->page_size = 512;
> > + else
> > + params->page_size = 256;
> > +
> > + return 0;
> > +}
> > +
> > +static struct spi_nor_fixups s28hs512t_fixups = {
> > + .default_init = s28hs512t_default_init,
> > + .post_sfdp = s28hs512t_post_sfdp_fixup,
> > + .post_bfpt = s28hs512t_post_bfpt_fixup,
> > +};
> > +
> > static int
> > s25fs_s_post_bfpt_fixups(struct spi_nor *nor,
> > const struct sfdp_parameter_header *bfpt_header,
> > @@ -104,6 +265,11 @@ static const struct flash_info spansion_parts[] = {
> > SPI_NOR_4B_OPCODES) },
> > { "cy15x104q", INFO6(0x042cc2, 0x7f7f7f, 512 * 1024, 1,
> > SPI_NOR_NO_ERASE) },
> > + { "s28hs512t", INFO(0x345b1a, 0, 256 * 1024, 256,
> > + SECT_4K | SPI_NOR_OCTAL_DTR_READ |
> > + SPI_NOR_OCTAL_DTR_PP)
> > + .fixups = &s28hs512t_fixups,
>
> the .fixups init is not aligned with the other .fixups in the file. Is the alignment wrong?
Ok. I'll fix the alignment.
> looks good.
>
> > + },
> > };
> >
> > static void spansion_post_sfdp_fixups(struct spi_nor *nor)
> > --
> > 2.28.0
> >
>
--
Regards,
Pratyush Yadav
Texas Instruments India
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH v13 15/15] mtd: spi-nor: micron-st: allow using MT35XU512ABA in Octal DTR mode
2020-09-16 12:44 [PATCH v13 00/15] mtd: spi-nor: add xSPI Octal DTR support Pratyush Yadav
` (13 preceding siblings ...)
2020-09-16 12:44 ` [PATCH v13 14/15] mtd: spi-nor: spansion: add support for Cypress Semper flash Pratyush Yadav
@ 2020-09-16 12:44 ` Pratyush Yadav
2020-09-30 9:12 ` Tudor.Ambarus
2020-09-29 9:59 ` [RFC PATCH 0/3] mtd: spi-nor: Tackle stateful modes Tudor Ambarus
2020-09-30 9:57 ` [PATCH v13 00/15] mtd: spi-nor: add xSPI Octal DTR support Tudor.Ambarus
16 siblings, 1 reply; 49+ messages in thread
From: Pratyush Yadav @ 2020-09-16 12:44 UTC (permalink / raw)
To: Tudor Ambarus, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, linux-mtd, linux-kernel
Cc: Boris Brezillon, Sekhar Nori, Pratyush Yadav
Since this flash doesn't have a Profile 1.0 table, the Octal DTR
capabilities are enabled in the post SFDP fixup, along with the 8D-8D-8D
fast read settings.
Enable Octal DTR mode with 20 dummy cycles to allow running at the
maximum supported frequency of 200Mhz.
The flash supports the soft reset sequence. So, add the flag in the
flash's info.
Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
---
drivers/mtd/spi-nor/micron-st.c | 102 +++++++++++++++++++++++++++++++-
1 file changed, 101 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/spi-nor/micron-st.c b/drivers/mtd/spi-nor/micron-st.c
index ef3695080710..cc02c1418d1d 100644
--- a/drivers/mtd/spi-nor/micron-st.c
+++ b/drivers/mtd/spi-nor/micron-st.c
@@ -8,10 +8,110 @@
#include "core.h"
+#define SPINOR_OP_MT_DTR_RD 0xfd /* Fast Read opcode in DTR mode */
+#define SPINOR_OP_MT_RD_ANY_REG 0x85 /* Read volatile register */
+#define SPINOR_OP_MT_WR_ANY_REG 0x81 /* Write volatile register */
+#define SPINOR_REG_MT_CFR0V 0x00 /* For setting octal DTR mode */
+#define SPINOR_REG_MT_CFR1V 0x01 /* For setting dummy cycles */
+#define SPINOR_MT_OCT_DTR 0xe7 /* Enable Octal DTR. */
+#define SPINOR_MT_EXSPI 0xff /* Enable Extended SPI (default) */
+
+static int spi_nor_micron_octal_dtr_enable(struct spi_nor *nor, bool enable)
+{
+ struct spi_mem_op op;
+ u8 *buf = nor->bouncebuf;
+ u8 addr_width;
+ int ret;
+
+ if (enable)
+ addr_width = 3;
+ else
+ addr_width = 4;
+
+ if (enable) {
+ /* Use 20 dummy cycles for memory array reads. */
+ ret = spi_nor_write_enable(nor);
+ if (ret)
+ return ret;
+
+ *buf = 20;
+ op = (struct spi_mem_op)
+ SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_MT_WR_ANY_REG, 1),
+ SPI_MEM_OP_ADDR(addr_width,
+ SPINOR_REG_MT_CFR1V, 1),
+ SPI_MEM_OP_NO_DUMMY,
+ SPI_MEM_OP_DATA_OUT(1, buf, 1));
+ ret = spi_mem_exec_op(nor->spimem, &op);
+ if (ret)
+ return ret;
+
+ ret = spi_nor_wait_till_ready(nor);
+ if (ret)
+ return ret;
+ }
+
+ ret = spi_nor_write_enable(nor);
+ if (ret)
+ return ret;
+
+ if (enable)
+ *buf = SPINOR_MT_OCT_DTR;
+ else
+ *buf = SPINOR_MT_EXSPI;
+ op = (struct spi_mem_op)
+ SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_MT_WR_ANY_REG, 1),
+ SPI_MEM_OP_ADDR(addr_width, SPINOR_REG_MT_CFR0V, 1),
+ SPI_MEM_OP_NO_DUMMY,
+ SPI_MEM_OP_DATA_OUT(1, buf, 1));
+
+ if (!enable)
+ spi_nor_spimem_setup_op(nor, &op, SNOR_PROTO_8_8_8_DTR);
+
+ ret = spi_mem_exec_op(nor->spimem, &op);
+ if (ret) {
+ dev_err(nor->dev, "Failed to enable octal DTR mode\n");
+ return ret;
+ }
+
+ return 0;
+}
+
+static void mt35xu512aba_default_init(struct spi_nor *nor)
+{
+ nor->params->octal_dtr_enable = spi_nor_micron_octal_dtr_enable;
+}
+
+static void mt35xu512aba_post_sfdp_fixup(struct spi_nor *nor)
+{
+ /* Set the Fast Read settings. */
+ nor->params->hwcaps.mask |= SNOR_HWCAPS_READ_8_8_8_DTR;
+ spi_nor_set_read_settings(&nor->params->reads[SNOR_CMD_READ_8_8_8_DTR],
+ 0, 20, SPINOR_OP_MT_DTR_RD,
+ SNOR_PROTO_8_8_8_DTR);
+
+ nor->cmd_ext_type = SPI_NOR_EXT_REPEAT;
+ nor->params->rdsr_dummy = 8;
+ nor->params->rdsr_addr_nbytes = 0;
+
+ /*
+ * The BFPT quad enable field is set to a reserved value so the quad
+ * enable function is ignored by spi_nor_parse_bfpt(). Make sure we
+ * disable it.
+ */
+ nor->params->quad_enable = NULL;
+}
+
+static struct spi_nor_fixups mt35xu512aba_fixups = {
+ .default_init = mt35xu512aba_default_init,
+ .post_sfdp = mt35xu512aba_post_sfdp_fixup,
+};
+
static const struct flash_info micron_parts[] = {
{ "mt35xu512aba", INFO(0x2c5b1a, 0, 128 * 1024, 512,
SECT_4K | USE_FSR | SPI_NOR_OCTAL_READ |
- SPI_NOR_4B_OPCODES) },
+ SPI_NOR_4B_OPCODES | SPI_NOR_OCTAL_DTR_READ |
+ SPI_NOR_OCTAL_DTR_PP)
+ .fixups = &mt35xu512aba_fixups},
{ "mt35xu02g", INFO(0x2c5b1c, 0, 128 * 1024, 2048,
SECT_4K | USE_FSR | SPI_NOR_OCTAL_READ |
SPI_NOR_4B_OPCODES) },
--
2.28.0
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 49+ messages in thread
* Re: [PATCH v13 15/15] mtd: spi-nor: micron-st: allow using MT35XU512ABA in Octal DTR mode
2020-09-16 12:44 ` [PATCH v13 15/15] mtd: spi-nor: micron-st: allow using MT35XU512ABA in Octal DTR mode Pratyush Yadav
@ 2020-09-30 9:12 ` Tudor.Ambarus
0 siblings, 0 replies; 49+ messages in thread
From: Tudor.Ambarus @ 2020-09-30 9:12 UTC (permalink / raw)
To: p.yadav, miquel.raynal, richard, vigneshr, linux-mtd, linux-kernel
Cc: boris.brezillon, nsekhar
On 9/16/20 3:44 PM, Pratyush Yadav wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> Since this flash doesn't have a Profile 1.0 table, the Octal DTR
> capabilities are enabled in the post SFDP fixup, along with the 8D-8D-8D
> fast read settings.
>
> Enable Octal DTR mode with 20 dummy cycles to allow running at the
> maximum supported frequency of 200Mhz.
>
> The flash supports the soft reset sequence. So, add the flag in the
> flash's info.
>
> Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
> ---
> drivers/mtd/spi-nor/micron-st.c | 102 +++++++++++++++++++++++++++++++-
> 1 file changed, 101 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mtd/spi-nor/micron-st.c b/drivers/mtd/spi-nor/micron-st.c
> index ef3695080710..cc02c1418d1d 100644
> --- a/drivers/mtd/spi-nor/micron-st.c
> +++ b/drivers/mtd/spi-nor/micron-st.c
> @@ -8,10 +8,110 @@
similar comments to those in 14/15 apply here as well.
Looks good. I couldn't find datasheets for neither of flashes though.
Cheers,
ta
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 49+ messages in thread
* [RFC PATCH 0/3] mtd: spi-nor: Tackle stateful modes
2020-09-16 12:44 [PATCH v13 00/15] mtd: spi-nor: add xSPI Octal DTR support Pratyush Yadav
` (14 preceding siblings ...)
2020-09-16 12:44 ` [PATCH v13 15/15] mtd: spi-nor: micron-st: allow using MT35XU512ABA in Octal DTR mode Pratyush Yadav
@ 2020-09-29 9:59 ` Tudor Ambarus
2020-09-29 9:59 ` [RFC PATCH 1/3] mtd: spi-nor: Introduce SNOR_F_IO_MODE_EN_VOLATILE Tudor Ambarus
` (2 more replies)
2020-09-30 9:57 ` [PATCH v13 00/15] mtd: spi-nor: add xSPI Octal DTR support Tudor.Ambarus
16 siblings, 3 replies; 49+ messages in thread
From: Tudor Ambarus @ 2020-09-29 9:59 UTC (permalink / raw)
To: p.yadav, vigneshr, boris.brezillon; +Cc: linux-mtd, linux-kernel, Tudor Ambarus
My biggest concern with Pratyush's patches is that the stateful modes case
(X-X-X modes that are entered via a non-volatile bit) is not handled.
This is an attempt to tackle this problem. Reasons and explanations in
the commit messages.
Tudor Ambarus (3):
mtd: spi-nor: Introduce SNOR_F_IO_MODE_EN_VOLATILE
mtd: spi-nor: Introduce MTD_SPI_NOR_ALLOW_STATEFUL_MODES
mtd: spi-nor: Parse SFDP SCCR Map
drivers/mtd/spi-nor/Kconfig | 10 +++++++
drivers/mtd/spi-nor/core.c | 8 ++++++
drivers/mtd/spi-nor/core.h | 6 +++++
drivers/mtd/spi-nor/sfdp.c | 52 +++++++++++++++++++++++++++++++++++++
4 files changed, 76 insertions(+)
--
2.25.1
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 49+ messages in thread
* [RFC PATCH 1/3] mtd: spi-nor: Introduce SNOR_F_IO_MODE_EN_VOLATILE
2020-09-29 9:59 ` [RFC PATCH 0/3] mtd: spi-nor: Tackle stateful modes Tudor Ambarus
@ 2020-09-29 9:59 ` Tudor Ambarus
2020-09-29 16:45 ` Vignesh Raghavendra
2020-09-29 9:59 ` [RFC PATCH 2/3] mtd: spi-nor: Introduce MTD_SPI_NOR_ALLOW_STATEFUL_MODES Tudor Ambarus
2020-09-29 9:59 ` [RFC PATCH 3/3] mtd: spi-nor: Parse SFDP SCCR Map Tudor Ambarus
2 siblings, 1 reply; 49+ messages in thread
From: Tudor Ambarus @ 2020-09-29 9:59 UTC (permalink / raw)
To: p.yadav, vigneshr, boris.brezillon; +Cc: linux-mtd, linux-kernel, Tudor Ambarus
We don't want to enter a stateful mode, where a X-X-X I/O mode
is entered by setting a non-volatile bit, because in case of a
reset or a crash, once in the non-volatile mode, we may not be able
to recover in bootloaders and we may break the SPI NOR boot.
Forbid by default the I/O modes that are set via a non-volatile bit.
SPI_NOR_IO_MODE_EN_VOLATILE should be set just for the flashes that
don't define the optional SFDP SCCR Map, so that we don't pollute the
flash info flags.
Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
drivers/mtd/spi-nor/core.c | 6 ++++++
drivers/mtd/spi-nor/core.h | 6 ++++++
2 files changed, 12 insertions(+)
diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 34edfcf33172..c149b318e2e8 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -3089,6 +3089,9 @@ static int spi_nor_octal_dtr_enable(struct spi_nor *nor, bool enable)
nor->write_proto == SNOR_PROTO_8_8_8_DTR))
return 0;
+ if (!(nor->flags & SNOR_F_IO_MODE_EN_VOLATILE))
+ return 0;
+
ret = nor->params->octal_dtr_enable(nor, enable);
if (ret)
return ret;
@@ -3474,6 +3477,9 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
if (info->flags & SPI_NOR_4B_OPCODES)
nor->flags |= SNOR_F_4B_OPCODES;
+ if (info->flags & SPI_NOR_IO_MODE_EN_VOLATILE)
+ nor->flags |= SNOR_F_IO_MODE_EN_VOLATILE;
+
ret = spi_nor_set_addr_width(nor);
if (ret)
return ret;
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index 10dc03506f93..c2755c82f978 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -27,6 +27,7 @@ enum spi_nor_option_flags {
SNOR_F_HAS_4BIT_BP = BIT(12),
SNOR_F_HAS_SR_BP3_BIT6 = BIT(13),
SNOR_F_SOFT_RESET = BIT(14),
+ SNOR_F_IO_MODE_EN_VOLATILE = BIT(15),
};
struct spi_nor_read_command {
@@ -323,6 +324,11 @@ struct flash_info {
*/
#define SPI_NOR_OCTAL_DTR_READ BIT(19) /* Flash supports octal DTR Read. */
#define SPI_NOR_OCTAL_DTR_PP BIT(20) /* Flash supports Octal DTR Page Program */
+#define SPI_NOR_IO_MODE_EN_VOLATILE BIT(21) /*
+ * Flash enables the best
+ * available I/O mode via a
+ * volatile bit.
+ */
/* Part specific fixup hooks. */
const struct spi_nor_fixups *fixups;
--
2.25.1
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 49+ messages in thread
* Re: [RFC PATCH 1/3] mtd: spi-nor: Introduce SNOR_F_IO_MODE_EN_VOLATILE
2020-09-29 9:59 ` [RFC PATCH 1/3] mtd: spi-nor: Introduce SNOR_F_IO_MODE_EN_VOLATILE Tudor Ambarus
@ 2020-09-29 16:45 ` Vignesh Raghavendra
0 siblings, 0 replies; 49+ messages in thread
From: Vignesh Raghavendra @ 2020-09-29 16:45 UTC (permalink / raw)
To: Tudor Ambarus, p.yadav, boris.brezillon; +Cc: linux-mtd, linux-kernel
On 9/29/20 3:29 PM, Tudor Ambarus wrote:
> We don't want to enter a stateful mode, where a X-X-X I/O mode
> is entered by setting a non-volatile bit, because in case of a
> reset or a crash, once in the non-volatile mode, we may not be able
> to recover in bootloaders and we may break the SPI NOR boot.
>
> Forbid by default the I/O modes that are set via a non-volatile bit.
>
> SPI_NOR_IO_MODE_EN_VOLATILE should be set just for the flashes that
> don't define the optional SFDP SCCR Map, so that we don't pollute the
> flash info flags.
>
I would make this SPI_NOR_IO_MODE_EN_NON_VOLATILE. Because flashes with
volatile bits to change IO modes are more common than non volatile ones
and above change makes its easier to spot such quirky flashes.
> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> ---
> drivers/mtd/spi-nor/core.c | 6 ++++++
> drivers/mtd/spi-nor/core.h | 6 ++++++
> 2 files changed, 12 insertions(+)
>
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index 34edfcf33172..c149b318e2e8 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -3089,6 +3089,9 @@ static int spi_nor_octal_dtr_enable(struct spi_nor *nor, bool enable)
> nor->write_proto == SNOR_PROTO_8_8_8_DTR))
> return 0;
>
> + if (!(nor->flags & SNOR_F_IO_MODE_EN_VOLATILE))
> + return 0;
> +
> ret = nor->params->octal_dtr_enable(nor, enable);
> if (ret)
> return ret;
> @@ -3474,6 +3477,9 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
> if (info->flags & SPI_NOR_4B_OPCODES)
> nor->flags |= SNOR_F_4B_OPCODES;
>
> + if (info->flags & SPI_NOR_IO_MODE_EN_VOLATILE)
> + nor->flags |= SNOR_F_IO_MODE_EN_VOLATILE;
> +
> ret = spi_nor_set_addr_width(nor);
> if (ret)
> return ret;
> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
> index 10dc03506f93..c2755c82f978 100644
> --- a/drivers/mtd/spi-nor/core.h
> +++ b/drivers/mtd/spi-nor/core.h
> @@ -27,6 +27,7 @@ enum spi_nor_option_flags {
> SNOR_F_HAS_4BIT_BP = BIT(12),
> SNOR_F_HAS_SR_BP3_BIT6 = BIT(13),
> SNOR_F_SOFT_RESET = BIT(14),
> + SNOR_F_IO_MODE_EN_VOLATILE = BIT(15),
> };
>
> struct spi_nor_read_command {
> @@ -323,6 +324,11 @@ struct flash_info {
> */
> #define SPI_NOR_OCTAL_DTR_READ BIT(19) /* Flash supports octal DTR Read. */
> #define SPI_NOR_OCTAL_DTR_PP BIT(20) /* Flash supports Octal DTR Page Program */
> +#define SPI_NOR_IO_MODE_EN_VOLATILE BIT(21) /*
> + * Flash enables the best
> + * available I/O mode via a
> + * volatile bit.
> + */
>
> /* Part specific fixup hooks. */
> const struct spi_nor_fixups *fixups;
>
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 49+ messages in thread
* [RFC PATCH 2/3] mtd: spi-nor: Introduce MTD_SPI_NOR_ALLOW_STATEFUL_MODES
2020-09-29 9:59 ` [RFC PATCH 0/3] mtd: spi-nor: Tackle stateful modes Tudor Ambarus
2020-09-29 9:59 ` [RFC PATCH 1/3] mtd: spi-nor: Introduce SNOR_F_IO_MODE_EN_VOLATILE Tudor Ambarus
@ 2020-09-29 9:59 ` Tudor Ambarus
2020-09-29 16:45 ` Vignesh Raghavendra
2020-09-29 9:59 ` [RFC PATCH 3/3] mtd: spi-nor: Parse SFDP SCCR Map Tudor Ambarus
2 siblings, 1 reply; 49+ messages in thread
From: Tudor Ambarus @ 2020-09-29 9:59 UTC (permalink / raw)
To: p.yadav, vigneshr, boris.brezillon; +Cc: linux-mtd, linux-kernel, Tudor Ambarus
Some users may teach their bootloaders to discover and recover a
flash even when left in a statefull mode (a X-X-X I/O mode that is
configured via a non-volatile bit).
Provide a way for those users to enter in stateful modes. A reset
or a crash will leave the flash in full I/O mode and if the bootloader
does not know how to recover, the SPI NOR boot will be broken.
Flashes that will enable stateful modes will be accepted only if a
hook to recover from the stateful mode is provided in the kernel.
With this, even if a user will break its SPI NOR boot, it'll be able
to recover the flash at the kernel level (on those systems that have
at least another boot media). Both the Kconfig and the acceptance
restriction are needed, so that we don't end up completely hopeless
and look at a flash for which there is no software to discover and
recover the flash. Even if we can recover the flash from a stateful
mode in kernel, entering the stateful mode is still dangerous if one's
bootloader can't handle it. We need a way to pass the responsibility
to the user and let him decide conciously about the risks of allowing
stateful modes.
Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
drivers/mtd/spi-nor/Kconfig | 10 ++++++++++
drivers/mtd/spi-nor/core.c | 2 ++
2 files changed, 12 insertions(+)
diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
index ffc4b380f2b1..ab62457559b2 100644
--- a/drivers/mtd/spi-nor/Kconfig
+++ b/drivers/mtd/spi-nor/Kconfig
@@ -24,6 +24,16 @@ config MTD_SPI_NOR_USE_4K_SECTORS
Please note that some tools/drivers/filesystems may not work with
4096 B erase size (e.g. UBIFS requires 15 KiB as a minimum).
+config MTD_SPI_NOR_ALLOW_STATEFUL_MODES
+ bool "Allow stateful modes (DANGEROUS)"
+ help
+ Allow the flash to enter in full I/O mode via a non-volatile bit.
+ A reset or a crash will leave the flash in the full I/O mode and if
+ the bootloader does not know how to recover, the SPI NOR boot will be
+ broken.
+
+ Say N, unless you absolutely know what you are doing.
+
source "drivers/mtd/spi-nor/controllers/Kconfig"
endif # MTD_SPI_NOR
diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index c149b318e2e8..e89c3ea9a736 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -3089,8 +3089,10 @@ static int spi_nor_octal_dtr_enable(struct spi_nor *nor, bool enable)
nor->write_proto == SNOR_PROTO_8_8_8_DTR))
return 0;
+#ifndef CONFIG_MTD_SPI_NOR_ALLOW_STATEFUL_MODES
if (!(nor->flags & SNOR_F_IO_MODE_EN_VOLATILE))
return 0;
+#endif
ret = nor->params->octal_dtr_enable(nor, enable);
if (ret)
--
2.25.1
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 49+ messages in thread
* Re: [RFC PATCH 2/3] mtd: spi-nor: Introduce MTD_SPI_NOR_ALLOW_STATEFUL_MODES
2020-09-29 9:59 ` [RFC PATCH 2/3] mtd: spi-nor: Introduce MTD_SPI_NOR_ALLOW_STATEFUL_MODES Tudor Ambarus
@ 2020-09-29 16:45 ` Vignesh Raghavendra
0 siblings, 0 replies; 49+ messages in thread
From: Vignesh Raghavendra @ 2020-09-29 16:45 UTC (permalink / raw)
To: Tudor Ambarus, p.yadav, boris.brezillon; +Cc: linux-mtd, linux-kernel
On 9/29/20 3:29 PM, Tudor Ambarus wrote:
> Some users may teach their bootloaders to discover and recover a
> flash even when left in a statefull mode (a X-X-X I/O mode that is
> configured via a non-volatile bit).
>
> Provide a way for those users to enter in stateful modes. A reset
> or a crash will leave the flash in full I/O mode and if the bootloader
> does not know how to recover, the SPI NOR boot will be broken.
>
> Flashes that will enable stateful modes will be accepted only if a
> hook to recover from the stateful mode is provided in the kernel.
> With this, even if a user will break its SPI NOR boot, it'll be able
> to recover the flash at the kernel level (on those systems that have
> at least another boot media). Both the Kconfig and the acceptance
> restriction are needed, so that we don't end up completely hopeless
> and look at a flash for which there is no software to discover and
> recover the flash. Even if we can recover the flash from a stateful
> mode in kernel, entering the stateful mode is still dangerous if one's
> bootloader can't handle it. We need a way to pass the responsibility
> to the user and let him decide conciously about the risks of allowing
> stateful modes.
>
Recovering from non-volatile (NV) stateful mode would mean unsetting
some NV bit. Doing this for every boot would mean NV bit will wear out
quite quickly which is a concern. And sometimes NV Octal Enable bits are
OTP only (Some Macronix flashes).
NV bits are not to be fiddled with too often. One would set stateful
mode in NV way only if entire system is capable of handling this
somehow. But, IMHO, since SPI NOR core currently does not support
flashes that boot in Quad or Octal mode at the moment, SPI NOR core
should not bother providing a hook for manipulating NV IO mode settings.
So my recommendation is to not support writing to non volatile IO modes
bits unless absolutely necessary.
Regards
Vignesh
> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> ---
> drivers/mtd/spi-nor/Kconfig | 10 ++++++++++
> drivers/mtd/spi-nor/core.c | 2 ++
> 2 files changed, 12 insertions(+)
>
> diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
> index ffc4b380f2b1..ab62457559b2 100644
> --- a/drivers/mtd/spi-nor/Kconfig
> +++ b/drivers/mtd/spi-nor/Kconfig
> @@ -24,6 +24,16 @@ config MTD_SPI_NOR_USE_4K_SECTORS
> Please note that some tools/drivers/filesystems may not work with
> 4096 B erase size (e.g. UBIFS requires 15 KiB as a minimum).
>
> +config MTD_SPI_NOR_ALLOW_STATEFUL_MODES
> + bool "Allow stateful modes (DANGEROUS)"
> + help
> + Allow the flash to enter in full I/O mode via a non-volatile bit.
> + A reset or a crash will leave the flash in the full I/O mode and if
> + the bootloader does not know how to recover, the SPI NOR boot will be
> + broken.
> +
> + Say N, unless you absolutely know what you are doing.
> +
> source "drivers/mtd/spi-nor/controllers/Kconfig"
>
> endif # MTD_SPI_NOR
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index c149b318e2e8..e89c3ea9a736 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -3089,8 +3089,10 @@ static int spi_nor_octal_dtr_enable(struct spi_nor *nor, bool enable)
> nor->write_proto == SNOR_PROTO_8_8_8_DTR))
> return 0;
>
> +#ifndef CONFIG_MTD_SPI_NOR_ALLOW_STATEFUL_MODES
> if (!(nor->flags & SNOR_F_IO_MODE_EN_VOLATILE))
> return 0;
> +#endif
>
> ret = nor->params->octal_dtr_enable(nor, enable);
> if (ret)
>
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 49+ messages in thread
* [RFC PATCH 3/3] mtd: spi-nor: Parse SFDP SCCR Map
2020-09-29 9:59 ` [RFC PATCH 0/3] mtd: spi-nor: Tackle stateful modes Tudor Ambarus
2020-09-29 9:59 ` [RFC PATCH 1/3] mtd: spi-nor: Introduce SNOR_F_IO_MODE_EN_VOLATILE Tudor Ambarus
2020-09-29 9:59 ` [RFC PATCH 2/3] mtd: spi-nor: Introduce MTD_SPI_NOR_ALLOW_STATEFUL_MODES Tudor Ambarus
@ 2020-09-29 9:59 ` Tudor Ambarus
2 siblings, 0 replies; 49+ messages in thread
From: Tudor Ambarus @ 2020-09-29 9:59 UTC (permalink / raw)
To: p.yadav, vigneshr, boris.brezillon; +Cc: linux-mtd, linux-kernel, Tudor Ambarus
Parse just the 22nd dword and look for the 'DTR Octal Mode Enable
Volatile bit'.
SPI_NOR_IO_MODE_EN_VOLATILE should be set just for the flashes
that don't define the optional SFDP SCCR Map. For the others,
let the SFDP do its job and fill the SNOR_F_IO_MODE_EN_VOLATILE
flag. We avoid this way polluting the flash flags when declaring
one.
Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
drivers/mtd/spi-nor/sfdp.c | 52 ++++++++++++++++++++++++++++++++++++++
1 file changed, 52 insertions(+)
diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
index f192710aca31..7bca64cbba02 100644
--- a/drivers/mtd/spi-nor/sfdp.c
+++ b/drivers/mtd/spi-nor/sfdp.c
@@ -21,6 +21,10 @@
#define SFDP_SECTOR_MAP_ID 0xff81 /* Sector Map Table */
#define SFDP_4BAIT_ID 0xff84 /* 4-byte Address Instruction Table */
#define SFDP_PROFILE1_ID 0xff05 /* xSPI Profile 1.0 table. */
+#define SFDP_SCCR_MAP_ID 0xff87 /*
+ * Status, Control and Configuration
+ * Register Map.
+ */
#define SFDP_SIGNATURE 0x50444653U
@@ -1199,6 +1203,50 @@ static int spi_nor_parse_profile1(struct spi_nor *nor,
return ret;
}
+#define SCCR_DWORD22_OCTAL_DTR_EN_VOLATILE BIT(31)
+
+/**
+ * spi_nor_parse_sccr() - Parse the Status, Control and Configuration Register
+ * Map.
+ * @nor: pointer to a 'struct spi_nor'
+ * @sccr_header: pointer to the 'struct sfdp_parameter_header' describing
+ * the SCCR Map table length and version.
+ * @params: pointer to the 'struct spi_nor_flash_parameter' to be.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int spi_nor_parse_sccr(struct spi_nor *nor,
+ const struct sfdp_parameter_header *sccr_header,
+ struct spi_nor_flash_parameter *params)
+{
+ u32 *dwords, addr;
+ size_t len;
+ int ret;
+ u8 io_mode_en_volatile;
+
+ len = sccr_header->length * sizeof(*dwords);
+ dwords = kmalloc(len, GFP_KERNEL);
+ if (!dwords)
+ return -ENOMEM;
+
+ addr = SFDP_PARAM_HEADER_PTP(sccr_header);
+ ret = spi_nor_read_sfdp(nor, addr, len, dwords);
+ if (ret)
+ goto out;
+
+ le32_to_cpu_array(dwords, sccr_header->length);
+
+ io_mode_en_volatile = FIELD_GET(SCCR_DWORD22_OCTAL_DTR_EN_VOLATILE,
+ dwords[22]);
+
+ if (io_mode_en_volatile)
+ nor->flags |= SNOR_F_IO_MODE_EN_VOLATILE;
+
+out:
+ kfree(dwords);
+ return ret;
+}
+
/**
* spi_nor_parse_sfdp() - parse the Serial Flash Discoverable Parameters.
* @nor: pointer to a 'struct spi_nor'
@@ -1304,6 +1352,10 @@ int spi_nor_parse_sfdp(struct spi_nor *nor,
err = spi_nor_parse_profile1(nor, param_header, params);
break;
+ case SFDP_SCCR_MAP_ID:
+ err = spi_nor_parse_sccr(nor, param_header, params);
+ break;
+
default:
break;
}
--
2.25.1
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 49+ messages in thread
* Re: [PATCH v13 00/15] mtd: spi-nor: add xSPI Octal DTR support
2020-09-16 12:44 [PATCH v13 00/15] mtd: spi-nor: add xSPI Octal DTR support Pratyush Yadav
` (15 preceding siblings ...)
2020-09-29 9:59 ` [RFC PATCH 0/3] mtd: spi-nor: Tackle stateful modes Tudor Ambarus
@ 2020-09-30 9:57 ` Tudor.Ambarus
2020-09-30 12:01 ` Pratyush Yadav
16 siblings, 1 reply; 49+ messages in thread
From: Tudor.Ambarus @ 2020-09-30 9:57 UTC (permalink / raw)
To: p.yadav, miquel.raynal, richard, vigneshr, linux-mtd, linux-kernel
Cc: boris.brezillon, nsekhar
On 9/16/20 3:44 PM, Pratyush Yadav wrote:
> Hi,
Hello,
>
> This series adds support for Octal DTR flashes in the SPI NOR framework,
> and then adds hooks for the Cypress Semper and Micron Xcella flashes to
> allow running them in Octal DTR mode. This series assumes that the flash
> is handed to the kernel in Legacy SPI mode.
>
I like this series. There are some comments that can be addressed, no big
deal though.
I think that we shouldn't let the door open for users with flashes that
enter in a X-X-X mode in a non-volatile way. Think of two flashes that have
the same X-X-X mode enable sequence, but in which only the EN bit differs:
for one the EN bit is volatile and for the other it is non-volatile. Users
of the later flash that try to enable the X-X-X mode (using our code) will
end up with the flash in a mode from which they can't recover. Thus my advice
is to consider by default all the flashes, as X-X-X mode non-volatile flashes,
and to not let them use the X-X-X mode enable methods. Flashes that can enter
X-X-X modes in a volatile way, should discover this capability by parsing the
optional SFDP SCCR Map. Those that don't define this table, should pass this
capability as a flash_info flag when declaring the flash. With these, users
should be conscious about the V or NV modes, and the risk to end up with
flashes for which there is no software to recover is diminished. This is what
I tried in RFC 1/3 and RFC 3/3. I think those 2 patches should be part of
this series. 14/15 and 15/15 should be updated accordingly. RFC 2/3 has room
for discussions because it provides access system-wise, while ideally would be
to do it at flash granularity. I'll wait for your feedback on those.
> Tested on TI J721E EVM with 1-bit ECC on the Cypress flash.
As a tip, when introducing some big changes to the core, would be nice to be
assured that things that worked previously are still working now.
An erase-write-read-back test in Quad SPI would suffice. Probably you have
already did that, but I haven't seen it mentioned.
Cheers,
ta
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH v13 00/15] mtd: spi-nor: add xSPI Octal DTR support
2020-09-30 9:57 ` [PATCH v13 00/15] mtd: spi-nor: add xSPI Octal DTR support Tudor.Ambarus
@ 2020-09-30 12:01 ` Pratyush Yadav
0 siblings, 0 replies; 49+ messages in thread
From: Pratyush Yadav @ 2020-09-30 12:01 UTC (permalink / raw)
To: Tudor.Ambarus
Cc: vigneshr, richard, nsekhar, linux-kernel, boris.brezillon,
linux-mtd, miquel.raynal
On 30/09/20 09:57AM, Tudor.Ambarus@microchip.com wrote:
> On 9/16/20 3:44 PM, Pratyush Yadav wrote:
> > Hi,
>
> Hello,
>
> >
> > This series adds support for Octal DTR flashes in the SPI NOR framework,
> > and then adds hooks for the Cypress Semper and Micron Xcella flashes to
> > allow running them in Octal DTR mode. This series assumes that the flash
> > is handed to the kernel in Legacy SPI mode.
> >
>
> I like this series. There are some comments that can be addressed, no big
> deal though.
>
> I think that we shouldn't let the door open for users with flashes that
> enter in a X-X-X mode in a non-volatile way. Think of two flashes that have
> the same X-X-X mode enable sequence, but in which only the EN bit differs:
> for one the EN bit is volatile and for the other it is non-volatile. Users
> of the later flash that try to enable the X-X-X mode (using our code) will
> end up with the flash in a mode from which they can't recover. Thus my advice
> is to consider by default all the flashes, as X-X-X mode non-volatile flashes,
> and to not let them use the X-X-X mode enable methods. Flashes that can enter
> X-X-X modes in a volatile way, should discover this capability by parsing the
> optional SFDP SCCR Map. Those that don't define this table, should pass this
> capability as a flash_info flag when declaring the flash. With these, users
> should be conscious about the V or NV modes, and the risk to end up with
> flashes for which there is no software to recover is diminished. This is what
> I tried in RFC 1/3 and RFC 3/3. I think those 2 patches should be part of
> this series. 14/15 and 15/15 should be updated accordingly. RFC 2/3 has room
> for discussions because it provides access system-wise, while ideally would be
> to do it at flash granularity. I'll wait for your feedback on those.
FWIW, I took a quick look at the RFC patches and I agree that patches
1/3 and 3/3 are good and I'll add them to my series. I don't like 2/3
mainly because of the same concerns that Vignesh has: wearing down the
NV bits. Also IMO we should avoid touching NV bits as much as we can
because I had used some on the Cypress flash during development, and I
ended up "bricking" the flash a few times (for example when I used the
wrong dummy cycles by mistake or the controller driver did something
wrong). Since I knew exactly which registers I was touching I could
recover it but the process is quite painful.
>
> > Tested on TI J721E EVM with 1-bit ECC on the Cypress flash.
>
> As a tip, when introducing some big changes to the core, would be nice to be
> assured that things that worked previously are still working now.
> An erase-write-read-back test in Quad SPI would suffice. Probably you have
> already did that, but I haven't seen it mentioned.
Ok, I'll include them next time. I tested this series on the CQSPI and
TI QSPI controllers on MT25Q, S25FL for regressions, and on MT35 and S28
for Octal DTR.
--
Regards,
Pratyush Yadav
Texas Instruments India
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 49+ messages in thread