From 1d0f90dcdf149ed0427eebbe51db0ee20c22413d Mon Sep 17 00:00:00 2001
From: Marian Buschsieweke <marian.buschsieweke@ovgu.de>
Date: Mon, 18 Jun 2018 09:10:25 +0200
Subject: [PATCH] cpu/lpc2387: Various fixes for GPIO driver
- Fixed documentation
- Use bitwise operation instead of multiplication and addition in `GPIO_PIN()`
- Allow GPIOs to be configured as input via `gpio_init()`
- Fixed bugs in `gpio_init_mux`:
- `0x01 << ((pin & 31) * 2)` was used before to generate the bitmask, but
this would shift by 62 to the left. Correct is `0x01 << ((pin & 15) * 2)`
(See [datasheet](https://www.nxp.com/docs/en/user-guide/UM10211.pdf) at
pages 156ff)
- Only one of the two bits was cleared previously
- Changed strategy to access GPIO pins:
- Previous strategy:
- Set all bits in FIOMASK except the one for the pin to control to
disable access to them
- Set/clear/read all pins in the target GPIO port (but access to all but
the target pin is ignored because of the applied FIOMASK)
- New strategy:
- Set/clear/read only the target pin
- Advantages:
- Only one access to a GPIO register instead of two
- Proven approach: Access to GPIOs on lpc2387 is mostly done by
accessing the GPIO registers directy (e.g. see the sht11 driver).
Those accesses never touch the FIOMASK register
- No unwanted side effects: Disabling all but one pin in a GPIO port
without undoing that seems not to be a good idea
---
cpu/lpc2387/include/periph_cpu.h | 24 +++++++++++++++-------
cpu/lpc2387/periph/gpio.c | 34 +++++++++++++++-----------------
2 files changed, 33 insertions(+), 25 deletions(-)
diff --git a/cpu/lpc2387/include/periph_cpu.h b/cpu/lpc2387/include/periph_cpu.h
index 841b63bbf3..d5a99d5a4c 100644
--- a/cpu/lpc2387/include/periph_cpu.h
+++ b/cpu/lpc2387/include/periph_cpu.h
@@ -39,12 +39,22 @@ extern "C" {
* @brief Fast GPIO register definition struct
*/
typedef struct {
- __IO uint32_t DIR; /**< he */
- uint32_t _reserved[3]; /**< really */
- __IO uint32_t MASK; /**< wants */
- __IO uint32_t PIN; /**< to */
- __IO uint32_t SET; /**< know */
- __IO uint32_t CLR; /**< everything */
+ /** @brief Direction: Output if corresponding bit is set, otherwise input */
+ __IO uint32_t DIR;
+ /** @brief 12 bytes of reseved memory we don't need to access */
+ uint32_t _reserved[3];
+ /** @brief Set bits to ignore corresponding bits when accessing `PIN`, `SET`
+ * or `CLR` register of this port
+ */
+ __IO uint32_t MASK;
+ /** @brief The current state of each pin of this port is accessible here
+ * (regardless of direction): If bit is set input is high
+ */
+ __IO uint32_t PIN;
+ /** @brief Output pins are set to high by setting the corresponding bit */
+ __IO uint32_t SET;
+ /** @brief Output pins are set to low by setting the corresponding bit */
+ __IO uint32_t CLR;
} FIO_PORT_t;
#define FIO_PORTS ((FIO_PORT_t*)FIO_BASE_ADDR)
@@ -54,7 +64,7 @@ typedef struct {
int gpio_init_mux(unsigned pin, unsigned mux);
void gpio_init_states(void);
-#define GPIO_PIN(port, pin) (port*32 + pin)
+#define GPIO_PIN(port, pin) (port<<5 | pin)
#ifndef DOXYGEN
#define HAVE_GPIO_FLANK_T
diff --git a/cpu/lpc2387/periph/gpio.c b/cpu/lpc2387/periph/gpio.c
index a36e28fc5e..3891c50b7f 100644
--- a/cpu/lpc2387/periph/gpio.c
+++ b/cpu/lpc2387/periph/gpio.c
@@ -67,18 +67,19 @@ int gpio_init(gpio_t pin, gpio_mode_t mode)
{
unsigned _pin = pin & 31;
unsigned port = pin >> 5;
-
- if (mode != GPIO_OUT) {
- return -1;
- }
-
FIO_PORT_t *_port = &FIO_PORTS[port];
- /* set mask */
- _port->MASK = ~(0x1<<_pin);
-
/* set direction */
- _port->DIR = ~0;
+ switch (mode){
+ case GPIO_OUT:
+ _port->DIR |= 1<<_pin;
+ break;
+ case GPIO_IN:
+ _port->DIR &= ~(1<<_pin);
+ break;
+ default:
+ return -1;
+ }
gpio_init_mux(pin, 0);
@@ -88,13 +89,13 @@ int gpio_init(gpio_t pin, gpio_mode_t mode)
int gpio_init_mux(unsigned pin, unsigned mux)
{
(void) mux;
- unsigned _pin = pin & 31;
- PINSEL[pin>>4] &= ~(0x1 << (_pin*2));
+ unsigned pos = (pin & 0xf) << 1;
+ PINSEL[pin>>4] &= ~(0x3 << pos);
return 0;
}
int gpio_init_int(gpio_t pin, gpio_mode_t mode, gpio_flank_t flank,
- gpio_cb_t cb, void *arg)
+ gpio_cb_t cb, void *arg)
{
(void)mode;
@@ -222,8 +223,7 @@ int gpio_read(gpio_t pin)
unsigned _pin = pin & 31;
unsigned port = pin >> 5;
FIO_PORT_t *_port = &FIO_PORTS[port];
- _port->MASK = ~(1<<_pin);
- return _port->PIN != 0;
+ return (_port->PIN & (1 << _pin)) != 0;
}
void gpio_set(gpio_t pin)
@@ -231,8 +231,7 @@ void gpio_set(gpio_t pin)
unsigned _pin = pin & 31;
unsigned port = pin >> 5;
FIO_PORT_t *_port = &FIO_PORTS[port];
- _port->MASK = ~(1<<_pin);
- _port->SET = ~0;
+ _port->SET = 1 << _pin;
}
void gpio_clear(gpio_t pin)
@@ -240,8 +239,7 @@ void gpio_clear(gpio_t pin)
unsigned _pin = pin & 31;
unsigned port = pin >> 5;
FIO_PORT_t *_port = &FIO_PORTS[port];
- _port->MASK = ~(1<<_pin);
- _port->CLR = ~0;
+ _port->CLR = 1 << _pin;
}
void gpio_toggle(gpio_t dev)
--
GitLab