2022-02-23 15:18:08

by Michael Walle

Subject: [PATCH v5 11/32] mtd: spi-nor: micron-st: unify function names

To avoid name clashes unify all the function and static object names and
use one of the following prefixes which should be sufficiently unique:
- <vendor>_nor_
- <flash_family>_nor_
- <flash_part>_

There are no functional changes.

Signed-off-by: Michael Walle <[email protected]>
Acked-by: Pratyush Yadav <[email protected]>
---
drivers/mtd/spi-nor/micron-st.c | 34 ++++++++++++++++-----------------
1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/mtd/spi-nor/micron-st.c b/drivers/mtd/spi-nor/micron-st.c
index bb95b1aabf74..7a68f2ad3ea1 100644
--- a/drivers/mtd/spi-nor/micron-st.c
+++ b/drivers/mtd/spi-nor/micron-st.c
@@ -17,7 +17,7 @@
#define SPINOR_MT_OCT_DTR 0xe7 /* Enable Octal DTR. */
#define SPINOR_MT_EXSPI 0xff /* Enable Extended SPI (default) */

-static int spi_nor_micron_octal_dtr_enable(struct spi_nor *nor, bool enable)
+static int micron_st_nor_octal_dtr_enable(struct spi_nor *nor, bool enable)
{
struct spi_mem_op op;
u8 *buf = nor->bouncebuf;
@@ -102,7 +102,7 @@ static int spi_nor_micron_octal_dtr_enable(struct spi_nor *nor, bool enable)

static void mt35xu512aba_default_init(struct spi_nor *nor)
{
- nor->params->octal_dtr_enable = spi_nor_micron_octal_dtr_enable;
+ nor->params->octal_dtr_enable = micron_st_nor_octal_dtr_enable;
}

static void mt35xu512aba_post_sfdp_fixup(struct spi_nor *nor)
@@ -130,7 +130,7 @@ static const struct spi_nor_fixups mt35xu512aba_fixups = {
.post_sfdp = mt35xu512aba_post_sfdp_fixup,
};

-static const struct flash_info micron_parts[] = {
+static const struct flash_info micron_nor_parts[] = {
{ "mt35xu512aba", INFO(0x2c5b1a, 0, 128 * 1024, 512)
FLAGS(USE_FSR)
NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_READ |
@@ -143,7 +143,7 @@ static const struct flash_info micron_parts[] = {
FIXUP_FLAGS(SPI_NOR_4B_OPCODES) },
};

-static const struct flash_info st_parts[] = {
+static const struct flash_info st_nor_parts[] = {
{ "n25q016a", INFO(0x20bb15, 0, 64 * 1024, 32)
NO_SFDP_FLAGS(SECT_4K | SPI_NOR_QUAD_READ) },
{ "n25q032", INFO(0x20ba16, 0, 64 * 1024, 64)
@@ -250,15 +250,15 @@ static const struct flash_info st_parts[] = {
};

/**
- * st_micron_set_4byte_addr_mode() - Set 4-byte address mode for ST and Micron
- * flashes.
+ * micron_st_nor_set_4byte_addr_mode() - Set 4-byte address mode for ST and
+ * Micron flashes.
* @nor: pointer to 'struct spi_nor'.
* @enable: true to enter the 4-byte address mode, false to exit the 4-byte
* address mode.
*
* Return: 0 on success, -errno otherwise.
*/
-static int st_micron_set_4byte_addr_mode(struct spi_nor *nor, bool enable)
+static int micron_st_nor_set_4byte_addr_mode(struct spi_nor *nor, bool enable)
{
int ret;

@@ -273,28 +273,28 @@ static int st_micron_set_4byte_addr_mode(struct spi_nor *nor, bool enable)
return spi_nor_write_disable(nor);
}

-static void micron_st_default_init(struct spi_nor *nor)
+static void micron_st_nor_default_init(struct spi_nor *nor)
{
nor->flags |= SNOR_F_HAS_LOCK;
nor->flags &= ~SNOR_F_HAS_16BIT_SR;
nor->params->quad_enable = NULL;
- nor->params->set_4byte_addr_mode = st_micron_set_4byte_addr_mode;
+ nor->params->set_4byte_addr_mode = micron_st_nor_set_4byte_addr_mode;
}

-static const struct spi_nor_fixups micron_st_fixups = {
- .default_init = micron_st_default_init,
+static const struct spi_nor_fixups micron_st_nor_fixups = {
+ .default_init = micron_st_nor_default_init,
};

const struct spi_nor_manufacturer spi_nor_micron = {
.name = "micron",
- .parts = micron_parts,
- .nparts = ARRAY_SIZE(micron_parts),
- .fixups = &micron_st_fixups,
+ .parts = micron_nor_parts,
+ .nparts = ARRAY_SIZE(micron_nor_parts),
+ .fixups = &micron_st_nor_fixups,
};

const struct spi_nor_manufacturer spi_nor_st = {
.name = "st",
- .parts = st_parts,
- .nparts = ARRAY_SIZE(st_parts),
- .fixups = &micron_st_fixups,
+ .parts = st_nor_parts,
+ .nparts = ARRAY_SIZE(st_nor_parts),
+ .fixups = &micron_st_nor_fixups,
};
--
2.30.2

2022-02-25 16:13:25

by Tudor Ambarus

Subject: Re: [PATCH v5 11/32] mtd: spi-nor: micron-st: unify function names

On 2/23/22 15:43, Michael Walle wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> To avoid name clashes unify all the function and static object names and
> use one of the following prefixes which should be sufficiently unique:
> - <vendor>_nor_
> - <flash_family>_nor_
> - <flash_part>_
>
> There are no functional changes.
>
> Signed-off-by: Michael Walle <[email protected]>
> Acked-by: Pratyush Yadav <[email protected]>
> ---
> drivers/mtd/spi-nor/micron-st.c | 34 ++++++++++++++++-----------------
> 1 file changed, 17 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/micron-st.c b/drivers/mtd/spi-nor/micron-st.c
> index bb95b1aabf74..7a68f2ad3ea1 100644
> --- a/drivers/mtd/spi-nor/micron-st.c
> +++ b/drivers/mtd/spi-nor/micron-st.c
> @@ -17,7 +17,7 @@
> #define SPINOR_MT_OCT_DTR 0xe7 /* Enable Octal DTR. */
> #define SPINOR_MT_EXSPI 0xff /* Enable Extended SPI (default) */
>
> -static int spi_nor_micron_octal_dtr_enable(struct spi_nor *nor, bool enable)
> +static int micron_st_nor_octal_dtr_enable(struct spi_nor *nor, bool enable)
> {
> struct spi_mem_op op;
> u8 *buf = nor->bouncebuf;
> @@ -102,7 +102,7 @@ static int spi_nor_micron_octal_dtr_enable(struct spi_nor *nor, bool enable)
>
> static void mt35xu512aba_default_init(struct spi_nor *nor)
> {
> - nor->params->octal_dtr_enable = spi_nor_micron_octal_dtr_enable;
> + nor->params->octal_dtr_enable = micron_st_nor_octal_dtr_enable;
> }
>
> static void mt35xu512aba_post_sfdp_fixup(struct spi_nor *nor)
> @@ -130,7 +130,7 @@ static const struct spi_nor_fixups mt35xu512aba_fixups = {
> .post_sfdp = mt35xu512aba_post_sfdp_fixup,
> };
>
> -static const struct flash_info micron_parts[] = {
> +static const struct flash_info micron_nor_parts[] = {
> { "mt35xu512aba", INFO(0x2c5b1a, 0, 128 * 1024, 512)
> FLAGS(USE_FSR)
> NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_READ |
> @@ -143,7 +143,7 @@ static const struct flash_info micron_parts[] = {
> FIXUP_FLAGS(SPI_NOR_4B_OPCODES) },
> };
>
> -static const struct flash_info st_parts[] = {
> +static const struct flash_info st_nor_parts[] = {
> { "n25q016a", INFO(0x20bb15, 0, 64 * 1024, 32)
> NO_SFDP_FLAGS(SECT_4K | SPI_NOR_QUAD_READ) },
> { "n25q032", INFO(0x20ba16, 0, 64 * 1024, 64)
> @@ -250,15 +250,15 @@ static const struct flash_info st_parts[] = {
> };
>
> /**
> - * st_micron_set_4byte_addr_mode() - Set 4-byte address mode for ST and Micron
> - * flashes.
> + * micron_st_nor_set_4byte_addr_mode() - Set 4-byte address mode for ST and
> + * Micron flashes.
> * @nor: pointer to 'struct spi_nor'.
> * @enable: true to enter the 4-byte address mode, false to exit the 4-byte
> * address mode.
> *
> * Return: 0 on success, -errno otherwise.
> */
> -static int st_micron_set_4byte_addr_mode(struct spi_nor *nor, bool enable)
> +static int micron_st_nor_set_4byte_addr_mode(struct spi_nor *nor, bool enable)
> {
> int ret;
>
> @@ -273,28 +273,28 @@ static int st_micron_set_4byte_addr_mode(struct spi_nor *nor, bool enable)
> return spi_nor_write_disable(nor);
> }
>
> -static void micron_st_default_init(struct spi_nor *nor)
> +static void micron_st_nor_default_init(struct spi_nor *nor)
> {
> nor->flags |= SNOR_F_HAS_LOCK;
> nor->flags &= ~SNOR_F_HAS_16BIT_SR;
> nor->params->quad_enable = NULL;
> - nor->params->set_4byte_addr_mode = st_micron_set_4byte_addr_mode;
> + nor->params->set_4byte_addr_mode = micron_st_nor_set_4byte_addr_mode;
> }
>
> -static const struct spi_nor_fixups micron_st_fixups = {
> - .default_init = micron_st_default_init,
> +static const struct spi_nor_fixups micron_st_nor_fixups = {
> + .default_init = micron_st_nor_default_init,
> };
>
> const struct spi_nor_manufacturer spi_nor_micron = {
> .name = "micron",
> - .parts = micron_parts,
> - .nparts = ARRAY_SIZE(micron_parts),
> - .fixups = &micron_st_fixups,
> + .parts = micron_nor_parts,
> + .nparts = ARRAY_SIZE(micron_nor_parts),
> + .fixups = &micron_st_nor_fixups,
> };
>
> const struct spi_nor_manufacturer spi_nor_st = {
> .name = "st",
> - .parts = st_parts,
> - .nparts = ARRAY_SIZE(st_parts),
> - .fixups = &micron_st_fixups,
> + .parts = st_nor_parts,

you have st_nor here

> + .nparts = ARRAY_SIZE(st_nor_parts),
> + .fixups = &micron_st_nor_fixups,

and here micron_st, it confuses me.

Any idea when should we use just st, or just micron or micron_st?

Cheers,
ta

2022-02-25 16:57:55

by Tudor Ambarus

Subject: Re: [PATCH v5 11/32] mtd: spi-nor: micron-st: unify function names

On 2/23/22 15:43, Michael Walle wrote:
> -static int spi_nor_micron_octal_dtr_enable(struct spi_nor *nor, bool enable)
> +static int micron_st_nor_octal_dtr_enable(struct spi_nor *nor, bool enable)

we should keep spi_nor_micron_octal_dtr_enable, because there are no ST flashes
that support octal DTR.

2022-02-25 17:24:59

by Tudor Ambarus

Subject: Re: [PATCH v5 11/32] mtd: spi-nor: micron-st: unify function names

On 2/25/22 15:35, Tudor Ambarus wrote:
> On 2/23/22 15:43, Michael Walle wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> To avoid name clashes unify all the function and static object names and
>> use one of the following prefixes which should be sufficiently unique:
>> - <vendor>_nor_
>> - <flash_family>_nor_
>> - <flash_part>_
>>
>> There are no functional changes.
>>
>> Signed-off-by: Michael Walle <[email protected]>
>> Acked-by: Pratyush Yadav <[email protected]>
>> ---
>> drivers/mtd/spi-nor/micron-st.c | 34 ++++++++++++++++-----------------
>> 1 file changed, 17 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/mtd/spi-nor/micron-st.c b/drivers/mtd/spi-nor/micron-st.c
>> index bb95b1aabf74..7a68f2ad3ea1 100644
>> --- a/drivers/mtd/spi-nor/micron-st.c
>> +++ b/drivers/mtd/spi-nor/micron-st.c
>> @@ -17,7 +17,7 @@
>> #define SPINOR_MT_OCT_DTR 0xe7 /* Enable Octal DTR. */
>> #define SPINOR_MT_EXSPI 0xff /* Enable Extended SPI (default) */
>>
>> -static int spi_nor_micron_octal_dtr_enable(struct spi_nor *nor, bool enable)
>> +static int micron_st_nor_octal_dtr_enable(struct spi_nor *nor, bool enable)
>> {
>> struct spi_mem_op op;
>> u8 *buf = nor->bouncebuf;
>> @@ -102,7 +102,7 @@ static int spi_nor_micron_octal_dtr_enable(struct spi_nor *nor, bool enable)
>>
>> static void mt35xu512aba_default_init(struct spi_nor *nor)
>> {
>> - nor->params->octal_dtr_enable = spi_nor_micron_octal_dtr_enable;
>> + nor->params->octal_dtr_enable = micron_st_nor_octal_dtr_enable;
>> }
>>
>> static void mt35xu512aba_post_sfdp_fixup(struct spi_nor *nor)
>> @@ -130,7 +130,7 @@ static const struct spi_nor_fixups mt35xu512aba_fixups = {
>> .post_sfdp = mt35xu512aba_post_sfdp_fixup,
>> };
>>
>> -static const struct flash_info micron_parts[] = {
>> +static const struct flash_info micron_nor_parts[] = {
>> { "mt35xu512aba", INFO(0x2c5b1a, 0, 128 * 1024, 512)
>> FLAGS(USE_FSR)
>> NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_READ |
>> @@ -143,7 +143,7 @@ static const struct flash_info micron_parts[] = {
>> FIXUP_FLAGS(SPI_NOR_4B_OPCODES) },
>> };
>>
>> -static const struct flash_info st_parts[] = {
>> +static const struct flash_info st_nor_parts[] = {
>> { "n25q016a", INFO(0x20bb15, 0, 64 * 1024, 32)
>> NO_SFDP_FLAGS(SECT_4K | SPI_NOR_QUAD_READ) },
>> { "n25q032", INFO(0x20ba16, 0, 64 * 1024, 64)
>> @@ -250,15 +250,15 @@ static const struct flash_info st_parts[] = {
>> };
>>
>> /**
>> - * st_micron_set_4byte_addr_mode() - Set 4-byte address mode for ST and Micron
>> - * flashes.
>> + * micron_st_nor_set_4byte_addr_mode() - Set 4-byte address mode for ST and
>> + * Micron flashes.
>> * @nor: pointer to 'struct spi_nor'.
>> * @enable: true to enter the 4-byte address mode, false to exit the 4-byte
>> * address mode.
>> *
>> * Return: 0 on success, -errno otherwise.
>> */
>> -static int st_micron_set_4byte_addr_mode(struct spi_nor *nor, bool enable)
>> +static int micron_st_nor_set_4byte_addr_mode(struct spi_nor *nor, bool enable)
>> {
>> int ret;
>>
>> @@ -273,28 +273,28 @@ static int st_micron_set_4byte_addr_mode(struct spi_nor *nor, bool enable)
>> return spi_nor_write_disable(nor);
>> }
>>
>> -static void micron_st_default_init(struct spi_nor *nor)
>> +static void micron_st_nor_default_init(struct spi_nor *nor)
>> {
>> nor->flags |= SNOR_F_HAS_LOCK;
>> nor->flags &= ~SNOR_F_HAS_16BIT_SR;
>> nor->params->quad_enable = NULL;
>> - nor->params->set_4byte_addr_mode = st_micron_set_4byte_addr_mode;
>> + nor->params->set_4byte_addr_mode = micron_st_nor_set_4byte_addr_mode;
>> }
>>
>> -static const struct spi_nor_fixups micron_st_fixups = {
>> - .default_init = micron_st_default_init,
>> +static const struct spi_nor_fixups micron_st_nor_fixups = {
>> + .default_init = micron_st_nor_default_init,
>> };
>>
>> const struct spi_nor_manufacturer spi_nor_micron = {
>> .name = "micron",
>> - .parts = micron_parts,
>> - .nparts = ARRAY_SIZE(micron_parts),
>> - .fixups = &micron_st_fixups,
>> + .parts = micron_nor_parts,
>> + .nparts = ARRAY_SIZE(micron_nor_parts),
>> + .fixups = &micron_st_nor_fixups,
>> };
>>
>> const struct spi_nor_manufacturer spi_nor_st = {
>> .name = "st",
>> - .parts = st_parts,
>> - .nparts = ARRAY_SIZE(st_parts),
>> - .fixups = &micron_st_fixups,
>> + .parts = st_nor_parts,
>
> you have st_nor here
>
>> + .nparts = ARRAY_SIZE(st_nor_parts),
>> + .fixups = &micron_st_nor_fixups,
>
> and here micron_st, it confuses me.

Oh, it's because micron_st_nor_fixups is used by both micron and st
parts. Let me revisit.

>
> Any idea when should we use just st, or just micron or micron_st?
>