[PATCH v2 1/2] mtd: spi-nor: add Octal DTR support for Macronix flash
Tudor Ambarus
tudor.ambarus at linaro.org
Tue Aug 1 01:16:13 PDT 2023
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?
>>>
>>> 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
>
>>>
>>> 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.
>
>>> }
>>>
>>> static void macronix_nor_late_init(struct spi_nor *nor)
>
> Thanks
> Jaime
More information about the linux-mtd
mailing list