Re: [PATCH v6 1/4] mtd: spi-nor: macronix: add support for Macronix Octal

[liao jaime <jaimeliao.tw@gmail.com>]

Hi Miquel


>
> Hi Jaime,
>
> You made a typo on Jagan's address, you might need to resend.
Yes your are right, I have re-send the cover letter to Jagan.

>
> The title does not look correct, maybe you miss a word after Octal. And
> is it something Macronix specific? I believe this is generic and you
> can drop "Macronix" (the second occurrence) from the title.
Ok got it.
>
> jaimeliao.tw@gmail.com wrote on Wed, 29 Dec 2021 13:56:17 +0800:
>
> > Follow patch "f6adec1af4b2f5d3012480c6cdce7743b74a6156" for adding
>
> When pointing to an upstream commit I think even in U-Boot the style
> should be something like:
> <12-digit hash> ("title of the commit")
> which at least allows to know which commit we are talking about.
OK.

>
> > Macronix flash in Octal DTR mode.
>
> This first part of the commit log should be moved below.
>
> >
> > 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
> >
> > Signed-off-by: JaimeLiao <jaimeliao.tw@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.
>
> This is very specific to Macronix I believe? Please just use a generic
> description here.
Yes I will modify this part next patch.

>
> > + * @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;
> > +}
> > +
> > +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 = &macronix_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 */
>
>
> Thanks,
> Miquèl

Thanks
Jaime