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.ya...@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...@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.
>
