From 300d2d79ae8ec22398c223e32703fa4aada00566 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Th=C3=A9baudeau?= Date: Fri, 15 Nov 2013 19:43:06 +0100 Subject: [PATCH 1/3] cc2538: gpio: Fix usage of parameters in macros MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The parameters in the GPIO macros were used without being parenthesized. This could generate wrong values for register assignments in the case of expressions passed as arguments to these macros. Signed-off-by: Benoît Thébaudeau --- cpu/cc2538/dev/gpio.h | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/cpu/cc2538/dev/gpio.h b/cpu/cc2538/dev/gpio.h index 84887cf18..eb2851270 100644 --- a/cpu/cc2538/dev/gpio.h +++ b/cpu/cc2538/dev/gpio.h @@ -91,42 +91,42 @@ typedef void (* gpio_callback_t)(uint8_t port, uint8_t pin); * \param PIN_MASK Pin number mask. Pin 0: 0x01, Pin 1: 0x02 ... Pin 7: 0x80 */ #define GPIO_SET_INPUT(PORT_BASE, PIN_MASK) \ - do { REG(PORT_BASE | GPIO_DIR) &= ~PIN_MASK; } while(0) + do { REG((PORT_BASE) | GPIO_DIR) &= ~(PIN_MASK); } while(0) /** \brief Set pins with PIN_MASK of port with PORT_BASE to output. * \param PORT_BASE GPIO Port register offset * \param PIN_MASK Pin number mask. Pin 0: 0x01, Pin 1: 0x02 ... Pin 7: 0x80 */ #define GPIO_SET_OUTPUT(PORT_BASE, PIN_MASK) \ - do { REG(PORT_BASE | GPIO_DIR) |= PIN_MASK; } while(0) + do { REG((PORT_BASE) | GPIO_DIR) |= (PIN_MASK); } while(0) /** \brief Set pins with PIN_MASK of port with PORT_BASE high. * \param PORT_BASE GPIO Port register offset * \param PIN_MASK Pin number mask. Pin 0: 0x01, Pin 1: 0x02 ... Pin 7: 0x80 */ #define GPIO_SET_PIN(PORT_BASE, PIN_MASK) \ - do { REG((PORT_BASE | GPIO_DATA) + (PIN_MASK << 2)) = 0xFF; } while(0) + do { REG(((PORT_BASE) | GPIO_DATA) + ((PIN_MASK) << 2)) = 0xFF; } while(0) /** \brief Set pins with PIN_MASK of port with PORT_BASE low. * \param PORT_BASE GPIO Port register offset * \param PIN_MASK Pin number mask. Pin 0: 0x01, Pin 1: 0x02 ... Pin 7: 0x80 */ #define GPIO_CLR_PIN(PORT_BASE, PIN_MASK) \ - do { REG((PORT_BASE | GPIO_DATA) + (PIN_MASK << 2)) = 0x00; } while(0) + do { REG(((PORT_BASE) | GPIO_DATA) + ((PIN_MASK) << 2)) = 0x00; } while(0) /** \brief Set pins with PIN_MASK of port with PORT_BASE to detect edge. * \param PORT_BASE GPIO Port register offset * \param PIN_MASK Pin number mask. Pin 0: 0x01, Pin 1: 0x02 ... Pin 7: 0x80 */ #define GPIO_DETECT_EDGE(PORT_BASE, PIN_MASK) \ - do { REG(PORT_BASE | GPIO_IS) &= ~PIN_MASK; } while(0) + do { REG((PORT_BASE) | GPIO_IS) &= ~(PIN_MASK); } while(0) /** \brief Set pins with PIN_MASK of port with PORT_BASE to detect level. * \param PORT_BASE GPIO Port register offset * \param PIN_MASK Pin number mask. Pin 0: 0x01, Pin 1: 0x02 ... Pin 7: 0x80 */ #define GPIO_DETECT_LEVEL(PORT_BASE, PIN_MASK) \ - do { REG(PORT_BASE | GPIO_IS) |= PIN_MASK; } while(0) + do { REG((PORT_BASE) | GPIO_IS) |= (PIN_MASK); } while(0) /** \brief Set pins with PIN_MASK of port with PORT_BASE to trigger an * interrupt on both edges. @@ -134,7 +134,7 @@ typedef void (* gpio_callback_t)(uint8_t port, uint8_t pin); * \param PIN_MASK Pin number mask. Pin 0: 0x01, Pin 1: 0x02 ... Pin 7: 0x80 */ #define GPIO_TRIGGER_BOTH_EDGES(PORT_BASE, PIN_MASK) \ - do { REG(PORT_BASE | GPIO_IBE) |= PIN_MASK; } while(0) + do { REG((PORT_BASE) | GPIO_IBE) |= (PIN_MASK); } while(0) /** \brief Set pins with PIN_MASK of port with PORT_BASE to trigger an * interrupt on single edge (controlled by GPIO_IEV). @@ -142,7 +142,7 @@ typedef void (* gpio_callback_t)(uint8_t port, uint8_t pin); * \param PIN_MASK Pin number mask. Pin 0: 0x01, Pin 1: 0x02 ... Pin 7: 0x80 */ #define GPIO_TRIGGER_SINGLE_EDGE(PORT_BASE, PIN_MASK) \ - do { REG(PORT_BASE | GPIO_IBE) &= ~PIN_MASK; } while(0) + do { REG((PORT_BASE) | GPIO_IBE) &= ~(PIN_MASK); } while(0) /** \brief Set pins with PIN_MASK of port with PORT_BASE to trigger an * interrupt on rising edge. @@ -150,7 +150,7 @@ typedef void (* gpio_callback_t)(uint8_t port, uint8_t pin); * \param PIN_MASK Pin number mask. Pin 0: 0x01, Pin 1: 0x02 ... Pin 7: 0x80 */ #define GPIO_DETECT_RISING(PORT_BASE, PIN_MASK) \ - do { REG(PORT_BASE | GPIO_IEV) |= PIN_MASK; } while(0) + do { REG((PORT_BASE) | GPIO_IEV) |= (PIN_MASK); } while(0) /** \brief Set pins with PIN_MASK of port with PORT_BASE to trigger an * interrupt on falling edge. @@ -158,7 +158,7 @@ typedef void (* gpio_callback_t)(uint8_t port, uint8_t pin); * \param PIN_MASK Pin number mask. Pin 0: 0x01, Pin 1: 0x02 ... Pin 7: 0x80 */ #define GPIO_DETECT_FALLING(PORT_BASE, PIN_MASK) \ - do { REG(PORT_BASE | GPIO_IEV) &= ~PIN_MASK; } while(0) + do { REG((PORT_BASE) | GPIO_IEV) &= ~(PIN_MASK); } while(0) /** \brief Enable interrupt triggering for pins with PIN_MASK of port with * PORT_BASE. @@ -166,7 +166,7 @@ typedef void (* gpio_callback_t)(uint8_t port, uint8_t pin); * \param PIN_MASK Pin number mask. Pin 0: 0x01, Pin 1: 0x02 ... Pin 7: 0x80 */ #define GPIO_ENABLE_INTERRUPT(PORT_BASE, PIN_MASK) \ - do { REG(PORT_BASE | GPIO_IE) |= PIN_MASK; } while(0) + do { REG((PORT_BASE) | GPIO_IE) |= (PIN_MASK); } while(0) /** \brief Disable interrupt triggering for pins with PIN_MASK of port with * PORT_BASE. @@ -174,7 +174,7 @@ typedef void (* gpio_callback_t)(uint8_t port, uint8_t pin); * \param PIN_MASK Pin number mask. Pin 0: 0x01, Pin 1: 0x02 ... Pin 7: 0x80 */ #define GPIO_DISABLE_INTERRUPT(PORT_BASE, PIN_MASK) \ - do { REG(PORT_BASE | GPIO_IE) &= ~PIN_MASK; } while(0) + do { REG((PORT_BASE) | GPIO_IE) &= ~(PIN_MASK); } while(0) /** \brief Configure the pin to be under peripheral control with PIN_MASK of * port with PORT_BASE. @@ -182,7 +182,7 @@ typedef void (* gpio_callback_t)(uint8_t port, uint8_t pin); * \param PIN_MASK Pin number mask. Pin 0: 0x01, Pin 1: 0x02 ... Pin 7: 0x80 */ #define GPIO_PERIPHERAL_CONTROL(PORT_BASE, PIN_MASK) \ - do { REG(PORT_BASE | GPIO_AFSEL) |= PIN_MASK; } while(0) + do { REG((PORT_BASE) | GPIO_AFSEL) |= (PIN_MASK); } while(0) /** \brief Configure the pin to be software controlled with PIN_MASK of port * with PORT_BASE. @@ -190,7 +190,7 @@ typedef void (* gpio_callback_t)(uint8_t port, uint8_t pin); * \param PIN_MASK Pin number mask. Pin 0: 0x01, Pin 1: 0x02 ... Pin 7: 0x80 */ #define GPIO_SOFTWARE_CONTROL(PORT_BASE, PIN_MASK) \ - do { REG(PORT_BASE | GPIO_AFSEL) &= ~PIN_MASK; } while(0) + do { REG((PORT_BASE) | GPIO_AFSEL) &= ~(PIN_MASK); } while(0) /** * \brief Converts a pin number to a pin mask @@ -198,7 +198,7 @@ typedef void (* gpio_callback_t)(uint8_t port, uint8_t pin); * \return A pin mask which can be used as the PIN_MASK argument of the macros * in this category */ -#define GPIO_PIN_MASK(PIN) (1 << PIN) +#define GPIO_PIN_MASK(PIN) (1 << (PIN)) /** * \brief Converts a port number to the port base address @@ -206,7 +206,7 @@ typedef void (* gpio_callback_t)(uint8_t port, uint8_t pin); * \return The base address for the registers corresponding to that port * number. */ -#define GPIO_PORT_TO_BASE(PORT) (GPIO_A_BASE + (PORT << 12)) +#define GPIO_PORT_TO_BASE(PORT) (GPIO_A_BASE + ((PORT) << 12)) /** @} */ /*---------------------------------------------------------------------------*/ /** \name GPIO Register offset declarations From 923f161b7b4da5a8833a9b95fc7faf7297eef1e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Th=C3=A9baudeau?= Date: Fri, 15 Nov 2013 19:48:24 +0100 Subject: [PATCH 2/3] cc2538: gpio: Add pin read / write and interrupt clear macros MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduce new useful GPIO macros to: - read the levels of some port pins, - write the levels of some port pins (pass bit-field value to be set), - clear the interrupt flags for some port pins. These macros are cleaner and less error prone than raw register access code copied all over the place. Signed-off-by: Benoît Thébaudeau --- cpu/cc2538/dev/gpio.h | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/cpu/cc2538/dev/gpio.h b/cpu/cc2538/dev/gpio.h index eb2851270..c81482c9e 100644 --- a/cpu/cc2538/dev/gpio.h +++ b/cpu/cc2538/dev/gpio.h @@ -114,6 +114,20 @@ typedef void (* gpio_callback_t)(uint8_t port, uint8_t pin); #define GPIO_CLR_PIN(PORT_BASE, PIN_MASK) \ do { REG(((PORT_BASE) | GPIO_DATA) + ((PIN_MASK) << 2)) = 0x00; } while(0) +/** \brief Set pins with PIN_MASK of port with PORT_BASE to value. + * \param PORT_BASE GPIO Port register offset + * \param PIN_MASK Pin number mask. Pin 0: 0x01, Pin 1: 0x02 ... Pin 7: 0x80 + */ +#define GPIO_WRITE_PIN(PORT_BASE, PIN_MASK, value) \ + do { REG(((PORT_BASE) | GPIO_DATA) + ((PIN_MASK) << 2)) = (value); } while(0) + +/** \brief Read pins with PIN_MASK of port with PORT_BASE. + * \param PORT_BASE GPIO Port register offset + * \param PIN_MASK Pin number mask. Pin 0: 0x01, Pin 1: 0x02 ... Pin 7: 0x80 + */ +#define GPIO_READ_PIN(PORT_BASE, PIN_MASK) \ + REG(((PORT_BASE) | GPIO_DATA) + ((PIN_MASK) << 2)) + /** \brief Set pins with PIN_MASK of port with PORT_BASE to detect edge. * \param PORT_BASE GPIO Port register offset * \param PIN_MASK Pin number mask. Pin 0: 0x01, Pin 1: 0x02 ... Pin 7: 0x80 @@ -176,6 +190,14 @@ typedef void (* gpio_callback_t)(uint8_t port, uint8_t pin); #define GPIO_DISABLE_INTERRUPT(PORT_BASE, PIN_MASK) \ do { REG((PORT_BASE) | GPIO_IE) &= ~(PIN_MASK); } while(0) +/** \brief Clear interrupt triggering for pins with PIN_MASK of port with + * PORT_BASE. + * \param PORT_BASE GPIO Port register offset + * \param PIN_MASK Pin number mask. Pin 0: 0x01, Pin 1: 0x02 ... Pin 7: 0x80 + */ +#define GPIO_CLEAR_INTERRUPT(PORT_BASE, PIN_MASK) \ + do { REG((PORT_BASE) | GPIO_IC) = (PIN_MASK); } while(0) + /** \brief Configure the pin to be under peripheral control with PIN_MASK of * port with PORT_BASE. * \param PORT_BASE GPIO Port register offset From 680050861c0f6b1b3e8ddfe378e3c1049a63ebc8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Th=C3=A9baudeau?= Date: Fri, 15 Nov 2013 19:57:44 +0100 Subject: [PATCH 3/3] cc2538: gpio: Use accessor macros MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use the GPIO accessor macros instead of copying raw register access code all over the place. This is cleaner and less error prone. This fixes the setting of the USB pull-up resistor that worked only by chance on the CC2538DK because it is controlled by the pin 0 of the used GPIO port. Signed-off-by: Benoît Thébaudeau --- cpu/cc2538/dev/gpio.c | 8 ++++---- cpu/cc2538/usb/usb-arch.c | 2 +- platform/cc2538dk/dev/board.h | 12 ++++++------ platform/cc2538dk/dev/leds-arch.c | 5 ++--- 4 files changed, 13 insertions(+), 14 deletions(-) diff --git a/cpu/cc2538/dev/gpio.c b/cpu/cc2538/dev/gpio.c index e7f45d5cf..80a1b7d8d 100644 --- a/cpu/cc2538/dev/gpio.c +++ b/cpu/cc2538/dev/gpio.c @@ -89,7 +89,7 @@ gpio_port_a_isr() notify(REG(GPIO_A_BASE | GPIO_MIS), GPIO_A_NUM); - REG(GPIO_A_BASE | GPIO_IC) = 0xFF; + GPIO_CLEAR_INTERRUPT(GPIO_A_BASE, 0xFF); ENERGEST_OFF(ENERGEST_TYPE_IRQ); } @@ -104,7 +104,7 @@ gpio_port_b_isr() notify(REG(GPIO_B_BASE | GPIO_MIS), GPIO_B_NUM); - REG(GPIO_B_BASE | GPIO_IC) = 0xFF; + GPIO_CLEAR_INTERRUPT(GPIO_B_BASE, 0xFF); ENERGEST_OFF(ENERGEST_TYPE_IRQ); } @@ -119,7 +119,7 @@ gpio_port_c_isr() notify(REG(GPIO_C_BASE | GPIO_MIS), GPIO_C_NUM); - REG(GPIO_C_BASE | GPIO_IC) = 0xFF; + GPIO_CLEAR_INTERRUPT(GPIO_C_BASE, 0xFF); ENERGEST_OFF(ENERGEST_TYPE_IRQ); } @@ -134,7 +134,7 @@ gpio_port_d_isr() notify(REG(GPIO_D_BASE | GPIO_MIS), GPIO_D_NUM); - REG(GPIO_D_BASE | GPIO_IC) = 0xFF; + GPIO_CLEAR_INTERRUPT(GPIO_D_BASE, 0xFF); ENERGEST_OFF(ENERGEST_TYPE_IRQ); } diff --git a/cpu/cc2538/usb/usb-arch.c b/cpu/cc2538/usb/usb-arch.c index 5ed1eee8e..3318f1f42 100644 --- a/cpu/cc2538/usb/usb-arch.c +++ b/cpu/cc2538/usb/usb-arch.c @@ -317,7 +317,7 @@ usb_arch_setup(void) /* Enable pull-up on usb port */ GPIO_SET_OUTPUT(USB_PULLUP_PORT, USB_PULLUP_PIN_MASK); - REG((USB_PULLUP_PORT | GPIO_DATA) + (USB_PULLUP_PIN_MASK << 2)) = 1; + GPIO_SET_PIN(USB_PULLUP_PORT, USB_PULLUP_PIN_MASK); for(i = 0; i < USB_MAX_ENDPOINTS; i++) { usb_endpoints[i].flags = 0; diff --git a/platform/cc2538dk/dev/board.h b/platform/cc2538dk/dev/board.h index 992c3c012..1a4703392 100644 --- a/platform/cc2538dk/dev/board.h +++ b/platform/cc2538dk/dev/board.h @@ -99,7 +99,7 @@ */ #define USB_PULLUP_PORT GPIO_C_BASE #define USB_PULLUP_PIN 0 -#define USB_PULLUP_PIN_MASK (1 << USB_PULLUP_PIN) +#define USB_PULLUP_PIN_MASK GPIO_PIN_MASK(USB_PULLUP_PIN) /** @} */ /*---------------------------------------------------------------------------*/ /** \name UART configuration @@ -143,35 +143,35 @@ #define BUTTON_SELECT_PORT_NO GPIO_A_NUM #define BUTTON_SELECT_PIN 3 #define BUTTON_SELECT_PORT GPIO_A_BASE -#define BUTTON_SELECT_PIN_MASK (1 << BUTTON_SELECT_PIN) +#define BUTTON_SELECT_PIN_MASK GPIO_PIN_MASK(BUTTON_SELECT_PIN) #define BUTTON_SELECT_VECTOR NVIC_INT_GPIO_PORT_A /** BUTTON_LEFT -> PC4 */ #define BUTTON_LEFT_PORT_NO GPIO_C_NUM #define BUTTON_LEFT_PIN 4 #define BUTTON_LEFT_PORT GPIO_C_BASE -#define BUTTON_LEFT_PIN_MASK (1 << BUTTON_LEFT_PIN) +#define BUTTON_LEFT_PIN_MASK GPIO_PIN_MASK(BUTTON_LEFT_PIN) #define BUTTON_LEFT_VECTOR NVIC_INT_GPIO_PORT_C /** BUTTON_RIGHT -> PC5 */ #define BUTTON_RIGHT_PORT_NO GPIO_C_NUM #define BUTTON_RIGHT_PIN 5 #define BUTTON_RIGHT_PORT GPIO_C_BASE -#define BUTTON_RIGHT_PIN_MASK (1 << BUTTON_RIGHT_PIN) +#define BUTTON_RIGHT_PIN_MASK GPIO_PIN_MASK(BUTTON_RIGHT_PIN) #define BUTTON_RIGHT_VECTOR NVIC_INT_GPIO_PORT_C /** BUTTON_UP -> PC6 */ #define BUTTON_UP_PORT_NO GPIO_C_NUM #define BUTTON_UP_PIN 6 #define BUTTON_UP_PORT GPIO_C_BASE -#define BUTTON_UP_PIN_MASK (1 << BUTTON_UP_PIN) +#define BUTTON_UP_PIN_MASK GPIO_PIN_MASK(BUTTON_UP_PIN) #define BUTTON_UP_VECTOR NVIC_INT_GPIO_PORT_C /** BUTTON_DOWN -> PC7 */ #define BUTTON_DOWN_PORT_NO GPIO_C_NUM #define BUTTON_DOWN_PIN 7 #define BUTTON_DOWN_PORT GPIO_C_BASE -#define BUTTON_DOWN_PIN_MASK (1 << BUTTON_DOWN_PIN) +#define BUTTON_DOWN_PIN_MASK GPIO_PIN_MASK(BUTTON_DOWN_PIN) #define BUTTON_DOWN_VECTOR NVIC_INT_GPIO_PORT_C /* Notify various examples that we have Buttons */ diff --git a/platform/cc2538dk/dev/leds-arch.c b/platform/cc2538dk/dev/leds-arch.c index 247147d9a..661f46160 100644 --- a/platform/cc2538dk/dev/leds-arch.c +++ b/platform/cc2538dk/dev/leds-arch.c @@ -45,7 +45,6 @@ #include "dev/leds.h" #include "dev/gpio.h" -#define LEDS_GPIO_DATA_MASK (LEDS_ALL << 2) #define LEDS_GPIO_PIN_MASK LEDS_ALL /*---------------------------------------------------------------------------*/ void @@ -57,13 +56,13 @@ leds_arch_init(void) unsigned char leds_arch_get(void) { - return REG((GPIO_C_BASE | GPIO_DATA) + LEDS_GPIO_DATA_MASK); + return GPIO_READ_PIN(GPIO_C_BASE, LEDS_GPIO_PIN_MASK); } /*---------------------------------------------------------------------------*/ void leds_arch_set(unsigned char leds) { - REG((GPIO_C_BASE | GPIO_DATA) + LEDS_GPIO_DATA_MASK) = leds; + GPIO_WRITE_PIN(GPIO_C_BASE, LEDS_GPIO_PIN_MASK, leds); } /*---------------------------------------------------------------------------*/