diff mbox series

[v12,3/6] mtd: spi-nor: spansion: Rename local macros

Message ID e7d067a754e3361c29960dec266671f0371c38ef.1650519576.git.Takahiro.Kuwano@infineon.com
State Changes Requested
Delegated to: Pratyush Yadav
Headers show
Series mtd: spi-nor: Add support for Infineon s25hl-t/s25hs-t | expand

Commit Message

Takahiro Kuwano April 21, 2022, 6:40 a.m. UTC 
From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>

Use CYPRESS_NOR_ prefix for local macros

Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
---
 drivers/mtd/spi-nor/spansion.c | 78 +++++++++++++++++-----------------
 1 file changed, 40 insertions(+), 38 deletions(-)

Comments

Tudor Ambarus April 21, 2022, 7:37 a.m. UTC | #1 
On 4/21/22 09:40, tkuw584924@gmail.com wrote:
> -#define SPINOR_OP_CLSR         0x30    /* Clear status register 1 */

This is a legacy spansion op. So you end up with "spansion" flashes that
use "cypress" ops, which is confusing. At the same time I don't care
about the name, but if you want to change it, I suggest to change all the
defines and get rid of the "spansion" name if that's what you want.
The driver name has to be changed as well.
Other thought is that cypress was acquired by infineon, so what will we
do in few years? Will we rename all macros and methods to contain infineon?

Let us know your thoughts and if you do this kind of change, explain why.

Cheers,
ta
Takahiro Kuwano April 21, 2022, 8 a.m. UTC | #2 
On 4/21/2022 4:37 PM, Tudor.Ambarus@microchip.com wrote:
> On 4/21/22 09:40, tkuw584924@gmail.com wrote:
>> -#define SPINOR_OP_CLSR         0x30    /* Clear status register 1 */
> 
> This is a legacy spansion op. So you end up with "spansion" flashes that
> use "cypress" ops, which is confusing. At the same time I don't care
Agree. Will use SPANSION_NOR_ for CLSR.

> about the name, but if you want to change it, I suggest to change all the
> defines and get rid of the "spansion" name if that's what you want.
> The driver name has to be changed as well.
> Other thought is that cypress was acquired by infineon, so what will we
> do in few years? Will we rename all macros and methods to contain infineon?
> 
No, we should keep using legacy names.

> Let us know your thoughts and if you do this kind of change, explain why.
> 
> Cheers,
> ta
Thanks,
Takahiro
diff mbox series

Patch

diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c
index 43cd6cd92537..c8abe46e63fe 100644
--- a/drivers/mtd/spi-nor/spansion.c
+++ b/drivers/mtd/spi-nor/spansion.c
@@ -9,35 +9,36 @@ 
 #include "core.h"
 
 /* flash_info mfr_flag. Used to clear sticky prorietary SR bits. */
-#define USE_CLSR	BIT(0)
-
-#define SPINOR_OP_CLSR		0x30	/* Clear status register 1 */
-#define SPINOR_OP_RD_ANY_REG			0x65	/* Read any register */
-#define SPINOR_OP_WR_ANY_REG			0x71	/* Write any register */
-#define SPINOR_REG_CYPRESS_CFR2V		0x00800003
-#define SPINOR_REG_CYPRESS_CFR2V_MEMLAT_11_24	0xb
-#define SPINOR_REG_CYPRESS_CFR3V		0x00800004
-#define SPINOR_REG_CYPRESS_CFR3V_PGSZ		BIT(4) /* Page size. */
-#define SPINOR_REG_CYPRESS_CFR5V		0x00800006
-#define SPINOR_REG_CYPRESS_CFR5V_OCT_DTR_EN	0x3
-#define SPINOR_REG_CYPRESS_CFR5V_OCT_DTR_DS	0
-#define SPINOR_OP_CYPRESS_RD_FAST		0xee
+#define CYPRESS_NOR_USE_CLSR		BIT(0)
+
+#define CYPRESS_NOR_OP_CLSR		0x30	/* Clear status register 1 */
+#define CYPRESS_NOR_OP_RD_ANY_REG	0x65	/* Read any register */
+#define CYPRESS_NOR_OP_WR_ANY_REG	0x71	/* Write any register */
+#define CYPRESS_NOR_OP_RD_FAST		0xee
+
+#define CYPRESS_NOR_REG_CFR2V			0x00800003
+#define CYPRESS_NOR_REG_CFR2V_MEMLAT_11_24	0xb
+#define CYPRESS_NOR_REG_CFR3V			0x00800004
+#define CYPRESS_NOR_REG_CFR3V_PGSZ		BIT(4) /* Page size. */
+#define CYPRESS_NOR_REG_CFR5V			0x00800006
+#define CYPRESS_NOR_REG_CFR5V_OCT_DTR_EN	0x3
+#define CYPRESS_NOR_REG_CFR5V_OCT_DTR_DS	0
 
 /* Cypress SPI NOR flash operations. */
 #define CYPRESS_NOR_WR_ANY_REG_OP(naddr, addr, ndata, buf)		\
-	SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WR_ANY_REG, 0),		\
+	SPI_MEM_OP(SPI_MEM_OP_CMD(CYPRESS_NOR_OP_WR_ANY_REG, 0),	\
 		   SPI_MEM_OP_ADDR(naddr, addr, 0),			\
 		   SPI_MEM_OP_NO_DUMMY,					\
 		   SPI_MEM_OP_DATA_OUT(ndata, buf, 0))
 
 #define CYPRESS_NOR_RD_ANY_REG_OP(naddr, addr, buf)			\
-	SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_RD_ANY_REG, 0),		\
+	SPI_MEM_OP(SPI_MEM_OP_CMD(CYPRESS_NOR_OP_RD_ANY_REG, 0),	\
 		   SPI_MEM_OP_ADDR(naddr, addr, 0),			\
 		   SPI_MEM_OP_NO_DUMMY,					\
 		   SPI_MEM_OP_DATA_IN(1, buf, 0))
 
-#define SPANSION_CLSR_OP						\
-	SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_CLSR, 0),			\
+#define CYPRESS_NOR_CLSR_OP						\
+	SPI_MEM_OP(SPI_MEM_OP_CMD(CYPRESS_NOR_OP_CLSR, 0),		\
 		   SPI_MEM_OP_NO_ADDR,					\
 		   SPI_MEM_OP_NO_DUMMY,					\
 		   SPI_MEM_OP_NO_DATA)
@@ -49,9 +50,9 @@  static int cypress_nor_octal_dtr_en(struct spi_nor *nor)
 	int ret;
 
 	/* Use 24 dummy cycles for memory array reads. */
-	*buf = SPINOR_REG_CYPRESS_CFR2V_MEMLAT_11_24;
+	*buf = CYPRESS_NOR_REG_CFR2V_MEMLAT_11_24;
 	op = (struct spi_mem_op)
-		CYPRESS_NOR_WR_ANY_REG_OP(3, SPINOR_REG_CYPRESS_CFR2V, 1, buf);
+		CYPRESS_NOR_WR_ANY_REG_OP(3, CYPRESS_NOR_REG_CFR2V, 1, buf);
 
 	ret = spi_nor_write_any_volatile_reg(nor, &op, nor->reg_proto);
 	if (ret)
@@ -60,9 +61,9 @@  static int cypress_nor_octal_dtr_en(struct spi_nor *nor)
 	nor->read_dummy = 24;
 
 	/* Set the octal and DTR enable bits. */
-	buf[0] = SPINOR_REG_CYPRESS_CFR5V_OCT_DTR_EN;
+	buf[0] = CYPRESS_NOR_REG_CFR5V_OCT_DTR_EN;
 	op = (struct spi_mem_op)
-		CYPRESS_NOR_WR_ANY_REG_OP(3, SPINOR_REG_CYPRESS_CFR5V, 1, buf);
+		CYPRESS_NOR_WR_ANY_REG_OP(3, CYPRESS_NOR_REG_CFR5V, 1, buf);
 
 	ret = spi_nor_write_any_volatile_reg(nor, &op, nor->reg_proto);
 	if (ret)
@@ -92,10 +93,10 @@  static int cypress_nor_octal_dtr_dis(struct spi_nor *nor)
 	 * in 8D-8D-8D mode. Since there is no register at the next location,
 	 * just initialize the value to 0 and let the transaction go on.
 	 */
-	buf[0] = SPINOR_REG_CYPRESS_CFR5V_OCT_DTR_DS;
+	buf[0] = CYPRESS_NOR_REG_CFR5V_OCT_DTR_DS;
 	buf[1] = 0;
 	op = (struct spi_mem_op)
-		CYPRESS_NOR_WR_ANY_REG_OP(4, SPINOR_REG_CYPRESS_CFR5V, 2, buf);
+		CYPRESS_NOR_WR_ANY_REG_OP(4, CYPRESS_NOR_REG_CFR5V, 2, buf);
 	ret = spi_nor_write_any_volatile_reg(nor, &op, SNOR_PROTO_8_8_8_DTR);
 	if (ret)
 		return ret;
@@ -143,7 +144,7 @@  static void s28hs512t_post_sfdp_fixup(struct spi_nor *nor)
 	 */
 	if (nor->params->reads[SNOR_CMD_READ_8_8_8_DTR].opcode == 0)
 		nor->params->reads[SNOR_CMD_READ_8_8_8_DTR].opcode =
-			SPINOR_OP_CYPRESS_RD_FAST;
+			CYPRESS_NOR_OP_RD_FAST;
 
 	/* This flash is also missing the 4-byte Page Program opcode bit. */
 	spi_nor_set_pp_settings(&nor->params->page_programs[SNOR_CMD_PP],
@@ -227,51 +228,51 @@  static const struct flash_info spansion_nor_parts[] = {
 		NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
 	{ "s25fl128s0", INFO6(0x012018, 0x4d0080, 256 * 1024, 64)
 		NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
-		MFR_FLAGS(USE_CLSR)
+		MFR_FLAGS(CYPRESS_NOR_USE_CLSR)
 	},
 	{ "s25fl128s1", INFO6(0x012018, 0x4d0180, 64 * 1024, 256)
 		NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
-		MFR_FLAGS(USE_CLSR)
+		MFR_FLAGS(CYPRESS_NOR_USE_CLSR)
 	},
 	{ "s25fl256s0", INFO6(0x010219, 0x4d0080, 256 * 1024, 128)
 		NO_SFDP_FLAGS(SPI_NOR_SKIP_SFDP | SPI_NOR_DUAL_READ |
 			      SPI_NOR_QUAD_READ)
-		MFR_FLAGS(USE_CLSR)
+		MFR_FLAGS(CYPRESS_NOR_USE_CLSR)
 	},
 	{ "s25fl256s1", INFO6(0x010219, 0x4d0180, 64 * 1024, 512)
 		NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
-		MFR_FLAGS(USE_CLSR)
+		MFR_FLAGS(CYPRESS_NOR_USE_CLSR)
 	},
 	{ "s25fl512s",  INFO6(0x010220, 0x4d0080, 256 * 1024, 256)
 		FLAGS(SPI_NOR_HAS_LOCK)
 		NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
-		MFR_FLAGS(USE_CLSR)
+		MFR_FLAGS(CYPRESS_NOR_USE_CLSR)
 	},
 	{ "s25fs128s1", INFO6(0x012018, 0x4d0181, 64 * 1024, 256)
 		NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
-		MFR_FLAGS(USE_CLSR)
+		MFR_FLAGS(CYPRESS_NOR_USE_CLSR)
 		.fixups = &s25fs_s_nor_fixups, },
 	{ "s25fs256s0", INFO6(0x010219, 0x4d0081, 256 * 1024, 128)
 		NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
-		MFR_FLAGS(USE_CLSR)
+		MFR_FLAGS(CYPRESS_NOR_USE_CLSR)
 	},
 	{ "s25fs256s1", INFO6(0x010219, 0x4d0181, 64 * 1024, 512)
 		NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
-		MFR_FLAGS(USE_CLSR)
+		MFR_FLAGS(CYPRESS_NOR_USE_CLSR)
 	},
 	{ "s25fs512s",  INFO6(0x010220, 0x4d0081, 256 * 1024, 256)
 		NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
-		MFR_FLAGS(USE_CLSR)
+		MFR_FLAGS(CYPRESS_NOR_USE_CLSR)
 		.fixups = &s25fs_s_nor_fixups, },
 	{ "s25sl12800", INFO(0x012018, 0x0300, 256 * 1024,  64) },
 	{ "s25sl12801", INFO(0x012018, 0x0301,  64 * 1024, 256) },
 	{ "s25fl129p0", INFO(0x012018, 0x4d00, 256 * 1024,  64)
 		NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
-		MFR_FLAGS(USE_CLSR)
+		MFR_FLAGS(CYPRESS_NOR_USE_CLSR)
 	},
 	{ "s25fl129p1", INFO(0x012018, 0x4d01,  64 * 1024, 256)
 		NO_SFDP_FLAGS(SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
-		MFR_FLAGS(USE_CLSR)
+		MFR_FLAGS(CYPRESS_NOR_USE_CLSR)
 	},
 	{ "s25sl004a",  INFO(0x010212,      0,  64 * 1024,   8) },
 	{ "s25sl008a",  INFO(0x010213,      0,  64 * 1024,  16) },
@@ -328,13 +329,14 @@  static void spansion_nor_clear_sr(struct spi_nor *nor)
 	int ret;
 
 	if (nor->spimem) {
-		struct spi_mem_op op = SPANSION_CLSR_OP;
+		struct spi_mem_op op = CYPRESS_NOR_CLSR_OP;
 
 		spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
 
 		ret = spi_mem_exec_op(nor->spimem, &op);
 	} else {
-		ret = spi_nor_controller_ops_write_reg(nor, SPINOR_OP_CLSR,
+		ret = spi_nor_controller_ops_write_reg(nor,
+						       CYPRESS_NOR_OP_CLSR,
 						       NULL, 0);
 	}
 
@@ -390,7 +392,7 @@  static void spansion_nor_late_init(struct spi_nor *nor)
 		nor->mtd.erasesize = nor->info->sector_size;
 	}
 
-	if (nor->info->mfr_flags & USE_CLSR)
+	if (nor->info->mfr_flags & CYPRESS_NOR_USE_CLSR)
 		nor->params->ready = spansion_nor_sr_ready_and_clear;
 }