[flashrom-gerrit] [S] Change in flashrom[master]: flashrom_tester: Check the WP state when setting

[Evan Benn (Code Review)]

Evan Benn has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/69402 )

Change subject: flashrom_tester: Check the WP state when setting
......................................................................

flashrom_tester: Check the WP state when setting

Check that the hardware and software WP state are as expected in the
setter methods.

BUG=None
BRANCH=None
TEST=None

Change-Id: Ie7f90ab478dca6f92eaa0908443e3cb156ea0319
Signed-off-by: Evan Benn 
---
M util/flashrom_tester/src/tester.rs
1 file changed, 38 insertions(+), 9 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/02/69402/1

diff --git a/util/flashrom_tester/src/tester.rs b/util/flashrom_tester/src/tester.rs
index 49a663c..96ba34e 100644
--- a/util/flashrom_tester/src/tester.rs
+++ b/util/flashrom_tester/src/tester.rs
@@ -220,28 +220,40 @@

/// Set the software write protect and check that the state is as expected.
     pub fn set_sw(&mut self, enable: bool) -> Result<&mut Self, String> {
-        info!("request={}, current={}", enable, self.current.sw);
+        info!("set_sw request={}, current={}", enable, self.current.sw);
         if self.current.sw != enable {
             self.cmd
                 .wp_toggle(/* en= */ enable)
                 .map_err(|e| e.to_string())?;
         }
-        Ok(self)
+        let actual_state = Self::get_sw(self.cmd).map_err(|e| e.to_string())?;
+        if actual_state != enable {
+            Err(format!("set_sw request={}, real={}", enable, actual_state))
+        } else {
+            self.current.sw = enable;
+            Ok(self)
+        }
     }

-    /// Set the hardware write protect.
+    /// Set the hardware write protect if supported and check that the state is as expected.
     pub fn set_hw(&mut self, enable: bool) -> Result<&mut Self, String> {
+        info!("set_hw request={}, current={}", enable, self.current.hw);
         if self.can_control_hw_wp() {
             if self.current.hw != enable {
                 super::utils::toggle_hw_wp(/* dis= */ !enable)?;
-                self.current.hw = enable;
-            } else if enable {
-                info!(
-                    "Ignoring attempt to enable hardware WP with {:?} programmer",
-                    self.fc
-                );
             }
+            // toggle_hw_wp does check, but we might not have called toggle_hw_wp so check again.
+            let actual_state = Self::get_hw(self.cmd)?;
+            if actual_state != enable {
+                return Err(format!("set_hw request={}, real={}", enable, actual_state));
+            }
+        } else {
+            info!(
+                "Ignoring attempt to set hardware WP with {:?} programmer",
+                self.fc
+            );
         }
+        self.current.hw = enable;
         Ok(self)
     }