[PATCH v2 1/2] mtd: spi-nor: add Octal DTR support for Macronix flash
liao jaime
jaimeliao.tw at gmail.com
Tue Aug 1 02:33:44 PDT 2023
Hi Tudor
>
>
>
> On 8/1/23 08:24, liao jaime wrote:
> > Hi Tudor
> >
> >>
> >>
> >>
> >> On 7/27/23 10:16, Jaime Liao wrote:
> >>> From: JaimeLiao <jaimeliao at mxic.com.tw>
> >>>
> >>> Create Macronix specify method for enable Octal DTR mode and
> >>> set 20 dummy cycles to allow running at the maximum supported
> >>> frequency for Macronix Octal flash.
> >>
> >> 20 dummy cycles is particular for a specific operating frequency.
> >>
> >> If you test the flash at different speeds, let's say at 30 MHz, and
> >> at their highest frequency (133, 200 MHZ?) does the flash work with
> >> current settings?
> > Yes, datasheet include "Dummy Cycle and Frequency Table" for checking.
> > For Macronix spi-nor flash, 20 or 18 dummy cycles should be use at 200MHz.
> > It alse means that DC 20 is suit for other frequency(<=200MHz).
> >
>
> You didn't answer my question. Does the flash work at 30 MHz by using 20
> dummy cycles?
Yes.
> >>>
> >>> For enable Macronix flashes in Octal DTR mode, Configuration
> >>> Register2 DOPI bit should be set and DC bit setting for dummy
> >>> cycles.
> >>
> >> Use imperative.
> > I didn't get it.
>
> Describe your changes in imperative mood, e.g. "make xyzzy do frotz"
> instead of "[This patch] makes xyzzy do frotz" or "[I] changed xyzzy
> to do frotz", as if you are giving orders to the codebase to change
> its behaviour. Please read:
> https://www.kernel.org/doc/html/latest/process/submitting-patches.html
Got it, thanks.
>
> >
> >>>
> >>> Signed-off-by: JaimeLiao <jaimeliao at mxic.com.tw>
> >>> Co-developed-by: Tudor Ambarus <tudor.ambarus at linaro.org>
> >>> ---
> >>> drivers/mtd/spi-nor/macronix.c | 97 ++++++++++++++++++++++++++++++++++
> >>> 1 file changed, 97 insertions(+)
> >>>
> >>> diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
> >>> index 04888258e891..b397fd274868 100644
> >>> --- a/drivers/mtd/spi-nor/macronix.c
> >>> +++ b/drivers/mtd/spi-nor/macronix.c
> >>> @@ -8,6 +8,22 @@
> >>>
> >>> #include "core.h"
> >>>
> >>> +#define SPINOR_OP_MXIC_RD_ANY_REG 0x71 /* Read volatile configuration register 2 */
> >>> +#define SPINOR_OP_MXIC_WR_ANY_REG 0x72 /* Write volatile configuration register 2 */
> >>> +#define SPINOR_REG_MXIC_CR2_MODE 0x00000000 /* For setting octal DTR mode */
> >>> +#define SPINOR_REG_MXIC_CR2_DC 0x00000300 /* For setting dummy cycles */
> >>> +#define SPINOR_REG_MXIC_OPI_DTR_EN 0x2 /* Enable Octal DTR */
> >>> +#define SPINOR_REG_MXIC_SPI_EN 0x0 /* Enable SPI */
> >>> +#define SPINOR_REG_MXIC_DC_20 0x0 /* Setting dummy cycles to 20 */
> >>> +#define SPINOR_REG_MXIC_ADDR_BYTES 4 /* Fixed R/W volatile address bytes to 4 */
> >>> +
> >>> +/* Macronix SPI NOR flash operations. */
> >>> +#define MXIC_NOR_WR_ANY_REG_OP(naddr, addr, ndata, buf) \
> >>> + SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_MXIC_WR_ANY_REG, 0), \
> >>> + SPI_MEM_OP_ADDR(naddr, addr, 0), \
> >>> + SPI_MEM_OP_NO_DUMMY, \
> >>> + SPI_MEM_OP_DATA_OUT(ndata, buf, 0))
> >>> +
> >>> static int
> >>> mx25l25635_post_bfpt_fixups(struct spi_nor *nor,
> >>> const struct sfdp_parameter_header *bfpt_header,
> >>> @@ -105,9 +121,90 @@ static const struct flash_info macronix_nor_parts[] = {
> >>> FIXUP_FLAGS(SPI_NOR_4B_OPCODES) },
> >>> };
> >>>
> >>> +static int macronix_nor_octal_dtr_en(struct spi_nor *nor)
> >>> +{
> >>> + struct spi_mem_op op;
> >>> + u8 *buf = nor->bouncebuf, i;
> >>> + int ret;
> >>> +
> >>> + /* Use 20 dummy cycles for memory array reads. */
> >>> + buf[0] = SPINOR_REG_MXIC_DC_20;
> >>> + op = (struct spi_mem_op)
> >>> + MXIC_NOR_WR_ANY_REG_OP(SPINOR_REG_MXIC_ADDR_BYTES,
> >>> + SPINOR_REG_MXIC_CR2_DC, 1, buf);
> >>> +
> >>> + ret = spi_nor_write_any_volatile_reg(nor, &op, nor->reg_proto);
> >>> + if (ret)
> >>> + return ret;
> >>> +
> >>> + /* Set the octal and DTR enable bits. */
> >>> + buf[0] = SPINOR_REG_MXIC_OPI_DTR_EN;
> >>> + op = (struct spi_mem_op)
> >>> + MXIC_NOR_WR_ANY_REG_OP(SPINOR_REG_MXIC_ADDR_BYTES,
> >>> + SPINOR_REG_MXIC_CR2_MODE, 1, buf);
> >>> + ret = spi_nor_write_any_volatile_reg(nor, &op, nor->reg_proto);
> >>> + if (ret)
> >>> + return ret;
> >>> +
> >>> + /* Read flash ID to make sure the switch was successful. */
> >>> + ret = spi_nor_read_id(nor, 4, 4, buf, SNOR_PROTO_8_8_8_DTR);
> >>> + if (ret) {
> >>> + dev_dbg(nor->dev, "error %d reading JEDEC ID after enabling 8D-8D-8D mode\n", ret);
> >>> + return ret;
> >>> + }
> >>> +
> >>> + /* Macronix SPI-NOR flash 8D-8D-8D read ID would get 6 bytes data A-A-B-B-C-C */
> >>> + for (i = 0; i < nor->info->id_len; i++)
> >>> + if (buf[i * 2] != nor->info->id[i])
> >>> + return -EINVAL;
> >>> +
> >>> + return 0;
> >>> +}
> >>> +
> >>> +static int macronix_nor_octal_dtr_dis(struct spi_nor *nor)
> >>> +{
> >>> + struct spi_mem_op op;
> >>> + u8 *buf = nor->bouncebuf;
> >>> + int ret;
> >>> +
> >>> + /*
> >>> + * The register is 1-byte wide, but 1-byte transactions are not
> >>> + * allowed in 8D-8D-8D mode. Since there is no register at the
> >>> + * next location, just initialize the value to 0 and let the
> >>> + * transaction go on.
> >>> + */
> >>> + buf[0] = SPINOR_REG_MXIC_SPI_EN;
> >>> + buf[1] = 0x0;
> >>> + op = (struct spi_mem_op)
> >>> + MXIC_NOR_WR_ANY_REG_OP(SPINOR_REG_MXIC_ADDR_BYTES,
> >>> + SPINOR_REG_MXIC_CR2_MODE, 2, buf);
> >>> + ret = spi_nor_write_any_volatile_reg(nor, &op, SNOR_PROTO_8_8_8_DTR);
> >>> + if (ret)
> >>> + return ret;
> >>> +
> >>> + /* Read flash ID to make sure the switch was successful. */
> >>> + ret = spi_nor_read_id(nor, 0, 0, buf, SNOR_PROTO_1_1_1);
> >>> + if (ret) {
> >>> + dev_dbg(nor->dev, "error %d reading JEDEC ID after disabling 8D-8D-8D mode\n", ret);
> >>> + return ret;
> >>> + }
> >>> +
> >>> + if (memcmp(buf, nor->info->id, nor->info->id_len))
> >>> + return -EINVAL;
> >>> +
> >>> + return 0;
> >>> +}
> >>> +
> >>> +static int macronix_nor_octal_dtr_enable(struct spi_nor *nor, bool enable)
> >>> +{
> >>> + return enable ? macronix_nor_octal_dtr_en(nor) :
> >>> + macronix_nor_octal_dtr_dis(nor);
> >>> +}
> >>> +
> >>> static void macronix_nor_default_init(struct spi_nor *nor)
> >>> {
> >>> nor->params->quad_enable = spi_nor_sr1_bit6_quad_enable;
> >>> + nor->params->octal_dtr_enable = macronix_nor_octal_dtr_enable;
> >>
> >> this must be done in the late_init hook, I'd like to get rid of the
> >> default_init hook.
> > nor->params->quad_enable should be move in late_init or
> > nor->params->octal_dtr_enable only?
>
> quad_enable should be parsed from sfdp, thus we have to get rid of
> setting quad_enable in a dedicated hook. Remove it entirely, but in a
> dedicated patch. Of course, I expect you test the change and verify
> that quad still works on all the affected flashes.
>
> octal_dtr_enable should be set in late_init as we don't retrieve the
> octal dtr enable from SFDP yet.
Ok, I will move it to late_init.
> >
> >>> }
> >>>
> >>> static void macronix_nor_late_init(struct spi_nor *nor)
> >
> > Thanks
> > Jaime
Thanks
Jaime
More information about the linux-mtd
mailing list