Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes to keyboard code, making PC-compatible initialization working. #1

Merged
merged 5 commits into from
Nov 1, 2023

Conversation

Hamberthm
Copy link

Hello! First of all, very nice approach using multi-threading.

In my testing I discovered three different issues that preventing the code from working in many PC-compatible systems and making it through the post succesfully. I made only changes related to keyboard part of the code (no mouse fixes yet).

All issues where tested using DEBUG ifdef's and strategically set serial console prints, ommited in these commits.

ISSUE 1
-Testing on: Toshiba Satellite 205CDS, combined keyboard-mouse PS/2 port, mini-DIN connector.
-Problem: This laptop uses the GET DEVICE ID COMMAND to decide if the divice connected is a Keyboard or a Mouse. For some reason the two byte writes for the ID were failing, and because of that the laptop defaulted the device to be a Mouse (and of course that won't work).
-SOLUTION: Adding two While loops to retry the writes as many times as needed solved the problem.

ISSUE 2
-Testing on: Toshiba Satellite 205CDS, combined keyboard-mouse PS/2 port, mini-DIN connector. Possibly affecting more systems.
-Problem: At first power-on the interface was waiting to send 0xAA command in a loop of Write-tries, during PS2Keyboard::begin(). This for some reason caused the interface to miss critical first-hand commands from the laptop, like the GET DEVICE ID discussed above. As a result the laptop was also defaulting the keyboard as a mouse.
-Solution: BAT SUCCESS code (0xAA) should ALWAYS be sent a coulple of houndred of milliseconds after first power up, regardless of bus state. Added a 200ms delay and a ensured write() solved the problem.

ISSUE 3
-Testing on: Intel 486 PC-Compatible desktop system, 5 pin full size DIN connector.
-Problem: For some reason the host wasn't getting the ACKs correctly during the SET/RESET LEDS command. This caused the host to abort initialization and ignore the keyboard from that function in advance.
-Solution: Changed the ACKs to While-Write loops solved the problem. Need testing in more implementations that use LED info, to ensure it's working OK.

Hope this helps. I'm new to GitHub, I'm king of a noob on repositories, so tell me if I'm doing something wrong.

Cheers!

The interface was missing some commands in one of my sytems because of waiting to send the 0xAA "BAT test success". BAT success should always be sent a couple of houndred of milliseconds after power up, regarding state.
Testing in my Toshiba 205CDS with a combined port, DEVICE ID command was failing to write the ID properly. Added some loops to shield time to the controller and get them properly.
On my 486 PC, ACKs were being missed by the host controller during LED set command handling. This made the process to end and the controller to ignore the keyboard after that command. This makes it stable.
@hrko
Copy link
Owner

hrko commented Nov 1, 2023

Very sorry I have not noticed your PR for a while...

Thank you for the comprehensive breakdown of the issues you encountered and for the solutions you've provided. Your detailed testing and descriptions are invaluable.

The code looks good to me, so I will merge it in.

@hrko hrko merged commit aa6d685 into hrko:master Nov 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants