From fe43cf1d3c5c8f8bb6f44aafe915417aa8d9f6f8 Mon Sep 17 00:00:00 2001 From: stople Date: Mon, 24 Jun 2024 06:52:26 +0200 Subject: [PATCH] Always use pullup with ps2 logic, except when PSU is off (#41) * GPIO library: Add dedicated inputWithPullup/driveLow, for PS2 This is to simplify toggling between the two meaningful states: Input with pullup, and output driving low. This is particularly useful for PS2, which is open collector logic, requiring a strong pullup. The external 4.7k pullup is not always strong enough, but together with the 20-50k additional internal pullup, it handles all known keyboards. * Use the new inputWithPullup/driveLow functions in PS2 logic * Disable PS2 pullup when power supply is off This is to prevent trying to power the keyboard via the PS2 ports when the power supply is off. * Bump version to 47.2.1 --- optimized_gpio.c | 49 +++++++++++++++++++++++++++++++++++++++++++----- optimized_gpio.h | 5 +++++ ps2.h | 37 ++++++++++++++++++------------------ version.h | 2 +- x16-smc.ino | 25 +++++++++++++++++++++--- 5 files changed, 91 insertions(+), 27 deletions(-) diff --git a/optimized_gpio.c b/optimized_gpio.c index 534787b..266d87c 100644 --- a/optimized_gpio.c +++ b/optimized_gpio.c @@ -69,15 +69,15 @@ static void pinMode_opt_params_not_known_compiletime(uint8_t pin, uint8_t mode) uint8_t tmp = SREG; cli(); if (mode == INPUT) { - *ddr_reg &= ~bitmask; - *port_reg &= ~bitmask; + *ddr_reg &= ~bitmask; // Intermediate step -> Input + (high Z or pullup) + *port_reg &= ~bitmask; // Input + High Z } else if (mode == INPUT_PULLUP) { - *ddr_reg &= ~bitmask; - *port_reg |= bitmask; + *ddr_reg &= ~bitmask; // Intermediate step -> Input + (high Z or pullup) + *port_reg |= bitmask; // Input + pullup } else { - *ddr_reg |= bitmask; + *ddr_reg |= bitmask; // Output, high/low is undefined } SREG = tmp; } @@ -176,3 +176,42 @@ inline uint8_t digitalRead_opt(uint8_t pin) return digitalRead_opt_params_not_known_compiletime(pin); } } + + +inline void gpio_inputWithPullup(uint8_t pin) +{ + // pinMode_opt changes both ddr and port when setting to INPUT_PULLUP + pinMode_opt(pin, INPUT_PULLUP); +} + + +// If pin is not known compile-time, the inlined gpio_driveLow will call this non-inlined function +static void gpio_driveLow_params_not_known_compiletime(uint8_t pin) __attribute__((noinline)); +static void gpio_driveLow_params_not_known_compiletime(uint8_t pin) +{ + volatile uint8_t *ddr_reg = get_ddr_address_from_pin(pin); + volatile uint8_t *port_reg = get_port_address_from_pin(pin); + uint8_t bitmask = get_bitmask_from_pin(pin); + uint8_t tmp = SREG; + cli(); + *port_reg &= ~bitmask; // Set port LOW -> Intermediate step, either high Z or driving pin low + *ddr_reg |= bitmask; // Set DDR HIGH -> Drive pin low + SREG = tmp; +} + +inline void gpio_driveLow(uint8_t pin) +{ + if (__builtin_constant_p(pin)) { + // pin and val is known at compile time. + // This will be inlined to two cbi/sbi instructions. + digitalWrite_opt(pin, LOW); + pinMode_opt(pin, OUTPUT); + } + else + { + // pin is not known at compile time. + // Implementation cannot use cbi/sbi. Hence, atomic pin manipulation is required. + // Call a non-inlined function from this inline function. + gpio_driveLow_params_not_known_compiletime(pin); + } +} diff --git a/optimized_gpio.h b/optimized_gpio.h index 445af60..e0b6b13 100644 --- a/optimized_gpio.h +++ b/optimized_gpio.h @@ -8,10 +8,15 @@ extern "C" { #endif +// Arduino-style interface extern inline void pinMode_opt(uint8_t pin, uint8_t mode) __attribute__((always_inline)); extern inline void digitalWrite_opt(uint8_t pin, uint8_t val) __attribute__((always_inline)); extern inline uint8_t digitalRead_opt(uint8_t pin) __attribute__((always_inline)); +// Convenience functions to choose between input+pullup and output+low, needed by e.g. PS2 +extern inline void gpio_inputWithPullup(uint8_t pin) __attribute__((always_inline)); +extern inline void gpio_driveLow(uint8_t pin) __attribute__((always_inline)); + #ifdef __cplusplus } #endif diff --git a/ps2.h b/ps2.h index f156b67..fa5800a 100644 --- a/ps2.h +++ b/ps2.h @@ -4,6 +4,8 @@ #include "optimized_gpio.h" #define SCANCODE_TIMEOUT_MS 50 +bool PWR_ON_active(); + enum PS2_CMD_STATUS : uint8_t { IDLE = 0, CMD_PENDING = 1, @@ -44,8 +46,15 @@ class PS2Port }; virtual void resetInput() { - pinMode_opt(datPin, INPUT); - pinMode_opt(clkPin, INPUT); + if (PWR_ON_active()) { + gpio_inputWithPullup(datPin); + gpio_inputWithPullup(clkPin); + } else { + // Prevent powering the keyboard via the pull-ups when system is off + // Call reset() after changing PWR_ON + pinMode_opt(datPin, INPUT); + pinMode_opt(clkPin, INPUT); + } curCode = 0; parity = 0; rxBitCount = 0; @@ -173,12 +182,10 @@ class PS2Port case 7: //Output data bits 0-7 if (outputBuffer[0] & 1) { - pinMode_opt(datPin, INPUT); - digitalWrite_opt(datPin, HIGH); + gpio_inputWithPullup(datPin); } else { - digitalWrite_opt(datPin, LOW); - pinMode_opt(datPin, OUTPUT); + gpio_driveLow(datPin); } //Update parity @@ -194,12 +201,10 @@ class PS2Port case 8: //Send odd parity bit if ((parity & 1) == 1) { - digitalWrite_opt(datPin, LOW); - pinMode_opt(datPin, OUTPUT); + gpio_driveLow(datPin); } else { - pinMode_opt(datPin, INPUT); - digitalWrite_opt(datPin, HIGH); + gpio_inputWithPullup(datPin); } //Prepare for stop bit @@ -208,8 +213,7 @@ class PS2Port case 9: //Stop bit - pinMode_opt(datPin, INPUT); - digitalWrite_opt(datPin, HIGH); + gpio_inputWithPullup(datPin); rxBitCount++; break; @@ -291,11 +295,8 @@ class PS2Port else if (timerCountdown == 3) { //Initiate request-to-send sequence - digitalWrite_opt(clkPin, LOW); - pinMode_opt(clkPin, OUTPUT); - - digitalWrite_opt(datPin, LOW); - pinMode_opt(datPin, OUTPUT); + gpio_driveLow(clkPin); + gpio_driveLow(datPin); ps2ddr = 1; timerCountdown--; @@ -303,7 +304,7 @@ class PS2Port else if (timerCountdown == 1) { //We are at end of the request-to-send clock hold time of minimum 100 us - pinMode_opt(clkPin, INPUT); + gpio_inputWithPullup(clkPin); timerCountdown = 0; rxBitCount = 0; diff --git a/version.h b/version.h index 007cd43..a3cabfc 100644 --- a/version.h +++ b/version.h @@ -1,3 +1,3 @@ #define version_major 47 #define version_minor 2 -#define version_patch 0 +#define version_patch 1 diff --git a/x16-smc.ino b/x16-smc.ino index 06def07..1d45062 100644 --- a/x16-smc.ino +++ b/x16-smc.ino @@ -185,8 +185,8 @@ void setup() { // Setup Power Supply pinMode_opt(PWR_OK, INPUT); - pinMode_opt(PWR_ON, OUTPUT); digitalWrite_opt(PWR_ON, HIGH); + pinMode_opt(PWR_ON, OUTPUT); // Turn Off Activity LED pinMode_opt(ACT_LED, OUTPUT); @@ -337,6 +337,8 @@ void PowerOffSeq() { digitalWrite_opt(ACT_LED, ACT_LED_OFF); // Ensure activity LED is off _delay_ms(AUDIOPOP_HOLDTIME_MS); // Wait for audio system to stabilize before power is turned off digitalWrite_opt(PWR_ON, HIGH); // Turn off supply + Keyboard.reset(); // Reset and deactivate pullup + Mouse.reset(); // Reset and deactivate pullup SYSTEM_POWERED = 0; // Global Power state Off _delay_ms(RESB_HOLDTIME_MS); // Mostly here to add some delay between presses deassertReset(); @@ -345,6 +347,8 @@ void PowerOffSeq() { void PowerOnSeq() { assertReset(); digitalWrite_opt(PWR_ON, LOW); // turn on power supply + Keyboard.reset(); // Reset and activate pullup + Mouse.reset(); // Reset and activate pullup unsigned long TimeDelta = 0; unsigned long StartTime = millis(); // get current time while (!digitalRead_opt(PWR_OK)) { // Time how long it takes @@ -356,8 +360,6 @@ void PowerOnSeq() { // insert error handler, flash activity light & Halt? IE, require hard power off before continue? } else { - Keyboard.flush(); - Mouse.reset(); defaultRequest = I2C_CMD_GET_KEYCODE_FAST; _delay_ms(RESB_HOLDTIME_MS); // Allow system to stabilize SYSTEM_POWERED = 1; // Global Power state On @@ -689,3 +691,20 @@ ISR(TIMER1_COMPA_vect) { Keyboard.timerInterrupt(); Mouse.timerInterrupt(); } + +bool PWR_ON_active() +{ + // Returns the status of the PWR_ON output port. + // PWR_ON is active low. + // Thus, return true if DDR is output (1) and if PORT is low (0). + // Similar to the SYSTEM_POWERED variable, but this is more accurate when power is changing. + // A future code improvement can be to make gpio functions to read from PORT/DDR register + + // The following code assumes PWR_ON is pin 5, which is PA5. +#if PWR_ON != 5 + #error Please adjust PWR_ON_active() +#endif + + if ((DDRA & _BV(5)) == 0) return false; // Port is input. This is the case when mouse and keyboard objects are created + return (PORTA & _BV(5)) ? false : true; +}