Re: [PATCH v2 1/2] mtd: spi-nor: add Octal DTR support for Macronix flash

[Tudor Ambarus <tudor.ambarus@linaro.org>]

On 7/27/23 10:16, Jaime Liao wrote:
> From: JaimeLiao <jaimeliao@mxic.com.tw>
> 
> Create Macronix specify method for enable Octal DTR mode and
> set 20 dummy cycles to allow running at the maximum supported
> frequency for Macronix Octal flash.

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?
> 
> For enable Macronix flashes in Octal DTR mode, Configuration
> Register2 DOPI bit should be set and DC bit setting for dummy
> cycles.

Use imperative.
> 
> Signed-off-by: JaimeLiao <jaimeliao@mxic.com.tw>
> Co-developed-by: Tudor Ambarus <tudor.ambarus@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.
>  }
>  
>  static void macronix_nor_late_init(struct spi_nor *nor)

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/