Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds a PS/2 GPIO bit-banging driver, which is needed for the zmk ps/2 mouse PR (zmkfirmware/zmk#1751).
Zephyr has support for PS/2, but only for very few PS/2 chips. It didn’t have support for a GPIO bit banging PS/2 driver.
This driver implements the basic communication over the PS/2 protocol.
The driver is completely interrupt driven and does zero polling. So it should be as battery efficient as possible.
Interrupt and timing challenges
Explanation
There were a few challenges with the implementation that I would like some feedback on.
Initially, I was testing it on a blackpill and over usb. It was working very well.
But as soon as I tried running it on a nice!nano, the driver wasn’t able to handle the interrupts fast enough.
To give you a little background info. The PS/2 protocol uses two pins, clock and data. The clock operates at 10.0–16.7 kHz (around 60-100 us per clock).
The ps/2 device controls the clock most of the time and data is read and written by the host on the falling edge. This has to happen within 30-50 us of the falling edge.
There is an interesting discussion on the [zephyr github about the expected latencies](zephyrproject-rtos#41401). And apparently just the lower half of the bluetooth interrupts consume over 100us.
So what I ended up having to do was change the interrupt priorities:
BT_CTLR_LLL_PRIO
from 0 to 1BT_CTLR_ULL_HIGH_PRIO
andBT_CTLR_ULL_LOW_PRIO
from 1 to 2NRF_DEFAULT_IRQ_PRIORITY
(basically all other interrupts) from 1 to 3Additionally, it’s not possible to change the priority of a single pin. Most controllers (including nrf52840) raise one interrupt if there is a change on any of the gpio pins. Then a driver, called gpiote, scans the pins and calls the appropriate callbacks.
So, raising the gpiote interrupt doesn’t just raise it for the PS/2 pins, but also for the key matrix.
In my testing so far, this hasn’t been an issue though.
I didn’t like that I had to do this, but I don’t see another choice.
Implementation Feedback
Part of the difficulty is how to implement these interrupt priority changes. The ideal scenario is that the end-user shouldn’t have to worry about interrupt priorities.
The user adds the PS/2 driver to their devicetree, and the priorities should be set correctly so that it works.
The problem is that there are so many variables. Some chips need the priorities changed, some don’t…
Some priorities can be changed through Kconfig options and some need to be changed in the device tree.
The default interrupt priority on nrf chips is changed through the devicetree define
#define NRF_DEFAULT_IRQ_PRIORITY 3
and it has to be added into the zmk (not zephyr owned file)app/dts/arm/nordic/override.dtsi
.As we discussed on discord devicetree files are processed before Kconfig settings. So we can’t rely on Kconfig alone.
For now I am overridng
NRF_DEFAULT_IRQ_PRIORITY
on the zmk side in my ps2 mouse PR. And I’m changing the BT priorities in the Kconfig.gpio on the zephyr side.I imagine that will need to be changed, but I just wanted to submit the PR and get the conversation going.
Conclusion
I think, everything else was pretty straight forward. I look forward to hearing how to improve the code further.