From: liao jaime <jaimeliao.tw@gmail.com>
To: Pratyush Yadav <p.yadav@ti.com>
Cc: u-boot@lists.denx.de, Jagan Teki <jagan@amarulasolutions.com>,
vigneshr@ti.com, zhengxunli@mxic.com.tw, ycllin@mxic.com.tw,
jaimeliao@mxic.com.tw
Subject: Re: [PATCH 1/4] mtd: spi-nor: macronix: add support for Macronix octaflash
Date: Mon, 23 Aug 2021 09:12:05 +0800 [thread overview]
Message-ID: <CAAQoYRmJ7i+mWKiF_wXv-QU4po2tkN0GtmM7jfcwQ5jY99eHXw@mail.gmail.com> (raw)
In-Reply-To: <20210813180628.muz3o7y3q635lftx@ti.com>
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.
>
next prev parent reply other threads:[~2021-08-23 12:27 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-08-13 7:25 [PATCH 0/4] Add octal DTR support for Macronix flash JaimeLiao
2021-08-13 7:25 ` [PATCH 1/4] mtd: spi-nor: macronix: add support for Macronix octaflash JaimeLiao
2021-08-13 18:06 ` Pratyush Yadav
2021-08-23 1:12 ` liao jaime [this message]
2021-08-13 7:25 ` [PATCH 2/4] mtd: spi-nor-core: Adding different type of command extension in Soft Reset JaimeLiao
2021-08-13 7:25 ` [PATCH 3/4] mtd: spi-nor-core: set 4byte opcode when possible JaimeLiao
2021-08-13 7:25 ` [PATCH 4/4] mtd: spi-nor-core: Add support for Macronix Octal flash JaimeLiao
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAAQoYRmJ7i+mWKiF_wXv-QU4po2tkN0GtmM7jfcwQ5jY99eHXw@mail.gmail.com \
--to=jaimeliao.tw@gmail.com \
--cc=jagan@amarulasolutions.com \
--cc=jaimeliao@mxic.com.tw \
--cc=p.yadav@ti.com \
--cc=u-boot@lists.denx.de \
--cc=vigneshr@ti.com \
--cc=ycllin@mxic.com.tw \
--cc=zhengxunli@mxic.com.tw \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.