Fix write and read op bugs found in Deck test

ecplorer/Cargo.toml

@@ -6,7 +6,9 @@ edition = "2021"
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
 
 [dependencies]
-smokepatio = { version = "0.1", path = "../" }
+smokepatio = { version = "0.1", path = "../", features = [ "std" ] }
 
 clap = { version = "4.4", features = [ "derive" ] }
 clap-num = "1.0"
+
+libc = "0.2"

ecplorer/src/main.rs

@@ -7,6 +7,13 @@ fn main() {
     println!("args: {:?}", cli);
 
     let mut ec = EmbeddedController::new(cli.data_address, cli.cmd_address);
+
+    #[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
+    unsafe {
+        libc::ioperm(cli.data_address as _, 1, 1);
+        libc::ioperm(cli.cmd_address as _, 1, 1);
+    }
+
     match cli.op {
         args::Operation::ReadAll => todo!(),
         args::Operation::Read { address } => {

src/ec/controller.rs

@@ -1,28 +1,45 @@
-pub trait SetValue<S> {
+pub trait SetValue<S, RV = u8> {
     const SETTING: S;
 
-    fn raw_value(&self) -> u8;
+    fn raw_value(&self) -> RV;
 }
 
-pub trait ControllerSet<S>: embedded_io::ErrorType {
+pub trait ControllerSet<S, RV = u8>: embedded_io::ErrorType {
-    fn set<V: SetValue<S>>(&mut self, value: V) -> Result<(), Self::Error>;
+    fn set<V: SetValue<S, RV>>(&mut self, value: V) -> Result<(), Self::Error>;
 }
 
-pub trait GetValue<S> {
+pub trait GetValue<S, RV = u8> {
     const SETTING: S;
 
-    fn raw_value(value: u8) -> Self;
+    fn raw_value(value: RV) -> Self;
 }
 
-pub trait ControllerGet<S>: embedded_io::ErrorType {
+pub trait ControllerGet<S, RV = u8>: embedded_io::ErrorType {
-    fn get<V: GetValue<S>>(&mut self) -> Result<V, Self::Error>;
+    fn get<V: GetValue<S, RV>>(&mut self) -> Result<V, Self::Error>;
 }
 
+/// Size of values to write and read from ports.
+/// System endianness is assumed.
+#[derive(Copy, Clone)]
+#[repr(u8)]
+pub enum ValueSize {
+    Byte = 1, // u8
+    Word = 2, // u16
+    Long = 4, // u32
+}
+
+#[cfg(feature = "std")]
+const WAIT_SLEEP: std::time::Duration = std::time::Duration::from_micros(10);
+#[cfg(feature = "std")]
+const MAX_WAIT_CHECKS: u16 = 0x07ff;
+#[cfg(not(feature = "std"))]
+const MAX_WAIT_CHECKS: u16 = 0x7fff;
+
 pub struct EmbeddedController {
     data_address: u16,
     cmd_address: u16,
-    cursor: u16,
+    cmd_requested: u16,
-    is_cmd_requested: bool,
+    rw_size: ValueSize,
 }
 
 impl EmbeddedController {
@@ -30,55 +47,84 @@ impl EmbeddedController {
         Self {
             data_address: data_addr,
             cmd_address: cmd_addr,
-            cursor: 0,
+            cmd_requested: 0,
-            is_cmd_requested: false,
+            rw_size: ValueSize::Byte,
         }
     }
 
     fn request_if_not_already(&mut self) {
-        if !self.is_cmd_requested {
+        if self.cmd_requested == 0 {
-            super::ports::outb(self.data_address, 0x81);
+            super::ports::outb(self.cmd_address, 0x81);
-            self.is_cmd_requested = true;
+            self.cmd_requested = 1;
+        } else {
+            self.cmd_requested += 1;
+        }
+    }
+
+    fn wait_for_write_ready(&mut self) {
+        use embedded_io::WriteReady;
+        while !self.write_ready().unwrap() {
+            #[cfg(feature = "std")]
+            std::thread::sleep(WAIT_SLEEP);
+        }
+    }
+
+    fn wait_for_read_ready(&mut self) {
+        use embedded_io::ReadReady;
+        while !self.read_ready().unwrap() {
+            #[cfg(feature = "std")]
+            std::thread::sleep(WAIT_SLEEP);
         }
     }
 
     pub fn set(&mut self, setting: u16, value: u8) {
-        use embedded_io::{Write, Seek};
+        use embedded_io::Write;
-        self.seek(embedded_io::SeekFrom::Start(setting as _)).unwrap();
+        self.write(&[setting as u8]).unwrap();
+        self.cmd_requested = 1; // don't re-request, but still wait
         self.write(&[value]).unwrap();
     }
 
     pub fn get(&mut self, setting: u16) -> u8 {
-        use embedded_io::{Read, Seek};
+        use embedded_io::{Read, Write};
-        self.seek(embedded_io::SeekFrom::Start(setting as _)).unwrap();
+        self.write(&[setting as u8]).unwrap();
+        self.cmd_requested = 1; // don't re-request, but still wait
         let mut buf = [0u8; 1];
         self.read(&mut buf).unwrap();
         buf[0]
     }
+
+    pub fn set_value_size(&mut self, size: ValueSize) {
+        self.rw_size = size;
+    }
 }
 
 impl embedded_io::ErrorType for EmbeddedController {
     type Error = core::convert::Infallible;
 }
 
-impl embedded_io::Seek for EmbeddedController {
-    fn seek(&mut self, pos: embedded_io::SeekFrom) -> Result<u64, Self::Error> {
-        self.cursor = match pos {
-            embedded_io::SeekFrom::Start(x) => x as u16,
-            embedded_io::SeekFrom::End(x) => (u16::MAX as i64 + x) as u16,
-            embedded_io::SeekFrom::Current(x) => (self.cursor as i64 + x) as u16
-        };
-        Ok(self.cursor as _)
-    }
-}
-
 impl embedded_io::Write for EmbeddedController {
     fn write(&mut self, buf: &[u8]) -> Result<usize, Self::Error> {
-        use embedded_io::WriteReady;
+        let size = self.rw_size as u8;
-        for &b in buf {
+        match self.rw_size {
-            while !self.write_ready()? {  }
+            ValueSize::Byte => for &b in buf {
-            super::ports::outb(self.cursor, b);
+                self.wait_for_write_ready();
+                self.cmd_requested = 1; // don't re-request, but still wait
+                super::ports::outb(self.data_address, b);
+            },
+            ValueSize::Word => for i in (0..buf.len()).step_by(size as usize) {
+                self.wait_for_write_ready();
+                self.cmd_requested = 1; // don't re-request, but still wait
+                let w = [buf[i], buf[i+1]];
+                super::ports::outw(self.data_address, u16::from_ne_bytes(w));
+            },
+            ValueSize::Long => for i in (0..buf.len()).step_by(size as usize) {
+                self.wait_for_write_ready();
+                self.cmd_requested = 1; // don't re-request, but still wait
+                let l = [buf[i], buf[i+1], buf[i+2], buf[i+3]];
+                super::ports::outl(self.data_address, u32::from_ne_bytes(l));
+            }
         }
+
         Ok(buf.len())
     }
 
@@ -90,9 +136,9 @@ impl embedded_io::Write for EmbeddedController {
 impl embedded_io::WriteReady for EmbeddedController {
     fn write_ready(&mut self) -> Result<bool, Self::Error> {
         self.request_if_not_already();
-        let ready = super::ports::inb(self.cmd_address) & 2 == 0;
+        let ready = super::ports::inb(self.cmd_address) & 2 == 0 || self.cmd_requested == MAX_WAIT_CHECKS;
         if ready {
-            self.is_cmd_requested = false;
+            self.cmd_requested = 0;
         }
         Ok(ready)
     }
@@ -100,10 +146,29 @@ impl embedded_io::WriteReady for EmbeddedController {
 
 impl embedded_io::Read for EmbeddedController {
     fn read(&mut self, buf: &mut [u8]) -> Result<usize, Self::Error> {
-        use embedded_io::ReadReady;
+        let size = self.rw_size as u8;
-        for b in buf.iter_mut() {
+        match self.rw_size {
-            while !self.read_ready()? {  }
+            ValueSize::Byte => for b in buf.iter_mut() {
-            *b = super::ports::inb(self.cursor)
+                self.wait_for_read_ready();
+                self.cmd_requested = 1; // don't re-request, but still wait
+                *b = super::ports::inb(self.data_address);
+            },
+            ValueSize::Word => for i in (0..buf.len()).step_by(size as usize) {
+                self.wait_for_read_ready();
+                self.cmd_requested = 1; // don't re-request, but still wait
+                let w = super::ports::inw(self.data_address).to_ne_bytes();
+                for j in 0..(size as usize) {
+                    buf[i + j] = w[j];
+                }
+            },
+            ValueSize::Long => for i in (0..buf.len()).step_by(size as usize) {
+                self.wait_for_read_ready();
+                self.cmd_requested = 1; // don't re-request, but still wait
+                let l = super::ports::inl(self.data_address).to_ne_bytes();
+                for j in 0..(size as usize) {
+                    buf[i + j] = l[j];
+                }
+            }
         }
         Ok(buf.len())
     }
@@ -112,9 +177,9 @@ impl embedded_io::Read for EmbeddedController {
 impl embedded_io::ReadReady for EmbeddedController {
     fn read_ready(&mut self) -> Result<bool, Self::Error> {
         self.request_if_not_already();
-        let ready = super::ports::inb(self.cmd_address) & 1 != 0;
+        let ready = super::ports::inb(self.cmd_address) & 1 != 0 || self.cmd_requested == MAX_WAIT_CHECKS;
         if ready {
-            self.is_cmd_requested = false;
+            self.cmd_requested = 0;
         }
         Ok(ready)
     }

src/ec/ports.rs

@@ -1,30 +1,111 @@
 // x86 and x86_64
-#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
+#[cfg(all(any(target_arch = "x86", target_arch = "x86_64")))]
-pub(crate) fn outb(port: u16, val: u8) {
+mod ports_internal {
-    //let val: i16 = val.into();
+    #[inline]
-    unsafe {
+    pub(crate) fn outb(port: u16, val: u8) {
-        std::arch::asm!("out dx, al", in("dx") port, in("al") val);
+        //println!("outb port:{:#04x} value:{:#02x}", port, val);
-        //std::arch::asm!("outb %al, %dx", in("al") val, in("dx") port, options(att_syntax));
+        unsafe {
+            std::arch::asm!("out dx, al", in("dx") port, in("al") val);
+            //std::arch::asm!("outb %al, %dx", in("al") val, in("dx") port, options(att_syntax));
+            //std::arch::asm!("out dx,al", in("al") val, in("dx") port);
+        }
+        //println!("outb done");
     }
-}
 
-#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
+    #[inline]
-pub(crate) fn inb(port: u16) -> u8 {
+    pub(crate) fn outw(port: u16, val: u16) {
-    let out: u8;
+        //println!("outw port:{:#04x} value:{:#02x}", port, val);
-    unsafe {
+        unsafe {
-        std::arch::asm!("in al, dx", in("dx") port, lateout("al") out);
+            std::arch::asm!("out dx, ax", in("dx") port, in("ax") val);
-        // asm!("inb %dx, %al", in("dx") port, out("al") ret, options(att_syntax));
+            //std::arch::asm!("outb %al, %dx", in("al") val, in("dx") port, options(att_syntax));
+            //std::arch::asm!("out dx,al", in("al") val, in("dx") port);
+        }
+        //println!("outw done");
+    }
+
+    #[inline]
+    pub(crate) fn outl(port: u16, val: u32) {
+        //println!("outl port:{:#04x} value:{:#02x}", port, val);
+        unsafe {
+            std::arch::asm!("out dx, eax", in("dx") port, in("eax") val);
+            //std::arch::asm!("outb %al, %dx", in("al") val, in("dx") port, options(att_syntax));
+            //std::arch::asm!("out dx,al", in("al") val, in("dx") port);
+        }
+        //println!("outl done");
+    }
+
+    #[inline]
+    pub(crate) fn inb(port: u16) -> u8 {
+        let out: u8;
+        //println!("inb port:{:#04x}", port);
+        unsafe {
+            std::arch::asm!("in al, dx", in("dx") port, lateout("al") out);
+            //std::arch::asm!("inb %dx, %al", in("dx") port, out("al") out, options(att_syntax));
+            //std::arch::asm!("in al,dx", in("dx") port, out("al") out);
+        }
+        //println!("inb port:{:#04x} -> value:{:#02x}", port, out);
+        out
+    }
+
+    #[inline]
+    pub(crate) fn inw(port: u16) -> u16 {
+        let out: u16;
+        //println!("inb port:{:#04x}", port);
+        unsafe {
+            std::arch::asm!("in ax, dx", in("dx") port, lateout("ax") out);
+            //std::arch::asm!("inb %dx, %al", in("dx") port, out("al") out, options(att_syntax));
+            //std::arch::asm!("in al,dx", in("dx") port, out("al") out);
+        }
+        //println!("inb port:{:#04x} -> value:{:#02x}", port, out);
+        out
+    }
+
+    #[inline]
+    pub(crate) fn inl(port: u16) -> u32 {
+        let out: u32;
+        //println!("inb port:{:#04x}", port);
+        unsafe {
+            std::arch::asm!("in eax, dx", in("dx") port, lateout("eax") out);
+            //std::arch::asm!("inb %dx, %al", in("dx") port, out("al") out, options(att_syntax));
+            //std::arch::asm!("in al,dx", in("dx") port, out("al") out);
+        }
+        //println!("inb port:{:#04x} -> value:{:#02x}", port, out);
+        out
     }
-    out
 }
 
 // Fallback (compiler error)
-#[cfg(not(any(any(target_arch = "x86", target_arch = "x86_64"), )))]
+#[cfg(not(any(all(any(target_arch = "x86", target_arch = "x86_64")), target_os = "linux")))]
-pub(crate) fn outb(port: u16, val: u8) {
+mod ports_internal {
-    compile_error!("outb not supported on this platform")
+    #[inline]
+    pub(crate) fn outb(port: u16, val: u8) {
+        compile_error!("outb not supported on this platform")
+    }
+
+    #[inline]
+    pub(crate) fn outw(port: u16, val: u16) {
+        compile_error!("outw not supported on this platform")
+    }
+
+    #[inline]
+    pub(crate) fn outl(port: u16, val: u32) {
+        compile_error!("outl not supported on this platform")
+    }
+
+    #[inline]
+    pub(crate) fn inb(port: u16) -> u8 {
+        compile_error!("inb not supported on this platform")
+    }
+
+    #[inline]
+    pub(crate) fn inw(port: u16) -> u16 {
+        compile_error!("inw not supported on this platform")
+    }
+
+    #[inline]
+    pub(crate) fn inl(port: u16) -> u32 {
+        compile_error!("inl not supported on this platform")
+    }
 }
 
-#[cfg(not(any(any(target_arch = "x86", target_arch = "x86_64"), )))]
+pub(crate) use ports_internal::*;
-pub(crate) fn inb(port: u16) -> u8 {
-    compile_error!("inb not supported on this platform")
-}