From 889ea1593696ca2a53b52fd6b17c4510158e6daa Mon Sep 17 00:00:00 2001
From: Koen Zandberg <koen@bergzand.net>
Date: Sat, 29 Aug 2020 09:24:02 +0200
Subject: [PATCH] fe310: Use read-modify-store instruction on GPIO

The rv32imac supports the A (atomic) extensions containing
read-modify-store operations. This commit modifies the GPIO code to use
these for all bitwise operations. The atomic operations are emitted with
relaxed ordering as they do not require multiprocessor synchronization.

This decreases the duration of the gpio operations from 59 ns to 50 ns
per call. depending a bit on the type of operation.
---
 cpu/fe310/periph/gpio.c | 71 +++++++++++++++++++++++++----------------
 1 file changed, 43 insertions(+), 28 deletions(-)

diff --git a/cpu/fe310/periph/gpio.c b/cpu/fe310/periph/gpio.c
index 1255bc0fda..6c7819871a 100644
--- a/cpu/fe310/periph/gpio.c
+++ b/cpu/fe310/periph/gpio.c
@@ -37,6 +37,20 @@ static gpio_flank_t isr_flank[GPIO_NUMOF];
 static gpio_isr_ctx_t isr_ctx[GPIO_NUMOF];
 #endif /* MODULE_PERIPH_GPIO_IRQ */
 
+/* Really always inline these functions These two should be only a few
+ * instructions as the atomic_fetch_or is a single instruction on rv32imac */
+static __attribute((always_inline)) inline
+void _set_pin_reg(uint32_t offset, gpio_t pin)
+{
+    __atomic_fetch_or(&GPIO_REG(offset), 1 << pin, __ATOMIC_RELAXED);
+}
+
+static __attribute((always_inline)) inline
+void _clr_pin_reg(uint32_t offset, gpio_t pin)
+{
+    __atomic_fetch_and(&GPIO_REG(offset), ~(1 << pin), __ATOMIC_RELAXED);
+}
+
 int gpio_init(gpio_t pin, gpio_mode_t mode)
 {
     /* Check for valid pin */
@@ -45,23 +59,24 @@ int gpio_init(gpio_t pin, gpio_mode_t mode)
     }
 
     /*  Configure the mode */
+
     switch (mode) {
         case GPIO_IN:
-            GPIO_REG(GPIO_INPUT_EN) |= (1 << pin);
-            GPIO_REG(GPIO_OUTPUT_EN) &= ~(1 << pin);
-            GPIO_REG(GPIO_PULLUP_EN) &= ~(1 << pin);
+            _set_pin_reg(GPIO_INPUT_EN, pin);
+            _clr_pin_reg(GPIO_OUTPUT_EN, pin);
+            _clr_pin_reg(GPIO_PULLUP_EN, pin);
             break;
 
         case GPIO_IN_PU:
-            GPIO_REG(GPIO_INPUT_EN) |= (1 << pin);
-            GPIO_REG(GPIO_OUTPUT_EN) &= ~(1 << pin);
-            GPIO_REG(GPIO_PULLUP_EN) |= (1 << pin);
+            _clr_pin_reg(GPIO_OUTPUT_EN, pin);
+            _set_pin_reg(GPIO_INPUT_EN, pin);
+            _set_pin_reg(GPIO_PULLUP_EN, pin);
             break;
 
         case GPIO_OUT:
-            GPIO_REG(GPIO_INPUT_EN) &= ~(1 << pin);
-            GPIO_REG(GPIO_OUTPUT_EN) |= (1 << pin);
-            GPIO_REG(GPIO_PULLUP_EN) &= ~(1 << pin);
+            _set_pin_reg(GPIO_OUTPUT_EN, pin);
+            _clr_pin_reg(GPIO_INPUT_EN, pin);
+            _clr_pin_reg(GPIO_PULLUP_EN, pin);
             break;
 
         default:
@@ -69,8 +84,8 @@ int gpio_init(gpio_t pin, gpio_mode_t mode)
     }
 
     /* Configure the pin muxing for the GPIO */
-    GPIO_REG(GPIO_IOF_EN) &= ~(1 << pin);
-    GPIO_REG(GPIO_IOF_SEL) &= ~(1 << pin);
+    _clr_pin_reg(GPIO_IOF_EN, pin);
+    _clr_pin_reg(GPIO_IOF_SEL, pin);
 
     return 0;
 }
@@ -82,26 +97,26 @@ int gpio_read(gpio_t pin)
 
 void gpio_set(gpio_t pin)
 {
-    GPIO_REG(GPIO_OUTPUT_VAL) |= (1 << pin);
+    _set_pin_reg(GPIO_OUTPUT_VAL, pin);
 }
 
 void gpio_clear(gpio_t pin)
 {
-    GPIO_REG(GPIO_OUTPUT_VAL) &= ~(1 << pin);
+    _clr_pin_reg(GPIO_OUTPUT_VAL, pin);
 }
 
 void gpio_toggle(gpio_t pin)
 {
-    GPIO_REG(GPIO_OUTPUT_VAL) ^= (1 << pin);
+    __atomic_fetch_xor(&GPIO_REG(GPIO_OUTPUT_VAL), (1 << pin), __ATOMIC_RELAXED);
 }
 
 void gpio_write(gpio_t pin, int value)
 {
     if (value) {
-        GPIO_REG(GPIO_OUTPUT_VAL) |= (1 << pin);
+        _set_pin_reg(GPIO_OUTPUT_VAL, pin);
     }
     else {
-        GPIO_REG(GPIO_OUTPUT_VAL) &= ~(1 << pin);
+        _clr_pin_reg(GPIO_OUTPUT_VAL, pin);
     }
 }
 
@@ -118,16 +133,16 @@ void gpio_isr(int num)
     /* Clear interrupt */
     switch (isr_flank[pin]) {
         case GPIO_FALLING:
-            GPIO_REG(GPIO_FALL_IP) |= (1 << pin);
+            _set_pin_reg(GPIO_FALL_IP, pin);
             break;
 
         case GPIO_RISING:
-            GPIO_REG(GPIO_RISE_IP) |= (1 << pin);
+            _set_pin_reg(GPIO_RISE_IP, pin);
             break;
 
         case GPIO_BOTH:
-            GPIO_REG(GPIO_FALL_IP) |= (1 << pin);
-            GPIO_REG(GPIO_RISE_IP) |= (1 << pin);
+            _set_pin_reg(GPIO_FALL_IP, pin);
+            _set_pin_reg(GPIO_RISE_IP, pin);
             break;
     }
 }
@@ -172,16 +187,16 @@ void gpio_irq_enable(gpio_t pin)
     /* Enable interrupt for pin */
     switch (isr_flank[pin]) {
         case GPIO_FALLING:
-            GPIO_REG(GPIO_FALL_IE) |= (1 << pin);
+            _set_pin_reg(GPIO_FALL_IE, pin);
             break;
 
         case GPIO_RISING:
-            GPIO_REG(GPIO_RISE_IE) |= (1 << pin);
+            _set_pin_reg(GPIO_RISE_IE, pin);
             break;
 
         case GPIO_BOTH:
-            GPIO_REG(GPIO_FALL_IE) |= (1 << pin);
-            GPIO_REG(GPIO_RISE_IE) |= (1 << pin);
+            _set_pin_reg(GPIO_FALL_IE, pin);
+            _set_pin_reg(GPIO_RISE_IE, pin);
             break;
 
         default:
@@ -199,16 +214,16 @@ void gpio_irq_disable(gpio_t pin)
     /* Disable interrupt for pin */
     switch (isr_flank[pin]) {
         case GPIO_FALLING:
-            GPIO_REG(GPIO_FALL_IE) &= ~(1 << pin);
+            _clr_pin_reg(GPIO_FALL_IE, pin);
             break;
 
         case GPIO_RISING:
-            GPIO_REG(GPIO_RISE_IE) &= ~(1 << pin);
+            _clr_pin_reg(GPIO_RISE_IE, pin);
             break;
 
         case GPIO_BOTH:
-            GPIO_REG(GPIO_FALL_IE) &= ~(1 << pin);
-            GPIO_REG(GPIO_RISE_IE) &= ~(1 << pin);
+            _clr_pin_reg(GPIO_FALL_IE, pin);
+            _clr_pin_reg(GPIO_RISE_IE, pin);
             break;
 
         default: