[flashrom-gerrit] Change in flashrom[master]: ichspi: Refactor Flashrom HW Sequencing Operation Part I

[Subrata Banik (Code Review)]

Subrata Banik has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/62868 )

Change subject: ichspi: Refactor Flashrom HW Sequencing Operation Part I
......................................................................

ichspi: Refactor Flashrom HW Sequencing Operation Part I

List of changes:
1. Introduced useful macros (read/write/erase/status etc.) and used
throughout the SPI operations.
2. Drop unused macros.
3. SPI operations are using `control register (offset 0x6)` hence
converted 32-bit reads into 16-bit read to cover the only hardware
sequencing control register alone.

BUG=b:223630977
TEST=Able to perform read/write/erase operation on brya.

Signed-off-by: Subrata Banik 
Change-Id: I3ea74b668e5f8d8e4b3da2a8ad8b81f1813e1e80
---
M ichspi.c
1 file changed, 29 insertions(+), 17 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/68/62868/1

diff --git a/ichspi.c b/ichspi.c
index dc03422..3aaf751 100644
--- a/ichspi.c
+++ b/ichspi.c
@@ -78,11 +78,7 @@
  */

/* Changed HSFC Control bits */
-#define PCH100_HSFC_FCYCLE_OFF	(17 - 16)	/* 1-4: FLASH Cycle */
-#define PCH100_HSFC_FCYCLE	(0xf << PCH100_HSFC_FCYCLE_OFF)
 /* New HSFC Control bit */
-#define HSFC_WET_OFF		(21 - 16)	/* 5: Write Enable Type */
-#define HSFC_WET		(0x1 << HSFC_WET_OFF)

#define PCH100_FADDR_FLA	0x07ffffff

@@ -140,10 +136,23 @@
 #define HSFC_FGO_OFF		0	/* 0: Flash Cycle Go */
 #define HSFC_FGO		(0x1 << HSFC_FGO_OFF)
 #define HSFC_FCYCLE_OFF		1	/* 1-2: FLASH Cycle */
-#define HSFC_FCYCLE		(0x3 << HSFC_FCYCLE_OFF)
+#define HSFC_FCYCLE_BIT_WIDTH_ICH9	3 /* 2 bits */
+#define HSFC_FCYCLE_BIT_WIDTH_PCH	0xf /* 4 bits */
+#define HSFC_FCYCLE_MASK(n)	(n << HSFC_FCYCLE_OFF)
+#define HSFC_FCYCLE(cyc)	((cyc) << HSFC_FCYCLE_OFF)
+#define HSFC_CYCLE_READ		HSFC_FCYCLE(0)
+#define HSFC_CYCLE_WRITE	HSFC_FCYCLE(2)
+#define HSFC_CYCLE_4K_ERASE	HSFC_FCYCLE(3)
+#define HSFC_CYCLE_64K_ERASE	HSFC_FCYCLE(4)
+#define HSFC_CYCLE_RDID		HSFC_FCYCLE(6)
+#define HSFC_CYCLE_WR_STATUS	HSFC_FCYCLE(7)
+#define HSFC_CYCLE_RD_STATUS	HSFC_FCYCLE(8)
+#define HSFC_WET_OFF		5
+#define HSFC_WET		(0x1 << HSFC_WET_OFF)
    				/* 3-7: reserved */
 #define HSFC_FDBC_OFF		8	/* 8-13: Flash Data Byte Count */
-#define HSFC_FDBC		(0x3f << HSFC_FDBC_OFF)
+#define HSFC_FDBC_MASK		(0x3f << HSFC_FDBC_OFF)
+#define HSFC_FDBC(n)		(((n) << HSFC_FDBC_OFF) & HSFC_FDBC_MASK)
    				/* 14: reserved */
 #define HSFC_SME_OFF		15	/* 15: SPI SMI# Enable */
 #define HSFC_SME		(0x1 << HSFC_SME_OFF)
@@ -495,14 +504,16 @@
    case CHIPSET_400_SERIES_COMET_POINT:
    case CHIPSET_500_SERIES_TIGER_POINT:
    case CHIPSET_ELKHART_LAKE:
-		_pprint_reg(HSFC, PCH100_HSFC_FCYCLE, PCH100_HSFC_FCYCLE_OFF, reg_val, ", ");
+		_pprint_reg(HSFC, HSFC_FCYCLE_MASK(HSFC_FCYCLE_BIT_WIDTH_PCH), HSFC_FCYCLE_OFF,
+			    reg_val, ", ");
    	pprint_reg(HSFC, WET, reg_val, ", ");
    	break;
    default:
-		pprint_reg(HSFC, FCYCLE, reg_val, ", ");
+		_pprint_reg(HSFC, HSFC_FCYCLE_MASK(HSFC_FCYCLE_BIT_WIDTH_ICH9), HSFC_FCYCLE_OFF,
+			    reg_val, ", ");
    	break;
    }
-	pprint_reg(HSFC, FDBC, reg_val, ", ");
+	_pprint_reg(HSFC, HSFC_FDBC_MASK, HSFC_FDBC_OFF, reg_val, ", ");
    pprint_reg(HSFC, SME, reg_val, "\n");
 }

@@ -1441,7 +1452,7 @@

hsfc = REGREAD16(ICH9_REG_HSFC);
    hsfc &= ~hwseq_data.hsfc_fcycle; /* clear operation */
-	hsfc |= (0x3 << HSFC_FCYCLE_OFF); /* set erase operation */
+	hsfc |= HSFC_CYCLE_4K_ERASE; /* set erase operation */
    hsfc |= HSFC_FGO; /* start */
    msg_pdbg("HSFC used for block erasing: ");
    prettyprint_ich9_reg_hsfc(hsfc, ich_generation);
@@ -1483,9 +1494,10 @@

hsfc = REGREAD16(ICH9_REG_HSFC);
    	hsfc &= ~hwseq_data.hsfc_fcycle; /* set read operation */
-		hsfc &= ~HSFC_FDBC; /* clear byte count */
+		hsfc &= ~HSFC_FDBC_MASK; /* clear byte count */
+		hsfc |= HSFC_CYCLE_READ; /* set erase operation */
    	/* set byte count */
-		hsfc |= (((block_len - 1) << HSFC_FDBC_OFF) & HSFC_FDBC);
+		hsfc |= HSFC_FDBC(block_len - 1);
    	hsfc |= HSFC_FGO; /* start */
    	REGWRITE16(ICH9_REG_HSFC, hsfc);

@@ -1529,10 +1541,10 @@

hsfc = REGREAD16(ICH9_REG_HSFC);
    	hsfc &= ~hwseq_data.hsfc_fcycle; /* clear operation */
-		hsfc |= (0x2 << HSFC_FCYCLE_OFF); /* set write operation */
-		hsfc &= ~HSFC_FDBC; /* clear byte count */
+		hsfc |= HSFC_CYCLE_WRITE; /* set write operation */
+		hsfc &= ~HSFC_FDBC_MASK; /* clear byte count */
    	/* set byte count */
-		hsfc |= (((block_len - 1) << HSFC_FDBC_OFF) & HSFC_FDBC);
+		hsfc |= HSFC_FDBC(block_len - 1);
    	hsfc |= HSFC_FGO; /* start */
    	REGWRITE16(ICH9_REG_HSFC, hsfc);

@@ -1840,7 +1852,7 @@
    	swseq->reg_opmenu	= PCH100_REG_OPMENU;
    	hwseq->addr_mask	= PCH100_FADDR_FLA;
    	hwseq->only_4k		= true;
-		hwseq->hsfc_fcycle	= PCH100_HSFC_FCYCLE;
+		hwseq->hsfc_fcycle	= HSFC_FCYCLE_MASK(HSFC_FCYCLE_BIT_WIDTH_PCH);
    	break;
    default:
    	*num_pr			= 5;
@@ -1851,7 +1863,7 @@
    	swseq->reg_opmenu	= ICH9_REG_OPMENU;
    	hwseq->addr_mask	= ICH9_FADDR_FLA;
    	hwseq->only_4k		= false;
-		hwseq->hsfc_fcycle	= HSFC_FCYCLE;
+		hwseq->hsfc_fcycle	= HSFC_FCYCLE_MASK(HSFC_FCYCLE_BIT_WIDTH_ICH9);
    	break;
    }