Always use pullup with ps2 logic, except when PSU is off

[stople]

This PR adds two GPIO helper functions, to simplify toggling between input+pullup and output+driving low.

This is used in PS2 logic, to simplify the code there.

This also fixes a few places where the old logic would set input without pullup. This causes problems on some keyboards, as this causes that only the external 4.7k pullup resistor is used. Which is not fast enough for at least one Dell keyboard.

This change causes the mentioned Dell keyboard to work again, without needing additional pull-up resistors.

[stople]

As commented on Discord, we should ensure that the pullup is not attempting to provide power to the keyboard when the PSU is powered off, as keyboard power seem to be the ordinary +5V power supply, and not the +5V standby power supply which SMC uses

[stople]

I have now added a commit which will set the ps2 ports to high Z when PSU is powered off. This brings the PS2 port pins to ~0.03V instead of ~0.74V when PSU is off. It also prevents the pullups to pull the +5v up to 0.20V via the external pullup-resistors. Now the +5V measures 0.03V instead when the PSU is off.

[stefan-b-jakobsson]

I've so far only read the code, and not yet tested it on hardware.

Would it be cleaner if the internal pull-ups are engaged in the power-on function and disabled in the power-off function? The functions in ps2.h would then be required to always leave the pins in either input/pullup or output/low mode. Or do you see some benefit of implementing it like this?

[stople]

I've so far only read the code, and not yet tested it on hardware.

Would it be cleaner if the internal pull-ups are engaged in the power-on function and disabled in the power-off function? The functions in ps2.h would then be required to always leave the pins in either input/pullup or output/low mode. Or do you see some benefit of implementing it like this?

We should at least reset all the logic when changing the power state.

The idea was to let the POWER_ON_active function act as an interface to ps2.h to check if power is supposed to be enabled or not. I also used this function to filter incomming bytes if power is off. But, I realized that I reset the state while button press is detected, and a very quick test shows that the keyboard does not seem to have power long enough to send bytes to the SMC. So I removed this check.

One benefit of controlling the pullup in power-on/power-off as you suggest, is that we can avoid the ugly implementation of PWR_ON_active. Then we can call reset as before.

Can one option be to make a new reset-function, "resetWithoutPullup()", for use in the power-off function?

[stefan-b-jakobsson]

A dedicated function to put the port in a disabled state would be clean.

We have quite a few functions containing the word "reset":

• resetInput()
• resetReceiver()
• reset()

And then there is also keyboardReset() in setup_keyboard.cpp.

It would be great if there was only one reset() function in the class, and that the other functions had more descriptive names of what they do.

resetInput() could possibly be renamed to listen().

resetReceiver() is only called in the class constructor and from the reset() function. Maybe we only need the reset() function.

A new dedicated function that sets the port in a disabled state could be called disable() or maybe unlisten() if we're inspired by the CBM serial functions.

But these are only ideas from the top of my head. Feel free to ignore :-)

[stefan-b-jakobsson]

I tested this code on the Otter. It worked fine with the Dell keyboard.

[stople]

When we power off the power supply, I think we will get falling clock interrupts on both PS2 ports. This causes that the code calls resetInput(). If this is called, we have no good way of knowing weather or not to activate the pullup or not. I think we either have to store a state, or, we have to check if PWR_ON is active.

I have now pushed a commit which stores this in a state, as a bit inside ps2ddr. I have also added a parameter to the reset function, where you can choose between enabled, disabled or keep last enabled state, It seem to do the job, but, it does add 30 more bytes of code compared to the first version which just checked if PWR_ON is active when performing the reset function.

If reduced code size is preferred, I suggest to stick with my first implementation. As this is what I suggest, I have pushed the alternative to a different branch, see this link: https://github.com/stople/x16-smc/tree/ps2-pullup-alternative

[stefan-b-jakobsson]

@stople Could you bump the version to 47.2.1 before this is merged. I have done some more testing and reading, and I think it's time to merge and move on to other things.

[stople]

I downloaded the rebased 47.2.1 build artifact and installed it using x16-update. Using the Dell keyboard, I succeeded changing bootloader back and forth between different versions. Downgrading to 47.2.0, with the Dell keyboard, caused that keyboard to stop functioning, as expected. I had to change to my Perixx keyboard (or, any other of my working keyboards) to be able to send commands to the PC.