Daniel Gröber (dxld) would like Daniel Gröber to review this change.

View Change

spi: Use common code for top-level flash block protection functions

This turns *_get_write_protection and *_set_write_protection into common
code by having the vendor code implement getter/setter functions for
bpbits and convertion functions between bpbits and a region.

Change-Id: I509a1cedff5de4bee34b8856ce651d100a32fd13
Signed-off-by: Daniel Gröber <dxld@darkboxed.org>
---
M src/drivers/spi/macronix.c
M src/drivers/spi/spi_flash.c
M src/drivers/spi/spi_winbond.h
M src/drivers/spi/winbond.c
M src/include/spi_flash.h
5 files changed, 156 insertions(+), 222 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/42123/1
diff --git a/src/drivers/spi/macronix.c b/src/drivers/spi/macronix.c
index 40f4a47..e5a7b98 100644
--- a/src/drivers/spi/macronix.c
+++ b/src/drivers/spi/macronix.c
@@ -316,64 +316,6 @@
return 0;
}

-
-/*
- * Read block protect bits from Status Reg.
- * Converts block protection bits to a region.
- *
- * Returns:
- * -1 on error
- * 1 if region is covered by write protection
- * 0 if a part of region isn't covered by write protection
- */
-static int macronix_get_write_protection(const struct spi_flash *flash,
- const struct region *region)
-{
- const struct spi_flash_part_id *params;
- struct region wp_region;
- int ret;
-
- params = flash->part;
-
- if (!params)
- return -1;
-
- if (!params->protection_granularity_shift)
- return -1;
-
- struct spi_flash_bpbits bpbits = {};
- ret = macronix_get_bpbits(flash, params, &bpbits);
- if (!ret)
- return ret;
-
- ret = macronix_bpbits_to_region(flash, params, &bpbits, &wp_region);
- if (!ret)
- return ret;
-
- if (!region_sz(&wp_region)) {
- printk(BIOS_DEBUG, "MACRONIX: flash isn't protected\n");
- return 0;
- }
-
- printk(BIOS_DEBUG, "MACRONIX: flash protected range 0x%08zx-0x%08zx\n",
- region_offset(&wp_region), region_end(&wp_region));
-
- return region_is_subregion(&wp_region, region);
-}
-
-/*
- * Protect a region starting from start of flash or end of flash.
- * The caller must provide a supported protected region size.
- * Writes block protect bits to Status Reg.
- * Optionally lock the status register if lock_sreg is set with the provided
- * mode.
- *
- * @param flash: The flash to operate on
- * @param region: The region to write protect
- * @param mode: Optional status register lock-down mode
- *
- * @return 0 on success
- */
static int macronix_set_bpbits(const struct spi_flash *flash,
const struct spi_flash_part_id *params,
const struct spi_flash_bpbits *bits)
@@ -400,52 +342,12 @@
return 0;
}

-
-static int
-macronix_set_write_protection(const struct spi_flash *flash,
- const struct region *region,
- const enum spi_flash_status_reg_lockdown mode)
-{
- const struct spi_flash_part_id *params;
- int ret;
-
- printk(BIOS_DEBUG, "MACRONIX: want to set protection for"
- " 0x%08zx-0x%08zx\n", region_offset(region), region_end(region));
-
- params = flash->part;
-
- if (!params)
- return -1;
-
- if (!params->protection_granularity_shift) {
- printk(BIOS_ERR, "MACRONIX: ERROR: spi_flash part doesn't support block protection\n");
- return -1;
- }
-
- struct spi_flash_bpbits bpbits = {};
- ret = macronix_get_bpbits(flash, params, &bpbits);
- if (!ret)
- return ret;
-
- ret = macronix_region_to_bpbits(flash, params, region, mode, &bpbits);
- if (!ret) {
- printk(BIOS_ERR, "MACRONIX: ERROR: unsupported block protection"
- " region requested: 0x%08zx-0x%08zx\n",
- region_offset(region), region_end(region));
- return ret;
- }
-
- ret = macronix_set_bpbits(flash, params, &bpbits);
- if (!ret)
- return ret;
-
- return ret;
-}
-
-
static const struct spi_flash_protection_ops spi_flash_protection_ops = {
- .get_write = macronix_get_write_protection,
- .set_write = macronix_set_write_protection,
+ .get_bpbits = macronix_get_bpbits,
+ .set_bpbits = macronix_set_bpbits,
+
+ .bpbits_to_region = macronix_bpbits_to_region,
+ .region_to_bpbits = macronix_region_to_bpbits,
};

const struct spi_flash_vendor_info spi_flash_macronix_vi = {
diff --git a/src/drivers/spi/spi_flash.c b/src/drivers/spi/spi_flash.c
index 33416b5..e57076b 100644
--- a/src/drivers/spi/spi_flash.c
+++ b/src/drivers/spi/spi_flash.c
@@ -532,6 +532,50 @@
return -1;
}

+/*
+ * Read block protect bits from Status Reg and convert to a region.
+ *
+ * Returns:
+ * -1 on error
+ * 1 if region is covered by write protection
+ * 0 if a part of region isn't covered by write protection
+ */
+static int spi_flash_get_write_protection(const struct spi_flash *flash,
+ const struct region *region)
+{
+ const struct spi_flash_part_id *params;
+ struct spi_flash_bpbits bpbits = {};
+ struct region wp_region;
+ int ret;
+
+ params = flash->part;
+ if (!params)
+ return -1;
+
+ if (!params->protection_granularity_shift)
+ return -1;
+
+ ret = flash->prot_ops->get_bpbits(flash, params, &bpbits);
+ if (ret)
+ return ret;
+
+ ret = flash->prot_ops->bpbits_to_region(flash, params, &bpbits,
+ &wp_region);
+ if (ret)
+ return ret;
+
+ if (!region_sz(&wp_region)) {
+ printk(BIOS_DEBUG, "SPI: flash isn't write-protected\n");
+ return 0;
+ }
+
+ printk(BIOS_DEBUG, "SPI: flash write protected range:"
+ " 0x%08zx-0x%08zx\n", region_offset(&wp_region),
+ region_end(&wp_region));
+
+ return region_is_subregion(&wp_region, region);
+}
+
int spi_flash_is_write_protected(const struct spi_flash *flash,
const struct region *region)
{
@@ -551,7 +595,64 @@
return -1;
}

- return flash->prot_ops->get_write(flash, region);
+ return spi_flash_get_write_protection(flash, region);
+}
+
+/*
+ * Protect a region starting from start of flash or end of flash.
+ * The caller must provide a supported protected region size.
+ * Writes block protect bits to Status Reg.
+ * Optionally lock the status register if appropriate mode is used.
+ *
+ * @param flash: The flash to operate on
+ * @param region: The region to write protect
+ * @param mode: Optional status register lock-down mode
+ *
+ * @return 0 on success
+ */
+static int spi_flash_set_write_protection(
+ const struct spi_flash *flash,
+ const struct region *region,
+ const enum spi_flash_status_reg_lockdown mode)
+{
+ const struct spi_flash_part_id *params;
+ int ret;
+
+ printk(BIOS_DEBUG, "SPI: want to set protection for"
+ " 0x%08zx-0x%08zx\n", region_offset(region), region_end(region));
+
+ params = flash->part;
+ if (!params)
+ return -1;
+
+ if (!params->protection_granularity_shift) {
+ printk(BIOS_ERR, "SPI: ERROR: flash part doesn't support block"
+ " protection\n");
+ return -1;
+ }
+
+ struct spi_flash_bpbits bpbits = {};
+ ret = flash->prot_ops->get_bpbits(flash, params, &bpbits);
+ if (ret)
+ return ret;
+
+ ret = flash->prot_ops->region_to_bpbits(flash, params, region, mode,
+ &bpbits);
+ if (ret) {
+ printk(BIOS_ERR, "SPI: ERROR: unsupported block protection"
+ " region requested: 0x%08zx-0x%08zx\n",
+ region_offset(region), region_end(region));
+ return ret;
+ }
+
+ ret = flash->prot_ops->set_bpbits(flash, params, &bpbits);
+ if (ret)
+ return ret;
+
+ printk(BIOS_DEBUG, "SPI: write-protection range set to "
+ "0x%08zx-0x%08zx\n", region_offset(region), region_end(region));
+
+ return 0;
}

int spi_flash_set_write_protected(const struct spi_flash *flash,
@@ -575,7 +676,7 @@
return -1;
}

- ret = flash->prot_ops->set_write(flash, region, mode);
+ ret = spi_flash_set_write_protection(flash, region, mode);

if (ret == 0 && mode != SPI_WRITE_PROTECTION_PRESERVE) {
printk(BIOS_INFO, "SPI: SREG lock-down was set to ");
diff --git a/src/drivers/spi/spi_winbond.h b/src/drivers/spi/spi_winbond.h
index 2a3a4ae..b4f455a8 100644
--- a/src/drivers/spi/spi_winbond.h
+++ b/src/drivers/spi/spi_winbond.h
@@ -29,7 +29,7 @@
const enum spi_flash_status_reg_lockdown mode,
struct spi_flash_bpbits *bits);

-void winbond_bpbits_to_region(
+int winbond_bpbits_to_region(
const struct spi_flash *flash,
const struct spi_flash_part_id *params,
const struct spi_flash_bpbits *bits,
diff --git a/src/drivers/spi/winbond.c b/src/drivers/spi/winbond.c
index d127287..9605279 100644
--- a/src/drivers/spi/winbond.c
+++ b/src/drivers/spi/winbond.c
@@ -219,10 +219,10 @@
* Convert BPx, TB and CMP to a region.
* SEC (if available) must be zero.
*/
-void winbond_bpbits_to_region(const struct spi_flash *flash,
- const struct spi_flash_part_id *params,
- const struct spi_flash_bpbits *bits,
- struct region *out)
+int winbond_bpbits_to_region(const struct spi_flash *flash,
+ const struct spi_flash_part_id *params,
+ const struct spi_flash_bpbits *bits,
+ struct region *out)
{
const size_t granularity = 1 << params->protection_granularity_shift;
size_t protected_size =
@@ -244,46 +244,8 @@
out->offset = tb ? 0 : flash->size - protected_size;
out->size = protected_size;
}
-}

-/*
- * Available on all devices.
- * Read block protect bits from Status/Status2 Reg.
- * Converts block protection bits to a region.
- *
- * Returns:
- * -1 on error
- * 1 if region is covered by write protection
- * 0 if a part of region isn't covered by write protection
- */
-static int winbond_get_write_protection(const struct spi_flash *flash,
- const struct region *region)
-{
- const struct spi_flash_part_id *params;
- struct region wp_region;
- struct spi_flash_bpbits bpbits;
- int ret;
-
- params = flash->part;
- if (!params)
- return -1;
-
- ret = winbond_get_bpbits(flash, params, &bpbits);
- if (ret)
- return ret;
-
- winbond_bpbits_to_region(flash, params, &bpbits, &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;
}

static int winbond_get_bpbits(const struct spi_flash *flash,
@@ -555,56 +517,12 @@
return 0;
}

-/*
- * Available on all devices.
- * Protect a region starting from start of flash or end of flash.
- * The caller must provide a supported protected region size.
- * SEC isn't supported and set to zero.
- * Write block protect bits to Status/Status2 Reg.
- * Optionally lock the status register if lock_sreg is set with the provided
- * mode.
- *
- * @param flash: The flash to operate on
- * @param region: The region to write protect
- * @param mode: Optional status register lock-down mode
- *
- * @return 0 on success
- */
-static int
-winbond_set_write_protection(const struct spi_flash *flash,
- const struct region *region,
- const enum spi_flash_status_reg_lockdown mode)
-{
- const struct spi_flash_part_id *params;
- struct spi_flash_bpbits bpbits;
- int ret;
-
- params = flash->part;
- if (!params)
- return -1;
-
- ret = winbond_get_bpbits(flash, params, &bpbits);
- if (!ret)
- return ret;
-
- ret = winbond_region_to_bpbits(flash, params, region, mode, &bpbits);
- if (!ret)
- return ret;
-
-
- ret = winbond_set_bpbits(flash, params, &bpbits);
- if (!ret)
- return ret;
-
- printk(BIOS_DEBUG, "WINBOND: write-protection set to range "
- "0x%08zx-0x%08zx\n", region_offset(region), region_end(region));
-
- return ret;
-}
-
static const struct spi_flash_protection_ops spi_flash_protection_ops = {
- .get_write = winbond_get_write_protection,
- .set_write = winbond_set_write_protection,
+ .get_bpbits = winbond_get_bpbits,
+ .set_bpbits = winbond_set_bpbits,
+
+ .bpbits_to_region = winbond_bpbits_to_region,
+ .region_to_bpbits = winbond_region_to_bpbits,
};

const struct spi_flash_vendor_info spi_flash_winbond_vi = {
diff --git a/src/include/spi_flash.h b/src/include/spi_flash.h
index fe62069..bf84ce0 100644
--- a/src/include/spi_flash.h
+++ b/src/include/spi_flash.h
@@ -85,37 +85,50 @@
};
};

+struct spi_flash_part_id;
+
/* Current code assumes all callbacks are supplied in this object. */
struct spi_flash_protection_ops {
- /*
- * Returns 1 if the whole region is software write protected.
- * Hardware write protection mechanism aren't accounted.
- * If the write protection could be changed, due to unlocked status
- * register for example, 0 should be returned.
- * Returns 0 on success.
- */
- int (*get_write)(const struct spi_flash *flash,
- const struct region *region);
- /*
- * Enable the status register write protection, if supported on the
- * requested region, and optionally enable status register lock-down.
- * Returns 0 if the whole region was software write protected.
- * Hardware write protection mechanism aren't accounted.
- * If the status register is locked and the requested configuration
- * doesn't match the selected one, return an error.
- * Only a single region is supported !
+
+ /**
+ * Read current block protection bits from device status registers.
*
- * @return 0 on success
+ * Returns non-zero on error, zero on success.
*/
- int
- (*set_write)(const struct spi_flash *flash,
+ int (*get_bpbits)(const struct spi_flash *flash,
+ const struct spi_flash_part_id *part,
+ struct spi_flash_bpbits *bpbits);
+ /**
+ * Write block protection bits to non-volatile device status registers.
+ *
+ * Returns non-zero on error, zero on success.
+ */
+ int (*set_bpbits)(const struct spi_flash *flash,
+ const struct spi_flash_part_id *part,
+ const struct spi_flash_bpbits *bpbits);
+
+ /**
+ * Convert block protection bits to write-protected address region.
+ *
+ * Returns non-zero on error, zero on success.
+ */
+ int (*bpbits_to_region)(const struct spi_flash *flash,
+ const struct spi_flash_part_id *part,
+ const struct spi_flash_bpbits *bpbits,
+ struct region *region);
+ /**
+ * Convert address region to be write-protected into block
+ * protection bits.
+ *
+ * Returns non-zero on error, zero on success.
+ */
+ int (*region_to_bpbits)(const struct spi_flash *flash,
+ const struct spi_flash_part_id *part,
const struct region *region,
- const enum spi_flash_status_reg_lockdown mode);
-
+ const enum spi_flash_status_reg_lockdown mode,
+ struct spi_flash_bpbits *bpbits);
};

-struct spi_flash_part_id;
-
struct spi_flash {
struct spi_slave spi;
u8 vendor;

To view, visit change 42123. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I509a1cedff5de4bee34b8856ce651d100a32fd13
Gerrit-Change-Number: 42123
Gerrit-PatchSet: 1
Gerrit-Owner: Daniel Gröber (dxld)
Gerrit-Reviewer: Daniel Gröber <dxld@darkboxed.org>
Gerrit-MessageType: newchange