Re: [PATCH v10 1/6] mtd: spi-nor: add Octal DTR support for Macronix flash
From: Tudor Ambarus
Date: Wed Oct 02 2024 - 03:16:42 EST
On 26.09.2024 17:19, AlvinZhou wrote:
> From: AlvinZhou <alvinzhou@xxxxxxxxxxx>
>
> Create Macronix specify method for enable Octal DTR mode and
> set 20 dummy cycles to allow running at the maximum supported
> frequency for Macronix Octal flash.
>
> Use number of dummy cycles which is parse by SFDP then convert
> it to bit pattern and set in CR2 register.
> Set CR2 register for enable octal DTR mode.
>
> Use Read ID to confirm that enabling/disabling octal DTR mode
> was successful.
>
> Macronix ID format is A-A-B-B-C-C in octal DTR mode.
> To ensure the successful enablement of octal DTR mode, confirm
> that the 6-byte data is entirely correct.
>
> Co-developed-by: Tudor Ambarus <tudor.ambarus@xxxxxxxxxx>
> Signed-off-by: Tudor Ambarus <tudor.ambarus@xxxxxxxxxx>
> Signed-off-by: JaimeLiao <jaimeliao@xxxxxxxxxxx>
> Signed-off-by: AlvinZhou <alvinzhou@xxxxxxxxxxx>
> ---
> drivers/mtd/spi-nor/macronix.c | 91 ++++++++++++++++++++++++++++++++++
> 1 file changed, 91 insertions(+)
>
> diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
> index ea6be95e75a5..f039819a5252 100644
> --- a/drivers/mtd/spi-nor/macronix.c
> +++ b/drivers/mtd/spi-nor/macronix.c
> @@ -8,6 +8,24 @@
>
> #include "core.h"
>
> +#define MXIC_NOR_OP_RD_CR2 0x71 /* Read configuration register 2 opcode */
> +#define MXIC_NOR_OP_WR_CR2 0x72 /* Write configuration register 2 opcode */
> +#define MXIC_NOR_ADDR_CR2_MODE 0x00000000 /* CR2 address for setting spi/sopi/dopi mode */
> +#define MXIC_NOR_ADDR_CR2_DC 0x00000300 /* CR2 address for setting dummy cycles */
> +#define MXIC_NOR_REG_DOPI_EN 0x2 /* Enable Octal DTR */
> +#define MXIC_NOR_REG_SPI_EN 0x0 /* Enable SPI */
> +
> +/* Convert dummy cycles to bit pattern */
> +#define MXIC_NOR_REG_DC(p) \
> + ((20 - (p)) >> 1)
This is unfortunate as we convert dummy cycles to bytes in mtd and then
we convert back from bytes to cycles in spi. I had an attempt fixing
this in the past, but couldn't allocate more time for spinning another
version for the patch set.
I won't block the patch set for this, but if someone cares to fix it,
would be great to take over.
> +
> +/* Macronix write CR2 operations */
I'll drop this comment when applying, as I can already see what the
macro is doing from its name.
> +#define MXIC_NOR_WR_CR2(addr, ndata, buf) \
> + SPI_MEM_OP(SPI_MEM_OP_CMD(MXIC_NOR_OP_WR_CR2, 0), \
> + SPI_MEM_OP_ADDR(4, 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,
> @@ -185,6 +203,78 @@ static const struct flash_info macronix_nor_parts[] = {
> }
> };
>
> +static int macronix_nor_octal_dtr_en(struct spi_nor *nor)
> +{
> + struct spi_mem_op op;
> + u8 *buf = nor->bouncebuf, i;
> + int ret;
> +
> + /* Use dummy cycles which is parse by SFDP and convert to bit pattern. */
> + buf[0] = MXIC_NOR_REG_DC(nor->params->reads[SNOR_CMD_READ_8_8_8_DTR].num_wait_states);
> + op = (struct spi_mem_op)MXIC_NOR_WR_CR2(MXIC_NOR_ADDR_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] = MXIC_NOR_REG_DOPI_EN;
> + op = (struct spi_mem_op)MXIC_NOR_WR_CR2(MXIC_NOR_ADDR_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);
can we use nor->addr_nbytes for the second argument? Please test and
confirm. No need to resend for this, just confirm and I can amend when
applying.
What about the third argument, the number of dummy nbytes. Can we get
the cycles needed for READID from somewhere in SFDP?
Looks good.