Refactor SMFI interface and ectool

rust-toolchain

@@ -1 +1 @@
-nightly-2019-11-04
+nightly-2020-07-27

src/board/system76/common/scratch/scratch.mk

@@ -17,7 +17,6 @@ SCRATCH_CFLAGS+=-I$(SCRATCH_DIR)/include -D__SCRATCH__
 
 # Add minimal source from other directories
 SCRATCH_SRC+=\
-	$(COMMON_DIR)/version.c \
 	$(SYSTEM76_COMMON_DIR)/smfi.c
 
 SCRATCH_BUILD=$(BUILD)/scratch

src/board/system76/common/smfi.c

@@ -1,4 +1,17 @@
 // SPDX-License-Identifier: GPL-3.0-only
+//
+// This defines a protocol for clients on the AP (application processor) to
+// communicate with the EC. The protocol is polled, and uses semaphores to
+// ensure correct sequencing.
+//
+// The client should check that the SMFI_CMD_CMD register is set to CMD_NONE,
+// indicating that the EC is waiting for a command. The client should set the
+// SMFI_CMD_DATA region as necessary for the command they wish to issue. They
+// should then set the SMFI_CMD_CMD byte to indicate to the EC what command to
+// execute. The EC will execute the command, and then set SMFI_CMD_RES to the
+// correct result. It will finally set SMFI_CMD_CMD to CMD_NONE, to indicate
+// the command is complete and the result is available. The client should only
+// read the SMFI_CMD_RES value when SMFI_CMD_CMD is set to CMD_NONE.
 
 #include <stdint.h>
 #include <stdio.h>
@@ -29,6 +42,14 @@ volatile uint8_t __xdata __at(0x105D) HRAMW0AAS;
 volatile uint8_t __xdata __at(0x105E) HRAMW1AAS;
 // Flash control register 3
 volatile uint8_t __xdata __at(0x1063) FLHCTRL3;
+// Host RAM window 2 base address
+volatile uint8_t __xdata __at(0x1076) HRAMW2BA;
+// Host RAM window 3 base address
+volatile uint8_t __xdata __at(0x1077) HRAMW3BA;
+// Host RAM window 2 access allow size
+volatile uint8_t __xdata __at(0x1078) HRAMW2AAS;
+// Host RAM window 3 access allow size
+volatile uint8_t __xdata __at(0x1079) HRAMW3AAS;
 
 // EC indirect flash access
 volatile uint8_t __xdata __at(0x103B) ECINDAR0;
@@ -37,12 +58,135 @@ volatile uint8_t __xdata __at(0x103D) ECINDAR2;
 volatile uint8_t __xdata __at(0x103E) ECINDAR3;
 volatile uint8_t __xdata __at(0x103F) ECINDDR;
 
+// Command region - allows client to send commands to EC
+#define SMFI_CMD_CMD 0x00
+#define SMFI_CMD_RES 0x01
+#define SMFI_CMD_DATA 0x02
 static volatile uint8_t __xdata __at(0xE00) smfi_cmd[256];
+
+// Debug region - ring buffer of EC firmware prints
+#define SMFI_DBG_TAIL 0x00
 static volatile uint8_t __xdata __at(0xF00) smfi_dbg[256];
 
+#if !defined(__SCRATCH__)
+void smfi_init(void) {
+    int i;
+
+    // Clear command region
+    for (i = (SMFI_CMD_CMD + 1); i < ARRAY_SIZE(smfi_cmd); i++) {
+        smfi_cmd[i] = 0x00;
+    }
+    // Clear host command last
+    smfi_cmd[SMFI_CMD_CMD] = 0x00;
+
+    // Clear debug region
+    for (i = (SMFI_DBG_TAIL + 1); i < ARRAY_SIZE(smfi_dbg); i++) {
+        smfi_dbg[i] = 0x00;
+    }
+    // Clear tail last
+    smfi_dbg[SMFI_DBG_TAIL] = 0x00;
+
+    // H2RAM window 0 address 0xE00 - 0xEFF, read/write
+    HRAMW0BA = 0xE0;
+    HRAMW0AAS = 0x04;
+
+    // H2RAM window 1 address 0xF00 - 0xFFF, read-only
+    HRAMW1BA = 0xF0;
+    HRAMW1AAS = 0x34;
+
+    // Enable H2RAM window 0 and 1 using LPC I/O
+    HRAMWC |= (1 << 4) | (1 << 1) | (1 << 0);
+
+    // Enable backup ROM access
+    FLHCTRL3 |= (1 << 3);
+}
+
+static enum Result cmd_print(void) {
+    uint8_t flags = smfi_cmd[SMFI_CMD_DATA];
+    uint8_t len = smfi_cmd[SMFI_CMD_DATA + 1];
+
+    uint8_t i;
+    for (i = 0; (i < len) && ((i + SMFI_CMD_DATA + 2) < ARRAY_SIZE(smfi_cmd)); i++) {
+        putchar(smfi_cmd[i + SMFI_CMD_DATA + 2]);
+    }
+
+    smfi_cmd[SMFI_CMD_DATA + 1] = i;
+
+    return RES_OK;
+}
+
+static enum Result cmd_fan_get(void) {
+    switch (smfi_cmd[SMFI_CMD_DATA]) {
+        case 0:
+            // Get duty of fan 0
+            smfi_cmd[SMFI_CMD_DATA + 1] = DCR2;
+            return RES_OK;
+        case 1:
+            // Get duty of fan 1
+            //TODO: only allow on platforms like addw2
+            smfi_cmd[SMFI_CMD_DATA + 1] = DCR4;
+            return RES_OK;
+    }
+
+    // Failed if fan not found
+    return RES_ERR;
+}
+
+static enum Result cmd_fan_set(void) {
+    switch (smfi_cmd[SMFI_CMD_DATA]) {
+        case 0:
+            // Set duty cycle of fan 0
+            DCR2 = smfi_cmd[SMFI_CMD_DATA + 1];
+            return RES_OK;
+        case 1:
+            // Set duty cycle of fan 1
+            //TODO: only allow on platforms like addw2
+            DCR4 = smfi_cmd[SMFI_CMD_DATA + 1];
+            return RES_OK;
+    }
+
+    // Failed if fan not found
+    return RES_ERR;
+}
+
+static enum Result cmd_keymap_get(void) {
+    int layer = smfi_cmd[SMFI_CMD_DATA];
+    int output = smfi_cmd[SMFI_CMD_DATA + 1];
+    int input = smfi_cmd[SMFI_CMD_DATA + 2];
+
+    if (layer < KM_LAY && output < KM_OUT && input < KM_IN) {
+        uint16_t key = KEYMAP[layer][output][input];
+        smfi_cmd[SMFI_CMD_DATA + 3] = (uint8_t)key;
+        smfi_cmd[SMFI_CMD_DATA + 4] = (uint8_t)(key >> 8);
+        return RES_OK;
+    }
+
+    // Failed if keyboard mapping not found
+    return RES_ERR;
+}
+
+static enum Result cmd_keymap_set(void) {
+    int layer = smfi_cmd[SMFI_CMD_DATA];
+    int output = smfi_cmd[SMFI_CMD_DATA + 1];
+    int input = smfi_cmd[SMFI_CMD_DATA + 2];
+
+    if (layer < KM_LAY && output < KM_OUT && input < KM_IN) {
+        uint16_t key =
+            ((uint16_t)smfi_cmd[SMFI_CMD_DATA + 3]) |
+            (((uint16_t)smfi_cmd[SMFI_CMD_DATA + 4]) << 8);
+        KEYMAP[layer][output][input] = key;
+        return RES_OK;
+    }
+
+    // Failed if keyboard mapping not found
+    return RES_ERR;
+}
+#endif // !defined(__SCRATCH__)
+
+#if defined(__SCRATCH__)
 static enum Result cmd_spi_scratch(void) __critical {
-    uint8_t flags = smfi_cmd[2];
+    uint8_t flags = smfi_cmd[SMFI_CMD_DATA];
-    uint8_t len = smfi_cmd[3];
+    uint8_t len = smfi_cmd[SMFI_CMD_DATA + 1];
 
     // Enable chip
     if (flags & CMD_SPI_FLAG_BACKUP) {
@@ -56,16 +200,16 @@ static enum Result cmd_spi_scratch(void) __critical {
 
     // Read or write len bytes
     uint8_t i;
-    for (i = 0; (i < len) && ((i + 4) < ARRAY_SIZE(smfi_cmd)); i++) {
+    for (i = 0; (i < len) && ((i + SMFI_CMD_DATA + 2) < ARRAY_SIZE(smfi_cmd)); i++) {
         if (flags & CMD_SPI_FLAG_READ) {
-            smfi_cmd[i + 4] = ECINDDR;
+            smfi_cmd[i + SMFI_CMD_DATA + 2] = ECINDDR;
         } else {
-            ECINDDR = smfi_cmd[i + 4];
+            ECINDDR = smfi_cmd[i + SMFI_CMD_DATA + 2];
         }
     }
 
     // Set actually read/written count
-    smfi_cmd[3] = i;
+    smfi_cmd[SMFI_CMD_DATA + 1] = i;
 
     if (flags & CMD_SPI_FLAG_DISABLE) {
         // Disable chip
@@ -75,65 +219,19 @@ static enum Result cmd_spi_scratch(void) __critical {
 
     return RES_OK;
 }
-
+#endif // defined(__SCRATCH__)
-void smfi_init(void) {
-    int i;
-
-    // Clear command region
-    for (i = 1; i < ARRAY_SIZE(smfi_cmd); i++) {
-        smfi_cmd[i] = 0x00;
-    }
-    // Clear host command last
-    smfi_cmd[0] = 0x00;
-
-    // Clear debug region
-    for (i = 1; i < ARRAY_SIZE(smfi_dbg); i++) {
-        smfi_dbg[i] = 0x00;
-    }
-    // Clear tail last
-    smfi_dbg[0] = 0x00;
-
-
-    // H2RAM window 0 address 0xE00 - 0xEFF, read/write
-    HRAMW0BA = 0xE0;
-    HRAMW0AAS = 0x04;
-
-    // H2RAM window 1 address 0xF00 - 0xFFF, read/write
-    HRAMW1BA = 0xF0;
-    HRAMW1AAS = 0x04;
-
-    // Enable H2RAM window 0 and 1 using LPC I/O
-    HRAMWC |= 0x13;
-
-    // Enable backup ROM access
-    FLHCTRL3 |= (1 << 3);
-}
-
-static enum Result cmd_print(void) {
-    uint8_t flags = smfi_cmd[2];
-    uint8_t len = smfi_cmd[3];
-
-    uint8_t i;
-    for (i = 0; (i < len) && ((i + 4) < ARRAY_SIZE(smfi_cmd)); i++) {
-        putchar(smfi_cmd[i + 4]);
-    }
-
-    smfi_cmd[3] = i;
-
-    return RES_OK;
-}
 
 static enum Result cmd_spi(void) {
-#ifdef __SCRATCH__
+#if defined(__SCRATCH__)
     return cmd_spi_scratch();
-#else
+#else // defined(__SCRATCH__)
-    if (smfi_cmd[2] & CMD_SPI_FLAG_SCRATCH) {
+    if (smfi_cmd[SMFI_CMD_DATA] & CMD_SPI_FLAG_SCRATCH) {
         scratch_trampoline();
     }
 
     // Cannot use follow mode unless running from scratch rom
     return RES_ERR;
-#endif
+#endif // defined(__SCRATCH__)
 }
 
 static enum Result cmd_reset(void) {
@@ -145,75 +243,6 @@ static enum Result cmd_reset(void) {
     return RES_ERR;
 }
 
-#ifndef __SCRATCH__
-static enum Result cmd_fan_get(void) {
-    switch (smfi_cmd[2]) {
-        case 0:
-            // Get duty of fan 0
-            smfi_cmd[3] = DCR2;
-            return RES_OK;
-        case 1:
-            // Get duty of fan 1
-            //TODO: only allow on platforms like addw2
-            smfi_cmd[3] = DCR4;
-            return RES_OK;
-    }
-
-    // Failed if fan not found
-    return RES_ERR;
-}
-
-static enum Result cmd_fan_set(void) {
-    switch (smfi_cmd[2]) {
-        case 0:
-            // Set duty cycle of fan 0
-            DCR2 = smfi_cmd[3];
-            return RES_OK;
-        case 1:
-            // Set duty cycle of fan 1
-            //TODO: only allow on platforms like addw2
-            DCR4 = smfi_cmd[3];
-            return RES_OK;
-    }
-
-    // Failed if fan not found
-    return RES_ERR;
-}
-
-static enum Result cmd_keymap_get(void) {
-    int layer = smfi_cmd[2];
-    int output = smfi_cmd[3];
-    int input = smfi_cmd[4];
-
-    if (layer < KM_LAY && output < KM_OUT && input < KM_IN) {
-        uint16_t key = KEYMAP[layer][output][input];
-        smfi_cmd[5] = (uint8_t)key;
-        smfi_cmd[6] = (uint8_t)(key >> 8);
-        return RES_OK;
-    }
-
-    // Failed if keyboard mapping not found
-    return RES_ERR;
-}
-
-static enum Result cmd_keymap_set(void) {
-    int layer = smfi_cmd[2];
-    int output = smfi_cmd[3];
-    int input = smfi_cmd[4];
-
-    if (layer < KM_LAY && output < KM_OUT && input < KM_IN) {
-        uint16_t key =
-            ((uint16_t)smfi_cmd[5]) |
-            (((uint16_t)smfi_cmd[6]) << 8);
-        KEYMAP[layer][output][input] = key;
-        return RES_OK;
-    }
-
-    // Failed if keyboard mapping not found
-    return RES_ERR;
-}
-#endif
-
 // Set a watchdog timer of 10 seconds
 void smfi_watchdog(void) {
     ET1CNTLLR = 0xFF;
@@ -222,72 +251,73 @@ void smfi_watchdog(void) {
 }
 
 void smfi_event(void) {
-    if (smfi_cmd[0]) {
+    if (smfi_cmd[SMFI_CMD_CMD]) {
-#ifdef __SCRATCH__
+#if defined(__SCRATCH__)
         // If in scratch ROM, restart watchdog timer when command received
         smfi_watchdog();
 #endif
 
-        switch (smfi_cmd[0]) {
+        switch (smfi_cmd[SMFI_CMD_CMD]) {
+#if !defined(__SCRATCH__)
             case CMD_PROBE:
                 // Signature
-                smfi_cmd[2] = 0x76;
+                smfi_cmd[SMFI_CMD_DATA + 0] = 0x76;
-                smfi_cmd[3] = 0xEC;
+                smfi_cmd[SMFI_CMD_DATA + 1] = 0xEC;
                 // Version
-                smfi_cmd[4] = 0x01;
+                smfi_cmd[SMFI_CMD_DATA + 2] = 0x01;
+                //TODO: bitmask of implemented commands?
                 // Always successful
-                smfi_cmd[1] = RES_OK;
+                smfi_cmd[SMFI_CMD_RES] = RES_OK;
                 break;
             case CMD_BOARD:
-                strncpy(&smfi_cmd[2], board(), ARRAY_SIZE(smfi_cmd) - 2);
+                strncpy(&smfi_cmd[SMFI_CMD_DATA], board(), ARRAY_SIZE(smfi_cmd) - SMFI_CMD_DATA);
                 // Always successful
-                smfi_cmd[1] = RES_OK;
+                smfi_cmd[SMFI_CMD_RES] = RES_OK;
                 break;
             case CMD_VERSION:
-                strncpy(&smfi_cmd[2], version(), ARRAY_SIZE(smfi_cmd) - 2);
+                strncpy(&smfi_cmd[SMFI_CMD_DATA], version(), ARRAY_SIZE(smfi_cmd) - SMFI_CMD_DATA);
                 // Always successful
-                smfi_cmd[1] = RES_OK;
+                smfi_cmd[SMFI_CMD_RES] = RES_OK;
                 break;
             case CMD_PRINT:
-                smfi_cmd[1] = cmd_print();
+                smfi_cmd[SMFI_CMD_RES] = cmd_print();
                 break;
-            case CMD_SPI:
-                smfi_cmd[1] = cmd_spi();
-                break;
-            case CMD_RESET:
-                smfi_cmd[1] = cmd_reset();
-                break;
-#ifndef __SCRATCH__
             case CMD_FAN_GET:
-                smfi_cmd[1] = cmd_fan_get();
+                smfi_cmd[SMFI_CMD_RES] = cmd_fan_get();
                 break;
             case CMD_FAN_SET:
-                smfi_cmd[1] = cmd_fan_set();
+                smfi_cmd[SMFI_CMD_RES] = cmd_fan_set();
                 break;
             case CMD_KEYMAP_GET:
-                smfi_cmd[1] = cmd_keymap_get();
+                smfi_cmd[SMFI_CMD_RES] = cmd_keymap_get();
                 break;
             case CMD_KEYMAP_SET:
-                smfi_cmd[1] = cmd_keymap_set();
+                smfi_cmd[SMFI_CMD_RES] = cmd_keymap_set();
+                break;
+#endif // !defined(__SCRATCH__)
+            case CMD_SPI:
+                smfi_cmd[SMFI_CMD_RES] = cmd_spi();
+                break;
+            case CMD_RESET:
+                smfi_cmd[SMFI_CMD_RES] = cmd_reset();
                 break;
-#endif // __SCRATCH__
             default:
                 // Command not found
-                smfi_cmd[1] = RES_ERR;
+                smfi_cmd[SMFI_CMD_RES] = RES_ERR;
                 break;
         }
 
         // Mark command as finished
-        smfi_cmd[0] = CMD_NONE;
+        smfi_cmd[SMFI_CMD_CMD] = CMD_NONE;
     }
 }
 
 void smfi_debug(unsigned char byte) {
-    int tail = (int)smfi_dbg[0];
+    int tail = (int)smfi_dbg[SMFI_DBG_TAIL];
     tail++;
     if (tail >= ARRAY_SIZE(smfi_dbg)) {
-        tail = 1;
+        tail = SMFI_DBG_TAIL + 1;
     }
     smfi_dbg[tail] = byte;
-    smfi_dbg[0] = (uint8_t)tail;
+    smfi_dbg[SMFI_DBG_TAIL] = (uint8_t)tail;
 }

tool/Cargo.lock

@@ -5,9 +5,14 @@ name = "lazy_static"
 version = "1.4.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 
+[[package]]
+name = "libc"
+version = "0.2.77"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+
 [[package]]
 name = "redox_hwio"
-version = "0.1.1"
+version = "0.1.3"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 dependencies = [
  "lazy_static 1.4.0 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -17,9 +22,11 @@ dependencies = [
 name = "system76_ectool"
 version = "0.1.3"
 dependencies = [
- "redox_hwio 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)",
+ "libc 0.2.77 (registry+https://github.com/rust-lang/crates.io-index)",
+ "redox_hwio 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
 [metadata]
 "checksum lazy_static 1.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "e2abad23fbc42b3700f2f279844dc832adb2b2eb069b2df918f455c4e18cc646"
-"checksum redox_hwio 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "847d8b48be152acb4b75dbc37d873ac54899389691f5de3358603c889883b25d"
+"checksum libc 0.2.77 (registry+https://github.com/rust-lang/crates.io-index)" = "f2f96b10ec2560088a8e76961b00d47107b3a625fecb76dedb29ee7ccbf98235"
+"checksum redox_hwio 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)" = "41aa2c4c67329a04106644cad336238aa5adecfd73d06fb10339d472ce6d8070"

tool/Cargo.toml

@@ -10,10 +10,15 @@ repository = "https://github.com/system76/ec"
 [lib]
 name = "ectool"
 
+[[bin]]
+name = "system76_ectool"
+required-features = ["std"]
+
 [dependencies]
-redox_hwio = "0.1.1"
+libc = { version = "0.2", optional = true }
+redox_hwio = "0.1.3"
 
 [features]
 default = ["std"]
 stable = ["redox_hwio/stable"]
-std = []
+std = ["libc"]

tool/src/access/lpc/direct.rs

@@ -0,0 +1,106 @@
+use hwio::{Io, Pio};
+
+use crate::{
+    Access,
+    Error,
+    SuperIo,
+    Timeout,
+    timeout,
+};
+
+use super::*;
+
+/// Use direct hardware access. Unsafe due to not having mutual exclusion
+pub struct AccessLpcDirect<T: Timeout> {
+    cmd: u16,
+    dbg: u16,
+    timeout: T,
+}
+
+impl<T: Timeout> AccessLpcDirect<T> {
+    /// Checks that Super I/O ID matches and then returns access object
+    pub unsafe fn new(timeout: T) -> Result<Self, Error> {
+        // Make sure EC ID matches
+        let mut sio = SuperIo::new(0x2E);
+        let id =
+            (sio.read(0x20) as u16) << 8 |
+            (sio.read(0x21) as u16);
+        match id {
+            0x5570 | 0x8587 => (),
+            _ => return Err(Error::SuperIoId(id)),
+        }
+
+        Ok(Self {
+            cmd: SMFI_CMD_BASE,
+            dbg: SMFI_DBG_BASE,
+            timeout,
+        })
+    }
+
+    /// Read from the command space
+    unsafe fn read_cmd(&mut self, addr: u8) -> u8 {
+        Pio::<u8>::new(
+            self.cmd + (addr as u16)
+        ).read()
+    }
+
+    /// Write to the command space
+    unsafe fn write_cmd(&mut self, addr: u8, data: u8) {
+        Pio::<u8>::new(
+            self.cmd + (addr as u16)
+        ).write(data)
+    }
+
+    /// Read from the debug space
+    //TODO: better public interface
+    pub unsafe fn read_debug(&mut self, addr: u8) -> u8 {
+        Pio::<u8>::new(
+            self.dbg + (addr as u16)
+        ).read()
+    }
+
+    /// Returns Ok if a command can be sent
+    unsafe fn command_check(&mut self) -> Result<(), Error> {
+        if self.read_cmd(SMFI_CMD_CMD) == 0 {
+            Ok(())
+        } else {
+            Err(Error::WouldBlock)
+        }
+    }
+}
+
+impl<T: Timeout> Access for AccessLpcDirect<T> {
+    unsafe fn command(&mut self, cmd: u8, data: &mut [u8]) -> Result<u8, Error> {
+        // Test data length
+        if data.len() > self.data_size() {
+            return Err(Error::DataLength(data.len()));
+        }
+
+        // All previous commands should be finished
+        self.command_check()?;
+
+        // Write data bytes, index should be valid due to length test above
+        for i in 0..data.len() {
+            self.write_cmd(i as u8 + SMFI_CMD_DATA, data[i]);
+        }
+
+        // Write command byte, which starts command
+        self.write_cmd(SMFI_CMD_CMD, cmd as u8);
+
+        // Wait for command to finish with timeout
+        self.timeout.reset();
+        timeout!(self.timeout, self.command_check())?;
+
+        // Read data bytes, index should be valid due to length test above
+        for i in 0..data.len() {
+            data[i] = self.read_cmd(i as u8 + SMFI_CMD_DATA);
+        }
+
+        // Return response byte
+        Ok(self.read_cmd(SMFI_CMD_RES))
+    }
+
+    fn data_size(&self) -> usize {
+        SMFI_CMD_SIZE - SMFI_CMD_DATA as usize
+    }
+}

tool/src/access/lpc/linux.rs

@@ -0,0 +1,183 @@
+use std::{
+    fs,
+    io::{
+        self,
+        Read,
+        Write,
+        Seek,
+        SeekFrom,
+    },
+    os::unix::io::AsRawFd,
+    path::Path,
+    time::Duration,
+};
+
+use crate::{
+    Access,
+    Error,
+    StdTimeout,
+    Timeout,
+    timeout,
+};
+
+use super::*;
+
+struct PortLock {
+    start: u16,
+    len: u16,
+    file: fs::File,
+}
+
+impl PortLock {
+    pub fn new(start: u16, end: u16) -> io::Result<Self> {
+        if end < start {
+            return Err(io::Error::new(
+                io::ErrorKind::InvalidInput,
+                "PortLock::new: end < start"
+            ));
+        }
+        let len = (end - start) + 1;
+
+        let file = fs::OpenOptions::new()
+            .read(true)
+            .write(true)
+            .open("/dev/port")?;
+
+        let mut flock = libc::flock {
+            l_type: libc::F_WRLCK as _,
+            l_whence: libc::SEEK_SET as _,
+            l_start: start as _,
+            l_len: len as _,
+            l_pid: 0,
+        };
+
+        if unsafe { libc::fcntl(file.as_raw_fd(), libc::F_SETLK, &mut flock) < 0 } {
+            return Err(io::Error::last_os_error());
+        }
+
+        Ok(Self {
+            start,
+            len,
+            file,
+        })
+    }
+
+    fn seek(&mut self, offset: u16) -> io::Result<()> {
+        if offset >= self.len {
+            return Err(io::Error::new(
+                io::ErrorKind::InvalidInput,
+                "PortLock::seek: offset >= len"
+            ));
+        }
+        let port = self.start + offset;
+        let pos = self.file.seek(SeekFrom::Start(port as u64))?;
+        if pos != port as u64 {
+            return Err(io::Error::new(
+                io::ErrorKind::UnexpectedEof,
+                "PortLock::seek: failed to seek to port"
+            ));
+        }
+        Ok(())
+    }
+
+    pub fn read(&mut self, offset: u16) -> io::Result<u8> {
+        self.seek(offset)?;
+        let mut data = [0];
+        self.file.read_exact(&mut data)?;
+        Ok(data[0])
+    }
+
+    pub fn write(&mut self, offset: u16, value: u8) -> io::Result<()> {
+        self.seek(offset)?;
+        self.file.write_all(&[value])
+    }
+}
+
+/// Use /dev/port access with file locking
+pub struct AccessLpcLinux {
+    cmd: PortLock,
+    dbg: PortLock,
+    timeout: StdTimeout,
+}
+
+impl AccessLpcLinux {
+    /// Locks ports and then returns access object
+    pub unsafe fn new(timeout: Duration) -> Result<Self, Error> {
+        // TODO: is there a better way to probe before running a command?
+        if ! Path::new("/sys/bus/acpi/devices/17761776:00").is_dir() {
+            return Err(Error::Io(io::Error::new(
+                io::ErrorKind::NotFound,
+                "Failed to find System76 ACPI device",
+            )));
+        }
+
+        let cmd = PortLock::new(SMFI_CMD_BASE, SMFI_CMD_BASE + SMFI_CMD_SIZE as u16 - 1).map_err(Error::Io)?;
+        let dbg = PortLock::new(SMFI_DBG_BASE, SMFI_DBG_BASE + SMFI_DBG_SIZE as u16 - 1).map_err(Error::Io)?;
+        Ok(Self {
+            cmd,
+            dbg,
+            timeout: StdTimeout::new(timeout),
+        })
+    }
+
+    /// Read from the command space
+    unsafe fn read_cmd(&mut self, addr: u8) -> Result<u8, Error> {
+        self.cmd.read(addr as u16).map_err(Error::Io)
+    }
+
+    /// Write to the command space
+    unsafe fn write_cmd(&mut self, addr: u8, data: u8) -> Result<(), Error> {
+        self.cmd.write(addr as u16, data).map_err(Error::Io)
+    }
+
+    /// Read from the debug space
+    //TODO: better public interface
+    pub unsafe fn read_debug(&mut self, addr: u8) -> Result<u8, Error> {
+        self.dbg.read(addr as u16).map_err(Error::Io)
+    }
+
+    /// Returns Ok if a command can be sent
+    unsafe fn command_check(&mut self) -> Result<(), Error> {
+        if self.read_cmd(SMFI_CMD_CMD)? == 0 {
+            Ok(())
+        } else {
+            Err(Error::WouldBlock)
+        }
+    }
+}
+
+impl Access for AccessLpcLinux {
+    unsafe fn command(&mut self, cmd: u8, data: &mut [u8]) -> Result<u8, Error> {
+        // Test data length
+        if data.len() > self.data_size() {
+            return Err(Error::DataLength(data.len()));
+        }
+
+        // All previous commands should be finished
+        self.command_check()?;
+
+        // Write data bytes, index should be valid due to length test above
+        for i in 0..data.len() {
+            self.write_cmd(i as u8 + SMFI_CMD_DATA, data[i])?;
+        }
+
+        // Write command byte, which starts command
+        self.write_cmd(SMFI_CMD_CMD, cmd as u8)?;
+
+        // Wait for command to finish with timeout
+        self.timeout.reset();
+        timeout!(self.timeout, self.command_check())?;
+
+        // Read data bytes, index should be valid due to length test above
+        for i in 0..data.len() {
+            data[i] = self.read_cmd(i as u8 + SMFI_CMD_DATA)?;
+        }
+
+        // Return response byte
+        self.read_cmd(SMFI_CMD_RES)
+    }
+
+    fn data_size(&self) -> usize {
+        SMFI_CMD_SIZE - SMFI_CMD_DATA as usize
+    }
+}

tool/src/access/lpc/mod.rs

@@ -0,0 +1,17 @@
+pub(crate) const SMFI_CMD_BASE: u16 = 0xE00;
+pub(crate) const SMFI_CMD_SIZE: usize = 0x100;
+
+pub(crate) const SMFI_DBG_BASE: u16 = 0xF00;
+pub(crate) const SMFI_DBG_SIZE: usize = 0x100;
+
+pub(crate) const SMFI_CMD_CMD: u8 = 0x00;
+pub(crate) const SMFI_CMD_RES: u8 = 0x01;
+pub(crate) const SMFI_CMD_DATA: u8 = 0x02;
+
+pub use self::direct::AccessLpcDirect;
+mod direct;
+
+#[cfg(all(feature = "std", target_os = "linux"))]
+pub use self::linux::AccessLpcLinux;
+#[cfg(all(feature = "std", target_os = "linux"))]
+mod linux;

tool/src/access/mod.rs

@@ -0,0 +1,13 @@
+use crate::Error;
+
+pub use self::lpc::*;
+mod lpc;
+
+/// Access method for running an EC command
+pub trait Access {
+    /// Sends a command using the access method. Only internal use is recommended
+    unsafe fn command(&mut self, cmd: u8, data: &mut [u8]) -> Result<u8, Error>;
+
+    /// The maximum size that can be provided for the data argument
+    fn data_size(&self) -> usize;
+}

tool/src/ec.rs

@@ -1,18 +1,14 @@
-use hwio::{Io, Pio};
-
 use crate::{
+    Access,
     Error,
     Spi,
     SpiTarget,
-    SuperIo,
-    Timeout,
-    timeout
 };
 
 #[derive(Clone, Copy, Debug)]
 #[repr(u8)]
-pub enum Cmd {
+enum Cmd {
-    None = 0,
+    // None = 0,
     Probe = 1,
     Board = 2,
     Version = 3,
@@ -25,88 +21,45 @@ pub enum Cmd {
     KeymapSet = 10,
 }
 
-pub const CMD_SPI_FLAG_READ: u8 = 1 << 0;
+const CMD_SPI_FLAG_READ: u8 = 1 << 0;
-pub const CMD_SPI_FLAG_DISABLE: u8 = 1 << 1;
+const CMD_SPI_FLAG_DISABLE: u8 = 1 << 1;
-pub const CMD_SPI_FLAG_SCRATCH: u8 = 1 << 2;
+const CMD_SPI_FLAG_SCRATCH: u8 = 1 << 2;
-pub const CMD_SPI_FLAG_BACKUP: u8 = 1 << 3;
+const CMD_SPI_FLAG_BACKUP: u8 = 1 << 3;
 
-pub struct Ec<T: Timeout> {
+/// Run EC commands using a provided access method
-    cmd: u16,
+pub struct Ec<A: Access> {
-    dbg: u16,
+    access: A,
-    timeout: T,
+    version: u8,
 }
 
-impl<T: Timeout> Ec<T> {
+impl<A: Access> Ec<A> {
     /// Probes for a compatible EC
-    pub unsafe fn new(timeout: T) -> Result<Self, Error> {
+    pub unsafe fn new(access: A) -> Result<Self, Error> {
-        let mut sio = SuperIo::new(0x2E);
+        // Create EC struct with provided access method and timeout
-
-        let id =
-            (sio.read(0x20) as u16) << 8 |
-            (sio.read(0x21) as u16);
-
-        match id {
-            0x5570 | 0x8587 => (),
-            _ => return Err(Error::SuperIoId(id)),
-        }
-
         let mut ec = Ec {
-            cmd: 0xE00,
+            access,
-            dbg: 0xF00,
+            version: 0,
-            timeout,
         };
 
-        ec.probe()?;
+        // Read version of protocol
+        ec.version = ec.probe()?;
+
+        // Make sure protocol version is supported
+        match ec.version {
+            1 => (),
+            _ => return Err(Error::Version(ec.version)),
+        }
 
         Ok(ec)
     }
 
-    /// Read from the command space
+    /// Unsafe access to access
-    pub unsafe fn read(&mut self, addr: u8) -> u8 {
+    pub unsafe fn access(&mut self) -> &mut A {
-        Pio::<u8>::new(
+        &mut self.access
-            self.cmd + (addr as u16)
-        ).read()
     }
 
-    /// Write to the command space
+    unsafe fn command(&mut self, cmd: Cmd, data: &mut [u8]) -> Result<(), Error> {
-    pub unsafe fn write(&mut self, addr: u8, data: u8) {
+        match self.access.command(cmd as u8, data)? {
-        Pio::<u8>::new(
-            self.cmd + (addr as u16)
-        ).write(data)
-    }
-
-    /// Read from the debug space
-    pub unsafe fn debug(&mut self, addr: u8) -> u8 {
-        Pio::<u8>::new(
-            self.dbg + (addr as u16)
-        ).read()
-    }
-
-    /// Returns Ok if a command can be sent
-    unsafe fn command_check(&mut self) -> Result<(), Error> {
-        if self.read(0) == Cmd::None as u8 {
-            Ok(())
-        } else {
-            Err(Error::WouldBlock)
-        }
-    }
-
-    /// Wait until a command can be sent
-    unsafe fn command_wait(&mut self) -> Result<(), Error> {
-        self.timeout.reset();
-        timeout!(self.timeout, self.command_check())
-    }
-
-    /// Run an EC command
-    pub unsafe fn command(&mut self, cmd: Cmd) -> Result<(), Error> {
-        // All previous commands should be finished
-        self.command_check()?;
-        // Write command byte
-        self.write(0, cmd as u8);
-        // Wait for command to finish
-        self.command_wait()?;
-        // Read response byte and test for error
-        match self.read(1) {
             0 => Ok(()),
             err => Err(Error::Protocol(err)),
         }
@@ -114,13 +67,11 @@ impl<T: Timeout> Ec<T> {
 
     /// Probe for EC
     pub unsafe fn probe(&mut self) -> Result<u8, Error> {
-        self.command(Cmd::Probe)?;
+        let mut data = [0; 3];
-        let signature = (
+        self.command(Cmd::Probe, &mut data)?;
-            self.read(2),
+        let signature = (data[0], data[1]);
-            self.read(3)
-        );
         if signature == (0x76, 0xEC) {
-            let version = self.read(4);
+            let version = data[2];
             Ok(version)
         } else {
             Err(Error::Signature(signature))
@@ -129,10 +80,9 @@ impl<T: Timeout> Ec<T> {
 
     /// Read board from EC
     pub unsafe fn board(&mut self, data: &mut [u8]) -> Result<usize, Error> {
-        self.command(Cmd::Board)?;
+        self.command(Cmd::Board, data)?;
         let mut i = 0;
-        while i < data.len() && (i + 2) < 256 {
+        while i < data.len() {
-            data[i] = self.read((i + 2) as u8);
             if data[i] == 0 {
                 break;
             }
@@ -143,10 +93,9 @@ impl<T: Timeout> Ec<T> {
 
     /// Read version from EC
     pub unsafe fn version(&mut self, data: &mut [u8]) -> Result<usize, Error> {
-        self.command(Cmd::Version)?;
+        self.command(Cmd::Version, data)?;
         let mut i = 0;
-        while i < data.len() && (i + 2) < 256 {
+        while i < data.len() {
-            data[i] = self.read((i + 2) as u8);
             if data[i] == 0 {
                 break;
             }
@@ -155,24 +104,27 @@ impl<T: Timeout> Ec<T> {
         Ok(i)
     }
 
+    /// Print data to EC console
     pub unsafe fn print(&mut self, data: &[u8]) -> Result<usize, Error> {
+        //TODO: use self.access.data_size()
         let flags = 0;
         for chunk in data.chunks(256 - 4) {
+            let mut data = [0; 256 - 2];
+            data[0] = flags;
+            data[1] = chunk.len() as u8;
             for i in 0..chunk.len() {
-                self.write(i as u8 + 4, chunk[i]);
+                data[i + 2] = chunk[i];
             }
-
+            self.command(Cmd::Print, &mut data)?;
-            self.write(2, flags);
+            if data[1] != chunk.len() as u8 {
-            self.write(3, chunk.len() as u8);
-            self.command(Cmd::Print)?;
-            if self.read(3) != chunk.len() as u8 {
                 return Err(Error::Verify);
             }
         }
         Ok(data.len())
     }
 
-    pub unsafe fn spi(&mut self, target: SpiTarget, scratch: bool) -> Result<EcSpi<T>, Error> {
+    /// Access EC SPI bus
+    pub unsafe fn spi(&mut self, target: SpiTarget, scratch: bool) -> Result<EcSpi<A>, Error> {
         let mut spi = EcSpi {
             ec: self,
             target,
@@ -182,50 +134,66 @@ impl<T: Timeout> Ec<T> {
         Ok(spi)
     }
 
+    /// Reset EC. Will also power off computer.
     pub unsafe fn reset(&mut self) -> Result<(), Error> {
-        self.command(Cmd::Reset)
+        self.command(Cmd::Reset, &mut [])
     }
 
+    /// Read fan duty cycle by fan index
     pub unsafe fn fan_get(&mut self, index: u8) -> Result<u8, Error> {
-        self.write(2, index);
+        let mut data = [
-        self.command(Cmd::FanGet)?;
+            index,
-        Ok(self.read(3))
+            0
+        ];
+        self.command(Cmd::FanGet, &mut data)?;
+        Ok(data[1])
     }
 
+    /// Set fan duty cycle by fan index
     pub unsafe fn fan_set(&mut self, index: u8, duty: u8) -> Result<(), Error> {
-        self.write(2, index);
+        let mut data = [
-        self.write(3, duty);
+            index,
-        self.command(Cmd::FanSet)
+            duty
+        ];
+        self.command(Cmd::FanSet, &mut data)
     }
 
+    /// Read keymap data by layout, output pin, and input pin
     pub unsafe fn keymap_get(&mut self, layer: u8, output: u8, input: u8) -> Result<u16, Error> {
-        self.write(2, layer);
+        let mut data = [
-        self.write(3, output);
+            layer,
-        self.write(4, input);
+            output,
-        self.command(Cmd::KeymapGet)?;
+            input,
+            0,
+            0
+        ];
+        self.command(Cmd::KeymapGet, &mut data)?;
         Ok(
-            (self.read(5) as u16) |
+            (data[3] as u16) |
-            ((self.read(6) as u16) << 8)
+            ((data[4] as u16) << 8)
         )
     }
 
+    /// Set keymap data by layout, output pin, and input pin
     pub unsafe fn keymap_set(&mut self, layer: u8, output: u8, input: u8, value: u16) -> Result<(), Error> {
-        self.write(2, layer);
+        let mut data = [
-        self.write(3, output);
+            layer,
-        self.write(4, input);
+            output,
-        self.write(5, value as u8);
+            input,
-        self.write(6, (value >> 8) as u8);
+            value as u8,
-        self.command(Cmd::KeymapSet)
+            (value >> 8) as u8
+        ];
+        self.command(Cmd::KeymapSet, &mut data)
     }
 }
 
-pub struct EcSpi<'a, T: Timeout> {
+pub struct EcSpi<'a, A: Access> {
-    ec: &'a mut Ec<T>,
+    ec: &'a mut Ec<A>,
     target: SpiTarget,
     scratch: bool,
 }
 
-impl<'a, T: Timeout> EcSpi<'a, T> {
+impl<'a, A: Access> EcSpi<'a, A> {
     fn flags(&self, read: bool, disable: bool) -> u8 {
         let mut flags = 0;
 
@@ -252,7 +220,7 @@ impl<'a, T: Timeout> EcSpi<'a, T> {
     }
 }
 
-impl<'a, T: Timeout> Spi for EcSpi<'a, T> {
+impl<'a, A: Access> Spi for EcSpi<'a, A> {
     fn target(&self) -> SpiTarget {
         self.target
     }
@@ -260,10 +228,12 @@ impl<'a, T: Timeout> Spi for EcSpi<'a, T> {
     /// Disable SPI chip, must be done before and after a transaction
     unsafe fn reset(&mut self) -> Result<(), Error> {
         let flags = self.flags(false, true);
-        self.ec.write(2, flags);
+        let mut data = [
-        self.ec.write(3, 0);
+            flags,
-        self.ec.command(Cmd::Spi)?;
+            0,
-        if self.ec.read(3) != 0 {
+        ];
+        self.ec.command(Cmd::Spi, &mut data)?;
+        if data[1] != 0 {
             return Err(Error::Verify);
         }
         Ok(())
@@ -271,17 +241,18 @@ impl<'a, T: Timeout> Spi for EcSpi<'a, T> {
 
     /// SPI read
     unsafe fn read(&mut self, data: &mut [u8]) -> Result<usize, Error> {
+        //TODO: use self.access.data_size()
         let flags = self.flags(true, false);
         for chunk in data.chunks_mut(256 - 4) {
-            self.ec.write(2, flags);
+            let mut data = [0; 256 - 2];
-            self.ec.write(3, chunk.len() as u8);
+            data[0] = flags;
-            self.ec.command(Cmd::Spi)?;
+            data[1] = chunk.len() as u8;
-            if self.ec.read(3) != chunk.len() as u8 {
+            self.ec.command(Cmd::Spi, &mut data)?;
+            if data[1] != chunk.len() as u8 {
                 return Err(Error::Verify);
             }
-
             for i in 0..chunk.len() {
-                chunk[i] = self.ec.read(i as u8 + 4);
+                chunk[i] = data[i + 2];
             }
         }
         Ok(data.len())
@@ -289,16 +260,17 @@ impl<'a, T: Timeout> Spi for EcSpi<'a, T> {
 
     /// SPI write
     unsafe fn write(&mut self, data: &[u8]) -> Result<usize, Error> {
+        //TODO: use self.access.data_size()
         let flags = self.flags(false, false);
         for chunk in data.chunks(256 - 4) {
+            let mut data = [0; 256 - 2];
+            data[0] = flags;
+            data[1] = chunk.len() as u8;
             for i in 0..chunk.len() {
-                self.ec.write(i as u8 + 4, chunk[i]);
+                data[i + 2] = chunk[i];
             }
-
+            self.ec.command(Cmd::Spi, &mut data)?;
-            self.ec.write(2, flags);
+            if data[1] != chunk.len() as u8 {
-            self.ec.write(3, chunk.len() as u8);
-            self.ec.command(Cmd::Spi)?;
-            if self.ec.read(3) != chunk.len() as u8 {
                 return Err(Error::Verify);
             }
         }
@@ -306,7 +278,7 @@ impl<'a, T: Timeout> Spi for EcSpi<'a, T> {
     }
 }
 
-impl<'a, T: Timeout> Drop for EcSpi<'a, T> {
+impl<'a, A: Access> Drop for EcSpi<'a, A> {
     fn drop(&mut self) {
         unsafe {
             let _ = self.reset();

tool/src/error.rs

@@ -1,11 +1,25 @@
+/// Errors returned by operations
 #[derive(Debug)]
 pub enum Error {
+    /// Data length is too large
+    DataLength(usize),
+    /// A parameter was invalid
     Parameter,
+    /// EC protocol returned an error result
     Protocol(u8),
+    /// EC protocol signature did not match
     Signature((u8, u8)),
+    /// Super I/O ID did not match
     SuperIoId(u16),
+    /// Blocking operation timed out
     Timeout,
+    /// Unexpected data from EC
     Verify,
+    /// EC protocol version is unsupported
+    Version(u8),
+    /// Indicates that a blocking operation would block
     WouldBlock,
+    /// Encountered a std::io::Error
+    #[cfg(feature = "std")]
+    Io(std::io::Error)
 }
-

tool/src/firmware.rs

@@ -1,3 +1,4 @@
+/// Parses firmware information from a firmware ROM
 pub struct Firmware<'a> {
     pub board: &'a [u8],
     pub version: &'a [u8],
@@ -36,6 +37,7 @@ fn firmware_str<'a>(data: &'a [u8], key: &[u8]) -> Option<&'a [u8]> {
 }
 
 impl<'a> Firmware<'a> {
+    /// Parses firmware board and version, and then returns firmware object
     pub fn new(data: &'a [u8]) -> Option<Self> {
         let board = firmware_str(data, b"76EC_BOARD=")?;
         let version = firmware_str(data, b"76EC_VERSION=")?;

tool/src/legacy.rs

@@ -5,12 +5,13 @@ use crate::{
     Timeout,
 };
 
+/// Run some EC commands on previous proprietary firmware
 pub struct EcLegacy<T: Timeout> {
     pub pmc: Pmc<T>,
 }
 
 impl<T: Timeout> EcLegacy<T> {
-    /// Probes for a compatible EC
+    /// Probes for EC using direct hardware access. Unsafe due to no mutual exclusion
     pub unsafe fn new(primary: bool, timeout: T) -> Result<Self, Error> {
         let mut sio = SuperIo::new(if primary { 0x2E } else { 0x4E });
 
@@ -30,6 +31,7 @@ impl<T: Timeout> EcLegacy<T> {
         })
     }
 
+    /// Read the EC firmware project, which uniquely identifies the board
     pub unsafe fn project(&mut self, data: &mut [u8]) -> Result<usize, Error> {
         let mut i = 0;
         self.pmc.command(0x92)?;
@@ -43,6 +45,7 @@ impl<T: Timeout> EcLegacy<T> {
         Ok(i)
     }
 
+    /// Read the EC firmware version
     pub unsafe fn version(&mut self, data: &mut [u8]) -> Result<usize, Error> {
         // Prepend `1.` to version string
         let mut i = 0;

tool/src/lib.rs

@@ -1,5 +1,8 @@
 #![cfg_attr(not(feature = "std"), no_std)]
 
+pub use self::access::*;
+mod access;
+
 pub use self::ec::Ec;
 mod ec;
 

tool/src/main.rs

@@ -1,4 +1,6 @@
 use ectool::{
+    Access,
+    AccessLpcLinux,
     Ec,
     Error,
     Firmware,
@@ -10,41 +12,32 @@ use ectool::{
 use std::{
     env,
     fs,
-    io,
     process,
     str::{self, FromStr},
     time::Duration,
     thread,
 };
 
-unsafe fn iopl() {
+unsafe fn ec() -> Result<Ec<AccessLpcLinux>, Error> {
-    extern {
+    let access = AccessLpcLinux::new(Duration::new(1, 0))?;
-        fn iopl(level: isize) -> isize;
+    Ec::new(access)
-    }
-
-    if iopl(3) < 0 {
-        eprintln!("failed to get I/O permission: {}", io::Error::last_os_error());
-        process::exit(1);
-    }
 }
 
 unsafe fn console() -> Result<(), Error> {
-    iopl();
+    //TODO: driver support for reading debug region?
+    let mut ec = ec()?;
+    let access = ec.access();
 
-    let mut ec = Ec::new(
+    let mut head = access.read_debug(0)? as usize;
-        StdTimeout::new(Duration::new(1, 0)),
-    )?;
-
-    let mut head = ec.debug(0) as usize;
     loop {
-        let tail = ec.debug(0) as usize;
+        let tail = access.read_debug(0)? as usize;
         if tail == 0 || head == tail {
             thread::sleep(Duration::from_millis(1));
         } else {
             while head != tail {
                 head += 1;
                 if head >= 256 { head = 1; }
-                let c = ec.debug(head as u8);
+                let c = access.read_debug(head as u8)?;
                 print!("{}", c as char);
             }
         }
@@ -67,7 +60,7 @@ unsafe fn flash_read<S: Spi>(spi: &mut SpiRom<S, StdTimeout>, rom: &mut [u8], se
     Ok(())
 }
 
-unsafe fn flash_inner(ec: &mut Ec<StdTimeout>, firmware: &Firmware, target: SpiTarget, scratch: bool) -> Result<(), Error> {
+unsafe fn flash_inner(ec: &mut Ec<AccessLpcLinux>, firmware: &Firmware, target: SpiTarget, scratch: bool) -> Result<(), Error> {
     let rom_size = 128 * 1024;
 
     let mut new_rom = firmware.data.to_vec();
@@ -153,14 +146,11 @@ unsafe fn flash(path: &str, target: SpiTarget) -> Result<(), Error> {
     println!("file board: {:?}", str::from_utf8(firmware.board));
     println!("file version: {:?}", str::from_utf8(firmware.version));
 
-    iopl();
+    let mut ec = ec()?;
-
+    let data_size = ec.access().data_size();
-    let mut ec = Ec::new(
-        StdTimeout::new(Duration::new(1, 0)),
-    )?;
 
     {
-        let mut data = [0; 256];
+        let mut data = vec![0; data_size];
         let size = ec.board(&mut data)?;
 
         let ec_board = &data[..size];
@@ -172,7 +162,7 @@ unsafe fn flash(path: &str, target: SpiTarget) -> Result<(), Error> {
     }
 
     {
-        let mut data = [0; 256];
+        let mut data = vec![0; data_size];
         let size = ec.version(&mut data)?;
 
         let ec_version = &data[..size];
@@ -208,15 +198,12 @@ unsafe fn flash(path: &str, target: SpiTarget) -> Result<(), Error> {
 }
 
 unsafe fn info() -> Result<(), Error> {
-    iopl();
+    let mut ec = ec()?;
-
+    let data_size = ec.access().data_size();
-    let mut ec = Ec::new(
-        StdTimeout::new(Duration::new(1, 0)),
-    )?;
 
     {
         print!("board: ");
-        let mut data = [0; 256];
+        let mut data = vec![0; data_size];
         let size = ec.board(&mut data)?;
         for &b in data[..size].iter() {
             print!("{}", b as char);
@@ -226,7 +213,7 @@ unsafe fn info() -> Result<(), Error> {
 
     {
         print!("version: ");
-        let mut data = [0; 256];
+        let mut data = vec![0; data_size];
         let size = ec.version(&mut data)?;
         for &b in data[..size].iter() {
             print!("{}", b as char);
@@ -238,11 +225,7 @@ unsafe fn info() -> Result<(), Error> {
 }
 
 unsafe fn print(message: &[u8]) -> Result<(), Error> {
-    iopl();
+    let mut ec = ec()?;
-
-    let mut ec = Ec::new(
-        StdTimeout::new(Duration::new(1, 0)),
-    )?;
 
     ec.print(message)?;
 
@@ -250,11 +233,7 @@ unsafe fn print(message: &[u8]) -> Result<(), Error> {
 }
 
 unsafe fn fan_get(index: u8) -> Result<(), Error> {
-    iopl();
+    let mut ec = ec()?;
-
-    let mut ec = Ec::new(
-        StdTimeout::new(Duration::new(1, 0)),
-    )?;
 
     let duty = ec.fan_get(index)?;
     println!("{}", duty);
@@ -263,21 +242,13 @@ unsafe fn fan_get(index: u8) -> Result<(), Error> {
 }
 
 unsafe fn fan_set(index: u8, duty: u8) -> Result<(), Error> {
-    iopl();
+    let mut ec = ec()?;
-
-    let mut ec = Ec::new(
-        StdTimeout::new(Duration::new(1, 0)),
-    )?;
 
     ec.fan_set(index, duty)
 }
 
 unsafe fn keymap_get(layer: u8, output: u8, input: u8) -> Result<(), Error> {
-    iopl();
+    let mut ec = ec()?;
-
-    let mut ec = Ec::new(
-        StdTimeout::new(Duration::new(1, 0)),
-    )?;
 
     let value = ec.keymap_get(layer, output, input)?;
     println!("{:04X}", value);
@@ -286,11 +257,7 @@ unsafe fn keymap_get(layer: u8, output: u8, input: u8) -> Result<(), Error> {
 }
 
 unsafe fn keymap_set(layer: u8, output: u8, input: u8, value: u16) -> Result<(), Error> {
-    iopl();
+    let mut ec = ec()?;
-
-    let mut ec = Ec::new(
-        StdTimeout::new(Duration::new(1, 0)),
-    )?;
 
     ec.keymap_set(layer, output, input, value)
 }

tool/src/pmc.rs

@@ -6,6 +6,7 @@ use crate::{
     timeout
 };
 
+/// Standard ACPI EC interface
 pub struct Pmc<T: Timeout> {
     data: Pio<u8>,
     cmd: Pio<u8>,
@@ -13,9 +14,9 @@ pub struct Pmc<T: Timeout> {
 }
 
 impl<T: Timeout> Pmc<T> {
-    /// Create a new PMC instance. `base` identifies the data port. The command
+    /// Create a new ACPI EC instance using direct hardware access. `base` identifies the data
-    /// port will be `base + 4`
+    /// port. The command port will be `base + 4`. Unsafe due to no mutual exclusion
-    pub fn new(base: u16, timeout: T) -> Self {
+    pub unsafe fn new(base: u16, timeout: T) -> Self {
         Self {
             data: Pio::new(base),
             cmd: Pio::new(base + 4),
@@ -62,6 +63,7 @@ impl<T: Timeout> Pmc<T> {
         }
     }
 
+    /// Read from the ACPI region, at a specific address
     pub unsafe fn acpi_read(&mut self, addr: u8) -> Result<u8, Error> {
         self.timeout.reset();
 

tool/src/spi.rs

@@ -3,25 +3,38 @@ use crate::{
     Timeout,
 };
 
+/// SPI bus transactions
 pub trait Spi {
+    /// Return the target of the SPI bus
     fn target(&self) -> SpiTarget;
+
+    /// Reset the SPI bus
     unsafe fn reset(&mut self) -> Result<(), Error>;
+
+    /// Read data from the SPI bus
     unsafe fn read(&mut self, data: &mut [u8]) -> Result<usize, Error>;
+
+    /// Write data to the SPI bus
     unsafe fn write(&mut self, data: &[u8]) -> Result<usize, Error>;
 }
 
+/// Target which will receive SPI commands
 #[derive(Clone, Copy)]
 pub enum SpiTarget {
+    /// The ROM normally used by the EC
     Main,
+    /// The ROM used by the EC should the main ROM be invalid
     Backup,
 }
 
+/// SPI ROM transactions
 pub struct SpiRom<'a, S: Spi, T: Timeout> {
     spi: &'a mut S,
     timeout: T,
 }
 
 impl<'a, S: Spi, T: Timeout> SpiRom<'a, S, T> {
+    /// Create a SPI ROM using the specified SPI bus and timeout
     pub fn new(spi: &'a mut S, timeout: T) -> Self {
         Self {
             spi,
@@ -29,13 +42,16 @@ impl<'a, S: Spi, T: Timeout> SpiRom<'a, S, T> {
         }
     }
 
+    /// Get sector size in bytes
     pub fn sector_size(&self) -> usize {
+        //TODO: can this be determined automatically?
         match self.spi.target() {
             SpiTarget::Main => 1024,
             SpiTarget::Backup => 4096,
         }
     }
 
+    /// Read the status register
     pub unsafe fn status(&mut self) -> Result<u8, Error> {
         let mut status = [0];
 
@@ -46,6 +62,7 @@ impl<'a, S: Spi, T: Timeout> SpiRom<'a, S, T> {
         Ok(status[0])
     }
 
+    /// Wait for the status register to have `mask` bits set to `value`
     pub unsafe fn status_wait(&mut self, mask: u8, value: u8) -> Result<(), Error> {
         self.timeout.reset();
         while self.timeout.running() {
@@ -56,6 +73,7 @@ impl<'a, S: Spi, T: Timeout> SpiRom<'a, S, T> {
         Err(Error::Timeout)
     }
 
+    /// Disable writes
     pub unsafe fn write_disable(&mut self) -> Result<(), Error> {
         self.spi.reset()?;
         self.spi.write(&[0x04])?;
@@ -66,6 +84,7 @@ impl<'a, S: Spi, T: Timeout> SpiRom<'a, S, T> {
         Ok(())
     }
 
+    /// Enable writes
     pub unsafe fn write_enable(&mut self) -> Result<(), Error> {
         self.spi.reset()?;
         self.spi.write(&[0x06])?;
@@ -76,6 +95,7 @@ impl<'a, S: Spi, T: Timeout> SpiRom<'a, S, T> {
         Ok(())
     }
 
+    /// Erase a sector with the specified address
     pub unsafe fn erase_sector(&mut self, address: u32) -> Result<(), Error> {
         if (address & 0xFF00_0000) > 0 {
             return Err(Error::Parameter);
@@ -104,6 +124,7 @@ impl<'a, S: Spi, T: Timeout> SpiRom<'a, S, T> {
         Ok(())
     }
 
+    /// Read at a specific address
     pub unsafe fn read_at(&mut self, address: u32, data: &mut [u8]) -> Result<usize, Error> {
         if (address & 0xFF00_0000) > 0 {
             return Err(Error::Parameter);
@@ -120,6 +141,7 @@ impl<'a, S: Spi, T: Timeout> SpiRom<'a, S, T> {
         self.spi.read(data)
     }
 
+    /// Write at a specific address
     pub unsafe fn write_at(&mut self, address: u32, data: &[u8]) -> Result<usize, Error> {
         if (address & 0xFF00_0000) > 0 {
             return Err(Error::Parameter);
@@ -127,6 +149,7 @@ impl<'a, S: Spi, T: Timeout> SpiRom<'a, S, T> {
 
         self.write_enable()?;
 
+        //TODO: automatically detect write command
         match self.spi.target() {
             SpiTarget::Main => for (i, word) in data.chunks(2).enumerate() {
                 let low = *word.get(0).unwrap_or(&0xFF);

tool/src/super_io.rs

@@ -1,25 +1,28 @@
 use hwio::{Io, Pio};
 
+/// Super I/O interface provided by LPC ECs
 pub struct SuperIo {
     addr: Pio<u8>,
     data: Pio<u8>,
 }
 
 impl SuperIo {
-    /// Create a new SuperIo. `base` identifies the address port. The data port
+    /// Create a new SuperIo using direct hardware access. `base` identifies the address port. The
-    /// will be `base + 1`
+    /// data port will be `base + 1`. Unsafe due to no mutual exclusion
-    pub fn new(base: u16) -> Self {
+    pub unsafe fn new(base: u16) -> Self {
         Self {
             addr: Pio::new(base),
             data: Pio::new(base + 1),
         }
     }
 
+    /// Read a Super I/O register
     pub unsafe fn read(&mut self, addr: u8) -> u8 {
         self.addr.write(addr);
         self.data.read()
     }
 
+    /// Write a Super I/O register
     pub unsafe fn write(&mut self, addr: u8, data: u8) {
         self.addr.write(addr);
         self.data.write(data);

tool/src/timeout.rs

@@ -24,11 +24,16 @@ macro_rules! timeout {
     }};
 }
 
+/// Timeout for use in blocking operations
 pub trait Timeout {
+    /// Reset the timeout to its initial state
     fn reset(&mut self);
+
+    /// Check if timeout is still running
     fn running(&self) -> bool;
 }
 
+/// Timeout implemented using std::time
 #[cfg(feature = "std")]
 pub struct StdTimeout {
     instant: Instant,
@@ -37,6 +42,7 @@ pub struct StdTimeout {
 
 #[cfg(feature = "std")]
 impl StdTimeout {
+    /// Create a timeout with the specified duration
     pub fn new(duration: Duration) -> Self {
         StdTimeout {
             instant: Instant::now(),