Repository: theseus-os/Theseus (language: rust)

Issue:
Init both keyboard and mouse in one ps2 controller driver instead
I think it's completely fine to initialize them together as part of a larger `ps2::init()` function or something like that, in which we follow your advice above and attempt to init keyboard, then mouse. That should be possible to directly invoke from the device manager or captain, wherever appropriate.

_Originally posted by @kevinaboos in https://github.com/theseus-os/Theseus/pull/667#discussion_r1013392822_
      


Discussion / hints:

Fix patch:
diff --git a/Cargo.lock b/Cargo.lock
index 0a75750bd3..79afd9b59c 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -754,6 +754,7 @@ dependencies = [
  "mpmc",
  "network_manager",
  "pci",
+ "ps2_controller",
  "serial_port",
  "spin 0.9.0",
  "storage_manager",
@@ -2438,6 +2439,16 @@ dependencies = [
  "spin 0.9.0",
 ]
 
+[[package]]
+name = "ps2_controller"
+version = "0.1.0"
+dependencies = [
+ "acpi",
+ "fadt",
+ "log",
+ "ps2",
+]
+
 [[package]]
 name = "psm"
 version = "0.1.18"
diff --git a/kernel/device_manager/Cargo.toml b/kernel/device_manager/Cargo.toml
index 9d77956623..2a57cae3f6 100644
--- a/kernel/device_manager/Cargo.toml
+++ b/kernel/device_manager/Cargo.toml
@@ -45,6 +45,9 @@ path = "../acpi"
 [dependencies.pci]
 path = "../pci"
 
+[dependencies.ps2_controller]
+path = "../ps2_controller"
+
 [dependencies.keyboard]
 path = "../keyboard"
 
diff --git a/kernel/device_manager/src/lib.rs b/kernel/device_manager/src/lib.rs
index 676a77433d..83a589ab7a 100644
--- a/kernel/device_manager/src/lib.rs
+++ b/kernel/device_manager/src/lib.rs
@@ -96,6 +96,7 @@ pub fn init(key_producer: Queue<Event>, mouse_producer: Queue<Event>) -> Result<
     init_serial_port(SerialPortAddress::COM1);
     init_serial_port(SerialPortAddress::COM2);
 
+    ps2_controller::init()?;
     keyboard::init(key_producer)?;
     mouse::init(mouse_producer)?;
 
diff --git a/kernel/keyboard/src/lib.rs b/kernel/keyboard/src/lib.rs
index 1bbdd096f4..06badc2609 100644
--- a/kernel/keyboard/src/lib.rs
+++ b/kernel/keyboard/src/lib.rs
@@ -10,7 +10,7 @@ use once_cell::unsync::Lazy;
 use spin::Once;
 use mpmc::Queue;
 use event_types::Event;
-use ps2::{init_ps2_port1, test_ps2_port1, keyboard_detect, KeyboardType, read_scancode, LEDState};
+use ps2::{keyboard_detect, KeyboardType, read_scancode, LEDState, keyboard_scancode_set, ScancodeSet};
 use x86_64::structures::idt::InterruptStackFrame;
 
 /// The first PS/2 port for the keyboard is connected directly to IRQ 1.
@@ -28,11 +28,6 @@ static KEYBOARD_PRODUCER: Once<Queue<Event>> = Once::new();
 /// * `keyboard_queue_producer`: the queue onto which the keyboard interrupt handler
 ///    will push new keyboard events when a key action occurs.
 pub fn init(keyboard_queue_producer: Queue<Event>) -> Result<(), &'static str> {
-    // Init the first PS/2 port, which is used for the keyboard.
-    init_ps2_port1();
-    // Test the first port.
-    test_ps2_port1()?;
-
     // Detect which kind of keyboard is connected.
     // TODO: actually do something with the keyboard type.
     match keyboard_detect() {
@@ -45,8 +40,8 @@ pub fn init(keyboard_queue_producer: Queue<Event>) -> Result<(), &'static str> {
         }
     }
 
-    // TODO: figure out what we should do
-    // keyboard_scancode_set(ScancodeSet::Set2)?;
+    // TODO: figure out what we should do, for now using set 1
+    keyboard_scancode_set(ScancodeSet::Set1)?;
 
     // Register the interrupt handler
     interrupts::register_interrupt(PS2_KEYBOARD_IRQ, ps2_keyboard_handler).map_err(|e| {
diff --git a/kernel/mouse/src/lib.rs b/kernel/mouse/src/lib.rs
index 363523b3b5..39baab88e2 100644
--- a/kernel/mouse/src/lib.rs
+++ b/kernel/mouse/src/lib.rs
@@ -9,7 +9,7 @@ use mpmc::Queue;
 use event_types::Event;
 use x86_64::structures::idt::InterruptStackFrame;
 use mouse_data::{MouseButtons, MouseEvent, MouseMovementRelative};
-use ps2::{mouse_id, init_ps2_port2, set_mouse_id, test_ps2_port2, read_mouse_packet, MouseId, MousePacket, status_register};
+use ps2::{mouse_id, set_mouse_id, read_mouse_packet, MouseId, MousePacket, status_register};
 
 /// The first PS/2 port for the mouse is connected directly to IRQ 0xC.
 /// Because we perform the typical PIC remapping, the remapped IRQ vector number is 0x2C.
@@ -24,11 +24,6 @@ static MOUSE_ID: Once<MouseId> = Once::new();
 /// * `mouse_queue_producer`: the queue onto which the mouse interrupt handler
 ///    will push new mouse events when a mouse action occurs.
 pub fn init(mouse_queue_producer: Queue<Event>) -> Result<(), &'static str> {
-    // Init the second PS/2 port, which is used for the mouse.
-    init_ps2_port2();
-    // Test the second port.
-    test_ps2_port2()?;
-
     //TODO: set to 3, failed? id = 0, otherwise set to 4, failed? id = 3, otherwise id = 4
     //the current code beneath just tries to set id = 4, so is not final
     // Set Mouse ID to 4.
diff --git a/kernel/ps2/src/lib.rs b/kernel/ps2/src/lib.rs
index b6d128b4fc..7c0af0ec94 100644
--- a/kernel/ps2/src/lib.rs
+++ b/kernel/ps2/src/lib.rs
@@ -1,6 +1,5 @@
 #![no_std]
 
-use log::debug;
 use num_enum::TryFromPrimitive;
 use port_io::Port;
 use spin::Mutex;
@@ -22,7 +21,7 @@ static PS2_COMMAND_AND_STATUS_PORT: Mutex<Port<u8>> = Mutex::new(Port::new(0x64)
 /// 
 /// Naming perspective: Output means "out of the device into the host".
 /// e.g. [WriteByteToPort2InputBuffer] means "write byte out of the host into port 2"
-enum HostToControllerCommand {
+pub enum HostToControllerCommand {
     /// returns [ControllerConfigurationByte]
     ReadFromInternalRAMByte0 = 0x20,
     
@@ -110,7 +109,7 @@ enum HostToControllerCommand {
 /// 
 /// Note: Devices attached to the controller should be disabled
 /// before sending commands that return data, otherwise the output buffer could get overwritten
-fn write_command(value: HostToControllerCommand) {
+pub fn write_command(value: HostToControllerCommand) {
     unsafe {
         PS2_COMMAND_AND_STATUS_PORT.lock().write(value as u8);
     }
@@ -207,14 +206,14 @@ fn write_data(value: WritableData) {
 // wiki.osdev.org/%228042%22_PS/2_Controller#PS.2F2_Controller_Configuration_Byte
 /// Used for [HostToControllerCommand::ReadFromInternalRAMByte0] and [HostToControllerCommand::WriteToInternalRAMByte0]
 #[bitfield(bits = 8)]
-#[derive(Debug)]
+#[derive(Debug, Clone, Copy)]
 pub struct ControllerConfigurationByte {
     /// interrupt on [ControllerToHostStatus] `output_buffer_full`
-    port1_interrupt_enabled: bool,
+    pub port1_interrupt_enabled: bool,
     /// interrupt on [ControllerToHostStatus] `mouse_output_buffer_full`
     /// 
     /// Note: only if 2 PS/2 ports supported
-    port2_interrupt_enabled: bool,
+    pub port2_interrupt_enabled: bool,
     /// Cleared on reset; set when the system passed Power-on self-test
     #[allow(dead_code)]
     system_passed_self_test: bool,
@@ -222,153 +221,37 @@ pub struct ControllerConfigurationByte {
     #[allow(dead_code)]
     should_be_zero: B1,
     /// disables the keyboard
-    port1_clock_disabled: bool,
+    pub port1_clock_disabled: bool,
     /// disables the auxilary device (mouse)
     /// 
     /// Note: only if 2 PS/2 ports supported
-    port2_clock_disabled: bool,
+    pub port2_clock_disabled: bool,
     /// whether IBM scancode translation is enabled (0=AT, 1=PC)
-    #[allow(dead_code)]
-    port1_translation_enabled: bool,
+    pub port1_translation_enabled: bool,
     #[allow(dead_code)]
     must_be_zero: B1,
 }
 
 /// read the config of the PS/2 port
-fn read_config() -> ControllerConfigurationByte {
+pub fn read_config() -> ControllerConfigurationByte {
     write_command(ReadFromInternalRAMByte0);
     ControllerConfigurationByte::from_bytes([read_data()])
 }
 
 /// write the new config to the PS/2 command port (0x64)
-fn write_config(value: ControllerConfigurationByte) {
+pub fn write_config(value: ControllerConfigurationByte) {
     write_command(WriteToInternalRAMByte0);
     write_data(WritableData::Configuration(value));
 }
 
-/// initialize the first PS/2 data port
-pub fn init_ps2_port1() {
-    init_ps2_port(PS2Port::One);
-}
-
-/// initialize the second PS/2 data port
-pub fn init_ps2_port2() {
-    init_ps2_port(PS2Port::Two);
-}
-
-#[derive(Clone, Copy)]
-enum PS2Port {
-    One,
-    Two,
-}
-
-// see https://wiki.osdev.org/%228042%22_PS/2_Controller#Initialising_the_PS.2F2_Controller
-fn init_ps2_port(port: PS2Port) {
-    // Step 1: Initialise USB Controllers
-    // no USB support yet
-
-    // Step 2: Determine if the PS/2 Controller Exists
-    // TODO: we support the FADT, so we can check this
-
-    // Step 3: Disable Devices
-    write_command(DisablePort2);
-    write_command(DisablePort1);
-
-    // Step 4: Flush The Output Buffer
-    flush_output_buffer();
-
-    // Step 5: Set the Controller Configuration Byte
-    let mut config = read_config();
-    match port {
-        PS2Port::One => {
-            config.set_port1_clock_disabled(false);
-            config.set_port1_interrupt_enabled(true);
-        }
-        PS2Port::Two => {
-            config.set_port2_clock_disabled(false);
-            config.set_port2_interrupt_enabled(true);
-        }
-    }
-    write_config(config);
-
-    // Step 6/7/8 are inside [test_ps2_port]
-
-    // Step 9: Enable Devices
-    write_command(EnablePort1);
-    // NOTE: we might not need to do this conditionally
-    match port {
-        PS2Port::Two => write_command(EnablePort2),
-        PS2Port::One => (),
-    };
-}
-
 /// Clean the [PS2_DATA_PORT] output buffer, skipping the [ControllerToHostStatus] `output_buffer_full` check
-fn flush_output_buffer() {
+pub fn flush_output_buffer() {
     read_data();
 }
 
-/// test the first PS/2 data port
-pub fn test_ps2_port1() -> Result<(), &'static str> {
-    test_ps2_port(PS2Port::One)
-}
-
-// test the second PS/2 data port
-pub fn test_ps2_port2() -> Result<(), &'static str> {
-    test_ps2_port(PS2Port::Two)
-}
-
-// see https://wiki.osdev.org/%228042%22_PS/2_Controller#Initialising_the_PS.2F2_Controller
-fn test_ps2_port(port: PS2Port) -> Result<(), &'static str> {
-    // Step 1-5 are inside [init_ps2_port]
-
-    // Step 6: Perform Controller Self Test
-    write_command(TestController);
-    read_controller_test_result()?;
-    debug!("passed PS/2 controller test");
-
-    // TODO: Step 7, but not here, maybe init and test both devices at once in a new PS/2 controller driver crate
-
-    // Step 8: Perform Interface Tests
-    match port {
-        PS2Port::One => write_command(TestPort1),
-        PS2Port::Two => write_command(TestPort2),
-    }
-    use PortTestResult::*;
-    match read_port_test_result()? {
-        Passed => debug!("passed PS/2 port {} test", port as u8 + 1),
-        ClockLineStuckLow => Err("failed PS/2 port test, clock line stuck low")?,
-        ClockLineStuckHigh => Err("failed PS/2 port test, clock line stuck high")?,
-        DataLineStuckLow => Err("failed PS/2 port test, data line stuck low")?,
-        DataLineStuckHigh => Err("failed PS/2 port test, clock line stuck high")?,
-    }
-
-    // print the config (?) see TODO above
-    let config = read_config();
-    let port_interrupt_enabled = match port {
-        PS2Port::One => config.port1_interrupt_enabled(),
-        PS2Port::Two => config.port2_interrupt_enabled(),
-    };
-    let clock_enabled = match port {
-        PS2Port::One => !config.port1_clock_disabled(),
-        PS2Port::Two => !config.port2_clock_disabled(),
-    };
-
-    if port_interrupt_enabled {
-        debug!("PS/2 port {} interrupt enabled", port as u8 + 1)
-    } else {
-        Err("PS/2 port test config's interrupt disabled")?
-    }
-    if clock_enabled {
-        debug!("PS/2 port {} clock enabled", port as u8 + 1)
-    } else {
-        Err("PS/2 port test config's clock disabled")?
-    }
-    Ok(())
-}
-
 /// must only be called after writing the [TestController] command
 /// otherwise would read bogus data
-fn read_controller_test_result() -> Result<(), &'static str> {
+pub fn read_controller_test_result() -> Result<(), &'static str> {
     const CONTROLLER_TEST_PASSED: u8 = 0x55;
     if read_data() != CONTROLLER_TEST_PASSED {
         Err("failed PS/2 controller test")
@@ -379,7 +262,7 @@ fn read_controller_test_result() -> Result<(), &'static str> {
 
 #[derive(TryFromPrimitive)]
 #[repr(u8)]
-enum PortTestResult {
+pub enum PortTestResult {
     Passed = 0x00,
     ClockLineStuckLow = 0x01,
     ClockLineStuckHigh = 0x02,
@@ -389,7 +272,7 @@ enum PortTestResult {
 
 /// must only be called after writing the [TestPort1] or [TestPort2] command
 /// otherwise would read bogus data
-fn read_port_test_result() -> Result<PortTestResult, &'static str> {
+pub fn read_port_test_result() -> Result<PortTestResult, &'static str> {
     read_data().try_into().map_err(|_| "failed to read port test result")
 }
 
@@ -498,7 +381,7 @@ pub struct LEDState {
 
 #[derive(Clone)]
 pub enum ScancodeSet {
-    GetCurrentSet = 0,
+    // GetCurrentSet = 0, //TODO, if needed
     Set1 = 1,
     Set2 = 2,
     Set3 = 3,
@@ -577,10 +460,10 @@ pub struct MousePacketGeneric {
     pub y_overflow: bool,
     //2. byte
     /// only a part of x_movement, needs to be combined with [x_9th_bit]
-    x_1st_to_8th_bit: B8, //u8 with #[bits = 8] attribute didn't work
+    x_1st_to_8th_bit: B8,
     //3. byte
     /// only a part of y_movement, needs to be combined with [y_9th_bit]
-    y_1st_to_8th_bit: B8, //u8 with #[bits = 8] attribute didn't work
+    y_1st_to_8th_bit: B8,
 }
 
 impl MousePacketGeneric {
@@ -821,19 +704,23 @@ pub fn mouse_id() -> Result<MouseId, &'static str> {
 }
 
 /// reset the mouse
-#[allow(dead_code)]
-fn reset_mouse() -> Result<(), &'static str> {
+pub fn reset_mouse() -> Result<(), &'static str> {
     command_to_mouse(HostToMouseCommandOrData::MouseCommand(Reset))?; 
-    for _ in 0..14 {
-        //BAT = Basic Assurance Test
-        let bat_code = read_data();
-        if bat_code == 0xAA {
-            // command reset mouse succeeded
-            return Ok(());
-        }
+    if let Ok(SelfTestPassed) = read_data().try_into() {
+        //returns mouse id 0
+        read_data();
+        return Ok(());
     }
     Err("failed to reset mouse")
 }
+/// reset the keyboard
+pub fn reset_keyboard() -> Result<(), &'static str> {
+    command_to_keyboard(HostToKeyboardCommandOrData::KeyboardCommand(ResetAndStartSelfTest))?; 
+    if let Ok(SelfTestPassed) = read_data().try_into() {
+        return Ok(())
+    }
+    Err("failed to reset keyboard")
+}
 
 /// resend the most recent packet again
 #[allow(dead_code)]
diff --git a/kernel/ps2_controller/Cargo.toml b/kernel/ps2_controller/Cargo.toml
new file mode 100644
index 0000000000..7f1fb53080
--- /dev/null
+++ b/kernel/ps2_controller/Cargo.toml
@@ -0,0 +1,21 @@
+[package]
+name = "ps2_controller"
+version = "0.1.0"
+description = "Driver for the 8042 PS/2 controller"
+authors = ["Hecatia Elegua"]
+edition = "2021"
+
+[dependencies.ps2]
+path = "../ps2"
+
+[dependencies.acpi]
+path = "../acpi"
+
+[dependencies.fadt]
+path = "../acpi/fadt"
+
+[dependencies.log]
+version = "0.4.8"
+
+[lib]
+crate-type = ["rlib"]
diff --git a/kernel/ps2_controller/src/lib.rs b/kernel/ps2_controller/src/lib.rs
new file mode 100644
index 0000000000..2ab5c08281
--- /dev/null
+++ b/kernel/ps2_controller/src/lib.rs
@@ -0,0 +1,106 @@
+#![no_std]
+
+use log::{debug, warn};
+use ps2::{write_command, flush_output_buffer, read_config, write_config, read_controller_test_result, read_port_test_result, PortTestResult, HostToControllerCommand::*, reset_keyboard, reset_mouse};
+use fadt::Fadt;
+
+// see https://wiki.osdev.org/%228042%22_PS/2_Controller#Initialising_the_PS.2F2_Controller
+/// initialize the first and second (if it exists) PS/2 port
+pub fn init() -> Result<(), &'static str> {
+    // Step 1: Initialise USB Controllers
+    // no USB support yet
+
+    // Step 2: Determine if the PS/2 Controller Exists
+    let acpi_tables = acpi::get_acpi_tables().lock();
+    // if ACPI and therefore FADT is unsupported, PS/2 controller is assumed to exist
+    if let Some(fadt) = Fadt::get(&acpi_tables) {
+        // if earlier than ACPI v2, PS/2 controller is assumed to exist
+        if fadt.header.revision > 1 {
+            let has_controller = fadt.iapc_boot_architecture_flags & 0b10 == 0b10;
+            if !has_controller {
+                return Err("no PS/2 Controller present");
+            }
+        }
+    }
+
+    // Step 3: Disable Devices
+    write_command(DisablePort1);
+    write_command(DisablePort2);
+
+    // Step 4: Flush The Output Buffer
+    flush_output_buffer();
+
+    // Step 5: Set the Controller Configuration Byte
+    let mut config = read_config();
+    config.set_port1_interrupt_enabled(false);
+    config.set_port2_interrupt_enabled(false);
+    config.set_port1_translation_enabled(false);
+    let has_mouse = config.port2_clock_disabled();
+    write_config(config);
+
+    // Step 6: Perform Controller Self Test
+    write_command(TestController);
+    read_controller_test_result()?;
+    debug!("passed PS/2 controller test");
+    // as this can reset the controller on some hardware, we restore the config
+    write_config(config);
+
+    // Step 7: Determine If There Are 2 Channels
+    // not needed, already done above
+
+    // Step 8: Perform Interface Tests
+    write_command(TestPort1);
+    use PortTestResult::*;
+    let port_1_result = match read_port_test_result()? {
+        Passed => Ok(()),
+        ClockLineStuckLow => Err("failed PS/2 port 1 test, clock line stuck low"),
+        ClockLineStuckHigh => Err("failed PS/2 port 1 test, clock line stuck high"),
+        DataLineStuckLow => Err("failed PS/2 port 1 test, data line stuck low"),
+        DataLineStuckHigh => Err("failed PS/2 port 1 test, clock line stuck high"),
+    };
+    let port_2_result = if has_mouse {
+        write_command(TestPort2);
+        match read_port_test_result()? {
+            Passed => Ok(()),
+            ClockLineStuckLow => Err("failed PS/2 port 2 test, clock line stuck low"),
+            ClockLineStuckHigh => Err("failed PS/2 port 2 test, clock line stuck high"),
+            DataLineStuckLow => Err("failed PS/2 port 2 test, data line stuck low"),
+            DataLineStuckHigh => Err("failed PS/2 port 2 test, clock line stuck high"),
+        }
+    } else {
+        Err("PS/2 controller is not dual channel, so no mouse available")
+    };
+    if port_1_result.is_err() && port_2_result.is_err() {
+        //TODO: does not need to crash the system if we have another input method (over USB)
+        return Err("failed both PS/2 port tests, terminating PS/2 controller driver");
+    }
+
+    // Step 9: Enable Devices
+    match port_1_result {
+        Ok(()) => {
+            debug!("passed PS/2 port 1 test");
+            write_command(EnablePort1);
+            config.set_port1_clock_disabled(false);
+            config.set_port1_interrupt_enabled(true);
+        }
+        Err(e) => warn!("{e}"),
+    }
+    match port_2_result {
+        Ok(()) => {
+            debug!("passed PS/2 port 2 test");
+            write_command(EnablePort2);
+            config.set_port2_clock_disabled(false);
+            config.set_port2_interrupt_enabled(true);
+        }
+        Err(e) => warn!("{e}"),
+    }
+    write_config(config);
+
+    // Step 10: Reset Devices
+    reset_keyboard()?;
+    reset_mouse()?;
+
+    // for debugging purposes
+    debug!("Final PS/2 {:?}", read_config());
+    Ok(())
+}


Test patch: