Re: [PATCH v13 14/15] mtd: spi-nor: spansion: add support for Cypress Semper flash
On 30/09/20 08:36AM, tudor.amba...@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
> > ---
> > 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 0x0083
> > +#define SPINOR_REG_CYPRESS_CFR2V_MEMLAT_11_24 0xb
> > +#define SPINOR_REG_CYPRESS_CFR3V 0x0084
> > +#define SPINOR_REG_CYPRESS_CFR3V_PGSZ BIT(4) /* Page size. */
> > +#define SPINOR_REG_CYPRESS_CFR5V 0x0086
> > +#define SPINOR_REG_CYPRESS_CFR5V_OCT_DTR_EN0x3
> > +#define SPINOR_REG_CYPRESS_CFR5V_OCT_DTR_DS0
> > +#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, );
> > + 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)
> > +
Re: [PATCH v13 14/15] mtd: spi-nor: spansion: add support for Cypress Semper flash
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
> ---
> 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 0x0083
> +#define SPINOR_REG_CYPRESS_CFR2V_MEMLAT_11_24 0xb
> +#define SPINOR_REG_CYPRESS_CFR3V 0x0084
> +#define SPINOR_REG_CYPRESS_CFR3V_PGSZ BIT(4) /* Page size. */
> +#define SPINOR_REG_CYPRESS_CFR5V 0x0086
> +#define SPINOR_REG_CYPRESS_CFR5V_OCT_DTR_EN0x3
> +#define SPINOR_REG_CYPRESS_CFR5V_OCT_DTR_DS0
> +#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, );
> + 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)
> +
[PATCH v13 14/15] mtd: spi-nor: spansion: add support for Cypress Semper flash
The Cypress Semper flash is an xSPI compliant octal DTR flash. Add
support for using it in octal DTR mode.
The flash by default boots in a hybrid sector mode. But the sector map
table on the part I had was programmed incorrectly and the SMPT values
on the flash don't match the public datasheet. Specifically, in some
places erase type 3 was used instead of 4. In addition, the region sizes
were incorrect in some places. So, for testing I set CFR3N[3] to enable
uniform sector sizes. Since the uniform sector mode bit is a
non-volatile bit, this series does not change it to avoid making any
permanent changes to the flash configuration. The correct data to
implement a fixup is not available right now and will be done in a
follow-up patch if needed.
Signed-off-by: Pratyush Yadav
---
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 0x0083
+#define SPINOR_REG_CYPRESS_CFR2V_MEMLAT_11_24 0xb
+#define SPINOR_REG_CYPRESS_CFR3V 0x0084
+#define SPINOR_REG_CYPRESS_CFR3V_PGSZ BIT(4) /* Page size. */
+#define SPINOR_REG_CYPRESS_CFR5V 0x0086
+#define SPINOR_REG_CYPRESS_CFR5V_OCT_DTR_EN0x3
+#define SPINOR_REG_CYPRESS_CFR5V_OCT_DTR_DS0
+#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, );
+ 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, , SNOR_PROTO_8_8_8_DTR);
+
+ ret = spi_mem_exec_op(nor->spimem, );
+ 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)
+
