Skip to content
Snippets Groups Projects
Unverified Commit 1d0f90dc authored by Marian Buschsieweke's avatar Marian Buschsieweke
Browse files

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
parent 99460669
No related branches found
No related tags found
No related merge requests found
...@@ -39,12 +39,22 @@ extern "C" { ...@@ -39,12 +39,22 @@ extern "C" {
* @brief Fast GPIO register definition struct * @brief Fast GPIO register definition struct
*/ */
typedef struct { typedef struct {
__IO uint32_t DIR; /**< he */ /** @brief Direction: Output if corresponding bit is set, otherwise input */
uint32_t _reserved[3]; /**< really */ __IO uint32_t DIR;
__IO uint32_t MASK; /**< wants */ /** @brief 12 bytes of reseved memory we don't need to access */
__IO uint32_t PIN; /**< to */ uint32_t _reserved[3];
__IO uint32_t SET; /**< know */ /** @brief Set bits to ignore corresponding bits when accessing `PIN`, `SET`
__IO uint32_t CLR; /**< everything */ * 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; } FIO_PORT_t;
#define FIO_PORTS ((FIO_PORT_t*)FIO_BASE_ADDR) #define FIO_PORTS ((FIO_PORT_t*)FIO_BASE_ADDR)
...@@ -54,7 +64,7 @@ typedef struct { ...@@ -54,7 +64,7 @@ typedef struct {
int gpio_init_mux(unsigned pin, unsigned mux); int gpio_init_mux(unsigned pin, unsigned mux);
void gpio_init_states(void); void gpio_init_states(void);
#define GPIO_PIN(port, pin) (port*32 + pin) #define GPIO_PIN(port, pin) (port<<5 | pin)
#ifndef DOXYGEN #ifndef DOXYGEN
#define HAVE_GPIO_FLANK_T #define HAVE_GPIO_FLANK_T
......
...@@ -67,18 +67,19 @@ int gpio_init(gpio_t pin, gpio_mode_t mode) ...@@ -67,18 +67,19 @@ int gpio_init(gpio_t pin, gpio_mode_t mode)
{ {
unsigned _pin = pin & 31; unsigned _pin = pin & 31;
unsigned port = pin >> 5; unsigned port = pin >> 5;
if (mode != GPIO_OUT) {
return -1;
}
FIO_PORT_t *_port = &FIO_PORTS[port]; FIO_PORT_t *_port = &FIO_PORTS[port];
/* set mask */
_port->MASK = ~(0x1<<_pin);
/* set direction */ /* 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); gpio_init_mux(pin, 0);
...@@ -88,13 +89,13 @@ int gpio_init(gpio_t pin, gpio_mode_t mode) ...@@ -88,13 +89,13 @@ int gpio_init(gpio_t pin, gpio_mode_t mode)
int gpio_init_mux(unsigned pin, unsigned mux) int gpio_init_mux(unsigned pin, unsigned mux)
{ {
(void) mux; (void) mux;
unsigned _pin = pin & 31; unsigned pos = (pin & 0xf) << 1;
PINSEL[pin>>4] &= ~(0x1 << (_pin*2)); PINSEL[pin>>4] &= ~(0x3 << pos);
return 0; return 0;
} }
int gpio_init_int(gpio_t pin, gpio_mode_t mode, gpio_flank_t flank, 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; (void)mode;
...@@ -222,8 +223,7 @@ int gpio_read(gpio_t pin) ...@@ -222,8 +223,7 @@ int gpio_read(gpio_t pin)
unsigned _pin = pin & 31; unsigned _pin = pin & 31;
unsigned port = pin >> 5; unsigned port = pin >> 5;
FIO_PORT_t *_port = &FIO_PORTS[port]; FIO_PORT_t *_port = &FIO_PORTS[port];
_port->MASK = ~(1<<_pin); return (_port->PIN & (1 << _pin)) != 0;
return _port->PIN != 0;
} }
void gpio_set(gpio_t pin) void gpio_set(gpio_t pin)
...@@ -231,8 +231,7 @@ void gpio_set(gpio_t pin) ...@@ -231,8 +231,7 @@ void gpio_set(gpio_t pin)
unsigned _pin = pin & 31; unsigned _pin = pin & 31;
unsigned port = pin >> 5; unsigned port = pin >> 5;
FIO_PORT_t *_port = &FIO_PORTS[port]; FIO_PORT_t *_port = &FIO_PORTS[port];
_port->MASK = ~(1<<_pin); _port->SET = 1 << _pin;
_port->SET = ~0;
} }
void gpio_clear(gpio_t pin) void gpio_clear(gpio_t pin)
...@@ -240,8 +239,7 @@ void gpio_clear(gpio_t pin) ...@@ -240,8 +239,7 @@ void gpio_clear(gpio_t pin)
unsigned _pin = pin & 31; unsigned _pin = pin & 31;
unsigned port = pin >> 5; unsigned port = pin >> 5;
FIO_PORT_t *_port = &FIO_PORTS[port]; FIO_PORT_t *_port = &FIO_PORTS[port];
_port->MASK = ~(1<<_pin); _port->CLR = 1 << _pin;
_port->CLR = ~0;
} }
void gpio_toggle(gpio_t dev) void gpio_toggle(gpio_t dev)
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment