From 860c6f203103f329d192547c4f62ff2471f99b43 Mon Sep 17 00:00:00 2001 From: "David A. Mellis" Date: Fri, 18 Feb 2011 10:41:29 -0500 Subject: Revert "Changes to optimized digitalWrte(), etc." This reverts commit aa1f1cbda9d6bb52785f98b40746920853d6579b. --- cores/arduino/pins_arduino.h | 46 +++++++++++-- cores/arduino/wiring.h | 148 +++++++++++------------------------------ cores/arduino/wiring_digital.c | 76 +++++++++++++++++++-- 3 files changed, 154 insertions(+), 116 deletions(-) (limited to 'cores/arduino') diff --git a/cores/arduino/pins_arduino.h b/cores/arduino/pins_arduino.h index 4bf6470..63f4257 100644 --- a/cores/arduino/pins_arduino.h +++ b/cores/arduino/pins_arduino.h @@ -339,8 +339,6 @@ INLINED uint8_t inlined_digitalPinToBitMask(uint8_t pin) } } -// XXX: this needs to return false (or -1) if the pin doesn't have a timer, -// rather than throwing a compilation error. INLINED uint8_t inlined_digitalPinToTimer(uint8_t pin) { switch(pin) { @@ -455,8 +453,6 @@ INLINED uint8_t inlined_digitalPinToBitMask(uint8_t pin) } } -// XXX: this needs to return false (or -1) if the pin doesn't have a timer, -// rather than throwing a compilation error. INLINED uint8_t inlined_digitalPinToTimer(uint8_t pin) { switch(pin) { @@ -481,4 +477,46 @@ INLINED uint8_t inlined_digitalPinToTimer(uint8_t pin) #define analogInPinToBit(P) (P) +INLINED uint8_t digitalPinToPort(uint8_t pin) { + if (__builtin_constant_p(pin)) + return inlined_digitalPinToPort(pin); + else + return pgm_read_byte( digital_pin_to_port_PGM + pin ); +} + +INLINED uint8_t digitalPinToBitMask(uint8_t pin) { + if (__builtin_constant_p(pin)) + return inlined_digitalPinToBitMask(pin); + else + return pgm_read_byte( digital_pin_to_bit_mask_PGM + pin ); +} + +INLINED uint8_t digitalPinToTimer(uint8_t pin) { + if (__builtin_constant_p(pin)) + return inlined_digitalPinToTimer(pin); + else + return pgm_read_byte( digital_pin_to_timer_PGM + pin ); +} + +INLINED volatile uint8_t *portOutputRegister(uint8_t index) { + if (__builtin_constant_p(index)) + return inlined_portOutputRegister(index); + else + return (volatile uint8_t *)( pgm_read_word( port_to_output_PGM + index ) ); +} + +INLINED volatile uint8_t* portInputRegister(uint8_t index) { + if (__builtin_constant_p(index)) + return inlined_portInputRegister(index); + else + return (volatile uint8_t *)( pgm_read_word( port_to_input_PGM + index) ); +} + +INLINED volatile uint8_t* portModeRegister(uint8_t index) { + if (__builtin_constant_p(index)) + return inlined_portModeRegister(index); + else + return (volatile uint8_t *)( pgm_read_word( port_to_mode_PGM + index) ); +} + #endif diff --git a/cores/arduino/wiring.h b/cores/arduino/wiring.h index 66ce2f7..433c87e 100755 --- a/cores/arduino/wiring.h +++ b/cores/arduino/wiring.h @@ -130,136 +130,68 @@ void detachInterrupt(uint8_t); void setup(void); void loop(void); -INLINED uint8_t digitalPinToPort(uint8_t pin) { - if (__builtin_constant_p(pin)) - return inlined_digitalPinToPort(pin); - else - return pgm_read_byte( digital_pin_to_port_PGM + pin ); -} - -INLINED uint8_t digitalPinToBitMask(uint8_t pin) { - if (__builtin_constant_p(pin)) - return inlined_digitalPinToBitMask(pin); - else - return pgm_read_byte( digital_pin_to_bit_mask_PGM + pin ); -} - -INLINED uint8_t digitalPinToTimer(uint8_t pin) { - if (__builtin_constant_p(pin)) - return inlined_digitalPinToTimer(pin); - else - return pgm_read_byte( digital_pin_to_timer_PGM + pin ); -} - -INLINED volatile uint8_t *portOutputRegister(uint8_t index) { - if (__builtin_constant_p(index)) - return inlined_portOutputRegister(index); - else - return (volatile uint8_t *)( pgm_read_word( port_to_output_PGM + index ) ); -} - -INLINED volatile uint8_t* portInputRegister(uint8_t index) { - if (__builtin_constant_p(index)) - return inlined_portInputRegister(index); - else - return (volatile uint8_t *)( pgm_read_word( port_to_input_PGM + index) ); -} - -INLINED volatile uint8_t* portModeRegister(uint8_t index) { - if (__builtin_constant_p(index)) - return inlined_portModeRegister(index); - else - return (volatile uint8_t *)( pgm_read_word( port_to_mode_PGM + index) ); -} - /* * Check if a given pin requires locking. * When accessing lower 32 IO ports we can use SBI/CBI instructions, which are atomic. However * other IO ports require load+modify+store and we need to make them atomic by disabling * interrupts. */ -INLINED int registerWriteNeedsLocking(volatile uint8_t *reg) +INLINED int portWriteNeedsLocking(uint8_t pin) { /* SBI/CBI instructions only work on lower 32 IO ports */ - if (reg > (volatile uint8_t*)&_SFR_IO8(0x1F)) { + if (inlined_portOutputRegister(inlined_digitalPinToPort(pin)) > (volatile uint8_t*)&_SFR_IO8(0x1F)) { return 1; } return 0; } -#define digitalWrite_implementation(pin, value)\ -do {\ - uint8_t oldSREG;\ - uint8_t bit = digitalPinToBitMask(pin);\ - uint8_t port = digitalPinToPort(pin);\ - volatile uint8_t *reg = portOutputRegister(port);\ -\ - if (!__builtin_constant_p(pin) || registerWriteNeedsLocking(reg)) {\ - oldSREG = SREG;\ - cli();\ - }\ -\ - if (value == LOW) {\ - *reg &= ~bit;\ - } else {\ - *reg |= bit;\ - }\ -\ - if (!__builtin_constant_p(pin) || registerWriteNeedsLocking(reg)) {\ - SREG = oldSREG;\ - }\ -} while(0) - +/* + * These functions will perform OR/AND on a given register, and are atomic. + */ +extern void __digitalWriteOR_locked(volatile uint8_t*out, uint8_t bit); +extern void __digitalWriteAND_locked(volatile uint8_t*out, uint8_t bit); + INLINED void digitalWrite(uint8_t pin, uint8_t value) { - if (__builtin_constant_p(pin)) digitalWrite_implementation(pin, value); - else digitalWrite_lookup(pin, value); + if (__builtin_constant_p(pin)) { + if (portWriteNeedsLocking(pin)) { + if (value==LOW) { + __digitalWriteAND_locked(inlined_portOutputRegister(inlined_digitalPinToPort(pin)),~inlined_digitalPinToBitMask(pin)); + } else { + __digitalWriteOR_locked(inlined_portOutputRegister(inlined_digitalPinToPort(pin)),inlined_digitalPinToBitMask(pin)); + } + } else { + if (value==LOW) { + *inlined_portOutputRegister(inlined_digitalPinToPort(pin)) &= ~(inlined_digitalPinToBitMask(pin)); + } else { + *inlined_portOutputRegister(inlined_digitalPinToPort(pin)) |= inlined_digitalPinToBitMask(pin); + } + } + } else { + digitalWrite_lookup(pin,value); + } } -#define pinMode_implementation(pin, value)\ -do {\ - uint8_t bit = digitalPinToBitMask(pin);\ - uint8_t oldSREG;\ - uint8_t port = digitalPinToPort(pin);\ - volatile uint8_t *reg = portModeRegister(port);\ -\ - if (!__builtin_constant_p(pin) || registerWriteNeedsLocking(reg)) {\ - oldSREG = SREG;\ - cli();\ - }\ -\ - if (value == INPUT) { \ - *reg &= ~bit;\ - } else {\ - *reg |= bit;\ - }\ -\ - if (!__builtin_constant_p(pin) || registerWriteNeedsLocking(reg)) {\ - SREG = oldSREG;\ - }\ -} while(0) - -INLINED void pinMode(uint8_t pin, uint8_t value) +INLINED void pinMode(uint8_t pin, uint8_t mode) { - if (__builtin_constant_p(pin)) pinMode_implementation(pin, value); - else pinMode_lookup(pin, value); + if (__builtin_constant_p(pin)) { + if (mode==INPUT) { + *inlined_portModeRegister(inlined_digitalPinToPort(pin)) &= ~(inlined_digitalPinToBitMask(pin)); + } else { + *inlined_portModeRegister(inlined_digitalPinToPort(pin)) |= inlined_digitalPinToBitMask(pin); + } + } else { + pinMode_lookup(pin,mode); + } } -#define digitalRead_implementation(pin)\ -do {\ - uint8_t bit = digitalPinToBitMask(pin);\ - uint8_t port = digitalPinToPort(pin);\ -\ - if (port == NOT_A_PIN) return LOW;\ -\ - if (*portInputRegister(port) & bit) return HIGH;\ - return LOW;\ -} while(0) - INLINED int digitalRead(uint8_t pin) { - if (__builtin_constant_p(pin)) digitalRead_implementation(pin); - else return digitalRead_lookup(pin); + if (__builtin_constant_p(pin)) { + return !! *inlined_portInputRegister(inlined_digitalPinToPort(pin)); + } else { + return digitalRead_lookup(pin); + } } diff --git a/cores/arduino/wiring_digital.c b/cores/arduino/wiring_digital.c index 9671045..95666b1 100755 --- a/cores/arduino/wiring_digital.c +++ b/cores/arduino/wiring_digital.c @@ -27,9 +27,28 @@ #include "wiring_private.h" #include "pins_arduino.h" -void pinMode_lookup(uint8_t pin, uint8_t val) +void pinMode_lookup(uint8_t pin, uint8_t mode) { - pinMode_implementation(pin, val); + uint8_t bit = digitalPinToBitMask(pin); + uint8_t port = digitalPinToPort(pin); + volatile uint8_t *reg; + + if (port == NOT_A_PIN) return; + + // JWS: can I let the optimizer do this? + reg = portModeRegister(port); + + if (mode == INPUT) { + uint8_t oldSREG = SREG; + cli(); + *reg &= ~bit; + SREG = oldSREG; + } else { + uint8_t oldSREG = SREG; + cli(); + *reg |= bit; + SREG = oldSREG; + } } // Forcing this inline keeps the callers from having to push their own stuff @@ -102,12 +121,61 @@ static void turnOffPWM(uint8_t timer) } } +void __digitalWriteOR_locked(volatile uint8_t*out, uint8_t bit) +{ + uint8_t oldSREG = SREG; + cli(); + *out |= bit; + SREG=oldSREG; +} + +void __digitalWriteAND_locked(volatile uint8_t*out, uint8_t bit) +{ + uint8_t oldSREG = SREG; + cli(); + *out &= bit; // NOTE - no inversion here, invert before calling!!! + SREG=oldSREG; +} + void digitalWrite_lookup(uint8_t pin, uint8_t val) { - digitalWrite_implementation(pin, val); + uint8_t timer = digitalPinToTimer(pin); + uint8_t bit = digitalPinToBitMask(pin); + uint8_t port = digitalPinToPort(pin); + volatile uint8_t *out; + + if (port == NOT_A_PIN) return; + + // If the pin that support PWM output, we need to turn it off + // before doing a digital write. + if (timer != NOT_ON_TIMER) turnOffPWM(timer); + + out = portOutputRegister(port); + + uint8_t oldSREG = SREG; + cli(); + + if (val == LOW) { + *out &= ~bit; + } else { + *out |= bit; + } + + SREG = oldSREG; } int digitalRead_lookup(uint8_t pin) { - digitalRead_implementation(pin); + uint8_t timer = digitalPinToTimer(pin); + uint8_t bit = digitalPinToBitMask(pin); + uint8_t port = digitalPinToPort(pin); + + if (port == NOT_A_PIN) return LOW; + + // If the pin that support PWM output, we need to turn it off + // before getting a digital reading. + if (timer != NOT_ON_TIMER) turnOffPWM(timer); + + if (*portInputRegister(port) & bit) return HIGH; + return LOW; } -- cgit v1.2.3-18-g5258