[M] Change in flashrom[master]: flashrom_tester: Change the wp_toggle semantics

[Edward O'Callaghan (Code Review)]

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/69417 )

Change subject: flashrom_tester: Change the wp_toggle semantics
......................................................................

flashrom_tester: Change the wp_toggle semantics

wp_toggle and wp_range had some confusing behaviour where enabling wp
would set a range, but disabling wp would not unset the range
(explicitly).  This was a way to workaround the MTD kernel driver
semantics. Now make things very explicit, enabling software write
protect will set the range to the whole chip. Disabling write protect
will set the range to 0,0. This makes all drivers behave the same as
MTD, and documents the exact behaviour explicitly.

BUG=b:244663741
BRANCH=None
TEST=flashrom_tester --libflashrom host # MTD and non-MTD
TEST=flashrom_tester --flashrom_binary # MTD and non-MTD

Change-Id: Ia01d612d988e6580a7c5f0fd448ccc319ce9b181
Signed-off-by: Evan Benn 
Reviewed-on: https://review.coreboot.org/c/flashrom/+/69417
Tested-by: build bot (Jenkins) 
Reviewed-by: Edward O'Callaghan 
---
M util/flashrom_tester/flashrom/src/cmd.rs
M util/flashrom_tester/flashrom/src/flashromlib.rs
M util/flashrom_tester/flashrom/src/lib.rs
3 files changed, 40 insertions(+), 25 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Edward O'Callaghan: Looks good to me, approved

diff --git a/util/flashrom_tester/flashrom/src/cmd.rs b/util/flashrom_tester/flashrom/src/cmd.rs
index 4078910..458f054 100644
--- a/util/flashrom_tester/flashrom/src/cmd.rs
+++ b/util/flashrom_tester/flashrom/src/cmd.rs
@@ -149,11 +149,12 @@
         Ok(true)
     }

-    fn wp_range(&self, range: (i64, i64), wp_enable: bool) -> Result<bool, FlashromError> {
+    fn wp_range(&self, range: (i64, i64), en: bool) -> Result<bool, FlashromError> {
         let opts = FlashromOpt {
             wp_opt: WPOpt {
+                enable: en,
+                disable: !en,
                 range: Some(range),
-                enable: wp_enable,
                 ..Default::default()
             },
             ..Default::default()
@@ -200,28 +201,14 @@
     }

fn wp_toggle(&self, en: bool) -> Result<bool, FlashromError> {
-        let status = if en { "en" } else { "dis" };
-
-        // For MTD, --wp-range and --wp-enable must be used simultaneously.
         let range = if en {
             let rom_sz: i64 = self.get_size()?;
-            Some((0, rom_sz)) // (start, len)
+            (0, rom_sz) // (start, len)
         } else {
-            None
+            (0, 0)
         };
-
-        let opts = FlashromOpt {
-            wp_opt: WPOpt {
-                range,
-                enable: en,
-                disable: !en,
-                ..Default::default()
-            },
-            ..Default::default()
-        };
-
-        self.dispatch(opts, "wp_toggle")?;
-
+        self.wp_range(range, en)?;
+        let status = if en { "en" } else { "dis" };
         match self.wp_status(true) {
             Ok(_ret) => {
                 info!("Successfully {}abled write-protect", status);
diff --git a/util/flashrom_tester/flashrom/src/flashromlib.rs b/util/flashrom_tester/flashrom/src/flashromlib.rs
index 3036394..6cdd55d 100644
--- a/util/flashrom_tester/flashrom/src/flashromlib.rs
+++ b/util/flashrom_tester/flashrom/src/flashromlib.rs
@@ -102,10 +102,8 @@
     }

fn wp_toggle(&self, en: bool) -> Result<bool, FlashromError> {
-        // TODO why does the cmd impl not do this?
-        // for cmd, range is only set for enable
-        // and disable is not sent for the wp_range command
-        self.wp_range((0, self.get_size()?), en)
+        let range = if en { (0, self.get_size()?) } else { (0, 0) };
+        self.wp_range(range, en)
     }

fn read_into_file(&self, path: &Path) -> Result<(), FlashromError> {
diff --git a/util/flashrom_tester/flashrom/src/lib.rs b/util/flashrom_tester/flashrom/src/lib.rs
index 11874f9..90e40e2 100644
--- a/util/flashrom_tester/flashrom/src/lib.rs
+++ b/util/flashrom_tester/flashrom/src/lib.rs
@@ -133,7 +133,7 @@
     /// Write only a region of the flash.
     fn write_file_with_layout(&self, rws: &ROMWriteSpecifics) -> Result<bool, FlashromError>;

-    /// Set write protect status for a range.
+    /// Set write protect status and range.
     fn wp_range(&self, range: (i64, i64), wp_enable: bool) -> Result<bool, FlashromError>;

/// Read the write protect regions for the flash.
@@ -143,6 +143,10 @@
     fn wp_status(&self, en: bool) -> Result<bool, FlashromError>;

/// Set write protect status.
+    /// If en=true sets wp_range to the whole chip (0,getsize()).
+    /// If en=false sets wp_range to (0,0).
+    /// This is due to the MTD driver, which requires wp enable to use a range
+    /// length != 0 and wp disable to have the range 0,0.
     fn wp_toggle(&self, en: bool) -> Result<bool, FlashromError>;

/// Read the whole flash to the file specified by `path`.