| Message ID | 20210813072552.21772-2-jaimeliao.tw@gmail.com |
|---|---|
| State | Superseded |
| Delegated to: | Jagannadha Sutradharudu Teki |
| Headers | show |
| Series | Add octal DTR support for Macronix flash | expand |
On 13/08/21 03:25PM, JaimeLiao wrote: > Follow patch "f6adec1af4b2f5d3012480c6cdce7743b74a6156" for adding > Macronix flash in Octal DTR mode. > Enable Octal DTR mode with 20 dummy cycles to allow running at the > maximum supported frequency. Please include a link to the flash datasheet so the reviewers can properly review your patch. > > Signed-off-by: JaimeLiao <jaimeliao.tw@gmail.com> > --- > drivers/mtd/spi/spi-nor-core.c | 75 ++++++++++++++++++++++++++++++++++ > include/linux/mtd/spi-nor.h | 13 +++++- > 2 files changed, 86 insertions(+), 2 deletions(-) > > diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c > index d5d905fa5a..6b195b1fd3 100644 > --- a/drivers/mtd/spi/spi-nor-core.c > +++ b/drivers/mtd/spi/spi-nor-core.c > @@ -3489,6 +3489,77 @@ static struct spi_nor_fixups mt35xu512aba_fixups = { > }; > #endif /* CONFIG_SPI_FLASH_MT35XU */ > > +#ifdef CONFIG_SPI_FLASH_MACRONIX > +/** > + * spi_nor_macronix_octal_dtr_enable() - set DTR OPI Enable bit in Configuration Register 2. > + * @nor: pointer to a 'struct spi_nor' > + * > + * Set the DTR OPI Enable (DOPI) bit in Configuration Register 2. Nitpick: Why the blank line here? > + * > + * bit 2 of Configuration Register 2 is the DOPI bit for Macronix like OPI memories. Nitpick: Capitalize the 'b'. > + * > + * Return: 0 on success, -errno otherwise. > + */ > +static int spi_nor_macronix_octal_dtr_enable(struct spi_nor *nor) > +{ > + struct spi_mem_op op; > + int ret; > + u8 buf; > + > + write_enable(nor); Need to check the return code here. > + > + buf = SPINOR_REG_MXIC_DC_20; > + op = (struct spi_mem_op) > + SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WR_CR2, 1), > + SPI_MEM_OP_ADDR(4, SPINOR_REG_MXIC_CR2_DC, 1), > + SPI_MEM_OP_NO_DUMMY, > + SPI_MEM_OP_DATA_OUT(1, &buf, 1)); > + > + ret = spi_mem_exec_op(nor->spi, &op); > + if (ret) > + return ret; > + > + ret = spi_nor_wait_till_ready(nor); > + if (ret) > + return ret; > + > + nor->read_dummy = MXIC_MAX_DC; > + write_enable(nor); > + > + buf = SPINOR_REG_MXIC_OPI_DTR_EN; > + op = (struct spi_mem_op) > + SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WR_CR2, 1), > + SPI_MEM_OP_ADDR(4, SPINOR_REG_MXIC_CR2_MODE, 1), > + SPI_MEM_OP_NO_DUMMY, > + SPI_MEM_OP_DATA_OUT(1, &buf, 1)); > + > + ret = spi_mem_exec_op(nor->spi, &op); > + if (ret) { > + dev_err(nor->dev, "Failed to enable octal DTR mode\n"); > + return ret; > + } > + nor->reg_proto = SNOR_PROTO_8_8_8_DTR; > + > + return 0; > +} > + > +static void macronix_default_init(struct spi_nor *nor) > +{ > + nor->octal_dtr_enable = spi_nor_macronix_octal_dtr_enable; > +} > + > +static void macronix_post_sfdp_fixup(struct spi_nor *nor, > + struct spi_nor_flash_parameter *params) > +{ > + params->hwcaps.mask |= SNOR_HWCAPS_PP_8_8_8_DTR; This does not seem right. You would mark every Macronix flash Octal DTR capable which is clearly not true. > +} > + > +static struct spi_nor_fixups macronix_fixups = { > + .default_init = macronix_default_init, > + .post_sfdp = macronix_post_sfdp_fixup, > +}; > +#endif /* CONFIG_SPI_FLASH_MACRONIX */ > + > /** spi_nor_octal_dtr_enable() - enable Octal DTR I/O if needed > * @nor: pointer to a 'struct spi_nor' > * > @@ -3655,6 +3726,10 @@ void spi_nor_set_fixups(struct spi_nor *nor) > if (!strcmp(nor->info->name, "mt35xu512aba")) > nor->fixups = &mt35xu512aba_fixups; > #endif > + > +#ifdef CONFIG_SPI_FLASH_MACRONIX > + nor->fixups = ¯onix_fixups; > +#endif > } > > int spi_nor_scan(struct spi_nor *nor) > diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h > index 7ddc4ba2bf..2ad579f66d 100644 > --- a/include/linux/mtd/spi-nor.h > +++ b/include/linux/mtd/spi-nor.h > @@ -116,8 +116,17 @@ > #define XSR_RDY BIT(7) /* Ready */ > > /* Used for Macronix and Winbond flashes. */ > -#define SPINOR_OP_EN4B 0xb7 /* Enter 4-byte mode */ > -#define SPINOR_OP_EX4B 0xe9 /* Exit 4-byte mode */ > +#define SPINOR_OP_EN4B 0xb7 /* Enter 4-byte mode */ > +#define SPINOR_OP_EX4B 0xe9 /* Exit 4-byte mode */ > +#define SPINOR_OP_RD_CR2 0x71 /* Read configuration register 2 */ > +#define SPINOR_OP_WR_CR2 0x72 /* Write configuration register 2 */ > +#define SPINOR_OP_MXIC_DTR_RD 0xee /* Fast Read opcode in DTR mode */ > +#define SPINOR_REG_MXIC_CR2_MODE 0x00000000 /* For setting octal DTR mode */ > +#define SPINOR_REG_MXIC_OPI_DTR_EN 0x2 /* Enable Octal DTR */ > +#define SPINOR_REG_MXIC_OPI_DTR_DIS 0x1 /* Disable Octal DTR */ > +#define SPINOR_REG_MXIC_CR2_DC 0x00000300 /* For setting dummy cycles */ > +#define SPINOR_REG_MXIC_DC_20 0x0 /* Setting dummy cycles to 20 */ > +#define MXIC_MAX_DC 20 /* Maximum value of dummy cycles */ > > /* Used for Spansion flashes only. */ > #define SPINOR_OP_BRWR 0x17 /* Bank register write */ > -- > 2.17.1 >
Hi Pratyush Thanks for your reply and I have send v2 patch , please help to review. I prefer to have v3 patch for replacing SPI_FLASH_MACRONIX with SPI_FLASH_MACRONIX_OCTAL. It would be great if you could help to review v2 and then I will add the modifications in v3. Thanks Jaime Pratyush Yadav <p.yadav@ti.com> 於 2021年8月14日 週六 上午2:06寫道： > On 13/08/21 03:25PM, JaimeLiao wrote: > > Follow patch "f6adec1af4b2f5d3012480c6cdce7743b74a6156" for adding > > Macronix flash in Octal DTR mode. > > Enable Octal DTR mode with 20 dummy cycles to allow running at the > > maximum supported frequency. > > Please include a link to the flash datasheet so the reviewers can > properly review your patch. > > > > > Signed-off-by: JaimeLiao <jaimeliao.tw@gmail.com> > > --- > > drivers/mtd/spi/spi-nor-core.c | 75 ++++++++++++++++++++++++++++++++++ > > include/linux/mtd/spi-nor.h | 13 +++++- > > 2 files changed, 86 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/mtd/spi/spi-nor-core.c > b/drivers/mtd/spi/spi-nor-core.c > > index d5d905fa5a..6b195b1fd3 100644 > > --- a/drivers/mtd/spi/spi-nor-core.c > > +++ b/drivers/mtd/spi/spi-nor-core.c > > @@ -3489,6 +3489,77 @@ static struct spi_nor_fixups mt35xu512aba_fixups > = { > > }; > > #endif /* CONFIG_SPI_FLASH_MT35XU */ > > > > +#ifdef CONFIG_SPI_FLASH_MACRONIX > > +/** > > + * spi_nor_macronix_octal_dtr_enable() - set DTR OPI Enable bit in > Configuration Register 2. > > + * @nor: pointer to a 'struct spi_nor' > > + * > > + * Set the DTR OPI Enable (DOPI) bit in Configuration Register 2. > > Nitpick: Why the blank line here? > > + * > > + * bit 2 of Configuration Register 2 is the DOPI bit for Macronix like > OPI memories. > > Nitpick: Capitalize the 'b'. > > > + * > > + * Return: 0 on success, -errno otherwise. > > + */ > > +static int spi_nor_macronix_octal_dtr_enable(struct spi_nor *nor) > > +{ > > + struct spi_mem_op op; > > + int ret; > > + u8 buf; > > + > > + write_enable(nor); > > Need to check the return code here. > > > + > > + buf = SPINOR_REG_MXIC_DC_20; > > + op = (struct spi_mem_op) > > + SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WR_CR2, 1), > > + SPI_MEM_OP_ADDR(4, SPINOR_REG_MXIC_CR2_DC, 1), > > + SPI_MEM_OP_NO_DUMMY, > > + SPI_MEM_OP_DATA_OUT(1, &buf, 1)); > > + > > + ret = spi_mem_exec_op(nor->spi, &op); > > + if (ret) > > + return ret; > > + > > + ret = spi_nor_wait_till_ready(nor); > > + if (ret) > > + return ret; > > + > > + nor->read_dummy = MXIC_MAX_DC; > > + write_enable(nor); > > + > > + buf = SPINOR_REG_MXIC_OPI_DTR_EN; > > + op = (struct spi_mem_op) > > + SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WR_CR2, 1), > > + SPI_MEM_OP_ADDR(4, SPINOR_REG_MXIC_CR2_MODE, 1), > > + SPI_MEM_OP_NO_DUMMY, > > + SPI_MEM_OP_DATA_OUT(1, &buf, 1)); > > + > > + ret = spi_mem_exec_op(nor->spi, &op); > > + if (ret) { > > + dev_err(nor->dev, "Failed to enable octal DTR mode\n"); > > + return ret; > > + } > > + nor->reg_proto = SNOR_PROTO_8_8_8_DTR; > > + > > + return 0; > > +} > > + > > +static void macronix_default_init(struct spi_nor *nor) > > +{ > > + nor->octal_dtr_enable = spi_nor_macronix_octal_dtr_enable; > > +} > > + > > +static void macronix_post_sfdp_fixup(struct spi_nor *nor, > > + struct spi_nor_flash_parameter > *params) > > +{ > > + params->hwcaps.mask |= SNOR_HWCAPS_PP_8_8_8_DTR; > > This does not seem right. You would mark every Macronix flash Octal DTR > capable which is clearly not true. > > > +} > > + > > +static struct spi_nor_fixups macronix_fixups = { > > + .default_init = macronix_default_init, > > + .post_sfdp = macronix_post_sfdp_fixup, > > +}; > > +#endif /* CONFIG_SPI_FLASH_MACRONIX */ > > + > > /** spi_nor_octal_dtr_enable() - enable Octal DTR I/O if needed > > * @nor: pointer to a 'struct spi_nor' > > * > > @@ -3655,6 +3726,10 @@ void spi_nor_set_fixups(struct spi_nor *nor) > > if (!strcmp(nor->info->name, "mt35xu512aba")) > > nor->fixups = &mt35xu512aba_fixups; > > #endif > > + > > +#ifdef CONFIG_SPI_FLASH_MACRONIX > > + nor->fixups = ¯onix_fixups; > > +#endif > > } > > > > int spi_nor_scan(struct spi_nor *nor) > > diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h > > index 7ddc4ba2bf..2ad579f66d 100644 > > --- a/include/linux/mtd/spi-nor.h > > +++ b/include/linux/mtd/spi-nor.h > > @@ -116,8 +116,17 @@ > > #define XSR_RDY BIT(7) /* Ready */ > > > > /* Used for Macronix and Winbond flashes. */ > > -#define SPINOR_OP_EN4B 0xb7 /* Enter 4-byte mode */ > > -#define SPINOR_OP_EX4B 0xe9 /* Exit 4-byte mode */ > > +#define SPINOR_OP_EN4B 0xb7 /* Enter > 4-byte mode */ > > +#define SPINOR_OP_EX4B 0xe9 /* Exit > 4-byte mode */ > > +#define SPINOR_OP_RD_CR2 0x71 /* Read > configuration register 2 */ > > +#define SPINOR_OP_WR_CR2 0x72 /* Write > configuration register 2 */ > > +#define SPINOR_OP_MXIC_DTR_RD 0xee /* Fast > Read opcode in DTR mode */ > > +#define SPINOR_REG_MXIC_CR2_MODE 0x00000000 /* For setting > octal DTR mode */ > > +#define SPINOR_REG_MXIC_OPI_DTR_EN 0x2 /* Enable Octal > DTR */ > > +#define SPINOR_REG_MXIC_OPI_DTR_DIS 0x1 /* Disable Octal > DTR */ > > +#define SPINOR_REG_MXIC_CR2_DC 0x00000300 /* For > setting dummy cycles */ > > +#define SPINOR_REG_MXIC_DC_20 0x0 /* Setting > dummy cycles to 20 */ > > +#define MXIC_MAX_DC 20 /* Maximum value > of dummy cycles */ > > > > /* Used for Spansion flashes only. */ > > #define SPINOR_OP_BRWR 0x17 /* Bank register write */ > > -- > > 2.17.1 > > > > -- > Regards, > Pratyush Yadav > Texas Instruments Inc. >
diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index d5d905fa5a..6b195b1fd3 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -3489,6 +3489,77 @@ static struct spi_nor_fixups mt35xu512aba_fixups = { }; #endif /* CONFIG_SPI_FLASH_MT35XU */ +#ifdef CONFIG_SPI_FLASH_MACRONIX +/** + * spi_nor_macronix_octal_dtr_enable() - set DTR OPI Enable bit in Configuration Register 2. + * @nor: pointer to a 'struct spi_nor' + * + * Set the DTR OPI Enable (DOPI) bit in Configuration Register 2. + * + * bit 2 of Configuration Register 2 is the DOPI bit for Macronix like OPI memories. + * + * Return: 0 on success, -errno otherwise. + */ +static int spi_nor_macronix_octal_dtr_enable(struct spi_nor *nor) +{ + struct spi_mem_op op; + int ret; + u8 buf; + + write_enable(nor); + + buf = SPINOR_REG_MXIC_DC_20; + op = (struct spi_mem_op) + SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WR_CR2, 1), + SPI_MEM_OP_ADDR(4, SPINOR_REG_MXIC_CR2_DC, 1), + SPI_MEM_OP_NO_DUMMY, + SPI_MEM_OP_DATA_OUT(1, &buf, 1)); + + ret = spi_mem_exec_op(nor->spi, &op); + if (ret) + return ret; + + ret = spi_nor_wait_till_ready(nor); + if (ret) + return ret; + + nor->read_dummy = MXIC_MAX_DC; + write_enable(nor); + + buf = SPINOR_REG_MXIC_OPI_DTR_EN; + op = (struct spi_mem_op) + SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WR_CR2, 1), + SPI_MEM_OP_ADDR(4, SPINOR_REG_MXIC_CR2_MODE, 1), + SPI_MEM_OP_NO_DUMMY, + SPI_MEM_OP_DATA_OUT(1, &buf, 1)); + + ret = spi_mem_exec_op(nor->spi, &op); + if (ret) { + dev_err(nor->dev, "Failed to enable octal DTR mode\n"); + return ret; + } + nor->reg_proto = SNOR_PROTO_8_8_8_DTR; + + return 0; +} + +static void macronix_default_init(struct spi_nor *nor) +{ + nor->octal_dtr_enable = spi_nor_macronix_octal_dtr_enable; +} + +static void macronix_post_sfdp_fixup(struct spi_nor *nor, + struct spi_nor_flash_parameter *params) +{ + params->hwcaps.mask |= SNOR_HWCAPS_PP_8_8_8_DTR; +} + +static struct spi_nor_fixups macronix_fixups = { + .default_init = macronix_default_init, + .post_sfdp = macronix_post_sfdp_fixup, +}; +#endif /* CONFIG_SPI_FLASH_MACRONIX */ + /** spi_nor_octal_dtr_enable() - enable Octal DTR I/O if needed * @nor: pointer to a 'struct spi_nor' * @@ -3655,6 +3726,10 @@ void spi_nor_set_fixups(struct spi_nor *nor) if (!strcmp(nor->info->name, "mt35xu512aba")) nor->fixups = &mt35xu512aba_fixups; #endif + +#ifdef CONFIG_SPI_FLASH_MACRONIX + nor->fixups = ¯onix_fixups; +#endif } int spi_nor_scan(struct spi_nor *nor) diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h index 7ddc4ba2bf..2ad579f66d 100644 --- a/include/linux/mtd/spi-nor.h +++ b/include/linux/mtd/spi-nor.h @@ -116,8 +116,17 @@ #define XSR_RDY BIT(7) /* Ready */ /* Used for Macronix and Winbond flashes. */ -#define SPINOR_OP_EN4B 0xb7 /* Enter 4-byte mode */ -#define SPINOR_OP_EX4B 0xe9 /* Exit 4-byte mode */ +#define SPINOR_OP_EN4B 0xb7 /* Enter 4-byte mode */ +#define SPINOR_OP_EX4B 0xe9 /* Exit 4-byte mode */ +#define SPINOR_OP_RD_CR2 0x71 /* Read configuration register 2 */ +#define SPINOR_OP_WR_CR2 0x72 /* Write configuration register 2 */ +#define SPINOR_OP_MXIC_DTR_RD 0xee /* Fast Read opcode in DTR mode */ +#define SPINOR_REG_MXIC_CR2_MODE 0x00000000 /* For setting octal DTR mode */ +#define SPINOR_REG_MXIC_OPI_DTR_EN 0x2 /* Enable Octal DTR */ +#define SPINOR_REG_MXIC_OPI_DTR_DIS 0x1 /* Disable Octal DTR */ +#define SPINOR_REG_MXIC_CR2_DC 0x00000300 /* For setting dummy cycles */ +#define SPINOR_REG_MXIC_DC_20 0x0 /* Setting dummy cycles to 20 */ +#define MXIC_MAX_DC 20 /* Maximum value of dummy cycles */ /* Used for Spansion flashes only. */ #define SPINOR_OP_BRWR 0x17 /* Bank register write */
Follow patch "f6adec1af4b2f5d3012480c6cdce7743b74a6156" for adding Macronix flash in Octal DTR mode. Enable Octal DTR mode with 20 dummy cycles to allow running at the maximum supported frequency. Signed-off-by: JaimeLiao <jaimeliao.tw@gmail.com> --- drivers/mtd/spi/spi-nor-core.c | 75 ++++++++++++++++++++++++++++++++++ include/linux/mtd/spi-nor.h | 13 +++++- 2 files changed, 86 insertions(+), 2 deletions(-)