Hi Tudor
>
> On 11/26/21 7:28 AM, liao jaime wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> > content is safe
> >
> > Hi Tudor
>
> Hi,
>
> >
> >>
> >> On 11/18/21 12:13 PM, JaimeLiao wrote:
> >>> EXTERNAL EMAIL: Do not click links or open attachments unless you know
> >>> the content is safe
> >>>
> >>
> >> Hi, Jaime,
> >>
> >>> 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.
> >>>
> >>> -https://www.mxic.com.tw/Lists/Datasheet/Attachments/7841/MX25LM51245G,%203V,%20512Mb,%20v1.1.pdf
> >>>
> >>
> >> I have submitted a similar patch at
> >> https://lore.kernel.org/all/20211103162454.179191-2-tudor.amba...@microchip.com/T/#me9b6e48c33fd545a9c0b5a5136778651ea64171a
> >>
> >> but I think yours has precedence, you're already at v5.
> >>
> >>> Signed-off-by: JaimeLiao <jaimeliao...@gmail.com>
> >>> ---
> >>> drivers/mtd/spi/spi-nor-core.c | 83 ++++++++++++++++++++++++++++++++++
> >>> include/linux/mtd/spi-nor.h | 12 ++++-
> >>> 2 files changed, 93 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/mtd/spi/spi-nor-core.c
> >>> b/drivers/mtd/spi/spi-nor-core.c
> >>> index d5d905fa5a..0a6550984b 100644
> >>> --- a/drivers/mtd/spi/spi-nor-core.c
> >>> +++ b/drivers/mtd/spi/spi-nor-core.c
> >>> @@ -3489,6 +3489,85 @@ static struct spi_nor_fixups mt35xu512aba_fixups =
> >>> {
> >>> };
> >>> #endif /* CONFIG_SPI_FLASH_MT35XU */
> >>>
> >>> +#if CONFIG_IS_ENABLED(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;
> >>> +
> >>> + ret = write_enable(nor);
> >>> + if (ret)
> >>> + return ret;
> >>> +
> >>> + 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;
> >>> + ret = write_enable(nor);
> >>> + if (ret)
> >>> + return ret;
> >>> +
> >>> + 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_octal_default_init(struct spi_nor *nor)
> >>> +{
> >>> + nor->octal_dtr_enable = spi_nor_macronix_octal_dtr_enable;
> >>> +}
> >>
> >> Can't we determine the octal dtr method by parsing SFDP?
> >>
> > It is a good idea for getting flash parameters by checking SFDP.
> > Your patchwork is on-going to solve the ID collision issues in Linux kernel.
> > I think U-boot should follow the method after ID collision patchwork
> > finish in Linux kenel.
> > So that octal dtr method follow the configuration for now.
> >
>
> What I meant was that if we can determine the octal dtr method from SFDP,
> we should use that. If we can't determine the octal dtr method from SFDP,
> or SFDP is skipped intentionally, then we can define explicit methods,
> as you did. But we'll need to differentiate between the two methods,
> otherwise maintaining the flash entries will become a burden.
>
Sure, I prefer to parsing information from SFDP but not got according
to configuration
and differentiate methods via configuration "SPI_NOR_SKIP_SFDP".
Hook corresponding function on .post_sfdp and .post_bfdp if
configuration SPI_NOR_SKIP_SFDP existing.
I need add the changes for v6 patch if you have any other idea.
But I hope that we can seperate octal dtr support by two patches.
> Cheers,
> ta
> > Thanks
> > Jaime
> >> Cheers,
> >> ta
> >>
> >>> +
> >>> +static void macronix_octal_post_sfdp_fixup(struct spi_nor *nor,
> >>> + struct spi_nor_flash_parameter
> >>> *params)
> >>> +{
> >>> + /*
> >>> + * Adding SNOR_HWCAPS_PP_8_8_8_DTR in hwcaps.mask when
> >>> + * SPI_NOR_OCTAL_DTR_READ flag exists.
> >>> + */
> >>> + if (params->hwcaps.mask & SNOR_HWCAPS_READ_8_8_8_DTR)
> >>> + params->hwcaps.mask |= SNOR_HWCAPS_PP_8_8_8_DTR;
> >>> +}
> >>> +
> >>> +static struct spi_nor_fixups macronix_octal_fixups = {
> >>> + .default_init = macronix_octal_default_init,
> >>> + .post_sfdp = macronix_octal_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 +3734,10 @@ void spi_nor_set_fixups(struct spi_nor *nor)
> >>> if (!strcmp(nor->info->name, "mt35xu512aba"))
> >>> nor->fixups = &mt35xu512aba_fixups;
> >>> #endif
> >>> +
> >>> +#if CONFIG_IS_ENABLED(SPI_FLASH_MACRONIX)
> >>> + nor->fixups = ¯onix_octal_fixups;
> >>> +#endif /* SPI_FLASH_MACRONIX */
> >>> }
> >>>
> >>> 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..8682368f2f 100644
> >>> --- a/include/linux/mtd/spi-nor.h
> >>> +++ b/include/linux/mtd/spi-nor.h
> >>> @@ -116,8 +116,16 @@
> >>> #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_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
> >>>
> >>
>
