[flashrom-gerrit] [M] Change in flashrom[master]: writeprotect.c: differentiate lack of WP from lack of implementation

[Sergii Dmytruk (Code Review)]

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons, Nikolai Artemiev.

Hello Nico Huber, Edward O'Callaghan, Angel Pons, Nikolai Artemiev,

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

https://review.coreboot.org/c/flashrom/+/68221

to review the following change.

Change subject: writeprotect.c: differentiate lack of WP from lack of implementation
......................................................................

writeprotect.c: differentiate lack of WP from lack of implementation

.tested.wp field allows telling these two cases apart, potentially
preventing confusion on seeing the error.

Change-Id: I6e75b124c21ccab51a9b5e1cd344b5310d384187
Signed-off-by: Sergii Dmytruk 
---
M cli_classic.c
M include/libflashrom.h
M writeprotect.c
3 files changed, 35 insertions(+), 15 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/21/68221/1

diff --git a/cli_classic.c b/cli_classic.c
index 3de0020..f790a83 100644
--- a/cli_classic.c
+++ b/cli_classic.c
@@ -202,6 +202,8 @@
    	return "the requested protection range is not supported";
    case FLASHROM_WP_ERR_RANGE_LIST_UNAVAILABLE:
    	return "could not determine what protection ranges are available";
+	case FLASHROM_WP_ERR_CHIP_HAS_NO_WP:
+		return "this chip doesn't support WP";
    }
    return "unknown WP error";
 }
diff --git a/include/libflashrom.h b/include/libflashrom.h
index 9bbdcc5..822877c 100644
--- a/include/libflashrom.h
+++ b/include/libflashrom.h
@@ -464,7 +464,8 @@
    FLASHROM_WP_ERR_VERIFY_FAILED = 5,
    FLASHROM_WP_ERR_RANGE_UNSUPPORTED = 6,
    FLASHROM_WP_ERR_MODE_UNSUPPORTED = 7,
-	FLASHROM_WP_ERR_RANGE_LIST_UNAVAILABLE = 8
+	FLASHROM_WP_ERR_RANGE_LIST_UNAVAILABLE = 8,
+	FLASHROM_WP_ERR_CHIP_HAS_NO_WP = 9
 };

enum flashrom_wp_mode {
diff --git a/writeprotect.c b/writeprotect.c
index 57d0f9b..a55b173 100644
--- a/writeprotect.c
+++ b/writeprotect.c
@@ -422,11 +422,20 @@
    }
 }

-static bool chip_supported(struct flashctx *flash)
+static bool verify_chip_support(struct flashctx *flash)
 {
-	return (flash->chip != NULL) && (flash->chip->decode_range != NULL);
-}
+	if (flash->chip == NULL)
+		return FLASHROM_WP_ERR_CHIP_HAS_NO_WP;

+	/* When not tested assume the chip has WP */
+	if (flash->chip->tested.wp == NA)
+		return FLASHROM_WP_ERR_CHIP_HAS_NO_WP;
+
+	if (flash->chip->decode_range == NULL)
+		return FLASHROM_WP_ERR_CHIP_UNSUPPORTED;
+
+	return FLASHROM_WP_OK;
+}

bool wp_operations_available(struct flashrom_flashctx *flash)
 {
@@ -439,10 +448,7 @@
 enum flashrom_wp_result wp_read_cfg(struct flashrom_wp_cfg *cfg, struct flashctx *flash)
 {
    struct wp_bits bits;
-	enum flashrom_wp_result ret = FLASHROM_WP_OK;
-
-	if (!chip_supported(flash))
-		ret = FLASHROM_WP_ERR_CHIP_UNSUPPORTED;
+	enum flashrom_wp_result ret = verify_chip_support(flash);

if (ret == FLASHROM_WP_OK)
    	ret = read_wp_bits(&bits, flash);
@@ -459,10 +465,7 @@
 enum flashrom_wp_result wp_write_cfg(struct flashctx *flash, const struct flashrom_wp_cfg *cfg)
 {
    struct wp_bits bits;
-	enum flashrom_wp_result ret = FLASHROM_WP_OK;
-
-	if (!chip_supported(flash))
-		ret = FLASHROM_WP_ERR_CHIP_UNSUPPORTED;
+	enum flashrom_wp_result ret = verify_chip_support(flash);

if (ret == FLASHROM_WP_OK)
    	ret = read_wp_bits(&bits, flash);
@@ -488,10 +491,11 @@
    struct wp_range_and_bits *range_pairs = NULL;
    size_t count;

-	if (!chip_supported(flash))
-		return FLASHROM_WP_ERR_CHIP_UNSUPPORTED;
+	enum flashrom_wp_result ret = verify_chip_support(flash);
+	if (ret != FLASHROM_WP_OK)
+		return ret;

-	enum flashrom_wp_result ret = read_wp_bits(&bits, flash);
+	ret = read_wp_bits(&bits, flash);
    if (ret != FLASHROM_WP_OK)
    	return ret;