[Bug] PS/2 interrupt handler silently drops bytes in case of errors

Describe the Bug

The PS/2 interrupt handler (ps2_interrupt_service_routine) places incoming bytes on a ring buffer, for later processing by the relevant driver (e.g. ps2_mouse_task). In case there is an error in the incoming data (e.g. parity check failure), the interrupt handler will silently drop the received byte, and set a global ps2_error flag. For the downstream driver (ps2_mouse), it is impossible to recover from an error, since it requires a 3-byte packet of movement data. Consequently, the driver receives three bytes which actually belong to different 3-byte packets!

Consider the following stream of incoming PS/2 bytes, where the letters indicate which 3-byte packet each byte belongs to:

A1 A2 A3 B1 B2 B3 C1 C2 C3

Imagine byte A2 contains a parity error, causing it to be silently dropped by the interrupt handler. The resulting ring buffer will then contain:

A1 A3 B1 B2 B3 C1 C2 C3

The ps2_driver, unknowingly consumes A1 A3 and B1 as a 3-byte movement packet, resulting in very strange mouse movements. To make matters worse, all the packets following the error will also be out of sync (B2 B3 C1).

Some background

I have a trackpoint module which, for some reason, doesn't respond correctly to the Read Data command. However, in stream mode, it will send movement packets continously whenever any movement is detected. As far as I understand (and please correct me if I'm wrong), this is the correct behavior for stream mode.

Stream Mode:

In stream mode, the mouse sends movement data when it detects movement or a change in state of one or more mouse buttons. The maximum rate at which this data reporting may occur is known as the sample rate. This parameter ranges from 10 samples/sec to 200 samples/sec. Its default value is 100 samples/sec and the host may change that value by using the "Set Sample Rate" (0xF3) command. Stream mode is the default mode of operation.

(From https://web.archive.org/web/20041117095622/http://panda.cs.ndsu.nodak.edu:80/~achapwes/PICmicro/mouse/mouse.html )

ps2_mouse_task always issues a Read Data command, regardless of mode, before reading a 3-byte movement packet. I have modified ps2_mouse_task to not send the Read Data command, and simply process the data as it lands in the ring buffer. For some weird reason, the parity check fails every now and again, causing the input stream to become out of sync, as described above. I'm not sure if this issue exists for other PS/2 devices that do respond correctly to the Read Data command, but if they also send movement packets continuously in stream mode, I suspect it will.

Possible solution:

For the ps2_mouse driver, I think it is safe to simply ignore any 3-byte packet with errors. To do so, the mouse driver needs to know if any of the 3 bytes contain errors. Hence the interrupt handler must stop dropping bytes, and each byte should be accompanied with an error flag. For example:

1. Change data type of ring buffer to uint16_t
2. Use lower byte for data, upper byte for error flags
3. Modify ps2_host_recv and ps2_host_recv_response to return an error code (0 for success), and a pointer to where the data should be written.

Alternatively, one could try issuing a Resend command, but this is probably overkill.

I would be happy to implement something like this and submit a PR, but would first like to hear your thoughts. Perhaps it's only a problem with my peculiar PS/2 device :)

System Information

Keyboard: crkbd Revision (if applicable): v3 Operating system: Linux qmk doctor output:

Ψ QMK Doctor is checking your environment.
Ψ CLI version: 1.1.0
Ψ QMK home: /home/joh/code/qmk_firmware
Ψ Detected Linux.
Ψ Git branch: master
Ψ Repo version: 0.17.6
Ψ - Latest master: 2022-08-03 12:23:17 -0500 (df8a538489) -- Userspace: muppetjones (#1) (#13461)
Ψ - Latest upstream/master: 2022-08-16 20:41:31 +0200 (837a57b3c5) -- Adjust the hierarchy of chapter `Deferred Execution` (#18075)
Ψ - Latest upstream/develop: None
Ψ - Common ancestor with upstream/master: 2022-08-03 12:23:17 -0500 (df8a538489) -- Userspace: muppetjones (#1) (#13461)
Ψ - Common ancestor with upstream/develop: None
Ψ All dependencies are installed.
Ψ Found arm-none-eabi-gcc version 9.2.1
Ψ Found avr-gcc version 5.4.0
Ψ Found avrdude version 6.3-20171130
Ψ Found dfu-util version 0.9
Ψ Found dfu-programmer version 0.6.1
Ψ Submodules are up to date.
Ψ Submodule status:
Ψ - lib/chibios: 2021-12-08 11:37:03 +0000 --  (257302333)
Ψ - lib/chibios-contrib: 2022-04-18 12:52:47 +0200 --  (2a6b73ff)
Ψ - lib/googletest: 2021-06-11 06:37:43 -0700 --  (e2239ee6)
Ψ - lib/lufa: 2021-07-31 15:44:44 +1000 --  (19a5d533f)
Ψ - lib/printf: 2020-01-01 18:16:48 +0100 --  (d3b9846)
Ψ - lib/vusb: 2020-08-16 01:07:21 +0100 --  (bdb53e4)
Ψ QMK is ready to go

Any keyboard related software installed?

• AutoHotKey (Windows)
• Karabiner (macOS)
• Other: