diff mbox series

[v5,1/3] mtd: spi-nor: macronix: add support for Macronix Octal

Message ID 20211118101303.26061-2-jaimeliao.tw@gmail.com
State Changes Requested
Delegated to: Jagannadha Sutradharudu Teki
Headers show
Series Add octal DTR support for Macronix flash | expand

Commit Message

liao jaime Nov. 18, 2021, 10:13 a.m. UTC
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@gmail.com>
---
 drivers/mtd/spi/spi-nor-core.c | 83 ++++++++++++++++++++++++++++++++++
 include/linux/mtd/spi-nor.h    | 12 ++++-
 2 files changed, 93 insertions(+), 2 deletions(-)

Comments

Tudor Ambarus Nov. 19, 2021, 8:34 a.m. UTC | #1
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.ambarus@microchip.com/T/#me9b6e48c33fd545a9c0b5a5136778651ea64171a

but I think yours has precedence, you're already at v5.

> 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.
> + * @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?

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 = &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
>
liao jaime Nov. 26, 2021, 5:28 a.m. UTC | #2
Hi Tudor

>
> 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.ambarus@microchip.com/T/#me9b6e48c33fd545a9c0b5a5136778651ea64171a
>
> but I think yours has precedence, you're already at v5.
>
> > 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.
> > + * @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.

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 = &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
> >
>
Tudor Ambarus Nov. 26, 2021, 6:15 a.m. UTC | #3
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.ambarus@microchip.com/T/#me9b6e48c33fd545a9c0b5a5136778651ea64171a
>>
>> but I think yours has precedence, you're already at v5.
>>
>>> 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.
>>> + * @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.

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 = &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
>>>
>>
liao jaime Nov. 26, 2021, 6:36 a.m. UTC | #4
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.ambarus@microchip.com/T/#me9b6e48c33fd545a9c0b5a5136778651ea64171a
> >>
> >> but I think yours has precedence, you're already at v5.
> >>
> >>> 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.
> >>> + * @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 = &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
> >>>
> >>
>
diff mbox series

Patch

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;
+}
+
+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 */