From 53b65ddc410191f646d50b55e81c6f7a6653dfe8 Mon Sep 17 00:00:00 2001 From: Liav A Date: Sat, 19 Feb 2022 20:34:45 +0200 Subject: [PATCH] Kernel: Make i8042 existence check more robust against faulty hardware Some hardware controllers might reset when trying to do self-test, so keep the configuration byte to restore it later on. To ensure we are not missing the response from the i8042 controller, bump the attempts count to 20 times after initiating self-test check. Also, try to drain the i8042 controller output buffer as it might be a early good indication on whether i8042 is present or not. To ensure we drain all the output buffer, we attempt to read from the buffer 50 times and not 20 times. --- Kernel/Devices/HID/I8042Controller.cpp | 56 ++++++++++++++++++++++---- 1 file changed, 49 insertions(+), 7 deletions(-) diff --git a/Kernel/Devices/HID/I8042Controller.cpp b/Kernel/Devices/HID/I8042Controller.cpp index daf354b0713..cb96e364d95 100644 --- a/Kernel/Devices/HID/I8042Controller.cpp +++ b/Kernel/Devices/HID/I8042Controller.cpp @@ -34,19 +34,61 @@ UNMAP_AFTER_INIT I8042Controller::I8042Controller() UNMAP_AFTER_INIT bool I8042Controller::check_existence_via_probing(Badge) { { + u8 configuration = 0; SpinlockLocker lock(m_lock); + + // This drains the output buffer and serves as an existence test. + if (auto result = drain_output_buffer(); result.is_error()) { + dbgln("I8042: Trying to flush output buffer as an existence test failed, error {}", result.error()); + return false; + } + // Note: Perform controller self-test before touching the controller // Try to probe the controller for 10 times and give up if nothing // responded. - for (int attempt = 0; attempt < 10; attempt++) { + // Some controllers will reset and behave abnormally on this, so let's ensure + // we keep the configuration before initiating this command. + + if (auto result = do_wait_then_write(I8042Port::Command, I8042Command::ReadConfiguration); result.is_error()) { + dbgln("I8042: Trying to read configuration failed during the existence test, error {}", result.error()); + return false; + } + + { + auto result = do_wait_then_read(I8042Port::Buffer); + if (result.is_error()) { + dbgln("I8042: Trying to read configuration failed during the existence test, error {}", result.error()); + return false; + } + configuration = result.release_value(); + } + + bool successful_self_test = false; + for (int attempt = 0; attempt < 20; attempt++) { do_write(I8042Port::Command, I8042Command::TestPS2Controller); - if (do_read(I8042Port::Buffer) == I8042Response::ControllerTestPassed) - return true; + if (do_read(I8042Port::Buffer) == I8042Response::ControllerTestPassed) { + successful_self_test = true; + break; + } // Note: Wait 500 microseconds in case the controller couldn't respond IO::delay(500); } - dbgln("I8042: Trying to probe for existence of controller failed"); - return false; + if (!successful_self_test) { + dbgln("I8042: Trying to probe for existence of controller failed"); + return false; + } + + if (auto result = do_wait_then_write(I8042Port::Command, I8042Command::WriteConfiguration); result.is_error()) { + dbgln("I8042: Trying to restore configuration after self-test failed with error {}", result.error()); + return false; + } + + if (auto result = do_wait_then_write(I8042Port::Buffer, configuration); result.is_error()) { + dbgln("I8042: Trying to write restored configuration after self-test failed with error {}", result.error()); + return false; + } + + return true; } } @@ -56,7 +98,7 @@ UNMAP_AFTER_INIT ErrorOr I8042Controller::detect_devices() u8 configuration; { SpinlockLocker lock(m_lock); - + // Note: This flushes all the garbage left in the controller registers TRY(drain_output_buffer()); TRY(do_wait_then_write(I8042Port::Command, I8042Command::DisableFirstPS2Port)); @@ -179,7 +221,7 @@ bool I8042Controller::irq_process_input_buffer(HIDDevice::Type instrument_type) ErrorOr I8042Controller::drain_output_buffer() { - for (int attempt = 0; attempt < 5; attempt++) { + for (int attempt = 0; attempt < 50; attempt++) { u8 status = IO::in8(I8042Port::Status); if (!(status & I8042StatusFlag::OutputBuffer)) return {};