[coreboot-gerrit] Change in coreboot[master]: spi/winbond: Pull out winbond_get_bpbits function

[Daniel Gröber (dxld) (Code Review)]

Hello Daniel Gröber,

I'd like you to do a code review. Please visit

https://review.coreboot.org/c/coreboot/+/42114

to review the following change.

Change subject: spi/winbond: Pull out winbond_get_bpbits function
......................................................................

spi/winbond: Pull out winbond_get_bpbits function

Split logic for retrieving bpbits values from
winbond_get_write_protection into a new function: winbond_get_bpbits.

Change-Id: Ifc23f0cce695cd8aebf5549a7ca098c08c759f37
Signed-off-by: Daniel Gröber 
---
M src/drivers/spi/winbond.c
1 file changed, 33 insertions(+), 15 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/42114/1

diff --git a/src/drivers/spi/winbond.c b/src/drivers/spi/winbond.c
index ccc7ae9..77191e4 100644
--- a/src/drivers/spi/winbond.c
+++ b/src/drivers/spi/winbond.c
@@ -208,6 +208,9 @@
    },
 };

+static int winbond_get_bpbits(const struct spi_flash *flash,
+			      struct spi_flash_bpbits *bpbits);
+
 /*
  * Convert BPx, TB and CMP to a region.
  * SEC (if available) must be zero.
@@ -255,8 +258,35 @@

const size_t granularity = (1 << params->protection_granularity_shift);

+	ret = winbond_get_bpbits(flash, &bpbits);
+	if (ret)
+		return ret;
+
+	winbond_bpbits_to_region(granularity, &bpbits, flash->size,
+				 &wp_region);
+
+	if (!region_sz(&wp_region)) {
+		printk(BIOS_DEBUG, "WINBOND: flash isn't protected\n");
+
+		return 0;
+	}
+
+	printk(BIOS_DEBUG, "WINBOND: flash protected range 0x%08zx-0x%08zx\n",
+	       region_offset(&wp_region), region_end(&wp_region));
+
+	return region_is_subregion(&wp_region, region);
+}
+
+static int winbond_get_bpbits(const struct spi_flash *flash,
+			      struct spi_flash_bpbits *bpbits)
+{
    union status_reg1 reg1 = { .u = 0 };
    union status_reg2 reg2 = { .u = 0 };
+	int ret;
+
+	const struct spi_flash_part_id *params = flash->part;
+	if (!params)
+		return -1;

ret = spi_flash_cmd(&flash->spi, flash->status_cmd, &reg1.u,
    		    sizeof(reg1.u));
@@ -274,7 +304,7 @@
    		return -1;
    	}

-		bpbits = (struct spi_flash_bpbits){
+		*bpbits = (struct spi_flash_bpbits){
    		.bp = reg1.bp3.bp,
    		.cmp = reg2.cmp,
    		.tb = reg1.bp3.tb,
@@ -284,7 +314,7 @@
    		},
    	};
    } else if (params->bp_bits == 4) {
-		bpbits = (struct spi_flash_bpbits){
+		*bpbits = (struct spi_flash_bpbits){
    		.bp = reg1.bp4.bp,
    		.cmp = reg2.cmp,
    		.tb = reg1.bp4.tb,
@@ -298,19 +328,7 @@
    	return -1;
    }

-	winbond_bpbits_to_region(granularity, &bpbits, flash->size,
-				 &wp_region);
-
-	if (!region_sz(&wp_region)) {
-		printk(BIOS_DEBUG, "WINBOND: flash isn't protected\n");
-
-		return 0;
-	}
-
-	printk(BIOS_DEBUG, "WINBOND: flash protected range 0x%08zx-0x%08zx\n",
-	       region_offset(&wp_region), region_end(&wp_region));
-
-	return region_is_subregion(&wp_region, region);
+	return 0;
 }

/**