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

Tudor.Ambarus at microchip.com Tudor.Ambarus at microchip.com
Mon Jan 3 11:38:13 CET 2022


Hi,

On 12/29/21 7:56 AM, JaimeLiao wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> 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
> 
> Signed-off-by: JaimeLiao <jaimeliao.tw at 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)

This method should be called just for the flashes that do not define the
JESD216 "Command Sequences to Change to Octal DDR (8D-8D-8D) mode" table.
Or where SFDP is skipped intentionally.

> +{
> +       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)

I think we should get rid of the default_init() hook, it messes how
the params are initialized, there's a spaghetti right now, it's very hard to
determine who initializes which params solely by reading the code. I would
advise to follow linux and use a late_init() hook where explicit octal dtr
enable method is required.


> +{
> +       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;

This confirms the spaghetti theory. This is not SFDP related, why do you use
the post_sfdp hook?

> +}
> +
> +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)
Not related to your patch, but ideally all the macronix code should reside
in a dedicated macronix.c driver.

Cheers,
ta

> +       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 */
> --
> 2.17.1
> 



More information about the U-Boot mailing list